Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Markus Armbruster]

"Michael S. Tsirkin"  writes:

> On Thu, Aug 22, 2013 at 01:24:50PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin"  writes:
>> 
>> > On Thu, Aug 22, 2013 at 10:39:34AM +0200, Markus Armbruster wrote:
>> >> "Michael S. Tsirkin"  writes:
>> >> 
>> >> > On Wed, Aug 21, 2013 at 06:01:45PM +0200, Markus Armbruster wrote:
>> >> >> "Michael S. Tsirkin"  writes:
>> >> >> 
>> >> >> > On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
>> >> >> >> "Michael S. Tsirkin"  writes:
>> >> >> >> 
>> >> >> >> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
>> >> >> >> >> We set default boot order "cad" in every single machine 
>> >> >> >> >> definition
>> >> >> >> >> except "pseries" and "moxiesim", even though very few
>> >> >> >> >> boards actually
>> >> >> >> >> care for boot order, and "cad" makes sense for even fewer.
>> >> >> >> >> 
>> >> >> >> >> Machines that care:
>> >> >> >> >> 
>> >> >> >> >> * pc and its variants
>> >> >> >> >> 
>> >> >> >> >>   Accept up to three letters 'a', 'b' (undocumented
>> >> >> >> >> alias for 'a'),
>> >> >> >> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
>> >> >> >> >> 
>> >> >> >> >> * nseries (n800, n810)
>> >> >> >> >> 
>> >> >> >> >>   Check whether order starts with 'n'.  Silently ignored
>> >> >> >> >> otherwise.
>> >> >> >> >> 
>> >> >> >> >> * prep, g3beige, mac99
>> >> >> >> >> 
>> >> >> >> >>   Extract the first character the machine understands (subset of
>> >> >> >> >>   'a'..'f').  Silently ignored otherwise.
>> >> >> >> >> 
>> >> >> >> >> * spapr
>> >> >> >> >> 
>> >> >> >> >>   Accept an arbitrary string (vl.c restricts it to contain only
>> >> >> >> >>   'a'..'p', no duplicates).
>> >> >> >> >> 
>> >> >> >> >> * sun4[mdc]
>> >> >> >> >> 
>> >> >> >> >>   Use the first character.  Silently ignored otherwise.
>> >> >> >> >> 
>> >> >> >> >> Strip characters these machines ignore from their
>> >> >> >> >> default boot order.
>> >> >> >> >> 
>> >> >> >> >> For all other machines, remove the unused default boot order
>> >> >> >> >> alltogether.
>> >> >> >> >> 
>> >> >> >> >> Note that my rename of QEMUMachine member boot_order to
>> >> >> >> >> default_boot_order and QEMUMachineInitArgs member boot_device to
>> >> >> >> >> boot_order has a welcome side effect: it makes every use of boot
>> >> >> >> >> orders visible in this patch, for easy review.
>> >> >> >> >> 
>> >> >> >> >> Signed-off-by: Markus Armbruster 
>> >> >> >> >> ---
>> >> >> >> [...]
>> >> >> >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> >> >> >> >> index 9327ac1..3700bd5 100644
>> >> >> >> >> --- a/hw/i386/pc_piix.c
>> >> >> >> >> +++ b/hw/i386/pc_piix.c
>> >> >> >> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs 
>> >> >> >> >> *args,
>> >> >> >> >>  }
>> >> >> >> >>  }
>> >> >> >> >>  
>> >> >> >> >> -pc_cmos_init(below_4g_mem_size, above_4g_mem_size, 
>> >> >> >> >> args->boot_device,
>> >> >> >> >> +pc_cmos_init(below_4g_mem_size, above_4g_mem_size, 
>> >> >> >> >> args->boot_order,
>> >> >> >> >>   floppy, idebus[0], idebus[1], rtc_state);
>> >> >> >> >>  
>> >> >> >> >>  if (pci_enabled && usb_enabled(false)) {
>> >> >> >> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>> >> >> >> >>  .hot_add_cpu = pc_hot_add_cpu,
>> >> >> >> >>  .max_cpus = 255,
>> >> >> >> >>  .is_default = 1,
>> >> >> >> >> -DEFAULT_MACHINE_OPTIONS,
>> >> >> >> >> +.default_boot_order = "cad",
>> >> >> >> >>  };
>> >> >> >> >>  
>> >> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >> >> >> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >> >> >> >>  PC_COMPAT_1_5,
>> >> >> >> >>  { /* end of list */ }
>> >> >> >> >>  },
>> >> >> >> >> -DEFAULT_MACHINE_OPTIONS,
>> >> >> >> >> +.default_boot_order = "cad",
>> >> >> >> >>  };
>> >> >> >> >>  
>> >> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
>> >> >> >> >
>> >> >> >> > So all PC machine types share this?
>> >> >> >> 
>> >> >> >> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my 
>> >> >> >> patch.
>> >> >> >> Which is defined as
>> >> >> >> 
>> >> >> >> #define DEFAULT_MACHINE_OPTIONS \
>> >> >> >> .boot_order = "cad"
>> >> >> >> 
>> >> >> >> I.e. my patch merely peels off a layer of obfuscation :)
>> >> >> >
>> >> >> > Using a macro in multiple places, instead of a hard-coded constant is
>> >> >> > not obfuscation.
>> >> >> >
>> >> >> >> > Can we set this in some common code, somehow?
>> >> >> >> 
>> >> >> >> We don't have an inheritance notion for machine types.
>> >> >> >> 
>> >> >> >> vl.c uses machine->boot_order before calling one of its methods, so
>> >> >> >> monkey-patching .boot_order from a method won't do.
>> >> >> >> Besides, that cure
>> >> >> >> looks much worse than the disease to me.
>> >> >> >> 
>> >> >> >> Can't think of anything else offhand.
>> >> >> >> 
>> >> >> >> [...]
>> >> >> >
>> >> >> > Se

Re: [Qemu-devel] [PATCH V9 07/12] NUMA: parse guest numa nodes memory policy

[Andrew Jones]

- Original Message -
> On 08/23/2013 10:11 PM, Andrew Jones wrote:
> > 
> > 
> > - Original Message -
> >> The memory policy setting format is like:
> >> 
> >> policy={default|membind|interleave|preferred}[,relative=true],host-nodes=N-N
> >> And we are adding this setting as a suboption of "-numa mem,",
> >> the memory policy then can be set like following:
> >> -numa node,nodeid=0,cpus=0 \
> >> -numa node,nodeid=1,cpus=1 \
> >> -numa mem,nodeid=0,size=1G,policy=membind,host-nodes=0-1 \
> >> -numa
> >> mem,nodeid=1,size=1G,policy=interleave,relative=true,host-nodes=1
> >>
> >> Signed-off-by: Wanlong Gao 
> >> ---
> >>  include/sysemu/sysemu.h |  3 +++
> >>  numa.c  | 13 +
> >>  qapi-schema.json| 31 +--
> >>  vl.c|  3 +++
> >>  4 files changed, 48 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> >> index b683d08..81d16a5 100644
> >> --- a/include/sysemu/sysemu.h
> >> +++ b/include/sysemu/sysemu.h
> >> @@ -134,6 +134,9 @@ extern int nb_numa_mem_nodes;
> >>  typedef struct node_info {
> >>  uint64_t node_mem;
> >>  DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
> >> +DECLARE_BITMAP(host_mem, MAX_CPUMASK_BITS);
> >> +NumaNodePolicy policy;
> >> +bool relative;
> >>  } NodeInfo;
> >>  extern NodeInfo numa_info[MAX_NODES];
> >>  extern QemuOptsList qemu_numa_opts;
> >> diff --git a/numa.c b/numa.c
> >> index 3e2dfc1..4ccc6cb 100644
> >> --- a/numa.c
> >> +++ b/numa.c
> >> @@ -74,6 +74,7 @@ static int numa_mem_parse(NumaMemOptions *opts)
> >>  {
> >>  uint16_t nodenr;
> >>  uint64_t mem_size;
> >> +uint16List *nodes;
> >>  
> >>  if (opts->has_nodeid) {
> >>  nodenr = opts->nodeid;
> >> @@ -91,6 +92,18 @@ static int numa_mem_parse(NumaMemOptions *opts)
> >>  numa_info[nodenr].node_mem = mem_size;
> >>  }
> >>  
> >> +if (opts->has_policy) {
> >> +numa_info[nodenr].policy = opts->policy;
> >> +}
> >> +
> >> +if (opts->has_relative) {
> >> +numa_info[nodenr].relative = opts->relative;
> >> +}
> >> +
> >> +for (nodes = opts->host_nodes; nodes; nodes = nodes->next) {
> >> +bitmap_set(numa_info[nodenr].host_mem, nodes->value, 1);
> >> +}
> >> +
> >>  return 0;
> >>  }
> >>  
> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> index 11851a1..650741f 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -3806,6 +3806,24 @@
> >> '*mem':'str' }}
> >>  
> >>  ##
> >> +# @NumaNodePolicy
> >> +#
> >> +# NUMA node policy types
> >> +#
> >> +# @default: restore default policy, remove any nondefault policy
> >> +#
> >> +# @membind: a strict policy that restricts memory allocation to the
> >> +#   nodes specified
> >> +#
> >> +# @interleave: the page allocations is interleaved across the set
> >> +#  of nodes specified
> >> +#
> >> +# @preferred: set the preferred node for allocation
> >> +##
> >> +{ 'enum': 'NumaNodePolicy',
> >> +  'data': [ 'default', 'membind', 'interleave', 'preferred' ] }
> >> +
> >> +##
> >>  # @NumaMemOptions
> >>  #
> >>  # Set memory information of guest NUMA node. (for OptsVisitor)
> >> @@ -3814,9 +3832,18 @@
> >>  #
> >>  # @size: #optional memory size of this node
> >>  #
> >> +# @policy: #optional memory policy of this node
> >> +#
> >> +# @relative: #optional if the nodes specified are relative
> >> +#
> >> +# @host-nodes: #optional host nodes for its memory policy
> >> +#
> >>  # Since 1.7
> >>  ##
> >>  { 'type': 'NumaMemOptions',
> >>'data': {
> >> -   '*nodeid': 'uint16',
> >> -   '*size':   'size' }}
> >> +   '*nodeid': 'uint16',
> >> +   '*size':   'size',
> >> +   '*policy': 'NumaNodePolicy',
> >> +   '*relative':   'bool',
> >> +   '*host-nodes': ['uint16'] }}
> >> diff --git a/vl.c b/vl.c
> >> index 2377b67..91b0d76 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -2888,6 +2888,9 @@ int main(int argc, char **argv, char **envp)
> >>  for (i = 0; i < MAX_NODES; i++) {
> >>  numa_info[i].node_mem = 0;
> >>  bitmap_zero(numa_info[i].node_cpu, MAX_CPUMASK_BITS);
> >> +bitmap_zero(numa_info[i].host_mem, MAX_CPUMASK_BITS);
> > 
> > Shouldn't the bitmap size of host_mem be MAX_NODES? If so, and you
> 
> MAX_NODES is for guest numa nodes number, but this bitmap is for host
> numa nodes. AFAIK, this MAX_NODES is not big enough for host nodes number,
> the default host kernel NODES_SHIFT is 9.

MAX_CPUMASK_BITS == 255 is also too small for a default node shift of 9.
You have to pick something, and then manage that limit. I think MAX_NODES
== 64 will be big enough for quite some time, but libnuma chooses 128 (see
/usr/include/numa.h:NUMA_NUM_NODES). So maybe we can bump MAX_NODES up to
128? You can also add a warning for when you detect starting on a machine
that has more than MAX_NODES nodes.

drew

> 
> Thanks,
> Wanlong Gao

[Qemu-devel] [Bug 1191326] Re: QNX 4 doesn't boot on qemu >= 1.3

[JQu]

problem appeared in this commit:

commit b90600eed3c0efe5f3260853c873caf51c0677b1
Author: Avi Kivity 
Date:   Wed Oct 3 16:42:37 2012 +0200

dma: make dma access its own address space

Instead of accessing the cpu address space, use an address space
configured by the caller.

Eventually all dma functionality will be folded into AddressSpace,
but we have to start from something.

Reviewed-by: Anthony Liguori 
Signed-off-by: Avi Kivity 

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

Title:
  QNX 4 doesn't boot on qemu >= 1.3

Status in QEMU:
  New

Bug description:
  
  I am using virtual machine with QNX4 operating system installed on it.  I 
updated my qemu from version
  to newer and QNX4 doesn't start any more. All is ok on version 1.2 but when I 
try to use any newer version 
  (1.3, 1.4, 1.5)  QNX4 doesn't boot.  I tried on windows and linux ubuntu 
hosts - effects are the same.

  When virtual machine boots qnx bootloader loads and starts operating system. 
In the next step
  qnx starts its ide driver, which detects qemu harddisk and cdrom. Problem 
starts when operating system
  tries mount partition - an error occur and qnx stop booting procedure:

  mount -p "No bios signature in partition sector on /dev/hd0"

  I have tried install qnx from cdrom but it seems that there is the same 
problem. QNX installer boot from
  cdrom, detects hard disk and cdrom, but cdrom can't be mounted in the next 
step of installation procedure.

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

Re: [Qemu-devel] [PATCH 03/16] ipack: Pass size to ipack_bus_new_inplace()

[Wenchao Xia]

> To be passed to qbus_create_inplace().


Simplify DEVICE() cast to avoid parent field access.

  s->dev will always point to pci_dev, so this change is safe.

Reviewed-by: Wenchao Xia 




Signed-off-by: Andreas Färber 
---
  hw/char/ipack.c   | 3 ++-
  hw/char/ipack.h   | 3 ++-
  hw/char/tpci200.c | 2 +-
  3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/char/ipack.c b/hw/char/ipack.c
index f890471..5fb7073 100644
--- a/hw/char/ipack.c
+++ b/hw/char/ipack.c
@@ -24,7 +24,8 @@ IPackDevice *ipack_device_find(IPackBus *bus, int32_t slot)
  return NULL;
  }

-void ipack_bus_new_inplace(IPackBus *bus, DeviceState *parent,
+void ipack_bus_new_inplace(IPackBus *bus, size_t bus_size,
+   DeviceState *parent,
 const char *name, uint8_t n_slots,
 qemu_irq_handler handler)
  {
diff --git a/hw/char/ipack.h b/hw/char/ipack.h
index f2b7a12..f8dc0f2 100644
--- a/hw/char/ipack.h
+++ b/hw/char/ipack.h
@@ -72,7 +72,8 @@ extern const VMStateDescription vmstate_ipack_device;
  VMSTATE_STRUCT(_field, _state, 1, vmstate_ipack_device, IPackDevice)

  IPackDevice *ipack_device_find(IPackBus *bus, int32_t slot);
-void ipack_bus_new_inplace(IPackBus *bus, DeviceState *parent,
+void ipack_bus_new_inplace(IPackBus *bus, size_t bus_size,
+   DeviceState *parent,
 const char *name, uint8_t n_slots,
 qemu_irq_handler handler);

diff --git a/hw/char/tpci200.c b/hw/char/tpci200.c
index d9e17b2..e04ff26 100644
--- a/hw/char/tpci200.c
+++ b/hw/char/tpci200.c
@@ -607,7 +607,7 @@ static int tpci200_initfn(PCIDevice *pci_dev)
  pci_register_bar(&s->dev, 4, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->las2);
  pci_register_bar(&s->dev, 5, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->las3);

-ipack_bus_new_inplace(&s->bus, DEVICE(&s->dev), NULL,
+ipack_bus_new_inplace(&s->bus, sizeof(s->bus), DEVICE(pci_dev), NULL,
N_MODULES, tpci200_set_irq);

  return 0;




--
Best Regards

Wenchao Xia

Re: [Qemu-devel] [RFC PATCH 00/47] Describing patchset

[Peter Maydell]

On 25 August 2013 23:58, Ákos Kovács  wrote:
> This is a request-for-comments patchset on the kconfig integration. The 
> patchset
> contains a 'scripts/kconfig' git submodule (PATCH 5, kconfig-frontends v3.10) 
> with
> the necessary modifications in the Makefile.objs (PATCHES 6-16).

Hi; could you describe the general reasons why we want to do this,
please? (ie the benefits we get from reworking our config/build system).

(also for next time round, make sure you write a useful subject in
your cover letter :-))

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 0/8] Implement reference count for BlockDriverState [resend]

[Stefan Hajnoczi]

On Fri, Aug 23, 2013 at 10:17:37AM +0800, Wenchao Xia wrote:
>   What is the correct steps to run full qemu-iotests?
> I modified qemu-iotests-quick.sh as:
> 
> #!/bin/sh
> 
> # We don't know which of the system emulator binaries there is (or
> if there is
> # any at all), so the 'quick' group doesn't contain any tests that require
> # running qemu proper. Assign a fake binary name so that
> qemu-iotests doesn't
> # complain about the missing binary.
> export QEMU_PROG="$(pwd)/x86_64-softmmu/qemu-system-x86_64"
> 
> export QEMU_IMG_PROG="$(pwd)/qemu-img"
> export QEMU_IO_PROG="$(pwd)/qemu-io"
> export QEMU_NBD_PROG="$(pwd)/qemu-nbd"
> 
> cd $SRC_PATH/tests/qemu-iotests
> 
> ret=0
> ./check -T -nocache -qcow2 || ret=1
> 
> exit $ret
> 
>   Then make check-block, 026 038 fail, 038 sometimes fail.
> The code from is upstream, host is RH6.3 @ x86_64. Do I missed some
> steps?

You are running it correctly.

There are a couple of known failures in qemu-iotests.  It would be good
to fix them but no one has posted patches so far.  If in doubt, run
qemu-iotests on qemu.git/master to see which tests are known failures.

Stefan

Re: [Qemu-devel] [PATCH] kvm: warn if num cpus is greater than num recommended

[Paolo Bonzini]

Il 23/08/2013 13:33, Andrew Jones ha scritto:
> Does smp_cpus map to the current
> number of cpus, or to the number of possible cpus? If it maps to the number
> of possible cpus, then this is the right place. If the former, then I guess
> it'll take more thought. I'ved added Igor (still on vacation) to this reply,
> but regardless I vote we worry about hot-plug limit checking in different
> patch.

smp_cpus is the initial number, max_cpus is the number of possible cpus.

Paolo

Re: [Qemu-devel] [PATCH 12/16] qdev: Pass size to qbus_create_inplace()

[Wenchao Xia]

于 2013-8-24 8:00, Andreas Färber 写道:

To be passed to object_initialize().

Since commit 39355c3826f5d9a2eb1ce3dc9b4cdd68893769d6 the argument is
void*, so drop some superfluous (BusState *) casts or direct parent
field usages.

Signed-off-by: Andreas Färber 
---
  hw/audio/intel-hda.c  | 2 +-
  hw/char/ipack.c   | 2 +-
  hw/char/virtio-serial-bus.c   | 4 ++--
  hw/core/qdev.c| 2 +-
  hw/core/sysbus.c  | 4 ++--
  hw/cpu/icc_bus.c  | 3 ++-
  hw/ide/qdev.c | 2 +-
  hw/misc/macio/cuda.c  | 4 ++--
  hw/pci/pci.c  | 2 +-
  hw/pci/pci_bridge.c   | 3 ++-
  hw/s390x/event-facility.c | 4 ++--
  hw/s390x/s390-virtio-bus.c| 4 ++--
  hw/s390x/virtio-ccw.c | 4 ++--
  hw/scsi/scsi-bus.c| 2 +-
  hw/usb/bus.c  | 2 +-
  hw/usb/dev-smartcard-reader.c | 3 ++-
  hw/virtio/virtio-mmio.c   | 2 +-
  hw/virtio/virtio-pci.c| 2 +-
  include/hw/qdev-core.h| 2 +-
  19 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 9c2fa88..8800dfe 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -44,7 +44,7 @@ void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus, 
size_t bus_size,
  hda_codec_response_func response,
  hda_codec_xfer_func xfer)
  {
-qbus_create_inplace(&bus->qbus, TYPE_HDA_BUS, dev, NULL);
+qbus_create_inplace(bus, bus_size, TYPE_HDA_BUS, dev, NULL);
  bus->response = response;
  bus->xfer = xfer;
  }
diff --git a/hw/char/ipack.c b/hw/char/ipack.c
index 5fb7073..b7e45be 100644
--- a/hw/char/ipack.c
+++ b/hw/char/ipack.c
@@ -29,7 +29,7 @@ void ipack_bus_new_inplace(IPackBus *bus, size_t bus_size,
 const char *name, uint8_t n_slots,
 qemu_irq_handler handler)
  {
-qbus_create_inplace(&bus->qbus, TYPE_IPACK_BUS, parent, name);
+qbus_create_inplace(bus, bus_size, TYPE_IPACK_BUS, parent, name);
  bus->n_slots = n_slots;
  bus->set_irq = handler;
  }
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index da417c7..d90fc2a 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -911,8 +911,8 @@ static int virtio_serial_device_init(VirtIODevice *vdev)
  sizeof(struct virtio_console_config));

  /* Spawn a new virtio-serial bus on which the ports will ride as devices 
*/
-qbus_create_inplace(&vser->bus.qbus, TYPE_VIRTIO_SERIAL_BUS, qdev,
-vdev->bus_name);
+qbus_create_inplace(&vser->bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS,
+qdev, vdev->bus_name);
  vser->bus.qbus.allow_hotplug = 1;
  vser->bus.vser = vser;
  QTAILQ_INIT(&vser->ports);
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 758de9f..81874da 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -470,7 +470,7 @@ static void bus_unparent(Object *obj)
  }
  }

-void qbus_create_inplace(void *bus, const char *typename,
+void qbus_create_inplace(void *bus, size_t size, const char *typename,
   DeviceState *parent, const char *name)
  {
  object_initialize(bus, typename);
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 9004d8c..b84cd4a 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -276,8 +276,8 @@ static void main_system_bus_create(void)
  /* assign main_system_bus before qbus_create_inplace()
   * in order to make "if (bus != sysbus_get_default())" work */
  main_system_bus = g_malloc0(system_bus_info.instance_size);
-qbus_create_inplace(main_system_bus, TYPE_SYSTEM_BUS, NULL,
-"main-system-bus");
+qbus_create_inplace(main_system_bus, system_bus_info.instance_size,
+TYPE_SYSTEM_BUS, NULL, "main-system-bus");
  OBJECT(main_system_bus)->free = g_free;
  object_property_add_child(container_get(qdev_get_machine(),
  "/unattached"),
diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 8748cc5..9a4ea7e 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -90,7 +90,8 @@ static void icc_bridge_init(Object *obj)
  ICCBridgeState *s = ICC_BRIGDE(obj);
  SysBusDevice *sb = SYS_BUS_DEVICE(obj);

-qbus_create_inplace(&s->icc_bus, TYPE_ICC_BUS, DEVICE(s), "icc");
+qbus_create_inplace(&s->icc_bus, sizeof(s->icc_bus), TYPE_ICC_BUS,
+DEVICE(s), "icc");

  /* Do not change order of registering regions,
   * APIC must be first registered region, board maps it by 0 index
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 8be76ab..18c4b7e 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -50,7 +50,7 @@ static const TypeInfo ide_bus_info = {
  void ide_bus_new(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
   int bus_id, int max_units)
  {
-qbus_

Re: [Qemu-devel] [PATCH V9 06/12] NUMA: Add Linux libnuma detection

[Andrew Jones]

- Original Message -
> On 08/23/2013 04:40 PM, Andrew Jones wrote:
> > 
> > 
> > - Original Message -
> >> Add detection of libnuma (mostly contained in the numactl package)
> >> to the configure script. Can be enabled or disabled on the command line,
> >> default is use if available.
> >>
> >> Signed-off-by: Andre Przywara 
> >> Signed-off-by: Wanlong Gao 
> > 
> > Is this patch still necessary? I thought that dropping the
> > numa_num_configured_nodes() calls from patch 8/12 got rid
> > of the need for this library. Maybe I missed other uses?
> 
> Yes, in 08/12 we also use mbind(), 

You don't need a whole library for mbind(), it's a syscall. See syscall(2).

> and in 09/12 we use max_numa_node().

Really? I didn't see it there. And anyway, that goes back to our discussion
about setting qemu's MAX_NODES to whatever we think qemu should support,
and then just checking that we don't blow that limit whenever reading
host node info, i.e.

maxnode = 0;
while (host_nodes[maxnode] && maxnode < MAX_NODES)
  node_read(&info[maxnode++]);

type of a thing.

And, if there's a place you really need to know the current online number
of host nodes, then, like I said earlier, you should just go to sysfs
yourself. libnuma:numa_max_node() returns an int that it only initializes
at library load time, so it's not going to adapt to onlining/offlining.

drew

> 
> Thanks,
> Wanlong Gao
> 
> > 
> > drew
> > 
> 
> 
> 

Re: [Qemu-devel] [PATCH] scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial

[Paolo Bonzini]

Il 23/08/2013 18:01, arm...@redhat.com ha scritto:
> From: Markus Armbruster 
> 
> scsi_bus_legacy_add_drive() creates either a scsi-disk or a
> scsi-generic device.  It sets property "serial" to argument serial
> unless null.  Crashes with scsi-generic, because it doesn't have such
> the property.
> 
> Only usb_msd_initfn_storage() passes non-null serial.  Reproducer:
> 
> $ qemu-system-x86_64 -nodefaults -display none -S -usb \
> -drive if=none,file=/dev/sg1,id=usb-drv0 \
> -device usb-storage,id=usb-msd0,drive=usb-drv0,serial=123
> qemu-system-x86_64: -device 
> usb-storage,id=usb-msd0,drive=usb-drv0,serial=123: Property '.serial' not 
> found
> Aborted (core dumped)
> 
> Fix by handling exactly like "removable": set the property only when
> it exists.
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Markus Armbruster 
> ---
>  hw/scsi/scsi-bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index fbf9173..8fe4f4c 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -224,7 +224,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, 
> BlockDriverState *bdrv,
>  if (object_property_find(OBJECT(dev), "removable", NULL)) {
>  qdev_prop_set_bit(dev, "removable", removable);
>  }
> -if (serial) {
> +if (serial && object_property_find(OBJECT(dev), "serial", NULL)) {
>  qdev_prop_set_string(dev, "serial", serial);
>  }
>  if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
> 

Applied to scsi-next branch, thanks.

Paolo

Re: [Qemu-devel] [PATCH 0/2] lsi53c895a: avoid integer undefined behaviours

[Paolo Bonzini]

Il 23/08/2013 18:16, Peter Maydell ha scritto:
> More patches to fix clang sanitizer warnings; in this case we
> can fix them and clean up the code a bit by replacing hand-coded
> operations with functions from the bitops header.
> 
> Peter Maydell (2):
>   hw/scsi/lsi53c895a: Use sextract32 for sign-extension
>   hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask
> 
>  hw/scsi/lsi53c895a.c |   19 ++-
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 

Applied both to scsi-next, thanks.

Paolo

Re: [Qemu-devel] [PATCH 15/16] qdev-monitor: Clean up qdev_device_add() variable naming

[Wenchao Xia]

于 2013-8-24 8:00, Andreas Färber 写道:

Avoid confusion between object and object class.

  between object class and device class?


Tidy DeviceClass variable while at it.

Signed-off-by: Andreas Färber 
---
  qdev-monitor.c | 22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 410cdcb..51bfec0 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -444,8 +444,8 @@ static BusState *qbus_find(const char *path)

  DeviceState *qdev_device_add(QemuOpts *opts)
  {
-ObjectClass *obj;
-DeviceClass *k;
+ObjectClass *oc;
+DeviceClass *dc;
  const char *driver, *path, *id;
  DeviceState *qdev;
  BusState *bus = NULL;
@@ -457,22 +457,22 @@ DeviceState *qdev_device_add(QemuOpts *opts)
  }

  /* find driver */
-obj = object_class_by_name(driver);
-if (!obj) {
+oc = object_class_by_name(driver);
+if (!oc) {
  const char *typename = find_typename_by_alias(driver);

  if (typename) {
  driver = typename;
-obj = object_class_by_name(driver);
+oc = object_class_by_name(driver);
  }
  }

-if (!obj) {
+if (!oc) {
  qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
  return NULL;
  }

-k = DEVICE_CLASS(obj);
+dc = DEVICE_CLASS(oc);

  /* find bus */
  path = qemu_opt_get(opts, "bus");
@@ -481,16 +481,16 @@ DeviceState *qdev_device_add(QemuOpts *opts)
  if (!bus) {
  return NULL;
  }
-if (!object_dynamic_cast(OBJECT(bus), k->bus_type)) {
+if (!object_dynamic_cast(OBJECT(bus), dc->bus_type)) {
  qerror_report(QERR_BAD_BUS_FOR_DEVICE,
driver, object_get_typename(OBJECT(bus)));
  return NULL;
  }
-} else if (k->bus_type != NULL) {
-bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_type);
+} else if (dc->bus_type != NULL) {
+bus = qbus_find_recursive(sysbus_get_default(), NULL, dc->bus_type);
  if (!bus) {
  qerror_report(QERR_NO_BUS_FOR_DEVICE,
-  k->bus_type, driver);
+  dc->bus_type, driver);
  return NULL;
  }
  }




--
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH 10/16] virtio-ccw: Pass size to virtio_ccw_bus_new()

[Cornelia Huck]

On Sat, 24 Aug 2013 02:00:30 +0200
Andreas Färber  wrote:

> To be passed to qbus_create_inplace().
> 
> Signed-off-by: Andreas Färber 
> ---
>  hw/s390x/virtio-ccw.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 

Acked-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH 1/2] hpet: entitle more irq pins for hpet

[Paolo Bonzini]

Il 26/08/2013 04:53, liu ping fan ha scritto:
> On Sun, Aug 25, 2013 at 2:45 PM, Paolo Bonzini  wrote:
>> Il 25/08/2013 04:16, Liu Ping Fan ha scritto:
>>> On PC, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23 of
>>> ioapic can be dynamically assigned to hpet as guest chooses.
>>>
>>> Signed-off-by: Liu Ping Fan 
>>> ---
>>>  hw/timer/hpet.c | 6 --
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
>>> index 648b383..cd95d39 100644
>>> --- a/hw/timer/hpet.c
>>> +++ b/hw/timer/hpet.c
>>> @@ -41,6 +41,8 @@
>>>  #endif
>>>
>>>  #define HPET_MSI_SUPPORT0
>>> +/* Hpet can use non-legacy IRQ16~23, and an IRQ2 ,IRQ8 */
>>> +#define HPET_TN_INT_CAP (0xff0104ULL << 32)
>>>
>>>  #define TYPE_HPET "hpet"
>>>  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
>>> @@ -653,8 +655,8 @@ static void hpet_reset(DeviceState *d)
>>>  if (s->flags & (1 << HPET_MSI_SUPPORT)) {
>>>  timer->config |= HPET_TN_FSB_CAP;
>>>  }
>>> -/* advertise availability of ioapic inti2 */
>>> -timer->config |=  0x0004ULL << 32;
>>> +/* advertise availability of ioapic int */
>>> +timer->config |=  HPET_TN_INT_CAP;
>>>  timer->period = 0ULL;
>>>  timer->wrap_flag = 0;
>>>  }
>>>
>>
>> These high 32-bits of timer->config need to be a property of the HPET
>> devices, so that the old value (4) is used when running with old machine
> 
> Sorry, but I had included old value (4) in macro HPET_TN_INT_CAP.

No, *only* GSI 2 must be available on old machine types (pc-1.6 and older).

Paolo

Re: [Qemu-devel] [PATCH] kvm: warn if num cpus is greater than num recommended

[Andrew Jones]

- Original Message -
> Il 23/08/2013 13:33, Andrew Jones ha scritto:
> > Does smp_cpus map to the current
> > number of cpus, or to the number of possible cpus? If it maps to the number
> > of possible cpus, then this is the right place. If the former, then I guess
> > it'll take more thought. I'ved added Igor (still on vacation) to this
> > reply,
> > but regardless I vote we worry about hot-plug limit checking in different
> > patch.
> 
> smp_cpus is the initial number, max_cpus is the number of possible cpus.
> 

Yeah, I noticed that this issue is at least partially addressed already with
my v2, which incorporates Marcelo's check against the number of hotpluggable
cpus (max_cpus).

drew

Re: [Qemu-devel] [PATCH 06/16] scsi: Pass size to scsi_bus_new()

[Paolo Bonzini]

Il 24/08/2013 02:00, Andreas Färber ha scritto:
> To be passed to qbus_create_inplace().
> 
> Use DEVICE() casts instead of direct parent field access.
> 
> Signed-off-by: Andreas Färber 
> ---
>  hw/scsi/esp-pci.c  | 2 +-
>  hw/scsi/esp.c  | 2 +-
>  hw/scsi/lsi53c895a.c   | 2 +-
>  hw/scsi/megasas.c  | 3 ++-
>  hw/scsi/scsi-bus.c | 4 ++--
>  hw/scsi/spapr_vscsi.c  | 3 ++-
>  hw/scsi/virtio-scsi.c  | 3 ++-
>  hw/scsi/vmw_pvscsi.c   | 3 ++-
>  hw/usb/dev-storage.c   | 6 --
>  hw/usb/dev-uas.c   | 3 ++-
>  include/hw/scsi/scsi.h | 4 ++--
>  11 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index d7ec173..99bf8ec 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -363,7 +363,7 @@ static int esp_pci_scsi_init(PCIDevice *dev)
>  pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->io);
>  s->irq = dev->irq[0];
>  
> -scsi_bus_new(&s->bus, d, &esp_pci_scsi_info, NULL);
> +scsi_bus_new(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info, NULL);
>  if (!d->hotplugged) {
>  scsi_bus_legacy_handle_cmdline(&s->bus, &err);
>  if (err != NULL) {
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 101e957..2d150bf 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -688,7 +688,7 @@ static void sysbus_esp_realize(DeviceState *dev, Error 
> **errp)
>  
>  qdev_init_gpio_in(dev, sysbus_esp_gpio_demux, 2);
>  
> -scsi_bus_new(&s->bus, dev, &esp_scsi_info, NULL);
> +scsi_bus_new(&s->bus, sizeof(s->bus), dev, &esp_scsi_info, NULL);
>  scsi_bus_legacy_handle_cmdline(&s->bus, &err);
>  if (err != NULL) {
>  error_propagate(errp, err);
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index 611f2aa..0c36842 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -2117,7 +2117,7 @@ static int lsi_scsi_init(PCIDevice *dev)
>  pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->ram_io);
>  QTAILQ_INIT(&s->queue);
>  
> -scsi_bus_new(&s->bus, d, &lsi_scsi_info, NULL);
> +scsi_bus_new(&s->bus, sizeof(s->bus), d, &lsi_scsi_info, NULL);
>  if (!d->hotplugged) {
>  scsi_bus_legacy_handle_cmdline(&s->bus, &err);
>  if (err != NULL) {
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index a6d5285..09b51b3 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -2171,7 +2171,8 @@ static int megasas_scsi_init(PCIDevice *dev)
>  s->frames[i].state = s;
>  }
>  
> -scsi_bus_new(&s->bus, DEVICE(dev), &megasas_scsi_info, NULL);
> +scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
> + &megasas_scsi_info, NULL);
>  if (!d->hotplugged) {
>  scsi_bus_legacy_handle_cmdline(&s->bus, &err);
>  if (err != NULL) {
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index fbf9173..968bf23 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -72,8 +72,8 @@ static void scsi_device_unit_attention_reported(SCSIDevice 
> *s)
>  }
>  
>  /* Create a scsi bus, and attach devices to it.  */
> -void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info,
> -  const char *bus_name)
> +void scsi_bus_new(SCSIBus *bus, size_t bus_size, DeviceState *host,
> +  const SCSIBusInfo *info, const char *bus_name)
>  {
>  qbus_create_inplace(&bus->qbus, TYPE_SCSI_BUS, host, bus_name);
>  bus->busnr = next_scsi_bus++;
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> index e9090e5..b2fcd4b 100644
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
> @@ -1020,7 +1020,8 @@ static int spapr_vscsi_init(VIOsPAPRDevice *dev)
>  
>  dev->crq.SendFunc = vscsi_do_crq;
>  
> -scsi_bus_new(&s->bus, &dev->qdev, &vscsi_scsi_info, NULL);
> +scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
> + &vscsi_scsi_info, NULL);
>  if (!dev->qdev.hotplugged) {
>  scsi_bus_legacy_handle_cmdline(&s->bus, &err);
>  if (err != NULL) {
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 05da56b..3bd690d 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -627,7 +627,8 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
>  return ret;
>  }
>  
> -scsi_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info, vdev->bus_name);
> +scsi_bus_new(&s->bus, sizeof(s->bus), qdev,
> + &virtio_scsi_scsi_info, vdev->bus_name);
>  
>  if (!qdev->hotplugged) {
>  scsi_bus_legacy_handle_cmdline(&s->bus, &err);
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index d42b359..819d671 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -1088,7 +1088,8 @@ pvscsi_init(PCIDevice *pci_dev)
>  return -ENOMEM;
>  }
>  
> -scsi_bus_new(&s->bus, &pci_dev->qdev, &pvscsi_scsi_info, NULL);
> +scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(pci_dev),
> +   

Re: [Qemu-devel] [PATCH 12/16] qdev: Pass size to qbus_create_inplace()

[Cornelia Huck]

On Sat, 24 Aug 2013 02:00:32 +0200
Andreas Färber  wrote:

> To be passed to object_initialize().
> 
> Since commit 39355c3826f5d9a2eb1ce3dc9b4cdd68893769d6 the argument is
> void*, so drop some superfluous (BusState *) casts or direct parent
> field usages.
> 
> Signed-off-by: Andreas Färber 
> ---
>  hw/audio/intel-hda.c  | 2 +-
>  hw/char/ipack.c   | 2 +-
>  hw/char/virtio-serial-bus.c   | 4 ++--
>  hw/core/qdev.c| 2 +-
>  hw/core/sysbus.c  | 4 ++--
>  hw/cpu/icc_bus.c  | 3 ++-
>  hw/ide/qdev.c | 2 +-
>  hw/misc/macio/cuda.c  | 4 ++--
>  hw/pci/pci.c  | 2 +-
>  hw/pci/pci_bridge.c   | 3 ++-
>  hw/s390x/event-facility.c | 4 ++--
>  hw/s390x/s390-virtio-bus.c| 4 ++--
>  hw/s390x/virtio-ccw.c | 4 ++--
>  hw/scsi/scsi-bus.c| 2 +-
>  hw/usb/bus.c  | 2 +-
>  hw/usb/dev-smartcard-reader.c | 3 ++-
>  hw/virtio/virtio-mmio.c   | 2 +-
>  hw/virtio/virtio-pci.c| 2 +-
>  include/hw/qdev-core.h| 2 +-
>  19 files changed, 28 insertions(+), 25 deletions(-)
> 

> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 579adbc..e95b831 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -699,8 +699,8 @@ static void virtio_s390_bus_new(VirtioBusState *bus, 
> size_t bus_size,
>  BusState *qbus;
>  char virtio_bus_name[] = "virtio-bus";
> 
> -qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_S390_BUS, qdev,
> -virtio_bus_name);
> +qbus_create_inplace((BusState *)bus, bus_size, TYPE_VIRTIO_S390_BUS,
> +qdev, virtio_bus_name);
>  qbus = BUS(bus);
>  qbus->allow_hotplug = 1;
>  }
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 36cbf42..cf7075e 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -1297,8 +1297,8 @@ static void virtio_ccw_bus_new(VirtioBusState *bus, 
> size_t bus_size,
>  BusState *qbus;
>  char virtio_bus_name[] = "virtio-bus";
> 
> -qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_CCW_BUS, qdev,
> -virtio_bus_name);
> +qbus_create_inplace((BusState *)bus, bus_size, TYPE_VIRTIO_CCW_BUS,
> +qdev, virtio_bus_name);
>  qbus = BUS(bus);
>  qbus->allow_hotplug = 1;
>  }

You'll probably want to drop the superflous casts here as well :)

Re: [Qemu-devel] [PATCHv4] add qemu-img convert -C option (skip target volume creation)

[Stefan Hajnoczi]

On Thu, Aug 22, 2013 at 08:27:12PM +0100, Alex Bligh wrote:
> 
> On 22 Aug 2013, at 12:45, Stefan Hajnoczi wrote:
> The behaviour of the current code appears to be as follows
> (Alexandre's code not mine):
> 
> a) if the target volume size > the converted volume size, convert
>leaving the remaining data on the target volume as is. This
>is I believe useful, as on (e.g.) rbd, the target volume size
>may be larger than required due to rounding requirements.

Seems ok.

> b) if the target volume size < the converted volume size, convert
>truncates it but does not error. I'm torn between whether this
>continue to do exactly as it asked, attempt to expand the
>volume, or error. These all seem reasonably easy (the expand
>option presumably being a call to bdrv_truncate).

Silently truncating can be a problem, e.g. the user deletes the original
file after conversion completes and later discovers not all data was
copied.

I think we should fail here.

Stefan

Re: [Qemu-devel] [PATCHv4] add qemu-img convert -C option (skip target volume creation)

[Stefan Hajnoczi]

On Thu, Aug 22, 2013 at 07:03:41PM +0100, Alex Bligh wrote:
> On 22 Aug 2013, at 13:35, Paolo Bonzini wrote:
> 
> > Also, this is the same as some HMP commands' "-n" option (live
> > snapshots, mirroring, backup) so I suggest to use that name.
> 
> You mean -n instead of -C? Sure I can do that, but is that
> something you feel strongly about? I am aware there are a number
> of people who have been using the patch with -C for some time.

Sharing the -n flag with other commands is better than introducing a new
flag.

Stefan

Re: [Qemu-devel] [RFC PATCH 3/4] qmp: Add "snapshot=" option to nbd-server-add

[Fam Zheng]

On Thu, 08/22 11:22, Paolo Bonzini wrote:
> Il 22/08/2013 10:53, Fam Zheng ha scritto:
> >> > This is handy but only works if the QEMU process has permission to
> >> > create temporary files.
> >> > 
> > Yes, this is a shortcut, it has this limitation, but the good side is
> > it's easy to get and no other dependency.
> > 
> > To avoid creating file, we'll need blockdev-add to override backing_hd,
> > and blockdev-backup to use an existing BDS as target, then we simply use
> > current nbd-server-add to export it.
> > 
> > This series is still RFC, with above said, we still need to decide which
> > is the way we (QEMU and libvirt) want for 1.7.  any thoughts?
> 
> I think this was the initial design, but Kevin already said he didn't
> like the idea.
> 
> (Also, if you do this you have to add nbd-server-add to qmp-transaction,
> to atomically start fleecing of multiple devices).
> 
Kevin,

The downside of this is unresolvable (create file in QEMU), so I am
giving up this and back to blockdev-add & blockdev-backup interface.

So I guess I'm depending on your blockdev-add command patches, do you
have a solution for overriding backing_hd there yet? If yes, do you have
a working branch that I can rebase my blockdev-backup onto?

Thanks.
Fam

Re: [Qemu-devel] [PATCH 13/16] qom: Pass available size to object_initialize()

[Cornelia Huck]

On Sat, 24 Aug 2013 02:00:33 +0200
Andreas Färber  wrote:

> This is to avoid objects initializing beyond allocated memory.
> 
> Inspired-by: Peter Maydell 
> Signed-off-by: Andreas Färber 
> ---
>  hw/core/qdev.c |  2 +-
>  hw/dma/xilinx_axidma.c |  6 --
>  hw/intc/xics.c |  2 +-
>  hw/misc/macio/macio.c  | 13 +++--
>  hw/net/xilinx_axienet.c|  6 --
>  hw/pci-host/prep.c |  2 +-
>  hw/pci-host/q35.c  |  2 +-
>  hw/pci-host/versatile.c|  2 +-
>  hw/s390x/s390-virtio-bus.c | 12 ++--
>  hw/s390x/virtio-ccw.c  | 14 +++---
>  hw/virtio/virtio-pci.c | 16 
>  include/qom/object.h   |  6 --
>  qom/object.c   |  9 +
>  13 files changed, 50 insertions(+), 42 deletions(-)
> 

Acked-by: Cornelia Huck  [virtio-ccw]

Re: [Qemu-devel] [PATCH 13/16] qom: Pass available size to object_initialize()

[Wenchao Xia]

于 2013-8-24 8:00, Andreas Färber 写道:

This is to avoid objects initializing beyond allocated memory.

Inspired-by: Peter Maydell 
Signed-off-by: Andreas Färber 
---
  hw/core/qdev.c |  2 +-
  hw/dma/xilinx_axidma.c |  6 --
  hw/intc/xics.c |  2 +-
  hw/misc/macio/macio.c  | 13 +++--
  hw/net/xilinx_axienet.c|  6 --
  hw/pci-host/prep.c |  2 +-
  hw/pci-host/q35.c  |  2 +-
  hw/pci-host/versatile.c|  2 +-
  hw/s390x/s390-virtio-bus.c | 12 ++--
  hw/s390x/virtio-ccw.c  | 14 +++---
  hw/virtio/virtio-pci.c | 16 
  include/qom/object.h   |  6 --
  qom/object.c   |  9 +
  13 files changed, 50 insertions(+), 42 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 81874da..533f6dd 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -473,7 +473,7 @@ static void bus_unparent(Object *obj)
  void qbus_create_inplace(void *bus, size_t size, const char *typename,
   DeviceState *parent, const char *name)
  {
-object_initialize(bus, typename);
+object_initialize(bus, size, typename);
  qbus_realize(bus, parent, name);
  }

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index a48e3ba..1e6a88d 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -578,8 +578,10 @@ static void xilinx_axidma_init(Object *obj)
   (Object **) &s->tx_control_dev, &errp);
  assert_no_error(errp);

-object_initialize(&s->rx_data_dev, TYPE_XILINX_AXI_DMA_DATA_STREAM);
-object_initialize(&s->rx_control_dev, TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
+object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
+  TYPE_XILINX_AXI_DMA_DATA_STREAM);
+object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
+  TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
  object_property_add_child(OBJECT(s), "axistream-connected-target",
(Object *)&s->rx_data_dev, &errp);
  assert_no_error(errp);
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 6b3c071..b96b041 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -650,7 +650,7 @@ static void xics_realize(DeviceState *dev, Error **errp)
  icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState));
  for (i = 0; i < icp->nr_servers; i++) {
  char buffer[32];
-object_initialize(&icp->ss[i], TYPE_ICP);
+object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_ICP);
  snprintf(buffer, sizeof(buffer), "icp[%d]", i);
  object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]), 
NULL);
  qdev_init_nofail(DEVICE(&icp->ss[i]));
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index c0d0bf7..1bee3d8 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -202,11 +202,12 @@ static int macio_oldworld_initfn(PCIDevice *d)
  return 0;
  }

-static void macio_init_ide(MacIOState *s, MACIOIDEState *ide, int index)
+static void macio_init_ide(MacIOState *s, MACIOIDEState *ide, size_t ide_size,
+   int index)
  {
  gchar *name;

-object_initialize(ide, TYPE_MACIO_IDE);
+object_initialize(ide, ide_size, TYPE_MACIO_IDE);
  qdev_set_parent_bus(DEVICE(ide), sysbus_get_default());
  memory_region_add_subregion(&s->bar, 0x1f000 + ((index + 1) * 0x1000),
  &ide->mem);
@@ -224,13 +225,13 @@ static void macio_oldworld_init(Object *obj)

  qdev_init_gpio_out(DEVICE(obj), os->irqs, ARRAY_SIZE(os->irqs));

-object_initialize(&os->nvram, TYPE_MACIO_NVRAM);
+object_initialize(&os->nvram, sizeof(os->nvram), TYPE_MACIO_NVRAM);
  dev = DEVICE(&os->nvram);
  qdev_prop_set_uint32(dev, "size", 0x2000);
  qdev_prop_set_uint32(dev, "it_shift", 4);

  for (i = 0; i < 2; i++) {
-macio_init_ide(s, &os->ide[i], i);
+macio_init_ide(s, &os->ide[i], sizeof(os->ide[i]), i);
  }
  }

@@ -310,7 +311,7 @@ static void macio_newworld_init(Object *obj)
  qdev_init_gpio_out(DEVICE(obj), ns->irqs, ARRAY_SIZE(ns->irqs));

  for (i = 0; i < 2; i++) {
-macio_init_ide(s, &ns->ide[i], i);
+macio_init_ide(s, &ns->ide[i], sizeof(ns->ide[i]), i);
  }
  }

@@ -321,7 +322,7 @@ static void macio_instance_init(Object *obj)

  memory_region_init(&s->bar, NULL, "macio", 0x8);

-object_initialize(&s->cuda, TYPE_CUDA);
+object_initialize(&s->cuda, sizeof(s->cuda), TYPE_CUDA);
  qdev_set_parent_bus(DEVICE(&s->cuda), sysbus_get_default());
  object_property_add_child(obj, "cuda", OBJECT(&s->cuda), NULL);

diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index f173429..3eb7715 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -990,8 +990,10 @@ static void xilinx_enet_init(Object *obj)
   (Object **) &s->tx_control_dev, &errp);
  assert_no_er

Re: [Qemu-devel] [PATCH 00/16] qom: Assert sufficient object instance size

[Wenchao Xia]

  I have not looked deep into QOM, so only reviewed the code in this
series, and have some minor comments for patch 12, 13, and 15. For
other part,
  Reviewed-by: Wenchao Xia 


Hello,

Peter remarked that object_initialize() on a GICState struct field would not
check whether the TypeInfo::instance_size for its typename argument exceeds
the length of the field we're initializing. This series therefore updates
all callers to explicitly pass the size available for the object.

While we don't have that many object_initialize() users yet, quite a few
devices are using qbus_create_inplace() or bus-specific functions based on it.
Still I consider this the safest solution in pushing the responsability for
supplying the length to the caller and thereby checking not only field type
lengths but also allocation lengths.

The series goes on to showcase an object_initialize() usage where we don't
know the instance_size in advance and need a QOM API to obtain it.

Based on Peter's OBJECT() elimination patch, which I have queued on qom-next.
This series conflicts with my *mpcore, virtio and ipack series among others.

Regards,
Andreas

Cc: Peter Maydell 
Cc: Anthony Liguori 

Andreas Färber (16):
   qom: Fix object_initialize_with_type() argument name in documentation
   intel-hda: Pass size to hda_codec_bus_init()
   ipack: Pass size to ipack_bus_new_inplace()
   ide: Pass size to ide_bus_new()
   pci: Pass size to pci_bus_new_inplace()
   scsi: Pass size to scsi_bus_new()
   usb: Pass size to usb_bus_new()
   virtio-pci: Pass size to virtio_pci_bus_new()
   s390-virtio-bus: Pass size to virtio_s390_bus_new()
   virtio-ccw: Pass size to virtio_ccw_bus_new()
   virtio-mmio: Pass size to virtio_mmio_bus_new()
   qdev: Pass size to qbus_create_inplace()
   qom: Pass available size to object_initialize()
   qom: Introduce type_get_instance_size()
   qdev-monitor: Clean up qdev_device_add() variable naming
   qdev-monitor: Avoid aborting on out-of-memory in qdev_device_add()

  hw/audio/intel-hda.c  |  6 +++---
  hw/audio/intel-hda.h  |  2 +-
  hw/char/ipack.c   |  5 +++--
  hw/char/ipack.h   |  3 ++-
  hw/char/tpci200.c |  2 +-
  hw/char/virtio-serial-bus.c   |  4 ++--
  hw/core/qdev.c|  4 ++--
  hw/core/sysbus.c  |  4 ++--
  hw/cpu/icc_bus.c  |  3 ++-
  hw/dma/xilinx_axidma.c|  6 --
  hw/ide/ahci.c |  2 +-
  hw/ide/cmd646.c   |  2 +-
  hw/ide/internal.h |  3 ++-
  hw/ide/isa.c  |  2 +-
  hw/ide/macio.c|  2 +-
  hw/ide/mmio.c |  2 +-
  hw/ide/piix.c |  2 +-
  hw/ide/qdev.c |  5 +++--
  hw/ide/via.c  |  2 +-
  hw/intc/xics.c|  2 +-
  hw/misc/macio/cuda.c  |  4 ++--
  hw/misc/macio/macio.c | 13 +++--
  hw/net/xilinx_axienet.c   |  6 --
  hw/pci-host/prep.c|  4 ++--
  hw/pci-host/q35.c |  2 +-
  hw/pci-host/versatile.c   |  4 ++--
  hw/pci/pci.c  |  4 ++--
  hw/pci/pci_bridge.c   |  3 ++-
  hw/s390x/event-facility.c |  4 ++--
  hw/s390x/s390-virtio-bus.c| 24 +---
  hw/s390x/virtio-ccw.c | 26 ++
  hw/scsi/esp-pci.c |  2 +-
  hw/scsi/esp.c |  2 +-
  hw/scsi/lsi53c895a.c  |  2 +-
  hw/scsi/megasas.c |  3 ++-
  hw/scsi/scsi-bus.c|  6 +++---
  hw/scsi/spapr_vscsi.c |  3 ++-
  hw/scsi/virtio-scsi.c |  3 ++-
  hw/scsi/vmw_pvscsi.c  |  3 ++-
  hw/usb/bus.c  |  5 +++--
  hw/usb/dev-smartcard-reader.c |  3 ++-
  hw/usb/dev-storage.c  |  6 --
  hw/usb/dev-uas.c  |  3 ++-
  hw/usb/hcd-ehci.c |  2 +-
  hw/usb/hcd-musb.c |  2 +-
  hw/usb/hcd-ohci.c |  2 +-
  hw/usb/hcd-uhci.c |  2 +-
  hw/usb/hcd-xhci.c |  2 +-
  hw/virtio/virtio-mmio.c   | 10 ++
  hw/virtio/virtio-pci.c| 26 ++
  include/hw/pci/pci.h  |  2 +-
  include/hw/qdev-core.h|  2 +-
  include/hw/scsi/scsi.h|  4 ++--
  include/hw/usb.h  |  3 ++-
  include/qom/object.h  | 16 +---
  qdev-monitor.c| 30 ++
  qom/object.c  | 16 
  57 files changed, 185 insertions(+), 132 deletions(-)




--
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH V9 06/12] NUMA: Add Linux libnuma detection

[Wanlong Gao]

On 08/26/2013 03:46 PM, Andrew Jones wrote:
>>> Is this patch still necessary? I thought that dropping the
>>> > > numa_num_configured_nodes() calls from patch 8/12 got rid
>>> > > of the need for this library. Maybe I missed other uses?
>> > 
>> > Yes, in 08/12 we also use mbind(), 
> You don't need a whole library for mbind(), it's a syscall. See syscall(2).
> 
>> > and in 09/12 we use max_numa_node().
> Really? I didn't see it there. And anyway, that goes back to our discussion
> about setting qemu's MAX_NODES to whatever we think qemu should support,
> and then just checking that we don't blow that limit whenever reading
> host node info, i.e.
> 
> maxnode = 0;
> while (host_nodes[maxnode] && maxnode < MAX_NODES)
>   node_read(&info[maxnode++]);
> 
> type of a thing.
> 
> And, if there's a place you really need to know the current online number
> of host nodes, then, like I said earlier, you should just go to sysfs
> yourself. libnuma:numa_max_node() returns an int that it only initializes
> at library load time, so it's not going to adapt to onlining/offlining.

OK, thank you.
Then I should define MPOL_* macros in QEMU and use mbind(2) syscall directly, 
right?

Thanks,
Wanlong Gao

> 
> drew
> 

Re: [Qemu-devel] [PATCH] vmxnet3: Eliminate __packed redefined warning

[Stefan Hajnoczi]

On Fri, Aug 23, 2013 at 12:28:25PM -0400, Brad Smith wrote:
> This eliminates a warning about __packed being redefined as exposed by the
> vmxnet3 code. __packed is not used anywhere in the vmxnet3 code.
> 
>   CChw/net/vmxnet3.o
> In file included from hw/net/vmxnet3.c:29:
> hw/net/vmxnet3.h:37:1: warning: "__packed" redefined
> In file included from /usr/include/stdlib.h:38,
>  from 
> /buildbot-qemu/default_openbsd_current/build/include/qemu-common.h:26,
>  from 
> /buildbot-qemu/default_openbsd_current/build/include/hw/hw.h:5,
>  from hw/net/vmxnet3.c:18:
> /usr/include/sys/cdefs.h:209:1: warning: this is the location of the previous 
> definition
> 
> Signed-off-by: Brad Smith 

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

Re: [Qemu-devel] [PATCH resend 3/3] arch_init: right return for ram_save_iterate

[Paolo Bonzini]

Il 23/08/2013 15:30, Lei Li ha scritto:
> -if (ret < 0) {
> -bytes_transferred += total_sent;
> -return ret;
> -}
> -
>  qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>  total_sent += 8;
>  bytes_transferred += total_sent;
>  
> -return total_sent;
> +return qemu_file_get_error(f);

No, this will never make ram_save_iterate (and thus
qemu_savevm_state_iterate) return a positive, non-zero value.  Thus:

ret = qemu_file_get_error(f);
if (ret < 0) {
return ret;
}
return total_sent;

If you look at the code, you can see that it never returns zero.
Probably it should do something like

bytes_transferred += total_sent;

/* Do not count these 8 bytes into total_sent, so that we can
 * return 0 if no page had been dirtied.
 */
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
bytes_transferred += 8;

and then proceed as above with "ret = qemu_file_get_error(f)".

Paolo

Re: [Qemu-devel] [PATCH 1/2] hpet: entitle more irq pins for hpet

[liu ping fan]

On Mon, Aug 26, 2013 at 3:59 PM, Paolo Bonzini  wrote:
> Il 26/08/2013 04:53, liu ping fan ha scritto:
>> On Sun, Aug 25, 2013 at 2:45 PM, Paolo Bonzini  wrote:
>>> Il 25/08/2013 04:16, Liu Ping Fan ha scritto:
 On PC, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23 of
 ioapic can be dynamically assigned to hpet as guest chooses.

 Signed-off-by: Liu Ping Fan 
 ---
  hw/timer/hpet.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
 index 648b383..cd95d39 100644
 --- a/hw/timer/hpet.c
 +++ b/hw/timer/hpet.c
 @@ -41,6 +41,8 @@
  #endif

  #define HPET_MSI_SUPPORT0
 +/* Hpet can use non-legacy IRQ16~23, and an IRQ2 ,IRQ8 */
 +#define HPET_TN_INT_CAP (0xff0104ULL << 32)

  #define TYPE_HPET "hpet"
  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
 @@ -653,8 +655,8 @@ static void hpet_reset(DeviceState *d)
  if (s->flags & (1 << HPET_MSI_SUPPORT)) {
  timer->config |= HPET_TN_FSB_CAP;
  }
 -/* advertise availability of ioapic inti2 */
 -timer->config |=  0x0004ULL << 32;
 +/* advertise availability of ioapic int */
 +timer->config |=  HPET_TN_INT_CAP;
  timer->period = 0ULL;
  timer->wrap_flag = 0;
  }

>>>
>>> These high 32-bits of timer->config need to be a property of the HPET
>>> devices, so that the old value (4) is used when running with old machine
>>
>> Sorry, but I had included old value (4) in macro HPET_TN_INT_CAP.
>
> No, *only* GSI 2 must be available on old machine types (pc-1.6 and older).
>
Oh, got your meaning, will fix.

Thx,
Pingfan

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Alexander Graf]

Am 26.08.2013 um 08:22 schrieb Benjamin Herrenschmidt 
:

> On Mon, 2013-08-26 at 06:44 +0100, Alexander Graf wrote:
>>> +cap.flags = 0;
>>> +cap.migration.ecl = 0;
>>> +cap.reserve.type = 0;
>>> +cap.migration.common.server_support = 0;
>>> +cap.reserve.common.server_support = 0;
>> 
>> My question stands unanswered. Is this just memset(0)?
> 
> If it is, then why bother reading it in the first place ? :-)

Because this is a revision that incorporates other feedback from that same 
review, so I'm quite sure he just missed it :)

> 
> Note that even if it is, I'd rather keep it that way as we are
> eventually going to populate some stuff and so we have just the
> placeholders to do it.

Do we ever need to preserve random fields in that struct? Or would it be 
clearer to just set the whole thing to 0 and then go from there?

It's definitely a case-by-case decision which way is better, but setting all 
fields by hand gives you potential for missing to set one.


Alex

> 
> Cheers,
> Ben.
> 
> 

[Qemu-devel] [Bug 1213797] Re: Guest hang after live migration

[chao zhou]

The bug has been fixed by the following commit:

commit df67696e97d3edd0cb1683bf2eb3b3236bd9a5ed
Author: Liu Jinsong 
Date: Mon Aug 19 09:33:30 2013 +0800

kvm: x86: fix setting IA32_FEATURE_CONTROL with nested VMX disabled

This patch is to fix the bug https://bugs.launchpad.net/qemu-
kvm/+bug/120762

IA32_FEATURE_CONTROL is pointless if not expose VMX or SMX bits to
cpuid.1.ecx of vcpu. Current qemu-kvm will error return when kvm_put_msrs
or kvm_get_msrs.

Signed-off-by: Liu Jinsong 
Signed-off-by: Paolo Bonzini 

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

Title:
  Guest hang after live migration

Status in QEMU:
  New

Bug description:
  Environment:
  
  Host OS (ia32/ia32e/IA64):ia32e
  Guest OS (ia32/ia32e/IA64):ia32e
  Guest OS Type (Linux/Windows):Linux
  kvm.git Commit:205befd9a5c701b56f569434045821f413f08f6d
  qemu-kvm uq/master Commit:ca916d3729564d0eb3c2374a96903f7e8aced8a7
  Host Kernel Version:3.11.0-rc1
  Hardware:Romley_EP, Ivytown_EP

  Bug detailed description:
  --
  after guest live migration, the guest will hang, and ping IP fail

  note:
  1. after guest save/restore, the guest will hang, and ping IP fail
  2. This should be a qemu-kvm bug.
  kvm  + qemu-kvm   =  result
  205befd9 + ca916d37   =  bad
  205befd9 + f03d07d4   =  good

  
  Reproduce steps:
  
  1.Start a TCP daemon for migration
  qemu-system-x86_64 -enable-kvm -m 2G -smp 2 -net nic,macaddr=00:12:34:56:13:45
  -net tap,script=/etc/kvm/qemu-ifup rhel6u2.qcow -incoming tcp:localhost:
  2. create guest
  qemu-system-x86_64 -enable-kvm -m 2G -smp 2 -net nic,macaddr=00:12:34:56:13:45
  -net tap,script=/etc/kvm/qemu-ifup rhel6u2.qcow 
  3. migrate tcp:localhost:

  Current result:
  
  guest hang

  Expected result:
  
  after live/migration or save/restore, the guest works fine

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

Re: [Qemu-devel] [PATCH V9 06/12] NUMA: Add Linux libnuma detection

[Andrew Jones]

- Original Message -
> On 08/26/2013 03:46 PM, Andrew Jones wrote:
> >>> Is this patch still necessary? I thought that dropping the
> >>> > > numa_num_configured_nodes() calls from patch 8/12 got rid
> >>> > > of the need for this library. Maybe I missed other uses?
> >> > 
> >> > Yes, in 08/12 we also use mbind(),
> > You don't need a whole library for mbind(), it's a syscall. See syscall(2).
> > 
> >> > and in 09/12 we use max_numa_node().
> > Really? I didn't see it there. And anyway, that goes back to our discussion
> > about setting qemu's MAX_NODES to whatever we think qemu should support,
> > and then just checking that we don't blow that limit whenever reading
> > host node info, i.e.
> > 
> > maxnode = 0;
> > while (host_nodes[maxnode] && maxnode < MAX_NODES)
> >   node_read(&info[maxnode++]);
> > 
> > type of a thing.
> > 
> > And, if there's a place you really need to know the current online number
> > of host nodes, then, like I said earlier, you should just go to sysfs
> > yourself. libnuma:numa_max_node() returns an int that it only initializes
> > at library load time, so it's not going to adapt to onlining/offlining.
> 
> OK, thank you.
> Then I should define MPOL_* macros in QEMU and use mbind(2) syscall directly,
> right?

Hmm, yeah, that's too bad that numaif.h is part of libnuma, and not a more
general lib. Whether or not we want to redefine those symbols within
qemu, in order to avoid the dependency on installing numactl-devel, isn't
something I can answer. That's a better question for Anthony. Anthony? Paolo,
any opinions? Maybe we should pick up uapi/linux/mempolicy.h with the
linux-header synch script?

thanks,
drew

> 
> Thanks,
> Wanlong Gao
> 
> > 
> > drew
> > 
> 
> 

Re: [Qemu-devel] [PATCH 2/9] bswap.h: Remove cpu_to_le32wu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:30PM +0100, Peter Maydell wrote:
> Replace the legacy cpu_to_le32wu() with stl_le_p().
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/net/ne2000.c  |2 +-
>  include/hw/pci/pci.h |2 +-
>  include/qemu/bswap.h |5 -
>  3 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH 4/9] bswap.h: Remove le32_to_cpupu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:32PM +0100, Peter Maydell wrote:
> Replace the legacy le32_to_cpupu() with ldl_le_p().
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/net/ne2000.c  |2 +-
>  include/hw/pci/pci.h |2 +-
>  include/qemu/bswap.h |5 -
>  3 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH 5/9] bswap.h: Remove be32_to_cpupu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:33PM +0100, Peter Maydell wrote:
> Replace the legacy be32_to_cpupu() with ldl_be_p().
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/net/e1000.c   |2 +-
>  include/qemu/bswap.h |5 -
>  2 files changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Nikunj A Dadhania]

Benjamin Herrenschmidt  writes:

> On Mon, 2013-08-26 at 10:02 +0530, Nikunj A Dadhania wrote:
>
>> 
>> From: Nikunj A Dadhania 
>> 
>> This implements capabilities exchange between host and client.
>> As at the moment no capability is supported, put zero flags everywhere
>> and return.
>> 
>> Signed-off-by: Nikunj A Dadhania 
>> ---
>>  hw/scsi/spapr_vscsi.c | 37 +
>>  1 file changed, 37 insertions(+)
>> 
>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>> index e9090e5..fae3644 100644
>> --- a/hw/scsi/spapr_vscsi.c
>> +++ b/hw/scsi/spapr_vscsi.c
>> @@ -858,6 +858,40 @@ static int vscsi_send_adapter_info(VSCSIState *s, 
>> vscsi_req *req)
>>  return vscsi_send_iu(s, req, sizeof(*sinfo), VIOSRP_MAD_FORMAT);
>>  }
>>  
>> +static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req)
>> +{
>> +struct viosrp_capabilities *vcap;
>> +struct capabilities cap;
>> +uint16_t len = 0;
>
> The above initialization isn't useful
>
>> +int rc = true;
>> +
>> +vcap = &req->iu.mad.capabilities;
>> +len = be16_to_cpu(vcap->common.length);
>> +if (len > sizeof(&cap)) {
> ^ Ugh ? Why the & here ?

Oops, got that wrong.

>
>> +fprintf(stderr, "vscsi_send_capabilities: size out of bound !\n");
>> +goto error_out;
>> +}
>
> I am not 100% familiar with the protocol, could it be that we should
> just read sizeof(cap) instead of erroring out or is there no way it
> can be correct and have a len too long ?

If the length is incorrect, can we trust whether cap is correct or is of
the type we are expecting?

Re: [Qemu-devel] [PATCH 6/9] bswap.h: Remove cpu_to_be16wu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:34PM +0100, Peter Maydell wrote:
> Replace the legacy cpu_to_be16wu() with stw_be_p().
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/block/cdrom.c |4 ++--
>  hw/ide/atapi.c   |   16 
>  hw/net/e1000.c   |   19 ---
>  include/qemu/bswap.h |5 -
>  4 files changed, 18 insertions(+), 26 deletions(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH 7/9] bswap.h: Remove cpu_to_be32wu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:35PM +0100, Peter Maydell wrote:
> Replace the legacy cpu_to_be32wu() with stl_be_p().
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/block/cdrom.c |6 +++---
>  hw/net/e1000.c   |3 +--
>  hw/pci/pcie_aer.c|4 ++--
>  include/qemu/bswap.h |5 -
>  4 files changed, 6 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Nikunj A Dadhania]

Alexander Graf  writes:

> Am 26.08.2013 um 08:22 schrieb Benjamin Herrenschmidt 
> :
>
>> On Mon, 2013-08-26 at 06:44 +0100, Alexander Graf wrote:
 +cap.flags = 0;
 +cap.migration.ecl = 0;
 +cap.reserve.type = 0;
 +cap.migration.common.server_support = 0;
 +cap.reserve.common.server_support = 0;
>>> 
>>> My question stands unanswered. Is this just memset(0)?
>> 
>> If it is, then why bother reading it in the first place ? :-)
>
> Because this is a revision that incorporates other feedback from that same 
> review, so I'm quite sure he just missed it :)

Nope, i did not miss it.

Regards
Nikunj

Re: [Qemu-devel] [PATCH 8/9] bswap.h: Remove cpu_to_be64wu()

[Stefan Hajnoczi]

On Sun, Aug 25, 2013 at 03:59:36PM +0100, Peter Maydell wrote:
> Replace the legacy cpu_to_be64wu() with stq_be_p().
> 
> Signed-off-by: Peter Maydell 
> ---
>  block/qcow2-cluster.c |2 +-
>  include/qemu/bswap.h  |5 -
>  2 files changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Stefan Hajnoczi 

[Qemu-devel] [PATCHv5] add qemu-img convert -n option (skip target volume creation)

[Alex Bligh]

From: Alexandre Derumier 

Add a -n option to skip volume creation on qemu-img convert.
This is useful for targets such as rbd / ceph, where the
target volume may already exist; we cannot always rely on
qemu-img convert to create the image, as dependent on the
output format, there may be parameters which are not possible
to specify through the qemu-img convert command line.

Signed-off-by: Alexandre Derumier 
Signed-off-by: Alex Bligh 
---
 qemu-img-cmds.hx   |4 +-
 qemu-img.c |   49 ++---
 qemu-img.texi  |   15 ++-
 tests/qemu-iotests/060 |  101 
 tests/qemu-iotests/060.out |   10 +
 tests/qemu-iotests/group   |1 +
 6 files changed, 162 insertions(+), 18 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 4ca7e95..2f6d579 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,9 +34,9 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-"convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] 
[-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
+"convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o 
options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] 
output_filename")
 STEXI
-@item convert [-c] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [-O 
@var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O 
@var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,
diff --git a/qemu-img.c b/qemu-img.c
index b9a848d..8e60ced 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -103,6 +103,8 @@ static void help(void)
"  '-S' indicates the consecutive number of bytes that must contain 
only zeros\n"
"   for qemu-img to create a sparse image during conversion\n"
"  '--output' takes the format in which the output must be done 
(human or json)\n"
+   "  '-n' skips the target volume creation (useful if the volume is 
created\n"
+   "   prior to running qemu-img)\n"
"\n"
"Parameters to check subcommand:\n"
"  '-r' tries to repair any inconsistencies that are found during 
the check.\n"
@@ -1116,7 +1118,8 @@ out3:
 
 static int img_convert(int argc, char **argv)
 {
-int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
+int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size,
+cluster_sectors, skip_create;
 int progress = 0, flags;
 const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
 BlockDriver *drv, *proto_drv;
@@ -1139,8 +1142,9 @@ static int img_convert(int argc, char **argv)
 cache = "unsafe";
 out_baseimg = NULL;
 compress = 0;
+skip_create = 0;
 for(;;) {
-c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:q");
+c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qn");
 if (c == -1) {
 break;
 }
@@ -1161,6 +1165,9 @@ static int img_convert(int argc, char **argv)
 case 'c':
 compress = 1;
 break;
+case 'n':
+skip_create = 1;
+break;
 case 'e':
 error_report("option -e is deprecated, please use \'-o "
   "encryption\' instead!");
@@ -1329,20 +1336,22 @@ static int img_convert(int argc, char **argv)
 }
 }
 
-/* Create the new image */
-ret = bdrv_create(drv, out_filename, param);
-if (ret < 0) {
-if (ret == -ENOTSUP) {
-error_report("Formatting not supported for file format '%s'",
- out_fmt);
-} else if (ret == -EFBIG) {
-error_report("The image size is too large for file format '%s'",
- out_fmt);
-} else {
-error_report("%s: error while converting %s: %s",
- out_filename, out_fmt, strerror(-ret));
+if (!skip_create) {
+/* Create the new image */
+ret = bdrv_create(drv, out_filename, param);
+if (ret < 0) {
+if (ret == -ENOTSUP) {
+error_report("Formatting not supported for file format '%s'",
+ out_fmt);
+} else if (ret == -EFBIG) {
+error_report("The image size is too large for file format 
'%s'",
+ out_fmt);
+} else {
+error_report("%s: error while converting %s: %s",
+ out_filename, out_fmt, strerror(-ret));
+}
+goto out;
 }
-goto out;
 }
 
 flags = BDRV_O_RDWR;
@@ -1363,6 +1372,16 @@ static int img_convert(int argc, char **argv)
 bdrv

[Qemu-devel] [PATCHv5] add qemu-img convert -n option (skip target volume creation)

[Alex Bligh]

Add a qemu-img convert -n option to skip target volume creation

Changes since v5:
* Fail conversion if the existing output image is smaller than the
  input image and the -n option is specified
* Use -n not -C
* Add a test routine

Alexandre Derumier (1):
  add qemu-img convert -n option (skip target volume creation)

 qemu-img-cmds.hx   |4 +-
 qemu-img.c |   49 ++---
 qemu-img.texi  |   15 ++-
 tests/qemu-iotests/060 |  101 
 tests/qemu-iotests/060.out |   10 +
 tests/qemu-iotests/group   |1 +
 6 files changed, 162 insertions(+), 18 deletions(-)
 create mode 100755 tests/qemu-iotests/060
 create mode 100644 tests/qemu-iotests/060.out

-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv4] add qemu-img convert -C option (skip target volume creation)

[Alex Bligh]

On 26 Aug 2013, at 09:03, Stefan Hajnoczi wrote:

> Silently truncating can be a problem, e.g. the user deletes the original
> file after conversion completes and later discovers not all data was
> copied.
> 
> I think we should fail here.

Fixed in v5 (just sent), together with using -n not -C (obviously
I'm fighting a losing battle there).

-- 
Alex Bligh

Re: [Qemu-devel] [PATCH resend 3/3] arch_init: right return for ram_save_iterate

[Lei Li]

On 08/26/2013 04:27 PM, Paolo Bonzini wrote:

Il 23/08/2013 15:30, Lei Li ha scritto:

-if (ret < 0) {
-bytes_transferred += total_sent;
-return ret;
-}
-
  qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
  total_sent += 8;
  bytes_transferred += total_sent;
  
-return total_sent;

+return qemu_file_get_error(f);

No, this will never make ram_save_iterate (and thus
qemu_savevm_state_iterate) return a positive, non-zero value.  Thus:

 ret = qemu_file_get_error(f);
 if (ret < 0) {
 return ret;
 }
 return total_sent;

If you look at the code, you can see that it never returns zero.
Probably it should do something like

 bytes_transferred += total_sent;

 /* Do not count these 8 bytes into total_sent, so that we can
  * return 0 if no page had been dirtied.
  */
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 bytes_transferred += 8;

and then proceed as above with "ret = qemu_file_get_error(f)".


Yes, you are right.



Paolo




--
Lei

[Qemu-devel] [PATCH 1/3 v2] savevm: add comments for qemu_file_get_error()

[Lei Li]

Add comments for qemu_file_get_error(), as its return value
is not very clear.

Signed-off-by: Lei Li 
---
 savevm.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/savevm.c b/savevm.c
index 03fc4d9..95a11f9 100644
--- a/savevm.c
+++ b/savevm.c
@@ -566,6 +566,13 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps 
*ops)
 return f;
 }
 
+/*
+ * Get last error for stream f
+ *
+ * Return negative error value if there has been an error on previous
+ * operations, return 0 if no error happened.
+ *
+ */
 int qemu_file_get_error(QEMUFile *f)
 {
 return f->last_error;
-- 
1.7.7.6

[Qemu-devel] [PATCH 3/3 v2] arch_init: right return for ram_save_iterate

[Lei Li]

Signed-off-by: Lei Li 
Signed-off-by: Paolo Bonzini 
---

Change since v1:
  Return fixes and improvement from Paolo Bonzini.
  
 arch_init.c |   15 ++-
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 94d45e1..a26bc89 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -709,15 +709,20 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
  */
 ram_control_after_iterate(f, RAM_CONTROL_ROUND);
 
+bytes_transferred += total_sent;
+
+/*
+ * Do not count these 8 bytes into total_sent, so that we can
+ * return 0 if no page had been dirtied.
+ */
+qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
+bytes_transferred += 8;
+
+ret = qemu_file_get_error(f);
 if (ret < 0) {
-bytes_transferred += total_sent;
 return ret;
 }
 
-qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
-total_sent += 8;
-bytes_transferred += total_sent;
-
 return total_sent;
 }
 
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/3 v2] savevm: fix wrong error set by ram_control_load_hook()

[Lei Li]

It should set negative error value if there has been an error.

Reviewed-by: Michael R. Hines 
Signed-off-by: Lei Li 
---
 savevm.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/savevm.c b/savevm.c
index 95a11f9..a0be109 100644
--- a/savevm.c
+++ b/savevm.c
@@ -649,7 +649,7 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
 
 void ram_control_load_hook(QEMUFile *f, uint64_t flags)
 {
-int ret = 0;
+int ret = -EINVAL;
 
 if (f->ops->hook_ram_load) {
 ret = f->ops->hook_ram_load(f, f->opaque, flags);
-- 
1.7.7.6

[Qemu-devel] [PATCH v2 07/47] hw/arm/Makefile.objs: CONFIG_* created for each board

[Ákos Kovács]

The new CONFIG_* definitions added to the default-configs/arm-softmmu.mak
Signed-off-by: Ákos Kovács 
---
 default-configs/arm-softmmu.mak |   13 +
 hw/arm/Makefile.objs|   35 ---
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index d13bc2b..885b0c1 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -79,3 +79,16 @@ CONFIG_VERSATILE_PCI=y
 CONFIG_VERSATILE_I2C=y
 
 CONFIG_SDHCI=y
+
+CONFIG_COLLIE=y
+CONFIG_GUMSTIX=y
+CONFIG_HIGHBANK=y
+CONFIG_INTEGRATORCP=y
+CONFIG_KZM=y
+CONFIG_MUSICPAL=y
+CONFIG_PALM=y
+CONFIG_SPITZ=y
+CONFIG_TOSA=y
+CONFIG_VERSATILEPB=y
+CONFIG_VXPRESS=y
+CONFIG_Z2=y
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 3671b42..fa0eabf 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -1,7 +1,28 @@
-obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
-obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
-obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
-obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
-
-obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
-obj-y += omap1.o omap2.o strongarm.o
+obj-y += boot.o
+obj-$(CONFIG_COLLIE)   += collie.o
+obj-$(CONFIG_EXYNOS4)  += exynos4_boards.o 
+obj-$(CONFIG_EXYNOS4) += exynos4210.o
+obj-$(CONFIG_GUMSTIX)  += gumstix.o
+obj-$(CONFIG_HIGHBANK) += highbank.o
+obj-$(CONFIG_INTEGRATORCP) += integratorcp.o
+obj-$(CONFIG_KZM)  += kzm.o
+obj-$(CONFIG_MAINSTONE)+= mainstone.o
+obj-$(CONFIG_MUSICPAL) += musicpal.o
+obj-$(CONFIG_NSERIES)  += nseries.o
+obj-$(CONFIG_OMAP) += omap_sx1.o
+obj-$(CONFIG_PALM) += palm.o
+obj-$(CONFIG_REALVIEW) += realview.o 
+obj-$(CONFIG_SPITZ)+= spitz.o
+obj-$(CONFIG_STELLARIS)+= stellaris.o
+obj-$(CONFIG_TOSA) += tosa.o
+obj-$(CONFIG_VERSATILEPB) += versatilepb.o
+obj-$(CONFIG_VXPRESS) += vexpress.o
+obj-$(CONFIG_ZYNQ) += xilinx_zynq.o 
+obj-$(CONFIG_ARMV7M) += armv7m.o
+obj-$(CONFIG_PXA2XX) += pxa2xx.o
+obj-$(CONFIG_PXA2XX) += z2.o
+obj-$(CONFIG_PXA2XX) += pxa2xx_gpio.o
+obj-$(CONFIG_PXA2XX) += pxa2xx_pic.o
+obj-$(CONFIG_OMAP) += omap1.o
+obj-$(CONFIG_OMAP) += omap2.o 
+obj-$(CONFIG_STRONGARM) += strongarm.o
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Alexander Graf]

On 26.08.2013, at 11:08, Nikunj A Dadhania wrote:

> Alexander Graf  writes:
> 
>> Am 26.08.2013 um 08:22 schrieb Benjamin Herrenschmidt 
>> :
>> 
>>> On Mon, 2013-08-26 at 06:44 +0100, Alexander Graf wrote:
> +cap.flags = 0;
> +cap.migration.ecl = 0;
> +cap.reserve.type = 0;
> +cap.migration.common.server_support = 0;
> +cap.reserve.common.server_support = 0;
 
 My question stands unanswered. Is this just memset(0)?
>>> 
>>> If it is, then why bother reading it in the first place ? :-)
>> 
>> Because this is a revision that incorporates other feedback from that same 
>> review, so I'm quite sure he just missed it :)
> 
> Nope, i did not miss it.

So you ignored it on purpose?


Alex

[Qemu-devel] Boot Problems Windows XP guest

[Erik Rull]

Hi all,

is it possible to get back to the "legacy" BIOS instead of the (u)efi based
BIOS? I have problems booting the guest on a SSD HDD, there it reboots
infinitely, when running the guest on a rotating HDD, it works. On qemu 1.2.0
both disks work properly. I didn't find a way to select the BIOS type in QEMU.

Best regards,

Erik

[Qemu-devel] [Bug 1213797] Re: Guest hang after live migration

[Paolo Bonzini]

The fix is in no released version, so the bug is stomped.

** Changed in: qemu
   Status: New => 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/1213797

Title:
  Guest hang after live migration

Status in QEMU:
  Fix Released

Bug description:
  Environment:
  
  Host OS (ia32/ia32e/IA64):ia32e
  Guest OS (ia32/ia32e/IA64):ia32e
  Guest OS Type (Linux/Windows):Linux
  kvm.git Commit:205befd9a5c701b56f569434045821f413f08f6d
  qemu-kvm uq/master Commit:ca916d3729564d0eb3c2374a96903f7e8aced8a7
  Host Kernel Version:3.11.0-rc1
  Hardware:Romley_EP, Ivytown_EP

  Bug detailed description:
  --
  after guest live migration, the guest will hang, and ping IP fail

  note:
  1. after guest save/restore, the guest will hang, and ping IP fail
  2. This should be a qemu-kvm bug.
  kvm  + qemu-kvm   =  result
  205befd9 + ca916d37   =  bad
  205befd9 + f03d07d4   =  good

  
  Reproduce steps:
  
  1.Start a TCP daemon for migration
  qemu-system-x86_64 -enable-kvm -m 2G -smp 2 -net nic,macaddr=00:12:34:56:13:45
  -net tap,script=/etc/kvm/qemu-ifup rhel6u2.qcow -incoming tcp:localhost:
  2. create guest
  qemu-system-x86_64 -enable-kvm -m 2G -smp 2 -net nic,macaddr=00:12:34:56:13:45
  -net tap,script=/etc/kvm/qemu-ifup rhel6u2.qcow 
  3. migrate tcp:localhost:

  Current result:
  
  guest hang

  Expected result:
  
  after live/migration or save/restore, the guest works fine

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

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Benjamin Herrenschmidt]

On Mon, 2013-08-26 at 10:43 +0200, Alexander Graf wrote:
> Do we ever need to preserve random fields in that struct? Or would it
> be clearer to just set the whole thing to 0 and then go from there?

Yeah sort of, we set/clear bits more/less based on what the guest
put in the first place, it's a bit strange.

> It's definitely a case-by-case decision which way is better, but
> setting all fields by hand gives you potential for missing to set one.

Right. I'll let Nikunj judge here as he looked at that part of the
protocol more than I have.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH -V3 1/4] target-ppc: Update slb array with correct index values.

[Alexander Graf]

On 26.08.2013, at 08:46, Aneesh Kumar K.V wrote:

> Alexander Graf  writes:
> 
>> On 23.08.2013, at 06:20, Aneesh Kumar K.V wrote:
>> 
>>> From: "Aneesh Kumar K.V" 
>>> 
>>> Without this, a value of rb=0 and rs=0 results in replacing the 0th
>>> index. This can be observed when using gdb remote debugging support.
>>> 
>>> (gdb) x/10i do_fork
>>> 0xc0085330 :Cannot access memory at address 
>>> 0xc0085330
>>> (gdb)
>>> 
>>> This is because when we do the slb sync via kvm_cpu_synchronize_state,
>>> we overwrite the slb entry (0th entry) for 0xc0085330
>>> 
>>> Signed-off-by: Aneesh Kumar K.V 
>>> ---
>>> target-ppc/kvm.c | 20 ++--
>>> 1 file changed, 18 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>> index 30a870e..6878af2 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -1033,9 +1033,25 @@ int kvm_arch_get_registers(CPUState *cs)
>>> 
>>>/* Sync SLB */
>>> #ifdef TARGET_PPC64
>>> +/*
>>> + * KVM_GET_SREGS doesn't return slb entry with slot information
>>> + * same as index. The ioctl zero fills the array and update only
>>> + * upto slb_max entries. We cannot depend on the slot value
>>> + * in the slbe field for update, because a zero slbe value would
>>> + * result in us wrongly updating the 0th index. Instead we zero 
>>> fill
>>> + * the env->slb array first so that we mark all entries invalid and
>>> + * update with only valid SLB entries.
>> 
>> Still too negative. How about something like this:
>> 
>> /*
>> * The packed SLB array we get from KVM only contains information
>> *  about valid entries. So we flush our internal copy to get rid of stale
>> *  ones, then put all valid SLB entries back in.
>> */
> 
> updated
> 
>> 
>>> + */
>>> +memset(env->slb, 0, 64 * sizeof(ppc_slb_t));
>> 
>> Can't we use ARRAY_SIZE here and below?
> 
> I was thinking we want to be explicit there saying we are zeroing out all
> the 64 entries. I could do sizeof(env->slb).

Yup, that works too. In the loop below s/64/ARRAY_SIZE(env->slb)/ should work 
too I think.


Alex

Re: [Qemu-devel] [PATCH 18/47] hw/arm/Kconfig: Add ARM Kconfig

[Peter Maydell]

On 25 August 2013 23:58, Ákos Kovács  wrote:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/arm/Kconfig |  235 
> 
>  1 file changed, 235 insertions(+)
>  create mode 100644 hw/arm/Kconfig
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> new file mode 100644
> index 000..c72b949
> --- /dev/null
> +++ b/hw/arm/Kconfig
> @@ -0,0 +1,235 @@
> +config ARM
> +bool
> +#select ARM_NVIC

Why is this here but commented out?

> +default y
> +
> +menu "ARM"
> +config EXYNOS4
> +bool "Samsung Exynos4210 SoC"
> +select A9SCU # snoop controll unit
> +select USB_EHCI
> +select LAN9118
> +select PL310 # cache controller
> +select PCI

No PCI on this board (well, there is, but not modelled).

> +select ARM_GIC
> +select ARM_NVIC

This is wrong -- this CPU doesn't have an NVIC.

> +select ARM9MPCORE

This is misnamed: should be A9MPCORE.

> +select ARM_MPTIMER

Each Cortex-A9 platform shouldn't have to individually
select all of the components of the A9 individually
(GIC, mpcore container, mp timer, SCU) -- A9MPCORE
should just pull them all in.

> +default y
> +
> +config HIGHBANK
> +bool "Calxeda Highbank SoC"
> +select A9SCU # snoop controll unit
> +select AHCI
> +select PL011 # UART
> +select PL022 # Serial port
> +select PL031 # RTC
> +select PL061 # GPIO
> +select PL310 # cache controller
> +select XGMAC # ethernet
> +select ARM_TIMER # sp804
> +select ARM_NVIC
> +select ARM_MPTIMER
> +select ARM9MPCORE
> +select PCI

No PCI (yet).

> +default y
> +
> +config INTEGRATORCP
> +bool "ARM Integrator CP"
> +select SMC91C111
> +select ARM_TIMER
> +select PL011 # UART
> +select PL031 # RTC
> +select PL050 # keyboard/mouse
> +select PL110 # pl111 LCD controller
> +select PL181 # display
> +select PCI

No PCI.

> +default y
> +
> +config KZM
> +bool "Kzm"
> +select SERIAL
> +select IMX
> +select LAN9118
> +select PCI

No PCI.

> +default y
> +
> +config MUSICPAL
> +bool "Marvell MV88W8618 / Freecom MusicPal"
> +select PFLASH_CFI02
> +select PTIMER
> +select PCI
> +select BITBANG_I2C
> +select MARVELL_88W8618
> +select WM8750
> +select SERIAL
> +default y
> +
> +config OMAP
> +bool "Texas Instruments Open Multimedia Applications Platform"

Don't we want separate configs for OMAP1 and OMAP2?

> +select SERIAL
> +select PFLASH_CFI01
> +select PCI

No PCI.

> +default y
> +
> +config NSERIES
> +bool "Nokia N-Series tablets"
> +select TMP105   # tempature sensor
> +select BLIZZARD # LCD/TV controller
> +select ONENAND
> +select TSC210X  # touchscreen/sensors/audio
> +select TSC2005  # touchscreen/sensors/keypad
> +select LM832X   # GPIO keyboard chip
> +select TWL92230 # energy-management
> +depends on OMAP

Why 'depends on' rather than 'select' ?

> +default y
> +
> +config PALM
> +bool "PalmOne PDAs"
> +select TSC210X
> +depends on OMAP
> +default y
> +
> +config STELLARIS
> +bool
> +select PL011 # UART
> +select PL022 # Serial port
> +select PL061 # GPIO
> +select STELLARIS_INPUT
> +select STELLARIS_ENET # ethernet
> +select SSD0303 # OLED display
> +select SSD0323 # OLED display
> +select SSI_SD
> +default y# for armv7m_nvic_*

What is this comment trying to say?

> +
> +config REALVIEW
> +bool "ARM Realview baseboard"
> +# networking
> +select SMC91C111
> +select LAN9118
> +select RTL8139_PCI

This doesn't make any sense. Any board with PCI
should be compiled with all the PCI card models, so
there shouldn't be any need to manually select one here.

> +
> +select VERSATILE_PCI
> +select WM8750 # audio codec
> +select PL011  # UART
> +select PL041  # audio codec
> +select PL050  # keyboard/mouse
> +select PL061  # GPIO
> +select PL080  # DMA controller
> +select VERSATILE_I2C
> +select DS1338 # I2C RTC+NVRAM
> +select USB_OHCI

This is just a PCI card too, right?

> +select ARM_GIC
> +select ARM_MPTIMER
> +select ARM15MPCORE
> +select ARM11MPCORE
> +select ARM9MPCORE
> +
> +config VERSATILEPB
> +bool "ARM Versatile platform"
> +select PFLASH_CFI01
> +select PL031  # RTC
> +select PL050  # keyboard/mouse
> +select PL080  # DMA controller
> +select PL181  # display
> +select PL

Re: [Qemu-devel] [PATCH 17/47] hw/9pfs/Kconfig: Add 9pfs Kconfig

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/9pfs/Kconfig |2 ++
>  1 file changed, 2 insertions(+)
>  create mode 100644 hw/9pfs/Kconfig
> 
> diff --git a/hw/9pfs/Kconfig b/hw/9pfs/Kconfig
> new file mode 100644
> index 000..f16ffed
> --- /dev/null
> +++ b/hw/9pfs/Kconfig
> @@ -0,0 +1,2 @@
> +config OPEN_BY_HANDLE
> +bool
> 

This is defined by config-host.mak, so it is not needed here.

However, you may want a CONFIG_VIRTIO_9P, controlling compilation of
virtio-9p-device.o.

Paolo

Re: [Qemu-devel] [PATCH 19/47] hw/audio/Kconfig: Add audio Kconfig

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/audio/Kconfig |   41 +
>  1 file changed, 41 insertions(+)
>  create mode 100644 hw/audio/Kconfig
> 
> diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig
> new file mode 100644
> index 000..bb6c31d
> --- /dev/null
> +++ b/hw/audio/Kconfig
> @@ -0,0 +1,41 @@
> +menu "Audio devices"
> +config SB16
> +bool
> +#select ISA

All these should have "depends on ISA".


> +config ES1370
> +bool "ES1370"
> +depends on PCI
> +
> +config AC97
> +bool "AC97 sound card"
> +default y
> +depends on PCI
> +
> +config ADLIB
> +bool
> +#select ISA
> +
> +config CS4231A
> +bool
> +
> +config HDA
> +bool
> +#select ISA
> +
> +config PCSPK
> +bool
> +select I8254

Perhaps "depends on I8254"?

> +config WM8750
> +bool
> +
> +config PL041
> +bool
> +
> +config CS4231
> +bool
> +
> +config MARVELL_88W8618
> +bool
> +endmenu
> 

Re: [Qemu-devel] [PATCH 07/47] hw/arm/Makefile.objs: CONFIG_* created for each board

[Peter Maydell]

On 25 August 2013 23:58, Ákos Kovács  wrote:
> The new CONFIG_* definitions added to the default-configs/arm-softmmu.mak
> Signed-off-by: Ákos Kovács 
> ---
>  default-configs/arm-softmmu.mak |   13 +
>  hw/arm/Makefile.objs|   35 ---
>  2 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index d13bc2b..885b0c1 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -79,3 +79,16 @@ CONFIG_VERSATILE_PCI=y
>  CONFIG_VERSATILE_I2C=y
>
>  CONFIG_SDHCI=y
> +
> +CONFIG_COLLIE=y
> +CONFIG_GUMSTIX=y
> +CONFIG_HIGHBANK=y
> +CONFIG_INTEGRATORCP=y
> +CONFIG_KZM=y
> +CONFIG_MUSICPAL=y
> +CONFIG_PALM=y
> +CONFIG_SPITZ=y
> +CONFIG_TOSA=y
> +CONFIG_VERSATILEPB=y
> +CONFIG_VXPRESS=y

VEXPRESS.

> +CONFIG_Z2=y
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 3671b42..fa0eabf 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -1,7 +1,28 @@
> -obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
> -obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
> -obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
> -obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
> -
> -obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
> -obj-y += omap1.o omap2.o strongarm.o
> +obj-y += boot.o
> +obj-$(CONFIG_COLLIE)   += collie.o
> +obj-$(CONFIG_EXYNOS4)  += exynos4_boards.o
> +obj-$(CONFIG_EXYNOS4) += exynos4210.o
> +obj-$(CONFIG_GUMSTIX)  += gumstix.o
> +obj-$(CONFIG_HIGHBANK) += highbank.o
> +obj-$(CONFIG_INTEGRATORCP) += integratorcp.o
> +obj-$(CONFIG_KZM)  += kzm.o
> +obj-$(CONFIG_MAINSTONE)+= mainstone.o
> +obj-$(CONFIG_MUSICPAL) += musicpal.o
> +obj-$(CONFIG_NSERIES)  += nseries.o
> +obj-$(CONFIG_OMAP) += omap_sx1.o

This is a board -- why is it sharing a config with OMAP
rather than getting its own?

> +obj-$(CONFIG_PALM) += palm.o
> +obj-$(CONFIG_REALVIEW) += realview.o
> +obj-$(CONFIG_SPITZ)+= spitz.o
> +obj-$(CONFIG_STELLARIS)+= stellaris.o
> +obj-$(CONFIG_TOSA) += tosa.o
> +obj-$(CONFIG_VERSATILEPB) += versatilepb.o
> +obj-$(CONFIG_VXPRESS) += vexpress.o
> +obj-$(CONFIG_ZYNQ) += xilinx_zynq.o
> +obj-$(CONFIG_ARMV7M) += armv7m.o
> +obj-$(CONFIG_PXA2XX) += pxa2xx.o
> +obj-$(CONFIG_PXA2XX) += z2.o

Why didn't this board get its own CONFIG_Z2 ?

> +obj-$(CONFIG_PXA2XX) += pxa2xx_gpio.o
> +obj-$(CONFIG_PXA2XX) += pxa2xx_pic.o
> +obj-$(CONFIG_OMAP) += omap1.o
> +obj-$(CONFIG_OMAP) += omap2.o
> +obj-$(CONFIG_STRONARM) += strongarm.o

Typo, as Max points out.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 20/47] hw/block/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/block/Kconfig |   34 ++
>  1 file changed, 34 insertions(+)
>  create mode 100644 hw/block/Kconfig
> 
> diff --git a/hw/block/Kconfig b/hw/block/Kconfig
> new file mode 100644
> index 000..400cab3
> --- /dev/null
> +++ b/hw/block/Kconfig
> @@ -0,0 +1,34 @@
> +config FDC
> +bool
> +#select ISA
> +
> +config SSI_M25P80
> +bool
> +select SSI
> +
> +config NAND
> +bool
> +
> +config PFLASH_CFI01
> +bool
> +
> +config PFLASH_CFI02
> +bool
> +
> +config ECC
> +bool
> +
> +config ONENAND
> +bool
> +
> +config PC_SYSFW
> +bool
> +#select ISA

This is not anymore in the tree.

> +config NVME_PCI
> +bool
> +depends on PCI
> +
> +config VIRTIO_BLK_DATA_PLANE
> +bool
> +select VIRTIO

This is controlled by config-host.mak, so no need for it here.

Perhaps, similar to 9p, you could have

config VIRTIO_BLK
bool
default y
depends on VIRTIO

but it can be added later, too.

Paolo

Re: [Qemu-devel] [PATCH 21/47] hw/char/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/char/Kconfig |   24 
>  1 file changed, 24 insertions(+)
>  create mode 100644 hw/char/Kconfig
> 
> diff --git a/hw/char/Kconfig b/hw/char/Kconfig
> new file mode 100644
> index 000..7ad0bd3
> --- /dev/null
> +++ b/hw/char/Kconfig
> @@ -0,0 +1,24 @@
> +config IPACK 
> +bool
> +depends on PCI

PCI devices are generally configurable, so you need to add prompts to them.

> +config ESCC
> +bool
> +
> +config PARALLEL
> +bool
> +#select ISA

Also "depends on" here.  You probably also want to make all these ISA
devices configurable, so add a prompt.

Paolo

> +config SERIAL
> +bool
> +#select ISA
> +
> +config SERIAL_PCI
> +bool
> +depends on PCI
> +
> +config SCLPCONSOLE
> +bool
> +
> +config PL011
> +bool
> 

Re: [Qemu-devel] [PATCH 22/47] hw/core/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/core/Kconfig |   11 +++
>  1 file changed, 11 insertions(+)
>  create mode 100644 hw/core/Kconfig
> 
> diff --git a/hw/core/Kconfig b/hw/core/Kconfig
> new file mode 100644
> index 000..8c275f7
> --- /dev/null
> +++ b/hw/core/Kconfig
> @@ -0,0 +1,11 @@
> +config EMPTY_SLOT
> +bool
> +
> +config XILINX_AXI
> +bool

Please make this "config STREAM" and "select" it from XILINX_AXI.

> +config PTIMER
> +bool

Remember that this should be selected from all devices that use ptimers.

> +config SOFTMMU
> +bool

This is not needed, it comes from config-target.mak.

Paolo

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Nikunj A Dadhania]

Alexander Graf  writes:
> Am 26.08.2013 um 05:32 schrieb Nikunj A Dadhania :
>
>> Benjamin Herrenschmidt  writes:
>> From: Nikunj A Dadhania 
>> 
>> This implements capabilities exchange between host and client.
>
> Client?

vscsi host implemented by VIOS and vscsi client running inside
the guest.

Regards
Nikunj

Re: [Qemu-devel] [PATCH 24/47] hw/display/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/display/Kconfig |   84 
> 
>  1 file changed, 84 insertions(+)
>  create mode 100644 hw/display/Kconfig
> 
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> new file mode 100644
> index 000..1f6f386
> --- /dev/null
> +++ b/hw/display/Kconfig
> @@ -0,0 +1,84 @@
> +config ADS7846
> +bool
> +select SSI

Similarly this should be "depends on".  To put it more clearly:

- devices that are on a particular bus should "depend on" that bus;

- devices that expose a particular bus (for example a PCI host bridge)
should "select" it so that the user can choose which devices to enable
on that board

- devices can also "select" library features that they use, for example
FRAMEBUFFER or PTIMER, or "sub-devices" that they include, for example
VGA or what you did in I82378

- boards should also "select" devices, but you got that part right so no
need to explain :)

Paolo

> +config VGA_CIRRUS
> +bool
> +depends on PCI
> +select VGA
> +#select ISA
> +
> +config G364FB
> +bool
> +
> +config JAZZ_LED
> +bool
> +
> +config PL110
> +bool
> +select FRAMEBUFFER
> +
> +config SSD0303
> +bool
> +#select I2C
> +
> +config SSD0323
> +bool
> +select SSI
> +
> +config VGA_PCI
> +bool
> +select VGA
> +depends on PCI
> +
> +config VGA_ISA
> +bool
> +select VGA
> +#select ISA
> +
> +config VGA_ISA_MM
> +bool
> +select VGA
> +
> +config VMWARE_VGA
> +bool
> +select VGA
> +depends on PCI
> +
> +config BLIZZARD
> +bool
> +
> +config FRAMEBUFFER
> +bool
> +
> +config MILKYMIST
> +bool
> +select FRAMEBUFFER
> +
> +config ZAURUS
> +bool
> +select NAND
> +select ECC
> +
> +config OMAP
> +bool
> +select FRAMEBUFFER
> +
> +config PXA2XX
> +bool
> +select FRAMEBUFFER
> +
> +config MILKYMIST_TMU2
> +bool
> +
> +config SM501
> +bool
> +
> +config TCX
> +bool
> +
> +config VGA
> +bool
> +
> +config QXL
> +bool
> 

Re: [Qemu-devel] [PATCH 25/47] hw/dma/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/dma/Kconfig |   25 +
>  1 file changed, 25 insertions(+)
>  create mode 100644 hw/dma/Kconfig
> 
> diff --git a/hw/dma/Kconfig b/hw/dma/Kconfig
> new file mode 100644
> index 000..1ef6b47
> --- /dev/null
> +++ b/hw/dma/Kconfig
> @@ -0,0 +1,25 @@
> +config RC4030
> +bool
> +
> +config PL080
> +bool
> +
> +config PL330
> +bool
> +
> +config I82374
> +bool
> +#select ISA
> +select I8257
> +
> +config I8257
> +bool
> +#select ISA

So, according to the rules in the previous email, "select I8257" is
right, but it must be once more "depends on ISA".

Paolo

> +config XILINX_AXI
> +bool
> +select PTIMER
> +
> +config STP2000
> +bool
> +select SUN4M
> 

Re: [Qemu-devel] [PATCH 27/47] hw/i2c/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/i2c/Kconfig |   10 ++
>  1 file changed, 10 insertions(+)
>  create mode 100644 hw/i2c/Kconfig
> 
> diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
> new file mode 100644
> index 000..eaf17cc
> --- /dev/null
> +++ b/hw/i2c/Kconfig
> @@ -0,0 +1,10 @@
> +config VERSATILE_I2C
> +bool
> +select BITBANG_I2C
> +
> +config ACPI
> +bool
> +depends on PCI

Please move this to hw/acpi/Kconfig.

Later we can add separate PM_SMBUS and SMBUS_ICH9 symbols with proper
dependencies.

> +config BITBANG_I2C
> +bool
> 

Re: [Qemu-devel] [PATCH 30/47] hw/intc/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/intc/Kconfig |   28 
>  1 file changed, 28 insertions(+)
>  create mode 100644 hw/intc/Kconfig
> 
> diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
> new file mode 100644
> index 000..c4dec79
> --- /dev/null
> +++ b/hw/intc/Kconfig
> @@ -0,0 +1,28 @@
> +config HEATHROW_PIC
> +bool
> +
> +config I8259
> +bool
> +
> +config PL190
> +bool
> +
> +config APIC
> +bool
> +
> +config ARM_GIC
> +bool
> +
> +config ARM_GIC_KVM
> +bool
> +depends on KVM

IIRC "KVM" is not available yet in Kconfig, so you need to leave this
dependency out.

> +config IOAPIC
> +bool
> +
> +config OPENPICS
> +bool
> +
> +config STELLARIS
> +bool
> +select ARM_GIC
> 

Please make this symbol ARMV7M_NVIC, and make STELLARIS select it.

Paolo

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Nikunj A Dadhania]

Alexander Graf  writes:
> Am 26.08.2013 um 05:32 schrieb Nikunj A Dadhania :
>
>> Benjamin Herrenschmidt  writes:
>> 
>>> On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote:
> +vcap = &req->iu.mad.capabilities;
> +rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer),
> +&cap,
 be16_to_cpu(vcap->common.length));
 
 While I don't think any harm could happen from it, this could lead to
 a potential timing attack where we read and write from different
 locations in memory if the guest swizzles the request while we're
 processing it.
>>> 
>>> BTW. While I disagree with your initial comment ... is there any bound
>>> checking here ? That looks like potential stack corruption unless I
>>> miss something if the guest passes a too big length...
>>> 
>>> So at least the length should be read once, bound-checked, then the read
>>> done with the result (don't bound check and read again, that would be
>>> indeed racy).
>> 

From: Nikunj A Dadhania 

This implements capabilities exchange between host and client.
As at the moment no capability is supported, put zero flags everywhere
and return.

Signed-off-by: Nikunj A Dadhania 
---
 hw/scsi/spapr_vscsi.c | 44 
 1 file changed, 44 insertions(+)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index e9090e5..0758263 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -858,6 +858,47 @@ static int vscsi_send_adapter_info(VSCSIState *s, 
vscsi_req *req)
 return vscsi_send_iu(s, req, sizeof(*sinfo), VIOSRP_MAD_FORMAT);
 }
 
+static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req)
+{
+struct viosrp_capabilities *vcap;
+struct capabilities cap;
+uint16_t len;
+uint64_t buffer;
+int rc;
+
+vcap = &req->iu.mad.capabilities;
+len = be16_to_cpu(vcap->common.length);
+buffer = be64_to_cpu(vcap->buffer);
+if (len > sizeof(cap)) {
+fprintf(stderr, "vscsi_send_capabilities: size out of bound !\n");
+rc = H_PARAMETER;
+goto error_out;
+}
+rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len);
+if (rc)  {
+fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");
+}
+
+/*
+ * Current implementation does not suppport any migration or
+ * reservation capabilities. Construct the response telling the
+ * guest not to use them.
+ */
+cap.flags = 0;
+cap.migration.ecl = 0;
+cap.reserve.type = 0;
+cap.migration.common.server_support = 0;
+cap.reserve.common.server_support = 0;
+
+rc = spapr_vio_dma_write(&s->vdev, buffer, &cap, len);
+if (rc)  {
+fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n");
+}
+error_out:
+vcap->common.status = rc ? cpu_to_be32(1) : 0;
+return vscsi_send_iu(s, req, sizeof(*vcap), VIOSRP_MAD_FORMAT);
+}
+
 static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req)
 {
 union mad_iu *mad = &req->iu.mad;
@@ -878,6 +919,9 @@ static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req 
*req)
 mad->host_config.common.status = cpu_to_be16(1);
 vscsi_send_iu(s, req, sizeof(mad->host_config), VIOSRP_MAD_FORMAT);
 break;
+case VIOSRP_CAPABILITIES_TYPE:
+vscsi_send_capabilities(s, req);
+break;
 default:
 fprintf(stderr, "VSCSI: Unknown MAD type %02x\n",
 be32_to_cpu(mad->empty_iu.common.type));
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 32/47] hw/misc/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/misc/Kconfig |   67 
> +++
>  1 file changed, 67 insertions(+)
>  create mode 100644 hw/misc/Kconfig
> 
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> new file mode 100644
> index 000..5a6477f
> --- /dev/null
> +++ b/hw/misc/Kconfig
> @@ -0,0 +1,67 @@
> +config APPLESMC
> +bool
> +#select ISA
> +
> +config MAX111X
> +bool
> +select SSI
> +
> +config TMP105
> +bool
> +#select I2C
> +
> +config ISA_DEBUG
> +bool
> +#select ISA
> +
> +config SGA
> +bool
> +#depends on PCI
> +
> +config ISA_TESTDEV
> +bool
> +#select ISA
> +
> +config PCI_TESTDEV
> +bool
> +depends on PCI
> +
> +config VMPORT
> +bool
> +#select ISA
> +
> +config PL310
> +bool
> +
> +config PUV3
> +bool
> +
> +config MACIO
> +bool
> +depends on PCI
> +select MAC_NVRAM

Correct: this is a PCI device that embeds an MacIONVRAMState.

But it also embeds a MAC_DBDMA device, so "select MAC_DBDMA" too.

> +config CUDA
> +bool
> +select MACIO

Should be "select ADB" since this is a device that exposes an ADB bus.

> +config MAC_DBDMA
> +bool   
> +select MACIO
> +select ADB
> +#select ISA

Why these?  I think it doesn't need any of the three.

> +config A9SCU
> +bool
> +
> +config ECCMEMCTL
> +bool
> +select ECC
> +
> +config OMAP
> +bool
> +select ECCMEMCTL
> +select NAND
> +
> +config PVPANIC
> +bool
> 

Re: [Qemu-devel] [PATCH 35/47] hw/pci/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> +
> +menu "PCI"
> +config PCI_HOTPLUG
> +bool "Enable hotplugging for PCI devices"
> +depends on PCI
> +endmenu

This has now been renamed to CONFIG_PCI_HOTPLUG_OLD.  It's a legacy
interface, so perhaps you can leave the prompt out.

Paolo

Re: [Qemu-devel] [PATCH 37/47] hw/pci-host/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/pci-host/Kconfig |   36 
>  1 file changed, 36 insertions(+)
>  create mode 100644 hw/pci-host/Kconfig
> 
> diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig
> new file mode 100644
> index 000..12014f5
> --- /dev/null
> +++ b/hw/pci-host/Kconfig
> @@ -0,0 +1,36 @@
> +config PREP_PCI
> +bool
> +select PCI
> +
> +config GRACKLE_PCI
> +bool
> +select PCI
> +
> +config UNIN_PCI
> +bool
> +select PCI
> +
> +config PPCE500_PCI
> +bool
> +select PCI
> +
> +config VERSATILE_PCI
> +bool
> +select PCI
> +
> +config PCI_APB
> +bool
> +select PCI
> +
> +config FULONG
> +bool
> +select PCI
> +
> +config PCI_PIIX
> +bool
> +select PCI
> +#select ISA

Here finally we have one that should be "select ISA"! :)  This is
because this file actually implements both a PCI bridge (the i440FX
"NorthBridge") and an ISA bridge (the PIIX3 "SouthBridge").

Paolo


> +config PCI_Q35
> +bool
> +select PCI
> 

Re: [Qemu-devel] [PATCH 31/47] hw/isa/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/isa/Kconfig |   42 ++
>  1 file changed, 42 insertions(+)
>  create mode 100644 hw/isa/Kconfig
> 
> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> new file mode 100644
> index 000..8b6baaf
> --- /dev/null
> +++ b/hw/isa/Kconfig
> @@ -0,0 +1,42 @@
> +#config ISA
> +#   bool

Why is it commented out?

> +config APM
> +bool
> +
> +config I82378
> +bool
> +#select ISA
> +select I8259  # irq
> +select PCSPK
> +select I82374 # DMA
> +select MC146818RTC # timer
> +depends on PCI
> +
> +config ISA_MMIO
> +bool
> +#select ISA 

This doesn't exist anymore.

> +config PC87312
> +bool
> +#select ISA 
> +select SERIAL
> +select PARALLEL
> +select FDC
> +select IDE_ISA
> +
> +config PIIX4
> +bool
> +#select ISA 
> +depends on PCI
> +
> +config VT82C686
> +bool
> +#select ISA 
> +depends on PCI
> +
> +config LPC_ICH9
> +bool
> +#select ISA 
> +depends on PCI 
> +select APM

"select ISA" is right for this file.

Paolo

Re: [Qemu-devel] [PATCH 39/47] hw/sd/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/sd/Kconfig |   23 +++
>  1 file changed, 23 insertions(+)
>  create mode 100644 hw/sd/Kconfig
> 
> diff --git a/hw/sd/Kconfig b/hw/sd/Kconfig
> new file mode 100644
> index 000..eaef4f7
> --- /dev/null
> +++ b/hw/sd/Kconfig
> @@ -0,0 +1,23 @@
> +config SD
> +bool
> +
> +config PL181
> +bool
> +select SD

"select SD" is right ("library" code).

> +config SSI_SD
> +bool
> +select SD
> +select SSI

But this needs to be "depends on".  In general, you'll usually have
"select SSI" inside hw/ssi, "select ISA" inside hw/isa, "select PCI"
inside hw/pci-host, etc.

Paolo

> +config SDHCI
> +bool
> +select SD
> +
> +config MILKYMIST
> +bool
> +select SD
> +
> +config PXA2XX
> +bool
> +select SD
> 

Re: [Qemu-devel] [PATCH 43/47] hw/usb/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/usb/Kconfig |   60 
> 
>  1 file changed, 60 insertions(+)
>  create mode 100644 hw/usb/Kconfig
> 
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
> new file mode 100644
> index 000..a39915c
> --- /dev/null
> +++ b/hw/usb/Kconfig
> @@ -0,0 +1,60 @@
> +menu "USB devices"
> +config USB_UHCI
> +bool "USB UHCI support"
> +default y
> +
> +config USB_OHCI
> +bool "USB OHCI support"
> +default y
> +depends on PCI
> +
> +config USB_EHCI
> +bool "USB EHCI support"
> +default y
> +
> +config USB_XHCI
> +bool "USB XHCI support"
> +default y
> +depends on PCI
> +
> +config USB_MUSB
> +bool "USB MUSB support"
> +default y
> +
> +config USB_TABLET_WACOM
> +bool "Wacom tablet USB"
> +default y
> +
> +config USB_STORAGE_BOT
> +bool "USB harddrive support"
> +default y
> +
> +config USB_STORAGE_UAS
> +bool "UAS storage"
> +default y
> +
> +config USB_AUDIO
> +bool "USB audio"
> +default y
> +
> +config USB_SERIAL
> +bool "USB serial port adapter"
> +default y
> +
> +config USB_NETWORK
> +bool "USB networking"
> +
> +config USB_BLUETOOTH
> +bool "USB bluetooth"
> +default y
> +
> +config USB_SMARTCARD
> +bool "USB Smart Card"
> +default y
> +
> +config USB_SMARTCARD_NSS
> +bool "Emulated USB Smart Card"
> +
> +config USB_REDIR
> +bool "USB redirection device"
> +endmenu
> 

You should include the 3 USB patches you sent last week before this one,
please.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 44/47] hw/watchdog/Kconfig: Add Kconfig file

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/watchdog/Kconfig |7 +++
>  1 file changed, 7 insertions(+)
>  create mode 100644 hw/watchdog/Kconfig
> 
> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
> new file mode 100644
> index 000..d579e32
> --- /dev/null
> +++ b/hw/watchdog/Kconfig
> @@ -0,0 +1,7 @@
> +config WDT_IB6300ESB
> +bool
> +depends on PCI
> +
> +config WDT_IB700
> +bool
> +#depends on ISA
> 

This one would have been right. :)

Paolo

Re: [Qemu-devel] [PATCH 18/47] hw/arm/Kconfig: Add ARM Kconfig

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  hw/arm/Kconfig |  235 
> 
>  1 file changed, 235 insertions(+)
>  create mode 100644 hw/arm/Kconfig
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> new file mode 100644
> index 000..c72b949
> --- /dev/null
> +++ b/hw/arm/Kconfig
> @@ -0,0 +1,235 @@
> +config ARM
> +bool
> +#select ARM_NVIC
> +default y

Why is this item needed?

Paolo

> +menu "ARM" 
> +config EXYNOS4
> +bool "Samsung Exynos4210 SoC"
> +select A9SCU # snoop controll unit
> +select USB_EHCI
> +select LAN9118
> +select PL310 # cache controller
> +select PCI
> +select ARM_GIC
> +select ARM_NVIC
> +select ARM9MPCORE
> +select ARM_MPTIMER
> +default y
> +
> +config HIGHBANK
> +bool "Calxeda Highbank SoC"
> +select A9SCU # snoop controll unit
> +select AHCI
> +select PL011 # UART
> +select PL022 # Serial port
> +select PL031 # RTC
> +select PL061 # GPIO
> +select PL310 # cache controller
> +select XGMAC # ethernet
> +select ARM_TIMER # sp804
> +select ARM_NVIC
> +select ARM_MPTIMER
> +select ARM9MPCORE
> +select PCI
> +default y
> +
> +config INTEGRATORCP
> +bool "ARM Integrator CP"
> +select SMC91C111
> +select ARM_TIMER
> +select PL011 # UART
> +select PL031 # RTC
> +select PL050 # keyboard/mouse
> +select PL110 # pl111 LCD controller
> +select PL181 # display
> +select PCI
> +default y
> +
> +config KZM
> +bool "Kzm"
> +select SERIAL
> +select IMX
> +select LAN9118
> +select PCI
> +default y
> +
> +config MUSICPAL
> +bool "Marvell MV88W8618 / Freecom MusicPal"
> +select PFLASH_CFI02
> +select PTIMER
> +select PCI
> +select BITBANG_I2C
> +select MARVELL_88W8618
> +select WM8750
> +select SERIAL
> +default y
> +
> +config OMAP
> +bool "Texas Instruments Open Multimedia Applications Platform"
> +select SERIAL
> +select PFLASH_CFI01
> +select PCI
> +default y
> +
> +config NSERIES
> +bool "Nokia N-Series tablets"
> +select TMP105   # tempature sensor
> +select BLIZZARD # LCD/TV controller
> +select ONENAND 
> +select TSC210X  # touchscreen/sensors/audio
> +select TSC2005  # touchscreen/sensors/keypad
> +select LM832X   # GPIO keyboard chip
> +select TWL92230 # energy-management
> +depends on OMAP
> +default y
> +
> +config PALM
> +bool "PalmOne PDAs"
> +select TSC210X
> +depends on OMAP
> +default y
> +
> +config STELLARIS
> +bool
> +select PL011 # UART
> +select PL022 # Serial port
> +select PL061 # GPIO
> +select STELLARIS_INPUT
> +select STELLARIS_ENET # ethernet
> +select SSD0303 # OLED display
> +select SSD0323 # OLED display
> +select SSI_SD
> +default y# for armv7m_nvic_*
> +
> +config REALVIEW
> +bool "ARM Realview baseboard"
> +# networking
> +select SMC91C111
> +select LAN9118
> +select RTL8139_PCI
> +
> +select VERSATILE_PCI
> +select WM8750 # audio codec
> +select PL011  # UART
> +select PL041  # audio codec
> +select PL050  # keyboard/mouse
> +select PL061  # GPIO
> +select PL080  # DMA controller
> +select VERSATILE_I2C
> +select DS1338 # I2C RTC+NVRAM
> +select USB_OHCI
> +select ARM_GIC
> +select ARM_MPTIMER
> +select ARM15MPCORE
> +select ARM11MPCORE
> +select ARM9MPCORE
> +
> +config VERSATILEPB
> +bool "ARM Versatile platform"
> +select PFLASH_CFI01
> +select PL031  # RTC
> +select PL050  # keyboard/mouse
> +select PL080  # DMA controller
> +select PL181  # display
> +select PL190  # Vector PIC
> +select ARM_TIMER # sp804
> +select USB_OHCI
> +select LSI_SCSI_PCI
> +select REALVIEW
> +default y
> +
> +config VXPRESS
> +bool "ARM Versatile Express"
> +select PFLASH_CFI01
> +select LAN9118
> +select PL011 # UART
> +select PL031  # RTC
> +select PL041 # audio codec
> +select PL110 # pl111 LCD controller
> +select PL181  # display
> +select PL310 # cache controller
> +select A9SCU # snoop controll unit
> +select ARM_TIMER # sp804
> +select ARM_MPTIMER
> +select ARM15MPCORE
> +select ARM9MPCORE
> +select REALVIEW
> +

Re: [Qemu-devel] [PATCH -V3 2/4] target-ppc: Fix page table lookup with kvm enabled

[Alexander Graf]

On 26.08.2013, at 07:46, Aneesh Kumar K.V wrote:

> Alexander Graf  writes:
> 
>> On 23.08.2013, at 06:20, Aneesh Kumar K.V wrote:
>> 
>>> From: "Aneesh Kumar K.V" 
>>> 
>>> With kvm enabled, we store the hash page table information in the 
>>> hypervisor.
>>> Use ioctl to read the htab contents. Without this we get the below error 
>>> when
>>> trying to read the guest address
>>> 
>>> (gdb) x/10 do_fork
>>> 0xc0098660 :   Cannot access memory at address 
>>> 0xc0098660
>>> (gdb)
>>> 
>>> Signed-off-by: Aneesh Kumar K.V 
>>> ---
>>> target-ppc/kvm.c| 45 +
>>> target-ppc/kvm_ppc.h|  9 -
>>> target-ppc/mmu-hash64.c | 25 -
>>> 3 files changed, 69 insertions(+), 10 deletions(-)
>>> 
>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>> index 6878af2..bcc6544 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -1891,3 +1891,48 @@ int kvm_arch_on_sigbus(int code, void *addr)
>>> void kvm_arch_init_irq_routing(KVMState *s)
>>> {
>>> }
>>> +
>>> +int kvmppc_hash64_load_hpte(PowerPCCPU *cpu, uint64_t index,
>>> +target_ulong *hpte0, target_ulong *hpte1)
>>> +{
>>> +int htab_fd;
>>> +struct kvm_get_htab_fd ghf;
>>> +struct kvm_get_htab_buf {
>>> +struct kvm_get_htab_header header;
>>> +/*
>>> + * Older kernel required one extra byte.
>>> + */
>>> +unsigned long hpte[3];
>>> +} hpte_buf;
>>> +
>>> +*hpte0 = 0;
>>> +*hpte1 = 0;
>>> +if (!cap_htab_fd) {
>>> +return 0;
>>> +}
>>> +/*
>>> + * At this point we are only interested in reading only bolted entries
>>> + */
>>> +ghf.flags = KVM_GET_HTAB_BOLTED_ONLY;
>>> +ghf.start_index = index;
>>> +htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
>> 
>> We should cache this.
> 
> The fd returned by KVM_PPC_GET_HTAB_FD doesn't support a proper lseek
> interface. ie, we cannot seek around to read the hpte entries at index.
> The call paths are also not in the hot path. Hence I didn't look at
> caching. We could definitely avoid doing the ioctl in loop. See below
> for the changes.
> 
> 
>> 
>>> +if (htab_fd < 0) {
>>> +return htab_fd;
>>> +}
>>> +
>>> +if (read(htab_fd, &hpte_buf, sizeof(hpte_buf)) < 0) {
>>> +goto out;
>>> +}
>>> +/*
>>> + * We only requested for one entry, So we should get only 1
>>> + * valid entry at the same index
>>> + */
>>> +if (hpte_buf.header.n_valid != 1 || hpte_buf.header.index != index) {
>>> +goto out;
>>> +}
>>> +*hpte0 = hpte_buf.hpte[0];
>>> +*hpte1 = hpte_buf.hpte[1];
>>> +out:
>>> +close(htab_fd);
>>> +return 0;
>>> +}
>>> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
>>> index 4ae7bf2..e25373a 100644
>>> --- a/target-ppc/kvm_ppc.h
>>> +++ b/target-ppc/kvm_ppc.h
>>> @@ -42,7 +42,8 @@ int kvmppc_get_htab_fd(bool write);
>>> int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns);
>>> int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
>>> uint16_t n_valid, uint16_t n_invalid);
>>> -
>>> +int kvmppc_hash64_load_hpte(PowerPCCPU *cpu, uint64_t index,
>>> +target_ulong *hpte0, target_ulong *hpte1);
>>> #else
>>> 
>>> static inline uint32_t kvmppc_get_tbfreq(void)
>>> @@ -181,6 +182,12 @@ static inline int kvmppc_load_htab_chunk(QEMUFile *f, 
>>> int fd, uint32_t index,
>>>  abort();
>>> }
>>> 
>>> +static inline int kvmppc_hash64_load_hpte(PowerPCCPU *cpu, uint64_t index,
>>> +  target_ulong *hpte0,
>>> +  target_ulong *hpte1)
>>> +{
>>> +abort();
>>> +}
>>> #endif
>>> 
>>> #ifndef CONFIG_KVM
>>> diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
>>> index 67fc1b5..4d8120c 100644
>>> --- a/target-ppc/mmu-hash64.c
>>> +++ b/target-ppc/mmu-hash64.c
>>> @@ -302,17 +302,26 @@ static int ppc_hash64_amr_prot(CPUPPCState *env, 
>>> ppc_hash_pte64_t pte)
>>>  return prot;
>>> }
>>> 
>>> -static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr pteg_off,
>>> +static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr hash,
>>>   bool secondary, target_ulong ptem,
>>>   ppc_hash_pte64_t *pte)
>>> {
>>> -hwaddr pte_offset = pteg_off;
>>> +uint64_t index;
>>> +hwaddr pte_offset;
>>>  target_ulong pte0, pte1;
>>>  int i;
>>> 
>>> +pte_offset = (hash * HASH_PTEG_SIZE_64) & env->htab_mask;;
>>> +index = (hash * HPTES_PER_GROUP) & env->htab_mask;
>>> +
>>>  for (i = 0; i < HPTES_PER_GROUP; i++) {
>>> -pte0 = ppc_hash64_load_hpte0(env, pte_offset);
>>> -pte1 = ppc_hash64_load_hpte1(env, pte_offset);
>>> +if (kvm_enabled()) {
>>> +index += i;
>>> +kvmppc_hash64_load_hpte(ppc_env_get_cpu(env), index

Re: [Qemu-devel] [PATCH 46/47] configure: Generate Kconfig.targets with --target-list

[Paolo Bonzini]

Il 26/08/2013 00:58, Ákos Kovács ha scritto:
> Signed-off-by: Ákos Kovács 
> ---
>  configure |   11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/configure b/configure
> index 18fa608..353c0cb 100755
> --- a/configure
> +++ b/configure
> @@ -4288,6 +4288,7 @@ case "$target_name" in
>;;
>sparc64)
>  TARGET_BASE_ARCH=sparc
> +kconfig_subdirs="$kconfig_subdirs sparc64"
>;;
>sparc32plus)
>  TARGET_ARCH=sparc64
> @@ -4311,6 +4312,7 @@ if [ "$TARGET_BASE_ARCH" = "" ]; then
>TARGET_BASE_ARCH=$TARGET_ARCH
>  fi
>  
> +kconfig_subdirs="$kconfig_subdirs $TARGET_BASE_ARCH"
>  symlink "$source_path/Makefile.target" "$target_dir/Makefile"
>  
>  upper() {
> @@ -4494,6 +4496,15 @@ echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak
>  
>  done # for target in $targets
>  
> +# Generate Kconfig.targets
> +kconfig_targets="Kconfig.targets"
> +kconfig_subdirs=$(echo $kconfig_subdirs | tr ' ' '\n' | sort -u | tr '\n' ' 
> ')
> +echo "# Automatically generated by configure - do not modify" > 
> $kconfig_targets
> +
> +for i in $kconfig_subdirs ; do
> +  echo "source \"hw/$i/Kconfig\"" >> $kconfig_targets
> +done
> +
>  if [ "$pixman" = "internal" ]; then
>echo "config-host.h: subdir-pixman" >> $config_host_mak
>  fi
> 

There is one issue that we have not solved yet here (pointed out by
Peter Maydell on IRC).  Right now, the presence of a file in
default-configs/ is used to check if a target name is correct.  We need
to figure out a different way to do the same thing.

This needs to cover both softmmu and user targets.  For softmmu, perhaps
there should be a hw/boards/ directory with a subdirectory for each
target.  "source hw/i386/Kconfig" (for Kconfig) or "obj-y += ../i386/"
can be used to recurse back to a common directory from there.

For linux-user, the problem is that we don't have anything to configure
with Kconfig---so no reason to have a Kconfig file or a directory
structure with one directory per target.

Paolo

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Alexander Graf]

On 26.08.2013, at 12:58, Nikunj A Dadhania wrote:

> Alexander Graf  writes:
>> Am 26.08.2013 um 05:32 schrieb Nikunj A Dadhania :
>> 
>>> Benjamin Herrenschmidt  writes:
>>> 
 On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote:
>> +vcap = &req->iu.mad.capabilities;
>> +rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer),
>> +&cap,
> be16_to_cpu(vcap->common.length));
> 
> While I don't think any harm could happen from it, this could lead to
> a potential timing attack where we read and write from different
> locations in memory if the guest swizzles the request while we're
> processing it.
 
 BTW. While I disagree with your initial comment ... is there any bound
 checking here ? That looks like potential stack corruption unless I
 miss something if the guest passes a too big length...
 
 So at least the length should be read once, bound-checked, then the read
 done with the result (don't bound check and read again, that would be
 indeed racy).
>>> 
> 
> From: Nikunj A Dadhania 
> 
> This implements capabilities exchange between host and client.
> As at the moment no capability is supported, put zero flags everywhere
> and return.
> 
> Signed-off-by: Nikunj A Dadhania 
> ---
> hw/scsi/spapr_vscsi.c | 44 
> 1 file changed, 44 insertions(+)
> 
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> index e9090e5..0758263 100644
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
> @@ -858,6 +858,47 @@ static int vscsi_send_adapter_info(VSCSIState *s, 
> vscsi_req *req)
> return vscsi_send_iu(s, req, sizeof(*sinfo), VIOSRP_MAD_FORMAT);
> }
> 
> +static int vscsi_send_capabilities(VSCSIState *s, vscsi_req *req)
> +{
> +struct viosrp_capabilities *vcap;
> +struct capabilities cap;
> +uint16_t len;
> +uint64_t buffer;
> +int rc;
> +
> +vcap = &req->iu.mad.capabilities;
> +len = be16_to_cpu(vcap->common.length);
> +buffer = be64_to_cpu(vcap->buffer);
> +if (len > sizeof(cap)) {
> +fprintf(stderr, "vscsi_send_capabilities: size out of bound !\n");
> +rc = H_PARAMETER;
> +goto error_out;
> +}
> +rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len);
> +if (rc)  {
> +fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");

At this point cap contains random host data, no?

> +}
> +
> +/*
> + * Current implementation does not suppport any migration or
> + * reservation capabilities. Construct the response telling the
> + * guest not to use them.
> + */
> +cap.flags = 0;
> +cap.migration.ecl = 0;
> +cap.reserve.type = 0;
> +cap.migration.common.server_support = 0;
> +cap.reserve.common.server_support = 0;
> +
> +rc = spapr_vio_dma_write(&s->vdev, buffer, &cap, len);
> +if (rc)  {
> +fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n");
> +}
> +error_out:

This is just "out" rather than "error_out", no? We also get here when we don't 
hit an error.


Alex

> +vcap->common.status = rc ? cpu_to_be32(1) : 0;
> +return vscsi_send_iu(s, req, sizeof(*vcap), VIOSRP_MAD_FORMAT);
> +}
> +
> static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req *req)
> {
> union mad_iu *mad = &req->iu.mad;
> @@ -878,6 +919,9 @@ static int vscsi_handle_mad_req(VSCSIState *s, vscsi_req 
> *req)
> mad->host_config.common.status = cpu_to_be16(1);
> vscsi_send_iu(s, req, sizeof(mad->host_config), VIOSRP_MAD_FORMAT);
> break;
> +case VIOSRP_CAPABILITIES_TYPE:
> +vscsi_send_capabilities(s, req);
> +break;
> default:
> fprintf(stderr, "VSCSI: Unknown MAD type %02x\n",
> be32_to_cpu(mad->empty_iu.common.type));
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [RFC 0/2] target-arm: Provide '-cpu host' when running KVM

[Gleb Natapov]

On Sun, Aug 25, 2013 at 04:11:51PM +0100, Peter Maydell wrote:
> On 25 August 2013 15:42, Gleb Natapov  wrote:
> > Are ARM cpu features not enumerable and the only way to know what is
> > supported by a core is by its id? I do see feature registers in the
> > spec, so why wouldn't we want to be able to disable/enable features
> > individually?
> 
> It's complicated. There are feature registers which have various
> bits, but only some combinations are architecturally valid. In any
> case a guest might reasonably legitimately assume "this is an A15
> so it will have architected timers", for example, and not bother
> testing feature bits.
That's sad. If feature bit is available I would expect well behaved
guest to test it. BTW is there architectural way to tell OS that it
runs as a guest? There is dedicated CPUID bit on x86 for that which is
obviously never set on real CPUs.

>   Some feature bits really do have to match the
> real hardware, like ones that say "you need to explicitly flush the
> branch predictor". A lot are feature bits that are simply mandatory
> for any ARMv7 or later processor (which is what you need for KVM
> to work at all).
What if you want to emulate older arm arch for a guest?

>   And buried in there might possibly be a few features
> that it might actually make sense to enable/disable separately (like
> "do we have Neon?").
> 
> So what we have for now is that the INIT_VCPU ioctl has space
> for feature flags (currently unused in v7; in v8 there's a "make the
> VM be a 32 bit VM on a 64 bit CPU" flag), so we could wire those
> up some day. (We don't have any support in QEMU for saying
> "I want an ARM CPU with features X and Y but not Z" yet either.)
> 
Do we have a way to ask KVM if it can support those features?

--
Gleb.

[Qemu-devel] [PATCH] curl: qemu_bh_new() can never return NULL

[Stefan Hajnoczi]

Drop error code path which cannot be taken since qemu_bh_new() does not
return NULL.

Signed-off-by: Stefan Hajnoczi 
---
 block/curl.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index e566855..ca2cedc 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -572,12 +572,6 @@ static BlockDriverAIOCB *curl_aio_readv(BlockDriverState 
*bs,
 acb->nb_sectors = nb_sectors;
 
 acb->bh = qemu_bh_new(curl_readv_bh_cb, acb);
-
-if (!acb->bh) {
-DPRINTF("CURL: qemu_bh_new failed\n");
-return NULL;
-}
-
 qemu_bh_schedule(acb->bh);
 return &acb->common;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH] Introduce RCU-enabled DQs (v2)

[Paolo Bonzini]

Il 25/08/2013 15:06, Mike Day ha scritto:
> 
> Paolo Bonzini  writes:
> 
>> Just a couple of questions, one of them on the new macro...
>>
>>> +/* prior to publication of the elm->prev->next value, some list
>>> + * readers  may still see the removed element when following
>>> + * the antecedent's next pointer.
>>> + */
>>> +
>>> +#define QLIST_REMOVE_RCU(elm, field) do {   \
>>> +if ((elm)->field.le_next != NULL) { \
>>> +   (elm)->field.le_next->field.le_prev =\
>>> +(elm)->field.le_prev;   \
>>> +}   \
>>> +atomic_rcu_set((elm)->field.le_prev, (elm)->field.le_next); \
>>> +} while (/*CONSTCOND*/0)
>>
>> Why is the barrier needed here, but not in Linux's list_del_rcu?
>>
>> I think it is not needed because all involved elements have already been
>> published and just have their pointers shuffled.
> 
> I read this as more than shuffling pointers. The intent here is 
> that the previous element's next pointer is being updated to omit the
> current element from the list.

Sorry if I were too concise... by "having their pointers shuffled" I
meant that all assigned values were already present in the list.  The
important point is that no new node has to be published in the list.

The importance of the write barrier with RCU is to ensure an item is
fully ready before it is added to a data structure.  For this to be
true, all writes to the item must be complete before a pointer to the
item is first written in memory.  Here, the pointer had already been
written in memory, so there's nothing to complete.

> atomic_set always deferences the pointer passed to it, and
> (field)->le_pre is a double pointer. So looking at the macro:
> 
> #define atomic_set(ptr, i) ((*(__typeof__(*ptr) *volatile) (ptr)) = (i))
> 
> It translates to: 
> 
> ( ( * (__typeof(*elm->field.le_prev) *volatile) (elm)->field.le_prev)  = 
> elm->field.le_next; ) 
> 
> Which is: 
> 
>  *((struct *elm) *volatile)(elm)->field.le_prev = elm->field.le_next; 
> 
> Which is:
> 
> *(elm)->field.le_prev = elm->field.le_next;
> 
> Because field.le_prev is a double pointer that has previously been set
> to &prev (the address of the previous list element) this is assiging the
> *previous* element's next pointer, the way I read it.

Correct.

> The Linux list_del_rcu is dealing with a singly linked list and
> therefore does not set a value in the previous node's element. 

Note that Linux list_head is a circular list; hlist is a singly-linked
list.  list_del_rcu still modifies the previous pointer via
__list_del_entry:

static inline void __list_del_entry(struct list_head *entry)
{
__list_del(entry->prev, entry->next);
}

static inline void __list_del(struct list_head * prev,
  struct list_head * next)
{
next->prev = prev;
prev->next = next;
}


> But I'm still unclear on whether or not the memory barrier is needed
> because the deleted element won't be reclaimed right away.

Right.  That memory barrier is not needed here, it is included in the
implementation of synchronize_rcu.

Paolo

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Nikunj A Dadhania]

Alexander Graf  writes:

> On 26.08.2013, at 12:58, Nikunj A Dadhania wrote:
>
>> This implements capabilities exchange between host and client.
>> As at the moment no capability is supported, put zero flags everywhere
>> and return.
>> 
>> Signed-off-by: Nikunj A Dadhania 
>> ---
>> +rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len);
>> +if (rc)  {
>> +fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");
>
> At this point cap contains random host data, no?

Yes, and we zero it out in this case.

>
>> +}
>> +
>> +/*
>> + * Current implementation does not suppport any migration or
>> + * reservation capabilities. Construct the response telling the
>> + * guest not to use them.
>> + */
>> +cap.flags = 0;
>> +cap.migration.ecl = 0;
>> +cap.reserve.type = 0;
>> +cap.migration.common.server_support = 0;
>> +cap.reserve.common.server_support = 0;
>> +
>> +rc = spapr_vio_dma_write(&s->vdev, buffer, &cap, len);
>> +if (rc)  {
>> +fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n");
>> +}
>> +error_out:
>
> This is just "out" rather than "error_out", no? We also get here when we 
> don't hit an error.

Yes, the label is more readable at the goto, where we set the error
return code. In case of no error return code is H_SUCCESS. So that we
send a response back to the guest.

Regards
Nikunj

Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities

[Alexander Graf]

On 26.08.2013, at 13:46, Nikunj A Dadhania wrote:

> Alexander Graf  writes:
> 
>> On 26.08.2013, at 12:58, Nikunj A Dadhania wrote:
>> 
>>> This implements capabilities exchange between host and client.
>>> As at the moment no capability is supported, put zero flags everywhere
>>> and return.
>>> 
>>> Signed-off-by: Nikunj A Dadhania 
>>> ---
>>> +rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len);
>>> +if (rc)  {
>>> +fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");
>> 
>> At this point cap contains random host data, no?
> 
> Yes, and we zero it out in this case.

Then please make this obvious to the reader. Either memset(0) it or do cap = { 
};. But do something that doesn't make me look up the header file to check 
whether you really did catch all the fields there are in the struct.

> 
>> 
>>> +}
>>> +
>>> +/*
>>> + * Current implementation does not suppport any migration or
>>> + * reservation capabilities. Construct the response telling the
>>> + * guest not to use them.
>>> + */
>>> +cap.flags = 0;
>>> +cap.migration.ecl = 0;
>>> +cap.reserve.type = 0;
>>> +cap.migration.common.server_support = 0;
>>> +cap.reserve.common.server_support = 0;
>>> +
>>> +rc = spapr_vio_dma_write(&s->vdev, buffer, &cap, len);
>>> +if (rc)  {
>>> +fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n");
>>> +}
>>> +error_out:
>> 
>> This is just "out" rather than "error_out", no? We also get here when we 
>> don't hit an error.
> 
> Yes, the label is more readable at the goto, where we set the error
> return code. In case of no error return code is H_SUCCESS. So that we
> send a response back to the guest.

... which means we're not jumping into an error-only label.


Alex
> 

Re: [Qemu-devel] [PATCH -V3 1/4] target-ppc: Update slb array with correct index values.

[Aneesh Kumar K.V]

Alexander Graf  writes:

> On 26.08.2013, at 08:46, Aneesh Kumar K.V wrote:
>
>> Alexander Graf  writes:
>> 



>>> 
 + */
 +memset(env->slb, 0, 64 * sizeof(ppc_slb_t));
>>> 
>>> Can't we use ARRAY_SIZE here and below?
>> 
>> I was thinking we want to be explicit there saying we are zeroing out all
>> the 64 entries. I could do sizeof(env->slb).
>
> Yup, that works too. In the loop below s/64/ARRAY_SIZE(env->slb)/ should work 
> too I think.
>

Updated with the above change.

-aneesh

Re: [Qemu-devel] [RFC 0/2] target-arm: Provide '-cpu host' when running KVM

[Peter Maydell]

On 26 August 2013 12:18, Gleb Natapov  wrote:
> On Sun, Aug 25, 2013 at 04:11:51PM +0100, Peter Maydell wrote:
>> A lot are feature bits that are simply mandatory
>> for any ARMv7 or later processor (which is what you need for KVM
>> to work at all).
> What if you want to emulate older arm arch for a guest?

I don't believe that was a design goal architecturally and it's
not a design goal for KVM/ARM either. If anybody actually
cares about that use case they're welcome to submit patches.
In any case, feature bits are probably the least of your worries
(in that if the guest looks at them and uses features not strictly
present on the older CPU it's still going to run, and if it doesn't
look at them then it doesn't care what the bit says); to pick
some random examples:
 * no way to make a v7 core provide v5 unaligned-access
   behaviour
 * a v6 OS is unlikely to be able to correctly manage/clean/
   invalidate the kind of cache a v7 CPU has
 * no support for the ARM v5 legacy pagetable format
There are no doubt others.

-- PMM

Re: [Qemu-devel] [PATCH -V3 2/4] target-ppc: Fix page table lookup with kvm enabled

[Aneesh Kumar K.V]

Alexander Graf  writes:

> On 26.08.2013, at 07:46, Aneesh Kumar K.V wrote:
>
>> Alexander Graf  writes:

...

>>> 
>>> This breaks PR KVM which doesn't have an HTAB fd.
>>> 
>>> I think what you want is code in kvmppc_set_papr() that tries to fetch
>>> an HTAB fd. You can then modify the check to if (kvm_enabled() &&
>>> kvmppc_has_htab_fd()), as the below case should work just fine on PR
>>> KVM.
>> 
>> As explained before caching htab fd may not really work. How about 
>> 
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index fce8835..cf6aca4 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -1892,19 +1892,24 @@ void kvm_arch_init_irq_routing(KVMState *s)
>> {
>> }
>> 
>> -int kvmppc_hash64_load_hpte(PowerPCCPU *cpu, uint64_t index,
>> -target_ulong *hpte0, target_ulong *hpte1)
>
> Against which tree is this? I don't even have this function in mine.

Sorry for the confusion, i wanted to show how it changed from the
current function. So it was done as a patch against this series. 

>
>> +hwaddr kvmppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
>> + bool secondary, target_ulong ptem,
>
> I'm having a hard time following this code. How do you tell the kernel which 
> PTE group you're interested in?
>
>> + target_ulong *hpte0, target_ulong *hpte1)
>> {
>> int htab_fd;
>> +uint64_t index;
>> +hwaddr pte_offset;
>> +target_ulong pte0, pte1;
>> struct kvm_get_htab_fd ghf;
>> struct kvm_get_htab_buf {
>> struct kvm_get_htab_header header;
>> /*
>>  * Older kernel required one extra byte.
>>  */
>> -unsigned long hpte[3];
>> +unsigned long hpte[(HPTES_PER_GROUP * 2) + 1];
>
> Does this need your kernel patch to work?

This is the updated patch.

commit f38b39855c383c55e4ad5d0a4ab617a99de134d8
Author: Aneesh Kumar K.V 
Date:   Wed Aug 7 11:34:02 2013 +0530

target-ppc: Fix page table lookup with kvm enabled

With kvm enabled, we store the hash page table information in the 
hypervisor.
Use ioctl to read the htab contents. Without this we get the below error 
when
trying to read the guest address

 (gdb) x/10 do_fork
 0xc0098660 :   Cannot access memory at address 
0xc0098660
 (gdb)

Signed-off-by: Aneesh Kumar K.V 

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 1838465..05b066c 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1888,3 +1888,62 @@ int kvm_arch_on_sigbus(int code, void *addr)
 void kvm_arch_init_irq_routing(KVMState *s)
 {
 }
+
+hwaddr kvmppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
+ bool secondary, target_ulong ptem,
+ target_ulong *hpte0, target_ulong *hpte1)
+{
+int htab_fd;
+uint64_t index;
+hwaddr pte_offset;
+target_ulong pte0, pte1;
+struct kvm_get_htab_fd ghf;
+struct kvm_get_htab_buf {
+struct kvm_get_htab_header header;
+/*
+ * Older kernel required one extra byte.
+ */
+unsigned long hpte[(HPTES_PER_GROUP * 2) + 1];
+} hpte_buf;
+
+index = (hash * HPTES_PER_GROUP) & cpu->env.htab_mask;
+*hpte0 = 0;
+*hpte1 = 0;
+if (!cap_htab_fd) {
+return 0;
+}
+
+ghf.flags = 0;
+ghf.start_index = index;
+htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
+if (htab_fd < 0) {
+goto error_out;
+}
+/*
+ * Read the hpte group
+ */
+if (read(htab_fd, &hpte_buf, sizeof(hpte_buf)) < 0) {
+goto out;
+}
+
+index = 0;
+pte_offset = (hash * HASH_PTEG_SIZE_64) & cpu->env.htab_mask;;
+while (index < hpte_buf.header.n_valid) {
+pte0 = hpte_buf.hpte[(index * 2)];
+pte1 = hpte_buf.hpte[(index * 2) + 1];
+if ((pte0 & HPTE64_V_VALID)
+&& (secondary == !!(pte0 & HPTE64_V_SECONDARY))
+&& HPTE64_V_COMPARE(pte0, ptem)) {
+*hpte0 = pte0;
+*hpte1 = pte1;
+close(htab_fd);
+return pte_offset;
+}
+index++;
+pte_offset += HASH_PTE_SIZE_64;
+}
+out:
+close(htab_fd);
+error_out:
+return -1;
+}
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 4ae7bf2..dad0e57 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -42,7 +42,9 @@ int kvmppc_get_htab_fd(bool write);
 int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns);
 int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
uint16_t n_valid, uint16_t n_invalid);
-
+hwaddr kvmppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
+ bool secondary, target_ulong ptem,
+ target_ulong *hpte0, target_ulong *hpte1);
 #else
 
 static inline uint32_t kvmppc_get_tbfreq(void)
@@ -181,6 +183,14 @@ static inline int kvmppc_

Re: [Qemu-devel] [PATCH -V3 3/4] target-ppc: Check for error on address translation in memsave command

[Aneesh Kumar K.V]

Andreas Färber  writes:

> Am 25.08.2013 20:32, schrieb Alexander Graf:
>> 
>> On 23.08.2013, at 06:20, Aneesh Kumar K.V wrote:
>> 
>>> From: "Aneesh Kumar K.V" 
>>>
>>> When we translate the virtual address to physical check for error.
>>>
>>> Signed-off-by: Aneesh Kumar K.V 
>> 
>> I think this change is sane, but I'd really prefer to see an ack from (or 
>> get this applied by) Luiz.
>> 
>> 
>> Alex
>> 
>>> ---
>>> cpus.c | 5 -
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/cpus.c b/cpus.c
>>> index 0f65e76..658366d 100644
>>> --- a/cpus.c
>>> +++ b/cpus.c
>>> @@ -1309,7 +1309,10 @@ void qmp_memsave(int64_t addr, int64_t size, const 
>>> char *filename,
>>>   l = sizeof(buf);
>>>   if (l > size)
>>>   l = size;
>>> -cpu_memory_rw_debug(cpu, addr, buf, l, 0);
>>> +if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
>>> +error_set(errp, QERR_INVALID_PARAMETER, "addr");
>
> I've been repeatedly told error_set() should no longer be used, in favor
> of error_setg(). :)
>

Updated to 

commit 23aa279e11f54808dd9f0f87b3c85e6303d00d9c
Author: Aneesh Kumar K.V 
Date:   Tue Aug 20 16:14:23 2013 +0530

target-ppc: Check for error on address translation in memsave command

When we translate the virtual address to physical check for error.

Signed-off-by: Aneesh Kumar K.V 

diff --git a/cpus.c b/cpus.c
index 0f65e76..51c38a0 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1309,7 +1309,10 @@ void qmp_memsave(int64_t addr, int64_t size, const char 
*filename,
 l = sizeof(buf);
 if (l > size)
 l = size;
-cpu_memory_rw_debug(cpu, addr, buf, l, 0);
+if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
+error_setg(errp, "Invalid addr 0x%016" PRIx64 "specified\n", addr);
+goto exit;
+}
 if (fwrite(buf, 1, l, f) != l) {
 error_set(errp, QERR_IO_ERROR);
 goto exit;

Re: [Qemu-devel] [PATCH for-1.6] isapc: disable kvmvapic

[Paolo Bonzini]

Il 13/08/2013 00:02, Paolo Bonzini ha scritto:
> vapic requires the VAPIC ROM to be mapped into RAM.  This is not
> possible without PAM hardware.  This fixes a segmentation fault
> running with -M isapc.
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Paolo Bonzini 
> ---
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a19e172..3aa244a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -743,6 +743,11 @@ static QEMUMachine isapc_machine = {
>  .init = pc_init_isa,
>  .max_cpus = 1,
>  .compat_props = (GlobalProperty[]) {
> +{
> +.driver   = "apic-common",
> +.property = "kvmvapic",
> +.value= "off",
> +},
>  { /* end of list */ }
>  },
>  DEFAULT_MACHINE_OPTIONS,
> 

Ping.

Paolo

Re: [Qemu-devel] [PATCH -V3 3/4] target-ppc: Check for error on address translation in memsave command

[Andreas Färber]

Am 26.08.2013 14:20, schrieb Aneesh Kumar K.V:
> Andreas Färber  writes:
> 
>> Am 25.08.2013 20:32, schrieb Alexander Graf:
>>>
>>> On 23.08.2013, at 06:20, Aneesh Kumar K.V wrote:
>>>
 From: "Aneesh Kumar K.V" 

 When we translate the virtual address to physical check for error.

 Signed-off-by: Aneesh Kumar K.V 
>>>
>>> I think this change is sane, but I'd really prefer to see an ack from (or 
>>> get this applied by) Luiz.
>>>
>>>
>>> Alex
>>>
 ---
 cpus.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/cpus.c b/cpus.c
 index 0f65e76..658366d 100644
 --- a/cpus.c
 +++ b/cpus.c
 @@ -1309,7 +1309,10 @@ void qmp_memsave(int64_t addr, int64_t size, const 
 char *filename,
   l = sizeof(buf);
   if (l > size)
   l = size;
 -cpu_memory_rw_debug(cpu, addr, buf, l, 0);
 +if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
 +error_set(errp, QERR_INVALID_PARAMETER, "addr");
>>
>> I've been repeatedly told error_set() should no longer be used, in favor
>> of error_setg(). :)
>>
> 
> Updated to 
> 
> commit 23aa279e11f54808dd9f0f87b3c85e6303d00d9c
> Author: Aneesh Kumar K.V 
> Date:   Tue Aug 20 16:14:23 2013 +0530
> 
> target-ppc: Check for error on address translation in memsave command
> 
> When we translate the virtual address to physical check for error.
> 
> Signed-off-by: Aneesh Kumar K.V 
> 
> diff --git a/cpus.c b/cpus.c
> index 0f65e76..51c38a0 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1309,7 +1309,10 @@ void qmp_memsave(int64_t addr, int64_t size, const 
> char *filename,
>  l = sizeof(buf);
>  if (l > size)
>  l = size;
> -cpu_memory_rw_debug(cpu, addr, buf, l, 0);
> +if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
> +error_setg(errp, "Invalid addr 0x%016" PRIx64 "specified\n", 
> addr);

Next trap is no \n please. ;)

Andreas

> +goto exit;
> +}
>  if (fwrite(buf, 1, l, f) != l) {
>  error_set(errp, QERR_IO_ERROR);
>  goto exit;
> 


-- 
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] [PATCH] spapr: support CPU hotplug

[Alexander Graf]

On 23.08.2013, at 13:30, Alexey Kardashevskiy wrote:

> PAPR+ requires two RTAS calls to be supported by the hypervisor in
> order to allow hotplugging VCPUs from the guest. The "start-cpu" RTAS
> call was already there but "stop-self" was not.
> 
> This adds the "stop-self" RTAS call.
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
> hw/ppc/spapr_rtas.c | 14 ++
> 1 file changed, 14 insertions(+)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 394ce05..8a4cfa0 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -202,6 +202,19 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, 
> sPAPREnvironment *spapr,
> rtas_st(rets, 0, -3);
> }
> 
> +static void rtas_stop_self(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> +   uint32_t token, uint32_t nargs,
> +   target_ulong args,
> +   uint32_t nret, target_ulong rets)
> +{
> +CPUState *cs = CPU(cpu);
> +CPUPPCState *env = &cpu->env;
> +
> +cs->halted = 1;
> +env->msr = 0;

So this is here to make sure we don't accidentally get out of halted state by 
an interrupt on that vcpu. Could you please somehow make that part obvious? 
Either by adding a comment or by only explicitly masking DEC and EE and a 
comment :).

> +cs->exit_request = 1;

This should probably be qemu_cpu_kick_self().


Alex

Re: [Qemu-devel] KVM guest cpu L3 cache and cpufreq

[Gleb Natapov]

On Tue, Aug 13, 2013 at 08:17:13PM +0200, Benoît Canet wrote:
> 
> Hi,
> 
> I noticed that the l3 cache size of a guest /proc/cpuinfo is not the same as
> the l3 cache size of the host.
> 
> I did not found any references to this in the qemu and KVM code.
> 
> Is the size of the guest L3 cache fixed in hardware ?
> 
No, it is hardcoded somewhere in qemu cpuid code.

> Can a patch be written to set it ?
> 
Yes.

> Similarly I noticed that the frequency in the guest was not reflecting the
> frequency scaling of the host.
> 
Not sure what you mean here. Frequency as seen where?

> Could anything be done for this ? 
> 
> Best regards
> 
> Benoît

--
Gleb.

Re: [Qemu-devel] KVM guest cpu L3 cache and cpufreq

[Benoît Canet]

Hi,

Thanks for the answer.

> On Tue, Aug 13, 2013 at 08:17:13PM +0200, Benoît Canet wrote:
> > 
> > Hi,
> > 
> > I noticed that the l3 cache size of a guest /proc/cpuinfo is not the same as
> > the l3 cache size of the host.
> > 
> > I did not found any references to this in the qemu and KVM code.
> > 
> > Is the size of the guest L3 cache fixed in hardware ?
> > 
> No, it is hardcoded somewhere in qemu cpuid code.
> 
> > Can a patch be written to set it ?
> > 
> Yes.

Ok I'll try to do that.

> 
> > Similarly I noticed that the frequency in the guest was not reflecting the
> > frequency scaling of the host.
> > 
> Not sure what you mean here. Frequency as seen where?

The frequency seen in the host /proc/cpuinfo and the guest /proc/cpuinfo differs
when a governor kick the cpu frequency scaling: the guest still show the maximum
frequency of the cpu.

Could this value be reflected dynamically ?

Best regards

Benoît

> 
> > Could anything be done for this ? 
> > 
> > Best regards
> > 
> > Benoît
> 
> --
>   Gleb.

[Qemu-devel] [PATCH 5/5] qemu-iotests: Overlapping cluster allocations

[Max Reitz]

A new test on corrupted images with overlapping cluster allocations.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/060 | 107 +
 tests/qemu-iotests/060.out |  43 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 151 insertions(+)
 create mode 100755 tests/qemu-iotests/060
 create mode 100644 tests/qemu-iotests/060.out

diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
new file mode 100755
index 000..2dd6711
--- /dev/null
+++ b/tests/qemu-iotests/060
@@ -0,0 +1,107 @@
+#!/bin/bash
+#
+# Test case for image corruption (overlapping data structures) in qcow2
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# 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 .
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qocw2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+rt_offset=65536  # 0x1 (XXX: just an assumption)
+rb_offset=131072 # 0x2 (XXX: just an assumption)
+l1_offset=196608 # 0x3 (XXX: just an assumption)
+l2_offset=262144 # 0x4 (XXX: just an assumption)
+
+IMGOPTS="compat=1.1"
+
+echo
+echo "=== Testing L2 reference into L1 ==="
+echo
+_make_test_img 64M
+# Link first L1 entry (first L2 table) onto itself
+# (Note the MSb in the L1 entry is set, ensuring the refcount is one - else any
+# later write will result in a COW operation, effectively ruining this attempt
+# on image corruption)
+poke_file "$TEST_IMG" "$l1_offset" "\x80\x00\x00\x00\x00\x03\x00\x00"
+_check_test_img
+
+# The corrupt bit should not be set anyway
+./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+
+# Try to write something, thereby forcing the corrupt bit to be set
+$QEMU_IO -c "write -P 0x2a 0 512" "$TEST_IMG" | _filter_qemu_io
+
+# The corrupt bit must now be set
+./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+
+# We could now try to fix the image, but this would probably fail (how should 
an
+# L2 table linked onto the L1 table be fixed?)
+
+echo
+echo "=== Testing cluster data reference into refcount block ==="
+echo
+_make_test_img 64M
+# Allocate L2 table
+truncate -s "$(($l2_offset+65536))" "$TEST_IMG"
+poke_file "$TEST_IMG" "$l1_offset" "\x80\x00\x00\x00\x00\x04\x00\x00"
+# Mark cluster as used
+poke_file "$TEST_IMG" "$(($rb_offset+8))" "\x00\x01"
+# Redirect new data cluster onto refcount block
+poke_file "$TEST_IMG" "$l2_offset" "\x80\x00\x00\x00\x00\x02\x00\x00"
+_check_test_img
+./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$QEMU_IO -c "write -P 0x2a 0 512" "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+
+cp $TEST_IMG foo.qcow2
+
+# Try to fix it
+_check_test_img -r all
+
+# The corrupt bit should be cleared
+./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+
+# Look if it's really really fixed
+$QEMU_IO -c "write -P 0x2a 0 512" "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
new file mode 100644
index 000..10feced
--- /dev/null
+++ b/tests/qemu-iotests/060.out
@@ -0,0 +1,43 @@
+QA output created by 060
+
+=== Testing L2 reference into L1 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+ERROR cluster 3 refcount=1 reference=3
+ERROR L2 0 overlaps with active L1 table
+
+2 errors were found on the image.
+Data may be corrupted, or further writes to the image may corrupt it.
+incompatible_features 0x0
+qcow2: Preventing invalid write on metadata; image marked as corrupt.
+write failed: Input/output error
+incompatible_features 0x2
+
+=== Testing cluster data reference into refcount block ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+ERROR refcount block 0 refcount=2
+ERROR cluster 2 refcount=1 reference=2
+ERROR data cluster 0x0 overlaps with refcount block
+
+3 errors were found on the image.
+Data may be corrupted, or further writes to the image may co

Re: [Qemu-devel] [RFC PATCH v4] powerpc: add PVR mask support

[Alexander Graf]

On 19.08.2013, at 06:06, Alexey Kardashevskiy wrote:

> IBM POWERPC processors encode PVR as a CPU family in higher 16 bits and
> a CPU version in lower 16 bits. Since there is no significant change
> in behavior between versions, there is no point to add every single CPU
> version in QEMU's CPU list. Also, new CPU versions of already supported
> CPU won't break the existing code.
> 
> This does the following:
> 1. add a PVR mask support for a CPU family (in this patch for POWER7 only);
> 2. make CPU family not abstract;
> 3. add a @pvr_default (probably bad name) - the idea when QEMU instantiates
> POWERPC CPU from a CPU family class, this value will be written to PVR
> before the guest starts; KVM uses this to pass the actual PVR value to
> the guest if QEMU was started with -cpu host.
> 
> As CPU family class name for POWER7 is "POWER7-family", there is no need
> to touch aliases.
> 
> Cc: Andreas Färber 
> Signed-off-by: Alexey Kardashevskiy 
> 
> ---
> 
> This does not pretend to be the final valid patch which can go to any tree
> (at least because it does need a POWER7+ family difinition and some bits
> for POWER8 family), it is me again asking stupid question in order to
> reduce my ignorance and get some understanding if anyone going to care of
> the PVR masks problem. Thank you for comments.
> 
> ps. :)
> ---
> Changes:
> v4:
> * removed bogus layer from hierarchy
> 
> v3:
> * renamed macros to describe the functionality better
> * added default PVR value for the powerpc cpu family (what alias used to do)
> 
> v2:
> * aliases are replaced with another level in class hierarchy
> ---
> target-ppc/cpu-models.c | 2 ++
> target-ppc/cpu-models.h | 7 +++
> target-ppc/cpu-qom.h| 2 ++
> target-ppc/kvm.c| 2 ++
> target-ppc/translate_init.c | 9 +++--
> 5 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
> index 8dea560..5ae88a5 100644
> --- a/target-ppc/cpu-models.c
> +++ b/target-ppc/cpu-models.c
> @@ -44,6 +44,8 @@
> PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);   \
> \
> pcc->pvr  = _pvr;   \
> +pcc->pvr_default  = 0;  \

There is no need for pvr_default if you limit family instantiation to -cpu 
host. Just make sure to filter out from cpu ? (and the qmp equivalent) and we 
should be good.

> +pcc->pvr_mask = CPU_POWERPC_DEFAULT_MASK;   \
> pcc->svr  = _svr;   \
> dc->desc  = _desc;  \
> }   \
> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
> index d9145d1..2233053 100644
> --- a/target-ppc/cpu-models.h
> +++ b/target-ppc/cpu-models.h
> @@ -39,6 +39,7 @@ extern PowerPCCPUAlias ppc_cpu_aliases[];
> /*/
> /* PVR definitions for most known PowerPC
> */
> enum {
> +CPU_POWERPC_DEFAULT_MASK   = 0x,
> /* PowerPC 401 family */
> /* Generic PowerPC 401 */
> #define CPU_POWERPC_401  CPU_POWERPC_401G2
> @@ -557,6 +558,12 @@ enum {
> CPU_POWERPC_POWER7_v23 = 0x003F0203,
> CPU_POWERPC_POWER7P_v21= 0x004A0201,
> CPU_POWERPC_POWER8_v10 = 0x004B0100,
> +CPU_POWERPC_POWER7 = 0x003F,
> +CPU_POWERPC_POWER7_MASK= 0x,
> +CPU_POWERPC_POWER7P= 0x004A,
> +CPU_POWERPC_POWER7P_MASK   = 0x,
> +CPU_POWERPC_POWER8 = 0x004B,
> +CPU_POWERPC_POWER8_MASK= 0x,

Please add support for all the other CPUs supported by PR and HV KVM at least 
on Book3S, but preferably everything Linux supports once this patch is out of 
RFC.

> CPU_POWERPC_970= 0x00390202,
> CPU_POWERPC_970FX_v10  = 0x00391100,
> CPU_POWERPC_970FX_v20  = 0x003C0200,
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index f3c710a..a1a712c 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -54,6 +54,8 @@ typedef struct PowerPCCPUClass {
> void (*parent_reset)(CPUState *cpu);
> 
> uint32_t pvr;
> +uint32_t pvr_default;
> +uint32_t pvr_mask;
> uint32_t svr;
> uint64_t insns_flags;
> uint64_t insns_flags2;
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 30a870e..315e499 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1731,6 +1731,8 @@ static void kvmppc_host_cpu_class_init(ObjectClass *oc, 
> void *data)
> uint32_t dcache_size = kvmppc_read_int_cpu_dt("d-cache-size");
> uint32_t icache_size = kvmppc_read_int_cpu_dt

[Qemu-devel] [PATCH 4/5] qcow2: Check allocations in qcow2_check

[Max Reitz]

Adds a new function checking for overlapping cluster allocations.
Furthermore, qcow2_check now marks the image as consistent if no
corruptions have been found.

Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 414 ++
 block/qcow2.c |  15 +-
 block/qcow2.h |   2 +
 3 files changed, 429 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index be35983..ea7d334 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1499,3 +1499,417 @@ fail:
 
 return ret;
 }
+
+static const char *const overlap_strings[8] = {
+"main header",
+"active L1 table",
+"active L2 table",
+"refcount table",
+"refcount block",
+"snapshot table",
+"inactive L1 table",
+"inactive L2 table"
+};
+
+/*
+ * Checks the table and cluster allocations for inconsistencies.
+ */
+int qcow2_check_allocations(BlockDriverState *bs, BdrvCheckResult *result,
+BdrvCheckMode fix)
+{
+BDRVQcowState *s = bs->opaque;
+int ret;
+int i, j;
+uint8_t *bm; /* bitmap */
+/* bitmap byte shift (bytes per bitmap-bit = 1 << bm_shift) */
+int bm_shift = s->cluster_bits;
+uint64_t bm_size;
+
+/* Checking every data cluster through qcow2_check_metadata_overlap takes a
+ * considerable amount of time; a bitmap would help out very much, although
+ * it quickly assumes ridiculous sizes if not limited; therefore we're
+ * grouping several clusters together to not exceed the size of the L1 
table
+ * in RAM (which seems a reasonable limit). */
+while ((bm_size = bs->total_sectors >> (bm_shift++ - BDRV_SECTOR_BITS)) >
+s->l1_size * sizeof(uint64_t));
+
+bm = g_malloc0(++bm_size);
+
+bm[0] = 1; /* main header */
+/* active L1 */
+for (i = 0; i < s->l1_size * sizeof(uint64_t) >> bm_shift; i++) {
+uintptr_t bit = i + (s->l1_table_offset >> bm_shift);
+if (bit / 8 >= bm_size) {
+fprintf(stderr, "ERROR L1 table offset is after EOF\n");
+result->corruptions++;
+/* this is such a fundamental error that it will probably crash
+ * qcow2_check_metadata_overlap if this function doesn't return
+ * already here (qcow2_check_metadata_overlap is for keeping
+ * consistency, therefore it always assumes a pretty consistent
+ * image) */
+ret = 0;
+goto fail;
+}
+bm[bit / 8] |= 1 << (bit % 8);
+}
+
+if (s->l1_table) {
+/* active L2 */
+for (i = 0; i < s->l1_size; i++) {
+uintptr_t bit = (s->l1_table[i] & L1E_OFFSET_MASK) >> bm_shift;
+if (bit / 8 >= bm_size) {
+fprintf(stderr, "ERROR L2 table %i offset is after EOF\n", i);
+result->corruptions++;
+ret = 0;
+goto fail;
+}
+/* the test for j != 0 is unneccessary, since that bit is set
+ * anyway (because of the main header) */
+bm[bit / 8] |= 1 << (bit % 8);
+}
+}
+
+/* snapshot table */
+for (i = 0; i < s->snapshots_size >> bm_shift; i++) {
+uintptr_t bit = i + (s->snapshots_offset >> bm_shift);
+if (bit / 8 >= bm_size) {
+fprintf(stderr, "ERROR snapshot table offset is after EOF\n");
+result->corruptions++;
+ret = 0;
+goto fail;
+}
+bm[bit / 8] |= 1 << (bit % 8);
+}
+
+if (s->snapshots) {
+/* inactive L1 */
+for (i = 0; i < s->nb_snapshots; i++) {
+for (j = 0; j < s->snapshots[i].l1_size * sizeof(uint64_t) >>
+bm_shift; j++) {
+uintptr_t bit =
+j + (s->snapshots[i].l1_table_offset >> bm_shift);
+if (bit / 8 >= bm_size) {
+fprintf(stderr, "ERROR inactive L1 table %i offset is "
+"after EOF\n", i);
+result->corruptions++;
+ret = 0;
+goto fail;
+}
+bm[bit / 8] |= 1 << (bit % 8);
+}
+}
+}
+
+/* refcount table */
+for (i = 0; i < s->refcount_table_size * sizeof(uint64_t) >> bm_shift; i++)
+{
+uintptr_t bit = i + (s->refcount_table_offset >> bm_shift);
+if (bit / 8 >= bm_size) {
+fprintf(stderr, "ERROR refcount table offset is after EOF\n");
+result->corruptions++;
+ret = 0;
+goto fail;
+}
+bm[bit / 8] |= 1 << (bit % 8);
+}
+
+if (s->refcount_table) {
+int initial_corruptions = result->corruptions;
+
+/* Do the refcount blocks last; that way, we can directly use the 
bitmap
+ * to check for overlaps */
+for (i = 0; i < s->refcount_table_size; i++) {
+uint64_t rb_offset = s->refcount_table[i] & 

[Qemu-devel] [PATCH 2/5] qcow2: Metadata overlap checks

[Max Reitz]

Two new functions are added; the first one checks a given range in the
image file for overlaps with metadata (main header, L1 tables, L2
tables, refcount table and blocks).

The second one should be used immediately before writing to the image
file as it calls the first function and, upon collision, marks the
image as corrupt and makes the BDS unusable, thereby preventing
further access.

Both functions take a bitmask argument specifying the structures which
should be checked for overlaps, making it possible to also check
metadata writes against colliding with other structures.

Signed-off-by: Max Reitz 
---
 block/qcow2-refcount.c | 142 +
 block/qcow2.h  |  28 ++
 2 files changed, 170 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 1244693..c8141c8 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -25,6 +25,7 @@
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "block/qcow2.h"
+#include "qemu/range.h"
 
 static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size);
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
@@ -1372,3 +1373,144 @@ fail:
 return ret;
 }
 
+/*
+ * Checks if the given offset into the image file is actually free to use by
+ * looking for overlaps with important metadata sections (L1/L2 tables etc.),
+ * i.e. a sanity check without relying on the refcount tables.
+ *
+ * The chk parameter specifies exactly what checks to perform.
+ *
+ * Returns:
+ * - 0 if writing to this offset will not affect the mentioned metadata
+ * - a positive QCow2MetadataOverlap value indicating one overlapping section
+ * - a negative value (-errno) indicating an error while performing a check,
+ *   e.g. when bdrv_read failed on QCOW2_OL_INACTIVE_L2
+ */
+int qcow2_check_metadata_overlap(BlockDriverState *bs, QCow2MetadataOverlap 
chk,
+ int64_t offset, int64_t size)
+{
+BDRVQcowState *s = bs->opaque;
+int i, j;
+
+if (!size) {
+return 0;
+}
+
+if (chk & QCOW2_OL_MAIN_HEADER) {
+if (offset < s->cluster_size) {
+return QCOW2_OL_MAIN_HEADER;
+}
+}
+
+if ((chk & QCOW2_OL_ACTIVE_L1) && s->l1_size) {
+if (ranges_overlap(offset, size, s->l1_table_offset,
+s->l1_size * sizeof(uint64_t))) {
+return QCOW2_OL_ACTIVE_L1;
+}
+}
+
+if ((chk & QCOW2_OL_REFCOUNT_TABLE) && s->refcount_table_size) {
+if (ranges_overlap(offset, size, s->refcount_table_offset,
+s->refcount_table_size * sizeof(uint64_t))) {
+return QCOW2_OL_REFCOUNT_TABLE;
+}
+}
+
+if ((chk & QCOW2_OL_SNAPSHOT_TABLE) && s->snapshots_size) {
+if (ranges_overlap(offset, size, s->snapshots_offset,
+s->snapshots_size)) {
+return QCOW2_OL_SNAPSHOT_TABLE;
+}
+}
+
+if ((chk & QCOW2_OL_INACTIVE_L1) && s->snapshots) {
+for (i = 0; i < s->nb_snapshots; i++) {
+if (s->snapshots[i].l1_size &&
+ranges_overlap(offset, size, s->snapshots[i].l1_table_offset,
+s->snapshots[i].l1_size * sizeof(uint64_t))) {
+return QCOW2_OL_INACTIVE_L1;
+}
+}
+}
+
+if ((chk & QCOW2_OL_ACTIVE_L2) && s->l1_table) {
+for (i = 0; i < s->l1_size; i++) {
+if ((s->l1_table[i] & L1E_OFFSET_MASK) &&
+ranges_overlap(offset, size, s->l1_table[i] & L1E_OFFSET_MASK,
+s->cluster_size)) {
+return QCOW2_OL_ACTIVE_L2;
+}
+}
+}
+
+if ((chk & QCOW2_OL_REFCOUNT_BLOCK) && s->refcount_table) {
+for (i = 0; i < s->refcount_table_size; i++) {
+if ((s->refcount_table[i] & REFT_OFFSET_MASK) &&
+ranges_overlap(offset, size, s->refcount_table[i] &
+REFT_OFFSET_MASK, s->cluster_size)) {
+return QCOW2_OL_REFCOUNT_BLOCK;
+}
+}
+}
+
+if ((chk & QCOW2_OL_INACTIVE_L2) && s->snapshots) {
+for (i = 0; i < s->nb_snapshots; i++) {
+uint64_t l1_ofs = s->snapshots[i].l1_table_offset;
+uint32_t l1_sz  = s->snapshots[i].l1_size;
+uint64_t *l1 = g_malloc(l1_sz * sizeof(uint64_t));
+int ret;
+
+ret = bdrv_read(bs->file, l1_ofs / BDRV_SECTOR_SIZE, (uint8_t *)l1,
+l1_sz * sizeof(uint64_t) / BDRV_SECTOR_SIZE);
+
+if (ret < 0) {
+g_free(l1);
+return ret;
+}
+
+for (j = 0; j < l1_sz; j++) {
+if ((l1[j] & L1E_OFFSET_MASK) &&
+ranges_overlap(offset, size, l1[j] & L1E_OFFSET_MASK,
+s->cluster_size)) {
+g_free(l1);
+return QCOW2_OL_INACTIVE_L2;
+}
+}
+
+g_free(l1);
+ 

[Qemu-devel] [PATCH 1/5] qcow2: Add corrupt bit

[Max Reitz]

This adds an incompatible bit indicating corruption to qcow2. Any image
with this bit set may not be written to unless for repairing (and
subsequently clearing the bit if the repair has been successful).

Signed-off-by: Max Reitz 
---
 block/qcow2.c  | 44 
 block/qcow2.h  |  7 ++-
 docs/specs/qcow2.txt   |  7 ++-
 include/block/block.h  |  2 ++
 qemu-img.c |  2 +-
 tests/qemu-iotests/031.out | 12 ++--
 tests/qemu-iotests/036.out |  2 +-
 7 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 3376901..1d0d7ca 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -272,6 +272,37 @@ static int qcow2_mark_clean(BlockDriverState *bs)
 return 0;
 }
 
+/*
+ * Marks the image as corrupt.
+ */
+int qcow2_mark_corrupt(BlockDriverState *bs)
+{
+BDRVQcowState *s = bs->opaque;
+
+s->incompatible_features |= QCOW2_INCOMPAT_CORRUPT;
+return qcow2_update_header(bs);
+}
+
+/*
+ * Marks the image as consistent, i.e., unsets the corrupt bit, and flushes
+ * before if necessary.
+ */
+int qcow2_mark_consistent(BlockDriverState *bs)
+{
+BDRVQcowState *s = bs->opaque;
+
+if (s->incompatible_features & QCOW2_INCOMPAT_CORRUPT) {
+int ret = bdrv_flush(bs);
+if (ret < 0) {
+return ret;
+}
+
+s->incompatible_features &= ~QCOW2_INCOMPAT_CORRUPT;
+return qcow2_update_header(bs);
+}
+return 0;
+}
+
 static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
BdrvCheckMode fix)
 {
@@ -402,6 +433,14 @@ static int qcow2_open(BlockDriverState *bs, QDict 
*options, int flags)
 goto fail;
 }
 
+if (s->incompatible_features & QCOW2_INCOMPAT_CORRUPT) {
+/* Corrupt images may not be written to unless they are being repaired 
*/
+if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_REPAIR)) {
+ret = -EACCES;
+goto fail;
+}
+}
+
 /* Check support for various header values */
 if (header.refcount_order != 4) {
 report_unsupported(bs, "%d bit reference counts",
@@ -1130,6 +1169,11 @@ int qcow2_update_header(BlockDriverState *bs)
 .name = "dirty bit",
 },
 {
+.type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
+.bit  = QCOW2_INCOMPAT_CORRUPT_BITNR,
+.name = "corrupt bit",
+},
+{
 .type = QCOW2_FEAT_TYPE_COMPATIBLE,
 .bit  = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR,
 .name = "lazy refcounts",
diff --git a/block/qcow2.h b/block/qcow2.h
index dba9771..4297487 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -119,9 +119,12 @@ enum {
 /* Incompatible feature bits */
 enum {
 QCOW2_INCOMPAT_DIRTY_BITNR   = 0,
+QCOW2_INCOMPAT_CORRUPT_BITNR = 1,
 QCOW2_INCOMPAT_DIRTY = 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
+QCOW2_INCOMPAT_CORRUPT   = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR,
 
-QCOW2_INCOMPAT_MASK  = QCOW2_INCOMPAT_DIRTY,
+QCOW2_INCOMPAT_MASK  = QCOW2_INCOMPAT_DIRTY
+ | QCOW2_INCOMPAT_CORRUPT,
 };
 
 /* Compatible feature bits */
@@ -361,6 +364,8 @@ int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector 
*qiov,
   int64_t sector_num, int nb_sectors);
 
 int qcow2_mark_dirty(BlockDriverState *bs);
+int qcow2_mark_corrupt(BlockDriverState *bs);
+int qcow2_mark_consistent(BlockDriverState *bs);
 int qcow2_update_header(BlockDriverState *bs);
 
 /* qcow2-refcount.c functions */
diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 36a559d..33eca36 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -80,7 +80,12 @@ in the description of a field.
 tables to repair refcounts before accessing the
 image.
 
-Bits 1-63:  Reserved (set to 0)
+Bit 1:  Corrupt bit.  If this bit is set then any data
+structure may be corrupt and the image must not
+be written to (unless for regaining
+consistency).
+
+Bits 2-63:  Reserved (set to 0)
 
  80 -  87:  compatible_features
 Bitmask of compatible features. An implementation can
diff --git a/include/block/block.h b/include/block/block.h
index 742fce5..7145bf0 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -74,6 +74,8 @@ typedef struct BlockDevOps {
 #define BDRV_O_CHECK   0x1000  /* open solely for consistency check */
 #define BDRV_O_ALLOW_RDWR  0x2000  /* allow reopen to change from r/o to r/w */
 #define BDRV_O_UNMAP   0x4000  /* execute guest UNMAP/TRIM operations */
+#define BDRV_O_REPAIR  0x8000  /* assure any writes are intended only for
+  repairs (important for cor

[Qemu-devel] [PATCH 3/5] qcow2: Employ metadata overlap checks

[Max Reitz]

The pre-write overlap check function is now called before most of the
qcow2 writes (aborting it on collision or other error).

Signed-off-by: Max Reitz 
---
 block/qcow2-cache.c| 17 +
 block/qcow2-cluster.c  | 23 +++
 block/qcow2-snapshot.c | 24 
 block/qcow2.c  | 38 +-
 4 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 2f3114e..da65297 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -115,6 +115,23 @@ static int qcow2_cache_entry_flush(BlockDriverState *bs, 
Qcow2Cache *c, int i)
 }
 
 if (c == s->refcount_block_cache) {
+ret = qcow2_pre_write_overlap_check(bs,
+QCOW2_OL_DEFAULT & ~QCOW2_OL_REFCOUNT_BLOCK,
+c->entries[i].offset, s->cluster_size);
+} else if (c == s->l2_table_cache) {
+ret = qcow2_pre_write_overlap_check(bs,
+QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L2,
+c->entries[i].offset, s->cluster_size);
+} else {
+ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
+c->entries[i].offset, s->cluster_size);
+}
+
+if (ret) {
+return (ret < 0) ? ret : -EIO;
+}
+
+if (c == s->refcount_block_cache) {
 BLKDBG_EVENT(bs->file, BLKDBG_REFBLOCK_UPDATE_PART);
 } else if (c == s->l2_table_cache) {
 BLKDBG_EVENT(bs->file, BLKDBG_L2_UPDATE);
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index cca76d4..be35983 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -80,6 +80,15 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t 
min_size,
 goto fail;
 }
 
+/* the L1 position has not yet been updated, so these clusters must
+ * indeed be completely free */
+ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
+new_l1_table_offset, new_l1_size2);
+if (ret) {
+ret = (ret < 0) ? ret : -EIO;
+goto fail;
+}
+
 BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_WRITE_TABLE);
 for(i = 0; i < s->l1_size; i++)
 new_l1_table[i] = cpu_to_be64(new_l1_table[i]);
@@ -149,6 +158,13 @@ static int write_l1_entry(BlockDriverState *bs, int 
l1_index)
 buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]);
 }
 
+ret = qcow2_pre_write_overlap_check(bs,
+QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L1,
+s->l1_table_offset + 8 * l1_start_index, sizeof(buf));
+if (ret) {
+return (ret < 0) ? ret : -EIO;
+}
+
 BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);
 ret = bdrv_pwrite_sync(bs->file, s->l1_table_offset + 8 * l1_start_index,
 buf, sizeof(buf));
@@ -368,6 +384,13 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
 &s->aes_encrypt_key);
 }
 
+ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
+((cluster_offset >> 9) + n_start) << 9, n * BDRV_SECTOR_SIZE);
+if (ret) {
+ret = (ret < 0) ? ret : -EIO;
+goto out;
+}
+
 BLKDBG_EVENT(bs->file, BLKDBG_COW_WRITE);
 ret = bdrv_co_writev(bs->file, (cluster_offset >> 9) + n_start, n, &qiov);
 if (ret < 0) {
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 0caac90..6f69ecc 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -189,6 +189,15 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 return ret;
 }
 
+/* The snapshot list position has not yet been updated, so these clusters
+ * must indeed be completely free */
+ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
+offset, s->nb_snapshots * sizeof(h));
+if (ret) {
+return (ret < 0) ? ret : -EIO;
+}
+
+
 /* Write all snapshots to the new list */
 for(i = 0; i < s->nb_snapshots; i++) {
 sn = s->snapshots + i;
@@ -363,6 +372,13 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 l1_table[i] = cpu_to_be64(s->l1_table[i]);
 }
 
+ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
+sn->l1_table_offset, s->l1_size * sizeof(uint64_t));
+if (ret) {
+ret = (ret < 0) ? ret : -EIO;
+goto fail;
+}
+
 ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table,
   s->l1_size * sizeof(uint64_t));
 if (ret < 0) {
@@ -475,6 +491,14 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char 
*snapshot_id)
 goto fail;
 }
 
+ret = qcow2_pre_write_overlap_check(bs,
+QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L1,
+s->l1_table_offset, cur_l1_bytes);
+if (ret) {
+ret = (ret < 0) ? ret : -EIO;
+goto fail;
+}
+
 ret = bdrv_pwrite_sync(bs->file, s->l1_table_offset, sn_l1_table,
cur_l1_bytes);
 if (ret < 0) {
diff

Re: [Qemu-devel] [PATCH v3 1/8] target-ppc: Add helper for KVM_PPC_RTAS_DEFINE_TOKEN

[Alexander Graf]

On 19.08.2013, at 07:55, Alexey Kardashevskiy wrote:

> From: David Gibson 
> 
> Recent PowerKVM allows the kernel to intercept some RTAS calls from the
> guest directly.  This is used to implement the more efficient in-kernel
> XICS for example.  qemu is still responsible for assigning the RTAS token
> numbers however, and needs to tell the kernel which RTAS function name is
> assigned to a given token value.  This patch adds a convenience wrapper for
> the KVM_PPC_RTAS_DEFINE_TOKEN ioctl() which is used for this purpose.
> 
> Signed-off-by: David Gibson 
> Signed-off-by: Alexey Kardashevskiy 
> ---
> target-ppc/kvm.c | 14 ++
> target-ppc/kvm_ppc.h |  7 +++
> 2 files changed, 21 insertions(+)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index eea8c09..ab46aaa 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1792,6 +1792,20 @@ static int kvm_ppc_register_host_cpu_type(void)
> return 0;
> }
> 
> +int kvmppc_define_rtas_token(uint32_t token, const char *function)

The naming here is slightly confusing. What the ioctl does is define an rtas 
token that gets handled in-kernel. The name of the function should reflect 
this. How about something like kvmppc_define_rtas_in_kernel()?


Alex

> +{
> +struct kvm_rtas_token_args args = {
> +.token = token,
> +};
> +
> +if (!kvm_check_extension(kvm_state, KVM_CAP_PPC_RTAS)) {
> +return -ENOENT;
> +}
> +
> +strncpy(args.name, function, sizeof(args.name));
> +
> +return kvm_vm_ioctl(kvm_state, KVM_PPC_RTAS_DEFINE_TOKEN, &args);
> +}
> 
> int kvmppc_get_htab_fd(bool write)
> {
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 4ae7bf2..12564ef 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -38,6 +38,7 @@ uint64_t kvmppc_rma_size(uint64_t current_size, unsigned 
> int hash_shift);
> #endif /* !CONFIG_USER_ONLY */
> int kvmppc_fixup_cpu(PowerPCCPU *cpu);
> bool kvmppc_has_cap_epr(void);
> +int kvmppc_define_rtas_token(uint32_t token, const char *function);
> int kvmppc_get_htab_fd(bool write);
> int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns);
> int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
> @@ -164,6 +165,12 @@ static inline bool kvmppc_has_cap_epr(void)
> return false;
> }
> 
> +static inline int kvmppc_define_rtas_token(uint32_t token,
> +   const char *function)
> +{
> +return -1;
> +}
> +
> static inline int kvmppc_get_htab_fd(bool write)
> {
> return -1;
> -- 
> 1.8.3.2
> 

[Qemu-devel] [PATCH 0/5] qcow2: Add metadata overlap checks

[Max Reitz]

If a qcow2 image file becomes corrupted, any write may inadvertently
overwrite important metadata structures such as the L1 table. This
series adds functionality for detecting, preventing and (to some
extent) repairing such collisions.

Max Reitz (5):
  qcow2: Add corrupt bit
  qcow2: Metadata overlap checks
  qcow2: Employ metadata overlap checks
  qcow2: Check allocations in qcow2_check
  qemu-iotests: Overlapping cluster allocations

 block/qcow2-cache.c|  17 +
 block/qcow2-cluster.c  | 437 

 block/qcow2-refcount.c | 142 ++
 block/qcow2-snapshot.c |  24 +++
 block/qcow2.c  |  97 +-
 block/qcow2.h  |  37 +-
 docs/specs/qcow2.txt   |   7 +-
 include/block/block.h  |   2 +
 qemu-img.c |   2 +-
 tests/qemu-iotests/031.out |  12 ++--
 tests/qemu-iotests/036.out |   2 +-
 tests/qemu-iotests/060 | 107 +
 tests/qemu-iotests/060.out |  43 
 tests/qemu-iotests/group   |   1 +
 14 files changed, 917 insertions(+), 13 deletions(-)
 create mode 100755 tests/qemu-iotests/060
 create mode 100644 tests/qemu-iotests/060.out

Re: [Qemu-devel] [PATCH 4/5] qcow2: Check allocations in qcow2_check

[Max Reitz]

Hi,

On Mon, Aug 26, 2013 at 15:04 +0200, Max Reitz wrote:

Adds a new function checking for overlapping cluster allocations.
Furthermore, qcow2_check now marks the image as consistent if no
corruptions have been found.

Signed-off-by: Max Reitz 
Such overlappings are often (if not always) found by the refcount checks 
as well. By increasing the refcount of the clusters affected and 
therefore enforcing COW, the collision is basically fixed as well 
without requiring this new function.


Thus I'm not sure whether this new function is actually necessary since 
the existing refcount check seems to cover all its functionality already.


But, well, I thought it couldn't hurt proposing this function at least. ;-)


Kind regards,

Max

Re: [Qemu-devel] [PATCH for-1.6? v2 21/21] qtest: Prepare QOM machine tests

[Luiz Capitulino]

On Sun, 25 Aug 2013 19:42:56 +0200
Andreas Färber  wrote:

> Am 25.08.2013 19:33, schrieb Peter Maydell:
> > On 5 August 2013 14:27, Andreas Färber  wrote:
> >> +static const char *arm_machines[] = {
> >> +"integratorcp",
> >> +"versatilepb",
> >> +"versatileab",
> >> +"lm3s811evb",
> >> +"lm3s6965evb",
> >> +"collie",
> > 
> > [etc]
> > 
> > I'd rather have these tests than not have them, but can't we autodetect
> > the list of machines (by parsing the output of -M help, or otherwise)?
> 
> If you want to implement that, feel free to. :) We neither have working
> QMP parsing nor a QMP method for obtaining machines that I'm aware of.

Not sure what the context is here, but we have query-machines. It's a
bit simple though. We also have query-cpu-definitions.

Re: [Qemu-devel] [PATCH v3 2/8] xics: add pre_save/post_load/cpu_setup dispatchers

[Alexander Graf]

On 19.08.2013, at 07:55, Alexey Kardashevskiy wrote:

> The upcoming support of in-kernel XICS will redefine migration callbacks
> for both ICS and ICP so classes and callback pointers are added.
> 
> This adds a cpu_setup callback to the XICS device class (as XICS-KVM
> will do it different) and xics_dispatch_cpu_setup(). This also moves
> the place where xics_dispatch_cpu_setup() is called a bit further to
> have VCPU initialized (required for XICS-KVM).
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
> Changes:
> v3:
> * fixed local variables names
> ---
> hw/intc/xics.c| 61 +++
> hw/ppc/spapr.c|  4 ++--
> include/hw/ppc/xics.h | 46 +-
> 3 files changed, 104 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 6b3c071..e3a957d 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -153,11 +153,35 @@ static void icp_irq(XICSState *icp, int server, int nr, 
> uint8_t priority)
> }
> }
> 
> +static void icp_dispatch_pre_save(void *opaque)
> +{
> +ICPState *ss = opaque;
> +ICPStateClass *info = ICP_GET_CLASS(ss);
> +
> +if (info->pre_save) {
> +info->pre_save(ss);
> +}
> +}
> +
> +static int icp_dispatch_post_load(void *opaque, int version_id)
> +{
> +ICPState *ss = opaque;
> +ICPStateClass *info = ICP_GET_CLASS(ss);
> +
> +if (info->post_load) {
> +return info->post_load(ss, version_id);
> +}
> +
> +return 0;
> +}
> +
> static const VMStateDescription vmstate_icp_server = {
> .name = "icp/server",
> .version_id = 1,
> .minimum_version_id = 1,
> .minimum_version_id_old = 1,
> +.pre_save = icp_dispatch_pre_save,
> +.post_load = icp_dispatch_post_load,
> .fields  = (VMStateField []) {
> /* Sanity check */
> VMSTATE_UINT32(xirr, ICPState),
> @@ -192,6 +216,7 @@ static TypeInfo icp_info = {
> .parent = TYPE_DEVICE,
> .instance_size = sizeof(ICPState),
> .class_init = icp_class_init,
> +.class_size = sizeof(ICPStateClass),
> };
> 
> /*
> @@ -353,10 +378,9 @@ static void ics_reset(DeviceState *dev)
> }
> }
> 
> -static int ics_post_load(void *opaque, int version_id)
> +static int ics_post_load(ICSState *ics, int version_id)
> {
> int i;
> -ICSState *ics = opaque;
> 
> for (i = 0; i < ics->icp->nr_servers; i++) {
> icp_resend(ics->icp, i);
> @@ -365,6 +389,28 @@ static int ics_post_load(void *opaque, int version_id)
> return 0;
> }
> 
> +static void ics_dispatch_pre_save(void *opaque)
> +{
> +ICSState *ics = opaque;
> +ICSStateClass *info = ICS_GET_CLASS(ics);
> +
> +if (info->pre_save) {
> +info->pre_save(ics);
> +}
> +}
> +
> +static int ics_dispatch_post_load(void *opaque, int version_id)
> +{
> +ICSState *ics = opaque;
> +ICSStateClass *info = ICS_GET_CLASS(ics);
> +
> +if (info->post_load) {
> +return info->post_load(ics, version_id);
> +}
> +
> +return 0;
> +}
> +
> static const VMStateDescription vmstate_ics_irq = {
> .name = "ics/irq",
> .version_id = 1,
> @@ -384,7 +430,8 @@ static const VMStateDescription vmstate_ics = {
> .version_id = 1,
> .minimum_version_id = 1,
> .minimum_version_id_old = 1,
> -.post_load = ics_post_load,
> +.pre_save = ics_dispatch_pre_save,
> +.post_load = ics_dispatch_post_load,
> .fields  = (VMStateField []) {
> /* Sanity check */
> VMSTATE_UINT32_EQUAL(nr_irqs, ICSState),
> @@ -409,10 +456,12 @@ static int ics_realize(DeviceState *dev)
> static void ics_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> +ICSStateClass *isc = ICS_CLASS(klass);
> 
> dc->init = ics_realize;
> dc->vmsd = &vmstate_ics;
> dc->reset = ics_reset;
> +isc->post_load = ics_post_load;
> }
> 
> static TypeInfo ics_info = {
> @@ -420,6 +469,7 @@ static TypeInfo ics_info = {
> .parent = TYPE_DEVICE,
> .instance_size = sizeof(ICSState),
> .class_init = ics_class_init,
> +.class_size = sizeof(ICSStateClass),
> };
> 
> /*
> @@ -612,7 +662,7 @@ static void xics_reset(DeviceState *d)
> device_reset(DEVICE(icp->ics));
> }
> 
> -void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
> +static void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
> {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> @@ -674,10 +724,12 @@ static Property xics_properties[] = {
> static void xics_class_init(ObjectClass *oc, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(oc);
> +XICSStateClass *k = XICS_CLASS(oc);
> 
> dc->realize = xics_realize;
> dc->props = xics_properties;
> dc->reset = xics_reset;
> +k->cpu_setup = xics_cpu_setup;
> 
> spapr_rtas_register("ibm,set-xive", rtas_set_xive);
> spapr_rtas_register("ibm,get-xive", rtas_get_xive);
> @@ -694,6 +746,7 @@ static const TypeInfo xics_info = {
> .name