Re: [Qemu-devel] [PATCH v2] ossaudio: fix memory leak

[Markus Armbruster]

 writes:

> From: Gonglei 
>
> Variable "conf" going out of scope leaks the storage
> it points to in line 856.
>
> Signed-off-by: Gonglei 
> ---
> v2:
>  using an better way to avoid memory leak. (Markus) 
> ---
>  audio/ossaudio.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/audio/ossaudio.c b/audio/ossaudio.c
> index 11e76a1..94b473b 100644
> --- a/audio/ossaudio.c
> +++ b/audio/ossaudio.c
> @@ -848,14 +848,11 @@ static OSSConf glob_conf = {
>  
>  static void *oss_audio_init (void)
>  {
> -OSSConf *conf = g_malloc(sizeof(OSSConf));
> -*conf = glob_conf;
> -
> -if (access(conf->devpath_in, R_OK | W_OK) < 0 ||
> -access(conf->devpath_out, R_OK | W_OK) < 0) {
> +if (access(glob_conf.devpath_in, R_OK | W_OK) < 0 ||
> +access(glob_conf.devpath_out, R_OK | W_OK) < 0) {
>  return NULL;
>  }
> -return conf;
> +return g_memdup(&glob_conf, sizeof(glob_conf));
>  }
>  
>  static void oss_audio_fini (void *opaque)

Reviewed-by: Markus Armbruster 

Now I get to add g_memdup() to scripts/coverity-model.c...

Re: [Qemu-devel] [PATCH] block.c: fix real cdrom detection

[Markus Armbruster]

Programmingkid  writes:

> On Jun 23, 2015, at 2:06 PM, John Snow wrote:
>
>> 
>> 
>> On 06/23/2015 01:56 PM, Programmingkid wrote:
>>> Fix real cdrom detection so that a real cdrom can actually be used.
>>> 
>>> signed-off-by: John Arbuckle >> >
>>> 
>>> This patch has been tested on Mac OS X host and guest. 
>>> Command used: qemu-system-ppc -cdrom /dev/cdrom
>>> 
>>> Note: I was able to view the files using OpenBIOS, but not on 
>>> Mac OS X. The size of the disc is reported correctly but some
>>> error happens that prevents it from mounting in Mac OS X. This
>>> is probably another bug with QEMU.
>>> 
>>> ---
>>> block.c |3 ++-
>>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>> 
>>> diff --git a/block.c b/block.c
>>> index dd4f58d..75ccfad 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs,
>>> const char *filename,
>>> int ret = 0;
>>> 
>>> 
>>> 
>>> /* Return the raw BlockDriver * to scsi-generic devices or empty
>>> drives */
>>> -if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
>>> +if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 
>>> +   || strcmp("/dev/cdrom", filename) == 0) {
>>> *pdrv = &bdrv_raw;
>>> return ret;
>>> }
>>> -- 
>>> 1.7.5.4
>>> 
>> 
>> So what's the issue that this patch attempts to fix and how did you
>> determine that the fix was needed here? It doesn't look like it respects
>> proper abstraction at a glance.
>
> Without the patch, QEMU would just quit when the "-cdrom /dev/cdrom"
> option is given.
>
> Before the patch, the bdrv_open_inherit() function would be
> incorrectly called. Its documentation says "Opens a disk image (raw,
> qcow2, vmdk, ...)" meaning only for disk image files (not for real
> media). This patch prevents the bdrv_open_inherit() function from ever
> being called. It sets the pdrv variable to the raw format. This made
> sense to me since a real cdrom is read in the raw format.
>
> A quick test does show the patch works. A real cdrom is successfully
> opened on qemu-system-i386 using a Windows XP guest.

What about /dev/sr0, /dev/sr1, and whatever other names could refer to a
block device without a medium?

Comparing filenames isn't a good way to test "is a block device without
a medium".

[Qemu-devel] [PATCH] more check for replaced node

[Wen Congyang]

Signed-off-by: Wen Congyang 
---
 block.c   | 5 +++--
 block/mirror.c| 3 ++-
 blockdev.c| 2 +-
 include/block/block.h | 3 ++-
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 7168575..70ee0f6 100644
--- a/block.c
+++ b/block.c
@@ -4033,7 +4033,8 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate)
 return false;
 }
 
-BlockDriverState *check_to_replace_node(const char *node_name, Error **errp)
+BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs,
+const char *node_name, Error **errp)
 {
 BlockDriverState *to_replace_bs = bdrv_find_node(node_name);
 AioContext *aio_context;
@@ -4056,7 +4057,7 @@ BlockDriverState *check_to_replace_node(const char 
*node_name, Error **errp)
  * Another benefit is that this tests exclude backing files which are
  * blocked by the backing blockers.
  */
-if (!bdrv_is_first_non_filter(to_replace_bs)) {
+if (!bdrv_recurse_is_first_non_filter(parent_bs, to_replace_bs)) {
 error_setg(errp, "Only top most non filter can be replaced");
 to_replace_bs = NULL;
 goto out;
diff --git a/block/mirror.c b/block/mirror.c
index 0d06cc2..f132f35 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -612,7 +612,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
 if (s->replaces) {
 AioContext *replace_aio_context;
 
-s->to_replace = check_to_replace_node(s->replaces, &local_err);
+s->to_replace = check_to_replace_node(s->common.bs, s->replaces,
+  &local_err);
 if (!s->to_replace) {
 error_propagate(errp, local_err);
 return;
diff --git a/blockdev.c b/blockdev.c
index f3a3097..a0e13b0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2756,7 +2756,7 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 goto out;
 }
 
-to_replace_bs = check_to_replace_node(replaces, &local_err);
+to_replace_bs = check_to_replace_node(bs, replaces, &local_err);
 
 if (!to_replace_bs) {
 error_propagate(errp, local_err);
diff --git a/include/block/block.h b/include/block/block.h
index 07bb724..e0dad54 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -311,7 +311,8 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
 bool bdrv_is_first_non_filter(BlockDriverState *candidate);
 
 /* check if a named node can be replaced when doing drive-mirror */
-BlockDriverState *check_to_replace_node(const char *node_name, Error **errp);
+BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs,
+const char *node_name, Error **errp);
 
 /* async block I/O */
 typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,

Re: [Qemu-devel] [PATCH v2] ossaudio: fix memory leak

[Gerd Hoffmann]

On Mi, 2015-06-24 at 17:18 +0800, arei.gong...@huawei.com wrote:
> Variable "conf" going out of scope leaks the storage
> it points to in line 856.
> 
Added to audio queue.

thanks,
  Gerd

Re: [Qemu-devel] TCG baremetal tests repo

[Alex Bennée]

Peter Maydell  writes:

> On 24 June 2015 at 17:39, Alex Bennée  wrote:
>>
>> Alexander Spyridakis  writes:
>>> You can find the latest tcg atomic test payload in the following repo:
 git clone https://git.virtualopensystems.com/dev/tcg_baremetal_tests.git
>>>
>>> You also need an arm baremetal cross-compiler like arm-none-gnueabi- (arm)
>>> and the usual aarch64-linux-gnu- (arm64). Due to a PSCI bug in the current
>>> multithreading tcg repo, the atomic test was modified to work also on the
>>> vexpress machine model.
>>
>> I sent a patch to fix the PSCI hang case to wake up the sleeping CPU. I
>> couldn't figure out how the vexpress code was waking it's CPUs. Do they
>> just start powered on?
>
> Yes -- our vexpress model is "like hardware", so all CPUs leap into
> the boot firmware at once, and the firmware deals with putting the
> secondaries into a pen to be released in a controlled manner later.
> [assuming you're not using -kernel; if you are then boot.c has the
> secondary pen code]

Ahh good. No mystery to solve then ;-)

-- 
Alex Bennée

[Qemu-devel] [PATCH] refresh filename after the node is replaced

[Wen Congyang]

We can use block job mirror to repair broken quorum files. But the command
'info block' doesn't output correct filename after block job mirror finishes.

Signed-off-by: Wen Congyang 
---
 block/mirror.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index 8aa2b21..2ca2c21 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -351,6 +351,9 @@ static void mirror_exit(BlockJob *job, void *opaque)
 bdrv_set_backing_hd(s->base, NULL);
 bdrv_unref(p);
 }
+if (s->to_replace != s->common.bs) {
+bdrv_refresh_filename(s->common.bs);
+}
 }
 if (s->to_replace) {
 bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
-- 
2.4.3

Re: [Qemu-devel] [PATCH] block/iscsi: add support for request timeouts

[Peter Lieven]

Am 23.06.2015 um 01:03 schrieb ronnie sahlberg:

LGTM

It is good to finally have timeouts that work in libiscsi,  and a consumer that 
can use and benefit from it.


Paolo, Kevin, Stefan, do you think this is sth for 2.4?

Peter

Re: [Qemu-devel] [PATCH V2] Re-attach usb device to kernel while usb_host_open fails

[Gerd Hoffmann]

On Mi, 2015-06-24 at 13:40 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> Reviewed-by: Gonglei 

Added to usb patch queue.

thanks,
  Gerd

Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)

[Alex Bennée]

Alexander Spyridakis  writes:

> On 24 June 2015 at 17:34, Alex Bennée  wrote:
>> Testing with Alexander's bare metal syncronisation tests fails in MTTCG
>> leaving one CPU spinning forever waiting for the second CPU to wake up.
>> We simply need to poke the halt_cond once we have processed the PSCI
>> power on call.
>
> Thanks Alex. Works for me, also with qemu_cpu_kick(target_cpu_state)
> as Paolo mentioned.
>
> The test seems to stress the current multi-threaded implementation
> quite a lot. With 8 CPUs running, the resulting errors are in the
> range of 500 per vCPU (10 million iterations).

We need to get to the bottom of this one first as obviously the
implementation needs to bullet proof for all the various synchronisation
patterns the CPU can use.

> Performance is another issue as mentioned before, but even more
> pronounced with 8 cores. Upstream QEMU needs around 10 seconds to
> complete, with multi-threading around 100 seconds for the same test.

I'm not overly surprised as this is a high-contention test and the
additional locking implies a lot of extra overhead. It's certainly a
useful test to compare the comparative performance of the various
approaches to atomics/exclusives but I hope in real world tasks we gain
a bunch of performance for normal unlocked code running across multiple
cores.

I wonder if the perf tools can give us some insight to where the extra
latency is coming from?

>
> Best regards.

-- 
Alex Bennée

[Qemu-devel] [PATCH] rocker: fix memory leak

[arei.gonglei]

From: Gonglei 

Meanwhile, using g_new0 instead of g_malloc0,
refer to commit 5839e53.

Signed-off-by: Gonglei 
---
 hw/net/rocker/rocker.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 4d25842..7e06015 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -96,7 +96,7 @@ World *rocker_get_world(Rocker *r, enum rocker_world_type 
type)
 
 RockerSwitch *qmp_query_rocker(const char *name, Error **errp)
 {
-RockerSwitch *rocker = g_malloc0(sizeof(*rocker));
+RockerSwitch *rocker;
 Rocker *r;
 
 r = rocker_find(name);
@@ -106,6 +106,7 @@ RockerSwitch *qmp_query_rocker(const char *name, Error 
**errp)
 return NULL;
 }
 
+rocker = g_new0(RockerSwitch, 1);
 rocker->name = g_strdup(r->name);
 rocker->id = r->switch_id;
 rocker->ports = r->fp_ports;
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 0/2] Remove CP15 timer from the device tree if KVM is used without in-kernel irqchip

[Peter Crosthwaite]

On Wed, Jun 24, 2015 at 4:58 AM, Pavel Fedin  wrote:
> Certain machines do not have working vGIC hardware. Linux kernel (at least
> up to v4) has configuration options which would still allow to use KVM,
> but GIC and timer have to be emulated in userspace. Unfortunately, ARM CPUs
> do not have an option to trap access to CP15 virtual timer registers.
> Consequently, timer operations cannot be trapped and emulated.
>
> The only possibility to work around is to use another timer hardware which
> is memory-mapped and can be emulated by qemu. In order to make guest kernel
> ignoring CP15 timer, we remove it from machine's device tree.
>

Curious, what is the kernels algorithm for choosing a timer when
multiple are in the device-tree?

There are a lot of QEMU reasons for knocking out device tree nodes,
un-emulated hardware being a big one. Should we be looking for a more
core solution to the "should this device tree node really be here"
problem?

> Of course this works only with machine models which actually have these
> timers (like vexpress).
>

Does an unedited vexpress DTS just work except for this one thing?

Regards,
Peter

> Pavel Fedin (2):
>   Introduce qemu_fdt_remove_compatible()
>   Remove CP15 timer from the device tree if KVM is used without
> in-kernel irqchip
>
>  device_tree.c| 10 ++
>  hw/arm/boot.c|  5 +
>  include/sysemu/device_tree.h | 10 ++
>  3 files changed, 25 insertions(+)
>
> --
> 1.9.5.msysgit.0
>
>

Re: [Qemu-devel] [PATCH 0/5] qapi flattening + some miscellaneous patches

[Gerd Hoffmann]

On Di, 2015-06-23 at 15:32 +0200, Kővágó, Zoltán wrote:
> I've cherry-picked the qapi related parts from my previous -audiodev
> patch series, we can hopefully concentrate on one thing at a time.  The
> most important changes in this patch series are the flattening of the
> Netdev structures.  This way we can add proper nested structure support
> into OptsVisitor, without requiring backward-compatibility hacks.

Applies and builds fine, no obvious regressions in testing.

Tested-by: Gerd Hoffmann 

Getting this merged before hard freeze would be great ...

Any takers?  Markus?

thanks,
  Gerd

[Qemu-devel] [PATCH v5 2/6] spapr: Add LMB DR connectors

[Bharata B Rao]

Enable memory hotplug for pseries 2.4 and add LMB DR connectors.
With memory hotplug, enforce RAM size, NUMA node memory size and maxmem
to be a multiple of SPAPR_MEMORY_BLOCK_SIZE (256M) since that's the
granularity in which LMBs are represented and hot-added.

LMB DR connectors will be used by the memory hotplug code.

Signed-off-by: Bharata B Rao 
Signed-off-by: Michael Roth 
   [spapr_drc_reset implementation]
---
 hw/ppc/spapr.c | 79 ++
 include/hw/ppc/spapr.h |  1 +
 2 files changed, 80 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 241ecad..d7e0e44 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -59,6 +59,7 @@
 #include "hw/nmi.h"
 
 #include "hw/compat.h"
+#include "qemu-common.h"
 
 #include 
 
@@ -1436,10 +1437,76 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu)
 qemu_register_reset(spapr_cpu_reset, cpu);
 }
 
+/*
+ * Reset routine for LMB DR devices.
+ *
+ * Unlike PCI DR devices, LMB DR devices explicitly register this reset
+ * routine. Reset for PCI DR devices will be handled by PHB reset routine
+ * when it walks all its children devices. LMB devices reset occurs
+ * as part of spapr_ppc_reset().
+ */
+static void spapr_drc_reset(void *opaque)
+{
+sPAPRDRConnector *drc = opaque;
+DeviceState *d = DEVICE(drc);
+
+if (d) {
+device_reset(d);
+}
+}
+
+static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
+{
+MachineState *machine = MACHINE(spapr);
+uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
+uint32_t nr_rma_lmbs = spapr->rma_size/lmb_size;
+uint32_t nr_lmbs = machine->maxram_size/lmb_size - nr_rma_lmbs;
+uint32_t nr_assigned_lmbs = machine->ram_size/lmb_size - nr_rma_lmbs;
+int i;
+
+for (i = 0; i < nr_lmbs; i++) {
+sPAPRDRConnector *drc;
+uint64_t addr;
+
+if (i < nr_assigned_lmbs) {
+addr = (i + nr_rma_lmbs) * lmb_size;
+} else {
+addr = (i - nr_assigned_lmbs) * lmb_size +
+spapr->hotplug_memory.base;
+}
+drc = spapr_dr_connector_new(OBJECT(spapr), 
SPAPR_DR_CONNECTOR_TYPE_LMB,
+ addr/lmb_size);
+qemu_register_reset(spapr_drc_reset, drc);
+}
+}
+
+/*
+ * If RAM size, maxmem size and individual node mem sizes aren't aligned
+ * to SPAPR_MEMORY_BLOCK_SIZE(256MB), then disable LMB DR feature.
+ */
+static void spapr_validate_node_memory(MachineState *machine)
+{
+int i;
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
+
+if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE ||
+machine->ram_size % SPAPR_MEMORY_BLOCK_SIZE) {
+smc->dr_lmb_enabled = false;
+}
+
+for (i = 0; i < nb_numa_nodes; i++) {
+if (numa_info[i].node_mem &&
+numa_info[i].node_mem % SPAPR_MEMORY_BLOCK_SIZE) {
+smc->dr_lmb_enabled = false;
+}
+}
+}
+
 /* pSeries LPAR / sPAPR hardware init */
 static void ppc_spapr_init(MachineState *machine)
 {
 sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
 const char *kernel_filename = machine->kernel_filename;
 const char *kernel_cmdline = machine->kernel_cmdline;
 const char *initrd_filename = machine->initrd_filename;
@@ -1518,6 +1585,10 @@ static void ppc_spapr_init(MachineState *machine)
smp_threads),
   XICS_IRQS);
 
+if (smc->dr_lmb_enabled) {
+spapr_validate_node_memory(machine);
+}
+
 /* init CPUs */
 if (machine->cpu_model == NULL) {
 machine->cpu_model = kvm_enabled() ? "host" : "POWER7";
@@ -1567,6 +1638,10 @@ static void ppc_spapr_init(MachineState *machine)
 &spapr->hotplug_memory.mr);
 }
 
+if (smc->dr_lmb_enabled) {
+spapr_create_lmb_dr_connectors(spapr);
+}
+
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
 if (!filename) {
 error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin");
@@ -1840,6 +1915,7 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error 
**errp)
 static void spapr_machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
+sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
 FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc);
 NMIClass *nc = NMI_CLASS(oc);
 
@@ -1853,6 +1929,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
 mc->kvm_type = spapr_kvm_type;
 mc->has_dynamic_sysbus = true;
 
+smc->dr_lmb_enabled = false;
 fwc->get_dev_path = spapr_get_fw_dev_path;
 nc->nmi_monitor_handler = spapr_nmi;
 }
@@ -1988,11 +2065,13 @@ static const TypeInfo spapr_machine_2_3_info = {
 static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = 

Re: [Qemu-devel] [RFC PATCH v4 3/5] spapr: Support ibm, dynamic-reconfiguration-memory

[Bharata B Rao]

On Wed, Jun 24, 2015 at 03:55:08PM +1000, David Gibson wrote:
> On Wed, Jun 24, 2015 at 07:55:44AM +0530, Bharata B Rao wrote:
> > On Tue, Jun 23, 2015 at 11:54:29AM +1000, David Gibson wrote:
> > > On Fri, Jun 19, 2015 at 03:47:55PM +0530, Bharata B Rao wrote:
> > > > Parse ibm,architecture.vec table obtained from the guest and enable
> > > > memory node configuration via ibm,dynamic-reconfiguration-memory if 
> > > > guest
> > > > supports it. This is in preparation to support memory hotplug for
> > > > sPAPR guests.
> > > > 
> > > > This changes the way memory node configuration is done. Currently all
> > > > memory nodes are built upfront. But after this patch, only memory@0 node
> > > > for RMA is built upfront. Guest kernel boots with just that and rest of
> > > > the memory nodes (via memory@XXX or ibm,dynamic-reconfiguration-memory)
> > > > are built when guest does ibm,client-architecture-support call.
> > > > 
> > > > Note: This patch needs a SLOF enhancement which is already part of
> > > > SLOF binary in QEMU.
> > > > 
> > > > Signed-off-by: Bharata B Rao 
> > > 
> > > [snip]
> > > > +int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
> > > > + target_ulong addr, target_ulong size,
> > > > + bool cpu_update, bool memory_update)
> > > > +{
> > > > +void *fdt, *fdt_skel;
> > > > +sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
> > > > +
> > > > +size -= sizeof(hdr);
> > > > +
> > > > +/* Create sceleton */
> > > > +fdt_skel = g_malloc0(size);
> > > > +_FDT((fdt_create(fdt_skel, size)));
> > > > +_FDT((fdt_begin_node(fdt_skel, "")));
> > > > +_FDT((fdt_end_node(fdt_skel)));
> > > > +_FDT((fdt_finish(fdt_skel)));
> > > > +fdt = g_malloc0(size);
> > > > +_FDT((fdt_open_into(fdt_skel, fdt, size)));
> > > > +g_free(fdt_skel);
> > > > +
> > > > +/* Fixup cpu nodes */
> > > > +if (cpu_update) {
> > > > +_FDT((spapr_fixup_cpu_dt(fdt, spapr)));
> > > > +}
> > > 
> > > The cpu_update parameter seems like its not related to memory hotplug
> > > at all.  I'm guessing it relates to CPU hotplug, in which case please
> > > defer it until those patches are ready to go.
> > 
> > This change isn't related to cpu hotplug. Earlier this compose response
> > routine only did CPU device tree fixup based on some conditions. I have
> > enabled it to check for availability DRCONF_MEMORY feature and accordingly
> > fixup memory DT. So this change just checks if cpu fixup is necessary
> > or not. Essentially we aren't changing any behaviour wrt cpu dt
> > fixup here.
> 
> Hm, ok.  Would there be any problem with just unconditionally doing
> both fixups?  This is about as far from a hot path as its possible to
> get.

Right now I don't fully understand under what circumstances cpu fixup
will be done. What I can deduce is that it is conditionally done from
ibm,client-architecture-support call and I have just ensured that I
retain the same behaviour with this change.

Regards,
Bharata.

[Qemu-devel] [PATCH v5 4/6] spapr: Make hash table size a factor of maxram_size

[Bharata B Rao]

The hash table size is dependent on ram_size, but since with hotplug
the memory can grow till maxram_size. Hence make hash table size dependent
on maxram_size.

This allows to hotplug huge amounts of memory to the guest.

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index aee8156..0ab78fa 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1697,7 +1697,7 @@ static void ppc_spapr_init(MachineState *machine)
  * more than needed for the Linux guests we support. */
 spapr->htab_shift = 18; /* Minimum architected size */
 while (spapr->htab_shift <= 46) {
-if ((1ULL << (spapr->htab_shift + 7)) >= machine->ram_size) {
+if ((1ULL << (spapr->htab_shift + 7)) >= machine->maxram_size) {
 break;
 }
 spapr->htab_shift++;
-- 
2.1.0

[Qemu-devel] [RFC PATCH v5 6/6] spapr: Don't allow memory hotplug to memory less nodes

[Bharata B Rao]

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.

Signed-off-by: Bharata B Rao 
---
 hw/ppc/spapr.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f4792b8..003ef14 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2139,6 +2139,28 @@ 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);
 }
 }
-- 
2.1.0

[Qemu-devel] [PATCH v5 3/6] spapr: Support ibm, dynamic-reconfiguration-memory

[Bharata B Rao]

Parse ibm,architecture.vec table obtained from the guest and enable
memory node configuration via ibm,dynamic-reconfiguration-memory if guest
supports it. This is in preparation to support memory hotplug for
sPAPR guests.

This changes the way memory node configuration is done. Currently all
memory nodes are built upfront. But after this patch, only memory@0 node
for RMA is built upfront. Guest kernel boots with just that and rest of
the memory nodes (via memory@XXX or ibm,dynamic-reconfiguration-memory)
are built when guest does ibm,client-architecture-support call.

Note: This patch needs a SLOF enhancement which is already part of
SLOF binary in QEMU.

Signed-off-by: Bharata B Rao 
---
 docs/specs/ppc-spapr-hotplug.txt |  48 +
 hw/ppc/spapr.c   | 210 +++
 hw/ppc/spapr_hcall.c |  51 --
 include/hw/ppc/spapr.h   |  15 ++-
 4 files changed, 274 insertions(+), 50 deletions(-)

diff --git a/docs/specs/ppc-spapr-hotplug.txt b/docs/specs/ppc-spapr-hotplug.txt
index 46e0719..9d574b5 100644
--- a/docs/specs/ppc-spapr-hotplug.txt
+++ b/docs/specs/ppc-spapr-hotplug.txt
@@ -302,4 +302,52 @@ consisting of ,  and .
 pseries guests use this property to note the maximum allowed CPUs for the
 guest.
 
+== ibm,dynamic-reconfiguration-memory ==
+
+ibm,dynamic-reconfiguration-memory is a device tree node that represents
+dynamically reconfigurable logical memory blocks (LMB). This node
+is generated only when the guest advertises the support for it via
+ibm,client-architecture-support call. Memory that is not dynamically
+reconfigurable is represented by /memory nodes. The properties of this
+node that are of interest to the sPAPR memory hotplug implementation
+in QEMU are described here.
+
+ibm,lmb-size
+
+This 64bit integer defines the size of each dynamically reconfigurable LMB.
+
+ibm,associativity-lookup-arrays
+
+This property defines a lookup array in which the NUMA associativity
+information for each LMB can be found. It is a property encoded array
+that begins with an integer M, the number of associativity lists followed
+by an integer N, the number of entries per associativity list and terminated
+by M associativity lists each of length N integers.
+
+This property provides the same information as given by ibm,associativity
+property in a /memory node. Each assigned LMB has an index value between
+0 and M-1 which is used as an index into this table to select which
+associativity list to use for the LMB. This index value for each LMB
+is defined in ibm,dynamic-memory property.
+
+ibm,dynamic-memory
+
+This property describes the dynamically reconfigurable memory. It is a
+property endoded array that has an integer N, the number of LMBs followed
+by N LMB list entires.
+
+Each LMB list entry consists of the following elements:
+
+- Logical address of the start of the LMB encoded as a 64bit integer. This
+  corresponds to reg property in /memory node.
+- DRC index of the LMB that corresponds to ibm,my-drc-index property
+  in a /memory node.
+- Four bytes reserved for expansion.
+- Associativity list index for the LMB that is used an index into
+  ibm,associativity-lookup-arrays property described earlier. This
+  is used to retrieve the right associativity list to be used for this
+  LMB.
+- A 32bit flags word. The bit at bit position 0x0008 defines whether
+  the LMB is assigned to the the partition as of boot time.
+
 [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 d7e0e44..aee8156 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -495,44 +495,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
 return fdt;
 }
 
-int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
- target_ulong addr, target_ulong size)
-{
-void *fdt, *fdt_skel;
-sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
-
-size -= sizeof(hdr);
-
-/* Create sceleton */
-fdt_skel = g_malloc0(size);
-_FDT((fdt_create(fdt_skel, size)));
-_FDT((fdt_begin_node(fdt_skel, "")));
-_FDT((fdt_end_node(fdt_skel)));
-_FDT((fdt_finish(fdt_skel)));
-fdt = g_malloc0(size);
-_FDT((fdt_open_into(fdt_skel, fdt, size)));
-g_free(fdt_skel);
-
-/* Fix skeleton up */
-_FDT((spapr_fixup_cpu_dt(fdt, spapr)));
-
-/* Pack resulting tree */
-_FDT((fdt_pack(fdt)));
-
-if (fdt_totalsize(fdt) + sizeof(hdr) > size) {
-trace_spapr_cas_failed(size);
-return -1;
-}
-
-cpu_physical_memory_write(addr, &hdr, sizeof(hdr));
-cpu_physical_memory_write(addr + sizeof(hdr), fdt, fdt_totalsize(fdt));
-trace_spapr_cas_continue(fdt_totalsize(fdt) + sizeof(hdr));
-g_free(fdt);
-
-return 0;
-}
-
-static void spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start,
+static int spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start,

[Qemu-devel] [PATCH v5 5/6] spapr: Memory hotplug support

[Bharata B Rao]

Make use of pc-dimm infrastructure to support memory hotplug
for PowerPC.

Signed-off-by: Bharata B Rao 
---
 hw/ppc/spapr.c| 129 ++
 hw/ppc/spapr_events.c |   8 ++--
 2 files changed, 134 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0ab78fa..f4792b8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -33,6 +33,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/cpus.h"
 #include "sysemu/kvm.h"
+#include "sysemu/device_tree.h"
 #include "kvm_ppc.h"
 #include "mmu-hash64.h"
 #include "qom/cpu.h"
@@ -855,6 +856,7 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
hwaddr rtas_size)
 {
 MachineState *machine = MACHINE(qdev_get_machine());
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
 const char *boot_device = machine->boot_order;
 int ret, i;
 size_t cb = 0;
@@ -938,6 +940,10 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
 spapr_populate_chosen_stdout(fdt, spapr->vio_bus);
 }
 
+if (smc->dr_lmb_enabled) {
+_FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB));
+}
+
 _FDT((fdt_pack(fdt)));
 
 if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
@@ -2036,12 +2042,131 @@ static void spapr_nmi(NMIState *n, int cpu_index, 
Error **errp)
 }
 }
 
+static void spapr_add_lmbs(DeviceState *dev, uint64_t addr, uint64_t size,
+   uint32_t node, Error **errp)
+{
+sPAPRDRConnector *drc;
+sPAPRDRConnectorClass *drck;
+uint32_t nr_lmbs = size/SPAPR_MEMORY_BLOCK_SIZE;
+int i, fdt_offset, fdt_size;
+void *fdt;
+Error *local_err = NULL;
+
+if (size % SPAPR_MEMORY_BLOCK_SIZE) {
+error_setg(errp, "Hotplugged memory size must be a multiple of "
+  "%lld MB", SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
+return;
+}
+
+/*
+ * Check for DRC connectors and send hotplug notification to the
+ * guest only in case of hotplugged memory. This allows cold plugged
+ * memory to be specified at boot time.
+ */
+if (!dev->hotplugged) {
+return;
+}
+
+for (i = 0; i < nr_lmbs; i++) {
+drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
+addr/SPAPR_MEMORY_BLOCK_SIZE);
+g_assert(drc);
+
+fdt = create_device_tree(&fdt_size);
+fdt_offset = spapr_populate_memory_node(fdt, node, addr,
+SPAPR_MEMORY_BLOCK_SIZE);
+
+drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
+if (local_err) {
+g_free(fdt);
+error_propagate(errp, local_err);
+return;
+}
+
+spapr_hotplug_req_add_event(drc);
+addr += SPAPR_MEMORY_BLOCK_SIZE;
+}
+}
+
+static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+  uint32_t node, Error **errp)
+{
+Error *local_err = NULL;
+sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
+PCDIMMDevice *dimm = PC_DIMM(dev);
+PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
+MemoryRegion *mr = ddc->get_memory_region(dimm);
+uint64_t align = memory_region_get_alignment(mr);
+uint64_t addr;
+
+pc_dimm_memory_plug(dev, &ms->hotplug_memory, mr, align, &local_err);
+if (local_err) {
+goto out;
+}
+
+addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, 
&local_err);
+if (local_err) {
+goto out_unplug;
+}
+
+spapr_add_lmbs(dev, addr, memory_region_size(mr), node, &local_err);
+if (local_err) {
+goto out_unplug;
+}
+return;
+
+out_unplug:
+pc_dimm_memory_unplug(dev, &ms->hotplug_memory, mr);
+out:
+error_propagate(errp, local_err);
+}
+
+static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
+  DeviceState *dev, Error **errp)
+{
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
+
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+uint32_t node;
+
+if (!smc->dr_lmb_enabled) {
+error_setg(errp, "Memory hotplug not supported for this machine");
+return;
+}
+node = object_property_get_int(OBJECT(dev), PC_DIMM_NODE_PROP, errp);
+if (*errp) {
+return;
+}
+
+spapr_memory_plug(hotplug_dev, dev, node, errp);
+}
+}
+
+static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
+  DeviceState *dev, Error **errp)
+{
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+error_setg(errp, "Memory hot unplug not supported by sPAPR");
+}
+}
+
+static HotplugHandler *spapr_get_hotpug_handler(MachineState *machine,
+ DeviceState *dev)
+{
+if (o

[Qemu-devel] [PATCH v5 0/6] Memory hotplug for PowerPC sPAPR guests

[Bharata B Rao]

Hi,

This is v5 of memory hotplug support patchset for PowerPC
sPAPR guests.

This patchset applies on spapr-next branch of David Gibson's tree with
the other prerequisite patchset applied. Pre-requistes patchset was
posted at:
https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg05157.html

Changes in v5
-
- Get rid of hotplug_mem_size alignment completely and start hotplug base
  right after RAM. (01/06)
- Disable LMB DR and revert to older style of memory DT representation
  if RAM size, maxmem size or individual node memory sizes aren't aligned
  to SPAPR_MEMORY_BLOCK_SIZE (256M). (02/06)
- Use dr_lmb_enabled from sPAPRMachineClass itself instead of copying
  it to sPAPRMachineState. (02/06)
- Ensure memory@0 is created even when -numa option isn't specified. (03/06)
- Calling pc_dimm_memory_unplug() when pc_dimm_memory_plug() fails isn't
  required. (05/06)
- Added a new patch to prevent memory hotplug to memory-less nodes (06/06)

v4: https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg05242.html
v3: https://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg02910.html

*** BLURB HERE ***

Bharata B Rao (6):
  spapr: Initialize hotplug memory address space
  spapr: Add LMB DR connectors
  spapr: Support ibm,dynamic-reconfiguration-memory
  spapr: Make hash table size a factor of maxram_size
  spapr: Memory hotplug support
  spapr: Don't allow memory hotplug to memory less nodes

 default-configs/ppc64-softmmu.mak |   1 +
 docs/specs/ppc-spapr-hotplug.txt  |  48 
 hw/ppc/spapr.c| 460 ++
 hw/ppc/spapr_events.c |   8 +-
 hw/ppc/spapr_hcall.c  |  51 -
 include/hw/ppc/spapr.h|  28 ++-
 6 files changed, 542 insertions(+), 54 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v5 1/6] spapr: Initialize hotplug memory address space

[Bharata B Rao]

Initialize a hotplug memory region under which all the hotplugged
memory is accommodated. Also enable memory hotplug by setting
CONFIG_MEM_HOTPLUG.

Modelled on i386 memory hotplug.

Signed-off-by: Bharata B Rao 
---
 default-configs/ppc64-softmmu.mak |  1 +
 hw/ppc/spapr.c| 18 ++
 include/hw/ppc/spapr.h| 12 
 3 files changed, 31 insertions(+)

diff --git a/default-configs/ppc64-softmmu.mak 
b/default-configs/ppc64-softmmu.mak
index ab62cc7..e77cb1a 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -52,3 +52,4 @@ CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
 # For PReP
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
+CONFIG_MEM_HOTPLUG=y
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5ca817c..241ecad 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1549,6 +1549,24 @@ static void ppc_spapr_init(MachineState *machine)
 memory_region_add_subregion(sysmem, 0, rma_region);
 }
 
+/* initialize hotplug memory address space */
+if (machine->ram_size < machine->maxram_size) {
+ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
+
+if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
+error_report("unsupported amount of memory slots: %"PRIu64,
+  machine->ram_slots);
+exit(EXIT_FAILURE);
+}
+
+spapr->hotplug_memory.base = ROUND_UP(machine->ram_size,
+  SPAPR_HOTPLUG_MEM_ALIGN);
+memory_region_init(&spapr->hotplug_memory.mr, OBJECT(spapr),
+   "hotplug-memory", hotplug_mem_size);
+memory_region_add_subregion(sysmem, spapr->hotplug_memory.base,
+&spapr->hotplug_memory.mr);
+}
+
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
 if (!filename) {
 error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin");
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 91a61ab..8a1929b 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -5,6 +5,7 @@
 #include "hw/boards.h"
 #include "hw/ppc/xics.h"
 #include "hw/ppc/spapr_drc.h"
+#include "hw/mem/pc-dimm.h"
 
 struct VIOsPAPRBus;
 struct sPAPRPHBState;
@@ -76,6 +77,7 @@ struct sPAPRMachineState {
 
 /*< public >*/
 char *kvm_type;
+MemoryHotplugState hotplug_memory;
 };
 
 #define H_SUCCESS 0
@@ -609,4 +611,14 @@ int spapr_rtc_import_offset(DeviceState *dev, int64_t 
legacy_offset);
 
 #define SPAPR_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */
 
+/*
+ * This defines the maximum number of DIMM slots we can have for sPAPR
+ * guest. This is not defined by sPAPR but we are defining it to 32 slots
+ * based on default number of slots provided by PowerPC kernel.
+ */
+#define SPAPR_MAX_RAM_SLOTS 32
+
+/* 1GB alignment for hotplug memory region */
+#define SPAPR_HOTPLUG_MEM_ALIGN (1ULL << 30)
+
 #endif /* !defined (__HW_SPAPR_H__) */
-- 
2.1.0

Re: [Qemu-devel] Implement Xfer:auxv:read in gdb stub

[Bhushan Attarde]

Hi Jan,

Thanks for the review.
I will resubmit the patch with suggested changes.

Regards,
Bhushan

-Original Message-
From: Jan Kiszka [mailto:jan.kis...@siemens.com] 
Sent: 25 June 2015 10:56
To: Bhushan Attarde; qemu-devel@nongnu.org
Cc: Yongbok Kim; Jaydeep Patil
Subject: Re: Implement Xfer:auxv:read in gdb stub

On 2015-06-24 08:34, Bhushan Attarde wrote:
> This patch implements support for "Xfer:auxv:read" to provide 
> auxiliary vector information to clients which relies on it.
> 
> For example: AT_ENTRY in auxiliary vector provides the entry point 
> information.
> Client can use this information to compare it with entry point 
> mentioned in executable to calculate load offset and then update the 
> load addresses accordingly.
> 
> 
> Signed-off-by: Bhushan Attarde 
> ---
> diff --git a/gdbstub.c b/gdbstub.c
> index cea2a84..e7db84a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1153,6 +1153,7 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>  if (cc->gdb_core_xml_file != NULL) {
>  pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
>  }
> +pstrcat(buf, sizeof(buf), ";qXfer:auxv:read+");

Is this code block restricted to CONFIG_USER_ONLY? It should be because the 
feature implementation is. Sorry, too lazy to check the full context, and the 
patch doesn't tell it.

Jan

>  put_packet(s, buf);
>  break;
>  }
> @@ -1199,6 +1200,50 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>  put_packet_binary(s, buf, len + 1);
>  break;
>  }
> +#ifdef CONFIG_USER_ONLY
> +if (strncmp(p, "Xfer:auxv:read:", 15) == 0) {
> +TaskState *ts = s->c_cpu->opaque;
> +target_ulong auxv = ts->info->saved_auxv;
> +target_ulong auxv_len = ts->info->auxv_len;
> +char *ptr;
> +
> +p += 15;
> +while (*p && *p != ':') {
> +p++;
> +}
> +p++;
> +
> +addr = strtoul(p, (char **)&p, 16);
> +if (*p == ',') {
> +p++;
> +}
> +len = strtoul(p, (char **)&p, 16);
> +
> +ptr = lock_user(VERIFY_READ, auxv, auxv_len, 0);
> +if (ptr == NULL) {
> +break;
> +}
> +
> +if (addr > len) {
> +snprintf(buf, sizeof(buf), "E00");
> +put_packet(s, buf);
> +break;
> +}
> +if (len > (MAX_PACKET_LENGTH - 5) / 2) {
> +len = (MAX_PACKET_LENGTH - 5) / 2;
> +}
> +if (len < auxv_len - addr) {
> +buf[0] = 'm';
> +len = memtox(buf + 1, ptr + addr, len);
> +} else {
> +buf[0] = 'l';
> +len = memtox(buf + 1, ptr + addr, auxv_len - addr);
> +}
> +put_packet_binary(s, buf, len + 1);
> +unlock_user(ptr, auxv, len);
> +break;
> +}
> +#endif /* !CONFIG_USER_ONLY */
>  if (is_query_packet(p, "Attached", ':')) {
>  put_packet(s, GDB_ATTACHED);
>  break;
> 
> 
> 
> 

--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center 
Embedded Linux

Re: [Qemu-devel] Implement Xfer:auxv:read in gdb stub

[Jan Kiszka]

On 2015-06-24 08:34, Bhushan Attarde wrote:
> This patch implements support for "Xfer:auxv:read" to provide auxiliary vector
> information to clients which relies on it.
> 
> For example: AT_ENTRY in auxiliary vector provides the entry point 
> information.
> Client can use this information to compare it with entry point mentioned in 
> executable to calculate load offset and then update the load addresses
> accordingly.
> 
> 
> Signed-off-by: Bhushan Attarde 
> ---
> diff --git a/gdbstub.c b/gdbstub.c
> index cea2a84..e7db84a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1153,6 +1153,7 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>  if (cc->gdb_core_xml_file != NULL) {
>  pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
>  }
> +pstrcat(buf, sizeof(buf), ";qXfer:auxv:read+");

Is this code block restricted to CONFIG_USER_ONLY? It should be because
the feature implementation is. Sorry, too lazy to check the full
context, and the patch doesn't tell it.

Jan

>  put_packet(s, buf);
>  break;
>  }
> @@ -1199,6 +1200,50 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>  put_packet_binary(s, buf, len + 1);
>  break;
>  }
> +#ifdef CONFIG_USER_ONLY
> +if (strncmp(p, "Xfer:auxv:read:", 15) == 0) {
> +TaskState *ts = s->c_cpu->opaque;
> +target_ulong auxv = ts->info->saved_auxv;
> +target_ulong auxv_len = ts->info->auxv_len;
> +char *ptr;
> +
> +p += 15;
> +while (*p && *p != ':') {
> +p++;
> +}
> +p++;
> +
> +addr = strtoul(p, (char **)&p, 16);
> +if (*p == ',') {
> +p++;
> +}
> +len = strtoul(p, (char **)&p, 16);
> +
> +ptr = lock_user(VERIFY_READ, auxv, auxv_len, 0);
> +if (ptr == NULL) {
> +break;
> +}
> +
> +if (addr > len) {
> +snprintf(buf, sizeof(buf), "E00");
> +put_packet(s, buf);
> +break;
> +}
> +if (len > (MAX_PACKET_LENGTH - 5) / 2) {
> +len = (MAX_PACKET_LENGTH - 5) / 2;
> +}
> +if (len < auxv_len - addr) {
> +buf[0] = 'm';
> +len = memtox(buf + 1, ptr + addr, len);
> +} else {
> +buf[0] = 'l';
> +len = memtox(buf + 1, ptr + addr, auxv_len - addr);
> +}
> +put_packet_binary(s, buf, len + 1);
> +unlock_user(ptr, auxv, len);
> +break;
> +}
> +#endif /* !CONFIG_USER_ONLY */
>  if (is_query_packet(p, "Attached", ':')) {
>  put_packet(s, GDB_ATTACHED);
>  break;
> 
> 
> 
> 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 5/9] memory: let address_space_rw/ld*/st* run outside the BQL

[Fam Zheng]

On Wed, 06/24 18:25, Paolo Bonzini wrote:
> From: Jan Kiszka 
> 
> The MMIO case is further broken up in two cases: if the caller does not
> hold the BQL on invocation, the unlocked one takes or avoids BQL depending
> on the locking strategy of the target memory region and its coalesced
> MMIO handling.  In this case, the caller should not hold _any_ lock
> (a friendly suggestion which is disregarded by virtio-scsi-dataplane).
> 
> Signed-off-by: Jan Kiszka 
> Cc: Frederic Konrad 
> Message-Id: <1434646046-27150-6-git-send-email-pbonz...@redhat.com>
> Signed-off-by: Paolo Bonzini 
> ---
>  exec.c | 69 
> +-
>  1 file changed, 60 insertions(+), 9 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index f2e6603..fd0401e 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -48,6 +48,7 @@
>  #endif
>  #include "exec/cpu-all.h"
>  #include "qemu/rcu_queue.h"
> +#include "qemu/main-loop.h"
>  #include "exec/cputlb.h"
>  #include "translate-all.h"
>  
> @@ -2316,11 +2317,27 @@ static int memory_access_size(MemoryRegion *mr, 
> unsigned l, hwaddr addr)
>  return l;
>  }
>  
> -static void prepare_mmio_access(MemoryRegion *mr)
> +static bool prepare_mmio_access(MemoryRegion *mr)
>  {
> +bool unlocked = !qemu_mutex_iothread_locked();
> +bool release_lock = false;
> +
> +if (unlocked && mr->global_locking) {
> +qemu_mutex_lock_iothread();
> +unlocked = false;
> +release_lock = true;
> +}
>  if (mr->flush_coalesced_mmio) {
> +if (unlocked) {
> +qemu_mutex_lock_iothread();
> +}
>  qemu_flush_coalesced_mmio_buffer();
> +if (unlocked) {
> +qemu_mutex_unlock_iothread();
> +}
>  }
> +
> +return release_lock;
>  }
>  
>  MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
> @@ -2332,6 +2349,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr 
> addr, MemTxAttrs attrs,
>  hwaddr addr1;
>  MemoryRegion *mr;
>  MemTxResult result = MEMTX_OK;
> +bool release_lock = false;
>  
>  rcu_read_lock();
>  while (len > 0) {
> @@ -2340,7 +2358,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr 
> addr, MemTxAttrs attrs,
>  
>  if (is_write) {
>  if (!memory_access_is_direct(mr, is_write)) {
> -prepare_mmio_access(mr);
> +release_lock |= prepare_mmio_access(mr);
>  l = memory_access_size(mr, l, addr1);
>  /* XXX: could force current_cpu to NULL to avoid
> potential bugs */
> @@ -2382,7 +2400,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr 
> addr, MemTxAttrs attrs,
>  } else {
>  if (!memory_access_is_direct(mr, is_write)) {
>  /* I/O case */
> -prepare_mmio_access(mr);
> +release_lock |= prepare_mmio_access(mr);
>  l = memory_access_size(mr, l, addr1);
>  switch (l) {
>  case 8:
> @@ -2418,6 +2436,12 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr 
> addr, MemTxAttrs attrs,
>  memcpy(buf, ptr, l);
>  }
>  }
> +
> +if (release_lock) {
> +qemu_mutex_unlock_iothread();
> +release_lock = false;
> +}
> +
>  len -= l;
>  buf += l;
>  addr += l;
> @@ -2744,11 +2768,12 @@ static inline uint32_t 
> address_space_ldl_internal(AddressSpace *as, hwaddr addr,
>  hwaddr l = 4;
>  hwaddr addr1;
>  MemTxResult r;
> +bool release_lock = false;
>  
>  rcu_read_lock();
>  mr = address_space_translate(as, addr, &addr1, &l, false);
>  if (l < 4 || !memory_access_is_direct(mr, false)) {
> -prepare_mmio_access(mr);
> +release_lock |= prepare_mmio_access(mr);
>  
>  /* I/O case */
>  r = memory_region_dispatch_read(mr, addr1, &val, 4, attrs);
> @@ -2782,6 +2807,9 @@ static inline uint32_t 
> address_space_ldl_internal(AddressSpace *as, hwaddr addr,
>  if (result) {
>  *result = r;
>  }
> +if (release_lock) {
> +qemu_mutex_unlock_iothread();
> +}
>  rcu_read_unlock();
>  return val;
>  }
> @@ -2834,12 +2862,13 @@ static inline uint64_t 
> address_space_ldq_internal(AddressSpace *as, hwaddr addr,
>  hwaddr l = 8;
>  hwaddr addr1;
>  MemTxResult r;
> +bool release_lock = false;
>  
>  rcu_read_lock();
>  mr = address_space_translate(as, addr, &addr1, &l,
>   false);
>  if (l < 8 || !memory_access_is_direct(mr, false)) {
> -prepare_mmio_access(mr);
> +release_lock |= prepare_mmio_access(mr);
>  
>  /* I/O case */
>  r = memory_region_dispatch_read(mr, addr1, &val, 8, attrs);
> @@ -2873,6 +2902,9 @@ static inline uint64_t 
> address_space_ldq_internal(AddressSpace *as, hwaddr addr,
>  if (result) {
>

Re: [Qemu-devel] [PATCH 4/9] exec: pull qemu_flush_coalesced_mmio_buffer() into address_space_rw/ld*/st*

[Fam Zheng]

On Wed, 06/24 18:25, Paolo Bonzini wrote:
> As memory_region_read/write_accessor will now be run also without BQL held,
> we need to move coalesced MMIO flushing earlier in the dispatch process.
> 
> Cc: Frederic Konrad 
> Message-Id: <1434646046-27150-5-git-send-email-pbonz...@redhat.com>
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH] pci : Add pba_offset PCI quirk for Chelsio T5 devices

[Bandan Das]

Hi Gabriel,

Glad that you got to the bottom of this! :)

Gabriel Laupre  writes:

> Fix pba_offset initialization value for Chelsio T5 devices.  The
> hardware doesn't return the correct pba_offset value, so add a
> quirk to instead return a hardcoded value of 0x1000 when a Chelsio
> T5 device is detected.
Two questions:
Is the array offset guaranteed to always be the same ?
What are the chances of this getting fixed by a firmware update ? :)

Bandan

> Signed-off-by: Gabriel Laupre 
> ---
>  hw/vfio/pci.c| 12 
>  include/hw/pci/pci_ids.h |  3 +++
>  2 files changed, 15 insertions(+)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index e0e339a..8a4c7cd 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2220,6 +2220,9 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
>  uint16_t ctrl;
>  uint32_t table, pba;
>  int fd = vdev->vbasedev.fd;
> +PCIDevice *pdev = &vdev->pdev;
> +uint16_t vendor = pci_get_word(pdev->config + PCI_VENDOR_ID);
> +uint16_t device = pci_get_word(pdev->config + PCI_DEVICE_ID);
>  
>  pos = pci_find_capability(&vdev->pdev, PCI_CAP_ID_MSIX);
>  if (!pos) {
> @@ -2252,6 +2255,15 @@ static int vfio_early_setup_msix(VFIOPCIDevice *vdev)
>  vdev->msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
>  vdev->msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
>  
> +/* Quirk to set the pba_offset value for Chelsio T5
> + * devices. Since hardware does not return value correctly,
> + * we override with a hardcoded value instead.
> + */
> +if (vendor == PCI_VENDOR_ID_CHELSIO &&
> +(device & 0xf000) == PCI_DEVICE_ID_CHELSIO_T5_SERIES) {
> +vdev->msix->pba_offset = 0x1000;
> +}
> +
>  trace_vfio_early_setup_msix(vdev->vbasedev.name, pos,
>  vdev->msix->table_bar,
>  vdev->msix->table_offset,
> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
> index 49c062b..9f649da 100644
> --- a/include/hw/pci/pci_ids.h
> +++ b/include/hw/pci/pci_ids.h
> @@ -114,6 +114,9 @@
>  #define PCI_VENDOR_ID_ENSONIQ0x1274
>  #define PCI_DEVICE_ID_ENSONIQ_ES1370 0x5000
>  
> +#define PCI_VENDOR_ID_CHELSIO0x1425
> +#define PCI_DEVICE_ID_CHELSIO_T5_SERIES  0x5000
> +
>  #define PCI_VENDOR_ID_FREESCALE  0x1957
>  #define PCI_DEVICE_ID_MPC8533E   0x0030

Re: [Qemu-devel] [PATCH 3/9] memory: Add global-locking property to memory regions

[Fam Zheng]

On Wed, 06/24 18:25, Paolo Bonzini wrote:
> From: Jan Kiszka 
> 
> This introduces the memory region property "global_locking". It is true
> by default. By setting it to false, a device model can request BQL-free
> dispatching of region accesses to its r/w handlers. The actual BQL
> break-up will be provided in a separate patch.
> 
> Signed-off-by: Jan Kiszka 
> Cc: Frederic Konrad 
> Signed-off-by: Paolo Bonzini 
> Message-Id: <1434646046-27150-4-git-send-email-pbonz...@redhat.com>
> ---
>  include/exec/memory.h | 26 ++
>  memory.c  | 11 +++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 8ae004e..fc33348 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -180,6 +180,7 @@ struct MemoryRegion {
>  bool rom_device;
>  bool warning_printed; /* For reservations */
>  bool flush_coalesced_mmio;
> +bool global_locking;
>  MemoryRegion *alias;
>  hwaddr alias_offset;
>  int32_t priority;
> @@ -825,6 +826,31 @@ void memory_region_set_flush_coalesced(MemoryRegion *mr);
>  void memory_region_clear_flush_coalesced(MemoryRegion *mr);
>  
>  /**
> + * memory_region_set_global_locking: Declares the access processing requires
> + *   QEMU's global lock.
> + *
> + * When this is invoked, access to this memory regions will be processed 
> while
> + * holding the global lock of QEMU. This is the default behavior of memory
> + * regions.
> + *
> + * @mr: the memory region to be updated.
> + */
> +void memory_region_set_global_locking(MemoryRegion *mr);
> +
> +/**
> + * memory_region_clear_global_locking: Declares that access processing does
> + * not depend on the QEMU global lock.
> + *
> + * By clearing this property, accesses to the memory region will be processed

Inconsistent with singlar form (access) in the previous one. Otherwise,

Reviewed-by: Fam Zheng 

> + * outside of QEMU's global lock (unless the lock is held on when issuing the
> + * access request). In this case, the device model implementing the access
> + * handlers is responsible for synchronization of concurrency.
> + *
> + * @mr: the memory region to be updated.
> + */
> +void memory_region_clear_global_locking(MemoryRegion *mr);
> +
> +/**
>   * memory_region_add_eventfd: Request an eventfd to be triggered when a word
>   *is written to a location.
>   *
> diff --git a/memory.c b/memory.c
> index 3ac0bd2..b0b8860 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1012,6 +1012,7 @@ static void memory_region_initfn(Object *obj)
>  mr->ram_addr = RAM_ADDR_INVALID;
>  mr->enabled = true;
>  mr->romd_mode = true;
> +mr->global_locking = true;
>  mr->destructor = memory_region_destructor_none;
>  QTAILQ_INIT(&mr->subregions);
>  QTAILQ_INIT(&mr->coalesced);
> @@ -1646,6 +1647,16 @@ void memory_region_clear_flush_coalesced(MemoryRegion 
> *mr)
>  }
>  }
>  
> +void memory_region_set_global_locking(MemoryRegion *mr)
> +{
> +mr->global_locking = true;
> +}
> +
> +void memory_region_clear_global_locking(MemoryRegion *mr)
> +{
> +mr->global_locking = false;
> +}
> +
>  void memory_region_add_eventfd(MemoryRegion *mr,
> hwaddr addr,
> unsigned size,
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH 1/9] main-loop: use qemu_mutex_lock_iothread consistently

[Fam Zheng]

On Wed, 06/24 18:25, Paolo Bonzini wrote:
> The next patch will require the BQL to be always taken with
> qemu_mutex_lock_iothread(), while right now this isn't the case.
> 
> Outside TCG mode this is not a problem.  In TCG mode, we need to be
> careful and avoid the "prod out of compiled code" step if already
> in a VCPU thread.  This is easily done with a check on current_cpu,
> i.e. qemu_in_vcpu_thread().
> 
> Hopefully, multithreaded TCG will get rid of the whole logic to kick
> VCPUs whenever an I/O event occurs!
> 
> Cc: Frederic Konrad 
> Message-Id: <1434646046-27150-2-git-send-email-pbonz...@redhat.com>

Why is this "Message-Id:" included in the commit message if it's not final?

> Signed-off-by: Paolo Bonzini 
> ---
>  cpus.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index b85fb5f..02cca5d 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -953,7 +953,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>  CPUState *cpu = arg;
>  int r;
>  
> -qemu_mutex_lock(&qemu_global_mutex);
> +qemu_mutex_lock_iothread();
>  qemu_thread_get_self(cpu->thread);
>  cpu->thread_id = qemu_get_thread_id();
>  cpu->can_do_io = 1;
> @@ -1033,10 +1033,10 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>  {
>  CPUState *cpu = arg;
>  
> +qemu_mutex_lock_iothread();
>  qemu_tcg_init_cpu_signals();
>  qemu_thread_get_self(cpu->thread);
>  
> -qemu_mutex_lock(&qemu_global_mutex);
>  CPU_FOREACH(cpu) {
>  cpu->thread_id = qemu_get_thread_id();
>  cpu->created = true;
> @@ -1148,7 +1148,11 @@ bool qemu_in_vcpu_thread(void)
>  void qemu_mutex_lock_iothread(void)
>  {
>  atomic_inc(&iothread_requesting_mutex);
> -if (!tcg_enabled() || !first_cpu || !first_cpu->thread) {
> +/* In the simple case there is no need to bump the VCPU thread out of
> + * TCG code execution.
> + */
> +if (!tcg_enabled() || qemu_in_vcpu_thread() ||
> +!first_cpu || !first_cpu->thread) {

This looks like a separate change from the above
"qemu_mutex_lock(&qemu_global_mutex)" conversion. Why do they belong to the
same patch?

Fam

>  qemu_mutex_lock(&qemu_global_mutex);
>  atomic_dec(&iothread_requesting_mutex);
>  } else {
> -- 
> 1.8.3.1
> 
> 

[Qemu-devel] [PATCH v2 2/6] block: Rename BLOCK_OP_TYPE_MIRROR to BLOCK_OP_TYPE_MIRROR_SOURCE

[Fam Zheng]

It's necessary to distinguish source and target before we can add
blockdev-mirror, because we would want a concrete type of operation to
check on target bs before starting.

Signed-off-by: Fam Zheng 
Reviewed-by: Max Reitz 
---
 blockdev.c  | 2 +-
 hw/block/dataplane/virtio-blk.c | 2 +-
 include/block/block.h   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 90f6e77..b573e56 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2962,7 +2962,7 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 }
 }
 
-if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) {
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
 goto out;
 }
 
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 3db139b..dac37de 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -206,7 +206,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, 
VirtIOBlkConf *conf,
 blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, 
s->blocker);
 blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
s->blocker);
-blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker);
+blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_MIRROR_SOURCE, s->blocker);
 blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker);
 blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker);
 
diff --git a/include/block/block.h b/include/block/block.h
index f5c0a90..69cb676 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -155,7 +155,7 @@ typedef enum BlockOpType {
 BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
 BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
 BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
-BLOCK_OP_TYPE_MIRROR,
+BLOCK_OP_TYPE_MIRROR_SOURCE,
 BLOCK_OP_TYPE_RESIZE,
 BLOCK_OP_TYPE_STREAM,
 BLOCK_OP_TYPE_REPLACE,
-- 
2.4.3

[Qemu-devel] [PATCH v2 6/6] iotests: Add test cases for blockdev-mirror

[Fam Zheng]

Signed-off-by: Fam Zheng 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests/041 | 99 --
 tests/qemu-iotests/041.out |  4 +-
 2 files changed, 80 insertions(+), 23 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 59a8f73..f708a06 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -65,8 +65,23 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
 event = self.wait_until_completed(drive=drive)
 self.assert_qmp(event, 'data/type', 'mirror')
 
+def blockdev_add(self, filename, drive_id=None, node_name=None):
+options = {'driver': iotests.imgfmt,
+   'file': {
+   'filename': filename,
+   'driver': 'file'}}
+if drive_id:
+options['id'] = drive_id
+if node_name:
+options['node-name'] = node_name
+result = self.vm.qmp('blockdev-add', options=options)
+self.assert_qmp(result, 'return', {})
+
 class TestSingleDrive(ImageMirroringTestCase):
 image_len = 1 * 1024 * 1024 # MB
+qmp_cmd = 'drive-mirror'
+qmp_target = target_img
+not_found_error = 'DeviceNotFound'
 
 def setUp(self):
 iotests.create_image(backing_img, self.image_len)
@@ -86,8 +101,8 @@ class TestSingleDrive(ImageMirroringTestCase):
 def test_complete(self):
 self.assert_no_active_block_jobs()
 
-result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- target=target_img)
+result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ target=self.qmp_target)
 self.assert_qmp(result, 'return', {})
 
 self.complete_and_wait()
@@ -100,8 +115,8 @@ class TestSingleDrive(ImageMirroringTestCase):
 def test_cancel(self):
 self.assert_no_active_block_jobs()
 
-result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- target=target_img)
+result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ target=self.qmp_target)
 self.assert_qmp(result, 'return', {})
 
 self.cancel_and_wait(force=True)
@@ -112,8 +127,8 @@ class TestSingleDrive(ImageMirroringTestCase):
 def test_cancel_after_ready(self):
 self.assert_no_active_block_jobs()
 
-result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- target=target_img)
+result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ target=self.qmp_target)
 self.assert_qmp(result, 'return', {})
 
 self.wait_ready_and_cancel()
@@ -126,8 +141,8 @@ class TestSingleDrive(ImageMirroringTestCase):
 def test_pause(self):
 self.assert_no_active_block_jobs()
 
-result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- target=target_img)
+result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ target=self.qmp_target)
 self.assert_qmp(result, 'return', {})
 
 result = self.vm.qmp('block-job-pause', device='drive0')
@@ -153,8 +168,8 @@ class TestSingleDrive(ImageMirroringTestCase):
 self.assert_no_active_block_jobs()
 
 # A small buffer is rounded up automatically
-result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- buf_size=4096, target=target_img)
+result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ buf_size=4096, target=self.qmp_target)
 self.assert_qmp(result, 'return', {})
 
 self.complete_and_wait()
@@ -169,8 +184,8 @@ class TestSingleDrive(ImageMirroringTestCase):
 
 qemu_img('create', '-f', iotests.imgfmt, '-o', 
'cluster_size=%d,size=%d'
 % (self.image_len, self.image_len), target_img)
-result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- buf_size=65536, mode='existing', 
target=target_img)
+result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ buf_size=65536, mode='existing', 
target=self.qmp_target)
 self.assert_qmp(result, 'return', {})
 
 self.complete_and_wait()
@@ -185,8 +200,8 @@ class TestSingleDrive(ImageMirroringTestCase):
 
 qemu_img('create', '-f', iotests.imgfmt, '-o', 
'cluster_size=%d,backing_file=%s'
 % (self.image_len, backing_img), target_img)
-result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- mode='existing', target=target_img)
+result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ mode='existing', target=self.qmp_target)
 self.assert_qmp(result, 'return', {})
 

[Qemu-devel] [PATCH v2 3/6] block: Extract blockdev part of qmp_drive_mirror

[Fam Zheng]

This is the part that will be reused by blockdev-mirror.

Signed-off-by: Fam Zheng 
---
 blockdev.c | 155 -
 1 file changed, 92 insertions(+), 63 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index b573e56..2a0c0e2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2883,29 +2883,22 @@ out:
 
 #define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
 
-void qmp_drive_mirror(const char *device, const char *target,
-  bool has_format, const char *format,
-  bool has_node_name, const char *node_name,
-  bool has_replaces, const char *replaces,
-  enum MirrorSyncMode sync,
-  bool has_mode, enum NewImageMode mode,
-  bool has_speed, int64_t speed,
-  bool has_granularity, uint32_t granularity,
-  bool has_buf_size, int64_t buf_size,
-  bool has_on_source_error, BlockdevOnError 
on_source_error,
-  bool has_on_target_error, BlockdevOnError 
on_target_error,
-  Error **errp)
+/* Parameter check and block job starting for mirroring.
+ * Caller should hold @device and @target's aio context (They must to be on the
+ * same aio context). */
+static void blockdev_mirror_common(BlockDriverState *bs,
+   BlockDriverState *target,
+   bool has_replaces, const char *replaces,
+   enum MirrorSyncMode sync,
+   bool has_speed, int64_t speed,
+   bool has_granularity, uint32_t granularity,
+   bool has_buf_size, int64_t buf_size,
+   bool has_on_source_error,
+   BlockdevOnError on_source_error,
+   bool has_on_target_error,
+   BlockdevOnError on_target_error,
+   Error **errp)
 {
-BlockBackend *blk;
-BlockDriverState *bs;
-BlockDriverState *source, *target_bs;
-AioContext *aio_context;
-BlockDriver *drv = NULL;
-Error *local_err = NULL;
-QDict *options = NULL;
-int flags;
-int64_t size;
-int ret;
 
 if (!has_speed) {
 speed = 0;
@@ -2916,9 +2909,6 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 if (!has_on_target_error) {
 on_target_error = BLOCKDEV_ON_ERROR_REPORT;
 }
-if (!has_mode) {
-mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
-}
 if (!has_granularity) {
 granularity = 0;
 }
@@ -2936,35 +2926,76 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 return;
 }
 
-blk = blk_by_name(device);
-if (!blk) {
-error_set(errp, QERR_DEVICE_NOT_FOUND, device);
-return;
-}
-
-aio_context = blk_get_aio_context(blk);
-aio_context_acquire(aio_context);
-
-if (!blk_is_available(blk)) {
-error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
-goto out;
-}
-bs = blk_bs(blk);
-
-if (!has_format) {
-format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
-}
-if (format) {
-drv = bdrv_find_format(format);
-if (!drv) {
-error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-goto out;
-}
-}
-
 if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
+return;
+}
+
+if (!bs->backing_hd && sync == MIRROR_SYNC_MODE_TOP) {
+sync = MIRROR_SYNC_MODE_FULL;
+}
+
+/* pass the node name to replace to mirror start since it's loose coupling
+ * and will allow to check whether the node still exist at mirror 
completion
+ */
+mirror_start(bs, target,
+ has_replaces ? replaces : NULL,
+ speed, granularity, buf_size, sync,
+ on_source_error, on_target_error,
+ block_job_cb, bs, errp);
+}
+
+void qmp_drive_mirror(const char *device, const char *target,
+  bool has_format, const char *format,
+  bool has_node_name, const char *node_name,
+  bool has_replaces, const char *replaces,
+  enum MirrorSyncMode sync,
+  bool has_mode, enum NewImageMode mode,
+  bool has_speed, int64_t speed,
+  bool has_granularity, uint32_t granularity,
+  bool has_buf_size, int64_t buf_size,
+  bool has_on_source_error, BlockdevOnError 
on_source_error,
+  bool has_on_target_error, BlockdevOnError 
on_target_error,
+  Error **errp)
+{
+BlockDriverState *bs;
+BlockBackend *blk;
+BlockDriverState *source, *target_bs;
+AioContext *aio_context;
+B

[Qemu-devel] [PATCH v2 5/6] qmp: Add blockdev-mirror command

[Fam Zheng]

This will start a mirror job from a named device to another named
device, its relation with drive-mirror is similar with blockdev-backup
to drive-backup.

In blockdev-mirror, the target node should be prepared by blockdev-add,
which will be responsible for assigning a name to the new node, so
'node-name' in drive-mirror is dropped.

Signed-off-by: Fam Zheng 
---
 blockdev.c   | 61 
 qapi/block-core.json | 47 
 qmp-commands.hx  | 48 +
 3 files changed, 156 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index de6383f..e0dba07 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2932,6 +2932,10 @@ static void blockdev_mirror_common(BlockDriverState *bs,
 if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_MIRROR_TARGET, errp)) {
 return;
 }
+if (target->blk) {
+error_setg(errp, "Cannot mirror to an attached block device");
+return;
+}
 
 if (!bs->backing_hd && sync == MIRROR_SYNC_MODE_TOP) {
 sync = MIRROR_SYNC_MODE_FULL;
@@ -3107,6 +3111,63 @@ out:
 aio_context_release(aio_context);
 }
 
+void qmp_blockdev_mirror(const char *device, const char *target,
+ bool has_replaces, const char *replaces,
+ MirrorSyncMode sync,
+ bool has_speed, int64_t speed,
+ bool has_granularity, uint32_t granularity,
+ bool has_buf_size, int64_t buf_size,
+ bool has_on_source_error,
+ BlockdevOnError on_source_error,
+ bool has_on_target_error,
+ BlockdevOnError on_target_error,
+ Error **errp)
+{
+BlockDriverState *bs;
+BlockBackend *blk;
+BlockDriverState *target_bs;
+AioContext *aio_context;
+Error *local_err = NULL;
+
+blk = blk_by_name(device);
+if (!blk) {
+error_setg(errp, "Device '%s' not found", device);
+return;
+}
+bs = blk_bs(blk);
+
+if (!bs) {
+error_setg(errp, "Device '%s' has no media", device);
+return;
+}
+
+target_bs = bdrv_lookup_bs(target, target, errp);
+if (!target_bs) {
+return;
+}
+
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
+bdrv_ref(target_bs);
+bdrv_set_aio_context(target_bs, aio_context);
+
+blockdev_mirror_common(bs, target_bs,
+   has_replaces, replaces, sync,
+   has_speed, speed,
+   has_granularity, granularity,
+   has_buf_size, buf_size,
+   has_on_source_error, on_source_error,
+   has_on_target_error, on_target_error,
+   &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+bdrv_unref(target_bs);
+}
+
+aio_context_release(aio_context);
+}
+
 /* Get the block job for a given device name and acquire its AioContext */
 static BlockJob *find_block_job(const char *device, AioContext **aio_context,
 Error **errp)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b5c4559..fe440e1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1059,6 +1059,53 @@
   'data': 'BlockDirtyBitmap' }
 
 ##
+# @blockdev-mirror
+#
+# Start mirroring a block device's writes to a new destination.
+#
+# @device: the name of the device whose writes should be mirrored.
+#
+# @target: the name of the device to mirror to.
+#
+# @replaces: #optional with sync=full graph node name to be replaced by the new
+#image when a whole image copy is done. This can be used to repair
+#broken Quorum files.
+#
+# @speed:  #optional the maximum speed, in bytes per second
+#
+# @sync: what parts of the disk image should be copied to the destination
+#(all the disk, only the sectors allocated in the topmost image, or
+#only new I/O).
+#
+# @granularity: #optional granularity of the dirty bitmap, default is 64K
+#   if the image format doesn't have clusters, 4K if the clusters
+#   are smaller than that, else the cluster size.  Must be a
+#   power of 2 between 512 and 64M
+#
+# @buf-size: #optional maximum amount of data in flight from source to
+#target
+#
+# @on-source-error: #optional the action to take on an error on the source,
+#   default 'report'.  'stop' and 'enospc' can only be used
+#   if the block device supports io-status (see BlockInfo).
+#
+# @on-target-error: #optional the action to take on an error on the target,
+#   default 'report' (no limitations, since this applies to
+#   a different block device than @device).
+#
+# Returns: nothing on suc

[Qemu-devel] [PATCH v2 4/6] block: Add check on mirror target

[Fam Zheng]

Signed-off-by: Fam Zheng 
Reviewed-by: Max Reitz 
---
 blockdev.c| 3 +++
 include/block/block.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 2a0c0e2..de6383f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2929,6 +2929,9 @@ static void blockdev_mirror_common(BlockDriverState *bs,
 if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
 return;
 }
+if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_MIRROR_TARGET, errp)) {
+return;
+}
 
 if (!bs->backing_hd && sync == MIRROR_SYNC_MODE_TOP) {
 sync = MIRROR_SYNC_MODE_FULL;
diff --git a/include/block/block.h b/include/block/block.h
index 69cb676..c96d47b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -156,6 +156,7 @@ typedef enum BlockOpType {
 BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
 BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
 BLOCK_OP_TYPE_MIRROR_SOURCE,
+BLOCK_OP_TYPE_MIRROR_TARGET,
 BLOCK_OP_TYPE_RESIZE,
 BLOCK_OP_TYPE_STREAM,
 BLOCK_OP_TYPE_REPLACE,
-- 
2.4.3

[Qemu-devel] [PATCH v2 0/6] qmp: Add blockdev-mirror

[Fam Zheng]

v2: 01: Move bdrv_op_block_all down. [Max]
02, 04: Add Max's rev-by.
03: Check has_mode and fix "return;". [Max]
05: Check target->blk.
Drop superfluous whitespace. [Max]
06: Drop superfluous whitespace hunk and add Max's rev-by. [Max]

This is the counterpart of blockdev-backup. The biggest value of this command
is to allow full flexibility on target image open options, via blockdev-add.
For example this could help solve the target provisioning issue in:

http://lists.gnu.org/archive/html/qemu-devel/2015-06/msg02139.html

Because mirror job uses bdrv_swap, this series depends on Max's BlockBackend
series:

[PATCH v3 00/38] blockdev: BlockBackend and media
http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg01109.html

which makes it possible for blockdev-add to insert a BDS without BB (by
omitting "id=" while only specifying "node-name=").


Fam Zheng (6):
  block: Add blocker on mirror target
  block: Rename BLOCK_OP_TYPE_MIRROR to BLOCK_OP_TYPE_MIRROR_SOURCE
  block: Extract blockdev part of qmp_drive_mirror
  block: Add check on mirror target
  qmp: Add blockdev-mirror command
  iotests: Add test cases for blockdev-mirror

 block/mirror.c  |   2 +
 blockdev.c  | 175 ++--
 hw/block/dataplane/virtio-blk.c |   2 +-
 include/block/block.h   |   3 +-
 qapi/block-core.json|  47 +++
 qmp-commands.hx |  48 +++
 tests/qemu-iotests/041  |  99 ++-
 tests/qemu-iotests/041.out  |   4 +-
 8 files changed, 314 insertions(+), 66 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH v2 1/6] block: Add blocker on mirror target

[Fam Zheng]

In block/backup.c, we already check and add blocker on the target bs,
which is necessary so that it won't be intervened with other operations.

In block/mirror.c we should also protect the mirror target bs, because it
could have a node-name (drive-mirror ... node-name=XXX), and on top of
that it's safe to add blockdev-mirror.

Signed-off-by: Fam Zheng 
---
 block/mirror.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index 33c640f..0afe585 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -360,6 +360,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
 aio_context_release(replace_aio_context);
 }
 g_free(s->replaces);
+bdrv_op_unblock_all(s->target, s->common.blocker);
 bdrv_unref(s->target);
 block_job_completed(&s->common, data->ret);
 g_free(data);
@@ -695,6 +696,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 if (!s->dirty_bitmap) {
 return;
 }
+bdrv_op_block_all(target, s->common.blocker);
 bdrv_set_enable_write_cache(s->target, true);
 if (s->target->blk) {
 blk_set_on_error(s->target->blk, on_target_error, on_target_error);
-- 
2.4.3

Re: [Qemu-devel] [PATCH 3/6] block: Extract blockdev part of qmp_drive_mirror

[Fam Zheng]

On Wed, 06/24 18:34, Max Reitz wrote:
> On 09.06.2015 04:13, Fam Zheng wrote:
> >This is the part that will be reused by blockdev-mirror.
> >
> >Signed-off-by: Fam Zheng 
> >---
> >  blockdev.c | 158 
> > +++--
> >  1 file changed, 92 insertions(+), 66 deletions(-)
> >
> >diff --git a/blockdev.c b/blockdev.c
> >index b573e56..c32a9a9 100644
> >--- a/blockdev.c
> >+++ b/blockdev.c
> >@@ -2883,29 +2883,22 @@ out:
> >  #define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
> >-void qmp_drive_mirror(const char *device, const char *target,
> >-  bool has_format, const char *format,
> >-  bool has_node_name, const char *node_name,
> >-  bool has_replaces, const char *replaces,
> >-  enum MirrorSyncMode sync,
> >-  bool has_mode, enum NewImageMode mode,
> >-  bool has_speed, int64_t speed,
> >-  bool has_granularity, uint32_t granularity,
> >-  bool has_buf_size, int64_t buf_size,
> >-  bool has_on_source_error, BlockdevOnError 
> >on_source_error,
> >-  bool has_on_target_error, BlockdevOnError 
> >on_target_error,
> >-  Error **errp)
> >+/* Parameter check and block job starting for mirroring.
> >+ * Caller should hold @device and @target's aio context (They must to be on 
> >the
> >+ * same aio context). */
> >+static void blockdev_mirror_common(BlockDriverState *bs,
> >+   BlockDriverState *target,
> >+   bool has_replaces, const char *replaces,
> >+   enum MirrorSyncMode sync,
> >+   bool has_speed, int64_t speed,
> >+   bool has_granularity, uint32_t 
> >granularity,
> >+   bool has_buf_size, int64_t buf_size,
> >+   bool has_on_source_error,
> >+   BlockdevOnError on_source_error,
> >+   bool has_on_target_error,
> >+   BlockdevOnError on_target_error,
> >+   Error **errp)
> >  {
> >-BlockBackend *blk;
> >-BlockDriverState *bs;
> >-BlockDriverState *source, *target_bs;
> >-AioContext *aio_context;
> >-BlockDriver *drv = NULL;
> >-Error *local_err = NULL;
> >-QDict *options = NULL;
> >-int flags;
> >-int64_t size;
> >-int ret;
> >  if (!has_speed) {
> >  speed = 0;
> >@@ -2916,9 +2909,6 @@ void qmp_drive_mirror(const char *device, const char 
> >*target,
> >  if (!has_on_target_error) {
> >  on_target_error = BLOCKDEV_ON_ERROR_REPORT;
> >  }
> >-if (!has_mode) {
> >-mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
> >-}
> >  if (!has_granularity) {
> >  granularity = 0;
> >  }
> >@@ -2936,35 +2926,73 @@ void qmp_drive_mirror(const char *device, const char 
> >*target,
> >  return;
> >  }
> >-blk = blk_by_name(device);
> >-if (!blk) {
> >-error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> >-return;
> >-}
> >-
> >-aio_context = blk_get_aio_context(blk);
> >-aio_context_acquire(aio_context);
> >-
> >-if (!blk_is_available(blk)) {
> >-error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> >-goto out;
> >-}
> >-bs = blk_bs(blk);
> >-
> >-if (!has_format) {
> >-format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : 
> >bs->drv->format_name;
> >-}
> >-if (format) {
> >-drv = bdrv_find_format(format);
> >-if (!drv) {
> >-error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> >-goto out;
> >-}
> >-}
> >-
> >  if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
> >+return;
> >+}
> >+
> >+if (!bs->backing_hd && sync == MIRROR_SYNC_MODE_TOP) {
> >+sync = MIRROR_SYNC_MODE_FULL;
> >+}
> >+
> >+/* pass the node name to replace to mirror start since it's loose 
> >coupling
> >+ * and will allow to check whether the node still exist at mirror 
> >completion
> >+ */
> >+mirror_start(bs, target,
> >+ has_replaces ? replaces : NULL,
> >+ speed, granularity, buf_size, sync,
> >+ on_source_error, on_target_error,
> >+ block_job_cb, bs, errp);
> >+}
> >+
> >+void qmp_drive_mirror(const char *device, const char *target,
> >+  bool has_format, const char *format,
> >+  bool has_node_name, const char *node_name,
> >+  bool has_replaces, const char *replaces,
> >+  enum MirrorSyncMode sync,
> >+  bool has_mode, enum NewImageMode mode,
> >+  bool has_speed, int64_

Re: [Qemu-devel] [PATCH 1/6] block: Add blocker on mirror target

[Fam Zheng]

On Wed, 06/24 18:14, Max Reitz wrote:
> On 09.06.2015 04:13, Fam Zheng wrote:
> >In block/backup.c, we already check and add blocker on the target bs,
> >which is necessary so that it won't be intervened with other operations.
> >
> >In block/mirror.c we should also protect the mirror target bs, because it
> >could have a node-name (drive-mirror ... node-name=XXX), and on top of
> >that it's safe to add blockdev-mirror.
> >
> >Signed-off-by: Fam Zheng 
> >---
> >  block/mirror.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> >diff --git a/block/mirror.c b/block/mirror.c
> >index 33c640f..8b23ddd 100644
> >--- a/block/mirror.c
> >+++ b/block/mirror.c
> >@@ -360,6 +360,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
> >  aio_context_release(replace_aio_context);
> >  }
> >  g_free(s->replaces);
> >+bdrv_op_unblock_all(s->target, s->common.blocker);
> >  bdrv_unref(s->target);
> >  block_job_completed(&s->common, data->ret);
> >  g_free(data);
> >@@ -682,6 +683,7 @@ static void mirror_start_job(BlockDriverState *bs, 
> >BlockDriverState *target,
> >  return;
> >  }
> >+bdrv_op_block_all(target, s->common.blocker);
> >  s->replaces = g_strdup(replaces);
> >  s->on_source_error = on_source_error;
> >  s->on_target_error = on_target_error;
> 
> Do we need a bdrv_op_unblock_all() if bdrv_create_dirty_bitmap() fails?
> 

Yes, good catch!

Fam

[Qemu-devel] [RESEND PATCH v8 4/4] icc_bus: drop the unused files

[Zhu Guihua]

ICC bus impl has been droped, so all icc related files are not useful
any more; delete them.

Signed-off-by: Zhu Guihua 
---
 default-configs/i386-softmmu.mak   |   1 -
 default-configs/x86_64-softmmu.mak |   1 -
 hw/cpu/Makefile.objs   |   1 -
 hw/cpu/icc_bus.c   | 118 -
 include/hw/cpu/icc_bus.h   |  82 --
 5 files changed, 203 deletions(-)
 delete mode 100644 hw/cpu/icc_bus.c
 delete mode 100644 include/hw/cpu/icc_bus.h

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 91d602c..0759f22 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -42,7 +42,6 @@ CONFIG_LPC_ICH9=y
 CONFIG_PCI_Q35=y
 CONFIG_APIC=y
 CONFIG_IOAPIC=y
-CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
 CONFIG_XIO3130=y
diff --git a/default-configs/x86_64-softmmu.mak 
b/default-configs/x86_64-softmmu.mak
index 62575eb..08aac8c 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -43,7 +43,6 @@ CONFIG_LPC_ICH9=y
 CONFIG_PCI_Q35=y
 CONFIG_APIC=y
 CONFIG_IOAPIC=y
-CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
 CONFIG_XIO3130=y
diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
index 6381238..0954a18 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -2,5 +2,4 @@ obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
 obj-$(CONFIG_REALVIEW) += realview_mpcore.o
 obj-$(CONFIG_A9MPCORE) += a9mpcore.o
 obj-$(CONFIG_A15MPCORE) += a15mpcore.o
-obj-$(CONFIG_ICC_BUS) += icc_bus.o
 
diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
deleted file mode 100644
index 6646ea2..000
--- a/hw/cpu/icc_bus.c
+++ /dev/null
@@ -1,118 +0,0 @@
-/* icc_bus.c
- * emulate x86 ICC (Interrupt Controller Communications) bus
- *
- * Copyright (c) 2013 Red Hat, Inc
- *
- * Authors:
- * Igor Mammedov 
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see 
- */
-#include "hw/cpu/icc_bus.h"
-#include "hw/sysbus.h"
-
-/* icc-bridge implementation */
-
-static const TypeInfo icc_bus_info = {
-.name = TYPE_ICC_BUS,
-.parent = TYPE_BUS,
-.instance_size = sizeof(ICCBus),
-};
-
-
-/* icc-device implementation */
-
-static void icc_device_realize(DeviceState *dev, Error **errp)
-{
-ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
-
-/* convert to QOM */
-if (idc->realize) {
-idc->realize(dev, errp);
-}
-
-}
-
-static void icc_device_class_init(ObjectClass *oc, void *data)
-{
-DeviceClass *dc = DEVICE_CLASS(oc);
-
-dc->realize = icc_device_realize;
-dc->bus_type = TYPE_ICC_BUS;
-}
-
-static const TypeInfo icc_device_info = {
-.name = TYPE_ICC_DEVICE,
-.parent = TYPE_DEVICE,
-.abstract = true,
-.instance_size = sizeof(ICCDevice),
-.class_size = sizeof(ICCDeviceClass),
-.class_init = icc_device_class_init,
-};
-
-
-/*  icc-bridge implementation */
-
-typedef struct ICCBridgeState {
-/*< private >*/
-SysBusDevice parent_obj;
-/*< public >*/
-
-ICCBus icc_bus;
-MemoryRegion apic_container;
-} ICCBridgeState;
-
-#define ICC_BRIDGE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
-
-static void icc_bridge_init(Object *obj)
-{
-ICCBridgeState *s = ICC_BRIDGE(obj);
-SysBusDevice *sb = SYS_BUS_DEVICE(obj);
-
-qbus_create_inplace(&s->icc_bus, sizeof(s->icc_bus), TYPE_ICC_BUS,
-DEVICE(s), "icc");
-
-/* Do not change order of registering regions,
- * APIC must be first registered region, board maps it by 0 index
- */
-memory_region_init(&s->apic_container, obj, "icc-apic-container",
-   APIC_SPACE_SIZE);
-sysbus_init_mmio(sb, &s->apic_container);
-s->icc_bus.apic_address_space = &s->apic_container;
-}
-
-static void icc_bridge_class_init(ObjectClass *oc, void *data)
-{
-DeviceClass *dc = DEVICE_CLASS(oc);
-
-set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-}
-
-static const TypeInfo icc_bridge_info = {
-.name  = TYPE_ICC_BRIDGE,
-.parent = TYPE_SYS_BUS_DEVICE,
-.instance_init  = icc_bridge_init,
-.instance_size  = sizeof(ICCBridgeState),
-.class_init = icc_bridge_class_init,
-};
-
-
-static void icc_bus_register_types(void)
-{
-type_register_static(&icc_bus_info);
-type_register_static(&icc_device_info);
-type_register_static(

[Qemu-devel] [RESEND PATCH v8 3/4] cpu/apic: drop icc bus/bridge

[Zhu Guihua]

From: Chen Fan 

After CPU hotplug has been converted to BUS-less hot-plug infrastructure,
the only function ICC bus performs is to propagate reset to LAPICs. However
LAPIC could be reset by registering its reset handler after all device are
initialized.
Do so and drop ~200LOC of not needed anymore ICCBus related code.

Signed-off-by: Chen Fan 
Signed-off-by: Zhu Guihua 
---
 hw/i386/pc.c| 24 +---
 hw/i386/pc_piix.c   |  9 +
 hw/i386/pc_q35.c|  9 +
 hw/intc/apic_common.c   |  5 ++---
 include/hw/i386/apic_internal.h |  7 ---
 include/hw/i386/pc.h|  2 +-
 target-i386/cpu.c   | 23 +++
 target-i386/cpu.h   |  1 +
 8 files changed, 34 insertions(+), 46 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9f16128..c547d74 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -59,7 +59,6 @@
 #include "qemu/error-report.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/cpu_hotplug.h"
-#include "hw/cpu/icc_bus.h"
 #include "hw/boards.h"
 #include "hw/pci/pci_host.h"
 #include "acpi-build.h"
@@ -969,27 +968,25 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int 
level)
 }
 }
 
+#define x86_cpu_apic_reset_order 0x1
+
 static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
-  DeviceState *icc_bridge, Error **errp)
+  Error **errp)
 {
 X86CPU *cpu = NULL;
 Error *local_err = NULL;
 
-if (icc_bridge == NULL) {
-error_setg(&local_err, "Invalid icc-bridge value");
-goto out;
-}
-
 cpu = cpu_x86_create(cpu_model, &local_err);
 if (local_err != NULL) {
 goto out;
 }
 
-qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc"));
-
 object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err);
 object_property_set_bool(OBJECT(cpu), true, "realized", &local_err);
 
+qemu_register_reset_common(x86_cpu_apic_reset, cpu,
+   x86_cpu_apic_reset_order);
+
 out:
 if (local_err) {
 error_propagate(errp, local_err);
@@ -1003,7 +1000,6 @@ static const char *current_cpu_model;
 
 void pc_hot_add_cpu(const int64_t id, Error **errp)
 {
-DeviceState *icc_bridge;
 X86CPU *cpu;
 int64_t apic_id = x86_cpu_apic_id_from_index(id);
 Error *local_err = NULL;
@@ -1032,9 +1028,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
 return;
 }
 
-icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
- TYPE_ICC_BRIDGE, NULL));
-cpu = pc_new_cpu(current_cpu_model, apic_id, icc_bridge, &local_err);
+cpu = pc_new_cpu(current_cpu_model, apic_id, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 return;
@@ -1042,7 +1036,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
 object_unref(OBJECT(cpu));
 }
 
-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
+void pc_cpus_init(const char *cpu_model)
 {
 int i;
 X86CPU *cpu = NULL;
@@ -1068,7 +1062,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
*icc_bridge)
 
 for (i = 0; i < smp_cpus; i++) {
 cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
- icc_bridge, &error);
+ &error);
 if (error) {
 error_report_err(error);
 exit(1);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e142f75..7e9a185 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -39,7 +39,6 @@
 #include "hw/kvm/clock.h"
 #include "sysemu/sysemu.h"
 #include "hw/sysbus.h"
-#include "hw/cpu/icc_bus.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/block-backend.h"
 #include "hw/i2c/smbus.h"
@@ -98,7 +97,6 @@ static void pc_init1(MachineState *machine)
 MemoryRegion *ram_memory;
 MemoryRegion *pci_memory;
 MemoryRegion *rom_memory;
-DeviceState *icc_bridge;
 PcGuestInfo *guest_info;
 ram_addr_t lowmem;
 
@@ -142,11 +140,7 @@ static void pc_init1(MachineState *machine)
 exit(1);
 }
 
-icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
-object_property_add_child(qdev_get_machine(), "icc-bridge",
-  OBJECT(icc_bridge), NULL);
-
-pc_cpus_init(machine->cpu_model, icc_bridge);
+pc_cpus_init(machine->cpu_model);
 
 if (kvm_enabled() && kvmclock_enabled) {
 kvmclock_create();
@@ -229,7 +223,6 @@ static void pc_init1(MachineState *machine)
 if (pci_enabled) {
 ioapic_init_gsi(gsi_state, "i440fx");
 }
-qdev_init_nofail(icc_bridge);
 
 pc_register_ferr_irq(gsi[13]);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 082cd93..9af1d06 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -43,7 +43,6 @@
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
 #include "hw/usb.h"
-#include "hw/cpu/icc_bus.h"
 #include "q

[Qemu-devel] [RESEND PATCH v8 1/4] apic: map APIC's MMIO region at each CPU's address space

[Zhu Guihua]

From: Chen Fan 

Replace mapping APIC at global system address space with
mapping it at per-CPU address spaces.

Signed-off-by: Chen Fan 
Signed-off-by: Zhu Guihua 
---
 exec.c|  5 +
 hw/i386/pc.c  |  7 ---
 hw/intc/apic_common.c | 14 --
 include/exec/memory.h |  5 +
 target-i386/cpu.c |  2 ++
 5 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/exec.c b/exec.c
index f7883d2..1cd2e74 100644
--- a/exec.c
+++ b/exec.c
@@ -2710,6 +2710,11 @@ void address_space_unmap(AddressSpace *as, void *buffer, 
hwaddr len,
 cpu_notify_map_clients();
 }
 
+MemoryRegion *address_space_root_memory_region(AddressSpace *as)
+{
+return as->root;
+}
+
 void *cpu_physical_memory_map(hwaddr addr,
   hwaddr *plen,
   int is_write)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7072930..9f16128 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1076,13 +1076,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
*icc_bridge)
 object_unref(OBJECT(cpu));
 }
 
-/* map APIC MMIO area if CPU has APIC */
-if (cpu && cpu->apic_state) {
-/* XXX: what if the base changes? */
-sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
-APIC_DEFAULT_ADDRESS, 0x1000);
-}
-
 /* tell smbios about cpuid version and features */
 smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
 }
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 0032b97..cf105f5 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -296,7 +296,8 @@ static void apic_common_realize(DeviceState *dev, Error 
**errp)
 APICCommonClass *info;
 static DeviceState *vapic;
 static int apic_no;
-static bool mmio_registered;
+CPUState *cpu = CPU(s->cpu);
+MemoryRegion *root;
 
 if (apic_no >= MAX_APICS) {
 error_setg(errp, "%s initialization failed.",
@@ -307,11 +308,12 @@ static void apic_common_realize(DeviceState *dev, Error 
**errp)
 
 info = APIC_COMMON_GET_CLASS(s);
 info->realize(dev, errp);
-if (!mmio_registered) {
-ICCBus *b = ICC_BUS(qdev_get_parent_bus(dev));
-memory_region_add_subregion(b->apic_address_space, 0, &s->io_memory);
-mmio_registered = true;
-}
+
+root = address_space_root_memory_region(cpu->as);
+memory_region_add_subregion_overlap(root,
+s->apicbase & MSR_IA32_APICBASE_BASE,
+&s->io_memory,
+0x1000);
 
 /* Note: We need at least 1M to map the VAPIC option ROM */
 if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8ae004e..811f027 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1308,6 +1308,11 @@ void *address_space_map(AddressSpace *as, hwaddr addr,
 void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
  int is_write, hwaddr access_len);
 
+/* address_space_root_memory_region: get root memory region
+ *
+ * @as: #AddressSpace to be accessed
+ */
+MemoryRegion *address_space_root_memory_region(AddressSpace *as);
 
 #endif
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 36b07f9..1fb88f6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2741,6 +2741,8 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 /* TODO: convert to link<> */
 apic = APIC_COMMON(cpu->apic_state);
 apic->cpu = cpu;
+cpu_set_apic_base(cpu->apic_state,
+  APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE);
 }
 
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
-- 
1.9.3

[Qemu-devel] [RESEND PATCH v8 0/4] remove icc bus/bridge

[Zhu Guihua]

ICC Bus was used for providing a hotpluggable bus for APIC and CPU,
but now we use HotplugHandler to make hotplug. So ICC Bus is
unnecessary.

This code has passed the new pc-cpu-test.
And I have tested with kvm along with kernel_irqchip=on/off,
it works fine.

This patch series is based on the latest master.

v8:
 -add a wrapper to specify reset order

v7:
 -update to register reset handler for main_system_bus when created
 -register reset handler for apic after all devices are initialized

v6:
 -reword commit message
 -drop NULL check for APIC device
 -use C cast instead of QOM cast

v5:
 -convert DEVICE() casts to C casts
 -use a local variable instead of doing the cast inline twice
 -drop to set cpu's parent bus
 -rename patch 3's subject
 -fix a bug about setting cpu's apic base

v4:
 -add wrapper to get root memory region from address space
 -set cpu apic base's default value in x86_cpu_apic_create()
 -drop NULL check for cpu apic_state
 -put drop of the unused files about icc_bus into a seprate patch
 -put DEVICE() casts into a seprate patch

v3:
 -replace init apic by object_new()
 -add reset apic at the time of CPU reset

Chen Fan (2):
  apic: map APIC's MMIO region at each CPU's address space
  cpu/apic: drop icc bus/bridge

Zhu Guihua (2):
  hw: add a wrapper for registering reset handler
  icc_bus: drop the unused files

 default-configs/i386-softmmu.mak   |   1 -
 default-configs/x86_64-softmmu.mak |   1 -
 exec.c |   5 ++
 hw/cpu/Makefile.objs   |   1 -
 hw/cpu/icc_bus.c   | 118 -
 hw/i386/pc.c   |  31 +++---
 hw/i386/pc_piix.c  |   9 +--
 hw/i386/pc_q35.c   |   9 +--
 hw/intc/apic_common.c  |  19 +++---
 include/exec/memory.h  |   5 ++
 include/hw/cpu/icc_bus.h   |  82 --
 include/hw/hw.h|   4 ++
 include/hw/i386/apic_internal.h|   7 ++-
 include/hw/i386/pc.h   |   2 +-
 target-i386/cpu.c  |  25 +---
 target-i386/cpu.h  |   1 +
 vl.c   |  18 +-
 17 files changed, 75 insertions(+), 263 deletions(-)
 delete mode 100644 hw/cpu/icc_bus.c
 delete mode 100644 include/hw/cpu/icc_bus.h

-- 
1.9.3

[Qemu-devel] [RESEND PATCH v8 2/4] hw: add a wrapper for registering reset handler

[Zhu Guihua]

Add a wrapper to specify reset order when registering reset handler,
instead of non-obvious initiazation code ordering.

Signed-off-by: Zhu Guihua 
---
 include/hw/hw.h |  4 
 vl.c| 18 +-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/hw/hw.h b/include/hw/hw.h
index c78adae..d9375e7 100644
--- a/include/hw/hw.h
+++ b/include/hw/hw.h
@@ -37,7 +37,11 @@
 #endif
 
 typedef void QEMUResetHandler(void *opaque);
+typedef uint64_t QEMUResetOrder;
+#define default_reset_order 0x0
 
+void qemu_register_reset_common(QEMUResetHandler *func, void *opaque,
+QEMUResetOrder reset_order);
 void qemu_register_reset(QEMUResetHandler *func, void *opaque);
 void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
 
diff --git a/vl.c b/vl.c
index 69ad90c..b205a9b 100644
--- a/vl.c
+++ b/vl.c
@@ -1589,6 +1589,7 @@ typedef struct QEMUResetEntry {
 QTAILQ_ENTRY(QEMUResetEntry) entry;
 QEMUResetHandler *func;
 void *opaque;
+QEMUResetOrder reset_order;
 } QEMUResetEntry;
 
 static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
@@ -1672,15 +1673,30 @@ static int qemu_debug_requested(void)
 return r;
 }
 
-void qemu_register_reset(QEMUResetHandler *func, void *opaque)
+void qemu_register_reset_common(QEMUResetHandler *func, void *opaque,
+QEMUResetOrder reset_order)
 {
+QEMUResetEntry *item;
 QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
 
 re->func = func;
 re->opaque = opaque;
+re->reset_order = reset_order;
+
+QTAILQ_FOREACH(item, &reset_handlers, entry) {
+if (re->reset_order >= item->reset_order)
+continue;
+QTAILQ_INSERT_BEFORE(item, re, entry);
+return;
+}
 QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
 }
 
+void qemu_register_reset(QEMUResetHandler *func, void *opaque)
+{
+qemu_register_reset_common(func, opaque, default_reset_order);
+}
+
 void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
 {
 QEMUResetEntry *re;
-- 
1.9.3

Re: [Qemu-devel] [PATCH qom v3 4/4] microblaze: boot: Use cpu_set_pc()

[Peter Crosthwaite]

On Wed, Jun 24, 2015 at 11:29 AM, Andreas Färber  wrote:
> Am 24.06.2015 um 20:00 schrieb Andreas Färber:
>> Am 24.06.2015 um 05:19 schrieb Peter Crosthwaite:
>>> Use cpu_set_pc() for setting program counters when bootloading. This
>>> removes an instance of system level code having to reach into the CPU
>>> env.
>>>
>>> Reviewed-by: Andreas Färber 
>>> Signed-off-by: Peter Crosthwaite 
>>> ---
>>> changed since v2:
>>> Add () to function names in commit messages
>>> ---
>>>  dtc  | 2 +-
>>>  hw/microblaze/boot.c | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/dtc b/dtc
>>> index 65cc4d2..bc895d6 16
>>> --- a/dtc
>>> +++ b/dtc
>>> @@ -1 +1 @@
>>> -Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
>>> +Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde
>>
>> Submodule strikes again. Preparing to queue.
>
> Will squash the following deduplication:
>

ACK.

> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> index 9f4698a..3e8820f 100644
> --- a/hw/microblaze/boot.c
> +++ b/hw/microblaze/boot.c
> @@ -48,13 +48,14 @@ static struct
>  static void main_cpu_reset(void *opaque)
>  {
>  MicroBlazeCPU *cpu = opaque;
> +CPUState *cs = CPU(cpu);
>  CPUMBState *env = &cpu->env;
>
> -cpu_reset(CPU(cpu));
> +cpu_reset(cs);
>  env->regs[5] = boot_info.cmdline;
>  env->regs[6] = boot_info.initrd_start;
>  env->regs[7] = boot_info.fdt;
> -cpu_set_pc(CPU(cpu), boot_info.bootstrap_pc);
> +cpu_set_pc(cs, boot_info.bootstrap_pc);
>  if (boot_info.machine_cpu_reset) {
>  boot_info.machine_cpu_reset(cpu);
>  }
>
> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
>

[Qemu-devel] [Bug 1467240] Re: Regression - bridged networking broken for Mac OS X guest

[Jonathan Liu]

Is there anything else you would like me to test?

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

Title:
  Regression - bridged networking broken for Mac OS X guest

Status in QEMU:
  New

Bug description:
  Using the instructions at
  http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/ for running Mac OS X
  Snow Leopard under QEMU, bridged networking is broken when using QEMU
  git. The result is that Mac OS X is unable to obtain an IP address
  using DHCP. It works in the latest stable release - QEMU 2.3.0.

  Replace "-netdev user,id=hub0port0" with "-netdev
  bridge,br=br0,id=hub0port0" when testing bridged networking.

  Bisecting the git repository shows the following bad commit:
  commit a90a7425cf592a3afeff3eaf32f543b83050ee5c
  Author: Fam Zheng 
  Date:   Thu Jun 4 14:45:17 2015 +0800

  tap: Drop tap_can_send

  This callback is called by main loop before polling s->fd, if it returns
  false, the fd will not be polled in this iteration.

  This is redundant with checks inside read callback. After this patch,
  the data will be sent to peer when it arrives. If the device can't
  receive, it will be queued to incoming_queue, and when the device status
  changes, this queue will be flushed.

  Signed-off-by: Fam Zheng 
  Message-id: 1433400324-7358-7-git-send-email-f...@redhat.com
  Signed-off-by: Stefan Hajnoczi 

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

Re: [Qemu-devel] [PATCH] gdb command: qemu iohandlers

[Fam Zheng]

On Wed, 06/24 16:19, Stefan Hajnoczi wrote:
> On Tue, Jun 23, 2015 at 03:43:53PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Add a gdb command to print the current set of IOHandlers and
> > if one of them is a thread yielding for data print the backtrace.
> > 
> > Useful for debugging why an incoming migration has stalled, e.g.
> > 
> >   
> >   {fd_read = 0x7fd4c8e40d00 , fd_write = 0x0, opaque =
> >   0x7fd4b8bfeb00, next = {le_next = 0x7fd4cac81b00, le_prev =
> >   0x7fd4c93d2bd0 }, fd = 22, pollfds_idx = 0, deleted =
> >   false}
> >   #0  qemu_coroutine_switch (from_=from_@entry=0x7fd4cbb33c00,
> >   to_=to_@entry=0x7fd4c8b13a90,
> >   action=action@entry=COROUTINE_YIELD) at coroutine-ucontext.c:177
> >   #1  0x7fd4c8e40507 in qemu_coroutine_yield () at
> >   qemu-coroutine.c:145
> >   #2  0x7fd4c8e40e75 in yield_until_fd_readable (fd=22) at
> >   qemu-coroutine-io.c:90
> >   #3  0x7fd4c8df347f in qemu_rdma_block_for_wrid
> >   (rdma=rdma@entry=0x7fd4b8c7e010,
> >   wrid_requested=wrid_requested@entry=2000,
> >   byte_len=byte_len@entry=0x0) at migration/rdma.c:1510
> >   #4  0x7fd4c8df388f in qemu_rdma_post_send_control
> >   (rdma=rdma@entry=0x7fd4b8c7e010, buf=buf@entry=0x0,
> >   head=head@entry=0x7fd4b8bfed00) at migration/rdma.c:1608
> >   #5  0x7fd4c8df4b8e in qemu_rdma_exchange_recv (rdma=0x7fd4b8c7e010,
> >   head=0x7fd4b8bfed50, expecting=3)
> >   at migration/rdma.c:1814
> >   #6  0x7fd4c8df5089 in qemu_rdma_get_buffer (opaque=0x7fd4cba34950,
> >   buf=0x7fd4cc24fd20 "TR\022",
> >   pos=, size=32768) at migration/rdma.c:2611
> >   #7  0x7fd4c8df000d in qemu_fill_buffer (f=f@entry=0x7fd4cc24fcf0) at
> >   migration/qemu-file.c:214
> >   #8  0x7fd4c8df08d4 in qemu_peek_byte (f=f@entry=0x7fd4cc24fcf0,
> >   offset=offset@entry=0)
> >   at migration/qemu-file.c:447
> >   #9  0x7fd4c8c2cad1 in qemu_loadvm_state (f=f@entry=0x7fd4cc24fcf0)
> >   at /root/qemu-world3/migration/savevm.c:1128
> >   #10 0x7fd4c8ded895 in process_incoming_migration_co
> >   (opaque=0x7fd4cc24fcf0) at migration/migration.c:145
> >   #11 0x7fd4c8e4112a in coroutine_trampoline (i0=,
> >   i1=) at coroutine-ucontext.c:80
> >   #12 0x7fd4c453e0f0 in ?? () from /lib64/libc.so.6
> >   #13 0x7ffee263e870 in ?? ()
> >   #14 0x in ?? ()
> >   
> >   {fd_read = 0x7fd4c8e3ace0 , fd_write = 0x0, opaque = 0x5,
> >   next = {le_next = 0x0, le_prev = 0x7fd4cb78a3d8}, fd = 5, pollfds_idx =
> >   1, deleted = false}
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  scripts/qemu-gdb.py | 25 +
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/scripts/qemu-gdb.py b/scripts/qemu-gdb.py
> > index 6c7f4fb..2670625 100644
> > --- a/scripts/qemu-gdb.py
> > +++ b/scripts/qemu-gdb.py
> > @@ -102,6 +102,30 @@ class CoroutineCommand(gdb.Command):
> >  coroutine_pointer = 
> > gdb.parse_and_eval(argv[0]).cast(gdb.lookup_type('CoroutineUContext').pointer())
> >  bt_jmpbuf(coroutine_pointer['env']['__jmpbuf'])
> >  
> > +class IOhandlersCommand(gdb.Command):
> > +'''Display a backtrace for io handlers'''
> > +def __init__(self):
> > +gdb.Command.__init__(self, 'qemu iohandlers', gdb.COMMAND_DATA,
> > + gdb.COMPLETE_NONE)
> > +
> > +def invoke(self, arg, from_tty):
> > +cur = gdb.parse_and_eval('io_handlers.lh_first')
> 
> Fam is getting rid of io_handlers and replacing it with an AioContext.
> 

Yes, the series is

  [PATCH 0/9] slirp: iohandler: Rebase onto aio

Fam

Re: [Qemu-devel] [Qemu-stable] [PATCH v7 0/8] block: Mirror discarded sectors

[Fam Zheng]

On Wed, 06/24 19:01, Paolo Bonzini wrote:
> 
> 
> On 24/06/2015 11:08, Fam Zheng wrote:
> >> Stefan,
> >>
> >> The only controversial patches are the qmp/drive-mirror ones (1-3), while
> >> patches 4-8 are still useful on their own: they fix the mentioned crash and
> >> improve iotests.
> >>
> >> Shall we merge the second half (of course none of them depend on 1-3) now 
> >> that
> >> softfreeze is approaching?
> > 
> > Stefan, would you consider applying patches 4-8?
> 
> Actually why not apply all of them?  Even if blockdev-mirror is a
> superior interface in the long run, the current behavior of drive-mirror
> can cause images to balloon up to the full size, which is bad.
> Extending drive-mirror is okay IMHO for 2.4.
> 

Before we do that, Andrey Korolyov has reported a hang issue with unmap=true,
I'll take a look at it today.

Fam

Re: [Qemu-devel] [Qemu-block] [PATCH COLO-Block v6 07/16] Add new block driver interface to connect/disconnect the remote target

[Wen Congyang]

On 06/24/2015 10:07 PM, Dr. David Alan Gilbert wrote:
> * Wen Congyang (ghost...@gmail.com) wrote:
>> At 2015/6/19 18:49, Stefan Hajnoczi Wrote:
>>> On Fri, Jun 19, 2015 at 08:54:56AM +0800, Wen Congyang wrote:
 On 06/19/2015 12:06 AM, Stefan Hajnoczi wrote:
> On Thu, Jun 18, 2015 at 10:36:39PM +0800, Wen Congyang wrote:
>> At 2015/6/18 20:55, Stefan Hajnoczi Wrote:
>>> On Thu, Jun 18, 2015 at 04:49:12PM +0800, Wen Congyang wrote:
 +void bdrv_connect(BlockDriverState *bs, Error **errp)
 +{
 +BlockDriver *drv = bs->drv;
 +
 +if (drv && drv->bdrv_connect) {
 +drv->bdrv_connect(bs, errp);
 +} else if (bs->file) {
 +bdrv_connect(bs->file, errp);
 +} else {
 +error_setg(errp, "this feature or command is not currently 
 supported");
 +}
 +}
 +
 +void bdrv_disconnect(BlockDriverState *bs)
 +{
 +BlockDriver *drv = bs->drv;
 +
 +if (drv && drv->bdrv_disconnect) {
 +drv->bdrv_disconnect(bs);
 +} else if (bs->file) {
 +bdrv_disconnect(bs->file);
 +}
 +}
>>>
>>> Please add doc comments describing the semantics of these commands.
>>
>> Where should it be documented? In the header file?
>
> block.h doesn't document prototypes in the header file, please document
> the function definition in block.c.  (QEMU is not consistent here, some
> places do it the other way around.)
>
>>> Why are these operations needed when there is already a bs->drv == NULL
>>> case which means the BDS is not ready for read/write?
>>>
>>
>> The purpos is that: don't connect to nbd server when opening a nbd 
>> client.
>> connect/disconnect
>> to nbd server when we need to do it.
>>
>> IIUC, if bs->drv is NULL, it means that the driver is ejected? Here,
>> connect/disconnect
>> means that connect/disconnect to remote target(The target may be in 
>> another
>> host).
>
> Connect/disconnect puts something on the QEMU command-line that isn't
> ready at startup time.
>
> How about using monitor commands to add objects when needed instead?
>
> That is cleaner because it doesn't introduce a new state (which is only
> implemented for nbd).
>

 The problem is that, nbd client is one child of quorum, and quorum must 
 have more
 than one child. The nbd server is not ready until colo is running.
>>>
>>> A monitor command to hot add/remove quorum children solves this problem
>>> and could also be used in other scenarios (e.g. user decides to take a
>>> quorum child offline).
>>>
>>
>> For replication case, we always do checkpoint again and again after
>> migration. If the disk is not synced before migration, we will use disk 
>> mirgation or mirror
>> job to sync it.
> 
> Can you document the way that you use disk migration or mirror, together
> with your COLO setup?   I think it would make it easier to understand this
> restriction.
> At the moment I don't understand how you can switch from doing a disk 
> migration
> into COLO mode - there seems to be a gap between the end of disk migration 
> and the start
> of COLO.

In my test, I use disk migration.

After migration and before starting COLO, we will disable disk migration:
+/* Disable block migration */
+s->params.blk = 0;
+s->params.shared = 0;
+qemu_savevm_state_begin(trans, &s->params);

If the user uses mirror job, we don't cancel the mirror job now.

> 
>> So we cannot start block replication when migration is running. We need
>> that nbd
>> client is not ready when migration is running, and it is ready between
>> migration ends
>> and checkpoint begins. Using a monotir command add the nbd client will cause
>> larger
>> downtime. So if the nbd client has been added(only not connect to the nbd
>> server),
>> we can connect to nbd server automatically.
> 
> Without the disk migration or mirror, I can't see the need for the delayed 
> bdrv_connect.
> I can see that you want to do a disconnect at failover, however you need
> to take care because if the network is broken at the point of failover
> you need to make sure nothing blocks.

disk migration is needed if the disk is not synced before migration.

Thanks
Wen Congyang

> 
> Dave
> 
>>
>> Thanks
>> Wen Congyang
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> .
> 

Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)

[Alexander Spyridakis]

On 24 June 2015 at 17:34, Alex Bennée  wrote:
> Testing with Alexander's bare metal syncronisation tests fails in MTTCG
> leaving one CPU spinning forever waiting for the second CPU to wake up.
> We simply need to poke the halt_cond once we have processed the PSCI
> power on call.

Thanks Alex. Works for me, also with qemu_cpu_kick(target_cpu_state)
as Paolo mentioned.

The test seems to stress the current multi-threaded implementation
quite a lot. With 8 CPUs running, the resulting errors are in the
range of 500 per vCPU (10 million iterations).
Performance is another issue as mentioned before, but even more
pronounced with 8 cores. Upstream QEMU needs around 10 seconds to
complete, with multi-threading around 100 seconds for the same test.

Best regards.

[Qemu-devel] [PATCH v4 06/15] target-mips: raise RI exceptions when FIR.PS = 0

[Yongbok Kim]

64-bit paired-single (PS) floating point data type is optional in the
pre-Release 6.
It has to raise RI exception when PS type is not implemented. (FIR.PS = 0)
(The PS data type is removed in the Release 6.)
Loongson-2E and Loongson-2F don't have any implementation field in FCSR0(FIR)
but do support PS data format, therefore for these cores RI will not be
signalled regardless of PS bit.

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
---
 target-mips/translate.c |   78 +++
 1 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index dc9aae6..851a27c 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -1429,6 +1429,7 @@ typedef struct DisasContext {
 uint64_t PAMask;
 bool mvh;
 int CP0_LLAddr_shift;
+bool ps;
 } DisasContext;
 
 enum {
@@ -1825,6 +1826,16 @@ static inline void check_insn_opc_removed(DisasContext 
*ctx, int flags)
 }
 }
 
+/* This code generates a "reserved instruction" exception if the
+   CPU does not support 64-bit paired-single (PS) floating point data type */
+static inline void check_ps(DisasContext *ctx)
+{
+if (unlikely(!ctx->ps)) {
+generate_exception(ctx, EXCP_RI);
+}
+check_cp1_64bitmode(ctx);
+}
+
 #ifdef TARGET_MIPS64
 /* This code generates a "reserved instruction" exception if 64-bit
instructions are not enabled. */
@@ -1858,7 +1869,7 @@ static inline void gen_cmp ## type ## _ ## 
fmt(DisasContext *ctx, int n,  \
 TCGv_i##bits fp1 = tcg_temp_new_i##bits ();   \
 switch (ifmt) {   \
 case FMT_PS:  \
-check_cp1_64bitmode(ctx); \
+check_ps(ctx);\
 break;\
 case FMT_D:   \
 if (abs) {\
@@ -8998,7 +9009,6 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 };
 enum { BINOP, CMPOP, OTHEROP } optype = OTHEROP;
 uint32_t func = ctx->opcode & 0x3f;
-
 switch (op1) {
 case OPC_ADD_S:
 {
@@ -9491,8 +9501,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "cvt.l.s";
 break;
 case OPC_CVT_PS_S:
-check_insn_opc_removed(ctx, ISA_MIPS32R6);
-check_cp1_64bitmode(ctx);
+check_ps(ctx);
 {
 TCGv_i64 fp64 = tcg_temp_new_i64();
 TCGv_i32 fp32_0 = tcg_temp_new_i32();
@@ -10109,8 +10118,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "cvt.d.l";
 break;
 case OPC_CVT_PS_PW:
-check_insn_opc_removed(ctx, ISA_MIPS32R6);
-check_cp1_64bitmode(ctx);
+check_ps(ctx);
 {
 TCGv_i64 fp0 = tcg_temp_new_i64();
 
@@ -10122,7 +10130,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "cvt.ps.pw";
 break;
 case OPC_ADD_PS:
-check_cp1_64bitmode(ctx);
+check_ps(ctx);
 {
 TCGv_i64 fp0 = tcg_temp_new_i64();
 TCGv_i64 fp1 = tcg_temp_new_i64();
@@ -10137,7 +10145,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "add.ps";
 break;
 case OPC_SUB_PS:
-check_cp1_64bitmode(ctx);
+check_ps(ctx);
 {
 TCGv_i64 fp0 = tcg_temp_new_i64();
 TCGv_i64 fp1 = tcg_temp_new_i64();
@@ -10152,7 +10160,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "sub.ps";
 break;
 case OPC_MUL_PS:
-check_cp1_64bitmode(ctx);
+check_ps(ctx);
 {
 TCGv_i64 fp0 = tcg_temp_new_i64();
 TCGv_i64 fp1 = tcg_temp_new_i64();
@@ -10167,7 +10175,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "mul.ps";
 break;
 case OPC_ABS_PS:
-check_cp1_64bitmode(ctx);
+check_ps(ctx);
 {
 TCGv_i64 fp0 = tcg_temp_new_i64();
 
@@ -10179,7 +10187,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "abs.ps";
 break;
 case OPC_MOV_PS:
-check_cp1_64bitmode(ctx);
+check_ps(ctx);
 {
 TCGv_i64 fp0 = tcg_temp_new_i64();
 
@@ -10190,7 +10198,7 @@ static void gen_farith (DisasContext *ctx, enum fopcode 
op1,
 opn = "mov.ps";
 break;
 case OPC_NEG_PS:
-check_cp1_64bitmode(ctx);
+check_ps(ctx);
 {
 TCGv_i64 fp0 = tcg_temp_new_i64();
 
@@ -10202,12 +10210,12 @@ static void gen_farith (DisasContext *ctx, enum 
fopcode op1,
 opn = "neg.ps";
  

[Qemu-devel] [PATCH v4 07/15] target-mips: signal RI for removed instructions in microMIPS R6

[Yongbok Kim]

Signal a Reserved Instruction exception for removed instruction encoding
in microMIPS Release 6.

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |   68 +++
 1 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 851a27c..2ab00fc 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -13254,15 +13254,19 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 gen_bshfl(ctx, OPC_WSBH, rs, rt);
 break;
 case MULT:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MULT;
 goto do_mul;
 case MULTU:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MULTU;
 goto do_mul;
 case DIV:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_DIV;
 goto do_div;
 case DIVU:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_DIVU;
 goto do_div;
 do_div:
@@ -13270,15 +13274,19 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 gen_muldiv(ctx, mips32_op, 0, rs, rt);
 break;
 case MADD:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MADD;
 goto do_mul;
 case MADDU:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MADDU;
 goto do_mul;
 case MSUB:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MSUB;
 goto do_mul;
 case MSUBU:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MSUBU;
 do_mul:
 check_insn(ctx, ISA_MIPS32);
@@ -13311,6 +13319,7 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 break;
 case JALRS:
 case JALRS_HB:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 gen_compute_branch(ctx, OPC_JALR, 4, rs, rt, 0, 2);
 ctx->hflags |= MIPS_HFLAG_BDS_STRICT;
 break;
@@ -13443,6 +13452,7 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 }
 break;
 case 0x35:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 switch (minor) {
 case MFHI32:
 gen_HILO(ctx, OPC_MFHI, 0, rs);
@@ -13715,6 +13725,7 @@ static void gen_pool32fxf(DisasContext *ctx, int rt, 
int rs)
 case COND_FLOAT_MOV(MOVT, 5):
 case COND_FLOAT_MOV(MOVT, 6):
 case COND_FLOAT_MOV(MOVT, 7):
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 gen_movci(ctx, rt, rs, (ctx->opcode >> 13) & 0x7, 1);
 break;
 case COND_FLOAT_MOV(MOVF, 0):
@@ -13725,6 +13736,7 @@ static void gen_pool32fxf(DisasContext *ctx, int rt, 
int rs)
 case COND_FLOAT_MOV(MOVF, 5):
 case COND_FLOAT_MOV(MOVF, 6):
 case COND_FLOAT_MOV(MOVF, 7):
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 gen_movci(ctx, rt, rs, (ctx->opcode >> 13) & 0x7, 0);
 break;
 default:
@@ -13795,6 +13807,7 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 mips32_op = OPC_SUBU;
 goto do_arith;
 case MUL:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MUL;
 do_arith:
 gen_arith(ctx, mips32_op, rd, rs, rt);
@@ -13926,47 +13939,61 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 check_cp1_enabled(ctx);
 switch (minor) {
 case ALNV_PS:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_ALNV_PS;
 goto do_madd;
 case MADD_S:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MADD_S;
 goto do_madd;
 case MADD_D:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MADD_D;
 goto do_madd;
 case MADD_PS:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MADD_PS;
 goto do_madd;
 case MSUB_S:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MSUB_S;
 goto do_madd;
 case MSUB_D:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MSUB_D;
 goto do_madd;
 case MSUB_PS:
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_MSUB_PS;
 goto do_madd;
 case NMADD_S:

[Qemu-devel] [PATCH v4 14/15] target-mips: microMIPS32 R6 POOL16{A, C} instructions

[Yongbok Kim]

microMIPS32 Release 6 POOL16A/ POOL16C instructions

Signed-off-by: Yongbok Kim 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |  133 +-
 1 files changed, 118 insertions(+), 15 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 72a284b..1dacf33 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -13163,6 +13163,110 @@ static void gen_pool16c_insn(DisasContext *ctx)
 }
 }
 
+static inline void gen_movep(DisasContext *ctx, int enc_dest, int enc_rt,
+ int enc_rs)
+{
+int rd, rs, re, rt;
+static const int rd_enc[] = { 5, 5, 6, 4, 4, 4, 4, 4 };
+static const int re_enc[] = { 6, 7, 7, 21, 22, 5, 6, 7 };
+static const int rs_rt_enc[] = { 0, 17, 2, 3, 16, 18, 19, 20 };
+rd = rd_enc[enc_dest];
+re = re_enc[enc_dest];
+rs = rs_rt_enc[enc_rs];
+rt = rs_rt_enc[enc_rt];
+if (rs) {
+tcg_gen_mov_tl(cpu_gpr[rd], cpu_gpr[rs]);
+} else {
+tcg_gen_movi_tl(cpu_gpr[rd], 0);
+}
+if (rt) {
+tcg_gen_mov_tl(cpu_gpr[re], cpu_gpr[rt]);
+} else {
+tcg_gen_movi_tl(cpu_gpr[re], 0);
+}
+}
+
+static void gen_pool16c_r6_insn(DisasContext *ctx)
+{
+int rt = mmreg((ctx->opcode >> 7) & 0x7);
+int rs = mmreg((ctx->opcode >> 4) & 0x7);
+
+switch (ctx->opcode & 0xf) {
+case R6_NOT16:
+gen_logic(ctx, OPC_NOR, rt, rs, 0);
+break;
+case R6_AND16:
+gen_logic(ctx, OPC_AND, rt, rt, rs);
+break;
+case R6_LWM16:
+{
+int lwm_converted = 0x11 + extract32(ctx->opcode, 8, 2);
+int offset = extract32(ctx->opcode, 4, 4);
+gen_ldst_multiple(ctx, LWM32, lwm_converted, 29, offset << 2);
+}
+break;
+case R6_JRC16: /* JRCADDIUSP */
+if ((ctx->opcode >> 4) & 1) {
+/* JRCADDIUSP */
+int imm = extract32(ctx->opcode, 5, 5);
+gen_compute_branch(ctx, OPC_JR, 2, 31, 0, 0, 0);
+gen_arith_imm(ctx, OPC_ADDIU, 29, 29, imm << 2);
+} else {
+/* JRC16 */
+int rs = extract32(ctx->opcode, 5, 5);
+gen_compute_branch(ctx, OPC_JR, 2, rs, 0, 0, 0);
+}
+break;
+case MOVEP ... MOVEP_07:
+case MOVEP_0C ... MOVEP_0F:
+{
+int enc_dest = uMIPS_RD(ctx->opcode);
+int enc_rt = uMIPS_RS2(ctx->opcode);
+int enc_rs = (ctx->opcode & 3) | ((ctx->opcode >> 1) & 4);
+gen_movep(ctx, enc_dest, enc_rt, enc_rs);
+}
+break;
+case R6_XOR16:
+gen_logic(ctx, OPC_XOR, rt, rt, rs);
+break;
+case R6_OR16:
+gen_logic(ctx, OPC_OR, rt, rt, rs);
+break;
+case R6_SWM16:
+{
+int swm_converted = 0x11 + extract32(ctx->opcode, 8, 2);
+int offset = extract32(ctx->opcode, 4, 4);
+gen_ldst_multiple(ctx, SWM32, swm_converted, 29, offset << 2);
+}
+break;
+case JALRC16: /* BREAK16, SDBBP16 */
+switch (ctx->opcode & 0x3f) {
+case JALRC16:
+case JALRC16 + 0x20:
+/* JALRC16 */
+gen_compute_branch(ctx, OPC_JALR, 2, (ctx->opcode >> 5) & 0x1f,
+   31, 0, 0);
+break;
+case R6_BREAK16:
+/* BREAK16 */
+generate_exception(ctx, EXCP_BREAK);
+break;
+case R6_SDBBP16:
+/* SDBBP16 */
+if (ctx->hflags & MIPS_HFLAG_SBRI) {
+generate_exception(ctx, EXCP_RI);
+} else {
+generate_exception(ctx, EXCP_DBp);
+}
+break;
+}
+break;
+default:
+generate_exception(ctx, EXCP_RI);
+break;
+}
+}
+
 static void gen_ldxs (DisasContext *ctx, int base, int index, int rd)
 {
 TCGv t0 = tcg_temp_new();
@@ -15168,8 +15272,14 @@ static int decode_micromips_opc (CPUMIPSState *env, 
DisasContext *ctx)
 opc = OPC_SUBU;
 break;
 }
-
-gen_arith(ctx, opc, rd, rs1, rs2);
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* In the Release 6 the register number location in
+ * the instruction encoding has changed.
+ */
+gen_arith(ctx, opc, rs1, rd, rs2);
+} else {
+gen_arith(ctx, opc, rd, rs1, rs2);
+}
 }
 break;
 case POOL16B:
@@ -15193,7 +15303,11 @@ static int decode_micromips_opc (CPUMIPSState *env, 
DisasContext *ctx)
 }
 break;
 case POOL16C:
-gen_pool16c_insn(ctx);
+if (ctx->insn_flags & ISA_MIPS32R6) {
+gen_pool16c_r6_insn(ctx);
+} else {
+gen_pool16c_insn(ctx);
+}
 break;
 case LWGP16:
 {
@@ -15213,18 +15327,7 @@ static int decode_micromips_opc (CPUMIPSState *env, 
DisasContext *

[Qemu-devel] [PATCH v4 15/15] target-mips: add mips32r6-generic CPU definition

[Yongbok Kim]

Define a new CPU definition supporting MIPS32 Release 6 ISA and
microMIPS32 Release 6 ISA.

Signed-off-by: Yongbok Kim 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate_init.c |   37 +
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index 30605da..ddfaff8 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -424,6 +424,43 @@ static const mips_def_t mips_defs[] =
 .insn_flags = CPU_MIPS32R5 | ASE_MIPS16 | ASE_MSA,
 .mmu_type = MMU_TYPE_R4000,
 },
+{
+/* A generic CPU supporting MIPS32 Release 6 ISA.
+   FIXME: Support IEEE 754-2008 FP.
+  Eventually this should be replaced by a real CPU model. */
+.name = "mips32r6-generic",
+.CP0_PRid = 0x0001,
+.CP0_Config0 = MIPS_CONFIG0 | (0x2 << CP0C0_AR) |
+   (MMU_TYPE_R4000 << CP0C0_MT),
+.CP0_Config1 = MIPS_CONFIG1 | (1 << CP0C1_FP) | (31 << CP0C1_MMU) |
+   (2 << CP0C1_IS) | (4 << CP0C1_IL) | (3 << CP0C1_IA) |
+   (2 << CP0C1_DS) | (4 << CP0C1_DL) | (3 << CP0C1_DA) |
+   (0 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
+.CP0_Config2 = MIPS_CONFIG2,
+.CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_BP) | (1 << CP0C3_BI) |
+   (2 << CP0C3_ISA) | (1 << CP0C3_ULRI) |
+   (1 << CP0C3_RXI) | (1U << CP0C3_M),
+.CP0_Config4 = MIPS_CONFIG4 | (0xfc << CP0C4_KScrExist) |
+   (3 << CP0C4_IE) | (1U << CP0C4_M),
+.CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_LLB),
+.CP0_Config5_rw_bitmask = (1 << CP0C5_SBRI) | (1 << CP0C5_FRE) |
+  (1 << CP0C5_UFE),
+.CP0_LLAddr_rw_bitmask = 0,
+.CP0_LLAddr_shift = 0,
+.SYNCI_Step = 32,
+.CCRes = 2,
+.CP0_Status_rw_bitmask = 0x3058FF1F,
+.CP0_PageGrain = (1 << CP0PG_IEC) | (1 << CP0PG_XIE) |
+ (1U << CP0PG_RIE),
+.CP0_PageGrain_rw_bitmask = 0,
+.CP1_fcr0 = (1 << FCR0_FREP) | (1 << FCR0_F64) | (1 << FCR0_L) |
+(1 << FCR0_W) | (1 << FCR0_D) | (1 << FCR0_S) |
+(0x00 << FCR0_PRID) | (0x0 << FCR0_REV),
+.SEGBITS = 32,
+.PABITS = 32,
+.insn_flags = CPU_MIPS32R6 | ASE_MICROMIPS,
+.mmu_type = MMU_TYPE_R4000,
+},
 #if defined(TARGET_MIPS64)
 {
 .name = "R4000",
-- 
1.7.5.4

[Qemu-devel] [PATCH v4 11/15] target-mips: microMIPS32 R6 POOL32F instructions

[Yongbok Kim]

Add new microMIPS32 Release 6 POOL32F instructions

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |  231 ---
 1 files changed, 199 insertions(+), 32 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index d091e6a..4c8cf9f 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -14195,6 +14195,14 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 goto pool32f_invalid;
 }
 break;
+case CMP_CONDN_S:
+check_insn(ctx, ISA_MIPS32R6);
+gen_r6_cmp_s(ctx, (ctx->opcode >> 6) & 0x1f, rt, rs, rd);
+break;
+case CMP_CONDN_D:
+check_insn(ctx, ISA_MIPS32R6);
+gen_r6_cmp_d(ctx, (ctx->opcode >> 6) & 0x1f, rt, rs, rd);
+break;
 case POOL32FXF:
 gen_pool32fxf(ctx, rt, rs);
 break;
@@ -14223,6 +14231,19 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 goto pool32f_invalid;
 }
 break;
+case MIN_FMT:
+check_insn(ctx, ISA_MIPS32R6);
+switch ((ctx->opcode >> 9) & 0x3) {
+case FMT_SDPS_S:
+gen_farith(ctx, OPC_MIN_S, rt, rs, rd, 0);
+break;
+case FMT_SDPS_D:
+gen_farith(ctx, OPC_MIN_D, rt, rs, rd, 0);
+break;
+default:
+goto pool32f_invalid;
+}
+break;
 case 0x08:
 /* [LS][WDU]XC1 */
 switch ((ctx->opcode >> 6) & 0x7) {
@@ -14256,6 +14277,19 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 goto pool32f_invalid;
 }
 break;
+case MAX_FMT:
+check_insn(ctx, ISA_MIPS32R6);
+switch ((ctx->opcode >> 9) & 0x3) {
+case FMT_SDPS_S:
+gen_farith(ctx, OPC_MAX_S, rt, rs, rd, 0);
+break;
+case FMT_SDPS_D:
+gen_farith(ctx, OPC_MAX_D, rt, rs, rd, 0);
+break;
+default:
+goto pool32f_invalid;
+}
+break;
 case 0x18:
 /* 3D insns */
 check_insn_opc_removed(ctx, ISA_MIPS32R6);
@@ -14304,40 +14338,70 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 }
 break;
 case 0x20:
-/* MOV[FT].fmt and PREFX */
+/* MOV[FT].fmt, PREFX, RINT.fmt, CLASS.fmt*/
 cc = (ctx->opcode >> 13) & 0x7;
 fmt = (ctx->opcode >> 9) & 0x3;
 switch ((ctx->opcode >> 6) & 0x7) {
-case MOVF_FMT:
-switch (fmt) {
-case FMT_SDPS_S:
-gen_movcf_s(ctx, rs, rt, cc, 0);
-break;
-case FMT_SDPS_D:
-gen_movcf_d(ctx, rs, rt, cc, 0);
-break;
-case FMT_SDPS_PS:
-check_ps(ctx);
-gen_movcf_ps(ctx, rs, rt, cc, 0);
-break;
-default:
-goto pool32f_invalid;
+case MOVF_FMT: /* RINT_FMT */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* RINT_FMT */
+switch (fmt) {
+case FMT_SDPS_S:
+gen_farith(ctx, OPC_RINT_S, 0, rt, rs, 0);
+break;
+case FMT_SDPS_D:
+gen_farith(ctx, OPC_RINT_D, 0, rt, rs, 0);
+break;
+default:
+goto pool32f_invalid;
+}
+} else {
+/* MOVF_FMT */
+switch (fmt) {
+case FMT_SDPS_S:
+gen_movcf_s(ctx, rs, rt, cc, 0);
+break;
+case FMT_SDPS_D:
+gen_movcf_d(ctx, rs, rt, cc, 0);
+break;
+case FMT_SDPS_PS:
+check_ps(ctx);
+gen_movcf_ps(ctx, rs, rt, cc, 0);
+break;
+default:
+goto pool32f_invalid;
+}
 }
 break;
-ca

[Qemu-devel] [PATCH v4 09/15] target-mips: microMIPS32 R6 branches and jumps

[Yongbok Kim]

Add new microMIPS32 Release 6 branch and jump instructions.

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |  242 +++
 1 files changed, 202 insertions(+), 40 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 0aaf5e0..0f5b407 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -8441,7 +8441,8 @@ static void gen_compute_branch1(DisasContext *ctx, 
uint32_t op,
 
 /* R6 CP1 Branches */
 static void gen_compute_branch1_r6(DisasContext *ctx, uint32_t op,
-   int32_t ft, int32_t offset)
+   int32_t ft, int32_t offset,
+   int delayslot_size)
 {
 target_ulong btarget;
 const char *opn = "cp1 cond branch";
@@ -8484,7 +8485,15 @@ static void gen_compute_branch1_r6(DisasContext *ctx, 
uint32_t op,
 MIPS_DEBUG("%s: cond %02x target " TARGET_FMT_lx, opn,
ctx->hflags, btarget);
 ctx->btarget = btarget;
-ctx->hflags |= MIPS_HFLAG_BDS32;
+
+switch (delayslot_size) {
+case 2:
+ctx->hflags |= MIPS_HFLAG_BDS16;
+break;
+case 4:
+ctx->hflags |= MIPS_HFLAG_BDS32;
+break;
+}
 
 out:
 tcg_temp_free_i64(t0);
@@ -10985,6 +10994,7 @@ static void gen_compute_compact_branch(DisasContext 
*ctx, uint32_t opc,
 int bcond_compute = 0;
 TCGv t0 = tcg_temp_new();
 TCGv t1 = tcg_temp_new();
+int m16_lowbit = (ctx->hflags & MIPS_HFLAG_M16) != 0;
 
 if (ctx->hflags & MIPS_HFLAG_BMASK) {
 #ifdef MIPS_DEBUG_DISAS
@@ -11006,7 +11016,7 @@ static void gen_compute_compact_branch(DisasContext 
*ctx, uint32_t opc,
 ctx->btarget = addr_add(ctx, ctx->pc + 4, offset);
 if (rs <= rt && rs == 0) {
 /* OPC_BEQZALC, OPC_BNEZALC */
-tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4);
+tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4 + m16_lowbit);
 }
 break;
 case OPC_BLEZC: /* OPC_BGEZC, OPC_BGEC */
@@ -11021,7 +11031,7 @@ static void gen_compute_compact_branch(DisasContext 
*ctx, uint32_t opc,
 if (rs == 0 || rs == rt) {
 /* OPC_BLEZALC, OPC_BGEZALC */
 /* OPC_BGTZALC, OPC_BLTZALC */
-tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4);
+tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4 + m16_lowbit);
 }
 gen_load_gpr(t0, rs);
 gen_load_gpr(t1, rt);
@@ -11061,13 +11071,13 @@ static void gen_compute_compact_branch(DisasContext 
*ctx, uint32_t opc,
 /* Uncoditional compact branch */
 switch (opc) {
 case OPC_JIALC:
-tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4);
+tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4 + m16_lowbit);
 /* Fallthrough */
 case OPC_JIC:
 ctx->hflags |= MIPS_HFLAG_BR;
 break;
 case OPC_BALC:
-tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4);
+tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4 + m16_lowbit);
 /* Fallthrough */
 case OPC_BC:
 ctx->hflags |= MIPS_HFLAG_B;
@@ -13403,10 +13413,16 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 break;
 case 0x3c:
 switch (minor) {
-case JALR:
-case JALR_HB:
-gen_compute_branch(ctx, OPC_JALR, 4, rs, rt, 0, 4);
-ctx->hflags |= MIPS_HFLAG_BDS_STRICT;
+case JALR:/* JALRC */
+case JALR_HB: /* JALRC_HB */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* JALRC, JALRC_HB */
+gen_compute_branch(ctx, OPC_JALR, 4, rs, rt, 0, 0);
+} else {
+/* JALR, JALR_HB */
+gen_compute_branch(ctx, OPC_JALR, 4, rs, rt, 0, 4);
+ctx->hflags |= MIPS_HFLAG_BDS_STRICT;
+}
 break;
 case JALRS:
 case JALRS_HB:
@@ -14381,12 +14397,28 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 break;
 
 /* Traps */
-case TLTI:
-mips32_op = OPC_TLTI;
-goto do_trapi;
-case TGEI:
-mips32_op = OPC_TGEI;
-goto do_trapi;
+case TLTI: /* BC1EQZC */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* BC1EQZC */
+check_cp1_enabled(ctx);
+gen_compute_branch1_r6(ctx, OPC_BC1EQZ, rs, imm << 1, 0);
+} else {
+/* TLTI */
+mips32_op = OPC_TLTI;
+goto do_trapi;
+}
+break;
+case TGEI: /* BC1NEZC */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* BC1NEZC */
+check_cp1_enabled(ctx);
+gen_compute_branch1_r6(ctx, OPC_BC1NEZ, rs, imm << 1, 0);
+} else {
+/* TGEI */
+  

[Qemu-devel] [PATCH v4 12/15] target-mips: microMIPS32 R6 POOL32{I, C} instructions

[Yongbok Kim]

Add new microMIPS32 Release 6 POOL32I/POOL32C type instructions

Signed-off-by: Yongbok Kim 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |   27 +--
 1 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 4c8cf9f..2eacbb1 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -14652,9 +14652,18 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_TGEIU;
 goto do_trapi;
-case TNEI:
-mips32_op = OPC_TNEI;
-goto do_trapi;
+case TNEI: /* SYNCI */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* SYNCI */
+/* Break the TB to be able to sync copied instructions
+   immediately */
+ctx->bstate = BS_STOP;
+} else {
+/* TNEI */
+mips32_op = OPC_TNEI;
+goto do_trapi;
+}
+break;
 case TEQI:
 check_insn_opc_removed(ctx, ISA_MIPS32R6);
 mips32_op = OPC_TEQI;
@@ -14727,6 +14736,8 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 break;
 case POOL32C:
 minor = (ctx->opcode >> 12) & 0xf;
+offset = sextract32(ctx->opcode, 0,
+(ctx->insn_flags & ISA_MIPS32R6) ? 9 : 12);
 switch (minor) {
 case LWL:
 check_insn_opc_removed(ctx, ISA_MIPS32R6);
@@ -14784,23 +14795,27 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 mips32_op = OPC_LL;
 goto do_ld_lr;
 do_ld_lr:
-gen_ld(ctx, mips32_op, rt, rs, SIMM(ctx->opcode, 0, 12));
+gen_ld(ctx, mips32_op, rt, rs, offset);
 break;
 do_st_lr:
 gen_st(ctx, mips32_op, rt, rs, SIMM(ctx->opcode, 0, 12));
 break;
 case SC:
-gen_st_cond(ctx, OPC_SC, rt, rs, SIMM(ctx->opcode, 0, 12));
+gen_st_cond(ctx, OPC_SC, rt, rs, offset);
 break;
 #if defined(TARGET_MIPS64)
 case SCD:
 check_insn(ctx, ISA_MIPS3);
 check_mips_64(ctx);
-gen_st_cond(ctx, OPC_SCD, rt, rs, SIMM(ctx->opcode, 0, 12));
+gen_st_cond(ctx, OPC_SCD, rt, rs, offset);
 break;
 #endif
 case PREF:
 /* Treat as no-op */
+if ((ctx->insn_flags & ISA_MIPS32R6) && (rt >= 24)) {
+/* hint codes 24-31 are reserved and signal RI */
+generate_exception(ctx, EXCP_RI);
+}
 break;
 default:
 MIPS_INVAL("pool32c");
-- 
1.7.5.4

[Qemu-devel] [PATCH v4 05/15] target-mips: rearrange gen_compute_compact_branch

[Yongbok Kim]

The function will be also used for microMIPS Release 6.

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |  472 +++---
 1 files changed, 236 insertions(+), 236 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index e294bb6..dc9aae6 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -10970,6 +10970,242 @@ static void gen_branch(DisasContext *ctx, int 
insn_bytes)
 }
 }
 
+/* Compact Branches */
+static void gen_compute_compact_branch(DisasContext *ctx, uint32_t opc,
+   int rs, int rt, int32_t offset)
+{
+int bcond_compute = 0;
+TCGv t0 = tcg_temp_new();
+TCGv t1 = tcg_temp_new();
+
+if (ctx->hflags & MIPS_HFLAG_BMASK) {
+#ifdef MIPS_DEBUG_DISAS
+LOG_DISAS("Branch in delay / forbidden slot at PC 0x" TARGET_FMT_lx
+  "\n", ctx->pc);
+#endif
+generate_exception(ctx, EXCP_RI);
+goto out;
+}
+
+/* Load needed operands and calculate btarget */
+switch (opc) {
+/* compact branch */
+case OPC_BOVC: /* OPC_BEQZALC, OPC_BEQC */
+case OPC_BNVC: /* OPC_BNEZALC, OPC_BNEC */
+gen_load_gpr(t0, rs);
+gen_load_gpr(t1, rt);
+bcond_compute = 1;
+ctx->btarget = addr_add(ctx, ctx->pc + 4, offset);
+if (rs <= rt && rs == 0) {
+/* OPC_BEQZALC, OPC_BNEZALC */
+tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4);
+}
+break;
+case OPC_BLEZC: /* OPC_BGEZC, OPC_BGEC */
+case OPC_BGTZC: /* OPC_BLTZC, OPC_BLTC */
+gen_load_gpr(t0, rs);
+gen_load_gpr(t1, rt);
+bcond_compute = 1;
+ctx->btarget = addr_add(ctx, ctx->pc + 4, offset);
+break;
+case OPC_BLEZALC: /* OPC_BGEZALC, OPC_BGEUC */
+case OPC_BGTZALC: /* OPC_BLTZALC, OPC_BLTUC */
+if (rs == 0 || rs == rt) {
+/* OPC_BLEZALC, OPC_BGEZALC */
+/* OPC_BGTZALC, OPC_BLTZALC */
+tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4);
+}
+gen_load_gpr(t0, rs);
+gen_load_gpr(t1, rt);
+bcond_compute = 1;
+ctx->btarget = addr_add(ctx, ctx->pc + 4, offset);
+break;
+case OPC_BC:
+case OPC_BALC:
+ctx->btarget = addr_add(ctx, ctx->pc + 4, offset);
+break;
+case OPC_BEQZC:
+case OPC_BNEZC:
+if (rs != 0) {
+/* OPC_BEQZC, OPC_BNEZC */
+gen_load_gpr(t0, rs);
+bcond_compute = 1;
+ctx->btarget = addr_add(ctx, ctx->pc + 4, offset);
+} else {
+/* OPC_JIC, OPC_JIALC */
+TCGv tbase = tcg_temp_new();
+TCGv toffset = tcg_temp_new();
+
+gen_load_gpr(tbase, rt);
+tcg_gen_movi_tl(toffset, offset);
+gen_op_addr_add(ctx, btarget, tbase, toffset);
+tcg_temp_free(tbase);
+tcg_temp_free(toffset);
+}
+break;
+default:
+MIPS_INVAL("Compact branch/jump");
+generate_exception(ctx, EXCP_RI);
+goto out;
+}
+
+if (bcond_compute == 0) {
+/* Uncoditional compact branch */
+switch (opc) {
+case OPC_JIALC:
+tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4);
+/* Fallthrough */
+case OPC_JIC:
+ctx->hflags |= MIPS_HFLAG_BR;
+break;
+case OPC_BALC:
+tcg_gen_movi_tl(cpu_gpr[31], ctx->pc + 4);
+/* Fallthrough */
+case OPC_BC:
+ctx->hflags |= MIPS_HFLAG_B;
+break;
+default:
+MIPS_INVAL("Compact branch/jump");
+generate_exception(ctx, EXCP_RI);
+goto out;
+}
+
+/* Generating branch here as compact branches don't have delay slot */
+gen_branch(ctx, 4);
+} else {
+/* Conditional compact branch */
+TCGLabel *fs = gen_new_label();
+save_cpu_state(ctx, 0);
+
+switch (opc) {
+case OPC_BLEZALC: /* OPC_BGEZALC, OPC_BGEUC */
+if (rs == 0 && rt != 0) {
+/* OPC_BLEZALC */
+tcg_gen_brcondi_tl(tcg_invert_cond(TCG_COND_LE), t1, 0, fs);
+} else if (rs != 0 && rt != 0 && rs == rt) {
+/* OPC_BGEZALC */
+tcg_gen_brcondi_tl(tcg_invert_cond(TCG_COND_GE), t1, 0, fs);
+} else {
+/* OPC_BGEUC */
+tcg_gen_brcond_tl(tcg_invert_cond(TCG_COND_GEU), t0, t1, fs);
+}
+break;
+case OPC_BGTZALC: /* OPC_BLTZALC, OPC_BLTUC */
+if (rs == 0 && rt != 0) {
+/* OPC_BGTZALC */
+tcg_gen_brcondi_tl(tcg_invert_cond(TCG_COND_GT), t1, 0, fs);
+} else if (rs != 0 && rt != 0 && rs == rt) {
+/* OPC_BLTZALC */
+tcg_gen_brcondi_tl(tcg_invert_cond(TCG_COND_LT), t1, 0, fs);
+} else

[Qemu-devel] [PATCH v4 10/15] target-mips: microMIPS32 R6 POOL32A{XF} instructions

[Yongbok Kim]

Add new microMIPS32 Release 6 pool32a/pool32axf instructions.

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |   80 --
 1 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 0f5b407..d091e6a 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -1,6 +1,10 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 break;
 case 0x2c:
 switch (minor) {
+case BITSWAP:
+check_insn(ctx, ISA_MIPS32R6);
+gen_bitswap(ctx, OPC_BITSWAP, rs, rt);
+break;
 case SEB:
 gen_bshfl(ctx, OPC_SEB, rs, rt);
 break;
@@ -13530,8 +13534,8 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 break;
 case SDBBP:
 check_insn(ctx, ISA_MIPS32);
-if (!(ctx->hflags & MIPS_HFLAG_DM)) {
-generate_exception(ctx, EXCP_DBp);
+if (ctx->hflags & MIPS_HFLAG_SBRI) {
+generate_exception(ctx, EXCP_RI);
 } else {
 generate_exception(ctx, EXCP_DBp);
 }
@@ -13893,6 +13897,14 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 do_shifti:
 gen_shift_imm(ctx, mips32_op, rt, rs, rd);
 break;
+case SELEQZ:
+check_insn(ctx, ISA_MIPS32R6);
+gen_cond_move(ctx, OPC_SELEQZ, rd, rs, rt);
+break;
+case SELNEZ:
+check_insn(ctx, ISA_MIPS32R6);
+gen_cond_move(ctx, OPC_SELNEZ, rd, rs, rt);
+break;
 default:
 goto pool32a_invalid;
 }
@@ -13966,16 +13978,52 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 minor = (ctx->opcode >> 6) & 0xf;
 switch (minor) {
 /* Conditional moves */
-case MOVN:
-mips32_op = OPC_MOVN;
-goto do_cmov;
-case MOVZ:
-mips32_op = OPC_MOVZ;
-do_cmov:
-gen_cond_move(ctx, mips32_op, rd, rs, rt);
+case MOVN: /* MUL */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* MUL */
+gen_r6_muldiv(ctx, R6_OPC_MUL, rd, rs, rt);
+} else {
+/* MOVN */
+gen_cond_move(ctx, OPC_MOVN, rd, rs, rt);
+}
+break;
+case MOVZ: /* MUH */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* MUH */
+gen_r6_muldiv(ctx, R6_OPC_MUH, rd, rs, rt);
+} else {
+/* MOVZ */
+gen_cond_move(ctx, OPC_MOVZ, rd, rs, rt);
+}
 break;
-case LWXS:
-gen_ldxs(ctx, rs, rt, rd);
+case MULU:
+check_insn(ctx, ISA_MIPS32R6);
+gen_r6_muldiv(ctx, R6_OPC_MULU, rd, rs, rt);
+break;
+case MUHU:
+check_insn(ctx, ISA_MIPS32R6);
+gen_r6_muldiv(ctx, R6_OPC_MUHU, rd, rs, rt);
+break;
+case LWXS: /* DIV */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* DIV */
+gen_r6_muldiv(ctx, R6_OPC_DIV, rd, rs, rt);
+} else {
+/* LWXS */
+gen_ldxs(ctx, rs, rt, rd);
+}
+break;
+case MOD:
+check_insn(ctx, ISA_MIPS32R6);
+gen_r6_muldiv(ctx, R6_OPC_MOD, rd, rs, rt);
+break;
+case R6_DIVU:
+check_insn(ctx, ISA_MIPS32R6);
+gen_r6_muldiv(ctx, R6_OPC_DIVU, rd, rs, rt);
+break;
+case MODU:
+check_insn(ctx, ISA_MIPS32R6);
+gen_r6_muldiv(ctx, R6_OPC_MODU, rd, rs, rt);
 break;
 default:
 goto pool32a_invalid;
@@ -13984,6 +14032,16 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 case INS:
 gen_bitops(ctx, OPC_INS, rt, rs, rr, rd);
 return;
+case LSA:
+check_insn(ctx, ISA_MIPS32R6);
+gen_lsa(ctx, OPC_LSA, rd, rs, rt,
+extract32(ctx->opcode, 9, 2));
+break;
+case ALIGN:
+check_insn(ctx, ISA_MIPS32R6);
+gen_align(ctx, OPC_ALIGN, rd, rs, rt,
+  extract32(ctx->opcode, 9, 2));
+break;
 case EXT:
 gen_bitops(ctx, OPC_EXT, rt, rs, rr, rd);
 return;
-- 
1.7.5.4

[Qemu-devel] [PATCH v4 13/15] target-mips: microMIPS32 R6 Major instructions

[Yongbok Kim]

Add new microMIPS32 Release 6 Major opcode instructions

Signed-off-by: Yongbok Kim 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |   62 ++-
 1 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 2eacbb1..72a284b 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -3208,45 +3208,46 @@ static inline void gen_r6_ld(target_long addr, int reg, 
int memidx,
 tcg_temp_free(t0);
 }
 
-static inline void gen_pcrel(DisasContext *ctx, int rs, int16_t imm)
+static inline void gen_pcrel(DisasContext *ctx, int opc, target_ulong pc,
+ int rs)
 {
 target_long offset;
 target_long addr;
 
-switch (MASK_OPC_PCREL_TOP2BITS(ctx->opcode)) {
+switch (MASK_OPC_PCREL_TOP2BITS(opc)) {
 case OPC_ADDIUPC:
 if (rs != 0) {
 offset = sextract32(ctx->opcode << 2, 0, 21);
-addr = addr_add(ctx, ctx->pc, offset);
+addr = addr_add(ctx, pc, offset);
 tcg_gen_movi_tl(cpu_gpr[rs], addr);
 }
 break;
 case R6_OPC_LWPC:
 offset = sextract32(ctx->opcode << 2, 0, 21);
-addr = addr_add(ctx, ctx->pc, offset);
+addr = addr_add(ctx, pc, offset);
 gen_r6_ld(addr, rs, ctx->mem_idx, MO_TESL);
 break;
 #if defined(TARGET_MIPS64)
 case OPC_LWUPC:
 check_mips_64(ctx);
 offset = sextract32(ctx->opcode << 2, 0, 21);
-addr = addr_add(ctx, ctx->pc, offset);
+addr = addr_add(ctx, pc, offset);
 gen_r6_ld(addr, rs, ctx->mem_idx, MO_TEUL);
 break;
 #endif
 default:
-switch (MASK_OPC_PCREL_TOP5BITS(ctx->opcode)) {
+switch (MASK_OPC_PCREL_TOP5BITS(opc)) {
 case OPC_AUIPC:
 if (rs != 0) {
-offset = imm << 16;
-addr = addr_add(ctx, ctx->pc, offset);
+offset = sextract32(ctx->opcode, 0, 16) << 16;
+addr = addr_add(ctx, pc, offset);
 tcg_gen_movi_tl(cpu_gpr[rs], addr);
 }
 break;
 case OPC_ALUIPC:
 if (rs != 0) {
-offset = imm << 16;
-addr = ~0x & addr_add(ctx, ctx->pc, offset);
+offset = sextract32(ctx->opcode, 0, 16) << 16;
+addr = ~0x & addr_add(ctx, pc, offset);
 tcg_gen_movi_tl(cpu_gpr[rs], addr);
 }
 break;
@@ -3257,7 +3258,7 @@ static inline void gen_pcrel(DisasContext *ctx, int rs, 
int16_t imm)
 case R6_OPC_LDPC + (3 << 16):
 check_mips_64(ctx);
 offset = sextract32(ctx->opcode << 3, 0, 21);
-addr = addr_add(ctx, (ctx->pc & ~0x7), offset);
+addr = addr_add(ctx, (pc & ~0x7), offset);
 gen_r6_ld(addr, rs, ctx->mem_idx, MO_TEQ);
 break;
 #endif
@@ -14823,9 +14824,16 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 break;
 }
 break;
-case ADDI32:
-mips32_op = OPC_ADDI;
-goto do_addi;
+case ADDI32: /* AUI, LUI */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* AUI, LUI */
+gen_logic_imm(ctx, OPC_LUI, rt, rs, imm);
+} else {
+/* ADDI32 */
+mips32_op = OPC_ADDI;
+goto do_addi;
+}
+break;
 case ADDIU32:
 mips32_op = OPC_ADDIU;
 do_addi:
@@ -14954,8 +14962,28 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 do_cop1:
 gen_cop1_ldst(ctx, mips32_op, rt, rs, imm);
 break;
-case ADDIUPC:
-{
+case ADDIUPC: /* PCREL: ADDIUPC, AUIPC, ALUIPC, LWPC */
+if (ctx->insn_flags & ISA_MIPS32R6) {
+/* PCREL: ADDIUPC, AUIPC, ALUIPC, LWPC */
+switch ((ctx->opcode >> 16) & 0x1f) {
+case ADDIUPC_00 ... ADDIUPC_07:
+gen_pcrel(ctx, OPC_ADDIUPC, ctx->pc & ~0x3, rt);
+break;
+case AUIPC:
+gen_pcrel(ctx, OPC_AUIPC, ctx->pc, rt);
+break;
+case ALUIPC:
+gen_pcrel(ctx, OPC_ALUIPC, ctx->pc, rt);
+break;
+case LWPC_08 ... LWPC_0F:
+gen_pcrel(ctx, R6_OPC_LWPC, ctx->pc & ~0x3, rt);
+break;
+default:
+generate_exception(ctx, EXCP_RI);
+break;
+}
+} else {
+/* ADDIUPC */
 int reg = mmreg(ZIMM(ctx->opcode, 23, 3));
 int offset = SIMM(ctx->opcode, 0, 23) << 2;
 
@@ -19972,7 +2,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext 
*ctx)
 break;
 case OPC_PCREL:
 check_insn(ctx, ISA_MIPS32R6);
-gen_pcrel(ctx, rs, imm);
+gen_pcrel(ctx, ctx->opcode, ctx->pc, rs);
 break;
 default:/* Invalid *

[Qemu-devel] [PATCH v4 04/15] target-mips: refactor {D}LSA, {D}ALIGN, {D}BITSWAP

[Yongbok Kim]

Refactor those instructions in order to reuse them for microMIPS32
Release 6.
Rearrange gen_move_low32().

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |  166 ---
 1 files changed, 99 insertions(+), 67 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 83dfb2f..e294bb6 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -1723,6 +1723,15 @@ static target_long addr_add(DisasContext *ctx, 
target_long base,
 return sum;
 }
 
+static inline void gen_move_low32(TCGv ret, TCGv_i64 arg)
+{
+#if defined(TARGET_MIPS64)
+tcg_gen_ext32s_tl(ret, arg);
+#else
+tcg_gen_trunc_i64_tl(ret, arg);
+#endif
+}
+
 static inline void check_cp0_enabled(DisasContext *ctx)
 {
 if (unlikely(!(ctx->hflags & MIPS_HFLAG_CP0)))
@@ -4845,17 +4854,94 @@ static void gen_bshfl (DisasContext *ctx, uint32_t op2, 
int rt, int rd)
 tcg_temp_free(t0);
 }
 
-#ifndef CONFIG_USER_ONLY
-/* CP0 (MMU and control) */
-static inline void gen_move_low32(TCGv ret, TCGv_i64 arg)
+static void gen_lsa(DisasContext *ctx, int opc, int rd, int rs, int rt,
+int imm2)
+{
+TCGv t0;
+TCGv t1;
+if (rd == 0) {
+/* Treat as NOP. */
+return;
+}
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+gen_load_gpr(t0, rs);
+gen_load_gpr(t1, rt);
+tcg_gen_shli_tl(t0, t0, imm2 + 1);
+tcg_gen_add_tl(cpu_gpr[rd], t0, t1);
+if (opc == OPC_LSA) {
+tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]);
+}
+
+tcg_temp_free(t1);
+tcg_temp_free(t0);
+
+return;
+}
+
+static void gen_align(DisasContext *ctx, int opc, int rd, int rs, int rt,
+  int bp)
 {
+TCGv t0;
+if (rd == 0) {
+/* Treat as NOP. */
+return;
+}
+t0 = tcg_temp_new();
+gen_load_gpr(t0, rt);
+if (bp == 0) {
+tcg_gen_mov_tl(cpu_gpr[rd], t0);
+} else {
+TCGv t1 = tcg_temp_new();
+gen_load_gpr(t1, rs);
+switch (opc) {
+case OPC_ALIGN:
+{
+TCGv_i64 t2 = tcg_temp_new_i64();
+tcg_gen_concat_tl_i64(t2, t1, t0);
+tcg_gen_shri_i64(t2, t2, 8 * (4 - bp));
+gen_move_low32(cpu_gpr[rd], t2);
+tcg_temp_free_i64(t2);
+}
+break;
 #if defined(TARGET_MIPS64)
-tcg_gen_ext32s_tl(ret, arg);
-#else
-tcg_gen_trunc_i64_tl(ret, arg);
+case OPC_DALIGN:
+tcg_gen_shli_tl(t0, t0, 8 * bp);
+tcg_gen_shri_tl(t1, t1, 8 * (8 - bp));
+tcg_gen_or_tl(cpu_gpr[rd], t1, t0);
+break;
 #endif
+}
+tcg_temp_free(t1);
+}
+
+tcg_temp_free(t0);
+}
+
+static void gen_bitswap(DisasContext *ctx, int opc, int rd, int rt)
+{
+TCGv t0;
+if (rd == 0) {
+/* Treat as NOP. */
+return;
+}
+t0 = tcg_temp_new();
+gen_load_gpr(t0, rt);
+switch (opc) {
+case OPC_BITSWAP:
+gen_helper_bitswap(cpu_gpr[rd], t0);
+break;
+#if defined(TARGET_MIPS64)
+case OPC_DBITSWAP:
+gen_helper_dbitswap(cpu_gpr[rd], t0);
+break;
+#endif
+}
+tcg_temp_free(t0);
 }
 
+#ifndef CONFIG_USER_ONLY
+/* CP0 (MMU and control) */
 static inline void gen_mthc0_entrylo(TCGv arg, target_ulong off)
 {
 TCGv_i64 t0 = tcg_temp_new_i64();
@@ -16432,18 +16518,7 @@ static void decode_opc_special_r6(CPUMIPSState *env, 
DisasContext *ctx)
 op1 = MASK_SPECIAL(ctx->opcode);
 switch (op1) {
 case OPC_LSA:
-if (rd != 0) {
-int imm2 = extract32(ctx->opcode, 6, 3);
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-gen_load_gpr(t0, rs);
-gen_load_gpr(t1, rt);
-tcg_gen_shli_tl(t0, t0, imm2 + 1);
-tcg_gen_add_tl(t0, t0, t1);
-tcg_gen_ext32s_tl(cpu_gpr[rd], t0);
-tcg_temp_free(t1);
-tcg_temp_free(t0);
-}
+gen_lsa(ctx, op1, rd, rs, rt, extract32(ctx->opcode, 6, 2));
 break;
 case OPC_MULT ... OPC_DIVU:
 op2 = MASK_R6_MULDIV(ctx->opcode);
@@ -16488,17 +16563,7 @@ static void decode_opc_special_r6(CPUMIPSState *env, 
DisasContext *ctx)
 #if defined(TARGET_MIPS64)
 case OPC_DLSA:
 check_mips_64(ctx);
-if (rd != 0) {
-int imm2 = extract32(ctx->opcode, 6, 3);
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-gen_load_gpr(t0, rs);
-gen_load_gpr(t1, rt);
-tcg_gen_shli_tl(t0, t0, imm2 + 1);
-tcg_gen_add_tl(cpu_gpr[rd], t0, t1);
-tcg_temp_free(t1);
-tcg_temp_free(t0);
-}
+gen_lsa(ctx, op1, rd, rs, rt, extract32(ctx->opcode, 6, 2));
 break;
 case R6_OPC_DCLO:
 case R6_OPC_DCLZ:
@@ -16923,35 +16988,15 @@ static void decode_opc_special3_r6(CPUMIPSState *e

[Qemu-devel] [PATCH v4 00/15] target-mips: add microMIPS32 R6 Instruction Set support

[Yongbok Kim]

The patchset implements the latest microMIPS32 Release 6 Instruction Set.
However LLX, LLXE, SCX and SCXE aren't included in the patchset.

For more information, microMIPS R6 Instruction Set document is available:
MIPS Architecture for Programmers Volume II-B: microMIPS32 Instruction Set
Revision 6.01
http://www.imgtec.com/mips/architectures/mips32.asp

---
v4:
* Allowed Loongson cores to use the PS data format (Aurelien)
* Refactored movep instruction (Aurelien)

v3:
* Avoided code duplication with pre-R6 (Leon)
* Cosmetic changes

v2:
* Updated for review comment (Leon, Aurelien)
* Added signal RI exception when FIR.PS = 0 (Leon) 
* Removed an unused argument from decode_micromips32_opc()
* Reused gen_pcrel() for pc relative instructions (Leon)

Yongbok Kim (15):
  target-mips: fix {RD,WR}PGPR in microMIPS
  target-mips: add microMIPS TLBINV, TLBINVF
  target-mips: remove an unused argument
  target-mips: refactor {D}LSA, {D}ALIGN, {D}BITSWAP
  target-mips: rearrange gen_compute_compact_branch
  target-mips: raise RI exceptions when FIR.PS = 0
  target-mips: signal RI for removed instructions in microMIPS R6
  target-mips: add microMIPS32 R6 opcode enum
  target-mips: microMIPS32 R6 branches and jumps
  target-mips: microMIPS32 R6 POOL32A{XF} instructions
  target-mips: microMIPS32 R6 POOL32F instructions
  target-mips: microMIPS32 R6 POOL32{I,C} instructions
  target-mips: microMIPS32 R6 Major instructions
  target-mips: microMIPS32 R6 POOL16{A,C} instructions
  target-mips: add mips32r6-generic CPU definition

 target-mips/translate.c  | 2111 --
 target-mips/translate_init.c |   37 +
 2 files changed, 1462 insertions(+), 686 deletions(-)

-- 
1.7.5.4

[Qemu-devel] [PATCH v4 02/15] target-mips: add microMIPS TLBINV, TLBINVF

[Yongbok Kim]

Add microMIPS TLBINV, TLBINVF

Signed-off-by: Yongbok Kim 
Reviewed-by: Aurelien Jarno 
Reviewed-by: Leon Alrae 
---
 target-mips/translate.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 97b74ba..963ff8b 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -12233,6 +12233,8 @@ enum {
 TLBR = 0x1,
 TLBWI = 0x2,
 TLBWR = 0x3,
+TLBINV = 0x4,
+TLBINVF = 0x5,
 WAIT = 0x9,
 IRET = 0xd,
 DERET = 0xe,
@@ -13017,6 +13019,12 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 case TLBWR:
 mips32_op = OPC_TLBWR;
 goto do_cp0;
+case TLBINV:
+mips32_op = OPC_TLBINV;
+goto do_cp0;
+case TLBINVF:
+mips32_op = OPC_TLBINVF;
+goto do_cp0;
 case WAIT:
 mips32_op = OPC_WAIT;
 goto do_cp0;
-- 
1.7.5.4

[Qemu-devel] [PATCH v4 01/15] target-mips: fix {RD, WR}PGPR in microMIPS

[Yongbok Kim]

rt, rs were swapped

Signed-off-by: Yongbok Kim 
Reviewed-by: Aurelien Jarno 
Reviewed-by: Leon Alrae 
---
 target-mips/translate.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 1d128ee..97b74ba 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -12991,12 +12991,12 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 case RDPGPR:
 check_cp0_enabled(ctx);
 check_insn(ctx, ISA_MIPS32R2);
-gen_load_srsgpr(rt, rs);
+gen_load_srsgpr(rs, rt);
 break;
 case WRPGPR:
 check_cp0_enabled(ctx);
 check_insn(ctx, ISA_MIPS32R2);
-gen_store_srsgpr(rt, rs);
+gen_store_srsgpr(rs, rt);
 break;
 default:
 goto pool32axf_invalid;
-- 
1.7.5.4

[Qemu-devel] [PATCH v4 08/15] target-mips: add microMIPS32 R6 opcode enum

[Yongbok Kim]

Add microMIPS32 Release 6 opcode enum.
Remove RI checking for pre-R6 reserved opcode.

Signed-off-by: Yongbok Kim 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |  119 --
 1 files changed, 103 insertions(+), 16 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 2ab00fc..0aaf5e0 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -12368,6 +12368,8 @@ enum {
 LBU16 = 0x02,
 MOVE16 = 0x03,
 ADDI32 = 0x04,
+R6_LUI = 0x04,
+AUI = 0x04,
 LBU32 = 0x05,
 SB32 = 0x06,
 LB32 = 0x07,
@@ -12390,56 +12392,88 @@ enum {
 POOL32S = 0x16,  /* MIPS64 */
 DADDIU32 = 0x17, /* MIPS64 */
 
-/* 0x1f is reserved */
 POOL32C = 0x18,
 LWGP16 = 0x19,
 LW16 = 0x1a,
 POOL16E = 0x1b,
 XORI32 = 0x1c,
 JALS32 = 0x1d,
+BOVC = 0x1d,
+BEQC = 0x1d,
+BEQZALC = 0x1d,
 ADDIUPC = 0x1e,
+PCREL = 0x1e,
+BNVC = 0x1f,
+BNEC = 0x1f,
+BNEZALC = 0x1f,
 
-/* 0x20 is reserved */
-RES_20 = 0x20,
+R6_BEQZC = 0x20,
+JIC = 0x20,
 POOL16F = 0x21,
 SB16 = 0x22,
 BEQZ16 = 0x23,
+BEQZC16 = 0x23,
 SLTI32 = 0x24,
 BEQ32 = 0x25,
+BC = 0x25,
 SWC132 = 0x26,
 LWC132 = 0x27,
 
-/* 0x28 and 0x29 are reserved */
-RES_28 = 0x28,
+/* 0x29 is reserved */
 RES_29 = 0x29,
+R6_BNEZC = 0x28,
+JIALC = 0x28,
 SH16 = 0x2a,
 BNEZ16 = 0x2b,
+BNEZC16 = 0x2b,
 SLTIU32 = 0x2c,
 BNE32 = 0x2d,
+BALC = 0x2d,
 SDC132 = 0x2e,
 LDC132 = 0x2f,
 
-/* 0x30 and 0x31 are reserved */
-RES_30 = 0x30,
+/* 0x31 is reserved */
 RES_31 = 0x31,
+BLEZALC = 0x30,
+BGEZALC = 0x30,
+BGEUC = 0x30,
 SWSP16 = 0x32,
 B16 = 0x33,
+BC16 = 0x33,
 ANDI32 = 0x34,
 J32 = 0x35,
+BGTZC = 0x35,
+BLTZC = 0x35,
+BLTC = 0x35,
 SD32 = 0x36, /* MIPS64 */
 LD32 = 0x37, /* MIPS64 */
 
-/* 0x38 and 0x39 are reserved */
-RES_38 = 0x38,
+/* 0x39 is reserved */
 RES_39 = 0x39,
+BGTZALC = 0x38,
+BLTZALC = 0x38,
+BLTUC = 0x38,
 SW16 = 0x3a,
 LI16 = 0x3b,
 JALX32 = 0x3c,
 JAL32 = 0x3d,
+BLEZC = 0x3d,
+BGEZC = 0x3d,
+BGEC = 0x3d,
 SW32 = 0x3e,
 LW32 = 0x3f
 };
 
+/* PCREL Instructions perform PC-Relative address calculation. bits 20..16 */
+enum {
+ADDIUPC_00 = 0x00,
+ADDIUPC_07 = 0x07,
+AUIPC = 0x1e,
+ALUIPC = 0x1f,
+LWPC_08 = 0x08,
+LWPC_0F = 0x0F,
+};
+
 /* POOL32A encoding of minor opcode field */
 
 enum {
@@ -12449,6 +12483,8 @@ enum {
 SRL32 = 0x1,
 SRA = 0x2,
 ROTR = 0x3,
+SELEQZ = 0x5,
+SELNEZ = 0x6,
 
 SLLV = 0x0,
 SRLV = 0x1,
@@ -12467,11 +12503,21 @@ enum {
 SLTU = 0xe,
 
 MOVN = 0x0,
+R6_MUL  = 0x0,
 MOVZ = 0x1,
+MUH  = 0x1,
+MULU = 0x2,
+MUHU = 0x3,
 LWXS = 0x4,
+R6_DIV  = 0x4,
+MOD  = 0x5,
+R6_DIVU = 0x6,
+MODU = 0x7,
 
 /* The following can be distinguished by their lower 6 bits. */
 INS = 0x0c,
+LSA = 0x0f,
+ALIGN = 0x1f,
 EXT = 0x2c,
 POOL32AXF = 0x3c
 };
@@ -12524,6 +12570,7 @@ enum {
 /* end of microMIPS32 DSP */
 
 /* bits 15..12 for 0x2c */
+BITSWAP = 0x0,
 SEB = 0x2,
 SEH = 0x3,
 CLO = 0x4,
@@ -12550,7 +12597,10 @@ enum {
 /* bits 15..12 for 0x3c */
 JALR = 0x0,
 JR = 0x0,   /* alias */
+JALRC = 0x0,
+JRC = 0x0,
 JALR_HB = 0x1,
+JALRC_HB = 0x1,
 JALRS = 0x4,
 JALRS_HB = 0x5,
 
@@ -12634,32 +12684,39 @@ enum {
 enum {
 /* These are the bit 7..6 values */
 ADD_FMT = 0x0,
-MOVN_FMT = 0x0,
 
 SUB_FMT = 0x1,
-MOVZ_FMT = 0x1,
 
 MUL_FMT = 0x2,
 
 DIV_FMT = 0x3,
 
 /* These are the bit 8..6 values */
+MOVN_FMT = 0x0,
 RSQRT2_FMT = 0x0,
 MOVF_FMT = 0x0,
+RINT_FMT = 0x0,
+SELNEZ_FMT = 0x0,
 
+MOVZ_FMT = 0x1,
 LWXC1 = 0x1,
 MOVT_FMT = 0x1,
+CLASS_FMT = 0x1,
+SELEQZ_FMT = 0x1,
 
 PLL_PS = 0x2,
 SWXC1 = 0x2,
+SEL_FMT = 0x2,
 
 PLU_PS = 0x3,
 LDXC1 = 0x3,
 
+MOVN_FMT_04 = 0x4,
 PUL_PS = 0x4,
 SDXC1 = 0x4,
 RECIP2_FMT = 0x4,
 
+MOVZ_FMT_05 = 0x05,
 PUU_PS = 0x5,
 LUXC1 = 0x5,
 
@@ -12667,8 +12724,10 @@ enum {
 SUXC1 = 0x6,
 ADDR_PS = 0x6,
 PREFX = 0x6,
+MADDF_FMT = 0x6,
 
 MULR_PS = 0x7,
+MSUBF_FMT = 0x7,
 
 MADD_S = 0x01,
 MADD_D = 0x09,
@@ -12685,10 +12744,17 @@ enum {
 NMSUB_D = 0x2a,
 NMSUB_PS = 0x32,
 
+MIN_FMT = 0x3,
+MAX_FMT = 0xb,
+MINA_FMT = 0x23,
+MAXA_FMT = 0x2b,
 POOL32FXF = 0x3b,
 
 CABS_COND_FMT = 0x1c,  /* MIPS3D */
-C_COND_FMT = 0x3c
+C_COND_FMT = 0x3c,
+
+CMP_CONDN_S = 0x5,
+CMP_CONDN_D = 0x15
 };
 
 /* POOL32Fxf encoding of minor opcode extension field */
@@ -12741,10 +12807,15 @@ enum {
 BGTZ = 0x06,
 BEQZC = 0x07,
 TLTI = 0x08,
+BC1EQZC = 0x08,
   

[Qemu-devel] [PATCH v4 03/15] target-mips: remove an unused argument

[Yongbok Kim]

Remove an unused argument from decode_micromips32_opc()

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Reviewed-by: Aurelien Jarno 
---
 target-mips/translate.c |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 963ff8b..83dfb2f 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -13404,8 +13404,7 @@ static void gen_pool32fxf(DisasContext *ctx, int rt, 
int rs)
 }
 }
 
-static void decode_micromips32_opc (CPUMIPSState *env, DisasContext *ctx,
-uint16_t insn_hw1)
+static void decode_micromips32_opc(CPUMIPSState *env, DisasContext *ctx)
 {
 int32_t offset;
 uint16_t insn;
@@ -14448,7 +14447,7 @@ static int decode_micromips_opc (CPUMIPSState *env, 
DisasContext *ctx)
 generate_exception(ctx, EXCP_RI);
 break;
 default:
-decode_micromips32_opc (env, ctx, op);
+decode_micromips32_opc(env, ctx);
 return 4;
 }
 
-- 
1.7.5.4

Re: [Qemu-devel] [RFC] QDev explicit constructors & destructors

[Liviu Ionescu]

> On 24 Jun 2015, at 17:11, Liviu Ionescu  wrote:
> 
> ... it might very well be C++, with some wrappers ...

unfortunately this is not technically feasible, for many reasons, one of them 
being the 'struct Object' member named 'class' :-(


regards,

Liviu

[Qemu-devel] [PATCH 2/2] block: qemu-iotests - add check for multiplication overflow in vpc

[Jeff Cody]

This checks that VPC is able to successfully fail (without segfault)
on an image file with a max_table_entries that exceeds 0x4000.

This table entry is within the valid range for VPC (although too large
for this sample image).

Signed-off-by: Jeff Cody 
---
 tests/qemu-iotests/135|  54 ++
 tests/qemu-iotests/135.out|   5 +++
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/sample_images/afl5.img.bz2 | Bin 0 -> 175 bytes
 4 files changed, 60 insertions(+)
 create mode 100755 tests/qemu-iotests/135
 create mode 100644 tests/qemu-iotests/135.out
 create mode 100644 tests/qemu-iotests/sample_images/afl5.img.bz2

diff --git a/tests/qemu-iotests/135 b/tests/qemu-iotests/135
new file mode 100755
index 000..16bf736
--- /dev/null
+++ b/tests/qemu-iotests/135
@@ -0,0 +1,54 @@
+#!/bin/bash
+#
+# Test VPC open of image with large Max Table Entries value.
+#
+# Copyright (C) 2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=jc...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt vpc
+_supported_proto generic
+_supported_os Linux
+
+_use_sample_img afl5.img.bz2
+
+echo
+echo "=== Verify image open and failure "
+$QEMU_IMG info "$TEST_IMG" 2>&1| _filter_testdir
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/135.out b/tests/qemu-iotests/135.out
new file mode 100644
index 000..70b305e
--- /dev/null
+++ b/tests/qemu-iotests/135.out
@@ -0,0 +1,5 @@
+QA output created by 135
+
+=== Verify image open and failure 
+qemu-img: Could not open 'TEST_DIR/afl5.img': block-vpc: 
free_data_block_offset points after the end of file. The image has been 
truncated.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 4597fc1..557e2a2 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -132,3 +132,4 @@
 130 rw auto quick
 131 rw auto quick
 134 rw auto quick
+135 rw auto
diff --git a/tests/qemu-iotests/sample_images/afl5.img.bz2 
b/tests/qemu-iotests/sample_images/afl5.img.bz2
new file mode 100644
index 
..1614348865e5b2cfcb0340eab9474841717be2c5
GIT binary patch
literal 175
zcmV;g08sxzT4*^jL0KkKSqT!KVgLXwfB*jgAVdfNFaTf(B!Frw|3pDR00;sy03ZSY
z3IG5B1Sp^YbSh$=r=cgVwVQS9W$Kd2?dJ>H~Ej+J=Q^
dtom#MI_=bg;S5HeF^MqnF64@Ep&$|^KE!naKsf*a

literal 0
HcmV?d1

-- 
1.9.3

[Qemu-devel] [PATCH 1/2] block: vpc - prevent overflow if max_table_entries >= 0x40000000

[Jeff Cody]

When we allocate the pagetable based on max_table_entries, we multiply
the max table entry value by 4 to accomodate a table of 32-bit integers.
However, max_table_entries is a uint32_t, and the VPC driver accepts
ranges for that entry over 0x4000.  So during this allocation:

s->pagetable = qemu_try_blockalign(bs->file, s->max_table_entries * 4);

The size arg overflows, allocating significantly less memory than
expected.

Since qemu_try_blockalign() size argument is size_t, cast the
multiplication correctly to prevent overflow.

The value of "max_table_entries * 4" is used elsewhere in the code as
well, so store the correct value for use in all those cases.

Reported-by: Richard W.M. Jones 
Signed-off-by: Jeff Cody 
---
 block/vpc.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 37572ba..4108b5e 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -168,6 +168,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
 uint8_t buf[HEADER_SIZE];
 uint32_t checksum;
 uint64_t computed_size;
+uint64_t pagetable_size;
 int disk_type = VHD_DYNAMIC;
 int ret;
 
@@ -269,7 +270,9 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
 goto fail;
 }
 
-s->pagetable = qemu_try_blockalign(bs->file, s->max_table_entries * 4);
+pagetable_size = (size_t) s->max_table_entries * 4;
+
+s->pagetable = qemu_try_blockalign(bs->file, pagetable_size);
 if (s->pagetable == NULL) {
 ret = -ENOMEM;
 goto fail;
@@ -277,14 +280,13 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
 
 s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
 
-ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable,
- s->max_table_entries * 4);
+ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable, 
pagetable_size);
 if (ret < 0) {
 goto fail;
 }
 
 s->free_data_block_offset =
-(s->bat_offset + (s->max_table_entries * 4) + 511) & ~511;
+(s->bat_offset + pagetable_size + 511) & ~511;
 
 for (i = 0; i < s->max_table_entries; i++) {
 be32_to_cpus(&s->pagetable[i]);
-- 
1.9.3

[Qemu-devel] [PATCH 0/2] block: vpc - prevent overflow

[Jeff Cody]

This series fixes a bug found by Richard Jones.

When we allocate the pagetable based on max_table_entries, we multiply
the max table entry value by 4 to accomodate a table of 32-bit integers.
However, max_table_entries is a uint32_t, and the VPC driver accepts
ranges for that entry over 0x4000.  So during this allocation:

s->pagetable = qemu_try_blockalign(bs->file, s->max_table_entries * 4);

The size arg overflows, allocating significantly less memory than
expected.

Since qemu_try_blockalign() size argument is size_t, cast the
multiplication correctly to prevent overflow.

The value of "max_table_entries * 4" is used elsewhere in the code as
well, so store the correct value for use in all those cases.

Jeff Cody (2):
  block: vpc - prevent overflow if max_table_entries >= 0x4000
  block: qemu-iotests - add check for multiplication overflow in vpc

 block/vpc.c   |  10 +++--
 tests/qemu-iotests/135|  54 ++
 tests/qemu-iotests/135.out|   5 +++
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/sample_images/afl5.img.bz2 | Bin 0 -> 175 bytes
 5 files changed, 66 insertions(+), 4 deletions(-)
 create mode 100755 tests/qemu-iotests/135
 create mode 100644 tests/qemu-iotests/135.out
 create mode 100644 tests/qemu-iotests/sample_images/afl5.img.bz2

-- 
1.9.3

[Qemu-devel] [Bug 1329956] Re: multi-core FreeBSD guest hangs after warm reboot

[John Nielsen]

I am no longer able to reproduce this issue on a fully-updated server.
My guess is that the issue was fixed in the kernel somewhere between
3.12 and 4.0, but for all I know it could be a Qemu (or even Seabios)
change. Here are details of my test that failed and the one that
succeeded.

Breaks (VM hangs during boot after pressing ctrl-alt-del):
kernel 3.12.22
qemu-kvm-1.7.0-3.el6.x86_64
seabios-1.7.3.1-1.el6.noarch
Intel(R) Xeon(R) CPU E5-2667 v2 @ 3.30GHz

Works (VM reboots normally):
kernel 4.0.4
qemu-kvm-2.3.0-6.el7.centos.x86_64
seabios-bin-1.8.1-1.el7.centos.noarch
Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz

I'd still like to narrow down the change that fixed it if possible.

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

Title:
  multi-core FreeBSD guest hangs after warm reboot

Status in QEMU:
  Incomplete

Bug description:
  On some Linux KVM hosts in our environment, FreeBSD guests fail to
  reboot properly if they have more than one CPU (socket, core, and/or
  thread). They will boot fine the first time, but after issuing a
  "reboot" command via the OS the guest starts to boot but hangs during
  SMP initialization. Fully shutting down and restarting the guest works
  in all cases.

  The only meaningful difference between hosts with the problem and those 
without is the CPU. Hosts with Xeon E5-26xx v2 processors have the problem, 
including at least the "Intel(R) Xeon(R) CPU E5-2667 v2" and the "Intel(R) 
Xeon(R) CPU E5-2650 v2".
  Hosts with any other CPU, including "Intel(R) Xeon(R) CPU E5-2650 0", 
"Intel(R) Xeon(R) CPU E5-2620 0", or "AMD Opteron(TM) Processor 6274" do not 
have the problem. Note the "v2" in the names of the problematic CPUs.

  On hosts with a "v2" Xeon, I can reproduce the problem under Linux
  kernel 3.10 or 3.12 and Qemu 1.7.0 or 2.0.0.

  The problem occurs with all currently-supported versions of FreeBSD,
  including 8.4, 9.2, 10.0 and 11-CURRENT.

  On a Linux KVM host with a "v2" Xeon, this command line is adequate to
  reproduce the problem:

  /usr/bin/qemu-system-x86_64 -machine accel=kvm -name bsdtest -m 512
  -smp 2,sockets=1,cores=1,threads=2 -drive
  file=./20140613_FreeBSD_9.2-RELEASE_ufs.qcow2,if=none,id=drive0,format=qcow2
  -device virtio-blk-pci,scsi=off,drive=drive0 -vnc 0.0.0.0:0 -net none

  I have tried many variations including different models of -machine
  and -cpu for the guest with no visible difference.

  A native FreeBSD installation on a host with a "v2" Xeon does not have
  the problem, nor do a paravirtualized FreeBSD guests under bhyve (the
  BSD legacy-free hypervisor) using the same FreeBSD disk images as on
  the Linux hosts. So it seems unlikely the cause is on the FreeBSD side
  of things.

  I would greatly appreciate any feedback or developer attention to
  this. I am happy to provide additional details, test patches, etc.

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

Re: [Qemu-devel] [PATCH] m68k: remove useless parameter op_size from gen_lea_indexed()

[Thomas Huth]

On Wed, 24 Jun 2015 02:51:49 +0200
Laurent Vivier  wrote:

> Signed-off-by: Laurent Vivier 
> ---
>  target-m68k/translate.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
> index d6c478f..bc83a6e 100644
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c
> @@ -297,8 +297,7 @@ static TCGv gen_addr_index(uint16_t ext, TCGv tmp)
>  
>  /* Handle a base + index + displacement effective addresss.
> A NULL_QREG base means pc-relative.  */
> -static TCGv gen_lea_indexed(CPUM68KState *env, DisasContext *s, int opsize,
> -TCGv base)
> +static TCGv gen_lea_indexed(CPUM68KState *env, DisasContext *s, TCGv base)
>  {
>  uint32_t offset;
>  uint16_t ext;
> @@ -529,7 +528,7 @@ static TCGv gen_lea(CPUM68KState *env, DisasContext *s, 
> uint16_t insn,
>  return tmp;
>  case 6: /* Indirect index + displacement.  */
>  reg = AREG(insn, 0);
> -return gen_lea_indexed(env, s, opsize, reg);
> +return gen_lea_indexed(env, s, reg);
>  case 7: /* Other */
>  switch (insn & 7) {
>  case 0: /* Absolute short.  */
> @@ -545,7 +544,7 @@ static TCGv gen_lea(CPUM68KState *env, DisasContext *s, 
> uint16_t insn,
>  s->pc += 2;
>  return tcg_const_i32(offset);
>  case 3: /* pc index+displacement.  */
> -return gen_lea_indexed(env, s, opsize, NULL_QREG);
> +return gen_lea_indexed(env, s, NULL_QREG);
>  case 4: /* Immediate.  */
>  default:
>  return NULL_QREG;

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH] m68k: is_mem is useless

[Peter Maydell]

On 24 June 2015 at 00:00, Laurent Vivier  wrote:
> Remove is_mem as it is never tested anymore since:
>
> commit bfa50bc2638d877cf2900712b7503be22e8811cb
> Author: aliguori 
> Date:   Tue Nov 18 20:26:41 2008 +
>
> Remove premature memop TB terminations (Jan Kiszka)
>
> Signed-off-by: Laurent Vivier 

How would you like to handle getting these cleanup patches into master?
If you want to collect up the ones which have got review and send
a pull request with them (sometime next week, maybe), I'm happy to
apply it.

(NB: given release cycle, that applies for the moment to the
ones like this which are relatively simple. We should probably
wait until after 2.4 to start trying to get more significant
changes to target-m68k upstream.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] m68k: is_mem is useless

[Thomas Huth]

On Wed, 24 Jun 2015 01:00:22 +0200
Laurent Vivier  wrote:

> Remove is_mem as it is never tested anymore since:
> 
> commit bfa50bc2638d877cf2900712b7503be22e8811cb
> Author: aliguori 
> Date:   Tue Nov 18 20:26:41 2008 +
> 
> Remove premature memop TB terminations (Jan Kiszka)
> 
> Signed-off-by: Laurent Vivier 
> ---
>  target-m68k/translate.c | 7 ---
>  1 file changed, 7 deletions(-)
> 
> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
> index 96d75bf..ce1fc92 100644
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c
> @@ -129,7 +129,6 @@ typedef struct DisasContext {
>  uint32_t fpcr;
>  struct TranslationBlock *tb;
>  int singlestep_enabled;
> -int is_mem;
>  TCGv_i64 mactmp;
>  int done_mac;
>  } DisasContext;
> @@ -179,7 +178,6 @@ static inline TCGv gen_load(DisasContext * s, int opsize, 
> TCGv addr, int sign)
>  {
>  TCGv tmp;
>  int index = IS_USER(s);
> -s->is_mem = 1;
>  tmp = tcg_temp_new_i32();
>  switch(opsize) {
>  case OS_BYTE:
> @@ -209,7 +207,6 @@ static inline TCGv_i64 gen_load64(DisasContext * s, TCGv 
> addr)
>  {
>  TCGv_i64 tmp;
>  int index = IS_USER(s);
> -s->is_mem = 1;
>  tmp = tcg_temp_new_i64();
>  tcg_gen_qemu_ldf64(tmp, addr, index);
>  gen_throws_exception = gen_last_qop;
> @@ -220,7 +217,6 @@ static inline TCGv_i64 gen_load64(DisasContext * s, TCGv 
> addr)
>  static inline void gen_store(DisasContext *s, int opsize, TCGv addr, TCGv 
> val)
>  {
>  int index = IS_USER(s);
> -s->is_mem = 1;
>  switch(opsize) {
>  case OS_BYTE:
>  tcg_gen_qemu_st8(val, addr, index);
> @@ -241,7 +237,6 @@ static inline void gen_store(DisasContext *s, int opsize, 
> TCGv addr, TCGv val)
>  static inline void gen_store64(DisasContext *s, TCGv addr, TCGv_i64 val)
>  {
>  int index = IS_USER(s);
> -s->is_mem = 1;
>  tcg_gen_qemu_stf64(val, addr, index);
>  gen_throws_exception = gen_last_qop;
>  }
> @@ -2227,7 +,6 @@ DISAS_INSN(fpu)
>  mask = 0x80;
>  for (i = 0; i < 8; i++) {
>  if (ext & mask) {
> -s->is_mem = 1;
>  dest = FREG(i, 0);
>  if (ext & (1 << 13)) {
>  /* store */
> @@ -2999,7 +2993,6 @@ gen_intermediate_code_internal(M68kCPU *cpu, 
> TranslationBlock *tb,
>  dc->singlestep_enabled = cs->singlestep_enabled;
>  dc->fpcr = env->fpcr;
>  dc->user = (env->sr & SR_S) == 0;
> -dc->is_mem = 0;
>  dc->done_mac = 0;
>  lj = -1;
>  num_insns = 0;

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH qom v3 3/4] arm: boot: Use cpu_set_pc()

[Peter Maydell]

On 24 June 2015 at 19:22, Andreas Färber  wrote:
> Am 24.06.2015 um 05:19 schrieb Peter Crosthwaite:
>> Use cpu_set_pc() across the board for setting program counters. This
>> removes instances of system level code having to reach into the CPU
>> env.
>>
>> Reviewed-by: Peter Maydell 
>> Reviewed-by: Andreas Färber 
>> Signed-off-by: Peter Crosthwaite 
>> ---
>> Changed since v2:
>> Add () to fn names in commit msg
>> Drop error argument
>> Changed since v1:
>> Lease thumb masking in boot.c
>> ---
>>  hw/arm/boot.c | 19 +++
>>  1 file changed, 7 insertions(+), 12 deletions(-)
>
> Any objection to avoiding repeated casts as follows?

Fine with me; you can keep my r-b tag.

-- PMM

Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)

[Peter Maydell]

On 24 June 2015 at 18:18, Alex Bennée  wrote:
>
> Paolo Bonzini  writes:
>
>> On 24/06/2015 17:34, Alex Bennée wrote:
>>> Testing with Alexander's bare metal syncronisation tests fails in MTTCG
>>> leaving one CPU spinning forever waiting for the second CPU to wake up.
>>> We simply need to poke the halt_cond once we have processed the PSCI
>>> power on call.
>>>
>>> Tested-by: Alex Bennée 
>>> CC: Alexander Spyridakis 
>>>
>>> ---
>>> TODO
>>>   - exactly how does the vexpress wake up it's sleeping CPUs?
>>> ---
>>>  target-arm/psci.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/target-arm/psci.c b/target-arm/psci.c
>>> index d8fafab..661ff28 100644
>>> --- a/target-arm/psci.c
>>> +++ b/target-arm/psci.c
>>> @@ -196,6 +196,8 @@ void arm_handle_psci_call(ARMCPU *cpu)
>>>  }
>>>  target_cpu_class->set_pc(target_cpu_state, entry);
>>>
>>> +qemu_cond_signal(target_cpu_state->halt_cond);
>>
>> That's called qemu_cpu_kick(target_cpu_state). :)  The patch should be
>> acceptable now upstream, I think.
>
> Oh so this might well fail in KVM too?

In KVM we won't use target-arm/psci.c because PSCI calls
are handled in the kernel.

-- PMM

Re: [Qemu-devel] [PATCH] m68k: remove useless file m68k-qreg.h

[Thomas Huth]

On Wed, 24 Jun 2015 02:07:24 +0200
Laurent Vivier  wrote:

> Unused since:
> 
> commit e1f3808e03f73e7a7fa966afbed2455dd052202e
> Author: pbrook 
> Date:   Sat May 24 22:29:16 2008 +
> 
> Convert m68k target to TCG.
> 
> Signed-off-by: Laurent Vivier 
> ---
>  target-m68k/m68k-qreg.h | 11 ---
>  1 file changed, 11 deletions(-)
>  delete mode 100644 target-m68k/m68k-qreg.h
> 
> diff --git a/target-m68k/m68k-qreg.h b/target-m68k/m68k-qreg.h
> deleted file mode 100644
> index c224d5e..000
> --- a/target-m68k/m68k-qreg.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -enum {
> -#define DEFO32(name, offset) QREG_##name,
> -#define DEFR(name, reg, mode) QREG_##name,
> -#define DEFF64(name, offset) QREG_##name,
> -QREG_NULL,
> -#include "qregs.def"
> -TARGET_NUM_QREGS = 0x100
> -#undef DEFO32
> -#undef DEFR
> -#undef DEFF64
> -};

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH qom v3 1/4] cpu: Add wrapper to the set-pc() hook

[Peter Maydell]

On 24 June 2015 at 19:09, Andreas Färber  wrote:
> s/set-pc/set_pc/
>
> Am 24.06.2015 um 05:19 schrieb Peter Crosthwaite:
>> Add a wrapper around the CPUClass::set_pc() hook.
>>
>> Signed-off-by: Peter Crosthwaite 
>> ---
>> changed since v2:
>> drop "qom" from commit message subject.
>> Add () to functions in commit messages.
>> Drop error argument
>> ---
>>  include/qom/cpu.h | 17 +
>>  1 file changed, 17 insertions(+)
>
> Queuing on qom-cpu-next with the following change:
>
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -604,16 +604,14 @@ static inline void cpu_unaligned_access(CPUState
> *cpu, vaddr addr,
>   * @cpu: The CPU to set the program counter for.
>   * @addr: Program counter value.
>   *
> - * Set the program counter for a CPU. If there is no available
> implementation
> - * no action occurs.
> + * Sets the program counter for a CPU.
>   */
>  static inline void cpu_set_pc(CPUState *cpu, vaddr addr)
>  {
>  CPUClass *cc = CPU_GET_CLASS(cpu);
>
> -if (cc->set_pc) {
> -cc->set_pc(cpu, addr);
> -}
> +g_assert(cc->set_pc != NULL);
> +cc->set_pc(cpu, addr);
>  }

Do we need this assert? If it would have fired
then we'll just crash immediately calling the null pointer,
so it's not like it's guarding against a more subtle failure
at a later point...

-- PMM

Re: [Qemu-devel] [PATCH v2 2/4] gdbstub: Use cpu_set_pc helper

[Peter Maydell]

On 24 June 2015 at 18:16, Andreas Färber  wrote:
> Guys, is there any target that does not implement set_pc today? If so,
> which? I'd rather implement it than carry around the iffery and
> resulting complications.

No, there are none, see my analysis in my review of patch 1 in this set.

-- PMM

Re: [Qemu-devel] TCG baremetal tests repo

[Peter Maydell]

On 24 June 2015 at 17:39, Alex Bennée  wrote:
>
> Alexander Spyridakis  writes:
>> You can find the latest tcg atomic test payload in the following repo:
>>> git clone https://git.virtualopensystems.com/dev/tcg_baremetal_tests.git
>>
>> You also need an arm baremetal cross-compiler like arm-none-gnueabi- (arm)
>> and the usual aarch64-linux-gnu- (arm64). Due to a PSCI bug in the current
>> multithreading tcg repo, the atomic test was modified to work also on the
>> vexpress machine model.
>
> I sent a patch to fix the PSCI hang case to wake up the sleeping CPU. I
> couldn't figure out how the vexpress code was waking it's CPUs. Do they
> just start powered on?

Yes -- our vexpress model is "like hardware", so all CPUs leap into
the boot firmware at once, and the firmware deals with putting the
secondaries into a pen to be released in a controlled manner later.
[assuming you're not using -kernel; if you are then boot.c has the
secondary pen code]

-- PMM

Re: [Qemu-devel] [PATCH 5/9] memory: let address_space_rw/ld*/st* run outside the BQL

[Alex Bennée]

Paolo Bonzini  writes:

> On 24/06/2015 18:56, Alex Bennée wrote:
>> This is where I get confused between the advantage of this over however
>> same pid recursive locking. If you use recursive locking you don't need
>> to add a bunch of state to remind you of when to release the lock. Then
>> you'd just need:
>> 
>> static void prepare_mmio_access(MemoryRegion *mr)
>> {
>> if (mr->global_locking) {
>> qemu_mutex_lock_iothread();
>> }
>> if (mr->flush_coalesced_mmio) {
>> qemu_mutex_lock_iothread();
>> qemu_flush_coalesced_mmio_buffer();
>> qemu_mutex_unlock_iothread();
>> }
>> }
>> 
>> and a bunch of:
>> 
>> if (mr->global_locking)
>>qemu_mutex_unlock_iothread();
>> 
>> in the access functions. Although I suspect you could push the
>> mr->global_locking up to the dispatch functions.
>> 
>> Am I missing something here?
>
> The semantics of recursive locking with respect to condition variables
> are not clear.  Either cond_wait releases all locks, and then the mutex
> can be released when the code doesn't expect it to be, or cond_wait
> doesn't release all locks and then you have deadlocks.
>
> POSIX says to do the latter:
>
>   It is advised that an application should not use a
>   PTHREAD_MUTEX_RECURSIVE mutex with condition variables because
>   the implicit unlock performed for a pthread_cond_timedwait() or
>   pthread_cond_wait() may not actually release the mutex (if it
>   had been locked multiple times). If this happens, no other
>   thread can satisfy the condition of the predicate."
>
> So, recursive locking is okay if you don't have condition variables
> attached to the lock (and if you cannot do without it), but
> qemu_global_mutex does have them.

Ahh OK, so I was missing something ;-)

>
> QEMU has so far tried to use the solution that Stevens outlines here:
> https://books.google.it/books?id=kCTMFpEcIOwC&pg=PA434
>
>   ... Leave the interfaces to func1 and func2 unchanged, and avoid
>   a recursive mutex by providing a private version of func2,
>   called func2_locked.  To call func2_locked, hold the mutex
>   embedded in the data structure whose address we pass as the
>   argument.
>
> as a way to avoid recursive locking.  This is much better because a) it
> is more efficient---taking locks can be expensive even if they're
> uncontended, especially if your VM spans multiple NUMA nodes on the
> host; b) it is always clear when a lock is taken and when it isn't.
>
> Note that Stevens has another example right after this one of recursive
> locking, involving callbacks, but it's ill-defined.  There's no reason
> for the "timeout" function in page 437 to hold the mutex when it calls
> "func".  It can unlock before and re-lock afterwards, like QEMU's own
> timerlist_run_timers function.

Unfortunately I can't read that link but it sounds like I should get
myself a copy of the book. I take it that approach wouldn't approve of:

static __thread int iothread_lock_count;

void qemu_mutex_lock_iothread(void)
{
if (iothread_lock_count == 0) {
qemu_mutex_lock(&qemu_global_mutex);
}
iothread_lock_count++;
}

void qemu_mutex_unlock_iothread(void)
{
iothread_lock_count--;
if (iothread_lock_count==0) {
qemu_mutex_unlock(&qemu_global_mutex);
}
if (iothread_lock_count < 0) {
fprintf(stderr,"%s: error, too many unlocks %d\n", __func__,
iothread_lock_count);
}
}

Which should achieve the same "only one lock held" semantics but still
make the calling code a little less worried about tracking the state.

I guess it depends if there is ever going to be a situation where we say
"lock is held, do something different"?

>
> Paolo

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH qom v3 0/4] qom-cpu: Wrap set_pc hook and use in bootloaders

[Andreas Färber]

Am 24.06.2015 um 05:19 schrieb Peter Crosthwaite:
> Wrap the CPUClass::set_pc fn hook in a caller helper to reduce
> verbosity of calls. Simplify the call from the gdbstub.
> 
> Then use the call to abstract away the PC env fields from the ARM and
> Microblaze bootloaders.
> 
> This moves towards the goal of minimising system level code of the CPU
> env (and one step closer to common-obj'ing the bootloaders). There's a
> long way to go (at least for ARM, not so far for MB), but this is a
> small win in that direction.
> 
> This helps with multi-arch where the current thinking is to compile
> out the maximum content possible from cpu.h. This removes program
> counter definitions from the multi-arch cpu.h compile-in list.
> 
> changed since v2:
> drop error argument
> misc commit messages tweaks
> 
> changed since v1:
> Remove thumb changes
> 
> Peter Crosthwaite (4):
>   cpu: Add wrapper to the set-pc() hook
>   gdbstub: Use cpu_set_pc() helper
>   arm: boot: Use cpu_set_pc()
>   microblaze: boot: Use cpu_set_pc()

Thanks, queued with mentioned code modifications on qom-cpu-next:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next

(This is a purely intermediate staging for the lesser reviewed patches
and does not indicate they will miss my belated 2.4 pull.)

Regards,
Andreas

> 
>  dtc  |  2 +-
>  gdbstub.c|  5 +
>  hw/arm/boot.c| 19 +++
>  hw/microblaze/boot.c |  2 +-
>  include/qom/cpu.h| 17 +
>  5 files changed, 27 insertions(+), 18 deletions(-)

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Re: [Qemu-devel] [RFC 5/9] block: add block job transactions

[Max Reitz]

On 12.06.2015 12:09, Stefan Hajnoczi wrote:

Sometimes block jobs must execute as a transaction group.  Finishing
jobs wait until all other jobs are ready to complete successfully.
Failure or cancellation of one job cancels the other jobs in the group.

Signed-off-by: Stefan Hajnoczi 
---
  blockjob.c| 160 ++
  include/block/block.h |   1 +
  include/block/block_int.h |   3 +-
  include/block/blockjob.h  |  49 ++
  trace-events  |   4 ++
  5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/blockjob.c b/blockjob.c
index 2755465..ff622f5 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -399,3 +399,163 @@ void block_job_defer_to_main_loop(BlockJob *job,
  
  qemu_bh_schedule(data->bh);

  }
+
+/* Transactional group of block jobs */
+struct BlockJobTxn {
+/* Jobs may be in different AioContexts so protect all fields */
+QemuMutex lock;
+
+/* Reference count for txn object */
+unsigned int ref;
+
+/* Is this txn cancelling its jobs? */
+bool aborting;
+
+/* Number of jobs still running */
+unsigned int jobs_pending;
+
+/* List of jobs */
+QLIST_HEAD(, BlockJob) jobs;
+};
+
+BlockJobTxn *block_job_txn_new(void)
+{
+BlockJobTxn *txn = g_new(BlockJobTxn, 1);
+qemu_mutex_init(&txn->lock);
+txn->ref = 1; /* dropped by block_job_txn_begin() */
+txn->aborting = false;
+txn->jobs_pending = 0;
+QLIST_INIT(&txn->jobs);
+return txn;
+}
+
+static void block_job_txn_unref(BlockJobTxn *txn)
+{
+qemu_mutex_lock(&txn->lock);
+
+if (--txn->ref > 0) {
+qemu_mutex_unlock(&txn->lock);
+return;
+}
+
+qemu_mutex_unlock(&txn->lock);
+qemu_mutex_destroy(&txn->lock);
+g_free(txn);
+}
+
+/* The purpose of this is to keep txn alive until all jobs have been added */
+void block_job_txn_begin(BlockJobTxn *txn)
+{
+block_job_txn_unref(txn);
+}
+
+void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
+{
+if (!txn) {
+return;
+}


Do you plan on making use of this case? I'm asking because while I'm 
usually in favor of handling everything gracefully as long as it's easy 
to implement, here I can't think of a case where using NULL with this 
function makes sense, that is to me it would seem like the caller made 
some bad mistake. This in turn would mean that dereferencing a NULL 
pointer or failing an assertion were preferable to hiding that mistake.


Other than this small thing and that it doesn't compile (until patch 7, 
I presume), looks good.


Max

Re: [Qemu-devel] [PATCH qom v3 4/4] microblaze: boot: Use cpu_set_pc()

[Andreas Färber]

Am 24.06.2015 um 20:00 schrieb Andreas Färber:
> Am 24.06.2015 um 05:19 schrieb Peter Crosthwaite:
>> Use cpu_set_pc() for setting program counters when bootloading. This
>> removes an instance of system level code having to reach into the CPU
>> env.
>>
>> Reviewed-by: Andreas Färber 
>> Signed-off-by: Peter Crosthwaite 
>> ---
>> changed since v2:
>> Add () to function names in commit messages
>> ---
>>  dtc  | 2 +-
>>  hw/microblaze/boot.c | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/dtc b/dtc
>> index 65cc4d2..bc895d6 16
>> --- a/dtc
>> +++ b/dtc
>> @@ -1 +1 @@
>> -Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
>> +Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde
> 
> Submodule strikes again. Preparing to queue.

Will squash the following deduplication:

diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 9f4698a..3e8820f 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -48,13 +48,14 @@ static struct
 static void main_cpu_reset(void *opaque)
 {
 MicroBlazeCPU *cpu = opaque;
+CPUState *cs = CPU(cpu);
 CPUMBState *env = &cpu->env;

-cpu_reset(CPU(cpu));
+cpu_reset(cs);
 env->regs[5] = boot_info.cmdline;
 env->regs[6] = boot_info.initrd_start;
 env->regs[7] = boot_info.fdt;
-cpu_set_pc(CPU(cpu), boot_info.bootstrap_pc);
+cpu_set_pc(cs, boot_info.bootstrap_pc);
 if (boot_info.machine_cpu_reset) {
 boot_info.machine_cpu_reset(cpu);
 }

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH qom v3 3/4] arm: boot: Use cpu_set_pc()

[Peter Crosthwaite]

On Wed, Jun 24, 2015 at 11:22 AM, Andreas Färber  wrote:
> Am 24.06.2015 um 05:19 schrieb Peter Crosthwaite:
>> Use cpu_set_pc() across the board for setting program counters. This
>> removes instances of system level code having to reach into the CPU
>> env.
>>
>> Reviewed-by: Peter Maydell 
>> Reviewed-by: Andreas Färber 
>> Signed-off-by: Peter Crosthwaite 
>> ---
>> Changed since v2:
>> Add () to fn names in commit msg
>> Drop error argument
>> Changed since v1:
>> Lease thumb masking in boot.c
>> ---
>>  hw/arm/boot.c | 19 +++
>>  1 file changed, 7 insertions(+), 12 deletions(-)
>
> Any objection to avoiding repeated casts as follows?

No objection from me. Ack to the squash or can be respun.

Regards,
Peter

>
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -168,9 +168,11 @@ static void default_write_secondary(ARMCPU *cpu,
>  static void default_reset_secondary(ARMCPU *cpu,
>  const struct arm_boot_info *info)
>  {
> +CPUState *cs = CPU(cpu);
> +
>  address_space_stl_notdirty(&address_space_memory,
> info->smp_bootreg_addr,
> 0, MEMTXATTRS_UNSPECIFIED, NULL);
> -cpu_set_pc(CPU(cpu), info->smp_loader_start);
> +cpu_set_pc(cs, info->smp_loader_start);
>  }
>
>  static inline bool have_dtb(const struct arm_boot_info *info)
> @@ -443,10 +445,11 @@ fail:
>  static void do_cpu_reset(void *opaque)
>  {
>  ARMCPU *cpu = opaque;
> +CPUState *cs = CPU(cpu);
>  CPUARMState *env = &cpu->env;
>  const struct arm_boot_info *info = env->boot_info;
>
> -cpu_reset(CPU(cpu));
> +cpu_reset(cs);
>  if (info) {
>  if (!info->is_linux) {
>  /* Jump to the entry point.  */
> @@ -456,7 +459,7 @@ static void do_cpu_reset(void *opaque)
>  env->thumb = info->entry & 1;
>  entry &= 0xfffe;
>  }
> -cpu_set_pc(CPU(cpu), entry);
> +cpu_set_pc(cs, entry);
>  } else {
>  /* If we are booting Linux then we need to check whether we are
>   * booting into secure or non-secure state and adjust the state
> @@ -486,8 +489,8 @@ static void do_cpu_reset(void *opaque)
>  }
>  }
>
> -if (CPU(cpu) == first_cpu) {
> -cpu_set_pc(CPU(cpu), info->loader_start);
> +if (cs == first_cpu) {
> +cpu_set_pc(cs, info->loader_start);
>
>  if (!have_dtb(info)) {
>  if (old_param) {
>
> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
>

Re: [Qemu-devel] [PATCH qom v3 3/4] arm: boot: Use cpu_set_pc()

[Andreas Färber]

Am 24.06.2015 um 05:19 schrieb Peter Crosthwaite:
> Use cpu_set_pc() across the board for setting program counters. This
> removes instances of system level code having to reach into the CPU
> env.
> 
> Reviewed-by: Peter Maydell 
> Reviewed-by: Andreas Färber 
> Signed-off-by: Peter Crosthwaite 
> ---
> Changed since v2:
> Add () to fn names in commit msg
> Drop error argument
> Changed since v1:
> Lease thumb masking in boot.c
> ---
>  hw/arm/boot.c | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)

Any objection to avoiding repeated casts as follows?

--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -168,9 +168,11 @@ static void default_write_secondary(ARMCPU *cpu,
 static void default_reset_secondary(ARMCPU *cpu,
 const struct arm_boot_info *info)
 {
+CPUState *cs = CPU(cpu);
+
 address_space_stl_notdirty(&address_space_memory,
info->smp_bootreg_addr,
0, MEMTXATTRS_UNSPECIFIED, NULL);
-cpu_set_pc(CPU(cpu), info->smp_loader_start);
+cpu_set_pc(cs, info->smp_loader_start);
 }

 static inline bool have_dtb(const struct arm_boot_info *info)
@@ -443,10 +445,11 @@ fail:
 static void do_cpu_reset(void *opaque)
 {
 ARMCPU *cpu = opaque;
+CPUState *cs = CPU(cpu);
 CPUARMState *env = &cpu->env;
 const struct arm_boot_info *info = env->boot_info;

-cpu_reset(CPU(cpu));
+cpu_reset(cs);
 if (info) {
 if (!info->is_linux) {
 /* Jump to the entry point.  */
@@ -456,7 +459,7 @@ static void do_cpu_reset(void *opaque)
 env->thumb = info->entry & 1;
 entry &= 0xfffe;
 }
-cpu_set_pc(CPU(cpu), entry);
+cpu_set_pc(cs, entry);
 } else {
 /* If we are booting Linux then we need to check whether we are
  * booting into secure or non-secure state and adjust the state
@@ -486,8 +489,8 @@ static void do_cpu_reset(void *opaque)
 }
 }

-if (CPU(cpu) == first_cpu) {
-cpu_set_pc(CPU(cpu), info->loader_start);
+if (cs == first_cpu) {
+cpu_set_pc(cs, info->loader_start);

 if (!have_dtb(info)) {
 if (old_param) {

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)

[Alex Bennée]

Paolo Bonzini  writes:

> On 24/06/2015 19:18, Alex Bennée wrote:
 >> @@ -196,6 +196,8 @@ void arm_handle_psci_call(ARMCPU *cpu)
 >>  }
 >>  target_cpu_class->set_pc(target_cpu_state, entry);
 >>  
 >> +qemu_cond_signal(target_cpu_state->halt_cond);
>>> >
>>> > That's called qemu_cpu_kick(target_cpu_state). :)  The patch should be
>>> > acceptable now upstream, I think.
>> Oh so this might well fail in KVM too?
>> 
>> The qemu_cpu_kick does a qemu_cond_broadcast(cpu->halt_cond) which seems
>> a little excessive? Won't all sleeping CPUs wake up (and return to sleep)?
>
> On KVM (and I assume on MT-TCG), each CPU has a different halt_cond.

You are right of course, I got my sense the wrong way around. Given it
is per-cpu I wonder if you will ever have multiple threads waiting on
it?

Anyway I'll fix that up and re-submit after I've tested to see of these
test cases break current KVM.

>
> Paolo

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH qom v3 1/4] cpu: Add wrapper to the set-pc() hook

[Andreas Färber]

s/set-pc/set_pc/

Am 24.06.2015 um 05:19 schrieb Peter Crosthwaite:
> Add a wrapper around the CPUClass::set_pc() hook.
> 
> Signed-off-by: Peter Crosthwaite 
> ---
> changed since v2:
> drop "qom" from commit message subject.
> Add () to functions in commit messages.
> Drop error argument
> ---
>  include/qom/cpu.h | 17 +
>  1 file changed, 17 insertions(+)

Queuing on qom-cpu-next with the following change:

--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -604,16 +604,14 @@ static inline void cpu_unaligned_access(CPUState
*cpu, vaddr addr,
  * @cpu: The CPU to set the program counter for.
  * @addr: Program counter value.
  *
- * Set the program counter for a CPU. If there is no available
implementation
- * no action occurs.
+ * Sets the program counter for a CPU.
  */
 static inline void cpu_set_pc(CPUState *cpu, vaddr addr)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);

-if (cc->set_pc) {
-cc->set_pc(cpu, addr);
-}
+g_assert(cc->set_pc != NULL);
+cc->set_pc(cpu, addr);
 }

 /**

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

[Qemu-devel] [PATCH v6 1/2] ich9: add TCO interface emulation

[Paulo Alcantara]

This interface provides some registers within a 32-byte range and can be
acessed through PCI-to-LPC bridge interface (PMBASE + 0x60).

It's commonly used as a watchdog timer to detect system lockups through
SMIs that are generated -- if TCO_EN bit is set -- on every timeout. If
NO_REBOOT bit is not set in GCS (General Control and Status register),
the system will be resetted upon second timeout if TCO_RLD register
wasn't previously written to prevent timeout.

This patch adds support to TCO watchdog logic and few other features
like mapping NMIs to SMIs (NMI2SMI_EN bit), system intruder detection,
etc. are not implemented yet.

Signed-off-by: Paulo Alcantara 
---
v1 -> v2:
  * add migration support for TCO I/O device state
  * wake up only when total time expired instead of every 0.6s
  * some cleanup suggested by Paolo Bonzini

v2 -> v3:
  * set SECOND_TO_STS and BOOT_STS bits in TCO2_STS instead
  * improve handling of TCO_LOCK bit in TCO1_CNT register

v3 -> v4:
  * fix some conflicts in hw/acpi/ich9.c after rebasing against master
  * remove meaningless "use_tco" field from TCOIORegs structure
  * add a object property named "enable_tco" and only enable TCO support
on pc-q35-2.4 and later

v4 -> v5:
  * remove unused field (use_tco) in TCOIORegs structure
  * move license to GPLv2+

v5 -> v6:
  * remove "io_tco" field from ICH9LPCPMRegs structure since it's no
longer used
  * set ICH9_CC_GCS_NO_REBOOT bit by default in ICH9's LPC
initialisation
---
 hw/acpi/Makefile.objs  |   2 +-
 hw/acpi/ich9.c |  55 ++-
 hw/acpi/tco.c  | 264 +
 hw/i386/pc_q35.c   |   4 +-
 hw/isa/lpc_ich9.c  |  16 ++-
 include/hw/acpi/ich9.h |   6 +-
 include/hw/acpi/tco.h  |  82 +++
 include/hw/boards.h|   3 +-
 include/hw/i386/ich9.h |  11 ++-
 include/hw/i386/pc.h   |   1 +
 10 files changed, 436 insertions(+), 8 deletions(-)
 create mode 100644 hw/acpi/tco.c
 create mode 100644 include/hw/acpi/tco.h

diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 29d46d8..3db1f07 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o ich9.o pcihp.o
+common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o ich9.o pcihp.o tco.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
 common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
 common-obj-$(CONFIG_ACPI) += acpi_interface.o
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 8a64ffb..d3d9953 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -30,6 +30,7 @@
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
 #include "hw/acpi/acpi.h"
+#include "hw/acpi/tco.h"
 #include "sysemu/kvm.h"
 #include "exec/address-spaces.h"
 
@@ -92,8 +93,16 @@ static void ich9_smi_writel(void *opaque, hwaddr addr, 
uint64_t val,
 unsigned width)
 {
 ICH9LPCPMRegs *pm = opaque;
+TCOIORegs *tr = &pm->tco_regs;
+uint64_t tco_en;
+
 switch (addr) {
 case 0:
+tco_en = pm->smi_en & ICH9_PMIO_SMI_EN_TCO_EN;
+/* once TCO_LOCK bit is set, TCO_EN bit cannot be overwritten */
+if (tr->tco.cnt1 & TCO_LOCK) {
+val = (val & ~ICH9_PMIO_SMI_EN_TCO_EN) | tco_en;
+}
 pm->smi_en &= ~pm->smi_en_wmask;
 pm->smi_en |= (val & pm->smi_en_wmask);
 break;
@@ -159,6 +168,25 @@ static const VMStateDescription vmstate_memhp_state = {
 }
 };
 
+static bool vmstate_test_use_tco(void *opaque)
+{
+ICH9LPCPMRegs *s = opaque;
+return s->enable_tco;
+}
+
+static const VMStateDescription vmstate_tco_io_state = {
+.name = "ich9_pm/tco",
+.version_id = 1,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.needed = vmstate_test_use_tco,
+.fields  = (VMStateField[]) {
+VMSTATE_STRUCT(tco_regs, ICH9LPCPMRegs, 1, vmstate_tco_io_sts,
+   TCOIORegs),
+VMSTATE_END_OF_LIST()
+}
+};
+
 const VMStateDescription vmstate_ich9_pm = {
 .name = "ich9_pm",
 .version_id = 1,
@@ -179,6 +207,10 @@ const VMStateDescription vmstate_ich9_pm = {
 .subsections = (const VMStateDescription*[]) {
 &vmstate_memhp_state,
 NULL
+},
+.subsections = (const VMStateDescription*[]) {
+&vmstate_tco_io_state,
+NULL
 }
 };
 
@@ -209,7 +241,7 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
 acpi_pm1_evt_power_down(&pm->acpi_regs);
 }
 
-void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
+void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, bool enable_tco,
   qemu_irq sci_irq)
 {
 memory_region_init(&pm->io, OBJECT(lpc_pci), "ich9-pm", ICH9_PMIO_SIZE);
@@ -231,6 +263,11 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
   "acpi-smi", 8);
 memory_region_add_subregion(&pm->io, ICH9_PMIO_SMI_EN, &pm->io_smi);
 
+pm->enable_tco = enable_tco;
+if (pm->ena

[Qemu-devel] [PATCH v6 2/2] tests: add testcase for TCO watchdog emulation

[Paulo Alcantara]

This patch adds a testcase that covers the following:
  1) TCO default values
  2) first and second TCO timeout
  3) watch and validate ticks counter through TCO_RLD register
  4) maximum supported TCO timeout (0x3ff)
  5) watchdog actions (pause/reset/shutdown/none) upon second TCO
 timeout
  6) set and get of TCO control and status bits

Signed-off-by: Paulo Alcantara 
---
v1 -> v2:
  * some cleanup
  * add test for TCO_LOCK bit

v2 -> v3:
  * add tests for TCO control & status bits
  * fix check of SECOND_TO_STS bit (it's set in TCO2_STS reg)

v3 -> v4:
  * add more description to commit log
  * use RCBA_BASE_ADDR macro defintion from hw/i386/ich9-cc.h instead

v4 -> v5:
  * use modified macros (now prefixed with ICH9_) from ich9-cc.h
  * move license to GPLv2+

v5 -> v6:
  * remove include of "hw/i386/ich9-cc.h" since it's no longer exist
---
 tests/Makefile   |   2 +
 tests/tco-test.c | 460 +++
 2 files changed, 462 insertions(+)
 create mode 100644 tests/tco-test.c

diff --git a/tests/Makefile b/tests/Makefile
index eff5e11..ef1e981 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -152,6 +152,7 @@ check-qtest-i386-y += tests/i440fx-test$(EXESUF)
 check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
 check-qtest-i386-y += tests/drive_del-test$(EXESUF)
 check-qtest-i386-y += tests/wdt_ib700-test$(EXESUF)
+check-qtest-i386-y += tests/tco-test$(EXESUF)
 gcov-files-i386-y += hw/watchdog/watchdog.c hw/watchdog/wdt_ib700.c
 check-qtest-i386-y += $(check-qtest-pci-y)
 gcov-files-i386-y += $(gcov-files-pci-y)
@@ -370,6 +371,7 @@ tests/eepro100-test$(EXESUF): tests/eepro100-test.o
 tests/vmxnet3-test$(EXESUF): tests/vmxnet3-test.o
 tests/ne2000-test$(EXESUF): tests/ne2000-test.o
 tests/wdt_ib700-test$(EXESUF): tests/wdt_ib700-test.o
+tests/tco-test$(EXESUF): tests/tco-test.o $(libqos-pc-obj-y)
 tests/virtio-balloon-test$(EXESUF): tests/virtio-balloon-test.o
 tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y)
 tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o $(libqos-pc-obj-y)
diff --git a/tests/tco-test.c b/tests/tco-test.c
new file mode 100644
index 000..1a2fe3d
--- /dev/null
+++ b/tests/tco-test.c
@@ -0,0 +1,460 @@
+/*
+ * QEMU ICH9 TCO emulation tests
+ *
+ * Copyright (c) 2015 Paulo Alcantara 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include 
+#include 
+#include 
+#include 
+
+#include "libqtest.h"
+#include "libqos/pci.h"
+#include "libqos/pci-pc.h"
+#include "hw/pci/pci_regs.h"
+#include "hw/i386/ich9.h"
+#include "hw/acpi/ich9.h"
+#include "hw/acpi/tco.h"
+
+#define RCBA_BASE_ADDR0xfed1c000
+#define PM_IO_BASE_ADDR   0xb000
+
+enum {
+TCO_RLD_DEFAULT = 0x,
+TCO_DAT_IN_DEFAULT  = 0x00,
+TCO_DAT_OUT_DEFAULT = 0x00,
+TCO1_STS_DEFAULT= 0x,
+TCO2_STS_DEFAULT= 0x,
+TCO1_CNT_DEFAULT= 0x,
+TCO2_CNT_DEFAULT= 0x0008,
+TCO_MESSAGE1_DEFAULT= 0x00,
+TCO_MESSAGE2_DEFAULT= 0x00,
+TCO_WDCNT_DEFAULT   = 0x00,
+TCO_TMR_DEFAULT = 0x0004,
+SW_IRQ_GEN_DEFAULT  = 0x03,
+};
+
+#define TCO_SECS_TO_TICKS(secs) (((secs) * 10) / 6)
+#define TCO_TICKS_TO_SECS(ticks)(((ticks) * 6) / 10)
+
+typedef struct {
+const char *args;
+QPCIDevice *dev;
+void *lpc_base;
+void *tco_io_base;
+} TestData;
+
+static void test_init(TestData *d)
+{
+QPCIBus *bus;
+QTestState *qs;
+char *s;
+
+s = g_strdup_printf("-machine q35 %s", !d->args ? "" : d->args);
+qs = qtest_start(s);
+qtest_irq_intercept_in(qs, "ioapic");
+g_free(s);
+
+bus = qpci_init_pc();
+d->dev = qpci_device_find(bus, QPCI_DEVFN(0x1f, 0x00));
+g_assert(d->dev != NULL);
+
+/* map PCI-to-LPC bridge interface BAR */
+d->lpc_base = qpci_iomap(d->dev, 0, NULL);
+
+qpci_device_enable(d->dev);
+
+g_assert(d->lpc_base != NULL);
+
+/* set ACPI PM I/O space base address */
+qpci_config_writel(d->dev, (uintptr_t)d->lpc_base + ICH9_LPC_PMBASE,
+   PM_IO_BASE_ADDR | 0x1);
+/* enable ACPI I/O */
+qpci_config_writeb(d->dev, (uintptr_t)d->lpc_base + ICH9_LPC_ACPI_CTRL,
+   0x80);
+/* set Root Complex BAR */
+qpci_config_writel(d->dev, (uintptr_t)d->lpc_base + ICH9_LPC_RCBA,
+   RCBA_BASE_ADDR | 0x1);
+
+d->tco_io_base = (void *)((uintptr_t)PM_IO_BASE_ADDR + 0x60);
+}
+
+static void stop_tco(const TestData *d)
+{
+uint32_t val;
+
+val = qpci_io_readw(d->dev, d->tco_io_base + TCO1_CNT);
+val |= TCO_TMR_HLT;
+qpci_io_writew(d->dev, d->tco_io_base + TCO1_CNT, val);
+}
+
+static void start_tco(const TestData *d)
+{
+uint32_t val;
+
+val = qpci_io_readw(d->dev, d->tco_io_base + TCO1_CNT);
+val &= ~TCO_TMR_HLT;
+qpci_io_writew(d->dev, d->tco_io_base + TCO1_CNT, val);
+}
+
+static

[Qemu-devel] vhost-user: current status of multiqueue support

[Nikita Kalyazin]

Hi,


What is the status of vhost-user multiqueue support in Qemu?

I am looking at 830d70db692e374b5f4407f96a1ceefdcc97 patch in master and 
observe the following:
-8<-
case VHOST_SET_VRING_KICK:
case VHOST_SET_VRING_CALL:
case VHOST_SET_VRING_ERR:
file = arg;
-   msg.u64 = file->index & VHOST_USER_VRING_IDX_MASK;
+   msg.u64 = (file->index + dev->vq_index) & VHOST_USER_VRING_IDX_MASK;
msg.size = sizeof(m.u64);
if (ioeventfd_enabled() && file->fd > 0) {
fds[fd_num++] = file->fd;
} else {
msg.u64 |= VHOST_USER_VRING_NOFD_MASK;
}
break;
->8-

What is the meaning of "file->index + dev->vq_index" expression?
Assuming "queues=4" parameter is passed at Qemu command line and "file->index" 
is either 0 or 1, Vhost receives the following sequence of VHOST_SET_VRING_CALL 
requests:
 - msg.u64 = 0 (file->index = 0, dev->vq_index = 0)
 - msg.u64 = 1 (file->index = 1, dev->vq_index = 0)
 - msg.u64 = 1 (file->index = 0, dev->vq_index = 1)
 - msg.u64 = 2 (file->index = 1, dev->vq_index = 1)
 - msg.u64 = 2 (file->index = 0, dev->vq_index = 2)
 - msg.u64 = 3 (file->index = 1, dev->vq_index = 2)
 - msg.u64 = 3 (file->index = 0, dev->vq_index = 3)
 - msg.u64 = 4 (file->index = 1, dev->vq_index = 3)
This means, CALLs for virtqueues 0-4 only would be configured (whereas 
virtqueues 5-7 remain unconfigured).

Has it been done intentionally?

-- 

Best regards,

Nikita Kalyazin,
n.kalya...@samsung.com

Software Engineer
CE OS Group
Samsung R&D Institute Russia
Tel: +7 (495) 797-25-00 #3816
Tel: +7 (495) 797-25-03
Office #1501, 12-1, Dvintsev str.,
Moscow, 127018, Russia

[Qemu-devel] vhost-user: current status of multiqueue support

[Nikita Kalyazin]

Hi,


What is the status of vhost-user multiqueue support in Qemu?

I am looking at 830d70db692e374b5f4407f96a1ceefdcc97 patch in master and 
observe the following:
-8<-
case VHOST_SET_VRING_KICK:
case VHOST_SET_VRING_CALL:
case VHOST_SET_VRING_ERR:
file = arg;
-   msg.u64 = file->index & VHOST_USER_VRING_IDX_MASK;
+   msg.u64 = (file->index + dev->vq_index) & VHOST_USER_VRING_IDX_MASK;
msg.size = sizeof(m.u64);
if (ioeventfd_enabled() && file->fd > 0) {
fds[fd_num++] = file->fd;
} else {
msg.u64 |= VHOST_USER_VRING_NOFD_MASK;
}
break;
->8-

What is the meaning of "file->index + dev->vq_index" expression?
Assuming "queues=4" parameter is passed at Qemu command line and "file->index" 
is either 0 or 1, Vhost receives the following sequence of VHOST_SET_VRING_CALL 
requests:
 - msg.u64 = 0 (file->index = 0, dev->vq_index = 0)
 - msg.u64 = 1 (file->index = 1, dev->vq_index = 0)
 - msg.u64 = 1 (file->index = 0, dev->vq_index = 1)
 - msg.u64 = 2 (file->index = 1, dev->vq_index = 1)
 - msg.u64 = 2 (file->index = 0, dev->vq_index = 2)
 - msg.u64 = 3 (file->index = 1, dev->vq_index = 2)
 - msg.u64 = 3 (file->index = 0, dev->vq_index = 3)
 - msg.u64 = 4 (file->index = 1, dev->vq_index = 3)
This means, CALLs for virtqueues 0-4 only would be configured (whereas 
virtqueues 5-7 remain unconfigured).

Has it been done intentionally?

-- 

Best regards,

Nikita Kalyazin,
n.kalya...@samsung.com

Software Engineer
CE OS Group
Samsung R&D Institute Russia
Tel: +7 (495) 797-25-00 #3816
Tel: +7 (495) 797-25-03
Office #1501, 12-1, Dvintsev str.,
Moscow, 127018, Russia

Re: [Qemu-devel] [PATCH qom v3 4/4] microblaze: boot: Use cpu_set_pc()

[Andreas Färber]

Am 24.06.2015 um 05:19 schrieb Peter Crosthwaite:
> Use cpu_set_pc() for setting program counters when bootloading. This
> removes an instance of system level code having to reach into the CPU
> env.
> 
> Reviewed-by: Andreas Färber 
> Signed-off-by: Peter Crosthwaite 
> ---
> changed since v2:
> Add () to function names in commit messages
> ---
>  dtc  | 2 +-
>  hw/microblaze/boot.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/dtc b/dtc
> index 65cc4d2..bc895d6 16
> --- a/dtc
> +++ b/dtc
> @@ -1 +1 @@
> -Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
> +Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde

Submodule strikes again. Preparing to queue.

Andreas

> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> index 4c44317..9f4698a 100644
> --- a/hw/microblaze/boot.c
> +++ b/hw/microblaze/boot.c
> @@ -54,7 +54,7 @@ static void main_cpu_reset(void *opaque)
>  env->regs[5] = boot_info.cmdline;
>  env->regs[6] = boot_info.initrd_start;
>  env->regs[7] = boot_info.fdt;
> -env->sregs[SR_PC] = boot_info.bootstrap_pc;
> +cpu_set_pc(CPU(cpu), boot_info.bootstrap_pc);
>  if (boot_info.machine_cpu_reset) {
>  boot_info.machine_cpu_reset(cpu);
>  }
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH v3 8/8] cpu-exec: Purge all uses of ENV_GET_CPU()

[Peter Crosthwaite]

On Wed, Jun 24, 2015 at 10:32 AM, Andreas Färber  wrote:
> Am 24.06.2015 um 04:10 schrieb Peter Crosthwaite:
>> On Thu, Jun 18, 2015 at 10:24 AM, Peter Crosthwaite
>>  wrote:
>>> Remove un-needed usages of ENV_GET_CPU() by converting the APIs to use
>>> CPUState pointers and retrieving the env_ptr as minimally needed.
>>>
>>> Scripted conversion for target-* change:
>>>
>>> for I in target-*/cpu.h; do
>>> sed -i \
>>> 's/\(^int cpu_[^_]*_exec(\)[^ ][^ ]* \*s);$/\1CPUState *cpu);/' \
>>> $I;
>>> done
>>>
>>> Signed-off-by: Peter Crosthwaite 
>>
>> Dropping this patch in v4 as no RBs yet.
>
> On a brief look this looks good to me, queued on qom-cpu-next for now.
>
> One comment inline.
>
> How good do we look after this? I spot 61 uses, with one bad one in
> target-arm/helper.c.

We are good. With the multi-arch series make changes, all remaining
ENV_GET_CPU uses are in arch-obj multi-compiled common code, or user
mode code. This series is a last of these ENV_GET_CPU patches (unless
more get added).

Regards,
Peter

Most of them in linux-user and softmmu headers, one
> in cputlb.c which we had previously discussed with Paolo to be a
> non-issue for multi-arch.
>
>>> ---
>>> Changed since v2 (Aurelien review):
>>> s/CPU_GET_ENV/ENV_GET_CPU/
>>> Changed since RFC v2 (RTH review):
>>> Apply target-* change pattern to all arches.
>>> Avoid use of cpu_ptr for X86 specifics
>>> Add () to ENV_GET_CPU macros in commit message
>>> Add BSD and Linux user needed changes
>>> Add missing architecture changes
>>> ---
>>>  bsd-user/main.c |  3 ++-
>>>  cpu-exec.c  | 28 +---
>>>  cpus.c  |  3 +--
>>>  linux-user/main.c   | 28 ++--
>>>  target-alpha/cpu.h  |  2 +-
>>>  target-arm/cpu.h|  2 +-
>>>  target-cris/cpu.h   |  2 +-
>>>  target-i386/cpu.h   |  2 +-
>>>  target-lm32/cpu.h   |  2 +-
>>>  target-m68k/cpu.h   |  2 +-
>>>  target-microblaze/cpu.h |  2 +-
>>>  target-mips/cpu.h   |  2 +-
>>>  target-moxie/cpu.h  |  2 +-
>>>  target-openrisc/cpu.h   |  2 +-
>>>  target-ppc/cpu.h|  2 +-
>>>  target-s390x/cpu.h  |  2 +-
>>>  target-sh4/cpu.h|  2 +-
>>>  target-sparc/cpu.h  |  2 +-
>>>  target-tricore/cpu.h|  2 +-
>>>  target-unicore32/cpu.h  |  3 ++-
>>>  target-xtensa/cpu.h |  2 +-
>>>  21 files changed, 48 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/bsd-user/main.c b/bsd-user/main.c
>>> index 45a1436..7196285 100644
>>> --- a/bsd-user/main.c
>>> +++ b/bsd-user/main.c
>>> @@ -166,6 +166,7 @@ static void set_idt(int n, unsigned int dpl)
>>>
>>>  void cpu_loop(CPUX86State *env)
>>>  {
>>> +CPUState *cs = CPU(x86_env_get_cpu(env));
>
> An (unwritten?) convention has been to avoid double-casts by having an
> explicit X86CPU *cpu variable. Will re-review the preceding patches for
> the same nit.
>
> Regards,
> Andreas
>
>>>  int trapnr;
>>>  abi_ulong pc;
>>>  //target_siginfo_t info;
>>> @@ -512,7 +513,7 @@ void cpu_loop(CPUSPARCState *env)
>>>  //target_siginfo_t info;
>>>
>>>  while (1) {
>>> -trapnr = cpu_sparc_exec (env);
>>> +trapnr = cpu_sparc_exec(cs);
>>>
>>>  switch (trapnr) {
>>>  #ifndef TARGET_SPARC64
> [snip]
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
>

Re: [Qemu-devel] [RFC 4/9] block: keep bitmap if incremental backup job is cancelled

[Max Reitz]

On 12.06.2015 12:09, Stefan Hajnoczi wrote:

Reclaim the dirty bitmap if an incremental backup block job is
cancelled.  The ret variable may be 0 when the job is cancelled so it's
not enough to check ret < 0.

Signed-off-by: Stefan Hajnoczi 
---
  block/backup.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/backup.c b/block/backup.c
index d3f648d..c1ad975 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -430,7 +430,7 @@ static void coroutine_fn backup_run(void *opaque)
  
  if (job->sync_bitmap) {

  BdrvDirtyBitmap *bm;
-if (ret < 0) {
+if (ret < 0 || block_job_is_cancelled(&job->common)) {
  /* Merge the successor back into the parent, delete nothing. */
  bm = bdrv_reclaim_dirty_bitmap(bs, job->sync_bitmap, NULL);
  assert(bm);


Since I can't find a seperate patch on qemu-devel or qemu-block yet:

Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v3 8/8] cpu-exec: Purge all uses of ENV_GET_CPU()

[Andreas Färber]

Am 24.06.2015 um 04:10 schrieb Peter Crosthwaite:
> On Thu, Jun 18, 2015 at 10:24 AM, Peter Crosthwaite
>  wrote:
>> Remove un-needed usages of ENV_GET_CPU() by converting the APIs to use
>> CPUState pointers and retrieving the env_ptr as minimally needed.
>>
>> Scripted conversion for target-* change:
>>
>> for I in target-*/cpu.h; do
>> sed -i \
>> 's/\(^int cpu_[^_]*_exec(\)[^ ][^ ]* \*s);$/\1CPUState *cpu);/' \
>> $I;
>> done
>>
>> Signed-off-by: Peter Crosthwaite 
> 
> Dropping this patch in v4 as no RBs yet.

On a brief look this looks good to me, queued on qom-cpu-next for now.

One comment inline.

How good do we look after this? I spot 61 uses, with one bad one in
target-arm/helper.c. Most of them in linux-user and softmmu headers, one
in cputlb.c which we had previously discussed with Paolo to be a
non-issue for multi-arch.

>> ---
>> Changed since v2 (Aurelien review):
>> s/CPU_GET_ENV/ENV_GET_CPU/
>> Changed since RFC v2 (RTH review):
>> Apply target-* change pattern to all arches.
>> Avoid use of cpu_ptr for X86 specifics
>> Add () to ENV_GET_CPU macros in commit message
>> Add BSD and Linux user needed changes
>> Add missing architecture changes
>> ---
>>  bsd-user/main.c |  3 ++-
>>  cpu-exec.c  | 28 +---
>>  cpus.c  |  3 +--
>>  linux-user/main.c   | 28 ++--
>>  target-alpha/cpu.h  |  2 +-
>>  target-arm/cpu.h|  2 +-
>>  target-cris/cpu.h   |  2 +-
>>  target-i386/cpu.h   |  2 +-
>>  target-lm32/cpu.h   |  2 +-
>>  target-m68k/cpu.h   |  2 +-
>>  target-microblaze/cpu.h |  2 +-
>>  target-mips/cpu.h   |  2 +-
>>  target-moxie/cpu.h  |  2 +-
>>  target-openrisc/cpu.h   |  2 +-
>>  target-ppc/cpu.h|  2 +-
>>  target-s390x/cpu.h  |  2 +-
>>  target-sh4/cpu.h|  2 +-
>>  target-sparc/cpu.h  |  2 +-
>>  target-tricore/cpu.h|  2 +-
>>  target-unicore32/cpu.h  |  3 ++-
>>  target-xtensa/cpu.h |  2 +-
>>  21 files changed, 48 insertions(+), 49 deletions(-)
>>
>> diff --git a/bsd-user/main.c b/bsd-user/main.c
>> index 45a1436..7196285 100644
>> --- a/bsd-user/main.c
>> +++ b/bsd-user/main.c
>> @@ -166,6 +166,7 @@ static void set_idt(int n, unsigned int dpl)
>>
>>  void cpu_loop(CPUX86State *env)
>>  {
>> +CPUState *cs = CPU(x86_env_get_cpu(env));

An (unwritten?) convention has been to avoid double-casts by having an
explicit X86CPU *cpu variable. Will re-review the preceding patches for
the same nit.

Regards,
Andreas

>>  int trapnr;
>>  abi_ulong pc;
>>  //target_siginfo_t info;
>> @@ -512,7 +513,7 @@ void cpu_loop(CPUSPARCState *env)
>>  //target_siginfo_t info;
>>
>>  while (1) {
>> -trapnr = cpu_sparc_exec (env);
>> +trapnr = cpu_sparc_exec(cs);
>>
>>  switch (trapnr) {
>>  #ifndef TARGET_SPARC64
[snip]

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH qom v4 4/7] translate-all: Change tb_flush() env argument to cpu

[Peter Crosthwaite]

On Wed, Jun 24, 2015 at 10:23 AM, Andreas Färber  wrote:
> Am 24.06.2015 um 19:06 schrieb Peter Crosthwaite:
>> On Wed, Jun 24, 2015 at 8:30 AM, Andreas Färber  wrote:
>>> Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:
 diff --git a/dtc b/dtc
 index 65cc4d2..bc895d6 16
 --- a/dtc
 +++ b/dtc
 @@ -1 +1 @@
 -Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
 +Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde
>>>
>>> This looks accidental.
>>>
>>
>> Doh :( It is. Do you need a respin?
>
> No, thanks. Each respin requires me to re-review, so I just deleted the
> patch lines from the email before applying.
>

Thanks. Appreciated.

Regards,
Peter

> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
>

Re: [Qemu-devel] [PATCH v2 2/4] gdbstub: Use cpu_set_pc helper

[Peter Crosthwaite]

On Wed, Jun 24, 2015 at 10:16 AM, Andreas Färber  wrote:
> Am 24.06.2015 um 19:04 schrieb Peter Crosthwaite:
>> On Wed, Jun 24, 2015 at 3:01 AM, Peter Maydell  
>> wrote:
>>> On 24 June 2015 at 03:50, Peter Crosthwaite
>>>  wrote:
 On Mon, Jun 22, 2015 at 10:31 AM, Andreas Färber  wrote:
> I believe this argument will probably go away; otherwise this should've
> been &error_abort or something instead of NULL.
>

 I'm not sure. As I don't see what is catching the case of a gdb 'c'
 packet for a CPU that doesn't implement set_pc. I'd rather preserve
 the existing behaviour, and have the qom wrapper do nothing if it is
 not implemented.
>>>
>>> Well, this is one reason why every CPU needs to implement set_pc...
>>>
>>
>> Well. I guess it works for a common case where a continue doesn't
>> change the PC? If the debugger doesn't change the PC the "c" should
>> work even without a set_pc call so we don't want to assert on this
>> valid use case.
>
> Guys, is there any target that does not implement set_pc today? If so,
> which? I'd rather implement it than carry around the iffery and
> resulting complications.
>
> I quickly counted 17 target-* in my tree and all 17 seemed to show up in
> git-grep. Can you confirm? Didn't check the latest tilegx series.
>

There are 17 targets and 18 sets by my count:
$ git grep "set_pc.*=.*set_pc" target-*
target-alpha/cpu.c:cc->set_pc = alpha_cpu_set_pc;
target-arm/cpu.c:cc->set_pc = arm_cpu_set_pc;
target-arm/cpu64.c:cc->set_pc = aarch64_cpu_set_pc;
target-cris/cpu.c:cc->set_pc = cris_cpu_set_pc;
target-i386/cpu.c:cc->set_pc = x86_cpu_set_pc;
target-lm32/cpu.c:cc->set_pc = lm32_cpu_set_pc;
target-m68k/cpu.c:cc->set_pc = m68k_cpu_set_pc;
target-microblaze/cpu.c:cc->set_pc = mb_cpu_set_pc;
target-mips/cpu.c:cc->set_pc = mips_cpu_set_pc;
target-moxie/cpu.c:cc->set_pc = moxie_cpu_set_pc;
target-openrisc/cpu.c:cc->set_pc = openrisc_cpu_set_pc;
target-ppc/translate_init.c:cc->set_pc = ppc_cpu_set_pc;
target-s390x/cpu.c:cc->set_pc = s390_cpu_set_pc;
target-sh4/cpu.c:cc->set_pc = superh_cpu_set_pc;
target-sparc/cpu.c:cc->set_pc = sparc_cpu_set_pc;
target-tricore/cpu.c:cc->set_pc = tricore_cpu_set_pc;
target-unicore32/cpu.c:cc->set_pc = uc32_cpu_set_pc;
target-xtensa/cpu.c:cc->set_pc = xtensa_cpu_set_pc;

ARM is doubled up. As long as no one else is missing a double up where
there is not default set by a single base class we are OK. If there is
a missing double up with a base class implementation, then there will
be no change by your proposal anyway.

Regards,
Peter

> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
>

Re: [Qemu-devel] [PATCH 0/3] update CMOS for ISA-FDC with iobase=0x3f0

[John Snow]

On 06/23/2015 12:58 PM, Laszlo Ersek wrote:
> This is (again) for the other pc-q35-2.4 ISA-FDC problem reported by
> Jan. Addressing comments from Markus.
> 
> Jan, can you give it another try please? I realize this is getting old
> pretty quick, so don't bother if you don't want to.
> 
> Cc: Jan Tomko 
> Cc: John Snow 
> Cc: Markus Armbruster 
> Cc: Paolo Bonzini 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (3):
>   hw/i386/pc: factor out pc_cmos_init_floppy()
>   hw/i386/pc: reflect any FDC @ ioport 0x3f0 in the CMOS
>   hw/i386/pc: don't carry FDC from pc_basic_device_init() to
> pc_cmos_init()
> 
>  include/hw/i386/pc.h |   3 +-
>  hw/i386/pc.c | 129 
> ++-
>  hw/i386/pc_piix.c|   5 +-
>  hw/i386/pc_q35.c |   5 +-
>  4 files changed, 101 insertions(+), 41 deletions(-)
> 

Reviewed-by: John Snow 

Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)

[Paolo Bonzini]

On 24/06/2015 19:18, Alex Bennée wrote:
>>> >> @@ -196,6 +196,8 @@ void arm_handle_psci_call(ARMCPU *cpu)
>>> >>  }
>>> >>  target_cpu_class->set_pc(target_cpu_state, entry);
>>> >>  
>>> >> +qemu_cond_signal(target_cpu_state->halt_cond);
>> >
>> > That's called qemu_cpu_kick(target_cpu_state). :)  The patch should be
>> > acceptable now upstream, I think.
> Oh so this might well fail in KVM too?
> 
> The qemu_cpu_kick does a qemu_cond_broadcast(cpu->halt_cond) which seems
> a little excessive? Won't all sleeping CPUs wake up (and return to sleep)?

On KVM (and I assume on MT-TCG), each CPU has a different halt_cond.

Paolo

Re: [Qemu-devel] [PATCH qom v4 4/7] translate-all: Change tb_flush() env argument to cpu

[Andreas Färber]

Am 24.06.2015 um 19:06 schrieb Peter Crosthwaite:
> On Wed, Jun 24, 2015 at 8:30 AM, Andreas Färber  wrote:
>> Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:
>>> diff --git a/dtc b/dtc
>>> index 65cc4d2..bc895d6 16
>>> --- a/dtc
>>> +++ b/dtc
>>> @@ -1 +1 @@
>>> -Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
>>> +Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde
>>
>> This looks accidental.
>>
> 
> Doh :( It is. Do you need a respin?

No, thanks. Each respin requires me to re-review, so I just deleted the
patch lines from the email before applying.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 5/9] memory: let address_space_rw/ld*/st* run outside the BQL

[Paolo Bonzini]

On 24/06/2015 18:56, Alex Bennée wrote:
> This is where I get confused between the advantage of this over however
> same pid recursive locking. If you use recursive locking you don't need
> to add a bunch of state to remind you of when to release the lock. Then
> you'd just need:
> 
> static void prepare_mmio_access(MemoryRegion *mr)
> {
> if (mr->global_locking) {
> qemu_mutex_lock_iothread();
> }
> if (mr->flush_coalesced_mmio) {
> qemu_mutex_lock_iothread();
> qemu_flush_coalesced_mmio_buffer();
> qemu_mutex_unlock_iothread();
> }
> }
> 
> and a bunch of:
> 
> if (mr->global_locking)
>qemu_mutex_unlock_iothread();
> 
> in the access functions. Although I suspect you could push the
> mr->global_locking up to the dispatch functions.
> 
> Am I missing something here?

The semantics of recursive locking with respect to condition variables
are not clear.  Either cond_wait releases all locks, and then the mutex
can be released when the code doesn't expect it to be, or cond_wait
doesn't release all locks and then you have deadlocks.

POSIX says to do the latter:

It is advised that an application should not use a
PTHREAD_MUTEX_RECURSIVE mutex with condition variables because
the implicit unlock performed for a pthread_cond_timedwait() or
pthread_cond_wait() may not actually release the mutex (if it
had been locked multiple times). If this happens, no other
thread can satisfy the condition of the predicate."

So, recursive locking is okay if you don't have condition variables
attached to the lock (and if you cannot do without it), but
qemu_global_mutex does have them.

QEMU has so far tried to use the solution that Stevens outlines here:
https://books.google.it/books?id=kCTMFpEcIOwC&pg=PA434

... Leave the interfaces to func1 and func2 unchanged, and avoid
a recursive mutex by providing a private version of func2,
called func2_locked.  To call func2_locked, hold the mutex
embedded in the data structure whose address we pass as the
argument.

as a way to avoid recursive locking.  This is much better because a) it
is more efficient---taking locks can be expensive even if they're
uncontended, especially if your VM spans multiple NUMA nodes on the
host; b) it is always clear when a lock is taken and when it isn't.

Note that Stevens has another example right after this one of recursive
locking, involving callbacks, but it's ill-defined.  There's no reason
for the "timeout" function in page 437 to hold the mutex when it calls
"func".  It can unlock before and re-lock afterwards, like QEMU's own
timerlist_run_timers function.

Paolo

Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)

[Alex Bennée]

Paolo Bonzini  writes:

> On 24/06/2015 17:34, Alex Bennée wrote:
>> Testing with Alexander's bare metal syncronisation tests fails in MTTCG
>> leaving one CPU spinning forever waiting for the second CPU to wake up.
>> We simply need to poke the halt_cond once we have processed the PSCI
>> power on call.
>> 
>> Tested-by: Alex Bennée 
>> CC: Alexander Spyridakis 
>> 
>> ---
>> TODO
>>   - exactly how does the vexpress wake up it's sleeping CPUs?
>> ---
>>  target-arm/psci.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/target-arm/psci.c b/target-arm/psci.c
>> index d8fafab..661ff28 100644
>> --- a/target-arm/psci.c
>> +++ b/target-arm/psci.c
>> @@ -196,6 +196,8 @@ void arm_handle_psci_call(ARMCPU *cpu)
>>  }
>>  target_cpu_class->set_pc(target_cpu_state, entry);
>>  
>> +qemu_cond_signal(target_cpu_state->halt_cond);
>
> That's called qemu_cpu_kick(target_cpu_state). :)  The patch should be
> acceptable now upstream, I think.

Oh so this might well fail in KVM too?

The qemu_cpu_kick does a qemu_cond_broadcast(cpu->halt_cond) which seems
a little excessive? Won't all sleeping CPUs wake up (and return to sleep)?

>
> Paolo
>
>>  ret = 0;
>>  break;
>>  case QEMU_PSCI_0_1_FN_CPU_OFF:
>> 

-- 
Alex Bennée