Re: [PATCH RESEND 13/15] ppc: spapr: Implement nested PAPR hcall - H_GUEST_RUN_VCPU

2023-10-12 Thread Harsh Prateek Bora




On 9/7/23 09:25, Nicholas Piggin wrote:

On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:

Once the L1 has created a nested guest and its associated VCPU, it can
request for the execution of nested guest by setting its initial state
which can be done either using the h_guest_set_state or using the input
buffers along with the call to h_guest_run_vcpu(). On guest exit, L0
uses output buffers to convey the exit cause to the L1. L0 takes care of
switching context from L1 to L2 during guest entry and restores L1 context
on guest exit.

Unlike nested-hv, L2 (nested) guest's entire state is retained with
L0 after guest exit and restored on next entry in case of nested-papr.

Signed-off-by: Michael Neuling 
Signed-off-by: Kautuk Consul 
Signed-off-by: Harsh Prateek Bora 
---
  hw/ppc/spapr_nested.c   | 471 +++-
  include/hw/ppc/spapr_cpu_core.h |   7 +-
  include/hw/ppc/spapr_nested.h   |   6 +
  3 files changed, 408 insertions(+), 76 deletions(-)

diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
index 67e389a762..3605f27115 100644
--- a/hw/ppc/spapr_nested.c
+++ b/hw/ppc/spapr_nested.c
@@ -12,6 +12,17 @@
  #ifdef CONFIG_TCG
  #define PRTS_MASK  0x1f
  
+static void exit_nested_restore_vcpu(PowerPCCPU *cpu, int excp,

+ SpaprMachineStateNestedGuestVcpu *vcpu);
+static void exit_process_output_buffer(PowerPCCPU *cpu,
+  SpaprMachineStateNestedGuest *guest,
+  target_ulong vcpuid,
+  target_ulong *r3);
+static void restore_common_regs(CPUPPCState *dst, CPUPPCState *src);
+static bool vcpu_check(SpaprMachineStateNestedGuest *guest,
+   target_ulong vcpuid,
+   bool inoutbuf);
+
  static target_ulong h_set_ptbl(PowerPCCPU *cpu,
 SpaprMachineState *spapr,
 target_ulong opcode,
@@ -187,21 +198,21 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
  return H_PARAMETER;
  }
  
-spapr_cpu->nested_host_state = g_try_new(struct nested_ppc_state, 1);

-if (!spapr_cpu->nested_host_state) {
+spapr_cpu->nested_hv_host = g_try_new(struct nested_ppc_state, 1);
+if (!spapr_cpu->nested_hv_host) {
  return H_NO_MEM;
  }


Don't rename existing thing in the same patch as adding new thing.



Sure, renaming in a separate patch before this patch.

  
  assert(env->spr[SPR_LPIDR] == 0);

  assert(env->spr[SPR_DPDES] == 0);
-nested_save_state(spapr_cpu->nested_host_state, cpu);
+nested_save_state(spapr_cpu->nested_hv_host, cpu);
  
  len = sizeof(*regs);

  regs = address_space_map(CPU(cpu)->as, regs_ptr, , false,
  MEMTXATTRS_UNSPECIFIED);
  if (!regs || len != sizeof(*regs)) {
  address_space_unmap(CPU(cpu)->as, regs, len, 0, false);
-g_free(spapr_cpu->nested_host_state);
+g_free(spapr_cpu->nested_hv_host);
  return H_P2;
  }
  
@@ -276,105 +287,146 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
  
  void spapr_exit_nested(PowerPCCPU *cpu, int excp)

  {
+SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+CPUState *cs = CPU(cpu);


I think it would be worth seeing how it looks to split these into
original and papr functions rather than try mash them together.



Yeh, that really sounds better approach. Updated patch to keep nested-hv 
code untouched and keeping both API code flows isolated.



  CPUPPCState *env = >env;
  SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
+target_ulong r3_return = env->excp_vectors[excp]; /* hcall return value */
  struct nested_ppc_state l2_state;
-target_ulong hv_ptr = spapr_cpu->nested_host_state->gpr[4];
-target_ulong regs_ptr = spapr_cpu->nested_host_state->gpr[5];
-target_ulong hsrr0, hsrr1, hdar, asdr, hdsisr;
+target_ulong hv_ptr, regs_ptr;
+target_ulong hsrr0 = 0, hsrr1 = 0, hdar = 0, asdr = 0, hdsisr = 0;
  struct kvmppc_hv_guest_state *hvstate;
  struct kvmppc_pt_regs *regs;
  hwaddr len;
+target_ulong lpid = 0, vcpuid = 0;
+struct SpaprMachineStateNestedGuestVcpu *vcpu = NULL;
+struct SpaprMachineStateNestedGuest *guest = NULL;
  
  assert(spapr_cpu->in_nested);

-
-nested_save_state(_state, cpu);
-hsrr0 = env->spr[SPR_HSRR0];
-hsrr1 = env->spr[SPR_HSRR1];
-hdar = env->spr[SPR_HDAR];
-hdsisr = env->spr[SPR_HDSISR];
-asdr = env->spr[SPR_ASDR];
+if (spapr->nested.api == NESTED_API_KVM_HV) {
+nested_save_state(_state, cpu);
+hsrr0 = env->spr[SPR_HSRR0];
+hsrr1 = env->spr[SPR_HSRR1];
+hdar = env->spr[SPR_HDAR];
+hdsisr = env->spr[SPR_HDSISR];
+asdr = env->spr[SPR_ASDR];
+} else if (spapr->nested.api == NESTED_API_PAPR) {
+lpid = spapr_cpu->nested_papr_host->gpr[5];
+   

Re: [PATCH RESEND 13/15] ppc: spapr: Implement nested PAPR hcall - H_GUEST_RUN_VCPU

2023-09-06 Thread Nicholas Piggin
On Wed Sep 6, 2023 at 2:33 PM AEST, Harsh Prateek Bora wrote:
> Once the L1 has created a nested guest and its associated VCPU, it can
> request for the execution of nested guest by setting its initial state
> which can be done either using the h_guest_set_state or using the input
> buffers along with the call to h_guest_run_vcpu(). On guest exit, L0
> uses output buffers to convey the exit cause to the L1. L0 takes care of
> switching context from L1 to L2 during guest entry and restores L1 context
> on guest exit.
>
> Unlike nested-hv, L2 (nested) guest's entire state is retained with
> L0 after guest exit and restored on next entry in case of nested-papr.
>
> Signed-off-by: Michael Neuling 
> Signed-off-by: Kautuk Consul 
> Signed-off-by: Harsh Prateek Bora 
> ---
>  hw/ppc/spapr_nested.c   | 471 +++-
>  include/hw/ppc/spapr_cpu_core.h |   7 +-
>  include/hw/ppc/spapr_nested.h   |   6 +
>  3 files changed, 408 insertions(+), 76 deletions(-)
>
> diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
> index 67e389a762..3605f27115 100644
> --- a/hw/ppc/spapr_nested.c
> +++ b/hw/ppc/spapr_nested.c
> @@ -12,6 +12,17 @@
>  #ifdef CONFIG_TCG
>  #define PRTS_MASK  0x1f
>  
> +static void exit_nested_restore_vcpu(PowerPCCPU *cpu, int excp,
> + SpaprMachineStateNestedGuestVcpu *vcpu);
> +static void exit_process_output_buffer(PowerPCCPU *cpu,
> +  SpaprMachineStateNestedGuest *guest,
> +  target_ulong vcpuid,
> +  target_ulong *r3);
> +static void restore_common_regs(CPUPPCState *dst, CPUPPCState *src);
> +static bool vcpu_check(SpaprMachineStateNestedGuest *guest,
> +   target_ulong vcpuid,
> +   bool inoutbuf);
> +
>  static target_ulong h_set_ptbl(PowerPCCPU *cpu,
> SpaprMachineState *spapr,
> target_ulong opcode,
> @@ -187,21 +198,21 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
>  return H_PARAMETER;
>  }
>  
> -spapr_cpu->nested_host_state = g_try_new(struct nested_ppc_state, 1);
> -if (!spapr_cpu->nested_host_state) {
> +spapr_cpu->nested_hv_host = g_try_new(struct nested_ppc_state, 1);
> +if (!spapr_cpu->nested_hv_host) {
>  return H_NO_MEM;
>  }

Don't rename existing thing in the same patch as adding new thing.

>  
>  assert(env->spr[SPR_LPIDR] == 0);
>  assert(env->spr[SPR_DPDES] == 0);
> -nested_save_state(spapr_cpu->nested_host_state, cpu);
> +nested_save_state(spapr_cpu->nested_hv_host, cpu);
>  
>  len = sizeof(*regs);
>  regs = address_space_map(CPU(cpu)->as, regs_ptr, , false,
>  MEMTXATTRS_UNSPECIFIED);
>  if (!regs || len != sizeof(*regs)) {
>  address_space_unmap(CPU(cpu)->as, regs, len, 0, false);
> -g_free(spapr_cpu->nested_host_state);
> +g_free(spapr_cpu->nested_hv_host);
>  return H_P2;
>  }
>  
> @@ -276,105 +287,146 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
>  
>  void spapr_exit_nested(PowerPCCPU *cpu, int excp)
>  {
> +SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +CPUState *cs = CPU(cpu);

I think it would be worth seeing how it looks to split these into
original and papr functions rather than try mash them together.

>  CPUPPCState *env = >env;
>  SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +target_ulong r3_return = env->excp_vectors[excp]; /* hcall return value 
> */
>  struct nested_ppc_state l2_state;
> -target_ulong hv_ptr = spapr_cpu->nested_host_state->gpr[4];
> -target_ulong regs_ptr = spapr_cpu->nested_host_state->gpr[5];
> -target_ulong hsrr0, hsrr1, hdar, asdr, hdsisr;
> +target_ulong hv_ptr, regs_ptr;
> +target_ulong hsrr0 = 0, hsrr1 = 0, hdar = 0, asdr = 0, hdsisr = 0;
>  struct kvmppc_hv_guest_state *hvstate;
>  struct kvmppc_pt_regs *regs;
>  hwaddr len;
> +target_ulong lpid = 0, vcpuid = 0;
> +struct SpaprMachineStateNestedGuestVcpu *vcpu = NULL;
> +struct SpaprMachineStateNestedGuest *guest = NULL;
>  
>  assert(spapr_cpu->in_nested);
> -
> -nested_save_state(_state, cpu);
> -hsrr0 = env->spr[SPR_HSRR0];
> -hsrr1 = env->spr[SPR_HSRR1];
> -hdar = env->spr[SPR_HDAR];
> -hdsisr = env->spr[SPR_HDSISR];
> -asdr = env->spr[SPR_ASDR];
> +if (spapr->nested.api == NESTED_API_KVM_HV) {
> +nested_save_state(_state, cpu);
> +hsrr0 = env->spr[SPR_HSRR0];
> +hsrr1 = env->spr[SPR_HSRR1];
> +hdar = env->spr[SPR_HDAR];
> +hdsisr = env->spr[SPR_HDSISR];
> +asdr = env->spr[SPR_ASDR];
> +} else if (spapr->nested.api == NESTED_API_PAPR) {
> +lpid = spapr_cpu->nested_papr_host->gpr[5];
> +vcpuid = spapr_cpu->nested_papr_host->gpr[6];
> +guest = 

[PATCH RESEND 13/15] ppc: spapr: Implement nested PAPR hcall - H_GUEST_RUN_VCPU

2023-09-05 Thread Harsh Prateek Bora
Once the L1 has created a nested guest and its associated VCPU, it can
request for the execution of nested guest by setting its initial state
which can be done either using the h_guest_set_state or using the input
buffers along with the call to h_guest_run_vcpu(). On guest exit, L0
uses output buffers to convey the exit cause to the L1. L0 takes care of
switching context from L1 to L2 during guest entry and restores L1 context
on guest exit.

Unlike nested-hv, L2 (nested) guest's entire state is retained with
L0 after guest exit and restored on next entry in case of nested-papr.

Signed-off-by: Michael Neuling 
Signed-off-by: Kautuk Consul 
Signed-off-by: Harsh Prateek Bora 
---
 hw/ppc/spapr_nested.c   | 471 +++-
 include/hw/ppc/spapr_cpu_core.h |   7 +-
 include/hw/ppc/spapr_nested.h   |   6 +
 3 files changed, 408 insertions(+), 76 deletions(-)

diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
index 67e389a762..3605f27115 100644
--- a/hw/ppc/spapr_nested.c
+++ b/hw/ppc/spapr_nested.c
@@ -12,6 +12,17 @@
 #ifdef CONFIG_TCG
 #define PRTS_MASK  0x1f
 
+static void exit_nested_restore_vcpu(PowerPCCPU *cpu, int excp,
+ SpaprMachineStateNestedGuestVcpu *vcpu);
+static void exit_process_output_buffer(PowerPCCPU *cpu,
+  SpaprMachineStateNestedGuest *guest,
+  target_ulong vcpuid,
+  target_ulong *r3);
+static void restore_common_regs(CPUPPCState *dst, CPUPPCState *src);
+static bool vcpu_check(SpaprMachineStateNestedGuest *guest,
+   target_ulong vcpuid,
+   bool inoutbuf);
+
 static target_ulong h_set_ptbl(PowerPCCPU *cpu,
SpaprMachineState *spapr,
target_ulong opcode,
@@ -187,21 +198,21 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
 return H_PARAMETER;
 }
 
-spapr_cpu->nested_host_state = g_try_new(struct nested_ppc_state, 1);
-if (!spapr_cpu->nested_host_state) {
+spapr_cpu->nested_hv_host = g_try_new(struct nested_ppc_state, 1);
+if (!spapr_cpu->nested_hv_host) {
 return H_NO_MEM;
 }
 
 assert(env->spr[SPR_LPIDR] == 0);
 assert(env->spr[SPR_DPDES] == 0);
-nested_save_state(spapr_cpu->nested_host_state, cpu);
+nested_save_state(spapr_cpu->nested_hv_host, cpu);
 
 len = sizeof(*regs);
 regs = address_space_map(CPU(cpu)->as, regs_ptr, , false,
 MEMTXATTRS_UNSPECIFIED);
 if (!regs || len != sizeof(*regs)) {
 address_space_unmap(CPU(cpu)->as, regs, len, 0, false);
-g_free(spapr_cpu->nested_host_state);
+g_free(spapr_cpu->nested_hv_host);
 return H_P2;
 }
 
@@ -276,105 +287,146 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
 
 void spapr_exit_nested(PowerPCCPU *cpu, int excp)
 {
+SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+CPUState *cs = CPU(cpu);
 CPUPPCState *env = >env;
 SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
+target_ulong r3_return = env->excp_vectors[excp]; /* hcall return value */
 struct nested_ppc_state l2_state;
-target_ulong hv_ptr = spapr_cpu->nested_host_state->gpr[4];
-target_ulong regs_ptr = spapr_cpu->nested_host_state->gpr[5];
-target_ulong hsrr0, hsrr1, hdar, asdr, hdsisr;
+target_ulong hv_ptr, regs_ptr;
+target_ulong hsrr0 = 0, hsrr1 = 0, hdar = 0, asdr = 0, hdsisr = 0;
 struct kvmppc_hv_guest_state *hvstate;
 struct kvmppc_pt_regs *regs;
 hwaddr len;
+target_ulong lpid = 0, vcpuid = 0;
+struct SpaprMachineStateNestedGuestVcpu *vcpu = NULL;
+struct SpaprMachineStateNestedGuest *guest = NULL;
 
 assert(spapr_cpu->in_nested);
-
-nested_save_state(_state, cpu);
-hsrr0 = env->spr[SPR_HSRR0];
-hsrr1 = env->spr[SPR_HSRR1];
-hdar = env->spr[SPR_HDAR];
-hdsisr = env->spr[SPR_HDSISR];
-asdr = env->spr[SPR_ASDR];
+if (spapr->nested.api == NESTED_API_KVM_HV) {
+nested_save_state(_state, cpu);
+hsrr0 = env->spr[SPR_HSRR0];
+hsrr1 = env->spr[SPR_HSRR1];
+hdar = env->spr[SPR_HDAR];
+hdsisr = env->spr[SPR_HDSISR];
+asdr = env->spr[SPR_ASDR];
+} else if (spapr->nested.api == NESTED_API_PAPR) {
+lpid = spapr_cpu->nested_papr_host->gpr[5];
+vcpuid = spapr_cpu->nested_papr_host->gpr[6];
+guest = spapr_get_nested_guest(spapr, lpid);
+assert(guest);
+vcpu_check(guest, vcpuid, false);
+vcpu = >vcpu[vcpuid];
+
+exit_nested_restore_vcpu(cpu, excp, vcpu);
+/* do the output buffer for run_vcpu*/
+exit_process_output_buffer(cpu, guest, vcpuid, _return);
+} else
+g_assert_not_reached();
 
 /*
  * Switch back to the host environment (including for any error).
  */
 assert(env->spr[SPR_LPIDR] != 0);
-