On 19/03/2018 10:04, chao....@linux.intel.com wrote:
> From: Qin Chao <chao....@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....@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
