On Mon, Dec 12, 2011 at 8:44 AM, Avi Kivity <avi at redhat.com> wrote:
> On 12/11/2011 12:25 PM, Christoffer Dall wrote:
>> From: Christoffer Dall <cdall at cs.columbia.edu>
>>
>> Adds a new important function in the main KVM/ARM code called
>> handle_exit() which is called from kvm_arch_vcpu_ioctl_run() on returns
>> from guest execution. This function examines the Hyp-Syndrome-Register
>> (HSR), which contains information telling KVM what caused the exit from
>> the guest.
>>
>> Some of the reasons for an exit are CP15 accesses, which are
>> not allowed from the guest and this commits handles these exits by
>
> commit
>
>> emulating the intented operation in software and skip the guest
>
> intended
>
>> instruction.
>>
>> @@ -306,6 +307,62 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
>> ? ? ? return 0;
>> ?}
>>
>> +static inline int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? int exception_index)
>> +{
>> + ? ? unsigned long hsr_ec;
>> +
>> + ? ? if (exception_index == ARM_EXCEPTION_IRQ)
>> + ? ? ? ? ? ? return 0;
>> +
>> + ? ? if (exception_index != ARM_EXCEPTION_HVC) {
>> + ? ? ? ? ? ? kvm_err(-EINVAL, "Unsupported exception type");
>> + ? ? ? ? ? ? return -EINVAL;
>> + ? ? }
>> +
>> + ? ? hsr_ec = (vcpu->arch.hsr & HSR_EC) >> HSR_EC_SHIFT;
>> + ? ? switch (hsr_ec) {
>> + ? ? case HSR_EC_WFI:
>> + ? ? ? ? ? ? return kvm_handle_wfi(vcpu, run);
>> + ? ? case HSR_EC_CP15_32:
>> + ? ? case HSR_EC_CP15_64:
>> + ? ? ? ? ? ? return kvm_handle_cp15_access(vcpu, run);
>> + ? ? case HSR_EC_CP14_MR:
>> + ? ? ? ? ? ? return kvm_handle_cp14_access(vcpu, run);
>> + ? ? case HSR_EC_CP14_LS:
>> + ? ? ? ? ? ? return kvm_handle_cp14_load_store(vcpu, run);
>> + ? ? case HSR_EC_CP14_64:
>> + ? ? ? ? ? ? return kvm_handle_cp14_access(vcpu, run);
>> + ? ? case HSR_EC_CP_0_13:
>> + ? ? ? ? ? ? return kvm_handle_cp_0_13_access(vcpu, run);
>> + ? ? case HSR_EC_CP10_ID:
>> + ? ? ? ? ? ? return kvm_handle_cp10_id(vcpu, run);
>> + ? ? case HSR_EC_SVC_HYP:
>> + ? ? ? ? ? ? /* SVC called from Hyp mode should never get here */
>> + ? ? ? ? ? ? kvm_msg("SVC called from Hyp mode shouldn't go here");
>> + ? ? ? ? ? ? BUG();
>> + ? ? case HSR_EC_HVC:
>> + ? ? ? ? ? ? kvm_msg("hvc: %x (at %08x)", vcpu->arch.hsr & ((1 << 16) - 1),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?vcpu->arch.regs.pc);
>> + ? ? ? ? ? ? kvm_msg(" ? ? ? ? HSR: %8x", vcpu->arch.hsr);
>> + ? ? ? ? ? ? break;
>> + ? ? case HSR_EC_IABT:
>> + ? ? case HSR_EC_DABT:
>> + ? ? ? ? ? ? return kvm_handle_guest_abort(vcpu, run);
>> + ? ? case HSR_EC_IABT_HYP:
>> + ? ? case HSR_EC_DABT_HYP:
>> + ? ? ? ? ? ? /* The hypervisor should never cause aborts */
>> + ? ? ? ? ? ? kvm_msg("The hypervisor itself shouldn't cause aborts");
>> + ? ? ? ? ? ? BUG();
>> + ? ? default:
>> + ? ? ? ? ? ? kvm_msg("Unkown exception class: %08x (%08x)", hsr_ec,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? vcpu->arch.hsr);
>> + ? ? ? ? ? ? BUG();
>> + ? ? }
>
> x86 uses a function table, which is slightly nicer.
>
ok, completely stole the design from x86.
>> +
>> + ? ? return 0;
>> +}
>> +
>> ?/**
>> ? * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
>> ? * @vcpu: ? ?The VCPU pointer
>> @@ -333,6 +390,26 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> ? ? ? ? ? ? ? local_irq_enable();
>>
>> ? ? ? ? ? ? ? trace_kvm_exit(vcpu->arch.regs.pc);
>> +
>> + ? ? ? ? ? ? ret = handle_exit(vcpu, run, ret);
>> + ? ? ? ? ? ? if (ret) {
>> + ? ? ? ? ? ? ? ? ? ? kvm_err(ret, "Error in handle_exit");
>> + ? ? ? ? ? ? ? ? ? ? break;
>> + ? ? ? ? ? ? }
>> +
>> + ? ? ? ? ? ? if (run->exit_reason == KVM_EXIT_MMIO)
>> + ? ? ? ? ? ? ? ? ? ? break;
>> +
>> + ? ? ? ? ? ? if (need_resched()) {
>> + ? ? ? ? ? ? ? ? ? ? vcpu_put(vcpu);
>> + ? ? ? ? ? ? ? ? ? ? schedule();
>> + ? ? ? ? ? ? ? ? ? ? vcpu_load(vcpu);
>> + ? ? ? ? ? ? }
>
> I don't think you need the vcpu_put()/vcpu_load() here; you can replace
> the whole thing with cond_resched().
>
sorry again about the stupid patch layout.
>> +
>> + ? ? ? ? ? ? if (signal_pending(current) && !(run->exit_reason)) {
>> + ? ? ? ? ? ? ? ? ? ? run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
>
> This exit reason doesn't fit with KVM_IRQ_LINE.
>
> With KVM_INTERRUPT, userspace is responsible for determining when to
> inject interrupts, so it needs to know when guest interrupts become
> enabled (this is KVM_EXIT_IRQ_WINDOWS_OPEN). ?With KVM_IRQ_LINE,
> userspace just sets the line status, and the kernel takes care of
> everything.
>
> Oh, and you need to exit to userspace unconditionally if a signal is
> pending.
>
also fixed. sorry again agin.
>> + ? ? ? ? ? ? ? ? ? ? break;
>> + ? ? ? ? ? ? }
>> ? ? ? }
>>
>> +
>> +/******************************************************************************
>> + * Co-processor emulation
>> + */
>> +
>> +struct coproc_params {
>> + ? ? unsigned long CRm;
>> + ? ? unsigned long CRn;
>> + ? ? unsigned long Op1;
>> + ? ? unsigned long Op2;
>> + ? ? unsigned long Rt1;
>> + ? ? unsigned long Rt2;
>> + ? ? bool is_64bit;
>> + ? ? bool is_write;
>> +};
>> +
>> +#define CP15_OP(_vcpu, _params, _cp15_reg) \
>> +do { \
>> + ? ? if (_params->is_write) \
>> + ? ? ? ? ? ? _vcpu->arch.cp15._cp15_reg = *vcpu_reg(_vcpu, _params->Rt1); \
>> + ? ? else \
>> + ? ? ? ? ? ? *vcpu_reg(_vcpu, _params->Rt1) = _vcpu->arch.cp15._cp15_reg; \
>> +} while (0);
>
> Ugly. ?How about an array of registers instead?
ugly, but fast work-around. it has been fixed, wait for new patch revision.
