[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [RFC PATCH v1 5/6] spapr: Unregister HPT savevm handlers
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [RFC PATCH v1 5/6] spapr: Unregister HPT savevm handlers for radix guests |
Date: |
Wed, 17 May 2017 16:59:33 +1000 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Wed, May 17, 2017 at 09:19:21AM +0530, Bharata B Rao wrote:
> HPT gets created by default and later when the guest turns out to be
> a radix guest, the HPT is destroyed when guest does H_REGISTER_PROC_TBL
> hcall.
I don't think that's entirely accurate. At least in some KVM
configurations, we assume radix first, and only allocate the HPT once
the guest confirms it is doing hash.
> Let HTAB savevm handlers registration and unregistration follow
> the same model so that we don't end up having unrequired HTAB savevm
> handlers for radix guests.
>
> This also ensures that HTAB savevm handlers seemlessly get destroyed and
> recreated like HTAB itself when hash guest reboots.
>
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
> hw/ppc/spapr.c | 15 +++++++++++++--
> hw/ppc/spapr_hcall.c | 1 +
> include/hw/ppc/spapr.h | 2 ++
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 521eef1..05abfc1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1237,6 +1237,7 @@ static void spapr_reallocate_hpt(sPAPRMachineState
> *spapr, int shift,
>
> /* Clean up any HPT info from a previous boot */
> spapr_free_hpt(spapr);
> + spapr_htab_savevm_unregister(spapr);
I'd prefer that the unregister be folded into spapr_free_hpt().
Basically we want calls that create or remove the HPT and handle
everything - allocation/freeing if necessary, informing KVM if
necessary, and registering/deregistering the savevm handlers if
necesary.
I think that will also remove the need for the trivial
spapr_htab_savevm_{un,}register() wrappers.
>
> rc = kvmppc_reset_htab(shift);
> if (rc < 0) {
> @@ -1275,6 +1276,7 @@ static void spapr_reallocate_hpt(sPAPRMachineState
> *spapr, int shift,
> DIRTY_HPTE(HPTE(spapr->htab, i));
> }
> }
> + spapr_htab_savevm_register(spapr);
> }
>
> void spapr_setup_hpt_and_vrma(sPAPRMachineState *spapr)
> @@ -1874,6 +1876,17 @@ static SaveVMHandlers savevm_htab_handlers = {
> .load_state = htab_load,
> };
>
> +void spapr_htab_savevm_register(sPAPRMachineState *spapr)
> +{
> + register_savevm_live(NULL, "spapr/htab", -1, 1,
> + &savevm_htab_handlers, spapr);
> +}
> +
> +void spapr_htab_savevm_unregister(sPAPRMachineState *spapr)
> +{
> + unregister_savevm_live(NULL, "spapr/htab", spapr);
> +}
> +
> static void spapr_boot_set(void *opaque, const char *boot_device,
> Error **errp)
> {
> @@ -2336,8 +2349,6 @@ static void ppc_spapr_init(MachineState *machine)
> * interface, this is a legacy from the sPAPREnvironment structure
> * which predated MachineState but had a similar function */
> vmstate_register(NULL, 0, &vmstate_spapr, spapr);
> - register_savevm_live(NULL, "spapr/htab", -1, 1,
> - &savevm_htab_handlers, spapr);
>
> /* used by RTAS */
> QTAILQ_INIT(&spapr->ccs_list);
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index be79e3d..768aa57 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -914,6 +914,7 @@ static void spapr_check_setup_free_hpt(sPAPRMachineState
> *spapr,
> } else if (!(patbe_old & PATBE1_GR)) {
> /* HASH->RADIX : Free HPT */
> spapr_free_hpt(spapr);
> + spapr_htab_savevm_unregister(spapr);
> } else if (!(patbe_new & PATBE1_GR)) {
> /* RADIX->HASH || NOTHING->HASH : Allocate HPT */
> spapr_setup_hpt_and_vrma(spapr);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 6f9cb85..5b39a26 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -636,6 +636,8 @@ void
> spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
> uint32_t count, uint32_t
> index);
> void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
> sPAPRMachineState *spapr);
> +void spapr_htab_savevm_register(sPAPRMachineState *spapr);
> +void spapr_htab_savevm_unregister(sPAPRMachineState *spapr);
>
> /* rtas-configure-connector state */
> struct sPAPRConfigureConnectorState {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature