* Re: [PATCH] target/i386: do not crash if microvm guest uses SGX CPUID leaves
2024-07-16 16:55 [PATCH] target/i386: do not crash if microvm guest uses SGX CPUID leaves Paolo Bonzini
@ 2024-07-17 3:00 ` Zhao Liu
0 siblings, 0 replies; 2+ messages in thread
From: Zhao Liu @ 2024-07-17 3:00 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-stable, Zhao Liu
Hi Paolo,
On Tue, Jul 16, 2024 at 06:55:30PM +0200, Paolo Bonzini wrote:
> Date: Tue, 16 Jul 2024 18:55:30 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH] target/i386: do not crash if microvm guest uses SGX CPUID
> leaves
> X-Mailer: git-send-email 2.45.2
>
> sgx_epc_get_section assumes a PC platform is in use:
>
> bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
> {
> PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
>
> However, sgx_epc_get_section is called by CPUID regardless of whether
> SGX state has been initialized or which platform is in use. Check
> whether the machine has the right QOM class and if not behave as if
> there are no EPC sections.
>
> Fixes: 1dec2e1f19f ("i386: Update SGX CPUID info according to hardware/KVM/user input", 2021-09-30)
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2142
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/i386/sgx.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> index de76397bcfb..25b2055d653 100644
> --- a/hw/i386/sgx.c
> +++ b/hw/i386/sgx.c
> @@ -266,10 +266,12 @@ void hmp_info_sgx(Monitor *mon, const QDict *qdict)
>
> bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
> {
> - PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> + PCMachineState *pcms =
> + (PCMachineState *)object_dynamic_cast(qdev_get_machine(),
> + TYPE_PC_MACHINE);
> SGXEPCDevice *epc;
>
> - if (pcms->sgx_epc.size == 0 || pcms->sgx_epc.nr_sections <= section_nr) {
> + if (!pcms || pcms->sgx_epc.size == 0 || pcms->sgx_epc.nr_sections <= section_nr) {
> return true;
> }
>
The above change is necessary...
...but it only avoids encoding sub-leafs CPUID.0x12.{0x2..N}, while
subleafs CPUID.0x12.{0x0,0x1} still have valid SGX information.
According to the error message in qmp_query_sgx(), sgx is only supported
on PC machines. So how about simply taking it a step further and masking
out the entire 0x12 leaf for microvm as well?
for example:
diff --git a/hw/i386/sgx-stub.c b/hw/i386/sgx-stub.c
index 16b1dfd90bb5..38ff75e9f377 100644
--- a/hw/i386/sgx-stub.c
+++ b/hw/i386/sgx-stub.c
@@ -32,6 +32,11 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
memset(&pcms->sgx_epc, 0, sizeof(SGXEPCState));
}
+bool check_sgx_support(void)
+{
+ return false;
+}
+
bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
{
return true;
diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
index de76397bcfb3..dcf178b1e755 100644
--- a/hw/i386/sgx.c
+++ b/hw/i386/sgx.c
@@ -264,6 +264,14 @@ void hmp_info_sgx(Monitor *mon, const QDict *qdict)
size);
}
+bool check_sgx_support(void)
+{
+ if(!object_dynamic_cast(qdev_get_machine(), TYPE_X86_MACHINE)) {
+ return false;
+ }
+ return true;
+}
+
bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
{
PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
diff --git a/include/hw/i386/sgx-epc.h b/include/hw/i386/sgx-epc.h
index 3e00efd870c9..41d55da47999 100644
--- a/include/hw/i386/sgx-epc.h
+++ b/include/hw/i386/sgx-epc.h
@@ -58,6 +58,7 @@ typedef struct SGXEPCState {
int nr_sections;
} SGXEPCState;
+bool check_sgx_support(void);
bool sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size);
void sgx_epc_build_srat(GArray *table_data);
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c05765eeafc8..f0b464f7ea79 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6702,7 +6702,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
case 0x12:
#ifndef CONFIG_USER_ONLY
if (!kvm_enabled() ||
- !(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_SGX)) {
+ !(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_SGX) ||
+ !check_sgx_support()) {
*eax = *ebx = *ecx = *edx = 0;
break;
}
^ permalink raw reply related [flat|nested] 2+ messages in thread