Re: [Qemu-devel] [PULL 00/15] Misc patches for 2015-01-14

2015-01-14 Thread Peter Maydell
On 14 January 2015 at 09:41, Paolo Bonzini pbonz...@redhat.com wrote:
 The following changes since commit f1c5831ca3e3eafb89331233221768b64db113e8:

   Merge remote-tracking branch 'remotes/amit-virtio-rng/tags/rng-for-2.3' 
 into staging (2015-01-09 18:55:29 +)

 are available in the git repository at:


   git://github.com/bonzini/qemu.git tags/for-upstream

 for you to fetch changes up to 71f35e66ce76dadd034cd463556be5e1a8230245:

   cpus: consistently use QEMU_CLOCK_VIRTUAL_RT for icount_warp_rt timer 
 (2015-01-14 10:16:12 +0100)

 
 Mostly bugfixes and cleanups from qemu-devel.  Yet another small patch from
 the record/replay series, and a few SCSI and i386 patches as well.

 

Applied, thanks.

-- PMM



[Qemu-devel] state of micro-checkpointing in qemu

2015-01-14 Thread guillaume LE LOUËT
 Hello.

Mr Hines implemented a version of micro-checkpointing (MC), known as
High-Availability(HA) in VMWare.

The implementation is in the mc branch of the git repo
http://github.com/hinesmr/qemu.git, described at
http://wiki.qemu.org/Features/MicroCheckpointing

My question is : will qemu support this feature ? I assume no, because
there has been no activity on this topic for months now.
Could someone tell me why or lead me to an explaination ?

I compiled it (after an error because libnl change some public methods),
and it seems to work to me, though not when the connection is lost due to
hardware (we have to close the application, not to crash it : ctrl-z will
not make the VM migrate)

Regards, Guillaume.


Re: [Qemu-devel] [PATCH] Fixes several full screen issues on Mac OS X

2015-01-14 Thread Programmingkid

On Jan 14, 2015, at 1:29 PM, Peter Maydell wrote:

 On 14 January 2015 at 18:18, Programmingkid programmingk...@gmail.com wrote:
 
 On Jan 14, 2015, at 12:19 PM, Peter Maydell wrote:
 (2) Having done this I find that all my other application windows
 have been squashed down into a corner of my screen, presumably because
 we've told MacOSX the screen is 640x480 and it's rearranged the
 app windows to suit. We mustn't mess things up like this.
 
 This can't be avoided. When the screen resolution changes, all the
 applications usually adjust to the new size. It is the same thing
 that happens when you use a full screen game.
 
 No, when I use other full screen programs this doesn't happen at all.
 QEMU with this patch is the first time I've ever seen this from
 any OSX app.

How long have you been a Mac user? I am shocked you have never seen this 
before. I'm guessing full screen gaming was never your thing. 

 
 (3) I managed to get at the underlying QEMU window with its title
 bar somehow even when in full screen mode: I could move it about the
 screen with the mouse...
 
 Really? I think you said you had Mac OS 10.7. I don't have that, but
 I do have access to Mac OS 10.9. Just send me the instructions on how
 to reproduce this.
 
 I run 10.9.5. I don't know exactly how I got to that window,
 and I don't really want to mess about with this patch because
 behaviour (2) above is so obnoxious.
 
 (4) I get a lot of compile warnings for this patch:
 
 
 Disabling the depreciation warning would eliminate these errors.
 
 Not all of them are deprecation warnings. Also I would prefer not
 to disable deprecation warnings, as then we'll have no notice
 of what might break on future OSX versions.

I'm sure we will know exactly when a depreciated function isn't implemented 
anymore. We will probably see some message in the Console. Or a backtrace to a 
crash will show us where and when QEMU crashed. I guess I could also use 
non-depreciated functions in the patch. The thing is QEMU will determine what 
to use at runtime, so the depreciation messages might not go away. 

 
int w = surface_width(surface);
int h = surface_height(surface);
 -/* cdx == 0 means this is our very first surface, in which case we 
 need
 - * to recalculate the content dimensions even if it happens to be the 
 size
 - * of the initial empty window.
 - */
 -bool isResize = (w != screen.width || h != screen.height || cdx == 
 0.0);
 -
 +bool isResize = (w != screen.width || h != screen.height);
 
 (6) This looks like you've just dropped a bug fix. How are you
 dealing with this case if not by the method described in the
 now-deleted comment?
 
 If the guest does change its resolution, then we try to match it
 in the host. When I eliminated this code, it made the guest look
 so much better. I was actually able to read documents in the guest
 at full screen.
 
 The point is that you've dropped a bugfix which isn't related to
 full screen at all -- if this is the first call to switchSurface
 we *must* display it, which is what the cdx check does. See
 commit 381600dad.

This bugfix does change the appearance of full screen mode. Have you seen a 
guest at full screen? The cdx and cdy variables make the guest OS look very 
distorted. 




Re: [Qemu-devel] [PATCH] Machine menu patch for Mac OS X

2015-01-14 Thread Programmingkid

On Jan 14, 2015, at 1:42 PM, Peter Maydell wrote:

 On 14 January 2015 at 18:34, Programmingkid programmingk...@gmail.com wrote:
 
 On Jan 14, 2015, at 12:42 PM, Peter Maydell wrote:
 
 On 13 January 2015 at 01:49, Programmingkid programmingk...@gmail.com 
 wrote:
 This patch adds a Machine menu to QEMU. This menu gives the user the 
 ability to easily work with floppy and CD image files.
 
 Features:
 Menu items to switch floppy and CD image files.
 Menu items to eject floppy and CD image files.
 Menu item to use /dev/cdrom.
 Verifies with the user before quitting QEMU by displaying a dialog box.
 
 Signed-off-by: John Arbuckle programmingk...@gmail.com
 
 Hi. I'm afraid I couldn't get this patch to apply -- is it dependent
 on one of your other patches?
 
 I have been making a lot of changes to cocoa.m. Can you suggest a way
 to make these changes not interfere with each other?
 
 It can be hard to avoid in that kind of situation. Sometimes the
 best you can do is just to say which patches it depends on.

That sounds like a good idea.

 
 
 Isn't this changing of the title string going to conflict with the
 changes that we make for mouse grab/ungrab?
 
 Unfortunately yes, but I still need a way to alert the user that QEMU
 is paused. I guess I could find a way to make both messages show up
 in the titlebar. Or I could display a message in the main window that
 says Paused. Will come up with something soon.
 
 ui/gtk.c has a 'gd_update_caption()' function which sets the title
 string as appropriate for the current situation; that is then called
 from the places which change the UI state. That's probably as good
 an approach as any.

I was thinking about displaying the word Paused behind the darked QEMU 
window. The Paused will be in big letters - maybe in a red color. 

 
 You don't know that the machine being emulated has a floppy drive
 at all, or that it's called floppy0...
 
 I did only use the PC and Mac targets. I know there are others. It looks
 like conditionally adding some of these menu items will have to do. I
 guess detecting if the guest machine has a floppy and/or cdrom drive can
 be done - hopefully...
 
 It's not impossible. But none of the other UIs are trying to do that
 kind of thing yet; I'd suggest postponing those parts. Bringing the
 OSX UI into line with features already implementing in another UI
 is simpler than adding features no other UI has, because you're not
 trying to break new ground.

I think innovation is exactly what QEMU needs. If everyone is waiting for 
someone else to make the first move, we all remain stuck. 

 
 In the GTK UI we call this Reset. We also have a Power Down which would
 probably be nice for consistency.
 
 Changing Restart to Reset will be done. I don't know how to
 implement Power Down, so I'm not sure about that one.
 
 It's easy: just call qmp_system_powerdown(NULL).

Ok, is that something that tells the guest operating system it is to shut down?

 
 Do you think anyone will ever make SDL or GTK UIs for QEMU work
 on Mac OS X?
 
 Hard to say. I won't, because I don't have the GTK dependencies
 available to me. And there don't really seem to be any other
 developers trying to work with QEMU on OSX except you.
 It should be possible in theory to get SDL or GTK UIs working,
 but you'd have to fix up places where the cocoa UI assumes
 it's always present, I expect.

Ok, which one is better - SDL, or GTK? If someone ever decided to port one of 
these UIs to Mac OS X, which one should that person work on?





[Qemu-devel] [Bug 1292234] Re: qcow2 image corruption in trusty (qemu 1.7 and 2.0 candidate)

2015-01-14 Thread Chris J Arges
FWIW, just re-reproduced this with latest upstream kernel / qemu / fresh
qcow2 image.

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

Title:
  qcow2 image corruption in trusty (qemu 1.7 and 2.0 candidate)

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  In Progress

Bug description:
  The security team uses a tool (http://bazaar.launchpad.net/~ubuntu-
  bugcontrol/ubuntu-qa-tools/master/view/head:/vm-tools/uvt) that uses
  libvirt snapshots quite a bit. I noticed after upgrading to trusty
  some time ago that qemu 1.7 (and the qemu 2.0 in the candidate ppa)
  has had stability problems such that the disk/partition table seems to
  be corrupted after removing a libvirt snapshot and then creating
  another with the same name. I don't have a very simple reproducer, but
  had enough that hallyn suggested I file a bug. First off:

  qemu-kvm 2.0~git-20140307.4c288ac-0ubuntu2

  $ cat /proc/version_signature
  Ubuntu 3.13.0-16.36-generic 3.13.5

  $ qemu-img info ./forhallyn-trusty-amd64.img
  image: ./forhallyn-trusty-amd64.img
  file format: qcow2
  virtual size: 8.0G (8589934592 bytes)
  disk size: 4.0G
  cluster_size: 65536
  Format specific information:
  compat: 0.10

  Steps to reproduce:
  1. create a virtual machine. For a simplified reproducer, I used virt-manager 
with:
    OS type: Linux
    Version: Ubuntu 14.04
    Memory: 768
    CPUs: 1

    Select managed or existing (Browse, new volume)
  Create a new storage volume:
    qcow2
    Max capacity: 8192
    Allocation: 0

    Advanced:
  NAT
  kvm
  x86_64
  firmware: default

  2. install a VM. I used trusty-desktop-amd64.iso from Jan 23 since it
  seems like I can hit the bug more reliably if I have lots of updates
  in a dist-upgrade. I have seen this with lucid-trusty guests that are
  i386 and amd64. After the install, reboot and then cleanly shutdown

  3. Backup the image file somewhere since steps 1 and 2 take a while :)

  4. Execute the following commands which are based on what our uvt tool
  does:

  $ virsh snapshot-create-as forhallyn-trusty-amd64 pristine uvt snapshot
  $ virsh snapshot-current --name forhallyn-trusty-amd64
  pristine
  $ virsh start forhallyn-trusty-amd64
  $ virsh snapshot-list forhallyn-trusty-amd64 # this is showing as shutoff 
after start, this might be different with qemu 1.5

  in guest:
  sudo apt-get update
  sudo apt-get dist-upgrade
  780 upgraded...
  shutdown -h now

  $ virsh snapshot-delete forhallyn-trusty-amd64 pristine --children
  $ virsh snapshot-create-as forhallyn-trusty-amd64 pristine uvt snapshot

  $ virsh start forhallyn-trusty-amd64 # this command works, but there
  is often disk corruption

  The idea behind the above is to create a new VM with a pristine
  snapshot that we could revert later if we wanted. Instead, we boot the
  VM, run apt-get dist-upgrade, cleanly shutdown and then remove the old
  'pristine' snapshot and create a new 'pristine' snapshot. The
  intention is to update the VM and the pristine snapshot so that when
  we boot the next time, we boot from the updated VM and can revert back
  to the updated VM.

  After running 'virsh start' after doing snapshot-delete/snapshot-
  create-as, the disk may be corrupted. This can be seen with grub
  failing to find .mod files, the kernel not booting, init failing, etc.

  This does not seem to be related to the machine type used. Ie, pc-
  i440fx-1.5, pc-i440fx-1.7 and pc-i440fx-2.0 all fail with qemu 2.0,
  pc-i440fx-1.5 and pc-i440fx-1.7 fail with qemu 1.7 and pc-i440fx-1.5
  works fine with qemu 1.5.

  Only workaround I know if is to downgrade qemu to 1.5.0+dfsg-
  3ubuntu5.4 from Ubuntu 13.10.

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



Re: [Qemu-devel] [PATCH v4 37/47] Page request: Consume pages off the post-copy queue

2015-01-14 Thread Dr. David Alan Gilbert
* David Gibson (da...@gibson.dropbear.id.au) wrote:
 On Fri, Oct 03, 2014 at 06:47:43PM +0100, Dr. David Alan Gilbert (git) wrote:
  From: Dr. David Alan Gilbert dgilb...@redhat.com
  
  When transmitting RAM pages, consume pages that have been queued by
  MIG_RPCOMM_REQPAGE commands and send them ahead of normal page scanning.
  
  Note:
a) After a queued page the linear walk carries on from after the
  unqueued page; there is a reasonable chance that the destination
  was about to ask for other closeby pages anyway.
  
b) We have to be careful of any assumptions that the page walking
  code makes, in particular it does some short cuts on its first linear
  walk that break as soon as we do a queued page.
  
  Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
  ---
   arch_init.c | 149 
  ++--
   1 file changed, 125 insertions(+), 24 deletions(-)
  
  diff --git a/arch_init.c b/arch_init.c
  index 72f9e17..a945990 100644
  +
  +/*
  + * Don't break host-page chunks up with queue items
 
 Why does this matter?

See the comment you make in a few patches time, it's about being able
to place the pages atomically on the destination.

  + * so only unqueue if,
  + *   a) The last item came from the queue anyway
  + *   b) The last sent item was the last target-page in a host page
  + */
  +if (last_was_from_queue || (!last_sent_block) ||
  +((last_offset  (hps - 1)) == (hps - TARGET_PAGE_SIZE))) {
  +tmpblock = ram_save_unqueue_page(ms, tmpoffset, bitoffset);
   }
  -if (offset = block-length) {
  -offset = 0;
  -block = QTAILQ_NEXT(block, next);
  -if (!block) {
  -block = QTAILQ_FIRST(ram_list.blocks);
  -complete_round = true;
  -ram_bulk_stage = false;
  +
  +if (tmpblock) {
  +/* We've got a block from the postcopy queue */
  +DPRINTF(%s: Got postcopy item '%s' offset=%zx bitoffset=%zx,
  +__func__, tmpblock-idstr, tmpoffset, bitoffset);
  +/* We're sending this page, and since it's postcopy nothing 
  else
  + * will dirty it, and we must make sure it doesn't get sent 
  again.
  + */
  +if (!migration_bitmap_clear_dirty(bitoffset  
  TARGET_PAGE_BITS)) {
 
 Ugh.. that's kind of subtle.  I think it would be clearer if you work
 in terms of a ram_addr_t throughout, rather than bitoffset whose
 meaning is not terribly obvious.

I've changed it to ram_addr_t as requested; it's slightly clearer but there
are a few places where we're dealing with the sentmap where we now need to shift
the other way.  In the end ram_addr_t is really a scaled offset into those
bitmaps.

Dave

 
 -- 
 David Gibson  | I'll have my music baroque, and my code
 david AT gibson.dropbear.id.au| minimalist, thank you.  NOT _the_ 
 _other_
   | _way_ _around_!
 http://www.ozlabs.org/~dgibson


--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH] Fixes several full screen issues on Mac OS X

2015-01-14 Thread Peter Maydell
On 14 January 2015 at 18:59, Programmingkid programmingk...@gmail.com wrote:

 On Jan 14, 2015, at 1:29 PM, Peter Maydell wrote:

 On 14 January 2015 at 18:18, Programmingkid programmingk...@gmail.com 
 wrote:

 On Jan 14, 2015, at 12:19 PM, Peter Maydell wrote:
 (2) Having done this I find that all my other application windows
 have been squashed down into a corner of my screen, presumably because
 we've told MacOSX the screen is 640x480 and it's rearranged the
 app windows to suit. We mustn't mess things up like this.

 This can't be avoided. When the screen resolution changes, all the
 applications usually adjust to the new size. It is the same thing
 that happens when you use a full screen game.

 No, when I use other full screen programs this doesn't happen at all.
 QEMU with this patch is the first time I've ever seen this from
 any OSX app.

 How long have you been a Mac user? I am shocked you have never seen
 this before. I'm guessing full screen gaming was never your thing.

Three years or so. I don't do much full screen stuff but I have
played a few games that fullscreen. In any case I think the
behaviour is definitely not something we can accept.

 The point is that you've dropped a bugfix which isn't related to
 full screen at all -- if this is the first call to switchSurface
 we *must* display it, which is what the cdx check does. See
 commit 381600dad.

 This bugfix does change the appearance of full screen mode.
 Have you seen a guest at full screen? The cdx and cdy variables
 make the guest OS look very distorted.

For me with current trunk the fullscreen is just usual display,
not expanded at all, as only thing on the screen. Regardless,
this code your patch deletes was added *to fix a bug* and you
cannot simply delete it because it happens to get in the way of
a different fix you want to make. You must come up with something
that fixes both problems.

-- PMM



Re: [Qemu-devel] [PATCH v15 2/2] sPAPR: Implement sPAPRPHBClass::eeh_handler

2015-01-14 Thread Gavin Shan
On Wed, Jan 14, 2015 at 12:41:59PM +1100, David Gibson wrote:
On Mon, Jan 05, 2015 at 11:26:28AM +1100, Gavin Shan wrote:
 The patch implements sPAPRPHBClass::eeh_handler so that the
 EEH RTAS requests can be routed to VFIO for further handling.
 
 Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
 ---
  hw/ppc/spapr_pci_vfio.c | 56 
 +
  hw/vfio/common.c|  1 +
  2 files changed, 57 insertions(+)
 
 diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
 index 144912b..73652a9 100644
 --- a/hw/ppc/spapr_pci_vfio.c
 +++ b/hw/ppc/spapr_pci_vfio.c
 @@ -71,6 +71,61 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState 
 *sphb, Error **errp)
  spapr_tce_get_iommu(tcet));
  }
  
 +static int spapr_phb_vfio_eeh_handler(sPAPRPHBState *sphb, int req, int opt)
 +{
 +sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
 +struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };

This is a local variable, which means it won't be initialized.  You
never memset() it and it's not obvious that all fields get
initialized, which makes it dangerous to pass to an ioctl().


Yeah, we might get uncertain flags passed to ioctl(). I'll change the
code like this if you agree:

struct vfio_eeh_pe_op op;

memset(op, 0, sizeof(op));
op.argsz = sizeof(op);

 +int cmd;
 +
 +switch (req) {
 +case RTAS_EEH_REQ_SET_OPTION:
 +switch (opt) {
 +case RTAS_EEH_DISABLE:
 +cmd = VFIO_EEH_PE_DISABLE;
 +break;
 +case RTAS_EEH_ENABLE:
 +cmd = VFIO_EEH_PE_ENABLE;
 +break;
 +case RTAS_EEH_THAW_IO:
 +cmd = VFIO_EEH_PE_UNFREEZE_IO;
 +break;
 +case RTAS_EEH_THAW_DMA:
 +cmd = VFIO_EEH_PE_UNFREEZE_DMA;
 +break;
 +default:
 +return -EINVAL;
 +}
 +break;
 +case RTAS_EEH_REQ_GET_STATE:
 +cmd = VFIO_EEH_PE_GET_STATE;
 +break;
 +case RTAS_EEH_REQ_RESET:
 +switch (opt) {
 +case RTAS_SLOT_RESET_DEACTIVATE:
 +cmd = VFIO_EEH_PE_RESET_DEACTIVATE;
 +break;
 +case RTAS_SLOT_RESET_HOT:
 +cmd = VFIO_EEH_PE_RESET_HOT;
 +break;
 +case RTAS_SLOT_RESET_FUNDAMENTAL:
 +cmd = VFIO_EEH_PE_RESET_FUNDAMENTAL;
 +break;
 +default:
 +return -EINVAL;
 +}
 +break;
 +case RTAS_EEH_REQ_CONFIGURE:
 +cmd = VFIO_EEH_PE_CONFIGURE;
 +break;
 +default:
 + return -EINVAL;
 +}
 +
 +op.op = cmd;
 +return vfio_container_ioctl(svphb-phb.iommu_as, svphb-iommugroupid,
 +VFIO_EEH_PE_OP, op);

Don't you need some sort of translation from the errnos the ioctl()
returns into RTAS error codes?


Currently, errors from ioctl() is simply translated to RTAS_OUT_HW_ERROR if
the RTAS call supports it. Otherwise, RTAS_OUT_PARAM_ERROR will be returned.

Thanks,
Gavin

 +}
 +
  static void spapr_phb_vfio_reset(DeviceState *qdev)
  {
  /* Do nothing */
 @@ -84,6 +139,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, 
 void *data)
  dc-props = spapr_phb_vfio_properties;
  dc-reset = spapr_phb_vfio_reset;
  spc-finish_realize = spapr_phb_vfio_finish_realize;
 +spc-eeh_handler = spapr_phb_vfio_eeh_handler;
  }
  
  static const TypeInfo spapr_phb_vfio_info = {
 diff --git a/hw/vfio/common.c b/hw/vfio/common.c
 index cf483ff..8a10c8b 100644
 --- a/hw/vfio/common.c
 +++ b/hw/vfio/common.c
 @@ -948,6 +948,7 @@ int vfio_container_ioctl(AddressSpace *as, int32_t 
 groupid,
  switch (req) {
  case VFIO_CHECK_EXTENSION:
  case VFIO_IOMMU_SPAPR_TCE_GET_INFO:
 +case VFIO_EEH_PE_OP:
  break;
  default:
  /* Return an error on unknown requests */

-- 
David Gibson   | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you.  NOT _the_ _other_
   | _way_ _around_!
http://www.ozlabs.org/~dgibson





Re: [Qemu-devel] Fedora FC21 - Bug: 100% CPU and hangs in gettimeofday(tp, NULL); forever

2015-01-14 Thread Gerhard Wiesinger

On 14.01.2015 18:52, Juan Quintela wrote:

Juan Quintela quint...@redhat.com wrote:
I forgot tell on the previous patch, I am using 2vcpus. with a single 
vcpu I have been unable to trigger this bug. 


There is already a fix with a new patched kernel available for the 
guest, see the bugzilla entry and my posts in this thread.


Ciao,
Gerhard




Re: [Qemu-devel] [PATCH v15 1/2] sPAPR: Implement EEH RTAS calls

2015-01-14 Thread Gavin Shan
On Wed, Jan 14, 2015 at 12:39:35PM +1100, David Gibson wrote:
On Mon, Jan 05, 2015 at 11:26:27AM +1100, Gavin Shan wrote:
 The emulation for EEH RTAS requests from guest isn't covered
 by QEMU yet and the patch implements them.
 
 The patch defines constants used by EEH RTAS calls and adds
 callback sPAPRPHBClass::eeh_handler, which is going to be used
 this way:
 
   * RTAS calls are received in spapr_pci.c, sanity check is done
 there.
   * RTAS handlers handle what they can. If there is something it
 cannot handle and sPAPRPHBClass::eeh_handler callback is defined,
 it is called.
   * sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It
 does ioctl() to the IOMMU container fd to complete the call. Error
 codes from that ioctl() are transferred back to the guest.
 
 [aik: defined RTAS tokens for EEH RTAS calls]
 Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
 ---
  hw/ppc/spapr_pci.c  | 275 
 
  include/hw/pci-host/spapr.h |   7 ++
  include/hw/ppc/spapr.h  |  43 ++-
  3 files changed, 323 insertions(+), 2 deletions(-)
 
 diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
 index 21b95b3..a150074 100644
 --- a/hw/ppc/spapr_pci.c
 +++ b/hw/ppc/spapr_pci.c
 @@ -406,6 +406,262 @@ static void 
 rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu,
  rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */
  }
  
 +static int rtas_handle_eeh_request(sPAPREnvironment *spapr,
 +   uint64_t buid, uint32_t req, uint32_t 
 opt)
 +{
 +sPAPRPHBState *sphb = find_phb(spapr, buid);
 +sPAPRPHBClass *info;
 +
 +if (!sphb) {
 +return -ENODEV;

I think it would make more sense to return RTAS error codes here,
rather than errnos.  At present all the callers seem to ignore the
exact value of this return value.

But it's not really correct to return RTAS_OUT_HW_ERROR for a bad
BUID, which is what this will do now.


It makes sense: RTAS_OUT_PARAM_ERROR, instead of RTAS_OUT_HW_ERROR
should be returned for invalid sPAPRPHBState and sPAPRPHBClass (as below).

It's a bit hard to have RTAS_OUT_* as the function's return value because
RTAS_OUT_PARAM_ERROR should be returned for some RTAS calls even 
info-eeh_handler()
returns negative value.

Also several of the callers have already done a find_phb() by the time
they call this.  Perhaps it would make more sense for this function to
take a sPAPRPHBState * instead of the buid.


It's not sure that find_phb() called by callers() before calling this
function. We did call find_dev() before calling this function for some
cases. How about changing the code like this way: Drop rtas_handle_eeh_request()
and put the logic into its callers, which would give more flexibility for
the callers to return proper values.

 +}
 +
 +info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
 +if (!info-eeh_handler) {
 +return -ENOENT;
 +}
 +
 +return info-eeh_handler(sphb, req, opt);
 +}
 +
 +static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
 +sPAPREnvironment *spapr,
 +uint32_t token, uint32_t nargs,
 +target_ulong args, uint32_t nret,
 +target_ulong rets)
 +{
 +uint32_t addr, option;
 +uint64_t buid;
 +int ret;
 +
 +if ((nargs != 4) || (nret != 1)) {
 +goto param_error_exit;
 +}
 +
 +buid = ((uint64_t)rtas_ld(args, 1)  32) | rtas_ld(args, 2);
 +addr = rtas_ld(args, 0);
 +option = rtas_ld(args, 3);
 +switch (option) {
 +case RTAS_EEH_ENABLE:
 +if (!find_dev(spapr, buid, addr)) {
 +goto param_error_exit;
 +}
 +break;
 +case RTAS_EEH_DISABLE:
 +case RTAS_EEH_THAW_IO:
 +case RTAS_EEH_THAW_DMA:

So, currently these are all implemented as no-ops.  Is that correct?


Currently, each PHB is binding to one PE. rtas_handle_eeh_request()
checks sPAPRPHBClass::eeh_handler accordingly, which is enough.

 +break;
 +default:
 +goto param_error_exit;
 +}
 +
 +ret = rtas_handle_eeh_request(spapr, buid,
 +  RTAS_EEH_REQ_SET_OPTION, option);
 +if (ret = 0) {
 +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 +} else {
 +rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
 +}
 +
 +return;
 +
 +param_error_exit:
 +rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
 +}
 +
 +static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
 +   sPAPREnvironment *spapr,
 +   uint32_t token, uint32_t nargs,
 +   target_ulong args, uint32_t nret,
 +   target_ulong rets)
 +{
 +uint32_t addr, option;
 +uint64_t buid;
 +sPAPRPHBState *sphb;
 +sPAPRPHBClass *info;
 +PCIDevice *pdev;
 +
 +if ((nargs 

[Qemu-devel] [Bug 1410288] Re: qemu-img conversion to qcow2 hangs with blank image less than 100kiB

2015-01-14 Thread Serge Hallyn
Went ahead and tested - it is in fact fixed in the v2.2 version.

** Changed in: qemu (Ubuntu)
   Importance: Undecided = Medium

** Changed in: qemu (Ubuntu)
   Status: Confirmed = Triaged

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

Title:
  qemu-img conversion to qcow2 hangs with blank image less than 100kiB

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Triaged

Bug description:
  If you try to convert a blank image to qcow2 that is less than 100kiB
  in size then qemu-img hangs trying to seek to the end of the file.

  $ truncate --size 102399 /tmp/temp
  $ qemu-img convert -p -O qcow2 /tmp/temp /tmp/temp2.qcow2

  I'm finding this on all versions of qemu-img v2.

  strace shows a seek loop.

  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0

  ProblemType: Bug
  DistroRelease: Ubuntu 14.04
  Package: qemu-utils 2.0.0+dfsg-2ubuntu1.10
  ProcVersionSignature: User Name 3.13.0-43.72-generic 3.13.11.11
  Uname: Linux 3.13.0-43-generic i686
  ApportVersion: 2.14.1-0ubuntu3.6
  Architecture: i386
  Date: Tue Jan 13 14:30:39 2015
  SourcePackage: qemu
  UpgradeStatus: No upgrade log present (probably fresh install)

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



Re: [Qemu-devel] [PATCH v3 6/9] rocker: add new rocker switch device

2015-01-14 Thread Scott Feldman
On Mon, Jan 12, 2015 at 4:57 AM, Paolo Bonzini pbonz...@redhat.com wrote:


 On 11/01/2015 04:57, sfel...@gmail.com wrote:
 +static const MemoryRegionOps rocker_mmio_ops = {
 +.read = rocker_mmio_read,
 +.write = rocker_mmio_write,
 +.endianness = DEVICE_LITTLE_ENDIAN,
 +.valid = {
 +.min_access_size = 4,
 +.max_access_size = 8,
 +},
 +.impl = {
 +.min_access_size = 4,
 +.max_access_size = 8,
 +},
 +};

 I suggest that you only use 32-bit registers in the internal
 implementation, where writing to the low part of a 64-bit register only
 writes to a latch.  You can then use .impl.max_access_size == 4 but keep
 .valid.max_access_size == 8.

 QEMU will then take care of passing 64-bit writes down as two 32-bit
 writes, in increasing address.

I tried setting .impl.max_access_size=4 and the Linux driver self-test
fails on 64-bit reg write/read.  32-bit regs writes/reads are OK.  On
the 64-bit write/read, the self test writes a value and expects to
read back 2x the value written.  After making the change
.impl.max_access_size=4, the reg reads back as 0, regardless of what
was written.

The driver uses writeq/readq for 64-bit writes/reads.

I'm inclined to not change the device and leave .impl.max_access_size=8.

-scott



Re: [Qemu-devel] [Bug 1410288] Re: qemu-img conversion to qcow2 hangs with blank image less than 100kiB

2015-01-14 Thread Serge Hallyn
Does it also fail with the qemu from
https://launchpad.net/~ubuntu-virt/+archive/ubuntu/virt-daily-upstream ?
(This isn't quite git head, but it is qemu v2.2)

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

Title:
  qemu-img conversion to qcow2 hangs with blank image less than 100kiB

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  If you try to convert a blank image to qcow2 that is less than 100kiB
  in size then qemu-img hangs trying to seek to the end of the file.

  $ truncate --size 102399 /tmp/temp
  $ qemu-img convert -p -O qcow2 /tmp/temp /tmp/temp2.qcow2

  I'm finding this on all versions of qemu-img v2.

  strace shows a seek loop.

  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0
  ioctl(6, FS_IOC_FIEMAP, 0xb5e68dc4) = 0
  _llseek(6, 0, [10], SEEK_END)   = 0

  ProblemType: Bug
  DistroRelease: Ubuntu 14.04
  Package: qemu-utils 2.0.0+dfsg-2ubuntu1.10
  ProcVersionSignature: User Name 3.13.0-43.72-generic 3.13.11.11
  Uname: Linux 3.13.0-43-generic i686
  ApportVersion: 2.14.1-0ubuntu3.6
  Architecture: i386
  Date: Tue Jan 13 14:30:39 2015
  SourcePackage: qemu
  UpgradeStatus: No upgrade log present (probably fresh install)

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



[Qemu-devel] [PATCH] qemu-iotests: Fix supported_oses check

2015-01-14 Thread Fam Zheng
There is a bug in the recently added sys.platform test and we no longer
run python tests, because linux2 is the value to compare here. So do a
prefix match, although the python documentation claims Linux is always
linux2.

Signed-off-by: Fam Zheng f...@redhat.com
---
 tests/qemu-iotests/iotests.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 87002e0..4011725 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -288,7 +288,7 @@ def main(supported_fmts=[], supported_oses=['linux']):
 if supported_fmts and (imgfmt not in supported_fmts):
 notrun('not suitable for this image format: %s' % imgfmt)
 
-if sys.platform not in supported_oses:
+if not any([sys.platform.startswith(x) for x in supported_oses]):
 notrun('not suitable for this OS: %s' % sys.platform)
 
 # We need to filter out the time taken from the output so that qemu-iotest
-- 
1.9.3




Re: [Qemu-devel] [PATCH v15 1/2] sPAPR: Implement EEH RTAS calls

2015-01-14 Thread David Gibson
On Thu, Jan 15, 2015 at 11:14:36AM +1100, Gavin Shan wrote:
 On Wed, Jan 14, 2015 at 12:39:35PM +1100, David Gibson wrote:
 On Mon, Jan 05, 2015 at 11:26:27AM +1100, Gavin Shan wrote:
  The emulation for EEH RTAS requests from guest isn't covered
  by QEMU yet and the patch implements them.
  
  The patch defines constants used by EEH RTAS calls and adds
  callback sPAPRPHBClass::eeh_handler, which is going to be used
  this way:
  
* RTAS calls are received in spapr_pci.c, sanity check is done
  there.
* RTAS handlers handle what they can. If there is something it
  cannot handle and sPAPRPHBClass::eeh_handler callback is defined,
  it is called.
* sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It
  does ioctl() to the IOMMU container fd to complete the call. Error
  codes from that ioctl() are transferred back to the guest.
  
  [aik: defined RTAS tokens for EEH RTAS calls]
  Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
  ---
   hw/ppc/spapr_pci.c  | 275 
  
   include/hw/pci-host/spapr.h |   7 ++
   include/hw/ppc/spapr.h  |  43 ++-
   3 files changed, 323 insertions(+), 2 deletions(-)
  
  diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
  index 21b95b3..a150074 100644
  --- a/hw/ppc/spapr_pci.c
  +++ b/hw/ppc/spapr_pci.c
  @@ -406,6 +406,262 @@ static void 
  rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu,
   rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */
   }
   
  +static int rtas_handle_eeh_request(sPAPREnvironment *spapr,
  +   uint64_t buid, uint32_t req, uint32_t 
  opt)
  +{
  +sPAPRPHBState *sphb = find_phb(spapr, buid);
  +sPAPRPHBClass *info;
  +
  +if (!sphb) {
  +return -ENODEV;
 
 I think it would make more sense to return RTAS error codes here,
 rather than errnos.  At present all the callers seem to ignore the
 exact value of this return value.
 
 But it's not really correct to return RTAS_OUT_HW_ERROR for a bad
 BUID, which is what this will do now.
 
 
 It makes sense: RTAS_OUT_PARAM_ERROR, instead of RTAS_OUT_HW_ERROR
 should be returned for invalid sPAPRPHBState and sPAPRPHBClass (as below).
 
 It's a bit hard to have RTAS_OUT_* as the function's return value because
 RTAS_OUT_PARAM_ERROR should be returned for some RTAS calls even 
 info-eeh_handler()
 returns negative value.

Um.. I don't quite see why that makes returning RTAS erorr values
difficult.  But I think it's made irrelevant by your suggestion later.

 Also several of the callers have already done a find_phb() by the time
 they call this.  Perhaps it would make more sense for this function to
 take a sPAPRPHBState * instead of the buid.
 
 
 It's not sure that find_phb() called by callers() before calling this
 function. We did call find_dev() before calling this function for some
 cases. How about changing the code like this way: Drop 
 rtas_handle_eeh_request()
 and put the logic into its callers, which would give more flexibility for
 the callers to return proper values.

Yes, I think that might be the best idea, rtas_handle_eeh_request()
seems like a bit of an odd multiplexer at the moment.

[snip]
 The ret  0 case isn't handled here.  It will fall through to
 param_error_exit, which is non-obvious, and it also seems unlikely
 that a parameter error is the only possible thing that can go wrong.
 
 
 Yeah, PAPR spec states the return value RTAS_OUT_SUCCESS or
 RTAS_OUT_PARAMETER_ERROR. There is no RTAS_OUT_HW_ERROR for
 this RTAS call ibm,read-slot-reset-state2.

Heh, ok.  I think you still want to make the fall-through more
obvious, it's easy to miss.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


pgpfQZdyesRRw.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH] pseries: Limit PCI host bridge index value

2015-01-14 Thread David Gibson
On Wed, Jan 14, 2015 at 11:23:10AM -0600, Michael Roth wrote:
 Quoting David Gibson (2015-01-13 20:33:39)
  pseries guests can have large numbers of PCI host bridges.  To avoid the
  user having to specify a number of different configuration values for every
  one, the device supports an index property which is a shorthand setting
  the various window and configuration addresses from a predefined sensible
  set.
  
  There are some problems with the details at present:
* The index propery is signed, but negative values will create PCI
  windows below where we expect, potentially colliding with other devices
* No limit is imposed on the index property and large values can
  translate to extremely large window addresses.  With PCI passthrough in
  particular this can mean we exceed various mapping and physical address
  limits causing the guest host bridge to not work in strange ways.
  
  This patch addresses this, by making index unsigned, and imposing a
  limit.  Currently the limit allows indices from 0..255 which is probably
  enough host bridges for the time being.  It's fairly easy to extend if
  we discover we need more.
  
  Signed-off-by: David Gibson da...@gibson.dropbear.id.au
 
 I think the limit makes sense, but since the check isn't triggered in cases
 where 'index' isn't specified and '[io,mem]_win_[size,offset]' are set
 explicitly, maybe it makes sense to sanity-check the final calculation for
 those values as well? We could actually drop the index limit in that case
 (if we decided we wanted to).
 
 But I think it's okay to assume such users know what they're doing in the
 meantime, so:

Yeah, that was my assumption, that if you're explicitly setting the
windows, then you know what you're doing.

 Reviewed-by: Michael Roth mdr...@linux.vnet.ibm.com

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


pgplyZJ7Ig17p.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH v15 1/2] sPAPR: Implement EEH RTAS calls

2015-01-14 Thread Gavin Shan
On Thu, Jan 15, 2015 at 12:33:59PM +1100, David Gibson wrote:
On Thu, Jan 15, 2015 at 11:14:36AM +1100, Gavin Shan wrote:
 On Wed, Jan 14, 2015 at 12:39:35PM +1100, David Gibson wrote:
 On Mon, Jan 05, 2015 at 11:26:27AM +1100, Gavin Shan wrote:
  The emulation for EEH RTAS requests from guest isn't covered
  by QEMU yet and the patch implements them.
  
  The patch defines constants used by EEH RTAS calls and adds
  callback sPAPRPHBClass::eeh_handler, which is going to be used
  this way:
  
* RTAS calls are received in spapr_pci.c, sanity check is done
  there.
* RTAS handlers handle what they can. If there is something it
  cannot handle and sPAPRPHBClass::eeh_handler callback is defined,
  it is called.
* sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It
  does ioctl() to the IOMMU container fd to complete the call. Error
  codes from that ioctl() are transferred back to the guest.
  
  [aik: defined RTAS tokens for EEH RTAS calls]
  Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
  ---
   hw/ppc/spapr_pci.c  | 275 
  
   include/hw/pci-host/spapr.h |   7 ++
   include/hw/ppc/spapr.h  |  43 ++-
   3 files changed, 323 insertions(+), 2 deletions(-)
  
  diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
  index 21b95b3..a150074 100644
  --- a/hw/ppc/spapr_pci.c
  +++ b/hw/ppc/spapr_pci.c
  @@ -406,6 +406,262 @@ static void 
  rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu,
   rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */
   }
   
  +static int rtas_handle_eeh_request(sPAPREnvironment *spapr,
  +   uint64_t buid, uint32_t req, uint32_t 
  opt)
  +{
  +sPAPRPHBState *sphb = find_phb(spapr, buid);
  +sPAPRPHBClass *info;
  +
  +if (!sphb) {
  +return -ENODEV;
 
 I think it would make more sense to return RTAS error codes here,
 rather than errnos.  At present all the callers seem to ignore the
 exact value of this return value.
 
 But it's not really correct to return RTAS_OUT_HW_ERROR for a bad
 BUID, which is what this will do now.
 
 
 It makes sense: RTAS_OUT_PARAM_ERROR, instead of RTAS_OUT_HW_ERROR
 should be returned for invalid sPAPRPHBState and sPAPRPHBClass (as below).
 
 It's a bit hard to have RTAS_OUT_* as the function's return value because
 RTAS_OUT_PARAM_ERROR should be returned for some RTAS calls even 
 info-eeh_handler()
 returns negative value.

Um.. I don't quite see why that makes returning RTAS erorr values
difficult.  But I think it's made irrelevant by your suggestion later.


Yeah, it's irrelevant after rtas_handle_eeh_request() is dropped.

 Also several of the callers have already done a find_phb() by the time
 they call this.  Perhaps it would make more sense for this function to
 take a sPAPRPHBState * instead of the buid.
 
 
 It's not sure that find_phb() called by callers() before calling this
 function. We did call find_dev() before calling this function for some
 cases. How about changing the code like this way: Drop 
 rtas_handle_eeh_request()
 and put the logic into its callers, which would give more flexibility for
 the callers to return proper values.

Yes, I think that might be the best idea, rtas_handle_eeh_request()
seems like a bit of an odd multiplexer at the moment.


Thanks for confirm. I'll change code accordingly in next revision.

[snip]
 The ret  0 case isn't handled here.  It will fall through to
 param_error_exit, which is non-obvious, and it also seems unlikely
 that a parameter error is the only possible thing that can go wrong.
 
 
 Yeah, PAPR spec states the return value RTAS_OUT_SUCCESS or
 RTAS_OUT_PARAMETER_ERROR. There is no RTAS_OUT_HW_ERROR for
 this RTAS call ibm,read-slot-reset-state2.

Heh, ok.  I think you still want to make the fall-through more
obvious, it's easy to miss.


Yes, I still need make it more obvious like this:

int ret;

ret = info-eeh_handler();
#if RTAS_CALL_SUPPORTS_HW_ERROR_AS_RETURN_VALUE
if (ret  0) {
rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
return;
}
#else
if (ret  0) {
goto param_error_exit;
}
#endif

/* Handle the result from info-eeh_handler() */
rtas_st(rets, 0, RTAS_OUT_SUCCESS);
return;
param_error_exit:
rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);

Thanks,
Gavin

-- 
David Gibson   | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you.  NOT _the_ _other_
   | _way_ _around_!
http://www.ozlabs.org/~dgibson





Re: [Qemu-devel] question about live migration with storage

2015-01-14 Thread Zhang Haoyu

On 2015-01-14 17:07:08, Paolo Bonzini wrote:
 
 
 On 14/01/2015 08:58, Zhang Haoyu wrote:
  2) Finer-grain control the parameters of block migration (dirty bitmap
  granularity).
 
  3) Block and RAM migration do not share the same socket and thus can
  more easily be parallelized.
 
drive_mirror job is done in main-thread,
then how to accept the qmp_migrate request while drive_mirror is performing?
If need to wait for the completion of drive_mirror,
how to implement the parallelization between block and ram migration?

Thanks,
Zhang Haoyu
  But, because of the parallelization, how to calculate the progress?
 
 You can use query-block-jobs.
 
  BTW, the traditional iterative mechanism is buggy-implemented?
  e.g., some bugs which are very difficult to fixed, or something?
 
 There are no bugs that corrupt data. It's simply less flexible.
 
 Regarding parallelization, the problems happen especially if you disable
 migration bandwidth limits.  Then you'll see large chunks of RAM and
 large chunks of block device data being sent.  This is a problem for
 convergence in some workloads. Instead, with NBD-based storage migration
 everything happens in parallel, and TCP makes sure that bandwidth is
 split between the sockets.
 
 If you have a migration bandwidth limit, NBD-based storage migration
 will use a separate bandwidth limit for each disk and for RAM. Thus
 network usage is less predictable than with block-migration.c.  On the
 other hand, storage migration uses a lot of network so it's usually more
 likely that you do not have migration bandwidth limits.
 
 Paolo
 
  Thanks,
  Zhang Haoyu
  Note that 1-2 are not yet supported by libvirt as far as I remember.
 
  Paolo




Re: [Qemu-devel] [sheepdog] [PATCH] sheepdog: selectable object size support

2015-01-14 Thread Hitoshi Mitake
At Tue, 13 Jan 2015 17:41:12 +0900,
Teruaki Ishizaki wrote:
 
 Previously, qemu block driver of sheepdog used hard-coded VDI object size.
 This patch enables users to handle block_size_shift value for
 calculating VDI object size.
 
 When you start qemu, you don't need to specify additional command option.
 
 But when you create the VDI which doesn't have default object size
 with qemu-img command, you specify block_size_shift option.
 
 If you want to create a VDI of 8MB(1  23) object size,
 you need to specify following command option.
 
  # qemu-img create -o block_size_shift=23 sheepdog:test1 100M
 
 In addition, when you don't specify qemu-img command option,
 a default value of sheepdog cluster is used for creating VDI.
 
  # qemu-img create sheepdog:test2 100M
 
 Signed-off-by: Teruaki Ishizaki ishizaki.teru...@lab.ntt.co.jp
 ---
  block/sheepdog.c  |  138 +---
  include/block/block_int.h |1 +
  2 files changed, 117 insertions(+), 22 deletions(-)


 
 @@ -1610,7 +1633,8 @@ out_with_err_set:
  if (bs) {
  bdrv_unref(bs);
  }
 -g_free(buf);
 +if (buf) 

The above line has a style problem (white space).

 @@ -1669,6 +1693,17 @@ static int parse_redundancy(BDRVSheepdogState *s, 
 const char *opt)
  return 0;
  }
  
 +static int parse_block_size_shift(BDRVSheepdogState *s, const char *opt)
 +{
 +struct SheepdogInode *inode = s-inode;
 +inode-block_size_shift = (uint8_t)atoi(opt);

This patch cannot create VDI with specified block size shift. Below is
the reason:

Initializing block_size_shift of the inode object here (in
parse_block_size_shift()) doesn't make sense. Because the
block_size_shift of newly created VDI is specified by block_size_shift
of SheepdogVdiReq (in sheepdog source code, sd_req.vdi). You need to
synchronize the struct and initialize the parameter. Below is a slack
solution:

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 525254e..3dc3359 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -168,7 +168,8 @@ typedef struct SheepdogVdiReq {
 uint32_t base_vdi_id;
 uint8_t copies;
 uint8_t copy_policy;
-uint8_t reserved[2];
+uint8_t store_policy;
+uint8_t block_size_shift;
 uint32_t snapid;
 uint32_t type;
 uint32_t pad[2];
@@ -1560,6 +1561,7 @@ static int do_sd_create(BDRVSheepdogState *s,
uint32_t *vdi_id, int snapshot,
 hdr.vdi_size = s-inode.vdi_size;
 hdr.copy_policy = s-inode.copy_policy;
 hdr.copies = s-inode.nr_copies;
+hdr.block_size_shift = s-inode.block_size_shift;
 
 ret = do_req(fd, s-aio_context, (SheepdogReq *)hdr, buf, wlen,
 rlen);

# This is a very slack solution. You need to avoid using inode as a
# temporal space of the parameter.

Thanks,
Hitoshi



Re: [Qemu-devel] [virtio] virtqueue allocation and thread-safety

2015-01-14 Thread Fam Zheng
On Wed, 01/14 14:01, Vasile Catalin-B50542 wrote:
 Hi,
 
 I'm trying to make a new virtio device.
 I got it running (I made a functional dummy device  guest driver).
 Now I'm trying to build some communication between the device and guest
 driver.
 I can't seem to find where the actual allocation of virtqueues are made.
 I've looked inside virtio_init(), but it just allocates the vq structures
 and I don't see
 any pointers inside that structure that might give and idea of something
 dynamically
 allocated. There is a member of that structure named vector, but it's type
 is uint16_t.
 I've also looked inside the virtio_add_queue(), and it just makes some
 constant assignments.
 Where are the vqs buffer space actually allocated?

The guest memory is prepared by guest. See the virtio spec Virtual I/O Device
(VIRTIO) Version 1.0 section 3.2.1 Supplying Buffers to The Device:

quote

The driver offers buffers to one of the device’s virtqueues as follows:

1. The driver places the buffer into free descriptor(s) in the descriptor
table, chaining as necessary (see 2.4.5 The Virtqueue Descriptor Table).

2. The driver places the index of the head of the descriptor chain into the
next ring entry of the available ring.

3. Steps 1 and 2 MAY be performed repeatedly if batching is possible.

4. The driver performs suitable a memory barrier to ensure the device sees the
updated descriptor table and available ring before the next step.

5. The available idx is increased by the number of descriptor chain heads added
to the available ring.

6. The driver performs a suitable memory barrier to ensure that it updates the
idx field before checking for notification suppression.

7. If notifications are not suppressed, the driver notifies the device of the
new available buffers.

/unquote

Fam

 One more thing. Are the virtqueue functions thread safe, from both point of
 views
 (qemu and guest driver API's view)?
 Also I don't see any dynamic allocations/freeing when pushing and popping,
 either.
 
 Cătă
 



Re: [Qemu-devel] [PATCH 18/19] block/parallels: add prealloc-mode and prealloc-size open paramemets

2015-01-14 Thread Denis V. Lunev

On 14/01/15 17:26, Roman Kagan wrote:

On Tue, Dec 30, 2014 at 01:07:11PM +0300, Denis V. Lunev wrote:

This is preparational commit for tweaks in Parallels image expansion.
The idea is that enlarge via truncate by one data block is slow. It
would be much better to use fallocate via bdrv_write_zeroes and
expand by some significant amount at once.

This patch just adds proper parameters into BDRVParallelsState and
performs options parsing in parallels_open.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
---
  block/parallels.c | 72 +++
  1 file changed, 72 insertions(+)

diff --git a/block/parallels.c b/block/parallels.c
index 18b9267..12a9cea 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -30,6 +30,7 @@
  #include qemu-common.h
  #include block/block_int.h
  #include qemu/module.h
+#include qapi/util.h
  
  /**/
  
@@ -54,6 +55,20 @@ typedef struct ParallelsHeader {

  char padding[12];
  } QEMU_PACKED ParallelsHeader;
  
+

+typedef enum ParallelsPreallocMode {
+PRL_PREALLOC_MODE_FALLOCATE = 0,
+PRL_PREALLOC_MODE_TRUNCATE = 1,
+PRL_PREALLOC_MODE_MAX = 2,
+} ParallelsPreallocMode;
+
+static const char *prealloc_mode_lookup[] = {
+falloc,
+truncate,
+NULL,

There is already generic preallocaton option, BLOCK_OPT_PREALLOC,
which is handled by qcow2 and raw-posix.  It means slightly different
thing: the *whole* image is preallocated using the method specified.

I think it would make sense to consolidate that option with this new
batched allocation in the generic block code.  I guess qcow2 and
raw-posix would benefit from it, too.

At any rate I think it's a matter for a separate patchset.

Roman.

it is too early :) I think that I should provide the rationale for
the preallocation in general. I am working on this with CEPH :)



[Qemu-devel] [PATCH 4/4] block: Eliminate silly QERR_ macros used for encryption keys

2015-01-14 Thread Markus Armbruster
The QERR_ macros are leftovers from the days of rich error objects.
They're used with error_set() and qerror_report(), and expand into the
first *two* arguments.  This trickiness has become pointless.  Clean
up QERR_DEVICE_ENCRYPTED and QERR_DEVICE_NOT_ENCRYPTED.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 block.c   | 6 --
 include/qapi/qmp/qerror.h | 6 --
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index ba005f3..4cf6d29 100644
--- a/block.c
+++ b/block.c
@@ -3737,14 +3737,16 @@ void bdrv_add_key(BlockDriverState *bs, const char 
*key, Error **errp)
 {
 if (key) {
 if (!bdrv_is_encrypted(bs)) {
-error_set(errp, QERR_DEVICE_NOT_ENCRYPTED,
+error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+  Device '%s' is not encrypted,
   bdrv_get_device_name(bs));
 } else if (bdrv_set_key(bs, key)  0) {
 error_set(errp, QERR_INVALID_PASSWORD);
 }
 } else {
 if (bdrv_key_required(bs)) {
-error_set(errp, QERR_DEVICE_ENCRYPTED,
+error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
+  '%s' (%s) is encrypted,
   bdrv_get_device_name(bs),
   bdrv_get_encrypted_filename(bs));
 }
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index becdca6..423d8a3 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -52,9 +52,6 @@ void qerror_report_err(Error *err);
 #define QERR_COMMAND_NOT_FOUND \
 ERROR_CLASS_COMMAND_NOT_FOUND, The command %s has not been found
 
-#define QERR_DEVICE_ENCRYPTED \
-ERROR_CLASS_DEVICE_ENCRYPTED, '%s' (%s) is encrypted
-
 #define QERR_DEVICE_HAS_NO_MEDIUM \
 ERROR_CLASS_GENERIC_ERROR, Device '%s' has no medium
 
@@ -73,9 +70,6 @@ void qerror_report_err(Error *err);
 #define QERR_DEVICE_NOT_ACTIVE \
 ERROR_CLASS_DEVICE_NOT_ACTIVE, No %s device has been activated
 
-#define QERR_DEVICE_NOT_ENCRYPTED \
-ERROR_CLASS_GENERIC_ERROR, Device '%s' is not encrypted
-
 #define QERR_DEVICE_NOT_FOUND \
 ERROR_CLASS_DEVICE_NOT_FOUND, Device '%s' not found
 
-- 
1.9.3




[Qemu-devel] [PATCH 0/4] block: Cleanups around error reporting

2015-01-14 Thread Markus Armbruster
Markus Armbruster (4):
  blockdev: Give find_block_job() an Error ** parameter
  blockdev: Eliminate silly QERR_BLOCK_JOB_NOT_ACTIVE macro
  block: New bdrv_add_key(), convert monitor to use it
  block: Eliminate silly QERR_ macros used for encryption keys

 block.c   | 31 +++
 blockdev.c| 44 +++-
 include/block/block.h |  1 +
 include/qapi/qmp/qerror.h |  9 -
 monitor.c | 16 +++-
 qmp.c |  8 
 6 files changed, 58 insertions(+), 51 deletions(-)

-- 
1.9.3




[Qemu-devel] [PATCH 2/4] blockdev: Eliminate silly QERR_BLOCK_JOB_NOT_ACTIVE macro

2015-01-14 Thread Markus Armbruster
From: Markus Armbruster arm...@pond.sub.org

The QERR_ macros are leftovers from the days of rich error objects.
They're used with error_set() and qerror_report(), and expand into the
first *two* arguments.  This trickiness has become pointless.  Clean
this one up.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 blockdev.c| 3 ++-
 include/qapi/qmp/qerror.h | 3 ---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 8d6ca35..287d7af 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2674,7 +2674,8 @@ static BlockJob *find_block_job(const char *device, 
AioContext **aio_context,
 return bs-job;
 
 notfound:
-error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
+error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
+  No active block job on device '%s', device);
 *aio_context = NULL;
 return NULL;
 }
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 0ca6cbd..becdca6 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -37,9 +37,6 @@ void qerror_report_err(Error *err);
 #define QERR_BASE_NOT_FOUND \
 ERROR_CLASS_GENERIC_ERROR, Base '%s' not found
 
-#define QERR_BLOCK_JOB_NOT_ACTIVE \
-ERROR_CLASS_DEVICE_NOT_ACTIVE, No active block job on device '%s'
-
 #define QERR_BLOCK_JOB_NOT_READY \
 ERROR_CLASS_GENERIC_ERROR, The active block job for device '%s' cannot be 
completed
 
-- 
1.9.3




Re: [Qemu-devel] [PATCH 0/4] block: Cleanups around error reporting

2015-01-14 Thread Eric Blake
On 01/14/2015 07:31 AM, Markus Armbruster wrote:
 Markus Armbruster (4):
   blockdev: Give find_block_job() an Error ** parameter
   blockdev: Eliminate silly QERR_BLOCK_JOB_NOT_ACTIVE macro
   block: New bdrv_add_key(), convert monitor to use it
   block: Eliminate silly QERR_ macros used for encryption keys

Reviewed-by: Eric Blake ebl...@redhat.com

 
  block.c   | 31 +++
  blockdev.c| 44 +++-
  include/block/block.h |  1 +
  include/qapi/qmp/qerror.h |  9 -
  monitor.c | 16 +++-
  qmp.c |  8 
  6 files changed, 58 insertions(+), 51 deletions(-)
 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend

2015-01-14 Thread Lluís Vilanova
Bastian Koppelmann writes:

 There is no overview, that shows all the frontend operation one can use, as
 found on the wiki. Unfortunatly the wiki is out of date, so let's try to move
 this documentation to the source files, which has the benefit, that it is easy
 to update the documentation, if the frontend is changed. This patch adds 
 doxygen
 tags to all the 32 bit versions of the tcg frontend operations, because the
 64 bit version would mostly have the same documentation, and all the type
 conversition operations. The file tag has a note, that makes the user aware of
 the missing 64 operations. In this version all the immediate variants are also
 documented by simply refering to the non immediate version. However I'm 
 willing
 to drop that.

 Any comments?

The operations (or at least most of them) are already documented in
tcg/README. If this change is accepted, I'd rather move the contents of the
README file into here.

Also, AFAIR it was decided to use gtk-doc instead of doxygen.


Cheers,
  Lluis


 Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
 ---
  tcg/tcg-op.h | 477 
 ++-
  1 file changed, 436 insertions(+), 41 deletions(-)

 diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
 index 96adf9a..5bb7e65 100644
 --- a/tcg/tcg-op.h
 +++ b/tcg/tcg-op.h
 @@ -26,6 +26,29 @@
  #include exec/helper-proto.h
  #include exec/helper-gen.h
 
 +/**
 +  * @file tcg-op.h
 +  * @brief These are the supported operations as implemented by the TCG 
 frontend
 +  *for the target cpu (what QEMU executes; not where QEMU executes).
 +  *This information is useful for people who want to port QEMU to
 +  *emulate a new processor.
 +  *The frontend helpers for generating TCG operations typically take 
 the
 +  *form: tcg_gen_op[i]_reg_size.
 +  * - The op is the TCG operation that will be generated for its
 +  *   arguments
 +  * - The [i] suffix is used to indicate the TCG operation takes an
 +  *   immediate rather than a normal register.
 +  * - The reg_size refers to the size of the TCG registers in use.
 +  *   The vast majority of the time, this will match the native size 
 of
 +  *   the emulated target, so rather than force people to type i32 or
 +  *   i64 all the time, the shorthand tl is made available for all
 +  *   helpers.
 +  * @note  This documentation only covers the 32 bit operations. If not 
 stated
 +  *otherwise, every operation is also available as a 64 bit 
 operation.
 +  *If no suffix reg_size is given, the operation is available for 
 32
 +  *and 64 bit guests, e.g. tcg_gen_br.
 +  */
 +
  /* Basic output routines.  Not for general consumption.  */
 
  void tcg_gen_op1(TCGContext *, TCGOpcode, TCGArg);
 @@ -251,13 +274,24 @@ static inline void tcg_gen_op6ii_i64(TCGOpcode opc, 
 TCGv_i64 a1, TCGv_i64 a2,
 
  /* Generic ops.  */
 
 +/**
 +  * Create a new label, used for branch instructions.
 +  * \sa tcg_gen_br
 +  */
  int gen_new_label(void);
 -
 +/**
 +  * Label the current location with label n, so branch instructions can jump
 +  * there.
 +  * \sa tcg_gen_br, gen_new_label
 +  */
  static inline void gen_set_label(int n)
  {
  tcg_gen_op1(tcg_ctx, INDEX_op_set_label, n);
  }
 -
 +/**
 +  * Jump to a label.
 +  * \sa gen_new_label, gen_set_label
 +  */
  static inline void tcg_gen_br(int label)
  {
  tcg_gen_op1(tcg_ctx, INDEX_op_br, label);
 @@ -267,163 +301,386 @@ static inline void tcg_gen_br(int label)
  /* Helper calls. */
 
  /* 32 bit ops */
 -
 +/**
 +  * \sa tcg_gen_add_i32
 +  */
  void tcg_gen_addi_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
 +/**
 +  * Subtracts  32 bit register arg2 from constant arg1.
 +  * \sa tcg_gen_sub_i32
 +  */
  void tcg_gen_subfi_i32(TCGv_i32 ret, int32_t arg1, TCGv_i32 arg2);
 +/**
 +  * \sa tcg_gen_sub_i32
 +  */
  void tcg_gen_subi_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
 +/**
 +  * \sa tcg_gen_and_i32
 +  */
  void tcg_gen_andi_i32(TCGv_i32 ret, TCGv_i32 arg1, uint32_t arg2);
 +/**
 +  * \sa tcg_gen_or_i32
 +  */
  void tcg_gen_ori_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
 +/**
 +  * \sa tcg_gen_xor_i32
 +  */
  void tcg_gen_xori_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
 +/**
 +  * \sa tcg_gen_shl_i32
 +  */
  void tcg_gen_shli_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
 +/**
 +  * \sa tcg_gen_shr_i32
 +  */
  void tcg_gen_shri_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
 +/**
 +  * \sa tcg_gen_sar_i32
 +  */
  void tcg_gen_sari_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
 +/**
 +  * \sa tcg_gen_mul_i32
 +  */
  void tcg_gen_muli_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
 +/**
 +  * ret = arg1/arg2 (signed).
 +  * Undefined behavior if division by zero or overflow.
 +  */
  void tcg_gen_div_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2);
 +/**
 +  * ret = arg1\%arg2 (signed).
 +  * Undefined 

Re: [Qemu-devel] [PATCH 17/19] block/parallels: delay writing to BAT till bdrv_co_flush_to_os

2015-01-14 Thread Denis V. Lunev

On 14/01/15 16:34, Roman Kagan wrote:

On Wed, Jan 14, 2015 at 04:08:50PM +0300, Denis V. Lunev wrote:

On 14/01/15 16:03, Roman Kagan wrote:

On Tue, Dec 30, 2014 at 01:07:10PM +0300, Denis V. Lunev wrote:

+static int cache_bat(BlockDriverState *bs, uint32_t idx, uint32_t new_data_off)
+{
+int ret, i, off, cache_off;
+int64_t first_idx, last_idx;
+BDRVParallelsState *s = bs-opaque;
+uint32_t *cache = s-bat_cache;
+
+off = bat_offset(idx);
+cache_off = (off / s-bat_cache_size) * s-bat_cache_size;
+
+if (s-bat_cache_off != -1  s-bat_cache_off != cache_off) {
+ret = write_bat_cache(bs);
+if (ret  0) {
+return ret;
+}
+}
+
+first_idx = idx - (off - cache_off) / sizeof(uint32_t);
+last_idx = first_idx + s-bat_cache_size / sizeof(uint32_t);
+if (first_idx  0) {
+memcpy(s-bat_cache, s-ph, sizeof(s-ph));
+first_idx = 0;
+cache = s-bat_cache + sizeof(s-ph) / sizeof(uint32_t);
+}
+
+if (last_idx  s-bat_size) {
+memset(cache + s-bat_size - first_idx, 0,
+   sizeof(uint32_t) * (last_idx - s-bat_size));
+}
+
+for (i = 0; i  last_idx - first_idx; i++) {
+cache[i] = cpu_to_le32(s-bat[first_idx + i]);
+}

You're re-populating the bat_cache on every bat update.  Why?

Shouldn't this be done only with empty cache, i.e. under
if (s-bat_cache_off == -1)?

reasonable, but condition should be a bit different, namely

if (s-bat_cache_off != -1  s-bat_cache_off != cache_off) {

No, this is the condition to flush the cache which you already do.

Then, either upon flushing it or on the first entry into cache_bat(),
the cache is empty which is indicated by -bat_cache_off == -1, at which
point you need to populate it from -bat.


you are wrong. BAT cache is a single page of the whole BAT
which can be more than several MBs. This page should be
repopulated when the off is changed.



Re: [Qemu-devel] [PATCH 8/9] balloon: Factor out common is balloon active test

2015-01-14 Thread Markus Armbruster
Eric Blake ebl...@redhat.com writes:

 On 01/13/2015 10:50 AM, Markus Armbruster wrote:
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---
  balloon.c | 29 +++--
  1 file changed, 15 insertions(+), 14 deletions(-)
 
 diff --git a/balloon.c b/balloon.c
 index 2884c2d..aa30617 100644
 --- a/balloon.c
 +++ b/balloon.c
 @@ -36,6 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn;
  static QEMUBalloonStatus *balloon_stat_fn;
  static void *balloon_opaque;
  
 +static int have_ballon(Error **errp)
 +{
 +if (kvm_enabled()  !kvm_has_sync_mmu()) {
 +error_set(errp, QERR_KVM_MISSING_CAP, synchronous MMU, balloon);
 +return 0;
 +}
 +if (!balloon_event_fn) {
 +error_set(errp, QERR_DEVICE_NOT_ACTIVE, balloon);
 +return 0;
 +}
 +return 1;

 Another case where I would have used bool, true, false.

Me too now that you mention it %-)

 Either way,
 Reviewed-by: Eric Blake ebl...@redhat.com

Thanks!



[Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend

2015-01-14 Thread Bastian Koppelmann
There is no overview, that shows all the frontend operation one can use, as
found on the wiki. Unfortunatly the wiki is out of date, so let's try to move
this documentation to the source files, which has the benefit, that it is easy
to update the documentation, if the frontend is changed. This patch adds doxygen
tags to all the 32 bit versions of the tcg frontend operations, because the
64 bit version would mostly have the same documentation, and all the type
conversition operations. The file tag has a note, that makes the user aware of
the missing 64 operations. In this version all the immediate variants are also
documented by simply refering to the non immediate version. However I'm willing
to drop that.

Any comments?

Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
---
 tcg/tcg-op.h | 477 ++-
 1 file changed, 436 insertions(+), 41 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 96adf9a..5bb7e65 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -26,6 +26,29 @@
 #include exec/helper-proto.h
 #include exec/helper-gen.h
 
+/**
+  * @file tcg-op.h
+  * @brief These are the supported operations as implemented by the TCG 
frontend
+  *for the target cpu (what QEMU executes; not where QEMU executes).
+  *This information is useful for people who want to port QEMU to
+  *emulate a new processor.
+  *The frontend helpers for generating TCG operations typically take 
the
+  *form: tcg_gen_op[i]_reg_size.
+  * - The op is the TCG operation that will be generated for its
+  *   arguments
+  * - The [i] suffix is used to indicate the TCG operation takes an
+  *   immediate rather than a normal register.
+  * - The reg_size refers to the size of the TCG registers in use.
+  *   The vast majority of the time, this will match the native size of
+  *   the emulated target, so rather than force people to type i32 or
+  *   i64 all the time, the shorthand tl is made available for all
+  *   helpers.
+  * @note  This documentation only covers the 32 bit operations. If not stated
+  *otherwise, every operation is also available as a 64 bit operation.
+  *If no suffix reg_size is given, the operation is available for 32
+  *and 64 bit guests, e.g. tcg_gen_br.
+  */
+
 /* Basic output routines.  Not for general consumption.  */
 
 void tcg_gen_op1(TCGContext *, TCGOpcode, TCGArg);
@@ -251,13 +274,24 @@ static inline void tcg_gen_op6ii_i64(TCGOpcode opc, 
TCGv_i64 a1, TCGv_i64 a2,
 
 /* Generic ops.  */
 
+/**
+  * Create a new label, used for branch instructions.
+  * \sa tcg_gen_br
+  */
 int gen_new_label(void);
-
+/**
+  * Label the current location with label n, so branch instructions can jump
+  * there.
+  * \sa tcg_gen_br, gen_new_label
+  */
 static inline void gen_set_label(int n)
 {
 tcg_gen_op1(tcg_ctx, INDEX_op_set_label, n);
 }
-
+/**
+  * Jump to a label.
+  * \sa gen_new_label, gen_set_label
+  */
 static inline void tcg_gen_br(int label)
 {
 tcg_gen_op1(tcg_ctx, INDEX_op_br, label);
@@ -267,163 +301,386 @@ static inline void tcg_gen_br(int label)
 /* Helper calls. */
 
 /* 32 bit ops */
-
+/**
+  * \sa tcg_gen_add_i32
+  */
 void tcg_gen_addi_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
+/**
+  * Subtracts  32 bit register arg2 from constant arg1.
+  * \sa tcg_gen_sub_i32
+  */
 void tcg_gen_subfi_i32(TCGv_i32 ret, int32_t arg1, TCGv_i32 arg2);
+/**
+  * \sa tcg_gen_sub_i32
+  */
 void tcg_gen_subi_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
+/**
+  * \sa tcg_gen_and_i32
+  */
 void tcg_gen_andi_i32(TCGv_i32 ret, TCGv_i32 arg1, uint32_t arg2);
+/**
+  * \sa tcg_gen_or_i32
+  */
 void tcg_gen_ori_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
+/**
+  * \sa tcg_gen_xor_i32
+  */
 void tcg_gen_xori_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
+/**
+  * \sa tcg_gen_shl_i32
+  */
 void tcg_gen_shli_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
+/**
+  * \sa tcg_gen_shr_i32
+  */
 void tcg_gen_shri_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
+/**
+  * \sa tcg_gen_sar_i32
+  */
 void tcg_gen_sari_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
+/**
+  * \sa tcg_gen_mul_i32
+  */
 void tcg_gen_muli_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
+/**
+  * ret = arg1/arg2 (signed).
+  * Undefined behavior if division by zero or overflow.
+  */
 void tcg_gen_div_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2);
+/**
+  * ret = arg1\%arg2 (signed).
+  * Undefined behavior if division by zero or overflow.
+  */
 void tcg_gen_rem_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2);
+/**
+  * ret = arg1/arg2 (unsigned). Undefined behavior if division by zero.
+  */
 void tcg_gen_divu_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2);
+/**
+  * ret = arg1\%arg2 (unsigned). Undefined behavior if division by zero.
+  */
 void tcg_gen_remu_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2);

Re: [Qemu-devel] [PATCH 18/19] block/parallels: add prealloc-mode and prealloc-size open paramemets

2015-01-14 Thread Roman Kagan
On Tue, Dec 30, 2014 at 01:07:11PM +0300, Denis V. Lunev wrote:
 This is preparational commit for tweaks in Parallels image expansion.
 The idea is that enlarge via truncate by one data block is slow. It
 would be much better to use fallocate via bdrv_write_zeroes and
 expand by some significant amount at once.
 
 This patch just adds proper parameters into BDRVParallelsState and
 performs options parsing in parallels_open.
 
 Signed-off-by: Denis V. Lunev d...@openvz.org
 CC: Kevin Wolf kw...@redhat.com
 CC: Stefan Hajnoczi stefa...@redhat.com
 ---
  block/parallels.c | 72 
 +++
  1 file changed, 72 insertions(+)
 
 diff --git a/block/parallels.c b/block/parallels.c
 index 18b9267..12a9cea 100644
 --- a/block/parallels.c
 +++ b/block/parallels.c
 @@ -30,6 +30,7 @@
  #include qemu-common.h
  #include block/block_int.h
  #include qemu/module.h
 +#include qapi/util.h
  
  /**/
  
 @@ -54,6 +55,20 @@ typedef struct ParallelsHeader {
  char padding[12];
  } QEMU_PACKED ParallelsHeader;
  
 +
 +typedef enum ParallelsPreallocMode {
 +PRL_PREALLOC_MODE_FALLOCATE = 0,
 +PRL_PREALLOC_MODE_TRUNCATE = 1,
 +PRL_PREALLOC_MODE_MAX = 2,
 +} ParallelsPreallocMode;
 +
 +static const char *prealloc_mode_lookup[] = {
 +falloc,
 +truncate,
 +NULL,

There is already generic preallocaton option, BLOCK_OPT_PREALLOC,
which is handled by qcow2 and raw-posix.  It means slightly different
thing: the *whole* image is preallocated using the method specified.

I think it would make sense to consolidate that option with this new
batched allocation in the generic block code.  I guess qcow2 and
raw-posix would benefit from it, too.

At any rate I think it's a matter for a separate patchset.

Roman.



Re: [Qemu-devel] [PATCH] AIO: Reduce number of threads for 32bit hosts

2015-01-14 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 On 14/01/2015 11:20, Kevin Wolf wrote:
  The same problem applies to coroutine stacks, and those cannot be
  throttled down as easily.  But I guess if you limit the number of
  threads, the guest gets slowed down and doesn't create as many coroutines.
 Shouldn't we rather try and decrease the stack sizes a bit? 1 MB per
 coroutine is really a lot, and as I understand it, threads take even
 more by default.

 Yup, 2 MB.  Last time I proposed this, I think Markus was strongly in 
 the better safe than sorry camp. :)

Yup.  Running out of stack is a nasty failure mode.

 But thread pool workers definitely don't need a big stack.

When analysis leads to an upper bound for stack size, by all means use
it.

Absent rigorous analysis backed by testing, I recommend to throw address
space at the problem.

32 bit limitations can force us to throw less generously (and be less
safe) there.  Not a good reason to compromise safety on 64 bit machines
as well, though.

[...]



[Qemu-devel] [PATCH] s390x/pci: fix 2 bugs found by coverity

2015-01-14 Thread Frank Blaschka
Signed-off-by: Frank Blaschka blasc...@linux.vnet.ibm.com
---
 hw/s390x/s390-pci-bus.c  | 1 +
 hw/s390x/s390-pci-inst.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 1201b8d..546dcf1 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -192,6 +192,7 @@ static void s390_pci_generate_event(uint8_t cc, uint16_t 
pec, uint32_t fh,
 object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
 
 if (!s) {
+g_free(sei_cont);
 return;
 }
 
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 5ea13e5..5596679 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -787,7 +787,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t 
fiba)
 data = (pbdev-isc  28) | (pbdev-noi  16) |
(pbdev-routes.adapter.ind_offset  8) | (pbdev-sum  7) |
pbdev-routes.adapter.summary_offset;
-stw_p(fib.data, data);
+stl_p(fib.data, data);
 
 if (pbdev-fh  ENABLE_BIT_OFFSET) {
 fib.fc |= 0x80;
-- 
2.1.4




[Qemu-devel] [PATCH 1/4] blockdev: Give find_block_job() an Error ** parameter

2015-01-14 Thread Markus Armbruster
When find_block_job() fails, all its callers build the same Error
object.  Build it in find_block_job() instead.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 blockdev.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index d59efd3..8d6ca35 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2653,7 +2653,8 @@ out:
 }
 
 /* Get the block job for a given device name and acquire its AioContext */
-static BlockJob *find_block_job(const char *device, AioContext **aio_context)
+static BlockJob *find_block_job(const char *device, AioContext **aio_context,
+Error **errp)
 {
 BlockDriverState *bs;
 
@@ -2673,6 +2674,7 @@ static BlockJob *find_block_job(const char *device, 
AioContext **aio_context)
 return bs-job;
 
 notfound:
+error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
 *aio_context = NULL;
 return NULL;
 }
@@ -2680,10 +2682,9 @@ notfound:
 void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp)
 {
 AioContext *aio_context;
-BlockJob *job = find_block_job(device, aio_context);
+BlockJob *job = find_block_job(device, aio_context, errp);
 
 if (!job) {
-error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
 return;
 }
 
@@ -2695,10 +2696,9 @@ void qmp_block_job_cancel(const char *device,
   bool has_force, bool force, Error **errp)
 {
 AioContext *aio_context;
-BlockJob *job = find_block_job(device, aio_context);
+BlockJob *job = find_block_job(device, aio_context, errp);
 
 if (!job) {
-error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
 return;
 }
 
@@ -2721,10 +2721,9 @@ out:
 void qmp_block_job_pause(const char *device, Error **errp)
 {
 AioContext *aio_context;
-BlockJob *job = find_block_job(device, aio_context);
+BlockJob *job = find_block_job(device, aio_context, errp);
 
 if (!job) {
-error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
 return;
 }
 
@@ -2736,10 +2735,9 @@ void qmp_block_job_pause(const char *device, Error 
**errp)
 void qmp_block_job_resume(const char *device, Error **errp)
 {
 AioContext *aio_context;
-BlockJob *job = find_block_job(device, aio_context);
+BlockJob *job = find_block_job(device, aio_context, errp);
 
 if (!job) {
-error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
 return;
 }
 
@@ -2751,10 +2749,9 @@ void qmp_block_job_resume(const char *device, Error 
**errp)
 void qmp_block_job_complete(const char *device, Error **errp)
 {
 AioContext *aio_context;
-BlockJob *job = find_block_job(device, aio_context);
+BlockJob *job = find_block_job(device, aio_context, errp);
 
 if (!job) {
-error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
 return;
 }
 
-- 
1.9.3




[Qemu-devel] [PATCH 3/4] block: New bdrv_add_key(), convert monitor to use it

2015-01-14 Thread Markus Armbruster
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 block.c   | 29 +
 blockdev.c| 24 ++--
 include/block/block.h |  1 +
 monitor.c | 16 +++-
 qmp.c |  8 
 5 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/block.c b/block.c
index cbe4a32..ba005f3 100644
--- a/block.c
+++ b/block.c
@@ -3722,6 +3722,35 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
 return ret;
 }
 
+/*
+ * Provide an encryption key for @bs.
+ * If @key is non-null:
+ * If @bs is not encrypted, fail.
+ * Else if the key is invalid, fail.
+ * Else set @bs's key to @key, replacing the existing key, if any.
+ * If @key is null:
+ * If @bs is encrypted and still lacks a key, fail.
+ * Else do nothing.
+ * On failure, store an error object through @errp if non-null.
+ */
+void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
+{
+if (key) {
+if (!bdrv_is_encrypted(bs)) {
+error_set(errp, QERR_DEVICE_NOT_ENCRYPTED,
+  bdrv_get_device_name(bs));
+} else if (bdrv_set_key(bs, key)  0) {
+error_set(errp, QERR_INVALID_PASSWORD);
+}
+} else {
+if (bdrv_key_required(bs)) {
+error_set(errp, QERR_DEVICE_ENCRYPTED,
+  bdrv_get_device_name(bs),
+  bdrv_get_encrypted_filename(bs));
+}
+}
+}
+
 const char *bdrv_get_format_name(BlockDriverState *bs)
 {
 return bs-drv ? bs-drv-format_name : NULL;
diff --git a/blockdev.c b/blockdev.c
index 287d7af..7d34960 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1793,7 +1793,6 @@ void qmp_block_passwd(bool has_device, const char *device,
 Error *local_err = NULL;
 BlockDriverState *bs;
 AioContext *aio_context;
-int err;
 
 bs = bdrv_lookup_bs(has_device ? device : NULL,
 has_node_name ? node_name : NULL,
@@ -1806,16 +1805,8 @@ void qmp_block_passwd(bool has_device, const char 
*device,
 aio_context = bdrv_get_aio_context(bs);
 aio_context_acquire(aio_context);
 
-err = bdrv_set_key(bs, password);
-if (err == -EINVAL) {
-error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
-goto out;
-} else if (err  0) {
-error_set(errp, QERR_INVALID_PASSWORD);
-goto out;
-}
+bdrv_add_key(bs, password, errp);
 
-out:
 aio_context_release(aio_context);
 }
 
@@ -1833,18 +1824,7 @@ static void qmp_bdrv_open_encrypted(BlockDriverState 
*bs, const char *filename,
 return;
 }
 
-if (bdrv_key_required(bs)) {
-if (password) {
-if (bdrv_set_key(bs, password)  0) {
-error_set(errp, QERR_INVALID_PASSWORD);
-}
-} else {
-error_set(errp, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs),
-  bdrv_get_encrypted_filename(bs));
-}
-} else if (password) {
-error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
-}
+bdrv_add_key(bs, password, errp);
 }
 
 void qmp_change_blockdev(const char *device, const char *filename,
diff --git a/include/block/block.h b/include/block/block.h
index 3082d2b..623c390 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -378,6 +378,7 @@ BlockDriverState *bdrv_next(BlockDriverState *bs);
 int bdrv_is_encrypted(BlockDriverState *bs);
 int bdrv_key_required(BlockDriverState *bs);
 int bdrv_set_key(BlockDriverState *bs, const char *key);
+void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp);
 int bdrv_query_missing_keys(void);
 void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
  void *opaque);
diff --git a/monitor.c b/monitor.c
index 1808e41..99876dd 100644
--- a/monitor.c
+++ b/monitor.c
@@ -5366,9 +5366,12 @@ static void bdrv_password_cb(void *opaque, const char 
*password,
 Monitor *mon = opaque;
 BlockDriverState *bs = readline_opaque;
 int ret = 0;
+Error *local_err = NULL;
 
-if (bdrv_set_key(bs, password) != 0) {
-monitor_printf(mon, invalid password\n);
+bdrv_add_key(bs, password, local_err);
+if (local_err) {
+monitor_printf(mon, %s\n, error_get_pretty(local_err));
+error_free(local_err);
 ret = -EPERM;
 }
 if (mon-password_completion_cb)
@@ -5386,17 +5389,20 @@ int monitor_read_bdrv_key_start(Monitor *mon, 
BlockDriverState *bs,
 BlockCompletionFunc *completion_cb,
 void *opaque)
 {
+Error *local_err = NULL;
 int err;
 
-if (!bdrv_key_required(bs)) {
+bdrv_add_key(bs, NULL, local_err);
+if (!local_err) {
 if (completion_cb)
 completion_cb(opaque, 0);
 return 0;
 }
 
+/* Need a key for @bs */
+
 if (monitor_ctrl_mode(mon)) {
-

Re: [Qemu-devel] [RFC PATCH 2/4] pcie-aer: Fix command pcie_aer_inject_error is invalid

2015-01-14 Thread Chen Fan


On 01/12/2015 09:56 PM, Marcel Apfelbaum wrote:

On 01/12/2015 05:04 AM, Chen Fan wrote:

in spec PCI Express 3.0 section 6.2.6 Figure 6-3 virtual bridge part,
the flowchart showing tell us SERR# enable at Bridge Control register
associate with system error at Secondary Status register can send error
message. but bridge_control from dev-config is NULL, and SERR# was set
in dev-wmask in pcie_aer_init()

wmask denotes the register bits that can be written by the guest.

If you are referring to:
   pci_word_test_and_set_mask(dev-wmask + PCI_BRIDGE_CONTROL,
  PCI_BRIDGE_CTL_SERR);
that means that the OS *is able* to turn on/off SERR forwarding.


 which was implemented by root port and
swith devices, so we should add wmask (for w/r) bit set for bridge 
control.

we can user command like:
qemu_system_x86_64:
-device ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,id=bridge1
-device x3130-upstream,bus=bridge1,id=up.1,addr=00.0
-device xio3130-downstream,bus=up.1,id=down.1,port=1,addr=00.0,chassis=5

(qemu) pcie_aer_inject_error net0 POISON_TLP

after that,
guest can output the error message.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
---
  hw/pci/pcie_aer.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 7ca077a..571dc92 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -231,7 +231,8 @@ pcie_aer_msg_alldev(PCIDevice *dev, const 
PCIEAERMsg *msg)

   */
  static bool pcie_aer_msg_vbridge(PCIDevice *dev, const PCIEAERMsg 
*msg)

  {
-uint16_t bridge_control = pci_get_word(dev-config + 
PCI_BRIDGE_CONTROL);
Here we check if the Guest OS/firmware actually turned the #SERR 
forwarding on.


+uint16_t bridge_control = pci_get_word(dev-config + 
PCI_BRIDGE_CONTROL) |
+  pci_get_word(dev-wmask + 
PCI_BRIDGE_CONTROL);

I don't think that this check is correct given the above comments.
Please correct me if I mislead you,

after sweep the code again,  I think you are right.

And for pcie spec  6.2.5 chapter. I think we should add a check for
validating the Fatal Error Reporting Enable bit in Device Control register
or whether #SERR is enabled either.

Thanks,
Chen




Thanks,
Marcel




  if (pcie_aer_msg_is_uncor(msg)) {
  /* Received System Error */



.






Re: [Qemu-devel] [PATCH v2 00/12] block/dmg: (compatibility) fixes and bzip2 support

2015-01-14 Thread Stefan Hajnoczi
On Tue, Jan 06, 2015 at 06:48:03PM +0100, Peter Wu wrote:
 Hi,
 
 This is the second revision of improvements to DMG image file support.
 See [1] for an overview of the previous patchset.
 
 Thanks to John Snow for his efforts in reviewing patches and providing
 suggestions. The errp suggestion from Stefan Hajnoczi is also
 incorporated.
 
 An overview of changes since v1 (also mentioned in each patch):
 
   block/dmg: properly detect the UDIF trailer [+R-b, set errp)
   block/dmg: extract mish block decoding functionality [+R-b, added
 comments, expanded commit message, renamed var]
   block/dmg: extract processing of resource forks [see patch]
   block/dmg: process a buffer instead of reading ints [+R-b, no changes]
   block/dmg: validate chunk size to avoid overflow [added offset check]
   block/dmg: process XML plists [added offset check]
   block/dmg: set virtual size to a non-zero value [fix commit message]
   block/dmg: fix sector data offset calculation [use data provided by file]
   block/dmg: use SectorNumber from BLKX header [new patch]
   block/dmg: factor out block type check [extracted from bzip patch]
   block/dmg: support bzip2 block entry types [set/use BZIP2_LIBS]
   block/dmg: improve zeroes handling [no changes]
 
 (the other length checking patch was squashed into the others)
 
 Note: in the previous patches I mentioned that dmg2img would result in a
 shorter file than qemu-img convert. That turns out to be a bug in
 dmg2img for which a patch is available[2].
 
 A quick test runner is available at
 https://lekensteyn.nl/files/dmg-tests/. This script depends on the fixed
 dmg2img program and can then be run as follows:
 
 QEMU_IMG=/tmp/qout/qemu-img ./run-dmg-tests.sh dmg-images/*.dmg
 
 You will first need some DMG files, I have collected four different
 public examples with different properties[1]. These can be found in
 urls.txt at https://lekensteyn.nl/files/dmg-tests/dmg-images/.
 
 Kind regards,
 Peter
 
  [1]: https://lists.nongnu.org/archive/html/qemu-devel/2014-12/msg03606.html
  [2]: 
 https://github.com/Lekensteyn/dmg2img/commit/a1d4861b4b0f2ac5090938233a1156bb130cb3ef
 
 Peter Wu (12):
   block/dmg: properly detect the UDIF trailer
   block/dmg: extract mish block decoding functionality
   block/dmg: extract processing of resource forks
   block/dmg: process a buffer instead of reading ints
   block/dmg: validate chunk size to avoid overflow
   block/dmg: process XML plists
   block/dmg: set virtual size to a non-zero value
   block/dmg: fix sector data offset calculation
   block/dmg: use SectorNumber from BLKX header
   block/dmg: factor out block type check
   block/dmg: support bzip2 block entry types
   block/dmg: improve zeroes handling
 
  block/Makefile.objs |   1 +
  block/dmg.c | 503 
 
  configure   |  31 
  3 files changed, 418 insertions(+), 117 deletions(-)

Hard to verify this does not introduce regressions since qemu-iotests
does not support dmg.  The code looks good though.

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

Stefan


pgpq4G036vUmV.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 01/12] block/dmg: properly detect the UDIF trailer

2015-01-14 Thread Stefan Hajnoczi
On Wed, Jan 07, 2015 at 03:19:13PM +0100, Peter Wu wrote:
 On Wednesday 07 January 2015 13:19:34 Stefan Hajnoczi wrote:
  On Tue, Jan 06, 2015 at 06:48:04PM +0100, Peter Wu wrote:
   DMG files have a variable length with a UDIF trailer at the end of a
   file. This UDIF trailer is essential as it describes the contents of
   the image. At the moment however, the start of this trailer is almost
   always incorrect as bdrv_getlength() returns a multiple of the block
   size (rounded up). This results in a failure to recognize DMG files,
   resulting in Invalid argument (EINVAL) errors.
   
   As there is no API to retrieve the real file size, look for the magic
   header in the last two sectors to find the start of this 512-byte UDIF
   trailer (the koly block).
   
   The resource fork offset (info_begin) has its offset adjusted as the
   initial value of offset does not mean end of file anymore, but begin
   of UDIF trailer.
   
   Signed-off-by: Peter Wu pe...@lekensteyn.nl
   Reviewed-by: John Snow js...@redhat.com
   ---
v2: added R-b, set errp as suggested by Stefan Hajnoczi
   ---
block/dmg.c | 49 +
1 file changed, 45 insertions(+), 4 deletions(-)
   
   diff --git a/block/dmg.c b/block/dmg.c
   index e455886..9183459 100644
   --- a/block/dmg.c
   +++ b/block/dmg.c
   @@ -131,6 +131,48 @@ static void update_max_chunk_size(BDRVDMGState *s, 
   uint32_t chunk,
}
}

   +static int64_t dmg_find_koly_offset(BlockDriverState *file_bs, Error 
   **errp)
   +{
   +int64_t length;
   +int64_t offset = 0;
   +uint8_t buffer[515];
   +int i, ret;
   +
   +/* bdrv_getlength returns a multiple of block size (512), rounded 
   up. Since
   + * dmg images can have odd sizes, try to look for the koly magic 
   which
   + * marks the begin of the UDIF trailer (512 bytes). This magic can 
   be found
   + * in the last 511 bytes of the second-last sector or the first 4 
   bytes of
   + * the last sector (search space: 515 bytes) */
   +length = bdrv_getlength(file_bs);
   +if (length  0) {
   +error_setg_errno(errp, -length,
   +Failed to get file size while reading UDIF trailer);
   +return length;
   +} else if (length  512) {
   +error_set(errp, ERROR_CLASS_GENERIC_ERROR,
   +dmg file must be at least 512 bytes long);
   +return -EINVAL;
  
  Not worth respinning but error_setg() is a shortcut for error_set(errp,
  ERROR_CLASS_GENERIC_ERROR, fmt, ...).
  
  Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
 
 Good to know, I got a compile error when ERROR_CLASS_GENERIC_ERROR was
 omitted, didn't think of using error_setg instead.
 
 When merging, please replace the above ERROR_CLASS_GENERIC_ERROR by:
 
 error_setg(errp, dmg file must be at least 512 bytes long);
 
 (and likewise for the last error_set in the function)

Done!

Stefan


pgpCtc_7Qv7iA.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH 17/19] block/parallels: delay writing to BAT till bdrv_co_flush_to_os

2015-01-14 Thread Denis V. Lunev

On 14/01/15 17:29, Denis V. Lunev wrote:

On 14/01/15 16:34, Roman Kagan wrote:

On Wed, Jan 14, 2015 at 04:08:50PM +0300, Denis V. Lunev wrote:

On 14/01/15 16:03, Roman Kagan wrote:

On Tue, Dec 30, 2014 at 01:07:10PM +0300, Denis V. Lunev wrote:
+static int cache_bat(BlockDriverState *bs, uint32_t idx, uint32_t 
new_data_off)

+{
+int ret, i, off, cache_off;
+int64_t first_idx, last_idx;
+BDRVParallelsState *s = bs-opaque;
+uint32_t *cache = s-bat_cache;
+
+off = bat_offset(idx);
+cache_off = (off / s-bat_cache_size) * s-bat_cache_size;
+
+if (s-bat_cache_off != -1  s-bat_cache_off != cache_off) {
+ret = write_bat_cache(bs);
+if (ret  0) {
+return ret;
+}
+}
+
+first_idx = idx - (off - cache_off) / sizeof(uint32_t);
+last_idx = first_idx + s-bat_cache_size / sizeof(uint32_t);
+if (first_idx  0) {
+memcpy(s-bat_cache, s-ph, sizeof(s-ph));
+first_idx = 0;
+cache = s-bat_cache + sizeof(s-ph) / sizeof(uint32_t);
+}
+
+if (last_idx  s-bat_size) {
+memset(cache + s-bat_size - first_idx, 0,
+   sizeof(uint32_t) * (last_idx - s-bat_size));
+}
+
+for (i = 0; i  last_idx - first_idx; i++) {
+cache[i] = cpu_to_le32(s-bat[first_idx + i]);
+}

You're re-populating the bat_cache on every bat update. Why?

Shouldn't this be done only with empty cache, i.e. under
if (s-bat_cache_off == -1)?

reasonable, but condition should be a bit different, namely

if (s-bat_cache_off != -1  s-bat_cache_off != cache_off) {

No, this is the condition to flush the cache which you already do.

Then, either upon flushing it or on the first entry into cache_bat(),
the cache is empty which is indicated by -bat_cache_off == -1, at which
point you need to populate it from -bat.


you are wrong. BAT cache is a single page of the whole BAT
which can be more than several MBs. This page should be
repopulated when the off is changed.

ok, you are spoken about write_bat_cache which effectively marks
cache as invalid. Thus we are both spoken about the same
condition and we are on the same page.



Re: [Qemu-devel] cpu hotplug and windows guest (win2012r2)

2015-01-14 Thread Andrey Korolyov
On Fri, Jan 9, 2015 at 4:35 PM, Andrey Korolyov and...@xdel.ru wrote:
 On Fri, Jan 9, 2015 at 1:26 PM, Alexandre DERUMIER aderum...@odiso.com 
 wrote:
 Hi,

 I'm currently testing cpu hotplug with a windows 2012R2 standard guest,

 and I can't get it too work. (works fine with linux guest).

 host kernel : rhel7 3.10 kernel
 qemu 2.2


 qemu command line : -smp cpus=1,sockets=2,cores=1,maxcpus=2

 Started with 1cpu, topogoly is 2sockets with 1cores.


 Then

 qmp# cpu-add 1


 I can see a new cpu is windows device manager, and event log in the device 
 said that it's online.

 So it should be ok, but.

 I can't see new processor in taskmanager or perfmon. (I had tried to 
 relaunch them to be sure.).


 So, it is a windows bug ? Does I need to do something else ?


 Did you populated appropriate topology in arguments? As far as I can
 remember from pre-1.1 era CPU hotplug not worked with windows, so it
 should be neither lack of configured NUMA or a simply OS-specific
 issue.

Just to let anyone know that it works well:

http://xdel.ru/downloads/windows-hotplugged-cpu.png

Topology is a bit rotten (I have two sockets with single CPU in each)
and I *need* to specify non-zero amount of memory for each numa node
to be seen, if each CPU belongs to different node for example but
everything else is just fine.



Re: [Qemu-devel] Fedora FC21 - Bug: 100% CPU and hangs in gettimeofday(tp, NULL); forever

2015-01-14 Thread Juan Quintela
Gerhard Wiesinger li...@wiesinger.com wrote:
 On 12.01.2015 12:41, Gerhard Wiesinger wrote:
 On 08.01.2015 23:28, Gerhard Wiesinger wrote:

 I'll keep you up to date in the next days whether it happens again
 or not.

 With qemu-kvm 2.2.0 release from the above repository the 100% usage
 didn't happen so far (although I had to reboot after kernel update).

 It happens also with qemu-kvm 2.2.0 on another VM where also
 PostgreSQL is running:
 (gdb) bt
 #0  0x7fff9a1feff4 in gettimeofday ()
 #1  0x006d425e in GetCurrentTimestamp () at timestamp.c:1274

 What we know:
 OK : F20: 3.17.6-200.fc20.x86_64 on guest/host,
 qemu-kvm-1.6.2-10.fc20.x86_64 on host
 NOK: F21: 3.17.7-300.fc21.x86_64 on guest/host,
 qemu-kvm-2.1.2-7.fc21.x86_64 on host
 NOK: F21: 3.17.8-300.fc21.x86_64 on guest/host,
 qemu-kvm-2.2.0-1.fc21.x86_64 on host

 No one less can reproduce or has similar problems?

I have similar problems, F21 guest on F21 host.
https://bugzilla.redhat.com/show_bug.cgi?id=1174664

For me, it is firefox that from time to time loops on vclock_gettime().
At that point the DSO have got corrupted (kvmclock msr's), and as you
say, only solution is reboot.  Could you look at the bug, and see if the
areas pointed by the MSR also got corrupted?  I tried using kernel-debug
with debugging for memory allocation (marcelo request).  I see pattens
like 0x5a5a5a5a, so clearly there is some corruption, but haven't been
able to find _what_ is making that happen.

 Any further ideas?

Nope :-(

 BTW: I'm running ntp in the following manner: internet = ntp server
 in VM = ntp client on KVM host (firewall runs in KVM)

I am uisng ntp on the host against Internet, but the guest don't use ntp
(I do a ntpdate if I see that the guest has drifted too much, and it is
a rare event.  Right now I don't rememeber having fixed it lately).

Later, Juan.



Re: [Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend

2015-01-14 Thread Bastian Koppelmann


On 01/14/2015 03:36 PM, Lluís Vilanova wrote:

Bastian Koppelmann writes:


There is no overview, that shows all the frontend operation one can use, as
found on the wiki. Unfortunatly the wiki is out of date, so let's try to move
this documentation to the source files, which has the benefit, that it is easy
to update the documentation, if the frontend is changed. This patch adds doxygen
tags to all the 32 bit versions of the tcg frontend operations, because the
64 bit version would mostly have the same documentation, and all the type
conversition operations. The file tag has a note, that makes the user aware of
the missing 64 operations. In this version all the immediate variants are also
documented by simply refering to the non immediate version. However I'm willing
to drop that.
Any comments?

The operations (or at least most of them) are already documented in
tcg/README. If this change is accepted, I'd rather move the contents of the
README file into here.
The README file contains IIRC only operation, which are also represented 
by the intermediate format of tcg. Unfortunately this excludes useful 
helpers like subfi. So Richard suggested commenting the header file. 
However I wouldn't add only those helpers like subfi to the doxygen 
documentation, since the idea is to have a good overview. If you have 
another suggestion I'm happy to hear it :).



Also, AFAIR it was decided to use gtk-doc instead of doxygen.

No problem, I could change that.

Cheers,
Bastian


Cheers,
   Lluis



Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
---
  tcg/tcg-op.h | 477 ++-
  1 file changed, 436 insertions(+), 41 deletions(-)
diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 96adf9a..5bb7e65 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -26,6 +26,29 @@
  #include exec/helper-proto.h
  #include exec/helper-gen.h
  

+/**
+  * @file tcg-op.h
+  * @brief These are the supported operations as implemented by the TCG 
frontend
+  *for the target cpu (what QEMU executes; not where QEMU executes).
+  *This information is useful for people who want to port QEMU to
+  *emulate a new processor.
+  *The frontend helpers for generating TCG operations typically take 
the
+  *form: tcg_gen_op[i]_reg_size.
+  * - The op is the TCG operation that will be generated for its
+  *   arguments
+  * - The [i] suffix is used to indicate the TCG operation takes an
+  *   immediate rather than a normal register.
+  * - The reg_size refers to the size of the TCG registers in use.
+  *   The vast majority of the time, this will match the native size of
+  *   the emulated target, so rather than force people to type i32 or
+  *   i64 all the time, the shorthand tl is made available for all
+  *   helpers.
+  * @note  This documentation only covers the 32 bit operations. If not stated
+  *otherwise, every operation is also available as a 64 bit operation.
+  *If no suffix reg_size is given, the operation is available for 32
+  *and 64 bit guests, e.g. tcg_gen_br.
+  */
+
  /* Basic output routines.  Not for general consumption.  */
  

  void tcg_gen_op1(TCGContext *, TCGOpcode, TCGArg);
@@ -251,13 +274,24 @@ static inline void tcg_gen_op6ii_i64(TCGOpcode opc, 
TCGv_i64 a1, TCGv_i64 a2,
  

  /* Generic ops.  */
  

+/**
+  * Create a new label, used for branch instructions.
+  * \sa tcg_gen_br
+  */
  int gen_new_label(void);
-
+/**
+  * Label the current location with label n, so branch instructions can jump
+  * there.
+  * \sa tcg_gen_br, gen_new_label
+  */
  static inline void gen_set_label(int n)
  {
  tcg_gen_op1(tcg_ctx, INDEX_op_set_label, n);
  }
-
+/**
+  * Jump to a label.
+  * \sa gen_new_label, gen_set_label
+  */
  static inline void tcg_gen_br(int label)
  {
  tcg_gen_op1(tcg_ctx, INDEX_op_br, label);
@@ -267,163 +301,386 @@ static inline void tcg_gen_br(int label)
  /* Helper calls. */
  

  /* 32 bit ops */
-
+/**
+  * \sa tcg_gen_add_i32
+  */
  void tcg_gen_addi_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
+/**
+  * Subtracts  32 bit register arg2 from constant arg1.
+  * \sa tcg_gen_sub_i32
+  */
  void tcg_gen_subfi_i32(TCGv_i32 ret, int32_t arg1, TCGv_i32 arg2);
+/**
+  * \sa tcg_gen_sub_i32
+  */
  void tcg_gen_subi_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
+/**
+  * \sa tcg_gen_and_i32
+  */
  void tcg_gen_andi_i32(TCGv_i32 ret, TCGv_i32 arg1, uint32_t arg2);
+/**
+  * \sa tcg_gen_or_i32
+  */
  void tcg_gen_ori_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
+/**
+  * \sa tcg_gen_xor_i32
+  */
  void tcg_gen_xori_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
+/**
+  * \sa tcg_gen_shl_i32
+  */
  void tcg_gen_shli_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
+/**
+  * \sa tcg_gen_shr_i32
+  */
  void tcg_gen_shri_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
+/**
+  * \sa tcg_gen_sar_i32
+  

Re: [Qemu-devel] Fedora FC21 - Bug: 100% CPU and hangs in gettimeofday(tp, NULL); forever

2015-01-14 Thread Juan Quintela
Juan Quintela quint...@redhat.com wrote:
 Gerhard Wiesinger li...@wiesinger.com wrote:
 On 12.01.2015 12:41, Gerhard Wiesinger wrote:
 On 08.01.2015 23:28, Gerhard Wiesinger wrote:

 I'll keep you up to date in the next days whether it happens again
 or not.

 With qemu-kvm 2.2.0 release from the above repository the 100% usage
 didn't happen so far (although I had to reboot after kernel update).

 It happens also with qemu-kvm 2.2.0 on another VM where also
 PostgreSQL is running:
 (gdb) bt
 #0  0x7fff9a1feff4 in gettimeofday ()
 #1  0x006d425e in GetCurrentTimestamp () at timestamp.c:1274

 What we know:
 OK : F20: 3.17.6-200.fc20.x86_64 on guest/host,
 qemu-kvm-1.6.2-10.fc20.x86_64 on host
 NOK: F21: 3.17.7-300.fc21.x86_64 on guest/host,
 qemu-kvm-2.1.2-7.fc21.x86_64 on host
 NOK: F21: 3.17.8-300.fc21.x86_64 on guest/host,
 qemu-kvm-2.2.0-1.fc21.x86_64 on host

 No one less can reproduce or has similar problems?

 I have similar problems, F21 guest on F21 host.
 https://bugzilla.redhat.com/show_bug.cgi?id=1174664

 For me, it is firefox that from time to time loops on vclock_gettime().
 At that point the DSO have got corrupted (kvmclock msr's), and as you
 say, only solution is reboot.  Could you look at the bug, and see if the
 areas pointed by the MSR also got corrupted?  I tried using kernel-debug
 with debugging for memory allocation (marcelo request).  I see pattens
 like 0x5a5a5a5a, so clearly there is some corruption, but haven't been
 able to find _what_ is making that happen.

 Any further ideas?

 Nope :-(

 BTW: I'm running ntp in the following manner: internet = ntp server
 in VM = ntp client on KVM host (firewall runs in KVM)

 I am uisng ntp on the host against Internet, but the guest don't use ntp
 (I do a ntpdate if I see that the guest has drifted too much, and it is
 a rare event.  Right now I don't rememeber having fixed it lately).

I forgot tell on the previous patch, I am using 2vcpus.  with a single
vcpu I have been unable to trigger this bug.

Later, Juan.



Re: [Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend

2015-01-14 Thread Bastian Koppelmann


On 01/14/2015 04:55 PM, Thomas Huth wrote:

On Wed, 14 Jan 2015 16:36:26 +0100
Lluís Vilanova vilan...@ac.upc.edu wrote:
...

Also, AFAIR it was decided to use gtk-doc instead of doxygen.

If there's a consensus about which source code documentation style
should be used for QEMU, could you (or somebody else) maybe add an
appropriate paragraph to the CODING_STYLE file to avoid future
confusion?
I can do that. If Richard is okay with the approach of this patch, I 
will send it in the next round.


Cheers,
Bastian


  Thomas






Re: [Qemu-devel] [PATCH 19/19] block/parallels: optimize linear image expansion

2015-01-14 Thread Roman Kagan
On Tue, Dec 30, 2014 at 01:07:12PM +0300, Denis V. Lunev wrote:
 Plain image expansion spends a lot of time to update image file size.
 This seriously affects the performance. The following simple test
   qemu_img create -f parallels -o cluster_size=64k ./1.hds 64G
   qemu_io -n -c write -P 0x11 0 1024M ./1.hds
 could be improved if the format driver will pre-allocate some space
 in the image file with a reasonable chunk.
 
 This patch preallocates 128 Mb using bdrv_write_zeroes, which should
 normally use fallocate() call inside. Fallback to older truncate()
 could be used as a fallback using image open options thanks to the
 previous patch.
 
 The benefit is around 15%.
 
 This patch is final in this series. Block driver has near native
 performance now.
 
 Signed-off-by: Denis V. Lunev d...@openvz.org
 CC: Kevin Wolf kw...@redhat.com
 CC: Stefan Hajnoczi stefa...@redhat.com
 ---
  block/parallels.c | 35 +--
  1 file changed, 33 insertions(+), 2 deletions(-)
 
 diff --git a/block/parallels.c b/block/parallels.c
 index 12a9cea..5ec4a0d 100644
 --- a/block/parallels.c
 +++ b/block/parallels.c
 @@ -82,6 +82,7 @@ typedef struct BDRVParallelsState {
  int bat_cache_off;
  int data_off;
  
 +int64_t  prealloc_off;

This field name confused me.  I think data_end would fit better.

Otherwise looks good to me.

Roman.



Re: [Qemu-devel] [PATCH 19/19] block/parallels: optimize linear image expansion

2015-01-14 Thread Denis V. Lunev

On 14/01/15 20:56, Roman Kagan wrote:

On Tue, Dec 30, 2014 at 01:07:12PM +0300, Denis V. Lunev wrote:

Plain image expansion spends a lot of time to update image file size.
This seriously affects the performance. The following simple test
   qemu_img create -f parallels -o cluster_size=64k ./1.hds 64G
   qemu_io -n -c write -P 0x11 0 1024M ./1.hds
could be improved if the format driver will pre-allocate some space
in the image file with a reasonable chunk.

This patch preallocates 128 Mb using bdrv_write_zeroes, which should
normally use fallocate() call inside. Fallback to older truncate()
could be used as a fallback using image open options thanks to the
previous patch.

The benefit is around 15%.

This patch is final in this series. Block driver has near native
performance now.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
---
  block/parallels.c | 35 +--
  1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 12a9cea..5ec4a0d 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -82,6 +82,7 @@ typedef struct BDRVParallelsState {
  int bat_cache_off;
  int data_off;
  
+int64_t  prealloc_off;

This field name confused me.  I think data_end would fit better.

Otherwise looks good to me.

Roman.

agree



Re: [Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend

2015-01-14 Thread Bastian Koppelmann


On 01/14/2015 03:51 PM, Bastian Koppelmann wrote:

There is no overview, that shows all the frontend operation one can use, as
found on the wiki. Unfortunatly the wiki is out of date, so let's try to move
this documentation to the source files, which has the benefit, that it is easy
to update the documentation, if the frontend is changed. This patch adds doxygen
tags to all the 32 bit versions of the tcg frontend operations, because the
64 bit version would mostly have the same documentation, and all the type
conversition operations. The file tag has a note, that makes the user aware of
the missing 64 operations. In this version all the immediate variants are also
documented by simply refering to the non immediate version. However I'm willing
to drop that.

Any comments?

Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de

I forgot to mention. This patch depends on Richards pull-request 
https://patchwork.ozlabs.org/patch/427250/


Cheers,
Bastian



Re: [Qemu-devel] [PATCH v6] block/raw-posix.c: Fixes raw_getlength() on Mac OS X so that it reports the correct length of a real CD

2015-01-14 Thread Peter Maydell
On 13 January 2015 at 20:07, Programmingkid programmingk...@gmail.com wrote:
 Allows QEMU on Mac OS X to use a real cdrom again.

 Signed-off-by: John Arbuckle programmingk...@gmail.com

 ---
 Added fallback code - uses lseek() if ioctl() fails.

  block/raw-posix.c |   25 -
  1 files changed, 24 insertions(+), 1 deletions(-)

 diff --git a/block/raw-posix.c b/block/raw-posix.c
 index e51293a..5815707 100644
 --- a/block/raw-posix.c
 +++ b/block/raw-posix.c
 @@ -1312,7 +1312,30 @@ again:
  if (size == 0)
  #endif
  #if defined(__APPLE__)  defined(__MACH__)
 -size = LLONG_MAX;
 +{
 +uint64_t sectors = 0;
 +uint32_t sector_size = 0;
 +bool ioctl_problem = false;
 +ret = 0;
 +
 +/* Query the number of sectors on the disk */
 +ret = ioctl(fd, DKIOCGETBLOCKCOUNT, sectors);
 +if (ret != 0)
 +ioctl_problem = true;
 +
 +/* Query the size of each sector */
 +ret = ioctl(fd, DKIOCGETBLOCKSIZE, sector_size);
 +if (ret != 0)
 +ioctl_problem = true;
 +
 +/* If everything is ok */
 +if (ioctl_problem == false)
 +size = sectors * sector_size;
 +
 +/* If a problem occurred with ioctl(), fallback to lseek() */
 +else
 +size = lseek(fd, 0LL, SEEK_END);
 +}
  #else

This fixes the make check problem, but you're not catching
the possibility of lseek failing. (Also the if() statements
are all missing braces our coding style requires.) You can
avoid that bool flag like this:

{
uint64_t sectors = 0;
uint32_t sector_size = 0;

if (ioctl(fd, DKIOCGETBLOCKCOUNT, sectors) == 0
 ioctl(fd, DKIOCGETBLOCKSIZE, sector_size) == 0) {
size = sectors * sector_size;
} else {
size = lseek(fd, 0LL, SEEK_END);
if (size  0) {
return -errno;
}
}
}

thanks
-- PMM



Re: [Qemu-devel] [PATCH] Fixes several full screen issues on Mac OS X

2015-01-14 Thread Peter Maydell
On 4 January 2015 at 23:44, Programmingkid programmingk...@gmail.com wrote:
 This patch makes several changes:
 - Fixes issue of returning to window mode and QEMU not setting
  the right graphic settings if there was a change during full screen mode.
 - Eliminated distorted full screen display.
 - Makes full screen mode available on Mac OS 10.7 and higher.
 - Removes unneeded global variables cdx, and cdy.
 - Changes a few comments to make them clearer.

 Signed-off-by: John Arbuckle programmingk...@gmail.com

I'm afraid this patch is badly broken for me.

(1) If I go into full screen then I can't get out again because the
menubar isn't displayed even when I use ctrl+alt to release the
mouse grab. I have to use my second monitor to get at the Dock so I
can kill QEMU.

(2) Having done this I find that all my other application windows
have been squashed down into a corner of my screen, presumably because
we've told MacOSX the screen is 640x480 and it's rearranged the
app windows to suit. We mustn't mess things up like this.

(3) I managed to get at the underlying QEMU window with its title
bar somehow even when in full screen mode: I could move it about the
screen with the mouse...

(4) I get a lot of compile warnings for this patch:

  OBJC  ui/cocoa.o
/Users/pm215/src/qemu/ui/cocoa.m:790:24: warning:
'CGDisplayCurrentMode' is deprecated: first deprecated in OS X 10.6
  [-Wdeprecated-declarations]
window_mode_dict = CGDisplayCurrentMode(kCGDirectMainDisplay);
   ^
/System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:455:27:
note: 'CGDisplayCurrentMode' has been explicitly
  marked deprecated here
CG_EXTERN CFDictionaryRef CGDisplayCurrentMode(CGDirectDisplayID display)
  ^
/Users/pm215/src/qemu/ui/cocoa.m:790:22: warning: incompatible pointer
types assigning to 'NSDictionary *' from 'CFDictionaryRef'
  (aka 'const struct __CFDictionary *') [-Wincompatible-pointer-types]
window_mode_dict = CGDisplayCurrentMode(kCGDirectMainDisplay);
 ^ ~~
/Users/pm215/src/qemu/ui/cocoa.m:796:5: warning:
'CGDisplaySwitchToMode' is deprecated: first deprecated in OS X 10.6
  [-Wdeprecated-declarations]
CGDisplaySwitchToMode(kCGDirectMainDisplay, window_mode_dict);
^
/System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:460:19:
note: 'CGDisplaySwitchToMode' has been explicitly
  marked deprecated here
CG_EXTERN CGError CGDisplaySwitchToMode(CGDirectDisplayID display,
  ^
/Users/pm215/src/qemu/ui/cocoa.m:796:49: warning: incompatible pointer
types passing 'NSDictionary *' to parameter of type
  'CFDictionaryRef' (aka 'const struct __CFDictionary *')
[-Wincompatible-pointer-types]
CGDisplaySwitchToMode(kCGDirectMainDisplay, window_mode_dict);
^~~~
/System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:461:19:
note: passing argument to parameter 'mode' here
  CFDictionaryRef mode) CG_AVAILABLE_BUT_DEPRECATED(__MAC_10_0, __MAC_10_6,
  ^
/Users/pm215/src/qemu/ui/cocoa.m:804:20: warning: incompatible pointer
types initializing 'NSDictionary *' with an expression of type
  'CFDictionaryRef' (aka 'const struct __CFDictionary *')
[-Wincompatible-pointer-types]
  ...* mode = CGDisplayBestModeForParameters(kCGDirectMainDisplay,
desired_bit_depth, (size_t)cw, (size_t)ch, exact_match);
   ^  
~
/Users/pm215/src/qemu/ui/cocoa.m:804:27: warning:
'CGDisplayBestModeForParameters' is deprecated: first deprecated in OS
X 10.6
  [-Wdeprecated-declarations]
NSDictionary * mode =
CGDisplayBestModeForParameters(kCGDirectMainDisplay,
desired_bit_depth, (size_t)cw, (size_t)ch, ex...
  ^
/System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:442:27:
note: 'CGDisplayBestModeForParameters' has been
  explicitly marked deprecated here
CG_EXTERN CFDictionaryRef CGDisplayBestModeForParameters(CGDirectDisplayID
  ^
/Users/pm215/src/qemu/ui/cocoa.m:807:9: warning:
'CGDisplaySwitchToMode' is deprecated: first deprecated in OS X 10.6
  [-Wdeprecated-declarations]
CGDisplaySwitchToMode(kCGDirectMainDisplay, mode);
^
/System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:460:19:
note: 'CGDisplaySwitchToMode' has been explicitly
  marked deprecated here
CG_EXTERN CGError CGDisplaySwitchToMode(CGDirectDisplayID display,
  ^
/Users/pm215/src/qemu/ui/cocoa.m:807:53: warning: incompatible pointer
types passing 'NSDictionary *' to parameter of type
  'CFDictionaryRef' (aka 'const struct __CFDictionary *')
[-Wincompatible-pointer-types]
CGDisplaySwitchToMode(kCGDirectMainDisplay, mode);
   

Re: [Qemu-devel] [PATCH] pseries: Limit PCI host bridge index value

2015-01-14 Thread Michael Roth
Quoting David Gibson (2015-01-13 20:33:39)
 pseries guests can have large numbers of PCI host bridges.  To avoid the
 user having to specify a number of different configuration values for every
 one, the device supports an index property which is a shorthand setting
 the various window and configuration addresses from a predefined sensible
 set.
 
 There are some problems with the details at present:
   * The index propery is signed, but negative values will create PCI
 windows below where we expect, potentially colliding with other devices
   * No limit is imposed on the index property and large values can
 translate to extremely large window addresses.  With PCI passthrough in
 particular this can mean we exceed various mapping and physical address
 limits causing the guest host bridge to not work in strange ways.
 
 This patch addresses this, by making index unsigned, and imposing a
 limit.  Currently the limit allows indices from 0..255 which is probably
 enough host bridges for the time being.  It's fairly easy to extend if
 we discover we need more.
 
 Signed-off-by: David Gibson da...@gibson.dropbear.id.au

I think the limit makes sense, but since the check isn't triggered in cases
where 'index' isn't specified and '[io,mem]_win_[size,offset]' are set
explicitly, maybe it makes sense to sanity-check the final calculation for
those values as well? We could actually drop the index limit in that case
(if we decided we wanted to).

But I think it's okay to assume such users know what they're doing in the
meantime, so:

Reviewed-by: Michael Roth mdr...@linux.vnet.ibm.com

 ---
  hw/ppc/spapr_pci.c  | 8 +++-
  include/hw/pci-host/spapr.h | 4 +++-
  2 files changed, 10 insertions(+), 2 deletions(-)
 
 diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
 index 21b95b3..6deeb19 100644
 --- a/hw/ppc/spapr_pci.c
 +++ b/hw/ppc/spapr_pci.c
 @@ -501,6 +501,12 @@ static void spapr_phb_realize(DeviceState *dev, Error 
 **errp)
  return;
  }
 
 +if (sphb-index  SPAPR_PCI_MAX_INDEX) {
 +error_setg(errp, \index\ for PAPR PHB is too large (max %u),
 +   SPAPR_PCI_MAX_INDEX);
 +return;
 +}
 +
  sphb-buid = SPAPR_PCI_BASE_BUID + sphb-index;
  sphb-dma_liobn = SPAPR_PCI_BASE_LIOBN + sphb-index;
 
 @@ -669,7 +675,7 @@ static void spapr_phb_reset(DeviceState *qdev)
  }
 
  static Property spapr_phb_properties[] = {
 -DEFINE_PROP_INT32(index, sPAPRPHBState, index, -1),
 +DEFINE_PROP_UINT32(index, sPAPRPHBState, index, -1),
  DEFINE_PROP_UINT64(buid, sPAPRPHBState, buid, -1),
  DEFINE_PROP_UINT32(liobn, sPAPRPHBState, dma_liobn, -1),
  DEFINE_PROP_UINT64(mem_win_addr, sPAPRPHBState, mem_win_addr, -1),
 diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
 index 4ea2a0d..876ecf0 100644
 --- a/include/hw/pci-host/spapr.h
 +++ b/include/hw/pci-host/spapr.h
 @@ -64,7 +64,7 @@ typedef struct spapr_pci_msi_mig {
  struct sPAPRPHBState {
  PCIHostState parent_obj;
 
 -int32_t index;
 +uint32_t index;
  uint64_t buid;
  char *dtbusname;
 
 @@ -94,6 +94,8 @@ struct sPAPRPHBVFIOState {
  int32_t iommugroupid;
  };
 
 +#define SPAPR_PCI_MAX_INDEX  255
 +
  #define SPAPR_PCI_BASE_BUID  0x8002000ULL
 
  #define SPAPR_PCI_WINDOW_BASE0x100ULL
 -- 
 2.1.0




Re: [Qemu-devel] [PATCH 6/9] block-migration: tiny refactoring

2015-01-14 Thread John Snow



On 01/14/2015 07:26 AM, Vladimir Sementsov-Ogievskiy wrote:

On 09.01.2015 00:23, John Snow wrote:



On 12/11/2014 09:17 AM, Vladimir Sementsov-Ogievskiy wrote:

Add blk_create and blk_free to remove code duplicates. Otherwise,
duplicates will rise in the following patches because of BlkMigBlock
sturcture extendin.

Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@parallels.com
---
  block-migration.c | 56
+--
  1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 5b4aa0f..d0c825f 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -113,6 +113,30 @@ static void blk_mig_unlock(void)
  qemu_mutex_unlock(block_mig_state.lock);
  }

+/* Only allocating and initializing structure fields, not copying
any data. */
+
+static BlkMigBlock *blk_create(BlkMigDevState *bmds, int64_t sector,
+int nr_sectors)
+{
+BlkMigBlock *blk = g_new(BlkMigBlock, 1);
+blk-buf = g_malloc(BLOCK_SIZE);
+blk-bmds = bmds;
+blk-sector = sector;
+blk-nr_sectors = nr_sectors;
+
+blk-iov.iov_base = blk-buf;
+blk-iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
+qemu_iovec_init_external(blk-qiov, blk-iov, 1);
+
+return blk;
+}
+
+static void blk_free(BlkMigBlock *blk)
+{
+g_free(blk-buf);
+g_free(blk);
+}
+
  /* Must run outside of the iothread lock during the bulk phase,
   * or the VM will stall.
   */
@@ -285,15 +309,7 @@ static int mig_save_device_bulk(QEMUFile *f,
BlkMigDevState *bmds)
  nr_sectors = total_sectors - cur_sector;
  }

-blk = g_new(BlkMigBlock, 1);
-blk-buf = g_malloc(BLOCK_SIZE);
-blk-bmds = bmds;
-blk-sector = cur_sector;
-blk-nr_sectors = nr_sectors;
-
-blk-iov.iov_base = blk-buf;
-blk-iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
-qemu_iovec_init_external(blk-qiov, blk-iov, 1);
+blk = blk_create(bmds, cur_sector, nr_sectors);

  blk_mig_lock();
  block_mig_state.submitted++;
@@ -467,17 +483,9 @@ static int mig_save_device_dirty(QEMUFile *f,
BlkMigDevState *bmds,
  } else {
  nr_sectors = BDRV_SECTORS_PER_DIRTY_CHUNK;
  }
-blk = g_new(BlkMigBlock, 1);
-blk-buf = g_malloc(BLOCK_SIZE);
-blk-bmds = bmds;
-blk-sector = sector;
-blk-nr_sectors = nr_sectors;
+blk = blk_create(bmds, sector, nr_sectors);

  if (is_async) {
-blk-iov.iov_base = blk-buf;
-blk-iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
-qemu_iovec_init_external(blk-qiov, blk-iov, 1);
-


I suppose in the (!is_async) branch we don't reference iov/qiov again,
but the functional difference caught my eye. It used to only be called
under the is_async branch, but now is going to be executed
unconditionally.

Is that fine?

It think it doesn't matter. I can add a parameter 'is_async' to
blk_create(), but what is worse - excess parameter or excess
initialization? And why not to initialize the whole structure in
blk_create() unconditionally?



If it's not a problem, leave it as-is. If I am not sure immediately 
myself, I like to ask questions.


Your answer to the question can always be Yes, that's fine!



Re: [Qemu-devel] [PATCH] s390x/pci: fix 2 bugs found by coverity

2015-01-14 Thread Thomas Huth
On Wed, 14 Jan 2015 16:20:47 +0100
Frank Blaschka blasc...@linux.vnet.ibm.com wrote:

 Signed-off-by: Frank Blaschka blasc...@linux.vnet.ibm.com
 ---
  hw/s390x/s390-pci-bus.c  | 1 +
  hw/s390x/s390-pci-inst.c | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
 index 1201b8d..546dcf1 100644
 --- a/hw/s390x/s390-pci-bus.c
 +++ b/hw/s390x/s390-pci-bus.c
 @@ -192,6 +192,7 @@ static void s390_pci_generate_event(uint8_t cc, uint16_t 
 pec, uint32_t fh,
  object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
 
  if (!s) {
 +g_free(sei_cont);
  return;
  }
 
 diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
 index 5ea13e5..5596679 100644
 --- a/hw/s390x/s390-pci-inst.c
 +++ b/hw/s390x/s390-pci-inst.c
 @@ -787,7 +787,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, 
 uint64_t fiba)
  data = (pbdev-isc  28) | (pbdev-noi  16) |
 (pbdev-routes.adapter.ind_offset  8) | (pbdev-sum  7) |
 pbdev-routes.adapter.summary_offset;
 -stw_p(fib.data, data);
 +stl_p(fib.data, data);
 
  if (pbdev-fh  ENABLE_BIT_OFFSET) {
  fib.fc |= 0x80;

Looks right.

Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com




Re: [Qemu-devel] [PATCH v2] Makes -full-screen option work on Mac OS X

2015-01-14 Thread Peter Maydell
On 12 January 2015 at 16:46, Programmingkid programmingk...@gmail.com wrote:
  This patch makes the -full-screen option actually instruct QEMU to enter 
 fullscreen at startup.

 Signed-off-by: John Arbuckle programmingk...@gmail.com

 ---
 Removed the set_to_full_screen variable.
 Removed the scanForFullScreenOption() function.

  ui/cocoa.m |6 ++
  1 files changed, 6 insertions(+), 0 deletions(-)

 diff --git a/ui/cocoa.m b/ui/cocoa.m
 index 685081e..4cb07ba 100644
 --- a/ui/cocoa.m
 +++ b/ui/cocoa.m
 @@ -1163,6 +1163,12 @@ void cocoa_display_init(DisplayState *ds, int 
 full_screen)
  {
  COCOA_DEBUG(qemu_cocoa: cocoa_display_init\n);

 +/* if fullscreen mode is to be used */
 +if (full_screen == true) {
 +[NSApp activateIgnoringOtherApps: YES];
 +[[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
 +}
 +
  dcl = g_malloc0(sizeof(DisplayChangeListener));

  // register vga output callbacks

This generates a compile warning for me:
/Users/pm215/src/qemu/ui/cocoa.m:1150:55: warning: instance method
'-toggleFullScreen:' not found (return type defaults to 'id')
  [-Wobjc-method-access]
[[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
  ^~~~

thanks
-- PMM



Re: [Qemu-devel] [PATCH] Machine menu patch for Mac OS X

2015-01-14 Thread Peter Maydell
On 13 January 2015 at 01:49, Programmingkid programmingk...@gmail.com wrote:
 This patch adds a Machine menu to QEMU. This menu gives the user the ability 
 to easily work with floppy and CD image files.

 Features:
 Menu items to switch floppy and CD image files.
 Menu items to eject floppy and CD image files.
 Menu item to use /dev/cdrom.
 Verifies with the user before quitting QEMU by displaying a dialog box.

 Signed-off-by: John Arbuckle programmingk...@gmail.com

Hi. I'm afraid I couldn't get this patch to apply -- is it dependent
on one of your other patches?

Some comments below, anyway.

 +/* Pause the guest */
 +- (void)pauseQemu:(id)sender
 +{
 +qmp_stop(NULL);
 +[sender setEnabled: NO];
 +[[[sender menu] itemWithTitle: @Resume] setEnabled: YES];
 +[normalWindow setTitle: @*** Paused ***];
 +}
 +
 +/* Resume running the guest operating system */
 +- (void)resumeQemu: (id) sender
 +{
 +qmp_cont(NULL);
 +[sender setEnabled: NO];
 +[[[sender menu] itemWithTitle: @Pause] setEnabled: YES];
 +[normalWindow setTitle: @QEMU];

Isn't this changing of the title string going to conflict with the
changes that we make for mouse grab/ungrab?

 +}
 +
 +/* Eject the floppy0 disk */
 +- (void)ejectFloppy:(id)sender
 +{
 +Error *err = NULL;
 +qmp_eject(floppy0, false, false, err);
 +handleAnyDeviceErrors(err);
 +}
 +
 +/* Displays a dialog box asking the user to select a floppy image to load */
 +- (void)changeFloppy:(id)sender
 +{
 +NSOpenPanel * open_panel;
 +open_panel = [NSOpenPanel openPanel];
 +[open_panel setCanChooseFiles: YES];
 +[open_panel setAllowsMultipleSelection: NO];
 +if([open_panel runModalForDirectory: nil file: nil] == NSOKButton) {
 +Error *err = NULL;
 +NSString * file = [[open_panel filenames] objectAtIndex: 0];
 +qmp_change_blockdev(floppy0, [file cString], raw, err);
 +handleAnyDeviceErrors(err);
 +}
 +}

You don't know that the machine being emulated has a floppy drive
at all, or that it's called floppy0...

 +
 +// Ejects the cdrom
 +- (void)ejectCdrom:(id)sender
 +{
 +Error *err = NULL;
 +qmp_eject(ide1-cd0, false, false, err);
 +handleAnyDeviceErrors(err);
 +}
 +
 +/* Displays a dialog box asking the user to select a CD image to load */
 +- (void)changeCdrom:(id)sender
 +{
 +NSOpenPanel * open_panel;
 +open_panel = [NSOpenPanel openPanel];
 +[open_panel setCanChooseFiles: YES];
 +[open_panel setAllowsMultipleSelection: NO];
 +if([open_panel runModalForDirectory: nil file: nil] == NSOKButton) {
 +NSString * file = [[open_panel filenames] objectAtIndex: 0];
 +Error *err = NULL;
 +qmp_change_blockdev(ide1-cd0, [file cString], raw, err);
 +handleAnyDeviceErrors(err);
 +}
 +}

Similarly, the CD may not exist in the guest machine or may
not be called ide1-cd0.

 +
 +/* Restarts QEMU */
 +- (void)restartQemu: (id) sender
 +{
 +qemu_system_reset_request();
 +}
 +
 +/* Switches QEMU to use the real cdrom drive */
 +- (void)useRealCdrom: (id) sender
 +{
 +Error *err = NULL;
 +qmp_change_blockdev(ide1-cd0, /dev/cdrom, raw, err);
 +handleAnyDeviceErrors(err);
 +}
 +
 +/* Verifies if the user really wants to quit */
 +- (void)verifyQuit: (id) sender
 +{
 +NSInteger response;
 +response = NSRunAlertPanel(@Quit?, @Are you sure you want to quit?, 
 @Cancel, @Quit, nil);

We don't have an are-you-sure prompt for closing the QEMU window via
the red button, and we don't for the Quit menu option in the GTK UI either...

 +if(response == NSAlertAlternateReturn)
 +exit(EXIT_SUCCESS);

You should use qmp_quit(NULL) rather than just exit().

 +}
 +
  @end


 @@ -1046,7 +1153,7 @@ int main (int argc, const char * argv[]) {
  [menuItem 
 setKeyEquivalentModifierMask:(NSAlternateKeyMask|NSCommandKeyMask)];
  [menu addItemWithTitle:@Show All 
 action:@selector(unhideAllApplications:) keyEquivalent:@]; // Show All
  [menu addItem:[NSMenuItem separatorItem]]; //Separator
 -[menu addItemWithTitle:@Quit QEMU action:@selector(terminate:) 
 keyEquivalent:@q];
 +[menu addItemWithTitle:@Quit QEMU action:@selector(verifyQuit:) 
 keyEquivalent:@q];
  menuItem = [[NSMenuItem alloc] initWithTitle:@Apple action:nil 
 keyEquivalent:@];
  [menuItem setSubmenu:menu];
  [[NSApp mainMenu] addItem:menuItem];
 @@ -1059,6 +1166,24 @@ int main (int argc, const char * argv[]) {
  [menuItem setSubmenu:menu];
  [[NSApp mainMenu] addItem:menuItem];

 +/* Machine menu */
 + menu = [[NSMenu alloc] initWithTitle: @Machine];
 + [menu setAutoenablesItems: NO];
 + [menu addItem: [[[NSMenuItem alloc] initWithTitle: @Pause action: 
 @selector(pauseQemu:) keyEquivalent: @] autorelease]];
 + [menu addItem: [[[NSMenuItem alloc] initWithTitle: @Resume action: 
 @selector(resumeQemu:) keyEquivalent: @] autorelease]];
 + [menu addItem: [NSMenuItem separatorItem]];
 + [menu addItem: 

Re: [Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend

2015-01-14 Thread Thomas Huth
On Wed, 14 Jan 2015 16:36:26 +0100
Lluís Vilanova vilan...@ac.upc.edu wrote:
...
 
 Also, AFAIR it was decided to use gtk-doc instead of doxygen.

If there's a consensus about which source code documentation style
should be used for QEMU, could you (or somebody else) maybe add an
appropriate paragraph to the CODING_STYLE file to avoid future
confusion?

 Thomas




Re: [Qemu-devel] [PATCH 18/19] block/parallels: add prealloc-mode and prealloc-size open paramemets

2015-01-14 Thread Roman Kagan
On Wed, Jan 14, 2015 at 05:31:20PM +0300, Denis V. Lunev wrote:
 On 14/01/15 17:26, Roman Kagan wrote:
 On Tue, Dec 30, 2014 at 01:07:11PM +0300, Denis V. Lunev wrote:
 This is preparational commit for tweaks in Parallels image expansion.
 The idea is that enlarge via truncate by one data block is slow. It
 would be much better to use fallocate via bdrv_write_zeroes and
 expand by some significant amount at once.
 
 This patch just adds proper parameters into BDRVParallelsState and
 performs options parsing in parallels_open.
 
 Signed-off-by: Denis V. Lunev d...@openvz.org
 CC: Kevin Wolf kw...@redhat.com
 CC: Stefan Hajnoczi stefa...@redhat.com
 ---
   block/parallels.c | 72 
  +++
   1 file changed, 72 insertions(+)
 
 diff --git a/block/parallels.c b/block/parallels.c
 index 18b9267..12a9cea 100644
 --- a/block/parallels.c
 +++ b/block/parallels.c
 @@ -30,6 +30,7 @@
   #include qemu-common.h
   #include block/block_int.h
   #include qemu/module.h
 +#include qapi/util.h
   /**/
 @@ -54,6 +55,20 @@ typedef struct ParallelsHeader {
   char padding[12];
   } QEMU_PACKED ParallelsHeader;
 +
 +typedef enum ParallelsPreallocMode {
 +PRL_PREALLOC_MODE_FALLOCATE = 0,
 +PRL_PREALLOC_MODE_TRUNCATE = 1,
 +PRL_PREALLOC_MODE_MAX = 2,
 +} ParallelsPreallocMode;
 +
 +static const char *prealloc_mode_lookup[] = {
 +falloc,
 +truncate,
 +NULL,
 There is already generic preallocaton option, BLOCK_OPT_PREALLOC,
 which is handled by qcow2 and raw-posix.  It means slightly different
 thing: the *whole* image is preallocated using the method specified.
 
 I think it would make sense to consolidate that option with this new
 batched allocation in the generic block code.  I guess qcow2 and
 raw-posix would benefit from it, too.
 
 At any rate I think it's a matter for a separate patchset.
 
 Roman.
 it is too early :) I think that I should provide the rationale for
 the preallocation in general. I am working on this with CEPH :)

OK then, maybe indeed it should land in parallels driver first, and move
into the generic code if found appropriate...

Then my only problem with this patch is the naming of the options: they
look too similar to the generic one while the meaning is different.
Maybe batched_alloc reflects the meaning better?

Roman.



Re: [Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend

2015-01-14 Thread Lluís Vilanova
Bastian Koppelmann writes:

 On 01/14/2015 03:36 PM, Lluís Vilanova wrote:
 Bastian Koppelmann writes:
 
 There is no overview, that shows all the frontend operation one can use, as
 found on the wiki. Unfortunatly the wiki is out of date, so let's try to 
 move
 this documentation to the source files, which has the benefit, that it is 
 easy
 to update the documentation, if the frontend is changed. This patch adds 
 doxygen
 tags to all the 32 bit versions of the tcg frontend operations, because the
 64 bit version would mostly have the same documentation, and all the type
 conversition operations. The file tag has a note, that makes the user aware 
 of
 the missing 64 operations. In this version all the immediate variants are 
 also
 documented by simply refering to the non immediate version. However I'm 
 willing
 to drop that.
 Any comments?
 The operations (or at least most of them) are already documented in
 tcg/README. If this change is accepted, I'd rather move the contents of the
 README file into here.
 The README file contains IIRC only operation, which are also represented by 
 the
 intermediate format of tcg. Unfortunately this excludes useful helpers like
 subfi. So Richard suggested commenting the header file. However I wouldn't add
 only those helpers like subfi to the doxygen documentation, since the idea is 
 to
 have a good overview. If you have another suggestion I'm happy to hear it :).

Sorry I wasn't clear. I meant that it might be better to remove tcg/README and
instead document all the operations (those in the readme and the additional
ones) in the header.


Thanks,
  Lluis

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



Re: [Qemu-devel] [PATCH] Fixes several full screen issues on Mac OS X

2015-01-14 Thread Programmingkid

On Jan 14, 2015, at 12:19 PM, Peter Maydell wrote:

 On 4 January 2015 at 23:44, Programmingkid programmingk...@gmail.com wrote:
 This patch makes several changes:
 - Fixes issue of returning to window mode and QEMU not setting
 the right graphic settings if there was a change during full screen mode.
 - Eliminated distorted full screen display.
 - Makes full screen mode available on Mac OS 10.7 and higher.
 - Removes unneeded global variables cdx, and cdy.
 - Changes a few comments to make them clearer.
 
 Signed-off-by: John Arbuckle programmingk...@gmail.com
 
 I'm afraid this patch is badly broken for me.
 
 (1) If I go into full screen then I can't get out again because the
 menubar isn't displayed even when I use ctrl+alt to release the
 mouse grab. I have to use my second monitor to get at the Dock so I
 can kill QEMU.

You would use Command - F after you have released the mouse grab to return to 
window mode.


 (2) Having done this I find that all my other application windows
 have been squashed down into a corner of my screen, presumably because
 we've told MacOSX the screen is 640x480 and it's rearranged the
 app windows to suit. We mustn't mess things up like this.

This can't be avoided. When the screen resolution changes, all the applications 
usually adjust to the new size. It is the same thing that happens when you use 
a full screen game. 

 
 (3) I managed to get at the underlying QEMU window with its title
 bar somehow even when in full screen mode: I could move it about the
 screen with the mouse...

Really? I think you said you had Mac OS 10.7. I don't have that, but I do have 
access to Mac OS 10.9. Just send me the instructions on how to reproduce this. 

 
 (4) I get a lot of compile warnings for this patch:
 
  OBJC  ui/cocoa.o
 /Users/pm215/src/qemu/ui/cocoa.m:790:24: warning:
 'CGDisplayCurrentMode' is deprecated: first deprecated in OS X 10.6
  [-Wdeprecated-declarations]
window_mode_dict = CGDisplayCurrentMode(kCGDirectMainDisplay);
   ^
 /System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:455:27:
 note: 'CGDisplayCurrentMode' has been explicitly
  marked deprecated here
 CG_EXTERN CFDictionaryRef CGDisplayCurrentMode(CGDirectDisplayID display)
  ^
 /Users/pm215/src/qemu/ui/cocoa.m:790:22: warning: incompatible pointer
 types assigning to 'NSDictionary *' from 'CFDictionaryRef'
  (aka 'const struct __CFDictionary *') [-Wincompatible-pointer-types]
window_mode_dict = CGDisplayCurrentMode(kCGDirectMainDisplay);
 ^ ~~
 /Users/pm215/src/qemu/ui/cocoa.m:796:5: warning:
 'CGDisplaySwitchToMode' is deprecated: first deprecated in OS X 10.6
  [-Wdeprecated-declarations]
CGDisplaySwitchToMode(kCGDirectMainDisplay, window_mode_dict);
^
 /System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:460:19:
 note: 'CGDisplaySwitchToMode' has been explicitly
  marked deprecated here
 CG_EXTERN CGError CGDisplaySwitchToMode(CGDirectDisplayID display,
  ^
 /Users/pm215/src/qemu/ui/cocoa.m:796:49: warning: incompatible pointer
 types passing 'NSDictionary *' to parameter of type
  'CFDictionaryRef' (aka 'const struct __CFDictionary *')
 [-Wincompatible-pointer-types]
CGDisplaySwitchToMode(kCGDirectMainDisplay, window_mode_dict);
^~~~
 /System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:461:19:
 note: passing argument to parameter 'mode' here
  CFDictionaryRef mode) CG_AVAILABLE_BUT_DEPRECATED(__MAC_10_0, __MAC_10_6,
  ^
 /Users/pm215/src/qemu/ui/cocoa.m:804:20: warning: incompatible pointer
 types initializing 'NSDictionary *' with an expression of type
  'CFDictionaryRef' (aka 'const struct __CFDictionary *')
 [-Wincompatible-pointer-types]
  ...* mode = CGDisplayBestModeForParameters(kCGDirectMainDisplay,
 desired_bit_depth, (size_t)cw, (size_t)ch, exact_match);
   ^  
 ~
 /Users/pm215/src/qemu/ui/cocoa.m:804:27: warning:
 'CGDisplayBestModeForParameters' is deprecated: first deprecated in OS
 X 10.6
  [-Wdeprecated-declarations]
NSDictionary * mode =
 CGDisplayBestModeForParameters(kCGDirectMainDisplay,
 desired_bit_depth, (size_t)cw, (size_t)ch, ex...
  ^
 /System/Library/Frameworks/CoreGraphics.framework/Headers/CGDirectDisplay.h:442:27:
 note: 'CGDisplayBestModeForParameters' has been
  explicitly marked deprecated here
 CG_EXTERN CFDictionaryRef CGDisplayBestModeForParameters(CGDirectDisplayID
  ^
 /Users/pm215/src/qemu/ui/cocoa.m:807:9: warning:
 'CGDisplaySwitchToMode' is deprecated: first deprecated in OS X 10.6
  [-Wdeprecated-declarations]
CGDisplaySwitchToMode(kCGDirectMainDisplay, mode);
  

Re: [Qemu-devel] [PATCH v2] Makes -full-screen option work on Mac OS X

2015-01-14 Thread Programmingkid

On Jan 14, 2015, at 12:09 PM, Peter Maydell wrote:

 On 12 January 2015 at 16:46, Programmingkid programmingk...@gmail.com wrote:
 This patch makes the -full-screen option actually instruct QEMU to enter 
 fullscreen at startup.
 
 Signed-off-by: John Arbuckle programmingk...@gmail.com
 
 ---
 Removed the set_to_full_screen variable.
 Removed the scanForFullScreenOption() function.
 
 ui/cocoa.m |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)
 
 diff --git a/ui/cocoa.m b/ui/cocoa.m
 index 685081e..4cb07ba 100644
 --- a/ui/cocoa.m
 +++ b/ui/cocoa.m
 @@ -1163,6 +1163,12 @@ void cocoa_display_init(DisplayState *ds, int 
 full_screen)
 {
 COCOA_DEBUG(qemu_cocoa: cocoa_display_init\n);
 
 +/* if fullscreen mode is to be used */
 +if (full_screen == true) {
 +[NSApp activateIgnoringOtherApps: YES];
 +[[[NSApplication sharedApplication] delegate] toggleFullScreen: 
 nil];
 +}
 +
 dcl = g_malloc0(sizeof(DisplayChangeListener));
 
 // register vga output callbacks
 
 This generates a compile warning for me:
 /Users/pm215/src/qemu/ui/cocoa.m:1150:55: warning: instance method
 '-toggleFullScreen:' not found (return type defaults to 'id')
  [-Wobjc-method-access]
[[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
  ^~~~

I will see what I can do. 


Re: [Qemu-devel] [PATCH v6] block/raw-posix.c: Fixes raw_getlength() on Mac OS X so that it reports the correct length of a real CD

2015-01-14 Thread Programmingkid

On Jan 14, 2015, at 12:02 PM, Peter Maydell wrote:

 On 13 January 2015 at 20:07, Programmingkid programmingk...@gmail.com wrote:
 Allows QEMU on Mac OS X to use a real cdrom again.
 
 Signed-off-by: John Arbuckle programmingk...@gmail.com
 
 ---
 Added fallback code - uses lseek() if ioctl() fails.
 
 block/raw-posix.c |   25 -
 1 files changed, 24 insertions(+), 1 deletions(-)
 
 diff --git a/block/raw-posix.c b/block/raw-posix.c
 index e51293a..5815707 100644
 --- a/block/raw-posix.c
 +++ b/block/raw-posix.c
 @@ -1312,7 +1312,30 @@ again:
 if (size == 0)
 #endif
 #if defined(__APPLE__)  defined(__MACH__)
 -size = LLONG_MAX;
 +{
 +uint64_t sectors = 0;
 +uint32_t sector_size = 0;
 +bool ioctl_problem = false;
 +ret = 0;
 +
 +/* Query the number of sectors on the disk */
 +ret = ioctl(fd, DKIOCGETBLOCKCOUNT, sectors);
 +if (ret != 0)
 +ioctl_problem = true;
 +
 +/* Query the size of each sector */
 +ret = ioctl(fd, DKIOCGETBLOCKSIZE, sector_size);
 +if (ret != 0)
 +ioctl_problem = true;
 +
 +/* If everything is ok */
 +if (ioctl_problem == false)
 +size = sectors * sector_size;
 +
 +/* If a problem occurred with ioctl(), fallback to lseek() */
 +else
 +size = lseek(fd, 0LL, SEEK_END);
 +}
 #else
 
 This fixes the make check problem, but you're not catching
 the possibility of lseek failing. (Also the if() statements
 are all missing braces our coding style requires.) You can
 avoid that bool flag like this:
 
{
uint64_t sectors = 0;
uint32_t sector_size = 0;
 
if (ioctl(fd, DKIOCGETBLOCKCOUNT, sectors) == 0
 ioctl(fd, DKIOCGETBLOCKSIZE, sector_size) == 0) {
size = sectors * sector_size;
} else {
size = lseek(fd, 0LL, SEEK_END);
if (size  0) {
return -errno;
}
}
}

Yeah, your code looks great. I will make a patch for it if you need me to. 




Re: [Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend

2015-01-14 Thread Peter Maydell
On 14 January 2015 at 18:15, Lluís Vilanova vilan...@ac.upc.edu wrote:
 Sorry I wasn't clear. I meant that it might be better to remove tcg/README and
 instead document all the operations (those in the readme and the additional
 ones) in the header.

I think the IR is worth documenting in itself, rather than merging
it into the documentation of either the functions called by the
front end or the functions that have to be implemented by the
back end.

-- PMM



Re: [Qemu-devel] [PATCH] Fixes several full screen issues on Mac OS X

2015-01-14 Thread Peter Maydell
On 14 January 2015 at 18:18, Programmingkid programmingk...@gmail.com wrote:

 On Jan 14, 2015, at 12:19 PM, Peter Maydell wrote:
 (2) Having done this I find that all my other application windows
 have been squashed down into a corner of my screen, presumably because
 we've told MacOSX the screen is 640x480 and it's rearranged the
 app windows to suit. We mustn't mess things up like this.

 This can't be avoided. When the screen resolution changes, all the
 applications usually adjust to the new size. It is the same thing
 that happens when you use a full screen game.

No, when I use other full screen programs this doesn't happen at all.
QEMU with this patch is the first time I've ever seen this from
any OSX app.

 (3) I managed to get at the underlying QEMU window with its title
 bar somehow even when in full screen mode: I could move it about the
 screen with the mouse...

 Really? I think you said you had Mac OS 10.7. I don't have that, but
 I do have access to Mac OS 10.9. Just send me the instructions on how
 to reproduce this.

I run 10.9.5. I don't know exactly how I got to that window,
and I don't really want to mess about with this patch because
behaviour (2) above is so obnoxious.

 (4) I get a lot of compile warnings for this patch:


 Disabling the depreciation warning would eliminate these errors.

Not all of them are deprecation warnings. Also I would prefer not
to disable deprecation warnings, as then we'll have no notice
of what might break on future OSX versions.

 int w = surface_width(surface);
 int h = surface_height(surface);
 -/* cdx == 0 means this is our very first surface, in which case we need
 - * to recalculate the content dimensions even if it happens to be the 
 size
 - * of the initial empty window.
 - */
 -bool isResize = (w != screen.width || h != screen.height || cdx == 
 0.0);
 -
 +bool isResize = (w != screen.width || h != screen.height);

 (6) This looks like you've just dropped a bug fix. How are you
 dealing with this case if not by the method described in the
 now-deleted comment?

 If the guest does change its resolution, then we try to match it
 in the host. When I eliminated this code, it made the guest look
 so much better. I was actually able to read documents in the guest
 at full screen.

The point is that you've dropped a bugfix which isn't related to
full screen at all -- if this is the first call to switchSurface
we *must* display it, which is what the cdx check does. See
commit 381600dad.

-- PMM



Re: [Qemu-devel] [PATCH] Machine menu patch for Mac OS X

2015-01-14 Thread Programmingkid

On Jan 14, 2015, at 12:42 PM, Peter Maydell wrote:

 On 13 January 2015 at 01:49, Programmingkid programmingk...@gmail.com wrote:
 This patch adds a Machine menu to QEMU. This menu gives the user the ability 
 to easily work with floppy and CD image files.
 
 Features:
 Menu items to switch floppy and CD image files.
 Menu items to eject floppy and CD image files.
 Menu item to use /dev/cdrom.
 Verifies with the user before quitting QEMU by displaying a dialog box.
 
 Signed-off-by: John Arbuckle programmingk...@gmail.com
 
 Hi. I'm afraid I couldn't get this patch to apply -- is it dependent
 on one of your other patches?

I have been making a lot of changes to cocoa.m. Can you suggest a way to make 
these changes not interfere with each other?

 
 Some comments below, anyway.
 
 +/* Pause the guest */
 +- (void)pauseQemu:(id)sender
 +{
 +qmp_stop(NULL);
 +[sender setEnabled: NO];
 +[[[sender menu] itemWithTitle: @Resume] setEnabled: YES];
 +[normalWindow setTitle: @*** Paused ***];
 +}
 +
 +/* Resume running the guest operating system */
 +- (void)resumeQemu: (id) sender
 +{
 +qmp_cont(NULL);
 +[sender setEnabled: NO];
 +[[[sender menu] itemWithTitle: @Pause] setEnabled: YES];
 +[normalWindow setTitle: @QEMU];
 
 Isn't this changing of the title string going to conflict with the
 changes that we make for mouse grab/ungrab?

Unfortunately yes, but I still need a way to alert the user that QEMU is 
paused. I guess I could find a way to make both messages show up in the 
titlebar. Or I could display a message in the main window that says Paused. 
Will come up with something soon.

 
 +}
 +
 +/* Eject the floppy0 disk */
 +- (void)ejectFloppy:(id)sender
 +{
 +Error *err = NULL;
 +qmp_eject(floppy0, false, false, err);
 +handleAnyDeviceErrors(err);
 +}
 +
 +/* Displays a dialog box asking the user to select a floppy image to load */
 +- (void)changeFloppy:(id)sender
 +{
 +NSOpenPanel * open_panel;
 +open_panel = [NSOpenPanel openPanel];
 +[open_panel setCanChooseFiles: YES];
 +[open_panel setAllowsMultipleSelection: NO];
 +if([open_panel runModalForDirectory: nil file: nil] == NSOKButton) {
 +Error *err = NULL;
 +NSString * file = [[open_panel filenames] objectAtIndex: 0];
 +qmp_change_blockdev(floppy0, [file cString], raw, err);
 +handleAnyDeviceErrors(err);
 +}
 +}
 
 You don't know that the machine being emulated has a floppy drive
 at all, or that it's called floppy0...

I did only use the PC and Mac targets. I know there are others. It looks like 
conditionally adding some of these menu items will have to do. I guess 
detecting if the guest machine has a floppy and/or cdrom drive can be done - 
hopefully...


 
 +
 +// Ejects the cdrom
 +- (void)ejectCdrom:(id)sender
 +{
 +Error *err = NULL;
 +qmp_eject(ide1-cd0, false, false, err);
 +handleAnyDeviceErrors(err);
 +}
 +
 +/* Displays a dialog box asking the user to select a CD image to load */
 +- (void)changeCdrom:(id)sender
 +{
 +NSOpenPanel * open_panel;
 +open_panel = [NSOpenPanel openPanel];
 +[open_panel setCanChooseFiles: YES];
 +[open_panel setAllowsMultipleSelection: NO];
 +if([open_panel runModalForDirectory: nil file: nil] == NSOKButton) {
 +NSString * file = [[open_panel filenames] objectAtIndex: 0];
 +Error *err = NULL;
 +qmp_change_blockdev(ide1-cd0, [file cString], raw, err);
 +handleAnyDeviceErrors(err);
 +}
 +}
 
 Similarly, the CD may not exist in the guest machine or may
 not be called ide1-cd0.

Ok, will find some way to determine what the CDROM drive is called - if that is 
possible...

 
 +
 +/* Restarts QEMU */
 +- (void)restartQemu: (id) sender
 +{
 +qemu_system_reset_request();
 +}
 +
 +/* Switches QEMU to use the real cdrom drive */
 +- (void)useRealCdrom: (id) sender
 +{
 +Error *err = NULL;
 +qmp_change_blockdev(ide1-cd0, /dev/cdrom, raw, err);
 +handleAnyDeviceErrors(err);
 +}
 +
 +/* Verifies if the user really wants to quit */
 +- (void)verifyQuit: (id) sender
 +{
 +NSInteger response;
 +response = NSRunAlertPanel(@Quit?, @Are you sure you want to quit?, 
 @Cancel, @Quit, nil);
 
 We don't have an are-you-sure prompt for closing the QEMU window via
 the red button, and we don't for the Quit menu option in the GTK UI either...

A prompt can be added to the close red button. As for GTK, we are allowed to 
be better than them. It really stinks when I'm working on something in the 
guest and I push Command-Q thinking it will only quit the one application I am 
working on, but instead takes out the entire emulator. I'm hoping that prompt 
will save someone from having a really bad day. 

 
 +if(response == NSAlertAlternateReturn)
 +exit(EXIT_SUCCESS);
 
 You should use qmp_quit(NULL) rather than just exit().

Not a problem.

 
 +}
 +
 @end
 
 
 @@ -1046,7 +1153,7 @@ int main (int argc, const char * argv[]) {
 [menuItem 
 

Re: [Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend

2015-01-14 Thread Richard Henderson
On 01/14/2015 10:15 AM, Lluís Vilanova wrote:
 Sorry I wasn't clear. I meant that it might be better to remove tcg/README and
 instead document all the operations (those in the readme and the additional
 ones) in the header.

If we did that, it would go somewhere else.

There are two different (but clearly closely related) things that
need documenting:

  (1) The functional interface used by target-*/translate.c.
  This is what Bastian is attempting to document now.

  (2) The intermediate language opcodes.
  This is what is currently documented in tcg/README.


r~




Re: [Qemu-devel] [PATCH] Machine menu patch for Mac OS X

2015-01-14 Thread Peter Maydell
On 14 January 2015 at 18:34, Programmingkid programmingk...@gmail.com wrote:

 On Jan 14, 2015, at 12:42 PM, Peter Maydell wrote:

 On 13 January 2015 at 01:49, Programmingkid programmingk...@gmail.com 
 wrote:
 This patch adds a Machine menu to QEMU. This menu gives the user the 
 ability to easily work with floppy and CD image files.

 Features:
 Menu items to switch floppy and CD image files.
 Menu items to eject floppy and CD image files.
 Menu item to use /dev/cdrom.
 Verifies with the user before quitting QEMU by displaying a dialog box.

 Signed-off-by: John Arbuckle programmingk...@gmail.com

 Hi. I'm afraid I couldn't get this patch to apply -- is it dependent
 on one of your other patches?

 I have been making a lot of changes to cocoa.m. Can you suggest a way
 to make these changes not interfere with each other?

It can be hard to avoid in that kind of situation. Sometimes the
best you can do is just to say which patches it depends on.


 Isn't this changing of the title string going to conflict with the
 changes that we make for mouse grab/ungrab?

 Unfortunately yes, but I still need a way to alert the user that QEMU
 is paused. I guess I could find a way to make both messages show up
 in the titlebar. Or I could display a message in the main window that
 says Paused. Will come up with something soon.

ui/gtk.c has a 'gd_update_caption()' function which sets the title
string as appropriate for the current situation; that is then called
from the places which change the UI state. That's probably as good
an approach as any.

 You don't know that the machine being emulated has a floppy drive
 at all, or that it's called floppy0...

 I did only use the PC and Mac targets. I know there are others. It looks
 like conditionally adding some of these menu items will have to do. I
 guess detecting if the guest machine has a floppy and/or cdrom drive can
 be done - hopefully...

It's not impossible. But none of the other UIs are trying to do that
kind of thing yet; I'd suggest postponing those parts. Bringing the
OSX UI into line with features already implementing in another UI
is simpler than adding features no other UI has, because you're not
trying to break new ground.

 In the GTK UI we call this Reset. We also have a Power Down which would
 probably be nice for consistency.

 Changing Restart to Reset will be done. I don't know how to
 implement Power Down, so I'm not sure about that one.

It's easy: just call qmp_system_powerdown(NULL).

 Do you think anyone will ever make SDL or GTK UIs for QEMU work
 on Mac OS X?

Hard to say. I won't, because I don't have the GTK dependencies
available to me. And there don't really seem to be any other
developers trying to work with QEMU on OSX except you.
It should be possible in theory to get SDL or GTK UIs working,
but you'd have to fix up places where the cocoa UI assumes
it's always present, I expect.

-- PMM



Re: [Qemu-devel] question about live migration with storage

2015-01-14 Thread Zhang Haoyu

On 2015-01-14 15:42:41, Paolo Bonzini wrote:
 
 On 14/01/2015 03:41, Zhang Haoyu wrote:
  Hi, Paolo,
  what's advantages of drive_mirror over traditional mechanism implemented in 
  block-migration.c ?
  Why libvirt use drive_mirror instead of traditional iterative mechanism as 
  the default way
  of live migration with non-shared storage?
 
 1) Being able to choose which block devices are migrated, and whether
 they are migrated incrementally or not.
 
Yes.
 2) Finer-grain control the parameters of block migration (dirty bitmap
 granularity).
 
 3) Block and RAM migration do not share the same socket and thus can
 more easily be parallelized.
 
But, because of the parallelization, how to calculate the progress?

BTW, the traditional iterative mechanism is buggy-implemented?
e.g., some bugs which are very difficult to fixed, or something?

Thanks,
Zhang Haoyu
 Note that 1-2 are not yet supported by libvirt as far as I remember.
 
 Paolo




Re: [Qemu-devel] a question for control queue

2015-01-14 Thread Jason Wang

On 01/09/2015 02:41 PM, Ouyang, Changchun wrote:

 Hi all,

 I have a question about the control queue in qemu,

 When the qemu have configured the control queue, and guest also
 negotiated the control queue successfully with qemu,

 Will the qemu will let vhost know guest try to use control queue to
 send some commands?


Currently not. Vhost is only in charge of data path so control virtqueue
is still handled by qemu. So the filtering does not even work if vhost
is used. The plan is let management (libvirt) to be notified when guest
want to do filtering. And then libvirt can configure the filter of host
devices.

  

 Or could the vhost also setup  the control queue to communicate
 directly with control queue on guest?


Technically, we can.

 How to do that?


Just do like what we did for rx virtqueue and tx virtqueue. But I see
several issues:

- For security reason, qemu was usually run as non-privileged process.
This means vhost kernel thread does not have the privilege to configure
filter in host.
- Vhost kernel thread know little about the backend (which could be
tun/macvtap or even packet socket).

But for vhost-user implementation, it may make sense but I'm not sure.

 Hope anyone could shed some lights on  this question.

 Thanks in advance!

  

 Thanks

 Changchun





[Qemu-devel] [PATCH v3] PPC: E500: Add FSL i2c controller and integrate RTC with it

2015-01-14 Thread Amit Tomar
This patch adds an emulation model for i2c controller found on most of the FSL 
SoCs.
It also integrates the RTC(ds1338) that sits on the i2c Bus with e500 machine 
model.


Signed-off-by: Amit Singh Tomar amit.to...@freescale.com
---
Changes in v3: 
  * Reordered the subject line to appropriate one

Changes in v2: 
* Moved it to GPL v2+ 
* Replaced the printf with DPRINTF
* Fixed the coding style issues
* Changed the subject line
---
 default-configs/ppc-softmmu.mak   |2 +
 default-configs/ppc64-softmmu.mak |2 +
 hw/i2c/Makefile.objs  |1 +
 hw/i2c/mpc_i2c.c  |  364 +
 hw/ppc/e500.c |   50 -
 5 files changed, 418 insertions(+), 1 deletion(-)  create mode 100644 
hw/i2c/mpc_i2c.c

diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak 
index d725b23..6fdd39a 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -9,6 +9,8 @@ CONFIG_M48T59=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
+CONFIG_MPC_I2C=y
+CONFIG_DS1338=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
diff --git a/default-configs/ppc64-softmmu.mak 
b/default-configs/ppc64-softmmu.mak
index bd30d69..3ad3f58 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -9,6 +9,8 @@ CONFIG_M48T59=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
+CONFIG_MPC_I2C=y
+CONFIG_DS1338=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs index 648278e..6e6d00d 
100644
--- a/hw/i2c/Makefile.objs
+++ b/hw/i2c/Makefile.objs
@@ -4,4 +4,5 @@ common-obj-$(CONFIG_ACPI) += smbus_ich9.o
 common-obj-$(CONFIG_APM) += pm_smbus.o
 common-obj-$(CONFIG_BITBANG_I2C) += bitbang_i2c.o
 common-obj-$(CONFIG_EXYNOS4) += exynos4210_i2c.o
+common-obj-$(CONFIG_MPC_I2C) += mpc_i2c.o
 obj-$(CONFIG_OMAP) += omap_i2c.o
diff --git a/hw/i2c/mpc_i2c.c b/hw/i2c/mpc_i2c.c new file mode 100644 index 
000..84d13dd
--- /dev/null
+++ b/hw/i2c/mpc_i2c.c
@@ -0,0 +1,364 @@
+/*
+ * Copyright (C) 2014 Freescale Semiconductor, Inc. All rights reserved.
+ *
+ * Author: Amit Tomar, amit.to...@freescale.com
+ *
+ * Description:
+ * This file is derived from IMX I2C controller,
+ * by Jean-Christophe DUBOIS .
+ *
+ * Thanks to Scott Wood and Alexander Graf for their kind help on this.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2 or 
+ * later, as published by the Free Software Foundation.
+ *
+ * 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/sysbus.h
+#include hw/i2c/i2c.h
+#include qemu/bitops.h
+#include hw/ptimer.h
+
+/*#define DEBUG_I2C*/
+
+#ifdef DEBUG_I2C
+#define DPRINTF(fmt, ...)  \
+do { fprintf(stderr, mpc_i2c[%s]:  fmt, __func__, ## __VA_ARGS__); \
+} while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0) #endif
+
+#define TYPE_MPC_I2C mpc-i2c
+#define MPC_I2C(obj) \
+OBJECT_CHECK(MPCI2CState, (obj), TYPE_MPC_I2C)
+
+#define MPC_I2C_ADR   0x00
+#define MPC_I2C_FDR   0x04
+#define MPC_I2C_CR0x08
+#define MPC_I2C_SR0x0c
+#define MPC_I2C_DR0x10
+#define MPC_I2C_DFSRR 0x14
+
+#define CCR_MEN  (17)
+#define CCR_MIEN (16)
+#define CCR_MSTA (15)
+#define CCR_MTX  (14)
+#define CCR_TXAK (13)
+#define CCR_RSTA (12)
+#define CCR_BCST (10)
+
+#define CSR_MCF  (17)
+#define CSR_MAAS (16)
+#define CSR_MBB  (15)
+#define CSR_MAL  (14)
+#define CSR_SRW  (12)
+#define CSR_MIF  (11)
+#define CSR_RXAK (10)
+
+#define CADR_MASK 0xFE
+#define CFDR_MASK 0x3F
+#define CCR_MASK  0xFC
+#define CSR_MASK  0xED
+#define CDR_MASK  0xFF
+
+#define CYCLE_RESET 0xFF
+
+typedef struct MPCI2CState {
+SysBusDevice parent_obj;
+
+I2CBus *bus;
+qemu_irq irq;
+MemoryRegion iomem;
+
+uint8_t address;
+uint8_t adr;
+uint8_t fdr;
+uint8_t cr;
+uint8_t sr;
+uint8_t dr;
+uint8_t dfssr;
+} MPCI2CState;
+
+static bool mpc_i2c_is_enabled(MPCI2CState *s) {
+return s-cr  CCR_MEN;
+}
+
+static bool mpc_i2c_is_master(MPCI2CState *s) {
+return s-cr  CCR_MSTA;
+}
+
+static bool mpc_i2c_direction_is_tx(MPCI2CState *s) {
+return s-cr  CCR_MTX;
+}
+
+static bool mpc_i2c_irq_pending(MPCI2CState *s) {
+return s-sr  CSR_MIF;
+}
+
+static bool mpc_i2c_irq_is_enabled(MPCI2CState *s) {
+return s-cr  CCR_MIEN;
+}
+
+static void mpc_i2c_reset(DeviceState *dev) {
+MPCI2CState *i2c = MPC_I2C(dev);
+
+i2c-address = 0xFF;
+i2c-adr = 0x00;
+i2c-fdr = 0x00;
+i2c-cr =  0x00;
+i2c-sr =  0x81;
+i2c-dr =  0x00;
+}
+
+static void mpc_i2c_irq(MPCI2CState *s) {
+bool irq_active = false;
+
+if (mpc_i2c_is_enabled(s)  mpc_i2c_irq_is_enabled(s)
+   mpc_i2c_irq_pending(s)) {
+irq_active = true;

Re: [Qemu-devel] [Bug 1409246] Re: ARM GIC / PL061 error on uni-processor system

2015-01-14 Thread Chris
On 12 January 2015 22:15:22 GMT+00:00, Christopher Covington 
c...@codeaurora.org wrote:
Hi Christopher,

On 01/10/2015 11:42 AM, Christopher Horler wrote:
 confirmed - not a bug, patching the kernel fixes the problem.

Mind pointing to your patch? I've been seeing the GIC CPU mask not
found -
kernel will fail to boot. message for a long time but since it was
apparently
spurious never found the time to get to the bottom of it.


I didn't fix that message, because it wasn't the cause of the mmc boot failure. 
 In the end what I did fix was a single line in pl061.c

I'll share that patch, when I can download an up to date git tree (probably 
this evening).  It could use a peer reviewing to know it's correct!





Re: [Qemu-devel] [RFC PATCH v7 08/21] replay: interrupts and exceptions

2015-01-14 Thread Pavel Dovgaluk
 From: Paolo Bonzini [mailto:pbonz...@redhat.com]
 On 12/01/2015 13:40, Pavel Dovgaluk wrote:
 
   Perhaps check the replay_interrupt() outside, in an  with if
   (unlikely(interrupt_request))?
  You mean that I should wrap whole condition into unlikely?
 
 
 No, I wanted to have a single check of replay_interrupt() and/or
 replay_has_interrupt().
 
 BTW, I think this is incorrect:
 
  +if ((replay_mode != REPLAY_MODE_PLAY
  +|| replay_has_interrupt())
  + cc-cpu_exec_interrupt(cpu, interrupt_request)) 
  {
  +replay_interrupt();
 
 because cc-cpu_exec_interrupt() can exit with cpu_loop_exit(cpu).

Haven't found any. Do you have an example?

 
 I guess it could be something like this:
 
 if (replay_mode == REPLAY_MODE_PLAY  !replay_has_interrupt()) {
 /* do nothing */
 } else if (interrupt_request  CPU_INTERRUPT_HALT) {
 replay_interrupt();
 ...
 cpu_loop_exit(cpu);
 } else if (interrupt_request  CPU_INTERRUPT_INIT) {
 replay_interrupt();
 ...
 cpu_loop_exit(cpu);
 } else {
 replay_interrupt();
 if (cc-cpu_exec_interrupt(cpu, interrupt_request)) {
 next_tb = 0;
 }
 }

Is it normal that processing of the reset request does not execute 
cpu_loop_exit(cpu)?

 #else
-if (interrupt_request  CPU_INTERRUPT_RESET) {
+if ((interrupt_request  CPU_INTERRUPT_RESET)
+ replay_interrupt()) {
 cpu_reset(cpu);
 }
 #endif


Pavel Dovgalyuk




Re: [Qemu-devel] question about live migration with storage

2015-01-14 Thread Paolo Bonzini


On 14/01/2015 08:58, Zhang Haoyu wrote:
 2) Finer-grain control the parameters of block migration (dirty bitmap
 granularity).

 3) Block and RAM migration do not share the same socket and thus can
 more easily be parallelized.

 But, because of the parallelization, how to calculate the progress?

You can use query-block-jobs.

 BTW, the traditional iterative mechanism is buggy-implemented?
 e.g., some bugs which are very difficult to fixed, or something?

There are no bugs that corrupt data. It's simply less flexible.

Regarding parallelization, the problems happen especially if you disable
migration bandwidth limits.  Then you'll see large chunks of RAM and
large chunks of block device data being sent.  This is a problem for
convergence in some workloads. Instead, with NBD-based storage migration
everything happens in parallel, and TCP makes sure that bandwidth is
split between the sockets.

If you have a migration bandwidth limit, NBD-based storage migration
will use a separate bandwidth limit for each disk and for RAM. Thus
network usage is less predictable than with block-migration.c.  On the
other hand, storage migration uses a lot of network so it's usually more
likely that you do not have migration bandwidth limits.

Paolo

 Thanks,
 Zhang Haoyu
 Note that 1-2 are not yet supported by libvirt as far as I remember.

 Paolo
 
 
 



Re: [Qemu-devel] Fedora FC21 - Bug: 100% CPU and hangs in gettimeofday(tp, NULL); forever

2015-01-14 Thread Gerhard Wiesinger

On 14.01.2015 01:59, Laine Stump wrote:


Take a look at the following kernel bug. It specifically deals with a
hang in gettimeofday() in a KVM guest:

https://bugzilla.redhat.com/show_bug.cgi?id=1178975

There is a link to a patched kernel you can try; it fixed my problems (I
was repeatedly getting hangs in python-urlgrabber during yum updates on
F21).



Looks to be fixed, commented in:
https://bugzilla.redhat.com/show_bug.cgi?id=1178975
Installed kernels.
http://koji.fedoraproject.org/koji/taskinfo?taskID=8575247

Time to release a new official kernel :-)

Thanx for the comments.

Ciao,
Gerhard



[Qemu-devel] [PULL 02/15] vl.c: fix regression when reading machine type from config file

2015-01-14 Thread Paolo Bonzini
From: Marcel Apfelbaum mar...@redhat.com

After 'Machine as QOM' series the machine type input triggers
the creation of the machine class.
If the machine type is set in the configuration file, the machine
class is not updated accordingly and remains the default.

Fixed that by querying the machine options after the configuration
file is loaded.

Cc: qemu-sta...@nongnu.org
Reported-by: William Dauchy will...@gandi.net
Signed-off-by: Marcel Apfelbaum mar...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 vl.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/vl.c b/vl.c
index 7786b2f..1a2da2b 100644
--- a/vl.c
+++ b/vl.c
@@ -2796,9 +2796,6 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 switch(popt-index) {
-case QEMU_OPTION_M:
-machine_class = machine_parse(optarg);
-break;
 case QEMU_OPTION_no_kvm_irqchip: {
 olist = qemu_find_opts(machine);
 qemu_opts_parse(olist, kernel_irqchip=off, 0);
@@ -3420,16 +3417,13 @@ int main(int argc, char **argv, char **envp)
 olist = qemu_find_opts(machine);
 qemu_opts_parse(olist, accel=kvm, 0);
 break;
+case QEMU_OPTION_M:
 case QEMU_OPTION_machine:
 olist = qemu_find_opts(machine);
 opts = qemu_opts_parse(olist, optarg, 1);
 if (!opts) {
 exit(1);
 }
-optarg = qemu_opt_get(opts, type);
-if (optarg) {
-machine_class = machine_parse(optarg);
-}
 break;
  case QEMU_OPTION_no_kvm:
 olist = qemu_find_opts(machine);
@@ -3752,6 +3746,13 @@ int main(int argc, char **argv, char **envp)
 }
 }
 }
+
+opts = qemu_get_machine_opts();
+optarg = qemu_opt_get(opts, type);
+if (optarg) {
+machine_class = machine_parse(optarg);
+}
+
 loc_set_none();
 
 os_daemonize();
-- 
1.8.3.1





[Qemu-devel] [PULL 08/15] target-i386: fix movntsd on big-endian hosts

2015-01-14 Thread Paolo Bonzini
This was accessing an XMM register's low half without going through XMM_Q.

Cc: qemu-sta...@nongnu.org
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 target-i386/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index ebdc350..5af4300 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -3074,7 +3074,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 goto illegal_op;
 gen_lea_modrm(env, s, modrm);
 if (b1  1) {
-gen_stq_env_A0(s, offsetof(CPUX86State, xmm_regs[reg]));
+gen_stq_env_A0(s, offsetof(CPUX86State,
+   xmm_regs[reg].XMM_Q(0)));
 } else {
 tcg_gen_ld32u_tl(cpu_T[0], cpu_env, offsetof(CPUX86State,
 xmm_regs[reg].XMM_L(0)));
-- 
1.8.3.1





[Qemu-devel] [PULL 14/15] qemu-timer: rename timer_init to timer_init_tl

2015-01-14 Thread Paolo Bonzini
timer_init is not called that often.  Free the name for an equivalent
of timer_new.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 include/block/aio.h  |  2 +-
 include/qemu/timer.h | 10 +-
 qemu-timer.c |  6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index 6bf0e04..7d1e26b 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -314,7 +314,7 @@ static inline void aio_timer_init(AioContext *ctx,
   int scale,
   QEMUTimerCB *cb, void *opaque)
 {
-timer_init(ts, ctx-tlg.tl[type], scale, cb, opaque);
+timer_init_tl(ts, ctx-tlg.tl[type], scale, cb, opaque);
 }
 
 /**
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index d9df094..0666920 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -410,7 +410,7 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg);
  */
 
 /**
- * timer_init:
+ * timer_init_tl:
  * @ts: the timer to be initialised
  * @timer_list: the timer list to attach the timer to
  * @scale: the scale value for the timer
@@ -423,9 +423,9 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg);
  * You need not call an explicit deinit call. Simply make
  * sure it is not on a list with timer_del.
  */
-void timer_init(QEMUTimer *ts,
-QEMUTimerList *timer_list, int scale,
-QEMUTimerCB *cb, void *opaque);
+void timer_init_tl(QEMUTimer *ts,
+   QEMUTimerList *timer_list, int scale,
+   QEMUTimerCB *cb, void *opaque);
 
 /**
  * timer_new_tl:
@@ -448,7 +448,7 @@ static inline QEMUTimer *timer_new_tl(QEMUTimerList 
*timer_list,
   void *opaque)
 {
 QEMUTimer *ts = g_malloc0(sizeof(QEMUTimer));
-timer_init(ts, timer_list, scale, cb, opaque);
+timer_init_tl(ts, timer_list, scale, cb, opaque);
 return ts;
 }
 
diff --git a/qemu-timer.c b/qemu-timer.c
index cb7d988..98d9d1b 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -331,9 +331,9 @@ int qemu_poll_ns(GPollFD *fds, guint nfds, int64_t timeout)
 }
 
 
-void timer_init(QEMUTimer *ts,
-QEMUTimerList *timer_list, int scale,
-QEMUTimerCB *cb, void *opaque)
+void timer_init_tl(QEMUTimer *ts,
+   QEMUTimerList *timer_list, int scale,
+   QEMUTimerCB *cb, void *opaque)
 {
 ts-timer_list = timer_list;
 ts-cb = cb;
-- 
1.8.3.1





[Qemu-devel] [PULL 05/15] vl: fix max_cpus check

2015-01-14 Thread Paolo Bonzini
From: Andrew Jones drjo...@redhat.com

We should confirm max_cpus, which is = smp_cpus, is
= the machine's true max_cpus, not just smp_cpus.

Signed-off-by: Andrew Jones drjo...@redhat.com
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 56a17dd..95451b6 100644
--- a/vl.c
+++ b/vl.c
@@ -3850,9 +3850,9 @@ int main(int argc, char **argv, char **envp)
 smp_parse(qemu_opts_find(qemu_find_opts(smp-opts), NULL));
 
 machine_class-max_cpus = machine_class-max_cpus ?: 1; /* Default to UP */
-if (smp_cpus  machine_class-max_cpus) {
+if (max_cpus  machine_class-max_cpus) {
 fprintf(stderr, Number of SMP cpus requested (%d), exceeds max cpus 
-supported by machine `%s' (%d)\n, smp_cpus,
+supported by machine `%s' (%d)\n, max_cpus,
 machine_class-name, machine_class-max_cpus);
 exit(1);
 }
-- 
1.8.3.1





[Qemu-devel] [PULL 07/15] vl.c: fix regression when reading memory size from config file

2015-01-14 Thread Paolo Bonzini
From: Marcel Apfelbaum mar...@redhat.com

This is happening because an actual logic is performed on the memory
arguments inside the main's switch, disregarding the config file content.

Solved by extracting the logic on a separate function and calling it
after the switch.

Signed-off-by: Marcel Apfelbaum mar...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 vl.c | 181 +++
 1 file changed, 94 insertions(+), 87 deletions(-)

diff --git a/vl.c b/vl.c
index f6b24c4..54e6bbd 100644
--- a/vl.c
+++ b/vl.c
@@ -2648,6 +2648,96 @@ out:
 return 0;
 }
 
+static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size)
+{
+uint64_t sz;
+const char *mem_str;
+const char *maxmem_str, *slots_str;
+const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
+1024 * 1024;
+QemuOpts *opts = qemu_find_opts_singleton(memory);
+
+ram_size = default_ram_size;
+*maxram_size = default_ram_size;
+
+mem_str = qemu_opt_get(opts, size);
+if (!mem_str) {
+goto done;
+}
+if (!*mem_str) {
+error_report(missing 'size' option value);
+exit(EXIT_FAILURE);
+}
+
+sz = qemu_opt_get_size(opts, size, ram_size);
+
+/* Fix up legacy suffix-less format */
+if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
+uint64_t overflow_check = sz;
+
+sz = 20;
+if ((sz  20) != overflow_check) {
+error_report(too large 'size' option value);
+exit(EXIT_FAILURE);
+}
+}
+
+/* backward compatibility behaviour for case -m 0 */
+if (sz == 0) {
+sz = default_ram_size;
+}
+
+sz = QEMU_ALIGN_UP(sz, 8192);
+ram_size = sz;
+if (ram_size != sz) {
+error_report(ram size too large);
+exit(EXIT_FAILURE);
+}
+*maxram_size = ram_size;
+
+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) {
+error_report(invalid -m option value: maxmem 
+(0x% PRIx64 ) = initial memory (0x
+RAM_ADDR_FMT ), sz, ram_size);
+exit(EXIT_FAILURE);
+}
+
+slots = qemu_opt_get_number(opts, slots, 0);
+if ((sz  ram_size)  !slots) {
+error_report(invalid -m option value: maxmem 
+(0x% PRIx64 ) more than initial memory (0x
+RAM_ADDR_FMT ) but no hotplug slots where 
+specified, sz, ram_size);
+exit(EXIT_FAILURE);
+}
+
+if ((sz = ram_size)  slots) {
+error_report(invalid -m option value:  %
+PRIu64  hotplug slots where specified but 
+maxmem (0x% PRIx64 ) = initial memory (0x
+RAM_ADDR_FMT ), slots, sz, ram_size);
+exit(EXIT_FAILURE);
+}
+*maxram_size = sz;
+*ram_slots = slots;
+} else if ((!maxmem_str  slots_str) ||
+(maxmem_str  !slots_str)) {
+error_report(invalid -m option value: missing 
+'%s' option, slots_str ? maxmem : slots);
+exit(EXIT_FAILURE);
+}
+
+done:
+/* store value for the future use */
+qemu_opt_set_number(opts, size, ram_size);
+}
+
 int main(int argc, char **argv, char **envp)
 {
 int i;
@@ -2683,9 +2773,7 @@ int main(int argc, char **argv, char **envp)
 };
 const char *trace_events = NULL;
 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;
+ram_addr_t maxram_size;
 uint64_t ram_slots = 0;
 FILE *vmstate_dump_file = NULL;
 Error *main_loop_err = NULL;
@@ -2736,7 +2824,6 @@ int main(int argc, char **argv, char **envp)
 module_call_init(MODULE_INIT_MACHINE);
 machine_class = find_default_machine();
 cpu_model = NULL;
-ram_size = default_ram_size;
 snapshot = 0;
 cyls = heads = secs = 0;
 translation = BIOS_ATA_TRANSLATION_AUTO;
@@ -3023,92 +3110,13 @@ int main(int argc, char **argv, char **envp)
 version();
 exit(0);
 break;
-case QEMU_OPTION_m: {
-uint64_t sz;
-const char *mem_str;
-const char *maxmem_str, *slots_str;
-
+case QEMU_OPTION_m:
 opts = qemu_opts_parse(qemu_find_opts(memory),
optarg, 1);
 if (!opts) {
 exit(EXIT_FAILURE);
 }
-
-mem_str = qemu_opt_get(opts, size);
-if (!mem_str) {
-error_report(invalid -m option, missing 'size' 

[Qemu-devel] [PULL 00/15] Misc patches for 2015-01-14

2015-01-14 Thread Paolo Bonzini
The following changes since commit f1c5831ca3e3eafb89331233221768b64db113e8:

  Merge remote-tracking branch 'remotes/amit-virtio-rng/tags/rng-for-2.3' into 
staging (2015-01-09 18:55:29 +)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 71f35e66ce76dadd034cd463556be5e1a8230245:

  cpus: consistently use QEMU_CLOCK_VIRTUAL_RT for icount_warp_rt timer 
(2015-01-14 10:16:12 +0100)


Mostly bugfixes and cleanups from qemu-devel.  Yet another small patch from
the record/replay series, and a few SCSI and i386 patches as well.


Andrew Jones (1):
  vl: fix max_cpus check

Eduardo Habkost (2):
  vl: Avoid unnecessary 'if' nesting
  vl: Don't silently change topology when all -smp options were set

Fam Zheng (1):
  rules.mak: Fix module build

Frediano Ziglio (1):
  qemu-common.h: optimise muldiv64 if int128 is available

Gal Hammer (1):
  char: restore stdio echo on resume from suspend.

Marcel Apfelbaum (2):
  vl.c: fix regression when reading machine type from config file
  vl.c: fix regression when reading memory size from config file

Paolo Bonzini (4):
  target-i386: fix movntsd on big-endian hosts
  target-i386: do not memcpy in and out of xmm_regs
  scsi: fix cancellation when I/O was completed but DMA was not.
  qemu-timer: rename timer_init to timer_init_tl

Pavel Dovgalyuk (1):
  cpus: consistently use QEMU_CLOCK_VIRTUAL_RT for icount_warp_rt timer

Peter Lieven (1):
  hw/scsi/lsi53c895a: add support for additional diag / debug registers

SeokYeon Hwang (1):
  9pfs: changed to use event_notifier instead of qemu_pipe

 cpus.c   |   2 +-
 hw/9pfs/virtio-9p-coth.c |  29 ++-
 hw/9pfs/virtio-9p-coth.h |   4 +-
 hw/scsi/lsi53c895a.c |   6 ++
 hw/scsi/scsi-bus.c   |   2 +
 include/block/aio.h  |   2 +-
 include/qemu-common.h|   7 ++
 include/qemu/timer.h |  10 +--
 qemu-char.c  |  15 
 qemu-timer.c |   6 +-
 rules.mak|  23 ++---
 target-i386/kvm.c|  30 +--
 target-i386/translate.c  |  11 +--
 vl.c | 218 +--
 14 files changed, 206 insertions(+), 159 deletions(-)
-- 
1.8.3.1




[Qemu-devel] [PULL 03/15] 9pfs: changed to use event_notifier instead of qemu_pipe

2015-01-14 Thread Paolo Bonzini
From: SeokYeon Hwang syeon.hw...@samsung.com

Changed to use event_notifier instead of qemu_pipe.
It is necessary for porting 9pfs to Windows and MacOS.

Signed-off-by: SeokYeon Hwang syeon.hw...@samsung.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 hw/9pfs/virtio-9p-coth.c | 29 +++--
 hw/9pfs/virtio-9p-coth.h |  4 ++--
 2 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/hw/9pfs/virtio-9p-coth.c b/hw/9pfs/virtio-9p-coth.c
index ae6cde8..8185c53 100644
--- a/hw/9pfs/virtio-9p-coth.c
+++ b/hw/9pfs/virtio-9p-coth.c
@@ -14,6 +14,7 @@
 
 #include fsdev/qemu-fsdev.h
 #include qemu/thread.h
+#include qemu/event_notifier.h
 #include block/coroutine.h
 #include virtio-9p-coth.h
 
@@ -26,15 +27,11 @@ void co_run_in_worker_bh(void *opaque)
 g_thread_pool_push(v9fs_pool.pool, co, NULL);
 }
 
-static void v9fs_qemu_process_req_done(void *arg)
+static void v9fs_qemu_process_req_done(EventNotifier *e)
 {
-char byte;
-ssize_t len;
 Coroutine *co;
 
-do {
-len = read(v9fs_pool.rfd, byte, sizeof(byte));
-} while (len == -1   errno == EINTR);
+event_notifier_test_and_clear(e);
 
 while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
 qemu_coroutine_enter(co, NULL);
@@ -43,22 +40,18 @@ static void v9fs_qemu_process_req_done(void *arg)
 
 static void v9fs_thread_routine(gpointer data, gpointer user_data)
 {
-ssize_t len;
-char byte = 0;
 Coroutine *co = data;
 
 qemu_coroutine_enter(co, NULL);
 
 g_async_queue_push(v9fs_pool.completed, co);
-do {
-len = write(v9fs_pool.wfd, byte, sizeof(byte));
-} while (len == -1  errno == EINTR);
+
+event_notifier_set(v9fs_pool.e);
 }
 
 int v9fs_init_worker_threads(void)
 {
 int ret = 0;
-int notifier_fds[2];
 V9fsThPool *p = v9fs_pool;
 sigset_t set, oldset;
 
@@ -66,10 +59,6 @@ int v9fs_init_worker_threads(void)
 /* Leave signal handling to the iothread.  */
 pthread_sigmask(SIG_SETMASK, set, oldset);
 
-if (qemu_pipe(notifier_fds) == -1) {
-ret = -1;
-goto err_out;
-}
 p-pool = g_thread_pool_new(v9fs_thread_routine, p, -1, FALSE, NULL);
 if (!p-pool) {
 ret = -1;
@@ -84,13 +73,9 @@ int v9fs_init_worker_threads(void)
 ret = -1;
 goto err_out;
 }
-p-rfd = notifier_fds[0];
-p-wfd = notifier_fds[1];
-
-fcntl(p-rfd, F_SETFL, O_NONBLOCK);
-fcntl(p-wfd, F_SETFL, O_NONBLOCK);
+event_notifier_init(p-e, 0);
 
-qemu_set_fd_handler(p-rfd, v9fs_qemu_process_req_done, NULL, NULL);
+event_notifier_set_handler(p-e, v9fs_qemu_process_req_done);
 err_out:
 pthread_sigmask(SIG_SETMASK, oldset, NULL);
 return ret;
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 86d5ed4..4f51b25 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -21,8 +21,8 @@
 #include glib.h
 
 typedef struct V9fsThPool {
-int rfd;
-int wfd;
+EventNotifier e;
+
 GThreadPool *pool;
 GAsyncQueue *completed;
 } V9fsThPool;
-- 
1.8.3.1





[Qemu-devel] [PULL 15/15] cpus: consistently use QEMU_CLOCK_VIRTUAL_RT for icount_warp_rt timer

2015-01-14 Thread Paolo Bonzini
From: Pavel Dovgalyuk pavel.dovga...@ispras.ru

Fix mismatch between timer_new_ms and timer_mod.

Signed-off-by: Pavel Dovgalyuk pavel.dovga...@ispras.ru
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 cpus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 2edb5cd..3a5323b 100644
--- a/cpus.c
+++ b/cpus.c
@@ -324,7 +324,7 @@ static void icount_adjust(void)
 static void icount_adjust_rt(void *opaque)
 {
 timer_mod(icount_rt_timer,
-   qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 1000);
+  qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_RT) + 1000);
 icount_adjust();
 }
 
-- 
1.8.3.1




[Qemu-devel] [PULL 01/15] char: restore stdio echo on resume from suspend.

2015-01-14 Thread Paolo Bonzini
From: Gal Hammer gham...@redhat.com

The monitor's auto-completion feature stopped working when stdio is used
as an input and qemu was resumed after it was suspended (using ctrl-z).

Signed-off-by: Gal Hammer gham...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 qemu-char.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index ef84b53..5430b87 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1112,6 +1112,9 @@ static struct termios oldtty;
 static int old_fd0_flags;
 static bool stdio_in_use;
 static bool stdio_allow_signal;
+static bool stdio_echo_state;
+
+static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo);
 
 static void term_exit(void)
 {
@@ -1119,10 +1122,17 @@ static void term_exit(void)
 fcntl(0, F_SETFL, old_fd0_flags);
 }
 
+static void term_stdio_handler(int sig)
+{
+/* restore echo after resume from suspend. */
+qemu_chr_set_echo_stdio(NULL, stdio_echo_state);
+}
+
 static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
 {
 struct termios tty;
 
+stdio_echo_state = echo;
 tty = oldtty;
 if (!echo) {
 tty.c_iflag = ~(IGNBRK|BRKINT|PARMRK|ISTRIP
@@ -1149,6 +1159,7 @@ static void qemu_chr_close_stdio(struct CharDriverState 
*chr)
 static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
 {
 CharDriverState *chr;
+struct sigaction act;
 
 if (is_daemonized()) {
 error_report(cannot use stdio with -daemonize);
@@ -1166,6 +1177,10 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio 
*opts)
 qemu_set_nonblock(0);
 atexit(term_exit);
 
+memset(act, 0, sizeof(act));
+act.sa_handler = term_stdio_handler;
+sigaction(SIGCONT, act, NULL);
+
 chr = qemu_chr_open_fd(0, 1);
 chr-chr_close = qemu_chr_close_stdio;
 chr-chr_set_echo = qemu_chr_set_echo_stdio;
-- 
1.8.3.1





[Qemu-devel] [PULL 09/15] target-i386: do not memcpy in and out of xmm_regs

2015-01-14 Thread Paolo Bonzini
After the next patch, we will move the high parts of AVX and AVX512 registers
in the same array as the SSE registers.  This will make it impossible to
memcpy an array of 128-bit values in and out of xmm_regs in one swoop.
Use a for loop instead.

Similarly, always use XMM_Q in translate.c.  This avoids introducing bugs
such as the one fixed in the previous patch.

Reviewed-by: Eduardo Habkost ehabk...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 target-i386/kvm.c   | 30 --
 target-i386/translate.c |  8 
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f92edfe..cf9f331 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1019,7 +1019,10 @@ static int kvm_put_fpu(X86CPU *cpu)
 fpu.ftwx |= (!env-fptags[i])  i;
 }
 memcpy(fpu.fpr, env-fpregs, sizeof env-fpregs);
-memcpy(fpu.xmm, env-xmm_regs, sizeof env-xmm_regs);
+for (i = 0; i  CPU_NB_REGS; i++) {
+stq_p(fpu.xmm[i][0], env-xmm_regs[i].XMM_Q(0));
+stq_p(fpu.xmm[i][8], env-xmm_regs[i].XMM_Q(1));
+}
 fpu.mxcsr = env-mxcsr;
 
 return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_FPU, fpu);
@@ -1045,6 +1048,7 @@ static int kvm_put_xsave(X86CPU *cpu)
 CPUX86State *env = cpu-env;
 struct kvm_xsave* xsave = env-kvm_xsave_buf;
 uint16_t cwd, swd, twd;
+uint8_t *xmm;
 int i, r;
 
 if (!kvm_has_xsave()) {
@@ -1065,8 +1069,6 @@ static int kvm_put_xsave(X86CPU *cpu)
 memcpy(xsave-region[XSAVE_CWD_RDP], env-fpdp, sizeof(env-fpdp));
 memcpy(xsave-region[XSAVE_ST_SPACE], env-fpregs,
 sizeof env-fpregs);
-memcpy(xsave-region[XSAVE_XMM_SPACE], env-xmm_regs,
-sizeof env-xmm_regs);
 xsave-region[XSAVE_MXCSR] = env-mxcsr;
 *(uint64_t *)xsave-region[XSAVE_XSTATE_BV] = env-xstate_bv;
 memcpy(xsave-region[XSAVE_YMMH_SPACE], env-ymmh_regs,
@@ -1079,6 +1081,13 @@ static int kvm_put_xsave(X86CPU *cpu)
 sizeof env-opmask_regs);
 memcpy(xsave-region[XSAVE_ZMM_Hi256], env-zmmh_regs,
 sizeof env-zmmh_regs);
+
+xmm = (uint8_t *)xsave-region[XSAVE_XMM_SPACE];
+for (i = 0; i  CPU_NB_REGS; i++, xmm += 16) {
+stq_p(xmm, env-xmm_regs[i].XMM_Q(0));
+stq_p(xmm+8,   env-xmm_regs[i].XMM_Q(1));
+}
+
 #ifdef TARGET_X86_64
 memcpy(xsave-region[XSAVE_Hi16_ZMM], env-hi16_zmm_regs,
 sizeof env-hi16_zmm_regs);
@@ -1384,7 +1393,10 @@ static int kvm_get_fpu(X86CPU *cpu)
 env-fptags[i] = !((fpu.ftwx  i)  1);
 }
 memcpy(env-fpregs, fpu.fpr, sizeof env-fpregs);
-memcpy(env-xmm_regs, fpu.xmm, sizeof env-xmm_regs);
+for (i = 0; i  CPU_NB_REGS; i++) {
+env-xmm_regs[i].XMM_Q(0) = ldq_p(fpu.xmm[i][0]);
+env-xmm_regs[i].XMM_Q(1) = ldq_p(fpu.xmm[i][8]);
+}
 env-mxcsr = fpu.mxcsr;
 
 return 0;
@@ -1395,6 +1407,7 @@ static int kvm_get_xsave(X86CPU *cpu)
 CPUX86State *env = cpu-env;
 struct kvm_xsave* xsave = env-kvm_xsave_buf;
 int ret, i;
+const uint8_t *xmm;
 uint16_t cwd, swd, twd;
 
 if (!kvm_has_xsave()) {
@@ -1421,8 +1434,6 @@ static int kvm_get_xsave(X86CPU *cpu)
 env-mxcsr = xsave-region[XSAVE_MXCSR];
 memcpy(env-fpregs, xsave-region[XSAVE_ST_SPACE],
 sizeof env-fpregs);
-memcpy(env-xmm_regs, xsave-region[XSAVE_XMM_SPACE],
-sizeof env-xmm_regs);
 env-xstate_bv = *(uint64_t *)xsave-region[XSAVE_XSTATE_BV];
 memcpy(env-ymmh_regs, xsave-region[XSAVE_YMMH_SPACE],
 sizeof env-ymmh_regs);
@@ -1434,6 +1445,13 @@ static int kvm_get_xsave(X86CPU *cpu)
 sizeof env-opmask_regs);
 memcpy(env-zmmh_regs, xsave-region[XSAVE_ZMM_Hi256],
 sizeof env-zmmh_regs);
+
+xmm = (const uint8_t *)xsave-region[XSAVE_XMM_SPACE];
+for (i = 0; i  CPU_NB_REGS; i++, xmm += 16) {
+env-xmm_regs[i].XMM_Q(0) = ldq_p(xmm);
+env-xmm_regs[i].XMM_Q(1) = ldq_p(xmm+8);
+}
+
 #ifdef TARGET_X86_64
 memcpy(env-hi16_zmm_regs, xsave-region[XSAVE_Hi16_ZMM],
 sizeof env-hi16_zmm_regs);
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 5af4300..9ebdf4b 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2621,10 +2621,10 @@ static inline void gen_sto_env_A0(DisasContext *s, int 
offset)
 
 static inline void gen_op_movo(int d_offset, int s_offset)
 {
-tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, s_offset);
-tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, d_offset);
-tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, s_offset + 8);
-tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, d_offset + 8);
+tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, s_offset + offsetof(XMMReg, 
XMM_Q(0)));
+tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, d_offset + offsetof(XMMReg, 
XMM_Q(0)));
+tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, s_offset + offsetof(XMMReg, 
XMM_Q(1)));
+tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, d_offset + offsetof(XMMReg, 
XMM_Q(1)));
 }
 
 static 

Re: [Qemu-devel] [PATCH 17/19] block/parallels: delay writing to BAT till bdrv_co_flush_to_os

2015-01-14 Thread Denis V. Lunev

On 14/01/15 16:03, Roman Kagan wrote:

On Tue, Dec 30, 2014 at 01:07:10PM +0300, Denis V. Lunev wrote:

The idea is that we do not need to immediately sync BAT to the image as
from the guest point of view there is a possibility that IO is lost
even in the physical controller until flush command was finished.
bdrv_co_flush_to_os is exactly the right place for this purpose.

Technically the patch aligns writes into BAT to MAX(bdrv_align, 4096),
which elliminates read-modify-write transactions on BAT update and
cache ready-to-write content in a special buffer in BDRVParallelsState.

This buffer possibly contains ParallelsHeader if the first page of the
image should be modified. The header occupies first 64 bytes of the image
and the BAT starts immediately after it.

It is also possible that BAT end is not aligned to the cluster size.
ParallelsHeader-data_off is not specified for this case. We should write
only part of the cache in that case.

This patch speed ups
   qemu-img create -f parallels -o cluster_size=64k ./1.hds 64G
   qemu-io -f parallels -c write -P 0x11 0 1024k 1.hds
writing from 50-60 Mb/sec to 80-90 Mb/sec on rotational media and
from 160 Mb/sec to 190 Mb/sec on SSD disk.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
---
  block/parallels.c | 99 ---
  1 file changed, 94 insertions(+), 5 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 46cf031..18b9267 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -62,6 +62,11 @@ typedef struct BDRVParallelsState {
  uint32_t *bat;
  unsigned int bat_size;
  
+uint32_t *bat_cache;

+unsigned bat_cache_size;
+int bat_cache_off;
+int data_off;
+
  unsigned int tracks;
  
  unsigned int off_multiplier;

@@ -148,6 +153,17 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
  le32_to_cpus(s-bat[i]);
  
  qemu_co_mutex_init(s-lock);

+
+s-bat_cache_off = -1;
+if (bs-open_flags  BDRV_O_RDWR) {
+s-bat_cache_size = MAX(bdrv_opt_mem_align(bs-file), 4096);
+s-bat_cache = qemu_blockalign(bs-file, s-bat_cache_size);
+}
+s-data_off = le32_to_cpu(s-ph.data_off) * BDRV_SECTOR_SIZE;
+if (s-data_off == 0) {
+s-data_off = ROUND_UP(bat_offset(s-bat_size), BDRV_SECTOR_SIZE);
+}
+
  return 0;
  
  fail_format:

@@ -171,10 +187,71 @@ static int64_t seek_to_sector(BDRVParallelsState *s, 
int64_t sector_num)
  return (uint64_t)s-bat[index] * s-off_multiplier + offset;
  }
  
+static int write_bat_cache(BlockDriverState *bs)

+{
+BDRVParallelsState *s = bs-opaque;
+int size, off;
+
+if (s-bat_cache_off == -1) {
+/* no cached data */
+return 0;
+}
+
+size = s-bat_cache_size;
+if (size + s-bat_cache_off  s-data_off) {
+/* avoid writing to the first data block */
+size = s-data_off - s-bat_cache_off;
+}
+
+off = s-bat_cache_off;
+s-bat_cache_off = -1;
+return bdrv_pwrite(bs-file, off, s-bat_cache, size);
+}
+
+static int cache_bat(BlockDriverState *bs, uint32_t idx, uint32_t new_data_off)
+{
+int ret, i, off, cache_off;
+int64_t first_idx, last_idx;
+BDRVParallelsState *s = bs-opaque;
+uint32_t *cache = s-bat_cache;
+
+off = bat_offset(idx);
+cache_off = (off / s-bat_cache_size) * s-bat_cache_size;
+
+if (s-bat_cache_off != -1  s-bat_cache_off != cache_off) {
+ret = write_bat_cache(bs);
+if (ret  0) {
+return ret;
+}
+}
+
+first_idx = idx - (off - cache_off) / sizeof(uint32_t);
+last_idx = first_idx + s-bat_cache_size / sizeof(uint32_t);
+if (first_idx  0) {
+memcpy(s-bat_cache, s-ph, sizeof(s-ph));
+first_idx = 0;
+cache = s-bat_cache + sizeof(s-ph) / sizeof(uint32_t);
+}
+
+if (last_idx  s-bat_size) {
+memset(cache + s-bat_size - first_idx, 0,
+   sizeof(uint32_t) * (last_idx - s-bat_size));
+}
+
+for (i = 0; i  last_idx - first_idx; i++) {
+cache[i] = cpu_to_le32(s-bat[first_idx + i]);
+}

You're re-populating the bat_cache on every bat update.  Why?

Shouldn't this be done only with empty cache, i.e. under
if (s-bat_cache_off == -1)?

reasonable, but condition should be a bit different, namely

if (s-bat_cache_off != -1  s-bat_cache_off != cache_off) {






+cache[idx - first_idx] = cpu_to_le32(new_data_off);
+s-bat[idx] = new_data_off;
+
+s-bat_cache_off = cache_off;
+return 0;
+}
+
  static int64_t allocate_sector(BlockDriverState *bs, int64_t sector_num)
  {
  BDRVParallelsState *s = bs-opaque;
-uint32_t idx, offset, tmp;
+uint32_t idx, offset;
  int64_t pos;
  int ret;
  
@@ -190,17 +267,27 @@ static int64_t allocate_sector(BlockDriverState *bs, int64_t sector_num)
  
  pos = bdrv_getlength(bs-file)  BDRV_SECTOR_BITS;

  

Re: [Qemu-devel] [RFC v3 2/2] hw/arm/virt: add generic-pci PCI host controller

2015-01-14 Thread Claudio Fontana
On 14.01.2015 11:16, Alvise Rigo wrote:
 The platform memory map has now three more memory ranges to map the
 device's memory regions (Configuration region, I/O region and Memory
 region).
 
 The dt node interrupt-map property tells how to route the PCI interrupts
 to system interrupts. In the mach-virt case, four IRQs are swizzled
 between all the possible interrupts coming from the PCI bus. In
 particular, the mapping ensures that the first four devices will use a
 system IRQ always different (supposing that only PIN_A of each device is
 used).
 
 Now that a PCI bus is provided, the machine can be launched with
 multiple PCI devices through the -device option (e.g., -device
 virtio-blk-pci).
 
 Signed-off-by: Alvise Rigo a.r...@virtualopensystems.com
 ---
  hw/arm/virt.c | 112 
 +-
  1 file changed, 111 insertions(+), 1 deletion(-)
 
 diff --git a/hw/arm/virt.c b/hw/arm/virt.c
 index 2353440..7b0326f 100644
 --- a/hw/arm/virt.c
 +++ b/hw/arm/virt.c
 @@ -44,6 +44,7 @@
  #include qemu/error-report.h
  
  #define NUM_VIRTIO_TRANSPORTS 32
 +#define NUM_PCI_IRQS 4
  
  /* Number of external interrupt lines to configure the GIC with */
  #define NUM_IRQS 128
 @@ -68,8 +69,12 @@ enum {
  VIRT_UART,
  VIRT_MMIO,
  VIRT_RTC,
 +VIRT_PCI_CFG,
 +VIRT_PCI_IO,
 +VIRT_PCI_MEM,
  VIRT_FW_CFG,
  };
 +#define VIRT_PCI VIRT_PCI_CFG
  
  typedef struct MemMapEntry {
  hwaddr base;
 @@ -129,13 +134,17 @@ static const MemMapEntry a15memmap[] = {
  [VIRT_FW_CFG] = { 0x0902, 0x000a },
  [VIRT_MMIO] =   { 0x0a00, 0x0200 },
  /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size 
 */
 -/* 0x1000 .. 0x4000 reserved for PCI */
 +/* PCI regions */
 +[VIRT_PCI_CFG] ={ 0x1000, 0x0100 },
 +[VIRT_PCI_IO] = { 0x1100, 0x0001 },
 +[VIRT_PCI_MEM] ={ 0x1200, 0x2e00 },
  [VIRT_MEM] ={ 0x4000, 30ULL * 1024 * 1024 * 1024 },
  };
  
  static const int a15irqmap[] = {
  [VIRT_UART] = 1,
  [VIRT_RTC] = 2,
 +[VIRT_PCI] = 3, /* ...to 3 + NUM_PCI_IRQS - 1 */
  [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
  };
  
 @@ -436,6 +445,105 @@ static void create_rtc(const VirtBoardInfo *vbi, 
 qemu_irq *pic)
  g_free(nodename);
  }
  
 +static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
 +{
 +DeviceState *dev;
 +SysBusDevice *busdev;
 +int i, j, intmap_rows;
 +const MemMapEntry *mme = vbi-memmap;
 +uint32_t plat_acells;
 +uint32_t plat_scells;
 +uint32_t gic_phandle;
 +char *nodename;
 +
 +dev = qdev_create(NULL, generic_pci);
 +qdev_prop_set_uint64(dev, mmio_win_size, mme[VIRT_PCI_MEM].size);
 +qdev_prop_set_uint64(dev, mmio_win_addr, mme[VIRT_PCI_MEM].base);
 +qdev_prop_set_uint32(dev, irqs, NUM_PCI_IRQS);
 +qdev_init_nofail(dev);
 +
 +busdev = SYS_BUS_DEVICE(dev);
 +sysbus_mmio_map(busdev, 0, mme[VIRT_PCI_CFG].base); /* PCI config */
 +sysbus_mmio_map(busdev, 1, mme[VIRT_PCI_IO].base);  /* PCI I/O */
 +sysbus_mmio_map(busdev, 2, mme[VIRT_PCI_MEM].base); /* PCI memory window 
 */
 +
 +for ( i = 0; i  NUM_PCI_IRQS; i++) {
 +sysbus_connect_irq(busdev, i, pic[vbi-irqmap[VIRT_PCI] + i]);
 +}
 +
 +/* add device tree node */
 +nodename = g_strdup_printf(/pci@% PRIx64, mme[VIRT_PCI_CFG].base);
 +qemu_fdt_add_subnode(vbi-fdt, nodename);
 +qemu_fdt_setprop_string(vbi-fdt, nodename, compatible,
 + pci-host-cam-generic);
 +qemu_fdt_setprop_string(vbi-fdt, nodename, device_type, pci);
 +qemu_fdt_setprop_cell(vbi-fdt, nodename, #address-cells, 0x3);
 +qemu_fdt_setprop_cell(vbi-fdt, nodename, #size-cells, 0x2);
 +qemu_fdt_setprop_cell(vbi-fdt, nodename, #interrupt-cells, 0x1);
 +
 +plat_acells = qemu_fdt_getprop_cell(vbi-fdt, /, #address-cells);
 +plat_scells = qemu_fdt_getprop_cell(vbi-fdt, /, #size-cells);
 +qemu_fdt_setprop_sized_cells(vbi-fdt, nodename, reg,
 +plat_acells, mme[VIRT_PCI_CFG].base,
 +plat_scells, mme[VIRT_PCI_CFG].size);
 +
 +/* Two regions (second and third of *busdev):
 +   - I/O region, PCI addr = 0x0, CPU addr = mme[VIRT_PCI_IO].base,
 + size = mme[VIRT_PCI_IO].size,
 +   - 32bit MMIO region, PCI addr = CPU addr = mme[VIRT_PCI_MEM].base,
 + size = mme[VIRT_PCI_MEM].size */
 +qemu_fdt_setprop_sized_cells(vbi-fdt, nodename, ranges,
 +1, 0x0100, 2, 0x, /* I/O space */
 +2, mme[VIRT_PCI_IO].base, 2, mme[VIRT_PCI_IO].size,
 +1, 0x0200, 2, mme[VIRT_PCI_MEM].base, /* 32bit memory space 
 */
 +2, mme[VIRT_PCI_MEM].base, 2, mme[VIRT_PCI_MEM].size);
 +
 +gic_phandle = qemu_fdt_get_phandle(vbi-fdt, /intc);
 +
 +#define IRQ_MAP_ENTRY_DESC_SZ 14
 +#define 

Re: [Qemu-devel] [PATCH 1/9] qmp hmp: Factor out common using spice test

2015-01-14 Thread Eric Blake
On 01/13/2015 10:50 AM, Markus Armbruster wrote:
 Into qemu_using_spice().  For want of a better place, put it next the
 existing monitor command handler dummies in qemu-spice.h.
 
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---
  include/ui/qemu-spice.h | 10 ++
  monitor.c   |  5 +++--
  qmp.c   | 11 +++
  3 files changed, 16 insertions(+), 10 deletions(-)
 
 diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
 index a93b4b2..b3f2679 100644
 --- a/include/ui/qemu-spice.h
 +++ b/include/ui/qemu-spice.h
 @@ -88,4 +88,14 @@ static inline int qemu_spice_display_add_client(int csock, 
 int skipauth,
  
  #endif /* CONFIG_SPICE */
  
 +static inline int qemu_using_spice(Error **errp)

Why not s/int/bool/

 +{
 +if (!using_spice) {
 +/* correct one? spice isn't a device ,,, */
 +error_set(errp, QERR_DEVICE_NOT_ACTIVE, spice);
 +return 0;
 +}
 +return 1;

and use true/false here? All callers use it only in a boolean context.

Either way,
Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC v3 1/2] pci/pci-host: Add generic-pci PCI host controller device

2015-01-14 Thread Claudio Fontana
On 14.01.2015 11:16, Alvise Rigo wrote:
 Add a generic PCI host controller for virtual platforms, based on the
 previous work by Rob Herring:
 http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
 
 The controller relies on a configuration memory region and provides two
 PCI memory regions for I/O (one port and one memory mapped).  The device
 needs the following qdev properties to configure the memory regions:
 - cfg_win_size: size of the configuration memory
 - pio_win_size: size of the port I/O space
 - mmio_win_size: size of the MMIO space
 - mmio_win_addr: offset of MMIO space in the system memory
 
 Signed-off-by: Alvise Rigo a.r...@virtualopensystems.com
 ---
  hw/pci-host/Makefile.objs |   2 +-
  hw/pci-host/generic-pci.c | 140 
 ++
  include/hw/pci-host/generic-pci.h |  45 
  3 files changed, 186 insertions(+), 1 deletion(-)
  create mode 100644 hw/pci-host/generic-pci.c
  create mode 100644 include/hw/pci-host/generic-pci.h
 
 diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
 index bb65f9c..8ef9fac 100644
 --- a/hw/pci-host/Makefile.objs
 +++ b/hw/pci-host/Makefile.objs
 @@ -1,4 +1,4 @@
 -common-obj-y += pam.o
 +common-obj-y += pam.o generic-pci.o
  
  # PPC devices
  common-obj-$(CONFIG_PREP_PCI) += prep.o
 diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
 new file mode 100644
 index 000..54c9647
 --- /dev/null
 +++ b/hw/pci-host/generic-pci.c
 @@ -0,0 +1,140 @@
 +/*
 + * Generic PCI host controller
 + *
 + * Copyright (c) 2014 Linaro, Ltd.
 + * Author: Rob Herring rob.herr...@linaro.org
 + *
 + * Based on ARM Versatile PCI controller (hw/pci-host/versatile.c):
 + * Copyright (c) 2006-2009 CodeSourcery.
 + * Written by Paul Brook
 + *
 + * This code is licensed under the LGPL.
 + */
 +
 +#include hw/sysbus.h
 +#include hw/pci-host/generic-pci.h
 +#include exec/address-spaces.h
 +#include sysemu/device_tree.h
 +
 +static const VMStateDescription pci_generic_host_vmstate = {
 +.name = generic-host-pci,
 +.version_id = 1,
 +.minimum_version_id = 1,
 +};
 +
 +static void pci_cam_config_write(void *opaque, hwaddr addr,
 + uint64_t val, unsigned size)
 +{
 +PCIGenState *s = opaque;
 +pci_data_write(s-pci_bus, addr, val, size);
 +}
 +
 +static uint64_t pci_cam_config_read(void *opaque, hwaddr addr, unsigned size)
 +{
 +PCIGenState *s = opaque;
 +uint32_t val;
 +val = pci_data_read(s-pci_bus, addr, size);
 +return val;
 +}
 +
 +static const MemoryRegionOps pci_generic_config_ops = {
 +.read = pci_cam_config_read,
 +.write = pci_cam_config_write,
 +.endianness = DEVICE_LITTLE_ENDIAN,
 +};
 +
 +static void pci_generic_set_irq(void *opaque, int irq_num, int level)
 +{
 +qemu_irq *pic = opaque;
 +qemu_set_irq(pic[irq_num], level);
 +}
 +
 +static void pci_generic_host_realize(DeviceState *dev, Error **errp)
 +{
 +PCIHostState *h = PCI_HOST_BRIDGE(dev);
 +PCIGenState *s = PCI_GEN(dev);
 +GenericPCIHostState *gps = s-pci_gen;
 +SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 +int i;
 +
 +memory_region_init(s-pci_io_window, OBJECT(s), pci_io, 
 s-pio_win_size);
 +memory_region_init(s-pci_mem_space, OBJECT(s), pci_mem, 1ULL  32);
 +
 +pci_bus_new_inplace(s-pci_bus, sizeof(s-pci_bus), dev, pci,
 +s-pci_mem_space, s-pci_io_window,
 +PCI_DEVFN(0, 0), TYPE_PCI_BUS);
 +h-bus = s-pci_bus;
 +
 +object_initialize(gps, sizeof(*gps), TYPE_GENERIC_PCI_HOST);
 +qdev_set_parent_bus(DEVICE(gps), BUS(s-pci_bus));
 +
 +for (i = 0; i  s-irqs; i++) {
 +sysbus_init_irq(sbd, s-irq[i]);
 +}
 +
 +pci_bus_irqs(s-pci_bus, pci_generic_set_irq, pci_swizzle_map_irq_fn,
 + s-irq, s-irqs);
 +memory_region_init_io(s-mem_config, OBJECT(s), 
 pci_generic_config_ops, s,
 +  pci-config, s-cfg_win_size);
 +memory_region_init_alias(s-pci_mem_window, OBJECT(s), pci-mem-win,
 +s-pci_mem_space, s-mmio_win_addr, s-mmio_win_size);
 +
 +sysbus_init_mmio(sbd, s-mem_config);
 +sysbus_init_mmio(sbd, s-pci_io_window);
 +sysbus_init_mmio(sbd, s-pci_mem_window);
 +}
 +
 +static void pci_generic_host_class_init(ObjectClass *klass, void *data)
 +{
 +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 +DeviceClass *dc = DEVICE_CLASS(klass);
 +
 +k-vendor_id = PCI_VENDOR_ID_REDHAT;
 +k-device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
 +k-class_id = PCI_CLASS_PROCESSOR_CO;
 +/*
 + * PCI-facing part of the host bridge, not usable without the
 + * host-facing part, which can't be device_add'ed, yet.
 + */
 +dc-cannot_instantiate_with_device_add_yet = true;
 +}
 +
 +static const TypeInfo pci_generic_host_info = {
 +.name  = TYPE_GENERIC_PCI_HOST,
 +.parent= TYPE_PCI_DEVICE,
 +

Re: [Qemu-devel] [PATCH 2/9] qmp hmp: Improve error messages when SPICE is not in use

2015-01-14 Thread Eric Blake
On 01/13/2015 10:50 AM, Markus Armbruster wrote:
 Commit 7572150 adopted QERR_DEVICE_NOT_ACTIVE for the purpose,
 probably because adding another error seemed cumbersome overkill.
 Produces No spice device has been activated, which is awkward.
 
 We've since abandoned our quest for rich error objects.  Time to
 undo the damage to this error message.  Replace it by SPICE is not in
 use.
 
 Keep the stupid DeviceNotActive ErrorClass for compatibility, even
 though Libvirt doesn't use it.

Libvirt has a few places that look explicitly for DeviceNotActive when
deciding how to handle failure reporting; I didn't audit closely enough
to see if it actually changes behavior if the class were to disappear.
Your approach in this patch is safest, at any rate.

 
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---
  include/ui/qemu-spice.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 1/4] target-i386: fix movntsd on big-endian hosts

2015-01-14 Thread Eduardo Habkost
On Tue, Jan 13, 2015 at 08:49:19PM +0100, Paolo Bonzini wrote:
 
 
 On 13/01/2015 19:48, Eduardo Habkost wrote:
if (b1  1) {
   -gen_stq_env_A0(s, offsetof(CPUX86State, xmm_regs[reg]));
   +gen_stq_env_A0(s, offsetof(CPUX86State,
   +   xmm_regs[reg].XMM_Q(0)));
  Do we have (or will patch 4/4 introduce) the same bug on the
  tcg_gen_addi_ptr() calls that don't use the XMM_Q macro?
 
 No, they all call into helpers that use the XMM_Q macro themselves.

tcg_gen_addi_ptr() is called sometimes using the fpregs[reg].mmx offset,
and sometimes using the xmm_regs[reg] offset. How can it know if the
XMM_Q macro is necessary or not?

-- 
Eduardo



Re: [Qemu-devel] [PATCH 1/4] target-i386: fix movntsd on big-endian hosts

2015-01-14 Thread Paolo Bonzini


On 14/01/2015 14:17, Eduardo Habkost wrote:
   Do we have (or will patch 4/4 introduce) the same bug on the
   tcg_gen_addi_ptr() calls that don't use the XMM_Q macro?
  
  No, they all call into helpers that use the XMM_Q macro themselves.
 tcg_gen_addi_ptr() is called sometimes using the fpregs[reg].mmx offset,
 and sometimes using the xmm_regs[reg] offset. How can it know if the
 XMM_Q macro is necessary or not?

It can't, but I audited the calls.

Note that one helper is foo_xmm, the other is foo_mmx:

tcg_gen_addi_ptr(cpu_ptr0, cpu_env, 
offsetof(CPUX86State,xmm_regs[rm]));
gen_helper_pmovmskb_xmm(cpu_tmp2_i32, cpu_env, cpu_ptr0);
} else {
rm = (modrm  7);
tcg_gen_addi_ptr(cpu_ptr0, cpu_env, 
offsetof(CPUX86State,fpregs[rm].mmx));
gen_helper_pmovmskb_mmx(cpu_tmp2_i32, cpu_env, cpu_ptr0);

Paolo



Re: [Qemu-devel] [PATCH 4/9] qmp: Clean up qmp_query_spice() #ifndef !CONFIG_SPICE dummy

2015-01-14 Thread Eric Blake
On 01/13/2015 10:50 AM, Markus Armbruster wrote:
 QMP command query-spice exists only #ifdef CONFIG_SPICE.  Due to QAPI
 limitations, we need a dummy function anyway, but it's unreachable.
 
 Our current dummy function goes out of its way to produce the exact
 same error as the QMP core does for unknown commands.  Cute, but both
 unclean and unnecessary.  Replace by straight abort().
 
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---

 + * qmp-commands.hx ensures that QMP command query-spice exists only
 + * #ifdef CONFIG_SPICE.  Necessary for an accurate query-commands
 + * result.  However, the QAPI schema is blissfully unaware of that,
 + * and the QAPI code generator happily generates a dead
 + * qmp_marshal_input_query_spice() that calls qmp_query_spice().
 + * Provide it one, or else linking fails.
 + * FIXME Educate the QAPI schema on CONFIG_SPICE.

There's probably several commands that are only conditionally compiled
in qmp-commands.hx, but where the qapi has no way to express that they
are conditional on how the binary was compiled.  I'm not sure if it
would help the user to document that a command listed in the .json might
not actually exist, but if enhancing the qapi code generator to
understand conditionals made for less maintenance, then that alone would
be worth cleaning up this FIXME.

In the meantime, your patch is just fine.

Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 8/9] balloon: Factor out common is balloon active test

2015-01-14 Thread Eric Blake
On 01/13/2015 10:50 AM, Markus Armbruster wrote:
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---
  balloon.c | 29 +++--
  1 file changed, 15 insertions(+), 14 deletions(-)
 
 diff --git a/balloon.c b/balloon.c
 index 2884c2d..aa30617 100644
 --- a/balloon.c
 +++ b/balloon.c
 @@ -36,6 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn;
  static QEMUBalloonStatus *balloon_stat_fn;
  static void *balloon_opaque;
  
 +static int have_ballon(Error **errp)
 +{
 +if (kvm_enabled()  !kvm_has_sync_mmu()) {
 +error_set(errp, QERR_KVM_MISSING_CAP, synchronous MMU, balloon);
 +return 0;
 +}
 +if (!balloon_event_fn) {
 +error_set(errp, QERR_DEVICE_NOT_ACTIVE, balloon);
 +return 0;
 +}
 +return 1;

Another case where I would have used bool, true, false.

Either way,
Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v5 0/5] Geometry and blocksize detection for backing devices.

2015-01-14 Thread Stefan Hajnoczi
On Tue, Jan 13, 2015 at 08:07:15PM +0100, Christian Borntraeger wrote:
 Am 13.01.2015 um 17:04 schrieb Stefan Hajnoczi:
 [...]
  I'm really starting to get worried that you are going to break things.
  This DASD hack is a layering violation but okay, go ahead if you want.
  But now you are also thinking about breaking live migration.
  
  The proper thing to do is to introduce libvirt XML syntax for DASD.
  That way the geometry can be handled as part of the machine
  configuration.  Then live migration and storage management tools can do
  the right thing.
  
  I've said this should be done in libvirt repeatedly but you keep wanting
  to hack QEMU instead of doing this cleanly :(.
  
  If you have plans to expand on this hack, please scrap this series and
  introduce libvirt XML syntax instead.
 
 We had no plans to expand on this band-aid. I just tried to come up with 
 ideas beyond this hack to make this more acceptible to you (because I though 
 that
 you want something on top). Seems that I misunderstood you, so if you  prefer
 to just keep this hack and not do anything further in that direction, this is
 totally fine with me.
 
 So lets just fix up the small nits and go ahead, ok?

Yes, I'm fine with this series (modulo the review comments which require
a v6).

Stefan


pgpQDfp1dN8wP.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH 1/2] qed: check for header size overflow

2015-01-14 Thread Kevin Wolf
Am 12.01.2015 um 13:31 hat Stefan Hajnoczi geschrieben:
 Header size is denoted in clusters.  The maximum cluster size is 64 MB
 but there is no limit on header size.  Check for uint32_t overflow in
 case the header size field has a whacky value.
 
 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
 ---
  block/qed.c | 6 ++
  1 file changed, 6 insertions(+)
 
 diff --git a/block/qed.c b/block/qed.c
 index 80f18d8..d2c6045 100644
 --- a/block/qed.c
 +++ b/block/qed.c
 @@ -440,6 +440,12 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict 
 *options, int flags,
  s-l2_mask = s-table_nelems - 1;
  s-l1_shift = s-l2_shift + ffs(s-table_nelems) - 1;
  
 +/* Header size calculation must not overflow uint32_t */
 +if ((uint64_t)s-header.cluster_size * s-header.header_size !=
 +s-header.cluster_size * s-header.header_size) {

Generally, I find checks that don't exercise the integer overflow easier
to read, i.e. in this case:

if (s-header.header_size  UINT32_MAX / s-header.cluster_size)

Or even just INT_MAX to be on the safe side. But I'll admit that it's a
matter of taste.

 +return -EINVAL;
 +}
 +
  if ((s-header.features  QED_F_BACKING_FILE)) {
  if ((uint64_t)s-header.backing_filename_offset +
  s-header.backing_filename_size 

Series: Reviewed-by: Kevin Wolf kw...@redhat.com



Re: [Qemu-devel] [PATCH v2 2/2] e1000: decrement RDT if equal to RDH

2015-01-14 Thread Jason Wang

On 01/14/2015 05:06 AM, Richard Tollerton wrote:
 Jason Wang jasow...@redhat.com writes:

 On Tue, Jan 13, 2015 at 3:12 AM, Richard Tollerton rich.toller...@ni.com 
 wrote:
 On Thu, Dec 18, 2014 at 12:01:48AM -0500, Jason Wang wrote:

   Some drivers set RDT=RDH. Oddly, this works on real hardware. To
   work around this, autodecrement RDT when this happens.
  
  Please describe more details on the issue. The spec 3.2.6 said:
 When the head pointer is equal to the tail pointer, the ring is
 empty. So RDT=RDH in fact empty the ring. No?
 That is incorrect; the spec explicitly states that RDT=RDH means the
 ring is full. The linux e1000 driver more or less implies the same
 thing.

 You forgot to include the sentence after that in section 3.2.6 :)

 When the head pointer is equal to the tail pointer, the ring is
 empty. Hardware stops storing packets in system memory until software
 advances the tail pointer, making more receive buffers available.

 Yeah, this seems really poorly worded to me too. :( You appear to be
 interpreting ring is empty in the usual sense, i.e. all N elements
 of the ring buffer are available for use by hardware. In fact, the
 correct interpretation [1] is the exact opposite, none of the
 elements are available for use by hardware.
 Yes, I do think 'empty' means no available buffer for device to receive 
 :)
 Ah, ok. But if you're concerned about breaking drivers with this... what
 legitimate reason could a driver possibly have to set RDT=RDH? (Besides a
 mistaken attempt to free the ring for hardware use, as I posit?)

One example is initialization in Linux (e1000_configure_rx()). What it
does is:

/* disable receives while setting up the descriptors */
rctl = er32(RCTL);
ew32(RCTL, rctl  ~E1000_RCTL_EN);
...
ew32(RDT, 0);
ew32(RDH, 0);
...
/* Enable Receives */
ew32(RCTL, rctl | E1000_RCTL_EN);

And the rx buffer allocations were done later. So with your patch, after
rx was enabled but before  rx buffer were allocated. Since RDT was set
to RDLEN, e1000_has_rxbuf() will return true. Qemu will try to receive
packet to uninitialized buffers. This seems wrong.

 The only reason I can think of is that maybe a driver is trying to
 implement some sort of crude flow control. But if that actually worked,
 then major packet loss would be observed under load, as any packets
 received by hardware but not yet processed by software would get
 dropped.

 I'm going to go out (further) on a limb and assert that no driver ever
 sets RDT=RDH to stop receives, because no hardware implements that
 behavior. The driver I'm trying to get working appears to have been
 setting RDT=RDH at the end of *every* iteration of the receive loop,
 since it was originally written in 2003. If I am to trust the comments,
 it's been ported/supported on 28 different chipset variants, and it's
 definitely been tested for performance and packet loss under load for a
 good number of those, including under polling modes where the ring is
 only checked every few milliseconds. If RDT=RDH ever did anything except
 free all buffers for hardware use, surely catastrophic packet loss would
 have been observed?

From device's point of view. It just need stop receiving when RDT=RDH.
It does not care whether the ring was full of received packets or empty.

I probably miss something but could you please show the (pseudo) code of
your driver to see why current qemu does not work?

 The last sentence in the quote makes this explicit. See also linux
 e1000 driver sources at [2] [3] [4].
 Btw, [2],[3],[4] are all codes that deal with driver's internal
 variable, not the one that the hardware use.
 No, it's directly used by hardware -- current_count increments RDT. See
 e1000_alloc_rx_buffers().

Do you mean cleaned_count? (Didn't find current_count).

 I'm *guessing* (?) that the DMA engine isn't correspondingly stopped
 when software sets RDT=RDH, so that once packets start getting
 received,
 Do you mean in qemu? I/O are single threaded, so looks like we are
 safe.
 I'm referring to the possibility that physical hardware is doing this.

 Interesting side note: while this driver sets RDT=RDH on every
 iteration, it *initializes* RDT=0 and RDH=1...

I want to know more about this driver.

RDT=0 and RDH=1 means all buffers were available for device to receive.
If no rx buffer refilling happens, RDH will be finally advanced by
device and finally equal to RDT and then receiving is stopped. When will
driver set RDT=RDH?
 the hardware can more or less ignore it. In this context, my patch
 makes sense.

 (Yes, this is totally an ex-post-facto justification for the patch; it
 arrived to me secondhand, and I had not been familiar with the driver
 source before now.)
 True, we've found many undocumented behavior in the past (some even 
 conflicts with spec). I don't have a 82540EM in my hand, but I think 
 the best thing is to check this behavior in real hardware to prevent 
 this patch from breaking many 

[Qemu-devel] [PATCH v9 1/4] hw/arm/sysbus-fdt: helpers for platform bus nodes addition

2015-01-14 Thread Eric Auger
This new C module will be used by ARM machine files to generate
platform bus node and their dynamic sysbus device tree nodes.

Dynamic sysbus device node addition is done in a machine init
done notifier. arm_register_platform_bus_fdt_creator does the
registration of this latter and is supposed to be called by
ARM machine files that support platform bus and their dynamic
sysbus. Addition of dynamic sysbus nodes is done only if the
user did not provide any dtb.

Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Eric Auger eric.au...@linaro.org
Reviewed-by: Shannon Zhao zhaoshengl...@huawei.com
Reviewed-by: Alexander Graf ag...@suse.de

---
v8 - v9:
- s/Fdt/FDT in struct type names
- reorder fields in PlatformBusFdtNotifierParams and use DO_UPCAST
  instead of container_of
- use assert() when relevant (board model issue)
- g_free the ARMPlatformBusFDTParams and PlatformBusFDTNotifierParams
  pointers in platform_bus_fdt_notify

v7 - v8:
add Reviewed-by from Alex and Shannon

v6 - v7:
- revert indentation in add_fdt_node_functions

v5 - v6:
- add_all_platform_bus_fdt_nodes is not a modify_dtb function anymore
- it now takes a handle to an ARMPlatformBusFdtParams.
- fdt pointer is checked in case this notifier is executed after the
  one that executes the load_dtb (this latter deallocates the fdt pointer)
- check of fdt_filename moved in here.
- upgrade_dtb is removed
- copyright aligned between .h and .c

v4 - v5:
- change indentation in add_fdt_node_functions. Also becomes a
  static const.
- ARMPlatformBusFdtParams.system_params becomes a pointer to
  a const ARMPlatformBusSystemParams
- removes platform-bus.h second inclusion

v3 - v4:
- dyn_sysbus_devtree.c renamed into sysbus-fdt.c
- use new PlatformBusDevice object
- the dtb upgrade is done through modify_dtb. Before the fdt
  was recreated from scratch. When the user provided a dtb this
  latter was overwritten which was not correct.
- an array contains the association between device type names
  and their node creation function
- I must aknowledge I did not find any cleaner way to implement
  a FDT_BUILDER interface, as suggested by Paolo. The class method
  would need to be initialized somewhere and since it cannot
  happen in the device itself - according to Alex  Peter comments -,
  I don't see when I shall associate the device type and its
  interface implementation.

v2 - v3:
- add arm_ prefix
- arm_sysbus_device_create_devtree becomes static

v1 - v2:
- Code moved in an arch specific file to accomodate architecture
  dependent specificities.
- remove platform_bus_base from PlatformDevtreeData

v1: code originally written by Alex Graf in e500.c and reused for
ARM [Eric Auger]
---
 hw/arm/Makefile.objs|   1 +
 hw/arm/sysbus-fdt.c | 173 
 include/hw/arm/sysbus-fdt.h |  60 +++
 3 files changed, 234 insertions(+)
 create mode 100644 hw/arm/sysbus-fdt.c
 create mode 100644 include/hw/arm/sysbus-fdt.h

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 6088e53..0cc63e1 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -3,6 +3,7 @@ obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
+obj-y += sysbus-fdt.o
 
 obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-$(CONFIG_DIGIC) += digic.o
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
new file mode 100644
index 000..ae14dbc
--- /dev/null
+++ b/hw/arm/sysbus-fdt.c
@@ -0,0 +1,173 @@
+/*
+ * ARM Platform Bus device tree generation helpers
+ *
+ * Copyright (c) 2014 Linaro Limited
+ *
+ * Authors:
+ *  Alex Graf ag...@suse.de
+ *  Eric Auger eric.au...@linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see http://www.gnu.org/licenses/.
+ *
+ */
+
+#include hw/arm/sysbus-fdt.h
+#include qemu/error-report.h
+#include sysemu/device_tree.h
+#include hw/platform-bus.h
+#include sysemu/sysemu.h
+
+/*
+ * internal struct that contains the information to create dynamic
+ * sysbus device node
+ */
+typedef struct PlatformBusFDTData {
+void *fdt; /* device tree handle */
+int irq_start; /* index of the first IRQ usable by platform bus devices */
+const char *pbus_node_name; /* name of the platform bus node */
+PlatformBusDevice *pbus;
+} PlatformBusFDTData;
+
+/*

[Qemu-devel] [PATCH v9 2/4] hw/arm/boot: arm_load_kernel implemented as a machine init done notifier

2015-01-14 Thread Eric Auger
Device tree nodes for the platform bus and its children dynamic sysbus
devices are added in a machine init done notifier. To load the dtb once,
after those latter nodes are built and before ROM freeze, the actual
arm_load_kernel existing code is moved into a notifier notify function,
arm_load_kernel_notify. arm_load_kernel now only registers the
corresponding notifier.

Machine files that do not support platform bus stay unchanged. Machine
files willing to support dynamic sysbus devices must call arm_load_kernel
before sysbus-fdt arm_register_platform_bus_fdt_creator to make sure
dynamic sysbus device nodes are integrated in the dtb.

Signed-off-by: Eric Auger eric.au...@linaro.org
Reviewed-by: Shannon Zhao zhaoshengl...@huawei.com
Reviewed-by: Alexander Graf ag...@suse.de

---

v8 - v9:
- fix compilation with arm-linux-user
- reorder fields in ArmLoadKernelNotifier and use DO_UPCAST

v7 - v8:
- Add Reviewed-by from Alex  Shannon
- rebase on 2.2.0

v6: creation of this patch file
---
 hw/arm/boot.c| 14 +-
 include/hw/arm/arm.h | 28 
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 52ebd8b..1af00e0 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -537,7 +537,7 @@ static void load_image_to_fw_cfg(FWCfgState *fw_cfg, 
uint16_t size_key,
 fw_cfg_add_bytes(fw_cfg, data_key, data, size);
 }
 
-void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
+static void arm_load_kernel_notify(Notifier *notifier, void *data)
 {
 CPUState *cs;
 int kernel_size;
@@ -548,6 +548,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 hwaddr entry, kernel_load_offset;
 int big_endian;
 static const ARMInsnFixup *primary_loader;
+ArmLoadKernelNotifier *n = DO_UPCAST(ArmLoadKernelNotifier,
+ notifier, notifier);
+ARMCPU *cpu = n-cpu;
+struct arm_boot_info *info =
+container_of(n, struct arm_boot_info, load_kernel_notifier);
 
 /* CPU objects (unlike devices) are not automatically reset on system
  * reset, so we must always register a handler to do so. If we're
@@ -755,3 +760,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 ARM_CPU(cs)-env.boot_info = info;
 }
 }
+
+void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
+{
+info-load_kernel_notifier.cpu = cpu;
+info-load_kernel_notifier.notifier.notify = arm_load_kernel_notify;
+qemu_add_machine_init_done_notifier(info-load_kernel_notifier.notifier);
+}
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index c4bf56d..3b4cfdd 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -13,12 +13,22 @@
 
 #include exec/memory.h
 #include hw/irq.h
+#include qemu/notify.h
 
 /* armv7m.c */
 qemu_irq *armv7m_init(MemoryRegion *system_memory,
   int flash_size, int sram_size,
   const char *kernel_filename, const char *cpu_model);
 
+/*
+ * struct used as a parameter of the arm_load_kernel machine init
+ * done notifier
+ */
+typedef struct {
+Notifier notifier; /* actual notifier */
+ARMCPU *cpu; /* handle to the first cpu object */
+} ArmLoadKernelNotifier;
+
 /* arm_boot.c */
 struct arm_boot_info {
 uint64_t ram_size;
@@ -65,6 +75,8 @@ struct arm_boot_info {
  * the user it should implement this hook.
  */
 void (*modify_dtb)(const struct arm_boot_info *info, void *fdt);
+/* machine init done notifier executing arm_load_dtb */
+ArmLoadKernelNotifier load_kernel_notifier;
 /* Used internally by arm_boot.c */
 int is_linux;
 hwaddr initrd_start;
@@ -76,6 +88,22 @@ struct arm_boot_info {
  */
 bool firmware_loaded;
 };
+
+/**
+ * arm_load_kernel - Loads memory with everything needed to boot
+ *
+ * @cpu: handle to the first CPU object
+ * @info: handle to the boot info struct
+ * Registers a machine init done notifier that copies to memory
+ * everything needed to boot, depending on machine and user options:
+ * kernel image, boot loaders, initrd, dtb. Also registers the CPU
+ * reset handler.
+ *
+ * In case the machine file supports the platform bus device and its
+ * dynamically instantiable sysbus devices, this function must be called
+ * before sysbus-fdt arm_register_platform_bus_fdt_creator. Indeed the
+ * machine init done notifiers are called in registration reverse order.
+ */
 void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info);
 
 /* Multiplication factor to convert from system clock ticks to qemu timer
-- 
1.8.3.2




[Qemu-devel] [PATCH v9 0/4] machvirt dynamic sysbus device instantiation

2015-01-14 Thread Eric Auger
This patch series enables machvirt to dynamically instantiate sysbus
devices from command line (using -device option).

All those sysbus devices are plugged onto a platform bus. This latter
device is instantiated in machvirt and takes care of the binding of
children sysbus devices on a machine init done notifier. The device
tree node generation for children dynamic sysbus device also happens
on a subsequent notifier that must be executed after the above one.
machvirt registers that notifier before the platform bus creation to
make sure notifiers are executed in the right order: dt generation after
actual QOM binding.

Very few sysbus devices are supposed to be instantiated that
way. VFIO devices belong to them.

Node creation really is architecture specific. On ARM the dynamic
sysbus device node creation is implemented in a new C module,
hw/arm/sysbus-fdt.c and not in the machine file.

Machvirt transformations and sysbus-fdt are largely inspired from Alex work.

The patch series can be found at:
http://git.linaro.org/people/eric.auger/qemu.git,
branch official_dynsysbus_v9
Previous version on official_dynsysbus_v8

Best Regards

Eric

v8 - v9:
Take into account Peter's comments:
hw/arm/sysbus-fdt:
- s/Fdt/FDT in struct type names
- reorder fields in PlatformBusFdtNotifierParams and use DO_UPCAST
- use assert() when relevant (board model issue)
- g_free the ARMPlatformBusFDTParams and PlatformBusFDTNotifierParams
  pointers in platform_bus_fdt_notify
hw/arm/boot.c
- fix compilation with arm-linux-user
- reorder fields in ArmLoadKernelNotifier
hw/arm/virt.c
- PLATFORM_BUS_NUM_IRQS set to 32 instead of 20
- platform bus irq now start at 64 instead of 48
- remove change of indentation in  a15memmap
- correct misc style issues

add a separate patch file for re-indentation invirt a15memmap

v7 - v8:
- rebase on 2.2.0 (boot.c and virt.c)
- in virt.c machvirt_init, create_platform_bus simply is added
  after the arm_load_kernel call instead of moving this latter.
  should ease the review.
- Add Alex  Shannon Reviewed-by

v6 - v7:
Take into account Shannon's comments:
- hw/arm/sysbus-fdt.c: revert indentation in add_fdt_node_functions
- hw/arm/virt.c:
  - add an additional comment in a15irqmap related to PLATFORM_BUS_NUM_IRQS
  - correct platform bus size to 0x40
  - remove PLATFORM_BUS_FIRST_IRQ macro

v5 - v6:
Take into account Peter's comments:
- dtb overload mechanism rewritten: arm_load_kernel code is moved into a
  machine init done notifier notify instead. arm_load_kernel only registers
  that notifier. As a consequence the dtb is loaded once.
- v5 1-4 patch files are removed and replaced by a single patch file moving
  arm_load_kernel in the notifier (2).
- as a consequence arm_load_kernel must be called before sysbus-fdt
  arm_register_platform_bus_fdt_creator.
- In virt, platform_bus_params not a const anymore since its fields are
  initialized from vbi-memmap and vbi-irqmap. Hence create_platform_bus
  proto can be simplified.
- In sysbus-fdt add_all_platform_bus_fdt_nodes now takes a handle to
  an ARMPlatformBusFdtParams. This is not a modify_dtb function anymore
  fdt pointer is checked in case the callback is called after the load_dtb
  (this latter deallocated fdt pointer). Check of fdt_filename moved in here.
  upgrade_dtb is removed. copyright aligned between .h and .c.

v4 - v5:
- in virt.c: platform_bus_params becomes static const
- sysbus-fdt: change indentation in add_fdt_node_functions array init
- s/load_dtb/arm_load_dtb in one boot.c comment

v3 - v4:
- dyn_sysbus_binding removed since binding stuff now are implemented by
  the platform bus device
- due to a change in ARM load_dtb implementation using rom_add_blob_fixed,
  the dt no more is generated in a reset notifier but is generated on a
  machine init done notifier
- the augmented device tree is not generated from scratch anymore but is
  added using a modify_dtb function. This required some small change in
  boot.c
- the case where the user provides a dtb file now is handled
- some cleanup in virt additions
- implement a list of dyanmically instantiable devices in sysbus-fdt

v2 - v3:
- patch now applies on top of Alex full patchset
- dyn_sysbus_devtree: add arm_prefix to emphasize the fact those
  functions are arm specific; arm_sysbus_device_create_devtree
  becomes static
- load_dtb renamed into arm_load_dtb
- add copyright in hw/arm/dyn_sysbus_devtree.c

v1 - v2:
- device node generation no more in sysbus device but in
  dyn_sysbus_devtree
- VFIO region shrinked to 4MB and relocated in machvirt to avoid PCI
  shrink (dynamic vfio-mmio support might come latter)
- platform_bus_base removed from PlatformDevtreeData

Eric Auger (4):
  hw/arm/sysbus-fdt: helpers for platform bus nodes addition
  hw/arm/boot: arm_load_kernel implemented as a machine init done
notifier
  hw/arm/virt: add dynamic sysbus device support
  hw/arm/virt: change indentation in a15memmap

 hw/arm/Makefile.objs|   1 +
 hw/arm/boot.c 

[Qemu-devel] [PATCH v9 3/4] hw/arm/virt: add dynamic sysbus device support

2015-01-14 Thread Eric Auger
Allows sysbus devices to be instantiated from command line by
using -device option. Machvirt creates a platform bus at init.
The dynamic sysbus devices are attached to this platform bus device.

The platform bus device registers a machine init done notifier
whose role will be to bind the dynamic sysbus devices. Indeed
dynamic sysbus devices are created after machine init.

machvirt also registers a notifier that will build the device
tree nodes for the platform bus and its children dynamic sysbus
devices.

Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Eric Auger eric.au...@linaro.org

---
v8 - v9:
- PLATFORM_BUS_NUM_IRQS set to 32 instead of 20
- platform bus irq now start at 64 instead of 48
- remove change of indentation in a15memmap
- correct misc style issues

v7 - v8:
- rebase on 2.2.0
- in machvirt_init, create_platform_bus simply is added
  after the arm_load_kernel call instead of moving this latter.
  Related comment slighly reworded.
- Due to those changes I dropped Alex and Shannon's Reviewed-by

v6 - v7:
Take into account Shannon comments:
- remove PLATFORM_BUS_FIRST_IRQ macro
- correct platform bus size to 0x40
- add an additional comment in a15irqmap related to
  PLATFORM_BUS_NUM_IRQS

v5 - v6:
- Take into account Peter's comments:
  - platform_bus_params initialized from vbi-memmap and vbi-irqmap.
As a consequence, const is removed. Also alignment in a15memmap
is slightly changed.
  - ARMPlatformBusSystemParams handle removed from create_platform_bus
prototype
- arm_load_kernel has become a machine init done notifier registration.
  It must be called before platform_bus_create to guarantee the correct
  notifier execution sequence

v4 - v5:
- platform_bus_params becomes static const
- reword comment in create_platform_bus
- reword the commit message

v3 - v4:
- use platform bus object, instantiated in create_platform_bus
- device tree generation for platform bus and children dynamic
  sysbus devices is no more handled at reset but in a
  machine_init_done_notifier (due to the change in implementaion
  of ARM load dtb using rom_add_blob_fixed).
- device tree enhancement now takes into account the case of
  user provided dtb. Before the user dtb was overwritten which
  was wrong. However in case the dtb is provided by the user,
  dynamic sysbus nodes are not added there.
- renaming of MACHVIRT_PLATFORM defines
- MACHVIRT_PLATFORM_PAGE_SHIFT and SIZE_PAGES not needed anymore,
  hence removed.
- DynSysbusParams struct renamed into ARMPlatformBusSystemParams
  and above params removed.
- separation of dt creation and QEMU binding is not mandated anymore
  since the device tree is not created from scratch anymore. Instead
  the modify_dtb function is used.
- create_platform_bus registers another machine init done notifier
  to start VFIO IRQ handling. This latter executes after the
  dynamic sysbus device binding.

v2 - v3:
- renaming of arm_platform_bus_create_devtree and arm_load_dtb
- add copyright in hw/arm/dyn_sysbus_devtree.c

v1 - v2:
- remove useless vfio-platform.h include file
- s/MACHVIRT_PLATFORM_HOLE/MACHVIRT_PLATFORM_SIZE
- use dyn_sysbus_binding and dyn_sysbus_devtree
- dynamic sysbus platform buse size shrinked to 4MB and
  moved between RTC and MMIO

v1:

Inspired from what Alex Graf did in ppc e500
https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html

Conflicts:
hw/arm/sysbus-fdt.c

Conflicts:
hw/arm/virt.c
---
 hw/arm/virt.c | 59 +++
 1 file changed, 59 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2353440..66cd553 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -42,6 +42,8 @@
 #include exec/address-spaces.h
 #include qemu/bitops.h
 #include qemu/error-report.h
+#include hw/arm/sysbus-fdt.h
+#include hw/platform-bus.h
 
 #define NUM_VIRTIO_TRANSPORTS 32
 
@@ -59,6 +61,8 @@
 #define GIC_FDT_IRQ_PPI_CPU_START 8
 #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
 
+#define PLATFORM_BUS_NUM_IRQS 32
+
 enum {
 VIRT_FLASH,
 VIRT_MEM,
@@ -69,8 +73,11 @@ enum {
 VIRT_MMIO,
 VIRT_RTC,
 VIRT_FW_CFG,
+VIRT_PLATFORM_BUS,
 };
 
+static ARMPlatformBusSystemParams platform_bus_params;
+
 typedef struct MemMapEntry {
 hwaddr base;
 hwaddr size;
@@ -127,6 +134,7 @@ static const MemMapEntry a15memmap[] = {
 [VIRT_UART] =   { 0x0900, 0x1000 },
 [VIRT_RTC] ={ 0x0901, 0x1000 },
 [VIRT_FW_CFG] = { 0x0902, 0x000a },
+[VIRT_PLATFORM_BUS] =   { 0x0940, 0x0040 },
 [VIRT_MMIO] =   { 0x0a00, 0x0200 },
 /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
 /* 0x1000 .. 0x4000 reserved for PCI */
@@ -137,6 +145,7 @@ static const int a15irqmap[] = {
 [VIRT_UART] = 1,
 [VIRT_RTC] = 2,
 [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
+[VIRT_PLATFORM_BUS] = 64, /* ... to 64 + PLATFORM_BUS_NUM_IRQS -1 */
 };
 
 static 

[Qemu-devel] [RFC v3 2/2] hw/arm/virt: add generic-pci PCI host controller

2015-01-14 Thread Alvise Rigo
The platform memory map has now three more memory ranges to map the
device's memory regions (Configuration region, I/O region and Memory
region).

The dt node interrupt-map property tells how to route the PCI interrupts
to system interrupts. In the mach-virt case, four IRQs are swizzled
between all the possible interrupts coming from the PCI bus. In
particular, the mapping ensures that the first four devices will use a
system IRQ always different (supposing that only PIN_A of each device is
used).

Now that a PCI bus is provided, the machine can be launched with
multiple PCI devices through the -device option (e.g., -device
virtio-blk-pci).

Signed-off-by: Alvise Rigo a.r...@virtualopensystems.com
---
 hw/arm/virt.c | 112 +-
 1 file changed, 111 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2353440..7b0326f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -44,6 +44,7 @@
 #include qemu/error-report.h
 
 #define NUM_VIRTIO_TRANSPORTS 32
+#define NUM_PCI_IRQS 4
 
 /* Number of external interrupt lines to configure the GIC with */
 #define NUM_IRQS 128
@@ -68,8 +69,12 @@ enum {
 VIRT_UART,
 VIRT_MMIO,
 VIRT_RTC,
+VIRT_PCI_CFG,
+VIRT_PCI_IO,
+VIRT_PCI_MEM,
 VIRT_FW_CFG,
 };
+#define VIRT_PCI VIRT_PCI_CFG
 
 typedef struct MemMapEntry {
 hwaddr base;
@@ -129,13 +134,17 @@ static const MemMapEntry a15memmap[] = {
 [VIRT_FW_CFG] = { 0x0902, 0x000a },
 [VIRT_MMIO] =   { 0x0a00, 0x0200 },
 /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
-/* 0x1000 .. 0x4000 reserved for PCI */
+/* PCI regions */
+[VIRT_PCI_CFG] ={ 0x1000, 0x0100 },
+[VIRT_PCI_IO] = { 0x1100, 0x0001 },
+[VIRT_PCI_MEM] ={ 0x1200, 0x2e00 },
 [VIRT_MEM] ={ 0x4000, 30ULL * 1024 * 1024 * 1024 },
 };
 
 static const int a15irqmap[] = {
 [VIRT_UART] = 1,
 [VIRT_RTC] = 2,
+[VIRT_PCI] = 3, /* ...to 3 + NUM_PCI_IRQS - 1 */
 [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
 };
 
@@ -436,6 +445,105 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq 
*pic)
 g_free(nodename);
 }
 
+static void create_pci_host(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+DeviceState *dev;
+SysBusDevice *busdev;
+int i, j, intmap_rows;
+const MemMapEntry *mme = vbi-memmap;
+uint32_t plat_acells;
+uint32_t plat_scells;
+uint32_t gic_phandle;
+char *nodename;
+
+dev = qdev_create(NULL, generic_pci);
+qdev_prop_set_uint64(dev, mmio_win_size, mme[VIRT_PCI_MEM].size);
+qdev_prop_set_uint64(dev, mmio_win_addr, mme[VIRT_PCI_MEM].base);
+qdev_prop_set_uint32(dev, irqs, NUM_PCI_IRQS);
+qdev_init_nofail(dev);
+
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, mme[VIRT_PCI_CFG].base); /* PCI config */
+sysbus_mmio_map(busdev, 1, mme[VIRT_PCI_IO].base);  /* PCI I/O */
+sysbus_mmio_map(busdev, 2, mme[VIRT_PCI_MEM].base); /* PCI memory window */
+
+for ( i = 0; i  NUM_PCI_IRQS; i++) {
+sysbus_connect_irq(busdev, i, pic[vbi-irqmap[VIRT_PCI] + i]);
+}
+
+/* add device tree node */
+nodename = g_strdup_printf(/pci@% PRIx64, mme[VIRT_PCI_CFG].base);
+qemu_fdt_add_subnode(vbi-fdt, nodename);
+qemu_fdt_setprop_string(vbi-fdt, nodename, compatible,
+ pci-host-cam-generic);
+qemu_fdt_setprop_string(vbi-fdt, nodename, device_type, pci);
+qemu_fdt_setprop_cell(vbi-fdt, nodename, #address-cells, 0x3);
+qemu_fdt_setprop_cell(vbi-fdt, nodename, #size-cells, 0x2);
+qemu_fdt_setprop_cell(vbi-fdt, nodename, #interrupt-cells, 0x1);
+
+plat_acells = qemu_fdt_getprop_cell(vbi-fdt, /, #address-cells);
+plat_scells = qemu_fdt_getprop_cell(vbi-fdt, /, #size-cells);
+qemu_fdt_setprop_sized_cells(vbi-fdt, nodename, reg,
+plat_acells, mme[VIRT_PCI_CFG].base,
+plat_scells, mme[VIRT_PCI_CFG].size);
+
+/* Two regions (second and third of *busdev):
+   - I/O region, PCI addr = 0x0, CPU addr = mme[VIRT_PCI_IO].base,
+ size = mme[VIRT_PCI_IO].size,
+   - 32bit MMIO region, PCI addr = CPU addr = mme[VIRT_PCI_MEM].base,
+ size = mme[VIRT_PCI_MEM].size */
+qemu_fdt_setprop_sized_cells(vbi-fdt, nodename, ranges,
+1, 0x0100, 2, 0x, /* I/O space */
+2, mme[VIRT_PCI_IO].base, 2, mme[VIRT_PCI_IO].size,
+1, 0x0200, 2, mme[VIRT_PCI_MEM].base, /* 32bit memory space */
+2, mme[VIRT_PCI_MEM].base, 2, mme[VIRT_PCI_MEM].size);
+
+gic_phandle = qemu_fdt_get_phandle(vbi-fdt, /intc);
+
+#define IRQ_MAP_ENTRY_DESC_SZ 14
+#define PHYSH_DEV_SHIFT 11
+/* Any interrupt from the PCI bus is represented by four 32bit values 
physH
+   physM physL pin# (unit-interrupt-specifier), where bits physH[11:15]

[Qemu-devel] [RFC v3 1/2] pci/pci-host: Add generic-pci PCI host controller device

2015-01-14 Thread Alvise Rigo
Add a generic PCI host controller for virtual platforms, based on the
previous work by Rob Herring:
http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html

The controller relies on a configuration memory region and provides two
PCI memory regions for I/O (one port and one memory mapped).  The device
needs the following qdev properties to configure the memory regions:
- cfg_win_size: size of the configuration memory
- pio_win_size: size of the port I/O space
- mmio_win_size: size of the MMIO space
- mmio_win_addr: offset of MMIO space in the system memory

Signed-off-by: Alvise Rigo a.r...@virtualopensystems.com
---
 hw/pci-host/Makefile.objs |   2 +-
 hw/pci-host/generic-pci.c | 140 ++
 include/hw/pci-host/generic-pci.h |  45 
 3 files changed, 186 insertions(+), 1 deletion(-)
 create mode 100644 hw/pci-host/generic-pci.c
 create mode 100644 include/hw/pci-host/generic-pci.h

diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
index bb65f9c..8ef9fac 100644
--- a/hw/pci-host/Makefile.objs
+++ b/hw/pci-host/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += pam.o
+common-obj-y += pam.o generic-pci.o
 
 # PPC devices
 common-obj-$(CONFIG_PREP_PCI) += prep.o
diff --git a/hw/pci-host/generic-pci.c b/hw/pci-host/generic-pci.c
new file mode 100644
index 000..54c9647
--- /dev/null
+++ b/hw/pci-host/generic-pci.c
@@ -0,0 +1,140 @@
+/*
+ * Generic PCI host controller
+ *
+ * Copyright (c) 2014 Linaro, Ltd.
+ * Author: Rob Herring rob.herr...@linaro.org
+ *
+ * Based on ARM Versatile PCI controller (hw/pci-host/versatile.c):
+ * Copyright (c) 2006-2009 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the LGPL.
+ */
+
+#include hw/sysbus.h
+#include hw/pci-host/generic-pci.h
+#include exec/address-spaces.h
+#include sysemu/device_tree.h
+
+static const VMStateDescription pci_generic_host_vmstate = {
+.name = generic-host-pci,
+.version_id = 1,
+.minimum_version_id = 1,
+};
+
+static void pci_cam_config_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+PCIGenState *s = opaque;
+pci_data_write(s-pci_bus, addr, val, size);
+}
+
+static uint64_t pci_cam_config_read(void *opaque, hwaddr addr, unsigned size)
+{
+PCIGenState *s = opaque;
+uint32_t val;
+val = pci_data_read(s-pci_bus, addr, size);
+return val;
+}
+
+static const MemoryRegionOps pci_generic_config_ops = {
+.read = pci_cam_config_read,
+.write = pci_cam_config_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void pci_generic_set_irq(void *opaque, int irq_num, int level)
+{
+qemu_irq *pic = opaque;
+qemu_set_irq(pic[irq_num], level);
+}
+
+static void pci_generic_host_realize(DeviceState *dev, Error **errp)
+{
+PCIHostState *h = PCI_HOST_BRIDGE(dev);
+PCIGenState *s = PCI_GEN(dev);
+GenericPCIHostState *gps = s-pci_gen;
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+int i;
+
+memory_region_init(s-pci_io_window, OBJECT(s), pci_io, 
s-pio_win_size);
+memory_region_init(s-pci_mem_space, OBJECT(s), pci_mem, 1ULL  32);
+
+pci_bus_new_inplace(s-pci_bus, sizeof(s-pci_bus), dev, pci,
+s-pci_mem_space, s-pci_io_window,
+PCI_DEVFN(0, 0), TYPE_PCI_BUS);
+h-bus = s-pci_bus;
+
+object_initialize(gps, sizeof(*gps), TYPE_GENERIC_PCI_HOST);
+qdev_set_parent_bus(DEVICE(gps), BUS(s-pci_bus));
+
+for (i = 0; i  s-irqs; i++) {
+sysbus_init_irq(sbd, s-irq[i]);
+}
+
+pci_bus_irqs(s-pci_bus, pci_generic_set_irq, pci_swizzle_map_irq_fn,
+ s-irq, s-irqs);
+memory_region_init_io(s-mem_config, OBJECT(s), pci_generic_config_ops, 
s,
+  pci-config, s-cfg_win_size);
+memory_region_init_alias(s-pci_mem_window, OBJECT(s), pci-mem-win,
+s-pci_mem_space, s-mmio_win_addr, s-mmio_win_size);
+
+sysbus_init_mmio(sbd, s-mem_config);
+sysbus_init_mmio(sbd, s-pci_io_window);
+sysbus_init_mmio(sbd, s-pci_mem_window);
+}
+
+static void pci_generic_host_class_init(ObjectClass *klass, void *data)
+{
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+k-vendor_id = PCI_VENDOR_ID_REDHAT;
+k-device_id = PCI_DEVICE_ID_REDHAT_BRIDGE;
+k-class_id = PCI_CLASS_PROCESSOR_CO;
+/*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+dc-cannot_instantiate_with_device_add_yet = true;
+}
+
+static const TypeInfo pci_generic_host_info = {
+.name  = TYPE_GENERIC_PCI_HOST,
+.parent= TYPE_PCI_DEVICE,
+.instance_size = sizeof(GenericPCIHostState),
+.class_init= pci_generic_host_class_init,
+};
+
+static Property pci_generic_props[] = {
+DEFINE_PROP_UINT32(cfg_win_size, PCIGenState, 

Re: [Qemu-devel] [PATCH] AIO: Reduce number of threads for 32bit hosts

2015-01-14 Thread Paolo Bonzini


On 14/01/2015 11:20, Kevin Wolf wrote:
  The same problem applies to coroutine stacks, and those cannot be
  throttled down as easily.  But I guess if you limit the number of
  threads, the guest gets slowed down and doesn't create as many coroutines.
 Shouldn't we rather try and decrease the stack sizes a bit? 1 MB per
 coroutine is really a lot, and as I understand it, threads take even
 more by default.

Yup, 2 MB.  Last time I proposed this, I think Markus was strongly in 
the better safe than sorry camp. :)

But thread pool workers definitely don't need a big stack.

  It would be nice to have a way to measure coroutine stack usage, similar
  to what the kernel does.
 The information which pages are mapped should be there somewhere...

Yes, there is mincore(2).  The complicated part is doing it fast, but
perhaps it doesn't need to be fast.

I tried gathering warning from GCC's -Wstack-usage=1023 option and the
block layer does not seem to have functions with huge stacks in the I/O
path.

So, assuming a maximum stack depth of 50 (already pretty generous since
there shouldn't be any recursive calls) a 100K stack should be pretty
much okay for coroutines and thread-pool threads.

That said there are some offenders in the device models.  Other
QemuThreads, especially VCPU threads, had better stay with a big stack.

Paolo

1024-2048:
block/linux-aio.c  ioq_submit stack 
usage is 1104 bytes
block/mirror.c mirror_run stack 
usage is 1280 bytes
block/qapi.c   bdrv_query_image_info  stack 
usage is 1152 bytes
block/vmdk.c   vmdk_open_vmdk4stack 
usage is 1840 bytes
block/vpc.cvpc_create stack 
usage is 1152 bytes
cpus.c qmp_memsavestack 
usage is 1120 bytes
cpus.c qmp_pmemsave   stack 
usage is 1120 bytes
disas/sparc.c  build_hash_table   stack 
usage is 1072 bytes
fsdev/virtfs-proxy-helper.cmain   stack 
usage is 1264 bytes
hw/arm/highbank.c  calxeda_init   stack 
usage is 1200 bytes
hw/arm/stellaris.c stellaris_init stack 
usage is 1120 bytes
hw/arm/virt.c  machvirt_init  stack 
usage is 1264 bytes
hw/block/dataplane/virtio-blk.chandle_notify  stack 
usage is 1632 bytes
hw/block/virtio-blk.c  virtio_blk_dma_restart_bh  stack 
usage is 1600 bytes
hw/block/virtio-blk.c  virtio_blk_handle_output   stack 
usage is 1584 bytes
hw/core/qdev.c qdev_get_legacy_property   stack 
usage is 1104 bytes
hw/net/vmxnet_tx_pkt.c vmxnet_tx_pkt_do_sw_fragmentation  stack 
usage is 1168 bytes
hw/ppc/e500.c  ppce500_load_device_tree   stack 
usage is 1072 bytes
hw/scsi/megasas.c  megasas_dcmd_ld_get_list   stack 
usage is 1152 bytes
hw/tpm/tpm_passthrough.c   tpm_passthrough_test_tpmdevstack 
usage is 1216 bytes
linux-user/main.c  main   stack 
usage is 1664 bytes
linux-user/main.c  main   stack 
usage is 1696 bytes
linux-user/main.c  main   stack 
usage is 1728 bytes
linux-user/main.c  main   stack 
usage is 1744 bytes
linux-user/main.c  main   stack 
usage is 1760 bytes
linux-user/main.c  main   stack 
usage is 1776 bytes
linux-user/main.c  main   stack 
usage is 1856 bytes
linux-user/main.c  main   stack 
usage is 1872 bytes
linux-user/main.c  main   stack 
usage is 1888 bytes
linux-user/main.c  main   stack 
usage is 1952 bytes
linux-user/main.c  main   stack 
usage is 1984 bytes
linux-user/main.c  main   stack 
usage is 2032 bytes
migration/vmstate.cget_unused_buffer  stack 
usage is 1088 bytes
monitor.c  monitor_parse_command  stack 
usage is 1424 bytes
monitor.c  parse_cmdline  stack 
usage is 1136 bytes
qemu-img.c img_rebase stack 
usage is 1248 bytes

  1   2   >