* [PATCH] Fix save/restore of guest PAT table in HAP paging mode.
@ 2012-04-11 23:04 Gianluca Guida
2012-04-12 6:14 ` Wei Huang
0 siblings, 1 reply; 7+ messages in thread
From: Gianluca Guida @ 2012-04-11 23:04 UTC (permalink / raw)
To: xen-devel; +Cc: Gianluca Guida
[-- Attachment #1: Type: text/plain, Size: 590 bytes --]
Hello,
HAP paging mode guests use direct MSR read/write into the VMCS/VMCB
for the guest PAT table, while the current save/restore code was
accessing only the pat_cr field in hvm_vcpu, used when intercepting
the MSR mostly in shadow mode (the Intel scenario is a bit more
complicated).
This patch fixes this issue creating a new couple of hvm_funcs,
get/set_guest_pat, that access the right PAT table based on the paging
mode and guest configuration.
As a major caveat, I haven't tested this patch on AMD, for lack of hardware.
Signed-off-by: Gianluca Guida <gianluca.guida@citrix.com>
[-- Attachment #2: save-pat-hap.patch --]
[-- Type: application/octet-stream, Size: 6322 bytes --]
diff -r 5bbda657a016 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c Tue Apr 10 10:42:35 2012 +0100
+++ b/xen/arch/x86/hvm/hvm.c Wed Apr 11 03:03:50 2012 -0700
@@ -216,6 +216,31 @@
hvm_funcs.set_rdtsc_exiting(v, enable);
}
+void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat)
+{
+ if ( !hvm_funcs.get_guest_pat(v, guest_pat) )
+ *guest_pat = v->arch.hvm_vcpu.pat_cr;
+}
+
+int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
+{
+ int i;
+ uint8_t *value = (uint8_t *)&guest_pat;
+
+ for ( i = 0; i < 8; i++ )
+ if ( unlikely(!(value[i] == 0 || value[i] == 1 ||
+ value[i] == 4 || value[i] == 5 ||
+ value[i] == 6 || value[i] == 7)) ) {
+ HVM_DBG_LOG(DBG_LEVEL_MSR, "invalid guest PAT: %"PRIx64"\n",
+ guest_pat);
+ return 0;
+ }
+
+ if ( !hvm_funcs.set_guest_pat(v, guest_pat) )
+ v->arch.hvm_vcpu.pat_cr = guest_pat;
+ return 1;
+}
+
void hvm_set_guest_tsc(struct vcpu *v, u64 guest_tsc)
{
uint64_t tsc;
@@ -2796,7 +2821,7 @@
break;
case MSR_IA32_CR_PAT:
- *msr_content = v->arch.hvm_vcpu.pat_cr;
+ hvm_get_guest_pat(v, msr_content);
break;
case MSR_MTRRcap:
@@ -2912,7 +2937,7 @@
break;
case MSR_IA32_CR_PAT:
- if ( !pat_msr_set(&v->arch.hvm_vcpu.pat_cr, msr_content) )
+ if ( !hvm_set_guest_pat(v, msr_content) )
goto gp_fault;
break;
diff -r 5bbda657a016 xen/arch/x86/hvm/mtrr.c
--- a/xen/arch/x86/hvm/mtrr.c Tue Apr 10 10:42:35 2012 +0100
+++ b/xen/arch/x86/hvm/mtrr.c Wed Apr 11 03:03:50 2012 -0700
@@ -403,26 +403,6 @@
return pat_type_2_pte_flags(pat_entry_value);
}
-/* Helper funtions for seting mtrr/pat */
-bool_t pat_msr_set(uint64_t *pat, uint64_t msr_content)
-{
- uint8_t *value = (uint8_t*)&msr_content;
- int32_t i;
-
- if ( *pat != msr_content )
- {
- for ( i = 0; i < 8; i++ )
- if ( unlikely(!(value[i] == 0 || value[i] == 1 ||
- value[i] == 4 || value[i] == 5 ||
- value[i] == 6 || value[i] == 7)) )
- return 0;
-
- *pat = msr_content;
- }
-
- return 1;
-}
-
bool_t mtrr_def_type_msr_set(struct mtrr_state *m, uint64_t msr_content)
{
uint8_t def_type = msr_content & 0xff;
@@ -631,7 +611,7 @@
{
mtrr_state = &v->arch.hvm_vcpu.mtrr;
- hw_mtrr.msr_pat_cr = v->arch.hvm_vcpu.pat_cr;
+ hvm_get_guest_pat(v, &hw_mtrr.msr_pat_cr);
hw_mtrr.msr_mtrr_def_type = mtrr_state->def_type
| (mtrr_state->enabled << 10);
@@ -677,7 +657,7 @@
mtrr_state = &v->arch.hvm_vcpu.mtrr;
- pat_msr_set(&v->arch.hvm_vcpu.pat_cr, hw_mtrr.msr_pat_cr);
+ hvm_set_guest_pat(v, hw_mtrr.msr_pat_cr);
mtrr_state->mtrr_cap = hw_mtrr.msr_mtrr_cap;
diff -r 5bbda657a016 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c Tue Apr 10 10:42:35 2012 +0100
+++ b/xen/arch/x86/hvm/svm/svm.c Wed Apr 11 03:03:50 2012 -0700
@@ -645,6 +645,28 @@
svm_vmload(vmcb);
}
+static int svm_set_guest_pat(struct vcpu *v, u64 gpat)
+{
+ struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
+
+ if ( !paging_mode_hap(v->domain) )
+ return 0;
+
+ vmcb_set_g_pat(vmcb, gpat);
+ return 1;
+}
+
+static int svm_get_guest_pat(struct vcpu *v, u64 *gpat)
+{
+ struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
+
+ if ( !paging_mode_hap(v->domain) )
+ return 0;
+
+ *gpat = vmcb_get_g_pat(vmcb);
+ return 1;
+}
+
static uint64_t svm_get_tsc_offset(uint64_t host_tsc, uint64_t guest_tsc,
uint64_t ratio)
{
@@ -1964,6 +1986,8 @@
.update_host_cr3 = svm_update_host_cr3,
.update_guest_cr = svm_update_guest_cr,
.update_guest_efer = svm_update_guest_efer,
+ .set_guest_pat = svm_set_guest_pat,
+ .get_guest_pat = svm_get_guest_pat,
.set_tsc_offset = svm_set_tsc_offset,
.inject_exception = svm_inject_exception,
.init_hypercall_page = svm_init_hypercall_page,
diff -r 5bbda657a016 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Tue Apr 10 10:42:35 2012 +0100
+++ b/xen/arch/x86/hvm/vmx/vmx.c Wed Apr 11 03:03:50 2012 -0700
@@ -942,6 +942,34 @@
vmx_vmcs_exit(v);
}
+static int vmx_set_guest_pat(struct vcpu *v, u64 gpat)
+{
+ if ( !cpu_has_vmx_pat || !paging_mode_hap(v->domain) )
+ return 0;
+
+ vmx_vmcs_enter(v);
+ __vmwrite(GUEST_PAT, gpat);
+#ifdef __i386__
+ __vmwrite(GUEST_PAT_HIGH, gpat >> 32);
+#endif
+ vmx_vmcs_exit(v);
+ return 1;
+}
+
+static int vmx_get_guest_pat(struct vcpu *v, u64 *gpat)
+{
+ if ( !cpu_has_vmx_pat || !paging_mode_hap(v->domain) )
+ return 0;
+
+ vmx_vmcs_enter(v);
+ *gpat = __vmread(GUEST_PAT);
+#ifdef __i386__
+ *gpat |= (u64)__vmread(GUEST_PAT_HIGH) << 32;
+#endif
+ vmx_vmcs_exit(v);
+ return 1;
+}
+
static void vmx_set_tsc_offset(struct vcpu *v, u64 offset)
{
vmx_vmcs_enter(v);
@@ -1486,6 +1514,8 @@
.update_host_cr3 = vmx_update_host_cr3,
.update_guest_cr = vmx_update_guest_cr,
.update_guest_efer = vmx_update_guest_efer,
+ .set_guest_pat = vmx_set_guest_pat,
+ .get_guest_pat = vmx_get_guest_pat,
.set_tsc_offset = vmx_set_tsc_offset,
.inject_exception = vmx_inject_exception,
.init_hypercall_page = vmx_init_hypercall_page,
diff -r 5bbda657a016 xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h Tue Apr 10 10:42:35 2012 +0100
+++ b/xen/include/asm-x86/hvm/hvm.h Wed Apr 11 03:03:50 2012 -0700
@@ -118,6 +118,9 @@
void (*update_guest_cr)(struct vcpu *v, unsigned int cr);
void (*update_guest_efer)(struct vcpu *v);
+ int (*get_guest_pat)(struct vcpu *v, u64 *);
+ int (*set_guest_pat)(struct vcpu *v, u64);
+
void (*set_tsc_offset)(struct vcpu *v, u64 offset);
void (*inject_exception)(unsigned int trapnr, int errcode,
@@ -200,6 +203,9 @@
bool_t hvm_send_assist_req(struct vcpu *v);
+void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat);
+int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat);
+
void hvm_set_guest_tsc(struct vcpu *v, u64 guest_tsc);
u64 hvm_get_guest_tsc(struct vcpu *v);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix save/restore of guest PAT table in HAP paging mode.
2012-04-11 23:04 [PATCH] Fix save/restore of guest PAT table in HAP paging mode Gianluca Guida
@ 2012-04-12 6:14 ` Wei Huang
2012-04-12 7:32 ` Gianluca Guida
0 siblings, 1 reply; 7+ messages in thread
From: Wei Huang @ 2012-04-12 6:14 UTC (permalink / raw)
To: Gianluca Guida; +Cc: xen-devel, Gianluca Guida
On 04/11/2012 06:04 PM, Gianluca Guida wrote:
> Hello,
>
> HAP paging mode guests use direct MSR read/write into the VMCS/VMCB
> for the guest PAT table, while the current save/restore code was
> accessing only the pat_cr field in hvm_vcpu, used when intercepting
> the MSR mostly in shadow mode (the Intel scenario is a bit more
> complicated).
> This patch fixes this issue creating a new couple of hvm_funcs,
> get/set_guest_pat, that access the right PAT table based on the paging
> mode and guest configuration.
>
> As a major caveat, I haven't tested this patch on AMD, for lack of hardware.
I can test it on my AMD box tomorrow. BTW from my understanding, this
patch doesn't have performance implication for nested paging mode, does it?
-Wei
>
> Signed-off-by: Gianluca Guida<gianluca.guida@citrix.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix save/restore of guest PAT table in HAP paging mode.
2012-04-12 6:14 ` Wei Huang
@ 2012-04-12 7:32 ` Gianluca Guida
2012-04-13 16:29 ` Huang2, Wei
0 siblings, 1 reply; 7+ messages in thread
From: Gianluca Guida @ 2012-04-12 7:32 UTC (permalink / raw)
To: Wei Huang; +Cc: xen-devel, Gianluca Guida
On Wed, Apr 11, 2012 at 11:14 PM, Wei Huang <wei.huang2@amd.com> wrote:
> On 04/11/2012 06:04 PM, Gianluca Guida wrote:
>> As a major caveat, I haven't tested this patch on AMD, for lack of
>> hardware.
>
> I can test it on my AMD box tomorrow. BTW from my understanding, this patch
> doesn't have performance implication for nested paging mode, does it?
Thank you! A good but not perfect way is to check that xen-hvmctx
doesn't return on HAP the reset PAT table after a linux/windows guest
has been running. And that its value is actually preserved across
save/restore.
As for the performances in nested mode, as far as I could see, I think
not -- but I haven't looked at eventually hidden details of the nested
vm implementation.
Gianluca
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix save/restore of guest PAT table in HAP paging mode.
2012-04-12 7:32 ` Gianluca Guida
@ 2012-04-13 16:29 ` Huang2, Wei
2012-04-13 17:08 ` Gianluca Guida
0 siblings, 1 reply; 7+ messages in thread
From: Huang2, Wei @ 2012-04-13 16:29 UTC (permalink / raw)
To: Gianluca Guida; +Cc: xen-devel, Gianluca Guida
I tested it with SLES11 SP1 and Windows 7 guest VM. With nested paging enabled, I saw XEN saved 0x0007010600070106ULL, instead of default 0x0007040600070406ULL, to guest disk files. So it behaved as expected.
-Wei
-----Original Message-----
From: Gianluca Guida [mailto:glguida@gmail.com]
Sent: Thursday, April 12, 2012 2:32 AM
To: Huang2, Wei
Cc: xen-devel@lists.xensource.com; Gianluca Guida
Subject: Re: [Xen-devel] [PATCH] Fix save/restore of guest PAT table in HAP paging mode.
On Wed, Apr 11, 2012 at 11:14 PM, Wei Huang <wei.huang2@amd.com> wrote:
> On 04/11/2012 06:04 PM, Gianluca Guida wrote:
>> As a major caveat, I haven't tested this patch on AMD, for lack of
>> hardware.
>
> I can test it on my AMD box tomorrow. BTW from my understanding, this patch
> doesn't have performance implication for nested paging mode, does it?
Thank you! A good but not perfect way is to check that xen-hvmctx
doesn't return on HAP the reset PAT table after a linux/windows guest
has been running. And that its value is actually preserved across
save/restore.
As for the performances in nested mode, as far as I could see, I think
not -- but I haven't looked at eventually hidden details of the nested
vm implementation.
Gianluca
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix save/restore of guest PAT table in HAP paging mode.
2012-04-13 16:29 ` Huang2, Wei
@ 2012-04-13 17:08 ` Gianluca Guida
2012-04-13 17:51 ` Tim Deegan
2012-04-13 18:42 ` Wei Huang
0 siblings, 2 replies; 7+ messages in thread
From: Gianluca Guida @ 2012-04-13 17:08 UTC (permalink / raw)
To: Huang2, Wei; +Cc: xen-devel, Gianluca Guida
On Fri, Apr 13, 2012 at 9:29 AM, Huang2, Wei <Wei.Huang2@amd.com> wrote:
> I tested it with SLES11 SP1 and Windows 7 guest VM. With nested paging enabled, I saw XEN saved 0x0007010600070106ULL, instead of default 0x0007040600070406ULL, to guest disk files. So it behaved as expected.
Yes, that is exactly what I expected to see (WC enabled). Thanks again.
Any change to get this checked in?
Gianluca
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix save/restore of guest PAT table in HAP paging mode.
2012-04-13 17:08 ` Gianluca Guida
@ 2012-04-13 17:51 ` Tim Deegan
2012-04-13 18:42 ` Wei Huang
1 sibling, 0 replies; 7+ messages in thread
From: Tim Deegan @ 2012-04-13 17:51 UTC (permalink / raw)
To: Gianluca Guida; +Cc: Huang2, Wei, xen-devel, Gianluca Guida
At 10:08 -0700 on 13 Apr (1334311697), Gianluca Guida wrote:
> On Fri, Apr 13, 2012 at 9:29 AM, Huang2, Wei <Wei.Huang2@amd.com> wrote:
> > I tested it with SLES11 SP1 and Windows 7 guest VM. With nested paging enabled, I saw XEN saved 0x0007010600070106ULL, instead of default 0x0007040600070406ULL, to guest disk files. So it behaved as expected.
>
> Yes, that is exactly what I expected to see (WC enabled). Thanks again.
>
> Any change to get this checked in?
FWIW, Acked-by: Tim Deegan <tim@xen.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix save/restore of guest PAT table in HAP paging mode.
2012-04-13 17:08 ` Gianluca Guida
2012-04-13 17:51 ` Tim Deegan
@ 2012-04-13 18:42 ` Wei Huang
1 sibling, 0 replies; 7+ messages in thread
From: Wei Huang @ 2012-04-13 18:42 UTC (permalink / raw)
To: Gianluca Guida; +Cc: xen-devel, Gianluca Guida
You can treat my experiments as an ack. But other folks need to ack it too.
-Wei
On 04/13/2012 12:08 PM, Gianluca Guida wrote:
> On Fri, Apr 13, 2012 at 9:29 AM, Huang2, Wei<Wei.Huang2@amd.com> wrote:
>> I tested it with SLES11 SP1 and Windows 7 guest VM. With nested paging enabled, I saw XEN saved 0x0007010600070106ULL, instead of default 0x0007040600070406ULL, to guest disk files. So it behaved as expected.
> Yes, that is exactly what I expected to see (WC enabled). Thanks again.
>
> Any change to get this checked in?
> Gianluca
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-04-13 18:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-11 23:04 [PATCH] Fix save/restore of guest PAT table in HAP paging mode Gianluca Guida
2012-04-12 6:14 ` Wei Huang
2012-04-12 7:32 ` Gianluca Guida
2012-04-13 16:29 ` Huang2, Wei
2012-04-13 17:08 ` Gianluca Guida
2012-04-13 17:51 ` Tim Deegan
2012-04-13 18:42 ` Wei Huang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.