| Message ID | 20240220083609.748325-5-harshpb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Nested PAPR API (KVM on PowerVM) | expand |
On Tue Feb 20, 2024 at 6:35 PM AEST, Harsh Prateek Bora wrote: > spapr_exit_nested and spapr_get_pate_nested_hv contains code which > is specific to nested-hv API. Isolating code flows based on API > helps extending it to be used with different API as well. > > Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > Suggested-by: Nicholas Piggin <npiggin@gmail.com> > --- > include/hw/ppc/spapr_nested.h | 4 ++++ > hw/ppc/spapr.c | 7 ++++++- > hw/ppc/spapr_caps.c | 1 + > hw/ppc/spapr_nested.c | 27 ++++++++++++++++++++++++--- > 4 files changed, 35 insertions(+), 4 deletions(-) > > diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h > index 2488ea98da..3f07c81c3d 100644 > --- a/include/hw/ppc/spapr_nested.h > +++ b/include/hw/ppc/spapr_nested.h > @@ -5,6 +5,8 @@ > > typedef struct SpaprMachineStateNested { > uint64_t ptcr; > + uint8_t api; > +#define NESTED_API_KVM_HV 1 > } SpaprMachineStateNested; > > /* > @@ -103,4 +105,6 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp); > typedef struct SpaprMachineState SpaprMachineState; > bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu, > target_ulong lpid, ppc_v3_pate_t *entry); > +void spapr_nested_init(SpaprMachineState *spapr); > +uint8_t spapr_nested_api(SpaprMachineState *spapr); > #endif /* HW_SPAPR_NESTED_H */ > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 97b69c0e42..51a1be027a 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1376,7 +1376,11 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu, > entry->dw1 = spapr->patb_entry; > return true; > } else { > - return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry); > + assert(spapr_nested_api(spapr)); > + if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) { > + return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry); > + } > + return false; > } > } > > @@ -3443,6 +3447,7 @@ static void spapr_instance_init(Object *obj) > spapr_get_host_serial, spapr_set_host_serial); > object_property_set_description(obj, "host-serial", > "Host serial number to advertise in guest device tree"); > + spapr_nested_init(spapr); I would maybe make this init a reset instead, and then it could do the hypercall unregistering as well? You could rework that part of it into patch 1 (or reorder the patches). Thanks, Nick
On 2/27/24 14:24, Nicholas Piggin wrote: > On Tue Feb 20, 2024 at 6:35 PM AEST, Harsh Prateek Bora wrote: >> spapr_exit_nested and spapr_get_pate_nested_hv contains code which >> is specific to nested-hv API. Isolating code flows based on API >> helps extending it to be used with different API as well. >> >> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com> >> Suggested-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> include/hw/ppc/spapr_nested.h | 4 ++++ >> hw/ppc/spapr.c | 7 ++++++- >> hw/ppc/spapr_caps.c | 1 + >> hw/ppc/spapr_nested.c | 27 ++++++++++++++++++++++++--- >> 4 files changed, 35 insertions(+), 4 deletions(-) >> >> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h >> index 2488ea98da..3f07c81c3d 100644 >> --- a/include/hw/ppc/spapr_nested.h >> +++ b/include/hw/ppc/spapr_nested.h >> @@ -5,6 +5,8 @@ >> >> typedef struct SpaprMachineStateNested { >> uint64_t ptcr; >> + uint8_t api; >> +#define NESTED_API_KVM_HV 1 >> } SpaprMachineStateNested; >> >> /* >> @@ -103,4 +105,6 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp); >> typedef struct SpaprMachineState SpaprMachineState; >> bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu, >> target_ulong lpid, ppc_v3_pate_t *entry); >> +void spapr_nested_init(SpaprMachineState *spapr); >> +uint8_t spapr_nested_api(SpaprMachineState *spapr); >> #endif /* HW_SPAPR_NESTED_H */ >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 97b69c0e42..51a1be027a 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -1376,7 +1376,11 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu, >> entry->dw1 = spapr->patb_entry; >> return true; >> } else { >> - return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry); >> + assert(spapr_nested_api(spapr)); >> + if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) { >> + return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry); >> + } >> + return false; >> } >> } >> >> @@ -3443,6 +3447,7 @@ static void spapr_instance_init(Object *obj) >> spapr_get_host_serial, spapr_set_host_serial); >> object_property_set_description(obj, "host-serial", >> "Host serial number to advertise in guest device tree"); >> + spapr_nested_init(spapr); > > I would maybe make this init a reset instead, and then it could do > the hypercall unregistering as well? You could rework that part of > it into patch 1 (or reorder the patches). If we do unregistering here, we still hit the assert during spapr_machine_reset which tries to reapply the caps and thus re-register hcalls. Also, We cant register hcalls in this since the caps havent been applied when init is called here. So we can do as you have previously suggested, reset in spapr_machine_reset based on caps applied. Let me know if you think otherwise? regards, Harsh > > Thanks, > Nick
On Tue Feb 27, 2024 at 7:45 PM AEST, Harsh Prateek Bora wrote: > > > On 2/27/24 14:24, Nicholas Piggin wrote: > > On Tue Feb 20, 2024 at 6:35 PM AEST, Harsh Prateek Bora wrote: > >> spapr_exit_nested and spapr_get_pate_nested_hv contains code which > >> is specific to nested-hv API. Isolating code flows based on API > >> helps extending it to be used with different API as well. > >> > >> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > >> Suggested-by: Nicholas Piggin <npiggin@gmail.com> > >> --- > >> include/hw/ppc/spapr_nested.h | 4 ++++ > >> hw/ppc/spapr.c | 7 ++++++- > >> hw/ppc/spapr_caps.c | 1 + > >> hw/ppc/spapr_nested.c | 27 ++++++++++++++++++++++++--- > >> 4 files changed, 35 insertions(+), 4 deletions(-) > >> > >> diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h > >> index 2488ea98da..3f07c81c3d 100644 > >> --- a/include/hw/ppc/spapr_nested.h > >> +++ b/include/hw/ppc/spapr_nested.h > >> @@ -5,6 +5,8 @@ > >> > >> typedef struct SpaprMachineStateNested { > >> uint64_t ptcr; > >> + uint8_t api; > >> +#define NESTED_API_KVM_HV 1 > >> } SpaprMachineStateNested; > >> > >> /* > >> @@ -103,4 +105,6 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp); > >> typedef struct SpaprMachineState SpaprMachineState; > >> bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu, > >> target_ulong lpid, ppc_v3_pate_t *entry); > >> +void spapr_nested_init(SpaprMachineState *spapr); > >> +uint8_t spapr_nested_api(SpaprMachineState *spapr); > >> #endif /* HW_SPAPR_NESTED_H */ > >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > >> index 97b69c0e42..51a1be027a 100644 > >> --- a/hw/ppc/spapr.c > >> +++ b/hw/ppc/spapr.c > >> @@ -1376,7 +1376,11 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu, > >> entry->dw1 = spapr->patb_entry; > >> return true; > >> } else { > >> - return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry); > >> + assert(spapr_nested_api(spapr)); > >> + if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) { > >> + return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry); > >> + } > >> + return false; > >> } > >> } > >> > >> @@ -3443,6 +3447,7 @@ static void spapr_instance_init(Object *obj) > >> spapr_get_host_serial, spapr_set_host_serial); > >> object_property_set_description(obj, "host-serial", > >> "Host serial number to advertise in guest device tree"); > >> + spapr_nested_init(spapr); > > > > I would maybe make this init a reset instead, and then it could do > > the hypercall unregistering as well? You could rework that part of > > it into patch 1 (or reorder the patches). > > If we do unregistering here, we still hit the assert during > spapr_machine_reset which tries to reapply the caps and thus re-register > hcalls. Also, We cant register hcalls in this since the caps havent been > applied when init is called here. So we can do as you have previously > suggested, reset in spapr_machine_reset based on caps applied. > Let me know if you think otherwise? Not unregistering here, I mean make it a spapr_nested_reset() call and call it from spapr_machine_reset(). If you call it after spapr_caps_apply() then you don't need to do the hcall registering in the caps functions, just do it in the reset. Thanks, Nick
diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h index 2488ea98da..3f07c81c3d 100644 --- a/include/hw/ppc/spapr_nested.h +++ b/include/hw/ppc/spapr_nested.h @@ -5,6 +5,8 @@ typedef struct SpaprMachineStateNested { uint64_t ptcr; + uint8_t api; +#define NESTED_API_KVM_HV 1 } SpaprMachineStateNested; /* @@ -103,4 +105,6 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp); typedef struct SpaprMachineState SpaprMachineState; bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu, target_ulong lpid, ppc_v3_pate_t *entry); +void spapr_nested_init(SpaprMachineState *spapr); +uint8_t spapr_nested_api(SpaprMachineState *spapr); #endif /* HW_SPAPR_NESTED_H */ diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 97b69c0e42..51a1be027a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1376,7 +1376,11 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu, entry->dw1 = spapr->patb_entry; return true; } else { - return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry); + assert(spapr_nested_api(spapr)); + if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) { + return spapr_get_pate_nested_hv(spapr, cpu, lpid, entry); + } + return false; } } @@ -3443,6 +3447,7 @@ static void spapr_instance_init(Object *obj) spapr_get_host_serial, spapr_set_host_serial); object_property_set_description(obj, "host-serial", "Host serial number to advertise in guest device tree"); + spapr_nested_init(spapr); } static void spapr_machine_finalizefn(Object *obj) diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c index f0c2f4de78..721ddad23b 100644 --- a/hw/ppc/spapr_caps.c +++ b/hw/ppc/spapr_caps.c @@ -487,6 +487,7 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState *spapr, error_append_hint(errp, "Try appending -machine cap-nested-hv=off " "or use threads=1 with -smp\n"); } + spapr->nested.api = NESTED_API_KVM_HV; spapr_unregister_nested_hv(); /* reset across reboots */ spapr_register_nested_hv(); } diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c index e43e867295..9096dd96a4 100644 --- a/hw/ppc/spapr_nested.c +++ b/hw/ppc/spapr_nested.c @@ -8,6 +8,16 @@ #include "hw/ppc/spapr_nested.h" #include "mmu-book3s-v3.h" +void spapr_nested_init(SpaprMachineState *spapr) +{ + spapr->nested.api = 0; +} + +uint8_t spapr_nested_api(SpaprMachineState *spapr) +{ + return spapr->nested.api; +} + #ifdef CONFIG_TCG bool spapr_get_pate_nested_hv(SpaprMachineState *spapr, PowerPCCPU *cpu, @@ -302,7 +312,7 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu, return env->gpr[3]; } -void spapr_exit_nested(PowerPCCPU *cpu, int excp) +static void spapr_exit_nested_hv(PowerPCCPU *cpu, int excp) { CPUPPCState *env = &cpu->env; SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); @@ -314,8 +324,6 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp) struct kvmppc_pt_regs *regs; hwaddr len; - assert(spapr_cpu->in_nested); - nested_save_state(&l2_state, cpu); hsrr0 = env->spr[SPR_HSRR0]; hsrr1 = env->spr[SPR_HSRR1]; @@ -405,6 +413,19 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp) address_space_unmap(CPU(cpu)->as, regs, len, len, true); } +void spapr_exit_nested(PowerPCCPU *cpu, int excp) +{ + SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); + SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); + + assert(spapr_cpu->in_nested); + if (spapr_nested_api(spapr) == NESTED_API_KVM_HV) { + spapr_exit_nested_hv(cpu, excp); + } else { + g_assert_not_reached(); + } +} + void spapr_register_nested_hv(void) { spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
spapr_exit_nested and spapr_get_pate_nested_hv contains code which is specific to nested-hv API. Isolating code flows based on API helps extending it to be used with different API as well. Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com> Suggested-by: Nicholas Piggin <npiggin@gmail.com> --- include/hw/ppc/spapr_nested.h | 4 ++++ hw/ppc/spapr.c | 7 ++++++- hw/ppc/spapr_caps.c | 1 + hw/ppc/spapr_nested.c | 27 ++++++++++++++++++++++++--- 4 files changed, 35 insertions(+), 4 deletions(-)