2020-08-24 09:21:00

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH v6 00/76] x86: SEV-ES Guest Support

From: Joerg Roedel <[email protected]>

Hi,

here is the new version of the SEV-ES client enabling patch-set. It is
based on the latest tip/master branch and contains the necessary
changes. In particular those ar:

- Enabling CR4.FSGSBASE early on supported processors so that
early #VC exceptions on APs can be handled.

- Add another patch (patch 1) to fix a KVM frame-size build
warning on 32bit.

The previous versions can be found as a linked-list starting here:

https://lore.kernel.org/lkml/[email protected]/

There you also find more detailed information about SEV-ES in general
and its implications.

Please review.

Thanks,

Joerg

Borislav Petkov (1):
KVM: SVM: Use __packed shorthand

Doug Covelli (1):
x86/vmware: Add VMware specific handling for VMMCALL under SEV-ES

Joerg Roedel (54):
KVM: SVM: nested: Don't allocate VMCB structures on stack
KVM: SVM: Add GHCB Accessor functions
x86/traps: Move pf error codes to <asm/trap_pf.h>
x86/insn: Make inat-tables.c suitable for pre-decompression code
x86/umip: Factor out instruction fetch
x86/umip: Factor out instruction decoding
x86/insn: Add insn_get_modrm_reg_off()
x86/insn: Add insn_has_rep_prefix() helper
x86/boot/compressed/64: Disable red-zone usage
x86/boot/compressed/64: Add IDT Infrastructure
x86/boot/compressed/64: Rename kaslr_64.c to ident_map_64.c
x86/boot/compressed/64: Add page-fault handler
x86/boot/compressed/64: Always switch to own page-table
x86/boot/compressed/64: Don't pre-map memory in KASLR code
x86/boot/compressed/64: Change add_identity_map() to take start and
end
x86/boot/compressed/64: Add stage1 #VC handler
x86/boot/compressed/64: Call set_sev_encryption_mask earlier
x86/boot/compressed/64: Check return value of
kernel_ident_mapping_init()
x86/boot/compressed/64: Add set_page_en/decrypted() helpers
x86/boot/compressed/64: Setup GHCB Based VC Exception handler
x86/boot/compressed/64: Unmap GHCB page before booting the kernel
x86/fpu: Move xgetbv()/xsetbv() into separate header
x86/idt: Move IDT to data segment
x86/idt: Split idt_data setup out of set_intr_gate()
x86/head/64: Install startup GDT
x86/head/64: Setup MSR_GS_BASE before calling into C code
x86/head/64: Load GDT after switch to virtual addresses
x86/head/64: Load segment registers earlier
x86/head/64: Switch to initial stack earlier
x86/head/64: Make fixup_pointer() static inline
x86/head/64: Load IDT earlier
x86/head/64: Move early exception dispatch to C code
x86/head/64: Set CR4.FSGSBASE early
x86/sev-es: Add SEV-ES Feature Detection
x86/sev-es: Print SEV-ES info into kernel log
x86/sev-es: Compile early handler code into kernel image
x86/sev-es: Setup early #VC handler
x86/sev-es: Setup GHCB based boot #VC handler
x86/sev-es: Allocate and Map IST stack for #VC handler
x86/sev-es: Adjust #VC IST Stack on entering NMI handler
x86/dumpstack/64: Add noinstr version of get_stack_info()
x86/entry/64: Add entry code for #VC handler
x86/sev-es: Wire up existing #VC exit-code handlers
x86/sev-es: Handle instruction fetches from user-space
x86/sev-es: Handle MMIO String Instructions
x86/sev-es: Handle #AC Events
x86/sev-es: Handle #DB Events
x86/paravirt: Allow hypervisor specific VMMCALL handling under SEV-ES
x86/realmode: Add SEV-ES specific trampoline entry point
x86/smpboot: Setup TSS for starting AP
x86/head/64: Don't call verify_cpu() on starting APs
x86/head/64: Rename start_cpu0
x86/sev-es: Support CPU offline/online
x86/sev-es: Handle NMI State

Martin Radev (1):
x86/sev-es: Check required CPU features for SEV-ES

Tom Lendacky (19):
KVM: SVM: Add GHCB definitions
x86/cpufeatures: Add SEV-ES CPU feature
x86/sev-es: Add support for handling IOIO exceptions
x86/sev-es: Add CPUID handling to #VC handler
x86/sev-es: Setup per-cpu GHCBs for the runtime handler
x86/sev-es: Add Runtime #VC Exception Handler
x86/sev-es: Handle MMIO events
x86/sev-es: Handle MSR events
x86/sev-es: Handle DR7 read/write events
x86/sev-es: Handle WBINVD Events
x86/sev-es: Handle RDTSC(P) Events
x86/sev-es: Handle RDPMC Events
x86/sev-es: Handle INVD Events
x86/sev-es: Handle MONITOR/MONITORX Events
x86/sev-es: Handle MWAIT/MWAITX Events
x86/sev-es: Handle VMMCALL Events
x86/kvm: Add KVM specific VMMCALL handling under SEV-ES
x86/realmode: Setup AP jump table
x86/efi: Add GHCB mappings when SEV-ES is active

arch/x86/Kconfig | 1 +
arch/x86/boot/compressed/Makefile | 9 +-
arch/x86/boot/compressed/cpuflags.c | 4 -
arch/x86/boot/compressed/head_64.S | 32 +-
arch/x86/boot/compressed/ident_map_64.c | 349 +++++
arch/x86/boot/compressed/idt_64.c | 54 +
arch/x86/boot/compressed/idt_handlers_64.S | 77 ++
arch/x86/boot/compressed/kaslr.c | 36 +-
arch/x86/boot/compressed/kaslr_64.c | 153 ---
arch/x86/boot/compressed/misc.c | 7 +
arch/x86/boot/compressed/misc.h | 50 +-
arch/x86/boot/compressed/sev-es.c | 214 +++
arch/x86/entry/entry_64.S | 78 ++
arch/x86/include/asm/cpu.h | 2 +-
arch/x86/include/asm/cpu_entry_area.h | 33 +-
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/desc_defs.h | 3 +
arch/x86/include/asm/fpu/internal.h | 30 +-
arch/x86/include/asm/fpu/xcr.h | 34 +
arch/x86/include/asm/idtentry.h | 50 +
arch/x86/include/asm/insn-eval.h | 6 +
arch/x86/include/asm/mem_encrypt.h | 5 +
arch/x86/include/asm/msr-index.h | 3 +
arch/x86/include/asm/page_64_types.h | 1 +
arch/x86/include/asm/pgtable.h | 2 +-
arch/x86/include/asm/processor.h | 7 +
arch/x86/include/asm/proto.h | 1 +
arch/x86/include/asm/realmode.h | 4 +
arch/x86/include/asm/segment.h | 2 +-
arch/x86/include/asm/setup.h | 20 +-
arch/x86/include/asm/sev-es.h | 113 ++
arch/x86/include/asm/stacktrace.h | 2 +
arch/x86/include/asm/svm.h | 100 +-
arch/x86/include/asm/trap_pf.h | 24 +
arch/x86/include/asm/trapnr.h | 1 +
arch/x86/include/asm/traps.h | 20 +-
arch/x86/include/asm/x86_init.h | 16 +-
arch/x86/include/uapi/asm/svm.h | 11 +
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/cpu/amd.c | 3 +-
arch/x86/kernel/cpu/common.c | 37 +-
arch/x86/kernel/cpu/scattered.c | 1 +
arch/x86/kernel/cpu/vmware.c | 50 +-
arch/x86/kernel/dumpstack.c | 7 +-
arch/x86/kernel/dumpstack_64.c | 47 +-
arch/x86/kernel/head64.c | 85 +-
arch/x86/kernel/head_32.S | 4 +-
arch/x86/kernel/head_64.S | 159 ++-
arch/x86/kernel/idt.c | 94 +-
arch/x86/kernel/kvm.c | 35 +-
arch/x86/kernel/nmi.c | 12 +
arch/x86/kernel/sev-es-shared.c | 507 +++++++
arch/x86/kernel/sev-es.c | 1404 ++++++++++++++++++++
arch/x86/kernel/smpboot.c | 10 +-
arch/x86/kernel/traps.c | 56 +
arch/x86/kernel/umip.c | 49 +-
arch/x86/kvm/svm/nested.c | 47 +-
arch/x86/kvm/svm/svm.c | 2 +
arch/x86/lib/insn-eval.c | 130 ++
arch/x86/mm/cpu_entry_area.c | 3 +-
arch/x86/mm/extable.c | 1 +
arch/x86/mm/mem_encrypt.c | 38 +-
arch/x86/mm/mem_encrypt_identity.c | 3 +
arch/x86/platform/efi/efi_64.c | 10 +
arch/x86/realmode/init.c | 24 +-
arch/x86/realmode/rm/header.S | 3 +
arch/x86/realmode/rm/trampoline_64.S | 20 +
arch/x86/tools/gen-insn-attr-x86.awk | 50 +-
tools/arch/x86/tools/gen-insn-attr-x86.awk | 50 +-
69 files changed, 4041 insertions(+), 456 deletions(-)
create mode 100644 arch/x86/boot/compressed/ident_map_64.c
create mode 100644 arch/x86/boot/compressed/idt_64.c
create mode 100644 arch/x86/boot/compressed/idt_handlers_64.S
delete mode 100644 arch/x86/boot/compressed/kaslr_64.c
create mode 100644 arch/x86/boot/compressed/sev-es.c
create mode 100644 arch/x86/include/asm/fpu/xcr.h
create mode 100644 arch/x86/include/asm/sev-es.h
create mode 100644 arch/x86/include/asm/trap_pf.h
create mode 100644 arch/x86/kernel/sev-es-shared.c
create mode 100644 arch/x86/kernel/sev-es.c

--
2.28.0


2020-08-24 09:21:30

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH v6 10/76] x86/insn: Add insn_get_modrm_reg_off()

From: Joerg Roedel <[email protected]>

Add a function to the instruction decoder which returns the pt_regs
offset of the register specified in the reg field of the modrm byte.

Signed-off-by: Joerg Roedel <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/insn-eval.h | 1 +
arch/x86/lib/insn-eval.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)

diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
index 392b4fe377f9..f748f57f1491 100644
--- a/arch/x86/include/asm/insn-eval.h
+++ b/arch/x86/include/asm/insn-eval.h
@@ -17,6 +17,7 @@

void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs);
int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs);
+int insn_get_modrm_reg_off(struct insn *insn, struct pt_regs *regs);
unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx);
int insn_get_code_seg_params(struct pt_regs *regs);
int insn_fetch_from_user(struct pt_regs *regs,
diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index f52046f90dd3..a8ac5c5e94f0 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -20,6 +20,7 @@

enum reg_type {
REG_TYPE_RM = 0,
+ REG_TYPE_REG,
REG_TYPE_INDEX,
REG_TYPE_BASE,
};
@@ -441,6 +442,13 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs,
regno += 8;
break;

+ case REG_TYPE_REG:
+ regno = X86_MODRM_REG(insn->modrm.value);
+
+ if (X86_REX_R(insn->rex_prefix.value))
+ regno += 8;
+ break;
+
case REG_TYPE_INDEX:
regno = X86_SIB_INDEX(insn->sib.value);
if (X86_REX_X(insn->rex_prefix.value))
@@ -809,6 +817,21 @@ int insn_get_modrm_rm_off(struct insn *insn, struct pt_regs *regs)
return get_reg_offset(insn, regs, REG_TYPE_RM);
}

+/**
+ * insn_get_modrm_reg_off() - Obtain register in reg part of the ModRM byte
+ * @insn: Instruction containing the ModRM byte
+ * @regs: Register values as seen when entering kernel mode
+ *
+ * Returns:
+ *
+ * The register indicated by the reg part of the ModRM byte. The
+ * register is obtained as an offset from the base of pt_regs.
+ */
+int insn_get_modrm_reg_off(struct insn *insn, struct pt_regs *regs)
+{
+ return get_reg_offset(insn, regs, REG_TYPE_REG);
+}
+
/**
* get_seg_base_limit() - obtain base address and limit of a segment
* @insn: Instruction. Must be valid.
--
2.28.0

2020-08-24 09:22:03

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH v6 04/76] KVM: SVM: Use __packed shorthand

From: Borislav Petkov <[email protected]>

Use the shorthand to make it more readable.

No functional changes.

Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/svm.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 50e5fa8d3249..da38eb195355 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -150,14 +150,14 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
#define SVM_NESTED_CTL_NP_ENABLE BIT(0)
#define SVM_NESTED_CTL_SEV_ENABLE BIT(1)

-struct __attribute__ ((__packed__)) vmcb_seg {
+struct vmcb_seg {
u16 selector;
u16 attrib;
u32 limit;
u64 base;
-};
+} __packed;

-struct __attribute__ ((__packed__)) vmcb_save_area {
+struct vmcb_save_area {
struct vmcb_seg es;
struct vmcb_seg cs;
struct vmcb_seg ss;
@@ -231,7 +231,7 @@ struct __attribute__ ((__packed__)) vmcb_save_area {
u64 xcr0;
u8 valid_bitmap[16];
u64 x87_state_gpa;
-};
+} __packed;

struct ghcb {
struct vmcb_save_area save;
@@ -252,11 +252,11 @@ static inline void __unused_size_checks(void)
BUILD_BUG_ON(sizeof(struct ghcb) != 4096);
}

-struct __attribute__ ((__packed__)) vmcb {
+struct vmcb {
struct vmcb_control_area control;
u8 reserved_control[1024 - sizeof(struct vmcb_control_area)];
struct vmcb_save_area save;
-};
+} __packed;

#define SVM_CPUID_FUNC 0x8000000a

--
2.28.0

2020-08-24 09:23:08

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH v6 01/76] KVM: SVM: nested: Don't allocate VMCB structures on stack

From: Joerg Roedel <[email protected]>

Do not allocate a vmcb_control_area and a vmcb_save_area on the stack,
as these structures will become larger with future extenstions of
SVM and thus the svm_set_nested_state() function will become a too large
stack frame.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kvm/svm/nested.c | 47 +++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index fb68467e6049..28036629abf8 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1060,10 +1060,14 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
struct vmcb *hsave = svm->nested.hsave;
struct vmcb __user *user_vmcb = (struct vmcb __user *)
&user_kvm_nested_state->data.svm[0];
- struct vmcb_control_area ctl;
- struct vmcb_save_area save;
+ struct vmcb_control_area *ctl;
+ struct vmcb_save_area *save;
+ int ret;
u32 cr0;

+ BUILD_BUG_ON(sizeof(struct vmcb_control_area) + sizeof(struct vmcb_save_area) >
+ KVM_STATE_NESTED_SVM_VMCB_SIZE);
+
if (kvm_state->format != KVM_STATE_NESTED_FORMAT_SVM)
return -EINVAL;

@@ -1095,13 +1099,22 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
return -EINVAL;
if (kvm_state->size < sizeof(*kvm_state) + KVM_STATE_NESTED_SVM_VMCB_SIZE)
return -EINVAL;
- if (copy_from_user(&ctl, &user_vmcb->control, sizeof(ctl)))
- return -EFAULT;
- if (copy_from_user(&save, &user_vmcb->save, sizeof(save)))
- return -EFAULT;

- if (!nested_vmcb_check_controls(&ctl))
- return -EINVAL;
+ ret = -ENOMEM;
+ ctl = kzalloc(sizeof(*ctl), GFP_KERNEL);
+ save = kzalloc(sizeof(*save), GFP_KERNEL);
+ if (!ctl || !save)
+ goto out_free;
+
+ ret = -EFAULT;
+ if (copy_from_user(ctl, &user_vmcb->control, sizeof(*ctl)))
+ goto out_free;
+ if (copy_from_user(save, &user_vmcb->save, sizeof(*save)))
+ goto out_free;
+
+ ret = -EINVAL;
+ if (!nested_vmcb_check_controls(ctl))
+ goto out_free;

/*
* Processor state contains L2 state. Check that it is
@@ -1109,15 +1122,15 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
*/
cr0 = kvm_read_cr0(vcpu);
if (((cr0 & X86_CR0_CD) == 0) && (cr0 & X86_CR0_NW))
- return -EINVAL;
+ goto out_free;

/*
* Validate host state saved from before VMRUN (see
* nested_svm_check_permissions).
* TODO: validate reserved bits for all saved state.
*/
- if (!(save.cr0 & X86_CR0_PG))
- return -EINVAL;
+ if (!(save->cr0 & X86_CR0_PG))
+ goto out_free;

/*
* All checks done, we can enter guest mode. L1 control fields
@@ -1126,15 +1139,21 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
* contains saved L1 state.
*/
copy_vmcb_control_area(&hsave->control, &svm->vmcb->control);
- hsave->save = save;
+ hsave->save = *save;

svm->nested.vmcb = kvm_state->hdr.svm.vmcb_pa;
- load_nested_vmcb_control(svm, &ctl);
+ load_nested_vmcb_control(svm, ctl);
nested_prepare_vmcb_control(svm);

out_set_gif:
svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));
- return 0;
+
+ ret = 0;
+out_free:
+ kfree(save);
+ kfree(ctl);
+
+ return ret;
}

struct kvm_x86_nested_ops svm_nested_ops = {
--
2.28.0

2020-08-24 09:23:17

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH v6 02/76] KVM: SVM: Add GHCB definitions

From: Tom Lendacky <[email protected]>

Extend the vmcb_safe_area with SEV-ES fields and add a new
'struct ghcb' which will be used for guest-hypervisor communication.

Signed-off-by: Tom Lendacky <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/svm.h | 45 +++++++++++++++++++++++++++++++++++++-
arch/x86/kvm/svm/svm.c | 2 ++
2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 8a1f5382a4ea..094b8f8fb1d4 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -200,13 +200,56 @@ struct __attribute__ ((__packed__)) vmcb_save_area {
u64 br_to;
u64 last_excp_from;
u64 last_excp_to;
+
+ /*
+ * The following part of the save area is valid only for
+ * SEV-ES guests when referenced through the GHCB.
+ */
+ u8 reserved_7[104];
+ u64 reserved_8; /* rax already available at 0x01f8 */
+ u64 rcx;
+ u64 rdx;
+ u64 rbx;
+ u64 reserved_9; /* rsp already available at 0x01d8 */
+ u64 rbp;
+ u64 rsi;
+ u64 rdi;
+ u64 r8;
+ u64 r9;
+ u64 r10;
+ u64 r11;
+ u64 r12;
+ u64 r13;
+ u64 r14;
+ u64 r15;
+ u8 reserved_10[16];
+ u64 sw_exit_code;
+ u64 sw_exit_info_1;
+ u64 sw_exit_info_2;
+ u64 sw_scratch;
+ u8 reserved_11[56];
+ u64 xcr0;
+ u8 valid_bitmap[16];
+ u64 x87_state_gpa;
};

+struct ghcb {
+ struct vmcb_save_area save;
+ u8 reserved_save[2048 - sizeof(struct vmcb_save_area)];
+
+ u8 shared_buffer[2032];
+
+ u8 reserved_1[10];
+ u16 protocol_version; /* negotiated SEV-ES/GHCB protocol version */
+ u32 ghcb_usage;
+} __packed;
+

static inline void __unused_size_checks(void)
{
- BUILD_BUG_ON(sizeof(struct vmcb_save_area) != 0x298);
+ BUILD_BUG_ON(sizeof(struct vmcb_save_area) != 1032);
BUILD_BUG_ON(sizeof(struct vmcb_control_area) != 256);
+ BUILD_BUG_ON(sizeof(struct ghcb) != 4096);
}

struct __attribute__ ((__packed__)) vmcb {
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 03dd7bac8034..4368b66615c1 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4164,6 +4164,8 @@ static struct kvm_x86_init_ops svm_init_ops __initdata = {

static int __init svm_init(void)
{
+ __unused_size_checks();
+
return kvm_init(&svm_init_ops, sizeof(struct vcpu_svm),
__alignof__(struct vcpu_svm), THIS_MODULE);
}
--
2.28.0

2020-08-24 10:48:28

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v6 02/76] KVM: SVM: Add GHCB definitions

On Mon, Aug 24, 2020 at 10:53:57AM +0200, Joerg Roedel wrote:
> static inline void __unused_size_checks(void)
> {
> - BUILD_BUG_ON(sizeof(struct vmcb_save_area) != 0x298);
> + BUILD_BUG_ON(sizeof(struct vmcb_save_area) != 1032);
> BUILD_BUG_ON(sizeof(struct vmcb_control_area) != 256);
> + BUILD_BUG_ON(sizeof(struct ghcb) != 4096);

Could those naked numbers be proper, meaningfully named defines?

--
Regards/Gruss,
Boris.

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

2020-08-25 00:23:01

by Mike Stunes

[permalink] [raw]
Subject: Re: [PATCH v6 00/76] x86: SEV-ES Guest Support

Hi Joerg,

> On Aug 24, 2020, at 1:53 AM, Joerg Roedel <[email protected]> wrote:
>
> From: Joerg Roedel <[email protected]>
>
> Hi,
>
> here is the new version of the SEV-ES client enabling patch-set. It is
> based on the latest tip/master branch and contains the necessary
> changes. In particular those ar:
>
> - Enabling CR4.FSGSBASE early on supported processors so that
> early #VC exceptions on APs can be handled.

Thanks for the new update! I still see the same FSGSBASE behavior on our platform.

That is, APs come up offline; masking out either FSGSBASE or RDPID from the
guest's CPUID results in all CPUs online.

Is that still expected with this patch set? (As you mentioned in an earlier reply,
I’m testing on a Rome system.)

Thanks!
Mike

2020-08-25 06:28:07

by Jörg Rödel

[permalink] [raw]
Subject: Re: [PATCH v6 00/76] x86: SEV-ES Guest Support

Hi Mike,

On Tue, Aug 25, 2020 at 12:21:03AM +0000, Mike Stunes wrote:
> Thanks for the new update! I still see the same FSGSBASE behavior on our platform.
>
> That is, APs come up offline; masking out either FSGSBASE or RDPID from the
> guest's CPUID results in all CPUs online.
>
> Is that still expected with this patch set? (As you mentioned in an earlier reply,
> I’m testing on a Rome system.)

The RDPID fix (removing RDPID usage from paranoid_entry) is probably not
yet merged into the base you have been using. But removing RDPID from
CPUID should make things work until the fix is merged.

Regards,

Joerg

2020-08-25 09:24:44

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH v6 02/76] KVM: SVM: Add GHCB definitions

On Mon, Aug 24, 2020 at 12:44:51PM +0200, Borislav Petkov wrote:
> On Mon, Aug 24, 2020 at 10:53:57AM +0200, Joerg Roedel wrote:
> > static inline void __unused_size_checks(void)
> > {
> > - BUILD_BUG_ON(sizeof(struct vmcb_save_area) != 0x298);
> > + BUILD_BUG_ON(sizeof(struct vmcb_save_area) != 1032);
> > BUILD_BUG_ON(sizeof(struct vmcb_control_area) != 256);
> > + BUILD_BUG_ON(sizeof(struct ghcb) != 4096);
>
> Could those naked numbers be proper, meaningfully named defines?

I don't think so, if I look at the history of these checks their whole
purpose seems to be to alert the developer/maintainer when their size
changes and that they might not fit on the stack anymore. But that is
taken care of in patch 1.

Regards,

Joerg

2020-08-25 11:05:48

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v6 02/76] KVM: SVM: Add GHCB definitions

On Tue, Aug 25, 2020 at 11:22:24AM +0200, Joerg Roedel wrote:
> I don't think so, if I look at the history of these checks their whole
> purpose seems to be to alert the developer/maintainer when their size
> changes and that they might not fit on the stack anymore. But that is
> taken care of in patch 1.

Why? What's wrong with:

BUILD_BUG_ON(sizeof(struct vmcb_save_area) != VMCB_SAVE_AREA_SIZE);
BUILD_BUG_ON(sizeof(struct vmcb_control_area) != VMCB_CONTROL_AREA_SIZE);
BUILD_BUG_ON(sizeof(struct ghcb) != PAGE_SIZE);

?

--
Regards/Gruss,
Boris.

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

2020-08-27 16:04:41

by Arvind Sankar

[permalink] [raw]
Subject: Re: [PATCH v6 02/76] KVM: SVM: Add GHCB definitions

On Tue, Aug 25, 2020 at 01:04:46PM +0200, Borislav Petkov wrote:
> On Tue, Aug 25, 2020 at 11:22:24AM +0200, Joerg Roedel wrote:
> > I don't think so, if I look at the history of these checks their whole
> > purpose seems to be to alert the developer/maintainer when their size
> > changes and that they might not fit on the stack anymore. But that is
> > taken care of in patch 1.
>
> Why? What's wrong with:
>
> BUILD_BUG_ON(sizeof(struct vmcb_save_area) != VMCB_SAVE_AREA_SIZE);
> BUILD_BUG_ON(sizeof(struct vmcb_control_area) != VMCB_CONTROL_AREA_SIZE);
> BUILD_BUG_ON(sizeof(struct ghcb) != PAGE_SIZE);
>
> ?
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Wouldn't we rather just remove the checks?

2020-08-28 11:56:46

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH v6 02/76] KVM: SVM: Add GHCB definitions

On Thu, Aug 27, 2020 at 12:01:13PM -0400, Arvind Sankar wrote:
> Wouldn't we rather just remove the checks?

I think that's a different topic to be discussed with the KVM
maintainers. For now I will add defines for the magic numbers.

Regards,

Joerg