[Qemu-devel] Did anybody study on the XEN-3.4.2 based on qemu for hardware virtualization machine?

2011-12-15 Thread ¤終於aware
Hi:
 Looking for the person who has the common goal.
 Looking forward to your reply!

Re: [Qemu-devel] multifunction pci virtio-blk devices

2011-12-15 Thread Hui Kai Ran

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

2011-12-15 Thread David Gibson
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

2011-12-15 Thread Paul Brook
> 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

2011-12-15 Thread Andreas Färber
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

2011-12-15 Thread Alex Williamson
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)

2011-12-15 Thread Alexey Korolev

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

2011-12-15 Thread Corey Bryant



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.)

2011-12-15 Thread 陳韋任
> 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

2011-12-15 Thread TeLeMan
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

2011-12-15 Thread zanghongyong
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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Jason Yin
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

2011-12-15 Thread David Gibson
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

2011-12-15 Thread qemu
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

2011-12-15 Thread David Gibson
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

2011-12-15 Thread Michael Ellerman
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

2011-12-15 Thread Peter Maydell
** 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

2011-12-15 Thread Peter Maydell
** 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

2011-12-15 Thread Alex Williamson
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()

2011-12-15 Thread Hervé Poussineau

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

2011-12-15 Thread Hervé Poussineau

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

2011-12-15 Thread Hervé Poussineau
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

2011-12-15 Thread Hervé Poussineau

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

2011-12-15 Thread Paul Brook
> 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

2011-12-15 Thread Hervé Poussineau

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

2011-12-15 Thread Hervé Poussineau

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

2011-12-15 Thread Hervé Poussineau

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

2011-12-15 Thread Hervé Poussineau

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

2011-12-15 Thread Hervé Poussineau

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

2011-12-15 Thread Hervé Poussineau

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()

2011-12-15 Thread Hervé Poussineau

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

2011-12-15 Thread Hervé Poussineau
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

2011-12-15 Thread Jordan Justen
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

2011-12-15 Thread Jordan Justen
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

2011-12-15 Thread Jordan Justen
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

2011-12-15 Thread Jordan Justen
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

2011-12-15 Thread Jordan Justen
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

2011-12-15 Thread Peter Maydell
"(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)

2011-12-15 Thread Peter Maydell
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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Peter Maydell
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

2011-12-15 Thread Paul Brook
> > 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

2011-12-15 Thread Peter Maydell
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

2011-12-15 Thread Peter Maydell
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

2011-12-15 Thread Corey Bryant



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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Anthony Liguori

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.

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Alex Williamson
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

2011-12-15 Thread Ian Jackson
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

2011-12-15 Thread Vincent Autefage
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

2011-12-15 Thread Michael Roth

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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Gerd Hoffmann
  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.

2011-12-15 Thread Luiz Capitulino
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

2011-12-15 Thread Marcelo Tosatti
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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Stefan Hajnoczi
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

2011-12-15 Thread Stefan Hajnoczi
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

2011-12-15 Thread Serge Hallyn
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

2011-12-15 Thread Daniel P. Berrange
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

2011-12-15 Thread Paul Moore
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()

2011-12-15 Thread Avi Kivity
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

2011-12-15 Thread Jiri Denemark
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

2011-12-15 Thread Avi Kivity
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()

2011-12-15 Thread Avi Kivity
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

2011-12-15 Thread Avi Kivity
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

2011-12-15 Thread Avi Kivity
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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Avi Kivity
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()

2011-12-15 Thread Avi Kivity
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

2011-12-15 Thread Avi Kivity
[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

2011-12-15 Thread Vincent Autefage
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

2011-12-15 Thread Kevin Wolf
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

2011-12-15 Thread Serge Hallyn
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.

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Gleb Natapov
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.

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Kevin Wolf
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

2011-12-15 Thread Kevin Wolf
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

2011-12-15 Thread Kevin Wolf
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

2011-12-15 Thread Luiz Capitulino
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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Kevin Wolf
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

2011-12-15 Thread Kevin Wolf
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

2011-12-15 Thread Luiz Capitulino
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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Anthony Liguori

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

2011-12-15 Thread Kevin Wolf
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

2011-12-15 Thread Jiri Denemark
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

2011-12-15 Thread Luiz Capitulino
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

2011-12-15 Thread Kevin Wolf
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




  1   2   3   >