Re: e6500 stvx instruction

[Fabiano Rosas]

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

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

> 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
>>  +SELINUX +IMA +APPARMOR +SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP +GCRYPT
>>  +GNUTLS +ACL +XZ +LZ4 +ZSTD +SECCOMP +BLKID +ELFUTILS +KMOD +IDN2 -IDN
>>  +PCRE2
>>  default-hierarchy=unified)
>>  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.