On 09/07/13 15:56, Dominik Dingel wrote: > This patch enables async page faults for s390 kvm guests. > It provides the userspace API to enable, disable or get the status of this > feature. Also it includes the diagnose code, called by the guest to enable > async page faults. > > The async page faults will use an already existing guest interface for this > purpose, as described in "CP Programming Services (SC24-6084)". > > Signed-off-by: Dominik Dingel <dingel@xxxxxxxxxxxxxxxxxx> Re-reading this patch again, found some thing that you should take care of (nothing major, just small details). Sorry for not seeing them earlier. [...] > + case 1: /* CANCEL */ > + if (vcpu->run->s.regs.gprs[rx] & 7) > + return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING); > + > + vcpu->run->s.regs.gprs[ry] = 0; > + > + if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID) > + vcpu->run->s.regs.gprs[ry] = 1; > + > + vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID; > + rc = 0; > + break; Dont we need a kvm_clear_async_pf_completion_queue(vcpu) or similar here as well? The cancel function is supposed to purge all outstanding requests (those were no completion signal was made pending yet) [...] > @@ -264,6 +292,7 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) > { > VCPU_EVENT(vcpu, 3, "%s", "free cpu"); > trace_kvm_s390_destroy_vcpu(vcpu->vcpu_id); > + kvm_clear_async_pf_completion_queue(vcpu); > if (!kvm_is_ucontrol(vcpu->kvm)) { > clear_bit(63 - vcpu->vcpu_id, > (unsigned long *) &vcpu->kvm->arch.sca->mcn); > @@ -313,6 +342,9 @@ void kvm_arch_destroy_vm(struct kvm *kvm) > /* Section: vcpu related */ > int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > { > + vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID; > + kvm_clear_async_pf_completion_queue(vcpu); > + kvm_async_pf_wakeup_all(vcpu); > if (kvm_is_ucontrol(vcpu->kvm)) { > vcpu->arch.gmap = gmap_alloc(current->mm); > if (!vcpu->arch.gmap) We should also reset pfault handling for CPU reset, no? > @@ -691,10 +723,75 @@ static void kvm_arch_fault_in_sync(struct kvm_vcpu *vcpu) > up_read(&mm->mmap_sem); > } > > +static void __kvm_inject_pfault_token(struct kvm_vcpu *vcpu, bool start_token, > + unsigned long token) > +{ > + struct kvm_s390_interrupt inti; > + inti.type = start_token ? KVM_S390_INT_PFAULT_INIT : > + KVM_S390_INT_PFAULT_DONE; > + inti.parm64 = token; > + if (kvm_s390_inject_vcpu(vcpu, &inti)) > + WARN(1, "pfault interrupt injection failed"); > +} The PFAULT_DONE is architectured as a floating interrupt (can happen on other CPUs). [...] > --- a/arch/s390/kvm/sigp.c > +++ b/arch/s390/kvm/sigp.c > @@ -187,6 +187,8 @@ static int __sigp_set_arch(struct kvm_vcpu *vcpu, u32 parameter) > { > int rc; > > + vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID; > + sigp set architecture affects all cpus, so we must reset pfault via kvm_for_each_vcpu, I guess. Otherwise patch looks good. I guess only one more iteration Christian -- 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