All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-24  1:42 ` Paul Mackerras
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Mackerras @ 2013-05-24  1:42 UTC (permalink / raw)
  To: kvm-ppc, kvm, linuxppc-dev; +Cc: Alexander Graf, Benjamin Herrenschmidt

This adds the remaining two hypercalls defined by PAPR for manipulating
the XICS interrupt controller, H_IPOLL and H_XIRR_X.  H_IPOLL returns
information about the priority and pending interrupts for a virtual
cpu, without changing any state.  H_XIRR_X is like H_XIRR in that it
reads and acknowledges the highest-priority pending interrupt, but it
also returns the timestamp (timebase register value) from when the
interrupt was first received by the hypervisor.  Currently we just
return the current time, since we don't do any software queueing of
virtual interrupts inside the XICS emulation code.

These hcalls are not currently used by Linux guests, but may be in
future.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
Unfortunately I missed these two hcalls in the previous submissions.
It would be good to get this patch into 3.10 so we don't have a
kernel version with these calls missing from the API, in case future
guest kernels want to use them.

Alex, given you're on vacation at the moment, are you OK with Ben
taking this through his tree?

 arch/powerpc/include/asm/hvcall.h |    1 +
 arch/powerpc/kvm/book3s_hv.c      |    2 ++
 arch/powerpc/kvm/book3s_pr_papr.c |    2 ++
 arch/powerpc/kvm/book3s_xics.c    |   29 +++++++++++++++++++++++++++++
 4 files changed, 34 insertions(+)

diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index cf4df8e..0c7f2bf 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -264,6 +264,7 @@
 #define H_GET_MPP		0x2D4
 #define H_HOME_NODE_ASSOCIATIVITY 0x2EC
 #define H_BEST_ENERGY		0x2F4
+#define H_XIRR_X		0x2FC
 #define H_RANDOM		0x300
 #define H_COP			0x304
 #define H_GET_MPP_X		0x314
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 065c9df..7e2059e 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -562,6 +562,8 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
 	case H_CPPR:
 	case H_EOI:
 	case H_IPI:
+	case H_IPOLL:
+	case H_XIRR_X:
 		if (kvmppc_xics_enabled(vcpu)) {
 			ret = kvmppc_xics_hcall(vcpu, req);
 			break;
diff --git a/arch/powerpc/kvm/book3s_pr_papr.c b/arch/powerpc/kvm/book3s_pr_papr.c
index 3750e3c..91d4b45 100644
--- a/arch/powerpc/kvm/book3s_pr_papr.c
+++ b/arch/powerpc/kvm/book3s_pr_papr.c
@@ -292,6 +292,8 @@ int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned long cmd)
 	case H_CPPR:
 	case H_EOI:
 	case H_IPI:
+	case H_IPOLL:
+	case H_XIRR_X:
 		if (kvmppc_xics_enabled(vcpu))
 			return kvmppc_h_pr_xics_hcall(vcpu, cmd);
 		break;
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index f7a1037..94c1dd4 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -650,6 +650,23 @@ static noinline int kvmppc_h_ipi(struct kvm_vcpu *vcpu, unsigned long server,
 	return H_SUCCESS;
 }
 
+static int kvmppc_h_ipoll(struct kvm_vcpu *vcpu, unsigned long server)
+{
+	union kvmppc_icp_state state;
+	struct kvmppc_icp *icp;
+
+	icp = vcpu->arch.icp;
+	if (icp->server_num != server) {
+		icp = kvmppc_xics_find_server(vcpu->kvm, server);
+		if (!icp)
+			return H_PARAMETER;
+	}
+	state = ACCESS_ONCE(icp->state);
+	kvmppc_set_gpr(vcpu, 4, ((u32)state.cppr << 24) | state.xisr);
+	kvmppc_set_gpr(vcpu, 5, state.mfrr);
+	return H_SUCCESS;
+}
+
 static noinline void kvmppc_h_cppr(struct kvm_vcpu *vcpu, unsigned long cppr)
 {
 	union kvmppc_icp_state old_state, new_state;
@@ -787,6 +804,18 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
 	if (!xics || !vcpu->arch.icp)
 		return H_HARDWARE;
 
+	/* These requests don't have real-mode implementations at present */
+	switch (req) {
+	case H_XIRR_X:
+		res = kvmppc_h_xirr(vcpu);
+		kvmppc_set_gpr(vcpu, 4, res);
+		kvmppc_set_gpr(vcpu, 5, get_tb());
+		return rc;
+	case H_IPOLL:
+		rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
+		return rc;
+	}
+
 	/* Check for real mode returning too hard */
 	if (xics->real_mode)
 		return kvmppc_xics_rm_complete(vcpu, req);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-24  1:42 ` Paul Mackerras
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Mackerras @ 2013-05-24  1:42 UTC (permalink / raw)
  To: kvm-ppc, kvm, linuxppc-dev; +Cc: Alexander Graf

This adds the remaining two hypercalls defined by PAPR for manipulating
the XICS interrupt controller, H_IPOLL and H_XIRR_X.  H_IPOLL returns
information about the priority and pending interrupts for a virtual
cpu, without changing any state.  H_XIRR_X is like H_XIRR in that it
reads and acknowledges the highest-priority pending interrupt, but it
also returns the timestamp (timebase register value) from when the
interrupt was first received by the hypervisor.  Currently we just
return the current time, since we don't do any software queueing of
virtual interrupts inside the XICS emulation code.

These hcalls are not currently used by Linux guests, but may be in
future.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
Unfortunately I missed these two hcalls in the previous submissions.
It would be good to get this patch into 3.10 so we don't have a
kernel version with these calls missing from the API, in case future
guest kernels want to use them.

Alex, given you're on vacation at the moment, are you OK with Ben
taking this through his tree?

 arch/powerpc/include/asm/hvcall.h |    1 +
 arch/powerpc/kvm/book3s_hv.c      |    2 ++
 arch/powerpc/kvm/book3s_pr_papr.c |    2 ++
 arch/powerpc/kvm/book3s_xics.c    |   29 +++++++++++++++++++++++++++++
 4 files changed, 34 insertions(+)

diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index cf4df8e..0c7f2bf 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -264,6 +264,7 @@
 #define H_GET_MPP		0x2D4
 #define H_HOME_NODE_ASSOCIATIVITY 0x2EC
 #define H_BEST_ENERGY		0x2F4
+#define H_XIRR_X		0x2FC
 #define H_RANDOM		0x300
 #define H_COP			0x304
 #define H_GET_MPP_X		0x314
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 065c9df..7e2059e 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -562,6 +562,8 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
 	case H_CPPR:
 	case H_EOI:
 	case H_IPI:
+	case H_IPOLL:
+	case H_XIRR_X:
 		if (kvmppc_xics_enabled(vcpu)) {
 			ret = kvmppc_xics_hcall(vcpu, req);
 			break;
diff --git a/arch/powerpc/kvm/book3s_pr_papr.c b/arch/powerpc/kvm/book3s_pr_papr.c
index 3750e3c..91d4b45 100644
--- a/arch/powerpc/kvm/book3s_pr_papr.c
+++ b/arch/powerpc/kvm/book3s_pr_papr.c
@@ -292,6 +292,8 @@ int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned long cmd)
 	case H_CPPR:
 	case H_EOI:
 	case H_IPI:
+	case H_IPOLL:
+	case H_XIRR_X:
 		if (kvmppc_xics_enabled(vcpu))
 			return kvmppc_h_pr_xics_hcall(vcpu, cmd);
 		break;
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index f7a1037..94c1dd4 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -650,6 +650,23 @@ static noinline int kvmppc_h_ipi(struct kvm_vcpu *vcpu, unsigned long server,
 	return H_SUCCESS;
 }
 
+static int kvmppc_h_ipoll(struct kvm_vcpu *vcpu, unsigned long server)
+{
+	union kvmppc_icp_state state;
+	struct kvmppc_icp *icp;
+
+	icp = vcpu->arch.icp;
+	if (icp->server_num != server) {
+		icp = kvmppc_xics_find_server(vcpu->kvm, server);
+		if (!icp)
+			return H_PARAMETER;
+	}
+	state = ACCESS_ONCE(icp->state);
+	kvmppc_set_gpr(vcpu, 4, ((u32)state.cppr << 24) | state.xisr);
+	kvmppc_set_gpr(vcpu, 5, state.mfrr);
+	return H_SUCCESS;
+}
+
 static noinline void kvmppc_h_cppr(struct kvm_vcpu *vcpu, unsigned long cppr)
 {
 	union kvmppc_icp_state old_state, new_state;
@@ -787,6 +804,18 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
 	if (!xics || !vcpu->arch.icp)
 		return H_HARDWARE;
 
+	/* These requests don't have real-mode implementations at present */
+	switch (req) {
+	case H_XIRR_X:
+		res = kvmppc_h_xirr(vcpu);
+		kvmppc_set_gpr(vcpu, 4, res);
+		kvmppc_set_gpr(vcpu, 5, get_tb());
+		return rc;
+	case H_IPOLL:
+		rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
+		return rc;
+	}
+
 	/* Check for real mode returning too hard */
 	if (xics->real_mode)
 		return kvmppc_xics_rm_complete(vcpu, req);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-24  1:42 ` Paul Mackerras
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Mackerras @ 2013-05-24  1:42 UTC (permalink / raw)
  To: kvm-ppc, kvm, linuxppc-dev; +Cc: Alexander Graf, Benjamin Herrenschmidt

This adds the remaining two hypercalls defined by PAPR for manipulating
the XICS interrupt controller, H_IPOLL and H_XIRR_X.  H_IPOLL returns
information about the priority and pending interrupts for a virtual
cpu, without changing any state.  H_XIRR_X is like H_XIRR in that it
reads and acknowledges the highest-priority pending interrupt, but it
also returns the timestamp (timebase register value) from when the
interrupt was first received by the hypervisor.  Currently we just
return the current time, since we don't do any software queueing of
virtual interrupts inside the XICS emulation code.

These hcalls are not currently used by Linux guests, but may be in
future.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
Unfortunately I missed these two hcalls in the previous submissions.
It would be good to get this patch into 3.10 so we don't have a
kernel version with these calls missing from the API, in case future
guest kernels want to use them.

Alex, given you're on vacation at the moment, are you OK with Ben
taking this through his tree?

 arch/powerpc/include/asm/hvcall.h |    1 +
 arch/powerpc/kvm/book3s_hv.c      |    2 ++
 arch/powerpc/kvm/book3s_pr_papr.c |    2 ++
 arch/powerpc/kvm/book3s_xics.c    |   29 +++++++++++++++++++++++++++++
 4 files changed, 34 insertions(+)

diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index cf4df8e..0c7f2bf 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -264,6 +264,7 @@
 #define H_GET_MPP		0x2D4
 #define H_HOME_NODE_ASSOCIATIVITY 0x2EC
 #define H_BEST_ENERGY		0x2F4
+#define H_XIRR_X		0x2FC
 #define H_RANDOM		0x300
 #define H_COP			0x304
 #define H_GET_MPP_X		0x314
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 065c9df..7e2059e 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -562,6 +562,8 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
 	case H_CPPR:
 	case H_EOI:
 	case H_IPI:
+	case H_IPOLL:
+	case H_XIRR_X:
 		if (kvmppc_xics_enabled(vcpu)) {
 			ret = kvmppc_xics_hcall(vcpu, req);
 			break;
diff --git a/arch/powerpc/kvm/book3s_pr_papr.c b/arch/powerpc/kvm/book3s_pr_papr.c
index 3750e3c..91d4b45 100644
--- a/arch/powerpc/kvm/book3s_pr_papr.c
+++ b/arch/powerpc/kvm/book3s_pr_papr.c
@@ -292,6 +292,8 @@ int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned long cmd)
 	case H_CPPR:
 	case H_EOI:
 	case H_IPI:
+	case H_IPOLL:
+	case H_XIRR_X:
 		if (kvmppc_xics_enabled(vcpu))
 			return kvmppc_h_pr_xics_hcall(vcpu, cmd);
 		break;
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index f7a1037..94c1dd4 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -650,6 +650,23 @@ static noinline int kvmppc_h_ipi(struct kvm_vcpu *vcpu, unsigned long server,
 	return H_SUCCESS;
 }
 
+static int kvmppc_h_ipoll(struct kvm_vcpu *vcpu, unsigned long server)
+{
+	union kvmppc_icp_state state;
+	struct kvmppc_icp *icp;
+
+	icp = vcpu->arch.icp;
+	if (icp->server_num != server) {
+		icp = kvmppc_xics_find_server(vcpu->kvm, server);
+		if (!icp)
+			return H_PARAMETER;
+	}
+	state = ACCESS_ONCE(icp->state);
+	kvmppc_set_gpr(vcpu, 4, ((u32)state.cppr << 24) | state.xisr);
+	kvmppc_set_gpr(vcpu, 5, state.mfrr);
+	return H_SUCCESS;
+}
+
 static noinline void kvmppc_h_cppr(struct kvm_vcpu *vcpu, unsigned long cppr)
 {
 	union kvmppc_icp_state old_state, new_state;
@@ -787,6 +804,18 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
 	if (!xics || !vcpu->arch.icp)
 		return H_HARDWARE;
 
+	/* These requests don't have real-mode implementations at present */
+	switch (req) {
+	case H_XIRR_X:
+		res = kvmppc_h_xirr(vcpu);
+		kvmppc_set_gpr(vcpu, 4, res);
+		kvmppc_set_gpr(vcpu, 5, get_tb());
+		return rc;
+	case H_IPOLL:
+		rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
+		return rc;
+	}
+
 	/* Check for real mode returning too hard */
 	if (xics->real_mode)
 		return kvmppc_xics_rm_complete(vcpu, req);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
  2013-05-24  1:42 ` Paul Mackerras
  (?)
@ 2013-05-28 17:41   ` Scott Wood
  -1 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2013-05-28 17:41 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: kvm-ppc, kvm, linuxppc-dev, Alexander Graf,
	Benjamin Herrenschmidt, Gleb Natapov, Marcelo Tosatti

On 05/23/2013 08:42:21 PM, Paul Mackerras wrote:
> This adds the remaining two hypercalls defined by PAPR for  
> manipulating
> the XICS interrupt controller, H_IPOLL and H_XIRR_X.  H_IPOLL returns
> information about the priority and pending interrupts for a virtual
> cpu, without changing any state.  H_XIRR_X is like H_XIRR in that it
> reads and acknowledges the highest-priority pending interrupt, but it
> also returns the timestamp (timebase register value) from when the
> interrupt was first received by the hypervisor.  Currently we just
> return the current time, since we don't do any software queueing of
> virtual interrupts inside the XICS emulation code.
> 
> These hcalls are not currently used by Linux guests, but may be in
> future.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
> Unfortunately I missed these two hcalls in the previous submissions.
> It would be good to get this patch into 3.10 so we don't have a
> kernel version with these calls missing from the API, in case future
> guest kernels want to use them.
> 
> Alex, given you're on vacation at the moment, are you OK with Ben
> taking this through his tree?

I believe Alex is staying far away from e-mail on his vacation.  He's  
asked me to fill in for him while he's gone.

The patch itself seems reasonable (though I don't know much about XICS,  
and do have one question...), but I'll leave it up to Gleb/Marcelo/Ben  
if it should go in for 3.10 and via which tree.  I understand the  
desire to not have an incomplete ABI in a released version, but Linus  
is already grumbling about how much went into rc3, and you say the  
hcalls aren't currently used...  Are they likely to be used in any  
timeframe in which we'd reasonably care about 3.10?  If so, would the  
effect of not having them implemented be such that it would be worse  
than not having in-kernel XICS at all?

> @@ -787,6 +804,18 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32  
> req)
>  	if (!xics || !vcpu->arch.icp)
>  		return H_HARDWARE;
> 
> +	/* These requests don't have real-mode implementations at  
> present */
> +	switch (req) {
> +	case H_XIRR_X:
> +		res = kvmppc_h_xirr(vcpu);
> +		kvmppc_set_gpr(vcpu, 4, res);
> +		kvmppc_set_gpr(vcpu, 5, get_tb());
> +		return rc;
> +	case H_IPOLL:
> +		rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
> +		return rc;
> +	}
> +
>  	/* Check for real mode returning too hard */
>  	if (xics->real_mode)
>  		return kvmppc_xics_rm_complete(vcpu, req);

Could you explain what's going on here relative to  
kvmppc_xics_rm_complete()?  What does "returning too hard" mean, and  
why must rm_action not be checked for these hcalls?

-Scott

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-28 17:41   ` Scott Wood
  0 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2013-05-28 17:41 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: kvm, Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm-ppc,
	linuxppc-dev

On 05/23/2013 08:42:21 PM, Paul Mackerras wrote:
> This adds the remaining two hypercalls defined by PAPR for =20
> manipulating
> the XICS interrupt controller, H_IPOLL and H_XIRR_X.  H_IPOLL returns
> information about the priority and pending interrupts for a virtual
> cpu, without changing any state.  H_XIRR_X is like H_XIRR in that it
> reads and acknowledges the highest-priority pending interrupt, but it
> also returns the timestamp (timebase register value) from when the
> interrupt was first received by the hypervisor.  Currently we just
> return the current time, since we don't do any software queueing of
> virtual interrupts inside the XICS emulation code.
>=20
> These hcalls are not currently used by Linux guests, but may be in
> future.
>=20
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
> Unfortunately I missed these two hcalls in the previous submissions.
> It would be good to get this patch into 3.10 so we don't have a
> kernel version with these calls missing from the API, in case future
> guest kernels want to use them.
>=20
> Alex, given you're on vacation at the moment, are you OK with Ben
> taking this through his tree?

I believe Alex is staying far away from e-mail on his vacation.  He's =20
asked me to fill in for him while he's gone.

The patch itself seems reasonable (though I don't know much about XICS, =20
and do have one question...), but I'll leave it up to Gleb/Marcelo/Ben =20
if it should go in for 3.10 and via which tree.  I understand the =20
desire to not have an incomplete ABI in a released version, but Linus =20
is already grumbling about how much went into rc3, and you say the =20
hcalls aren't currently used...  Are they likely to be used in any =20
timeframe in which we'd reasonably care about 3.10?  If so, would the =20
effect of not having them implemented be such that it would be worse =20
than not having in-kernel XICS at all?

> @@ -787,6 +804,18 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 =20
> req)
>  	if (!xics || !vcpu->arch.icp)
>  		return H_HARDWARE;
>=20
> +	/* These requests don't have real-mode implementations at =20
> present */
> +	switch (req) {
> +	case H_XIRR_X:
> +		res =3D kvmppc_h_xirr(vcpu);
> +		kvmppc_set_gpr(vcpu, 4, res);
> +		kvmppc_set_gpr(vcpu, 5, get_tb());
> +		return rc;
> +	case H_IPOLL:
> +		rc =3D kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
> +		return rc;
> +	}
> +
>  	/* Check for real mode returning too hard */
>  	if (xics->real_mode)
>  		return kvmppc_xics_rm_complete(vcpu, req);

Could you explain what's going on here relative to =20
kvmppc_xics_rm_complete()?  What does "returning too hard" mean, and =20
why must rm_action not be checked for these hcalls?

-Scott=

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-28 17:41   ` Scott Wood
  0 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2013-05-28 17:41 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: kvm-ppc, kvm, linuxppc-dev, Alexander Graf,
	Benjamin Herrenschmidt, Gleb Natapov, Marcelo Tosatti

On 05/23/2013 08:42:21 PM, Paul Mackerras wrote:
> This adds the remaining two hypercalls defined by PAPR for  
> manipulating
> the XICS interrupt controller, H_IPOLL and H_XIRR_X.  H_IPOLL returns
> information about the priority and pending interrupts for a virtual
> cpu, without changing any state.  H_XIRR_X is like H_XIRR in that it
> reads and acknowledges the highest-priority pending interrupt, but it
> also returns the timestamp (timebase register value) from when the
> interrupt was first received by the hypervisor.  Currently we just
> return the current time, since we don't do any software queueing of
> virtual interrupts inside the XICS emulation code.
> 
> These hcalls are not currently used by Linux guests, but may be in
> future.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
> Unfortunately I missed these two hcalls in the previous submissions.
> It would be good to get this patch into 3.10 so we don't have a
> kernel version with these calls missing from the API, in case future
> guest kernels want to use them.
> 
> Alex, given you're on vacation at the moment, are you OK with Ben
> taking this through his tree?

I believe Alex is staying far away from e-mail on his vacation.  He's  
asked me to fill in for him while he's gone.

The patch itself seems reasonable (though I don't know much about XICS,  
and do have one question...), but I'll leave it up to Gleb/Marcelo/Ben  
if it should go in for 3.10 and via which tree.  I understand the  
desire to not have an incomplete ABI in a released version, but Linus  
is already grumbling about how much went into rc3, and you say the  
hcalls aren't currently used...  Are they likely to be used in any  
timeframe in which we'd reasonably care about 3.10?  If so, would the  
effect of not having them implemented be such that it would be worse  
than not having in-kernel XICS at all?

> @@ -787,6 +804,18 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32  
> req)
>  	if (!xics || !vcpu->arch.icp)
>  		return H_HARDWARE;
> 
> +	/* These requests don't have real-mode implementations at  
> present */
> +	switch (req) {
> +	case H_XIRR_X:
> +		res = kvmppc_h_xirr(vcpu);
> +		kvmppc_set_gpr(vcpu, 4, res);
> +		kvmppc_set_gpr(vcpu, 5, get_tb());
> +		return rc;
> +	case H_IPOLL:
> +		rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
> +		return rc;
> +	}
> +
>  	/* Check for real mode returning too hard */
>  	if (xics->real_mode)
>  		return kvmppc_xics_rm_complete(vcpu, req);

Could you explain what's going on here relative to  
kvmppc_xics_rm_complete()?  What does "returning too hard" mean, and  
why must rm_action not be checked for these hcalls?

-Scott

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
  2013-05-28 17:41   ` Scott Wood
@ 2013-05-29  0:41     ` Benjamin Herrenschmidt
  -1 siblings, 0 replies; 17+ messages in thread
From: Benjamin Herrenschmidt @ 2013-05-29  0:41 UTC (permalink / raw)
  To: Scott Wood
  Cc: kvm, Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm-ppc,
	linuxppc-dev, Paul Mackerras

On Tue, 2013-05-28 at 12:41 -0500, Scott Wood wrote:

> I believe Alex is staying far away from e-mail on his vacation.  He's  
> asked me to fill in for him while he's gone.
> 
> The patch itself seems reasonable (though I don't know much about XICS,  
> and do have one question...), but I'll leave it up to Gleb/Marcelo/Ben  
> if it should go in for 3.10 and via which tree.  I understand the  
> desire to not have an incomplete ABI in a released version, but Linus  
> is already grumbling about how much went into rc3, and you say the  
> hcalls aren't currently used...  Are they likely to be used in any  
> timeframe in which we'd reasonably care about 3.10? 

Yes. I'd like to have them in. Their implementation is actually fairly
trivial and they cannot be emulated by qemu if the rest of the XICS is
in the kernel, so it's a problem.

>  If so, would the  
> effect of not having them implemented be such that it would be worse  
> than not having in-kernel XICS at all?

Well, that would mean that running that potential future kernel that
uses them (or some other OS that already uses them) would not work
without the user explicitly disable in kernel irq chip.

> > @@ -787,6 +804,18 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32  
> > req)
> >  	if (!xics || !vcpu->arch.icp)
> >  		return H_HARDWARE;
> > 
> > +	/* These requests don't have real-mode implementations at  
> > present */
> > +	switch (req) {
> > +	case H_XIRR_X:
> > +		res = kvmppc_h_xirr(vcpu);
> > +		kvmppc_set_gpr(vcpu, 4, res);
> > +		kvmppc_set_gpr(vcpu, 5, get_tb());
> > +		return rc;
> > +	case H_IPOLL:
> > +		rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
> > +		return rc;
> > +	}
> > +
> >  	/* Check for real mode returning too hard */
> >  	if (xics->real_mode)
> >  		return kvmppc_xics_rm_complete(vcpu, req);
> 
> Could you explain what's going on here relative to  
> kvmppc_xics_rm_complete()?  What does "returning too hard" mean, and  
> why must rm_action not be checked for these hcalls?

This is related to how we handle some hcalls in real mode as a fast
path. The real-mode stuff cannot handle cases that require for example a
re-emit of the interrupt, a reject, etc... so in some cases, it returns
H_TOO_HARD which causes KVM to exit and try to handle the hcall again in
kernel virtual mode.

When doing so as the result of a XICS hcall, it sets a bit mask of
"tasks" to handle in virtual mode (because it will have already
partially done the operation, it cannot just re-play the whole hcall).

So when real-mode is supported we must not just call the normal virtual
mode version of the hcalls, we instead go to kvmppc_xics_rm_complete()
to handle those "tasks".

However, for those 2 "missing" hcalls, we have no real mode
implementation at all (we didn't bother, we will do that later if
needed, it's purely a performance issue). So we need to fully handle
them in virtual mode, and we know there will be no "tasks" to handle in
rm_complete.

We could have done a real mode variant but we wanted to keep the patch
as small as possible for 3.10.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-29  0:41     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 17+ messages in thread
From: Benjamin Herrenschmidt @ 2013-05-29  0:41 UTC (permalink / raw)
  To: Scott Wood
  Cc: kvm, Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm-ppc,
	linuxppc-dev, Paul Mackerras

On Tue, 2013-05-28 at 12:41 -0500, Scott Wood wrote:

> I believe Alex is staying far away from e-mail on his vacation.  He's  
> asked me to fill in for him while he's gone.
> 
> The patch itself seems reasonable (though I don't know much about XICS,  
> and do have one question...), but I'll leave it up to Gleb/Marcelo/Ben  
> if it should go in for 3.10 and via which tree.  I understand the  
> desire to not have an incomplete ABI in a released version, but Linus  
> is already grumbling about how much went into rc3, and you say the  
> hcalls aren't currently used...  Are they likely to be used in any  
> timeframe in which we'd reasonably care about 3.10? 

Yes. I'd like to have them in. Their implementation is actually fairly
trivial and they cannot be emulated by qemu if the rest of the XICS is
in the kernel, so it's a problem.

>  If so, would the  
> effect of not having them implemented be such that it would be worse  
> than not having in-kernel XICS at all?

Well, that would mean that running that potential future kernel that
uses them (or some other OS that already uses them) would not work
without the user explicitly disable in kernel irq chip.

> > @@ -787,6 +804,18 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32  
> > req)
> >  	if (!xics || !vcpu->arch.icp)
> >  		return H_HARDWARE;
> > 
> > +	/* These requests don't have real-mode implementations at  
> > present */
> > +	switch (req) {
> > +	case H_XIRR_X:
> > +		res = kvmppc_h_xirr(vcpu);
> > +		kvmppc_set_gpr(vcpu, 4, res);
> > +		kvmppc_set_gpr(vcpu, 5, get_tb());
> > +		return rc;
> > +	case H_IPOLL:
> > +		rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
> > +		return rc;
> > +	}
> > +
> >  	/* Check for real mode returning too hard */
> >  	if (xics->real_mode)
> >  		return kvmppc_xics_rm_complete(vcpu, req);
> 
> Could you explain what's going on here relative to  
> kvmppc_xics_rm_complete()?  What does "returning too hard" mean, and  
> why must rm_action not be checked for these hcalls?

This is related to how we handle some hcalls in real mode as a fast
path. The real-mode stuff cannot handle cases that require for example a
re-emit of the interrupt, a reject, etc... so in some cases, it returns
H_TOO_HARD which causes KVM to exit and try to handle the hcall again in
kernel virtual mode.

When doing so as the result of a XICS hcall, it sets a bit mask of
"tasks" to handle in virtual mode (because it will have already
partially done the operation, it cannot just re-play the whole hcall).

So when real-mode is supported we must not just call the normal virtual
mode version of the hcalls, we instead go to kvmppc_xics_rm_complete()
to handle those "tasks".

However, for those 2 "missing" hcalls, we have no real mode
implementation at all (we didn't bother, we will do that later if
needed, it's purely a performance issue). So we need to fully handle
them in virtual mode, and we know there will be no "tasks" to handle in
rm_complete.

We could have done a real mode variant but we wanted to keep the patch
as small as possible for 3.10.

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
  2013-05-29  0:41     ` Benjamin Herrenschmidt
  (?)
@ 2013-05-29 23:38       ` Scott Wood
  -1 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2013-05-29 23:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Paul Mackerras, kvm-ppc, kvm, linuxppc-dev, Alexander Graf,
	Gleb Natapov, Marcelo Tosatti

On 05/28/2013 07:41:18 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2013-05-28 at 12:41 -0500, Scott Wood wrote:
> 
> > I believe Alex is staying far away from e-mail on his vacation.   
> He's
> > asked me to fill in for him while he's gone.
> >
> > The patch itself seems reasonable (though I don't know much about  
> XICS,
> > and do have one question...), but I'll leave it up to  
> Gleb/Marcelo/Ben
> > if it should go in for 3.10 and via which tree.  I understand the
> > desire to not have an incomplete ABI in a released version, but  
> Linus
> > is already grumbling about how much went into rc3, and you say the
> > hcalls aren't currently used...  Are they likely to be used in any
> > timeframe in which we'd reasonably care about 3.10?
> 
> Yes. I'd like to have them in. Their implementation is actually fairly
> trivial and they cannot be emulated by qemu if the rest of the XICS is
> in the kernel, so it's a problem.

OK.  Does it make more sense for you to take it as Paul suggested, or  
for Gleb or Marcelo to pick it up directly?

> > > +	/* These requests don't have real-mode implementations at
> > > present */
> > > +	switch (req) {
> > > +	case H_XIRR_X:
> > > +		res = kvmppc_h_xirr(vcpu);
> > > +		kvmppc_set_gpr(vcpu, 4, res);
> > > +		kvmppc_set_gpr(vcpu, 5, get_tb());
> > > +		return rc;
> > > +	case H_IPOLL:
> > > +		rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
> > > +		return rc;
> > > +	}
> > > +
> > >  	/* Check for real mode returning too hard */
> > >  	if (xics->real_mode)
> > >  		return kvmppc_xics_rm_complete(vcpu, req);
> >
> > Could you explain what's going on here relative to
> > kvmppc_xics_rm_complete()?  What does "returning too hard" mean, and
> > why must rm_action not be checked for these hcalls?
> 
> This is related to how we handle some hcalls in real mode as a fast
> path. The real-mode stuff cannot handle cases that require for  
> example a
> re-emit of the interrupt, a reject, etc... so in some cases, it  
> returns
> H_TOO_HARD which causes KVM to exit and try to handle the hcall again  
> in
> kernel virtual mode.
> 
> When doing so as the result of a XICS hcall, it sets a bit mask of
> "tasks" to handle in virtual mode (because it will have already
> partially done the operation, it cannot just re-play the whole hcall).
> 
> So when real-mode is supported we must not just call the normal  
> virtual
> mode version of the hcalls, we instead go to kvmppc_xics_rm_complete()
> to handle those "tasks".
> 
> However, for those 2 "missing" hcalls, we have no real mode
> implementation at all (we didn't bother, we will do that later if
> needed, it's purely a performance issue). So we need to fully handle
> them in virtual mode, and we know there will be no "tasks" to handle  
> in
> rm_complete.

Then rm_action should always be 0 for these hcalls, right?  So there's  
no correctness reason to keep the hcalls in separate switch  
statements.  You shave off a few cycles checking rm_action, at the cost  
of needing to change kvmppc_xics_hcall() if a real-mode version of  
these hcalls is ever done.

-Scott

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-29 23:38       ` Scott Wood
  0 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2013-05-29 23:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: kvm, Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm-ppc,
	linuxppc-dev, Paul Mackerras

On 05/28/2013 07:41:18 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2013-05-28 at 12:41 -0500, Scott Wood wrote:
>=20
> > I believe Alex is staying far away from e-mail on his vacation.  =20
> He's
> > asked me to fill in for him while he's gone.
> >
> > The patch itself seems reasonable (though I don't know much about =20
> XICS,
> > and do have one question...), but I'll leave it up to =20
> Gleb/Marcelo/Ben
> > if it should go in for 3.10 and via which tree.  I understand the
> > desire to not have an incomplete ABI in a released version, but =20
> Linus
> > is already grumbling about how much went into rc3, and you say the
> > hcalls aren't currently used...  Are they likely to be used in any
> > timeframe in which we'd reasonably care about 3.10?
>=20
> Yes. I'd like to have them in. Their implementation is actually fairly
> trivial and they cannot be emulated by qemu if the rest of the XICS is
> in the kernel, so it's a problem.

OK.  Does it make more sense for you to take it as Paul suggested, or =20
for Gleb or Marcelo to pick it up directly?

> > > +	/* These requests don't have real-mode implementations at
> > > present */
> > > +	switch (req) {
> > > +	case H_XIRR_X:
> > > +		res =3D kvmppc_h_xirr(vcpu);
> > > +		kvmppc_set_gpr(vcpu, 4, res);
> > > +		kvmppc_set_gpr(vcpu, 5, get_tb());
> > > +		return rc;
> > > +	case H_IPOLL:
> > > +		rc =3D kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
> > > +		return rc;
> > > +	}
> > > +
> > >  	/* Check for real mode returning too hard */
> > >  	if (xics->real_mode)
> > >  		return kvmppc_xics_rm_complete(vcpu, req);
> >
> > Could you explain what's going on here relative to
> > kvmppc_xics_rm_complete()?  What does "returning too hard" mean, and
> > why must rm_action not be checked for these hcalls?
>=20
> This is related to how we handle some hcalls in real mode as a fast
> path. The real-mode stuff cannot handle cases that require for =20
> example a
> re-emit of the interrupt, a reject, etc... so in some cases, it =20
> returns
> H_TOO_HARD which causes KVM to exit and try to handle the hcall again =20
> in
> kernel virtual mode.
>=20
> When doing so as the result of a XICS hcall, it sets a bit mask of
> "tasks" to handle in virtual mode (because it will have already
> partially done the operation, it cannot just re-play the whole hcall).
>=20
> So when real-mode is supported we must not just call the normal =20
> virtual
> mode version of the hcalls, we instead go to kvmppc_xics_rm_complete()
> to handle those "tasks".
>=20
> However, for those 2 "missing" hcalls, we have no real mode
> implementation at all (we didn't bother, we will do that later if
> needed, it's purely a performance issue). So we need to fully handle
> them in virtual mode, and we know there will be no "tasks" to handle =20
> in
> rm_complete.

Then rm_action should always be 0 for these hcalls, right?  So there's =20
no correctness reason to keep the hcalls in separate switch =20
statements.  You shave off a few cycles checking rm_action, at the cost =20
of needing to change kvmppc_xics_hcall() if a real-mode version of =20
these hcalls is ever done.

-Scott=

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-29 23:38       ` Scott Wood
  0 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2013-05-29 23:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Paul Mackerras, kvm-ppc, kvm, linuxppc-dev, Alexander Graf,
	Gleb Natapov, Marcelo Tosatti

On 05/28/2013 07:41:18 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2013-05-28 at 12:41 -0500, Scott Wood wrote:
> 
> > I believe Alex is staying far away from e-mail on his vacation.   
> He's
> > asked me to fill in for him while he's gone.
> >
> > The patch itself seems reasonable (though I don't know much about  
> XICS,
> > and do have one question...), but I'll leave it up to  
> Gleb/Marcelo/Ben
> > if it should go in for 3.10 and via which tree.  I understand the
> > desire to not have an incomplete ABI in a released version, but  
> Linus
> > is already grumbling about how much went into rc3, and you say the
> > hcalls aren't currently used...  Are they likely to be used in any
> > timeframe in which we'd reasonably care about 3.10?
> 
> Yes. I'd like to have them in. Their implementation is actually fairly
> trivial and they cannot be emulated by qemu if the rest of the XICS is
> in the kernel, so it's a problem.

OK.  Does it make more sense for you to take it as Paul suggested, or  
for Gleb or Marcelo to pick it up directly?

> > > +	/* These requests don't have real-mode implementations at
> > > present */
> > > +	switch (req) {
> > > +	case H_XIRR_X:
> > > +		res = kvmppc_h_xirr(vcpu);
> > > +		kvmppc_set_gpr(vcpu, 4, res);
> > > +		kvmppc_set_gpr(vcpu, 5, get_tb());
> > > +		return rc;
> > > +	case H_IPOLL:
> > > +		rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
> > > +		return rc;
> > > +	}
> > > +
> > >  	/* Check for real mode returning too hard */
> > >  	if (xics->real_mode)
> > >  		return kvmppc_xics_rm_complete(vcpu, req);
> >
> > Could you explain what's going on here relative to
> > kvmppc_xics_rm_complete()?  What does "returning too hard" mean, and
> > why must rm_action not be checked for these hcalls?
> 
> This is related to how we handle some hcalls in real mode as a fast
> path. The real-mode stuff cannot handle cases that require for  
> example a
> re-emit of the interrupt, a reject, etc... so in some cases, it  
> returns
> H_TOO_HARD which causes KVM to exit and try to handle the hcall again  
> in
> kernel virtual mode.
> 
> When doing so as the result of a XICS hcall, it sets a bit mask of
> "tasks" to handle in virtual mode (because it will have already
> partially done the operation, it cannot just re-play the whole hcall).
> 
> So when real-mode is supported we must not just call the normal  
> virtual
> mode version of the hcalls, we instead go to kvmppc_xics_rm_complete()
> to handle those "tasks".
> 
> However, for those 2 "missing" hcalls, we have no real mode
> implementation at all (we didn't bother, we will do that later if
> needed, it's purely a performance issue). So we need to fully handle
> them in virtual mode, and we know there will be no "tasks" to handle  
> in
> rm_complete.

Then rm_action should always be 0 for these hcalls, right?  So there's  
no correctness reason to keep the hcalls in separate switch  
statements.  You shave off a few cycles checking rm_action, at the cost  
of needing to change kvmppc_xics_hcall() if a real-mode version of  
these hcalls is ever done.

-Scott

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
  2013-05-29 23:38       ` Scott Wood
  (?)
@ 2013-05-29 23:57         ` Benjamin Herrenschmidt
  -1 siblings, 0 replies; 17+ messages in thread
From: Benjamin Herrenschmidt @ 2013-05-29 23:57 UTC (permalink / raw)
  To: Scott Wood
  Cc: Paul Mackerras, kvm-ppc, kvm, linuxppc-dev, Alexander Graf,
	Gleb Natapov, Marcelo Tosatti

On Wed, 2013-05-29 at 18:38 -0500, Scott Wood wrote:

> > Yes. I'd like to have them in. Their implementation is actually fairly
> > trivial and they cannot be emulated by qemu if the rest of the XICS is
> > in the kernel, so it's a problem.
> 
> OK.  Does it make more sense for you to take it as Paul suggested, or  
> for Gleb or Marcelo to pick it up directly?

I'll take it.

> Then rm_action should always be 0 for these hcalls, right?  So there's  
> no correctness reason to keep the hcalls in separate switch  
> statements.  You shave off a few cycles checking rm_action, at the cost  
> of needing to change kvmppc_xics_hcall() if a real-mode version of  
> these hcalls is ever done.

No, because rm_action will also be 0 if the hcall was fully done in real
mode (which can happen, that's our fast path), in which case we do *NOT*
want to to be re-done in virtual mode.

That's why we always return whether rm_action is 0 or not when real-mode
is enabled.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-29 23:57         ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 17+ messages in thread
From: Benjamin Herrenschmidt @ 2013-05-29 23:57 UTC (permalink / raw)
  To: Scott Wood
  Cc: kvm, Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm-ppc,
	linuxppc-dev, Paul Mackerras

On Wed, 2013-05-29 at 18:38 -0500, Scott Wood wrote:

> > Yes. I'd like to have them in. Their implementation is actually fairly
> > trivial and they cannot be emulated by qemu if the rest of the XICS is
> > in the kernel, so it's a problem.
> 
> OK.  Does it make more sense for you to take it as Paul suggested, or  
> for Gleb or Marcelo to pick it up directly?

I'll take it.

> Then rm_action should always be 0 for these hcalls, right?  So there's  
> no correctness reason to keep the hcalls in separate switch  
> statements.  You shave off a few cycles checking rm_action, at the cost  
> of needing to change kvmppc_xics_hcall() if a real-mode version of  
> these hcalls is ever done.

No, because rm_action will also be 0 if the hcall was fully done in real
mode (which can happen, that's our fast path), in which case we do *NOT*
want to to be re-done in virtual mode.

That's why we always return whether rm_action is 0 or not when real-mode
is enabled.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-29 23:57         ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 17+ messages in thread
From: Benjamin Herrenschmidt @ 2013-05-29 23:57 UTC (permalink / raw)
  To: Scott Wood
  Cc: Paul Mackerras, kvm-ppc, kvm, linuxppc-dev, Alexander Graf,
	Gleb Natapov, Marcelo Tosatti

On Wed, 2013-05-29 at 18:38 -0500, Scott Wood wrote:

> > Yes. I'd like to have them in. Their implementation is actually fairly
> > trivial and they cannot be emulated by qemu if the rest of the XICS is
> > in the kernel, so it's a problem.
> 
> OK.  Does it make more sense for you to take it as Paul suggested, or  
> for Gleb or Marcelo to pick it up directly?

I'll take it.

> Then rm_action should always be 0 for these hcalls, right?  So there's  
> no correctness reason to keep the hcalls in separate switch  
> statements.  You shave off a few cycles checking rm_action, at the cost  
> of needing to change kvmppc_xics_hcall() if a real-mode version of  
> these hcalls is ever done.

No, because rm_action will also be 0 if the hcall was fully done in real
mode (which can happen, that's our fast path), in which case we do *NOT*
want to to be re-done in virtual mode.

That's why we always return whether rm_action is 0 or not when real-mode
is enabled.

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
  2013-05-29 23:57         ` Benjamin Herrenschmidt
  (?)
@ 2013-05-30  0:07           ` Scott Wood
  -1 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2013-05-30  0:07 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Paul Mackerras, kvm-ppc, kvm, linuxppc-dev, Alexander Graf,
	Gleb Natapov, Marcelo Tosatti

On 05/29/2013 06:57:32 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2013-05-29 at 18:38 -0500, Scott Wood wrote:
> 
> > > Yes. I'd like to have them in. Their implementation is actually  
> fairly
> > > trivial and they cannot be emulated by qemu if the rest of the  
> XICS is
> > > in the kernel, so it's a problem.
> >
> > OK.  Does it make more sense for you to take it as Paul suggested,  
> or
> > for Gleb or Marcelo to pick it up directly?
> 
> I'll take it.

Acked-by: Scott Wood <scottwood@freescale.com>

> > Then rm_action should always be 0 for these hcalls, right?  So  
> there's
> > no correctness reason to keep the hcalls in separate switch
> > statements.  You shave off a few cycles checking rm_action, at the  
> cost
> > of needing to change kvmppc_xics_hcall() if a real-mode version of
> > these hcalls is ever done.
> 
> No, because rm_action will also be 0 if the hcall was fully done in  
> real
> mode (which can happen, that's our fast path), in which case we do  
> *NOT*
> want to to be re-done in virtual mode.
> 
> That's why we always return whether rm_action is 0 or not when  
> real-mode
> is enabled.

Oh, I misread the code and thought the decision to return was based on  
the return value of kvmppc_xics_rm_complete.  Sorry about that. :-(

-Scott

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-30  0:07           ` Scott Wood
  0 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2013-05-30  0:07 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: kvm, Gleb Natapov, Marcelo Tosatti, Alexander Graf, kvm-ppc,
	linuxppc-dev, Paul Mackerras

On 05/29/2013 06:57:32 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2013-05-29 at 18:38 -0500, Scott Wood wrote:
>=20
> > > Yes. I'd like to have them in. Their implementation is actually =20
> fairly
> > > trivial and they cannot be emulated by qemu if the rest of the =20
> XICS is
> > > in the kernel, so it's a problem.
> >
> > OK.  Does it make more sense for you to take it as Paul suggested, =20
> or
> > for Gleb or Marcelo to pick it up directly?
>=20
> I'll take it.

Acked-by: Scott Wood <scottwood@freescale.com>

> > Then rm_action should always be 0 for these hcalls, right?  So =20
> there's
> > no correctness reason to keep the hcalls in separate switch
> > statements.  You shave off a few cycles checking rm_action, at the =20
> cost
> > of needing to change kvmppc_xics_hcall() if a real-mode version of
> > these hcalls is ever done.
>=20
> No, because rm_action will also be 0 if the hcall was fully done in =20
> real
> mode (which can happen, that's our fast path), in which case we do =20
> *NOT*
> want to to be re-done in virtual mode.
>=20
> That's why we always return whether rm_action is 0 or not when =20
> real-mode
> is enabled.

Oh, I misread the code and thought the decision to return was based on =20
the return value of kvmppc_xics_rm_complete.  Sorry about that. :-(

-Scott=

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation
@ 2013-05-30  0:07           ` Scott Wood
  0 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2013-05-30  0:07 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Paul Mackerras, kvm-ppc, kvm, linuxppc-dev, Alexander Graf,
	Gleb Natapov, Marcelo Tosatti

On 05/29/2013 06:57:32 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2013-05-29 at 18:38 -0500, Scott Wood wrote:
> 
> > > Yes. I'd like to have them in. Their implementation is actually  
> fairly
> > > trivial and they cannot be emulated by qemu if the rest of the  
> XICS is
> > > in the kernel, so it's a problem.
> >
> > OK.  Does it make more sense for you to take it as Paul suggested,  
> or
> > for Gleb or Marcelo to pick it up directly?
> 
> I'll take it.

Acked-by: Scott Wood <scottwood@freescale.com>

> > Then rm_action should always be 0 for these hcalls, right?  So  
> there's
> > no correctness reason to keep the hcalls in separate switch
> > statements.  You shave off a few cycles checking rm_action, at the  
> cost
> > of needing to change kvmppc_xics_hcall() if a real-mode version of
> > these hcalls is ever done.
> 
> No, because rm_action will also be 0 if the hcall was fully done in  
> real
> mode (which can happen, that's our fast path), in which case we do  
> *NOT*
> want to to be re-done in virtual mode.
> 
> That's why we always return whether rm_action is 0 or not when  
> real-mode
> is enabled.

Oh, I misread the code and thought the decision to return was based on  
the return value of kvmppc_xics_rm_complete.  Sorry about that. :-(

-Scott

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-05-30  0:07 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-24  1:42 [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation Paul Mackerras
2013-05-24  1:42 ` Paul Mackerras
2013-05-24  1:42 ` Paul Mackerras
2013-05-28 17:41 ` Scott Wood
2013-05-28 17:41   ` Scott Wood
2013-05-28 17:41   ` Scott Wood
2013-05-29  0:41   ` Benjamin Herrenschmidt
2013-05-29  0:41     ` Benjamin Herrenschmidt
2013-05-29 23:38     ` Scott Wood
2013-05-29 23:38       ` Scott Wood
2013-05-29 23:38       ` Scott Wood
2013-05-29 23:57       ` Benjamin Herrenschmidt
2013-05-29 23:57         ` Benjamin Herrenschmidt
2013-05-29 23:57         ` Benjamin Herrenschmidt
2013-05-30  0:07         ` Scott Wood
2013-05-30  0:07           ` Scott Wood
2013-05-30  0:07           ` Scott Wood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.