Re: e6500 stvx instruction

[Fabiano Rosas]

BALATON Zoltan <balaton@eik.bme.hu> writes:

> On Wed, 16 Jun 2021, Mark Cave-Ayland wrote:
>> On 16/06/2021 10:57, BALATON Zoltan wrote:
>>> Ah. thanks. I've missed that as I've only looked in .c files and it's in 
>>> .c.inc. If I get this macro correctly it pastes name to gen_st so I expect 
>>> to see a GEN_VR_STX(vx,...) somewhere for stvx but I can only find:
>>> 
>>> GEN_VR_STX(svx, 0x07, 0x07);
>>> /* As we don't emulate the cache, stvxl is stricly equivalent to stvx */
>>> GEN_VR_STX(svxl, 0x07, 0x0F);
>>> 
>>> so where is stvx defined at the end or how does this work?
>>> 
>>> Then I've checked the opcode above: 0x7e8029ce which seems to be 
>>> 1f-07-07-00 so the GEN_VR_STX(svx, 0x07, 0x07) seems to match that.
>>> 
>>>> From this you can see the exception is thrown if ctx->altivec_enabled 
>>>> isn't true, where ctx->altivec_enabled is set in 
>>>> ppc_tr_init_disas_context() to:
>>>> 
>>>>    ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1;
>>>> 
>>>> It seems the issue is that somehow HFLAGS_VR isn't being set from the 
>>>> CPUPPCState env->flags in hreg_compute_hflags_value(). There was a recent 
>>>> patchset from Richard that tidied up the hflags (see 
>>>> https://patchew.org/QEMU/20210323184340.619757-1-richard.henderson@linaro.org/)
>>>>  
>>>> so it could be the issue was accidentally introduced there.
>>> 
>>> As far as I could trace this back MSR_VR that should correspond to 
>>> HFLAGS_VR is turned on by POWERPC_FLAG_VRE which is set in 
>>> PowerPCCPUClass::flags in POWERPC_FAMILY(e6500) in cpu_init.c (the last 
>>> thing in that function). This is done in hreg_compute_hflags_value(). Other 
>>> than that HFLAGS_VR only appears in ppc_tr_init_disas_context() so probably 
>>> this should correspond to MSR_VR which the e6500 manual calls MSR[SPV] and 
>>> seems to be off in the register dump above. So maybe it's a guest bug 
>>> trying to execute Altivec instruction without enabling the vector unit?
>>
>> I guess it could be either the guest is trying to execute Altivec 
>> instructions without configuring the MSR correctly, or perhaps that the 
>> default value of the MSR is incorrect and should have the SPV bit set for 
>> that particular CPU?
>
> From the e6500 manual:
>
> "11.4.5 MSR, FPSCR, and VSCR
>
> At reset, the MSR, FPSCR, and VSCR of each thread are set to 0. The FPSCR 
> and VSCR do not require initialization and can be set at a later time 
> before floating-point or AltiVec is used depending on which modes software 
> wishes to operate in."
>
> So this suggests it's not a problem with defaults unless the manual is 
> wrong or there's some interaction between regs that should turn this bit 
> on while setting another reg.
>
> Then there may be two bugs: one that we don't have a handler for this 
> exception to raise it in the guest and another in the guest that it does 
> not init MSR properly. On a real machine the firmware may do it, so maybe 
> we could do that in reset method if it works with u-boot as the problem 
> was reported using -kernel but I think the ppce500 machine did not expect 
> to have e6500 CPU so it probably does not try to do anything with this. 
> It's probably a kind of imaginary machine (ppce500 with -cpu e6500) not 
> emulating any real machine.

It seems that when altivec support was added to the e6500 kernel in
2012[1], the QEMU code was not changed, so we don't register the
VPU/VPUA exceptions in init_excp_e200. And due to the way
spr_write_excp_vector works, when the kernel code tries to write the
IVOR for altivec exceptions, it ends up writing over SPEU (#32) and
DFPDI (#33) instead.

arch/powerpc/kernel/exceptions-64e.S:

_GLOBAL(setup_altivec_ivors)
        SET_IVOR(32, 0x200) /* AltiVec Unavailable */
        SET_IVOR(33, 0x220) /* AltiVec Assist */
        blr


target/ppc/translate.c:

void spr_write_excp_vector(DisasContext *ctx, int sprn, int gprn)
{
    int sprn_offs;

    if (sprn >= SPR_BOOKE_IVOR0 && sprn <= SPR_BOOKE_IVOR15) {
        sprn_offs = sprn - SPR_BOOKE_IVOR0;
    } else if (sprn >= SPR_BOOKE_IVOR32 && sprn <= SPR_BOOKE_IVOR37) {
        sprn_offs = sprn - SPR_BOOKE_IVOR32 + 32;
    } else if (sprn >= SPR_BOOKE_IVOR38 && sprn <= SPR_BOOKE_IVOR42) {
        sprn_offs = sprn - SPR_BOOKE_IVOR38 + 38;
    } else {
        printf("Trying to write an unknown exception vector %d %03x\n",
               sprn, sprn);
        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
        return;
    }

    TCGv t0 = tcg_temp_new();
    tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUPPCState, ivor_mask));
    tcg_gen_and_tl(t0, t0, cpu_gpr[gprn]);
    tcg_gen_st_tl(t0, cpu_env, offsetof(CPUPPCState, excp_vectors[sprn_offs])); 
<-- HERE
    gen_store_spr(sprn, t0);
    tcg_temp_free(t0);
}


runtime:

Thread 4 "qemu-system-ppc" hit Breakpoint 1, spr_write_excp_vector
(gdb) p sprn_offs 
$6 = 32

Thread 4 "qemu-system-ppc" hit Breakpoint 2, powerpc_excp
(gdb) p/x env->excp_vectors[32] <-- SPEU
$1 = 0x1200
(gdb) p/x env->excp_vectors[33] <-- DFPDI
$2 = 0x1220

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):

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 */
---

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

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


1- 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd66cc2ee52

>
> Mario, did you try booting the kernel with u-boot instead of using -kernel 
> or do you have more info on how this works on real hardware? If you can 
> find what code this stvx instruction is part of then we may see if it 
> expects an exception that would make it fall back to non-altivec here. 
> That may also explain why it works on real hardware but may also mean that 
> it does not use altivec where it could.
>
> Regards,
> BALATON Zoltan