Re: [Qemu-devel] [PATCH] qom: abort on error in property setter if caller passed errp == NULL

[Markus Armbruster]

Peter Crosthwaite  writes:

> Hi,
>
> On Thu, Nov 28, 2013 at 11:24 AM, Igor Mammedov  wrote:
>> in case if caller setting property doesn't care about error and
>> passes in NULL as errp argument but error occurs in property setter,
>> it is silently discarded leaving object in undefined state.
>>
>> As result it leads to hard to find bugs, so if caller doesn't
>> care about error it must be sure that property exists and
>> accepts provided value, otherwise it's better to abort early
>> since error case couldn't be handled gracefully and find
>> invalid usecase early.
>>
>> In addition multitude of property setters will be always
>> guarantied to have error object present and won't be required
>> to handle this condition individually.
>>
>
> So there seems to be contention between different APIs and use cases
> on what to do in the NULL case. Personally, I'm of the opinion that
> NULL should be a silent don't care policy. But you are right in that
> checking every API call at call site is cumbersome, and its better to
> have some sort of collective mechanism to implement different policys.
> I think this can be done caller side efficiently by having a special
> Error * that can be passed in that tells the error API to assert or
> error raise.
>
> extern error *abort_on_err;
>
> And update your call sites to do this:
>
> object_property_set(Foo, bar, "baz", &abort_on_err);
>
> Error_set and friends are then taught to recognise abort_on_err and
> abort accordingly and theres no need for this form of change pattern -
> its all solved in the error API.

The existing practice is to have a pair of functions, like this one:

Foo *foo(int arg, Error **errp)
{
...
}

Foo *foo_nofail(int arg)
{
Error *local_err = NULL;
Foo *f = foo(arg, &local_err);
assert_no_error(local_err);
return f;
}

The asserting function's name is intentionally longer.

Passing NULL is still temptingly short.  Code review has to catch abuse
of it.

Your proposal is more flexible.  If we adopt it, we may want to retire
the _nofail idiom.

> You can also implement a range of automatic error handling policies e.g:
>
> extern Error *report_err; /* report any errors to monitor/stdout */
>
> To report an error without the assertion.

monitor/stderr, you mean.

Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event

[Wenchao Xia]

于 2013/11/28 8:48, Luiz Capitulino 写道:

On Wed, 13 Nov 2013 09:44:52 +0800
Wenchao Xia  wrote:


Nested structure is not supported now, so following define is not valid:
{ 'event': 'EVENT_C',
   'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' }


I think your general approach is reasonable, but there are a number of
details to fix.

The first one is documentation. This patch's changelog is quite bad,
you don't say anything about what the does. You just mention a corner
case which doesn't happen to work. Please, add full changelog explaining
what this patch is about and please add examples of how an event
entry would look like in the schema and maybe you could also add the
important parts of a generated event function. Also, please add a
"event" section to docs/writing-qmp-commands.txt (in a different patch).


  OK.



Secondly, for changes like this it's a very good idea to provide
conversion examples (as patches, not as changelog doc) at least
one or two so that we can see how it will look like.



  Will add some in the intro part. By the way I think test
cases in patch 3 shows a bit.


More below.




Signed-off-by: Wenchao Xia 
---
  Makefile  |9 +-
  Makefile.objs |2 +-
  qapi/Makefile.objs|1 +
  scripts/qapi-event.py |  355 +
  4 files changed, 363 insertions(+), 4 deletions(-)
  create mode 100644 scripts/qapi-event.py

diff --git a/Makefile b/Makefile
index b15003f..a3e465f 100644
--- a/Makefile
+++ b/Makefile
@@ -38,8 +38,8 @@ endif
  endif

  GENERATED_HEADERS = config-host.h qemu-options.def
-GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
-GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
+GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
+GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c

  GENERATED_HEADERS += trace/generated-events.h
  GENERATED_SOURCES += trace/generated-events.c
@@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
  # Build libraries

  libqemustub.a: $(stub-obj-y)
-libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
+libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o

  ##

@@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json 
$(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
  qapi-visit.c qapi-visit.h :\
  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." 
-b < $<, "  GEN   $@")
+qapi-event.c qapi-event.h :\
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o "." 
-b < $<, "  GEN   $@")
  qmp-commands.h qmp-marshal.c :\
  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o 
"." < $<, "  GEN   $@")
diff --git a/Makefile.objs b/Makefile.objs
index 2b6c1fe..33f5950 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
  block-obj-$(CONFIG_POSIX) += aio-posix.o
  block-obj-$(CONFIG_WIN32) += aio-win32.o
  block-obj-y += block/
-block-obj-y += qapi-types.o qapi-visit.o
+block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
  block-obj-y += qemu-io-cmds.o

  block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 1f9c973..d14b769 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
  util-obj-y += string-input-visitor.o string-output-visitor.o

  util-obj-y += opts-visitor.o
+util-obj-y += qmp-event.o
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
new file mode 100644
index 000..4c6a0fe


I didn't review this hunk, but this series doesn't build for me:

   CCqapi/string-output-visitor.o
   CCqapi/opts-visitor.o
make: *** No rule to make target `qapi/qmp-event.o', needed by `libqemuutil.a'. 
 Stop.
~/work/src/upstream/qmp-unstable/build (tmp|AM)/


  A draft file I forgot to remove in Makefile, will fix.


--- /dev/null
+++ b/scripts/qapi-event.py
@@ -0,0 +1,355 @@
+#
+# QAPI event generator
+#
+# Copyright IBM, Corp. 2013
+#
+# Authors:
+#  Wenchao Xia 
+#
+# This work is licensed under the terms of the GNU GPLv2.
+# See the COPYING.LIB file in the top-level directory.
+
+from ordereddict import OrderedDict
+from qapi import *
+import sys
+import os
+import getopt
+import errno
+
+def _generate_event_api_name_with_params(event_name, params):
+api_name = "void qapi_event_gen_%s(" % c_fun(event_name);


I'd prefer a name like qmp_notify_NAME() or qmp_send_event_NAME().



  do you think NAME should be capitalized as:
qmp_n

Re: [Qemu-devel] [PULL v2 00/11] target-lm32 updates

[Antony Pavlov]

On Mon, 14 Oct 2013 19:20:45 +0200
Michael Walle  wrote:

> 
> Am Montag, 14. Oktober 2013, 18:29:24 schrieb Michael Walle:
> > This is a pull for various updates and fixes for the LatticeMico32 target.
> > 
> > Please pull.
> > 
> > changes since v1:
> >  - rebased
> >  - dropped patch "target-lm32: register helper functions". This is
> >no longer needed.
> >  - added patch "target-lm32: stop VM on illegal or unknown instruction".
> >Was posted as request for comments before. But since there were no
> >comments, include it here.
> 
> Hi Anthony, Hi Blue,
> 
> please discard this pull request. There was some feedback, i wasn't getting 
> before. I try to make the suggested changes, sending update patches for 
> reviews and after that i'll make a new pull request.

Hi!

Are you planning on pull request renewing?

-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC] create a single workqueue for each vm to update vm irq routing table

[Zhanghaoyu (A)]

> > >>I understood the proposal was also to eliminate the 
> > >>synchronize_rcu(), so while new interrupts would see the new 
> > >>routing table, interrupts already in flight could pick up the old one.
 > >Isn't that always the case with RCU?  (See my answer above: "the 
 > >vcpus already see the new routing table after the 
 > >rcu_assign_pointer that is in kvm_irq_routing_update").
>>> > 
>>> > With synchronize_rcu(), you have the additional guarantee that any 
>>> > parallel accesses to the old routing table have completed.  Since 
>>> > we also trigger the irq from rcu context, you know that after
>>> > synchronize_rcu() you won't get any interrupts to the old 
>>> > destination (see kvm_set_irq_inatomic()).
>> We do not have this guaranty for other vcpus that do not call 
>> synchronize_rcu(). They may still use outdated routing table while a 
>> vcpu or iothread that performed table update sits in synchronize_rcu().
>
>Avi's point is that, after the VCPU resumes execution, you know that no 
>interrupt will be sent to the old destination because kvm_set_msi_inatomic 
>(and ultimately kvm_irq_delivery_to_apic_fast) is also called within the RCU 
>read-side critical section.
>
>Without synchronize_rcu you could have
>
>VCPU writes to routing table
>   e = entry from IRQ routing table
>kvm_irq_routing_update(kvm, new);
>VCPU resumes execution
>   kvm_set_msi_irq(e, &irq);
>   kvm_irq_delivery_to_apic_fast();
>
>where the entry is stale but the VCPU has already resumed execution.
>
If we use call_rcu()(Not consider the problem that Gleb pointed out 
temporarily) instead of synchronize_rcu(), should we still ensure this?

Thanks,
Zhang Haoyu

>If we want to ensure, we need to use a different mechanism for synchronization 
>than the global RCU.  QRCU would work; readers are not wait-free but only if 
>there is a concurrent synchronize_qrcu, which should be rare.
>
>Paolo

[Qemu-devel] [PATCH 2/2] hw/mips: use sizes.h macros

[Antony Pavlov]

Signed-off-by: Antony Pavlov 
Reviewed-by: Richard Henderson 
---
 hw/mips/mips_malta.c   | 25 +
 include/hw/mips/bios.h |  3 ++-
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 05c8771..604832f 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -51,6 +51,7 @@
 #include "sysemu/qtest.h"
 #include "qemu/error-report.h"
 #include "hw/empty_slot.h"
+#include "qemu/sizes.h"
 
 //#define DEBUG_BOARD_INIT
 
@@ -63,7 +64,7 @@
 #define FPGA_ADDRESS  0x1f00ULL
 #define RESET_ADDRESS 0x1fc0ULL
 
-#define FLASH_SIZE0x40
+#define FLASH_SIZESZ_4M
 
 #define MAX_IDE_BUS 2
 
@@ -827,8 +828,8 @@ static int64_t load_kernel (void)
 }
 
 prom_set(prom_buf, prom_index++, "memsize");
-prom_set(prom_buf, prom_index++, "%i",
- MIN(loaderparams.ram_size, 256 << 20));
+prom_set(prom_buf, prom_index++, "%li",
+ MIN(loaderparams.ram_size, SZ_256M));
 prom_set(prom_buf, prom_index++, "modetty0");
 prom_set(prom_buf, prom_index++, "38400n8r");
 prom_set(prom_buf, prom_index++, NULL);
@@ -954,10 +955,10 @@ void mips_malta_init(QEMUMachineInitArgs *args)
 env = &cpu->env;
 
 /* allocate RAM */
-if (ram_size > (2048u << 20)) {
+if (ram_size > SZ_2G) {
 fprintf(stderr,
-"qemu: Too much memory for this machine: %d MB, maximum 2048 
MB\n",
-((unsigned int)ram_size / (1 << 20)));
+"qemu: Too much memory for this machine: %ld MB, maximum 2048 
MB\n",
+((unsigned long)ram_size / SZ_1M));
 exit(1);
 }
 
@@ -968,17 +969,17 @@ void mips_malta_init(QEMUMachineInitArgs *args)
 
 /* alias for pre IO hole access */
 memory_region_init_alias(ram_low_preio, NULL, "mips_malta_low_preio.ram",
- ram_high, 0, MIN(ram_size, (256 << 20)));
+ ram_high, 0, MIN(ram_size, SZ_256M));
 memory_region_add_subregion(system_memory, 0, ram_low_preio);
 
 /* alias for post IO hole access, if there is enough RAM */
-if (ram_size > (512 << 20)) {
+if (ram_size > SZ_512M) {
 ram_low_postio = g_new(MemoryRegion, 1);
 memory_region_init_alias(ram_low_postio, NULL,
  "mips_malta_low_postio.ram",
- ram_high, 512 << 20,
- ram_size - (512 << 20));
-memory_region_add_subregion(system_memory, 512 << 20, ram_low_postio);
+ ram_high, SZ_512M,
+ ram_size - SZ_512M);
+memory_region_add_subregion(system_memory, SZ_512M, ram_low_postio);
 }
 
 /* generate SPD EEPROM data */
@@ -1012,7 +1013,7 @@ void mips_malta_init(QEMUMachineInitArgs *args)
 fl_idx++;
 if (kernel_filename) {
 /* Write a small bootloader to the flash location. */
-loaderparams.ram_size = MIN(ram_size, 256 << 20);
+loaderparams.ram_size = MIN(ram_size, SZ_256M);
 loaderparams.kernel_filename = kernel_filename;
 loaderparams.kernel_cmdline = kernel_cmdline;
 loaderparams.initrd_filename = initrd_filename;
diff --git a/include/hw/mips/bios.h b/include/hw/mips/bios.h
index b4b88ac..3d7da4b 100644
--- a/include/hw/mips/bios.h
+++ b/include/hw/mips/bios.h
@@ -1,6 +1,7 @@
 #include "cpu.h"
+#include "qemu/sizes.h"
 
-#define BIOS_SIZE (4 * 1024 * 1024)
+#define BIOS_SIZE SZ_4M
 #ifdef TARGET_WORDS_BIGENDIAN
 #define BIOS_FILENAME "mips_bios.bin"
 #else
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v3 3/4] hw/timer: Introduce ARM A9 Global Timer.

[Peter Crosthwaite]

The ARM A9 MPCore has a timer that is global to all CPUs in the mpcore.
The timer is shared but each CPU has a private independent comparator
and interrupt.

Based on version contributed by Francois LEGAL.

Signed-off-by: François LEGAL 
[PC changes:
 * New commit message
 * Re-implemented as single timer model
 * Fixed backwards counting issue in polled mode
 * completed VMSD fields
 * macroified magic numbers (and headerified reg definitions)
 * split of as device-model-only patch
 * use bitops for 64 bit register access
 * Fixed auto increment mode to check condition properly
 * general cleanup (names/style etc).
]
Signed-off-by: Peter Crosthwaite 
---
Changed since v2 (PMM - review):
Refactored for container embedding
Made frequency scaler consistent with mptimer
Fixed missing VMSD ref_counter field
Fixed missing gtb->inc reset
Changed Memory region names
Changed since v1:
Added /*< private >*/ to struct definition.
Pulled register definitions out into a header (AF review)
SOB Francois LEGAL with PC changes indicated.

 hw/timer/Makefile.objs  |   2 +-
 hw/timer/a9gtimer.c | 369 
 include/hw/timer/a9gtimer.h |  97 
 3 files changed, 467 insertions(+), 1 deletion(-)
 create mode 100644 hw/timer/a9gtimer.c
 create mode 100644 include/hw/timer/a9gtimer.h

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index eca5905..42d96df 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -1,5 +1,5 @@
 common-obj-$(CONFIG_ARM_TIMER) += arm_timer.o
-common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
+common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o a9gtimer.o
 common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
 common-obj-$(CONFIG_DS1338) += ds1338.o
 common-obj-$(CONFIG_HPET) += hpet.o
diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c
new file mode 100644
index 000..0f08c67
--- /dev/null
+++ b/hw/timer/a9gtimer.c
@@ -0,0 +1,369 @@
+/*
+ * Global peripheral timer block for ARM A9MP
+ *
+ * (C) 2013 Xilinx Inc.
+ *
+ * Written by François LEGAL
+ * Written by Peter Crosthwaite 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "hw/timer/a9gtimer.h"
+#include "qemu/timer.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+
+#ifndef A9_GTIMER_ERR_DEBUG
+#define A9_GTIMER_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT_L(level, ...) do { \
+if (A9_GTIMER_ERR_DEBUG > (level)) { \
+fprintf(stderr,  ": %s: ", __func__); \
+fprintf(stderr, ## __VA_ARGS__); \
+} \
+} while (0);
+
+#define DB_PRINT(...) DB_PRINT_L(0, ## __VA_ARGS__)
+
+static inline int a9_gtimer_get_current_cpu(A9GTimerState *s)
+{
+if (current_cpu->cpu_index >= s->num_cpu) {
+hw_error("arm_mptimer: num-cpu %d but this cpu is %d!\n",
+ s->num_cpu, current_cpu->cpu_index);
+}
+return current_cpu->cpu_index;
+}
+
+static inline uint64_t a9_gtimer_get_conv(A9GTimerState *s)
+{
+uint64_t prescale = extract32(s->control, R_CONTROL_PRESCALER_SHIFT,
+  R_CONTROL_PRESCALER_LEN);
+
+return (prescale + 1) * 10;
+}
+
+static A9GTimerUpdate a9_gtimer_get_update(A9GTimerState *s)
+{
+A9GTimerUpdate ret;
+
+ret.now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ret.new = s->ref_counter +
+  (ret.now - s->cpu_ref_time) / a9_gtimer_get_conv(s);
+return ret;
+}
+
+static void a9_gtimer_update(A9GTimerState *s, bool sync)
+{
+
+A9GTimerUpdate update = a9_gtimer_get_update(s);
+int i;
+int64_t next_cdiff = 0;
+
+for (i = 0; i < s->num_cpu; ++i) {
+A9GTimerPerCPU *gtb = &s->per_cpu[i];
+int64_t cdiff = 0;
+
+if ((s->control & R_CONTROL_TIMER_ENABLE) &&
+(gtb->control & R_CONTROL_COMP_ENABLE)) {
+/* R2p0+, where the compare function is >= */
+while (gtb->compare < update.new) {
+DB_PRINT("Compare event happened for CPU %d\n", i);
+gtb->status = 1;
+if (gtb->control & R_CONTROL_AUTO_INCREMENT) {
+DB_PRINT("Auto incrementing timer compare by %" PRId32 
"\n",
+ gtb->inc);
+gtb->compare += gtb->inc;
+} else {
+break;
+}
+}
+cdiff = (int64_t)gtb->compare - (int64_t)update.new + 1;
+ 

[Qemu-devel] [PATCH arm-devs v3 4/4] cpu/a9mpcore: Add Global Timer

[Peter Crosthwaite]

From: François LEGAL 

Add the global timer to A9 MPCore.

Signed-off-by: François LEGAL 
[PC Changes:
 * new commit message
 * split off original version as a separate patch
 * Rebased against new mpcore implementation (with struct embedding)
]
Signed-off-by: Peter Crosthwaite 
---
changed from v2:
Rebased against major mpcore changed.
Changed from v1:
Set authorship to Francois and added SOB.

 hw/cpu/a9mpcore.c | 26 +-
 include/hw/cpu/a9mpcore.h |  2 ++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index a38464b..c09358c 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -30,6 +30,9 @@ static void a9mp_priv_initfn(Object *obj)
 object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
 qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
 
+object_initialize(&s->gtimer, sizeof(s->gtimer), TYPE_A9_GTIMER);
+qdev_set_parent_bus(DEVICE(&s->gtimer), sysbus_get_default());
+
 object_initialize(&s->mptimer, sizeof(s->mptimer), TYPE_ARM_MPTIMER);
 qdev_set_parent_bus(DEVICE(&s->mptimer), sysbus_get_default());
 
@@ -41,8 +44,9 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
 {
 SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 A9MPPrivState *s = A9MPCORE_PRIV(dev);
-DeviceState *scudev, *gicdev, *mptimerdev, *wdtdev;
-SysBusDevice *scubusdev, *gicbusdev, *mptimerbusdev, *wdtbusdev;
+DeviceState *scudev, *gicdev, *gtimerdev, *mptimerdev, *wdtdev;
+SysBusDevice *scubusdev, *gicbusdev, *gtimerbusdev, *mptimerbusdev,
+ *wdtbusdev;
 Error *err = NULL;
 int i;
 
@@ -71,6 +75,15 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
 /* Pass through inbound GPIO lines to the GIC */
 qdev_init_gpio_in(dev, a9mp_priv_set_irq, s->num_irq - 32);
 
+gtimerdev = DEVICE(&s->gtimer);
+qdev_prop_set_uint32(gtimerdev, "num-cpu", s->num_cpu);
+object_property_set_bool(OBJECT(&s->gtimer), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
+gtimerbusdev = SYS_BUS_DEVICE(&s->gtimer);
+
 mptimerdev = DEVICE(&s->mptimer);
 qdev_prop_set_uint32(mptimerdev, "num-cpu", s->num_cpu);
 object_property_set_bool(OBJECT(&s->mptimer), true, "realized", &err);
@@ -97,14 +110,14 @@ static void a9mp_priv_realize(DeviceState *dev, Error 
**errp)
  *  0x0600-0x06ff -- private timers and watchdogs
  *  0x0700-0x0fff -- nothing
  *  0x1000-0x1fff -- GIC Distributor
- *
- * We should implement the global timer but don't currently do so.
  */
 memory_region_add_subregion(&s->container, 0,
 sysbus_mmio_get_region(scubusdev, 0));
 /* GIC CPU interface */
 memory_region_add_subregion(&s->container, 0x100,
 sysbus_mmio_get_region(gicbusdev, 1));
+memory_region_add_subregion(&s->container, 0x200,
+sysbus_mmio_get_region(gtimerbusdev, 0));
 /* Note that the A9 exposes only the "timer/watchdog for this core"
  * memory region, not the "timer/watchdog for core X" ones 11MPcore has.
  */
@@ -116,10 +129,13 @@ static void a9mp_priv_realize(DeviceState *dev, Error 
**errp)
 sysbus_mmio_get_region(gicbusdev, 0));
 
 /* Wire up the interrupt from each watchdog and timer.
- * For each core the timer is PPI 29 and the watchdog PPI 30.
+ * For each core the global timer is PPI 27, the private
+ * timer is PPI 29 and the watchdog PPI 30.
  */
 for (i = 0; i < s->num_cpu; i++) {
 int ppibase = (s->num_irq - 32) + i * 32;
+sysbus_connect_irq(gtimerbusdev, i,
+   qdev_get_gpio_in(gicdev, ppibase + 27));
 sysbus_connect_irq(mptimerbusdev, i,
qdev_get_gpio_in(gicdev, ppibase + 29));
 sysbus_connect_irq(wdtbusdev, i,
diff --git a/include/hw/cpu/a9mpcore.h b/include/hw/cpu/a9mpcore.h
index 8eece07..5d67ca2 100644
--- a/include/hw/cpu/a9mpcore.h
+++ b/include/hw/cpu/a9mpcore.h
@@ -14,6 +14,7 @@
 #include "hw/intc/arm_gic.h"
 #include "hw/misc/a9scu.h"
 #include "hw/timer/arm_mptimer.h"
+#include "hw/timer/a9gtimer.h"
 
 #define TYPE_A9MPCORE_PRIV "a9mpcore_priv"
 #define A9MPCORE_PRIV(obj) \
@@ -30,6 +31,7 @@ typedef struct A9MPPrivState {
 
 A9SCUState scu;
 GICState gic;
+A9GTimerState gtimer;
 ARMMPTimerState mptimer;
 ARMMPTimerState wdt;
 } A9MPPrivState;
-- 
1.8.4.4

[Qemu-devel] [PATCH 1/2] include/qemu: introduce sizes.h

[Antony Pavlov]

The header file sizes.h is used in linux kernel,
barebox bootloader and u-boot bootloader. It provides
the short and easy-to-read names for power-of-two
numbers. The numbers like this are othen used
for memory range sizes.

Signed-off-by: Antony Pavlov 
Reviewed-by: Richard Henderson 
---
 include/qemu/sizes.h | 61 
 1 file changed, 61 insertions(+)
 create mode 100644 include/qemu/sizes.h

diff --git a/include/qemu/sizes.h b/include/qemu/sizes.h
new file mode 100644
index 000..a683b60
--- /dev/null
+++ b/include/qemu/sizes.h
@@ -0,0 +1,61 @@
+/*
+ * Handy size definitions
+ *
+ * Copyright (C) 2013 Antony Pavlov 
+ *
+ * Inspired by linux/sizes.h
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef QEMU_SIZES_H
+#define QEMU_SIZES_H
+
+#include 
+
+#define SZ_1BIT(0)
+#define SZ_2BIT(1)
+#define SZ_4BIT(2)
+#define SZ_8BIT(3)
+#define SZ_16   BIT(4)
+#define SZ_32   BIT(5)
+#define SZ_64   BIT(6)
+#define SZ_128  BIT(7)
+#define SZ_256  BIT(8)
+#define SZ_512  BIT(9)
+
+#define SZ_1K   BIT(10)
+#define SZ_2K   BIT(11)
+#define SZ_4K   BIT(12)
+#define SZ_8K   BIT(13)
+#define SZ_16K  BIT(14)
+#define SZ_32K  BIT(15)
+#define SZ_64K  BIT(16)
+#define SZ_128K BIT(17)
+#define SZ_256K BIT(18)
+#define SZ_512K BIT(19)
+
+#define SZ_1M   BIT(20)
+#define SZ_2M   BIT(21)
+#define SZ_4M   BIT(22)
+#define SZ_8M   BIT(23)
+#define SZ_16M  BIT(24)
+#define SZ_32M  BIT(25)
+#define SZ_64M  BIT(26)
+#define SZ_128M BIT(27)
+#define SZ_256M BIT(28)
+#define SZ_512M BIT(29)
+
+#define SZ_1G   BIT(30)
+#define SZ_2G   BIT(31)
+
+#endif /* QEMU_SIZES_H */
-- 
1.8.4.4

[Qemu-devel] [PATCH 0/2] use sizes.h macros for power-of-two sizes

[Antony Pavlov]

[PATCH 1/2] include/qemu: introduce sizes.h
[PATCH 2/2] hw/mips: use sizes.h macros

The sizes.h macros is a easy-to-read method of
power-of-two memory sizes representation. The sizes.h
macros are actively used in linux kernel and other
projects, so let's use them in QEMU too.

[Qemu-devel] [PATCH arm-devs v3 2/4] cpu/a9mpcore: reorder operations/declarations

[Peter Crosthwaite]

To make it consistent for easier code reading. The order in which
variables are defined and functions are called is set to match the
address map ordering.

The new  consistent order of doing stuff is:

SCU -> GIC -> MPTimer -> WDT.

0 functional change.

Signed-off-by: Peter Crosthwaite 
---

 hw/cpu/a9mpcore.c | 28 ++--
 include/hw/cpu/a9mpcore.h |  2 +-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index 1123101..a38464b 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -24,12 +24,12 @@ static void a9mp_priv_initfn(Object *obj)
 memory_region_init(&s->container, obj, "a9mp-priv-container", 0x2000);
 sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->container);
 
-object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
-qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
-
 object_initialize(&s->scu, sizeof(s->scu), TYPE_A9_SCU);
 qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
 
+object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
+qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
+
 object_initialize(&s->mptimer, sizeof(s->mptimer), TYPE_ARM_MPTIMER);
 qdev_set_parent_bus(DEVICE(&s->mptimer), sysbus_get_default());
 
@@ -41,11 +41,20 @@ static void a9mp_priv_realize(DeviceState *dev, Error 
**errp)
 {
 SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 A9MPPrivState *s = A9MPCORE_PRIV(dev);
-DeviceState *gicdev, *scudev, *mptimerdev, *wdtdev;
-SysBusDevice *mptimerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
+DeviceState *scudev, *gicdev, *mptimerdev, *wdtdev;
+SysBusDevice *scubusdev, *gicbusdev, *mptimerbusdev, *wdtbusdev;
 Error *err = NULL;
 int i;
 
+scudev = DEVICE(&s->scu);
+qdev_prop_set_uint32(scudev, "num-cpu", s->num_cpu);
+object_property_set_bool(OBJECT(&s->scu), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
+scubusdev = SYS_BUS_DEVICE(&s->scu);
+
 gicdev = DEVICE(&s->gic);
 qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
 qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
@@ -62,15 +71,6 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
 /* Pass through inbound GPIO lines to the GIC */
 qdev_init_gpio_in(dev, a9mp_priv_set_irq, s->num_irq - 32);
 
-scudev = DEVICE(&s->scu);
-qdev_prop_set_uint32(scudev, "num-cpu", s->num_cpu);
-object_property_set_bool(OBJECT(&s->scu), true, "realized", &err);
-if (err != NULL) {
-error_propagate(errp, err);
-return;
-}
-scubusdev = SYS_BUS_DEVICE(&s->scu);
-
 mptimerdev = DEVICE(&s->mptimer);
 qdev_prop_set_uint32(mptimerdev, "num-cpu", s->num_cpu);
 object_property_set_bool(OBJECT(&s->mptimer), true, "realized", &err);
diff --git a/include/hw/cpu/a9mpcore.h b/include/hw/cpu/a9mpcore.h
index 010489b..8eece07 100644
--- a/include/hw/cpu/a9mpcore.h
+++ b/include/hw/cpu/a9mpcore.h
@@ -28,8 +28,8 @@ typedef struct A9MPPrivState {
 MemoryRegion container;
 uint32_t num_irq;
 
-GICState gic;
 A9SCUState scu;
+GICState gic;
 ARMMPTimerState mptimer;
 ARMMPTimerState wdt;
 } A9MPPrivState;
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v3 0/4] A9 global timer + mpcore trivials

[Peter Crosthwaite]

Hi Peter,

Another spin of the ARM MPCore global timer work. Patches 1 & 2 are some
trivial cleanup to MPCore I did along the way.

Regards,
Peter


François LEGAL (1):
  cpu/a9mpcore: Add Global Timer

Peter Crosthwaite (3):
  cpu/a9mpcore: rename timerbusdev variable
  cpu/a9mpcore: reorder operations/declarations
  hw/timer: Introduce ARM A9 Global Timer.

 hw/cpu/a9mpcore.c   |  44 --
 hw/timer/Makefile.objs  |   2 +-
 hw/timer/a9gtimer.c | 369 
 include/hw/cpu/a9mpcore.h   |   4 +-
 include/hw/timer/a9gtimer.h |  97 
 5 files changed, 500 insertions(+), 16 deletions(-)
 create mode 100644 hw/timer/a9gtimer.c
 create mode 100644 include/hw/timer/a9gtimer.h

-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v3 1/4] cpu/a9mpcore: rename timerbusdev variable

[Peter Crosthwaite]

Rename this variable for consistency with the above defined mptimerdev
variable.

Signed-off-by: Peter Crosthwaite 
---

 hw/cpu/a9mpcore.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index 918a7d1..1123101 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -42,7 +42,7 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
 SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 A9MPPrivState *s = A9MPCORE_PRIV(dev);
 DeviceState *gicdev, *scudev, *mptimerdev, *wdtdev;
-SysBusDevice *timerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
+SysBusDevice *mptimerbusdev, *wdtbusdev, *gicbusdev, *scubusdev;
 Error *err = NULL;
 int i;
 
@@ -78,7 +78,7 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
 error_propagate(errp, err);
 return;
 }
-timerbusdev = SYS_BUS_DEVICE(&s->mptimer);
+mptimerbusdev = SYS_BUS_DEVICE(&s->mptimer);
 
 wdtdev = DEVICE(&s->wdt);
 qdev_prop_set_uint32(wdtdev, "num-cpu", s->num_cpu);
@@ -109,7 +109,7 @@ static void a9mp_priv_realize(DeviceState *dev, Error 
**errp)
  * memory region, not the "timer/watchdog for core X" ones 11MPcore has.
  */
 memory_region_add_subregion(&s->container, 0x600,
-sysbus_mmio_get_region(timerbusdev, 0));
+sysbus_mmio_get_region(mptimerbusdev, 0));
 memory_region_add_subregion(&s->container, 0x620,
 sysbus_mmio_get_region(wdtbusdev, 0));
 memory_region_add_subregion(&s->container, 0x1000,
@@ -120,7 +120,7 @@ static void a9mp_priv_realize(DeviceState *dev, Error 
**errp)
  */
 for (i = 0; i < s->num_cpu; i++) {
 int ppibase = (s->num_irq - 32) + i * 32;
-sysbus_connect_irq(timerbusdev, i,
+sysbus_connect_irq(mptimerbusdev, i,
qdev_get_gpio_in(gicdev, ppibase + 29));
 sysbus_connect_irq(wdtbusdev, i,
qdev_get_gpio_in(gicdev, ppibase + 30));
-- 
1.8.4.4

Re: [Qemu-devel] [PATCH V2 0/8] qapi script: support enum as discriminator and better enum name

[Wenchao Xia]

于 2013/11/26 0:47, Luiz Capitulino 写道:

On Wed, 13 Nov 2013 06:25:00 +0800
Wenchao Xia  wrote:


This series is respined from RFC series at:
http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html

Patch 1-6 add support for enum as discriminator.
Patch 7 improve enum name generation, now AIOContext->AIO_CONTEXT, X86CPU->
X86_CPU.
Patch 8 are the test cases.


Can you please clarify what is the problem this series is trying to
solve, how it does it and provide before/after type of examples?

That's what I'd expect from an intro email, but this one has only a
reference to an RFC series that has no better info, and some crypt
changelog with magic numbers :(

Besides, this doesn't apply anymore...


  Let me have an introduction:

1. support using enum as discriminator in union.
For example, if we have following define in qapi schema:
{ 'enum': 'EnumOne',
  'data': [ 'value1', 'value2', 'value3' ] }

{ 'type': 'UserDefBase0',
  'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }

Before this series, discriminator in union must be a string, and a
hidden enum type as discriminator is generated. After this series,
qapi schema can directly use predefined enum type:
{ 'union': 'UserDefEnumDiscriminatorUnion',
  'base': 'UserDefBase0',
  'discriminator' : 'base-enum0',
  'data': { 'value1' : 'UserDefA',
'value2' : 'UserDefInherit',
'value3' : 'UserDefB' } }

The implement is done by:
1.1 remember the enum defines by qapi scripts.(patch 1)
1.2 use the remembered enum define to check correctness at compile
time.(patch 3)
1.3 use the same enum name generation rule to avoid C code mismatch,
esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
1.4 when pre-defined enum type is used as discriminator, don't generate
a hidden enum type, switch the code path to support enum as
discriminator.
1.5 test case shows how it looks like.(Patch 8)

2. Better enum name generation
Before this patch, AIOContext->A_I_O_Context, after this patch,
AIOContet->AIO_Context. Since previous patch has foldered enum
name generation codes into one function, it is done easily by modifying
it.(Patch 7)



Changes from RFC:
   Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
enum value in union, add related test case, remove direct inherit support 
"_base"
and related test case.

v2:
   General:
   3: use Raise exception instead of sys.error.write in qapi.py.
   Address Eric's comments:
   2,3: more check for enum value at compile time.
   8: correspond test case change.

Wenchao Xia (8):
   1 qapi script: remember enum values
   2 qapi script: add check for duplicated key
   3 qapi script: check correctness of discriminator values in union
   4 qapi script: code move for generate_enum_name()
   5 qapi script: use same function to generate enum string
   6 qapi script: not generate hidden enum type for pre-defined enum 
discriminator
   7 qapi script: do not add "_" for every capitalized char in enum
   8 tests: add cases for inherited struct and union with discriminator

  include/qapi/qmp/qerror.h   |2 +-
  scripts/qapi-types.py   |   34 
  scripts/qapi-visit.py   |   55 +--
  scripts/qapi.py |   84 -
  target-i386/cpu.c   |2 +-
  tests/qapi-schema/comments.out  |2 +-
  tests/qapi-schema/qapi-schema-test.json |   27 ++
  tests/qapi-schema/qapi-schema-test.out  |   15 +++-
  tests/test-qmp-input-visitor.c  |  120 +
  tests/test-qmp-output-visitor.c |  149 +++
  10 files changed, 454 insertions(+), 36 deletions(-)

Re: [Qemu-devel] Patch v3 : POSIX timer implementation for linux-user.

[Erik de Castro Lopo]

Erik de Castro Lopo wrote:

> Erik de Castro Lopo wrote:
> 
> > mle...@mega-nerd.com wrote:
> > 
> > > 
> > > Changes from original:
> > > 
> > > * Call host's libc functions directly rather than _syscall*() (as 
> > > suggested
> > >   by Peter Maydell).
> > > * Remove un-needed #defines.
> > > 
> > > Launchpad bug is here: https://bugs.launchpad.net/bugs/1042388
> > 
> > 
> > Ping?
> > http://patchwork.ozlabs.org/patch/284786/
> 
> Anyone willing to take a look at this one?


Ok, 1.7 has been released and 2.0 is open. Can someone now please 
look at this one?

Cheers,
Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/

Re: [Qemu-devel] [PATCH] qom: abort on error in property setter if caller passed errp == NULL

[Peter Crosthwaite]

Hi,

On Thu, Nov 28, 2013 at 11:24 AM, Igor Mammedov  wrote:
> in case if caller setting property doesn't care about error and
> passes in NULL as errp argument but error occurs in property setter,
> it is silently discarded leaving object in undefined state.
>
> As result it leads to hard to find bugs, so if caller doesn't
> care about error it must be sure that property exists and
> accepts provided value, otherwise it's better to abort early
> since error case couldn't be handled gracefully and find
> invalid usecase early.
>
> In addition multitude of property setters will be always
> guarantied to have error object present and won't be required
> to handle this condition individually.
>

So there seems to be contention between different APIs and use cases
on what to do in the NULL case. Personally, I'm of the opinion that
NULL should be a silent don't care policy. But you are right in that
checking every API call at call site is cumbersome, and its better to
have some sort of collective mechanism to implement different policys.
I think this can be done caller side efficiently by having a special
Error * that can be passed in that tells the error API to assert or
error raise.

extern error *abort_on_err;

And update your call sites to do this:

object_property_set(Foo, bar, "baz", &abort_on_err);

Error_set and friends are then taught to recognise abort_on_err and
abort accordingly and theres no need for this form of change pattern -
its all solved in the error API.

You can also implement a range of automatic error handling policies e.g:

extern Error *report_err; /* report any errors to monitor/stdout */

To report an error without the assertion.

Regards,
Peter

> Signed-off-by: Igor Mammedov 
> ---
>  qom/object.c | 19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index fc19cf6..2c0bb64 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -792,16 +792,25 @@ void object_property_get(Object *obj, Visitor *v, const 
> char *name,
>  void object_property_set(Object *obj, Visitor *v, const char *name,
>   Error **errp)
>  {
> -ObjectProperty *prop = object_property_find(obj, name, errp);
> -if (prop == NULL) {
> -return;
> +Error *local_error = NULL;
> +ObjectProperty *prop = object_property_find(obj, name, &local_error);
> +if (local_error) {
> +goto out;
>  }
>
>  if (!prop->set) {
> -error_set(errp, QERR_PERMISSION_DENIED);
> +error_set(&local_error, QERR_PERMISSION_DENIED);
>  } else {
> -prop->set(obj, v, prop->opaque, name, errp);
> +prop->set(obj, v, prop->opaque, name, &local_error);
>  }
> +out:
> +if (local_error) {
> +if (!errp) {
> +assert_no_error(local_error);
> +}
> +error_propagate(errp, local_error);
> +}
> +
>  }
>
>  void object_property_set_str(Object *obj, const char *value,
> --
> 1.8.3.1
>
>

Re: [Qemu-devel] [PATCH] qom: abort on error in property setter if caller passed errp == NULL

[Eric Blake]

On 11/27/2013 06:24 PM, Igor Mammedov wrote:
> in case if caller setting property doesn't care about error and
> passes in NULL as errp argument but error occurs in property setter,
> it is silently discarded leaving object in undefined state.
> 
> As result it leads to hard to find bugs, so if caller doesn't
> care about error it must be sure that property exists and
> accepts provided value, otherwise it's better to abort early
> since error case couldn't be handled gracefully and find
> invalid usecase early.
> 
> In addition multitude of property setters will be always
> guarantied to have error object present and won't be required

s/guarantied/guaranteed/

> to handle this condition individually.
> 
> Signed-off-by: Igor Mammedov 
> ---

> +out:
> +if (local_error) {

This says local_error was set...

> +if (!errp) {
> +assert_no_error(local_error);

so this assert_no_error() is dead code in its current position.  To be
useful, you probably want:

if (!errp) {
assert_no_error(local_error);
} else if (local_error) {
error_propagate(errp, local_error);
}

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH qom v1 1/1] qom/object.c: Split out object and class caches.

[Peter Crosthwaite]

The object-cast and class-cast caches cannot be shared because class
caching is conditional on the target type not being an interface and
object caching is unconditional. Leads to a bug when a class cast
to an interface follows an object cast to the same interface type:

FooObject = FOO(obj);
FooClass = FOO_GET_CLASS(obj);

Where TYPE_FOO is an interface. The first (object) cast will be
successful and cache the casting result (i.e. TYPE_FOO will be cached).
The second (class) cast will then check the shared cast cache
and register a hit. The issue is, when a class cast hits in the cache
it just returns a pointer cast of the input class (i.e. the concrete
class).

When casting to an interface, the cast itself must return the
interface class, not the concrete class. The implementation of class
cast caching already ensures that the returned cast result is only
a pointer cast before caching. The object cast logic however does
not have this check.

Resolve by just splitting the object and class caches.

Signed-off-by: Peter Crosthwaite 
---

 include/qom/object.h |  3 ++-
 qom/object.c | 13 +++--
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index a275db2..5f78847 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -358,7 +358,8 @@ struct ObjectClass
 Type type;
 GSList *interfaces;
 
-const char *cast_cache[OBJECT_CLASS_CAST_CACHE];
+const char *object_cast_cache[OBJECT_CLASS_CAST_CACHE];
+const char *class_cast_cache[OBJECT_CLASS_CAST_CACHE];
 
 ObjectUnparent *unparent;
 };
diff --git a/qom/object.c b/qom/object.c
index fc19cf6..21b5a0b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -458,7 +458,7 @@ Object *object_dynamic_cast_assert(Object *obj, const char 
*typename,
 Object *inst;
 
 for (i = 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) {
-if (obj->class->cast_cache[i] == typename) {
+if (obj->class->object_cast_cache[i] == typename) {
 goto out;
 }
 }
@@ -475,9 +475,10 @@ Object *object_dynamic_cast_assert(Object *obj, const char 
*typename,
 
 if (obj && obj == inst) {
 for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
-obj->class->cast_cache[i - 1] = obj->class->cast_cache[i];
+obj->class->object_cast_cache[i - 1] =
+obj->class->object_cast_cache[i];
 }
-obj->class->cast_cache[i - 1] = typename;
+obj->class->object_cast_cache[i - 1] = typename;
 }
 
 out:
@@ -547,7 +548,7 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass 
*class,
 int i;
 
 for (i = 0; class && i < OBJECT_CLASS_CAST_CACHE; i++) {
-if (class->cast_cache[i] == typename) {
+if (class->class_cast_cache[i] == typename) {
 ret = class;
 goto out;
 }
@@ -568,9 +569,9 @@ ObjectClass *object_class_dynamic_cast_assert(ObjectClass 
*class,
 #ifdef CONFIG_QOM_CAST_DEBUG
 if (class && ret == class) {
 for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
-class->cast_cache[i - 1] = class->cast_cache[i];
+class->class_cast_cache[i - 1] = class->class_cast_cache[i];
 }
-class->cast_cache[i - 1] = typename;
+class->class_cast_cache[i - 1] = typename;
 }
 out:
 #endif
-- 
1.8.4.4

Re: [Qemu-devel] outlined TLB lookup on x86

[Xin Tong]

On Wed, Nov 27, 2013 at 6:12 PM, Richard Henderson  wrote:

> On 11/27/2013 08:41 PM, Xin Tong wrote:
> > I am trying to implement a out-of-line TLB lookup for QEMU
> softmmu-x86-64 on
> > x86-64 machine, potentially for better instruction cache performance, I
> have a
> > few  questions.
> >
> > 1. I see that tcg_out_qemu_ld_slow_path/tcg_out_qemu_st_slow_path are
> generated
> > when tcg_out_tb_finalize is called. And when a TLB lookup misses, it
> jumps to
> > the generated slow path and slow path refills the TLB, then load/store
> and
> > jumps to the next emulated instruction. I am wondering is it easy to
> outline
> > the code for the slow path.
>
> Hard.  There's quite a bit of code on that slow path that's unique to the
> surrounding code context -- which registers contain inputs and outputs,
> where
> to continue after slow path.
>
> The amount of code that's in the TB slow path now is approximately
> minimal, as
> far as I can see.  If you've got an idea for improvement, please share.
>  ;-)
>
>
> > I am thinking when a TLB misses, the outlined TLB
> > lookup code should generate a call out to the qemu_ld/st_helpers[opc &
> > ~MO_SIGN] and rewalk the TLB after its refilled ? This code is off the
> critical
> > path, so its not as important as the code when TLB hits.
>
> That would work for true TLB misses to RAM, but does not work for memory
> mapped
> I/O.
>
> > 2. why not use a TLB or bigger size?  currently the TLB has 1<<8
> entries. the
> > TLB lookup is 10 x86 instructions , but every miss needs ~450
> instructions, i
> > measured this using Intel PIN. so even the miss rate is low (say 3%) the
> > overall time spent in the cpu_x86_handle_mmu_fault is still signifcant.
>
> I'd be interested to experiment with different TLB sizes, to see what
> effect
> that has on performance.  But I suspect that lack of TLB contexts mean
> that we
> wind up flushing the TLB more often than real hardware does, and therefore
> a
> larger TLB merely takes longer to flush.


Hardware TLBs are limited in size primarily due to the fact that increasing
their sizes increases their access latency as well. but software tlb does
not suffer from that problem. so i think the size of the softtlb should be
not influenced by the size of the hardware tlb.

Flushing the TLB is minimal unless we have a really really large TLB, e.g.
a TLB with 1M entries. I vaguely remember that i see ~8% of the time is
spent in the cpu_x86_mmu_fault function in one of the speccpu2006 workload
some time ago. so if we increase the size of the TLB significantly and
potential getting rid of most of the TLB misses, we can get rid of most of
the 8%. ( there are still compulsory misses and a few conflict misses, but
i think compulsory misses is not the major player here).

>
> But be aware that we can't simply make the change universally.  E.g. ARM
> can
> use an immediate 8-bit operand during the TLB lookup, but would have to use
> several insns to perform a 9-bit mask.


This can be handled with ifndefs. most of the tlb code common to all cpus
need not be changed.

> 
>
> >  I am
> > thinking the tlb may need to be organized in a set associative fashion to
> > reduce conflict miss, e.g. 2 way set associative to reduce the miss
> rate. or
> > have a victim tlb that is 4 way associative and use x86 simd
> instructions to do
> > the lookup once the direct-mapped tlb misses. Has anybody done any work
> on this
> > front ?
>
> Even with SIMD, I don't believe you could make the fast-path of a set
> associative lookup fast.  This is the sort of thing for which you really
> need
> the dedicated hardware of the real TLB.  Feel free to prove me wrong with
> code,
> of course.
>

I am thinking the primary TLB should remain what it is, i.e. a direct
mapped. but we can have a victim TLB with bigger assoiciatity. the victim
TLB can be walked either sequentially or in parallel with simd
instructions. its going to be slower than hitting in the direct mapped TLB.
but (much) better than having to rewalk the page table.

For OS with ASID, we can have a shared TLB with ASID. and this can
potentially get rid of some compulsory misses for us. e.g. multiple threads
in a process share the same ASID and we can check for the shared TLB on
miss before walk the page table.

>
>
> r~
>

Re: [Qemu-devel] [PATCH V6 0/6] qcow2: rollback the modification on fail in snapshot creation

[Wenchao Xia]

Hi, Any comments for it? respin?

Re: [Qemu-devel] [RFC] create a single workqueue for each vm to update vm irq routing table

[Zhanghaoyu (A)]

>> > I don't think a workqueue is even needed.  You just need to use 
>> > call_rcu to free "old" after releasing kvm->irq_lock.
>> > 
>> > What do you think?
>> 
>> It should be rate limited somehow. Since it guest triggarable guest 
>> may cause host to allocate a lot of memory this way.
>
Why does "use call_rcu to free "old" after releasing kvm->irq_lock" may cause 
host to allocate a lot of memory?
Do you mean that malicious guest's frequent irq-routing-table updating 
operations will result in too many delayed mem-free of old irq-routing-tables?

Thanks,
Zhang Haoyu

>True, though if I understand Zhanghaoyu's proposal a workqueue would be even 
>worse.
>

>Paolo

Re: [Qemu-devel] [PATCH v5 4/7] block: Add checks of blocker in block operations

[Fam Zheng]

On 2013年11月27日 00:13, Paolo Bonzini wrote:

Il 26/11/2013 05:05, Fam Zheng ha scritto:

--- a/blockdev.c
+++ b/blockdev.c
@@ -1001,6 +1001,11 @@ SnapshotInfo 
*qmp_blockdev_snapshot_delete_internal_sync(const char *device,
  return NULL;
  }

+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
+   errp)) {
+return NULL;
+}


Why not check in bdrv_snapshot_delete...


  ret = bdrv_snapshot_find_by_id_and_name(bs, id, name, &sn, &local_err);
  if (error_is_set(&local_err)) {
  error_propagate(errp, local_err);
@@ -1098,6 +1103,10 @@ static void 
internal_snapshot_prepare(BlkTransactionState *common,
  return;
  }

+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, errp)) {
+return;
+}


... and bdrv_snapshot_create?



OK.


  if (!bdrv_is_inserted(bs)) {
  error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
  return;
@@ -1536,6 +1545,10 @@ void qmp_change_blockdev(const char *device, const char 
*filename,
  return;
  }

+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_CHANGE, errp)) {
+return;
+}
+
  if (format) {
  drv = bdrv_find_whitelisted_format(format, bs->read_only);
  if (!drv) {
@@ -1684,6 +1697,10 @@ void qmp_block_resize(const char *device, int64_t size, 
Error **errp)
  return;
  }

+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, errp)) {
+return;
+}
+
  if (size < 0) {
  error_set(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size");
  return;
--


This should be redundant since bdrv_truncate already checks.



It doesn't hurt and skips bdrv_drain_all if op is blocked.

Fam

[Qemu-devel] [PATCH arm-devs v2 8/8] arm/highbank.c: Fix MPCore periphbase name

[Peter Crosthwaite]

GIC_BASE_ADDR is not the base address of the GIC. Its clear from the
code that this is the base address of the MPCore. Rename to
MPCORE_PERIPHBASE accordingly.

Signed-off-by: Peter Crosthwaite 
---

 hw/arm/highbank.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index cb32325..be2cc20 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -28,11 +28,11 @@
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
 
-#define SMP_BOOT_ADDR 0x100
-#define SMP_BOOT_REG  0x40
-#define GIC_BASE_ADDR 0xfff1
+#define SMP_BOOT_ADDR   0x100
+#define SMP_BOOT_REG0x40
+#define MPCORE_PERIPHBASE   0xfff1
 
-#define NIRQ_GIC  160
+#define NIRQ_GIC160
 
 /* Board init.  */
 
@@ -55,7 +55,7 @@ static void hb_write_secondary(ARMCPU *cpu, const struct 
arm_boot_info *info)
 0xe1110001, /* tst r1, r1 */
 0x0afb, /* beq  */
 0xe12fff11, /* bx  r1 */
-GIC_BASE_ADDR  /* privbase: gic address.  */
+MPCORE_PERIPHBASE   /* privbase: gic address.  */
 };
 for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
 smpboot[n] = tswap32(smpboot[n]);
@@ -236,7 +236,8 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
cxmachines machine)
 
 cpu = ARM_CPU(object_new(object_class_get_name(oc)));
 
-object_property_set_int(OBJECT(cpu), GIC_BASE_ADDR, "reset-cbar", 
&err);
+object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar",
+&err);
 if (err) {
 error_report("%s", error_get_pretty(err));
 exit(1);
@@ -287,7 +288,7 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
cxmachines machine)
 qdev_prop_set_uint32(dev, "num-irq", NIRQ_GIC);
 qdev_init_nofail(dev);
 busdev = SYS_BUS_DEVICE(dev);
-sysbus_mmio_map(busdev, 0, GIC_BASE_ADDR);
+sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
 for (n = 0; n < smp_cpus; n++) {
 sysbus_connect_irq(busdev, n, cpu_irq[n]);
 }
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 7/8] arm/xilinx_zynq: Implement CBAR intialisation

[Peter Crosthwaite]

Fix the CBAR initialisation by using the newly defined static property.
Zynq will now correctly init the CBAR to the SCU base address.

Needed to boot Linux on the xilinx_zynq machine model.

Signed-off-by: Peter Crosthwaite 
---
changed since v1:
use error report rather than fprintf(stderr
rename SCU_BASE_ADDR to MPCORE_PERIPHBASE

 hw/arm/xilinx_zynq.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 1c954a3..17251c7 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -36,6 +36,8 @@
 
 #define IRQ_OFFSET 32 /* pic interrupts start from index 32 */
 
+#define MPCORE_PERIPHBASE 0xF8F0
+
 static const int dma_irqs[8] = {
 46, 47, 48, 49, 72, 73, 74, 75
 };
@@ -122,6 +124,11 @@ static void zynq_init(QEMUMachineInitArgs *args)
 
 cpu = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
 
+object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", 
&err);
+if (err) {
+error_report("%s", error_get_pretty(err));
+exit(1);
+}
 object_property_set_bool(OBJECT(cpu), true, "realized", &err);
 if (err) {
 error_report("%s", error_get_pretty(err));
@@ -160,7 +167,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
 qdev_prop_set_uint32(dev, "num-cpu", 1);
 qdev_init_nofail(dev);
 busdev = SYS_BUS_DEVICE(dev);
-sysbus_mmio_map(busdev, 0, 0xF8F0);
+sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
 sysbus_connect_irq(busdev, 0,
qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
 
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 6/8] arm/xilinx_zynq: Use object_new() rather than cpu_arm_init()

[Peter Crosthwaite]

To allow the machine model to set device properties before CPU
realization.

Signed-off-by: Peter Crosthwaite 
---
changed since v1:
use error report rather than fprintf(stderr

 hw/arm/xilinx_zynq.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 46924a0..1c954a3 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -25,6 +25,7 @@
 #include "sysemu/blockdev.h"
 #include "hw/loader.h"
 #include "hw/ssi.h"
+#include "qemu/error-report.h"
 
 #define NUM_SPI_FLASHES 4
 #define NUM_QSPI_FLASHES 2
@@ -102,6 +103,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
 const char *kernel_filename = args->kernel_filename;
 const char *kernel_cmdline = args->kernel_cmdline;
 const char *initrd_filename = args->initrd_filename;
+ObjectClass *cpu_oc;
 ARMCPU *cpu;
 MemoryRegion *address_space_mem = get_system_memory();
 MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
@@ -110,15 +112,19 @@ static void zynq_init(QEMUMachineInitArgs *args)
 SysBusDevice *busdev;
 qemu_irq pic[64];
 NICInfo *nd;
+Error *err = NULL;
 int n;
 
 if (!cpu_model) {
 cpu_model = "cortex-a9";
 }
+cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
 
-cpu = cpu_arm_init(cpu_model);
-if (!cpu) {
-fprintf(stderr, "Unable to find CPU definition\n");
+cpu = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
+
+object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+if (err) {
+error_report("%s", error_get_pretty(err));
 exit(1);
 }
 
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 5/8] arm/highbank: Fix CBAR intialisation

[Peter Crosthwaite]

Fix the CBAR initialisation by using the newly defined static property.
CBAR is now set before realization, so the intended value is now
actually used.

So I have kinda tested this. I booted an ARM kernel on Highbank with the
stock Highbank DTB. It doesnt boot (and I will be doing something
wrong), but before this patch I got this:

[ cut here ]
WARNING: CPU: 0 PID: 0 at 
/workspaces/pcrost/public/linux2.git/arch/arm/mm/ioremap.c:301 
__arm_ioremap_pfn_caller+0x180/0x198()
CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW 
3.13.0-rc1-next-20131126-dirty #2
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0x78/0x90)
[] (dump_stack) from [] (warn_slowpath_common+0x68/0x84)
[] (warn_slowpath_common) from [] 
(warn_slowpath_null+0x1c/0x24)
[] (warn_slowpath_null) from [] 
(__arm_ioremap_pfn_caller+0x180/0x198)
[] (__arm_ioremap_pfn_caller) from [] 
(__arm_ioremap_caller+0x54/0x5c)
[] (__arm_ioremap_caller) from [] (__arm_ioremap+0x18/0x1c)
[] (__arm_ioremap) from [] (highbank_init_irq+0x34/0x8c)
[] (highbank_init_irq) from [] (init_IRQ+0x28/0x2c)
[] (init_IRQ) from [] (start_kernel+0x234/0x398)
[] (start_kernel) from [<8074>] (0x8074)
---[ end trace 3406ff24bd97382f ]---

Which dissappears with this patch.

Signed-off-by: Peter Crosthwaite 
---
changed since v1:
use error report rather than fprintf(stderr

 hw/arm/highbank.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 1d19d8f..cb32325 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -236,14 +236,16 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
cxmachines machine)
 
 cpu = ARM_CPU(object_new(object_class_get_name(oc)));
 
+object_property_set_int(OBJECT(cpu), GIC_BASE_ADDR, "reset-cbar", 
&err);
+if (err) {
+error_report("%s", error_get_pretty(err));
+exit(1);
+}
 object_property_set_bool(OBJECT(cpu), true, "realized", &err);
 if (err) {
 error_report("%s", error_get_pretty(err));
 exit(1);
 }
-
-/* This will become a QOM property eventually */
-cpu->reset_cbar = GIC_BASE_ADDR;
 cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
 }
 
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 3/8] target-arm/cpu: Convert reset CBAR to a property

[Peter Crosthwaite]

The reset Value of the CP15 CBAR is a vendor (machine) configurable
property. If ARM_FEATURE_CBAR is set, add it as a property at
post_init time.

Signed-off-by: Peter Crosthwaite 
---
Change since v1:
Re-implement as dynamic property

 target-arm/cpu.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index a82fa61..7ad2496 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -20,6 +20,7 @@
 
 #include "cpu.h"
 #include "qemu-common.h"
+#include "qapi/qmp/qerror.h"
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/loader.h"
 #endif
@@ -223,6 +224,18 @@ static void arm_cpu_initfn(Object *obj)
 }
 }
 
+static void arm_cpu_post_init(Object *obj)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+Error *err = NULL;
+
+if (arm_feature(&cpu->env, ARM_FEATURE_CBAR)) {
+object_property_add_uint32_ptr(obj, "reset-cbar", &cpu->reset_cbar,
+   &err);
+assert_no_error(err);
+}
+}
+
 static void arm_cpu_finalizefn(Object *obj)
 {
 ARMCPU *cpu = ARM_CPU(obj);
@@ -933,6 +946,7 @@ static const TypeInfo arm_cpu_type_info = {
 .parent = TYPE_CPU,
 .instance_size = sizeof(ARMCPU),
 .instance_init = arm_cpu_initfn,
+.instance_post_init = arm_cpu_post_init,
 .instance_finalize = arm_cpu_finalizefn,
 .abstract = true,
 .class_size = sizeof(ARMCPUClass),
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 4/8] arm/highbank: Use object_new() rather than cpu_arm_init()

[Peter Crosthwaite]

To allow the machine model to set device properties before CPU
realization.

Signed-off-by: Peter Crosthwaite 
---
changed since v1:
use error_report rather than fprintf(stderr

 hw/arm/highbank.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index fe98ef1..1d19d8f 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -26,6 +26,7 @@
 #include "hw/boards.h"
 #include "sysemu/blockdev.h"
 #include "exec/address-spaces.h"
+#include "qemu/error-report.h"
 
 #define SMP_BOOT_ADDR 0x100
 #define SMP_BOOT_REG  0x40
@@ -229,10 +230,15 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum 
cxmachines machine)
 }
 
 for (n = 0; n < smp_cpus; n++) {
+ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
 ARMCPU *cpu;
-cpu = cpu_arm_init(cpu_model);
-if (cpu == NULL) {
-fprintf(stderr, "Unable to find CPU definition\n");
+Error *err = NULL;
+
+cpu = ARM_CPU(object_new(object_class_get_name(oc)));
+
+object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+if (err) {
+error_report("%s", error_get_pretty(err));
 exit(1);
 }
 
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 2/8] target-arm: Define and use ARM_FEATURE_CBAR

[Peter Crosthwaite]

Some processors (notably A9 within Highbank) define and use the
CP15 configuration base address (CBAR). This is vendor specific
so its best implemented as a CPU property (otherwise we would need
vendor specific child classes for every ARM implementation).

This patch prepares support for converting CBAR reset value to
a CPU property by moving the CP registration out of the CPU
init fn, as registration will need to happen at realize time
to pick up any property updates. The easiest way to do this
is via definition of a new ARM_FEATURE to flag the existance
of the register.

Signed-off-by: Peter Crosthwaite 
---

 target-arm/cpu.c| 11 ++-
 target-arm/cpu.h|  1 +
 target-arm/helper.c |  9 +
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d40f2a7..a82fa61 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -585,6 +585,7 @@ static void cortex_a9_initfn(Object *obj)
 set_feature(&cpu->env, ARM_FEATURE_VFP_FP16);
 set_feature(&cpu->env, ARM_FEATURE_NEON);
 set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
+set_feature(&cpu->env, ARM_FEATURE_CBAR);
 /* Note that A9 supports the MP extensions even for
  * A9UP and single-core A9MP (which are both different
  * and valid configurations; we don't model A9UP).
@@ -612,15 +613,7 @@ static void cortex_a9_initfn(Object *obj)
 cpu->clidr = (1 << 27) | (1 << 24) | 3;
 cpu->ccsidr[0] = 0xe00fe015; /* 16k L1 dcache. */
 cpu->ccsidr[1] = 0x200fe015; /* 16k L1 icache. */
-{
-ARMCPRegInfo cbar = {
-.name = "CBAR", .cp = 15, .crn = 15,  .crm = 0, .opc1 = 4,
-.opc2 = 0, .access = PL1_R|PL3_W, .resetvalue = cpu->reset_cbar,
-.fieldoffset = offsetof(CPUARMState, cp15.c15_config_base_address)
-};
-define_one_arm_cp_reg(cpu, &cbar);
-define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
-}
+define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
 }
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 9f110f1..859750a 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -461,6 +461,7 @@ enum arm_features {
 ARM_FEATURE_CACHE_DIRTY_REG, /* 1136/1176 cache dirty status register */
 ARM_FEATURE_CACHE_BLOCK_OPS, /* v6 optional cache block operations */
 ARM_FEATURE_MPIDR, /* has cp15 MPIDR */
+ARM_FEATURE_CBAR, /* has cp15 CBAR */
 ARM_FEATURE_PXN, /* has Privileged Execute Never bit */
 ARM_FEATURE_LPAE, /* has Large Physical Address Extension */
 ARM_FEATURE_V8,
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3445813..1bf0305 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1744,6 +1744,15 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 define_one_arm_cp_reg(cpu, &auxcr);
 }
 
+if (arm_feature(env, ARM_FEATURE_CBAR)) {
+ARMCPRegInfo cbar = {
+.name = "CBAR", .cp = 15, .crn = 15, .crm = 0, .opc1 = 4, .opc2 = 
0,
+.access = PL1_R|PL3_W, .resetvalue = cpu->reset_cbar,
+.fieldoffset = offsetof(CPUARMState, cp15.c15_config_base_address)
+};
+define_one_arm_cp_reg(cpu, &cbar);
+}
+
 /* Generic registers whose values depend on the implementation */
 {
 ARMCPRegInfo sctlr = {
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 1/8] qom/object: Make uintXX added properties writable

[Peter Crosthwaite]

Currently the uintXX property adders make a read only property. This
is not useful for devices that want to create board (or container)
configurable dynamic device properties. Fix by trivally adding property
setters to object_property_add_uintXX.

Signed-off-by: Peter Crosthwaite 
---

 qom/object.c | 44 
 1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index fc19cf6..07b454b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1353,6 +1353,15 @@ static void property_get_uint8_ptr(Object *obj, Visitor 
*v,
 visit_type_uint8(v, &value, name, errp);
 }
 
+static void property_set_uint8_ptr(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+uint8_t value;
+visit_type_uint8(v, &value, name, errp);
+*(uint8_t *)opaque = value;
+}
+
 static void property_get_uint16_ptr(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -1361,6 +1370,15 @@ static void property_get_uint16_ptr(Object *obj, Visitor 
*v,
 visit_type_uint16(v, &value, name, errp);
 }
 
+static void property_set_uint16_ptr(Object *obj, Visitor *v,
+void *opaque, const char *name,
+Error **errp)
+{
+uint16_t value;
+visit_type_uint16(v, &value, name, errp);
+*(uint16_t *)opaque = value;
+}
+
 static void property_get_uint32_ptr(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -1369,6 +1387,15 @@ static void property_get_uint32_ptr(Object *obj, Visitor 
*v,
 visit_type_uint32(v, &value, name, errp);
 }
 
+static void property_set_uint32_ptr(Object *obj, Visitor *v,
+void *opaque, const char *name,
+Error **errp)
+{
+uint32_t value;
+visit_type_uint32(v, &value, name, errp);
+*(uint32_t *)opaque = value;
+}
+
 static void property_get_uint64_ptr(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -1377,32 +1404,41 @@ static void property_get_uint64_ptr(Object *obj, 
Visitor *v,
 visit_type_uint64(v, &value, name, errp);
 }
 
+static void property_set_uint64_ptr(Object *obj, Visitor *v,
+void *opaque, const char *name,
+Error **errp)
+{
+uint64_t value;
+visit_type_uint64(v, &value, name, errp);
+*(uint64_t *)opaque = value;
+}
+
 void object_property_add_uint8_ptr(Object *obj, const char *name,
const uint8_t *v, Error **errp)
 {
 object_property_add(obj, name, "uint8", property_get_uint8_ptr,
-NULL, NULL, (void *)v, errp);
+property_set_uint8_ptr, NULL, (void *)v, errp);
 }
 
 void object_property_add_uint16_ptr(Object *obj, const char *name,
 const uint16_t *v, Error **errp)
 {
 object_property_add(obj, name, "uint16", property_get_uint16_ptr,
-NULL, NULL, (void *)v, errp);
+property_set_uint16_ptr, NULL, (void *)v, errp);
 }
 
 void object_property_add_uint32_ptr(Object *obj, const char *name,
 const uint32_t *v, Error **errp)
 {
 object_property_add(obj, name, "uint32", property_get_uint32_ptr,
-NULL, NULL, (void *)v, errp);
+property_set_uint32_ptr, NULL, (void *)v, errp);
 }
 
 void object_property_add_uint64_ptr(Object *obj, const char *name,
 const uint64_t *v, Error **errp)
 {
 object_property_add(obj, name, "uint64", property_get_uint64_ptr,
-NULL, NULL, (void *)v, errp);
+property_set_uint64_ptr, NULL, (void *)v, errp);
 }
 
 static void object_instance_init(Object *obj)
-- 
1.8.4.4

[Qemu-devel] [PATCH arm-devs v2 0/8] Fix Support for ARM A9 CBAR

[Peter Crosthwaite]

Hi Peter,

This patch series fixed the Configuration base address init logic for
ARM CPUs, most notably for A9. Fixes both Zynq and Highbank which both
had broken CBAR.

Regards,
Peter

Changed since v1:
Fix QOM to support writeable dynamic properties
Use dynamic props instead (PMM/AF discussion)
Use error_report (AF reivew)
Use reset- prefix on propname (AF reivew)
Fix machine model namings or the MPCore PERIPHBASE


Peter Crosthwaite (8):
  qom/object: Make uintXX added properties writable
  target-arm: Define and use ARM_FEATURE_CBAR
  target-arm/cpu: Convert reset CBAR to a property
  arm/highbank: Use object_new() rather than cpu_arm_init()
  arm/highbank: Fix CBAR intialisation
  arm/xilinx_zynq: Use object_new() rather than cpu_arm_init()
  arm/xilinx_zynq: Implement CBAR intialisation
  arm/highbank.c: Fix MPCore periphbase name

 hw/arm/highbank.c| 33 +
 hw/arm/xilinx_zynq.c | 21 +
 qom/object.c | 44 
 target-arm/cpu.c | 25 -
 target-arm/cpu.h |  1 +
 target-arm/helper.c  |  9 +
 6 files changed, 104 insertions(+), 29 deletions(-)

-- 
1.8.4.4

[Qemu-devel] [Bug 739785] Re: qemu-i386 user mode can't fork (bash: fork: Invalid argument)

[Bug Watch Updater]

** Changed in: qemu (Debian)
   Status: Confirmed => Fix Released

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

Title:
  qemu-i386 user mode can't fork (bash: fork: Invalid argument)

Status in QEMU:
  Fix Committed
Status in “qemu” package in Debian:
  Fix Released

Bug description:
  Good time of day everybody,

  I have been trying to make usermode qemu on ARM with plugapps
  (archlinux) with archlinux i386 chroot to work.

  1. I installed arch linux in a virtuabox and created a chroot for it with 
mkarchroot. Transferred it to my pogo plug into /i386/
  2. I comiled qemu-i386 static and put it into /i386/usr/bin/
  ./configure --static --disable-blobs --disable-system 
--target-list=i386-linux-user
  make

  3. I also compiled linux kernel 2.6.38 with CONFIG_BINFMT_MISC=y and 
installed it.
  uname -a
  Linux Plugbox 2.6.38 #4 PREEMPT Fri Mar 18 22:19:10 CDT 2011 armv5tel 
Feroceon 88FR131 rev 1 (v5l) Marvell SheevaPlug Reference Board GNU/Linux

  4. Added the following options into /etc/rc.local
  /sbin/modprobe binfmt_misc
  /bin/mount binfmt_misc -t binfmt_misc /proc/sys/fs/binfmt_misc
  echo 
':qemu-i386:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00:\xff\xff\xff\xff\xff\xfe\xfe\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfb\xff\xff\xff:/usr/bin/qemu-i386:'
 >/proc/sys/fs/binfmt_misc/register

  5. Also copied ld-linux.so.3 (actually ld-2.13.so because ld-
  linux.so.3 is a link to that file) from /lib/ to /i386/lib/

  6.Now i chroot into /i386 and I get this:
  [root@Plugbox i386]# chroot .
  [II aI hnve ao n@P /]# pacman -Suy
  bash: fork: Invalid argument

  7.I also downloaded linux-user-test-0.3 from qemu website and ran the test:
  [root@Plugbox linux-user-test-0.3]# make
  ./qemu-linux-user.sh
  [qemu-i386]
  ../qemu-0.14.0/i386-linux-user/qemu-i386 -L ./gnemul/qemu-i386 i386/ls -l 
dummyfile
  BUG IN DYNAMIC LINKER ld.so: dl-version.c: 210: _dl_check_map_versions: 
Assertion `needed != ((void *)0)' failed!
  make: *** [test] Error 127

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

[Qemu-devel] [Bug 1239008] Re: qemu fails to scroll screen on ^Vidmem output

[Serge Hallyn]

Thanks for reporting this bug.  Is there any other reproducer you can
give us?

** No longer affects: kvm (Ubuntu)

** Also affects: qemu (Ubuntu)
   Importance: Undecided
   Status: New

** Changed in: qemu (Ubuntu)
   Status: New => Incomplete

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  qemu fails to scroll screen on ^Vidmem output

Status in QEMU:
  Incomplete
Status in “qemu” package in Ubuntu:
  Incomplete

Bug description:
  Pascal uses ^Vidmem for B800 console output. The terminal does not
  oblige the Pascal OS code to scroll the output. Virtualbox emulation
  works, so this must be a qemu bug. Using QEMU in KVM mode as Ubuntu
  LTS.

  Source line to trip bug(in theory pushes VideoMem up one line):

  procedure Scroll;
  //this is whats causing crashes. FIXME:Virtualbox not affected.QEMU BUG?
  begin
if scrolldisabled then exit;
if (CursorPosY >= 24) then begin  //in case called before end of screen
  blank:= $20 or (TextAttr shl 8);
  Move((VidMem+(2*80))^,VidMem^,24*(2*80));
  // Empty last line
  FillWord((VidMem+(24*2*80))^,80,Blank);
  CursorPosX:=1;
  CursorPosY:=23;
  update_cursor;
end;
  end;

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

[Qemu-devel] usecase for QEMU

[Xin Tong]

I am wondering what are some of the use cases for QEMU as an instruction
set emulator(not KVM). I know QEMU is used for the android emulator and
QEMU is used to host a few cycle accurate simulators ?

what else ?

Thank you,
Xin

Re: [Qemu-devel] outlined TLB lookup on x86

[Richard Henderson]

On 11/27/2013 08:41 PM, Xin Tong wrote:
> I am trying to implement a out-of-line TLB lookup for QEMU softmmu-x86-64 on
> x86-64 machine, potentially for better instruction cache performance, I have a
> few  questions.
> 
> 1. I see that tcg_out_qemu_ld_slow_path/tcg_out_qemu_st_slow_path are 
> generated
> when tcg_out_tb_finalize is called. And when a TLB lookup misses, it jumps to
> the generated slow path and slow path refills the TLB, then load/store and
> jumps to the next emulated instruction. I am wondering is it easy to outline
> the code for the slow path.

Hard.  There's quite a bit of code on that slow path that's unique to the
surrounding code context -- which registers contain inputs and outputs, where
to continue after slow path.

The amount of code that's in the TB slow path now is approximately minimal, as
far as I can see.  If you've got an idea for improvement, please share.  ;-)


> I am thinking when a TLB misses, the outlined TLB
> lookup code should generate a call out to the qemu_ld/st_helpers[opc &
> ~MO_SIGN] and rewalk the TLB after its refilled ? This code is off the 
> critical
> path, so its not as important as the code when TLB hits.

That would work for true TLB misses to RAM, but does not work for memory mapped
I/O.

> 2. why not use a TLB or bigger size?  currently the TLB has 1<<8 entries. the
> TLB lookup is 10 x86 instructions , but every miss needs ~450 instructions, i
> measured this using Intel PIN. so even the miss rate is low (say 3%) the
> overall time spent in the cpu_x86_handle_mmu_fault is still signifcant.

I'd be interested to experiment with different TLB sizes, to see what effect
that has on performance.  But I suspect that lack of TLB contexts mean that we
wind up flushing the TLB more often than real hardware does, and therefore a
larger TLB merely takes longer to flush.

But be aware that we can't simply make the change universally.  E.g. ARM can
use an immediate 8-bit operand during the TLB lookup, but would have to use
several insns to perform a 9-bit mask.

>  I am
> thinking the tlb may need to be organized in a set associative fashion to
> reduce conflict miss, e.g. 2 way set associative to reduce the miss rate. or
> have a victim tlb that is 4 way associative and use x86 simd instructions to 
> do
> the lookup once the direct-mapped tlb misses. Has anybody done any work on 
> this
> front ?

Even with SIMD, I don't believe you could make the fast-path of a set
associative lookup fast.  This is the sort of thing for which you really need
the dedicated hardware of the real TLB.  Feel free to prove me wrong with code,
of course.


r~

Re: [Qemu-devel] outlined TLB lookup on x86

[Xin Tong]

Hi LIuis

we can probably generate vector intrinsics using the tcg, e.g. add support
to tcg to emit vector instructions directly in code cache

why would a larger TLB make some operations slower, the TLB is a
direct-mapped hash and lookup should be O(1) there. In the cputlb, the 
CPU_TLB_SIZE  is always used to index into the TLB, i.e. (X & (CPU_TLB_SIZE
 -1)).

Thank you
Xin


On Wed, Nov 27, 2013 at 5:12 AM, Lluís Vilanova  wrote:

> Xin Tong writes:
>
> > I am trying to implement a out-of-line TLB lookup for QEMU
> softmmu-x86-64 on
> > x86-64 machine, potentially for better instruction cache performance, I
> have a
> > few questions.
>
> > 1. I see that tcg_out_qemu_ld_slow_path/tcg_out_qemu_st_slow_path are
> generated
> > when tcg_out_tb_finalize is called. And when a TLB lookup misses, it
> jumps to
> > the generated slow path and slow path refills the TLB, then load/store
> and jumps
> > to the next emulated instruction. I am wondering is it easy to outline
> the code
> > for the slow path. I am thinking when a TLB misses, the outlined TLB
> lookup code
> > should generate a call out to the qemu_ld/st_helpers[opc & ~MO_SIGN] and
> rewalk
> > the TLB after its refilled ? This code is off the critical path, so its
> not as
> > important as the code when TLB hits.
> > 2. why not use a TLB or bigger size? currently the TLB has 1<<8 entries.
> the TLB
> > lookup is 10 x86 instructions , but every miss needs ~450 instructions, i
> > measured this using Intel PIN. so even the miss rate is low (say 3%) the
> overall
> > time spent in the cpu_x86_handle_mmu_fault is still signifcant. I am
> thinking
> > the tlb may need to be organized in a set associative fashion to reduce
> conflict
> > miss, e.g. 2 way set associative to reduce the miss rate. or have a
> victim tlb
> > that is 4 way associative and use x86 simd instructions to do the lookup
> once
> > the direct-mapped tlb misses. Has anybody done any work on this front ?
> > 3. what are some of the drawbacks of using a superlarge TLB, i.e. a TLB
> with 4K
> > entries ?
>
> Using vector intrinsics for the TLB lookup will probably make the code less
> portable. I don't know how compatible are the GCC and LLVM vectorizing
> intrinsics between each other (since there has been some efforts on making
> QEMU
> also compile with LLVM).
>
> A larger TLB will make some operations slower (e.g., look for CPU_TLB_SIZE
> in
> cputlb.c), but the higher hit ratio could pay off, although I don't know
> how the
> current size was chosen.
>
>
> 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 v2 2/2] qemu-iotests: Add sample image and test for VMDK version 3

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/059|   5 +
 tests/qemu-iotests/059.out|   5 +
 tests/qemu-iotests/sample_images/iotest-version3.vmdk.bz2 | Bin 0 -> 414 bytes
 3 files changed, 10 insertions(+)
 create mode 100644 tests/qemu-iotests/sample_images/iotest-version3.vmdk.bz2

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 6a27ac9..4926645 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -75,6 +75,11 @@ echo
 echo "=== Testing monolithicFlat with zeroed_grain ==="
 IMGOPTS="subformat=monolithicFlat,zeroed_grain=on" _make_test_img 2G
 
+echo
+echo "=== Testing version 3 ==="
+_use_sample_img iotest-version3.vmdk.bz2
+_img_info
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 2ded8a9..0aadd56 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -25,4 +25,9 @@ virtual size: 2.0G (2147483648 bytes)
 === Testing monolithicFlat with zeroed_grain ===
 qemu-img: TEST_DIR/t.IMGFMT: Flat image can't enable zeroed grain
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648
+
+=== Testing version 3 ===
+image: TEST_DIR/iotest-version3.IMGFMT
+file format: IMGFMT
+virtual size: 1.0G (1073741824 bytes)
 *** done
diff --git a/tests/qemu-iotests/sample_images/iotest-version3.vmdk.bz2 
b/tests/qemu-iotests/sample_images/iotest-version3.vmdk.bz2
new file mode 100644
index 
..30abf217e72d38e97b1e34a5db6add15ca3812d9
GIT binary patch
literal 414
zcmV;P0b%|^T4*^jL0KkKS-4ab^Z){!-`M?Q}oWup($ZYNuP+onz
zC89&oo+5m#SRo3f$tj3j4cj-uY<9AFkA%eEJTaa)*;e;U06X$QBY(3aGcne4@Z5Q`
zOld1c2c2n5eaHx1cF@GqGfd88M9D~UB914v;WOL96@WSu-<9};viD>nCB_lZ=L5MA
zWAhh{E!lb{@J3L6J+%};gkaKNrNrxivB)qiB85a>b>(iaNAFyd9Z|7XRWs!&h*2TY
z2uM_*TOC2Lp=5yL%rudbN4;JP^)svmH_P%A-P+35?p=v$3M>tU*);6x0S=pmRj{{|
z7O

Re: [Qemu-devel] [PATCH arm-devs v1 2/6] target-arm/cpu: Convert reset CBAR to a property

[Peter Crosthwaite]

On Wed, Nov 27, 2013 at 9:47 PM, Peter Maydell  wrote:
> On 27 November 2013 11:39, Peter Crosthwaite
>  wrote:
>> Is the "periphbase" ever runtime configurable? If not I'm not sure we
>> need the "reset".
>
> You can't runtime configure it (it's a bunch of signals into the
> core that determine where the decoder sits the peripherals in
> the memory map).

So that is a top level signal of MPCore rather than A9 CPUs right?
Assuming so, that name "PERIPHBASE" is a ideally property of the the
mpcore container device. And in this ideal world that container
contains the A9 CPUs themselves and propagates "periphbase" through to
the CPU as "cbar" during its own init/realize. Looking at ARM docco,
the definition of CBAR == PERIPHBASE is A9MPCore specific. From ARM
infocentre:


Cortex-A9 Technical Reference ManualRevision: r4p1
Home > System Control > Register descriptions > Configuration Base
Address Register

4.3.24. Configuration Base Address Registe

...
Configurations

In Cortex-A9 uniprocessor implementations the base address is set to zero.

In Cortex-A9 MPCore implementations, the base address is reset to
PERIPHBASE[31:13] so that software can determine the location of the
private memory region"
---

So the best name for this register AFAICT is simply CBAR and either
MPCore container or whatever are responsible for setting it to an
appropriate value.

> However, the CBAR register which on reset
> starts out with the value of the base address is a writable
> register (writing it won't change where the peripherals live,
> it just reads-as-written).
>

So with that in mind i think the "reset-" prefix is appropriate.

Regards,
Peter

> -- PMM
>

[Qemu-devel] [PATCH v2 1/2] vmdk: Allow read only open of VMDK version 3

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 block/vmdk.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index a7ebd0f..6fd20dc 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -605,13 +605,20 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
 header = footer.header;
 }
 
-if (le32_to_cpu(header.version) >= 3) {
+if (le32_to_cpu(header.version) > 3) {
 char buf[64];
 snprintf(buf, sizeof(buf), "VMDK version %d",
  le32_to_cpu(header.version));
 qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
 bs->device_name, "vmdk", buf);
 return -ENOTSUP;
+} else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) {
+/* VMware KB 2064959 explains that version 3 added support for
+ * persistent changed block tracking (CBT), and backup software can
+ * read it as version=1 if it doesn't care about the changed area
+ * information. So we are safe to enable read only. */
+error_setg(errp, "VMDK version 3 must be read only");
+return -EINVAL;
 }
 
 if (le32_to_cpu(header.num_gtes_per_gt) > 512) {
-- 
1.8.4.2

[Qemu-devel] [PATCH v2 0/2] vmdk: Allow version 3 read only open

[Fam Zheng]

According to an update on VMware Knowledge Base [KB 2064959], we should be safe
to open version 3 as read only. This is meaningful as an compatibility
improvement, so let's enable it.

v2: [01] Parentheses around "flags & BDRV_O_RDWR". (Paolo)
 Add comments. (Stefan)


Fam Zheng (2):
  vmdk: Allow read only open of VMDK version 3
  qemu-iotests: Add sample image and test for VMDK version 3

 block/vmdk.c  |   9 -
 tests/qemu-iotests/059|   5 +
 tests/qemu-iotests/059.out|   5 +
 tests/qemu-iotests/sample_images/iotest-version3.vmdk.bz2 | Bin 0 -> 414 bytes
 4 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/sample_images/iotest-version3.vmdk.bz2

-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 0/5] qemu-io: readline command completion

[Luiz Capitulino]

On Thu, 14 Nov 2013 11:54:13 +0100
Stefan Hajnoczi  wrote:

> This series decouples readline.c from the QEMU monitor and then reuses it in
> qemu-io.  This adds history and command completion to the qemu-io interactive
> prompt.

I don't know what's the state of this series, but I reviewed the HMP
parts in detail, and took a glance at the qemu-io parts:

Reviewed-by: Luiz Capitulino 

> 
> Stefan Hajnoczi (5):
>   readline: decouple readline from the monitor
>   readline: move readline to a generic location
>   osdep: add qemu_set_tty_echo()
>   qemu-io: use readline.c
>   qemu-io: add command completion
> 
>  Makefile.objs  |   1 -
>  hmp.c  |   6 +-
>  include/monitor/monitor.h  |   2 +-
>  include/monitor/readline.h |  56 -
>  include/qemu-io.h  |   3 +
>  include/qemu/osdep.h   |   2 +
>  include/qemu/readline.h|  62 ++
>  monitor.c  |  41 +++-
>  qemu-io-cmds.c |  15 ++
>  qemu-io.c  | 109 +-
>  readline.c | 493 
>  util/Makefile.objs |   1 +
>  util/oslib-posix.c |  18 ++
>  util/oslib-win32.c |  19 ++
>  util/readline.c| 495 
> +
>  15 files changed, 716 insertions(+), 607 deletions(-)
>  delete mode 100644 include/monitor/readline.h
>  create mode 100644 include/qemu/readline.h
>  delete mode 100644 readline.c
>  create mode 100644 util/readline.c
> 

[Qemu-devel] [PATCH] qom: abort on error in property setter if caller passed errp == NULL

[Igor Mammedov]

in case if caller setting property doesn't care about error and
passes in NULL as errp argument but error occurs in property setter,
it is silently discarded leaving object in undefined state.

As result it leads to hard to find bugs, so if caller doesn't
care about error it must be sure that property exists and
accepts provided value, otherwise it's better to abort early
since error case couldn't be handled gracefully and find
invalid usecase early.

In addition multitude of property setters will be always
guarantied to have error object present and won't be required
to handle this condition individually.

Signed-off-by: Igor Mammedov 
---
 qom/object.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index fc19cf6..2c0bb64 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -792,16 +792,25 @@ void object_property_get(Object *obj, Visitor *v, const 
char *name,
 void object_property_set(Object *obj, Visitor *v, const char *name,
  Error **errp)
 {
-ObjectProperty *prop = object_property_find(obj, name, errp);
-if (prop == NULL) {
-return;
+Error *local_error = NULL;
+ObjectProperty *prop = object_property_find(obj, name, &local_error);
+if (local_error) {
+goto out;
 }
 
 if (!prop->set) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(&local_error, QERR_PERMISSION_DENIED);
 } else {
-prop->set(obj, v, prop->opaque, name, errp);
+prop->set(obj, v, prop->opaque, name, &local_error);
 }
+out:
+if (local_error) {
+if (!errp) {
+assert_no_error(local_error);
+}
+error_propagate(errp, local_error);
+}
+
 }
 
 void object_property_set_str(Object *obj, const char *value,
-- 
1.8.3.1

Re: [Qemu-devel] [RFC qom-cpu v4 05/10] qmp: add 'cpu-del' command support

[Chen Fan]

On Wed, 2013-11-27 at 07:00 -0700, Eric Blake wrote:
> On 10/09/2013 03:43 AM, Chen Fan wrote:
> > Signed-off-by: Chen Fan 
> > ---
> >  hw/i386/pc.c |  6 ++
> >  hw/i386/pc_piix.c|  3 ++-
> >  include/hw/boards.h  |  2 ++
> >  include/hw/i386/pc.h |  1 +
> >  qapi-schema.json | 12 
> >  qmp-commands.hx  | 23 +++
> >  qmp.c|  9 +
> >  7 files changed, 55 insertions(+), 1 deletion(-)
> 
> > +++ b/qapi-schema.json
> > @@ -1479,6 +1479,18 @@
> >  ##
> >  { 'command': 'cpu-add', 'data': {'id': 'int'} }
> >  
> > +# @cpu-del
> > +
> > +# Deletes CPU with specified ID
> > +#
> > +# @id: ID of CPU to be deleted, valid values [0..max_cpus)
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since 1.7
> 
> Since this mail is still titled RFC, there's no way it will make it into
> 1.7.  Are you shooting for 1.8?
> 
Hi, this patchseries had not been commented yet, maybe I should send it
again after Rebase without RFC for 1.8. 

Thanks,
Chen

Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event

[Luiz Capitulino]

On Wed, 13 Nov 2013 09:44:52 +0800
Wenchao Xia  wrote:

> Nested structure is not supported now, so following define is not valid:
> { 'event': 'EVENT_C',
>   'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' }

I think your general approach is reasonable, but there are a number of
details to fix.

The first one is documentation. This patch's changelog is quite bad,
you don't say anything about what the does. You just mention a corner
case which doesn't happen to work. Please, add full changelog explaining
what this patch is about and please add examples of how an event
entry would look like in the schema and maybe you could also add the
important parts of a generated event function. Also, please add a
"event" section to docs/writing-qmp-commands.txt (in a different patch).

Secondly, for changes like this it's a very good idea to provide
conversion examples (as patches, not as changelog doc) at least
one or two so that we can see how it will look like.

More below.

> Signed-off-by: Wenchao Xia 
> ---
>  Makefile  |9 +-
>  Makefile.objs |2 +-
>  qapi/Makefile.objs|1 +
>  scripts/qapi-event.py |  355 
> +
>  4 files changed, 363 insertions(+), 4 deletions(-)
>  create mode 100644 scripts/qapi-event.py
> 
> diff --git a/Makefile b/Makefile
> index b15003f..a3e465f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -38,8 +38,8 @@ endif
>  endif
>  
>  GENERATED_HEADERS = config-host.h qemu-options.def
> -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
> +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
>  
>  GENERATED_HEADERS += trace/generated-events.h
>  GENERATED_SOURCES += trace/generated-events.c
> @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
>  # Build libraries
>  
>  libqemustub.a: $(stub-obj-y)
> -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
> +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
>  
>  ##
>  
> @@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json 
> $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
>  qapi-visit.c qapi-visit.h :\
>  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py 
> $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> +qapi-event.c qapi-event.h :\
> +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
> + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py 
> $(gen-out-type) -o "." -b < $<, "  GEN   $@")
>  qmp-commands.h qmp-marshal.c :\
>  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py 
> $(gen-out-type) -m -o "." < $<, "  GEN   $@")
> diff --git a/Makefile.objs b/Makefile.objs
> index 2b6c1fe..33f5950 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
>  block-obj-$(CONFIG_POSIX) += aio-posix.o
>  block-obj-$(CONFIG_WIN32) += aio-win32.o
>  block-obj-y += block/
> -block-obj-y += qapi-types.o qapi-visit.o
> +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
>  block-obj-y += qemu-io-cmds.o
>  
>  block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
> diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
> index 1f9c973..d14b769 100644
> --- a/qapi/Makefile.objs
> +++ b/qapi/Makefile.objs
> @@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o 
> qmp-dispatch.o
>  util-obj-y += string-input-visitor.o string-output-visitor.o
>  
>  util-obj-y += opts-visitor.o
> +util-obj-y += qmp-event.o
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> new file mode 100644
> index 000..4c6a0fe

I didn't review this hunk, but this series doesn't build for me:

  CCqapi/string-output-visitor.o
  CCqapi/opts-visitor.o
make: *** No rule to make target `qapi/qmp-event.o', needed by `libqemuutil.a'. 
 Stop.
~/work/src/upstream/qmp-unstable/build (tmp|AM)/ 

> --- /dev/null
> +++ b/scripts/qapi-event.py
> @@ -0,0 +1,355 @@
> +#
> +# QAPI event generator
> +#
> +# Copyright IBM, Corp. 2013
> +#
> +# Authors:
> +#  Wenchao Xia 
> +#
> +# This work is licensed under the terms of the GNU GPLv2.
> +# See the COPYING.LIB file in the top-level directory.
> +
> +from ordereddict import OrderedDict
> +from qapi import *
> +import sys
> +import os
> +import getopt
> +import errno
> +
> +def _generate_event_api_name_with_params(event_name, params):
> +api_name = "void qapi_event_gen_%s(" % c_fun(event_name);

I'd prefer a name like qmp_notify_NAME() or qmp_send_event_NAME().

> +l = len(api_name)
> +
> +for argname, argentry, optional, structured in pars

[Qemu-devel] [PATCH v2] hw/i386/pc_sysfw: support two flash drives

[Laszlo Ersek]

This patch allows the user to usefully specify

  -drive file=img_1,if=pflash,format=raw,readonly \
  -drive file=img_2,if=pflash,format=raw

on the command line. The flash images will be mapped under 4G in their
reverse unit order -- that is, with their base addresses progressing
downwards, in increasing unit order.

(The unit number increases with command line order if not explicitly
specified.)

This accommodates the following use case: suppose that OVMF is split in
two parts, a writeable host file for non-volatile variable storage, and a
read-only part for bootstrap and decompressible executable code.

The binary code part would be read-only, centrally managed on the host
system, and passed in as unit 0. The variable store would be writeable,
VM-specific, and passed in as unit 1.

  ffe0-ffe1 (prio 0, R-): system.flash1
  ffe2- (prio 0, R-): system.flash0

(If the guest tries to write to the flash range that is backed by the
read-only drive, pflash_update() is never called; various flash
programming/erase errors are returned to the guest instead. See the
callers of pflash_update(), and the initialization of "pfl->ro", in
"hw/block/pflash_cfi01.c".)

Signed-off-by: Laszlo Ersek 
---
 v2:
 - don't map flash devices beyond unit#1 [Markus]
 - explicit low bound on cumulative base address [Markus]
 - updated comment block on pc_system_flash_init() [Laszlo]
 - dropped (DriveInfo*) param of pc_system_flash_init(), use drive_get()
   internally for unit#0 too [Markus]
 - turn do-loop into for-loop [Markus]
 - check bdrv_getlength() for errors [Laszlo]
 - reject zero-sized flash [Laszlo]
 - use Location of -pflash / -drive if=pflash,... option in error
   reporting [Markus]
 - describe the real spots where write attempts to r/o flash are caught
   [Laszlo]

 hw/i386/pc_sysfw.c | 105 +++--
 1 file changed, 86 insertions(+), 19 deletions(-)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index e917c83..75a7ebb 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -72,35 +72,102 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory,
 memory_region_set_readonly(isa_bios, true);
 }
 
-static void pc_system_flash_init(MemoryRegion *rom_memory,
- DriveInfo *pflash_drv)
+#define FLASH_MAP_UNIT_MAX 2
+
+/* We don't have a theoretically justifiable exact lower bound on the base
+ * address of any flash mapping. In practice, the IO-APIC MMIO range is
+ * [0xFEE0..0xFEE01000[ -- see IO_APIC_DEFAULT_ADDRESS --, leaving free
+ * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in
+ * size.
+ */
+#define FLASH_MAP_BASE_MIN ((hwaddr)(0x1ULL - 8*1024*1024))
+
+/* This function maps flash drives from 4G downward, in order of their unit
+ * numbers. The mapping starts at unit#0, with unit number increments of 1, and
+ * stops before the first missing flash drive, or before
+ * unit#FLASH_MAP_UNIT_MAX, whichever is reached first.
+ *
+ * Addressing within one flash drive is of course not reversed.
+ *
+ * An error message is printed and the process exits if:
+ * - the size of the backing file for a flash drive is non-positive, or not a
+ *   multiple of the required sector size, or
+ * - the current mapping's base address would fall below FLASH_MAP_BASE_MIN.
+ *
+ * The drive with unit#0 (if available) is mapped at the highest address, and
+ * it is passed to pc_isa_bios_init(). Merging several drives for isa-bios is
+ * not supported.
+ */
+static void pc_system_flash_init(MemoryRegion *rom_memory)
 {
+int unit;
+DriveInfo *pflash_drv;
 BlockDriverState *bdrv;
 int64_t size;
-hwaddr phys_addr;
+char *fatal_errmsg = NULL;
+hwaddr phys_addr = 0x1ULL;
 int sector_bits, sector_size;
 pflash_t *system_flash;
 MemoryRegion *flash_mem;
+char name[64];
 
-bdrv = pflash_drv->bdrv;
-size = bdrv_getlength(pflash_drv->bdrv);
 sector_bits = 12;
 sector_size = 1 << sector_bits;
 
-if ((size % sector_size) != 0) {
-fprintf(stderr,
-"qemu: PC system firmware (pflash) must be a multiple of 
0x%x\n",
-sector_size);
-exit(1);
+for (unit = 0;
+ (unit < FLASH_MAP_UNIT_MAX &&
+  (pflash_drv = drive_get(IF_PFLASH, 0, unit)) != NULL);
+ ++unit) {
+bdrv = pflash_drv->bdrv;
+size = bdrv_getlength(bdrv);
+if (size < 0) {
+fatal_errmsg = g_strdup_printf("failed to get backing file size");
+} else if (size == 0) {
+fatal_errmsg = g_strdup_printf("PC system firmware (pflash) "
+   "cannot have zero size");
+} else if ((size % sector_size) != 0) {
+fatal_errmsg = g_strdup_printf("PC system firmware (pflash) "
+   "must be a multiple of 0x%x", sector_size);
+} else if (phys_addr < size || phys_ad

[Qemu-devel] [PATCH 07/16] target-i386: cpu: convert 'stepping' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
v3:
 - cpu_x86_properties changed to x86_cpu_properties,
   upstream rebase on top of it.
v2:
 - afaerber: inline property definition inside of
 property array.
---
 target-i386/cpu.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index efbcaba..5fb6c68 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1396,6 +1396,12 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 env->cpuid_version |= value & 0xf;
 }
 
+static PropertyInfo qdev_prop_stepping = {
+.name  = "uint32",
+.get   = x86_cpuid_version_get_stepping,
+.set   = x86_cpuid_version_set_stepping,
+};
+
 static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -2647,9 +2653,6 @@ static void x86_cpu_initfn(Object *obj)
 cs->env_ptr = env;
 cpu_exec_init(env);
 
-object_property_add(obj, "stepping", "int",
-x86_cpuid_version_get_stepping,
-x86_cpuid_version_set_stepping, NULL, NULL, NULL);
 object_property_add_str(obj, "vendor",
 x86_cpuid_get_vendor,
 x86_cpuid_set_vendor, NULL);
@@ -2722,6 +2725,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
 { .name = "family", .info = &qdev_prop_family },
 { .name = "model", .info = &qdev_prop_model },
+{ .name = "stepping", .info = &qdev_prop_stepping },
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 02/16] target-i386: cleanup 'foo=val' feature handling

[Igor Mammedov]

features family, model, stepping, level, hv_spinlocks are treated similarly
when passed from command line, so it's not necessary to handle each of them
individually. Collapse them to one catch-all branch which will treat
any not explicitly handled feature in format 'foo=val'.

PS:
Any unknown feature will be rejected by property setter so there is no
need to check for unknown feature in cpu_x86_parse_featurestr(), therefore
it's replaced by above mentioned catch-all handler.

Signed-off-by: Igor Mammedov 
---
v2:
  - style fixes
---
 target-i386/cpu.c | 17 ++---
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8cc4330..34d3968 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1706,15 +1706,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char 
*features, Error **errp)
 } else if ((val = strchr(featurestr, '='))) {
 *val = 0; val++;
 feat2prop(featurestr);
-if (!strcmp(featurestr, "family")) {
-object_property_parse(OBJECT(cpu), val, featurestr, errp);
-} else if (!strcmp(featurestr, "model")) {
-object_property_parse(OBJECT(cpu), val, featurestr, errp);
-} else if (!strcmp(featurestr, "stepping")) {
-object_property_parse(OBJECT(cpu), val, featurestr, errp);
-} else if (!strcmp(featurestr, "level")) {
-object_property_parse(OBJECT(cpu), val, featurestr, errp);
-} else if (!strcmp(featurestr, "xlevel")) {
+if (!strcmp(featurestr, "xlevel")) {
 char *err;
 char num[32];
 
@@ -1730,10 +1722,6 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char 
*features, Error **errp)
 }
 snprintf(num, sizeof(num), "%" PRIu32, numvalue);
 object_property_parse(OBJECT(cpu), num, featurestr, errp);
-} else if (!strcmp(featurestr, "vendor")) {
-object_property_parse(OBJECT(cpu), val, featurestr, errp);
-} else if (!strcmp(featurestr, "model-id")) {
-object_property_parse(OBJECT(cpu), val, featurestr, errp);
 } else if (!strcmp(featurestr, "tsc-freq")) {
 int64_t tsc_freq;
 char *err;
@@ -1765,8 +1753,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char 
*features, Error **errp)
 snprintf(num, sizeof(num), "%" PRId32, numvalue);
 object_property_parse(OBJECT(cpu), num, featurestr, errp);
 } else {
-error_setg(errp, "unrecognized feature %s", featurestr);
-goto out;
+object_property_parse(OBJECT(cpu), val, featurestr, errp);
 }
 } else {
 feat2prop(featurestr);
-- 
1.8.3.1

[Qemu-devel] [PATCH 09/16] target-i386: cpu: convert 'model-id' to static property

[Igor Mammedov]

* check "if (model_id == NULL)" looks unnecessary now, since all
builtin model-ids are not NULL and user shouldn't be able to set
it NULL (cpumodel string parsing code takes care of it, if feature
is specified as "model-id=" on command line, its parsing will
result in an empty string as value).

Signed-off-by: Igor Mammedov 
---
v4:
 - fix invalid use of error_is_set(errp) if errp == NULL
v3:
 - cpu_x86_properties changed to x86_cpu_properties
   upstream, rebase on top of it.
v2:
  - afaerber: inline property definition inside of
  property array.
---
 target-i386/cpu.c | 29 -
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1ad1087..4389ffa 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1454,7 +1454,8 @@ static PropertyInfo qdev_prop_vendor = {
 .set   = x86_cpuid_set_vendor,
 };
 
-static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
+static void x86_cpuid_get_model_id(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
 CPUX86State *env = &cpu->env;
@@ -1466,18 +1467,23 @@ static char *x86_cpuid_get_model_id(Object *obj, Error 
**errp)
 value[i] = env->cpuid_model[i >> 2] >> (8 * (i & 3));
 }
 value[48] = '\0';
-return value;
+visit_type_str(v, &value, name, errp);
+g_free(value);
 }
 
-static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
-   Error **errp)
+static void x86_cpuid_set_model_id(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
 CPUX86State *env = &cpu->env;
+Error *err = NULL;
 int c, len, i;
+char *model_id;
 
-if (model_id == NULL) {
-model_id = "";
+visit_type_str(v, &model_id, name, &err);
+if (err) {
+error_propagate(errp, err);
+return;
 }
 len = strlen(model_id);
 memset(env->cpuid_model, 0, 48);
@@ -1489,8 +1495,15 @@ static void x86_cpuid_set_model_id(Object *obj, const 
char *model_id,
 }
 env->cpuid_model[i >> 2] |= c << (8 * (i & 3));
 }
+g_free(model_id);
 }
 
+static PropertyInfo qdev_prop_model_id = {
+.name  = "string",
+.get   = x86_cpuid_get_model_id,
+.set   = x86_cpuid_set_model_id,
+};
+
 static void x86_cpuid_get_tsc_freq(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
 {
@@ -2670,9 +2683,6 @@ static void x86_cpu_initfn(Object *obj)
 cs->env_ptr = env;
 cpu_exec_init(env);
 
-object_property_add_str(obj, "model-id",
-x86_cpuid_get_model_id,
-x86_cpuid_set_model_id, NULL);
 object_property_add(obj, "tsc-frequency", "int",
 x86_cpuid_get_tsc_freq,
 x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
@@ -2741,6 +2751,7 @@ static Property x86_cpu_properties[] = {
 { .name = "model", .info = &qdev_prop_model },
 { .name = "stepping", .info = &qdev_prop_stepping },
 { .name = "vendor", .info  = &qdev_prop_vendor },
+{ .name  = "model-id", .info  = &qdev_prop_model_id },
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/16] target-i386: cpu: convert 'tsc-frequency' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
v3:
 - cpu_x86_properties changed to x86_cpu_properties
   upstream, rebase on top of it.
v2:
  - afaerber: inline property definition inside of
  property array.
---
 target-i386/cpu.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4389ffa..b4f616e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1535,6 +1535,12 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
*v, void *opaque,
 cpu->env.tsc_khz = value / 1000;
 }
 
+static PropertyInfo qdev_prop_tsc_freq = {
+.name  = "int64",
+.get   = x86_cpuid_get_tsc_freq,
+.set   = x86_cpuid_set_tsc_freq,
+};
+
 static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
   const char *name, Error **errp)
 {
@@ -2683,9 +2689,6 @@ static void x86_cpu_initfn(Object *obj)
 cs->env_ptr = env;
 cpu_exec_init(env);
 
-object_property_add(obj, "tsc-frequency", "int",
-x86_cpuid_get_tsc_freq,
-x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
 object_property_add(obj, "apic-id", "int",
 x86_cpuid_get_apic_id,
 x86_cpuid_set_apic_id, NULL, NULL, NULL);
@@ -2752,6 +2755,7 @@ static Property x86_cpu_properties[] = {
 { .name = "stepping", .info = &qdev_prop_stepping },
 { .name = "vendor", .info  = &qdev_prop_vendor },
 { .name  = "model-id", .info  = &qdev_prop_model_id },
+{ .name  = "tsc-frequency", .info  = &qdev_prop_tsc_freq },
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 04/16] target-i386: cpu: convert 'xlevel' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
v22:
 - cpu_x86_properties changed to x86_cpu_properties,
   upstream rebase on top of it.
---
 target-i386/cpu.c | 20 +---
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0f1066a..8a1f786 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1384,22 +1384,6 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 env->cpuid_version |= value & 0xf;
 }
 
-static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
-X86CPU *cpu = X86_CPU(obj);
-
-visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
-}
-
-static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
-X86CPU *cpu = X86_CPU(obj);
-
-visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
-}
-
 static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -2660,9 +2644,6 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "stepping", "int",
 x86_cpuid_version_get_stepping,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
-object_property_add(obj, "xlevel", "int",
-x86_cpuid_get_xlevel,
-x86_cpuid_set_xlevel, NULL, NULL, NULL);
 object_property_add_str(obj, "vendor",
 x86_cpuid_get_vendor,
 x86_cpuid_set_vendor, NULL);
@@ -2732,6 +2713,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
 DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
 DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
+DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 16/16] target-i386: cpu: fix invalid use of error_is_set(errp) if errp == NULL

[Igor Mammedov]

in generic case errp may be NULL and if an Error gets raised in visitor
but not set to *errp for the lack of pointer, value might be uninitialized:
object_property_parse(obj, "invalid value", "foo", NULL);
and accessed futher in property setter leading to incorrect property
value of object instance.
So we cannot rely on error_is_set(errp) but must use a local variable
to detect error condition and return earlier.

Signed-off-by: Igor Mammedov 
---
 target-i386/cpu.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2220eae..7064818 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1110,10 +1110,12 @@ static void x86_cpuid_version_set_family(Object *obj, 
Visitor *v, void *opaque,
 CPUX86State *env = &cpu->env;
 const int64_t min = 0;
 const int64_t max = 0xff + 0xf;
+Error *err = NULL;
 int64_t value;
 
-visit_type_int(v, &value, name, errp);
-if (error_is_set(errp)) {
+visit_type_int(v, &value, name, &err);
+if (err) {
+error_propagate(errp, err);
 return;
 }
 if (value < min || value > max) {
@@ -1155,10 +1157,12 @@ static void x86_cpuid_version_set_model(Object *obj, 
Visitor *v, void *opaque,
 CPUX86State *env = &cpu->env;
 const int64_t min = 0;
 const int64_t max = 0xff;
+Error *err = NULL;
 int64_t value;
 
-visit_type_int(v, &value, name, errp);
-if (error_is_set(errp)) {
+visit_type_int(v, &value, name, &err);
+if (err) {
+error_propagate(errp, err);
 return;
 }
 if (value < min || value > max) {
@@ -1197,10 +1201,12 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 CPUX86State *env = &cpu->env;
 const int64_t min = 0;
 const int64_t max = 0xf;
+Error *err = NULL;
 int64_t value;
 
-visit_type_int(v, &value, name, errp);
-if (error_is_set(errp)) {
+visit_type_int(v, &value, name, &err);
+if (err) {
+error_propagate(errp, err);
 return;
 }
 if (value < min || value > max) {
@@ -1337,10 +1343,12 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
*v, void *opaque,
 X86CPU *cpu = X86_CPU(obj);
 const int64_t min = 0;
 const int64_t max = INT64_MAX;
+Error *err = NULL;
 int64_t value;
 
-visit_type_int(v, &value, name, errp);
-if (error_is_set(errp)) {
+visit_type_int(v, &value, name, &err);
+if (err) {
+error_propagate(errp, err);
 return;
 }
 if (value < min || value > max) {
-- 
1.8.3.1

[Qemu-devel] [PATCH] Fix QEMU build on OpenBSD on x86 archs

[Brad Smith]

This resolves the build issue with building the ROMs on OpenBSD on x86 archs.
As of OpenBSD 5.3 the compiler builds PIE binaries by default and thus the
whole OS/packages and so forth. The ROMs need to have PIE disabled. This
is my initial attempt at trying to get somehting upstream so that QEMU
both builds out of the box and to resolve the build issue with the
buildbots that has been around for awhile. We have a patch in our ports
tree but it is just the flags hardcoded into the Makefile which obviously
is not appropriate for upstream.

>From the OpenBSD buildbots..
  Building optionrom/multiboot.img
ld: multiboot.o: relocation R_X86_64_16 can not be used when making a shared 
object; recompile with -fPIC


Signed-off by: Brad Smith 


diff --git a/configure b/configure
index 508f6a5..6d84885 100755
--- a/configure
+++ b/configure
@@ -1342,6 +1342,10 @@ EOF
   if compile_prog "-fPIE -DPIE" "-pie"; then
 QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
 LDFLAGS="-pie $LDFLAGS"
+if test "$targetos" = OpenBSD; then
+  CC_NOPIE="-fno-pie"
+  LD_NOPIE="-nopie"
+fi
 pie="yes"
 if compile_prog "" "-Wl,-z,relro -Wl,-z,now" ; then
   LDFLAGS="-Wl,-z,relro -Wl,-z,now $LDFLAGS"
@@ -4307,6 +4311,8 @@ if test "$gcov" = "yes" ; then
   echo "CONFIG_GCOV=y" >> $config_host_mak
   echo "GCOV=$gcov_tool" >> $config_host_mak
 fi
+echo "CC_NOPIE=$CC_NOPIE" >> $config_host_mak
+echo "LD_NOPIE=$LD_NOPIE" >> $config_host_mak
 
 # use included Linux headers
 if test "$linux" = "yes" ; then
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 57d8bd0..0b35000 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -12,6 +12,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/optionrom)
 CFLAGS := -Wall -Wstrict-prototypes -Werror -fomit-frame-pointer -fno-builtin
 CFLAGS += -I$(SRC_PATH)
 CFLAGS += $(call cc-option, $(CFLAGS), -fno-stack-protector)
+CFLAGS += $(CC_NOPIE)
 QEMU_CFLAGS = $(CFLAGS)
 
 build-all: multiboot.bin linuxboot.bin kvmvapic.bin
@@ -20,7 +21,7 @@ build-all: multiboot.bin linuxboot.bin kvmvapic.bin
 .SECONDARY:
 
 %.img: %.o
-   $(call quiet-command,$(LD) -Ttext 0 -e _start -s -o $@ $<,"  Building 
$(TARGET_DIR)$@")
+   $(call quiet-command,$(LD) $(LD_NOPIE) -Ttext 0 -e _start -s -o $@ $<," 
 Building $(TARGET_DIR)$@")
 
 %.raw: %.img
$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"  Building 
$(TARGET_DIR)$@")

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

[Qemu-devel] [PATCH 01/16] target-i386: cleanup 'foo' feature handling'

[Igor Mammedov]

features check, enforce, hv_relaxed and hv_vapic are treated as boolean set to 
'on'
when passed from command line, so it's not neccessary to handle each of them
separetly. Collapse them to one catch-all branch which will treat
any feature in format 'foo' as boolean set to 'on'.

PS:
Any unknown feature will be rejected by CPU property setter so there is no
need to check for unknown feature in cpu_x86_parse_featurestr(), therefore
it's replaced by above mentioned catch-all handler.

Signed-off-by: Igor Mammedov 
Reviewed-by: Eduardo Habkost 
---
v2:
  * use feat2prop() to perform name convertion for hv_vapic and hv_relaxed
---
 target-i386/cpu.c | 13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7f74021..8cc4330 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1768,18 +1768,9 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char 
*features, Error **errp)
 error_setg(errp, "unrecognized feature %s", featurestr);
 goto out;
 }
-} else if (!strcmp(featurestr, "check")) {
-object_property_parse(OBJECT(cpu), "on", featurestr, errp);
-} else if (!strcmp(featurestr, "enforce")) {
-object_property_parse(OBJECT(cpu), "on", featurestr, errp);
-} else if (!strcmp(featurestr, "hv_relaxed")) {
-object_property_parse(OBJECT(cpu), "on", "hv-relaxed", errp);
-} else if (!strcmp(featurestr, "hv_vapic")) {
-object_property_parse(OBJECT(cpu), "on", "hv-vapic", errp);
 } else {
-error_setg(errp, "feature string `%s' not in format (+feature|"
-   "-feature|feature=xyz)", featurestr);
-goto out;
+feat2prop(featurestr);
+object_property_parse(OBJECT(cpu), "on", featurestr, errp);
 }
 if (error_is_set(errp)) {
 goto out;
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/16] target-i386: cpu: convert 'level' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
v2:
 - cpu_x86_properties changed to x86_cpu_properties,
   upstream rebase on top of it.
---
 target-i386/cpu.c | 20 +---
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 34d3968..0f1066a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1384,22 +1384,6 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 env->cpuid_version |= value & 0xf;
 }
 
-static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
-const char *name, Error **errp)
-{
-X86CPU *cpu = X86_CPU(obj);
-
-visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
-}
-
-static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
-const char *name, Error **errp)
-{
-X86CPU *cpu = X86_CPU(obj);
-
-visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
-}
-
 static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
  const char *name, Error **errp)
 {
@@ -2676,9 +2660,6 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "stepping", "int",
 x86_cpuid_version_get_stepping,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
-object_property_add(obj, "level", "int",
-x86_cpuid_get_level,
-x86_cpuid_set_level, NULL, NULL, NULL);
 object_property_add(obj, "xlevel", "int",
 x86_cpuid_get_xlevel,
 x86_cpuid_set_xlevel, NULL, NULL, NULL);
@@ -2750,6 +2731,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
 DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
 DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
+DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 15/16] target-i386: remove unused *_feature_name arrays

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
v2:
 fix conflict when removing kvm_feature_name[] with "kvm_pv_unhalt"
 added by f010bc6 "target-i386: add feature kvm_pv_unhalt", earlier
 patch "target-i386: set [+-]feature using static properties" were
 ammended to include "feat-kvm-pv-unhalt" as static property
---
 target-i386/cpu.c | 99 ---
 1 file changed, 99 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a9297cc..2220eae 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -172,98 +172,7 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t 
vendor1,
 dst[CPUID_VENDOR_SZ] = '\0';
 }
 
-/* feature flags taken from "Intel Processor Identification and the CPUID
- * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
- * between feature naming conventions, aliases may be added.
- */
-static const char *feature_name[] = {
-"fpu", "vme", "de", "pse",
-"tsc", "msr", "pae", "mce",
-"cx8", "apic", NULL, "sep",
-"mtrr", "pge", "mca", "cmov",
-"pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
-NULL, "ds" /* Intel dts */, "acpi", "mmx",
-"fxsr", "sse", "sse2", "ss",
-"ht" /* Intel htt */, "tm", "ia64", "pbe",
-};
-static const char *ext_feature_name[] = {
-"pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor",
-"ds_cpl", "vmx", "smx", "est",
-"tm2", "ssse3", "cid", NULL,
-"fma", "cx16", "xtpr", "pdcm",
-NULL, "pcid", "dca", "sse4.1|sse4_1",
-"sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
-"tsc-deadline", "aes", "xsave", "osxsave",
-"avx", "f16c", "rdrand", "hypervisor",
-};
-/* Feature names that are already defined on feature_name[] but are set on
- * CPUID[8000_0001].EDX on AMD CPUs don't have their names on
- * ext2_feature_name[]. They are copied automatically to cpuid_ext2_features
- * if and only if CPU vendor is AMD.
- */
-static const char *ext2_feature_name[] = {
-NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
-NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
-NULL /* cx8 */ /* AMD CMPXCHG8B */, NULL /* apic */, NULL, "syscall",
-NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
-NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
-"nx|xd", NULL, "mmxext", NULL /* mmx */,
-NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb" /* AMD Page1GB */, "rdtscp",
-NULL, "lm|i64", "3dnowext", "3dnow",
-};
-static const char *ext3_feature_name[] = {
-"lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD 
ExtApicSpace */,
-"cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
-"3dnowprefetch", "osvw", "ibs", "xop",
-"skinit", "wdt", NULL, "lwp",
-"fma4", "tce", NULL, "nodeid_msr",
-NULL, "tbm", "topoext", "perfctr_core",
-"perfctr_nb", NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-};
-
-static const char *ext4_feature_name[] = {
-NULL, NULL, "xstore", "xstore-en",
-NULL, NULL, "xcrypt", "xcrypt-en",
-"ace2", "ace2-en", "phe", "phe-en",
-"pmm", "pmm-en", NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-};
-
-static const char *kvm_feature_name[] = {
-"kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock",
-"kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", "kvm_pv_unhalt",
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-};
-
-static const char *svm_feature_name[] = {
-"npt", "lbrv", "svm_lock", "nrip_save",
-"tsc_scale", "vmcb_clean",  "flushbyasid", "decodeassists",
-NULL, NULL, "pause_filter", NULL,
-"pfthreshold", NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-NULL, NULL, NULL, NULL,
-};
-
-static const char *cpuid_7_0_ebx_feature_name[] = {
-"fsgsbase", NULL, NULL, "bmi1", "hle", "avx2", NULL, "smep",
-"bmi2", "erms", "invpcid", "rtm", NULL, NULL, NULL, NULL,
-NULL, NULL, "rdseed", "adx", "smap", NULL, NULL, NULL,
-NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-};
-
 typedef struct FeatureWordInfo {
-const char **feat_names;
 uint32_t cpuid_eax;   /* Input EAX for CPUID */
 bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */
 uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
@@ -272,35 +181,27 @@ typedef struct FeatureWordInfo {
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 [FEAT_1_EDX] = {
-.feat_names = feature_name,
 .cpuid_eax = 1, .cpuid_reg = R_EDX,
 },
 [FEAT_1_ECX] = {
-.feat_names = ext_feature_name,
 .cpuid_eax = 1, .cpuid_reg = R_ECX,
 },
 [FEAT_8000_0001_EDX] = {
-.feat_names = ext2_feature_name,
 .cpuid_eax = 0x8001, .cpuid_reg = R_EDX,
  

[Qemu-devel] [PATCH 14/16] target-i386: use static properties to list CPUID features

[Igor Mammedov]

- it breaks compatibility with previous output format by printing all features
  in one string with "feat-" prefixes and all "_" replaced by "-"

Signed-off-by: Igor Mammedov 
---
 target-i386/cpu.c | 44 ++--
 1 file changed, 10 insertions(+), 34 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5c3455f..a9297cc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1742,42 +1742,13 @@ out:
 return;
 }
 
-/* generate a composite string into buf of all cpuid names in featureset
- * selected by fbits.  indicate truncation at bufsize in the event of overflow.
- * if flags, suppress names undefined in featureset.
- */
-static void listflags(char *buf, int bufsize, uint32_t fbits,
-const char **featureset, uint32_t flags)
-{
-const char **p = &featureset[31];
-char *q, *b, bit;
-int nc;
-
-b = 4 <= bufsize ? buf + (bufsize -= 3) - 1 : NULL;
-*buf = '\0';
-for (q = buf, bit = 31; fbits && bufsize; --p, fbits &= ~(1 << bit), --bit)
-if (fbits & 1 << bit && (*p || !flags)) {
-if (*p)
-nc = snprintf(q, bufsize, "%s%s", q == buf ? "" : " ", *p);
-else
-nc = snprintf(q, bufsize, "%s[%d]", q == buf ? "" : " ", bit);
-if (bufsize <= nc) {
-if (b) {
-memcpy(b, "...", sizeof("..."));
-}
-return;
-}
-q += nc;
-bufsize -= nc;
-}
-}
-
 /* generate CPU information. */
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
 x86_def_t *def;
 char buf[256];
 int i;
+const Property *prop;
 
 for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
 def = &builtin_x86_defs[i];
@@ -1791,12 +1762,17 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 #endif
 
 (*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
-for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
-FeatureWordInfo *fw = &feature_word_info[i];
 
-listflags(buf, sizeof(buf), (uint32_t)~0, fw->feat_names, 1);
-(*cpu_fprintf)(f, "  %s\n", buf);
+(*cpu_fprintf)(f, " ");
+QDEV_PROP_FOREACH(prop, object_class_by_name(TYPE_X86_CPU)) {
+const char *name = prop ? prop->name : "";
+
+if (!g_str_has_prefix(name, "feat-")) {
+continue;
+}
+(*cpu_fprintf)(f, " %s", name);
 }
+(*cpu_fprintf)(f, "\n");
 }
 
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
-- 
1.8.3.1

[Qemu-devel] [PATCH 12/16] qdev: introduce qdev_prop_find_bit()

[Igor Mammedov]

helper to find a static property corresponding to a specific bit
in a specified field.

Signed-off-by: Igor Mammedov 
---
 hw/core/qdev-properties.c| 15 +++
 include/hw/qdev-properties.h | 13 +
 2 files changed, 28 insertions(+)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index dc8ae69..5b8b059 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -965,6 +965,21 @@ static Property *qdev_prop_find(DeviceState *dev, const 
char *name)
 return NULL;
 }
 
+const Property *qdev_prop_find_bit(const DeviceClass *dc, const int offset,
+   const uint8_t bitnr)
+{
+const Property *prop;
+
+QDEV_CLASS_FOREACH(dc, dc) {
+QDEV_PROP_FOREACH(prop, dc) {
+if (prop->offset == offset && prop->bitnr == bitnr) {
+return prop;
+}
+}
+}
+return NULL;
+}
+
 void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
 Property *prop, const char *value)
 {
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 692f82e..c559197 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -195,4 +195,17 @@ void qdev_property_add_static(DeviceState *dev, Property 
*prop, Error **errp);
  */
 void qdev_prop_set_after_realize(DeviceState *dev, const char *name,
  Error **errp);
+
+#define QDEV_PROP_FOREACH(_var, _class)   \
+for ((_var) = DEVICE_CLASS((_class))->props;  \
+ (_var) && (_var)->name;  \
+ (_var)++)
+
+#define QDEV_CLASS_FOREACH(_var, _class)  \
+for ((_var) = (_class);   \
+ (_var) != DEVICE_CLASS(object_class_by_name(TYPE_DEVICE));   \
+ (_var) = DEVICE_CLASS(object_class_get_parent(OBJECT_CLASS((_var)
+
+const Property *qdev_prop_find_bit(const DeviceClass *dc, const int offset,
+   const uint8_t bitnr);
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 11/16] target-i386: set [+-]feature using static properties

[Igor Mammedov]

 * Define static properties for cpuid feature bits
* property names of CPUID feature bits are changed to have "feat-"
  prefix, so that it would be easy to distinguish them from other
  properties.

 * Convert [+-]cpuid_features to a set(QDict) of key, value pairs, where
 +foo => (feat-foo, on)
 -foo => (feat-foo, off)
 [+-] unknown foo => (feat-foo, on/off) - it's expected to be rejected
  by property setter later
   QDict is used as convinience sturcture to keep '-foo' semantic.
   Once all +-foo are parsed, collected features are applied to CPU instance.

 * When parsing 'kvmclock' feature, set additional feat-kvmclock2 feature
   in cpu_x86_parse_featurestr() to mantain legacy kvmclock flag behavior

 * Cleanup unused anymore:
 add_flagname_to_bitmaps(), lookup_feature(), altcmp(), sstrcmp(),

Signed-off-by: Igor Mammedov 
---
 v8:
   - add feat-kvm-pv-unhalt introduced by
   f010bc643 "target-i386: add feature kvm_pv_unhalt"
 v7:
   - fix mispelling "feat-kvm-steal-tm" -> "feat-kvm-steal-time"
   - cpu_x86_properties changed to x86_cpu_properties
 upstream, rebase on top of it.
 v6:
   - substitute '_' with '-' for +-foo feature bits as well
   - change "f-" feature prefix to "feat-" (req: afaerber)
   - replace F_* macros with a single X86CPU_FEAT() macro
   - simplify F_XXX macros to set default value to 0, defaults to be handled
 in initfn()
   - ammend F_XXX macros to use feature-words
   - kvmclock fix endless loop on compat kvmclock2 append
 v5:
   - instead of introducing composit f-kvmclock property to maintain
 legace behavior, set additional f-kvmclock2 in cpu_x86_parse_featurestr()
 when kvmclock is parsed.
 v4:
   - major patch reordering, rebasing on current qom-cpu-next
   - merged patches: "define static properties..." and "set [+-]feature..."
   - merge in "make 'f-kvmclock' compatible..." to aviod behavior breakage
   - use register name in feature macros, so that if we rename cpuid_* fields,
 it wouldn't involve mass rename in features array.
   - when converting feature names into property names, replace '_' with '-',
   Requested-By: Don Slutz ,
 mgs-id: <5085d4aa.1060...@cloudswitch.com>

 v3:
   - new static properties after rebase:
  - add features "f-rdseed" & "f-adx"
  - add features added by c8acc380
  - add features f-kvm_steal_tm and f-kvmclock_stable
  - ext4 features f-xstore, f-xstore-en, f-xcrypt, f-xcrypt-en,
f-ace2, f-ace2-en, f-phe, f-phe-en, f-pmm, f-pmm-en

   - f-hypervisor set default to false as for the rest of features
   - define helper FEAT for declaring CPUID feature properties to
 make shorter lines (<80 characters)

 v2:
   - use X86CPU as a type to count of offset correctly, because env field
 isn't starting at CPUstate beginning, but located after it.
---
 target-i386/cpu.c | 303 +++---
 1 file changed, 197 insertions(+), 106 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b4f616e..1503e9a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -401,85 +401,6 @@ void host_cpuid(uint32_t function, uint32_t count,
 #endif
 }
 
-#define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c)))
-
-/* general substring compare of *[s1..e1) and *[s2..e2).  sx is start of
- * a substring.  ex if !NULL points to the first char after a substring,
- * otherwise the string is assumed to sized by a terminating nul.
- * Return lexical ordering of *s1:*s2.
- */
-static int sstrcmp(const char *s1, const char *e1, const char *s2,
-const char *e2)
-{
-for (;;) {
-if (!*s1 || !*s2 || *s1 != *s2)
-return (*s1 - *s2);
-++s1, ++s2;
-if (s1 == e1 && s2 == e2)
-return (0);
-else if (s1 == e1)
-return (*s2);
-else if (s2 == e2)
-return (*s1);
-}
-}
-
-/* compare *[s..e) to *altstr.  *altstr may be a simple string or multiple
- * '|' delimited (possibly empty) strings in which case search for a match
- * within the alternatives proceeds left to right.  Return 0 for success,
- * non-zero otherwise.
- */
-static int altcmp(const char *s, const char *e, const char *altstr)
-{
-const char *p, *q;
-
-for (q = p = altstr; ; ) {
-while (*p && *p != '|')
-++p;
-if ((q == p && !*s) || (q != p && !sstrcmp(s, e, q, p)))
-return (0);
-if (!*p)
-return (1);
-else
-q = ++p;
-}
-}
-
-/* search featureset for flag *[s..e), if found set corresponding bit in
- * *pval and return true, otherwise return false
- */
-static bool lookup_feature(uint32_t *pval, const char *s, const char *e,
-   const char **featureset)
-{
-uint32_t mask;
-const char **ppc;
-bool found = false;
-
-for (mask = 1, ppc = featureset; mask; mask <<= 1, ++ppc) {
-if (*ppc && !altcmp(s, e, *ppc

[Qemu-devel] [PATCH 13/16] target-i386: use static properties in check_features_against_host() to print CPUID feature names

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
 target-i386/cpu.c | 33 ++---
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1503e9a..5c3455f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1119,24 +1119,6 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
 #endif /* CONFIG_KVM */
 }
 
-static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
-{
-int i;
-
-for (i = 0; i < 32; ++i)
-if (1 << i & mask) {
-const char *reg = get_register_name_32(f->cpuid_reg);
-assert(reg);
-fprintf(stderr, "warning: host doesn't support requested feature: "
-"CPUID.%02XH:%s%s%s [bit %d]\n",
-f->cpuid_eax, reg,
-f->feat_names[i] ? "." : "",
-f->feat_names[i] ? f->feat_names[i] : "", i);
-break;
-}
-return 0;
-}
-
 /* Check if all requested cpu flags are making their way to the guest
  *
  * Returns 0 if all flags are supported by the host, non-zero otherwise.
@@ -1175,6 +1157,7 @@ static int kvm_check_features_against_host(X86CPU *cpu)
 &host_def.features[FEAT_KVM],
 FEAT_KVM },
 };
+const DeviceClass *dc = DEVICE_CLASS(object_get_class(OBJECT(cpu)));
 
 assert(kvm_enabled());
 
@@ -1182,10 +1165,22 @@ static int kvm_check_features_against_host(X86CPU *cpu)
 for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i) {
 FeatureWord w = ft[i].feat_word;
 FeatureWordInfo *wi = &feature_word_info[w];
+int offset = (char *)&((X86CPU *)0)->env.features[w] - (char *)0;
 for (mask = 1; mask; mask <<= 1) {
 if (*ft[i].guest_feat & mask &&
 !(*ft[i].host_feat & mask)) {
-unavailable_host_feature(wi, mask);
+int bitnr = ffsl(mask) - 1;
+const Property *prop = qdev_prop_find_bit(dc, offset, bitnr);
+const char *name = prop ? prop->name : NULL;
+const char *reg = get_register_name_32(wi->cpuid_reg);
+
+assert(reg);
+fprintf(stderr, "warning: host doesn't support requested"
+"feature: CPUID.%02XH:%s%s%s [bit %d]\n",
+ wi->cpuid_eax,
+ reg, name ? "." : "",
+ name ? name : "",
+ bitnr);
 rv = 1;
 }
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/16] target-i386: cpu: convert 'model' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
v3:
  - cpu_x86_properties changed to x86_cpu_properties,
upstream rebase on top of it.
v2:
  - afaerber: inline property definition inside of
  property array.
---
 target-i386/cpu.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7483f28..efbcaba 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1354,6 +1354,12 @@ static void x86_cpuid_version_set_model(Object *obj, 
Visitor *v, void *opaque,
 env->cpuid_version |= ((value & 0xf) << 4) | ((value >> 4) << 16);
 }
 
+static PropertyInfo qdev_prop_model = {
+.name  = "uint32",
+.get   = x86_cpuid_version_get_model,
+.set   = x86_cpuid_version_set_model,
+};
+
 static void x86_cpuid_version_get_stepping(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -2641,9 +2647,6 @@ static void x86_cpu_initfn(Object *obj)
 cs->env_ptr = env;
 cpu_exec_init(env);
 
-object_property_add(obj, "model", "int",
-x86_cpuid_version_get_model,
-x86_cpuid_version_set_model, NULL, NULL, NULL);
 object_property_add(obj, "stepping", "int",
 x86_cpuid_version_get_stepping,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
@@ -2718,6 +2721,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
 DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
 { .name = "family", .info = &qdev_prop_family },
+{ .name = "model", .info = &qdev_prop_model },
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 08/16] target-i386: cpu: convert 'vendor' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
v4:
 - fix invalid use of error_is_set(errp) if errp == NULL
v3:
 - cpu_x86_properties changed to x86_cpu_properties
   upstream, rebase on top of it.
v2:
  - afaerber: inline property definition inside of
  property array.
---
 target-i386/cpu.c | 29 ++---
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5fb6c68..1ad1087 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1402,7 +1402,8 @@ static PropertyInfo qdev_prop_stepping = {
 .set   = x86_cpuid_version_set_stepping,
 };
 
-static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
+static void x86_cpuid_get_vendor(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
 CPUX86State *env = &cpu->env;
@@ -1411,16 +1412,25 @@ static char *x86_cpuid_get_vendor(Object *obj, Error 
**errp)
 value = (char *)g_malloc(CPUID_VENDOR_SZ + 1);
 x86_cpu_vendor_words2str(value, env->cpuid_vendor1, env->cpuid_vendor2,
  env->cpuid_vendor3);
-return value;
+visit_type_str(v, &value, name, errp);
+g_free(value);
 }
 
-static void x86_cpuid_set_vendor(Object *obj, const char *value,
- Error **errp)
+static void x86_cpuid_set_vendor(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
 CPUX86State *env = &cpu->env;
+Error *err = NULL;
+char *value;
 int i;
 
+visit_type_str(v, &value, name, &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
 if (strlen(value) != CPUID_VENDOR_SZ) {
 error_set(errp, QERR_PROPERTY_VALUE_BAD, "",
   "vendor", value);
@@ -1435,8 +1445,15 @@ static void x86_cpuid_set_vendor(Object *obj, const char 
*value,
 env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i);
 env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i);
 }
+g_free(value);
 }
 
+static PropertyInfo qdev_prop_vendor = {
+.name  = "string",
+.get   = x86_cpuid_get_vendor,
+.set   = x86_cpuid_set_vendor,
+};
+
 static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -2653,9 +2670,6 @@ static void x86_cpu_initfn(Object *obj)
 cs->env_ptr = env;
 cpu_exec_init(env);
 
-object_property_add_str(obj, "vendor",
-x86_cpuid_get_vendor,
-x86_cpuid_set_vendor, NULL);
 object_property_add_str(obj, "model-id",
 x86_cpuid_get_model_id,
 x86_cpuid_set_model_id, NULL);
@@ -2726,6 +2740,7 @@ static Property x86_cpu_properties[] = {
 { .name = "family", .info = &qdev_prop_family },
 { .name = "model", .info = &qdev_prop_model },
 { .name = "stepping", .info = &qdev_prop_stepping },
+{ .name = "vendor", .info  = &qdev_prop_vendor },
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 05/16] target-i386: cpu: convert 'family' to static property

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
v3:
 - cpu_x86_properties changed to x86_cpu_properties,
   upstream rebase on top of it.
v2:
  - afaerber: inline property definition inside of
  property array.
---
 target-i386/cpu.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8a1f786..7483f28 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1313,6 +1313,12 @@ static void x86_cpuid_version_set_family(Object *obj, 
Visitor *v, void *opaque,
 }
 }
 
+static PropertyInfo qdev_prop_family = {
+.name  = "uint32",
+.get   = x86_cpuid_version_get_family,
+.set   = x86_cpuid_version_set_family,
+};
+
 static void x86_cpuid_version_get_model(Object *obj, Visitor *v, void *opaque,
 const char *name, Error **errp)
 {
@@ -2635,9 +2641,6 @@ static void x86_cpu_initfn(Object *obj)
 cs->env_ptr = env;
 cpu_exec_init(env);
 
-object_property_add(obj, "family", "int",
-x86_cpuid_version_get_family,
-x86_cpuid_version_set_family, NULL, NULL, NULL);
 object_property_add(obj, "model", "int",
 x86_cpuid_version_get_model,
 x86_cpuid_version_set_model, NULL, NULL, NULL);
@@ -2714,6 +2717,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
 DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
 DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
+{ .name = "family", .info = &qdev_prop_family },
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH qom-cpu 00/16 v10] target-i386: convert CPU features into properties

[Igor Mammedov]

Changes since v9:
 * rebased on top of https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next
   based on v1.7.0-rc2 release, fixing several conflicts
 * skipped patches already commited to qom-cpu-next
 * fixed conflit introduced by
 f010bc6 "target-i386: add feature kvm_pv_unhalt",
   patch "target-i386: set [+-]feature using static properties" was
   ammended to include "feat-kvm-pv-unhalt" as static property
 * added patch to fix potentially uninitialized value accesses due to
   incorrect use of error_is_set(errp) if errp == NULL
   "target-i386: cpu: fix invalid use of error_is_set(errp) if errp == NULL"

v10 is available at: 
https://github.com/imammedo/qemu/commits/x86-cpu-properties.v10

reference to previous v9: 
http://qemu.11.n7.nabble.com/PATCH-qom-cpu-00-21-v9-target-i386-convert-CPU-features-into-properties-td213920.html

---

It's reordered and rebased v8 plus CPUID feature bits conversion to properties
and cleanups that are removing unused anymore *_feature_name arrays.

dynamic => static properties conversion is still making sense as cleanup of
initfn(), consolidating all properties in one place and making uniform
property setters signatures, so it was kept.

hyperv and dynamic => static properties conversion are covered by virt-test's
qemu_cpu test group.

On top of that, CPUID feature bits conversion and cleanups it's allowed.

git for testing: https://github.com/imammedo/qemu/tree/x86-cpu-properties.v9


Igor Mammedov (16):
  target-i386: cleanup 'foo' feature handling'
  target-i386: cleanup 'foo=val' feature handling
  target-i386: cpu: convert 'level' to static property
  target-i386: cpu: convert 'xlevel' to static property
  target-i386: cpu: convert 'family' to static property
  target-i386: cpu: convert 'model' to static property
  target-i386: cpu: convert 'stepping' to static property
  target-i386: cpu: convert 'vendor' to static property
  target-i386: cpu: convert 'model-id' to static property
  target-i386: cpu: convert 'tsc-frequency' to static property
  target-i386: set [+-]feature using static properties
  qdev: introduce qdev_prop_find_bit()
  target-i386: use static properties in check_features_against_host() to
print CPUID feature names
  target-i386: use static properties to list CPUID features
  target-i386: remove unused *_feature_name arrays
  target-i386: cpu: fix invalid use of error_is_set(errp) if errp ==
NULL

 hw/core/qdev-properties.c|  15 +
 include/hw/qdev-properties.h |  13 +
 target-i386/cpu.c| 665 ---
 3 files changed, 338 insertions(+), 355 deletions(-)

-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-1.7] Fix QEMU build on OpenBSD on x86 archs

[Brad Smith]

On Wed, Nov 27, 2013 at 11:32:51AM -0800, Anthony Liguori wrote:
> Brad Smith  writes:
> 
> > This resolves the build issue with building the ROMs on OpenBSD on x86 
> > archs.
> > As of OpenBSD 5.3 the compiler builds PIE binaries by default and thus the
> > whole OS/packages and so forth. The ROMs need to have PIE disabled. This
> > is my initial attempt at trying to get somehting upstream so that QEMU
> > both builds out of the box and to resolve the build issue with the
> > buildbots that has been around for awhile. We have a patch in our ports
> > tree but it is just the flags hardcoded into the Makefile which obviously
> > is not appropriate for upstream.
> >
> > From the OpenBSD buildbots..
> >   Building optionrom/multiboot.img
> > ld: multiboot.o: relocation R_X86_64_16 can not be used when making a 
> > shared object; recompile with -fPIC
> >
> >
> > Signed-off by: Brad Smith 
> >
> > diff --git a/configure b/configure
> > index 508f6a5..7f060a4 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1342,6 +1342,10 @@ EOF
> >if compile_prog "-fPIE -DPIE" "-pie"; then
> >  QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
> >  LDFLAGS="-pie $LDFLAGS"
> > +if test "$targetos" == OpenBSD; then
> 
> '==' is not portable syntax.  You need to use '='.
 
Doh. My bad.

> Regards,
> 
> Anthony Liguori
> 
> > +  CC_NOPIE="-fno-pie"
> > +  LD_NOPIE="-nopie"
> > +fi
> >  pie="yes"
> >  if compile_prog "" "-Wl,-z,relro -Wl,-z,now" ; then
> >LDFLAGS="-Wl,-z,relro -Wl,-z,now $LDFLAGS"
> > @@ -4307,6 +4311,8 @@ if test "$gcov" = "yes" ; then
> >echo "CONFIG_GCOV=y" >> $config_host_mak
> >echo "GCOV=$gcov_tool" >> $config_host_mak
> >  fi
> > +echo "CC_NOPIE=$CC_NOPIE" >> $config_host_mak
> > +echo "LD_NOPIE=$LD_NOPIE" >> $config_host_mak
> >  
> >  # use included Linux headers
> >  if test "$linux" = "yes" ; then
> > diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
> > index 57d8bd0..0b35000 100644
> > --- a/pc-bios/optionrom/Makefile
> > +++ b/pc-bios/optionrom/Makefile
> > @@ -12,6 +12,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/optionrom)
> >  CFLAGS := -Wall -Wstrict-prototypes -Werror -fomit-frame-pointer 
> > -fno-builtin
> >  CFLAGS += -I$(SRC_PATH)
> >  CFLAGS += $(call cc-option, $(CFLAGS), -fno-stack-protector)
> > +CFLAGS += $(CC_NOPIE)
> >  QEMU_CFLAGS = $(CFLAGS)
> >  
> >  build-all: multiboot.bin linuxboot.bin kvmvapic.bin
> > @@ -20,7 +21,7 @@ build-all: multiboot.bin linuxboot.bin kvmvapic.bin
> >  .SECONDARY:
> >  
> >  %.img: %.o
> > -   $(call quiet-command,$(LD) -Ttext 0 -e _start -s -o $@ $<,"  Building 
> > $(TARGET_DIR)$@")
> > +   $(call quiet-command,$(LD) $(LD_NOPIE) -Ttext 0 -e _start -s -o $@ $<," 
> >  Building $(TARGET_DIR)$@")
> >  
> >  %.raw: %.img
> > $(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"  Building 
> > $(TARGET_DIR)$@")
> >
> > -- 
> > This message has been scanned for viruses and
> > dangerous content by MailScanner, and is
> > believed to be clean.
> 
> -- 
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
> 

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

Re: [Qemu-devel] [Bug 1253777] Re: OpenBSD VM running on OpenBSD host has sleep calls taking twice as long as they should

[Martin van den Nieuwelaar]

I downloaded 1.7.0-rc2 and compiled it.  Running it, I see the version 
number reported as 1.6.92!?

In any case, I don't see any improvement, ie. the bug is still there.

Regards,

-Martin


On 28/11/13 01:58, Paolo Bonzini wrote:
> Hi, please test qemu 1.7.0-rc.  There were several changes to the timer
> machinery that can help this bug.
>

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

Title:
  OpenBSD VM running on OpenBSD host has sleep calls taking twice as
  long as they should

Status in QEMU:
  New

Bug description:
  Running a script like

  while [ 1 ]
  do
date
sleep 1
  done

  on the VM will result in the (correct) date being displayed, but it is
  displayed only every two (!) seconds.  We have also noticed that if we
  connect to the VM's console using VNC, and move the mouse pointer
  constantly in the VNC window, the script runs normally with updates
  every second!  Note that the script doesn't have to be running on the
  VM's console - it's also possible to (say) ssh to the VM from a
  separate machine and run the script and it will display the '2 second'
  issue, but as soon as you move the mouse pointer constantly in the VNC
  console window the script starts behaving normally with updates every
  second.

  I have only seen this bug when running an OpenBSD VM on an OpenBSD
  host.  Running an OpenBSD VM on a Linux host does not exhibit the
  problem for me.  I also belive (am told) that a Linux VM running on an
  OpenBSD host does not exhibit the problem.

  I have been using the OpenBSD 5.4 64 bit distro which comes with qemu
  1.5.1 in a package, however I tried compiling qemu 1.6.1 and that has
  the same bug.  In fact older OpenBSD distros have the same issue -
  going back to OpenBSD distros from two years ago still have the
  problem.  This is not a 'new' bug recently introduced.

  Initially I wondered if it could be traced to an incorrectly set
  command line option, but I've since gone through many of the options
  in the man page simply trying different values (eg. different CPU
  types ( -cpu) , different emulated PC (-M)) but so far the problem
  remains.

  I'm quite happy to run tests in order to track this bug down better.
  We use qemu running on OpenBSD extensively and find it very useful!

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

Re: [Qemu-devel] [PATCH 29/60] AArch64: Add orri instruction emulation

[Richard Henderson]

On 11/27/2013 12:56 AM, Claudio Fontana wrote:
> On 09/27/2013 09:42 PM, Richard Henderson wrote:
>> On 09/26/2013 05:48 PM, Alexander Graf wrote:
>>> +if (setflags) {
>>> +tcg_dst = cpu_reg(dest);
>>> +} else {
>>> +tcg_dst = cpu_reg_sp(dest);
>>> +}
>>
>> Never sp for logicals.
> 
> This should be ok in my view, the manual explicitly shows in the pseudocode:
> 
> if d == 31 && !setflags then
>  SP[] = result;
> else
>  X[d] = result;

Sure enough.  I mis-read that Logical (register) and Logical (immediate) are
different in their ability to use XSP as an output.


r~

[Qemu-devel] [Bug 1254672] Re: ps segfaults with qemu-{arm, armel, mips, powerpc}-static

[Ken Sharp]

** Summary changed:

- ps segfaults with qemu-arm-static
+ ps segfaults with qemu-{arm,armel,mips,powerpc}-static

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

Title:
  ps segfaults with qemu-{arm,armel,mips,powerpc}-static

Status in QEMU:
  New
Status in Linaro QEMU:
  New
Status in “qemu-linaro” package in Ubuntu:
  New

Bug description:
  Host: Ubuntu Precise AMD64
  Guest: Debian Testing armhf (or armel)

  After running a debootstrap for Debian testing/armhf and entering the
  chroot, simply running "ps" causes a segmentation fault.

  $ sudo qemu-debootstrap --arch=armhf testing armhf 
http://ftp.uk.debian.org/debian/
  [...]
  $ sudo chroot armhf
  # ps
  Signal 11 (SEGV) caught by ps (procps-ng version 3.3.4).
  /bin/ps:display.c:59: please report this bug

  I couldn't find a bug report for procps, which would be unusual if
  such a bug existed, so I'm assuming the bug is in qemu.

  Unfortunately trying to run debootstrap for an Ubuntu variant fails to
  find the release file.

  ps is used a lot, as you can imagine, but specifically it fails when
  trying to install some packages via "apt-get install" and no attempt
  is made to recover.

  ProblemType: Bug
  DistroRelease: Ubuntu 12.04
  Package: qemu-user-static 1.0.50-2012.03-0ubuntu2.1
  ProcVersionSignature: Ubuntu 3.8.0-33.48~precise1-generic 3.8.13.11
  Uname: Linux 3.8.0-33-generic x86_64
  NonfreeKernelModules: wl
  ApportVersion: 2.0.1-0ubuntu17.6
  Architecture: amd64
  Date: Mon Nov 25 10:43:12 2013
  Dependencies:

  InstallationMedia: Ubuntu 12.04.3 LTS "Precise Pangolin" - Release amd64 
(20130820.1)
  MarkForUpload: True
  ProcEnviron:
   LANGUAGE=en_GB:en
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_GB.UTF-8
   SHELL=/bin/bash
  SourcePackage: qemu-linaro
  UpgradeStatus: No upgrade log present (probably fresh install)

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

Re: [Qemu-devel] [PATCH 02/20] target-i386: convert 'hv_spinlocks' to static property

[Igor Mammedov]

On Wed, 27 Nov 2013 18:55:57 +0100
Andreas Färber  wrote:

> Am 16.07.2013 00:25, schrieb Igor Mammedov:
> > Signed-off-by: Igor Mammedov 
> > ---
> > v2:
> >  - rebase on top of hyperv_spinlock_attempts in X86CPU
> > ---
> >  target-i386/cpu.c | 48 +++-
> >  1 file changed, 47 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 14e9c7e..00c2882 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1473,6 +1473,49 @@ static void x86_cpu_get_feature_words(Object *obj, 
> > Visitor *v, void *opaque,
> >  error_propagate(errp, err);
> >  }
> >  
> > +static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
> > + const char *name, Error **errp)
> > +{
> > +X86CPU *cpu = X86_CPU(obj);
> > +int64_t value = cpu->hyperv_spinlock_attempts;
> > +
> > +visit_type_int(v, &value, name, errp);
> > +}
> > +
> > +static void x86_set_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
> > + const char *name, Error **errp)
> > +{
> > +const int64_t min = 0xFFF;
> > +const int64_t max = UINT_MAX;
> > +X86CPU *cpu = X86_CPU(obj);
> > +int64_t value;
> > +
> > +visit_type_int(v, &value, name, errp);
> > +if (error_is_set(errp)) {
> > +return;
> > +}
> 
> errp may be NULL. And if an Error gets raised here but not set to *errp
> for lack of pointer, value might be uninitialized:
> object_property_parse(obj, "not-a-number", "hv-spinlocks", NULL);
> So we cannot rely on error_is_set(errp) but must use a local variable to
> enforce any return. Fixed on qom-cpu-next as follows:
in this particular above couldn't happen since all callers provide
not NULL errp. But it'd be cleaner to be prepared for generic case.

But anyway any caller that supplies invalid value && NULL errp
are going to get object in incorrect state and suffer later from it.

> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 435b3b9..0a5a4f0 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1611,10 +1611,12 @@ static void x86_set_hv_spinlocks(Object *obj,
> Visitor *v, void *opaque,
>  const int64_t min = 0xFFF;
>  const int64_t max = UINT_MAX;
>  X86CPU *cpu = X86_CPU(obj);
> +Error *err = NULL;
>  int64_t value;
> 
> -visit_type_int(v, &value, name, errp);
> -if (error_is_set(errp)) {
> +visit_type_int(v, &value, name, &err);
> +if (err) {
> +error_propagate(errp, err);
>  return;
>  }
> 
> 
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next
> 
> Regards,
> Andreas
> 
> 
> > +
> > +if (value < min || value > max) {
> > +error_setg(errp, "Property %s.%s doesn't take value %" PRId64
> > +  " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
> > +  object_get_typename(obj), name ? name : "null",
> > +  value, min, max);
> > +return;
> > +}
> > +cpu->hyperv_spinlock_attempts = value;
> > +}
> > +
> > +static PropertyInfo qdev_prop_spinlocks = {
> > +.name  = "int",
> > +.get   = x86_get_hv_spinlocks,
> > +.set   = x86_set_hv_spinlocks,
> > +};
> > +
> > +static Property cpu_x86_properties[] = {
> > +{ .name  = "hv-spinlocks", .info  = &qdev_prop_spinlocks },
> > +DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> >  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
> >  {
> >  x86_def_t *def;
> > @@ -1586,6 +1629,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, 
> > char *features, Error **errp)
> >  } else if (!strcmp(featurestr, "hv-spinlocks")) {
> >  char *err;
> >  const int min = 0xFFF;
> > +char num[32];
> >  numvalue = strtoul(val, &err, 0);
> >  if (!*val || *err) {
> >  error_setg(errp, "bad numerical value %s", val);
> > @@ -1597,7 +1641,8 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, 
> > char *features, Error **errp)
> >  min);
> >  numvalue = min;
> >  }
> > -cpu->hyperv_spinlock_attempts = numvalue;
> > +snprintf(num, sizeof(num), "%" PRId32, numvalue);
> > +object_property_parse(OBJECT(cpu), num, featurestr, errp);
> >  } else {
> >  error_setg(errp, "unrecognized feature %s", featurestr);
> >  goto out;
> > @@ -2521,6 +2566,7 @@ static void x86_cpu_common_class_init(ObjectClass 
> > *oc, void *data)
> >  xcc->parent_realize = dc->realize;
> >  dc->realize = x86_cpu_realizefn;
> >  dc->bus_type = TYPE_ICC_BUS;
> > +dc->props = cpu_x86_properties;
> >  
> >  xcc->parent_reset = cc->reset;
> >  cc->reset = x86_cpu_reset;
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennif

Re: [Qemu-devel] [Bug 1253777] Re: OpenBSD VM running on OpenBSD host has sleep calls taking twice as long as they should

[Martin van den Nieuwelaar]

I'll have a look at it now.

Regards,

-Martin


On 28/11/13 01:58, Paolo Bonzini wrote:
> Hi, please test qemu 1.7.0-rc.  There were several changes to the timer
> machinery that can help this bug.
>


-- 
R A Ward Ltd. | We take the privacy of our customers seriously.
Christchurch  | All sensitive E-Mail attachments MUST be encrypted.
New Zealand

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

Title:
  OpenBSD VM running on OpenBSD host has sleep calls taking twice as
  long as they should

Status in QEMU:
  New

Bug description:
  Running a script like

  while [ 1 ]
  do
date
sleep 1
  done

  on the VM will result in the (correct) date being displayed, but it is
  displayed only every two (!) seconds.  We have also noticed that if we
  connect to the VM's console using VNC, and move the mouse pointer
  constantly in the VNC window, the script runs normally with updates
  every second!  Note that the script doesn't have to be running on the
  VM's console - it's also possible to (say) ssh to the VM from a
  separate machine and run the script and it will display the '2 second'
  issue, but as soon as you move the mouse pointer constantly in the VNC
  console window the script starts behaving normally with updates every
  second.

  I have only seen this bug when running an OpenBSD VM on an OpenBSD
  host.  Running an OpenBSD VM on a Linux host does not exhibit the
  problem for me.  I also belive (am told) that a Linux VM running on an
  OpenBSD host does not exhibit the problem.

  I have been using the OpenBSD 5.4 64 bit distro which comes with qemu
  1.5.1 in a package, however I tried compiling qemu 1.6.1 and that has
  the same bug.  In fact older OpenBSD distros have the same issue -
  going back to OpenBSD distros from two years ago still have the
  problem.  This is not a 'new' bug recently introduced.

  Initially I wondered if it could be traced to an incorrectly set
  command line option, but I've since gone through many of the options
  in the man page simply trying different values (eg. different CPU
  types ( -cpu) , different emulated PC (-M)) but so far the problem
  remains.

  I'm quite happy to run tests in order to track this bug down better.
  We use qemu running on OpenBSD extensively and find it very useful!

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

Re: [Qemu-devel] [qemu PATCH] hw/i386/pc_sysfw: support more than one flash drive

[Markus Armbruster]

Laszlo Ersek  writes:

> On 11/27/13 18:22, Markus Armbruster wrote:
>
>> Perhaps the proper way to back partially writable flash contents isn't
>> splitting it into two devices, but backing a single device with a COW.
>> The backing file has initial contents (say BIOS image), the delta may
>> have additional contents (say non-volatile configuration), and picks up
>> whatever the device model permits to be written to the flash.
>
> Kevin brought this up before. The problem is that this prevents you from
> upgrading the read-only part in O(1).

Err, what's wrong with rebasing the COW to a new backing file?

Oh, I see you considered it:

> (Unless of course you're willing to make the backing file raw *and* to
> poke directly in it when you upgrade the bios binary part. Theoretically

No, don't update the backing file, *replace* it.

If the device model permitted a write to a cluster within the BIOS part,
the replacement won't be visible, so don't do that :)

> you could do this, because the divide between the varstore and the
> binary part at 128KB falls at a qcow2 block boundary (supposing a 64K
> qcow2 block size), but in practice this is a horrible idea :))

Yes, with COW the COW clustering apply to the division between BIOS part
and non-volatile configuration part.

> Let's not veer off into architecting a new star destroyer. I think I
> have plenty ammo from your great notes thus far that I can hack up a v2
> with :)

H, pretty star destroyers...

[Qemu-devel] [PATCH] block: Close backing file early in bdrv_img_create

[Max Reitz]

Leaving the backing file open although it is not needed anymore can
cause problems if it is opened through a block driver which allows
exclusive access only and if the create function of the block driver
used for the top image (the one being created) tries to close and reopen
the image file (which will include opening the backing file a second
time).

In particular, this will happen with a backing file opened through
qemu-nbd and using qcow2 as the top image file format (which reopens the
image to flush it to disk).

In addition, the BlockDriverState in bdrv_img_create() is used for the
backing file only; it should therefore be made local to the respective
block.

Signed-off-by: Max Reitz 
---
 block.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 382ea71..e5a8a4c 100644
--- a/block.c
+++ b/block.c
@@ -4504,7 +4504,6 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
 {
 QEMUOptionParameter *param = NULL, *create_options = NULL;
 QEMUOptionParameter *backing_fmt, *backing_file, *size;
-BlockDriverState *bs = NULL;
 BlockDriver *drv, *proto_drv;
 BlockDriver *backing_drv = NULL;
 Error *local_err = NULL;
@@ -4583,6 +4582,7 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
 size = get_option_parameter(param, BLOCK_OPT_SIZE);
 if (size && size->value.n == -1) {
 if (backing_file && backing_file->value.s) {
+BlockDriverState *bs;
 uint64_t size;
 char buf[32];
 int back_flags;
@@ -4608,6 +4608,8 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
 
 snprintf(buf, sizeof(buf), "%" PRId64, size);
 set_option_parameter(param, BLOCK_OPT_SIZE, buf);
+
+bdrv_unref(bs);
 } else {
 error_setg(errp, "Image creation needs a size parameter");
 goto out;
@@ -4638,9 +4640,6 @@ out:
 free_option_parameters(create_options);
 free_option_parameters(param);
 
-if (bs) {
-bdrv_unref(bs);
-}
 if (error_is_set(&local_err)) {
 error_propagate(errp, local_err);
 }
-- 
1.8.4.2

[Qemu-devel] [PATCH 3/7] target-i386: Move KVM default-vendor hack to instance_init

[Eduardo Habkost]

As we will not have a cpu_x86_find_by_name() function anymore,
move the KVM default-vendor hack to instance_init.

Unfortunately we can't move that code to class_init because it depends
on KVM being initialized.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 32 +++-
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bfe2453..9bd6c20 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1560,18 +1560,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t 
*x86_cpu_def,
 def = &builtin_x86_defs[i];
 if (strcmp(name, def->name) == 0) {
 memcpy(x86_cpu_def, def, sizeof(*def));
-/* sysenter isn't supported in compatibility mode on AMD,
- * syscall isn't supported in compatibility mode on Intel.
- * Normally we advertise the actual CPU vendor, but you can
- * override this using the 'vendor' property if you want to use
- * KVM's sysenter/syscall emulation in compatibility mode and
- * when doing cross vendor migration
- */
-if (kvm_enabled()) {
-uint32_t  ebx = 0, ecx = 0, edx = 0;
-host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
-x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
-}
 return 0;
 }
 }
@@ -1822,7 +1810,6 @@ static void cpu_x86_register(X86CPU *cpu, const char 
*name, Error **errp)
 return;
 }
 
-object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
 object_property_set_int(OBJECT(cpu), def->level, "level", errp);
 object_property_set_int(OBJECT(cpu), def->family, "family", errp);
 object_property_set_int(OBJECT(cpu), def->model, "model", errp);
@@ -1846,6 +1833,25 @@ static void cpu_x86_register(X86CPU *cpu, const char 
*name, Error **errp)
 env->features[FEAT_KVM] |= kvm_default_features;
 }
 env->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
+
+/* sysenter isn't supported in compatibility mode on AMD,
+ * syscall isn't supported in compatibility mode on Intel.
+ * Normally we advertise the actual CPU vendor, but you can
+ * override this using the 'vendor' property if you want to use
+ * KVM's sysenter/syscall emulation in compatibility mode and
+ * when doing cross vendor migration
+ */
+const char *vendor = def->vendor;
+char host_vendor[CPUID_VENDOR_SZ + 1];
+if (kvm_enabled()) {
+uint32_t  ebx = 0, ecx = 0, edx = 0;
+host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
+x86_cpu_vendor_words2str(host_vendor, ebx, edx, ecx);
+vendor = host_vendor;
+}
+
+object_property_set_str(OBJECT(cpu), vendor, "vendor", errp);
+
 }
 
 X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
-- 
1.8.3.1

[Qemu-devel] [RFC 7/7] target-i386: CPU model subclasses

[Eduardo Habkost]

Register separate QOM classes for each x86 CPU model.

This will allow management code to more easily probe what each CPU model
provides, by simply creating objects using the appropriate class name,
without having to restart QEMU.

This also allows us to eliminate the qdev_prop_set_globals_for_type()
hack to set CPU-model-specific global properties.

Instead of creating separate class_init functions for each class, I just
used class_dat to store a pointer to the X86CPUDefinition struct for
each CPU model. This should make the patch shorter and easier to review.
Later we can gradually convert each X86CPUDefinition field to lists of
per-class property defaults.

Signed-off-by: Eduardo Habkost 
---
This version is closer to the version sent by Andrea and then later
resubmitted by Igor as "[RFC v5] target-i386: Slim conversion to X86CPU
subclasses + KVM subclasses", as it doesn't create one new class_init
function for each subclass. One main difference is that this version
does not use KVM-specific subclasses, to keep things simpler.
---
 target-i386/cpu-qom.h |  13 ++
 target-i386/cpu.c | 343 +++---
 target-i386/cpu.h |   2 -
 3 files changed, 228 insertions(+), 130 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index f4fab15..468fea7 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -37,6 +37,9 @@
 #define X86_CPU_GET_CLASS(obj) \
 OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
 
+
+typedef struct X86CPUDefinition X86CPUDefinition;
+
 /**
  * X86CPUClass:
  * @parent_realize: The parent class' realize handler.
@@ -49,6 +52,16 @@ typedef struct X86CPUClass {
 CPUClass parent_class;
 /*< public >*/
 
+/* CPU model definition
+ * Should be eventually replaced by subclass-specific property defaults
+ */
+X86CPUDefinition *cpu_def;
+/* CPU model requires KVM to be enabled */
+bool kvm_required;
+/* Optional description of CPU model.
+ * If unavailable, cpu_def->model_id is used */
+const char *model_description;
+
 DeviceRealize parent_realize;
 void (*parent_reset)(CPUState *cpu);
 } X86CPUClass;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ab80081..5898d52 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -481,7 +481,10 @@ static void add_flagname_to_bitmaps(const char *flagname,
 }
 }
 
-typedef struct X86CPUDefinition {
+/* CPU model definition data that was not converted to QOM per-subclass
+ * property defaults yet.
+ */
+struct X86CPUDefinition {
 const char *name;
 uint32_t level;
 uint32_t xlevel;
@@ -494,7 +497,7 @@ typedef struct X86CPUDefinition {
 FeatureWordArray features;
 char model_id[48];
 bool cache_info_passthrough;
-} X86CPUDefinition;
+};
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
@@ -544,8 +547,41 @@ typedef struct X86CPUDefinition {
   CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
   CPUID_7_0_EBX_RDSEED */
 
-/* built-in CPU model definitions
+/* CPU class name definitions: */
+
+#define X86_CPU_CLASS_SUFFIX "-" TYPE_X86_CPU
+#define CPU_CLASS_NAME(name) (name X86_CPU_CLASS_SUFFIX)
+
+
+static char *x86_cpu_class_get_model_name(X86CPUClass *cc)
+{
+const char *class_name = object_class_get_name(OBJECT_CLASS(cc));
+if (g_str_has_suffix(class_name, X86_CPU_CLASS_SUFFIX)) {
+return g_strndup(class_name,
+ strlen(class_name) - strlen(X86_CPU_CLASS_SUFFIX));
+} else {
+return g_strdup(class_name);
+}
+}
+
+/* Return class name for a given CPU model name
+ * Caller is responsible for freeing the returned string.
  */
+static char *x86_cpu_class_name(const char *model_name)
+{
+return g_strdup_printf(CPU_CLASS_NAME("%s"), model_name);
+}
+
+/* Return X86CPUClass for a CPU model name */
+static X86CPUClass *x86_cpu_class_by_name(const char *name)
+{
+X86CPUClass *cc;
+char *class_name = x86_cpu_class_name(name);
+cc =  X86_CPU_CLASS(object_class_by_name(class_name));
+g_free(class_name);
+return cc;
+}
+
 static X86CPUDefinition builtin_x86_defs[] = {
 {
 .name = "qemu64",
@@ -1090,6 +1126,33 @@ static X86CPUDefinition builtin_x86_defs[] = {
 },
 };
 
+static void x86_cpu_class_init_cpudef(ObjectClass *oc, void *data)
+{
+X86CPUDefinition *cpudef = data;
+X86CPUClass *xcc = X86_CPU_CLASS(oc);
+xcc->cpu_def = cpudef;
+}
+
+static void x86_register_cpudef_classes(void)
+{
+int i;
+for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
+X86CPUDefinition *def = &builtin_x86_defs[i];
+char *class_name = x86_cpu_class_name(def->name);
+TypeInfo ti = {
+.name = class_name,
+.parent = TYPE_X86_CPU,
+.instance_size = sizeof(X86CPU),
+.abstract = false,
+.class_size = sizeof(X86CPUClass),
+   

[Qemu-devel] [PATCH 5/7] target-i386: Call x86_cpu_load_def() earlier

[Eduardo Habkost]

As we will initialize the X86CPU fields on instance_init eventually,
move the code that initializes the X86CPU data based on the CPU model
name closer to the object_new() call.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f55caea..7f94a08 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1874,6 +1874,11 @@ X86CPU *cpu_x86_create(const char *cpu_model, 
DeviceState *icc_bridge,
 features = model_pieces[1];
 
 cpu = X86_CPU(object_new(TYPE_X86_CPU));
+x86_cpu_load_def(cpu, name, &error);
+if (error) {
+goto out;
+}
+
 #ifndef CONFIG_USER_ONLY
 if (icc_bridge == NULL) {
 error_setg(&error, "Invalid icc-bridge value");
@@ -1883,11 +1888,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, 
DeviceState *icc_bridge,
 object_unref(OBJECT(cpu));
 #endif
 
-x86_cpu_load_def(cpu, name, &error);
-if (error) {
-goto out;
-}
-
 /* Emulate per-model subclasses for global properties */
 typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
 qdev_prop_set_globals_for_type(DEVICE(cpu), typename, &error);
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/7] target-i386: Rename x86_def_t to X86CPUDefinition

[Eduardo Habkost]

As the new X86CPU subclass code is going to change lots of the code
invoving x86_def_t, let's rename the struct to match coding style first.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7f94a08..ab80081 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -481,7 +481,7 @@ static void add_flagname_to_bitmaps(const char *flagname,
 }
 }
 
-typedef struct x86_def_t {
+typedef struct X86CPUDefinition {
 const char *name;
 uint32_t level;
 uint32_t xlevel;
@@ -494,7 +494,7 @@ typedef struct x86_def_t {
 FeatureWordArray features;
 char model_id[48];
 bool cache_info_passthrough;
-} x86_def_t;
+} X86CPUDefinition;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
@@ -546,7 +546,7 @@ typedef struct x86_def_t {
 
 /* built-in CPU model definitions
  */
-static x86_def_t builtin_x86_defs[] = {
+static X86CPUDefinition builtin_x86_defs[] = {
 {
 .name = "qemu64",
 .level = 4,
@@ -1105,7 +1105,7 @@ static x86_def_t builtin_x86_defs[] = {
 void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
  uint32_t feat_add, uint32_t feat_remove)
 {
-x86_def_t *def;
+X86CPUDefinition *def;
 int i;
 for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
 def = &builtin_x86_defs[i];
@@ -1131,12 +1131,12 @@ static int cpu_x86_fill_model_id(char *str)
 return 0;
 }
 
-/* Fill a x86_def_t struct with information about the host CPU, and
+/* Fill a X86CPUDefinition struct with information about the host CPU, and
  * the CPU features supported by the host hardware + host kernel
  *
  * This function may be called only if KVM is enabled.
  */
-static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
+static void kvm_cpu_fill_host(X86CPUDefinition *x86_cpu_def)
 {
 KVMState *s = kvm_state;
 uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
@@ -1539,10 +1539,10 @@ static void x86_cpu_get_feature_words(Object *obj, 
Visitor *v, void *opaque,
 error_propagate(errp, err);
 }
 
-static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
+static int cpu_x86_find_by_name(X86CPU *cpu, X86CPUDefinition *x86_cpu_def,
 const char *name)
 {
-x86_def_t *def;
+X86CPUDefinition *def;
 Error *err = NULL;
 int i;
 
@@ -1732,7 +1732,7 @@ static void listflags(char *buf, int bufsize, uint32_t 
fbits,
 /* generate CPU information. */
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
-x86_def_t *def;
+X86CPUDefinition *def;
 char buf[256];
 int i;
 
@@ -1759,7 +1759,7 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 {
 CpuDefinitionInfoList *cpu_list = NULL;
-x86_def_t *def;
+X86CPUDefinition *def;
 int i;
 
 for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
@@ -1803,7 +1803,7 @@ static void filter_features_for_kvm(X86CPU *cpu)
 static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
 {
 CPUX86State *env = &cpu->env;
-x86_def_t def1, *def = &def1;
+X86CPUDefinition def1, *def = &def1;
 
 memset(def, 0, sizeof(*def));
 
@@ -1830,7 +1830,7 @@ static void x86_cpu_load_def(X86CPU *cpu, const char 
*name, Error **errp)
 
 object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
 
-/* Special cases not set in the x86_def_t structs: */
+/* Special cases not set in the X86CPUDefinition structs: */
 if (kvm_enabled()) {
 env->features[FEAT_KVM] |= kvm_default_features;
 }
@@ -1952,7 +1952,7 @@ void x86_cpudef_setup(void)
 static const char *model_with_versions[] = { "qemu32", "qemu64", "athlon" 
};
 
 for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
-x86_def_t *def = &builtin_x86_defs[i];
+X86CPUDefinition *def = &builtin_x86_defs[i];
 
 /* Look for specific "cpudef" models that */
 /* have the QEMU version in .model_id */
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/7] target-i386: Rename cpu_x86_register() to x86_cpu_load_def()

[Eduardo Habkost]

There isn't any kind of "registration" involved in cpu_x86_register()
anymore: it is simply looking up a CPU model name and loading the model
definition data into the X86CPU object. Rename it to x86_cpu_load_def()
to reflect what it does.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9bd6c20..f55caea 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1798,7 +1798,9 @@ static void filter_features_for_kvm(X86CPU *cpu)
 }
 }
 
-static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
+/* Load CPU definition for a given CPU model name
+ */
+static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
 {
 CPUX86State *env = &cpu->env;
 x86_def_t def1, *def = &def1;
@@ -1881,7 +1883,7 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState 
*icc_bridge,
 object_unref(OBJECT(cpu));
 #endif
 
-cpu_x86_register(cpu, name, &error);
+x86_cpu_load_def(cpu, name, &error);
 if (error) {
 goto out;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/7] target-i386: Don't change x86_def_t struct on cpu_x86_register()

[Eduardo Habkost]

As eventually the x86_def_t data is going to be provided by the CPU
class, it's better to not touch it, and handle the special cases on the
X86CPU object itself.

Current behavior of the code should stay exactly the same.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b525592..bfe2453 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1822,11 +1822,6 @@ static void cpu_x86_register(X86CPU *cpu, const char 
*name, Error **errp)
 return;
 }
 
-if (kvm_enabled()) {
-def->features[FEAT_KVM] |= kvm_default_features;
-}
-def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
-
 object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
 object_property_set_int(OBJECT(cpu), def->level, "level", errp);
 object_property_set_int(OBJECT(cpu), def->family, "family", errp);
@@ -1845,6 +1840,12 @@ static void cpu_x86_register(X86CPU *cpu, const char 
*name, Error **errp)
 cpu->cache_info_passthrough = def->cache_info_passthrough;
 
 object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
+
+/* Special cases not set in the x86_def_t structs: */
+if (kvm_enabled()) {
+env->features[FEAT_KVM] |= kvm_default_features;
+}
+env->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
 }
 
 X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
-- 
1.8.3.1

[Qemu-devel] [PATCH 0/7] x86 CPU subclasses, take 6

[Eduardo Habkost]

I want to try to get this in 1.8, because I have found one additional use-case
for the new subclasses:

libvirt needs to be able to query details about the existing CPU models, and it
can't do that today without restarting QEMU every time. Having separate classes
for each CPU model allows libvirt to create/destroy CPU objects in a loop just
to query the resulting properties (especially "feature-words") for each CPU
model.

This version is closer to the version sent by Andrea and then later resubmitted
by Igor as "[RFC v5] target-i386: Slim conversion to X86CPU subclasses + KVM
subclasses", than the previous version I have sent, as it doesn't create one new
class_init function for each subclass. One main difference is that this version
does not use KVM-specific subclasses, to keep things simpler.

Another difference is that instead of late registration of the "host" class, I
simply changed the "host" subclass to do KVM-dependent initialization steps on
instance_init instead of class_init. This way we won't require any
initialization-ordering tricks. In the past this was a problem, but today the
global properties are being set by qdev code on post_init, so we can safely
initialize property defaults on instance_init without having to worry that it
would break the setting of global properties.

This series needs to be applied on top of:
   Subject: [PATCH 0/8] target-i386: Simplify kvm_cpu_fill_host() and 
kvm_check_features_against_host()
   Message-Id: <1385322940-27325-1-git-send-email-ehabk...@redhat.com>
   http://article.gmane.org/gmane.comp.emulators.qemu/243013


Eduardo Habkost (7):
  target-i386: Eliminate CONFIG_KVM #ifdefs
  target-i386: Don't change x86_def_t struct on cpu_x86_register()
  target-i386: Move KVM default-vendor hack to instance_init
  target-i386: Rename cpu_x86_register() to x86_cpu_load_def()
  target-i386: Call x86_cpu_load_def() earlier
  target-i386: Rename x86_def_t to X86CPUDefinition
  target-i386: CPU model subclasses

 target-i386/cpu-qom.h |  13 ++
 target-i386/cpu.c | 407 ++
 target-i386/cpu.h |   2 -
 3 files changed, 260 insertions(+), 162 deletions(-)

-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-1.7 0/2] block/drive-mirror: Reuse backing HD for sync=none

[Anthony Liguori]

Kevin Wolf  writes:

> Am 26.11.2013 um 19:02 hat Anthony Liguori geschrieben:
>> Max Reitz  writes:
>> 
>> > This series fixes the drive-mirror blockjob in case of "none" sync mode
>> > to always use the old (current) image file as the backing file of the
>> > newly created mirrored file (in case of "absolute-paths" mode).
>> >
>> > It is rather important to get this into 1.7, as we will introduce an at
>> > least pretty strange API in case the original file is unbacked
>> > otherwise.
>> 
>> Kevin/Stefan?  Do we need this for 1.7?
>
> Yes, it would be good to pick it up in order to avoid changing the API
> in 1.8 (I guess we would do it anyway because the current behaviour
> doesn't make any sense and we'd call it bug fix, but libvirt would have
> to deal with it and better to do it right in the first release.)
>
> Do you want to pick it up yourself or should I send a pull request?

I'll pick it up.

Regards,

Anthony Liguori

>
> Reviewed-by: Kevin Wolf 
>
> Kevin

[Qemu-devel] [PATCH 1/7] target-i386: Eliminate CONFIG_KVM #ifdefs

[Eduardo Habkost]

The compiler is capable of eliminating the KVM-specific function calls
as long as the calling function has an assert(kvm_enabled()) line, so we
don't need to wrap all KVM-specific inside #ifdefs.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d747e04..b525592 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -373,7 +373,6 @@ void disable_kvm_pv_eoi(void)
 void host_cpuid(uint32_t function, uint32_t count,
 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
 {
-#if defined(CONFIG_KVM)
 uint32_t vec[4];
 
 #ifdef __x86_64__
@@ -401,7 +400,6 @@ void host_cpuid(uint32_t function, uint32_t count,
 *ecx = vec[2];
 if (edx)
 *edx = vec[3];
-#endif
 }
 
 #define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c)))
@@ -1118,7 +1116,6 @@ void x86_cpu_compat_set_features(const char *cpu_model, 
FeatureWord w,
 }
 }
 
-#ifdef CONFIG_KVM
 static int cpu_x86_fill_model_id(char *str)
 {
 uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
@@ -1133,7 +1130,6 @@ static int cpu_x86_fill_model_id(char *str)
 }
 return 0;
 }
-#endif
 
 /* Fill a x86_def_t struct with information about the host CPU, and
  * the CPU features supported by the host hardware + host kernel
@@ -1142,7 +1138,6 @@ static int cpu_x86_fill_model_id(char *str)
  */
 static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
 {
-#ifdef CONFIG_KVM
 KVMState *s = kvm_state;
 uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
 
@@ -1172,8 +1167,6 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
 kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
  wi->cpuid_reg);
 }
-
-#endif /* CONFIG_KVM */
 }
 
 static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
@@ -1798,13 +1791,14 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error 
**errp)
 return cpu_list;
 }
 
-#ifdef CONFIG_KVM
 static void filter_features_for_kvm(X86CPU *cpu)
 {
 CPUX86State *env = &cpu->env;
 KVMState *s = kvm_state;
 FeatureWord w;
 
+assert(kvm_enabled());
+
 for (w = 0; w < FEATURE_WORDS; w++) {
 FeatureWordInfo *wi = &feature_word_info[w];
 uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
@@ -1815,7 +1809,6 @@ static void filter_features_for_kvm(X86CPU *cpu)
 cpu->filtered_features[w] = requested_features & ~env->features[w];
 }
 }
-#endif
 
 static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
 {
@@ -2525,9 +2518,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
"Host's CPU doesn't support requested features");
 goto out;
 }
-#ifdef CONFIG_KVM
 filter_features_for_kvm(cpu);
-#endif
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-1.7] Fix QEMU build on OpenBSD on x86 archs

[Anthony Liguori]

Brad Smith  writes:

> This resolves the build issue with building the ROMs on OpenBSD on x86 archs.
> As of OpenBSD 5.3 the compiler builds PIE binaries by default and thus the
> whole OS/packages and so forth. The ROMs need to have PIE disabled. This
> is my initial attempt at trying to get somehting upstream so that QEMU
> both builds out of the box and to resolve the build issue with the
> buildbots that has been around for awhile. We have a patch in our ports
> tree but it is just the flags hardcoded into the Makefile which obviously
> is not appropriate for upstream.
>
> From the OpenBSD buildbots..
>   Building optionrom/multiboot.img
> ld: multiboot.o: relocation R_X86_64_16 can not be used when making a shared 
> object; recompile with -fPIC
>
>
> Signed-off by: Brad Smith 
>
> diff --git a/configure b/configure
> index 508f6a5..7f060a4 100755
> --- a/configure
> +++ b/configure
> @@ -1342,6 +1342,10 @@ EOF
>if compile_prog "-fPIE -DPIE" "-pie"; then
>  QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
>  LDFLAGS="-pie $LDFLAGS"
> +if test "$targetos" == OpenBSD; then

'==' is not portable syntax.  You need to use '='.

Regards,

Anthony Liguori

> +  CC_NOPIE="-fno-pie"
> +  LD_NOPIE="-nopie"
> +fi
>  pie="yes"
>  if compile_prog "" "-Wl,-z,relro -Wl,-z,now" ; then
>LDFLAGS="-Wl,-z,relro -Wl,-z,now $LDFLAGS"
> @@ -4307,6 +4311,8 @@ if test "$gcov" = "yes" ; then
>echo "CONFIG_GCOV=y" >> $config_host_mak
>echo "GCOV=$gcov_tool" >> $config_host_mak
>  fi
> +echo "CC_NOPIE=$CC_NOPIE" >> $config_host_mak
> +echo "LD_NOPIE=$LD_NOPIE" >> $config_host_mak
>  
>  # use included Linux headers
>  if test "$linux" = "yes" ; then
> diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
> index 57d8bd0..0b35000 100644
> --- a/pc-bios/optionrom/Makefile
> +++ b/pc-bios/optionrom/Makefile
> @@ -12,6 +12,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/optionrom)
>  CFLAGS := -Wall -Wstrict-prototypes -Werror -fomit-frame-pointer -fno-builtin
>  CFLAGS += -I$(SRC_PATH)
>  CFLAGS += $(call cc-option, $(CFLAGS), -fno-stack-protector)
> +CFLAGS += $(CC_NOPIE)
>  QEMU_CFLAGS = $(CFLAGS)
>  
>  build-all: multiboot.bin linuxboot.bin kvmvapic.bin
> @@ -20,7 +21,7 @@ build-all: multiboot.bin linuxboot.bin kvmvapic.bin
>  .SECONDARY:
>  
>  %.img: %.o
> - $(call quiet-command,$(LD) -Ttext 0 -e _start -s -o $@ $<,"  Building 
> $(TARGET_DIR)$@")
> + $(call quiet-command,$(LD) $(LD_NOPIE) -Ttext 0 -e _start -s -o $@ $<," 
>  Building $(TARGET_DIR)$@")
>  
>  %.raw: %.img
>   $(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"  Building 
> $(TARGET_DIR)$@")
>
> -- 
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.

[Qemu-devel] [PATCH] coroutine: remove qemu_co_queue_wait_insert_head

[Marc-André Lureau]

qemu_co_queue_wait_insert_head() is unused in qemu code base now.

Signed-off-by: Marc-André Lureau 
---
 include/block/coroutine.h | 6 --
 qemu-coroutine-lock.c | 8 
 2 files changed, 14 deletions(-)

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 4d5c0cf..b122c0c 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -121,12 +121,6 @@ void qemu_co_queue_init(CoQueue *queue);
 void coroutine_fn qemu_co_queue_wait(CoQueue *queue);
 
 /**
- * Adds the current coroutine to the head of the CoQueue and transfers control 
to the
- * caller of the coroutine.
- */
-void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue);
-
-/**
  * Restarts the next coroutine in the CoQueue and removes it from the queue.
  *
  * Returns true if a coroutine was restarted, false if the queue is empty.
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index aeb33b9..e4860ae 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -41,14 +41,6 @@ void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
 assert(qemu_in_coroutine());
 }
 
-void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue)
-{
-Coroutine *self = qemu_coroutine_self();
-QTAILQ_INSERT_HEAD(&queue->entries, self, co_queue_next);
-qemu_coroutine_yield();
-assert(qemu_in_coroutine());
-}
-
 /**
  * qemu_co_queue_run_restart:
  *
-- 
1.8.4.2

[Qemu-devel] [PATCH v3] i386: Add _PXM ACPI method to CPU objects

[Vasilis Liaskovitis]

This patch adds a _PXM method to ACPI CPU objects for the pc machine. The _PXM
value is derived from the passed in guest info, same way as CPU SRAT entries.

Currently, CPU SRAT entries are only enabled for cpus that are already present
in the system. The SRAT entries for hotpluggable processors are disabled (flags
bit 0 set to 0 in hw/i385/acpi-build.c:build_srat). Section 5.2.16.1 of ACPI
spec mentions "If the Local APIC ID of a dynamically added processor is not
present in the SRAT, a _PXM object must exist for the processor’s device or one
of its ancestors in the ACPI Namespace." Since SRAT entries are not available
for the hot-pluggable processors, a _PXM method must exist for them. Otherwise,
the CPU is hot-added in the wrong NUMA node (default node 0).

Even if CPU SRAT entries are enabled, _PXM method is what the linux kernel
consults on hot-add time. Section 17.2.1 of ACPI spec mentions " OSPM will
consume the SRAT only at boot time. OSPM should use _PXM for any devices that
are hot-added into the system after boot up." To be more precise if SRAT
information is available to the guest kernel, it is used.  However, parsed SRAT
info is reset and lost after hot-remove operations, see kernel commit c4c60524.
This means that on a hot-unplug / hot-replug scenario, and without a _PXM
method, the kernel may put a CPU on different nodes because SRAT info has been
reset by a previous hot-remove operation.

The above hot-remove/hot-add scenario has been tested on master, plus cpu-del
patches from:
https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg01085.html
With the curret _PXM patch, hot-added CPUs are always placed into the correct
NUMA node, regardless of kernel behaviour.

v1->v2:
   Make method return a DWORD integer
   Tested on qemu master + cpu-del patches
v2->v3:
   Add changed hw/i386/sdt-proc.hex.generated file
   Change PXM constant name to CPXM

Signed-off-by: Vasilis Liaskovitis 
Reviewed-by: Thilo Fromm 

---
 hw/i386/acpi-build.c|5 
 hw/i386/ssdt-proc.dsl   |5 
 hw/i386/ssdt-proc.hex.generated |   57 ---
 3 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b48c930..387a869 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -605,6 +605,7 @@ static inline char acpi_get_hex(uint32_t val)
 #define ACPI_PROC_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2)
 #define ACPI_PROC_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4)
 #define ACPI_PROC_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start)
+#define ACPI_PROC_OFFSET_CPUPXM (*ssdt_proc_pxm - *ssdt_proc_start)
 #define ACPI_PROC_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
 #define ACPI_PROC_AML (ssdp_proc_aml + *ssdt_proc_start)
 
@@ -726,6 +727,10 @@ build_ssdt(GArray *table_data, GArray *linker,
 proc[ACPI_PROC_OFFSET_CPUHEX+1] = acpi_get_hex(i);
 proc[ACPI_PROC_OFFSET_CPUID1] = i;
 proc[ACPI_PROC_OFFSET_CPUID2] = i;
+proc[ACPI_PROC_OFFSET_CPUPXM] = guest_info->node_cpu[i];
+proc[ACPI_PROC_OFFSET_CPUPXM + 1] = 0;
+proc[ACPI_PROC_OFFSET_CPUPXM + 2] = 0;
+proc[ACPI_PROC_OFFSET_CPUPXM + 3] = 0;
 }
 
 /* build this code:
diff --git a/hw/i386/ssdt-proc.dsl b/hw/i386/ssdt-proc.dsl
index 8229bfd..52b44e3 100644
--- a/hw/i386/ssdt-proc.dsl
+++ b/hw/i386/ssdt-proc.dsl
@@ -47,6 +47,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", 
"BXSSDT", 0x1)
  * also updating the C code.
  */
 Name(_HID, "ACPI0007")
+ACPI_EXTRACT_NAME_DWORD_CONST ssdt_proc_pxm
+Name(CPXM, 0x)
 External(CPMA, MethodObj)
 External(CPST, MethodObj)
 External(CPEJ, MethodObj)
@@ -59,5 +61,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", 
"BXSSDT", 0x1)
 Method(_EJ0, 1, NotSerialized) {
 CPEJ(ID, Arg0)
 }
+Method(_PXM, 0) {
+Return (CPXM)
+}
 }
 }
diff --git a/hw/i386/ssdt-proc.hex.generated b/hw/i386/ssdt-proc.hex.generated
index bb9920d..8497866 100644
--- a/hw/i386/ssdt-proc.hex.generated
+++ b/hw/i386/ssdt-proc.hex.generated
@@ -1,17 +1,26 @@
+static unsigned char ssdt_proc_end[] = {
+0x8e
+};
 static unsigned char ssdt_proc_name[] = {
 0x28
 };
+static unsigned char ssdt_proc_pxm[] = {
+0x4e
+};
+static unsigned char ssdt_proc_id[] = {
+0x38
+};
 static unsigned char ssdp_proc_aml[] = {
 0x53,
 0x53,
 0x44,
 0x54,
-0x78,
+0x8e,
 0x0,
 0x0,
 0x0,
 0x1,
-0xb8,
+0x19,
 0x42,
 0x58,
 0x50,
@@ -34,21 +43,21 @@ static unsigned char ssdp_proc_aml[] = {
 0x4e,
 0x54,
 0x4c,
-0x23,
-0x8,
-0x13,
+0x28,
+0x5,
+0x10,
 0x20,
 0x5b,
 0x83,
-0x42,
-0x5,
+0x48,
+0x6,
 0x43,
 0x50,
 0x41,
 0x41,
 0xaa,
-0x10,
-0xb0,
+0x0,
+0x0,
 0x0,
 0x0,
 0x0,
@@ -74,6 +83,16 @@ static unsigned char ssdp_proc_aml[] = {
 0x30,
 0x37,
 0x0,
+0x8,
+0x43,
+0x50,
+0x58,
+0x4d,
+0xc,
+0xaa,
+0xaa,
+0xaa,
+0xaa,
 0x14,
 0xf,
 0x

Re: [Qemu-devel] [PATCH 02/20] target-i386: convert 'hv_spinlocks' to static property

[Igor Mammedov]

On Wed, 27 Nov 2013 18:55:57 +0100
Andreas Färber  wrote:

> Am 16.07.2013 00:25, schrieb Igor Mammedov:
> > Signed-off-by: Igor Mammedov 
> > ---
> > v2:
> >  - rebase on top of hyperv_spinlock_attempts in X86CPU
> > ---
> >  target-i386/cpu.c | 48 +++-
> >  1 file changed, 47 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 14e9c7e..00c2882 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1473,6 +1473,49 @@ static void x86_cpu_get_feature_words(Object *obj, 
> > Visitor *v, void *opaque,
> >  error_propagate(errp, err);
> >  }
> >  
> > +static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
> > + const char *name, Error **errp)
> > +{
> > +X86CPU *cpu = X86_CPU(obj);
> > +int64_t value = cpu->hyperv_spinlock_attempts;
> > +
> > +visit_type_int(v, &value, name, errp);
> > +}
> > +
> > +static void x86_set_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
> > + const char *name, Error **errp)
> > +{
> > +const int64_t min = 0xFFF;
> > +const int64_t max = UINT_MAX;
> > +X86CPU *cpu = X86_CPU(obj);
> > +int64_t value;
> > +
> > +visit_type_int(v, &value, name, errp);
> > +if (error_is_set(errp)) {
> > +return;
> > +}
> 
> errp may be NULL. And if an Error gets raised here but not set to *errp
> for lack of pointer, value might be uninitialized:
> object_property_parse(obj, "not-a-number", "hv-spinlocks", NULL);
> So we cannot rely on error_is_set(errp) but must use a local variable to
> enforce any return. Fixed on qom-cpu-next as follows:

There is ~29 easily found instances of invalid use in current master, we
probably should fix them all.


> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 435b3b9..0a5a4f0 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1611,10 +1611,12 @@ static void x86_set_hv_spinlocks(Object *obj,
> Visitor *v, void *opaque,
>  const int64_t min = 0xFFF;
>  const int64_t max = UINT_MAX;
>  X86CPU *cpu = X86_CPU(obj);
> +Error *err = NULL;
>  int64_t value;
> 
> -visit_type_int(v, &value, name, errp);
> -if (error_is_set(errp)) {
> +visit_type_int(v, &value, name, &err);
> +if (err) {
> +error_propagate(errp, err);
>  return;
>  }
> 
> 
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next
Reviewed-By: Igor Mammedov 


> 
> Regards,
> Andreas
> 
> 
> > +
> > +if (value < min || value > max) {
> > +error_setg(errp, "Property %s.%s doesn't take value %" PRId64
> > +  " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
> > +  object_get_typename(obj), name ? name : "null",
> > +  value, min, max);
> > +return;
> > +}
> > +cpu->hyperv_spinlock_attempts = value;
> > +}
> > +
> > +static PropertyInfo qdev_prop_spinlocks = {
> > +.name  = "int",
> > +.get   = x86_get_hv_spinlocks,
> > +.set   = x86_set_hv_spinlocks,
> > +};
> > +
> > +static Property cpu_x86_properties[] = {
> > +{ .name  = "hv-spinlocks", .info  = &qdev_prop_spinlocks },
> > +DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> >  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
> >  {
> >  x86_def_t *def;
> > @@ -1586,6 +1629,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, 
> > char *features, Error **errp)
> >  } else if (!strcmp(featurestr, "hv-spinlocks")) {
> >  char *err;
> >  const int min = 0xFFF;
> > +char num[32];
> >  numvalue = strtoul(val, &err, 0);
> >  if (!*val || *err) {
> >  error_setg(errp, "bad numerical value %s", val);
> > @@ -1597,7 +1641,8 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, 
> > char *features, Error **errp)
> >  min);
> >  numvalue = min;
> >  }
> > -cpu->hyperv_spinlock_attempts = numvalue;
> > +snprintf(num, sizeof(num), "%" PRId32, numvalue);
> > +object_property_parse(OBJECT(cpu), num, featurestr, errp);
> >  } else {
> >  error_setg(errp, "unrecognized feature %s", featurestr);
> >  goto out;
> > @@ -2521,6 +2566,7 @@ static void x86_cpu_common_class_init(ObjectClass 
> > *oc, void *data)
> >  xcc->parent_realize = dc->realize;
> >  dc->realize = x86_cpu_realizefn;
> >  dc->bus_type = TYPE_ICC_BUS;
> > +dc->props = cpu_x86_properties;
> >  
> >  xcc->parent_reset = cc->reset;
> >  cc->reset = x86_cpu_reset;
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg


-- 
Regards,
  Igor

Re: [Qemu-devel] [PATCH 02/20] target-i386: convert 'hv_spinlocks' to static property

[Paolo Bonzini]

Il 27/11/2013 18:55, Andreas Färber ha scritto:
> Am 16.07.2013 00:25, schrieb Igor Mammedov:
>> Signed-off-by: Igor Mammedov 
>> ---
>> v2:
>>  - rebase on top of hyperv_spinlock_attempts in X86CPU
>> ---
>>  target-i386/cpu.c | 48 +++-
>>  1 file changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 14e9c7e..00c2882 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -1473,6 +1473,49 @@ static void x86_cpu_get_feature_words(Object *obj, 
>> Visitor *v, void *opaque,
>>  error_propagate(errp, err);
>>  }
>>  
>> +static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
>> + const char *name, Error **errp)
>> +{
>> +X86CPU *cpu = X86_CPU(obj);
>> +int64_t value = cpu->hyperv_spinlock_attempts;
>> +
>> +visit_type_int(v, &value, name, errp);
>> +}
>> +
>> +static void x86_set_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
>> + const char *name, Error **errp)
>> +{
>> +const int64_t min = 0xFFF;
>> +const int64_t max = UINT_MAX;
>> +X86CPU *cpu = X86_CPU(obj);
>> +int64_t value;
>> +
>> +visit_type_int(v, &value, name, errp);
>> +if (error_is_set(errp)) {
>> +return;
>> +}
> 
> errp may be NULL. And if an Error gets raised here but not set to *errp
> for lack of pointer, value might be uninitialized:
> object_property_parse(obj, "not-a-number", "hv-spinlocks", NULL);
> So we cannot rely on error_is_set(errp) but must use a local variable to
> enforce any return. Fixed on qom-cpu-next as follows:
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 435b3b9..0a5a4f0 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1611,10 +1611,12 @@ static void x86_set_hv_spinlocks(Object *obj,
> Visitor *v, void *opaque,
>  const int64_t min = 0xFFF;
>  const int64_t max = UINT_MAX;
>  X86CPU *cpu = X86_CPU(obj);
> +Error *err = NULL;
>  int64_t value;
> 
> -visit_type_int(v, &value, name, errp);
> -if (error_is_set(errp)) {
> +visit_type_int(v, &value, name, &err);
> +if (err) {
> +error_propagate(errp, err);
>  return;
>  }

Reviewed-by: Paolo Bonzini 

Re: [Qemu-devel] [PATCH 13/27] acpi: memory hotplug ACPI hardware implementation

[Eric Blake]

On 11/20/2013 07:38 PM, Igor Mammedov wrote:
> - implements QEMU hardware part of memory hotplug protocol
>   described at "docs/specs/acpi_mem_hotplug.txt"
> - handles only memory add notification event for now
> 
> Signed-off-by: Igor Mammedov 
> ---

> +0xa00:
> +  read access:
> +  [0x0-0x3] Lo part of memory device phys address
> +  [0x4-0x7] Hi part of memory device phys address
> +  [0x8-0xb] Lo part of memory device size in bytes
> +  [0xc-0xf] Hi part of memory device size in bytes
> +  [0x14] highest memory hot-plug interface version supported by QEMU

Nothing about [0x10-0x13]? [Ah, I see you mention it later]

> +  [0x15] Memory device status fields
> +  bits:
> +  1: device is enabled and may be used by guest
> +  2: device insert event, used by ACPI BIOS to distinguish
> + device for which no device check event to OSPM was issued

Why start at bit 1 instead of bit 0?  Also, should you document that
remaining bits (2-7) should be ignored on read?

> +  [0x16-0x17] reserved
> +
> +  write access:
> +  [0x0-0x3] Memory device slot selector, selects active memory device.
> +All following accesses to other registers in 0xaf80-0xaf97
> +region will read/store data from/to selected memory device.
> +  [0x4-0x7] OST event code reported by OSPM
> +  [0x8-0xb] OST status code reported by OSPM
> +  [0x15] Memory device status fields

Nothing about behavior of [0xc-0x14]?  Is it an error to write there, or
are the writes just ignored, or...?

> +  bits:
> +  2: if set to 1 clears device insert event, set by ACPI BIOS
> + after it has sent device check event to OSPM for
> + seleted memory device

s/seleted/selected/

What must be written into the other bits?  Must reserved bits be written
as 0, or the user do a read-modify-write, or...?

> +
> +Selecting memory device slot beyond present range has no effect on platform:
> +   - not documented above write accesses to memory hot-plug registers
> + are ignored;

Reads awkwardly.  Better is:

write accesses to memory hot-plug registers not documented above are ignored

> +   - not documented above read accesses to memory hot-plug registers return 
> 0xFF

Similar awkward wording.

> +case 0xc: /* Hi part of DIMM size */
> +val = object_property_get_int(OBJECT(mdev->dimm), "size", NULL) >> 
> 32;
> +break;
> +case 0x10: /* node proximity for _PXM method */
> +val = object_property_get_int(OBJECT(mdev->dimm), "node", NULL);
> +break;

This wasn't documented.

> +case 0x14: /* intf version */
> +val = 1;
> +break;
> +case 0x15: /* pack and return is_* fields */
> +val |= mdev->is_enabled   ? 1 : 0;
> +val |= mdev->is_inserting ? 2 : 0;

This is bit 0 and 1, not bit 1 and 2.

-- 
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 02/20] target-i386: convert 'hv_spinlocks' to static property

[Andreas Färber]

Am 16.07.2013 00:25, schrieb Igor Mammedov:
> Signed-off-by: Igor Mammedov 
> ---
> v2:
>  - rebase on top of hyperv_spinlock_attempts in X86CPU
> ---
>  target-i386/cpu.c | 48 +++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 14e9c7e..00c2882 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1473,6 +1473,49 @@ static void x86_cpu_get_feature_words(Object *obj, 
> Visitor *v, void *opaque,
>  error_propagate(errp, err);
>  }
>  
> +static void x86_get_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> +X86CPU *cpu = X86_CPU(obj);
> +int64_t value = cpu->hyperv_spinlock_attempts;
> +
> +visit_type_int(v, &value, name, errp);
> +}
> +
> +static void x86_set_hv_spinlocks(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> +const int64_t min = 0xFFF;
> +const int64_t max = UINT_MAX;
> +X86CPU *cpu = X86_CPU(obj);
> +int64_t value;
> +
> +visit_type_int(v, &value, name, errp);
> +if (error_is_set(errp)) {
> +return;
> +}

errp may be NULL. And if an Error gets raised here but not set to *errp
for lack of pointer, value might be uninitialized:
object_property_parse(obj, "not-a-number", "hv-spinlocks", NULL);
So we cannot rely on error_is_set(errp) but must use a local variable to
enforce any return. Fixed on qom-cpu-next as follows:

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 435b3b9..0a5a4f0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1611,10 +1611,12 @@ static void x86_set_hv_spinlocks(Object *obj,
Visitor *v, void *opaque,
 const int64_t min = 0xFFF;
 const int64_t max = UINT_MAX;
 X86CPU *cpu = X86_CPU(obj);
+Error *err = NULL;
 int64_t value;

-visit_type_int(v, &value, name, errp);
-if (error_is_set(errp)) {
+visit_type_int(v, &value, name, &err);
+if (err) {
+error_propagate(errp, err);
 return;
 }


https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next

Regards,
Andreas


> +
> +if (value < min || value > max) {
> +error_setg(errp, "Property %s.%s doesn't take value %" PRId64
> +  " (minimum: %" PRId64 ", maximum: %" PRId64 ")",
> +  object_get_typename(obj), name ? name : "null",
> +  value, min, max);
> +return;
> +}
> +cpu->hyperv_spinlock_attempts = value;
> +}
> +
> +static PropertyInfo qdev_prop_spinlocks = {
> +.name  = "int",
> +.get   = x86_get_hv_spinlocks,
> +.set   = x86_set_hv_spinlocks,
> +};
> +
> +static Property cpu_x86_properties[] = {
> +{ .name  = "hv-spinlocks", .info  = &qdev_prop_spinlocks },
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
>  {
>  x86_def_t *def;
> @@ -1586,6 +1629,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char 
> *features, Error **errp)
>  } else if (!strcmp(featurestr, "hv-spinlocks")) {
>  char *err;
>  const int min = 0xFFF;
> +char num[32];
>  numvalue = strtoul(val, &err, 0);
>  if (!*val || *err) {
>  error_setg(errp, "bad numerical value %s", val);
> @@ -1597,7 +1641,8 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char 
> *features, Error **errp)
>  min);
>  numvalue = min;
>  }
> -cpu->hyperv_spinlock_attempts = numvalue;
> +snprintf(num, sizeof(num), "%" PRId32, numvalue);
> +object_property_parse(OBJECT(cpu), num, featurestr, errp);
>  } else {
>  error_setg(errp, "unrecognized feature %s", featurestr);
>  goto out;
> @@ -2521,6 +2566,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
> void *data)
>  xcc->parent_realize = dc->realize;
>  dc->realize = x86_cpu_realizefn;
>  dc->bus_type = TYPE_ICC_BUS;
> +dc->props = cpu_x86_properties;
>  
>  xcc->parent_reset = cc->reset;
>  cc->reset = x86_cpu_reset;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [qemu PATCH] hw/i386/pc_sysfw: support more than one flash drive

[Laszlo Ersek]

On 11/27/13 18:22, Markus Armbruster wrote:

> Perhaps the proper way to back partially writable flash contents isn't
> splitting it into two devices, but backing a single device with a COW.
> The backing file has initial contents (say BIOS image), the delta may
> have additional contents (say non-volatile configuration), and picks up
> whatever the device model permits to be written to the flash.

Kevin brought this up before. The problem is that this prevents you from
upgrading the read-only part in O(1).

(Unless of course you're willing to make the backing file raw *and* to
poke directly in it when you upgrade the bios binary part. Theoretically
you could do this, because the divide between the varstore and the
binary part at 128KB falls at a qcow2 block boundary (supposing a 64K
qcow2 block size), but in practice this is a horrible idea :))

Let's not veer off into architecting a new star destroyer. I think I
have plenty ammo from your great notes thus far that I can hack up a v2
with :)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH 04/27] vl: convert -m to qemu_opts_parse()

[Markus Armbruster]

Igor Mammedov  writes:

> On Wed, 27 Nov 2013 15:35:09 +0100
> Markus Armbruster  wrote:
>
>> Igor Mammedov  writes:
>> 
>> > On Tue, 26 Nov 2013 15:49:05 +0100
>> > Markus Armbruster  wrote:
>> >
>> >> Igor Mammedov  writes:
>> >> 
>> >> > On Thu, 21 Nov 2013 11:12:43 +0100
>> >> > Markus Armbruster  wrote:
>> >> >
>> >> >> Igor Mammedov  writes:
>> >> >> 
>> > [...]
>> >> Two separate issues here:
>> >> 
>> >> 1. The "no qemu_mem_opts have been specified" case
>> >> 
>> >>This is equivalent to "empty options".  Therefore, the case can be
>> >>eliminated by pre-creating empty options.  No objection.
>> >> 
>> >>The three existing merge_lists users don't do that.  Perhaps they
>> >>should.
>> >> 
>> >> 2. How to provide default values
>> >> 
>> >>Supplying defaults is left to the caller of qemu_opt_get_FOO() by
>> >>design.
>> >> 
>> >>Pre-creating option parameters deviates from that pattern.  You
>> >>justify it by saying it "eliminates need to pepper code with
>> >>DEFAULT_RAM_SIZE * 1024 * 1024".  How many occurrences?
>> > beside of vl.c for:
>> >   mem & maxmem 1 in hw/i386/pc.c
>> >   slots - 6 in several files
>> 
>> Could the common code be factored out the old-fashioned way?
> replacing one one-liner with another might help a little but
> won't change a thing in general. It will be essentially the same.

Need to review your latest to have an opinion here :)

>> Precedence: qemu_get_machine_opts() encapsulates some QemuOpts-related
>> details, so its many users don't have to deal with them.
>> 
>> > see below for continuation:
>> >
>> >> 
>> >>Drawback: you lose the ability to see whether the user gave a value.
>> >>See below.
>> >> 
>> > [...]
>> >> >> Ugly.
>> >> >> 
>> >> >> Why is the variable called 'end'?
>> >> > would be 'suffix' better?
>> >> 
>> >> end points to the whole value string, not the end of anything, and
>> >> neither to a suffix of anything.
>> > Any suggestions?
>> 
>> What about val?
> I've replaced it with "mem_str" see
> "[PATCH 04/28] vl: convert -m to QemuOpts"

Works for me.

>> > [...]
>> >> >> If you refrain from putting defaults into opts, you can distinguish the
>> >> >> cases "user didn't specify maxmem, so assume mem", and "user specified
>> >> >> maxmem, so check it's >= mem".
>> >> > So foar, there is no point in distinguishing above cases,
>> >> > since maxmem <= mem is invalid value and hotplug should be disabled.
>> >> > So setting default maxmem to mem or anything less effectively
>> >> > disables hotplug.
>> >> 
>> >> Yes, setting maxmem < mem is invalid and should be rejected, but not
>> >> setting maxmem at all should be accepted even when you set mem.
>> >> 
>> >> Your patch like this pseudo-code:
>> >> 
>> >> mem = DEFAULT_RAM_SIZE * 1024 * 1024
>> >> maxmem = mem
>> >> 
>> >> if user specifies mem:
>> >> mem = user's mem
>> >> if user specifes max-mem:
>> >> mem = user's max-mem
>> >> 
>> >> if max-mem < mem
>> >> what now?
>> >> should error our if max-mem and mem were specified by the user
>> >> shouldn't if user didn't specify max-mem!
>> >> but can't say whether he did
>> >> 
>> >> I'd do it this way:
>> >> 
>> >> mem = unset
>> >> maxmem = unset
>> >> 
>> >> if user specifies mem:
>> >> mem = user's mem
>> >> if user specifes max-mem:
>> >> mem = user's max-mem
>> >> 
>> >> if mem != unset && max-mem != unset && max-mem < mem
>> >> error
>> >>
>> >> I'd use QemuOpts for the user's command line, and no more.  For anything
>> >> beyond that, I'd use ordinary variables, such as ram_size.
>> > Ok, I'll revert to the old code where options users check for option
>> > availability, it's not that much code.
>> >
>> >
>> > As for using QemuOpts as global store for global variables:
>> >
>> >  * using local variables would require changing of machine init or/and
>> >QEMUMachine and changing functions signature pass them down the stack to
>> >consumers.
>> 
>> Extending QEMUMachineInitArgs should suffice.  Once you're inside the
>> board code, passing stuff around as C parameters is probably an
>> improvement over passing around QemuOpts.
>> 
>> >  * adding "slots" readonly property to i440fx & q35 for consumption in
>> >ACPI hotplug code and building ACPI tables. It would be
>> > essentially another
>> >global lookup for i440fx & q35  object and pulling "slots" property,
>> >which is much longer way/complex way to get global value. That's a lot 
>> > of
>> >boilerplate code for the same outcome.
>> 
>> Can't say without seeing the code.
>> 
>> >  * about setting default for "mem" value: if default "mem" is not set and
>> >no -m is provided on CLI, we get case where
>> >   ram_size = foo & "mem" unset  
>> >And if I recall correctly there was an effort to provide interface for
>> >currently used QemuOpts to external users. So "mem" would get 
>> > inconsistent
>> > 

Re: [Qemu-devel] [qemu PATCH] hw/i386/pc_sysfw: support more than one flash drive

[Markus Armbruster]

Laszlo Ersek  writes:

> On 11/27/13 15:45, Markus Armbruster wrote:
>> Laszlo Ersek  writes:
>> 
>>> On 11/27/13 14:52, Markus Armbruster wrote:
 Jordan Justen  writes:

> On Tue, Nov 26, 2013 at 5:32 AM, Laszlo Ersek  wrote:
>> On 11/26/13 13:36, Markus Armbruster wrote:
>>
>>> Your stated purpose for multiple -pflash:
>>>
>>> This accommodates the following use case: suppose that OVMF
>>> is split in
>>> two parts, a writeable host file for non-volatile variable
>>> storage, and a
>>> read-only part for bootstrap and decompressible executable code.
>>>
>>> Such a split between writable part and read-only part makes sense to me.
>>> How is it done in physical hardware?  Single device with configurable
>>> write-protect, or two separate devices?
>>
>> (Jordan could help more.)
>>
>> Likely one device that's fully writeable.
>
> Most parts will have a dedicated read-only line.
>
> Many devices have 'block-locking' that will make some subset of blocks
> read-only until a reset.
>
> In addition to this, many chipsets will allow flash writes to be
> protected by triggering SMM when a flash write occurs.
>
> Using multiple chips are less common due to cost, but this is not a
> factor for QEMU. :)

 Should we stick to what real hardware does?  Single device, perhaps with
 block locking.
>>>
>>> I can't back a single flash device with two drives (= two host-side
>>> files), which is the incentive for this change.
>> 
>> There's no fundamental reason why a single device model instance could
>> not be backed by two block backends, named by two drive properties.
>> 
>> I'm not claiming this is the best solution, just offering it for
>> consideration.
>
> I'll pass :) I guess in theory we could push down the multi-drive thing
> to "pflash_cfi01.c". But:
> - It is used by a bunch of other boards.
> - Regarding i386, the second drive could automatically become (dependent
> on the first drive's size) part of the range that is mirrored in
> isa-bios space. Probably not intended.

I suspect real hardware mirrors the last 128KiB of its only flash chip
regardless of where the write-protected part begins.

> - The previous point strengthens the "pflash is used by other boards
> too" concern: in case of i386, I'm at least halfway aware of the
> board-specific consequences of sneaking in another drive, but I have no
> clue about the other boards.
> - If we decide at some point to map / paste backing drives in a loop,
> then the (at that time) existent device properties of pflash, let's call
> them "drive0" and "drive1", will look clumsy. We'll need a way to parse
> an unknown number of backend (drive) IDs. A mess.

Perhaps the proper way to back partially writable flash contents isn't
splitting it into two devices, but backing a single device with a COW.
The backing file has initial contents (say BIOS image), the delta may
have additional contents (say non-volatile configuration), and picks up
whatever the device model permits to be written to the flash.

Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor

[Paolo Bonzini]

Il 27/11/2013 18:02, Markus Armbruster ha scritto:
> I have to admit I can't tell offhand what the heck QMP's netdev_add
> accepts, because I don't understand what "'*props:': '**'" means in
> qapi-schema.json.  Even today, we permit code to serve as documentation
> and specification for new features.  I wish we didn't.

It is ignored, because qmp_netdev_add does not use an
automatically-generated unmarshaler.

> Anyway, whatever it parses ends up in a QDict, as usual.

The QDict comes straight from the monitor dispatch.  It is the QDict
that ordinarily would be unmarshaled by automatically-generated code.

> The part that sucks is the use of QemuOpts as netdev_add() parameter,
> 
> It made some sense when it was done, because then the command line was
> the only user, its data type for option parameters was (and is)
> QemuOpts, and QemuOpts was the least inconvenient way to do a function
> that wants a a discriminated union of parameters, like netdev_add does.
> 
> It stopped making sense when we started using it from QMP, whose data
> type for parameters is QDict.  I shoehorned netdev_add into QMP in its
> early days.  Hinsight 20/20...

Yes.

> In my opinion, use of QemuOpts for anything but parsing parameter
> strings has become a mistake.

Yes.  OTOH QemuOpts is the only reason why QMP and HMP can use a common
back-end (netdev_add).  With object-add you don't use QemuOpts (for good
reasons: you want to separate the human-oriented parsing and the
JSON-oriented parsing) and thus you need separate code for QMP and
HMP---at least for now.

The HMP code can use QemuOpts just like vl.c.  The QMP code would use
qdict_iter or qdict_first/next and call object_set_property for each
element of the dictionary.

> Laszlo converted net.c to saner internal interfaces (commit
> d195325..1a0c095).  Perhaps we can build on that.

Yeah, that could be an idea.  Another is to convert netdevs to QOM
(reusing Laszlo's data structures of course) and then just use
object-add and -object.

Paolo

Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor

[Markus Armbruster]

Paolo Bonzini  writes:

> Il 27/11/2013 15:15, Markus Armbruster ha scritto:
 >> This is unfortunately a counter-example to the rule that HMP commands
 >> should always be implemented in terms of their QMP counterparts.  I do
 >> not believe this is really a problem.  It can be fixed later; for now, I
 >> think "perfect is the enemy of good" applies.
>> I don't get why there's a counter-example.
>
> Take as an example the current implementation of netdev_add.
>
> void hmp_netdev_add(Monitor *mon, const QDict *qdict)
> {
> ...
> QemuOpts *opts = qemu_opts_from_qdict(qemu_find_opts("netdev"),
> qdict, &err);
> netdev_add(opts, &err);
> ...
> }
>
> int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret)
> {
> opts_list = qemu_find_opts_err("netdev", &local_err);
> opts = qemu_opts_from_qdict(opts_list, qdict, &local_err);
> netdev_add(opts, &local_err);
> ...
> return 0;
>
> exit_err:
> ...
> }
>
> These obey the rules you have below (there's just the weirdness that the QMP
> command handler is not autogenerated).
>
> But as you see above, this is really the same code for both handlers,
> they only differ in error handling minutiae.  This is possible with
> netdev_add (and you could do the same for device_add) because the QMP
> interface sucks and it is not well defined unless you make all values
> strings (qemu_opts_from_dict tries to do something meaningful for integers
> and floats, but given the existence of qdev property types such as hex32
> I wouldn't trust it too much).

netdev_add is a problematic example, because its a horrible, horrbible
QMP command.  I may say that, because I committed the crime :)

hmp_netdev_add() is the HMP command handler, and qmp_netdev_add() is the
QMP command handler.

Their error handling differs only because the former still implements
the legacy handler interface.

HMP's netdev_add parses arguments into a QemuOpts, exactly like -netdev.

I have to admit I can't tell offhand what the heck QMP's netdev_add
accepts, because I don't understand what "'*props:': '**'" means in
qapi-schema.json.  Even today, we permit code to serve as documentation
and specification for new features.  I wish we didn't.

Anyway, whatever it parses ends up in a QDict, as usual.

The part that sucks is the use of QemuOpts as netdev_add() parameter,

It made some sense when it was done, because then the command line was
the only user, its data type for option parameters was (and is)
QemuOpts, and QemuOpts was the least inconvenient way to do a function
that wants a a discriminated union of parameters, like netdev_add does.

It stopped making sense when we started using it from QMP, whose data
type for parameters is QDict.  I shoehorned netdev_add into QMP in its
early days.  Hinsight 20/20...

In my opinion, use of QemuOpts for anything but parsing parameter
strings has become a mistake.

Laszlo converted net.c to saner internal interfaces (commit
d195325..1a0c095).  Perhaps we can build on that.

> If we want a really different interface between HMP and QMP, one
> human-oriented and the other JSON-oriented, I'm not sure that you can
> share much code between the two implementation.

You should be able to share everything but the QMP marshalling, and the
HMP parsing and printing.

> Paolo
>
>> The rules are
>> 
>> 1. A QMP command handler (which needs to be of type int (*)(Monitor *,
>> const QDict *params, QObject **ret_data)) should be a thin wrapper
>> around a function with a type appropriate for C that contains all the
>> logic.  The wrapper merely translates.
>> 
>> 2. HMP commands are to be built on top of these functions, not their
>> handler wrappers.

Re: [Qemu-devel] [PATCH 0/17 v3] Localhost migration with side channel for ram

[Andrea Arcangeli]

On Tue, Nov 26, 2013 at 12:17:09PM +0100, Paolo Bonzini wrote:
> Il 26/11/2013 12:07, Lei Li ha scritto:
> > For this, I am not quite sure I understand it correctly, seems the latest
> > update of post copy migration was sent on last Oct, would you please give
> > some insights on what else could I do for the coupling with postcopy
> > migration?
> 
> I don't know the state exactly.  Orit and Andrea should know.

Ok, about the last update sent, so I'm not optimistic the kernel
backend is good because it uses a device driver that allocates the
memory locally and effectively disables THP KSM swap compression
overcommit and automatic NUMA balancing.

I wrote a new kernel backend by introducing two new kernel features:

1) MADV_USERFAULT (to deliver the KVM/qemu page fault to qemu userland)

2) remap_anon_pages (new syscall that qemu will use inside the
   migration thread that gets out of band events from the userland
   page fault, and also to do the background network transfer of all
   RAM while the guest already runs on the destination node)

Now you use vmsplice so you don't need remap_anon_pages in your case.

You only need MADV_USERFAULT.

I added a FOLL_USERFAULT too, as if it's KVM trapping on it, it will
have to deliver the fault to qemu through a vmexit and it's not doing
that yet. KVM page faults calling gup_fast, will have to use
FOLL_USERFAULT. This also means changing the API of all gup_fast to
get a "foll" parameter, but we need to do that anyway to remove the
FOLL_GET and fix /dev/mem mapped as guest physical memory (FOLL_GET on
/dev/mem backfires), and to speedup the page fault too to avoid those
useless get_page/put_page during every fault (MMU notifier don't
require FOLL_GET or any page reference at any time as long as the page
goes in the spte and the proper spte locks are hold to serialize
against the MMU notifier events).

For the non-local case, remap_anon_pages should be faster than
vmsplice as it doesn't need to pass through a pipe and just mangles
two pagetables and two pmds based on the virtual address given as
parameter.

If you want to review the kernel backend I implemented for postcopy,
this is updated on my latest aa.git tree:

http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?id=e69e1067f1d7e0f441c0c222a1017a07afe0bfc9
http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?id=d182b5118e2b22dd73018b75dce027c4ebabce14

I also looked into sharing code with the volatile range for android
temporary page mappings that can be discared but that has various
reasons to want putting placeholders into the pagetable. And the
functionality is different too, which is why the volatile range needs
to put placeholders into the empty pagetables, after all...

I don't think we can use the volatile range because that would discard
the pages too. MADV_USERFAULT is also somewhat simpler and it provides
just the user fault functionality (it cannot discard the pages). It
sends a sigbus instead of mapping a zero page and it doesn't even
require to allocate empty pagetables for the userfault range.

Once the live migration is complete MADV_USERFAULT should be cleared
from the vma simply with an madvise call, and any sign of it will go
away (unlike the device driver that stays forever). And once postcopy
completes all RAM is already entirely anonymous, already backed by THP
(if the out of band network transfers are 2M large it'll create 2M
pages in zero copy and there will never any sign of 4k pages for the
whole duration of migration) and the userfaulted memory can be NUMA
migrated or swapped out at any time. MADV_USERFAULT doesn't interfere
with swapouts.

remap_anon_pages also doesn't interfere with swapouts or automatic
NUMA migrations: if the received page gets swapped out before the
migration threads maps it in the guest physical address space, the
swap entry is transferred from the temporary address to the guest
physical address still with a single copy that reads and writes 8
bytes (just 1 cacheline written, modulo PT locks), and no I/O
triggers.

It would have been possible to also extend remap_file_pages to work on
anonymous memory instead of only nonlinear file mappings, however that
would alter the API as it wouldn't return -EINVAL anymore. It's easy
to change things if we want to use remap_file_pages for anonymous
memory too. Some larger discussion on the API details will be needed
but we're not at that point yet I think, and currently I'm more
interested to sort out the lowlevel details first, the kernel backend
API should be frozen at the last possible moment I think.

The qemu userland details of postcopy using the new kernel features are
still not finished, but conceptually the design is pretty clear.

This is far from definitive, if somebody has better ideas, please
comment of course.

Thanks,
Andrea

Re: [Qemu-devel] [PATCH V16 09/11] NUMA: set guest numa nodes memory policy

[Andrew Jones]

On Wed, Nov 27, 2013 at 03:35:53PM +0100, Paolo Bonzini wrote:
> > +
> > +len = numa_info[nodeid].node_mem;
> > +bind_mode = node_parse_bind_mode(nodeid);
> > +unsigned long *nodes = numa_info[nodeid].host_mem;
> > +
> > +/* This is a workaround for a long standing bug in Linux'
> > + * mbind implementation, which cuts off the last specified
> > + * node. To stay compatible should this bug be fixed, we
> > + * specify one more node and zero this one out.
> > + */
> > +unsigned long maxnode = find_last_bit(nodes, MAX_NODES);
> > +if (syscall(SYS_mbind, ram_ptr + ram_offset, len, bind_mode,
> > +nodes, maxnode + 2, 0)) {
> > +perror("mbind");
> > +return -1;
> > +}
> 
> Also, it's still not clear to me why we're not using libnuma.
>

There only seem to be two reasons to use it right now; 1)
it provides a wrapper around mbind 2) it exists. IMHO libnuma
needs some work before it would be suitable for qemu to marry
it. Its interfaces don't buy a lot of elegance, it only reads
system information on link time (i.e. no good if you want to
consider hotplug of cpus and nodes), and it's currently linux
specific anyway.  It's a good idea, but is it worth yet another
qemu build dependency?

drew

Re: [Qemu-devel] [PATCH 07/27] add memdev backend infrastructure

[Igor Mammedov]

On Wed, 27 Nov 2013 08:25:22 -0700
Eric Blake  wrote:

> On 11/20/2013 07:38 PM, Igor Mammedov wrote:
> > Provides framework for splitting host RAM allocation/
> > policies into a separate backend that could be used
> > by devices. It would allow to separate host specific
> > options from device model like it's done for netdev & co.
> 
> Just an interface review:
> 
> > +++ b/hmp-commands.hx
> > @@ -1719,3 +1719,16 @@ ETEXI
> >  STEXI
> >  @end table
> >  ETEXI
> > +
> > +{
> > +.name   = "memdev-add",
> 
> Typically, we've been naming HMP commands with underscore: memdev_add
there is several DASH command in HMP and I thought we stopped to
do UNDERSCORE and that DASH is preffered way now.

> 
> > +++ b/qapi-schema.json
> > @@ -4212,3 +4212,21 @@
> >  # Since: 1.7
> >  ##
> >  { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
> > +
> > +##
> > +# @memdev_add:
> 
> Whereas QMP commands have a dash: memdev-add
I'll fix it.

> 
> > +#
> > +# Add a host memory backend.
> > +#
> > +# @id: the name of the new memory backend
> > +# @size: amount of memory backend should allocate
> 
> Maybe add "in bytes" somewhere in the phrase to make it clear
Sure 
> 
> > +# @type: backend type. [default: compat-ram-host-memory]
> 
> That's the default, but what other types are valid?  I'd much rather
> have an enum of valid types than an open-coded string.
there is only one so far, I plan to add additional hugepage backend later.

> 
> > +#
> > +# Since: 1.8
> > +#
> > +# Returns: Nothing on success
> > +##
> > +{ 'command': 'memdev-add',
> 
> Ah, you did name the QMP command with dash, so it's the docs above that
> are wrong.
> 
> > +  'data': {'id': 'str', 'size': 'size', '*type': 'str'},
> > +  'gen': 'no'
> > +}
> 
> > +Arguments:
> > +
> > +- "id": the device's ID, must be unique (json-string)
> > +- "size": amount of memory backend should allocate (json-int)
> > +- "type": backend type (json-string, optional), default: 
> > compat-ram-host-memory
> > +
> > +Example:
> >  
> > +-> { "execute": "memdev-add",
> > + "arguments": { "id": "memdev1",
> > +"size": "1G",
> 
> I don't think this is valid QMP; you want to be passing sizes in bytes
> as an integer, not as a string that requires further parsing.
I'll amend it.

Thanks!

Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor

[Paolo Bonzini]

Il 27/11/2013 15:15, Markus Armbruster ha scritto:
>>> >> This is unfortunately a counter-example to the rule that HMP commands
>>> >> should always be implemented in terms of their QMP counterparts.  I do
>>> >> not believe this is really a problem.  It can be fixed later; for now, I
>>> >> think "perfect is the enemy of good" applies.
> I don't get why there's a counter-example.

Take as an example the current implementation of netdev_add.

void hmp_netdev_add(Monitor *mon, const QDict *qdict)
{
...
QemuOpts *opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, 
&err);
netdev_add(opts, &err);
...
}

int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret)
{
opts_list = qemu_find_opts_err("netdev", &local_err);
opts = qemu_opts_from_qdict(opts_list, qdict, &local_err);
netdev_add(opts, &local_err);
...
return 0;

exit_err:
...
}

These obey the rules you have below (there's just the weirdness that the QMP
command handler is not autogenerated).

But as you see above, this is really the same code for both handlers,
they only differ in error handling minutiae.  This is possible with
netdev_add (and you could do the same for device_add) because the QMP
interface sucks and it is not well defined unless you make all values
strings (qemu_opts_from_dict tries to do something meaningful for integers
and floats, but given the existence of qdev property types such as hex32
I wouldn't trust it too much).

If we want a really different interface between HMP and QMP, one
human-oriented and the other JSON-oriented, I'm not sure that you can
share much code between the two implementation.

Paolo

> The rules are
> 
> 1. A QMP command handler (which needs to be of type int (*)(Monitor *,
> const QDict *params, QObject **ret_data)) should be a thin wrapper
> around a function with a type appropriate for C that contains all the
> logic.  The wrapper merely translates.
> 
> 2. HMP commands are to be built on top of these functions, not their
> handler wrappers.

Re: [Qemu-devel] [PATCH 07/27] add memdev backend infrastructure

[Paolo Bonzini]

Il 27/11/2013 15:37, Igor Mammedov ha scritto:
> It looks like "realize" for -object / object-add implemented via
> an interface.

It does---but without unrealize and with the additional get_base_path.

> Maybe it should be renamed from QOMCommandLineIface to QOMRealizeIface
> and s/complete/realize/ so anyone who knows about Device.realize would
> get meaning without digging in complete() implementations.

There is an important difference; realize is an internal method in
Device, the external interface is the property.  So perhaps it's the
other way round; if Device implements QOMCommandLineIface you could
start creating devices with -object.

> Alternative would be to behave just like Rng/Tpm do, i.e. use -object
> to do late initialization in a backend user (DimmDevice.realize).
> Draw back of it would be user won't get error during the first command
> "object-add" and only will get error when creating DimmDevice calling
> "device_add".

That's also a possibility.  But again, maybe it's the other way round
and Rng/Tpm could enjoy better error handling if we add the interface.

Paolo

Re: [Qemu-devel] [RFC PATCH v2] i386: Add _PXM ACPI method to CPU objects

[Paolo Bonzini]

Il 27/11/2013 16:53, Igor Mammedov ha scritto:
> Patch looks good,
> Please add patch to update hw/i386/ssdt-proc.hex.generated for hosts without 
> iasl
> for completness

Also please rename PXM to CPXM or CPPX for consistency.

Paolo

>> > 
>> > ---
>> >  hw/i386/acpi-build.c  |5 +
>> >  hw/i386/ssdt-proc.dsl |5 +
>> >  2 files changed, 10 insertions(+)
>> > 
>> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> > index d089e1e..3c11ddc 100644
>> > --- a/hw/i386/acpi-build.c
>> > +++ b/hw/i386/acpi-build.c
>> > @@ -603,6 +603,7 @@ static inline char acpi_get_hex(uint32_t val)
>> >  #define ACPI_PROC_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2)
>> >  #define ACPI_PROC_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4)
>> >  #define ACPI_PROC_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start)
>> > +#define ACPI_PROC_OFFSET_CPUPXM (*ssdt_proc_pxm - *ssdt_proc_start)
>> >  #define ACPI_PROC_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
>> >  #define ACPI_PROC_AML (ssdp_proc_aml + *ssdt_proc_start)
>> >  
>> > @@ -724,6 +725,10 @@ build_ssdt(GArray *table_data, GArray *linker,
>> >  proc[ACPI_PROC_OFFSET_CPUHEX+1] = acpi_get_hex(i);
>> >  proc[ACPI_PROC_OFFSET_CPUID1] = i;
>> >  proc[ACPI_PROC_OFFSET_CPUID2] = i;
>> > +proc[ACPI_PROC_OFFSET_CPUPXM] = guest_info->node_cpu[i];
>> > +proc[ACPI_PROC_OFFSET_CPUPXM + 1] = 0;
>> > +proc[ACPI_PROC_OFFSET_CPUPXM + 2] = 0;
>> > +proc[ACPI_PROC_OFFSET_CPUPXM + 3] = 0;
>> >  }
>> >  
>> >  /* build this code:
>> > diff --git a/hw/i386/ssdt-proc.dsl b/hw/i386/ssdt-proc.dsl
>> > index 8229bfd..8d4c5bf 100644
>> > --- a/hw/i386/ssdt-proc.dsl
>> > +++ b/hw/i386/ssdt-proc.dsl
>> > @@ -47,6 +47,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", 
>> > "BXSSDT", 0x1)
>> >   * also updating the C code.
>> >   */
>> >  Name(_HID, "ACPI0007")
>> > +ACPI_EXTRACT_NAME_DWORD_CONST ssdt_proc_pxm
>> > +Name(PXM, 0x)
>> >  External(CPMA, MethodObj)
>> >  External(CPST, MethodObj)
>> >  External(CPEJ, MethodObj)
>> > @@ -59,5 +61,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", 
>> > "BXSSDT", 0x1)
>> >  Method(_EJ0, 1, NotSerialized) {
>> >  CPEJ(ID, Arg0)
>> >  }
>> > +Method(_PXM, 0) {
>> > +Return (PXM)
>> > +}
>> >  }

Re: [Qemu-devel] [PATCH 07/27] add memdev backend infrastructure

[Igor Mammedov]

On Wed, 27 Nov 2013 16:21:23 +0100
Paolo Bonzini  wrote:

> Il 27/11/2013 15:37, Igor Mammedov ha scritto:
> > It looks like "realize" for -object / object-add implemented via
> > an interface.
> 
> It does---but without unrealize and with the additional get_base_path.
> 
> > Maybe it should be renamed from QOMCommandLineIface to QOMRealizeIface
> > and s/complete/realize/ so anyone who knows about Device.realize would
> > get meaning without digging in complete() implementations.
> 
> There is an important difference; realize is an internal method in
> Device, the external interface is the property.  So perhaps it's the
> other way round; if Device implements QOMCommandLineIface you could
> start creating devices with -object.
> 
> > Alternative would be to behave just like Rng/Tpm do, i.e. use -object
> > to do late initialization in a backend user (DimmDevice.realize).
> > Draw back of it would be user won't get error during the first command
> > "object-add" and only will get error when creating DimmDevice calling
> > "device_add".
> 
> That's also a possibility.  But again, maybe it's the other way round
> and Rng/Tpm could enjoy better error handling if we add the interface.

Sure, I'll try to do as you described, it's much better to get error
earlier and from command/object that throws it than via proxy.

Thanks for suggestion, looking at netdev-add I even haven't thought
about using -object.

> Paolo

Re: [Qemu-devel] [RFC PATCH v2] i386: Add _PXM ACPI method to CPU objects

[Igor Mammedov]

On Wed, 27 Nov 2013 14:02:43 +0100
Vasilis Liaskovitis  wrote:

> This patch adds a _PXM method to ACPI CPU objects for the pc machine. The _PXM
> value is derived from the passed in guest info, same way as CPU SRAT entries.
> 
> Currently, CPU SRAT entries are only enabled for cpus that are already present
> in the system. The SRAT entries for hotpluggable processors are disabled 
> (flags
> bit 0 set to 0 in hw/i385/acpi-build.c:build_srat). Section 5.2.16.1 of ACPI
> spec mentions "If the Local APIC ID of a dynamically added processor is not
> present in the SRAT, a _PXM object must exist for the processor’s device or 
> one
> of its ancestors in the ACPI Namespace." Since SRAT entries are not available
> for the hot-pluggable processors, a _PXM method must exist for them. 
> Otherwise,
> the CPU is hot-added in the wrong NUMA node (default node 0).
> 
> Even if CPU SRAT entries are enabled, _PXM method is what the linux kernel
> consults on hot-add time. Section 17.2.1 of ACPI spec mentions " OSPM will
> consume the SRAT only at boot time. OSPM should use _PXM for any devices that
> are hot-added into the system after boot up." To be more precise if SRAT
> information is available to the guest kernel, it is used.  However, parsed 
> SRAT
> info is reset and lost after hot-remove operations, see kernel commit 
> c4c60524.
> This means that on a hot-unplug / hot-replug scenario, and without a _PXM
> method, the kernel may put a CPU on different nodes because SRAT info has been
> reset by a previous hot-remove operation.
> 
> The above hot-remove/hot-add scenario has been tested on master, plus cpu-del
> patches from:
> https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg01085.html
> With the curret _PXM patch, hot-added CPUs are always placed into the correct
> NUMA node, regardless of kernel behaviour.
> 
> v1->v2:
>Make method return a DWORD integer
>Tested on qemu master + cpu-del patches
> 
> Signed-off-by: Vasilis Liaskovitis 
> Reviewed-by: Thilo Fromm 
Patch looks good,
Please add patch to update hw/i386/ssdt-proc.hex.generated for hosts without 
iasl
for completness

> 
> ---
>  hw/i386/acpi-build.c  |5 +
>  hw/i386/ssdt-proc.dsl |5 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d089e1e..3c11ddc 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -603,6 +603,7 @@ static inline char acpi_get_hex(uint32_t val)
>  #define ACPI_PROC_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2)
>  #define ACPI_PROC_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4)
>  #define ACPI_PROC_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start)
> +#define ACPI_PROC_OFFSET_CPUPXM (*ssdt_proc_pxm - *ssdt_proc_start)
>  #define ACPI_PROC_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
>  #define ACPI_PROC_AML (ssdp_proc_aml + *ssdt_proc_start)
>  
> @@ -724,6 +725,10 @@ build_ssdt(GArray *table_data, GArray *linker,
>  proc[ACPI_PROC_OFFSET_CPUHEX+1] = acpi_get_hex(i);
>  proc[ACPI_PROC_OFFSET_CPUID1] = i;
>  proc[ACPI_PROC_OFFSET_CPUID2] = i;
> +proc[ACPI_PROC_OFFSET_CPUPXM] = guest_info->node_cpu[i];
> +proc[ACPI_PROC_OFFSET_CPUPXM + 1] = 0;
> +proc[ACPI_PROC_OFFSET_CPUPXM + 2] = 0;
> +proc[ACPI_PROC_OFFSET_CPUPXM + 3] = 0;
>  }
>  
>  /* build this code:
> diff --git a/hw/i386/ssdt-proc.dsl b/hw/i386/ssdt-proc.dsl
> index 8229bfd..8d4c5bf 100644
> --- a/hw/i386/ssdt-proc.dsl
> +++ b/hw/i386/ssdt-proc.dsl
> @@ -47,6 +47,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", 
> "BXSSDT", 0x1)
>   * also updating the C code.
>   */
>  Name(_HID, "ACPI0007")
> +ACPI_EXTRACT_NAME_DWORD_CONST ssdt_proc_pxm
> +Name(PXM, 0x)
>  External(CPMA, MethodObj)
>  External(CPST, MethodObj)
>  External(CPEJ, MethodObj)
> @@ -59,5 +61,8 @@ DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", 
> "BXSSDT", 0x1)
>  Method(_EJ0, 1, NotSerialized) {
>  CPEJ(ID, Arg0)
>  }
> +Method(_PXM, 0) {
> +Return (PXM)
> +}
>  }
>  }

Re: [Qemu-devel] [PATCH 0/2] vmdk: Allow version 3 read only open

[Stefan Hajnoczi]

On Wed, Nov 27, 2013 at 10:06:29AM +0800, Fam Zheng wrote:
> According to an update on VMware Knowledge Base [KB 2064959], we should be 
> safe
> to open version 3 as read only. This is meaningful as an compatibility
> improvement, so let's enable it.

Acked-by: Stefan Hajnoczi 

But please add a comment in vmdk.c that links to the Knowledge Base or
explains that version 3 simply adds the new CBT file which we can ignore
for reads.