[PATCH 0/3] few hppa fixes for 64bit mode

[PATCH 0/3] few hppa fixes for 64bit mode

[Sven Schnelle]

Hi,

in preparation of getting 64bit HP-UX running in qemu, here are a few fixes
to make HP-UX progress a bit further.

Sven Schnelle (3):
  target/hppa: use gva_offset_mask() everywhere
  target/hppa: mask offset bits in gva
  target/hppa: fix building gva for wide mode

 target/hppa/cpu.h        | 11 +++++++++--
 target/hppa/mem_helper.c | 13 +++++++------
 target/hppa/translate.c  | 12 +++---------
 3 files changed, 19 insertions(+), 17 deletions(-)

-- 
2.43.2

[PATCH 1/3] target/hppa: use gva_offset_mask() everywhere

[Sven Schnelle]

move it to cpu.h, so it can also be used in hppa_form_gva_psw()

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/cpu.h       | 10 ++++++++--
 target/hppa/translate.c | 12 +++---------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index a92dc352cb..a072d0bb63 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -285,14 +285,20 @@ void hppa_translate_init(void);
 
 #define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
 
+static inline uint64_t gva_offset_mask(target_ulong psw)
+{
+    return (psw & PSW_W
+            ? MAKE_64BIT_MASK(0, 62)
+            : MAKE_64BIT_MASK(0, 32));
+}
+
 static inline target_ulong hppa_form_gva_psw(target_ulong psw, uint64_t spc,
                                              target_ulong off)
 {
 #ifdef CONFIG_USER_ONLY
     return off;
 #else
-    off &= psw & PSW_W ? MAKE_64BIT_MASK(0, 62) : MAKE_64BIT_MASK(0, 32);
-    return spc | off;
+    return spc | (off & gva_offset_mask(psw));
 #endif
 }
 
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 19594f917e..0af125ed74 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -586,17 +586,10 @@ static bool nullify_end(DisasContext *ctx)
     return true;
 }
 
-static uint64_t gva_offset_mask(DisasContext *ctx)
-{
-    return (ctx->tb_flags & PSW_W
-            ? MAKE_64BIT_MASK(0, 62)
-            : MAKE_64BIT_MASK(0, 32));
-}
-
 static void copy_iaoq_entry(DisasContext *ctx, TCGv_i64 dest,
                             uint64_t ival, TCGv_i64 vval)
 {
-    uint64_t mask = gva_offset_mask(ctx);
+    uint64_t mask = gva_offset_mask(ctx->tb_flags);
 
     if (ival != -1) {
         tcg_gen_movi_i64(dest, ival & mask);
@@ -1403,7 +1396,8 @@ static void form_gva(DisasContext *ctx, TCGv_i64 *pgva, TCGv_i64 *pofs,
 
     *pofs = ofs;
     *pgva = addr = tcg_temp_new_i64();
-    tcg_gen_andi_i64(addr, modify <= 0 ? ofs : base, gva_offset_mask(ctx));
+    tcg_gen_andi_i64(addr, modify <= 0 ? ofs : base,
+                     gva_offset_mask(ctx->tb_flags));
 #ifndef CONFIG_USER_ONLY
     if (!is_phys) {
         tcg_gen_or_i64(addr, addr, space_select(ctx, sp, base));
-- 
2.43.2

Re: [PATCH 1/3] target/hppa: use gva_offset_mask() everywhere

[Helge Deller]

On 3/24/24 09:09, Sven Schnelle wrote:
> move it to cpu.h, so it can also be used in hppa_form_gva_psw()
>
> Signed-off-by: Sven Schnelle <svens@stackframe.org>

Reviewed-by: Helge Deller <deller@gmx.de>

Helge

> ---
>   target/hppa/cpu.h       | 10 ++++++++--
>   target/hppa/translate.c | 12 +++---------
>   2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
> index a92dc352cb..a072d0bb63 100644
> --- a/target/hppa/cpu.h
> +++ b/target/hppa/cpu.h
> @@ -285,14 +285,20 @@ void hppa_translate_init(void);
>
>   #define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
>
> +static inline uint64_t gva_offset_mask(target_ulong psw)
> +{
> +    return (psw & PSW_W
> +            ? MAKE_64BIT_MASK(0, 62)
> +            : MAKE_64BIT_MASK(0, 32));
> +}
> +
>   static inline target_ulong hppa_form_gva_psw(target_ulong psw, uint64_t spc,
>                                                target_ulong off)
>   {
>   #ifdef CONFIG_USER_ONLY
>       return off;
>   #else
> -    off &= psw & PSW_W ? MAKE_64BIT_MASK(0, 62) : MAKE_64BIT_MASK(0, 32);
> -    return spc | off;
> +    return spc | (off & gva_offset_mask(psw));
>   #endif
>   }
>
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 19594f917e..0af125ed74 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -586,17 +586,10 @@ static bool nullify_end(DisasContext *ctx)
>       return true;
>   }
>
> -static uint64_t gva_offset_mask(DisasContext *ctx)
> -{
> -    return (ctx->tb_flags & PSW_W
> -            ? MAKE_64BIT_MASK(0, 62)
> -            : MAKE_64BIT_MASK(0, 32));
> -}
> -
>   static void copy_iaoq_entry(DisasContext *ctx, TCGv_i64 dest,
>                               uint64_t ival, TCGv_i64 vval)
>   {
> -    uint64_t mask = gva_offset_mask(ctx);
> +    uint64_t mask = gva_offset_mask(ctx->tb_flags);
>
>       if (ival != -1) {
>           tcg_gen_movi_i64(dest, ival & mask);
> @@ -1403,7 +1396,8 @@ static void form_gva(DisasContext *ctx, TCGv_i64 *pgva, TCGv_i64 *pofs,
>
>       *pofs = ofs;
>       *pgva = addr = tcg_temp_new_i64();
> -    tcg_gen_andi_i64(addr, modify <= 0 ? ofs : base, gva_offset_mask(ctx));
> +    tcg_gen_andi_i64(addr, modify <= 0 ? ofs : base,
> +                     gva_offset_mask(ctx->tb_flags));
>   #ifndef CONFIG_USER_ONLY
>       if (!is_phys) {
>           tcg_gen_or_i64(addr, addr, space_select(ctx, sp, base));

Re: [PATCH 1/3] target/hppa: use gva_offset_mask() everywhere

[Richard Henderson]

On 3/23/24 22:09, Sven Schnelle wrote:
> move it to cpu.h, so it can also be used in hppa_form_gva_psw()
> 
> Signed-off-by: Sven Schnelle<svens@stackframe.org>
> ---
>   target/hppa/cpu.h       | 10 ++++++++--
>   target/hppa/translate.c | 12 +++---------
>   2 files changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 2/3] target/hppa: mask offset bits in gva

[Sven Schnelle]

The CPU seems to mask a few bits in the offset when running
under HP-UX. ISR/IOR register contents for an address in
the processor HPA (0xfffffffffffa0000) on my C8000 and J6750:

running on Linux: 000000003fffffff c0000000fffa0500
running on HP-UX: 00000000301fffff c0000000fffa0500

I haven't found how this is switched (guess some diag in the
firmware), but linux + seabios seems to handle that as well,
so lets mask out the additional bits.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/cpu.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index a072d0bb63..9bc4d208fa 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -283,12 +283,13 @@ static inline int HPPA_BTLB_ENTRIES(CPUHPPAState *env)
 
 void hppa_translate_init(void);
 
+#define HPPA_GVA_OFFSET_MASK64 0x301fffffffffffff
 #define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
 
 static inline uint64_t gva_offset_mask(target_ulong psw)
 {
     return (psw & PSW_W
-            ? MAKE_64BIT_MASK(0, 62)
+            ? HPPA_GVA_OFFSET_MASK64
             : MAKE_64BIT_MASK(0, 32));
 }
 
-- 
2.43.2

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Helge Deller]

On 3/24/24 09:09, Sven Schnelle wrote:
> The CPU seems to mask a few bits in the offset when running
> under HP-UX. ISR/IOR register contents for an address in
> the processor HPA (0xfffffffffffa0000) on my C8000 and J6750:
>
> running on Linux: 000000003fffffff c0000000fffa0500
> running on HP-UX: 00000000301fffff c0000000fffa0500
>
> I haven't found how this is switched (guess some diag in the
> firmware), but linux + seabios seems to handle that as well,
> so lets mask out the additional bits.
>
> Signed-off-by: Sven Schnelle <svens@stackframe.org>

I've seen the issue on HP-UX too, and can confirm the patch does
not break existing 32- and 64-bit Linux installations, so:

Tested-by: Helge Deller <deller@gmx.de>

Thanks!
Helge


> ---
>   target/hppa/cpu.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
> index a072d0bb63..9bc4d208fa 100644
> --- a/target/hppa/cpu.h
> +++ b/target/hppa/cpu.h
> @@ -283,12 +283,13 @@ static inline int HPPA_BTLB_ENTRIES(CPUHPPAState *env)
>
>   void hppa_translate_init(void);
>
> +#define HPPA_GVA_OFFSET_MASK64 0x301fffffffffffff
>   #define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
>
>   static inline uint64_t gva_offset_mask(target_ulong psw)
>   {
>       return (psw & PSW_W
> -            ? MAKE_64BIT_MASK(0, 62)
> +            ? HPPA_GVA_OFFSET_MASK64
>               : MAKE_64BIT_MASK(0, 32));
>   }
>

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Richard Henderson]

On 3/23/24 22:09, Sven Schnelle wrote:
> The CPU seems to mask a few bits in the offset when running
> under HP-UX. ISR/IOR register contents for an address in
> the processor HPA (0xfffffffffffa0000) on my C8000 and J6750:
> 
> running on Linux: 000000003fffffff c0000000fffa0500
> running on HP-UX: 00000000301fffff c0000000fffa0500
> 
> I haven't found how this is switched (guess some diag in the
> firmware), but linux + seabios seems to handle that as well,
> so lets mask out the additional bits.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>   target/hppa/cpu.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
> index a072d0bb63..9bc4d208fa 100644
> --- a/target/hppa/cpu.h
> +++ b/target/hppa/cpu.h
> @@ -283,12 +283,13 @@ static inline int HPPA_BTLB_ENTRIES(CPUHPPAState *env)
>   
>   void hppa_translate_init(void);
>   
> +#define HPPA_GVA_OFFSET_MASK64 0x301fffffffffffff
>   #define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
>   
>   static inline uint64_t gva_offset_mask(target_ulong psw)
>   {
>       return (psw & PSW_W
> -            ? MAKE_64BIT_MASK(0, 62)
> +            ? HPPA_GVA_OFFSET_MASK64
>               : MAKE_64BIT_MASK(0, 32));
>   }
>   

I'm not keen on this, because it contradicts the manual for forming an address.

Where I can imagine this sort of thing creeping in is the fact that you're getting a 
result from trap registers.  The cpu does not actually retain the original {space, offset} 
tuple that formed the GVA to fill the trap registers, but takes bits [62:32] and 
back-computes a space, and subtracts to re-form an offset.  See "Interruption Parameter 
Registers" in the pa20 manual.

In particular Figure 2-14 for "data translation disabled" may be instructive.  Suppose the 
cpu does not implement all of the physical address lines (true for all extant pa-risc 
cpus; qemu implements 40 bits to match pa-8500 iirc).  Suppose when reporting a trap with 
translation disabled, it is a truncated physical address that is used as input to Figure 2-14.

If that is so, then the fix might be in hppa_set_ior_and_isr.  Perhaps

-    env->cr[CR_ISR] &= 0x3fffffff;
+    env->cr[CR_ISR] &= 0x301fffff;

Though my argument would suggest the mask should be 0xff for the 40-bit physical address, 
which is not what you see at all, so perhaps the thing is moot.  I am at a loss to explain 
why or how HP-UX gets a 7-bit hole in the ISR result.

On the other hand, there are some not-well-documented shenanigans (aka implementation 
defined behaviour) between Figure H-8 and Figure H-11, where the 62-bit absolute address 
is expanded to a 64-bit logical physical address and then compacted to a 40-bit 
implementation physical address.

We've already got hacks in place for this in hppa_abs_to_phys_pa2_w1, which just truncates 
everything down to 40 bits.  But that's probably not what the processor is really doing.

Anyhow, will you please try the hppa_set_ior_and_isr change and see if that fixes your 
HP-UX problems?


r~

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Sven Schnelle]

Hi Richard,

Richard Henderson <richard.henderson@linaro.org> writes:

> In particular Figure 2-14 for "data translation disabled" may be
> instructive.  Suppose the cpu does not implement all of the physical
> address lines (true for all extant pa-risc cpus; qemu implements 40
> bits to match pa-8500 iirc).  Suppose when reporting a trap with
> translation disabled, it is a truncated physical address that is used
> as input to Figure 2-14.
>
> If that is so, then the fix might be in hppa_set_ior_and_isr.  Perhaps
>
> -    env->cr[CR_ISR] &= 0x3fffffff;
> +    env->cr[CR_ISR] &= 0x301fffff;
>
> Though my argument would suggest the mask should be 0xff for the
> 40-bit physical address, which is not what you see at all, so perhaps
> the thing is moot.  I am at a loss to explain why or how HP-UX gets a
> 7-bit hole in the ISR result.
>
> On the other hand, there are some not-well-documented shenanigans (aka
> implementation defined behaviour) between Figure H-8 and Figure H-11,
> where the 62-bit absolute address is expanded to a 64-bit logical
> physical address and then compacted to a 40-bit implementation
> physical address.
>
> We've already got hacks in place for this in hppa_abs_to_phys_pa2_w1,
> which just truncates everything down to 40 bits.  But that's probably
> not what the processor is really doing.
>
> Anyhow, will you please try the hppa_set_ior_and_isr change and see if
> that fixes your HP-UX problems?

The problem occurs with data address translation - it's working without,
which is not suprising because no exception can happen there. But as
soon as the kernel enables address translation it will hit a data tlb
miss exception because it can't find 0xfffffffffffb0500 in the page
tables. Trying to truncate the ISR in hppa_set_ior_and_isr() for the
data translation enabled case leads to this loop:

hppa_tlb_fill_excp env=0x55bf06e976e0 addr=0x3ffffffffffb0500 size=4 type=0 mmu_idx=9
hppa_tlb_find_entry env=0x55bf06e976e0 ent=0x55bf06e97b30 valid=1 va_b=0x200000 va_e=0x2fffff pa=0x200000
hppa_tlb_get_physical_address env=0x55bf06e976e0 ret=-1 prot=5 addr=0x26170c phys=0x26170c
hppa_tlb_flush_ent env=0x55bf06e976e0 ent=0x55bf06e97bf0 va_b=0x301ffffffffb0000 va_e=0x301ffffffffb0fff pa=0xfffffffffffb0000
hppa_tlb_itlba env=0x55bf06e976e0 ent=0x55bf06e97bf0 va_b=0x301ffffffffb0000 va_e=0x301ffffffffb0fff pa=0xfffffffffffb0000
hppa_tlb_itlbp env=0x55bf06e976e0 ent=0x55bf06e97bf0 access_id=0 u=1 pl2=0 pl1=0 type=1 b=0 d=0 t=0

So qemu is looking up 0x3ffffffffffb0500 in the TLB, can't find it,
raises an exception, HP-UX says: "ah nice, i have a translation for
you", but that doesn't match because we're only stripping the bits
in the ISR.

As i was a bit puzzled in the beginning what's going on, i dumped the
pagetables and wrote a small dump program:

680000: val=000f47ff301fffff r2=110e0f0000000001 r1=01ffffffffe8ffe0 phys=fffffffff47ff000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
680020: val=000f47fe301fffff r2=110e0f0000000001 r1=01ffffffffe8ffc0 phys=fffffffff47fe000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
680060: val=000f47fc301fffff r2=110e0f0000000001 r1=01ffffffffe8ff80 phys=fffffffff47fc000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5860: val=000fed3c301fffff r2=010e000000000001 r1=01fffffffffda780 phys=fffffffffed3c000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d58e0: val=000fed38301fffff r2=010e000000000001 r1=01fffffffffda700 phys=fffffffffed38000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d59a0: val=000fed32301fffff r2=010e000000000001 r1=01fffffffffda640 phys=fffffffffed32000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d59e0: val=000fed30301fffff r2=110e0f0000000001 r1=01fffffffffda600 phys=fffffffffed30000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5a00: val=000fed2f301fffff r2=010e000000000001 r1=01fffffffffda5e0 phys=fffffffffed2f000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5a20: val=000fed2e301fffff r2=010e000000000001 r1=01fffffffffda5c0 phys=fffffffffed2e000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5a40: val=000fed2d301fffff r2=010e000000000001 r1=01fffffffffda5a0 phys=fffffffffed2d000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5a60: val=000fed2c301fffff r2=010e000000000001 r1=01fffffffffda580 phys=fffffffffed2c000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5a80: val=000fed2b301fffff r2=010e000000000001 r1=01fffffffffda560 phys=fffffffffed2b000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5aa0: val=000fed2a301fffff r2=010e000000000001 r1=01fffffffffda540 phys=fffffffffed2a000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5ac0: val=000fed29301fffff r2=010e000000000001 r1=01fffffffffda520 phys=fffffffffed29000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5ae0: val=000fed28301fffff r2=010e000000000001 r1=01fffffffffda500 phys=fffffffffed28000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5b00: val=000fed27301fffff r2=010e000000000001 r1=01fffffffffda4e0 phys=fffffffffed27000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5b20: val=000fed26301fffff r2=010e000000000001 r1=01fffffffffda4c0 phys=fffffffffed26000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5b40: val=000fed25301fffff r2=010e000000000001 r1=01fffffffffda4a0 phys=fffffffffed25000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5b60: val=000fed24301fffff r2=010e000000000001 r1=01fffffffffda480 phys=fffffffffed24000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5b80: val=000fed23301fffff r2=010e000000000001 r1=01fffffffffda460 phys=fffffffffed23000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5ba0: val=000fed22301fffff r2=110e0f0000000001 r1=01fffffffffda440 phys=fffffffffed22000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5bc0: val=000fed21301fffff r2=010e000000000001 r1=01fffffffffda420 phys=fffffffffed21000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5be0: val=000fed20301fffff r2=010e000000000001 r1=01fffffffffda400 phys=fffffffffed20000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5de0: val=000fed10301fffff r2=010e000000000001 r1=01fffffffffda200 phys=fffffffffed10000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7d5fe0: val=000fed00301fffff r2=110e0f0000000001 r1=01fffffffffda000 phys=fffffffffed00000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7f07e0: val=000fffc0301fffff r2=010e000000000001 r1=01fffffffffff800 phys=fffffffffffc0000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
7f09e0: val=000fffb0301fffff r2=110e0f0000000001 r1=01fffffffffff600 phys=fffffffffffb0000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)

'val' is the value constructed from IOR/ISR, r1/r2 are the args for the
idtlbt instructions, while the GVA just stays in IOR/ISR. If you look
at the val value, you'll recognize the 0301ffff... part. First i was
assuming some bug when creating the pagetables, but dumping pagetables
on my C3750/J6750 showed the same values.

The fastpath of the fault handler is:

         $i_dtlb_miss_2_0      
$TLB$:0002a1e0 02 a0 08 a9    mfctl      IOR,r9
$TLB$:0002a1e4 d9 21 0a 6c    extrd,u,*  r9,51,20,r1
$TLB$:0002a1e8 02 80 08 a8    mfctl      ISR,r8
$TLB$:0002a1ec 35 18 00 00    copy       r8,r24
$TLB$:0002a1f0 f0 28 06 96    depd,*     r8,43,10,r1
$TLB$:0002a1f4 d9 11 1a aa    extrd,u,*  r8,53,54,r17
         dtlb_bl_patch_2_0                     
$TLB$:0002a1f8 e8 00 18 80    b          dtlbmss_PCXU
$TLB$:0002a1fc 0a 21 02 91    xor        r1,r17,r17
         dtlbmss_PCXU                              
$TLB$:0002ae40 d9 19 03 e0    extrd,u,*  r8,31,32,r25
$TLB$:0002ae44 0b 21 02 99    xor        r1,r25,r25
$TLB$:0002ae48 f3 19 0c 0c    depd,*     r25,31,20,r24
         pdir_base_patch_017                            
$TLB$:0002ae4c 20 20 00 0a    ldil       0x500000,r1
         pdir_shift_patch_017
$TLB$:0002ae50 f0 21 00 00    depd,z,*   r1,0x3f,0x20,r1
         pdir_mask_patch_017
$TLB$:0002ae54 f0 31 04 a8    depd,*     r17,58,24,r1
$TLB$:0002ae58 0c 20 10 d1    ldd        0x0(r1),r17
$TLB$:0002ae5c bf 11 20 5a    cmpb,*<>,n r17,r24,d_target_miss_PCXU
$TLB$:0002ae60 50 29 00 20    ldd        0x10(r1),r9
$TLB$:0002ae64 0c 30 10 c8    ldd        0x8(r1),r8
$TLB$:0002ae68 d9 10 02 de    extrd,u,*  r8,0x16,0x2,r16
$TLB$:0002ae6c 8e 06 20 12    cmpib,<>,n 0x3,r16,make_nop_if_split_TLB_2_0_7
$TLB$:0002ae70 05 09 18 00    idtlbt     r9,r8
$TLB$:0002ae74 00 00 0c a0    rfi,r
$TLB$:0002ae78 08 00 02 40    nop

So the patch above was the only thing i could come up with - if you have
any better idea, let me know.

I also patched linux to execute exactly the same instruction with the
same address (space is 0), and i've seen different ISR/IOR values
compared to the values presented when HPUX is running. I think the
only explanation is that HPUX or firmware switches the behaviour
during runtime.

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Richard Henderson]

On 3/24/24 08:41, Sven Schnelle wrote:
> 7f09e0: val=000fffb0301fffff r2=110e0f0000000001 r1=01fffffffffff600 phys=fffffffffffb0000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
> 
> 'val' is the value constructed from IOR/ISR,

Is this byte swapped in some weird way?  I do not see how 'val' corresponds to any of the 
addresses we're talking about.  From here, the string "301fffff" appears to an 
out-of-context grep hit.


r~

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Sven Schnelle]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 3/24/24 08:41, Sven Schnelle wrote:
>> 7f09e0: val=000fffb0301fffff r2=110e0f0000000001 r1=01fffffffffff600 phys=fffffffffffb0000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW)
>> 'val' is the value constructed from IOR/ISR,
>
> Is this byte swapped in some weird way?  I do not see how 'val'
> corresponds to any of the addresses we're talking about.  From here,
> the string "301fffff" appears to an out-of-context grep hit.

It's just both values combined together, where the 301fffff is basically
the ISR content. It's not a out of context grep - the real machines i have
are constructing the same value, and the same offset into the pagetable.
I verified that by patching the DTLB miss handler in HPUX to write the
ISR/IOR and calulated pagetable offset/value to PAGE0 and looked with
the kernel debugger at the values.

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Sven Schnelle]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 3/23/24 22:09, Sven Schnelle wrote:
>> The CPU seems to mask a few bits in the offset when running
>> under HP-UX. ISR/IOR register contents for an address in
>> the processor HPA (0xfffffffffffa0000) on my C8000 and J6750:
>> running on Linux: 000000003fffffff c0000000fffa0500
>> running on HP-UX: 00000000301fffff c0000000fffa0500
>> I haven't found how this is switched (guess some diag in the
>> firmware), but linux + seabios seems to handle that as well,
>> so lets mask out the additional bits.
>> Signed-off-by: Sven Schnelle <svens@stackframe.org>
>> [..]
> [..]
> Though my argument would suggest the mask should be 0xff for the
> 40-bit physical address, which is not what you see at all, so perhaps
> the thing is moot.  I am at a loss to explain why or how HP-UX gets a
> 7-bit hole in the ISR result.
>
> On the other hand, there are some not-well-documented shenanigans (aka
> implementation defined behaviour) between Figure H-8 and Figure H-11,
> where the 62-bit absolute address is expanded to a 64-bit logical
> physical address and then compacted to a 40-bit implementation
> physical address.
>
> We've already got hacks in place for this in hppa_abs_to_phys_pa2_w1,
> which just truncates everything down to 40 bits.  But that's probably
> not what the processor is really doing.

I looked into this again, and it's caused by Space-ID hashing. HP-UX asks
PDC/Firmware how many bits are used for the hashing. seabios returns
zero, in which case HP-UX uses a default mask of 0xf01fffffffffffff.
By modifying seabios, i can make HP-UX use the appropriate mask, but
switching of SpaceID hashing entirely is impossible. The reason why
the CPU doesn't strip the bits when running linux is that Linux switches
of Space-ID hashing early in the startup code (before mm gets
initialized).

My J6750 Firmware only returns two values: 0 when Space-ID hashing is
off, 0xfe0 when it is enabled. This is hardcoded in the firmware - the
only thing PDC checks is a bit in Debug Register 2, which enables
Space-ID hashing. 0xfe0 matches the 0xf01f... mask used by HP-UX
pretty well.

So if qemu wants to run 64 Bit HP-UX the proper way, i guess it needs
to implement Space-ID hashing.

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Sven Schnelle]

Richard,

Sven Schnelle <svens@stackframe.org> writes:

> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> On 3/23/24 22:09, Sven Schnelle wrote:
>>> The CPU seems to mask a few bits in the offset when running
>>> under HP-UX. ISR/IOR register contents for an address in
>>> the processor HPA (0xfffffffffffa0000) on my C8000 and J6750:
>>> running on Linux: 000000003fffffff c0000000fffa0500
>>> running on HP-UX: 00000000301fffff c0000000fffa0500
>>> I haven't found how this is switched (guess some diag in the
>>> firmware), but linux + seabios seems to handle that as well,
>>> so lets mask out the additional bits.
>>> Signed-off-by: Sven Schnelle <svens@stackframe.org>
>>> [..]
>> [..]
>> Though my argument would suggest the mask should be 0xff for the
>> 40-bit physical address, which is not what you see at all, so perhaps
>> the thing is moot.  I am at a loss to explain why or how HP-UX gets a
>> 7-bit hole in the ISR result.
>>
>> On the other hand, there are some not-well-documented shenanigans (aka
>> implementation defined behaviour) between Figure H-8 and Figure H-11,
>> where the 62-bit absolute address is expanded to a 64-bit logical
>> physical address and then compacted to a 40-bit implementation
>> physical address.
>>
>> We've already got hacks in place for this in hppa_abs_to_phys_pa2_w1,
>> which just truncates everything down to 40 bits.  But that's probably
>> not what the processor is really doing.
>
> I looked into this again, and it's caused by Space-ID hashing. HP-UX asks
> PDC/Firmware how many bits are used for the hashing. seabios returns
> zero, in which case HP-UX uses a default mask of 0xf01fffffffffffff.
> By modifying seabios, i can make HP-UX use the appropriate mask, but
> switching of SpaceID hashing entirely is impossible. The reason why
> the CPU doesn't strip the bits when running linux is that Linux switches
> of Space-ID hashing early in the startup code (before mm gets
> initialized).
>
> My J6750 Firmware only returns two values: 0 when Space-ID hashing is
> off, 0xfe0 when it is enabled. This is hardcoded in the firmware - the
> only thing PDC checks is a bit in Debug Register 2, which enables
> Space-ID hashing. 0xfe0 matches the 0xf01f... mask used by HP-UX
> pretty well.
>
> So if qemu wants to run 64 Bit HP-UX the proper way, i guess it needs
> to implement Space-ID hashing.

I wonder wether it would be acceptable to implement this masking in a
switchable way?

This would mean:

Implement dr2 and the mfdiag/mtdiag instructions. dr2 contains a bit
which enables/disables space id hashing. Seabios would then set
this bit when booting. Linux would disable it again during boot (this
would be the same like on real hardware), while HP-UX would leave it
enabled.

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Richard Henderson]

On 4/1/24 20:01, Sven Schnelle wrote:
> Implement dr2 and the mfdiag/mtdiag instructions. dr2 contains a bit
> which enables/disables space id hashing. Seabios would then set
> this bit when booting. Linux would disable it again during boot (this
> would be the same like on real hardware), while HP-UX would leave it
> enabled.

Pointer to documentation?


r~

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Sven Schnelle]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 4/1/24 20:01, Sven Schnelle wrote:
>> Implement dr2 and the mfdiag/mtdiag instructions. dr2 contains a bit
>> which enables/disables space id hashing. Seabios would then set
>> this bit when booting. Linux would disable it again during boot (this
>> would be the same like on real hardware), while HP-UX would leave it
>> enabled.
>
> Pointer to documentation?

There's no documentation about that in the public. There's this code since the
beginning of linux on hppa in the linux kernel (arch/parisc/kernel/pacache.S):

/* Disable Space Register Hashing for PCXL */

.word         0x141c0600      /* mfdiag %dr0, %r28 */
depwi           0,28,2, %r28  /* Clear DHASH_EN & IHASH_EN */
.word         0x141c0240      /* mtdiag %r28, %dr0 */
b,n           srdis_done

srdis_pa20:

/* Disable Space Register Hashing for PCXU,PCXU+,PCXW,PCXW+,PCXW2 */

.word         0x144008bc            /* mfdiag %dr2, %r28 */
depdi         0, 54,1, %r28         /* clear DIAG_SPHASH_ENAB (bit 54) */
.word         0x145c1840            /* mtdiag %r28, %dr2 */

So PCXL (32 bit) uses dr0, while 64 bit uses dr2. This still is the same
on my C8000 - i see firmware still contains code reading dr2 to figure
out whether space id hashing is enabled. The mfdiag/mtdiag instructions
are described in the PCXL/PCXL2 ERS.

https://parisc.wiki.kernel.org/index.php/File:PCXL_ers.pdf
https://parisc.wiki.kernel.org/index.php/File:Pcxl2_ers.pdf

There was a discussion mentioning disabling Space ID hashing in Linux:

https://yhbt.net/lore/linux-parisc/199912161642.IAA11478@lucy.cup.hp.com/

Re: [PATCH 2/3] target/hppa: mask offset bits in gva

[Helge Deller]

On 4/2/24 08:29, Sven Schnelle wrote:
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> On 4/1/24 20:01, Sven Schnelle wrote:
>>> Implement dr2 and the mfdiag/mtdiag instructions. dr2 contains a bit
>>> which enables/disables space id hashing. Seabios would then set
>>> this bit when booting. Linux would disable it again during boot (this
>>> would be the same like on real hardware), while HP-UX would leave it
>>> enabled.

This is how it works on real physical hardware, and since qemu should mimic
real hardware as best as it can, IMHO we should implement it exactly like this.

Helge

>> Pointer to documentation?
>
> There's no documentation about that in the public. There's this code since the
> beginning of linux on hppa in the linux kernel (arch/parisc/kernel/pacache.S):
>
> /* Disable Space Register Hashing for PCXL */
>
> .word         0x141c0600      /* mfdiag %dr0, %r28 */
> depwi           0,28,2, %r28  /* Clear DHASH_EN & IHASH_EN */
> .word         0x141c0240      /* mtdiag %r28, %dr0 */
> b,n           srdis_done
>
> srdis_pa20:
>
> /* Disable Space Register Hashing for PCXU,PCXU+,PCXW,PCXW+,PCXW2 */
>
> .word         0x144008bc            /* mfdiag %dr2, %r28 */
> depdi         0, 54,1, %r28         /* clear DIAG_SPHASH_ENAB (bit 54) */
> .word         0x145c1840            /* mtdiag %r28, %dr2 */
>
> So PCXL (32 bit) uses dr0, while 64 bit uses dr2. This still is the same
> on my C8000 - i see firmware still contains code reading dr2 to figure
> out whether space id hashing is enabled. The mfdiag/mtdiag instructions
> are described in the PCXL/PCXL2 ERS.
>
> https://parisc.wiki.kernel.org/index.php/File:PCXL_ers.pdf
> https://parisc.wiki.kernel.org/index.php/File:Pcxl2_ers.pdf
>
> There was a discussion mentioning disabling Space ID hashing in Linux:
>
> https://yhbt.net/lore/linux-parisc/199912161642.IAA11478@lucy.cup.hp.com/
>

[PATCH 3/3] target/hppa: fix building gva for wide mode

[Sven Schnelle]

64 Bit hppa no longer has a fixed 32/32 bit split between space and
offset. Instead it uses 42 bits for the offset. The lower 10 bits of
the space are always zero, leaving 22 bits actually used. Simply or
the values together to build the gva.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/mem_helper.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 84785b5a5c..6f895fced7 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong addr, target_ulong reg)
 }
 
 static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
-                       target_ulong r2, vaddr va_b)
+                       target_ulong r2, uint64_t spc, uint64_t off)
 {
     HPPATLBEntry *ent;
-    vaddr va_e;
+    vaddr va_b, va_e;
     uint64_t va_size;
     int mask_shift;
 
+    va_b = off & gva_offset_mask(env->psw);
+    va_b |= spc << 32;
+
     mask_shift = 2 * (r1 & 0xf);
     va_size = (uint64_t)TARGET_PAGE_SIZE << mask_shift;
     va_b &= -va_size;
@@ -569,14 +572,12 @@ static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
 
 void HELPER(idtlbt_pa20)(CPUHPPAState *env, target_ulong r1, target_ulong r2)
 {
-    vaddr va_b = deposit64(env->cr[CR_IOR], 32, 32, env->cr[CR_ISR]);
-    itlbt_pa20(env, r1, r2, va_b);
+    itlbt_pa20(env, r1, r2, env->cr[CR_ISR], env->cr[CR_IOR]);
 }
 
 void HELPER(iitlbt_pa20)(CPUHPPAState *env, target_ulong r1, target_ulong r2)
 {
-    vaddr va_b = deposit64(env->cr[CR_IIAOQ], 32, 32, env->cr[CR_IIASQ]);
-    itlbt_pa20(env, r1, r2, va_b);
+    itlbt_pa20(env, r1, r2, env->cr[CR_IIASQ], env->cr[CR_IIAOQ]);
 }
 
 /* Purge (Insn/Data) TLB. */
-- 
2.43.2

Re: [PATCH 3/3] target/hppa: fix building gva for wide mode

[Helge Deller]

On 3/24/24 09:09, Sven Schnelle wrote:
> 64 Bit hppa no longer has a fixed 32/32 bit split between space and
> offset. Instead it uses 42 bits for the offset. The lower 10 bits of
> the space are always zero, leaving 22 bits actually used. Simply or
> the values together to build the gva.
>
> Signed-off-by: Sven Schnelle <svens@stackframe.org>

Reviewed-by: Helge Deller <deller@gmx.de>
Tested-by: Helge Deller <deller@gmx.de>

Helge

> ---
>   target/hppa/mem_helper.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
> index 84785b5a5c..6f895fced7 100644
> --- a/target/hppa/mem_helper.c
> +++ b/target/hppa/mem_helper.c
> @@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong addr, target_ulong reg)
>   }
>
>   static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
> -                       target_ulong r2, vaddr va_b)
> +                       target_ulong r2, uint64_t spc, uint64_t off)
>   {
>       HPPATLBEntry *ent;
> -    vaddr va_e;
> +    vaddr va_b, va_e;
>       uint64_t va_size;
>       int mask_shift;
>
> +    va_b = off & gva_offset_mask(env->psw);
> +    va_b |= spc << 32;
> +
>       mask_shift = 2 * (r1 & 0xf);
>       va_size = (uint64_t)TARGET_PAGE_SIZE << mask_shift;
>       va_b &= -va_size;
> @@ -569,14 +572,12 @@ static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
>
>   void HELPER(idtlbt_pa20)(CPUHPPAState *env, target_ulong r1, target_ulong r2)
>   {
> -    vaddr va_b = deposit64(env->cr[CR_IOR], 32, 32, env->cr[CR_ISR]);
> -    itlbt_pa20(env, r1, r2, va_b);
> +    itlbt_pa20(env, r1, r2, env->cr[CR_ISR], env->cr[CR_IOR]);
>   }
>
>   void HELPER(iitlbt_pa20)(CPUHPPAState *env, target_ulong r1, target_ulong r2)
>   {
> -    vaddr va_b = deposit64(env->cr[CR_IIAOQ], 32, 32, env->cr[CR_IIASQ]);
> -    itlbt_pa20(env, r1, r2, va_b);
> +    itlbt_pa20(env, r1, r2, env->cr[CR_IIASQ], env->cr[CR_IIAOQ]);
>   }
>
>   /* Purge (Insn/Data) TLB. */

Re: [PATCH 3/3] target/hppa: fix building gva for wide mode

[Richard Henderson]

On 3/23/24 22:09, Sven Schnelle wrote:
> 64 Bit hppa no longer has a fixed 32/32 bit split between space and
> offset. Instead it uses 42 bits for the offset. The lower 10 bits of
> the space are always zero, leaving 22 bits actually used. Simply or
> the values together to build the gva.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>   target/hppa/mem_helper.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
> index 84785b5a5c..6f895fced7 100644
> --- a/target/hppa/mem_helper.c
> +++ b/target/hppa/mem_helper.c
> @@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong addr, target_ulong reg)
>   }
>   
>   static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
> -                       target_ulong r2, vaddr va_b)
> +                       target_ulong r2, uint64_t spc, uint64_t off)
>   {
>       HPPATLBEntry *ent;
> -    vaddr va_e;
> +    vaddr va_b, va_e;
>       uint64_t va_size;
>       int mask_shift;
>   
> +    va_b = off & gva_offset_mask(env->psw);
> +    va_b |= spc << 32;

Indeed, this ought to be using

     va_b = hppa_form_gva_psw(env->psw, off, spc << 32);

With that,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 3/3] target/hppa: fix building gva for wide mode

[Richard Henderson]

On 3/23/24 22:09, Sven Schnelle wrote:
> 64 Bit hppa no longer has a fixed 32/32 bit split between space and
> offset. Instead it uses 42 bits for the offset. The lower 10 bits of
> the space are always zero, leaving 22 bits actually used. Simply or
> the values together to build the gva.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>   target/hppa/mem_helper.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
> index 84785b5a5c..6f895fced7 100644
> --- a/target/hppa/mem_helper.c
> +++ b/target/hppa/mem_helper.c
> @@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong addr, target_ulong reg)
>   }
>   
>   static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
> -                       target_ulong r2, vaddr va_b)
> +                       target_ulong r2, uint64_t spc, uint64_t off)
>   {
>       HPPATLBEntry *ent;
> -    vaddr va_e;
> +    vaddr va_b, va_e;
>       uint64_t va_size;
>       int mask_shift;
>   
> +    va_b = off & gva_offset_mask(env->psw);
> +    va_b |= spc << 32;

Actually, no, these instructions don't form a GVA in the normal fashion:

space ← ISR;
offset ← cat(ISR{32..63},IOR{32..63});
VIRTUAL_ADDR ← (space<<32) | (offset);

> -    vaddr va_b = deposit64(env->cr[CR_IOR], 32, 32, env->cr[CR_ISR]);

But this is wrong too.

Since the input here is from the result of a previous memory access, which populated 
ISR+IOR, I presume any masking would be done from the prior memory access.

We do need to fix the merge, per the docs though.


r~

Re: [PATCH 3/3] target/hppa: fix building gva for wide mode

[Richard Henderson]

On 3/24/24 11:39, Richard Henderson wrote:
> On 3/23/24 22:09, Sven Schnelle wrote:
>> 64 Bit hppa no longer has a fixed 32/32 bit split between space and
>> offset. Instead it uses 42 bits for the offset. The lower 10 bits of
>> the space are always zero, leaving 22 bits actually used. Simply or
>> the values together to build the gva.
>>
>> Signed-off-by: Sven Schnelle <svens@stackframe.org>
>> ---
>>   target/hppa/mem_helper.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
>> index 84785b5a5c..6f895fced7 100644
>> --- a/target/hppa/mem_helper.c
>> +++ b/target/hppa/mem_helper.c
>> @@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong addr, 
>> target_ulong reg)
>>   }
>>   static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
>> -                       target_ulong r2, vaddr va_b)
>> +                       target_ulong r2, uint64_t spc, uint64_t off)
>>   {
>>       HPPATLBEntry *ent;
>> -    vaddr va_e;
>> +    vaddr va_b, va_e;
>>       uint64_t va_size;
>>       int mask_shift;
>> +    va_b = off & gva_offset_mask(env->psw);
>> +    va_b |= spc << 32;
> 
> Actually, no, these instructions don't form a GVA in the normal fashion:
> 
> space ← ISR;
> offset ← cat(ISR{32..63},IOR{32..63});
> VIRTUAL_ADDR ← (space<<32) | (offset);
> 
>> -    vaddr va_b = deposit64(env->cr[CR_IOR], 32, 32, env->cr[CR_ISR]);
> 
> But this is wrong too.

Actually, no.  The

   VIRTUAL_ADDR = (space << 32) | offset

line constructs a 96-bit virtual address.  The low 32 bits of ISR have already replaced 
the high 32 bits of IOR, so this line really only adds the high 32 bits of space as bits 
[96:64] of the full virtual address.

Truncated to 64 bits, the deposit as written is exactly right.


r~