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