* [PATCH] Add PAT, cr8 and EFER for 32-bit qemu to hax ioctl interface
[not found] <1786823015.3514736.1582923592218.ref@mail.yahoo.com>
@ 2020-02-28 20:59 ` Alexey Romko
2020-04-02 0:57 ` Colin Xu
0 siblings, 1 reply; 3+ messages in thread
From: Alexey Romko @ 2020-02-28 20:59 UTC (permalink / raw)
To: qemu-devel@nongnu.org; +Cc: wenchao.wang@intel.com, colin.xu@intel.com
Add PAT, cr8 and EFER for 32-bit qemu to hax ioctl interface, part of HAX PR 204
Signed-off-by: Alexey Romko <nevilad@yahoo.com>
---
target/i386/hax-all.c | 32 ++++++++++++++++++++++++++++----
target/i386/hax-i386.h | 2 +-
target/i386/hax-interface.h | 2 ++
3 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index a8b6e5aeb8..0bdd309665 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -45,7 +45,7 @@
} while (0)
/* Current version */
-const uint32_t hax_cur_version = 0x4; /* API v4: unmapping and MMIO moves */
+const uint32_t hax_cur_version = 0x5; /* API v5: supports CR8/EFER/PAT */
/* Minimum HAX kernel version */
const uint32_t hax_min_version = 0x4; /* API v4: supports unmapping */
@@ -137,6 +137,7 @@ static int hax_version_support(struct hax_state *hax)
return 0;
}
+ hax_global.cur_api_version = version.cur_version;
return 1;
}
@@ -845,12 +846,24 @@ static int hax_sync_vcpu_register(CPUArchState *env, int set)
regs._cr2 = env->cr[2];
regs._cr3 = env->cr[3];
regs._cr4 = env->cr[4];
+
+ if( hax_global.cur_api_version >= 0x5 ) {
+ CPUState *cs = env_cpu(env);
+ X86CPU *x86_cpu = X86_CPU(cs);
+ regs._cr8 = cpu_get_apic_tpr(x86_cpu->apic_state);
+ }
+
hax_set_segments(env, ®s);
} else {
env->cr[0] = regs._cr0;
env->cr[2] = regs._cr2;
env->cr[3] = regs._cr3;
env->cr[4] = regs._cr4;
+
+ //if( hax_global.cur_api_version >= 0x5 ) {
+ //no need to update TPR from regs._cr8, since all changes are notified.
+ //}
+
hax_get_segments(env, ®s);
}
@@ -881,14 +894,18 @@ static int hax_get_msrs(CPUArchState *env)
msrs[n++].entry = MSR_IA32_SYSENTER_ESP;
msrs[n++].entry = MSR_IA32_SYSENTER_EIP;
msrs[n++].entry = MSR_IA32_TSC;
-#ifdef TARGET_X86_64
msrs[n++].entry = MSR_EFER;
+#ifdef TARGET_X86_64
msrs[n++].entry = MSR_STAR;
msrs[n++].entry = MSR_LSTAR;
msrs[n++].entry = MSR_CSTAR;
msrs[n++].entry = MSR_FMASK;
msrs[n++].entry = MSR_KERNELGSBASE;
#endif
+ if( hax_global.cur_api_version >= 0x5 ) {
+ msrs[n++].entry = MSR_PAT;
+ }
+
md.nr_msr = n;
ret = hax_sync_msr(env, &md, 0);
if (ret < 0) {
@@ -909,10 +926,10 @@ static int hax_get_msrs(CPUArchState *env)
case MSR_IA32_TSC:
env->tsc = msrs[i].value;
break;
-#ifdef TARGET_X86_64
case MSR_EFER:
env->efer = msrs[i].value;
break;
+#ifdef TARGET_X86_64
case MSR_STAR:
env->star = msrs[i].value;
break;
@@ -929,6 +946,9 @@ static int hax_get_msrs(CPUArchState *env)
env->kernelgsbase = msrs[i].value;
break;
#endif
+ case MSR_PAT:
+ env->pat = msrs[i].value;
+ break;
}
}
@@ -947,14 +967,18 @@ static int hax_set_msrs(CPUArchState *env)
hax_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
hax_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
hax_msr_entry_set(&msrs[n++], MSR_IA32_TSC, env->tsc);
-#ifdef TARGET_X86_64
hax_msr_entry_set(&msrs[n++], MSR_EFER, env->efer);
+#ifdef TARGET_X86_64
hax_msr_entry_set(&msrs[n++], MSR_STAR, env->star);
hax_msr_entry_set(&msrs[n++], MSR_LSTAR, env->lstar);
hax_msr_entry_set(&msrs[n++], MSR_CSTAR, env->cstar);
hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask);
hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, env->kernelgsbase);
#endif
+ if( hax_global.cur_api_version >= 0x5 ) {
+ hax_msr_entry_set(&msrs[n++], MSR_PAT, env->pat);
+ }
+
md.nr_msr = n;
md.done = 0;
diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
index 54e9d8b057..9515803184 100644
--- a/target/i386/hax-i386.h
+++ b/target/i386/hax-i386.h
@@ -34,7 +34,7 @@ struct hax_vcpu_state {
struct hax_state {
hax_fd fd; /* the global hax device interface */
- uint32_t version;
+ uint32_t cur_api_version;
struct hax_vm *vm;
uint64_t mem_quota;
bool supports_64bit_ramblock;
diff --git a/target/i386/hax-interface.h b/target/i386/hax-interface.h
index 537ae084e9..c87aedbc2e 100644
--- a/target/i386/hax-interface.h
+++ b/target/i386/hax-interface.h
@@ -218,6 +218,8 @@ struct vcpu_state_t {
uint32_t _activity_state;
uint32_t pad;
interruptibility_state_t _interruptibility_state;
+
+ uint64_t _cr8;
};
/* HAX exit status */
--
2.15.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Add PAT, cr8 and EFER for 32-bit qemu to hax ioctl interface
2020-02-28 20:59 ` [PATCH] Add PAT, cr8 and EFER for 32-bit qemu to hax ioctl interface Alexey Romko
@ 2020-04-02 0:57 ` Colin Xu
2020-04-02 20:12 ` Alexey Romko
0 siblings, 1 reply; 3+ messages in thread
From: Colin Xu @ 2020-04-02 0:57 UTC (permalink / raw)
To: Alexey Romko, "wenchao.wang; +Cc: qemu-devel
Sorry for missing this one.
If I remembered correctly, this patch together with the HAXM patch in
github will cause some regression in SMP case. So we'd like to fully
understand the technique details to make proper change, not only for a
very specific purpose, i.e. boot Windows on Windows.
This patch together with PR#204 doens't change the IOCTL interface
itself, but extend set/get regs with a version check, so the description
here isn't quite clear.
And the change looks just sync the qemu/haxm status but no more. Could
you provide more details that why Windows can't boot without the change.
Like CR8 (TPR), is there logic that need to be handled when TPR is
read/write?
On 2/29/2020 04:59, Alexey Romko wrote:
> Add PAT, cr8 and EFER for 32-bit qemu to hax ioctl interface, part of HAX PR 204
>
> Signed-off-by: Alexey Romko <nevilad@yahoo.com>
> ---
> target/i386/hax-all.c | 32 ++++++++++++++++++++++++++++----
> target/i386/hax-i386.h | 2 +-
> target/i386/hax-interface.h | 2 ++
> 3 files changed, 31 insertions(+), 5 deletions(-)
>
>
> diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
> index a8b6e5aeb8..0bdd309665 100644
> --- a/target/i386/hax-all.c
> +++ b/target/i386/hax-all.c
> @@ -45,7 +45,7 @@
> } while (0)
>
> /* Current version */
> -const uint32_t hax_cur_version = 0x4; /* API v4: unmapping and MMIO moves */
> +const uint32_t hax_cur_version = 0x5; /* API v5: supports CR8/EFER/PAT */
> /* Minimum HAX kernel version */
> const uint32_t hax_min_version = 0x4; /* API v4: supports unmapping */
>
> @@ -137,6 +137,7 @@ static int hax_version_support(struct hax_state *hax)
> return 0;
> }
>
> + hax_global.cur_api_version = version.cur_version;
> return 1;
> }
>
> @@ -845,12 +846,24 @@ static int hax_sync_vcpu_register(CPUArchState *env, int set)
> regs._cr2 = env->cr[2];
> regs._cr3 = env->cr[3];
> regs._cr4 = env->cr[4];
> +
> + if( hax_global.cur_api_version >= 0x5 ) {
> + CPUState *cs = env_cpu(env);
> + X86CPU *x86_cpu = X86_CPU(cs);
> + regs._cr8 = cpu_get_apic_tpr(x86_cpu->apic_state);
> + }
> +
> hax_set_segments(env, ®s);
> } else {
> env->cr[0] = regs._cr0;
> env->cr[2] = regs._cr2;
> env->cr[3] = regs._cr3;
> env->cr[4] = regs._cr4;
> +
> + //if( hax_global.cur_api_version >= 0x5 ) {
> + //no need to update TPR from regs._cr8, since all changes are notified.
> + //}
> +
> hax_get_segments(env, ®s);
> }
>
> @@ -881,14 +894,18 @@ static int hax_get_msrs(CPUArchState *env)
> msrs[n++].entry = MSR_IA32_SYSENTER_ESP;
> msrs[n++].entry = MSR_IA32_SYSENTER_EIP;
> msrs[n++].entry = MSR_IA32_TSC;
> -#ifdef TARGET_X86_64
> msrs[n++].entry = MSR_EFER;
> +#ifdef TARGET_X86_64
> msrs[n++].entry = MSR_STAR;
> msrs[n++].entry = MSR_LSTAR;
> msrs[n++].entry = MSR_CSTAR;
> msrs[n++].entry = MSR_FMASK;
> msrs[n++].entry = MSR_KERNELGSBASE;
> #endif
> + if( hax_global.cur_api_version >= 0x5 ) {
> + msrs[n++].entry = MSR_PAT;
> + }
> +
> md.nr_msr = n;
> ret = hax_sync_msr(env, &md, 0);
> if (ret < 0) {
> @@ -909,10 +926,10 @@ static int hax_get_msrs(CPUArchState *env)
> case MSR_IA32_TSC:
> env->tsc = msrs[i].value;
> break;
> -#ifdef TARGET_X86_64
> case MSR_EFER:
> env->efer = msrs[i].value;
> break;
> +#ifdef TARGET_X86_64
> case MSR_STAR:
> env->star = msrs[i].value;
> break;
> @@ -929,6 +946,9 @@ static int hax_get_msrs(CPUArchState *env)
> env->kernelgsbase = msrs[i].value;
> break;
> #endif
> + case MSR_PAT:
> + env->pat = msrs[i].value;
> + break;
> }
> }
>
> @@ -947,14 +967,18 @@ static int hax_set_msrs(CPUArchState *env)
> hax_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
> hax_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
> hax_msr_entry_set(&msrs[n++], MSR_IA32_TSC, env->tsc);
> -#ifdef TARGET_X86_64
> hax_msr_entry_set(&msrs[n++], MSR_EFER, env->efer);
> +#ifdef TARGET_X86_64
> hax_msr_entry_set(&msrs[n++], MSR_STAR, env->star);
> hax_msr_entry_set(&msrs[n++], MSR_LSTAR, env->lstar);
> hax_msr_entry_set(&msrs[n++], MSR_CSTAR, env->cstar);
> hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask);
> hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, env->kernelgsbase);
> #endif
> + if( hax_global.cur_api_version >= 0x5 ) {
> + hax_msr_entry_set(&msrs[n++], MSR_PAT, env->pat);
> + }
> +
> md.nr_msr = n;
> md.done = 0;
>
> diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
> index 54e9d8b057..9515803184 100644
> --- a/target/i386/hax-i386.h
> +++ b/target/i386/hax-i386.h
> @@ -34,7 +34,7 @@ struct hax_vcpu_state {
>
> struct hax_state {
> hax_fd fd; /* the global hax device interface */
> - uint32_t version;
> + uint32_t cur_api_version;
> struct hax_vm *vm;
> uint64_t mem_quota;
> bool supports_64bit_ramblock;
> diff --git a/target/i386/hax-interface.h b/target/i386/hax-interface.h
> index 537ae084e9..c87aedbc2e 100644
> --- a/target/i386/hax-interface.h
> +++ b/target/i386/hax-interface.h
> @@ -218,6 +218,8 @@ struct vcpu_state_t {
> uint32_t _activity_state;
> uint32_t pad;
> interruptibility_state_t _interruptibility_state;
> +
> + uint64_t _cr8;
> };
>
> /* HAX exit status */
> --
> 2.15.0.windows.1
>
--
Best Regards,
Colin Xu
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Add PAT, cr8 and EFER for 32-bit qemu to hax ioctl interface
2020-04-02 0:57 ` Colin Xu
@ 2020-04-02 20:12 ` Alexey Romko
0 siblings, 0 replies; 3+ messages in thread
From: Alexey Romko @ 2020-04-02 20:12 UTC (permalink / raw)
To: "wenchao.wang, Colin Xu; +Cc: qemu-devel
Hi,
>If I remembered correctly, this patch together with the HAXM patch in github will cause some regression in SMP case
Only on MacOS host. Since there are problems in logging in MacOS, we can't figure yet the source of the problem. We can find the problem by trial and error, but this can be done after haxm is released.
> why Windows can't boot without the change.
Windows can boot without any MSR/CR8 saves to haxm, but when you want to use snapshots you should save and restore these since windows changes and uses these. Currently snapshots are not working on windows (see https://bugs.launchpad.net/qemu/+bug/1855617). I've tested snapshots on a modified version of haxm+qemu when a agent in the guest executes vmcall instruction, this event is returned by haxm to qemu, registers and MSRs are synchronized and snapshot created. Snapshot restore is done in standart way.
>Like CR8 (TPR), is there logic that need to be handled when TPR is read/write?
I've created CR8 read/write handling in haxm, but it is not yet submitted in a PR. Originally the haxm PR was for x86 and x64 windows, but the x64 did not work. I decided to do a one time interface change and not commit all these changes by pieces. CR8 is needed by MacOS too. I plan to submit a PR for it's support after haxm release too.
Colin Xu <colin.xu@intel.com> wrote:
Sorry for missing this one.
If I remembered correctly, this patch together with the HAXM patch in
github will cause some regression in SMP case. So we'd like to fully
understand the technique details to make proper change, not only for a
very specific purpose, i.e. boot Windows on Windows.
This patch together with PR#204 doens't change the IOCTL interface
itself, but extend set/get regs with a version check, so the description
here isn't quite clear.
And the change looks just sync the qemu/haxm status but no more. Could
you provide more details that why Windows can't boot without the change.
Like CR8 (TPR), is there logic that need to be handled when TPR is
read/write?
On 2/29/2020 04:59, Alexey Romko wrote:
> Add PAT, cr8 and EFER for 32-bit qemu to hax ioctl interface, part of HAX PR 204
>
> Signed-off-by: Alexey Romko <nevilad@yahoo.com>
> ---
> target/i386/hax-all.c | 32 ++++++++++++++++++++++++++++----
> target/i386/hax-i386.h | 2 +-
> target/i386/hax-interface.h | 2 ++
> 3 files changed, 31 insertions(+), 5 deletions(-)
>
>
> diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
> index a8b6e5aeb8..0bdd309665 100644
> --- a/target/i386/hax-all.c
> +++ b/target/i386/hax-all.c
> @@ -45,7 +45,7 @@
> } while (0)
>
> /* Current version */
> -const uint32_t hax_cur_version = 0x4; /* API v4: unmapping and MMIO moves */
> +const uint32_t hax_cur_version = 0x5; /* API v5: supports CR8/EFER/PAT */
> /* Minimum HAX kernel version */
> const uint32_t hax_min_version = 0x4; /* API v4: supports unmapping */
>
> @@ -137,6 +137,7 @@ static int hax_version_support(struct hax_state *hax)
> return 0;
> }
>
> + hax_global.cur_api_version = version.cur_version;
> return 1;
> }
>
> @@ -845,12 +846,24 @@ static int hax_sync_vcpu_register(CPUArchState *env, int set)
> regs._cr2 = env->cr[2];
> regs._cr3 = env->cr[3];
> regs._cr4 = env->cr[4];
> +
> + if( hax_global.cur_api_version >= 0x5 ) {
> + CPUState *cs = env_cpu(env);
> + X86CPU *x86_cpu = X86_CPU(cs);
> + regs._cr8 = cpu_get_apic_tpr(x86_cpu->apic_state);
> + }
> +
> hax_set_segments(env, ®s);
> } else {
> env->cr[0] = regs._cr0;
> env->cr[2] = regs._cr2;
> env->cr[3] = regs._cr3;
> env->cr[4] = regs._cr4;
> +
> + //if( hax_global.cur_api_version >= 0x5 ) {
> + //no need to update TPR from regs._cr8, since all changes are notified.
> + //}
> +
> hax_get_segments(env, ®s);
> }
>
> @@ -881,14 +894,18 @@ static int hax_get_msrs(CPUArchState *env)
> msrs[n++].entry = MSR_IA32_SYSENTER_ESP;
> msrs[n++].entry = MSR_IA32_SYSENTER_EIP;
> msrs[n++].entry = MSR_IA32_TSC;
> -#ifdef TARGET_X86_64
> msrs[n++].entry = MSR_EFER;
> +#ifdef TARGET_X86_64
> msrs[n++].entry = MSR_STAR;
> msrs[n++].entry = MSR_LSTAR;
> msrs[n++].entry = MSR_CSTAR;
> msrs[n++].entry = MSR_FMASK;
> msrs[n++].entry = MSR_KERNELGSBASE;
> #endif
> + if( hax_global.cur_api_version >= 0x5 ) {
> + msrs[n++].entry = MSR_PAT;
> + }
> +
> md.nr_msr = n;
> ret = hax_sync_msr(env, &md, 0);
> if (ret < 0) {
> @@ -909,10 +926,10 @@ static int hax_get_msrs(CPUArchState *env)
> case MSR_IA32_TSC:
> env->tsc = msrs[i].value;
> break;
> -#ifdef TARGET_X86_64
> case MSR_EFER:
> env->efer = msrs[i].value;
> break;
> +#ifdef TARGET_X86_64
> case MSR_STAR:
> env->star = msrs[i].value;
> break;
> @@ -929,6 +946,9 @@ static int hax_get_msrs(CPUArchState *env)
> env->kernelgsbase = msrs[i].value;
> break;
> #endif
> + case MSR_PAT:
> + env->pat = msrs[i].value;
> + break;
> }
> }
>
> @@ -947,14 +967,18 @@ static int hax_set_msrs(CPUArchState *env)
> hax_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
> hax_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
> hax_msr_entry_set(&msrs[n++], MSR_IA32_TSC, env->tsc);
> -#ifdef TARGET_X86_64
> hax_msr_entry_set(&msrs[n++], MSR_EFER, env->efer);
> +#ifdef TARGET_X86_64
> hax_msr_entry_set(&msrs[n++], MSR_STAR, env->star);
> hax_msr_entry_set(&msrs[n++], MSR_LSTAR, env->lstar);
> hax_msr_entry_set(&msrs[n++], MSR_CSTAR, env->cstar);
> hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask);
> hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, env->kernelgsbase);
> #endif
> + if( hax_global.cur_api_version >= 0x5 ) {
> + hax_msr_entry_set(&msrs[n++], MSR_PAT, env->pat);
> + }
> +
> md.nr_msr = n;
> md.done = 0;
>
> diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
> index 54e9d8b057..9515803184 100644
> --- a/target/i386/hax-i386.h
> +++ b/target/i386/hax-i386.h
> @@ -34,7 +34,7 @@ struct hax_vcpu_state {
>
> struct hax_state {
> hax_fd fd; /* the global hax device interface */
> - uint32_t version;
> + uint32_t cur_api_version;
> struct hax_vm *vm;
> uint64_t mem_quota;
> bool supports_64bit_ramblock;
> diff --git a/target/i386/hax-interface.h b/target/i386/hax-interface.h
> index 537ae084e9..c87aedbc2e 100644
> --- a/target/i386/hax-interface.h
> +++ b/target/i386/hax-interface.h
> @@ -218,6 +218,8 @@ struct vcpu_state_t {
> uint32_t _activity_state;
> uint32_t pad;
> interruptibility_state_t _interruptibility_state;
> +
> + uint64_t _cr8;
> };
>
> /* HAX exit status */
> --
> 2.15.0.windows.1
>
--
Best Regards,
Colin Xu
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-04-02 20:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1786823015.3514736.1582923592218.ref@mail.yahoo.com>
2020-02-28 20:59 ` [PATCH] Add PAT, cr8 and EFER for 32-bit qemu to hax ioctl interface Alexey Romko
2020-04-02 0:57 ` Colin Xu
2020-04-02 20:12 ` Alexey Romko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).