Re: [PATCH 8/8] KVM:PPC:booke: Allow debug interrupt injection to guest

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/31/2013 01:20:39 PM, Alexander Graf wrote:

On 31.01.2013, at 20:05, Alexander Graf wrote:

>
> On 31.01.2013, at 19:54, Scott Wood wrote:
>
>> On 01/31/2013 12:52:41 PM, Alexander Graf wrote:
>>> On 31.01.2013, at 19:43, Scott Wood wrote:
>>>> On 01/31/2013 12:21:07 PM, Alexander Graf wrote:
>>>>> How about something like this? Then both targets at least suck as much :).
>>>>
>>>> I'm not sure that should be the goal...
>>>>
>>>>> Thanks to e500mc's awful hardware design, we don't know who sets the MSR_DE bit. Once we forced it onto the guest, we have no change to know whether the guest also set it or not. We could only guess.
>>>>
>>>> MSRP[DEP] can prevent the guest from modifying MSR[DE] -- but we still need to set it in the first place.
>>>>
>>>> According to ISA V2.06B, the hypervisor should set DBCR0[EDM] to let the guest know that the debug resources are not available, and that "the value of MSR[DE] is not specified and not modifiable". >>> So what would the guest do then to tell the hypervisor that it actually wants to know about debug events?
>>
>> The guest is out of luck, just as if a JTAG were in use.
>
> Hrm.
>
> Can we somehow generalize this "out of luck" behavior?
>
> Every time we would set or clear an MSR bit in shadow_msr on e500v2, we would instead set or clear it in the real MSR. That way only e500mc is out of luck, but the code would still be shared.

I don't follow. e500v2 is just as out-of-luck. The mechanism simply does not support sharing debug resources.

What do you mean by "the real MSR"? The real MSR is shadow_msr, and MSR_DE must always be set there if the host is debugging the guest. As for reflecting it into the guest MSR, we could, but I don't really see the point. We're never going to actually send a debug exception to the guest when the host owns the debug resources.

Speaking of naming issues, "guest_debug" is very ambiguous...

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 38a62ef..9bdb845 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu)
 #endif
 }

+static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu)
+{
+	u32 is_debug = vcpu->arch.shared->msr & MSR_DE;
+
+ /* Force debug to on in guest space when user space wants to debug */
+	if (vcpu->guest_debug)
+		is_debug = MSR_DE;
+
+#ifdef CONFIG_KVM_BOOKE_HV
+	/*
+	 * Since there is no shadow MSR, sync MSR_DE into the guest
+	 * visible MSR.
+	 */
+	vcpu->arch.shared->msr &= ~MSR_DE;
+	vcpu->arch.shared->msr |= is_debug;
+#endif
+
+#ifndef CONFIG_KVM_BOOKE_HV
+	vcpu->arch.shadow_msr &= ~MSR_DE;
+	vcpu->arch.shadow_msr |= is_debug;
+#endif
+}

The "&= ~MSR_DE" line is pointless on bookehv, and makes it harder to read. I had to stare at it a while before noticing that you initially set is_debug from the guest MSR and that you'd never really clear MSR_DE here on bookehv.

-Scott
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux