[Qemu-devel] [PATCH 0/2] fw_cfg reboot_time and splash_time fix

2018-11-01 Thread Li Qiang
The first patch ensure the reboot_time is never negative
and second change the splash-time and reboot-timeout 
type to qemu_opt_get_number per Markus's advice:

https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg06975.html


Li Qiang (2):
  hw: fw_cfg: ensure reboot_time is nonegative
  hw: fw_cfg: use qemu_opt_get_number to get splash-time and 
reboot-timeout

 hw/nvram/fw_cfg.c | 38 +++---
 vl.c  |  4 ++--
 2 files changed, 17 insertions(+), 25 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH 2/2] hw: fw_cfg: use qemu_opt_get_number to get splash-time and reboot-timeout

2018-11-01 Thread Li Qiang
Currently the splash-time and reboot-timeout in boot_opts
uses qemu_opt_get() to get a string, then convert it to a number
ourselves. This is wrong. Change the opt's type to QEMU_OPT_NUMBER
and use qemu_opt_get_number to parse it.

Signed-off-by: Li Qiang 
---
 hw/nvram/fw_cfg.c | 29 -
 vl.c  |  4 ++--
 2 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index dff6e06..282dc6a 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -117,9 +117,8 @@ error:
 
 static void fw_cfg_bootsplash(FWCfgState *s)
 {
-int boot_splash_time = -1;
+uint64_t boot_splash_time = -1;
 const char *boot_splash_filename = NULL;
-char *p;
 char *filename, *file_data;
 gsize file_size;
 int file_type;
@@ -133,18 +132,14 @@ static void fw_cfg_bootsplash(FWCfgState *s)
 if (temp != NULL) {
 boot_splash_filename = temp;
 }
-temp = qemu_opt_get(opts, "splash-time");
-if (temp != NULL) {
-p = (char *)temp;
-boot_splash_time = strtol(p, , 10);
-}
+boot_splash_time = qemu_opt_get_number(opts, "splash-time", -1);
 }
 
 /* insert splash time if user configurated */
-if (boot_splash_time >= 0) {
+if ((int64_t)boot_splash_time >= 0) {
 /* validate the input */
 if (boot_splash_time > 0x) {
-error_report("splash time is big than 65535, force it to 65535.");
+error_report("splash time is big than 65535, force it to 65535");
 boot_splash_time = 0x;
 }
 /* use little endian format */
@@ -185,26 +180,18 @@ static void fw_cfg_bootsplash(FWCfgState *s)
 
 static void fw_cfg_reboot(FWCfgState *s)
 {
-int reboot_timeout = -1;
-char *p;
-const char *temp;
+uint64_t reboot_timeout = -1;
 
 /* get user configuration */
 QemuOptsList *plist = qemu_find_opts("boot-opts");
 QemuOpts *opts = QTAILQ_FIRST(>head);
-if (opts != NULL) {
-temp = qemu_opt_get(opts, "reboot-timeout");
-if (temp != NULL) {
-p = (char *)temp;
-reboot_timeout = strtol(p, , 10);
-}
-}
+reboot_timeout = qemu_opt_get_number(opts, "reboot-timeout", -1);
 
-if (reboot_timeout >= 0) {
+if ((int64_t)reboot_timeout >= 0) {
 /* validate the input */
 if (reboot_timeout > 0x) {
 error_report("reboot timeout is larger than 65535,"
- "force it to 65535.");
+ "force it to 65535");
 reboot_timeout = 0x;
 }
 fw_cfg_add_file(s, "etc/boot-fail-wait",
diff --git a/vl.c b/vl.c
index f0bd899..60daa17 100644
--- a/vl.c
+++ b/vl.c
@@ -336,10 +336,10 @@ static QemuOptsList qemu_boot_opts = {
 .type = QEMU_OPT_STRING,
 }, {
 .name = "splash-time",
-.type = QEMU_OPT_STRING,
+.type = QEMU_OPT_NUMBER,
 }, {
 .name = "reboot-timeout",
-.type = QEMU_OPT_STRING,
+.type = QEMU_OPT_NUMBER,
 }, {
 .name = "strict",
 .type = QEMU_OPT_BOOL,
-- 
1.8.3.1




[Qemu-devel] [PATCH 1/2] hw: fw_cfg: ensure reboot_time is nonegative

2018-11-01 Thread Li Qiang
This can avoid setting a negative value to
etc/boot-fail-wait.

Signed-off-by: Li Qiang 
---
 hw/nvram/fw_cfg.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 3fcfa35..dff6e06 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -199,12 +199,17 @@ static void fw_cfg_reboot(FWCfgState *s)
 reboot_timeout = strtol(p, , 10);
 }
 }
-/* validate the input */
-if (reboot_timeout > 0x) {
-error_report("reboot timeout is larger than 65535, force it to 
65535.");
-reboot_timeout = 0x;
+
+if (reboot_timeout >= 0) {
+/* validate the input */
+if (reboot_timeout > 0x) {
+error_report("reboot timeout is larger than 65535,"
+ "force it to 65535.");
+reboot_timeout = 0x;
+}
+fw_cfg_add_file(s, "etc/boot-fail-wait",
+g_memdup(_timeout, 4), 4);
 }
-fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(_timeout, 4), 4);
 }
 
 static void fw_cfg_write(FWCfgState *s, uint8_t value)
-- 
1.8.3.1




[Qemu-devel] [PATCH] hw: fw_cfg: Improve error message when can't load splash file

2018-11-01 Thread Li Qiang
read_splashfile() reports "failed to read splash file" without
further details. Get the details from g_file_get_contents(), and
include them in the error message. Also remove unnecessary 'res'
variable.

Signed-off-by: Li Qiang 
---
 hw/nvram/fw_cfg.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 946f765..3fcfa35 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -68,15 +68,14 @@ static char *read_splashfile(char *filename, gsize 
*file_sizep,
  int *file_typep)
 {
 GError *err = NULL;
-gboolean res;
 gchar *content;
 int file_type;
 unsigned int filehead;
 int bmp_bpp;
 
-res = g_file_get_contents(filename, , file_sizep, );
-if (res == FALSE) {
-error_report("failed to read splash file '%s'", filename);
+if (!g_file_get_contents(filename, , file_sizep, )) {
+error_report("failed to read splash file '%s': %s",
+ filename, err->message);
 g_error_free(err);
 return NULL;
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH] vl: Improve error message when we can't load fw_cfg from file

2018-11-01 Thread Li Qiang
parse_fw_cfg() reports "can't load" without further details.  Get
the details from g_file_get_contents(), and include them in the
error message.

Signed-off-by: Li Qiang 
---
 vl.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 1fcacc5..f0bd899 100644
--- a/vl.c
+++ b/vl.c
@@ -2244,8 +2244,10 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, 
Error **errp)
 size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
 buf = g_memdup(str, size);
 } else {
-if (!g_file_get_contents(file, , , NULL)) {
-error_setg(errp, "can't load %s", file);
+GError *err = NULL;
+if (!g_file_get_contents(file, , , )) {
+error_setg(errp, "can't load %s: %s", file, err->message);
+g_error_free(err);
 return -1;
 }
 }
-- 
1.8.3.1




[Qemu-devel] [Question] About object oriented programming in qemu

2018-10-30 Thread Li Qiang
Hello all,

Today I read the BusClass’ definition.

The comment 
/* FIXME first arg should be BusState */
remind me that OOP in qemu is not very OOP.

Maybe we should take the first arg as the Object pointer, just as the cpp’s 
this pointer.

For example, we can define the BusClass function as this

void (*print_dev)(BusState*, Monitor *mon, DeviceState *dev, int indent);
char *(*get_dev_path)(BusState*,  DeviceState *dev);
char *(*get_fw_dev_path)(BusState*, DeviceState *dev);

So we don’t need get the BusState in the callback function, such as 
‘usb_bus_dev_print’.

I want to know do do you think it make senses?
If not I will not spend time to write the patch.

Thanks,
Li Qiang


[Qemu-devel] [PATCH] hw: qdev: fix error in comment

2018-10-30 Thread Li Qiang
Cc: qemu-triv...@nongnu.org

Signed-off-by: Li Qiang 
---
 include/hw/qdev-core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index a24d0dd566..92851e55df 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -197,7 +197,7 @@ typedef struct BusChild {
 
 /**
  * BusState:
- * @hotplug_device: link to a hotplug device associated with bus.
+ * @hotplug_handler: link to a hotplug handler associated with bus.
  */
 struct BusState {
 Object obj;
-- 
2.17.1





Re: [Qemu-devel] [PATCH v2 2/2] tests: fw_cfg: add reboot_timeout test case

2018-10-30 Thread Li Qiang
Hi Paolo,

Paolo Bonzini  于2018年10月30日周二 下午6:52写道:

> On 30/10/2018 05:28, Li Qiang wrote:
> > Signed-off-by: Li Qiang 
> > ---
> >  tests/fw_cfg-test.c | 10 ++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
> > index 1c5103fe1c..aeabd17ec0 100644
> > --- a/tests/fw_cfg-test.c
> > +++ b/tests/fw_cfg-test.c
> > @@ -99,6 +99,15 @@ static void test_fw_cfg_boot_menu(void)
> >  g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==,
> boot_menu);
> >  }
> >
> > +static void test_fw_cfg_reboot_timeout(void)
> > +{
> > +uint32_t reboot_timeout;
> > +
> > +qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
> > + _timeout, sizeof(reboot_timeout));
> > +g_assert_cmpint(reboot_timeout, <=, 65535);
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >  QTestState *s;
> > @@ -125,6 +134,7 @@ int main(int argc, char **argv)
> >  qtest_add_func("fw_cfg/max_cpus", test_fw_cfg_max_cpus);
> >  qtest_add_func("fw_cfg/numa", test_fw_cfg_numa);
> >  qtest_add_func("fw_cfg/boot_menu", test_fw_cfg_boot_menu);
> > +qtest_add_func("fw_cfg/reboot_timeout", test_fw_cfg_reboot_timeout);
> >
> >  ret = g_test_run();
>
> This test is not doing much; you could add "-boot reboot-timeout=15" and
> check the value.
>
>
This test is for this fix:
https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05254.html
As the reboot_timeout will be negative.

May I add another  "-boot reboot-timeout=15"  test?

Thanks,
Li QIang


> Paolo
>
>


[Qemu-devel] [PATCH 4/4] nvme: use pci_dev directly in nvme_realize

2018-10-29 Thread Li Qiang
There is no need to make another reference.

Signed-off-by: Li Qiang 
---
 hw/block/nvme.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index a406c23..f63e344 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1235,7 +1235,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 pci_conf[PCI_INTERRUPT_PIN] = 1;
 pci_config_set_prog_interface(pci_dev->config, 0x2);
 pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
-pcie_endpoint_cap_init(>parent_obj, 0x80);
+pcie_endpoint_cap_init(pci_dev, 0x80);
 
 n->num_namespaces = 1;
 n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
@@ -1247,10 +1247,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
**errp)
 
 memory_region_init_io(>iomem, OBJECT(n), _mmio_ops, n,
   "nvme", n->reg_size);
-pci_register_bar(>parent_obj, 0,
+pci_register_bar(pci_dev, 0,
 PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
 >iomem);
-if (msix_init_exclusive_bar(>parent_obj, n->num_queues, 4, NULL)) {
+if (msix_init_exclusive_bar(pci_dev, n->num_queues, 4, NULL)) {
 error_setg(errp, "msix_init_exclusive_bar failed");
 return;
 }
@@ -1308,7 +1308,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
 memory_region_init_io(>ctrl_mem, OBJECT(n), _cmb_ops, n,
   "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
-pci_register_bar(>parent_obj, NVME_CMBLOC_BIR(n->bar.cmbloc),
+pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
 PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 |
 PCI_BASE_ADDRESS_MEM_PREFETCH, >ctrl_mem);
 
-- 
1.8.3.1




[Qemu-devel] [PATCH 2/4] nvme: ensure the num_queues is not zero

2018-10-29 Thread Li Qiang
When it is zero, it causes segv. Backtrack:
Thread 5 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffc6c17700 (LWP 51808)]
0x55acbd90 in nvme_start_ctrl (n=0x624c8100) at hw/block/nvme.c:820
warning: Source file is more recent than executable.
820 if (unlikely(n->cq[0])) {
(gdb) bt
0  0x55acbd90 in nvme_start_ctrl (n=0x624c8100) at 
hw/block/nvme.c:820
1  0x55accdbc in nvme_write_bar (n=0x624c8100, offset=20, 
data=4587521, size=4) at hw/block/nvme.c:964
2  0x55acdd2b in nvme_mmio_write (opaque=0x624c8100, addr=20, 
data=4587521, size=4) at hw/block/nvme.c:1158
3  0x558973ed in memory_region_write_accessor (mr=0x624c89e0, 
addr=20, value=0x7fffc6c14428, size=4, shift=0, mask=4294967295, attrs=...) at 
/home/liqiang02/qemu-upstream/qemu/memory.c:500
4  0x55897600 in access_with_adjusted_size (addr=20, 
value=0x7fffc6c14428, size=4, access_size_min=2, access_size_max=8, 
access_fn=0x55897304 , mr=0x624c89e0, 
attrs=...) at /home/liqiang02/qemu-upstream/qemu/memory.c:566
5  0x5589a200 in memory_region_dispatch_write (mr=0x624c89e0, 
addr=20, data=4587521, size=4, attrs=...) at 
/home/liqiang02/qemu-upstream/qemu/memory.c:1442
6  0x55835151 in flatview_write_continue (fv=0x606e6fc0, 
addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", len=4, addr1=20, l=4, 
mr=0x624c89e0) at /home/liqiang02/qemu-upstream/qemu/exec.c:3233
7  0x5583529b in flatview_write (fv=0x606e6fc0, addr=4273930260, 
attrs=..., buf=0x7fffc8a18028 "\001", len=4) at 
/home/liqiang02/qemu-upstream/qemu/exec.c:3272
8  0x558355a1 in address_space_write (as=0x5683ade0 
, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", 
len=4) at /home/liqiang02/qemu-upstream/qemu/exec.c:3362
9  0x558355f2 in address_space_rw (as=0x5683ade0 
, addr=4273930260, attrs=..., buf=0x7fffc8a18028 "\001", 
len=4, is_write=true) at /home/liqiang02/qemu-upstream/qemu/exec.c:3373
10 0x558b66ac in kvm_cpu_exec (cpu=0x63114800) at 
/home/liqiang02/qemu-upstream/qemu/accel/kvm/kvm-all.c:2031
11 0x5587c3ac in qemu_kvm_cpu_thread_fn (arg=0x63114800) at 
/home/liqiang02/qemu-upstream/qemu/cpus.c:1277
12 0x55e54ae6 in qemu_thread_start (args=0x6032c170) at 
util/qemu-thread-posix.c:504
13 0x7fffdadbd494 in start_thread () from 
/lib/x86_64-linux-gnu/libpthread.so.0
14 0x7fffdaaffacf in clone () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) q

Signed-off-by: Li Qiang 
---
 hw/block/nvme.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 676cc48..72c9644 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1221,6 +1221,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
**errp)
 error_setg(errp, "serial property not set");
 return;
 }
+
+if (!n->num_queues) {
+error_setg(errp, "num_queues can't be zero");
+}
 blkconf_blocksizes(>conf);
 if (!blkconf_apply_backend_options(>conf, blk_is_read_only(n->conf.blk),
false, errp)) {
-- 
1.8.3.1




[Qemu-devel] [PATCH 3/4] nvme: check msix_init_exclusive_bar return value

2018-10-29 Thread Li Qiang
As this function can fail.

Signed-off-by: Li Qiang 
---
 hw/block/nvme.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 72c9644..a406c23 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1250,7 +1250,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
**errp)
 pci_register_bar(>parent_obj, 0,
 PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
 >iomem);
-msix_init_exclusive_bar(>parent_obj, n->num_queues, 4, NULL);
+if (msix_init_exclusive_bar(>parent_obj, n->num_queues, 4, NULL)) {
+error_setg(errp, "msix_init_exclusive_bar failed");
+return;
+}
 
 id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
 id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
-- 
1.8.3.1




[Qemu-devel] [PATCH 0/4] nvme: some small fixes

2018-10-29 Thread Li Qiang
Including sanity check and code cleanup.

Li Qiang (4):
  nvme: use TYPE_NVME instead of constant string
  nvme: ensure the num_queues is not zero
  nvme: check msix_init_exclusive_bar return value
  nvme: use pci_dev directly in nvme_realize

 hw/block/nvme.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH 1/4] nvme: use TYPE_NVME instead of constant string

2018-10-29 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index fc7dacb..676cc48 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1379,7 +1379,7 @@ static void nvme_instance_init(Object *obj)
 }
 
 static const TypeInfo nvme_info = {
-.name  = "nvme",
+.name  = TYPE_NVME,
 .parent= TYPE_PCI_DEVICE,
 .instance_size = sizeof(NvmeCtrl),
 .class_init= nvme_class_init,
-- 
1.8.3.1




[Qemu-devel] [PATCH v2 1/2] tests: fw_cfg: add a function to get the fw_cfg file entry

2018-10-29 Thread Li Qiang
This is useful to write qtest about fw_cfg file entry.

Signed-off-by: Li Qiang 
---
 tests/libqos/fw_cfg.c | 33 +
 tests/libqos/fw_cfg.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
index d0889d1e22..45338655f4 100644
--- a/tests/libqos/fw_cfg.c
+++ b/tests/libqos/fw_cfg.c
@@ -16,6 +16,7 @@
 #include "libqos/fw_cfg.h"
 #include "libqtest.h"
 #include "qemu/bswap.h"
+#include "standard-headers/linux/qemu_fw_cfg.h"
 
 void qfw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
@@ -54,6 +55,38 @@ uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key)
 return le64_to_cpu(value);
 }
 
+size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
+  void *data, size_t buflen)
+{
+uint32_t count;
+uint32_t i;
+unsigned char *filesbuf = NULL;
+uint32_t dsize;
+struct fw_cfg_file *p;
+size_t filesize = 0;
+
+qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, , sizeof(count));
+count = be32_to_cpu(count);
+dsize = sizeof(uint32_t) + count * sizeof(struct fw_cfg_file);
+filesbuf = g_malloc0(dsize);
+qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, filesbuf, dsize);
+p = (struct fw_cfg_file *)(filesbuf + 4);
+for (i = 0; i < count; ++i, ++p) {
+if (!strcmp(p->name, filename)) {
+uint32_t len = be32_to_cpu(p->size);
+uint16_t sel = be16_to_cpu(p->select);
+filesize = len;
+if (len > buflen) {
+len = buflen;
+}
+qfw_cfg_get(fw_cfg, sel, data, len);
+break;
+}
+}
+g_free(filesbuf);
+return filesize;
+}
+
 static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
 qtest_writew(fw_cfg->qts, fw_cfg->base, key);
diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
index 0353416af0..de73722e67 100644
--- a/tests/libqos/fw_cfg.h
+++ b/tests/libqos/fw_cfg.h
@@ -31,6 +31,8 @@ void qfw_cfg_get(QFWCFG *fw_cfg, uint16_t key, void *data, 
size_t len);
 uint16_t qfw_cfg_get_u16(QFWCFG *fw_cfg, uint16_t key);
 uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
 uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
+size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
+  void *data, size_t buflen);
 
 QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
 QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
-- 
2.11.0




[Qemu-devel] [PATCH v2 0/2] test: fw_cfg: add reboot-timeout test case

2018-10-29 Thread Li Qiang
This patchset is the test case for the following patch:

https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05254.html

v2:
Change qfw_cfg_get_file per Philippe's review:
https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg06340.html

Li Qiang (2):
  tests: fw_cfg: add a function to get the fw_cfg file entry
  tests: fw_cfg: add reboot_timeout test case

 tests/fw_cfg-test.c   | 10 ++
 tests/libqos/fw_cfg.c | 33 +
 tests/libqos/fw_cfg.h |  2 ++
 3 files changed, 45 insertions(+)

-- 
2.11.0




[Qemu-devel] [PATCH v2 2/2] tests: fw_cfg: add reboot_timeout test case

2018-10-29 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 tests/fw_cfg-test.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 1c5103fe1c..aeabd17ec0 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -99,6 +99,15 @@ static void test_fw_cfg_boot_menu(void)
 g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==, boot_menu);
 }
 
+static void test_fw_cfg_reboot_timeout(void)
+{
+uint32_t reboot_timeout;
+
+qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
+ _timeout, sizeof(reboot_timeout));
+g_assert_cmpint(reboot_timeout, <=, 65535);
+}
+
 int main(int argc, char **argv)
 {
 QTestState *s;
@@ -125,6 +134,7 @@ int main(int argc, char **argv)
 qtest_add_func("fw_cfg/max_cpus", test_fw_cfg_max_cpus);
 qtest_add_func("fw_cfg/numa", test_fw_cfg_numa);
 qtest_add_func("fw_cfg/boot_menu", test_fw_cfg_boot_menu);
+qtest_add_func("fw_cfg/reboot_timeout", test_fw_cfg_reboot_timeout);
 
 ret = g_test_run();
 
-- 
2.11.0




Re: [Qemu-devel] [PATCH 1/2] tests: fw_cfg: add a function to get the fw_cfg file entry

2018-10-29 Thread li qiang

Hello Philippe,


Thanks for your review,

I will send v2 later.


Thanks.

Li Qiang

在 2018/10/30 7:46, Philippe Mathieu-Daudé 写道:
> Hi Li,
>
> On 28/10/18 13:40, Li Qiang wrote:
>> This is useful to write qtest abount fw_cfg file entry.
>>
>> Signed-off-by: Li Qiang 
>> ---
>>   tests/libqos/fw_cfg.c | 30 ++
>>   tests/libqos/fw_cfg.h |  2 ++
>>   2 files changed, 32 insertions(+)
>>
>> diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
>> index d0889d1e22..2dd7a498ab 100644
>> --- a/tests/libqos/fw_cfg.c
>> +++ b/tests/libqos/fw_cfg.c
>> @@ -16,6 +16,7 @@
>>   #include "libqos/fw_cfg.h"
>>   #include "libqtest.h"
>>   #include "qemu/bswap.h"
>> +#include "standard-headers/linux/qemu_fw_cfg.h"
>>     void qfw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
>>   {
>> @@ -54,6 +55,35 @@ uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t 
>> key)
>>   return le64_to_cpu(value);
>>   }
>
> Can this return size_t the size of the file?
> So qfw_cfg_get_file(..., buflen=0) returns the file size.
>
>> +void qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
>> +  void *data, size_t buflen)
>> +{
>> +    uint32_t count;
>> +    uint32_t i;
>> +    unsigned char *filesbuf = NULL;
>> +    uint32_t dsize;
>> +    struct fw_cfg_file *p;
>
>    size_t filesize = 0;
>
>> +
>> +    qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, , sizeof(count));
>> +    count = be32_to_cpu(count);
>> +    dsize = sizeof(uint32_t) + count * sizeof(struct fw_cfg_file);
>> +    filesbuf = g_malloc0(dsize);
>> +    qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, filesbuf, dsize);
>> +    p = (struct fw_cfg_file *)(filesbuf + 4);
>> +    for (i = 0; i < count; ++i, ++p) {
>> +    if (!strcmp(p->name, filename)) {
>> +    uint32_t len = be32_to_cpu(p->size);
>
>
>> +    uint16_t sel = be16_to_cpu(p->select);
>> +    void *buf = g_malloc0(len);
>
> Why call malloc() here?
>
>> +    qfw_cfg_get(fw_cfg, sel, buf, len);
>
> And not copy directly to 'data':
>
>    filesize = len;
>    if (len > buflen) {
>    len = buflen;
>    }
>    qfw_cfg_get(fw_cfg, sel, data, len);
>
>> +    memcpy(data, buf, buflen);
>> +    g_free(buf);
>
> Dropping 2 previous lines.
>
>> +    break;
>> +    }
>> +    }
>> +    g_free(filesbuf);
>
>    return filesize;
>
>> +}
>> +
>>   static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
>>   {
>>   qtest_writew(fw_cfg->qts, fw_cfg->base, key);
>> diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
>> index 0353416af0..50e4227638 100644
>> --- a/tests/libqos/fw_cfg.h
>> +++ b/tests/libqos/fw_cfg.h
>> @@ -31,6 +31,8 @@ void qfw_cfg_get(QFWCFG *fw_cfg, uint16_t key, void 
>> *data, size_t len);
>>   uint16_t qfw_cfg_get_u16(QFWCFG *fw_cfg, uint16_t key);
>>   uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
>>   uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
>> +void qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
>> +  void *data, size_t buflen);
>>     QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
>>   QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
>>
>


Re: [Qemu-devel] [PATCH] fw_cfg_reboot: ensure reboot_time is nonegative

2018-10-29 Thread Li Qiang
Philippe Mathieu-Daudé  于2018年10月30日周二 上午7:50写道:

> On 25/10/18 3:45, Li Qiang wrote:
> > Hello Laszlo and Philippe ,
> >
> > Thanks for your review,
> >
> >
> >
> > Philippe Mathieu-Daudé mailto:phi...@redhat.com>> 于
> > 2018年10月25日周四 上午6:56写道:
> >
> > Hi,
> >
> > On 24/10/18 13:35, Laszlo Ersek wrote:
> >  > On 10/24/18 09:11, Li Qiang wrote:
> >      >> This can avoid setting a negative value to
> >  >> etc/boot-fail-wait.
> >
> > Li Qiang, can you add a qtest for this?
> >
> >
> > I will try to write one.
> > Is it ok to write it without this patch, as the test will be not passed?
>
> It is OK to add the test after the fix, else the test would fail.
> But now your test protect from future regressions.
>
>
Ok, I think this fix is not that bad, this is the same as
'boot_splash_time'.
 As for Laszlo's consideration,  we think we can send another patch to
drop strtol in both 'boot_splash_time' and this 'reboot-timeout'.

Thanks,
Li Qiang


>
> > Thanks,
> > Li Qiang
> >
> >
> >  >>
> >  >> Signed-off-by: Li Qiang  liq...@gmail.com>>
> >  >> ---
> >  >>   hw/nvram/fw_cfg.c | 15 ++-
> >  >>   1 file changed, 10 insertions(+), 5 deletions(-)
> >  >>
> >  >> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> >  >> index f4a52d8..276dcb1 100644
> >  >> --- a/hw/nvram/fw_cfg.c
> >  >> +++ b/hw/nvram/fw_cfg.c
> >  >> @@ -199,12 +199,17 @@ static void fw_cfg_reboot(FWCfgState *s)
> >  >>   reboot_timeout = strtol(p, , 10);
> >  >>   }
> >  >>   }
> >  >> -/* validate the input */
> >  >> -if (reboot_timeout > 0x) {
> >  >> -error_report("reboot timeout is larger than 65535,
> > force it to 65535.");
> >  >> -reboot_timeout = 0x;
> >  >> +
> >  >> +if (reboot_timeout >= 0) {
> >  >> +/* validate the input */
> >  >> +if (reboot_timeout > 0x) {
> >  >> +error_report("reboot timeout is larger than 65535,"
> >  >> + "force it to 65535.");
> >  >> +reboot_timeout = 0x;
> >  >> +}
> >  >> +fw_cfg_add_file(s, "etc/boot-fail-wait",
> >  >> +g_memdup(_timeout, 4), 4);
> >  >>   }
> >  >> -fw_cfg_add_file(s, "etc/boot-fail-wait",
> > g_memdup(_timeout, 4), 4);
> >  >>   }
> >  >>
> >  >>   static void fw_cfg_write(FWCfgState *s, uint8_t value)
> >  >>
> >  >
> >  > I don't feel strongly about fixing this issue.
> >  >
> >  > However, if we decide to fix it, we should start with the
> bare-bones
> >  > strtol() call, visible at the top of the context. I'm not
> > up-to-date on
> >  > what's the best QEMU helper function for this, but I seem to
> > remember it
> >  > checks for trailing garbage, and perhaps even for range. Maybe we
> > should
> >
> > Are you suggesting qemu_strtoul()? I agree this would be cleaner.
> >
> >
> >
> > I reference the 'boot_splash_time' in fw_cfg_bootsplash.
> > We can also change there.
> >
> >  > even use a different (better) option parsing facility thatn
> >  > qemu_opt_get(). Adding Eric and Markus.
> >  >
> >  > Also, I would suggest forcing negative values (that were
> explicitly
> >  > specified) to some sensible positive default, such as 5 seconds
> > or so.
> >  >
> >  > Thanks
> >  > Laszlo
> >  >
> >
>


Re: [Qemu-devel] [PATCH 2/2] tests: fw_cfg: add reboot_timeout test case

2018-10-29 Thread li qiang

在 2018/10/30 8:08, Philippe Mathieu-Daudé 写道:
> On 28/10/18 13:40, Li Qiang wrote:
>> Signed-off-by: Li Qiang 
>> ---
>>   tests/fw_cfg-test.c | 13 -
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
>> index 1c5103fe1c..37765f15f8 100644
>> --- a/tests/fw_cfg-test.c
>> +++ b/tests/fw_cfg-test.c
>> @@ -99,6 +99,15 @@ static void test_fw_cfg_boot_menu(void)
>>   g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==, 
>> boot_menu);
>>   }
>>   +static void test_fw_cfg_reboot_timeout(void)
>> +{
>> +    uint32_t reboot_timeout;
>> +
>> +    qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
>> + _timeout, sizeof(reboot_timeout));
>> +    g_assert_cmpint(reboot_timeout, <=, 65535);
>> +}
>> +
>>   int main(int argc, char **argv)
>>   {
>>   QTestState *s;
>> @@ -106,7 +115,8 @@ int main(int argc, char **argv)
>>     g_test_init(, , NULL);
>>   -    s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8");
>> +    s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8"
>> +   " -boot reboot-timeout=4294967295");
>
> I'd rather change this test to use qtest_add_data_func() ...:
>
>     qtest_add_data_func("fw_cfg/reboot_timeout", "-boot 
> reboot-timeout=4294967295 ", test_fw_cfg_reboot_timeout);
>
> ... to avoid adding this command line option to all the tests, because 
> all tests are now failing:
>
> $ make check-qtest-i386
> [...]
> ERROR:qemu/tests/fw_cfg-test.c:33:test_fw_cfg_signature: assertion 
> failed (buf == "QEMU"): ("\377\377\377\377" == "QEMU")
> ERROR:qemu/tests/fw_cfg-test.c:40:test_fw_cfg_id: assertion failed: 
> ((id == 1) || (id == 3))
> ERROR:qemu/tests/fw_cfg-test.c:52:test_fw_cfg_uuid: assertion failed: 
> (memcmp(buf, uuid, sizeof(buf)) == 0)
> ERROR:qemu/tests/fw_cfg-test.c:57:test_fw_cfg_ram_size: assertion 
> failed (qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE) == ram_size): (-1 == 
> 134217728)
> ERROR:qemu/tests/fw_cfg-test.c:62:test_fw_cfg_nographic: assertion 
> failed (qfw_cfg_get_u16(fw_cfg, FW_CFG_NOGRAPHIC) == 0): (65535 == 0)
> ERROR:qemu/tests/fw_cfg-test.c:67:test_fw_cfg_nb_cpus: assertion 
> failed (qfw_cfg_get_u16(fw_cfg, FW_CFG_NB_CPUS) == nb_cpus): (65535 == 1)
> ERROR:qemu/tests/fw_cfg-test.c:72:test_fw_cfg_max_cpus: assertion 
> failed (qfw_cfg_get_u16(fw_cfg, FW_CFG_MAX_CPUS) == max_cpus): (65535 
> == 1)
> ERROR:qemu/tests/fw_cfg-test.c:80:test_fw_cfg_numa: assertion failed 
> (qfw_cfg_get_u64(fw_cfg, FW_CFG_NUMA) == nb_nodes): (-1 == 0)
> ERROR:qemu/tests/fw_cfg-test.c:99:test_fw_cfg_boot_menu: assertion 
> failed (qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU) == boot_menu): 
> (65535 == 0)
>
>
> (did you test your patch?)
>
Of course I test it, but only in x86_64.


Here is the result without the fix patch.

xxxqemu# QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 
tests/fw_cfg-test
/x86_64/fw_cfg/signature: OK
/x86_64/fw_cfg/id: OK
/x86_64/fw_cfg/uuid: OK
/x86_64/fw_cfg/ram_size: OK
/x86_64/fw_cfg/nographic: OK
/x86_64/fw_cfg/nb_cpus: OK
/x86_64/fw_cfg/max_cpus: OK
/x86_64/fw_cfg/numa: OK
/x86_64/fw_cfg/boot_menu: OK
/x86_64/fw_cfg/reboot_timeout: **
ERROR:tests/fw_cfg-test.c:108:test_fw_cfg_reboot_timeout: assertion 
failed (reboot_timeout <= 65535): (4294967295 <= 65535)
Aborted
xxxqemu# make check-qtest-x86_64
   GTESTER check-qtest-x86_64
**
ERROR:tests/fw_cfg-test.c:108:test_fw_cfg_reboot_timeout: assertion 
failed (reboot_timeout <= 65535): (4294967295 <= 65535)
GTester: last random seed: R02Sfed7191ede4ed60d4f3069f9ec808e73
xxxqemu/tests/Makefile.include:805: recipe for target 
'check-qtest-x86_64' failed
make: *** [check-qtest-x86_64] Error 1

I did it in i386 right now. Following is the results:

xxxqemu# QTEST_QEMU_BINARY=i386-softmmu/qemu-system-i386 tests/fw_cfg-test
/i386/fw_cfg/signature: OK
/i386/fw_cfg/id: OK
/i386/fw_cfg/uuid: OK
/i386/fw_cfg/ram_size: OK
/i386/fw_cfg/nographic: OK
/i386/fw_cfg/nb_cpus: OK
/i386/fw_cfg/max_cpus: OK
/i386/fw_cfg/numa: OK
/i386/fw_cfg/boot_menu: OK
/i386/fw_cfg/reboot_timeout: **
ERROR:tests/fw_cfg-test.c:108:test_fw_cfg_reboot_timeout: assertion 
failed (reboot_timeout <= 65535): (4294967295 <= 65535)
Aborted
xxxqemu# make chek-qtest-i386
make: *** No rule to make target 'chek-qtest-i386'.  Stop.
xxxqemu# make check-qtest-i386
   GTESTER check-qtest-i386
**
ERROR:tests/fw_cfg-test.c:108:test_fw_cfg_reboot_timeout: assertion 
failed (reboot_timeout <= 65535): (4294967295 <= 65535)
GTester: last random seed: R02S7659b379e665961e0060afeb449948b2
xxxqemu/tests/Ma

[Qemu-devel] [PATCH 2/2] nvme: free cmbuf in nvme_exit

2018-10-29 Thread Li Qiang
This avoid a memory leak in unhotplug nvme device.

Signed-off-by: Li Qiang 
---
 hw/block/nvme.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 359a06d0ad..09d7c90259 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1332,6 +1332,9 @@ static void nvme_exit(PCIDevice *pci_dev)
 g_free(n->cq);
 g_free(n->sq);
 
+if (n->cmb_size_mb) {
+g_free(n->cmbuf);
+}
 msix_uninit_exclusive_bar(pci_dev);
 }
 
-- 
2.11.0




[Qemu-devel] [PATCH 0/2] nvme: fix two issues in nvme unhotplug

2018-10-29 Thread Li Qiang
The first corrent the refcount and second fix a memory leak.

Li Qiang (2):
  nvme: don't unref ctrl_mem when device unrealized
  nvme: free cmbuf in nvme_exit

 hw/block/nvme.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.11.0




[Qemu-devel] [PATCH 1/2] nvme: don't unref ctrl_mem when device unrealized

2018-10-29 Thread Li Qiang
Currently, when hotplug/unhotplug nvme device, it will cause an
assert in object.c. Following is the backtrack:

ERROR:qom/object.c:981:object_unref: assertion failed: (obj->ref > 0)

Thread 2 "qemu-system-x86" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffcbd32700 (LWP 18844)]
0x7fffdb9e4fff in raise () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) bt
/lib/x86_64-linux-gnu/libglib-2.0.so.0
/lib/x86_64-linux-gnu/libglib-2.0.so.0
qom/object.c:981
/home/liqiang02/qemu-upstream/qemu/memory.c:1732
/home/liqiang02/qemu-upstream/qemu/memory.c:285
util/qemu-thread-posix.c:504
/lib/x86_64-linux-gnu/libpthread.so.0

This is caused by memory_region_unref in nvme_exit.

Remove it to make the PCIdevice refcount correct.

Signed-off-by: Li Qiang 
---
 hw/block/nvme.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index fc7dacb816..359a06d0ad 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1331,9 +1331,6 @@ static void nvme_exit(PCIDevice *pci_dev)
 g_free(n->namespaces);
 g_free(n->cq);
 g_free(n->sq);
-if (n->cmbsz) {
-memory_region_unref(>ctrl_mem);
-}
 
 msix_uninit_exclusive_bar(pci_dev);
 }
-- 
2.11.0




[Qemu-devel] [PATCH 2/2] tests: fw_cfg: add reboot_timeout test case

2018-10-28 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 tests/fw_cfg-test.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 1c5103fe1c..37765f15f8 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -99,6 +99,15 @@ static void test_fw_cfg_boot_menu(void)
 g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==, boot_menu);
 }
 
+static void test_fw_cfg_reboot_timeout(void)
+{
+uint32_t reboot_timeout;
+
+qfw_cfg_get_file(fw_cfg, "etc/boot-fail-wait",
+ _timeout, sizeof(reboot_timeout));
+g_assert_cmpint(reboot_timeout, <=, 65535);
+}
+
 int main(int argc, char **argv)
 {
 QTestState *s;
@@ -106,7 +115,8 @@ int main(int argc, char **argv)
 
 g_test_init(, , NULL);
 
-s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8");
+s = qtest_init("-uuid 4600cb32-38ec-4b2f-8acb-81c6ea54f2d8"
+   " -boot reboot-timeout=4294967295");
 
 fw_cfg = pc_fw_cfg_init(s);
 
@@ -125,6 +135,7 @@ int main(int argc, char **argv)
 qtest_add_func("fw_cfg/max_cpus", test_fw_cfg_max_cpus);
 qtest_add_func("fw_cfg/numa", test_fw_cfg_numa);
 qtest_add_func("fw_cfg/boot_menu", test_fw_cfg_boot_menu);
+qtest_add_func("fw_cfg/reboot_timeout", test_fw_cfg_reboot_timeout);
 
 ret = g_test_run();
 
-- 
2.17.1





[Qemu-devel] [PATCH 0/2] test: fw_cfg: add reboot-timeout test case

2018-10-28 Thread Li Qiang
This patchset is the test case for the following patch:

https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05254.html

Li Qiang (2):
  tests: fw_cfg: add a function to get the fw_cfg file entry
  tests: fw_cfg: add reboot_timeout test case

 tests/fw_cfg-test.c   | 13 -
 tests/libqos/fw_cfg.c | 30 ++
 tests/libqos/fw_cfg.h |  2 ++
 3 files changed, 44 insertions(+), 1 deletion(-)

-- 
2.17.1





[Qemu-devel] [PATCH 1/2] tests: fw_cfg: add a function to get the fw_cfg file entry

2018-10-28 Thread Li Qiang
This is useful to write qtest abount fw_cfg file entry.

Signed-off-by: Li Qiang 
---
 tests/libqos/fw_cfg.c | 30 ++
 tests/libqos/fw_cfg.h |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/tests/libqos/fw_cfg.c b/tests/libqos/fw_cfg.c
index d0889d1e22..2dd7a498ab 100644
--- a/tests/libqos/fw_cfg.c
+++ b/tests/libqos/fw_cfg.c
@@ -16,6 +16,7 @@
 #include "libqos/fw_cfg.h"
 #include "libqtest.h"
 #include "qemu/bswap.h"
+#include "standard-headers/linux/qemu_fw_cfg.h"
 
 void qfw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
@@ -54,6 +55,35 @@ uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key)
 return le64_to_cpu(value);
 }
 
+void qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
+  void *data, size_t buflen)
+{
+uint32_t count;
+uint32_t i;
+unsigned char *filesbuf = NULL;
+uint32_t dsize;
+struct fw_cfg_file *p;
+
+qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, , sizeof(count));
+count = be32_to_cpu(count);
+dsize = sizeof(uint32_t) + count * sizeof(struct fw_cfg_file);
+filesbuf = g_malloc0(dsize);
+qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, filesbuf, dsize);
+p = (struct fw_cfg_file *)(filesbuf + 4);
+for (i = 0; i < count; ++i, ++p) {
+if (!strcmp(p->name, filename)) {
+uint32_t len = be32_to_cpu(p->size);
+uint16_t sel = be16_to_cpu(p->select);
+void *buf = g_malloc0(len);
+qfw_cfg_get(fw_cfg, sel, buf, len);
+memcpy(data, buf, buflen);
+g_free(buf);
+break;
+}
+}
+g_free(filesbuf);
+}
+
 static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
 qtest_writew(fw_cfg->qts, fw_cfg->base, key);
diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
index 0353416af0..50e4227638 100644
--- a/tests/libqos/fw_cfg.h
+++ b/tests/libqos/fw_cfg.h
@@ -31,6 +31,8 @@ void qfw_cfg_get(QFWCFG *fw_cfg, uint16_t key, void *data, 
size_t len);
 uint16_t qfw_cfg_get_u16(QFWCFG *fw_cfg, uint16_t key);
 uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
 uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
+void qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
+  void *data, size_t buflen);
 
 QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
 QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
-- 
2.17.1





Re: [Qemu-devel] [Qemu-trivial] [PATCH] piix: use TYPE_FOO constants than string constats

2018-10-25 Thread Li Qiang
Hello Laurent,
Thanks,

This patch has been in Michael's pull request.
Maybe you can drop it.

Thanks,
Li Qiang



Laurent Vivier  于2018年10月25日周四 下午9:52写道:

> On 11/10/2018 13:38, Li Qiang wrote:
> > Make them more QOMConventional.
> > Cc:qemu-triv...@nongnu.org
> >
> > Signed-off-by: Li Qiang 
> > ---
> >  hw/pci-host/piix.c | 11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
>
> Applied,
>
> Thanks,
> Laurent
>


Re: [Qemu-devel] [PATCH] fw_cfg_reboot: ensure reboot_time is nonegative

2018-10-24 Thread Li Qiang
Hello   Laszlo and Philippe ,

Thanks for your review,



Philippe Mathieu-Daudé  于2018年10月25日周四 上午6:56写道:

> Hi,
>
> On 24/10/18 13:35, Laszlo Ersek wrote:
> > On 10/24/18 09:11, Li Qiang wrote:
> >> This can avoid setting a negative value to
> >> etc/boot-fail-wait.
>
> Li Qiang, can you add a qtest for this?
>
>
I will try to write one.
Is it ok to write it without this patch, as the test will be not passed?

Thanks,
Li Qiang




> >>
> >> Signed-off-by: Li Qiang 
> >> ---
> >>   hw/nvram/fw_cfg.c | 15 ++-
> >>   1 file changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> >> index f4a52d8..276dcb1 100644
> >> --- a/hw/nvram/fw_cfg.c
> >> +++ b/hw/nvram/fw_cfg.c
> >> @@ -199,12 +199,17 @@ static void fw_cfg_reboot(FWCfgState *s)
> >>   reboot_timeout = strtol(p, , 10);
> >>   }
> >>   }
> >> -/* validate the input */
> >> -if (reboot_timeout > 0x) {
> >> -error_report("reboot timeout is larger than 65535, force it to
> 65535.");
> >> -reboot_timeout = 0x;
> >> +
> >> +if (reboot_timeout >= 0) {
> >> +/* validate the input */
> >> +if (reboot_timeout > 0x) {
> >> +error_report("reboot timeout is larger than 65535,"
> >> + "force it to 65535.");
> >> +reboot_timeout = 0x;
> >> +}
> >> +fw_cfg_add_file(s, "etc/boot-fail-wait",
> >> +g_memdup(_timeout, 4), 4);
> >>   }
> >> -fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(_timeout,
> 4), 4);
> >>   }
> >>
> >>   static void fw_cfg_write(FWCfgState *s, uint8_t value)
> >>
> >
> > I don't feel strongly about fixing this issue.
> >
> > However, if we decide to fix it, we should start with the bare-bones
> > strtol() call, visible at the top of the context. I'm not up-to-date on
> > what's the best QEMU helper function for this, but I seem to remember it
> > checks for trailing garbage, and perhaps even for range. Maybe we should
>
> Are you suggesting qemu_strtoul()? I agree this would be cleaner.
>
>

I reference the 'boot_splash_time' in fw_cfg_bootsplash.
We can also change there.



> > even use a different (better) option parsing facility thatn
> > qemu_opt_get(). Adding Eric and Markus.
> >
> > Also, I would suggest forcing negative values (that were explicitly
> > specified) to some sensible positive default, such as 5 seconds or so.
> >
> > Thanks
> > Laszlo
> >
>


[Qemu-devel] [PATCH] fw_cfg_reboot: ensure reboot_time is nonegative

2018-10-24 Thread Li Qiang
This can avoid setting a negative value to
etc/boot-fail-wait.

Signed-off-by: Li Qiang 
---
 hw/nvram/fw_cfg.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index f4a52d8..276dcb1 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -199,12 +199,17 @@ static void fw_cfg_reboot(FWCfgState *s)
 reboot_timeout = strtol(p, , 10);
 }
 }
-/* validate the input */
-if (reboot_timeout > 0x) {
-error_report("reboot timeout is larger than 65535, force it to 
65535.");
-reboot_timeout = 0x;
+
+if (reboot_timeout >= 0) {
+/* validate the input */
+if (reboot_timeout > 0x) {
+error_report("reboot timeout is larger than 65535,"
+ "force it to 65535.");
+reboot_timeout = 0x;
+}
+fw_cfg_add_file(s, "etc/boot-fail-wait",
+g_memdup(_timeout, 4), 4);
 }
-fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(_timeout, 4), 4);
 }
 
 static void fw_cfg_write(FWCfgState *s, uint8_t value)
-- 
1.8.3.1




[Qemu-devel] [PATCH] fw_cfg: print error message when reading splashfile failed

2018-10-23 Thread Li Qiang
Also remove unnecessary 'res' variable.

Signed-off-by: Li Qiang 
---
 hw/nvram/fw_cfg.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 946f765..f4a52d8 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -68,15 +68,14 @@ static char *read_splashfile(char *filename, gsize 
*file_sizep,
  int *file_typep)
 {
 GError *err = NULL;
-gboolean res;
 gchar *content;
 int file_type;
 unsigned int filehead;
 int bmp_bpp;
 
-res = g_file_get_contents(filename, , file_sizep, );
-if (res == FALSE) {
-error_report("failed to read splash file '%s'", filename);
+if (!g_file_get_contents(filename, , file_sizep, )) {
+error_report("failed to read splash file '%s', %s",
+ filename, err->message);
 g_error_free(err);
 return NULL;
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH] vl.c: print error message if loading fw_cfg file failed

2018-10-23 Thread Li Qiang
It makes sense to print the error message while reading
file failed.

Signed-off-by: Li Qiang 
---
 vl.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index b2a405f..ee6f982 100644
--- a/vl.c
+++ b/vl.c
@@ -2234,8 +2234,10 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, 
Error **errp)
 size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
 buf = g_memdup(str, size);
 } else {
-if (!g_file_get_contents(file, , , NULL)) {
-error_setg(errp, "can't load %s", file);
+GError *err = NULL;
+if (!g_file_get_contents(file, , , )) {
+error_setg(errp, "can't load %s, %s", file, err->message);
+g_error_free(err);
 return -1;
 }
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH] usb: ohci: make num_ports to an unsinged integer

2018-10-22 Thread Li Qiang
This can avoid setting OCHIState.num_ports to a negative num.

Signed-off-by: Li Qiang 
---
 hw/usb/hcd-ohci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 66656a1..c34cf5b 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -57,7 +57,7 @@ typedef struct {
 qemu_irq irq;
 MemoryRegion mem;
 AddressSpace *as;
-int num_ports;
+uint32_t num_ports;
 const char *name;
 
 QEMUTimer *eof_timer;
@@ -1850,7 +1850,7 @@ static USBBusOps ohci_bus_ops = {
 };
 
 static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
-  int num_ports, dma_addr_t localmem_base,
+  uint32_t num_ports, dma_addr_t localmem_base,
   char *masterbus, uint32_t firstport,
   AddressSpace *as, Error **errp)
 {
@@ -1860,7 +1860,7 @@ static void usb_ohci_init(OHCIState *ohci, DeviceState 
*dev,
 ohci->as = as;
 
 if (num_ports > OHCI_MAX_PORTS) {
-error_setg(errp, "OHCI num-ports=%d is too big (limit is %d ports)",
+error_setg(errp, "OHCI num-ports=%u is too big (limit is %u ports)",
num_ports, OHCI_MAX_PORTS);
 return;
 }
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH 7/7] vfio: platform: destory mutex in error path

2018-10-21 Thread Li Qiang
Hello Auger,

Auger Eric  于2018年10月20日周六 上午12:41写道:

> Hi Li,
>
> On 10/19/18 7:20 AM, Li Qiang wrote:
> > Signed-off-by: Li Qiang 
> > ---
> >  hw/vfio/platform.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> > index ba19143..e9d9e80 100644
> > --- a/hw/vfio/platform.c
> > +++ b/hw/vfio/platform.c
> > @@ -668,7 +668,7 @@ static void vfio_platform_realize(DeviceState *dev,
> Error **errp)
> >  error_setg(errp, "%s", gerr->message);
> >  g_error_free(gerr);
> >  g_free(path);
> > -return;
> > +goto out;
> You must set ret to != 0 otherwise the qemu_mutex_destroy will not be
> reached I think.

Also this will fix the fact we are not prepending the
> vfio error prefix in that case, as we should.
>
> Besides I am unsure about the cleanup strategy in case or error in
> vfio_platform_realize(). The qemu process should always exit in case of
> failure in vfio_platform_realize(). Platform devices can only be
> cold-plugged through the qemu CLI.


Got this.


> Cleaning all the allocated resources
> may add a substantial amount of code.


Agree.


Thanks,
Li Qiang


> For instance resources allocated
> in vfio_base_device_init() are not freed either. Comprehensive free in
> realize() functions may only be needed in case of hotplug I think.
>
> Thanks
>
> Eric
> >  }
> >  g_free(path);
> >  vdev->compat = contents;
> > @@ -691,6 +691,8 @@ out:
> >  return;
> >  }
> >
> > +qemu_mutex_destroy(>intp_mutex);
> > +
> >  if (vdev->vbasedev.name) {
> >  error_prepend(errp, ERR_PREFIX, vdev->vbasedev.name);
> >  } else {
> >
>


[Qemu-devel] [PATCH 0/2] hw: ccid-card-emulated: fix some resources leak

2018-10-19 Thread Li Qiang


Li Qiang (2):
  hw: ccid-card-emulated: introduce clean_event_notifier
  hw: ccid-card-emulated: cleanup resource when realize in error path

 hw/usb/ccid-card-emulated.c | 27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH 1/2] hw: ccid-card-emulated: introduce clean_event_notifier

2018-10-19 Thread Li Qiang
Call it in device unrealize function.

Signed-off-by: Li Qiang 
---
 hw/usb/ccid-card-emulated.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index 5c8b3c9..b356edb 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -409,6 +409,12 @@ static int init_event_notifier(EmulatedState *card, Error 
**errp)
 return 0;
 }
 
+static void clean_event_notifier(EmulatedState *card)
+{
+event_notifier_set_handler(>notifier, NULL);
+event_notifier_cleanup(>notifier);
+}
+
 #define CERTIFICATES_DEFAULT_DB "/etc/pki/nssdb"
 #define CERTIFICATES_ARGS_TEMPLATE\
 "db=\"%s\" use_hw=no soft=(,Virtual Reader,CAC,,%s,%s,%s)"
@@ -556,6 +562,7 @@ static void emulated_unrealize(CCIDCardState *base, Error 
**errp)
 qemu_cond_signal(>handle_apdu_cond);
 qemu_thread_join(>apdu_thread_id);
 
+clean_event_notifier(card);
 /* threads exited, can destroy all condvars/mutexes */
 qemu_cond_destroy(>handle_apdu_cond);
 qemu_mutex_destroy(>handle_apdu_mutex);
-- 
1.8.3.1




[Qemu-devel] [PATCH 2/2] hw: ccid-card-emulated: cleanup resource when realize in error path

2018-10-19 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/usb/ccid-card-emulated.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index b356edb..25976ed 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -499,7 +499,7 @@ static void emulated_realize(CCIDCardState *base, Error 
**errp)
 card->reader = NULL;
 card->quit_apdu_thread = 0;
 if (init_event_notifier(card, errp) < 0) {
-return;
+goto out1;
 }
 
 card->backend = 0;
@@ -513,7 +513,7 @@ static void emulated_realize(CCIDCardState *base, Error 
**errp)
 for (ptable = backend_enum_table; ptable->name != NULL; ++ptable) {
 error_append_hint(errp, "%s\n", ptable->name);
 }
-return;
+goto out2;
 }
 
 /* TODO: a passthru backened that works on local machine. third card 
type?*/
@@ -523,31 +523,39 @@ static void emulated_realize(CCIDCardState *base, Error 
**errp)
 } else {
 error_setg(errp, "%s: you must provide all three certs for"
" certificates backend", TYPE_EMULATED_CCID);
-return;
+goto out2;
 }
 } else {
 if (card->backend != BACKEND_NSS_EMULATED) {
 error_setg(errp, "%s: bad backend specified. The options are:%s"
" (default), %s.", TYPE_EMULATED_CCID,
BACKEND_NSS_EMULATED_NAME, BACKEND_CERTIFICATES_NAME);
-return;
+goto out2;
 }
 if (card->cert1 != NULL || card->cert2 != NULL || card->cert3 != NULL) 
{
 error_setg(errp, "%s: unexpected cert parameters to nss emulated "
"backend", TYPE_EMULATED_CCID);
-return;
+goto out2;
 }
 /* default to mirroring the local hardware readers */
 ret = wrap_vcard_emul_init(NULL);
 }
 if (ret != VCARD_EMUL_OK) {
 error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
-return;
+goto out2;
 }
 qemu_thread_create(>event_thread_id, "ccid/event", event_thread,
card, QEMU_THREAD_JOINABLE);
 qemu_thread_create(>apdu_thread_id, "ccid/apdu", handle_apdu_thread,
card, QEMU_THREAD_JOINABLE);
+
+out2:
+clean_event_notifier(card);
+out1:
+qemu_cond_destroy(>handle_apdu_cond);
+qemu_mutex_destroy(>handle_apdu_mutex);
+qemu_mutex_destroy(>vreader_mutex);
+qemu_mutex_destroy(>event_list_mutex);
 }
 
 static void emulated_unrealize(CCIDCardState *base, Error **errp)
-- 
1.8.3.1




[Qemu-devel] [PATCH 7/7] vfio: platform: destory mutex in error path

2018-10-18 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/vfio/platform.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index ba19143..e9d9e80 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -668,7 +668,7 @@ static void vfio_platform_realize(DeviceState *dev, Error 
**errp)
 error_setg(errp, "%s", gerr->message);
 g_error_free(gerr);
 g_free(path);
-return;
+goto out;
 }
 g_free(path);
 vdev->compat = contents;
@@ -691,6 +691,8 @@ out:
 return;
 }
 
+qemu_mutex_destroy(>intp_mutex);
+
 if (vdev->vbasedev.name) {
 error_prepend(errp, ERR_PREFIX, vdev->vbasedev.name);
 } else {
-- 
1.8.3.1




[Qemu-devel] [PATCH 6/7] vfio: platform: free timer in error path

2018-10-18 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/vfio/platform.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 6a4fd7b..ba19143 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -518,6 +518,7 @@ static int vfio_populate_device(VFIODevice *vbasedev, Error 
**errp)
 return 0;
 irq_err:
 timer_del(vdev->mmap_timer);
+timer_free(vdev->mmap_timer);
 QLIST_FOREACH_SAFE(intp, >intp_list, next, tmp) {
 QLIST_REMOVE(intp, next);
 g_free(intp);
-- 
1.8.3.1




[Qemu-devel] [PATCH 1/7] vfio-pci: make "vfio-pci-nohotplug" as MACRO

2018-10-18 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/vfio/pci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 8b73582..1f05b57 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -40,6 +40,8 @@
 #define TYPE_VFIO_PCI "vfio-pci"
 #define PCI_VFIO(obj)OBJECT_CHECK(VFIOPCIDevice, obj, TYPE_VFIO_PCI)
 
+#define TYPE_VFIO_PCI_NOHOTPLUG "vfio-pci-nohotplug"
+
 static void vfio_disable_interrupts(VFIOPCIDevice *vdev);
 static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled);
 
@@ -3277,8 +3279,8 @@ static void vfio_pci_nohotplug_dev_class_init(ObjectClass 
*klass, void *data)
 }
 
 static const TypeInfo vfio_pci_nohotplug_dev_info = {
-.name = "vfio-pci-nohotplug",
-.parent = "vfio-pci",
+.name = TYPE_VFIO_PCI_NOHOTPLUG,
+.parent = TYPE_VFIO_PCI,
 .instance_size = sizeof(VFIOPCIDevice),
 .class_init = vfio_pci_nohotplug_dev_class_init,
 };
-- 
1.8.3.1




[Qemu-devel] [PATCH 4/7] vfio: paltform: fix a typo

2018-10-18 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/vfio/platform.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index ba03dcd..5992fe7 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -72,7 +72,7 @@ static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev,
 g_free(intp->interrupt);
 g_free(intp);
 error_setg_errno(errp, -ret,
- "failed to initialize trigger eventd notifier");
+ "failed to initialize trigger eventfd notifier");
 return NULL;
 }
 if (vfio_irq_is_automasked(intp)) {
@@ -84,7 +84,7 @@ static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev,
 g_free(intp->unmask);
 g_free(intp);
 error_setg_errno(errp, -ret,
- "failed to initialize resample eventd notifier");
+ "failed to initialize resample eventfd notifier");
 return NULL;
 }
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH 3/7] vfio: drop TYPE_FOO MACRO in VMStateDescription

2018-10-18 Thread Li Qiang
As the vmstate structure names aren't related with
the QOM type names.
Per Peter's mail:
-->https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02175.html

Signed-off-by: Li Qiang 
---
 hw/vfio/amd-xgbe.c  | 2 +-
 hw/vfio/ap.c| 2 +-
 hw/vfio/calxeda-xgmac.c | 2 +-
 hw/vfio/ccw.c   | 2 +-
 hw/vfio/platform.c  | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/amd-xgbe.c b/hw/vfio/amd-xgbe.c
index ee64a3b..1b06c0f 100644
--- a/hw/vfio/amd-xgbe.c
+++ b/hw/vfio/amd-xgbe.c
@@ -26,7 +26,7 @@ static void amd_xgbe_realize(DeviceState *dev, Error **errp)
 }
 
 static const VMStateDescription vfio_platform_amd_xgbe_vmstate = {
-.name = TYPE_VFIO_AMD_XGBE,
+.name = "vfio-amd-xgbe",
 .unmigratable = 1,
 };
 
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index b332395..35b01b3 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -147,7 +147,7 @@ static void vfio_ap_reset(DeviceState *dev)
 }
 
 static const VMStateDescription vfio_ap_vmstate = {
-.name = TYPE_VFIO_AP_DEVICE,
+.name = "vfio-ap",
 .unmigratable = 1,
 };
 
diff --git a/hw/vfio/calxeda-xgmac.c b/hw/vfio/calxeda-xgmac.c
index e7767c4..6cc608b 100644
--- a/hw/vfio/calxeda-xgmac.c
+++ b/hw/vfio/calxeda-xgmac.c
@@ -26,7 +26,7 @@ static void calxeda_xgmac_realize(DeviceState *dev, Error 
**errp)
 }
 
 static const VMStateDescription vfio_platform_calxeda_xgmac_vmstate = {
-.name = TYPE_VFIO_CALXEDA_XGMAC,
+.name = "vfio-calxeda-xgmac",
 .unmigratable = 1,
 };
 
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 9246729..4786d46 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -466,7 +466,7 @@ static Property vfio_ccw_properties[] = {
 };
 
 static const VMStateDescription vfio_ccw_vmstate = {
-.name = TYPE_VFIO_CCW,
+.name = "vfio-ccw",
 .unmigratable = 1,
 };
 
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 64c1af6..ba03dcd 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -697,7 +697,7 @@ out:
 }
 
 static const VMStateDescription vfio_platform_vmstate = {
-.name = TYPE_VFIO_PLATFORM,
+.name = "vfio-platform",
 .unmigratable = 1,
 };
 
-- 
1.8.3.1




[Qemu-devel] [PATCH 5/7] vfio: platform: cleanup the notifier in error path

2018-10-18 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/vfio/platform.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 5992fe7..6a4fd7b 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -80,6 +80,7 @@ static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev,
 intp->unmask = g_malloc0(sizeof(EventNotifier));
 ret = event_notifier_init(intp->unmask, 0);
 if (ret) {
+event_notifier_cleanup(intp->interrupt);
 g_free(intp->interrupt);
 g_free(intp->unmask);
 g_free(intp);
-- 
1.8.3.1




[Qemu-devel] [PATCH 2/7] vfio: ap-device: make it more QOMConventional

2018-10-18 Thread Li Qiang
As the documentation says "use TYPE_FOO constants"
This also changes the parent of ap-device's MACRO.

Signed-off-by: Li Qiang 
---
 hw/s390x/ap-device.c |  2 +-
 hw/vfio/ap.c | 12 ++--
 include/hw/s390x/ap-device.h |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/s390x/ap-device.c b/hw/s390x/ap-device.c
index f5ac8db..64a5f11 100644
--- a/hw/s390x/ap-device.c
+++ b/hw/s390x/ap-device.c
@@ -22,7 +22,7 @@ static void ap_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ap_device_info = {
-.name = AP_DEVICE_TYPE,
+.name = TYPE_AP_DEVICE,
 .parent = TYPE_DEVICE,
 .instance_size = sizeof(APDevice),
 .class_size = sizeof(DeviceClass),
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 3962bb7..b332395 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -28,7 +28,7 @@
 #include "hw/s390x/ap-bridge.h"
 #include "exec/address-spaces.h"
 
-#define VFIO_AP_DEVICE_TYPE  "vfio-ap"
+#define TYPE_VFIO_AP_DEVICE  "vfio-ap"
 
 typedef struct VFIOAPDevice {
 APDevice apdev;
@@ -36,7 +36,7 @@ typedef struct VFIOAPDevice {
 } VFIOAPDevice;
 
 #define VFIO_AP_DEVICE(obj) \
-OBJECT_CHECK(VFIOAPDevice, (obj), VFIO_AP_DEVICE_TYPE)
+OBJECT_CHECK(VFIOAPDevice, (obj), TYPE_VFIO_AP_DEVICE)
 
 static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
 {
@@ -69,7 +69,7 @@ static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, 
Error **errp)
 
 if (!group_path) {
 error_setg(errp, "%s: no iommu_group found for %s: %s",
-   VFIO_AP_DEVICE_TYPE, vapdev->vdev.sysfsdev, 
gerror->message);
+   TYPE_VFIO_AP_DEVICE, vapdev->vdev.sysfsdev, 
gerror->message);
 return NULL;
 }
 
@@ -147,7 +147,7 @@ static void vfio_ap_reset(DeviceState *dev)
 }
 
 static const VMStateDescription vfio_ap_vmstate = {
-.name = VFIO_AP_DEVICE_TYPE,
+.name = TYPE_VFIO_AP_DEVICE,
 .unmigratable = 1,
 };
 
@@ -167,8 +167,8 @@ static void vfio_ap_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo vfio_ap_info = {
-.name = VFIO_AP_DEVICE_TYPE,
-.parent = AP_DEVICE_TYPE,
+.name = TYPE_VFIO_AP_DEVICE,
+.parent = TYPE_AP_DEVICE,
 .instance_size = sizeof(VFIOAPDevice),
 .class_init = vfio_ap_class_init,
 };
diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h
index 765e908..d7eced4 100644
--- a/include/hw/s390x/ap-device.h
+++ b/include/hw/s390x/ap-device.h
@@ -10,13 +10,13 @@
 #ifndef HW_S390X_AP_DEVICE_H
 #define HW_S390X_AP_DEVICE_H
 
-#define AP_DEVICE_TYPE   "ap-device"
+#define TYPE_AP_DEVICE  "ap-device"
 
 typedef struct APDevice {
 DeviceState parent_obj;
 } APDevice;
 
 #define AP_DEVICE(obj) \
-OBJECT_CHECK(APDevice, (obj), AP_DEVICE_TYPE)
+OBJECT_CHECK(APDevice, (obj), TYPE_AP_DEVICE)
 
 #endif /* HW_S390X_AP_DEVICE_H */
-- 
1.8.3.1




[Qemu-devel] [PATCH 0/7] vfio: some trivial fixes

2018-10-18 Thread Li Qiang
This patch set contains some trivial issue such as
QOMConvetion, typo and resources leak in vfio.

Li Qiang (7):
  vfio-pci: make "vfio-pci-nohotplug" as MACRO
  vfio: ap-device: make it more QOMConventional
  vfio: drop TYPE_FOO MACRO in VMStateDescription
  vfio: paltform: fix a typo
  vfio: platform: cleanup the notifier in error path
  vfio: platform: free timer in error path
  vfio: platform: destory mutex in error path

 hw/s390x/ap-device.c |  2 +-
 hw/vfio/amd-xgbe.c   |  2 +-
 hw/vfio/ap.c | 12 ++--
 hw/vfio/calxeda-xgmac.c  |  2 +-
 hw/vfio/ccw.c|  2 +-
 hw/vfio/pci.c|  6 --
 hw/vfio/platform.c   | 12 
 include/hw/s390x/ap-device.h |  4 ++--
 8 files changed, 24 insertions(+), 18 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH] qga: fix an off-by-one issue

2018-10-17 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 qga/commands-win32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 98d9735..e3842d1 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -465,7 +465,7 @@ static STORAGE_BUS_TYPE win2qemu[] = {
 
 static GuestDiskBusType find_bus_type(STORAGE_BUS_TYPE bus)
 {
-if (bus > ARRAY_SIZE(win2qemu) || (int)bus < 0) {
+if (bus >= ARRAY_SIZE(win2qemu) || (int)bus < 0) {
 return GUEST_DISK_BUS_TYPE_UNKNOWN;
 }
 return win2qemu[(int)bus];
-- 
1.8.3.1




[Qemu-devel] [PATCH] pc_init1: fix a typo in comment

2018-10-16 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/i386/pc_piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index dc09466b3e..0133668ce2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -111,7 +111,7 @@ static void pc_init1(MachineState *machine,
  *so legacy non-PAE guests can get as much memory as possible in
  *the 32bit address space below 4G.
  *
- *  - Note that Xen has its own ram setp code in xen_ram_init(),
+ *  - Note that Xen has its own ram setup code in xen_ram_init(),
  *called via xen_hvm_init().
  *
  * Examples:
-- 
2.17.1





[Qemu-devel] [PATCH] kvm: move vcpu mmap size to KVMState

2018-10-16 Thread Li Qiang
First the mmap size is a kvm ioctl, so it can go kvm initialization.
Second this can avoid triggering KVM_GET_VCPU_MMAP_SIZE while
initializing and destroying every VCPU.

Signed-off-by: Li Qiang 
---
 accel/kvm/kvm-all.c | 28 ++--
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index de12f78eb8..f5cc47ec9f 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -101,6 +101,7 @@ struct KVMState
 int nr_allocated_irq_routes;
 unsigned long *used_gsi_bitmap;
 unsigned int gsi_count;
+long mmap_size;
 QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
 #endif
 KVMMemoryListener memory_listener;
@@ -285,20 +286,12 @@ static int kvm_set_user_memory_region(KVMMemoryListener 
*kml, KVMSlot *slot, boo
 int kvm_destroy_vcpu(CPUState *cpu)
 {
 KVMState *s = kvm_state;
-long mmap_size;
 struct KVMParkedVcpu *vcpu = NULL;
 int ret = 0;
 
 DPRINTF("kvm_destroy_vcpu\n");
 
-mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
-if (mmap_size < 0) {
-ret = mmap_size;
-DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
-goto err;
-}
-
-ret = munmap(cpu->kvm_run, mmap_size);
+ret = munmap(cpu->kvm_run, s->mmap_size);
 if (ret < 0) {
 goto err;
 }
@@ -332,7 +325,6 @@ static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
 int kvm_init_vcpu(CPUState *cpu)
 {
 KVMState *s = kvm_state;
-long mmap_size;
 int ret;
 
 DPRINTF("kvm_init_vcpu\n");
@@ -347,14 +339,7 @@ int kvm_init_vcpu(CPUState *cpu)
 cpu->kvm_state = s;
 cpu->vcpu_dirty = true;
 
-mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
-if (mmap_size < 0) {
-ret = mmap_size;
-DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
-goto err;
-}
-
-cpu->kvm_run = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED,
+cpu->kvm_run = mmap(NULL, s->mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED,
 cpu->kvm_fd, 0);
 if (cpu->kvm_run == MAP_FAILED) {
 ret = -errno;
@@ -1583,6 +1568,13 @@ static int kvm_init(MachineState *ms)
 
 s->vmfd = ret;
 
+s->mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
+if (s->mmap_size < 0) {
+ret = s->mmap_size;
+fprintf(stderr, "KVM_GET_VCPU_MMAP_SIZE failed\n");
+goto err;
+}
+
 /* check the vcpu limits */
 soft_vcpus_limit = kvm_recommended_vcpus(s);
 hard_vcpus_limit = kvm_max_vcpus(s);
-- 
2.11.0




[Qemu-devel] [PATCH v2] hw: scsi-disk: make it more QOMconventional

2018-10-13 Thread Li Qiang
Signed-off-by: Li Qiang 
---

Change since v1:
-fix a typo in scsi_cd_info's name

 hw/scsi/scsi-disk.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index c43163cef4..8e9fa5ca6b 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -55,6 +55,10 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 #define DEFAULT_MAX_IO_SIZE INT_MAX /* 2 GB - 1 block */
 
 #define TYPE_SCSI_DISK_BASE "scsi-disk-base"
+#define TYPE_SCSI_DISK  "scsi-disk"
+#define TYPE_SCSI_HD"scsi-hd"
+#define TYPE_SCSI_BLOCK "scsi-block"
+#define TYPE_SCSI_CD"scsi-cd"
 
 #define SCSI_DISK_BASE(obj) \
  OBJECT_CHECK(SCSIDiskState, (obj), TYPE_SCSI_DISK_BASE)
@@ -2357,7 +2361,7 @@ static const BlockDevOps scsi_disk_block_ops = {
 
 static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 if (s->media_changed) {
 s->media_changed = false;
 scsi_device_set_ua(>qdev, SENSE_CODE(MEDIUM_CHANGED));
@@ -2366,7 +2370,7 @@ static void scsi_disk_unit_attention_reported(SCSIDevice 
*dev)
 
 static void scsi_realize(SCSIDevice *dev, Error **errp)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 
 if (!s->qdev.conf.blk) {
 error_setg(errp, "drive property not set");
@@ -2429,7 +2433,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
 
 static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 /* can happen for devices without drive. The error message for missing
  * backend will be issued in scsi_realize
  */
@@ -2446,7 +2450,7 @@ static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
 
 static void scsi_cd_realize(SCSIDevice *dev, Error **errp)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 int ret;
 
 if (!dev->conf.blk) {
@@ -2549,7 +2553,7 @@ static const SCSIReqOps *const 
scsi_disk_reqops_dispatch[256] = {
 static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
  uint8_t *buf, void *hba_private)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+SCSIDiskState *s = SCSI_DISK_BASE(d);
 SCSIRequest *req;
 const SCSIReqOps *ops;
 uint8_t command;
@@ -2601,7 +2605,7 @@ static int get_device_type(SCSIDiskState *s)
 
 static void scsi_block_realize(SCSIDevice *dev, Error **errp)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 int sg_version;
 int rc;
 
@@ -2879,7 +2883,7 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, 
uint32_t tag,
uint32_t lun, uint8_t *buf,
void *hba_private)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+SCSIDiskState *s = SCSI_DISK_BASE(d);
 
 if (scsi_block_is_passthrough(s, buf)) {
 return scsi_req_alloc(_generic_req_ops, >qdev, tag, lun,
@@ -2893,7 +2897,7 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, 
uint32_t tag,
 static int scsi_block_parse_cdb(SCSIDevice *d, SCSICommand *cmd,
   uint8_t *buf, void *hba_private)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+SCSIDiskState *s = SCSI_DISK_BASE(d);
 
 if (scsi_block_is_passthrough(s, buf)) {
 return scsi_bus_parse_cdb(>qdev, cmd, buf, hba_private);
@@ -3002,7 +3006,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo scsi_hd_info = {
-.name  = "scsi-hd",
+.name  = TYPE_SCSI_HD,
 .parent= TYPE_SCSI_DISK_BASE,
 .class_init= scsi_hd_class_initfn,
 };
@@ -3033,7 +3037,7 @@ static void scsi_cd_class_initfn(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo scsi_cd_info = {
-.name  = "scsi-cd",
+.name  = TYPE_SCSI_CD,
 .parent= TYPE_SCSI_DISK_BASE,
 .class_init= scsi_cd_class_initfn,
 };
@@ -3071,7 +3075,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, 
void *data)
 }
 
 static const TypeInfo scsi_block_info = {
-.name  = "scsi-block",
+.name  = TYPE_SCSI_BLOCK,
 .parent= TYPE_SCSI_DISK_BASE,
 .class_init= scsi_block_class_initfn,
 };
@@ -3111,7 +3115,7 @@ static void scsi_disk_class_initfn(ObjectClass *klass, 
void *data)
 }
 
 static const TypeInfo scsi_disk_info = {
-.name 

[Qemu-devel] [PATCH] block: change some function return type to bool

2018-10-13 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 block/block-backend.c  | 8 
 include/sysemu/block-backend.h | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index dc0cd57724..2a8f3b55f8 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1708,7 +1708,7 @@ void blk_error_action(BlockBackend *blk, BlockErrorAction 
action,
 }
 }
 
-int blk_is_read_only(BlockBackend *blk)
+bool blk_is_read_only(BlockBackend *blk)
 {
 BlockDriverState *bs = blk_bs(blk);
 
@@ -1719,18 +1719,18 @@ int blk_is_read_only(BlockBackend *blk)
 }
 }
 
-int blk_is_sg(BlockBackend *blk)
+bool blk_is_sg(BlockBackend *blk)
 {
 BlockDriverState *bs = blk_bs(blk);
 
 if (!bs) {
-return 0;
+return false;
 }
 
 return bdrv_is_sg(bs);
 }
 
-int blk_enable_write_cache(BlockBackend *blk)
+bool blk_enable_write_cache(BlockBackend *blk)
 {
 return blk->enable_write_cache;
 }
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 830d873f24..c96bcdee14 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -166,9 +166,9 @@ BlockErrorAction blk_get_error_action(BlockBackend *blk, 
bool is_read,
   int error);
 void blk_error_action(BlockBackend *blk, BlockErrorAction action,
   bool is_read, int error);
-int blk_is_read_only(BlockBackend *blk);
-int blk_is_sg(BlockBackend *blk);
-int blk_enable_write_cache(BlockBackend *blk);
+bool blk_is_read_only(BlockBackend *blk);
+bool blk_is_sg(BlockBackend *blk);
+bool blk_enable_write_cache(BlockBackend *blk);
 void blk_set_enable_write_cache(BlockBackend *blk, bool wce);
 void blk_invalidate_cache(BlockBackend *blk, Error **errp);
 bool blk_is_inserted(BlockBackend *blk);
-- 
2.17.1





[Qemu-devel] [PATCH] hw: scsi-disk: make it more QOMconventional

2018-10-13 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/scsi/scsi-disk.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index c43163cef4..15582fa0e2 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -55,6 +55,10 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 #define DEFAULT_MAX_IO_SIZE INT_MAX /* 2 GB - 1 block */
 
 #define TYPE_SCSI_DISK_BASE "scsi-disk-base"
+#define TYPE_SCSI_DISK  "scsi-disk"
+#define TYPE_SCSI_HD"scsi-hd"
+#define TYPE_SCSI_BLOCK "scsi-block"
+#define TYPE_SCSI_CD"scsi-cd"
 
 #define SCSI_DISK_BASE(obj) \
  OBJECT_CHECK(SCSIDiskState, (obj), TYPE_SCSI_DISK_BASE)
@@ -2357,7 +2361,7 @@ static const BlockDevOps scsi_disk_block_ops = {
 
 static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 if (s->media_changed) {
 s->media_changed = false;
 scsi_device_set_ua(>qdev, SENSE_CODE(MEDIUM_CHANGED));
@@ -2366,7 +2370,7 @@ static void scsi_disk_unit_attention_reported(SCSIDevice 
*dev)
 
 static void scsi_realize(SCSIDevice *dev, Error **errp)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 
 if (!s->qdev.conf.blk) {
 error_setg(errp, "drive property not set");
@@ -2429,7 +2433,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
 
 static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 /* can happen for devices without drive. The error message for missing
  * backend will be issued in scsi_realize
  */
@@ -2446,7 +2450,7 @@ static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
 
 static void scsi_cd_realize(SCSIDevice *dev, Error **errp)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 int ret;
 
 if (!dev->conf.blk) {
@@ -2549,7 +2553,7 @@ static const SCSIReqOps *const 
scsi_disk_reqops_dispatch[256] = {
 static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
  uint8_t *buf, void *hba_private)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+SCSIDiskState *s = SCSI_DISK_BASE(d);
 SCSIRequest *req;
 const SCSIReqOps *ops;
 uint8_t command;
@@ -2601,7 +2605,7 @@ static int get_device_type(SCSIDiskState *s)
 
 static void scsi_block_realize(SCSIDevice *dev, Error **errp)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = SCSI_DISK_BASE(dev);
 int sg_version;
 int rc;
 
@@ -2879,7 +2883,7 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, 
uint32_t tag,
uint32_t lun, uint8_t *buf,
void *hba_private)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+SCSIDiskState *s = SCSI_DISK_BASE(d);
 
 if (scsi_block_is_passthrough(s, buf)) {
 return scsi_req_alloc(_generic_req_ops, >qdev, tag, lun,
@@ -2893,7 +2897,7 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, 
uint32_t tag,
 static int scsi_block_parse_cdb(SCSIDevice *d, SCSICommand *cmd,
   uint8_t *buf, void *hba_private)
 {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+SCSIDiskState *s = SCSI_DISK_BASE(d);
 
 if (scsi_block_is_passthrough(s, buf)) {
 return scsi_bus_parse_cdb(>qdev, cmd, buf, hba_private);
@@ -3002,7 +3006,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo scsi_hd_info = {
-.name  = "scsi-hd",
+.name  = TYPE_SCSI_HD,
 .parent= TYPE_SCSI_DISK_BASE,
 .class_init= scsi_hd_class_initfn,
 };
@@ -3033,7 +3037,7 @@ static void scsi_cd_class_initfn(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo scsi_cd_info = {
-.name  = "scsi-cd",
+.name  = TYPE_SCSI_HD,
 .parent= TYPE_SCSI_DISK_BASE,
 .class_init= scsi_cd_class_initfn,
 };
@@ -3071,7 +3075,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, 
void *data)
 }
 
 static const TypeInfo scsi_block_info = {
-.name  = "scsi-block",
+.name  = TYPE_SCSI_BLOCK,
 .parent= TYPE_SCSI_DISK_BASE,
 .class_init= scsi_block_class_initfn,
 };
@@ -3111,7 +3115,7 @@ static void scsi_disk_class_initfn(ObjectClass *klass, 
void *data)
 }
 
 static const TypeInfo scsi_disk_info = {
-.name  = "scsi-disk",
+.name  = TYPE_SCSI_DISK,
 .parent= TYPE_SCSI_DISK_BASE,
 .class_init= scsi_disk_class_initfn,
 };
-- 
2.17.1





[Qemu-devel] [PATCH] hw: ne2000: make it more QOMconventional

2018-10-13 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/net/ne2000.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 07d79e317f..ab71ad49cb 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -33,6 +33,10 @@
 
 #define MAX_ETH_FRAME_SIZE 1514
 
+#define TYPE_NE2000 "ne2k_pci"
+#define NE2000(obj) \
+OBJECT_CHECK(PCINE2000State, (obj), TYPE_NE2000)
+
 #define E8390_CMD  0x00  /* The command register (for all pages) */
 /* Page 0 register offsets. */
 #define EN0_CLDALO 0x01/* Low byte of current local dma addr  RD */
@@ -720,7 +724,7 @@ static NetClientInfo net_ne2000_info = {
 
 static void pci_ne2000_realize(PCIDevice *pci_dev, Error **errp)
 {
-PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+PCINE2000State *d = NE2000(pci_dev);
 NE2000State *s;
 uint8_t *pci_conf;
 
@@ -742,7 +746,7 @@ static void pci_ne2000_realize(PCIDevice *pci_dev, Error 
**errp)
 
 static void pci_ne2000_exit(PCIDevice *pci_dev)
 {
-PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+PCINE2000State *d = NE2000(pci_dev);
 NE2000State *s = >ne2000;
 
 qemu_del_nic(s->nic);
@@ -751,13 +755,12 @@ static void pci_ne2000_exit(PCIDevice *pci_dev)
 
 static void ne2000_instance_init(Object *obj)
 {
-PCIDevice *pci_dev = PCI_DEVICE(obj);
-PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+PCINE2000State *d = NE2000(obj);
 NE2000State *s = >ne2000;
 
 device_add_bootindex_property(obj, >c.bootindex,
   "bootindex", "/ethernet-phy@0",
-  _dev->qdev, NULL);
+  DEVICE(obj), NULL);
 }
 
 static Property ne2000_properties[] = {
@@ -782,7 +785,7 @@ static void ne2000_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo ne2000_info = {
-.name  = "ne2k_pci",
+.name  = TYPE_NE2000,
 .parent= TYPE_PCI_DEVICE,
 .instance_size = sizeof(PCINE2000State),
 .class_init= ne2000_class_init,
-- 
2.17.1





[Qemu-devel] [PATCH] hw: AC97: make it more QOMconventional

2018-10-13 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/audio/ac97.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 337402e9c6..d799533aa9 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -123,6 +123,10 @@ enum {
 
 #define MUTE_SHIFT 15
 
+#define TYPE_AC97 "AC97"
+#define AC97(obj) \
+OBJECT_CHECK(AC97LinkState, (obj), TYPE_AC97)
+
 #define REC_MASK 7
 enum {
 REC_MIC = 0,
@@ -1340,7 +1344,7 @@ static void ac97_on_reset (DeviceState *dev)
 
 static void ac97_realize(PCIDevice *dev, Error **errp)
 {
-AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
+AC97LinkState *s = AC97(dev);
 uint8_t *c = s->dev.config;
 
 /* TODO: no need to override */
@@ -1389,7 +1393,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
 
 static void ac97_exit(PCIDevice *dev)
 {
-AC97LinkState *s = DO_UPCAST(AC97LinkState, dev, dev);
+AC97LinkState *s = AC97(dev);
 
 AUD_close_in(>card, s->voice_pi);
 AUD_close_out(>card, s->voice_po);
@@ -1399,7 +1403,7 @@ static void ac97_exit(PCIDevice *dev)
 
 static int ac97_init (PCIBus *bus)
 {
-pci_create_simple (bus, -1, "AC97");
+pci_create_simple(bus, -1, TYPE_AC97);
 return 0;
 }
 
@@ -1427,7 +1431,7 @@ static void ac97_class_init (ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo ac97_info = {
-.name  = "AC97",
+.name  = TYPE_AC97,
 .parent= TYPE_PCI_DEVICE,
 .instance_size = sizeof (AC97LinkState),
 .class_init= ac97_class_init,
-- 
2.17.1





[Qemu-devel] [PATCH] hw: edu: drop DO_UPCAST

2018-10-12 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/misc/edu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 0687ffd343..cdcf550dd7 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -342,7 +342,7 @@ static void *edu_fact_thread(void *opaque)
 
 static void pci_edu_realize(PCIDevice *pdev, Error **errp)
 {
-EduState *edu = DO_UPCAST(EduState, pdev, pdev);
+EduState *edu = EDU(pdev);
 uint8_t *pci_conf = pdev->config;
 
 pci_config_set_interrupt_pin(pci_conf, 1);
@@ -365,7 +365,7 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
 
 static void pci_edu_uninit(PCIDevice *pdev)
 {
-EduState *edu = DO_UPCAST(EduState, pdev, pdev);
+EduState *edu = EDU(pdev);
 
 qemu_mutex_lock(>thr_mutex);
 edu->stopping = true;
-- 
2.17.1





[Qemu-devel] [PATCH] vfio-pci: make vfio-pci device more QOM conventional

2018-10-11 Thread Li Qiang
Define a TYPE_VFIO_PCI and drop DO_UPCAST.

Signed-off-by: Li Qiang 
---
 hw/vfio/pci.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 866f0deeb7..3f232aedff 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -37,6 +37,9 @@
 
 #define MSIX_CAP_LENGTH 12
 
+#define TYPE_VFIO_PCI "vfio-pci"
+#define PCI_VFIO(obj)OBJECT_CHECK(VFIOPCIDevice, obj, TYPE_VFIO_PCI)
+
 static void vfio_disable_interrupts(VFIOPCIDevice *vdev);
 static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled);
 
@@ -222,7 +225,7 @@ static void vfio_intx_disable_kvm(VFIOPCIDevice *vdev)
 
 static void vfio_intx_update(PCIDevice *pdev)
 {
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+VFIOPCIDevice *vdev = PCI_VFIO(pdev);
 PCIINTxRoute route;
 Error *err = NULL;
 
@@ -477,7 +480,7 @@ static void vfio_update_kvm_msi_virq(VFIOMSIVector *vector, 
MSIMessage msg,
 static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
MSIMessage *msg, IOHandler *handler)
 {
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+VFIOPCIDevice *vdev = PCI_VFIO(pdev);
 VFIOMSIVector *vector;
 int ret;
 
@@ -574,7 +577,7 @@ static int vfio_msix_vector_use(PCIDevice *pdev,
 
 static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr)
 {
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+VFIOPCIDevice *vdev = PCI_VFIO(pdev);
 VFIOMSIVector *vector = >msi_vectors[nr];
 
 trace_vfio_msix_vector_release(vdev->vbasedev.name, nr);
@@ -1086,7 +1089,7 @@ static const MemoryRegionOps vfio_vga_ops = {
  */
 static void vfio_sub_page_bar_update_mapping(PCIDevice *pdev, int bar)
 {
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+VFIOPCIDevice *vdev = PCI_VFIO(pdev);
 VFIORegion *region = >bars[bar].region;
 MemoryRegion *mmap_mr, *region_mr, *base_mr;
 PCIIORegion *r;
@@ -1132,7 +1135,7 @@ static void vfio_sub_page_bar_update_mapping(PCIDevice 
*pdev, int bar)
  */
 uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len)
 {
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+VFIOPCIDevice *vdev = PCI_VFIO(pdev);
 uint32_t emu_bits = 0, emu_val = 0, phys_val = 0, val;
 
 memcpy(_bits, vdev->emulated_config_bits + addr, len);
@@ -1165,7 +1168,7 @@ uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t 
addr, int len)
 void vfio_pci_write_config(PCIDevice *pdev,
uint32_t addr, uint32_t val, int len)
 {
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+VFIOPCIDevice *vdev = PCI_VFIO(pdev);
 uint32_t val_le = cpu_to_le32(val);
 
 trace_vfio_pci_write_config(vdev->vbasedev.name, addr, val, len);
@@ -2801,7 +2804,7 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice 
*vdev)
 
 static void vfio_realize(PCIDevice *pdev, Error **errp)
 {
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+VFIOPCIDevice *vdev = PCI_VFIO(pdev);
 VFIODevice *vbasedev_iter;
 VFIOGroup *group;
 char *tmp, *subsys, group_path[PATH_MAX], *group_name;
@@ -3084,8 +3087,7 @@ error:
 
 static void vfio_instance_finalize(Object *obj)
 {
-PCIDevice *pci_dev = PCI_DEVICE(obj);
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pci_dev);
+VFIOPCIDevice *vdev = PCI_VFIO(obj);
 VFIOGroup *group = vdev->vbasedev.group;
 
 vfio_display_finalize(vdev);
@@ -3105,7 +3107,7 @@ static void vfio_instance_finalize(Object *obj)
 
 static void vfio_exitfn(PCIDevice *pdev)
 {
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+VFIOPCIDevice *vdev = PCI_VFIO(pdev);
 
 vfio_unregister_req_notifier(vdev);
 vfio_unregister_err_notifier(vdev);
@@ -3120,8 +3122,7 @@ static void vfio_exitfn(PCIDevice *pdev)
 
 static void vfio_pci_reset(DeviceState *dev)
 {
-PCIDevice *pdev = DO_UPCAST(PCIDevice, qdev, dev);
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+VFIOPCIDevice *vdev = PCI_VFIO(dev);
 
 trace_vfio_pci_reset(vdev->vbasedev.name);
 
@@ -3161,7 +3162,7 @@ post_reset:
 static void vfio_instance_init(Object *obj)
 {
 PCIDevice *pci_dev = PCI_DEVICE(obj);
-VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, PCI_DEVICE(obj));
+VFIOPCIDevice *vdev = PCI_VFIO(obj);
 
 device_add_bootindex_property(obj, >bootindex,
   "bootindex", NULL,
@@ -3245,7 +3246,7 @@ static void vfio_pci_dev_class_init(ObjectClass *klass, 
void *data)
 }
 
 static const TypeInfo vfio_pci_dev_info = {
-.name = "vfio-pci",
+.name = TYPE_VFIO_PCI,
 .parent = TYPE_PCI_DEVICE,
 .instance_size = sizeof(VFIOPCIDevice),
 .class_init = vfio_pci_dev_class_init,
-- 
2.11.0




[Qemu-devel] [PATCH v2] piix_pci: change the i440fx data sheet link

2018-10-11 Thread Li Qiang
Seems the intel link is unavailable, change it to qemu site.

Signed-off-by: Li Qiang 
Reviewed-by: Philippe Mathieu-Daud?? 
---
 hw/pci-host/piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0e608347c1..56a42055f1 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -40,7 +40,7 @@
 
 /*
  * I440FX chipset data sheet.
- * http://download.intel.com/design/chipsets/datashts/29054901.pdf
+ * https://wiki.qemu.org/File:29054901.pdf
  */
 
 #define I440FX_PCI_HOST_BRIDGE(obj) \
-- 
2.17.1





[Qemu-devel] [PATCH] piix_pci: change the i440fx data sheet link

2018-10-11 Thread Li Qiang
Seems the intel link is unavailable, change it to qemu site.

Signed-off-by: Li Qiang 
---
 hw/pci-host/piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0e608347c1..e709abeb08 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -40,7 +40,7 @@
 
 /*
  * I440FX chipset data sheet.
- * http://download.intel.com/design/chipsets/datashts/29054901.pdf
+ * https://wiki.qemu.org/images/b/bb/29054901.pdf
  */
 
 #define I440FX_PCI_HOST_BRIDGE(obj) \
-- 
2.17.1





[Qemu-devel] [PATCH] piix: use TYPE_FOO constants than string constats

2018-10-11 Thread Li Qiang
Make them more QOMConventional.
Cc:qemu-triv...@nongnu.org 

Signed-off-by: Li Qiang 
---
 hw/pci-host/piix.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 230d5d2ea3..5881e63364 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -95,6 +95,9 @@ typedef struct PIIX3State {
 #define I440FX_PCI_DEVICE(obj) \
 OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
 
+#define TYPE_PIIX3_DEVICE "PIIX3"
+#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
+
 struct PCII440FXState {
 /*< private >*/
 PCIDevice parent_obj;
@@ -413,13 +416,13 @@ PCIBus *i440fx_init(const char *host_type, const char 
*pci_type,
  * These additional routes can be discovered through ACPI. */
 if (xen_enabled()) {
 PCIDevice *pci_dev = pci_create_simple_multifunction(b,
- -1, true, "PIIX3-xen");
+ -1, true, TYPE_PIIX3_XEN_DEVICE);
 piix3 = PIIX3_PCI_DEVICE(pci_dev);
 pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
 piix3, XEN_PIIX_NUM_PIRQS);
 } else {
 PCIDevice *pci_dev = pci_create_simple_multifunction(b,
- -1, true, "PIIX3");
+ -1, true, TYPE_PIIX3_DEVICE);
 piix3 = PIIX3_PCI_DEVICE(pci_dev);
 pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3,
 PIIX_NUM_PIRQS);
@@ -737,7 +740,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo piix3_info = {
-.name  = "PIIX3",
+.name  = TYPE_PIIX3_DEVICE,
 .parent= TYPE_PIIX3_PCI_DEVICE,
 .class_init= piix3_class_init,
 };
@@ -750,7 +753,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void 
*data)
 };
 
 static const TypeInfo piix3_xen_info = {
-.name  = "PIIX3-xen",
+.name  = TYPE_PIIX3_XEN_DEVICE,
 .parent= TYPE_PIIX3_PCI_DEVICE,
 .class_init= piix3_xen_class_init,
 };
-- 
2.11.0




[Qemu-devel] [PATCH] i440fx: use ARRAY_SIZE for pam_regions

2018-10-11 Thread Li Qiang
Cc: qemu-triv...@nongnu.org 

Signed-off-by: Li Qiang 
---
 hw/pci-host/piix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0e608347c1..230d5d2ea3 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -142,7 +142,7 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
 PCIDevice *pd = PCI_DEVICE(d);
 
 memory_region_transaction_begin();
-for (i = 0; i < 13; i++) {
+for (i = 0; i < ARRAY_SIZE(d->pam_regions); i++) {
 pam_update(>pam_regions[i], i,
pd->config[I440FX_PAM + (DIV_ROUND_UP(i, 2))]);
 }
@@ -401,7 +401,7 @@ PCIBus *i440fx_init(const char *host_type, const char 
*pci_type,
 
 init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
  >pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
-for (i = 0; i < 12; ++i) {
+for (i = 0; i < ARRAY_SIZE(f->pam_regions) - 1; ++i) {
 init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
  >pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
  PAM_EXPAN_SIZE);
-- 
2.11.0




[Qemu-devel] [PATCH] machine: fix a typo

2018-10-10 Thread Li Qiang
Cc: qemu-triv...@nongnu.org
Signed-off-by: Li Qiang 
---
 hw/core/machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1987557833..da50ad6de7 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -636,7 +636,7 @@ static void machine_class_init(ObjectClass *oc, void *data)
 machine_get_memory_encryption, machine_set_memory_encryption,
 _abort);
 object_class_property_set_description(oc, "memory-encryption",
-"Set memory encyption object to use", _abort);
+"Set memory encryption object to use", _abort);
 }
 
 static void machine_class_base_init(ObjectClass *oc, void *data)
-- 
2.17.1





Re: [Qemu-devel] Strange behavior abount bootindex

2018-10-10 Thread Li Qiang
Gerd Hoffmann  于2018年10月10日周三 下午4:37写道:

>   Hi,
>
> > However, when I launch a VM with two virtio-blck-pci device:
> > disk1.test, bootindex=1
> > disk1, bootindex=2
> >
> > I see the above picture alternately. Sometimes the hostname is
> "localhost",
> > sometimes the hostname is "nimlite-test". Seems the bootindex has no
> effect.
>
> I'd bet the root filesystem identity (uuid or lvm volume) is the same on
> both disk images.  So whe linux looks for the root filesystem it may
> pick the one or the other depending on which is initialized first.
>
>
Indeed, the uuid is the same. Thank you so much Gerd.

Li Qiang


> cheers,
>   Gerd
>
>


[Qemu-devel] [PATCH] ide: piix: convert constant device name to MACRO

2018-10-09 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/ide/piix.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index a3afe1f..5f29cce 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -35,6 +35,10 @@
 #include "hw/ide/pci.h"
 #include "trace.h"
 
+#define TYPE_PIIX3_IDE "piix3-ide"
+#define TYPE_PIIX3_IDE_XEN "piix3-ide-xen"
+#define TYPE_PIIX4_IDE "piix4-ide"
+
 static uint64_t bmdma_read(void *opaque, hwaddr addr, unsigned size)
 {
 BMDMAState *bm = opaque;
@@ -204,7 +208,7 @@ PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo 
**hd_table, int devfn)
 {
 PCIDevice *dev;
 
-dev = pci_create_simple(bus, devfn, "piix3-ide-xen");
+dev = pci_create_simple(bus, devfn, TYPE_PIIX3_IDE_XEN);
 pci_ide_create_devs(dev, hd_table);
 return dev;
 }
@@ -226,7 +230,7 @@ PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo 
**hd_table, int devfn)
 {
 PCIDevice *dev;
 
-dev = pci_create_simple(bus, devfn, "piix3-ide");
+dev = pci_create_simple(bus, devfn, TYPE_PIIX3_IDE);
 pci_ide_create_devs(dev, hd_table);
 return dev;
 }
@@ -237,7 +241,7 @@ PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo 
**hd_table, int devfn)
 {
 PCIDevice *dev;
 
-dev = pci_create_simple(bus, devfn, "piix4-ide");
+dev = pci_create_simple(bus, devfn, TYPE_PIIX4_IDE);
 pci_ide_create_devs(dev, hd_table);
 return dev;
 }
@@ -257,13 +261,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo piix3_ide_info = {
-.name  = "piix3-ide",
+.name  = TYPE_PIIX3_IDE,
 .parent= TYPE_PCI_IDE,
 .class_init= piix3_ide_class_init,
 };
 
 static const TypeInfo piix3_ide_xen_info = {
-.name  = "piix3-ide-xen",
+.name  = TYPE_PIIX3_IDE_XEN,
 .parent= TYPE_PCI_IDE,
 .class_init= piix3_ide_class_init,
 };
@@ -283,7 +287,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo piix4_ide_info = {
-.name  = "piix4-ide",
+.name  = TYPE_PIIX4_IDE,
 .parent= TYPE_PCI_IDE,
 .class_init= piix4_ide_class_init,
 };
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] memory.h: fix types in comments

2018-10-09 Thread Li Qiang
Peter Maydell  于2018年10月9日周二 下午6:26写道:

> On 9 October 2018 at 11:21, Li Qiang  wrote:
> > Signed-off-by: Li Qiang 
> > ---
> >  include/exec/memory.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
>
> Thanks. Contents of patch
> Reviewed-by: Peter Maydell 
>
> but the commit message should say "fix typos", not "fix types"
> (a 'typo' is a typing mistake; a 'type' is something like "uint32_t".)
>
>
Indeed, hope the maintainer can fix this typo when queuing this patch, :)

In additon, Cc: qemu-triv...@nongnu.org


>
> thanks
> -- PMM
>


[Qemu-devel] [PATCH] memory.h: fix types in comments

2018-10-09 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 include/exec/memory.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 3a427aa..1fbbdaf 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -935,7 +935,7 @@ uint64_t memory_region_size(MemoryRegion *mr);
 /**
  * memory_region_is_ram: check whether a memory region is random access
  *
- * Returns %true is a memory region is random access.
+ * Returns %true if a memory region is random access.
  *
  * @mr: the memory region being queried
  */
@@ -947,7 +947,7 @@ static inline bool memory_region_is_ram(MemoryRegion *mr)
 /**
  * memory_region_is_ram_device: check whether a memory region is a ram device
  *
- * Returns %true is a memory region is a device backed ram region
+ * Returns %true if a memory region is a device backed ram region
  *
  * @mr: the memory region being queried
  */
@@ -1161,7 +1161,7 @@ uint8_t memory_region_get_dirty_log_mask(MemoryRegion 
*mr);
 /**
  * memory_region_is_rom: check whether a memory region is ROM
  *
- * Returns %true is a memory region is read-only memory.
+ * Returns %true if a memory region is read-only memory.
  *
  * @mr: the memory region being queried
  */
-- 
1.8.3.1




[Qemu-devel] [PATCH] Rom: change Rom's 'isrom' to bool type

2018-10-09 Thread Li Qiang
Signed-off-by: Li Qiang 
---
 hw/core/loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc867..910f9a97a9 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -837,7 +837,7 @@ struct Rom {
 uint8_t *data;
 MemoryRegion *mr;
 AddressSpace *as;
-int isrom;
+bool isrom;
 char *fw_dir;
 char *fw_file;
 
-- 
2.11.0




Re: [Qemu-devel] [PATCH v2] vl.c: print error message if load fw_cfg file failed

2018-10-08 Thread Li Qiang
Hello  Philippe,

Philippe Mathieu-Daudé  于2018年10月9日周二 下午1:52写道:

> Hi Li,
>
> On 09/10/2018 04:39, Li Qiang wrote:
> > It makes sense to print the error message while reading
> > file failed.
>
> OK
>
> >
> > Change since v1:
> > free error
>
> Changes are useful for reviewer, but not in the git history.
> You can have them automatically stripped if you place them below the
> next '---' separator.
>
>
Thanks for your advice.


> Hopefully the maintainer taking this patch can clean this up.
>
> >
> > Signed-off-by: Li Qiang 
> > Reviewed-by: Philippe Mathieu-Daudé 
> > ---
>
> Here goes comment not worth to stay forever in git history:
>
>   Since v1: ...
>
> >  vl.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 4e25c78..69fc77c 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2207,8 +2207,10 @@ static int parse_fw_cfg(void *opaque, QemuOpts
> *opts, Error **errp)
> >  size = strlen(str); /* NUL terminator NOT included in fw_cfg
> blob */
> >  buf = g_memdup(str, size);
> >  } else {
> > -if (!g_file_get_contents(file, , , NULL)) {
> > -error_report("can't load %s", file);
> > +GError *error = NULL;
> > +if (!g_file_get_contents(file, , , )) {
> > +error_report("can't load %s, %s", file, error->message);
>
> If you ever respin, you can help Markus [1] effort using:
>
> error_setg(errp, "can't load %s, %s", file, error->message);
>
> Else your patch will clash with [2].
>

OK, I will send out this patch based Markus' tree or later when his patch
goes to upstream.

Thanks,
Li Qiang


>
> > +g_error_free(error);
> >  return -1;
> >  }
> >  }
> >
>
> Regards,
>
> Phil.
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg01394.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg01406.html
>


[Qemu-devel] [PATCH] i386: pc_sysfw: load bios using rom API

2018-10-08 Thread Li Qiang
As the bios is a ROM, just using rom API.
AFAICS no functionality changed.

Signed-off-by: Li Qiang 
---
 hw/i386/pc_sysfw.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 091e22dd60..7f469fd582 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -209,9 +209,9 @@ static void old_pc_system_rom_init(MemoryRegion 
*rom_memory, bool isapc_ram_fw)
 goto bios_error;
 }
 bios = g_malloc(sizeof(*bios));
-memory_region_init_ram(bios, NULL, "pc.bios", bios_size, _fatal);
-if (!isapc_ram_fw) {
-memory_region_set_readonly(bios, true);
+memory_region_init_rom(bios, NULL, "pc.bios", bios_size, _fatal);
+if (isapc_ram_fw) {
+memory_region_set_readonly(bios, false);
 }
 ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
 if (ret != 0) {
@@ -230,8 +230,8 @@ static void old_pc_system_rom_init(MemoryRegion 
*rom_memory, bool isapc_ram_fw)
 0x10 - isa_bios_size,
 isa_bios,
 1);
-if (!isapc_ram_fw) {
-memory_region_set_readonly(isa_bios, true);
+if (isapc_ram_fw) {
+memory_region_set_readonly(isa_bios, false);
 }
 
 /* map all the bios at the top of memory */
-- 
2.11.0




[Qemu-devel] [PATCH v2] vl.c: print error message if load fw_cfg file failed

2018-10-08 Thread Li Qiang
It makes sense to print the error message while reading
file failed.

Change since v1:
free error

Signed-off-by: Li Qiang 
Reviewed-by: Philippe Mathieu-Daud?? 
---
 vl.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 4e25c78..69fc77c 100644
--- a/vl.c
+++ b/vl.c
@@ -2207,8 +2207,10 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, 
Error **errp)
 size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
 buf = g_memdup(str, size);
 } else {
-if (!g_file_get_contents(file, , , NULL)) {
-error_report("can't load %s", file);
+GError *error = NULL;
+if (!g_file_get_contents(file, , , )) {
+error_report("can't load %s, %s", file, error->message);
+g_error_free(error);
 return -1;
 }
 }
-- 
1.8.3.1




[Qemu-devel] Strange behavior abount bootindex

2018-10-08 Thread Li Qiang
Hello Paolo, Lei, Gerd and all,


Recently I encounter a strange issue about bootindex.

I have two disk named "disk1.test" and "disk1".

When using disk1.test alone, the login picture is: (hostname is localhost)

[image: image.png]

When using disk1 alone, the login picture is:(hostname is nimlite-test)

[image: image.png]

However, when I launch a VM with two virtio-blck-pci device:
disk1.test, bootindex=1
disk1, bootindex=2

I see the above picture alternately. Sometimes the hostname is "localhost",
sometimes the hostname is "nimlite-test". Seems the bootindex has no effect.

I have see the qemu and seabios code, seems no obvious bug. There is no
'handle_18' called in seabios.

The disk1 is originated from disk1.test(CentOS). and when I repalce with
disk.test with a
Debian image, there seems no issue.

So I think there maybe a bug in qemu block layer or guest os?

Could anyone give some hints?

Thanks,
Li Qiang

The command I used is below:

qemu-devel/qemu/x86_64-softmmu/qemu-system-x86_64 -m 1024 -smp 2 -drive
file=disk1.test,format=raw,if=none,id=id1,cache=none -device
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x6,drive=id1,id=virtio1,bootindex=1
-vnc :100 --enable-kvm -drive
file=disk1,format=raw,if=none,id=id2,cache=none -device
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x7,drive=id2,bootindex=2 -boot
strict=on  -boot menu=on  -debugcon file:/home/liqiang02/qemu-devel/4.txt
-global isa-debugcon.iobase=0x402 -bios qemu-devel/seabios/out/bios.bin


[Qemu-devel] [PATCH] util: aio-posix: fix a typo

2018-10-07 Thread Li Qiang
Cc: qemu-triv...@nongnu.org
Signed-off-by: Li Qiang 
---
 util/aio-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index 621b302..51c41ed 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -40,7 +40,7 @@ struct AioHandler
 
 #ifdef CONFIG_EPOLL_CREATE1
 
-/* The fd number threashold to switch to epoll */
+/* The fd number threshold to switch to epoll */
 #define EPOLL_ENABLE_THRESHOLD 64
 
 static void aio_epoll_disable(AioContext *ctx)
-- 
1.8.3.1




[Qemu-devel] [PATCH] vl.c: print error message if load fw_cfg file failed

2018-10-06 Thread Li Qiang
It makes sense to print the error message while reading
file failed.

Signed-off-by: Li Qiang 
---
 vl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index cc55fe04a2..3db410e771 100644
--- a/vl.c
+++ b/vl.c
@@ -2207,8 +2207,9 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, 
Error **errp)
 size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
 buf = g_memdup(str, size);
 } else {
-if (!g_file_get_contents(file, , , NULL)) {
-error_report("can't load %s", file);
+GError *error = NULL;
+if (!g_file_get_contents(file, , , )) {
+error_report("can't load %s, %s", file, error->message);
 return -1;
 }
 }
-- 
2.17.1





[Qemu-devel] [PATCH] target/i386: kvm: just return after migrate_add_blocker failed

2018-10-06 Thread Li Qiang
When migrate_add_blocker failed, the invtsc_mig_blocker is not
appended so no need to remove. This can save several instructions.

Signed-off-by: Li Qiang 
---
 target/i386/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 0b2a07d3a4..6ba84a39f3 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1153,7 +1153,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 if (local_err) {
 error_report_err(local_err);
 error_free(invtsc_mig_blocker);
-goto fail;
+return r;
 }
 /* for savevm */
 vmstate_x86_cpu.unmigratable = 1;
-- 
2.17.1





[Qemu-devel] [PATCH] tests: check-qjson: fix a memory leak

2018-10-02 Thread Li Qiang
Spotted by ASAN.

=
==17531==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1120 byte(s) in 28 object(s) allocated from:
#0 0x7f9fb85eeb50 in __interceptor_malloc 
(/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
#1 0x7f9fb824b8d8 in g_malloc 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x518d8)
#2 0x5654ae94f073 in qstring_from_str qobject/qstring.c:66
#3 0x5654ae94ede7 in qstring_new qobject/qstring.c:23
#4 0x5654ae953b6b in parse_string qobject/json-parser.c:143
#5 0x5654ae955e3e in parse_literal qobject/json-parser.c:484
#6 0x5654ae956227 in parse_value qobject/json-parser.c:547
#7 0x5654ae9565c7 in json_parser_parse qobject/json-parser.c:573
#8 0x5654ae952d5d in json_message_process_token qobject/json-streamer.c:92
#9 0x5654ae9608fa in json_lexer_feed_char qobject/json-lexer.c:313
#10 0x5654ae960ea5 in json_lexer_feed qobject/json-lexer.c:350
#11 0x5654ae9530de in json_message_parser_feed qobject/json-streamer.c:121
#12 0x5654ae950b6e in qobject_from_jsonv qobject/qjson.c:69
#13 0x5654ae950d30 in qobject_from_json qobject/qjson.c:83
#14 0x5654ae9449af in from_json_str tests/check-qjson.c:30
#15 0x5654ae946399 in utf8_string tests/check-qjson.c:781
#16 0x7f9fb826cc39  (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x72c39)

Signed-off-by: Li Qiang 
---
 tests/check-qjson.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index cc13f3d41e..d876a7a96e 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -780,6 +780,7 @@ static void utf8_string(void)
 if (!strstr(json_out, "\\uFFFD")) {
 str = from_json_str(json_out, j, _abort);
 g_assert_cmpstr(qstring_get_try_str(str), ==, utf8_in);
+qobject_unref(str);
 }
 }
 }
-- 
2.17.1





[Qemu-devel] [PATCH] qom: fix comments for object_property_set_qobject function

2018-09-16 Thread Li Qiang
Also make the definition and declare of this function's argument name
the same.

Signed-off-by: Li Qiang 
---
 include/qom/qom-qobject.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qom/qom-qobject.h b/include/qom/qom-qobject.h
index 77cd717e3f..11803c6b57 100644
--- a/include/qom/qom-qobject.h
+++ b/include/qom/qom-qobject.h
@@ -30,13 +30,13 @@ struct QObject *object_property_get_qobject(Object *obj, 
const char *name,
 /**
  * object_property_set_qobject:
  * @obj: the object
- * @ret: The value that will be written to the property.
+ * @value: The value that will be written to the property.
  * @name: the name of the property
  * @errp: returns an error if this function fails
  *
  * Writes a property to a object.
  */
-void object_property_set_qobject(Object *obj, struct QObject *qobj,
+void object_property_set_qobject(Object *obj, struct QObject *value,
  const char *name, struct Error **errp);
 
 #endif
-- 
2.11.0




[Qemu-devel] [PATCH] hw: edu: replace device name with macro

2018-09-13 Thread Li Qiang
Just as other devices do.

Signed-off-by: Li Qiang 
---
 hw/misc/edu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index df26a4d046..0687ffd343 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -30,7 +30,8 @@
 #include "qemu/main-loop.h" /* iothread mutex */
 #include "qapi/visitor.h"
 
-#define EDU(obj)OBJECT_CHECK(EduState, obj, "edu")
+#define TYPE_PCI_EDU_DEVICE "edu"
+#define EDU(obj)OBJECT_CHECK(EduState, obj, TYPE_PCI_EDU_DEVICE)
 
 #define FACT_IRQ0x0001
 #define DMA_IRQ 0x0100
@@ -414,7 +415,7 @@ static void pci_edu_register_types(void)
 { },
 };
 static const TypeInfo edu_info = {
-.name  = "edu",
+.name  = TYPE_PCI_EDU_DEVICE,
 .parent= TYPE_PCI_DEVICE,
 .instance_size = sizeof(EduState),
 .instance_init = edu_instance_init,
-- 
2.11.0




Re: [Qemu-devel] [PATCH 5/8] hw: designware: add read memory region callback

2018-09-13 Thread Li Qiang
Paolo Bonzini  于2018年9月13日周四 下午11:12写道:

> On 12/09/2018 18:01, Li Qiang wrote:
> > From: Li Qiang 
> >
> > Signed-off-by: Li Qiang 
> > ---
> >  hw/pci-host/designware.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> > index 29ea313798..f5641b5c8c 100644
> > --- a/hw/pci-host/designware.c
> > +++ b/hw/pci-host/designware.c
> > @@ -57,6 +57,12 @@ designware_pcie_root_to_host(DesignwarePCIERoot *root)
> >  return DESIGNWARE_PCIE_HOST(bus->parent);
> >  }
> >
> > +static uint64_t designware_pcie_root_msi_read(void *opaque, hwaddr addr,
> > +  unsigned size)
> > +{
> > +return 0;
> > +}
> > +
> >  static void designware_pcie_root_msi_write(void *opaque, hwaddr addr,
> > uint64_t val, unsigned len)
> >  {
> > @@ -71,6 +77,7 @@ static void designware_pcie_root_msi_write(void
> *opaque, hwaddr addr,
> >  }
> >
> >  static const MemoryRegionOps designware_pci_host_msi_ops = {
> > +.read = designware_pcie_root_msi_read,
> >  .write = designware_pcie_root_msi_write,
> >  .endianness = DEVICE_LITTLE_ENDIAN,
> >  .valid = {
> >
>
> This probably needs to generate an unassigned access too, because the
> datasheet says that the device basically traps memory writes.
>
> Generating an unassigned access is probably a good idea for the memory
> core; devices can then override the behavior to return zero.  I'm
> queuing patches 1-4, with slightly expanded commit messages.
>
>
Hi Paolo,
Thanks for your review!

These patches has been got a deep discuss, I'm not sure these patches is
necessory.
The discussion can be found in this thread.
-->https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01391.html


Thanks,
Li Qiang


> Paolo
>


Re: [Qemu-devel] [PATCH 0/8] Add missed read callback for some memory region

2018-09-12 Thread Li Qiang
Philippe Mathieu-Daudé  于2018年9月13日周四 上午1:12写道:

> Hi Li,
>
> On 9/12/18 6:01 PM, Li Qiang wrote:
> > From: Li Qiang 
> >
> > This patch set try to add the missed read callback for memory region.
> > Without this patchset, when the guest reads the IO port/memory, it will
> > cause an NULL-dereference issue. For example, add
> > "-device isa-debug-exit" to command, then read the 0x501 port, it causes
> a
> > SIGSEGV.
> >
> > The only exception is 'readonly_mem_ops' as its read is directly
> > access the underlying host ram as the comments says.
> >
> > These missed read callback is mostly pointed by Laszlo Ersek.
> >
> >
> >
> > Li Qiang (8):
> >   fw_cfg_mem: add read memory region callback
> >   hw: debugexit: add read callback
> >   hw: hyperv_testdev: add read callback
> >   hw: pc-testdev: add read memory region callback
> >   hw: designware: add read memory region callback
> >   hw: pvrdma: add read memory region callback
> >   hw: sun4c: add read memory region callback
> >   exec: add read callback for notdirty memory region
>
> Why not rather simply add a check in
> memory_region_oldmmio_read_accessor() instead?
>
> Eventually:
>
> {
> uint64_t tmp;
> int idx = ctz32(size);
>
> if (unlikely(mr->ops->old_mmio.write[idx]
>  && !mr->ops->old_mmio.read[idx])) {
> tmp = 0; /* XXX is 0 the expected value??? */
> } else {
>     tmp = mr->ops->old_mmio.read[idx](mr->opaque, addr);
> }
> ...
>

Hi, I have sent this patch. But...


We have discussed in another thread:
-->https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01391.html

Thanks,
Li Qiang


Re: [Qemu-devel] [PATCH] memory region: check the old.mmio.read status

2018-09-12 Thread Li Qiang
Peter Maydell  于2018年9月13日周四 上午8:31写道:

> On 12 September 2018 at 18:43, Laszlo Ersek  wrote:
> > On 09/12/18 14:54, Peter Maydell wrote:
> >> There's patches on-list which drop the old_mmio field from the
> MemoryRegion
> >> struct entirely, so I think this patch as it stands is obsolete.
> >>
> >> Currently our semantics are "you must provide both read and write, even
> >> if one of them just always returns 0 / does nothing / returns an error".
> >
> > That's new to me. Has this always been the case?
>
> Pretty sure it has, yes, because the code assumes that if you can
> get a guest read then your MemoryRegion provides an accessor for it.
> If your guest never actually tries to do a read then of course we'll
> never notice...
>
> > There are several
> > static MemoryRegionOps structures that don't conform. (See the end of my
> > other email:
> > <
> 84da6f02-1f60-4bc7-92da-6a7f74deded3@redhat.com">http://mid.mail-archive.com/84da6f02-1f60-4bc7-92da-6a7f74deded3@redhat.com
> >.)
> > Beyond the one that Li Qiang reported directly ("fw_cfg_ctl_mem_read").
> >
> > Are all of those ops guest-triggerable QEMU crashers?
>
> Some of them are special cases like the notdirty-memory one where
> reads always go to host RAM rather than taking the slow path via
> the io accessor. But others are probably guest crashers.
>
> >> We could probably reasonably assert this at the point when the
> >> MemoryRegionOps is registered.
> >
> > Apparently, we should have...
>
> Yeah. Or we could define a default for if there's no read function,
> which I guess should be the same as what we do if
> memory_region_access_valid() fails.

If we want that then the
> simplest thing is for memory_region_access_valid() itself to
> check that at least one of the accessor functions exists and
> return false if none do.


I thinks this proposal makes sense as every memory region write/read will
go to this path and also the device code can make no change.

Thanks,
Li Qiang

(But as I mention above we should get
> all the "old_mmio is going away" patches in first and base the
> change on that, or there'll be a conflict.)
>
>



> thanks
> -- PMM
>


[Qemu-devel] [PATCH 5/8] hw: designware: add read memory region callback

2018-09-12 Thread Li Qiang
From: Li Qiang 

Signed-off-by: Li Qiang 
---
 hw/pci-host/designware.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 29ea313798..f5641b5c8c 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -57,6 +57,12 @@ designware_pcie_root_to_host(DesignwarePCIERoot *root)
 return DESIGNWARE_PCIE_HOST(bus->parent);
 }
 
+static uint64_t designware_pcie_root_msi_read(void *opaque, hwaddr addr,
+  unsigned size)
+{
+return 0;
+}
+
 static void designware_pcie_root_msi_write(void *opaque, hwaddr addr,
uint64_t val, unsigned len)
 {
@@ -71,6 +77,7 @@ static void designware_pcie_root_msi_write(void *opaque, 
hwaddr addr,
 }
 
 static const MemoryRegionOps designware_pci_host_msi_ops = {
+.read = designware_pcie_root_msi_read,
 .write = designware_pcie_root_msi_write,
 .endianness = DEVICE_LITTLE_ENDIAN,
 .valid = {
-- 
2.17.1





[Qemu-devel] [PATCH 6/8] hw: pvrdma: add read memory region callback

2018-09-12 Thread Li Qiang
From: Li Qiang 

Signed-off-by: Li Qiang 
---
 hw/rdma/vmw/pvrdma_main.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index ca5fa8d981..a6211d416d 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -455,6 +455,11 @@ static const MemoryRegionOps regs_ops = {
 },
 };
 
+static uint64_t uar_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
 static void uar_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
 {
 PVRDMADev *dev = opaque;
@@ -496,6 +501,7 @@ static void uar_write(void *opaque, hwaddr addr, uint64_t 
val, unsigned size)
 }
 
 static const MemoryRegionOps uar_ops = {
+.read = uar_read,
 .write = uar_write,
 .endianness = DEVICE_LITTLE_ENDIAN,
 .impl = {
-- 
2.17.1





[Qemu-devel] [PATCH 3/8] hw: hyperv_testdev: add read callback

2018-09-12 Thread Li Qiang
From: Li Qiang 

Signed-off-by: Li Qiang 
---
 hw/misc/hyperv_testdev.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/misc/hyperv_testdev.c b/hw/misc/hyperv_testdev.c
index bf6bbfa8cf..7549f470b1 100644
--- a/hw/misc/hyperv_testdev.c
+++ b/hw/misc/hyperv_testdev.c
@@ -105,7 +105,12 @@ static void hv_synic_test_dev_control(HypervTestDev *dev, 
uint32_t ctl,
 }
 }
 
-static void hv_test_dev_control(void *opaque, hwaddr addr, uint64_t data,
+static uint64_t hv_test_dev_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
+static void hv_test_dev_write(void *opaque, hwaddr addr, uint64_t data,
 uint32_t len)
 {
 HypervTestDev *dev = HYPERV_TEST_DEV(opaque);
@@ -127,7 +132,8 @@ static void hv_test_dev_control(void *opaque, hwaddr addr, 
uint64_t data,
 }
 
 static const MemoryRegionOps synic_test_sint_ops = {
-.write = hv_test_dev_control,
+.read = hv_test_dev_read,
+.write = hv_test_dev_write,
 .valid.min_access_size = 4,
 .valid.max_access_size = 4,
 .endianness = DEVICE_LITTLE_ENDIAN,
-- 
2.17.1





[Qemu-devel] [PATCH 7/8] hw: sun4c: add read memory region callback

2018-09-12 Thread Li Qiang
From: Li Qiang 

Signed-off-by: Li Qiang 
---
 hw/sparc64/sun4u.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d16843b30e..74c55a82f4 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -212,6 +212,11 @@ typedef struct PowerDevice {
 MemoryRegion power_mmio;
 } PowerDevice;
 
+static uint64_t power_mem_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
 /* Power */
 static void power_mem_write(void *opaque, hwaddr addr,
 uint64_t val, unsigned size)
@@ -223,6 +228,7 @@ static void power_mem_write(void *opaque, hwaddr addr,
 }
 
 static const MemoryRegionOps power_mem_ops = {
+.read = power_mem_read,
 .write = power_mem_write,
 .endianness = DEVICE_NATIVE_ENDIAN,
 .valid = {
-- 
2.17.1





[Qemu-devel] [PATCH 8/8] exec: add read callback for notdirty memory region

2018-09-12 Thread Li Qiang
From: Li Qiang 

Signed-off-by: Li Qiang 
---
 exec.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/exec.c b/exec.c
index 6826c8337d..3cd5ad2cae 100644
--- a/exec.c
+++ b/exec.c
@@ -2681,6 +2681,11 @@ void memory_notdirty_write_complete(NotDirtyInfo *ndi)
 }
 }
 
+static uint64_t notdirty_mem_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
 /* Called within RCU critical section.  */
 static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
uint64_t val, unsigned size)
@@ -2702,6 +2707,7 @@ static bool notdirty_mem_accepts(void *opaque, hwaddr 
addr,
 }
 
 static const MemoryRegionOps notdirty_mem_ops = {
+.read = notdirty_mem_read,
 .write = notdirty_mem_write,
 .valid.accepts = notdirty_mem_accepts,
 .endianness = DEVICE_NATIVE_ENDIAN,
@@ -2965,6 +2971,7 @@ static uint16_t dummy_section(PhysPageMap *map, FlatView 
*fv, MemoryRegion *mr)
 return phys_section_add(map, );
 }
 
+
 static void readonly_mem_write(void *opaque, hwaddr addr,
uint64_t val, unsigned size)
 {
-- 
2.17.1





[Qemu-devel] [PATCH 1/8] fw_cfg_mem: add read memory region callback

2018-09-12 Thread Li Qiang
From: Li Qiang 

Signed-off-by: Li Qiang 
---
 hw/nvram/fw_cfg.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index d79a568f54..6de7809f1a 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -434,6 +434,11 @@ static bool fw_cfg_data_mem_valid(void *opaque, hwaddr 
addr,
 return addr == 0;
 }
 
+static uint64_t fw_cfg_ctl_mem_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
 static void fw_cfg_ctl_mem_write(void *opaque, hwaddr addr,
  uint64_t value, unsigned size)
 {
@@ -468,6 +473,7 @@ static bool fw_cfg_comb_valid(void *opaque, hwaddr addr,
 }
 
 static const MemoryRegionOps fw_cfg_ctl_mem_ops = {
+.read = fw_cfg_ctl_mem_read,
 .write = fw_cfg_ctl_mem_write,
 .endianness = DEVICE_BIG_ENDIAN,
 .valid.accepts = fw_cfg_ctl_mem_valid,
-- 
2.17.1





[Qemu-devel] [PATCH 4/8] hw: pc-testdev: add read memory region callback

2018-09-12 Thread Li Qiang
From: Li Qiang 

Also change the write callback name.

Signed-off-by: Li Qiang 
---
 hw/misc/pc-testdev.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/misc/pc-testdev.c b/hw/misc/pc-testdev.c
index b81d820084..697eb88c97 100644
--- a/hw/misc/pc-testdev.c
+++ b/hw/misc/pc-testdev.c
@@ -58,7 +58,12 @@ typedef struct PCTestdev {
 #define TESTDEV(obj) \
  OBJECT_CHECK(PCTestdev, (obj), TYPE_TESTDEV)
 
-static void test_irq_line(void *opaque, hwaddr addr, uint64_t data,
+static uint64_t test_irq_line_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
+static void test_irq_line_write(void *opaque, hwaddr addr, uint64_t data,
   unsigned len)
 {
 PCTestdev *dev = opaque;
@@ -68,7 +73,8 @@ static void test_irq_line(void *opaque, hwaddr addr, uint64_t 
data,
 }
 
 static const MemoryRegionOps test_irq_ops = {
-.write = test_irq_line,
+.read = test_irq_line_read,
+.write = test_irq_line_write,
 .valid.min_access_size = 1,
 .valid.max_access_size = 1,
 .endianness = DEVICE_LITTLE_ENDIAN,
@@ -110,7 +116,12 @@ static const MemoryRegionOps test_ioport_byte_ops = {
 .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static void test_flush_page(void *opaque, hwaddr addr, uint64_t data,
+static uint64_t test_flush_page_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
+static void test_flush_page_write(void *opaque, hwaddr addr, uint64_t data,
 unsigned len)
 {
 hwaddr page = 4096;
@@ -126,7 +137,8 @@ static void test_flush_page(void *opaque, hwaddr addr, 
uint64_t data,
 }
 
 static const MemoryRegionOps test_flush_ops = {
-.write = test_flush_page,
+.read = test_flush_page_read,
+.write = test_flush_page_write,
 .valid.min_access_size = 4,
 .valid.max_access_size = 4,
 .endianness = DEVICE_LITTLE_ENDIAN,
-- 
2.17.1





[Qemu-devel] [PATCH 2/8] hw: debugexit: add read callback

2018-09-12 Thread Li Qiang
From: Li Qiang 

Signed-off-by: Li Qiang 
---
 hw/misc/debugexit.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/misc/debugexit.c b/hw/misc/debugexit.c
index 84fa1a5b9d..bed293247e 100644
--- a/hw/misc/debugexit.c
+++ b/hw/misc/debugexit.c
@@ -23,6 +23,11 @@ typedef struct ISADebugExitState {
 MemoryRegion io;
 } ISADebugExitState;
 
+static uint64_t debug_exit_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
 static void debug_exit_write(void *opaque, hwaddr addr, uint64_t val,
  unsigned width)
 {
@@ -30,6 +35,7 @@ static void debug_exit_write(void *opaque, hwaddr addr, 
uint64_t val,
 }
 
 static const MemoryRegionOps debug_exit_ops = {
+.read = debug_exit_read,
 .write = debug_exit_write,
 .valid.min_access_size = 1,
 .valid.max_access_size = 4,
-- 
2.17.1





[Qemu-devel] [PATCH 0/8] Add missed read callback for some memory region

2018-09-12 Thread Li Qiang
From: Li Qiang 

This patch set try to add the missed read callback for memory region.
Without this patchset, when the guest reads the IO port/memory, it will
cause an NULL-dereference issue. For example, add 
"-device isa-debug-exit" to command, then read the 0x501 port, it causes a 
SIGSEGV.

The only exception is 'readonly_mem_ops' as its read is directly 
access the underlying host ram as the comments says.

These missed read callback is mostly pointed by Laszlo Ersek.



Li Qiang (8):
  fw_cfg_mem: add read memory region callback
  hw: debugexit: add read callback
  hw: hyperv_testdev: add read callback
  hw: pc-testdev: add read memory region callback
  hw: designware: add read memory region callback
  hw: pvrdma: add read memory region callback
  hw: sun4c: add read memory region callback
  exec: add read callback for notdirty memory region

 exec.c|  7 +++
 hw/misc/debugexit.c   |  6 ++
 hw/misc/hyperv_testdev.c  | 10 --
 hw/misc/pc-testdev.c  | 20 
 hw/nvram/fw_cfg.c |  6 ++
 hw/pci-host/designware.c  |  7 +++
 hw/rdma/vmw/pvrdma_main.c |  6 ++
 hw/sparc64/sun4u.c|  6 ++
 8 files changed, 62 insertions(+), 6 deletions(-)

-- 
2.17.1





Re: [Qemu-devel] [PATCH] memory region: check the old.mmio.read status

2018-09-12 Thread Li Qiang
Peter Maydell  于2018年9月12日周三 下午8:55写道:

> On 12 September 2018 at 13:32, Li Qiang  wrote:
> > To avoid NULL-deref for the devices without read callbacks
> >
> > Signed-off-by: Li Qiang 
> > ---
> >  memory.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/memory.c b/memory.c
> > index 9b73892768..48d025426b 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -406,6 +406,10 @@ static MemTxResult
> memory_region_oldmmio_read_accessor(MemoryRegion *mr,
> >  {
> >  uint64_t tmp;
> >
> > +if (!mr->ops->old_mmio.read[ctz32(size)]) {
> > +return MEMTX_DECODE_ERROR;
> > +}
> > +
> >  tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
> >  if (mr->subpage) {
> >  trace_memory_region_subpage_read(get_cpu_index(), mr, addr,
> tmp, size);
> > --
> > 2.11.0
> >
>
> There's patches on-list which drop the old_mmio field from the MemoryRegion
> struct entirely, so I think this patch as it stands is obsolete.
>
> Currently our semantics are "you must provide both read and write, even
> if one of them just always returns 0 / does nothing / returns an error".
> We could probably reasonably assert this at the point when the
> MemoryRegionOps is registered.
>

This patch is sent as the results of this thread:
-->https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01332.html

So I think  I should send a path set to add all the missing read function
as Laszlo Ersek points
in the above thread discussion, right?

Thanks,
Li Qiang




>
> thanks
> -- PMM
>


Re: [Qemu-devel] [PATCH] fw_cfg_mem: add read memory region callback

2018-09-12 Thread Li Qiang
Hi Laszlo,

Laszlo Ersek  于2018年9月12日周三 下午6:36写道:

> On 09/12/18 10:02, Li Qiang wrote:
> > Hi,
> >
> > Marc-André Lureau  于2018年9月12日周三 下午3:16写道:
> >
> >> Hi
> >>
> >> On Wed, Sep 12, 2018 at 9:22 AM Li Qiang  wrote:
> >>>
> >>> The write/read should be paired, this can avoid the
> >>> NULL-deref while the guest reads the fw_cfg port.
> >>>
> >>> Signed-off-by: Li Qiang 
> >>
> >> Do you have a reproducer and/or a backtrace?
> >> memory_region_dispatch_write() checks if ops->write != NULL.
> >>
> >
> > As far as I can see, the fw_cfg_mem is not used in x86 and used in arm.
> > And from my impression, this will NULL read will be a issue. So I just
> > omit the 'read' field in fw_cfg_comb_mem_ops(this is used for x86) to
> > emulate the
> > issue.
> >
> > When using gdb, I got the following backtrack.
> >
> > Thread 5 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 0x7fffca15b700 (LWP 7637)]
> > 0x in ?? ()
> > (gdb) bt
> > #0  0x in ?? ()
> > #1  0x55872492 in memory_region_oldmmio_read_accessor
> > (mr=0x567fa870, addr=1, value=0x7fffca158510, size=1, shift=0,
> > mask=255, attrs=...) at /home/liqiang02/qemu-devel/qemu/memory.c:409
> > #2  0x5586f2dd in access_with_adjusted_size (addr=addr@entry=1,
> > value=value@entry=0x7fffca158510, size=size@entry=1,
> > access_size_min=, access_size_max=,
> > access_fn=0x55872440 ,
> > mr=0x567fa870, attrs=...) at
> > /home/liqiang02/qemu-devel/qemu/memory.c:593
> > #3  0x55873e90 in memory_region_dispatch_read1 (attrs=...,
> size=1,
> > pval=0x7fffca158510, addr=1, mr=0x567fa870) at
> > /home/liqiang02/qemu-devel/qemu/memory.c:1404
> > #4  memory_region_dispatch_read (mr=mr@entry=0x567fa870, addr=1,
> > pval=pval@entry=0x7fffca158510, size=1, attrs=attrs@entry=...) at
> > /home/liqiang02/qemu-devel/qemu/memory.c:1423
> > #5  0x55821e42 in flatview_read_continue (fv=fv@entry
> =0x7fffbc03f370,
> > addr=addr@entry=1297, attrs=..., buf=, buf@entry
> =0x77fee000
> > "", len=len@entry=1, addr1=, l=,
> > mr=0x567fa870)
> > at /home/liqiang02/qemu-devel/qemu/exec.c:3293
> > #6  0x55822006 in flatview_read (fv=0x7fffbc03f370, addr=1297,
> > attrs=..., buf=0x77fee000 "", len=1) at
> > /home/liqiang02/qemu-devel/qemu/exec.c:3331
> > #7  0x5582211f in address_space_read_full (as=,
> > addr=addr@entry=1297, attrs=..., buf=buf@entry=0x77fee000 "",
> > len=len@entry=1) at /home/liqiang02/qemu-devel/qemu/exec.c:3344
> > #8  0x5582225a in address_space_rw (as=,
> > addr=addr@entry=1297, attrs=..., attrs@entry=..., buf=buf@entry
> =0x77fee000
> > "", len=len@entry=1, is_write=is_write@entry=false)
> > at /home/liqiang02/qemu-devel/qemu/exec.c:3374
> > #9  0x55886239 in kvm_handle_io (count=1, size=1,
> > direction=, data=, attrs=..., port=1297) at
> > /home/liqiang02/qemu-devel/qemu/accel/kvm/kvm-all.c:1731
> > #10 kvm_cpu_exec (cpu=cpu@entry=0x566e9990) at
> > /home/liqiang02/qemu-devel/qemu/accel/kvm/kvm-all.c:1971
> > #11 0x5585d3de in qemu_kvm_cpu_thread_fn (arg=0x566e9990) at
> > /home/liqiang02/qemu-devel/qemu/cpus.c:1257
> > #12 0x7fffdbd58494 in start_thread () from
> > /lib/x86_64-linux-gnu/libpthread.so.0
> > #13 0x7fffdba9aacf in clone () from /lib/x86_64-linux-gnu/libc.so.6
> >
> > So I send out this path.
> >
> > But this time when I not use gdb, there is no segmentation.
> >
> > So this may not a issue.
> >
> > If who has a arm environment, he can try this, the PoC is just easy. just
> > read the 0x510 port with word.
>
> I don't understand your description of how you managed to trigger this
> SIGSEGV.
>
> FWIW, looking at the codebase, there's a good number of static
> MemoryRegionOps structures for which the "read_with_attrs" and "read"
> members are default-initialized to NULL. It seems unlikely they are all
> wrong.
>
>
I uses the debugexit.

QEMU command: gdb --args qemu-system-x86_64  -m 2048 -hda
/home/liqiang02/ubuntu1801.img -enable-kvm  -vnc :100 -device isa-debug-exit

guest: inw(0x501)

We can get the following backtrack.

Starting program: /usr/local/bin/qemu-system-x86_64 -m 2048 -hda
/home/liqiang02/ubuntu1801.img -enable-kvm -vnc :100 -device isa-debug-exit
[Thread debugging using libthread_db enabled]
Usin

[Qemu-devel] [PATCH] memory region: check the old.mmio.read status

2018-09-12 Thread Li Qiang
To avoid NULL-deref for the devices without read callbacks

Signed-off-by: Li Qiang 
---
 memory.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/memory.c b/memory.c
index 9b73892768..48d025426b 100644
--- a/memory.c
+++ b/memory.c
@@ -406,6 +406,10 @@ static MemTxResult 
memory_region_oldmmio_read_accessor(MemoryRegion *mr,
 {
 uint64_t tmp;
 
+if (!mr->ops->old_mmio.read[ctz32(size)]) {
+return MEMTX_DECODE_ERROR;
+}
+
 tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
 if (mr->subpage) {
 trace_memory_region_subpage_read(get_cpu_index(), mr, addr, tmp, size);
-- 
2.11.0




Re: [Qemu-devel] [PATCH] fw_cfg_mem: add read memory region callback

2018-09-12 Thread Li Qiang
Hi,

Marc-André Lureau  于2018年9月12日周三 下午3:16写道:

> Hi
>
> On Wed, Sep 12, 2018 at 9:22 AM Li Qiang  wrote:
> >
> > The write/read should be paired, this can avoid the
> > NULL-deref while the guest reads the fw_cfg port.
> >
> > Signed-off-by: Li Qiang 
>
> Do you have a reproducer and/or a backtrace?
> memory_region_dispatch_write() checks if ops->write != NULL.
>

As far as I can see, the fw_cfg_mem is not used in x86 and used in arm.
And from my impression, this will NULL read will be a issue. So I just
omit the 'read' field in fw_cfg_comb_mem_ops(this is used for x86) to
emulate the
issue.

When using gdb, I got the following backtrack.

Thread 5 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffca15b700 (LWP 7637)]
0x in ?? ()
(gdb) bt
#0  0x in ?? ()
#1  0x55872492 in memory_region_oldmmio_read_accessor
(mr=0x567fa870, addr=1, value=0x7fffca158510, size=1, shift=0,
mask=255, attrs=...) at /home/liqiang02/qemu-devel/qemu/memory.c:409
#2  0x5586f2dd in access_with_adjusted_size (addr=addr@entry=1,
value=value@entry=0x7fffca158510, size=size@entry=1,
access_size_min=, access_size_max=,
access_fn=0x55872440 ,
mr=0x567fa870, attrs=...) at
/home/liqiang02/qemu-devel/qemu/memory.c:593
#3  0x55873e90 in memory_region_dispatch_read1 (attrs=..., size=1,
pval=0x7fffca158510, addr=1, mr=0x567fa870) at
/home/liqiang02/qemu-devel/qemu/memory.c:1404
#4  memory_region_dispatch_read (mr=mr@entry=0x567fa870, addr=1,
pval=pval@entry=0x7fffca158510, size=1, attrs=attrs@entry=...) at
/home/liqiang02/qemu-devel/qemu/memory.c:1423
#5  0x55821e42 in flatview_read_continue (fv=fv@entry=0x7fffbc03f370,
addr=addr@entry=1297, attrs=..., buf=, buf@entry=0x77fee000
"", len=len@entry=1, addr1=, l=,
mr=0x567fa870)
at /home/liqiang02/qemu-devel/qemu/exec.c:3293
#6  0x55822006 in flatview_read (fv=0x7fffbc03f370, addr=1297,
attrs=..., buf=0x77fee000 "", len=1) at
/home/liqiang02/qemu-devel/qemu/exec.c:3331
#7  0x5582211f in address_space_read_full (as=,
addr=addr@entry=1297, attrs=..., buf=buf@entry=0x77fee000 "",
len=len@entry=1) at /home/liqiang02/qemu-devel/qemu/exec.c:3344
#8  0x5582225a in address_space_rw (as=,
addr=addr@entry=1297, attrs=..., attrs@entry=..., buf=buf@entry=0x77fee000
"", len=len@entry=1, is_write=is_write@entry=false)
at /home/liqiang02/qemu-devel/qemu/exec.c:3374
#9  0x55886239 in kvm_handle_io (count=1, size=1,
direction=, data=, attrs=..., port=1297) at
/home/liqiang02/qemu-devel/qemu/accel/kvm/kvm-all.c:1731
#10 kvm_cpu_exec (cpu=cpu@entry=0x566e9990) at
/home/liqiang02/qemu-devel/qemu/accel/kvm/kvm-all.c:1971
#11 0x5585d3de in qemu_kvm_cpu_thread_fn (arg=0x566e9990) at
/home/liqiang02/qemu-devel/qemu/cpus.c:1257
#12 0x7fffdbd58494 in start_thread () from
/lib/x86_64-linux-gnu/libpthread.so.0
#13 0x7fffdba9aacf in clone () from /lib/x86_64-linux-gnu/libc.so.6

So I send out this path.

But this time when I not use gdb, there is no segmentation.

So this may not a issue.

If who has a arm environment, he can try this, the PoC is just easy. just
read the 0x510 port with word.

Thanks,
Li Qiang


>
> > ---
> >  hw/nvram/fw_cfg.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> > index d79a568f54..6de7809f1a 100644
> > --- a/hw/nvram/fw_cfg.c
> > +++ b/hw/nvram/fw_cfg.c
> > @@ -434,6 +434,11 @@ static bool fw_cfg_data_mem_valid(void *opaque,
> hwaddr addr,
> >  return addr == 0;
> >  }
> >
> > +static uint64_t fw_cfg_ctl_mem_read(void *opaque, hwaddr addr, unsigned
> size)
> > +{
> > +return 0;
> > +}
> > +
> >  static void fw_cfg_ctl_mem_write(void *opaque, hwaddr addr,
> >   uint64_t value, unsigned size)
> >  {
> > @@ -468,6 +473,7 @@ static bool fw_cfg_comb_valid(void *opaque, hwaddr
> addr,
> >  }
> >
> >  static const MemoryRegionOps fw_cfg_ctl_mem_ops = {
> > +.read = fw_cfg_ctl_mem_read,
> >  .write = fw_cfg_ctl_mem_write,
> >  .endianness = DEVICE_BIG_ENDIAN,
> >  .valid.accepts = fw_cfg_ctl_mem_valid,
> > --
> > 2.11.0
> >
> >
>
>
> --
> Marc-André Lureau
>


[Qemu-devel] [PATCH] fw_cfg_mem: add read memory region callback

2018-09-11 Thread Li Qiang
The write/read should be paired, this can avoid the
NULL-deref while the guest reads the fw_cfg port.

Signed-off-by: Li Qiang 
---
 hw/nvram/fw_cfg.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index d79a568f54..6de7809f1a 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -434,6 +434,11 @@ static bool fw_cfg_data_mem_valid(void *opaque, hwaddr 
addr,
 return addr == 0;
 }
 
+static uint64_t fw_cfg_ctl_mem_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
 static void fw_cfg_ctl_mem_write(void *opaque, hwaddr addr,
  uint64_t value, unsigned size)
 {
@@ -468,6 +473,7 @@ static bool fw_cfg_comb_valid(void *opaque, hwaddr addr,
 }
 
 static const MemoryRegionOps fw_cfg_ctl_mem_ops = {
+.read = fw_cfg_ctl_mem_read,
 .write = fw_cfg_ctl_mem_write,
 .endianness = DEVICE_BIG_ENDIAN,
 .valid.accepts = fw_cfg_ctl_mem_valid,
-- 
2.11.0




[Qemu-devel] [Question] Question about ACPI table in qemu

2018-09-11 Thread Li Qiang
Hi all,

I noticed that both qemu and seabios create the ACPI table.
Once I think he bios' ACPI table will overwrite the qemu's if seabios
compiled with CONFIG_ACPI.

But after I read this -->
https://lists.gnu.org/archive/html/qemu-devel/2013-02/msg04555.html

There say:

"just have QEMU pass the tables to SeaBIOS for it to copy
>>> into memory like it does on CSM, coreboot, and Xen"

So where does this 'copy' happens? or is there something others I missed?

Thanks,
Li Qiang


Re: [Qemu-devel] [Question] Question about the i440FX device

2018-09-09 Thread Li Qiang
Thanks Paolo,

Paolo Bonzini  于2018年9月10日周一 上午7:11写道:

> On 07/09/2018 08:32, Li Qiang wrote:
> > Hello all,
> >
> > I want to know why the i440FX in the following 'info qtree' information
> is
> > laid under the pci.0 bus.  In the chip spec here:
> > -->https://wiki.qemu.org/images/b/bb/29054901.pdf
> > I don't see this device.
> >
> > Can anyone give me some hints?
>
> Hi,
>
> the device implements what is in "3.2. PCI Configuration Space Mapped
> Registers" in the i440FX spec.
>
>
Indeed, also I find the comments in 'i440fx_class_init' is useful.
As I got the 'PCII440FXState' is for the "PCI-facing part of the host
bridge"
and 'I440FXState' is for the "host-facing part".

Thanks,
Li Qiang



> Paolo
>


[Qemu-devel] [Question] Question about the i440FX device

2018-09-07 Thread Li Qiang
Hello all,

I want to know why the i440FX in the following 'info qtree' information is
laid under the pci.0 bus.  In the chip spec here:
-->https://wiki.qemu.org/images/b/bb/29054901.pdf
I don't see this device.

Can anyone give me some hints?

Thanks,
Li Qiang

bus: main-system-bus
  type System
  dev: hpet, id ""
  dev: kvm-ioapic, id ""
  dev: i440FX-pcihost, id ""
pci-hole64-size = 18446744073709551615 (16 EiB)
short_root_bus = 0 (0x0)
bus: pci.0
  type PCI
  dev: virtio-balloon-pci, id "balloon0"
  dev: cirrus-vga, id "video0"
  dev: PIIX4_PM, id "
  dev: piix3-ide, id ""

  dev: PIIX3, id ""

  dev: *i440FX*, id ""
addr = 00.0
romfile = ""
rombar = 1 (0x1)
multifunction = false
command_serr_enable = true
x-pcie-lnksta-dllla = true
class Host bridge, addr 00:00.0, pci id 8086:1237 (sub 1af4:1100)


[Qemu-devel] [PATCH] cpu.h: fix a typo in comment

2018-09-05 Thread Li Qiang
Found by reading the code.

Signed-off-by: Li Qiang 
---
 include/qom/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index dc130cd307..5bb94a9f86 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -852,7 +852,7 @@ extern CPUInterruptHandler cpu_interrupt_handler;
 /**
  * cpu_interrupt:
  * @cpu: The CPU to set an interrupt on.
- * @mask: The interupts to set.
+ * @mask: The interrupts to set.
  *
  * Invokes the interrupt handler.
  */
-- 
2.11.0




[Qemu-devel] [PATCH] qdev: fix a typo in comment

2018-09-05 Thread Li Qiang
Found by reading code.

Signed-off-by: Li Qiang 
---
 hw/core/qdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 529b82de18..7e4bfcb8f7 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -643,7 +643,7 @@ static void qdev_get_legacy_property(Object *obj, Visitor 
*v,
  * the string depends on the property type.  Legacy properties are only
  * needed for "info qtree".
  *
- * Do not use this is new code!  QOM Properties added through this interface
+ * Do not use this in new code!  QOM Properties added through this interface
  * will be given names in the "legacy" namespace.
  */
 static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
-- 
2.11.0




Re: [Qemu-devel] Question about dirty page statistics for live migration

2018-08-13 Thread Li Qiang
2018-08-13 19:30 GMT+08:00 Dr. David Alan Gilbert :

> * Li Qiang (liq...@gmail.com) wrote:
> > 2018-08-13 17:35 GMT+08:00 Dr. David Alan Gilbert :
> >
> > > * Li Qiang (liq...@gmail.com) wrote:
> > > > Hello Dave, Juan and all,
> > > >
> > > > It is useful to get the dirty page rates in guest to evaluate the
> guest
> > > > loads
> > > > so that we can make a decide to live migrate it or not. So I think
> we can
> > > > add a on-demand qmp for showing the dirty page rates.
> > > >
> > > > I found someone has done this work in here:
> > > > -->https://github.com/grivon/yabusame-qemu-dpt
> > > > and here:
> > > > https://github.com/btrplace/qemu-patch
> > > >
> > > > But seems not go to the upstream.
> > > >
> > > > I want to know your opinions about adding this qmp.
> > >
> > > Something like that could be good;
> >
> > one easy idea we had was
> > > a 'migrate null:' uri and then you would use most of the existing
> > > migration code to do the measurement; you would only have to
> > > add a dummy file backend,
> >
> >
> > As far as I understand, here dummy file backend just means the file
> migrate
> > to, right?
>
> Yes; because then if you have a dummy migration destination, the rest of
> the migration code will run, sync the bitmap and clear the bits, and
> give you an approximation of the dirty rate.
>
> It might still be worth having the separate code to measure it without
> the overhead of the migration code; but then that's more complex - and
> if you're trying to measure it to know how hard it is to migrate then
> perhaps it's better to use the migration code anyway.
>

Agree

But here is another issue, if we choice 'migrate null', then we will break
the 'migrate' interface.
the migrate doesn't return the info, so we need to uses 'info migrate'.
However only we want
is the dirty pages rates. It's too heavy for the user. Anyway we just want
to get the only dirty page rates
using one command.

I will first try to implement a separate qmp.

Thanks,
Li Qiang



> Dave
>
> > Thanks,
> > Li Qiang
> >
> >
> >
> > > and something to stop the migration ever
> > > terminating (maybe just set the downtime very low).
> > >
> > >
> > > Dave
> > >
> > >
> > > > Thanks,
> > > > Li Qiang
> > > --
> > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> > >
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>


Re: [Qemu-devel] Question about dirty page statistics for live migration

2018-08-13 Thread Li Qiang
2018-08-13 17:35 GMT+08:00 Dr. David Alan Gilbert :

> * Li Qiang (liq...@gmail.com) wrote:
> > Hello Dave, Juan and all,
> >
> > It is useful to get the dirty page rates in guest to evaluate the guest
> > loads
> > so that we can make a decide to live migrate it or not. So I think we can
> > add a on-demand qmp for showing the dirty page rates.
> >
> > I found someone has done this work in here:
> > -->https://github.com/grivon/yabusame-qemu-dpt
> > and here:
> > https://github.com/btrplace/qemu-patch
> >
> > But seems not go to the upstream.
> >
> > I want to know your opinions about adding this qmp.
>
> Something like that could be good;

one easy idea we had was
> a 'migrate null:' uri and then you would use most of the existing
> migration code to do the measurement; you would only have to
> add a dummy file backend,


As far as I understand, here dummy file backend just means the file migrate
to, right?

Thanks,
Li Qiang



> and something to stop the migration ever
> terminating (maybe just set the downtime very low).
>
>
> Dave
>
>
> > Thanks,
> > Li Qiang
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>


[Qemu-devel] Question about dirty page statistics for live migration

2018-08-12 Thread Li Qiang
Hello Dave, Juan and all,

It is useful to get the dirty page rates in guest to evaluate the guest
loads
so that we can make a decide to live migrate it or not. So I think we can
add a on-demand qmp for showing the dirty page rates.

I found someone has done this work in here:
-->https://github.com/grivon/yabusame-qemu-dpt
and here:
https://github.com/btrplace/qemu-patch

But seems not go to the upstream.

I want to know your opinions about adding this qmp.

Thanks,
Li Qiang


[Qemu-devel] [Bug 1785670] Re: Guest(ubuntu 18.04) crashes when trying uploading file

2018-08-07 Thread Li Qiang
Hi, 
 
I have find the overflow point using ASAN.
 
void
m_cat(struct mbuf *m, struct mbuf *n)
{
 /*
  * If there's no room, realloc
  */
 if (M_FREEROOM(m) < n->m_len)
  m_inc(m, m->m_len + n->m_len);
 
 memcpy(m->m_data+m->m_len, n->m_data, n->m_len);
 m->m_len += n->m_len;
 
 m_free(n);
}
 

/* make m 'size' bytes large from m_data */
void
m_inc(struct mbuf *m, int size)
{
int datasize;
 
/* some compilers throw up on gotos.  This one we can fake. */
if (m->m_size > size) {
return;
}
 
if (m->m_flags & M_EXT) {
datasize = m->m_data - m->m_ext;
m->m_ext = g_realloc(m->m_ext, size + datasize);
} else {
datasize = m->m_data - m->m_dat;
m->m_ext = g_malloc(size + datasize);
memcpy(m->m_ext, m->m_dat, m->m_size);
m->m_flags |= M_EXT;
}
 
m->m_data = m->m_ext + datasize;
m->m_size = size + datasize;
}
 
Here m_cat catenates two mbuf, when the first has no buffer, it allocates an 
M_EXT.
In m_inc, g_malloc called, then return m_cat, the next call to m_cat will 
trigger oob write.
 
Seems the m_len is too big.
In my debug, I see the m->m_len is 0x5b0, but datasize in m_inc is 0x40. Is 
this right?
 
Thanks,
Li Qiang
 
==17835==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x61f41dd0 at pc 0x76e9ad7b bp 0x7fffc6b215d0 sp 0x7fffc6b20d80
WRITE of size 28 at 0x61f41dd0 thread T4
#0 0x76e9ad7a  (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x5cd7a)
#1 0x5663fa71 in m_cat slirp/mbuf.c:143
#2 0x56632cdd in ip_reass slirp/ip_input.c:341
#3 0x56631609 in ip_input slirp/ip_input.c:190
#4 0x5663bd91 in slirp_input slirp/slirp.c:874
#5 0x56600d6f in net_slirp_receive net/slirp.c:121
#6 0x565e8192 in nc_sendv_compat net/net.c:701
#7 0x565e8322 in qemu_deliver_packet_iov net/net.c:728
#8 0x565edda2 in qemu_net_queue_deliver_iov net/queue.c:179
#9 0x565edfaa in qemu_net_queue_send_iov net/queue.c:224
#10 0x565e8547 in qemu_sendv_packet_async net/net.c:764
#11 0x565e8574 in qemu_sendv_packet net/net.c:772
#12 0x5636657c in net_tx_pkt_sendv hw/net/net_tx_pkt.c:546
#13 0x563668f3 in net_tx_pkt_do_sw_fragmentation hw/net/net_tx_pkt.c:588
#14 0x56366c93 in net_tx_pkt_send hw/net/net_tx_pkt.c:625
#15 0x5638586c in e1000e_tx_pkt_send hw/net/e1000e_core.c:665
#16 0x56385fca in e1000e_process_tx_desc hw/net/e1000e_core.c:742
#17 0x56387680 in e1000e_start_xmit hw/net/e1000e_core.c:933
#18 0x5638f390 in e1000e_set_tdt hw/net/e1000e_core.c:2450
#19 0x563911cb in e1000e_core_write hw/net/e1000e_core.c:3255
#20 0x56370524 in e1000e_mmio_write hw/net/e1000e.c:105
#21 0x55d4ec07 in memory_region_write_accessor 
/home/liqiang02/qemu-devel/qemu/memory.c:527
#22 0x55d4eee3 in access_with_adjusted_size 
/home/liqiang02/qemu-devel/qemu/memory.c:594
#23 0x55d54d16 in memory_region_dispatch_write 
/home/liqiang02/qemu-devel/qemu/memory.c:1473
#24 0x55c94b76 in flatview_write_continue 
/home/liqiang02/qemu-devel/qemu/exec.c:3255
#25 0x55c94da1 in flatview_write 
/home/liqiang02/qemu-devel/qemu/exec.c:3294
#26 0x55c95354 in address_space_write 
/home/liqiang02/qemu-devel/qemu/exec.c:3384
#27 0x55c953a5 in address_space_rw 
/home/liqiang02/qemu-devel/qemu/exec.c:3395
#28 0x55d92c4d in kvm_cpu_exec 
/home/liqiang02/qemu-devel/qemu/accel/kvm/kvm-all.c:1979
#29 0x55d18936 in qemu_kvm_cpu_thread_fn 
/home/liqiang02/qemu-devel/qemu/cpus.c:1215
#30 0x569afef1 in qemu_thread_start util/qemu-thread-posix.c:504
#31 0x7fffdadbd493 in start_thread 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x7493)
#32 0x7fffdaafface in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xe8ace)
 
AddressSanitizer can not describe address in more detail (wild memory access 
suspected).
SUMMARY: AddressSanitizer: heap-buffer-overflow 
(/usr/lib/x86_64-linux-gnu/libasan.so.3+0x5cd7a) 
Shadow bytes around the buggy address:
  0x0c3e8360: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3e8370: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3e8380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3e8390: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3e83a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c3e83b0: fa fa fa fa fa fa fa fa fa fa[fa]fa fa fa fa fa
  0x0c3e83c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3e83d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3e83e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3e83f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3e8400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:   00
  Part

Re: [Qemu-devel] [PATCH] migrate/cpu-throttle: Add max-cpu-throttle migration parameter

2018-08-07 Thread Li Qiang
 Hello Dave,

What's status of this patch, Do you gonna merge it?

Thanks,
Li Qiang


2018-08-02 18:47 GMT+08:00 Dr. David Alan Gilbert :

> * Li Qiang (liq...@gmail.com) wrote:
> > Currently, the default maximum CPU throttle for migration is
> > 99(CPU_THROTTLE_PCT_MAX). This is too big and can make a remarkable
> > performance effect for the guest. We see a lot of packets latency
> > exceed 500ms when the CPU_THROTTLE_PCT_MAX reached. This patch set
> > adds a new max-cpu-throttle parameter to limit the CPU throttle.
>
> I think this is OK, so
>
> Reviewed-by: Dr. David Alan Gilbert 
>
> but I do have one comment below which made me think
>
> > Signed-off-by: Li Qiang 
> > ---
> >  hmp.c |  8 
> >  migration/migration.c | 23 ++-
> >  migration/migration.h |  1 +
> >  migration/ram.c   |  4 +++-
> >  qapi/migration.json   | 21 ++---
> >  5 files changed, 52 insertions(+), 5 deletions(-)
> >
> > diff --git a/hmp.c b/hmp.c
> > index 2aafb50e8e..c38e8b1f78 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -339,6 +339,10 @@ void hmp_info_migrate_parameters(Monitor *mon,
> const QDict *qdict)
> >  monitor_printf(mon, "%s: %u\n",
> >  MigrationParameter_str(MIGRATION_PARAMETER_CPU_
> THROTTLE_INCREMENT),
> >  params->cpu_throttle_increment);
> > +assert(params->has_max_cpu_throttle);
> > +monitor_printf(mon, "%s: %u\n",
> > +MigrationParameter_str(MIGRATION_PARAMETER_MAX_CPU_
> THROTTLE),
> > +params->max_cpu_throttle);
> >  assert(params->has_tls_creds);
> >  monitor_printf(mon, "%s: '%s'\n",
> >  MigrationParameter_str(MIGRATION_PARAMETER_TLS_CREDS),
> > @@ -1635,6 +1639,10 @@ void hmp_migrate_set_parameter(Monitor *mon,
> const QDict *qdict)
> >  p->has_cpu_throttle_increment = true;
> >  visit_type_int(v, param, >cpu_throttle_increment, );
> >  break;
> > +case MIGRATION_PARAMETER_MAX_CPU_THROTTLE:
> > +p->has_max_cpu_throttle = true;
> > +visit_type_int(v, param, >max_cpu_throttle, );
> > +break;
> >  case MIGRATION_PARAMETER_TLS_CREDS:
> >  p->has_tls_creds = true;
> >  p->tls_creds = g_new0(StrOrNull, 1);
> > diff --git a/migration/migration.c b/migration/migration.c
> > index b7d9854bda..570da6c0e7 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -71,6 +71,7 @@
> >  /* Define default autoconverge cpu throttle migration parameters */
> >  #define DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL 20
> >  #define DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT 10
> > +#define DEFAULT_MIGRATE_MAX_CPU_THROTTLE 99
> >
> >  /* Migration XBZRLE default cache size */
> >  #define DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE (64 * 1024 * 1024)
> > @@ -697,6 +698,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error
> **errp)
> >  params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
> >  params->has_max_postcopy_bandwidth = true;
> >  params->max_postcopy_bandwidth = s->parameters.max_postcopy_
> bandwidth;
> > +params->has_max_cpu_throttle = true;
> > +params->max_cpu_throttle = s->parameters.max_cpu_throttle;
> >
> >  return params;
> >  }
> > @@ -1043,6 +1046,15 @@ static bool migrate_params_check(MigrationParameters
> *params, Error **errp)
> >  return false;
> >  }
> >
> > +if (params->has_max_cpu_throttle &&
> > +(params->max_cpu_throttle < params->cpu_throttle_initial ||
> > + params->max_cpu_throttle > 99)) {
> > +error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> > +   "max_cpu_throttle",
> > +   "an integer in the range of cpu_throttle_initial to
> 99");
> > +return false;
> > +}
> > +
> >  return true;
> >  }
> >
> > @@ -1110,6 +1122,9 @@ static void 
> > migrate_params_test_apply(MigrateSetParameters
> *params,
> >  if (params->has_max_postcopy_bandwidth) {
> >  dest->max_postcopy_bandwidth = params->max_postcopy_bandwidth;
> >  }
> > +if (params->has_max_cpu_throttle) {
> > +dest->max_cpu_throttle = params->max_cpu_throttle;
> > +}
> >  }
> >
> >  static void migrate_params_apply(MigrateSetParameters *params, 

<    2   3   4   5   6   7   8   9   >