diff mbox

[1/1] KVM: PPC: Introduce KVM_CAP_PPC_HTM

Message ID 741e7a7860e055603a311bcc6051bd88fdad1215.1467785081.git.sam.bobroff@au1.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sam Bobroff July 6, 2016, 6:05 a.m. UTC
Introduce a new KVM capability, KVM_CAP_PPC_HTM, that can be queried to
determine if a PowerPC KVM guest should use HTM (Hardware Transactional
Memory).

This will be used by QEMU to populate the pa-features bits in the
guest's device tree.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---

 arch/powerpc/kvm/powerpc.c | 4 ++++
 include/uapi/linux/kvm.h   | 1 +
 2 files changed, 5 insertions(+)

Comments

Education Directorate July 6, 2016, 11:05 a.m. UTC | #1
On Wed, 2016-07-06 at 16:05 +1000, Sam Bobroff wrote:
> Introduce a new KVM capability, KVM_CAP_PPC_HTM, that can be queried to
> determine if a PowerPC KVM guest should use HTM (Hardware Transactional
> Memory).

> This will be used by QEMU to populate the pa-features bits in the
> guest's device tree.

> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---

Makes sense

Acked-by: Balbir Singh <bsingharora@gmail.com>

>
David Gibson July 7, 2016, 12:07 a.m. UTC | #2
On Wed, Jul 06, 2016 at 04:05:54PM +1000, Sam Bobroff wrote:
> Introduce a new KVM capability, KVM_CAP_PPC_HTM, that can be queried to
> determine if a PowerPC KVM guest should use HTM (Hardware Transactional
> Memory).
> 
> This will be used by QEMU to populate the pa-features bits in the
> guest's device tree.
> 
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
>  arch/powerpc/kvm/powerpc.c | 4 ++++
>  include/uapi/linux/kvm.h   | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 02416fe..06d79bc 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -588,6 +588,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  		r = 1;
>  		break;
>  #endif
> +	case KVM_CAP_PPC_HTM:
> +		r = cpu_has_feature(CPU_FTR_TM)
> +		    && is_kvmppc_hv_enabled(kvm);
> +		break;
>  	default:
>  		r = 0;
>  		break;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 05ebf47..f421d0e 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -866,6 +866,7 @@ struct kvm_ppc_smmu_info {
>  #define KVM_CAP_ARM_PMU_V3 126
>  #define KVM_CAP_VCPU_ATTRIBUTES 127
>  #define KVM_CAP_MAX_VCPU_ID 128
> +#define KVM_CAP_PPC_HTM 129
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>
Michael Ellerman July 8, 2016, 10:49 a.m. UTC | #3
On Wed, 2016-06-07 at 06:05:54 UTC, Sam bobroff wrote:
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 02416fe..06d79bc 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -588,6 +588,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  		r = 1;
>  		break;
>  #endif
> +	case KVM_CAP_PPC_HTM:
> +		r = cpu_has_feature(CPU_FTR_TM)
> +		    && is_kvmppc_hv_enabled(kvm);

I think it should be using CPU_FTR_TM_COMP.

And AFAICS you don't need to break that line.

cheers
Sam Bobroff July 19, 2016, 3:23 a.m. UTC | #4
On Fri, Jul 08, 2016 at 08:49:49PM +1000, Michael Ellerman wrote:
> On Wed, 2016-06-07 at 06:05:54 UTC, Sam bobroff wrote:
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 02416fe..06d79bc 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -588,6 +588,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >  		r = 1;
> >  		break;
> >  #endif
> > +	case KVM_CAP_PPC_HTM:
> > +		r = cpu_has_feature(CPU_FTR_TM)
> > +		    && is_kvmppc_hv_enabled(kvm);
> 
> I think it should be using CPU_FTR_TM_COMP.

Oh, why is that? I'm happy to respin the patch I'm just curious.

(I did it that way becuase that seems to be the way the other flags are used,
e.g. CPU_FTR_ALTIVEC).

If I read the code correctly, using CPU_FTR_TM_COMP will work fine: it should
cause the cpu_has_feature() test to always return false if CPU_FTR_TM_COMP is
0.

> And AFAICS you don't need to break that line.

Sure, I'll un-split it when I respin.

> cheers

Cheers,
Sam.
Michael Ellerman July 19, 2016, 9:24 a.m. UTC | #5
Sam Bobroff <sam.bobroff@au1.ibm.com> writes:

> On Fri, Jul 08, 2016 at 08:49:49PM +1000, Michael Ellerman wrote:
>> On Wed, 2016-06-07 at 06:05:54 UTC, Sam bobroff wrote:
>> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> > index 02416fe..06d79bc 100644
>> > --- a/arch/powerpc/kvm/powerpc.c
>> > +++ b/arch/powerpc/kvm/powerpc.c
>> > @@ -588,6 +588,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>> >  		r = 1;
>> >  		break;
>> >  #endif
>> > +	case KVM_CAP_PPC_HTM:
>> > +		r = cpu_has_feature(CPU_FTR_TM)
>> > +		    && is_kvmppc_hv_enabled(kvm);
>> 
>> I think it should be using CPU_FTR_TM_COMP.
>
> Oh, why is that? I'm happy to respin the patch I'm just curious.
>
> (I did it that way becuase that seems to be the way the other flags are used,
> e.g. CPU_FTR_ALTIVEC).
>
> If I read the code correctly, using CPU_FTR_TM_COMP will work fine: it should
> cause the cpu_has_feature() test to always return false if CPU_FTR_TM_COMP is
> 0.

CPU_FTR_TM says the CPU supports TM.

CPU_FTR_TM_COMP says the CPU supports TM *and* the kernel is built with
TM support.

The distinction exists because currently the assembly patching macros
don't deal correctly with a feature bit that is defined to 0. (And
possibly other reasons I don't remember)

We should fix that, but until we have, anything that is advertising
support to userspace should be using the COMP bits, when they exist.

cheers
diff mbox

Patch

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 02416fe..06d79bc 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -588,6 +588,10 @@  int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		r = 1;
 		break;
 #endif
+	case KVM_CAP_PPC_HTM:
+		r = cpu_has_feature(CPU_FTR_TM)
+		    && is_kvmppc_hv_enabled(kvm);
+		break;
 	default:
 		r = 0;
 		break;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 05ebf47..f421d0e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -866,6 +866,7 @@  struct kvm_ppc_smmu_info {
 #define KVM_CAP_ARM_PMU_V3 126
 #define KVM_CAP_VCPU_ATTRIBUTES 127
 #define KVM_CAP_MAX_VCPU_ID 128
+#define KVM_CAP_PPC_HTM 129
 
 #ifdef KVM_CAP_IRQ_ROUTING