The size of the data in the scratch buffer is not divided by the size of each port I/O operation, so vcpu->arch.pio.count ends up being larger than it should be by a factor of size.
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/svm/sev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index c36b5fe4c27c..e672493b5d8d 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2583,7 +2583,7 @@ int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in) return -EINVAL;
return kvm_sev_es_string_io(&svm->vcpu, size, port, - svm->ghcb_sa, svm->ghcb_sa_len, in); + svm->ghcb_sa, svm->ghcb_sa_len / size, in); }
void sev_es_init_vmcb(struct vcpu_svm *svm)
On 10/13/21 11:56 AM, Paolo Bonzini wrote:
The size of the data in the scratch buffer is not divided by the size of each port I/O operation, so vcpu->arch.pio.count ends up being larger than it should be by a factor of size.
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
Ugh, can't believe I did that...
Acked-by: Tom Lendacky thomas.lendacky@amd.com
arch/x86/kvm/svm/sev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index c36b5fe4c27c..e672493b5d8d 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2583,7 +2583,7 @@ int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in) return -EINVAL; return kvm_sev_es_string_io(&svm->vcpu, size, port,
svm->ghcb_sa, svm->ghcb_sa_len, in);
svm->ghcb_sa, svm->ghcb_sa_len / size, in);void sev_es_init_vmcb(struct vcpu_svm *svm)
On Wed, 2021-10-13 at 12:56 -0400, Paolo Bonzini wrote:
The size of the data in the scratch buffer is not divided by the size of each port I/O operation, so vcpu->arch.pio.count ends up being larger than it should be by a factor of size.
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/svm/sev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index c36b5fe4c27c..e672493b5d8d 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2583,7 +2583,7 @@ int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in) return -EINVAL; return kvm_sev_es_string_io(&svm->vcpu, size, port,
svm->ghcb_sa, svm->ghcb_sa_len, in);
svm->ghcb_sa, svm->ghcb_sa_len / size, in);} void sev_es_init_vmcb(struct vcpu_svm *svm)
This ends in kvm_sev_es_ins/outs and both indeed expect count of operations which they pass to emulator_pio_{out|in}_emulated
Reviewed-by: Maxim Levitsky mlevitsk@redhat.com
Best regards, Maxim Levitsky
On Wed, Oct 13, 2021 at 9:56 AM Paolo Bonzini pbonzini@redhat.com wrote:
The size of the data in the scratch buffer is not divided by the size of each port I/O operation, so vcpu->arch.pio.count ends up being larger than it should be by a factor of size.
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/svm/sev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index c36b5fe4c27c..e672493b5d8d 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2583,7 +2583,7 @@ int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in) return -EINVAL;
return kvm_sev_es_string_io(&svm->vcpu, size, port,
svm->ghcb_sa, svm->ghcb_sa_len, in);
svm->ghcb_sa, svm->ghcb_sa_len / size, in);}
void sev_es_init_vmcb(struct vcpu_svm *svm)
2.27.0
I could be missing something, but I'm pretty sure that this is wrong. The GHCB spec says that `exit_info_2` is the `rep` count. Not the string length.
For example, given a `rep outsw` instruction, with `ECX` set to `8`, the rep count written into `SW_EXITINFO2` should be eight x86 words (i.e., 16 bytes) and the IO size should be one x86 word (i.e., 2 bytes). In other words, the code was correct before this patch. This patch is incorrectly dividing the rep count by the IO size, causing the string IO to be truncated.
On 25/10/21 03:31, Marc Orr wrote:
I could be missing something, but I'm pretty sure that this is wrong. The GHCB spec says that `exit_info_2` is the `rep` count. Not the string length.
For example, given a `rep outsw` instruction, with `ECX` set to `8`, the rep count written into `SW_EXITINFO2` should be eight x86 words (i.e., 16 bytes) and the IO size should be one x86 word (i.e., 2 bytes). In other words, the code was correct before this patch. This patch is incorrectly dividing the rep count by the IO size, causing the string IO to be truncated.
Then what's wrong is _also_ the call to setup_vmgexit_scratch, because that one definitely expects bytes:
scratch_va = kzalloc(len, GFP_KERNEL_ACCOUNT);
Paolo
linux-stable-mirror@lists.linaro.org
-
Marc Orr -
Maxim Levitsky -
Paolo Bonzini -
Tom Lendacky