[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: e6500 stvx instruction

From: BALATON Zoltan
Subject: Re: e6500 stvx instruction
Date: Sat, 19 Jun 2021 14:40:19 +0200 (CEST)

On Fri, 18 Jun 2021, Fabiano Rosas wrote:
BALATON Zoltan <balaton@eik.bme.hu> writes:
So spr_write_excp_vector depends on the order of the exceptions in the
POWERPC_EXCP enum matching the IVOR# that corresponds to that
exception. If I move VPU/VPUA to index 32/33 I can get the machine to
boot further (just a debug hack):

Thanks for debugging this, I've also got to the conclusion that it's
missing IVORs causing this problem. The e6500 docs (see below) say that
these indeed should be IVOR32 and 33 so this may not be just a debug hack
but what is needed for e6500 but I'm not sure if other cores may put other
exceptions on these IVORs and if so what to do with that.

Well, this particular patch is a hack because it will break whatever
processor expects SPEU to use IVOR32. But yeah, something similar to
this is definitely needed.

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index b4de0db7ff..ec2469cb28 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -66,8 +66,8 @@ enum {
    POWERPC_EXCP_ITLB     = 14, /* Instruction TLB miss                      */
    POWERPC_EXCP_DEBUG    = 15, /* Debug interrupt                           */
    /* Vectors 16 to 31 are reserved                                         */
-    POWERPC_EXCP_SPEU     = 32, /* SPE/embedded floating-point unavailable   */
-    POWERPC_EXCP_EFPDI    = 33, /* Embedded floating-point data interrupt    */
+    POWERPC_EXCP_VPU     = 32,
+    POWERPC_EXCP_VPUA    = 33,
    POWERPC_EXCP_EFPRI    = 34, /* Embedded floating-point round interrupt   */
    POWERPC_EXCP_EPERFM   = 35, /* Embedded performance monitor interrupt    */
    POWERPC_EXCP_DOORI    = 36, /* Embedded doorbell interrupt               */
@@ -86,7 +86,7 @@ enum {
    POWERPC_EXCP_HISI     = 70, /* Hypervisor instruction storage exception  */
    POWERPC_EXCP_HDSEG    = 71, /* Hypervisor data segment exception         */
    POWERPC_EXCP_HISEG    = 72, /* Hypervisor instruction segment exception  */
-    POWERPC_EXCP_VPU      = 73, /* Vector unavailable exception              */
+    POWERPC_EXCP_SPEU      = 73,
    /* 40x specific exceptions                                               */
    POWERPC_EXCP_PIT      = 74, /* Programmable interval timer interrupt     */
    /* 601 specific exceptions                                               */
@@ -107,7 +107,7 @@ enum {
    /* 7xx/74xx specific exceptions                                          */
    POWERPC_EXCP_THERM    = 86, /* Thermal interrupt                         */
    /* 74xx specific exceptions                                              */
-    POWERPC_EXCP_VPUA     = 87, /* Vector assist exception                   */
+    POWERPC_EXCP_EFPDI     = 87,
    /* 970FX specific exceptions                                             */
    POWERPC_EXCP_SOFTP    = 88, /* Soft patch exception                      */
    POWERPC_EXCP_MAINT    = 89, /* Maintenance exception                     */
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index d0411e7302..a33c3c36a1 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -2277,6 +2277,8 @@ static void init_excp_e200(CPUPPCState *env, target_ulong 
    env->excp_vectors[POWERPC_EXCP_SPEU]     = 0x00000000;
    env->excp_vectors[POWERPC_EXCP_EFPDI]    = 0x00000000;
    env->excp_vectors[POWERPC_EXCP_EFPRI]    = 0x00000000;
+    env->excp_vectors[POWERPC_EXCP_VPU]    = 0x00000000;
+    env->excp_vectors[POWERPC_EXCP_VPUA]    = 0x00000000;
    env->ivor_mask = 0x0000FFF7UL;
    env->ivpr_mask = ivpr_mask;
    /* Hardware reset vector */

I've also ended up at these functions and noticed that these could be
simplified by chaining them as they are done from POWER7 upwards. But I'm
not sure these are left as they are because they have to be checked
against corresponding docs or were just copy&pasted and modified for
adding new models.

I have been working on a refactoring to split the exception models into
separate files. This thread is being useful in exposing some of the
shortcomings of the current design and one of them is too much coupling
between processors. I do intend to add some chaining, as you mention,
but David is leaning more towards completely independent per-processor
code. I'll probably have an RFC ready in the next week or two that we
can look at and discuss.

Apart from avoiding code duplication I think sharing what's possible would be desirable also because that way improvements in one part would also help other CPUs otherwise less used CPU models may fall behind and diverge from more developed ones. Sharing code ensures this won't happen because you have to make sure older CPUs still work when changing new ones. I think previously ppc and ppc64 were separate but due to most developers moving to ppc64 the older ppc was merged into ppc64 to avoid having to maintain two mostly similar but slightly different targets. Of course where CPU models differ we need different code and instead of ifdefs these could also be put in different files but I think keeping the common parts shared would be better than duplicating them for different CPU models.

One suspicious part (but unrelated to this thread) that
I've noticed is that init_excp_MPC5xx and init_excp_MPC8xx has

     env->excp_vectors[POWERPC_EXCP_FPU]      = 0x00000900;
     env->excp_vectors[POWERPC_EXCP_DECR]     = 0x00000900;

Is that a typo and should one be 800?

Yep, looks like a bug to me. We would need to look at the manual or the
kernel code and see what goes where. I have just started looking into
these older processors, so I'm still figuring out where to find the
resources and how to build/run them. So no promises on fixing it.

Some of these CPUs were related, e.g. https://en.wikipedia.org/wiki/PowerPC_e200 says e200 is decendant of MPC5xx which comes from MPC8xx so these are probably similar and the functions could be chained (as long as later versions only added stuff not removed). For CPUs in different families (such as IBM POWER CPUs or PPC405-440) can be chained too within the family but maybe not between these. So instead of sharing everything or separate everything per model a better approach may be to distinguish between families that are similar within the family but can be more differences between different families. But exploring the genealogy of these CPUs might be challenging as there are so many of these. Probably there are at least IBM BookS, IBM BookE, Motorola BookS and Motorola BookE with maybe some more corner cases and may be generations within with more changes than just slight addition compared to previous models so that may count as a new familiy.


As for this problem with e6500 probably we need a new init_excp function
for e6500 as the e200 one is used for all e500 cores most of which don't
have altivec so we can't just add these to init_excp_e200 I think. I've
tried to do the above clean up but stopped because I don't have enough
knowledge about different cores to know if these are correct and had no
time to dig up and check each manual so I hope somebody with more
knowledge about different PPC chips can do it.

We could have an init_excp_e500, separate from e200 and use the version
parameter to operate on e6500 (more on why below).


$ qemu-system-ppc64 -nographic -machine ppce500 -cpu e6500 -serial \
mon:stdio -smp 2 -m 1024 -kernel book3e_e6500_kernel_5.12.10 -append \
"root=/dev/vda1 rw" -drive \
format=qcow2,file=hdd_debian_sid_ppc64_basic.qcow2,index=0,if=virtio \
 Run /sbin/init as init process
 random: fast init done
 systemd[1]: systemd 247.3-5 running in system mode. (+PAM +AUDIT
 systemd[1]: Detected architecture ppc64.

 Welcome to Debian GNU/Linux 11 (bullseye)!

 systemd[1]: Set hostname to <debianppc64>.

Unfornately, the boot still fails further down the line due to:

Bad kernel stack pointer 3fffe830c200 at c000000000052afc
Oops: Bad kernel stack pointer, sig: 6 [#17]

But I suspect this is another issue by itself.

I did not get to that point yet so no idea but Mario said the same worked
on real hardware so maybe another bug.

The rest of the dump shows trap as 0x300, so it looks like the
Instruction Storage handler might be bugged in the kernel.

Well, we will get to that eventually.

So the question is: If SPEU/DPFDI and VPU/VPUA share IVOR#, is one them
being incorrectly registered for e6500?

According to docs at
https://www.nxp.com/docs/en/reference-manual/E6500RM.pdf Table 4-2 on page
4-9 IVOR32 and 33 are Altivec exceptions while Floating pont unavailable
is IVOR7. Not sure what DPFDI is but the table does not list anything
similar as far as I can tell.

I took a closer look at the manual and section 1.5 lists the changes
from version to version:

- e500v2 --> e500mc:
SPE and embedded floating-point - Not present

- e500mc --> e5500:
No mention of SPE or Altivec

- e5500 --> e6500:
Altivec vector registers and instructons - Present

It looks like SPE was removed for e500mc and never added back, while
Altivec was only introduced in e6500. So we should probably register the
exceptions like this:

e500v1, e500v2: only SPEU/EFPDI/EFPRI
e500mc, e5500:  no SPEU/EFPDI/EFPRI/VPU/VPUA
e6500:          only VPU/VPUA

Now, the way the code is structured today does not allow us to have two
different exceptions with the same number - because of the switch
statement in powerpc_excp. And we need the exception to match the IVOR
number - because of spr_write_excp_vector. Since we don't have
per-processor CONFIGs, I see no clean way to fix this. So we either push
some ugly hack or wait until we managed to refactor some of these parts.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]