On Wed, Aug 03, 2022, Maxim Levitsky wrote:
> Those structs will be used to read/write the smram state image.
>
> Also document the differences between KVM's SMRAM layout and SMRAM
> layout that is used by real Intel/AMD cpus.
>
> Signed-off-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> ---
> arch/x86/kvm/emulate.c | 6 +
> arch/x86/kvm/kvm_emulate.h | 218 +++++++++++++++++++++++++++++++++++++
> arch/x86/kvm/x86.c | 1 +
> 3 files changed, 225 insertions(+)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 18551611cb13af..55d9328e6074a2 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -5864,3 +5864,9 @@ bool emulator_can_use_gpa(struct x86_emulate_ctxt *ctxt)
>
> return true;
> }
> +
> +void __init kvm_emulator_init(void)
> +{
> + __check_smram32_offsets();
> + __check_smram64_offsets();
> +}
...
> +static inline void __check_smram64_offsets(void)
Why double underscores? Same question for the macros.
> +{
> +#define __CHECK_SMRAM64_OFFSET(field, offset) \
> + ASSERT_STRUCT_OFFSET(struct kvm_smram_state_64, field, offset - 0xFE00)
> +
> + __CHECK_SMRAM64_OFFSET(es, 0xFE00);
> + __CHECK_SMRAM64_OFFSET(cs, 0xFE10);
> + __CHECK_SMRAM64_OFFSET(ss, 0xFE20);
> + __CHECK_SMRAM64_OFFSET(ds, 0xFE30);
> + __CHECK_SMRAM64_OFFSET(fs, 0xFE40);
> + __CHECK_SMRAM64_OFFSET(gs, 0xFE50);
> + __CHECK_SMRAM64_OFFSET(gdtr, 0xFE60);
> + __CHECK_SMRAM64_OFFSET(ldtr, 0xFE70);
> + __CHECK_SMRAM64_OFFSET(idtr, 0xFE80);
> + __CHECK_SMRAM64_OFFSET(tr, 0xFE90);
> + __CHECK_SMRAM64_OFFSET(io_restart_rip, 0xFEA0);
> + __CHECK_SMRAM64_OFFSET(io_restart_rcx, 0xFEA8);
> + __CHECK_SMRAM64_OFFSET(io_restart_rsi, 0xFEB0);
> + __CHECK_SMRAM64_OFFSET(io_restart_rdi, 0xFEB8);
> + __CHECK_SMRAM64_OFFSET(io_restart_dword, 0xFEC0);
> + __CHECK_SMRAM64_OFFSET(reserved1, 0xFEC4);
> + __CHECK_SMRAM64_OFFSET(io_inst_restart, 0xFEC8);
> + __CHECK_SMRAM64_OFFSET(auto_hlt_restart, 0xFEC9);
> + __CHECK_SMRAM64_OFFSET(reserved2, 0xFECA);
> + __CHECK_SMRAM64_OFFSET(efer, 0xFED0);
> + __CHECK_SMRAM64_OFFSET(svm_guest_flag, 0xFED8);
> + __CHECK_SMRAM64_OFFSET(svm_guest_vmcb_gpa, 0xFEE0);
> + __CHECK_SMRAM64_OFFSET(svm_guest_virtual_int, 0xFEE8);
> + __CHECK_SMRAM64_OFFSET(reserved3, 0xFEF0);
> + __CHECK_SMRAM64_OFFSET(smm_revison, 0xFEFC);
> + __CHECK_SMRAM64_OFFSET(smbase, 0xFF00);
> + __CHECK_SMRAM64_OFFSET(reserved4, 0xFF04);
> + __CHECK_SMRAM64_OFFSET(ssp, 0xFF18);
> + __CHECK_SMRAM64_OFFSET(svm_guest_pat, 0xFF20);
> + __CHECK_SMRAM64_OFFSET(svm_host_efer, 0xFF28);
> + __CHECK_SMRAM64_OFFSET(svm_host_cr4, 0xFF30);
> + __CHECK_SMRAM64_OFFSET(svm_host_cr3, 0xFF38);
> + __CHECK_SMRAM64_OFFSET(svm_host_cr0, 0xFF40);
> + __CHECK_SMRAM64_OFFSET(cr4, 0xFF48);
> + __CHECK_SMRAM64_OFFSET(cr3, 0xFF50);
> + __CHECK_SMRAM64_OFFSET(cr0, 0xFF58);
> + __CHECK_SMRAM64_OFFSET(dr7, 0xFF60);
> + __CHECK_SMRAM64_OFFSET(dr6, 0xFF68);
> + __CHECK_SMRAM64_OFFSET(rflags, 0xFF70);
> + __CHECK_SMRAM64_OFFSET(rip, 0xFF78);
> + __CHECK_SMRAM64_OFFSET(gprs, 0xFF80);
> +#undef __CHECK_SMRAM64_OFFSET
> +}
> +
> +union kvm_smram {
> + struct kvm_smram_state_64 smram64;
> + struct kvm_smram_state_32 smram32;
> + u8 bytes[512];
> +};
> +
> +void __init kvm_emulator_init(void);
> +
> +
Unnecessary newline.
> /* Host execution mode. */
> #if defined(CONFIG_X86_32)
> #define X86EMUL_MODE_HOST X86EMUL_MODE_PROT32
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 33560bfa0cac6e..bea7e5015d592e 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -13355,6 +13355,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_vmgexit_msr_protocol_exit);
> static int __init kvm_x86_init(void)
> {
> kvm_mmu_x86_module_init();
> + kvm_emulator_init();
Please don't add an init call that is nop at runtime, e.g. I was _really_ curious
what initialization needed to be done in the emulator. And it makes it look like
kvm_x86_exit() forgot to call kvm_emulator_exit().
em_rsm() already ends up with
BUILD_BUG_ON(sizeof(smram) != 512);
just put all the assertions there.
> return 0;
> }
> module_init(kvm_x86_init);
> --
> 2.26.3
>
