Messages in this thread Patch in this message | | | Date | Tue, 3 Mar 2015 17:42:36 +0100 | From | Radim Krčmář <> | Subject | Re: [PATCH v3] x86: svm: use kvm_fast_pio_in() |
| |
2015-03-02 15:02-0600, Joel Schopp: > From: David Kaplan <David.Kaplan@amd.com> > > We can make the in instruction go faster the same way the out instruction is > already.
(How much faster do benchmarks run?)
> Changes from v2[Joel]: > * changed rax from u32 to unsigned long > * changed a couple return 0 to BUG_ON() > * changed 8 to sizeof(new_rax) > * added trace hook > * removed redundant clearing of count > Changes from v1[Joel] > * Added kvm_fast_pio_in() implementation that was left out of v1 > > Signed-off-by: David Kaplan <David.Kaplan@amd.com> > [extracted from larger unlrelated patch, forward ported, addressed reviews, tested] > Signed-off-by: Joel Schopp <joel.schopp@amd.com> > --- > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > @@ -5463,6 +5463,36 @@ int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port) > } > EXPORT_SYMBOL_GPL(kvm_fast_pio_out); > > +static int complete_fast_pio(struct kvm_vcpu *vcpu)
(complete_fast_pio_in()?)
> +{ > + unsigned long new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
Shouldn't we handle writes in EAX differently than in AX and AL, because of implicit zero extension.
> + > + BUG_ON(!vcpu->arch.pio.count); > + BUG_ON(vcpu->arch.pio.count * vcpu->arch.pio.size > sizeof(new_rax));
(Looking at it again, a check for 'vcpu->arch.pio.count == 1' would be sufficient.)
> + > + memcpy(&new_rax, vcpu, sizeof(new_rax)); > + trace_kvm_pio(KVM_PIO_IN, vcpu->arch.pio.port, vcpu->arch.pio.size, > + vcpu->arch.pio.count, vcpu->arch.pio_data); > + kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax); > + vcpu->arch.pio.count = 0;
I think it is better to call emulator_pio_in_emulated directly, like
emulator_pio_in_out(&vcpu->arch.emulate_ctxt, vcpu->arch.pio.size, vcpu->arch.pio.port, &new_rax, 1); kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
because we know that vcpu->arch.pio.count != 0.
Refactoring could avoid the weird vcpu->ctxt->vcpu conversion. (A better name is always welcome.)
--- diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 96a8333f3db0..d0e5b086f2e1 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4663,22 +4663,23 @@ static int emulator_pio_in_out(struct kvm_vcpu *vcpu, int size, return 0; } +static void emulator_complete_pio_in(struct kvm_vcpu *vcpu, int size, + unsigned short port, void *val, unsigned int count) +{ + memcpy(val, vcpu->arch.pio_data, size * count); + trace_kvm_pio(KVM_PIO_IN, port, size, count, vcpu->arch.pio_data); + vcpu->arch.pio.count = 0; +} + static int emulator_pio_in_emulated(struct x86_emulate_ctxt *ctxt, int size, unsigned short port, void *val, unsigned int count) { struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt); - int ret; - if (vcpu->arch.pio.count) - goto data_avail; - - ret = emulator_pio_in_out(vcpu, size, port, val, count, true); - if (ret) { -data_avail: - memcpy(val, vcpu->arch.pio_data, size * count); - trace_kvm_pio(KVM_PIO_IN, port, size, count, vcpu->arch.pio_data); - vcpu->arch.pio.count = 0; + if (vcpu->arch.pio.count || + emulator_pio_in_out(vcpu, size, port, val, count, true)) { + emulator_complete_pio_in(vcpu, size, port, val, count); return 1; }
| |