[Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Magnus Damm]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

Hello,

This patch adds powerpc host support to the CONFIG_MMU_MAP patch
written by Piotrek. My patch should be applied on top of
v1-part[1-3].patch.gz. I have only tested the code with a x86 guest on
a ppc host running Linux - someone, please test on a host running OSX.

Performance gain reported by nbench:

Memory index: 50%
Integer index: 44%
Fp index: 4%

Right now each map-memory access consists of 5-6 powerpc instructions.
If a direct pointer to mem_map could be kept in a register then we
would be down to 3-4 instructions per memoy access...

/ magnus

[-- Attachment #2: qemu-mmap_20041217-ppc.patch --]
[-- Type: application/octet-stream, Size: 5338 bytes --]

diff -urN qemu-mmap_20041217/configure qemu-mmap_20041217-ppc/configure
--- qemu-mmap_20041217/configure	2004-12-19 00:01:12.000000000 +0100
+++ qemu-mmap_20041217-ppc/configure	2004-12-19 00:20:59.000000000 +0100
@@ -547,6 +547,10 @@
   echo "CONFIG_MMU_MAP=yes" >> $config_mak
   echo "#define CONFIG_MMU_MAP 1" >> $config_h
 fi
+if test "$target_softmmu" = "yes" -a "$target_cpu" = "i386" -a "$cpu" = "powerpc" ; then
+  echo "CONFIG_MMU_MAP=yes" >> $config_mak
+  echo "#define CONFIG_MMU_MAP 1" >> $config_h
+fi
 if test "$target_user_only" = "yes" ; then
   echo "CONFIG_USER_ONLY=yes" >> $config_mak
   echo "#define CONFIG_USER_ONLY 1" >> $config_h
diff -urN qemu-mmap_20041217/mmu_map.h qemu-mmap_20041217-ppc/mmu_map.h
--- qemu-mmap_20041217/mmu_map.h	2004-12-19 00:01:12.000000000 +0100
+++ qemu-mmap_20041217-ppc/mmu_map.h	2004-12-20 12:54:38.000000000 +0100
@@ -132,5 +132,44 @@
 }
 
 #else
+#if defined(__powerpc__)
+
+#ifdef linux
+#define R3 uc->uc_mcontext.regs->gpr[3]
+#endif
+
+#ifdef __APPLE__
+#include <sys/ucontext.h>
+#define R3 uc->uc_mcontext->ss.r3
+#endif
+
+#ifndef R3
+#error unsupported host operating system
+#endif
+
+static inline target_ulong mmu_map_fault_get_vaddr(ucontext_t *uc,
+                                                   unsigned long addr)
+{
+    target_ulong vaddr;
+
+    vaddr = addr - R3;
+    return vaddr;
+}
+
+static inline void mmu_map_fault_update_uc(ucontext_t *uc, MmuMap *map,
+                                           unsigned long vaddr, int is_write)
+{
+    target_ulong vpage;
+
+    vpage = vaddr >> TARGET_PAGE_BITS;
+    if (is_write) {
+        R3 = map->add_write[vpage];
+    } else {
+        R3 = map->add_read[vpage];
+    }
+}
+
+#else
 #error unsupported host CPU
 #endif
+#endif
diff -urN qemu-mmap_20041217/mmu_map_templ.h qemu-mmap_20041217-ppc/mmu_map_templ.h
--- qemu-mmap_20041217/mmu_map_templ.h	2004-12-19 00:01:12.000000000 +0100
+++ qemu-mmap_20041217-ppc/mmu_map_templ.h	2004-12-20 18:23:40.000000000 +0100
@@ -125,9 +125,145 @@
 }
 
 #else
-#error unsupported host CPU
+#if defined(__powerpc__)
+
+/* this code assumes that the host ppc is running in big endian mode */
+
+#ifdef TARGET_I386
+#define ENDIAN_SWAP
+#endif
+
+#if 0
+        /* slow lookup, first shift right, then left */
+        "rlwinm 3, %1, 32 - %2, %2, 31\n"
+        "addis  4, %3, %4@ha\n"
+        "rlwinm 3, 3, 2, 0, 31 - 2\n"
+        "addi   4, 4, %4@l\n"
+        "lwzx   3, 4, 3\n"
+
+        /* current lookup, one shift and mask operation */
+        "addis  4, %3, %4@ha\n"
+        "rlwinm 3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
+        "addi   4, 4, %4@l\n"
+        "lwzx   3, 4, 3\n"
+
+        /* better lookup, one shift and mask operation, mmu_map in register */
+        "rlwinm 3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
+        "lwzx   3, %3, 3\n"
+#endif
+
+static inline RES_TYPE glue(glue(ld, USUFFIX), MEMSUFFIX)(void *ptr)
+{
+    RES_TYPE val;
+
+    asm volatile (
+        "addis  4, %3, %4@ha\n"
+        "rlwinm 3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
+        "addi   4, 4, %4@l\n"
+        "lwzx   3, 4, 3\n"
+
+#if DATA_SIZE == 1
+        "lbzx   %0, %1, 3\n"
+#elif DATA_SIZE == 2
+#ifdef ENDIAN_SWAP
+        "lhbrx  %0, %1, 3\n"
+#else
+        "lhzx   %0, %1, 3\n"
+#endif
+#elif DATA_SIZE == 4
+#ifdef ENDIAN_SWAP
+        "lwbrx  %0, %1, 3\n"
+#else
+        "lwzx   %0, %1, 3\n"
 #endif
+#else
+#error unsupported size
+#endif
+        : "=r" (val)
+        : "r" (ptr),
+          "I" (TARGET_PAGE_BITS),
+          "r" (env),
+          "i" (offsetof(CPUState, mmu_map[CPU_MEM_INDEX].add_read))
+        : "%r3", "%r4", "memory");
 
+    return val;
+}
+
+#if DATA_SIZE <= 2
+static inline int glue(glue(lds, SUFFIX), MEMSUFFIX)(void *ptr)
+{
+    int val;
+
+    asm volatile (
+        "addis  4, %3, %4@ha\n"
+        "rlwinm 3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
+        "addi   4, 4, %4@l\n"
+        "lwzx   3, 4, 3\n"
+
+#if DATA_SIZE == 1
+        "lbzx   4, %1, 3\n"
+        "extsb  %0, 4\n"
+#elif DATA_SIZE == 2
+#ifdef ENDIAN_SWAP
+        "lhbrx   4, %1, 3\n"
+        "extsh  %0, 4\n"
+#else
+        "lhax   %0, %1, 3\n"
+#endif
+#else
+#error unsupported size
+#endif
+        : "=r" (val)
+        : "r" (ptr),
+          "I" (TARGET_PAGE_BITS),
+          "r" (env),
+          "i" (offsetof(CPUState, mmu_map[CPU_MEM_INDEX].add_read))
+        : "%r3", "%r4", "memory");
+
+    return val;
+}
+#endif
+
+static inline void glue(glue(st, SUFFIX), MEMSUFFIX)(void *ptr, RES_TYPE val)
+{
+
+    asm volatile (
+        "addis  4, %3, %4@ha\n"
+        "rlwinm 3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
+        "addi   4, 4, %4@l\n"
+        "lwzx   3, 4, 3\n"
+
+#if DATA_SIZE == 1
+        "stbx   %0, %1, 3\n"
+#elif DATA_SIZE == 2
+#ifdef ENDIAN_SWAP
+        "sthbrx %0, %1, 3\n"
+#else
+        "sthx   %0, %1, 3\n"
+#endif
+#elif DATA_SIZE == 4
+#ifdef ENDIAN_SWAP
+        "stwbrx %0, %1, 3\n"
+#else
+        "stwx   %0, %1, 3\n"
+#endif
+#else
+#error unsupported size
+#endif
+        :
+        : "r" (val),
+          "r" (ptr),
+          "I" (TARGET_PAGE_BITS),
+          "r" (env),
+          "i" (offsetof(CPUState, mmu_map[CPU_MEM_INDEX].add_write))
+        : "%r3", "%r4", "memory");
+
+}
+
+#else
+#error unsupported host CPU
+#endif
+#endif
 
 #undef RES_TYPE
 #undef DATA_TYPE

[Qemu-devel] Re: [PATCH] CONFIG_MMU_MAP powerpc host support

[Piotras]

Great!

Do you have an estimate of possible performance gain by introducing 
direct pointer to mmu_map for memory read?

I have two ideas for future experimentation.

There is a trick possible without wasting another register for global 
variable: use two copies of CPUState (one for privileged and another 
for user mode), then make mmu_map.add_read first member of the 
struct. This would introduce guest register coping for user/supervisor 
switch, but maybe performance gain would justify this.

Another idea: if we could align add_read/add_write on 64k boundary, 
"addi" could be removed.


Regards,

Piotrek

On Mon, 20 Dec 2004 18:55:21 +0100, Magnus Damm <magnus.damm@gmail.com> wrote:
> Hello,
> 
> This patch adds powerpc host support to the CONFIG_MMU_MAP patch
> written by Piotrek. My patch should be applied on top of
> v1-part[1-3].patch.gz. I have only tested the code with a x86 guest on
> a ppc host running Linux - someone, please test on a host running OSX.
> 
> Performance gain reported by nbench:
> 
> Memory index: 50%
> Integer index: 44%
> Fp index: 4%
> 
> Right now each map-memory access consists of 5-6 powerpc instructions.
> If a direct pointer to mem_map could be kept in a register then we
> would be down to 3-4 instructions per memoy access...
> 
> / magnus

[Qemu-devel] Re: [PATCH] CONFIG_MMU_MAP powerpc host support

[Magnus Damm]

On Mon, 20 Dec 2004 22:31:34 +0100, Piotras <piotras@gmail.com> wrote:
> Great!
> 
> Do you have an estimate of possible performance gain by introducing
> direct pointer to mmu_map for memory read?

No, I have no idea. The speed gain is probably host cpu specific.

I believe one downside with the current micro operations design is
that the compiler is unable to order the host instructions in a
optimal way because each micro operation is so damn small... And of
course we want them small.  But if we could combine the most popular
guest instructions into one micro operation (which is not so micro
anymore) then the compiler could rearrange things to fully take
advantage of the host cpu.

It all boils down to some table-based generic guest opcode matching
code that does a longest prefix match and supports masking of
bitfields...

I think it would be very interesting to collect opcode statistics for
certain guest operating systems. Or maybe someone already has done
that?

> I have two ideas for future experimentation.
> 
> There is a trick possible without wasting another register for global
> variable: use two copies of CPUState (one for privileged and another
> for user mode), then make mmu_map.add_read first member of the
> struct. This would introduce guest register coping for user/supervisor
> switch, but maybe performance gain would justify this.
>
> Another idea: if we could align add_read/add_write on 64k boundary,
> "addi" could be removed.

Yes, both are good ideas IMHO. Would your future experimentation
improve x86 performance? If this is powerpc-specific then I think we
could use one or two registers (one for read and one for write) and
modify these registers each time the processor changes between user
and kernel mode. I think the big limitation right now for powerpc is
that the good old /* suppress this hack */ never worked out...

And I also think we are reaching a limit here, of course we would gain
some by reducing the number of instructions but soon there are not
many instructions left to remove... So the bottleneck must be
somewhere else.

/ magnus

> On Mon, 20 Dec 2004 18:55:21 +0100, Magnus Damm <magnus.damm@gmail.com> wrote:
> > Hello,
> >
> > This patch adds powerpc host support to the CONFIG_MMU_MAP patch
> > written by Piotrek. My patch should be applied on top of
> > v1-part[1-3].patch.gz. I have only tested the code with a x86 guest on
> > a ppc host running Linux - someone, please test on a host running OSX.
> >
> > Performance gain reported by nbench:
> >
> > Memory index: 50%
> > Integer index: 44%
> > Fp index: 4%
> >
> > Right now each map-memory access consists of 5-6 powerpc instructions.
> > If a direct pointer to mem_map could be kept in a register then we
> > would be down to 3-4 instructions per memoy access...
> >
> > / magnus
>

Re: [Qemu-devel] Re: [PATCH] CONFIG_MMU_MAP powerpc host support

[Daniel Serpell]

Hi!

El Mon, Dec 20, 2004 at 11:12:31PM +0100, Magnus Damm escribio:
>
[...]
> But if we could combine the most popular
> guest instructions into one micro operation (which is not so micro
> anymore) then the compiler could rearrange things to fully take
> advantage of the host cpu.
> 
> It all boils down to some table-based generic guest opcode matching
> code that does a longest prefix match and supports masking of
> bitfields...
> 
> I think it would be very interesting to collect opcode statistics for
> certain guest operating systems. Or maybe someone already has done
> that?
> 

I have done that, it's not so difficult. I added a fixed static two
dimensional table (int ins_usage[MAX_INS][MAX_INS]), an then a global
variable "int last_ins".

Also, I moddified the "ops*.h" and "op.c" files, adding at the
beggining: "ins_usage[last_ins][THIS_INS]++; last_inst=THIS_INS;",
with THIS_INS initialized to some unique constant.

The problem is, I then realized that I measured the instructions
usage but not the time spent in each instruction, and the instructions
doing memory-access where spending almost all the emulated time.

So, I presumed that accelerating memory-access were a better project.

If you are interested, I could send the patches.

        Daniel.

Re: [Qemu-devel] Re: [PATCH] CONFIG_MMU_MAP powerpc host support

[Fabrice Bellard]

Piotras wrote:
> Great!
> 
> Do you have an estimate of possible performance gain by introducing 
> direct pointer to mmu_map for memory read?
> 
> I have two ideas for future experimentation.
> 
> There is a trick possible without wasting another register for global 
> variable: use two copies of CPUState (one for privileged and another 
> for user mode), then make mmu_map.add_read first member of the 
> struct. This would introduce guest register coping for user/supervisor 
> switch, but maybe performance gain would justify this.
> Another idea: if we could align add_read/add_write on 64k boundary, 
> "addi" could be removed.

IMHO, using a single global register and interleaving the 4 access types 
is the best solution...

Fabrice.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Laurent Amon]

On 20 déc. 04, at 18:55, Magnus Damm wrote:

> Hello,
>
> This patch adds powerpc host support to the CONFIG_MMU_MAP patch
> written by Piotrek. My patch should be applied on top of
> v1-part[1-3].patch.gz. I have only tested the code with a x86 guest on
> a ppc host running Linux - someone, please test on a host running OSX.
>

Hi,

On a first try, it doesn't compile on OSX 10.3.7, with gcc version 3.3  
20030304 (Apple Computer, Inc. build 1495). I'll try to look why  
another time.

I get this :
In file included from /usr/include/ucontext.h:27,
                  from  
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/mmu_map.h:22,
                  from  
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/target-i386/ 
cpu.h:34,
                  from  
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/target-i386/ 
exec.h:116,
                  from  
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/target-i386/ 
op.c:22:
/usr/include/sys/ucontext.h:30: error: parse error before "sigset_t"
/usr/include/sys/ucontext.h:35: error: parse error before '}' token
/usr/include/sys/ucontext.h:42: error: parse error before "sigset_t"
/usr/include/sys/ucontext.h:47: error: parse error before '}' token
In file included from  
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/target-i386/ 
cpu.h:34,
                  from  
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/target-i386/ 
exec.h:116,
                  from  
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/target-i386/ 
op.c:22:
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/mmu_map.h: In  
function `mmu_map_fault_get_vaddr':
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/mmu_map.h:155:  
error: dereferencing pointer to incomplete type
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/mmu_map.h: In  
function `mmu_map_fault_update_uc':
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/mmu_map.h:166:  
error: dereferencing pointer to incomplete type
/Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/mmu_map.h:168:  
error: dereferencing pointer to incomplete type
make[1]: *** [op.o] Error 1
make: *** [all] Error 1


Merry Christmas,

Lga.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Norikatsu Shigemura]

On Sat, 25 Dec 2004 01:04:58 +0100
Laurent Amon <amon@stanfordalumni.org> wrote:
> On 20 dec. 04, at 18:55, Magnus Damm wrote:
> > This patch adds powerpc host support to the CONFIG_MMU_MAP patch
> > written by Piotrek. My patch should be applied on top of
> > v1-part[1-3].patch.gz. I have only tested the code with a x86 guest on
> > a ppc host running Linux - someone, please test on a host running OSX.
> Hi,
> On a first try, it doesn't compile on OSX 10.3.7, with gcc version 3.3  
> 20030304 (Apple Computer, Inc. build 1495). I'll try to look why  
> another time.

	Humm.. I think that 'struct ucontext' should be changed
	to 'ucontext_t'.  ucontext_t isn't necessarily struct
	ucontext.  But... I don't know how to fix following
	problem.

> /Users/lamon/src/qemustuff/qemu-snapshot-2004-12-22_23/target-i386/ 
> op.c:22:
> /usr/include/sys/ucontext.h:30: error: parse error before "sigset_t"
> /usr/include/sys/ucontext.h:35: error: parse error before '}' token
> /usr/include/sys/ucontext.h:42: error: parse error before "sigset_t"
> /usr/include/sys/ucontext.h:47: error: parse error before '}' token


	I have a patch for FreeBSD support.  But CONFIG_MMUMAP doesn't
	works..  Anyone, please debug it.

	ports style:
	http://people.FreeBSD.org/~nork/qemu/qemu.20041223.port.tar.gz

	v1-part[1-3].patch.gz + qemu-mmap_20041217-ppc.patch
	 + following patch:
	http://people.FreeBSD.org/~nork/qemu/extra-patch-v1-partFIX

P.S.
  I think that 'struct siginfo' shuould be changed to 'siginfo_t', too...

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Laurent Amon]

On 25 déc. 04, at 14:48, Norikatsu Shigemura wrote:

> On Sat, 25 Dec 2004 01:04:58 +0100
> Laurent Amon <amon@stanfordalumni.org> wrote:
>> On 20 dec. 04, at 18:55, Magnus Damm wrote:
>>> This patch adds powerpc host support to the CONFIG_MMU_MAP patch
>>> written by Piotrek. My patch should be applied on top of
>>> v1-part[1-3].patch.gz. I have only tested the code with a x86 guest 
>>> on
>>> a ppc host running Linux - someone, please test on a host running 
>>> OSX.
>> Hi,
>> On a first try, it doesn't compile on OSX 10.3.7, with gcc version 3.3
>> 20030304 (Apple Computer, Inc. build 1495). I'll try to look why
>> another time.
>
> 	Humm.. I think that 'struct ucontext' should be changed
> 	to 'ucontext_t'.  ucontext_t isn't necessarily struct
> 	ucontext.  But... I don't know how to fix following
> 	problem.
>
Actually, I just included sys/signal.h and I passed this hurdle.

However, I have a lot of assembly error when compiling op.c, and I 
suspect it may be caused by the asm code Magnus added in his ppc patch. 
I'm even more out of my depth than usual here. Magnus, for which 
PowerPC did you write the code? I have a 7410 G4(TiBook/400).

Using Norikatsu's extra patch gives exactly the same error, so I'll 
reverse-patch for now.

This is a excerpt of the errors I get :
/var/tmp//ccrclido.s:6176:Parameter syntax error (parameter 1)
/var/tmp//ccrclido.s:6176:Invalid mnemonic 'ha'
/var/tmp//ccrclido.s:6177:Parameter syntax error (parameter 1)
/var/tmp//ccrclido.s:6178:Parameter syntax error (parameter 1)
/var/tmp//ccrclido.s:6178:Invalid mnemonic 'l'
/var/tmp//ccrclido.s:6179:Parameter syntax error (parameter 1)
/var/tmp//ccrclido.s:6180:Parameter syntax error (parameter 3)
/var/tmp//ccrclido.s:6198:Parameter syntax error (parameter 1)
/var/tmp//ccrclido.s:6198:Invalid mnemonic 'ha'
/var/tmp//ccrclido.s:6199:Parameter syntax error (parameter 1)
/var/tmp//ccrclido.s:6200:Parameter syntax error (parameter 1)
/var/tmp//ccrclido.s:6200:Invalid mnemonic 'l'
/var/tmp//ccrclido.s:6201:Parameter syntax error (parameter 1)
/var/tmp//ccrclido.s:6202:Parameter syntax error (parameter 1)
/var/tmp//ccrclido.s:6203:Parameter syntax error (parameter 2)
[repeat...]

Season's Greetings,

Lga.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Pierre d'Herbemont]

[-- Attachment #1: Type: text/plain, Size: 1738 bytes --]


On 27 déc. 04, at 17:06, Laurent Amon wrote:
>
> However, I have a lot of assembly error when compiling op.c, and I 
> suspect it may be caused by the asm code Magnus added in his ppc 
> patch. I'm even more out of my depth than usual here. Magnus, for 
> which PowerPC did you write the code? I have a 7410 G4(TiBook/400).

Mac OS X's assembler expects a slightly different syntax.
Basically you'll have to change the register name from x to rx, and 
word selector from mem@s to s16(mem), for example:

lis 9, (import+32)@ha	; on Linux, will become :
lis r9,ha16(import+32)	; on Mac OS X.

So you'll have to write asm code which is platform independent. I think 
the best way to do that is simply to use the C Preprocessor, and define 
constant string for registers names, for instance :

#ifdef __powerpc__
# ifdef __APPLE__
# define ppc_high(mem) "ha16(" mem ")"
# define ppc_low(mem)  "lo16(" mem ")"
# define ppc_r0	"r0"
# define ppc_r1	"r1"
/* ... */

# else  /* __APPLE__ */
# define ppc_high(mem) "(" mem ")@hi"
# define ppc_low(mem)  "(" mem ")@l"
# define ppc_r0	"0"
# define ppc_r1	"1"
/* ... */

# endif  /* __APPLE__ */
#define stringify(x)	#x
#define ppc_asm_1(instr, param1)						stringify(instr) " " param1
#define ppc_asm_2(instr, param1, param2)				ppc_asm_1(instr, param1) 
"," param2
#define ppc_asm_3(instr, param1, param2 , param3)		ppc_asm_2(instr, 
param1, param2) "," param3
#define ppc_asm_4(instr, param1, param2 , param3, 
param4)	ppc_asm_3(instr, param1, param2, param3) "," param4
#endif  /* __powerpc__ */

#ifdef __powerpc__
/* asm("addis r4, %3, ha16(%4)"); */
asm (ppc_asm_4(addis,  ppc_r4, "%3", ppc_high("%4")) "\n");
#endif

Hope it helps.

Pierre.

[-- Attachment #2: Type: text/enriched, Size: 1858 bytes --]



On 27 déc. 04, at 17:06, Laurent Amon wrote:

<excerpt>

However, I have a lot of assembly error when compiling op.c, and I
suspect it may be caused by the asm code Magnus added in his ppc
patch. I'm even more out of my depth than usual here. Magnus, for
which PowerPC did you write the code? I have a 7410 G4(TiBook/400).

</excerpt>

Mac OS X's assembler expects a slightly different syntax.

Basically you'll have to change the register name from x to rx, and
word selector from mem@s to s16(mem), for example:


lis 9, (import+32)@ha	; on Linux, will become :

lis r9,ha16(import+32)	; on Mac OS X.


So you'll have to write asm code which is platform independent. I
think the best way to do that is simply to use the C Preprocessor, and
define constant string for registers names, for instance :<fontfamily><param>Courier</param>

</fontfamily>

#ifdef __powerpc__

# ifdef __APPLE__

# define ppc_high(mem) "ha16(" mem ")"

# define ppc_low(mem)  "lo16(" mem ")"

# define ppc_r0	"r0"

# define ppc_r1	"r1"

/* ... */


# else  /* __APPLE__ */

# define ppc_high(mem) "(" mem ")@hi"

# define ppc_low(mem)  "(" mem ")@l"

# define ppc_r0	"0"

# define ppc_r1	"1"

/* ... */


# endif  /* __APPLE__ */

#define stringify(x)	#x

#define ppc_asm_1(instr, param1)						stringify(instr) " " param1

#define ppc_asm_2(instr, param1, param2)				ppc_asm_1(instr, param1)
"," param2

#define ppc_asm_3(instr, param1, param2 , param3)		ppc_asm_2(instr,
param1, param2) "," param3

#define ppc_asm_4(instr, param1, param2 , param3,
param4)	ppc_asm_3(instr, param1, param2, param3) "," param4

#endif  /* __powerpc__ */


#ifdef __powerpc__

/* asm("addis r4, %3, ha16(%4)"); */

asm (ppc_asm_4(addis,  ppc_r4, "%3", ppc_high("%4")) "\n");

#endif


Hope it helps.


Pierre.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Laurent Amon]

[-- Attachment #1: Type: text/plain, Size: 935 bytes --]


On 27 déc. 04, at 22:41, Pierre d'Herbemont wrote:
>
> So you'll have to write asm code which is platform independent. I 
> think the best way to do that is simply to use the C Preprocessor, and 
> define constant string for registers names, for instance :
>
[...]
> #ifdef __powerpc__
> /* asm("addis r4, %3, ha16(%4)"); */
> asm (ppc_asm_4(addis,  ppc_r4, "%3", ppc_high("%4")) "\n");
> #endif
>
Merci Pierre,

That looks pretty heavy, though, and I don't think that the result 
would be very easy to read.

For now, there are only 28 lines impacted so I'lll settle with 
duplicating them for the purpose of testing this patch. No tonight, 
though, tomorrow's a working day and it's late already.

In the long run, if we get to do more powerpc asm for optimization, I 
would rather settle on one syntax and use a bit of perl in the making 
process to transform it if necessary.

Thanks again,

Lga.



[-- Attachment #2: Type: text/enriched, Size: 1033 bytes --]



On 27 déc. 04, at 22:41, Pierre d'Herbemont wrote:

<excerpt>

So you'll have to write asm code which is platform independent. I
think the best way to do that is simply to use the C Preprocessor, and
define constant string for registers names, for instance :<fontfamily><param>Courier</param>

</fontfamily>

</excerpt>[...]

<excerpt>#ifdef __powerpc__

/* asm("addis r4, %3, ha16(%4)"); */

asm (ppc_asm_4(addis,  ppc_r4, "%3", ppc_high("%4")) "\n");

#endif


</excerpt>Merci Pierre,


That looks pretty heavy, though, and I don't think that the result
would be very easy to read. 


For now, there are only 28 lines impacted so I'lll settle with
duplicating them for the purpose of testing this patch. No tonight,
though, tomorrow's a working day and it's late already.

 

In the long run, if we get to do more powerpc asm for optimization, I
would rather settle on one syntax and use a bit of perl in the making
process to transform it if necessary. 


Thanks again,


Lga.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 261 bytes --]

On 27.12.2004, at 22:41, Pierre d'Herbemont wrote:

> Mac OS X's assembler expects a slightly different syntax.

But GNU binutils should accept both, so simply changing to the syntax
of the crippled Mac OSX assembler should do the trick.

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Pierre d'Herbemont]

On 28 déc. 04, at 10:15, Daniel Egger wrote:

> On 27.12.2004, at 22:41, Pierre d'Herbemont wrote:
>
>> Mac OS X's assembler expects a slightly different syntax.
>
> But GNU binutils should accept both, so simply changing to the syntax
> of the crippled Mac OSX assembler should do the trick.

GNU binutils on Darwin is not a regular thing. By default Apple ships 
their own binutils, "cctools". And their is no option to switch the 
syntax used in the cctools, so we'll have to handle both syntax.

You may also notice that in addition to the register naming convention 
and the word selector name's differences, the many asm keywords like 
.data or .fill are different. You can explain that with the fact that 
darwin and linux uses two different ABI : Linux uses elf, darwin uses 
Mach-O.

Pierre.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On 28.12.2004, at 10:55, Pierre d'Herbemont wrote:

>> But GNU binutils should accept both, so simply changing to the syntax
>> of the crippled Mac OSX assembler should do the trick.

> GNU binutils on Darwin is not a regular thing. By default Apple ships 
> their own binutils, "cctools". And their is no option to switch the 
> syntax used in the cctools, so we'll have to handle both syntax.

Sorry for being unclear. I mean that the syntax should be changed
to the one of cctools because the GNU binutils are supposed to
handle that just fine.

Actually I'm a bit unsure about the displacement syntax but
I know that register names are accepted by GNU binutils and
cctools because I've written quite a lot of AltiVec code
which works on both Linux and MacOSX without any tricks.

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Pierre d'Herbemont]

On 28 déc. 04, at 11:04, Daniel Egger wrote:

> On 28.12.2004, at 10:55, Pierre d'Herbemont wrote:
>
>>> But GNU binutils should accept both, so simply changing to the syntax
>>> of the crippled Mac OSX assembler should do the trick.
>
>> GNU binutils on Darwin is not a regular thing. By default Apple ships 
>> their own binutils, "cctools". And their is no option to switch the 
>> syntax used in the cctools, so we'll have to handle both syntax.
>
> Sorry for being unclear. I mean that the syntax should be changed
> to the one of cctools because the GNU binutils are supposed to
> handle that just fine.

My fault... I didn't pay enough attention to the sentence.

> Actually I'm a bit unsure about the displacement syntax but
> I know that register names are accepted by GNU binutils and
> cctools because I've written quite a lot of AltiVec code
> which works on both Linux and MacOSX without any tricks.

Hum, seems to be a good trick... Google search returns the "-mregnames" 
option. But it seems that it doesn't deal with ha16() or lo16(), so 
we'll have to deal with it. However I think it is still better than 
duplicating the PowerPC code.

Pierre.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Piotras]

Hi!

The problem can be avoided by replacing some of asm with C. 
The gcc seems to generate equally-efficient code in this case. 
I don't have a ppc machine to test it, but something like this may 
work:

RES_TYPE glue(glue(ld, USUFFIX), MEMSUFFIX)(void *ptr)
{
    target_ulong addr;
    register target_phys_addr_t add asm ("r3");
    RES_TYPE val;

    addr = (target_ulong)ptr;
    add = env->mmu_map[CPU_MEM_INDEX].add_read[addr >> TARGET_PAGE_BITS];
    asm volatile (
#if (DATA_SIZE == 1)
        "lbzx   %0, %1, %2\n"
#elif (DATA_SIZE == 2) && defined(BSWAP_NEEDED)
        "lhbrx  %0, %1, %2\n"
#elif (DATA_SIZE == 2)
        "lhzx   %0, %1, %2\n"
#elif (DATA_SIZE == 4) && defined(BSWAP_NEEDED)
        "lwbrx  %0, %1, %2\n"
#elif (DATA_SIZE == 4)
        "lwzx   %0, %1, %2\n"
#else
#error unsupported size
#endif
        : "=r" (val)
        : "r" (addr),
          "r" (add)
        : "memory");

    return val;
}

You will have to modify lds and st as well (make sure to replace 
"add_read" with "add_write" for st). It is critical to use a specific 
register as %2 (Magnus decided to use r3).


Regards,

Piotrek

On Tue, 28 Dec 2004 12:18:57 +0100, Pierre d'Herbemont <stegefin@free.fr> wrote:
> 
> [...]
> 
> > Actually I'm a bit unsure about the displacement syntax but
> > I know that register names are accepted by GNU binutils and
> > cctools because I've written quite a lot of AltiVec code
> > which works on both Linux and MacOSX without any tricks.
> 
> Hum, seems to be a good trick... Google search returns the "-mregnames"
> option. But it seems that it doesn't deal with ha16() or lo16(), so
> we'll have to deal with it. However I think it is still better than
> duplicating the PowerPC code.
> 
> Pierre.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

On 20.12.2004, at 18:55, Magnus Damm wrote:

> This patch adds powerpc host support to the CONFIG_MMU_MAP patch
> written by Piotrek. My patch should be applied on top of
> v1-part[1-3].patch.gz. I have only tested the code with a x86 guest on
> a ppc host running Linux - someone, please test on a host running OSX.

Segfaults for me on 10.3.7 after adjusting the assembler code and some
C to compile on this machine.

Starting program: /Users/egger/qemu/i386-softmmu/qemu -cdrom 
~/Desktop/KNOPPIX_V3.7-2004-12-08-DE/KNOPPIX_V3.7-2004-12-08-DE.iso
Reading symbols for shared libraries 
+.............................................................. done
Reading symbols for shared libraries . done
mmu: acceleration enabled

Program received signal EXC_BAD_ACCESS, Could not access memory.
0x002a3a48 in code_gen_buffer ()
(gdb) bt
#0  0x002a3a48 in code_gen_buffer ()
#1  0x00077be4 in cpu_x86_exec (env1=0xeae000) at 
/Users/egger/qemu/cpu-exec.c:545
#2  0x000067e8 in main_loop () at /Users/egger/qemu/vl.c:2581
#3  0x00008e38 in SDL_main (argc=3, argv=0xbffffad8) at 
/Users/egger/qemu/vl.c:3574
#4  0x000a78cc in -[SDLMain applicationDidFinishLaunching:] 
(self=0xeae000, _cmd=0x1c8f98a8, note=0xffffffff) at 
macosx/SDLMain.m:243
#5  0x909f7184 in _nsnote_callback ()
#6  0x901aa5c0 in __CFXNotificationPost ()

Hard to tell why, but code_gen_buffer contains only zeros after a
few iterations of cpu_x86_exec ().

Any idea how to debug this?

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Laurent Amon]

On 2 janv. 05, at 19:26, Daniel Egger wrote:

> On 20.12.2004, at 18:55, Magnus Damm wrote:
>
>> This patch adds powerpc host support to the CONFIG_MMU_MAP patch
>> written by Piotrek. My patch should be applied on top of
>> v1-part[1-3].patch.gz. I have only tested the code with a x86 guest on
>> a ppc host running Linux - someone, please test on a host running OSX.
>
> Segfaults for me on 10.3.7 after adjusting the assembler code and some
> C to compile on this machine.
>
Could you please post the patch of what you did? I'm trying to do the 
same, but I don't even compile yet.

Lga.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Daniel Egger]

[-- Attachment #1.1: Type: text/plain, Size: 733 bytes --]

On 02.01.2005, at 21:18, Laurent Amon wrote:

> Could you please post the patch of what you did? I'm trying to do the 
> same, but I don't even compile yet.

Tough, since not many of the patches are in CVS as of yet.

This is what I did:
- Apply the 3 softmmu patches
- Apply the PPC host specific addon patch
- Replace all of "struct siginfo *" by "siginfo_t *"
- Fix the assembly to Apple cctool syntax in mmu_map_templ.h
   as in the attached file

Once I have sorted out out all the current troubles with my
gfs' iBook (poweradapter defect and thus borrowed from my
second PowerBook that runs Linux, display troubles) I shall
test that the syntax also works for GNU binutils or create
another patch with different asm for both.


[-- Attachment #1.2: mmu_map_templ.h --]
[-- Type: text/plain, Size: 6539 bytes --]

/*
 *  MMU implementation using indirection table (map)
 *
 *  Copyright (c) 2004 Piotr Krysiuk
 *
 * This library is free software; you can redistribute it and/or
 * modify it under the terms of the GNU Lesser General Public
 * License as published by the Free Software Foundation; either
 * version 2 of the License, or (at your option) any later version.
 *
 * This library is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 * Lesser General Public License for more details.
 *
 * You should have received a copy of the GNU Lesser General Public
 * License along with this library; if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 */
#if DATA_SIZE == 4
#define SUFFIX l
#define USUFFIX l
#define DATA_TYPE uint32_t
#elif DATA_SIZE == 2
#define SUFFIX w
#define USUFFIX uw
#define DATA_TYPE uint16_t
#define DATA_STYPE int16_t
#elif DATA_SIZE == 1
#define SUFFIX b
#define USUFFIX ub
#define DATA_TYPE uint8_t
#define DATA_STYPE int8_t
#else
#error unsupported data size
#endif

#if ACCESS_TYPE == 0
#define CPU_MEM_INDEX 0
#elif ACCESS_TYPE == 1
#define CPU_MEM_INDEX 1
#else
#error invalid ACCESS_TYPE
#endif

#define RES_TYPE int


#if defined(__i386__)

static inline RES_TYPE glue(glue(ld, USUFFIX), MEMSUFFIX)(void *ptr)
{
    RES_TYPE val;

    asm volatile (
        "mov    %1, %%eax\n"
        "shr    %2, %%eax\n"
        "mov    %4 (%3, %%eax, 4), %%eax\n"
#if DATA_SIZE == 1
        "movzbl (%1, %%eax, 1), %0\n"
#elif DATA_SIZE == 2
        "movzwl (%1, %%eax, 1), %0\n"
#elif DATA_SIZE == 4
        "movl   (%1, %%eax, 1), %0\n"
#else
#error unsupported size
#endif
        : "=r" (val)
        : "r" (ptr),
          "I" (TARGET_PAGE_BITS),
          "r" (env),
          "m" (*(char *)offsetof(CPUState, mmu_map[CPU_MEM_INDEX].add_read))
        : "%eax", "memory", "cc");
    return val;
}

#if DATA_SIZE <= 2
static inline int glue(glue(lds, SUFFIX), MEMSUFFIX)(void *ptr)
{
    int val;

    asm volatile (
        "mov    %1, %%eax\n"
        "shr    %2, %%eax\n"
        "mov    %4 (%3, %%eax, 4), %%eax\n"
#if DATA_SIZE == 1
        "movsbl (%1, %%eax, 1), %0\n"
#elif DATA_SIZE == 2
        "movswl (%1, %%eax, 1), %0\n"
#else
#error unsupported size
#endif
        : "=r" (val)
        : "r" (ptr),
          "I" (TARGET_PAGE_BITS),
          "r" (env),
          "m" (*(char *)offsetof(CPUState, mmu_map[CPU_MEM_INDEX].add_read))
        : "%eax", "memory", "cc");
    return val;
}
#endif

static inline void glue(glue(st, SUFFIX), MEMSUFFIX)(void *ptr, RES_TYPE val)
{
    asm volatile (
        "mov    %0, %%eax\n"
        "shr    %2, %%eax\n"
        "mov    %4 (%3, %%eax, 4), %%eax\n"
#if DATA_SIZE == 1
        "movb   %b1, (%0, %%eax, 1)\n"
#elif DATA_SIZE == 2
        "movw   %w1, (%0, %%eax, 1)\n"
#elif DATA_SIZE == 4
        "movl   %1, (%0, %%eax, 1)\n"
#else
#error unsupported size
#endif
        :
        : "r" (ptr),
          "r" (val),
          "I" (TARGET_PAGE_BITS),
          "r" (env),
          "m" (*(char *)offsetof(CPUState, mmu_map[CPU_MEM_INDEX].add_write))
        : "%eax", "memory", "cc");
}

#else
#if defined(__powerpc__)

/* this code assumes that the host ppc is running in big endian mode */

#ifdef TARGET_I386
#define ENDIAN_SWAP
#endif

#if 0
        /* slow lookup, first shift right, then left */
        "rlwinm 3, %1, 32 - %2, %2, 31\n"
        "addis  4, %3, %4@ha\n"
        "rlwinm 3, 3, 2, 0, 31 - 2\n"
        "addi   4, 4, %4@l\n"
        "lwzx   3, 4, 3\n"

        /* current lookup, one shift and mask operation */
        "addis  4, %3, %4@ha\n"
        "rlwinm 3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
        "addi   4, 4, %4@l\n"
        "lwzx   3, 4, 3\n"

        /* better lookup, one shift and mask operation, mmu_map in register */
        "rlwinm 3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
        "lwzx   3, %3, 3\n"
#endif

static inline RES_TYPE glue(glue(ld, USUFFIX), MEMSUFFIX)(void *ptr)
{
    RES_TYPE val;

    asm volatile (
        "addis  r4, %3, ha16(%4)\n"
        "rlwinm r3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
        "addi   r4, r4, lo16(%4)\n"
        "lwzx   r3, r4, r3\n"

#if DATA_SIZE == 1
        "lbzx   %0, %1, r3\n"
#elif DATA_SIZE == 2
#ifdef ENDIAN_SWAP
        "lhbrx  %0, %1, r3\n"
#else
        "lhzx   %0, %1, r3\n"
#endif
#elif DATA_SIZE == 4
#ifdef ENDIAN_SWAP
        "lwbrx  %0, %1, r3\n"
#else
        "lwzx   %0, %1, r3\n"
#endif
#else
#error unsupported size
#endif
        : "=r" (val)
        : "r" (ptr),
          "I" (TARGET_PAGE_BITS),
          "r" (env),
          "i" (offsetof(CPUState, mmu_map[CPU_MEM_INDEX].add_read))
        : "%r3", "%r4", "memory");

    return val;
}

#if DATA_SIZE <= 2
static inline int glue(glue(lds, SUFFIX), MEMSUFFIX)(void *ptr)
{
    int val;

    asm volatile (
        "addis  r4, %3, ha16(%4)\n"
        "rlwinm r3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
        "addi   r4, r4, lo16(%4)\n"
        "lwzx   r3, r4, r3\n"

#if DATA_SIZE == 1
        "lbzx   r4, %1, r3\n"
        "extsb  %0, r4\n"
#elif DATA_SIZE == 2
#ifdef ENDIAN_SWAP
        "lhbrx   r4, %1, r3\n"
        "extsh  %0, r4\n"
#else
        "lhax   %0, %1, r3\n"
#endif
#else
#error unsupported size
#endif
        : "=r" (val)
        : "r" (ptr),
          "I" (TARGET_PAGE_BITS),
          "r" (env),
          "i" (offsetof(CPUState, mmu_map[CPU_MEM_INDEX].add_read))
        : "%r3", "%r4", "memory");

    return val;
}
#endif

static inline void glue(glue(st, SUFFIX), MEMSUFFIX)(void *ptr, RES_TYPE val)
{

    asm volatile (
        "addis  r4, %3, ha16(%4)\n"
        "rlwinm r3, %1, 32 - (%2 - 2), (%2 - 2), 31 - 2\n"
        "addi   r4, r4, lo16(%4)\n"
        "lwzx   r3, r4, r3\n"

#if DATA_SIZE == 1
        "stbx   %0, %1, r3\n"
#elif DATA_SIZE == 2
#ifdef ENDIAN_SWAP
        "sthbrx %0, %1, r3\n"
#else
        "sthx   %0, %1, r3\n"
#endif
#elif DATA_SIZE == 4
#ifdef ENDIAN_SWAP
        "stwbrx %0, %1, r3\n"
#else
        "stwx   %0, %1, r3\n"
#endif
#else
#error unsupported size
#endif
        :
        : "r" (val),
          "r" (ptr),
          "I" (TARGET_PAGE_BITS),
          "r" (env),
          "i" (offsetof(CPUState, mmu_map[CPU_MEM_INDEX].add_write))
        : "%r3", "%r4", "memory");

}

#else
#error unsupported host CPU
#endif
#endif

#undef RES_TYPE
#undef DATA_TYPE
#undef DATA_STYPE
#undef SUFFIX
#undef USUFFIX
#undef DATA_SIZE
#undef CPU_MEM_INDEX

[-- Attachment #1.3: Type: text/plain, Size: 24 bytes --]



Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Laurent Amon]

Selon Daniel Egger <de@axiros.com>:

> On 02.01.2005, at 21:18, Laurent Amon wrote:
>
> > Could you please post the patch of what you did? I'm trying to do the
> > same, but I don't even compile yet.
>
> Tough, since not many of the patches are in CVS as of yet.
>
> This is what I did:
> - Apply the 3 softmmu patches
> - Apply the PPC host specific addon patch
> - Replace all of "struct siginfo *" by "siginfo_t *"
> - Fix the assembly to Apple cctool syntax in mmu_map_templ.h
>    as in the attached file

Thanks. I don't know much about OSX assembler syntax so that's where I had some
trouble.

I also included sys/signal.h instead of using siginfo_t (I am working on the
2004/12/23 snapshot) and it seems to work so far.

Best wishes,

Lga.

Re: [Qemu-devel] [PATCH] CONFIG_MMU_MAP powerpc host support

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

On 03.01.2005, at 13:08, Laurent Amon wrote:

> Thanks. I don't know much about OSX assembler syntax so that's where I 
> had some
> trouble.

> I also included sys/signal.h instead of using siginfo_t (I am working 
> on the
> 2004/12/23 snapshot) and it seems to work so far.

You need to re-run configure after patching or set an enviroment
variable before compilation to activate this stuff. It will print
something along the line "mmu acceleration enabled" if it is
enabled.

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]