Re: [Qemu-devel] [PATCH] gdbstub: Fix buffer overflows in gdb_handle_packet()

[P J P]

+-- On Tue, 13 Oct 2015, Kevin Wolf wrote --+
| diff --git a/gdbstub.c b/gdbstub.c
| index d2c95b5..9c29aa0 100644
| --- a/gdbstub.c
| +++ b/gdbstub.c
| @@ -956,6 +956,13 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
|  if (*p == ',')
|  p++;
|  len = strtoull(p, NULL, 16);
| +
| +/* memtohex() doubles the required space */
| +if (len > MAX_PACKET_LENGTH / 2) {
| +put_packet (s, "E22");
| +break;
| +}
| +
|  if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len, false) != 
0) {
|  put_packet (s, "E14");
|  } else {
| @@ -970,6 +977,12 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
|  len = strtoull(p, (char **)&p, 16);
|  if (*p == ':')
|  p++;
| +
| +/* hextomem() reads 2*len bytes */
| +if (len > strlen(p) / 2) {
| +put_packet (s, "E22");
| +break;
| +}
|  hextomem(mem_buf, p, len);
|  if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len,
| true) != 0) {
| @@ -1107,7 +1120,8 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
|  cpu = find_cpu(thread);
|  if (cpu != NULL) {
|  cpu_synchronize_state(cpu);
| -len = snprintf((char *)mem_buf, sizeof(mem_buf),
| +/* memtohex() doubles the required space */
| +len = snprintf((char *)mem_buf, sizeof(buf) / 2,
| "CPU#%d [%s]", cpu->cpu_index,
| cpu->halted ? "halted " : "running");
|  memtohex(buf, mem_buf, len);
| @@ -1136,8 +1150,8 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
|  put_packet(s, "E01");
|  break;
|  }
| -hextomem(mem_buf, p + 5, len);
|  len = len / 2;
| +hextomem(mem_buf, p + 5, len);
|  mem_buf[len++] = 0;
|  qemu_chr_be_write(s->mon_chr, mem_buf, len);
|  put_packet(s, "OK");
| 

Yes, patch looks good to fix the said buffer overflows.

Nevertheless, I wonder how hextomem() & memtohex() routines help? Because IIUC 
they seem to be changing command semantics. Ie host gdb(1) user would need to 
supply len/2 value to read/write 'len' bytes.

--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH] Add mp-affinity property for ARM CPU class

[Pavel Fedin]

 Hello!

> Nothing wrong with this patch, but I'd rather add it as part of
> the series which actually uses it.

 Ok, as you wish, i just tried to decrease amount of out-of-tree stuff. 
Additionally, i remember Andrew also wanted to do something with affinities, he 
thought about adding emulation of some HW with particular affinity layout, so 
he might also want it.

> (I see you have it in one of your GICv3 patchsets.)

 Yes, i decided to copy it there because some people might want to test the 
patchset, and they would not have to dig out this small piece.

 P.S. By the way, what about GICv3 data format? Are you going to review this 
part and do something with it? This would also advance Shlomo's work i think.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH] Add mp-affinity property for ARM CPU class

[Pavel Fedin]

 Hello!

> Can we have separate props for each affinity level?

 Yes, we can, but i don't see any use for them in this form. Except merging 
back into full MPIDR value. :)

> Are you assuming MPIDR format for this?

 Yes. This is convenient because everything else (KVM, GICv3, etc) also uses 
MPIDR format. Check out GICv3 live migration RFC v2 for one possible use case.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH] Limit memory r/w length to buffer size

[P J P]

+-- On Tue, 13 Oct 2015, Markus Armbruster wrote --+
| How is this related to Kevin's
| [PATCH] gdbstub: Fix buffer overflows in gdb_handle_packet()
| Message-Id: <1444721930-5121-1-git-send-email-kw...@redhat.com> ?

  Oh, didn't know there was already a patch. Yes it fixes the same issues; 
Also the length check is correct. In my patch I did not consider the size 
alterations that occur in memtohex() & hextomem().

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

[Qemu-devel] [PATCH 1/2] target-xtensa: implement depbits instruction

[Max Filippov]

This option provides an instruction for depositing a bit field from the
least significant position of one register to an arbitrary position in
another register.

Signed-off-by: Max Filippov 
---
 target-xtensa/cpu.h  |  1 +
 target-xtensa/overlay_tool.h |  5 +
 target-xtensa/translate.c| 20 
 3 files changed, 26 insertions(+)

diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index 96bfc82..a45cb39 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -65,6 +65,7 @@ enum {
 XTENSA_OPTION_MP_SYNCHRO,
 XTENSA_OPTION_CONDITIONAL_STORE,
 XTENSA_OPTION_ATOMCTL,
+XTENSA_OPTION_DEPBITS,
 
 /* Interrupts and exceptions */
 XTENSA_OPTION_EXCEPTION,
diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h
index eda03aa..e8a7fda 100644
--- a/target-xtensa/overlay_tool.h
+++ b/target-xtensa/overlay_tool.h
@@ -30,6 +30,10 @@
 { .targno = (no), .type = (typ), .group = (grp), .size = (sz) },
 #define XTREG_END { .targno = -1 },
 
+#ifndef XCHAL_HAVE_DEPBITS
+#define XCHAL_HAVE_DEPBITS 0
+#endif
+
 #ifndef XCHAL_HAVE_DIV32
 #define XCHAL_HAVE_DIV32 0
 #endif
@@ -69,6 +73,7 @@
 XCHAL_OPTION(XCHAL_HAVE_S32C1I, XTENSA_OPTION_CONDITIONAL_STORE) | \
 XCHAL_OPTION(XCHAL_HAVE_S32C1I && XCHAL_HW_MIN_VERSION >= 23, \
 XTENSA_OPTION_ATOMCTL) | \
+XCHAL_OPTION(XCHAL_HAVE_DEPBITS, XTENSA_OPTION_DEPBITS) | \
 /* Interrupts and exceptions */ \
 XCHAL_OPTION(XCHAL_HAVE_EXCEPTIONS, XTENSA_OPTION_EXCEPTION) | \
 XCHAL_OPTION(XCHAL_HAVE_VECBASE, XTENSA_OPTION_RELOCATABLE_VECTOR) | \
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index f2118c2..cd2148e 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -1970,6 +1970,16 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 break;
 
 case 10: /*FP0*/
+/*DEPBITS*/
+if (option_enabled(dc, XTENSA_OPTION_DEPBITS)) {
+if (!gen_window_check2(dc, RRR_S, RRR_T)) {
+break;
+}
+tcg_gen_deposit_i32(cpu_R[RRR_T], cpu_R[RRR_T], cpu_R[RRR_S],
+OP2, RRR_R + 1);
+break;
+}
+
 HAS_OPTION(XTENSA_OPTION_FP_COPROCESSOR);
 switch (OP2) {
 case 0: /*ADD.Sf*/
@@ -2104,6 +2114,16 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 break;
 
 case 11: /*FP1*/
+/*DEPBITS*/
+if (option_enabled(dc, XTENSA_OPTION_DEPBITS)) {
+if (!gen_window_check2(dc, RRR_S, RRR_T)) {
+break;
+}
+tcg_gen_deposit_i32(cpu_R[RRR_T], cpu_R[RRR_T], cpu_R[RRR_S],
+OP2 + 16, RRR_R + 1);
+break;
+}
+
 HAS_OPTION(XTENSA_OPTION_FP_COPROCESSOR);
 
 #define gen_compare(rel, br, a, b) \
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/2] target-xtensa: implement S32NB

[Max Filippov]

S32NB provides the same functionality as S32I with two exceptions.
First, when its operation leaves the processor, the external transaction
is marked Non-Bufferable. Second, it may not be used to write to
Instruction RAM.
In QEMU S32NB is equivalent to S32I.

Signed-off-by: Max Filippov 
---
 target-xtensa/translate.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index cd2148e..9b6033d 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -1963,6 +1963,17 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 }
 break;
 
+case 5: /*S32N*/
+if (gen_window_check2(dc, RRI4_S, RRI4_T)) {
+TCGv_i32 addr = tcg_temp_new_i32();
+
+tcg_gen_addi_i32(addr, cpu_R[RRI4_S], RRI4_IMM4 << 2);
+gen_load_store_alignment(dc, 2, addr, false);
+tcg_gen_qemu_st32(cpu_R[RRI4_T], addr, dc->cring);
+tcg_temp_free(addr);
+}
+break;
+
 default:
 RESERVED();
 break;
-- 
1.8.1.4

[Qemu-devel] [PATCH 0/2] target-xtensa: two new instructions

[Max Filippov]

Hi,

this series adds two new xtensa instructions:
- s32nb is a new core instruction;
- depbits is enabled by dedicated option.

Max Filippov (2):
  target-xtensa: implement depbits instruction
  target-xtensa: implement S32NB

 target-xtensa/cpu.h  |  1 +
 target-xtensa/overlay_tool.h |  5 +
 target-xtensa/translate.c| 31 +++
 3 files changed, 37 insertions(+)

-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v3 0/2] PCI-e device multi-function hot-add support

[Cao jin]

On 10/13/2015 09:10 PM, Michael S. Tsirkin wrote:

On Tue, Oct 13, 2015 at 07:54:07PM +0800, Cao jin wrote:



On 10/13/2015 04:49 PM, Michael S. Tsirkin wrote:

On Tue, Oct 13, 2015 at 04:41:33PM +0800, Cao jin wrote:

Support PCI-e device hot-add multi-function via device_add, just ensure
add the function 0 is added last. While allow user to roll back in the
middle via device_del, in case user regret.


This patch doesn't seem to account of AIR though.



Yes, but the AIR function seems never be used(nobody calls the function
pcie_ari_init()), so I am a little confused about should it be consindered?


Yes please - we'll likely use that in the future. Pls add an API
that takes ari into account.


Ok, I am on it


changelog:
1. Flag device as unexposed when func 0 doesn`t exist, via return 0xFF
in case of gratuitous pci bus scan.
2. Since device is unexposed to guest, can remove function individually,
without interaction with the guest.

Cao jin (2):
   enable multi-function hot-add
   remove function during multi-function hot-add

  hw/pci/pci.c  | 10 ++
  hw/pci/pci_host.c |  6 +-
  hw/pci/pcie.c | 38 +-
  3 files changed, 40 insertions(+), 14 deletions(-)

--
2.1.0

.



--
Yours Sincerely,

Cao Jin

.



--
Yours Sincerely,

Cao Jin

Re: [Qemu-devel] [PATCH v3 2/2] remove function during multi-function hot-add

[Cao jin]

Hi, Alex

On 10/13/2015 11:27 PM, Alex Williamson wrote:

On Tue, 2015-10-13 at 16:41 +0800, Cao jin wrote:

In case user regret when hot-adding multi-function, should roll back,
device_del the function added but not exposed to the guest.


As Michael suggests, this patch should come first, before we actually
enable multi-function hot-add.


OK.

Signed-off-by: Cao jin 
---
  hw/pci/pci_host.c |  6 +-
  hw/pci/pcie.c | 22 +-
  2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 3e26f92..35e5cf3 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -20,6 +20,7 @@

  #include "hw/pci/pci.h"
  #include "hw/pci/pci_host.h"
+#include "hw/pci/pci_bus.h"
  #include "trace.h"

  /* debug PCI */
@@ -88,10 +89,13 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, 
int len)
  uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
  {
  PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
+PCIDevice *f0 = NULL;
  uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
  uint32_t val;
+uint8_t slot = (addr >> 11) & 0x1F;

-if (!pci_dev) {
+f0 = s->devices[PCI_DEVFN(slot, 0)];
+if (!pci_dev || (!f0 && pci_dev)) {



This uses a lot more variables and operations than it needs to:

if (!pci_dev || !s->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]) {


Ok. variables is intended to make the line shorter.


Shouldn't we do the same on pci_data_write()?  A well behaved guest
won't blindly write to config space, but not all guests are well
behaved.



Yup, agree. I missed the consideration of bad behavior. I thought anyone 
use the device should read the vendor ID first(good behavior), then do 
anything he/she want. Thanks for reminding



Comments in the code would be nice here to explain that non-zero
functions are only exposed when function zero is present, allowing
direct removal of unexposed devices.


OK


I imagine that due to qemu locking that we don't have a race here, but
note that devices[] is populated early in the core pci realize function,
prior to the device initialize function, and there are any number of
reasons that failure could still occur, which would create a window
where the function is accessible.  I doubt this is an issue, but simply
note it for completeness.


Ok, will consider the "function access window" condition, to see what I 
can do with it



  return ~0x0;
  }

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 89bf61b..58d2153 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -261,13 +261,30 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
*hotplug_dev, DeviceState *dev,
  }
  }

+static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
+{
+object_unparent(OBJECT(dev));
+}
+
  void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
   DeviceState *dev, Error **errp)
  {
  uint8_t *exp_cap;
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+PCIBus *bus = pci_dev->bus;

  pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, 
errp);

+/* In case user regret when hot-adding multi function, remove the function
+ * that is unexposed to guest individually, without interaction with guest.
+ */
+if (PCI_FUNC(pci_dev->devfn) > 0 &&
+bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)] == NULL) {


Similarly,

if (PCI_FUNC(pci_dev->devfn) && 
!bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]) {



Ok


+pcie_unplug_device(bus, pci_dev, NULL);
+
+return;
+}
+
  pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
  }

@@ -378,11 +395,6 @@ void pcie_cap_slot_reset(PCIDevice *dev)
  hotplug_event_update_event_status(dev);
  }

-static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
-{
-object_unparent(OBJECT(dev));
-}
-
  void pcie_cap_slot_write_config(PCIDevice *dev,
  uint32_t addr, uint32_t val, int len)
  {




.



--
Yours Sincerely,

Cao Jin

Re: [Qemu-devel] [PATCH v4 3/5] acpi: pc: add fw_cfg device node to ssdt

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 07:43:00PM -0300, Eduardo Habkost wrote:
> On Wed, Oct 14, 2015 at 12:18:10AM +0300, Michael S. Tsirkin wrote:
> > On Tue, Oct 13, 2015 at 04:10:03PM -0300, Eduardo Habkost wrote:
> > > One of the things that may break if guest-visible bits of the machine
> > > change is Windows license activation, but the rules Windows use to
> > > trigger reactivation aren't very clear.
> > 
> > They are easy to find on the internet.
> 
> I couldn't find them[1]. If you have a pointer to a clear description of
> the rules for all Windows versions, I would love to see it.


The detailed info is not hard to find:
http://en.wikipedia.org/wiki/Microsoft_Product_Activation
links to:
http://technet.microsoft.com/en-us/library/bb457054.aspx


1
Display Adapter
00010 (5)
2
SCSI Adapter
00011 (5)
3
IDE Adapter
0011 (4)
4
Network Adapter MAC Address
1001011000 (10)
5
RAM Amount Range (i.e. 0-64mb, 64-128mb, etc)
101 (3)
6
Processor Type
011 (3)
7
Processor Serial Number
00 (6)
8
Hard Drive Device
1101100 (7)
9
Hard Drive Volume Serial Number
100101 (10)
10
CD—ROM / CD-RW / DVD-ROM
010111 (6)
-
"Dockable"
0 (1)
-
Hardware Hash version (version of algorithm used)
001 (3)

Also:
Microsoft defines "substantially different" hardware differently for PCs
that are configured to be dockable. Additionally, the network adapter is
given a superior "weighting." If the PC is not dockable and a network
adapter exists and is not changed, 6 or more of the other above values
would have to change before reactivation was required. If a network
adapter existed but is changed or never existed at all, 4 or more
changes (including the changed network adapter if it previously existed)
will result in a requirement to reactivate.

So it's also not hard to try it out with kvm.  Apply a patch, change
3 other things: nic mac, ram size (by a factor of >2), cpu type and boot.



> >
> >___
> >Xen-devel mailing list
> >xen-de...@lists.xen.org
> >http://lists.xen.org/xen-devel
> >
> >-
> >No virus found in this message.
> >Checked by AVG - www.avg.com
> >Version: 2014.0.4592 / Virus Database: 3986/7769 - Release Date: 06/30/14
> >
> 
> 
> -- 
> Ross Philipson

Re: [Qemu-devel] [PATCH v3 00/32] implement vNVDIMM

[Xiao Guangrong]

On 10/13/2015 02:36 PM, Dan Williams wrote:

On Mon, Oct 12, 2015 at 10:49 PM, Xiao Guangrong
 wrote:



On 10/13/2015 11:38 AM, Dan Williams wrote:


On Mon, Oct 12, 2015 at 8:14 PM, Xiao Guangrong
 wrote:


On 10/13/2015 12:36 AM, Dan Williams wrote:


Static namespaces can be emitted without a label.  Linux needs this to
support existing "label-less" bare metal NVDIMMs.




This is Linux specific? As i did not see it has been documented in the
spec...



I expect most NVDIMMs, especially existing ones available today, do
not have a label area.  This is not Linux specific and ACPI 6 does not
specify a label area, only the Intel DSM Interface Example.



Yup, label data is accessed via DSM interface, the spec I mentioned
is Intel DSM Interface Example.

However, IIRC Linux NVDIMM driver refused to use the device if no
DSM GET_LABEL support, are you going to update it?


Label-less DIMMs are tested as part of the unit test [1] and the
"memmap=nn!ss" kernel parameter that registers a persistent-memory
address range without a DIMM.  What error do you see when label
support is disabled?

[1]: https://github.com/pmem/ndctl/blob/master/README.md



After revert my commits on NVDIMM driver, yeah, it works.

Okay, i will drop the namespace part and make it as label-less
instead.

Thank you, Dan!

Re: [Qemu-devel] [PATCH v2 4/5] net/dump: Provide the dumping facility as a net-filter

[Yang Hongyang]

On 10/13/2015 06:40 PM, Thomas Huth wrote:

Use the net-filter infrastructure to provide the dumping
functions for netdev devices, too.

Signed-off-by: Thomas Huth 


Reviewed-by: Yang Hongyang 



---
  net/dump.c | 129 -
  vl.c   |   7 +++-
  2 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index e3e82bd..dd0555f 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -28,7 +28,8 @@
  #include "qemu/iov.h"
  #include "qemu/log.h"
  #include "qemu/timer.h"
-#include "hub.h"
+#include "qapi/visitor.h"
+#include "net/filter.h"

  typedef struct DumpState {
  int64_t start_ts;
@@ -225,3 +226,129 @@ int net_init_dump(const NetClientOptions *opts, const 
char *name,
  }
  return rc;
  }
+
+/* Dumping via filter */
+
+#define TYPE_FILTER_DUMP "filter-dump"
+
+#define FILTER_DUMP(obj) \
+OBJECT_CHECK(NetFilterDumpState, (obj), TYPE_FILTER_DUMP)
+
+struct NetFilterDumpState {
+NetFilterState nfs;
+DumpState ds;
+char *filename;
+uint32_t maxlen;
+};
+typedef struct NetFilterDumpState NetFilterDumpState;
+
+static ssize_t filter_dump_receive_iov(NetFilterState *nf, NetClientState 
*sndr,
+   unsigned flags, const struct iovec *iov,
+   int iovcnt, NetPacketSent *sent_cb)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(nf);
+
+dump_receive_iov(&nfds->ds, iov, iovcnt);
+return 0;
+}
+
+static void filter_dump_cleanup(NetFilterState *nf)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(nf);
+
+dump_cleanup(&nfds->ds);
+}
+
+static void filter_dump_setup(NetFilterState *nf, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(nf);
+
+if (!nfds->filename) {
+error_setg(errp, "dump filter needs 'file' property set!");
+return;
+}
+
+net_dump_state_init(&nfds->ds, nfds->filename, nfds->maxlen, errp);
+}
+
+static void filter_dump_get_maxlen(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+uint32_t value = nfds->maxlen;
+
+visit_type_uint32(v, &value, name, errp);
+}
+
+static void filter_dump_set_maxlen(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+Error *local_err = NULL;
+uint32_t value;
+
+visit_type_uint32(v, &value, name, &local_err);
+if (local_err) {
+goto out;
+}
+if (value == 0) {
+error_setg(&local_err, "Property '%s.%s' doesn't take value '%u'",
+   object_get_typename(obj), name, value);
+goto out;
+}
+nfds->maxlen = value;
+
+out:
+error_propagate(errp, local_err);
+}
+
+static char *file_dump_get_filename(Object *obj, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+
+return g_strdup(nfds->filename);
+}
+
+static void file_dump_set_filename(Object *obj, const char *value, Error 
**errp)
+{
+   NetFilterDumpState *nfds = FILTER_DUMP(obj);
+
+g_free(nfds->filename);
+nfds->filename = g_strdup(value);
+}
+
+static void filter_dump_instance_init(Object *obj)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+
+nfds->maxlen = 65536;
+
+object_property_add(obj, "maxlen", "int", filter_dump_get_maxlen,
+filter_dump_set_maxlen, NULL, NULL, NULL);
+object_property_add_str(obj, "file", file_dump_get_filename,
+file_dump_set_filename, NULL);
+}
+
+static void filter_dump_class_init(ObjectClass *oc, void *data)
+{
+NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+nfc->setup = filter_dump_setup;
+nfc->cleanup = filter_dump_cleanup;
+nfc->receive_iov = filter_dump_receive_iov;
+}
+
+static const TypeInfo filter_dump_info = {
+.name = TYPE_FILTER_DUMP,
+.parent = TYPE_NETFILTER,
+.class_init = filter_dump_class_init,
+.instance_init = filter_dump_instance_init,
+.instance_size = sizeof(NetFilterDumpState),
+};
+
+static void filter_dump_register_types(void)
+{
+type_register_static(&filter_dump_info);
+}
+
+type_init(filter_dump_register_types);
diff --git a/vl.c b/vl.c
index 7c806a2..b54a800 100644
--- a/vl.c
+++ b/vl.c
@@ -2748,7 +2748,12 @@ static bool object_create_initial(const char *type)
  return false;
  }

-if (g_str_equal(type, "filter-buffer")) {
+/*
+ * return false for concrete netfilters since
+ * they depend on netdevs already existing
+ */
+if (g_str_equal(type, "filter-buffer") ||
+g_str_equal(type, "filter-dump")) {
  return false;
  }




--
Thanks,
Yang.

Re: [Qemu-devel] [PATCH v2 3/5] net/dump: Separate the NetClientState from the DumpState

[Yang Hongyang]

On 10/13/2015 06:40 PM, Thomas Huth wrote:

With the upcoming dumping-via-netfilter patch, the DumpState
should not be related to NetClientState anymore, so move the
related information to a new struct called DumpNetClient.

Signed-off-by: Thomas Huth 


Reviewed-by: Yang Hongyang 


---
  net/dump.c | 74 +-
  1 file changed, 49 insertions(+), 25 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index e6f6be0..e3e82bd 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -31,7 +31,6 @@
  #include "hub.h"

  typedef struct DumpState {
-NetClientState nc;
  int64_t start_ts;
  int fd;
  int pcap_caplen;
@@ -58,10 +57,8 @@ struct pcap_sf_pkthdr {
  uint32_t len;
  };

-static ssize_t dump_receive_iov(NetClientState *nc, const struct iovec *iov,
-int cnt)
+static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt)
  {
-DumpState *s = DO_UPCAST(DumpState, nc, nc);
  struct pcap_sf_pkthdr hdr;
  int64_t ts;
  int caplen;
@@ -94,30 +91,12 @@ static ssize_t dump_receive_iov(NetClientState *nc, const 
struct iovec *iov,
  return size;
  }

-static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t 
size)
-{
-struct iovec iov = {
-.iov_base = (void *)buf,
-.iov_len = size
-};
-return dump_receive_iov(nc, &iov, 1);
-}
-
-static void dump_cleanup(NetClientState *nc)
+static void dump_cleanup(DumpState *s)
  {
-DumpState *s = DO_UPCAST(DumpState, nc, nc);
-
  close(s->fd);
+s->fd = -1;
  }

-static NetClientInfo net_dump_info = {
-.type = NET_CLIENT_OPTIONS_KIND_DUMP,
-.size = sizeof(DumpState),
-.receive = dump_receive,
-.receive_iov = dump_receive_iov,
-.cleanup = dump_cleanup,
-};
-
  static int net_dump_state_init(DumpState *s, const char *filename,
 int len, Error **errp)
  {
@@ -154,6 +133,49 @@ static int net_dump_state_init(DumpState *s, const char 
*filename,
  return 0;
  }

+/* Dumping via VLAN netclient */
+
+struct DumpNetClient {
+NetClientState nc;
+DumpState ds;
+};
+typedef struct DumpNetClient DumpNetClient;
+
+static ssize_t dumpclient_receive(NetClientState *nc, const uint8_t *buf,
+  size_t size)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+struct iovec iov = {
+.iov_base = (void *)buf,
+.iov_len = size
+};
+
+return dump_receive_iov(&dc->ds, &iov, 1);
+}
+
+static ssize_t dumpclient_receive_iov(NetClientState *nc,
+  const struct iovec *iov, int cnt)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+
+return dump_receive_iov(&dc->ds, iov, cnt);
+}
+
+static void dumpclient_cleanup(NetClientState *nc)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+
+dump_cleanup(&dc->ds);
+}
+
+static NetClientInfo net_dump_info = {
+.type = NET_CLIENT_OPTIONS_KIND_DUMP,
+.size = sizeof(DumpNetClient),
+.receive = dumpclient_receive,
+.receive_iov = dumpclient_receive_iov,
+.cleanup = dumpclient_cleanup,
+};
+
  int net_init_dump(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
  {
@@ -162,6 +184,7 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
  char def_file[128];
  const NetdevDumpOptions *dump;
  NetClientState *nc;
+DumpNetClient *dnc;

  assert(opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP);
  dump = opts->dump;
@@ -195,7 +218,8 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
  snprintf(nc->info_str, sizeof(nc->info_str),
   "dump to %s (len=%d)", file, len);

-rc = net_dump_state_init(DO_UPCAST(DumpState, nc, nc), file, len, errp);
+dnc = DO_UPCAST(DumpNetClient, nc, nc);
+rc = net_dump_state_init(&dnc->ds, file, len, errp);
  if (rc) {
  qemu_del_net_client(nc);
  }



--
Thanks,
Yang.

Re: [Qemu-devel] [PATCH] net: cadence_gem: Set initial MAC address

[Jason Wang]

On 10/12/2015 04:25 PM, Sebastian Huber wrote:
> Set initial MAC address to the one specified by the command line.
>
> Signed-off-by: Sebastian Huber 
> Reviewed-by: Jason Wang 
> Reviewed-by: Peter Crosthwaite 
> ---
>  hw/net/cadence_gem.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 1127223..3639fc1 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -964,6 +964,7 @@ static void gem_reset(DeviceState *d)
>  {
>  int i;
>  CadenceGEMState *s = CADENCE_GEM(d);
> +const uint8_t *a;
>  
>  DB_PRINT("\n");
>  
> @@ -982,6 +983,11 @@ static void gem_reset(DeviceState *d)
>  s->regs[GEM_DESCONF5] = 0x002f2145;
>  s->regs[GEM_DESCONF6] = 0x0200;
>  
> +/* Set MAC address */
> +a = &s->conf.macaddr.a[0];
> +s->regs[GEM_SPADDR1LO] = a[0] | (a[1] << 8) | (a[2] << 16) | (a[3] << 
> 24);
> +s->regs[GEM_SPADDR1HI] = a[4] | (a[5] << 8);
> +
>  for (i = 0; i < 4; i++) {
>  s->sar_active[i] = false;
>  }

Applied in https://github.com/jasowang/qemu/commits/net

Thanks

Re: [Qemu-devel] [PATCH v2 2/5] net/dump: Rework net-dump init functions

[Yang Hongyang]

On 10/13/2015 06:39 PM, Thomas Huth wrote:

Move the creation of the dump client from net_dump_init() into
net_init_dump(), so we can later use the former function for
dump via netfilter, too. Also rename net_dump_init() to
net_dump_state_init() to make it easier distinguishable from
net_init_dump().

Signed-off-by: Thomas Huth 



Reviewed-by: Yang Hongyang 


---
  net/dump.c | 27 +--
  1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index aa0d45d..e6f6be0 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -118,13 +118,10 @@ static NetClientInfo net_dump_info = {
  .cleanup = dump_cleanup,
  };

-static int net_dump_init(NetClientState *peer, const char *device,
- const char *name, const char *filename, int len,
- Error **errp)
+static int net_dump_state_init(DumpState *s, const char *filename,
+   int len, Error **errp)
  {
  struct pcap_file_hdr hdr;
-NetClientState *nc;
-DumpState *s;
  struct tm tm;
  int fd;

@@ -148,13 +145,6 @@ static int net_dump_init(NetClientState *peer, const char 
*device,
  return -1;
  }

-nc = qemu_new_net_client(&net_dump_info, peer, device, name);
-
-snprintf(nc->info_str, sizeof(nc->info_str),
- "dump to %s (len=%d)", filename, len);
-
-s = DO_UPCAST(DumpState, nc, nc);
-
  s->fd = fd;
  s->pcap_caplen = len;

@@ -167,10 +157,11 @@ static int net_dump_init(NetClientState *peer, const char 
*device,
  int net_init_dump(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
  {
-int len;
+int len, rc;
  const char *file;
  char def_file[128];
  const NetdevDumpOptions *dump;
+NetClientState *nc;

  assert(opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP);
  dump = opts->dump;
@@ -200,5 +191,13 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
  len = 65536;
  }

-return net_dump_init(peer, "dump", name, file, len, errp);
+nc = qemu_new_net_client(&net_dump_info, peer, "dump", name);
+snprintf(nc->info_str, sizeof(nc->info_str),
+ "dump to %s (len=%d)", file, len);
+
+rc = net_dump_state_init(DO_UPCAST(DumpState, nc, nc), file, len, errp);
+if (rc) {
+qemu_del_net_client(nc);
+}
+return rc;
  }



--
Thanks,
Yang.

Re: [Qemu-devel] [PATCH v2 1/5] net/dump: Add support for receive_iov function

[Yang Hongyang]

On 10/13/2015 06:39 PM, Thomas Huth wrote:

Adding a proper receive_iov function to the net dump module.
This will make it easier to support the dump filter feature for
the -netdev option in later patches.

Signed-off-by: Thomas Huth 


Reviewed-by: Yang Hongyang 


---
  net/dump.c | 24 +---
  1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index 08259af..aa0d45d 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -25,6 +25,7 @@
  #include "clients.h"
  #include "qemu-common.h"
  #include "qemu/error-report.h"
+#include "qemu/iov.h"
  #include "qemu/log.h"
  #include "qemu/timer.h"
  #include "hub.h"
@@ -57,12 +58,15 @@ struct pcap_sf_pkthdr {
  uint32_t len;
  };

-static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t 
size)
+static ssize_t dump_receive_iov(NetClientState *nc, const struct iovec *iov,
+int cnt)
  {
  DumpState *s = DO_UPCAST(DumpState, nc, nc);
  struct pcap_sf_pkthdr hdr;
  int64_t ts;
  int caplen;
+size_t size = iov_size(iov, cnt);
+struct iovec dumpiov[cnt + 1];

  /* Early return in case of previous error. */
  if (s->fd < 0) {
@@ -76,8 +80,12 @@ static ssize_t dump_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
  hdr.ts.tv_usec = ts % 100;
  hdr.caplen = caplen;
  hdr.len = size;
-if (write(s->fd, &hdr, sizeof(hdr)) != sizeof(hdr) ||
-write(s->fd, buf, caplen) != caplen) {
+
+dumpiov[0].iov_base = &hdr;
+dumpiov[0].iov_len = sizeof(hdr);
+cnt = iov_copy(&dumpiov[1], cnt, iov, cnt, 0, caplen);
+
+if (writev(s->fd, dumpiov, cnt + 1) != sizeof(hdr) + caplen) {
  qemu_log("-net dump write error - stop dump\n");
  close(s->fd);
  s->fd = -1;
@@ -86,6 +94,15 @@ static ssize_t dump_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
  return size;
  }

+static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t 
size)
+{
+struct iovec iov = {
+.iov_base = (void *)buf,
+.iov_len = size
+};
+return dump_receive_iov(nc, &iov, 1);
+}
+
  static void dump_cleanup(NetClientState *nc)
  {
  DumpState *s = DO_UPCAST(DumpState, nc, nc);
@@ -97,6 +114,7 @@ static NetClientInfo net_dump_info = {
  .type = NET_CLIENT_OPTIONS_KIND_DUMP,
  .size = sizeof(DumpState),
  .receive = dump_receive,
+.receive_iov = dump_receive_iov,
  .cleanup = dump_cleanup,
  };




--
Thanks,
Yang.

[Qemu-devel] [Bug 893208] Re: qemu on ARM hosts can't boot i386 image

[PeteVine]

BTW, it seems the more expensive (but vastly less popular) odroids like
the xu4 are built around kvm enabled processors which is why this bug
doesn't affect them.

The most popular C1/C1+'s processor doesn't support kvm though so any
update would be appreciated.

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

Title:
  qemu on ARM hosts can't boot i386 image

Status in QEMU:
  New
Status in Linaro QEMU:
  New

Bug description:
  If you apply some workarounds for bug 870990, bug 883133 and bug
  883136 QEMU still cannot boot the i386
  debian_squeeze_i386_standard.qcow2 image from
  http://people.debian.org/~aurel32/qemu/i386/ -- grub starts to boot
  but something causes the system to reset just before display of the
  blue-background grub menu, so we go round in a loop forever. This
  image boots OK on i386 hosted qemu so this indicates some kind of ARM-
  host specific bug.

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

Re: [Qemu-devel] [Qemu-block] [PATCH v10 08/10] Implement new driver for block replication

[Wen Congyang]

On 10/13/2015 06:12 PM, Fam Zheng wrote:
> On Tue, 10/13 17:46, Wen Congyang wrote:
>> On 10/13/2015 05:41 PM, Fam Zheng wrote:
>>> On Tue, 10/13 16:59, Wen Congyang wrote:
 On 10/13/2015 12:25 AM, Stefan Hajnoczi wrote:
> On Fri, Sep 25, 2015 at 02:17:36PM +0800, Wen Congyang wrote:
>> +static void backup_job_completed(void *opaque, int ret)
>> +{
>> +BDRVReplicationState *s = opaque;
>> +
>> +if (s->replication_state != BLOCK_REPLICATION_DONE) {
>> +/* The backup job is cancelled unexpectedly */
>> +s->error = -EIO;
>> +}
>> +
>> +bdrv_op_block(s->hidden_disk, BLOCK_OP_TYPE_BACKUP_TARGET,
>> +  s->active_disk->backing_blocker);
>> +bdrv_op_block(s->secondary_disk, BLOCK_OP_TYPE_BACKUP_SOURCE,
>> +  s->hidden_disk->backing_blocker);
>> +
>> +bdrv_put_ref_bh_schedule(s->secondary_disk);
>
> Why is bdrv_put_ref_bh_schedule() necessary?

 It is copied from block_job_cb(). According to the comments in 
 bdrv_put_ref_bh_schedule():
 /*
  * Release a BDS reference in a BH
  *
  * It is not safe to use bdrv_unref() from a callback function when the 
 callers
  * still need the BlockDriverState.  In such cases we schedule a BH to 
 release
  * the reference.
  */

 If the comment is right, I think it is necessary to call 
 bdrv_put_ref_bh_schedule() here.
 Because the job is created on the BDS s->secondary disk, 
 backup_job_completed() is
 called in block_job_completed(), and we will still use s->secondary_disk 
 in block_job_release().
>>>
>>> Where is the matching bdrv_ref called?
>>
>> It is in block_job_create()
>>
>> source: we call in bdrv_ref() in block_job_create(), and the user should 
>> unref it.
>> target: the user call bdrv_ref(), and we will unref it in the job
>>
>> I don't know why we design it like this...
>>
> 
> Maybe it's better to unref it in block_job_release. Then we can simply drop
> bdrv_put_ref_bh_schedule.

I agree with it.

Thanks
Wen Congyang

> 
> Fam
> .
> 

[Qemu-devel] [PATCH v5 2/2] arm_mptimer: Convert to use ptimer

[Dmitry Osipenko]

Current ARM MPTimer implementation uses QEMUTimer for the actual timer,
this implementation isn't complete and mostly tries to duplicate of what
generic ptimer is already doing fine.

Conversion to ptimer brings the following benefits and fixes:
- Simple timer pausing implementation
- Fixes counter value preservation after stopping the timer
- Code simplification and reduction

Bump VMSD to version 3, since VMState is changed and is not compatible
with the previuos implementation.

Signed-off-by: Dmitry Osipenko 
---

Changelog:
V2: Fixed changing periodic timer counter value "on the fly". I added a
test to the gist to cover that issue.
V3: Fixed starting the timer with load = 0 and counter != 0, added tests
to the gist for this issue. Changed vmstate version for all VMSD's,
since loadvm doesn't check version of nested VMSD.
V4: Fixed spurious IT bit set for the timer starting in the periodic mode
with counter = 0. Test added.
V5: Code cleanup, now depends on ptimer_set_limit() fix.

Tests: https://gist.github.com/digetx/dbd46109503b1a91941a

 hw/timer/arm_mptimer.c | 110 ++---
 include/hw/timer/arm_mptimer.h |   4 +-
 2 files changed, 49 insertions(+), 65 deletions(-)

diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index 3e59c2a..c06da5e 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -19,8 +19,9 @@
  * with this program; if not, see .
  */
 
+#include "hw/ptimer.h"
 #include "hw/timer/arm_mptimer.h"
-#include "qemu/timer.h"
+#include "qemu/main-loop.h"
 #include "qom/cpu.h"
 
 /* This device implements the per-cpu private timer and watchdog block
@@ -47,28 +48,10 @@ static inline uint32_t timerblock_scale(TimerBlock *tb)
 return (((tb->control >> 8) & 0xff) + 1) * 10;
 }
 
-static void timerblock_reload(TimerBlock *tb, int restart)
-{
-if (tb->count == 0) {
-return;
-}
-if (restart) {
-tb->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-}
-tb->tick += (int64_t)tb->count * timerblock_scale(tb);
-timer_mod(tb->timer, tb->tick);
-}
-
 static void timerblock_tick(void *opaque)
 {
 TimerBlock *tb = (TimerBlock *)opaque;
 tb->status = 1;
-if (tb->control & 2) {
-tb->count = tb->load;
-timerblock_reload(tb, 0);
-} else {
-tb->count = 0;
-}
 timerblock_update_irq(tb);
 }
 
@@ -76,21 +59,11 @@ static uint64_t timerblock_read(void *opaque, hwaddr addr,
 unsigned size)
 {
 TimerBlock *tb = (TimerBlock *)opaque;
-int64_t val;
 switch (addr) {
 case 0: /* Load */
 return tb->load;
 case 4: /* Counter.  */
-if (((tb->control & 1) == 0) || (tb->count == 0)) {
-return 0;
-}
-/* Slow and ugly, but hopefully won't happen too often.  */
-val = tb->tick - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-val /= timerblock_scale(tb);
-if (val < 0) {
-val = 0;
-}
-return val;
+return ptimer_get_count(tb->timer);
 case 8: /* Control.  */
 return tb->control;
 case 12: /* Interrupt status.  */
@@ -100,6 +73,19 @@ static uint64_t timerblock_read(void *opaque, hwaddr addr,
 }
 }
 
+static void timerblock_run(TimerBlock *tb, uint64_t count, int set_count)
+{
+if (set_count) {
+if (((tb->control & 3) == 3) && (count == 0)) {
+count = tb->load;
+}
+ptimer_set_count(tb->timer, count);
+}
+if ((tb->control & 1) && (count != 0)) {
+ptimer_run(tb->timer, !(tb->control & 2));
+}
+}
+
 static void timerblock_write(void *opaque, hwaddr addr,
  uint64_t value, unsigned size)
 {
@@ -108,32 +94,34 @@ static void timerblock_write(void *opaque, hwaddr addr,
 switch (addr) {
 case 0: /* Load */
 tb->load = value;
-/* Fall through.  */
-case 4: /* Counter.  */
-if ((tb->control & 1) && tb->count) {
-/* Cancel the previous timer.  */
-timer_del(tb->timer);
+/* Setting load to 0 stops the timer.  */
+if (tb->load == 0) {
+ptimer_stop(tb->timer);
 }
-tb->count = value;
-if (tb->control & 1) {
-timerblock_reload(tb, 1);
+ptimer_set_limit(tb->timer, tb->load, 1);
+timerblock_run(tb, tb->load, 0);
+break;
+case 4: /* Counter.  */
+/* Setting counter to 0 stops the one-shot timer.  */
+if (!(tb->control & 2) && (value == 0)) {
+ptimer_stop(tb->timer);
 }
+timerblock_run(tb, value, 1);
 break;
 case 8: /* Control.  */
 old = tb->control;
 tb->control = value;
-if (value & 1) {
-if ((old & 1) && (tb->count != 0)) {
-/* Do nothing if timer is ticking right now.  */
-brea

[Qemu-devel] [PATCH v5 1/2] hw/ptimer: Set delta to the original limit on reload in ptimer_set_limit()

[Dmitry Osipenko]

ptimer_get_count() returns incorrect value for the disabled timer after
reloading the counter with a small value, because corrected limit value
is used instead of the original.

For instance:
1) ptimer_stop(t)
2) ptimer_set_period(t, 1)
3) ptimer_set_limit(t, 0, 1)
4) ptimer_get_count(t) <-- would return 1 instead of 0

Signed-off-by: Dmitry Osipenko 
---
 hw/core/ptimer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 8437bd6..abc3a20 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -180,6 +180,8 @@ void ptimer_set_freq(ptimer_state *s, uint32_t freq)
count = limit.  */
 void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload)
 {
+uint64_t count = limit;
+
 /*
  * Artificially limit timeout rate to something
  * achievable under QEMU.  Otherwise, QEMU spends all
@@ -195,7 +197,7 @@ void ptimer_set_limit(ptimer_state *s, uint64_t limit, int 
reload)
 
 s->limit = limit;
 if (reload)
-s->delta = limit;
+s->delta = count;
 if (s->enabled && reload) {
 s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 ptimer_reload(s);
-- 
2.6.0

Re: [Qemu-devel] [PATCH v9 00/24] Generate ACPI v5.1 tables and expose them to guest over fw_cfg on ARM

[liang yan]

Hello, Laszlo,

On 10/10/15 00:34, liang yan wrote:
>/Hello, Shannon,/
>/> From: Shannon Zhao /
>/>/
>/> This patch series generate seven ACPI tables for machine virt on ARM./
>/> The set of generated tables are:/
>/> - RSDP/
>/> - RSDT/
>/> - MADT/
>/> - GTDT/
>/> - FADT/
>/> - DSDT/
>/> - MCFG (For PCIe host bridge)/
>/>/
>/> These tables are created dynamically using the function of aml-build.c,/
>/> taking into account the needed information passed from the virt machine/
>/> model. When the generation is finalized, it use fw_cfg to expose the/
>/> tables to guest./
>/>/
>/> You can fetch this from following repo:/
>/> http://git.linaro.org/people/shannon.zhao/qemu.git ACPI_ARM_v9/
>/>/
>/> And this patchset refers to Alexander Spyridakis's patches which are/
>/> sent to qemu-devel mailing list before./
>/> /
>/> http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg03987.html/
>/>/
>/> Thanks to Laszlo's work on UEFI (ArmVirtualizationQemu) supporting/
>/> downloading ACPI tables over fw_cfg, we now can use ACPI in VM./
>/>/
>/> Now upstream kernel applies ACPI patchset, so we can boot it with ACPI,/
>/> while we need to apply patches[1] to make tty work, patch[2] to make/
>/> virtio-mmio work and apply patch[3] and the relevant patches to make 
PCI/

>/> devices works, e.g. virtio-net-pci, e1000./
>/> On the other hand, you can directly use the Fedora Linux kernel from/
>/> following address:/
>/> https://git.fedorahosted.org/cgit/kernel-arm64.git/log/?h=devel/
>/>/
>/> I've done test with following VM:/
>/> xp, windows2008, sles11 on X86/
>/> upstream kernel and Fedora Linux kernel on ARM64/
>/>/
>/> In addtion, dump all the acpi tables, use iasl -d *.dat to convert to/
>/> *.asl and use iasl -tc *.asl to compile them to *.hex. No error 
appears./

>/>/
>/> If you want to test, you could get kernel Image from [4] which contains/
>/> uart, virtio-mmio, pci drivers, UEFI binary from [5] and Qemu command/
>/> line example from [6]./
>/I tested with your kernel and bios, all runs well. But when I try to/
>/build a new debian(upstream) with your qemu patch and bios,/
>/it always told me could find the right driver, or could not enable ACPI/
>/from kernel command line. Do you have a full vm for fedora or/
>/you just use the kernel there? Could you tell me more about your detail?/
>/Thanks./
>//
>/Also, we have our own EDK-II, and it could not work now, so I need to do/
>/patches too. Do you mind to tell me how you build your QMEU.fd? Where/
>/can I access those source code? Thanks./

The relevant edk2 patches have been in the upstream repo for quite some
time now; you shouldn't need anything extra.

I built a new QEMU.fd file, and it worked fine. Thanks for your reply.

Best,
Liang


You can clone the repo from .

Build instructions are written up for example in the linaro wiki
, but someone else asked about
the same just the other day on the edk2 mailing list, and I answered there:

http://news.gmane.org/address@hidden

Wrt. the QEMU command line, I recommend something like:

   # recreate first flash drive from most recent firmware build
   cat \
 .../Build/ArmVirtQemu-AARCH64/DEBUG_GCC48/FV/QEMU_EFI.fd \
 /dev/zero \
   | head -c $((64 * 1024 * 1024)) >| flash0.img

   # create second flash drive (varstore) if it doesn't exist
   if ! [ -e flash1.img ]; then
 head -c $((64 * 1024 * 1024)) /dev/zero > flash1.img
   fi

   # launch qemu (TCG)
   .../qemu-system-aarch64 \
 -nodefaults \
 -nodefconfig \
 -nographic \
 \
 -m 2048 \
 -cpu cortex-a57 \
 -M virt \
 \
 -drive if=pflash,format=raw,file=flash0.img,readonly \
 -drive if=pflash,format=raw,file=flash1.img \
 \
 -chardev stdio,signal=off,mux=on,id=char0 \
 -mon chardev=char0,mode=readline,default \
 -serial chardev:char0 \
 \
 ...

These commands are appropriate for a "persistent" virtual machine (ie.
one where you want to preserve the non-volatile UEFI variables from boot
to boot).

If you want to start again with an empty varstore, just delete
"flash1.img". (Normally, you'd only do that when also zapping the VM's
system disk.)

However, if you can (and are willing to) use libvirt, I certainly
recommend that you do. See eg.

(although virt-install has become even more convenient since then).

HTH
Laszlo


>//
>//
>/Best,/
>/Liang/
>//
>//
>/> 
[1]http://git.linaro.org/leg/acpi/acpi.git/shortlog/refs/heads/acpi-sbsa/

>/> [2]/
>/> 
http://git.linaro.org/leg/acpi/acpi.git/commit/57acba56d55e3fb521fd6ce767446459ef7a4943/

>/>/
>/> [3]/
>/> 
https://git.fedorahosted.org/cgit/kernel-arm64.git/commit/?h=devel&id=8cf58cbe94b982b680229e5b164231eea0ca2d11/

>/>/
>/> [4]http://people.linaro.org/~shannon.zhao/ACPI_ARM/Image.gz 
/

>/> 

Re: [Qemu-devel] [Qemu-block] [PATCH v7 5/5] block: add tests for the 'blockdev-snapshot' command

[Jeff Cody]

On Mon, Oct 12, 2015 at 12:16:17PM +0300, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia 
> Reviewed-by: Max Reitz 
> ---
>  tests/qemu-iotests/085 | 102 
> ++---
>  tests/qemu-iotests/085.out |  34 ++-
>  2 files changed, 128 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
> index 56cd6f8..9484117 100755
> --- a/tests/qemu-iotests/085
> +++ b/tests/qemu-iotests/085
> @@ -7,6 +7,7 @@
>  # snapshots are performed.
>  #
>  # Copyright (C) 2014 Red Hat, Inc.
> +# Copyright (C) 2015 Igalia, S.L.
>  #
>  # This program is free software; you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -34,17 +35,17 @@ status=1  # failure is the default!
>  snapshot_virt0="snapshot-v0.qcow2"
>  snapshot_virt1="snapshot-v1.qcow2"
>  
> -MAX_SNAPSHOTS=10
> +SNAPSHOTS=10
>  
>  _cleanup()
>  {
>  _cleanup_qemu
> -for i in $(seq 1 ${MAX_SNAPSHOTS})
> +for i in $(seq 1 ${SNAPSHOTS})
>  do
>  rm -f "${TEST_DIR}/${i}-${snapshot_virt0}"
>  rm -f "${TEST_DIR}/${i}-${snapshot_virt1}"
>  done
> - _cleanup_test_img
> +rm -f "${TEST_IMG}.1" "${TEST_IMG}.2"
>  
>  }
>  trap "_cleanup; exit \$status" 0 1 2 3 15
> @@ -85,18 +86,50 @@ function create_group_snapshot()
>  _send_qemu_cmd $h "${cmd}" "return"
>  }
>  
> +# ${1}: unique identifier for the snapshot filename
> +# ${2}: true: open backing images; false: don't open them (default)
> +function add_snapshot_image()
> +{
> +if [ "${2}" = "true" ]; then
> +extra_params=""
> +else
> +extra_params="'backing': '', "
> +fi
> +base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}"
> +snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"
> +_make_test_img -b "${base_image}" "$size"
> +mv "${TEST_IMG}" "${snapshot_file}"
> +cmd="{ 'execute': 'blockdev-add', 'arguments':
> +   { 'options':
> + { 'driver': 'qcow2', 'node-name': 'snap_"${1}"', 
> "${extra_params}"
> +   'file':
> +   { 'driver': 'file', 'filename': '"${snapshot_file}"' } } } }"
> +_send_qemu_cmd $h "${cmd}" "return"
> +}
> +
> +# ${1}: unique identifier for the snapshot filename
> +# ${2}: expected response, defaults to 'return'
> +function blockdev_snapshot()
> +{
> +cmd="{ 'execute': 'blockdev-snapshot',
> +  'arguments': { 'node': 'virtio0',
> + 'overlay':'snap_"${1}"' } }"
> +_send_qemu_cmd $h "${cmd}" "${2:-return}"
> +}
> +
>  size=128M
>  
>  _make_test_img $size
> -mv "${TEST_IMG}" "${TEST_IMG}.orig"
> +mv "${TEST_IMG}" "${TEST_IMG}.1"
>  _make_test_img $size
> +mv "${TEST_IMG}" "${TEST_IMG}.2"
>  
>  echo
>  echo === Running QEMU ===
>  echo
>  
>  qemu_comm_method="qmp"
> -_launch_qemu -drive file="${TEST_IMG}.orig",if=virtio -drive 
> file="${TEST_IMG}",if=virtio
> +_launch_qemu -drive file="${TEST_IMG}.1",if=virtio -drive 
> file="${TEST_IMG}.2",if=virtio
>  h=$QEMU_HANDLE
>  
>  echo
> @@ -105,6 +138,8 @@ echo
>  
>  _send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return"
>  
> +# Tests for the blockdev-snapshot-sync command
> +
>  echo
>  echo === Create a single snapshot on virtio0 ===
>  echo
> @@ -132,11 +167,66 @@ echo
>  echo === Create several transactional group snapshots ===
>  echo
>  
> -for i in $(seq 2 ${MAX_SNAPSHOTS})
> +for i in $(seq 2 ${SNAPSHOTS})
>  do
>  create_group_snapshot ${i}
>  done
>  
> +# Tests for the blockdev-snapshot command
> +
> +echo
> +echo === Create a couple of snapshots using blockdev-snapshot ===
> +echo
> +
> +SNAPSHOTS=$((${SNAPSHOTS}+1))
> +add_snapshot_image ${SNAPSHOTS}
> +blockdev_snapshot ${SNAPSHOTS}
> +
> +SNAPSHOTS=$((${SNAPSHOTS}+1))
> +add_snapshot_image ${SNAPSHOTS}
> +blockdev_snapshot ${SNAPSHOTS}
> +
> +echo
> +echo === Invalid command - snapshot node used as active layer ===
> +echo
> +
> +blockdev_snapshot ${SNAPSHOTS} error
> +
> +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
> + 'arguments': { 'node':'virtio0',
> +'overlay':'virtio0' }
> +   }" "error"
> +
> +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
> + 'arguments': { 'node':'virtio0',
> +'overlay':'virtio1' }
> +   }" "error"
> +
> +echo
> +echo === Invalid command - snapshot node used as backing hd ===
> +echo
> +
> +blockdev_snapshot $((${SNAPSHOTS}-1)) error
> +
> +echo
> +echo === Invalid command - snapshot node has a backing image ===
> +echo
> +
> +SNAPSHOTS=$((${SNAPSHOTS}+1))
> +add_snapshot_image ${SNAPSHOTS} true
> +blockdev_snapshot ${SNAPSHOTS} error
> +
> +echo
> +echo === Invalid command - The node does not exist ===
> +echo
> +
> +blockdev_snapshot $((${SNAPSHOTS}+1)) error
> +
> +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
> +  

Re: [Qemu-devel] [Qemu-block] [PATCH v7 2/5] block: rename BlockdevSnapshot to BlockdevSnapshotSync

[Jeff Cody]

On Mon, Oct 12, 2015 at 12:16:14PM +0300, Alberto Garcia wrote:
> We will introduce the 'blockdev-snapshot' command that will require
> its own struct for the parameters, so we need to rename this one in
> order to avoid name clashes.
> 
> Signed-off-by: Alberto Garcia 
> Reviewed-by: Eric Blake 
> Reviewed-by: Max Reitz 
> Reviewed-by: Kevin Wolf 
> ---
>  blockdev.c   | 2 +-
>  qapi-schema.json | 2 +-
>  qapi/block-core.json | 8 
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 0898d1f..12741a0 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1166,7 +1166,7 @@ void qmp_blockdev_snapshot_sync(bool has_device, const 
> char *device,
>  bool has_format, const char *format,
>  bool has_mode, NewImageMode mode, Error 
> **errp)
>  {
> -BlockdevSnapshot snapshot = {
> +BlockdevSnapshotSync snapshot = {
>  .has_device = has_device,
>  .device = (char *) device,
>  .has_node_name = has_node_name,
> diff --git a/qapi-schema.json b/qapi-schema.json
> index a05794e..65701dc 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1534,7 +1534,7 @@
>  ##
>  { 'union': 'TransactionAction',
>'data': {
> -   'blockdev-snapshot-sync': 'BlockdevSnapshot',
> +   'blockdev-snapshot-sync': 'BlockdevSnapshotSync',
> 'drive-backup': 'DriveBackup',
> 'blockdev-backup': 'BlockdevBackup',
> 'abort': 'Abort',
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 5f12af7..6b5ac02 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -682,7 +682,7 @@
>'data': [ 'existing', 'absolute-paths' ] }
>  
>  ##
> -# @BlockdevSnapshot
> +# @BlockdevSnapshotSync
>  #
>  # Either @device or @node-name must be set but not both.
>  #
> @@ -699,7 +699,7 @@
>  # @mode: #optional whether and how QEMU should create a new image, default is
>  #'absolute-paths'.
>  ##
> -{ 'struct': 'BlockdevSnapshot',
> +{ 'struct': 'BlockdevSnapshotSync',
>'data': { '*device': 'str', '*node-name': 'str',
>  'snapshot-file': 'str', '*snapshot-node-name': 'str',
>  '*format': 'str', '*mode': 'NewImageMode' } }
> @@ -790,7 +790,7 @@
>  #
>  # Generates a synchronous snapshot of a block device.
>  #
> -# For the arguments, see the documentation of BlockdevSnapshot.
> +# For the arguments, see the documentation of BlockdevSnapshotSync.
>  #
>  # Returns: nothing on success
>  #  If @device is not a valid block device, DeviceNotFound
> @@ -798,7 +798,7 @@
>  # Since 0.14.0
>  ##
>  { 'command': 'blockdev-snapshot-sync',
> -  'data': 'BlockdevSnapshot' }
> +  'data': 'BlockdevSnapshotSync' }
>  
>  ##
>  # @change-backing-file
> -- 
> 2.6.1
> 
> 
Reviewed-by: Jeff Cody 

[Qemu-devel] [PATCH] target-*: Advance pc after recognizing a breakpoint

[Richard Henderson]

Some targets already had this within their logic, but make sure
it's present for all targets.

Signed-off-by: Richard Henderson 
---
 target-alpha/translate.c  | 3 +++
 target-cris/translate.c   | 3 +++
 target-i386/translate.c   | 3 +++
 target-lm32/translate.c   | 3 +++
 target-m68k/translate.c   | 3 +++
 target-microblaze/translate.c | 3 +++
 target-moxie/translate.c  | 3 +++
 target-openrisc/translate.c   | 3 +++
 target-ppc/translate.c| 3 +++
 target-s390x/translate.c  | 3 +++
 target-sh4/translate.c| 3 +++
 target-sparc/translate.c  | 2 +-
 target-unicore32/translate.c  | 2 +-
 target-xtensa/translate.c | 3 +++
 14 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index f936d1b..1a2d284 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2917,6 +2917,9 @@ void gen_intermediate_code(CPUAlphaState *env, struct 
TranslationBlock *tb)
 
 if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) {
 gen_excp(&ctx, EXCP_DEBUG, 0);
+/* Advance PC so that clearing the breakpoint will
+   invalidate this TB.  */
+ctx.pc += 4;
 break;
 }
 if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 964845c..460cedd 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3166,6 +3166,9 @@ void gen_intermediate_code(CPUCRISState *env, struct 
TranslationBlock *tb)
 tcg_gen_movi_tl(env_pc, dc->pc);
 t_gen_raise_exception(EXCP_DEBUG);
 dc->is_jmp = DISAS_UPDATE;
+/* Advance PC so that clearing the breakpoint will
+   invalidate this TB.  */
+dc->pc += 2;
 break;
 }
 
diff --git a/target-i386/translate.c b/target-i386/translate.c
index ef10e68..e4c3e7e 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7942,6 +7942,9 @@ void gen_intermediate_code(CPUX86State *env, 
TranslationBlock *tb)
  tb->flags & HF_RF_MASK
  ? BP_GDB : BP_ANY))) {
 gen_debug(dc, pc_ptr - dc->cs_base);
+/* Advance PC so that clearing the breakpoint will
+   invalidate this TB.  */
+pc_ptr += 1;
 goto done_generating;
 }
 if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index c61ad0f..0ade098 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1078,6 +1078,9 @@ void gen_intermediate_code(CPULM32State *env, struct 
TranslationBlock *tb)
 tcg_gen_movi_tl(cpu_pc, dc->pc);
 t_gen_raise_exception(dc, EXCP_DEBUG);
 dc->is_jmp = DISAS_UPDATE;
+/* Advance PC so that clearing the breakpoint will
+   invalidate this TB.  */
+dc->pc += 4;
 break;
 }
 
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 5995cce..93b5d2c 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -3004,6 +3004,9 @@ void gen_intermediate_code(CPUM68KState *env, 
TranslationBlock *tb)
 if (unlikely(cpu_breakpoint_test(cs, dc->pc, BP_ANY))) {
 gen_exception(dc, dc->pc, EXCP_DEBUG);
 dc->is_jmp = DISAS_JUMP;
+/* Advance PC so that clearing the breakpoint will
+   invalidate this TB.  */
+dc->pc += 2;
 break;
 }
 
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index a9c5010..ce76e9e 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1693,6 +1693,9 @@ void gen_intermediate_code(CPUMBState *env, struct 
TranslationBlock *tb)
 if (unlikely(cpu_breakpoint_test(cs, dc->pc, BP_ANY))) {
 t_gen_raise_exception(dc, EXCP_DEBUG);
 dc->is_jmp = DISAS_UPDATE;
+/* Advance PC so that clearing the breakpoint will
+   invalidate this TB.  */
+dc->pc += 4;
 break;
 }
 
diff --git a/target-moxie/translate.c b/target-moxie/translate.c
index f84841e..9fb9082 100644
--- a/target-moxie/translate.c
+++ b/target-moxie/translate.c
@@ -848,6 +848,9 @@ void gen_intermediate_code(CPUMoxieState *env, struct 
TranslationBlock *tb)
 tcg_gen_movi_i32(cpu_pc, ctx.pc);
 gen_helper_debug(cpu_env);
 ctx.bstate = BS_EXCP;
+/* Advance PC so that clearing the breakpoint will
+   invalidate this TB.  */
+ctx.pc += 2;
 goto done_generating;
 }
 
diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
index b66fde1..0932249 100644
--- a/target-openrisc/translate.c
+++ b/target-openrisc/translate.c
@@ -1665,6 +1665,9 @@ voi

Re: [Qemu-devel] [Qemu-block] [PATCH v7 3/5] block: support passing 'backing': '' to 'blockdev-add'

[Jeff Cody]

On Mon, Oct 12, 2015 at 12:16:15PM +0300, Alberto Garcia wrote:
> Passing an empty string allows opening an image but not its backing
> file. This was already described in the API documentation, only the
> implementation was missing.
> 
> This is useful for creating snapshots using images opened with
> blockdev-add, since they are not supposed to have a backing image
> before the operation.
> 
> Signed-off-by: Alberto Garcia 
> Reviewed-by: Max Reitz 
> Reviewed-by: Eric Blake 
> Reviewed-by: Kevin Wolf 
> ---
>  block.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/block.c b/block.c
> index c9e3c6c..31d1b6e 100644
> --- a/block.c
> +++ b/block.c
> @@ -1406,6 +1406,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
> const char *filename,
>  BlockDriverState *file = NULL, *bs;
>  BlockDriver *drv = NULL;
>  const char *drvname;
> +const char *backing;
>  Error *local_err = NULL;
>  int snapshot_flags = 0;
>  
> @@ -1473,6 +1474,12 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
> const char *filename,
>  
>  assert(drvname || !(flags & BDRV_O_PROTOCOL));
>  
> +backing = qdict_get_try_str(options, "backing");
> +if (backing && *backing == '\0') {
> +flags |= BDRV_O_NO_BACKING;
> +qdict_del(options, "backing");
> +}
> +
>  bs->open_flags = flags;
>  bs->options = options;
>  options = qdict_clone_shallow(options);
> -- 
> 2.6.1
> 
> 
Reviewed-by: Jeff Cody 

Re: [Qemu-devel] [Qemu-block] [PATCH v7 1/5] block: check for existing device IDs in external_snapshot_prepare()

[Jeff Cody]

On Mon, Oct 12, 2015 at 12:16:13PM +0300, Alberto Garcia wrote:
> The 'snapshot-node-name' parameter of blockdev-snapshot-sync allows
> setting the node name of the image that is going to be created.
> 
> Before creating the image, external_snapshot_prepare() checks that the
> name is not already being used. The check is however incomplete since
> it only considers existing node names, but node names must not clash
> with device IDs either because they share the same namespace.
> 
> If the user attempts to create a snapshot using the name of an
> existing device for the 'snapshot-node-name' parameter the operation
> will eventually fail, but only after the new image has been created.
> 
> This patch replaces bdrv_find_node() with bdrv_lookup_bs() to extend
> the check to existing device IDs, and thus detect possible name
> clashes before the new image is created.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  blockdev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 4731843..0898d1f 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1570,8 +1570,9 @@ static void 
> external_snapshot_prepare(BlkTransactionState *common,
>  return;
>  }
>  
> -if (has_snapshot_node_name && bdrv_find_node(snapshot_node_name)) {
> -error_setg(errp, "New snapshot node name already existing");
> +if (has_snapshot_node_name &&
> +bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
> +error_setg(errp, "New snapshot node name already in use");
>  return;
>  }
>  
> -- 
> 2.6.1
> 
>

A downfall is we don't communicate back if the collision is with a
device name or a node name, but I guess that is relatively minor (and
probably not worth splitting this into two different calls to
bdrv_lookup_bs() to differentiate).

Reviewed-by: Jeff Cody 

Re: [Qemu-devel] [PATCH v4 3/5] acpi: pc: add fw_cfg device node to ssdt

[Eduardo Habkost]

On Wed, Oct 14, 2015 at 12:18:10AM +0300, Michael S. Tsirkin wrote:
> On Tue, Oct 13, 2015 at 04:10:03PM -0300, Eduardo Habkost wrote:
> > One of the things that may break if guest-visible bits of the machine
> > change is Windows license activation, but the rules Windows use to
> > trigger reactivation aren't very clear.
> 
> They are easy to find on the internet.

I couldn't find them[1]. If you have a pointer to a clear description of
the rules for all Windows versions, I would love to see it.

[1] All I have found were things like:

  "Do I need to activate Windows after making a hardware change?
  Maybe. [...]"
  
http://windows.microsoft.com/en-us/windows/activating-windows-faq#1TC=windows-7

  "Microsoft is characteristically shy about discussing the details of
  activation. [...]"
  
http://www.zdnet.com/article/microsoft-quietly-rewrites-its-activation-rules-for-windows-10/

  "What triggers the need to reactivate Windows? As intended, each
  hardware component gets a relative weight, and from that WGA determines
  whether your copy of Windows 7 needs reactivation. The weight and the
  number of changes is apparently a guarded secret. If you upgrade too
  much at once, WAT decides that your PC is new, and things can get messy.
  The actual algorithm that Microsoft uses is not disclosed, [...]"
  
http://searchitchannel.techtarget.com/feature/How-Windows-7-hardware-upgrades-affect-licensing


-- 
Eduardo

Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add real CDROM menu item

[Programmingkid]

On Oct 13, 2015, at 4:44 PM, Peter Maydell wrote:

> On 26 September 2015 at 04:01, Programmingkid  
> wrote:
>> Add a menu item to the Machine menu called "Use Real CDROM". It gives the 
>> user
>> the ability to use a real CDROM disc with QEMU by simply selecting a menu 
>> item.
>> 
>> Signed-off-by: John Arbuckle 
> 
> This feature is not present in any of our other UI frontends,
> and the implementation makes assumptions about the names of
> block devices in the models (ie that if it has "cd" in the name
> anywhere it's a CD). I also don't have a mac with a CD drive
> so I have no way of testing it.
> 
> I'm afraid that I think that this comes under the heading of
> "features that should be in the VM management layer, not QEMU
> itself", so I would prefer not to take it into QEMU.

Mac OS X users don't have any VM management layer. That is pretty much
a Linux-only feature. What we do have is the cocoa interface. 

Re: [Qemu-devel] [PATCH v4 3/5] acpi: pc: add fw_cfg device node to ssdt

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 04:10:03PM -0300, Eduardo Habkost wrote:
> One of the things that may break if guest-visible bits of the machine
> change is Windows license activation, but the rules Windows use to
> trigger reactivation aren't very clear.

They are easy to find on the internet.

> -- 
> Eduardo

Re: [Qemu-devel] [PATCH] target-arm: Use common CPU cycle infrastructure

[Peter Maydell]

On 24 September 2015 at 20:43, Christopher Covington  
wrote:
> cpu_get_ticks() provides a common interface across targets for
> calculating CPU cycles. Using this fixes PMCCNTR reads when -icount
> is specified (previously a non-increasing value was returned).
>
> Signed-off-by: Christopher Covington 
> ---
>  target-arm/helper.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)

So, I think the conclusion from this thread was that we should
(a) change the default cpu_get_host_ticks() implementation to
call get_clock()
(b) rebase this patch and apply it

Is anybody planning to do that?

In any case, I'm taking this thread off my "must-review" list :-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4] arm_mptimer: Convert to use ptimer

[Dmitry Osipenko]

13.10.2015 18:38, Dmitry Osipenko пишет:

13.10.2015 17:11, Dmitry Osipenko пишет:

+ptimer_set_limit(tb->timer, value, 0);
+ptimer_set_count(tb->timer, value);



Umm... looking more at these lines, I think it might put in a trouble in some
corner case. It might worth trying to make ptimer more flexible rather than to
workaround it's imperfection.



I looked into moving specific trigger/start/stop logic into the ptimer and it 
really doesn't worth churning the ptimer code, since no other device has similar 
requirements (or they are just incomplete/broken). I'll add one-line patch in V5 
for ptimer_set_limit() to make ptimer_get_count() return correct value for the 
disabled timer, so we won't need the above ad-hoc.


--
Dmitry

Re: [Qemu-devel] [PULL 04/26] target-*: Introduce and use cpu_breakpoint_test

[Richard Henderson]

On 10/14/2015 12:40 AM, Sergey Fedorov wrote:

Otherwise, "!(tb_end <= start || tb_start >= end)" condition
check will fail

...

So we either
need to change the condition in tb_invalidate_phys_page_range() or do
the PC advancement trick during translation, no matter can instructions
cross a page boundary or not.


I think you're right.

The reason I've not seen an issue so far is that breakpoint_invalidate expands 
the requested range to the entire page.  So the failure to invalidate would 
require the breakpoint to be at the page boundary.



r~

Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add real CDROM menu item

[Peter Maydell]

On 26 September 2015 at 04:01, Programmingkid  wrote:
> Add a menu item to the Machine menu called "Use Real CDROM". It gives the user
> the ability to use a real CDROM disc with QEMU by simply selecting a menu 
> item.
>
> Signed-off-by: John Arbuckle 

This feature is not present in any of our other UI frontends,
and the implementation makes assumptions about the names of
block devices in the models (ie that if it has "cd" in the name
anywhere it's a CD). I also don't have a mac with a CD drive
so I have no way of testing it.

I'm afraid that I think that this comes under the heading of
"features that should be in the VM management layer, not QEMU
itself", so I would prefer not to take it into QEMU.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] ui/cocoa.m: fix help menus

[Peter Maydell]

On 27 September 2015 at 15:31, Programmingkid  wrote:
> Make the help menus actually work.
>
> Signed-off-by: John Arbuckle 
>
> ---
>  ui/cocoa.m |   20 
>  1 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..2c81785 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -992,16 +992,28 @@ QemuCocoaView *cocoaView;
>  {
>  COCOA_DEBUG("QemuCocoaAppController: showQEMUDoc\n");
>
> -[[NSWorkspace sharedWorkspace] openFile:[NSString 
> stringWithFormat:@"%@/../doc/qemu/qemu-doc.html",
> -[[NSBundle mainBundle] resourcePath]] withApplication:@"Help 
> Viewer"];
> +NSString *path;
> +path = [[NSBundle mainBundle] resourcePath];
> +path = [path stringByDeletingLastPathComponent];
> +path = [NSString stringWithFormat: @"%@/%s", path, "qemu-doc.html"];
> +if([[NSWorkspace sharedWorkspace] openFile: path] == NO) {
> +NSBeep();
> +QEMU_Alert(@"Failed to open file qemu-doc.html!");
> +}

This looks like it changes the place we look for the docs
from "../doc/qemu/" to "../".  The latter will work if you're
running QEMU directly from a build tree, but if you've installed
QEMU via 'make install' then we put docs into
 ${prefix}/share/doc/qemu
and binaries into
 ${prefix}/bin
so we need also to search "../share/doc/qemu/". (I think the
current code is attempting to do the latter but is buggy.
This also doesn't account for users manually passing --docdir
to configure, but I think that's pretty hard to do.)

If you want to add support for finding the docs when running
directly from a build tree then we need to check that search
path as well as the one for an installed setup.

>  }
>
>  - (void)showQEMUTec:(id)sender
>  {
>  COCOA_DEBUG("QemuCocoaAppController: showQEMUTec\n");
>
> -[[NSWorkspace sharedWorkspace] openFile:[NSString 
> stringWithFormat:@"%@/../doc/qemu/qemu-tech.html",
> -[[NSBundle mainBundle] resourcePath]] withApplication:@"Help 
> Viewer"];
> +NSString *path;
> +path = [[NSBundle mainBundle] resourcePath];
> +path = [path stringByDeletingLastPathComponent];
> +path = [NSString stringWithFormat: @"%@/%s", path, "qemu-tech.html"];
> +if([[NSWorkspace sharedWorkspace] openFile: path] == NO) {
> +NSBeep();
> +QEMU_Alert(@"Failed to open file qemu-tech.html!");
> +}
>  }
>
>  /* Stretches video to fit host monitor size */

You now have two lots of identical code which differ only by
what file they're looking for; this would be better refactored
out into a separate function (especially if you want to search
more than one path).

thanks
-- PMM

Re: [Qemu-devel] [PATCH] target-arm: Provide model numbers for Sharp PDAs

[Ryo ONODERA]

Dear Peter Maydell,

From: Peter Maydell , Date: Tue, 13 Oct 2015 18:17:57 
+0100

> On 12 October 2015 at 12:36, Ryo ONODERA  wrote:
>> qemu-system-arm -M ? provides code names for Sharp Zaurus PDAs.
>> The code names are difficult to identify what model is emulated.
>> Please add model numbers to the output of qemu-system-arm -M ?.
>>
>> Ryo ONODERA (1):
>>   target-arm: Provide model numbers for Sharp PDAs
>>
>>  hw/arm/collie.c | 2 +-
>>  hw/arm/spitz.c  | 8 
>>  hw/arm/tosa.c   | 2 +-
>>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> Thanks for this patch; I have applied it to my target-arm.next
> branch.

Thank you very much for your quick response.

> -- PMM

Best Regards,

--
Ryo ONODERA // ryo...@yk.rim.or.jp
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: [Qemu-devel] [PATCH] ui/cocoa.m: blinky mouse cursor fix

[Peter Maydell]

On 27 September 2015 at 02:32, Programmingkid  wrote:
> The mouse cursor can become blinky when being moved a lot. This patch fixes 
> that
> problem by issuing the redraw sooner.
>
> Signed-off-by: John Arbuckle 
>
> ---
>  ui/cocoa.m |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..cf372a4 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1275,6 +1275,7 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
>  NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
>
>  COCOA_DEBUG("qemu_cocoa: cocoa_refresh\n");
> +graphic_hw_update(NULL);
>
>  if (qemu_input_is_absolute()) {
>  if (![cocoaView isAbsoluteEnabled]) {
> @@ -1295,7 +1296,6 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
>  [cocoaView handleEvent:event];
>  }
>  } while(event != nil);
> -graphic_hw_update(NULL);
>  [pool release];
>  }
>
> --
> 1.7.5.4

Thanks, applied to cocoa.next. (This brings us into line with
the other UI front ends which update the graphics first and
process events second.)

-- PMM

Re: [Qemu-devel] [PATCH] ui/cocoa.m: addRemovableDevicesMenuItems() warning fix

[Peter Maydell]

On 25 September 2015 at 23:44, Programmingkid  wrote:
> Eliminate this warning associated with the addRemovableDevicesMenuItems()
> function:
>
> ui/cocoa.m:1344:13: warning: function declaration isn't a prototype
> [-Wstrict-prototypes]
>  static void addRemovableDevicesMenuItems()
>  ^
> ui/cocoa.m: In function 'addRemovableDevicesMenuItems':
> ui/cocoa.m:1344:13: warning: old-style function definition 
> [-Wold-style-definition]
>
>
> Signed-off-by: John Arbuckle 
>
> ---
>  ui/cocoa.m |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..a94cc68 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1341,7 +1341,7 @@ static void add_console_menu_entries(void)
>  /* Make menu items for all removable devices.
>   * Each device is given an 'Eject' and 'Change' menu item.
>   */
> -static void addRemovableDevicesMenuItems()
> +static void addRemovableDevicesMenuItems(void)
>  {
>  NSMenu *menu;
>  NSMenuItem *menuItem;
> --
> 1.7.5.4
>

Thanks, applied to cocoa.next.

-- PMM

Re: [Qemu-devel] [PATCH] ui/cocoa.m: eliminate normalWindow warning

[Peter Maydell]

On 25 September 2015 at 23:34, Programmingkid  wrote:
> Eliminate this warning associated with the setting of the normalWindow's 
> title:
>
> ui/cocoa.m: In function '-[QemuCocoaAppController init]':
> ui/cocoa.m:888:9: warning: format not a string literal and no format arguments
>  [-Wformat-security]
>  [normalWindow setTitle:[NSString stringWithFormat:@"QEMU"]];
>
>
> Signed-off-by: John Arbuckle 
>
> ---
>  ui/cocoa.m |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..fa21fdb 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -855,7 +855,7 @@ QemuCocoaView *cocoaView;
>  exit(1);
>  }
>  [normalWindow setAcceptsMouseMovedEvents:YES];
> -[normalWindow setTitle:[NSString stringWithFormat:@"QEMU"]];
> +[normalWindow setTitle:@"QEMU"];
>  [normalWindow setContentView:cocoaView];
>  #if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_10)
>  [normalWindow useOptimizedDrawing:YES];
> --
> 1.7.5.4

Thanks, applied to cocoa.next (sorry it's taken me a while
to get back to these).

-- PMM

[Qemu-devel] [PULL v3 49/51] ivshmem: use kvm irqfd for msi notifications

[marcandre . lureau]

From: Marc-André Lureau 

Use irqfd for improving context switch when notifying the guest.
If the host doesn't support kvm irqfd, regular msi notifications are
still supported.

Note: the ivshmem implementation doesn't allow switching between MSI and
IO interrupts, this patch doesn't either.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Paolo Bonzini 
---
 hw/misc/ivshmem.c | 180 --
 1 file changed, 174 insertions(+), 6 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 8581d43..7c7c80d 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -19,6 +19,7 @@
 #include "hw/hw.h"
 #include "hw/i386/pc.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
 #include "sysemu/kvm.h"
 #include "migration/migration.h"
@@ -68,6 +69,7 @@ typedef struct Peer {
 
 typedef struct MSIVector {
 PCIDevice *pdev;
+int virq;
 } MSIVector;
 
 typedef struct IVShmemState {
@@ -293,13 +295,73 @@ static void fake_irqfd(void *opaque, const uint8_t *buf, 
int size) {
 msix_notify(pdev, vector);
 }
 
+static int ivshmem_vector_unmask(PCIDevice *dev, unsigned vector,
+ MSIMessage msg)
+{
+IVShmemState *s = IVSHMEM(dev);
+EventNotifier *n = &s->peers[s->vm_id].eventfds[vector];
+MSIVector *v = &s->msi_vectors[vector];
+int ret;
+
+IVSHMEM_DPRINTF("vector unmask %p %d\n", dev, vector);
+
+ret = kvm_irqchip_update_msi_route(kvm_state, v->virq, msg);
+if (ret < 0) {
+return ret;
+}
+
+return kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, v->virq);
+}
+
+static void ivshmem_vector_mask(PCIDevice *dev, unsigned vector)
+{
+IVShmemState *s = IVSHMEM(dev);
+EventNotifier *n = &s->peers[s->vm_id].eventfds[vector];
+int ret;
+
+IVSHMEM_DPRINTF("vector mask %p %d\n", dev, vector);
+
+ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n,
+s->msi_vectors[vector].virq);
+if (ret != 0) {
+error_report("remove_irqfd_notifier_gsi failed");
+}
+}
+
+static void ivshmem_vector_poll(PCIDevice *dev,
+unsigned int vector_start,
+unsigned int vector_end)
+{
+IVShmemState *s = IVSHMEM(dev);
+unsigned int vector;
+
+IVSHMEM_DPRINTF("vector poll %p %d-%d\n", dev, vector_start, vector_end);
+
+vector_end = MIN(vector_end, s->vectors);
+
+for (vector = vector_start; vector < vector_end; vector++) {
+EventNotifier *notifier = &s->peers[s->vm_id].eventfds[vector];
+
+if (!msix_is_masked(dev, vector)) {
+continue;
+}
+
+if (event_notifier_test_and_clear(notifier)) {
+msix_set_pending(dev, vector);
+}
+}
+}
+
 static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier 
*n,
   int vector)
 {
 /* create a event character device based on the passed eventfd */
 IVShmemState *s = opaque;
-CharDriverState * chr;
+PCIDevice *pdev = PCI_DEVICE(s);
 int eventfd = event_notifier_get_fd(n);
+CharDriverState *chr;
+
+s->msi_vectors[vector].pdev = pdev;
 
 chr = qemu_chr_open_eventfd(eventfd);
 
@@ -484,6 +546,58 @@ static bool fifo_update_and_get(IVShmemState *s, const 
uint8_t *buf, int size,
 return true;
 }
 
+static int ivshmem_add_kvm_msi_virq(IVShmemState *s, int vector)
+{
+PCIDevice *pdev = PCI_DEVICE(s);
+MSIMessage msg = msix_get_message(pdev, vector);
+int ret;
+
+IVSHMEM_DPRINTF("ivshmem_add_kvm_msi_virq vector:%d\n", vector);
+
+if (s->msi_vectors[vector].pdev != NULL) {
+return 0;
+}
+
+ret = kvm_irqchip_add_msi_route(kvm_state, msg);
+if (ret < 0) {
+error_report("ivshmem: kvm_irqchip_add_msi_route failed");
+return -1;
+}
+
+s->msi_vectors[vector].virq = ret;
+s->msi_vectors[vector].pdev = pdev;
+
+return 0;
+}
+
+static void setup_interrupt(IVShmemState *s, int vector)
+{
+EventNotifier *n = &s->peers[s->vm_id].eventfds[vector];
+bool with_irqfd = kvm_msi_via_irqfd_enabled() &&
+ivshmem_has_feature(s, IVSHMEM_MSI);
+PCIDevice *pdev = PCI_DEVICE(s);
+
+IVSHMEM_DPRINTF("setting up interrupt for vector: %d\n", vector);
+
+if (!with_irqfd) {
+IVSHMEM_DPRINTF("with eventfd");
+s->eventfd_chr[vector] = create_eventfd_chr_device(s, n, vector);
+} else if (msix_enabled(pdev)) {
+IVSHMEM_DPRINTF("with irqfd");
+if (ivshmem_add_kvm_msi_virq(s, vector) < 0) {
+return;
+}
+
+if (!msix_is_masked(pdev, vector)) {
+kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL,
+   s->msi_vectors[vector].virq);
+}
+} else {
+/* it will be delayed until msix is enabled, in write_config */
+IVSHMEM_DPRINTF("with

Re: [Qemu-devel] [PATCH v8 15/18] qapi: Move duplicate member checks to schema check()

[Eric Blake]

On 10/13/2015 02:17 PM, Eric Blake wrote:

 I can take PATCH 01-09,12 into my tree right away, with PATCH 07's two
 redundant is_implicit() methods dropped, and PATCH 12's comment touched
 up.
>>>
>>> Okay.
>>
>> Done & pushed to http://repo.or.cz/qemu/armbru.git branch qapi-next.
> 
> I didn't see any mentioned changes on patch 7, at least not in commit
> 4ad5066.  Last paragraph of the commit message would also need a
> massage, if you do want to squash it in:

And of course, if you move the hunks out of 7, then they DO need to be
added back into 12, since it is 12 that provides non-None info on
implicit types (not seen in commit 20bfea1).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v8 15/18] qapi: Move duplicate member checks to schema check()

[Eric Blake]

On 10/13/2015 12:32 PM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> On 10/13/2015 11:13 AM, Markus Armbruster wrote:
>>
>
> I've come to the conclusion that we should get rid of the self-inflicted
> pain before we attempt to detect all collisions.

 Then that sounds like I should try harder to get the kind/type naming,
 the boxed base naming, and even the anonymous union naming all hoisted
 into this subset, and spin a v9?
>>>
>>> I can take PATCH 01-09,12 into my tree right away, with PATCH 07's two
>>> redundant is_implicit() methods dropped, and PATCH 12's comment touched
>>> up.
>>
>> Okay.
> 
> Done & pushed to http://repo.or.cz/qemu/armbru.git branch qapi-next.

I didn't see any mentioned changes on patch 7, at least not in commit
4ad5066.  Last paragraph of the commit message would also need a
massage, if you do want to squash it in:

Instead, add an is_implicit() method to QAPISchemaEntity, and use it.
It can be overridden later for ObjectType and EnumType, when implicit
instances of those classes gain info.

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e263ecf..d7cf0f3 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -903,10 +903,6 @@ class QAPISchemaEnumType(QAPISchemaType):
 def check(self, schema):
 assert len(set(self.values)) == len(self.values)

-def is_implicit(self):
-# See QAPISchema._make_implicit_enum_type()
-return self.name[-4:] == 'Kind'
-
 def c_type(self, is_param=False):
 return c_name(self.name)

@@ -977,10 +973,6 @@ class QAPISchemaObjectType(QAPISchemaType):
 self.variants.check(schema, members, seen)
 self.members = members

-def is_implicit(self):
-# See QAPISchema._make_implicit_object_type()
-return self.name[0] == ':'
-
 def c_name(self):
 assert not self.is_implicit()
 return QAPISchemaType.c_name(self)


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] Add mp-affinity property for ARM CPU class

[Peter Crosthwaite]

On Tue, Oct 13, 2015 at 10:54 AM, Peter Maydell
 wrote:
> On 9 October 2015 at 10:52, Pavel Fedin  wrote:
>> This allows to override default affinity IDs on a per-machine basis, and
>> possibility to retrieve IDs will be used by vGICv3 live migration code.
>>
>> Signed-off-by: Pavel Fedin 
>> ---
>> Since this popped up several times on the mailing list, i decided to publish 
>> this early.
>> ---
>>  target-arm/cpu.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> index b48da33..3ebcebe 100644
>> --- a/target-arm/cpu.c
>> +++ b/target-arm/cpu.c
>> @@ -1379,6 +1379,7 @@ static Property arm_cpu_properties[] = {
>>  DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
>>  DEFINE_PROP_UINT32("psci-conduit", ARMCPU, psci_conduit, 0),
>>  DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0),
>> +DEFINE_PROP_UINT64("mp-affinity", ARMCPU, mp_affinity, 0),

Can we have separate props for each affinity level? Are you assuming
MPIDR format for this?

Regards,
Peter

>>  DEFINE_PROP_END_OF_LIST()
>>  };
>
> Nothing wrong with this patch, but I'd rather add it as part of
> the series which actually uses it. (I see you have it in one of
> your GICv3 patchsets.)
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH] target-arm: Provide model numbers for Sharp PDAs

[Peter Maydell]

On 12 October 2015 at 12:36, Ryo ONODERA  wrote:
> qemu-system-arm -M ? provides code names for Sharp Zaurus PDAs.
> The code names are difficult to identify what model is emulated.
> Please add model numbers to the output of qemu-system-arm -M ?.
>
> Ryo ONODERA (1):
>   target-arm: Provide model numbers for Sharp PDAs
>
>  hw/arm/collie.c | 2 +-
>  hw/arm/spitz.c  | 8 
>  hw/arm/tosa.c   | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)

Thanks for this patch; I have applied it to my target-arm.next
branch.

-- PMM

[Qemu-devel] [PATCHv17/8] [trivial] Track when QEMU has finished initialization

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova 
---
 Makefile.objs|2 +-
 bsd-user/main.c  |1 +
 include/qemu-common.h|3 +++
 linux-user/main.c|1 +
 qemu-common.c|   14 ++
 stubs/Makefile.objs  |1 +
 stubs/qemu-common-stub.c |   21 +
 vl.c |2 ++
 8 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 qemu-common.c
 create mode 100644 stubs/qemu-common-stub.c

diff --git a/Makefile.objs b/Makefile.objs
index bc43e5c..e1d7b25 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -2,7 +2,7 @@
 # Common libraries for tools and emulators
 stub-obj-y = stubs/
 util-obj-y = util/ qobject/ qapi/
-util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o
+util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o 
qemu-common.o
 
 ###
 # block-obj-y is code used by both qemu system emulation and qemu-img
diff --git a/bsd-user/main.c b/bsd-user/main.c
index f0a1268..43f85e6 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -1121,6 +1121,7 @@ int main(int argc, char **argv)
 gdbserver_start (gdbstub_port);
 gdb_handlesig(cpu, 0);
 }
+qemu_initialized = true;
 cpu_loop(env);
 /* never exits */
 return 0;
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 0bd212b..33f6909 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -35,6 +35,9 @@
 # error Unknown pointer size
 #endif
 
+/* Whether the system has finished initializing */
+extern bool qemu_initialized;
+
 void cpu_ticks_init(void);
 
 /* icount */
diff --git a/linux-user/main.c b/linux-user/main.c
index 1f60ff2..9bd2912 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4663,6 +4663,7 @@ int main(int argc, char **argv, char **envp)
 }
 gdb_handlesig(cpu, 0);
 }
+qemu_initialized = true;
 cpu_loop(env);
 /* never exits */
 return 0;
diff --git a/qemu-common.c b/qemu-common.c
new file mode 100644
index 000..4b5ca1a
--- /dev/null
+++ b/qemu-common.c
@@ -0,0 +1,14 @@
+/*
+ * Common symbol definitions for all tools and targets.
+ *
+ * Copyright (C) 2011-2015 Lluís Vilanova 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+*/
+
+#include "qemu-common.h"
+
+
+/* Whether QEMU has been initialized. */
+bool qemu_initialized;
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 85e4e81..259504f 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -39,3 +39,4 @@ stub-obj-y += cpus.o
 stub-obj-y += kvm.o
 stub-obj-y += qmp_pc_dimm_device_list.o
 stub-obj-y += target-monitor-defs.o
+stub-obj-y += qemu-common-stub.o
diff --git a/stubs/qemu-common-stub.c b/stubs/qemu-common-stub.c
new file mode 100644
index 000..f34f019
--- /dev/null
+++ b/stubs/qemu-common-stub.c
@@ -0,0 +1,21 @@
+/*
+ * Common symbol definitions for all tools and targets.
+ *
+ * Copyright (C) 2011-2015 Lluís Vilanova 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+*/
+
+#include "qemu-common.h"
+#include 
+
+
+static void do_qemu_initialized_init(void)
+{
+/* Tools always consider QEMU as inited */
+qemu_initialized = true;
+}
+
+/* Block is always inited on both tools (and targets) */
+block_init(do_qemu_initialized_init);
diff --git a/vl.c b/vl.c
index f2bd8d2..aae8d47 100644
--- a/vl.c
+++ b/vl.c
@@ -4637,6 +4637,8 @@ int main(int argc, char **argv, char **envp)
 }
 }
 
+qemu_initialized = true;
+
 main_loop();
 bdrv_close_all();
 pause_all_vcpus();

Re: [Qemu-devel] [PATCH] target-i386: allow any alignment for SMBASE

[Eduardo Habkost]

On Tue, Oct 13, 2015 at 09:42:50PM +0200, Paolo Bonzini wrote:
> 
> 
> On 13/10/2015 21:30, Eduardo Habkost wrote:
> > Yeah, the shutdown behavior was never implemented before, so now we are
> > just deviating from the documented Pentium & 486 behavior in a different
> > (and less surprising?) way.
> > 
> >> > 
> >> > With the typo fix in the Cc tag:
> >> > 
> >> > Reviewed-by: Laszlo Ersek 
> > Reviewed-by: Eduardo Habkost 
> 
> Eduardo, it's okay if you take it through your x86 branch; it's not
> super urgent.

Applied. Thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCHv18/8] trace: [tcg] Add per-vCPU tracing states for events with the 'vcpu' property

[Lluís Vilanova]

Eric Blake writes:

> On 10/13/2015 11:11 AM, Lluís Vilanova wrote:
[...]
>> +##
>> +{ 'command': 'trace-event-get-cpu-state',
>> +  'data': {'name': 'str', 'vcpu': 'int'},
>> +  'returns': ['TraceEventInfo'] }
>> +
>> +##
>> +# @trace-event-set-cpu-state:
>> +#
>> +# Set the dynamic state of events in a given vCPU.
>> +#
>> +# @name: Event name pattern.
>> +# @vcpu: The vCPU to act upon.
>> +# @enable: Whether to enable tracing.
>> +# @ignore-unavailable: #optional Do not match unavailable events with @name.
>> +#
>> +# Since 2.2

> 2.5, not 2.2

Old patches :)

>> +##
>> +{ 'command': 'trace-event-set-cpu-state',
>> +  'data': {'name': 'str', 'vcpu': 'int', 'enable': 'bool', 
>> '*ignore-unavailable': 'bool'} }

> This looks identical to trace-event-set-state, except that it now has a
> 'vcpu':'int' argument.  Would it be any simpler to just modify the
> existing command:

> ##
> # @trace-event-set-state:
> ...
> # @vcpu: #optional If provided, limit the state change to the given vcpu
> (default act on all vcpus) (since 2.5)
> #
> # Since 2.2
> ##
>  { 'command': 'trace-event-set-state',
>'data': {'name': 'str', 'enable': 'bool',
> '*vcpu': 'int', '*ignore-unavailable': 'bool'} }

Hmmm, this certainly minimizes the interface size, but in exchange breaks the
symmetry between trace-event-get-state/trace-event-set-state and
trace-event-get-cpu-state/trace-event-set-cpu-state, and the symmetry between
these commands and the function inside QEMU (this last one probably not very
important).

What I could do is also merge trace-event-get-state and
trace-event-get-cpu-state into a single command with an optional cpu.

This reminds me that the meaning of "get-state" for per-vCPU events is somewhat
tricky in the current implementation. There's a per-event boolean with the
system-wide tracing state, and a per-cpu-and-event boolean for the per-CPU
tracing state. The get/set-event functions let you interact with type of
booleans separately.

If anyone sees it useful, I can make it a bit easier to understand: if no CPU
number is provided, get-state returns true if it's set on any CPU, and set-state
sets the state on all CPUs by default.


Thanks,
  Lluis

-- 
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth

Re: [Qemu-devel] [PATCH] target-i386: allow any alignment for SMBASE

[Paolo Bonzini]

On 13/10/2015 21:30, Eduardo Habkost wrote:
> Yeah, the shutdown behavior was never implemented before, so now we are
> just deviating from the documented Pentium & 486 behavior in a different
> (and less surprising?) way.
> 
>> > 
>> > With the typo fix in the Cc tag:
>> > 
>> > Reviewed-by: Laszlo Ersek 
> Reviewed-by: Eduardo Habkost 

Eduardo, it's okay if you take it through your x86 branch; it's not
super urgent.

Paolo

Re: [Qemu-devel] [PATCH v8 17/18] qapi: Add test for alternate branch 'kind' clash

[Eric Blake]

On 10/13/2015 12:43 PM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Rename alternate-clash to alternate-clash-members, and add a
>> new test alternate-clash-type.  While similar to the earlier
>> addition of union-clash-type, we have one major difference: a
>> future patch will be simplifying alternates to not need an
>> implict AlternateKind enum, but we still need to detect the
>> collision with the resulting C 'qtype_code type;' tag.
> 
> You're alluding to a future change of the generated code from
> 
> struct BlockdevRef {
> BlockdevRefKind kind;
> union { /* union tag is @kind */
> void *data;
> BlockdevOptions *definition;
> char *reference;
> };
> };
> 
> to
> 
> struct BlockdevRef {
> qtype_code type;
> union { /* union tag is @type */
> void *data;
> BlockdevOptions *definition; /* QTYPE_QDICT */
> char *reference; /* QTYPE_QSTRING */
> };
> };
> 
> right?

Yes.

> 
> I don't think that affects collision checking at all.  Both before and
> after, we have a tag member, and we need to check for collisions with
> its name.

I guess I wrote that at one point where I was using
alternate.variants.tag_member = None for alternates; but in the
meantime, I've reworked things to just use a special subclass of
QAPISchemaObjectTypeMember instead, at which point normal collision
checking still works.  So yeah, I can probably simplify or drop wording
here.

>> +++ b/tests/qapi-schema/alternate-clash-type.json
>> @@ -0,0 +1,10 @@
>> +# Alternate branch 'type'
>> +# Reject this, because we would have a clash in generated C, between the
>> +# alternate's implicit tag member 'kind' and the branch name 'kind'
>> +# within the alternate.
>> +# TODO: Even if alternates are simplified in the future to use a simpler
>> +# 'qtype_code type' tag, rather than a full QAPISchemaObjectTypeMember,
>> +# we must still flag the collision, or else munge the generated C branch
>> +# names to allow compilation.
> 
> I don't think there's a TODO here.

Again, probably leftover text from my first implementation.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v8 16/18] qapi: Move duplicate enum value checks to schema check()

[Eric Blake]

On 10/13/2015 12:35 PM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Similar to the previous commit, move the detection of a collision
>> in enum values from parse time to QAPISchemaEnumType.check().
>> This happens to also detect collisions in union branch names,
>> so for a decent error message, we have to determine if the enum
>> is implicit (and if so where the real collision lies).
>>
>> Testing this showed that the test union-bad-branch and
>> union-clash-branches were basically testing the same thing;
>> with the minor difference that the former clashes only in the
>> enum, while the latter also clashes in the C union member
>> names that would be generated. So delete the weaker test.
>>
>> No change to generated code.
>>
>> Signed-off-by: Eric Blake 
>>

>> +++ b/scripts/qapi.py
>> @@ -527,7 +527,6 @@ def check_union(expr, expr_info):
>>  base = expr.get('base')
>>  discriminator = expr.get('discriminator')
>>  members = expr['data']
>> -values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
> 
> Stupid / tired question: I can see 'MAX' in the new code further down,
> but I can't see 'KIND'.  Why?  Was it perhaps covered by the previous
> patch?

MAX is covered by enum collisions.  'kind' is covered by the previous
patch's non-variant vs. branch-name collisions.

Hmm, maybe when respinning this I can simplify 15/18 to drop KIND from
this spot (because KIND is not part of the enum).


>>
>>  def check(self, schema):
>> -assert len(set(self.values)) == len(self.values)
>> +# Check for collisions on the generated C enum values
>> +seen = {c_enum_const(self.name, 'MAX'): '(automatic MAX)'}
>> +for value in self.values:
>> +c_value = c_enum_const(self.name, value)
>> +if c_value in seen:
>> +# If the enum is implicit, report the error on behalf of
>> +# the union or alternate that triggered the enum
>> +if self.is_implicit():
>> +owner = schema.lookup_type(self.name[:-4])
>> +assert owner
>> +if isinstance(owner, QAPISchemaAlternateType):
>> +description = "Alternate '%s' branch" % owner.name
>> +else:
>> +description = "Union '%s' branch" % owner.name
>> +else:
>> +description = "Enum '%s' value" % self.name
> 
> Computing a reasonable description distracts from the checking job.
> Suggest to outline this into a private method.

Good idea.

>> +++ b/tests/qapi-schema/alternate-clash.err
>> @@ -1 +1 @@
>> -tests/qapi-schema/alternate-clash.json:7: Alternate 'Alt1' member 'a_b' 
>> clashes with 'a-b'
>> +tests/qapi-schema/alternate-clash.json:7: Alternate 'Alt1' branch 'a_b' 
>> clashes with 'a-b'
> 
> Our terminology isn't consistent: we use both "branch" and "case".  Not
> this patch's problem to fix.
> 
>> diff --git a/tests/qapi-schema/enum-clash-member.err 
>> b/tests/qapi-schema/enum-clash-member.err
>> index 48bd136..84030c5 100644
>> --- a/tests/qapi-schema/enum-clash-member.err
>> +++ b/tests/qapi-schema/enum-clash-member.err
>> @@ -1 +1 @@
>> -tests/qapi-schema/enum-clash-member.json:2: Enum 'MyEnum' member 'ONE' 
>> clashes with 'one'
>> +tests/qapi-schema/enum-clash-member.json:2: Enum 'MyEnum' value 'ONE' 
>> clashes with 'one'
> 
> I actually prefer calling ONE a member of MyEnum, because that leaves
> value for its actual value.
> 
> For what it's worth, the C standard also talks about "members of an
> enumeration".

Easy enough to fix.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] target-i386: allow any alignment for SMBASE

[Eduardo Habkost]

On Mon, Oct 12, 2015 at 07:32:17PM +0200, Laszlo Ersek wrote:
> On 10/12/15 18:25, Paolo Bonzini wrote:
> > Processors up to the Pentium (says Bochs---I do not have old enough
> > manuals) require a 32KiB alignment for the SMBASE, but newer processors
> > do not need that, and Tiano Core will use non-aligned SMBASE values.
> 
> The Quark package has the following comment & code:
> 
>   //
>   // Allocate buffer for all of the tiles.
>   // For 486/Pentium, the SMBASE (and hence the buffer) must be aligned on a 
> 32KB boundary
>   //
>   Buffer = AllocatePages (EFI_SIZE_TO_PAGES (SIZE_64KB + TileSize * 
> (gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus - 1)));
>   Buffer = (VOID *)(((UINTN)Buffer & 0x8000) + SIZE_32KB);
> 
> whereas Mike's patch
> 
> has:
> 
>   // Allocate buffer for all of the tiles.
>   //
>   // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
>   // Volume 3C, Section 34.11 SMBASE Relocation
>   // For 486(FamilyId:4)/Pentium(FamilyId:5), the SMBASE (and hence the 
> buffer)
>   // must be aligned on a 32KB boundary
>   //
>   if ((FamilyId == 4) || (FamilyId == 5)) {
> Buffer = AllocateAlignedPages (EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * 
> (mMaxNumberOfCpus - 1)), SIZE_32KB);
>   } else {
> Buffer = AllocatePages (EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * 
> (mMaxNumberOfCpus - 1)));
>   }
> 
> The most recent Intel SDM I have on my disk (the unified mammoth
> edition) is "325462-054US, April 2015"; the section that Mike marked
> above states the following (parentheses theirs):
> 
>   (For Pentium and Intel486 processors, the SMBASE values must be
>   aligned on a 32-KByte boundary or the processor will enter shutdown
>   state during the execution of a RSM instruction.)

An old Pentium manual I have here says the same:

"The Pentium processor (510\60, 567\66) provides a control register,
SMBASE. The address space used as SMRAM can be modified by changing the
SMBASE register before exiting an SMI handler routine. SMBASE can be
changed to any 32K aligned value (values that are not 32K aligned will
cause the CPU to enter the shutdown state when executing the RSM
instruction). SMBASE is set to the default value of 3H on RESET, but
is not changed on INIT. If the SMBASE register is changed during an SMM
handler, all following SMI# requests will initiate a state save at the
new SMBASE."

> 
> > Reported-by: Michael D Kinney 
> > Cc: Laszlo Ersek 
> 
> This is a typo, please fix it up as
> 
> Cc: Laszlo Ersek 
> 
> > Cc: Jordan Justen 
> > Cc: Eduardo Habkost 
> > Signed-off-by: Paolo Bonzini 
> > ---
> >  target-i386/smm_helper.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-i386/smm_helper.c b/target-i386/smm_helper.c
> > index 02e24b9..c272a98 100644
> > --- a/target-i386/smm_helper.c
> > +++ b/target-i386/smm_helper.c
> > @@ -266,7 +266,7 @@ void helper_rsm(CPUX86State *env)
> >
> >  val = x86_ldl_phys(cs, sm_state + 0x7efc); /* revision ID */
> >  if (val & 0x2) {
> > -env->smbase = x86_ldl_phys(cs, sm_state + 0x7f00) & ~0x7fff;
> > +env->smbase = x86_ldl_phys(cs, sm_state + 0x7f00);
> >  }
> 
> This is under #ifdef TARGET_X86_64, so I guess it's right to remove the
> masking unconditionally -- I think that because I believe the
> intersection of TARGET_X86_64 and (FamilyId == 4 || FamilyId == 5)
> should be empty. (In particular, the save state area in this branch is
> laid out according to the AMD definition, not the Intel one, so the
> Intel SDM is not normative here anyway.)
> 
> >  #else
> >  cpu_x86_update_cr0(env, x86_ldl_phys(cs, sm_state + 0x7ffc));
> > @@ -319,7 +319,7 @@ void helper_rsm(CPUX86State *env)
> >
> >  val = x86_ldl_phys(cs, sm_state + 0x7efc); /* revision ID */
> >  if (val & 0x2) {
> > -env->smbase = x86_ldl_phys(cs, sm_state + 0x7ef8) & ~0x7fff;
> > +env->smbase = x86_ldl_phys(cs, sm_state + 0x7ef8);
> >  }
> >  #endif
> >  if ((env->hflags2 & HF2_SMM_INSIDE_NMI_MASK) == 0) {
> >
> 
> I first thought it would be better to keep the mask for (FamilyId == 4
> || FamilyId == 5). But, since we don't "enter shutdown state" even if
> the alignment is incorrect (i.e., we don't emulate the letter of the SDM
> for incorrect guest code anyway), and the patch retains behavior for
> correct guest code, I think this branch should be fine as well.

Yeah, the shutdown behavior was never implemented before, so now we are
just deviating from the documented Pentium & 486 behavior in a different
(and less surprising?) way.

> 
> With the typo fix in the Cc tag:
> 
> Reviewed-by: Laszlo Ersek 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 1/4] util - add automated ID generation utility

[Programmingkid]

On Oct 13, 2015, at 11:26 AM, Markus Armbruster wrote:

> Jeff Cody  writes:
> 
>> On Tue, Oct 13, 2015 at 09:37:29AM +0200, Markus Armbruster wrote:
>>> Jeff Cody  writes:
>>> 
 Multiple sub-systems in QEMU may find it useful to generate IDs
 for objects that a user may reference via QMP or HMP.  This patch
 presents a standardized way to do it, so that automatic ID generation
 follows the same rules.
 
 This patch enforces the following rules when generating an ID:
 
 1.) Guarantee no collisions with a user-specified ID
 2.) Identify the sub-system the ID belongs to
 3.) Guarantee of uniqueness
 4.) Spoiling predictability, to avoid creating an assumption
of object ordering and parsing (i.e., we don't want users to think
they can guess the next ID based on prior behavior).
 
 The scheme for this is as follows (no spaces):
 
# subsys D RR
 Reserved char --||   | |
 Subsystem String |   | |
 Unique number (64-bit) --| |
 Two-digit random number ---|
 
 For example, a generated node-name for the block sub-system may look
 like this:
 
#block076
 
 The caller of id_generate() is responsible for freeing the generated
 node name string with g_free().
 
 Reviewed-by: John Snow 
 Reviewed-by: Eric Blake 
 Reviewed-by: Alberto Garcia 
 Signed-off-by: Jeff Cody 
 ---
 include/qemu-common.h |  8 
 util/id.c | 37 +
 2 files changed, 45 insertions(+)
 
 diff --git a/include/qemu-common.h b/include/qemu-common.h
 index 0bd212b..2f74540 100644
 --- a/include/qemu-common.h
 +++ b/include/qemu-common.h
 @@ -246,6 +246,14 @@ int64_t qemu_strtosz_suffix_unit(const char *nptr, 
 char **end,
 #define STR_OR_NULL(str) ((str) ? (str) : "null")
 
 /* id.c */
 +
 +typedef enum IdSubSystems {
 +ID_QDEV,
>>> 
>>> ID_QDEV is not used in this series.  Do you intend to use it in a
>>> followup-series?  Can we reasonably expect that series will be accepted?
>>> 
>> 
>> John Arbuckle has a patch on list that uses it.  I haven't reviewed
>> it, however - but I guess it depends ultimately on whether qdev will
>> allow autogeneration for its IDs or not.
> 
> Then that patch should add ID_QDEV.
> 
>>> You could sidestep these questions by making id_generate() take a string
>>> argument ;)
>>> 
>> 
>> I'd rather avoid having each system specifying a string inline in
>> their code.  It is cleaner to have the strings defined in a central
>> location, I think (not to mention, easier to reference).

I can see the benefit of using a string. The id_generate() function
could use va_args like printf() uses to allow almost any kind of 
string argument. An empty string argument could mean to default to
ID_MAX. But I also think using an enumeration is good enough, so 
either way is good.

Re: [Qemu-devel] [PATCH v4 3/5] acpi: pc: add fw_cfg device node to ssdt

[Eduardo Habkost]

On Sat, Oct 10, 2015 at 12:00:16AM -0400, Gabriel L. Somlo wrote:
> On Thu, Oct 01, 2015 at 01:33:50PM +0200, Igor Mammedov wrote:
> > On Thu, 1 Oct 2015 10:27:15 +0200
> > Laszlo Ersek  wrote:
> > 
> > > On 10/01/15 09:02, Igor Mammedov wrote:
> > > > On Sun, 27 Sep 2015 17:29:00 -0400
> > > > "Gabriel L. Somlo"  wrote:
> > > > 
> > > >> Add a fw_cfg device node to the ACPI SSDT, on machine types
> > > >> pc-*-2.5 and up. While the guest-side BIOS can't utilize
> > > >> this information (since it has to access the hard-coded
> > > >> fw_cfg device to extract ACPI tables to begin with), having
> > > >> fw_cfg listed in ACPI will help the guest kernel keep a more
> > > >> accurate inventory of in-use IO port regions.
> > > >>
> > > >> Signed-off-by: Gabriel Somlo 
> > > >> ---
> > > >>  hw/i386/acpi-build.c | 23 +++
> > > >>  hw/i386/pc_piix.c|  1 +
> > > >>  hw/i386/pc_q35.c |  1 +
> > > >>  include/hw/i386/pc.h |  1 +
> > > >>  4 files changed, 26 insertions(+)
> > > >>
> > > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > >> index 95e0c65..ece2710 100644
> > > >> --- a/hw/i386/acpi-build.c
> > > >> +++ b/hw/i386/acpi-build.c
> > > >> @@ -906,6 +906,7 @@ build_ssdt(GArray *table_data, GArray *linker,
> > > >> PcPciInfo *pci, PcGuestInfo *guest_info)
> > > >>  {
> > > >>  MachineState *machine = MACHINE(qdev_get_machine());
> > > >> +PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> > > >>  uint32_t nr_mem = machine->ram_slots;
> > > >>  unsigned acpi_cpus = guest_info->apic_id_limit;
> > > >>  Aml *ssdt, *sb_scope, *scope, *pkg, *dev, *method, *crs, *field, 
> > > >> *ifctx;
> > > >> @@ -1071,6 +1072,28 @@ build_ssdt(GArray *table_data, GArray *linker,
> > > >>  aml_append(scope, aml_name_decl("_S5", pkg));
> > > >>  aml_append(ssdt, scope);
> > > >>  
> > > >> +if (!pcmc->acpi_no_fw_cfg_node) {
> > > > we don't really care about SSDT size changes since during
> > > > migration ACPI blobs will be migrated from source, so
> > > > machine compat bloat is excessive here. It would be better
> > > > to just remove it.

What about non-live migration?

> > > 
> > > This was Eduardo's suggestion, if I recall correctly:
> > > 
> > > http://thread.gmane.org/gmane.comp.emulators.qemu/361930/focus=361983
> > > 
> > > The idea being, if you move a guest from older QEMU to newer QEMU, but
> > > keep the machine type (or more precisely, the full machine config, like
> > > the domain XML) intact, the ACPI device node should not appear out of
> > > the blue.
> > This ACPI device is an always used resource declaration regardless
> > of machine type so it makes sense to tell guest about used resource.
> > 
> > The only reason for machine compat code would be if guest suddenly
> > started to ask for a driver but as Gabriel showed with _STA(0xB)
> > it doesn't, so I'm trying to keep ACPI code machine compat agnostic
> > as much as possible.
> 
> Eduardo, what do you think about this ? I'm hoping to do a v5 over the
> weekend or early next week, and which way this should go is one of the
> couple of decisions that I still have open.

The general rule is that anything that's visible to the guest shouldn't
change on a QEMU upgrade if the machine-type is kept the same. If we
want to avoid the compat code, we need careful testing to ensure this
won't make any guest OS do something unexpected.

One of the things that may break if guest-visible bits of the machine
change is Windows license activation, but the rules Windows use to
trigger reactivation aren't very clear.

-- 
Eduardo

Re: [Qemu-devel] [PATCH] Limit memory r/w length to buffer size

[Markus Armbruster]

P J P  writes:

>Hello,
>
> An OOB r/w access issue was reported by Mr Gerben Lubbe(CC'd here).
>
> The GDB(1) stub protocol supports commands 'm/M' to read & write 'len'
> bytes from/to the stub memory area. In that, the 'len' parameter value
> supplied by the host gdb(1) is not validated against the local buffer
> size. Which in turn could lead to OOB r/w memory access.
>
> Below is a proposed patch to fix this issue.

How is this related to Kevin's
[PATCH] gdbstub: Fix buffer overflows in gdb_handle_packet()
Message-Id: <1444721930-5121-1-git-send-email-kw...@redhat.com>
?

Re: [Qemu-devel] [PATCHv13/8] trace: [tcg] Identify events with the 'vcpu' property

[Markus Armbruster]

Lluís Vilanova  writes:

> Eric Blake writes:
>
>> On 10/13/2015 11:10 AM, Lluís Vilanova wrote:
>>> Signed-off-by: Lluís Vilanova 
>
>> If you'd send with 'qemu format-patch/send-email -v1', then your subject
>> line would be formatted [PATCH v1 3/8] instead of the confusing results
>> you got by omitting spaces [PATCHv13/8] (this is v13? out of 8?).  Also,
>> no need to include v1 (it's fairly obvious that an unversioned patch is
>> the first), you really only need the designation for -v2 and beyond.
>
> Right, it's just that "stgit mail" does not add the spaces for you.

Then you get to add them yourself, or you get to switch to a tool that
does :)

[...]

Re: [Qemu-devel] [PATCHv17/8] [trivial] Track when QEMU has finished initialization

[Markus Armbruster]

Lluís Vilanova  writes:

> Signed-off-by: Lluís Vilanova 

Drive-by shooting: the commit message should explain *why* you need to
track this.

Re: [Qemu-devel] [PATCH v8 06.5/18] qapi: Drop redundant args-member-array test

[Markus Armbruster]

Eric Blake  writes:

> qapi-schema-test already ensures that we can correctly compile
> an array of enums (__org.qemu_x-command), an array of builtins
> (UserDefNativeListUnion), and an array of structs (again
> __org.qemu_x-command).  That means args-member-array is not
> adding any additional parse-only test coverage, so drop it.
>
> Signed-off-by: Eric Blake 
>
> ---
> v8: new patch, but in response to the same review of v7 that created
> patches 4-6, so we might as well put it in the qapi tree now

Spliced into qapi-next & pushed, thanks!

Re: [Qemu-devel] [PATCH v8 00/18] post-introspection cleanups, subset B

[Markus Armbruster]

Eric Blake  writes:

> Pending prerequisite: Markus' qapi-next branch (which has my
> subset A patches):
> git://repo.or.cz/qemu/armbru.git pull-qapi-2015-10-12
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02796.html
>
> Also available as a tag at this location:
> git fetch git://repo.or.cz/qemu/ericb.git qapi-cleanupv8b
>
> and I plan to eventually forcefully update my branch with the rest
> of the v5 series, at:
> http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi

Okay, I got through subset B.  First time, actually.  Progress!

Re: [Qemu-devel] [PATCH v8 17/18] qapi: Add test for alternate branch 'kind' clash

[Markus Armbruster]

Eric Blake  writes:

> Rename alternate-clash to alternate-clash-members, and add a
> new test alternate-clash-type.  While similar to the earlier
> addition of union-clash-type, we have one major difference: a
> future patch will be simplifying alternates to not need an
> implict AlternateKind enum, but we still need to detect the
> collision with the resulting C 'qtype_code type;' tag.

You're alluding to a future change of the generated code from

struct BlockdevRef {
BlockdevRefKind kind;
union { /* union tag is @kind */
void *data;
BlockdevOptions *definition;
char *reference;
};
};

to

struct BlockdevRef {
qtype_code type;
union { /* union tag is @type */
void *data;
BlockdevOptions *definition; /* QTYPE_QDICT */
char *reference; /* QTYPE_QSTRING */
};
};

right?

I don't think that affects collision checking at all.  Both before and
after, we have a tag member, and we need to check for collisions with
its name.

> No change to generated code.
>
> Signed-off-by: Eric Blake 
>
> ---
> v8: no change
> v7: retitle 10/12 and limit to just testsuite changes
> v6: New patch
> ---
>  tests/Makefile |  3 ++-
>  tests/qapi-schema/alternate-clash-members.err  |  1 +
>  .../{alternate-clash.exit => alternate-clash-members.exit} |  0
>  .../{alternate-clash.json => alternate-clash-members.json} |  0
>  .../{alternate-clash.out => alternate-clash-members.out}   |  0
>  tests/qapi-schema/alternate-clash-type.err |  1 +
>  tests/qapi-schema/alternate-clash-type.exit|  1 +
>  tests/qapi-schema/alternate-clash-type.json| 10 
> ++
>  tests/qapi-schema/alternate-clash-type.out |  0
>  tests/qapi-schema/alternate-clash.err  |  1 -
>  10 files changed, 15 insertions(+), 2 deletions(-)
>  create mode 100644 tests/qapi-schema/alternate-clash-members.err
>  rename tests/qapi-schema/{alternate-clash.exit =>
> alternate-clash-members.exit} (100%)
>  rename tests/qapi-schema/{alternate-clash.json =>
> alternate-clash-members.json} (100%)
>  rename tests/qapi-schema/{alternate-clash.out =>
> alternate-clash-members.out} (100%)
>  create mode 100644 tests/qapi-schema/alternate-clash-type.err
>  create mode 100644 tests/qapi-schema/alternate-clash-type.exit
>  create mode 100644 tests/qapi-schema/alternate-clash-type.json
>  create mode 100644 tests/qapi-schema/alternate-clash-type.out
>  delete mode 100644 tests/qapi-schema/alternate-clash.err
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 2cd5d31..443e345 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -226,7 +226,8 @@ check-qtest-generic-y += tests/qom-test$(EXESUF)
>
>  qapi-schema += alternate-array.json
>  qapi-schema += alternate-base.json
> -qapi-schema += alternate-clash.json
> +qapi-schema += alternate-clash-members.json
> +qapi-schema += alternate-clash-type.json
>  qapi-schema += alternate-conflict-dict.json
>  qapi-schema += alternate-conflict-string.json
>  qapi-schema += alternate-empty.json
> diff --git a/tests/qapi-schema/alternate-clash-members.err
> b/tests/qapi-schema/alternate-clash-members.err
> new file mode 100644
> index 000..0adf737
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-clash-members.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/alternate-clash-members.json:7: Alternate 'Alt1'
> branch 'a_b' clashes with 'a-b'
> diff --git a/tests/qapi-schema/alternate-clash.exit
> b/tests/qapi-schema/alternate-clash-members.exit
> similarity index 100%
> rename from tests/qapi-schema/alternate-clash.exit
> rename to tests/qapi-schema/alternate-clash-members.exit
> diff --git a/tests/qapi-schema/alternate-clash.json
> b/tests/qapi-schema/alternate-clash-members.json
> similarity index 100%
> rename from tests/qapi-schema/alternate-clash.json
> rename to tests/qapi-schema/alternate-clash-members.json
> diff --git a/tests/qapi-schema/alternate-clash.out
> b/tests/qapi-schema/alternate-clash-members.out
> similarity index 100%
> rename from tests/qapi-schema/alternate-clash.out
> rename to tests/qapi-schema/alternate-clash-members.out
> diff --git a/tests/qapi-schema/alternate-clash-type.err
> b/tests/qapi-schema/alternate-clash-type.err
> new file mode 100644
> index 000..cdd2090
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-clash-type.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/alternate-clash-type.json:9: 'kind' (branch of
> Alt1) collides with 'kind' (implicit tag of Alt1)
> diff --git a/tests/qapi-schema/alternate-clash-type.exit
> b/tests/qapi-schema/alternate-clash-type.exit
> new file mode 100644
> index 000..d00491f
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-clash-type.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/alternate-clash-type.json
> b/tests/qapi-schema/alternat

Re: [Qemu-devel] [PATCH v8 16/18] qapi: Move duplicate enum value checks to schema check()

[Markus Armbruster]

Eric Blake  writes:

> Similar to the previous commit, move the detection of a collision
> in enum values from parse time to QAPISchemaEnumType.check().
> This happens to also detect collisions in union branch names,
> so for a decent error message, we have to determine if the enum
> is implicit (and if so where the real collision lies).
>
> Testing this showed that the test union-bad-branch and
> union-clash-branches were basically testing the same thing;
> with the minor difference that the former clashes only in the
> enum, while the latter also clashes in the C union member
> names that would be generated. So delete the weaker test.
>
> No change to generated code.
>
> Signed-off-by: Eric Blake 
>
> ---
> v8: rebase to earlier changes; better comments
> v7: retitle and improve commit message; earlier subclass patches
> avoid problem with detecting 'kind' collision
> v6: new patch
> ---
>  scripts/qapi.py| 54 
> +-
>  tests/Makefile |  1 -
>  tests/qapi-schema/alternate-clash.err  |  2 +-
>  tests/qapi-schema/enum-clash-member.err|  2 +-
>  tests/qapi-schema/enum-max-member.err  |  2 +-
>  tests/qapi-schema/union-bad-branch.err |  1 -
>  tests/qapi-schema/union-bad-branch.exit|  1 -
>  tests/qapi-schema/union-bad-branch.json|  8 -
>  tests/qapi-schema/union-bad-branch.out |  0
>  tests/qapi-schema/union-clash-branches.err |  2 +-
>  tests/qapi-schema/union-clash-type.err |  2 +-
>  tests/qapi-schema/union-max.err|  2 +-
>  12 files changed, 29 insertions(+), 48 deletions(-)
>  delete mode 100644 tests/qapi-schema/union-bad-branch.err
>  delete mode 100644 tests/qapi-schema/union-bad-branch.exit
>  delete mode 100644 tests/qapi-schema/union-bad-branch.json
>  delete mode 100644 tests/qapi-schema/union-bad-branch.out
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 144dd4a..b21e38e 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -527,7 +527,6 @@ def check_union(expr, expr_info):
>  base = expr.get('base')
>  discriminator = expr.get('discriminator')
>  members = expr['data']
> -values = {'MAX': '(automatic)', 'KIND': '(automatic)'}

Stupid / tired question: I can see 'MAX' in the new code further down,
but I can't see 'KIND'.  Why?  Was it perhaps covered by the previous
patch?

>
>  # Two types of unions, determined by discriminator.
>
> @@ -587,34 +586,16 @@ def check_union(expr, expr_info):
>  "enum '%s'" %
>  (key, enum_define["enum_name"]))
>
> -# Otherwise, check for conflicts in the generated enum
> -else:
> -c_key = camel_to_upper(key)
> -if c_key in values:
> -raise QAPIExprError(expr_info,
> -"Union '%s' member '%s' clashes with 
> '%s'"
> -% (name, key, values[c_key]))
> -values[c_key] = key
> -
>
>  def check_alternate(expr, expr_info):
>  name = expr['alternate']
>  members = expr['data']
> -values = {'MAX': '(automatic)'}
>  types_seen = {}
>
>  # Check every branch
>  for (key, value) in members.items():
>  check_name(expr_info, "Member of alternate '%s'" % name, key)
>
> -# Check for conflicts in the generated enum
> -c_key = camel_to_upper(key)
> -if c_key in values:
> -raise QAPIExprError(expr_info,
> -"Alternate '%s' member '%s' clashes with 
> '%s'"
> -% (name, key, values[c_key]))
> -values[c_key] = key
> -
>  # Ensure alternates have no type conflicts.
>  check_type(expr_info, "Member '%s' of alternate '%s'" % (key, name),
> value,
> @@ -633,7 +614,6 @@ def check_enum(expr, expr_info):
>  name = expr['enum']
>  members = expr.get('data')
>  prefix = expr.get('prefix')
> -values = {'MAX': '(automatic)'}
>
>  if not isinstance(members, list):
>  raise QAPIExprError(expr_info,
> @@ -644,12 +624,6 @@ def check_enum(expr, expr_info):
>  for member in members:
>  check_name(expr_info, "Member of enum '%s'" % name, member,
> enum_member=True)
> -key = camel_to_upper(member)
> -if key in values:
> -raise QAPIExprError(expr_info,
> -"Enum '%s' member '%s' clashes with '%s'"
> -% (name, member, values[key]))
> -values[key] = member
>
>
>  def check_struct(expr, expr_info):
> @@ -878,7 +852,26 @@ class QAPISchemaEnumType(QAPISchemaType):
>  self.prefix = prefix
>
>  def check(self, schema):
> -assert len(set(self.values)) == len(self.values)
> +# Check for collisions on the generated C enum values
> +seen = {c_enum_const(self.name, 'MAX'): '(automatic

Re: [Qemu-devel] [PATCH v8 15/18] qapi: Move duplicate member checks to schema check()

[Markus Armbruster]

Eric Blake  writes:

> On 10/13/2015 11:13 AM, Markus Armbruster wrote:
>

 I've come to the conclusion that we should get rid of the self-inflicted
 pain before we attempt to detect all collisions.
>>>
>>> Then that sounds like I should try harder to get the kind/type naming,
>>> the boxed base naming, and even the anonymous union naming all hoisted
>>> into this subset, and spin a v9?
>> 
>> I can take PATCH 01-09,12 into my tree right away, with PATCH 07's two
>> redundant is_implicit() methods dropped, and PATCH 12's comment touched
>> up.
>
> Okay.

Done & pushed to http://repo.or.cz/qemu/armbru.git branch qapi-next.

>> 
>> I could take PATCH 10, but let's at least try to make a plan for
>> c_name() first.  If we fail, I'll take the patch, perhaps less the % to
>> + change, and we'll revisit c_name() later when we see more clearly.
>
> At this point, I'm not sure whether 10 disappears completely after the
> type/kind fix, so that alone is a good enough reason to leave 10 out of
> your tree for another round.
>
>> 
>> You want to move PATCH 11 to later in the queue, and I like that.
>> 
>> PATCH 13 needs a fix squashed in, and a few nits touched up.  If you
>> want me to do that on commit, please propose a patch for me to squash
>> in.  But a respin is probably easier for all.
>> 
>> PATCH 14 is fine, but it depends on 13.
>> 
>> I haven't finished review of PATCH 15-18.
>> 
>> Taken together, I think the easiest way forward is I take 01-09,12, and
>> you respin the rest after we finish its review.  Makes sense?
>> 
>
> Sounds like we're agreed then: take the obvious patches into your tree,
> and let me rework the tail of this subset on top of cleanups that reduce
> self-inflicted collisions.

Yes, please.  I'll try to review v8 16-18 quickly.

Re: [Qemu-devel] [PATCH] Limit memory r/w length to buffer size

[P J P]

  Hello,

+-- On Tue, 13 Oct 2015, P J P wrote --+
| Below is a proposed patch to fix this issue.
| 
| ===
| > From 88edb457a66f8ff96209a1603914171eade0658b Mon Sep 17 00:00:00 2001
| From: Prasad J Pandit 
| Date: Mon, 12 Oct 2015 22:56:41 +0530
| Subject: Limit memory r/w length to buffer size
| 
| GDB(1) stub communication protocol supports commands m/M to read
| and write 'len' bytes from/to the stub memory area.
| 
|m addr,len: read 'len' bytes from address 'addr'
|M addr,len: : write 'len' bytes of 'data' to 'addr'
| 
| Qemu stub uses automatic buffers of size 'MAX_PACKET_LENGTH=4096'
| to process these commands. Limit 'len' parameter value supplied
| by the host gdb(1) to the maximum buffer size to avoid any OOB
| buffer access.
| 
| Reported-by: Gerben van der Lubbe 
| Signed-off-by: Prasad J Pandit 
| ---
|  gdbstub.c | 2 ++
|  1 file changed, 2 insertions(+)

Could someone review it please?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [RFC 3/4] ahci: Add allwinner AHCI

[Beniamino Galvani]

On Sun, Oct 11, 2015 at 09:21:35AM -0700, Peter Crosthwaite wrote:
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1692,9 +1692,107 @@ static const TypeInfo sysbus_ahci_info = {
>  .class_init= sysbus_ahci_class_init,
>  };
>  
> +#define ALLWINNER_AHCI_MMIO_OFF  0x80
> +#define ALLWINNER_AHCI_MMIO_SIZE 0x80

These are already defined in the header file.

> +static const VMStateDescription vmstate_allwinner_ahci = {
> +.name = "a10.pic",

.name = "allwinner-ahci" ?

Beniamino

Re: [Qemu-devel] [PATCHv13/8] trace: [tcg] Identify events with the 'vcpu' property

[Lluís Vilanova]

Eric Blake writes:

> On 10/13/2015 11:10 AM, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova 

> If you'd send with 'qemu format-patch/send-email -v1', then your subject
> line would be formatted [PATCH v1 3/8] instead of the confusing results
> you got by omitting spaces [PATCHv13/8] (this is v13? out of 8?).  Also,
> no need to include v1 (it's fairly obvious that an unversioned patch is
> the first), you really only need the designation for -v2 and beyond.

Right, it's just that "stgit mail" does not add the spaces for you.


> The one-line commit subject correctly explains the 'what', but there is
> no commit body explaining the 'why'.  While a commit body is not
> mandatory, it usually helps.

I've had these lying around for a very long time and now I see I got lazy in
writing the descriptions. Sorry about that.


>> +++ b/qapi/trace.json
>> @@ -1,6 +1,6 @@
>> # -*- mode: python -*-
>> #
>> -# Copyright (C) 2011-2014 Lluís Vilanova 
>> +# Copyright (C) 2011-2015 Lluís Vilanova 
>> #
>> # This work is licensed under the terms of the GNU GPL, version 2 or later.
>> # See the COPYING file in the top-level directory.
>> @@ -29,11 +29,12 @@
>> #
>> # @name: Event name.
>> # @state: Tracing state.
>> +# @vcpu: Whether this is a per-vCPU event.
>> #

> Missing a '(since 2.5)' comment on the @vcpu line.

[...]
>> +++ b/trace/qmp.c
>> @@ -22,6 +22,8 @@ TraceEventInfoList *qmp_trace_event_get_state(const char 
>> *name, Error **errp)
>> while ((ev = trace_event_pattern(name, ev)) != NULL) {
>> TraceEventInfoList *elem = g_new(TraceEventInfoList, 1);
elem-> value = g_new(TraceEventInfo, 1);
>> +elem->value->vcpu =
>> +trace_event_get_cpu_id(ev) == TRACE_CPU_EVENT_COUNT ? false : 
>> true;

> I'm not a fan of the over-verbose 'cond ? false : true'.  It can almost
> always be written '!cond' with just as much clarity.  In your case:

> elem-> value->vcpu = trace_event_get_cpu_id(ev) != TRACE_CPU_EVENT_COUNT;

I guess it's a matter of personal taste. I'll add both to version 2.


Thanks,
  Lluis

-- 
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth

[Qemu-devel] [PATCH v8 06.5/18] qapi: Drop redundant args-member-array test

[Eric Blake]

qapi-schema-test already ensures that we can correctly compile
an array of enums (__org.qemu_x-command), an array of builtins
(UserDefNativeListUnion), and an array of structs (again
__org.qemu_x-command).  That means args-member-array is not
adding any additional parse-only test coverage, so drop it.

Signed-off-by: Eric Blake 

---
v8: new patch, but in response to the same review of v7 that created
patches 4-6, so we might as well put it in the qapi tree now
---
 tests/Makefile   | 1 -
 tests/qapi-schema/args-member-array.err  | 0
 tests/qapi-schema/args-member-array.exit | 1 -
 tests/qapi-schema/args-member-array.json | 4 
 tests/qapi-schema/args-member-array.out  | 9 -
 5 files changed, 15 deletions(-)
 delete mode 100644 tests/qapi-schema/args-member-array.err
 delete mode 100644 tests/qapi-schema/args-member-array.exit
 delete mode 100644 tests/qapi-schema/args-member-array.json
 delete mode 100644 tests/qapi-schema/args-member-array.out

diff --git a/tests/Makefile b/tests/Makefile
index 35a63f3..cb221de 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -239,7 +239,6 @@ qapi-schema += args-array-unknown.json
 qapi-schema += args-int.json
 qapi-schema += args-invalid.json
 qapi-schema += args-member-array-bad.json
-qapi-schema += args-member-array.json
 qapi-schema += args-member-unknown.json
 qapi-schema += args-name-clash.json
 qapi-schema += args-union.json
diff --git a/tests/qapi-schema/args-member-array.err 
b/tests/qapi-schema/args-member-array.err
deleted file mode 100644
index e69de29..000
diff --git a/tests/qapi-schema/args-member-array.exit 
b/tests/qapi-schema/args-member-array.exit
deleted file mode 100644
index 573541a..000
--- a/tests/qapi-schema/args-member-array.exit
+++ /dev/null
@@ -1 +0,0 @@
-0
diff --git a/tests/qapi-schema/args-member-array.json 
b/tests/qapi-schema/args-member-array.json
deleted file mode 100644
index e6f7f5d..000
--- a/tests/qapi-schema/args-member-array.json
+++ /dev/null
@@ -1,4 +0,0 @@
-# valid array members
-{ 'enum': 'abc', 'data': [ 'a', 'b', 'c' ] }
-{ 'struct': 'def', 'data': { 'array': [ 'abc' ] } }
-{ 'command': 'okay', 'data': { 'member1': [ 'int' ], 'member2': [ 'def' ] } }
diff --git a/tests/qapi-schema/args-member-array.out 
b/tests/qapi-schema/args-member-array.out
deleted file mode 100644
index b3b92df..000
--- a/tests/qapi-schema/args-member-array.out
+++ /dev/null
@@ -1,9 +0,0 @@
-object :empty
-object :obj-okay-arg
-member member1: intList optional=False
-member member2: defList optional=False
-enum abc ['a', 'b', 'c']
-object def
-member array: abcList optional=False
-command okay :obj-okay-arg -> None
-   gen=True success_response=True
-- 
2.4.3

[Qemu-devel] [PATCHv15/8] exec: [ŧcg] Use multiple physical TB caches

[Lluís Vilanova]

The physical translation block cache is split into as many caches as wanted, and
the virtual TB cache on each guest CPU uses a (potentially) different physical
TB cache.

This is later exploited to support different tracing event states on a per-vCPU
basis.

Signed-off-by: Lluís Vilanova 
---
 cpu-exec.c  |   17 +
 include/exec/exec-all.h |   10 +++
 include/qom/cpu.h   |5 ++
 qom/cpu.c   |9 +++
 translate-all.c |  146 +--
 translate-all.h |   49 
 6 files changed, 214 insertions(+), 22 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 8fd56a6..b47d57b 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -27,6 +27,7 @@
 #include "exec/address-spaces.h"
 #include "qemu/rcu.h"
 #include "exec/tb-hash.h"
+#include "translate-all.h"
 #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
 #include "hw/i386/apic.h"
 #endif
@@ -222,7 +223,7 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
 phys_pc = get_page_addr_code(env, pc);
 phys_page1 = phys_pc & TARGET_PAGE_MASK;
 h = tb_phys_hash_func(phys_pc);
-ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h];
+ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[cpu->tb_phys_idx][h];
 for(;;) {
 tb = *ptb1;
 if (!tb) {
@@ -251,8 +252,8 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
 
 /* Move the TB to the head of the list */
 *ptb1 = tb->phys_hash_next;
-tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h];
-tcg_ctx.tb_ctx.tb_phys_hash[h] = tb;
+tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[cpu->tb_phys_idx][h];
+tcg_ctx.tb_ctx.tb_phys_hash[cpu->tb_phys_idx][h] = tb;
 return tb;
 }
 
@@ -459,6 +460,16 @@ int cpu_exec(CPUState *cpu)
 cpu->exception_index = EXCP_INTERRUPT;
 cpu_loop_exit(cpu);
 }
+if (unlikely(tcg_ctx.tb_ctx.tb_phys_hash_size_req !=
+ tcg_ctx.tb_ctx.tb_phys_hash_size)) {
+if (tb_caches_apply() < 0) {
+next_tb = 0;
+}
+}
+if (unlikely(cpu->tb_phys_idx != cpu->tb_phys_idx_req)) {
+cpu_tb_cache_apply(cpu);
+next_tb = 0;
+}
 tb_lock();
 tb = tb_find_fast(cpu);
 /* Note: we do it here to avoid a gcc bug on Mac OS X when
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index de9ffa0..82aa1d0 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -179,6 +179,10 @@ static inline void tlb_flush_by_mmuidx(CPUState *cpu, ...)
 #define USE_DIRECT_JUMP
 #endif
 
+/**
+ * TranslationBlock:
+ * @phys_idx: Index of physical TB cache where this TB has been allocated.
+ */
 struct TranslationBlock {
 target_ulong pc;   /* simulated PC corresponding to this block (EIP + CS 
base) */
 target_ulong cs_base; /* CS base for this block */
@@ -217,6 +221,8 @@ struct TranslationBlock {
jmp_first */
 struct TranslationBlock *jmp_next[2];
 struct TranslationBlock *jmp_first;
+
+unsigned int phys_idx;
 };
 
 #include "qemu/thread.h"
@@ -226,7 +232,9 @@ typedef struct TBContext TBContext;
 struct TBContext {
 
 TranslationBlock *tbs;
-TranslationBlock *tb_phys_hash[CODE_GEN_PHYS_HASH_SIZE];
+TranslationBlock ***tb_phys_hash;
+size_t tb_phys_hash_size;
+size_t tb_phys_hash_size_req;
 int nb_tbs;
 /* any access to the tbs or the page table must use this lock */
 QemuMutex tb_lock;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index b613ff0..743e13b 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -246,6 +246,8 @@ struct kvm_run;
  * @kvm_fd: vCPU file descriptor for KVM.
  * @work_mutex: Lock to prevent multiple access to queued_work_*.
  * @queued_work_first: First asynchronous work pending.
+ * @tb_phys_idx: Index of current phsyical TB cache.
+ * @tb_phys_idx_req: Index of requested phsyical TB cache.
  *
  * State of one CPU core or thread.
  */
@@ -311,6 +313,9 @@ struct CPUState {
 struct KVMState *kvm_state;
 struct kvm_run *kvm_run;
 
+unsigned int tb_phys_idx;
+unsigned int tb_phys_idx_req;
+
 /* TODO Move common fields from CPUArchState here. */
 int cpu_index; /* used by alpha TCG */
 uint32_t halted; /* used by alpha, cris, ppc TCG */
diff --git a/qom/cpu.c b/qom/cpu.c
index fb80d13..bb7a618 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -363,6 +363,14 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 dc->cannot_instantiate_with_device_add_yet = true;
 }
 
+static void cpu_init(Object *obj)
+{
+CPUState *cpu = CPU(obj);
+
+cpu->tb_phys_idx = 0;
+cpu->tb_phys_idx_req = 0;
+}
+
 static const TypeInfo cpu_type_info = {
 .name = TYPE_CPU,
 .parent = TYPE_DEVICE,
@@ -372,6 +380,7 @@ static const TypeInfo cpu_type_info = {
 .abstract = true,
  

Re: [Qemu-devel] [PATCH v3 1/4] util - add automated ID generation utility

[John Snow]

On 10/13/2015 11:26 AM, Markus Armbruster wrote:
> Jeff Cody  writes:
> 
>> On Tue, Oct 13, 2015 at 09:37:29AM +0200, Markus Armbruster wrote:
>>> Jeff Cody  writes:
>>>
 Multiple sub-systems in QEMU may find it useful to generate IDs
 for objects that a user may reference via QMP or HMP.  This patch
 presents a standardized way to do it, so that automatic ID generation
 follows the same rules.

 This patch enforces the following rules when generating an ID:

 1.) Guarantee no collisions with a user-specified ID
 2.) Identify the sub-system the ID belongs to
 3.) Guarantee of uniqueness
 4.) Spoiling predictability, to avoid creating an assumption
 of object ordering and parsing (i.e., we don't want users to think
 they can guess the next ID based on prior behavior).

 The scheme for this is as follows (no spaces):

 # subsys D RR
 Reserved char --||   | |
 Subsystem String |   | |
 Unique number (64-bit) --| |
 Two-digit random number ---|

 For example, a generated node-name for the block sub-system may look
 like this:

 #block076

 The caller of id_generate() is responsible for freeing the generated
 node name string with g_free().

 Reviewed-by: John Snow 
 Reviewed-by: Eric Blake 
 Reviewed-by: Alberto Garcia 
 Signed-off-by: Jeff Cody 
 ---
  include/qemu-common.h |  8 
  util/id.c | 37 +
  2 files changed, 45 insertions(+)

 diff --git a/include/qemu-common.h b/include/qemu-common.h
 index 0bd212b..2f74540 100644
 --- a/include/qemu-common.h
 +++ b/include/qemu-common.h
 @@ -246,6 +246,14 @@ int64_t qemu_strtosz_suffix_unit(const char *nptr, 
 char **end,
  #define STR_OR_NULL(str) ((str) ? (str) : "null")
  
  /* id.c */
 +
 +typedef enum IdSubSystems {
 +ID_QDEV,
>>>
>>> ID_QDEV is not used in this series.  Do you intend to use it in a
>>> followup-series?  Can we reasonably expect that series will be accepted?
>>>
>>
>> John Arbuckle has a patch on list that uses it.  I haven't reviewed
>> it, however - but I guess it depends ultimately on whether qdev will
>> allow autogeneration for its IDs or not.
> 
> Then that patch should add ID_QDEV.
> 
>>> You could sidestep these questions by making id_generate() take a string
>>> argument ;)
>>>
>>
>> I'd rather avoid having each system specifying a string inline in
>> their code.  It is cleaner to have the strings defined in a central
>> location, I think (not to mention, easier to reference).
> 
> Covered by your artistic license :)
> 

I think our engineering license would have us do it the way Jeff already
is -- If we want a central unique ID generator utility, it would be best
to disallow different areas of code from specifying their own IDs ...
it's bound to lead to collisions and heartbreak someday.

Easiest to just keep an enum and if you decide you need to use this
facility, add your name to the Registry Of Accepted Subcomponents and
keep moving.

 +ID_BLOCK,
 +ID_MAX  /* last element, used as array size */
 +} IdSubSystems;
 +
 +char *id_generate(IdSubSystems id);
  bool id_wellformed(const char *id);
  
  /* path.c */
>>> [...]

Re: [Qemu-devel] Question about xen disk unplug support for ahci missed in qemu

[Stefano Stabellini]

On Tue, 13 Oct 2015, John Snow wrote:
> On 10/13/2015 11:55 AM, Fabio Fantoni wrote:
> > I added ahci disk support in libxl and using it for week seems that was
> > ok, after a reply of Stefano Stabellini seems that xen disk unplug
> > support only ide disks:
> > http://git.qemu.org/?p=qemu.git;a=commitdiff;h=679f4f8b178e7c66fbc2f39c905374ee8663d5d8
> > 
> > Today Paul Durrant told me that even if pv disk is ok also with ahci and
> > the emulated one is offline can be a risk:
> > http://lists.xenproject.org/archives/html/win-pv-devel/2015-10/msg00021.html
> > 
> > 
> > I tried to take a fast look in qemu code but I not understand the needed
> > thing for add the xen disk unplug support also for ahci, can someone do
> > it or tell me useful information for do it please?
> > 
> > Thanks for any reply and sorry for my bad english.
> > 
> 
> I'm not entirely sure what features you need AHCI to support in order
> for Xen to be happy.
> 
> I'd guess hotplugging, but where I get confused is that IDE disks don't
> support hotplugging either, so I guess I'm not sure sure what you need.
> 
> Stefano, can you help bridge my Xen knowledge gap?
 
Hi John,

we need something like hw/i386/xen/xen_platform.c:unplug_disks but that
can unplug AHCI disk. And by unplug, I mean "make disappear" like
pci_piix3_xen_ide_unplug does for ide.

Re: [Qemu-devel] [PATCH] Add mp-affinity property for ARM CPU class

[Peter Maydell]

On 9 October 2015 at 10:52, Pavel Fedin  wrote:
> This allows to override default affinity IDs on a per-machine basis, and
> possibility to retrieve IDs will be used by vGICv3 live migration code.
>
> Signed-off-by: Pavel Fedin 
> ---
> Since this popped up several times on the mailing list, i decided to publish 
> this early.
> ---
>  target-arm/cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index b48da33..3ebcebe 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -1379,6 +1379,7 @@ static Property arm_cpu_properties[] = {
>  DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
>  DEFINE_PROP_UINT32("psci-conduit", ARMCPU, psci_conduit, 0),
>  DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0),
> +DEFINE_PROP_UINT64("mp-affinity", ARMCPU, mp_affinity, 0),
>  DEFINE_PROP_END_OF_LIST()
>  };

Nothing wrong with this patch, but I'd rather add it as part of
the series which actually uses it. (I see you have it in one of
your GICv3 patchsets.)

thanks
-- PMM

Re: [Qemu-devel] [PULL v3 00/51] Ivshmem patches

[John Snow]

On 10/13/2015 01:16 PM, Marc-André Lureau wrote:
> 
> 
> - Original Message -
>>
>>
>> On 10/13/2015 12:10 PM, Marc-André Lureau wrote:
>>> Hi
>>>
>>> - Original Message -
 On 13 October 2015 at 15:25,   wrote:
> From: Marc-André Lureau 
>
> The following changes since commit
> c49d3411faae8ffaab8f7e5db47405a008411c10:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12'
>   into staging (2015-10-13 10:42:06 +0100)
>
> are available in the git repository at:
>
>   g...@github.com:elmarco/qemu.git tags/ivshmem-pull-request
>
> for you to fetch changes up to feb3f96c4ff1613dd4d0bebda09fe349f8c3e2dd:
>
>   doc: document ivshmem & hugepages (2015-10-13 15:29:53 +0200)
>
> 
> v3 with build fixes on osx & x86
> 

 This asserts in the tests on OSX:

 GTESTER check-qtest-i386
 blkdebug: Suspended request 'A'
 blkdebug: Resuming request 'A'
 Using POSIX shared memory: /qtest-68262-3644687833
 ftruncate(/qtest-68262-3644687833) failed: Invalid argument
 **
>>>
>>> I'll try to reproduce on freebsd. It's weird that this ftruncate() would
>>> fail on osx but not on linux, perhaps a osx security?
>>>
 ERROR:/Users/pm215/src/qemu-for-merges/tests/ivshmem-test.c:299:void
 test_ivshmem_server(): assertion failed (ret == 0): (-1 == 0)
 GTester: last random seed: R02S141a4c6774f852248b61ebcd666b7ad5

 (I'm afraid I didn't notice this in earlier testing because
 for some reason I'm not clear on an assertion failure doesn't
 always cause the test harness to fail.)

 Some asides, which you should look into but which don't need
 to be fixed for this pull request:
  * having the test use 'is QTEST_LOG set' as its "should we be verbose
 in the server failure path" is not terribly helpful because QTEST_LOG
 enables vast volumes of libqtest tracing of communications between
 qemu and the test harness, and anything else is lost in the noise
>>>
>>> What do you suggest instead?
>>>
>>
>> https://developer.gnome.org/glib/stable/glib-Testing.html#g-test-verbose
> 
> Yes, but the question is why we have QTEST_LOG then :) I don't mind much 
> using g_test_verbose(), except it is a different way to enable logging, also 
> it could spew a lot of debug too :)
> 

In my mind, verbose was for printing extra test messages, but QTEST_LOG
was for printing qemu-qtest interactions and messages passed over the
qtest socket.

If you're worried about using the standard gtester verbose flag (and
producing too much output,) you could just add your own verbose flag to
the qtest if you wanted to and it would turn into a developer-only flag
of sorts for just that one test.

You can also use g_test_message() which will be hidden by default and
revealed by --verbose.

Up to you.

>>
  * ivshmem_server_init() has uses of IVSHMEM_SERVER_DEBUG before the
 verbose flag is copied into server->verbose, which means they won't
 print things out when they should
>>>
>>> True, I moved it up.
>>>
  * ivshmem_server_start() is inconsistent about whether it wants
 to report "something failed messages to stderr or via the debug macro
>>>
>>> stderr is for fatal errors. Only gethugepagesize() that I added uses stderr
>>> too because it's a generic function. I'll make it take IvshmemServer
>>> argument and use IVSHMEM_SERVER_DEBUG.
>>>
  * ivshmem_server_ftruncate() is using some bizarre code to
 align up to a power of 2. We have pow2ceil() for this
>>>
>>> ok
>>>
  * Printing "Using POSIX shared memory" in the test output for a
 normal non-verbose test run isn't great: generally our tests should
 be silent except regarding failures
>>>
>>> This was added by earlier reviewer request. I guess during make check,
>>> there could be a way to make it silent instead.
>>>
>>> thanks
>>>
>>
>>

-- 
—js

Re: [Qemu-devel] [PATCHv18/8] trace: [tcg] Add per-vCPU tracing states for events with the 'vcpu' property

[Eric Blake]

On 10/13/2015 11:11 AM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova 
> ---

Interface review only.

> +++ b/qapi/trace.json
> @@ -64,3 +64,34 @@
>  ##
>  { 'command': 'trace-event-set-state',
>'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool'} }
> +
> +##
> +# @trace-event-get-cpu-state:
> +#
> +# Query the state of events in a given vCPU.
> +#
> +# @name: Event name pattern.
> +# @vcpu: The vCPU to check.
> +#
> +# Returns: @TraceEventInfo of the matched events
> +#
> +# Since 2.2

2.5, not 2.2

> +##
> +{ 'command': 'trace-event-get-cpu-state',
> +  'data': {'name': 'str', 'vcpu': 'int'},
> +  'returns': ['TraceEventInfo'] }
> +
> +##
> +# @trace-event-set-cpu-state:
> +#
> +# Set the dynamic state of events in a given vCPU.
> +#
> +# @name: Event name pattern.
> +# @vcpu: The vCPU to act upon.
> +# @enable: Whether to enable tracing.
> +# @ignore-unavailable: #optional Do not match unavailable events with @name.
> +#
> +# Since 2.2

2.5, not 2.2

> +##
> +{ 'command': 'trace-event-set-cpu-state',
> +  'data': {'name': 'str', 'vcpu': 'int', 'enable': 'bool', 
> '*ignore-unavailable': 'bool'} }

This looks identical to trace-event-set-state, except that it now has a
'vcpu':'int' argument.  Would it be any simpler to just modify the
existing command:

##
# @trace-event-set-state:
...
# @vcpu: #optional If provided, limit the state change to the given vcpu
(default act on all vcpus) (since 2.5)
#
# Since 2.2
##
 { 'command': 'trace-event-set-state',
   'data': {'name': 'str', 'enable': 'bool',
'*vcpu': 'int', '*ignore-unavailable': 'bool'} }

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v8 15/18] qapi: Move duplicate member checks to schema check()

[Eric Blake]

On 10/13/2015 11:13 AM, Markus Armbruster wrote:

>>>
>>> I've come to the conclusion that we should get rid of the self-inflicted
>>> pain before we attempt to detect all collisions.
>>
>> Then that sounds like I should try harder to get the kind/type naming,
>> the boxed base naming, and even the anonymous union naming all hoisted
>> into this subset, and spin a v9?
> 
> I can take PATCH 01-09,12 into my tree right away, with PATCH 07's two
> redundant is_implicit() methods dropped, and PATCH 12's comment touched
> up.

Okay.

> 
> I could take PATCH 10, but let's at least try to make a plan for
> c_name() first.  If we fail, I'll take the patch, perhaps less the % to
> + change, and we'll revisit c_name() later when we see more clearly.

At this point, I'm not sure whether 10 disappears completely after the
type/kind fix, so that alone is a good enough reason to leave 10 out of
your tree for another round.

> 
> You want to move PATCH 11 to later in the queue, and I like that.
> 
> PATCH 13 needs a fix squashed in, and a few nits touched up.  If you
> want me to do that on commit, please propose a patch for me to squash
> in.  But a respin is probably easier for all.
> 
> PATCH 14 is fine, but it depends on 13.
> 
> I haven't finished review of PATCH 15-18.
> 
> Taken together, I think the easiest way forward is I take 01-09,12, and
> you respin the rest after we finish its review.  Makes sense?
> 

Sounds like we're agreed then: take the obvious patches into your tree,
and let me rework the tail of this subset on top of cleanups that reduce
self-inflicted collisions.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v8 15/18] qapi: Move duplicate member checks to schema check()

[Markus Armbruster]

First reply: commit message review only.  Patch review to follow.

Eric Blake  writes:

> With the previous commit, we have two different locations for
> detecting member name clashes - one at parse time, and another
> at QAPISchema*.check() time.  Consolidate some of the checks
> into a single place, which is also in line with our TODO to
> eventually move all of the parse time semantic checking into
> the newer schema code.  The check_member_clash() function is
> no longer needed.
>
> Checking variants is tricky:

Indeed.

Struct types aren't as tricky, but tricky enough to warrant spelling
them out, so let me do that.

QMP: The member names of the JSON object are exactly the member names of
the struct type.  Thus, members can clash with each other (d'oh!).

C: The member names of the C struct are exactly the C names of the *own*
(non-inherited) member names of the struct type, plus 'base' if it has a
base type, plus a has_FOO flag for each optional local member with C
name FOO.  Therefore, local members can clash with each other (d'oh
again!), and additionally with 'base' and the has_FOOs.

The additional clashes are self-inflicted pain:

* Less foolish names for the flags, i.e. ones that cannot occur as
  member names, would eliminate the has_FOO clashes.

* Unboxing base types like we do for unions would eliminate the 'base'
  clash.  Heck, even renaming base to something that cannot occur as
  member name would.

If we check for clashes with *inherited* member names, too, then
checking for C clashes suffices, because when the QMP member names
clash, the C member names clash, too.

>  we need to check that the branch
> name will not cause a collision (important for C code, but
> no bearing on QMP).  Then, if the variant belongs to a union
> (flat or simple), we need to check that QMP members of that
> type will not collide with non-variant QMP members (but do
> not care if they collide with C branch names).

Union types.

QMP: The member names of the JSON object are exactly the names of the
union type's non-variant members (this includes the tag member; a simple
union's implicit tag is named 'type') plus the names of a single case's
variant members.  Branch names occurs only as (tag) value, not as key.

Thus, members can clash with each other, except for variant members from
different cases.

C: The member names of the C struct are

* the C names of the non-variant members (this includes the tag member;
  a simple union's implicit tag is named 'kind' now, soon 'type')

* a has_FOO for each optional non-variant member with C name FOO

* the branch names, wrapped in an unnamed union

* 'data', wrapped in the same unnamed union

Therefore, non-variant members can clash with each other as for struct
types (except here the inherited members *are* unboxed already), and
additionally

* branch names can clash with each other (but that's caught when
  checking the tag enumeration, which has the branch names as values)

* branch names can clash with non-variant members

* 'data' can clash with branch names and non-variant members

The additional clashes are all self-inflicted pain: either give the
union a name that cannot clash with a non-variant member, or unbox the
cases and rename or kill 'data'.

If we check for clashes between the non-variant members and each single
case's variant members, too, then checking for C clashes suffices,
because when the QMP member names clash, the C member names clash, too.

> Each call to
> variant.check() has a 'seen' that contains all [*] non-variant
> C names (which includes all non-variant QMP names), but does

What exactly do you mean by the parenthesis?

> not add to that array (QMP members of one branch do not cause
> collisions with other branches).  This means that there is
> one form of collision we still miss: when two branch names
> collide.  However, that will be dealt with in the next patch.

Well, it's already dealt with.  The next patch merely moves the dealing
into the .check().

> [*] Exception - the 'seen' array doesn't contain 'base', which
> is currently a non-variant C member of structs; but since
> structs don't contain variants, it doesn't hurt. Besides, a
> later patch will be unboxing structs the way flat unions
> have already been unboxed.
>
> The wording of several error messages has changed, but in many
> cases feels like an improvement rather than a regression.  The
> recent change (commit 7b2a5c2) to avoid an assertion failure
> when a flat union branch name collides with its discriminator
> name is also handled nicely by this code; but there is more work
> needed before we can detect all collisions in simple union branch
> names done by the old code.

Only simple?

I've come to the conclusion that we should get rid of the self-inflicted
pain before we attempt to detect all collisions.

> No change to generated code.
>
> Signed-off-by: Eric Blake 

[Qemu-devel] [PATCHv16/8] exec: [tcg] Track which vCPU is performing translation and execution

[Lluís Vilanova]

Information is tracked inside the TCGContext structure.

Signed-off-by: Lluís Vilanova 
---
 target-alpha/translate.c  |1 +
 target-arm/translate.c|1 +
 target-cris/translate.c   |1 +
 target-cris/translate_v10.c   |1 +
 target-i386/translate.c   |1 +
 target-lm32/translate.c   |1 +
 target-m68k/translate.c   |1 +
 target-microblaze/translate.c |1 +
 target-mips/translate.c   |1 +
 target-moxie/translate.c  |1 +
 target-openrisc/translate.c   |1 +
 target-ppc/translate.c|1 +
 target-s390x/translate.c  |1 +
 target-sh4/translate.c|1 +
 target-sparc/translate.c  |1 +
 target-tilegx/translate.c |1 +
 target-tricore/translate.c|1 +
 target-unicore32/translate.c  |1 +
 target-xtensa/translate.c |1 +
 tcg/tcg.h |4 
 translate-all.c   |2 ++
 21 files changed, 25 insertions(+)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 6154519..9395813 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -148,6 +148,7 @@ void alpha_translate_init(void)
 done_init = 1;
 
 cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+tcg_ctx.tcg_env = cpu_env;
 
 for (i = 0; i < 31; i++) {
 cpu_std_ir[i] = tcg_global_mem_new_i64(TCG_AREG0,
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 7dc7862..61803ca 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -87,6 +87,7 @@ void arm_translate_init(void)
 int i;
 
 cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+tcg_ctx.tcg_env = cpu_env;
 
 for (i = 0; i < 16; i++) {
 cpu_R[i] = tcg_global_mem_new_i32(TCG_AREG0,
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 9dd5c5f..576a5c6 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3355,6 +3355,7 @@ void cris_initialize_tcg(void)
 int i;
 
 cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+tcg_ctx.tcg_env = cpu_env;
 cc_x = tcg_global_mem_new(TCG_AREG0,
   offsetof(CPUCRISState, cc_x), "cc_x");
 cc_src = tcg_global_mem_new(TCG_AREG0,
diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c
index 3ab1c39..1ef8995 100644
--- a/target-cris/translate_v10.c
+++ b/target-cris/translate_v10.c
@@ -1249,6 +1249,7 @@ void cris_initialize_crisv10_tcg(void)
int i;
 
cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+tcg_ctx.tcg_env = cpu_env;
cc_x = tcg_global_mem_new(TCG_AREG0,
  offsetof(CPUCRISState, cc_x), "cc_x");
cc_src = tcg_global_mem_new(TCG_AREG0,
diff --git a/target-i386/translate.c b/target-i386/translate.c
index e191224..c304c87 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7818,6 +7818,7 @@ void optimize_flags_init(void)
 int i;
 
 cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+tcg_ctx.tcg_env = cpu_env;
 cpu_cc_op = tcg_global_mem_new_i32(TCG_AREG0,
offsetof(CPUX86State, cc_op), "cc_op");
 cpu_cc_dst = tcg_global_mem_new(TCG_AREG0, offsetof(CPUX86State, cc_dst),
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index 45abe53..a74847d 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1184,6 +1184,7 @@ void lm32_translate_init(void)
 int i;
 
 cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+tcg_ctx.tcg_env = cpu_env;
 
 for (i = 0; i < ARRAY_SIZE(cpu_R); i++) {
 cpu_R[i] = tcg_global_mem_new(TCG_AREG0,
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index e3ee9e0..a3f03b7 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -91,6 +91,7 @@ void m68k_tcg_init(void)
  "EXCEPTION");
 
 cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+tcg_ctx.tcg_env = cpu_env;
 
 p = cpu_reg_names;
 for (i = 0; i < 8; i++) {
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 6b0915e..2d73b64 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1862,6 +1862,7 @@ void mb_tcg_init(void)
 int i;
 
 cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+tcg_ctx.tcg_env = cpu_env;
 
 env_debug = tcg_global_mem_new(TCG_AREG0, 
 offsetof(CPUMBState, debug),
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 9ffb100..dc44458 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -19783,6 +19783,7 @@ void mips_tcg_init(void)
 return;
 
 cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+tcg_ctx.tcg_env = cpu_env;
 TCGV_UNUSED(cpu_gpr[0]);
 for (i = 1; i < 32; i++)
 cpu_gpr[i] = tcg_global_mem_new(TCG_AREG0,
diff --git a/target-moxie/translate.c b/target-moxie/translate.c
index

[Qemu-devel] [Bug 1505759] [NEW] Usb passthrough on q35 broken.

[José Ramón Muñoz Pekkarinen]

Public bug reported:

I'm trying to setup a q35 vm with windows 7 guest for vga passthrough.
The machine works well for this purpose, but the usb devices passed to
the vm does not. I receive the following errors on screen:

qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
libusb: error [_open_sysfs_attr} open 
/sys/bus/usb/devices/3-5/bConfigurationValue failed ret=-1 errno=2
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
libusb: error [_open_sysfs_attr} open 
/sys/bus/usb/devices/4-1/bConfigurationValue failed ret=-1 errno=2
Disabling IRQ #18
Disabling IRQ #17

And from the system log I can see the following:

Oct 13 20:13:25 koalita kernel: vfio-pci :01:00.1: enabling device (0400 -> 
0402)
Oct 13 20:13:29 koalita kernel: usb 3-5: reset low-speed USB device number 2 
using ohci-pci
Oct 13 20:13:30 koalita kernel: usb 4-1: reset low-speed USB device number 2 
using ohci-pci
Oct 13 20:13:30 koalita kernel: usb 10-2: reset low-speed USB device number 2 
using xhci_hcd
Oct 13 20:13:31 koalita kernel: usb 10-2: ep 0x81 - rounding interval to 64 
microframes, ep desc says 80 microframes
Oct 13 20:13:31 koalita kernel: usb 10-2: ep 0x1 - rounding interval to 64 
microframes, ep desc says 80 microframes
Oct 13 20:13:31 koalita kernel: usb 3-5: reset low-speed USB device number 2 
using ohci-pci
Oct 13 20:13:31 koalita kernel: usb 10-2: reset low-speed USB device number 2 
using xhci_hcd
Oct 13 20:13:32 koalita kernel: usb 10-2: ep 0x81 - rounding interval to 64 
microframes, ep desc says 80 microframes
Oct 13 20:13:32 koalita kernel: usb 10-2: ep 0x1 - rounding interval to 64 
microframes, ep desc says 80 microframes
Oct 13 20:13:32 koalita kernel: usb 4-1: reset low-speed USB device number 2 
using ohci-pci
Oct 13 20:13:33 koalita kernel: usb 3-5: reset low-speed USB device number 2 
using ohci-pci
Oct 13 20:13:33 koalita kernel: usb 4-1: reset low-speed USB device number 2 
using ohci-pci
Oct 13 20:13:34 koalita kernel: usb 3-5: reset low-speed USB device number 2 
using ohci-pci
Oct 13 20:13:34 koalita kernel: usb 10-2: reset low-speed USB device number 2 
using xhci_hcd
Oct 13 20:13:35 koalita kernel: usb 10-2: ep 0x81 - rounding interval to 64 
microframes, ep desc says 80 microframes
Oct 13 20:13:35 koalita kernel: usb 10-2: ep 0x1 - rounding interval to 64 
microframes, ep desc says 80 microframes
Oct 13 20:13:35 koalita kernel: usb 10-2: reset low-speed USB device number 2 
using xhci_hcd
Oct 13 20:13:35 koalita kernel: usb 10-2: ep 0x81 - rounding interval to 64 
microframes, ep desc says 80 microframes
Oct 13 20:13:35 koalita kernel: usb 10-2: ep 0x1 - rounding interval to 64 
microframes, ep desc says 80 microframes

I tried to any combination of usb devices, and even disabling the ICH9
usb devices to make the setup looks close to the 440fx machine that is
working for me.

Version of qemu is 2.2.1(all newer versions fails on usb passthrough,
even in 440fx machines), and kernel is 4.1.8.

The script to launch it is the following:

qemu-system-x86_64 -enable-kvm -M q35 -vga none -cpu host -smp 
3,cores=3,threads=1 -m 6144 \
-L /usr/x86_64-pc-linux-gnu/usr/share/qemu \
-nodefaults -nodefconfig \
-device ioh3420,multifunction=on,id=pcie \
-device 
vfio-pci,host=01:00.0,addr=1c.0,x-vga=on,multifunction=on,bus=pcie \
-device vfio-pci,host=01:00.1,addr=1c.1,bus=pcie \
-netdev user,id=user.0 -device virtio-net-pci,netdev=user.0 \
-device usb-ehci,id=ehci -device nec-usb-xhci,id=xhci \
-usb -usbdevice host:03f0:134a -usbdevice host:03f0:0024 -usbdevice 
host:0079:0006 \
-drive file=q35_win7.img,format=raw,cache=none,aio=native,if=virtio

Thanks!

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Usb passthrough on q35 broken.

Status in QEMU:
  New

Bug description:
  I'm trying to setup a q35 vm with windows 7 guest for vga passthrough.
  The machine works well for this purpose, but the usb devices passed to
  the vm does not. I receive the following errors on screen:

  qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
  libusb: error [_open_sysfs_attr} open 
  /sys/bus/usb/devices/3-5/bConfigurationValue failed ret=-1 errno=2
  qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
  libusb: error [_open_sysfs_attr} open 
  /sys/bus/usb/devices/4-1/bConfigurationValue failed ret=-1 errno=2
  Disabling IRQ #18
  Disabling IRQ #17

  And from the system log I can see the following:

  Oct 13 20:13:25 koalita kernel: vfio-pci :01:00.1: enabling device (0400 
-> 0402)
  Oct 13 20:13:29 koalita kernel: usb 3-5: reset low-speed USB device number 2 
using ohci-pci
  Oct 13 20:13:30 koalita kernel: usb 4-1: reset low-speed USB device number 2 
using ohci-pci
  Oct 13 20:13:30 koalita kernel: usb 10-2: reset low-speed US

[Qemu-devel] [PATCHv14/8] exec: [tcg] Refactor flush of per-CPU virtual TB cache

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova 
---
 cputlb.c|2 +-
 include/exec/exec-all.h |6 ++
 translate-all.c |7 ++-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index bf1d50a..74bf989 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -61,7 +61,7 @@ void tlb_flush(CPUState *cpu, int flush_global)
 
 memset(env->tlb_table, -1, sizeof(env->tlb_table));
 memset(env->tlb_v_table, -1, sizeof(env->tlb_v_table));
-memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
+tb_flush_jmp_cache_all(cpu);
 
 env->vtlb_index = 0;
 env->tlb_flush_addr = -1;
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index a63fd60..de9ffa0 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -239,6 +239,12 @@ struct TBContext {
 };
 
 void tb_free(TranslationBlock *tb);
+/**
+ * tb_flush_jmp_cache_all:
+ *
+ * Flush the virtual translation block cache.
+ */
+void tb_flush_jmp_cache_all(CPUState *env);
 void tb_flush(CPUState *cpu);
 void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
 
diff --git a/translate-all.c b/translate-all.c
index 333eba4..e5eb6db 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -844,7 +844,7 @@ void tb_flush(CPUState *cpu)
 tcg_ctx.tb_ctx.nb_tbs = 0;
 
 CPU_FOREACH(cpu) {
-memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
+tb_flush_jmp_cache_all(cpu);
 }
 
 memset(tcg_ctx.tb_ctx.tb_phys_hash, 0, 
sizeof(tcg_ctx.tb_ctx.tb_phys_hash));
@@ -1584,6 +1584,11 @@ void tb_check_watchpoint(CPUState *cpu)
 }
 }
 
+void tb_flush_jmp_cache_all(CPUState *cpu)
+{
+memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
+}
+
 #ifndef CONFIG_USER_ONLY
 /* in deterministic execution mode, instructions doing device I/Os
must be at the end of the TB */

Re: [Qemu-devel] [PATCH v2] hw/arm/virt: Allow zero address for PCI IO space

[Peter Maydell]

On 13 October 2015 at 15:35, Alexander Gordeev  wrote:
> Currently PCI IO address 0 is not allowed even though
> the IO space starts from 0. This update makes  PCI IO
> address 0 usable.
>
> CC: Peter Maydell 
> CC: Andrew Jones 
> Signed-off-by: Alexander Gordeev 
> ---
>  hw/arm/virt.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index d25d6cf..3620489 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1157,6 +1157,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
>  mc->has_dynamic_sysbus = true;
>  mc->block_default_type = IF_VIRTIO;
>  mc->no_cdrom = 1;
> +mc->pci_allow_0_address = true;
>  }
>
>  static const TypeInfo machvirt_info = {

Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCHv13/8] trace: [tcg] Identify events with the 'vcpu' property

[Eric Blake]

On 10/13/2015 11:10 AM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova 

If you'd send with 'qemu format-patch/send-email -v1', then your subject
line would be formatted [PATCH v1 3/8] instead of the confusing results
you got by omitting spaces [PATCHv13/8] (this is v13? out of 8?).  Also,
no need to include v1 (it's fairly obvious that an unversioned patch is
the first), you really only need the designation for -v2 and beyond.

The one-line commit subject correctly explains the 'what', but there is
no commit body explaining the 'why'.  While a commit body is not
mandatory, it usually helps.

> +++ b/qapi/trace.json
> @@ -1,6 +1,6 @@
>  # -*- mode: python -*-
>  #
> -# Copyright (C) 2011-2014 Lluís Vilanova 
> +# Copyright (C) 2011-2015 Lluís Vilanova 
>  #
>  # This work is licensed under the terms of the GNU GPL, version 2 or later.
>  # See the COPYING file in the top-level directory.
> @@ -29,11 +29,12 @@
>  #
>  # @name: Event name.
>  # @state: Tracing state.
> +# @vcpu: Whether this is a per-vCPU event.
>  #

Missing a '(since 2.5)' comment on the @vcpu line.

>  # Since 2.2
>  ##
>  { 'struct': 'TraceEventInfo',
> -  'data': {'name': 'str', 'state': 'TraceEventState'} }
> +  'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} }

This is okay from the qapi interface perspective (events are output, so
unconditional addition of new fields won't break existing clients).

I did not closely review the rest, however, I did spot this:

> +++ b/trace/qmp.c
> @@ -22,6 +22,8 @@ TraceEventInfoList *qmp_trace_event_get_state(const char 
> *name, Error **errp)
>  while ((ev = trace_event_pattern(name, ev)) != NULL) {
>  TraceEventInfoList *elem = g_new(TraceEventInfoList, 1);
>  elem->value = g_new(TraceEventInfo, 1);
> +elem->value->vcpu =
> +trace_event_get_cpu_id(ev) == TRACE_CPU_EVENT_COUNT ? false : 
> true;

I'm not a fan of the over-verbose 'cond ? false : true'.  It can almost
always be written '!cond' with just as much clarity.  In your case:

elem->value->vcpu = trace_event_get_cpu_id(ev) != TRACE_CPU_EVENT_COUNT;

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 25/32] nvdimm: build ACPI nvdimm devices

[Xiao Guangrong]

On 10/13/2015 10:39 PM, Igor Mammedov wrote:

On Sun, 11 Oct 2015 11:52:57 +0800
Xiao Guangrong  wrote:


NVDIMM devices is defined in ACPI 6.0 9.20 NVDIMM Devices

There is a root device under \_SB and specified NVDIMM devices are under the
root device. Each NVDIMM device has _ADR which returns its handle used to
associate MEMDEV structure in NFIT

We reserve handle 0 for root device. In this patch, we save handle, arg0,
arg1 and arg2. Arg3 is conditionally saved in later patch

Signed-off-by: Xiao Guangrong 
---
  hw/mem/nvdimm/acpi.c | 203 +++
  1 file changed, 203 insertions(+)

diff --git a/hw/mem/nvdimm/acpi.c b/hw/mem/nvdimm/acpi.c

I'd suggest to put ACPI parts to hw/acpi/nvdimm.c file so that ACPI
maintainers won't miss changes to this files.



Sounds reasonable to me.




index 1450a6a..d9fa0fd 100644
--- a/hw/mem/nvdimm/acpi.c
+++ b/hw/mem/nvdimm/acpi.c
@@ -308,15 +308,38 @@ static void build_nfit(void *fit, GSList *device_list, 
GArray *table_offsets,
   "NFIT", table_data->len - nfit_start, 1);
  }

+#define NOTIFY_VALUE  0x99
+
+struct dsm_in {
+uint32_t handle;
+uint8_t arg0[16];
+uint32_t arg1;
+uint32_t arg2;
+   /* the remaining size in the page is used by arg3. */
+uint8_t arg3[0];
+} QEMU_PACKED;
+typedef struct dsm_in dsm_in;
+
+struct dsm_out {
+/* the size of buffer filled by QEMU. */
+uint16_t len;
+uint8_t data[0];
+} QEMU_PACKED;
+typedef struct dsm_out dsm_out;
+
  static uint64_t dsm_read(void *opaque, hwaddr addr,
   unsigned size)
  {
+fprintf(stderr, "BUG: we never read DSM notification MMIO.\n");
  return 0;
  }

  static void dsm_write(void *opaque, hwaddr addr,
uint64_t val, unsigned size)
  {
+if (val != NOTIFY_VALUE) {
+fprintf(stderr, "BUG: unexepected notify value 0x%" PRIx64, val);
+}
  }

  static const MemoryRegionOps dsm_ops = {
@@ -372,6 +395,183 @@ static MemoryRegion *build_dsm_memory(NVDIMMState *state)
  return dsm_fit_mr;
  }

+#define BUILD_STA_METHOD(_dev_, _method_)  \
+do {   \
+_method_ = aml_method("_STA", 0);  \
+aml_append(_method_, aml_return(aml_int(0x0f)));   \
+aml_append(_dev_, _method_);   \
+} while (0)
+
+#define SAVE_ARG012_HANDLE_LOCK(_method_, _handle_)\
+do {   \
+aml_append(_method_, aml_acquire(aml_name("NLCK"), 0x));   \

how about making method serialized, then you could drop explicit lock/unlock 
logic
for that you'd need to extend existing aml_method() to something like this:

   aml_method("FOO", 3/*count*/, AML_SERIALIZED, 0 /* sync_level */)


I am not sure if multiple methods under different namespace objects can be
serialized, for example:
Device("__D0") {
Method("FOO", 3, AML_SERIALIZED, 0) {
BUF = Arg0
}
}

Device("__D1") {
Method("FOO", 3, AML_SERIALIZED, 0) {
BUF = Arg0
}
}

__D0.FOO and __D1.FOO can be serialized?

Your suggestion definitely valuable to me, i will abstract the access of
shared-memory into one method as your comment below.




+aml_append(_method_, aml_store(_handle_, aml_name("HDLE")));   \
+aml_append(_method_, aml_store(aml_arg(0), aml_name("ARG0"))); \

Could you describe QEMU<->ASL interface in a separate spec
file (for example like: docs/specs/acpi_mem_hotplug.txt),
it will help to with review process as there will be something to compare
patches with.
Once that is finalized/agreed upon, it should be easy to review and probably
to write corresponding patches.


Sure, i considered it too and was planing to make this kind of spec after this
patchset is merged... I will document the interface in the next version.



Also I'd try to minimize QEMU<->ASL interface and implement as much as possible
of ASL logic in AML instead of pushing it in hardware (QEMU).


Okay, i agree.

Since ACPI ASL/AML is new knowledge to me, i did it using the opposite way - 
move
the control to QEMU side as possible ... :)


For example there isn't really any need to tell QEMU ARG0 (UUID), _DSM method
could just compare UUIDs itself and execute a corresponding branch.
Probably something else could be optimized as well but that we can find out
during discussion over QEMU<->ASL interface spec.


Okay.




+aml_append(_method_, aml_store(aml_arg(1), aml_name("ARG1"))); \
+aml_append(_method_, aml_store(aml_arg(2), aml_name("ARG2"))); \
+} while (0)
+
+#define NOTIFY_AND_RETURN_UNLOCK(_method_)   \
+do {   \
+aml_append(_method_, am

Re: [Qemu-devel] [PATCH v2] README: fill out some useful quickstart information

[Eric Blake]

On 10/12/2015 11:41 AM, Daniel P. Berrange wrote:
> The README file is usually the first thing consulted when a user
> or developer obtains a copy of the QEMU source. The current QEMU
> README is lacking immediately useful information and so not very
> friendly for first time encounters. It either redirects users to
> qemu-doc.html (which does not exist until they've actually
> compiled QEMU), or the website (which assumes the user has
> convenient internet access at time of reading).
> 
> This fills out the README file as simple quick-start guide on
> the topics of building source, submitting patches, licensing
> and how to contact the QEMU community. It does not intend to be
> comprehensive, instead referring people to an appropriate web
> page to obtain more detailed information. The intent is to give
> users quick guidance to get them going in the right direction.
> 
> Signed-off-by: Daniel P. Berrange 
> ---

> +++ b/README
> @@ -1,3 +1,107 @@
> -Read the documentation in qemu-doc.html or on http://wiki.qemu-project.org
> + QEMU README
> +  ===

TAB damage.

>  
> -- QEMU team
> +QEMU is a generic and open source machine & userspace emulator and
> +virtualizer.

I might have done s/&/and/ (two instances in the overall document)


> +Building
> +
> +
> +QEMU is multi-platform software intended to be buildable on all modern
> +Linux platforms, OS-X, Win32 (via the Mingw64 toolchain) and a variety
> +of other UNIX targets. The simple steps to build QEMU are:
> +
> +  mkdir build
> +  cd build
> +  ./configure

Doesn't this need to be ../configure?

Looks like I missed reviewing before Paolo queued it, so whether these
fixes are squashed in or done as a followup doesn't bother me.

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] target-arm: Add MDCR_EL2

[Peter Maydell]

On 9 October 2015 at 10:43, Sergey Fedorov  wrote:
> Signed-off-by: Sergey Fedorov 
> ---
>
> Changes in v2:
>  * Reset value is simply made zero
>
>  target-arm/cpu.h|  1 +
>  target-arm/helper.c | 11 +++
>  2 files changed, 12 insertions(+)
>

Applied to target-arm.next, thanks (I expanded the commit
message a little).

-- PMM

Re: [Qemu-devel] [RFC][PATCHv10/8] trace: Per-vCPU tracing states

[Lluís Vilanova]

Sorry I messed the title format, but I guess this does not warrant a re-send.

Cheers,
  Lluis

-- 
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth

Re: [Qemu-devel] [PATCH v2] README: fill out some useful quickstart information

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> The README file is usually the first thing consulted when a user
> or developer obtains a copy of the QEMU source. The current QEMU
> README is lacking immediately useful information and so not very
> friendly for first time encounters. It either redirects users to
> qemu-doc.html (which does not exist until they've actually
> compiled QEMU), or the website (which assumes the user has
> convenient internet access at time of reading).
>
> This fills out the README file as simple quick-start guide on
> the topics of building source, submitting patches, licensing
> and how to contact the QEMU community. It does not intend to be
> comprehensive, instead referring people to an appropriate web
> page to obtain more detailed information. The intent is to give
> users quick guidance to get them going in the right direction.
>
> Signed-off-by: Daniel P. Berrange 
> ---
>
> Changed in v2:
>
>  - Rewrote initial introduction to explain userspace emulators
>as well as system emulators
>  - Reformatted to 72 char line width
>  - Illustrate VPATH build instead of in-tree build
>  - Remove duplicated licensing info pointing to LICENSE file
>
> I've not yet attempted to deal with the possible qemu-tech.texi
> changes discussed in the previous thread, as I figure that's best
> done separately.
>
>  README | 108 
> +++--
>  1 file changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/README b/README
> index c7c990d..819c985 100644
> --- a/README
> +++ b/README
> @@ -1,3 +1,107 @@
> -Read the documentation in qemu-doc.html or on http://wiki.qemu-project.org
> + QEMU README
> +  ===
>  
> -- QEMU team
> +QEMU is a generic and open source machine & userspace emulator and
> +virtualizer.
> +
> +QEMU is capable of emulating a complete machine in software without any
> +need for hardware virtualization support. By using dynamic translation,
> +it achieves very good performance. QEMU can also integrate with the Xen
> +and KVM hypervisors to provide emulated hardware while allowing the
> +hypervisor to manage the CPU. With hypervisor support, QEMU can achieve
> +near native performance for CPUs. When QEMU emulates CPUs directly it is
> +capable of running operating systems made for one machine (e.g. an ARMv7
> +board) on a different machine (e.g. an x86_64 PC board).
> +
> +QEMU is also capable of providing userspace API virtualization for Linux
> +and BSD kernel interfaces. This allows binaries compiled against one
> +architecture ABI (e.g. the Linux PPC64 ABI) to be run on a host using a
> +different architecture ABI (e.g. the Linux x86_64 ABI). This does not
> +involve any hardware emulation, simply CPU and syscall emulation.
> +
> +QEMU aims to fit into a variety of use cases. It can be invoked directly
> +by users wishing to have full control over its behaviour and settings.
> +It also aims to facilitate integration into higher level management
> +layers, by providing a stable command line interface and monitor API.
> +It is commonly invoked indirectly via the libvirt library when using
> +open source applications such as oVirt, OpenStack and virt-manager.
> +
> +QEMU as a whole is released under the GNU General Public License,
> +version 2. For full licensing details, consult the LICENSE file.
> +
> +
> +Building
> +
> +
> +QEMU is multi-platform software intended to be buildable on all modern
> +Linux platforms, OS-X, Win32 (via the Mingw64 toolchain) and a variety
> +of other UNIX targets. The simple steps to build QEMU are:
> +
> +  mkdir build
> +  cd build
> +  ./configure
> +  make
> +
> +Complete details of the process for building and configuring QEMU for
> +all supported host platforms can be found in the qemu-tech.html file.

Make that qemu-doc.texi.

> +Additional information can also be found online via the QEMU website:
> +
> +  http://qemu-project.org/Hosts/Linux
> +  http://qemu-project.org/Hosts/W32
> +
> +
> +Submitting patches
> +==
> +
> +The QEMU source code is maintained under the GIT version control system.
> +
> +   git clone git://git.qemu-project.org/qemu.git
> +
> +When submitting patches, the preferred approach is to use 'git
> +format-patch' and/or 'git send-email' to format & send the mail to the
> +qemu-devel@nongnu.org mailing list. All patches submitted must contain
> +a 'Signed-off-by' line from the author. Patches should follow the
> +guidelines set out in the HACKING and CODING_STYLE files.
> +
> +Additional information on submitting patches can be found online via
> +the QEMU website
> +
> +  http://qemu-project.org/Contribute/SubmitAPatch
> +  http://qemu-project.org/Contribute/TrivialPatches
> +
> +
> +Bug reporting
> +=
> +
> +The QEMU project uses Launchpad as its primary upstream bug tracker. Bugs
> +found when running code built from QEMU git or upstream released sources
> +should be reported via:
> +
> +  https://bugs.launchpad

Re: [Qemu-devel] [PATCH] misc: zynq_slcr: Fix MMIO writes

[Peter Maydell]

On 10 October 2015 at 03:53, Peter Crosthwaite
 wrote:
> The /4 for offset calculation in MMIO writes was happening twice giving
> wrong write offsets. Fix.
>
> While touching the code, change the if-else to be a short returning if
> and convert the debug message to a GUEST_ERROR, which is more accurate
> for this condition.
>
> Cc: qemu-sta...@nongnu.org
> Cc: Guenter Roeck 
> Signed-off-by: Peter Crosthwaite 
> ---
>  hw/misc/zynq_slcr.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)



Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] arm: imx25-pdk: Fix machine name

[Peter Maydell]

On 10 October 2015 at 03:56, Peter Crosthwaite
 wrote:
> ARM uses dashes instead of underscores for machine names. Fix imx25_pdk
> which has not seen a release yet (so there is no legacy yet).
>
> Cc: Jean-Christophe Dubois 
> Signed-off-by: Peter Crosthwaite 
> ---
>  hw/arm/imx25_pdk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
> index 4250114..59a4c11 100644
> --- a/hw/arm/imx25_pdk.c
> +++ b/hw/arm/imx25_pdk.c
> @@ -151,4 +151,4 @@ static void imx25_pdk_machine_init(MachineClass *mc)
>  mc->init = imx25_pdk_init;
>  }
>
> -DEFINE_MACHINE("imx25_pdk", imx25_pdk_machine_init)
> +DEFINE_MACHINE("imx25-pdk", imx25_pdk_machine_init)
> --
> 1.9.1
>



Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PULL v3 01/51] tests: Add ivshmem qtest

[Andreas Färber]

Am 13.10.2015 um 19:07 schrieb Marc-André Lureau:
> On Tue, Oct 13, 2015 at 4:25 PM,   wrote:
>> +check-qtest-pci-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
> 
> This is not being built. CONFIG_IVSHMEM nor CONFIG_KVM are available
> here, but CONFIG_POSIX is.

Correct, because this is for the host and not per target. Which is why I
raised that discussion with my RFC originally. :)

Unfortunately I never finished the QMP code that I started looking into,
therefore there had been no v2 of mine...

Regards,
Andreas

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

Re: [Qemu-devel] [PATCH v8 15/18] qapi: Move duplicate member checks to schema check()

[Markus Armbruster]

Eric Blake  writes:

> On 10/13/2015 09:06 AM, Markus Armbruster wrote:
>> First reply: commit message review only.  Patch review to follow.
>> 
>> Eric Blake  writes:
>> 
>>> With the previous commit, we have two different locations for
>>> detecting member name clashes - one at parse time, and another
>>> at QAPISchema*.check() time.  Consolidate some of the checks
>>> into a single place, which is also in line with our TODO to
>>> eventually move all of the parse time semantic checking into
>>> the newer schema code.  The check_member_clash() function is
>>> no longer needed.
>>>
>>> Checking variants is tricky:
>> 
>> Indeed.
>> 
>> Struct types aren't as tricky, but tricky enough to warrant spelling
>> them out, so let me do that.
>
> Thanks. May be worth cribbing this information into the commit message,
> if there is a reason for a v9 respin.
>
>> 
>> QMP: The member names of the JSON object are exactly the member names of
>> the struct type.  Thus, members can clash with each other (d'oh!).
>> 
>> C: The member names of the C struct are exactly the C names of the *own*
>> (non-inherited) member names of the struct type, plus 'base' if it has a
>> base type, plus a has_FOO flag for each optional local member with C
>> name FOO.  Therefore, local members can clash with each other (d'oh
>> again!), and additionally with 'base' and the has_FOOs.
>> 
>> The additional clashes are self-inflicted pain:
>> 
>> * Less foolish names for the flags, i.e. ones that cannot occur as
>>   member names, would eliminate the has_FOO clashes.
>
> Or even eliminating flags: at least for 'str' and objects, we can use
> foo==NULL in place of has_foo==false.  I have plans to add a markup to
> various structs for when it is safe to eliminate the has_PTR bools, and
> then over a series of patches clean up the .json files to take advantage
> of it - but not until after the pending patches are flushed.

I've wanted to get rid of the "have flags" for pointers since basically
forever.

But even then we'll still have other "have flags", and we'll want less
foolish names for them.

>> 
>> * Unboxing base types like we do for unions would eliminate the 'base'
>>   clash.  Heck, even renaming base to something that cannot occur as
>>   member name would.
>
> My unflushed queue already includes this change, and I'm likely to
> promote it to the front of the queue after subset B goes in (along with
> the kind/type fixes).

Sounds good.

>> If we check for clashes with *inherited* member names, too, then
>> checking for C clashes suffices, because when the QMP member names
>> clash, the C member names clash, too.
>
> Another clash is when two QMP names map to the same C name, as in 'a-b'
> vs. 'a_b'.  Checking for C member name clashes rejects things that are
> not a QMP collision now, but also leaves the door open in case we later
> decide to make QMP key detection looser and allow '-' vs. '_' to be
> synonyms (if we allowed both spellings in the same struct now, then we
> prevent making them synonyms in the future for anyone that (ab)uses both
> names in the same struct).

Keeping the door open for cleaning up the '-' vs. '_' mess is a good
point.

>>>  we need to check that the branch
>>> name will not cause a collision (important for C code, but
>>> no bearing on QMP).  Then, if the variant belongs to a union
>>> (flat or simple), we need to check that QMP members of that
>>> type will not collide with non-variant QMP members (but do
>>> not care if they collide with C branch names).
>> 
>> Union types.
>> 
>> QMP: The member names of the JSON object are exactly the names of the
>> union type's non-variant members (this includes the tag member; a simple
>> union's implicit tag is named 'type') plus the names of a single case's
>> variant members.  Branch names occurs only as (tag) value, not as key.
>> 
>> Thus, members can clash with each other, except for variant members from
>> different cases.
>> 
>> C: The member names of the C struct are
>> 
>> * the C names of the non-variant members (this includes the tag member;
>>   a simple union's implicit tag is named 'kind' now, soon 'type')
>> 
>> * a has_FOO for each optional non-variant member with C name FOO
>> 
>> * the branch names, wrapped in an unnamed union
>> 
>> * 'data', wrapped in the same unnamed union
>> 
>> Therefore, non-variant members can clash with each other as for struct
>> types (except here the inherited members *are* unboxed already), and
>> additionally
>> 
>> * branch names can clash with each other (but that's caught when
>>   checking the tag enumeration, which has the branch names as values)
>> 
>> * branch names can clash with non-variant members
>> 
>> * 'data' can clash with branch names and non-variant members
>> 
>> The additional clashes are all self-inflicted pain: either give the
>> union a name that cannot clash with a non-variant member, or unbox the
>> cases and rename or kill 'data'.
>
> Later patches kill 'data'. 

[Qemu-devel] [PATCHv11/8] trace: Add support for vCPU pointers in trace events

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova 
---
 scripts/tracetool/transform.py |9 -
 target-alpha/translate.c   |2 +-
 target-arm/translate.c |2 +-
 target-arm/translate.h |2 +-
 target-cris/translate.c|2 +-
 target-i386/translate.c|2 +-
 target-lm32/translate.c|2 +-
 target-m68k/translate.c|2 +-
 target-microblaze/translate.c  |2 +-
 target-mips/translate.c|2 +-
 target-moxie/translate.c   |2 +-
 target-openrisc/translate.c|2 +-
 target-ppc/translate.c |2 +-
 target-s390x/translate.c   |2 +-
 target-sh4/translate.c |2 +-
 target-sparc/translate.c   |5 +++--
 target-tilegx/translate.c  |2 +-
 target-tricore/translate.c |2 +-
 target-unicore32/translate.c   |2 +-
 target-xtensa/translate.c  |2 +-
 tcg/tcg-op.h   |2 --
 tcg/tcg.h  |6 ++
 trace/control.h|5 -
 23 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/scripts/tracetool/transform.py b/scripts/tracetool/transform.py
index fc5e679..db3ed54 100644
--- a/scripts/tracetool/transform.py
+++ b/scripts/tracetool/transform.py
@@ -6,7 +6,7 @@ Type-transformation rules.
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2014, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2015, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -74,6 +74,7 @@ TCG_2_HOST = {
 "TCGv_i32": "uint32_t",
 "TCGv_i64": "uint64_t",
 "TCGv_ptr": "void *",
+"TCGv_cpu": "CPUState *",
 None: _tcg_2_host,
 }
 
@@ -98,6 +99,7 @@ HOST_2_TCG = {
 "uint32_t": "TCGv_i32",
 "uint64_t": "TCGv_i64",
 "void *"  : "TCGv_ptr",
+"CPUState *": "TCGv_cpu",
 None: _host_2_tcg,
 }
 
@@ -115,6 +117,8 @@ TCG_2_TCG_HELPER_DEF = {
 "TCGv_i32": "uint32_t",
 "TCGv_i64": "uint64_t",
 "TCGv_ptr": "void *",
+"TCGv_cpu": "void *",
+"CPUState *": "void *",
 None: _tcg_2_helper_def,
 }
 
@@ -130,6 +134,7 @@ TCG_2_TCG_HELPER_DECL = {
 "TCGv_ptr": "ptr",
 "TCGv_i32": "i32",
 "TCGv_i64": "i64",
+"TCGv_cpu": "ptr",
 None: _tcg_2_tcg_helper_decl_error,
 }
 
@@ -146,6 +151,7 @@ HOST_2_TCG_TMP_NEW = {
 "uint32_t": "tcg_const_i32",
 "uint64_t": "tcg_const_i64",
 "void *"  : "tcg_const_ptr",
+"CPUState *": "tcg_const_ptr",
 None: _host_2_tcg_tmp_new,
 }
 
@@ -162,5 +168,6 @@ HOST_2_TCG_TMP_FREE = {
 "uint32_t": "tcg_temp_free_i32",
 "uint64_t": "tcg_temp_free_i64",
 "void *"  : "tcg_temp_free_ptr",
+"CPUState *": "tcg_temp_free_ptr",
 None: _host_2_tcg_tmp_free,
 }
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index f936d1b..6154519 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -91,7 +91,7 @@ typedef enum {
 } ExitStatus;
 
 /* global register indexes */
-static TCGv_ptr cpu_env;
+static TCGv_cpu cpu_env;
 static TCGv cpu_std_ir[31];
 static TCGv cpu_fir[31];
 static TCGv cpu_pc;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 22c3587..7dc7862 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -59,7 +59,7 @@
 #define IS_USER(s) (s->user)
 #endif
 
-TCGv_ptr cpu_env;
+TCGv_cpu cpu_env;
 /* We reuse the same 64-bit temporaries for efficiency.  */
 static TCGv_i64 cpu_V0, cpu_V1, cpu_M0;
 static TCGv_i32 cpu_R[16];
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 53ef971..6e8eb7d 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -70,7 +70,7 @@ typedef struct DisasCompare {
 } DisasCompare;
 
 /* Share the TCG temporaries common between 32 and 64 bit modes.  */
-extern TCGv_ptr cpu_env;
+extern TCGv_cpu cpu_env;
 extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
 extern TCGv_i64 cpu_exclusive_addr;
 extern TCGv_i64 cpu_exclusive_val;
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 964845c..9dd5c5f 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -58,7 +58,7 @@
 #define CC_MASK_NZVC 0xf
 #define CC_MASK_RNZV 0x10e
 
-static TCGv_ptr cpu_env;
+static TCGv_cpu cpu_env;
 static TCGv cpu_R[16];
 static TCGv cpu_PR[16];
 static TCGv cc_x;
diff --git a/target-i386/translate.c b/target-i386/translate.c
index ef10e68..e191224 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -62,7 +62,7 @@
 //#define MACRO_TEST   1
 
 /* global register indexes */
-static TCGv_ptr cpu_env;
+static TCGv_cpu cpu_env;
 static TCGv cpu_A0;
 static TCGv cpu_cc_dst, cpu_cc_src, cpu_cc_src2, cpu_cc_srcT;
 static TCGv_i32 cpu_cc_op;
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index c61ad0f..45abe53 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -42,7 +42,7 @@
 
 #define MEM_INDEX 0
 
-static TCGv_ptr cpu_env;
+static TCGv_cpu cpu_env;
 st

[Qemu-devel] [PATCHv18/8] trace: [tcg] Add per-vCPU tracing states for events with the 'vcpu' property

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova 
---
 Makefile.objs|1 
 include/qom/cpu.h|5 +
 qapi/trace.json  |   31 +++
 qmp-commands.hx  |   27 ++
 qom/cpu.c|   12 +++
 scripts/tracetool/format/tcg_h.py|6 +
 scripts/tracetool/format/tcg_helper_c.py |   11 ++-
 trace/Makefile.objs  |2 
 trace/control-internal.h |   17 +++-
 trace/control-stub.c |   29 +++
 trace/control-target.c   |   53 +
 trace/control.h  |   53 -
 trace/qmp.c  |  127 --
 translate-all.c  |1 
 ui/Makefile.objs |2 
 15 files changed, 357 insertions(+), 20 deletions(-)
 create mode 100644 trace/control-stub.c
 create mode 100644 trace/control-target.c

diff --git a/Makefile.objs b/Makefile.objs
index e1d7b25..99f9d2a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -100,6 +100,7 @@ version-lobj-$(CONFIG_WIN32) += $(BUILD_DIR)/version.lo
 # tracing
 util-obj-y +=  trace/
 target-obj-y += trace/
+stub-obj-y += trace/
 
 ##
 # guest agent
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 743e13b..94a4c1d 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -29,6 +29,7 @@
 #include "qemu/queue.h"
 #include "qemu/thread.h"
 #include "qemu/typedefs.h"
+#include "trace/generated-events.h"
 
 typedef int (*WriteCoreDumpFunction)(const void *buf, size_t size,
  void *opaque);
@@ -316,6 +317,10 @@ struct CPUState {
 unsigned int tb_phys_idx;
 unsigned int tb_phys_idx_req;
 
+/* Ensure 'tb_phys_idx' can encode event states as a bitmask */
+bool too_many_tcg_vcpu_events[
+TRACE_CPU_EVENT_COUNT > sizeof(unsigned int)*8 ? -1 : 0];
+
 /* TODO Move common fields from CPUArchState here. */
 int cpu_index; /* used by alpha TCG */
 uint32_t halted; /* used by alpha, cris, ppc TCG */
diff --git a/qapi/trace.json b/qapi/trace.json
index 94a3a3b..369f4aa 100644
--- a/qapi/trace.json
+++ b/qapi/trace.json
@@ -64,3 +64,34 @@
 ##
 { 'command': 'trace-event-set-state',
   'data': {'name': 'str', 'enable': 'bool', '*ignore-unavailable': 'bool'} }
+
+##
+# @trace-event-get-cpu-state:
+#
+# Query the state of events in a given vCPU.
+#
+# @name: Event name pattern.
+# @vcpu: The vCPU to check.
+#
+# Returns: @TraceEventInfo of the matched events
+#
+# Since 2.2
+##
+{ 'command': 'trace-event-get-cpu-state',
+  'data': {'name': 'str', 'vcpu': 'int'},
+  'returns': ['TraceEventInfo'] }
+
+##
+# @trace-event-set-cpu-state:
+#
+# Set the dynamic state of events in a given vCPU.
+#
+# @name: Event name pattern.
+# @vcpu: The vCPU to act upon.
+# @enable: Whether to enable tracing.
+# @ignore-unavailable: #optional Do not match unavailable events with @name.
+#
+# Since 2.2
+##
+{ 'command': 'trace-event-set-cpu-state',
+  'data': {'name': 'str', 'vcpu': 'int', 'enable': 'bool', 
'*ignore-unavailable': 'bool'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d2ba800..b6b55af 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4189,6 +4189,33 @@ Move mouse pointer to absolute coordinates (2, 400).
{ "type": "abs", "data" : { "axis": "X", "value" : 2 } },
{ "type": "abs", "data" : { "axis": "Y", "value" : 400 } } ] } }
 <- { "return": {} }
+EQMP
+
+{
+.name   = "trace-event-get-cpu-state",
+.args_type  = "name:s,vcpu:i",
+.mhandler.cmd_new = qmp_marshal_trace_event_get_cpu_state,
+},
+
+SQMP
+trace-event-get-state
+-
+
+Query the state of events in a given vCPU.
+
+EQMP
+
+{
+.name   = "trace-event-set-cpu-state",
+.args_type  = "name:s,vcpu:i,enable:b,ignore-unavailable:b?",
+.mhandler.cmd_new = qmp_marshal_trace_event_set_cpu_state,
+},
+
+SQMP
+trace-event-set-state
+-
+
+Set the state of events in a given vCPU.
 
 EQMP
 
diff --git a/qom/cpu.c b/qom/cpu.c
index bb7a618..bc27381 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -25,6 +25,7 @@
 #include "qemu/log.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
+#include "trace/control.h"
 
 bool cpu_exists(int64_t id)
 {
@@ -227,6 +228,17 @@ void cpu_dump_statistics(CPUState *cpu, FILE *f, 
fprintf_function cpu_fprintf,
 void cpu_reset(CPUState *cpu)
 {
 CPUClass *klass = CPU_GET_CLASS(cpu);
+TraceEvent *ev = NULL;
+
+if (!qemu_initialized) {
+/* trace enabled events on all initial vCPUs */
+while ((ev = trace_event_pattern("*", ev)) != NULL) {
+if (trace_event_get_cpu_id(ev) != TRACE_CPU_EVENT_COUNT &&
+trace_event_get_state_dynamic(ev)) {
+trace_event_set_sta

Re: [Qemu-devel] [PULL v3 00/51] Ivshmem patches

[Marc-André Lureau]

- Original Message -
> 
> 
> On 10/13/2015 12:10 PM, Marc-André Lureau wrote:
> > Hi
> > 
> > - Original Message -
> >> On 13 October 2015 at 15:25,   wrote:
> >>> From: Marc-André Lureau 
> >>>
> >>> The following changes since commit
> >>> c49d3411faae8ffaab8f7e5db47405a008411c10:
> >>>
> >>>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12'
> >>>   into staging (2015-10-13 10:42:06 +0100)
> >>>
> >>> are available in the git repository at:
> >>>
> >>>   g...@github.com:elmarco/qemu.git tags/ivshmem-pull-request
> >>>
> >>> for you to fetch changes up to feb3f96c4ff1613dd4d0bebda09fe349f8c3e2dd:
> >>>
> >>>   doc: document ivshmem & hugepages (2015-10-13 15:29:53 +0200)
> >>>
> >>> 
> >>> v3 with build fixes on osx & x86
> >>> 
> >>
> >> This asserts in the tests on OSX:
> >>
> >> GTESTER check-qtest-i386
> >> blkdebug: Suspended request 'A'
> >> blkdebug: Resuming request 'A'
> >> Using POSIX shared memory: /qtest-68262-3644687833
> >> ftruncate(/qtest-68262-3644687833) failed: Invalid argument
> >> **
> > 
> > I'll try to reproduce on freebsd. It's weird that this ftruncate() would
> > fail on osx but not on linux, perhaps a osx security?
> > 
> >> ERROR:/Users/pm215/src/qemu-for-merges/tests/ivshmem-test.c:299:void
> >> test_ivshmem_server(): assertion failed (ret == 0): (-1 == 0)
> >> GTester: last random seed: R02S141a4c6774f852248b61ebcd666b7ad5
> >>
> >> (I'm afraid I didn't notice this in earlier testing because
> >> for some reason I'm not clear on an assertion failure doesn't
> >> always cause the test harness to fail.)
> >>
> >> Some asides, which you should look into but which don't need
> >> to be fixed for this pull request:
> >>  * having the test use 'is QTEST_LOG set' as its "should we be verbose
> >> in the server failure path" is not terribly helpful because QTEST_LOG
> >> enables vast volumes of libqtest tracing of communications between
> >> qemu and the test harness, and anything else is lost in the noise
> > 
> > What do you suggest instead?
> > 
> 
> https://developer.gnome.org/glib/stable/glib-Testing.html#g-test-verbose

Yes, but the question is why we have QTEST_LOG then :) I don't mind much using 
g_test_verbose(), except it is a different way to enable logging, also it could 
spew a lot of debug too :)

> 
> >>  * ivshmem_server_init() has uses of IVSHMEM_SERVER_DEBUG before the
> >> verbose flag is copied into server->verbose, which means they won't
> >> print things out when they should
> > 
> > True, I moved it up.
> > 
> >>  * ivshmem_server_start() is inconsistent about whether it wants
> >> to report "something failed messages to stderr or via the debug macro
> > 
> > stderr is for fatal errors. Only gethugepagesize() that I added uses stderr
> > too because it's a generic function. I'll make it take IvshmemServer
> > argument and use IVSHMEM_SERVER_DEBUG.
> > 
> >>  * ivshmem_server_ftruncate() is using some bizarre code to
> >> align up to a power of 2. We have pow2ceil() for this
> > 
> > ok
> > 
> >>  * Printing "Using POSIX shared memory" in the test output for a
> >> normal non-verbose test run isn't great: generally our tests should
> >> be silent except regarding failures
> > 
> > This was added by earlier reviewer request. I guess during make check,
> > there could be a way to make it silent instead.
> > 
> > thanks
> > 
> 
> 

[Qemu-devel] [PATCHv13/8] trace: [tcg] Identify events with the 'vcpu' property

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova 
---
 qapi/trace.json  |5 +++--
 scripts/tracetool/format/events_c.py |   11 +--
 scripts/tracetool/format/events_h.py |   12 +++-
 trace/control-internal.h |8 +++-
 trace/control.h  |7 +++
 trace/event-internal.h   |4 +++-
 trace/qmp.c  |2 ++
 7 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/qapi/trace.json b/qapi/trace.json
index 01b0a52..94a3a3b 100644
--- a/qapi/trace.json
+++ b/qapi/trace.json
@@ -1,6 +1,6 @@
 # -*- mode: python -*-
 #
-# Copyright (C) 2011-2014 Lluís Vilanova 
+# Copyright (C) 2011-2015 Lluís Vilanova 
 #
 # This work is licensed under the terms of the GNU GPL, version 2 or later.
 # See the COPYING file in the top-level directory.
@@ -29,11 +29,12 @@
 #
 # @name: Event name.
 # @state: Tracing state.
+# @vcpu: Whether this is a per-vCPU event.
 #
 # Since 2.2
 ##
 { 'struct': 'TraceEventInfo',
-  'data': {'name': 'str', 'state': 'TraceEventState'} }
+  'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} }
 
 ##
 # @trace-event-get-state:
diff --git a/scripts/tracetool/format/events_c.py 
b/scripts/tracetool/format/events_c.py
index 2d97fa3..693f6d7 100644
--- a/scripts/tracetool/format/events_c.py
+++ b/scripts/tracetool/format/events_c.py
@@ -6,7 +6,7 @@ trace/generated-events.c
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2014, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2015, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -27,8 +27,15 @@ def generate(events, backend):
 out('TraceEvent trace_events[TRACE_EVENT_COUNT] = {')
 
 for e in events:
-out('{ .id = %(id)s, .name = \"%(name)s\", .sstate = %(sstate)s, 
.dstate = 0 },',
+if "vcpu" in e.properties and "tcg-trans" in e.properties:
+vcpu_id = "TRACE_CPU_" + e.name.upper()
+else:
+vcpu_id = "TRACE_CPU_EVENT_COUNT"
+out('{ .id = %(id)s, .cpu_id = %(vcpu_id)s,'
+' .name = \"%(name)s\",'
+' .sstate = %(sstate)s, .dstate = 0 },',
 id = "TRACE_" + e.name.upper(),
+vcpu_id = vcpu_id,
 name = e.name,
 sstate = "TRACE_%s_ENABLED" % e.name.upper())
 
diff --git a/scripts/tracetool/format/events_h.py 
b/scripts/tracetool/format/events_h.py
index 9f114a3..e89e4ab 100644
--- a/scripts/tracetool/format/events_h.py
+++ b/scripts/tracetool/format/events_h.py
@@ -6,7 +6,7 @@ trace/generated-events.h
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2014, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2015, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -34,6 +34,16 @@ def generate(events, backend):
 out('TRACE_EVENT_COUNT',
 '} TraceEventID;')
 
+# per-vCPU event identifiers
+out('typedef enum {')
+
+for e in events:
+if "vcpu" in e.properties and "tcg-trans" in e.properties:
+out('TRACE_CPU_%s,' % e.name.upper())
+
+out('TRACE_CPU_EVENT_COUNT',
+'} TraceEventCPUID;')
+
 # static state
 for e in events:
 if 'disable' in e.properties:
diff --git a/trace/control-internal.h b/trace/control-internal.h
index 5a8df28..70e55df 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -1,7 +1,7 @@
 /*
  * Interface for configuring and controlling the state of tracing events.
  *
- * Copyright (C) 2011-2014 Lluís Vilanova 
+ * Copyright (C) 2011-2015 Lluís Vilanova 
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -39,6 +39,12 @@ static inline TraceEventID trace_event_get_id(TraceEvent *ev)
 return ev->id;
 }
 
+static inline TraceEventCPUID trace_event_get_cpu_id(TraceEvent *ev)
+{
+assert(ev != NULL);
+return ev->cpu_id;
+}
+
 static inline const char * trace_event_get_name(TraceEvent *ev)
 {
 assert(ev != NULL);
diff --git a/trace/control.h b/trace/control.h
index 1a78a7b..c0680c7 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -89,6 +89,13 @@ static TraceEventID trace_event_count(void);
 static TraceEventID trace_event_get_id(TraceEvent *ev);
 
 /**
+ * trace_event_get_cpu_id:
+ *
+ * Get the per-vCPU identifier of an event.
+ */
+static TraceEventCPUID trace_event_get_cpu_id(TraceEvent *ev);
+
+/**
  * trace_event_get_name:
  *
  * Get the name of an event.
diff --git a/trace/event-internal.h b/trace/event-internal.h
index b2310d9..ae18b48 100644
--- a/trace/event-internal.h
+++ b/trace/event-internal.h
@@ -1,7 +1,7 @@
 /*
  * Interface for configuring and controlling the state of tracing events.
  *
- * Copyright (C) 2012 Lluís Vilanova 
+ * Copyright (C) 2012-2015 Lluís Vilanova 

[Qemu-devel] [RFC][PATCHv10/8] trace: Per-vCPU tracing states

[Lluís Vilanova]

Adds the "vcpu" event property to identify events tied to a specific vCPU. In
addition, it allows controlling the tracing state of such events on a per-vCPU
basis.

Events with the "vcpu" property are identified as being tied to a particular
virtual CPU, like executing an instruction. The state of such events can be
controlled idependently; this is specially useful to, for example, trace memory
access events of a process executing on a specific virtual CPU.

This event plays in combination with the "tcg" property to avoid generating a
call to the execution-time event tracer when a vCPU is not actively tracing such
event.

Virtual CPUs tracing the same set of events use the same physical translation
cache, improving their reuse. The system has 2^N physical translation caches,
where "N" is the number of TCG events with the "vcpu" property. Every vCPU has a
bitmap with the states of these events, which can be controlled separately, and
uses it to index into its physical translation cache. At translation time, QEMU
generates the code to trace an event at execution time only if the event is
enabled.

Signed-off-by: Lluís Vilanova 
---

Lluís Vilanova (8):
  trace: Add support for vCPU pointers in trace events
  trace: Add 'vcpu' event property
  trace: [tcg] Identify events with the 'vcpu' property
  exec: [tcg] Refactor flush of per-CPU virtual TB cache
  exec: [ŧcg] Use multiple physical TB caches
  exec: [tcg] Track which vCPU is performing translation and execution
  [trivial] Track when QEMU has finished initialization
  trace: [tcg] Add per-vCPU tracing states for events with the 'vcpu' 
property


 Makefile.objs|3 -
 bsd-user/main.c  |1 
 cpu-exec.c   |   17 +++
 cputlb.c |2 
 docs/tracing.txt |   30 ++
 include/exec/exec-all.h  |   16 +++
 include/qemu-common.h|3 +
 include/qom/cpu.h|   10 ++
 linux-user/main.c|1 
 qapi/trace.json  |   36 +++
 qemu-common.c|   14 +++
 qmp-commands.hx  |   27 +
 qom/cpu.c|   21 
 scripts/tracetool/__init__.py|   24 -
 scripts/tracetool/format/events_c.py |   11 ++
 scripts/tracetool/format/events_h.py |   12 ++
 scripts/tracetool/format/h.py|4 +
 scripts/tracetool/format/tcg_h.py|   19 +++-
 scripts/tracetool/format/tcg_helper_c.py |   11 ++
 scripts/tracetool/format/ust_events_c.py |4 +
 scripts/tracetool/transform.py   |9 ++
 stubs/Makefile.objs  |1 
 stubs/qemu-common-stub.c |   21 
 target-alpha/translate.c |3 -
 target-arm/translate.c   |3 -
 target-arm/translate.h   |2 
 target-cris/translate.c  |3 -
 target-cris/translate_v10.c  |1 
 target-i386/translate.c  |3 -
 target-lm32/translate.c  |3 -
 target-m68k/translate.c  |3 -
 target-microblaze/translate.c|3 -
 target-mips/translate.c  |3 -
 target-moxie/translate.c |3 -
 target-openrisc/translate.c  |3 -
 target-ppc/translate.c   |3 -
 target-s390x/translate.c |3 -
 target-sh4/translate.c   |3 -
 target-sparc/translate.c |6 +
 target-tilegx/translate.c|3 -
 target-tricore/translate.c   |3 -
 target-unicore32/translate.c |3 -
 target-xtensa/translate.c|3 -
 tcg/tcg-op.h |2 
 tcg/tcg.h|   10 ++
 trace/Makefile.objs  |2 
 trace/control-internal.h |   25 -
 trace/control-stub.c |   29 ++
 trace/control-target.c   |   53 ++
 trace/control.h  |   63 
 trace/event-internal.h   |4 +
 trace/qmp.c  |  129 +++--
 translate-all.c  |  156 ++
 translate-all.h  |   49 +
 ui/Makefile.objs |2 
 vl.c |2 
 56 files changed, 801 insertions(+), 82 deletions(-)
 create mode 100644 qemu-common.c
 create mode 100644 stubs/qemu-common-stub.c
 create mode 100644 trace/control-stub.c
 create mode 100644 trace/control-target.c


To: qemu-devel@nongnu.org
Cc: Stefan Hajnoczi 

[Qemu-devel] [Bug 1479717] Re: Auto resize VM doesn't work with windows 10 guest

[Jean-Pierre van Riel]

I've observed the same issue (in Ubuntu Gnome 15.04). In addition, when
I select '' from the Virtual Machine Manager, View, Scale Display, Auto
Resize VM with Window, the guest's screen starts flickering.

On Windows 10 64bit, virtio drivers and QXL installed from ISO extracted
out the RPM here: https://fedorapeople.org/groups/virt/virtio-
win/repo/latest/virtio-win-0.1.110-2.noarch.rpm. In all cases, I
installed the Windows 8.1 x64 option, given w10 drivers are not yet
included in the ISO. The windows QXL drivers haven't been updated since
July 28th (v22.33.46.473) .

There is a commit on github about windows 10 signatures here which
suggests more formal support for Windows 10 is getting closer for some
of the virtio driver set: https://github.com/crobinso/virtio-win-pkg-
scripts/commit/342a5aaf632fa11cfd2e69acf589dd00c15f72ca

Note, qxl-dod comes from http://cgit.freedesktop.org/spice/win32/qxl. I
don't see any recent commits related to Windows 10 support.

Red Hat bugzilla doesn't seem to have the auto resize VM bug noted
anywhere? Perhaps this should be propagated upstream?

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

Title:
  Auto resize VM doesn't work with windows 10 guest

Status in QEMU:
  New

Bug description:
  I,m using a Ubuntu 15.04 host and a windows 10 guest (both 64 bit) on
  a intel i7 proc. My ubuntu system is up-to-date and I'm using QEMU
  emulator version 2.2.0. I use virt-manager 1.0.1 and SPICE guest tools
  0.100 are installed on the guest.

  With the exactly same setup and a windows 7 guest I can set "Auto
  resize VM with window" and it perfectly works. After installing SPICE
  in windows 10 I can still select this box, but it doesn't work any
  longer.

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

[Qemu-devel] [PATCHv12/8] trace: Add 'vcpu' event property

[Lluís Vilanova]

This property identifies events that trace vCPU-specific information.

It adds an implicit "CPUState*" argument that identifies the vCPU raising this
event. TCG translation events have an additional "TCGv_cpu" implicit argument
that is later used as the "CPUState*" argument at execution time.

Signed-off-by: Lluís Vilanova 
---
 docs/tracing.txt |   30 ++
 scripts/tracetool/__init__.py|   24 ++--
 scripts/tracetool/format/h.py|4 +++-
 scripts/tracetool/format/tcg_h.py|   13 ++---
 scripts/tracetool/format/ust_events_c.py |4 +++-
 5 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index 7117c5e..f0dba3d 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -347,3 +347,33 @@ This will immediately call:
 and will generate the TCG code to call:
 
 void trace_foo(uint8_t a1, uint32_t a2);
+
+=== "vcpu" ===
+
+Identifies events that trace vCPU-specific information. The property adds a
+"CPUState*" argument that identifies the vCPU raising the event. If used
+together with the "tcg" property, it adds a second "TCGv_cpu" argument that
+identifies the vCPU when guest code is executed.
+
+The following example events:
+
+foo(uint32_t a) "a=%x"
+vcpu bar(uint32_t a) "cpu=%p a=%x"
+tcg vcpu baz(uint32_t a) "cpu=%p a=%x", "cpu=%p a=%x"
+
+Can be used as:
+
+#include "trace-tcg.h"
+
+CPUArchState *env;
+TCGv_ptr cpu_env;
+
+void some_disassembly_func(...)
+{
+/* trace emitted at this point */
+trace_foo(0xcafe);
+/* trace emitted at this point */
+trace_bar(ENV_GET_CPU(env), 0xcafe);
+/* trace emitted at this point (env) and when guest code is executed 
(cpu_env) */
+trace_baz_tcg(ENV_GET_CPU(env), cpu_env, 0xcafe);
+}
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 181675f..b75c66d 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -6,7 +6,7 @@ Machinery for generating tracing-related intermediate files.
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2014, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2015, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -146,7 +146,7 @@ class Event(object):
   "(?:(?:(?P\".+),)?\s*(?P\".+))?"
   "\s*")
 
-_VALID_PROPS = set(["disable", "tcg", "tcg-trans", "tcg-exec"])
+_VALID_PROPS = set(["disable", "tcg", "tcg-trans", "tcg-exec", "vcpu"])
 
 def __init__(self, name, props, fmt, args, orig=None):
 """
@@ -215,6 +215,19 @@ class Event(object):
 if "tcg" in props and isinstance(fmt, str):
 raise ValueError("Events with 'tcg' property must have two 
formats")
 
+# add implicit arguments when using the 'vcpu' property
+if "vcpu" in props:
+assert "tcg-trans" not in props and "tcg-exec" not in props
+# events with 'tcg-trans' and 'tcg-exec' are auto-generated, they
+# have already been transformed
+if "tcg" in props:
+types = ["TCGv_cpu"] + args.types()
+names = ["_tcg_cpu"] + args.names()
+else:
+types = ["CPUState *"] + args.types()
+names = ["_cpu"] + args.names()
+args = Arguments(zip(types, names))
+
 return Event(name, props, fmt, args)
 
 def __repr__(self):
@@ -270,6 +283,7 @@ def _read_events(fobj):
 event_trans.name += "_trans"
 event_trans.properties += ["tcg-trans"]
 event_trans.fmt = event.fmt[0]
+# ignore TCG arguments
 args_trans = []
 for atrans, aorig in zip(
 event_trans.transform(tracetool.transform.TCG_2_HOST).args,
@@ -279,6 +293,12 @@ def _read_events(fobj):
 event_trans.args = Arguments(args_trans)
 event_trans = event_trans.copy()
 
+# trace the vCPU performing the translation
+if "vcpu" in event_trans.properties:
+event_trans.args = Arguments(zip(
+["CPUState *"] + list(event_trans.args.types()),
+["_cpu"] + list(event_trans.args.names(
+
 event_exec = event.copy()
 event_exec.name += "_exec"
 event_exec.properties += ["tcg-exec"]
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 9b39430..4bdb48f 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -6,7 +6,7 @@ trace/generated-tracers.h
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2014, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2015, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any

Re: [Qemu-devel] [PULL v3 01/51] tests: Add ivshmem qtest

[Marc-André Lureau]

On Tue, Oct 13, 2015 at 4:25 PM,   wrote:
> +check-qtest-pci-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)

This is not being built. CONFIG_IVSHMEM nor CONFIG_KVM are available
here, but CONFIG_POSIX is.


-- 
Marc-André Lureau

[Qemu-devel] [PULL v3 36/51] ivshmem-server: fix hugetlbfs support

[marcandre . lureau]

From: Marc-André Lureau 

As pointed out on the ML by Andrew Jones, glibc no longer permits
creating POSIX shm on hugetlbfs directly. When given a hugetlbfs path,
create a shareable file there.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 contrib/ivshmem-server/ivshmem-server.c | 50 -
 contrib/ivshmem-server/ivshmem-server.h |  3 +-
 contrib/ivshmem-server/main.c   |  5 ++--
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index ea2b65e..e2d81f7 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -12,6 +12,9 @@
 #include 
 #include 
 #include 
+#ifdef CONFIG_LINUX
+#include 
+#endif
 
 #include "ivshmem-server.h"
 
@@ -272,6 +275,35 @@ ivshmem_server_init(IvshmemServer *server, const char 
*unix_sock_path,
 return 0;
 }
 
+#ifdef CONFIG_LINUX
+
+#define HUGETLBFS_MAGIC   0x958458f6
+
+static long gethugepagesize(const char *path)
+{
+struct statfs fs;
+int ret;
+
+do {
+ret = statfs(path, &fs);
+} while (ret != 0 && errno == EINTR);
+
+if (ret != 0) {
+if (errno != ENOENT) {
+fprintf(stderr, "cannot stat shm file %s: %s\n", path,
+strerror(errno));
+}
+return -1;
+}
+
+if (fs.f_type != HUGETLBFS_MAGIC) {
+return -1;
+}
+
+return fs.f_bsize;
+}
+#endif
+
 /* open shm, create and bind to the unix socket */
 int
 ivshmem_server_start(IvshmemServer *server)
@@ -280,7 +312,23 @@ ivshmem_server_start(IvshmemServer *server)
 int shm_fd, sock_fd, ret;
 
 /* open shm file */
-shm_fd = shm_open(server->shm_path, O_CREAT|O_RDWR, S_IRWXU);
+#ifdef CONFIG_LINUX
+long hpagesize;
+
+hpagesize = gethugepagesize(server->shm_path);
+if (hpagesize > 0) {
+gchar *filename = g_strdup_printf("%s/ivshmem.XX", 
server->shm_path);
+fprintf(stdout, "Using hugepages: %s\n", server->shm_path);
+shm_fd = mkstemp(filename);
+unlink(filename);
+g_free(filename);
+} else
+#endif
+{
+fprintf(stdout, "Using POSIX shared memory: %s\n", server->shm_path);
+shm_fd = shm_open(server->shm_path, O_CREAT|O_RDWR, S_IRWXU);
+}
+
 if (shm_fd < 0) {
 fprintf(stderr, "cannot open shm file %s: %s\n", server->shm_path,
 strerror(errno));
diff --git a/contrib/ivshmem-server/ivshmem-server.h 
b/contrib/ivshmem-server/ivshmem-server.h
index 83c751c..8261e86 100644
--- a/contrib/ivshmem-server/ivshmem-server.h
+++ b/contrib/ivshmem-server/ivshmem-server.h
@@ -82,8 +82,7 @@ typedef struct IvshmemServer {
  * @server: A pointer to an uninitialized IvshmemServer structure
  * @unix_sock_path: The pointer to the unix socket file name
  * @shm_path:   Path to the shared memory. The path corresponds to a POSIX
- *  shm name. To use a real file, for instance in a hugetlbfs,
- *  it is possible to use /../../abspath/to/file.
+ *  shm name or a hugetlbfs mount point.
  * @shm_size:   Size of shared memory
  * @n_vectors:  Number of interrupt vectors per client
  * @verbose:True to enable verbose mode
diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
index 064de02..b960048 100644
--- a/contrib/ivshmem-server/main.c
+++ b/contrib/ivshmem-server/main.c
@@ -47,9 +47,8 @@ ivshmem_server_usage(const char *name, int code)
 " to listen to.\n"
 " Default=%s\n", 
IVSHMEM_SERVER_DEFAULT_UNIX_SOCK_PATH);
 fprintf(stderr, "  -m : path to the shared memory.\n"
-" The path corresponds to a POSIX shm name. To use a\n"
-" real file, for instance in a hugetlbfs, use\n"
-" /../../abspath/to/file.\n"
+" The path corresponds to a POSIX shm name or a\n"
+" hugetlbfs mount point.\n"
 " default=%s\n", IVSHMEM_SERVER_DEFAULT_SHM_PATH);
 fprintf(stderr, "  -l : size of shared memory in bytes. The suffix\n"
 " K, M and G can be used (ex: 1K means 1024).\n"
-- 
2.4.3

Re: [Qemu-devel] [RFH PATCH 0/4] record/replay fixups and doubts

[Paolo Bonzini]

On 13/10/2015 10:10, Pavel Dovgaluk wrote:
> Sometimes replay cannot continue after stopping/restarting of the virtual 
> machine.
> This happens because warp on stopped machine and on running machine behaves 
> differently.
> Timers deadline calculation depends on enabled flag of the virtual timer.
> The following patch fixes the problem - it disables warp when machine is 
> stopped.
> 
> index 5130806..7a337d9 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -411,7 +411,7 @@ void qemu_clock_warp(QEMUClockType type)
>  }
> 
>  /* warp clock deterministically in record/replay mode */
> -if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
> +if (!runstate_is_running() || !replay_checkpoint(CHECKPOINT_CLOCK_WARP)) 
> {
>  return;
>  }

Thanks, I'm adding a comment too:

@@ -410,6 +410,13 @@ void qemu_clock_warp(QEMUClockType type)
 return;
 }

+/* Nothing to do if the VM is stopped: QEMU_CLOCK_VIRTUAL timers
+ * do not fire, so computing the deadline does not make sense.
+ */
+if (!runstate_is_running()) {
+return;
+}
+
 /* warp clock deterministically in record/replay mode */
 if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
 return;

Re: [Qemu-devel] [PULL v3 00/51] Ivshmem patches

[John Snow]

On 10/13/2015 12:10 PM, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
>> On 13 October 2015 at 15:25,   wrote:
>>> From: Marc-André Lureau 
>>>
>>> The following changes since commit
>>> c49d3411faae8ffaab8f7e5db47405a008411c10:
>>>
>>>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12'
>>>   into staging (2015-10-13 10:42:06 +0100)
>>>
>>> are available in the git repository at:
>>>
>>>   g...@github.com:elmarco/qemu.git tags/ivshmem-pull-request
>>>
>>> for you to fetch changes up to feb3f96c4ff1613dd4d0bebda09fe349f8c3e2dd:
>>>
>>>   doc: document ivshmem & hugepages (2015-10-13 15:29:53 +0200)
>>>
>>> 
>>> v3 with build fixes on osx & x86
>>> 
>>
>> This asserts in the tests on OSX:
>>
>> GTESTER check-qtest-i386
>> blkdebug: Suspended request 'A'
>> blkdebug: Resuming request 'A'
>> Using POSIX shared memory: /qtest-68262-3644687833
>> ftruncate(/qtest-68262-3644687833) failed: Invalid argument
>> **
> 
> I'll try to reproduce on freebsd. It's weird that this ftruncate() would fail 
> on osx but not on linux, perhaps a osx security?
> 
>> ERROR:/Users/pm215/src/qemu-for-merges/tests/ivshmem-test.c:299:void
>> test_ivshmem_server(): assertion failed (ret == 0): (-1 == 0)
>> GTester: last random seed: R02S141a4c6774f852248b61ebcd666b7ad5
>>
>> (I'm afraid I didn't notice this in earlier testing because
>> for some reason I'm not clear on an assertion failure doesn't
>> always cause the test harness to fail.)
>>
>> Some asides, which you should look into but which don't need
>> to be fixed for this pull request:
>>  * having the test use 'is QTEST_LOG set' as its "should we be verbose
>> in the server failure path" is not terribly helpful because QTEST_LOG
>> enables vast volumes of libqtest tracing of communications between
>> qemu and the test harness, and anything else is lost in the noise
> 
> What do you suggest instead?
> 

https://developer.gnome.org/glib/stable/glib-Testing.html#g-test-verbose

>>  * ivshmem_server_init() has uses of IVSHMEM_SERVER_DEBUG before the
>> verbose flag is copied into server->verbose, which means they won't
>> print things out when they should
> 
> True, I moved it up.
> 
>>  * ivshmem_server_start() is inconsistent about whether it wants
>> to report "something failed messages to stderr or via the debug macro
> 
> stderr is for fatal errors. Only gethugepagesize() that I added uses stderr 
> too because it's a generic function. I'll make it take IvshmemServer argument 
> and use IVSHMEM_SERVER_DEBUG.
> 
>>  * ivshmem_server_ftruncate() is using some bizarre code to
>> align up to a power of 2. We have pow2ceil() for this
> 
> ok
> 
>>  * Printing "Using POSIX shared memory" in the test output for a
>> normal non-verbose test run isn't great: generally our tests should
>> be silent except regarding failures
> 
> This was added by earlier reviewer request. I guess during make check, there 
> could be a way to make it silent instead.
> 
> thanks
> 

Re: [Qemu-devel] [PATCH v3 11/32] hostmem-file: use whole file size if possible

[Xiao Guangrong]

On 10/13/2015 07:50 PM, Vladimir Sementsov-Ogievskiy wrote:

On 11.10.2015 06:52, Xiao Guangrong wrote:

Use the whole file size if @size is not specified which is useful
if we want to directly pass a file to guest

Signed-off-by: Xiao Guangrong 
---
  backends/hostmem-file.c | 47 +++
  1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 9097a57..adf2835 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -9,6 +9,9 @@
   * This work is licensed under the terms of the GNU GPL, version 2 or later.
   * See the COPYING file in the top-level directory.
   */
+#include 
+#include 
+
  #include "qemu-common.h"
  #include "sysemu/hostmem.h"
  #include "sysemu/sysemu.h"
@@ -33,20 +36,56 @@ struct HostMemoryBackendFile {
  char *mem_path;
  };
+static uint64_t get_file_size(const char *file)
+{
+struct stat stat_buf;
+uint64_t size = 0;
+int fd;
+
+fd = open(file, O_RDONLY);
+if (fd < 0) {
+return 0;
+}
+
+if (stat(file, &stat_buf) < 0) {
+goto exit;
+}
+
+if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, &size)) {
+goto exit;
+}
+
+size = lseek(fd, 0, SEEK_END);
+if (size == -1) {
+size = 0;
+}
+exit:
+close(fd);
+return size;
+}
+
  static void
  file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
  {
  HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
-if (!backend->size) {
-error_setg(errp, "can't create backend with size 0");
-return;
-}
  if (!fb->mem_path) {
  error_setg(errp, "mem-path property not set");
  return;
  }
+if (!backend->size) {
+/*
+ * use the whole file size if @size is not specified.
+ */
+backend->size = get_file_size(fb->mem_path);
+}
+
+if (!backend->size) {
+error_setg(errp, "can't create backend with size 0");
+return;
+}


in case of any error in get_file_size (open, stat, lseek) it will write about 
"backend with size 0"
which may be not appropriate..


Okay, i will change it to:
("failed to get file size for %s, can't create backend on it", mem_path);

Re: [Qemu-devel] [PATCH v8 10/18] qapi: Move union tag quirks into subclass

[Markus Armbruster]

Eric Blake  writes:

> On 10/13/2015 06:10 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> Right now, simple unions have a quirk of using 'kind' in the C
>>> struct to match the QMP wire name 'type'.  This has resulted in
>>> messy clients each doing special cases.  While we plan to
>>> eventually rename things to match, it is better in the meantime
>>> to consolidate the quirks into a special subclass, by adding a
>>> new member.c_name() function.  This will also make it easier
>>> for reworking how alternate types are laid out in a future
>>> patch.  Use the new c_name() function where possible.
>> 
>> Terminology: "the new c_name() method".
>> 
>> I don't like having both function c_name() and method c_name(), because
>> it's very easy to use the function when you should use the method, and
>> the mistakes will make things break only rarely, so they can go
>> undetected easily.  I'm okay with this patch only because you assure me
>> the whole thing is temporary: "# TODO Drop this class once we no longer
>> have the 'type'/'kind' mismatch".
>
> Hmm. Even after my type/kind fix is in, my local tree doesn't (yet)
> remove uses of c_name(), because of its shorter typing.  But you are
> correct that once the rename is in and the temporary
> QAPISchemaObjectTypeUnionTag is deleted, then there is no longer a
> difference between member.c_name() and the longer c_name(member.name).
>
> On the other hand, my patch subset C adds a member.c_type() function
> which is required for use on simplified alternate layout, because it is
> not always the same as member.type.c_type() or c_type(member.type.name).

Can't say how I like it until I reviewed it :)

> As it is, we already have cases where c_name(entity.name) is not the
> same as entity.c_name(), so we already have the confusion of when to use
> the global function vs. the member function.

They are:

* QAPISchemaBuiltinType.c_name() returns its argument instead

* QAPISchemaObjectType.c_name() additionally asserts "not implicit".

* QAPISchemaObjectTypeUnionTag.c_name() returns 'kind' instead, but
  that'll go away.

Anything else?

> Is it worth renaming things so that the global function and member
> functions have different names? If so, which one would be renamed, and
> to what?

Renaming one of them can perhaps make misuse of the function stand out a
bit more.

The only way I can see to keep obvious use of the interface correct is
getting rid of the function entirely.  Involves passing objects instead
of names around, then calling the objects' method instead of passing the
name to the function.  Can't say whether a suitable object always exists
without trying it.

>>> No change to generated code.
>>>
>>> Signed-off-by: Eric Blake 
>>>
>>> ---
>> 
>> v8: use + instead of interpolation in a few places, rebase to simpler
>> .is_implicit(), use .c_name() more.
>
> Whoops, forgot to 'git commit --amend' this one.  Looks like you are
> also viewing interdiffs, though, which makes me feel better about how
> the series is progressing.

When I expect only small and scattered change, I like to feed patches to
ediff :)

>>> +++ b/scripts/qapi-commands.py
>>> @@ -32,8 +32,8 @@ def gen_call(name, arg_type, ret_type):
>>>  if arg_type:
>>>  for memb in arg_type.members:
>>>  if memb.optional:
>>> -argstr += 'has_%s, ' % c_name(memb.name)
>>> -argstr += '%s, ' % c_name(memb.name)
>>> +argstr += 'has_' + memb.c_name() + ', '
>>> +argstr += memb.c_name() + ', '
>> 
>> I like to use + instead of % in sufficiently simple cases myself.  Not
>> sure this is one.  See also change to gen_params() below.
>
>>> @@ -1555,8 +1567,8 @@ def gen_params(arg_type, extra):
>>>  ret += sep
>>>  sep = ', '
>>>  if memb.optional:
>>> -ret += 'bool has_%s, ' % c_name(memb.name)
>>> -ret += '%s %s' % (memb.type.c_type(is_param=True), 
>>> c_name(memb.name))
>>> +ret += 'bool has_' + memb.c_name() + sep
>>> +ret += '%s %s' % (memb.type.c_type(is_param=True), memb.c_name())
>>>  if extra:
>>>  ret += sep + extra
>>>  return ret
>> 
>> New hunk in v8, to remain consistent with gen_call().
>> 
>> I doubt replacing literal ', ' by sep is making things clearer.
>
> Fair enough - if there is reason for respin, I can undo the changes to
> using '+'.
>
>>> @@ -1604,7 +1616,7 @@ def gen_visit_fields(members, prefix='', 
>>> need_cast=False, skiperr=False):
>>>  visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", 
>>> %(errp)s);
>>>  ''',
>>>   c_type=memb.type.c_name(), prefix=prefix, cast=cast,
>>> - c_name=c_name(memb.name), name=memb.name,
>>> + c_name=memb.c_name(), name=memb.name,
>>>   errp=errparg)
>>>  ret += gen_err_check(skiperr=skiperr)
>> 
>> New hunks in v8.  Do you change from function c_name() to method
>> .c_name() 

Re: [Qemu-devel] [PATCH v3 04/32] acpi: add aml_mutex, aml_acquire, aml_release

[Xiao Guangrong]

On 10/13/2015 09:34 PM, Igor Mammedov wrote:

On Sun, 11 Oct 2015 11:52:36 +0800
Xiao Guangrong  wrote:


Implement Mutex, Acquire and Release terms which are used by NVDIMM _DSM method
in later patch

Signed-off-by: Xiao Guangrong 
---
  hw/acpi/aml-build.c | 32 
  include/hw/acpi/aml-build.h |  3 +++
  2 files changed, 35 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 9fe5e7b..ab52692 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1164,6 +1164,38 @@ Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml *len, 
const char *name)
  return var;
  }

+/* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefMutex */
+Aml *aml_mutex(const char *name, uint8_t flags)

s/flags/sync_level/


Oops, will fix.




+{
+Aml *var = aml_alloc();
+build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
+build_append_byte(var->buf, 0x01); /* MutexOp */
+build_append_namestring(var->buf, "%s", name);


add assert here to check that reserved bits are 0


Good idea, will do.