The monitor will coredump if we use 'info sgx' in the '-machine none' or
other non-x86 platform, add the sgx_enabled bool variable to avoid this
coredump issue.

./qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -machine none,accel=kvm -monitor stdio
QEMU 6.1.50 monitor - type 'help' for more information
(qemu) info sgx
/home/vmm/project/upstream/9-8/qemu/include/hw/i386/x86.h:93:X86_MACHINE: Object 0x5615d71a6fd0 is not an instance of type x86-machine
Aborted (core dumped)

Signed-off-by: Yang Zhong <yang.zhong@intel.com>
---
 hw/i386/sgx.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
index 854532fb98..118126fc70 100644
--- a/hw/i386/sgx.c
+++ b/hw/i386/sgx.c
@@ -27,6 +27,8 @@
 #define SGX_CPUID_EPC_SECTION           0x1
 #define SGX_CPUID_EPC_MASK              GENMASK(3, 0)
 
+bool sgx_enabled;
+
 static uint64_t sgx_calc_section_metric(uint64_t low, uint64_t high)
 {
     return (low & GENMASK_ULL(31, 12)) +
@@ -88,19 +90,22 @@ SGXInfo *sgx_get_capabilities(Error **errp)
 SGXInfo *sgx_get_info(void)
 {
     SGXInfo *info = NULL;
-    MachineState *ms = MACHINE(qdev_get_machine());
-    X86MachineState *x86ms = X86_MACHINE(qdev_get_machine());
-
-    if (x86ms->sgx_epc_list) {
-        PCMachineState *pcms = PC_MACHINE(ms);
-        SGXEPCState *sgx_epc = &pcms->sgx_epc;
-        info = g_new0(SGXInfo, 1);
-
-        info->sgx = true;
-        info->sgx1 = true;
-        info->sgx2 = true;
-        info->flc = true;
-        info->section_size = sgx_epc->size;
+
+    if (sgx_enabled) {
+        MachineState *ms = MACHINE(qdev_get_machine());
+        X86MachineState *x86ms = X86_MACHINE(qdev_get_machine());
+
+        if (x86ms->sgx_epc_list) {
+            PCMachineState *pcms = PC_MACHINE(ms);
+            SGXEPCState *sgx_epc = &pcms->sgx_epc;
+            info = g_new0(SGXInfo, 1);
+
+            info->sgx = true;
+            info->sgx1 = true;
+            info->sgx2 = true;
+            info->flc = true;
+            info->section_size = sgx_epc->size;
+        }
     }
     return info;
 }
@@ -168,4 +173,5 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
     }
 
     memory_region_set_size(&sgx_epc->mr, sgx_epc->size);
+    sgx_enabled = true;
 }

On 9/8/21 10:19 AM, Yang Zhong wrote:
> The monitor will coredump if we use 'info sgx' in the '-machine none' or
> other non-x86 platform, add the sgx_enabled bool variable to avoid this
> coredump issue.

But this code is only built for x86...

> 
> ./qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -machine none,accel=kvm -monitor stdio
> QEMU 6.1.50 monitor - type 'help' for more information
> (qemu) info sgx
> /home/vmm/project/upstream/9-8/qemu/include/hw/i386/x86.h:93:X86_MACHINE: Object 0x5615d71a6fd0 is not an instance of type x86-machine
> Aborted (core dumped)

Paolo suggested on IRC to use object_class_dynamic_cast().

> 
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> ---
>  hw/i386/sgx.c | 32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)

On 08/09/21 10:19, Yang Zhong wrote:
> +    if (sgx_enabled) {
> +        MachineState *ms = MACHINE(qdev_get_machine());
> +        X86MachineState *x86ms = X86_MACHINE(qdev_get_machine());
> +

You can use object_dynamic_cast(ms, TYPE_PC_MACHINE), something like:


PCMachineState *pcms =
     object_dynamic_cast(qdev_get_machine(), TYPE_PC_MACHINE);
X86MachineState *x86ms;

if (!pcms) {
     return NULL;
}
x86ms = X86_MACHINE(pcms);
if (!x86ms->sgx_epc_list) {
     return NULL;
}
info = g_new0(SGXInfo, 1);
...
return info;

Also please squash this patch directly into patch 3 that introduces 
sgx_get_info.

Paolo

On Wed, Sep 08, 2021 at 10:54:06AM +0200, Paolo Bonzini wrote:
> On 08/09/21 10:19, Yang Zhong wrote:
> >+    if (sgx_enabled) {
> >+        MachineState *ms = MACHINE(qdev_get_machine());
> >+        X86MachineState *x86ms = X86_MACHINE(qdev_get_machine());
> >+
> 
> You can use object_dynamic_cast(ms, TYPE_PC_MACHINE), something like:
> 
> 
> PCMachineState *pcms =
>     object_dynamic_cast(qdev_get_machine(), TYPE_PC_MACHINE);
> X86MachineState *x86ms;
> 
> if (!pcms) {
>     return NULL;
> }
> x86ms = X86_MACHINE(pcms);
> if (!x86ms->sgx_epc_list) {
>     return NULL;
> }
> info = g_new0(SGXInfo, 1);
> ...
> return info;
> 
> Also please squash this patch directly into patch 3 that introduces
> sgx_get_info.
>

  Paolo, Good suggestion! This solution can avoid coredump. thanks!

  Yang 

   
> Paolo