Re: [Qemu-devel] [PATCH 20/20] block: Remove bdrv_(set_)enable_write_cache()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> The only remaining users were block jobs (mirror and backup) which
> unconditionally enabled WCE on the BlockBackend of the target image. As
> these block jobs don't go through BlockBackend for their I/O requests,
> they aren't affected by this setting anyway but always get a writeback
> mode, so that call can be removed.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 16 
>  block/backup.c|  1 -
>  block/mirror.c|  1 -
>  include/block/block.h |  2 --
>  4 files changed, 20 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 19/20] block: Remove BDRV_O_CACHE_WB

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> The previous patches have successively made blk->enable_write_cache the
> true source for the information whether a writethrough mode must be
> implemented. The corresponding BDRV_O_CACHE_WB is only useless baggage
> we're carrying around, so now's the time to remove it.
> 
> At the same time, we remove the 'cache.writeback' option parsing on the
> BDS level as the only effect was setting the BDRV_O_CACHE_WB flag.
> 
> This change requires test cases that explicitly enabled the option to
> drop it. Other than that and the change of the error message when
> writethrough is enabled on the BDS level (from "Can't set writethrough
> mode" to "doesn't support the option"), there should be no change in
> behaviour.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 48 
> ++-
>  block/block-backend.c | 11 --
>  block/vvfat.c |  3 +--
>  blockdev.c| 21 ++-
>  include/block/block.h |  3 +--
>  qemu-img.c|  2 +-
>  qemu-io-cmds.c|  1 -
>  tests/qemu-iotests/051|  2 +-
>  tests/qemu-iotests/051.pc.out | 10 -
>  tests/qemu-iotests/142|  6 +++---
>  tests/qemu-iotests/142.out| 36 
>  11 files changed, 34 insertions(+), 109 deletions(-)
> 

[...]

> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 7de3754..35ee50b 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -2149,7 +2149,6 @@ static int reopen_f(BlockBackend *blk, int argc, char 
> **argv)
>  opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
>  qemu_opts_reset(_opts);
>  
> -flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;

Well... :-P

>  brq = bdrv_reopen_queue(NULL, bs, opts, flags);
>  bdrv_reopen_multiple(brq, _err);
>  if (local_err) {
> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
> index 7bfe9ff..88b3d91 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -218,7 +218,7 @@ run_qemu -drive driver=null-co,cache=invalid_value
>  
>  for cache in writeback writethrough unsafe invalid_value; do
>  echo -e "info block\ninfo block file\ninfo block backing\ninfo block 
> backing-file" | \
> -run_qemu -drive 
> file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id
>  -nodefaults
> +run_qemu -drive 
> file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id
>  -nodefaults
>  done
>  
>  echo
> diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
> index 73cc15a..ec6d222 100644
> --- a/tests/qemu-iotests/051.pc.out
> +++ b/tests/qemu-iotests/051.pc.out

051.out needs the same changes.

Aside from that: Looks good.

Max

> @@ -239,7 +239,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  Testing: -drive driver=null-co,cache=invalid_value
>  QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
>  
> -Testing: -drive 
> file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
>  -nodefaults
> +Testing: -drive 
> file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
>  -nodefaults
>  QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) iininfinfoinfo 
> info binfo 
> blinfo bloinfo 
> blocinfo block
>  drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
> @@ -259,7 +259,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
>  Cache mode:   writeback, ignore flushes
>  (qemu) qququiquit
>  
> -Testing: -drive 
> file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
>  -nodefaults
> +Testing: -drive 
> file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
>  -nodefaults
>  QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) 

Re: [Qemu-devel] [PATCH 18/20] block: Remove bdrv_parse_cache_flags()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> All users are converted to bdrv_parse_cache_mode() now.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 29 +++--
>  include/block/block.h |  1 -
>  2 files changed, 7 insertions(+), 23 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> We must forbid changing the WCE flag in bdrv_reopen() in the same patch,
> as otherwise the behaviour would change so that the flag takes
> precedence over the explicitly specified option.
> 
> The correct value of the WCE flag depends on the BlockBackend user (e.g.
> guest device) and isn't a decision that the QMP client makes, so this
> change is what we want.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c| 18 ++
>  qemu-io-cmds.c | 14 +-
>  tests/qemu-iotests/142 |  2 +-
>  tests/qemu-iotests/142.out |  2 +-
>  4 files changed, 21 insertions(+), 15 deletions(-)
> 

[...]

> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index e929d24..7de3754 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c

[...]

> @@ -2136,14 +2137,25 @@ static int reopen_f(BlockBackend *blk, int argc, char 
> **argv)
>  return qemuio_command_usage(_cmd);
>  }
>  
> +if (writethrough != blk_enable_write_cache(blk) &&
> +blk_get_attached_dev(blk))
> +{
> +error_report("Cannot change cache.writeback: Device attached");
> +qemu_opts_reset(_opts);
> +return 0;
> +}
> +
>  qopts = qemu_opts_find(_opts, NULL);
>  opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
>  qemu_opts_reset(_opts);
>  
> +flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;

Shouldn't this be bdrv_enable_write_cache(bs)?

With blk_enable_write_cache(blk), reopening a non-WB BB should always
fail because bdrv_reopen_multiple() accuses us of trying to change the
WB mode (whereas we really don't want to change the BDS's mode).

Max

>  brq = bdrv_reopen_queue(NULL, bs, opts, flags);
>  bdrv_reopen_multiple(brq, _err);
>  if (local_err) {
>  error_report_err(local_err);
> +} else {
> +blk_set_enable_write_cache(blk, !writethrough);
>  }
>  
>  return 0;



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 16/20] block: Use bdrv_parse_cache_mode() in drive_init()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  blockdev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 15/20] raw: Support BDRV_REQ_FUA

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Pass through the FUA flag to the lower layer so that the separate flush
> can be saved in practically relevant cases where a (raw) format driver
> sits on top of the protocol driver.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/raw_bsd.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)

Looks good, modulo the bdrv_co_writev() issue.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 14/20] nbd: Support BDRV_REQ_FUA

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> The NBD server already used to send a FUA flag when the writethrough
> mode was set. This code was a remnant from the times where protocol
> drivers actually had to implement writethrough modes. Since nowadays the
> block layer sends flushes in writethrough mode and non-root nodes are
> always writeback, this was mostly dead code - only mostly because if NBD
> was configured to be used without a format, we sent _both_ FUA and an
> explicit flush afterwards, which makes the code not technically dead,
> but useless overhead.
> 
> This patch changes the code so that the block layer's FUA flag is
> recognised and translated into a NBD FUA flag. The additional flush is
> avoided now.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/nbd-client.c | 13 +++--
>  block/nbd-client.h |  2 +-
>  block/nbd.c| 26 +-
>  3 files changed, 29 insertions(+), 12 deletions(-)

Looks good, but I have the same issue as with patch 13.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> This replaces the existing hack in the iscsi driver that sent the FUA
> bit in writethrough mode and ignored the following flush in order to
> optimise the number of roundtrips (see commit 73b5394e).
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/iscsi.c | 24 +++-
>  1 file changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 3b54536..4f75204 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c

[...]

> @@ -1851,7 +1840,8 @@ static BlockDriver bdrv_iscsi = {
>  .bdrv_co_discard  = iscsi_co_discard,
>  .bdrv_co_write_zeroes = iscsi_co_write_zeroes,
>  .bdrv_co_readv = iscsi_co_readv,
> -.bdrv_co_writev= iscsi_co_writev,
> +.bdrv_co_writev_flags  = iscsi_co_writev_flags,
> +.supported_write_flags = BDRV_REQ_FUA,
>  .bdrv_co_flush_to_disk = iscsi_co_flush,
>  
>  #ifdef __linux__
> 

Hm, wait, maybe not R-b. I can see three places in block/io.c which call
bdrv_co_writev(), and only one of them diverts to bdrv_co_writev_flags()
if that is available. Maybe we don't need to care about the
bounce-buffer case for write_zeroes, but I do think we need to care
about the COR case.

Of course bdrv_co_writev() can trivially be forwarded to
bdrv_co_writev_flags(), but I'm not sure who is supposed to do this
forwarding. I can imagine three ways:

(1) Keep a wrapper per block driver. Simple, but not so elegant.
(2) Make all bdrv_co_writev() callers call bdrv_co_writev_flags() if
the former is not available but the latter is.
(3) Introduce a generic function replacing every drv->bdrv_co_writev()
call which then decides which driver function to invoke.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> This replaces the existing hack in the iscsi driver that sent the FUA
> bit in writethrough mode and ignored the following flush in order to
> optimise the number of roundtrips (see commit 73b5394e).
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/iscsi.c | 24 +++-
>  1 file changed, 7 insertions(+), 17 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 12/20] block: Introduce bdrv_co_writev_flags()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> This function will allow drivers to implement BDRV_REQ_FUA natively
> instead of sending a separate flush after the write.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/io.c| 9 -
>  include/block/block_int.h | 5 +
>  2 files changed, 13 insertions(+), 1 deletion(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 11/20] block/qapi: Use blk_enable_write_cache()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Now that WCE is handled on the BlockBackend level, the flag is
> meaningless for BDSes. As the schema requires us to fill the field,
> we return an enabled write cache for them.
> 
> Note that this means that querying the BlockBackend name may return
> writethrough as the cache information, whereas querying the node-name of
> the root of that same BlockBackend will return writeback.
> 
> This may appear odd at first,

Yeah, intuitively I'd think that a BDS shares the writeback mode of the
BB at the tree root, but...

>   but it actually makes sense because it
> correctly repesents the layer that implements the WCE handling.

...when you actually access the BDS directly, i.e., not through the
whole tree and the BB at its root, then you will indeed get WB behavior.

> This
> becomes more apparent when you consider nodes that are the root node of
> multiple BlockBackends, where each BB can have its own WCE setting.

True.

> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c|  2 +-
>  block/qapi.c   |  7 +++---
>  include/block/qapi.h   |  3 ++-
>  tests/qemu-iotests/142 |  7 +-
>  tests/qemu-iotests/142.out | 57 
> ++
>  5 files changed, 60 insertions(+), 16 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 10/20] block: Move enable_write_cache to BB level

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Whether a write cache is used or not is a decision that concerns the
> user (e.g. the guest device) rather than the backend. It was already
> logically part of the BB level as bdrv_move_feature_fields() always kept
> it on top of the BDS tree; with this patch, the core of it (the actual
> flag and the additional flushes) is also implemented there.
> 
> Direct callers of bdrv_open() must pass BDRV_O_CACHE_WB now if bs
> doesn't have a BlockBackend attached.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c| 26 +-
>  block/block-backend.c  | 42 +++---
>  block/io.c |  2 +-
>  block/iscsi.c  |  2 +-
>  include/block/block.h  |  1 +
>  include/block/block_int.h  |  3 ---
>  tests/qemu-iotests/142 |  4 ++--
>  tests/qemu-iotests/142.out |  8 
>  8 files changed, 53 insertions(+), 35 deletions(-)

Reviewed-by: Max Reitz 

I'm not so sure about the state bdrv_{set_,}enable_write_cache() are in
after this patch (e.g. the NBD client will always think the write cache
is enabled; and bdrv_set_enable_write_cache() can be used to unset
BDRV_O_CACHE_WB on BDSs), but looking at the following patches' titles,
they'll clear that up.

It appears to me that multiwrite will ignore the writethrough status,
but then again, qemu-io seems to be the only multiwrite user.

> diff --git a/block.c b/block.c
> index 172f865..9271dbb 100644
> --- a/block.c
> +++ b/block.c

[...]

> @@ -3618,8 +3626,8 @@ void bdrv_img_create(const char *filename, const char 
> *fmt,
>  }
>  
>  /* backing files always opened read-only */
> -back_flags =
> -flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
> +back_flags = flags | BDRV_O_CACHE_WB;
> +back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | 
> BDRV_O_NO_BACKING);

Actually, this is the only thing the @flags parameter of this function
is used for. Maybe it can be dropped since we already regulate the
back_flags pretty strictly.

>  
>  if (backing_fmt) {
>  backing_options = qdict_new();



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v5 1/2] QMP: add query-hotpluggable-cpus

[Igor Mammedov]

it will allow mgmt to query present and hotpluggable
CPU objects, it is required from a target platform that
wish to support command to implement and set
 MachineClass.query_hotpluggable_cpus
callback, which will return a list of possible CPU objects
with options that would be needed for hotplugging possible
CPU objects.

There are:
'type': 'str' - QOM CPU object type for usage with device_add
'vcpus-count': 'int' - number of logical VCPU threads per
CPU object (mgmt needs to know)

and a set of optional fields that are to used for hotplugging
a CPU objects and would allows mgmt tools to know what/where
it could be hotplugged;
[node],[socket],[core],[thread]

For present CPUs there is a 'qom-path' field which
would allow mgmt to inspect whatever object/abstraction
the target platform considers as CPU object.

Signed-off-by: Igor Mammedov 
---
v5:
 - fix s390 build failure:
undefined reference to `qmp_query_hotpluggable_cpus'
v4:
 - add MachineClass method to get CPU object list
v3:
 - add 'vcpus-count' field, pkre...@redhat.com
 - s/CpuInstanceProps/CpuInstanceProperties/
 - use '#optional' marker
 - make "props" as always present even if it's empty
 - fix JSON examples
 - fix minor typos
---
 include/hw/boards.h |  5 +
 monitor.c   | 13 +
 qapi-schema.json| 41 +
 qmp-commands.hx | 43 +++
 4 files changed, 102 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 8efce0f..a8f7dba 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -82,6 +82,10 @@ typedef struct {
  *Returns an array of @CPUArchId architecture-dependent CPU IDs
  *which includes CPU IDs for present and possible to hotplug CPUs.
  *Caller is responsible for freeing returned list.
+ * @query_hotpluggable_cpus:
+ *Returns a @HotpluggableCPUList, which describes CPUs objects which
+ *could be added with -device/device_add.
+ *Caller is responsible for freeing returned list.
  */
 struct MachineClass {
 /*< private >*/
@@ -124,6 +128,7 @@ struct MachineClass {
DeviceState *dev);
 unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
 CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
+HotpluggableCPUList *(*query_hotpluggable_cpus)(MachineState *machine);
 };
 
 /**
diff --git a/monitor.c b/monitor.c
index 4c02f0f..876dde4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4258,3 +4258,16 @@ void qmp_dump_skeys(const char *filename, Error **errp)
 error_setg(errp, QERR_FEATURE_DISABLED, "dump-skeys");
 }
 #endif
+
+HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
+{
+MachineState *ms = MACHINE(qdev_get_machine());
+MachineClass *mc = MACHINE_GET_CLASS(ms);
+
+if (!mc->query_hotpluggable_cpus) {
+error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
+return NULL;
+}
+
+return mc->query_hotpluggable_cpus(ms);
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 88f9b81..71e20e1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4126,3 +4126,44 @@
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# CpuInstanceProperties
+#
+# @node: NUMA node ID the CPU belongs to, optional
+# @socket: #optional socket number within node/board the CPU belongs to
+# @core: #optional core number within socket the CPU belongs to
+# @thread: #optional thread number within core the CPU belongs to
+#
+# Since: 2.7
+{ 'struct': 'CpuInstanceProperties',
+  'data': { '*node': 'int',
+'*socket': 'int',
+'*core': 'int',
+'*thread': 'int'
+  }
+}
+
+##
+# @HotpluggableCPU
+#
+# @type: CPU object type for usage with device_add command
+# @props: list of properties to be used for hotplugging CPU
+# @vcpus-count: number of logical VCPU threads @HotpluggableCPU provides
+# @qom-path: #optional link to existing CPU object if CPU is present or
+#omitted if CPU is not present.
+#
+# Since: 2.7
+{ 'struct': 'HotpluggableCPU',
+  'data': { 'type': 'str',
+'vcpus-count': 'int',
+'props': 'CpuInstanceProperties',
+'*qom-path': 'str'
+  }
+}
+
+##
+# @query-hotpluggable-cpus
+#
+# Since: 2.7
+{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9e05365..85ffba3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4853,3 +4853,46 @@ Example:
  {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
   "pop-vlan": 1, "id": 251658240}
]}
+
+EQMP
+
+{
+.name   = "query-hotpluggable-cpus",
+.args_type  = "",
+.mhandler.cmd_new = qmp_marshal_query_hotpluggable_cpus,
+},
+
+SQMP
+Show  existing/possible CPUs
+---
+
+Arguments: None.
+
+Example for 

Re: [Qemu-devel] [PATCH 09/20] block: Handle flush error in bdrv_pwrite_sync()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> We don't want to silently ignore a flush error.
> 
> Also, there is little point in avoiding the flush for writethrough modes
> and once WCE is moved to the BB layer, we definitely need the flush here
> because bdrv_pwrite() won't involve one any more.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/io.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 08/20] block: Always set writeback mode in blk_new_open()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> All callers of blk_new_open() either don't rely on the WCE bit set after
> blk_new_open() because they explicitly set it anyway, or they pass
> BDRV_O_CACHE_WB unconditionally.
> 
> This patch changes blk_new_open() so that it always enables writeback
> mode and asserts that BDRV_O_CACHE_WB is clear. For those callers that
> used to pass BDRV_O_CACHE_WB unconditionally, the flag is removed now.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/block-backend.c | 4 
>  block/parallels.c | 3 +--
>  block/qcow.c  | 3 +--
>  block/qcow2.c | 9 +++--
>  block/qed.c   | 3 +--
>  block/sheepdog.c  | 5 ++---
>  block/vdi.c   | 3 +--
>  block/vhdx.c  | 3 +--
>  block/vmdk.c  | 8 +++-
>  block/vpc.c   | 3 +--
>  blockdev.c| 1 +
>  11 files changed, 19 insertions(+), 26 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 07/20] block: blockdev_init(): Call blk_set_enable_write_cache() explicitly

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  blockdev.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 06/20] xen_disk: Call blk_set_enable_write_cache() explicitly

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  hw/block/xen_disk.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 05/20] qemu-img: Call blk_set_enable_write_cache() explicitly

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  qemu-img.c | 79 
> ++
>  1 file changed, 48 insertions(+), 31 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 839e05b..96b51d4 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c

[...]

> @@ -2862,26 +2874,30 @@ static int img_rebase(int argc, char **argv)
>  qemu_progress_print(0, 100);
>  
>  flags = BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
> -ret = bdrv_parse_cache_flags(cache, );
> +ret = bdrv_parse_cache_mode(cache, , );
>  if (ret < 0) {
>  error_report("Invalid cache option: %s", cache);
>  goto out;
>  }
>  
> -src_flags = BDRV_O_CACHE_WB;
> -ret = bdrv_parse_cache_flags(src_cache, _flags);
> +src_flags = 0;
> +ret = bdrv_parse_cache_mode(src_cache, _flags, _writethrough);
>  if (ret < 0) {
>  error_report("Invalid source cache option: %s", src_cache);
>  goto out;
>  }
>  
> +/* The source files are opened read-only, don't care about WCE */
> +assert((src_writethrough & BDRV_O_RDWR) == 0);

Well, yeah, that is a trivial assertion to make because BDRV_O_RDWR is 2.

I guess you meant s/src_writethrough/src_flags/.

With that fixed:

Reviewed-by: Max Reitz 

> +(void) src_writethrough;
> +
>  /*
>   * Open the images.
>   *
>   * Ignore the old backing file for unsafe rebase in case we want to 
> correct
>   * the reference to a renamed or moved backing file.
>   */
> -blk = img_open(image_opts, filename, fmt, flags, true, quiet);
> +blk = img_open(image_opts, filename, fmt, flags, true, writethrough, 
> quiet);
>  if (!blk) {
>  ret = -1;
>  goto out;



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 04/20] qemu-img: Expand all BDRV_O_FLAGS uses

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> It always only set the BDRV_O_CACHE_WB flag, which is going to go away.
> In order to make the next changes more local for better reviewability
> this patches expands the macro.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  qemu-img.c | 23 +++
>  1 file changed, 11 insertions(+), 12 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 03/20] qemu-io: Call blk_set_enable_write_cache() explicitly

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  qemu-io.c | 21 +
>  1 file changed, 13 insertions(+), 8 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 02/20] qemu-nbd: Call blk_set_enable_write_cache() explicitly

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  qemu-nbd.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 01/20] block: Add bdrv_parse_cache_mode()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> It's like bdrv_parse_cache_flags(), except that writethrough mode isn't
> included in the flags, but returned as a separate bool.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 17 +
>  include/block/block.h |  1 +
>  2 files changed, 18 insertions(+)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC for-2.7 1/1] block/qapi: Add query-block-node-tree

[Max Reitz]

On 25.03.2016 07:54, Wen Congyang wrote:
> On 03/25/2016 03:07 AM, Max Reitz wrote:
>> This command returns the tree of BlockDriverStates under a given root
>> node.
>>
>> Every tree node is described by its node name and the connection of a
>> parent node to its children additionally contains the role the child
>> assumes.
>>
>> A node's name can then be used e.g. in conjunction with
>> query-named-block-nodes to get more information about the node.
> 
> I found another problem:
> 
> {'execute': 'query-block-node-tree', 'arguments': {'root-node': 'disk1' } }
> {"return": {"children": [{"role": "children.1", "node": {"children": 
> [{"role": "file", "node": {}}], "node-name": "test1"}}, {"role": 
> "children.0", "node": {"children": [{"role": "file", "node": {}}]}}]}}
> 
> s->children[0] is children.0, and s->children[1] is children.1.
> But we output them in reverse order. The reason is:
> 
> BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
>  BlockDriverState *child_bs,
>  const char *child_name,
>  const BdrvChildRole *child_role)
> {
> BdrvChild *child = bdrv_root_attach_child(child_bs, child_name, 
> child_role);
> QLIST_INSERT_HEAD(_bs->children, child, next);
> return child;
> }
> 
> We insert the new child to the head, not the tail...

Well, the idea is that the order of children doesn't really matter; The
only thing that describes the behavior of a child is its role. For
instance, for qcow2 it doesn't matter whether the "file" or the
"backing" BDS is the first child.

However, for quorum, the order might matter (e.g. in FIFO mode). But
then again, the order is clearly specified by the role again: The first
child is the one with the "children.0" role.

So I don't think this is real problem as long as I add a note to the
documentation that the order of objects in the @children array is
undefined and does not have any significance.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC for-2.7 1/1] block/qapi: Add query-block-node-tree

[Max Reitz]

On 25.03.2016 03:50, Wen Congyang wrote:
> On 03/25/2016 03:07 AM, Max Reitz wrote:
>> This command returns the tree of BlockDriverStates under a given root
>> node.
>>
>> Every tree node is described by its node name and the connection of a
>> parent node to its children additionally contains the role the child
>> assumes.
>>
>> A node's name can then be used e.g. in conjunction with
>> query-named-block-nodes to get more information about the node.
> 
> I test this patch, and it works.
> {'execute': 'query-block-node-tree', 'arguments': {'root-node': 'disk1' } }
> {"return": {"children": [{"role": "children.0", "node": {"children": 
> [{"role": "file", "node": {"children": [], "node-name": "#block175"}}], 
> "node-name": "#block267"}}], "node-name": "#block040"}}
> 
> Shoule we hide the node name like "#blockxxx"?

No, I don't think so. In fact I was thinking of making the node name
non-optional because every BDS should have one due to these
auto-generated ones.

> If the bs doesn't have any child, should we output: '"children": [], '?

Omitting an empty array (thus making the @children key optional)
occurred to me, too. I didn't find any strong reason to do so, however.
It makes generating the output a bit more complicated and may also make
parsing it just a tiny bit more complicated.

I think that omitting it would make more sense to a human reader, but
QMP is a machine protocol, so this is not a very strong argument.

So all in all I saw neither strong arguments to omit an empty array nor
to include it. Thus I included it because that makes the code simpler.

> Can we add a new parameter: depth? For example, If I only want to know the 
> quorum's
> child name, we can limit the depth, and the output may be very clear.

Good idea. I can do so in the next version, but first I'd like to hear
more opinions on what other people think of this command. If they think
it's fine, I'll include a depth parameter.

Thank you for you comments!

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH][RFC] IOMMU: Add Support to VFIO devices with vIOMMU present

[Aviv B.D.]

On Wed, Mar 23, 2016 at 5:33 PM, Michael S. Tsirkin  wrote:

> On Sat, Mar 12, 2016 at 06:13:17PM +0200, Aviv B.D. wrote:
> > From: "Aviv B.D." 
> >
> >  * Fix bug that prevent qemu from starting up when vIOMMU and VFIO
> > device are present.
> >  * Advertise Cache Mode capability in iommu cap register.
> >  * Register every VFIO device with IOMMU state.
> >  * On page cache invalidation in vIOMMU, check if the domain belong to
> >VFIO device and mirror the guest requests to host.
> >
> > Not working (Yet!):
> >  * Tested only with network interface card (ixgbevf) and
> > intel_iommu=strict in guest's kernel command line.
> >  * Lock up under high load.
> >  * Errors on guest poweroff.
> >  * High relative latency compare to VFIO without IOMMU.
> >
> > Signed-off-by: Aviv B.D. 
> > ---
> >  hw/i386/intel_iommu.c  | 76
> ++
> >  hw/i386/intel_iommu_internal.h |  1 +
> >  hw/vfio/common.c   | 12 +--
> >  include/hw/i386/intel_iommu.h  |  4 +++
> >  include/hw/vfio/vfio-common.h  |  1 +
> >  5 files changed, 85 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 347718f..046688f 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -43,6 +44,9 @@ static int vtd_dbgflags = VTD_DBGBIT(GENERAL) |
> VTD_DBGBIT
> > (CSR);
> >  #define VTD_DPRINTF(what, fmt, ...) do {} while (0)
> >  #endif
> >
> > +static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num,
> > +uint8_t devfn, VTDContextEntry *ce);
> > +
> >  static void vtd_define_quad(IntelIOMMUState *s, hwaddr addr, uint64_t
> val,
> >  uint64_t wmask, uint64_t w1cmask)
> >  {
> > @@ -126,6 +130,19 @@ static uint32_t
> vtd_set_clear_mask_long(IntelIOMMUState
> > *s, hwaddr addr,
> >  return new_val;
> >  }
> >
> > +static uint16_t vtd_get_did_dev(IntelIOMMUState *s, uint8_t bus_num,
> uint8_t
> > devfn)
> > +{
> > +VTDContextEntry ce;
> > +int ret_fr;
> > +
> > +ret_fr = vtd_dev_to_context_entry(s, bus_num, devfn, );
> > +if (ret_fr){
> > +return -1;
> > +}
> > +
> > +return VTD_CONTEXT_ENTRY_DID(ce.hi);
> > +}
> > +
> >  static uint64_t vtd_set_clear_mask_quad(IntelIOMMUState *s, hwaddr addr,
> >  uint64_t clear, uint64_t mask)
> >  {
> > @@ -711,9 +728,9 @@ static int vtd_dev_to_context_entry(IntelIOMMUState
> *s,
> > uint8_t bus_num,
> >  }
> >
> >  if (!vtd_context_entry_present(ce)) {
> > -VTD_DPRINTF(GENERAL,
> > +/*VTD_DPRINTF(GENERAL,
> >  "error: context-entry #%"PRIu8 "(bus #%"PRIu8 ") "
> > -"is not present", devfn, bus_num);
> > +"is not present", devfn, bus_num);*/
> >  return -VTD_FR_CONTEXT_ENTRY_P;
> >  } else if ((ce->hi & VTD_CONTEXT_ENTRY_RSVD_HI) ||
> > (ce->lo & VTD_CONTEXT_ENTRY_RSVD_LO)) {
> > @@ -1020,14 +1037,53 @@ static void
> vtd_iotlb_page_invalidate(IntelIOMMUState
> > *s, uint16_t domain_id,
> >hwaddr addr, uint8_t am)
> >  {
> >  VTDIOTLBPageInvInfo info;
> > +VFIOGuestIOMMU * giommu;
> > +bool flag = false;
> >
> >  assert(am <= VTD_MAMV);
> >  info.domain_id = domain_id;
> >  info.addr = addr;
> >  info.mask = ~((1 << am) - 1);
> > +
> > +QLIST_FOREACH(giommu, &(s->giommu_list), iommu_next){
> > +VTDAddressSpace *vtd_as = container_of(giommu->iommu,
> VTDAddressSpace,
> > iommu);
> > +uint16_t vfio_source_id =
> vtd_make_source_id(pci_bus_num(vtd_as->bus),
> > vtd_as->devfn);
> > +uint16_t vfio_domain_id = vtd_get_did_dev(s,
> pci_bus_num(vtd_as->bus),
> > vtd_as->devfn);
> > +if (vfio_domain_id != (uint16_t)-1 &&
> > +domain_id == vfio_domain_id){
> > +VTDIOTLBEntry *iotlb_entry = vtd_lookup_iotlb(s,
> vfio_source_id,
> > addr);
> > +if (iotlb_entry != NULL){
> > +IOMMUTLBEntry entry;
> > +VTD_DPRINTF(GENERAL, "Remove addr 0x%"PRIx64 " mask
> %d", addr,
> > am);
> > +entry.iova = addr & VTD_PAGE_MASK_4K;
> > +entry.translated_addr =
> vtd_get_slpte_addr(iotlb_entry->slpte)
> > & VTD_PAGE_MASK_4K;
> > +entry.addr_mask = ~VTD_PAGE_MASK_4K;
> > +entry.perm = IOMMU_NONE;
> > +memory_region_notify_iommu(giommu->iommu, entry);
> > +flag = true;
> > +
> > +}
> > +}
> > +
> > +}
> > +
> >  g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_page,
> );
> > -}
> >
> > +QLIST_FOREACH(giommu, &(s->giommu_list), iommu_next){
> > +VTDAddressSpace *vtd_as = container_of(giommu->iommu,
> VTDAddressSpace,
> > iommu);
> > +uint16_t vfio_domain_id = 

[Qemu-devel] [PATCH v5 4/6] i.MX: Add the Freescale SPI Controller

[Jean-Christophe Dubois]

Signed-off-by: Jean-Christophe Dubois 
---

Changes since V1:
 * Access SPI slave only at a byte level.
 * rework the CS activation to avoid to reset access to SPI slaves.
 
Changes since V2:
 * Added #include "qemu/osdep.h"
 * remove previous_level from state struct
 * save burst_length in VMSTATE
 
Changes since V3:
 * Don't call qemu_set_irq() in reset function

Changes since V4:
 * None

 hw/ssi/Makefile.objs |   1 +
 hw/ssi/imx_spi.c | 454 +++
 include/hw/ssi/imx_spi.h | 103 +++
 3 files changed, 558 insertions(+)
 create mode 100644 hw/ssi/imx_spi.c
 create mode 100644 include/hw/ssi/imx_spi.h

diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
index 9555825..fcbb79e 100644
--- a/hw/ssi/Makefile.objs
+++ b/hw/ssi/Makefile.objs
@@ -4,3 +4,4 @@ common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
 common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
 
 obj-$(CONFIG_OMAP) += omap_spi.o
+obj-$(CONFIG_IMX) += imx_spi.o
diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
new file mode 100644
index 000..d5dd42a
--- /dev/null
+++ b/hw/ssi/imx_spi.c
@@ -0,0 +1,454 @@
+/*
+ * IMX SPI Controller
+ *
+ * Copyright (c) 2016 Jean-Christophe Dubois 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/ssi/imx_spi.h"
+#include "sysemu/sysemu.h"
+
+#ifndef DEBUG_IMX_SPI
+#define DEBUG_IMX_SPI 0
+#endif
+
+#define DPRINTF(fmt, args...) \
+do { \
+if (DEBUG_IMX_SPI) { \
+fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_SPI, \
+ __func__, ##args); \
+} \
+} while (0)
+
+static char const *imx_spi_reg_name(uint32_t reg)
+{
+static char unknown[20];
+
+switch (reg) {
+case ECSPI_RXDATA:
+return  "ECSPI_RXDATA";
+case ECSPI_TXDATA:
+return  "ECSPI_TXDATA";
+case ECSPI_CONREG:
+return  "ECSPI_CONREG";
+case ECSPI_CONFIGREG:
+return  "ECSPI_CONFIGREG";
+case ECSPI_INTREG:
+return  "ECSPI_INTREG";
+case ECSPI_DMAREG:
+return  "ECSPI_DMAREG";
+case ECSPI_STATREG:
+return  "ECSPI_STATREG";
+case ECSPI_PERIODREG:
+return  "ECSPI_PERIODREG";
+case ECSPI_TESTREG:
+return  "ECSPI_TESTREG";
+case ECSPI_MSGDATA:
+return  "ECSPI_MSGDATA";
+default:
+sprintf(unknown, "%d ?", reg);
+return unknown;
+}
+}
+
+static const VMStateDescription vmstate_imx_spi = {
+.name = TYPE_IMX_SPI,
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_FIFO32(tx_fifo, IMXSPIState),
+VMSTATE_FIFO32(rx_fifo, IMXSPIState),
+VMSTATE_INT16(burst_length, IMXSPIState),
+VMSTATE_UINT32_ARRAY(regs, IMXSPIState, ECSPI_MAX),
+VMSTATE_END_OF_LIST()
+},
+};
+
+static void imx_spi_txfifo_reset(IMXSPIState *s)
+{
+fifo32_reset(>tx_fifo);
+s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TE;
+s->regs[ECSPI_STATREG] &= ~ECSPI_STATREG_TF;
+}
+
+static void imx_spi_rxfifo_reset(IMXSPIState *s)
+{
+fifo32_reset(>rx_fifo);
+s->regs[ECSPI_STATREG] &= ~ECSPI_STATREG_RR;
+s->regs[ECSPI_STATREG] &= ~ECSPI_STATREG_RF;
+s->regs[ECSPI_STATREG] &= ~ECSPI_STATREG_RO;
+}
+
+static void imx_spi_update_irq(IMXSPIState *s)
+{
+int level;
+
+if (fifo32_is_empty(>rx_fifo)) {
+s->regs[ECSPI_STATREG] &= ~ECSPI_STATREG_RR;
+} else {
+s->regs[ECSPI_STATREG] |= ECSPI_STATREG_RR;
+}
+
+if (fifo32_is_full(>rx_fifo)) {
+s->regs[ECSPI_STATREG] |= ECSPI_STATREG_RF;
+} else {
+s->regs[ECSPI_STATREG] &= ~ECSPI_STATREG_RF;
+}
+
+if (fifo32_is_empty(>tx_fifo)) {
+s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TE;
+} else {
+s->regs[ECSPI_STATREG] &= ~ECSPI_STATREG_TE;
+}
+
+if (fifo32_is_full(>tx_fifo)) {
+s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TF;
+} else {
+s->regs[ECSPI_STATREG] &= ~ECSPI_STATREG_TF;
+}
+
+level = s->regs[ECSPI_STATREG] & s->regs[ECSPI_INTREG] ? 1 : 0;
+
+qemu_set_irq(s->irq, level);
+
+DPRINTF("IRQ level is %d\n", level);
+}
+
+static uint8_t imx_spi_selected_channel(IMXSPIState *s)
+{
+return EXTRACT(s->regs[ECSPI_CONREG], ECSPI_CONREG_CHANNEL_SELECT);
+}
+
+static uint32_t imx_spi_burst_length(IMXSPIState *s)
+{
+return EXTRACT(s->regs[ECSPI_CONREG], ECSPI_CONREG_BURST_LENGTH) + 1;
+}
+
+static bool imx_spi_is_enabled(IMXSPIState *s)
+{
+return s->regs[ECSPI_CONREG] & ECSPI_CONREG_EN;
+}
+
+static bool imx_spi_channel_is_master(IMXSPIState *s)
+{
+uint8_t mode = EXTRACT(s->regs[ECSPI_CONREG], ECSPI_CONREG_CHANNEL_MODE);
+
+return (mode & (1 << imx_spi_selected_channel(s))) ? true : false;
+}
+
+static bool imx_spi_is_multiple_master_burst(IMXSPIState *s)
+{
+uint8_t wave = 

[Qemu-devel] [PATCH v5 1/6] ARM: Factor out ARM on/off PSCI control functions

[Jean-Christophe Dubois]

Split ARM on/off function from PSCI support code.

This will allow to reuse these functions in other code.

Signed-off-by: Jean-Christophe Dubois 
---

Changes since V1:
 * Not present on V1

Changes since V2:
 * Not present on V2

Changes since V3:
 * Move to a more generic/usefull API
 * Manage CPU level/mode change at startup
 * Allow PSCI to cope with EL2/HYP level.
 * Keep distinct errors for different causes.

Changes since V4:
 * Rework documentation in header file.
 * Rework exception level handling.
 * Added a TODO for mode handling.

 target-arm/Makefile.objs  |   1 +
 target-arm/arm-powerctl.c | 239 ++
 target-arm/arm-powerctl.h |  75 +++
 target-arm/psci.c |  68 +
 4 files changed, 320 insertions(+), 63 deletions(-)
 create mode 100644 target-arm/arm-powerctl.c
 create mode 100644 target-arm/arm-powerctl.h

diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index a80eb39..60fd1dd 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -9,3 +9,4 @@ obj-y += neon_helper.o iwmmxt_helper.o
 obj-y += gdbstub.o
 obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
 obj-y += crypto_helper.o
+obj-y += arm-powerctl.o
diff --git a/target-arm/arm-powerctl.c b/target-arm/arm-powerctl.c
new file mode 100644
index 000..67e1579
--- /dev/null
+++ b/target-arm/arm-powerctl.c
@@ -0,0 +1,239 @@
+/*
+ * QEMU support -- ARM Power Control specific functions.
+ *
+ * Copyright (c) 2016 Jean-Christophe Dubois
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include 
+#include 
+#include "internals.h"
+#include "arm-powerctl.h"
+
+#ifndef DEBUG_ARM_POWERCTL
+#define DEBUG_ARM_POWERCTL 0
+#endif
+
+#define DPRINTF(fmt, args...) \
+do { \
+if (DEBUG_ARM_POWERCTL) { \
+fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \
+} \
+} while (0)
+
+CPUState *arm_get_cpu_by_id(uint64_t id)
+{
+CPUState *cpu;
+
+DPRINTF("cpu %" PRId64 "\n", id);
+
+CPU_FOREACH(cpu) {
+ARMCPU *armcpu = ARM_CPU(cpu);
+
+if (armcpu->mp_affinity == id) {
+return cpu;
+}
+}
+
+qemu_log_mask(LOG_GUEST_ERROR,
+  "[ARM]%s: Requesting unknown CPU %" PRId64 "\n",
+  __func__, id);
+
+return NULL;
+}
+
+int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id,
+   int target_el, bool target_aa64)
+{
+CPUState *target_cpu_state;
+ARMCPU *target_cpu;
+
+DPRINTF("cpu %" PRId64 " (EL %d, %s) @ 0x%" PRIx64 " with R0 = 0x%" PRIx64
+"\n", cpuid, target_el, target_aa64 ? "aarch64" : "aarch32", entry,
+context_id);
+
+/* requested EL level need to be above 0 */
+assert(target_el >= 1 && target_el <= 3);
+
+/* change to the cpu we are powering up */
+target_cpu_state = arm_get_cpu_by_id(cpuid);
+if (!target_cpu_state) {
+/* The cpu was not found */
+return QEMU_ARM_POWERCTL_INVALID_PARAM;
+}
+target_cpu = ARM_CPU(target_cpu_state);
+if (!target_cpu->powered_off) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "[ARM]%s: CPU %" PRId64 " is already on\n",
+  __func__, cpuid);
+return QEMU_ARM_POWERCTL_ALREADY_ON;
+}
+
+/* Initialize the cpu we are turning on */
+cpu_reset(target_cpu_state);
+target_cpu->powered_off = false;
+target_cpu_state->halted = 0;
+
+/*
+ * The newly brought CPU is requested to enter the exception level
+ * "target_el" and be in the requested mode (aarch64 ou aarch32).
+ */
+
+/*
+ * Check that the requested mode is a supported feature.
+ * 64 bits processors do support 32 bits mode but the opposite
+ * is not true.
+ */
+if (target_aa64 && !arm_feature(_cpu->env, ARM_FEATURE_AARCH64)) {
+return QEMU_ARM_POWERCTL_INVALID_PARAM;
+}
+
+/* We check that the request can be fulfilled */
+if (target_aa64 != arm_el_is_aa64(_cpu->env, target_el)) {
+/*
+ * TODO: We need to revisit this as the default config of a 64 bits
+ * CPU might not allow to boot in 32 bits even if the CPU is
+ * capable of it. We should implement some code to "force" the CPU in
+ * the requested mode if it is not its default mode for the considered
+ * exception level.
+ */
+return QEMU_ARM_POWERCTL_INVALID_PARAM;
+}
+
+/*
+ * Check that the CPU is supporting the requested level
+ */
+switch (target_el) {
+case 3:
+if (arm_feature(_cpu->env, ARM_FEATURE_EL3)) {
+if (target_aa64) {
+target_cpu->env.pstate = PSTATE_MODE_EL3h;
+} else {
+cpsr_write(_cpu->env, ARM_CPU_MODE_MON, CPSR_M,
+ 

[Qemu-devel] [PATCH v5 3/6] FIFO: Add a FIFO32 implementation

[Jean-Christophe Dubois]

This one is build on top of the existing FIFO8

Reviewed-by: Peter Maydell 
Signed-off-by: Jean-Christophe Dubois 
---

Changes since V1:
 * None 
 
Changes since V2:
 * Added copyright
 * define Fifo32 as a struct containing Fifo8
 * remove fifo32_pop_buf()

Changes since V3:
 * Added comment on unsupported fifo32_pop_buf()

Changes since V4:
 * None 

 include/qemu/fifo32.h | 191 ++
 1 file changed, 191 insertions(+)
 create mode 100644 include/qemu/fifo32.h

diff --git a/include/qemu/fifo32.h b/include/qemu/fifo32.h
new file mode 100644
index 000..2e5a0cc
--- /dev/null
+++ b/include/qemu/fifo32.h
@@ -0,0 +1,191 @@
+/*
+ * Generic FIFO32 component, based on FIFO8.
+ *
+ * Copyright (c) 2016 Jean-Christophe Dubois
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#ifndef FIFO32_H
+#define FIFO32_H
+
+#include "qemu/osdep.h"
+#include "qemu/fifo8.h"
+
+typedef struct {
+Fifo8 fifo;
+} Fifo32;
+
+/**
+ * fifo32_create:
+ * @fifo: struct Fifo32 to initialise with new FIFO
+ * @capacity: capacity of the newly created FIFO expressed in 32 bit words
+ *
+ * Create a FIFO of the specified size. Clients should call fifo32_destroy()
+ * when finished using the fifo. The FIFO is initially empty.
+ */
+
+static inline void fifo32_create(Fifo32 *fifo, uint32_t capacity)
+{
+fifo8_create(>fifo, capacity * sizeof(uint32_t));
+}
+
+/**
+ * fifo32_destroy:
+ * @fifo: FIFO to cleanup
+ *
+ * Cleanup a FIFO created with fifo32_create(). Frees memory created for FIFO
+ * storage. The FIFO is no longer usable after this has been called.
+ */
+
+static inline void fifo32_destroy(Fifo32 *fifo)
+{
+fifo8_destroy(>fifo);
+}
+
+/**
+ * fifo32_num_free:
+ * @fifo: FIFO to check
+ *
+ * Return the number of free uint32_t slots in the FIFO.
+ *
+ * Returns: Number of free 32 bit words.
+ */
+
+static inline uint32_t fifo32_num_free(Fifo32 *fifo)
+{
+return DIV_ROUND_UP(fifo8_num_free(>fifo), sizeof(uint32_t));
+}
+
+/**
+ * fifo32_num_used:
+ * @fifo: FIFO to check
+ *
+ * Return the number of used uint32_t slots in the FIFO.
+ *
+ * Returns: Number of used 32 bit words.
+ */
+
+static inline uint32_t fifo32_num_used(Fifo32 *fifo)
+{
+return DIV_ROUND_UP(fifo8_num_used(>fifo), sizeof(uint32_t));
+}
+
+/**
+ * fifo32_push:
+ * @fifo: FIFO to push to
+ * @data: 32 bits data word to push
+ *
+ * Push a 32 bits data word to the FIFO. Behaviour is undefined if the FIFO
+ * is full. Clients are responsible for checking for fullness using
+ * fifo32_is_full().
+ */
+
+static inline void fifo32_push(Fifo32 *fifo, uint32_t data)
+{
+int i;
+
+for (i = 0; i < sizeof(data); i++) {
+fifo8_push(>fifo, data & 0xff);
+data >>= 8;
+}
+}
+
+/**
+ * fifo32_push_all:
+ * @fifo: FIFO to push to
+ * @data: data to push
+ * @size: number of 32 bit words to push
+ *
+ * Push a 32 bit word array to the FIFO. Behaviour is undefined if the FIFO
+ * is full. Clients are responsible for checking the space left in the FIFO
+ * using fifo32_num_free().
+ */
+
+static inline void fifo32_push_all(Fifo32 *fifo, const uint32_t *data,
+   uint32_t num)
+{
+int i;
+
+for (i = 0; i < num; i++) {
+fifo32_push(fifo, data[i]);
+}
+}
+
+/**
+ * fifo32_pop:
+ * @fifo: fifo to pop from
+ *
+ * Pop a 32 bits data word from the FIFO. Behaviour is undefined if the FIFO
+ * is empty. Clients are responsible for checking for emptiness using
+ * fifo32_is_empty().
+ *
+ * Returns: The popped 32 bits data word.
+ */
+
+static inline uint32_t fifo32_pop(Fifo32 *fifo)
+{
+uint32_t ret = 0;
+int i;
+
+for (i = 0; i < sizeof(uint32_t); i++) {
+ret |= (fifo8_pop(>fifo) << (i * 8));
+}
+
+return ret;
+}
+
+/**
+ * There is no fifo32_pop_buf() because the data is not stored in the buffer
+ * as a set of native-order words.
+ */
+
+/**
+ * fifo32_reset:
+ * @fifo: FIFO to reset
+ *
+ * Reset a FIFO. All data is discarded and the FIFO is emptied.
+ */
+
+static inline void fifo32_reset(Fifo32 *fifo)
+{
+fifo8_reset(>fifo);
+}
+
+/**
+ * fifo32_is_empty:
+ * @fifo: FIFO to check
+ *
+ * Check if a FIFO is empty.
+ *
+ * Returns: True if the fifo is empty, false otherwise.
+ */
+
+static inline bool fifo32_is_empty(Fifo32 *fifo)
+{
+return fifo8_is_empty(>fifo);
+}
+
+/**
+ * fifo32_is_full:
+ * @fifo: FIFO to check
+ *
+ * Check if a FIFO is full.
+ *
+ * Returns: True if the fifo is full, false otherwise.
+ */
+
+static inline bool fifo32_is_full(Fifo32 *fifo)
+{
+return fifo8_num_free(>fifo) < 

[Qemu-devel] [PATCH v5 2/6] i.MX: Add i.MX6 System Reset Controller device.

[Jean-Christophe Dubois]

This controller is also present in i.MX5X devices but they are not
yet emulated by QEMU.

Reviewed-by: Peter Maydell 
Signed-off-by: Jean-Christophe Dubois 
---

Changes since V1:
 * Change "reset" sematic to mean full power cyvle.

Changes since V2:
 * use arm-powerctl API
 * Added #include "qemu/osdep.h"

Changes since V3:
 * Minor fixes.

Changes since V4:
 * None

 hw/misc/Makefile.objs  |   1 +
 hw/misc/imx6_src.c | 264 +
 include/hw/misc/imx6_src.h |  73 +
 3 files changed, 338 insertions(+)
 create mode 100644 hw/misc/imx6_src.c
 create mode 100644 include/hw/misc/imx6_src.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 44ac2e1..2562fa2 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -29,6 +29,7 @@ obj-$(CONFIG_IMX) += imx_ccm.o
 obj-$(CONFIG_IMX) += imx31_ccm.o
 obj-$(CONFIG_IMX) += imx25_ccm.o
 obj-$(CONFIG_IMX) += imx6_ccm.o
+obj-$(CONFIG_IMX) += imx6_src.o
 obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
 obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
 obj-$(CONFIG_MAINSTONE) += mst_fpga.o
diff --git a/hw/misc/imx6_src.c b/hw/misc/imx6_src.c
new file mode 100644
index 000..6b026b4
--- /dev/null
+++ b/hw/misc/imx6_src.c
@@ -0,0 +1,264 @@
+/*
+ * IMX6 System Reset Controller
+ *
+ * Copyright (c) 2015 Jean-Christophe Dubois 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/misc/imx6_src.h"
+#include "sysemu/sysemu.h"
+#include "qemu/bitops.h"
+#include "arm-powerctl.h"
+
+#ifndef DEBUG_IMX6_SRC
+#define DEBUG_IMX6_SRC 0
+#endif
+
+#define DPRINTF(fmt, args...) \
+do { \
+if (DEBUG_IMX6_SRC) { \
+fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX6_SRC, \
+ __func__, ##args); \
+} \
+} while (0)
+
+static char const *imx6_src_reg_name(uint32_t reg)
+{
+static char unknown[20];
+
+switch (reg) {
+case SRC_SCR:
+return "SRC_SCR";
+case SRC_SBMR1:
+return "SRC_SBMR1";
+case SRC_SRSR:
+return "SRC_SRSR";
+case SRC_SISR:
+return "SRC_SISR";
+case SRC_SIMR:
+return "SRC_SIMR";
+case SRC_SBMR2:
+return "SRC_SBMR2";
+case SRC_GPR1:
+return "SRC_GPR1";
+case SRC_GPR2:
+return "SRC_GPR2";
+case SRC_GPR3:
+return "SRC_GPR3";
+case SRC_GPR4:
+return "SRC_GPR4";
+case SRC_GPR5:
+return "SRC_GPR5";
+case SRC_GPR6:
+return "SRC_GPR6";
+case SRC_GPR7:
+return "SRC_GPR7";
+case SRC_GPR8:
+return "SRC_GPR8";
+case SRC_GPR9:
+return "SRC_GPR9";
+case SRC_GPR10:
+return "SRC_GPR10";
+default:
+sprintf(unknown, "%d ?", reg);
+return unknown;
+}
+}
+
+static const VMStateDescription vmstate_imx6_src = {
+.name = TYPE_IMX6_SRC,
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32_ARRAY(regs, IMX6SRCState, SRC_MAX),
+VMSTATE_END_OF_LIST()
+},
+};
+
+static void imx6_src_reset(DeviceState *dev)
+{
+IMX6SRCState *s = IMX6_SRC(dev);
+
+DPRINTF("\n");
+
+memset(s->regs, 0, sizeof(s->regs));
+
+/* Set reset values */
+s->regs[SRC_SCR] = 0x521;
+s->regs[SRC_SRSR] = 0x1;
+s->regs[SRC_SIMR] = 0x1F;
+}
+
+static uint64_t imx6_src_read(void *opaque, hwaddr offset, unsigned size)
+{
+uint32_t value = 0;
+IMX6SRCState *s = (IMX6SRCState *)opaque;
+uint32_t index = offset >> 2;
+
+if (index < SRC_MAX) {
+value = s->regs[index];
+} else {
+qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
+  HWADDR_PRIx "\n", TYPE_IMX6_SRC, __func__, offset);
+
+}
+
+DPRINTF("reg[%s] => 0x%" PRIx32 "\n", imx6_src_reg_name(index), value);
+
+return value;
+}
+
+static void imx6_src_write(void *opaque, hwaddr offset, uint64_t value,
+   unsigned size)
+{
+IMX6SRCState *s = (IMX6SRCState *)opaque;
+uint32_t index = offset >> 2;
+unsigned long change_mask;
+unsigned long current_value = value;
+
+if (index >=  SRC_MAX) {
+qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
+  HWADDR_PRIx "\n", TYPE_IMX6_SRC, __func__, offset);
+return;
+}
+
+DPRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx6_src_reg_name(index),
+(uint32_t)current_value);
+
+change_mask = s->regs[index] ^ (uint32_t)current_value;
+
+switch (index) {
+case SRC_SCR:
+/*
+ * On real hardware when the system reset controller starts a
+ * secondary CPU it runs through some boot ROM code which reads
+ * the SRC_GPRX registers controlling the start address and branches
+ 

[Qemu-devel] [PATCH v5 6/6] i.MX: Add sabrelite i.MX6 emulation.

[Jean-Christophe Dubois]

The sabrelite supports one SPI FLASH memory on SPI1

Signed-off-by: Jean-Christophe Dubois 
---

Changes since V1:
 * output a message and exit if RAM size is unsupported.

Changes since V2:
 * Added include "qemu/osdep.h"
 * Added access to controllers through properties.

Changes since V3: 
 * None

Changes since V3: 
 * Added a TODO.

 hw/arm/Makefile.objs |   2 +-
 hw/arm/sabrelite.c   | 122 +++
 2 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/sabrelite.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 2b20e49..12764ef 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -16,5 +16,5 @@ obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
 obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
 obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
 obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
-obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
+obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
 obj-$(CONFIG_ASPEED_SOC) += ast2400.o palmetto-bmc.o
diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
new file mode 100644
index 000..4037327
--- /dev/null
+++ b/hw/arm/sabrelite.c
@@ -0,0 +1,122 @@
+/*
+ * SABRELITE Board System emulation.
+ *
+ * Copyright (c) 2015 Jean-Christophe Dubois 
+ *
+ * This code is licensed under the GPL, version 2 or later.
+ * See the file `COPYING' in the top level directory.
+ *
+ * It (partially) emulates a sabrelite board, with a Freescale
+ * i.MX6 SoC
+ */
+
+#include "qemu/osdep.h"
+#include "hw/arm/fsl-imx6.h"
+#include "hw/boards.h"
+#include "qemu/error-report.h"
+#include "exec/address-spaces.h"
+#include "net/net.h"
+#include "hw/devices.h"
+#include "hw/char/serial.h"
+#include "sysemu/qtest.h"
+
+typedef struct IMX6Sabrelite {
+FslIMX6State soc;
+MemoryRegion ram;
+} IMX6Sabrelite;
+
+static struct arm_boot_info sabrelite_binfo = {
+/* DDR memory start */
+.loader_start = FSL_IMX6_MMDC_ADDR,
+/* No board ID, we boot from DT tree */
+.board_id = -1,
+};
+
+/* No need to do any particular setup for secondary boot */
+static void sabrelite_write_secondary(ARMCPU *cpu,
+  const struct arm_boot_info *info)
+{
+}
+
+/* Secondary cores are reset through SRC device */
+static void sabrelite_reset_secondary(ARMCPU *cpu,
+  const struct arm_boot_info *info)
+{
+}
+
+static void sabrelite_init(MachineState *machine)
+{
+IMX6Sabrelite *s = g_new0(IMX6Sabrelite, 1);
+Error *err = NULL;
+
+/* Check the amount of memory is compatible with the SOC */
+if (machine->ram_size > FSL_IMX6_MMDC_SIZE) {
+error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
+ machine->ram_size, FSL_IMX6_MMDC_SIZE);
+exit(1);
+}
+
+object_initialize(>soc, sizeof(s->soc), TYPE_FSL_IMX6);
+object_property_add_child(OBJECT(machine), "soc", OBJECT(>soc),
+  _abort);
+
+object_property_set_bool(OBJECT(>soc), true, "realized", );
+if (err != NULL) {
+error_report("%s", error_get_pretty(err));
+exit(1);
+}
+
+memory_region_allocate_system_memory(>ram, NULL, "sabrelite.ram",
+ machine->ram_size);
+memory_region_add_subregion(get_system_memory(), FSL_IMX6_MMDC_ADDR,
+>ram);
+
+{
+/*
+ * TODO: Ideally we would expose the chip select and spi bus on the
+ * SoC object using alias properties; then we would not need to
+ * directly access the underlying spi device object.
+ */
+/* Add the sst25vf016b NOR FLASH memory to first SPI */
+Object *spi_dev;
+
+spi_dev = object_resolve_path_component(OBJECT(>soc), "spi1");
+if (spi_dev) {
+SSIBus *spi_bus;
+
+spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(spi_dev), "spi");
+if (spi_bus) {
+DeviceState *flash_dev;
+
+flash_dev = ssi_create_slave(spi_bus, "sst25vf016b");
+if (flash_dev) {
+qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev,
+  SSI_GPIO_CS, 0);
+sysbus_connect_irq(SYS_BUS_DEVICE(spi_dev), 1, cs_line);
+}
+}
+}
+}
+
+sabrelite_binfo.ram_size = machine->ram_size;
+sabrelite_binfo.kernel_filename = machine->kernel_filename;
+sabrelite_binfo.kernel_cmdline = machine->kernel_cmdline;
+sabrelite_binfo.initrd_filename = machine->initrd_filename;
+sabrelite_binfo.nb_cpus = smp_cpus;
+sabrelite_binfo.secure_boot = true;
+sabrelite_binfo.write_secondary_boot = sabrelite_write_secondary;
+sabrelite_binfo.secondary_cpu_reset_hook = sabrelite_reset_secondary;
+
+if (!qtest_enabled()) {
+

[Qemu-devel] [PATCH v5 5/6] i.MX: Add i.MX6 SOC implementation.

[Jean-Christophe Dubois]

For now we only support the following devices:
* up to 4 Cortex A9 cores
* A9 MPCORE (SCU, GIC, TWD)
* 5 i.MX UARTs
* 2 EPIT timers
* 1 GPT timer
* 3 I2C controllers
* 7 GPIO controllers
* 6 SDHC controllers
* 5 SPI controllers
* 1 CCM device
* 1 SRC device
* various ROM/RAM areas.

Reviewed-by: Peter Maydell 
Signed-off-by: Jean-Christophe Dubois 
---

Changes since V1:
 * use g_strdup_printf/g_free instead of local char array.
 * output a message on exit for unsupported number of cores.
 
Changes since V2:
 * Added Include "osdep/qemu.h"
 * Added support for EL3
 * Added SPI controllers
 * Added a property for each controller.
 
Changes since V3:
 * None.

Changes since V3:
 * None.

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs|   1 +
 hw/arm/fsl-imx6.c   | 449 +++
 include/hw/arm/fsl-imx6.h   | 450 
 4 files changed, 901 insertions(+)
 create mode 100644 hw/arm/fsl-imx6.c
 create mode 100644 include/hw/arm/fsl-imx6.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index c63cdd0..c5bcba7 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -100,6 +100,7 @@ CONFIG_ALLWINNER_A10_PIT=y
 CONFIG_ALLWINNER_A10_PIC=y
 CONFIG_ALLWINNER_A10=y
 
+CONFIG_FSL_IMX6=y
 CONFIG_FSL_IMX31=y
 CONFIG_FSL_IMX25=y
 
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 954c9fe..2b20e49 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -16,4 +16,5 @@ obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
 obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
 obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
 obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
+obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
 obj-$(CONFIG_ASPEED_SOC) += ast2400.o palmetto-bmc.o
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
new file mode 100644
index 000..b8e93a6
--- /dev/null
+++ b/hw/arm/fsl-imx6.c
@@ -0,0 +1,449 @@
+/*
+ * Copyright (c) 2015 Jean-Christophe Dubois 
+ *
+ * i.MX6 SOC emulation.
+ *
+ * Based on hw/arm/fsl-imx31.c
+ *
+ *  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 .
+ */
+
+#include "qemu/osdep.h"
+#include "hw/arm/fsl-imx6.h"
+#include "sysemu/sysemu.h"
+#include "exec/address-spaces.h"
+#include "hw/boards.h"
+#include "sysemu/char.h"
+#include "qemu/error-report.h"
+
+#define NAME_SIZE 20
+
+static void fsl_imx6_init(Object *obj)
+{
+FslIMX6State *s = FSL_IMX6(obj);
+char name[NAME_SIZE];
+int i;
+
+if (smp_cpus > FSL_IMX6_NUM_CPUS) {
+error_report("%s: Only %d CPUs are supported (%d requested)",
+ TYPE_FSL_IMX6, FSL_IMX6_NUM_CPUS, smp_cpus);
+exit(1);
+}
+
+for (i = 0; i < smp_cpus; i++) {
+object_initialize(>cpu[i], sizeof(s->cpu[i]),
+  "cortex-a9-" TYPE_ARM_CPU);
+snprintf(name, NAME_SIZE, "cpu%d", i);
+object_property_add_child(obj, name, OBJECT(>cpu[i]), NULL);
+}
+
+object_initialize(>a9mpcore, sizeof(s->a9mpcore), TYPE_A9MPCORE_PRIV);
+qdev_set_parent_bus(DEVICE(>a9mpcore), sysbus_get_default());
+object_property_add_child(obj, "a9mpcore", OBJECT(>a9mpcore), NULL);
+
+object_initialize(>ccm, sizeof(s->ccm), TYPE_IMX6_CCM);
+qdev_set_parent_bus(DEVICE(>ccm), sysbus_get_default());
+object_property_add_child(obj, "ccm", OBJECT(>ccm), NULL);
+
+object_initialize(>src, sizeof(s->src), TYPE_IMX6_SRC);
+qdev_set_parent_bus(DEVICE(>src), sysbus_get_default());
+object_property_add_child(obj, "src", OBJECT(>src), NULL);
+
+for (i = 0; i < FSL_IMX6_NUM_UARTS; i++) {
+object_initialize(>uart[i], sizeof(s->uart[i]), TYPE_IMX_SERIAL);
+qdev_set_parent_bus(DEVICE(>uart[i]), sysbus_get_default());
+snprintf(name, NAME_SIZE, "uart%d", i + 1);
+object_property_add_child(obj, name, OBJECT(>uart[i]), NULL);
+}
+
+object_initialize(>gpt, sizeof(s->gpt), TYPE_IMX_GPT);
+qdev_set_parent_bus(DEVICE(>gpt), sysbus_get_default());
+object_property_add_child(obj, "gpt", OBJECT(>gpt), NULL);
+
+for (i = 0; i < FSL_IMX6_NUM_EPITS; i++) {
+object_initialize(>epit[i], sizeof(s->epit[i]), TYPE_IMX_EPIT);
+qdev_set_parent_bus(DEVICE(>epit[i]), 

[Qemu-devel] [PATCH v5 0/6] Add i.MX6 (Single/Dual/Quad) support

[Jean-Christophe Dubois]

This patch series adds support for the Freescale i.MX6 processor.

For now we only support the following devices:
* up to 4 Cortex A9 cores
* A9 MPCORE (SCU, GIC, TWD)
* 5 i.MX UARTs
* 2 EPIT timers
* 1 GPT timer
* 7 GPIO controllers
* 6 SDHC controllers
* 5 SPI controllers
* 1 CCM device
* 1 SRC device
* 3 I2C devices
* various ROM/RAM areas.

This also adds the sabrelite board as a an actual platform for i.MX6.

This series was tested by booting a 4.4 linux kernel (using the
imx_v6_v7_defconfig file as kernel configuration).

Note1: as sabrelite uses uart2 as console, you have to redirect the second
QEMU serial port to stdout.
qemu-system-arm -M sabrelite -display none ... -serial null -serial stdio

Note2: You need to disable the GPIO section related to physical push buttons
in the Linux DTS tree in order to avoid excecive interrupt triggering on
GPIO devices for non existant buttons.

Jean-Christophe Dubois (6):
  ARM: Factor out ARM on/off PSCI control functions
  i.MX: Add i.MX6 System Reset Controller device.
  FIFO: Add a FIFO32 implementation
  i.MX: Add the Freescale SPI Controller
  i.MX: Add i.MX6 SOC implementation.
  i.MX: Add sabrelite i.MX6 emulation.

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs|   1 +
 hw/arm/fsl-imx6.c   | 449 +++
 hw/arm/sabrelite.c  | 122 +++
 hw/misc/Makefile.objs   |   1 +
 hw/misc/imx6_src.c  | 264 +++
 hw/ssi/Makefile.objs|   1 +
 hw/ssi/imx_spi.c| 454 
 include/hw/arm/fsl-imx6.h   | 450 +++
 include/hw/misc/imx6_src.h  |  73 +++
 include/hw/ssi/imx_spi.h| 103 +
 include/qemu/fifo32.h   | 191 +
 target-arm/Makefile.objs|   1 +
 target-arm/arm-powerctl.c   | 239 +
 target-arm/arm-powerctl.h   |  75 +++
 target-arm/psci.c   |  68 +-
 16 files changed, 2430 insertions(+), 63 deletions(-)
 create mode 100644 hw/arm/fsl-imx6.c
 create mode 100644 hw/arm/sabrelite.c
 create mode 100644 hw/misc/imx6_src.c
 create mode 100644 hw/ssi/imx_spi.c
 create mode 100644 include/hw/arm/fsl-imx6.h
 create mode 100644 include/hw/misc/imx6_src.h
 create mode 100644 include/hw/ssi/imx_spi.h
 create mode 100644 include/qemu/fifo32.h
 create mode 100644 target-arm/arm-powerctl.c
 create mode 100644 target-arm/arm-powerctl.h

-- 
2.5.0

Re: [Qemu-devel] [PATCH 3/3] hid.c: Add debug support

[Alex Bennée]

Programmingkid  writes:

> Add debug macros to the code for easier debugging.
>
> Signed-off-by: John Arbuckle 
> ---
>  hw/input/hid.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/hw/input/hid.c b/hw/input/hid.c
> index 329a27b..42ca592 100644
> --- a/hw/input/hid.c
> +++ b/hw/input/hid.c
> @@ -37,6 +37,13 @@
>  #define RELEASED -1
>  #define PUSHED -2
>
> +/* #define DEBUG_HID_CODE */
> +#ifdef DEBUG_HID_CODE
> +#define DEBUG_HID(fmt, ...) printf(fmt, __VA_ARGS__)
> +#else
> +#define DEBUG_HID(fmt, ...) (void)0
> +#endif
> +

This style of debug setup is discouraged these days as its prone to
bitrot. It's better to define like this:

#define DEBUG_HID(fmt, ...) \
  if (DEBUG_HID_CODE) { \
 printf(fmt, __VA_ARGS);\
  }

This means you get the benefit of the compiler checking your format
strings even if the code gets optimised away when DEBUG_HID_CODE isn't
defined.

>  /* Translates a QKeyCode to USB HID value */
>  static const uint8_t qcode_to_usb_hid[] = {
>  [Q_KEY_CODE_SHIFT] = USB_HID_LEFT_SHIFT,
> @@ -331,6 +338,7 @@ static void hid_keyboard_event(DeviceState *dev, 
> QemuConsole *src,
>  return;
>  }
>  keycode = qcode_to_usb_hid[qcode];
> +DEBUG_HID("keycode = 0x%x qcode:%d\n", keycode, qcode);
>
>  count = 2;
>  if (evt->u.key.data->down == false) { /* if key up event */
> @@ -381,6 +389,9 @@ static void hid_keyboard_process_keycode(HIDState *hs)
>  slot = hs->head & QUEUE_MASK; QUEUE_INCR(hs->head); hs->n--;
>  keycode = hs->kbd.keycodes[slot];
>
> +DEBUG_HID("keycode:0x%x status:%s\n", keycode, (status == PUSHED ? 
> "Pushed"
> +  : "Released"));
> +
>  /* handle Control, Option, GUI/Windows/Command, and Shift keys */
>  if (keycode >= 0xe0) {
>  process_modifier_key(status, keycode, &(hs->kbd.modifiers));


--
Alex Bennée