Re: [Qemu-ppc] [RFC NO-MERGE 06/12] target/ppc: Add new H-CALL shells for in memory table translation

[Suraj Jitindar Singh]

On Mon, 2017-02-20 at 12:18 +1100, David Gibson wrote:
> On Fri, Feb 17, 2017 at 04:08:06PM +1100, Suraj Jitindar Singh wrote:
> > 
> > The use of the new in memory tables introduced in ISAv3.00 for
> > translation,
> > also referred to as process tables, requires the introduction of 3
> > new
> > H-CALLs; H_REGISTER_PROCESS_TABLE, H_CLEAN_SLB, and
> > H_INVALIDATE_PID.
> > 
> > Add shells for each of these and register them as the hypercall
> > handlers.
> > Currently they all log an unimplemented hypercall and return
> > H_FUNCTION.
> > 
> > Signed-off-by: Suraj Jitindar Singh <address@hidden>
> > ---
> >  hw/ppc/spapr_hcall.c   | 31 +++++++++++++++++++++++++++++++
> >  include/hw/ppc/spapr.h |  3 +++
> >  2 files changed, 34 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 42d20e0..7afedbd 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -881,6 +881,32 @@ static target_ulong h_set_mode(PowerPCCPU
> > *cpu, sPAPRMachineState *spapr,
> >      return ret;
> >  }
> >  
> > +static target_ulong h_clean_slb(PowerPCCPU *cpu, sPAPRMachineState
> > *spapr,
> > +                                target_ulong opcode, target_ulong
> > *args)
> > +{
> > +    qemu_log_mask(LOG_UNIMP, "Unimplemented SPAPR hcall 0x"
> > TARGET_FMT_lx "%s\n",
> > +                  opcode, " (H_CLEAN_SLB)");
> qemu_log() isn't much used these days.  I'd suggest using trace
> points instead.

I use it... :)
This is how it's done currently but I'll take a look at other ways.

> 
> > 
> > +    return H_FUNCTION;
> > +}
> > +
> > +static target_ulong h_invalidate_pid(PowerPCCPU *cpu,
> > sPAPRMachineState *spapr,
> > +                                     target_ulong opcode,
> > target_ulong *args)
> > +{
> > +    qemu_log_mask(LOG_UNIMP, "Unimplemented SPAPR hcall 0x"
> > TARGET_FMT_lx "%s\n",
> > +                  opcode, " (H_INVALIDATE_PID)");
> > +    return H_FUNCTION;
> > +}
> > +
> > +static target_ulong h_register_process_table(PowerPCCPU *cpu,
> > +                                             sPAPRMachineState
> > *spapr,
> > +                                             target_ulong opcode,
> > +                                             target_ulong *args)
> > +{
> > +    qemu_log_mask(LOG_UNIMP, "Unimplemented SPAPR hcall 0x"
> > TARGET_FMT_lx "%s\n",
> > +                  opcode, " (H_REGISTER_PROC_TBL)");
> > +    return H_FUNCTION;
> > +}
> > +
> >  #define H_SIGNAL_SYS_RESET_ALL         -1
> >  #define H_SIGNAL_SYS_RESET_ALLBUTSELF  -2
> >  
> > @@ -1087,6 +1113,11 @@ static void hypercall_register_types(void)
> >      spapr_register_hypercall(H_PAGE_INIT, h_page_init);
> >      spapr_register_hypercall(H_SET_MODE, h_set_mode);
> >  
> > +    /* In Memory Table MMU h-calls */
> > +    spapr_register_hypercall(H_CLEAN_SLB, h_clean_slb);
> > +    spapr_register_hypercall(H_INVALIDATE_PID, h_invalidate_pid);
> > +    spapr_register_hypercall(H_REGISTER_PROC_TBL,
> > h_register_process_table);
> > +
> >      /* "debugger" hcalls (also used by SLOF). Note: We do -not-
> > differenciate
> >       * here between the "CI" and the "CACHE" variants, they will
> > use whatever
> >       * mapping attributes qemu is using. When using KVM, the
> > kernel will
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index c6a929a..a975944 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -348,6 +348,9 @@ struct sPAPRMachineState {
> >  #define H_XIRR_X                0x2FC
> >  #define H_RANDOM                0x300
> >  #define H_SET_MODE              0x31C
> > +#define H_CLEAN_SLB             0x374
> > +#define H_INVALIDATE_PID        0x378
> > +#define H_REGISTER_PROC_TBL     0x37C
> Here you call it REGISTER_PROC_TBL but in the comment it's
> REGISTER_PROCESS_TABLE; which is it?

I think we're going with H_REGISTER_PROC_TBL

> 
> > 
> >  #define H_SIGNAL_SYS_RESET      0x380
> >  #define MAX_HCALL_OPCODE        H_SIGNAL_SYS_RESET
> >