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?

+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.

+ 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.

+
+ 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.

+
+ 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.

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