| Message ID | 20240220083609.748325-2-harshpb@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Nested PAPR API (KVM on PowerVM) | expand |
On Tue Feb 20, 2024 at 6:35 PM AEST, Harsh Prateek Bora wrote: > Since cap-nested-hv is an optional capability, it makes sense to register > api specfic hcalls only when respective capability is enabled. This > requires to introduce a new API to unregister hypercalls to maintain > sanity across guest reboot since caps are re-applied across reboots and > re-registeration of hypercalls would hit assert otherwise. > > Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > --- > include/hw/ppc/spapr.h | 3 +++ > include/hw/ppc/spapr_nested.h | 1 - > hw/ppc/spapr_caps.c | 2 ++ > hw/ppc/spapr_hcall.c | 24 ++++++++++++++++++++++-- > hw/ppc/spapr_nested.c | 17 +++++++++++++++-- > 5 files changed, 42 insertions(+), 5 deletions(-) > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index e91791a1a9..b7ad2a0360 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -631,6 +631,7 @@ typedef target_ulong (*spapr_hcall_fn)(PowerPCCPU *cpu, SpaprMachineState *sm, > target_ulong *args); > > void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn); > +void spapr_unregister_hypercall(target_ulong opcode); > target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode, > target_ulong *args); > > @@ -1025,5 +1026,7 @@ void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt); > > /* H_WATCHDOG */ > void spapr_watchdog_init(SpaprMachineState *spapr); > +void spapr_register_nested_hv(void); > +void spapr_unregister_nested_hv(void); > > #endif /* HW_SPAPR_H */ > diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h > index d312a5d61d..09d95182b2 100644 > --- a/include/hw/ppc/spapr_nested.h > +++ b/include/hw/ppc/spapr_nested.h > @@ -95,7 +95,6 @@ struct nested_ppc_state { > int64_t tb_offset; > }; > > -void spapr_register_nested(void); > void spapr_exit_nested(PowerPCCPU *cpu, int excp); > > #endif /* HW_SPAPR_NESTED_H */ > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > index e889244e52..f0c2f4de78 100644 > --- a/hw/ppc/spapr_caps.c > +++ b/hw/ppc/spapr_caps.c > @@ -487,6 +487,8 @@ 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_unregister_nested_hv(); /* reset across reboots */ > + spapr_register_nested_hv(); This looks a bit odd. And actually it also ends up registering them in the SMT error case (which seems harmelss but a bit inconsistent). I wonder whether you could make a spapr_nested_reset() function called by spapr_machine_reset that would take care of unregistering and reregistering nested hypercalls based on the caps that were set. Thanks, Nick
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index e91791a1a9..b7ad2a0360 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -631,6 +631,7 @@ typedef target_ulong (*spapr_hcall_fn)(PowerPCCPU *cpu, SpaprMachineState *sm, target_ulong *args); void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn); +void spapr_unregister_hypercall(target_ulong opcode); target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode, target_ulong *args); @@ -1025,5 +1026,7 @@ void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt); /* H_WATCHDOG */ void spapr_watchdog_init(SpaprMachineState *spapr); +void spapr_register_nested_hv(void); +void spapr_unregister_nested_hv(void); #endif /* HW_SPAPR_H */ diff --git a/include/hw/ppc/spapr_nested.h b/include/hw/ppc/spapr_nested.h index d312a5d61d..09d95182b2 100644 --- a/include/hw/ppc/spapr_nested.h +++ b/include/hw/ppc/spapr_nested.h @@ -95,7 +95,6 @@ struct nested_ppc_state { int64_t tb_offset; }; -void spapr_register_nested(void); void spapr_exit_nested(PowerPCCPU *cpu, int excp); #endif /* HW_SPAPR_NESTED_H */ diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c index e889244e52..f0c2f4de78 100644 --- a/hw/ppc/spapr_caps.c +++ b/hw/ppc/spapr_caps.c @@ -487,6 +487,8 @@ 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_unregister_nested_hv(); /* reset across reboots */ + spapr_register_nested_hv(); } } diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index fcefd1d1c7..8dbc2863a8 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -1521,6 +1521,28 @@ void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn) *slot = fn; } +void spapr_unregister_hypercall(target_ulong opcode) +{ + spapr_hcall_fn *slot; + + if (opcode <= MAX_HCALL_OPCODE) { + assert((opcode & 0x3) == 0); + + slot = &papr_hypercall_table[opcode / 4]; + } else if (opcode >= SVM_HCALL_BASE && opcode <= SVM_HCALL_MAX) { + /* we only have SVM-related hcall numbers assigned in multiples of 4 */ + assert((opcode & 0x3) == 0); + + slot = &svm_hypercall_table[(opcode - SVM_HCALL_BASE) / 4]; + } else { + assert((opcode >= KVMPPC_HCALL_BASE) && (opcode <= KVMPPC_HCALL_MAX)); + + slot = &kvmppc_hypercall_table[opcode - KVMPPC_HCALL_BASE]; + } + + *slot = NULL; +} + target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode, target_ulong *args) { @@ -1634,8 +1656,6 @@ static void hypercall_register_types(void) spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support); spapr_register_hypercall(KVMPPC_H_UPDATE_DT, h_update_dt); - - spapr_register_nested(); } type_init(hypercall_register_types) diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c index 121aa96ddc..a071092eff 100644 --- a/hw/ppc/spapr_nested.c +++ b/hw/ppc/spapr_nested.c @@ -375,20 +375,33 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp) address_space_unmap(CPU(cpu)->as, regs, len, len, true); } -void spapr_register_nested(void) +void spapr_register_nested_hv(void) { spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl); spapr_register_hypercall(KVMPPC_H_ENTER_NESTED, h_enter_nested); spapr_register_hypercall(KVMPPC_H_TLB_INVALIDATE, h_tlb_invalidate); spapr_register_hypercall(KVMPPC_H_COPY_TOFROM_GUEST, h_copy_tofrom_guest); } + +void spapr_unregister_nested_hv(void) +{ + spapr_unregister_hypercall(KVMPPC_H_SET_PARTITION_TABLE); + spapr_unregister_hypercall(KVMPPC_H_ENTER_NESTED); + spapr_unregister_hypercall(KVMPPC_H_TLB_INVALIDATE); + spapr_unregister_hypercall(KVMPPC_H_COPY_TOFROM_GUEST); +} #else void spapr_exit_nested(PowerPCCPU *cpu, int excp) { g_assert_not_reached(); } -void spapr_register_nested(void) +void spapr_register_nested_hv(void) +{ + /* DO NOTHING */ +} + +void spapr_unregister_nested_hv(void) { /* DO NOTHING */ }