Re: [Qemu-devel] [RFC v1 19/25] sysbus: Setup memory regions as dynamic props

[Peter Crosthwaite]

On Fri, May 16, 2014 at 12:00 PM, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
 Dynamically allocate Memory Region pointers and set them up as QOM
 links. Plug these dynamic links to the sysbus_init_mmio() and
 sysbus_mmio_get_region APIs.

 This allows for removal of the Sysbus Memory Regions as state. All
 that's needed now is the counter for total number of regions. Another
 piece of SysBus state bites the dust!

 This also removes the artificial limit of 32 memory regions per device.
 The number of memory regions is now practically unbounded (unless you
 cause a wrap on the total counter).

 Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
 ---

  hw/core/sysbus.c| 29 ++---
  include/hw/sysbus.h |  4 
  2 files changed, 18 insertions(+), 15 deletions(-)

 diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
 index 7cdc428..6858336 100644
 --- a/hw/core/sysbus.c
 +++ b/hw/core/sysbus.c
 @@ -50,7 +50,7 @@ static void sysbus_mmio_map_common(SysBusDevice *dev, int 
 n, hwaddr addr,
  MemoryRegion *mr;
  assert(n = 0  n  dev-num_mmio);

 -mr = dev-mmio[n].memory;
 +mr = sysbus_mmio_get_region(dev, n);

  object_property_set_link(OBJECT(mr), OBJECT(get_system_memory()),
   container, error_abort);
 @@ -85,16 +85,22 @@ void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice 
 *target)

  void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
  {
 -int n;
 +char *propname = g_strdup_printf(sysbus-mr-%d, dev-num_mmio++);

 -assert(dev-num_mmio  QDEV_MAX_MMIO);
 -n = dev-num_mmio++;
 -dev-mmio[n].memory = memory;
 +object_property_add_child(OBJECT(dev), propname, OBJECT(memory),
 +  error_abort);

So relying on sysbus to do the MR parenting doesn't work too well when
you have a mix of sysbus and non-sysbus MRs in the system. It also
increases the sysbus API incumbency. I thinks better to let the Memory
API do the QOM parenting to the owner argument in memory_region_init
(It's an object after all). If you pass NULL to memory_region_init,
then you are responsible for parenting the MR yourself.

This child-adder is then converted to a regular link-adder for sysbus's sake.

Regards,
Peter

[Qemu-devel] [Bug 1321684] Re: block_stream command stalls

[mcpacino]

To Stefan:

yes, I can reproduce this on qemu.git/master.

Actually, I've found the cause of this bug and sent a patch to qemu-devel 
mailing list a few days ago: 
http://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg05777.html

It's very kind of you helping review the patch and give some comments.

Thanks.

Cong Meng.

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

Title:
  block_stream command stalls

Status in QEMU:
  New

Bug description:
  block_stream command stalls near finishing.
  I tried 1.7.1, 2.0.0 and the master versions. All of them stalled.
  But the 1.1.2 could finish the job.

  Here is how to reproduce it:

  $ sudo $QEMU \
  -enable-kvm  -cpu qemu64  -m 1024  \
  -monitor stdio \
  -drive file=./i1,if=none,id=drive_0,cache=none,aio=native -device 
virtio-blk-pci,drive=drive_0,bus=pci.0,addr=0x5 \

  QEMU 2.0.50 monitor - type 'help' for more information
  (qemu) VNC server running on `127.0.0.1:5900'
  (qemu) snapshot_blkdev drive_0 s1
  Formatting 's1', fmt=qcow2 size=26843545600 backing_file='./i1' 
backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off 
  (qemu) block_stream drive_0 
  (qemu) info block-jobs 
  Streaming device drive_0: Completed 400818176 of 26843545600 bytes, speed 
limit 0 bytes/s
  (qemu) info block-jobs 
  Streaming device drive_0: Completed 904396800 of 26843545600 bytes, speed 
limit 0 bytes/s
  (qemu) info block-jobs 
  Streaming device drive_0: Completed 23401070592 of 26843545600 bytes, speed 
limit 0 bytes/s
  (qemu) info block-jobs 
  Streaming device drive_0: Completed 26513768448 of 26843545600 bytes, speed 
limit 0 bytes/s
  (qemu) main-loop: WARNING: I/O thread spun for 1000 iterations
  info block-jobs 
  Streaming device drive_0: Completed 26841513984 of 26843545600 bytes, speed 
limit 0 bytes/s
  (qemu) info block-jobs 
  Streaming device drive_0: Completed 26841513984 of 26843545600 bytes, speed 
limit 0 bytes/s
  (qemu) info block-jobs 
  Streaming device drive_0: Completed 26841513984 of 26843545600 bytes, speed 
limit 0 bytes/s

   here, the progress stopped at 26841513984 

  
  $ qemu-img info i1 
  image: i1
  file format: qcow2
  virtual size: 25G (26843545600 bytes)
  disk size: 1.0G
  cluster_size: 2097152
  Format specific information:
  compat: 1.1
  lazy refcounts: false

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

Re: [Qemu-devel] [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6

[Fabio Fantoni]

Il 28/05/2014 18:53, Fabio Fantoni ha scritto:
2014-05-28 18:41 GMT+02:00 Stefano Stabellini 
stefano.stabell...@eu.citrix.com 
mailto:stefano.stabell...@eu.citrix.com:


On Wed, 28 May 2014, Stefano Stabellini wrote:
 On Wed, 28 May 2014, Fabio Fantoni wrote:
  Il 26/05/2014 10:00, Fabio Fantoni ha scritto:
   Il 25/05/2014 16:14, Stefano Stabellini ha scritto:
On Fri, 23 May 2014, Fabio Fantoni wrote:
 Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
  Choose pc-i440fx-1.6 instead of pc for HVM guests, so
that we know for
  sure what is the machine that we are emulating.
 
  Use pc-i440fx-1.6 regardless of the xen_platform_pci
option. Add the
  xen-platform device if requested. Choose slot 2 for
the xen-platform
  device for compatibility with current installations.
In case of Intel
  graphic passthrough, slot 2 might be needed by the
grafic card.
  However
  now that we can specify the slot explicitly, it is
easy to change the
  position of the xen-platform device on the PCI bus if
graphic
  passthrough is enabled.
 
  Move the machine options earlier, before any other
emulated devices
  options. Otherwise the selected PCI slot for the
xen-platform device
  is
  not available in QEMU.
 
  Specify
PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
  differently from xenfv, the other QEMU machines do not
have that
  option
  off by default.
 
  This patch does not change the emulated environment in
the guest.
 
  Refer to this thread:
  http://marc.info/?l=xen-develm=140023775929625w=2
 
  Signed-off-by: Stefano Stabellini
stefano.stabell...@eu.citrix.com
mailto:stefano.stabell...@eu.citrix.com
 
  diff --git a/tools/libxl/libxl_dm.c
b/tools/libxl/libxl_dm.c
  index 8abed7b..fef684f 100644
  --- a/tools/libxl/libxl_dm.c
  +++ b/tools/libxl/libxl_dm.c
  @@ -476,6 +476,29 @@ static char **
  libxl__build_device_model_args_new(libxl__gc *gc,
   flexarray_vappend(dm_args, -k, keymap, NULL);
 }
 +  flexarray_append(dm_args, -machine);
  +switch (b_info-type) {
  +case LIBXL_DOMAIN_TYPE_PV:
  +  flexarray_append(dm_args, xenpv);
  +for (i = 0; b_info-extra_pv 
b_info-extra_pv[i] != NULL;
  i++)
  +  flexarray_append(dm_args, b_info-extra_pv[i]);
  +break;
  +case LIBXL_DOMAIN_TYPE_HVM:
  +  flexarray_append(dm_args, pc-i440fx-1.6,accel=xen);
  
   I think that a note in README should be added: qemu =1.6.1
is required for
   hvm domUs.
  
   About the other xl parameter to be added I think that should be:
   qemu_machine_type=i440fx|q35 (when also q35 will be
supported on xen)
   qemu_machine_version=x.y to specify the machine version,
useful for
   debug/testing and other some cases.
  
  +  flexarray_append(dm_args, -global);
  +  flexarray_append(dm_args,
  PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off);
 I think is good add a comment for remember to remove
this workaround
 when pc
  =2.1 will be the default since qemu 2.1 will fix it.

https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html
The workaround is not actually harmful, it doesn't need to
be removed
when pc = 2.1 in QEMU.
   
   
  +if
(libxl_defbool_val(b_info-u.hvm.xen_platform_pci)) {
  +  flexarray_append(dm_args, -device);
  +  flexarray_append(dm_args, xen-platform,addr=0x2);
 The fixed pci address to 0x2 probably is a problem with
intel gpu
 passthrough:

http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html
Right, however we cannot really change the default
position of the
xen-platform device on the PCI bus, otherwise we'll end up
changing the
emulated environment for all the VMs out there.
   
I'll leave it to Tiejun to move xen-platform to another
slot when
graphic passthrough is enabled.
  
 
  I found another case of problem with xen-platform's fixed pci
slot.
  I tested this patch and I saw that qemu not start also in
other cases, for
  example the domU of my test:
   qemu-system-i386: -device xen-platform,addr=0x2: PCI: slot 2
function 0 not
   available for xen-platform, in use by intel-hda
   qemu-system-i386: -device xen-platform,addr=0x2: Device
initialization
   failed.
   qemu-system-i386: -device xen-platform,addr=0x2: Device
'xen-platform' could
   not be 

[Qemu-devel] Expansion Ratio Issue

[Chaos Shu]

Hi all

 

I'm new to the list, and recently I'm digging in Qemu's source code, I've
got something confused me much, simply list the items:

 

1.   Any benchmarks paying attention to TCG code generate quality
measured by code expansion ratio? Of course I've got some news said that the
ratio maybe 4 or 5 in X86 to MIPS, that is to say 1 x86 insn to 4 or 5 mips
insns, Does it mean the industry level or average level? Any official report
given?

2.   I've noticed that once Apple merge from PowerPC to X86, they
developed the software named Rosetta which is described by apple to be
successful, is it the same to Qemu? Any internal infos covered?

3.   Assume that we just wanna x86 to arm, so may we can strip out the
little operations and work on insn to insn such as move in x86 to move in
arm, insn level translate but not insn-op-insn, I think there must be
someone have ever made this try, anyone got their news?

4.   Why Qemu use only one TCG runtime, I found a project named PQEMU
once try to make TCG running on multicore but it's out of date and got some
commercial issues, is there any project trying to make it go?

 

Case I'm really new to Qemu, maybe many results or achievements I don't
know, If anyone can provide some tips about things mentioned above, I'll
appreciate much.

 

Thanks

Chaos

Re: [Qemu-devel] [RFC PATCH 2/2] qemu: support xen hvm direct kernel boot

[Paolo Bonzini]

Il 29/05/2014 05:23, Chunyan Liu ha scritto:
 [support xen HVM direct kernel boot]
 qemu side patch: if -kernel exists, calls xen_load_linux(), which
 will read kernel/initrd and add a linuxboot.bin or multiboot.bin
 option rom. The linuxboot.bin/multiboot.bin will load kernel/initrd
 and jump to execute kernel directly. It's working when xen uses
 seabios.
 
 Signed-off-by: Chunyan Liu cy...@suse.com
 ---
  hw/i386/pc.c | 29 +
  hw/i386/pc_piix.c|  7 +++
  include/hw/i386/pc.h |  5 +
  3 files changed, 41 insertions(+)
 
 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index e6369d5..dcd5d48 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -1187,6 +1187,35 @@ void pc_acpi_init(const char *default_dsdt)
  }
  }
  
 +FWCfgState *xen_load_linux(const char *kernel_filename,
 +   const char *kernel_cmdline,
 +   const char *initrd_filename,
 +   ram_addr_t below_4g_mem_size,
 +   PcGuestInfo *guest_info)
 +{
 +int i;
 +FWCfgState *fw_cfg;
 +
 +assert(kernel_filename != NULL);
 +
 +fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
 +rom_set_fw(fw_cfg);
 +
 +load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, 
 below_4g_mem_size);
 +for (i = 0; i  nb_option_roms; i++) {
 +/* For xen, we only want to add the linuxboot.bin/multiboot.bin 
 option rom.
 + * But in option_rom, there is still kvmvapic.bin. We don't want to 
 add it.
 + */
 +if (strcmp(option_rom[i].name, linuxboot.bin) 
 +strcmp(option_rom[i].name, multiboot.bin)) {
 +continue;
 +}
 +rom_add_option(option_rom[i].name, option_rom[i].bootindex);
 +}

Instead of this hack, you can use:

diff --git a/hw/i386/xen/xen_apic.c b/hw/i386/xen/xen_apic.c
index 63bb7f7..f5acd6a 100644
--- a/hw/i386/xen/xen_apic.c
+++ b/hw/i386/xen/xen_apic.c
@@ -40,6 +40,7 @@ static void xen_apic_realize(DeviceState *dev, Error **errp)
 {
 APICCommonState *s = APIC_COMMON(dev);
 
+s-vapic_control = 0;
 memory_region_init_io(s-io_memory, OBJECT(s), xen_apic_io_ops, s,
   xen-apic-msi, APIC_SPACE_SIZE);
 

 +guest_info-fw_cfg = fw_cfg;
 +return fw_cfg;
 +}
 +
  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
 const char *kernel_filename,
 const char *kernel_cmdline,
 diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
 index eaf3e61..14d4164 100644
 --- a/hw/i386/pc_piix.c
 +++ b/hw/i386/pc_piix.c
 @@ -157,6 +157,13 @@ static void pc_init1(QEMUMachineInitArgs *args,
 args-initrd_filename,
 below_4g_mem_size, above_4g_mem_size,
 rom_memory, ram_memory, guest_info);
 +} else if (args-kernel_filename != NULL) {
 +/* For xen HVM direct kernel boot, load linux here */
 +fw_cfg = xen_load_linux(args-kernel_filename,
 +args-kernel_cmdline,
 +args-initrd_filename,
 +below_4g_mem_size,
 +guest_info);
  }
  
  gsi_state = g_malloc0(sizeof(*gsi_state));
 diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
 index 32a7687..e472184 100644
 --- a/include/hw/i386/pc.h
 +++ b/include/hw/i386/pc.h
 @@ -134,6 +134,11 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t 
 below_4g_mem_size,
  void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
  MemoryRegion *pci_address_space);
  
 +FWCfgState *xen_load_linux(const char *kernel_filename,
 +   const char *kernel_cmdline,
 +   const char *initrd_filename,
 +   ram_addr_t below_4g_mem_size,
 +   PcGuestInfo *guest_info);
  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
 const char *kernel_filename,
 const char *kernel_cmdline,
 

Re: [Qemu-devel] [PATCH v2 1/7] qom: add object_property_add_alias()

[Peter Crosthwaite]

On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi stefa...@redhat.com wrote:
 Sometimes an object needs to present a property which is actually on
 another object, or it needs to provide an alias name for an existing
 property.

 Examples:
   a.foo - b.foo
   a.old_name - a.new_name

 The new object_property_add_alias() API allows objects to alias a
 property on the same object or another object.  The source and target
 names can be different.

 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com

Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com

 ---
 v2:
  * Explain refcount handling in doc comment [Paolo]
  * Fix property duplicate typo [Peter Crosthwaite]
  * Add the same object or to clarify commit description [Igor]
 ---
  include/qom/object.h | 20 
  qom/object.c | 52 
 
  2 files changed, 72 insertions(+)

 diff --git a/include/qom/object.h b/include/qom/object.h
 index a641dcd..854a0d5 100644
 --- a/include/qom/object.h
 +++ b/include/qom/object.h
 @@ -1203,6 +1203,26 @@ void object_property_add_uint64_ptr(Object *obj, const 
 char *name,
  const uint64_t *v, Error **Errp);

  /**
 + * object_property_add_alias:
 + * @obj: the object to add a property to
 + * @name: the name of the property
 + * @target_obj: the object to forward property access to
 + * @target_name: the name of the property on the forwarded object
 + * @errp: if an error occurs, a pointer to an area to store the error
 + *
 + * Add an alias for a property on an object.  This function will add a 
 property
 + * of the same type as the forwarded property.
 + *
 + * The caller must ensure that code@target_obj/code stays alive as long 
 as
 + * this property exists.  In the case of a child object or an alias on the 
 same
 + * object this will be the case.  For aliases to other objects the caller is
 + * responsible for taking a reference.
 + */
 +void object_property_add_alias(Object *obj, const char *name,
 +   Object *target_obj, const char *target_name,
 +   Error **errp);
 +
 +/**
   * object_child_foreach:
   * @obj: the object whose children will be navigated
   * @fn: the iterator function to be called
 diff --git a/qom/object.c b/qom/object.c
 index e42b254..ff50f37 100644
 --- a/qom/object.c
 +++ b/qom/object.c
 @@ -1515,6 +1515,58 @@ void object_property_add_uint64_ptr(Object *obj, const 
 char *name,
  NULL, NULL, (void *)v, errp);
  }

 +typedef struct
 +{
 +Object *target_obj;
 +const char *target_name;
 +} AliasProperty;
 +
 +static void property_get_alias(Object *obj, struct Visitor *v, void *opaque,
 +   const char *name, Error **errp)
 +{
 +AliasProperty *prop = opaque;
 +
 +object_property_get(prop-target_obj, v, prop-target_name, errp);
 +}
 +
 +static void property_set_alias(Object *obj, struct Visitor *v, void *opaque,
 +   const char *name, Error **errp)
 +{
 +AliasProperty *prop = opaque;
 +
 +object_property_set(prop-target_obj, v, prop-target_name, errp);
 +}
 +
 +static void property_release_alias(Object *obj, const char *name, void 
 *opaque)
 +{
 +AliasProperty *prop = opaque;
 +
 +g_free(prop);
 +}
 +
 +void object_property_add_alias(Object *obj, const char *name,
 +   Object *target_obj, const char *target_name,
 +   Error **errp)
 +{
 +AliasProperty *prop;
 +ObjectProperty *target_prop;
 +
 +target_prop = object_property_find(target_obj, target_name, errp);
 +if (!target_prop) {
 +return;
 +}
 +
 +prop = g_malloc(sizeof(*prop));
 +prop-target_obj = target_obj;
 +prop-target_name = target_name;
 +
 +object_property_add(obj, name, target_prop-type,
 +property_get_alias,
 +property_set_alias,
 +property_release_alias,
 +prop, errp);
 +}
 +
  static void object_instance_init(Object *obj)
  {
  object_property_add_str(obj, type, qdev_get_type, NULL, NULL);
 --
 1.9.0

Re: [Qemu-devel] [PATCH v2 2/7] virtio-blk: avoid qdev property definition duplication

[Peter Crosthwaite]

On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi stefa...@redhat.com wrote:
 It becomes unwiedly to duplicate all virtio-blk qdev property
 definitions due to an #ifdef.  The C preprocessor syntax makes it a
 little hard to resolve this cleanly but we can extract the #ifdef and
 call a macro it defines later.

 Avoiding duplication is important since it will only get worse when we
 move the x-data-plane qdev property here too.  We'd have a combinatorial
 explosion since x-data-plane has its own #ifdef.

 Suggested-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com

Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com

 ---
  include/hw/virtio/virtio-blk.h | 14 ++
  1 file changed, 6 insertions(+), 8 deletions(-)

 diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
 index e4c41ff..fa416db 100644
 --- a/include/hw/virtio/virtio-blk.h
 +++ b/include/hw/virtio/virtio-blk.h
 @@ -137,21 +137,19 @@ typedef struct VirtIOBlock {
  DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)

  #ifdef __linux__
 -#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field) 
  \
 -DEFINE_BLOCK_PROPERTIES(_state, _field.conf),
  \
 -DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),
  \
 -DEFINE_PROP_STRING(serial, _state, _field.serial), 
  \
 -DEFINE_PROP_BIT(config-wce, _state, _field.config_wce, 0, true),   
  \
 -DEFINE_PROP_BIT(scsi, _state, _field.scsi, 0, true),   
  \
 -DEFINE_PROP_IOTHREAD(x-iothread, _state, _field.iothread)
 +#define DEFINE_VIRTIO_BLK_PROPERTIES_LINUX(_state, _field) \
 +DEFINE_PROP_BIT(scsi, _state, _field.scsi, 0, true),
  #else
 +#define DEFINE_VIRTIO_BLK_PROPERTIES_LINUX(_state, _field)
 +#endif
 +
  #define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field) 
  \
 +DEFINE_VIRTIO_BLK_PROPERTIES_LINUX(_state, _field)   
  \
  DEFINE_BLOCK_PROPERTIES(_state, _field.conf),
  \
  DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),
  \
  DEFINE_PROP_STRING(serial, _state, _field.serial), 
  \
  DEFINE_PROP_BIT(config-wce, _state, _field.config_wce, 0, true),   
  \
  DEFINE_PROP_IOTHREAD(x-iothread, _state, _field.iothread)
 -#endif /* __linux__ */

  void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);

 --
 1.9.0

Re: [Qemu-devel] [PATCH v2 3/7] virtio-blk: move x-data-plane qdev property to virtio-blk.h

[Peter Crosthwaite]

On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi stefa...@redhat.com wrote:
 Move the x-data-plane property.  Originally it was outside since not
 every transport may wish to support dataplane.  But that makes little
 sense when we have a dedicated CONFIG_VIRTIO_BLK_DATA_PLANE ifdef
 already.

 This move makes it easier to switch to property aliases in the next
 patch.

 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com

Its worth pointing out, I'm not the ideal reviewer for virtio but from
a pure QOM/qdev perspective it looks ok to me.

Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com

 ---
  hw/s390x/virtio-ccw.c  | 3 ---
  hw/virtio/virtio-pci.c | 3 ---
  include/hw/virtio/virtio-blk.h | 8 
  3 files changed, 8 insertions(+), 6 deletions(-)

 diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
 index 1cb4e2c..082bb42 100644
 --- a/hw/s390x/virtio-ccw.c
 +++ b/hw/s390x/virtio-ccw.c
 @@ -1129,9 +1129,6 @@ static Property virtio_ccw_blk_properties[] = {
  DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk),
  DEFINE_PROP_BIT(ioeventfd, VirtioCcwDevice, flags,
  VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
 -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
 -DEFINE_PROP_BIT(x-data-plane, VirtIOBlkCcw, blk.data_plane, 0, false),
 -#endif
  DEFINE_PROP_END_OF_LIST(),
  };

 diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
 index ce97514..0751a1e 100644
 --- a/hw/virtio/virtio-pci.c
 +++ b/hw/virtio/virtio-pci.c
 @@ -1067,9 +1067,6 @@ static Property virtio_blk_pci_properties[] = {
  DEFINE_PROP_BIT(ioeventfd, VirtIOPCIProxy, flags,
  VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
  DEFINE_PROP_UINT32(vectors, VirtIOPCIProxy, nvectors, 2),
 -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
 -DEFINE_PROP_BIT(x-data-plane, VirtIOBlkPCI, blk.data_plane, 0, false),
 -#endif
  DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
  DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, blk),
  DEFINE_PROP_END_OF_LIST(),
 diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
 index fa416db..78e7f81 100644
 --- a/include/hw/virtio/virtio-blk.h
 +++ b/include/hw/virtio/virtio-blk.h
 @@ -143,8 +143,16 @@ typedef struct VirtIOBlock {
  #define DEFINE_VIRTIO_BLK_PROPERTIES_LINUX(_state, _field)
  #endif

 +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
 +#define DEFINE_VIRTIO_BLK_PROPERTIES_DATA_PLANE(_state, _field) \
 +DEFINE_PROP_BIT(x-data-plane, _state, _field.data_plane, 0, false),
 +#else
 +#define DEFINE_VIRTIO_BLK_PROPERTIES_DATA_PLANE(_state, _field)
 +#endif
 +
  #define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field) 
  \
  DEFINE_VIRTIO_BLK_PROPERTIES_LINUX(_state, _field)   
  \
 +DEFINE_VIRTIO_BLK_PROPERTIES_DATA_PLANE(_state, _field)  
  \
  DEFINE_BLOCK_PROPERTIES(_state, _field.conf),
  \
  DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),
  \
  DEFINE_PROP_STRING(serial, _state, _field.serial), 
  \
 --
 1.9.0

Re: [Qemu-devel] [PATCH v2 4/7] virtio-blk: use aliases instead of duplicate qdev properties

[Peter Crosthwaite]

On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi stefa...@redhat.com wrote:
 virtio-blk-pci, virtio-blk-s390, and virtio-blk-ccw all duplicate the
 qdev properties of their VirtIOBlock child.  This approach does not work
 well with string or pointer properties since we must be careful about
 leaking or double-freeing them.

 Use the QOM alias property to forward property accesses to the
 VirtIOBlock child.  This way no duplication is necessary.

 Remember to stop calling virtio_blk_set_conf() so that we don't clobber
 the values already set on the VirtIOBlock instance.

 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
 ---
 v2:
  * Add qdev_alias_all_properties() instead of virtio-blk-specific
function [Paolo]
 ---
  hw/core/qdev.c   | 19 +++

The commit message proper makes no reference to the new qedev core
feature for alias properties. That said, I think the qdev change would
work nicely as a separate patch and this ones commit message would
stand nicely as-is without the qdev patch content.

  hw/s390x/s390-virtio-bus.c   |  9 +
  hw/s390x/s390-virtio-bus.h   |  1 -
  hw/s390x/virtio-ccw.c|  3 +--
  hw/s390x/virtio-ccw.h|  1 -
  hw/virtio/virtio-pci.c   |  3 +--
  hw/virtio/virtio-pci.h   |  1 -
  include/hw/qdev-properties.h |  2 ++
  8 files changed, 24 insertions(+), 15 deletions(-)

 diff --git a/hw/core/qdev.c b/hw/core/qdev.c
 index 936eae6..e41f5b8 100644
 --- a/hw/core/qdev.c
 +++ b/hw/core/qdev.c
 @@ -724,6 +724,25 @@ void qdev_property_add_static(DeviceState *dev, Property 
 *prop,
  }
  }

 +/* @qdev_alias_all_properties - Add alias properties to the source object for
 + * all qdev properties on the target DeviceState.
 + */
 +void qdev_alias_all_properties(DeviceState *target, Object *source)
 +{
 +ObjectClass *class;
 +Property *prop;
 +
 +class = object_get_class(OBJECT(target));
 +do {
 +for (prop = DEVICE_CLASS(class)-props; prop  prop-name; prop++) {

Don't de-reference an inline QOM cast macro. DEVICE_CLASS(class) will
need its own local variable. Andreas, can we update QOM conventions
with this finer point? I think we already have Avoid using cast
macros other than OBJECT() inline but I remember a thread a while
back along the lines of no de-referencing being the main intention of
this rule.

Regards,
Peter

Re: [Qemu-devel] [PATCH v2 5/7] virtio-blk: drop virtio_blk_set_conf()

[Peter Crosthwaite]

On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi stefa...@redhat.com wrote:
 This function is no longer used since parent objects now use child
 aliases to set the VirtIOBlkConf directly.

 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com

Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com

 ---
  hw/block/virtio-blk.c  | 6 --
  include/hw/virtio/virtio-blk.h | 2 --
  2 files changed, 8 deletions(-)

 diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
 index 8a568e5..4781351 100644
 --- a/hw/block/virtio-blk.c
 +++ b/hw/block/virtio-blk.c
 @@ -642,12 +642,6 @@ static const BlockDevOps virtio_block_ops = {
  .resize_cb = virtio_blk_resize,
  };

 -void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk)
 -{
 -VirtIOBlock *s = VIRTIO_BLK(dev);
 -memcpy((s-blk), blk, sizeof(struct VirtIOBlkConf));
 -}
 -
  #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
  /* Disable dataplane thread during live migration since it does not
   * update the dirty memory bitmap yet.
 diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
 index 78e7f81..f8d4ac1 100644
 --- a/include/hw/virtio/virtio-blk.h
 +++ b/include/hw/virtio/virtio-blk.h
 @@ -159,6 +159,4 @@ typedef struct VirtIOBlock {
  DEFINE_PROP_BIT(config-wce, _state, _field.config_wce, 0, true),   
  \
  DEFINE_PROP_IOTHREAD(x-iothread, _state, _field.iothread)

 -void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
 -
  #endif
 --
 1.9.0

Re: [Qemu-devel] Expansion Ratio Issue

[Peter Maydell]

On 29 May 2014 08:58, Chaos Shu chaos.s...@live.com wrote:
 1.   Any benchmarks paying attention to TCG code generate quality
 measured by code expansion ratio? Of course I’ve got some news said that the
 ratio maybe 4 or 5 in X86 to MIPS, that is to say 1 x86 insn to 4 or 5 mips
 insns, Does it mean the industry level or average level? Any official report
 given?

No, we don't in general have any benchmarking of TCG
codegen. I think if we did do benchmarking we'd be interested
in performance benchmarking -- code expansion ratio doesn't
seem like a very interesting thing to measure to me.

 2.   I’ve noticed that once Apple merge from PowerPC to X86, they
 developed the software named Rosetta which is described by apple to be
 successful, is it the same to Qemu? Any internal infos covered?

It's a similar concept, though as I understand it it focused
on doing translation for a single application (like QEMU's
linux-user mode, not like our system emulation mode). I have
no idea about its internal design.

 3.   Assume that we just wanna x86 to arm, so may we can strip out the
 little operations and work on insn to insn such as move in x86 to move in
 arm, insn level translate but not insn-op-insn, I think there must be
 someone have ever made this try, anyone got their news?

Certainly if you started from scratch with the intention of
doing a more specifically targeted design (and in particular
if you wanted to do single-application translation as your core
focus rather than as a bolt-on extension to system emulation)
you could probably get better performance than QEMU. QEMU
generally aims to be a general-purpose project, though.

Personally I would (even if doing only x86-to-ARM) still
include an intermediate representation of some form: the
history of compiler design shows that it has a lot of utility.

 4.   Why Qemu use only one TCG runtime, I found a project named PQEMU
 once try to make TCG running on multicore but it’s out of date and got some
 commercial issues, is there any project trying to make it go?

Not that I currently know of. Truly parallel TCG execution
of multiple guest cores is a hard problem, especially if you
want to produce maintainable solid code that can be included
upstream, rather than just enough of a prototype to demonstrate
proof of concept and run some simple benchmarks for an academic
paper.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 6/7] virtio: fix virtio-blk child refcount in transports

[Peter Crosthwaite]

On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi stefa...@redhat.com wrote:
 object_initialize() leaves the object with a refcount of 1.
 object_property_add_child() adds its own reference which is dropped
 again when the property is deleted.

 The upshot of this is that we always have a refcount = 1.  Upon hot
 unplug the virtio-blk child is not finalized!


Doesn't this suggest that hot unplug is what's broken? My
understanding (which is fresh and not 100% yet) is the original == 1
refcount should be dropped at object deletion time which is this sense
would be unplug time. This would mean that hot-unplug should
explicitly object_unref the object (should the intention of hot-unplug
be to always finalise the device?).

Regards,
Peter

 Drop our reference after the child property has been added to the
 parent.


 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
 ---
  hw/s390x/s390-virtio-bus.c | 1 +
  hw/s390x/virtio-ccw.c  | 1 +
  hw/virtio/virtio-pci.c | 1 +
  3 files changed, 3 insertions(+)

 diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
 index 041d4e2..bf92cb6 100644
 --- a/hw/s390x/s390-virtio-bus.c
 +++ b/hw/s390x/s390-virtio-bus.c
 @@ -191,6 +191,7 @@ static void s390_virtio_blk_instance_init(Object *obj)
  VirtIOBlkS390 *dev = VIRTIO_BLK_S390(obj);
  object_initialize(dev-vdev, sizeof(dev-vdev), TYPE_VIRTIO_BLK);
  object_property_add_child(obj, virtio-backend, OBJECT(dev-vdev), 
 NULL);
 +object_unref(OBJECT(dev-vdev));
  qdev_alias_all_properties(DEVICE(dev-vdev), obj);
  }

 diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
 index f33293d..eb8427b 100644
 --- a/hw/s390x/virtio-ccw.c
 +++ b/hw/s390x/virtio-ccw.c
 @@ -738,6 +738,7 @@ static void virtio_ccw_blk_instance_init(Object *obj)
  VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(obj);
  object_initialize(dev-vdev, sizeof(dev-vdev), TYPE_VIRTIO_BLK);
  object_property_add_child(obj, virtio-backend, OBJECT(dev-vdev), 
 NULL);
 +object_unref(OBJECT(dev-vdev));
  qdev_alias_all_properties(DEVICE(dev-vdev), obj);
  }

 diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
 index 3bb782f..abf05a9 100644
 --- a/hw/virtio/virtio-pci.c
 +++ b/hw/virtio/virtio-pci.c
 @@ -1102,6 +1102,7 @@ static void virtio_blk_pci_instance_init(Object *obj)
  VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(obj);
  object_initialize(dev-vdev, sizeof(dev-vdev), TYPE_VIRTIO_BLK);
  object_property_add_child(obj, virtio-backend, OBJECT(dev-vdev), 
 NULL);
 +object_unref(OBJECT(dev-vdev));
  qdev_alias_all_properties(DEVICE(dev-vdev), obj);
  }

 --
 1.9.0

Re: [Qemu-devel] [PATCH v2 7/7] virtio-blk: move qdev properties into virtio-blk.c

[Peter Crosthwaite]

On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi stefa...@redhat.com wrote:
 There is no need to make DEFINE_VIRTIO_BLK_PROPERTIES() public.  Inline
 it into virtio-blk.c so it cannot be used by mistake from other source
 files.

 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com

Reviewed-by: Peter Crosthwaite peter.crosthw...@xilinx.com

 ---
  hw/block/virtio-blk.c  | 12 +++-
  include/hw/virtio/virtio-blk.h | 23 ---
  2 files changed, 11 insertions(+), 24 deletions(-)

 diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
 index 4781351..3a261c8 100644
 --- a/hw/block/virtio-blk.c
 +++ b/hw/block/virtio-blk.c
 @@ -749,7 +749,17 @@ static void virtio_blk_device_unrealize(DeviceState 
 *dev, Error **errp)
  }

  static Property virtio_blk_properties[] = {
 -DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlock, blk),
 +DEFINE_BLOCK_PROPERTIES(VirtIOBlock, blk.conf),
 +DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, blk.conf),
 +DEFINE_PROP_STRING(serial, VirtIOBlock, blk.serial),
 +DEFINE_PROP_BIT(config-wce, VirtIOBlock, blk.config_wce, 0, true),
 +DEFINE_PROP_IOTHREAD(x-iothread, VirtIOBlock, blk.iothread),
 +#ifdef __linux__
 +DEFINE_PROP_BIT(scsi, VirtIOBlock, blk.scsi, 0, true),
 +#endif
 +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
 +DEFINE_PROP_BIT(x-data-plane, VirtIOBlock, blk.data_plane, 0, false),
 +#endif
  DEFINE_PROP_END_OF_LIST(),
  };

 diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
 index f8d4ac1..beecb7e 100644
 --- a/include/hw/virtio/virtio-blk.h
 +++ b/include/hw/virtio/virtio-blk.h
 @@ -136,27 +136,4 @@ typedef struct VirtIOBlock {
  #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
  DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)

 -#ifdef __linux__
 -#define DEFINE_VIRTIO_BLK_PROPERTIES_LINUX(_state, _field) \
 -DEFINE_PROP_BIT(scsi, _state, _field.scsi, 0, true),
 -#else
 -#define DEFINE_VIRTIO_BLK_PROPERTIES_LINUX(_state, _field)
 -#endif
 -
 -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
 -#define DEFINE_VIRTIO_BLK_PROPERTIES_DATA_PLANE(_state, _field) \
 -DEFINE_PROP_BIT(x-data-plane, _state, _field.data_plane, 0, false),
 -#else
 -#define DEFINE_VIRTIO_BLK_PROPERTIES_DATA_PLANE(_state, _field)
 -#endif
 -
 -#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field) 
  \
 -DEFINE_VIRTIO_BLK_PROPERTIES_LINUX(_state, _field)   
  \
 -DEFINE_VIRTIO_BLK_PROPERTIES_DATA_PLANE(_state, _field)  
  \
 -DEFINE_BLOCK_PROPERTIES(_state, _field.conf),
  \
 -DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),
  \
 -DEFINE_PROP_STRING(serial, _state, _field.serial), 
  \
 -DEFINE_PROP_BIT(config-wce, _state, _field.config_wce, 0, true),   
  \
 -DEFINE_PROP_IOTHREAD(x-iothread, _state, _field.iothread)
 -
  #endif
 --
 1.9.0

Re: [Qemu-devel] [PATCH RFC 8/8] virtio: add endian-ambivalent support to VirtIODevice

[Greg Kurz]

On Tue, 27 May 2014 17:01:38 +0200
Paolo Bonzini pbonz...@redhat.com wrote:

 Il 14/05/2014 17:42, Greg Kurz ha scritto:
  +{ .name = virtio/is_big_endian,
  +  .version_id = 1,
  +  .save = virtio_save_device_endian,
  +  .load = virtio_load_device_endian,
  +},
   { .name = NULL }
 
 I think this is okay, but you need to add support for a needed 
 function like you have in normal vmstate subsections.  You only need the 
 subsection if
 
  if (target_words_bigendian()) {
  return vdev-device_endian != VIRTIO_DEVICE_ENDIAN_BIG;
  } else {
  return vdev-device_endian != VIRTIO_DEVICE_ENDIAN_LITTLE;
  }
 
 Paolo
 

Hi Paolo,

As suggested by Andreas, I have reworked the patch set to use
VMState directly instead of mimicking... and of course, I end
up with a virtio_device_endian_needed() function that does just
that ! :)

I'm on leave now, I'll try to send an update next week.

BTW, I have spotted two locations where the migration code is
affected by the device endianness at load time:

int virtio_load(VirtIODevice *vdev, QEMUFile *f)
{
[...]
nheads = vring_avail_idx(vdev-vq[i]) - vdev-vq[i].last_avail_idx;
   ^^^
/* Check it isn't doing very strange things with descriptor 
numbers. */
if (nheads  vdev-vq[i].vring.num) {
[...]
}

and

static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
{
[...]
/* The config space */
qemu_get_be16s(f, s-config.cols);
qemu_get_be16s(f, s-config.rows);

qemu_get_be32s(f, max_nr_ports);
tswap32s(max_nr_ports);
 ^^
if (max_nr_ports  tswap32(s-config.max_nr_ports)) {
[...]
}

If we stream subsections after the device descriptor as it is done
in VMState, these two will break because the device endian is stale.

The first one can be easily dealt with: just defer the sanity check
to a post_load function. The second is a bit more tricky: the
virtio serial migrates its config space (target endian) and the
active ports bitmap. The load code assumes max_nr_ports from the
config space tells the size of the ports bitmap... that means the
virtio migration protocol is also contaminated by target endianness. :-\

I have questions about virtio serial:
- what is exactly the point at migrating the virtio device config space ?
- what is the scenario that calls for the active ports bitmap checking
  at load time ?
- we currently have 0  max_nr_ports  VIRTIO_PCI_QUEUE_MAX / 2 hardcoded.
  With the current code, that means the ports bitmap is always a single
  32-bit fixed size value... are there plans to support virtio serial
  with 0 port ? with more than 32 ports ?

In the case the answer for above is legacy virtio really sucks then,
is it acceptable to not honor bug-compatibility with older versions and
fix the code ? :)

A solution could be to simply remove all that crap and bump versions,
or at least send zeroes on save and skip them on load.

Another solution I haven't tried yet would be to stream subsections before
the device descriptor...

Any suggestions ?

Cheers.

-- 
Gregory Kurz kurzg...@fr.ibm.com
 gk...@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys  http://www.ibm.com
Tel +33 (0)562 165 496

Anarchy is about taking complete responsibility for yourself.
Alan Moore.

Re: [Qemu-devel] [PATCH 33/35] pc: ACPI BIOS: reserve SRAT entry for hotplug mem hole

[Igor Mammedov]

On Wed, 28 May 2014 18:38:13 +0200
Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com wrote:

 On Wed, May 28, 2014 at 03:26:42PM +0200, Igor Mammedov wrote:
  On Wed, 28 May 2014 14:23:13 +0200
  Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com wrote:
  
   On Wed, May 28, 2014 at 10:07:22AM +0200, Igor Mammedov wrote:
On Tue, 27 May 2014 17:57:31 +0200
Anshul Makkar anshul.mak...@profitbricks.com wrote:

 Hi,
 
 I tested the hot unplug patch and doesn't seem to work properly with 
 Debian
 6 and Ubuntu host.
 
 Scenario:
 I added 3 dimm devices of 1G each:
 
 object_add memory-ram,id=ram0,size=1G, device_add 
 dimm,id=dimm1,memdev=ram0
 
 object_add memory-ram,id=ram1,size=1G, device_add 
 dimm,id=dimm2,memdev=ram1
 
 object_add memory-ram,id=ram2,size=1G, device_add 
 dimm,id=dimm3,memdev=ram2
 
 device_del dimm3: I get the OST EVENT EJECT 0x3 and OST STATUS as 
 0x84(IN
 PROGRESS) If I check on the guest, the device has been successfully
 removed. But no OST EJECT SUCCESS event was received.
I think there should be a SUCCESS event,
it should be investigated from the guest side first, OST support in 
kernel
is relatively new.
   
   When testing older guest kernel (3.11), _OST success events are not sent
   from the guest. I haven't tried newer versions yet.
   
   In terms of OSPM _OST behaviour, I am not sure if returning OST success 
   status
   on succcesful removal is *required*. Figure 6-37, page 306 of ACPI spec5.0
   shows that on succcesfull OS ejection ejection, _EJ0 is evaluated. 
   Evaluating
   _OST does not seem to be a requirement, is it? (cc'ing linux-acpi for 
   input)
   
   In linux guests, on successful removal, _EJ0 is always evaluated. I 
   believe we
   should be handling _EJ0 and doing the dimm removal (object_unparent) 
   there.
   Currently OST successes are never received and dimm devices remain in 
   QEMU even
   when successfully ejected from guest.
   E.g. a quick patch for _EJ0 handling, on top of Hu Tao's series:
   
   acpi, memory-hotplug: Add _EJ0 handling
   
   ---
docs/specs/acpi_mem_hotplug.txt  |3 ++-
hw/acpi/memory_hotplug.c |   13 +++--
hw/i386/ssdt-misc.dsl|3 ++-
include/hw/acpi/memory_hotplug.h |1 +
4 files changed, 12 insertions(+), 8 deletions(-)
   
   diff --git a/docs/specs/acpi_mem_hotplug.txt 
   b/docs/specs/acpi_mem_hotplug.txt
   index 1290994..1352962 100644
   --- a/docs/specs/acpi_mem_hotplug.txt
   +++ b/docs/specs/acpi_mem_hotplug.txt
   @@ -28,7 +28,8 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 
   byte access):
region will read/store data from/to selected memory 
   device.
  [0x4-0x7] OST event code reported by OSPM
  [0x8-0xb] OST status code reported by OSPM
   -  [0xc-0x13] reserved, writes into it are ignored
   +  [0xc]  EJ device if written to
   +  [0xd-0x13] reserved, writes into it are ignored
  [0x14] Memory device control fields
  bits:
  0: reserved, OSPM must clear it before writing to register
   diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
   index 8aa829d..d3edd28 100644
   --- a/hw/acpi/memory_hotplug.c
   +++ b/hw/acpi/memory_hotplug.c
   @@ -93,9 +93,6 @@ static void acpi_memory_hotplug_write(void *opaque, 
   hwaddr addr, uint64_t data,
case 0x03: /* EJECT */
switch (mdev-ost_status) {
case 0x0: /* SUCCESS */
   -object_unparent(OBJECT(mdev-dimm));
   -mdev-is_removing = false;
   -mdev-dimm = NULL;
break;
case 0x1: /* FAILURE */
case 0x2: /* UNRECOGNIZED NOTIFY */
   @@ -115,9 +112,6 @@ static void acpi_memory_hotplug_write(void *opaque, 
   hwaddr addr, uint64_t data,
case 0x103: /* OSPM EJECT */
switch (mdev-ost_status) {
case 0x0: /* SUCCESS */
   -object_unparent(OBJECT(mdev-dimm));
   -mdev-is_removing = false;
   -mdev-dimm = NULL;
break;
case 0x84: /* EJECTION IN PROGRESS */
mdev-is_enabled = false;
   @@ -137,6 +131,12 @@ static void acpi_memory_hotplug_write(void *opaque, 
   hwaddr addr, uint64_t data,
mdev-is_enabled = false;
}
break;
   +case 0x0c:
   +mdev = mem_st-devs[mem_st-selector];
   +object_unparent(OBJECT(mdev-dimm));
   +mdev-is_removing = false;
   +mdev-dimm = NULL;
   +break;
}
}

   @@ -238,6 +238,7 @@ static const VMStateDescription vmstate_memhp_sts = {
VMSTATE_BOOL(is_inserting, MemStatus),
VMSTATE_UINT32(ost_event, MemStatus),
VMSTATE_UINT32(ost_status, 

[Qemu-devel] [PATCH microblaze v1 0/6] Microblaze Device QOM cleanups

[Peter Crosthwaite]

Hi Andreas, Edgar,

Another 4 users of SysBusDevice::init bite the dust!

A round of styling cleanup for Microblaze devices as per QOM
conventions. Tested with published MB test images.

Regards,
Peter


Peter Crosthwaite (6):
  timer: xilinx_timer: Convert to realize()
  net: xilinx_ethlite: Don't reset from init
  net: xilinx_ethlite: Convert to realize()
  char: xilinx_uartlite: Don't reset from init
  char: xilinx_uartlite: Convert to realize()
  intc: xilinx_uartlite: Convert SBD::init - instance_init

 hw/char/xilinx_uartlite.c | 32 +---
 hw/intc/xilinx_intc.c | 17 +++--
 hw/net/xilinx_ethlite.c   | 34 ++
 hw/timer/xilinx_timer.c   | 20 
 4 files changed, 62 insertions(+), 41 deletions(-)

-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH microblaze v1 0/6] Microblaze Device QOM cleanups

[Peter Crosthwaite]

Hi Andreas, Edgar,

Another 4 users of SysBusDevice::init bite the dust!

A round of styling cleanup for Microblaze devices as per QOM
conventions. Tested with published MB test images.

Regards,
Peter


Peter Crosthwaite (6):
  timer: xilinx_timer: Convert to realize()
  net: xilinx_ethlite: Don't reset from init
  net: xilinx_ethlite: Convert to realize()
  char: xilinx_uartlite: Don't reset from init
  char: xilinx_uartlite: Convert to realize()
  intc: xilinx_uartlite: Convert SBD::init - instance_init

 hw/char/xilinx_uartlite.c | 32 +---
 hw/intc/xilinx_intc.c | 17 +++--
 hw/net/xilinx_ethlite.c   | 34 ++
 hw/timer/xilinx_timer.c   | 20 
 4 files changed, 62 insertions(+), 41 deletions(-)

-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH microblaze v1 2/6] net: xilinx_ethlite: Don't reset from init

[Peter Crosthwaite]

This zeroing-out of the rxbuf variable (ping pong state) is a reset
side effect. Extract into a proper reset.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

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

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 3a2a6c2..6cbd95d 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -204,6 +204,13 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t 
*buf, size_t size)
 return size;
 }
 
+static void xilinx_ethlite_reset(DeviceState *dev)
+{
+struct xlx_ethlite *s = XILINX_ETHLITE(dev);
+
+s-rxbuf = 0;
+}
+
 static void eth_cleanup(NetClientState *nc)
 {
 struct xlx_ethlite *s = qemu_get_nic_opaque(nc);
@@ -225,7 +232,6 @@ static int xilinx_ethlite_init(SysBusDevice *sbd)
 struct xlx_ethlite *s = XILINX_ETHLITE(dev);
 
 sysbus_init_irq(sbd, s-irq);
-s-rxbuf = 0;
 
 memory_region_init_io(s-mmio, OBJECT(s), eth_ops, s,
   xlnx.xps-ethernetlite, R_MAX * 4);
@@ -251,6 +257,7 @@ static void xilinx_ethlite_class_init(ObjectClass *klass, 
void *data)
 SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
 k-init = xilinx_ethlite_init;
+dc-reset = xilinx_ethlite_reset;
 dc-props = xilinx_ethlite_properties;
 }
 
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH microblaze v1 1/6] timer: xilinx_timer: Convert to realize()

[Peter Crosthwaite]

SysBusDevice::init is depracated. Convert to Object::init and
Device::realize as prescribed by QOM conventions.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/timer/xilinx_timer.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 3ff1da9..cdb3355 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -204,14 +204,11 @@ static void timer_hit(void *opaque)
 timer_update_irq(t);
 }
 
-static int xilinx_timer_init(SysBusDevice *dev)
+static void xilinx_timer_realize(DeviceState *dev, Error **errp)
 {
 struct timerblock *t = XILINX_TIMER(dev);
 unsigned int i;
 
-/* All timers share a single irq line.  */
-sysbus_init_irq(dev, t-irq);
-
 /* Init all the ptimers.  */
 t-timers = g_malloc0(sizeof t-timers[0] * num_timers(t));
 for (i = 0; i  num_timers(t); i++) {
@@ -226,8 +223,15 @@ static int xilinx_timer_init(SysBusDevice *dev)
 
 memory_region_init_io(t-mmio, OBJECT(t), timer_ops, t, xlnx.xps-timer,
   R_MAX * 4 * num_timers(t));
-sysbus_init_mmio(dev, t-mmio);
-return 0;
+sysbus_init_mmio(SYS_BUS_DEVICE(dev), t-mmio);
+}
+
+static void xilinx_timer_init(Object *obj)
+{
+struct timerblock *t = XILINX_TIMER(obj);
+
+/* All timers share a single irq line.  */
+sysbus_init_irq(SYS_BUS_DEVICE(obj), t-irq);
 }
 
 static Property xilinx_timer_properties[] = {
@@ -240,9 +244,8 @@ static Property xilinx_timer_properties[] = {
 static void xilinx_timer_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k-init = xilinx_timer_init;
+dc-realize = xilinx_timer_realize;
 dc-props = xilinx_timer_properties;
 }
 
@@ -250,6 +253,7 @@ static const TypeInfo xilinx_timer_info = {
 .name  = TYPE_XILINX_TIMER,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(struct timerblock),
+.instance_init = xilinx_timer_init,
 .class_init= xilinx_timer_class_init,
 };
 
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH microblaze v1 3/6] net: xilinx_ethlite: Convert to realize()

[Peter Crosthwaite]

SysBusDevice::init is depracated. Convert to Object::init and
Device::realize as prescribed by QOM conventions.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/net/xilinx_ethlite.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 6cbd95d..5a434f6 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -226,22 +226,25 @@ static NetClientInfo net_xilinx_ethlite_info = {
 .cleanup = eth_cleanup,
 };
 
-static int xilinx_ethlite_init(SysBusDevice *sbd)
+static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
 {
-DeviceState *dev = DEVICE(sbd);
 struct xlx_ethlite *s = XILINX_ETHLITE(dev);
 
-sysbus_init_irq(sbd, s-irq);
-
-memory_region_init_io(s-mmio, OBJECT(s), eth_ops, s,
-  xlnx.xps-ethernetlite, R_MAX * 4);
-sysbus_init_mmio(sbd, s-mmio);
-
 qemu_macaddr_default_if_unset(s-conf.macaddr);
 s-nic = qemu_new_nic(net_xilinx_ethlite_info, s-conf,
   object_get_typename(OBJECT(dev)), dev-id, s);
 qemu_format_nic_info_str(qemu_get_queue(s-nic), s-conf.macaddr.a);
-return 0;
+}
+
+static void xilinx_ethlite_init(Object *obj)
+{
+struct xlx_ethlite *s = XILINX_ETHLITE(obj);
+
+sysbus_init_irq(SYS_BUS_DEVICE(obj), s-irq);
+
+memory_region_init_io(s-mmio, obj, eth_ops, s,
+  xlnx.xps-ethernetlite, R_MAX * 4);
+sysbus_init_mmio(SYS_BUS_DEVICE(obj), s-mmio);
 }
 
 static Property xilinx_ethlite_properties[] = {
@@ -254,9 +257,8 @@ static Property xilinx_ethlite_properties[] = {
 static void xilinx_ethlite_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k-init = xilinx_ethlite_init;
+dc-realize = xilinx_ethlite_realize;
 dc-reset = xilinx_ethlite_reset;
 dc-props = xilinx_ethlite_properties;
 }
@@ -265,6 +267,7 @@ static const TypeInfo xilinx_ethlite_info = {
 .name  = TYPE_XILINX_ETHLITE,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(struct xlx_ethlite),
+.instance_init = xilinx_ethlite_init,
 .class_init= xilinx_ethlite_class_init,
 };
 
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH microblaze v1 4/6] char: xilinx_uartlite: Don't reset from init

[Peter Crosthwaite]

This refresh of the device state is intended to be a reset side
effect. Move it to a proper reset handler rather than do it at
init time.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/char/xilinx_uartlite.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index b0d1d04..d810e5f 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -87,6 +87,11 @@ static void uart_update_status(XilinxUARTLite *s)
 s-regs[R_STATUS] = r;
 }
 
+static void xilinx_uartlite_reset(DeviceState *dev)
+{
+uart_update_status(XILINX_UARTLITE(dev));
+}
+
 static uint64_t
 uart_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -202,7 +207,6 @@ static int xilinx_uartlite_init(SysBusDevice *dev)
 
 sysbus_init_irq(dev, s-irq);
 
-uart_update_status(s);
 memory_region_init_io(s-mmio, OBJECT(s), uart_ops, s,
   xlnx.xps-uartlite, R_MAX * 4);
 sysbus_init_mmio(dev, s-mmio);
@@ -216,8 +220,10 @@ static int xilinx_uartlite_init(SysBusDevice *dev)
 static void xilinx_uartlite_class_init(ObjectClass *klass, void *data)
 {
 SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
 
 sdc-init = xilinx_uartlite_init;
+dc-reset = xilinx_uartlite_reset;
 }
 
 static const TypeInfo xilinx_uartlite_info = {
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH microblaze v1 5/6] char: xilinx_uartlite: Convert to realize()

[Peter Crosthwaite]

SysBusDevice::init is depracated. Convert to Object::init and
Device::realize as prescribed by QOM conventions.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/char/xilinx_uartlite.c | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index d810e5f..f7c3cae 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -201,35 +201,39 @@ static void uart_event(void *opaque, int event)
 
 }
 
-static int xilinx_uartlite_init(SysBusDevice *dev)
+static void xilinx_uartlite_realize(DeviceState *dev, Error **errp)
 {
 XilinxUARTLite *s = XILINX_UARTLITE(dev);
 
-sysbus_init_irq(dev, s-irq);
-
-memory_region_init_io(s-mmio, OBJECT(s), uart_ops, s,
-  xlnx.xps-uartlite, R_MAX * 4);
-sysbus_init_mmio(dev, s-mmio);
-
 s-chr = qemu_char_get_next_serial();
 if (s-chr)
 qemu_chr_add_handlers(s-chr, uart_can_rx, uart_rx, uart_event, s);
-return 0;
+}
+
+static void xilinx_uartlite_init(Object *obj)
+{
+XilinxUARTLite *s = XILINX_UARTLITE(obj);
+
+sysbus_init_irq(SYS_BUS_DEVICE(obj), s-irq);
+
+memory_region_init_io(s-mmio, obj, uart_ops, s,
+  xlnx.xps-uartlite, R_MAX * 4);
+sysbus_init_mmio(SYS_BUS_DEVICE(obj), s-mmio);
 }
 
 static void xilinx_uartlite_class_init(ObjectClass *klass, void *data)
 {
-SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-sdc-init = xilinx_uartlite_init;
 dc-reset = xilinx_uartlite_reset;
+dc-realize = xilinx_uartlite_realize;
 }
 
 static const TypeInfo xilinx_uartlite_info = {
 .name  = TYPE_XILINX_UARTLITE,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(XilinxUARTLite),
+.instance_init = xilinx_uartlite_init,
 .class_init= xilinx_uartlite_class_init,
 };
 
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH microblaze v1 6/6] intc: xilinx_uartlite: Convert SBD::init - instance_init

[Peter Crosthwaite]

SysBusDevice::init is depracated. Convert to Object::init
as prescribed by QOM conventions.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/intc/xilinx_intc.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index c3682f1..12804ab 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -161,18 +161,16 @@ static void irq_handler(void *opaque, int irq, int level)
 update_irq(p);
 }
 
-static int xilinx_intc_init(SysBusDevice *sbd)
+static void xilinx_intc_init(Object *obj)
 {
-DeviceState *dev = DEVICE(sbd);
-struct xlx_pic *p = XILINX_INTC(dev);
+struct xlx_pic *p = XILINX_INTC(obj);
 
-qdev_init_gpio_in(dev, irq_handler, 32);
-sysbus_init_irq(sbd, p-parent_irq);
+qdev_init_gpio_in(DEVICE(obj), irq_handler, 32);
+sysbus_init_irq(SYS_BUS_DEVICE(obj), p-parent_irq);
 
-memory_region_init_io(p-mmio, OBJECT(p), pic_ops, p, xlnx.xps-intc,
+memory_region_init_io(p-mmio, obj, pic_ops, p, xlnx.xps-intc,
   R_MAX * 4);
-sysbus_init_mmio(sbd, p-mmio);
-return 0;
+sysbus_init_mmio(SYS_BUS_DEVICE(obj), p-mmio);
 }
 
 static Property xilinx_intc_properties[] = {
@@ -183,9 +181,7 @@ static Property xilinx_intc_properties[] = {
 static void xilinx_intc_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k-init = xilinx_intc_init;
 dc-props = xilinx_intc_properties;
 }
 
@@ -193,6 +189,7 @@ static const TypeInfo xilinx_intc_info = {
 .name  = TYPE_XILINX_INTC,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(struct xlx_pic),
+.instance_init = xilinx_intc_init,
 .class_init= xilinx_intc_class_init,
 };
 
-- 
1.9.3.1.ga73a6ad

Re: [Qemu-devel] [PATCH microblaze v1 0/6] Microblaze Device QOM cleanups

[Peter Crosthwaite]

Sorry about the dup mail.

Sent this one with the wrong SMTP server then aborted it. Proper send on list.

Regards,
Peter

On Thu, May 29, 2014 at 7:21 PM, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
 Hi Andreas, Edgar,

 Another 4 users of SysBusDevice::init bite the dust!

 A round of styling cleanup for Microblaze devices as per QOM
 conventions. Tested with published MB test images.

 Regards,
 Peter


 Peter Crosthwaite (6):
   timer: xilinx_timer: Convert to realize()
   net: xilinx_ethlite: Don't reset from init
   net: xilinx_ethlite: Convert to realize()
   char: xilinx_uartlite: Don't reset from init
   char: xilinx_uartlite: Convert to realize()
   intc: xilinx_uartlite: Convert SBD::init - instance_init

  hw/char/xilinx_uartlite.c | 32 +---
  hw/intc/xilinx_intc.c | 17 +++--
  hw/net/xilinx_ethlite.c   | 34 ++
  hw/timer/xilinx_timer.c   | 20 
  4 files changed, 62 insertions(+), 41 deletions(-)

 --
 1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH microblaze v1 1/6] timer: xilinx_timer: Convert to realize()

[Peter Crosthwaite]

SysBusDevice::init is depracated. Convert to Object::init and
Device::realize as prescribed by QOM conventions.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/timer/xilinx_timer.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 3ff1da9..cdb3355 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -204,14 +204,11 @@ static void timer_hit(void *opaque)
 timer_update_irq(t);
 }
 
-static int xilinx_timer_init(SysBusDevice *dev)
+static void xilinx_timer_realize(DeviceState *dev, Error **errp)
 {
 struct timerblock *t = XILINX_TIMER(dev);
 unsigned int i;
 
-/* All timers share a single irq line.  */
-sysbus_init_irq(dev, t-irq);
-
 /* Init all the ptimers.  */
 t-timers = g_malloc0(sizeof t-timers[0] * num_timers(t));
 for (i = 0; i  num_timers(t); i++) {
@@ -226,8 +223,15 @@ static int xilinx_timer_init(SysBusDevice *dev)
 
 memory_region_init_io(t-mmio, OBJECT(t), timer_ops, t, xlnx.xps-timer,
   R_MAX * 4 * num_timers(t));
-sysbus_init_mmio(dev, t-mmio);
-return 0;
+sysbus_init_mmio(SYS_BUS_DEVICE(dev), t-mmio);
+}
+
+static void xilinx_timer_init(Object *obj)
+{
+struct timerblock *t = XILINX_TIMER(obj);
+
+/* All timers share a single irq line.  */
+sysbus_init_irq(SYS_BUS_DEVICE(obj), t-irq);
 }
 
 static Property xilinx_timer_properties[] = {
@@ -240,9 +244,8 @@ static Property xilinx_timer_properties[] = {
 static void xilinx_timer_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-k-init = xilinx_timer_init;
+dc-realize = xilinx_timer_realize;
 dc-props = xilinx_timer_properties;
 }
 
@@ -250,6 +253,7 @@ static const TypeInfo xilinx_timer_info = {
 .name  = TYPE_XILINX_TIMER,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(struct timerblock),
+.instance_init = xilinx_timer_init,
 .class_init= xilinx_timer_class_init,
 };
 
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH microblaze v1 2/6] net: xilinx_ethlite: Don't reset from init

[Peter Crosthwaite]

This zeroing-out of the rxbuf variable (ping pong state) is a reset
side effect. Extract into a proper reset.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

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

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 3a2a6c2..6cbd95d 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -204,6 +204,13 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t 
*buf, size_t size)
 return size;
 }
 
+static void xilinx_ethlite_reset(DeviceState *dev)
+{
+struct xlx_ethlite *s = XILINX_ETHLITE(dev);
+
+s-rxbuf = 0;
+}
+
 static void eth_cleanup(NetClientState *nc)
 {
 struct xlx_ethlite *s = qemu_get_nic_opaque(nc);
@@ -225,7 +232,6 @@ static int xilinx_ethlite_init(SysBusDevice *sbd)
 struct xlx_ethlite *s = XILINX_ETHLITE(dev);
 
 sysbus_init_irq(sbd, s-irq);
-s-rxbuf = 0;
 
 memory_region_init_io(s-mmio, OBJECT(s), eth_ops, s,
   xlnx.xps-ethernetlite, R_MAX * 4);
@@ -251,6 +257,7 @@ static void xilinx_ethlite_class_init(ObjectClass *klass, 
void *data)
 SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
 k-init = xilinx_ethlite_init;
+dc-reset = xilinx_ethlite_reset;
 dc-props = xilinx_ethlite_properties;
 }
 
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Igor Mammedov]

... fixes freeing constant from vl.c by machine_finalize()

Signed-off-by: Igor Mammedov imamm...@redhat.com
---
 hw/core/machine.c |3 +++
 vl.c  |7 +--
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index cbba679..37bd676 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -237,6 +237,8 @@ static void machine_set_firmware(Object *obj, const char 
*value, Error **errp)
 
 static void machine_initfn(Object *obj)
 {
+MachineState *ms = MACHINE(obj);
+
 object_property_add_str(obj, accel,
 machine_get_accel, machine_set_accel, NULL);
 object_property_add_bool(obj, kernel_irqchip,
@@ -251,6 +253,7 @@ static void machine_initfn(Object *obj)
 machine_get_kernel, machine_set_kernel, NULL);
 object_property_add_str(obj, initrd,
 machine_get_initrd, machine_set_initrd, NULL);
+ms-kernel_cmdline = g_strdup();
 object_property_add_str(obj, append,
 machine_get_append, machine_set_append, NULL);
 object_property_add_str(obj, dtb,
diff --git a/vl.c b/vl.c
index 0c15608..b0b1a35 100644
--- a/vl.c
+++ b/vl.c
@@ -4269,14 +4269,9 @@ int main(int argc, char **argv, char **envp)
 boot_strict = qemu_opt_get_bool(opts, strict, false);
 }
 
-if (!kernel_cmdline) {
-kernel_cmdline = ;
-current_machine-kernel_cmdline = (char *)kernel_cmdline;
-}
-
 linux_boot = (kernel_filename != NULL);
 
-if (!linux_boot  *kernel_cmdline != '\0') {
+if (!linux_boot  kernel_cmdline  *kernel_cmdline != '\0') {
 fprintf(stderr, -append only allowed with -kernel option\n);
 exit(1);
 }
-- 
1.7.1

[Qemu-devel] 答复: Expansion Ratio Issue

[Chaos Shu]

On 29 May 2014 08:58, Chaos Shu chaos.s...@live.com wrote:
 1.   Any benchmarks paying attention to TCG code generate quality
 measured by code expansion ratio? Of course I’ve got some news said 
 that the ratio maybe 4 or 5 in X86 to MIPS, that is to say 1 x86 insn 
 to 4 or 5 mips insns, Does it mean the industry level or average 
 level? Any official report given?

No, we don't in general have any benchmarking of TCG codegen. I think if we did 
do benchmarking we'd be interested in performance benchmarking -- code 
expansion ratio doesn't seem like a very interesting thing to measure to me.

[Chaos] Assuming that we just care about running x86 application on arm, in 
general way we translate x86 insn to operations then to arm insns, but that 
means we need more cycles in arm to finish issuing the insns if the code 
expansion ratio is high.

I've investigated some industry methods such as registers map(x86 registers 
maps to arm registers directly but not op on register memory table), actually 
the improvement is limited

And another idea such insn-insn directly, according the runtime statistics, 
mostly we use 20% insn such as br move load, I mean is that possible to map 
those 20% insns directly from x86 to arm and make the left 80% right, but this 
is just original and more important maybe impossible to practice idea, is there 
any research about this on the way? How do you think about this?

 2.   I’ve noticed that once Apple merge from PowerPC to X86, they
 developed the software named Rosetta which is described by apple to be 
 successful, is it the same to Qemu? Any internal infos covered?

It's a similar concept, though as I understand it it focused on doing 
translation for a single application (like QEMU's linux-user mode, not like our 
system emulation mode). I have no idea about its internal design.

[Chaos] I think ARM should provide a runtime library help the customers to 
merge from x86 more smoothly even performance loss, So does arm really get 
that? 

 3.   Assume that we just wanna x86 to arm, so may we can strip out the
 little operations and work on insn to insn such as move in x86 to move 
 in arm, insn level translate but not insn-op-insn, I think there must 
 be someone have ever made this try, anyone got their news?

Certainly if you started from scratch with the intention of doing a more 
specifically targeted design (and in particular if you wanted to do 
single-application translation as your core focus rather than as a bolt-on 
extension to system emulation) you could probably get better performance than 
QEMU. QEMU generally aims to be a general-purpose project, though.

Personally I would (even if doing only x86-to-ARM) still include an 
intermediate representation of some form: the history of compiler design shows 
that it has a lot of utility.

[Chaos] Case be compiled, all syntax information has been stripped out, all we 
get is op_reg_reg different from JVM, we only dance with registers and insn 
without anything, that's the problem.

 4.   Why Qemu use only one TCG runtime, I found a project named PQEMU
 once try to make TCG running on multicore but it’s out of date and got 
 some commercial issues, is there any project trying to make it go?

Not that I currently know of. Truly parallel TCG execution of multiple guest 
cores is a hard problem, especially if you want to produce maintainable solid 
code that can be included upstream, rather than just enough of a prototype to 
demonstrate proof of concept and run some simple benchmarks for an academic 
paper.

[Chaos] After all, what's current status of industry product making x86 
application running on arm? Is still dark night in middle age and I have to 
make big effort to make Qemu to be so? 

Anyway, any infos about that issue is welcome, thanks very much.

Thanks
Chaos

Re: [Qemu-devel] [PATCH RFC 8/8] virtio: add endian-ambivalent support to VirtIODevice

[Paolo Bonzini]

Il 29/05/2014 11:12, Greg Kurz ha scritto:

int virtio_load(VirtIODevice *vdev, QEMUFile *f)
{
[...]
nheads = vring_avail_idx(vdev-vq[i]) - vdev-vq[i].last_avail_idx;
   ^^^
/* Check it isn't doing very strange things with descriptor 
numbers. */
if (nheads  vdev-vq[i].vring.num) {
[...]
}

and

static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
{
[...]
/* The config space */
qemu_get_be16s(f, s-config.cols);
qemu_get_be16s(f, s-config.rows);

qemu_get_be32s(f, max_nr_ports);
tswap32s(max_nr_ports);
 ^^
if (max_nr_ports  tswap32(s-config.max_nr_ports)) {
[...]
}

If we stream subsections after the device descriptor as it is done
in VMState, these two will break because the device endian is stale.

The first one can be easily dealt with: just defer the sanity check
to a post_load function.


Good, we're lucky here.


The second is a bit more tricky: the
virtio serial migrates its config space (target endian) and the
active ports bitmap. The load code assumes max_nr_ports from the
config space tells the size of the ports bitmap... that means the
virtio migration protocol is also contaminated by target endianness. :-\


Ouch.

I guess we could break migration in the case of host endianness != 
target endianness, like this:


/* These three used to be fetched in target endianness and then
 * stored as big endian.  It ended up as little endian if host and
 * target endianness doesn't match.
 *
 * Starting with qemu 2.1, we always store as big endian.  The
 * version wasn't bumped to avoid breaking backwards compatibility.
 * We check the validity of max_nr_ports, and the incorrect-
 * endianness max_nr_ports will be huge, which will abort migration
 * anyway.
 */
uint16_t cols = tswap16(s-config.cols);
uint16_t rows = tswap16(s-config.rows);
uint32_t max_nr_ports = tswap32(s-config.max_nr_ports);

qemu_put_be16s(f, cols);
qemu_put_be16s(f, rows);
qemu_put_be32s(f, max_nr_ports);

...

uint16_t cols, rows;

qemu_get_be16s(f, cols);
qemu_get_be16s(f, rows);
qemu_get_be32s(f, max_nr_ports);

/* Convert back to target endianness when storing into the config
 * space.
 */
s-config.cols = tswap16(cols);
s-config.rows = tswap16(rows);
if (max_nr_ports  tswap32(s-config.max_nr_ports) {
...
}


In the case the answer for above is legacy virtio really sucks then,
is it acceptable to not honor bug-compatibility with older versions and
fix the code ? :)


As long as the common cases don't break, yes.  The question is what are 
the common cases.  Here I think the only non-obscure case that could 
break is x86-on-PPC, and it's not that common.


Paolo

Re: [Qemu-devel] [PATCH v2 6/7] virtio: fix virtio-blk child refcount in transports

[Paolo Bonzini]

Il 29/05/2014 11:11, Peter Crosthwaite ha scritto:

 object_initialize() leaves the object with a refcount of 1.
 object_property_add_child() adds its own reference which is dropped
 again when the property is deleted.

 The upshot of this is that we always have a refcount = 1.  Upon hot
 unplug the virtio-blk child is not finalized!


Doesn't this suggest that hot unplug is what's broken? My
understanding (which is fresh and not 100% yet) is the original == 1
refcount should be dropped at object deletion time which is this sense
would be unplug time. This would mean that hot-unplug should
explicitly object_unref the object (should the intention of hot-unplug
be to always finalise the device?).


I think it makes sense either way, as long as it's used consistently.

Paolo

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Marcel Apfelbaum]

On Thu, 2014-05-29 at 11:47 +0200, Igor Mammedov wrote:
 ... fixes freeing constant from vl.c by machine_finalize()
Nice cleanup, thanks!

Reviewed-by: Marcel Apfelbaum marce...@redhat.com

 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  hw/core/machine.c |3 +++
  vl.c  |7 +--
  2 files changed, 4 insertions(+), 6 deletions(-)
 
 diff --git a/hw/core/machine.c b/hw/core/machine.c
 index cbba679..37bd676 100644
 --- a/hw/core/machine.c
 +++ b/hw/core/machine.c
 @@ -237,6 +237,8 @@ static void machine_set_firmware(Object *obj, const char 
 *value, Error **errp)
  
  static void machine_initfn(Object *obj)
  {
 +MachineState *ms = MACHINE(obj);
 +
  object_property_add_str(obj, accel,
  machine_get_accel, machine_set_accel, NULL);
  object_property_add_bool(obj, kernel_irqchip,
 @@ -251,6 +253,7 @@ static void machine_initfn(Object *obj)
  machine_get_kernel, machine_set_kernel, NULL);
  object_property_add_str(obj, initrd,
  machine_get_initrd, machine_set_initrd, NULL);
 +ms-kernel_cmdline = g_strdup();
  object_property_add_str(obj, append,
  machine_get_append, machine_set_append, NULL);
  object_property_add_str(obj, dtb,
 diff --git a/vl.c b/vl.c
 index 0c15608..b0b1a35 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -4269,14 +4269,9 @@ int main(int argc, char **argv, char **envp)
  boot_strict = qemu_opt_get_bool(opts, strict, false);
  }
  
 -if (!kernel_cmdline) {
 -kernel_cmdline = ;
 -current_machine-kernel_cmdline = (char *)kernel_cmdline;
 -}
 -
  linux_boot = (kernel_filename != NULL);
  
 -if (!linux_boot  *kernel_cmdline != '\0') {
 +if (!linux_boot  kernel_cmdline  *kernel_cmdline != '\0') {
  fprintf(stderr, -append only allowed with -kernel option\n);
  exit(1);
  }

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Andreas Färber]

Am 29.05.2014 11:47, schrieb Igor Mammedov:
 ... fixes freeing constant from vl.c by machine_finalize()
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com

Did you check whether there are any others in need of changes? I could
imagine kernel_irqchip does, and I see that we forgot to fix the
underscore in the property name, damn. Marcel, can you please look into
fixing that? I had outlined how to.

Andreas

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

[Qemu-devel] [PATCH] slirp: Remove unused zero_ethaddr[] variable

[Peter Maydell]

The zero_ethaddr[] array is never used; delete it.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
clang 3.4 warns about unused const variables like this and
also about unused functions; we have over 400 such warnings
currently, of which some are simple forgot to clean up
redundant code like this one, some are deliberate in some
sense (usually the function/var is used in one config but not
another), and some are outright bugs. Anybody feel like wading
through them? :-)

 slirp/slirp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index 3fb48a4..b7f3726 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -37,8 +37,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
 0x52, 0x55, 0x00, 0x00, 0x00, 0x00
 };
 
-static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
-
 u_int curtime;
 
 static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
-- 
1.9.2

[Qemu-devel] [PATCH] hw/i386/pc.c: Remove unused parallel_io and parallel_irq variables

[Peter Maydell]

The variables parallel_io and parallel_irq are unused; delete them.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
 hw/i386/pc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e6369d5..32d1632 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -922,9 +922,6 @@ static const int ne2000_io[NE2000_NB_MAX] = { 0x300, 0x320, 
0x340, 0x360,
   0x280, 0x380 };
 static const int ne2000_irq[NE2000_NB_MAX] = { 9, 10, 11, 3, 4, 5 };
 
-static const int parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
-static const int parallel_irq[MAX_PARALLEL_PORTS] = { 7, 7, 7 };
-
 void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
 {
 static int nb_ne2k = 0;
-- 
1.9.2

[Qemu-devel] [PATCH] hw/pci-host/ppce500: Fix typo in vmstate definition

[Peter Maydell]

Fix a typo in the ppce500_pci vmstate definition which meant that
we were migrating the struct pci_inbound using the vmstate for
pci_outbound. Fortunately the two structures have exactly the same
format at the moment (four uint32_ts) so this was harmless, and
we can correcting the typo without a migration compatibility
break because the vmstate name doesn't go out on the wire.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
Big fat disclaimer: I believe the remark about this not
being a compat break to be true, but I haven't tested it!
---
 hw/pci-host/ppce500.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index c80b7cb..5ce7433 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -311,7 +311,7 @@ static const VMStateDescription vmstate_ppce500_pci = {
 VMSTATE_STRUCT_ARRAY(pob, PPCE500PCIState, PPCE500_PCI_NR_POBS, 1,
  vmstate_pci_outbound, struct pci_outbound),
 VMSTATE_STRUCT_ARRAY(pib, PPCE500PCIState, PPCE500_PCI_NR_PIBS, 1,
- vmstate_pci_outbound, struct pci_inbound),
+ vmstate_pci_inbound, struct pci_inbound),
 VMSTATE_UINT32(gasket_time, PPCE500PCIState),
 VMSTATE_END_OF_LIST()
 }
-- 
1.9.2

Re: [Qemu-devel] Expansion Ratio Issue

[Alex Bennée]

Peter Maydell peter.mayd...@linaro.org writes:

 On 29 May 2014 08:58, Chaos Shu chaos.s...@live.com wrote:
 1.   Any benchmarks paying attention to TCG code generate quality
 measured by code expansion ratio? Of course I’ve got some news said that the
 ratio maybe 4 or 5 in X86 to MIPS, that is to say 1 x86 insn to 4 or 5 mips
 insns, Does it mean the industry level or average level? Any official report
 given?

 No, we don't in general have any benchmarking of TCG
 codegen. I think if we did do benchmarking we'd be interested
 in performance benchmarking -- code expansion ratio doesn't
 seem like a very interesting thing to measure to me.

Not to mention that raw instruction counts are probably misleading
compared to the effect you can get from instruction ordering and cache
behaviour.

 2.   I’ve noticed that once Apple merge from PowerPC to X86, they
 developed the software named Rosetta which is described by apple to be
 successful, is it the same to Qemu? Any internal infos covered?

 It's a similar concept, though as I understand it it focused
 on doing translation for a single application (like QEMU's
 linux-user mode, not like our system emulation mode). I have
 no idea about its internal design.

Rosetta was based on QuickTransit from Transitive (since bought by IBM).
It's broadly analogous to QEMU's linux-user mode emulation although it
attempted a more complete separation between translated and native
processes. In the QEMU world all the processes can see each other
which can cause issues if they are expecting certain endianess on
wire-formats.

The biggest difference internally is the translator was IR based, it
built up a DAG of operations which it manipulated/optimised much like a
compiler does before generating the final target code. QEMU instead
generates a simple set of TCG ops for each instruction which after a
little optimisation spits out a set of target instructions.

QuickTransit was certainly pretty high performance compared what else
was out there at the time. From memory it implemented a number of other
features to get this speed:

 * Group blocks - hot paths of basic blocks would be regenerated as a
   single block.
 * Block cache - it would cache previous translations to avoid heavy
   start-up cost
 * Native binding - it could optionally detect special code paths in
   translated code and replace them with a direct binding to a native
   code (e.g. call the native memcpy rather than translate the guest
   version).


 3.   Assume that we just wanna x86 to arm, so may we can strip out the
 little operations and work on insn to insn such as move in x86 to move in
 arm, insn level translate but not insn-op-insn, I think there must be
 someone have ever made this try, anyone got their news?

 Certainly if you started from scratch with the intention of
 doing a more specifically targeted design (and in particular
 if you wanted to do single-application translation as your core
 focus rather than as a bolt-on extension to system emulation)
 you could probably get better performance than QEMU. QEMU
 generally aims to be a general-purpose project, though.

 Personally I would (even if doing only x86-to-ARM) still
 include an intermediate representation of some form: the
 history of compiler design shows that it has a lot of utility.

 4.   Why Qemu use only one TCG runtime, I found a project named PQEMU
 once try to make TCG running on multicore but it’s out of date and got some
 commercial issues, is there any project trying to make it go?

In the linux-user case you do utilise multiple core with multiple
instances of QEMU running (which is handy for package building type
tasks). However fixing QEMU for fully multi-threaded translation is a
hard task. You may even find you don't get that much from it as ideally
you should spend more time running translated code than doing the
transaltion.


 Not that I currently know of. Truly parallel TCG execution
 of multiple guest cores is a hard problem, especially if you
 want to produce maintainable solid code that can be included
 upstream, rather than just enough of a prototype to demonstrate
 proof of concept and run some simple benchmarks for an academic
 paper.


 thanks
 -- PMM

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Igor Mammedov]

On Thu, 29 May 2014 12:47:45 +0200
Andreas Färber afaer...@suse.de wrote:

 Am 29.05.2014 11:47, schrieb Igor Mammedov:
  ... fixes freeing constant from vl.c by machine_finalize()
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 Did you check whether there are any others in need of changes? I could
 imagine kernel_irqchip does, and I see that we forgot to fix the
Nope, the only wrong freeing I've noticed was only this one.

 underscore in the property name, damn. Marcel, can you please look into
 fixing that? I had outlined how to.
 
 Andreas
 

Re: [Qemu-devel] [PATCH] libcacard: improve documentation

[Alon Levy]

On 05/28/2014 07:15 PM, Paolo Bonzini wrote:
 Using the file-backed smartcard backend is black magic, but it can
 be useful if your only smartcard bricks itself if it is accessed
 the wrong way too many times.
 
 Complete the documentation to include the art of creating certificates
 and using them with QEMU, based on Ray Strode's useful tutorial at
 https://blogs.gnome.org/halfline/2013/09/08/another-smartcard-post/
 but with ccid-card-emulated or vscclient instead of SPICE.
 
 Cc: Alon Levy al...@redhat.com
 Cc: Ray Strode rstr...@redhat.com

Did you mean Robert Relyea rrel...@redhat.com ?

Thanks, looks good.
Reviewed-by: Alon Levy al...@redhat.com

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  docs/ccid.txt | 80 
 ++-
  1 file changed, 63 insertions(+), 17 deletions(-)
 
 diff --git a/docs/ccid.txt b/docs/ccid.txt
 index 83c174d..c7fda6d 100644
 --- a/docs/ccid.txt
 +++ b/docs/ccid.txt
 @@ -47,6 +47,7 @@ In ubuntu/debian:
  Configuring and building:
  ./configure --enable-smartcard  make
  
 +
  3. Using ccid-card-emulated with hardware
  
  Assuming you have a working smartcard on the host with the current
 @@ -54,19 +55,55 @@ user, using NSS, qemu acts as another NSS client using 
 ccid-card-emulated:
  
  qemu -usb -device usb-ccid -device ccid-card-emulated
  
 -4. Using ccid-card-emulated with certificates
  
 -You must create the certificates. This is a one time process. We use NSS
 -certificates:
 +4. Using ccid-card-emulated with certificates stored in files
 +
 +You must create the CA and card certificates. This is a one time process.
 +We use NSS certificates:
  
 -certutil -d /etc/pki/nssdb -x -t CT,CT,CT -S -s CN=cert1 -n cert1
 +mkdir fake-smartcard
 +cd fake-smartcard
 +certutil -N -d sql:$PWD
 +certutil -S -d sql:$PWD -s CN=Fake Smart Card CA -x -t TC,TC,TC -n 
 fake-smartcard-ca
 +certutil -S -d sql:$PWD -t ,, -s CN=John Doe -n id-cert -c 
 fake-smartcard-ca
 +certutil -S -d sql:$PWD -t ,, -s CN=John Doe (signing) --nsCertType 
 smime -n signing-cert -c fake-smartcard-ca
 +certutil -S -d sql:$PWD -t ,, -s CN=John Doe (encryption) --nsCertType 
 sslClient -n encryption-cert -c fake-smartcard-ca
  
  Note: you must have exactly three certificates.
  
 -Assuming the current user can access the certificates (use certutil -L to
 -verify), you can use the emulated card type with the certificates backend:
 +You can use the emulated card type with the certificates backend:
 +
 +qemu -usb -device usb-ccid -device 
 ccid-card-emulated,backend=certificates,db=sql:$PWD,cert1=id-cert,cert2=signing-cert,cert3=encryption-cert
 +
 +To use the certificates in the guest, export the CA certificate:
 +
 +certutil -L -r -d sql:$PWD -o fake-smartcard-ca.cer -n fake-smartcard-ca
 +
 +and import it in the guest:
 +
 +certutil -A -d /etc/pki/nssdb -i fake-smartcard-ca.cer -t TC,TC,TC -n 
 fake-smartcard-ca
 +
 +In a Linux guest you can then use the CoolKey PKCS #11 module to access
 +the card:
 +
 +certutil -d /etc/pki/nssdb -L -h all
 +
 +It will prompt you for the PIN (which is the password you assigned to the
 +certificate database early on), and then show you all three certificates
 +together with the manually imported CA cert:
 +
 +Certificate NicknameTrust Attributes
 +fake-smartcard-ca   CT,C,C
 +John Doe:CAC ID Certificate u,u,u
 +John Doe:CAC Email Signature Certificateu,u,u
 +John Doe:CAC Email Encryption Certificate   u,u,u
 +
 +If this does not happen, CoolKey is not installed or not registered with
 +NSS.  Registration can be done from Firefox or the command line:
 +
 +modutil -dbdir /etc/pki/nssdb -add CAC Module -libfile 
 /usr/lib64/pkcs11/libcoolkeypk11.so
 +modutil -dbdir /etc/pki/nssdb -list
  
 -qemu -usb -device usb-ccid -device 
 ccid-card-emulated,backend=certificates,cert1=cert1,cert2=cert2,cert3=cert3
  
  5. Using ccid-card-passthru with client side hardware
  
 @@ -74,15 +111,23 @@ on the host specify the ccid-card-passthru device with a 
 suitable chardev:
  
  qemu -chardev socket,server,host=0.0.0.0,port=2001,id=ccid,nowait -usb 
 -device usb-ccid -device ccid-card-passthru,chardev=ccid
  
 -on the client run vscclient, built when you built the libcacard library:
 -libcacard/vscclient qemu-host 2001
 +on the client run vscclient, built when you built QEMU:
 +
 +vscclient qemu-host 2001
 +
  
  6. Using ccid-card-passthru with client side certificates
  
 -Run qemu as per #5, and run vscclient as follows:
 -(Note: vscclient command line interface is in a state of change)
 +This case is not particularly useful, but you can use it to debug
 +your setup if #4 works but #5 does not.
 +
 +Follow instructions as per #4, except run QEMU and vscclient as follows:
 +Run qemu as per #5, and run vscclient from the fake-smartcard
 +directory as follows:
 +
 +

Re: [Qemu-devel] [PATCH] hw/i386/pc.c: Remove unused parallel_io and parallel_irq variables

[Michael S. Tsirkin]

On Thu, May 29, 2014 at 12:01:49PM +0100, Peter Maydell wrote:
 The variables parallel_io and parallel_irq are unused; delete them.
 
 Signed-off-by: Peter Maydell peter.mayd...@linaro.org


Thanks, applied.


Reviewed-by: Michael S. Tsirkin m...@redhat.com

 ---
  hw/i386/pc.c | 3 ---
  1 file changed, 3 deletions(-)
 
 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index e6369d5..32d1632 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -922,9 +922,6 @@ static const int ne2000_io[NE2000_NB_MAX] = { 0x300, 
 0x320, 0x340, 0x360,
0x280, 0x380 };
  static const int ne2000_irq[NE2000_NB_MAX] = { 9, 10, 11, 3, 4, 5 };
  
 -static const int parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
 -static const int parallel_irq[MAX_PARALLEL_PORTS] = { 7, 7, 7 };
 -
  void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
  {
  static int nb_ne2k = 0;
 -- 
 1.9.2

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Marcel Apfelbaum]

On Thu, 2014-05-29 at 12:47 +0200, Andreas Färber wrote:
 Am 29.05.2014 11:47, schrieb Igor Mammedov:
  ... fixes freeing constant from vl.c by machine_finalize()
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 Did you check whether there are any others in need of changes? I could
 imagine kernel_irqchip does, and I see that we forgot to fix the
 underscore in the property name, damn. Marcel, can you please look into
 fixing that? I had outlined how to.
Hi Andreas,

I didn't forget to do that, I responded on the mail thread why I think we 
shouldn't,
even if is against QOM best practices.
I'll copy-paste:

Anyway, the real problem here is that what is elegant in this solution is:
machine_opts = qemu_get_machine_opts();
if (qemu_opt_foreach(machine_opts, object_set_property, current_machine, 1) 
 0) {
object_unref(OBJECT(current_machine));
exit(1);
}
It automatically fills in the machine state properties with the options from 
the command line.
It will work with machine sub-types that have specific properties without the 
need
to manually add it to vl.c. The error flow is also elegant (if a sub-type does 
not have the
user supplied property).

If we use machine-specific wrappers to convert _ - - we loose the above.
As an alternative we could rename the machine option to use -...

Thanks,
Marcel
 
 
 Andreas
 

[Qemu-devel] [PATCH v4 02/10] trace: [tcg] Argument type transformation rules

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 scripts/tracetool/transform.py |  166 
 1 file changed, 166 insertions(+)
 create mode 100644 scripts/tracetool/transform.py

diff --git a/scripts/tracetool/transform.py b/scripts/tracetool/transform.py
new file mode 100644
index 000..fc5e679
--- /dev/null
+++ b/scripts/tracetool/transform.py
@@ -0,0 +1,166 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+
+Type-transformation rules.
+
+
+__author__ = Lluís Vilanova vilan...@ac.upc.edu
+__copyright__  = Copyright 2012-2014, Lluís Vilanova vilan...@ac.upc.edu
+__license__= GPL version 2 or (at your option) any later version
+
+__maintainer__ = Stefan Hajnoczi
+__email__  = stefa...@linux.vnet.ibm.com
+
+
+def _transform_type(type_, trans):
+if isinstance(trans, str):
+return trans
+elif isinstance(trans, dict):
+if type_ in trans:
+return _transform_type(type_, trans[type_])
+elif None in trans:
+return _transform_type(type_, trans[None])
+else:
+return type_
+elif callable(trans):
+return trans(type_)
+else:
+raise ValueError(Invalid type transformation rule: %s % trans)
+
+
+def transform_type(type_, *trans):
+Return a new type transformed according to the given rules.
+
+Applies each of the transformation rules in trans in order.
+
+If an element of trans is a string, return it.
+
+If an element of trans is a function, call it with type_ as its only
+argument.
+
+If an element of trans is a dict, search type_ in its keys. If type_ is
+a key, use the value as a transformation rule for type_. Otherwise, if
+None is a key use the value as a transformation rule for type_.
+
+Otherwise, return type_.
+
+Parameters
+--
+type_ : str
+Type to transform.
+trans : list of function or dict
+Type transformation rules.
+
+if len(trans) == 0:
+raise ValueError
+res = type_
+for t in trans:
+res = _transform_type(res, t)
+return res
+
+
+##
+# tcg - host
+
+def _tcg_2_host(type_):
+if type_ == TCGv:
+# force a fixed-size type (target-independent)
+return uint64_t
+else:
+return type_
+
+TCG_2_HOST = {
+TCGv_i32: uint32_t,
+TCGv_i64: uint64_t,
+TCGv_ptr: void *,
+None: _tcg_2_host,
+}
+
+
+##
+# host - host compatible with tcg sizes
+
+HOST_2_TCG_COMPAT = {
+uint8_t: uint32_t,
+}
+
+
+##
+# host/tcg - tcg
+
+def _host_2_tcg(type_):
+if type_.startswith(TCGv):
+return type_
+raise ValueError(Don't know how to translate '%s' into a TCG type\n % 
type_)
+
+HOST_2_TCG = {
+uint32_t: TCGv_i32,
+uint64_t: TCGv_i64,
+void *  : TCGv_ptr,
+None: _host_2_tcg,
+}
+
+
+##
+# tcg - tcg helper definition
+
+def _tcg_2_helper_def(type_):
+if type_ == TCGv:
+return target_ulong
+else:
+return type_
+
+TCG_2_TCG_HELPER_DEF = {
+TCGv_i32: uint32_t,
+TCGv_i64: uint64_t,
+TCGv_ptr: void *,
+None: _tcg_2_helper_def,
+}
+
+
+##
+# tcg - tcg helper declaration
+
+def _tcg_2_tcg_helper_decl_error(type_):
+raise ValueError(Don't know how to translate type '%s' into a TCG helper 
declaration type\n % type_)
+
+TCG_2_TCG_HELPER_DECL = {
+TCGv: tl,
+TCGv_ptr: ptr,
+TCGv_i32: i32,
+TCGv_i64: i64,
+None: _tcg_2_tcg_helper_decl_error,
+}
+
+
+##
+# host/tcg - tcg temporal constant allocation
+
+def _host_2_tcg_tmp_new(type_):
+if type_.startswith(TCGv):
+return tcg_temp_new_nop
+raise ValueError(Don't know how to translate type '%s' into a TCG 
temporal allocation % type_)
+
+HOST_2_TCG_TMP_NEW = {
+uint32_t: tcg_const_i32,
+uint64_t: tcg_const_i64,
+void *  : tcg_const_ptr,
+None: _host_2_tcg_tmp_new,
+}
+
+
+##
+# host/tcg - tcg temporal constant deallocation
+
+def _host_2_tcg_tmp_free(type_):
+if type_.startswith(TCGv):
+return tcg_temp_free_nop
+raise ValueError(Don't know how to translate type '%s' into a TCG 
temporal deallocation % type_)
+
+HOST_2_TCG_TMP_FREE = {
+uint32_t: tcg_temp_free_i32,
+uint64_t: tcg_temp_free_i64,
+void *  : tcg_temp_free_ptr,
+None: _host_2_tcg_tmp_free,
+}

[Qemu-devel] [PATCH v4 00/10] trace: [tcg] Allow tracing guest events in TCG-generated code

[Lluís Vilanova]

NOTE: TCG code for execution-time event tracing is always generated, regardless
  of wether the event has been dynamically disabled or not (unless the event
  has the static disable property).

  This approach keeps this series simple, and a future series will handle
  the case of having per-CPU tracing states for these events, and only
  generate the code if the event is dynamicaly enabled.

Adds the base ability to specify which events in the trace-events file may be
used to trace guest activity in the TCG code (using the tcg event propery).

An event with that property actually generates two independent events
${name}_trans and ${name}_exec, and a set of conveniency functions. Having
two events allows us to trace both translation and execution of guest code
(e.g., generating vs executing a basic block). See the first patch for more
information on its usage.

Files generating guest code (TCG) must include trace-tcg.h.

The flow of the generated routines is (convenience wrappers are inlined):


[At translation time]

* trace_${name}_tcg(bool, TCGv)
  Declared: trace/generated-tcg-tracers.h
  Defined : trace/generated-tcg-tracers.h

  Invokes trace_${name}_trans (with all non-TCG arguments) and
  gen_helper_trace_${name}_exec (with all arguments).

* trace_${name}_trans(bool)
  Declared: trace/generated-tracers.h
  Defined : trace/generated-tracers.h

  Invokes the actual tracing backends for the translation-time event.

* gen_helper_trace_${name}_exec(bool, TCGv)
  Declared: trace/generated-helpers.h
  Defined : trace/generated-helpers.h

  Convenience wrapper that will allocate (and free) TCG temporaries for all
  non-TCG arguments before calling gen_helper_trace_${name}_exec_proxy.

* gen_helper_trace_${name}_exec_proxy(TCGi32, TCGv)
  Declared: trace/generated-helpers.h
  Defined : trace/generated-helpers.h (using helper machinery)

  The actual TCG helper function, created using QEMU's TCG helper machinery.


[At execution time]

* helper_trace_${name}_exec_proxy(uint32_t, uint64_t)
  Declared: trace/generated-helpers.h
  Defined : trace/generated-helpers.c

  Convenience wrapper that casts arguments to the appropriate type before
  calling trace_${name}_exec.

  This is necessary because TCG helpers can only receive a limited number of
  argument types (e.g., must use 'uint32_t' instead of 'bool').

* trace_${name}_exec(bool, uint64_t)
  Declared: trace/generated-tracers.h
  Defined : trace/generated-tracers.h

  Invokes the actual tracing backends for the execution-time event.

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---

Changes in v4:

* Rebase on d7d3d60 (use new helper header machinery).

Changes in v3:

* None (re-send with cover).

Changes in v2:

* Split the guest memory access event out of this series.
* Generate two independent events from each tcg event (one for
  translation-time events and one for execution-time events).


Lluís Vilanova (10):
  trace: [tcg] Add documentation
  trace: [tcg] Argument type transformation rules
  trace: [tcg] Argument type transformation machinery
  trace: [tcg] Add 'tcg' event property
  trace: [tcg] Declare TCG tracing helper routines
  trace: [tcg] Define TCG tracing helper routines
  trace: [tcg] Include TCG-tracing helpers
  trace: [tcg] Generate TCG tracing routines
  trace: [tcg] Include event definitions in trace.h
  trace: [tcg] Include TCG-tracing header on all targets


 .gitignore   |3 +
 Makefile |5 +
 Makefile.objs|7 +
 Makefile.target  |5 +
 docs/tracing.txt |   40 +++
 include/exec/helper-gen.h|3 +
 include/exec/helper-proto.h  |1 
 include/exec/helper-tcg.h|1 
 include/trace-tcg.h  |7 +
 include/trace.h  |1 
 scripts/tracetool/__init__.py|   95 -
 scripts/tracetool/format/events_h.py |5 +
 scripts/tracetool/format/tcg_h.py|   57 ++
 scripts/tracetool/format/tcg_helper_c.py |   50 +
 scripts/tracetool/format/tcg_helper_h.py |   90 
 scripts/tracetool/transform.py   |  166 ++
 target-alpha/translate.c |3 +
 target-arm/translate-a64.c   |2 
 target-arm/translate.c   |3 +
 target-cris/translate.c  |3 +
 target-i386/translate.c  |3 +
 target-lm32/translate.c  |3 +
 target-m68k/translate.c  |3 +
 target-microblaze/translate.c|3 +
 target-mips/translate.c  |3 +
 target-openrisc/translate.c  |3 +
 target-ppc/translate.c   |3 +
 target-s390x/translate.c |2 
 target-sh4/translate.c

[Qemu-devel] [PATCH v4 04/10] trace: [tcg] Add 'tcg' event property

[Lluís Vilanova]

Transforms event:

  tcg name(...) ..., ...

into two internal events:

  tcg-trans name_trans(...) ...
  tcg-exec name_exec(...) ...

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 scripts/tracetool/__init__.py|   64 +++---
 scripts/tracetool/format/events_h.py |5 +++
 2 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index bd3fd85..de48c80 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -19,6 +19,7 @@ import weakref
 
 import tracetool.format
 import tracetool.backend
+import tracetool.transform
 
 
 def error_write(*lines):
@@ -137,9 +138,14 @@ class Event(object):
 The event arguments.
 
 
-_CRE = 
re.compile(((?Pprops.*)\s+)?(?Pname[^(\s]+)\((?Pargs[^)]*)\)\s*(?Pfmt\.*)?)
+_CRE = re.compile(((?Pprops.*)\s+)?
+  (?Pname[^(\s]+)
+  \((?Pargs[^)]*)\)
+  \s*
+  (?:(?:(?Pfmt_trans\.+),)?\s*(?Pfmt\.+))?
+  \s*)
 
-_VALID_PROPS = set([disable])
+_VALID_PROPS = set([disable, tcg, tcg-trans, tcg-exec])
 
 def __init__(self, name, props, fmt, args, orig=None):
 
@@ -149,8 +155,8 @@ class Event(object):
 Event name.
 props : list of str
 Property names.
-fmt : str
-Event printing format.
+fmt : str, list of str
+Event printing format (or formats).
 args : Arguments
 Event arguments.
 orig : Event or None
@@ -170,6 +176,7 @@ class Event(object):
 if len(unknown_props)  0:
 raise ValueError(Unknown properties: %s
  % , .join(unknown_props))
+assert isinstance(self.fmt, str) or len(self.fmt) == 2
 
 def copy(self):
 Create a new copy.
@@ -192,16 +199,32 @@ class Event(object):
 name = groups[name]
 props = groups[props].split()
 fmt = groups[fmt]
+fmt_trans = groups[fmt_trans]
+if len(fmt_trans)  0:
+fmt = [fmt_trans, fmt]
 args = Arguments.build(groups[args])
 
+if tcg-trans in props:
+raise ValueError(Invalid property 'tcg-trans')
+if tcg-exec in props:
+raise ValueError(Invalid property 'tcg-exec')
+if tcg not in props and not isinstance(fmt, str):
+raise ValueError(Only events with 'tcg' property can have two 
formats)
+if tcg in props and isinstance(fmt, str):
+raise ValueError(Events with 'tcg' property must have two 
formats)
+
 return Event(name, props, fmt, args)
 
 def __repr__(self):
 Evaluable string representation for this object.
+if isinstance(self.fmt, str):
+fmt = self.fmt
+else:
+fmt = %s, %s % (self.fmt[0], self.fmt[1])
 return Event('%s %s(%s) %s') % ( .join(self.properties),
   self.name,
   self.args,
-  self.fmt)
+  fmt)
 
 QEMU_TRACE   = trace_%(name)s
 
@@ -300,4 +323,35 @@ def generate(fevents, format, backends,
 
 events = _read_events(fevents)
 
+# transform TCG-enabled events
+new_events = []
+for event in events:
+if tcg not in event.properties:
+new_events.append(event)
+else:
+event_trans = event.copy()
+event_trans.name += _trans
+event_trans.properties += [tcg-trans]
+event_trans.fmt = event.fmt[0]
+args_trans = []
+for atrans, aorig in zip(
+event_trans.transform(tracetool.transform.TCG_2_HOST).args,
+event.args):
+if atrans == aorig:
+args_trans.append(atrans)
+event_trans.args = Arguments(args_trans)
+event_trans = event_trans.copy()
+
+event_exec = event.copy()
+event_exec.name += _exec
+event_exec.properties += [tcg-exec]
+event_exec.fmt = event.fmt[1]
+event_exec = event_exec.transform(tracetool.transform.TCG_2_HOST)
+
+new_event = [event_trans, event_exec]
+event.event_trans, event.event_exec = new_event
+
+new_events.extend(new_event)
+events = new_events
+
 tracetool.format.generate(events, format, backend)
diff --git a/scripts/tracetool/format/events_h.py 
b/scripts/tracetool/format/events_h.py
index 25d913b..9f114a3 100644
--- a/scripts/tracetool/format/events_h.py
+++ b/scripts/tracetool/format/events_h.py
@@ -40,6 +40,11 @@ def generate(events, backend):
 enabled = 0
 else:
 enabled = 1
+if tcg-trans in e.properties:
+# a single define for the two sub-events
+   

[Qemu-devel] [PATCH v4 03/10] trace: [tcg] Argument type transformation machinery

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 scripts/tracetool/__init__.py |   30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index e8e8edc..bd3fd85 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -15,6 +15,7 @@ __email__  = stefa...@linux.vnet.ibm.com
 
 import re
 import sys
+import weakref
 
 import tracetool.format
 import tracetool.backend
@@ -108,6 +109,18 @@ class Arguments:
 List of argument types.
 return [ type_ for type_, _ in self._args ]
 
+def transform(self, *trans):
+Return a new Arguments instance with transformed types.
+
+The types in the resulting Arguments instance are transformed according
+to tracetool.transform.transform_type.
+
+res = []
+for type_, name in self._args:
+res.append((tracetool.transform.transform_type(type_, *trans),
+name))
+return Arguments(res)
+
 
 class Event(object):
 Event description.
@@ -128,7 +141,7 @@ class Event(object):
 
 _VALID_PROPS = set([disable])
 
-def __init__(self, name, props, fmt, args):
+def __init__(self, name, props, fmt, args, orig=None):
 
 Parameters
 --
@@ -140,12 +153,19 @@ class Event(object):
 Event printing format.
 args : Arguments
 Event arguments.
+orig : Event or None
+Original Event before transformation.
 
 self.name = name
 self.properties = props
 self.fmt = fmt
 self.args = args
 
+if orig is None:
+self.original = weakref.ref(self)
+else:
+self.original = orig
+
 unknown_props = set(self.properties) - self._VALID_PROPS
 if len(unknown_props)  0:
 raise ValueError(Unknown properties: %s
@@ -190,6 +210,14 @@ class Event(object):
 fmt = Event.QEMU_TRACE
 return fmt % {name: self.name}
 
+def transform(self, *trans):
+Return a new Event with transformed Arguments.
+return Event(self.name,
+ list(self.properties),
+ self.fmt,
+ self.args.transform(*trans),
+ self)
+
 
 def _read_events(fobj):
 res = []

[Qemu-devel] [PATCH v4 01/10] trace: [tcg] Add documentation

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 docs/tracing.txt |   40 
 1 file changed, 40 insertions(+)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index c6ab1c1..2e035a5 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -307,3 +307,43 @@ guard such computations and avoid its compilation when the 
event is disabled:
 You can check both if the event has been disabled and is dynamically enabled at
 the same time using the 'trace_event_get_state' routine (see header
 trace/control.h for more information).
+
+=== tcg ===
+
+Guest code generated by TCG can be traced by defining an event with the tcg
+event property. Internally, this property generates two events:
+eventname_trans to trace the event at translation time, and
+eventname_exec to trace the event at execution time.
+
+Instead of using these two events, you should instead use the function
+trace_eventname_tcg during translation (TCG code generation). This function
+will automatically call trace_eventname_trans, and will generate the
+necessary TCG code to call trace_eventname_exec during guest code 
execution.
+
+Events with the tcg property can be declared in the trace-events file with 
a
+mix of native and TCG types, and trace_eventname_tcg will gracefully 
forward
+them to the eventname_trans and eventname_exec events. Since TCG values
+are not known at translation time, these are ignored by the eventname_trans
+event. Because of this, the entry in the trace-events file needs two printing
+formats (separated by a comma):
+
+tcg foo(uint8_t a1, TCGv_i32 a2) a1=%d, a1=%d a2=%d
+
+For example:
+
+#include trace-tcg.h
+
+void some_disassembly_func (...)
+{
+uint8_t a1 = ...;
+TCGv_i32 a2 = ...;
+trace_foo_tcg(a1, a2);
+}
+
+This will immediately call:
+
+void trace_foo_trans(uint8_t a1);
+
+and will generate the TCG code to call:
+
+void trace_foo(uint8_t a1, uint32_t a2);

[Qemu-devel] [PATCH v4 05/10] trace: [tcg] Declare TCG tracing helper routines

[Lluís Vilanova]

Generate header trace/generated-helpers.h with the necessary TCG helper
declarations for tracing events in guest code at execution time:

* gen_helper_trace_${event}_exec

  Routine to transform mixed native and TCG argument types to TCG types and call
  TCG helper 'gen_helper_trace_${event}_exec_proxy'.

* helper_trace_${event}_exec_proxy

  TCG helper to cast TCG-compatible argument types to native types and call
  tracing routine 'trace_${event}_exec'.

NOTE: 'gen_gelper_trace_${event}_exec_proxy' is generated by the TCG helper
  machinery.

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 .gitignore   |1 
 Makefile |2 +
 scripts/tracetool/__init__.py|1 
 scripts/tracetool/format/tcg_helper_h.py |   90 ++
 trace/Makefile.objs  |   24 ++--
 5 files changed, 113 insertions(+), 5 deletions(-)
 create mode 100644 scripts/tracetool/format/tcg_helper_h.py

diff --git a/.gitignore b/.gitignore
index c658613..cc6f6c4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -11,6 +11,7 @@
 /trace/generated-tracers.dtrace
 /trace/generated-events.h
 /trace/generated-events.c
+/trace/generated-helpers.h
 /trace/generated-ust-provider.h
 /trace/generated-ust.c
 /libcacard/trace/generated-tracers.c
diff --git a/Makefile b/Makefile
index 3c8f19c..e3ac3fd 100644
--- a/Makefile
+++ b/Makefile
@@ -57,6 +57,8 @@ GENERATED_HEADERS += trace/generated-tracers-dtrace.h
 endif
 GENERATED_SOURCES += trace/generated-tracers.c
 
+GENERATED_HEADERS += trace/generated-helpers.h
+
 ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
 GENERATED_HEADERS += trace/generated-ust-provider.h
 GENERATED_SOURCES += trace/generated-ust.c
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index de48c80..a51e0f2 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -227,6 +227,7 @@ class Event(object):
   fmt)
 
 QEMU_TRACE   = trace_%(name)s
+QEMU_TRACE_TCG   = QEMU_TRACE + _tcg
 
 def api(self, fmt=None):
 if fmt is None:
diff --git a/scripts/tracetool/format/tcg_helper_h.py 
b/scripts/tracetool/format/tcg_helper_h.py
new file mode 100644
index 000..ba74483
--- /dev/null
+++ b/scripts/tracetool/format/tcg_helper_h.py
@@ -0,0 +1,90 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+
+Generate trace/generated-helpers.h.
+
+
+__author__ = Lluís Vilanova vilan...@ac.upc.edu
+__copyright__  = Copyright 2012-2014, Lluís Vilanova vilan...@ac.upc.edu
+__license__= GPL version 2 or (at your option) any later version
+
+__maintainer__ = Stefan Hajnoczi
+__email__  = stefa...@linux.vnet.ibm.com
+
+
+from tracetool import out
+from tracetool.transform import *
+
+
+def generate(events, backend):
+events = [e for e in events
+  if disable not in e.properties]
+
+out('/* This file is autogenerated by tracetool, do not edit. */',
+'',
+'#define tcg_temp_new_nop(v) (v)',
+'#define tcg_temp_free_nop(v)',
+'',
+)
+
+for e in events:
+if tcg-exec not in e.properties:
+continue
+
+# tracetool.generate always transforms types to host
+e_args = e.original.args
+
+# TCG helper proxy declaration
+fmt = DEF_HELPER_FLAGS_%(argc)d(%(name)s, %(flags)svoid%(types)s)
+args = e_args.transform(HOST_2_TCG_COMPAT, HOST_2_TCG,
+TCG_2_TCG_HELPER_DECL)
+types = , .join(args.types())
+if types != :
+types = ,  + types
+
+flags = TCG_CALL_NO_RWG, 
+
+out(fmt,
+flags=flags,
+argc=len(args),
+name=e.api() + _proxy,
+types=types,
+)
+
+# mixed-type to TCG helper bridge
+args_tcg_compat = e_args.transform(HOST_2_TCG_COMPAT)
+
+code_new = [
+%(tcg_type)s __%(name)s = %(tcg_func)s(%(name)s); %
+{tcg_type: transform_type(type_, HOST_2_TCG),
+ tcg_func: transform_type(type_, HOST_2_TCG_TMP_NEW),
+ name: name}
+for (type_, name) in args_tcg_compat
+]
+
+code_free = [
+%(tcg_func)s(__%(name)s); %
+{tcg_func: transform_type(type_, HOST_2_TCG_TMP_FREE),
+ name: name}
+for (type_, name) in args_tcg_compat
+]
+
+gen_name = gen_helper_ + e.api()
+
+out(
+'#ifdef HELPER_GEN_TRACE_PROXY',
+'static inline void %(name)s(%(args)s)',
+'{',
+'%(code_new)s',
+'%(proxy_name)s(%(tmp_names)s);',
+'%(code_free)s',
+'}',
+'#endif',
+name=gen_name,
+args=e_args,
+proxy_name=gen_name + _proxy,
+code_new=\n.join(code_new),
+code_free=\n

[Qemu-devel] [PATCH v4 07/10] trace: [tcg] Include TCG-tracing helpers

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 include/exec/helper-gen.h   |3 +++
 include/exec/helper-proto.h |1 +
 include/exec/helper-tcg.h   |1 +
 3 files changed, 5 insertions(+)

diff --git a/include/exec/helper-gen.h b/include/exec/helper-gen.h
index a04a034..ba5a379 100644
--- a/include/exec/helper-gen.h
+++ b/include/exec/helper-gen.h
@@ -57,6 +57,9 @@ static inline void glue(gen_helper_, 
name)(dh_retvar_decl(ret)  \
 }
 
 #include helper.h
+#define HELPER_GEN_TRACE_PROXY 1
+#include trace/generated-helpers.h
+#undef HELPER_GEN_TRACE_PROXY
 #include tcg-runtime.h
 
 #undef DEF_HELPER_FLAGS_0
diff --git a/include/exec/helper-proto.h b/include/exec/helper-proto.h
index 828951c..effdd43 100644
--- a/include/exec/helper-proto.h
+++ b/include/exec/helper-proto.h
@@ -27,6 +27,7 @@ dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), 
dh_ctype(t3), \
 dh_ctype(t4), dh_ctype(t5));
 
 #include helper.h
+#include trace/generated-helpers.h
 #include tcg-runtime.h
 
 #undef DEF_HELPER_FLAGS_0
diff --git a/include/exec/helper-tcg.h b/include/exec/helper-tcg.h
index d704c81..79fa3c8 100644
--- a/include/exec/helper-tcg.h
+++ b/include/exec/helper-tcg.h
@@ -36,6 +36,7 @@
 | dh_sizemask(t5, 5) },
 
 #include helper.h
+#include trace/generated-helpers.h
 #include tcg-runtime.h
 
 #undef DEF_HELPER_FLAGS_0

[Qemu-devel] [PATCH v4 10/10] trace: [tcg] Include TCG-tracing header on all targets

[Lluís Vilanova]

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 target-alpha/translate.c  |3 +++
 target-arm/translate-a64.c|2 ++
 target-arm/translate.c|3 +++
 target-cris/translate.c   |3 +++
 target-i386/translate.c   |3 +++
 target-lm32/translate.c   |3 +++
 target-m68k/translate.c   |3 +++
 target-microblaze/translate.c |3 +++
 target-mips/translate.c   |3 +++
 target-openrisc/translate.c   |3 +++
 target-ppc/translate.c|3 +++
 target-s390x/translate.c  |2 ++
 target-sh4/translate.c|3 +++
 target-sparc/translate.c  |3 +++
 target-unicore32/translate.c  |3 +++
 target-xtensa/translate.c |3 +++
 16 files changed, 46 insertions(+)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index e31d56c..9e2a773 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -25,6 +25,9 @@
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 #undef ALPHA_DEBUG_DISAS
 #define CONFIG_SOFTFLOAT_INLINE
 
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 9f964df..505c812 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -34,6 +34,8 @@
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
 static TCGv_i64 cpu_X[32];
 static TCGv_i64 cpu_pc;
 static TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 7f6fcd6..1a5a838 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -34,6 +34,9 @@
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 #define ENABLE_ARCH_4Tarm_feature(env, ARM_FEATURE_V4T)
 #define ENABLE_ARCH_5 arm_feature(env, ARM_FEATURE_V5)
 /* currently all emulated v5 cores are also v5TE, so don't bother */
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 90fe0a2..a4027f0 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -32,6 +32,9 @@
 
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 #define DISAS_CRIS 0
 #if DISAS_CRIS
 #  define LOG_DIS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 3aa52eb..60a2954 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -31,6 +31,9 @@
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 #define PREFIX_REPZ   0x01
 #define PREFIX_REPNZ  0x02
 #define PREFIX_LOCK   0x04
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index 51eca06..d88c44b 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -26,6 +26,9 @@
 
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 #define DISAS_LM32 1
 #if DISAS_LM32
 #  define LOG_DIS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index fa248d9..d6c39e3 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -26,6 +26,9 @@
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 //#define DEBUG_DISPATCH 1
 
 /* Fake floating point.  */
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 488df2d..373b09a 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -25,6 +25,9 @@
 #include microblaze-decode.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 #define SIM_COMPAT 0
 #define DISAS_GNU 1
 #define DISAS_MB 1
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 13cf29b..3930e7f 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -28,6 +28,9 @@
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 #define MIPS_DEBUG_DISAS 0
 //#define MIPS_DEBUG_SIGN_EXTENSIONS
 
diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
index 40084f9..2143328 100644
--- a/target-openrisc/translate.c
+++ b/target-openrisc/translate.c
@@ -30,6 +30,9 @@
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 #define OPENRISC_DISAS
 
 #ifdef OPENRISC_DISAS
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 6283b2c..e292387 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -26,6 +26,9 @@
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
+
 #define CPU_SINGLE_STEP 0x1
 #define CPU_BRANCH_STEP 0x2
 #define GDBSTUB_SINGLE_STEP 0x4
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index cf65f01..60ea3b1 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -41,6 +41,8 @@ static TCGv_ptr cpu_env;
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+#include trace-tcg.h
+
 
 /* Information that (most) every instruction needs to manipulate.  */
 typedef struct DisasContext DisasContext;
diff --git 

[Qemu-devel] [PATCH v2] vexpress: Add support for the -bios flag to provide firmware

[Peter Maydell]

From: Grant Likely grant.lik...@linaro.org

Right now to run firmware inside the QEMU VExpress model requires
padding out the firmware image to the size of the virtual flash and
passing it in via the -pflash argument. If the firmware image is passed
without padding, then QEMU will fail. Also, when passed as a -pflash
argument, QEMU treats the file as persistent storage and will modify the
file.

The -bios flag provides the semantics that we want for providing a
firmware image. This patch maps the contents of the -bios file into the
address space at the boot flash location.

Tested with the vexpress-a15 model and the Tianocore port.

Signed-off-by: Grant Likely grant.lik...@linaro.org
Tested-by: Roy Franz roy.fr...@linaro.org
[PMM: folded long line, removed stray \n from error message,
 use correct variable for printing image name, exit(1) rather than 0]
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
Grant submitted v1 of this a few months back; it was pretty nearly
correct, so I've just tidied up the loose ends so we can get it into
QEMU 2.1. Tested by booting the UEFI blob from
https://wiki.linaro.org/LEG/Engineering/Kernel/UEFI/VersatileExpress/QEMU

 hw/arm/vexpress.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 33ff422..0f8f175 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -28,6 +28,7 @@
 #include net/net.h
 #include sysemu/sysemu.h
 #include hw/boards.h
+#include hw/loader.h
 #include exec/address-spaces.h
 #include sysemu/blockdev.h
 #include hw/block/flash.h
@@ -528,6 +529,18 @@ static void vexpress_common_init(VEDBoardInfo 
*daughterboard,
 daughterboard-init(daughterboard, machine-ram_size, machine-cpu_model,
 pic);
 
+/*
+ * If a bios file was provided, attempt to map it into memory
+ */
+if (bios_name) {
+const char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+if (!fn || load_image_targphys(fn, map[VE_NORFLASH0],
+   VEXPRESS_FLASH_SIZE)  0) {
+error_report(Could not load rom image '%s', bios_name);
+exit(1);
+}
+}
+
 /* Motherboard peripherals: the wiring is the same but the
  * addresses vary between the legacy and A-Series memory maps.
  */
-- 
1.9.2

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Andreas Färber]

Am 29.05.2014 14:21, schrieb Marcel Apfelbaum:
 On Thu, 2014-05-29 at 12:47 +0200, Andreas Färber wrote:
 Am 29.05.2014 11:47, schrieb Igor Mammedov:
 ... fixes freeing constant from vl.c by machine_finalize()

 Signed-off-by: Igor Mammedov imamm...@redhat.com

 Did you check whether there are any others in need of changes? I could
 imagine kernel_irqchip does, and I see that we forgot to fix the
 underscore in the property name, damn. Marcel, can you please look into
 fixing that? I had outlined how to.
 Hi Andreas,
 
 I didn't forget to do that, I responded on the mail thread why I think we 
 shouldn't,
 even if is against QOM best practices.
 I'll copy-paste:
 
 Anyway, the real problem here is that what is elegant in this solution is:
 machine_opts = qemu_get_machine_opts();
 if (qemu_opt_foreach(machine_opts, object_set_property, current_machine, 
 1)  0) {
 object_unref(OBJECT(current_machine));
 exit(1);
 }
 It automatically fills in the machine state properties with the options from 
 the command line.
 It will work with machine sub-types that have specific properties without the 
 need
 to manually add it to vl.c. The error flow is also elegant (if a sub-type 
 does not have the
 user supplied property).
 
 If we use machine-specific wrappers to convert _ - - we loose the above.

There was nothing machine-specific in my proposal. Only the
implementation of object_set_property() would need to be extended or
copiedmodified à la X86CPU and only --based properties added in
machine_initfn().

 As an alternative we could rename the machine option to use -...

That's not been an option for command line compatibility reasons.

Regards,
Andreas

-- 
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] machine: set default kernel_cmdline in machine_initfn()

[Igor Mammedov]

On Thu, 29 May 2014 14:25:31 +0200
Andreas Färber afaer...@suse.de wrote:

 Am 29.05.2014 14:21, schrieb Marcel Apfelbaum:
  On Thu, 2014-05-29 at 12:47 +0200, Andreas Färber wrote:
  Am 29.05.2014 11:47, schrieb Igor Mammedov:
  ... fixes freeing constant from vl.c by machine_finalize()
 
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
  Did you check whether there are any others in need of changes? I could
  imagine kernel_irqchip does, and I see that we forgot to fix the
  underscore in the property name, damn. Marcel, can you please look into
  fixing that? I had outlined how to.
  Hi Andreas,
  
  I didn't forget to do that, I responded on the mail thread why I think we 
  shouldn't,
  even if is against QOM best practices.
  I'll copy-paste:
  
  Anyway, the real problem here is that what is elegant in this solution is:
  machine_opts = qemu_get_machine_opts();
  if (qemu_opt_foreach(machine_opts, object_set_property, 
  current_machine, 1)  0) {
  object_unref(OBJECT(current_machine));
  exit(1);
  }
  It automatically fills in the machine state properties with the options 
  from the command line.
  It will work with machine sub-types that have specific properties without 
  the need
  to manually add it to vl.c. The error flow is also elegant (if a sub-type 
  does not have the
  user supplied property).
  
  If we use machine-specific wrappers to convert _ - - we loose the 
  above.
 
 There was nothing machine-specific in my proposal. Only the
 implementation of object_set_property() would need to be extended or
 copiedmodified à la X86CPU and only --based properties added in
 machine_initfn().

Maybe we can hack QemuOpts to do s/foo_moo/foo-moo/ ?
Than there won't be need for an explicit conversion and it could be reused by 
others,
including X86CPU.


  As an alternative we could rename the machine option to use -...
 
 That's not been an option for command line compatibility reasons.
 
 Regards,
 Andreas
 

[Qemu-devel] [PATCH v4 06/10] trace: [tcg] Define TCG tracing helper routines

[Lluís Vilanova]

Generate file trace/generated-helpers.c with the necessary TCG helper routines
for tracing events in guest code at execution time:

* helper_trace_${event}_exec_proxy

  TCG helper implementation to cast TCG-compatible argument types to native
  types and call tracing routine 'trace_${event}_exec'.


Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 .gitignore   |1 +
 Makefile |1 +
 Makefile.objs|7 
 Makefile.target  |5 +++
 scripts/tracetool/format/tcg_helper_c.py |   50 ++
 trace/Makefile.objs  |   12 +++
 6 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 scripts/tracetool/format/tcg_helper_c.py

diff --git a/.gitignore b/.gitignore
index cc6f6c4..f82a79f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,6 +12,7 @@
 /trace/generated-events.h
 /trace/generated-events.c
 /trace/generated-helpers.h
+/trace/generated-helpers.c
 /trace/generated-ust-provider.h
 /trace/generated-ust.c
 /libcacard/trace/generated-tracers.c
diff --git a/Makefile b/Makefile
index e3ac3fd..8544290 100644
--- a/Makefile
+++ b/Makefile
@@ -58,6 +58,7 @@ endif
 GENERATED_SOURCES += trace/generated-tracers.c
 
 GENERATED_HEADERS += trace/generated-helpers.h
+GENERATED_SOURCES += trace/generated-helpers.c
 
 ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
 GENERATED_HEADERS += trace/generated-ust-provider.h
diff --git a/Makefile.objs b/Makefile.objs
index b897e1d..4e0dea7 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,7 +1,7 @@
 ###
 # Common libraries for tools and emulators
 stub-obj-y = stubs/
-util-obj-y = util/ qobject/ qapi/ trace/
+util-obj-y = util/ qobject/ qapi/
 
 ###
 # block-obj-y is code used by both qemu system emulation and qemu-img
@@ -107,6 +107,11 @@ version-obj-$(CONFIG_WIN32) += $(BUILD_DIR)/version.o
 version-lobj-$(CONFIG_WIN32) += $(BUILD_DIR)/version.lo
 
 ##
+# tracing
+util-obj-y +=  trace/
+target-obj-y += trace/
+
+##
 # guest agent
 
 # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed
diff --git a/Makefile.target b/Makefile.target
index 8155496..edafa35 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -145,15 +145,20 @@ endif # CONFIG_SOFTMMU
 dummy := $(call unnest-vars,,obj-y)
 all-obj-y := $(obj-y)
 
+target-obj-y :=
 block-obj-y :=
 common-obj-y :=
 include $(SRC_PATH)/Makefile.objs
+dummy := $(call unnest-vars,,target-obj-y)
+target-obj-y-save := $(target-obj-y)
 dummy := $(call unnest-vars,.., \
block-obj-y \
block-obj-m \
common-obj-y \
common-obj-m)
+target-obj-y := $(target-obj-y-save)
 all-obj-y += $(common-obj-y)
+all-obj-y += $(target-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
 
 ifndef CONFIG_HAIKU
diff --git a/scripts/tracetool/format/tcg_helper_c.py 
b/scripts/tracetool/format/tcg_helper_c.py
new file mode 100644
index 000..96655a0
--- /dev/null
+++ b/scripts/tracetool/format/tcg_helper_c.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+
+Generate trace/generated-helpers.c.
+
+
+__author__ = Lluís Vilanova vilan...@ac.upc.edu
+__copyright__  = Copyright 2012-2014, Lluís Vilanova vilan...@ac.upc.edu
+__license__= GPL version 2 or (at your option) any later version
+
+__maintainer__ = Stefan Hajnoczi
+__email__  = stefa...@linux.vnet.ibm.com
+
+
+from tracetool import out
+from tracetool.transform import *
+
+
+def generate(events, backend):
+events = [e for e in events
+  if disable not in e.properties]
+
+out('/* This file is autogenerated by tracetool, do not edit. */',
+'',
+'#include qemu-common.h',
+'#include trace.h',
+'#include exec/helper-proto.h',
+'',
+)
+
+for e in events:
+if tcg-exec not in e.properties:
+continue
+
+# tracetool.generate always transforms types to host
+e_args = e.original.args
+
+values = [(%s)%s % (t, n)
+  for t, n in e.args.transform(TCG_2_TCG_HELPER_DEF)]
+
+out('void %(name_tcg)s(%(args)s)',
+'{',
+'%(name)s(%(values)s);',
+'}',
+name_tcg=helper_%s_proxy % e.api(),
+name=e.api(),
+args=e_args.transform(HOST_2_TCG_COMPAT, TCG_2_TCG_HELPER_DEF),
+values=, .join(values),
+)
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 0a2e6cd..2d36142 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -107,6 +107,18 @@ $(obj)/generated-helpers.h-timestamp: 
$(SRC_PATH)/trace-events $(BUILD_DIR)/conf
  

Re: [Qemu-devel] [PATCH] trace: Replace fprintf with error_report and print location

[Lluís Vilanova]

Alexey Kardashevskiy writes:

 This replaces fprintf(stderr) with error_report.
 This prints line number of the trace which does not exist or is not
 traceable.

A little nit pick; it shows an error when some of the events in the list of
events to enable (not the trace) does not exist or is not traceable.


Thanks,
  Lluis


 This moves local variables to the beginning of the function because of
 the QEMU coding style.

 Suggested-by: Lluís Vilanova vilan...@ac.upc.edu
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---

 Lluís, or it is s/Suggested-by/From/ ?

 Stefan, this is made on top of trace: Replace error with warning if event is 
 not defined

 ---
  trace/control.c | 31 ++-
  1 file changed, 18 insertions(+), 13 deletions(-)

 diff --git a/trace/control.c b/trace/control.c
 index 4aa02cf..4ee2bd2 100644
 --- a/trace/control.c
 +++ b/trace/control.c
 @@ -8,7 +8,7 @@
   */
 
  #include trace/control.h
 -
 +#include qemu/error-report.h
 
  TraceEvent *trace_event_name(const char *name)
  {
 @@ -81,18 +81,24 @@ TraceEvent *trace_event_pattern(const char *pat, 
 TraceEvent *ev)
 
  void trace_backend_init_events(const char *fname)
  {
 +Location loc;
 +FILE *fp;
 +char line_buf[1024];
 +size_t line_idx = 0;
 +
  if (fname == NULL) {
  return;
  }
 
 -FILE *fp = fopen(fname, r);
 +loc_push_none(loc);
 +loc_set_file(fname, 0);
 +fp = fopen(fname, r);
  if (!fp) {
 -fprintf(stderr, error: could not open trace events file '%s': %s\n,
 -fname, strerror(errno));
 +error_report(%s, strerror(errno));
  exit(1);
  }
 -char line_buf[1024];
  while (fgets(line_buf, sizeof(line_buf), fp)) {
 +loc_set_file(fname, ++line_idx);
  size_t len = strlen(line_buf);
  if (len  1) {  /* skip empty lines */
  line_buf[len - 1] = '\0';
 @@ -111,13 +117,11 @@ void trace_backend_init_events(const char *fname)
  } else {
  TraceEvent *ev = trace_event_name(line_ptr);
  if (ev == NULL) {
 -fprintf(stderr,
 -WARNING: trace event '%s' does not exist\n,
 -line_ptr);
 +error_report(WARNING: trace event '%s' does not exist,
 + line_ptr);
  } else if (!trace_event_get_state_static(ev)) {
 -fprintf(stderr,
 -WARNING: trace event '%s' is not traceable\n,
 -line_ptr);
 +error_report(WARNING: trace event '%s' is not 
 traceable\n,
 + line_ptr);
  } else {
  trace_event_set_state_dynamic(ev, enable);
  }
 @@ -125,8 +129,9 @@ void trace_backend_init_events(const char *fname)
  }
  }
  if (fclose(fp) != 0) {
 -fprintf(stderr, error: closing file '%s': %s\n,
 -fname, strerror(errno));
 +loc_set_file(fname, 0);
 +error_report(%s, strerror(errno));
  exit(1);
  }
 +loc_pop(loc);
  }
 -- 
 1.8.4.rc4


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

Re: [Qemu-devel] [PATCH v2] vexpress: Add support for the -bios flag to provide firmware

[Peter Crosthwaite]

On Thu, May 29, 2014 at 10:24 PM, Peter Maydell
peter.mayd...@linaro.org wrote:
 From: Grant Likely grant.lik...@linaro.org

 Right now to run firmware inside the QEMU VExpress model requires
 padding out the firmware image to the size of the virtual flash and
 passing it in via the -pflash argument. If the firmware image is passed
 without padding, then QEMU will fail. Also, when passed as a -pflash
 argument, QEMU treats the file as persistent storage and will modify the
 file.


A little out of scope, but no support for read-only backing images is
somewhat annoying. Is it feasible to patch the block layer (or perhaps
we need to do something to pflash?) to handle a read-only -pflash file
as init-only data (and then use the normal volatile ram-based storage
once the file is loaded)?

Then the semantics of don't change my ROM file even if the real
hardware can do it in real life can just be handled with file perms.

 The -bios flag provides the semantics that we want for providing a
 firmware image. This patch maps the contents of the -bios file into the
 address space at the boot flash location.

 Tested with the vexpress-a15 model and the Tianocore port.

 Signed-off-by: Grant Likely grant.lik...@linaro.org
 Tested-by: Roy Franz roy.fr...@linaro.org
 [PMM: folded long line, removed stray \n from error message,
  use correct variable for printing image name, exit(1) rather than 0]
 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
 Grant submitted v1 of this a few months back; it was pretty nearly
 correct, so I've just tidied up the loose ends so we can get it into
 QEMU 2.1. Tested by booting the UEFI blob from
 https://wiki.linaro.org/LEG/Engineering/Kernel/UEFI/VersatileExpress/QEMU

  hw/arm/vexpress.c | 13 +
  1 file changed, 13 insertions(+)

 diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
 index 33ff422..0f8f175 100644
 --- a/hw/arm/vexpress.c
 +++ b/hw/arm/vexpress.c
 @@ -28,6 +28,7 @@
  #include net/net.h
  #include sysemu/sysemu.h
  #include hw/boards.h
 +#include hw/loader.h
  #include exec/address-spaces.h
  #include sysemu/blockdev.h
  #include hw/block/flash.h
 @@ -528,6 +529,18 @@ static void vexpress_common_init(VEDBoardInfo 
 *daughterboard,
  daughterboard-init(daughterboard, machine-ram_size, machine-cpu_model,
  pic);

 +/*
 + * If a bios file was provided, attempt to map it into memory
 + */
 +if (bios_name) {
 +const char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 +if (!fn || load_image_targphys(fn, map[VE_NORFLASH0],
 +   VEXPRESS_FLASH_SIZE)  0) {
 +error_report(Could not load rom image '%s', bios_name);

ROM

Otherwise

Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com

Regards,
Peter

 +exit(1);
 +}
 +}
 +
  /* Motherboard peripherals: the wiring is the same but the
   * addresses vary between the legacy and A-Series memory maps.
   */
 --
 1.9.2

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Aurelien Jarno]

On Thu, May 29, 2014 at 02:58:20AM +0200, Petar Jovanovic wrote:
 From: Petar Jovanovic petar.jovano...@imgtec.com
 
 From MIPS documentation (Volume III):
 
 UserLocal Register (CP0 Register 4, Select 2)
 Compliance Level: Recommended.
 
 The UserLocal register is a read-write register that is not interpreted by
 the hardware and conditionally readable via the RDHWR instruction.
 
 This register only exists if the Config3-ULRI register field is set.
 
 Privileged software may write this register with arbitrary information and
 make it accessible to unprivileged software via register 29 (ULR) of the
 RDHWR instruction. To do so, bit 29 of the HWREna register must be set to a
 1 to enable unprivileged access to the register.
 
 Signed-off-by: Petar Jovanovic petar.jovano...@imgtec.com
 ---
 v3:
 - new hflag MIPS_HFLAG_HWRENA_ULR introduced, it is set when ULR bit
   from HWREna is set
 - helper rdhwr_ul removed, now the checks for rdhwr are done at
   translation time
 - CPU_SAVE_VERSION switched to 4, load_tc supports both (3 and 4)
   version ids
 
 v2:
 - Defined MIPS_HFLAG_CP0UL flag, checks are now based on hflags
 - CP0_UserLocal moved to struct TCState
 - Added tc-CP0_UserLocal in save_tc/load_tc in target-mips/machine.c
 - Reused CP0_UserLocal field for user-mode purpose
 
  linux-user/mips/target_cpu.h |2 +-
  linux-user/syscall.c |2 +-
  target-mips/cpu.h|   13 +++---
  target-mips/machine.c|   13 +++---
  target-mips/op_helper.c  |   14 ++-
  target-mips/translate.c  |   55 
 +++---
  6 files changed, 85 insertions(+), 14 deletions(-)
 
 diff --git a/linux-user/mips/target_cpu.h b/linux-user/mips/target_cpu.h
 index ba8e9eb..19b8855 100644
 --- a/linux-user/mips/target_cpu.h
 +++ b/linux-user/mips/target_cpu.h
 @@ -30,7 +30,7 @@ static inline void cpu_clone_regs(CPUMIPSState *env, 
 target_ulong newsp)
  
  static inline void cpu_set_tls(CPUMIPSState *env, target_ulong newtls)
  {
 -env-tls_value = newtls;
 +env-active_tc.CP0_UserLocal = newtls;
  }
  
  #endif
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 6efeeff..fda8dd6 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -8686,7 +8686,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
 arg1,
  #ifdef TARGET_NR_set_thread_area
  case TARGET_NR_set_thread_area:
  #if defined(TARGET_MIPS)
 -  ((CPUMIPSState *) cpu_env)-tls_value = arg1;
 +  ((CPUMIPSState *) cpu_env)-active_tc.CP0_UserLocal = arg1;
ret = 0;
break;
  #elif defined(TARGET_CRIS)
 diff --git a/target-mips/cpu.h b/target-mips/cpu.h
 index 6c2014e..67bf441 100644
 --- a/target-mips/cpu.h
 +++ b/target-mips/cpu.h
 @@ -167,6 +167,7 @@ struct TCState {
  target_ulong CP0_TCSchedule;
  target_ulong CP0_TCScheFBack;
  int32_t CP0_Debug_tcstatus;
 +target_ulong CP0_UserLocal;
  };
  
  typedef struct CPUMIPSState CPUMIPSState;
 @@ -361,6 +362,7 @@ struct CPUMIPSState {
  int32_t CP0_Config3;
  #define CP0C3_M31
  #define CP0C3_ISA_ON_EXC 16
 +#define CP0C3_ULRI 13
  #define CP0C3_DSPP 10
  #define CP0C3_LPA  7
  #define CP0C3_VEIC 6
 @@ -469,6 +471,10 @@ struct CPUMIPSState {
  /* MIPS DSP resources access. */
  #define MIPS_HFLAG_DSP   0x4  /* Enable access to MIPS DSP resources. */
  #define MIPS_HFLAG_DSPR2 0x8  /* Enable access to MIPS DSPR2 resources. 
 */
 +/* Extra flags about UserLocal register. */
 +#define MIPS_HFLAG_CP0UL  0x10 /* CP0_UserLocal is implemented. */
 +#define MIPS_HFLAG_HWRENA_ULR 0x20 /* ULR bit from HWREna is set. */
 +#define MIPS_HFLAG_UL_MASK  (MIPS_HFLAG_CP0UL | MIPS_HFLAG_HWRENA_ULR)

While adding MIPS_HFLAG_HWRENA_ULR is a good idea for performance
reasons, I don't think we should add MIPS_HFLAG_CP0UL. This value is
always the same (true or false) during all the run time of the
qemu-system-mips binary, and thus we don't need to take care of code
generated with it being true or being false.


  target_ulong btarget;/* Jump / branch target   */
  target_ulong bcond;  /* Branch condition (if needed)   */
  
 @@ -478,8 +484,6 @@ struct CPUMIPSState {
  uint32_t CP0_TCStatus_rw_bitmask; /* Read/write bits in CP0_TCStatus */
  int insn_flags; /* Supported instruction set */
  
 -target_ulong tls_value; /* For usermode emulation */
 -
  CPU_COMMON
  
  /* Fields from here on are preserved across CPU reset. */
 @@ -522,7 +526,7 @@ void mips_cpu_list (FILE *f, fprintf_function 
 cpu_fprintf);
  extern void cpu_wrdsp(uint32_t rs, uint32_t mask_num, CPUMIPSState *env);
  extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
  
 -#define CPU_SAVE_VERSION 3
 +#define CPU_SAVE_VERSION 4
  
  /* MMU modes definitions. We carefully match the indices with our
 hflags layout. */
 @@ -681,7 +685,8 @@ static inline void cpu_get_tb_cpu_state(CPUMIPSState 
 *env, target_ulong *pc,
  {

[Qemu-devel] [PATCH v4 09/10] trace: [tcg] Include event definitions in trace.h

[Lluís Vilanova]

Otherwise the user has to explicitly include an auto-generated header.

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 include/trace.h |1 +
 1 file changed, 1 insertion(+)

diff --git a/include/trace.h b/include/trace.h
index c15f498..44a1f1f 100644
--- a/include/trace.h
+++ b/include/trace.h
@@ -2,5 +2,6 @@
 #define TRACE_H
 
 #include trace/generated-tracers.h
+#include trace/generated-events.h
 
 #endif  /* TRACE_H */

Re: [Qemu-devel] [PATCH] slirp: Remove unused zero_ethaddr[] variable

[Stefan Weil]

Am 29.05.2014 13:00, schrieb Peter Maydell:
 The zero_ethaddr[] array is never used; delete it.
 
 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
 clang 3.4 warns about unused const variables like this and
 also about unused functions; we have over 400 such warnings
 currently, of which some are simple forgot to clean up
 redundant code like this one, some are deliberate in some
 sense (usually the function/var is used in one config but not
 another), and some are outright bugs. Anybody feel like wading
 through them? :-)

I can add some more:

* Missing 'static' attributes for local variables and functions

* Use of 0 instead of NULL for pointers (do we want to fix those?)

 
  slirp/slirp.c | 2 --
  1 file changed, 2 deletions(-)
 
 diff --git a/slirp/slirp.c b/slirp/slirp.c
 index 3fb48a4..b7f3726 100644
 --- a/slirp/slirp.c
 +++ b/slirp/slirp.c
 @@ -37,8 +37,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
  0x52, 0x55, 0x00, 0x00, 0x00, 0x00
  };
  
 -static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
 -
  u_int curtime;
  
  static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
 

Reviewed-by: Stefan Weil s...@weilnetz.de

Re: [Qemu-devel] [PATCH] target-arm/cpu64.c: Actually register Cortex-A57 impdef registers

[Peter Crosthwaite]

On Wed, May 28, 2014 at 7:30 AM, Peter Maydell peter.mayd...@linaro.org wrote:
 cpu64.c contains a reginfo list for the impdef registers on
 the Cortex-A57; however we forgot to actually call define_arm_cp_regs(),
 so it was sitting there doing nothing. Remedy this omission.

 Signed-off-by: Peter Maydell peter.mayd...@linaro.org

I have this locally in my tree. You beat me to the post. So I guess:

Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Tested-by: Peter Crosthwaite peter.crosthwa...@xilinx.com

Regards,
Peter

 ---
  target-arm/cpu64.c | 1 +
  1 file changed, 1 insertion(+)

 Oops. clang 3.4 warns about the unused variable (along with
 a lot of similar issues in other parts of the codebase).
 Linux doesn't actually look at any of the A57 impdef regs,
 or we'd have noticed this earlier.

 diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
 index 8daa622..ff4c2b4 100644
 --- a/target-arm/cpu64.c
 +++ b/target-arm/cpu64.c
 @@ -128,6 +128,7 @@ static void aarch64_a57_initfn(Object *obj)
  cpu-ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
  cpu-ccsidr[2] = 0x70ffe07a; /* 2048KB L2 cache */
  cpu-dcz_blocksize = 4; /* 64 bytes */
 +define_arm_cp_regs(cpu, cortexa57_cp_reginfo);
  }

  #ifdef CONFIG_USER_ONLY
 --
 1.9.2

[Qemu-devel] [PATCH v4 08/10] trace: [tcg] Generate TCG tracing routines

[Lluís Vilanova]

Generate header trace/generated-tcg-tracers.h with the necessary routines for
tracing events in guest code:

* trace_${event}_tcg

  Convenience wrapper that calls the translation-time tracer
  'trace_${event}_trans', and calls 'gen_helper_trace_${event}_exec to
  generate the TCG code to later trace the event at execution time.

Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
---
 .gitignore|1 +
 Makefile  |2 +
 include/trace-tcg.h   |7 +
 scripts/tracetool/format/tcg_h.py |   57 +
 trace/Makefile.objs   |9 ++
 5 files changed, 76 insertions(+)
 create mode 100644 include/trace-tcg.h
 create mode 100644 scripts/tracetool/format/tcg_h.py

diff --git a/.gitignore b/.gitignore
index f82a79f..f3f07bd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,6 +13,7 @@
 /trace/generated-events.c
 /trace/generated-helpers.h
 /trace/generated-helpers.c
+/trace/generated-tcg-tracers.h
 /trace/generated-ust-provider.h
 /trace/generated-ust.c
 /libcacard/trace/generated-tracers.c
diff --git a/Makefile b/Makefile
index 8544290..e89aa06 100644
--- a/Makefile
+++ b/Makefile
@@ -57,6 +57,8 @@ GENERATED_HEADERS += trace/generated-tracers-dtrace.h
 endif
 GENERATED_SOURCES += trace/generated-tracers.c
 
+GENERATED_HEADERS += trace/generated-tcg-tracers.h
+
 GENERATED_HEADERS += trace/generated-helpers.h
 GENERATED_SOURCES += trace/generated-helpers.c
 
diff --git a/include/trace-tcg.h b/include/trace-tcg.h
new file mode 100644
index 000..6f6bdbb
--- /dev/null
+++ b/include/trace-tcg.h
@@ -0,0 +1,7 @@
+#ifndef TRACE_TCG_H
+#define TRACE_TCG_H
+
+#include trace/generated-tcg-tracers.h
+#include trace/generated-events.h
+
+#endif  /* TRACE_TCG_H */
diff --git a/scripts/tracetool/format/tcg_h.py 
b/scripts/tracetool/format/tcg_h.py
new file mode 100644
index 000..f676b66
--- /dev/null
+++ b/scripts/tracetool/format/tcg_h.py
@@ -0,0 +1,57 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+
+Generate .h file for TCG code generation.
+
+
+__author__ = Lluís Vilanova vilan...@ac.upc.edu
+__copyright__  = Copyright 2012-2014, Lluís Vilanova vilan...@ac.upc.edu
+__license__= GPL version 2 or (at your option) any later version
+
+__maintainer__ = Stefan Hajnoczi
+__email__  = stefa...@linux.vnet.ibm.com
+
+
+from tracetool import out
+
+
+def generate(events, backend):
+out('/* This file is autogenerated by tracetool, do not edit. */',
+'/* You must include this file after the inclusion of helper.h */',
+'',
+'#ifndef TRACE__GENERATED_TCG_TRACERS_H',
+'#define TRACE__GENERATED_TCG_TRACERS_H',
+'',
+'#include stdint.h',
+'',
+'#include trace.h',
+'#include exec/helper-proto.h',
+'',
+)
+
+for e in events:
+# just keep one of them
+if tcg-trans not in e.properties:
+continue
+
+# get the original event definition
+e = e.original.original
+
+out('static inline void %(name_tcg)s(%(args)s)',
+'{',
+name_tcg=e.api(e.QEMU_TRACE_TCG),
+args=e.args)
+
+if disable not in e.properties:
+out('%(name_trans)s(%(argnames_trans)s);',
+'gen_helper_%(name_exec)s(%(argnames_exec)s);',
+name_trans=e.event_trans.api(e.QEMU_TRACE),
+name_exec=e.event_exec.api(e.QEMU_TRACE),
+argnames_trans=, .join(e.event_trans.args.names()),
+argnames_exec=, .join(e.event_exec.args.names()))
+
+out('}')
+
+out('',
+'#endif /* TRACE__GENERATED_TCG_TRACERS_H */')
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 2d36142..889764f 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -120,6 +120,15 @@ $(obj)/generated-helpers.o: $(obj)/generated-helpers.c
 target-obj-y += generated-helpers.o
 
 
+$(obj)/generated-tcg-tracers.h: $(obj)/generated-tcg-tracers.h-timestamp
+$(obj)/generated-tcg-tracers.h-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak
+   $(call quiet-command,$(TRACETOOL) \
+   --format=tcg-h \
+   --backend=$(TRACE_BACKENDS) \
+$  $@,  GEN   $(patsubst %-timestamp,%,$@))
+   @cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst 
%-timestamp,%,$@)
+
+
 ##
 # Backend code
 

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Marcel Apfelbaum]

On Thu, 2014-05-29 at 14:40 +0200, Igor Mammedov wrote:
 On Thu, 29 May 2014 14:25:31 +0200
 Andreas Färber afaer...@suse.de wrote:
 
  Am 29.05.2014 14:21, schrieb Marcel Apfelbaum:
   On Thu, 2014-05-29 at 12:47 +0200, Andreas Färber wrote:
   Am 29.05.2014 11:47, schrieb Igor Mammedov:
   ... fixes freeing constant from vl.c by machine_finalize()
  
   Signed-off-by: Igor Mammedov imamm...@redhat.com
  
   Did you check whether there are any others in need of changes? I could
   imagine kernel_irqchip does, and I see that we forgot to fix the
   underscore in the property name, damn. Marcel, can you please look into
   fixing that? I had outlined how to.
   Hi Andreas,
   
   I didn't forget to do that, I responded on the mail thread why I think we 
   shouldn't,
   even if is against QOM best practices.
   I'll copy-paste:
   
   Anyway, the real problem here is that what is elegant in this solution is:
   machine_opts = qemu_get_machine_opts();
   if (qemu_opt_foreach(machine_opts, object_set_property, 
   current_machine, 1)  0) {
   object_unref(OBJECT(current_machine));
   exit(1);
   }
   It automatically fills in the machine state properties with the options 
   from the command line.
   It will work with machine sub-types that have specific properties without 
   the need
   to manually add it to vl.c. The error flow is also elegant (if a sub-type 
   does not have the
   user supplied property).
   
   If we use machine-specific wrappers to convert _ - - we loose the 
   above.
  
  There was nothing machine-specific in my proposal. Only the
  implementation of object_set_property() would need to be extended or
  copiedmodified à la X86CPU and only --based properties added in
  machine_initfn().
 
 Maybe we can hack QemuOpts to do s/foo_moo/foo-moo/ ?
 Than there won't be need for an explicit conversion and it could be reused by 
 others,
 including X86CPU.
I like this idea, but this can't be generic for all QemuOpts right? Do you
mean adding a flag specifying we want this conversion?

Thanks,
Marcel 
 
 
   As an alternative we could rename the machine option to use -...
  
  That's not been an option for command line compatibility reasons.
  
  Regards,
  Andreas
  
 

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Petar Jovanovic]

 While adding MIPS_HFLAG_HWRENA_ULR is a good idea for performance
 reasons, I don't think we should add MIPS_HFLAG_CP0UL. This value is
 always the same (true or false) during all the run time of the
 qemu-system-mips binary, and thus we don't need to take care of code
 generated with it being true or being false.

Initial version of this patch did not have this flag, but it was added
as requested in code review.
What do you suggest?


From: Aurelien Jarno [aurel...@aurel32.net]
Sent: Thursday, May 29, 2014 2:49 PM
To: Petar Jovanovic
Cc: qemu-devel@nongnu.org; Petar Jovanovic; afaer...@suse.de; r...@twiddle.net
Subject: Re: [v3 PATCH] target-mips: implement UserLocal Register

On Thu, May 29, 2014 at 02:58:20AM +0200, Petar Jovanovic wrote:
 From: Petar Jovanovic petar.jovano...@imgtec.com

 From MIPS documentation (Volume III):

 UserLocal Register (CP0 Register 4, Select 2)
 Compliance Level: Recommended.

 The UserLocal register is a read-write register that is not interpreted by
 the hardware and conditionally readable via the RDHWR instruction.

 This register only exists if the Config3-ULRI register field is set.

 Privileged software may write this register with arbitrary information and
 make it accessible to unprivileged software via register 29 (ULR) of the
 RDHWR instruction. To do so, bit 29 of the HWREna register must be set to a
 1 to enable unprivileged access to the register.

 Signed-off-by: Petar Jovanovic petar.jovano...@imgtec.com
 ---
 v3:
 - new hflag MIPS_HFLAG_HWRENA_ULR introduced, it is set when ULR bit
   from HWREna is set
 - helper rdhwr_ul removed, now the checks for rdhwr are done at
   translation time
 - CPU_SAVE_VERSION switched to 4, load_tc supports both (3 and 4)
   version ids

 v2:
 - Defined MIPS_HFLAG_CP0UL flag, checks are now based on hflags
 - CP0_UserLocal moved to struct TCState
 - Added tc-CP0_UserLocal in save_tc/load_tc in target-mips/machine.c
 - Reused CP0_UserLocal field for user-mode purpose

  linux-user/mips/target_cpu.h |2 +-
  linux-user/syscall.c |2 +-
  target-mips/cpu.h|   13 +++---
  target-mips/machine.c|   13 +++---
  target-mips/op_helper.c  |   14 ++-
  target-mips/translate.c  |   55 
 +++---
  6 files changed, 85 insertions(+), 14 deletions(-)

 diff --git a/linux-user/mips/target_cpu.h b/linux-user/mips/target_cpu.h
 index ba8e9eb..19b8855 100644
 --- a/linux-user/mips/target_cpu.h
 +++ b/linux-user/mips/target_cpu.h
 @@ -30,7 +30,7 @@ static inline void cpu_clone_regs(CPUMIPSState *env, 
 target_ulong newsp)

  static inline void cpu_set_tls(CPUMIPSState *env, target_ulong newtls)
  {
 -env-tls_value = newtls;
 +env-active_tc.CP0_UserLocal = newtls;
  }

  #endif
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 6efeeff..fda8dd6 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -8686,7 +8686,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
 arg1,
  #ifdef TARGET_NR_set_thread_area
  case TARGET_NR_set_thread_area:
  #if defined(TARGET_MIPS)
 -  ((CPUMIPSState *) cpu_env)-tls_value = arg1;
 +  ((CPUMIPSState *) cpu_env)-active_tc.CP0_UserLocal = arg1;
ret = 0;
break;
  #elif defined(TARGET_CRIS)
 diff --git a/target-mips/cpu.h b/target-mips/cpu.h
 index 6c2014e..67bf441 100644
 --- a/target-mips/cpu.h
 +++ b/target-mips/cpu.h
 @@ -167,6 +167,7 @@ struct TCState {
  target_ulong CP0_TCSchedule;
  target_ulong CP0_TCScheFBack;
  int32_t CP0_Debug_tcstatus;
 +target_ulong CP0_UserLocal;
  };

  typedef struct CPUMIPSState CPUMIPSState;
 @@ -361,6 +362,7 @@ struct CPUMIPSState {
  int32_t CP0_Config3;
  #define CP0C3_M31
  #define CP0C3_ISA_ON_EXC 16
 +#define CP0C3_ULRI 13
  #define CP0C3_DSPP 10
  #define CP0C3_LPA  7
  #define CP0C3_VEIC 6
 @@ -469,6 +471,10 @@ struct CPUMIPSState {
  /* MIPS DSP resources access. */
  #define MIPS_HFLAG_DSP   0x4  /* Enable access to MIPS DSP resources. */
  #define MIPS_HFLAG_DSPR2 0x8  /* Enable access to MIPS DSPR2 resources. 
 */
 +/* Extra flags about UserLocal register. */
 +#define MIPS_HFLAG_CP0UL  0x10 /* CP0_UserLocal is implemented. */
 +#define MIPS_HFLAG_HWRENA_ULR 0x20 /* ULR bit from HWREna is set. */
 +#define MIPS_HFLAG_UL_MASK  (MIPS_HFLAG_CP0UL | MIPS_HFLAG_HWRENA_ULR)

While adding MIPS_HFLAG_HWRENA_ULR is a good idea for performance
reasons, I don't think we should add MIPS_HFLAG_CP0UL. This value is
always the same (true or false) during all the run time of the
qemu-system-mips binary, and thus we don't need to take care of code
generated with it being true or being false.


  target_ulong btarget;/* Jump / branch target   */
  target_ulong bcond;  /* Branch condition (if needed)   */

 @@ -478,8 +484,6 @@ struct CPUMIPSState {
  uint32_t CP0_TCStatus_rw_bitmask; /* Read/write bits 

Re: [Qemu-devel] [PATCH] slirp: Remove unused zero_ethaddr[] variable

[Peter Maydell]

On 29 May 2014 13:52, Stefan Weil s...@weilnetz.de wrote:
 I can add some more:

 * Missing 'static' attributes for local variables and functions

Those seem worth fixing.

 * Use of 0 instead of NULL for pointers (do we want to fix those?)

I wouldn't bother personally.

There's also still a tail of coverity issues.

thanks
-- PMM

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Aurelien Jarno]

On Thu, May 29, 2014 at 01:01:38PM +, Petar Jovanovic wrote:
  While adding MIPS_HFLAG_HWRENA_ULR is a good idea for performance
  reasons, I don't think we should add MIPS_HFLAG_CP0UL. This value is
  always the same (true or false) during all the run time of the
  qemu-system-mips binary, and thus we don't need to take care of code
  generated with it being true or being false.
 
 Initial version of this patch did not have this flag, but it was added
 as requested in code review.
 What do you suggest?

Well maybe I missed something, but it looks to me the value can never
change as CP0_Config3 is a read-only register. If I am right, probably
the best is to check directly env-CP0_Config3.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Igor Mammedov]

On Thu, 29 May 2014 15:56:16 +0300
Marcel Apfelbaum marce...@redhat.com wrote:

 On Thu, 2014-05-29 at 14:40 +0200, Igor Mammedov wrote:
  On Thu, 29 May 2014 14:25:31 +0200
  Andreas Färber afaer...@suse.de wrote:
  
   Am 29.05.2014 14:21, schrieb Marcel Apfelbaum:
On Thu, 2014-05-29 at 12:47 +0200, Andreas Färber wrote:
Am 29.05.2014 11:47, schrieb Igor Mammedov:
... fixes freeing constant from vl.c by machine_finalize()
   
Signed-off-by: Igor Mammedov imamm...@redhat.com
   
Did you check whether there are any others in need of changes? I could
imagine kernel_irqchip does, and I see that we forgot to fix the
underscore in the property name, damn. Marcel, can you please look into
fixing that? I had outlined how to.
Hi Andreas,

I didn't forget to do that, I responded on the mail thread why I think 
we shouldn't,
even if is against QOM best practices.
I'll copy-paste:

Anyway, the real problem here is that what is elegant in this solution 
is:
machine_opts = qemu_get_machine_opts();
if (qemu_opt_foreach(machine_opts, object_set_property, 
current_machine, 1)  0) {
object_unref(OBJECT(current_machine));
exit(1);
}
It automatically fills in the machine state properties with the options 
from the command line.
It will work with machine sub-types that have specific properties 
without the need
to manually add it to vl.c. The error flow is also elegant (if a 
sub-type does not have the
user supplied property).

If we use machine-specific wrappers to convert _ - - we loose the 
above.
   
   There was nothing machine-specific in my proposal. Only the
   implementation of object_set_property() would need to be extended or
   copiedmodified à la X86CPU and only --based properties added in
   machine_initfn().
  
  Maybe we can hack QemuOpts to do s/foo_moo/foo-moo/ ?
  Than there won't be need for an explicit conversion and it could be reused 
  by others,
  including X86CPU.
 I like this idea, but this can't be generic for all QemuOpts right? Do you
 mean adding a flag specifying we want this conversion?
Flag would be less risky than doing it for all QemuOpts unconditionally.
It would open road for conversion per a QemuOpts which probably should be
done eventually if options are translated into properties.

 
 Thanks,
 Marcel 
  
  
As an alternative we could rename the machine option to use -...
   
   That's not been an option for command line compatibility reasons.
   
   Regards,
   Andreas
   
  
 
 
 

Re: [Qemu-devel] [PATCH v2 1/1] bsd-user: refresh freebsd system call numbers

[Peter Maydell]

On 28 May 2014 19:00, Sean Bruno sbr...@freebsd.org wrote:
 From: Stacey Son s...@freebsd.org

 Update FreeBSD system call numbers in freebsd/syscall_nr.h.

 Signed-off bY: Stacey Son s...@freebsd.org

(stray uppercase Y)

 Signed-off-by: Sean Bruno sbr...@freebsd.org

 Reviewed-by: Ed Maste ema...@freebsd.org

Reviewed-by: Peter Maydell peter.mayd...@linaro.org

thanks
-- PMM

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Marcel Apfelbaum]

On Thu, 2014-05-29 at 15:08 +0200, Igor Mammedov wrote:
 On Thu, 29 May 2014 15:56:16 +0300
 Marcel Apfelbaum marce...@redhat.com wrote:
 
  On Thu, 2014-05-29 at 14:40 +0200, Igor Mammedov wrote:
   On Thu, 29 May 2014 14:25:31 +0200
   Andreas Färber afaer...@suse.de wrote:
   
Am 29.05.2014 14:21, schrieb Marcel Apfelbaum:
 On Thu, 2014-05-29 at 12:47 +0200, Andreas Färber wrote:
 Am 29.05.2014 11:47, schrieb Igor Mammedov:
 ... fixes freeing constant from vl.c by machine_finalize()

 Signed-off-by: Igor Mammedov imamm...@redhat.com

 Did you check whether there are any others in need of changes? I 
 could
 imagine kernel_irqchip does, and I see that we forgot to fix the
 underscore in the property name, damn. Marcel, can you please look 
 into
 fixing that? I had outlined how to.
 Hi Andreas,
 
 I didn't forget to do that, I responded on the mail thread why I 
 think we shouldn't,
 even if is against QOM best practices.
 I'll copy-paste:
 
 Anyway, the real problem here is that what is elegant in this 
 solution is:
 machine_opts = qemu_get_machine_opts();
 if (qemu_opt_foreach(machine_opts, object_set_property, 
 current_machine, 1)  0) {
 object_unref(OBJECT(current_machine));
 exit(1);
 }
 It automatically fills in the machine state properties with the 
 options from the command line.
 It will work with machine sub-types that have specific properties 
 without the need
 to manually add it to vl.c. The error flow is also elegant (if a 
 sub-type does not have the
 user supplied property).
 
 If we use machine-specific wrappers to convert _ - - we loose 
 the above.

There was nothing machine-specific in my proposal. Only the
implementation of object_set_property() would need to be extended or
copiedmodified à la X86CPU and only --based properties added in
machine_initfn().
   
   Maybe we can hack QemuOpts to do s/foo_moo/foo-moo/ ?
   Than there won't be need for an explicit conversion and it could be 
   reused by others,
   including X86CPU.
  I like this idea, but this can't be generic for all QemuOpts right? Do you
  mean adding a flag specifying we want this conversion?
 Flag would be less risky than doing it for all QemuOpts unconditionally.
 It would open road for conversion per a QemuOpts which probably should be
 done eventually if options are translated into properties.
Sounds fine, I can go for it.
Andreas, do you agree?

 
  
  Thanks,
  Marcel 
   
   
 As an alternative we could rename the machine option to use -...

That's not been an option for command line compatibility reasons.

Regards,
Andreas

   
  
  
  
 

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[James Hogan]

On 29/05/14 14:08, Aurelien Jarno wrote:
 On Thu, May 29, 2014 at 01:01:38PM +, Petar Jovanovic wrote:
 While adding MIPS_HFLAG_HWRENA_ULR is a good idea for performance
 reasons, I don't think we should add MIPS_HFLAG_CP0UL. This value is
 always the same (true or false) during all the run time of the
 qemu-system-mips binary, and thus we don't need to take care of code
 generated with it being true or being false.

 Initial version of this patch did not have this flag, but it was added
 as requested in code review.
 What do you suggest?
 
 Well maybe I missed something, but it looks to me the value can never
 change as CP0_Config3 is a read-only register. If I am right, probably
 the best is to check directly env-CP0_Config3.
 

loadvm/migration will write Config3, possibly with a different value if
the snapshot being loaded is an older one without userlocal support.
Does tlb_flush() definitely get called in that case? I can't see how
from a quick look, although memory will have been reloaded so perhaps
that is sufficient to guarantee no old translations are lying around.

By the way v3 looks reasonable to me, and seems to work (although I
failed to test savevm/loadvm as it seemed to be already broken with the
particular guest kernel I was using).

Cheers
James

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Marcel Apfelbaum]

On Thu, 2014-05-29 at 14:25 +0200, Andreas Färber wrote:
 Am 29.05.2014 14:21, schrieb Marcel Apfelbaum:
  On Thu, 2014-05-29 at 12:47 +0200, Andreas Färber wrote:
  Am 29.05.2014 11:47, schrieb Igor Mammedov:
  ... fixes freeing constant from vl.c by machine_finalize()
 
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
  Did you check whether there are any others in need of changes? I could
  imagine kernel_irqchip does, and I see that we forgot to fix the
  underscore in the property name, damn. Marcel, can you please look into
  fixing that? I had outlined how to.
  Hi Andreas,
  
  I didn't forget to do that, I responded on the mail thread why I think we 
  shouldn't,
  even if is against QOM best practices.
  I'll copy-paste:
  
  Anyway, the real problem here is that what is elegant in this solution is:
  machine_opts = qemu_get_machine_opts();
  if (qemu_opt_foreach(machine_opts, object_set_property, 
  current_machine, 1)  0) {
  object_unref(OBJECT(current_machine));
  exit(1);
  }
  It automatically fills in the machine state properties with the options 
  from the command line.
  It will work with machine sub-types that have specific properties without 
  the need
  to manually add it to vl.c. The error flow is also elegant (if a sub-type 
  does not have the
  user supplied property).
  
  If we use machine-specific wrappers to convert _ - - we loose the 
  above.
 
 There was nothing machine-specific in my proposal. Only the
 implementation of object_set_property() would need to be extended or
 copiedmodified à la X86CPU and only --based properties added in
 machine_initfn().
OK, sorry, I got it wrong. Anyway, I also like Igor's idea.

Thanks,
Marcel
 
  As an alternative we could rename the machine option to use -...
 
 That's not been an option for command line compatibility reasons.
 
 Regards,
 Andreas
 

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Peter Maydell]

On 29 May 2014 14:22, James Hogan james.ho...@imgtec.com wrote:
 loadvm/migration will write Config3, possibly with a different value if
 the snapshot being loaded is an older one without userlocal support.

Migration load always happens with a freshly reset system,
so this isn't a problem.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Andreas Färber]

Am 29.05.2014 15:21, schrieb Marcel Apfelbaum:
 On Thu, 2014-05-29 at 15:08 +0200, Igor Mammedov wrote:
 On Thu, 29 May 2014 15:56:16 +0300
 Marcel Apfelbaum marce...@redhat.com wrote:
 On Thu, 2014-05-29 at 14:40 +0200, Igor Mammedov wrote:
 Maybe we can hack QemuOpts to do s/foo_moo/foo-moo/ ?
 Than there won't be need for an explicit conversion and it could be reused 
 by others,
 including X86CPU.
 I like this idea, but this can't be generic for all QemuOpts right? Do you
 mean adding a flag specifying we want this conversion?
 Flag would be less risky than doing it for all QemuOpts unconditionally.
 It would open road for conversion per a QemuOpts which probably should be
 done eventually if options are translated into properties.
 Sounds fine, I can go for it.
 Andreas, do you agree?

Whatever works to get a sane QOM ABI without bad side-effects.

Andreas

-- 
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] [v3 PATCH] target-mips: implement UserLocal Register

[Petar Jovanovic]

 change as CP0_Config3 is a read-only register. If I am right, probably
 the best is to check directly env-CP0_Config3.

If you take a look at v1 of the patch, that's what was done.
In the code review, this was marked as unacceptable because it
required [1] passing env within the translator.


[1] http://patchwork.ozlabs.org/patch/349709/

From: Aurelien Jarno [aurel...@aurel32.net]
Sent: Thursday, May 29, 2014 3:08 PM
To: Petar Jovanovic
Cc: Petar Jovanovic; qemu-devel@nongnu.org; afaer...@suse.de; r...@twiddle.net
Subject: Re: [v3 PATCH] target-mips: implement UserLocal Register

On Thu, May 29, 2014 at 01:01:38PM +, Petar Jovanovic wrote:
  While adding MIPS_HFLAG_HWRENA_ULR is a good idea for performance
  reasons, I don't think we should add MIPS_HFLAG_CP0UL. This value is
  always the same (true or false) during all the run time of the
  qemu-system-mips binary, and thus we don't need to take care of code
  generated with it being true or being false.

 Initial version of this patch did not have this flag, but it was added
 as requested in code review.
 What do you suggest?

Well maybe I missed something, but it looks to me the value can never
change as CP0_Config3 is a read-only register. If I am right, probably
the best is to check directly env-CP0_Config3.

--
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Andreas Färber]

Am 29.05.2014 15:35, schrieb Petar Jovanovic:
 change as CP0_Config3 is a read-only register. If I am right, probably
 the best is to check directly env-CP0_Config3.
 
 If you take a look at v1 of the patch, that's what was done.
 In the code review, this was marked as unacceptable because it
 required [1] passing env within the translator.

That was saying you shouldn't add an env argument to the code generator
function. Agree.

If hflags is the wrong field, then you can add a new field to
DisasContext based on env. Note that a while back we eliminated some env
fields from other DisasContexts, so please don't just add an env field.

Andreas

 
 
 [1] http://patchwork.ozlabs.org/patch/349709/
 
 From: Aurelien Jarno [aurel...@aurel32.net]
 Sent: Thursday, May 29, 2014 3:08 PM
 To: Petar Jovanovic
 Cc: Petar Jovanovic; qemu-devel@nongnu.org; afaer...@suse.de; r...@twiddle.net
 Subject: Re: [v3 PATCH] target-mips: implement UserLocal Register
 
 On Thu, May 29, 2014 at 01:01:38PM +, Petar Jovanovic wrote:
 While adding MIPS_HFLAG_HWRENA_ULR is a good idea for performance
 reasons, I don't think we should add MIPS_HFLAG_CP0UL. This value is
 always the same (true or false) during all the run time of the
 qemu-system-mips binary, and thus we don't need to take care of code
 generated with it being true or being false.

 Initial version of this patch did not have this flag, but it was added
 as requested in code review.
 What do you suggest?
 
 Well maybe I missed something, but it looks to me the value can never
 change as CP0_Config3 is a read-only register. If I am right, probably
 the best is to check directly env-CP0_Config3.
 
 --
 Aurelien Jarno  GPG: 4096R/1DDD8C9B
 aurel...@aurel32.net http://www.aurel32.net
 


-- 
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] [v3 PATCH] target-mips: implement UserLocal Register

[Aurelien Jarno]

On Thu, May 29, 2014 at 01:35:17PM +, Petar Jovanovic wrote:
  change as CP0_Config3 is a read-only register. If I am right, probably
  the best is to check directly env-CP0_Config3.
 
 If you take a look at v1 of the patch, that's what was done.
 In the code review, this was marked as unacceptable because it
 required [1] passing env within the translator.

Well, I agree it should not be done to access things that will change at
runtime, as in that case the TB might re-executed with different values
later.

That said in the case of CP0_Config3, it is only used to access a
read-only value, so in my opinion it is something fine. It is something
already done in plenty of places in this code.

What we have there is more a philosophical issue, probably the best way
to fix that is to have a copy of CP0_ConfigX in DisasContext, so that we
don't explicitly access env anymore from the translator.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Peter Maydell]

On 29 May 2014 14:35, Petar Jovanovic petar.jovano...@imgtec.com wrote:
 change as CP0_Config3 is a read-only register. If I am right, probably
 the best is to check directly env-CP0_Config3.

 If you take a look at v1 of the patch, that's what was done.
 In the code review, this was marked as unacceptable because it
 required [1] passing env within the translator.


 [1] http://patchwork.ozlabs.org/patch/349709/

The intention of that review comment is to say that the
bulk of the translator code should not have access directly
to env. This is because most of the fields of env are
not safe to use as a basis for code generation decisions.
Having direct acess to an env pointer makes it very easy
to accidentally use a field that's not safe to use.
Instead we prefer to set up a DisasContext which has only
the required information in it, and pass that around.
The DisasContext is initialised in
gen_intermediate_code_internal from the TB flags and in
some cases from env- fields where this is safe (for
instancec ctx.insn_flags is a copy of env-insn_flags).
This makes it easy to see at a glance what parts of env are
being relied on by the code generation and when something
new is added it's easy to see and check in code review.

In this case we might choose to copy CP0_Config3 (or
just the relevant flag from it) from env into ctx;
I have no particular opinion there.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-ppc] [V4 PATCH 1/6] target-ppc: Support little-endian PPC64 in user mode.

[Tom Musta]

On 5/28/2014 5:59 PM, Alexander Graf wrote:
 
 On 28.05.14 22:34, Tom Musta wrote:
 Look at ELF header to determine ABI version on PPC64.  This is required
 for executing the first instruction correctly.  Also print correct machine
 name in uname() system call.

 Signed-off-by: Doug Kwan address@hidden
 
 You sure that's his email address? :)
 
 
 Alex
 

I will correct this (and the authorship) and resubmit V4.

Re: [Qemu-devel] [PATCH v3 05/34] add memdev backend infrastructure

[Peter Crosthwaite]

On Tue, May 27, 2014 at 11:01 PM, Igor Mammedov imamm...@redhat.com wrote:
 Provides framework for splitting host RAM allocation/
 policies into a separate backend that could be used
 by devices.

 Initially only legacy RAM backend is provided, which
 uses memory_region_init_ram() allocator and compatible
 with every CLI option that affects memory_region_init_ram().

 Signed-off-by: Igor Mammedov imamm...@redhat.com
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
 v4:
  - don't use nonexisting anymore error_is_set()
 v3:
  - fix path leak  use object_get_canonical_path_component()
for getting object name
 v2:
  - reuse UserCreatable interface instead of custom callbacks
 ---
  backends/Makefile.objs   |2 +
  backends/hostmem-ram.c   |   54 ++
  backends/hostmem.c   |  113 
 ++
  include/sysemu/hostmem.h |   60 
  4 files changed, 229 insertions(+), 0 deletions(-)
  create mode 100644 backends/hostmem-ram.c
  create mode 100644 backends/hostmem.c
  create mode 100644 include/sysemu/hostmem.h

 diff --git a/backends/Makefile.objs b/backends/Makefile.objs
 index 591ddcf..7fb7acd 100644
 --- a/backends/Makefile.objs
 +++ b/backends/Makefile.objs
 @@ -6,3 +6,5 @@ common-obj-$(CONFIG_BRLAPI) += baum.o
  baum.o-cflags := $(SDL_CFLAGS)

  common-obj-$(CONFIG_TPM) += tpm.o
 +
 +common-obj-y += hostmem.o hostmem-ram.o
 diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
 new file mode 100644
 index 000..cbf7e5a
 --- /dev/null
 +++ b/backends/hostmem-ram.c
 @@ -0,0 +1,54 @@
 +/*
 + * QEMU Host Memory Backend
 + *
 + * Copyright (C) 2013 Red Hat Inc
 + *
 + * Authors:
 + *   Igor Mammedov imamm...@redhat.com
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or later.
 + * See the COPYING file in the top-level directory.
 + */
 +#include sysemu/hostmem.h
 +#include qom/object_interfaces.h
 +
 +#define TYPE_MEMORY_BACKEND_RAM memory-ram
 +
 +
 +static void
 +ram_backend_memory_init(UserCreatable *uc, Error **errp)
 +{
 +HostMemoryBackend *backend = MEMORY_BACKEND(uc);
 +char *path;
 +
 +if (!backend-size) {
 +error_setg(errp, can't create backend with size 0);
 +return;
 +}
 +
 +path = object_get_canonical_path_component(OBJECT(backend));
 +memory_region_init_ram(backend-mr, OBJECT(backend), path,

Passing the full canonical path as the name of memory region is
redundant as that information is already passed via the owner
argument. It should just be a shorthand.

 +   backend-size);
 +g_free(path);
 +}
 +
 +static void
 +ram_backend_class_init(ObjectClass *oc, void *data)
 +{
 +UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 +
 +ucc-complete = ram_backend_memory_init;
 +}
 +
 +static const TypeInfo ram_backend_info = {
 +.name = TYPE_MEMORY_BACKEND_RAM,
 +.parent = TYPE_MEMORY_BACKEND,
 +.class_init = ram_backend_class_init,
 +};
 +
 +static void register_types(void)
 +{
 +type_register_static(ram_backend_info);
 +}
 +
 +type_init(register_types);
 diff --git a/backends/hostmem.c b/backends/hostmem.c
 new file mode 100644
 index 000..8a34b0f
 --- /dev/null
 +++ b/backends/hostmem.c
 @@ -0,0 +1,113 @@
 +/*
 + * QEMU Host Memory Backend
 + *
 + * Copyright (C) 2013 Red Hat Inc
 + *
 + * Authors:
 + *   Igor Mammedov imamm...@redhat.com
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or later.
 + * See the COPYING file in the top-level directory.
 + */
 +#include sysemu/hostmem.h
 +#include sysemu/sysemu.h
 +#include qapi/visitor.h
 +#include qapi/qmp/qerror.h
 +#include qemu/config-file.h
 +#include qom/object_interfaces.h
 +
 +static void
 +hostmemory_backend_get_size(Object *obj, Visitor *v, void *opaque,
 +const char *name, Error **errp)
 +{
 +HostMemoryBackend *backend = MEMORY_BACKEND(obj);
 +uint64_t value = backend-size;
 +
 +visit_type_size(v, value, name, errp);
 +}
 +
 +static void
 +hostmemory_backend_set_size(Object *obj, Visitor *v, void *opaque,
 +const char *name, Error **errp)
 +{
 +HostMemoryBackend *backend = MEMORY_BACKEND(obj);
 +Error *local_err = NULL;
 +uint64_t value;
 +
 +if (memory_region_size(backend-mr)) {
 +error_setg(local_err, cannot change property value\n);
 +goto out;
 +}
 +
 +visit_type_size(v, value, name, errp);
 +if (local_err) {
 +goto out;
 +}
 +if (!value) {
 +error_setg(local_err, Property '%s.%s' doesn't take value '%
 +   PRIu64 ', object_get_typename(obj), name , value);
 +goto out;
 +}
 +backend-size = value;
 +out:
 +error_propagate(errp, local_err);
 +}
 +
 +static void hostmemory_backend_initfn(Object *obj)

can you just call this _init and ..

 +{
 +object_property_add(obj, size, int,
 +

Re: [Qemu-devel] [PATCH v3 06/34] vl.c: extend -m option to support options for memory hotplug

[Peter Crosthwaite]

On Tue, May 27, 2014 at 11:01 PM, Igor Mammedov imamm...@redhat.com wrote:
 Add following parameters:
   slots - total number of hotplug memory slots
   maxmem - maximum possible memory

 slots and maxmem should go in pair and maxmem should be greater
 than mem for memory hotplug to be enabled.

 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
 v4:
  - store maxmem  slots values in MachineState
 v3:
  - store maxmem  slots values in QEMUMachineInitArgs
 v2:
  - rebased on top of the latest vl: convert -m to QemuOpts
 ---
  include/hw/boards.h |3 ++-
  qemu-options.hx |9 ++---
  vl.c|   51 
 +++
  3 files changed, 59 insertions(+), 4 deletions(-)

 diff --git a/include/hw/boards.h b/include/hw/boards.h
 index b62de4a..f6fbbe1 100644
 --- a/include/hw/boards.h
 +++ b/include/hw/boards.h
 @@ -8,7 +8,6 @@
  #include hw/qdev.h
  #include qom/object.h

 -

A nit, but this is out of scope.

Regards,
Peter

  typedef struct MachineState MachineState;

  typedef void QEMUMachineInitFunc(MachineState *ms);
 @@ -113,6 +112,8 @@ struct MachineState {
  char *firmware;

  ram_addr_t ram_size;
 +ram_addr_t maxram_size;
 +uint64_t   ram_slots;
  const char *boot_order;
  const char *kernel_filename;
  const char *kernel_cmdline;
 diff --git a/qemu-options.hx b/qemu-options.hx
 index c2c0823..dc3d8d5 100644
 --- a/qemu-options.hx
 +++ b/qemu-options.hx
 @@ -210,17 +210,20 @@ use is discouraged as it may be removed from future 
 versions.
  ETEXI

  DEF(m, HAS_ARG, QEMU_OPTION_m,
 --m [size=]megs\n
 +-m[emory] [size=]megs[,slots=n,maxmem=size]\n
  configure guest RAM\n
  size: initial amount of guest memory (default: 
 -stringify(DEFAULT_RAM_SIZE) MiB)\n,
 +stringify(DEFAULT_RAM_SIZE) MiB)\n
 +slots: number of hotplug slots (default: none)\n
 +maxmem: maximum amount of guest memory (default: 
 none)\n,
  QEMU_ARCH_ALL)
  STEXI
  @item -m [size=]@var{megs}
  @findex -m
  Set virtual RAM size to @var{megs} megabytes. Default is 128 MiB.  
 Optionally,
  a suffix of ``M'' or ``G'' can be used to signify a value in megabytes or
 -gigabytes respectively.
 +gigabytes respectively. Optional pair @var{slots}, @var{maxmem} could be used
 +to set amount of hotluggable memory slots and possible maximum amount of 
 memory.
  ETEXI

  DEF(mem-path, HAS_ARG, QEMU_OPTION_mempath,
 diff --git a/vl.c b/vl.c
 index 8fd4ed9..9fb6fa4 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -520,6 +520,14 @@ static QemuOptsList qemu_mem_opts = {
  .name = size,
  .type = QEMU_OPT_SIZE,
  },
 +{
 +.name = slots,
 +.type = QEMU_OPT_NUMBER,
 +},
 +{
 +.name = maxmem,
 +.type = QEMU_OPT_SIZE,
 +},
  { /* end of list */ }
  },
  };
 @@ -2989,6 +2997,8 @@ int main(int argc, char **argv, char **envp)
  const char *trace_file = NULL;
  const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
  1024 * 1024;
 +ram_addr_t maxram_size = default_ram_size;
 +uint64_t ram_slots = 0;

  atexit(qemu_run_exit_notifiers);
  error_set_progname(argv[0]);
 @@ -3324,6 +3334,7 @@ int main(int argc, char **argv, char **envp)
  case QEMU_OPTION_m: {
  uint64_t sz;
  const char *mem_str;
 +const char *maxmem_str, *slots_str;

  opts = qemu_opts_parse(qemu_find_opts(memory),
 optarg, 1);
 @@ -3365,6 +3376,44 @@ int main(int argc, char **argv, char **envp)
  error_report(ram size too large);
  exit(EXIT_FAILURE);
  }
 +
 +maxmem_str = qemu_opt_get(opts, maxmem);
 +slots_str = qemu_opt_get(opts, slots);
 +if (maxmem_str  slots_str) {
 +uint64_t slots;
 +
 +sz = qemu_opt_get_size(opts, maxmem, 0);
 +if (sz  ram_size) {
 +fprintf(stderr, qemu: invalid -m option value: 
 maxmem 
 +(% PRIu64 ) = initial memory (%
 +PRIu64 )\n, sz, ram_size);
 +exit(EXIT_FAILURE);
 +}
 +
 +slots = qemu_opt_get_number(opts, slots, 0);
 +if ((sz  ram_size)  !slots) {
 +fprintf(stderr, qemu: invalid -m option value: 
 maxmem 
 +(% PRIu64 ) more than initial memory (%
 +PRIu64 ) but no hotplug slots where 
 +specified\n, sz, ram_size);
 +exit(EXIT_FAILURE);
 +}
 +
 +if ((sz = ram_size)  

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Aurelien Jarno]

On Thu, May 29, 2014 at 02:48:28PM +0100, Peter Maydell wrote:
 On 29 May 2014 14:35, Petar Jovanovic petar.jovano...@imgtec.com wrote:
  change as CP0_Config3 is a read-only register. If I am right, probably
  the best is to check directly env-CP0_Config3.
 
  If you take a look at v1 of the patch, that's what was done.
  In the code review, this was marked as unacceptable because it
  required [1] passing env within the translator.
 
 
  [1] http://patchwork.ozlabs.org/patch/349709/
 
 The intention of that review comment is to say that the
 bulk of the translator code should not have access directly
 to env. This is because most of the fields of env are
 not safe to use as a basis for code generation decisions.
 Having direct acess to an env pointer makes it very easy
 to accidentally use a field that's not safe to use.
 Instead we prefer to set up a DisasContext which has only
 the required information in it, and pass that around.
 The DisasContext is initialised in
 gen_intermediate_code_internal from the TB flags and in
 some cases from env- fields where this is safe (for
 instancec ctx.insn_flags is a copy of env-insn_flags).
 This makes it easy to see at a glance what parts of env are
 being relied on by the code generation and when something
 new is added it's easy to see and check in code review.
 
 In this case we might choose to copy CP0_Config3 (or
 just the relevant flag from it) from env into ctx;
 I have no particular opinion there.

It looks like one bit of CP0_Config3 is RW when microMIPS is
implemented, so it might be better to copy only the corresponding bit.


-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

[Qemu-devel] [V4 PATCH (resend) 0/6] Adding New Target ppc64le-linux-user

[Tom Musta]

This is a follow up to the patch series initiated by Doug Kwan
http://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg01936.html

The resubmission of V4 cleans up some attribution issues (corrects Doug's 
email address in the Signed-off-by and also cites Doug as the author
of the first three patches)

Doug Kwan (3):
  target-ppc: Support little-endian PPC64 in user mode.
  target-ppc: Allow little-endian user mode.
  target-ppc: Add a new user mode target for little-endian PPC64.

Tom Musta (3):
  target-ppc: Support VSX in PPC User Mode
  target-ppc: Confirm That .bss Pages Are Valid
  target-ppc: Store Quadword Conditional Drops Size Bit

 configure   |6 ++
 include/elf.h   |5 ++
 linux-user/elfload.c|   24 ++--
 linux-user/main.c   |   10 ++-
 linux-user/ppc/syscall.h|4 +
 target-ppc/mem_helper.c |   26 ++-
 target-ppc/translate.c  |  151 +--
 target-ppc/translate_init.c |4 +
 8 files changed, 128 insertions(+), 102 deletions(-)

[Qemu-devel] [V4 PATCH (resend) 1/6] target-ppc: Support little-endian PPC64 in user mode.

[Tom Musta]

From: Doug Kwan dougk...@google.com

Look at ELF header to determine ABI version on PPC64.  This is required
for executing the first instruction correctly.  Also print correct machine
name in uname() system call.

Signed-off-by: Doug Kwan dougk...@google.com
Signed-off-by: Tom Musta tommu...@gmail.com

---
V3: Addressing Peter Maydell's comments regarding how and where to define
UNAME_MACHINE.  Addressed indentation issue (checkpatch failure).

 include/elf.h|5 +
 linux-user/elfload.c |   17 +++--
 linux-user/ppc/syscall.h |4 
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/include/elf.h b/include/elf.h
index 1599ab2..e88d52f 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -561,6 +561,11 @@ typedef struct {
 #define SHF_ALPHA_GPREL0x1000
 
 
+/* PowerPC specific definitions.  */
+
+/* Processor specific flags for the ELF header e_flags field.  */
+#define EF_PPC64_ABI   0x3
+
 /* PowerPC relocations defined by the ABIs */
 #define R_PPC_NONE 0
 #define R_PPC_ADDR32   1   /* 32bit absolute address */
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 995f999..def9698 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -777,12 +777,18 @@ static uint32_t get_elf_hwcap(void)
 NEW_AUX_ENT(AT_IGNOREPPC, AT_IGNOREPPC);\
 } while (0)
 
+static inline uint32_t get_ppc64_abi(struct image_info *infop);
+
 static inline void init_thread(struct target_pt_regs *_regs, struct image_info 
*infop)
 {
 _regs-gpr[1] = infop-start_stack;
 #if defined(TARGET_PPC64)  !defined(TARGET_ABI32)
-_regs-gpr[2] = ldq_raw(infop-entry + 8) + infop-load_bias;
-infop-entry = ldq_raw(infop-entry) + infop-load_bias;
+if (get_ppc64_abi(infop)  2) {
+_regs-gpr[2] = ldq_raw(infop-entry + 8) + infop-load_bias;
+infop-entry = ldq_raw(infop-entry) + infop-load_bias;
+} else {
+_regs-gpr[12] = infop-entry;  /* r12 set to global entry address */
+}
 #endif
 _regs-nip = infop-entry;
 }
@@ -1152,6 +1158,13 @@ static inline void init_thread(struct target_pt_regs 
*regs, struct image_info *i
 
 #include elf.h
 
+#ifdef TARGET_PPC
+static inline uint32_t get_ppc64_abi(struct image_info *infop)
+{
+  return infop-elf_flags  EF_PPC64_ABI;
+}
+#endif
+
 struct exec
 {
 unsigned int a_info;   /* Use macros N_MAGIC, etc for access */
diff --git a/linux-user/ppc/syscall.h b/linux-user/ppc/syscall.h
index 6514c63..db92bbe 100644
--- a/linux-user/ppc/syscall.h
+++ b/linux-user/ppc/syscall.h
@@ -58,8 +58,12 @@ struct target_revectored_struct {
  */
 
 #if defined(TARGET_PPC64)  !defined(TARGET_ABI32)
+#ifdef TARGET_WORDS_BIGENDIAN
 #define UNAME_MACHINE ppc64
 #else
+#define UNAME_MACHINE ppc64le
+#endif
+#else
 #define UNAME_MACHINE ppc
 #endif
 #define UNAME_MINIMUM_RELEASE 2.6.32
-- 
1.7.1

[Qemu-devel] [V4 PATCH (resend) 3/6] target-ppc: Add a new user mode target for little-endian PPC64.

[Tom Musta]

From: Doug Kwan dougk...@google.com

Signed-off-by: Doug Kwan dougk...@google.com
Signed-off-by: Tom Musta tommu...@gmail.com

---
V3: Addressing comment from Peter Maydell.  Adding libdecnumber enable.

 configure |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 0e516f9..3ccbe4b 100755
--- a/configure
+++ b/configure
@@ -4930,6 +4930,12 @@ case $target_name in
 TARGET_ABI_DIR=ppc
 gdb_xml_files=power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml
   ;;
+  ppc64le)
+TARGET_ARCH=ppc64
+TARGET_BASE_ARCH=ppc
+TARGET_ABI_DIR=ppc
+gdb_xml_files=power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml
+  ;;
   ppc64abi32)
 TARGET_ARCH=ppc64
 TARGET_BASE_ARCH=ppc
-- 
1.7.1

[Qemu-devel] [V4 PATCH (resend) 5/6] target-ppc: Confirm That .bss Pages Are Valid

[Tom Musta]

The existing code does a check to ensure that a .bss region is properly
mmap'd.  When additional mmap is required, the (guest) pages are also
validated.  However, this code has a bug: when host page size is larger
than target page size, it is possible for the .bss pages to already be
(host) mapped but the guest .bss pages may not be valid.

The check to mmap additional space is separated from the flagging of the
target (guest) pages, thus ensuring that both aspects are done properly.

Signed-off-by: Tom Musta tommu...@gmail.com

---
V3: new patch

 linux-user/elfload.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index def9698..0af6292 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1418,10 +1418,11 @@ static void zero_bss(abi_ulong elf_bss, abi_ulong 
last_bss, int prot)
 perror(cannot mmap brk);
 exit(-1);
 }
+}
 
-/* Since we didn't use target_mmap, make sure to record
-   the validity of the pages with qemu.  */
-page_set_flags(elf_bss  TARGET_PAGE_MASK, last_bss, prot|PAGE_VALID);
+/* Ensure that the bss page(s) are valid */
+if ((page_get_flags(last_bss-1)  prot) != prot) {
+page_set_flags(elf_bss  TARGET_PAGE_MASK, last_bss, prot | 
PAGE_VALID);
 }
 
 if (host_start  host_map_start) {
-- 
1.7.1

[Qemu-devel] [V4 PATCH (resend) 6/6] target-ppc: Store Quadword Conditional Drops Size Bit

[Tom Musta]

The size and register information are encoded into the reserve_info field
of CPU state in the store conditional translation code.  Specifically, the
size is shifted left by 5 bits (see target-ppc/translate.c 
gen_conditional_store).

The user-mode store conditional code erroneously extracts the size by ANDing
with a 4 bit mask; this breaks if size = 16.

Eliminate the mask to make the extraction of size mirror its encoding.

Signed-off-by: Tom Musta tommu...@gmail.com

---
V4: new patch.

 linux-user/main.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index c5668af..d7609f7 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1497,7 +1497,7 @@ static int do_store_exclusive(CPUPPCState *env)
 segv = 1;
 } else {
 int reg = env-reserve_info  0x1f;
-int size = (env-reserve_info  5)  0xf;
+int size = env-reserve_info  5;
 int stored = 0;
 
 if (addr == env-reserve_addr) {
-- 
1.7.1

[Qemu-devel] [V4 PATCH (resend) 4/6] target-ppc: Support VSX in PPC User Mode

[Tom Musta]

Some modern tool chains use VSX instructions.  Therefore attempt to enable the 
VSX MSR
bit by default, just like similar bits (FP, VEC, SPE, etc.).

Signed-off-by: Tom Musta tommu...@gmail.com

---
V3: new patch

 target-ppc/translate_init.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 4a9f5b8..ec08e45 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8416,6 +8416,7 @@ static void ppc_cpu_reset(CPUState *s)
 #if defined(CONFIG_USER_ONLY)
 msr |= (target_ulong)1  MSR_FP; /* Allow floating point usage */
 msr |= (target_ulong)1  MSR_VR; /* Allow altivec usage */
+msr |= (target_ulong)1  MSR_VSX; /* Allow VSX usage */
 msr |= (target_ulong)1  MSR_SPE; /* Allow SPE usage */
 msr |= (target_ulong)1  MSR_PR;
 #if !defined(TARGET_WORDS_BIGENDIAN)
-- 
1.7.1

[Qemu-devel] [V4 PATCH (resend) 2/6] target-ppc: Allow little-endian user mode.

[Tom Musta]

From: Doug Kwan dougk...@google.com

This allows running PPC64 little-endian in user mode if target is configured
that way.  In PPC64 LE user mode we set MSR.LE during initialization.

Signed-off-by: Doug Kwan dougk...@google.com
Signed-off-by: Tom Musta tommu...@gmail.com

---
V2: Overhaul handling of byteswapping in code generation and mem helpers.

V3: Eliminating MSR[LE] check in user mode per Alex Graf's review.  Addressed
checkpatch violations.

V4: Fixed endianness problem in user-mode emulation of stqcx (spotted by Peter
Maydell)

 linux-user/main.c   |8 ++-
 target-ppc/mem_helper.c |   26 ++-
 target-ppc/translate.c  |  151 +--
 target-ppc/translate_init.c |3 +
 4 files changed, 92 insertions(+), 96 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 882186e..c5668af 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1484,7 +1484,7 @@ static int do_store_exclusive(CPUPPCState *env)
 {
 target_ulong addr;
 target_ulong page_addr;
-target_ulong val, val2 __attribute__((unused));
+target_ulong val, val2 __attribute__((unused)) = 0;
 int flags;
 int segv = 0;
 
@@ -1527,6 +1527,12 @@ static int do_store_exclusive(CPUPPCState *env)
 case 8: segv = put_user_u64(val, addr); break;
 case 16: {
 if (val2 == env-reserve_val2) {
+if (msr_le) {
+val2 = val;
+val = env-gpr[reg+1];
+} else {
+val2 = env-gpr[reg+1];
+}
 segv = put_user_u64(val, addr);
 if (!segv) {
 segv = put_user_u64(val2, addr + 8);
diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index d9c8c36..5275f90 100644
--- a/target-ppc/mem_helper.c
+++ b/target-ppc/mem_helper.c
@@ -28,6 +28,15 @@
 
 //#define DEBUG_OP
 
+static inline bool needs_byteswap(const CPUPPCState *env)
+{
+#if defined(TARGET_WORDS_BIGENDIAN)
+  return msr_le;
+#else
+  return !msr_le;
+#endif
+}
+
 /*/
 /* Memory load and stores */
 
@@ -47,7 +56,7 @@ static inline target_ulong addr_add(CPUPPCState *env, 
target_ulong addr,
 void helper_lmw(CPUPPCState *env, target_ulong addr, uint32_t reg)
 {
 for (; reg  32; reg++) {
-if (msr_le) {
+if (needs_byteswap(env)) {
 env-gpr[reg] = bswap32(cpu_ldl_data(env, addr));
 } else {
 env-gpr[reg] = cpu_ldl_data(env, addr);
@@ -59,7 +68,7 @@ void helper_lmw(CPUPPCState *env, target_ulong addr, uint32_t 
reg)
 void helper_stmw(CPUPPCState *env, target_ulong addr, uint32_t reg)
 {
 for (; reg  32; reg++) {
-if (msr_le) {
+if (needs_byteswap(env)) {
 cpu_stl_data(env, addr, bswap32((uint32_t)env-gpr[reg]));
 } else {
 cpu_stl_data(env, addr, (uint32_t)env-gpr[reg]);
@@ -202,6 +211,11 @@ target_ulong helper_lscbx(CPUPPCState *env, target_ulong 
addr, uint32_t reg,
 #define LO_IDX 0
 #endif
 
+/* We use msr_le to determine index ordering in a vector.  However,
+   byteswapping is not simply controlled by msr_le.  We also need to take
+   into account endianness of the target.  This is done for the little-endian
+   PPC64 user-mode target. */
+
 #define LVE(name, access, swap, element)\
 void helper_##name(CPUPPCState *env, ppc_avr_t *r,  \
target_ulong addr)   \
@@ -210,9 +224,11 @@ target_ulong helper_lscbx(CPUPPCState *env, target_ulong 
addr, uint32_t reg,
 int adjust = HI_IDX*(n_elems - 1);  \
 int sh = sizeof(r-element[0])  1;\
 int index = (addr  0xf)  sh; \
-\
 if (msr_le) {   \
 index = n_elems - index - 1;\
+}   \
+\
+if (needs_byteswap(env)) {  \
 r-element[LO_IDX ? index : (adjust - index)] = \
 swap(access(env, addr));\
 } else {\
@@ -235,9 +251,11 @@ LVE(lvewx, cpu_ldl_data, bswap32, u32)
 int adjust = HI_IDX * (n_elems - 1);\
 int sh = sizeof(r-element[0])  1;\
 int index = (addr  0xf)  sh; \
-\
 if (msr_le) {   \
 index 

Re: [Qemu-devel] [PATCH v3 10/34] dimm: implement dimm device abstraction

[Peter Crosthwaite]

On Tue, May 27, 2014 at 11:01 PM, Igor Mammedov imamm...@redhat.com wrote:
 From: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com

 Each hotplug-able memory slot is a DimmDevice.
 A hot-add operation for a DIMM:
 - creates a new DimmDevice and makes hotplug controller to map it into
   guest address space

 Hotplug operations are done through normal device_add commands.
 For migration case, all hotplugged DIMMs on source should be specified
 on target's command line using '-device' option with properties set to
 the same values as on source.

 To simplify review, patch introduces only DimmDevice QOM skeleton that
 will be extended by following patches to implement actual memory hotplug
 and related functions.

 Signed-off-by: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
 v4:
   drop DimmBus in favor of bus-less device hotplug
   rename property/field 'start' to 'addr'
   use defines for property names
 v3:
   pc: compile memhotplug on i386 target too
 v2:
   fix typo s/DimmBus/DimmDevice/ in doc comment
   s/klass/oc/;s/*/parent_obj/;a/gtk-doc markup/
 ---
  default-configs/i386-softmmu.mak   |1 +
  default-configs/x86_64-softmmu.mak |1 +
  hw/Makefile.objs   |1 +
  hw/mem/Makefile.objs   |1 +
  hw/mem/dimm.c  |  103 
 
  include/hw/mem/dimm.h  |   73 +
  6 files changed, 180 insertions(+), 0 deletions(-)
  create mode 100644 hw/mem/Makefile.objs
  create mode 100644 hw/mem/dimm.c
  create mode 100644 include/hw/mem/dimm.h

 diff --git a/default-configs/i386-softmmu.mak 
 b/default-configs/i386-softmmu.mak
 index 37ef90f..8e08841 100644
 --- a/default-configs/i386-softmmu.mak
 +++ b/default-configs/i386-softmmu.mak
 @@ -44,3 +44,4 @@ CONFIG_APIC=y
  CONFIG_IOAPIC=y
  CONFIG_ICC_BUS=y
  CONFIG_PVPANIC=y
 +CONFIG_MEM_HOTPLUG=y
 diff --git a/default-configs/x86_64-softmmu.mak 
 b/default-configs/x86_64-softmmu.mak
 index 31bddce..66557ac 100644
 --- a/default-configs/x86_64-softmmu.mak
 +++ b/default-configs/x86_64-softmmu.mak
 @@ -44,3 +44,4 @@ CONFIG_APIC=y
  CONFIG_IOAPIC=y
  CONFIG_ICC_BUS=y
  CONFIG_PVPANIC=y
 +CONFIG_MEM_HOTPLUG=y
 diff --git a/hw/Makefile.objs b/hw/Makefile.objs
 index d178b65..52a1464 100644
 --- a/hw/Makefile.objs
 +++ b/hw/Makefile.objs
 @@ -29,6 +29,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += usb/
  devices-dirs-$(CONFIG_VIRTIO) += virtio/
  devices-dirs-$(CONFIG_SOFTMMU) += watchdog/
  devices-dirs-$(CONFIG_SOFTMMU) += xen/
 +devices-dirs-$(CONFIG_MEM_HOTPLUG) += mem/
  devices-dirs-y += core/
  common-obj-y += $(devices-dirs-y)
  obj-y += $(devices-dirs-y)
 diff --git a/hw/mem/Makefile.objs b/hw/mem/Makefile.objs
 new file mode 100644
 index 000..7563ef5
 --- /dev/null
 +++ b/hw/mem/Makefile.objs
 @@ -0,0 +1 @@
 +common-obj-$(CONFIG_MEM_HOTPLUG) += dimm.o
 diff --git a/hw/mem/dimm.c b/hw/mem/dimm.c
 new file mode 100644
 index 000..bb81679
 --- /dev/null
 +++ b/hw/mem/dimm.c
 @@ -0,0 +1,103 @@
 +/*
 + * Dimm device for Memory Hotplug
 + *
 + * Copyright ProfitBricks GmbH 2012
 + * Copyright (C) 2014 Red Hat Inc
 + *
 + * This library is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU Lesser General Public
 + * License as published by the Free Software Foundation; either
 + * version 2 of the License, or (at your option) any later version.
 + *
 + * This library is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * Lesser General Public License for more details.
 + *
 + * You should have received a copy of the GNU Lesser General Public
 + * License along with this library; if not, see 
 http://www.gnu.org/licenses/
 + */
 +
 +#include hw/mem/dimm.h
 +#include qemu/config-file.h
 +#include qapi/visitor.h
 +
 +static Property dimm_properties[] = {
 +DEFINE_PROP_UINT64(DIMM_ADDR_PROP, DimmDevice, addr, 0),

One of the long-standing rules of sysbus device design, is devices
should not have awareness of their memory mappings. Especially in
cases where that mapping is handled by buses and controllers (which is
the case in DIMM? - does the actual DIMM card have any awareness of
its own mapping?).

That said I'm generally against that policy as sometimes devices
really do know their absolute address in real HW implementation. So
despite being generally against an old consensus I think this might be
ok.

 +DEFINE_PROP_UINT32(DIMM_NODE_PROP, DimmDevice, node, 0),
 +DEFINE_PROP_INT32(DIMM_SLOT_PROP, DimmDevice, slot, 
 DIMM_UNASSIGNED_SLOT),

I think this slot property may however reduce the re-usability of DIMM
beyond PC. Is the concept of enumerated DIMM slots a DIMM level
feature or is it more PC specific?

 +DEFINE_PROP_END_OF_LIST(),
 +};
 +
 +static void dimm_get_size(Object *obj, 

[Qemu-devel] [PATCH] target-mips: copy CP0_Config1 into DisasContext

[Aurelien Jarno]

In order to avoid access to the CPUMIPSState structure in the
translator, keep a copy of CP0_Config1 into DisasContext. The whole
register is read-only so it can be copied as a single value.

Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
 target-mips/translate.c |   20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 13cf29b..bb89413 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -1066,6 +1066,7 @@ typedef struct DisasContext {
 uint32_t opcode;
 int singlestep_enabled;
 int insn_flags;
+int32_t CP0_Config1;
 /* Routine used to access memory */
 int mem_idx;
 uint32_t hflags, saved_hflags;
@@ -1919,10 +1920,10 @@ static void gen_flt_ldst (DisasContext *ctx, uint32_t 
opc, int ft,
 tcg_temp_free(t0);
 }
 
-static void gen_cop1_ldst(CPUMIPSState *env, DisasContext *ctx,
-  uint32_t op, int rt, int rs, int16_t imm)
+static void gen_cop1_ldst(DisasContext *ctx, uint32_t op, int rt,
+  int rs, int16_t imm)
 {
-if (env-CP0_Config1  (1  CP0C1_FP)) {
+if (ctx-CP0_Config1  (1  CP0C1_FP)) {
 check_cp1_enabled(ctx);
 gen_flt_ldst(ctx, op, rt, rs, imm);
 } else {
@@ -11789,7 +11790,7 @@ static void decode_micromips32_opc (CPUMIPSState *env, 
DisasContext *ctx,
 }
 break;
 case POOL32F:
-if (env-CP0_Config1  (1  CP0C1_FP)) {
+if (ctx-CP0_Config1  (1  CP0C1_FP)) {
 minor = ctx-opcode  0x3f;
 check_cp1_enabled(ctx);
 switch (minor) {
@@ -12303,7 +12304,7 @@ static void decode_micromips32_opc (CPUMIPSState *env, 
DisasContext *ctx,
 case SDC132:
 mips32_op = OPC_SDC1;
 do_cop1:
-gen_cop1_ldst(env, ctx, mips32_op, rt, rs, imm);
+gen_cop1_ldst(ctx, mips32_op, rt, rs, imm);
 break;
 case ADDIUPC:
 {
@@ -14551,7 +14552,7 @@ static void decode_opc (CPUMIPSState *env, DisasContext 
*ctx)
 
 case OPC_MOVCI:
 check_insn(ctx, ISA_MIPS4 | ISA_MIPS32);
-if (env-CP0_Config1  (1  CP0C1_FP)) {
+if (ctx-CP0_Config1  (1  CP0C1_FP)) {
 check_cp1_enabled(ctx);
 gen_movci(ctx, rd, rs, (ctx-opcode  18)  0x7,
   (ctx-opcode  16)  1);
@@ -15430,11 +15431,11 @@ static void decode_opc (CPUMIPSState *env, 
DisasContext *ctx)
 case OPC_LDC1:
 case OPC_SWC1:
 case OPC_SDC1:
-gen_cop1_ldst(env, ctx, op, rt, rs, imm);
+gen_cop1_ldst(ctx, op, rt, rs, imm);
 break;
 
 case OPC_CP1:
-if (env-CP0_Config1  (1  CP0C1_FP)) {
+if (ctx-CP0_Config1  (1  CP0C1_FP)) {
 check_cp1_enabled(ctx);
 op1 = MASK_CP1(ctx-opcode);
 switch (op1) {
@@ -15496,7 +15497,7 @@ static void decode_opc (CPUMIPSState *env, DisasContext 
*ctx)
 break;
 
 case OPC_CP3:
-if (env-CP0_Config1  (1  CP0C1_FP)) {
+if (ctx-CP0_Config1  (1  CP0C1_FP)) {
 check_cp1_enabled(ctx);
 op1 = MASK_CP3(ctx-opcode);
 switch (op1) {
@@ -15604,6 +15605,7 @@ gen_intermediate_code_internal(MIPSCPU *cpu, 
TranslationBlock *tb,
 ctx.saved_pc = -1;
 ctx.singlestep_enabled = cs-singlestep_enabled;
 ctx.insn_flags = env-insn_flags;
+ctx.CP0_Config1 = env-CP0_Config1;
 ctx.tb = tb;
 ctx.bstate = BS_NONE;
 /* Restore delay slot state from the tb context.  */
-- 
1.7.10.4

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Aurelien Jarno]

On Thu, May 29, 2014 at 04:06:56PM +0200, Aurelien Jarno wrote:
 On Thu, May 29, 2014 at 02:48:28PM +0100, Peter Maydell wrote:
  On 29 May 2014 14:35, Petar Jovanovic petar.jovano...@imgtec.com wrote:
   change as CP0_Config3 is a read-only register. If I am right, probably
   the best is to check directly env-CP0_Config3.
  
   If you take a look at v1 of the patch, that's what was done.
   In the code review, this was marked as unacceptable because it
   required [1] passing env within the translator.
  
  
   [1] http://patchwork.ozlabs.org/patch/349709/
  
  The intention of that review comment is to say that the
  bulk of the translator code should not have access directly
  to env. This is because most of the fields of env are
  not safe to use as a basis for code generation decisions.
  Having direct acess to an env pointer makes it very easy
  to accidentally use a field that's not safe to use.
  Instead we prefer to set up a DisasContext which has only
  the required information in it, and pass that around.
  The DisasContext is initialised in
  gen_intermediate_code_internal from the TB flags and in
  some cases from env- fields where this is safe (for
  instancec ctx.insn_flags is a copy of env-insn_flags).
  This makes it easy to see at a glance what parts of env are
  being relied on by the code generation and when something
  new is added it's easy to see and check in code review.
  
  In this case we might choose to copy CP0_Config3 (or
  just the relevant flag from it) from env into ctx;
  I have no particular opinion there.
 
 It looks like one bit of CP0_Config3 is RW when microMIPS is
 implemented, so it might be better to copy only the corresponding bit.

I have just posted a patch copying CP0_Config1 into DisasContext, and
moving existing accesses to this variable accordingly. This can be used
as an example how to do it.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[James Hogan]

On 29/05/14 14:26, Peter Maydell wrote:
 On 29 May 2014 14:22, James Hogan james.ho...@imgtec.com wrote:
 loadvm/migration will write Config3, possibly with a different value if
 the snapshot being loaded is an older one without userlocal support.
 
 Migration load always happens with a freshly reset system,
 so this isn't a problem.

Why does that make a difference? The snapshot may have been saved on a
slightly different version without that bit set (if that isn't supported
then we may as well not bother trying to support loading different
snapshot versions). Also loadvm can happen at any time, not just after
reset.

Is this patch missing something to actually set that bit in Config3?

Cheers
James

Re: [Qemu-devel] [PATCH v3 11/34] memory: add memory_region_is_mapped() API

[Peter Crosthwaite]

On Tue, May 27, 2014 at 11:01 PM, Igor Mammedov imamm...@redhat.com wrote:
 which allows to check if MemoryRegion is already mapped.

 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  include/exec/memory.h |8 
  memory.c  |   15 ++-
  2 files changed, 22 insertions(+), 1 deletions(-)

 diff --git a/include/exec/memory.h b/include/exec/memory.h
 index 1d55ad9..ab11c32 100644
 --- a/include/exec/memory.h
 +++ b/include/exec/memory.h
 @@ -848,6 +848,14 @@ void memory_region_set_alias_offset(MemoryRegion *mr,
  bool memory_region_present(MemoryRegion *parent, hwaddr addr);

  /**
 + * memory_region_is_mapped: returns true if #MemoryRegion is mapped
 + * into any address space.
 + *
 + * @mr: a #MemoryRegion which should be checked if it's mapped
 + */
 +bool memory_region_is_mapped(MemoryRegion *mr);
 +
 +/**
   * memory_region_find: translate an address/size relative to a
   * MemoryRegion into a #MemoryRegionSection.
   *
 diff --git a/memory.c b/memory.c
 index 3f1df23..f4d8e69 100644
 --- a/memory.c
 +++ b/memory.c
 @@ -492,7 +492,7 @@ static AddressSpace 
 *memory_region_to_address_space(MemoryRegion *mr)
  return as;
  }
  }
 -abort();
 +return NULL;
  }

  /* Render a memory region into the global view.  Ranges in @view obscure
 @@ -1569,6 +1569,16 @@ bool memory_region_present(MemoryRegion *parent, 
 hwaddr addr)
  return true;
  }

 +bool memory_region_is_mapped(MemoryRegion *mr)
 +{

Is it not enough to just return mr-parent? Memory mapping assertion
will happen if you try and map the same twice, even if one of the
mappings is not contained within an AddressSpace. Checking for just
the parent mr may be a simpler and more accurate check.

Regards,
Peter

 +mr = memory_region_find(mr, 0, 1).mr;
 +if (!mr) {
 +return false;
 +}
 +memory_region_unref(mr);
 +return true;
 +}
 +
  MemoryRegionSection memory_region_find(MemoryRegion *mr,
 hwaddr addr, uint64_t size)
  {
 @@ -1586,6 +1596,9 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
  }

  as = memory_region_to_address_space(root);
 +if (!as) {
 +return ret;
 +}
  range = addrrange_make(int128_make64(addr), int128_make64(size));

  view = address_space_get_flatview(as);
 --
 1.7.1

Re: [Qemu-devel] [PATCH v3 00/34] pc: ACPI memory hotplug

[Peter Crosthwaite]

On Tue, May 27, 2014 at 11:00 PM, Igor Mammedov imamm...@redhat.com wrote:
 NOTE to commiter:
  * update precompiled ACPI hex files for iasl-less hosts
and ACPI tables test to match new ACPI tables.

 What's new since v9:
  * drop usage of error_is_set()
  * exit QEMU if it's started with memory hotplug but
compat machine doesn't support it
  * rename cpu_hotplug_defs.h to pc-hotplug.h
  * use subsection when migrating acpi memory hotplug state
  * add property to PC_MACHINE for getting hotplug memory region size

 What's new since v8:
  * rebased on top of Marcel's QOMyfing machine work
 depends on patch from qom-next:
  machine: Conversion of QEMUMachineInitArgs to MachineState
  * fixed QEMU abort if it's running in daemonized mode
  * fixed leak in memdev backend
  * introduced custom PCMachine
  * DIMM devices are now bus-less and use bus-less hotplug method
  * DIMMDevice: renamed property/field 'start' to 'addr'
  * ACPI tables:
* avoid punching hotples in PCI CRS by placing
  MEMORY_HOPTLUG_DEVICE on PCI0 bus
  * incorporated most of comments/fixes from reviewers

 What's new since v7:
 * Per Andreas' suggestion dropped DIMMBus concept.
 * Added hotplug binding for bus-less devices
 * DIMM device is split to backend and frontend. Therefore following
   command/options were added for supporting it:

   For memory-ram backend:
   CLI: -object-add memory-ram,
   with options: 'id' and 'size'
   For dimm frontend:
   option size became readonly, pulling it's size from attached backend
   added option memdev for specifying backend by 'id'

 * dropped support for 32 bit guests
 * failed hotplug action doesn't consume 1 slot anymore
 * vaious fixes adressing reviewer's comments most of them in ACPI part
 ---

 This series allows to hotplug 'arbitrary' DIMM devices specifying size,
 NUMA node mapping (guest side), slot and address where to map it, at runtime.

 Due to ACPI limitation there is need to specify a number of possible
 DIMM devices. For this task -m option was extended to support
 following format:

   -m [mem=]RamSize[,slots=N,maxmem=M]

 To allow memory hotplug user must specify a pair of additional parameters:
 'slots' - number of possible increments
 'maxmem' - max possible total memory size QEMU is allowed to use,
including RamSize.

 minimal monitor command syntax to hotplug DIMM device:

   object_add memory-ram,id=memX,size=1G
   device_add dimm,id=dimmX,memdev=memX

 DIMM device provides following properties that could be used with
 device_add / -device to alter default behavior:

   id- unique string identifying device [mandatory]
   slot  - number in range [0-slots) [optional], if not specified
   the first free slot is used
   node  - NUMA node id [optional] (default: 0)
   size  - amount of memory to add, readonly derived from backing memdev
   addr - guest's physical address where to plug DIMM [optional],
   if not specified the first gap in hotplug memory region
   that fits DIMM is used

  -device option could be used for adding potentially hotunplugable DIMMs
 and also for specifying hotplugged DIMMs in migration case.


So this has some themes in common with my MemoryRegion QOMification
series. This series does a lot to facilitate linking and exposing
memory regions between QOM objects which was one of my goals. Series
here:

https://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg05374.html
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03265.html

That said, your on V10 and your patches are pretty close IMO. So i'd
rather see it go through and tweak the DIMM/Membackend stuff to use
QOMified MRs as follow up. I've made a few comments on the Memoryish
bits of the series but Ive glossed over the PC/ACPI specific stuffs.
General approach WRT QOM and the linkages and user interface looks
good to me. The busless machine level hotplug idea looks good.

So for all uncommented patches,

Acked-by: Peter Crosthwaite peter.crostwa...@xilinx.com

 Tested guests:
  - RHEL 6x64
  - Windows 2012DCx64
  - Windows 2008DCx64

 Known limitations/bugs/TODOs:
  - hot-remove is not supported, yet
  - max number of supported DIMM devices 256 (due to ACPI object name
limit), could be increased creating several containers and putting
DIMMs there. (exercise for future)
  - e820 table doesn't include DIMM devices added with -device /
(or after reboot devices added with device_add)
  - Windows 2008 remembers DIMM configuration, so if DIMM with other
addr/size is added into the same slot, it refuses to use it insisting
on old mapping.

 QEMU git tree for testing is available at:
   https://github.com/imammedo/qemu/commits/memory-hotplug-v10

 Example QEMU cmd line:
   qemu-system-x86_64 -enable-kvm -monitor unix:/tmp/mon,server,nowait \
  -m 4096,slots=4,maxmem=8G guest.img

 PS:
   Windows guest requires SRAT table for hotplug to work so add an extra 
 option:
-numa 

Re: [Qemu-devel] [PATCH v2] vexpress: Add support for the -bios flag to provide firmware

[Laszlo Ersek]

CC'ing Drew, Eric and Michal.

On 05/29/14 14:45, Peter Crosthwaite wrote:
 On Thu, May 29, 2014 at 10:24 PM, Peter Maydell
 peter.mayd...@linaro.org wrote:
 From: Grant Likely grant.lik...@linaro.org

 Right now to run firmware inside the QEMU VExpress model requires
 padding out the firmware image to the size of the virtual flash and
 passing it in via the -pflash argument. If the firmware image is passed
 without padding, then QEMU will fail. Also, when passed as a -pflash
 argument, QEMU treats the file as persistent storage and will modify the
 file.

 
 A little out of scope, but no support for read-only backing images is
 somewhat annoying. Is it feasible to patch the block layer (or perhaps
 we need to do something to pflash?) to handle a read-only -pflash file
 as init-only data (and then use the normal volatile ram-based storage
 once the file is loaded)?
 
 Then the semantics of don't change my ROM file even if the real
 hardware can do it in real life can just be handled with file perms.
 
 The -bios flag provides the semantics that we want for providing a
 firmware image. This patch maps the contents of the -bios file into the
 address space at the boot flash location.

 Tested with the vexpress-a15 model and the Tianocore port.

 Signed-off-by: Grant Likely grant.lik...@linaro.org
 Tested-by: Roy Franz roy.fr...@linaro.org
 [PMM: folded long line, removed stray \n from error message,
  use correct variable for printing image name, exit(1) rather than 0]
 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
 Grant submitted v1 of this a few months back; it was pretty nearly
 correct, so I've just tidied up the loose ends so we can get it into
 QEMU 2.1. Tested by booting the UEFI blob from
 https://wiki.linaro.org/LEG/Engineering/Kernel/UEFI/VersatileExpress/QEMU

  hw/arm/vexpress.c | 13 +
  1 file changed, 13 insertions(+)

 diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
 index 33ff422..0f8f175 100644
 --- a/hw/arm/vexpress.c
 +++ b/hw/arm/vexpress.c
 @@ -28,6 +28,7 @@
  #include net/net.h
  #include sysemu/sysemu.h
  #include hw/boards.h
 +#include hw/loader.h
  #include exec/address-spaces.h
  #include sysemu/blockdev.h
  #include hw/block/flash.h
 @@ -528,6 +529,18 @@ static void vexpress_common_init(VEDBoardInfo 
 *daughterboard,
  daughterboard-init(daughterboard, machine-ram_size, 
 machine-cpu_model,
  pic);

 +/*
 + * If a bios file was provided, attempt to map it into memory
 + */
 +if (bios_name) {
 +const char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 +if (!fn || load_image_targphys(fn, map[VE_NORFLASH0],
 +   VEXPRESS_FLASH_SIZE)  0) {
 +error_report(Could not load rom image '%s', bios_name);
 
 ROM
 
 Otherwise
 
 Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
 
 Regards,
 Peter
 
 +exit(1);
 +}
 +}
 +
  /* Motherboard peripherals: the wiring is the same but the
   * addresses vary between the legacy and A-Series memory maps.
   */
 --
 1.9.2


 

OK, big chaos in my mind now, because I don't know enough about ARM, so
I'll just shoot out some thoughts and questions.

(1) The pflash interface perfectly well supports read-only backing files
(or read-write backing files opened in read-only mode), and the flash
programming interface does communicate that to the guest. Please search
hw/block/pflash_cfi01.c for bdrv_is_read_only(), and follow the code
from there.

One just needs to use the

  -drive file=img,if=pflash,format=raw,readonly

option rather than the -pflash shorthand. Please refer to qemu commit
637a5acb (hw/i386/pc_sysfw: support two flash drives).

(2) Can anyone post / link a concise, *up-to-date* command line for
upstream qemu, in connection with *upstream* edk2 build instructions, so
that I can test aarch64 UEFI stuff on an x86_64 host (with TCG)?

What DSC platform is built in edk2? What qemu machine type is used? If
the needed edk2 changes are downstream only, what branch of what repo
should I look at?

The last info I have on this is that ArmPlatformPkg intended to grow a
dedicated platform (a DSC) for qemu's -M virt, because Olivier didn't
like the vexpress DSCs being reused for virtual machines.

(Side note: I had posted a simple virtio-mmio transport enumeration
patchset (suiting -M virt) to edk2-devel, for ArmFvpDxe. The goal was to
auto-detect all virtio (block, net, scsi) devices of qemu's -M virt.
Olivier seemed to approve, but didn't apply the relevant part of the set
because of this pending split of platforms (DSCs). Did that go anywhere?)

(3) I'm interested in the qemu command line because I'd like to see how
the UEFI variable store is differentiated from the flash device that
contains the executable (potentially post-decompression executable)
firmware code.

In *upstream* OVMF the variable store and the main fw image are built
into the same FD file. I have a 

Re: [Qemu-devel] [PATCH v3 05/34] add memdev backend infrastructure

[Paolo Bonzini]

Il 29/05/2014 16:05, Peter Crosthwaite ha scritto:

 +path = object_get_canonical_path_component(OBJECT(backend));
 +memory_region_init_ram(backend-mr, OBJECT(backend), path,

Passing the full canonical path as the name of memory region is
redundant as that information is already passed via the owner
argument. It should just be a shorthand.



It's not the full canonical path, it's basically the id.

Paolo

Re: [Qemu-devel] [PATCH v2] vexpress: Add support for the -bios flag to provide firmware

[Peter Crosthwaite]

On Fri, May 30, 2014 at 12:46 AM, Laszlo Ersek ler...@redhat.com wrote:
 CC'ing Drew, Eric and Michal.

 On 05/29/14 14:45, Peter Crosthwaite wrote:
 On Thu, May 29, 2014 at 10:24 PM, Peter Maydell
 peter.mayd...@linaro.org wrote:
 From: Grant Likely grant.lik...@linaro.org

 Right now to run firmware inside the QEMU VExpress model requires
 padding out the firmware image to the size of the virtual flash and
 passing it in via the -pflash argument. If the firmware image is passed
 without padding, then QEMU will fail. Also, when passed as a -pflash
 argument, QEMU treats the file as persistent storage and will modify the
 file.


 A little out of scope, but no support for read-only backing images is
 somewhat annoying. Is it feasible to patch the block layer (or perhaps
 we need to do something to pflash?) to handle a read-only -pflash file
 as init-only data (and then use the normal volatile ram-based storage
 once the file is loaded)?

 Then the semantics of don't change my ROM file even if the real
 hardware can do it in real life can just be handled with file perms.

 The -bios flag provides the semantics that we want for providing a
 firmware image. This patch maps the contents of the -bios file into the
 address space at the boot flash location.

 Tested with the vexpress-a15 model and the Tianocore port.

 Signed-off-by: Grant Likely grant.lik...@linaro.org
 Tested-by: Roy Franz roy.fr...@linaro.org
 [PMM: folded long line, removed stray \n from error message,
  use correct variable for printing image name, exit(1) rather than 0]
 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
 Grant submitted v1 of this a few months back; it was pretty nearly
 correct, so I've just tidied up the loose ends so we can get it into
 QEMU 2.1. Tested by booting the UEFI blob from
 https://wiki.linaro.org/LEG/Engineering/Kernel/UEFI/VersatileExpress/QEMU

  hw/arm/vexpress.c | 13 +
  1 file changed, 13 insertions(+)

 diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
 index 33ff422..0f8f175 100644
 --- a/hw/arm/vexpress.c
 +++ b/hw/arm/vexpress.c
 @@ -28,6 +28,7 @@
  #include net/net.h
  #include sysemu/sysemu.h
  #include hw/boards.h
 +#include hw/loader.h
  #include exec/address-spaces.h
  #include sysemu/blockdev.h
  #include hw/block/flash.h
 @@ -528,6 +529,18 @@ static void vexpress_common_init(VEDBoardInfo 
 *daughterboard,
  daughterboard-init(daughterboard, machine-ram_size, 
 machine-cpu_model,
  pic);

 +/*
 + * If a bios file was provided, attempt to map it into memory
 + */
 +if (bios_name) {
 +const char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 +if (!fn || load_image_targphys(fn, map[VE_NORFLASH0],
 +   VEXPRESS_FLASH_SIZE)  0) {
 +error_report(Could not load rom image '%s', bios_name);

 ROM

 Otherwise

 Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com

 Regards,
 Peter

 +exit(1);
 +}
 +}
 +
  /* Motherboard peripherals: the wiring is the same but the
   * addresses vary between the legacy and A-Series memory maps.
   */
 --
 1.9.2




 OK, big chaos in my mind now, because I don't know enough about ARM, so
 I'll just shoot out some thoughts and questions.

 (1) The pflash interface perfectly well supports read-only backing files
 (or read-write backing files opened in read-only mode), and the flash
 programming interface does communicate that to the guest.

Well that would be a different semantic again:). Changing the guest
visible machine state based on backing store may actually be unwanted.
If read only mode is selected for the -drive then the pflash is
unwritable for the lifetime of the system. This is essentially saying
that the pflash solely exists for read-only firmware, which is not the
case in many embedded platforms (we have systems where the full image
stack as well as user disk storage is on one pflash).

A useful case, is to blob in some firmware at reset or machine
creation (from a read-only file) but the the actual device presents as
guest-writeable, backing onto RAM. This means you can blob in a image,
but then have a guest use the device as read/write storage.

Regards,
Peter

Re: [Qemu-devel] [PATCH v3 11/34] memory: add memory_region_is_mapped() API

[Paolo Bonzini]

Il 29/05/2014 16:22, Peter Crosthwaite ha scritto:


 +bool memory_region_is_mapped(MemoryRegion *mr)
 +{

Is it not enough to just return mr-parent? Memory mapping assertion
will happen if you try and map the same twice, even if one of the
mappings is not contained within an AddressSpace. Checking for just
the parent mr may be a simpler and more accurate check.


I can see a difference if the memory region is completely overlapped by 
a higher-priority one.  Igor, what was your idea with this function?


Paolo

Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Peter Maydell]

On 29 May 2014 15:29, James Hogan james.ho...@imgtec.com wrote:
 On 29/05/14 14:26, Peter Maydell wrote:
 On 29 May 2014 14:22, James Hogan james.ho...@imgtec.com wrote:
 loadvm/migration will write Config3, possibly with a different value if
 the snapshot being loaded is an older one without userlocal support.

 Migration load always happens with a freshly reset system,
 so this isn't a problem.

 Why does that make a difference? The snapshot may have been saved on a
 slightly different version without that bit set (if that isn't supported
 then we may as well not bother trying to support loading different
 snapshot versions).

A key requirement of migration or vm restore is that the
machine configuration (including all command line parameters)
must be exactly identical at both ends. (We don't attempt to
catch this user error, which typically results in weird stuff
happening or in a migration failure with an obscure message).
So you'll never get a migration from a bit-set config to a
bit-cleared config. Note that this is different from a cross
version migration, which would be from QEMU 2.0 with config-X
to QEMU 2.1 with config-X.

 Also loadvm can happen at any time, not just after
 reset.

The loadvm monitor command is implemented as first reset;
then load state.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] vexpress: Add support for the -bios flag to provide firmware

[Peter Maydell]

On 29 May 2014 15:46, Laszlo Ersek ler...@redhat.com wrote:
 (2) Can anyone post / link a concise, *up-to-date* command line for
 upstream qemu, in connection with *upstream* edk2 build instructions, so
 that I can test aarch64 UEFI stuff on an x86_64 host (with TCG)?

Note that AArch64 isn't relevant to this patch, because
this patch is for the vexpress models, which are AArch32 only.

AArch64 UEFI-on-QEMU stuff is very much work in progress as
I understand it and needs hacks to all of UEFI, QEMU and
the guest kernel. You could probably try starting with
https://wiki.linaro.org/LEG/UEFIforQEMU

 What DSC platform is built in edk2? What qemu machine type is used? If
 the needed edk2 changes are downstream only, what branch of what repo
 should I look at?

 The last info I have on this is that ArmPlatformPkg intended to grow a
 dedicated platform (a DSC) for qemu's -M virt, because Olivier didn't
 like the vexpress DSCs being reused for virtual machines.

This sort of question is probably better asked on a UEFI
mailing list...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 05/34] add memdev backend infrastructure

[Paolo Bonzini]

Il 29/05/2014 17:39, Peter Crosthwaite ha scritto:

 It's not the full canonical path, it's basically the id.


Fair enough. Although its still redundant though isn't it?

Should we ever clean up messages etc in memory api to include both
owner and name this is going to come out:

foo/bar/baz:baz

or some such.


Yeah.  But it's not exposed in any way, we can change it later.

Paolo


Then again, if we are only worried about getting a sane shorthand,
perhaps what Igor has done here is a sane default for the memory API
when passed a NULL name?

Re: [Qemu-devel] [PATCH v3 05/34] add memdev backend infrastructure

[Peter Crosthwaite]

On Fri, May 30, 2014 at 1:25 AM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 29/05/2014 16:05, Peter Crosthwaite ha scritto:

  +path = object_get_canonical_path_component(OBJECT(backend));
  +memory_region_init_ram(backend-mr, OBJECT(backend), path,

 Passing the full canonical path as the name of memory region is
 redundant as that information is already passed via the owner
 argument. It should just be a shorthand.


 It's not the full canonical path, it's basically the id.


Fair enough. Although its still redundant though isn't it?

Should we ever clean up messages etc in memory api to include both
owner and name this is going to come out:

foo/bar/baz:baz

or some such.

Then again, if we are only worried about getting a sane shorthand,
perhaps what Igor has done here is a sane default for the memory API
when passed a NULL name?

Regards,
Peter

 Paolo

Re: [Qemu-devel] [PATCH] machine: set default kernel_cmdline in machine_initfn()

[Paolo Bonzini]

Il 29/05/2014 15:26, Andreas Färber ha scritto:

Am 29.05.2014 15:21, schrieb Marcel Apfelbaum:

On Thu, 2014-05-29 at 15:08 +0200, Igor Mammedov wrote:

On Thu, 29 May 2014 15:56:16 +0300
Marcel Apfelbaum marce...@redhat.com wrote:

On Thu, 2014-05-29 at 14:40 +0200, Igor Mammedov wrote:

Maybe we can hack QemuOpts to do s/foo_moo/foo-moo/ ?
Than there won't be need for an explicit conversion and it could be reused by 
others,
including X86CPU.

I like this idea, but this can't be generic for all QemuOpts right? Do you
mean adding a flag specifying we want this conversion?

Flag would be less risky than doing it for all QemuOpts unconditionally.
It would open road for conversion per a QemuOpts which probably should be
done eventually if options are translated into properties.

Sounds fine, I can go for it.
Andreas, do you agree?


Whatever works to get a sane QOM ABI without bad side-effects.


What exactly is part of the API/ABI?  Is any tool actually using 
qom-list/qom-get/qom-set?  Because if not, it's pointless to stick to a 
well-defined API/ABI.


In any case, I guess we could just s/_/-/g in object_set_property?

Paolo