Re: [PATCH v5 04/14] spapr: nested: keep nested-hv related code restricted to its API.

[Nicholas Piggin]

On Fri Mar 8, 2024 at 9:19 PM AEST, Harsh Prateek Bora wrote:
> spapr_exit_nested and spapr_get_pate_nested_hv contains code which
> is specific to nested-hv API. Isolating code flows based on API
> helps extending it to be used with different API as well.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>

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

> ---
>  include/hw/ppc/spapr_nested.h |  3 +++
>  hw/ppc/spapr.c                |  6 +++++-
>  hw/ppc/spapr_nested.c         | 25 ++++++++++++++++++++++---
>  3 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
> index 2488ea98da..bf3a7b8d89 100644
> --- a/include/hw/ppc/spapr_nested.h
> +++ b/include/hw/ppc/spapr_nested.h
> @@ -5,6 +5,8 @@
>  
>  typedef struct SpaprMachineStateNested {
>      uint64_t ptcr;
> +    uint8_t api;
> +#define NESTED_API_KVM_HV  1
>  } SpaprMachineStateNested;
>  
>  /*
> @@ -103,4 +105,5 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp);
>  typedef struct SpaprMachineState SpaprMachineState;
>  bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu,
>                                target_ulong lpid, ppc_v3_pate_t *entry);
> +uint8_t spapr_nested_api(SpaprMachineState *spapr);
>  #endif /* HW_SPAPR_NESTED_H */
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 65d766b898..a08ffe55b6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1376,7 +1376,11 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, 
> PowerPCCPU *cpu,
>          entry->dw1 = spapr->patb_entry;
>          return true;
>      } else {
> -        return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry);
> +        assert(spapr_nested_api(spapr));
> +        if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) {
> +            return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry);
> +        }
> +        return false;
>      }
>  }
>  
> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> index c2a33fc3a9..12fdbe2aba 100644
> --- a/hw/ppc/spapr_nested.c
> +++ b/hw/ppc/spapr_nested.c
> @@ -11,11 +11,19 @@
>  void spapr_nested_reset(SpaprMachineState *spapr)
>  {
>      if (spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV)) {
> +        spapr->nested.api = NESTED_API_KVM_HV;
>          spapr_unregister_nested_hv();
>          spapr_register_nested_hv();
> +    } else {
> +        spapr->nested.api = 0;
>      }
>  }
>  
> +uint8_t spapr_nested_api(SpaprMachineState *spapr)
> +{
> +    return spapr->nested.api;
> +}
> +
>  #ifdef CONFIG_TCG
>  
>  bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu,
> @@ -310,7 +318,7 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
>      return env->gpr[3];
>  }
>  
> -void spapr_exit_nested(PowerPCCPU *cpu, int excp)
> +static void spapr_exit_nested_hv(PowerPCCPU *cpu, int excp)
>  {
>      CPUPPCState *env = &cpu->env;
>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> @@ -322,8 +330,6 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp)
>      struct kvmppc_pt_regs *regs;
>      hwaddr len;
>  
> -    assert(spapr_cpu->in_nested);
> -
>      nested_save_state(&l2_state, cpu);
>      hsrr0 = env->spr[SPR_HSRR0];
>      hsrr1 = env->spr[SPR_HSRR1];
> @@ -413,6 +419,19 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp)
>      address_space_unmap(CPU(cpu)->as, regs, len, len, true);
>  }
>  
> +void spapr_exit_nested(PowerPCCPU *cpu, int excp)
> +{
> +    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +    SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +
> +    assert(spapr_cpu->in_nested);
> +    if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) {
> +        spapr_exit_nested_hv(cpu, excp);
> +    } else {
> +        g_assert_not_reached();
> +    }
> +}
> +
>  void spapr_register_nested_hv(void)
>  {
>      spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);