Re: [PATCH RESEND 03/15] ppc: spapr: Use SpaprMachineStateNested's ptcr instead of nested_ptcr

["Nicholas Piggin" <npiggin@gmail.com>]

On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:
> Use nested guest state specific struct for storing related info.

So this is the patch I would introduce the SpaprMachineStateNested
struct, with just the .ptrc member. Add other members to it as they
are used in later patches.

>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
>  hw/ppc/spapr.c         | 4 ++--
>  hw/ppc/spapr_nested.c  | 4 ++--
>  include/hw/ppc/spapr.h | 3 ++-
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 07e91e3800..e44686b04d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1340,8 +1340,8 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu,
>  
>          assert(lpid != 0);
>  
> -        patb = spapr->nested_ptcr & PTCR_PATB;
> -        pats = spapr->nested_ptcr & PTCR_PATS;
> +        patb = spapr->nested.ptcr & PTCR_PATB;
> +        pats = spapr->nested.ptcr & PTCR_PATS;
>  
>          /* Check if partition table is properly aligned */
>          if (patb & MAKE_64BIT_MASK(0, pats + 12)) {

At this point I wonder if we should first move the nested part of
spapr_get_pate into nested code. It's a bit of a wart to have it
here when most of the other nested cases are abstracted from non
nested code quite well.

> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> index 121aa96ddc..a669470f1a 100644
> --- a/hw/ppc/spapr_nested.c
> +++ b/hw/ppc/spapr_nested.c
> @@ -25,7 +25,7 @@ static target_ulong h_set_ptbl(PowerPCCPU *cpu,
>          return H_PARAMETER;
>      }
>  
> -    spapr->nested_ptcr = ptcr; /* Save new partition table */
> +    spapr->nested.ptcr = ptcr; /* Save new partition table */
>  
>      return H_SUCCESS;
>  }
> @@ -157,7 +157,7 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
>      struct kvmppc_pt_regs *regs;
>      hwaddr len;
>  
> -    if (spapr->nested_ptcr == 0) {
> +    if (spapr->nested.ptcr == 0) {
>          return H_NOT_AVAILABLE;
>      }
>  
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 3990fed1d9..c8b42af430 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -12,6 +12,7 @@
>  #include "hw/ppc/spapr_xive.h"  /* For SpaprXive */
>  #include "hw/ppc/xics.h"        /* For ICSState */
>  #include "hw/ppc/spapr_tpm_proxy.h"
> +#include "hw/ppc/spapr_nested.h" /* for SpaprMachineStateNested */
>  
>  struct SpaprVioBus;
>  struct SpaprPhbState;
> @@ -216,7 +217,7 @@ struct SpaprMachineState {
>      uint32_t vsmt;       /* Virtual SMT mode (KVM's "core stride") */
>  
>      /* Nested HV support (TCG only) */
> -    uint64_t nested_ptcr;
> +    struct SpaprMachineStateNested nested;

I think convention says to use the typedef for these?

Thanks,
Nick

>  
>      Notifier epow_notifier;
>      QTAILQ_HEAD(, SpaprEventLogEntry) pending_events;