Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Andreas Färber

Hi Michael,

Am 28.08.2007 um 21:57 schrieb Michael Matz:


the below patch let's qemu be compiled by GCC 4.2


Thanks for your effort!


Here's some justification
   of why that doesn't really cost performance: with three free  
regs
   GCC is already spilling like mad in the snippets, we just  
trade one
   of those memory accesses (to stack) with one other mem  
access to

   the cpu_state structure, which will be in cache.

   Additionally I made sure that I put the least used Tx global  
into
   memory.  I haven't done much performance measurements but  
noticed

   no glaring problems.


Am I right to assume that compiling with gcc3 will still work with  
your patch? In that case your patch would enable qemu to run on gcc4- 
only platforms (where performance doesn't matter too much yet) while  
allowing to compile with gcc3 for performance reasons where necessary.


The whole patch is against a 0.9.0-cvs version from 2007-07-09  
(Alex might

know the exact checkout date), so chances are that it still applies :)


What do you mean with 0.9.0-cvs? The 0.9.0 GCC4 patches for OSX/Intel  
don't apply to HEAD possibly due to some inline assembler changes,  
and if merged manually resulted in a crash...


Andreas




Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Johannes Schindelin
Hi,

On Tue, 28 Aug 2007, Michael Matz wrote:

> The whole patch is against a 0.9.0-cvs version from 2007-07-09 (Alex 
> might know the exact checkout date), so chances are that it still 
> applies :)

It is based on the z80 fork, but it applies relatively cleanly (one 
trailing whitespace) to the version as of "Use unsigned 32-bit load for 
ld/lduw".

However, I still get this error:

../dyngen -o op.h op.o
dyngen: ret or jmp expected at the end of op_tadd_T1_T0_ccTV
make[1]: *** [op.h] Fehler 1
make[1]: Leaving directory `/home/me/qemu/sparc-linux-user'

When only making i386-softmmu, I still get this (on SuSE 10.2):

In file included from /home/gene099/my/qemu/usb-linux.c:29:
/usr/include/linux/usbdevice_fs.h:49: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
or ‘__attribute__’ before ‘*’ token
/usr/include/linux/usbdevice_fs.h:56: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
or ‘__attribute__’ before ‘*’ token
/usr/include/linux/usbdevice_fs.h:66: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
or ‘__attribute__’ before ‘*’ token
/usr/include/linux/usbdevice_fs.h:100: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
or ‘__attribute__’ before ‘*’ token
/usr/include/linux/usbdevice_fs.h:116: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
or ‘__attribute__’ before ‘*’ token
/home/me/qemu/usb-linux.c: In function ‘usb_host_handle_data’:
/home/me/qemu/usb-linux.c:130: error: ‘struct 
usbdevfs_bulktransfer’ has no member named ‘data’
make: *** [usb-linux.o] Fehler 1

Ciao,
Dscho


Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Michael Matz
Hi,

On Wed, 29 Aug 2007, Andreas Färber wrote:

> Am I right to assume that compiling with gcc3 will still work with your 
> patch?

Yes.

> In that case your patch would enable qemu to run on gcc4-only platforms 
> (where performance doesn't matter too much yet) while allowing to 
> compile with gcc3 for performance reasons where necessary.

If necessary yes.  The patch obviously would need to be changed to be 
conditional not only on HOST_I386 but also on GCC version.  Better to do 
benchmarks first, if it's worthwhile.

> >The whole patch is against a 0.9.0-cvs version from 2007-07-09 (Alex 
> >might know the exact checkout date), so chances are that it still 
> >applies :)
> 
> What do you mean with 0.9.0-cvs? The 0.9.0 GCC4 patches for OSX/Intel 

Do you mean my patches?

> don't apply to HEAD possibly due to some inline assembler changes,

It seems target-z80 doesn't exist in CVS, apart from that there's a small 
s/target_ulong/ppc_gpr_t/ in the target-ppc hunk.  Otherwise it applies 
just fine (I'll attach the patch against CVS, in case pine managed to 
mangle it last time).

> and if merged manually resulted in a crash...

I just tested the attached patch with qemu fresh from CVS on HOST_I386 
with target i386 (no kqemu, so it's really emulating), and target mipsel.
It works just fine.  (gcc 4.1.2 for that matter)


Ciao,
Michael.Index: Makefile.target
===
RCS file: /sources/qemu/qemu/Makefile.target,v
retrieving revision 1.194
diff -u -p -r1.194 Makefile.target
--- Makefile.target	26 Aug 2007 17:45:59 -	1.194
+++ Makefile.target	29 Aug 2007 10:29:42 -
@@ -125,6 +125,7 @@ endif
 
 ifeq ($(ARCH),ppc)
 CPPFLAGS+= -D__powerpc__
+OP_CFLAGS+= -fno-section-anchors
 ifdef CONFIG_LINUX_USER
 BASE_LDFLAGS+=-Wl,-T,$(SRC_PATH)/$(ARCH).ld
 endif
Index: softmmu_header.h
===
RCS file: /sources/qemu/qemu/softmmu_header.h,v
retrieving revision 1.15
diff -u -p -r1.15 softmmu_header.h
--- softmmu_header.h	23 May 2007 19:58:10 -	1.15
+++ softmmu_header.h	29 Aug 2007 10:29:42 -
@@ -250,14 +250,18 @@ static inline void glue(glue(st, SUFFIX)
   : "r" (ptr), 
 /* NOTE: 'q' would be needed as constraint, but we could not use it
with T1 ! */
+#if DATA_SIZE == 1 || DATA_SIZE == 2
+		  "q" (v),
+#else
   "r" (v), 
+#endif
   "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS), 
   "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS), 
   "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
   "m" (*(uint32_t *)offsetof(CPUState, tlb_table[CPU_MEM_INDEX][0].addr_write)),
   "i" (CPU_MEM_INDEX),
   "m" (*(uint8_t *)&glue(glue(__st, SUFFIX), MMUSUFFIX))
-  : "%eax", "%ecx", "%edx", "memory", "cc");
+  : "%eax", "%edx", "memory", "cc");
 }
 
 #else
Index: target-alpha/cpu.h
===
RCS file: /sources/qemu/qemu/target-alpha/cpu.h,v
retrieving revision 1.4
diff -u -p -r1.4 cpu.h
--- target-alpha/cpu.h	3 Jun 2007 21:02:37 -	1.4
+++ target-alpha/cpu.h	29 Aug 2007 10:29:43 -
@@ -278,6 +278,8 @@ struct CPUAlphaState {
  * used to emulate 64 bits target on 32 bits hosts
  */ 
 target_ulong t0, t1, t2;
+#elif defined(HOST_I386)
+target_ulong t2;
 #endif
 /* */
 double ft0, ft1, ft2;
Index: target-alpha/exec.h
===
RCS file: /sources/qemu/qemu/target-alpha/exec.h,v
retrieving revision 1.2
diff -u -p -r1.2 exec.h
--- target-alpha/exec.h	3 Jun 2007 17:44:36 -	1.2
+++ target-alpha/exec.h	29 Aug 2007 10:29:43 -
@@ -40,7 +40,11 @@ register struct CPUAlphaState *env asm(A
 
 register uint64_t T0 asm(AREG1);
 register uint64_t T1 asm(AREG2);
+#ifndef HOST_I386
 register uint64_t T2 asm(AREG3);
+#else
+#define T2 (env->t2)
+#endif
 
 #endif /* TARGET_LONG_BITS > HOST_LONG_BITS */
 
Index: target-alpha/op_template.h
===
RCS file: /sources/qemu/qemu/target-alpha/op_template.h,v
retrieving revision 1.1
diff -u -p -r1.1 op_template.h
--- target-alpha/op_template.h	5 Apr 2007 06:58:33 -	1.1
+++ target-alpha/op_template.h	29 Aug 2007 10:29:43 -
@@ -28,7 +28,26 @@ void OPPROTO glue(op_reset_T, REG) (void
 
 void OPPROTO glue(op_reset_FT, REG) (void)
 {
+#ifdef HOST_PPC
+/* We have a problem with HOST_PPC here:
+   We want this code:
+ glue(FT, REG) = 0;
+   unfortunately GCC4 notices that this stores (double)0.0 into
+   env->ft0 and emits that constant into the .rodata, and instructions
+   to load that zero from there.  But that construct can't be parsed by dyngen.
+   We could add -ffast-math for compiling op.c, that would just make it generate
+   two stores of zeros into both words of ft0.  But -ffast-m

Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Michael Matz
Hi,

On Wed, 29 Aug 2007, Johannes Schindelin wrote:

> > The whole patch is against a 0.9.0-cvs version from 2007-07-09 (Alex 
> > might know the exact checkout date), so chances are that it still 
> > applies :)
> 
> It is based on the z80 fork, but it applies relatively cleanly (one 
> trailing whitespace) to the version as of "Use unsigned 32-bit load for 
> ld/lduw".
> 
> However, I still get this error:
> 
> ../dyngen -o op.h op.o
> dyngen: ret or jmp expected at the end of op_tadd_T1_T0_ccTV
> make[1]: *** [op.h] Fehler 1
> make[1]: Leaving directory `/home/me/qemu/sparc-linux-user'

Using SuSE 10.2, i.e. gcc 4.1.2?  For qemu CVS (see my other mail) I 
didn't test linux-user at all.  In our qemu package itself (which builds 
also the linux-user parts) there are some more patches which might fix the 
above problem, don't know yet.  I would try --disable-linux-user in your 
case.  I can look at it later.

> 
> When only making i386-softmmu, I still get this (on SuSE 10.2):
> 
> In file included from /home/gene099/my/qemu/usb-linux.c:29:
> /usr/include/linux/usbdevice_fs.h:49: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
> or ‘__attribute__’ before ‘*’ token
> /usr/include/linux/usbdevice_fs.h:56: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
> or ‘__attribute__’ before ‘*’ token
> /usr/include/linux/usbdevice_fs.h:66: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
> or ‘__attribute__’ before ‘*’ token
> /usr/include/linux/usbdevice_fs.h:100: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
> or ‘__attribute__’ before ‘*’ token
> /usr/include/linux/usbdevice_fs.h:116: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
> or ‘__attribute__’ before ‘*’ token
> /home/me/qemu/usb-linux.c: In function ‘usb_host_handle_data’:
> /home/me/qemu/usb-linux.c:130: error: ‘struct 
> usbdevfs_bulktransfer’ has no member named ‘data’
> make: *** [usb-linux.o] Fehler 1

Yes, that's a problem of the kernel headers on 10.2.  You can work around 
this with the below snippet.


Ciao,
Michael.
-- 
Index: usb-linux.c
===
RCS file: /sources/qemu/qemu/usb-linux.c,v
retrieving revision 1.10
diff -u -p -r1.10 usb-linux.c
--- usb-linux.c 10 Dec 2006 22:11:04 -  1.10
+++ usb-linux.c 29 Aug 2007 11:45:13 -
@@ -26,6 +26,7 @@
 #if defined(__linux__)
 #include 
 #include 
+#define __user
 #include 
 #include 



Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Johannes Schindelin
Hi,

On Wed, 29 Aug 2007, Michael Matz wrote:

> On Wed, 29 Aug 2007, Johannes Schindelin wrote:
> 
> > > The whole patch is against a 0.9.0-cvs version from 2007-07-09 (Alex 
> > > might know the exact checkout date), so chances are that it still 
> > > applies :)
> > 
> > It is based on the z80 fork, but it applies relatively cleanly (one 
> > trailing whitespace) to the version as of "Use unsigned 32-bit load 
> > for ld/lduw".
> > 
> > However, I still get this error:
> > 
> > ../dyngen -o op.h op.o
> > dyngen: ret or jmp expected at the end of op_tadd_T1_T0_ccTV
> > make[1]: *** [op.h] Fehler 1
> > make[1]: Leaving directory `/home/me/qemu/sparc-linux-user'
> 
> Using SuSE 10.2, i.e. gcc 4.1.2?

Yep.  It would be good to fix this error before applying the patch to CVS 
;-)

> > When only making i386-softmmu, I still get this (on SuSE 10.2):
> > 
> > In file included from /home/gene099/my/qemu/usb-linux.c:29:
> > /usr/include/linux/usbdevice_fs.h:49: error: expected ‘:’, ‘,’, ‘;’, ‘}’ 
> > or ‘__attribute__’ before ‘*’ token
> > [...]
> 
> Yes, that's a problem of the kernel headers on 10.2.  You can work 
> around this with the below snippet.

I did it differently now.

My changes can be seen (and fetched) at

http://repo.or.cz/qemu/dscho.git/

in the "gcc4" branch.  This is just your patch applied to the revision I 
mentioned (which is available in the "gcc4-original" branch), then rebased 
onto current CVS (or what Jens Axboe's clone is current at right now).  I 
adjusted one target_ulong, and added another commit for the 
linux/compiler.h issue.

Ciao,
Dscho


Re: [Qemu-devel] Re: [et-mgmt-tools] Image Corruption Possible with qemu and qemu-kvm

2007-08-29 Thread Sven Oehme
[EMAIL PROTECTED] wrote on 08/28/2007 
04:13:02 AM:

> Anthony Liguori wrote:
> >> In the scenario you mention, libvirt should probably do a sanity 
check for
> >> this before letting you start the guest. libvirt already supports the 
idea
> >> of 'shared' disk images where two or more guests can be 
> optionally configured 
> >> to have write access - basically assumes the admin requesting sharing 
knows
> >> what they're doing.
> >> 
> >
> > I think this is the right level myself.  Advisory locks work okay but
> > not all filesystems support them.  It's particularly nasty when you 
have
> > a clustered filesystem in the host.  I think it would do more harm 
than
> > good to have a feature like that was supposed to provide a safe-guard
> > but then frequently didn't work.
> > 
> 
> There's still the unmanaged use case to worry about.  I think qemu can 
> default to advisory locking, and management tools can do their own 
> locking and always override qemu.
> 
> It's too easy to kill an image by starting up another instance right 
now.
> 

i agree default should be advisory locking and a switch to disable it ..
would that be hard to implement ?

thanks. Sven

Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Andreas Färber


Am 29.08.2007 um 13:40 schrieb Michael Matz:


The whole patch is against a 0.9.0-cvs version from 2007-07-09 (Alex
might know the exact checkout date), so chances are that it still
applies :)


What do you mean with 0.9.0-cvs? The 0.9.0 GCC4 patches for OSX/Intel


Do you mean my patches?


No. Are yours already OSX-compatible? I referred to previous patches,  
specifically those currently applied for Q:

http://www.kju-app.org/proj/browser/trunk/patches
There were also some in the QEMU forums. Those only apply to the  
0.9.0 tag.


So I was wondering whether 0.9.0-cvs from date meant 0.9.0 tag or  
HEAD as of date. Sorry if this is obvious but I'm used to Subversion.  
In your patch I first read qemu-0.9.0, which confused me as I don't  
consider CVS HEAD to be 0.9.0. Will try your updated patch later.


Best regards,

Andreas




Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Johannes Schindelin
Hi,

On Wed, 29 Aug 2007, Andreas F?rber wrote:

> I referred to previous patches, specifically those currently applied for 
> Q: http://www.kju-app.org/proj/browser/trunk/patches

Is there _any_ way to get at the raw diffs instead of some @!%! marked up 
HTML abomination that cannot be applied?

Ciao,
Dscho





Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Michael Matz
Hi,

On Wed, 29 Aug 2007, Andreas Färber wrote:

> > >What do you mean with 0.9.0-cvs? The 0.9.0 GCC4 patches for OSX/Intel
> >
> >Do you mean my patches?
> 
> No. Are yours already OSX-compatible?

No OSX, no idea :)

> I referred to previous patches, specifically those currently applied for 
> Q: http://www.kju-app.org/proj/browser/trunk/patches There were also 
> some in the QEMU forums. Those only apply to the 0.9.0 tag.

I see.  We seem to use some of these patches (perhaps in a modified form) 
in our qemu package too.  Sorry that I'm so unsure about the qemu topics, 
I'm only a compiler developer hitting qemu hard enough until it works with 
gcc4 :-)

> So I was wondering whether 0.9.0-cvs from date meant 0.9.0 tag or HEAD 
> as of date.

It was supposed to mean some CVS checkout from after 0.9.0 release, where 
I myself couldn't really make out at which date the checkout was done 
(because it wasn't done by me, and I didn't feel like digging too long, as 
the patch obviously still applied mostly).

> Sorry if this is obvious but I'm used to Subversion. In your patch I 
> first read qemu-0.9.0, which confused me as I don't consider CVS HEAD to 
> be 0.9.0. Will try your updated patch later.

That btw. is against CVS HEAD (as of today).  I have no idea what's 
necessary for OSX, it might be that you need additional patches, or not, 
can't help with that.


Ciao,
Michael.

Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Mulyadi Santosa

Hi ...

Thanks for your effor Michael! Now, I only hope, one of the patches that 
makes qemu gcc4 compliant are soon merged. Or, is there any plan to 
merge qops?


regards,

Mulyadi





Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Johannes Schindelin
Hi,

On Wed, 29 Aug 2007, Mulyadi Santosa wrote:

> Thanks for your effor Michael! Now, I only hope, one of the patches that
> makes qemu gcc4 compliant are soon merged.

Well, to throw a spanner in the works: this patch is the 4th patch along 
the lines that I came about.  None of them (AFAICT) was tested well enough 
to be included in CVS.  Indeed, the biggest problem seems to be to make a 
patch that not only works on the machine of the poster, but on other 
machines, too.

> Or, is there any plan to merge qops?

I was pretty enthusiastic about qops a while ago.  But then I saw that 
they hurt performance.  And performance is the primary reason I use QEmu, 
so I am not at all convinced that this is the way to go.  Even if parts 
have been merged already.

Ciao,
Dscho





Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Ronald

Johannes Schindelin schreef:

Hi,

On Wed, 29 Aug 2007, Andreas F?rber wrote:

  
I referred to previous patches, specifically those currently applied for 
Q: http://www.kju-app.org/proj/browser/trunk/patches



Is there _any_ way to get at the raw diffs instead of some @!%! marked up 
HTML abomination that cannot be applied?


Ciao,
Dscho




  

Click a link to a patch and check below the page:


 Download in other formats:

   * Plain Text
 

   * Original Format
 






Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Johannes Schindelin
Hi,

On Wed, 29 Aug 2007, Ronald wrote:

> Johannes Schindelin schreef:
> 
> > On Wed, 29 Aug 2007, Andreas F?rber wrote:
> >   
> > > I referred to previous patches, specifically those currently applied 
> > > for Q: http://www.kju-app.org/proj/browser/trunk/patches
> > 
> > Is there _any_ way to get at the raw diffs instead of some @!%! marked 
> > up HTML abomination that cannot be applied?
>   
> Click a link to a patch and check below the page:

Thanks.  Somehow that slipped by me.

I'll try to put them on my repo.or.cz repository, too.

Ciao,
Dscho




Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Andreas Färber

Hi,

Am 29.08.2007 um 15:59 schrieb Johannes Schindelin:

I referred to previous patches, specifically those currently  
applied for

Q: http://www.kju-app.org/proj/browser/trunk/patches


Is there _any_ way to get at the raw diffs instead of some @!%!  
marked up

HTML abomination that cannot be applied?


There's a download link at the bottom of each HTML page. Or  
alternatively check the relevant files out via anonymous SVN.


For applying them have a look at the ../build_i386.sh script and note  
that the script is simple and does not handle patch errors so needs  
to be changed to use.


Andreas




Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Andreas Färber


Am 29.08.2007 um 16:19 schrieb Johannes Schindelin:

I referred to previous patches, specifically those currently  
applied

for Q: http://www.kju-app.org/proj/browser/trunk/patches


I'll try to put them on my repo.or.cz repository, too.


Note that from briefly looking at them they appear to contain some  
permanent changes, i.e. some of them should probably only be applied  
on the respective platform itself.


Regards,
Andreas




Re: [Qemu-devel] Re: [et-mgmt-tools] Image Corruption Possible with qemu and qemu-kvm

2007-08-29 Thread Paul Jakma

On Mon, 27 Aug 2007, Anthony Liguori wrote:


I think this is the right level myself.  Advisory locks work okay but
not all filesystems support them.  It's particularly nasty when you have
a clustered filesystem in the host.  I think it would do more harm than
good to have a feature like that was supposed to provide a safe-guard
but then frequently didn't work.

   ^^

Are you trying to say that any kind of significant portion of QEMU 
users have clustered file-systems?


I think that's unlikely, and it'd be nice if QEMU by default did a 
fcntl() on writeable image files to lock itself from multiple access, 
that'd benefit the vast majority of users.


Let the 0.x% of users who need to run with weird/esoteric fses cope..

regards,
--
Paul Jakma  [EMAIL PROTECTED]   [EMAIL PROTECTED]   Key ID: 64A2FF6A
Fortune:
Fashions have done more harm than revolutions.
-- Victor Hugo




Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Paul Brook
>I solved that by placing one of the T[012] operands into memory
>for HOST_I386, thereby freeing one reg.  Here's some justification
>of why that doesn't really cost performance: with three free regs
>GCC is already spilling like mad in the snippets, we just trade one
>of those memory accesses (to stack) with one other mem access to
>the cpu_state structure, which will be in cache.

Do you have any evidence to support this claim? Last time I did this it caused 
a significant performance hit. I'd guess that most common ops are simple 
enough that we don't need more than 3 registers.

> --- qemu-0.9.0.cvs.orig/softmmu_header.h
> -                  : "%eax", "%ecx", "%edx", "memory", "cc");
> +                  : "%eax", "%edx", "memory", "cc");

This change is wrong. The inline asm calls C code which clobbers %ecx.

Paul




Re: [Qemu-devel] Re: [et-mgmt-tools] Image Corruption Possible with qemu and qemu-kvm

2007-08-29 Thread Alexander Graf
Paul Jakma wrote:
> On Mon, 27 Aug 2007, Anthony Liguori wrote:
>
>> I think this is the right level myself.  Advisory locks work okay but
>> not all filesystems support them.  It's particularly nasty when you have
>> a clustered filesystem in the host.  I think it would do more harm than
>> good to have a feature like that was supposed to provide a safe-guard
>> but then frequently didn't work.
>^^
>
> Are you trying to say that any kind of significant portion of QEMU
> users have clustered file-systems?
>
> I think that's unlikely, and it'd be nice if QEMU by default did a
> fcntl() on writeable image files to lock itself from multiple access,
> that'd benefit the vast majority of users.
>
> Let the 0.x% of users who need to run with weird/esoteric fses cope..
>
> regards,
I'm usually running one "main" qemu instance that has read/write access
to the disk file and several others for tests that use the -snapshot
option, so I think it's very important to have an easy means of
switching this check off.

just my 2 cents,

Alexander Graf




Re: [Qemu-devel] Re: [et-mgmt-tools] Image Corruption Possible with qemu and qemu-kvm

2007-08-29 Thread Paul Jakma

On Wed, 29 Aug 2007, Alexander Graf wrote:


Paul Jakma wrote:



I think that's unlikely, and it'd be nice if QEMU by default did a
fcntl() on writeable image files to lock itself from multiple access,

   

I'm usually running one "main" qemu instance that has read/write 
access to the disk file and several others for tests that use the 
-snapshot option, so I think it's very important to have an easy 
means of switching this check off.


Of course. Though, I said 'writeable' - so you might not need to use 
such an option.. ;)


regards,
--
Paul Jakma  [EMAIL PROTECTED]   [EMAIL PROTECTED]   Key ID: 64A2FF6A
Fortune:
  This report is filled with omissions.




[Qemu-devel] CC_DST problem

2007-08-29 Thread Alexander Graf
Hi,

I'm still trying to implement SVM correctly and hit a serious problem.
If I set CC_OP to EFLAGS / DYNAMIC after each instruction (so most
conditional operations are based on EFLAGS) everything works as expected.
If using CC_OP==CC_OP_EFLAGS only CC_SRC should be used and CC_DST is
supposed to be completely ignored.

So I set CC_DST to 0 (this happens when leaving and rejoining the
virtual machine, so this is the real problem) and if I do that, I get
funny segmentation faults in x86_64 guest userspace programs running in
the virtual machine (this is exactly what I see in kvm with my current
patchset as well), while 32 bit userspace programs simply hang.
So I guess this is the real problem.

Is there any logical reason CC_DST could be used with CC_OP==CC_OP_EFLAGS?

Attached to this email you will find a small patch that triggers this
problem.

Thanks for any reply that could help on this,

Alexander Graf
Index: qemu/target-i386/op.c
===
--- qemu.orig/target-i386/op.c
+++ qemu/target-i386/op.c
@@ -1248,6 +1248,13 @@ void OPPROTO op_movl_crN_T0(void)
 helper_movl_crN_T0(PARAM1);
 }
 
+void OPPROTO op_geneflags(void)
+{
+CC_SRC = cc_table[CC_OP].compute_all();
+CC_DST = 0;
+CC_OP = CC_OP_EFLAGS;
+}
+
 #if !defined(CONFIG_USER_ONLY) 
 void OPPROTO op_movtl_T0_cr8(void)
 {
Index: qemu/target-i386/translate.c
===
--- qemu.orig/target-i386/translate.c
+++ qemu/target-i386/translate.c
@@ -3154,6 +3154,12 @@ static target_ulong disas_insn(DisasCont
 target_ulong next_eip, tval;
 int rex_w, rex_r;
 
+ // DEBUG
+if (s->cc_op != CC_OP_DYNAMIC)
+gen_op_set_cc_op(s->cc_op);
+		gen_op_geneflags();
+		s->cc_op = CC_OP_DYNAMIC;
+ ///
 s->pc = pc_start;
 prefixes = 0;
 aflag = s->code32;


Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Michael Matz
Hi,

On Wed, 29 Aug 2007, Johannes Schindelin wrote:

> > Thanks for your effor Michael! Now, I only hope, one of the patches 
> > that makes qemu gcc4 compliant are soon merged.
> 
> Well, to throw a spanner in the works: this patch is the 4th patch along 
> the lines that I came about.  None of them (AFAICT) was tested well 
> enough to be included in CVS.  Indeed, the biggest problem seems to be 
> to make a patch that not only works on the machine of the poster, but on 
> other machines, too.

Please?  qemu with that patch builds on four architectures, and tests fine 
on two of them (including cross targets) and only because I haven't run it 
on the other two architectures yet.  It might or might not need more 
patches than just mine (as I started from our package), but in that case 
they are independend of making qemu work with gcc 4, which is the only 
thing my patch is concerned about.


Ciao,
Michael.




Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Johannes Schindelin
Hi,

On Wed, 29 Aug 2007, Michael Matz wrote:

> On Wed, 29 Aug 2007, Johannes Schindelin wrote:
> 
> > > Thanks for your effor Michael! Now, I only hope, one of the patches 
> > > that makes qemu gcc4 compliant are soon merged.
> > 
> > Well, to throw a spanner in the works: this patch is the 4th patch along 
> > the lines that I came about.  None of them (AFAICT) was tested well 
> > enough to be included in CVS.  Indeed, the biggest problem seems to be 
> > to make a patch that not only works on the machine of the poster, but on 
> > other machines, too.
> 
> Please?  qemu with that patch builds on four architectures, and tests 
> fine on two of them (including cross targets) and only because I haven't 
> run it on the other two architectures yet.

Hey, I am not criticising you!  Instead, I am thankful enough that I went 
so far as to expose it with git.

> It might or might not need more patches than just mine (as I started 
> from our package), but in that case they are independend of making qemu 
> work with gcc 4, which is the only thing my patch is concerned about.

I already reported the non-compiling state of sparc-linux-user...  Any 
idea what I could do about it?

Ciao,
Dscho





Re: [Qemu-devel] Re: PATCH, RFC: Generic DMA framework

2007-08-29 Thread Blue Swirl
On 8/28/07, Paul Brook <[EMAIL PROTECTED]> wrote:
> > On second thought, there is a huge difference between a write access
> > originating from CPU destined for the device and the device writing to
> > main memory. The CPU address could be 0xf000 1000, which may translate
> > to a bus address of 0x1000, as an example. The device could write to
> > main memory using the same bus address 0x1000, but this time the IOMMU
> > would map this to for example 0x1234 5000, or without an IOMMU it
> > would be just 0x1000.
>
> While your concern is valid, your example is not.
>
> You can't have the same bus address mapping onto both a device and main
> memory. Your example works if e.g. IO bus address 0x2000 1000 (or worse still
> 0xf000 1000) maps onto system memory 0x1234 5000.

This is a bit mysterious for me too. SBus address space is 28 bits
(256MB). Usually each slot maps to a different area. So the CPU sees
one slot for example at 0x3000  and other at 0x4000 .

IOMMU can map max 2G of memory, usually a 32 or 64MB region. For the
devices, this device virtual memory access (DVMA) space exists at the
top of address space (for example 0xfc00 ). Each page can map to a
different address. But these mappings can not be seen from CPU, for
example the boot prom is located at 0xffd0 . I wonder how the
devices access the DVMA space in case of >256M DVMA.

The device can't obviously supply the address bits 28-31, I don't know
where they come from (=1?). But from tracing Linux I'm pretty sure
that the bus address can be 0 disregarding the higher bits and also
the device (or device FCode prom more likely) can exist at that
location. How? Maybe IOMMU does not see CPU accesses at all and the
devices see neither each other nor themselves, so it's not a really a
shared bus?

> Conceptually you can have a separate IOMMU on every bus-bus or bus/host
> bridge, with asymmetric mappings depending where the transaction originates.

IOMMU on Sun4m maps DVMA addresses to physical addresses, which (I
think) in turn can be other device's registers or memory, but the
mappings are same for all devices.




Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Michael Matz
Hi,

On Wed, 29 Aug 2007, Paul Brook wrote:

> >I solved that by placing one of the T[012] operands into memory
> >for HOST_I386, thereby freeing one reg.  Here's some justification
> >of why that doesn't really cost performance: with three free regs
> >GCC is already spilling like mad in the snippets, we just trade one
> >of those memory accesses (to stack) with one other mem access to
> >the cpu_state structure, which will be in cache.
> 
> Do you have any evidence to support this claim?

Not really, only an apple and orange comparison.  A 1 iteration 
tests/sha1 run in the same Linux image, with -no-kqemu, on host and target 
i386:  time ./sha1

with qemu-0.8.2 (compiled by gcc 3.3-hammer): 7.92 seconds
with qemu-0.9.0-cvs (gcc4.1 compiled, with the patch): 8.15 seconds

I'll try to get a better comparison.  Note, though, that this is the only 
easy solution.  Any other solution would either involve improving reload 
pretty much, or rewriting many of the op.c patterns (for all targets) to 
never require more than three registers, and ensure that gcc doesn't 
become clever and mashes some insns together again (and in trying to do 
that probably slow down the snippets again).  Or doing away with dyngen at 
all.  All three solutions are fairly involved, so I'm personally fine with 
the above slowdown (for i386 targets you won't normally get any noticable 
slowdown as you'd use kqemu).

> Last time I did this it caused a significant performance hit. I'd guess 
> that most common ops are simple enough that we don't need more than 3 
> registers.

For i386 target I'm redefining only T1 (as I figured that A0 and T0 are 
used a bit more), it might be that some of the code could be generated in 
a way to not make use of T1 too much, I haven't really poked at that.

> > --- qemu-0.9.0.cvs.orig/softmmu_header.h
> > -                  : "%eax", "%ecx", "%edx", "memory", "cc");
> > +                  : "%eax", "%edx", "memory", "cc");
> 
> This change is wrong. The inline asm calls C code which clobbers %ecx.

Indeed, must have been blind.  Okay these are too many clobbers for poor 
gcc4 nevertheless, so a push/pop %ecx around the call needs to be done.  
Courious that this didn't lead to fire all over the place.  See patch 
below.


Ciao,
Michael.

Index: softmmu_header.h
===
RCS file: /sources/qemu/qemu/softmmu_header.h,v
retrieving revision 1.15
diff -u -p -r1.15 softmmu_header.h
--- softmmu_header.h23 May 2007 19:58:10 -  1.15
+++ softmmu_header.h29 Aug 2007 17:27:37 -
@@ -230,9 +230,11 @@ static inline void glue(glue(st, SUFFIX)
 #else
 #error unsupported size
 #endif
+ "pushl %%ecx\n"
   "pushl %6\n"
   "call %7\n"
   "popl %%eax\n"
+  "popl %%ecx\n"
   "jmp 2f\n"
   "1:\n"
   "addl 8(%%edx), %%eax\n"
@@ -250,14 +252,18 @@ static inline void glue(glue(st, SUFFIX)
   : "r" (ptr), 
 /* NOTE: 'q' would be needed as constraint, but we could not use it
with T1 ! */
+#if DATA_SIZE == 1 || DATA_SIZE == 2
+ "q" (v),
+#else
   "r" (v), 
+#endif
   "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS), 
   "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS), 
   "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
   "m" (*(uint32_t *)offsetof(CPUState, 
tlb_table[CPU_MEM_INDEX][0].addr_write)),
   "i" (CPU_MEM_INDEX),
   "m" (*(uint8_t *)&glue(glue(__st, SUFFIX), MMUSUFFIX))
-  : "%eax", "%ecx", "%edx", "memory", "cc");
+  : "%eax", "%edx", "memory", "cc");
 }
 
 #else

Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Anthony Liguori
On Tue, 2007-08-28 at 21:57 +0200, Michael Matz wrote:
> Hi,
> 
> [please keep me CCed, I'm not on this list]
> 
> the below patch let's qemu be compiled by GCC 4.2 (probably also 4.1 and 
> others) for most hosts (i386,x86_64,ia64,ppc).  s390 as host is missing, 
> and needs a compiler change to emit the literal store inline again, as the 
> literal pool at the end fundamentally breaks the assumption that qemu can 
> paste together the code snippets by patching out the return.  I have no 
> HOST_{ARM,MIPS*,ALPHA,SPARC*,M68K} machines to compile for that.

There have been similar patches to this.  While QEMU will compile,
dyngen still will generate invalid code due to ret's in the middle of
translation blocks.  Your patch doesn't seem to address this.  How do
you cope with this?

Regards,

Anthony Liguori

> It specifically changes these things:
> 
> * ppc: adds -fno-section-anchors to OP_CFLAGS, as dyngen isn't prepared
>to deal with the relocs resulting from using section anchors
> * ppc: on target-alpha op_reset_FT GCC4 uses a floating point constant 0.0
>to reset the ft regs, which in turn is loaded from the data 
>section.  The reloc for that is unhandled.  Using -ffast-math would 
>work around this, but I chose to be conservative and change only
>the op.c snippet in question.  See the comment there.
> * i386: well, most of you will know that GCC4 doesn't compile qemu because 
>of reload.  The inherent problem is, that qemu uses 64bit
>entities in some places (sometimes structs), which GCC (4.x) 
>manages to place in registers, i.e. needs 2 hardregs.  But it 
>sometimes just so happens that an instruction needing such DImode
>reg also has a memory operand with an indexed address (reg plus 
>reg), hence two hardregs more.  But qemu by default leaves just 
>three free registers for compiling op.c --> boom.  This is somewhat 
>hard to work around in GCC (trust me :) ).
> 
>I solved that by placing one of the T[012] operands into memory
>for HOST_I386, thereby freeing one reg.  Here's some justification 
>of why that doesn't really cost performance: with three free regs
>GCC is already spilling like mad in the snippets, we just trade one
>of those memory accesses (to stack) with one other mem access to 
>the cpu_state structure, which will be in cache.
> 
>Additionally I made sure that I put the least used Tx global into 
>memory.  I haven't done much performance measurements but noticed 
>no glaring problems.
> 
>Two more obvious changes in an inline asm in softmmu_header.h were 
>necessary too.
> 
> A qemu with that patch was tested on HOST_I386 with all images from the 
> qemu homepage (i.e. arm, mips, mipsel, coldfire (that doesn't work) and 
> sparc), some i386 boot isos and a freedos image.  It also was tested on 
> HOST_X86_64 installing openSUSE beta-something for i386 and x86_64.  I 
> haven't yet tested it on ia64 or ppc hosts.
> 
> The whole patch is against a 0.9.0-cvs version from 2007-07-09 (Alex might 
> know the exact checkout date), so chances are that it still applies :)
> 
> 
> Ciao,
> Michael.
> 
> diff -urp qemu-0.9.0.cvs.orig/softmmu_header.h qemu-0.9.0.cvs/softmmu_header.h
> --- qemu-0.9.0.cvs.orig/softmmu_header.h  2007-08-21 18:58:00.0 
> +0200
> +++ qemu-0.9.0.cvs/softmmu_header.h   2007-08-21 20:40:23.0 +0200
> @@ -254,14 +254,18 @@ static inline void glue(glue(st, SUFFIX)
>: "r" (ptr), 
>  /* NOTE: 'q' would be needed as constraint, but we could not use it
> with T1 ! */
> +#if DATA_SIZE == 1 || DATA_SIZE == 2
> +   "q" (v),
> +#else
>"r" (v), 
> +#endif
>"i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS), 
>"i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS), 
>"i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
>"m" (*(uint32_t *)offsetof(CPUState, 
> tlb_table[CPU_MEM_INDEX][0].addr_write)),
>"i" (CPU_MEM_INDEX),
>"m" (*(uint8_t *)&glue(glue(__st, SUFFIX), MMUSUFFIX))
> -  : "%eax", "%ecx", "%edx", "memory", "cc");
> +  : "%eax", "%edx", "memory", "cc");
>  }
>  
>  #else
> diff -urp qemu-0.9.0.cvs.orig/target-alpha/cpu.h 
> qemu-0.9.0.cvs/target-alpha/cpu.h
> --- qemu-0.9.0.cvs.orig/target-alpha/cpu.h2007-06-03 23:02:37.0 
> +0200
> +++ qemu-0.9.0.cvs/target-alpha/cpu.h 2007-08-22 00:54:15.0 +0200
> @@ -278,6 +278,8 @@ struct CPUAlphaState {
>   * used to emulate 64 bits target on 32 bits hosts
>   */ 
>  target_ulong t0, t1, t2;
> +#elif defined(HOST_I386)
> +target_ulong t2;
>  #endif
>  /* */
>  double ft0, ft1, ft2;
> diff -urp qemu-0.9.0.cvs.orig/target-alpha/exec.h 
> qemu-0.9.0.cvs/target-alpha/exec.h
> --- qemu-0.9.0.cvs.orig/target

Re: [Qemu-devel] [patch] make qemu work with GCC 4

2007-08-29 Thread Blue Swirl
On 8/29/07, Johannes Schindelin <[EMAIL PROTECTED]> wrote:
> Hi,
>
> On Wed, 29 Aug 2007, Michael Matz wrote:
>
> > On Wed, 29 Aug 2007, Johannes Schindelin wrote:
> >
> > > > Thanks for your effor Michael! Now, I only hope, one of the patches
> > > > that makes qemu gcc4 compliant are soon merged.
> > >
> > > Well, to throw a spanner in the works: this patch is the 4th patch along
> > > the lines that I came about.  None of them (AFAICT) was tested well
> > > enough to be included in CVS.  Indeed, the biggest problem seems to be
> > > to make a patch that not only works on the machine of the poster, but on
> > > other machines, too.
> >
> > Please?  qemu with that patch builds on four architectures, and tests
> > fine on two of them (including cross targets) and only because I haven't
> > run it on the other two architectures yet.
>
> Hey, I am not criticising you!  Instead, I am thankful enough that I went
> so far as to expose it with git.
>
> > It might or might not need more patches than just mine (as I started
> > from our package), but in that case they are independend of making qemu
> > work with gcc 4, which is the only thing my patch is concerned about.
>
> I already reported the non-compiling state of sparc-linux-user...  Any
> idea what I could do about it?

With the attached patch I can run Sparc32 and Sparc64 emulators on x86_64 host.

The performance feels slightly worse. Maybe the patch should be
conditional on GCC >= 4?

I think that the decision of using host registers vs. env fields
should be done in one file at top level.
Index: qemu/target-sparc/op.c
===
--- qemu.orig/target-sparc/op.c	2007-08-29 17:57:21.0 +
+++ qemu/target-sparc/op.c	2007-08-29 17:58:00.0 +
@@ -520,8 +520,11 @@
 {
 target_ulong src1;
 
-if ((T0 & 0x03) || (T1 & 0x03))
+if ((T0 & 0x03) || (T1 & 0x03)) {
 raise_exception(TT_TOVF);
+FORCE_RET();
+return;
+}
 
 src1 = T0;
 T0 += T1;


Re: [Qemu-devel] Re: PATCH, RFC: Generic DMA framework

2007-08-29 Thread Paul Brook
> This is a bit mysterious for me too. SBus address space is 28 bits
> (256MB). Usually each slot maps to a different area. So the CPU sees
> one slot for example at 0x3000  and other at 0x4000 .
>
> IOMMU can map max 2G of memory, usually a 32 or 64MB region. For the
> devices, this device virtual memory access (DVMA) space exists at the
> top of address space (for example 0xfc00 ). Each page can map to a
> different address. But these mappings can not be seen from CPU, for
> example the boot prom is located at 0xffd0 . I wonder how the
> devices access the DVMA space in case of >256M DVMA.
>
> The device can't obviously supply the address bits 28-31, I don't know 
> where they come from (=1?). But from tracing Linux I'm pretty sure
> that the bus address can be 0 disregarding the higher bits and also
> the device (or device FCode prom more likely) can exist at that
> location. How? Maybe IOMMU does not see CPU accesses at all and the
> devices see neither each other nor themselves, so it's not a really a
> shared bus?

I can't find a copy of the SBus specification, so I'm guessing how this fits 
together.

The key bit is that SBus controller performs device selection. c.f. PCI and 
ISA where each device does full address decoding.
What information I've found indicates that SBus supports an unlimited number 
of slave devices, and master devices use a 32-bit virtual address space.

This leads me to the conclusion that it's as if each slave device is on its 
own 28-bit bus, and the sbus devices master transactions go via the IOMMU 
onto the CPU bus. From there they may be routed back to an SBus device.
Actual implementation may need to do some short-circuiting to prevent 
deadlock, so I'm not entirely sure about this.

If this is the case, it means we don't need anything complicated. Devices map 
themselves straight into the system address space at the appropriate slot 
address (no plug-n-play to worry about), and device "DMA" goes via the IOMMU.

Because devices do not do address decoding I suspect this isn't going to 
nicely fit into a generic bus framework that would work for most systems.

Paul




Re: [Qemu-devel] CC_DST problem

2007-08-29 Thread Fabrice Bellard

Alexander Graf wrote:

Hi,

I'm still trying to implement SVM correctly and hit a serious problem.
If I set CC_OP to EFLAGS / DYNAMIC after each instruction (so most
conditional operations are based on EFLAGS) everything works as expected.
If using CC_OP==CC_OP_EFLAGS only CC_SRC should be used and CC_DST is
supposed to be completely ignored.

So I set CC_DST to 0 (this happens when leaving and rejoining the
virtual machine, so this is the real problem) and if I do that, I get
funny segmentation faults in x86_64 guest userspace programs running in
the virtual machine (this is exactly what I see in kvm with my current
patchset as well), while 32 bit userspace programs simply hang.
So I guess this is the real problem.

Is there any logical reason CC_DST could be used with CC_OP==CC_OP_EFLAGS?

Attached to this email you will find a small patch that triggers this
problem.

Thanks for any reply that could help on this,

Alexander Graf


If you play with the CC_OP logic, it is better to disable the eflags 
optimization code in the translator (optimize_flags() function).


Regarding the implementation for SVM, you can look at how the CC are 
handled in SMM (do_smm_enter and helper_rsm). I see no particular 
problem here.


I suggest to try to suppress the additions in the static translator 
state as I feel most of the SVM intercepts can be tested in helpers 
where speed is not critical.


Regards,

Fabrice.





Re: [Qemu-devel] Re: PATCH, RFC: Generic DMA framework

2007-08-29 Thread Paul Brook
> If this is the case, it means we don't need anything complicated. Devices
> map themselves straight into the system address space at the appropriate
> slot address (no plug-n-play to worry about), and device "DMA" goes via the
> IOMMU.

Further searching by google suggests I may be wrong.

The alternative is that the controller maps the 32-bit VA onto a device 
select+28-bit address, using some as-yet undiscovered mechanism.
There are then a couple of different options for how the CPU/memory bus is 
accessed:
a) The IOMMU is one or more slave devices, than feed the 28-bit address 
possibly plus a few other bits from the device ID into the translation table. 
This effectively allows you to map a proportion of the SBus 32-bit master VA 
space onto CPU address space via the IOMMU, and map the remainder onto 
devices on the same bus. For a system with <=8 slots per bus a fixed mapping 
using the first 2G as 256Mb for each slot and the top 2G for IOMMU is 
entirely feasible.
b) The 32-bit SBus VA is looked up directly into the IOMMU. Each IOMMU entry 
can refer to either a CPU address, or a device+28-bit address on the local 
SBUS.

Paul