acrn-dev@lists.projectacrn.org

Home Messages Hashtags Subgroups

Between

and

Re: [PATCH v2] microcode: Enable microcode update from SOS. Xu, Anthony #21 The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony toggle quoted message Show quoted text -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); uint64_t +get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH 1/2] Add CHECK and CHECK_RET macro Eddie Dong #22 Hi Like: I felt we have so many error check code now, and it becomes a little over-complicated. May be we can do: 1) Add dump_context in ASSERT. But no need to introduce another one (CHECK) 2) We need to carefully review all the usage of ASSERT and make sure no one will block host if a tamped guest trigger ASSERT. Thx Eddie toggle quoted message Show quoted text -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of like.yan@... Sent: Wednesday, March 14, 2018 6:15 AM To: acrn-dev@... Cc: Yan, Like <like.yan@...>; Yan Subject: [acrn-dev] [PATCH 1/2] Add CHECK and CHECK_RET macro From: "Yan, Like" <like.yan@...> - CHECK macro checks the given condition, if false, print a fatal message, dump both host and guest context and return void; - CHECK_RET macro checks the given condition, if false, print a fatal message, dump both host and guest context and return a given value; - dump_contexts function dump host and guest contexts Change-Id: I482998c0ac7de32474097032abbf3835057c1f21 Signed-off-by: Yan, Like <like.yan@...> --- debug/dump.c \| 10 ++++++++++ include/debug/assert.h \| 33 ++++++++++++++++++++++++++++++--- include/debug/dump.h \| 5 +++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/debug/dump.c b/debug/dump.c index 87fb334..9e02caf 100644 --- a/debug/dump.c +++ b/debug/dump.c @@ -293,6 +293,16 @@ void __assert(uint32_t line, const char file, char txt) } while (1); } +void dump_contexts(void) +{ + uint32_t cpu_id = get_cpu_id(); + uint64_t rsp = cpu_rsp_get(); + uint64_t rbp = cpu_rbp_get(); + + show_host_call_trace(rsp, rbp, cpu_id); + dump_guest_context(cpu_id); +} + void dump_exception(struct intr_ctx ctx, uint32_t cpu_id) { const char name = "Not defined"; diff --git a/include/debug/assert.h b/include/debug/assert.h index 83b26c1..b777fdd 100644 --- a/include/debug/assert.h +++ b/include/debug/assert.h @@ -34,18 +34,45 @@ #ifdef HV_DEBUG void __assert(uint32_t line, const char file, char txt); -#define ASSERT(x, ...) \ - if (!(x)) {\ - pr_fatal(__VA_ARGS__);\ +#define ASSERT(x, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ __assert(__LINE__, __FILE__, "fatal error");\ } + +#define CHECK(x, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + dump_contexts(); \ + return ; \ + } + +#define CHECK_RET(x, r, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + dump_contexts(); \ + return (r); \ + } #else #define ASSERT(x, ...) \ if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ do { \ asm volatile ("pause" ::: "memory"); \ } while (1); \ } + +#define CHECK(x, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + return ; \ + } + +#define CHECK_RET(x, r, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + return (r); \ + } #endif /* Force a compilation error if condition is false / diff --git a/include/debug/dump.h b/include/debug/dump.h index 1d3ebf3..d38415a 100644 --- a/include/debug/dump.h +++ b/include/debug/dump.h @@ -37,11 +37,16 @@ struct intr_ctx; #define CALL_TRACE_HIERARCHY_MAX 20 #define DUMP_STACK_SIZE 0x200 +void dump_contexts(void); void dump_exception(struct intr_ctx ctx, uint32_t cpu_id); void dump_interrupt(struct intr_ctx ctx); #else +static inline void dump_contexts(void) +{ +} + static inline void dump_exception(__unused struct intr_ctx ctx, __unused uint32_t cpu_id) { -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Eddie Dong #23 SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie toggle quoted message Show quoted text -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t +gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); uint64_t +get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Xu, Anthony #24 Then we probably want to wait until SOS have all CPUs. Anthony toggle quoted message Show quoted text -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t +gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); uint64_t +get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Eddie Dong #25 BTW, we are generating a VM environment, with no assumption of what SOS it may be. For native usage, Intel CPU provides the capability for OSes to update uCode, we are going to support too. Thx Eddie toggle quoted message Show quoted text -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t +gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Eddie Dong #26 Make sence.. toggle quoted message Show quoted text -----Original Message----- From: Xu, Anthony Sent: Wednesday, March 14, 2018 12:03 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: RE: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Then we probably want to wait until SOS have all CPUs. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with +or without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t +gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with +or without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Xu, Anthony #27 Per below spec, microcode update in VMX non-root operation does nothing. Anthony * WRMSR. Section 25.1.3 identifies when executions of the WRMSR instruction cause VM exits. If such an execution neither a fault due to CPL > 0 nor a VM exit, the instruction's behavior may be modified for certain values of ECX: - If ECX contains 79H (indicating IA32_BIOS_UPDT_TRIG MSR), no microcode update is loaded, and control passes to the next instruction. This implies that microcode updates cannot be loaded in VMX non-root operation. toggle quoted message Show quoted text -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t +gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); uint64_t +get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Eddie Dong #28 OK, that explains... It is an VMX limitation... Thanks! Eddie toggle quoted message Show quoted text -----Original Message----- From: Xu, Anthony Sent: Wednesday, March 14, 2018 1:11 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: RE: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Per below spec, microcode update in VMX non-root operation does nothing. Anthony * WRMSR. Section 25.1.3 identifies when executions of the WRMSR instruction cause VM exits. If such an execution neither a fault due to CPL > 0 nor a VM exit, the instruction's behavior may be modified for certain values of ECX: - If ECX contains 79H (indicating IA32_BIOS_UPDT_TRIG MSR), no microcode update is loaded, and control passes to the next instruction. This implies that microcode updates cannot be loaded in VMX non-root operation. -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with +or without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t +gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with +or without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Tian, Kevin #29 From: acrn-dev@... [mailto:acrn- dev@...] On Behalf Of Yin, Fengwei On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. in the future please include your TODO plan in the commit msg so reviewers can save effort on commenting those... Thanks Kevin
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Tian, Kevin #30 From: acrn-dev@... [mailto:acrn- dev@...] On Behalf Of Yin, Fengwei On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) +{ + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? Yes. It's in Table 9-7 of SDM vol 3. Should I add comments for 2000? yes please. +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) +{ + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. For uCode case, there is valid mapping for sure. The uCode itself is loaded by kernel to memory allocated with vmalloc. When the uCode is copied to this memory, SOS kernel setup guest mapping. And then kernel will update MSR_IA32_BIOS_UPDT_TRIG and trap to HV. So, at this moment, the mapping is setup already. why did you take it for sure? hypervisor doesn't control linear mapping inside SOS. there could be software bugs or other issues resulting an invalid mapping. When we give some permission to SOS, it doesn't mean bypass all necessary checks. especially when the mapping is invalid, GPA could be a random number and then if you don't stop immediately the error could be propagated to a more severe one when invalid GPA is further used as input to other operations (e.g. GPA2HVA). + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. Sorry for the variables name which is confusing. hva is host virtual address of v instead of gva (It may be better to change gva to v_aligned). gva is page aligned while hva is not. I will change gva to v_aligned. I didn't get it. gva is page aligned, thus gpa is page aligned thus hva is also page aligned. I didn't see you add additional offset in above translation. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? There is no return value for MSR_IA32_BIOS_UPDT_TRIG update. SOS kernel will query the uCode version again to check whether the uCode update is success or fail. + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? My understanding is we don't need to do anything here. SOS kernel will query uCode version again. If the uCode version read from msr is same as uCode file, that means the uCode update is success. Otherwise, uCode update is fail. if that's the hardware behavior, then current logic is ok. + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? The policy is only allow SOS to do uCode update and deny uCode update for all UOS. allow/deny are all done in interception. above you actually passthrough the msr to UOS which is not a deny policy. It is fine based on Anthony's information, that write to this msr in non-root mode is ignored. In that case you want to add such comment to explain why passthrough of this msr is OK here. Regards Yin, Fengwei + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH 1/2] Add CHECK and CHECK_RET macro Yan, Like #31 Hi Eddie, Current ASSERT dumps both host and guest context already. Yes, the purpose of the patches is to do the ASSERT cleanup. CHECK is not a MUST, just added to replace some ASSERT, which is not necessary or could be triggered by guest. I will list the ASSERTs to be cleared for a discussion before sending more patches. Thanks, Like toggle quoted message Show quoted text -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Thursday, March 15, 2018 2:53 AM To: acrn-dev@... Cc: Yan, Like <like.yan@...>; Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH 1/2] Add CHECK and CHECK_RET macro Hi Like: I felt we have so many error check code now, and it becomes a little over-complicated. May be we can do: 1) Add dump_context in ASSERT. But no need to introduce another one (CHECK) 2) We need to carefully review all the usage of ASSERT and make sure no one will block host if a tamped guest trigger ASSERT. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of like.yan@... Sent: Wednesday, March 14, 2018 6:15 AM To: acrn-dev@... Cc: Yan, Like <like.yan@...>; Yan Subject: [acrn-dev] [PATCH 1/2] Add CHECK and CHECK_RET macro From: "Yan, Like" <like.yan@...> - CHECK macro checks the given condition, if false, print a fatal message, dump both host and guest context and return void; - CHECK_RET macro checks the given condition, if false, print a fatal message, dump both host and guest context and return a given value; - dump_contexts function dump host and guest contexts Change-Id: I482998c0ac7de32474097032abbf3835057c1f21 Signed-off-by: Yan, Like <like.yan@...> --- debug/dump.c \| 10 ++++++++++ include/debug/assert.h \| 33 ++++++++++++++++++++++++++++++--- include/debug/dump.h \| 5 +++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/debug/dump.c b/debug/dump.c index 87fb334..9e02caf 100644 --- a/debug/dump.c +++ b/debug/dump.c @@ -293,6 +293,16 @@ void __assert(uint32_t line, const char file, char txt) } while (1); } +void dump_contexts(void) +{ + uint32_t cpu_id = get_cpu_id(); + uint64_t rsp = cpu_rsp_get(); + uint64_t rbp = cpu_rbp_get(); + + show_host_call_trace(rsp, rbp, cpu_id); + dump_guest_context(cpu_id); +} + void dump_exception(struct intr_ctx ctx, uint32_t cpu_id) { const char name = "Not defined"; diff --git a/include/debug/assert.h b/include/debug/assert.h index 83b26c1..b777fdd 100644 --- a/include/debug/assert.h +++ b/include/debug/assert.h @@ -34,18 +34,45 @@ #ifdef HV_DEBUG void __assert(uint32_t line, const char file, char txt); -#define ASSERT(x, ...) \ - if (!(x)) {\ - pr_fatal(__VA_ARGS__);\ +#define ASSERT(x, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ __assert(__LINE__, __FILE__, "fatal error");\ } + +#define CHECK(x, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + dump_contexts(); \ + return ; \ + } + +#define CHECK_RET(x, r, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + dump_contexts(); \ + return (r); \ + } #else #define ASSERT(x, ...) \ if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ do { \ asm volatile ("pause" ::: "memory"); \ } while (1); \ } + +#define CHECK(x, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + return ; \ + } + +#define CHECK_RET(x, r, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + return (r); \ + } #endif /* Force a compilation error if condition is false / diff --git a/include/debug/dump.h b/include/debug/dump.h index 1d3ebf3..d38415a 100644 --- a/include/debug/dump.h +++ b/include/debug/dump.h @@ -37,11 +37,16 @@ struct intr_ctx; #define CALL_TRACE_HIERARCHY_MAX 20 #define DUMP_STACK_SIZE 0x200 +void dump_contexts(void); void dump_exception(struct intr_ctx ctx, uint32_t cpu_id); void dump_interrupt(struct intr_ctx ctx); #else +static inline void dump_contexts(void) { } + static inline void dump_exception(__unused struct intr_ctx ctx, __unused uint32_t cpu_id) { -- 2.7.4
More All Messages By This Member
Re: [PATCH 1/2] Add CHECK and CHECK_RET macro Tian, Kevin #32 I'm not sure whether new MACROs are really required. the readability is worse than explicitly checking error and return in caller place. Looks the main addition is to dump guest context upon those errors, which is not necessary since as long as system is not hang there are many methods to dump guest contexts. dump context is required upon ASSERT is because system is hang thus we want to know the lastest context at that time. So I'd suggest to simply replace all ASSERTS, which if possible caused by guest, with explicit error check and return. toggle quoted message Show quoted text From: acrn-dev@... [mailto:acrn- dev@...] On Behalf Of Yan, Like Hi Eddie, Current ASSERT dumps both host and guest context already. Yes, the purpose of the patches is to do the ASSERT cleanup. CHECK is not a MUST, just added to replace some ASSERT, which is not necessary or could be triggered by guest. I will list the ASSERTs to be cleared for a discussion before sending more patches. Thanks, Like -----Original Message----- From: acrn-dev@... [mailto:acrn- dev@...] On Behalf Of Eddie Dong Sent: Thursday, March 15, 2018 2:53 AM To: acrn-dev@... Cc: Yan, Like <like.yan@...>; Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH 1/2] Add CHECK and CHECK_RET macro Hi Like: I felt we have so many error check code now, and it becomes a little over-complicated. May be we can do: 1) Add dump_context in ASSERT. But no need to introduce another one (CHECK) 2) We need to carefully review all the usage of ASSERT and make sure no one will block host if a tamped guest trigger ASSERT. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of like.yan@... Sent: Wednesday, March 14, 2018 6:15 AM To: acrn-dev@... Cc: Yan, Like <like.yan@...>; Yan Subject: [acrn-dev] [PATCH 1/2] Add CHECK and CHECK_RET macro From: "Yan, Like" <like.yan@...> - CHECK macro checks the given condition, if false, print a fatal message, dump both host and guest context and return void; - CHECK_RET macro checks the given condition, if false, print a fatal message, dump both host and guest context and return a given value; - dump_contexts function dump host and guest contexts Change-Id: I482998c0ac7de32474097032abbf3835057c1f21 Signed-off-by: Yan, Like <like.yan@...> --- debug/dump.c \| 10 ++++++++++ include/debug/assert.h \| 33 ++++++++++++++++++++++++++++++--- include/debug/dump.h \| 5 +++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/debug/dump.c b/debug/dump.c index 87fb334..9e02caf 100644 --- a/debug/dump.c +++ b/debug/dump.c @@ -293,6 +293,16 @@ void __assert(uint32_t line, const char file, char txt) } while (1); } +void dump_contexts(void) +{ + uint32_t cpu_id = get_cpu_id(); + uint64_t rsp = cpu_rsp_get(); + uint64_t rbp = cpu_rbp_get(); + + show_host_call_trace(rsp, rbp, cpu_id); + dump_guest_context(cpu_id); +} + void dump_exception(struct intr_ctx ctx, uint32_t cpu_id) { const char name = "Not defined"; diff --git a/include/debug/assert.h b/include/debug/assert.h index 83b26c1..b777fdd 100644 --- a/include/debug/assert.h +++ b/include/debug/assert.h @@ -34,18 +34,45 @@ #ifdef HV_DEBUG void __assert(uint32_t line, const char file, char txt); -#define ASSERT(x, ...) \ - if (!(x)) {\ - pr_fatal(__VA_ARGS__);\ +#define ASSERT(x, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ __assert(__LINE__, __FILE__, "fatal error");\ } + +#define CHECK(x, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + dump_contexts(); \ + return ; \ + } + +#define CHECK_RET(x, r, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + dump_contexts(); \ + return (r); \ + } #else #define ASSERT(x, ...) \ if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ do { \ asm volatile ("pause" ::: "memory"); \ } while (1); \ } + +#define CHECK(x, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + return ; \ + } + +#define CHECK_RET(x, r, ...) \ + if (!(x)) { \ + pr_fatal(__VA_ARGS__); \ + return (r); \ + } #endif /* Force a compilation error if condition is false / diff --git a/include/debug/dump.h b/include/debug/dump.h index 1d3ebf3..d38415a 100644 --- a/include/debug/dump.h +++ b/include/debug/dump.h @@ -37,11 +37,16 @@ struct intr_ctx; #define CALL_TRACE_HIERARCHY_MAX 20 #define DUMP_STACK_SIZE 0x200 +void dump_contexts(void); void dump_exception(struct intr_ctx ctx, uint32_t cpu_id); void dump_interrupt(struct intr_ctx ctx); #else +static inline void dump_contexts(void) { } + static inline void dump_exception(__unused struct intr_ctx ctx, __unused uint32_t cpu_id) { -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Yin, Fengwei <fengwei.yin@...> #33 Hi Kevin, -------------------8<----------------------------------- +uint64_t get_microcode_version(void) +{ + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? Yes. It's in Table 9-7 of SDM vol 3. Should I add comments for 2000? yes please. OK. Will add comments. +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) +{ + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + /* gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. For uCode case, there is valid mapping for sure. The uCode itself is loaded by kernel to memory allocated with vmalloc. When the uCode is copied to this memory, SOS kernel setup guest mapping. And then kernel will update MSR_IA32_BIOS_UPDT_TRIG and trap to HV. So, at this moment, the mapping is setup already. why did you take it for sure? hypervisor doesn't control linear mapping inside SOS. there could be software bugs or other issues resulting an invalid mapping. When we give some permission to SOS, it doesn't mean bypass all necessary checks. especially when the mapping is invalid, GPA could be a random number and then if you don't stop immediately the error could be propagated to a more severe one when invalid GPA is further used as input to other operations (e.g. GPA2HVA). OK. I will add the return value check for gva2gpa and GPA2HVA. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. Sorry for the variables name which is confusing. hva is host virtual address of v instead of gva (It may be better to change gva to v_aligned). gva is page aligned while hva is not. I will change gva to v_aligned. I didn't get it. gva is page aligned, thus gpa is page aligned thus hva is also page aligned. I didn't see you add additional offset in above translation. Here we have: hva = GPA2HVA(gva2gpa(v)) gva = v & ~(CPU_PAGE_SIZE - 1) But not: gva = GPA2HVA(gva2gpa(hva)) I will change gva name to v_aligned to avoid the confusion. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? There is no return value for MSR_IA32_BIOS_UPDT_TRIG update. SOS kernel will query the uCode version again to check whether the uCode update is success or fail. + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? My understanding is we don't need to do anything here. SOS kernel will query uCode version again. If the uCode version read from msr is same as uCode file, that means the uCode update is success. Otherwise, uCode update is fail. if that's the hardware behavior, then current logic is ok. It's in SDM vol3 9.11.7. + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? The policy is only allow SOS to do uCode update and deny uCode update for all UOS. allow/deny are all done in interception. above you actually passthrough the msr to UOS which is not a deny policy. It is fine based on Anthony's information, that write to this msr in non-root mode is ignored. In that case you want to add such comment to explain why passthrough of this msr is OK here. I will add comments to explain why passthru msr to UOS. Regards Yin, Fengwei Regards Yin, Fengwei + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Yin, Fengwei <fengwei.yin@...> #34 Hi Anthony, On Wed, Mar 14, 2018 at 08:10:45PM +0000, Xu, Anthony wrote: Per below spec, microcode update in VMX non-root operation does nothing. Anthony * WRMSR. Section 25.1.3 identifies when executions of the WRMSR instruction cause VM exits. If such an execution neither a fault due to CPL > 0 nor a VM exit, the instruction's behavior may be modified for certain values of ECX: - If ECX contains 79H (indicating IA32_BIOS_UPDT_TRIG MSR), no microcode update is loaded, and control passes to the next instruction. This implies that microcode updates cannot be loaded in VMX non-root operation. Thanks a lot for the information. Regards Yin, Fengwei -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t +gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or +without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); uint64_t +get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Yin, Fengwei <fengwei.yin@...> #35 On Wed, Mar 14, 2018 at 07:36:58PM +0000, Eddie Dong wrote: Make sence.. -----Original Message----- From: Xu, Anthony Sent: Wednesday, March 14, 2018 12:03 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: RE: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Then we probably want to wait until SOS have all CPUs. What about we do uCode update for all pCPUs when SOS is trying ot update uCode? SOS kernel could trigger uCode update after the pCPUs is assigned to UOS. At that situation, SOS can't request to update uCode for all pCPUs. Or are you suggest we delay uCode update enabling till SOS could own all pCPUs (like core sharing is ready)? Regards Yin, Fengwei Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with +or without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t +gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with +or without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Yin, Fengwei <fengwei.yin@...> #36 On Thu, Mar 15, 2018 at 12:30:41AM +0000, Tian, Kevin wrote: From: acrn-dev@... [mailto:acrn- dev@...] On Behalf Of Yin, Fengwei On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. in the future please include your TODO plan in the commit msg so reviewers can save effort on commenting those... Sure. Regards Yin, Fengwei Thanks Kevin
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Yin, Fengwei <fengwei.yin@...> #37 On 3/15/2018 9:29 AM, Yin, Fengwei wrote: On Wed, Mar 14, 2018 at 07:36:58PM +0000, Eddie Dong wrote: Make sence.. OK. Will hold the patch till the SOS could own all CPUs. Thanks. Regards Yin, Fengwei -----Original Message----- From: Xu, Anthony Sent: Wednesday, March 14, 2018 12:03 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: RE: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Then we probably want to wait until SOS have all CPUs. What about we do uCode update for all pCPUs when SOS is trying ot update uCode? SOS kernel could trigger uCode update after the pCPUs is assigned to UOS. At that situation, SOS can't request to update uCode for all pCPUs. Or are you suggest we delay uCode update enabling till SOS could own all pCPUs (like core sharing is ready)? Regards Yin, Fengwei Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with +or without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> +#include <hv_arch.h> +#include <hv_debug.h> +#include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t +gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with +or without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Eddie Dong #38 I think we want to let SOS own all the PCPU at boot time, and then offline the cores not necessary... Thx Eddie toggle quoted message Show quoted text -----Original Message----- From: Yin, Fengwei Sent: Wednesday, March 14, 2018 6:30 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 07:36:58PM +0000, Eddie Dong wrote: Make sence.. -----Original Message----- From: Xu, Anthony Sent: Wednesday, March 14, 2018 12:03 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: RE: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Then we probably want to wait until SOS have all CPUs. What about we do uCode update for all pCPUs when SOS is trying ot update uCode? SOS kernel could trigger uCode update after the pCPUs is assigned to UOS. At that situation, SOS can't request to update uCode for all pCPUs. Or are you suggest we delay uCode update enabling till SOS could own all pCPUs (like core sharing is ready)? Regards Yin, Fengwei Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, +with or without + * modification, are permitted provided that the +following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY + OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> #include <hv_arch.h> #include +<hv_debug.h> #include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, +&ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, +uint64_t gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, +with or without + * modification, are permitted provided that the +following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY + OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Yin, Fengwei <fengwei.yin@...> #39 On 3/15/2018 1:17 PM, Dong, Eddie wrote: I think we want to let SOS own all the PCPU at boot time, and then offline the cores not necessary... There is one case of uCode update which is triggered by writing 1 to /sys/devices/system/cpu/microcode/reload. In this case, uCode update could be triggered after SOS offline the cores which is used by UOS. And only pCPUs owned by SOS (not all pCPUs) get uCode updated. Regards Yin, Fengwei Thx Eddie -----Original Message----- From: Yin, Fengwei Sent: Wednesday, March 14, 2018 6:30 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 07:36:58PM +0000, Eddie Dong wrote: Make sence.. -----Original Message----- From: Xu, Anthony Sent: Wednesday, March 14, 2018 12:03 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: RE: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Then we probably want to wait until SOS have all CPUs. What about we do uCode update for all pCPUs when SOS is trying ot update uCode? SOS kernel could trigger uCode update after the pCPUs is assigned to UOS. At that situation, SOS can't request to update uCode for all pCPUs. Or are you suggest we delay uCode update enabling till SOS could own all pCPUs (like core sharing is ready)? Regards Yin, Fengwei Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, +with or without + * modification, are permitted provided that the +following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY + OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> #include <hv_arch.h> #include +<hv_debug.h> #include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, +&ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, +uint64_t gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, +with or without + * modification, are permitted provided that the +following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY + OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member
Re: [PATCH v2] microcode: Enable microcode update from SOS. Eddie Dong #40 This is fine. The SOS knows the cores are offlined already, so the user should not trigger update at that point. toggle quoted message Show quoted text -----Original Message----- From: Yin, Fengwei Sent: Wednesday, March 14, 2018 10:23 PM To: Dong, Eddie <eddie.dong@...>; acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On 3/15/2018 1:17 PM, Dong, Eddie wrote: I think we want to let SOS own all the PCPU at boot time, and then offline the cores not necessary... There is one case of uCode update which is triggered by writing 1 to /sys/devices/system/cpu/microcode/reload. In this case, uCode update could be triggered after SOS offline the cores which is used by UOS. And only pCPUs owned by SOS (not all pCPUs) get uCode updated. Regards Yin, Fengwei Thx Eddie -----Original Message----- From: Yin, Fengwei Sent: Wednesday, March 14, 2018 6:30 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 07:36:58PM +0000, Eddie Dong wrote: Make sence.. -----Original Message----- From: Xu, Anthony Sent: Wednesday, March 14, 2018 12:03 PM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: RE: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Then we probably want to wait until SOS have all CPUs. What about we do uCode update for all pCPUs when SOS is trying ot update uCode? SOS kernel could trigger uCode update after the pCPUs is assigned to UOS. At that situation, SOS can't request to update uCode for all pCPUs. Or are you suggest we delay uCode update enabling till SOS could own all pCPUs (like core sharing is ready)? Regards Yin, Fengwei Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 11:57 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. SOS will have all CPUS at boot time, so this is not a problem. Enabling uCode update is good to me. BIOS update could be, but OS update is also useful. The only issue to me is that why we cannot rely on passthru of the MSR registers. But anyway, it is fine for now. Thx Eddie -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Xu, Anthony Sent: Wednesday, March 14, 2018 11:47 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. The platform might end up with multiple microcode versions with the patch, which is not what we want. Should we support microcode update in embedded HV/SOS? is run time microcode update needed in embedded system? If no, don't support microcode update in HV or SOS, user can update microcode in BIOS/EFI or native OS. If yes, Need to update microcode on all CPUS. Anthony -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Eddie Dong Sent: Wednesday, March 14, 2018 7:59 AM To: acrn-dev@... Cc: Dong, Eddie <eddie.dong@...> Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. Acked by <Eddie.dong@...> Leave to Anthony... -----Original Message----- From: acrn-dev@... [mailto:acrn-dev@...] On Behalf Of Yin, Fengwei Sent: Wednesday, March 14, 2018 3:28 AM To: acrn-dev@... Subject: Re: [acrn-dev] [PATCH v2] microcode: Enable microcode update from SOS. On Wed, Mar 14, 2018 at 10:17:53AM +0000, Tian, Kevin wrote: From: Yin, Fengwei Sent: Wednesday, March 14, 2018 4:14 PM From: Yin Fengwei <fengwei.yin@...> microcode update from UOS is disabled. microcode version checking is available for both SOS and UOS. there is a problem with current implementation. microcode needs being updated on all CPU cores, however SOS only sees partial. If there is no plan to introduce a hypercall, we need hook on the 1st MSR write from SOS and then IPI to all physical cores to do update, and then simply report success for subsequent MSR writes from SOS. Yes. It's on my TODO list. Because we only allow SOS to update uCode, only pCPU running SOS get uCode updated. We try not to change kernel source. So I prefer IPI approach( cpu offline depends on the IPI also) and plan to address all pCPU uCode update once we have IPI call ready. Regards Yin, Fengwei Signed-off-by: Yin Fengwei <fengwei.yin@...> --- Changes in v2: - Not clear the bit63 of the address of uCode. Makefile \| 1 + arch/x86/guest/instr_emul_wrapper.h \| 1 - arch/x86/guest/ucode.c \| 94 +++++++++++++++++++++++++++++++++++++ arch/x86/guest/vmsr.c \| 23 +++++++++ include/arch/x86/guest/guest.h \| 1 + include/arch/x86/guest/ucode.h \| 50 ++++++++++++++++++++ 6 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/x86/guest/ucode.c create mode 100644 include/arch/x86/guest/ucode.h diff --git a/Makefile b/Makefile index c35b705..13e408b 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ C_SRCS += arch/x86/guest/vpic.c C_SRCS += arch/x86/guest/vmsr.c C_SRCS += arch/x86/guest/vioapic.c C_SRCS += arch/x86/guest/instr_emul.c +C_SRCS += arch/x86/guest/ucode.c C_SRCS += lib/spinlock.c C_SRCS += lib/udelay.c C_SRCS += lib/strnlen.c diff --git a/arch/x86/guest/instr_emul_wrapper.h b/arch/x86/guest/instr_emul_wrapper.h index 3581e9b..48e311f 100644 --- a/arch/x86/guest/instr_emul_wrapper.h +++ b/arch/x86/guest/instr_emul_wrapper.h @@ -200,4 +200,3 @@ int vm_get_seg_desc(struct vcpu vcpu, int reg, int vm_set_seg_desc(struct vcpu vcpu, int reg, struct seg_desc desc); int vm_restart_instruction(struct vcpu vcpu); -void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, uint64_t gpa); diff --git a/arch/x86/guest/ucode.c b/arch/x86/guest/ucode.c new file mode 100644 index 0000000..44396dc --- /dev/null +++ b/arch/x86/guest/ucode.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with +or without + * modification, are permitted provided that the following +conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + / + +#include <hv_lib.h> +#include <acrn_common.h> #include <hv_arch.h> #include +<hv_debug.h> #include <ucode.h> + +uint64_t get_microcode_version(void) { + uint64_t val; + uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; + + msr_write(MSR_IA32_BIOS_SIGN_ID, 0); + native_cpuid_count(CPUID_FEATURES, 0, &eax, &ebx, &ecx, &edx); + val = msr_read(MSR_IA32_BIOS_SIGN_ID); + + return val; +} + +#define GET_DATA_SIZE(hdptr) ((hdptr)->data_size ? : 2000) is 0x2000 defined by SDM? +void acrn_update_ucode(struct vcpu vcpu, uint64_t v) { + uint64_t hva, gpa, gva; + struct ucode_header uhdr; + int data_size, data_page_num; + uint8_t ptr, ucode_ptr; + + / gva will be used as iterator to copy uCode to HV buffer. + * It will be aligned to CPU_PAGE_SIZE of v, increased + * by CPU_PAGE_SIZE and convert to hva for each step. + / + gva = v & ~(CPU_PAGE_SIZE - 1); + + vm_gva2gpa(vcpu, v, &gpa); we shouldn't assume there is always an valid mapping here. I know it's not this patch specific, as gva2gpa doesn't return any error today. but it should be a cleanup task to emulate hardware behavior, i.e. guest page table walking error should trigger exceptions back to guest according to SDM. + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + + / hva is used to get uhdr here / + uhdr = (struct ucode_header )(hva - sizeof(struct +ucode_header)); hva is already page aligned, meaning uhdr is in another page. there is no guarantee that HPAs of two guest pages are contiguous. you need walk guest pgtable again as long as page boundary is crossed. + + data_size = GET_DATA_SIZE(uhdr) + sizeof(struct ucode_header) + + ((uint64_t)uhdr & (CPU_PAGE_SIZE - 1)); + data_page_num = + (data_size + CPU_PAGE_SIZE - 1) >> CPU_PAGE_SHIFT; + + ptr = ucode_ptr = alloc_pages(data_page_num); + if (ptr == NULL) + return; do we need an error value returned here to emulate any expected hardware behavior to SOS, e.g. changing of some bit in MSR? + + hva = hva & ~(CPU_PAGE_SIZE - 1); + while (data_page_num--) { + memcpy_s(ptr, CPU_PAGE_SIZE, (void )hva, CPU_PAGE_SIZE); + + ptr += CPU_PAGE_SIZE; + gva += CPU_PAGE_SIZE; + + vm_gva2gpa(vcpu, gva, &gpa); + hva = (uint64_t)GPA2HVA(vcpu->vm, gpa); + } + + msr_write(MSR_IA32_BIOS_UPDT_TRIG, + (uint64_t)ucode_ptr + (v & (CPU_PAGE_SIZE - 1))); + get_microcode_version(); if microcode update fails, what should be done here? + + free(ucode_ptr); +} diff --git a/arch/x86/guest/vmsr.c b/arch/x86/guest/vmsr.c index edc456c..b371223 100644 --- a/arch/x86/guest/vmsr.c +++ b/arch/x86/guest/vmsr.c @@ -32,10 +32,13 @@ #include <acrn_common.h> #include <hv_arch.h> #include <hv_debug.h> +#include <ucode.h> /MRS need to be emulated, the order in this array better as freq of ops/ static const uint32_t emulated_msrs[] = { MSR_IA32_TSC_DEADLINE, / Enable TSC_DEADLINE VMEXIT / + MSR_IA32_BIOS_SIGN_ID, / Enable MSR_IA32_BIOS_SIGN_ID / + / following MSR not emulated now / / @@ -51,6 +54,7 @@ static const uint32_t emulated_msrs[] = { /* the index is matched with emulated msrs array/ enum { IDX_TSC_DEADLINE, + IDX_BIOS_SIGN_ID, IDX_MAX_MSR }; @@ -129,6 +133,11 @@ void init_msr_emulation(struct vcpu vcpu) for (i = 0; i < msrs_count; i++) enable_msr_interception(msr_bitmap, emulated_msrs[i]); + /* Only enable uCode update for SOS / + if (is_vm0(vcpu->vm)) + enable_msr_interception(msr_bitmap, + MSR_IA32_BIOS_UPDT_TRIG); why only intercept for vm0? do you want other vm to trigger update? + / below MSR protected from guest OS, if access to inject gp/ enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_CAP); enable_msr_interception(msr_bitmap, MSR_IA32_MTRR_DEF_TYPE); @@ -185,6 +194,11 @@ int rdmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + v = get_microcode_version(); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: @@ -273,6 +287,15 @@ int wrmsr_handler(struct vcpu vcpu) vcpu_inject_gp(vcpu); break; } + case MSR_IA32_BIOS_SIGN_ID: + { + break; + } + case MSR_IA32_BIOS_UPDT_TRIG: + { + acrn_update_ucode(vcpu, v); + break; + } /* following MSR not emulated now just left for future / case MSR_IA32_SYSENTER_CS: diff --git a/include/arch/x86/guest/guest.h b/include/arch/x86/guest/guest.h index 683433f..2649263 100644 --- a/include/arch/x86/guest/guest.h +++ b/include/arch/x86/guest/guest.h @@ -85,6 +85,7 @@ uint64_t vcpumask2pcpumask(struct vm vm, uint64_t vdmask); int init_vm0_boot_info(struct vm vm); uint64_t gva2gpa(struct vm vm, uint64_t cr3, uint64_t gva); +void vm_gva2gpa(struct vcpu vcpu, uint64_t gla, +uint64_t gpa); struct vcpu get_primary_vcpu(struct vm vm); struct vcpu vcpu_from_vid(struct vm vm, int vcpu_id); diff --git a/include/arch/x86/guest/ucode.h b/include/arch/x86/guest/ucode.h new file mode 100644 index 0000000..a49c43b --- /dev/null +++ b/include/arch/x86/guest/ucode.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018 Intel Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, +with or without + * modification, are permitted provided that the +following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Intel Corporation nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY + OF SUCH DAMAGE. + / + +#ifndef _ARCH_X86_UCODE_H +#define _ARCH_X86_UCODE_H + +struct ucode_header { + uint32_t header_ver; + uint32_t update_ver; + uint32_t date; + uint32_t proc_sig; + uint32_t checksum; + uint32_t loader_ver; + uint32_t proc_flags; + uint32_t data_size; + uint32_t total_size; + uint32_t reserved[3]; +}; + +void acrn_update_ucode(struct vcpu vcpu, uint64_t v); +uint64_t get_microcode_version(void); + +#endif -- 2.7.4
More All Messages By This Member

21 - 40 of 37371

Home Hashtags Subgroups Terms