| Message ID | 147916175150.16347.2494044229416144843.stgit@brijesh-build-machine (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 14/11/2016 23:15, Brijesh Singh wrote: > + /* For size less than 4 we merge, else we zero extend */ > + val = (size < 4) ? kvm_register_read(vcpu, VCPU_REGS_RAX) : 0; Are you sure it shouldn't always zero extend the high 32-bits? So "val" should be declared as u32. Paolo > + ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size, port, > + &val, 1); > + if (ret) { > + kvm_register_write(vcpu, VCPU_REGS_RAX, val); > + return ret; > + } -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/21/2016 8:50 AM, Paolo Bonzini wrote: > > > On 14/11/2016 23:15, Brijesh Singh wrote: >> + /* For size less than 4 we merge, else we zero extend */ >> + val = (size < 4) ? kvm_register_read(vcpu, VCPU_REGS_RAX) : 0; > > Are you sure it shouldn't always zero extend the high 32-bits? So "val" > should be declared as u32. > > Paolo It should only zero extend when dealing with a 32-bit operation. Any use of 8 or 16 bit registers leaves the upper 56 or 48 bits as is (see section 3.1.2.3 in http://support.amd.com/TechDocs/24592.pdf). Thanks, Tom > >> + ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size, port, >> + &val, 1); >> + if (ret) { >> + kvm_register_write(vcpu, VCPU_REGS_RAX, val); >> + return ret; >> + } -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- Original Message ----- > From: "Tom Lendacky" <thomas.lendacky@amd.com> > To: "Paolo Bonzini" <pbonzini@redhat.com>, "Brijesh Singh" <brijesh.singh@amd.com>, kvm@vger.kernel.org > Cc: rkrcmar@redhat.com, joro@8bytes.org, x86@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com, > hpa@zytor.com, tglx@linutronix.de, bp@suse.de > Sent: Tuesday, November 22, 2016 3:25:52 PM > Subject: Re: [PATCH v1 2/3] kvm: svm: Add kvm_fast_pio_in support > > On 11/21/2016 8:50 AM, Paolo Bonzini wrote: > > > > > > On 14/11/2016 23:15, Brijesh Singh wrote: > >> + /* For size less than 4 we merge, else we zero extend */ > >> + val = (size < 4) ? kvm_register_read(vcpu, VCPU_REGS_RAX) : 0; > > > > Are you sure it shouldn't always zero extend the high 32-bits? So "val" > > should be declared as u32. > > > > Paolo > > It should only zero extend when dealing with a 32-bit operation. Any use > of 8 or 16 bit registers leaves the upper 56 or 48 bits as is (see > section 3.1.2.3 in http://support.amd.com/TechDocs/24592.pdf). Duh, right, see also assign_register in arch/x86/kvm/emulate.c. Paolo > Thanks, > Tom > > > > >> + ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size, port, > >> + &val, 1); > >> + if (ret) { > >> + kvm_register_write(vcpu, VCPU_REGS_RAX, val); > >> + return ret; > >> + } > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index da07e17..77cb3f9 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1133,6 +1133,7 @@ int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr); struct x86_emulate_ctxt; int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port); +int kvm_fast_pio_in(struct kvm_vcpu *vcpu, int size, unsigned short port); void kvm_emulate_cpuid(struct kvm_vcpu *vcpu); int kvm_emulate_halt(struct kvm_vcpu *vcpu); int kvm_vcpu_halt(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 4e462bb..5e64e656 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2270,7 +2270,7 @@ static int io_interception(struct vcpu_svm *svm) ++svm->vcpu.stat.io_exits; string = (io_info & SVM_IOIO_STR_MASK) != 0; in = (io_info & SVM_IOIO_TYPE_MASK) != 0; - if (string || in) + if (string) return emulate_instruction(vcpu, 0) == EMULATE_DONE; port = io_info >> 16; @@ -2278,7 +2278,8 @@ static int io_interception(struct vcpu_svm *svm) svm->next_rip = svm->vmcb->control.exit_info_2; skip_emulated_instruction(&svm->vcpu); - return kvm_fast_pio_out(vcpu, size, port); + return in ? kvm_fast_pio_in(vcpu, size, port) + : kvm_fast_pio_out(vcpu, size, port); } static int nmi_interception(struct vcpu_svm *svm) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 3017de0..d02aeff 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5617,6 +5617,49 @@ 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_in(struct kvm_vcpu *vcpu) +{ + unsigned long val; + + /* We should only ever be called with arch.pio.count equal to 1 */ + BUG_ON(vcpu->arch.pio.count != 1); + + /* For size less than 4 we merge, else we zero extend */ + val = (vcpu->arch.pio.size < 4) ? kvm_register_read(vcpu, VCPU_REGS_RAX) + : 0; + + /* + * Since vcpu->arch.pio.count == 1 let emulator_pio_in_emulated perform + * the copy and tracing + */ + emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, vcpu->arch.pio.size, + vcpu->arch.pio.port, &val, 1); + kvm_register_write(vcpu, VCPU_REGS_RAX, val); + + return 1; +} + +int kvm_fast_pio_in(struct kvm_vcpu *vcpu, int size, unsigned short port) +{ + unsigned long val; + int ret; + + /* For size less than 4 we merge, else we zero extend */ + val = (size < 4) ? kvm_register_read(vcpu, VCPU_REGS_RAX) : 0; + + ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size, port, + &val, 1); + if (ret) { + kvm_register_write(vcpu, VCPU_REGS_RAX, val); + return ret; + } + + vcpu->arch.complete_userspace_io = complete_fast_pio_in; + + return 0; +} +EXPORT_SYMBOL_GPL(kvm_fast_pio_in); + static int kvmclock_cpu_down_prep(unsigned int cpu) { __this_cpu_write(cpu_tsc_khz, 0);