Re: [Qemu-devel] [PATCH v2 01/16] qom/object: Add a new function object_initialize_child()

[Thomas Huth]

On 13.07.2018 23:46, Eduardo Habkost wrote:
> On Fri, Jul 13, 2018 at 11:29:17PM +0200, Andreas Färber wrote:
>> Am 13.07.2018 um 23:16 schrieb Eduardo Habkost:
>>> I wonder if we should deprecate object_initialize() and support
>>> only object_initialize_child() later.  Initializing an object
>>> contained inside another one without making it a child of the
>>> parent object is a recipe for trouble.
>>
>> The root container object needs to be initialized, too.
> 
> If the object is not embedded in another struct, I would expect
> it to be created using object_new() instead of
> object_initialize().

True. So I guess having a closer look at code that calls
object_initialize() only is something for our TODO lists once 3.0 has
been released...

 Thomas

Re: [Qemu-devel] [PATCH v2 03/16] hw/arm/bcm2836: Fix crash with device_add bcm2837 on unsupported machines

[Thomas Huth]

On 13.07.2018 23:26, Eduardo Habkost wrote:
> On Fri, Jul 13, 2018 at 10:27:31AM +0200, Thomas Huth wrote:
>> When trying to "device_add bcm2837" on a machine that is not suitable for
>> this device, you can quickly crash QEMU afterwards, e.g. with "info qtree":
>>
>> echo "{'execute':'qmp_capabilities'} {'execute':'device_add', " \
>>  "'arguments':{'driver':'bcm2837'}} {'execute': 'human-monitor-command', " \
>>  "'arguments': {'command-line': 'info qtree'}}" | \
>>  aarch64-softmmu/qemu-system-aarch64 -M integratorcp,accel=qtest -S -qmp 
>> stdio
>>
>> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2},
>>  "package": "build-all"}, "capabilities": []}}
>> {"return": {}}
>> {"error": {"class": "GenericError", "desc": "Device 'bcm2837' can not be
>>  hotplugged on this machine"}}
>> Segmentation fault (core dumped)
>>
>> The qdev_set_parent_bus() from instance_init adds a link to the child devices
>> which is not valid anymore after the bcm2837 instance has been destroyed.
>> Unfortunately, the child devices do not get destroyed / unlinked correctly
>> because both object_initialize() and object_property_add_child() increase
>> the reference count of the child objects by one, but only one reference
>> is dropped when the parent gets removed. So let's use the new functions
>> object_initialize_child() and sysbus_init_child_obj() instead to create
>> the objects, which will take care of creating the child objects with the
>> correct reference count of one.
>>
>> Signed-off-by: Thomas Huth 
> 
> Reviewed-by: Eduardo Habkost 
> 
> The usage of &error_abort in code that can be triggered from
> device-list-properties still makes me nervous, but that's a
> separate issue.

I first had similar thoughts, but I think it's a clear coding issue if
the abort triggers here (and not something that the user should normally
be able to trigger somehow), so error_abort should be ok in this case.

 Thomas

Re: [Qemu-devel] [PATCH v2 01/16] qom/object: Add a new function object_initialize_child()

[Thomas Huth]

On 14.07.2018 00:57, Eduardo Habkost wrote:
> On Fri, Jul 13, 2018 at 10:27:29AM +0200, Thomas Huth wrote:
>> A lot of code is using the object_initialize() function followed by a call
>> to object_property_add_child() to add the newly initialized object as a child
>> of the current object. Both functions increase the reference counter of the
>> new object, but many spots that call these two functions then forget to drop
>> one of the superfluous references. So the newly created object is often not
>> cleaned up correctly when the parent is destroyed. In the worst case, this
>> can cause crashes, e.g. because device objects are not correctly removed from
>> their parent_bus.
>>
>> Since this is a common pattern between many code spots, let's introdcue a
>> new function that takes care of calling all three required initialization
>> functions, first object_initialize(), then object_property_add_child() and
>> finally object_unref().
>>
>> And while we're at object.h, also fix some copy-n-paste errors in the
>> comments there ("to store the area" --> "to store the error").
>>
>> Signed-off-by: Thomas Huth 
> 
> Potential candidates for using the new function, found using the
> following Coccinelle patch:
> 
> @@
> expression child, size, type, parent, errp, propname;
> @@
> -object_initialize(child, size, type);
> -object_property_add_child(
> +object_initialize_child(
>parent, propname, 
> -  OBJECT(child),
> +  child, size, type,
>errp);
> 
> Some of them (very few) already call object_unref() and need to
> be fixed manually.
> 
> Most of the remaining ~50 object_initialize() callers are also
> candidates, even if they don't call object_property_add_child()
> today.
> 
> Signed-off-by: Eduardo Habkost 
> ---
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index bb9d33848d..e5923f85af 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -189,8 +189,8 @@ static void aspeed_board_init(MachineState *machine,
>  DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
>  
>  bmc = g_new0(AspeedBoardState, 1);
> -object_initialize(&bmc->soc, (sizeof(bmc->soc)), cfg->soc_name);
> -object_property_add_child(OBJECT(machine), "soc", OBJECT(&bmc->soc),
> +object_initialize_child(OBJECT(machine), "soc",
> +  &bmc->soc, (sizeof(bmc->soc)), cfg->soc_name,
>&error_abort);
>  
>  sc = ASPEED_SOC_GET_CLASS(&bmc->soc);
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index e68911af0f..994262756f 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -106,11 +106,11 @@ static void aspeed_soc_init(Object *obj)
>  AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
>  int i;
>  
> -object_initialize(&s->cpu, sizeof(s->cpu), sc->info->cpu_type);
> -object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> +object_initialize_child(obj, "cpu",  &s->cpu, sizeof(s->cpu),
> +sc->info->cpu_type, NULL);
>  
> -object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
> -object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
> +object_initialize_child(obj, "scu",  &s->scu, sizeof(s->scu),
> +TYPE_ASPEED_SCU, NULL);
>  qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
>  qdev_prop_set_uint32(DEVICE(&s->scu), "silicon-rev",

Thanks, that's definitely something we should do for 3.1! But for 3.0, I
think we better only focus on the ones that cause reproducible problem.

And the spots that also use qdev_set_parent_bus(...,
sysbus_get_default()) should use sysbus_init_child_obj() instead.

 Thomas

[Qemu-devel] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

[Markus Armbruster]

Libvirt developers would like to be copied on patches to qemu-doc
appendix "Deprecated features".  Do them the favor.

Signed-off-by: Markus Armbruster 
---
 MAINTAINERS | 4 
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 20eef3cb61..666e936812 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2194,6 +2194,10 @@ M: Daniel P. Berrange 
 S: Odd Fixes
 F: docs/devel/build-system.txt
 
+Incompatible changes
+R: libvir-l...@redhat.com
+F: qemu-deprecated.texi
+
 Build System
 
 GIT submodules
-- 
2.17.1

[Qemu-devel] [PATCH 0/2] Make it easier to track feature deprecation

[Markus Armbruster]

Markus Armbruster (2):
  qemu-doc: Move appendix "Deprecated features" to its own file
  MAINTAINERS: New section "Incompatible changes", copy libvir-list

 MAINTAINERS  |   4 +
 qemu-deprecated.texi | 234 ++
 qemu-doc.texi| 235 +--
 3 files changed, 239 insertions(+), 234 deletions(-)
 create mode 100644 qemu-deprecated.texi

-- 
2.17.1

[Qemu-devel] [PATCH 1/2] qemu-doc: Move appendix "Deprecated features" to its own file

[Markus Armbruster]

Consumers of QEMU need to track feature deprecation.  Keeping
deprecation documentation in its own file helps in two small ways:

* You can track changes the easy and obvious way, with git-log.
  Before, you had to resort to more complex gittery like "git-log
  --oneline -L '/@node Deprecated features/,/@node Supported build
  platforms/:qemu-doc.texi'"

* It lets us use MAINTAINERS to copy interested parties on deprecation
  patches, so they can advise or object before they're a done deal.
  The next commit will do that for libvirt.

Signed-off-by: Markus Armbruster 
---
 qemu-deprecated.texi | 234 ++
 qemu-doc.texi| 235 +--
 2 files changed, 235 insertions(+), 234 deletions(-)
 create mode 100644 qemu-deprecated.texi

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
new file mode 100644
index 00..9920a85adc
--- /dev/null
+++ b/qemu-deprecated.texi
@@ -0,0 +1,234 @@
+@node Deprecated features
+@appendix Deprecated features
+
+In general features are intended to be supported indefinitely once
+introduced into QEMU. In the event that a feature needs to be removed,
+it will be listed in this appendix. The feature will remain functional
+for 2 releases prior to actual removal. Deprecated features may also
+generate warnings on the console when QEMU starts up, or if activated
+via a monitor command, however, this is not a mandatory requirement.
+
+Prior to the 2.10.0 release there was no official policy on how
+long features would be deprecated prior to their removal, nor
+any documented list of which features were deprecated. Thus
+any features deprecated prior to 2.10.0 will be treated as if
+they were first deprecated in the 2.10.0 release.
+
+What follows is a list of all features currently marked as
+deprecated.
+
+@section Build options
+
+@subsection GTK 2.x
+
+Previously QEMU has supported building against both GTK 2.x
+and 3.x series APIs. Support for the GTK 2.x builds will be
+discontinued, so maintainers should switch to using GTK 3.x,
+which is the default.
+
+@subsection SDL 1.2
+
+Previously QEMU has supported building against both SDL 1.2
+and 2.0 series APIs. Support for the SDL 1.2 builds will be
+discontinued, so maintainers should switch to using SDL 2.0,
+which is the default.
+
+@section System emulator command line arguments
+
+@subsection -no-kvm (since 1.3.0)
+
+The ``-no-kvm'' argument is now a synonym for setting
+``-machine accel=tcg''.
+
+@subsection -vnc tls (since 2.5.0)
+
+The ``-vnc tls'' argument is now a synonym for setting
+``-object tls-creds-anon,id=tls0'' combined with
+``-vnc tls-creds=tls0'
+
+@subsection -vnc x509 (since 2.5.0)
+
+The ``-vnc x509=/path/to/certs'' argument is now a
+synonym for setting
+``-object tls-creds-x509,dir=/path/to/certs,id=tls0,verify-peer=no''
+combined with ``-vnc tls-creds=tls0'
+
+@subsection -vnc x509verify (since 2.5.0)
+
+The ``-vnc x509verify=/path/to/certs'' argument is now a
+synonym for setting
+``-object tls-creds-x509,dir=/path/to/certs,id=tls0,verify-peer=yes''
+combined with ``-vnc tls-creds=tls0'
+
+@subsection -tftp (since 2.6.0)
+
+The ``-tftp /some/dir'' argument is replaced by either
+``-netdev user,id=x,tftp=/some/dir '' (for pluggable NICs, accompanied
+with ``-device ...,netdev=x''), or ``-nic user,tftp=/some/dir''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
+
+@subsection -bootp (since 2.6.0)
+
+The ``-bootp /some/file'' argument is replaced by either
+``-netdev user,id=x,bootp=/some/file '' (for pluggable NICs, accompanied
+with ``-device ...,netdev=x''), or ``-nic user,bootp=/some/file''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
+
+@subsection -redir (since 2.6.0)
+
+The ``-redir [tcp|udp]:hostport:[guestaddr]:guestport'' argument is
+replaced by either
+``-netdev 
user,id=x,hostfwd=[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport''
+(for pluggable NICs, accompanied with ``-device ...,netdev=x'') or
+``-nic user,hostfwd=[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
+
+@subsection -smb (since 2.6.0)
+
+The ``-smb /some/dir'' argument is replaced by either
+``-netdev user,id=x,smb=/some/dir '' (for pluggable NICs, accompanied
+with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
+
+@subsection -drive cyls=...,heads=...,secs=...,trans=... (since 2.10.0)
+
+The drive geometry arguments are replaced by the the geometry arguments
+that can be specified with the ``-device'' parameter.
+
+@subsection -drive serial=... (since 2.10.0)
+
+The drive serial argument is replaced by the the serial argument
+that can be specified with the ``-device'' parameter.
+
+@subsection -drive addr=... (since 2.10.0)
+
+The drive addr argument is replaced by th

[Qemu-devel] [PATCH 01/13] hw/rdma: Make distinction between device init and start modes

[Yuval Shaia]

There are certain operations that are well considered as part of device
configuration while others are needed only when "start" command is
triggered by the guest driver. An example of device initialization step
is msix_init and example of "device start" stage is the creation of a CQ
completion handler thread.

Driver expects such distinction - implement it.

Signed-off-by: Yuval Shaia 
---
 hw/rdma/rdma_backend.c  |  96 +--
 hw/rdma/rdma_backend.h  |   2 +
 hw/rdma/rdma_backend_defs.h |   3 +-
 hw/rdma/vmw/pvrdma_main.c   | 129 +---
 4 files changed, 155 insertions(+), 75 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index e9ced6f9ef..647e16399f 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -35,6 +35,7 @@
 #define VENDOR_ERR_MR_SMALL 0x208
 
 #define THR_NAME_LEN 16
+#define THR_POLL_TO  5000
 
 typedef struct BackendCtx {
 uint64_t req_id;
@@ -91,35 +92,82 @@ static void *comp_handler_thread(void *arg)
 int rc;
 struct ibv_cq *ev_cq;
 void *ev_ctx;
+int flags;
+GPollFD pfds[1];
+
+/* Change to non-blocking mode */
+flags = fcntl(backend_dev->channel->fd, F_GETFL);
+rc = fcntl(backend_dev->channel->fd, F_SETFL, flags | O_NONBLOCK);
+if (rc < 0) {
+pr_dbg("Fail to change to non-blocking mode\n");
+return NULL;
+}
 
 pr_dbg("Starting\n");
 
+pfds[0].fd = backend_dev->channel->fd;
+pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+
+backend_dev->comp_thread.is_running = true;
+
 while (backend_dev->comp_thread.run) {
-pr_dbg("Waiting for completion on channel %p\n", backend_dev->channel);
-rc = ibv_get_cq_event(backend_dev->channel, &ev_cq, &ev_ctx);
-pr_dbg("ibv_get_cq_event=%d\n", rc);
-if (unlikely(rc)) {
-pr_dbg("---> ibv_get_cq_event (%d)\n", rc);
-continue;
-}
+do {
+rc = qemu_poll_ns(pfds, 1, THR_POLL_TO * (int64_t)SCALE_MS);
+} while (!rc && backend_dev->comp_thread.run);
+
+if (backend_dev->comp_thread.run) {
+pr_dbg("Waiting for completion on channel %p\n", 
backend_dev->channel);
+rc = ibv_get_cq_event(backend_dev->channel, &ev_cq, &ev_ctx);
+pr_dbg("ibv_get_cq_event=%d\n", rc);
+if (unlikely(rc)) {
+pr_dbg("---> ibv_get_cq_event (%d)\n", rc);
+continue;
+}
 
-rc = ibv_req_notify_cq(ev_cq, 0);
-if (unlikely(rc)) {
-pr_dbg("Error %d from ibv_req_notify_cq\n", rc);
-}
+rc = ibv_req_notify_cq(ev_cq, 0);
+if (unlikely(rc)) {
+pr_dbg("Error %d from ibv_req_notify_cq\n", rc);
+}
 
-poll_cq(backend_dev->rdma_dev_res, ev_cq);
+poll_cq(backend_dev->rdma_dev_res, ev_cq);
 
-ibv_ack_cq_events(ev_cq, 1);
+ibv_ack_cq_events(ev_cq, 1);
+}
 }
 
 pr_dbg("Going down\n");
 
 /* TODO: Post cqe for all remaining buffs that were posted */
 
+backend_dev->comp_thread.is_running = false;
+
+qemu_thread_exit(0);
+
 return NULL;
 }
 
+static void stop_comp_thread(RdmaBackendDev *backend_dev)
+{
+backend_dev->comp_thread.run = false;
+while (backend_dev->comp_thread.is_running) {
+pr_dbg("Waiting for thread to complete\n");
+sleep(THR_POLL_TO / SCALE_US / 2);
+}
+}
+
+static void start_comp_thread(RdmaBackendDev *backend_dev)
+{
+char thread_name[THR_NAME_LEN] = {0};
+
+stop_comp_thread(backend_dev);
+
+snprintf(thread_name, sizeof(thread_name), "rdma_comp_%s",
+ ibv_get_device_name(backend_dev->ib_dev));
+backend_dev->comp_thread.run = true;
+qemu_thread_create(&backend_dev->comp_thread.thread, thread_name,
+   comp_handler_thread, backend_dev, QEMU_THREAD_DETACHED);
+}
+
 void rdma_backend_register_comp_handler(void (*handler)(int status,
 unsigned int vendor_err, void *ctx))
 {
@@ -706,7 +754,6 @@ int rdma_backend_init(RdmaBackendDev *backend_dev,
 int i;
 int ret = 0;
 int num_ibv_devices;
-char thread_name[THR_NAME_LEN] = {0};
 struct ibv_device **dev_list;
 struct ibv_port_attr port_attr;
 
@@ -800,11 +847,8 @@ int rdma_backend_init(RdmaBackendDev *backend_dev,
 pr_dbg("interface_id=0x%" PRIx64 "\n",
be64_to_cpu(backend_dev->gid.global.interface_id));
 
-snprintf(thread_name, sizeof(thread_name), "rdma_comp_%s",
- ibv_get_device_name(backend_dev->ib_dev));
-backend_dev->comp_thread.run = true;
-qemu_thread_create(&backend_dev->comp_thread.thread, thread_name,
-   comp_handler_thread, backend_dev, QEMU_THREAD_DETACHED);
+backend_dev->comp_thread.run = false;
+backend_dev->comp_thread.is_running = false;
 
 ah_cache_init();
 
@@ -823,8 +867,22 @@ out:
 return r

[Qemu-devel] [PATCH 02/13] hw/pvrdma: Bugfix - provide the correct attr_mask to query_qp

[Yuval Shaia]

Calling rdma_rm_query_qp with attr_mask equals to -1 leads to error
where backend query_qp fails to retrieve the needed QP attributes.
Fix it by providing the attr_mask we got from driver.

Signed-off-by: Yuval Shaia 
---
 hw/rdma/vmw/pvrdma_cmd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
index 14255d609f..e7d6589cdc 100644
--- a/hw/rdma/vmw/pvrdma_cmd.c
+++ b/hw/rdma/vmw/pvrdma_cmd.c
@@ -524,6 +524,7 @@ static int query_qp(PVRDMADev *dev, union pvrdma_cmd_req 
*req,
 struct ibv_qp_init_attr init_attr;
 
 pr_dbg("qp_handle=%d\n", cmd->qp_handle);
+pr_dbg("attr_mask=0x%x\n", cmd->attr_mask);
 
 memset(rsp, 0, sizeof(*rsp));
 rsp->hdr.response = cmd->hdr.response;
@@ -531,8 +532,8 @@ static int query_qp(PVRDMADev *dev, union pvrdma_cmd_req 
*req,
 
 rsp->hdr.err = rdma_rm_query_qp(&dev->rdma_dev_res, &dev->backend_dev,
 cmd->qp_handle,
-(struct ibv_qp_attr *)&resp->attrs, -1,
-&init_attr);
+(struct ibv_qp_attr *)&resp->attrs,
+cmd->attr_mask, &init_attr);
 
 pr_dbg("ret=%d\n", rsp->hdr.err);
 return rsp->hdr.err;
-- 
2.17.1

[Qemu-devel] [PATCH 00/13] Misc fixes for pvrdma device

[Yuval Shaia]

Hi,
Please review some changes i've made for pvrdma device.

Thanks,
Yuval

[Qemu-devel] [PATCH 01/13] hw/rdma: Make distinction between device init and 
start
[Qemu-devel] [PATCH 02/13] hw/pvrdma: Bugfix - provide the correct attr_mask to
[Qemu-devel] [PATCH 03/13] hw/rdma: Modify debug macros
[Qemu-devel] [PATCH 04/13] hw/pvrdma: Clean CQE before use
[Qemu-devel] [PATCH 05/13] hw/pvrdma: Make default pkey 0x
[Qemu-devel] [PATCH 06/13] hw/rdma: Get rid of unneeded structure
[Qemu-devel] [PATCH 07/13] hw/rdma: Do not allocate memory for non-dma MR
[Qemu-devel] [PATCH 08/13] hw/rdma: Reorder resource cleanup
[Qemu-devel] [PATCH 09/13] hw/pvrdma: Cosmetic change - indent right
[Qemu-devel] [PATCH 10/13] hw/rdma: Cosmetic change - move to generic function
[Qemu-devel] [PATCH 11/13] hw/rdma: Print backend QP number in hex format
[Qemu-devel] [PATCH 12/13] hw/rdma: Bugfix: Support non-aligned buffers
[Qemu-devel] [PATCH 13/13] hw/rdma: Save pci dev in backend_dev

[Qemu-devel] [PATCH 05/13] hw/pvrdma: Make default pkey 0xFFFF

[Yuval Shaia]

0x7FFF is not the default pkey - fix it.

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

diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
index e7d6589cdc..bb898265a3 100644
--- a/hw/rdma/vmw/pvrdma_cmd.c
+++ b/hw/rdma/vmw/pvrdma_cmd.c
@@ -166,7 +166,7 @@ static int query_pkey(PVRDMADev *dev, union pvrdma_cmd_req 
*req,
 resp->hdr.ack = PVRDMA_CMD_QUERY_PKEY_RESP;
 resp->hdr.err = 0;
 
-resp->pkey = 0x7FFF;
+resp->pkey = 0x;
 pr_dbg("pkey=0x%x\n", resp->pkey);
 
 return 0;
-- 
2.17.1

[Qemu-devel] [PATCH 03/13] hw/rdma: Modify debug macros

[Yuval Shaia]

- Add line counter to ease navigation in log
- Print rdma instead of pvrdma

Signed-off-by: Yuval Shaia 
---
 hw/rdma/rdma_utils.c  |  4 
 hw/rdma/rdma_utils.h  | 16 
 hw/rdma/vmw/pvrdma_main.c |  2 ++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/hw/rdma/rdma_utils.c b/hw/rdma/rdma_utils.c
index d713f635f1..dc23f158f3 100644
--- a/hw/rdma/rdma_utils.c
+++ b/hw/rdma/rdma_utils.c
@@ -15,6 +15,10 @@
 
 #include "rdma_utils.h"
 
+#ifdef PVRDMA_DEBUG
+unsigned long pr_dbg_cnt;
+#endif
+
 void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen)
 {
 void *p;
diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h
index 3dc07891bc..04c7c2ef5b 100644
--- a/hw/rdma/rdma_utils.h
+++ b/hw/rdma/rdma_utils.h
@@ -22,18 +22,26 @@
 #include "sysemu/dma.h"
 
 #define pr_info(fmt, ...) \
-fprintf(stdout, "%s: %-20s (%3d): " fmt, "pvrdma",  __func__, __LINE__,\
+fprintf(stdout, "%s: %-20s (%3d): " fmt, "rdma",  __func__, __LINE__,\
## __VA_ARGS__)
 
 #define pr_err(fmt, ...) \
-fprintf(stderr, "%s: Error at %-20s (%3d): " fmt, "pvrdma", __func__, \
+fprintf(stderr, "%s: Error at %-20s (%3d): " fmt, "rdma", __func__, \
 __LINE__, ## __VA_ARGS__)
 
 #ifdef PVRDMA_DEBUG
+extern unsigned long pr_dbg_cnt;
+
+#define init_pr_dbg(void) \
+{ \
+pr_dbg_cnt = 0; \
+}
+
 #define pr_dbg(fmt, ...) \
-fprintf(stdout, "%s: %-20s (%3d): " fmt, "pvrdma", __func__, __LINE__,\
-   ## __VA_ARGS__)
+fprintf(stdout, "%lx %ld: %-20s (%3d): " fmt, pthread_self(), 
pr_dbg_cnt++, \
+__func__, __LINE__, ## __VA_ARGS__)
 #else
+#define init_pr_dbg(void)
 #define pr_dbg(fmt, ...)
 #endif
 
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 6a5073974d..1b1330e113 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -577,6 +577,8 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
 Object *memdev_root;
 bool ram_shared = false;
 
+init_pr_dbg();
+
 pr_dbg("Initializing device %s %x.%x\n", pdev->name,
PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
 
-- 
2.17.1

[Qemu-devel] [PATCH 08/13] hw/rdma: Reorder resource cleanup

[Yuval Shaia]

To be consistence with allocation do the reverse order in deallocation

Signed-off-by: Yuval Shaia 
---
 hw/rdma/rdma_rm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index bf4a5c71b4..1f014b4ab2 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -543,8 +543,9 @@ void rdma_rm_fini(RdmaDeviceResources *dev_res)
 res_tbl_free(&dev_res->uc_tbl);
 res_tbl_free(&dev_res->cqe_ctx_tbl);
 res_tbl_free(&dev_res->qp_tbl);
-res_tbl_free(&dev_res->cq_tbl);
 res_tbl_free(&dev_res->mr_tbl);
+res_tbl_free(&dev_res->cq_tbl);
 res_tbl_free(&dev_res->pd_tbl);
+
 g_hash_table_destroy(dev_res->qp_hash);
 }
-- 
2.17.1

[Qemu-devel] [PATCH 09/13] hw/pvrdma: Cosmetic change - indent right

[Yuval Shaia]

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

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 1b1330e113..3d448bffc4 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -430,7 +430,7 @@ static void regs_write(void *opaque, hwaddr addr, uint64_t 
val, unsigned size)
 reset_device(dev);
 break;
 }
-break;
+break;
 case PVRDMA_REG_IMR:
 pr_dbg("Interrupt mask=0x%" PRIx64 "\n", val);
 dev->interrupt_mask = val;
@@ -439,7 +439,7 @@ static void regs_write(void *opaque, hwaddr addr, uint64_t 
val, unsigned size)
 if (val == 0) {
 execute_command(dev);
 }
-break;
+break;
 default:
 break;
 }
-- 
2.17.1

[Qemu-devel] [PATCH 07/13] hw/rdma: Do not allocate memory for non-dma MR

[Yuval Shaia]

There is no use in the memory allocated for non-dma MR (one with
host_virt equals to NULL).
Delete the code that allocates it.

Signed-off-by: Yuval Shaia 
---
 hw/rdma/rdma_rm.c | 52 +++
 1 file changed, 21 insertions(+), 31 deletions(-)

diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index 7403d24674..bf4a5c71b4 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -144,8 +144,6 @@ int rdma_rm_alloc_mr(RdmaDeviceResources *dev_res, uint32_t 
pd_handle,
 RdmaRmMR *mr;
 int ret = 0;
 RdmaRmPD *pd;
-void *addr;
-size_t length;
 
 pd = rdma_rm_get_pd(dev_res, pd_handle);
 if (!pd) {
@@ -158,40 +156,29 @@ int rdma_rm_alloc_mr(RdmaDeviceResources *dev_res, 
uint32_t pd_handle,
 pr_dbg("Failed to allocate obj in table\n");
 return -ENOMEM;
 }
+pr_dbg("mr_handle=%d\n", *mr_handle);
 
-if (!host_virt) {
-/* TODO: This is my guess but not so sure that this needs to be
- * done */
-length = TARGET_PAGE_SIZE;
-addr = g_malloc(length);
-} else {
+pr_dbg("host_virt=0x%p\n", host_virt);
+pr_dbg("guest_start=0x%" PRIx64 "\n", guest_start);
+pr_dbg("length=%zu\n", guest_length);
+
+if (host_virt) {
 mr->virt = host_virt;
-pr_dbg("host_virt=0x%p\n", mr->virt);
-mr->length = guest_length;
-pr_dbg("length=%zu\n", guest_length);
 mr->start = guest_start;
-pr_dbg("guest_start=0x%" PRIx64 "\n", mr->start);
-
-length = mr->length;
-addr = mr->virt;
-}
+mr->length = guest_length;
 
-ret = rdma_backend_create_mr(&mr->backend_mr, &pd->backend_pd, addr, 
length,
- access_flags);
-if (ret) {
-pr_dbg("Fail in rdma_backend_create_mr, err=%d\n", ret);
-ret = -EIO;
-goto out_dealloc_mr;
+ret = rdma_backend_create_mr(&mr->backend_mr, &pd->backend_pd, 
mr->virt,
+ mr->length, access_flags);
+if (ret) {
+pr_dbg("Fail in rdma_backend_create_mr, err=%d\n", ret);
+ret = -EIO;
+goto out_dealloc_mr;
+}
 }
 
-if (!host_virt) {
-*lkey = mr->lkey = rdma_backend_mr_lkey(&mr->backend_mr);
-*rkey = mr->rkey = rdma_backend_mr_rkey(&mr->backend_mr);
-} else {
-/* We keep mr_handle in lkey so send and recv get get mr ptr */
-*lkey = *mr_handle;
-*rkey = -1;
-}
+/* We keep mr_handle in lkey so send and recv get get mr ptr */
+*lkey = *mr_handle;
+*rkey = -1;
 
 mr->pd_handle = pd_handle;
 
@@ -214,7 +201,10 @@ void rdma_rm_dealloc_mr(RdmaDeviceResources *dev_res, 
uint32_t mr_handle)
 
 if (mr) {
 rdma_backend_destroy_mr(&mr->backend_mr);
-munmap(mr->virt, mr->length);
+pr_dbg("start=0x%" PRIx64 "\n", mr->start);
+if (mr->start) {
+munmap(mr->virt, mr->length);
+}
 res_tbl_dealloc(&dev_res->mr_tbl, mr_handle);
 }
 }
-- 
2.17.1

[Qemu-devel] [PATCH 04/13] hw/pvrdma: Clean CQE before use

[Yuval Shaia]

Next CQE is fetched from CQ ring, clean it before usage as it still
carries old CQE values.

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

diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
index 99bb5e..a8664f40c8 100644
--- a/hw/rdma/vmw/pvrdma_qp_ops.c
+++ b/hw/rdma/vmw/pvrdma_qp_ops.c
@@ -69,6 +69,7 @@ static int pvrdma_post_cqe(PVRDMADev *dev, uint32_t cq_handle,
 return -EINVAL;
 }
 
+memset(cqe1, 0, sizeof(*cqe1));
 cqe1->wr_id = cqe->wr_id;
 cqe1->qp = cqe->qp;
 cqe1->opcode = cqe->opcode;
-- 
2.17.1

[Qemu-devel] [PATCH 13/13] hw/rdma: Save pci dev in backend_dev

[Yuval Shaia]

This field is not initialized - fix it.

Signed-off-by: Yuval Shaia 
---
 hw/rdma/rdma_backend.c| 6 +-
 hw/rdma/rdma_backend.h| 2 +-
 hw/rdma/vmw/pvrdma_main.c | 2 +-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index d29acc505b..d7a4bbd91f 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -744,7 +744,7 @@ static int init_device_caps(RdmaBackendDev *backend_dev,
 return 0;
 }
 
-int rdma_backend_init(RdmaBackendDev *backend_dev,
+int rdma_backend_init(RdmaBackendDev *backend_dev, PCIDevice *pdev,
   RdmaDeviceResources *rdma_dev_res,
   const char *backend_device_name, uint8_t port_num,
   uint8_t backend_gid_idx, struct ibv_device_attr 
*dev_attr,
@@ -756,6 +756,10 @@ int rdma_backend_init(RdmaBackendDev *backend_dev,
 struct ibv_device **dev_list;
 struct ibv_port_attr port_attr;
 
+memset(backend_dev, 0, sizeof(*backend_dev));
+
+backend_dev->dev = pdev;
+
 backend_dev->backend_gid_idx = backend_gid_idx;
 backend_dev->port_num = port_num;
 backend_dev->rdma_dev_res = rdma_dev_res;
diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h
index 3049a73962..86e8fe8ab6 100644
--- a/hw/rdma/rdma_backend.h
+++ b/hw/rdma/rdma_backend.h
@@ -46,7 +46,7 @@ static inline uint32_t rdma_backend_mr_rkey(const 
RdmaBackendMR *mr)
 return mr->ibmr ? mr->ibmr->rkey : 0;
 }
 
-int rdma_backend_init(RdmaBackendDev *backend_dev,
+int rdma_backend_init(RdmaBackendDev *backend_dev, PCIDevice *pdev,
   RdmaDeviceResources *rdma_dev_res,
   const char *backend_device_name, uint8_t port_num,
   uint8_t backend_gid_idx, struct ibv_device_attr 
*dev_attr,
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 3d448bffc4..ca5fa8d981 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -611,7 +611,7 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
 goto out;
 }
 
-rc = rdma_backend_init(&dev->backend_dev, &dev->rdma_dev_res,
+rc = rdma_backend_init(&dev->backend_dev, pdev, &dev->rdma_dev_res,
dev->backend_device_name, dev->backend_port_num,
dev->backend_gid_idx, &dev->dev_attr, errp);
 if (rc) {
-- 
2.17.1

[Qemu-devel] [PATCH 11/13] hw/rdma: Print backend QP number in hex format

[Yuval Shaia]

To be consistent with other prints throughout the code fix places that
print it as decimal number.

Signed-off-by: Yuval Shaia 
---
 hw/rdma/rdma_rm.c   | 4 ++--
 hw/rdma/vmw/pvrdma_qp_ops.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index 1f014b4ab2..859c93 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -389,7 +389,7 @@ int rdma_rm_modify_qp(RdmaDeviceResources *dev_res, 
RdmaBackendDev *backend_dev,
 RdmaRmQP *qp;
 int ret;
 
-pr_dbg("qpn=%d\n", qp_handle);
+pr_dbg("qpn=0x%x\n", qp_handle);
 
 qp = rdma_rm_get_qp(dev_res, qp_handle);
 if (!qp) {
@@ -447,7 +447,7 @@ int rdma_rm_query_qp(RdmaDeviceResources *dev_res, 
RdmaBackendDev *backend_dev,
 {
 RdmaRmQP *qp;
 
-pr_dbg("qpn=%d\n", qp_handle);
+pr_dbg("qpn=0x%x\n", qp_handle);
 
 qp = rdma_rm_get_qp(dev_res, qp_handle);
 if (!qp) {
diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
index a8664f40c8..c668afd0ed 100644
--- a/hw/rdma/vmw/pvrdma_qp_ops.c
+++ b/hw/rdma/vmw/pvrdma_qp_ops.c
@@ -130,7 +130,7 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle)
 PvrdmaSqWqe *wqe;
 PvrdmaRing *ring;
 
-pr_dbg("qp_handle=%d\n", qp_handle);
+pr_dbg("qp_handle=0x%x\n", qp_handle);
 
 qp = rdma_rm_get_qp(&dev->rdma_dev_res, qp_handle);
 if (unlikely(!qp)) {
@@ -174,7 +174,7 @@ int pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle)
 PvrdmaRqWqe *wqe;
 PvrdmaRing *ring;
 
-pr_dbg("qp_handle=%d\n", qp_handle);
+pr_dbg("qp_handle=0x%x\n", qp_handle);
 
 qp = rdma_rm_get_qp(&dev->rdma_dev_res, qp_handle);
 if (unlikely(!qp)) {
-- 
2.17.1

[Qemu-devel] [PATCH 10/13] hw/rdma: Cosmetic change - move to generic function

[Yuval Shaia]

To ease maintenance of struct comp_thread move all related code to
dedicated function.

Signed-off-by: Yuval Shaia 
---
 hw/rdma/rdma_backend.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 52981d652d..d29acc505b 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -146,10 +146,10 @@ static void *comp_handler_thread(void *arg)
 return NULL;
 }
 
-static void stop_comp_thread(RdmaBackendDev *backend_dev)
+static void stop_backend_thread(RdmaBackendThread *thread)
 {
-backend_dev->comp_thread.run = false;
-while (backend_dev->comp_thread.is_running) {
+thread->run = false;
+while (thread->is_running) {
 pr_dbg("Waiting for thread to complete\n");
 sleep(THR_POLL_TO / SCALE_US / 2);
 }
@@ -159,7 +159,7 @@ static void start_comp_thread(RdmaBackendDev *backend_dev)
 {
 char thread_name[THR_NAME_LEN] = {0};
 
-stop_comp_thread(backend_dev);
+stop_backend_thread(&backend_dev->comp_thread);
 
 snprintf(thread_name, sizeof(thread_name), "rdma_comp_%s",
  ibv_get_device_name(backend_dev->ib_dev));
@@ -876,7 +876,7 @@ void rdma_backend_start(RdmaBackendDev *backend_dev)
 void rdma_backend_stop(RdmaBackendDev *backend_dev)
 {
 pr_dbg("Stopping rdma_backend\n");
-stop_comp_thread(backend_dev);
+stop_backend_thread(&backend_dev->comp_thread);
 }
 
 void rdma_backend_fini(RdmaBackendDev *backend_dev)
-- 
2.17.1

[Qemu-devel] [PATCH 06/13] hw/rdma: Get rid of unneeded structure

[Yuval Shaia]

This structure make no sense - removing it.

Signed-off-by: Yuval Shaia 
---
 hw/rdma/rdma_backend.c |  3 +--
 hw/rdma/rdma_rm.c  | 16 
 hw/rdma/rdma_rm_defs.h | 10 +++---
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 647e16399f..52981d652d 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -271,8 +271,7 @@ static int build_host_sge_array(RdmaDeviceResources 
*rdma_dev_res,
 return VENDOR_ERR_INVLKEY | ssge[ssge_idx].lkey;
 }
 
-dsge->addr = (uintptr_t)mr->user_mr.host_virt + ssge[ssge_idx].addr -
- mr->user_mr.guest_start;
+dsge->addr = (uintptr_t)mr->virt + ssge[ssge_idx].addr - mr->start;
 dsge->length = ssge[ssge_idx].length;
 dsge->lkey = rdma_backend_mr_lkey(&mr->backend_mr);
 
diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index 415da15efe..7403d24674 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -165,15 +165,15 @@ int rdma_rm_alloc_mr(RdmaDeviceResources *dev_res, 
uint32_t pd_handle,
 length = TARGET_PAGE_SIZE;
 addr = g_malloc(length);
 } else {
-mr->user_mr.host_virt = host_virt;
-pr_dbg("host_virt=0x%p\n", mr->user_mr.host_virt);
-mr->user_mr.length = guest_length;
+mr->virt = host_virt;
+pr_dbg("host_virt=0x%p\n", mr->virt);
+mr->length = guest_length;
 pr_dbg("length=%zu\n", guest_length);
-mr->user_mr.guest_start = guest_start;
-pr_dbg("guest_start=0x%" PRIx64 "\n", mr->user_mr.guest_start);
+mr->start = guest_start;
+pr_dbg("guest_start=0x%" PRIx64 "\n", mr->start);
 
-length = mr->user_mr.length;
-addr = mr->user_mr.host_virt;
+length = mr->length;
+addr = mr->virt;
 }
 
 ret = rdma_backend_create_mr(&mr->backend_mr, &pd->backend_pd, addr, 
length,
@@ -214,7 +214,7 @@ void rdma_rm_dealloc_mr(RdmaDeviceResources *dev_res, 
uint32_t mr_handle)
 
 if (mr) {
 rdma_backend_destroy_mr(&mr->backend_mr);
-munmap(mr->user_mr.host_virt, mr->user_mr.length);
+munmap(mr->virt, mr->length);
 res_tbl_dealloc(&dev_res->mr_tbl, mr_handle);
 }
 }
diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
index 226011176d..7228151239 100644
--- a/hw/rdma/rdma_rm_defs.h
+++ b/hw/rdma/rdma_rm_defs.h
@@ -55,16 +55,12 @@ typedef struct RdmaRmCQ {
 bool notify;
 } RdmaRmCQ;
 
-typedef struct RdmaRmUserMR {
-void *host_virt;
-uint64_t guest_start;
-size_t length;
-} RdmaRmUserMR;
-
 /* MR (DMA region) */
 typedef struct RdmaRmMR {
 RdmaBackendMR backend_mr;
-RdmaRmUserMR user_mr;
+void *virt;
+uint64_t start;
+size_t length;
 uint32_t pd_handle;
 uint32_t lkey;
 uint32_t rkey;
-- 
2.17.1

[Qemu-devel] [PATCH 12/13] hw/rdma: Bugfix: Support non-aligned buffers

[Yuval Shaia]

RDMA applications can provide non-aligned buffers to be registered. In
such case the DMA address passed by driver is pointing to the beginning
of the physical address of the mapped page so we can't distinguish
between two addresses from the same page.

Fix it by keeping the offset of the virtual address in mr->virt.

Signed-off-by: Yuval Shaia 
---
 hw/rdma/rdma_rm.c| 2 ++
 hw/rdma/vmw/pvrdma_cmd.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index 859c93..8d59a42cd1 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -166,6 +166,7 @@ int rdma_rm_alloc_mr(RdmaDeviceResources *dev_res, uint32_t 
pd_handle,
 mr->virt = host_virt;
 mr->start = guest_start;
 mr->length = guest_length;
+mr->virt += (mr->start & (TARGET_PAGE_SIZE - 1));
 
 ret = rdma_backend_create_mr(&mr->backend_mr, &pd->backend_pd, 
mr->virt,
  mr->length, access_flags);
@@ -203,6 +204,7 @@ void rdma_rm_dealloc_mr(RdmaDeviceResources *dev_res, 
uint32_t mr_handle)
 rdma_backend_destroy_mr(&mr->backend_mr);
 pr_dbg("start=0x%" PRIx64 "\n", mr->start);
 if (mr->start) {
+mr->virt -= (mr->start & (TARGET_PAGE_SIZE - 1));
 munmap(mr->virt, mr->length);
 }
 res_tbl_dealloc(&dev_res->mr_tbl, mr_handle);
diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
index bb898265a3..4ff242e793 100644
--- a/hw/rdma/vmw/pvrdma_cmd.c
+++ b/hw/rdma/vmw/pvrdma_cmd.c
@@ -59,6 +59,7 @@ static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t 
pdir_dma,
 }
 
 host_virt = mremap(curr_page, 0, length, MREMAP_MAYMOVE);
+pr_dbg("mremap %p -> %p\n", curr_page, host_virt);
 if (host_virt == MAP_FAILED) {
 host_virt = NULL;
 error_report("PVRDMA: Failed to remap memory for host_virt");
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/2] qemu-doc: Move appendix "Deprecated features" to its own file

[Thomas Huth]

On 16.07.2018 09:32, Markus Armbruster wrote:
> Consumers of QEMU need to track feature deprecation.  Keeping
> deprecation documentation in its own file helps in two small ways:
> 
> * You can track changes the easy and obvious way, with git-log.
>   Before, you had to resort to more complex gittery like "git-log
>   --oneline -L '/@node Deprecated features/,/@node Supported build
>   platforms/:qemu-doc.texi'"
> 
> * It lets us use MAINTAINERS to copy interested parties on deprecation
>   patches, so they can advise or object before they're a done deal.
>   The next commit will do that for libvirt.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  qemu-deprecated.texi | 234 ++
>  qemu-doc.texi| 235 +--
>  2 files changed, 235 insertions(+), 234 deletions(-)
>  create mode 100644 qemu-deprecated.texi

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

[Thomas Huth]

On 16.07.2018 09:32, Markus Armbruster wrote:
> Libvirt developers would like to be copied on patches to qemu-doc
> appendix "Deprecated features".  Do them the favor.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  MAINTAINERS | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 20eef3cb61..666e936812 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2194,6 +2194,10 @@ M: Daniel P. Berrange 
>  S: Odd Fixes
>  F: docs/devel/build-system.txt
>  
> +Incompatible changes
> +R: libvir-l...@redhat.com
> +F: qemu-deprecated.texi

Should we have a maintainer for the file, too? (I guess not, because
deprecation patches should go through the specific subsystems...)
And what about a "S:" line?

Anyway:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH 1/2] qemu-doc: Move appendix "Deprecated features" to its own file

[Cornelia Huck]

On Mon, 16 Jul 2018 09:32:25 +0200
Markus Armbruster  wrote:

> Consumers of QEMU need to track feature deprecation.  Keeping
> deprecation documentation in its own file helps in two small ways:
> 
> * You can track changes the easy and obvious way, with git-log.
>   Before, you had to resort to more complex gittery like "git-log
>   --oneline -L '/@node Deprecated features/,/@node Supported build
>   platforms/:qemu-doc.texi'"
> 
> * It lets us use MAINTAINERS to copy interested parties on deprecation
>   patches, so they can advise or object before they're a done deal.
>   The next commit will do that for libvirt.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  qemu-deprecated.texi | 234 ++
>  qemu-doc.texi| 235 +--
>  2 files changed, 235 insertions(+), 234 deletions(-)
>  create mode 100644 qemu-deprecated.texi

Reviewed-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

[Cornelia Huck]

On Mon, 16 Jul 2018 09:32:26 +0200
Markus Armbruster  wrote:

> Libvirt developers would like to be copied on patches to qemu-doc
> appendix "Deprecated features".  Do them the favor.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  MAINTAINERS | 4 
>  1 file changed, 4 insertions(+)

Reviewed-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

[Cornelia Huck]

On Mon, 16 Jul 2018 09:54:12 +0200
Thomas Huth  wrote:

> On 16.07.2018 09:32, Markus Armbruster wrote:
> > Libvirt developers would like to be copied on patches to qemu-doc
> > appendix "Deprecated features".  Do them the favor.
> > 
> > Signed-off-by: Markus Armbruster 
> > ---
> >  MAINTAINERS | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 20eef3cb61..666e936812 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2194,6 +2194,10 @@ M: Daniel P. Berrange 
> >  S: Odd Fixes
> >  F: docs/devel/build-system.txt
> >  
> > +Incompatible changes
> > +R: libvir-l...@redhat.com
> > +F: qemu-deprecated.texi  
> 
> Should we have a maintainer for the file, too? (I guess not, because
> deprecation patches should go through the specific subsystems...)

I don't think adding a maintainer makes sense for this file.

> And what about a "S:" line?

I don't think that makes too much sense, either.

If anything, qemu-deprecated.texi should be in a category 'maintained
by everybody', i.e. qemu-devel. Just like qemu-doc.texi, which does not
have an entry in MAINTAINERS at all.

Re: [Qemu-devel] [RFC v3 1/2] libnvdimm: Add flush callback for virtio pmem

[Pankaj Gupta]

Hi Luiz,

> 
> > This patch adds functionality to perform flush from guest to host
> > over VIRTIO. We are registering a callback based on 'nd_region' type.
> > As virtio_pmem driver requires this special flush interface, for rest
> > of the region types we are registering existing flush function.
> > Also report the error returned by virtio flush interface.
> 
> This patch doesn't apply against latest upstream. A few more comments
> below.

My bad, I tested it with 4.17-rc1. Will rebase it.

> 
> > 
> > Signed-off-by: Pankaj Gupta 
> > ---
> >  drivers/nvdimm/nd.h  |  1 +
> >  drivers/nvdimm/pmem.c|  4 ++--
> >  drivers/nvdimm/region_devs.c | 24 ++--
> >  include/linux/libnvdimm.h|  5 -
> >  4 files changed, 25 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> > index 32e0364..1b62f79 100644
> > --- a/drivers/nvdimm/nd.h
> > +++ b/drivers/nvdimm/nd.h
> > @@ -159,6 +159,7 @@ struct nd_region {
> > struct badblocks bb;
> > struct nd_interleave_set *nd_set;
> > struct nd_percpu_lane __percpu *lane;
> > +   int (*flush)(struct device *dev);
> > struct nd_mapping mapping[0];
> >  };
> >  
> > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> > index 9d71492..29fd2cd 100644
> > --- a/drivers/nvdimm/pmem.c
> > +++ b/drivers/nvdimm/pmem.c
> > @@ -180,7 +180,7 @@ static blk_qc_t pmem_make_request(struct request_queue
> > *q, struct bio *bio)
> > struct nd_region *nd_region = to_region(pmem);
> >  
> > if (bio->bi_opf & REQ_FLUSH)
> > -   nvdimm_flush(nd_region);
> > +   bio->bi_status = nvdimm_flush(nd_region);
> >  
> > do_acct = nd_iostat_start(bio, &start);
> > bio_for_each_segment(bvec, bio, iter) {
> > @@ -196,7 +196,7 @@ static blk_qc_t pmem_make_request(struct request_queue
> > *q, struct bio *bio)
> > nd_iostat_end(bio, start);
> >  
> > if (bio->bi_opf & REQ_FUA)
> > -   nvdimm_flush(nd_region);
> > +   bio->bi_status = nvdimm_flush(nd_region);
> >  
> > bio_endio(bio);
> > return BLK_QC_T_NONE;
> > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> > index a612be6..124aae7 100644
> > --- a/drivers/nvdimm/region_devs.c
> > +++ b/drivers/nvdimm/region_devs.c
> > @@ -1025,6 +1025,7 @@ static struct nd_region *nd_region_create(struct
> > nvdimm_bus *nvdimm_bus,
> > dev->of_node = ndr_desc->of_node;
> > nd_region->ndr_size = resource_size(ndr_desc->res);
> > nd_region->ndr_start = ndr_desc->res->start;
> > +   nd_region->flush = ndr_desc->flush;
> > nd_device_register(dev);
> >  
> > return nd_region;
> > @@ -1065,13 +1066,10 @@ struct nd_region
> > *nvdimm_volatile_region_create(struct nvdimm_bus *nvdimm_bus,
> >  }
> >  EXPORT_SYMBOL_GPL(nvdimm_volatile_region_create);
> >  
> > -/**
> > - * nvdimm_flush - flush any posted write queues between the cpu and pmem
> > media
> > - * @nd_region: blk or interleaved pmem region
> > - */
> > -void nvdimm_flush(struct nd_region *nd_region)
> > +void pmem_flush(struct device *dev)
> >  {
> > -   struct nd_region_data *ndrd = dev_get_drvdata(&nd_region->dev);
> > +   struct nd_region_data *ndrd = dev_get_drvdata(dev);
> > +   struct nd_region *nd_region = to_nd_region(dev);
> > int i, idx;
> >  
> > /*
> > @@ -1094,6 +1092,20 @@ void nvdimm_flush(struct nd_region *nd_region)
> > writeq(1, ndrd_get_flush_wpq(ndrd, i, idx));
> > wmb();
> >  }
> > +
> > +/**
> > + * nvdimm_flush - flush any posted write queues between the cpu and pmem
> > media
> > + * @nd_region: blk or interleaved pmem region
> > + */
> > +int nvdimm_flush(struct nd_region *nd_region)
> > +{
> > +   if (nd_region->flush)
> > +   return(nd_region->flush(&nd_region->dev));
> > +
> > +   pmem_flush(&nd_region->dev);
> 
> IMHO, a better way of doing this would be to allow nvdimm_flush() to
> be overridden. That is, in nd_region_create() you set nd_region->flush
> to the original nvdimm_flush() if ndr_desc->flush is NULL. And then
> always call nd_region->flush() where nvdimm_flush() is called today.

I wanted to do minimal changes for actual 'nvdimm_flush' function because it
does not return an error or return status for fsync. So, I needed to 
differentiate
between 'fake DAX' & 'NVDIMM' at the time of calling 'flush', otherwise I need 
to 
change 'nvdimm_flush' to return zero for all the calls.

Looks like I am already doing this, will change as suggested.  
 
> 
> > +
> > +   return 0;
> > +}
> >  EXPORT_SYMBOL_GPL(nvdimm_flush);
> >  
> >  /**
> > diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
> > index 097072c..33b617f 100644
> > --- a/include/linux/libnvdimm.h
> > +++ b/include/linux/libnvdimm.h
> > @@ -126,6 +126,7 @@ struct nd_region_desc {
> > int numa_node;
> > unsigned long flags;
> > struct device_node *of_node;
> > +   int (*flush)(struct device *dev);
> >  };
> >  
> >  struct device;
> > @@ -

[Qemu-devel] [PATCH v2 2/2] virtio-scsi: fix hotplug ->reset() vs event race

[Stefan Hajnoczi]

There is a race condition during hotplug when iothread is used.  It
occurs because virtio-scsi may be processing command queues in the
iothread while the monitor performs SCSI device hotplug.

When a SCSI device is hotplugged the HotplugHandler->plug() callback is
invoked and virtio-scsi emits a rescan event to the guest.

If the guest submits a SCSI command at this point then it may be
cancelled before hotplug completes.  This happens because ->reset() is
called by hw/core/qdev.c:device_set_realized() after
HotplugHandler->plug() has been called and
hw/scsi/scsi-disk.c:scsi_disk_reset() purges all requests.

This patch uses the new HotplugHandler->post_plug() callback to emit the
rescan event after ->reset().  This eliminates the race conditions where
requests could be cancelled.

Reported-by: l00284672 
Cc: Paolo Bonzini 
Cc: Fam Zheng 
Signed-off-by: Stefan Hajnoczi 
---
 hw/scsi/virtio-scsi.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3aa99717e2..5a3057d1f8 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -797,8 +797,16 @@ static void virtio_scsi_hotplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 virtio_scsi_acquire(s);
 blk_set_aio_context(sd->conf.blk, s->ctx);
 virtio_scsi_release(s);
-
 }
+}
+
+/* Announce the new device after it has been plugged */
+static void virtio_scsi_post_hotplug(HotplugHandler *hotplug_dev,
+ DeviceState *dev)
+{
+VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev);
+VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+SCSIDevice *sd = SCSI_DEVICE(dev);
 
 if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
 virtio_scsi_acquire(s);
@@ -968,6 +976,7 @@ static void virtio_scsi_class_init(ObjectClass *klass, void 
*data)
 vdc->start_ioeventfd = virtio_scsi_dataplane_start;
 vdc->stop_ioeventfd = virtio_scsi_dataplane_stop;
 hc->plug = virtio_scsi_hotplug;
+hc->post_plug = virtio_scsi_post_hotplug;
 hc->unplug = virtio_scsi_hotunplug;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH v2 1/2] qdev: add HotplugHandler->post_plug() callback

[Stefan Hajnoczi]

The ->pre_plug() callback is invoked before the device is realized.  The
->plug() callback is invoked when the device is being realized but
before it is reset.

This patch adds a ->post_plug() callback which is invoked after the
device has been reset.  This callback is needed by HotplugHandlers that
need to wait until after ->reset().

Signed-off-by: Stefan Hajnoczi 
---
 include/hw/hotplug.h | 11 +++
 hw/core/hotplug.c| 10 ++
 hw/core/qdev.c   |  4 
 3 files changed, 25 insertions(+)

diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
index 1a0516a479..51541d63e1 100644
--- a/include/hw/hotplug.h
+++ b/include/hw/hotplug.h
@@ -47,6 +47,8 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
  * @parent: Opaque parent interface.
  * @pre_plug: pre plug callback called at start of device.realize(true)
  * @plug: plug callback called at end of device.realize(true).
+ * @post_plug: post plug callback called after device.realize(true) and device
+ * reset
  * @unplug_request: unplug request callback.
  *  Used as a means to initiate device unplug for devices that
  *  require asynchronous unplug handling.
@@ -61,6 +63,7 @@ typedef struct HotplugHandlerClass {
 /*  */
 hotplug_fn pre_plug;
 hotplug_fn plug;
+void (*post_plug)(HotplugHandler *plug_handler, DeviceState *plugged_dev);
 hotplug_fn unplug_request;
 hotplug_fn unplug;
 } HotplugHandlerClass;
@@ -83,6 +86,14 @@ void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
   DeviceState *plugged_dev,
   Error **errp);
 
+/**
+ * hotplug_handler_post_plug:
+ *
+ * Call #HotplugHandlerClass.post_plug callback of @plug_handler.
+ */
+void hotplug_handler_post_plug(HotplugHandler *plug_handler,
+   DeviceState *plugged_dev);
+
 /**
  * hotplug_handler_unplug_request:
  *
diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
index 17ac986685..2253072d0e 100644
--- a/hw/core/hotplug.c
+++ b/hw/core/hotplug.c
@@ -35,6 +35,16 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
 }
 }
 
+void hotplug_handler_post_plug(HotplugHandler *plug_handler,
+   DeviceState *plugged_dev)
+{
+HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
+
+if (hdc->post_plug) {
+hdc->post_plug(plug_handler, plugged_dev);
+}
+}
+
 void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
 DeviceState *plugged_dev,
 Error **errp)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index cf0db4b6da..529b82de18 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -867,6 +867,10 @@ static void device_set_realized(Object *obj, bool value, 
Error **errp)
 device_reset(dev);
 }
 dev->pending_deleted_event = false;
+
+if (hotplug_ctrl) {
+hotplug_handler_post_plug(hotplug_ctrl, dev);
+}
 } else if (!value && dev->realized) {
 Error **local_errp = NULL;
 QLIST_FOREACH(bus, &dev->child_bus, sibling) {
-- 
2.17.1

[Qemu-devel] [PULL 5/6] sam460ex: Correct use after free error

[David Gibson]

From: BALATON Zoltan 

Commit 51b0d834c changed error handling to report file name in error
message but forgot to move freeing it after usage. Noticed by Coverity.

Fixes: CID 1394217
Reported-by: Paolo Bonzini 
Signed-off-by: BALATON Zoltan 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: David Gibson 
---
 hw/ppc/sam460ex.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index e2b7028843..0999efcc1e 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -269,11 +269,12 @@ static int sam460ex_load_device_tree(hwaddr addr,
 exit(1);
 }
 fdt = load_device_tree(filename, &fdt_size);
-g_free(filename);
 if (!fdt) {
 error_report("Couldn't load dtb file `%s'", filename);
+g_free(filename);
 exit(1);
 }
+g_free(filename);
 
 /* Manipulate device tree in memory. */
 
-- 
2.17.1

[Qemu-devel] [PATCH v2 0/2] virtio-scsi: fix hotplug ->reset() vs event race

[Stefan Hajnoczi]

v2:
 * Drop Error **errp argument to post_plug() handler [Paolo]
 * Move post_plug() call outside if (dev->hotplugged)

The virtio-scsi command virtqueues run during hotplug.  This creates the
possibility of race conditions since the guest can submit commands while the
monitor is performing hotplug.

See Patch 2 for a fix for the ->reset() vs event race condition that Zhengui Li
encountered.

Stefan Hajnoczi (2):
  qdev: add HotplugHandler->post_plug() callback
  virtio-scsi: fix hotplug ->reset() vs event race

 include/hw/hotplug.h  | 11 +++
 hw/core/hotplug.c | 10 ++
 hw/core/qdev.c|  4 
 hw/scsi/virtio-scsi.c | 11 ++-
 4 files changed, 35 insertions(+), 1 deletion(-)

-- 
2.17.1

[Qemu-devel] [PULL 6/6] sm501: Fix warning about unreachable code

[David Gibson]

From: BALATON Zoltan 

Coverity warned that the false arm of conditional expression is
unreachable when it is inside an if with the same condition.
Remove the unreachable code to avoid the warning.

Fixes: CID 1394215
Reported-by: Paolo Bonzini 
Signed-off-by: BALATON Zoltan 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: David Gibson 
---
 hw/display/sm501.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 9ab29d35dd..874260a143 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1024,7 +1024,7 @@ static void sm501_i2c_write(void *opaque, hwaddr addr, 
uint64_t value,
 if (res) {
 SM501_DPRINTF("sm501 i2c : transfer failed"
   " i=%d, res=%d\n", i, res);
-s->i2c_status |= (res ? SM501_I2C_STATUS_ERROR : 
0);
+s->i2c_status |= SM501_I2C_STATUS_ERROR;
 return;
 }
 }
-- 
2.17.1

[Qemu-devel] [PULL 0/6] ppc-for-3.0 queue 20180716

[David Gibson]

The following changes since commit 9277d81f5c2c6f4d0b5e47c8476eb7ee7e5c0beb:

  docs: Grammar and spelling fixes (2018-07-13 10:16:04 +0100)

are available in the Git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-3.0-20180716

for you to fetch changes up to 6730df0514d3aec35e646ff9833fbe8b05fd0776:

  sm501: Fix warning about unreachable code (2018-07-16 11:19:10 +1000)


ppc patch queue 2018-07-16

Here's my first hard freeze pull request for qemu-3.0.  This contains
an assortment of bugfixes. Several are for regressions, others are for
bugs that I think are significant enough to address during hard freeze.


BALATON Zoltan (3):
  sm501: Update screen on frame buffer address change
  sam460ex: Correct use after free error
  sm501: Fix warning about unreachable code

David Gibson (1):
  spapr: Correct inverted test in spapr_pc_dimm_node()

Greg Kurz (1):
  ppc/xics: fix ICP reset path

Michael Davidsaver (1):
  etsec: fix IRQ (un)masking

 hw/display/sm501.c   |  4 ++-
 hw/intc/xics.c   | 14 +++--
 hw/net/fsl_etsec/etsec.c | 68 +++-
 hw/net/fsl_etsec/etsec.h |  2 ++
 hw/net/fsl_etsec/registers.h | 10 +++
 hw/net/fsl_etsec/rings.c | 12 +---
 hw/ppc/sam460ex.c|  3 +-
 hw/ppc/spapr.c   |  2 +-
 8 files changed, 66 insertions(+), 49 deletions(-)

[Qemu-devel] [PULL 2/6] spapr: Correct inverted test in spapr_pc_dimm_node()

[David Gibson]

This function was introduced between v2.11 and v2.12 to replace obsolete
ways of specifying the NUMA nodes for DIMMs.  It's used to find the correct
node for an LMB, by locating which DIMM object it lies within.

Unfortunately, one of the checks is inverted, so we check whether the
address is less than two different things, rather than actually checking
a range.  This introduced a regression, meaning that after a reboot qemu
will advertise incorrect node information for memory to the guest.

Signed-off-by: David Gibson 
Reviewed-by: Greg Kurz 
Reviewed-by: Igor Mammedov 
---
 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 3f5e1d3ec2..421b2dd09b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -665,7 +665,7 @@ static uint32_t spapr_pc_dimm_node(MemoryDeviceInfoList 
*list, ram_addr_t addr)
 if (value && value->type == MEMORY_DEVICE_INFO_KIND_DIMM) {
 PCDIMMDeviceInfo *pcdimm_info = value->u.dimm.data;
 
-if (pcdimm_info->addr >= addr &&
+if (addr >= pcdimm_info->addr &&
 addr < (pcdimm_info->addr + pcdimm_info->size)) {
 return pcdimm_info->node;
 }
-- 
2.17.1

[Qemu-devel] [PULL 4/6] etsec: fix IRQ (un)masking

[David Gibson]

From: Michael Davidsaver 

Interrupt conditions occurring while masked are not being
signaled when later unmasked.
The fix is to raise/lower IRQs when IMASK is changed.

To avoid problems like this in future, consolidate
IRQ pin update logic in one function.

Also fix probable typo "IEVENT_TXF | IEVENT_TXF",
and update IRQ pins on reset.

Signed-off-by: Michael Davidsaver 
Reviewed-by: Cédric Le Goater 
Signed-off-by: David Gibson 
---
 hw/net/fsl_etsec/etsec.c | 68 +++-
 hw/net/fsl_etsec/etsec.h |  2 ++
 hw/net/fsl_etsec/registers.h | 10 ++
 hw/net/fsl_etsec/rings.c | 12 +--
 4 files changed, 49 insertions(+), 43 deletions(-)

diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index 9da1932970..0b66274ce3 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -49,6 +49,28 @@ static const int debug_etsec;
 }  \
 } while (0)
 
+/* call after any change to IEVENT or IMASK */
+void etsec_update_irq(eTSEC *etsec)
+{
+uint32_t ievent = etsec->regs[IEVENT].value;
+uint32_t imask  = etsec->regs[IMASK].value;
+uint32_t active = ievent & imask;
+
+int tx  = !!(active & IEVENT_TX_MASK);
+int rx  = !!(active & IEVENT_RX_MASK);
+int err = !!(active & IEVENT_ERR_MASK);
+
+DPRINTF("%s IRQ ievent=%"PRIx32" imask=%"PRIx32" %c%c%c",
+__func__, ievent, imask,
+tx  ? 'T' : '_',
+rx  ? 'R' : '_',
+err ? 'E' : '_');
+
+qemu_set_irq(etsec->tx_irq, tx);
+qemu_set_irq(etsec->rx_irq, rx);
+qemu_set_irq(etsec->err_irq, err);
+}
+
 static uint64_t etsec_read(void *opaque, hwaddr addr, unsigned size)
 {
 eTSEC  *etsec = opaque;
@@ -139,31 +161,6 @@ static void write_rbasex(eTSEC  *etsec,
 etsec->regs[RBPTR0 + (reg_index - RBASE0)].value = value & ~0x7;
 }
 
-static void write_ievent(eTSEC  *etsec,
- eTSEC_Register *reg,
- uint32_treg_index,
- uint32_tvalue)
-{
-/* Write 1 to clear */
-reg->value &= ~value;
-
-if (!(reg->value & (IEVENT_TXF | IEVENT_TXF))) {
-qemu_irq_lower(etsec->tx_irq);
-}
-if (!(reg->value & (IEVENT_RXF | IEVENT_RXF))) {
-qemu_irq_lower(etsec->rx_irq);
-}
-
-if (!(reg->value & (IEVENT_MAG | IEVENT_GTSC | IEVENT_GRSC | IEVENT_TXC |
-IEVENT_RXC | IEVENT_BABR | IEVENT_BABT | IEVENT_LC |
-IEVENT_CRL | IEVENT_FGPI | IEVENT_FIR | IEVENT_FIQ |
-IEVENT_DPE | IEVENT_PERR | IEVENT_EBERR | IEVENT_TXE |
-IEVENT_XFUN | IEVENT_BSY | IEVENT_MSRO | IEVENT_MMRD |
-IEVENT_MMRW))) {
-qemu_irq_lower(etsec->err_irq);
-}
-}
-
 static void write_dmactrl(eTSEC  *etsec,
   eTSEC_Register *reg,
   uint32_treg_index,
@@ -178,9 +175,7 @@ static void write_dmactrl(eTSEC  *etsec,
 } else {
 /* Graceful receive stop now */
 etsec->regs[IEVENT].value |= IEVENT_GRSC;
-if (etsec->regs[IMASK].value & IMASK_GRSCEN) {
-qemu_irq_raise(etsec->err_irq);
-}
+etsec_update_irq(etsec);
 }
 }
 
@@ -191,9 +186,7 @@ static void write_dmactrl(eTSEC  *etsec,
 } else {
 /* Graceful transmit stop now */
 etsec->regs[IEVENT].value |= IEVENT_GTSC;
-if (etsec->regs[IMASK].value & IMASK_GTSCEN) {
-qemu_irq_raise(etsec->err_irq);
-}
+etsec_update_irq(etsec);
 }
 }
 
@@ -222,7 +215,16 @@ static void etsec_write(void *opaque,
 
 switch (reg_index) {
 case IEVENT:
-write_ievent(etsec, reg, reg_index, value);
+/* Write 1 to clear */
+reg->value &= ~value;
+
+etsec_update_irq(etsec);
+break;
+
+case IMASK:
+reg->value = value;
+
+etsec_update_irq(etsec);
 break;
 
 case DMACTRL:
@@ -337,6 +339,8 @@ static void etsec_reset(DeviceState *d)
 MII_SR_EXTENDED_STATUS  | MII_SR_100T2_HD_CAPS | MII_SR_100T2_FD_CAPS |
 MII_SR_10T_HD_CAPS  | MII_SR_10T_FD_CAPS   | MII_SR_100X_HD_CAPS  |
 MII_SR_100X_FD_CAPS | MII_SR_100T4_CAPS;
+
+etsec_update_irq(etsec);
 }
 
 static ssize_t etsec_receive(NetClientState *nc,
diff --git a/hw/net/fsl_etsec/etsec.h b/hw/net/fsl_etsec/etsec.h
index 30c828e241..877988572e 100644
--- a/hw/net/fsl_etsec/etsec.h
+++ b/hw/net/fsl_etsec/etsec.h
@@ -163,6 +163,8 @@ DeviceState *etsec_create(hwaddrbase,
   qemu_irq  rx_irq,
   qemu_irq  err_irq);
 
+void etsec_update_irq(eTSEC *etsec);
+
 void etsec_walk_tx_ring(eTSEC *etsec, int ring_nbr);
 void etsec_walk_rx_ring(eTSEC *etsec, int ring_nbr);
 ssize_t et

[Qemu-devel] [PULL 1/6] sm501: Update screen on frame buffer address change

[David Gibson]

From: BALATON Zoltan 

When the guest changes the address of the frame buffer we need to
refresh the screen to correctly display the new content. This fixes
display update problems when changing between screens on AmigaOS.

Signed-off-by: BALATON Zoltan 
Signed-off-by: David Gibson 
---
 hw/display/sm501.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 3661a89f60..9ab29d35dd 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1235,6 +1235,7 @@ static void sm501_disp_ctrl_write(void *opaque, hwaddr 
addr,
 if (value & 0x800) {
 qemu_log_mask(LOG_UNIMP, "Panel external memory not supported\n");
 }
+s->do_full_update = true;
 break;
 case SM501_DC_PANEL_FB_OFFSET:
 s->dc_panel_fb_offset = value & 0x3FF03FF0;
@@ -1298,6 +1299,7 @@ static void sm501_disp_ctrl_write(void *opaque, hwaddr 
addr,
 if (value & 0x800) {
 qemu_log_mask(LOG_UNIMP, "CRT external memory not supported\n");
 }
+s->do_full_update = true;
 break;
 case SM501_DC_CRT_FB_OFFSET:
 s->dc_crt_fb_offset = value & 0x3FF03FF0;
-- 
2.17.1

[Qemu-devel] [PULL 3/6] ppc/xics: fix ICP reset path

[David Gibson]

From: Greg Kurz 

Recent cleanup in commit a028dd423ee6 dropped the ICPStateClass::reset
handler. It is now up to child ICP classes to call the DeviceClass::reset
handler of the parent class, thanks to device_class_set_parent_reset().
This is a better object programming pattern, but unfortunately it causes
QEMU to crash during CPU hotplug:

(qemu) device_add host-spapr-cpu-core,id=core1,core-id=1
Segmentation fault (core dumped)

When the hotplug path tries to reset the ICP device, we end up calling:

static void icp_kvm_reset(DeviceState *dev)
{
ICPStateClass *icpc = ICP_GET_CLASS(dev);

icpc->parent_reset(dev);

but icpc->parent_reset is NULL... This happens because icp_kvm_class_init()
calls:

device_class_set_parent_reset(dc, icp_kvm_reset,
  &icpc->parent_reset);

but dc->reset, ie, DeviceClass::reset for the TYPE_ICP type, is
itself NULL.

This patch hence sets DeviceClass::reset for the TYPE_ICP type to
point to icp_reset(). It then registers a reset handler that calls
DeviceClass::reset. If the ICP subtype has configured its own reset
handler with device_class_set_parent_reset(), this ensures it will
be called first and it can then call ICPStateClass::parent_reset
safely. This fixes the reset path for the TYPE_KVM_ICP type, which
is the only subtype that defines its own reset function.

Reported-by: Satheesh Rajendran 
Suggested-by: David Gibson 
Fixes: a028dd423ee6dfd091a8c63028240832bf10f671
Signed-off-by: Greg Kurz 
Signed-off-by: David Gibson 
---
 hw/intc/xics.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index b9f1a3c972..c90c893228 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -291,7 +291,7 @@ static const VMStateDescription vmstate_icp_server = {
 },
 };
 
-static void icp_reset(void *dev)
+static void icp_reset(DeviceState *dev)
 {
 ICPState *icp = ICP(dev);
 
@@ -303,6 +303,13 @@ static void icp_reset(void *dev)
 qemu_set_irq(icp->output, 0);
 }
 
+static void icp_reset_handler(void *dev)
+{
+DeviceClass *dc = DEVICE_GET_CLASS(dev);
+
+dc->reset(dev);
+}
+
 static void icp_realize(DeviceState *dev, Error **errp)
 {
 ICPState *icp = ICP(dev);
@@ -345,7 +352,7 @@ static void icp_realize(DeviceState *dev, Error **errp)
 return;
 }
 
-qemu_register_reset(icp_reset, dev);
+qemu_register_reset(icp_reset_handler, dev);
 vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
 }
 
@@ -354,7 +361,7 @@ static void icp_unrealize(DeviceState *dev, Error **errp)
 ICPState *icp = ICP(dev);
 
 vmstate_unregister(NULL, &vmstate_icp_server, icp);
-qemu_unregister_reset(icp_reset, dev);
+qemu_unregister_reset(icp_reset_handler, dev);
 }
 
 static void icp_class_init(ObjectClass *klass, void *data)
@@ -363,6 +370,7 @@ static void icp_class_init(ObjectClass *klass, void *data)
 
 dc->realize = icp_realize;
 dc->unrealize = icp_unrealize;
+dc->reset = icp_reset;
 }
 
 static const TypeInfo icp_info = {
-- 
2.17.1

Re: [Qemu-devel] [PATCH] arm: Add ARMv6-M programmer's model support

[Stefan Hajnoczi]

On Fri, Jul 13, 2018 at 01:30:59PM +0300, Julia Suvorova wrote:
> Forbid stack alignment change. (CCR)
> Reserve FAULTMASK, BASEPRI registers.
> Report any fault as HardFault. Disable MemManage, BusFault and
> UsageFault, so they always escalated to HardFault. (SHCSR)
> 
> Signed-off-by: Julia Suvorova 
> ---
> This is the last cortex-m0 patch.
> 
>  hw/intc/armv7m_nvic.c | 10 ++
>  target/arm/cpu.c  | 10 ++
>  target/arm/helper.c   | 13 +++--
>  3 files changed, 31 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] hw/char/serial: Only retry if qemu_chr_fe_write returns 0

[Igor Mammedov]

On Fri, 13 Jul 2018 16:55:15 +0200
Marc-André Lureau  wrote:

> Hi
> 
> On Tue, Jul 10, 2018 at 3:48 PM, Igor Mammedov  wrote:
> > On Tue, 5 Jun 2018 11:18:35 +0200
> > Paolo Bonzini  wrote:
> >  
> >> On 05/06/2018 09:54, Sergio Lopez wrote:  
> >> > Only retry on serial_xmit if qemu_chr_fe_write returns 0, as this is the
> >> > only recoverable error.
> >> >
> >> > Retrying with any other scenario, in addition to being a waste of CPU
> >> > cycles, can compromise the Guest stability if by the vCPU issuing the
> >> > write and the main loop thread are, by chance or explicit pinning,
> >> > running on the same pCPU.
> >> >
> >> > Previous discussion:
> >> >
> >> > https://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg06998.html
> >> >
> >> > Signed-off-by: Sergio Lopez 
> >> > ---
> >> >  hw/char/serial.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/hw/char/serial.c b/hw/char/serial.c
> >> > index 605b0d0..6de6c29 100644
> >> > --- a/hw/char/serial.c
> >> > +++ b/hw/char/serial.c
> >> > @@ -260,7 +260,7 @@ static void serial_xmit(SerialState *s)
> >> >  if (s->mcr & UART_MCR_LOOP) {
> >> >  /* in loopback mode, say that we just received a char */
> >> >  serial_receive1(s, &s->tsr, 1);
> >> > -} else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) != 1 &&
> >> > +} else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) == 0 &&
> >> > s->tsr_retry < MAX_XMIT_RETRY) {
> >> >  assert(s->watch_tag == 0);
> >> >  s->watch_tag =
> >> >  
> > Hi Sergio, Paolo,
> >
> > it looks like commit
> > commit 019288bf137183bf3407c9824655b753bfafc99f
> > Author: Sergio Lopez 
> > Date:   Tue Jun 5 03:54:55 2018 -0400
> >
> > hw/char/serial: Only retry if qemu_chr_fe_write returns 0
> >
> > introduced regression wrt 2.12 and broke windows guest remote kernel debug 
> > over
> > serial, where windbg can't connect to target and target hangs when windbg 
> > tries
> > to connect to it.
> >
> > Reverting the commit fixes issue
> >  
> 
> is this a possible solution?
it fixes windbg over serial (with and without EINTR)


> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index cd7d747c68..046c4684ff 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -261,15 +261,20 @@ static void serial_xmit(SerialState *s)
>  if (s->mcr & UART_MCR_LOOP) {
>  /* in loopback mode, say that we just received a char */
>  serial_receive1(s, &s->tsr, 1);
> -} else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) == 0 &&
> -   s->tsr_retry < MAX_XMIT_RETRY) {
> -assert(s->watch_tag == 0);
> -s->watch_tag =
> -qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
> -  serial_watch_cb, s);
> -if (s->watch_tag > 0) {
> -s->tsr_retry++;
> -return;
> +} else {
> +int rc = qemu_chr_fe_write(&s->chr, &s->tsr, 1);
> +
> +if ((rc == 0 ||
> + (rc == -1 && (errno == EAGAIN || errno == EINTR))) &&
> +s->tsr_retry < MAX_XMIT_RETRY) {
> +assert(s->watch_tag == 0);
> +s->watch_tag =
> +qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
> +  serial_watch_cb, s);
> +if (s->watch_tag > 0) {
> +s->tsr_retry++;
> +return;
> +}
>  }
>  }
>  s->tsr_retry = 0;
> 
> 

Re: [Qemu-devel] [PATCH v2 for 3.0 11/16] docker: add expansion for docker-test-FOO to Makefile.include

[Alex Bennée]

Philippe Mathieu-Daudé  writes:

> Hi Alex,
>
> On 07/13/2018 09:17 AM, Alex Bennée wrote:
>> This allows us to run a particular test on all docker images. For
>> example:
>>
>>   make docker-test-unit
>>
>> Will run the unit tests on every supported image. At the same time
>> rename docker-test to docker-all-tests to be clearer.
>>
>> Signed-off-by: Alex Bennée 
>>
>> ---
>> v2
>>   - docker-test -> docker-all-tests
>> ---
>>  tests/docker/Makefile.include | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index fe63aacf69..e32c35be0d 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -151,7 +151,8 @@ $(foreach i,$(filter-out 
>> $(DOCKER_PARTIAL_IMAGES),$(DOCKER_IMAGES) $(DOCKER_DEPR
>>  $(eval docker-$t@$i: docker-image-$i docker-run-$t@$i) \
>>  ) \
>>  $(foreach t,$(DOCKER_TESTS), \
>> -$(eval docker-test: docker-$t@$i) \
>> +$(eval docker-all-tests: docker-$t@$i) \
>> +$(eval docker-$t: docker-$t@$i) \
>
> Is this supposed to work this way?
>
> $ make docker-test-quick@debian-alpha-cross
> make: *** No rule to make target
> 'docker-test-quick@debian-alpha-cross'.

No

  make docker-test-quick

will make the test-quick on all images.

Your example fails because debian-alpha-cross is a PARTIAL image, good
for building test cases but not QEMU so it's not expanded in the above
bit: $(eval docker-$t@$i: docker-image-$i docker-run-$t@$i)

>  Stop.
>
>>  ) \
>>  )
>>
>> @@ -161,7 +162,8 @@ docker:
>>  @echo 'Available targets:'
>>  @echo
>>  @echo 'docker:  Print this help.'
>> -@echo 'docker-test: Run all image/test combinations.'
>> +@echo 'docker-all-tests:Run all image/test combinations.'
>> +@echo 'docker-TEST: Run TEST on all image combinations.'
>>  @echo 'docker-clean:Kill and remove residual docker testing 
>> containers.'
>>  @echo 'docker-TEST@IMAGE:   Run "TEST" in container "IMAGE".'
>>  @echo ' Note: "TEST" is one of the listed test 
>> name,'
>>


--
Alex Bennée

Re: [Qemu-devel] [RFC PATCH] hw: arm: Add basic support for cprman (clock subsystem)

[Peter Maydell]

On 15 July 2018 at 23:06, Guenter Roeck  wrote:
> Add basic support for BCM283x CPRMAN. Provide support for reading and
> writing CPRMAN registers and initialize registers with sensible default
> values. During runtime retain any written values.
>
> Basic CPRMAN support is necessary and sufficient to boot Linux on raspi2
> and raspi3 systems.

I can boot Linux on raspi3 with current upstream's level
of cprman support:
https://translatedcode.wordpress.com/2018/04/25/debian-on-qemus-raspberry-pi-3-model/

> Signed-off-by: Guenter Roeck 
> ---
> I don't seriously expect this patch to get accepted, but I thought
> it might be valuable enough for others to use it when playing with
> raspi2 and raspi3 emulations.

I'm not necessarily going to rule it out entirely, but I'm
definitely not very happy about having a model of hardware
that's very clearly "do something that Linux likes". I'd
really rather see hardware documentation here -- presumably
whoever is writing the Linux drivers has that?

thanks
-- PMM

[Qemu-devel] [PATCH] monitor: Fix tracepoint crash on JSON syntax error

[Markus Armbruster]

When tracepoint handle_qmp_command is enabled, we crash on JSON syntax
errors.  Broken in commit 1cc37471525.  Fix by skipping the tracepoint
on JSON syntax error.  Before the flawed commit, we skipped it by
returning early.

Fixes: CID 1394216
Signed-off-by: Markus Armbruster 
---
 monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index 7af1f18d13..be29634a00 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4224,7 +4224,7 @@ static void handle_qmp_command(JSONMessageParser *parser, 
GQueue *tokens)
 qdict_del(qdict, "id");
 } /* else will fail qmp_dispatch() */
 
-if (trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
+if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
 QString *req_json = qobject_to_json(req);
 trace_handle_qmp_command(mon, qstring_get_str(req_json));
 qobject_unref(req_json);
-- 
2.17.1

Re: [Qemu-devel] [PATCH] monitor: Fix tracepoint crash on JSON syntax error

[Marc-André Lureau]

On Mon, Jul 16, 2018 at 11:10 AM, Markus Armbruster  wrote:
> When tracepoint handle_qmp_command is enabled, we crash on JSON syntax
> errors.  Broken in commit 1cc37471525.  Fix by skipping the tracepoint
> on JSON syntax error.  Before the flawed commit, we skipped it by
> returning early.
>
> Fixes: CID 1394216
> Signed-off-by: Markus Armbruster 

Reviewed-by: Marc-André Lureau 

> ---
>  monitor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/monitor.c b/monitor.c
> index 7af1f18d13..be29634a00 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4224,7 +4224,7 @@ static void handle_qmp_command(JSONMessageParser 
> *parser, GQueue *tokens)
>  qdict_del(qdict, "id");
>  } /* else will fail qmp_dispatch() */
>
> -if (trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
> +if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
>  QString *req_json = qobject_to_json(req);
>  trace_handle_qmp_command(mon, qstring_get_str(req_json));
>  qobject_unref(req_json);
> --
> 2.17.1
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] QEMU advent calendar

[Thomas Huth]

On 13.07.2018 10:34, Richard Jansson wrote:
> Hi
> 
> I checked in on the QEMU advent calendar side of which I have very fond
> memories. I'm very impressed and grateful for the work that you've put into
> this site. My question to you is the following, what are your plans for the
> continuation of the program?

 Hi Richard,

thanks for the kind words and your interest in the QEMU advent calendar!
This year, QEMU had its 15th anniversary, so we indeed already thought
about doing a third round of the QEMU advent calendar this year...

> In case there's anything I can do I'll gladly put in some work to help you
> out to support you in your noble cause :)

The problem is to come up with ideas for 24 suitable images [1], and to
prepare them (it's quite time-consuming). So far, we're not certain yet
whether we could do a 2018 edition of the calendar since we hardly have
any images yet. So if you've got ideas for some images, and/or could
help to prepare some, that would be very appreciated! Please get in
touch with us off-list in that case (to avoid to spoil the surprise for
the public).

 Cheers,
  Thomas


[1] See my announcement mail from 2016 for the rules of suitable images:
https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00039.html

Re: [Qemu-devel] [PATCH 11/14] timer: generalize ds1338

[David Gibson]

On Thu, Jul 05, 2018 at 11:19:58AM -0700, Michael Davidsaver wrote:
> Add class level handling for address space size
> and control register.
> 
> Signed-off-by: Michael Davidsaver 

Reviewed-by: David Gibson 

Although I don't love the name "addr_size" - just "nvram_size" seems
clearer.

> ---
>  hw/timer/ds-rtc.c | 62 
> +++
>  1 file changed, 49 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
> index 126566ce1f..6d3a8b5586 100644
> --- a/hw/timer/ds-rtc.c
> +++ b/hw/timer/ds-rtc.c
> @@ -21,8 +21,10 @@
>   */
>  #define NVRAM_SIZE 64
>  
> -#define TYPE_DSRTC "ds1338"
> +#define TYPE_DSRTC "dsrtc"
>  #define DSRTC(obj) OBJECT_CHECK(DSRTCState, (obj), TYPE_DSRTC)
> +#define DSRTC_CLASS(klass) OBJECT_CLASS_CHECK(DSRTCClass, (klass), 
> TYPE_DSRTC)
> +#define DSRTC_GET_CLASS(obj) OBJECT_GET_CLASS(DSRTCClass, (obj), TYPE_DSRTC)
>  
>  /* values stored in BCD */
>  /* 00-59 */
> @@ -38,7 +40,7 @@
>  /* 0-99 */
>  #define R_YEAR  (0x6)
>  
> -#define R_CTRL  (0x7)
> +#define R_DS1338_CTRL (0x7)
>  
>  /* use 12 hour mode when set */
>  FIELD(HOUR, SET12, 6, 1)
> @@ -65,6 +67,15 @@ typedef struct DSRTCState {
>  bool addr_byte;
>  } DSRTCState;
>  
> +typedef struct DSRTCClass {
> +I2CSlaveClass parent_obj;
> +
> +/* actual address space size must be <= NVRAM_SIZE */
> +unsigned addr_size;
> +unsigned ctrl_offset;
> +void (*ctrl_write)(DSRTCState *s, uint8_t);
> +} DSRTCClass;
> +
>  static const VMStateDescription vmstate_dsrtc = {
>  .name = "ds1338",
>  .version_id = 2,
> @@ -119,11 +130,12 @@ static void capture_current_time(DSRTCState *s)
>  
>  static void inc_regptr(DSRTCState *s)
>  {
> -/* The register pointer wraps around after 0x3F; wraparound
> +DSRTCClass *k = DSRTC_GET_CLASS(s);
> +/* The register pointer wraps around after k->addr_size-1; wraparound
>   * causes the current time/date to be retransferred into
>   * the secondary registers.
>   */
> -s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1);
> +s->ptr = (s->ptr + 1) % k->addr_size;
>  if (!s->ptr) {
>  capture_current_time(s);
>  }
> @@ -205,20 +217,15 @@ static void dsrtc_update(DSRTCState *s)
>  static int dsrtc_send(I2CSlave *i2c, uint8_t data)
>  {
>  DSRTCState *s = DSRTC(i2c);
> +DSRTCClass *k = DSRTC_GET_CLASS(s);
>  
>  if (s->addr_byte) {
> -s->ptr = data & (NVRAM_SIZE - 1);
> +s->ptr = data % k->addr_size;
>  s->addr_byte = false;
>  return 0;
>  }
> -if (s->ptr == R_CTRL) {
> -/* Control register. */
> -
> -/* Allow guest to set no-op controls for clock out pin and
> - * rate select.  Ignore write 1 to clear OSF.  We don't model
> - * oscillator stop, so it is never set.
> - */
> -data = data & 0x93;
> +if (s->ptr == k->ctrl_offset) {
> +(k->ctrl_write)(s, data);
>  }
>  s->nvram[s->ptr] = data;
>  if (s->ptr <= R_YEAR) {
> @@ -253,15 +260,44 @@ static void dsrtc_class_init(ObjectClass *klass, void 
> *data)
>  }
>  
>  static const TypeInfo dsrtc_info = {
> +.abstract  = true,
>  .name  = TYPE_DSRTC,
>  .parent= TYPE_I2C_SLAVE,
>  .instance_size = sizeof(DSRTCState),
>  .class_init= dsrtc_class_init,
>  };
>  
> +static void ds1338_control_write(DSRTCState *s, uint8_t data)
> +{
> +/* Control register. */
> +
> +/* Allow guest to set no-op controls for clock out pin and
> + * rate select.  Ignore write 1 to clear OSF.  We don't model
> + * oscillator stop, so it is never set.
> + */
> +s->nvram[R_DS1338_CTRL] = data & 0x93;
> +}
> +
> +static void ds1338_class_init(ObjectClass *klass, void *data)
> +{
> +DSRTCClass *k = DSRTC_CLASS(klass);
> +
> +k->addr_size = 0x40;
> +k->ctrl_offset = R_DS1338_CTRL;
> +k->ctrl_write = ds1338_control_write;
> +}
> +
> +static const TypeInfo ds1338_info = {
> +.name  = "ds1338",
> +.parent= TYPE_DSRTC,
> +.class_size= sizeof(DSRTCClass),
> +.class_init= ds1338_class_init,
> +};
> +
>  static void dsrtc_register_types(void)
>  {
>  type_register_static(&dsrtc_info);
> +type_register_static(&ds1338_info);
>  }
>  
>  type_init(dsrtc_register_types)

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 0/2] virtio-scsi: fix hotplug ->reset() vs event race

[Igor Mammedov]

On Mon, 16 Jul 2018 09:37:30 +0100
Stefan Hajnoczi  wrote:

> v2:
>  * Drop Error **errp argument to post_plug() handler [Paolo]
>  * Move post_plug() call outside if (dev->hotplugged)
> 
> The virtio-scsi command virtqueues run during hotplug.  This creates the
> possibility of race conditions since the guest can submit commands while the
> monitor is performing hotplug.
> 
> See Patch 2 for a fix for the ->reset() vs event race condition that Zhengui 
> Li
> encountered.
> 
> Stefan Hajnoczi (2):
>   qdev: add HotplugHandler->post_plug() callback
>   virtio-scsi: fix hotplug ->reset() vs event race
> 
>  include/hw/hotplug.h  | 11 +++
>  hw/core/hotplug.c | 10 ++
>  hw/core/qdev.c|  4 
>  hw/scsi/virtio-scsi.c | 11 ++-
>  4 files changed, 35 insertions(+), 1 deletion(-)
> 

Reviewed-by: Igor Mammedov 

Re: [Qemu-devel] [PATCH] monitor: Fix tracepoint crash on JSON syntax error

[Peter Xu]

On Mon, Jul 16, 2018 at 11:10:12AM +0200, Markus Armbruster wrote:
> When tracepoint handle_qmp_command is enabled, we crash on JSON syntax
> errors.  Broken in commit 1cc37471525.  Fix by skipping the tracepoint
> on JSON syntax error.  Before the flawed commit, we skipped it by
> returning early.
> 
> Fixes: CID 1394216
> Signed-off-by: Markus Armbruster 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [libvirt] [PULL 25/26] block: Remove deprecated -drive option serial

[Daniel P . Berrangé]

On Thu, Jul 12, 2018 at 05:47:00PM +0200, Thomas Huth wrote:
> On 12.07.2018 08:32, Markus Armbruster wrote:
> > Daniel P. Berrangé  writes:
> [...]
> >> For libvirt, I think whenever something is proposed for deprecation
> >> we could just CC libvir-list, or ask one of the libvirt people to
> >> confirm its not being used. If it is, then we should file BZ against
> >> libvirt.
> > 
> > Makes sense, but relying on developers getting their cc: right every
> > time is a setting us up for failure.
> > 
> > Our tool to help with getting cc: wrong less often is the MAINTAINERS
> > file.  Could one of the libvirt developers watch changes to qemu-doc
> > appendix "Deprecated features"?  Would putting the appendix in its own
> > .texi help with that?
> 
> Sound like a good idea to put the appendix in its own texi file. Then
> add an "R: libvir-list" entry for that file to MAINTAINERS and we should
> be fine (at least for the people who use the get_maintainers.pl script).

That's a neat idea and gets my vote.

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

Re: [Qemu-devel] [PATCH] seccomp: allow sched_setscheduler() with SCHED_IDLE policy

[Daniel P . Berrangé]

On Wed, Jul 11, 2018 at 04:48:55PM +0200, Eduardo Otubo wrote:
> On 10/07/2018 - 16:55:57, Marc-André Lureau wrote:
> > Current and upcoming mesa releases rely on a shader disk cash. It uses
> > a thread job queue with low priority, set with
> > sched_setscheduler(SCHED_IDLE). However, that syscall is rejected by
> > the "resourcecontrol" seccomp qemu filter.
> > 
> > Since it should be safe to allow lowering thread priority, let's allow
> > scheduling thread to idle policy.
> > 
> > Related to:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1594456
> > 
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  qemu-seccomp.c | 12 ++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> > index 148e4c6f24..9cd8eb9499 100644
> > --- a/qemu-seccomp.c
> > +++ b/qemu-seccomp.c
> > @@ -34,6 +34,12 @@
> >  struct QemuSeccompSyscall {
> >  int32_t num;
> >  uint8_t set;
> > +uint8_t narg;
> > +const struct scmp_arg_cmp *arg_cmp;
> > +};
> > +
> > +const struct scmp_arg_cmp sched_setscheduler_arg[] = {
> > +SCMP_A1(SCMP_CMP_NE, SCHED_IDLE)
> >  };
> >  
> >  static const struct QemuSeccompSyscall blacklist[] = {
> > @@ -92,7 +98,8 @@ static const struct QemuSeccompSyscall blacklist[] = {
> >  { SCMP_SYS(setpriority),QEMU_SECCOMP_SET_RESOURCECTL },
> >  { SCMP_SYS(sched_setparam), QEMU_SECCOMP_SET_RESOURCECTL },
> >  { SCMP_SYS(sched_getparam), QEMU_SECCOMP_SET_RESOURCECTL },
> > -{ SCMP_SYS(sched_setscheduler), QEMU_SECCOMP_SET_RESOURCECTL },
> > +{ SCMP_SYS(sched_setscheduler), QEMU_SECCOMP_SET_RESOURCECTL,
> > +  ARRAY_SIZE(sched_setscheduler_arg), sched_setscheduler_arg },
> >  { SCMP_SYS(sched_getscheduler), QEMU_SECCOMP_SET_RESOURCECTL },
> >  { SCMP_SYS(sched_setaffinity),  QEMU_SECCOMP_SET_RESOURCECTL },
> >  { SCMP_SYS(sched_getaffinity),  QEMU_SECCOMP_SET_RESOURCECTL },
> > @@ -118,7 +125,8 @@ static int seccomp_start(uint32_t seccomp_opts)
> >  continue;
> >  }
> >  
> > -rc = seccomp_rule_add(ctx, SCMP_ACT_KILL, blacklist[i].num, 0);
> > +rc = seccomp_rule_add_array(ctx, SCMP_ACT_KILL, blacklist[i].num,
> > +blacklist[i].narg, 
> > blacklist[i].arg_cmp);
> >  if (rc < 0) {
> >  goto seccomp_return;
> >  }
> > -- 
> > 2.18.0.129.ge3331758f1
> > 
> 
> Acked-by: Eduardo Otubo 
> 
> Patch looks safe enough for me. If everyone else is OK with this I'll send a
> pull-request tomorrow morning.

Reviewed-by: Daniel P. Berrangé 



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

Re: [Qemu-devel] [PATCH 2/2] MAINTAINERS: New section "Incompatible changes", copy libvir-list

[Daniel P . Berrangé]

On Mon, Jul 16, 2018 at 09:32:26AM +0200, Markus Armbruster wrote:
> Libvirt developers would like to be copied on patches to qemu-doc
> appendix "Deprecated features".  Do them the favor.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  MAINTAINERS | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 20eef3cb61..666e936812 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2194,6 +2194,10 @@ M: Daniel P. Berrange 
>  S: Odd Fixes
>  F: docs/devel/build-system.txt
>  
> +Incompatible changes
> +R: libvir-l...@redhat.com
> +F: qemu-deprecated.texi
> +
>  Build System
>  
>  GIT submodules

Reviewed-by: Daniel P. Berrangé 


Note, that libvir-list requires senders to be subscribed before mails are
delivered, otherwise they go into moderation queue. My normal approach. as
the person moderating, is to whitelist all human senders who get moderated,
so it is only a one-time pain point per person.

Thankyou spammers :-(

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

Re: [Qemu-devel] [PATCH 1/2] qemu-doc: Move appendix "Deprecated features" to its own file

[Daniel P . Berrangé]

On Mon, Jul 16, 2018 at 09:32:25AM +0200, Markus Armbruster wrote:
> Consumers of QEMU need to track feature deprecation.  Keeping
> deprecation documentation in its own file helps in two small ways:
> 
> * You can track changes the easy and obvious way, with git-log.
>   Before, you had to resort to more complex gittery like "git-log
>   --oneline -L '/@node Deprecated features/,/@node Supported build
>   platforms/:qemu-doc.texi'"
> 
> * It lets us use MAINTAINERS to copy interested parties on deprecation
>   patches, so they can advise or object before they're a done deal.
>   The next commit will do that for libvirt.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  qemu-deprecated.texi | 234 ++
>  qemu-doc.texi| 235 +--
>  2 files changed, 235 insertions(+), 234 deletions(-)
>  create mode 100644 qemu-deprecated.texi

Reviewed-by: Daniel P. Berrangé 


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

Re: [Qemu-devel] [PATCH] vhost: fix invalid downcast

[Юрий Котов]

+ qemu-stable@

13.07.2018, 19:16, "Michael S. Tsirkin" :
> On Fri, Jul 13, 2018 at 05:04:05PM +0300, Yury Kotov wrote:
>>  virtio_queue_get_desc_addr returns 64-bit hwaddr while int is usually 
>> 32-bit.
>>  If returned hwaddr is not equal to 0 but least-significant 32 bits are
>>  equal to 0 then this code will not actually stop running queue.
>>
>>  Signed-off-by: Yury Kotov 
>
> So IIUC
>
> Fixes: fb20fbb764aa1 ("vhost: avoid to start/stop virtqueue which is not 
> ready")
> And
> Cc: qemu-sta...@nongnu.org
>

Ok, done. Or did you mean I have to resend the patch-message to qemu-stable?

>>  ---
>>   hw/virtio/vhost.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>>  diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>>  index b129cb9..7edeee7 100644
>>  --- a/hw/virtio/vhost.c
>>  +++ b/hw/virtio/vhost.c
>>  @@ -1071,10 +1071,8 @@ static void vhost_virtqueue_stop(struct vhost_dev 
>> *dev,
>>   .index = vhost_vq_index,
>>   };
>>   int r;
>>  - int a;
>>
>>  - a = virtio_queue_get_desc_addr(vdev, idx);
>>  - if (a == 0) {
>>  + if (virtio_queue_get_desc_addr(vdev, idx) == 0) {
>>   /* Don't stop the virtqueue which might have not been started */
>>   return;
>>   }
>>  --
>>  2.7.4

Re: [Qemu-devel] [PATCH 13/14] timer: ds-rtc model ds1375

[David Gibson]

On Thu, Jul 05, 2018 at 11:20:00AM -0700, Michael Davidsaver wrote:
> differences from ds1338
> 
> * Has alarms (not modeled)
> * different control register (not modeled)
> * smaller address space (0x20 vs. 0x40)
> 
> Signed-off-by: Michael Davidsaver 
> Reviewed-by: Peter Maydell 

Reviewed-by: David Gibson 

> ---
>  hw/timer/ds-rtc.c | 30 --
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
> index 9abac38628..3a8ad1a00f 100644
> --- a/hw/timer/ds-rtc.c
> +++ b/hw/timer/ds-rtc.c
> @@ -1,8 +1,9 @@
>  /*
> - * MAXIM DS1338 I2C RTC+NVRAM
> + * MAXIM/Dallas DS1338 and DS1375 I2C RTC+NVRAM
>   *
> + * Copyright (c) 2018 Michael Davidsaver
>   * Copyright (c) 2009 CodeSourcery.
> - * Written by Paul Brook
> + * Written by Paul Brook, Michael Davidsaver
>   *
>   * This code is licensed under the GNU GPL v2.
>   *
> @@ -41,6 +42,7 @@
>  #define R_YEAR  (0x6)
>  
>  #define R_DS1338_CTRL (0x7)
> +#define R_DS1375_CTRL (0xe)
>  
>  /* use 12 hour mode when set */
>  FIELD(HOUR, SET12, 6, 1)
> @@ -300,10 +302,34 @@ static const TypeInfo ds1338_info = {
>  .class_init= ds1338_class_init,
>  };
>  
> +static void ds1375_control_write(DSRTCState *s, uint8_t data)
> +{
> +/* just store it, we don't model any features */
> +s->nvram[R_DS1375_CTRL] = data;
> +}
> +
> +static void ds1375_class_init(ObjectClass *klass, void *data)
> +{
> +DSRTCClass *k = DSRTC_CLASS(klass);
> +
> +k->has_century = true;
> +k->addr_size = 0x20;
> +k->ctrl_offset = R_DS1375_CTRL;
> +k->ctrl_write = ds1375_control_write;
> +}
> +
> +static const TypeInfo ds1375_info = {
> +.name  = "ds1375",
> +.parent= TYPE_DSRTC,
> +.class_size= sizeof(DSRTCClass),
> +.class_init= ds1375_class_init,
> +};
> +
>  static void dsrtc_register_types(void)
>  {
>  type_register_static(&dsrtc_info);
>  type_register_static(&ds1338_info);
> +type_register_static(&ds1375_info);
>  }
>  
>  type_init(dsrtc_register_types)

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 12/14] timer: ds-rtc handle CENTURY bit

[David Gibson]

On Thu, Jul 05, 2018 at 11:19:59AM -0700, Michael Davidsaver wrote:
> Signed-off-by: Michael Davidsaver 
> Reviewed-by: Peter Maydell 

Reviewed-by: David Gibson 

> ---
>  hw/timer/ds-rtc.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/timer/ds-rtc.c b/hw/timer/ds-rtc.c
> index 6d3a8b5586..9abac38628 100644
> --- a/hw/timer/ds-rtc.c
> +++ b/hw/timer/ds-rtc.c
> @@ -70,6 +70,7 @@ typedef struct DSRTCState {
>  typedef struct DSRTCClass {
>  I2CSlaveClass parent_obj;
>  
> +bool has_century;
>  /* actual address space size must be <= NVRAM_SIZE */
>  unsigned addr_size;
>  unsigned ctrl_offset;
> @@ -93,6 +94,7 @@ static const VMStateDescription vmstate_dsrtc = {
>  
>  static void capture_current_time(DSRTCState *s)
>  {
> +DSRTCClass *k = DSRTC_GET_CLASS(s);
>  /* Capture the current time into the secondary registers
>   * which will be actually read by the data transfer operation.
>   */
> @@ -125,7 +127,10 @@ static void capture_current_time(DSRTCState *s)
>  }
>  s->nvram[R_DATE] = to_bcd(now.tm_mday);
>  s->nvram[R_MONTH] = to_bcd(now.tm_mon + 1);
> -s->nvram[R_YEAR] = to_bcd(now.tm_year - 100);
> +s->nvram[R_YEAR] = to_bcd(now.tm_year % 100u);
> +
> +ARRAY_FIELD_DP32(s->nvram, MONTH, CENTURY,
> + k->has_century && now.tm_year >= 100)
>  }
>  
>  static void inc_regptr(DSRTCState *s)
> @@ -282,6 +287,7 @@ static void ds1338_class_init(ObjectClass *klass, void 
> *data)
>  {
>  DSRTCClass *k = DSRTC_CLASS(klass);
>  
> +k->has_century = false;
>  k->addr_size = 0x40;
>  k->ctrl_offset = R_DS1338_CTRL;
>  k->ctrl_write = ds1338_control_write;

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL 0/4] Linux user for 3.0 patches

[Peter Maydell]

On 15 July 2018 at 20:52, Laurent Vivier  wrote:
> The following changes since commit 9277d81f5c2c6f4d0b5e47c8476eb7ee7e5c0beb:
>
>   docs: Grammar and spelling fixes (2018-07-13 10:16:04 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-3.0-pull-request
>
> for you to fetch changes up to 1d3d1b23e1c8f52ec431ddaa8deea1322bc25cbf:
>
>   Zero out the host's `msg_control` buffer (2018-07-15 16:04:38 +0200)
>
> 
> Some fixes for linux-user:
> - workaround for CMSG_NXTHDR bug
> - two patches for ppc64/ppc64le host:
>   fix fcntl() with *LK64 commands
>   (seen when dpkg wants to lock the DB)
>   fix reserved_va alignment (ppc64 needs
>   a 64kB alignment)
> - convert a forgotten fcntl() to safe_fcntl()

Applied, thanks.

-- PMM

Re: [Qemu-devel] [libvirt] [PULL 25/26] block: Remove deprecated -drive option serial

[Kashyap Chamarthy]

On Fri, Jul 13, 2018 at 01:35:02PM +0200, Cornelia Huck wrote:
> On Thu, 12 Jul 2018 17:47:00 +0200
> Thomas Huth  wrote:
> > On 12.07.2018 08:32, Markus Armbruster wrote:
> > > Daniel P. Berrangé  writes:  

[...]

> > > Our tool to help with getting cc: wrong less often is the MAINTAINERS
> > > file.  Could one of the libvirt developers watch changes to qemu-doc
> > > appendix "Deprecated features"?  Would putting the appendix in its own
> > > .texi help with that?  
> > 
> > Sound like a good idea to put the appendix in its own texi file. Then
> > add an "R: libvir-list" entry for that file to MAINTAINERS and we should
> > be fine (at least for the people who use the get_maintainers.pl script).
> 
> +1, like that idea
> 
> Are there other consumers of QEMU's interfaces which should be cc:ed in
> a similar way?

Perhaps starting with libvirt is fine -- as most of the open source
management applications rely on it.  (But if we do know other consumers,
then they can be trivially added.)

-- 
/kashyap

Re: [Qemu-devel] QEMU advent calendar

[Kashyap Chamarthy]

On Mon, Jul 16, 2018 at 11:15:43AM +0200, Thomas Huth wrote:
> On 13.07.2018 10:34, Richard Jansson wrote:
> > Hi
> > 
> > I checked in on the QEMU advent calendar side of which I have very fond
> > memories. I'm very impressed and grateful for the work that you've put into
> > this site. My question to you is the following, what are your plans for the
> > continuation of the program?
> 
>  Hi Richard,
> 
> thanks for the kind words and your interest in the QEMU advent calendar!
> This year, QEMU had its 15th anniversary, so we indeed already thought
> about doing a third round of the QEMU advent calendar this year...
> 
> > In case there's anything I can do I'll gladly put in some work to help you
> > out to support you in your noble cause :)
> 
> The problem is to come up with ideas for 24 suitable images [1], and to
> prepare them (it's quite time-consuming). So far, we're not certain yet
> whether we could do a 2018 edition of the calendar since we hardly have
> any images yet. So if you've got ideas for some images, and/or could
> help to prepare some, that would be very appreciated! Please get in
> touch with us off-list in that case (to avoid to spoil the surprise for
> the public).

Yeah, what Thomas said above.

That said, I've been on the fence for the 2018 edition.  As Thomas said,
we've been thinking about it for some months, and I've jotted down some
ideas and images; just haven't gotten around to make images and other
related tasks.  I've been trying to make time, but was consumed with
several things work and non-work related, that it was hard so far.  :-(

Still, there is time, and I don't want to "give up" -- it'll be nice to
do a 15-year edition.  Maybe I'll get renewed energy after discussions
at the KVM Forum in Edinburgh later this year (in October) and make
time.  And it's also nice to see renewed interest here!

-- 
/kashyap

Re: [Qemu-devel] [RFC 3/3] pci: Document ownership rules of pci_root_bus_new*()

[Marcel Apfelbaum]

On 07/12/2018 10:45 PM, Eduardo Habkost wrote:

The ownership rules of pci_root_bus_new*() aren't trivial: the
caller owns the new object if parent is NULL, otherwise ownership
is transferred to the parent.  Clarify that on comments.

Signed-off-by: Eduardo Habkost 
---
  include/hw/pci/pci.h | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 990d6fcbde..5d445c431c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -396,15 +396,30 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, 
int pin);
  
  bool pci_bus_is_express(PCIBus *bus);

  bool pci_bus_is_root(PCIBus *bus);
+
+/**
+ * pci_root_bus_new_inplace:
+ *
+* If @parent is not NULL the returned object will be owned by @parent,
+* otherwise it will be owned by the caller.
+*/
  void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState 
*parent,
const char *name,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
uint8_t devfn_min, const char *typename);
+
+/**
+ * pci_root_bus_new:
+ *
+* If @parent is not NULL the returned object will be owned by @parent,
+* otherwise it will be owned by the caller.
+*/
  PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
   MemoryRegion *address_space_mem,
   MemoryRegion *address_space_io,
   uint8_t devfn_min, const char *typename);
+
  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque, int nirq);
  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);


Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [RFC 2/3] qdev: Document ownership rules of qbus_create*()

[Marcel Apfelbaum]

On 07/12/2018 10:45 PM, Eduardo Habkost wrote:

The ownership rules of those functions aren't trivial: the caller
owns the new object if parent is NULL, otherwise ownership is
transferred to the parent.  Clarify that on comments.

Signed-off-by: Eduardo Habkost 
---
  include/hw/qdev-core.h | 24 
  hw/core/bus.c  |  5 +
  2 files changed, 29 insertions(+)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index f1fd0f8736..27d1ac3781 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -343,8 +343,32 @@ DeviceState *qdev_find_recursive(BusState *bus, const char 
*id);
  typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
  typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
  
+/**

+ * qbus_create_inplace:
+ * @bus: A pointer to the memory to be used for the bus object.
+ * @size: The maximum size available at @bus for the bus object.
+ * @typename: The name of the type of bus to instantiate.
+ * @parent: parent device
+ * @name: name for the new bus
+ *
+ * Creates bus in place.
+ *
+ * If @parent is not NULL the bus will be owned by @parent, otherwise
+ * the bus will be owned by the caller.
+ */
  void qbus_create_inplace(void *bus, size_t size, const char *typename,
   DeviceState *parent, const char *name);
+/**
+ * qbus_create:
+ * @typename: The name of the type of bus to instantiate.
+ * @parent: parent device
+ * @name: name for the new bus
+ *
+ * Creates bus object.
+ *
+ * If @parent is not NULL the returned object will be owned by @parent,
+ * otherwise it will be owned by the caller.
+ */
  BusState *qbus_create(const char *typename, DeviceState *parent, const char 
*name);
  /* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
   * < 0 if either devfn or busfn terminate walk somewhere in cursion,
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 4651f24486..68a0d5b085 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -74,6 +74,7 @@ int qbus_walk_children(BusState *bus,
  return 0;
  }
  
+/* If @parent is not NULL, ownership of @bus is transferred to @parent */

  static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
  {
  const char *typename = object_get_typename(OBJECT(bus));
@@ -102,6 +103,10 @@ static void qbus_realize(BusState *bus, DeviceState 
*parent, const char *name)
  QLIST_INSERT_HEAD(&bus->parent->child_bus, bus, sibling);
  bus->parent->num_child_bus++;
  object_property_add_child(OBJECT(bus->parent), bus->name, 
OBJECT(bus), NULL);
+/*
+ * object_property_add_child() takes a new reference, drop the
+ * reference that was transferred to us.
+ */
  object_unref(OBJECT(bus));
  } else if (bus != sysbus_get_default()) {
  /* TODO: once all bus devices are qdevified,


Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [PATCH 02/13] hw/pvrdma: Bugfix - provide the correct attr_mask to query_qp

[Marcel Apfelbaum]

On 07/16/2018 10:40 AM, Yuval Shaia wrote:

Calling rdma_rm_query_qp with attr_mask equals to -1 leads to error
where backend query_qp fails to retrieve the needed QP attributes.
Fix it by providing the attr_mask we got from driver.

Signed-off-by: Yuval Shaia 
---
  hw/rdma/vmw/pvrdma_cmd.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
index 14255d609f..e7d6589cdc 100644
--- a/hw/rdma/vmw/pvrdma_cmd.c
+++ b/hw/rdma/vmw/pvrdma_cmd.c
@@ -524,6 +524,7 @@ static int query_qp(PVRDMADev *dev, union pvrdma_cmd_req 
*req,
  struct ibv_qp_init_attr init_attr;
  
  pr_dbg("qp_handle=%d\n", cmd->qp_handle);

+pr_dbg("attr_mask=0x%x\n", cmd->attr_mask);
  
  memset(rsp, 0, sizeof(*rsp));

  rsp->hdr.response = cmd->hdr.response;
@@ -531,8 +532,8 @@ static int query_qp(PVRDMADev *dev, union pvrdma_cmd_req 
*req,
  
  rsp->hdr.err = rdma_rm_query_qp(&dev->rdma_dev_res, &dev->backend_dev,

  cmd->qp_handle,
-(struct ibv_qp_attr *)&resp->attrs, -1,
-&init_attr);
+(struct ibv_qp_attr *)&resp->attrs,
+cmd->attr_mask, &init_attr);
  
  pr_dbg("ret=%d\n", rsp->hdr.err);

  return rsp->hdr.err;



Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [PATCH 04/13] hw/pvrdma: Clean CQE before use

[Marcel Apfelbaum]

On 07/16/2018 10:40 AM, Yuval Shaia wrote:

Next CQE is fetched from CQ ring, clean it before usage as it still
carries old CQE values.

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

diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
index 99bb5e..a8664f40c8 100644
--- a/hw/rdma/vmw/pvrdma_qp_ops.c
+++ b/hw/rdma/vmw/pvrdma_qp_ops.c
@@ -69,6 +69,7 @@ static int pvrdma_post_cqe(PVRDMADev *dev, uint32_t cq_handle,
  return -EINVAL;
  }
  
+memset(cqe1, 0, sizeof(*cqe1));

  cqe1->wr_id = cqe->wr_id;
  cqe1->qp = cqe->qp;
  cqe1->opcode = cqe->opcode;


Reviewed-by: Marcel Apfelbaum



Thanks,

Marcel

Re: [Qemu-devel] [PATCH 05/13] hw/pvrdma: Make default pkey 0xFFFF

[Marcel Apfelbaum]

On 07/16/2018 10:40 AM, Yuval Shaia wrote:

0x7FFF is not the default pkey - fix it.

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

diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
index e7d6589cdc..bb898265a3 100644
--- a/hw/rdma/vmw/pvrdma_cmd.c
+++ b/hw/rdma/vmw/pvrdma_cmd.c
@@ -166,7 +166,7 @@ static int query_pkey(PVRDMADev *dev, union pvrdma_cmd_req 
*req,
  resp->hdr.ack = PVRDMA_CMD_QUERY_PKEY_RESP;
  resp->hdr.err = 0;
  
-resp->pkey = 0x7FFF;

+resp->pkey = 0x;
  pr_dbg("pkey=0x%x\n", resp->pkey);
  
  return 0;


A MACRO will be more readable. It doesn't worth a re-spin though.

Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [PATCH 06/13] hw/rdma: Get rid of unneeded structure

[Marcel Apfelbaum]

On 07/16/2018 10:40 AM, Yuval Shaia wrote:

This structure make no sense - removing it.
     What structure? I would explicitly name it in both subject 
and message.


Thanks,
Marcel



Signed-off-by: Yuval Shaia 
---
  hw/rdma/rdma_backend.c |  3 +--
  hw/rdma/rdma_rm.c  | 16 
  hw/rdma/rdma_rm_defs.h | 10 +++---
  3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 647e16399f..52981d652d 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -271,8 +271,7 @@ static int build_host_sge_array(RdmaDeviceResources 
*rdma_dev_res,
  return VENDOR_ERR_INVLKEY | ssge[ssge_idx].lkey;
  }
  
-dsge->addr = (uintptr_t)mr->user_mr.host_virt + ssge[ssge_idx].addr -

- mr->user_mr.guest_start;
+dsge->addr = (uintptr_t)mr->virt + ssge[ssge_idx].addr - mr->start;
  dsge->length = ssge[ssge_idx].length;
  dsge->lkey = rdma_backend_mr_lkey(&mr->backend_mr);
  
diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c

index 415da15efe..7403d24674 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -165,15 +165,15 @@ int rdma_rm_alloc_mr(RdmaDeviceResources *dev_res, 
uint32_t pd_handle,
  length = TARGET_PAGE_SIZE;
  addr = g_malloc(length);
  } else {
-mr->user_mr.host_virt = host_virt;
-pr_dbg("host_virt=0x%p\n", mr->user_mr.host_virt);
-mr->user_mr.length = guest_length;
+mr->virt = host_virt;
+pr_dbg("host_virt=0x%p\n", mr->virt);
+mr->length = guest_length;
  pr_dbg("length=%zu\n", guest_length);
-mr->user_mr.guest_start = guest_start;
-pr_dbg("guest_start=0x%" PRIx64 "\n", mr->user_mr.guest_start);
+mr->start = guest_start;
+pr_dbg("guest_start=0x%" PRIx64 "\n", mr->start);
  
-length = mr->user_mr.length;

-addr = mr->user_mr.host_virt;
+length = mr->length;
+addr = mr->virt;
  }
  
  ret = rdma_backend_create_mr(&mr->backend_mr, &pd->backend_pd, addr, length,

@@ -214,7 +214,7 @@ void rdma_rm_dealloc_mr(RdmaDeviceResources *dev_res, 
uint32_t mr_handle)
  
  if (mr) {

  rdma_backend_destroy_mr(&mr->backend_mr);
-munmap(mr->user_mr.host_virt, mr->user_mr.length);
+munmap(mr->virt, mr->length);
  res_tbl_dealloc(&dev_res->mr_tbl, mr_handle);
  }
  }
diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
index 226011176d..7228151239 100644
--- a/hw/rdma/rdma_rm_defs.h
+++ b/hw/rdma/rdma_rm_defs.h
@@ -55,16 +55,12 @@ typedef struct RdmaRmCQ {
  bool notify;
  } RdmaRmCQ;
  
-typedef struct RdmaRmUserMR {

-void *host_virt;
-uint64_t guest_start;
-size_t length;
-} RdmaRmUserMR;
-
  /* MR (DMA region) */
  typedef struct RdmaRmMR {
  RdmaBackendMR backend_mr;
-RdmaRmUserMR user_mr;
+void *virt;
+uint64_t start;
+size_t length;
  uint32_t pd_handle;
  uint32_t lkey;
  uint32_t rkey;

Re: [Qemu-devel] [PATCH 09/13] hw/pvrdma: Cosmetic change - indent right

[Marcel Apfelbaum]

On 07/16/2018 10:40 AM, Yuval Shaia wrote:

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

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 1b1330e113..3d448bffc4 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -430,7 +430,7 @@ static void regs_write(void *opaque, hwaddr addr, uint64_t 
val, unsigned size)
  reset_device(dev);
  break;
  }
-break;
+break;
  case PVRDMA_REG_IMR:
  pr_dbg("Interrupt mask=0x%" PRIx64 "\n", val);
  dev->interrupt_mask = val;
@@ -439,7 +439,7 @@ static void regs_write(void *opaque, hwaddr addr, uint64_t 
val, unsigned size)
  if (val == 0) {
  execute_command(dev);
  }
-break;
+break;
  default:
  break;
  }

Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [PATCH 10/13] hw/rdma: Cosmetic change - move to generic function

[Marcel Apfelbaum]

On 07/16/2018 10:40 AM, Yuval Shaia wrote:

To ease maintenance of struct comp_thread move all related code to
dedicated function.

Signed-off-by: Yuval Shaia 
---
  hw/rdma/rdma_backend.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 52981d652d..d29acc505b 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -146,10 +146,10 @@ static void *comp_handler_thread(void *arg)
  return NULL;
  }
  
-static void stop_comp_thread(RdmaBackendDev *backend_dev)

+static void stop_backend_thread(RdmaBackendThread *thread)
  {
-backend_dev->comp_thread.run = false;
-while (backend_dev->comp_thread.is_running) {
+thread->run = false;
+while (thread->is_running) {
  pr_dbg("Waiting for thread to complete\n");
  sleep(THR_POLL_TO / SCALE_US / 2);
  }
@@ -159,7 +159,7 @@ static void start_comp_thread(RdmaBackendDev *backend_dev)
  {
  char thread_name[THR_NAME_LEN] = {0};
  
-stop_comp_thread(backend_dev);

+stop_backend_thread(&backend_dev->comp_thread);
  
  snprintf(thread_name, sizeof(thread_name), "rdma_comp_%s",

   ibv_get_device_name(backend_dev->ib_dev));
@@ -876,7 +876,7 @@ void rdma_backend_start(RdmaBackendDev *backend_dev)
  void rdma_backend_stop(RdmaBackendDev *backend_dev)
  {
  pr_dbg("Stopping rdma_backend\n");
-stop_comp_thread(backend_dev);
+stop_backend_thread(&backend_dev->comp_thread);
  }
  
  void rdma_backend_fini(RdmaBackendDev *backend_dev)


Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [PATCH 08/13] hw/rdma: Reorder resource cleanup

[Marcel Apfelbaum]

On 07/16/2018 10:40 AM, Yuval Shaia wrote:

To be consistence with allocation do the reverse order in deallocation

Signed-off-by: Yuval Shaia 
---
  hw/rdma/rdma_rm.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index bf4a5c71b4..1f014b4ab2 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -543,8 +543,9 @@ void rdma_rm_fini(RdmaDeviceResources *dev_res)
  res_tbl_free(&dev_res->uc_tbl);
  res_tbl_free(&dev_res->cqe_ctx_tbl);
  res_tbl_free(&dev_res->qp_tbl);
-res_tbl_free(&dev_res->cq_tbl);
  res_tbl_free(&dev_res->mr_tbl);
+res_tbl_free(&dev_res->cq_tbl);
  res_tbl_free(&dev_res->pd_tbl);
+
  g_hash_table_destroy(dev_res->qp_hash);
  }



Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

Re: [Qemu-devel] [PATCH 11/13] hw/rdma: Print backend QP number in hex format

[Marcel Apfelbaum]

On 07/16/2018 10:40 AM, Yuval Shaia wrote:

To be consistent with other prints throughout the code fix places that
print it as decimal number.

Signed-off-by: Yuval Shaia 
---
  hw/rdma/rdma_rm.c   | 4 ++--
  hw/rdma/vmw/pvrdma_qp_ops.c | 4 ++--
  2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index 1f014b4ab2..859c93 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -389,7 +389,7 @@ int rdma_rm_modify_qp(RdmaDeviceResources *dev_res, 
RdmaBackendDev *backend_dev,
  RdmaRmQP *qp;
  int ret;
  
-pr_dbg("qpn=%d\n", qp_handle);

+pr_dbg("qpn=0x%x\n", qp_handle);
  
  qp = rdma_rm_get_qp(dev_res, qp_handle);

  if (!qp) {
@@ -447,7 +447,7 @@ int rdma_rm_query_qp(RdmaDeviceResources *dev_res, 
RdmaBackendDev *backend_dev,
  {
  RdmaRmQP *qp;
  
-pr_dbg("qpn=%d\n", qp_handle);

+pr_dbg("qpn=0x%x\n", qp_handle);
  
  qp = rdma_rm_get_qp(dev_res, qp_handle);

  if (!qp) {
diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
index a8664f40c8..c668afd0ed 100644
--- a/hw/rdma/vmw/pvrdma_qp_ops.c
+++ b/hw/rdma/vmw/pvrdma_qp_ops.c
@@ -130,7 +130,7 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle)
  PvrdmaSqWqe *wqe;
  PvrdmaRing *ring;
  
-pr_dbg("qp_handle=%d\n", qp_handle);

+pr_dbg("qp_handle=0x%x\n", qp_handle);
  
  qp = rdma_rm_get_qp(&dev->rdma_dev_res, qp_handle);

  if (unlikely(!qp)) {
@@ -174,7 +174,7 @@ int pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle)
  PvrdmaRqWqe *wqe;
  PvrdmaRing *ring;
  
-pr_dbg("qp_handle=%d\n", qp_handle);

+pr_dbg("qp_handle=0x%x\n", qp_handle);
  
  qp = rdma_rm_get_qp(&dev->rdma_dev_res, qp_handle);

  if (unlikely(!qp)) {


Reviewed-by: Marcel Apfelbaum

Thanks,
Marcel

[Qemu-devel] [RFC v2 1/2] arm: Add nRF51 GPIO peripheral

[Steffen Görtz]

Signed-off-by: Steffen Görtz 
---
 Changes in v2:
   - Only call QEMU GPIO update handlers if value changes
   - Code style changes
   - Removed unused includes

 hw/gpio/Makefile.objs|   1 +
 hw/gpio/nrf51_gpio.c | 320 +++
 include/hw/gpio/nrf51_gpio.h |  57 +++
 3 files changed, 378 insertions(+)
 create mode 100644 hw/gpio/nrf51_gpio.c
 create mode 100644 include/hw/gpio/nrf51_gpio.h

diff --git a/hw/gpio/Makefile.objs b/hw/gpio/Makefile.objs
index fa0a72e6d0..e5da0cb54f 100644
--- a/hw/gpio/Makefile.objs
+++ b/hw/gpio/Makefile.objs
@@ -8,3 +8,4 @@ common-obj-$(CONFIG_GPIO_KEY) += gpio_key.o
 obj-$(CONFIG_OMAP) += omap_gpio.o
 obj-$(CONFIG_IMX) += imx_gpio.o
 obj-$(CONFIG_RASPI) += bcm2835_gpio.o
+obj-$(CONFIG_NRF51_SOC) += nrf51_gpio.o
diff --git a/hw/gpio/nrf51_gpio.c b/hw/gpio/nrf51_gpio.c
new file mode 100644
index 00..033d013882
--- /dev/null
+++ b/hw/gpio/nrf51_gpio.c
@@ -0,0 +1,320 @@
+/*
+ * nRF51 System-on-Chip general purpose input/output register definition
+ *
+ * Reference Manual: http://infocenter.nordicsemi.com/pdf/nRF51_RM_v3.0.pdf
+ * Product Spec: http://infocenter.nordicsemi.com/pdf/nRF51822_PS_v3.1.pdf
+ *
+ * Copyright 2018 Steffen Görtz 
+ *
+ * This code is licensed under the GPL version 2 or later.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/gpio/nrf51_gpio.h"
+
+#define NRF51_GPIO_SIZE 0x1000
+
+#define NRF51_GPIO_REG_OUT  0x504
+#define NRF51_GPIO_REG_OUTSET   0x508
+#define NRF51_GPIO_REG_OUTCLR   0x50C
+#define NRF51_GPIO_REG_IN   0x510
+#define NRF51_GPIO_REG_DIR  0x514
+#define NRF51_GPIO_REG_DIRSET   0x518
+#define NRF51_GPIO_REG_DIRCLR   0x51C
+#define NRF51_GPIO_REG_CNF_START0x700
+#define NRF51_GPIO_REG_CNF_END  0x77F
+
+#define GPIO_PULLDOWN 1
+#define GPIO_PULLUP 3
+
+#ifndef DEBUG_NRF51_GPIO
+#define DEBUG_NRF51_GPIO 0
+#endif
+
+#define DPRINTF(fmt, args...) \
+do { \
+if (DEBUG_NRF51_GPIO) { \
+fprintf(stderr, "[%s]%s: " fmt , TYPE_NRF51_GPIO, \
+ __func__, ##args); \
+} \
+} while (0)
+
+static uint64_t gpio_read(void *opaque, hwaddr offset, unsigned int size)
+{
+Nrf51GPIOState *s = NRF51_GPIO(opaque);
+uint64_t r = 0;
+size_t idx;
+
+switch (offset) {
+case NRF51_GPIO_REG_OUT ... NRF51_GPIO_REG_OUTCLR:
+DPRINTF("read out\n");
+r = s->out;
+break;
+case NRF51_GPIO_REG_IN:
+DPRINTF("read in\n");
+r = s->in;
+break;
+case NRF51_GPIO_REG_DIR ... NRF51_GPIO_REG_DIRCLR:
+DPRINTF("read dir\n");
+r = s->dir;
+break;
+case NRF51_GPIO_REG_CNF_START ... NRF51_GPIO_REG_CNF_END:
+idx = (offset - NRF51_GPIO_REG_CNF_START) / 4;
+DPRINTF("read config %d\n", (uint32_t)idx);
+r = s->cnf[idx];
+break;
+
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: bad read offset 0x%" HWADDR_PRIx "\n",
+  __func__, offset);
+}
+
+return r;
+}
+
+/**
+ * Check if the output driver is connected to the direction switch
+ * given the current configuration and logic level.
+ * It is not differentiated between standard and "high"(-power) drive modes.
+ */
+static bool is_connected(uint32_t config, uint32_t level)
+{
+bool state;
+uint32_t drive_config = extract32(config, 8, 3);
+
+switch (drive_config) {
+case 0 ... 3:
+state = true;
+break;
+case 4 ... 5:
+state = level != 0;
+break;
+case 6 ... 7:
+state = level == 0;
+break;
+}
+
+return state;
+}
+
+static void update_output_irq(Nrf51GPIOState *s, size_t i,
+  bool connected, bool level)
+{
+bool old_connected = extract32(s->old_out_connected, i, 1);
+bool old_level = extract32(s->old_out, i, 1);
+
+if ((old_connected != connected) || (old_level != level)) {
+if (connected) {
+DPRINTF("qemu output to %s\n", level ? "HIGH" : "LOW");
+qemu_set_irq(s->output[i], level);
+} else {
+DPRINTF("qemu output to %s\n", "DISCONNECTED");
+qemu_set_irq(s->output[i], -1);
+}
+}
+
+s->old_out = deposit32(s->old_out, i, 1, level);
+s->old_out_connected = deposit32(s->old_out_connected, i, 1, connected);
+}
+
+static void gpio_update_state(Nrf51GPIOState *s)
+{
+uint32_t pull;
+bool connected_out, dir, connected_in, out, input;
+
+for (size_t i = 0; i < NRF51_GPIO_PINS; i++) {
+DPRINTF("=== calc update for state %zu ===\n", i);
+
+pull = extract32(s->cnf[i], 2, 2);
+dir = extract32(s->cnf[i], 0, 1);
+connected_in = extract32(s->in_mask, i, 1);
+out = extract32(s->out, i, 1);
+input = !extract32(s->cnf[i], 1, 1);
+connected_out = is_connected(s->cn

[Qemu-devel] [RFC v2 2/2] arm: Add nRF51 GPIO tests

[Steffen Görtz]

Signed-off-by: Steffen Görtz 
---
 tests/microbit-test.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tests/microbit-test.c b/tests/microbit-test.c
index c502ee3976..a1e2f67855 100644
--- a/tests/microbit-test.c
+++ b/tests/microbit-test.c
@@ -101,6 +101,12 @@ static void test_nrf51_nvmc(void)
 }
 }
 
+static void test_nrf51_gpio(void)
+{
+/* TODO add tests for GPIO */
+g_assert_true(false);
+}
+
 int main(int argc, char **argv)
 {
 int ret;
@@ -110,6 +116,7 @@ int main(int argc, char **argv)
 global_qtest = qtest_startf("-machine microbit");
 
 qtest_add_func("/microbit/nrf51/nvmc", test_nrf51_nvmc);
+qtest_add_func("/microbit/nrf51/gpio", test_nrf51_gpio);
 
 ret = g_test_run();
 
-- 
2.18.0

Re: [Qemu-devel] [PATCH 13/13] hw/rdma: Save pci dev in backend_dev

[Marcel Apfelbaum]

On 07/16/2018 10:40 AM, Yuval Shaia wrote:

This field is not initialized - fix it.


 What field ? Please use a more readable message.

Regarding the subject, maybe you can use "cache pci_dev"
or "add a reference to pci_dev " instead of "save".

Thanks,
Marcel



Signed-off-by: Yuval Shaia 
---
  hw/rdma/rdma_backend.c| 6 +-
  hw/rdma/rdma_backend.h| 2 +-
  hw/rdma/vmw/pvrdma_main.c | 2 +-
  3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index d29acc505b..d7a4bbd91f 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -744,7 +744,7 @@ static int init_device_caps(RdmaBackendDev *backend_dev,
  return 0;
  }
  
-int rdma_backend_init(RdmaBackendDev *backend_dev,

+int rdma_backend_init(RdmaBackendDev *backend_dev, PCIDevice *pdev,
RdmaDeviceResources *rdma_dev_res,
const char *backend_device_name, uint8_t port_num,
uint8_t backend_gid_idx, struct ibv_device_attr 
*dev_attr,
@@ -756,6 +756,10 @@ int rdma_backend_init(RdmaBackendDev *backend_dev,
  struct ibv_device **dev_list;
  struct ibv_port_attr port_attr;
  
+memset(backend_dev, 0, sizeof(*backend_dev));

+
+backend_dev->dev = pdev;
+
  backend_dev->backend_gid_idx = backend_gid_idx;
  backend_dev->port_num = port_num;
  backend_dev->rdma_dev_res = rdma_dev_res;
diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h
index 3049a73962..86e8fe8ab6 100644
--- a/hw/rdma/rdma_backend.h
+++ b/hw/rdma/rdma_backend.h
@@ -46,7 +46,7 @@ static inline uint32_t rdma_backend_mr_rkey(const 
RdmaBackendMR *mr)
  return mr->ibmr ? mr->ibmr->rkey : 0;
  }
  
-int rdma_backend_init(RdmaBackendDev *backend_dev,

+int rdma_backend_init(RdmaBackendDev *backend_dev, PCIDevice *pdev,
RdmaDeviceResources *rdma_dev_res,
const char *backend_device_name, uint8_t port_num,
uint8_t backend_gid_idx, struct ibv_device_attr 
*dev_attr,
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 3d448bffc4..ca5fa8d981 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -611,7 +611,7 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
  goto out;
  }
  
-rc = rdma_backend_init(&dev->backend_dev, &dev->rdma_dev_res,

+rc = rdma_backend_init(&dev->backend_dev, pdev, &dev->rdma_dev_res,
 dev->backend_device_name, dev->backend_port_num,
 dev->backend_gid_idx, &dev->dev_attr, errp);
  if (rc) {

Re: [Qemu-devel] [Spice-devel] Ongoing migration: gitlab.com/spice -> gitlab.freedesktop.org/spice

[Victor Toso]

Hi,

On Thu, Jul 12, 2018 at 05:50:56PM +0200, Victor Toso wrote:
> Hi,
> 
> JFYI, CC'ing:
> 
> * qemu-devel
> * virt-tools-list
> * de...@ovirt.org
> 
> If nobody complains, I'll finish whatever is pending next Monday
> morning (GMT+2).

Migration is done. All Spice related components can be seen at
https://gitlab.freedesktop.org/spice.

A read-only mirror is being kept at https://gitlab.com/spice.

Cheers,
toso

> 
> Cheers,
> toso
> 
> On Thu, Jul 12, 2018 at 02:12:43PM +0200, Victor Toso wrote:
> > Hi,
> > 
> > The official repository for Spice components should stay in
> > gitlab.freedesktop.org/spice - As suggested in the previous
> > thread [0], instead of closing gitlab.com/spice we can keep it
> > read-only mirror from Freedesktop's gitlab instance.
> > 
> > [0] https://lists.freedesktop.org/archives/spice-devel/2018-July/044664.html
> > 
> > Please, read the email below to see what's going on :)
> > 
> > At this moment, I've only changed components that were already a
> > mirror of gitlab.freedesktop.org, the first part of this email is
> > about the projects that are not a mirror.
> > 
> > This email's intent is to inform, discuss and give some time to
> > let contributors know about the change. People CC'ed are to be
> > affected.
> > 
> > [*] Components to be created in gitlab.freedesktop.org/spice and
> > mirrored from there.
> > 
> > * https://gitlab.com/spice/x11spice
> > * https://gitlab.com/spice/spice-space
> > * https://gitlab.com/spice/spice-space-pages
> > * https://gitlab.com/spice/spice-streaming-agent
> > 
> > * https://gitlab.com/spice/virtio-gpu-wddm/virtio-gpu-wddm-dod
> > - Taking the opportunity, should we move this under win32
> >   subgroup or create the virtio-gpu-wddm subgroup once more?
> > - Migrate issue #1
> > 
> > * https://gitlab.com/spice/qxl-wddm-dod
> > - Update and mirror from 
> > https://gitlab.freedesktop.org/spice/win32/qxl-wddm-dod
> > - The mirrored in gitlab will also be moved into win32 subgroup
> > 
> > * https://gitlab.com/spice/spice-nsis
> > - Update and mirror from https://gitlab.freedesktop.org/spice/spice-nsis
> > - Should we also include under win32 subgroup or stay outside?
> > 
> > [*] Components I'm ignoring on this mirroring due too old or
> > untouched or unrelated (qemu?) code
> > 
> > * win32/usbclerk
> > * win32/vdi_port
> > * qemu 
> > * slirp
> > * spicec
> > * vdesktop
> > 
> > 
> > [*] Components that are already mirrored, some updates around
> > them below:
> > 
> > * The mirrored components will be always overwritten when
> >   branches diverge, please don't push to gitlab.com - about
> >   read-only option:
> >   - https://gitlab.com/gitlab-org/gitlab-ce/issues/40891
> > 
> > * Added common description: "Read only repository, mirror from
> >   Freedesktop's instance of Gitlab"
> > 
> > * General permissions
> >  - Visibility: Public
> >  - Issues, Merge request, Wiki, etc: Disabled
> > 
> > No problems seen:
> > * spice [ https://gitlab.com/spice/spice ]
> > * spice-protocol [ https://gitlab.com/spice/spice-protocol ]
> > * spice-common [ https://gitlab.com/spice/spice-common ]
> > * usbredir [ https://gitlab.com/spice/usbredir ]
> > * spice-jhbuild [ https://gitlab.com/spice/spice-jhbuild ]
> > * spice-html5 [ https://gitlab.com/spice/spice-html5 ]
> > * libcacard [ https://gitlab.com/spice/libcacard ]
> > 
> > Small problem but fixed:
> > * Spice-gtk [ https://gitlab.com/spice/spice-gtk ]
> >   - I had to delete and mirror again due some ongoing failure to
> > pull from freedesktop.org
> > 
> > * linux/vd_agent [ https://gitlab.com/spice/linux/vd_agent ]
> >   - I had to create the linux subgroup and clone a new instance
> >   - removed spice/vdagent one (previous)
> > 
> > * win32/vd_agent [ https://gitlab.com/spice/win32/vd_agent ]
> >   - I had to create the win32 subgroup and clone a new instance
> >   - removed spice/vdagent-win (previous)
> > 
> > * win32/usbdk [ https://gitlab.com/spice/win32/usbdk ]
> >   - Moved to win32 subgroup (delete + clone again)
> >   - removed spice/usbdk (previous)
> > 
> > * win32/qxl [ https://gitlab.com/spice/win32/qxl ]
> >   - Moved to win32 subgroup (delete + clone again)
> >   - removed spice/qxl-win (previous)
> > 
> > I hope all is good to have everything migrated early next week :)
> > PS: Let me know if I forgot something.
> > 
> > Cheers,
> > toso
> 
> 
> 
> > ___
> > Spice-devel mailing list
> > spice-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 



> ___
> Spice-devel mailing list
> spice-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



signature.asc
Description: PGP signature

[Qemu-devel] [PATCH] hw/char/serial: retry write if EAGAIN

[Marc-André Lureau]

If the chardev returns -1 with EAGAIN errno on write(), it should try
to send it again (EINTR is handled by the chardev itself).

This fixes commit 019288bf137183bf3407c9824655b753bfafc99f
"hw/char/serial: Only retry if qemu_chr_fe_write returns 0"

Tested-by: Igor Mammedov 
Signed-off-by: Marc-André Lureau 
---
 hw/char/serial.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index cd7d747c68..8bd1da2fa3 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -261,15 +261,20 @@ static void serial_xmit(SerialState *s)
 if (s->mcr & UART_MCR_LOOP) {
 /* in loopback mode, say that we just received a char */
 serial_receive1(s, &s->tsr, 1);
-} else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) == 0 &&
-   s->tsr_retry < MAX_XMIT_RETRY) {
-assert(s->watch_tag == 0);
-s->watch_tag =
-qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
-  serial_watch_cb, s);
-if (s->watch_tag > 0) {
-s->tsr_retry++;
-return;
+} else {
+int rc = qemu_chr_fe_write(&s->chr, &s->tsr, 1);
+
+if ((rc == 0 ||
+ (rc == -1 && errno == EAGAIN)) &&
+s->tsr_retry < MAX_XMIT_RETRY) {
+assert(s->watch_tag == 0);
+s->watch_tag =
+qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
+  serial_watch_cb, s);
+if (s->watch_tag > 0) {
+s->tsr_retry++;
+return;
+}
 }
 }
 s->tsr_retry = 0;
-- 
2.18.0.129.ge3331758f1

Re: [Qemu-devel] [PATCH v2] trace/simple: fix hang in child after fork(2)

[Cornelia Huck]

On Sun, 15 Jul 2018 14:42:23 +0200
Paolo Bonzini  wrote:

> On 13/07/2018 20:42, Stefan Hajnoczi wrote:
> > +#ifndef _WIN32
> > +static void stop_writeout_thread(void)
> > +{
> > +g_mutex_lock(&trace_lock);
> > +trace_writeout_running = false;
> > +g_cond_signal(&trace_available_cond);
> > +g_mutex_unlock(&trace_lock);
> > +
> > +g_thread_join(trace_writeout_thread);
> > +trace_writeout_thread = NULL;
> > +}  
> 
> After stop_writeout_thread returns, another could start a write to the
> shared data structure---and the write would never finish, because the
> thread disappears after fork(2) returns.  This would leave the mutex
> locked, causing a deadlock soon after the fork.  So you need to lock
> trace_lock again here, and unlock it in restart_writeout_thread.

So, I suppose there will be a v3, right?

> 
> Apart from this, it looks good!

Did a quick run and it fixed the problems for me.

[Qemu-devel] tcg_register_jit_int() interface problem

[Aleksandar Markovic]

Hello, all.


Just a little background:


One of the fields in an ELF header is e_machine. It is meant to contain 
"machine architecture". The numerical value is approved by a committee. Some 
values for common targets are: EM_PPC, EM_PPC64, EM_ARM, EM_AARCH64, etc.


The TCG function tcg_register_jit_int() expects that the preprocessor constant 
ELF_HOST_MACHINE is defined, and uses internally the value of that constant.


Consider, let's say, ARM: It defines ELF_HOST_MACHINE in 
tcg/arm/tcg-target.inc.c as


#define ELF_HOST_MACHINE EM_ARM


while in tcg/aarch64/tcg-target.inc.c it defines it as


#define ELF_HOST_MACHINE EM_AARCH64


For MIPS, the situation is different: Up until recently, MIPS had only one 
value for e_machine: that was EM_MIPS (used for both 32-bit and 64-bit MIPS 
systems). However, new nanoMIPS systems have a new value in that field: 
EM_NANOMIPS (that is to be applied for both 32-bit and 64-bit nanoMIPS systems).


The situation described above represent a problem for us developing nanoMIPS 
support, forcing us to have different compilations (and executables) for MIPS 
and nanoMIPS.


Is there a possibility to refactor tcg_register_jit_int() to receive 
"e_machine" value as an argument, instead of expecting preprocessor constant to 
be set?


Please advice if you have something better in mind.


Yours,

Aleksandar


(cc. Laurent and Riku since this is also indirectly related to handling ELF 
headers linux user mode, and there will be a couple other nanoMIPS patches 
comming related to that.)

Re: [Qemu-devel] tcg_register_jit_int() interface problem

[Laurent Vivier]

Le 16/07/2018 à 13:10, Aleksandar Markovic a écrit :
> Hello, all.
> 
> 
> Just a little background:
> 
> 
> One of the fields in an ELF header is e_machine. It is meant to contain
> "machine architecture". The numerical value is approved by a committee.
> Some values for common targets are: EM_PPC, EM_PPC64, EM_ARM,
> EM_AARCH64, etc.
> 
> 
> The TCG function tcg_register_jit_int() expects that the preprocessor
> constant ELF_HOST_MACHINE is defined, and uses internally the value of
> that constant.
> 
> 
> Consider, let's say, ARM: It definesELF_HOST_MACHINE in
> tcg/arm/tcg-target.inc.c as
> 
> 
> #define ELF_HOST_MACHINE EM_ARM
> 
> 
> while in tcg/aarch64/tcg-target.inc.c it defines it as
> 
> 
> #define ELF_HOST_MACHINE EM_AARCH64
> 
> 
> For MIPS, the situation is different: Up until recently, MIPS had only
> one value for e_machine: that was EM_MIPS (used for both 32-bit and
> 64-bit MIPS systems). However, new nanoMIPS systems have a new value in
> that field: EM_NANOMIPS (that is to be applied for both 32-bit and
> 64-bit nanoMIPS systems).
> 
> 
> The situation described above represent a problem for us developing
> nanoMIPS support, forcing us to have different compilations (and
> executables) for MIPS and nanoMIPS.
> 
> 
> Is there a possibility to refactor tcg_register_jit_int() to receive
> "e_machine" value as an argument, instead of expecting preprocessor
> constant to be set?

I don't know this par of code, but for PPC we have:

#if TCG_TARGET_REG_BITS == 64
# define ELF_HOST_MACHINE EM_PPC64
#else
# define ELF_HOST_MACHINE EM_PPC
#endif

So perhaps you could have something like:

#if defined(HOST_NANOMIPS)
# define ELF_HOST_MACHINE EM_NANOMIPS
#else
# define ELF_HOST_MACHINE EM_MIPS
#endif

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 06/16] hw/display/xlnx_dp: Move problematic code from instance_init to realize

[Thomas Huth]

On 13.07.2018 19:13, Paolo Bonzini wrote:
> On 13/07/2018 17:59, Thomas Huth wrote:
>> Your patch looks good at a first quick glance, but it seems not to work as 
>> expected: When I now run QEMU like this:
>>
>> echo "{'execute':'qmp_capabilities'}" \
>>  "{'execute':'device-list-properties'," \
>>  "'arguments':{'typename':'xlnx,zynqmp'}}" \
>>  "{'execute': 'human-monitor-command', " \
>>  "'arguments': {'command-line': 'info qtree'}}" | \
>>  aarch64-softmmu/qemu-system-aarch64 -M none,accel=qtest -qmp stdio
>>
>> then QEMU ends up in an endless loop and I've got to kill it.
> 
> There are two more bugs that my patch makes un-latent, where the
> objects are created but not added as children.  Therefore when
> you call object_unparent on them, nothing happens.
> 
> In particular dpcd and edid give you an infinite loop in bus_unparent,
> because device_unparent is not called and does not remove them from
> the list of devices on the bus.
> 
> The following incremental changes fix everything for me.  Note that
> aux_create_slave/qdev_create already do the unref for you.

Thanks, that fixes the problem, indeed. I'll squash this into your patch
and send out a v3 series.

 Thomas

Re: [Qemu-devel] tcg_register_jit_int() interface problem

[Aleksandar Markovic]

> From: Laurent Vivier 
> Sent: Monday, July 16, 2018 1:30 PM
>
> Le 16/07/2018 à 13:10, Aleksandar Markovic a écrit :
> > Hello, all.
> >
> >
> > Just a little background:
> >
> >
> > One of the fields in an ELF header is e_machine. It is meant to contain
> > "machine architecture". The numerical value is approved by a committee.
> > Some values for common targets are: EM_PPC, EM_PPC64, EM_ARM,
> > EM_AARCH64, etc.
> >
> >
> > The TCG function tcg_register_jit_int() expects that the preprocessor
> > constant ELF_HOST_MACHINE is defined, and uses internally the value of
> > that constant.
> >
> >
> > Consider, let's say, ARM: It definesELF_HOST_MACHINE in
> > tcg/arm/tcg-target.inc.c as
> >
> >
> > #define ELF_HOST_MACHINE EM_ARM
> >
> >
> > while in tcg/aarch64/tcg-target.inc.c it defines it as
> >
> >
> > #define ELF_HOST_MACHINE EM_AARCH64
> >
> >
> > For MIPS, the situation is different: Up until recently, MIPS had only
> > one value for e_machine: that was EM_MIPS (used for both 32-bit and
> > 64-bit MIPS systems). However, new nanoMIPS systems have a new value in
> > that field: EM_NANOMIPS (that is to be applied for both 32-bit and
> > 64-bit nanoMIPS systems).
> >
> >
> > The situation described above represent a problem for us developing
> > nanoMIPS support, forcing us to have different compilations (and
> > executables) for MIPS and nanoMIPS.
> >
> >
> > Is there a possibility to refactor tcg_register_jit_int() to receive
> > "e_machine" value as an argument, instead of expecting preprocessor
> > constant to be set?
>
> I don't know this par of code, but for PPC we have:
>
> #if TCG_TARGET_REG_BITS == 64
> # define ELF_HOST_MACHINE EM_PPC64
> #else
> # define ELF_HOST_MACHINE EM_PPC
> #endif
>
> So perhaps you could have something like:
>
> #if defined(HOST_NANOMIPS)
> # define ELF_HOST_MACHINE EM_NANOMIPS
> #else
> # define ELF_HOST_MACHINE EM_MIPS
> #endif

The problem is, this means there will be separate executables for QEMU for MIPS 
and QEMU for nanoMIPS. Would that be acceptable from overall QEMU design point 
of view? Peter, Richard?

Regards,
Aleksandar

Re: [Qemu-devel] [PATCH 1/2] qga-win: prevent crash when executing fsinfo command

[Sameeh Jubran]

I've found another bug which is related to this one, where pci_controller
might also be null:
in the function "build_guest_disk_info" in "qga/commands-win32.c"

if (bus == BusTypeScsi || bus == BusTypeAta || bus == BusTypeRAID

#if (_WIN32_WINNT >= 0x0600)

/* This bus type is not supported before Windows Server 2003 SP1 */

|| bus == BusTypeSas

#endif

) {

/* We are able to use the same ioctls for different bus types

* according to Microsoft docs

* https://technet.microsoft.com/en-us/library/ee851589(v=ws.10).aspx */

if (DeviceIoControl(vol_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,

sizeof(SCSI_ADDRESS), &len, NULL)) {

disk->unit = addr.Lun;

disk->target = addr.TargetId;

disk->bus = addr.PathId;

disk->pci_controller = get_pci_info(name, errp);

}

/* We do not set error in this case, because we still have enough

* information about volume. */

} else {

disk->pci_controller = NULL;

}

This can be easily prevented by always calling "get_pci_info(name, errp);",
is this a separate patch, or should be combined with this one? what do you
think?

On Tue, Jul 10, 2018 at 1:58 AM, Michael Roth 
wrote:

> Quoting Sameeh Jubran (2018-06-28 18:33:58)
> >
> >
> > On Fri, Jun 29, 2018 at 12:44 AM, Eric Blake  wrote:
> >
> > On 06/26/2018 10:10 AM, Sameeh Jubran wrote:
> >
> > From: Sameeh Jubran 
> >
> > The fsinfo command is currently implemented for Windows only and
> it's
> > disk
> > parameter can be enabled by adding the define
> "CONFIG_QGA_NTDDSCSI" to
> > the qga
> > code. When enabled and executed the qemu-ga crashed with the
> following
> > message:
> >
> > 
> > File qapi/qapi-visit-core.c, Line 49
> >
> > Expression: !(v->type & VISITOR_OUTPUT) || *obj)
> > 
> >
> > After some digging, turns out that the GuestPCIAddress is null
> and the
> > qapi visitor doesn't like that, so we can always allocate it
> instead
> > and
> > initiate all it's members to -1.
> >
> >
> > Adding Markus for a qapi back-compat question.
> >
> > Is faking an invalid address better than making the output optional
> > instead?
> >
> >
> > +++ b/qga/commands-win32.c
> > @@ -485,6 +485,11 @@ static GuestPCIAddress *get_pci_info(char
> *guid,
> > Error **errp)
> >   char *buffer = NULL;
> >   GuestPCIAddress *pci = NULL;
> >   char *name = g_strdup(&guid[4]);
> > +pci = g_malloc0(sizeof(*pci));
> > +pci->domain = -1;
> > +pci->slot = -1;
> > +pci->function = -1;
> > +pci->bus = -1;
> >
> >
> > Right now, we have:
> >
> > ##
> > # @GuestDiskAddress:
> > #
> > # @pci-controller: controller's PCI address
> > # @bus-type: bus type
> > # @bus: bus id
> > # @target: target id
> > # @unit: unit id
> > #
> > # Since: 2.2
> > ##
> > { 'struct': 'GuestDiskAddress',
> >   'data': {'pci-controller': 'GuestPCIAddress',
> >'bus-type': 'GuestDiskBusType',
> >'bus': 'int', 'target': 'int', 'unit': 'int'} }
> >
> > and the problem you ran into is that under certain conditions, we
> have no
> > idea what to populate in GuestPCIAddress.  Your patch populates
> garbage
> > instead; but I'm wondering if it would be better to instead mark
> > pci-controller as optional, where code that CAN populate it would set
> > has_pci_controller=true, and the code that crashed will now work by
> leaving
> > the struct NULL (and has_pci_controller=false).  But that removes
> output
> > that the client may expect to be present, hence, this is a
> back-compat
> > question of the best way to approach this.
> >
> > Since all of the fields are ints, I believe that "-1" is very
> informative even
> > though I don't like this approach but it does preserve backward
> compatibility
> > as you've already clarified.
> >
> > I don't want to assume anything but this bug has been laying around for
> quite a
> > while and no one complained, moreover,  the disk option is not enabled by
> > default in upstream code so I don't think that backward compatibility is
> > actually disturbed but then again this is just an assumption.
> >
> > One more thing to consider is the second patch of this series which
> actually
> > was the root cause of why we didn't allocate the " GuestDiskAddress"
> struct
> > which is some registry keys being absent for the specific device which
> would
> > leave us with two options:
> > 1. Don not return anything for all of the fields (leave it null)
> > 2. Return partial information
> > I think that the second option is preferable for obvious reasons.
> >
> > I am not that familiar with schema files, but it is possible to make int
> fields
> > optional too?
> >
> > I can live with either 

Re: [Qemu-devel] [PATCH] qga-win: Handle fstrim for OSes lower than Win8

[Sameeh Jubran]

ping.

On Sun, Jun 24, 2018 at 3:45 PM, Sameeh Jubran  wrote:

> From: Sameeh Jubran 
>
> The defrag.exe tool which is used for executing the fstrim command
> on Windows doesn't support retrim for OSes lower than Win8. This
> commit handles this case and returns a suitable error.
>
> Output of fstrim before this commit:
> {"execute":"guest-fstrim"}
> {"return": {"paths": [{"path": "C:\\", "error": "An invalid command line
> option
> was specified. (0x8908)"}, {"path": "F:\\", "error": "An invalid
> command
> line option was specified. (0x8908)"}, {"path": "S:\\", "error": "An
> invalid command line option was specified. (0x8908)"}]}}
>
> Reported on:
> https://bugzilla.redhat.com/show_bug.cgi?id=1594113
>
> Signed-off-by: Sameeh Jubran 
> ---
>  qga/commands-win32.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index d79974f212..0bdcd9dd38 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -30,6 +30,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include "qga/guest-agent-core.h"
>  #include "qga/vss-win32.h"
> @@ -852,6 +853,11 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum,
> Error **errp)
>  HANDLE handle;
>  WCHAR guid[MAX_PATH] = L"";
>
> +   if (!IsWindows8OrGreater()) {
> +error_setg(errp, "fstrim is only supported for Win8+");
> +return NULL;
> +   }
> +
>  handle = FindFirstVolumeW(guid, ARRAYSIZE(guid));
>  if (handle == INVALID_HANDLE_VALUE) {
>  error_setg_win32(errp, GetLastError(), "failed to find any
> volume");
> --
> 2.13.6
>
>


-- 
Respectfully,
*Sameeh Jubran*
*Linkedin *
*Software Engineer @ Daynix .*

Re: [Qemu-devel] [RFC v3 2/2] virtio-pmem: Add virtio pmem driver

[Pankaj Gupta]

> 
> > This patch adds virtio-pmem driver for KVM guest.
> > 
> > Guest reads the persistent memory range information from Qemu over
> > VIRTIO and registers it on nvdimm_bus. It also creates a nd_region
> > object with the persistent memory range information so that existing
> > 'nvdimm/pmem' driver can reserve this into system memory map. This way
> > 'virtio-pmem' driver uses existing functionality of pmem driver to
> > register persistent memory compatible for DAX capable filesystems.
> > 
> > This also provides function to perform guest flush over VIRTIO from
> > 'pmem' driver when userspace performs flush on DAX memory range.
> > 
> > Signed-off-by: Pankaj Gupta 
> > ---
> >  drivers/virtio/Kconfig   |   9 ++
> >  drivers/virtio/Makefile  |   1 +
> >  drivers/virtio/virtio_pmem.c | 190
> >  +++
> >  include/linux/virtio_pmem.h  |  44 +
> >  include/uapi/linux/virtio_ids.h  |   1 +
> >  include/uapi/linux/virtio_pmem.h |  40 +
> >  6 files changed, 285 insertions(+)
> >  create mode 100644 drivers/virtio/virtio_pmem.c
> >  create mode 100644 include/linux/virtio_pmem.h
> >  create mode 100644 include/uapi/linux/virtio_pmem.h
> > 
> > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> > index 3589764..a331e23 100644
> > --- a/drivers/virtio/Kconfig
> > +++ b/drivers/virtio/Kconfig
> > @@ -42,6 +42,15 @@ config VIRTIO_PCI_LEGACY
> >  
> >   If unsure, say Y.
> >  
> > +config VIRTIO_PMEM
> > +   tristate "Support for virtio pmem driver"
> > +   depends on VIRTIO
> > +   help
> > +   This driver provides support for virtio based flushing interface
> > +   for persistent memory range.
> > +
> > +   If unsure, say M.
> > +
> >  config VIRTIO_BALLOON
> > tristate "Virtio balloon driver"
> > depends on VIRTIO
> > diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> > index 3a2b5c5..cbe91c6 100644
> > --- a/drivers/virtio/Makefile
> > +++ b/drivers/virtio/Makefile
> > @@ -6,3 +6,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
> >  virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
> >  obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
> >  obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
> > +obj-$(CONFIG_VIRTIO_PMEM) += virtio_pmem.o
> > diff --git a/drivers/virtio/virtio_pmem.c b/drivers/virtio/virtio_pmem.c
> > new file mode 100644
> > index 000..6200b5e
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_pmem.c
> > @@ -0,0 +1,190 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * virtio_pmem.c: Virtio pmem Driver
> > + *
> > + * Discovers persistent memory range information
> > + * from host and provides a virtio based flushing
> > + * interface.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +
> > +static struct virtio_device_id id_table[] = {
> > +   { VIRTIO_ID_PMEM, VIRTIO_DEV_ANY_ID },
> > +   { 0 },
> > +};
> > +
> > + /* The interrupt handler */
> > +static void host_ack(struct virtqueue *vq)
> > +{
> > +   unsigned int len;
> > +   unsigned long flags;
> > +   struct virtio_pmem_request *req;
> > +   struct virtio_pmem *vpmem = vq->vdev->priv;
> > +
> > +   spin_lock_irqsave(&vpmem->pmem_lock, flags);
> > +   while ((req = virtqueue_get_buf(vq, &len)) != NULL) {
> > +   req->done = true;
> > +   wake_up(&req->acked);
> > +   }
> > +   spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
> 
> Honest question: why do you need to disable interrupts here?

To avoid interrupt for VQ trying to take same spinlock already taken by process 
context and resulting in deadlock. Looks like interrupts are already disabled 
in 
function call, see [1]. But still to protect with any future work. 

[1]
   vp_interrupt
   vp_vring_interrupt
   vring_interrupt
> 
> > +}
> > + /* Initialize virt queue */
> > +static int init_vq(struct virtio_pmem *vpmem)
> > +{
> > +   struct virtqueue *vq;
> > +
> > +   /* single vq */
> > +   vpmem->req_vq = vq = virtio_find_single_vq(vpmem->vdev,
> > +   host_ack, "flush_queue");
> > +   if (IS_ERR(vq))
> > +   return PTR_ERR(vq);
> > +   spin_lock_init(&vpmem->pmem_lock);
> > +
> > +   return 0;
> > +};
> > +
> > + /* The request submission function */
> > +static int virtio_pmem_flush(struct device *dev)
> > +{
> > +   int err;
> > +   unsigned long flags;
> > +   struct scatterlist *sgs[2], sg, ret;
> > +   struct virtio_device *vdev = dev_to_virtio(dev->parent->parent);
> > +   struct virtio_pmem *vpmem = vdev->priv;
> > +   struct virtio_pmem_request *req = kmalloc(sizeof(*req), GFP_KERNEL);
> 
> Not checking kmalloc() return.

Will add it.
> 
> > +
> > +   req->done = false;
> > +   init_waitqueue_head(&req->acked);
> > +   spin_lock_irqsave(&vpmem->pmem_lock, flags);
> 
> Why do you need spin_lock_irqsave()? There are two points consider:
> 
> 1. Will virtio_pmem_flush() ever be called with interrupts disabled?
>If yes, then it's broken since you should be using

Re: [Qemu-devel] [PATCH V1 RESEND 1/6] hmat acpi: Build Memory Subsystem Address Range Structure(s) in ACPI HMAT

[Igor Mammedov]

On Tue, 19 Jun 2018 23:20:52 +0800
Liu Jingqi  wrote:

> HMAT is defined in ACPI 6.2: 5.2.27 Heterogeneous Memory Attribute Table 
> (HMAT).
> The specification references below link:
> http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> 
> It describes the memory attributes, such as memory side cache
> attributes and bandwidth and latency details, related to the
> System Physical Address (SPA) Memory Ranges. The software is
> expected to use this information as hint for optimization.
> 
> This structure describes the System Physical Address(SPA) range
> occupied by memory subsystem and its associativity with processor
> proximity domain as well as hint for memory usage.
> 
> Signed-off-by: Liu Jingqi 
> ---
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/acpi/Makefile.objs  |   1 +
>  hw/acpi/hmat.c | 139 
> +
>  hw/acpi/hmat.h |  73 +++
>  hw/i386/acpi-build.c   | 120 
>  hw/i386/acpi-build.h   |  10 +++
>  include/sysemu/numa.h  |   2 +
>  numa.c |   6 ++
>  8 files changed, 307 insertions(+), 45 deletions(-)
>  create mode 100644 hw/acpi/hmat.c
>  create mode 100644 hw/acpi/hmat.h
> 
> diff --git a/default-configs/x86_64-softmmu.mak 
> b/default-configs/x86_64-softmmu.mak
> index 0390b43..3b4a37d 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -66,3 +66,4 @@ CONFIG_I2C=y
>  CONFIG_SEV=$(CONFIG_KVM)
>  CONFIG_VTD=y
>  CONFIG_AMD_IOMMU=y
> +CONFIG_ACPI_HMAT=y
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index 11c35bc..21889fd 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -6,6 +6,7 @@ common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
>  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
>  common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> +common-obj-$(CONFIG_ACPI_HMAT) += hmat.o
>  common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
>  
>  common-obj-y += acpi_interface.o
> diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
> new file mode 100644
> index 000..d4e586d
> --- /dev/null
> +++ b/hw/acpi/hmat.c
> @@ -0,0 +1,139 @@
> +/*
> + * HMAT ACPI Implementation
> + *
> + * Copyright(C) 2018 Intel Corporation.
> + *
> + * Author:
> + *  Liu jingqi 
> + *
> + * HMAT is defined in ACPI 6.2.
> + *
> + * 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 "unistd.h"
> +#include "fcntl.h"
> +#include "qemu/osdep.h"
> +#include "sysemu/numa.h"
> +#include "hw/i386/pc.h"
> +#include "hw/i386/acpi-build.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/hmat.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/nvram/fw_cfg.h"
> +#include "hw/acpi/bios-linker-loader.h"
Do all this headers are really needed here?


> +/* Build Memory Subsystem Address Range Structure */
> +static void hmat_build_spa_info(GArray *table_data,
> +uint64_t base, uint64_t length, int node)
how about:

s/hmat_build_spa_info/build_hmat_spa/

1st put 'build_" prefix then table "hmat_" prefix and at last entry name
and drop any else.

and similar changes to other function names

> +{
> +uint16_t flags = 0;
> +
> +if (numa_info[node].is_initiator) {
> +flags |= HMAT_SPA_PROC_VALID;
> +}
> +if (numa_info[node].is_target) {
> +flags |= HMAT_SPA_MEM_VALID;
> +}
SPA entry doesn't care about node at all,
I'd figure flags value in the caller and pass it to hmat_build_spa_info()
as an argument

> +
> +/* Type */
> +build_append_int_noprefix(table_data, ACPI_HMAT_SPA, sizeof(uint16_t));
sizeof(type) here and below is rater pointless,
just use hardcoded numbers here, so it would be closer to spec table
also  ACPI_HMAT_SPA is used only once and probably won't be ever reused
so I'd replace it wit a number and comment being verbatim copy from Sspec
like:

   /* Memory Subsystem Address Range Structure */
   build_append_int_noprefix(table_data, 0, 2);



> +/* Reserved0 */
there is no Reseved0 in spec

> +build_append_int_noprefix(table_data, 0, sizeof(uint16_t));
> +/* Length */
> +build_append_int_noprefix(table_data, sizeof(AcpiHmatSpaRange

Re: [Qemu-devel] tcg_register_jit_int() interface problem

[Peter Maydell]

On 16 July 2018 at 12:36, Aleksandar Markovic  wrote:
>> From: Laurent Vivier 
>> Le 16/07/2018 à 13:10, Aleksandar Markovic a écrit :
>> > For MIPS, the situation is different: Up until recently, MIPS had only
>> > one value for e_machine: that was EM_MIPS (used for both 32-bit and
>> > 64-bit MIPS systems). However, new nanoMIPS systems have a new value in
>> > that field: EM_NANOMIPS (that is to be applied for both 32-bit and
>> > 64-bit nanoMIPS systems).
>> >
>> >
>> > The situation described above represent a problem for us developing
>> > nanoMIPS support, forcing us to have different compilations (and
>> > executables) for MIPS and nanoMIPS.
>> >
>> >
>> > Is there a possibility to refactor tcg_register_jit_int() to receive
>> > "e_machine" value as an argument, instead of expecting preprocessor
>> > constant to be set?
>>
>> I don't know this par of code, but for PPC we have:
>>
>> #if TCG_TARGET_REG_BITS == 64
>> # define ELF_HOST_MACHINE EM_PPC64
>> #else
>> # define ELF_HOST_MACHINE EM_PPC
>> #endif
>>
>> So perhaps you could have something like:
>>
>> #if defined(HOST_NANOMIPS)
>> # define ELF_HOST_MACHINE EM_NANOMIPS
>> #else
>> # define ELF_HOST_MACHINE EM_MIPS
>> #endif
>
> The problem is, this means there will be separate executables for QEMU for
> MIPS and QEMU for nanoMIPS. Would that be acceptable from overall QEMU
> design point of view? Peter, Richard?

The assumption has previously been that there must be a value known
at compile time (because it would be the value of whatever is in the
ELF header for the QEMU executable itself, effectively). I'm not
sure why this doesn't work for MIPS still -- if you're building
on a host with the new nanoMIPS stuff and with that support enabled,
then report EM_NANOMIPS; if you're building a binary that works with
the older MIPS CPUs, report EM_MIPS ?

That said, if this doesn't work for MIPS, then it would be a trivial
refactor to allow the tcg per-target code to do
#define ELF_HOST_MACHINE mips_get_elf_host_machine()
so you could compute the value at runtime.

thanks
-- PMM

Re: [Qemu-devel] [PATCH V1 RESEND 0/6] Build ACPI Heterogeneous Memory Attribute Table (HMAT)

[Igor Mammedov]

On Tue, 19 Jun 2018 23:20:51 +0800
Liu Jingqi  wrote:

> This series of patches will build Heterogeneous Memory Attribute Table (HMAT)
> according to the command line. The ACPI HMAT describes the memory attributes,
> such as memory side cache attributes and bandwidth and latency details,
> related to the System Physical Address (SPA) Memory Ranges.
> The software is expected to use this information as hint for optimization.
> 
> OSPM evaluates HMAT only during system initialization. Any changes to the HMAT
> state at runtime or information regarding HMAT for hot plug are communicated
> using the _HMA method.
> 
> Liu Jingqi (6):
>   hmat acpi: Build Memory Subsystem Address Range Structure(s) in ACPI
> HMAT
you've converted this patch to build_append_int_noprefix() API as requested

>   hmat acpi: Build System Locality Latency and Bandwidth Information
> Structure(s) in ACPI HMAT
>   hmat acpi: Build Memory Side Cache Information Structure(s) in ACPI
> HMAT
but left out above 2 with the same issues.
So I'd repeat,
using "struct FOO {} packed" is discouraged, you should use
build_append_int_noprefix() API to build ACPI tables

>   numa: Extend the command-line to provide memory latency and bandwidth
> information
>   numa: Extend the command-line to provide memory side cache information
>   hmat acpi: Implement _HMA method to update HMAT at runtime
> 
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/acpi/Makefile.objs  |   1 +
>  hw/acpi/hmat.c | 649 
> +
>  hw/acpi/hmat.h | 264 +++
>  hw/i386/acpi-build.c   | 122 ---
>  hw/i386/acpi-build.h   |  10 +
>  hw/i386/pc.c   |   2 +
>  hw/i386/pc_piix.c  |   3 +
>  hw/i386/pc_q35.c   |   3 +
>  include/hw/i386/pc.h   |   2 +
>  include/sysemu/numa.h  |   2 +
>  numa.c | 202 
>  qapi/misc.json | 160 -
>  qemu-options.hx|  28 +-
>  14 files changed, 1401 insertions(+), 48 deletions(-)
>  create mode 100644 hw/acpi/hmat.c
>  create mode 100644 hw/acpi/hmat.h
> 

Re: [Qemu-devel] [PATCH V1 RESEND 6/6] hmat acpi: Implement _HMA method to update HMAT at runtime

[Igor Mammedov]

On Tue, 19 Jun 2018 23:20:57 +0800
Liu Jingqi  wrote:

> OSPM evaluates HMAT only during system initialization.
> Any changes to the HMAT state at runtime or information
> regarding HMAT for hot plug are communicated using _HMA method.
> 
> _HMA is an optional object that enables the platform to provide
> the OS with updated Heterogeneous Memory Attributes information
> at runtime. _HMA provides OSPM with the latest HMAT in entirety
> overriding existing HMAT.

this patch is too big and lacks any documentation how this thing
is supposed to work.
Pls restructure and split in mode sensible chunks.

Now beside above ranting I noticed that it's build using
NFIT as template. However it's adding extra ABI and
a lot of complex code on both qemu/AML sides to transfer
updated HMAT table to guest similar to NFIT.

I don't think that duplicating NFIT approach for every new
table is sustainable both in terms of consuming limited
IO/memory resources and maintainability (too much
complex code duplication and extra ABI to keep stable).

We should generalize/reuse NFIT code and ABI (io/memory buffer)
that intersects with this series first and then build _HMA update
on top of it.


> Signed-off-by: Liu Jingqi 
> ---
>  hw/acpi/hmat.c   | 356 
> +++
>  hw/acpi/hmat.h   |  71 ++
>  hw/i386/acpi-build.c |   2 +
>  hw/i386/pc.c |   2 +
>  hw/i386/pc_piix.c|   3 +
>  hw/i386/pc_q35.c |   3 +
>  include/hw/i386/pc.h |   2 +
>  7 files changed, 439 insertions(+)
> 
> diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
> index 9d29ef7..cf17c0a 100644
> --- a/hw/acpi/hmat.c
> +++ b/hw/acpi/hmat.c
> @@ -275,6 +275,267 @@ static void hmat_build_hma(GArray *hma, PCMachineState 
> *pcms)
>  hmat_build_cache(hma);
>  }
>  
> +static uint64_t
> +hmat_hma_method_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +printf("BUG: we never read _HMA IO Port.\n");
> +return 0;
> +}
> +
> +/* _HMA Method: read HMA data. */
> +static void hmat_handle_hma_method(AcpiHmaState *state,
> +   HmatHmamIn *in, hwaddr hmam_mem_addr)
> +{
> +HmatHmaBuffer *hma_buf = &state->hma_buf;
> +HmatHmamOut *read_hma_out;
> +GArray *hma;
> +uint32_t read_len = 0, ret_status;
> +int size;
> +
> +le32_to_cpus(&in->offset);
> +
> +hma = hma_buf->hma;
> +if (in->offset > hma->len) {
> +ret_status = HMAM_RET_STATUS_INVALID;
> +goto exit;
> +}
> +
> +   /* It is the first time to read HMA. */
> +if (!in->offset) {
> +hma_buf->dirty = false;
> +} else if (hma_buf->dirty) { /* HMA has been changed during Reading HMA. 
> */
> +ret_status = HMAM_RET_STATUS_HMA_CHANGED;
> +goto exit;
> +}
> +
> +ret_status = HMAM_RET_STATUS_SUCCESS;
> +read_len = MIN(hma->len - in->offset,
> +   HMAM_MEMORY_SIZE - 2 * sizeof(uint32_t));
> +exit:
> +size = sizeof(HmatHmamOut) + read_len;
> +read_hma_out = g_malloc(size);
> +
> +read_hma_out->len = cpu_to_le32(size);
> +read_hma_out->ret_status = cpu_to_le32(ret_status);
> +memcpy(read_hma_out->data, hma->data + in->offset, read_len);
> +
> +cpu_physical_memory_write(hmam_mem_addr, read_hma_out, size);
> +
> +g_free(read_hma_out);
> +}
> +
> +static void
> +hmat_hma_method_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +AcpiHmaState *state = opaque;
> +hwaddr hmam_mem_addr = val;
> +HmatHmamIn *in;
> +
> +in = g_new(HmatHmamIn, 1);
> +cpu_physical_memory_read(hmam_mem_addr, in, sizeof(*in));
> +
> +hmat_handle_hma_method(state, in, hmam_mem_addr);
> +}
> +
> +static const MemoryRegionOps hmat_hma_method_ops = {
> +.read = hmat_hma_method_read,
> +.write = hmat_hma_method_write,
> +.endianness = DEVICE_LITTLE_ENDIAN,
> +.valid = {
> +.min_access_size = 4,
> +.max_access_size = 4,
> +},
> +};
> +
> +static void hmat_init_hma_buffer(HmatHmaBuffer *hma_buf)
> +{
> +hma_buf->hma = g_array_new(false, true /* clear */, 1);
> +}
> +
> +static uint8_t hmat_acpi_table_checksum(uint8_t *buffer, uint32_t length)
> +{
> +uint8_t sum = 0;
> +uint8_t *end = buffer + length;
> +
> +while (buffer < end) {
> +sum = (uint8_t) (sum + *(buffer++));
> +}
> +return (uint8_t)(0 - sum);
> +}
> +
> +static void hmat_build_header(AcpiTableHeader *h,
> + const char *sig, int len, uint8_t rev,
> + const char *oem_id, const char *oem_table_id)
> +{
> +memcpy(&h->signature, sig, 4);
> +h->length = cpu_to_le32(len);
> +h->revision = rev;
> +
> +if (oem_id) {
> +strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
> +} else {
> +memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
> +}
> +
> +if (oem_table_id) {
> +strncpy((char *)h->oem_table_id, oem_table_id, 
> sizeof(h->oem_table_id));
> +} else {
> +memcpy(h->oem_table_id, ACPI

Re: [Qemu-devel] [PATCH 0/6] accel/tcg: Support execution from MMIO and small MMU regions

[KONRAD Frederic]

Hi Peter,

Nice! Thanks for that.

A little question though.. What will happen in the case where the
CPU start executing code at random place because of eg: a badly
configured kernel?

Seeing the patch 5 I guess it will really execute stuff.. So
maybe this is less user-friendly?

Cheers,
Fred

On 07/10/2018 06:00 PM, Peter Maydell wrote:

This series adds support to TCG for executing from MMIO regions
and small MMU regions. The basic principle is that if get_page_addr_code()
finds that the region is not backed by a full page of RAM then it
returns -1, and tb_gen_code() then generates a non-cached TB
containing a single instruction. Execution from these regions
thus performs the instruction fetch every time, ensuring that we
get the read-from-MMIO and check-small-MMU-region permissions
checks right.

This means that the code path for "generate bus fault for failing
to load an instruction" no longer goes through get_page_addr_code(),
but instead via each target's translate code and its calls to
the cpu_ld*_code() or similar functions. Patch 1 makes sure we
can distinguish insn fetches from data loads when generating the
bus fault exceptions. (Aside: I have assumed that all cpu_ld*_code()
loads should trigger iside faults rather than dside. Hopefully this
is true...)

Patches 2 and 3 make trivial fixes to various callers of
get_page_addr_code(); patch 4 does the work of generating our
single-insn TBs. Patch 5 can then remove all the code that
(mis)handles MMIO regions from get_page_addr_code(). Finally
patch 6 drops the target/arm workarounds for not having support
for executing from small MPU regions.

Note for the Xilinx folks: this patchset makes the mmio-exec
testcase for running from the SPI flash pass. Cedric: you might
like to test the aspeed image you had that relies on execution
from an MMIO region too.

The diffstat is pretty satisfying for a patchset that adds
a feature, but it actually undersells it: this code renders the
hw/misc/mmio_interface.c and the mmio_ptr related code in memory.c
and the xilinx-spips device all obsolete, so there are another
couple of hundred lines of code to be deleted there. I opted not
to include that in this patchset, for ease of review.

NB: I tested this with icount, but there are potentially
some weird things that could happen with interactions between
icount's io-recompile and execution from an MMIO device
that returns different instructions each time it's read.

thanks
-- PMM


Peter Maydell (6):
   accel/tcg: Pass read access type through to io_readx()
   accel/tcg: Handle get_page_addr_code() returning -1 in hashtable
 lookups
   accel/tcg: Handle get_page_addr_code() returning -1 in
 tb_check_watchpoint()
   accel/tcg: tb_gen_code(): Create single-insn TB for execution from
 non-RAM
   accel/tcg: Return -1 for execution from MMIO regions in
 get_page_addr_code()
   target/arm: Allow execution from small regions

  accel/tcg/softmmu_template.h |  11 ++--
  include/qom/cpu.h|   6 +++
  accel/tcg/cpu-exec.c |   3 ++
  accel/tcg/cputlb.c   | 100 +--
  accel/tcg/translate-all.c|  23 +++-
  memory.c |   3 +-
  target/arm/helper.c  |  23 
  7 files changed, 52 insertions(+), 117 deletions(-)

Re: [Qemu-devel] [PATCH v2 2/2] virtio-scsi: fix hotplug ->reset() vs event race

[Michael S. Tsirkin]

On Mon, Jul 16, 2018 at 09:37:32AM +0100, Stefan Hajnoczi wrote:
> There is a race condition during hotplug when iothread is used.  It
> occurs because virtio-scsi may be processing command queues in the
> iothread while the monitor performs SCSI device hotplug.
> 
> When a SCSI device is hotplugged the HotplugHandler->plug() callback is
> invoked and virtio-scsi emits a rescan event to the guest.
> 
> If the guest submits a SCSI command at this point then it may be
> cancelled before hotplug completes.  This happens because ->reset() is
> called by hw/core/qdev.c:device_set_realized() after
> HotplugHandler->plug() has been called and
> hw/scsi/scsi-disk.c:scsi_disk_reset() purges all requests.
> 
> This patch uses the new HotplugHandler->post_plug() callback to emit the
> rescan event after ->reset().  This eliminates the race conditions where
> requests could be cancelled.
> 
> Reported-by: l00284672 
> Cc: Paolo Bonzini 
> Cc: Fam Zheng 
> Signed-off-by: Stefan Hajnoczi 

Acked-by: Michael S. Tsirkin 

Pls merge through scsi tree.

> ---
>  hw/scsi/virtio-scsi.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 3aa99717e2..5a3057d1f8 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -797,8 +797,16 @@ static void virtio_scsi_hotplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>  virtio_scsi_acquire(s);
>  blk_set_aio_context(sd->conf.blk, s->ctx);
>  virtio_scsi_release(s);
> -
>  }
> +}
> +
> +/* Announce the new device after it has been plugged */
> +static void virtio_scsi_post_hotplug(HotplugHandler *hotplug_dev,
> + DeviceState *dev)
> +{
> +VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev);
> +VirtIOSCSI *s = VIRTIO_SCSI(vdev);
> +SCSIDevice *sd = SCSI_DEVICE(dev);
>  
>  if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
>  virtio_scsi_acquire(s);
> @@ -968,6 +976,7 @@ static void virtio_scsi_class_init(ObjectClass *klass, 
> void *data)
>  vdc->start_ioeventfd = virtio_scsi_dataplane_start;
>  vdc->stop_ioeventfd = virtio_scsi_dataplane_stop;
>  hc->plug = virtio_scsi_hotplug;
> +hc->post_plug = virtio_scsi_post_hotplug;
>  hc->unplug = virtio_scsi_hotunplug;
>  }
>  
> -- 
> 2.17.1

Re: [Qemu-devel] [PATCH 4/7 V9] hostmem-file: add the 'pmem' option

[Igor Mammedov]

On Tue, 10 Jul 2018 18:30:17 +0800
junyan...@gmx.com wrote:

> From: Junyan He 
> 
> When QEMU emulates vNVDIMM labels and migrates vNVDIMM devices, it
> needs to know whether the backend storage is a real persistent memory,
> in order to decide whether special operations should be performed to
> ensure the data persistence.
> 
> This boolean option 'pmem' allows users to specify whether the backend
> storage of memory-backend-file is a real persistent memory. If
> 'pmem=on', QEMU will set the flag RAM_PMEM in the RAM block of the
> corresponding memory region. If 'pmem' is set while lack of libpmem
> support, a error is generated.
> 
> Signed-off-by: Junyan He 
> Signed-off-by: Haozhong Zhang 
> Reviewed-by: Stefan Hajnoczi 
See a nit below, with it fixed:

Reviewed-by: Igor Mammedov 

> ---
>  backends/hostmem-file.c | 41 -
>  docs/nvdimm.txt | 22 ++
>  exec.c  |  8 
>  include/exec/memory.h   |  4 
>  include/exec/ram_addr.h |  3 +++
>  qemu-options.hx |  7 +++
>  6 files changed, 84 insertions(+), 1 deletion(-)
> 
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 34c68bb..b1a2453 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -12,6 +12,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu-common.h"
> +#include "qemu/error-report.h"
>  #include "sysemu/hostmem.h"
>  #include "sysemu/sysemu.h"
>  #include "qom/object_interfaces.h"
> @@ -34,6 +35,7 @@ struct HostMemoryBackendFile {
>  bool discard_data;
>  char *mem_path;
>  uint64_t align;
> +bool is_pmem;
>  };
>  
>  static void
> @@ -59,7 +61,8 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
> **errp)
>  memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>   path,
>   backend->size, fb->align,
> - backend->share ? RAM_SHARED : 0,
> + (backend->share ? RAM_SHARED : 0) |
> + (fb->is_pmem ? RAM_PMEM : 0),
>   fb->mem_path, errp);
>  g_free(path);
>  }
> @@ -131,6 +134,39 @@ static void file_memory_backend_set_align(Object *o, 
> Visitor *v,
>  error_propagate(errp, local_err);
>  }
>  
> +static bool file_memory_backend_get_pmem(Object *o, Error **errp)
> +{
> +return MEMORY_BACKEND_FILE(o)->is_pmem;
> +}
> +
> +static void file_memory_backend_set_pmem(Object *o, bool value, Error **errp)
> +{
> +HostMemoryBackend *backend = MEMORY_BACKEND(o);
> +HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
> +
> +if (host_memory_backend_mr_inited(backend)) {
> +error_setg(errp, "cannot change property 'pmem' of %s '%s'",
> +   object_get_typename(o),
> +   object_get_canonical_path_component(o));
> +return;
> +}
> +
> +#ifndef CONFIG_LIBPMEM
> +if (value) {
> +Error *local_err = NULL;
> +error_setg(&local_err,
> +   "Lack of libpmem support while setting the 'pmem=on'"
> +   " of %s '%s'. We can't ensure data persistence.",
> +   object_get_typename(o),
> +   object_get_canonical_path_component(o));
> +error_propagate(errp, local_err);
> +return;
> +}
> +#endif
> +
> +fb->is_pmem = value;
> +}
> +
>  static void file_backend_unparent(Object *obj)
>  {
>  HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> @@ -162,6 +198,9 @@ file_backend_class_init(ObjectClass *oc, void *data)
>  file_memory_backend_get_align,
>  file_memory_backend_set_align,
>  NULL, NULL, &error_abort);
> +object_class_property_add_bool(oc, "pmem",
> +file_memory_backend_get_pmem, file_memory_backend_set_pmem,
> +&error_abort);
>  }
>  
>  static void file_backend_instance_finalize(Object *o)
> diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
> index 24b443b..8c83baf 100644
> --- a/docs/nvdimm.txt
> +++ b/docs/nvdimm.txt
> @@ -173,3 +173,25 @@ There are currently two valid values for this option:
>   the NVDIMMs in the event of power loss.  This implies that the
>   platform also supports flushing dirty data through the memory
>   controller on power loss.
> +
> +If the vNVDIMM backend is on the host persistent memory that can be
> +accessed in SNIA NVM Programming Model [1] (e.g., Intel NVDIMM), it's
> +suggested to set the 'pmem' option of memory-backend-file to 'on'. When
> +'pmem' is 'on' and QEMU is built with libpmem [2] support (configured with
> +--enable-libpmem), QEMU will take necessary operations to guarantee the
> +persistence of its own writes to the vNVDIMM backend(e.g., in vNVDIMM label
> +emulation and live migration). If 'pmem' is 'on' while there is no libpmem
> +support, qemu will exit

Re: [Qemu-devel] tcg_register_jit_int() interface problem

[Aleksandar Markovic]

> From: Peter Maydell 
> Sent: Monday, July 16, 2018 1:55 PM
>
> On 16 July 2018 at 12:36, Aleksandar Markovic  wrote:
> >> From: Laurent Vivier 
> >> Le 16/07/2018 à 13:10, Aleksandar Markovic a écrit :
> >> > For MIPS, the situation is different: Up until recently, MIPS had only
> >> > one value for e_machine: that was EM_MIPS (used for both 32-bit and
> >> > 64-bit MIPS systems). However, new nanoMIPS systems have a new value in
> >> > that field: EM_NANOMIPS (that is to be applied for both 32-bit and
> >> > 64-bit nanoMIPS systems).
> >> >
> >> >
> >> > The situation described above represent a problem for us developing
> >> > nanoMIPS support, forcing us to have different compilations (and
> >> > executables) for MIPS and nanoMIPS.
> >> >
> >> >
> >> > Is there a possibility to refactor tcg_register_jit_int() to receive
> >> > "e_machine" value as an argument, instead of expecting preprocessor
> >> > constant to be set?
> >>
> >> I don't know this par of code, but for PPC we have:
> >>
> >> #if TCG_TARGET_REG_BITS == 64
> >> # define ELF_HOST_MACHINE EM_PPC64
> >> #else
> >> # define ELF_HOST_MACHINE EM_PPC
> >> #endif
> >>
> >> So perhaps you could have something like:
> >>
> >> #if defined(HOST_NANOMIPS)
> >> # define ELF_HOST_MACHINE EM_NANOMIPS
> >> #else
> >> # define ELF_HOST_MACHINE EM_MIPS
> >> #endif
> >
> > The problem is, this means there will be separate executables for QEMU for
> > MIPS and QEMU for nanoMIPS. Would that be acceptable from overall QEMU
> > design point of view? Peter, Richard?
>
> The assumption has previously been that there must be a value known
> at compile time (because it would be the value of whatever is in the
> ELF header for the QEMU executable itself, effectively). I'm not
> sure why this doesn't work for MIPS still -- if you're building
> on a host with the new nanoMIPS stuff and with that support enabled,
> then report EM_NANOMIPS; if you're building a binary that works with
> the older MIPS CPUs, report EM_MIPS ?
>
> That said, if this doesn't work for MIPS, then it would be a trivial
> refactor to allow the tcg per-target code to do
> #define ELF_HOST_MACHINE mips_get_elf_host_machine()
> so you could compute the value at runtime.
>

Thanks for hints and suggestions. I will have to revisit the code and find the 
most suitable solution.

Ragards,
Aleksandar

[Qemu-devel] [PATCH v3 03/17] hw/arm/bcm2836: Fix crash with device_add bcm2837 on unsupported machines

[Thomas Huth]

When trying to "device_add bcm2837" on a machine that is not suitable for
this device, you can quickly crash QEMU afterwards, e.g. with "info qtree":

echo "{'execute':'qmp_capabilities'} {'execute':'device_add', " \
 "'arguments':{'driver':'bcm2837'}} {'execute': 'human-monitor-command', " \
 "'arguments': {'command-line': 'info qtree'}}" | \
 aarch64-softmmu/qemu-system-aarch64 -M integratorcp,accel=qtest -S -qmp stdio

{"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2},
 "package": "build-all"}, "capabilities": []}}
{"return": {}}
{"error": {"class": "GenericError", "desc": "Device 'bcm2837' can not be
 hotplugged on this machine"}}
Segmentation fault (core dumped)

The qdev_set_parent_bus() from instance_init adds a link to the child devices
which is not valid anymore after the bcm2837 instance has been destroyed.
Unfortunately, the child devices do not get destroyed / unlinked correctly
because both object_initialize() and object_property_add_child() increase
the reference count of the child objects by one, but only one reference
is dropped when the parent gets removed. So let's use the new functions
object_initialize_child() and sysbus_init_child_obj() instead to create
the objects, which will take care of creating the child objects with the
correct reference count of one.

Reviewed-by: Richard Henderson 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 hw/arm/bcm2836.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 6805a7d..2595d93 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -51,25 +51,19 @@ static void bcm2836_init(Object *obj)
 int n;
 
 for (n = 0; n < BCM283X_NCPUS; n++) {
-object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
-  info->cpu_type);
-object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
-  &error_abort);
+object_initialize_child(obj, "cpu[*]", &s->cpus[n], sizeof(s->cpus[n]),
+info->cpu_type, &error_abort, NULL);
 }
 
-object_initialize(&s->control, sizeof(s->control), TYPE_BCM2836_CONTROL);
-object_property_add_child(obj, "control", OBJECT(&s->control), NULL);
-qdev_set_parent_bus(DEVICE(&s->control), sysbus_get_default());
+sysbus_init_child_obj(obj, "control", &s->control, sizeof(s->control),
+  TYPE_BCM2836_CONTROL);
 
-object_initialize(&s->peripherals, sizeof(s->peripherals),
-  TYPE_BCM2835_PERIPHERALS);
-object_property_add_child(obj, "peripherals", OBJECT(&s->peripherals),
-  &error_abort);
+sysbus_init_child_obj(obj, "peripherals", &s->peripherals,
+  sizeof(s->peripherals), TYPE_BCM2835_PERIPHERALS);
 object_property_add_alias(obj, "board-rev", OBJECT(&s->peripherals),
   "board-rev", &error_abort);
 object_property_add_alias(obj, "vcram-size", OBJECT(&s->peripherals),
   "vcram-size", &error_abort);
-qdev_set_parent_bus(DEVICE(&s->peripherals), sysbus_get_default());
 }
 
 static void bcm2836_realize(DeviceState *dev, Error **errp)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 01/17] qom/object: Add a new function object_initialize_child()

[Thomas Huth]

A lot of code is using the object_initialize() function followed by a call
to object_property_add_child() to add the newly initialized object as a child
of the current object. Both functions increase the reference counter of the
new object, but many spots that call these two functions then forget to drop
one of the superfluous references. So the newly created object is often not
cleaned up correctly when the parent is destroyed. In the worst case, this
can cause crashes, e.g. because device objects are not correctly removed from
their parent_bus.

Since this is a common pattern between many code spots, let's introduce a
new function that takes care of calling all three required initialization
functions, first object_initialize(), then object_property_add_child() and
finally object_unref(). And since the function does a similar job like
object_new_with_props(), also allow to set additional properties via
varargs, and use user_creatable_complete() to make sure that the functions
can be used similarly.

And while we're at object.h, also fix some copy-n-paste errors in the
comments there ("to store the area" --> "to store the error").

Signed-off-by: Thomas Huth 
---
 include/qom/object.h | 45 +--
 qom/object.c | 54 
 2 files changed, 97 insertions(+), 2 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index f3d2308..f0b0bf3 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -749,6 +749,47 @@ int object_set_propv(Object *obj,
 void object_initialize(void *obj, size_t size, const char *typename);
 
 /**
+ * object_initialize_child:
+ * @parentobj: The parent object to add a property to
+ * @propname: The name of the property
+ * @childobj: A pointer to the memory to be used for the object.
+ * @size: The maximum size available at @childobj for the object.
+ * @type: The name of the type of the object to instantiate.
+ * @errp: If an error occurs, a pointer to an area to store the error
+ * @...: list of property names and values
+ *
+ * This function will initialize an object. The memory for the object should
+ * have already been allocated. The object will then be added as child property
+ * to a parent with object_property_add_child() function. The returned object
+ * has a reference count of 1 (for the "child<...>" property from the parent),
+ * so the object will be finalized automatically when the parent gets removed.
+ *
+ * The variadic parameters are a list of pairs of (propname, propvalue)
+ * strings. The propname of %NULL indicates the end of the property list.
+ * If the object implements the user creatable interface, the object will
+ * be marked complete once all the properties have been processed.
+ */
+void object_initialize_child(Object *parentobj, const char *propname,
+ void *childobj, size_t size, const char *type,
+ Error **errp, ...) QEMU_SENTINEL;
+
+/**
+ * object_initialize_childv:
+ * @parentobj: The parent object to add a property to
+ * @propname: The name of the property
+ * @childobj: A pointer to the memory to be used for the object.
+ * @size: The maximum size available at @childobj for the object.
+ * @type: The name of the type of the object to instantiate.
+ * @errp: If an error occurs, a pointer to an area to store the error
+ * @vargs: list of property names and values
+ *
+ * See object_initialize_child() for documentation.
+ */
+void object_initialize_childv(Object *parentobj, const char *propname,
+  void *childobj, size_t size, const char *type,
+  Error **errp, va_list vargs);
+
+/**
  * object_dynamic_cast:
  * @obj: The object to cast.
  * @typename: The @typename to cast to.
@@ -1382,7 +1423,7 @@ Object *object_resolve_path_component(Object *parent, 
const gchar *part);
  * @obj: the object to add a property to
  * @name: the name of the property
  * @child: the child object
- * @errp: if an error occurs, a pointer to an area to store the area
+ * @errp: if an error occurs, a pointer to an area to store the error
  *
  * Child properties form the composition tree.  All objects need to be a child
  * of another object.  Objects can only be a child of one object.
@@ -1420,7 +1461,7 @@ void object_property_allow_set_link(const Object *, const 
char *,
  * @child: a pointer to where the link object reference is stored
  * @check: callback to veto setting or NULL if the property is read-only
  * @flags: additional options for the link
- * @errp: if an error occurs, a pointer to an area to store the area
+ * @errp: if an error occurs, a pointer to an area to store the error
  *
  * Links establish relationships between objects.  Links are unidirectional
  * although two links can be combined to form a bidirectional relationship
diff --git a/qom/object.c b/qom/object.c
index 4609e34..75d1d48 100644
--- a/qom/object.c
+++ b/qom/obje

[Qemu-devel] [PATCH v3 00/17] Fix crashes with introspection of ARM devices

[Thomas Huth]

As discovered recently, you can crash QEMU with a lot of devices
that do not get the reference counting of child objects right.
You just have to run 'device-list-properties' and call 'info qtree'
afterwards.
This patch series fixes these problems in the ARM code. When all
patches have been applied, I now do not get any more hangs or crashes
when I add a hmp("info qtree") to the device-introspect-test.

Please have a look at patch #1, #15 and #16, they still need reviews.

v3:
 - Reworked object_initialize_child according to Paolos suggestions
   (patch 1)
 - Added prototype description in the 2nd patch (as suggested by Eduardo)
 - Replaced the xlnx_dp "realize" patch with the one from Paolo
 - Added a patch for the "stm32f205_soc" device (surprisingly this was
   already the last one that caused trouble - I originally expected more)

v2:
 - Updated the first patch according to the review feedback from v1
 - Added more patches with additional fixes

Paolo Bonzini (1):
  hw/display/xlnx_dp: Move problematic code from instance_init to
realize

Thomas Huth (16):
  qom/object: Add a new function object_initialize_child()
  hw/core/sysbus: Add a function for creating and attaching an object
  hw/arm/bcm2836: Fix crash with device_add bcm2837 on unsupported
machines
  hw/arm/armv7: Fix crash when introspecting the "iotkit" device
  hw/cpu/a15mpcore: Fix introspection problem with the a15mpcore_priv
device
  hw/arm/msf2-soc: Fix introspection problem with the "msf2-soc" device
  hw/cpu/a9mpcore: Fix introspection problems with the "a9mpcore_priv"
device
  hw/arm/fsl-imx6: Fix introspection problems with the "fsl,imx6" device
  hw/arm/fsl-imx7: Fix introspection problems with the "fsl,imx7" device
  hw/arm/fsl-imx25: Fix introspection problem with the "fsl,imx25"
device
  hw/arm/fsl-imx31: Fix introspection problem with the "fsl,imx31"
device
  hw/cpu/arm11mpcore: Fix introspection problem with 'arm11mpcore_priv'
  hw/*/realview: Fix introspection problem with 'realview_mpcore' &
'realview_gic'
  hw/arm/allwinner-a10: Fix introspection problem with 'allwinner-a10'
  hw/arm/stm32f205_soc: Fix introspection problem with 'stm32f205-soc'
device
  hw/arm/xlnx-zynqmp: Fix crash when introspecting the "xlnx,zynqmp"
device

 hw/arm/allwinner-a10.c   | 19 +-
 hw/arm/armv7m.c  |  7 ++--
 hw/arm/bcm2836.c | 18 +++--
 hw/arm/fsl-imx25.c   | 30 +++
 hw/arm/fsl-imx31.c   | 26 ++---
 hw/arm/fsl-imx6.c| 56 ++--
 hw/arm/fsl-imx7.c| 97 
 hw/arm/iotkit.c  | 74 
 hw/arm/msf2-soc.c| 15 
 hw/arm/stm32f205_soc.c   | 28 ++
 hw/arm/xlnx-zynqmp.c | 61 ++
 hw/core/sysbus.c |  8 
 hw/cpu/a15mpcore.c   |  8 ++--
 hw/cpu/a9mpcore.c| 18 -
 hw/cpu/arm11mpcore.c | 14 +++
 hw/cpu/realview_mpcore.c |  8 ++--
 hw/display/xlnx_dp.c |  8 +++-
 hw/intc/armv7m_nvic.c|  5 +--
 hw/intc/realview_gic.c   |  7 +---
 hw/misc/auxbus.c | 18 ++---
 include/hw/misc/auxbus.h | 14 ++-
 include/hw/sysbus.h  | 17 +
 include/qom/object.h | 45 +-
 qom/object.c | 54 +++
 24 files changed, 355 insertions(+), 300 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 02/17] hw/core/sysbus: Add a function for creating and attaching an object

[Thomas Huth]

A lot of functions are initializing an object and attach it immediately
afterwards to the system bus. Provide a common function for this, which
also uses object_initialize_child() to make sure that the reference
counter is correctly initialized to 1 afterwards.

Reviewed-by: Richard Henderson 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 hw/core/sysbus.c|  8 
 include/hw/sysbus.h | 17 +
 2 files changed, 25 insertions(+)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index ecfb0cf..3c8e53b 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -376,6 +376,14 @@ BusState *sysbus_get_default(void)
 return main_system_bus;
 }
 
+void sysbus_init_child_obj(Object *parent, const char *childname, void *child,
+   size_t childsize, const char *childtype)
+{
+object_initialize_child(parent, childname, child, childsize, childtype,
+&error_abort, NULL);
+qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
+}
+
 static void sysbus_register_types(void)
 {
 type_register_static(&system_bus_info);
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index e88bb6d..0b59a3b 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -96,6 +96,23 @@ void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
MemoryRegion *mem);
 MemoryRegion *sysbus_address_space(SysBusDevice *dev);
 
+/**
+ * sysbus_init_child_obj:
+ * @parent: The parent object
+ * @childname: Used as name of the "child<>" property in the parent
+ * @child: A pointer to the memory to be used for the object.
+ * @childsize: The maximum size available at @child for the object.
+ * @childtype: The name of the type of the object to instantiate.
+ *
+ * This function will initialize an object and attach it to the main system
+ * bus. The memory for the object should have already been allocated. The
+ * object will then be added as child to the given parent. The returned object
+ * has a reference count of 1 (for the "child<...>" property from the parent),
+ * so the object will be finalized automatically when the parent gets removed.
+ */
+void sysbus_init_child_obj(Object *parent, const char *childname, void *child,
+   size_t childsize, const char *childtype);
+
 /* Call func for every dynamically created sysbus device in the system */
 void foreach_dynamic_sysbus_device(FindSysbusDeviceFunc *func, void *opaque);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 05/17] hw/cpu/a15mpcore: Fix introspection problem with the a15mpcore_priv device

[Thomas Huth]

There is a memory management problem when introspecting the a15mpcore_priv
device. It can be seen with valgrind when running QEMU like this:

echo "{'execute':'qmp_capabilities'} {'execute':'device-list-properties'," \
 "'arguments':{'typename':'a15mpcore_priv'}}"\
 "{'execute': 'human-monitor-command', " \
 "'arguments': {'command-line': 'info qtree'}}"  | \
 valgrind -q aarch64-softmmu/qemu-system-aarch64 -M none,accel=qtest -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2},
 "package": "build-all"}, "capabilities": []}}
{"return": {}}
{"return": [{"name": "num-cpu", "type": "uint32"}, {"name": "num-irq",
 "type": "uint32"}, {"name": "a15mp-priv-container[0]", "type":
  "child"}]}
==24978== Invalid read of size 8
==24978==at 0x618EBA: qdev_print (qdev-monitor.c:686)
==24978==by 0x618EBA: qbus_print (qdev-monitor.c:719)
[...]

Use the new sysbus_init_child_obj() function to make sure that we get
the reference counting of the child objects right.

Reviewed-by: Richard Henderson 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 hw/cpu/a15mpcore.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
index bc05152..43c1079 100644
--- a/hw/cpu/a15mpcore.c
+++ b/hw/cpu/a15mpcore.c
@@ -35,15 +35,13 @@ static void a15mp_priv_initfn(Object *obj)
 {
 SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 A15MPPrivState *s = A15MPCORE_PRIV(obj);
-DeviceState *gicdev;
 
 memory_region_init(&s->container, obj, "a15mp-priv-container", 0x8000);
 sysbus_init_mmio(sbd, &s->container);
 
-object_initialize(&s->gic, sizeof(s->gic), gic_class_name());
-gicdev = DEVICE(&s->gic);
-qdev_set_parent_bus(gicdev, sysbus_get_default());
-qdev_prop_set_uint32(gicdev, "revision", 2);
+sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic),
+  gic_class_name());
+qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
 }
 
 static void a15mp_priv_realize(DeviceState *dev, Error **errp)
-- 
1.8.3.1

[Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Marc-André Lureau]

From: Stefan Berger 

Implement a virtual memory device for the TPM Physical Presence interface.
The memory is located at 0xFED45000 and used by ACPI to send messages to the
firmware (BIOS) and by the firmware to provide parameters for each one of
the supported codes.

This device should be used by all TPM interfaces on x86 and can be added
by calling tpm_ppi_init_io().

Note: bios_linker cannot be used to allocate the PPI memory region,
since the reserved memory should stay stable across reboots, and might
be needed before the ACPI tables are installed.

Signed-off-by: Stefan Berger 
Signed-off-by: Marc-André Lureau 
---
 hw/tpm/tpm_ppi.h  | 26 ++
 include/hw/acpi/tpm.h |  6 ++
 hw/tpm/tpm_crb.c  |  8 
 hw/tpm/tpm_ppi.c  | 31 +++
 hw/tpm/tpm_tis.c  |  8 
 hw/tpm/Makefile.objs  |  1 +
 6 files changed, 80 insertions(+)
 create mode 100644 hw/tpm/tpm_ppi.h
 create mode 100644 hw/tpm/tpm_ppi.c

diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
new file mode 100644
index 00..f6458bf87e
--- /dev/null
+++ b/hw/tpm/tpm_ppi.h
@@ -0,0 +1,26 @@
+/*
+ * TPM Physical Presence Interface
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef TPM_TPM_PPI_H
+#define TPM_TPM_PPI_H
+
+#include "hw/acpi/tpm.h"
+#include "exec/address-spaces.h"
+
+typedef struct TPMPPI {
+MemoryRegion ram;
+uint8_t buf[TPM_PPI_ADDR_SIZE];
+} TPMPPI;
+
+bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
+  hwaddr addr, Object *obj, Error **errp);
+
+#endif /* TPM_TPM_PPI_H */
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 3580ffd50c..b8796df916 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -188,4 +188,10 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM2_START_METHOD_MMIO  6
 #define TPM2_START_METHOD_CRB   7
 
+/*
+ * Physical Presence Interface
+ */
+#define TPM_PPI_ADDR_SIZE   0x400
+#define TPM_PPI_ADDR_BASE   0xFED45000
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index d5b0ac5920..b243222fd6 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -29,6 +29,7 @@
 #include "sysemu/reset.h"
 #include "tpm_int.h"
 #include "tpm_util.h"
+#include "tpm_ppi.h"
 #include "trace.h"
 
 typedef struct CRBState {
@@ -43,6 +44,7 @@ typedef struct CRBState {
 size_t be_buffer_size;
 
 bool ppi_enabled;
+TPMPPI ppi;
 } CRBState;
 
 #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
@@ -294,6 +296,12 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
 memory_region_add_subregion(get_system_memory(),
 TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
 
+if (s->ppi_enabled &&
+!tpm_ppi_init(&s->ppi, get_system_memory(),
+  TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
+return;
+}
+
 qemu_register_reset(tpm_crb_reset, dev);
 }
 
diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
new file mode 100644
index 00..8b46b9dd4b
--- /dev/null
+++ b/hw/tpm/tpm_ppi.c
@@ -0,0 +1,31 @@
+/*
+ * tpm_ppi.c - TPM Physical Presence Interface
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger 
+ *
+ * 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 "qapi/error.h"
+#include "cpu.h"
+#include "sysemu/memory_mapping.h"
+#include "migration/vmstate.h"
+#include "tpm_ppi.h"
+
+bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
+  hwaddr addr, Object *obj, Error **errp)
+{
+memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
+  TPM_PPI_ADDR_SIZE, tpmppi->buf);
+vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
+
+memory_region_add_subregion(m, addr, &tpmppi->ram);
+return true;
+}
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index d9ddf9b723..70432ffe8b 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -31,6 +31,7 @@
 #include "sysemu/tpm_backend.h"
 #include "tpm_int.h"
 #include "tpm_util.h"
+#include "tpm_ppi.h"
 #include "trace.h"
 
 #define TPM_TIS_NUM_LOCALITIES  5 /* per spec */
@@ -83,6 +84,7 @@ typedef struct TPMState {
 size_t be_buffer_size;
 
 bool ppi_enabled;
+TPMPPI ppi;
 } TPMState;
 
 #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
@@ -979,6 +981,12 @@ static void tpm_tis_realizefn(DeviceState *dev, Error 
**errp)
 
 memory_region_add_subregion(isa_address_space(ISA_DEVICE(dev)),
 TPM_TIS_ADDR_BASE, &s->mmio);
+
+if (s->ppi_enabled &&
+!tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
+  TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
+

[Qemu-devel] [PATCH v3 07/17] hw/cpu/a9mpcore: Fix introspection problems with the "a9mpcore_priv" device

[Thomas Huth]

Running QEMU with valgrind indicates a problem here:

echo "{'execute':'qmp_capabilities'} {'execute':'device-list-properties'," \
 "'arguments':{'typename':'a9mpcore_priv'}}" \
 "{'execute': 'human-monitor-command', " \
 "'arguments': {'command-line': 'info qtree'}}" | \
 valgrind -q aarch64-softmmu/qemu-system-aarch64 -M none,accel=qtest -qmp stdio
[...]
==30996== Invalid read of size 8
==30996==at 0x6185DA: qdev_print (qdev-monitor.c:686)
==30996==by 0x6185DA: qbus_print (qdev-monitor.c:719)
==30996==by 0x452B38: handle_hmp_command (monitor.c:3446)
[...]

Use the new sysbus_init_child_obj() function to make sure that the objects
are cleaned up correctly when the parent gets destroyed.

Reviewed-by: Richard Henderson 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 hw/cpu/a9mpcore.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index f17f292..a5b8678 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -27,20 +27,18 @@ static void a9mp_priv_initfn(Object *obj)
 memory_region_init(&s->container, obj, "a9mp-priv-container", 0x2000);
 sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->container);
 
-object_initialize(&s->scu, sizeof(s->scu), TYPE_A9_SCU);
-qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
+sysbus_init_child_obj(obj, "scu", &s->scu, sizeof(s->scu), TYPE_A9_SCU);
 
-object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
-qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
+sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic), TYPE_ARM_GIC);
 
-object_initialize(&s->gtimer, sizeof(s->gtimer), TYPE_A9_GTIMER);
-qdev_set_parent_bus(DEVICE(&s->gtimer), sysbus_get_default());
+sysbus_init_child_obj(obj, "gtimer", &s->gtimer, sizeof(s->gtimer),
+  TYPE_A9_GTIMER);
 
-object_initialize(&s->mptimer, sizeof(s->mptimer), TYPE_ARM_MPTIMER);
-qdev_set_parent_bus(DEVICE(&s->mptimer), sysbus_get_default());
+sysbus_init_child_obj(obj, "mptimer", &s->mptimer, sizeof(s->mptimer),
+  TYPE_ARM_MPTIMER);
 
-object_initialize(&s->wdt, sizeof(s->wdt), TYPE_ARM_MPTIMER);
-qdev_set_parent_bus(DEVICE(&s->wdt), sysbus_get_default());
+sysbus_init_child_obj(obj, "wdt", &s->wdt, sizeof(s->wdt),
+  TYPE_ARM_MPTIMER);
 }
 
 static void a9mp_priv_realize(DeviceState *dev, Error **errp)
-- 
1.8.3.1

[Qemu-devel] [PATCH v8 0/5] Add support for TPM Physical Presence interface

[Marc-André Lureau]

Hi,

The following patches implement the TPM Physical Presence Interface
that allows a user to set a command via ACPI (sysfs entry in Linux)
that, upon the next reboot, the firmware looks for and acts upon by
sending sequences of commands to the TPM.

A dedicated memory region is added to the TPM CRB & TIS devices, at
address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
holds the location for that PPI region and some version details, to
allow for future flexibility.

With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
now runs successfully.

It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
Implement Physical Presence interface for TPM 1.2 and 2")

The edk2 support is merged upstream.

v8: ACPI code improvements
- replace various aml_int() with variables
- make op/op_flags common variables
- replace the switch field indexing hack by a dynamic operation region

v7:
- relace RAM with RAM device ptr
- a few ACPI code & comments improvements
- add back proof-of-concept ACPI memory clear interface, handled in
  qemu (pass again WHLK TCG tests)

Marc-André Lureau (2):
  tpm: add a "ppi" boolean property
  PoC: tpm: add ACPI memory clear interface

Stefan Berger (3):
  tpm: implement virtual memory device for TPM PPI
  acpi: add fw_cfg file for TPM and PPI virtual memory device
  acpi: build TPM Physical Presence interface

 hw/tpm/tpm_ppi.h  |  26 +++
 include/hw/acpi/tpm.h |  17 ++
 include/hw/compat.h   |  10 +
 hw/i386/acpi-build.c  | 456 +-
 hw/tpm/tpm_crb.c  |  11 +
 hw/tpm/tpm_ppi.c  |  56 ++
 hw/tpm/tpm_tis.c  |  11 +
 docs/specs/tpm.txt| 101 ++
 hw/tpm/Makefile.objs  |   1 +
 hw/tpm/trace-events   |   3 +
 10 files changed, 690 insertions(+), 2 deletions(-)
 create mode 100644 hw/tpm/tpm_ppi.h
 create mode 100644 hw/tpm/tpm_ppi.c

-- 
2.18.0.129.ge3331758f1

[Qemu-devel] [PATCH v3 06/17] hw/arm/msf2-soc: Fix introspection problem with the "msf2-soc" device

[Thomas Huth]

Valgrind currently reports a problem when running QEMU like this:

echo "{'execute':'qmp_capabilities'} {'execute':'device-list-properties'," \
 "'arguments':{'typename':'msf2-soc'}}" \
 "{'execute': 'human-monitor-command', " \
 "'arguments': {'command-line': 'info qtree'}}" | \
 valgrind -q aarch64-softmmu/qemu-system-aarch64 -M none,accel=qtest -qmp stdio
[...]
==23097== Invalid read of size 8
==23097==at 0x6192AA: qdev_print (qdev-monitor.c:686)
==23097==by 0x6192AA: qbus_print (qdev-monitor.c:719)
[...]

Use the new sysbus_init_child_obj() function to make sure that the child
objects are cleaned up correctly when the parent gets destroyed.

Reviewed-by: Richard Henderson 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 hw/arm/msf2-soc.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
index edb3ba8..dbefade 100644
--- a/hw/arm/msf2-soc.c
+++ b/hw/arm/msf2-soc.c
@@ -68,19 +68,18 @@ static void m2sxxx_soc_initfn(Object *obj)
 MSF2State *s = MSF2_SOC(obj);
 int i;
 
-object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
-qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
+sysbus_init_child_obj(obj, "armv7m", &s->armv7m, sizeof(s->armv7m),
+  TYPE_ARMV7M);
 
-object_initialize(&s->sysreg, sizeof(s->sysreg), TYPE_MSF2_SYSREG);
-qdev_set_parent_bus(DEVICE(&s->sysreg), sysbus_get_default());
+sysbus_init_child_obj(obj, "sysreg", &s->sysreg, sizeof(s->sysreg),
+  TYPE_MSF2_SYSREG);
 
-object_initialize(&s->timer, sizeof(s->timer), TYPE_MSS_TIMER);
-qdev_set_parent_bus(DEVICE(&s->timer), sysbus_get_default());
+sysbus_init_child_obj(obj, "timer", &s->timer, sizeof(s->timer),
+  TYPE_MSS_TIMER);
 
 for (i = 0; i < MSF2_NUM_SPIS; i++) {
-object_initialize(&s->spi[i], sizeof(s->spi[i]),
+sysbus_init_child_obj(obj, "spi[*]", &s->spi[i], sizeof(s->spi[i]),
   TYPE_MSS_SPI);
-qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
 }
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 04/17] hw/arm/armv7: Fix crash when introspecting the "iotkit" device

[Thomas Huth]

QEMU currently crashes when introspecting the "iotkit" device and
runnint "info qtree" afterwards, e.g. when running QEMU like this:

echo "{'execute':'qmp_capabilities'} {'execute':'device-list-properties'," \
 "'arguments':{'typename':'iotkit'}}" "{'execute': 'human-monitor-command', " \
 "'arguments': {'command-line': 'info qtree'}}" | \
 aarch64-softmmu/qemu-system-aarch64 -M none,accel=qtest -qmp stdio

Use the new functions object_initialize_child() and sysbus_init_child_obj()
to make sure that all objects get cleaned up correctly when the instances
are destroyed.

Reviewed-by: Richard Henderson 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 hw/arm/armv7m.c   |  7 +++--
 hw/arm/iotkit.c   | 74 ++-
 hw/intc/armv7m_nvic.c |  5 ++--
 3 files changed, 37 insertions(+), 49 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 9e00d40..6b07666 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -134,14 +134,13 @@ static void armv7m_instance_init(Object *obj)
 
 memory_region_init(&s->container, obj, "armv7m-container", UINT64_MAX);
 
-object_initialize(&s->nvic, sizeof(s->nvic), TYPE_NVIC);
-qdev_set_parent_bus(DEVICE(&s->nvic), sysbus_get_default());
+sysbus_init_child_obj(obj, "nvnic", &s->nvic, sizeof(s->nvic), TYPE_NVIC);
 object_property_add_alias(obj, "num-irq",
   OBJECT(&s->nvic), "num-irq", &error_abort);
 
 for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
-object_initialize(&s->bitband[i], sizeof(s->bitband[i]), TYPE_BITBAND);
-qdev_set_parent_bus(DEVICE(&s->bitband[i]), sysbus_get_default());
+sysbus_init_child_obj(obj, "bitband[*]", &s->bitband[i],
+  sizeof(s->bitband[i]), TYPE_BITBAND);
 }
 }
 
diff --git a/hw/arm/iotkit.c b/hw/arm/iotkit.c
index 133d5bb..c76d3ed 100644
--- a/hw/arm/iotkit.c
+++ b/hw/arm/iotkit.c
@@ -30,15 +30,6 @@ static void make_alias(IoTKit *s, MemoryRegion *mr, const 
char *name,
 memory_region_add_subregion_overlap(&s->container, base, mr, -1500);
 }
 
-static void init_sysbus_child(Object *parent, const char *childname,
-  void *child, size_t childsize,
-  const char *childtype)
-{
-object_initialize(child, childsize, childtype);
-object_property_add_child(parent, childname, OBJECT(child), &error_abort);
-qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
-}
-
 static void irq_status_forwarder(void *opaque, int n, int level)
 {
 qemu_irq destirq = opaque;
@@ -119,53 +110,52 @@ static void iotkit_init(Object *obj)
 
 memory_region_init(&s->container, obj, "iotkit-container", UINT64_MAX);
 
-init_sysbus_child(obj, "armv7m", &s->armv7m, sizeof(s->armv7m),
-  TYPE_ARMV7M);
+sysbus_init_child_obj(obj, "armv7m", &s->armv7m, sizeof(s->armv7m),
+  TYPE_ARMV7M);
 qdev_prop_set_string(DEVICE(&s->armv7m), "cpu-type",
  ARM_CPU_TYPE_NAME("cortex-m33"));
 
-init_sysbus_child(obj, "secctl", &s->secctl, sizeof(s->secctl),
-  TYPE_IOTKIT_SECCTL);
-init_sysbus_child(obj, "apb-ppc0", &s->apb_ppc0, sizeof(s->apb_ppc0),
-  TYPE_TZ_PPC);
-init_sysbus_child(obj, "apb-ppc1", &s->apb_ppc1, sizeof(s->apb_ppc1),
-  TYPE_TZ_PPC);
-init_sysbus_child(obj, "mpc", &s->mpc, sizeof(s->mpc), TYPE_TZ_MPC);
-object_initialize(&s->mpc_irq_orgate, sizeof(s->mpc_irq_orgate),
-  TYPE_OR_IRQ);
-object_property_add_child(obj, "mpc-irq-orgate",
-  OBJECT(&s->mpc_irq_orgate), &error_abort);
+sysbus_init_child_obj(obj, "secctl", &s->secctl, sizeof(s->secctl),
+  TYPE_IOTKIT_SECCTL);
+sysbus_init_child_obj(obj, "apb-ppc0", &s->apb_ppc0, sizeof(s->apb_ppc0),
+  TYPE_TZ_PPC);
+sysbus_init_child_obj(obj, "apb-ppc1", &s->apb_ppc1, sizeof(s->apb_ppc1),
+  TYPE_TZ_PPC);
+sysbus_init_child_obj(obj, "mpc", &s->mpc, sizeof(s->mpc), TYPE_TZ_MPC);
+object_initialize_child(obj, "mpc-irq-orgate", &s->mpc_irq_orgate,
+sizeof(s->mpc_irq_orgate), TYPE_OR_IRQ,
+&error_abort, NULL);
+
 for (i = 0; i < ARRAY_SIZE(s->mpc_irq_splitter); i++) {
 char *name = g_strdup_printf("mpc-irq-splitter-%d", i);
 SplitIRQ *splitter = &s->mpc_irq_splitter[i];
 
-object_initialize(splitter, sizeof(*splitter), TYPE_SPLIT_IRQ);
-object_property_add_child(obj, name, OBJECT(splitter), &error_abort);
+object_initialize_child(obj, name, splitter, sizeof(*splitter),
+TYPE_SPLIT_IRQ, &error_abort, NULL);
 g_free(name);
 }
-init_sysbus_child(obj, "timer0", &s->timer0, sizeof(s->timer0),
-  TY

[Qemu-devel] [PATCH v8 1/5] tpm: add a "ppi" boolean property

[Marc-André Lureau]

The following patches implement the TPM Physical Presence Interface,
make use of a new memory region and a fw_cfg entry. Enable PPI by
default with >2.12 machine type, to avoid migration issues.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Igor Mammedov 
---
 include/hw/compat.h | 10 ++
 hw/tpm/tpm_crb.c|  3 +++
 hw/tpm/tpm_tis.c|  3 +++
 3 files changed, 16 insertions(+)

diff --git a/include/hw/compat.h b/include/hw/compat.h
index c08f4040bb..92fdac360b 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,6 +2,16 @@
 #define HW_COMPAT_H
 
 #define HW_COMPAT_2_12 \
+{\
+.driver   = "tpm-crb",\
+.property = "ppi",\
+.value= "false",\
+},\
+{\
+.driver   = "tpm-tis",\
+.property = "ppi",\
+.value= "false",\
+},\
 {\
 .driver   = "migration",\
 .property = "decompress-error-check",\
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index a92dd50437..d5b0ac5920 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -41,6 +41,8 @@ typedef struct CRBState {
 MemoryRegion cmdmem;
 
 size_t be_buffer_size;
+
+bool ppi_enabled;
 } CRBState;
 
 #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
@@ -221,6 +223,7 @@ static const VMStateDescription vmstate_tpm_crb = {
 
 static Property tpm_crb_properties[] = {
 DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe),
+DEFINE_PROP_BOOL("ppi", CRBState, ppi_enabled, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 12f5c9a759..d9ddf9b723 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -81,6 +81,8 @@ typedef struct TPMState {
 TPMVersion be_tpm_version;
 
 size_t be_buffer_size;
+
+bool ppi_enabled;
 } TPMState;
 
 #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
@@ -950,6 +952,7 @@ static const VMStateDescription vmstate_tpm_tis = {
 static Property tpm_tis_properties[] = {
 DEFINE_PROP_UINT32("irq", TPMState, irq_num, TPM_TIS_IRQ),
 DEFINE_PROP_TPMBE("tpmdev", TPMState, be_driver),
+DEFINE_PROP_BOOL("ppi", TPMState, ppi_enabled, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.18.0.129.ge3331758f1

[Qemu-devel] [PATCH v3 13/17] hw/*/realview: Fix introspection problem with 'realview_mpcore' & 'realview_gic'

[Thomas Huth]

echo "{'execute':'qmp_capabilities'} {'execute':'device-list-properties'," \
 "'arguments':{'typename':'realview_mpcore'}}" \
 "{'execute': 'human-monitor-command', " \
 "'arguments': {'command-line': 'info qtree'}}" | \
 valgrind -q aarch64-softmmu/qemu-system-aarch64 -M none,accel=qtest -qmp stdio
[...]
==2654== Invalid read of size 8
==2654==at 0x61878A: qdev_print (qdev-monitor.c:686)
==2654==by 0x61878A: qbus_print (qdev-monitor.c:719)
==2654==by 0x452B38: handle_hmp_command (monitor.c:3446)
==2654==by 0x452D70: qmp_human_monitor_command (monitor.c:821)
[...]

Use sysbus_init_child_obj() to fix it.

Reviewed-by: Richard Henderson 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 hw/cpu/realview_mpcore.c | 8 
 hw/intc/realview_gic.c   | 7 ++-
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/hw/cpu/realview_mpcore.c b/hw/cpu/realview_mpcore.c
index 39d4ebe..9d3f837 100644
--- a/hw/cpu/realview_mpcore.c
+++ b/hw/cpu/realview_mpcore.c
@@ -101,14 +101,14 @@ static void mpcore_rirq_init(Object *obj)
 SysBusDevice *privbusdev;
 int i;
 
-object_initialize(&s->priv, sizeof(s->priv), TYPE_ARM11MPCORE_PRIV);
-qdev_set_parent_bus(DEVICE(&s->priv), sysbus_get_default());
+sysbus_init_child_obj(obj, "a11priv", &s->priv, sizeof(s->priv),
+  TYPE_ARM11MPCORE_PRIV);
 privbusdev = SYS_BUS_DEVICE(&s->priv);
 sysbus_init_mmio(sbd, sysbus_mmio_get_region(privbusdev, 0));
 
 for (i = 0; i < 4; i++) {
-object_initialize(&s->gic[i], sizeof(s->gic[i]), TYPE_REALVIEW_GIC);
-qdev_set_parent_bus(DEVICE(&s->gic[i]), sysbus_get_default());
+sysbus_init_child_obj(obj, "gic[*]", &s->gic[i], sizeof(s->gic[i]),
+  TYPE_REALVIEW_GIC);
 }
 }
 
diff --git a/hw/intc/realview_gic.c b/hw/intc/realview_gic.c
index 50bbab6..7f2ff85 100644
--- a/hw/intc/realview_gic.c
+++ b/hw/intc/realview_gic.c
@@ -54,16 +54,13 @@ static void realview_gic_init(Object *obj)
 {
 SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 RealViewGICState *s = REALVIEW_GIC(obj);
-DeviceState *gicdev;
 
 memory_region_init(&s->container, OBJECT(s),
"realview-gic-container", 0x2000);
 sysbus_init_mmio(sbd, &s->container);
 
-object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
-gicdev = DEVICE(&s->gic);
-qdev_set_parent_bus(gicdev, sysbus_get_default());
-qdev_prop_set_uint32(gicdev, "num-cpu", 1);
+sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic), TYPE_ARM_GIC);
+qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", 1);
 }
 
 static void realview_gic_class_init(ObjectClass *oc, void *data)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 08/17] hw/arm/fsl-imx6: Fix introspection problems with the "fsl, imx6" device

[Thomas Huth]

Running QEMU with valgrind indicates a problem here:

echo "{'execute':'qmp_capabilities'} {'execute':'device-list-properties'," \
 "'arguments':{'typename':'fsl,imx6'}}" \
 "{'execute': 'human-monitor-command', " \
 "'arguments': {'command-line': 'info qtree'}}" | \
 valgrind -q aarch64-softmmu/qemu-system-aarch64 -M none,accel=qtest -qmp stdio
[...]
==32417== Invalid read of size 8
==32417==at 0x618A7A: qdev_print (qdev-monitor.c:686)
==32417==by 0x618A7A: qbus_print (qdev-monitor.c:719)
==32417==by 0x452B38: handle_hmp_command (monitor.c:3446)
[...]

Use the new sysbus_init_child_obj() and object_initialize_child() to make
sure that the objects are removed correctly when the parent gets destroyed.

Reviewed-by: Richard Henderson 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 hw/arm/fsl-imx6.c | 56 ---
 1 file changed, 20 insertions(+), 36 deletions(-)

diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index 4f51bd9..7b7b97f 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -38,73 +38,57 @@ static void fsl_imx6_init(Object *obj)
 int i;
 
 for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) {
-object_initialize(&s->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(&s->cpu[i]), NULL);
+object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]),
+"cortex-a9-" TYPE_ARM_CPU, &error_abort, NULL);
 }
 
-object_initialize(&s->a9mpcore, sizeof(s->a9mpcore), TYPE_A9MPCORE_PRIV);
-qdev_set_parent_bus(DEVICE(&s->a9mpcore), sysbus_get_default());
-object_property_add_child(obj, "a9mpcore", OBJECT(&s->a9mpcore), NULL);
+sysbus_init_child_obj(obj, "a9mpcore", &s->a9mpcore, sizeof(s->a9mpcore),
+  TYPE_A9MPCORE_PRIV);
 
-object_initialize(&s->ccm, sizeof(s->ccm), TYPE_IMX6_CCM);
-qdev_set_parent_bus(DEVICE(&s->ccm), sysbus_get_default());
-object_property_add_child(obj, "ccm", OBJECT(&s->ccm), NULL);
+sysbus_init_child_obj(obj, "ccm", &s->ccm, sizeof(s->ccm), TYPE_IMX6_CCM);
 
-object_initialize(&s->src, sizeof(s->src), TYPE_IMX6_SRC);
-qdev_set_parent_bus(DEVICE(&s->src), sysbus_get_default());
-object_property_add_child(obj, "src", OBJECT(&s->src), NULL);
+sysbus_init_child_obj(obj, "src", &s->src, sizeof(s->src), TYPE_IMX6_SRC);
 
 for (i = 0; i < FSL_IMX6_NUM_UARTS; i++) {
-object_initialize(&s->uart[i], sizeof(s->uart[i]), TYPE_IMX_SERIAL);
-qdev_set_parent_bus(DEVICE(&s->uart[i]), sysbus_get_default());
 snprintf(name, NAME_SIZE, "uart%d", i + 1);
-object_property_add_child(obj, name, OBJECT(&s->uart[i]), NULL);
+sysbus_init_child_obj(obj, name, &s->uart[i], sizeof(s->uart[i]),
+  TYPE_IMX_SERIAL);
 }
 
-object_initialize(&s->gpt, sizeof(s->gpt), TYPE_IMX6_GPT);
-qdev_set_parent_bus(DEVICE(&s->gpt), sysbus_get_default());
-object_property_add_child(obj, "gpt", OBJECT(&s->gpt), NULL);
+sysbus_init_child_obj(obj, "gpt", &s->gpt, sizeof(s->gpt), TYPE_IMX6_GPT);
 
 for (i = 0; i < FSL_IMX6_NUM_EPITS; i++) {
-object_initialize(&s->epit[i], sizeof(s->epit[i]), TYPE_IMX_EPIT);
-qdev_set_parent_bus(DEVICE(&s->epit[i]), sysbus_get_default());
 snprintf(name, NAME_SIZE, "epit%d", i + 1);
-object_property_add_child(obj, name, OBJECT(&s->epit[i]), NULL);
+sysbus_init_child_obj(obj, name, &s->epit[i], sizeof(s->epit[i]),
+  TYPE_IMX_EPIT);
 }
 
 for (i = 0; i < FSL_IMX6_NUM_I2CS; i++) {
-object_initialize(&s->i2c[i], sizeof(s->i2c[i]), TYPE_IMX_I2C);
-qdev_set_parent_bus(DEVICE(&s->i2c[i]), sysbus_get_default());
 snprintf(name, NAME_SIZE, "i2c%d", i + 1);
-object_property_add_child(obj, name, OBJECT(&s->i2c[i]), NULL);
+sysbus_init_child_obj(obj, name, &s->i2c[i], sizeof(s->i2c[i]),
+  TYPE_IMX_I2C);
 }
 
 for (i = 0; i < FSL_IMX6_NUM_GPIOS; i++) {
-object_initialize(&s->gpio[i], sizeof(s->gpio[i]), TYPE_IMX_GPIO);
-qdev_set_parent_bus(DEVICE(&s->gpio[i]), sysbus_get_default());
 snprintf(name, NAME_SIZE, "gpio%d", i + 1);
-object_property_add_child(obj, name, OBJECT(&s->gpio[i]), NULL);
+sysbus_init_child_obj(obj, name, &s->gpio[i], sizeof(s->gpio[i]),
+  TYPE_IMX_GPIO);
 }
 
 for (i = 0; i < FSL_IMX6_NUM_ESDHCS; i++) {
-object_initialize(&s->esdhc[i], sizeof(s->esdhc[i]), TYPE_IMX_USDHC);
-qdev_set_parent_bus(DEVICE(&s->esdhc[i]), sysbus_get_default());
 snprintf(name, NAME_SIZE, "sdhc%d", i + 1);
-object_property_add_child(obj, name, OBJECT(&s->esdhc[i]), NULL);
+sysbus_init_child