Re: [PATCH v4 07/10] ARM: KVM: Emulation framework and CP15 emulation

[Avi Kivity <avi@redhat.com>]

On 08/06/2011 01:39 PM, Christoffer Dall wrote:
> 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
> emulating the intented operation in software and skip the guest
> instruction.
>
>
>   /**
>    * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
>    * @vcpu:	The VCPU pointer
> @@ -339,6 +396,26 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>   		kvm_guest_exit();
>   		debug_ws_exit(vcpu->arch.regs.pc);
>   		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);
> +		}

Preempt notifiers mean you don't need vcpu_put()/vcpu_load() - the 
scheduler will call kvm_arch_vcpu_put/load() automatically during 
context switch.

> +
> +		if (signal_pending(current)&&  !(run->exit_reason)) {
> +			run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
> +			break;
> +		}
>   	}

You're supposed to return -EINTR on a signal.  run->exit_reason isn't 
defined in this case, but traditionally we return KVM_EXIT_INTR (which 
means host signal, not guest signal - yes it's confusing).

> +
> +/**
> + * emulate_cp15_c15_access -- emulates cp15 accesses for CRn == 15
> + * @vcpu: The VCPU pointer
> + * @p:    The coprocessor parameters struct pointer holding trap inst. details
> + *
> + * The CP15 c15 register is implementation defined, but some guest kernels
> + * attempt to read/write a diagnostics register here. We always return 0 and
> + * ignore writes and hope for the best. This may need to be refined.
> + */
> +static int emulate_cp15_c15_access(struct kvm_vcpu *vcpu,
> +				   struct coproc_params *p)
> +{
> +	trace_kvm_emulate_cp15_imp(p->Op1, p->Rt1, p->CRn, p->CRm,
> +				   p->Op2, p->is_write);

_imp?

> +
> +	if (!p->is_write)
> +		*vcpu_reg(vcpu, p->Rt1) = 0;
> +
> +	return 0;
> +}
> +
>

-- 
error compiling committee.c: too many arguments to function