Re: [PATCH v2 3/6] hppa: Add support for an emulated TOC/NMI button.

2024-05-29 Thread Helge Deller

On 5/29/24 17:11, Philippe Mathieu-Daudé wrote:

Hi Helge & Richard,


Hi Philippe,


Nevermind the missed review comments, I'm revisiting this
patch while looking at building libtcg-hppa.so.


Ok.


On 1/2/22 00:56, Philippe Mathieu-Daudé wrote:

On 31/1/22 22:35, Helge Deller wrote:

Almost all PA-RISC machines have either a button that is labeled with 'TOC' or
a BMC/GSP function to trigger a TOC.  TOC is a non-maskable interrupt that is
sent to the processor.  This can be used for diagnostic purposes like obtaining
a stack trace/register dump or to enter KDB/KGDB in Linux.

This patch adds support for such an emulated TOC button.

It wires up the qemu monitor "nmi" command to trigger a TOC.  For that it


s/qemu/QEMU/ (few others).


provides the hppa_nmi function which is assigned to the nmi_monitor_handler
function pointer.  When called it raises the EXCP_TOC hardware interrupt in the
hppa_cpu_do_interrupt() function.  The interrupt function then calls the
architecturally defined TOC function in SeaBIOS-hppa firmware (at fixed address
0xf000).

According to the PA-RISC PDC specification, the SeaBIOS firmware then writes
the CPU registers into PIM (processor internal memmory) for later analysis.  In


Typo "memory".


order to write all registers it needs to know the contents of the CPU "shadow
registers" and the IASQ- and IAOQ-back values. The IAOQ/IASQ values are
provided by qemu in shadow registers when entering the SeaBIOS TOC function.
This patch adds a new aritificial opcode "getshadowregs" (0xfffdead2) which


Typo "artificial".


restores the original values of the shadow registers. With this opcode SeaBIOS
can store those registers as well into PIM before calling an OS-provided TOC
handler.

To trigger a TOC, switch to the qemu monitor with Ctrl-A C, and type in the
command "nmi".  After the TOC started the OS-debugger, exit the qemu monitor
with Ctrl-A C.


IIUC you are abusing TOC to communicate with SeaBIOS, filling
iaoq_f with SeaBIOS-specific 0xf000, unrelated to the pa2.0
spec; is that correct?


No.
A TOC on a physical machine sets the instruction pointer (iaoq_f)
to 0xf000 (for 32-bit).
Real physical firmware (and SeaBIOS) are located at that address
to be able to boot the machine.


I'm trying to see how to integrate firmware specific knowledge
into libtcg-hppa.so which is supposed to be only architectured
parts (usually we handle firmware stuffs in machine code, not
translation one).


That reset code is architectured.

Helge



Regards,

Phil.


Signed-off-by: Helge Deller 
---
  hw/hppa/machine.c    | 35 ++-
  target/hppa/cpu.c    |  2 +-
  target/hppa/cpu.h    |  5 +
  target/hppa/helper.h |  1 +
  target/hppa/insns.decode |  1 +
  target/hppa/int_helper.c | 19 ++-
  target/hppa/op_helper.c  |  7 ++-
  target/hppa/translate.c  | 10 ++
  8 files changed, 76 insertions(+), 4 deletions(-)
+static const TypeInfo machine_hppa_machine_init_typeinfo = {
+    .name = ("hppa" "-machine"),


    .name = MACHINE_TYPE_NAME("hppa"),


+    .parent = "machine",
+    .class_init = machine_hppa_machine_init_class_init,
+    .interfaces = (InterfaceInfo[]) {
+    { TYPE_NMI },
+    { }
+    },
+};







Re: [PATCH 3/4] usb/ohci-pci: deprecate, don't build by default

2024-05-28 Thread Helge Deller

On 5/28/24 12:35, Thomas Huth wrote:

On 28/05/2024 11.54, Gerd Hoffmann wrote:

The xhci host adapter is the much better choice.

Signed-off-by: Gerd Hoffmann 
---
  hw/usb/hcd-ohci-pci.c | 1 +
  hw/usb/Kconfig    | 1 -
  2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ohci-pci.c b/hw/usb/hcd-ohci-pci.c
index 33ed9b6f5a52..88de657def71 100644
--- a/hw/usb/hcd-ohci-pci.c
+++ b/hw/usb/hcd-ohci-pci.c
@@ -143,6 +143,7 @@ static void ohci_pci_class_init(ObjectClass *klass, void 
*data)
  dc->hotpluggable = false;
  dc->vmsd = _ohci;
  dc->reset = usb_ohci_reset_pci;
+    klass->deprecation_note = "use qemu-xhci instead";
  }
  static const TypeInfo ohci_pci_info = {
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 84bc7fbe36cd..c4a6ea5a687f 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -17,7 +17,6 @@ config USB_OHCI_SYSBUS
  config USB_OHCI_PCI
  bool
-    default y if PCI_DEVICES
  depends on PCI
  select USB_OHCI


Not sure whether we should disable it by default just because it is deprecated. 
We don't do that for any other devices as far as I know.

Anyway, you should add the device to docs/about/deprecated.rst to really mark 
it as deprecated, since that's our official list (AFAIK).

Also, there are still some machines that use this device:

$ grep -r USB_OHCI_PCI *
hw/hppa/Kconfig:    imply USB_OHCI_PCI
hw/mips/Kconfig:    imply USB_OHCI_PCI
hw/ppc/Kconfig:    imply USB_OHCI_PCI
hw/ppc/Kconfig:    imply USB_OHCI_PCI

pseries could certainly continue without OHCI AFAICT, but the others? Maybe 
this needs some discussion first... (thus putting some more people on CC:)


There was never a XHCI host on any of the hppa machines, but
the latest generation of HP machines do have built-in OHCI controllers.
So, deprecating OHCI in favor of XHCI will prevent emulation of HP-UX
on the hppa target.
So, for hppa the "xhci host adapter is NOT the much better choice.".

Helge



Re: [PATCH v2 41/45] target/hppa: Implement CF_PCREL

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Now that the groundwork has been laid, enabling CF_PCREL within the
> translator proper is a simple matter of updating copy_iaoq_entry
> and install_iaq_entries.
> 
> We also need to modify the unwind info, since we no longer have
> absolute addresses to install.
> 
> As expected, this reduces the runtime overhead of compilation when
> running a Linux kernel with address space randomization enabled.

Ah! I was wondering why you tried to convert to CF_PCREL at all.
So, that's the overall reason.

> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 40/45] target/hppa: Adjust priv for B,GATE at runtime

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Do not compile in the priv change based on the first
> translation; look up the PTE at execution time.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 39/45] target/hppa: Drop tlb_entry return from hppa_get_physical_address

2024-05-14 Thread Helge Deller
* Richard Henderson :
> The return-by-reference is never used.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 38/45] target/hppa: Implement PSW_X

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Use PAGE_WRITE_INV to temporarily enable write permission
> on for a given page, driven by PSW_X being set.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 37/45] target/hppa: Implement PSW_B

2024-05-14 Thread Helge Deller
* Richard Henderson :
> PSW_B causes B,GATE to trap as an illegal instruction, removing
> the sequential execution test that was merely an approximation.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 36/45] target/hppa: Manage PSW_X and PSW_B in translator

2024-05-14 Thread Helge Deller
* Richard Henderson :
> PSW_X is cleared after every instruction, and only set by RFI.
> PSW_B is cleared after every non-branch, or branch not taken,
> and only set by taken branches.  We can clear both bits with a
> single store, at most once per TB.  Taken branches set PSW_B,
> at most once per TB.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 35/45] target/hppa: Split PSW X and B into their own field

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Generally, both of these bits are cleared at the end of each
> instruction.  By separating these, we will be able to clear
> both with a single insn, instead of 2 or 3.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 34/45] target/hppa: Improve hppa_cpu_dump_state

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Print both raw IAQ_Front and IAQ_Back as well as the GVAs.
> Print control registers in system mode.
> Print floating point register if CPU_DUMP_FPU.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 33/45] target/hppa: Do not mask in copy_iaoq_entry

2024-05-14 Thread Helge Deller
* Richard Henderson :
> As with loads and stores, code offsets are kept intact until the
> full gva is formed.  In qemu, this is in cpu_get_tb_cpu_state.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 32/45] target/hppa: Store full iaoq_f and page offset of iaoq_b in TB

2024-05-14 Thread Helge Deller
* Richard Henderson :
> In preparation for CF_PCREL. store the iaoq_f in 3 parts: high
> bits in cs_base, middle bits in pc, and low bits in priv.
> For iaoq_b, set a bit for either of space or page differing,
> else the page offset.
> 
> Install iaq entries before goto_tb. The change to not record
> the full direct branch difference in TB means that we have to
> store at least iaoq_b before goto_tb.  But we since we'll need
> both updated before goto_tb for CF_PCREL, do that now.

Does this sentence make sense? ^^
For me as non-native english speaker it sounds wrong, or missing commas,
but maybe I'm just wrong...?
Other than that...:

Reviewed-by: Helge Deller 

> Signed-off-by: Richard Henderson 




Re: [PATCH v2 31/45] linux-user/hppa: Force all code addresses to PRIV_USER

2024-05-14 Thread Helge Deller
* Richard Henderson :
> The kernel does this along the return path to user mode.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

>  linux-user/hppa/target_cpu.h |  4 ++--
>  target/hppa/cpu.h|  3 +++
>  linux-user/elfload.c |  4 ++--
>  linux-user/hppa/cpu_loop.c   | 14 +++---
>  linux-user/hppa/signal.c |  6 --
>  target/hppa/cpu.c|  7 +--
>  target/hppa/gdbstub.c|  6 ++
>  target/hppa/translate.c  |  4 ++--
>  8 files changed, 31 insertions(+), 17 deletions(-)



Re: [PATCH v2 30/45] target/hppa: Use delay_excp for conditional trap on overflow

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Signed-off-by: Richard Henderson 
> ---
>  target/hppa/helper.h |  1 -
>  target/hppa/int_helper.c |  2 +-
>  target/hppa/op_helper.c  |  7 ---
>  target/hppa/translate.c  | 21 +
>  4 files changed, 14 insertions(+), 17 deletions(-)

Reviewed-by: Helge Deller 



Re: [PATCH v2 29/45] target/hppa: Use delay_excp for conditional traps

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

>  target/hppa/helper.h |  1 -
>  target/hppa/int_helper.c |  2 +-
>  target/hppa/op_helper.c  |  7 ---
>  target/hppa/translate.c  | 41 ++--
>  4 files changed, 32 insertions(+), 19 deletions(-)



Re: [PATCH v2 28/45] target/hppa: Introduce DisasDelayException

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Allow an exception to be emitted at the end of the TranslationBlock,
> leaving only the conditional branch inline.  Use it for simple
> exception instructions like break, which happen to be nullified.
> 
> Signed-off-by: Richard Henderson 
> ---
>  target/hppa/translate.c | 60 +
>  1 file changed, 55 insertions(+), 5 deletions(-)

Reviewed-by: Helge Deller 



Re: [PATCH v2 27/45] target/hppa: Remove cond_free

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Now that we do not need to free tcg temporaries, the only
> thing cond_free does is reset the condition to never.
> Instead, simply write a new condition over the old, which
> may be simply cond_make_f() for the never condition.
> 
> The do_*_cond functions do the right thing with c or cf == 0,
> so there's no need for a special case anymore.
> 
> Signed-off-by: Richard Henderson 
> ---
>  target/hppa/translate.c | 102 +++-
>  1 file changed, 27 insertions(+), 75 deletions(-)

Reviewed-by: Helge Deller 



Re: [PATCH v2 26/45] target/hppa: Use TCG_COND_TST* in trans_ftest

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Signed-off-by: Richard Henderson 
> ---
>  target/hppa/translate.c | 22 ++
>  1 file changed, 6 insertions(+), 16 deletions(-)

Reviewed-by: Helge Deller 



Re: [PATCH v2 25/45] target/hppa: Use registerfields.h for FPSR

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Define all of the context dependent field definitions.
> Use FIELD_EX32 and FIELD_DP32 with named fields instead
> of extract32 and deposit32 with raw constants.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/cpu.h| 25 +
>  target/hppa/fpu_helper.c | 26 +-
>  target/hppa/translate.c  | 18 --
>  3 files changed, 46 insertions(+), 23 deletions(-)



Re: [PATCH v2 24/45] target/hppa: Use TCG_COND_TST* in trans_bb_imm

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 47f4b23d1b..d8973a63df 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -3515,18 +3515,12 @@ static bool trans_bb_sar(DisasContext *ctx, 
> arg_bb_sar *a)
>  
>  static bool trans_bb_imm(DisasContext *ctx, arg_bb_imm *a)
>  {
> -TCGv_i64 tmp, tcg_r;
>  DisasCond cond;
> -int p;
> +int p = a->p | (a->d ? 0 : 32);
>  
>  nullify_over(ctx);
> -
> -tmp = tcg_temp_new_i64();
> -tcg_r = load_gpr(ctx, a->r);
> -p = a->p | (a->d ? 0 : 32);
> -tcg_gen_shli_i64(tmp, tcg_r, p);
> -
> -cond = cond_make_ti(a->c ? TCG_COND_GE : TCG_COND_LT, tmp, 0);
> +cond = cond_make_vi(a->c ? TCG_COND_TSTEQ : TCG_COND_TSTNE,
> +load_gpr(ctx, a->r), 1ull << (63 - p));

I wonder if this actually fixes a bug...
Before it tested against all values >= tmp (even for which the bit
wasn't set), and now it correctly just checks the bit.


>  return do_cbranch(ctx, a->disp, a->n, );
>  }
>  
> -- 
> 2.34.1
> 



Re: [PATCH v2 23/45] target/hppa: Use TCG_COND_TST* in do_unit_addsub

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 22/45] target/hppa: Use TCG_COND_TST* in do_unit_zero_cond

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 21/45] target/hppa: Use TCG_COND_TST* in do_log_cond

2024-05-14 Thread Helge Deller
* Richard Henderson :
> We can directly test bits of a 32-bit comparison without
> zero or sign-extending an intermediate result.
> We can directly test bit 0 for odd/even.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 20/45] target/hppa: Use TCG_COND_TST* in do_cond

2024-05-14 Thread Helge Deller
* Richard Henderson :
> We can directly test bits of a 32-bit comparison without
> zero or sign-extending an intermediate result.
> We can directly test bit 0 for odd/even.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 19/45] target/hppa: Rename cond_make_* helpers

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Use 'v' for a variable that needs copying, 't' for a temp that
> doesn't need copying, and 'i' for an immediate, and use this
> naming for both arguments of the comparison.  So:
> 
>cond_make_tmp -> cond_make_tt
>cond_make_0_tmp -> cond_make_ti
>cond_make_0 -> cond_make_vi
>cond_make -> cond_make_vv
> 
> Pass 0 explictly, rather than implicitly in the function name.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 18/45] target/hppa: Use displacements in DisasIAQE

2024-05-14 Thread Helge Deller
* Richard Henderson :
> This is a first step in enabling CF_PCREL, but for now
> we regenerate the absolute address before writeback.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 17/45] target/hppa: Introduce and use DisasIAQE for branch management

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Wrap offset and space together in one structure, ensuring
> that they're copied together as required.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 08/45] target/hppa: Add install_link

2024-05-14 Thread Helge Deller

On 5/14/24 16:37, Helge Deller wrote:

* Richard Henderson :

Add a common routine for writing the return address.

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 


---
  target/hppa/translate.c | 54 +++--
  1 file changed, 31 insertions(+), 23 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 08d5e2a4bc..f816b337ee 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -634,6 +634,23 @@ static void install_iaq_entries(DisasContext *ctx, 
uint64_t bi, TCGv_i64 bv,
  }
  }

+static void install_link(DisasContext *ctx, unsigned link, bool with_sr0)
+{
+tcg_debug_assert(ctx->null_cond.c == TCG_COND_NEVER);
+if (link) {


Just wondering:
Doesn't it makes it easier to write here:
if (!link) {
return;
}
and then don't indent the few following lines?


I see you change and partly revert it again in patch #17...
so forget this remark for now.

Helge



Re: [PATCH v2 14/45] target/hppa: Add space argument to do_ibranch

2024-05-14 Thread Helge Deller
* Richard Henderson :
> This allows unification of BE, BLR, BV, BVE with a common helper.
> Since we can now track space with IAQ_Next, we can now let the
> TranslationBlock continue across the delay slot with BE, BVE.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 



Re: [PATCH v2 13/45] target/hppa: Add space arguments to install_iaq_entries

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Move space assighments to a central location.
> 
> Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 58 +++--
>  1 file changed, 27 insertions(+), 31 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index d24220c60f..05383dcd04 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -624,8 +624,9 @@ static void copy_iaoq_entry(DisasContext *ctx, TCGv_i64 
> dest,
>  }
>  }
>  
> -static void install_iaq_entries(DisasContext *ctx, uint64_t bi, TCGv_i64 bv,
> -uint64_t ni, TCGv_i64 nv)
> +static void install_iaq_entries(DisasContext *ctx,
> +uint64_t bi, TCGv_i64 bv, TCGv_i64 bs,
> +uint64_t ni, TCGv_i64 nv, TCGv_i64 ns)
>  {
>  copy_iaoq_entry(ctx, cpu_iaoq_f, bi, bv);
>  
> @@ -639,6 +640,12 @@ static void install_iaq_entries(DisasContext *ctx, 
> uint64_t bi, TCGv_i64 bv,
>  tcg_gen_andi_i64(cpu_iaoq_b, cpu_iaoq_b,
>   gva_offset_mask(ctx->tb_flags));
>  }
> +if (bs) {
> +tcg_gen_mov_i64(cpu_iasq_f, bs);
> +}
> +if (ns || bs) {
> +tcg_gen_mov_i64(cpu_iasq_b, ns ? ns : bs);
> +}
>  }
>  
>  static void install_link(DisasContext *ctx, unsigned link, bool with_sr0)
> @@ -670,7 +677,8 @@ static void gen_excp_1(int exception)
>  
>  static void gen_excp(DisasContext *ctx, int exception)
>  {
> -install_iaq_entries(ctx, ctx->iaoq_f, cpu_iaoq_f, ctx->iaoq_b, 
> cpu_iaoq_b);
> +install_iaq_entries(ctx, ctx->iaoq_f, cpu_iaoq_f, NULL,
> +ctx->iaoq_b, cpu_iaoq_b, NULL);
>  nullify_save(ctx);
>  gen_excp_1(exception);
>  ctx->base.is_jmp = DISAS_NORETURN;
> @@ -724,10 +732,11 @@ static void gen_goto_tb(DisasContext *ctx, int which,
>  {
>  if (use_goto_tb(ctx, b, n)) {
>  tcg_gen_goto_tb(which);
> -install_iaq_entries(ctx, b, NULL, n, NULL);
> +install_iaq_entries(ctx, b, NULL, NULL, n, NULL, NULL);
>  tcg_gen_exit_tb(ctx->base.tb, which);
>  } else {
> -install_iaq_entries(ctx, b, cpu_iaoq_b, n, ctx->iaoq_n_var);
> +install_iaq_entries(ctx, b, cpu_iaoq_b, ctx->iasq_b,
> +n, ctx->iaoq_n_var, ctx->iasq_n);
>  tcg_gen_lookup_and_goto_ptr();
>  }
>  }
> @@ -1916,7 +1925,7 @@ static bool do_ibranch(DisasContext *ctx, TCGv_i64 dest,
>  install_link(ctx, link, false);
>  if (is_n) {
>  if (use_nullify_skip(ctx)) {
> -install_iaq_entries(ctx, -1, next, -1, NULL);
> +install_iaq_entries(ctx, -1, next, NULL, -1, NULL, NULL);
>  nullify_set(ctx, 0);
>  ctx->base.is_jmp = DISAS_IAQ_N_UPDATED;
>  return true;
> @@ -1935,10 +1944,11 @@ static bool do_ibranch(DisasContext *ctx, TCGv_i64 
> dest,
>  
>  install_link(ctx, link, false);
>  if (is_n && use_nullify_skip(ctx)) {
> -install_iaq_entries(ctx, -1, next, -1, NULL);
> +install_iaq_entries(ctx, -1, next, NULL, -1, NULL, NULL);
>  nullify_set(ctx, 0);
>  } else {
> -install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b, -1, next);
> +install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b, ctx->iasq_b,
> +-1, next, NULL);
>  nullify_set(ctx, is_n);
>  }
>  
> @@ -2026,7 +2036,7 @@ static void do_page_zero(DisasContext *ctx)
>  tcg_gen_st_i64(cpu_gr[26], tcg_env, offsetof(CPUHPPAState, cr[27]));
>  tmp = tcg_temp_new_i64();
>  tcg_gen_ori_i64(tmp, cpu_gr[31], 3);
> -install_iaq_entries(ctx, -1, tmp, -1, NULL);
> +install_iaq_entries(ctx, -1, tmp, NULL, -1, NULL, NULL);
>  ctx->base.is_jmp = DISAS_IAQ_N_UPDATED;
>  break;
>  
> @@ -2770,8 +2780,8 @@ static bool trans_or(DisasContext *ctx, arg_rrr_cf_d *a)
>  nullify_over(ctx);
>  
>  /* Advance the instruction queue.  */
> -install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b,
> -ctx->iaoq_n, ctx->iaoq_n_var);
> +install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b, ctx->iasq_b,
> +ctx->iaoq_n, ctx->iaoq_n_var, ctx->iasq_n);
>  nullify_set(ctx, 0);
>  
>  /* Tell the qemu main loop to halt until this cpu has work.  */
> @@ -3921,16 +3931,11 @@ static bool trans_be(DisasContext *ctx, 

Re: [PATCH v2 12/45] target/hppa: Add IASQ entries to DisasContext

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Add variable to track space changes to IAQ.  So far, no such changes
> are introduced, but the new checks vs ctx->iasq_b may eliminate an
> unnecessary copy to cpu_iasq_f with e.g. BLR.
> 
> Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 39 ++-
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 13a48d1b6c..d24220c60f 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -50,6 +50,13 @@ typedef struct DisasContext {
>  uint64_t iaoq_b;
>  uint64_t iaoq_n;
>  TCGv_i64 iaoq_n_var;
> +/*
> + * Null when IASQ_Back unchanged from IASQ_Front,
> + * or cpu_iasq_b, when IASQ_Back has been changed.
> + */
> +TCGv_i64 iasq_b;
> +/* Null when IASQ_Next unchanged from IASQ_Back, or set by branch. */
> +TCGv_i64 iasq_n;
>  
>  DisasCond null_cond;
>  TCGLabel *null_lab;
> @@ -3916,12 +3923,12 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
>  if (a->n && use_nullify_skip(ctx)) {
>  install_iaq_entries(ctx, -1, tmp, -1, NULL);
>  tcg_gen_mov_i64(cpu_iasq_f, new_spc);
> -tcg_gen_mov_i64(cpu_iasq_b, cpu_iasq_f);
> +tcg_gen_mov_i64(cpu_iasq_b, new_spc);
>  nullify_set(ctx, 0);
>  } else {
>  install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b, -1, tmp);
> -if (ctx->iaoq_b == -1) {
> -tcg_gen_mov_i64(cpu_iasq_f, cpu_iasq_b);
> +if (ctx->iasq_b) {
> +tcg_gen_mov_i64(cpu_iasq_f, ctx->iasq_b);
>  }
>  tcg_gen_mov_i64(cpu_iasq_b, new_spc);
>  nullify_set(ctx, a->n);
> @@ -4035,8 +4042,8 @@ static bool trans_bve(DisasContext *ctx, arg_bve *a)
>  
>  install_link(ctx, a->l, false);
>  install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b, -1, dest);
> -if (ctx->iaoq_b == -1) {
> -tcg_gen_mov_i64(cpu_iasq_f, cpu_iasq_b);
> +if (ctx->iasq_b) {
> +tcg_gen_mov_i64(cpu_iasq_f, ctx->iasq_b);
>  }
>  tcg_gen_mov_i64(cpu_iasq_b, space_select(ctx, 0, dest));
>  nullify_set(ctx, a->n);
> @@ -4617,6 +4624,7 @@ static void hppa_tr_init_disas_context(DisasContextBase 
> *dcbase, CPUState *cs)
>  ctx->mmu_idx = MMU_USER_IDX;
>  ctx->iaoq_f = ctx->base.pc_first | ctx->privilege;
>  ctx->iaoq_b = ctx->base.tb->cs_base | ctx->privilege;
> +ctx->iasq_b = NULL;
>  ctx->unalign = (ctx->tb_flags & TB_FLAG_UNALIGN ? MO_UNALN : MO_ALIGN);
>  #else
>  ctx->privilege = (ctx->tb_flags >> TB_FLAG_PRIV_SHIFT) & 3;
> @@ -4631,6 +4639,7 @@ static void hppa_tr_init_disas_context(DisasContextBase 
> *dcbase, CPUState *cs)
>  
>  ctx->iaoq_f = (ctx->base.pc_first & ~iasq_f) + ctx->privilege;
>  ctx->iaoq_b = (diff ? ctx->iaoq_f + diff : -1);
> +ctx->iasq_b = (diff ? NULL : cpu_iasq_b);
>  #endif
>  
>  ctx->zero = tcg_constant_i64(0);
> @@ -4683,6 +4692,7 @@ static void hppa_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cs)
>  
>  /* Set up the IA queue for the next insn.
> This will be overwritten by a branch.  */
> +ctx->iasq_n = NULL;
>  ctx->iaoq_n_var = NULL;
>  ctx->iaoq_n = ctx->iaoq_b == -1 ? -1 : ctx->iaoq_b + 4;
>  
> @@ -4705,7 +4715,7 @@ static void hppa_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cs)
>  return;
>  }
>  /* Note this also detects a priority change. */
> -if (ctx->iaoq_b != ctx->iaoq_f + 4) {
> +if (ctx->iaoq_b != ctx->iaoq_f + 4 || ctx->iasq_b) {
>  ctx->base.is_jmp = DISAS_IAQ_N_STALE;
>  return;
>  }
> @@ -4725,6 +4735,10 @@ static void hppa_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cs)
>   gva_offset_mask(ctx->tb_flags));
>  }
>  }
> +if (ctx->iasq_n) {
> +tcg_gen_mov_i64(cpu_iasq_b, ctx->iasq_n);
> +ctx->iasq_b = cpu_iasq_b;
> +}
>  }
>  
>  static void hppa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
> @@ -4733,14 +4747,15 @@ static void hppa_tr_tb_stop(DisasContextBase *dcbase, 
> CPUState *cs)
>  DisasJumpType is_jmp = ctx->base.is_jmp;
>  uint64_t fi, bi;
>  TCGv_i64 fv, bv;
> -TCGv_i64 fs;
> +TCGv_i64 fs, bs;
>  
>  /* Assume the insn queue has not been advanced. */
>  fi = ctx->iaoq_b;
>  fv = cpu_iaoq_b;
> -fs = fi == -1 ?

Re: [PATCH v2 11/45] target/hppa: Simplify TB end

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Minimize the amount of code in hppa_tr_translate_insn advancing the
> insn queue for the next insn.  Move the goto_tb path to hppa_tr_tb_stop.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 109 +---
>  1 file changed, 57 insertions(+), 52 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index ca979f4137..13a48d1b6c 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -4699,54 +4699,31 @@ static void hppa_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cs)
>  }
>  }
>  
> -/* Advance the insn queue.  Note that this check also detects
> -   a priority change within the instruction queue.  */
> -if (ret == DISAS_NEXT && ctx->iaoq_b != ctx->iaoq_f + 4) {
> -if (use_goto_tb(ctx, ctx->iaoq_b, ctx->iaoq_n)
> -&& (ctx->null_cond.c == TCG_COND_NEVER
> -|| ctx->null_cond.c == TCG_COND_ALWAYS)) {
> -nullify_set(ctx, ctx->null_cond.c == TCG_COND_ALWAYS);
> -gen_goto_tb(ctx, 0, ctx->iaoq_b, ctx->iaoq_n);
> -ctx->base.is_jmp = ret = DISAS_NORETURN;
> -} else {
> -ctx->base.is_jmp = ret = DISAS_IAQ_N_STALE;
> -}
> +/* If the TranslationBlock must end, do so. */
> +ctx->base.pc_next += 4;
> +if (ret != DISAS_NEXT) {
> +return;
>  }
> +/* Note this also detects a priority change. */
> +if (ctx->iaoq_b != ctx->iaoq_f + 4) {
> +ctx->base.is_jmp = DISAS_IAQ_N_STALE;
> +return;
> +}
> +
> +/*
> + * Advance the insn queue.
> + * The only exit now is DISAS_TOO_MANY from the translator loop.
> + */
>  ctx->iaoq_f = ctx->iaoq_b;
>  ctx->iaoq_b = ctx->iaoq_n;
> -ctx->base.pc_next += 4;
> -
> -switch (ret) {
> -case DISAS_NORETURN:
> -case DISAS_IAQ_N_UPDATED:
> -break;
> -
> -case DISAS_NEXT:
> -case DISAS_IAQ_N_STALE:
> -case DISAS_IAQ_N_STALE_EXIT:
> -if (ctx->iaoq_f == -1) {
> -install_iaq_entries(ctx, -1, cpu_iaoq_b,
> -ctx->iaoq_n, ctx->iaoq_n_var);
> -#ifndef CONFIG_USER_ONLY
> -tcg_gen_mov_i64(cpu_iasq_f, cpu_iasq_b);
> -#endif
> -nullify_save(ctx);
> -ctx->base.is_jmp = (ret == DISAS_IAQ_N_STALE_EXIT
> -? DISAS_EXIT
> -: DISAS_IAQ_N_UPDATED);
> -} else if (ctx->iaoq_b == -1) {
> -if (ctx->iaoq_n_var) {
> -copy_iaoq_entry(ctx, cpu_iaoq_b, -1, ctx->iaoq_n_var);
> -} else {
> -tcg_gen_addi_i64(cpu_iaoq_b, cpu_iaoq_b, 4);
> -tcg_gen_andi_i64(cpu_iaoq_b, cpu_iaoq_b,
> - gva_offset_mask(ctx->tb_flags));
> -}
> +if (ctx->iaoq_b == -1) {
> +if (ctx->iaoq_n_var) {
> +copy_iaoq_entry(ctx, cpu_iaoq_b, -1, ctx->iaoq_n_var);
> +} else {
> +tcg_gen_addi_i64(cpu_iaoq_b, cpu_iaoq_b, 4);
> +tcg_gen_andi_i64(cpu_iaoq_b, cpu_iaoq_b,
> + gva_offset_mask(ctx->tb_flags));
>  }
> -break;
> -
> -default:
> -g_assert_not_reached();
>  }
>  }
>  
> @@ -4754,23 +4731,51 @@ static void hppa_tr_tb_stop(DisasContextBase *dcbase, 
> CPUState *cs)
>  {
>  DisasContext *ctx = container_of(dcbase, DisasContext, base);
>  DisasJumpType is_jmp = ctx->base.is_jmp;
> +uint64_t fi, bi;
> +TCGv_i64 fv, bv;
> +TCGv_i64 fs;
> +
> +/* Assume the insn queue has not been advanced. */
> +fi = ctx->iaoq_b;
> +fv = cpu_iaoq_b;
> +fs = fi == -1 ? cpu_iasq_b : NULL;
> +bi = ctx->iaoq_n;
> +bv = ctx->iaoq_n_var;
>  
>  switch (is_jmp) {
>  case DISAS_NORETURN:
>  break;
>  case DISAS_TOO_MANY:
> -case DISAS_IAQ_N_STALE:
> -case DISAS_IAQ_N_STALE_EXIT:
> -install_iaq_entries(ctx, ctx->iaoq_f, cpu_iaoq_f,
> -ctx->iaoq_b, cpu_iaoq_b);
> -nullify_save(ctx);
> +/* The insn queue has not been advanced. */
> +bi = fi;
> +bv = fv;
> +fi = ctx->iaoq_f;
> +fv = NULL;
> +fs = NULL;
>  /* FALLTHRU */
> -case DISAS_IAQ_N_UPDATED:
> -if (is_jmp != DISAS_IAQ_N_STALE_EXIT) {
> -tcg_gen_lookup_and_goto_ptr(

Re: [PATCH v2 10/45] target/hppa: Skip nullified insns in unconditional dbranch path

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index a9196050dc..ca979f4137 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -1805,11 +1805,17 @@ static bool do_dbranch(DisasContext *ctx, int64_t 
> disp,
>  
>  if (ctx->null_cond.c == TCG_COND_NEVER && ctx->null_lab == NULL) {
>  install_link(ctx, link, false);
> -ctx->iaoq_n = dest;
> -ctx->iaoq_n_var = NULL;
>  if (is_n) {
> +if (use_nullify_skip(ctx)) {
> +nullify_set(ctx, 0);
> +gen_goto_tb(ctx, 0, dest, dest + 4);
> +ctx->base.is_jmp = DISAS_NORETURN;
> +return true;
> +}
>  ctx->null_cond.c = TCG_COND_ALWAYS;
>  }
> +ctx->iaoq_n = dest;
> +ctx->iaoq_n_var = NULL;
>  } else {
>  nullify_over(ctx);
>  
> -- 
> 2.34.1
> 



Re: [PATCH v2 09/45] target/hppa: Delay computation of IAQ_Next

2024-05-14 Thread Helge Deller
* Richard Henderson :
> We no longer have to allocate a temp and perform an
> addition before translation of the rest of the insn.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 26 ++
>  1 file changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index f816b337ee..a9196050dc 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -1806,6 +1806,7 @@ static bool do_dbranch(DisasContext *ctx, int64_t disp,
>  if (ctx->null_cond.c == TCG_COND_NEVER && ctx->null_lab == NULL) {
>  install_link(ctx, link, false);
>  ctx->iaoq_n = dest;
> +ctx->iaoq_n_var = NULL;
>  if (is_n) {
>  ctx->null_cond.c = TCG_COND_ALWAYS;
>  }
> @@ -1862,11 +1863,6 @@ static bool do_cbranch(DisasContext *ctx, int64_t 
> disp, bool is_n,
>  ctx->null_lab = NULL;
>  }
>  nullify_set(ctx, n);
> -if (ctx->iaoq_n == -1) {
> -/* The temporary iaoq_n_var died at the branch above.
> -   Regenerate it here instead of saving it.  */
> -tcg_gen_addi_i64(ctx->iaoq_n_var, cpu_iaoq_b, 4);
> -}
>  gen_goto_tb(ctx, 0, ctx->iaoq_b, ctx->iaoq_n);
>  }
>  
> @@ -4630,8 +4626,6 @@ static void hppa_tr_init_disas_context(DisasContextBase 
> *dcbase, CPUState *cs)
>  ctx->iaoq_f = (ctx->base.pc_first & ~iasq_f) + ctx->privilege;
>  ctx->iaoq_b = (diff ? ctx->iaoq_f + diff : -1);
>  #endif
> -ctx->iaoq_n = -1;
> -ctx->iaoq_n_var = NULL;
>  
>  ctx->zero = tcg_constant_i64(0);
>  
> @@ -4683,14 +4677,8 @@ static void hppa_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cs)
>  
>  /* Set up the IA queue for the next insn.
> This will be overwritten by a branch.  */
> -if (ctx->iaoq_b == -1) {
> -ctx->iaoq_n = -1;
> -ctx->iaoq_n_var = tcg_temp_new_i64();
> -tcg_gen_addi_i64(ctx->iaoq_n_var, cpu_iaoq_b, 4);
> -} else {
> -ctx->iaoq_n = ctx->iaoq_b + 4;
> -ctx->iaoq_n_var = NULL;
> -}
> +ctx->iaoq_n_var = NULL;
> +ctx->iaoq_n = ctx->iaoq_b == -1 ? -1 : ctx->iaoq_b + 4;
>  
>  if (unlikely(ctx->null_cond.c == TCG_COND_ALWAYS)) {
>  ctx->null_cond.c = TCG_COND_NEVER;
> @@ -4741,7 +4729,13 @@ static void hppa_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cs)
>  ? DISAS_EXIT
>  : DISAS_IAQ_N_UPDATED);
>  } else if (ctx->iaoq_b == -1) {
> -copy_iaoq_entry(ctx, cpu_iaoq_b, -1, ctx->iaoq_n_var);
> +if (ctx->iaoq_n_var) {
> +copy_iaoq_entry(ctx, cpu_iaoq_b, -1, ctx->iaoq_n_var);
> +} else {
> +tcg_gen_addi_i64(cpu_iaoq_b, cpu_iaoq_b, 4);
> +tcg_gen_andi_i64(cpu_iaoq_b, cpu_iaoq_b,
> + gva_offset_mask(ctx->tb_flags));
> +}
>  }
>  break;
>  
> -- 
> 2.34.1
> 



Re: [PATCH v2 08/45] target/hppa: Add install_link

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Add a common routine for writing the return address.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 54 +++--
>  1 file changed, 31 insertions(+), 23 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 08d5e2a4bc..f816b337ee 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -634,6 +634,23 @@ static void install_iaq_entries(DisasContext *ctx, 
> uint64_t bi, TCGv_i64 bv,
>  }
>  }
>  
> +static void install_link(DisasContext *ctx, unsigned link, bool with_sr0)
> +{
> +tcg_debug_assert(ctx->null_cond.c == TCG_COND_NEVER);
> +if (link) {

Just wondering:
Doesn't it makes it easier to write here:
if (!link) {
return;
}
and then don't indent the few following lines?

> +if (ctx->iaoq_b == -1) {
> +tcg_gen_addi_i64(cpu_gr[link], cpu_iaoq_b, 4);
> +} else {
> +tcg_gen_movi_i64(cpu_gr[link], ctx->iaoq_b + 4);
> +}
> +#ifndef CONFIG_USER_ONLY
> +if (with_sr0) {
> +tcg_gen_mov_i64(cpu_sr[0], cpu_iasq_b);
> +}
> +#endif
> +}
> +}
> +
>  static inline uint64_t iaoq_dest(DisasContext *ctx, int64_t disp)
>  {
>  return ctx->iaoq_f + disp + 8;
> @@ -1787,9 +1804,7 @@ static bool do_dbranch(DisasContext *ctx, int64_t disp,
>  uint64_t dest = iaoq_dest(ctx, disp);
>  
>  if (ctx->null_cond.c == TCG_COND_NEVER && ctx->null_lab == NULL) {
> -if (link != 0) {
> -copy_iaoq_entry(ctx, cpu_gr[link], ctx->iaoq_n, ctx->iaoq_n_var);
> -}
> +install_link(ctx, link, false);
>  ctx->iaoq_n = dest;
>  if (is_n) {
>  ctx->null_cond.c = TCG_COND_ALWAYS;
> @@ -1797,10 +1812,7 @@ static bool do_dbranch(DisasContext *ctx, int64_t disp,
>  } else {
>  nullify_over(ctx);
>  
> -if (link != 0) {
> -copy_iaoq_entry(ctx, cpu_gr[link], ctx->iaoq_n, ctx->iaoq_n_var);
> -}
> -
> +install_link(ctx, link, false);
>  if (is_n && use_nullify_skip(ctx)) {
>  nullify_set(ctx, 0);
>  gen_goto_tb(ctx, 0, dest, dest + 4);
> @@ -1892,9 +1904,7 @@ static bool do_ibranch(DisasContext *ctx, TCGv_i64 dest,
>  next = tcg_temp_new_i64();
>  tcg_gen_mov_i64(next, dest);
>  
> -if (link != 0) {
> -copy_iaoq_entry(ctx, cpu_gr[link], ctx->iaoq_n, ctx->iaoq_n_var);
> -}
> +install_link(ctx, link, false);
>  if (is_n) {
>  if (use_nullify_skip(ctx)) {
>  install_iaq_entries(ctx, -1, next, -1, NULL);
> @@ -1911,16 +1921,17 @@ static bool do_ibranch(DisasContext *ctx, TCGv_i64 
> dest,
>  
>  nullify_over(ctx);
>  
> +next = tcg_temp_new_i64();
> +tcg_gen_mov_i64(next, dest);
> +
> +install_link(ctx, link, false);
>  if (is_n && use_nullify_skip(ctx)) {
> -install_iaq_entries(ctx, -1, dest, -1, NULL);
> +install_iaq_entries(ctx, -1, next, -1, NULL);
>  nullify_set(ctx, 0);
>  } else {
> -install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b, -1, dest);
> +install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b, -1, next);
>  nullify_set(ctx, is_n);
>  }
> -if (link != 0) {
> -copy_iaoq_entry(ctx, cpu_gr[link], ctx->iaoq_n, ctx->iaoq_n_var);
> -}
>  
>  tcg_gen_lookup_and_goto_ptr();
>  ctx->base.is_jmp = DISAS_NORETURN;
> @@ -3899,10 +3910,7 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
>  nullify_over(ctx);
>  
>  load_spr(ctx, new_spc, a->sp);
> -if (a->l) {
> -copy_iaoq_entry(ctx, cpu_gr[31], ctx->iaoq_n, ctx->iaoq_n_var);
> -tcg_gen_mov_i64(cpu_sr[0], cpu_iasq_b);
> -}
> +install_link(ctx, a->l, true);
>  if (a->n && use_nullify_skip(ctx)) {
>  install_iaq_entries(ctx, -1, tmp, -1, NULL);
>  tcg_gen_mov_i64(cpu_iasq_f, new_spc);
> @@ -4019,16 +4027,16 @@ static bool trans_bve(DisasContext *ctx, arg_bve *a)
>  return do_ibranch(ctx, dest, a->l, a->n);
>  #else
>  nullify_over(ctx);
> -dest = do_ibranch_priv(ctx, load_gpr(ctx, a->b));
> +dest = tcg_temp_new_i64();
> +tcg_gen_mov_i64(dest, load_gpr(ctx, a->b));
> +dest = do_ibranch_priv(ctx, dest);
>  
> +install_link(ctx, a->l, false);
>  install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b, -1, dest);
>  if (ctx->iaoq_b == -1) {
>  tcg_gen_mov_i64(cpu_iasq_f, cpu_iasq_b);
>  }
>  tcg_gen_mov_i64(cpu_iasq_b, space_select(ctx, 0, dest));
> -if (a->l) {
> -copy_iaoq_entry(ctx, cpu_gr[a->l], ctx->iaoq_n, ctx->iaoq_n_var);
> -}
>  nullify_set(ctx, a->n);
>  tcg_gen_lookup_and_goto_ptr();
>  ctx->base.is_jmp = DISAS_NORETURN;
> -- 
> 2.34.1
> 



Re: [PATCH v2 07/45] target/hppa: Add install_iaq_entries

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Instead of two separate cpu_iaoq_entry calls, use one call to update
> both IAQ_Front and IAQ_Back.  Simplify with an argument combination
> that automatically handles a simple increment from Front to Back.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 64 +
>  1 file changed, 33 insertions(+), 31 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index d272be0e6e..08d5e2a4bc 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -617,6 +617,23 @@ static void copy_iaoq_entry(DisasContext *ctx, TCGv_i64 
> dest,
>  }
>  }
>  
> +static void install_iaq_entries(DisasContext *ctx, uint64_t bi, TCGv_i64 bv,
> +uint64_t ni, TCGv_i64 nv)
> +{
> +copy_iaoq_entry(ctx, cpu_iaoq_f, bi, bv);
> +
> +/* Allow ni variable, with nv null, to indicate a trivial advance. */
> +if (ni != -1 || nv) {
> +copy_iaoq_entry(ctx, cpu_iaoq_b, ni, nv);
> +} else if (bi != -1) {
> +copy_iaoq_entry(ctx, cpu_iaoq_b, bi + 4, NULL);
> +} else {
> +tcg_gen_addi_i64(cpu_iaoq_b, cpu_iaoq_f, 4);
> +tcg_gen_andi_i64(cpu_iaoq_b, cpu_iaoq_b,
> + gva_offset_mask(ctx->tb_flags));
> +}
> +}
> +
>  static inline uint64_t iaoq_dest(DisasContext *ctx, int64_t disp)
>  {
>  return ctx->iaoq_f + disp + 8;
> @@ -629,8 +646,7 @@ static void gen_excp_1(int exception)
>  
>  static void gen_excp(DisasContext *ctx, int exception)
>  {
> -copy_iaoq_entry(ctx, cpu_iaoq_f, ctx->iaoq_f, cpu_iaoq_f);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, ctx->iaoq_b, cpu_iaoq_b);
> +install_iaq_entries(ctx, ctx->iaoq_f, cpu_iaoq_f, ctx->iaoq_b, 
> cpu_iaoq_b);
>  nullify_save(ctx);
>  gen_excp_1(exception);
>  ctx->base.is_jmp = DISAS_NORETURN;
> @@ -684,12 +700,10 @@ static void gen_goto_tb(DisasContext *ctx, int which,
>  {
>  if (use_goto_tb(ctx, b, n)) {
>  tcg_gen_goto_tb(which);
> -copy_iaoq_entry(ctx, cpu_iaoq_f, b, NULL);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, n, NULL);
> +install_iaq_entries(ctx, b, NULL, n, NULL);
>  tcg_gen_exit_tb(ctx->base.tb, which);
>  } else {
> -copy_iaoq_entry(ctx, cpu_iaoq_f, b, cpu_iaoq_b);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, n, ctx->iaoq_n_var);
> +install_iaq_entries(ctx, b, cpu_iaoq_b, n, ctx->iaoq_n_var);
>  tcg_gen_lookup_and_goto_ptr();
>  }
>  }
> @@ -1883,9 +1897,7 @@ static bool do_ibranch(DisasContext *ctx, TCGv_i64 dest,
>  }
>  if (is_n) {
>  if (use_nullify_skip(ctx)) {
> -copy_iaoq_entry(ctx, cpu_iaoq_f, -1, next);
> -tcg_gen_addi_i64(next, next, 4);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, -1, next);
> +install_iaq_entries(ctx, -1, next, -1, NULL);
>  nullify_set(ctx, 0);
>  ctx->base.is_jmp = DISAS_IAQ_N_UPDATED;
>  return true;
> @@ -1900,14 +1912,10 @@ static bool do_ibranch(DisasContext *ctx, TCGv_i64 
> dest,
>  nullify_over(ctx);
>  
>  if (is_n && use_nullify_skip(ctx)) {
> -copy_iaoq_entry(ctx, cpu_iaoq_f, -1, dest);
> -next = tcg_temp_new_i64();
> -tcg_gen_addi_i64(next, dest, 4);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, -1, next);
> +install_iaq_entries(ctx, -1, dest, -1, NULL);
>  nullify_set(ctx, 0);
>  } else {
> -copy_iaoq_entry(ctx, cpu_iaoq_f, ctx->iaoq_b, cpu_iaoq_b);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, -1, dest);
> +install_iaq_entries(ctx, ctx->iaoq_b, cpu_iaoq_b, -1, dest);
>  nullify_set(ctx, is_n);
>  }
>  if (link != 0) {
> @@ -1998,9 +2006,7 @@ static void do_page_zero(DisasContext *ctx)
>  tcg_gen_st_i64(cpu_gr[26], tcg_env, offsetof(CPUHPPAState, cr[27]));
>  tmp = tcg_temp_new_i64();
>  tcg_gen_ori_i64(tmp, cpu_gr[31], 3);
> -copy_iaoq_entry(ctx, cpu_iaoq_f, -1, tmp);
> -tcg_gen_addi_i64(tmp, tmp, 4);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, -1, tmp);
> +install_iaq_entries(ctx, -1, tmp, -1, NULL);
>  ctx->base.is_jmp = DISAS_IAQ_N_UPDATED;
>  break;
>  
> @@ -2744,8 +2750,8 @@ static bool trans_or(DisasContext *ctx, arg_rrr_cf_d *a)
>  nullify_over(ctx);
>  
>  /* Advance the instruction queue.  */
> -copy_iaoq_entry(ctx, cpu_iaoq_f, ctx->iaoq_b, cpu_iaoq_b);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, c

Re: [PATCH v2 06/45] target/hppa: Use CF_BP_PAGE instead of cpu_breakpoint_test

2024-05-14 Thread Helge Deller
* Richard Henderson :
> The generic tcg driver will have already checked for breakpoints.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 140dfb747a..d272be0e6e 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -674,8 +674,9 @@ static bool use_goto_tb(DisasContext *ctx, uint64_t bofs, 
> uint64_t nofs)
> executing a TB that merely branches to the next TB.  */
>  static bool use_nullify_skip(DisasContext *ctx)
>  {
> -return (((ctx->iaoq_b ^ ctx->iaoq_f) & TARGET_PAGE_MASK) == 0
> -&& !cpu_breakpoint_test(ctx->cs, ctx->iaoq_b, BP_ANY));
> +return (!(tb_cflags(ctx->base.tb) & CF_BP_PAGE)
> +&& ctx->iaoq_b != -1
> +&& is_same_page(>base, ctx->iaoq_b));
>  }
>  
>  static void gen_goto_tb(DisasContext *ctx, int which,
> -- 
> 2.34.1
> 



Re: [PATCH v2 05/45] target/hppa: Allow prior nullification in do_ibranch

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Simplify the function by not attempting a conditional move
> on the branch destination -- just use nullify_over normally.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 73 +++--
>  1 file changed, 20 insertions(+), 53 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 4c42b518c5..140dfb747a 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -1871,17 +1871,15 @@ static bool do_cbranch(DisasContext *ctx, int64_t 
> disp, bool is_n,
>  static bool do_ibranch(DisasContext *ctx, TCGv_i64 dest,
> unsigned link, bool is_n)
>  {
> -TCGv_i64 a0, a1, next, tmp;
> -TCGCond c;
> +TCGv_i64 next;
>  
> -assert(ctx->null_lab == NULL);
> +if (ctx->null_cond.c == TCG_COND_NEVER && ctx->null_lab == NULL) {
> +next = tcg_temp_new_i64();
> +tcg_gen_mov_i64(next, dest);
>  
> -if (ctx->null_cond.c == TCG_COND_NEVER) {
>  if (link != 0) {
>  copy_iaoq_entry(ctx, cpu_gr[link], ctx->iaoq_n, ctx->iaoq_n_var);
>  }
> -next = tcg_temp_new_i64();
> -tcg_gen_mov_i64(next, dest);
>  if (is_n) {
>  if (use_nullify_skip(ctx)) {
>  copy_iaoq_entry(ctx, cpu_iaoq_f, -1, next);
> @@ -1895,60 +1893,29 @@ static bool do_ibranch(DisasContext *ctx, TCGv_i64 
> dest,
>  }
>  ctx->iaoq_n = -1;
>  ctx->iaoq_n_var = next;
> -} else if (is_n && use_nullify_skip(ctx)) {
> -/* The (conditional) branch, B, nullifies the next insn, N,
> -   and we're allowed to skip execution N (no single-step or
> -   tracepoint in effect).  Since the goto_ptr that we must use
> -   for the indirect branch consumes no special resources, we
> -   can (conditionally) skip B and continue execution.  */
> -/* The use_nullify_skip test implies we have a known control path.  
> */
> -tcg_debug_assert(ctx->iaoq_b != -1);
> -tcg_debug_assert(ctx->iaoq_n != -1);
> +return true;
> +}
>  
> -/* We do have to handle the non-local temporary, DEST, before
> -   branching.  Since IOAQ_F is not really live at this point, we
> -   can simply store DEST optimistically.  Similarly with IAOQ_B.  */
> +nullify_over(ctx);
> +
> +if (is_n && use_nullify_skip(ctx)) {
>  copy_iaoq_entry(ctx, cpu_iaoq_f, -1, dest);
>  next = tcg_temp_new_i64();
>  tcg_gen_addi_i64(next, dest, 4);
>  copy_iaoq_entry(ctx, cpu_iaoq_b, -1, next);
> -
> -nullify_over(ctx);
> -if (link != 0) {
> -copy_iaoq_entry(ctx, cpu_gr[link], ctx->iaoq_n, ctx->iaoq_n_var);
> -}
> -tcg_gen_lookup_and_goto_ptr();
> -return nullify_end(ctx);
> +nullify_set(ctx, 0);
>  } else {
> -c = ctx->null_cond.c;
> -a0 = ctx->null_cond.a0;
> -a1 = ctx->null_cond.a1;
> -
> -tmp = tcg_temp_new_i64();
> -next = tcg_temp_new_i64();
> -
> -copy_iaoq_entry(ctx, tmp, ctx->iaoq_n, ctx->iaoq_n_var);
> -tcg_gen_movcond_i64(c, next, a0, a1, tmp, dest);
> -ctx->iaoq_n = -1;
> -ctx->iaoq_n_var = next;
> -
> -if (link != 0) {
> -tcg_gen_movcond_i64(c, cpu_gr[link], a0, a1, cpu_gr[link], tmp);
> -}
> -
> -if (is_n) {
> -/* The branch nullifies the next insn, which means the state of N
> -   after the branch is the inverse of the state of N that applied
> -   to the branch.  */
> -tcg_gen_setcond_i64(tcg_invert_cond(c), cpu_psw_n, a0, a1);
> -cond_free(>null_cond);
> -ctx->null_cond = cond_make_n();
> -ctx->psw_n_nonzero = true;
> -} else {
> -cond_free(>null_cond);
> -}
> +copy_iaoq_entry(ctx, cpu_iaoq_f, ctx->iaoq_b, cpu_iaoq_b);
> +copy_iaoq_entry(ctx, cpu_iaoq_b, -1, dest);
> +nullify_set(ctx, is_n);
>  }
> -return true;
> +if (link != 0) {
> +copy_iaoq_entry(ctx, cpu_gr[link], ctx->iaoq_n, ctx->iaoq_n_var);
> +}
> +
> +tcg_gen_lookup_and_goto_ptr();
> +ctx->base.is_jmp = DISAS_NORETURN;
> +return nullify_end(ctx);
>  }
>  
>  /* Implement
> -- 
> 2.34.1
> 



Re: [PATCH v2 04/45] target/hppa: Pass displacement to do_dbranch

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Pass a displacement instead of an absolute value.
> 
> In trans_be, remove the user-only do_dbranch case.  The branch we are
> attempting to optimize is to the zero page, which is perforce on a
> different page than the code currently executing, which means that
> we will *not* use a goto_tb.  Use a plain indirect branch instead,
> which is what we got out of the attempted direct branch anyway.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 33 +
>  1 file changed, 9 insertions(+), 24 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 398803981c..4c42b518c5 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -1766,9 +1766,11 @@ static bool do_fop_dedd(DisasContext *ctx, unsigned rt,
>  
>  /* Emit an unconditional branch to a direct target, which may or may not
> have already had nullification handled.  */
> -static bool do_dbranch(DisasContext *ctx, uint64_t dest,
> +static bool do_dbranch(DisasContext *ctx, int64_t disp,
> unsigned link, bool is_n)
>  {
> +uint64_t dest = iaoq_dest(ctx, disp);
> +
>  if (ctx->null_cond.c == TCG_COND_NEVER && ctx->null_lab == NULL) {
>  if (link != 0) {
>  copy_iaoq_entry(ctx, cpu_gr[link], ctx->iaoq_n, ctx->iaoq_n_var);
> @@ -1815,10 +1817,7 @@ static bool do_cbranch(DisasContext *ctx, int64_t 
> disp, bool is_n,
>  
>  /* Handle TRUE and NEVER as direct branches.  */
>  if (c == TCG_COND_ALWAYS) {
> -return do_dbranch(ctx, dest, 0, is_n && disp >= 0);
> -}
> -if (c == TCG_COND_NEVER) {
> -return do_dbranch(ctx, ctx->iaoq_n, 0, is_n && disp < 0);
> +return do_dbranch(ctx, disp, 0, is_n && disp >= 0);
>  }
>  
>  taken = gen_new_label();
> @@ -3914,22 +3913,6 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
>  {
>  TCGv_i64 tmp;
>  
> -#ifdef CONFIG_USER_ONLY
> -/* ??? It seems like there should be a good way of using
> -   "be disp(sr2, r0)", the canonical gateway entry mechanism
> -   to our advantage.  But that appears to be inconvenient to
> -   manage along side branch delay slots.  Therefore we handle
> -   entry into the gateway page via absolute address.  */
> -/* Since we don't implement spaces, just branch.  Do notice the special
> -   case of "be disp(*,r0)" using a direct branch to disp, so that we can
> -   goto_tb to the TB containing the syscall.  */
> -if (a->b == 0) {
> -return do_dbranch(ctx, a->disp, a->l, a->n);
> -}
> -#else
> -nullify_over(ctx);
> -#endif
> -
>  tmp = tcg_temp_new_i64();
>  tcg_gen_addi_i64(tmp, load_gpr(ctx, a->b), a->disp);
>  tmp = do_ibranch_priv(ctx, tmp);
> @@ -3939,6 +3922,8 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
>  #else
>  TCGv_i64 new_spc = tcg_temp_new_i64();
>  
> +nullify_over(ctx);
> +
>  load_spr(ctx, new_spc, a->sp);
>  if (a->l) {
>  copy_iaoq_entry(ctx, cpu_gr[31], ctx->iaoq_n, ctx->iaoq_n_var);
> @@ -3968,7 +3953,7 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
>  
>  static bool trans_bl(DisasContext *ctx, arg_bl *a)
>  {
> -return do_dbranch(ctx, iaoq_dest(ctx, a->disp), a->l, a->n);
> +return do_dbranch(ctx, a->disp, a->l, a->n);
>  }
>  
>  static bool trans_b_gate(DisasContext *ctx, arg_b_gate *a)
> @@ -4022,7 +4007,7 @@ static bool trans_b_gate(DisasContext *ctx, arg_b_gate 
> *a)
>  save_gpr(ctx, a->l, tmp);
>  }
>  
> -return do_dbranch(ctx, dest, 0, a->n);
> +return do_dbranch(ctx, dest - iaoq_dest(ctx, 0), 0, a->n);
>  }
>  
>  static bool trans_blr(DisasContext *ctx, arg_blr *a)
> @@ -4035,7 +4020,7 @@ static bool trans_blr(DisasContext *ctx, arg_blr *a)
>  return do_ibranch(ctx, tmp, a->l, a->n);
>  } else {
>  /* BLR R0,RX is a good way to load PC+8 into RX.  */
> -return do_dbranch(ctx, ctx->iaoq_f + 8, a->l, a->n);
> +return do_dbranch(ctx, 0, a->l, a->n);
>  }
>  }
>  
> -- 
> 2.34.1
> 



Re: [PATCH v2 03/45] target/hppa: Move constant destination check into use_goto_tb

2024-05-14 Thread Helge Deller
* Richard Henderson :
> Share this check between gen_goto_tb and hppa_tr_translate_insn.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Helge Deller 

> ---
>  target/hppa/translate.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index 6d45611888..398803981c 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -662,9 +662,10 @@ static bool gen_illegal(DisasContext *ctx)
>  } while (0)
>  #endif
>  
> -static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
> +static bool use_goto_tb(DisasContext *ctx, uint64_t bofs, uint64_t nofs)
>  {
> -return translator_use_goto_tb(>base, dest);
> +return (bofs != -1 && nofs != -1 &&
> +translator_use_goto_tb(>base, bofs));
>  }
>  
>  /* If the next insn is to be nullified, and it's on the same page,
> @@ -678,16 +679,16 @@ static bool use_nullify_skip(DisasContext *ctx)
>  }
>  
>  static void gen_goto_tb(DisasContext *ctx, int which,
> -uint64_t f, uint64_t b)
> +uint64_t b, uint64_t n)
>  {
> -if (f != -1 && b != -1 && use_goto_tb(ctx, f)) {
> +if (use_goto_tb(ctx, b, n)) {
>  tcg_gen_goto_tb(which);
> -copy_iaoq_entry(ctx, cpu_iaoq_f, f, NULL);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, b, NULL);
> +copy_iaoq_entry(ctx, cpu_iaoq_f, b, NULL);
> +copy_iaoq_entry(ctx, cpu_iaoq_b, n, NULL);
>  tcg_gen_exit_tb(ctx->base.tb, which);
>  } else {
> -copy_iaoq_entry(ctx, cpu_iaoq_f, f, cpu_iaoq_b);
> -copy_iaoq_entry(ctx, cpu_iaoq_b, b, ctx->iaoq_n_var);
> +copy_iaoq_entry(ctx, cpu_iaoq_f, b, cpu_iaoq_b);
> +copy_iaoq_entry(ctx, cpu_iaoq_b, n, ctx->iaoq_n_var);
>  tcg_gen_lookup_and_goto_ptr();
>  }
>  }
> @@ -4744,8 +4745,7 @@ static void hppa_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cs)
>  /* Advance the insn queue.  Note that this check also detects
> a priority change within the instruction queue.  */
>  if (ret == DISAS_NEXT && ctx->iaoq_b != ctx->iaoq_f + 4) {
> -if (ctx->iaoq_b != -1 && ctx->iaoq_n != -1
> -&& use_goto_tb(ctx, ctx->iaoq_b)
> +if (use_goto_tb(ctx, ctx->iaoq_b, ctx->iaoq_n)
>  && (ctx->null_cond.c == TCG_COND_NEVER
>  || ctx->null_cond.c == TCG_COND_ALWAYS)) {
>  nullify_set(ctx, ctx->null_cond.c == TCG_COND_ALWAYS);
> -- 
> 2.34.1
> 



Re: [PATCH v2 02/45] target/hppa: Use hppa_form_gva_psw in hppa_cpu_get_pc

2024-05-14 Thread Helge Deller
* Richard Henderson :
> This function is for log_pc(), which needs to produce a
> similar result to cpu_get_tb_cpu_state().
> 
> Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 

---
> ---
>  target/hppa/cpu.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
> index 582036b31e..be8c558014 100644
> --- a/target/hppa/cpu.c
> +++ b/target/hppa/cpu.c
> @@ -38,9 +38,10 @@ static void hppa_cpu_set_pc(CPUState *cs, vaddr value)
>  
>  static vaddr hppa_cpu_get_pc(CPUState *cs)
>  {
> -HPPACPU *cpu = HPPA_CPU(cs);
> +CPUHPPAState *env = cpu_env(cs);
>  
> -return cpu->env.iaoq_f;
> +return hppa_form_gva_psw(env->psw, (env->psw & PSW_C ? env->iasq_f : 0),
> + env->iaoq_f & -4);
>  }
>  
>  void cpu_get_tb_cpu_state(CPUHPPAState *env, vaddr *pc,
> @@ -61,8 +62,7 @@ void cpu_get_tb_cpu_state(CPUHPPAState *env, vaddr *pc,
>  flags |= env->psw & (PSW_W | PSW_C | PSW_D | PSW_P);
>  flags |= (env->iaoq_f & 3) << TB_FLAG_PRIV_SHIFT;
>  
> -*pc = hppa_form_gva_psw(env->psw, (env->psw & PSW_C ? env->iasq_f : 0),
> -env->iaoq_f & -4);
> +*pc = hppa_cpu_get_pc(env_cpu(env));
>  *cs_base = env->iasq_f;
>  
>  /* Insert a difference between IAOQ_B and IAOQ_F within the otherwise 
> zero
> -- 
> 2.34.1
> 



Re: [PATCH v2 00/45] target/hppa: Misc improvements

2024-05-14 Thread Helge Deller

On 5/13/24 09:46, Richard Henderson wrote:

Most of the patches lead up to implementing CF_PCREL.
Along the way there is a grab bag of code updates (TCG_COND_TST*),
bug fixes (space changes during branch-in-branch-delay-slot),
and implementation of features (PSW bits B, X, T, H, L).

Sven reported that PSW L tripped up HP/UX, so possibly there's
something wrong there, but that's right at the end of the patch set.
So I'd like some feedback on the rest leading up to that too.


I do see the PSW_L issue as well (32-bit machine when starting HP-UX 11).
When leaving out patches 42-45 ("target/hppa: Implement PSW_T" and following)
I can boot 32-bit HP-UX 10 and 11.

Will review the patches now individually, but maybe you should
push the patches 1-41 first and we take the PSWT/L patches later?

Helge



Changes for v2:
   - Rebase and update for tcg_cflags_set.


r~


Richard Henderson (45):
   target/hppa: Move cpu_get_tb_cpu_state out of line
   target/hppa: Use hppa_form_gva_psw in hppa_cpu_get_pc
   target/hppa: Move constant destination check into use_goto_tb
   target/hppa: Pass displacement to do_dbranch
   target/hppa: Allow prior nullification in do_ibranch
   target/hppa: Use CF_BP_PAGE instead of cpu_breakpoint_test
   target/hppa: Add install_iaq_entries
   target/hppa: Add install_link
   target/hppa: Delay computation of IAQ_Next
   target/hppa: Skip nullified insns in unconditional dbranch path
   target/hppa: Simplify TB end
   target/hppa: Add IASQ entries to DisasContext
   target/hppa: Add space arguments to install_iaq_entries
   target/hppa: Add space argument to do_ibranch
   target/hppa: Use umax in do_ibranch_priv
   target/hppa: Always make a copy in do_ibranch_priv
   target/hppa: Introduce and use DisasIAQE for branch management
   target/hppa: Use displacements in DisasIAQE
   target/hppa: Rename cond_make_* helpers
   target/hppa: Use TCG_COND_TST* in do_cond
   target/hppa: Use TCG_COND_TST* in do_log_cond
   target/hppa: Use TCG_COND_TST* in do_unit_zero_cond
   target/hppa: Use TCG_COND_TST* in do_unit_addsub
   target/hppa: Use TCG_COND_TST* in trans_bb_imm
   target/hppa: Use registerfields.h for FPSR
   target/hppa: Use TCG_COND_TST* in trans_ftest
   target/hppa: Remove cond_free
   target/hppa: Introduce DisasDelayException
   target/hppa: Use delay_excp for conditional traps
   target/hppa: Use delay_excp for conditional trap on overflow
   linux-user/hppa: Force all code addresses to PRIV_USER
   target/hppa: Store full iaoq_f and page offset of iaoq_b in TB
   target/hppa: Do not mask in copy_iaoq_entry
   target/hppa: Improve hppa_cpu_dump_state
   target/hppa: Split PSW X and B into their own field
   target/hppa: Manage PSW_X and PSW_B in translator
   target/hppa: Implement PSW_B
   target/hppa: Implement PSW_X
   target/hppa: Drop tlb_entry return from hppa_get_physical_address
   target/hppa: Adjust priv for B,GATE at runtime
   target/hppa: Implement CF_PCREL
   target/hppa: Implement PSW_T
   target/hppa: Implement PSW_H, PSW_L
   target/hppa: Log cpu state at interrupt
   target/hppa: Log cpu state on return-from-interrupt

  linux-user/hppa/target_cpu.h |4 +-
  target/hppa/cpu.h|   80 +--
  target/hppa/helper.h |3 +-
  linux-user/elfload.c |4 +-
  linux-user/hppa/cpu_loop.c   |   14 +-
  linux-user/hppa/signal.c |6 +-
  target/hppa/cpu.c|   92 ++-
  target/hppa/fpu_helper.c |   26 +-
  target/hppa/gdbstub.c|6 +
  target/hppa/helper.c |   66 +-
  target/hppa/int_helper.c |   33 +-
  target/hppa/mem_helper.c |   99 +--
  target/hppa/op_helper.c  |   17 +-
  target/hppa/sys_helper.c |   12 +
  target/hppa/translate.c  | 1232 ++
  15 files changed, 947 insertions(+), 747 deletions(-)






Re: [PATCH v2 01/45] target/hppa: Move cpu_get_tb_cpu_state out of line

2024-05-14 Thread Helge Deller

On 5/13/24 09:46, Richard Henderson wrote:

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 


---
  target/hppa/cpu.h | 43 ++-
  target/hppa/cpu.c | 42 ++
  2 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index fb2e4c4a98..61f1353133 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -314,47 +314,8 @@ hwaddr hppa_abs_to_phys_pa2_w1(vaddr addr);
  #define TB_FLAG_PRIV_SHIFT  8
  #define TB_FLAG_UNALIGN 0x400

-static inline void cpu_get_tb_cpu_state(CPUHPPAState *env, vaddr *pc,
-uint64_t *cs_base, uint32_t *pflags)
-{
-uint32_t flags = env->psw_n * PSW_N;
-
-/* TB lookup assumes that PC contains the complete virtual address.
-   If we leave space+offset separate, we'll get ITLB misses to an
-   incomplete virtual address.  This also means that we must separate
-   out current cpu privilege from the low bits of IAOQ_F.  */
-#ifdef CONFIG_USER_ONLY
-*pc = env->iaoq_f & -4;
-*cs_base = env->iaoq_b & -4;
-flags |= TB_FLAG_UNALIGN * !env_cpu(env)->prctl_unalign_sigbus;
-#else
-/* ??? E, T, H, L, B bits need to be here, when implemented.  */
-flags |= env->psw & (PSW_W | PSW_C | PSW_D | PSW_P);
-flags |= (env->iaoq_f & 3) << TB_FLAG_PRIV_SHIFT;
-
-*pc = hppa_form_gva_psw(env->psw, (env->psw & PSW_C ? env->iasq_f : 0),
-env->iaoq_f & -4);
-*cs_base = env->iasq_f;
-
-/* Insert a difference between IAOQ_B and IAOQ_F within the otherwise zero
-   low 32-bits of CS_BASE.  This will succeed for all direct branches,
-   which is the primary case we care about -- using goto_tb within a page.
-   Failure is indicated by a zero difference.  */
-if (env->iasq_f == env->iasq_b) {
-target_long diff = env->iaoq_b - env->iaoq_f;
-if (diff == (int32_t)diff) {
-*cs_base |= (uint32_t)diff;
-}
-}
-if ((env->sr[4] == env->sr[5])
-& (env->sr[4] == env->sr[6])
-& (env->sr[4] == env->sr[7])) {
-flags |= TB_FLAG_SR_SAME;
-}
-#endif
-
-*pflags = flags;
-}
+void cpu_get_tb_cpu_state(CPUHPPAState *env, vaddr *pc,
+  uint64_t *cs_base, uint32_t *pflags);

  target_ulong cpu_hppa_get_psw(CPUHPPAState *env);
  void cpu_hppa_put_psw(CPUHPPAState *env, target_ulong);
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 393a81988d..582036b31e 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -43,6 +43,48 @@ static vaddr hppa_cpu_get_pc(CPUState *cs)
  return cpu->env.iaoq_f;
  }

+void cpu_get_tb_cpu_state(CPUHPPAState *env, vaddr *pc,
+  uint64_t *cs_base, uint32_t *pflags)
+{
+uint32_t flags = env->psw_n * PSW_N;
+
+/* TB lookup assumes that PC contains the complete virtual address.
+   If we leave space+offset separate, we'll get ITLB misses to an
+   incomplete virtual address.  This also means that we must separate
+   out current cpu privilege from the low bits of IAOQ_F.  */
+#ifdef CONFIG_USER_ONLY
+*pc = env->iaoq_f & -4;
+*cs_base = env->iaoq_b & -4;
+flags |= TB_FLAG_UNALIGN * !env_cpu(env)->prctl_unalign_sigbus;
+#else
+/* ??? E, T, H, L, B bits need to be here, when implemented.  */
+flags |= env->psw & (PSW_W | PSW_C | PSW_D | PSW_P);
+flags |= (env->iaoq_f & 3) << TB_FLAG_PRIV_SHIFT;
+
+*pc = hppa_form_gva_psw(env->psw, (env->psw & PSW_C ? env->iasq_f : 0),
+env->iaoq_f & -4);
+*cs_base = env->iasq_f;
+
+/* Insert a difference between IAOQ_B and IAOQ_F within the otherwise zero
+   low 32-bits of CS_BASE.  This will succeed for all direct branches,
+   which is the primary case we care about -- using goto_tb within a page.
+   Failure is indicated by a zero difference.  */
+if (env->iasq_f == env->iasq_b) {
+target_long diff = env->iaoq_b - env->iaoq_f;
+if (diff == (int32_t)diff) {
+*cs_base |= (uint32_t)diff;
+}
+}
+if ((env->sr[4] == env->sr[5])
+& (env->sr[4] == env->sr[6])
+& (env->sr[4] == env->sr[7])) {
+flags |= TB_FLAG_SR_SAME;
+}
+#endif
+
+*pflags = flags;
+}
+
  static void hppa_cpu_synchronize_from_tb(CPUState *cs,
   const TranslationBlock *tb)
  {





Re: [PATCH 00/45] target/hppa: Misc improvements

2024-05-13 Thread Helge Deller

On 5/12/24 18:08, Sven Schnelle wrote:

Philippe Mathieu-Daudé  writes:


Cc'ing Helge & Sven as I'm going to skip this series.

Suggestion:

-- >8 --
diff --git a/MAINTAINERS b/MAINTAINERS
index 1b79767d61..be7535b55e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -254,6 +254,8 @@ F: target/hexagon/gen_idef_parser_funcs.py

  HPPA (PA-RISC) TCG CPUs
  M: Richard Henderson 
+R: Helge Deller 
+R: Sven Schnelle 
  S: Maintained
  F: target/hppa/
  F: disas/hppa.c
@@ -1214,6 +1216,7 @@ HP-PARISC Machines
  HP B160L, HP C3700
  M: Richard Henderson 
  R: Helge Deller 
+R: Sven Schnelle 
  S: Odd Fixes
  F: configs/devices/hppa-softmmu/default.mak
  F: hw/display/artist.c


Please don't add me as reviewer - i'm only looking in irregular
intervals at hppa tcg in qemu.


I'm open to be a reviewer for hppa.
I already briefly tested and looked into this patch series and
hope to finish in the next few days. Sadly it doesn't apply
on top of git head any longer...

Helge




Re: hppa-firmware.img missing build-id

2024-04-23 Thread Helge Deller

On 4/23/24 17:10, Daniel P. Berrangé wrote:

On Tue, Apr 23, 2024 at 05:07:17PM +0200, Helge Deller wrote:

On 4/23/24 16:58, Cole Robinson wrote:

On 4/23/24 10:11 AM, Cole Robinson wrote:

Hi,

hppa-firmware.img and hppa-firmware64.img in qemu.git are missing ELF
build-id annotations. rpm builds on Fedora will error if an ELF binary
doesn't have build-id:

RPM build errors:
  Missing build-id in
/tmp/rpmbuild/BUILDROOT/qemu-9.0.0-1.rc2.fc41.x86_64/usr/share/qemu/hppa-firmware.img
  Missing build-id in
/tmp/rpmbuild/BUILDROOT/qemu-9.0.0-1.rc2.fc41.x86_64/usr/share/qemu/hppa-firmware64.img
  Generating build-id links failed

I didn't hit this with qemu 8.2.* builds FWIW



Though checking older bundled hppa-firmware binaries with `readelf` I
don't see build-id either, so now I'm not sure why those RPM builds were
passing.

FWIW the RPM check is deep in RPM code:
https://github.com/rpm-software-management/rpm/blob/68d0f3119c3d46b6184f4704edb51749ce9f819e/build/files.c#L1976

Maybe something else in hppa-firmware ELF headers caused this check to
be skipped in the past


Maybe Fedora ignores binaries which don't have the executable flag set?


Yes, that's probably it. qemu 9.0.0 has +x set on the hppa-firmware
images, while qemu 8.2.0 does not have +x set.


I just added a patch to the seabios-hppa Makefile
to drop the +x flag with upcoming hppa-firmware builds.

Helge



Re: hppa-firmware.img missing build-id

2024-04-23 Thread Helge Deller

On 4/23/24 16:58, Cole Robinson wrote:

On 4/23/24 10:11 AM, Cole Robinson wrote:

Hi,

hppa-firmware.img and hppa-firmware64.img in qemu.git are missing ELF
build-id annotations. rpm builds on Fedora will error if an ELF binary
doesn't have build-id:

RPM build errors:
 Missing build-id in
/tmp/rpmbuild/BUILDROOT/qemu-9.0.0-1.rc2.fc41.x86_64/usr/share/qemu/hppa-firmware.img
 Missing build-id in
/tmp/rpmbuild/BUILDROOT/qemu-9.0.0-1.rc2.fc41.x86_64/usr/share/qemu/hppa-firmware64.img
 Generating build-id links failed

I didn't hit this with qemu 8.2.* builds FWIW



Though checking older bundled hppa-firmware binaries with `readelf` I
don't see build-id either, so now I'm not sure why those RPM builds were
passing.

FWIW the RPM check is deep in RPM code:
https://github.com/rpm-software-management/rpm/blob/68d0f3119c3d46b6184f4704edb51749ce9f819e/build/files.c#L1976

Maybe something else in hppa-firmware ELF headers caused this check to
be skipped in the past


Maybe Fedora ignores binaries which don't have the executable flag set?
Qemu does not need it.
If so, a "chmod -x pc-bios/hppa-firmware*.img" should be sufficient.


Otherwise, adding "--build-id" to the link line when building the SeaBIOS
hppa-firmware.img does adds a build-id:

diff --git a/Makefile.parisc b/Makefile.parisc
index 5c34eb3d..256142f4 100644
--- a/Makefile.parisc
+++ b/Makefile.parisc
@@ -169,7 +169,7 @@ $(OUT)hppa-firmware$(BIT_SUFFIX).img: $(OUT)autoconf.h 
$(OUT)head.o $(OUT)ccode3
@echo "  Linking $@"
$(Q)$(CPP) $(CPPFLAGS) -Isrc -D__ASSEMBLY__ -DBITS=$(BITS) 
src/parisc/pafirmware.lds.S -o $(OUT)pafirmware.lds
$(Q)$(CC) $(CFLAGS32FLAT) -c src/version.c -o $(OUT)version.o
-   $(Q)$(LD) -N -T $(OUT)pafirmware.lds $(OUT)head.o $(OUT)version.o -X -o 
$@ -e startup --as-needed $(OUT)ccode32flat.o $(LIBGCC)
+   $(Q)$(LD) -N -T $(OUT)pafirmware.lds $(OUT)head.o $(OUT)version.o -X -o 
$@ -e startup --as-needed --build-id $(OUT)ccode32flat.o $(LIBGCC)


deller@carbonx1:/home/cvs/LINUX/seabios$ eu-readelf -n out/hppa-firmware.img

Note section [ 1] '.note.gnu.build-id' of 36 bytes at offset 0x100:
  Owner  Data size  Type
  GNU   20  GNU_BUILD_ID
Build ID: 61a59ebba32fd40eadda7083983a1e1b04ec4082

Are you using the firmware blobs which come with qemu, or do you build 
Seabios-hppa yourself?

Helge



Re: [PATCH] target/hppa: Fix IIAOQ, IIASQ for pa2.0

2024-04-03 Thread Helge Deller

On 4/2/24 03:25, Richard Henderson wrote:

The contents of IIAOQ depend on PSW_W.
Follow the text in "Interruption Instruction Address Queues",
pages 2-13 through 2-15.

Reported-by: Sven Schnelle 
Fixes: b10700d826c ("target/hppa: Update IIAOQ, IIASQ for pa2.0")
Signed-off-by: Richard Henderson 


Tested-by: Helge Deller 

Helge


---

Sven, I looked again through IIAOQ documentation and it does seem
like some of the bits are wrong, both on interrupt delivery and RFI.


r~

---
  target/hppa/int_helper.c | 20 +++-
  target/hppa/sys_helper.c | 18 +-
  2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
index 90437a92cd..a667ee380d 100644
--- a/target/hppa/int_helper.c
+++ b/target/hppa/int_helper.c
@@ -107,14 +107,10 @@ void hppa_cpu_do_interrupt(CPUState *cs)

  /* step 3 */
  /*
- * For pa1.x, IIASQ is simply a copy of IASQ.
- * For pa2.0, IIASQ is the top bits of the virtual address,
- *or zero if translation is disabled.
+ * IIASQ is the top bits of the virtual address, or zero if translation
+ * is disabled -- with PSW_W == 0, this will reduce to the space.
   */
-if (!hppa_is_pa20(env)) {
-env->cr[CR_IIASQ] = env->iasq_f >> 32;
-env->cr_back[0] = env->iasq_b >> 32;
-} else if (old_psw & PSW_C) {
+if (old_psw & PSW_C) {
  env->cr[CR_IIASQ] =
  hppa_form_gva_psw(old_psw, env->iasq_f, env->iaoq_f) >> 32;
  env->cr_back[0] =
@@ -123,8 +119,14 @@ void hppa_cpu_do_interrupt(CPUState *cs)
  env->cr[CR_IIASQ] = 0;
  env->cr_back[0] = 0;
  }
-env->cr[CR_IIAOQ] = env->iaoq_f;
-env->cr_back[1] = env->iaoq_b;
+/* IIAOQ is the full offset for wide mode, or 32 bits for narrow mode. */
+if (old_psw & PSW_W) {
+env->cr[CR_IIAOQ] = env->iaoq_f;
+env->cr_back[1] = env->iaoq_b;
+} else {
+env->cr[CR_IIAOQ] = (uint32_t)env->iaoq_f;
+env->cr_back[1] = (uint32_t)env->iaoq_b;
+}

  if (old_psw & PSW_Q) {
  /* step 5 */
diff --git a/target/hppa/sys_helper.c b/target/hppa/sys_helper.c
index 208e51c086..22d6c89964 100644
--- a/target/hppa/sys_helper.c
+++ b/target/hppa/sys_helper.c
@@ -78,21 +78,21 @@ target_ulong HELPER(swap_system_mask)(CPUHPPAState *env, 
target_ulong nsm)

  void HELPER(rfi)(CPUHPPAState *env)
  {
-env->iasq_f = (uint64_t)env->cr[CR_IIASQ] << 32;
-env->iasq_b = (uint64_t)env->cr_back[0] << 32;
-env->iaoq_f = env->cr[CR_IIAOQ];
-env->iaoq_b = env->cr_back[1];
+uint64_t mask;
+
+cpu_hppa_put_psw(env, env->cr[CR_IPSW]);

  /*
   * For pa2.0, IIASQ is the top bits of the virtual address.
   * To recreate the space identifier, remove the offset bits.
+ * For pa1.x, the mask reduces to no change to space.
   */
-if (hppa_is_pa20(env)) {
-env->iasq_f &= ~env->iaoq_f;
-env->iasq_b &= ~env->iaoq_b;
-}
+mask = gva_offset_mask(env->psw);

-cpu_hppa_put_psw(env, env->cr[CR_IPSW]);
+env->iaoq_f = env->cr[CR_IIAOQ];
+env->iaoq_b = env->cr_back[1];
+env->iasq_f = (env->cr[CR_IIASQ] << 32) & ~(env->iaoq_f & mask);
+env->iasq_b = (env->cr_back[0] << 32) & ~(env->iaoq_b & mask);
  }

  static void getshadowregs(CPUHPPAState *env)





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

2024-04-02 Thread Helge Deller

On 4/2/24 08:29, Sven Schnelle wrote:

Richard Henderson  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.iaa11...@lucy.cup.hp.com/






Re: [PATCH v2 3/3] target/hppa: Fix diag instructions to set/restore shadow registers

2024-03-26 Thread Helge Deller

On 3/26/24 19:10, Richard Henderson wrote:

The 32-bit 7300LC CPU and the 64-bit PCX-W 8500 CPU use different


I initially should have used the correct naming in my patch.
So this sentence above should read:
The 32-bit PA-7300LC (PCX-L2) CPU and the 64-bit PA8700 (PCX-W2) CPU use 
different



diag instructions to save or restore the CPU registers to/from
the shadow registers.

Implement those per-CPU architecture diag instructions to fix those
parts of the HP ODE testcases (L2DIAG and WDIAG, section 1) which test
the shadow registers.

Signed-off-by: Helge Deller 
[rth: Use decodetree to distinguish cases]
Signed-off-by: Richard Henderson 
---
  target/hppa/insns.decode | 10 ++
  target/hppa/translate.c  | 34 ++
  2 files changed, 44 insertions(+)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 9f6ffd8e2c..c2acb3796c 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -65,6 +65,8 @@
  # Argument set definitions
  

+
+
  # All insns that need to form a virtual address should use this set.
 t b x disp sp m scale size

@@ -638,6 +640,14 @@ xmpyu   001110 . . 010 .0111 .00 t:5
r1=%ra64 r2=%rb64
[
  diag_btlb   000101 00    0001  
  diag_cout   000101 00    0001  0001
+
+# For 32-bit 7300C


For 32-bit PA-7300LC (PCX-L2) CPU:



+diag_getshadowregs_pa1  000101 00   0001 1010  
+diag_putshadowregs_pa1  000101 00   0001 1010 0100 
+
+# For 64-bit PCX-W 8500


For 64-bit PA8700 (PCX-W2) CPU:


other than that:
Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Thanks a lot for the nice rewrite & cleanup!!
Helge



+diag_getshadowregs_pa2  000101 00 0111 1000 0001 1000 0100 
+diag_putshadowregs_pa2  000101 00 0111  0001 1000 0100 
]
diag_unimp000101 i:26
  }
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 42dd3f2c8d..143818c2d9 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2399,6 +2399,20 @@ static bool do_getshadowregs(DisasContext *ctx)
  return nullify_end(ctx);
  }

+static bool do_putshadowregs(DisasContext *ctx)
+{
+CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
+nullify_over(ctx);
+tcg_gen_st_i64(cpu_gr[1], tcg_env, offsetof(CPUHPPAState, shadow[0]));
+tcg_gen_st_i64(cpu_gr[8], tcg_env, offsetof(CPUHPPAState, shadow[1]));
+tcg_gen_st_i64(cpu_gr[9], tcg_env, offsetof(CPUHPPAState, shadow[2]));
+tcg_gen_st_i64(cpu_gr[16], tcg_env, offsetof(CPUHPPAState, shadow[3]));
+tcg_gen_st_i64(cpu_gr[17], tcg_env, offsetof(CPUHPPAState, shadow[4]));
+tcg_gen_st_i64(cpu_gr[24], tcg_env, offsetof(CPUHPPAState, shadow[5]));
+tcg_gen_st_i64(cpu_gr[25], tcg_env, offsetof(CPUHPPAState, shadow[6]));
+return nullify_end(ctx);
+}
+
  static bool trans_getshadowregs(DisasContext *ctx, arg_getshadowregs *a)
  {
  return do_getshadowregs(ctx);
@@ -4594,6 +4608,26 @@ static bool trans_diag_cout(DisasContext *ctx, 
arg_diag_cout *a)
  #endif
  }

+static bool trans_diag_getshadowregs_pa1(DisasContext *ctx, arg_empty *a)
+{
+return !ctx->is_pa20 && do_getshadowregs(ctx);
+}
+
+static bool trans_diag_getshadowregs_pa2(DisasContext *ctx, arg_empty *a)
+{
+return ctx->is_pa20 && do_getshadowregs(ctx);
+}
+
+static bool trans_diag_putshadowregs_pa1(DisasContext *ctx, arg_empty *a)
+{
+return !ctx->is_pa20 && do_putshadowregs(ctx);
+}
+
+static bool trans_diag_putshadowregs_pa2(DisasContext *ctx, arg_empty *a)
+{
+return ctx->is_pa20 && do_putshadowregs(ctx);
+}
+
  static bool trans_diag_unimp(DisasContext *ctx, arg_diag_unimp *a)
  {
  CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);





Re: [PATCH v2 1/3] target/hppa: Generate getshadowregs inline

2024-03-26 Thread Helge Deller

On 3/26/24 19:10, Richard Henderson wrote:

This operation is trivial and does not require a helper.

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 

Helge


---
  target/hppa/helper.h |  1 -
  target/hppa/sys_helper.c |  4 ++--
  target/hppa/translate.c  | 17 +
  3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/target/hppa/helper.h b/target/hppa/helper.h
index 8fd7ba65d8..5900fd70bc 100644
--- a/target/hppa/helper.h
+++ b/target/hppa/helper.h
@@ -86,7 +86,6 @@ DEF_HELPER_FLAGS_0(read_interval_timer, TCG_CALL_NO_RWG, tl)
  #ifndef CONFIG_USER_ONLY
  DEF_HELPER_1(halt, noreturn, env)
  DEF_HELPER_1(reset, noreturn, env)
-DEF_HELPER_1(getshadowregs, void, env)
  DEF_HELPER_1(rfi, void, env)
  DEF_HELPER_1(rfi_r, void, env)
  DEF_HELPER_FLAGS_2(write_interval_timer, TCG_CALL_NO_RWG, void, env, tl)
diff --git a/target/hppa/sys_helper.c b/target/hppa/sys_helper.c
index 4a31748342..208e51c086 100644
--- a/target/hppa/sys_helper.c
+++ b/target/hppa/sys_helper.c
@@ -95,7 +95,7 @@ void HELPER(rfi)(CPUHPPAState *env)
  cpu_hppa_put_psw(env, env->cr[CR_IPSW]);
  }

-void HELPER(getshadowregs)(CPUHPPAState *env)
+static void getshadowregs(CPUHPPAState *env)
  {
  env->gr[1] = env->shadow[0];
  env->gr[8] = env->shadow[1];
@@ -108,7 +108,7 @@ void HELPER(getshadowregs)(CPUHPPAState *env)

  void HELPER(rfi_r)(CPUHPPAState *env)
  {
-helper_getshadowregs(env);
+getshadowregs(env);
  helper_rfi(env);
  }

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 6da9503f33..29e4a64e40 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2385,14 +2385,23 @@ static bool trans_reset(DisasContext *ctx, arg_reset *a)
  #endif
  }

-static bool trans_getshadowregs(DisasContext *ctx, arg_getshadowregs *a)
+static bool do_getshadowregs(DisasContext *ctx)
  {
  CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
-#ifndef CONFIG_USER_ONLY
  nullify_over(ctx);
-gen_helper_getshadowregs(tcg_env);
+tcg_gen_ld_i64(cpu_gr[1], tcg_env, offsetof(CPUHPPAState, shadow[0]));
+tcg_gen_ld_i64(cpu_gr[8], tcg_env, offsetof(CPUHPPAState, shadow[1]));
+tcg_gen_ld_i64(cpu_gr[9], tcg_env, offsetof(CPUHPPAState, shadow[2]));
+tcg_gen_ld_i64(cpu_gr[16], tcg_env, offsetof(CPUHPPAState, shadow[3]));
+tcg_gen_ld_i64(cpu_gr[17], tcg_env, offsetof(CPUHPPAState, shadow[4]));
+tcg_gen_ld_i64(cpu_gr[24], tcg_env, offsetof(CPUHPPAState, shadow[5]));
+tcg_gen_ld_i64(cpu_gr[25], tcg_env, offsetof(CPUHPPAState, shadow[6]));
  return nullify_end(ctx);
-#endif
+}
+
+static bool trans_getshadowregs(DisasContext *ctx, arg_getshadowregs *a)
+{
+return do_getshadowregs(ctx);
  }

  static bool trans_nop_addrx(DisasContext *ctx, arg_ldst *a)





Re: [PATCH v2 2/3] target/hppa: Move diag argument handling to decodetree

2024-03-26 Thread Helge Deller

On 3/26/24 19:10, Richard Henderson wrote:

Split trans_diag into per-operation functions.

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 

Helge


---
  target/hppa/insns.decode |  8 +++-
  target/hppa/translate.c  | 34 +-
  2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 6a74cf23cd..9f6ffd8e2c 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -634,4 +634,10 @@ fdiv_d  001110 . . 011 . ... .  
@f0e_d_3
  xmpyu   001110 . . 010 .0111 .00 t:5r1=%ra64 r2=%rb64

  # diag
-diag000101 i:26
+{
+  [
+diag_btlb   000101 00    0001  
+diag_cout   000101 00    0001  0001
+  ]
+  diag_unimp000101 i:26
+}
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 29e4a64e40..42dd3f2c8d 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -4572,23 +4572,31 @@ static bool trans_fmpyfadd_d(DisasContext *ctx, 
arg_fmpyfadd_d *a)
  return nullify_end(ctx);
  }

-static bool trans_diag(DisasContext *ctx, arg_diag *a)
+/* Emulate PDC BTLB, called by SeaBIOS-hppa */
+static bool trans_diag_btlb(DisasContext *ctx, arg_diag_btlb *a)
  {
  CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
  #ifndef CONFIG_USER_ONLY
-if (a->i == 0x100) {
-/* emulate PDC BTLB, called by SeaBIOS-hppa */
-nullify_over(ctx);
-gen_helper_diag_btlb(tcg_env);
-return nullify_end(ctx);
-}
-if (a->i == 0x101) {
-/* print char in %r26 to first serial console, used by SeaBIOS-hppa */
-nullify_over(ctx);
-gen_helper_diag_console_output(tcg_env);
-return nullify_end(ctx);
-}
+nullify_over(ctx);
+gen_helper_diag_btlb(tcg_env);
+return nullify_end(ctx);
  #endif
+}
+
+/* Print char in %r26 to first serial console, used by SeaBIOS-hppa */
+static bool trans_diag_cout(DisasContext *ctx, arg_diag_cout *a)
+{
+CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
+#ifndef CONFIG_USER_ONLY
+nullify_over(ctx);
+gen_helper_diag_console_output(tcg_env);
+return nullify_end(ctx);
+#endif
+}
+
+static bool trans_diag_unimp(DisasContext *ctx, arg_diag_unimp *a)
+{
+CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
  qemu_log_mask(LOG_UNIMP, "DIAG opcode 0x%04x ignored\n", a->i);
  return true;
  }





Re: [PATCH for-9.0 0/3] target/hppa: Fix overflow computation for shladd

2024-03-26 Thread Helge Deller

On 3/26/24 07:44, Richard Henderson wrote:

These ??? notes have been there since day one.
This fixes l2diag test 59.


Your patches fix the 64-bit wdiag test 66 (shladd) too.

I tested 32/64-bit Linux & 32-bit HP-UX.
No regressions.

Helge



Richard Henderson (3):
   target/hppa: Squash d for pa1.x during decode
   target/hppa: Replace c with uv in do_cond
   target/hppa: Fix overflow computation for shladd

  target/hppa/insns.decode |  20 --
  target/hppa/translate.c  | 135 ++-
  2 files changed, 104 insertions(+), 51 deletions(-)






Re: [PATCH 3/3] target/hppa: Fix overflow computation for shladd

2024-03-26 Thread Helge Deller

On 3/26/24 07:44, Richard Henderson wrote:

Overflow indicator should include the effect of the shift step.
We had previously left ??? comments about the issue.

Signed-off-by: Richard Henderson 



Tested-by: Helge Deller 

Helge


---
  target/hppa/translate.c | 85 -
  1 file changed, 66 insertions(+), 19 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 9d31ef5764..0976372d16 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -994,7 +994,8 @@ static TCGv_i64 get_psw_carry(DisasContext *ctx, bool d)

  /* Compute signed overflow for addition.  */
  static TCGv_i64 do_add_sv(DisasContext *ctx, TCGv_i64 res,
-  TCGv_i64 in1, TCGv_i64 in2)
+  TCGv_i64 in1, TCGv_i64 in2,
+  TCGv_i64 orig_in1, int shift, bool d)
  {
  TCGv_i64 sv = tcg_temp_new_i64();
  TCGv_i64 tmp = tcg_temp_new_i64();
@@ -1003,9 +1004,50 @@ static TCGv_i64 do_add_sv(DisasContext *ctx, TCGv_i64 
res,
  tcg_gen_xor_i64(tmp, in1, in2);
  tcg_gen_andc_i64(sv, sv, tmp);

+switch (shift) {
+case 0:
+break;
+case 1:
+/* Shift left by one and compare the sign. */
+tcg_gen_add_i64(tmp, orig_in1, orig_in1);
+tcg_gen_xor_i64(tmp, tmp, orig_in1);
+/* Incorporate into the overflow. */
+tcg_gen_or_i64(sv, sv, tmp);
+break;
+default:
+{
+int sign_bit = d ? 63 : 31;
+uint64_t mask = MAKE_64BIT_MASK(sign_bit - shift, shift);
+
+/* Compare the sign against all lower bits. */
+tcg_gen_sextract_i64(tmp, orig_in1, sign_bit, 1);
+tcg_gen_xor_i64(tmp, tmp, orig_in1);
+/*
+ * If one of the bits shifting into or through the sign
+ * differs, then we have overflow.
+ */
+tcg_gen_movcond_i64(TCG_COND_TSTNE, sv,
+tmp, tcg_constant_i64(mask),
+tcg_constant_i64(-1), sv);
+}
+}
  return sv;
  }

+/* Compute unsigned overflow for addition.  */
+static TCGv_i64 do_add_uv(DisasContext *ctx, TCGv_i64 cb, TCGv_i64 cb_msb,
+  TCGv_i64 in1, int shift, bool d)
+{
+if (shift == 0) {
+return get_carry(ctx, d, cb, cb_msb);
+} else {
+TCGv_i64 tmp = tcg_temp_new_i64();
+tcg_gen_extract_i64(tmp, in1, (d ? 63 : 31) - shift, shift);
+tcg_gen_or_i64(tmp, tmp, get_carry(ctx, d, cb, cb_msb));
+return tmp;
+}
+}
+
  /* Compute signed overflow for subtraction.  */
  static TCGv_i64 do_sub_sv(DisasContext *ctx, TCGv_i64 res,
TCGv_i64 in1, TCGv_i64 in2)
@@ -1020,19 +1062,19 @@ static TCGv_i64 do_sub_sv(DisasContext *ctx, TCGv_i64 
res,
  return sv;
  }

-static void do_add(DisasContext *ctx, unsigned rt, TCGv_i64 in1,
+static void do_add(DisasContext *ctx, unsigned rt, TCGv_i64 orig_in1,
 TCGv_i64 in2, unsigned shift, bool is_l,
 bool is_tsv, bool is_tc, bool is_c, unsigned cf, bool d)
  {
-TCGv_i64 dest, cb, cb_msb, cb_cond, sv, tmp;
+TCGv_i64 dest, cb, cb_msb, in1, uv, sv, tmp;
  unsigned c = cf >> 1;
  DisasCond cond;

  dest = tcg_temp_new_i64();
  cb = NULL;
  cb_msb = NULL;
-cb_cond = NULL;

+in1 = orig_in1;
  if (shift) {
  tmp = tcg_temp_new_i64();
  tcg_gen_shli_i64(tmp, in1, shift);
@@ -1050,9 +1092,6 @@ static void do_add(DisasContext *ctx, unsigned rt, 
TCGv_i64 in1,
  }
  tcg_gen_xor_i64(cb, in1, in2);
  tcg_gen_xor_i64(cb, cb, dest);
-if (cond_need_cb(c)) {
-cb_cond = get_carry(ctx, d, cb, cb_msb);
-}
  } else {
  tcg_gen_add_i64(dest, in1, in2);
  if (is_c) {
@@ -1063,18 +1102,23 @@ static void do_add(DisasContext *ctx, unsigned rt, 
TCGv_i64 in1,
  /* Compute signed overflow if required.  */
  sv = NULL;
  if (is_tsv || cond_need_sv(c)) {
-sv = do_add_sv(ctx, dest, in1, in2);
+sv = do_add_sv(ctx, dest, in1, in2, orig_in1, shift, d);
  if (is_tsv) {
  if (!d) {
  tcg_gen_ext32s_i64(sv, sv);
  }
-/* ??? Need to include overflow from shift.  */
  gen_helper_tsv(tcg_env, sv);
  }
  }

+/* Compute unsigned overflow if required.  */
+uv = NULL;
+if (cond_need_cb(c)) {
+uv = do_add_uv(ctx, cb, cb_msb, orig_in1, shift, d);
+}
+
  /* Emit any conditional trap before any writeback.  */
-cond = do_cond(ctx, cf, d, dest, cb_cond, sv);
+cond = do_cond(ctx, cf, d, dest, uv, sv);
  if (is_tc) {
  tmp = tcg_temp_new_i64();
  tcg_gen_setcond_i64(cond.c, tmp, cond.a0, cond.a1);
@@ -2843,7 +2887,6 @@ static bool trans_dcor_i(DisasContext *ctx, arg_rr_cf_d 
*a)
  static bool trans_ds(DisasContex

Re: [PATCH 2/3] target/hppa: Replace c with uv in do_cond

2024-03-26 Thread Helge Deller

On 3/26/24 07:44, Richard Henderson wrote:

Prepare for proper indication of shladd unsigned overflow.
The UV indicator will be zero/not-zero instead of a single bit.

Signed-off-by: Richard Henderson 



Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Helge


---
  target/hppa/translate.c | 12 +---
  1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index a70d644c0b..9d31ef5764 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -707,7 +707,7 @@ static bool cond_need_cb(int c)
   */

  static DisasCond do_cond(DisasContext *ctx, unsigned cf, bool d,
- TCGv_i64 res, TCGv_i64 cb_msb, TCGv_i64 sv)
+ TCGv_i64 res, TCGv_i64 uv, TCGv_i64 sv)
  {
  DisasCond cond;
  TCGv_i64 tmp;
@@ -754,14 +754,12 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
  }
  cond = cond_make_0_tmp(TCG_COND_EQ, tmp);
  break;
-case 4: /* NUV / UV  (!C / C) */
-/* Only bit 0 of cb_msb is ever set. */
-cond = cond_make_0(TCG_COND_EQ, cb_msb);
+case 4: /* NUV / UV  (!UV / UV) */
+cond = cond_make_0(TCG_COND_EQ, uv);
  break;
-case 5: /* ZNV / VNZ (!C | Z / C & !Z) */
+case 5: /* ZNV / VNZ (!UV | Z / UV & !Z) */
  tmp = tcg_temp_new_i64();
-tcg_gen_neg_i64(tmp, cb_msb);
-tcg_gen_and_i64(tmp, tmp, res);
+tcg_gen_movcond_i64(TCG_COND_EQ, tmp, uv, ctx->zero, ctx->zero, res);
  if (!d) {
  tcg_gen_ext32u_i64(tmp, tmp);
  }





Re: [PATCH 1/3] target/hppa: Squash d for pa1.x during decode

2024-03-26 Thread Helge Deller

On 3/26/24 07:44, Richard Henderson wrote:

The cond_need_ext predicate was created while we still had a
32-bit compilation mode.  It now makes more sense to treat D
as an absolute indicator of a 64-bit operation.

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Helge



---
  target/hppa/insns.decode | 20 +---
  target/hppa/translate.c  | 38 --
  2 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index f58455dfdb..6a74cf23cd 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -57,6 +57,9 @@
  %neg_to_m   0:1  !function=neg_to_m
  %a_to_m 2:1  !function=neg_to_m
  %cmpbid_c   13:2 !function=cmpbid_c
+%d_55:1  !function=pa20_d
+%d_11   11:1 !function=pa20_d
+%d_13   13:1 !function=pa20_d

  
  # Argument set definitions
@@ -84,15 +87,16 @@
  # Format definitions
  

-@rr_cf_d.. r:5 . cf:4 .. d:1 t:5_cf_d
+@rr_cf_d.. r:5 . cf:4 .. . t:5  _cf_d d=%d_5
  @rrr.. r2:5 r1:5  ... t:5   
  @rrr_cf .. r2:5 r1:5 cf:4 ... t:5   _cf
-@rrr_cf_d   .. r2:5 r1:5 cf:4 .. d:1 t:5_cf_d
+@rrr_cf_d   .. r2:5 r1:5 cf:4 .. . t:5  _cf_d d=%d_5
  @rrr_sh .. r2:5 r1:5  sh:2 . t:5_sh
-@rrr_cf_d_sh.. r2:5 r1:5 cf:4  sh:2 d:1 t:5 _cf_d_sh
-@rrr_cf_d_sh0   .. r2:5 r1:5 cf:4 .. d:1 t:5_cf_d_sh sh=0
+@rrr_cf_d_sh.. r2:5 r1:5 cf:4  sh:2 . t:5   _cf_d_sh d=%d_5
+@rrr_cf_d_sh0   .. r2:5 r1:5 cf:4 .. . t:5  _cf_d_sh d=%d_5 
sh=0
  @rri_cf .. r:5  t:5  cf:4 . ... _cf i=%lowsign_11
-@rri_cf_d   .. r:5  t:5  cf:4 d:1 ...   _cf_d i=%lowsign_11
+@rri_cf_d   .. r:5  t:5  cf:4 . ... \
+_cf_d d=%d_11 i=%lowsign_11

  @rrb_cf .. r2:5 r1:5 c:3 ... n:1 .  \
  _c_f disp=%assemble_12
@@ -368,8 +372,10 @@ fmpysub_d   100110 . . . . 1 .  
@mpyadd
  # Conditional Branches
  

-bb_sar  11 0 r:5 c:1 1 d:1 ... n:1 . disp=%assemble_12
-bb_imm  110001 p:5   r:5 c:1 1 d:1 ... n:1 . disp=%assemble_12
+bb_sar  11 0 r:5 c:1 1 . ... n:1 . \
+disp=%assemble_12 d=%d_13
+bb_imm  110001 p:5   r:5 c:1 1 . ... n:1 . \
+disp=%assemble_12 d=%d_13

  movb110010 . . ... ... . .  @rrb_cf f=0
  movbi   110011 . . ... ... . .  @rib_cf f=0
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 99c5c4cbca..a70d644c0b 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -200,6 +200,14 @@ static int cmpbid_c(DisasContext *ctx, int val)
  return val ? val : 4; /* 0 == "*<<" */
  }

+/*
+ * In many places pa1.x did not decode the bit that later became
+ * the pa2.0 D bit.  Suppress D unless the cpu is pa2.0.
+ */
+static int pa20_d(DisasContext *ctx, int val)
+{
+return ctx->is_pa20 & val;
+}

  /* Include the auto-generated decoder.  */
  #include "decode-insns.c.inc"
@@ -693,12 +701,6 @@ static bool cond_need_cb(int c)
  return c == 4 || c == 5;
  }

-/* Need extensions from TCGv_i32 to TCGv_i64. */
-static bool cond_need_ext(DisasContext *ctx, bool d)
-{
-return !(ctx->is_pa20 && d);
-}
-
  /*
   * Compute conditional for arithmetic.  See Page 5-3, Table 5-1, of
   * the Parisc 1.1 Architecture Reference Manual for details.
@@ -715,7 +717,7 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
  cond = cond_make_f();
  break;
  case 1: /* = / <>(Z / !Z) */
-if (cond_need_ext(ctx, d)) {
+if (!d) {
  tmp = tcg_temp_new_i64();
  tcg_gen_ext32u_i64(tmp, res);
  res = tmp;
@@ -725,7 +727,7 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
  case 2: /* < / >=(N ^ V / !(N ^ V) */
  tmp = tcg_temp_new_i64();
  tcg_gen_xor_i64(tmp, res, sv);
-if (cond_need_ext(ctx, d)) {
+if (!d) {
  tcg_gen_ext32s_i64(tmp, tmp);
  }
  cond = cond_make_0_tmp(TCG_COND_LT, tmp);
@@ -742,7 +744,7 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
   */
  tmp = tcg_temp_new_i64();
  tcg_gen_eqv_i64(tmp, res, sv);
-if (cond_need_ext(ctx, d)) {
+if (!d) {
  tcg_gen_sextract_i64(tmp, tmp, 31, 1);
  tcg_gen_and_i64(tmp, tmp, res);
  tcg_gen_ext32u_i64(tmp, tmp);
@@ -760,13 +762,13 @@ static DisasCond do_cond(DisasContext *ctx, unsigned cf, 
bool d,
  tmp = tcg_temp_new_i64();
 

[PATCH] target/hppa: Fix diag instructions to set/restore shadow registers

2024-03-25 Thread Helge Deller
The 32-bit 7300LC CPU and the 64-bit PCX-W 8500 CPU use different
diag instructions to save or restore the CPU registers to/from
the shadow registers.

Implement those per-CPU architecture diag instructions to fix those
parts of the HP ODE testcases (L2DIAG and WDIAG, section 1) which test
the shadow registers.

Signed-off-by: Helge Deller 

diff --git a/target/hppa/helper.h b/target/hppa/helper.h
index 8fd7ba65d8..2c5d58bec9 100644
--- a/target/hppa/helper.h
+++ b/target/hppa/helper.h
@@ -86,6 +86,7 @@ DEF_HELPER_FLAGS_0(read_interval_timer, TCG_CALL_NO_RWG, tl)
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_1(halt, noreturn, env)
 DEF_HELPER_1(reset, noreturn, env)
+DEF_HELPER_1(putshadowregs, void, env)
 DEF_HELPER_1(getshadowregs, void, env)
 DEF_HELPER_1(rfi, void, env)
 DEF_HELPER_1(rfi_r, void, env)
diff --git a/target/hppa/sys_helper.c b/target/hppa/sys_helper.c
index 4a31748342..3727f4ce8b 100644
--- a/target/hppa/sys_helper.c
+++ b/target/hppa/sys_helper.c
@@ -95,6 +95,17 @@ void HELPER(rfi)(CPUHPPAState *env)
 cpu_hppa_put_psw(env, env->cr[CR_IPSW]);
 }
 
+void HELPER(putshadowregs)(CPUHPPAState *env)
+{
+env->shadow[0] = env->gr[1];
+env->shadow[1] = env->gr[8];
+env->shadow[2] = env->gr[9];
+env->shadow[3] = env->gr[16];
+env->shadow[4] = env->gr[17];
+env->shadow[5] = env->gr[24];
+env->shadow[6] = env->gr[25];
+}
+
 void HELPER(getshadowregs)(CPUHPPAState *env)
 {
 env->gr[1] = env->shadow[0];
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 99c5c4cbca..40f1cbe7ed 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -4533,6 +4533,18 @@ static bool trans_diag(DisasContext *ctx, arg_diag *a)
 gen_helper_diag_console_output(tcg_env);
 return nullify_end(ctx);
 }
+if ((ctx->is_pa20 && a->i == 0x701840) ||
+(!ctx->is_pa20 && a->i == 0x1a40)) {
+/* save shadow registers */
+nullify_over(ctx);
+gen_helper_putshadowregs(tcg_env);
+return nullify_end(ctx);
+}
+if ((ctx->is_pa20 && a->i == 0x781840) ||
+(!ctx->is_pa20 && a->i == 0x1a00)) {
+/* restore shadow registers */
+return trans_getshadowregs(ctx, NULL);
+}
 #endif
 qemu_log_mask(LOG_UNIMP, "DIAG opcode 0x%04x ignored\n", a->i);
 return true;



Re: [PATCH v2] target/hppa: Fix unit carry conditions

2024-03-25 Thread Helge Deller

On 3/25/24 20:02, Richard Henderson wrote:

Split do_unit_cond to do_unit_zero_cond to only handle conditions
versus zero.  These are the only ones that are legal for UXOR.
Simplify trans_uxor accordingly.

Rename do_unit to do_unit_addsub, since xor has been split.
Properly compute carry-out bits for add and subtract, mirroring
the code in do_add and do_sub.

Signed-off-by: Richard Henderson 


this patch does not break test #55 (uaddcm) any longer, and with the other
two patches test #58 (uaddcm & dcor) is OK as well.

So, for the whole series:
Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Thanks!
Helge




---

v2: Cut and paste error between 64- and 32-bit paths.
 Shift 32-bit carry down 1 bit like 64-bit carry;
 tradeoff is shift vs needing a 64-bit constant for the mask.
 Don't use of TCG_COND_TST{NE,EQ}, as this will limit backports
 of the actual bug fix.  We can convert the port to test conditions
 en masse during the next devel cycle.

---
  target/hppa/translate.c | 218 +---
  1 file changed, 113 insertions(+), 105 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 3fc3e7754c..99c5c4cbca 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -936,98 +936,44 @@ static DisasCond do_sed_cond(DisasContext *ctx, unsigned 
orig, bool d,
  return do_log_cond(ctx, c * 2 + f, d, res);
  }

-/* Similar, but for unit conditions.  */
-
-static DisasCond do_unit_cond(unsigned cf, bool d, TCGv_i64 res,
-  TCGv_i64 in1, TCGv_i64 in2)
+/* Similar, but for unit zero conditions.  */
+static DisasCond do_unit_zero_cond(unsigned cf, bool d, TCGv_i64 res)
  {
-DisasCond cond;
-TCGv_i64 tmp, cb = NULL;
+TCGv_i64 tmp;
  uint64_t d_repl = d ? 0x00010001ull : 1;
-
-if (cf & 8) {
-/* Since we want to test lots of carry-out bits all at once, do not
- * do our normal thing and compute carry-in of bit B+1 since that
- * leaves us with carry bits spread across two words.
- */
-cb = tcg_temp_new_i64();
-tmp = tcg_temp_new_i64();
-tcg_gen_or_i64(cb, in1, in2);
-tcg_gen_and_i64(tmp, in1, in2);
-tcg_gen_andc_i64(cb, cb, res);
-tcg_gen_or_i64(cb, cb, tmp);
-}
+uint64_t ones = 0, sgns = 0;

  switch (cf >> 1) {
-case 0: /* never / TR */
-cond = cond_make_f();
-break;
-
  case 1: /* SBW / NBW */
  if (d) {
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x0001u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x8000u);
-cond = cond_make_0(TCG_COND_NE, tmp);
-} else {
-/* undefined */
-cond = cond_make_f();
+ones = d_repl;
+sgns = d_repl << 31;
  }
  break;
-
  case 2: /* SBZ / NBZ */
-/* See hasless(v,1) from
- * https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
- */
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x01010101u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x80808080u);
-cond = cond_make_0(TCG_COND_NE, tmp);
+ones = d_repl * 0x01010101u;
+sgns = ones << 7;
  break;
-
  case 3: /* SHZ / NHZ */
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x00010001u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x80008000u);
-cond = cond_make_0(TCG_COND_NE, tmp);
+ones = d_repl * 0x00010001u;
+sgns = ones << 15;
  break;
-
-case 4: /* SDC / NDC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0xu);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-case 5: /* SWC / NWC */
-if (d) {
-tcg_gen_andi_i64(cb, cb, d_repl * 0x8000u);
-cond = cond_make_0(TCG_COND_NE, cb);
-} else {
-/* undefined */
-cond = cond_make_f();
-}
-break;
-
-case 6: /* SBC / NBC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0x80808080u);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-case 7: /* SHC / NHC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0x80008000u);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-default:
-g_assert_not_reached();
  }
-if (cf & 1) {
-cond.c = tcg_invert_cond(cond.c);
+if (ones == 0) {
+/* Undefined, or 0/1 (never/always). */
+return cf & 1 ? cond_make_t() : cond_make_f();
  }

-return cond;
+/*
+ * See hasless(v,1) from
+ * https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
+ */
+tmp = tcg_temp_new_i64();
+tcg_gen_subi_i64(tmp, re

Re: [PATCH 3/3] target/hppa: Fix unit carry conditions

2024-03-25 Thread Helge Deller

On 3/25/24 04:04, Richard Henderson wrote:

Split do_unit_cond to do_unit_zero_cond to only handle
conditions versus zero.  These are the only ones that
are legal for UXOR.  Simplify trans_uxor accordingly.

Rename do_unit to do_unit_addsub, since xor has been split.
Properly compute carry-out bits for add and subtract,
mirroring the code in do_add and do_sub.

Signed-off-by: Richard Henderson 


This patch triggers a failure in SECTION 055 (32-bit)
ERROR 0999 IN SECTION 055
UNEXPECTED TRAP# 13
IN:
0x001a2b2c:  uaddcm,tc,shc r13,r14,r15
r13..r15:   




---
  target/hppa/translate.c | 214 
  1 file changed, 109 insertions(+), 105 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 3fc3e7754c..2bf213c938 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -936,98 +936,44 @@ static DisasCond do_sed_cond(DisasContext *ctx, unsigned 
orig, bool d,
  return do_log_cond(ctx, c * 2 + f, d, res);
  }

-/* Similar, but for unit conditions.  */
-
-static DisasCond do_unit_cond(unsigned cf, bool d, TCGv_i64 res,
-  TCGv_i64 in1, TCGv_i64 in2)
+/* Similar, but for unit zero conditions.  */
+static DisasCond do_unit_zero_cond(unsigned cf, bool d, TCGv_i64 res)
  {
-DisasCond cond;
-TCGv_i64 tmp, cb = NULL;
+TCGv_i64 tmp;
  uint64_t d_repl = d ? 0x00010001ull : 1;
-
-if (cf & 8) {
-/* Since we want to test lots of carry-out bits all at once, do not
- * do our normal thing and compute carry-in of bit B+1 since that
- * leaves us with carry bits spread across two words.
- */
-cb = tcg_temp_new_i64();
-tmp = tcg_temp_new_i64();
-tcg_gen_or_i64(cb, in1, in2);
-tcg_gen_and_i64(tmp, in1, in2);
-tcg_gen_andc_i64(cb, cb, res);
-tcg_gen_or_i64(cb, cb, tmp);
-}
+uint64_t ones = 0, sgns = 0;

  switch (cf >> 1) {
-case 0: /* never / TR */
-cond = cond_make_f();
-break;
-
  case 1: /* SBW / NBW */
  if (d) {
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x0001u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x8000u);
-cond = cond_make_0(TCG_COND_NE, tmp);
-} else {
-/* undefined */
-cond = cond_make_f();
+ones = d_repl;
+sgns = d_repl << 31;
  }
  break;
-
  case 2: /* SBZ / NBZ */
-/* See hasless(v,1) from
- * https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
- */
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x01010101u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x80808080u);
-cond = cond_make_0(TCG_COND_NE, tmp);
+ones = d_repl * 0x01010101u;
+sgns = ones << 7;
  break;
-
  case 3: /* SHZ / NHZ */
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x00010001u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x80008000u);
-cond = cond_make_0(TCG_COND_NE, tmp);
+ones = d_repl * 0x00010001u;
+sgns = ones << 15;
  break;
-
-case 4: /* SDC / NDC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0xu);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-case 5: /* SWC / NWC */
-if (d) {
-tcg_gen_andi_i64(cb, cb, d_repl * 0x8000u);
-cond = cond_make_0(TCG_COND_NE, cb);
-} else {
-/* undefined */
-cond = cond_make_f();
-}
-break;
-
-case 6: /* SBC / NBC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0x80808080u);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-case 7: /* SHC / NHC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0x80008000u);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-default:
-g_assert_not_reached();
  }
-if (cf & 1) {
-cond.c = tcg_invert_cond(cond.c);
+if (ones == 0) {
+/* Undefined, or 0/1 (never/always). */
+return cf & 1 ? cond_make_t() : cond_make_f();
  }

-return cond;
+/*
+ * See hasless(v,1) from
+ * https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
+ */
+tmp = tcg_temp_new_i64();
+tcg_gen_subi_i64(tmp, res, ones);
+tcg_gen_andc_i64(tmp, tmp, res);
+
+return cond_make_tmp(cf & 1 ? TCG_COND_TSTEQ : TCG_COND_TSTNE,
+ tmp, tcg_constant_i64(sgns));
  }

  static TCGv_i64 get_carry(DisasContext *ctx, bool d,
@@ -1330,34 +1276,82 @@ static bool do_log_reg(DisasContext *ctx, arg_rrr_cf_d 
*a,
  return nullify_end(ctx);
  }

-static void do_unit(DisasContext *ctx, unsigned rt, TCGv_i64 in1,
-

Re: [PATCH 2/3] target/hppa: Optimize UADDCM with no condition

2024-03-25 Thread Helge Deller

On 3/25/24 04:04, Richard Henderson wrote:

With r1 as zero is by far the only usage of UADDCM, as the easiest
way to invert a register.  The compiler does occasionally use the
addition step as well, and we can simplify that to avoid a temp
and write directly into the destination.

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Helge



---
  target/hppa/translate.c | 24 ++--
  1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index a3f425d861..3fc3e7754c 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2763,9 +2763,29 @@ static bool do_uaddcm(DisasContext *ctx, arg_rrr_cf_d 
*a, bool is_tc)
  {
  TCGv_i64 tcg_r1, tcg_r2, tmp;

-if (a->cf) {
-nullify_over(ctx);
+if (a->cf == 0) {
+tcg_r2 = load_gpr(ctx, a->r2);
+tmp = dest_gpr(ctx, a->t);
+
+if (a->r1 == 0) {
+/* UADDCM r0,src,dst is the common idiom for dst = ~src. */
+tcg_gen_not_i64(tmp, tcg_r2);
+} else {
+/*
+ * Recall that r1 - r2 == r1 + ~r2 + 1.
+ * Thus r1 + ~r2 == r1 - r2 - 1,
+ * which does not require an extra temporary.
+ */
+tcg_r1 = load_gpr(ctx, a->r1);
+tcg_gen_sub_i64(tmp, tcg_r1, tcg_r2);
+tcg_gen_subi_i64(tmp, tmp, 1);
+}
+save_gpr(ctx, a->t, tmp);
+cond_free(>null_cond);
+return true;
  }
+
+nullify_over(ctx);
  tcg_r1 = load_gpr(ctx, a->r1);
  tcg_r2 = load_gpr(ctx, a->r2);
  tmp = tcg_temp_new_i64();





Re: [PATCH 1/3] targt/hppa: Fix DCOR reconstruction of carry bits

2024-03-25 Thread Helge Deller

On 3/25/24 04:04, Richard Henderson wrote:

The carry bits for each nibble N are located in bit (N+1)*4,
so the shift by 3 was off by one.  Furthermore, the carry bit
for the most significant carry bit is indeed located in bit 64,
which is located in a different storage word.

Use a double-word shift-right to reassemble into a single word
and place them all at bit 0 of their respective nibbles.

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Helge


---
  target/hppa/translate.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index e041310207..a3f425d861 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2791,7 +2791,7 @@ static bool do_dcor(DisasContext *ctx, arg_rr_cf_d *a, 
bool is_i)
  nullify_over(ctx);

  tmp = tcg_temp_new_i64();
-tcg_gen_shri_i64(tmp, cpu_psw_cb, 3);
+tcg_gen_extract2_i64(tmp, cpu_psw_cb, cpu_psw_cb_msb, 4);
  if (!is_i) {
  tcg_gen_not_i64(tmp, tmp);
  }





Re: [PATCH][RFC] target/hppa: Avoid nullification for uaddcmt instruction

2024-03-24 Thread Helge Deller

On 3/24/24 18:13, Richard Henderson wrote:

On 3/23/24 11:15, Helge Deller wrote:

The uaddcmt (UNIT ADD COMPLEMENT AND TRAP ON CONDITION) instruction
triggers a trap if the condition is true, and stores the result of the
addition in the target register otherwise.
It does not use the condition to nullify the following instruction, so
drop the calculated condition and do not install it as null_cond.


That's not what the manual says:

if (trapc == TC && cond_satisfied)
     conditional_trap;
else {
     GR[t] ← res;
     if (cond_satisfied) PSW[N] ← 1;
}


The above is from the 64-bit manual.
My mail was based on the info from the 32-bit manual, which says:
res ← GR[r1] + ∼GR[r2];
if (cond_satisfied)
conditional_trap;
else
GR[t] ← res

But basically it doesn't matter, since as you explain below,
if it traps, it won't return anyway and as such PSW[N] will
not be modified.


We have implemented this like so:

if (trapc == TC)
     conditional_trap(cond_satisfied);
GR[t] ← res;
if (cond_satisfied) PSW[N] ← 1;

because the conditional trap step does not return if it traps.


Yes.
Thanks for review anyway!

Helge


This patch is not tested and as such sent as RFC.
I just stumbled over the apparently wrong behaviour while debugging the
uaddcm instruction.


Under separate cover you said the condition might be computed incorrectly.  I 
think that is more likely the root cause of the wrong behaviour.


r~






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

2024-03-24 Thread 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 


Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

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 2/3] target/hppa: mask offset bits in gva

2024-03-24 Thread 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 (0xfffa) on my C8000 and J6750:

running on Linux: 3fff c000fffa0500
running on HP-UX: 301f c000fffa0500

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 


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 

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 0x301f
  #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 1/3] target/hppa: use gva_offset_mask() everywhere

2024-03-24 Thread 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 


Reviewed-by: Helge Deller 

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





[PATCH][RFC] target/hppa: Avoid nullification for uaddcmt instruction

2024-03-23 Thread Helge Deller
The uaddcmt (UNIT ADD COMPLEMENT AND TRAP ON CONDITION) instruction
triggers a trap if the condition is true, and stores the result of the
addition in the target register otherwise.
It does not use the condition to nullify the following instruction, so
drop the calculated condition and do not install it as null_cond.

This patch is not tested and as such sent as RFC.
I just stumbled over the apparently wrong behaviour while debugging the
uaddcm instruction.

Signed-off-by: Helge Deller 

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 46b2d6508d..6088e9bbf3 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -1363,7 +1363,11 @@ static void do_unit(DisasContext *ctx, unsigned rt, 
TCGv_i64 in1,
 save_gpr(ctx, rt, dest);
 
 cond_free(>null_cond);
-ctx->null_cond = cond;
+if (is_tc) {
+cond_free();
+} else {
+ctx->null_cond = cond;
+}
 }
 }
 



Re: [PATCH v2 7/7] target/hppa: Fix EIRR, EIEM versus icount

2024-03-23 Thread Helge Deller

On 3/23/24 18:29, Richard Henderson wrote:

Call translator_io_start before write to EIRR.
Move evaluation of EIRR vs EIEM to hppa_cpu_exec_interrupt.
Exit TB after write to EIEM, but otherwise use a straight store.

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Thanks!
Helge



---
  target/hppa/helper.h |  1 -
  target/hppa/int_helper.c | 14 --
  target/hppa/translate.c  | 10 +++---
  3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/target/hppa/helper.h b/target/hppa/helper.h
index 1bdbcd8f98..8fd7ba65d8 100644
--- a/target/hppa/helper.h
+++ b/target/hppa/helper.h
@@ -91,7 +91,6 @@ DEF_HELPER_1(rfi, void, env)
  DEF_HELPER_1(rfi_r, void, env)
  DEF_HELPER_FLAGS_2(write_interval_timer, TCG_CALL_NO_RWG, void, env, tl)
  DEF_HELPER_FLAGS_2(write_eirr, TCG_CALL_NO_RWG, void, env, tl)
-DEF_HELPER_FLAGS_2(write_eiem, TCG_CALL_NO_RWG, void, env, tl)
  DEF_HELPER_FLAGS_2(swap_system_mask, TCG_CALL_NO_RWG, tl, env, tl)
  DEF_HELPER_FLAGS_3(itlba_pa11, TCG_CALL_NO_RWG, void, env, tl, tl)
  DEF_HELPER_FLAGS_3(itlbp_pa11, TCG_CALL_NO_RWG, void, env, tl, tl)
diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
index efe638b36e..90437a92cd 100644
--- a/target/hppa/int_helper.c
+++ b/target/hppa/int_helper.c
@@ -28,7 +28,7 @@
  static void eval_interrupt(HPPACPU *cpu)
  {
  CPUState *cs = CPU(cpu);
-if (cpu->env.cr[CR_EIRR] & cpu->env.cr[CR_EIEM]) {
+if (cpu->env.cr[CR_EIRR]) {
  cpu_interrupt(cs, CPU_INTERRUPT_HARD);
  } else {
  cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
@@ -89,14 +89,6 @@ void HELPER(write_eirr)(CPUHPPAState *env, target_ulong val)
  bql_unlock();
  }

-void HELPER(write_eiem)(CPUHPPAState *env, target_ulong val)
-{
-env->cr[CR_EIEM] = val;
-bql_lock();
-eval_interrupt(env_archcpu(env));
-bql_unlock();
-}
-
  void hppa_cpu_do_interrupt(CPUState *cs)
  {
  HPPACPU *cpu = HPPA_CPU(cs);
@@ -280,7 +272,9 @@ bool hppa_cpu_exec_interrupt(CPUState *cs, int 
interrupt_request)
  }

  /* If interrupts are requested and enabled, raise them.  */
-if ((env->psw & PSW_I) && (interrupt_request & CPU_INTERRUPT_HARD)) {
+if ((interrupt_request & CPU_INTERRUPT_HARD)
+&& (env->psw & PSW_I)
+&& (env->cr[CR_EIRR] & env->cr[CR_EIEM])) {
  cs->exception_index = EXCP_EXT_INTERRUPT;
  hppa_cpu_do_interrupt(cs);
  return true;
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 5b8c1b06c3..46b2d6508d 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2166,10 +2166,10 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
  gen_helper_write_interval_timer(tcg_env, reg);
  break;
  case CR_EIRR:
+/* Helper modifies interrupt lines and is therefore IO. */
+translator_io_start(>base);
  gen_helper_write_eirr(tcg_env, reg);
-break;
-case CR_EIEM:
-gen_helper_write_eiem(tcg_env, reg);
+/* Exit to re-evaluate interrupts in the main loop. */
  ctx->base.is_jmp = DISAS_IAQ_N_STALE_EXIT;
  break;

@@ -2195,6 +2195,10 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
  #endif
  break;

+case CR_EIEM:
+/* Exit to re-evaluate interrupts in the main loop. */
+ctx->base.is_jmp = DISAS_IAQ_N_STALE_EXIT;
+/* FALLTHRU */
  default:
  tcg_gen_st_i64(reg, tcg_env, offsetof(CPUHPPAState, cr[ctl]));
  break;





Re: [PATCH v2 5/7] target/hppa: Mark interval timer write as io

2024-03-23 Thread Helge Deller

On 3/23/24 18:29, Richard Henderson wrote:

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Thanks!
Helge


---
  target/hppa/translate.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index ceb739c54a..8c1a564c5d 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2162,6 +2162,9 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)

  switch (ctl) {
  case CR_IT:
+if (translator_io_start(>base)) {
+ctx->base.is_jmp = DISAS_IAQ_N_STALE;
+}
  gen_helper_write_interval_timer(tcg_env, reg);
  break;
  case CR_EIRR:





Re: [PATCH v2 6/7] target/hppa: Tidy read of interval timer

2024-03-23 Thread Helge Deller

On 3/23/24 18:29, Richard Henderson wrote:

The call to gen_helper_read_interval_timer is
identical on both sides of the IF.

Signed-off-by: Richard Henderson 



Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Thanks!
Helge


---
  target/hppa/translate.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 8c1a564c5d..5b8c1b06c3 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2082,11 +2082,9 @@ static bool trans_mfctl(DisasContext *ctx, arg_mfctl *a)
  nullify_over(ctx);
  tmp = dest_gpr(ctx, rt);
  if (translator_io_start(>base)) {
-gen_helper_read_interval_timer(tmp);
  ctx->base.is_jmp = DISAS_IAQ_N_STALE;
-} else {
-gen_helper_read_interval_timer(tmp);
  }
+gen_helper_read_interval_timer(tmp);
  save_gpr(ctx, rt, tmp);
  return nullify_end(ctx);
  case 26:





Re: [PATCH v2 1/7] target/hppa: Fix BE,L set of sr0

2024-03-23 Thread Helge Deller

On 3/23/24 18:29, Richard Henderson wrote:

The return address comes from IA*Q_Next, and IASQ_Next
is always equal to IASQ_Back, not IASQ_Front.

Signed-off-by: Richard Henderson 


Tested-by: Helge Deller 



---
  target/hppa/translate.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 19594f917e..1766a63001 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3817,7 +3817,7 @@ static bool trans_be(DisasContext *ctx, arg_be *a)
  load_spr(ctx, new_spc, a->sp);
  if (a->l) {
  copy_iaoq_entry(ctx, cpu_gr[31], ctx->iaoq_n, ctx->iaoq_n_var);
-tcg_gen_mov_i64(cpu_sr[0], cpu_iasq_f);
+tcg_gen_mov_i64(cpu_sr[0], cpu_iasq_b);
  }
  if (a->n && use_nullify_skip(ctx)) {
  copy_iaoq_entry(ctx, cpu_iaoq_f, -1, tmp);





Re: [PATCH 2/2] target/hppa: Fix B,GATE for wide mode

2024-03-22 Thread Helge Deller

On 3/21/24 20:28, Richard Henderson wrote:

Do not clobber the high bits of the address by using a 32-bit deposit.

Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 

Helge


---
  target/hppa/translate.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 1766a63001..f875d76a23 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3880,7 +3880,7 @@ static bool trans_b_gate(DisasContext *ctx, arg_b_gate *a)
  }
  /* No change for non-gateway pages or for priv decrease.  */
  if (type >= 4 && type - 4 < ctx->privilege) {
-dest = deposit32(dest, 0, 2, type - 4);
+dest = deposit64(dest, 0, 2, type - 4);
  }
  } else {
  dest &= -4;  /* priv = 0 */





Re: [PULL 0/9] target/hppa fixes for 9.0

2024-03-21 Thread Helge Deller

On 3/21/24 19:25, Sven Schnelle wrote:

Michael Tokarev  writes:


20.03.2024 03:32, Richard Henderson :


Richard Henderson (3):
target/hppa: Fix assemble_16 insns for wide mode
target/hppa: Fix assemble_11a insns for wide mode
target/hppa: Fix assemble_12a insns for wide mode
Sven Schnelle (6):
target/hppa: ldcw,s uses static shift of 3
target/hppa: fix shrp for wide mode
target/hppa: fix access_id check
target/hppa: exit tb on flush cache instructions
target/hppa: mask privilege bits in mfia
target/hppa: fix do_stdby_e()


Is it all -stable material (when appropriate)?


I'd say yes.


Yes.

Helge



Re: [PATCH 6/7] target/hppa: mask privilege bits in mfia

2024-03-18 Thread Helge Deller

On 3/17/24 23:14, Sven Schnelle wrote:

mfia should return only the iaoq bits without privilege
bits.

Signed-off-by: Sven Schnelle 


Reviewed-by: Helge Deller 

Helge



---
  target/hppa/translate.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index a09112e4ae..e47f8f9f47 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -1962,7 +1962,7 @@ static bool trans_mfia(DisasContext *ctx, arg_mfia *a)
  {
  unsigned rt = a->t;
  TCGv_i64 tmp = dest_gpr(ctx, rt);
-tcg_gen_movi_i64(tmp, ctx->iaoq_f);
+tcg_gen_movi_i64(tmp, ctx->iaoq_f & ~3ULL);
  save_gpr(ctx, rt, tmp);

  cond_free(>null_cond);





Re: [PATCH 4/7] target/hppa: exit tb on flush cache instructions

2024-03-18 Thread Helge Deller

On 3/17/24 23:14, Sven Schnelle wrote:

When the guest modifies the tb it is currently executing from,
it executes a fic instruction. Exit the tb on such instruction,
otherwise we might execute stale code.

Signed-off-by: Sven Schnelle 
---
  target/hppa/translate.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 8ba31567e8..58d7ec1ade 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -1940,6 +1940,7 @@ static void do_page_zero(DisasContext *ctx)
  static bool trans_nop(DisasContext *ctx, arg_nop *a)
  {
  cond_free(>null_cond);
+ctx->base.is_jmp = DISAS_IAQ_N_STALE;
  return true;
  }

@@ -2290,6 +2291,7 @@ static bool trans_nop_addrx(DisasContext *ctx, arg_ldst 
*a)
  save_gpr(ctx, a->b, dest);
  }
  cond_free(>null_cond);
+ctx->base.is_jmp = DISAS_IAQ_N_STALE;


I wonder if it makes sense to rename trans_nop() and trans_nop_addrx()
to something like trans_cache_flush() and trans_cache_flush_addrx() ?

Other than that:
Reviewed-by: Helge Deller 

Helge



Re: [PATCH 3/7] target/hppa: fix access_id check

2024-03-18 Thread Helge Deller

On 3/17/24 23:14, Sven Schnelle wrote:

PA2.0 provides 8 instead of 4 PID registers.

Signed-off-by: Sven Schnelle 


Reviewed-by: Helge Deller 
with a few comments below...

Helge


---
  roms/SLOF|  2 +-
  target/hppa/mem_helper.c | 67 +++-
  2 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/roms/SLOF b/roms/SLOF
index 3a259df244..6b6c16b4b4 16
--- a/roms/SLOF
+++ b/roms/SLOF
@@ -1 +1 @@
-Subproject commit 3a259df2449fc4a4e43ab5f33f0b2c66484b4bc3
+Subproject commit 6b6c16b4b40763507cf1f518096f3c3883c5cf2d


this doesn't belong here.



diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 80f51e753f..e4e3f6cdbe 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -152,6 +152,59 @@ static HPPATLBEntry *hppa_alloc_tlb_ent(CPUHPPAState *env)
  return ent;
  }

+static uint32_t get_pid(CPUHPPAState *env, int num)
+{
+const struct pid_map {
+int reg;
+bool shift;


does it makes sense to condense it, e.g.:
 +unsigned char reg:7,
 +unsigned char shift:1;

Helge



+} *pid;
+
+const struct pid_map pids64[] = {
+{ .reg = 8,  .shift = true  },
+{ .reg = 8,  .shift = false },
+{ .reg = 9,  .shift = true  },
+{ .reg = 9,  .shift = false },
+{ .reg = 12, .shift = true  },
+{ .reg = 12, .shift = false },
+{ .reg = 13, .shift = true  },
+{ .reg = 13, .shift = false }
+};
+
+const struct pid_map pids32[] = {
+{ .reg = 8,  .shift = false  },
+{ .reg = 9,  .shift = false  },
+{ .reg = 12, .shift = false  },
+{ .reg = 13, .shift = false  },
+};
+
+if (hppa_is_pa20(env)) {
+pid = pids64 + num;
+} else {
+pid = pids32 + num;
+}
+uint64_t cr = env->cr[pid->reg];
+if (pid->shift) {
+cr >>= 32;
+} else {
+cr &= 0x;
+}
+return cr;
+}
+
+#define ACCESS_ID_MASK 0x
+
+static bool match_prot_id(CPUHPPAState *env, uint32_t access_id, uint32_t 
*_pid)
+{
+for (int i = 0; i < 8; i++) {
+uint32_t pid = get_pid(env, i);
+if ((access_id & ACCESS_ID_MASK) == ((pid >> 1) & ACCESS_ID_MASK)) {
+*_pid = pid;
+return true;
+}
+}
+return false;
+}
+
  int hppa_get_physical_address(CPUHPPAState *env, vaddr addr, int mmu_idx,
int type, hwaddr *pphys, int *pprot,
HPPATLBEntry **tlb_entry)
@@ -227,15 +280,13 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr 
addr, int mmu_idx,
  /* access_id == 0 means public page and no check is performed */
  if (ent->access_id && MMU_IDX_TO_P(mmu_idx)) {
  /* If bits [31:1] match, and bit 0 is set, suppress write.  */
-int match = ent->access_id * 2 + 1;
-
-if (match == env->cr[CR_PID1] || match == env->cr[CR_PID2] ||
-match == env->cr[CR_PID3] || match == env->cr[CR_PID4]) {
-prot &= PAGE_READ | PAGE_EXEC;
-if (type == PAGE_WRITE) {
-ret = EXCP_DMPI;
-goto egress;
+uint32_t pid;
+if (match_prot_id(env, ent->access_id, )) {
+if ((pid & 1) && (prot & PROT_WRITE)) {
+prot &= ~PROT_WRITE;
  }
+} else {
+prot = 0;
  }
  }






Re: [PATCH 2/7] target/hppa: fix shrp for wide mode

2024-03-18 Thread Helge Deller

On 3/17/24 23:14, Sven Schnelle wrote:

Signed-off-by: Sven Schnelle 


Reviewed-by: Helge Deller 

Helge


---
  target/hppa/translate.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 6a513d7d5c..8ba31567e8 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3462,7 +3462,7 @@ static bool trans_shrp_sar(DisasContext *ctx, 
arg_shrp_sar *a)
  /* Install the new nullification.  */
  cond_free(>null_cond);
  if (a->c) {
-ctx->null_cond = do_sed_cond(ctx, a->c, false, dest);
+ctx->null_cond = do_sed_cond(ctx, a->c, a->d, dest);
  }
  return nullify_end(ctx);
  }
@@ -3505,7 +3505,7 @@ static bool trans_shrp_imm(DisasContext *ctx, 
arg_shrp_imm *a)
  /* Install the new nullification.  */
  cond_free(>null_cond);
  if (a->c) {
-ctx->null_cond = do_sed_cond(ctx, a->c, false, dest);
+ctx->null_cond = do_sed_cond(ctx, a->c, a->d, dest);
  }
  return nullify_end(ctx);
  }





Re: [PATCH v2 1/3] target/hppa: Fix assemble_16 insns for wide mode

2024-03-18 Thread Helge Deller

On 3/13/24 23:51, Richard Henderson wrote:

Reported-by: Sven Schnelle 
Signed-off-by: Richard Henderson 


Reviewed-by: Helge Deller 




---
  target/hppa/insns.decode | 15 +--
  target/hppa/translate.c  | 22 ++
  2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index f5a3f02fd1..0d9f8159ec 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -27,13 +27,14 @@
  %assemble_11a   0:s1 4:10!function=expand_shl3
  %assemble_120:s1 2:1 3:10!function=expand_shl2
  %assemble_12a   0:s1 3:11!function=expand_shl2
+%assemble_160:16 !function=expand_16
  %assemble_170:s1 16:5 2:1 3:10   !function=expand_shl2
  %assemble_220:s1 16:10 2:1 3:10  !function=expand_shl2
+%assemble_sp14:2 !function=sp0_if_wide

  %assemble_210:s1 1:11 14:2 16:5 12:2  !function=expand_shl11

  %lowsign_11 0:s1 1:10
-%lowsign_14 0:s1 1:13

  %sm_imm 16:10 !function=expand_sm_imm

@@ -221,7 +222,7 @@ sub_b_tsv   10 . .  110100 . .  
@rrr_cf_d

  ldil001000 t:5 .i=%assemble_21
  addil   001010 r:5 .i=%assemble_21
-ldo 001101 b:5 t:5 -- ..i=%lowsign_14
+ldo 001101 b:5 t:5  i=%assemble_16

  addi101101 . .  0 ...   @rri_cf
  addi_tsv101101 . .  1 ...   @rri_cf
@@ -306,10 +307,12 @@ fstd001011 . . .. . 1 -- 100 0 . 
.  @fldstdi

  @ldstim11   .. b:5 t:5 sp:2 ..  \
   disp=%assemble_11a m=%ma2_to_m x=0 scale=0 size=3
-@ldstim14   .. b:5 t:5 sp:2 ..  \
- disp=%lowsign_14 x=0 scale=0 m=0
-@ldstim14m  .. b:5 t:5 sp:2 ..  \
- disp=%lowsign_14 x=0 scale=0 m=%neg_to_m
+@ldstim14   .. b:5 t:5   \
+ sp=%assemble_sp disp=%assemble_16  \
+x=0 scale=0 m=0
+@ldstim14m  .. b:5 t:5   \
+ sp=%assemble_sp disp=%assemble_16  \
+x=0 scale=0 m=%neg_to_m
  @ldstim12m  .. b:5 t:5 sp:2 ..  \
   disp=%assemble_12a x=0 scale=0 m=%pos_to_m

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index eb2046c5ad..cbe44ef75a 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -144,6 +144,28 @@ static int assemble_6(DisasContext *ctx, int val)
  return (val ^ 31) + 1;
  }

+/* Expander for assemble_16(s,im14). */
+static int expand_16(DisasContext *ctx, int val)
+{
+/*
+ * @val is bits [0:15], containing both im14 and s.
+ * Swizzle thing around depending on PSW.W.
+ */
+int s = extract32(val, 14, 2);
+int i = (-(val & 1) << 13) | extract32(val, 1, 13);
+
+if (ctx->tb_flags & PSW_W) {
+i ^= s << 13;
+}
+return i;
+}
+
+/* The sp field is only present with !PSW_W. */
+static int sp0_if_wide(DisasContext *ctx, int sp)
+{
+return ctx->tb_flags & PSW_W ? 0 : sp;
+}
+
  /* Translate CMPI doubleword conditions to standard. */
  static int cmpbid_c(DisasContext *ctx, int val)
  {





Re: [PATCH v3] hw/scsi/lsi53c895a: add timer to scripts processing

2024-03-10 Thread Helge Deller

On 3/10/24 16:35, Michael Tokarev wrote:

04.03.2024 19:37, Sven Schnelle :

HP-UX 10.20 seems to make the lsi53c895a spinning on a memory location
under certain circumstances. As the SCSI controller and CPU are not
running at the same time this loop will never finish. After some
time, the check loop interrupts with a unexpected device disconnect.
This works, but is slow because the kernel resets the scsi controller.
Instead of signaling UDC, start a timer and exit the loop. Until the
timer fires, the CPU can process instructions which might change the
memory location.

The limit of instructions is also reduced because scripts running on
the SCSI processor are usually very short. This keeps the time until
the loop is exited short.


This one is a bit large(ish), yet, - is it okay for -stable?  From the
description it feels like it should be picked up, and it applies cleanly
to 7.2.x too (after picking up the forgotten 8b09b7fe47082c692
"hw/scsi/lsi53c895a: add missing decrement of reentrancy counter" there).


Please include it in stable.
HP-UX feels incredible sluggish and slow without it, and even with Linux
guests the disconnects can be seen sometimes.

Helge



Re: [PATCH 1/4] disas: introduce no_raw_bytes

2024-03-04 Thread Helge Deller

On 3/4/24 20:13, Alex Bennée wrote:

For plugins we don't expect the raw bytes in the disassembly. We
already deal with this by hand crafting our capstone call but for
other diassemblers we need a flag.

Signed-off-by: Alex Bennée 
---
  include/disas/dis-asm.h | 7 +++
  disas/disas.c   | 1 +
  2 files changed, 8 insertions(+)

diff --git a/include/disas/dis-asm.h b/include/disas/dis-asm.h
index 2324f6b1a46..5c32e7a310c 100644
--- a/include/disas/dis-asm.h
+++ b/include/disas/dis-asm.h
@@ -396,6 +396,13 @@ typedef struct disassemble_info {
/* Command line options specific to the target disassembler.  */
char * disassembler_options;

+  /*
+   * When true instruct the disassembler to not preface opcodes with
+   * raw bytes. This is mainly for the benefit of the plugin
+   * interface.
+   */
+  bool no_raw_bytes;


Patch in general and idea is OK, but I don't like
the "no_raw_bytes" naming very much.
In patch #2 you use "if (!info->no_raw_bytes) {.."
which is double-negation.

"hide_raw_bytes" is better but still double negation.

Maybe something like "show_opcodes" which defaults to "false"
when used with plugins is better?

Helge


+
/* Field intended to be used by targets in any way they deem suitable.  */
void *target_info;

diff --git a/disas/disas.c b/disas/disas.c
index 0d2d06c2ecc..feb5bc4b665 100644
--- a/disas/disas.c
+++ b/disas/disas.c
@@ -273,6 +273,7 @@ char *plugin_disas(CPUState *cpu, uint64_t addr, size_t 
size)
  s.info.buffer_vma = addr;
  s.info.buffer_length = size;
  s.info.print_address_func = plugin_print_address;
+s.info.no_raw_bytes = true;

  if (s.info.cap_arch >= 0 && cap_disas_plugin(, addr, size)) {
  ; /* done */





Re: [PATCH 3/3] target/hppa: Fix assemble_12a insns for wide mode

2024-03-02 Thread Helge Deller

On 3/3/24 03:19, Richard Henderson wrote:

Reported-by: Sven Schnelle 
Signed-off-by: Richard Henderson 


Tested-by: Helge Deller 


---
  target/hppa/insns.decode | 27 ---
  target/hppa/translate.c  | 17 +
  2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 9c6f92444c..5412ff9836 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -26,7 +26,7 @@

  %assemble_11a   4:12 0:1 !function=expand_11a
  %assemble_120:s1 2:1 3:10!function=expand_shl2
-%assemble_12a   0:s1 3:11!function=expand_shl2
+%assemble_12a   3:13 0:1 !function=expand_12a
  %assemble_160:16 !function=expand_16
  %assemble_170:s1 16:5 2:1 3:10   !function=expand_shl2
  %assemble_220:s1 16:10 2:1 3:10  !function=expand_shl2
@@ -314,8 +314,9 @@ fstd001011 . . .. . 1 -- 100 0 . .  
@fldstdi
  @ldstim14m  .. b:5 t:5   \
   sp=%assemble_sp disp=%assemble_16  \
  x=0 scale=0 m=%neg_to_m
-@ldstim12m  .. b:5 t:5 sp:2 ..  \
- disp=%assemble_12a x=0 scale=0 m=%pos_to_m
+@ldstim12m  .. b:5 t:5   \
+ sp=%assemble_sp disp=%assemble_12a \
+x=0 scale=0 m=%pos_to_m

  # LDB, LDH, LDW, LDWM
  ld  01 . . .. ..@ldstim14  size=0
@@ -331,15 +332,19 @@ st  011010 . . .. ..
@ldstim14  size=2
  st  011011 . . .. ..@ldstim14m size=2
  st  01 . . .. ...10.@ldstim12m size=2

-fldw010110 b:5 . sp:2 ..\
- disp=%assemble_12a t=%rm64 m=%a_to_m x=0 scale=0 size=2
-fldw010111 b:5 . sp:2 ...0..\
- disp=%assemble_12a t=%rm64 m=0 x=0 scale=0 size=2
+fldw010110 b:5 . \
+ disp=%assemble_12a sp=%assemble_sp \
+t=%rm64 m=%a_to_m x=0 scale=0 size=2
+fldw010111 b:5 . .0..\
+ disp=%assemble_12a sp=%assemble_sp \
+t=%rm64 m=0 x=0 scale=0 size=2

-fstw00 b:5 . sp:2 ..\
- disp=%assemble_12a t=%rm64 m=%a_to_m x=0 scale=0 size=2
-fstw01 b:5 . sp:2 ...0..\
- disp=%assemble_12a t=%rm64 m=0 x=0 scale=0 size=2
+fstw00 b:5 . \
+ disp=%assemble_12a sp=%assemble_sp \
+t=%rm64 m=%a_to_m x=0 scale=0 size=2
+fstw01 b:5 . .0..\
+ disp=%assemble_12a sp=%assemble_sp \
+t=%rm64 m=0 x=0 scale=0 size=2

  ld  010100 . . .. 0.@ldstim11
  fldd010100 . . .. 1.@ldstim11
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 6dcc74e681..1ef266c403 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -155,6 +155,23 @@ static int expand_11a(DisasContext *ctx, int val)
  return i;
  }

+/* Expander for assemble_16a(s,im11a,i). */
+static int expand_12a(DisasContext *ctx, int val)
+{
+/*
+ * @val is bit 0 and bits [3:15].
+ * Swizzle thing around depending on PSW.W.
+ */
+int im11a = extract32(val, 1, 11);
+int sp = extract32(val, 12, 2);
+int i = (-(val & 1) << 13) | (im11a << 2);
+
+if (ctx->tb_flags & PSW_W) {
+i ^= sp << 13;
+}
+return i;
+}
+
  /* Expander for assemble_16(s,im14). */
  static int expand_16(DisasContext *ctx, int val)
  {





Re: [PATCH 2/3] target/hppa: Fix assemble_11a insns for wide mode

2024-03-02 Thread Helge Deller

On 3/3/24 07:52, Helge Deller wrote:

On 3/3/24 03:19, Richard Henderson wrote:

Reported-by: Sven Schnelle 
Signed-off-by: Richard Henderson 
---
  target/hppa/insns.decode |  7 ---
  target/hppa/translate.c  | 23 +--
  2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 0d9f8159ec..9c6f92444c 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -24,7 +24,7 @@
  %assemble_sr3   13:1 14:2
  %assemble_sr3x  13:1 14:2 !function=expand_sr3x

-%assemble_11a   0:s1 4:10    !function=expand_shl3
+%assemble_11a   4:12 0:1 !function=expand_11a
  %assemble_12    0:s1 2:1 3:10    !function=expand_shl2
  %assemble_12a   0:s1 3:11    !function=expand_shl2
  %assemble_16    0:16 !function=expand_16
@@ -305,8 +305,9 @@ fstd    001011 . . .. . 1 -- 100 0 . .  
    @fldstdi
  # Offset Mem
  

-@ldstim11   .. b:5 t:5 sp:2 ..  \
-     disp=%assemble_11a m=%ma2_to_m x=0 scale=0 size=3
+@ldstim11   .. b:5 t:5   \
+     sp=%assemble_sp disp=%assemble_11a \
+    m=%ma2_to_m x=0 scale=0 size=3
  @ldstim14   .. b:5 t:5   \
   sp=%assemble_sp disp=%assemble_16  \
  x=0 scale=0 m=0
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 585d836959..6dcc74e681 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -121,12 +121,6 @@ static int expand_shl2(DisasContext *ctx, int val)
  return val << 2;
  }

-/* Used for fp memory ops.  */
-static int expand_shl3(DisasContext *ctx, int val)
-{
-    return val << 3;
-}
-
  /* Used for assemble_21.  */
  static int expand_shl11(DisasContext *ctx, int val)
  {
@@ -144,6 +138,23 @@ static int assemble_6(DisasContext *ctx, int val)
  return (val ^ 31) + 1;
  }

+/* Expander for assemble_16a(s,cat(im10a,0),i). */


Typo above, should be assemble_11a().


^^ Ignore that.

Helge

 

Otherwise:
Tested-by: Helge Deller 
Reviewed-by: Helge Deller 


+static int expand_11a(DisasContext *ctx, int val)
+{
+    /*
+ * @val is bit 0 and bits [4:15].
+ * Swizzle thing around depending on PSW.W.
+ */
+    int im10a = extract32(val, 1, 10);
+    int sp = extract32(val, 11, 2);
+    int i = (-(val & 1) << 13) | (im10a << 3);
+
+    if (ctx->tb_flags & PSW_W) {
+    i ^= sp << 13;
+    }
+    return i;
+}
+
  /* Expander for assemble_16(s,im14). */
  static int expand_16(DisasContext *ctx, int val)
  {






Re: [PATCH 2/3] target/hppa: Fix assemble_11a insns for wide mode

2024-03-02 Thread Helge Deller

On 3/3/24 03:19, Richard Henderson wrote:

Reported-by: Sven Schnelle 
Signed-off-by: Richard Henderson 
---
  target/hppa/insns.decode |  7 ---
  target/hppa/translate.c  | 23 +--
  2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 0d9f8159ec..9c6f92444c 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -24,7 +24,7 @@
  %assemble_sr3   13:1 14:2
  %assemble_sr3x  13:1 14:2 !function=expand_sr3x

-%assemble_11a   0:s1 4:10!function=expand_shl3
+%assemble_11a   4:12 0:1 !function=expand_11a
  %assemble_120:s1 2:1 3:10!function=expand_shl2
  %assemble_12a   0:s1 3:11!function=expand_shl2
  %assemble_160:16 !function=expand_16
@@ -305,8 +305,9 @@ fstd001011 . . .. . 1 -- 100 0 . .  
@fldstdi
  # Offset Mem
  

-@ldstim11   .. b:5 t:5 sp:2 ..  \
- disp=%assemble_11a m=%ma2_to_m x=0 scale=0 size=3
+@ldstim11   .. b:5 t:5   \
+ sp=%assemble_sp disp=%assemble_11a \
+m=%ma2_to_m x=0 scale=0 size=3
  @ldstim14   .. b:5 t:5   \
   sp=%assemble_sp disp=%assemble_16  \
  x=0 scale=0 m=0
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 585d836959..6dcc74e681 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -121,12 +121,6 @@ static int expand_shl2(DisasContext *ctx, int val)
  return val << 2;
  }

-/* Used for fp memory ops.  */
-static int expand_shl3(DisasContext *ctx, int val)
-{
-return val << 3;
-}
-
  /* Used for assemble_21.  */
  static int expand_shl11(DisasContext *ctx, int val)
  {
@@ -144,6 +138,23 @@ static int assemble_6(DisasContext *ctx, int val)
  return (val ^ 31) + 1;
  }

+/* Expander for assemble_16a(s,cat(im10a,0),i). */


Typo above, should be assemble_11a().

Otherwise:
Tested-by: Helge Deller 
Reviewed-by: Helge Deller 


+static int expand_11a(DisasContext *ctx, int val)
+{
+/*
+ * @val is bit 0 and bits [4:15].
+ * Swizzle thing around depending on PSW.W.
+ */
+int im10a = extract32(val, 1, 10);
+int sp = extract32(val, 11, 2);
+int i = (-(val & 1) << 13) | (im10a << 3);
+
+if (ctx->tb_flags & PSW_W) {
+i ^= sp << 13;
+}
+return i;
+}
+
  /* Expander for assemble_16(s,im14). */
  static int expand_16(DisasContext *ctx, int val)
  {





Re: [PATCH 1/3] target/hppa: Fix assemble_16 insns for wide mode

2024-03-02 Thread Helge Deller

On 3/3/24 03:19, Richard Henderson wrote:

Reported-by: Sven Schnelle 
Signed-off-by: Richard Henderson 
---
  target/hppa/insns.decode | 15 +--
  target/hppa/translate.c  | 21 +
  2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index f5a3f02fd1..0d9f8159ec 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -27,13 +27,14 @@
  %assemble_11a   0:s1 4:10!function=expand_shl3
  %assemble_120:s1 2:1 3:10!function=expand_shl2
  %assemble_12a   0:s1 3:11!function=expand_shl2
+%assemble_160:16 !function=expand_16
  %assemble_170:s1 16:5 2:1 3:10   !function=expand_shl2
  %assemble_220:s1 16:10 2:1 3:10  !function=expand_shl2
+%assemble_sp14:2 !function=sp0_if_wide

  %assemble_210:s1 1:11 14:2 16:5 12:2  !function=expand_shl11

  %lowsign_11 0:s1 1:10
-%lowsign_14 0:s1 1:13

  %sm_imm 16:10 !function=expand_sm_imm

@@ -221,7 +222,7 @@ sub_b_tsv   10 . .  110100 . .  
@rrr_cf_d

  ldil001000 t:5 .i=%assemble_21
  addil   001010 r:5 .i=%assemble_21
-ldo 001101 b:5 t:5 -- ..i=%lowsign_14
+ldo 001101 b:5 t:5  i=%assemble_16

  addi101101 . .  0 ...   @rri_cf
  addi_tsv101101 . .  1 ...   @rri_cf
@@ -306,10 +307,12 @@ fstd001011 . . .. . 1 -- 100 0 . 
.  @fldstdi

  @ldstim11   .. b:5 t:5 sp:2 ..  \
   disp=%assemble_11a m=%ma2_to_m x=0 scale=0 size=3
-@ldstim14   .. b:5 t:5 sp:2 ..  \
- disp=%lowsign_14 x=0 scale=0 m=0
-@ldstim14m  .. b:5 t:5 sp:2 ..  \
- disp=%lowsign_14 x=0 scale=0 m=%neg_to_m
+@ldstim14   .. b:5 t:5   \
+ sp=%assemble_sp disp=%assemble_16  \
+x=0 scale=0 m=0
+@ldstim14m  .. b:5 t:5   \
+ sp=%assemble_sp disp=%assemble_16  \
+x=0 scale=0 m=%neg_to_m
  @ldstim12m  .. b:5 t:5 sp:2 ..  \
   disp=%assemble_12a x=0 scale=0 m=%pos_to_m

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 01f3188656..585d836959 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -144,6 +144,27 @@ static int assemble_6(DisasContext *ctx, int val)
  return (val ^ 31) + 1;
  }

+/* Expander for assemble_16(s,im14). */
+static int expand_16(DisasContext *ctx, int val)
+{
+/*
+ * @val is bits [0:15], containing both im14 and s.
+ * Swizzle thing around depending on PSW.W.
+ */
+int i = (-(val & 1) << 13) | extract32(val, 1, 13);
+
+if (ctx->tb_flags & PSW_W) {
+i ^= val & (3 << 13);


Patch boots when I change to:
+i ^= ((val >> 14) & 3) << 13;

Helge


+}
+return i;
+}
+
+/* The sp field is only present with !PSW_W. */
+static int sp0_if_wide(DisasContext *ctx, int sp)
+{
+return ctx->tb_flags & PSW_W ? 0 : sp;
+}
+
  /* Translate CMPI doubleword conditions to standard. */
  static int cmpbid_c(DisasContext *ctx, int val)
  {





Re: [PATCH 1/5] target/hppa: Fix unaligned double word accesses for hppa64

2024-03-02 Thread Helge Deller

On 3/3/24 03:10, Richard Henderson wrote:

On 3/2/24 12:35, del...@kernel.org wrote:

From: Guenter Roeck 

Unaligned 64-bit accesses were found in Linux to clobber carry bits,
resulting in bad results if an arithmetic operation involving a
carry bit was executed after an unaligned 64-bit operation.

hppa 2.0 defines additional carry bits in PSW register bits 32..39.
When restoring PSW after executing an unaligned instruction trap,
those bits were not cleared and ended up to be active all the time.
Clearing bit 32..39 in psw prior to restoring it solves the problem.

Fixes: 931adff31478 ("target/hppa: Update cpu_hppa_get/put_psw for hppa64")
Cc: Richard Henderson 
Cc: Charlie Jenkins 
Cc: Helge Deller 
Signed-off-by: Guenter Roeck 
Reviewed-by: Richard Henderson 
---
  target/hppa/helper.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/hppa/helper.c b/target/hppa/helper.c
index 859644c47a..7b798d1227 100644
--- a/target/hppa/helper.c
+++ b/target/hppa/helper.c
@@ -76,7 +76,12 @@ void cpu_hppa_put_psw(CPUHPPAState *env, target_ulong psw)
  }
  psw &= ~reserved;
-    env->psw = psw & ~(PSW_N | PSW_V | PSW_CB);
+    if (hppa_is_pa20(env)) {
+    env->psw = psw & ~(PSW_N | PSW_V | PSW_CB | 0xffull);
+    } else {
+    env->psw = psw & ~(PSW_N | PSW_V | PSW_CB);
+    }


https://patchew.org/QEMU/20240217015811.1975411-1-li...@roeck-us.net/
was the better version.


Oh, yes. Will use that one in the pull request.
Thanks!
Helge




Re: [PATCH] hw/scsi/lsi53c895a: stop script on phase mismatch

2024-03-02 Thread Helge Deller

On 3/2/24 22:44, Sven Schnelle wrote:

Netbsd isn't happy with qemu lsi53c895a emulation:

cd0(esiop0:0:2:0): command with tag id 0 reset
esiop0: autoconfiguration error: phase mismatch without command
esiop0: autoconfiguration error: unhandled scsi interrupt, sist=0x80 sstat1=0x0 
DSA=0x23a64b1 DSP=0x50

This is because lsi_bad_phase() triggers a phase mismatch, which
stops SCRIPT processing. However, after returning to
lsi_command_complete(), SCRIPT is restarted with lsi_resume_script().
Fix this by adding a return value to lsi_bad_phase(), and only resume
script processing when lsi_bad_phase() didn't trigger a host interrupt.

Signed-off-by: Sven Schnelle 


Tested-by: Helge Deller 

Helge


---
  hw/scsi/lsi53c895a.c | 16 
  1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 4ff9470381..59b88aff3f 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -573,8 +573,9 @@ static inline void lsi_set_phase(LSIState *s, int phase)
  s->sstat1 = (s->sstat1 & ~PHASE_MASK) | phase;
  }

-static void lsi_bad_phase(LSIState *s, int out, int new_phase)
+static int lsi_bad_phase(LSIState *s, int out, int new_phase)
  {
+int ret = 0;
  /* Trigger a phase mismatch.  */
  if (s->ccntl0 & LSI_CCNTL0_ENPMJ) {
  if ((s->ccntl0 & LSI_CCNTL0_PMJCTL)) {
@@ -587,8 +588,10 @@ static void lsi_bad_phase(LSIState *s, int out, int 
new_phase)
  trace_lsi_bad_phase_interrupt();
  lsi_script_scsi_interrupt(s, LSI_SIST0_MA, 0);
  lsi_stop_script(s);
+ret = 1;
  }
  lsi_set_phase(s, new_phase);
+return ret;
  }


@@ -792,7 +795,7 @@ static int lsi_queue_req(LSIState *s, SCSIRequest *req, 
uint32_t len)
  static void lsi_command_complete(SCSIRequest *req, size_t resid)
  {
  LSIState *s = LSI53C895A(req->bus->qbus.parent);
-int out;
+int out, stop = 0;

  out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
  trace_lsi_command_complete(req->status);
@@ -800,7 +803,10 @@ static void lsi_command_complete(SCSIRequest *req, size_t 
resid)
  s->command_complete = 2;
  if (s->waiting && s->dbc != 0) {
  /* Raise phase mismatch for short transfers.  */
-lsi_bad_phase(s, out, PHASE_ST);
+stop = lsi_bad_phase(s, out, PHASE_ST);
+if (stop) {
+s->waiting = 0;
+}
  } else {
  lsi_set_phase(s, PHASE_ST);
  }
@@ -810,7 +816,9 @@ static void lsi_command_complete(SCSIRequest *req, size_t 
resid)
  lsi_request_free(s, s->current);
  scsi_req_unref(req);
  }
-lsi_resume_script(s);
+if (!stop) {
+lsi_resume_script(s);
+}
  }

   /* Callback to indicate that the SCSI layer has completed a transfer.  */





Re: [RFC PATCH] disas/hppa: drop raw opcode dump

2024-02-29 Thread Helge Deller

On 2/29/24 15:05, Alex Bennée wrote:

The hppa disassembly is different from the others due to leading with
the raw opcode data. This confuses plugins looking for instruction
prefixes to match instructions. For plugins like execlog there is
another mechanism for getting the instruction byte data.

For the sake of consistently just present the instruction assembly
code.

Signed-off-by: Alex Bennée 


This effectively reverts commit 2f926bfd5b79e6219ae65a1e530b38f37d62b384
("disas/hppa: Show hexcode of instruction along with disassembly").

Sad, but Ok.

Acked-by: Helge Deller 



---
  disas/hppa.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/disas/hppa.c b/disas/hppa.c
index 22dce9b41bb..dd34cce211b 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -1972,10 +1972,6 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)

insn = bfd_getb32 (buffer);

-  info->fprintf_func(info->stream, " %02x %02x %02x %02x   ",
-(insn >> 24) & 0xff, (insn >> 16) & 0xff,
-(insn >>  8) & 0xff, insn & 0xff);
-
for (i = 0; i < NUMOPCODES; ++i)
  {
const struct pa_opcode *opcode = _opcodes[i];





Re: [PATCH] target: hppa: Fix unaligned double word accesses for hppa64

2024-02-15 Thread Helge Deller

On 2/16/24 06:58, Charlie Jenkins wrote:

On Thu, Feb 15, 2024 at 09:34:15PM -0800, Guenter Roeck wrote:

Unaligned 64-bit accesses were found in Linux to clobber carry bits,
resulting in bad results if an arithmetic operation involving a
carry bit was executed after an unaligned 64-bit operation.

hppa 2.0 defines additional carry bits in PSW register bits 32..39.
When restoring PSW after executing an unaligned instruction trap,
those bits were not cleared and ended up to be active all the time.
Clearing bit 32..39 in psw prior to restoring it solves the problem.

Fixes: 931adff31478 ("target/hppa: Update cpu_hppa_get/put_psw for hppa64")
Cc: Richard Henderson 
Cc: Charlie Jenkins 
Cc: Helge Deller 
Signed-off-by: Guenter Roeck 
---
  target/hppa/helper.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/hppa/helper.c b/target/hppa/helper.c
index 859644c47a..7b798d1227 100644
--- a/target/hppa/helper.c
+++ b/target/hppa/helper.c
@@ -76,7 +76,12 @@ void cpu_hppa_put_psw(CPUHPPAState *env, target_ulong psw)
  }
  psw &= ~reserved;

-env->psw = psw & ~(PSW_N | PSW_V | PSW_CB);
+if (hppa_is_pa20(env)) {
+env->psw = psw & ~(PSW_N | PSW_V | PSW_CB | 0xffull);


I thought there was something fishy in this function but was slow on the
uptake...

How about defining a new macro (PSW_CB_HIGH) to hold this value?


...and avoid the hppa_is_pa20() by using PSW_CB_HIGH unconditionally
on 32-bit too (which then gets optimized-out by the compiler).

Nice catch btw!
I wonder if this finally fixes 64-bit Linux kernels on qemu-hppa20?

Helge


- Charlie


+} else {
+env->psw = psw & ~(PSW_N | PSW_V | PSW_CB);
+}
+
  env->psw_n = (psw / PSW_N) & 1;
  env->psw_v = -((psw / PSW_V) & 1);

--
2.39.2






Re: [PULL 00/12] Hppa64 patches

2024-02-14 Thread Helge Deller

On 2/14/24 10:07, Michael Tokarev wrote:

13.02.2024 22:16, Helge Deller:

On 2/13/24 19:09, Michael Tokarev wrote:

13.02.2024 02:47, del...@kernel.org:



Helge Deller (11):
   disas/hppa: Add disassembly for qemu specific instructions
   target/hppa: Add "diag 0x101" for console output support
   hw/pci-host/astro: Avoid aborting on access failure
   hw/pci-host/astro: Implement Hard Fail and Soft Fail mode
   lasi: allow access to LAN MAC address registers
   target/hppa: Implement do_transaction_failed handler for I/O errors
   lasi: Add reset I/O ports for LASI audio and FDC
   target/hppa: Allow read-access to PSW with rsm 0,reg instruction
   target/hppa: PDC_BTLB_INFO uses 32-bit ints
   target/hppa: Update SeaBIOS-hppa to version 16
   hw/hppa/machine: Load 64-bit firmware on 64-bit machines

Sven Schnelle (1):
   hw/net/tulip: add chip status register values


Is there anything in there which is relevant for -stable?


Ideally all patches.
At minimum the tulip patch.


Heh.

Ideally there's no bugs and everyone knows everything ;)
In reality though, you know this area *much* better than
me, and I rely on your judgement here.

Still the whole lot seems a bit too much here, while some fixes
in seabios-hppa (so whole new release of it, maybe without the
separate 64bit firmware variant) looks worth to have.

It's a mix of features and bugfixes, and some features (like
this tulip thing for example) actually *are* bugfixes :)


I suggest to just downport the tulip patch.
32-bit currently works in stable series as-is.

Most current fixes in SeaBIOS and hppa code are to fix & enable the 64-bit
Linux kernel & HP-UX. I don't think we want to completely back port those,
as they aren't yet fully functional either. So, this 64-bit is stuff which
should be limited to upcoming versions.

In case we want the newer SeaBIOS in stable, I suggest to additionally downport:
- target/hppa: Add "diag 0x101" for console output support
- target/hppa: Update SeaBIOS-hppa to version 16 (minus the 64-bit binary)

Helge



Re: [PULL 00/12] Hppa64 patches

2024-02-13 Thread Helge Deller

On 2/13/24 19:09, Michael Tokarev wrote:

13.02.2024 02:47, del...@kernel.org пишет:

From: Helge Deller 

The following changes since commit 39a6e4f87e7b75a45b08d6dc8b8b7c2954c87440:

   Merge tag 'pull-qapi-2024-02-03' of https://repo.or.cz/qemu/armbru into 
staging (2024-02-03 13:31:58 +)

are available in the Git repository at:

   https://github.com/hdeller/qemu-hppa.git tags/hppa64-pull-request

for you to fetch changes up to a9314795f068515ff5925d0f68adf0a3215f6d2d:

   hw/hppa/machine: Load 64-bit firmware on 64-bit machines (2024-02-13 
00:44:06 +0100)


target/hppa: Enhancements and fixes

Some enhancements and fixes for the hppa target.

The major change is, that this patchset adds a new SeaBIOS-hppa firmware
which is built as 32- and 64-bit firmware.
The new 64-bit firmware is necessary to fully support 64-bit operating systems
(HP-UX, Linux, NetBSD,...).



Helge Deller (11):
   disas/hppa: Add disassembly for qemu specific instructions
   target/hppa: Add "diag 0x101" for console output support
   hw/pci-host/astro: Avoid aborting on access failure
   hw/pci-host/astro: Implement Hard Fail and Soft Fail mode
   lasi: allow access to LAN MAC address registers
   target/hppa: Implement do_transaction_failed handler for I/O errors
   lasi: Add reset I/O ports for LASI audio and FDC
   target/hppa: Allow read-access to PSW with rsm 0,reg instruction
   target/hppa: PDC_BTLB_INFO uses 32-bit ints
   target/hppa: Update SeaBIOS-hppa to version 16
   hw/hppa/machine: Load 64-bit firmware on 64-bit machines

Sven Schnelle (1):
   hw/net/tulip: add chip status register values


Is there anything in there which is relevant for -stable?


Ideally all patches.
At minimum the tulip patch.

Helge




Re: [PULL 00/12] Hppa64 patches

2024-02-13 Thread Helge Deller

On 2/13/24 10:10, Peter Maydell wrote:

On Mon, 12 Feb 2024 at 23:04, Helge Deller  wrote:


On 2/12/24 22:16, Peter Maydell wrote:

This fails "make check", eg:
https://gitlab.com/qemu-project/qemu/-/jobs/6154451100

because when the qom-test etc tests run qemu-system-hppa, it
barfs with "qemu-system-hppa: no firmware provided".

That kind of firmware check needs to not fire when
using the qtest accel.


Ok. But how do people usually work around this kind of issue?
Test if the qtest accel is in use?
Ignore if the firmware can't be loaded?
Any hint would be great!


There's a qtest_enabled() function -- see eg hw/mips/malta.c
for an example of skipping the "fail on no firmware" check
when it's enabled. (There are a bunch of others in the tree too.)


Yes, I found that
I've used that function in the latest pull request which I sent...
Thanks!
Helge



Re: [PULL 11/12] target/hppa: Update SeaBIOS-hppa to version 16

2024-02-12 Thread Helge Deller

On 2/11/24 19:49, Michael Tokarev wrote:

11.02.2024 15:29, del...@kernel.org

From: Helge Deller 

SeaBIOS-hppa version 16 news & enhancements:



  pc-bios/hppa-firmware.img   | Bin 163324 -> 167820 bytes
  pc-bios/hppa-firmware64.img | Bin 0 -> 206024 bytes
  roms/seabios-hppa   |   2 +-
  3 files changed, 1 insertion(+), 1 deletion(-)
  mode change 100644 => 100755 pc-bios/hppa-firmware.img
  create mode 100755 pc-bios/hppa-firmware64.img


Can we have build instructions for these files in roms/Makefile please?


Sure, I'll add then in the next round when I send a v17 firmware.

Helge




Re: [PULL 00/12] Hppa64 patches

2024-02-12 Thread Helge Deller

On 2/12/24 22:16, Peter Maydell wrote:

On Sun, 11 Feb 2024 at 12:30,  wrote:


From: Helge Deller 

The following changes since commit 39a6e4f87e7b75a45b08d6dc8b8b7c2954c87440:

   Merge tag 'pull-qapi-2024-02-03' of https://repo.or.cz/qemu/armbru into 
staging (2024-02-03 13:31:58 +)

are available in the Git repository at:

   https://github.com/hdeller/qemu-hppa.git tags/hppa64-pull-request

for you to fetch changes up to f9d2270c85872bd71a01e15b2ebda2569f17f811:

   hw/hppa/machine: Load 64-bit firmware on 64-bit machines (2024-02-11 
13:25:15 +0100)


target/hppa: Enhancements and fixes

A new SeaBIOS-hppa firmware which is built as 32- and 64-bit firmware.
Necessary to fully support 64-bit operating systems (HP-UX, Linux, NetBSD,...).




This fails "make check", eg:
https://gitlab.com/qemu-project/qemu/-/jobs/6154451100

because when the qom-test etc tests run qemu-system-hppa, it
barfs with "qemu-system-hppa: no firmware provided".

That kind of firmware check needs to not fire when
using the qtest accel.


Ok. But how do people usually work around this kind of issue?
Test if the qtest accel is in use?
Ignore if the firmware can't be loaded?
Any hint would be great!

Helge



Re: [PATCH] hw/hppa/Kconfig: Fix building with "configure --without-default-devices"

2024-02-09 Thread Helge Deller

On 2/9/24 19:55, Thomas Huth wrote:

When running "configure" with "--without-default-devices", building
of qemu-system-hppa currently fails with:

  /usr/bin/ld: libqemu-hppa-softmmu.fa.p/hw_hppa_machine.c.o: in function 
`machine_HP_common_init_tail':
  hw/hppa/machine.c:399: undefined reference to `usb_bus_find'
  /usr/bin/ld: hw/hppa/machine.c:399: undefined reference to `usb_create_simple'
  /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to `usb_bus_find'
  /usr/bin/ld: hw/hppa/machine.c:400: undefined reference to `usb_create_simple'
  collect2: error: ld returned 1 exit status
  ninja: build stopped: subcommand failed.
  make: *** [Makefile:162: run-ninja] Error 1

And after fixing this, the qemu-system-hppa binary refuses to run
due to the missing 'pci-ohci' and 'pci-serial' devices. Let's add
the right config switches to fix these problems.

Signed-off-by: Thomas Huth 
---
  hw/hppa/Kconfig | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
index ff8528aaa8..124d5e9e47 100644
--- a/hw/hppa/Kconfig
+++ b/hw/hppa/Kconfig
@@ -6,7 +6,7 @@ config HPPA_B160L
  select ASTRO
  select DINO
  select LASI
-select SERIAL
+select SERIAL_PCI


I think the "SERIAL" is needed too for the B160L machine.
Other than that,

Acked-by: Helge Deller 

Thank you!
Helge


  select ISA_BUS
  select I8259
  select IDE_CMD646
@@ -16,3 +16,4 @@ config HPPA_B160L
  select LASIPS2
  select PARALLEL
  select ARTIST
+select USB_OHCI_PCI





Re: [PATCH] hw/net/tulip: add chip status register values

2024-02-09 Thread Helge Deller

On 2/6/24 23:33, Helge Deller wrote:

On 2/5/24 20:47, Sven Schnelle wrote:

Netbsd isn't able to detect a link on the emulated tulip card. That's
because netbsd reads the Chip Status Register of the Phy (address
0x14). The default phy data in the qemu tulip driver is all zero,
which means no link is established and autonegotation isn't complete.

Therefore set the register to 0x3b40, which means:

Link is up, Autonegotation complete, Full Duplex, 100MBit/s Link
speed.

Also clear the mask because this register is read only.

Signed-off-by: Sven Schnelle 


Reviewed-by: Helge Deller 
Tested-by: Helge Deller 


Unless someone complains, I'll include this patch
in the upcoming hppa pull request...

Helge



Re: [PATCH 03/13] target/hppa: Fix PSW_W and PSW_E bits in rsm, ssm and mtsm

2024-02-09 Thread Helge Deller

On 2/8/24 21:43, Richard Henderson wrote:

On 2/7/24 08:20, del...@kernel.org wrote:

  #define PSW_E    0x0400
+#define PSW_E_BIT    37 /* PA2.0 only */
  #define PSW_W    0x0800 /* PA2.0 only */
+#define PSW_W_BIT    36 /* PA2.0 only */

...

+target_ulong HELPER(get_system_mask)(CPUHPPAState *env)
+{
+    target_ulong psw = env->psw;
+
+    /* mask out invalid bits */
+    target_ulong psw_new = psw & PSW_SM;
+
+    /* ssm/rsm instructions number PSW_W and PSW_E differently */
+    psw_new &= ~PSW_W;
+    if (psw & PSW_W) {
+    psw_new |= 1ull << (63 - PSW_W_BIT);
+    }


Um, this has changed nothing, since 1 << (63 - 36) == 0x800 == PSW_W.


Yep.
I seem to have mixed strange things when writing that.
I've dropped the patch for now.

Thanks!
Helge



The conversion of PSW_SM_W to PSW_W happens in expand_sm_imm().
There's a comment there about keeping unimplemented bits disabled, including 
PSW_E. Perhaps this is the wrong layer in which to do this?

In any case, what is the actual problem that you're seeing?  Because it *isn't* 
that we were not considering the different placement of the bits, as your 
commit message suggests.


diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 53ec57ee86..10fdc0813d 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2163,13 +2163,20 @@ static bool trans_rsm(DisasContext *ctx, arg_rsm *a)
  nullify_over(ctx);
  tmp = tcg_temp_new_i64();
-    tcg_gen_ld_i64(tmp, tcg_env, offsetof(CPUHPPAState, psw));
-    tcg_gen_andi_i64(tmp, tmp, ~a->i);
-    gen_helper_swap_system_mask(tmp, tcg_env, tmp);
-    save_gpr(ctx, a->t, tmp);
+    if (a->t != 0) {
+    gen_helper_get_system_mask(tmp, tcg_env);
+    save_gpr(ctx, a->t, tmp);
+    }


If a->t == 0, tmp is uninitialized...


+
+    if (a->i) {
+    tcg_gen_ld_i64(tmp, tcg_env, offsetof(CPUHPPAState, psw));


... but read here.


@@ -2183,11 +2190,17 @@ static bool trans_ssm(DisasContext *ctx, arg_ssm *a)
  nullify_over(ctx);
  tmp = tcg_temp_new_i64();
-    tcg_gen_ld_i64(tmp, tcg_env, offsetof(CPUHPPAState, psw));
-    tcg_gen_ori_i64(tmp, tmp, a->i);
-    gen_helper_swap_system_mask(tmp, tcg_env, tmp);
-    save_gpr(ctx, a->t, tmp);
+    if (a->t != 0) {
+    gen_helper_get_system_mask(tmp, tcg_env);
+    save_gpr(ctx, a->t, tmp);
+    }
+
+    if (a->i) {
+    tcg_gen_ld_i64(tmp, tcg_env, offsetof(CPUHPPAState, psw));
+    tcg_gen_ori_i64(tmp, tmp, a->i);


Likewise.


r~





Re: [PATCH] hw/net/tulip: add chip status register values

2024-02-06 Thread Helge Deller

On 2/5/24 20:47, Sven Schnelle wrote:

Netbsd isn't able to detect a link on the emulated tulip card. That's
because netbsd reads the Chip Status Register of the Phy (address
0x14). The default phy data in the qemu tulip driver is all zero,
which means no link is established and autonegotation isn't complete.

Therefore set the register to 0x3b40, which means:

Link is up, Autonegotation complete, Full Duplex, 100MBit/s Link
speed.

Also clear the mask because this register is read only.

Signed-off-by: Sven Schnelle 


Reviewed-by: Helge Deller 
Tested-by: Helge Deller 

Can be easily tested without installation:
Download: wget 
https://cdn.netbsd.org/pub/NetBSD/NetBSD-9.3/iso/NetBSD-9.3-hppa.iso
Run: ./qemu-system-hppa -cdrom NetBSD-9.3-hppa.iso -nographic
-> a) Installation on English
-> e) Utility Menu
-> c) configure network

Helge


---
  hw/net/tulip.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 6d4fb06dad..1f2ef20977 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -421,7 +421,7 @@ static uint16_t tulip_mdi_default[] = {
  /* MDI Registers 8 - 15 */
  0x, 0x, 0x, 0x, 0x, 0x, 0x, 0x,
  /* MDI Registers 16 - 31 */
-0x0003, 0x, 0x0001, 0x, 0x, 0x, 0x, 0x,
+0x0003, 0x, 0x0001, 0x, 0x3b40, 0x, 0x, 0x,
  0x, 0x, 0x, 0x, 0x, 0x, 0x, 0x,
  };

@@ -429,7 +429,7 @@ static uint16_t tulip_mdi_default[] = {
  static const uint16_t tulip_mdi_mask[] = {
  0x, 0x, 0x, 0x, 0xc01f, 0x, 0x, 0x,
  0x, 0x, 0x, 0x, 0x, 0x, 0x, 0x,
-0x0fff, 0x, 0x, 0x, 0x, 0x, 0x, 0x,
+0x0fff, 0x, 0x, 0x, 0x, 0x, 0x, 0x,
  0x, 0x, 0x, 0x, 0x, 0x, 0x, 0x,
  };






Re: [PULL 02/10] hw/hppa/machine: Disable default devices with --nodefaults option

2024-02-02 Thread Helge Deller

On 2/2/24 19:04, Guenter Roeck wrote:

On Fri, Feb 02, 2024 at 10:54:20AM +0100, Helge Deller wrote:

Hi Guenter,

On 2/2/24 05:22, Guenter Roeck wrote:

On Sat, Jan 13, 2024 at 06:57:20AM +0100, del...@kernel.org wrote:

From: Helge Deller 

Recognize the qemu --nodefaults option, which will disable the
following default devices on hppa:
- lsi53c895a SCSI controller,
- artist graphics card,
- LASI 82596 NIC,
- tulip PCI NIC,
- second serial PCI card,
- USB OHCI controller.

Adding this option is very useful to allow manual testing and
debugging of the other possible devices on the command line.



With this patch in the tree, I get some interesting crashes in Seabios
if I provide a somewhat unusual command line option. For example,
something like

  -usb -device usb-ehci,id=ehci \
  -device usb-uas,bus=ehci.0,id=uas \
  -device scsi-hd,bus=uas.0,scsi-id=0,lun=0,drive=d0 \
  -drive file= ...

is accepted as command line option but results in

SeaBIOS PA-RISC 32-bit Firmware Version 15 (QEMU 8.2.1)
Duplex Console IO Dependent Code (IODC) revision 1
--
(c) Copyright 2017-2024 Helge Deller  and SeaBIOS developers.
--
Processor   SpeedState   Coprocessor State  Cache Size
-     -  -  --
0  250 MHzActive Functional0 KB
1  250 MHzIdle   Functional0 KB
2  250 MHzIdle   Functional0 KB
3  250 MHzIdle   Functional0 KB
Emulated machine: HP C3700 (64-bit PA2.0) with 32-bit PDC
Available memory: 1024 MB
Good memory required: 16 MB
Primary boot path:FWSCSI.0.0
Alternate boot path:  FWSCSI.0.0
Console path: SERIAL_2.9600.8.none
Keyboard path:SERIAL_2.9600.8.none
*ERROR* in SeaBIOS-hppa-v15:
prepare_boot_path:2898
SeaBIOS wants SYSTEM HALT.

This is without --nodefaults, and it used to work. Is that intentional ?


This should now be fixed in the upcoming SeaBIOS-hppa-v16 version ("devel" 
branch):
https://github.com/hdeller/seabios-hppa/tree/devel
Could you test?


I was able to build from the 'master' branch, but 'devel' gives me

hppa64-linux-ld: target elf32-hppa-linux not found


The devel branch now includes a 64-bit firmware too.
You need both, hppa (32-bit) and hppa64 (64-bit) gcc
and binutils packages installed.


Do you have a binary seabios image, by any chance ?


http://www.dellerweb.de/temp/hppa-firmware.img


If it doesn't work, please give me the full command line.



qemu-system-hppa -M C3700 -smp 4 \
-kernel vmlinux -no-reboot -snapshot \
-usb -device usb-ehci,id=ehci \
-device usb-uas,bus=ehci.0,id=uas \
-device scsi-hd,bus=uas.0,scsi-id=0,lun=0,drive=d0 \
-drive file=rootfs.ext2,if=none,format=raw,id=d0 \
-append "root=/dev/sda rootwait console=ttyS0,115200" \
-nographic -monitor null


That line boots for me now.


This is with qemu 8.2.1. Note that the number of CPUs doesn't make a
difference. It turns out this also crashes/aborts immediately with
"nodefaults".


Adding "--nodefaults -serial mon:stdio" to the line above works too.


If I do use the --nodefaults parameter, I was unable to figure out how
to configure the serial console. What command line parameter(s) do I need to
get it ?


You need to add:
-serial mon:stdio
This will create a serial port if it's not yet there.



And there was me trying all variants of "-device pci-serial-4x..." I could
think of ;-).


:-)

Helge




Re: [PULL 02/10] hw/hppa/machine: Disable default devices with --nodefaults option

2024-02-02 Thread Helge Deller

Hi Guenter,

On 2/2/24 05:22, Guenter Roeck wrote:

On Sat, Jan 13, 2024 at 06:57:20AM +0100, del...@kernel.org wrote:

From: Helge Deller 

Recognize the qemu --nodefaults option, which will disable the
following default devices on hppa:
- lsi53c895a SCSI controller,
- artist graphics card,
- LASI 82596 NIC,
- tulip PCI NIC,
- second serial PCI card,
- USB OHCI controller.

Adding this option is very useful to allow manual testing and
debugging of the other possible devices on the command line.



With this patch in the tree, I get some interesting crashes in Seabios
if I provide a somewhat unusual command line option. For example,
something like

 -usb -device usb-ehci,id=ehci \
 -device usb-uas,bus=ehci.0,id=uas \
 -device scsi-hd,bus=uas.0,scsi-id=0,lun=0,drive=d0 \
 -drive file= ...

is accepted as command line option but results in

SeaBIOS PA-RISC 32-bit Firmware Version 15 (QEMU 8.2.1)
Duplex Console IO Dependent Code (IODC) revision 1
--
   (c) Copyright 2017-2024 Helge Deller  and SeaBIOS developers.
--
   Processor   SpeedState   Coprocessor State  Cache Size
   -     -  -  --
   0  250 MHzActive Functional0 KB
   1  250 MHzIdle   Functional0 KB
   2  250 MHzIdle   Functional0 KB
   3  250 MHzIdle   Functional0 KB
   Emulated machine: HP C3700 (64-bit PA2.0) with 32-bit PDC
   Available memory: 1024 MB
   Good memory required: 16 MB
   Primary boot path:FWSCSI.0.0
   Alternate boot path:  FWSCSI.0.0
   Console path: SERIAL_2.9600.8.none
   Keyboard path:SERIAL_2.9600.8.none
*ERROR* in SeaBIOS-hppa-v15:
prepare_boot_path:2898
SeaBIOS wants SYSTEM HALT.

This is without --nodefaults, and it used to work. Is that intentional ?


This should now be fixed in the upcoming SeaBIOS-hppa-v16 version ("devel" 
branch):
https://github.com/hdeller/seabios-hppa/tree/devel
Could you test?
If it doesn't work, please give me the full command line.


If I do use the --nodefaults parameter, I was unable to figure out how
to configure the serial console. What command line parameter(s) do I need to
get it ?


You need to add:
-serial mon:stdio
This will create a serial port if it's not yet there.

Thanks for your testing!

Helge



Re: [PATCH] linux-user: Make TARGET_NR_setgroups affect only the current thread

2024-01-30 Thread Helge Deller

On 1/31/24 01:18, Ilya Leoshkevich wrote:

Like TARGET_NR_setuid, TARGET_NR_setgroups should affect only the
calling thread, and not the entire process. Therefore, implement it
using a syscall, and not a libc call.

Cc: qemu-sta...@nongnu.org
Fixes: 19b84f3c35d7 ("added setgroups and getgroups syscalls")
Signed-off-by: Ilya Leoshkevich 


Patch seems ok, but just out of interest, how did you noticed?

Helge



---
  linux-user/syscall.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ff245dade51..da15d727e16 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7203,11 +7203,17 @@ static inline int tswapid(int id)
  #else
  #define __NR_sys_setresgid __NR_setresgid
  #endif
+#ifdef __NR_setgroups32
+#define __NR_sys_setgroups __NR_setgroups32
+#else
+#define __NR_sys_setgroups __NR_setgroups
+#endif

  _syscall1(int, sys_setuid, uid_t, uid)
  _syscall1(int, sys_setgid, gid_t, gid)
  _syscall3(int, sys_setresuid, uid_t, ruid, uid_t, euid, uid_t, suid)
  _syscall3(int, sys_setresgid, gid_t, rgid, gid_t, egid, gid_t, sgid)
+_syscall2(int, sys_setgroups, int, size, gid_t *, grouplist)

  void syscall_init(void)
  {
@@ -11772,7 +11778,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
num, abi_long arg1,
  unlock_user(target_grouplist, arg2,
  gidsetsize * sizeof(target_id));
  }
-return get_errno(setgroups(gidsetsize, grouplist));
+return get_errno(sys_setgroups(gidsetsize, grouplist));
  }
  case TARGET_NR_fchown:
  return get_errno(fchown(arg1, low2highuid(arg2), low2highgid(arg3)));
@@ -12108,7 +12114,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
num, abi_long arg1,
  }
  unlock_user(target_grouplist, arg2, 0);
  }
-return get_errno(setgroups(gidsetsize, grouplist));
+return get_errno(sys_setgroups(gidsetsize, grouplist));
  }
  #endif
  #ifdef TARGET_NR_fchown32





Re: [PATCH] hw/scsi/lsi53c895a: add missing decrement of reentrancy counter

2024-01-30 Thread Helge Deller

On 1/28/24 21:22, Sven Schnelle wrote:

When the maximum count of SCRIPTS instructions is reached, the code
stops execution and returns, but fails to decrement the reentrancy
counter. This effectively renders the SCSI controller unusable
because on next entry the reentrancy counter is still above the limit.

This bug was seen on HP-UX 10.20 which seems to trigger SCRIPTS
loops.

Fixes: b987718bbb ("hw/scsi/lsi53c895a: Fix reentrancy issues in the LSI controller 
(CVE-2023-0330)")
Signed-off-by: Sven Schnelle 


Tested-by: Helge Deller 

Thanks!
Helge


---
  hw/scsi/lsi53c895a.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 34e3b89287..d607a5f9fb 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1159,6 +1159,7 @@ again:
  lsi_script_scsi_interrupt(s, LSI_SIST0_UDC, 0);
  lsi_disconnect(s);
  trace_lsi_execute_script_stop();
+reentrancy_level--;
  return;
  }
  insn = read_dword(s, s->dsp);





Re: [PATCH 09/33] target/hppa: Populate CPUClass.mmu_index

2024-01-29 Thread Helge Deller

On 1/30/24 00:30, Richard Henderson wrote:

Signed-off-by: Richard Henderson 
---
  target/hppa/cpu.h |  7 ++-
  target/hppa/cpu.c | 12 
  2 files changed, 14 insertions(+), 5 deletions(-)


Reviewed-by: Helge Deller 





diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 6a153405d2..04439f247d 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -281,16 +281,13 @@ static inline int HPPA_BTLB_ENTRIES(CPUHPPAState *env)
  return hppa_is_pa20(env) ? 0 : PA10_BTLB_FIXED + PA10_BTLB_VARIABLE;
  }

+int hppa_cpu_mmu_index(CPUState *cs, bool ifetch);
  static inline int cpu_mmu_index(CPUHPPAState *env, bool ifetch)
  {
  #ifdef CONFIG_USER_ONLY
  return MMU_USER_IDX;
  #else
-if (env->psw & (ifetch ? PSW_C : PSW_D)) {
-return PRIV_P_TO_MMU_IDX(env->iaoq_f & 3, env->psw & PSW_P);
-}
-/* mmu disabled */
-return env->psw & PSW_W ? MMU_ABS_W_IDX : MMU_ABS_IDX;
+return hppa_cpu_mmu_index(env_cpu(env), ifetch);
  #endif
  }

diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 3c019855b4..fbb37e541e 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -94,6 +94,17 @@ static bool hppa_cpu_has_work(CPUState *cs)
  return cs->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI);
  }

+int hppa_cpu_mmu_index(CPUState *cs, bool ifetch)
+{
+CPUHPPAState *env = cpu_env(cs);
+
+if (env->psw & (ifetch ? PSW_C : PSW_D)) {
+return PRIV_P_TO_MMU_IDX(env->iaoq_f & 3, env->psw & PSW_P);
+}
+/* mmu disabled */
+return env->psw & PSW_W ? MMU_ABS_W_IDX : MMU_ABS_IDX;
+}
+
  static void hppa_cpu_disas_set_info(CPUState *cs, disassemble_info *info)
  {
  info->mach = bfd_mach_hppa20;
@@ -194,6 +205,7 @@ static void hppa_cpu_class_init(ObjectClass *oc, void *data)

  cc->class_by_name = hppa_cpu_class_by_name;
  cc->has_work = hppa_cpu_has_work;
+cc->mmu_index = hppa_cpu_mmu_index;
  cc->dump_state = hppa_cpu_dump_state;
  cc->set_pc = hppa_cpu_set_pc;
  cc->get_pc = hppa_cpu_get_pc;





  1   2   3   4   5   6   7   8   9   >