[PATCH 0/4] PPC: KVM: fix RESUME_GUEST checks

[PATCH 0/4] PPC: KVM: fix RESUME_GUEST checks

[Greg Kurz]

As discussed in this thread:

http://patchwork.ozlabs.org/patch/309166/

We need some consistency in the way we check whether the guest
should resume or not because:
- new RESUME_GUEST_XXX values may show up
- more locations in KVM may need to perform a similar check

This serie introduces a helper and patches the locations where it
should be called. There is yet another location in __kvmppc_vcpu_run,
but it is assembly and cannot call a C inlined function.

---

Alexey Kardashevskiy (1):
      PPC: KVM: fix VCPU run for HV KVM (v2)

Greg Kurz (3):
      PPC: KVM: introduce helper to check RESUME_GUEST and related
      PPC: KVM: fix RESUME_GUEST check before ending CEDE in kvmppc_run_core()
      PPC: KVM: fix RESUME_GUEST check before returning from kvmppc_run_core()


 arch/powerpc/include/asm/kvm_book3s.h |    5 +++++
 arch/powerpc/kvm/book3s_hv.c          |    6 +++---
 2 files changed, 8 insertions(+), 3 deletions(-)

-- 
Greg Kurz

[PATCH 1/4] PPC: KVM: introduce helper to check RESUME_GUEST and related

[Greg Kurz]

There are some locations where we check if the return value of a service
indicates that we should resume the guest. This patch introduces a helper
to handle the two possible values we currently have:
- RESUME_GUEST
- RESUME_GUEST_NV

Since it only affects Book3S HV for now, the helper is added to
the kvm_book3s.h header file.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/kvm_book3s.h |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index 83851aa..bb1e38a 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -304,6 +304,11 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu)
 	return vcpu->arch.fault_dar;
 }
 
+static inline bool is_kvmppc_resume_guest(int r)
+{
+	return (r == RESUME_GUEST || r == RESUME_GUEST_NV);
+}
+
 /* Magic register values loaded into r3 and r4 before the 'sc' assembly
  * instruction for the OSI hypercalls */
 #define OSI_SC_MAGIC_R3			0x113724FA

[PATCH 2/4] PPC: KVM: fix VCPU run for HV KVM (v2)

[Greg Kurz]

From: Alexey Kardashevskiy <aik@ozlabs.ru>

When write to MMIO happens and there is an ioeventfd for that and
is handled successfully, ioeventfd_write() returns 0 (success) and
kvmppc_handle_store() returns EMULATE_DONE. Then kvmppc_emulate_mmio()
converts EMULATE_DONE to RESUME_GUEST_NV and this broke from the loop.

This adds handling of RESUME_GUEST_NV in kvmppc_vcpu_run_hv().

changes in v2:
	- as suggesed by Alex Graf on kvm-ppc@, do it with a
          helper

Cc: Michael S. Tsirkin <mst@redhat.com>
Suggested-by: Paul Mackerras <paulus@samba.org>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 17fc949..d62dc6c 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1731,7 +1731,7 @@ static int kvmppc_vcpu_run_hv(struct kvm_run *run, struct kvm_vcpu *vcpu)
 				vcpu->arch.fault_dar, vcpu->arch.fault_dsisr);
 			srcu_read_unlock(&vcpu->kvm->srcu, srcu_idx);
 		}
-	} while (r == RESUME_GUEST);
+	} while (is_kvmppc_resume_guest(r));
 
  out:
 	vcpu->arch.state = KVMPPC_VCPU_NOTREADY;

[PATCH 3/4] PPC: KVM: fix RESUME_GUEST check before ending CEDE in kvmppc_run_core()

[Greg Kurz]

Let's use a helper for this, in case new RESUME_GUEST_XXX values
are introduced.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index d62dc6c..85df6a4 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1530,7 +1530,7 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
 		vcpu->arch.trap = 0;
 
 		if (vcpu->arch.ceded) {
-			if (ret != RESUME_GUEST)
+			if (!is_kvmppc_resume_guest(ret))
 				kvmppc_end_cede(vcpu);
 			else
 				kvmppc_set_timer(vcpu);

[PATCH 4/4] PPC: KVM: fix RESUME_GUEST check before returning from kvmppc_run_core()

[Greg Kurz]

Let's use a helper for this, in case new RESUME_GUEST_XXX values
are introduced.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 85df6a4..3b498d9 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1541,7 +1541,7 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
 	vc->vcore_state = VCORE_INACTIVE;
 	list_for_each_entry_safe(vcpu, vnext, &vc->runnable_threads,
 				 arch.run_list) {
-		if (vcpu->arch.ret != RESUME_GUEST) {
+		if (!is_kvmppc_resume_guest(vcpu->arch.ret)) {
 			kvmppc_remove_runnable(vc, vcpu);
 			wake_up(&vcpu->arch.cpu_run);
 		}

Re: [PATCH 0/4] PPC: KVM: fix RESUME_GUEST checks

[Alexander Graf]

On 06.02.2014, at 17:36, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:

> As discussed in this thread:
> 
> http://patchwork.ozlabs.org/patch/309166/
> 
> We need some consistency in the way we check whether the guest
> should resume or not because:
> - new RESUME_GUEST_XXX values may show up
> - more locations in KVM may need to perform a similar check
> 
> This serie introduces a helper and patches the locations where it
> should be called. There is yet another location in __kvmppc_vcpu_run,
> but it is assembly and cannot call a C inlined function.

Thanks, applied all to kvm-ppc-queue. I think the splitting on this one is quite excessive - a single patch would've done :).


Alex

Re: [PATCH 0/4] PPC: KVM: fix RESUME_GUEST checks

[Alexey Kardashevskiy]

On 02/07/2014 04:39 AM, Alexander Graf wrote:
> 
> On 06.02.2014, at 17:36, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> 
>> As discussed in this thread:
>>
>> http://patchwork.ozlabs.org/patch/309166/
>>
>> We need some consistency in the way we check whether the guest
>> should resume or not because:
>> - new RESUME_GUEST_XXX values may show up
>> - more locations in KVM may need to perform a similar check
>>
>> This serie introduces a helper and patches the locations where it
>> should be called. There is yet another location in __kvmppc_vcpu_run,
>> but it is assembly and cannot call a C inlined function.
> 
> Thanks, applied all to kvm-ppc-queue. I think the splitting on this one is quite excessive - a single patch would've done :).

Why did it get applied immediately? #3 or #4 (I do not remember for sure)
break HV KVM, this is why I do not repost it and keep trying Paul to reply
to the initial thread.



-- 
Alexey

Re: [PATCH 0/4] PPC: KVM: fix RESUME_GUEST checks

[Alexey Kardashevskiy]

On 02/07/2014 11:04 AM, Alexey Kardashevskiy wrote:
> On 02/07/2014 04:39 AM, Alexander Graf wrote:
>>
>> On 06.02.2014, at 17:36, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
>>
>>> As discussed in this thread:
>>>
>>> http://patchwork.ozlabs.org/patch/309166/
>>>
>>> We need some consistency in the way we check whether the guest
>>> should resume or not because:
>>> - new RESUME_GUEST_XXX values may show up
>>> - more locations in KVM may need to perform a similar check
>>>
>>> This serie introduces a helper and patches the locations where it
>>> should be called. There is yet another location in __kvmppc_vcpu_run,
>>> but it is assembly and cannot call a C inlined function.
>>
>> Thanks, applied all to kvm-ppc-queue. I think the splitting on this one is quite excessive - a single patch would've done :).
> 
> Why did it get applied immediately? #3 or #4 (I do not remember for sure)
> break HV KVM, this is why I do not repost it and keep trying Paul to reply
> to the initial thread.

Ah. No, false alarm, sorry. I think "while(!(r & RESUME_FLAG_HOST));"
failed but this is different.


-- 
Alexey

Re: [PATCH 0/4] PPC: KVM: fix RESUME_GUEST checks

[Greg Kurz]

On Thu, 6 Feb 2014 18:39:30 +0100
Alexander Graf <agraf@suse.de> wrote:
> 
> On 06.02.2014, at 17:36, Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> 
> > As discussed in this thread:
> > 
> > http://patchwork.ozlabs.org/patch/309166/
> > 
> > We need some consistency in the way we check whether the guest
> > should resume or not because:
> > - new RESUME_GUEST_XXX values may show up
> > - more locations in KVM may need to perform a similar check
> > 
> > This serie introduces a helper and patches the locations where it
> > should be called. There is yet another location in __kvmppc_vcpu_run,
> > but it is assembly and cannot call a C inlined function.
> 
> Thanks, applied all to kvm-ppc-queue. I think the splitting on this one
> is quite excessive - a single patch would've done :).
> 
> 
> Alex
> 

Heh... I paranoid ! :)

Thanks Alex !

--
Greg