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 <mi...@neuling.org>
Signed-off-by: Harsh Prateek Bora <hars...@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;