Remove the data argument and pull setting vcpu->arch.sev_pio_data into the caller; remove unnecessary clearing of vcpu->arch.pio.count when emulation is done by the kernel.
No functional change intended.
Cc: stable@vger.kernel.org Fixes: 7ed9abfe8e9f ("KVM: SVM: Support string IO operations for an SEV-ES guest") Signed-off-by: Paolo Bonzini pbonzini@redhat.com --- arch/x86/kvm/x86.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 218877e297e5..8880dc36a2b4 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -12382,34 +12382,32 @@ static int complete_sev_es_emulated_ins(struct kvm_vcpu *vcpu) }
static int kvm_sev_es_outs(struct kvm_vcpu *vcpu, unsigned int size, - unsigned int port, void *data, unsigned int count) + unsigned int port, unsigned int count) { - int ret; + int ret = emulator_pio_out(vcpu, size, port, + vcpu->arch.sev_pio_data, count);
- ret = emulator_pio_out_emulated(vcpu->arch.emulate_ctxt, size, port, - data, count); - if (ret) + if (ret) { + /* Emulation done by the kernel. */ return ret; + }
vcpu->arch.pio.count = 0; - return 0; }
static int kvm_sev_es_ins(struct kvm_vcpu *vcpu, unsigned int size, - unsigned int port, void *data, unsigned int count) + unsigned int port, unsigned int count) { - int ret; + int ret = emulator_pio_in(vcpu, size, port, + vcpu->arch.sev_pio_data, count);
- ret = emulator_pio_in_emulated(vcpu->arch.emulate_ctxt, size, port, - data, count); if (ret) { - vcpu->arch.pio.count = 0; - } else { - vcpu->arch.sev_pio_data = data; - vcpu->arch.complete_userspace_io = complete_sev_es_emulated_ins; + /* Emulation done by the kernel. */ + return ret; }
+ vcpu->arch.complete_userspace_io = complete_sev_es_emulated_ins; return 0; }
@@ -12417,8 +12415,9 @@ int kvm_sev_es_string_io(struct kvm_vcpu *vcpu, unsigned int size, unsigned int port, void *data, unsigned int count, int in) { - return in ? kvm_sev_es_ins(vcpu, size, port, data, count) - : kvm_sev_es_outs(vcpu, size, port, data, count); + vcpu->arch.sev_pio_data = data; + return in ? kvm_sev_es_ins(vcpu, size, port, count) + : kvm_sev_es_outs(vcpu, size, port, count); } EXPORT_SYMBOL_GPL(kvm_sev_es_string_io);
On Wed, 2021-10-13 at 12:56 -0400, Paolo Bonzini wrote:
Remove the data argument and pull setting vcpu->arch.sev_pio_data into the caller; remove unnecessary clearing of vcpu->arch.pio.count when emulation is done by the kernel.
You forgot to mention that you inlined the call to emulator_pio_out_emulated/emulator_pio_in_emulated.
No functional change intended.
Cc: stable@vger.kernel.org Fixes: 7ed9abfe8e9f ("KVM: SVM: Support string IO operations for an SEV-ES guest") Signed-off-by: Paolo Bonzini pbonzini@redhat.com
arch/x86/kvm/x86.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 218877e297e5..8880dc36a2b4 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -12382,34 +12382,32 @@ static int complete_sev_es_emulated_ins(struct kvm_vcpu *vcpu) } static int kvm_sev_es_outs(struct kvm_vcpu *vcpu, unsigned int size,
unsigned int port, void *data, unsigned int count)
unsigned int port, unsigned int count){
- int ret;
- int ret = emulator_pio_out(vcpu, size, port,
vcpu->arch.sev_pio_data, count);
- ret = emulator_pio_out_emulated(vcpu->arch.emulate_ctxt, size, port,
data, count);- if (ret)
- if (ret) {
/* Emulation done by the kernel. */
^^ This is a very good comment to have here!
return ret;
- }
vcpu->arch.pio.count = 0;
^^^ I wonder what the rules are for clearing vcpu->arch.pio.count for userspace PIO vm exits. Looks like complete_fast_pio_out clears it, but otherwise the only other place that clears it in this case is x86_emulate_instruction when it restarts the instuction. Do I miss something?
- return 0;
} static int kvm_sev_es_ins(struct kvm_vcpu *vcpu, unsigned int size,
unsigned int port, void *data, unsigned int count)
unsigned int port, unsigned int count){
- int ret;
- int ret = emulator_pio_in(vcpu, size, port,
vcpu->arch.sev_pio_data, count);
- ret = emulator_pio_in_emulated(vcpu->arch.emulate_ctxt, size, port,
data, count);vcpu->arch.pio.count = 0;- } else {
vcpu->arch.sev_pio_data = data;vcpu->arch.complete_userspace_io = complete_sev_es_emulated_ins;
/* Emulation done by the kernel. */return ret;
- vcpu->arch.complete_userspace_io = complete_sev_es_emulated_ins; return 0;
} @@ -12417,8 +12415,9 @@ int kvm_sev_es_string_io(struct kvm_vcpu *vcpu, unsigned int size, unsigned int port, void *data, unsigned int count, int in) {
- return in ? kvm_sev_es_ins(vcpu, size, port, data, count)
: kvm_sev_es_outs(vcpu, size, port, data, count);
- vcpu->arch.sev_pio_data = data;
- return in ? kvm_sev_es_ins(vcpu, size, port, count)
: kvm_sev_es_outs(vcpu, size, port, count);} EXPORT_SYMBOL_GPL(kvm_sev_es_string_io);
Looks OK to me, I might have missed something though.
Reviewed-by: Maxim Levitsky mlevitsk@redhat.com
Best regards, Maxim Levitsky
On 22/10/21 01:14, Maxim Levitsky wrote:
vcpu->arch.pio.count = 0;
^^^ I wonder what the rules are for clearing vcpu->arch.pio.count for userspace PIO vm exits. Looks like complete_fast_pio_out clears it, but otherwise the only other place that clears it in this case is x86_emulate_instruction when it restarts the instuction. Do I miss something?
For IN, it is cleared by the completion callback.
For OUT, it can be cleared either by the completion callback or before calling it, because the completion callback will not need it. I would like to standardize towards clearing it in the callback for out, too, even if sometimes it's unnecessary to have a callback in the first place; this is what patch 8 does for example. This way vcpu->arch.pio.count > 0 tells you whether the other fields have a recent value.
Paolo
linux-stable-mirror@lists.linaro.org
-
Maxim Levitsky -
Paolo Bonzini