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 <hars...@linux.ibm.com>
> Reviewed-by: Nicholas Piggin <npig...@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
