On 4/21/25 15:44, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.ka...@amd.com>
>
No need to add to what Boris already said on the commit message and
comments, so just looking at the code.
> +
> + /*
> + * Issue AP destroy on all APs (to ensure they are kicked out
> + * of guest mode) to allow using RMPADJUST to remove the VMSA
> + * tag on VMSA pages especially for guests that allow HLT to
> + * not be intercepted.
> + */
> +
> + local_irq_save(flags);
> +
> + ghcb = __sev_get_ghcb(&state);
> +
> + vc_ghcb_invalidate(ghcb);
> + ghcb_set_rax(ghcb, vmsa->sev_features);
> +
> + /* Issue VMGEXIT AP Destroy NAE event */
> +
> + ghcb_set_sw_exit_code(ghcb, SVM_VMGEXIT_AP_CREATION);
> + ghcb_set_sw_exit_info_1(ghcb,
> + ((u64)apic_id << 32) |
> + ((u64)snp_vmpl << 16) |
> + SVM_VMGEXIT_AP_DESTROY);
> + ghcb_set_sw_exit_info_2(ghcb, __pa(vmsa));
> +
> + sev_es_wr_ghcb_msr(__pa(ghcb));
> + VMGEXIT();
> +
> + if (!ghcb_sw_exit_info_1_is_valid(ghcb) ||
> + lower_32_bits(ghcb->save.sw_exit_info_1)) {
> + pr_err("SNP AP Destroy error\n");
> + local_irq_restore(flags);
> + return;
> + }
> +
> + __sev_put_ghcb(&state);
> +
> + local_irq_restore(flags);
It looks like this whole block above is very similar to the block in
wakeup_cpu_via_vmgexit(). It makes sense to create a single function
that takes either SVM_VMGEXIT_AP_CREATE or SVM_VMGEXIT_AP_DESTROY as an
argument and processes appropriately.
> +
> + snp_cleanup_vmsa(vmsa, apic_id);
> + }
> +}
> +
> void snp_kexec_finish(void)
> {
> struct sev_es_runtime_data *data;
> @@ -987,6 +1088,8 @@ void snp_kexec_finish(void)
> if (!IS_ENABLED(CONFIG_KEXEC_CORE))
> return;
>
> + sev_snp_shutdown_all_aps();
> +
> unshare_all_memory();
>
> /*
> @@ -1254,6 +1357,8 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned
> long start_ip)
> /* Record the current VMSA page */
> per_cpu(sev_vmsa, cpu) = vmsa;
>
> + per_cpu(sev_vcpu_apic_id, cpu) = apic_id;
Is this really needed? Can't you use the cpuid_to_apicid array or create
a function in the topology.c file to return the APIC ID?
Thanks,
Tom
> +
> return ret;
> }
>
