Re: [PATCH v5 09/14] spapr: nested: Extend nested_ppc_state for nested PAPR API

[Nicholas Piggin]

On Fri Mar 8, 2024 at 9:19 PM AEST, Harsh Prateek Bora wrote:
> Currently, nested_ppc_state stores a certain set of registers and works
> with nested_[load|save]_state() for state transfer as reqd for nested-hv API.
> Extending these with additional registers state as reqd for nested PAPR API.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>

I still have concerns with exactly how registers are saved and restored,
but it's not necessarily a new problem with v2, and not so much
fundamental design flow more of verifying details so I'm inclined to take
this for now.

Acked-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  include/hw/ppc/spapr_nested.h |  50 ++++++++++++++++
>  target/ppc/cpu.h              |   2 +
>  hw/ppc/spapr_nested.c         | 106 ++++++++++++++++++++++++++++++++++
>  3 files changed, 158 insertions(+)
>
> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
> index 2ac3076fac..d232014ccb 100644
> --- a/include/hw/ppc/spapr_nested.h
> +++ b/include/hw/ppc/spapr_nested.h
> @@ -7,6 +7,7 @@ typedef struct SpaprMachineStateNested {
>      uint64_t ptcr;
>      uint8_t api;
>  #define NESTED_API_KVM_HV  1
> +#define NESTED_API_PAPR    2
>      bool capabilities_set;
>      uint32_t pvr_base;
>      GHashTable *guests;
> @@ -121,6 +122,55 @@ struct nested_ppc_state {
>      uint64_t ppr;
>  
>      int64_t tb_offset;
> +    /* Nested PAPR API */
> +    uint64_t amor;
> +    uint64_t dawr0;
> +    uint64_t dawrx0;
> +    uint64_t ciabr;
> +    uint64_t purr;
> +    uint64_t spurr;
> +    uint64_t ic;
> +    uint64_t vtb;
> +    uint64_t hdar;
> +    uint64_t hdsisr;
> +    uint64_t heir;
> +    uint64_t asdr;
> +    uint64_t dawr1;
> +    uint64_t dawrx1;
> +    uint64_t dexcr;
> +    uint64_t hdexcr;
> +    uint64_t hashkeyr;
> +    uint64_t hashpkeyr;
> +    ppc_vsr_t vsr[64] QEMU_ALIGNED(16);
> +    uint64_t ebbhr;
> +    uint64_t tar;
> +    uint64_t ebbrr;
> +    uint64_t bescr;
> +    uint64_t iamr;
> +    uint64_t amr;
> +    uint64_t uamor;
> +    uint64_t dscr;
> +    uint64_t fscr;
> +    uint64_t pspb;
> +    uint64_t ctrl;
> +    uint64_t vrsave;
> +    uint64_t dar;
> +    uint64_t dsisr;
> +    uint64_t pmc1;
> +    uint64_t pmc2;
> +    uint64_t pmc3;
> +    uint64_t pmc4;
> +    uint64_t pmc5;
> +    uint64_t pmc6;
> +    uint64_t mmcr0;
> +    uint64_t mmcr1;
> +    uint64_t mmcr2;
> +    uint64_t mmcra;
> +    uint64_t sdar;
> +    uint64_t siar;
> +    uint64_t sier;
> +    uint32_t vscr;
> +    uint64_t fpscr;
>  };
>  
>  typedef struct SpaprMachineStateNestedGuestVcpu {
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 0133da4e07..4cffd46c79 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1755,9 +1755,11 @@ void ppc_compat_add_property(Object *obj, const char 
> *name,
>  #define SPR_PSPB              (0x09F)
>  #define SPR_DPDES             (0x0B0)
>  #define SPR_DAWR0             (0x0B4)
> +#define SPR_DAWR1             (0x0B5)
>  #define SPR_RPR               (0x0BA)
>  #define SPR_CIABR             (0x0BB)
>  #define SPR_DAWRX0            (0x0BC)
> +#define SPR_DAWRX1            (0x0BD)
>  #define SPR_HFSCR             (0x0BE)
>  #define SPR_VRSAVE            (0x100)
>  #define SPR_USPRG0            (0x100)
> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> index 4c0e2e91e1..09ebf42a57 100644
> --- a/hw/ppc/spapr_nested.c
> +++ b/hw/ppc/spapr_nested.c
> @@ -108,6 +108,7 @@ static target_ulong h_copy_tofrom_guest(PowerPCCPU *cpu,
>  static void nested_save_state(struct nested_ppc_state *save, PowerPCCPU *cpu)
>  {
>      CPUPPCState *env = &cpu->env;
> +    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  
>      memcpy(save->gpr, env->gpr, sizeof(save->gpr));
>  
> @@ -134,6 +135,58 @@ static void nested_save_state(struct nested_ppc_state 
> *save, PowerPCCPU *cpu)
>      save->pidr = env->spr[SPR_BOOKS_PID];
>      save->ppr = env->spr[SPR_PPR];
>  
> +    if (spapr_nested_api(spapr) == NESTED_API_PAPR) {
> +        save->pvr = env->spr[SPR_PVR];
> +        save->amor = env->spr[SPR_AMOR];
> +        save->dawr0 = env->spr[SPR_DAWR0];
> +        save->dawrx0 = env->spr[SPR_DAWRX0];
> +        save->ciabr = env->spr[SPR_CIABR];
> +        save->purr = env->spr[SPR_PURR];
> +        save->spurr = env->spr[SPR_SPURR];
> +        save->ic = env->spr[SPR_IC];
> +        save->vtb = env->spr[SPR_VTB];
> +        save->hdar = env->spr[SPR_HDAR];
> +        save->hdsisr = env->spr[SPR_HDSISR];
> +        save->heir = env->spr[SPR_HEIR];
> +        save->asdr = env->spr[SPR_ASDR];
> +        save->dawr1 = env->spr[SPR_DAWR1];
> +        save->dawrx1 = env->spr[SPR_DAWRX1];
> +        save->dexcr = env->spr[SPR_DEXCR];
> +        save->hdexcr = env->spr[SPR_HDEXCR];
> +        save->hashkeyr = env->spr[SPR_HASHKEYR];
> +        save->hashpkeyr = env->spr[SPR_HASHPKEYR];
> +        memcpy(save->vsr, env->vsr, sizeof(save->vsr));
> +        save->ebbhr = env->spr[SPR_EBBHR];
> +        save->tar = env->spr[SPR_TAR];
> +        save->ebbrr = env->spr[SPR_EBBRR];
> +        save->bescr = env->spr[SPR_BESCR];
> +        save->iamr = env->spr[SPR_IAMR];
> +        save->amr = env->spr[SPR_AMR];
> +        save->uamor = env->spr[SPR_UAMOR];
> +        save->dscr = env->spr[SPR_DSCR];
> +        save->fscr = env->spr[SPR_FSCR];
> +        save->pspb = env->spr[SPR_PSPB];
> +        save->ctrl = env->spr[SPR_CTRL];
> +        save->vrsave = env->spr[SPR_VRSAVE];
> +        save->dar = env->spr[SPR_DAR];
> +        save->dsisr = env->spr[SPR_DSISR];
> +        save->pmc1 = env->spr[SPR_POWER_PMC1];
> +        save->pmc2 = env->spr[SPR_POWER_PMC2];
> +        save->pmc3 = env->spr[SPR_POWER_PMC3];
> +        save->pmc4 = env->spr[SPR_POWER_PMC4];
> +        save->pmc5 = env->spr[SPR_POWER_PMC5];
> +        save->pmc6 = env->spr[SPR_POWER_PMC6];
> +        save->mmcr0 = env->spr[SPR_POWER_MMCR0];
> +        save->mmcr1 = env->spr[SPR_POWER_MMCR1];
> +        save->mmcr2 = env->spr[SPR_POWER_MMCR2];
> +        save->mmcra = env->spr[SPR_POWER_MMCRA];
> +        save->sdar = env->spr[SPR_POWER_SDAR];
> +        save->siar = env->spr[SPR_POWER_SIAR];
> +        save->sier = env->spr[SPR_POWER_SIER];
> +        save->vscr = ppc_get_vscr(env);
> +        save->fpscr = env->fpscr;
> +    }
> +
>      save->tb_offset = env->tb_env->tb_offset;
>  }
>  
> @@ -141,6 +194,7 @@ static void nested_load_state(PowerPCCPU *cpu, struct 
> nested_ppc_state *load)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> +    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  
>      memcpy(env->gpr, load->gpr, sizeof(env->gpr));
>  
> @@ -167,6 +221,58 @@ static void nested_load_state(PowerPCCPU *cpu, struct 
> nested_ppc_state *load)
>      env->spr[SPR_BOOKS_PID] = load->pidr;
>      env->spr[SPR_PPR] = load->ppr;
>  
> +    if (spapr_nested_api(spapr) == NESTED_API_PAPR) {
> +        env->spr[SPR_PVR] = load->pvr;
> +        env->spr[SPR_AMOR] = load->amor;
> +        env->spr[SPR_DAWR0] = load->dawr0;
> +        env->spr[SPR_DAWRX0] = load->dawrx0;
> +        env->spr[SPR_CIABR] = load->ciabr;
> +        env->spr[SPR_PURR] = load->purr;
> +        env->spr[SPR_SPURR] = load->purr;
> +        env->spr[SPR_IC] = load->ic;
> +        env->spr[SPR_VTB] = load->vtb;
> +        env->spr[SPR_HDAR] = load->hdar;
> +        env->spr[SPR_HDSISR] = load->hdsisr;
> +        env->spr[SPR_HEIR] = load->heir;
> +        env->spr[SPR_ASDR] = load->asdr;
> +        env->spr[SPR_DAWR1] = load->dawr1;
> +        env->spr[SPR_DAWRX1] = load->dawrx1;
> +        env->spr[SPR_DEXCR] = load->dexcr;
> +        env->spr[SPR_HDEXCR] = load->hdexcr;
> +        env->spr[SPR_HASHKEYR] = load->hashkeyr;
> +        env->spr[SPR_HASHPKEYR] = load->hashpkeyr;
> +        memcpy(env->vsr, load->vsr, sizeof(env->vsr));
> +        env->spr[SPR_EBBHR] = load->ebbhr;
> +        env->spr[SPR_TAR] = load->tar;
> +        env->spr[SPR_EBBRR] = load->ebbrr;
> +        env->spr[SPR_BESCR] = load->bescr;
> +        env->spr[SPR_IAMR] = load->iamr;
> +        env->spr[SPR_AMR] = load->amr;
> +        env->spr[SPR_UAMOR] = load->uamor;
> +        env->spr[SPR_DSCR] = load->dscr;
> +        env->spr[SPR_FSCR] = load->fscr;
> +        env->spr[SPR_PSPB] = load->pspb;
> +        env->spr[SPR_CTRL] = load->ctrl;
> +        env->spr[SPR_VRSAVE] = load->vrsave;
> +        env->spr[SPR_DAR] = load->dar;
> +        env->spr[SPR_DSISR] = load->dsisr;
> +        env->spr[SPR_POWER_PMC1] = load->pmc1;
> +        env->spr[SPR_POWER_PMC2] = load->pmc2;
> +        env->spr[SPR_POWER_PMC3] = load->pmc3;
> +        env->spr[SPR_POWER_PMC4] = load->pmc4;
> +        env->spr[SPR_POWER_PMC5] = load->pmc5;
> +        env->spr[SPR_POWER_PMC6] = load->pmc6;
> +        env->spr[SPR_POWER_MMCR0] = load->mmcr0;
> +        env->spr[SPR_POWER_MMCR1] = load->mmcr1;
> +        env->spr[SPR_POWER_MMCR2] = load->mmcr2;
> +        env->spr[SPR_POWER_MMCRA] = load->mmcra;
> +        env->spr[SPR_POWER_SDAR] = load->sdar;
> +        env->spr[SPR_POWER_SIAR] = load->siar;
> +        env->spr[SPR_POWER_SIER] = load->sier;
> +        ppc_store_vscr(env, load->vscr);
> +        ppc_store_fpscr(env, load->fpscr);
> +    }
> +
>      env->tb_env->tb_offset = load->tb_offset;
>  
>      /*