Re: [PATCH v5 07/13] ARM: KVM: Emulation framework and CP15 emulation

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

 



On Mon, Dec 12, 2011 at 8:44 AM, Avi Kivity <avi@xxxxxxxxxx> wrote:
> On 12/11/2011 12:25 PM, Christoffer Dall wrote:
>> From: Christoffer Dall <cdall@xxxxxxxxxxxxxxx>
>>
>> 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.
--
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