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