Re: [PATCH v6 69/76] x86/realmode: Setup AP jump table

[Borislav Petkov]

On Mon, Aug 24, 2020 at 10:55:04AM +0200, Joerg Roedel wrote:
> diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
> index 8f36ae021a7f..a19ce9681ec2 100644
> --- a/arch/x86/include/uapi/asm/svm.h
> +++ b/arch/x86/include/uapi/asm/svm.h
> @@ -84,6 +84,9 @@
>  /* SEV-ES software-defined VMGEXIT events */
>  #define SVM_VMGEXIT_MMIO_READ			0x80000001
>  #define SVM_VMGEXIT_MMIO_WRITE			0x80000002
> +#define SVM_VMGEXIT_AP_JUMP_TABLE		0x80000005
> +#define		SVM_VMGEXIT_SET_AP_JUMP_TABLE			0
> +#define		SVM_VMGEXIT_GET_AP_JUMP_TABLE			1
	  ^^^^^^^^^^^^

One tab too many here.

>  #define SVM_VMGEXIT_UNSUPPORTED_EVENT		0x8000ffff
>  
>  #define SVM_EXIT_ERR           -1
> diff --git a/arch/x86/kernel/sev-es.c b/arch/x86/kernel/sev-es.c
> index 28fe95ecd508..09a45ccd6c1d 100644
> --- a/arch/x86/kernel/sev-es.c
> +++ b/arch/x86/kernel/sev-es.c
> @@ -21,6 +21,8 @@
>  #include <linux/mm.h>
>  
>  #include <asm/cpu_entry_area.h>
> +#include <asm/stacktrace.h>
> +#include <asm/realmode.h>
>  #include <asm/sev-es.h>
>  #include <asm/insn-eval.h>
>  #include <asm/fpu/internal.h>
> @@ -219,6 +221,9 @@ static __always_inline void sev_es_put_ghcb(struct ghcb_state *state)
>  	}
>  }
>  
> +/* Needed in vc_early_vc_forward_exception */

vc_early_forward_exception()

> +void do_early_exception(struct pt_regs *regs, int trapnr);
> +
>  static inline u64 sev_es_rd_ghcb_msr(void)
>  {
>  	return native_read_msr(MSR_AMD64_SEV_ES_GHCB);
> @@ -407,6 +412,69 @@ static bool vc_slow_virt_to_phys(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
>  /* Include code shared with pre-decompression boot stage */
>  #include "sev-es-shared.c"
>  
> +static u64 sev_es_get_jump_table_addr(void)

Static and used here only once - you can drop the previx "sev_es".

> +{
> +	struct ghcb_state state;
> +	unsigned long flags;
> +	struct ghcb *ghcb;
> +	u64 ret;
> +
> +	local_irq_save(flags);
> +
> +	ghcb = sev_es_get_ghcb(&state);
> +
> +	vc_ghcb_invalidate(ghcb);
> +	ghcb_set_sw_exit_code(ghcb, SVM_VMGEXIT_AP_JUMP_TABLE);
> +	ghcb_set_sw_exit_info_1(ghcb, SVM_VMGEXIT_GET_AP_JUMP_TABLE);
> +	ghcb_set_sw_exit_info_2(ghcb, 0);
> +
> +	sev_es_wr_ghcb_msr(__pa(ghcb));
> +	VMGEXIT();
> +
> +	if (!ghcb_sw_exit_info_1_is_valid(ghcb) ||
> +	    !ghcb_sw_exit_info_2_is_valid(ghcb))
> +		ret = 0;
> +
> +	ret = ghcb->save.sw_exit_info_2;
> +
> +	sev_es_put_ghcb(&state);
> +
> +	local_irq_restore(flags);
> +
> +	return ret;
> +}
> +
> +int sev_es_setup_ap_jump_table(struct real_mode_header *rmh)
> +{
> +	u16 startup_cs, startup_ip;
> +	phys_addr_t jump_table_pa;
> +	u64 jump_table_addr;
> +	u16 __iomem *jump_table;
> +
> +	jump_table_addr = sev_es_get_jump_table_addr();
> +
> +	/* Check if AP Jump Table is non-zero and page-aligned */
> +	if (!jump_table_addr || jump_table_addr & ~PAGE_MASK)
> +		return 0;

I think you need to return !0 here so that the panic() below fires with
a modified message:

	panic("Failed to get/update SEV-ES AP Jump Table");

or are we gonna boot an UP guest still?

If not panic, at least a warning that at least some APs didn't come
up...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette