Re: [Qemu-devel] [PULL 00/15] Misc patches for 2015-01-14
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
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
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
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)
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
* 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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