On Mon, Jan 21, 2013 at 12:43 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > On Mon, 21 Jan 2013 09:50:18 -0500, Christoffer Dall > <c.dall@xxxxxxxxxxxxxxxxxxxxxx> wrote: >> On Mon, Jan 21, 2013 at 5:04 AM, Marc Zyngier <marc.zyngier@xxxxxxx> > wrote: >>> On Sun, 20 Jan 2013 18:35:51 -0500, Christoffer Dall >>> <c.dall@xxxxxxxxxxxxxxxxxxxxxx> wrote: >>>> On Thu, Jan 17, 2013 at 10:55 AM, Marc Zyngier <marc.zyngier@xxxxxxx> >>>> wrote: >>>>> On 16/01/13 17:59, Christoffer Dall wrote: >>>>>> From: Marc Zyngier <marc.zyngier@xxxxxxx> >>>>>> >>>>>> Implement the PSCI specification (ARM DEN 0022A) to control >>>>>> virtual CPUs being "powered" on or off. >>>>>> >>>>>> PSCI/KVM is detected using the KVM_CAP_ARM_PSCI capability. >>>>>> >>>>>> A virtual CPU can now be initialized in a "powered off" state, >>>>>> using the KVM_ARM_VCPU_POWER_OFF feature flag. >>>>>> >>>>>> The guest can use either SMC or HVC to execute a PSCI function. >>>>>> >>>>>> Reviewed-by: Will Deacon <will.deacon@xxxxxxx> >>>>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> >>>>>> Signed-off-by: Christoffer Dall <c.dall@xxxxxxxxxxxxxxxxxxxxxx> >>>>> >>>>> A few bits went wrong when you reworked this patch. See below. >>> >>> [...] >>> >>>>>> @@ -443,13 +445,17 @@ static int handle_hvc(struct kvm_vcpu *vcpu, >>>>>> struct kvm_run *run) >>>>>> trace_kvm_hvc(*vcpu_pc(vcpu), *vcpu_reg(vcpu, 0), >>>>>> vcpu->arch.hsr & HSR_HVC_IMM_MASK); >>>>>> >>>>>> + if (kvm_psci_call(vcpu)) >>>>>> + return 1; >>>>>> + >>>>>> return 1; >>>>> >>>>> No undef injection if there is no PSCI match? >>> >>> You haven't addressed this issue in you patch. >>> >> right, well, it's actually quite nice not having it give you an >> undefined exception when it logs the trace event. The psci protocol >> relies on a confirmation in form of a return value anyhow, so it was >> actually on purpose to remove it, so you can do things like easily >> measure exit times or probe places in the guest. > > If that's for tracing purpose, why don't you allocate another hypercall? > Returning to the guest without signalling that nothing was executed seems > very wrong to me. > hmm, yeah, maybe you're right. I was just debating with myself whether an undefined isn't too rough. It made sense when we didn't have any kind of handling of hvc, but now an hvc isn't really an undefined instruction, and if we assume that we have a series of hypercalls, multiplexed by a number in r0, but you don't really know what's available on your VM host, it also seems very wrong to have an ABI that says, try it, and if it's not implemented handle the undefined exception.... Know what I mean? perhaps we should return -1 in r0 instead or something. We can of course argue that we should have an HVC discovery API or that everything should be set in device tree, but the latter doesn't really fit well with the way we do things now with qemu at least. -- 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
