Re: [PATCH v5 12/14] spapr: nested: Use correct source for parttbl info for nested PAPR API.

[Harsh Prateek Bora]

On 3/12/24 17:11, Nicholas Piggin wrote:
> On Fri Mar 8, 2024 at 9:19 PM AEST, Harsh Prateek Bora wrote:
> > For nested PAPR API, we use SpaprMachineStateNestedGuest struct to store
> > partition table info, use the same in spapr_get_pate_nested() via
> > helper.
> >
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> > Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> > ---
> >   include/hw/ppc/spapr_nested.h |  4 ++++
> >   hw/ppc/spapr.c                |  6 ++++--
> >   hw/ppc/spapr_nested.c         | 22 +++++++++++++++++++++-
> >   3 files changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
> > index bd43c6b6ef..152019fe3d 100644
> > --- a/include/hw/ppc/spapr_nested.h
> > +++ b/include/hw/ppc/spapr_nested.h
> > @@ -518,4 +518,8 @@ 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);
> >   void spapr_nested_gsb_init(void);
> > +bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
> > +                                target_ulong lpid, ppc_v3_pate_t *entry);
> > +SpaprMachineStateNestedGuest *spapr_get_nested_guest(SpaprMachineState *spapr,
> > +                                                     target_ulong lpid);
>
> Why is this made non-static? Doesn't seem to be needed in later patches
> either? Other than that,
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

You're right, looks like I missed it in v5, but kindly squash in below
incremental update with this patch, just relocating static helper above
caller?

regards,
Harsh

diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h
index 152019fe3d..3a36ba446b 100644
--- a/include/hw/ppc/spapr_nested.h
+++ b/include/hw/ppc/spapr_nested.h
@@ -520,6 +520,4 @@ uint8_t spapr_nested_api(SpaprMachineState *spapr);
 void spapr_nested_gsb_init(void);
 bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
                                 target_ulong lpid, ppc_v3_pate_t *entry);
-SpaprMachineStateNestedGuest *spapr_get_nested_guest(SpaprMachineState 
*spapr,
-                                                     target_ulong lpid);
 #endif /* HW_SPAPR_NESTED_H */
diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
index 8db9dc19e3..df22bd69bd 100644
--- a/hw/ppc/spapr_nested.c
+++ b/hw/ppc/spapr_nested.c
@@ -60,6 +60,15 @@ bool spapr_get_pate_nested_hv(SpaprMachineState 
*spapr, PowerPCCPU *cpu,
     return true;
 }

+static SpaprMachineStateNestedGuest 
*spapr_get_nested_guest(SpaprMachineState *spapr,
+                                                     target_ulong guestid)
+{
+    SpaprMachineStateNestedGuest *guest;
+
+    guest = g_hash_table_lookup(spapr->nested.guests, 
GINT_TO_POINTER(guestid));
+    return guest;
+}
+
 bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
                                 target_ulong lpid, ppc_v3_pate_t *entry)
 {
@@ -549,15 +558,6 @@ static void spapr_exit_nested_hv(PowerPCCPU *cpu, 
int excp)
     address_space_unmap(CPU(cpu)->as, regs, len, len, true);
 }

-SpaprMachineStateNestedGuest *spapr_get_nested_guest(SpaprMachineState 
*spapr,
-                                                     target_ulong guestid)
-{
-    SpaprMachineStateNestedGuest *guest;
-
-    guest = g_hash_table_lookup(spapr->nested.guests, 
GINT_TO_POINTER(guestid));
-    return guest;
-}
-
 static bool spapr_nested_vcpu_check(SpaprMachineStateNestedGuest *guest,
                                     target_ulong vcpuid, bool inoutbuf)
 {



> >   #endif /* HW_SPAPR_NESTED_H */
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index a08ffe55b6..54fc01e462 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1376,11 +1376,13 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, 
> > PowerPCCPU *cpu,
> >           entry->dw1 = spapr->patb_entry;
> >           return true;
> >       } else {
> > -        assert(spapr_nested_api(spapr));
> >           if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) {
> >               return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry);
> > +        } else if (spapr_nested_api(spapr) == NESTED_API_PAPR) {
> > +            return spapr_get_pate_nested_papr(spapr, cpu, lpid, entry);
> > +        } else {
> > +            g_assert_not_reached();
> >           }
> > -        return false;
> >       }
> >   }
> >   
> > diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> > index ca99805ce8..e0b234c786 100644
> > --- a/hw/ppc/spapr_nested.c
> > +++ b/hw/ppc/spapr_nested.c
> > @@ -58,6 +58,21 @@ bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, 
> > PowerPCCPU *cpu,
> >       return true;
> >   }
> >   
> > +bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
> > +                                target_ulong lpid, ppc_v3_pate_t *entry)
> > +{
> > +    SpaprMachineStateNestedGuest *guest;
> > +    assert(lpid != 0);
> > +    guest = spapr_get_nested_guest(spapr, lpid);
> > +    if (!guest) {
> > +        return false;
> > +    }
> > +
> > +    entry->dw0 = guest->parttbl[0];
> > +    entry->dw1 = guest->parttbl[1];
> > +    return true;
> > +}
> > +
> >   #define PRTS_MASK      0x1f
> >   
> >   static target_ulong h_set_ptbl(PowerPCCPU *cpu,
> > @@ -540,7 +555,6 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp)
> >       }
> >   }
> >   
> > -static
> >   SpaprMachineStateNestedGuest *spapr_get_nested_guest(SpaprMachineState *spapr,
> >                                                        target_ulong guestid)
> >   {
> > @@ -1585,6 +1599,12 @@ bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, 
> > PowerPCCPU *cpu,
> >       return false;
> >   }
> >   
> > +bool spapr_get_pate_nested_papr(SpaprMachineState *spapr, PowerPCCPU *cpu,
> > +                                target_ulong lpid, ppc_v3_pate_t *entry)
> > +{
> > +    return false;
> > +}
> > +
> >   void spapr_register_nested_papr(void)
> >   {
> >       /* DO NOTHING */