On 02.02.2010, at 12:01, Liu Yu wrote:
> Old method prematurely sets ESR and DEAR.
> Move this part after we decide to inject interrupt,
> which is more like hardware behave.
In general the patch looks good. I only have minor questions.
>
> Signed-off-by: Liu Yu <yu.liu@xxxxxxxxxxxxx>
> ---
> v2:
> use new fields queued_esr queued_dear to queue flags
>
> arch/powerpc/include/asm/kvm_host.h | 2 +
> arch/powerpc/kvm/booke.c | 58 ++++++++++++++++++++++++++---------
> arch/powerpc/kvm/emulate.c | 4 +-
> 3 files changed, 47 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 715aa6b..5e5bae7 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -259,6 +259,8 @@ struct kvm_vcpu_arch {
> #endif
> ulong fault_dear;
> ulong fault_esr;
> + ulong queued_dear;
> + ulong queued_esr;
> gpa_t paddr_accessed;
>
> u8 io_gpr; /* GPR used as IO source/target */
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index e283e44..ba93088 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -82,9 +82,31 @@ static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,
> set_bit(priority, &vcpu->arch.pending_exceptions);
> }
>
> -void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
> +void kvmppc_core_queue_dtlb_miss(struct kvm_vcpu *vcpu,
> + ulong dear_flags, ulong esr_flags)
Is this a new function? I don't see it getting removed anywhere else by this patch. Since it's not static, it needs a header change too, right? Or you just make it static.
> {
> - /* BookE does flags in ESR, so ignore those we get here */
> + vcpu->arch.queued_dear = dear_flags;
> + vcpu->arch.queued_esr = esr_flags;
> + kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DTLB_MISS);
> +}
> +
> +void kvmppc_core_queue_data_storage(struct kvm_vcpu *vcpu,
> + ulong dear_flags, ulong esr_flags)
Same as above.
> +{
> + vcpu->arch.queued_dear = dear_flags;
> + vcpu->arch.queued_esr = esr_flags;
> + kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DATA_STORAGE);
> +}
> +
> +void kvmppc_core_queue_inst_storage(struct kvm_vcpu *vcpu, ulong esr_flags)
Same as above.
> +{
> + vcpu->arch.queued_esr = esr_flags;
> + kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_INST_STORAGE);
> +}
> +
> +void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong esr_flags)
arch/powerpc/include/asm/kvm_ppc.h:extern void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags);
>
This is good.
> +{
> + vcpu->arch.queued_esr = esr_flags;
> kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
> }
>
> @@ -115,14 +137,19 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
> {
> int allowed = 0;
> ulong msr_mask;
> + bool update_esr = false, update_dear = false;
>
> switch (priority) {
> - case BOOKE_IRQPRIO_PROGRAM:
> case BOOKE_IRQPRIO_DTLB_MISS:
> - case BOOKE_IRQPRIO_ITLB_MISS:
> - case BOOKE_IRQPRIO_SYSCALL:
> case BOOKE_IRQPRIO_DATA_STORAGE:
> + update_dear = true;
> + /* fall through */
> case BOOKE_IRQPRIO_INST_STORAGE:
> + case BOOKE_IRQPRIO_PROGRAM:
> + update_esr = true;
> + /* fall through */
> + case BOOKE_IRQPRIO_ITLB_MISS:
> + case BOOKE_IRQPRIO_SYSCALL:
> case BOOKE_IRQPRIO_FP_UNAVAIL:
> case BOOKE_IRQPRIO_SPE_UNAVAIL:
> case BOOKE_IRQPRIO_SPE_FP_DATA:
> @@ -157,6 +184,10 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
> vcpu->arch.srr0 = vcpu->arch.pc;
> vcpu->arch.srr1 = vcpu->arch.msr;
> vcpu->arch.pc = vcpu->arch.ivpr | vcpu->arch.ivor[priority];
> + if (update_esr == true)
> + vcpu->arch.esr = vcpu->arch.queued_esr;
> + if (update_dear == true)
> + vcpu->arch.dear = vcpu->arch.queued_dear;
> kvmppc_set_msr(vcpu, vcpu->arch.msr & msr_mask);
>
> clear_bit(priority, &vcpu->arch.pending_exceptions);
> @@ -229,8 +260,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> if (vcpu->arch.msr & MSR_PR) {
> /* Program traps generated by user-level software must be handled
> * by the guest kernel. */
> - vcpu->arch.esr = vcpu->arch.fault_esr;
> - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
> + kvmppc_core_queue_program(vcpu, vcpu->arch.fault_esr);
> r = RESUME_GUEST;
> kvmppc_account_exit(vcpu, USR_PR_INST);
> break;
> @@ -286,16 +316,14 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> break;
>
> case BOOKE_INTERRUPT_DATA_STORAGE:
> - vcpu->arch.dear = vcpu->arch.fault_dear;
> - vcpu->arch.esr = vcpu->arch.fault_esr;
> - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DATA_STORAGE);
> + kvmppc_core_queue_data_storage(vcpu, vcpu->arch.fault_dear,
> + vcpu->arch.fault_esr);
Is that the only user of queue_data_storage? I guess it should be static then.
> kvmppc_account_exit(vcpu, DSI_EXITS);
> r = RESUME_GUEST;
> break;
>
> case BOOKE_INTERRUPT_INST_STORAGE:
> - vcpu->arch.esr = vcpu->arch.fault_esr;
> - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_INST_STORAGE);
> + kvmppc_core_queue_inst_storage(vcpu, vcpu->arch.fault_esr);
Same as above.
> kvmppc_account_exit(vcpu, ISI_EXITS);
> r = RESUME_GUEST;
> break;
> @@ -316,9 +344,9 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> gtlb_index = kvmppc_mmu_dtlb_index(vcpu, eaddr);
> if (gtlb_index < 0) {
> /* The guest didn't have a mapping for it. */
> - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DTLB_MISS);
> - vcpu->arch.dear = vcpu->arch.fault_dear;
> - vcpu->arch.esr = vcpu->arch.fault_esr;
> + kvmppc_core_queue_dtlb_miss(vcpu,
> + vcpu->arch.fault_dear,
> + vcpu->arch.fault_esr);
Same as above.
Alex--
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
