On 01/02/2011 07:25, "Wei Huang" <wei.huang2@xxxxxxx> wrote:
> But there is another issue: upper 32bit of sysenter MSRs in VMCB save
> area will be truncated with VMSARE/VMEXIT (see comments in vmcb.h).
> Could we use these VMCB fields as a storage for 64bit MSRs?
Is that a bug? Seems unfortunate and unnecessary.
Well, in that case I would suggest to *always* intercept the MSRs, keep the
vcpu-structure fields, and in svm_msr_write_intercept(MSR_IA32_SYSENTER_*)
update vcpu structure *and* do svm_sync_vmcb(); update vmcb; svm_vmload().
This keeps the canonical version of the msrs always in the vcpu structure,
with a (possibly 32-bit truncated) copy in the vmcb. The truncation is safe
since the MSRs will only directly be used in guest mode by legacy-mode
execution of SYSENTER.
How's that?
-- Keir
> Thanks,
> -Wei
>
> On Tue, 2011-02-01 at 00:14 -0600, Keir Fraser wrote:
>> On 31/01/2011 22:38, "Wei Huang" <wei.huang2@xxxxxxx> wrote:
>>
>>>> This handling of the SYSENTER MSRs is overly complicated. I suggest
>>>> reverting a bunch of the original handling of cross-vendor migration as
>>>> follows:
>>>> * Never intercept the SYSENTER MSRs.
>>> The reason for Christoph to create this patch is AMD doesn't support
>>> SYSENTER in long mode.
>>
>> Yes.
>>
>>> If we don't intercept MSRs under long mode, we
>>> will get stuck with #UD after migration from Intel platform.
>>
>> It's the SYSENTER instruction that causes the UD, right, not the WRMSR
>> writes to the SYSENTER MSRs? Then my described approach will work -- the
>> SYSENTER instruction will be handled by Xen's x86_emulate(), calling out to
>> svm_msr_read_intercept() to grab the SYSENTER MSR values (from the VMCB, as
>> I described). In fact x86_emulate() handles WRMSR too, so even if WRMSR
>> caused UD we'd still handle it.
>>
>>> Did you
>>> actually mean "* Always intercept the SYSENTER MSRs" here?
>>
>> No, I think my approach works as I described it.
>>
>> -- Keir
>>
>>>> * Remove the vcpu->arch.hvm_svm.guest_sysenter_* fields.
>>>> * Always hvm save/restore from/to the values in the vmcb.
>>>> * Modify svm_msr_read_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb()
>>>> and
>>>> then read the sysenter msr value from vmcb
>>>> * Modify svm_msr_write_intercept(MSR_IA32_SYSENTER_*) to svm_sync_vmcb(),
>>>> then modify the sysenter msr in the vmcb, and then svm_vmload().
>>>>
>>>> Result is that we get rid of some redundant fields from the vcpu structure
>>>> and have one canonical place we always keep the sysenter msr values, in the
>>>> vmcb. The extra cost in the msr read/write functions is totally
>>>> inconsequential, and only used after guest migration from an Intel CPU
>>>> anyway. Hardly something to optimise for.
>>>>
>>>> -- Keir
>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Xen-devel mailing list
>>>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>>>>> http://lists.xensource.com/xen-devel
>>>>
>>>>
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>>>> http://lists.xensource.com/xen-devel
>>>>
>>>
>>>
>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
>>
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home