Jan Kiszka wrote:
> Avi Kivity wrote:
>> On 10/29/2009 10:03 AM, Jan Kiszka wrote:
>>> Avi Kivity wrote:
>>>
>>>> On 10/28/2009 10:40 PM, Jan Kiszka wrote:
>>>>
>>>>>
>>>>>> [you can get longer, more detailed traces by using
>>>>>> /sys/kernel/debug/tracing/trace instead of dmesg]
>>>>>>
>>>>>> Oct 28 14:29:56 mchn012c kernel: qemu-sys-7200 0...1. 676996395us :
>>>>>> kvm_msr: msr_read c0000080 = 0x500
>>>>>> Oct 28 14:29:56 mchn012c kernel: qemu-sys-7200 0...1. 676996403us :
>>>>>> kvm_msr: msr_write c0000080 = 0xd01
>>>>>>
>>>>>> So Windows is setting EFER.SCE and EFER.NX while in long mode -
>>>>>> perfectly reasonable. Can you rerun with the attached debug patch?
>>>>>>
>>>>>>
>>>>>>
>>>>> Log attached.
>>>>>
>>>>>
>>>> So the last bits are:
>>>>
>>>> Oct 28 21:26:41 mchn012c kernel: update_transition_efer: efer_offset 4
>>>> efer d01
>>>> Oct 28 21:26:41 mchn012c kernel: update_transition_efer: ignoring all
>>>> bits
>>>> Oct 28 21:26:41 mchn012c kernel: setup_msrs: marking efer for no reload
>>>>
>>>> We're not reloading efer (correctly, as guest efer == host efer), yet
>>>> vmx_save_host_state() fails while loading efer. I've looked at
>>>> move_msr_up() (which is used by setup_msrs() to partition the msr space
>>>> into reloaded and non-reloaded msrs), and it seems correct.
>>>>
>>>> Can you see any way where update_transition_efer() returns false, yet
>>>> efer turns up in the first save_nmsrs entries of vmx->guest_msrs?
>>>>
>>>>
>>> Without understanding the code completely yet: When you push the slot
>>> containing EFER around, do you also update msr_offset_efer?
>>>
>>>
>> We don't, but msr_offset_efer is only used from
>> update_transition_efer(), which is only ever called from setup_msrs()
>> immediately after updating msr_offset_efer.
>
> Indeed.
>
>> Of course, it should be an argument to update_transition_efer(), I'll
>> clean up this leftover.
>>
>
> OK, will see that I can debug this later today.
>
Haven't found the actual problem yet, but some oddities:
> static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
> {
> ...
> for (i = 0; i < NR_VMX_MSR; ++i) {
> u32 index = vmx_msr_index[i];
> u32 data_low, data_high;
> u64 data;
> int j = vmx->nmsrs;
>
> if (rdmsr_safe(index, &data_low, &data_high) < 0)
> continue;
> if (wrmsr_safe(index, data_low, data_high) < 0)
> continue;
> data = data_low | ((u64)data_high << 32);
> vmx->guest_msrs[j].index = i;
> vmx->guest_msrs[j].data = 0;
^^^^^
Local 'data' drops on the floor. Is that correct (then it deserves a
cleanup)? Previous version did a "guest = host".
> static void vmx_set_efer(struct kvm_vcpu *vcpu, u64 efer)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> struct shared_msr_entry *msr = find_msr_entry(vmx, MSR_EFER);
>
> if (!msr)
> return;
> vcpu->arch.shadow_efer = efer;
> if (!msr)
> return;
One "if (!msr)" too much - really the second one?
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
