Hi Haren,
Mostly this seems OK to me. Some questions:
Haren Myneni <ha...@linux.ibm.com> writes:
> Since the VAS windows belong to the VAS hardware resource, the
> hypervisor expects the partition to close them on source partition
> and reopen them after the partition migrated on the destination
> machine.
Not clear to me what "expects" really means here. Would it be accurate
to say "requires" instead? If the OS fails to close the windows before
suspend, what happens?
> This handler is called before pseries_suspend() to close these
> windows and again invoked after migration. All active windows
> for both default and QoS types will be closed and mark them
> in-active and reopened after migration with this handler.
> During the migration, the user space receives paste instruction
> failure if it issues copy/paste on these in-active windows.
OK, I assume that's tolerable to existing users, is that correct? I.e.
users should already be prepared to incur arbitrary paste instruction
failures.
>
> Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
> ---
> arch/powerpc/platforms/pseries/mobility.c | 5 ++
> arch/powerpc/platforms/pseries/vas.c | 86 +++++++++++++++++++++++
> arch/powerpc/platforms/pseries/vas.h | 6 ++
> 3 files changed, 97 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/mobility.c
> b/arch/powerpc/platforms/pseries/mobility.c
> index 85033f392c78..70004243e25e 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -26,6 +26,7 @@
> #include <asm/machdep.h>
> #include <asm/rtas.h>
> #include "pseries.h"
> +#include "vas.h" /* vas_migration_handler() */
> #include "../../kernel/cacheinfo.h"
>
> static struct kobject *mobility_kobj;
> @@ -669,12 +670,16 @@ static int pseries_migrate_partition(u64 handle)
> if (ret)
> return ret;
>
> + vas_migration_handler(VAS_SUSPEND);
> +
vas_migration_handler() can return an error value. Is that OK to ignore
before going into the suspend?
> ret = pseries_suspend(handle);
> if (ret == 0)
> post_mobility_fixup();
> else
> pseries_cancel_migration(handle, ret);
>
> + vas_migration_handler(VAS_RESUME);
> +
No concerns here though. Nothing to be done about errors encountered in
the resume path.
> return ret;
> }
>
> diff --git a/arch/powerpc/platforms/pseries/vas.c
> b/arch/powerpc/platforms/pseries/vas.c
> index e4797fc73553..b53e3fe02971 100644
> --- a/arch/powerpc/platforms/pseries/vas.c
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -873,6 +873,92 @@ static struct notifier_block pseries_vas_nb = {
> .notifier_call = pseries_vas_notifier,
> };
>
> +/*
> + * For LPM, all windows have to be closed on the source partition
> + * before migration and reopen them on the destination partition
> + * after migration. So closing windows during suspend and
> + * reopen them during resume.
> + */
> +int vas_migration_handler(int action)
> +{
> + struct hv_vas_cop_feat_caps *hv_caps;
> + struct vas_cop_feat_caps *caps;
> + int lpar_creds, new_creds = 0;
> + struct vas_caps *vcaps;
> + int i, rc = 0;
> +
> + hv_caps = kmalloc(sizeof(*hv_caps), GFP_KERNEL);
> + if (!hv_caps)
> + return -ENOMEM;
> +
> + mutex_lock(&vas_pseries_mutex);
> +
> + for (i = 0; i < VAS_MAX_FEAT_TYPE; i++) {
> + vcaps = &vascaps[i];
> + caps = &vcaps->caps;
> + lpar_creds = atomic_read(&caps->target_creds);
> +
> + rc = h_query_vas_capabilities(H_QUERY_VAS_CAPABILITIES,
> + vcaps->feat,
> + (u64)virt_to_phys(hv_caps));
> + if (!rc) {
> + new_creds = be16_to_cpu(hv_caps->target_lpar_creds);
> + /*
> + * Should not happen. But incase print messages, close
> + * all windows in the list during suspend and reopen
> + * windows based on new lpar_creds on the destination
> + * system.
> + */
> + if (lpar_creds != new_creds) {
> + pr_err("state(%d): lpar creds: %d HV lpar
> creds: %d\n",
> + action, lpar_creds, new_creds);
> + pr_err("Used creds: %d, Active creds: %d\n",
> + atomic_read(&caps->used_creds),
> + vcaps->num_wins - vcaps->close_wins);
> + }
> + } else {
> + pr_err("state(%d): Get VAS capabilities failed with
> %d\n",
> + action, rc);
> + /*
> + * We can not stop migration with the current lpm
> + * implementation. So continue closing all windows in
> + * the list (during suspend) and return without
> + * opening windows (during resume) if VAS capabilities
> + * HCALL failed.
> + */
> + if (action == VAS_RESUME)
> + goto out;
> + }
> +
> + switch (action) {
> + case VAS_SUSPEND:
> + rc = reconfig_close_windows(vcaps, vcaps->num_wins,
> + true);
> + break;
> + case VAS_RESUME:
> + atomic_set(&caps->target_creds, new_creds);
> + rc = reconfig_open_windows(vcaps, new_creds, true);
> + break;
> + default:
> + /* should not happen */
> + pr_err("Invalid migration action %d\n", action);
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + /*
> + * Ignore errors during suspend and return for resume.
> + */
> + if (rc && (action == VAS_RESUME))
> + goto out;
> + }
> +
> +out:
> + mutex_unlock(&vas_pseries_mutex);
> + kfree(hv_caps);
> + return rc;
> +}
The control flow with respect to releasing the allocation and the mutex
looks correct to me. I also verified that H_QUERY_VAS_CAPABILITIES does
not return a busy/retry status, so this code appears to handle all the
architected statuses for that call.
