[Qemu-devel] Did anybody study on the XEN-3.4.2 based on qemu for hardware virtualization machine?
Hi: Looking for the person who has the common goal. Looking forward to your reply!
Re: [Qemu-devel] multifunction pci virtio-blk devices
On 12/15/2011 04:18 PM, Stefan Hajnoczi wrote: On Thu, Dec 15, 2011 at 02:00:11PM +0800, Hui Kai Ran wrote: but for virtio blk device , how can i open multifunction ability? Please search the mailing list archives, Anthony has posted instructions in previous threads. Stefan I found it. thanks!
Re: [Qemu-devel] [RFC] Device isolation infrastructure v2
On Thu, Dec 15, 2011 at 09:49:05PM -0700, Alex Williamson wrote: > On Fri, 2011-12-16 at 12:40 +1100, David Gibson wrote: > > On Thu, Dec 15, 2011 at 11:05:07AM -0700, Alex Williamson wrote: > > > On Thu, 2011-12-15 at 17:25 +1100, David Gibson wrote: > > > > Here's the second spin of my preferred approach to handling grouping > > > > of devices for safe assignment to guests. > > > > > > > > Changes since v1: > > > > * Many name changes and file moves for improved consistency > > > > * Bugfixes and cleanups > > > > * The interface to the next layer up is considerably fleshed out, > > > >although it still needs work. > > > > * Example initialization of groups for p5ioc2 and p7ioc. > > > > > > > > TODO: > > > > * Need sample initialization of groups for intel and/or amd iommus > > > > > > I think this very well might imposed significant bloat for those > > > implementations. On POWER you typically don't have singleton groups, > > > while it's the norm on x86. I don't know that either intel or amd iommu > > > code have existing structures that they can simply tack the group > > > pointer to. > > > > Actually, I think they can probably just use the group pointer in the > > struct device. Each PCI function will typically allocate a new group > > and put the pointer in the struct device and no-where else. Devices > > hidden under bridges copy the pointer from the bridge parent instead. > > I will have to check the unplug path to ensure we can manage the group > > lifetime properly, of course. > > > > > Again, this is one of the reasons that I think the current > > > vfio implementation is the right starting point. We keep groups within > > > vfio, imposing zero overhead for systems not making use of it and only > > > require iommu drivers to implement a trivial function to opt-in. As we > > > start to make groups more pervasive in the dma layer, independent of > > > userspace driver exposure, we can offload pieces to the core. Starting > > > with it in the core and hand waving some future use that we don't plan > > > to implement right now seems like the wrong direction. > > > > Well, I think we must agree to disagree here; I think treating groups > > as identifiable objects is worthwhile. That said, I am looking for > > ways to whittle down the overhead when they're not in use. > > > > > > * Use of sysfs attributes to control group permission is probably a > > > >mistake. Although it seems a bit odd, registering a chardev for > > > >each group is probably better, because perms can be set from udev > > > >rules, just like everything else. > > > > > > I agree, this is a horrible mistake. Reinventing file permissions via > > > sysfs is bound to be broken and doesn't account for selinux permissions. > > > Again, I know you don't like aspects of the vfio group management, but > > > it gets this right imho. > > > > Yeah. I came up with this because I was trying to avoid registering a > > device whose only purpose was to act as a permissioned "handle" on the > > group. But it is a better approach, despite that. I just wanted to > > send out the new patches for comment without waiting to do that > > rework. > > So we end up with a chardev created by the core, whose only purpose is > setting the group access permissions for userspace usage, which only > becomes useful with something like vfio? It's an odd conflict that > isolation groups would get involved with userspace permissions to access > the group, but leave enforcement of isolation via iommu groups to the > "binder" driver Hm, perhaps. I'll think about it. > (where it seems like vfio is still going to need some > kind of merge interface to share a domain between isolation groups). That was always going to be the case, but I wish we could stop thinking of it as the "merge" interface, since I think that term is distorting thinking about how the interface works. For example, I think opening a new domain, then adding / removing groups provides a much cleaner model than "merging' groups without a separate handle on the iommu domain we're building. > Is this same chardev going to be a generic conduit for > read/write/mmap/ioctl to the "binder" driver or does vfio need to create > it's own chardev for that? Right, I was thinking that the binder could supply its own fops or something for the group chardev once the group is bound. > In the former case, are we ok with a chardev > that has an entirely modular API behind it, or maybe you're planning to > define some basic API infrastructure, in which case this starts smelling > like implementing vfio in the core. I think it can work, but I do need to look closer. > In the latter case (isolation > chardev + vfio chardev) coordinating permissions sounds like a mess. Absolutely. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _wa
Re: [Qemu-devel] [RFC] Plan for moving forward with QOM
> I also don't want the user to have to always make the decision about how to > hook up IRQs for every single device because in a lot of circumstances, > there's no point. How else are we going to figure out how the IRQ lines are wired up? > A basic premise for me is that simple things should be simple. It > shouldn't take a 800 line config file to create a PC. IMO the PC is a bad example. There's basically only one way things are ever wired up. For other targets there is no standard reference design, and the hardware configuration is entirely a the whim of the vendor. > >> But this entire use-case seems to be synthetic. Do you have a real case > >> where you would want to inherit twice from the same interface? > > > > A GPIO controller (of which interrupt controllers are a subset). We want > > to expose and use many single-bit control line interfaces. > > I don't see it but perhaps it's because I don't have sufficient > imagination. Can you point me to a concrete example? Pick pretty much an of them. pl190 and pl061 are fairly standard examples. The former has 32 input pins to which other devices can link, and two output pins that can be linked to other output pins. The latter has 8 inputs and 9 outputs. The distinction between IRQ lines and GPIO output pins is a qdev wart that we should not repeat. Both should be identified by name. The contraints here are that each output pin will be linked to at most one input pin, and vice-versa. Any output pin can by linked to any input pin. > > I suppose pckbd.c is annother example. This implements a pair of PS/2 > > serial busses. > > I've dropped the notion of a "bus" in QOM. They don't exist at all. > > The way this gets modeled in QOM is just to have a link named > kbd and a link named aux. This much I understand. > pckbd implements PS2Bus and PS2Bus looks something like: > > typedef struct PS2Bus { > Interface parent; > void (*set_irq)(PS2Bus *bus, PS2Device *dev, int level); > } PS2Bus; This is where I get a bit sketchy. Is set_irq a pure virtual function that pckbk is expected to override? And pckbd has to figure out which link this is based on the otherwise unnecessary dev argument? > PS2Device looks like: > > typedef struct PS2Device { > Device parent; > > PS2Bus *bus; > // ... > } PS2Device; > > The device just does: > > void myfunc(PS2Keyboard *s) > { > ps2_bus_set_irq(s->bus, PS2_DEVICE(s), 1); > } How does s->bus get set? I guess you're expecting pckbd to do something like: static void i8049_set_irq(PS2Bus *bus, PS2Device *dev, int level) { Device *me = dynamic_cast bus; if (dev == get_dev_property(me, "kbd")) { kbd_update_kbd_irq(me, level); } else if (dev == get_dev_property(me, "aux")) { kbd_update_aux_irq(me, level); } else { abort(); } } I guess that'll work, as long as both keyboard and mouse aren't implemented by the same device (which they might be for IRQ lines). Paul
Re: [Qemu-devel] [PATCH 2/4] docs: add build infrastructure for gtkdocs
Am 15.12.2011 14:30, schrieb Anthony Liguori: > On 12/15/2011 03:37 AM, Avi Kivity wrote: >> On 12/14/2011 06:20 PM, Anthony Liguori wrote: >>> By convention, documented headers now go in include/ >> >> Dislike. +1 > I've been planning on doing this for a while. I think it's a useful way > to help improve internal modularity. It provides a consistent way to > indicate which headers represent "public" internal interfaces (like the > memory API) verses things that are really private headers specific to a > submodule (say block_int.h). So far you've always opposed the idea of a stable public API. If that's still not what you want, then there's no real distinction here and an include/ directory would give a wrong impression. If you just want a TODO list, then surely having a shell script print the *.h file names and copying them into the Wiki would be easier. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC] Device isolation infrastructure v2
On Fri, 2011-12-16 at 12:40 +1100, David Gibson wrote: > On Thu, Dec 15, 2011 at 11:05:07AM -0700, Alex Williamson wrote: > > On Thu, 2011-12-15 at 17:25 +1100, David Gibson wrote: > > > Here's the second spin of my preferred approach to handling grouping > > > of devices for safe assignment to guests. > > > > > > Changes since v1: > > > * Many name changes and file moves for improved consistency > > > * Bugfixes and cleanups > > > * The interface to the next layer up is considerably fleshed out, > > >although it still needs work. > > > * Example initialization of groups for p5ioc2 and p7ioc. > > > > > > TODO: > > > * Need sample initialization of groups for intel and/or amd iommus > > > > I think this very well might imposed significant bloat for those > > implementations. On POWER you typically don't have singleton groups, > > while it's the norm on x86. I don't know that either intel or amd iommu > > code have existing structures that they can simply tack the group > > pointer to. > > Actually, I think they can probably just use the group pointer in the > struct device. Each PCI function will typically allocate a new group > and put the pointer in the struct device and no-where else. Devices > hidden under bridges copy the pointer from the bridge parent instead. > I will have to check the unplug path to ensure we can manage the group > lifetime properly, of course. > > > Again, this is one of the reasons that I think the current > > vfio implementation is the right starting point. We keep groups within > > vfio, imposing zero overhead for systems not making use of it and only > > require iommu drivers to implement a trivial function to opt-in. As we > > start to make groups more pervasive in the dma layer, independent of > > userspace driver exposure, we can offload pieces to the core. Starting > > with it in the core and hand waving some future use that we don't plan > > to implement right now seems like the wrong direction. > > Well, I think we must agree to disagree here; I think treating groups > as identifiable objects is worthwhile. That said, I am looking for > ways to whittle down the overhead when they're not in use. > > > > * Use of sysfs attributes to control group permission is probably a > > >mistake. Although it seems a bit odd, registering a chardev for > > >each group is probably better, because perms can be set from udev > > >rules, just like everything else. > > > > I agree, this is a horrible mistake. Reinventing file permissions via > > sysfs is bound to be broken and doesn't account for selinux permissions. > > Again, I know you don't like aspects of the vfio group management, but > > it gets this right imho. > > Yeah. I came up with this because I was trying to avoid registering a > device whose only purpose was to act as a permissioned "handle" on the > group. But it is a better approach, despite that. I just wanted to > send out the new patches for comment without waiting to do that > rework. So we end up with a chardev created by the core, whose only purpose is setting the group access permissions for userspace usage, which only becomes useful with something like vfio? It's an odd conflict that isolation groups would get involved with userspace permissions to access the group, but leave enforcement of isolation via iommu groups to the "binder" driver (where it seems like vfio is still going to need some kind of merge interface to share a domain between isolation groups). Is this same chardev going to be a generic conduit for read/write/mmap/ioctl to the "binder" driver or does vfio need to create it's own chardev for that? In the former case, are we ok with a chardev that has an entirely modular API behind it, or maybe you're planning to define some basic API infrastructure, in which case this starts smelling like implementing vfio in the core. In the latter case (isolation chardev + vfio chardev) coordinating permissions sounds like a mess. Alex
Re: [Qemu-devel] [BUG] [Seabios] PCI 64bit BARs on Win2008 - unable to start the device. (ACPI lacks the _DSM method)
I wonder if there any particular reason to separate prefetchable a non-prefetchable memory regions in pciinit? Extra two more regions would make code more complex. Oh yes, there is. Which reminds me that the whole thing isn't that easy unfortunaly ... The reason are pci bridges. They have two memory regions, one for prefetchable and one for non-prefetchable memory. All devices behind a pci bridge must have the bars within the bridges memory regions, thats why they are grouped together. This also implies that a 32bit and a 64bit bar (of the same type) behind a pci bridge must be mapped next to each other, so moving up 64bit bars unconditionally isn't going to fly. Devices behind bridges need some extra care, only when all bars are 64bit capable they can actually be mapped above 4G. The qemu actually does not simulate PCI bridges at all. So good question shall we bother about this? I did some preliminary tests: 64bit BARs are working quite well for linux 2.6.18 - 3.0 and windows 2008. Also I've found important detail that according to PCI architecture specification, the bridges can describe 64bit ranges for prefetchable type of memory only. So it's very unlikely that devices exporting 64bit non-prefetchable BARs. I guess we just need to add one extra type. Even if there are two separate prefetchable memory regions (32 bit and 64bit), it won't be a problem as there is no bridge on the 440FX inside the virtual machine. Cheers, Alexey
Re: [Qemu-devel] [PATCH v5 4/4] Add support for net bridge
On 12/15/2011 10:26 AM, Anthony Liguori wrote: On 11/13/2011 09:45 PM, Corey Bryant wrote: The most common use of -net tap is to connect a tap device to a bridge. This requires the use of a script and running qemu as root in order to allocate a tap device to pass to the script. This patch breaks the build: anthony@titi:~/build/qemu$ make CC net/tap.o cc1: warnings being treated as errors /home/anthony/git/qemu/net/tap.c: In function ‘net_init_tap’: /home/anthony/git/qemu/net/tap.c:560:15: error: ‘s’ may be used uninitialized in this function make: *** [net/tap.o] Error 1 I was wondering how I missed this. I typically configure with --enable-debug, which appears to suppress warnings. - s = net_tap_fd_init(vlan, "tap", name, fd, vnet_hdr); - if (!s) { - close(fd); - return -1; + s = net_tap_fd_init(vlan, "tap", name, fd, vnet_hdr); + if (!s) { + close(fd); + return -1; + } } And indeed, you've changed the function from unconditionally initializing s to conditionally initializing it. Specifically, you've broken -net tap,fd=X which would break tools like libvirt. Regards, Anthony Liguori -- Regards, Corey
Re: [Qemu-devel] The reason behind block linking constraint? (Cont.)
> Yes. Did you build one bzImage based on Linus kernel git tree, and > then use unmodified QEMU to boot it? Can it succeed to start up? Current buildroot (snapshot version) build linux 3.x by default, and unmodified QEMU can boot it. Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] [PATCH] w32: QEMU applications with SDL are always GUI applications
On Sun, Dec 4, 2011 at 05:32, Stefan Weil wrote: > Since commit 1d14ffa97eacd3cb722271eaf6f093038396eac4 (in 2005), > QEMU applications on W32 don't use the default SDL compiler flags: > > Instead of a GUI application, a console application is created. > > This has disadvantages (there is always an empty console window) and > no obvious reason, so this patch removes the strange flag modification. > > The SDL GUI applications still can be run from a console window > and even send stdout and stderr to that console by setting environment > variable SDL_STDIO_REDIRECT=no. Did you test it? Windows GUI applications can not send stdout to the startup console window unless they create their own console window. > Signed-off-by: Stefan Weil > --- > configure | 3 --- > 1 files changed, 0 insertions(+), 3 deletions(-) > > diff --git a/configure b/configure > index ac4840d..f2fdb1d 100755 > --- a/configure > +++ b/configure > @@ -1522,9 +1522,6 @@ EOF > if compile_prog "$sdl_cflags" "$sdl_libs" ; then > sdl_libs="$sdl_libs -lX11" > fi > - if test "$mingw32" = "yes" ; then > - sdl_libs="`echo $sdl_libs | sed s/-mwindows//g` -mconsole" > - fi > libs_softmmu="$sdl_libs $libs_softmmu" > fi > > -- > 1.7.2.5 > >
[Qemu-devel] [PATCH] virtio-serial: Allow one MSI-X vector per virtqueue
From: Hongyong Zang In pci_enable_msix(), the guest's virtio-serial driver tries to set msi-x with one vector per queue. But it fails and eventually all virtio-serial ports share one MSI-X vector. Because every virtio-serial port has *two* virtqueues, virtio-serial needs (port+1)*2 vectors other than (port+1). This patch allows every virtqueue to have its own MSI-X vector. (When the MSI-X vectors needed are more than MSIX_MAX_ENTRIES defined in qemu: msix.c, all the queues still share one MSI-X vector as before.) Signed-off-by: Hongyong Zang --- hw/virtio-pci.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 77b75bc..2c9c6fb 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -718,8 +718,11 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev) return -1; } vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED -? proxy->serial.max_virtserial_ports + 1 +? (proxy->serial.max_virtserial_ports + 1) * 2 : proxy->nvectors; +/*msix.c: #define MSIX_MAX_ENTRIES 32*/ +if (vdev->nvectors > 32) +vdev->nvectors = 32; virtio_init_pci(proxy, vdev); proxy->nvectors = vdev->nvectors; return 0; -- 1.7.1
Re: [Qemu-devel] [RFC] Plan for moving forward with QOM
On 12/15/2011 03:28 PM, Paul Brook wrote: There is a second class of "user" that is doing very sophisticated things and cares about this information. But this sophisticated user (i.e. libvirt) wants to be able to probe QEMU to understand what features are probably available because it very likely is evolving just as quickly as QEMU is. I disagree. Maybe this is because you're coming from the PC/Linux world where all hardware is basically the same. In the embedded world there are many more vastly different machines. And some of those have several similar but different enough to matter variants. There's a good chance the guest OS needs exactly the right one. Take the Stellaris boards as an example. We currently implement two boards. There are well over a hundred variants in this SoC family. A good proportion of these chips will be used in projects that include a custom PCB design. There are at least three different reference boards for the LM3S6965 alone. This is not unusual. I really don't want to have to teach qemu about hunderds of different SoCs connected to thousands of different "motherboards", most of which we'll never even know about. I'd like for users to be able to create their own hardware config without having to track qemu implementation details. Yes, we're on the same page here. One of my primary objectives in this refactoring is to eliminate the notion of a "machine" as anything that is at all special. I want the ability to have a "pc" device that has (via composition) all of the normal PC components via composition. But don't confuse flexibility with compatibility. If you're hooking up IRQ lines to make a custom SOC, then I don't think you're the type of user that can't handle the fact that we may change whether the PIIX inherits from or implements it's bus in a new version. I also don't want the user to have to always make the decision about how to hook up IRQs for every single device because in a lot of circumstances, there's no point. A basic premise for me is that simple things should be simple. It shouldn't take a 800 line config file to create a PC. You're advocating only connecting properties to properties, and never an object to a property? I think that's needlessly complicated. In the vast majority of cases, you just case about saying "connect this CharDriverState to this Serial device". We should make it much more complicated than that. Yes, that's what I'm saying. I'm finding it hard to believe that more than one link being stateful is such an exceptional case. Am I right in thinking that you're effectively implementing multiple inheritance via properties? If so I guess works around the single-inheritance limitation in many cases. No. Properties are strictly for composition. A lot of times people advocate using composition instead of inheritance. In fact, it's covered extensively in "Effective C++" if you're interested. But this entire use-case seems to be synthetic. Do you have a real case where you would want to inherit twice from the same interface? A GPIO controller (of which interrupt controllers are a subset). We want to expose and use many single-bit control line interfaces. I don't see it but perhaps it's because I don't have sufficient imagination. Can you point me to a concrete example? I very much want to avoid over engineering things so I would prefer to only worry problems we know we need to solve. Honestly, the details we're discussing right now are minor and it wouldn't be that hard to change the way inheritance works if we wanted to support true MI. I suppose pckbd.c is annother example. This implements a pair of PS/2 serial busses. I've dropped the notion of a "bus" in QOM. They don't exist at all. The way this gets modeled in QOM is just to have a link named kbd and a link named aux. pckbd implements PS2Bus and PS2Bus looks something like: typedef struct PS2Bus { Interface parent; void (*set_irq)(PS2Bus *bus, PS2Device *dev, int level); } PS2Bus; PS2Device looks like: typedef struct PS2Device { Device parent; PS2Bus *bus; // ... } PS2Device; The device just does: void myfunc(PS2Keyboard *s) { ps2_bus_set_irq(s->bus, PS2_DEVICE(s), 1); } You could call bus "controller", "other-device", "fubar", or whatever. Currently we don't model these and use hardcoded callbacks when creating the keyboard/mouse devices. Yes, it sucks, and once my qom-next tree is merged I'll fix this so it works as above and you can instantiate a PS2Mouse and set the i8054.aux property to the mouse device that you create. Once modelled properly I'd expect a board to be able to replace either with a third [as yet unimplemented] PS/2 device. The i8042 is not as generic as you think, but yes, if you made a PS/2 AUX compatible device (perhaps a tablet or something), you could just drop it in. Any of the those devices should also be usable with the single-port ARM PL050 controlle
[Qemu-devel] New RC List
Dear Sir, Nice day. We might have communicated before. It is Jason from PINIPOWER. We are professional manufactory for LIPO Batteries of RC market. We are capable of producing the LIPO batteries for brands like Align, Esky, Walkera, Futaba, JR, and all kinds. Guarantee:"A" grade of battery cells. We have our own R&D team, technical team and QC system, please visit www.pinipower.com We would like to develop mutual business and send you the best offer. Thanks and best regards, Jason Export manager Web: www.pinipower.com Tel: 0086-20-34482709 Add.: 9 Dongcuibei Street, Hailian Road Guangzhou, Guangdong 510230 China.
Re: [Qemu-devel] [RFC] Device isolation infrastructure v2
On Thu, Dec 15, 2011 at 11:05:07AM -0700, Alex Williamson wrote: > On Thu, 2011-12-15 at 17:25 +1100, David Gibson wrote: > > Here's the second spin of my preferred approach to handling grouping > > of devices for safe assignment to guests. > > > > Changes since v1: > > * Many name changes and file moves for improved consistency > > * Bugfixes and cleanups > > * The interface to the next layer up is considerably fleshed out, > >although it still needs work. > > * Example initialization of groups for p5ioc2 and p7ioc. > > > > TODO: > > * Need sample initialization of groups for intel and/or amd iommus > > I think this very well might imposed significant bloat for those > implementations. On POWER you typically don't have singleton groups, > while it's the norm on x86. I don't know that either intel or amd iommu > code have existing structures that they can simply tack the group > pointer to. Actually, I think they can probably just use the group pointer in the struct device. Each PCI function will typically allocate a new group and put the pointer in the struct device and no-where else. Devices hidden under bridges copy the pointer from the bridge parent instead. I will have to check the unplug path to ensure we can manage the group lifetime properly, of course. > Again, this is one of the reasons that I think the current > vfio implementation is the right starting point. We keep groups within > vfio, imposing zero overhead for systems not making use of it and only > require iommu drivers to implement a trivial function to opt-in. As we > start to make groups more pervasive in the dma layer, independent of > userspace driver exposure, we can offload pieces to the core. Starting > with it in the core and hand waving some future use that we don't plan > to implement right now seems like the wrong direction. Well, I think we must agree to disagree here; I think treating groups as identifiable objects is worthwhile. That said, I am looking for ways to whittle down the overhead when they're not in use. > > * Use of sysfs attributes to control group permission is probably a > >mistake. Although it seems a bit odd, registering a chardev for > >each group is probably better, because perms can be set from udev > >rules, just like everything else. > > I agree, this is a horrible mistake. Reinventing file permissions via > sysfs is bound to be broken and doesn't account for selinux permissions. > Again, I know you don't like aspects of the vfio group management, but > it gets this right imho. Yeah. I came up with this because I was trying to avoid registering a device whose only purpose was to act as a permissioned "handle" on the group. But it is a better approach, despite that. I just wanted to send out the new patches for comment without waiting to do that rework. > > * Need more details of what the binder structure will need to > >contain. > > * Handle complete removal of groups. > > * Clarify what will need to happen on the hot unplug path. > > We're still also removing devices from the driver model, this means > drivers like vfio need to re-implement a lot of the driver core for > driving each individual device in the group, Ah, so, that's a bit I've yet to flesh out. The subtle distinction is that we prevent driver _matching_ not driver _binding. It's intentionally still possible to explicitly bind drivers to devices in the group, by bypassing the automatic match mechanism. I'm intending that when a group is bound, the binder struct will (optionally) specify a driver (or several, for different bus types) which will be "force bound" to all the devices in the group. > and as you indicate, it's > unclear what happens in the hotplug path It's clear enough in concept. I just have to work out exactly where we need to call d_i_dev_remove(), and whether we'll need some sort of callback to the bridge / iommu code. > and I wonder if things like > suspend/resume will also require non-standard support. I really prefer > attaching individual bus drivers to devices using the standard > bind/unbind mechanisms. I have a hard time seeing how this is an > improvement from vfio. Thanks, > > Alex > -- 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
[Qemu-devel] buildbot failure in qemu on block_mingw32
The Buildbot has detected a new failure on builder block_mingw32 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/block_mingw32/builds/66 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: kraxel_rhel61 Build Reason: The Nightly scheduler named 'nightly_block' triggered this build Build Source Stamp: [branch block] HEAD Blamelist: BUILD FAILED: failed compile sincerely, -The Buildbot
Re: [Qemu-devel] [PATCH 2/2] device_isolation: Support isolation on POWER p7ioc (IODA) bridges
On Thu, Dec 15, 2011 at 05:48:38PM +1100, Michael Ellerman wrote: > On Thu, 2011-12-15 at 17:08 +1100, David Gibson wrote: > > This patch adds code to the code for the powernv platform to create > > and populate isolation groups on hardware using the p7ioc (aka IODA) PCI > > host > > bridge used on some IBM POWER systems. > > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c > > b/arch/powerpc/platforms/powernv/pci-ioda.c > > index 0cdc8302..6df632e 100644 > > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > > @@ -861,6 +862,9 @@ static void __devinit pnv_ioda_setup_bus_dma(struct > > pnv_ioda_pe *pe, > > set_iommu_table_base(&dev->dev, &pe->tce32_table); > > if (dev->subordinate) > > pnv_ioda_setup_bus_dma(pe, dev->subordinate); > > +#ifdef CONFIG_DEVICE_ISOLATION > > + device_isolation_dev_add(&pe->di_group, &dev->dev); > > +#endif > > You already have a nop version of that in device_isolation.h, so the > ifdef is not required AFAICS. Alas, this is not true, since the pe->di_group member will not exist if !CONFIG_DEVICE_ISOLATION. And the stub is an inline, not a macro. The stubs probably can be fine tuned to avoid ifdefs in more places. -- 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 signature.asc Description: Digital signature
[Qemu-devel] [PATCH] kvm: Print something before calling abort() if KVM_RUN fails
It's a little unfriendly to call abort() without printing any sort of error message. So turn the DPRINTK() into an fprintf(stderr, ...). Signed-off-by: Michael Ellerman --- kvm-all.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 4c466d6..ac048bc 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -977,7 +977,8 @@ int kvm_cpu_exec(CPUState *env) ret = EXCP_INTERRUPT; break; } -DPRINTF("kvm run failed %s\n", strerror(-run_ret)); +fprintf(stderr, "error: kvm run failed %s\n", +strerror(-run_ret)); abort(); } -- 1.7.7.3
[Qemu-devel] [Bug 823902] Re: multithreaded ARM seg/longjmp causes uninitialized stack frame due to0d10193870b5a81c3bce13a602a5403c3a55cf6c
** Changed in: qemu Status: Fix Committed => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/823902 Title: multithreaded ARM seg/longjmp causes uninitialized stack frame due to0d10193870b5a81c3bce13a602a5403c3a55cf6c Status in QEMU: Fix Released Bug description: Hi, I've got an ARM multithreaded test program that I wrote as a gcc testcase (attached) that fails on QEmu, firefox from Ubuntu ARM maverick also fails in the same way. The failure is either a seg fault or '*** longjmp causes uninitialized stack frame ***: ./arm-linux-user/qemu-arm terminated' and it fails every time. The test works on real hardware - a dual core A9 panda board. Firefox in an ARM maverick chroot also fails in the same way and is fixed in the same way. On 64bit Oneiric (i7-860 quad core) the backtrace from the seg looks like: #0 __sigsetjmp () at ../sysdeps/x86_64/setjmp.S:26 #1 0x60034cf4 in cpu_arm_exec (env=0x0) at /media/crypt/work/qemu/cpu-exec.c:233 #2 0x60006467 in cpu_loop (env=0x6226d060) at /media/crypt/work/qemu/linux-user/main.c:599 #3 0x60007984 in main (argc=, argv=, envp=) at /media/crypt/work/qemu/linux-user/main.c:3567 On 32bit lucid (core2 duo dual core) when it gives the longjmp error it's taken a bit of a more tortuous route but it looks like it originally took a seg at about the same place: #0 pthread_cond_wait () at ../nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S:123 #1 0x6344 in exclusive_idle () at /home/dg/linaro/git/qemu/linux-user/main.c:134 #2 start_exclusive () at /home/dg/linaro/git/qemu/linux-user/main.c:144 #3 stop_all_tasks () at /home/dg/linaro/git/qemu/linux-user/main.c:2996 #4 0x60016491 in force_sig (target_sig=6) at /home/dg/linaro/git/qemu/linux-user/signal.c:378 #5 0x60016f1d in queue_signal (env=0x639ff698, sig=6, info=0xb5610280) at /home/dg/linaro/git/qemu/linux-user/signal.c:451 #6 0x60017375 in host_signal_handler (host_signum=6, info=0xb561031c, puc=0xb561039c) at /home/dg/linaro/git/qemu/linux-user/signal.c:504 #7 #8 0x600c53d1 in raise () #9 0x6009a133 in abort () #10 0x600a0345 in __libc_message () #11 0x600b977c in __fortify_fail () #12 0x600b9717 in longjmp_chk () #13 0x600b9697 in __longjmp_chk () #14 0x6002b478 in cpu_loop_exit (env=0xb5611068) at /home/dg/linaro/git/qemu/cpu-exec.c:37 #15 0x6001d4ff in exception_action (host_signum=11, pinfo=0xb5610c8c, puc=0xb5610d0c) at /home/dg/linaro/git/qemu/user-exec.c:46 ---Type to continue, or q to quit--- #16 handle_cpu_signal (host_signum=11, pinfo=0xb5610c8c, puc=0xb5610d0c) at /home/dg/linaro/git/qemu/user-exec.c:123 #17 cpu_arm_signal_handler (host_signum=11, pinfo=0xb5610c8c, puc=0xb5610d0c) at /home/dg/linaro/git/qemu/user-exec.c:186 #18 0x600172f6 in host_signal_handler (host_signum=11, info=0xb5610c8c, puc=0xb5610d0c) at /home/dg/linaro/git/qemu/linux-user/signal.c:492 #19 #20 0x60099ac6 in _setjmp () #21 0x6002b4eb in cpu_arm_exec (env=0x0) at /home/dg/linaro/git/qemu/cpu-exec.c:233 #22 0x65bc in cpu_loop (env=0x639ff698) at /home/dg/linaro/git/qemu/linux-user/main.c:739 #23 0x60006134 in clone_func (arg=0xbfdcf95c) at /home/dg/linaro/git/qemu/linux-user/syscall.c:3953 #24 0x6008a8d0 in start_thread (arg=0xb5611b70) at pthread_create.c:300 #25 0x600b7f1e in clone () Things I've tried (with suggestions from Pete Maydell): If I remove the 'env = cpu_single_env;' added by 0d10193870b5a81c3bce13a602a5403c3a55cf6c (tcg: Reload local variables after return from longjmp) the test works reliably (10 out of 10 passes) on 32bit Lucid and partially (7 out of 10 passes) on 64 bit Oneiric (some segs, some hangs). If I make cpu_single_env thread local with __thread and leave 0d101... in, then again it works reliably on 32bit Lucid, and is flaky on 64 bit Oneiric (5/10 2 hangs, 3 segs) I've also tried using a volatile local variable in cpu_exec to hold a copy of env and restore that rather than cpu_single_env. With this it's solid on 32bit lucid and flaky on 64bit Oneirc; these failures on 64bit OO look like it running off the end of the code buffer (all 0 code), jumping to non-existent code addresses and a seg in tb_reset_jump_recursive2. With both __thread and the volatile local I still get failures on 64bit oneiric; they look mostly like they've run off the end of generated code (they're executing out of a buffer of all 0's). (I also tried some of the 64bit tests on an EC2 Xen Natty VM with similar results). My guess is I'm hitting multiple bugs here: 1) The Lucid install is probably too old to hit the compiler bugs for which 0d101... is a fix - but it is in itself triggering a new bug on the old compiler. 2) The 64bit
[Qemu-devel] [Bug 883133] Re: qemu on ARM hosts asserts due to code buffer/libc heap conflict
** Changed in: qemu Status: New => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/883133 Title: qemu on ARM hosts asserts due to code buffer/libc heap conflict Status in QEMU: Fix Committed Status in Linaro QEMU: In Progress Bug description: On ARM hosts qemu (about half the time) asserts on startup: qemu-system-i386: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed. This turns out to be because code_gen_alloc() is using mmap(MAP_FIXED) to map the code buffer at address 0x0100UL, which is in the area glibc happens to be using for its heap. This tends to make the next malloc() abort, although occasionally the stars align and we pass that and fail weirdly later on. I suspect we need to drop the MAP_FIXED requirement and fix the TCG code to cope with emitting code for longer-range branches for calls to host fns etc (calls/branches within the generated code should be ok to keep using the short-range branch insn I think). There is already no guarantee that the generated code and the host C code are within short branch range of each other... To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/883133/+subscriptions
Re: [Qemu-devel] [RFC] Device isolation infrastructure v2
On Thu, 2011-12-15 at 11:05 -0700, Alex Williamson wrote: > On Thu, 2011-12-15 at 17:25 +1100, David Gibson wrote: > > Here's the second spin of my preferred approach to handling grouping > > of devices for safe assignment to guests. > > > > Changes since v1: > > * Many name changes and file moves for improved consistency > > * Bugfixes and cleanups > > * The interface to the next layer up is considerably fleshed out, > >although it still needs work. > > * Example initialization of groups for p5ioc2 and p7ioc. > > > > TODO: > > * Need sample initialization of groups for intel and/or amd iommus > > I think this very well might imposed significant bloat for those > implementations. On POWER you typically don't have singleton groups, > while it's the norm on x86. I don't know that either intel or amd iommu > code have existing structures that they can simply tack the group > pointer to. Again, this is one of the reasons that I think the current > vfio implementation is the right starting point. We keep groups within > vfio, imposing zero overhead for systems not making use of it and only > require iommu drivers to implement a trivial function to opt-in. As we > start to make groups more pervasive in the dma layer, independent of > userspace driver exposure, we can offload pieces to the core. Starting > with it in the core and hand waving some future use that we don't plan > to implement right now seems like the wrong direction. > > > * Use of sysfs attributes to control group permission is probably a > >mistake. Although it seems a bit odd, registering a chardev for > >each group is probably better, because perms can be set from udev > >rules, just like everything else. > > I agree, this is a horrible mistake. Reinventing file permissions via > sysfs is bound to be broken and doesn't account for selinux permissions. > Again, I know you don't like aspects of the vfio group management, but > it gets this right imho. > > > * Need more details of what the binder structure will need to > >contain. > > * Handle complete removal of groups. > > * Clarify what will need to happen on the hot unplug path. > > We're still also removing devices from the driver model, this means > drivers like vfio need to re-implement a lot of the driver core for > driving each individual device in the group, and as you indicate, it's > unclear what happens in the hotplug path and I wonder if things like > suspend/resume will also require non-standard support. I really prefer > attaching individual bus drivers to devices using the standard > bind/unbind mechanisms. I have a hard time seeing how this is an > improvement from vfio. Thanks, I should also mention that I just pushed a new version of the vfio series out to github, it can be found here: git://github.com/awilliam/linux-vfio.git vfio-next-20111215 This fixes many bugs, including PCI config space access sizes and the todo item of actually preventing user access to the MSI-X table area. I think the vfio-pci driver is much closer to being read to submit now. It think I've addressed all the previous review comments. Still pending is a documentation refresh and some decision about what and how we expose iommu information. As usual, the matching qemu tree is here: git://github.com/awilliam/qemu-vfio.git vfio-ng I've tested this with an Intel 82576 (both PF and VF), Broadcom BCM5755, Intel HD Audio controller, and legacy PCI SB Live. Thanks, Alex
[Qemu-devel] [PATCH v3 10/11] isa: always use provided ISA bus in isa_bus_irqs()
Signed-off-by: Hervé Poussineau --- hw/isa-bus.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/isa-bus.c b/hw/isa-bus.c index 3207680..5af790b 100644 --- a/hw/isa-bus.c +++ b/hw/isa-bus.c @@ -53,8 +53,10 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io) void isa_bus_irqs(ISABus *bus, qemu_irq *irqs) { -assert(!bus || bus == isabus); -isabus->irqs = irqs; +if (!bus) { +hw_error("Can't set isa irqs with no isa bus present."); +} +bus->irqs = irqs; } /* -- 1.7.7.3
[Qemu-devel] [PATCH v3 07/11] fulong2e: give ISA bus to ISA methods
Signed-off-by: Hervé Poussineau --- hw/mips_fulong2e.c |6 ++ hw/vt82c686.c |4 ++-- hw/vt82c686.h |2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c index e6e120c..5e87665 100644 --- a/hw/mips_fulong2e.c +++ b/hw/mips_fulong2e.c @@ -264,7 +264,6 @@ static void mips_fulong2e_init(ram_addr_t ram_size, const char *boot_device, int64_t kernel_entry; qemu_irq *i8259; qemu_irq *cpu_exit_irq; -int via_devfn; PCIBus *pci_bus; ISABus *isa_bus; i2c_bus *smbus; @@ -338,12 +337,11 @@ static void mips_fulong2e_init(ram_addr_t ram_size, const char *boot_device, /* South bridge */ ide_drive_get(hd, MAX_IDE_BUS); -via_devfn = vt82c686b_init(pci_bus, PCI_DEVFN(FULONG2E_VIA_SLOT, 0)); -if (via_devfn < 0) { +isa_bus = vt82c686b_init(pci_bus, PCI_DEVFN(FULONG2E_VIA_SLOT, 0)); +if (!isa_bus) { fprintf(stderr, "vt82c686b_init error\n"); exit(1); } -isa_bus = NULL; /* Interrupt controller */ /* The 8259 -> IP5 */ diff --git a/hw/vt82c686.c b/hw/vt82c686.c index 2845959..038128b 100644 --- a/hw/vt82c686.c +++ b/hw/vt82c686.c @@ -507,13 +507,13 @@ static int vt82c686b_initfn(PCIDevice *d) return 0; } -int vt82c686b_init(PCIBus *bus, int devfn) +ISABus *vt82c686b_init(PCIBus *bus, int devfn) { PCIDevice *d; d = pci_create_simple_multifunction(bus, devfn, true, "VT82C686B"); -return d->devfn; +return DO_UPCAST(ISABus, qbus, qdev_get_child_bus(&d->qdev, "isa.0")); } static PCIDeviceInfo via_info = { diff --git a/hw/vt82c686.h b/hw/vt82c686.h index e3270ca..6ef876d 100644 --- a/hw/vt82c686.h +++ b/hw/vt82c686.h @@ -2,7 +2,7 @@ #define HW_VT82C686_H /* vt82c686.c */ -int vt82c686b_init(PCIBus * bus, int devfn); +ISABus *vt82c686b_init(PCIBus * bus, int devfn); void vt82c686b_ac97_init(PCIBus *bus, int devfn); void vt82c686b_mc97_init(PCIBus *bus, int devfn); i2c_bus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base, -- 1.7.7.3
[Qemu-devel] [PATCH v3 01/11] isa: give ISABus/ISADevice to isa_create(), isa_bus_irqs() and isa_get_irq() functions
NULL is a valid bus/device, so there is no change in behaviour. Signed-off-by: Hervé Poussineau --- arch_init.c|8 arch_init.h|2 +- hw/adlib.c |2 +- hw/alpha_dp264.c | 10 ++ hw/alpha_typhoon.c |7 --- hw/audiodev.h |8 hw/cs4231a.c |4 ++-- hw/fdc.h |4 ++-- hw/gus.c |4 ++-- hw/i8254.c |2 +- hw/i8259.c |6 +++--- hw/ide.h |2 +- hw/ide/isa.c |4 ++-- hw/ide/piix.c |2 +- hw/ide/via.c |2 +- hw/isa-bus.c | 18 +++--- hw/isa.h | 11 +-- hw/m48t59.c|5 +++-- hw/mc146818rtc.c |4 ++-- hw/mc146818rtc.h |2 +- hw/mips_fulong2e.c | 16 +--- hw/mips_jazz.c | 13 +++-- hw/mips_malta.c| 26 ++ hw/mips_r4k.c | 21 +++-- hw/nvram.h |3 ++- hw/pc.c| 28 ++-- hw/pc.h| 35 ++- hw/pc_piix.c | 19 +++ hw/pcspk.c |2 +- hw/ppc_prep.c | 20 +++- hw/sb16.c |4 ++-- hw/sun4u.c | 20 qemu-common.h |1 + 33 files changed, 170 insertions(+), 145 deletions(-) diff --git a/arch_init.c b/arch_init.c index a411fdf..3bc2a41 100644 --- a/arch_init.c +++ b/arch_init.c @@ -473,7 +473,7 @@ struct soundhw { int enabled; int isa; union { -int (*init_isa) (qemu_irq *pic); +int (*init_isa) (ISABus *bus, qemu_irq *pic); int (*init_pci) (PCIBus *bus); } init; }; @@ -628,7 +628,7 @@ void select_soundhw(const char *optarg) } } -void audio_init(qemu_irq *isa_pic, PCIBus *pci_bus) +void audio_init(ISABus *isa_bus, qemu_irq *isa_pic, PCIBus *pci_bus) { struct soundhw *c; @@ -636,7 +636,7 @@ void audio_init(qemu_irq *isa_pic, PCIBus *pci_bus) if (c->enabled) { if (c->isa) { if (isa_pic) { -c->init.init_isa(isa_pic); +c->init.init_isa(isa_bus, isa_pic); } } else { if (pci_bus) { @@ -650,7 +650,7 @@ void audio_init(qemu_irq *isa_pic, PCIBus *pci_bus) void select_soundhw(const char *optarg) { } -void audio_init(qemu_irq *isa_pic, PCIBus *pci_bus) +void audio_init(ISABus *isa_bus, qemu_irq *isa_pic, PCIBus *pci_bus) { } #endif diff --git a/arch_init.h b/arch_init.h index a74187a..074f02a 100644 --- a/arch_init.h +++ b/arch_init.h @@ -27,7 +27,7 @@ void do_acpitable_option(const char *optarg); void do_smbios_option(const char *optarg); void cpudef_init(void); int audio_available(void); -void audio_init(qemu_irq *isa_pic, PCIBus *pci_bus); +void audio_init(ISABus *isa_bus, qemu_irq *isa_pic, PCIBus *pci_bus); int tcg_available(void); int kvm_available(void); int xen_available(void); diff --git a/hw/adlib.c b/hw/adlib.c index e4bfcc6..b5e1564 100644 --- a/hw/adlib.c +++ b/hw/adlib.c @@ -275,7 +275,7 @@ static void Adlib_fini (AdlibState *s) AUD_remove_card (&s->card); } -int Adlib_init (qemu_irq *pic) +int Adlib_init (ISABus *bus, qemu_irq *pic) { AdlibState *s = &glob_adlib; struct audsettings as; diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c index 598b830..a07a2ff 100644 --- a/hw/alpha_dp264.c +++ b/hw/alpha_dp264.c @@ -50,6 +50,7 @@ static void clipper_init(ram_addr_t ram_size, { CPUState *cpus[4]; PCIBus *pci_bus; +ISABus *isa_bus; qemu_irq rtc_irq; long size, i; const char *palcode_filename; @@ -68,10 +69,11 @@ static void clipper_init(ram_addr_t ram_size, /* Init the chipset. */ pci_bus = typhoon_init(ram_size, &rtc_irq, cpus, clipper_pci_map_irq); +isa_bus = NULL; -rtc_init(1980, rtc_irq); -pit_init(0x40, 0); -isa_create_simple("i8042"); +rtc_init(isa_bus, 1980, rtc_irq); +pit_init(isa_bus, 0x40, 0); +isa_create_simple(isa_bus, "i8042"); /* VGA setup. Don't bother loading the bios. */ alpha_pci_vga_setup(pci_bus); @@ -79,7 +81,7 @@ static void clipper_init(ram_addr_t ram_size, /* Serial code setup. */ for (i = 0; i < MAX_SERIAL_PORTS; ++i) { if (serial_hds[i]) { -serial_isa_init(i, serial_hds[i]); +serial_isa_init(isa_bus, i, serial_hds[i]); } } diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c index c7608bb..113837d 100644 --- a/hw/alpha_typhoon.c +++ b/hw/alpha_typhoon.c @@ -791,11 +791,12 @@ PCIBus *typhoon_init(ram_addr_t ram_size, qemu_irq *p_rtc_irq, /* ??? Technically there should be a cy82c693ub pci-isa bridge. */ { qemu_irq isa_pci_irq, *isa_irqs; +ISABus *isa_bus; -isa_bus_new(NULL, addr_space_io); +isa_bus = isa_bus_new(NULL, addr_space_io); isa_pci_irq = *qemu_allocate_irqs(typho
[Qemu-devel] [PATCH v3 09/11] isa: always use provided ISA bus when creating an isa device
Signed-off-by: Hervé Poussineau --- hw/isa-bus.c | 10 -- 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/hw/isa-bus.c b/hw/isa-bus.c index 7c94f0b..3207680 100644 --- a/hw/isa-bus.c +++ b/hw/isa-bus.c @@ -130,12 +130,11 @@ ISADevice *isa_create(ISABus *bus, const char *name) { DeviceState *dev; -assert(!bus || bus == isabus); -if (!isabus) { +if (!bus) { hw_error("Tried to create isa device %s with no isa bus present.", name); } -dev = qdev_create(&isabus->qbus, name); +dev = qdev_create(&bus->qbus, name); return DO_UPCAST(ISADevice, qdev, dev); } @@ -143,12 +142,11 @@ ISADevice *isa_try_create(ISABus *bus, const char *name) { DeviceState *dev; -assert(!bus || bus == isabus); -if (!isabus) { +if (!bus) { hw_error("Tried to create isa device %s with no isa bus present.", name); } -dev = qdev_try_create(&isabus->qbus, name); +dev = qdev_try_create(&bus->qbus, name); return DO_UPCAST(ISADevice, qdev, dev); } -- 1.7.7.3
Re: [Qemu-devel] [RFC] Plan for moving forward with QOM
> But there are two distinct classes of user. One class of user really is > thinking: > > "I want a KVM virtual machine, with 3 disks and 2 network cards" > > They could give a flying hoot whether the i440fx inherits from pcidevice or > implements pcibus. > > We need to provide an obviously distinct UI and API for these users. The > vast majority of command line users fall into this category. And once > this user learns how do create a guest with 3 disks and 2 network cards, > they should never have to learn another way of doing it. Ok. > There is a second class of "user" that is doing very sophisticated things > and cares about this information. But this sophisticated user (i.e. > libvirt) wants to be able to probe QEMU to understand what features > are probably available because it very likely is evolving just as quickly as > QEMU is. I disagree. Maybe this is because you're coming from the PC/Linux world where all hardware is basically the same. In the embedded world there are many more vastly different machines. And some of those have several similar but different enough to matter variants. There's a good chance the guest OS needs exactly the right one. Take the Stellaris boards as an example. We currently implement two boards. There are well over a hundred variants in this SoC family. A good proportion of these chips will be used in projects that include a custom PCB design. There are at least three different reference boards for the LM3S6965 alone. This is not unusual. I really don't want to have to teach qemu about hunderds of different SoCs connected to thousands of different "motherboards", most of which we'll never even know about. I'd like for users to be able to create their own hardware config without having to track qemu implementation details. I'm fine with having to hack/rebuild qemu to add a new device implementation. Having to do that for every trivial rewiring of existing devices is not so fun. > > I haven't worked out the details, maybe we need a "Self" property, plus a > > policy of never having user visible stuff link to an primary device node. > > If the primary object happens to implement/inherit from that Interface > > then it sets the property to itself. Otherwise it creates a stateful > > bus object (maybe using composition). > > You're advocating only connecting properties to properties, and never an > object to a property? I think that's needlessly complicated. In the vast > majority of cases, you just case about saying "connect this > CharDriverState to this Serial device". We should make it much more > complicated than that. Yes, that's what I'm saying. I'm finding it hard to believe that more than one link being stateful is such an exceptional case. Am I right in thinking that you're effectively implementing multiple inheritance via properties? If so I guess works around the single-inheritance limitation in many cases. > If you aren't using inheritance, yes, you need to pass closures to the > child objects. I dislike that kind of proxy modeling. > >>> > >>> How would you solve this using inheritance? > >>> > >>> I can see how it works when the device knows its address, but it seems > >>> kinda lame to tell a device "You have a dedicated communication > >>> channel. > >>> > >>> But I'm lazy and will smush them all together. Please add this > >>> > >>> additional token to every signal you send". > >> > >> Yes, adding a token is how you would have to do it. > > > > Ugh. Except it's worse than I thought. That token has to come from the > > user. Either via some arbitrary property on the client device, which is > > going to be different for every device especially when a device can link > > to multiple interfaces of the same class. Or we need some mechanism for > > attaching data to a link, rather than just conecting the two interfaces > > together. Neither of which sound desirable. > > But this entire use-case seems to be synthetic. Do you have a real case > where you would want to inherit twice from the same interface? A GPIO controller (of which interrupt controllers are a subset). We want to expose and use many single-bit control line interfaces. I suppose pckbd.c is annother example. This implements a pair of PS/2 serial busses. Currently we don't model these and use hardcoded callbacks when creating the keyboard/mouse devices. Once modelled properly I'd expect a board to be able to replace either with a third [as yet unimplemented] PS/2 device. Any of the those devices should also be usable with the single-port ARM PL050 controller. Paul
[Qemu-devel] [PATCH v3 08/11] malta: give ISA bus to ISA methods
Signed-off-by: Hervé Poussineau --- hw/mips_malta.c |3 +-- hw/pc.h |2 +- hw/piix4.c |3 ++- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/mips_malta.c b/hw/mips_malta.c index 621e661..330924e 100644 --- a/hw/mips_malta.c +++ b/hw/mips_malta.c @@ -942,8 +942,7 @@ void mips_malta_init (ram_addr_t ram_size, /* Southbridge */ ide_drive_get(hd, MAX_IDE_BUS); -piix4_devfn = piix4_init(pci_bus, 80); -isa_bus = NULL; +piix4_devfn = piix4_init(pci_bus, &isa_bus, 80); /* Interrupt controller */ /* The 8259 is attached to the MIPS CPU INT0 pin, ie interrupt 2 */ diff --git a/hw/pc.h b/hw/pc.h index 04e72cc..17648cc 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -196,7 +196,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, /* piix4.c */ extern PCIDevice *piix4_dev; -int piix4_init(PCIBus *bus, int devfn); +int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn); /* vga.c */ enum vga_retrace_method { diff --git a/hw/piix4.c b/hw/piix4.c index 2fd1171..51af459 100644 --- a/hw/piix4.c +++ b/hw/piix4.c @@ -93,11 +93,12 @@ static int piix4_initfn(PCIDevice *dev) return 0; } -int piix4_init(PCIBus *bus, int devfn) +int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn) { PCIDevice *d; d = pci_create_simple_multifunction(bus, devfn, true, "PIIX4"); +*isa_bus = DO_UPCAST(ISABus, qbus, qdev_get_child_bus(&d->qdev, "isa.0")); return d->devfn; } -- 1.7.7.3
[Qemu-devel] [PATCH v3 11/11] audio: remove unused parameter isa_pic
Signed-off-by: Hervé Poussineau --- arch_init.c | 10 +- arch_init.h |2 +- hw/adlib.c |2 +- hw/audiodev.h |8 hw/cs4231a.c|2 +- hw/gus.c|2 +- hw/mips_jazz.c |2 +- hw/mips_malta.c |2 +- hw/pc.h |2 +- hw/pc_piix.c|2 +- hw/pcspk.c |2 +- hw/sb16.c |2 +- 12 files changed, 19 insertions(+), 19 deletions(-) diff --git a/arch_init.c b/arch_init.c index 3bc2a41..d4c92b0 100644 --- a/arch_init.c +++ b/arch_init.c @@ -473,7 +473,7 @@ struct soundhw { int enabled; int isa; union { -int (*init_isa) (ISABus *bus, qemu_irq *pic); +int (*init_isa) (ISABus *bus); int (*init_pci) (PCIBus *bus); } init; }; @@ -628,15 +628,15 @@ void select_soundhw(const char *optarg) } } -void audio_init(ISABus *isa_bus, qemu_irq *isa_pic, PCIBus *pci_bus) +void audio_init(ISABus *isa_bus, PCIBus *pci_bus) { struct soundhw *c; for (c = soundhw; c->name; ++c) { if (c->enabled) { if (c->isa) { -if (isa_pic) { -c->init.init_isa(isa_bus, isa_pic); +if (isa_bus) { +c->init.init_isa(isa_bus); } } else { if (pci_bus) { @@ -650,7 +650,7 @@ void audio_init(ISABus *isa_bus, qemu_irq *isa_pic, PCIBus *pci_bus) void select_soundhw(const char *optarg) { } -void audio_init(ISABus *isa_bus, qemu_irq *isa_pic, PCIBus *pci_bus) +void audio_init(ISABus *isa_bus, PCIBus *pci_bus) { } #endif diff --git a/arch_init.h b/arch_init.h index 074f02a..828256c 100644 --- a/arch_init.h +++ b/arch_init.h @@ -27,7 +27,7 @@ void do_acpitable_option(const char *optarg); void do_smbios_option(const char *optarg); void cpudef_init(void); int audio_available(void); -void audio_init(ISABus *isa_bus, qemu_irq *isa_pic, PCIBus *pci_bus); +void audio_init(ISABus *isa_bus, PCIBus *pci_bus); int tcg_available(void); int kvm_available(void); int xen_available(void); diff --git a/hw/adlib.c b/hw/adlib.c index b5e1564..dd8b188 100644 --- a/hw/adlib.c +++ b/hw/adlib.c @@ -275,7 +275,7 @@ static void Adlib_fini (AdlibState *s) AUD_remove_card (&s->card); } -int Adlib_init (ISABus *bus, qemu_irq *pic) +int Adlib_init (ISABus *bus) { AdlibState *s = &glob_adlib; struct audsettings as; diff --git a/hw/audiodev.h b/hw/audiodev.h index bfa324a..ed2790f 100644 --- a/hw/audiodev.h +++ b/hw/audiodev.h @@ -2,19 +2,19 @@ int es1370_init(PCIBus *bus); /* sb16.c */ -int SB16_init(ISABus *bus, qemu_irq *pic); +int SB16_init(ISABus *bus); /* adlib.c */ -int Adlib_init(ISABus *bus, qemu_irq *pic); +int Adlib_init(ISABus *bus); /* gus.c */ -int GUS_init(ISABus *bus, qemu_irq *pic); +int GUS_init(ISABus *bus); /* ac97.c */ int ac97_init(PCIBus *bus); /* cs4231a.c */ -int cs4231a_init(ISABus *bus, qemu_irq *pic); +int cs4231a_init(ISABus *bus); /* intel-hda.c + hda-audio.c */ int intel_hda_and_codec_init(PCIBus *bus); diff --git a/hw/cs4231a.c b/hw/cs4231a.c index 0238829..dc77a3a 100644 --- a/hw/cs4231a.c +++ b/hw/cs4231a.c @@ -659,7 +659,7 @@ static int cs4231a_initfn (ISADevice *dev) return 0; } -int cs4231a_init (ISABus *bus, qemu_irq *pic) +int cs4231a_init (ISABus *bus) { isa_create_simple (bus, "cs4231a"); return 0; diff --git a/hw/gus.c b/hw/gus.c index 17cceee..ab872d8 100644 --- a/hw/gus.c +++ b/hw/gus.c @@ -293,7 +293,7 @@ static int gus_initfn (ISADevice *dev) return 0; } -int GUS_init (ISABus *bus, qemu_irq *pic) +int GUS_init (ISABus *bus) { isa_create_simple (bus, "gus"); return 0; diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c index 8ed66ce..da04982 100644 --- a/hw/mips_jazz.c +++ b/hw/mips_jazz.c @@ -281,7 +281,7 @@ static void mips_jazz_init(MemoryRegion *address_space, /* Sound card */ /* FIXME: missing Jazz sound at 0x8000c000, rc4030[2] */ -audio_init(isa_bus, i8259, NULL); +audio_init(isa_bus, NULL); /* NVRAM */ dev = qdev_create(NULL, "ds1225y"); diff --git a/hw/mips_malta.c b/hw/mips_malta.c index 330924e..d94ad1d 100644 --- a/hw/mips_malta.c +++ b/hw/mips_malta.c @@ -973,7 +973,7 @@ void mips_malta_init (ram_addr_t ram_size, fdctrl_init_isa(isa_bus, fd); /* Sound card */ -audio_init(isa_bus, NULL, pci_bus); +audio_init(isa_bus, pci_bus); /* Network card */ network_init(); diff --git a/hw/pc.h b/hw/pc.h index 17648cc..13e41f1 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -176,7 +176,7 @@ extern int no_hpet; /* pcspk.c */ void pcspk_init(ISADevice *pit); -int pcspk_audio_init(ISABus *bus, qemu_irq *pic); +int pcspk_audio_init(ISABus *bus); /* piix_pci.c */ struct PCII440FXState; diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 4670f8f..d4b5cff 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -222,7 +222,7 @@ static void pc_init1(MemoryRegion *system_memory, qdev_property_add_child(qdev_resolve_path("/
[Qemu-devel] [PATCH v3 06/11] sun4u: give ISA bus to ISA methods
Signed-off-by: Hervé Poussineau --- hw/sun4u.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/sun4u.c b/hw/sun4u.c index dfb81da..e3e8dde 100644 --- a/hw/sun4u.c +++ b/hw/sun4u.c @@ -530,10 +530,12 @@ static ISABus * pci_ebus_init(PCIBus *bus, int devfn) { qemu_irq *isa_irq; +PCIDevice *pci_dev; ISABus *isa_bus; -pci_create_simple(bus, devfn, "ebus"); -isa_bus = NULL; +pci_dev = pci_create_simple(bus, devfn, "ebus"); +isa_bus = DO_UPCAST(ISABus, qbus, +qdev_get_child_bus(&pci_dev->qdev, "isa.0")); isa_irq = qemu_allocate_irqs(dummy_isa_irq_handler, NULL, 16); isa_bus_irqs(isa_bus, isa_irq); return isa_bus; -- 1.7.7.3
[Qemu-devel] [PATCH v3 04/11] pc: give ISA bus to ISA methods
Signed-off-by: Hervé Poussineau --- hw/pc.h |2 +- hw/pc_piix.c |3 +-- hw/piix_pci.c |8 +--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/hw/pc.h b/hw/pc.h index afb7535..04e72cc 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -183,7 +183,7 @@ struct PCII440FXState; typedef struct PCII440FXState PCII440FXState; PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, -qemu_irq *pic, +ISABus **isa_bus, qemu_irq *pic, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, ram_addr_t ram_size, diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 2939a55..4670f8f 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -137,7 +137,7 @@ static void pc_init1(MemoryRegion *system_memory, gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS); if (pci_enabled) { -pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, gsi, +pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi, system_memory, system_io, ram_size, below_4g_mem_size, 0x1ULL - below_4g_mem_size, @@ -146,7 +146,6 @@ static void pc_init1(MemoryRegion *system_memory, ? 0 : ((uint64_t)1 << 62)), pci_memory, ram_memory); -isa_bus = NULL; } else { pci_bus = NULL; i440fx_state = NULL; diff --git a/hw/piix_pci.c b/hw/piix_pci.c index d785d4b..57f5ea6 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -263,7 +263,7 @@ static int i440fx_initfn(PCIDevice *dev) static PCIBus *i440fx_common_init(const char *device_name, PCII440FXState **pi440fx_state, int *piix3_devfn, - qemu_irq *pic, + ISABus **isa_bus, qemu_irq *pic, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, ram_addr_t ram_size, @@ -328,6 +328,8 @@ static PCIBus *i440fx_common_init(const char *device_name, qdev_property_add_child(dev, "piix3", &piix3->dev.qdev, NULL); } piix3->pic = pic; +*isa_bus = DO_UPCAST(ISABus, qbus, + qdev_get_child_bus(&piix3->dev.qdev, "isa.0")); (*pi440fx_state)->piix3 = piix3; @@ -344,7 +346,7 @@ static PCIBus *i440fx_common_init(const char *device_name, } PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, -qemu_irq *pic, +ISABus **isa_bus, qemu_irq *pic, MemoryRegion *address_space_mem, MemoryRegion *address_space_io, ram_addr_t ram_size, @@ -357,7 +359,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, { PCIBus *b; -b = i440fx_common_init("i440FX", pi440fx_state, piix3_devfn, pic, +b = i440fx_common_init("i440FX", pi440fx_state, piix3_devfn, isa_bus, pic, address_space_mem, address_space_io, ram_size, pci_hole_start, pci_hole_size, pci_hole64_size, pci_hole64_size, -- 1.7.7.3
[Qemu-devel] [PATCH v3 05/11] alpha: give ISA bus to ISA methods
Signed-off-by: Hervé Poussineau --- hw/alpha_dp264.c |4 ++-- hw/alpha_sys.h |3 ++- hw/alpha_typhoon.c | 10 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c index a07a2ff..876335a 100644 --- a/hw/alpha_dp264.c +++ b/hw/alpha_dp264.c @@ -68,8 +68,8 @@ static void clipper_init(ram_addr_t ram_size, cpus[0]->trap_arg2 = smp_cpus; /* Init the chipset. */ -pci_bus = typhoon_init(ram_size, &rtc_irq, cpus, clipper_pci_map_irq); -isa_bus = NULL; +pci_bus = typhoon_init(ram_size, &isa_bus, &rtc_irq, cpus, + clipper_pci_map_irq); rtc_init(isa_bus, 1980, rtc_irq); pit_init(isa_bus, 0x40, 0); diff --git a/hw/alpha_sys.h b/hw/alpha_sys.h index 13f0177..d54b18f 100644 --- a/hw/alpha_sys.h +++ b/hw/alpha_sys.h @@ -12,7 +12,8 @@ #include "irq.h" -PCIBus *typhoon_init(ram_addr_t, qemu_irq *, CPUState *[4], pci_map_irq_fn); +PCIBus *typhoon_init(ram_addr_t, ISABus **, qemu_irq *, CPUState *[4], + pci_map_irq_fn); /* alpha_pci.c. */ extern const MemoryRegionOps alpha_pci_bw_io_ops; diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c index 113837d..adf7382 100644 --- a/hw/alpha_typhoon.c +++ b/hw/alpha_typhoon.c @@ -691,7 +691,8 @@ static void typhoon_alarm_timer(void *opaque) cpu_interrupt(s->cchip.cpu[cpu], CPU_INTERRUPT_TIMER); } -PCIBus *typhoon_init(ram_addr_t ram_size, qemu_irq *p_rtc_irq, +PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus, + qemu_irq *p_rtc_irq, CPUState *cpus[4], pci_map_irq_fn sys_map_irq) { const uint64_t MB = 1024 * 1024; @@ -791,12 +792,11 @@ PCIBus *typhoon_init(ram_addr_t ram_size, qemu_irq *p_rtc_irq, /* ??? Technically there should be a cy82c693ub pci-isa bridge. */ { qemu_irq isa_pci_irq, *isa_irqs; -ISABus *isa_bus; -isa_bus = isa_bus_new(NULL, addr_space_io); +*isa_bus = isa_bus_new(NULL, addr_space_io); isa_pci_irq = *qemu_allocate_irqs(typhoon_set_isa_irq, s, 1); -isa_irqs = i8259_init(isa_bus, isa_pci_irq); -isa_bus_irqs(isa_bus, isa_irqs); +isa_irqs = i8259_init(*isa_bus, isa_pci_irq); +isa_bus_irqs(*isa_bus, isa_irqs); } return b; -- 1.7.7.3
[Qemu-devel] [PATCH v3 02/11] isa: move ISABus structure definition to header file
Signed-off-by: Hervé Poussineau --- hw/isa-bus.c |5 - hw/isa.h |6 ++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/hw/isa-bus.c b/hw/isa-bus.c index dcbb134..7c94f0b 100644 --- a/hw/isa-bus.c +++ b/hw/isa-bus.c @@ -22,11 +22,6 @@ #include "isa.h" #include "exec-memory.h" -struct ISABus { -BusState qbus; -MemoryRegion *address_space_io; -qemu_irq *irqs; -}; static ISABus *isabus; target_phys_addr_t isa_mem_base = 0; diff --git a/hw/isa.h b/hw/isa.h index 1e75f87..b11a0be 100644 --- a/hw/isa.h +++ b/hw/isa.h @@ -12,6 +12,12 @@ typedef struct ISADevice ISADevice; typedef struct ISADeviceInfo ISADeviceInfo; +struct ISABus { +BusState qbus; +MemoryRegion *address_space_io; +qemu_irq *irqs; +}; + struct ISADevice { DeviceState qdev; uint32_t isairq[2]; -- 1.7.7.3
[Qemu-devel] [PATCH v3 03/11] i8259: give ISA device to isa_register_ioport()
Signed-off-by: Hervé Poussineau --- hw/i8259.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/i8259.c b/hw/i8259.c index 4446339..7331e0e 100644 --- a/hw/i8259.c +++ b/hw/i8259.c @@ -469,9 +469,9 @@ static int pic_initfn(ISADevice *dev) memory_region_init_io(&s->base_io, &pic_base_ioport_ops, s, "pic", 2); memory_region_init_io(&s->elcr_io, &pic_elcr_ioport_ops, s, "elcr", 1); -isa_register_ioport(NULL, &s->base_io, s->iobase); +isa_register_ioport(dev, &s->base_io, s->iobase); if (s->elcr_addr != -1) { -isa_register_ioport(NULL, &s->elcr_io, s->elcr_addr); +isa_register_ioport(dev, &s->elcr_io, s->elcr_addr); } qdev_init_gpio_out(&dev->qdev, s->int_out, ARRAY_SIZE(s->int_out)); -- 1.7.7.3
[Qemu-devel] [PATCH v3 00/11] isa: preliminary work for multiple buses
Current patches are a rework of my patches already available at [1]. They don't provide full support for multiple ISA buses (yet), but add a ISABus or ISADevice argument to all ISA functions. They are mostly mechanically touching every instanciation of ISA devices, so number of lines is quite high even if impact is quite low. Some patches don't pass checkpass check due to spaces around parentheses, but malc asked to do so on files he maintains. Some more patches need to be provided to support multiple ISA buses, but they will mostly touch ISA bridges and hw/isa-bus.c file. Thanks [1] http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg00094.html Changes v2->v3 rebased fixed compilation with some compilers (typedef redefinition) Changes v1->v2 rebased Hervé Poussineau (11): isa: give ISABus/ISADevice to isa_create(), isa_bus_irqs() and isa_get_irq() functions isa: move ISABus structure definition to header file i8259: give ISA device to isa_register_ioport() pc: give ISA bus to ISA methods alpha: give ISA bus to ISA methods sun4u: give ISA bus to ISA methods fulong2e: give ISA bus to ISA methods malta: give ISA bus to ISA methods isa: always use provided ISA bus when creating an isa device isa: always use provided ISA bus in isa_bus_irqs() audio: remove unused parameter isa_pic arch_init.c| 10 +- arch_init.h|2 +- hw/adlib.c |2 +- hw/alpha_dp264.c | 12 +++- hw/alpha_sys.h |3 ++- hw/alpha_typhoon.c |9 + hw/audiodev.h |8 hw/cs4231a.c |4 ++-- hw/fdc.h |4 ++-- hw/gus.c |4 ++-- hw/i8254.c |2 +- hw/i8259.c | 10 +- hw/ide.h |2 +- hw/ide/isa.c |4 ++-- hw/ide/piix.c |2 +- hw/ide/via.c |2 +- hw/isa-bus.c | 33 - hw/isa.h | 17 +++-- hw/m48t59.c|5 +++-- hw/mc146818rtc.c |4 ++-- hw/mc146818rtc.h |2 +- hw/mips_fulong2e.c | 20 ++-- hw/mips_jazz.c | 13 +++-- hw/mips_malta.c| 27 ++- hw/mips_r4k.c | 21 +++-- hw/nvram.h |3 ++- hw/pc.c| 28 ++-- hw/pc.h| 39 --- hw/pc_piix.c | 20 +++- hw/pcspk.c |2 +- hw/piix4.c |3 ++- hw/piix_pci.c |8 +--- hw/ppc_prep.c | 20 +++- hw/sb16.c |4 ++-- hw/sun4u.c | 24 +++- hw/vt82c686.c |4 ++-- hw/vt82c686.h |2 +- qemu-common.h |1 + 38 files changed, 204 insertions(+), 176 deletions(-) -- 1.7.7.3
Re: [Qemu-devel] [PATCH v9 0/3] PC system flash support
I verified that 'info mtree' and 'info qdev' are equivalent for pc-1.0 when using master and with my patches. However, I did discover that v1.0 seems to differ from master for this same test. I've attached the logs. --- v1.0-pc.log 2011-12-15 12:14:53.0 -0800 +++ master-pc-1.0.log 2011-12-15 12:25:04.0 -0800 @@ -1,4 +1,4 @@ -QEMU 1.0 monitor - type 'help' for more information +QEMU 1.0.50 monitor - type 'help' for more information (qemu) info mtree memory -7ffe (prio 0): system @@ -18,6 +18,8 @@ 000ec000-000e (prio 1): alias pam-ram @pc.ram 000ec000-000e 000f-000f (prio 1): alias pam-rom @pc.ram 000f-000f 0800- (prio 0): alias pci-hole @pci 0800- + fec0-fec00fff (prio 0): ioapic + fed0-fed003ff (prio 0): hpet fee0-feef (prio 0): apic 4000-7fff (prio 0): alias pci-hole64 @pci 4000-7fff pc.ram @@ -56,6 +58,7 @@ 03f8-03ff (prio 0): serial 04d0-04d0 (prio 0): elcr 04d1-04d1 (prio 0): elcr + 0510-0511 (prio 0): fwcfg 0cf8-0cfb (prio 0): pci-conf-idx 0cfc-0cff (prio 0): pci-conf-data 5658-5658 (prio 0): vmport @@ -126,7 +129,7 @@ dev-prop: opt_io_size = 0 dev-prop: bootindex = -1 dev-prop: discard_granularity = 0 -dev-prop: ver = "1.0" +dev-prop: ver = "1.0.50" dev-prop: serial = "QM3" bus-prop: unit = 0 bus: ide.0 @@ -218,7 +221,7 @@ dev-prop: data_iobase = 0x511 irq 0 mmio /0002 -mmio /0002 +mmio /0001 dev: apic, id "" dev-prop: id = 0 irq 0 Thanks, -Jordan On Thu, Dec 15, 2011 at 12:51, Jordan Justen wrote: > Enable flash emulation in a PC system using pflash_cfi01. > > v9: > * Add pc-1.1 > * pc-1.0 uses previous rom firmware init code path > > v8: > * Cleanup two chunks of debug code (printf messages) > * Fix comment in pc.h (pcflash.c => pc_sysfw.c) > > v7: > * Do not add system firmware to qemu roms > * If kvm is enabled, copy pflash drive contents into a > read-only ram region, since kvm cannot currently execute > code from a pflash device. > * Rename pcflash.c to pc_sysfw.c > > v6: > * Rebase for memory API > * pflash_cfi01: Set error in status register when a write to > erase is attempted in read-only mode. > * Add system firmware to qemu roms > > v5: > * Enable pflash read-only mode > * Enable -drive with if=pflash to define system firmware image > > v4: > * Rebase > > v3: > * Fix code style issues > * Add additional comments > > v2: > * Convert debug printf to DPRINTF > > Jordan Justen (3): > pc: Add pc-1.1 machine type > pflash: Support read-only mode > pc: Support system flash memory with pflash > > Makefile.target | 1 + > blockdev.c | 3 +- > default-configs/i386-softmmu.mak | 1 + > default-configs/x86_64-softmmu.mak | 1 + > hw/boards.h | 1 + > hw/pc.c | 58 +--- > hw/pc.h | 7 +- > hw/pc_piix.c | 40 +- > hw/pc_sysfw.c | 255 > > hw/pflash_cfi01.c | 44 -- > hw/pflash_cfi02.c | 83 +++-- > vl.c | 2 +- > 12 files changed, 382 insertions(+), 114 deletions(-) > create mode 100644 hw/pc_sysfw.c > > master-pc-1.0.log Description: Binary data v1.0-pc.log Description: Binary data
[Qemu-devel] [PATCH v9 1/3] pc: Add pc-1.1 machine type
This machine type adds a new system_firmware_enabled parameter which is passed into pc_memory_init. This will be used by the system firmware support which enables a flash memory to be used with qemu. Signed-off-by: Jordan Justen Cc: Avi Kivity --- hw/pc.c |3 ++- hw/pc.h |3 ++- hw/pc_piix.c | 40 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index 33778fe..cc6cfad 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -971,7 +971,8 @@ void pc_memory_init(MemoryRegion *system_memory, ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, -MemoryRegion **ram_memory) +MemoryRegion **ram_memory, +int system_firmware_enabled) { char *filename; int ret, linux_boot, i; diff --git a/hw/pc.h b/hw/pc.h index b7b7e40..49471cb 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -138,7 +138,8 @@ void pc_memory_init(MemoryRegion *system_memory, ram_addr_t below_4g_mem_size, ram_addr_t above_4g_mem_size, MemoryRegion *rom_memory, -MemoryRegion **ram_memory); +MemoryRegion **ram_memory, +int system_firmware_enabled); qemu_irq *pc_allocate_cpu_irq(void); void pc_vga_init(PCIBus *pci_bus); void pc_basic_device_init(qemu_irq *gsi, diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 970f43c..007f10c 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -79,7 +79,8 @@ static void pc_init1(MemoryRegion *system_memory, const char *initrd_filename, const char *cpu_model, int pci_enabled, - int kvmclock_enabled) + int kvmclock_enabled, + int system_firmware_enabled) { int i; ram_addr_t below_4g_mem_size, above_4g_mem_size; @@ -128,7 +129,8 @@ static void pc_init1(MemoryRegion *system_memory, pc_memory_init(system_memory, kernel_filename, kernel_cmdline, initrd_filename, below_4g_mem_size, above_4g_mem_size, - pci_enabled ? rom_memory : system_memory, &ram_memory); + pci_enabled ? rom_memory : system_memory, &ram_memory, + system_firmware_enabled); } gsi_state = g_malloc0(sizeof(*gsi_state)); @@ -246,7 +248,21 @@ static void pc_init_pci(ram_addr_t ram_size, get_system_io(), ram_size, boot_device, kernel_filename, kernel_cmdline, - initrd_filename, cpu_model, 1, 1); + initrd_filename, cpu_model, 1, 1, 1); +} + +static void pc_init_pci_system_rom_only(ram_addr_t ram_size, +const char *boot_device, +const char *kernel_filename, +const char *kernel_cmdline, +const char *initrd_filename, +const char *cpu_model) +{ +pc_init1(get_system_memory(), + get_system_io(), + ram_size, boot_device, + kernel_filename, kernel_cmdline, + initrd_filename, cpu_model, 1, 1, 0); } static void pc_init_pci_no_kvmclock(ram_addr_t ram_size, @@ -260,7 +276,7 @@ static void pc_init_pci_no_kvmclock(ram_addr_t ram_size, get_system_io(), ram_size, boot_device, kernel_filename, kernel_cmdline, - initrd_filename, cpu_model, 1, 0); + initrd_filename, cpu_model, 1, 0, 0); } static void pc_init_isa(ram_addr_t ram_size, @@ -276,7 +292,7 @@ static void pc_init_isa(ram_addr_t ram_size, get_system_io(), ram_size, boot_device, kernel_filename, kernel_cmdline, - initrd_filename, cpu_model, 0, 1); + initrd_filename, cpu_model, 0, 1, 0); } #ifdef CONFIG_XEN @@ -297,8 +313,8 @@ static void pc_xen_hvm_init(ram_addr_t ram_size, } #endif -static QEMUMachine pc_machine_v1_0 = { -.name = "pc-1.0", +static QEMUMachine pc_machine_v1_1 = { +.name = "pc-1.1", .alias = "pc", .desc = "Standard PC", .init = pc_init_pci, @@ -306,10 +322,17 @@ static QEMUMachine pc_machine_v1_0 = { .is_default = 1, }; +static QEMUMachine pc_machine_v1_0 = { +.name = "pc-1.0", +.desc = "Standard PC", +.init = pc_init_pci_system_rom_only, +.max_cpus = 255, +}; + static QEMUMachine pc_machine_v0_14 = { .name = "pc-0.14", .desc = "Standard PC", -.init = pc_init_pci, +.init = pc_init_pci_system_rom_only, .max_cpus = 255, .compat_props = (GlobalProperty[]) { { @@ -556,6 +579,7 @@ static QEMUMachine xenfv_machine = { static void pc_machine_init(void) { +
[Qemu-devel] [PATCH v9 3/3] pc: Support system flash memory with pflash
If a pflash image is found, then it is used for the system firmware image. If a pflash image is not initially found, then a read-only pflash device is created using the -bios filename. KVM cannot execute from a pflash region currently. Therefore, when KVM is enabled, a (read-only) ram memory region is created and filled with the contents of the pflash drive. Signed-off-by: Jordan Justen Cc: Anthony Liguori --- Makefile.target|1 + default-configs/i386-softmmu.mak |1 + default-configs/x86_64-softmmu.mak |1 + hw/boards.h|1 + hw/pc.c| 55 +--- hw/pc.h|4 + hw/pc_sysfw.c | 255 vl.c |2 +- 8 files changed, 269 insertions(+), 51 deletions(-) create mode 100644 hw/pc_sysfw.c diff --git a/Makefile.target b/Makefile.target index a111521..b1dc882 100644 --- a/Makefile.target +++ b/Makefile.target @@ -236,6 +236,7 @@ obj-i386-y += vmport.o obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o obj-i386-y += pc_piix.o +obj-i386-y += pc_sysfw.o obj-i386-$(CONFIG_KVM) += kvmclock.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index e67ebb3..cd407a9 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -22,3 +22,4 @@ CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y CONFIG_I8259=y +CONFIG_PFLASH_CFI01=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index b75757e..47734ea 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -22,3 +22,4 @@ CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y CONFIG_I8259=y +CONFIG_PFLASH_CFI01=y diff --git a/hw/boards.h b/hw/boards.h index 716fd7b..45a31a1 100644 --- a/hw/boards.h +++ b/hw/boards.h @@ -33,6 +33,7 @@ typedef struct QEMUMachine { } QEMUMachine; int qemu_register_machine(QEMUMachine *m); +QEMUMachine *find_default_machine(void); extern QEMUMachine *current_machine; diff --git a/hw/pc.c b/hw/pc.c index cc6cfad..e5550ca 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -57,10 +57,6 @@ #define DPRINTF(fmt, ...) #endif -#define BIOS_FILENAME "bios.bin" - -#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024) - /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables. */ #define ACPI_DATA_SIZE 0x1 #define BIOS_CFG_IOPORT 0x510 @@ -974,11 +970,9 @@ void pc_memory_init(MemoryRegion *system_memory, MemoryRegion **ram_memory, int system_firmware_enabled) { -char *filename; -int ret, linux_boot, i; -MemoryRegion *ram, *bios, *isa_bios, *option_rom_mr; +int linux_boot, i; +MemoryRegion *ram, *option_rom_mr; MemoryRegion *ram_below_4g, *ram_above_4g; -int bios_size, isa_bios_size; void *fw_cfg; linux_boot = (kernel_filename != NULL); @@ -1003,43 +997,9 @@ void pc_memory_init(MemoryRegion *system_memory, ram_above_4g); } -/* BIOS load */ -if (bios_name == NULL) -bios_name = BIOS_FILENAME; -filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); -if (filename) { -bios_size = get_image_size(filename); -} else { -bios_size = -1; -} -if (bios_size <= 0 || -(bios_size % 65536) != 0) { -goto bios_error; -} -bios = g_malloc(sizeof(*bios)); -memory_region_init_ram(bios, NULL, "pc.bios", bios_size); -memory_region_set_readonly(bios, true); -ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); -if (ret != 0) { -bios_error: -fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name); -exit(1); -} -if (filename) { -g_free(filename); -} -/* map the last 128KB of the BIOS in ISA space */ -isa_bios_size = bios_size; -if (isa_bios_size > (128 * 1024)) -isa_bios_size = 128 * 1024; -isa_bios = g_malloc(sizeof(*isa_bios)); -memory_region_init_alias(isa_bios, "isa-bios", bios, - bios_size - isa_bios_size, isa_bios_size); -memory_region_add_subregion_overlap(rom_memory, -0x10 - isa_bios_size, -isa_bios, -1); -memory_region_set_readonly(isa_bios, true); + +/* Initialize ROM or flash ranges for PC firmware */ +pc_system_firmware_init(rom_memory, system_firmware_enabled); option_rom_mr = g_malloc(sizeof(*option_rom_mr)); memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE); @@ -1048,11 +1008,6 @@ void pc_memory_init(MemoryRegion *system_memory, option_rom_mr,
[Qemu-devel] [PATCH v9 2/3] pflash: Support read-only mode
When read-only mode is enabled, no changes will be made to the flash image in memory, and no bdrv_write calls will be made. For pflash_cfi01 (Intel), if the flash is in read-only mode then the status register will signal block erase error or program error when these operations are attempted. For pflash_cfi02 (AMD), if the flash is in read-only mode then the pflash will silently ignore all write/erase commands. Signed-off-by: Jordan Justen --- blockdev.c|3 +- hw/pflash_cfi01.c | 44 +++- hw/pflash_cfi02.c | 83 3 files changed, 77 insertions(+), 53 deletions(-) diff --git a/blockdev.c b/blockdev.c index dbf0251..9096ae6 100644 --- a/blockdev.c +++ b/blockdev.c @@ -556,7 +556,8 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) /* CDROM is fine for any interface, don't check. */ ro = 1; } else if (ro == 1) { -if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && type != IF_NONE) { +if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && +type != IF_NONE && type != IF_PFLASH) { error_report("readonly not supported by this bus type"); goto err; } diff --git a/hw/pflash_cfi01.c b/hw/pflash_cfi01.c index 69b8e3d..1e0a053 100644 --- a/hw/pflash_cfi01.c +++ b/hw/pflash_cfi01.c @@ -283,8 +283,12 @@ static void pflash_write(pflash_t *pfl, target_phys_addr_t offset, TARGET_FMT_plx "\n", __func__, offset, pfl->sector_len); -memset(p + offset, 0xff, pfl->sector_len); -pflash_update(pfl, offset, pfl->sector_len); +if (!pfl->ro) { +memset(p + offset, 0xff, pfl->sector_len); +pflash_update(pfl, offset, pfl->sector_len); +} else { +pfl->status |= 0x20; /* Block erase error */ +} pfl->status |= 0x80; /* Ready! */ break; case 0x50: /* Clear status bits */ @@ -323,8 +327,12 @@ static void pflash_write(pflash_t *pfl, target_phys_addr_t offset, case 0x10: /* Single Byte Program */ case 0x40: /* Single Byte Program */ DPRINTF("%s: Single Byte Program\n", __func__); -pflash_data_write(pfl, offset, value, width, be); -pflash_update(pfl, offset, width); +if (!pfl->ro) { +pflash_data_write(pfl, offset, value, width, be); +pflash_update(pfl, offset, width); +} else { +pfl->status |= 0x10; /* Programming error */ +} pfl->status |= 0x80; /* Ready! */ pfl->wcycle = 0; break; @@ -372,7 +380,11 @@ static void pflash_write(pflash_t *pfl, target_phys_addr_t offset, case 2: switch (pfl->cmd) { case 0xe8: /* Block write */ -pflash_data_write(pfl, offset, value, width, be); +if (!pfl->ro) { +pflash_data_write(pfl, offset, value, width, be); +} else { +pfl->status |= 0x10; /* Programming error */ +} pfl->status |= 0x80; @@ -382,8 +394,12 @@ static void pflash_write(pflash_t *pfl, target_phys_addr_t offset, DPRINTF("%s: block write finished\n", __func__); pfl->wcycle++; -/* Flush the entire write buffer onto backing storage. */ -pflash_update(pfl, offset & mask, pfl->writeblock_size); +if (!pfl->ro) { +/* Flush the entire write buffer onto backing storage. */ +pflash_update(pfl, offset & mask, pfl->writeblock_size); +} else { +pfl->status |= 0x10; /* Programming error */ +} } pfl->counter--; @@ -605,13 +621,13 @@ pflash_t *pflash_cfi01_register(target_phys_addr_t base, } bdrv_attach_dev_nofail(pfl->bs, pfl); } -#if 0 /* XXX: there should be a bit to set up read-only, - * the same way the hardware does (with WP pin). - */ -pfl->ro = 1; -#else -pfl->ro = 0; -#endif + +if (pfl->bs) { +pfl->ro = bdrv_is_read_only(pfl->bs); +} else { +pfl->ro = 0; +} + pfl->timer = qemu_new_timer_ns(vm_clock, pflash_timer, pfl); pfl->base = base; pfl->sector_len = sector_len; diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c index e5a63da..9e91bdd 100644 --- a/hw/pflash_cfi02.c +++ b/hw/pflash_cfi02.c @@ -329,35 +329,37 @@ static void pflash_write (pflash_t *pfl, target_phys_addr_t offset, DPRINTF("%s: write data offset " TARGET_FMT_plx " %08x %d\n", __func__, offset, value, width); p = pfl->storage; -switch (width) { -case 1: -p[offset] &= value; -pflash_update(pfl, offset, 1); -
[Qemu-devel] [PATCH v9 0/3] PC system flash support
Enable flash emulation in a PC system using pflash_cfi01. v9: * Add pc-1.1 * pc-1.0 uses previous rom firmware init code path v8: * Cleanup two chunks of debug code (printf messages) * Fix comment in pc.h (pcflash.c => pc_sysfw.c) v7: * Do not add system firmware to qemu roms * If kvm is enabled, copy pflash drive contents into a read-only ram region, since kvm cannot currently execute code from a pflash device. * Rename pcflash.c to pc_sysfw.c v6: * Rebase for memory API * pflash_cfi01: Set error in status register when a write to erase is attempted in read-only mode. * Add system firmware to qemu roms v5: * Enable pflash read-only mode * Enable -drive with if=pflash to define system firmware image v4: * Rebase v3: * Fix code style issues * Add additional comments v2: * Convert debug printf to DPRINTF Jordan Justen (3): pc: Add pc-1.1 machine type pflash: Support read-only mode pc: Support system flash memory with pflash Makefile.target|1 + blockdev.c |3 +- default-configs/i386-softmmu.mak |1 + default-configs/x86_64-softmmu.mak |1 + hw/boards.h|1 + hw/pc.c| 58 +--- hw/pc.h|7 +- hw/pc_piix.c | 40 +- hw/pc_sysfw.c | 255 hw/pflash_cfi01.c | 44 -- hw/pflash_cfi02.c | 83 +++-- vl.c |2 +- 12 files changed, 382 insertions(+), 114 deletions(-) create mode 100644 hw/pc_sysfw.c
[Qemu-devel] [Bug 749522] Re: qemu-system-arm reads wrong entry in L1 page table for cortex-a8
"(We do seem to not quite be getting the effect of TTBCR.N right, though: if N > 0 then although we correctly take more bits from TTBR0 (by adjusting c2_base_mask) we aren't masking out the high bits [31..32-N] of the MVA. But that's a different problem.)" Looking more closely, I was wrong here. In the case where N>0 and we're using TTBR0 then we are guaranteed that [31..32-N] of the MVA are zero, because that is exactly the condition that controls using TTBR0 rather than TTBR1. So the code as it stands is correct. "Why do you think this is wrong?" Since the bug submitter never replied to this, and the code is as far as I can tell correct both in theory and in practice, I'm going to resolve this bug as invalid. ** Changed in: qemu Status: New => Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/749522 Title: qemu-system-arm reads wrong entry in L1 page table for cortex-a8 Status in QEMU: Invalid Bug description: target-arm/helper.c:920 [current] table |= (address >> 18) & 0x3ffc [fix] table |= (address >> 20) & 0xfff To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/749522/+subscriptions
[Qemu-devel] [Bug 889868] Re: CM_CTRL always reads as 0x00000000 (arm/integratorcp)
1.0 is out now, so let's call this bug fixed. ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/889868 Title: CM_CTRL always reads as 0x (arm/integratorcp) Status in QEMU: Fix Released Bug description: qemu -version: QEMU PC emulator version 0.12.5, Copyright (c) 2003-2008 Fabrice Bellard uname -a: Linux zenwalk 2.6.37.4 #1 SMP PREEMPT Fri Mar 18 18:17:50 CET 2011 i686 Intel(R) Pentium(R) Dual CPU T2330 @ 1.60GHz GenuineIntel GNU/Linux command-line: qemu-system-arm -M integratorcp -cpu arm926 -m 16 -kernel mykernel.elf -serial stdio steps to reproduce: --8< @ assembler @ after trivial set up for arm read_ccmr: ld r4,=0x1000 ld r0,[r4,#0x0C] -->8 ---8< ; in C int main(void) { unsigned int volatile *ccmr,test1,test2,value; ccmr = (unsigned int volatile *)0x100C; test1 = (*ccmr); // reads as zero (printk, printf,etc) value = 0x0001; // set GREEN LED (*ccmr) = value; // should set bit 0 in CM_CTL test2 = (*ccmr); // should return 1! if (test1 == test2) { // printk.printf,etc // if test2 == 1, this code // is not reached } } -->8- To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/889868/+subscriptions
Re: [Qemu-devel] [RFC] Plan for moving forward with QOM
On 12/15/2011 12:59 PM, Paul Brook wrote: Do we have a user interface issue here? I want to separate backwards compatibility from ABI compatibility. We should provide nice high level interfaces that are forever backwards compatible. But when it comes to hooking up IRQs between devices, that interface should just be ABI compatible, not necessarily backwards compatible. To achieve ABI compatibility, we just have to be strict about renaming types if they change significantly and introducing new field names instead of changing the semantics of existing fields. Relying on type to disambiguate between different links to an object while only allowing one of those types to be stateful make using a single object to implement logically distinct functionality (e.g. Device v.s. Bus, or different types of device interface) is a user visible implementation detail. But there are two distinct classes of user. One class of user really is thinking: "I want a KVM virtual machine, with 3 disks and 2 network cards" They could give a flying hoot whether the i440fx inherits from pcidevice or implements pcibus. We need to provide an obviously distinct UI and API for these users. The vast majority of command line users fall into this category. And once this user learns how do create a guest with 3 disks and 2 network cards, they should never have to learn another way of doing it. There is a second class of "user" that is doing very sophisticated things and cares about this information. But this sophisticated user (i.e. libvirt) wants to be able to probe QEMU to understand what features are available because it very likely is evolving just as quickly as QEMU is. For this user, it's more import to provide this introspection interface than it is to guarantee that we never add/remove devices and interfaces. We can be flexible with this class of user provided they have clear and obvious ways to figure out what we're doing. In practice we really do want to inherit state (which presumably includes properties) from multiple classes. I think before we can declare something "in practice", we have to actually experience it, in practice :-) Object oriented design is not an exact science. There is no Right Way to model things because all models are lossy in some way. We can debate the merits of MI verses SI w/interfaces literally for years but there's enough existence proof out there that with SI w/interfaces, you can build complex systems. I'd be amazed if we last many releases without breaking machine descriptions and/or the "qemu -device blah" because of this. I haven't worked out the details, maybe we need a "Self" property, plus a policy of never having user visible stuff link to an primary device node. If the primary object happens to implement/inherit from that Interface then it sets the property to itself. Otherwise it creates a stateful bus object (maybe using composition). You're advocating only connecting properties to properties, and never an object to a property? I think that's needlessly complicated. In the vast majority of cases, you just case about saying "connect this CharDriverState to this Serial device". We should make it much more complicated than that. This allows you to have i440fx implement PciBus directly when it's convenient, but the board description always attaches devices to ::i440x::pcibus. I think I'm starting to see now why Java code is often a twisty mess of interfaces and adaptors. Java is a pure OO language. There is no such thing as a free standing function. As a result, there are numerous things that are very complicated because things that you would naturally express as a function end up being expressed as an Adaptor class or something like that. Just about anything taken to it's logical extreme is bad.. I don't see how this can work without a closure object. We need a central device that is capable of recieving signals from many client devices. Those client devices don't know where they are, they just shout down a point-point link. I'd say this is a fairly common topology. If the central device implements that point-point interface directly then it has no idea which device is talking to it. We need to create child objects with a port ID property and implementing the p-p interface, then bind each client device to one of these child objects. The child objects can't do anything useful on their own, so need to proxy the signal to an interface on the main object, adding in their port id. If you aren't using inheritance, yes, you need to pass closures to the child objects. I dislike that kind of proxy modeling. How would you solve this using inheritance? I can see how it works when the device knows its address, but it seems kinda lame to tell a device "You have a dedicated communication channel. But I'm lazy and will smush them all together. Please add this additional token to every signal you send". Yes, adding a toke
[Qemu-devel] [Bug 696094] Re: TI Stellaris lm3s811evb (ARM Cortex-M3) : Systick interrupt not working
NB: the attached project fails for me like this: qemu: hardware error: gic_dist_writeb: Bad offset d23 CPU #0: R00= R01=e000ed00 R02=00e0 R03=e000ed0b R04= R05= R06= R07=24bb R08= R09= R10= R11= R12= R13=24bb R14=03bd R15=0338 PSR=8173 N--- T svc32 This is because we don't support byte wide accesses to the SHPR* registers. (The error message refers to the GIC because we currently map the whole of that area of address space as part of the GIC and then have it redirect some areas to code in arm7m_nvic.c. That should probably be cleaned up.) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/696094 Title: TI Stellaris lm3s811evb (ARM Cortex-M3) : Systick interrupt not working Status in QEMU: New Bug description: I've tried to create a small project that uses the CMSIS as base library. The problem is that the SysTick_interrupt_handler() doesn't get executed when the systick event is detected in QEMU. Furthermore, it seems asif QEMU gets stuck in an endless loop. QEMU doesn't respond to Ctrl-C on the command line and the GDB session also stalls. 'kill -9' is the only way to stop QEMU. It seems asif the initialisation of the NVIC works fine. I've traced the function calls in QEMU as follows: stellaris.c: stellaris_init() - Perform generic armv7 init: armv7m_init() armv7m.c: armv7m_init() - Create and init the nvic: nvic = qdev_create(NULL, "armv7m_nvic"); env->nvic = nvic; qdev_init_nofail(nvic); - Configure the programmable interrupt controller: Call: arm_pic_init_cpu() qemu_allocate_irqs(arm_pic_cpu_handler) - Initialise 64 interrupt structures. The following call sequence is observed when the systick event occur: armv7m_nvic.c: systick_timer_tick(): set pending interrupt armv7m_nvic.c: armv7m_nvic_set_pending() for irq:15 arm_gic.c: gic_set_pending_private(): GIC_SET_PENDING(15,) arm_gic.c: gic_update() - Raise IRQ with qemu_set_irq() irq.c: eqmu_set_irq() - Call the irq->handler -- I assume the irq handler is 'arm_pic_cpu_handler()', since that was passed as the parameter when qemu_allocate_irqs() was called in ... arm_pic.c: arm_pic_cpu_handler() - After evaluation, call cpu_interrupt() exec.c: cpu_interrupt() is called. The tools that were used during the testing of this project: GCC: Codesourcery ARM eabi 2010q3 QEMU: Checked out on 31/12/2010 - Last commit: 0fcec41eec0432c77645b4a407d3a3e030c4abc4 The project files are attached, for reproducing of the errors. Note: The CMSIS wants to perform byte accesses to the NVIC. For the Cortex-M3, unaligned 8 bit and 16 bit accesses are allowed. The current QEMU implementation doesn't yet cater for it. As a work around, updated versions of arm_gic.c armv7m_nvic.h armv7m_nvic.c is also included. Launch project with: go_gdb.sh Attach debugger with: arm-none-eabi-gdbtui --command=gdbCommands_tui (s = step, n = next, c = continue, Ctrl-C = stop, print to look at variable contents) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/696094/+subscriptions
Re: [Qemu-devel] [RFC] Plan for moving forward with QOM
> > Do we have a user interface issue here? > > I want to separate backwards compatibility from ABI compatibility. We > should provide nice high level interfaces that are forever backwards > compatible. But when it comes to hooking up IRQs between devices, that > interface should just be ABI compatible, not necessarily backwards > compatible. > > To achieve ABI compatibility, we just have to be strict about renaming > types if they change significantly and introducing new field names instead > of changing the semantics of existing fields. Relying on type to disambiguate between different links to an object while only allowing one of those types to be stateful make using a single object to implement logically distinct functionality (e.g. Device v.s. Bus, or different types of device interface) is a user visible implementation detail. In practice we really do want to inherit state (which presumably includes properties) from multiple classes. I'd be amazed if we last many releases without breaking machine descriptions and/or the "qemu -device blah" because of this. I haven't worked out the details, maybe we need a "Self" property, plus a policy of never having user visible stuff link to an primary device node. If the primary object happens to implement/inherit from that Interface then it sets the property to itself. Otherwise it creates a stateful bus object (maybe using composition). This allows you to have i440fx implement PciBus directly when it's convenient, but the board description always attaches devices to ::i440x::pcibus. I think I'm starting to see now why Java code is often a twisty mess of interfaces and adaptors. > >>> I don't see how this can work without a closure object. We need a > >>> central device that is capable of recieving signals from many client > >>> devices. Those client devices don't know where they are, they just > >>> shout down a point-point link. I'd say this is a fairly common > >>> topology. > >>> > >>> If the central device implements that point-point interface directly > >>> then it has no idea which device is talking to it. We need to create > >>> child objects with a port ID property and implementing the p-p > >>> interface, then bind each client device to one of these child objects. > >>> The child objects can't do anything useful on their own, so need to > >>> proxy the signal to an interface on the main object, adding in their > >>> port id. > >> > >> If you aren't using inheritance, yes, you need to pass closures to the > >> child objects. I dislike that kind of proxy modeling. > > > > How would you solve this using inheritance? > > > > I can see how it works when the device knows its address, but it seems > > kinda lame to tell a device "You have a dedicated communication channel. > > But I'm lazy and will smush them all together. Please add this > > additional token to every signal you send". > > Yes, adding a token is how you would have to do it. Ugh. Except it's worse than I thought. That token has to come from the user. Either via some arbitrary property on the client device, which is going to be different for every device especially when a device can link to multiple interfaces of the same class. Or we need some mechanism for attaching data to a link, rather than just conecting the two interfaces together. Neither of which sound desirable. There's also the issue that the token itsef is device specific. Identifying physical incarnations of logically independent interfaces is well outside the scome of the relevant bus, and varies from device to device. e.g. one interrupt controller mugh device to label its pins A-P, or 0-16, or 128-144, or "alice"/"bob". Paul
[Qemu-devel] [PATCH] stellaris: Calculate system clock period on reset
Calculate the system clock period on reset; otherwise it remains set to the default value of zero and attempting to use it provokes a hang. This is one of the issues noted in LP:696094. Signed-off-by: Peter Maydell --- hw/stellaris.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/stellaris.c b/hw/stellaris.c index ce62a98..7a73074 100644 --- a/hw/stellaris.c +++ b/hw/stellaris.c @@ -621,6 +621,7 @@ static void ssys_reset(void *opaque) s->rcgc[0] = 1; s->scgc[0] = 1; s->dcgc[0] = 1; +ssys_calculate_system_clock(s); } static int stellaris_sys_post_load(void *opaque, int version_id) -- 1.7.5.4
[Qemu-devel] [PATCH] Makefile.target: Remove unnecessary dependency rules
Remove some dependency rules which aren't necessary (the automatically generated .d files cover all these). These were leftovers from dyngen days, when the object files also had a dependency on some generated files. Signed-off-by: Peter Maydell --- Makefile.target |6 -- 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/Makefile.target b/Makefile.target index 8be9b9a..3261383 100644 --- a/Makefile.target +++ b/Makefile.target @@ -92,12 +92,6 @@ tci-dis.o: QEMU_CFLAGS += -I$(SRC_PATH)/tcg -I$(SRC_PATH)/tcg/tci $(libobj-y): $(GENERATED_HEADERS) -translate.o: translate.c cpu.h - -translate-all.o: translate-all.c cpu.h - -tcg/tcg.o: cpu.h - # HELPER_CFLAGS is used for all the code compiled with static register # variables op_helper.o ldst_helper.o user-exec.o: QEMU_CFLAGS += $(HELPER_CFLAGS) -- 1.7.5.4
Re: [Qemu-devel] [PATCH v5 4/4] Add support for net bridge
On 12/15/2011 10:26 AM, Anthony Liguori wrote: On 11/13/2011 09:45 PM, Corey Bryant wrote: The most common use of -net tap is to connect a tap device to a bridge. This requires the use of a script and running qemu as root in order to allocate a tap device to pass to the script. This patch breaks the build: anthony@titi:~/build/qemu$ make CC net/tap.o cc1: warnings being treated as errors /home/anthony/git/qemu/net/tap.c: In function ‘net_init_tap’: /home/anthony/git/qemu/net/tap.c:560:15: error: ‘s’ may be used uninitialized in this function make: *** [net/tap.o] Error 1 - s = net_tap_fd_init(vlan, "tap", name, fd, vnet_hdr); - if (!s) { - close(fd); - return -1; + s = net_tap_fd_init(vlan, "tap", name, fd, vnet_hdr); + if (!s) { + close(fd); + return -1; + } } And indeed, you've changed the function from unconditionally initializing s to conditionally initializing it. Specifically, you've broken -net tap,fd=X which would break tools like libvirt. Regards, Anthony Liguori That's a problem. I'll get another version out that fixes this. -- Regards, Corey
Re: [Qemu-devel] [PATCH] HACKING: clarify allocation/free recommendations
On 12/15/2011 07:33 AM, Peter Maydell wrote: Clarify the allocation/free recommendations; this is mostly just tidying up following the global-search-and-replace done with the conversion to the GLib g_malloc and friends. Signed-off-by: Peter Maydell Applied. Thanks. Regards, Anthony Liguori --- HACKING | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/HACKING b/HACKING index 733eab2..471cf1d 100644 --- a/HACKING +++ b/HACKING @@ -77,11 +77,13 @@ avoided. Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign APIs is not allowed in the QEMU codebase. Instead of these routines, -use the replacement g_malloc/g_malloc0/g_realloc/g_free or -qemu_vmalloc/qemu_memalign/qemu_vfree APIs. +use the GLib memory allocation routines g_malloc/g_malloc0/g_new/ +g_new0/g_realloc/g_free or QEMU's qemu_vmalloc/qemu_memalign/qemu_vfree +APIs. -Please note that NULL check for the g_malloc result is redundant and -that g_malloc() call with zero size is not allowed. +Please note that g_malloc will exit on allocation failure, so there +is no need to test for failure (as you would have to with malloc). +Calling g_malloc with a zero size is valid and will return NULL. Memory allocated by qemu_vmalloc or qemu_memalign must be freed with qemu_vfree, since breaking this will cause problems on Win32 and user
Re: [Qemu-devel] [PATCH] HACKING: clarify allocation/free recommendations
On 12/15/2011 07:33 AM, Peter Maydell wrote: Clarify the allocation/free recommendations; this is mostly just tidying up following the global-search-and-replace done with the conversion to the GLib g_malloc and friends. Signed-off-by: Peter Maydell Applied. Thanks. Regards, Anthony Liguori --- HACKING | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/HACKING b/HACKING index 733eab2..471cf1d 100644 --- a/HACKING +++ b/HACKING @@ -77,11 +77,13 @@ avoided. Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign APIs is not allowed in the QEMU codebase. Instead of these routines, -use the replacement g_malloc/g_malloc0/g_realloc/g_free or -qemu_vmalloc/qemu_memalign/qemu_vfree APIs. +use the GLib memory allocation routines g_malloc/g_malloc0/g_new/ +g_new0/g_realloc/g_free or QEMU's qemu_vmalloc/qemu_memalign/qemu_vfree +APIs. -Please note that NULL check for the g_malloc result is redundant and -that g_malloc() call with zero size is not allowed. +Please note that g_malloc will exit on allocation failure, so there +is no need to test for failure (as you would have to with malloc). +Calling g_malloc with a zero size is valid and will return NULL. Memory allocated by qemu_vmalloc or qemu_memalign must be freed with qemu_vfree, since breaking this will cause problems on Win32 and user
Re: [Qemu-devel] [PATCH] usb: fix usb_qdev_init() error handling again
On 12/15/2011 04:05 AM, Stefan Hajnoczi wrote: Commit f462141f18ffdd75847f6459ef83d90b831d12c0 introduced clean up code when usb_qdev_init() fails. Unfortunately it calls .handle_destroy() when .init() was never invoked or failed. This can lead to crashes when .handle_destroy() tries to clean up things that were never initialized. This patch is careful to undo only those steps that completed along the usb_qdev_init() code path. It's not as pretty as the unified error handling in f462141f18ffdd75847f6459ef83d90b831d12c0 but it's necessary. Signed-off-by: Stefan Hajnoczi Applied. Thanks. Regards, Anthony Liguori --- hw/usb-bus.c | 12 +--- 1 files changed, 5 insertions(+), 7 deletions(-) diff --git a/hw/usb-bus.c b/hw/usb-bus.c index 8cafb76..8203390 100644 --- a/hw/usb-bus.c +++ b/hw/usb-bus.c @@ -77,23 +77,21 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base) QLIST_INIT(&dev->strings); rc = usb_claim_port(dev); if (rc != 0) { -goto err; +return rc; } rc = dev->info->init(dev); if (rc != 0) { -goto err; +usb_release_port(dev); +return rc; } if (dev->auto_attach) { rc = usb_device_attach(dev); if (rc != 0) { -goto err; +usb_qdev_exit(qdev); +return rc; } } return 0; - -err: -usb_qdev_exit(qdev); -return rc; } static int usb_qdev_exit(DeviceState *qdev)
Re: [Qemu-devel] [PATCH] fix win32 build
On 12/13/2011 06:43 AM, Paolo Bonzini wrote: On Windows, cpus.c needs access to the hThread. Add a Windows-specific function to grab it. This requires changing the CPU threads to joinable. There is no substantial change because the threads run in an infinite loop. Signed-off-by: Paolo Bonzini Applied. Thanks. Regards, Anthony Liguori --- cpu-defs.h |9 + cpus.c | 11 +++ qemu-thread-win32.c | 29 +++-- qemu-thread-win32.h |3 +++ 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/cpu-defs.h b/cpu-defs.h index db48a7a..57a709b 100644 --- a/cpu-defs.h +++ b/cpu-defs.h @@ -153,6 +153,14 @@ typedef struct CPUWatchpoint { QTAILQ_ENTRY(CPUWatchpoint) entry; } CPUWatchpoint; +#ifdef _WIN32 +#define CPU_COMMON_THREAD \ +void *hThread; + +#else +#define CPU_COMMON_THREAD +#endif + #define CPU_TEMP_BUF_NLONGS 128 #define CPU_COMMON \ struct TranslationBlock *current_tb; /* currently executing TB */ \ @@ -211,6 +219,7 @@ typedef struct CPUWatchpoint { uint32_t stop; /* Stop request */ \ uint32_t stopped; /* Artificially stopped */\ struct QemuThread *thread; \ +CPU_COMMON_THREAD \ struct QemuCond *halt_cond; \ int thread_kicked; \ struct qemu_work_item *queued_work_first, *queued_work_last;\ diff --git a/cpus.c b/cpus.c index 1ada0f5..11893d5 100644 --- a/cpus.c +++ b/cpus.c @@ -793,9 +793,9 @@ static void qemu_cpu_kick_thread(CPUState *env) } #else /* _WIN32 */ if (!qemu_cpu_is_self(env)) { -SuspendThread(env->thread->thread); +SuspendThread(env->hThread); cpu_signal(0); -ResumeThread(env->thread->thread); +ResumeThread(env->hThread); } #endif } @@ -911,7 +911,10 @@ static void qemu_tcg_init_vcpu(void *_env) qemu_cond_init(env->halt_cond); tcg_halt_cond = env->halt_cond; qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env, - QEMU_THREAD_DETACHED); + QEMU_THREAD_JOINABLE); +#ifdef _WIN32 +env->hThread = qemu_thread_get_handle(env->thread); +#endif while (env->created == 0) { qemu_cond_wait(&qemu_cpu_cond,&qemu_global_mutex); } @@ -928,7 +931,7 @@ static void qemu_kvm_start_vcpu(CPUState *env) env->halt_cond = g_malloc0(sizeof(QemuCond)); qemu_cond_init(env->halt_cond); qemu_thread_create(env->thread, qemu_kvm_cpu_thread_fn, env, - QEMU_THREAD_DETACHED); + QEMU_THREAD_JOINABLE); while (env->created == 0) { qemu_cond_wait(&qemu_cpu_cond,&qemu_global_mutex); } diff --git a/qemu-thread-win32.c b/qemu-thread-win32.c index a13ffcc..fe9b931 100644 --- a/qemu-thread-win32.c +++ b/qemu-thread-win32.c @@ -252,14 +252,10 @@ void *qemu_thread_join(QemuThread *thread) * discard the handle that _beginthreadex gives back, and * get another copy of the handle here. */ -EnterCriticalSection(&data->cs); -if (!data->exited) { -handle = OpenThread(SYNCHRONIZE, FALSE, thread->tid); -LeaveCriticalSection(&data->cs); +handle = qemu_thread_get_handle(thread); +if (handle) { WaitForSingleObject(handle, INFINITE); CloseHandle(handle); -} else { -LeaveCriticalSection(&data->cs); } ret = data->ret; DeleteCriticalSection(&data->cs); @@ -308,6 +304,27 @@ void qemu_thread_get_self(QemuThread *thread) thread->tid = GetCurrentThreadId(); } +HANDLE qemu_thread_get_handle(QemuThread *thread) +{ +QemuThreadData *data; +HANDLE handle; + +data = thread->data; +if (!data) { +return NULL; +} + +EnterCriticalSection(&data->cs); +if (!data->exited) { +handle = OpenThread(SYNCHRONIZE | THREAD_SUSPEND_RESUME, FALSE, +thread->tid); +} else { +handle = NULL; +} +LeaveCriticalSection(&data->cs); +return handle; +} + int qemu_thread_is_self(QemuThread *thread) { return GetCurrentThreadId() == thread->tid; diff --git a/qemu-thread-win32.h b/qemu-thread-win32.h index 2983490..b9d1be8 100644 --- a/qemu-thread-win32.h +++ b/qemu-thread-win32.h @@ -19,4 +19,7 @@ struct QemuThread { unsigned tid; }; +/* Only valid for joinable threads. */ +HANDLE qemu_thread_get_handle(QemuThread *thread); + #endif
Re: [Qemu-devel] [PATCH] phys_page_find_alloc: Use correct initial region_offset.
On 12/13/2011 04:52 AM, alex_rozen...@mentor.com wrote: From: Alex Rozenman This fixes a common bug with initial region_offset value. Usually, the pages are re-assigned afterwards, so the bug has a very small effect on regular QEMU use flows. Signed-off-by: Alex Rozenman Applied. Thanks. Regards, Anthony Liguori --- exec.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/exec.c b/exec.c index d8b2180..14628d9 100644 --- a/exec.c +++ b/exec.c @@ -418,6 +418,7 @@ static PhysPageDesc *phys_page_find_alloc(target_phys_addr_t index, int alloc) pd = *lp; if (pd == NULL) { int i; +int first_index = index& ~(L2_SIZE - 1); if (!alloc) { return NULL; @@ -427,7 +428,7 @@ static PhysPageDesc *phys_page_find_alloc(target_phys_addr_t index, int alloc) for (i = 0; i< L2_SIZE; i++) { pd[i].phys_offset = IO_MEM_UNASSIGNED; -pd[i].region_offset = (index + i)<< TARGET_PAGE_BITS; +pd[i].region_offset = (first_index + i)<< TARGET_PAGE_BITS; } }
Re: [Qemu-devel] [PATCH 1/2] error: Add an accessor for progname
On 12/12/2011 07:22 PM, mich...@ellerman.id.au wrote: We'd like to get the progname for help output, so add an accessor. Signed-off-by: Michael Ellerman Applied. Thanks. Regards, Anthony Liguori --- qemu-error.c |5 + qemu-error.h |1 + 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/qemu-error.c b/qemu-error.c index 4b20d28..7cd5ffe 100644 --- a/qemu-error.c +++ b/qemu-error.c @@ -157,6 +157,11 @@ void error_set_progname(const char *argv0) progname = p ? p + 1 : argv0; } +const char *error_get_progname(void) +{ +return progname; +} + /* * Print current location to current monitor if we have one, else to stderr. */ diff --git a/qemu-error.h b/qemu-error.h index 4d5c537..93d74b4 100644 --- a/qemu-error.h +++ b/qemu-error.h @@ -36,5 +36,6 @@ void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2); void error_print_loc(void); void error_set_progname(const char *argv0); void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); +const char *error_get_progname(void); #endif
Re: [Qemu-devel] [PATCH v3 00/20] qom: dynamic properties and composition tree
On 12/12/2011 02:29 PM, Anthony Liguori wrote: This is a follow up to my previous series to get us started in the QOM direction. A few things are different this time around. Most notably: 1) Devices no longer have names. Instead, path names are always used to identify devices. 2) In order to support (1), dynamic properties are now supported. 3) The concept of a "root device" has been introduced. The root device is roughly modelling the motherboard of a machine. This type is a container type and it's best to think of it as something like a PCB board I guess. See my other mail for a description of my merge plan for QOM. Applied. Regards, Anthony Liguori To try it out, here's an example session: Launch: anthony@titi:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -hda ~/images/linux.img -snapshot -device virtio-balloon-pci,id=foo -qmp unix:/tmp/server.sock,server,nowait Explore the object model: anthony@titi:~/git/qemu/QMP$ ./qom-list / peripheral/ i440fx/ anthony@titi:~/git/qemu/QMP$ ./qom-list /i440fx/ piix3/ anthony@titi:~/git/qemu/QMP$ ./qom-list /i440fx/piix3 rtc/ anthony@titi:~/git/qemu/QMP$ ./qom-list /i440fx/piix3/rtc date base_year anthony@titi:~/git/qemu/QMP$ ./qom-get /i440fx/piix3/rtc.date tm_sec: 33 tm_hour: 21 tm_mday: 30 tm_year: 111 tm_mon: 10 tm_min: 2 anthony@titi:~/git/qemu/QMP$ ./qom-get rtc.date tm_sec: 38 tm_hour: 21 tm_mday: 30 tm_year: 111 tm_mon: 10 tm_min: 2 anthony@titi:~/git/qemu/QMP$ ./qom-list /peripheral foo/ anthony@titi:~/git/qemu/QMP$ ./qom-list /peripheral/foo event_idx indirect_desc Anthony Liguori (20): qom: add a reference count to qdev objects qom: add new dynamic property infrastructure based on Visitors (v2) qom: register legacy properties as new style properties (v2) qom: introduce root device qdev: provide an interface to return canonical path from root (v2) qdev: provide a path resolution (v2) qom: add child properties (composition) (v3) qom: add link properties (v2) qapi: allow a 'gen' key to suppress code generation qmp: add qom-list command qom: qom_{get,set} monitor commands (v2) qdev: add explicitly named devices to the root complex dev: add an anonymous peripheral container rtc: make piix3 set the rtc as a child (v2) rtc: add a dynamic property for retrieving the date qom: optimize qdev_get_canonical_path using a parent link qom: add vga node to the pc composition tree qom: add string property type qdev: add a qdev_get_type() function and expose as a 'type' property pc: fill out most of the composition tree Makefile.objs|2 +- hw/acpi_piix4.c |7 +- hw/cirrus_vga.c |8 +- hw/container.c | 20 ++ hw/ide/pci.c | 11 +- hw/kvmclock.c|5 +- hw/kvmclock.h|5 +- hw/mc146818rtc.c | 27 +++ hw/mips_malta.c |5 +- hw/pc.c | 75 +-- hw/pc.h | 51 +++-- hw/pc_piix.c | 61 -- hw/piix_pci.c| 11 +- hw/qdev.c| 555 +- hw/qdev.h| 281 +++ hw/usb-uhci.c|4 +- hw/usb-uhci.h|2 +- hw/vga-pci.c |5 +- hw/vmware_vga.h |6 +- monitor.h|4 + qapi-schema.json | 107 + qerror.c |4 + qerror.h |3 + qmp-commands.hx | 18 ++ qmp.c| 93 scripts/qapi-commands.py |1 + scripts/qapi-types.py|1 + 27 files changed, 1289 insertions(+), 83 deletions(-) create mode 100644 hw/container.c
Re: [Qemu-devel] [PATCH v3] network scripts: don't block SIGCHLD before forking
On 12/07/2011 09:48 PM, Michael Roth wrote: This patch fixes a bug where child processes of launch_script() can misbehave due to SIGCHLD being blocked. In the case of `sudo`, this causes a permanent hang. Previously a SIGCHLD handler was added to reap fork_exec()'d zombie processes by calling waitpid(-1, ...). This required other fork()/waitpid() callers to temporarilly block SIGCHILD to avoid having the final wait status being intercepted by the SIGCHLD handler: 7c3370d4fe3fa6cda8655f109e4659afc8ca4269 Since then, the qemu_add_child_watch() interface was added to allow registration of such processes and reap only from that specific set of PIDs: 4d54ec7898bd951007cb6122d5315584bd41d0c4 As a result, we can now avoid blocking SIGCHLD in launch_script(), so drop that behavior. Signed-off-by: Michael Roth Applied. Thanks. Regards, Anthony Liguori --- net/tap.c |6 -- 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/net/tap.c b/net/tap.c index 1f26dc9..6c27a94 100644 --- a/net/tap.c +++ b/net/tap.c @@ -346,15 +346,10 @@ static TAPState *net_tap_fd_init(VLANState *vlan, static int launch_script(const char *setup_script, const char *ifname, int fd) { -sigset_t oldmask, mask; int pid, status; char *args[3]; char **parg; -sigemptyset(&mask); -sigaddset(&mask, SIGCHLD); -sigprocmask(SIG_BLOCK,&mask,&oldmask); - /* try to launch network script */ pid = fork(); if (pid == 0) { @@ -378,7 +373,6 @@ static int launch_script(const char *setup_script, const char *ifname, int fd) while (waitpid(pid,&status, 0) != pid) { /* loop */ } -sigprocmask(SIG_SETMASK,&oldmask, NULL); if (WIFEXITED(status)&& WEXITSTATUS(status) == 0) { return 0;
Re: [Qemu-devel] [PATCH v2] exec.c: Fix subpage memory access to RAM MemoryRegion
On 11/30/2011 09:26 AM, Andreas Färber wrote: Commit 95c318f5e1f88d7e5bcc6deac17330fd4806a2d3 (Fix segfault in mmio subpage handling code.) prevented a segfault by making all subpage registrations over an existing memory page perform an unassigned access. Symptoms were writes not taking effect and reads returning zero. Very small page sizes are not currently supported either, so subpage memory areas cannot fully be avoided. Therefore change the previous fix to use a new IO_MEM_SUBPAGE_RAM instead of IO_MEM_UNASSIGNED. Suggested by Avi. Signed-off-by: Andreas Färber Cc: Avi Kivity Cc: Gleb Natapov Applied. Thanks. Regards, Anthony Liguori --- cpu-common.h |1 + exec.c | 65 - 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/cpu-common.h b/cpu-common.h index c9878ba..3f45428 100644 --- a/cpu-common.h +++ b/cpu-common.h @@ -172,6 +172,7 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr, #define IO_MEM_ROM (1<< IO_MEM_SHIFT) /* hardcoded offset */ #define IO_MEM_UNASSIGNED (2<< IO_MEM_SHIFT) #define IO_MEM_NOTDIRTY(3<< IO_MEM_SHIFT) +#define IO_MEM_SUBPAGE_RAM (4<< IO_MEM_SHIFT) /* Acts like a ROM when read and like a device when written. */ #define IO_MEM_ROMD(1) diff --git a/exec.c b/exec.c index 6b92198..6c206ff 100644 --- a/exec.c +++ b/exec.c @@ -3570,6 +3570,63 @@ static CPUWriteMemoryFunc * const subpage_write[] = { &subpage_writel, }; +static uint32_t subpage_ram_readb(void *opaque, target_phys_addr_t addr) +{ +ram_addr_t raddr = addr; +void *ptr = qemu_get_ram_ptr(raddr); +return ldub_p(ptr); +} + +static void subpage_ram_writeb(void *opaque, target_phys_addr_t addr, + uint32_t value) +{ +ram_addr_t raddr = addr; +void *ptr = qemu_get_ram_ptr(raddr); +stb_p(ptr, value); +} + +static uint32_t subpage_ram_readw(void *opaque, target_phys_addr_t addr) +{ +ram_addr_t raddr = addr; +void *ptr = qemu_get_ram_ptr(raddr); +return lduw_p(ptr); +} + +static void subpage_ram_writew(void *opaque, target_phys_addr_t addr, + uint32_t value) +{ +ram_addr_t raddr = addr; +void *ptr = qemu_get_ram_ptr(raddr); +stw_p(ptr, value); +} + +static uint32_t subpage_ram_readl(void *opaque, target_phys_addr_t addr) +{ +ram_addr_t raddr = addr; +void *ptr = qemu_get_ram_ptr(raddr); +return ldl_p(ptr); +} + +static void subpage_ram_writel(void *opaque, target_phys_addr_t addr, + uint32_t value) +{ +ram_addr_t raddr = addr; +void *ptr = qemu_get_ram_ptr(raddr); +stl_p(ptr, value); +} + +static CPUReadMemoryFunc * const subpage_ram_read[] = { +&subpage_ram_readb, +&subpage_ram_readw, +&subpage_ram_readl, +}; + +static CPUWriteMemoryFunc * const subpage_ram_write[] = { +&subpage_ram_writeb, +&subpage_ram_writew, +&subpage_ram_writel, +}; + static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end, ram_addr_t memory, ram_addr_t region_offset) { @@ -3583,8 +3640,9 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end, printf("%s: %p start %08x end %08x idx %08x eidx %08x mem %ld\n", __func__, mmio, start, end, idx, eidx, memory); #endif -if ((memory& ~TARGET_PAGE_MASK) == IO_MEM_RAM) -memory = IO_MEM_UNASSIGNED; +if ((memory& ~TARGET_PAGE_MASK) == IO_MEM_RAM) { +memory = IO_MEM_SUBPAGE_RAM; +} memory = (memory>> IO_MEM_SHIFT)& (IO_MEM_NB_ENTRIES - 1); for (; idx<= eidx; idx++) { mmio->sub_io_index[idx] = memory; @@ -3817,6 +3875,9 @@ static void io_mem_init(void) cpu_register_io_memory_fixed(IO_MEM_NOTDIRTY, error_mem_read, notdirty_mem_write, NULL, DEVICE_NATIVE_ENDIAN); +cpu_register_io_memory_fixed(IO_MEM_SUBPAGE_RAM, subpage_ram_read, + subpage_ram_write, NULL, + DEVICE_NATIVE_ENDIAN); for (i=0; i<5; i++) io_mem_used[i] = 1;
Re: [Qemu-devel] [RFC] Device isolation infrastructure v2
On Thu, 2011-12-15 at 17:25 +1100, David Gibson wrote: > Here's the second spin of my preferred approach to handling grouping > of devices for safe assignment to guests. > > Changes since v1: > * Many name changes and file moves for improved consistency > * Bugfixes and cleanups > * The interface to the next layer up is considerably fleshed out, >although it still needs work. > * Example initialization of groups for p5ioc2 and p7ioc. > > TODO: > * Need sample initialization of groups for intel and/or amd iommus I think this very well might imposed significant bloat for those implementations. On POWER you typically don't have singleton groups, while it's the norm on x86. I don't know that either intel or amd iommu code have existing structures that they can simply tack the group pointer to. Again, this is one of the reasons that I think the current vfio implementation is the right starting point. We keep groups within vfio, imposing zero overhead for systems not making use of it and only require iommu drivers to implement a trivial function to opt-in. As we start to make groups more pervasive in the dma layer, independent of userspace driver exposure, we can offload pieces to the core. Starting with it in the core and hand waving some future use that we don't plan to implement right now seems like the wrong direction. > * Use of sysfs attributes to control group permission is probably a >mistake. Although it seems a bit odd, registering a chardev for >each group is probably better, because perms can be set from udev >rules, just like everything else. I agree, this is a horrible mistake. Reinventing file permissions via sysfs is bound to be broken and doesn't account for selinux permissions. Again, I know you don't like aspects of the vfio group management, but it gets this right imho. > * Need more details of what the binder structure will need to >contain. > * Handle complete removal of groups. > * Clarify what will need to happen on the hot unplug path. We're still also removing devices from the driver model, this means drivers like vfio need to re-implement a lot of the driver core for driving each individual device in the group, and as you indicate, it's unclear what happens in the hotplug path and I wonder if things like suspend/resume will also require non-standard support. I really prefer attaching individual bus drivers to devices using the standard bind/unbind mechanisms. I have a hard time seeing how this is an improvement from vfio. Thanks, Alex
Re: [Qemu-devel] [Xen-devel] [PATCH] xen-qemu: register virtual iommu device on qemu pci bus
Wei Wang2 writes ("[Xen-devel] [PATCH] xen-qemu: register virtual iommu device on qemu pci bus"): > Attached patch is for qemu to register iommu device on pci bus. Guest OS > requires this to access iommu pci config space in some cases. Thanks for this submission. However, we are trying to focus development on the new xen-supporting upstream qemu branches. This old qemu branch is basically dead. We will have to give it bugfixes and security fixes, it in parallel with the new trees, for a long time (because old guests may need it). So for that reason I would really prefer not to add new features to it. Would you be able to rebase your work on the upstream qemu ? You will need Anthony Perard's PCI passthrough series of course, and you will also need to liase with qemu upstream (so I've CC'd their list) since it will be their tree that you'll be targeting. If there is a particular reason why this work should be in qemu-xen-unstable then please do say. I'm open to being persuaded. Thanks, Ian.
Re: [Qemu-devel] [Bug 899140] Re: Problem with Linux Kernel Traffic Control
Here is the problem ! The Ubuntu version works only because it not uses an *Intel e1000* but a *rtl8139*. Therefore, the problem about the e1000 is present in *all* version (original or ubuntu ones). Thus, the file *e1000.c* must contain some instructions which imply the bad TC behavior. Vincent Le 15/12/2011 17:09, Stefan Hajnoczi a écrit : > On Thu, Dec 15, 2011 at 3:03 PM, Vincent Autefage > <899...@bugs.launchpad.net> wrote: >> Ok, >> >> So the e1000.c and the e1000_hw.h have absolutely no difference between >> the original and the ubuntu version... >> The only differences witch refers to the *Intel e1000* in the wall >> sources is this one : >> >> >> diff -ru qemu//hw/pc_piix.c qemu-kvm-0.14.0+noroms//hw/pc_piix.c >> --- qemu//hw/pc_piix.c 2011-12-15 15:37:28.53929 +0100 >> +++ qemu-kvm-0.14.0+noroms//hw/pc_piix.c2011-02-22 >> 14:34:38.0 +0100 >> >> @@ -123,7 +141,7 @@ >> if (!pci_enabled || (nd->model&& strcmp(nd->model, >> "ne2k_isa") == 0)) >> pc_init_ne2k_isa(nd); >> else >> -pci_nic_init_nofail(nd, "e1000", NULL); >> +pci_nic_init_nofail(nd, "rtl8139", NULL); >> } >> >> if (drive_get_max_bus(IF_IDE)>= MAX_IDE_BUS) { > That looks like it is only changing the default NIC from e1000 to > rtl8139. > > Stefan > -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/899140 Title: Problem with Linux Kernel Traffic Control Status in QEMU: New Bug description: Hi, The last main versions of QEMU (0.14.1, 0.15 and 1.0) have an important problem when running on a Linux distribution which running itself a Traffic Control (TC) instance. Indeed, when TC is configured with a Token Bucket Filter (TBF) with a particular rate, the effective rate is very slower than the desired one. For instance, lets consider the following configuration : # tc qdisc add dev eth0 root tbf rate 20mbit burst 20k latency 50ms The effective rate will be about 100kbit/s ! (verified with iperf) I've encountered this problem on versions 0.14.1, 0.15 and 1.0 but not with the 0.14.0... In the 0.14.0, we have a rate of 19.2 mbit/s which is quiet normal. I've done the experimentation on several hosts : - Debian 32bit core i7, 4GB RAM - Debian 64bit core i7, 8GB RAM - 3 different high performance servers : Ubuntu 64 bits, 48 AMD Opteron, 128GB of RAM The problem is always the same... The problem is also seen with a Class Based Queuing (CBQ) in TC. Thanks To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/899140/+subscriptions
Re: [Qemu-devel] [PATCH v3] qemu-ga: Add the guest-suspend command
On 12/15/2011 09:09 AM, Luiz Capitulino wrote: It supports three modes: "hibernate" (suspend to disk), "sleep" (suspend to ram) and "hybrid" (save RAM contents to disk, but suspend instead of powering off). The command will try to execute the scripts provided by the pm-utils package. If that fails, it will try to suspend manually by writing to the "/sys/power/state" file. To reap terminated children, a new signal handler is installed to catch SIGCHLD signals and a non-blocking call to waitpid() is done to collect their exit statuses. Spotted a trailing whitespace in one of the comments, but patch looks good. Not sure what to expect right now with regarding to testing though... I'm aware of the issue with hibernate and virtio S4 support, but what's the expected behavior for sleep? I'd seen Dor mention that QEMU currently wakes the guest back up immediately after, and that some pause/cont logic would need to be added to address that, but for me (Fedora 15 x86_64), the guest appears to stay asleep. I guess it depends on what devices are currently whitelisted in /proc/acpi/wakeup? On Fedora I don't have anything: [mdroth@vm qemu-build2]$ cat /proc/acpi/wakeup Device S-state Status Sysfs node [mdroth@vm qemu-build2]$ Which I guess explains why I don't wake up immediately after, but I guess it also means there's no way to wake up, ever? On my Ubuntu host I have: mdroth@illuin:~$ cat /proc/acpi/wakeup Device S-state Status Sysfs node LID S3*enabled SLPB S3*enabled IGBE S4*enabled pci::00:19.0 EXP0 S4*disabled pci::00:1c.0 EXP1 S4*disabled pci::00:1c.1 EXP2 S4*disabled pci::00:1c.2 EXP3 S4*disabled pci::00:1c.3 EXP4 S4*disabled pci::00:1c.4 PCI1 S4*disabled pci::00:1e.0 USB0 S3*disabled pci::00:1d.0 USB1 S3*disabled pci::00:1d.1 USB2 S3*disabled pci::00:1d.2 USB3 S3*disabled pci::00:1a.0 USB4 S3*disabled pci::00:1a.1 EHC0 S3*disabled pci::00:1d.7 EHC1 S3*disabled pci::00:1a.7 HDEF S4*disabled pci::00:1b.0 DURT S3*disabled pnp:00:0a mdroth@illuin:~$ Are we lacking an equivalent to SLPB right now? Or is that list incomplete and there's some other mechanism available? Signed-off-by: Luiz Capitulino --- v3 o Add 'hybrid' mode o Use execlp() instead of execl() o Don't ignore SIGCHLD, catch it instead and call waitpid() from the signal handler to avoid zombies o Improve the schema doc qapi-schema-guest.json | 26 ++ qemu-ga.c | 17 +++- qga/guest-agent-commands.c | 64 3 files changed, 106 insertions(+), 1 deletions(-) diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json index 5f8a18d..6b9657a 100644 --- a/qapi-schema-guest.json +++ b/qapi-schema-guest.json @@ -219,3 +219,29 @@ ## { 'command': 'guest-fsfreeze-thaw', 'returns': 'int' } + +## +# @guest-suspend +# +# Suspend guest execution by entering ACPI power state S3 or S4. +# +# This command tries to execute the scripts provided by the pm-utils +# package. If they are not available, it will perform the suspend +# operation by manually writing to a sysfs file. +# +# For the best results it's strongly recommended to have the pm-utils +# package installed in the guest. +# +# @mode: 'hibernate' RAM content is saved to the disk and the guest is +#powered off (this corresponds to ACPI S4) +#'sleep' execution is suspended but the RAM retains its contents +#(this corresponds to ACPI S3) +#'hybrid'RAM content is saved to the disk but the guest is +#suspended instead of powering off +# +# Notes: This is an asynchronous request. There's no guarantee a response +# will be sent. Errors will be logged to guest's syslog. +# +# Since: 1.1 +## +{ 'command': 'guest-suspend', 'data': { 'mode': 'str' } } diff --git a/qemu-ga.c b/qemu-ga.c index 60d4972..a7c5973 100644 --- a/qemu-ga.c +++ b/qemu-ga.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "qemu_socket.h" #include "json-streamer.h" #include "json-parser.h" @@ -59,9 +60,15 @@ static void quit_handler(int sig) } } +static void child_handler(int sig) +{ +int status; +waitpid(-1,&status, WNOHANG); +} + static void register_signal_handlers(void) { -struct sigaction sigact; +struct sigaction sigact, sigact_chld; int ret; memset(&sigact, 0, sizeof(struct sigaction)); @@ -76,6 +83,14 @@ static void register_signal_handlers(void) if (ret == -1) { g_error("error configuring signal handler: %s", strerror(errno)); } + +memset(&sigact_chld, 0, sizeof(struct sigaction)); +sigact_chld.sa_handler = child_handler; +sigact_chld.sa_flags = SA_NOCLDSTOP; +ret = sigac
Re: [Qemu-devel] [PATCH] usb-ohci: return USBBus in usb_ohci_init_pci
On 12/15/2011 10:36 AM, Gerd Hoffmann wrote: Hi, Heh, awesome :-) Okay, I'll actually test it next time. Could you also add some justification for the change to the commit message please? Just for itself the change looks somewhat odd as there is no obvious reason for it ... Yup. Regards, Anthony Liguori thanks, Gerd
Re: [Qemu-devel] [PATCH] usb-ohci: return USBBus in usb_ohci_init_pci
Hi, > Heh, awesome :-) Okay, I'll actually test it next time. Could you also add some justification for the change to the commit message please? Just for itself the change looks somewhat odd as there is no obvious reason for it ... thanks, Gerd
Re: [Qemu-devel] [PATCH V2 3/5] Introduce premigrate RunState.
On Thu, 15 Dec 2011 09:14:00 -0600 Anthony Liguori wrote: > On 12/09/2011 03:54 PM, Anthony PERARD wrote: > > This new state will be used by Xen functions to know QEMU will wait for a > > migration. This is important to know for memory related function because the > > memory is already allocated and reallocated them will not works. How is premigrate different from inmigrate? It looks like the same thing to me. > > > > Signed-off-by: Anthony PERARD > > Luiz, please Ack. In the future, when you make QMP changes, please CC the > appropriate maintainer. I should improve my filter too. > > Regards, > > Anthony Liguori > > > --- > > qapi-schema.json |2 +- > > vl.c |4 > > 2 files changed, 5 insertions(+), 1 deletions(-) > > > > diff --git a/qapi-schema.json b/qapi-schema.json > > index cb1ba77..bd77444 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -121,7 +121,7 @@ > > { 'enum': 'RunState', > > 'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', > > 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', > > -'running', 'save-vm', 'shutdown', 'watchdog' ] } > > +'running', 'save-vm', 'shutdown', 'watchdog', 'premigrate' ] } > > > > ## > > # @StatusInfo: > > diff --git a/vl.c b/vl.c > > index e7dced2..a291416 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -351,8 +351,11 @@ static const RunStateTransition > > runstate_transitions_def[] = { > > > > { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING }, > > { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE }, > > +{ RUN_STATE_PRELAUNCH, RUN_STATE_PREMIGRATE }, > > { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE }, > > > > +{ RUN_STATE_PREMIGRATE, RUN_STATE_INMIGRATE }, > > + > > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING }, > > { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE }, > > > > @@ -2975,6 +2978,7 @@ int main(int argc, char **argv, char **envp) > > break; > > case QEMU_OPTION_incoming: > > incoming = optarg; > > +runstate_set(RUN_STATE_PREMIGRATE); > > break; > > case QEMU_OPTION_nodefaults: > > default_serial = 0; >
Re: [Qemu-devel] [PATCH v3 2/9] block: add BlockJob interface for long-running operations
On Wed, Dec 14, 2011 at 02:51:47PM +0100, Kevin Wolf wrote: > Am 13.12.2011 14:52, schrieb Stefan Hajnoczi: > > Signed-off-by: Stefan Hajnoczi > > --- > > block_int.h | 83 > > +++ > > 1 files changed, 83 insertions(+), 0 deletions(-) > > Why are these functions static inline in the header instead of being > implemented in block.c? > > The other thing I was going to ask, but don't really know to which patch > I should reply, is whether we need to take care of bdrv_close/delete > while a block job is still running. What happens if you unplug a device > that is being streamed? There is an additional reference, its not deleted until the operation completes. Or at least it should be like that, Stefan?
Re: [Qemu-devel] [PATCH v2 0/3] remove sysbus_init_mmio_cb2 usage
On 12/14/2011 08:32 AM, Benoît Canet wrote: These patches remove sysbus_init_mmio_cb2 usage from the codebase. Breaks the build: anthony@titi:~/build/qemu$ make CCppc-softmmu/ppce500_mpc8544ds.o cc1: warnings being treated as errors /home/anthony/git/qemu/hw/ppce500_mpc8544ds.c: In function ‘mpc8544ds_init’: /home/anthony/git/qemu/hw/ppce500_mpc8544ds.c:247:19: error: unused variable ‘busdev’ make[1]: *** [ppce500_mpc8544ds.o] Error 1 make: *** [subdir-ppc-softmmu] Error 2 Regards, Anthony Liguori v2: dont't change ppce500 initialisation method (peter) Benoît Canet (3): sh_pci: remove sysbus_init_mmio_cb2 usage ppce500_pci: remove sysbus_init_mmio_cb2 usage sysbus: remove sysbus_init_mmio_cb2 hw/ppce500_mpc8544ds.c |1 + hw/ppce500_pci.c | 27 ++- hw/r2d.c | 14 -- hw/sh_pci.c| 29 - hw/sysbus.c| 16 hw/sysbus.h|5 - 6 files changed, 23 insertions(+), 69 deletions(-)
Re: [Qemu-devel] [Bug 899140] Re: Problem with Linux Kernel Traffic Control
On Thu, Dec 15, 2011 at 4:09 PM, Stefan Hajnoczi wrote: > On Thu, Dec 15, 2011 at 3:03 PM, Vincent Autefage > <899...@bugs.launchpad.net> wrote: >> Ok, >> >> So the e1000.c and the e1000_hw.h have absolutely no difference between >> the original and the ubuntu version... >> The only differences witch refers to the *Intel e1000* in the wall >> sources is this one : >> >> >> diff -ru qemu//hw/pc_piix.c qemu-kvm-0.14.0+noroms//hw/pc_piix.c >> --- qemu//hw/pc_piix.c 2011-12-15 15:37:28.53929 +0100 >> +++ qemu-kvm-0.14.0+noroms//hw/pc_piix.c 2011-02-22 >> 14:34:38.0 +0100 >> >> @@ -123,7 +141,7 @@ >> if (!pci_enabled || (nd->model && strcmp(nd->model, >> "ne2k_isa") == 0)) >> pc_init_ne2k_isa(nd); >> else >> - pci_nic_init_nofail(nd, "e1000", NULL); >> + pci_nic_init_nofail(nd, "rtl8139", NULL); >> } >> >> if (drive_get_max_bus(IF_IDE) >= MAX_IDE_BUS) { > > That looks like it is only changing the default NIC from e1000 to rtl8139. Perhaps you can stop Ubuntu from applying its patches on top of vanilla QEMU but still use the same build process. In other words, try building the vanilla QEMU source but using Ubuntu's method (not sure if you are using dpkg build tools here). If it turns out the binary does not have the bug then we know it's an environmental issue like a ./configure difference or similar. Stefan
Re: [Qemu-devel] [Bug 899140] Re: Problem with Linux Kernel Traffic Control
On Thu, Dec 15, 2011 at 3:03 PM, Vincent Autefage <899...@bugs.launchpad.net> wrote: > Ok, > > So the e1000.c and the e1000_hw.h have absolutely no difference between > the original and the ubuntu version... > The only differences witch refers to the *Intel e1000* in the wall > sources is this one : > > > diff -ru qemu//hw/pc_piix.c qemu-kvm-0.14.0+noroms//hw/pc_piix.c > --- qemu//hw/pc_piix.c 2011-12-15 15:37:28.53929 +0100 > +++ qemu-kvm-0.14.0+noroms//hw/pc_piix.c 2011-02-22 > 14:34:38.0 +0100 > > @@ -123,7 +141,7 @@ > if (!pci_enabled || (nd->model && strcmp(nd->model, > "ne2k_isa") == 0)) > pc_init_ne2k_isa(nd); > else > - pci_nic_init_nofail(nd, "e1000", NULL); > + pci_nic_init_nofail(nd, "rtl8139", NULL); > } > > if (drive_get_max_bus(IF_IDE) >= MAX_IDE_BUS) { That looks like it is only changing the default NIC from e1000 to rtl8139. Stefan
Re: [Qemu-devel] [RFC] Device sandboxing
Quoting Paul Moore (pmo...@redhat.com): > On Thursday, December 15, 2011 09:14:11 AM Serge Hallyn wrote: > > Quoting Corey Bryant (cor...@linux.vnet.ibm.com): > > > On 12/14/2011 06:56 PM, Paul Moore wrote: > > > >On Wednesday, December 14, 2011 11:15:58 AM Serge E. Hallyn wrote: > > > >>Hey Paul, > > > >> > > > >>just wondering, exactly which approache(s) are you prototyping? Are > > > >>you touching seccomp2? > > > > > > > >The decomposed approach as I felt (well, still do for that matter) > > > >that the enhanced seccomp stuff could be put to even better use in a > > > >decomposed mode of operation. > > > > > > > >However, earlier this week those of us involved in this effort were > > > >strongly discouraged (this probably isn't the best term to use, but > > > >there is a reason I'm a programmer and not an english student) from > > > >pursuing the decomposed prototype further so work on it has dropped > > > >off considerably. > > > > > > > >I still think it is worth pursuing, if for no other reason than to > > > >answer questions that right now we can only answer with educated > > > >guesses, but it is no longer my main focus. If anyone else is > > > >interested in this feel free to drop me some email and I can bring > > > >you up to speed on the current status. > > > > Thanks, Paul. I don't know for sure that I'll have time, but I'd > > definately be interested in anything you have about current status > > of that approach. On my own I would've pursued the seccomp2 way > > if only because I'll be doing the same for lxc, but if noone else > > is following up on decomposition I might take a look over break. > > And as you say, if the design ends up being maintaineable and with > > acceptable performance overhead, I have no doubt it would be well > > merged with seccomp2. > > The current status of the prototype is that it is still largely incomplete; > most of the "how do I do this?" work is done, now it is just a matter of > coding. > > I *think* I've identified all the function calls that the e1000 device > emulation makes into the core QEMU code as well as a good spot for forking, > most of the implementation is blank (lots of empty function bodies). About > the only part of the implementation that currently has any substance to it is > the pipe based message passing and the code trickery that allows us to go > from > straight functions calls to RPC/IPC. Neither have been tested yet, and the > former isn't as elegant as I would like, but at least they all compile > cleanly > ... ;) > > As I said earlier, I still plan to allocate some time to working on this, but > much less than before. I'll drop you another email, offlist, and if you've > got some interest/time in helping out you're more than welcome to join in. Thanks, Paul. -serge
Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt
On Thu, Dec 15, 2011 at 03:54:15PM +0100, Jiri Denemark wrote: > Hi, > > Recently I realized that all modern CPU models defined in > /etc/qemu/target-x86_64.conf are useless when qemu is used through libvirt. > That's because we start qemu with -nodefconfig which results in qemu ignoring > that file with CPU model definitions. We have a very good reason for using > -nodefconfig because we need to control the ABI presented to a guest OS and we > don't want any configuration file that can contain lots of things including > device definitions to be read by qemu. However, we would really like the new > CPU models to be understood by qemu even if used through libvirt. What would > be the best way to solve this? Ideally libvirt would just write out a config file with the CPU that was configured in libvirt and pass -readconfig /var/lib/libvirt/qemu/guestcpu.conf That way libvirt can configure CPU models without regard for what the particular QEMU version might have in its config. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [RFC] Device sandboxing
On Thursday, December 15, 2011 09:14:11 AM Serge Hallyn wrote: > Quoting Corey Bryant (cor...@linux.vnet.ibm.com): > > On 12/14/2011 06:56 PM, Paul Moore wrote: > > >On Wednesday, December 14, 2011 11:15:58 AM Serge E. Hallyn wrote: > > >>Hey Paul, > > >> > > >>just wondering, exactly which approache(s) are you prototyping? Are > > >>you touching seccomp2? > > > > > >The decomposed approach as I felt (well, still do for that matter) > > >that the enhanced seccomp stuff could be put to even better use in a > > >decomposed mode of operation. > > > > > >However, earlier this week those of us involved in this effort were > > >strongly discouraged (this probably isn't the best term to use, but > > >there is a reason I'm a programmer and not an english student) from > > >pursuing the decomposed prototype further so work on it has dropped > > >off considerably. > > > > > >I still think it is worth pursuing, if for no other reason than to > > >answer questions that right now we can only answer with educated > > >guesses, but it is no longer my main focus. If anyone else is > > >interested in this feel free to drop me some email and I can bring > > >you up to speed on the current status. > > Thanks, Paul. I don't know for sure that I'll have time, but I'd > definately be interested in anything you have about current status > of that approach. On my own I would've pursued the seccomp2 way > if only because I'll be doing the same for lxc, but if noone else > is following up on decomposition I might take a look over break. > And as you say, if the design ends up being maintaineable and with > acceptable performance overhead, I have no doubt it would be well > merged with seccomp2. The current status of the prototype is that it is still largely incomplete; most of the "how do I do this?" work is done, now it is just a matter of coding. I *think* I've identified all the function calls that the e1000 device emulation makes into the core QEMU code as well as a good spot for forking, most of the implementation is blank (lots of empty function bodies). About the only part of the implementation that currently has any substance to it is the pipe based message passing and the code trickery that allows us to go from straight functions calls to RPC/IPC. Neither have been tested yet, and the former isn't as elegant as I would like, but at least they all compile cleanly ... ;) As I said earlier, I still plan to allocate some time to working on this, but much less than before. I'll drop you another email, offlist, and if you've got some interest/time in helping out you're more than welcome to join in. -- paul moore virtualization @ redhat
[Qemu-devel] [PATCH 1/7] memory: introduce memory_region_set_enabled()
This allows users to disable a memory region without removing it from the hierarchy, simplifying the implementation of memory routers. Signed-off-by: Avi Kivity --- memory.c | 40 +--- memory.h | 17 + 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/memory.c b/memory.c index adfdf14..d0f90ca 100644 --- a/memory.c +++ b/memory.c @@ -528,6 +528,10 @@ static void render_memory_region(FlatView *view, FlatRange fr; AddrRange tmp; +if (!mr->enabled) { +return; +} + int128_addto(&base, int128_make64(mr->addr)); readonly |= mr->readonly; @@ -750,12 +754,16 @@ static void address_space_update_topology(AddressSpace *as) address_space_update_ioeventfds(as); } -static void memory_region_update_topology(void) +static void memory_region_update_topology(MemoryRegion *mr) { if (memory_region_transaction_depth) { return; } +if (mr && !mr->enabled) { +return; +} + if (address_space_memory.root) { address_space_update_topology(&address_space_memory); } @@ -773,7 +781,7 @@ void memory_region_transaction_commit(void) { assert(memory_region_transaction_depth); --memory_region_transaction_depth; -memory_region_update_topology(); +memory_region_update_topology(NULL); } static void memory_region_destructor_none(MemoryRegion *mr) @@ -813,6 +821,7 @@ void memory_region_init(MemoryRegion *mr, } mr->addr = 0; mr->offset = 0; +mr->enabled = true; mr->terminates = false; mr->readable = true; mr->readonly = false; @@ -1058,7 +1067,7 @@ void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client) uint8_t mask = 1 << client; mr->dirty_log_mask = (mr->dirty_log_mask & ~mask) | (log * mask); -memory_region_update_topology(); +memory_region_update_topology(mr); } bool memory_region_get_dirty(MemoryRegion *mr, target_phys_addr_t addr, @@ -1090,7 +1099,7 @@ void memory_region_set_readonly(MemoryRegion *mr, bool readonly) { if (mr->readonly != readonly) { mr->readonly = readonly; -memory_region_update_topology(); +memory_region_update_topology(mr); } } @@ -1098,7 +1107,7 @@ void memory_region_rom_device_set_readable(MemoryRegion *mr, bool readable) { if (mr->readable != readable) { mr->readable = readable; -memory_region_update_topology(); +memory_region_update_topology(mr); } } @@ -1203,7 +1212,7 @@ void memory_region_add_eventfd(MemoryRegion *mr, memmove(&mr->ioeventfds[i+1], &mr->ioeventfds[i], sizeof(*mr->ioeventfds) * (mr->ioeventfd_nb-1 - i)); mr->ioeventfds[i] = mrfd; -memory_region_update_topology(); +memory_region_update_topology(mr); } void memory_region_del_eventfd(MemoryRegion *mr, @@ -1233,7 +1242,7 @@ void memory_region_del_eventfd(MemoryRegion *mr, --mr->ioeventfd_nb; mr->ioeventfds = g_realloc(mr->ioeventfds, sizeof(*mr->ioeventfds)*mr->ioeventfd_nb + 1); -memory_region_update_topology(); +memory_region_update_topology(mr); } static void memory_region_add_subregion_common(MemoryRegion *mr, @@ -1274,7 +1283,7 @@ static void memory_region_add_subregion_common(MemoryRegion *mr, } QTAILQ_INSERT_TAIL(&mr->subregions, subregion, subregions_link); done: -memory_region_update_topology(); +memory_region_update_topology(mr); } @@ -1303,19 +1312,28 @@ void memory_region_del_subregion(MemoryRegion *mr, assert(subregion->parent == mr); subregion->parent = NULL; QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link); -memory_region_update_topology(); +memory_region_update_topology(mr); +} + +void memory_region_set_enabled(MemoryRegion *mr, bool enabled) +{ +if (enabled == mr->enabled) { +return; +} +mr->enabled = enabled; +memory_region_update_topology(NULL); } void set_system_memory_map(MemoryRegion *mr) { address_space_memory.root = mr; -memory_region_update_topology(); +memory_region_update_topology(NULL); } void set_system_io_map(MemoryRegion *mr) { address_space_io.root = mr; -memory_region_update_topology(); +memory_region_update_topology(NULL); } typedef struct MemoryRegionList MemoryRegionList; diff --git a/memory.h b/memory.h index 53bf261..c6997c4 100644 --- a/memory.h +++ b/memory.h @@ -123,6 +123,7 @@ struct MemoryRegion { bool terminates; bool readable; bool readonly; /* For RAM regions */ +bool enabled; MemoryRegion *alias; target_phys_addr_t alias_offset; unsigned priority; @@ -501,6 +502,22 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr, void memory_region_del_subregion(MemoryRegion *mr, MemoryRegion *subregion); + +/* + * memory_region_set_enabled: dynamically enable or disable a region + *
Re: [Qemu-devel] Modern CPU models cannot be used with libvirt
On Thu, Dec 15, 2011 at 08:58:55 -0600, Anthony Liguori wrote: > Pass '-readconfig /etc/qemu/target-x86_64.conf' to pick up those models and > if > you are absolutely insistent on not giving the user any ability to change > things > on their own, cp the file from qemu.git into libvirt.git and install it in a > safe place. Ah, this looks like a good idea (and we could even generate that file dynamically if we add support for family/stepping/... and other things that we do not model now). However, separating these definitions from qemu may result in incompatibilities with older qemu versions. I guess mainly because our configuration file would mention a CPU feature that an installed qemu version doesn't understand. Currently, qemu seems to just ignore such feature (although it prints an error) and continues happily without it. Is there any way for us to ask qemu what CPU features it knows about so that we could avoid using a CPU models which include features qemu doesn't understand? Jirka
[Qemu-devel] [PATCH 7/7] docs: document memory API interaction with migration
Signed-off-by: Avi Kivity --- docs/migration.txt | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/docs/migration.txt b/docs/migration.txt index 4848c1e..f3ddd2f 100644 --- a/docs/migration.txt +++ b/docs/migration.txt @@ -219,6 +219,18 @@ The functions to do that are inside a vmstate definition, and are called: Example: You can look at hpet.c, that uses the three function to massage the state that is transferred. +If you use memory API functions that update memory layout outside +initialization (i.e., in response to a guest action), this is a strong +indication that you need to call these functions in a post_load callback. +Examples of such memory API functions are: + + - memory_region_add_subregion() + - memory_region_del_subregion() + - memory_region_set_readonly() + - memory_region_set_enabled() + - memory_region_set_address() + - memory_region_set_alias_offset() + === Subsections === The use of version_id allows to be able to migrate from older versions -- 1.7.7.1
[Qemu-devel] [PATCH 3/7] memory: introduce memory_region_set_alias_offset()
Add an API to update an alias offset of an active alias. This can be used to simplify implementation of dynamic memory banks. Signed-off-by: Avi Kivity --- memory.c | 14 ++ memory.h | 12 2 files changed, 26 insertions(+), 0 deletions(-) diff --git a/memory.c b/memory.c index a080d21..7e842b3 100644 --- a/memory.c +++ b/memory.c @@ -1345,6 +1345,20 @@ void memory_region_set_address(MemoryRegion *mr, target_phys_addr_t addr) memory_region_transaction_commit(); } +void memory_region_set_alias_offset(MemoryRegion *mr, target_phys_addr_t offset) +{ +target_phys_addr_t old_offset = mr->alias_offset; + +assert(mr->alias); +mr->alias_offset = offset; + +if (offset == old_offset || !mr->parent) { +return; +} + +memory_region_update_topology(mr); +} + void set_system_memory_map(MemoryRegion *mr) { address_space_memory.root = mr; diff --git a/memory.h b/memory.h index db53422..c07bcca 100644 --- a/memory.h +++ b/memory.h @@ -529,6 +529,18 @@ void memory_region_set_enabled(MemoryRegion *mr, bool enabled); */ void memory_region_set_address(MemoryRegion *mr, target_phys_addr_t addr); +/* + * memory_region_set_alias_offset: dynamically update a memory alias's offset + * + * Dynamically updates the offset into the target region that an alias points + * to, as if the fourth argument to memory_region_init_alias() has changed. + * + * @mr: the #MemoryRegion to be updated; should be an alias. + * @offset: the new offset into the target memory region + */ +void memory_region_set_alias_offset(MemoryRegion *mr, +target_phys_addr_t offset); + /* Start a transaction; changes will be accumulated and made visible only * when the transaction ends. */ -- 1.7.7.1
[Qemu-devel] [PATCH 5/7] cirrus_vga: adapt to memory mutators API
Simplify the code by avoiding dynamic creation and destruction of memory regions. Signed-off-by: Avi Kivity --- hw/cirrus_vga.c | 50 +- 1 files changed, 17 insertions(+), 33 deletions(-) diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index c7e365b..9f7fea1 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -205,7 +205,7 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s, bool linear_vram; /* vga.vram mapped over cirrus_linear_io */ MemoryRegion low_mem_container; /* container for 0xa-0xc */ MemoryRegion low_mem; /* always mapped, overridden by: */ -MemoryRegion *cirrus_bank[2]; /* aliases at 0xa-0xb */ +MemoryRegion cirrus_bank[2];/* aliases at 0xa-0xb */ uint32_t cirrus_addr_mask; uint32_t linear_mmio_mask; uint8_t cirrus_shadow_gr0; @@ -2363,40 +2363,16 @@ static void cirrus_linear_bitblt_write(void *opaque, }, }; -static void unmap_bank(CirrusVGAState *s, unsigned bank) -{ -if (s->cirrus_bank[bank]) { -memory_region_del_subregion(&s->low_mem_container, -s->cirrus_bank[bank]); -memory_region_destroy(s->cirrus_bank[bank]); -g_free(s->cirrus_bank[bank]); -s->cirrus_bank[bank] = NULL; -} -} - static void map_linear_vram_bank(CirrusVGAState *s, unsigned bank) { -MemoryRegion *mr; -static const char *names[] = { "vga.bank0", "vga.bank1" }; - -if (!(s->cirrus_srcptr != s->cirrus_srcptr_end) +MemoryRegion *mr = &s->cirrus_bank[bank]; +bool enabled = !(s->cirrus_srcptr != s->cirrus_srcptr_end) && !((s->vga.sr[0x07] & 0x01) == 0) && !((s->vga.gr[0x0B] & 0x14) == 0x14) -&& !(s->vga.gr[0x0B] & 0x02)) { - -mr = g_malloc(sizeof(*mr)); -memory_region_init_alias(mr, names[bank], &s->vga.vram, - s->cirrus_bank_base[bank], 0x8000); -memory_region_add_subregion_overlap( -&s->low_mem_container, -0x8000 * bank, -mr, -1); -unmap_bank(s, bank); -s->cirrus_bank[bank] = mr; -} else { -unmap_bank(s, bank); -} +&& !(s->vga.gr[0x0B] & 0x02); + +memory_region_set_enabled(mr, enabled); +memory_region_set_alias_offset(mr, s->cirrus_bank_base[bank]); } static void map_linear_vram(CirrusVGAState *s) @@ -2415,8 +2391,8 @@ static void unmap_linear_vram(CirrusVGAState *s) s->linear_vram = false; memory_region_del_subregion(&s->pci_bar, &s->vga.vram); } -unmap_bank(s, 0); -unmap_bank(s, 1); +memory_region_set_enabled(&s->cirrus_bank[0], false); +memory_region_set_enabled(&s->cirrus_bank[1], false); } /* Compute the memory access functions */ @@ -2856,6 +2832,14 @@ static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci, memory_region_init_io(&s->low_mem, &cirrus_vga_mem_ops, s, "cirrus-low-memory", 0x2); memory_region_add_subregion(&s->low_mem_container, 0, &s->low_mem); +for (i = 0; i < 2; ++i) { +static const char *names[] = { "vga.bank0", "vga.bank1" }; +MemoryRegion *bank = &s->cirrus_bank[i]; +memory_region_init_alias(bank, names[i], &s->vga.vram, 0, 0x8000); +memory_region_set_enabled(bank, false); +memory_region_add_subregion_overlap(&s->low_mem_container, i * 0x8000, +bank, 1); +} memory_region_add_subregion_overlap(system_memory, isa_mem_base + 0x000a, &s->low_mem_container, -- 1.7.7.1
[Qemu-devel] [PATCH 6/7] piix_pci: adapt smram mapping to use memory mutators
Eliminates fake state ->smram_enabled. Signed-off-by: Avi Kivity --- hw/piix_pci.c | 20 ++-- 1 files changed, 6 insertions(+), 14 deletions(-) diff --git a/hw/piix_pci.c b/hw/piix_pci.c index d183443..ac3d898 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -81,7 +81,6 @@ struct PCII440FXState { PAMMemoryRegion pam_regions[13]; MemoryRegion smram_region; uint8_t smm_enabled; -bool smram_enabled; PIIX3State *piix3; }; @@ -141,6 +140,7 @@ static void i440fx_update_memory_mappings(PCII440FXState *d) { int i, r; uint32_t smram; +bool smram_enabled; memory_region_transaction_begin(); update_pam(d, 0xf, 0x10, (d->dev.config[I440FX_PAM] >> 4) & 3, @@ -151,18 +151,8 @@ static void i440fx_update_memory_mappings(PCII440FXState *d) &d->pam_regions[i+1]); } smram = d->dev.config[I440FX_SMRAM]; -if ((d->smm_enabled && (smram & 0x08)) || (smram & 0x40)) { -if (!d->smram_enabled) { -memory_region_del_subregion(d->system_memory, &d->smram_region); -d->smram_enabled = true; -} -} else { -if (d->smram_enabled) { -memory_region_add_subregion_overlap(d->system_memory, 0xa, -&d->smram_region, 1); -d->smram_enabled = false; -} -} +smram_enabled = (d->smm_enabled && (smram & 0x08)) || (smram & 0x40); +memory_region_set_enabled(&d->smram_region, !smram_enabled); memory_region_transaction_commit(); } @@ -307,7 +297,9 @@ static int i440fx_initfn(PCIDevice *dev) } memory_region_init_alias(&f->smram_region, "smram-region", f->pci_address_space, 0xa, 0x2); -f->smram_enabled = true; +memory_region_add_subregion_overlap(f->system_memory, 0xa, +&f->smram_region, 1); +memory_region_set_enabled(&f->smram_region, false); /* Xen supports additional interrupt routes from the PCI devices to * the IOAPIC: the four pins of each PCI device on the bus are also -- 1.7.7.1
Re: [Qemu-devel] [PATCH v5 4/4] Add support for net bridge
On 11/13/2011 09:45 PM, Corey Bryant wrote: The most common use of -net tap is to connect a tap device to a bridge. This requires the use of a script and running qemu as root in order to allocate a tap device to pass to the script. This patch breaks the build: anthony@titi:~/build/qemu$ make CCnet/tap.o cc1: warnings being treated as errors /home/anthony/git/qemu/net/tap.c: In function ‘net_init_tap’: /home/anthony/git/qemu/net/tap.c:560:15: error: ‘s’ may be used uninitialized in this function make: *** [net/tap.o] Error 1 -s = net_tap_fd_init(vlan, "tap", name, fd, vnet_hdr); -if (!s) { -close(fd); -return -1; +s = net_tap_fd_init(vlan, "tap", name, fd, vnet_hdr); +if (!s) { +close(fd); +return -1; +} } And indeed, you've changed the function from unconditionally initializing s to conditionally initializing it. Specifically, you've broken -net tap,fd=X which would break tools like libvirt. Regards, Anthony Liguori
[Qemu-devel] [PATCH 4/7] memory: optimize empty transactions due to mutators
The mutating memory APIs can easily cause empty transactions, where the mutators don't actually change anything, or perhaps only modify disabled regions. Detect these conditions and avoid regenerating the memory topology. Signed-off-by: Avi Kivity --- memory.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/memory.c b/memory.c index 7e842b3..87639ab 100644 --- a/memory.c +++ b/memory.c @@ -19,6 +19,7 @@ #include unsigned memory_region_transaction_depth = 0; +static bool memory_region_update_pending = false; typedef struct AddrRange AddrRange; @@ -757,6 +758,7 @@ static void address_space_update_topology(AddressSpace *as) static void memory_region_update_topology(MemoryRegion *mr) { if (memory_region_transaction_depth) { +memory_region_update_pending |= !mr || mr->enabled; return; } @@ -770,6 +772,8 @@ static void memory_region_update_topology(MemoryRegion *mr) if (address_space_io.root) { address_space_update_topology(&address_space_io); } + +memory_region_update_pending = false; } void memory_region_transaction_begin(void) @@ -781,7 +785,9 @@ void memory_region_transaction_commit(void) { assert(memory_region_transaction_depth); --memory_region_transaction_depth; -memory_region_update_topology(NULL); +if (!memory_region_transaction_depth && memory_region_update_pending) { +memory_region_update_topology(NULL); +} } static void memory_region_destructor_none(MemoryRegion *mr) -- 1.7.7.1
[Qemu-devel] [PATCH 2/7] memory: introduce memory_region_set_address()
Allow changing the address of a memory region while it is in the memory hierarchy. Signed-off-by: Avi Kivity --- memory.c | 21 + memory.h | 11 +++ 2 files changed, 32 insertions(+), 0 deletions(-) diff --git a/memory.c b/memory.c index d0f90ca..a080d21 100644 --- a/memory.c +++ b/memory.c @@ -1324,6 +1324,27 @@ void memory_region_set_enabled(MemoryRegion *mr, bool enabled) memory_region_update_topology(NULL); } +void memory_region_set_address(MemoryRegion *mr, target_phys_addr_t addr) +{ +MemoryRegion *parent = mr->parent; +unsigned priority = mr->priority; +bool may_overlap = mr->may_overlap; + +if (addr == mr->addr || !parent) { +mr->addr = addr; +return; +} + +memory_region_transaction_begin(); +memory_region_del_subregion(parent, mr); +if (may_overlap) { +memory_region_add_subregion_overlap(parent, addr, mr, priority); +} else { +memory_region_add_subregion(parent, addr, mr); +} +memory_region_transaction_commit(); +} + void set_system_memory_map(MemoryRegion *mr) { address_space_memory.root = mr; diff --git a/memory.h b/memory.h index c6997c4..db53422 100644 --- a/memory.h +++ b/memory.h @@ -518,6 +518,17 @@ void memory_region_del_subregion(MemoryRegion *mr, */ void memory_region_set_enabled(MemoryRegion *mr, bool enabled); +/* + * memory_region_set_address: dynamically update the address of a region + * + * Dynamically updates the address of a region, relative to its parent. + * May be used on regions are currently part of a memory hierarchy. + * + * @mr: the region to be updated + * @addr: new address, relative to parent region + */ +void memory_region_set_address(MemoryRegion *mr, target_phys_addr_t addr); + /* Start a transaction; changes will be accumulated and made visible only * when the transaction ends. */ -- 1.7.7.1
[Qemu-devel] [PULL v3 0/7] Memory API mutators
[repost w/ qemu-devel copied this time] This patchset introduces memory_region_set_enabled() and memory_region_set_address() to avoid the requirement on memory routers to track the internal state of the memory API (so they know whether they need to add or remove a region). Instead, they can simply copy the state of the region from the guest-exposed register to the memory core, via the new mutator functions. Please pull from git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/mutators v3: - fix confusion in patch 3 wrt function arguments and doc comments - add migration documentation v2: - fix minor bug in set_address() - add set_alias_offset() - two example users Avi Kivity (7): memory: introduce memory_region_set_enabled() memory: introduce memory_region_set_address() memory: introduce memory_region_set_alias_offset() memory: optimize empty transactions due to mutators cirrus_vga: adapt to memory mutators API piix_pci: adapt smram mapping to use memory mutators docs: document memory API interaction with migration docs/migration.txt | 12 hw/cirrus_vga.c| 50 +++- hw/piix_pci.c | 20 - memory.c | 81 --- memory.h | 40 + 5 files changed, 145 insertions(+), 58 deletions(-) -- 1.7.7.1
Re: [Qemu-devel] [Bug 899140] Re: Problem with Linux Kernel Traffic Control
Ok, So the e1000.c and the e1000_hw.h have absolutely no difference between the original and the ubuntu version... The only differences witch refers to the *Intel e1000* in the wall sources is this one : diff -ru qemu//hw/pc_piix.c qemu-kvm-0.14.0+noroms//hw/pc_piix.c --- qemu//hw/pc_piix.c 2011-12-15 15:37:28.53929 +0100 +++ qemu-kvm-0.14.0+noroms//hw/pc_piix.c2011-02-22 14:34:38.0 +0100 @@ -123,7 +141,7 @@ if (!pci_enabled || (nd->model && strcmp(nd->model, "ne2k_isa") == 0)) pc_init_ne2k_isa(nd); else -pci_nic_init_nofail(nd, "e1000", NULL); +pci_nic_init_nofail(nd, "rtl8139", NULL); } if (drive_get_max_bus(IF_IDE) >= MAX_IDE_BUS) { Vincent Le 15/12/2011 09:07, Stefan Hajnoczi a écrit : > On Wed, Dec 14, 2011 at 02:42:12PM -, Vincent Autefage wrote: >> Ok so the *Intel e1000* seems the only card which is impacted by the >> bug. > Let me recap with a summary of your debugging: > > QEMU 0.14.0, 0.15.0, and 1.0 built from source all have poor network > performance below a 20 Mbit/s limit set with tc inside the guest. > > Ubuntu's 0.14.0 QEMU package does not have poor network performance. > > This problem only occurs with the emulated e1000 device. All other > emulated NICs operate correctly. > > Now you could diff the e1000 emulation code to get the code changes > between vanilla and Ubuntu: > > $ diff -u qemu-0.14.0-vanilla/hw/e1000.c qemu-0.14.0-ubuntu/hw/e1000.c > > (It's possible that there are no significant changes and this bug is > caused by something outside e1000.c but this is place to check first.) > > Or you could even try copying Ubuntu's e1000.c into the vanilla QEMU > source tree and retesting to see if the behavior changes. > > Stefan > -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/899140 Title: Problem with Linux Kernel Traffic Control Status in QEMU: New Bug description: Hi, The last main versions of QEMU (0.14.1, 0.15 and 1.0) have an important problem when running on a Linux distribution which running itself a Traffic Control (TC) instance. Indeed, when TC is configured with a Token Bucket Filter (TBF) with a particular rate, the effective rate is very slower than the desired one. For instance, lets consider the following configuration : # tc qdisc add dev eth0 root tbf rate 20mbit burst 20k latency 50ms The effective rate will be about 100kbit/s ! (verified with iperf) I've encountered this problem on versions 0.14.1, 0.15 and 1.0 but not with the 0.14.0... In the 0.14.0, we have a rate of 19.2 mbit/s which is quiet normal. I've done the experimentation on several hosts : - Debian 32bit core i7, 4GB RAM - Debian 64bit core i7, 8GB RAM - 3 different high performance servers : Ubuntu 64 bits, 48 AMD Opteron, 128GB of RAM The problem is always the same... The problem is also seen with a Class Based Queuing (CBQ) in TC. Thanks To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/899140/+subscriptions
[Qemu-devel] [PATCH 02/14] block: simplify failure handling for bdrv_aio_multiwrite
From: Paolo Bonzini Now that early failure of bdrv_aio_writev is not possible anymore, mcb->num_requests can be set before the loop starts. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block.c | 28 ++-- 1 files changed, 2 insertions(+), 26 deletions(-) diff --git a/block.c b/block.c index 434c13d..16a4c42 100644 --- a/block.c +++ b/block.c @@ -2842,37 +2842,13 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs) trace_bdrv_aio_multiwrite(mcb, mcb->num_callbacks, num_reqs); -/* - * Run the aio requests. As soon as one request can't be submitted - * successfully, fail all requests that are not yet submitted (we must - * return failure for all requests anyway) - * - * num_requests cannot be set to the right value immediately: If - * bdrv_aio_writev fails for some request, num_requests would be too high - * and therefore multiwrite_cb() would never recognize the multiwrite - * request as completed. We also cannot use the loop variable i to set it - * when the first request fails because the callback may already have been - * called for previously submitted requests. Thus, num_requests must be - * incremented for each request that is submitted. - * - * The problem that callbacks may be called early also means that we need - * to take care that num_requests doesn't become 0 before all requests are - * submitted - multiwrite_cb() would consider the multiwrite request - * completed. A dummy request that is "completed" by a manual call to - * multiwrite_cb() takes care of this. - */ -mcb->num_requests = 1; - -// Run the aio requests +/* Run the aio requests. */ +mcb->num_requests = num_reqs; for (i = 0; i < num_reqs; i++) { -mcb->num_requests++; bdrv_aio_writev(bs, reqs[i].sector, reqs[i].qiov, reqs[i].nb_sectors, multiwrite_cb, mcb); } -/* Complete the dummy request */ -multiwrite_cb(mcb, 0); - return 0; } -- 1.7.6.4
Re: [Qemu-devel] [RFC] Device sandboxing
Quoting Corey Bryant (cor...@linux.vnet.ibm.com): > > > On 12/14/2011 06:56 PM, Paul Moore wrote: > >On Wednesday, December 14, 2011 11:15:58 AM Serge E. Hallyn wrote: > >>Quoting Paul Moore (pmo...@redhat.com): > >>>On Wednesday, December 07, 2011 12:48:16 PM Anthony Liguori wrote: > On 12/07/2011 12:25 PM, Corey Bryant wrote: > >A group of us are starting to work on sandboxing QEMU device > >emulation code. We're just getting started investigating > >various approaches, and want to engage the community to gather > >input. > > >Following are the design points that we are currently considering: > To be perfectly honest, I think prototyping and measuring > performance is going to be the only way to figure out the right > approach here.> > >>>Agreed. I'm currently working on a prototype to play around with some > >>>of the ideas discussed in this thread. As soon as it is functional > >>>I'll send a pointer/patches/etc. to the list. > >> > >>Hey Paul, > >> > >>just wondering, exactly which approache(s) are you prototyping? Are you > >>touching seccomp2? > > > >The decomposed approach as I felt (well, still do for that matter) that the > >enhanced seccomp stuff could be put to even better use in a decomposed mode > >of > >operation. > > > >However, earlier this week those of us involved in this effort were strongly > >discouraged (this probably isn't the best term to use, but there is a reason > >I'm a programmer and not an english student) from pursuing the decomposed > >prototype further so work on it has dropped off considerably. > > > >I still think it is worth pursuing, if for no other reason than to answer > >questions that right now we can only answer with educated guesses, but it is > >no longer my main focus. If anyone else is interested in this feel free to > >drop me some email and I can bring you up to speed on the current status. Thanks, Paul. I don't know for sure that I'll have time, but I'd definately be interested in anything you have about current status of that approach. On my own I would've pursued the seccomp2 way if only because I'll be doing the same for lxc, but if noone else is following up on decomposition I might take a look over break. And as you say, if the design ends up being maintaineable and with acceptable performance overhead, I have no doubt it would be well merged with seccomp2. > >As far as the enhanced seccomp patches for QEMU, I believe Corey said that > >IBM > >was starting work on a prototype based on the patches that Will posted > >earlier > >this year. I don't expect this change to be very substantial, the hard part > >will be determining the syscall filter and maintaining it over time. > > > > Paul covered the current state of affairs above so I won't expand on > that much. One of the major concerns from the QEMU community > revolved around the maintenance complexity introduced by decomposing > QEMU into separate processes, and that patches doing so were > unlikely to be accepted. > > With that in mind we're going to pursue a single process mode 2 > approach. We'll put together a trivial prototype for evaluation > purposes. Like Paul mentioned, one of the complex parts is > determining the correct call parameter filters, and there will be > tweaking required as new syscalls/parameters are introduced in the > future. But the biggest hurdle is getting mode 2 patches into the > mainline kernel, which has been an unsuccessful effort for a few > years now. I might be wrong but I think that's a bit overly pessimistic :) Pretty sure it's only been a few months. Compared to some other things like checkpoint/restart and user namespaces, it's positively on a fast track. And if qemu demonstrates true value, that can only help. thanks, -serge
Re: [Qemu-devel] [PATCH V2 3/5] Introduce premigrate RunState.
On 12/09/2011 03:54 PM, Anthony PERARD wrote: This new state will be used by Xen functions to know QEMU will wait for a migration. This is important to know for memory related function because the memory is already allocated and reallocated them will not works. Signed-off-by: Anthony PERARD Luiz, please Ack. In the future, when you make QMP changes, please CC the appropriate maintainer. Regards, Anthony Liguori --- qapi-schema.json |2 +- vl.c |4 2 files changed, 5 insertions(+), 1 deletions(-) diff --git a/qapi-schema.json b/qapi-schema.json index cb1ba77..bd77444 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -121,7 +121,7 @@ { 'enum': 'RunState', 'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', -'running', 'save-vm', 'shutdown', 'watchdog' ] } +'running', 'save-vm', 'shutdown', 'watchdog', 'premigrate' ] } ## # @StatusInfo: diff --git a/vl.c b/vl.c index e7dced2..a291416 100644 --- a/vl.c +++ b/vl.c @@ -351,8 +351,11 @@ static const RunStateTransition runstate_transitions_def[] = { { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING }, { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE }, +{ RUN_STATE_PRELAUNCH, RUN_STATE_PREMIGRATE }, { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE }, +{ RUN_STATE_PREMIGRATE, RUN_STATE_INMIGRATE }, + { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING }, { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE }, @@ -2975,6 +2978,7 @@ int main(int argc, char **argv, char **envp) break; case QEMU_OPTION_incoming: incoming = optarg; +runstate_set(RUN_STATE_PREMIGRATE); break; case QEMU_OPTION_nodefaults: default_serial = 0;
Re: [Qemu-devel] [PATCH] enable architectural PMU cpuid leaf for kvm
On Thu, Dec 15, 2011 at 09:09:32AM -0600, Anthony Liguori wrote: > On 12/15/2011 04:44 AM, Gleb Natapov wrote: > > > >Signed-off-by: Gleb Natapov > > This should go in via uq/master. uq/master maintainers can you take it? > > Regards, > > Anthony Liguori > > >diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c > >index 0b3af90..91a104b 100644 > >--- a/target-i386/cpuid.c > >+++ b/target-i386/cpuid.c > >@@ -1180,10 +1180,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, > >uint32_t count, > > break; > > case 0xA: > > /* Architectural Performance Monitoring Leaf */ > >-*eax = 0; > >-*ebx = 0; > >-*ecx = 0; > >-*edx = 0; > >+if (kvm_enabled()) { > >+KVMState *s = env->kvm_state; > >+ > >+*eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX); > >+*ebx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EBX); > >+*ecx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_ECX); > >+*edx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EDX); > >+} else { > >+*eax = 0; > >+*ebx = 0; > >+*ecx = 0; > >+*edx = 0; > >+} > > break; > > case 0xD: > > /* Processor Extended State */ > >-- > > Gleb. > > > > -- Gleb.
Re: [Qemu-devel] [PATCH V2 1/5] vl.c: Do not save RAM state when Xen is used.
On 12/09/2011 03:54 PM, Anthony PERARD wrote: In Xen case, the guest RAM is not handle by QEMU, and it is saved by Xen tools. So, we just avoid to register the RAM save state handler. Signed-off-by: Anthony PERARD --- vl.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/vl.c b/vl.c index f5afed4..e7dced2 100644 --- a/vl.c +++ b/vl.c @@ -3273,8 +3273,10 @@ int main(int argc, char **argv, char **envp) default_drive(default_sdcard, snapshot, machine->use_scsi, IF_SD, 0, SD_OPTS); -register_savevm_live(NULL, "ram", 0, 4, NULL, ram_save_live, NULL, - ram_load, NULL); +if (!xen_enabled()) { +register_savevm_live(NULL, "ram", 0, 4, NULL, ram_save_live, NULL, + ram_load, NULL); +} Why don't you just unregister the section in the xen initialization code? That way we don't have xen_enabled()'s sprinkled all over the place. Regards, Anthony Liguori if (nb_numa_nodes> 0) { int i;
Re: [Qemu-devel] [PATCH] usb-ohci: return USBBus in usb_ohci_init_pci
On 12/15/2011 02:17 AM, Stefan Hajnoczi wrote: On Wed, Dec 14, 2011 at 06:10:17PM -0600, Anthony Liguori wrote: Untested, but seemingly obvious and hard to screw up.. Sounds like a challenge so let's take a look... -void usb_ohci_init_pci(struct PCIBus *bus, int devfn) +USBBus *usb_ohci_init_pci(struct PCIBus *bus, int devfn) { -pci_create_simple(bus, devfn, "pci-ohci"); +PCIDevice *dev = pci_create_simple(bus, devfn, "pci-ohci"); +OHCIPCIState *ohci = DO_UPCAST(OHCIPCIState, pci_dev, dev); + +return&ohci->state.bus Missing semicolon. Heh, awesome :-) Okay, I'll actually test it next time. Regards, Anthony Liguori Stefan
[Qemu-devel] [PATCH 05/14] block: dma_bdrv_* does not return NULL
From: Paolo Bonzini Initially attempted with the following semantic patch: @ rule1 @ expression E; statement S; @@ E = ( dma_bdrv_io | dma_bdrv_read | dma_bdrv_write ) (...); ( - if (E == NULL) { ... } | - if (E) { <... S ...> } ) which however did not match anything. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- hw/ide/core.c |6 -- hw/ide/macio.c |4 +--- 2 files changed, 1 insertions(+), 9 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 7071326..de9ed41 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -549,7 +549,6 @@ void ide_dma_cb(void *opaque, int ret) int n; int64_t sector_num; -handle_rw_error: if (ret < 0) { int op = BM_STATUS_DMA_RETRY; @@ -608,11 +607,6 @@ handle_rw_error: ide_issue_trim, ide_dma_cb, s, true); break; } - -if (!s->bus->dma->aiocb) { -ret = -1; -goto handle_rw_error; -} return; eot: diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 40f60f0..abbc41b 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -152,10 +152,8 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) ide_issue_trim, pmac_ide_transfer_cb, s, true); break; } - -if (!m->aiocb) -pmac_ide_transfer_cb(io, -1); return; + done: if (s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) { bdrv_acct_done(s->bs, &s->acct); -- 1.7.6.4
[Qemu-devel] [PATCH 06/14] block: avoid useless checks on acb->bh
From: Paolo Bonzini Coverity is confused by this "if" and reports leaks on acb->bh. The bottom half is always deleted before releasing the AIOCB, in either bdrv_aio_cancel_em or bdrv_aio_bh_cb. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block.c |4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/block.c b/block.c index 16a4c42..3f072f6 100644 --- a/block.c +++ b/block.c @@ -3077,9 +3077,7 @@ static BlockDriverAIOCB *bdrv_aio_rw_vector(BlockDriverState *bs, acb->is_write = is_write; acb->qiov = qiov; acb->bounce = qemu_blockalign(bs, qiov->size); - -if (!acb->bh) -acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb); +acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb); if (is_write) { qemu_iovec_to_buffer(acb->qiov, acb->bounce); -- 1.7.6.4
[Qemu-devel] [PATCH 03/14] block: qemu_aio_get does not return NULL
From: Paolo Bonzini Initially done with the following semantic patch: @ rule1 @ expression E; statement S; @@ E = qemu_aio_get (...); ( - if (E == NULL) { ... } | - if (E) { <... S ...> } ) which however missed occurrences in linux-aio.c and posix-aio-compat.c. Those were done by hand. The change in vdi_aio_setup's caller was also done by hand. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block/curl.c |4 block/rbd.c|3 --- block/vdi.c| 46 ++ linux-aio.c|2 -- posix-aio-compat.c |4 5 files changed, 18 insertions(+), 41 deletions(-) diff --git a/block/curl.c b/block/curl.c index 4209ac8..e9102e3 100644 --- a/block/curl.c +++ b/block/curl.c @@ -509,10 +509,6 @@ static BlockDriverAIOCB *curl_aio_readv(BlockDriverState *bs, acb = qemu_aio_get(&curl_aio_pool, bs, cb, opaque); -if (!acb) { -return NULL; -} - acb->qiov = qiov; acb->sector_num = sector_num; acb->nb_sectors = nb_sectors; diff --git a/block/rbd.c b/block/rbd.c index 9088c52..312584a 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -632,9 +632,6 @@ static BlockDriverAIOCB *rbd_aio_rw_vector(BlockDriverState *bs, BDRVRBDState *s = bs->opaque; acb = qemu_aio_get(&rbd_aio_pool, bs, cb, opaque); -if (!acb) { -return NULL; -} acb->write = write; acb->qiov = qiov; acb->bounce = qemu_blockalign(bs, qiov->size); diff --git a/block/vdi.c b/block/vdi.c index 6bb43b8..31cdfab 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -515,28 +515,26 @@ static VdiAIOCB *vdi_aio_setup(BlockDriverState *bs, int64_t sector_num, bs, sector_num, qiov, nb_sectors, cb, opaque, is_write); acb = qemu_aio_get(&vdi_aio_pool, bs, cb, opaque); -if (acb) { -acb->hd_aiocb = NULL; -acb->sector_num = sector_num; -acb->qiov = qiov; -acb->is_write = is_write; - -if (qiov->niov > 1) { -acb->buf = qemu_blockalign(bs, qiov->size); -acb->orig_buf = acb->buf; -if (is_write) { -qemu_iovec_to_buffer(qiov, acb->buf); -} -} else { -acb->buf = (uint8_t *)qiov->iov->iov_base; +acb->hd_aiocb = NULL; +acb->sector_num = sector_num; +acb->qiov = qiov; +acb->is_write = is_write; + +if (qiov->niov > 1) { +acb->buf = qemu_blockalign(bs, qiov->size); +acb->orig_buf = acb->buf; +if (is_write) { +qemu_iovec_to_buffer(qiov, acb->buf); } -acb->nb_sectors = nb_sectors; -acb->n_sectors = 0; -acb->bmap_first = VDI_UNALLOCATED; -acb->bmap_last = VDI_UNALLOCATED; -acb->block_buffer = NULL; -acb->header_modified = 0; -} +} else { +acb->buf = (uint8_t *)qiov->iov->iov_base; +} +acb->nb_sectors = nb_sectors; +acb->n_sectors = 0; +acb->bmap_first = VDI_UNALLOCATED; +acb->bmap_last = VDI_UNALLOCATED; +acb->block_buffer = NULL; +acb->header_modified = 0; return acb; } @@ -653,10 +651,6 @@ static BlockDriverAIOCB *vdi_aio_readv(BlockDriverState *bs, logout("\n"); acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, 0); -if (!acb) { -return NULL; -} - ret = vdi_schedule_bh(vdi_aio_rw_bh, acb); if (ret < 0) { if (acb->qiov->niov > 1) { @@ -807,10 +801,6 @@ static BlockDriverAIOCB *vdi_aio_writev(BlockDriverState *bs, logout("\n"); acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, 1); -if (!acb) { -return NULL; -} - ret = vdi_schedule_bh(vdi_aio_rw_bh, acb); if (ret < 0) { if (acb->qiov->niov > 1) { diff --git a/linux-aio.c b/linux-aio.c index 1c635ef..d2fc2e7 100644 --- a/linux-aio.c +++ b/linux-aio.c @@ -166,8 +166,6 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd, off_t offset = sector_num * 512; laiocb = qemu_aio_get(&laio_pool, bs, cb, opaque); -if (!laiocb) -return NULL; laiocb->nbytes = nb_sectors * 512; laiocb->ctx = s; laiocb->ret = -EINPROGRESS; diff --git a/posix-aio-compat.c b/posix-aio-compat.c index c380ec1..cccb673 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -611,8 +611,6 @@ BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd, struct qemu_paiocb *acb; acb = qemu_aio_get(&raw_aio_pool, bs, cb, opaque); -if (!acb) -return NULL; acb->aio_type = type; acb->aio_fildes = fd; @@ -638,8 +636,6 @@ BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd, struct qemu_paiocb *acb; acb = qemu_aio_get(&raw_aio_pool, bs, cb, opaque); -if (!acb) -return NULL; acb->aio_type = QEMU_AIO_IOCTL; acb->aio_fildes = fd; acb->aio_offset = 0; -- 1.7.6.4
[Qemu-devel] [PATCH v3] qemu-ga: Add the guest-suspend command
It supports three modes: "hibernate" (suspend to disk), "sleep" (suspend to ram) and "hybrid" (save RAM contents to disk, but suspend instead of powering off). The command will try to execute the scripts provided by the pm-utils package. If that fails, it will try to suspend manually by writing to the "/sys/power/state" file. To reap terminated children, a new signal handler is installed to catch SIGCHLD signals and a non-blocking call to waitpid() is done to collect their exit statuses. Signed-off-by: Luiz Capitulino --- v3 o Add 'hybrid' mode o Use execlp() instead of execl() o Don't ignore SIGCHLD, catch it instead and call waitpid() from the signal handler to avoid zombies o Improve the schema doc qapi-schema-guest.json | 26 ++ qemu-ga.c | 17 +++- qga/guest-agent-commands.c | 64 3 files changed, 106 insertions(+), 1 deletions(-) diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json index 5f8a18d..6b9657a 100644 --- a/qapi-schema-guest.json +++ b/qapi-schema-guest.json @@ -219,3 +219,29 @@ ## { 'command': 'guest-fsfreeze-thaw', 'returns': 'int' } + +## +# @guest-suspend +# +# Suspend guest execution by entering ACPI power state S3 or S4. +# +# This command tries to execute the scripts provided by the pm-utils +# package. If they are not available, it will perform the suspend +# operation by manually writing to a sysfs file. +# +# For the best results it's strongly recommended to have the pm-utils +# package installed in the guest. +# +# @mode: 'hibernate' RAM content is saved to the disk and the guest is +#powered off (this corresponds to ACPI S4) +#'sleep' execution is suspended but the RAM retains its contents +#(this corresponds to ACPI S3) +#'hybrid'RAM content is saved to the disk but the guest is +#suspended instead of powering off +# +# Notes: This is an asynchronous request. There's no guarantee a response +# will be sent. Errors will be logged to guest's syslog. +# +# Since: 1.1 +## +{ 'command': 'guest-suspend', 'data': { 'mode': 'str' } } diff --git a/qemu-ga.c b/qemu-ga.c index 60d4972..a7c5973 100644 --- a/qemu-ga.c +++ b/qemu-ga.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "qemu_socket.h" #include "json-streamer.h" #include "json-parser.h" @@ -59,9 +60,15 @@ static void quit_handler(int sig) } } +static void child_handler(int sig) +{ +int status; +waitpid(-1, &status, WNOHANG); +} + static void register_signal_handlers(void) { -struct sigaction sigact; +struct sigaction sigact, sigact_chld; int ret; memset(&sigact, 0, sizeof(struct sigaction)); @@ -76,6 +83,14 @@ static void register_signal_handlers(void) if (ret == -1) { g_error("error configuring signal handler: %s", strerror(errno)); } + +memset(&sigact_chld, 0, sizeof(struct sigaction)); +sigact_chld.sa_handler = child_handler; +sigact_chld.sa_flags = SA_NOCLDSTOP; +ret = sigaction(SIGCHLD, &sigact_chld, NULL); +if (ret == -1) { +g_error("error configuring signal handler: %s", strerror(errno)); +} } static void usage(const char *cmd) diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c index a09c8ca..cc466ba 100644 --- a/qga/guest-agent-commands.c +++ b/qga/guest-agent-commands.c @@ -574,6 +574,70 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err) } #endif +#define LINUX_SYS_STATE_FILE "/sys/power/state" + +void qmp_guest_suspend(const char *mode, Error **err) +{ +pid_t pid; +const char *pmutils_bin; + +if (strcmp(mode, "hibernate") == 0) { +pmutils_bin = "pm-hibernate"; +} else if (strcmp(mode, "sleep") == 0) { +pmutils_bin = "pm-suspend"; +} else if (strcmp(mode, "hybrid") == 0) { +pmutils_bin = "pm-suspend-hybrid"; +} else { +error_set(err, QERR_INVALID_PARAMETER, "mode"); +return; +} + +pid = fork(); +if (pid == 0) { +/* child */ +int fd; +const char *cmd; + +setsid(); +fclose(stdin); +fclose(stdout); +fclose(stderr); + +execlp(pmutils_bin, pmutils_bin, NULL); + +/* + * The exec call should not return, if it does something went wrong. + * In this case we try to suspend manually if 'mode' is 'hibernate' + * or 'sleep' + */ +slog("could not execute %s: %s\n", pmutils_bin, strerror(errno)); +if (strcmp(mode, "hybrid") == 0) { +exit(1); +} + +slog("trying to suspend using the manual method...\n"); + +fd = open(LINUX_SYS_STATE_FILE, O_WRONLY); +if (fd < 0) { +slog("can't open file %s: %s\n", LINUX_SYS_STATE_FILE, +strerror(errno)); +exit(1); +} + +cmd = strcmp(mode, "sleep") == 0 ? "mem" : "disk"; +
Re: [Qemu-devel] [PATCH] enable architectural PMU cpuid leaf for kvm
On 12/15/2011 04:44 AM, Gleb Natapov wrote: Signed-off-by: Gleb Natapov This should go in via uq/master. Regards, Anthony Liguori diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c index 0b3af90..91a104b 100644 --- a/target-i386/cpuid.c +++ b/target-i386/cpuid.c @@ -1180,10 +1180,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 0xA: /* Architectural Performance Monitoring Leaf */ -*eax = 0; -*ebx = 0; -*ecx = 0; -*edx = 0; +if (kvm_enabled()) { +KVMState *s = env->kvm_state; + +*eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX); +*ebx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EBX); +*ecx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_ECX); +*edx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EDX); +} else { +*eax = 0; +*ebx = 0; +*ecx = 0; +*edx = 0; +} break; case 0xD: /* Processor Extended State */ -- Gleb.
[Qemu-devel] [PATCH 08/14] rbd: always set out parameter in qemu_rbd_snap_list
From: Josh Durgin The caller expects psn_tab to be NULL when there are no snapshots or an error occurs. This results in calling g_free on an invalid address. Reported-by: Oliver Francke Signed-off-by: Josh Durgin Signed-off-by: Kevin Wolf --- block/rbd.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/block/rbd.c b/block/rbd.c index 312584a..7a2384c 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -805,7 +805,7 @@ static int qemu_rbd_snap_list(BlockDriverState *bs, } while (snap_count == -ERANGE); if (snap_count <= 0) { -return snap_count; +goto done; } sn_tab = g_malloc0(snap_count * sizeof(QEMUSnapshotInfo)); @@ -824,6 +824,7 @@ static int qemu_rbd_snap_list(BlockDriverState *bs, } rbd_snap_list_end(snaps); + done: *psn_tab = sn_tab; return snap_count; } -- 1.7.6.4
[Qemu-devel] [PATCH 07/14] block/qcow2.c: call qcow2_free_snapshots in the function of qcow2_close
From: Li Zhi Hui Signed-off-by: Li Zhi Hui Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/qcow2.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 37cd442..aa32e8d 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -635,6 +635,7 @@ static void qcow2_close(BlockDriverState *bs) g_free(s->cluster_cache); qemu_vfree(s->cluster_data); qcow2_refcount_close(bs); +qcow2_free_snapshots(bs); } static void qcow2_invalidate_cache(BlockDriverState *bs) -- 1.7.6.4
Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU
On Thu, 15 Dec 2011 14:57:51 +0100 Jan Kiszka wrote: > On 2011-12-15 14:53, Stefan Hajnoczi wrote: > > On Thu, Dec 15, 2011 at 1:49 PM, Kevin Wolf wrote: > >> Am 15.12.2011 14:39, schrieb Jan Kiszka: > >>> On 2011-12-15 14:38, Lucas Meneghel Rodrigues wrote: > On 12/15/2011 11:33 AM, Kevin Wolf wrote: > > Am 15.12.2011 14:18, schrieb Jan Kiszka: > >> On 2011-12-15 14:02, Stefan Hajnoczi wrote: > >>> What is the status of QEMU's transition from HMP to the QMP interface? > >>> > >>> My current understanding is that QEMU provides new HMP commands for > >>> humans, but HMP is being phased out as an API. Management tools > >>> should rely only on QMP for new commands. That would mean new HMP > >>> commands are not guaranteed to produce backwards-compatible output > >>> because tools are not supposed to parse the output. > >>> > >>> On the libvirt side, new QEMU features should only be supported via > >>> the json monitor in the future (i.e. human monitor patches should not > >>> be sent/merged)? Existing HMP commands will still need the human > >>> monitor support in order to handle old QEMU versions gracefully, but > >>> I'm thinking about new commands only. > >>> > >>> Does everyone agree on this? I think this is an important discussion > >>> if we want our management interface to get better and more consistent > >>> in the future. > >> > >> To phase out the classic HMP implementation, we need an internal > >> HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console > >> and gdbstub monitors continue to benefit from new commands. Those > >> interfaces will stay for a long time, I'm sure. > > > > I think we're not talking about dropping HMP here, only about how long > > to support it as a stable API for management tools. I believe that we > > have been in a transitional phase for long enough now that we can start > > changing the output format of HMP commands without considering it an API > > breakage. > > Yes, I've got the same impression. But while we are at it, forgive my > naiveness, but wouldn't be worthwhile to consider dropping the human > monitor in the long run? > >>> > >>> Surely not the interface (for virtual console & gdbstub), but the > >>> internal implementation I hope. > >> > >> Isn't HMP implemented in terms of QMP these days? Yes, if you look at hmp.c you'll see that HMP is using QMP as a client. Of course that there are a lot of commands to be converted, but it's just a matter of time to get this done. > > > > Yes and no, I don't mean writing text manipulation code on to of QMP > > command handlers the way we're doing now. It's a pain. > > > > I meant more along the lines of making qmp-shell more human-friendly. > > You already can specify the command in a command-line fashion - you > > don't need to write raw JSON. I think it's a question of improving > > this and perhaps integrating the documentation for the QMP/QAPI > > commands right at the prompt so that it's easy to learn about the > > available commands. This would be a new interactive shell that stays > > much closer to QMP so that we don't bother with maintaining > > per-command text formatting functions like we do with HMP today. > > Monitor pass-through via gdbstub requires text formatting on QEMU side. > We could start providing a python plugin for gdb at some point that does > the pretty printing on the client side, but moving over will be a > lengthy process as well. Yes, I expect some HMP commands to be difficult to port and that will require time. But if anyone is interested, we could start making qmp-shell a decent shell as Stefan suggests above. In the beginning it won't have all commands HMP has today, but in the future it could replace it. > > Jan >
Re: [Qemu-devel] Modern CPU models cannot be used with libvirt
On 12/15/2011 08:54 AM, Jiri Denemark wrote: Hi, Recently I realized that all modern CPU models defined in /etc/qemu/target-x86_64.conf are useless when qemu is used through libvirt. That's because we start qemu with -nodefconfig which results in qemu ignoring that file with CPU model definitions. We have a very good reason for using -nodefconfig because we need to control the ABI presented to a guest OS and we don't want any configuration file that can contain lots of things including device definitions to be read by qemu. However, we would really like the new CPU models to be understood by qemu even if used through libvirt. What would be the best way to solve this? Pass '-readconfig /etc/qemu/target-x86_64.conf' to pick up those models and if you are absolutely insistent on not giving the user any ability to change things on their own, cp the file from qemu.git into libvirt.git and install it in a safe place. Regards, Anthony Liguori I suspect this could have been already discussed in the past but obviously a workable solution was either not found or just not implemented. Jirka
Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU
On 12/15/2011 08:52 AM, Luiz Capitulino wrote: On Thu, 15 Dec 2011 08:06:41 -0600 Anthony Liguori wrote: There are still a lot of HMP commands that don't have an QMP analog. Luiz, it might make sense to setup a wiki page which instructions on how to convert an HMP command to a QMP command using QAPI. I wrote on how to write new QMP commands using the QAPI (docs/writing-qmp-commands.txt), going from there to a conversion shouldn't be difficult, but I can write a wiki page. Btw, counting with a series I haven't posted yet, we have less than 10 QMP commands left to be converted to the QAPI. I expect to start working on HMP conversions in January. That is awesome! Regards, Anthony Liguori If we did that, we could probably get more folks involved in the conversion process. That would be wonderful. Regards, Anthony Liguori Does everyone agree on this? I think this is an important discussion if we want our management interface to get better and more consistent in the future. Stefan
[Qemu-devel] [PULL 00/14] Block patches
The following changes since commit 222f23f508a8d778f56eddef14752dfd26d225b4: tcg/arm: remove fixed map code buffer restriction (2011-12-14 21:58:18 +0100) are available in the git repository at: git://repo.or.cz/qemu/kevin.git for-anthony Avi Kivity (1): coroutine: switch per-thread free pool to a global pool Josh Durgin (1): rbd: always set out parameter in qemu_rbd_snap_list Kevin Wolf (3): qemu-img rebase: Fix for undersized backing files Documentation: Add qemu-img -t parameter in man page qcow2: Allow >4 GB VM state Li Zhi Hui (2): block/qcow2.c: call qcow2_free_snapshots in the function of qcow2_close block/cow: Return real error code Paolo Bonzini (7): block: bdrv_aio_* do not return NULL block: simplify failure handling for bdrv_aio_multiwrite block: qemu_aio_get does not return NULL dma: the passed io_func does not return NULL block: dma_bdrv_* does not return NULL block: avoid useless checks on acb->bh qiov: prevent double free or use-after-free block-migration.c | 13 - block.c| 56 +++- block.h|2 +- block/blkverify.c | 24 ++--- block/cow.c| 44 +--- block/curl.c |4 --- block/qcow2-snapshot.c | 34 +++- block/qcow2.c |1 + block/qcow2.h |2 +- block/qed-table.c | 22 +--- block/qed.c| 60 +++ block/rbd.c|6 +--- block/vdi.c| 66 +-- coroutine-ucontext.c | 30 +++-- cutils.c |3 ++ dma-helpers.c |4 +-- docs/specs/qcow2.txt |8 +- hw/ide/atapi.c |8 +- hw/ide/core.c | 13 + hw/ide/macio.c | 11 +--- hw/scsi-disk.c |9 -- hw/scsi-generic.c |4 --- hw/virtio-blk.c| 19 +++--- linux-aio.c|2 - posix-aio-compat.c |4 --- qemu-img-cmds.hx |6 ++-- qemu-img.c | 42 -- qemu-img.texi | 10 +-- qemu-io.c | 39 ++-- savevm.c |2 +- trace-events |2 - 31 files changed, 205 insertions(+), 345 deletions(-)
[Qemu-devel] Modern CPU models cannot be used with libvirt
Hi, Recently I realized that all modern CPU models defined in /etc/qemu/target-x86_64.conf are useless when qemu is used through libvirt. That's because we start qemu with -nodefconfig which results in qemu ignoring that file with CPU model definitions. We have a very good reason for using -nodefconfig because we need to control the ABI presented to a guest OS and we don't want any configuration file that can contain lots of things including device definitions to be read by qemu. However, we would really like the new CPU models to be understood by qemu even if used through libvirt. What would be the best way to solve this? I suspect this could have been already discussed in the past but obviously a workable solution was either not found or just not implemented. Jirka
Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU
On Thu, 15 Dec 2011 08:06:41 -0600 Anthony Liguori wrote: > On 12/15/2011 07:57 AM, Luiz Capitulino wrote: > > On Thu, 15 Dec 2011 13:02:40 + > > Stefan Hajnoczi wrote: > > > >> What is the status of QEMU's transition from HMP to the QMP interface? > > > > Depends on what you consider the transition to be. > > > > For management tools the transition can be considered done already because > > we > > do not support HMP as a stable interface. > > > >> My current understanding is that QEMU provides new HMP commands for > >> humans, but HMP is being phased out as an API. > > > > It already did. > > > >> Management tools > >> should rely only on QMP for new commands. That would mean new HMP > >> commands are not guaranteed to produce backwards-compatible output > >> because tools are not supposed to parse the output. > > > > Exactly. > > > >> On the libvirt side, new QEMU features should only be supported via > >> the json monitor in the future (i.e. human monitor patches should not > >> be sent/merged)? Existing HMP commands will still need the human > >> monitor support in order to handle old QEMU versions gracefully, but > >> I'm thinking about new commands only. > > > > Maybe it's a matter of terminology, but I have the impression you're > > talking about two things here: > > > > 1. HMP will always exist, in the meaning that qemu will always provide > > a human interface. If we move it to a python script or some kind of > > external process, that's an implementation detail. > > > > This means that, if you're adding new functionality to qemu and it > > does make sense for humans to use it, then it should have a HMP > > version. > > > > 2. If you do add the HMP interface, that's for humans to consume and > > its output/semantics should make sense for humans, not for management > > tools. > > 3. All HMP commands will be implemented in terms of QMP commands (and only in > terms of QMP commands). Right. > There are still a lot of HMP commands that don't have an QMP analog. Luiz, > it > might make sense to setup a wiki page which instructions on how to convert an > HMP command to a QMP command using QAPI. I wrote on how to write new QMP commands using the QAPI (docs/writing-qmp-commands.txt), going from there to a conversion shouldn't be difficult, but I can write a wiki page. Btw, counting with a series I haven't posted yet, we have less than 10 QMP commands left to be converted to the QAPI. I expect to start working on HMP conversions in January. > If we did that, we could probably get more folks involved in the conversion > process. That would be wonderful. > > Regards, > > Anthony Liguori > > > > >> Does everyone agree on this? I think this is an important discussion > >> if we want our management interface to get better and more consistent > >> in the future. > >> > >> Stefan > >> > > > > >
[Qemu-devel] [PATCH 13/14] block/cow: Return real error code
From: Li Zhi Hui Signed-off-by: Li Zhi Hui Signed-off-by: Kevin Wolf --- block/cow.c | 44 +--- 1 files changed, 29 insertions(+), 15 deletions(-) diff --git a/block/cow.c b/block/cow.c index 3c52735..bb5927c 100644 --- a/block/cow.c +++ b/block/cow.c @@ -64,15 +64,26 @@ static int cow_open(BlockDriverState *bs, int flags) struct cow_header_v2 cow_header; int bitmap_size; int64_t size; +int ret; /* see if it is a cow image */ -if (bdrv_pread(bs->file, 0, &cow_header, sizeof(cow_header)) != -sizeof(cow_header)) { +ret = bdrv_pread(bs->file, 0, &cow_header, sizeof(cow_header)); +if (ret < 0) { +goto fail; +} + +if (be32_to_cpu(cow_header.magic) != COW_MAGIC) { +ret = -EINVAL; goto fail; } -if (be32_to_cpu(cow_header.magic) != COW_MAGIC || -be32_to_cpu(cow_header.version) != COW_VERSION) { +if (be32_to_cpu(cow_header.version) != COW_VERSION) { +char version[64]; +snprintf(version, sizeof(version), + "COW version %d", cow_header.version); +qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, +bs->device_name, "cow", version); +ret = -ENOTSUP; goto fail; } @@ -88,7 +99,7 @@ static int cow_open(BlockDriverState *bs, int flags) qemu_co_mutex_init(&s->lock); return 0; fail: -return -1; +return ret; } /* @@ -182,17 +193,19 @@ static int coroutine_fn cow_read(BlockDriverState *bs, int64_t sector_num, ret = bdrv_pread(bs->file, s->cow_sectors_offset + sector_num * 512, buf, n * 512); -if (ret != n * 512) -return -1; +if (ret < 0) { +return ret; +} } else { if (bs->backing_hd) { /* read from the base image */ ret = bdrv_read(bs->backing_hd, sector_num, buf, n); -if (ret < 0) -return -1; +if (ret < 0) { +return ret; +} } else { -memset(buf, 0, n * 512); -} +memset(buf, 0, n * 512); +} } nb_sectors -= n; sector_num += n; @@ -220,8 +233,9 @@ static int cow_write(BlockDriverState *bs, int64_t sector_num, ret = bdrv_pwrite(bs->file, s->cow_sectors_offset + sector_num * 512, buf, nb_sectors * 512); -if (ret != nb_sectors * 512) -return -1; +if (ret < 0) { +return ret; +} return cow_update_bitmap(bs, sector_num, nb_sectors); } @@ -288,14 +302,14 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) cow_header.sectorsize = cpu_to_be32(512); cow_header.size = cpu_to_be64(image_sectors * 512); ret = bdrv_pwrite(cow_bs, 0, &cow_header, sizeof(cow_header)); -if (ret != sizeof(cow_header)) { +if (ret < 0) { goto exit; } /* resize to include at least all the bitmap */ ret = bdrv_truncate(cow_bs, sizeof(cow_header) + ((image_sectors + 7) >> 3)); -if (ret) { +if (ret < 0) { goto exit; } -- 1.7.6.4