| Message ID | 20230906043333.448244-4-harshpb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Nested PAPR API (KVM on PowerVM) | expand |
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;
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; >
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)) { 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; Notifier epow_notifier; QTAILQ_HEAD(, SpaprEventLogEntry) pending_events;