| Message ID | 20211223211528.3560711-3-farosas@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: PPC: Minor fixes | expand |
Excerpts from Fabiano Rosas's message of December 24, 2021 7:15 am: > The MMIO emulation code for vector instructions is duplicated between > VSX and VMX. When emulating VMX we should check the VMX copy size > instead of the VSX one. > > Fixes: acc9eb9305fe ("KVM: PPC: Reimplement LOAD_VMX/STORE_VMX instruction ...") > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> Good catch. AFAIKS handle_vmx_store needs the same treatment? If you agree then Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/kvm/powerpc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 1e130bb087c4..793d42bd6c8f 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -1507,7 +1507,7 @@ int kvmppc_handle_vmx_load(struct kvm_vcpu *vcpu, > { > enum emulation_result emulated = EMULATE_DONE; > > - if (vcpu->arch.mmio_vsx_copy_nums > 2) > + if (vcpu->arch.mmio_vmx_copy_nums > 2) > return EMULATE_FAIL; > > while (vcpu->arch.mmio_vmx_copy_nums) { > -- > 2.33.1 > >
Nicholas Piggin <npiggin@gmail.com> writes: > Excerpts from Fabiano Rosas's message of December 24, 2021 7:15 am: >> The MMIO emulation code for vector instructions is duplicated between >> VSX and VMX. When emulating VMX we should check the VMX copy size >> instead of the VSX one. >> >> Fixes: acc9eb9305fe ("KVM: PPC: Reimplement LOAD_VMX/STORE_VMX instruction ...") >> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > > Good catch. AFAIKS handle_vmx_store needs the same treatment? If you > agree then Half the bug now, half the bug next year... haha I'll send a v2. aside: All this duplication is kind of annoying. I'm looking into what it would take to have quadword instruction emulation here as well (Alexey caught a bug with syskaller) and the code would be really similar. I see that x86 has a more generic implementation that maybe we could take advantage of. See "f78146b0f923 (KVM: Fix page-crossing MMIO)"
On 28/12/2021 04:28, Fabiano Rosas wrote: > Nicholas Piggin <npiggin@gmail.com> writes: > >> Excerpts from Fabiano Rosas's message of December 24, 2021 7:15 am: >>> The MMIO emulation code for vector instructions is duplicated between >>> VSX and VMX. When emulating VMX we should check the VMX copy size >>> instead of the VSX one. >>> >>> Fixes: acc9eb9305fe ("KVM: PPC: Reimplement LOAD_VMX/STORE_VMX instruction ...") >>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> >> >> Good catch. AFAIKS handle_vmx_store needs the same treatment? If you >> agree then > > Half the bug now, half the bug next year... haha I'll send a v2. > > aside: > All this duplication is kind of annoying. I'm looking into what it would > take to have quadword instruction emulation here as well (Alexey caught > a bug with syskaller) and the code would be really similar. I see that > x86 has a more generic implementation that maybe we could take advantage > of. See "f78146b0f923 (KVM: Fix page-crossing MMIO)" Uff. My head exploded with vsx/vmx/vec :) But this seems to have fixed "lvx" (which is vmx, right?). Tested with: https://github.com/aik/linux/commits/my_kvm_tests
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 1e130bb087c4..793d42bd6c8f 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -1507,7 +1507,7 @@ int kvmppc_handle_vmx_load(struct kvm_vcpu *vcpu, { enum emulation_result emulated = EMULATE_DONE; - if (vcpu->arch.mmio_vsx_copy_nums > 2) + if (vcpu->arch.mmio_vmx_copy_nums > 2) return EMULATE_FAIL; while (vcpu->arch.mmio_vmx_copy_nums) {
The MMIO emulation code for vector instructions is duplicated between VSX and VMX. When emulating VMX we should check the VMX copy size instead of the VSX one. Fixes: acc9eb9305fe ("KVM: PPC: Reimplement LOAD_VMX/STORE_VMX instruction ...") Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- arch/powerpc/kvm/powerpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)