gmane.comp.emulators.qemu

buildbot failure in qemu on default_x86_64_rhel61

2012-05-17T01:05:19

The Buildbot has detected a new failure on builder default_x86_64_rhel61 while building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/default_x86_64_rhel61/builds/266

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: kraxel_rhel61

Build Reason: The Nightly scheduler named 'nightly_default' triggered this build
Build Source Stamp: [branch master] HEAD
Blamelist: 

BUILD FAILED: failed test

sincerely,
 -The Buildbot

[RFC/PATCH] Add a memory barrier to guest memoryaccess functions

Benjamin Herrenschmidt — 2012-05-17T00:52:08

The emulated devices can run simultaneously with the guest, so
we need to be careful with ordering of load and stores done by
them to the guest system memory, which need to be observed in
the right order by the guest operating system.

In order to avoid unnecessary overhead on i386, we define a new
barrier dma_mb() which is a full barrier on powerpc and a nop
on i386 and x86_64 (see the comment I added in the code).

This barrier is then added to qemu_get_ram_ptr() which is easier
than sprinkling into all the functions that provide guest
access and are all more or less open coded.

Signed-off-by: Benjamin Herrenschmidt <benh< at >kernel.crashing.org>
---

Discussion: So the other option is to do it only in
cpu_physical_memory_rw() and leave the responsibility to use explicit
barriers to the callers of ld*/st* accessors. For example virtio already
does it in a few places explicitly.

 exec.c         |   11 +++++++++++
 qemu-barrier.h |   29 ++++++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 3 deleti

Re: [PATCH 13/13] iommu: Add a memory barrier to DMA RW function

Benjamin Herrenschmidt — 2012-05-17T00:24:53

 .../...


Finally ... something like smp_mb() in qemu will turn into a lock op or
an mfence on x86, ie not a nop.

That means overhead from today's implementation, which leads to the
question ... is today implementation correct ? IE. Is a barrier needed
on x86 as well or not ?

If not (I'm trying to figure out why exactly does x86 have a barrier in
the first place and when it's in order), then I might add a new barrier
type in qemu-barriers.h, something like dma_mb(), and define it as a nop
on x86, a lwsync or sync (still thinking about it) on ppc, and
__sync_synchronize() on unknown archs.

Any x86 guru around cares to explain me what exactly is the x86 memory
model and when does it need barriers ?

Cheers,
Ben.

Re: [PATCH 2/2] Get system state configuration from QEMU and patcth DSDT with it.

Kevin O'Connor — 2012-05-17T00:20:05

If it has to be patched anyway (eg, to remove _S3 definition) then,
yes, might as well do the whole thing via patching.


Agreed.

BTW, what's the background to the requirement to configure the Sx
sleep levels?  As we've discussed some time back, I'm leery of
building custom QEMU->SeaBIOS interfaces just so SeaBIOS can then
reencode into ACPI for the OS.


Yes.  I'd like to change this in SeaBIOS by caching the "romfile"
entries.  It would actually simplify the code.


The real benefit to using QEMU_CFG_FILE_DIR is that we can get at the
size of the data in a standard way.  Without that, each new data item
has to have its own fw_cfg reading code which is just a waste.


I've been meaning to build a qemu patch that puts all fw_cfg entries
in QEMU_CFG_FILE_DIR.  (There's no harm in making names for the
existing hard-coded fw_cfg "ports".)  Unfortunately, I haven't gotten
around to it.  :-(

-Kevin

Re: [PATCH 13/13] iommu: Add a memory barrier to DMA RW function

Benjamin Herrenschmidt — 2012-05-17T00:07:37

So followup ....

For those on the list: Anthony and I had a chat and we agree that a
better thing to do is to have all cpu_physical_memory_* accesses to be
ordered in program order from the perspective of the VCPUs. Devices that
have performance critical accesses and want to do home made ordering can
use map/unmap.

Now looking at the code, however, there seem to be a lot of duplication,
ie cpu_physical_memory_rw() is an obvious choice to add a barrier but
what about all of the ldl_*, ldq_* etc... ? In fact there's about 45
different ways code can dig into guest memory, should they all be made
ordered ?

At this point, it might be easier to just stick a barrier in
qemu_get_ram_ptr() which seems to be called by everybody however that
means that things like cpu_physical_memory_rw() will end up hitting the
barrier for every page. It's safe but it might be a performance hit
(measurable ? I can give it a try, probably not).

Or we can just sprinkle the barrier everywhere, mostly it's going to be
in exec.c, all t

Re: [PATCH next v2 00/74] QOM CPUState, part 3: CPU reset

Andreas Färber — 2012-05-16T22:04:26

Am 10.05.2012 02:13, schrieb Andreas Färber:
[...]

Seeing that no one currently cares about sh4, I have re-tested the r2d
test image and applied these to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

I'm still hoping that someone will ack the remaining mips ones though...

/-F

Re: [PATCH 13/13] iommu: Add a memory barrier to DMA RW function

Benjamin Herrenschmidt — 2012-05-16T21:12:40


BTW, this is going to also hurt SMP ARMs ... ARM is getting increasingly
out of order as well.

Cheers,
Ben.

Re: [PATCH 13/13] iommu: Add a memory barrier to DMA RW function

Benjamin Herrenschmidt — 2012-05-16T21:10:45


So the precise ordering rules of various busses can vary slightly.

We could try to get as precise & fine grained as those busses are in HW
or ... it's my belief that it makes sense to simply guarantee that the
DMA accesses done by emulated devices always appear to other VCPUs in
the order they were done by the device emulation code.

IE. If we can prove that the cost of doing so is negligible, then it's
also the simplest approach since just sticking that one barrier here
will provide that ordering guarantee (at least for anything using the
dma_* accessors).

Ordering problems can be really sneaky & nasty to debug and so I'm
really tempted to use that big hammer approach here, provided there is
no problematic performance loss.


So virtio doesn't use the dma_* interface since it bypasses the iommu
(on purpose).


Well, my idea is to provide a well defined ordering semantic of all DMA
accesses issued by a device :-) IE. All DMAs done by the device
emulation appear to other VCPUs in the order they were issue

Re: [PATCHv2 2/6] qemu-ga: don't leak a file descriptor upon failed lockf

Michael Roth — 2012-05-16T20:58:42

Acked-by: Michael Roth <mdroth< at >linux.vnet.ibm.com>

Re: [PATCH next v2 73/74] linux-user: Use cpu_reset()after cpu_init() / cpu_copy()

Igor Mammedov — 2012-05-16T20:52:42

----- Original Message -----
Yep, it certainly could be different between for other targets.
May be then it would be acceptable to do cpu_reset for target-i386
in realize at least. It matches how x86 should behave.

Thanks for expanded explanation. (/me went to read history)


And if cpu_init could be/is a combination of object_new + set properties + realize
then on some targets calling cpu_reset at the end of cpu_init or 
at the end of realize would do the thing? :)

Re: [RFC PATCH] qemu spapr-pci: added IRQ list toPCIBus

Alex Williamson — 2012-05-16T20:39:15

Right, the gap is that we can signal the interrupt in qemu, but don't
know which EOI to look for to re-enable the physical device.  Later
we'll want to know the interrupt when we inject it so we can do so via
an irqfd directly into kvm.


It actually can change dynamically on x86 due to acpi interrupt links
which allow the guest a generic way to select from a set of possible
interrupt routing schemes.  And of course a chipset driver could twiddle
bits if it wanted as well.  So, we really do need the update notifiers
from my tree that this patch drops.  pci_get_irq is one of the few
things a qemu chipset needs to implement to get device assignment with
vfio.  Doing it this way with a common array in PCIBus makes the patch
smaller, but I don't know that it really simplifies anything.  The
chipset function is trivial on x86, it's just knowing what to do that's
the problem.


How does that solve the initial problem of getting the EOI?


It indicates support?  Thanks,

Alex

Re: [RFC PATCH 2/2] Split fdd devices off the floppycontroller

Markus Armbruster — 2012-05-16T20:30:51


Thanks, I'll rebase.


Shouldn't there be one?

[...]

Yes, please.


I'll cook up something.


No problem.

I guess for q35 we could model the fdc as part of the super I/O device,
which connected to the south bridge via LPC.


Will do, thanks.


Last sentence no verb?

Re: [PATCH 13/13] iommu: Add a memory barrier to DMA RW function

Anthony Liguori — 2012-05-16T19:39:59

I must confess, I have no idea what PCI et al guarantee with respect to 
ordering.  What's nasty about this patch is that you're not just ordering wrt 
device writes/reads, but also with the other VCPUs.  I don't suspect this would 
be prohibitively expensive but it still worries me.


My concern would really be limited to virtio ring processing. It all depends on 
where you place the barriers in the end.

I really don't want to just conservatively stick barriers everywhere either. 
I'd like to have a specific ordering guarantee and then implement that and deal 
with the performance consequences.

I also wonder if the "fix" that you see from this is papering around a bigger 
problem.  Can you explain the ohci problem that led you to do this in the first 
place?

Regards,

Anthony Liguori

[PATCHv2 2/6] qemu-ga: don't leak a file descriptorupon failed lockf

Jim Meyering — 2012-05-16T20:19:55

Signed-off-by: Jim Meyering <meyering< at >redhat.com>
---

 qemu-ga.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu-ga.c b/qemu-ga.c
index 680997e..24b236a 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
< at >< at > -246,6 +246,9 < at >< at > static bool ga_open_pidfile(const char *pidfile)
     pidfd = open(pidfile, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
     if (pidfd == -1 || lockf(pidfd, F_TLOCK, 0)) {
         g_critical("Cannot lock pid file, %s", strerror(errno));
+        if (pidfd != -1) {
+            close(pidfd);
+        }
         return false;
     }

--
1.7.10.2.520.g6a4a482

Re: [PATCH 2/6] qemu-ga: avoid unconditional lockfilefile descriptor leak

Jim Meyering — 2012-05-16T20:17:31

Hi Michael,

Thanks for the feedback.  You're right.  I missed the significance
of the lockf call.  For now, I'm about to post a much less ambitious V2
patch that simply avoids the FD leak upon failed lockf.


Typically not, since if there's a primary failure,
that will be reported, and the only reason to perform
the this sort of close is to avoid an FD leak.  Additional
warning about close (aka write) failure is usually unwelcome.

Re: [RFC PATCH 2/2] Split fdd devices off the floppycontroller

Markus Armbruster — 2012-05-16T20:09:26


Sounds like a plan.

Re: Add support for new image type

Kai Meyer — 2012-05-16T17:06:01

I simply can't get over how simple it was to integrate our closed-source 
block-wise access library with qemu, or how many uses there are from 
other projects like libguestfs that would automatically gain support for 
our format. Since we last spoke, we've been working on iSCSI, but I've 
also received some conflicting interpretations on the GPL. As I learn 
more about the GPL, it is clear that the intent is to help the owners 
retain as many rights as they choose to retain. In the spirit of 
pleasing the owners of qemu, I'd like to ask two questions. We are not 
interested in playing in legal "grey" areas, so unless there is a clear 
"sure go for it" answer, we're only too happy to comply with your wishes.

1) It's been suggested to me that since we have the rights to distribute 
our closed source shared library, there is a precedence for being able 
to distributed a modified version of qemu that does run-time linking 
against our shared library. The absence or presence of our shared 
library simply enables

Re: [PATCH 08/13] iommu: Introduce IOMMU emulationinfrastructure

Anthony Liguori — 2012-05-16T19:36:00

I meant, if you wanted to have a synchronous hypercall function to dispatch, and 
then later call "hypercall_finish()", you would need a way to return an error in 
the synchronous hypercall to identify that something else would eventually call 
"hypercall_finish()."  The sync hypercall would need to return something like 
-EWOULDBLOC.

Setting env->halted=1 ought to be enough to delay returning to the guest 
although i'd have to go through the code to verify.

Regards,

Anthony Liguori

[PATCH 1.1 4/4] sheepdog: use heap instead of stackfor BDRVSheepdogState

MORITA Kazutaka — 2012-05-16T18:15:34

bdrv_create() is called in coroutine context now, so we cannot use
more stack than 1 MB in the function if we use ucontext coroutine.
This patch allocates BDRVSheepdogState, whose size is 4 MB, on the
heap in sd_create().

Signed-off-by: MORITA Kazutaka <morita.kazutaka< at >lab.ntt.co.jp>
---
 block/sheepdog.c |   35 ++++++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 42f2ee8..b614a49 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
< at >< at > -1249,24 +1249,26 < at >< at > out:
 
 static int sd_create(const char *filename, QEMUOptionParameter *options)
 {
-    int ret;
+    int ret = 0;
     uint32_t vid = 0, base_vid = 0;
     int64_t vdi_size = 0;
     char *backing_file = NULL;
-    BDRVSheepdogState s;
+    BDRVSheepdogState *s;
     char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN];
     uint32_t snapid;
     int prealloc = 0;
     const char *vdiname;
 
+    s = g_malloc0(sizeof(BDRVSheepdogState));
+
     strstart(filename, "sh

[Bug 882997] Re: 64-bit linux guests fail to start ononeiric running3.0 kernel

Serge Hallyn — 2012-05-15T21:37:39

Seems like this may be the same bug as 997311?

I want to try installing grub1 into a lucid VM and see if I can get that
to crash.

** Changed in: qemu-kvm (Ubuntu)
       Status: Incomplete => New

** Tags added: grub

Re: Add support for new image type

Kai Meyer — 2012-05-16T19:20:54

I want to respect the lines that the GPL draws, and this helps clarify 
for me where some of those lines are. To help me better understand, what 
would be the terminology used for the explanation between what I would 
call "source code" licensing, and "project" licensing? Also, where in 
the code (or rather what file) can I see this distinction? It seems like 
something critical to be aware of, and I'd like to avoid missing 
something like this in the future as I give advice on what software we 
can use.

Thanks for being patient with me.

If you would help clarify a separate point, I would be grateful. As I 
understand it, I am able to modify qemu for my own purposes (like 
testing the filesystem integrity inside a backup image by using 
guestmount to mount it). How much of that work (source code, principles, 
explanations, ect) can I share, and with whom can I share it with? For 
instance, would qemu be opposed to a StorageCraft wiki article on "How 
to add support for our Backup Images to qemu"? And would