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

[Harsh Prateek Bora]

On 9/7/23 06:43, Nicholas Piggin wrote:
> 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.

Sure, will do.

> > 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.

Yeh, I also felt similar when modifying the existing nested code here.
Let me do the needful in next version.

> > 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?

Sure, will update. Thanks.

regards,
Harsh

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