[Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR

[Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR

[chao.qin]

From: Qin Chao <chao.qin@intel.com>

Emulation of IA32_APIC_BASE MSR in HAXM is not correct, such as bit
8, which is BSP flag and should be set to 1 for the bootstrap
processor and set to 0 for the application processors, but it's set
to 0 for all processors in HAXM. So guest OSes that expect a valid
BSP flag, such as Zircon (the core of Google Fuchsia OS), cannot
boot with "-accel hax". To solve this problem, HAXM (which lacks
APIC virtualization) and QEMU must notify each other of any change
to guest IA32_APIC_BASE MSR. The HAXM patch has been merged into
HAXM source. QEMU needs to use the new HAXM API (apic_base in
"struct hax_tunnel") to initialize the guest IA32_APIC_BASE MSR,
and then, update its own copy at every return from
HAX_VCPU_IOCTL_RUN.

There will be a backward compatility issue caused by the new field
"apic_base" added into "struct hax_tunnel". In order to fix the
problem, the validation for size of "struct hax_tunnel" is removed
and a new capability flag "HAX_CAP_TUNNEL_PAGE" is added, which
means that one page (4KB) is allocated in HAXM kernel to store
"struct hax_tunnel", instead of the size of "struct hax_tunnel".

Change-Id: I8505bc1d75c495dd2765e581d6014125dcb538f3
Signed-off-by: Qin Chao <chao.qin@intel.com>
---
 target/i386/hax-all.c       | 24 +++++++++++++++++++-----
 target/i386/hax-darwin.c    |  6 ------
 target/i386/hax-i386.h      |  2 +-
 target/i386/hax-interface.h |  3 +++
 target/i386/hax-windows.c   |  5 -----
 5 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index cad7531..6a840d9 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -62,11 +62,6 @@ int hax_enabled(void)
     return hax_allowed;
 }
 
-int valid_hax_tunnel_size(uint16_t size)
-{
-    return size >= sizeof(struct hax_tunnel);
-}
-
 hax_fd hax_vcpu_get_fd(CPUArchState *env)
 {
     struct hax_vcpu_state *vcpu = ENV_GET_CPU(env)->hax_vcpu;
@@ -104,6 +99,7 @@ static int hax_get_capability(struct hax_state *hax)
     }
 
     hax->supports_64bit_ramblock = !!(cap->winfo & HAX_CAP_64BIT_RAMBLOCK);
+    hax->supports_tunnel_page = !!(cap->winfo & HAX_CAP_TUNNEL_PAGE);
 
     if (cap->wstatus & HAX_CAP_MEMQUOTA) {
         if (cap->mem_quota < hax->mem_quota) {
@@ -520,6 +516,21 @@ static int hax_vcpu_hax_exec(CPUArchState *env)
         cpu_exec_end(cpu);
         qemu_mutex_lock_iothread();
 
+        /*
+         * Every time HAXM exits to QEMU, sync IA32_APIC_BASE MSR from HAXM and
+         * pass it to the emulated APIC.
+         */
+        if (hax_global.supports_tunnel_page) {
+            /*
+             * ht->apic_base is not available in HAXM kernel module if HAXM does
+             * not support HAX_CAP_SUPPORT_TUNNEL_PAGE.
+             * TODO: HAX_CAP_SUPPORT_TUNNEL_PAGE is used for backward
+             * compatibility with HAXM kernel module. Remove this check when we
+             * drop support for HAXM versions that lack this feature.
+             */
+            cpu_set_apic_base(x86_cpu->apic_state, ht->apic_base);
+        }
+
         /* Simply continue the vcpu_run if system call interrupted */
         if (hax_ret == -EINTR || hax_ret == -EAGAIN) {
             DPRINTF("io window interrupted\n");
@@ -933,6 +944,9 @@ static int hax_set_msrs(CPUArchState *env)
     hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask);
     hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, env->kernelgsbase);
 #endif
+    hax_msr_entry_set(&msrs[n++], MSR_IA32_APICBASE, \
+                      cpu_get_apic_base(x86_env_get_cpu(env)->apic_state));
+
     md.nr_msr = n;
     md.done = 0;
 
diff --git a/target/i386/hax-darwin.c b/target/i386/hax-darwin.c
index acdde47..3e2fd4f 100644
--- a/target/i386/hax-darwin.c
+++ b/target/i386/hax-darwin.c
@@ -244,12 +244,6 @@ int hax_host_setup_vcpu_channel(struct hax_vcpu_state *vcpu)
         return ret;
     }
 
-    if (!valid_hax_tunnel_size(info.size)) {
-        fprintf(stderr, "Invalid hax tunnel size %x\n", info.size);
-        ret = -EINVAL;
-        return ret;
-    }
-
     vcpu->tunnel = (struct hax_tunnel *) (intptr_t) (info.va);
     vcpu->iobuf = (unsigned char *) (intptr_t) (info.io_va);
     return 0;
diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
index 6abc156..b04bf24 100644
--- a/target/i386/hax-i386.h
+++ b/target/i386/hax-i386.h
@@ -38,6 +38,7 @@ struct hax_state {
     struct hax_vm *vm;
     uint64_t mem_quota;
     bool supports_64bit_ramblock;
+    bool supports_tunnel_page;
 };
 
 #define HAX_MAX_VCPU 0x10
@@ -53,7 +54,6 @@ struct hax_vm {
 #ifdef NEED_CPU_H
 /* Functions exported to host specific mode */
 hax_fd hax_vcpu_get_fd(CPUArchState *env);
-int valid_hax_tunnel_size(uint16_t size);
 
 /* Host specific functions */
 int hax_mod_version(struct hax_state *hax, struct hax_module_version *version);
diff --git a/target/i386/hax-interface.h b/target/i386/hax-interface.h
index 93d5fcb..715a64a 100644
--- a/target/i386/hax-interface.h
+++ b/target/i386/hax-interface.h
@@ -280,6 +280,7 @@ struct hax_tunnel {
         struct {
         } state;
     };
+    uint64_t apic_base;
 } __attribute__ ((__packed__));
 
 struct hax_module_version {
@@ -335,6 +336,8 @@ struct hax_set_ram_info {
 #define HAX_CAP_MEMQUOTA           0x2
 #define HAX_CAP_UG                 0x4
 #define HAX_CAP_64BIT_RAMBLOCK     0x8
+#define HAX_CAP_TUNNEL_PAGE        0x20
+
 
 struct hax_capabilityinfo {
     /* bit 0: 1 - working
diff --git a/target/i386/hax-windows.c b/target/i386/hax-windows.c
index b1ac737..6ed4f22 100644
--- a/target/i386/hax-windows.c
+++ b/target/i386/hax-windows.c
@@ -347,11 +347,6 @@ int hax_host_setup_vcpu_channel(struct hax_vcpu_state *vcpu)
         return -1;
     }
 
-    if (!valid_hax_tunnel_size(info.size)) {
-        fprintf(stderr, "Invalid hax tunnel size %x\n", info.size);
-        ret = -EINVAL;
-        return ret;
-    }
     vcpu->tunnel = (struct hax_tunnel *) (intptr_t) (info.va);
     vcpu->iobuf = (unsigned char *) (intptr_t) (info.io_va);
     return 0;
-- 
1.9.1

Re: [Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR

[Paolo Bonzini]

On 19/03/2018 10:04, chao.qin@linux.intel.com wrote:
> From: Qin Chao <chao.qin@intel.com>
> 
> Emulation of IA32_APIC_BASE MSR in HAXM is not correct, such as bit
> 8, which is BSP flag and should be set to 1 for the bootstrap
> processor and set to 0 for the application processors, but it's set
> to 0 for all processors in HAXM. So guest OSes that expect a valid
> BSP flag, such as Zircon (the core of Google Fuchsia OS), cannot
> boot with "-accel hax". To solve this problem, HAXM (which lacks
> APIC virtualization) and QEMU must notify each other of any change
> to guest IA32_APIC_BASE MSR. The HAXM patch has been merged into
> HAXM source. QEMU needs to use the new HAXM API (apic_base in
> "struct hax_tunnel") to initialize the guest IA32_APIC_BASE MSR,
> and then, update its own copy at every return from
> HAX_VCPU_IOCTL_RUN.
> 
> There will be a backward compatility issue caused by the new field
> "apic_base" added into "struct hax_tunnel". In order to fix the
> problem, the validation for size of "struct hax_tunnel" is removed
> and a new capability flag "HAX_CAP_TUNNEL_PAGE" is added, which
> means that one page (4KB) is allocated in HAXM kernel to store
> "struct hax_tunnel", instead of the size of "struct hax_tunnel".
> 
> Change-Id: I8505bc1d75c495dd2765e581d6014125dcb538f3
> Signed-off-by: Qin Chao <chao.qin@intel.com>
> ---
>  target/i386/hax-all.c       | 24 +++++++++++++++++++-----
>  target/i386/hax-darwin.c    |  6 ------
>  target/i386/hax-i386.h      |  2 +-
>  target/i386/hax-interface.h |  3 +++
>  target/i386/hax-windows.c   |  5 -----
>  5 files changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
> index cad7531..6a840d9 100644
> --- a/target/i386/hax-all.c
> +++ b/target/i386/hax-all.c
> @@ -62,11 +62,6 @@ int hax_enabled(void)
>      return hax_allowed;
>  }
>  
> -int valid_hax_tunnel_size(uint16_t size)
> -{
> -    return size >= sizeof(struct hax_tunnel);
> -}
> -
>  hax_fd hax_vcpu_get_fd(CPUArchState *env)
>  {
>      struct hax_vcpu_state *vcpu = ENV_GET_CPU(env)->hax_vcpu;
> @@ -104,6 +99,7 @@ static int hax_get_capability(struct hax_state *hax)
>      }
>  
>      hax->supports_64bit_ramblock = !!(cap->winfo & HAX_CAP_64BIT_RAMBLOCK);
> +    hax->supports_tunnel_page = !!(cap->winfo & HAX_CAP_TUNNEL_PAGE);
>  
>      if (cap->wstatus & HAX_CAP_MEMQUOTA) {
>          if (cap->mem_quota < hax->mem_quota) {
> @@ -520,6 +516,21 @@ static int hax_vcpu_hax_exec(CPUArchState *env)
>          cpu_exec_end(cpu);
>          qemu_mutex_lock_iothread();
>  
> +        /*
> +         * Every time HAXM exits to QEMU, sync IA32_APIC_BASE MSR from HAXM and
> +         * pass it to the emulated APIC.
> +         */
> +        if (hax_global.supports_tunnel_page) {
> +            /*
> +             * ht->apic_base is not available in HAXM kernel module if HAXM does
> +             * not support HAX_CAP_SUPPORT_TUNNEL_PAGE.
> +             * TODO: HAX_CAP_SUPPORT_TUNNEL_PAGE is used for backward
> +             * compatibility with HAXM kernel module. Remove this check when we
> +             * drop support for HAXM versions that lack this feature.
> +             */
> +            cpu_set_apic_base(x86_cpu->apic_state, ht->apic_base);
> +        }
> +
>          /* Simply continue the vcpu_run if system call interrupted */
>          if (hax_ret == -EINTR || hax_ret == -EAGAIN) {
>              DPRINTF("io window interrupted\n");
> @@ -933,6 +944,9 @@ static int hax_set_msrs(CPUArchState *env)
>      hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask);
>      hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, env->kernelgsbase);
>  #endif
> +    hax_msr_entry_set(&msrs[n++], MSR_IA32_APICBASE, \
> +                      cpu_get_apic_base(x86_env_get_cpu(env)->apic_state));
> +
>      md.nr_msr = n;
>      md.done = 0;
>  
> diff --git a/target/i386/hax-darwin.c b/target/i386/hax-darwin.c
> index acdde47..3e2fd4f 100644
> --- a/target/i386/hax-darwin.c
> +++ b/target/i386/hax-darwin.c
> @@ -244,12 +244,6 @@ int hax_host_setup_vcpu_channel(struct hax_vcpu_state *vcpu)
>          return ret;
>      }
>  
> -    if (!valid_hax_tunnel_size(info.size)) {
> -        fprintf(stderr, "Invalid hax tunnel size %x\n", info.size);
> -        ret = -EINVAL;
> -        return ret;
> -    }
> -
>      vcpu->tunnel = (struct hax_tunnel *) (intptr_t) (info.va);
>      vcpu->iobuf = (unsigned char *) (intptr_t) (info.io_va);
>      return 0;
> diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
> index 6abc156..b04bf24 100644
> --- a/target/i386/hax-i386.h
> +++ b/target/i386/hax-i386.h
> @@ -38,6 +38,7 @@ struct hax_state {
>      struct hax_vm *vm;
>      uint64_t mem_quota;
>      bool supports_64bit_ramblock;
> +    bool supports_tunnel_page;
>  };
>  
>  #define HAX_MAX_VCPU 0x10
> @@ -53,7 +54,6 @@ struct hax_vm {
>  #ifdef NEED_CPU_H
>  /* Functions exported to host specific mode */
>  hax_fd hax_vcpu_get_fd(CPUArchState *env);
> -int valid_hax_tunnel_size(uint16_t size);
>  
>  /* Host specific functions */
>  int hax_mod_version(struct hax_state *hax, struct hax_module_version *version);
> diff --git a/target/i386/hax-interface.h b/target/i386/hax-interface.h
> index 93d5fcb..715a64a 100644
> --- a/target/i386/hax-interface.h
> +++ b/target/i386/hax-interface.h
> @@ -280,6 +280,7 @@ struct hax_tunnel {
>          struct {
>          } state;
>      };
> +    uint64_t apic_base;
>  } __attribute__ ((__packed__));
>  
>  struct hax_module_version {
> @@ -335,6 +336,8 @@ struct hax_set_ram_info {
>  #define HAX_CAP_MEMQUOTA           0x2
>  #define HAX_CAP_UG                 0x4
>  #define HAX_CAP_64BIT_RAMBLOCK     0x8
> +#define HAX_CAP_TUNNEL_PAGE        0x20
> +
>  
>  struct hax_capabilityinfo {
>      /* bit 0: 1 - working
> diff --git a/target/i386/hax-windows.c b/target/i386/hax-windows.c
> index b1ac737..6ed4f22 100644
> --- a/target/i386/hax-windows.c
> +++ b/target/i386/hax-windows.c
> @@ -347,11 +347,6 @@ int hax_host_setup_vcpu_channel(struct hax_vcpu_state *vcpu)
>          return -1;
>      }
>  
> -    if (!valid_hax_tunnel_size(info.size)) {
> -        fprintf(stderr, "Invalid hax tunnel size %x\n", info.size);
> -        ret = -EINVAL;
> -        return ret;
> -    }
>      vcpu->tunnel = (struct hax_tunnel *) (intptr_t) (info.va);
>      vcpu->iobuf = (unsigned char *) (intptr_t) (info.io_va);
>      return 0;
> 

Queued, thanks.

Paolo

Re: [Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR

[Igor Mammedov]

On Mon, 19 Mar 2018 17:04:49 +0800
chao.qin@linux.intel.com wrote:

> From: Qin Chao <chao.qin@intel.com>
> 
> Emulation of IA32_APIC_BASE MSR in HAXM is not correct, such as bit
> 8, which is BSP flag and should be set to 1 for the bootstrap
> processor and set to 0 for the application processors, but it's set
> to 0 for all processors in HAXM. So guest OSes that expect a valid
> BSP flag, such as Zircon (the core of Google Fuchsia OS), cannot
> boot with "-accel hax". To solve this problem, HAXM (which lacks
> APIC virtualization) and QEMU must notify each other of any change
> to guest IA32_APIC_BASE MSR. The HAXM patch has been merged into
> HAXM source. QEMU needs to use the new HAXM API (apic_base in
> "struct hax_tunnel") to initialize the guest IA32_APIC_BASE MSR,
> and then, update its own copy at every return from
> HAX_VCPU_IOCTL_RUN.
> 
> There will be a backward compatility issue caused by the new field
> "apic_base" added into "struct hax_tunnel". In order to fix the
> problem, the validation for size of "struct hax_tunnel" is removed
> and a new capability flag "HAX_CAP_TUNNEL_PAGE" is added, which
> means that one page (4KB) is allocated in HAXM kernel to store
> "struct hax_tunnel", instead of the size of "struct hax_tunnel".
> 
> Change-Id: I8505bc1d75c495dd2765e581d6014125dcb538f3
> Signed-off-by: Qin Chao <chao.qin@intel.com>
> ---
>  target/i386/hax-all.c       | 24 +++++++++++++++++++-----
>  target/i386/hax-darwin.c    |  6 ------
>  target/i386/hax-i386.h      |  2 +-
>  target/i386/hax-interface.h |  3 +++
>  target/i386/hax-windows.c   |  5 -----
>  5 files changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
> index cad7531..6a840d9 100644
> --- a/target/i386/hax-all.c
> +++ b/target/i386/hax-all.c
[...]
> @@ -933,6 +944,9 @@ static int hax_set_msrs(CPUArchState *env)
>      hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask);
>      hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, env->kernelgsbase);
>  #endif
> +    hax_msr_entry_set(&msrs[n++], MSR_IA32_APICBASE, \
> +                      cpu_get_apic_base(x86_env_get_cpu(env)->apic_state));
> +
>      md.nr_msr = n;
>      md.done = 0;
Does it work for you if you drop everything except of this chunk?

There is no much point in syncing BPS flag from HAXM since Seabios
nor OVMF do not implement BSP selection protocol, it's
hard-coded in QEMU that BSP is the first CPU.

Considering that typically no sane OS does change MSR_IA32_APICBASE
there is no need to sync it back to QEMU.
So no need to introduce all that complexity for no gain.

[...]

Re: [Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR

[Qin, Chao]

On 3/20/2018 12:05 AM, Igor Mammedov wrote:
> On Mon, 19 Mar 2018 17:04:49 +0800 chao.qin@linux.intel.com wrote:  > >> From: Qin Chao <chao.qin@intel.com> >> >> Emulation of 
IA32_APIC_BASE MSR in HAXM is not correct, such as >> bit 8, which is 
BSP flag and should be set to 1 for the bootstrap >> processor and set 
to 0 for the application processors, but it's >> set to 0 for all 
processors in HAXM. So guest OSes that expect a >> valid BSP flag, such 
as Zircon (the core of Google Fuchsia OS), >> cannot boot with "-accel 
hax". To solve this problem, HAXM (which >> lacks APIC virtualization) 
and QEMU must notify each other of any >> change to guest IA32_APIC_BASE 
MSR. The HAXM patch has been merged >> into HAXM source. QEMU needs to 
use the new HAXM API (apic_base in >> "struct hax_tunnel") to initialize 
the guest IA32_APIC_BASE MSR, >> and then, update its own copy at every 
return from >> HAX_VCPU_IOCTL_RUN. >> >> There will be a backward 
compatility issue caused by the new field >> "apic_base" added into 
"struct hax_tunnel". In order to fix the >> problem, the validation for 
size of "struct hax_tunnel" is removed >> and a new capability flag 
"HAX_CAP_TUNNEL_PAGE" is added, which >> means that one page (4KB) is 
allocated in HAXM kernel to store >> "struct hax_tunnel", instead of the 
size of "struct hax_tunnel". >> >> Change-Id: 
I8505bc1d75c495dd2765e581d6014125dcb538f3 Signed-off-by: >> Qin Chao 
<chao.qin@intel.com> --- target/i386/hax-all.c | 24 >> 
+++++++++++++++++++----- target/i386/hax-darwin.c | 6 ------ >> 
target/i386/hax-i386.h | 2 +- target/i386/hax-interface.h | >> 3 +++ 
target/i386/hax-windows.c | 5 ----- 5 files changed, 23 >> 
insertions(+), 17 deletions(-) >> >> diff --git a/target/i386/hax-all.c 
b/target/i386/hax-all.c index >> cad7531..6a840d9 100644 --- 
a/target/i386/hax-all.c +++ >> b/target/i386/hax-all.c > [...] >> @@ 
-933,6 +944,9 @@ static int hax_set_msrs(CPUArchState *env) >> 
hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask); >> 
hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, >> env->kernelgsbase); 
#endif + hax_msr_entry_set(&msrs[n++], >> MSR_IA32_APICBASE, \ + >> 
cpu_get_apic_base(x86_env_get_cpu(env)->apic_state)); + md.nr_msr = >> 
n; md.done = 0; > Does it work for you if you drop everything except of 
this chunk?
Yes, it works just with this chunk.

>  > There is no much point in syncing BPS flag from HAXM since Seabios > 
nor OVMF do not implement BSP selection protocol, it's hard-coded in > 
QEMU that BSP is the first CPU. > > Considering that typically no sane 
OS does change MSR_IA32_APICBASE > there is no need to sync it back to 
QEMU. So no need to introduce all > that complexity for no gain.
Yes, the BSP is hard-coded in QEMU. But other bits, not just BSP flag, are
also needed to sync from HAXM, such as x2APIC mode flag (bit 10) and
APIC enable/disable flag(bit 11). As in the Google Zircon
(https://github.com/fuchsia-mirror/zircon/blob/master/kernel/arch/x86/lapic.cpp#L157),
it will change IA32_APIC_BASE[10] and set the bit to 1 if x2APIC enabled.
Although x2APIC mode is not supported yet for TCG mode, it's worthy to
keep the codes that syncing IA32_APIC_BASE from HAXM to QEMU and if
x2APIC mode is supported for TCG in future, there is no any effort needed
to make HAXM to work with this mode. Also, in KVM it will synced the
IA32_APIC_BASE MSR to QEMU at every VM-Exit.

>  > [...] > > >

Re: [Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR

[Igor Mammedov]

On Tue, 20 Mar 2018 13:29:24 +0800
"Qin, Chao" <chao.qin@linux.intel.com> wrote:

> On 3/20/2018 12:05 AM, Igor Mammedov wrote:
> > On Mon, 19 Mar 2018 17:04:49 +0800 chao.qin@linux.intel.com wrote:  > >> From: Qin Chao <chao.qin@intel.com>
/ something horribly wrong with mail client used for reply /

>> @@ 
> -933,6 +944,9 @@ static int hax_set_msrs(CPUArchState *env) >> 
> hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask); >> 
> hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, >> env->kernelgsbase); 
> #endif + hax_msr_entry_set(&msrs[n++], >> MSR_IA32_APICBASE, \ + >> 
> cpu_get_apic_base(x86_env_get_cpu(env)->apic_state)); + md.nr_msr = >> 
> n; md.done = 0; > Does it work for you if you drop everything except of 
> this chunk?
> Yes, it works just with this chunk.
Could you send v2 dropping unnecessary chunks pls?
(provided that Paolo would drop queued v1).

Re: [Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR

[Qin, Chao]

On 3/20/2018 3:12 PM, Igor Mammedov wrote:
> On Tue, 20 Mar 2018 13:29:24 +0800 "Qin, Chao"  > <chao.qin@linux.intel.com> wrote: > >> On 3/20/2018 12:05 AM, Igor 
Mammedov wrote: >>> On Mon, 19 Mar 2018 17:04:49 +0800 
chao.qin@linux.intel.com >>> wrote: > >> From: Qin Chao 
<chao.qin@intel.com> > / something horribly wrong with mail client used 
for reply / >
Very sorry for the wrong reply style.

>>> @@  >> -933,6 +944,9 @@ static int hax_set_msrs(CPUArchState *env) >> >> 
hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask); >> >> 
hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, >> >> 
env->kernelgsbase); #endif + hax_msr_entry_set(&msrs[n++], >> >> 
MSR_IA32_APICBASE, \ + >> >> 
cpu_get_apic_base(x86_env_get_cpu(env)->apic_state)); + md.nr_msr = >> 
 >> n; md.done = 0; > Does it work for you if you drop everything >> 
except of this chunk? Yes, it works just with this chunk. > Could you 
send v2 dropping unnecessary chunks pls? (provided that > Paolo would 
drop queued v1).
I think you may missed the clarification for the codes that syncing
IA32_APIC_BASE MSR from HAXM to QEMU due to rhe wrong reply style.
Sorry again for the inconvenience. Please refer to the following.

Yes, the BSP is hard-coded in QEMU. But other bits, not just BSP flag, are
alse needed to sync from HAXM, such as x2APIC mode flag (bit 10) and
APIC enable/disable flag (bit 11). As in the Google Zircon
(https://github.com/fuchsia-mirror/zircon/blob/master/kernel/arch/x86/lapic.cpp#L157),
it will change IA32_APIC_BASE[10] and the the bit to 1 if x2APIC enabled.
Although x2APIC mode is not supported yet for TCG mode, it's worthy to
keep the codes that syncing IA32_APIC_BASE from HAXM to QEMU and
if x2APIC mode is supported fro TCG in future, there is no any effort needed
to make HAXM to work with this mode. Alos, in KVM it synced the 
IA32_APIC_BASE
MSR to QEMU at every VM-Exit.