Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-10-13 Thread Paul Mackerras 
On Thu, Sep 14, 2017 at 11:56:25PM +0200, Greg Kurz wrote:
> The following program causes a kernel oops:
> 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> main()
> {
> int fd = open("/dev/kvm", O_RDWR);
> ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
> }
> 
> This happens because when using the global KVM fd with
> KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
> called with a NULL kvm argument, which gets dereferenced
> in is_kvmppc_hv_enabled(). Spotted while reading the code.
> 
> Let's use the hv_enabled fallback variable, like everywhere
> else in this function.
> 
> Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
> Cc: sta...@vger.kernel.org # v4.7+
> Signed-off-by: Greg Kurz 

Thanks, applied to my kvm-ppc-fixes branch.

Paul.

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-10-13 Thread Paul Mackerras 
On Fri, Oct 13, 2017 at 06:14:00PM +0200, Paolo Bonzini wrote:
> On 13/10/2017 01:16, Greg Kurz wrote:
> > Ping ?
> 
> When is Paul back from vacation? :)

Now. :)

Paul.

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-10-13 Thread Paolo Bonzini 
On 13/10/2017 01:16, Greg Kurz wrote:
> Ping ?

When is Paul back from vacation? :)

Paolo

> On Thu, 14 Sep 2017 23:56:25 +0200
> Greg Kurz  wrote:
> 
>> The following program causes a kernel oops:
>>
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>>
>> main()
>> {
>> int fd = open("/dev/kvm", O_RDWR);
>> ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
>> }
>>
>> This happens because when using the global KVM fd with
>> KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
>> called with a NULL kvm argument, which gets dereferenced
>> in is_kvmppc_hv_enabled(). Spotted while reading the code.
>>
>> Let's use the hv_enabled fallback variable, like everywhere
>> else in this function.
>>
>> Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
>> Cc: sta...@vger.kernel.org # v4.7+
>> Signed-off-by: Greg Kurz 
>> ---
>>  arch/powerpc/kvm/powerpc.c |3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 3480faaf1ef8..ee279c7f4802 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -644,8 +644,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
>> ext)
>>  break;
>>  #endif
>>  case KVM_CAP_PPC_HTM:
>> -r = cpu_has_feature(CPU_FTR_TM_COMP) &&
>> -is_kvmppc_hv_enabled(kvm);
>> +r = cpu_has_feature(CPU_FTR_TM_COMP) && hv_enabled;
>>  break;
>>  default:
>>  r = 0;
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-10-12 Thread Greg Kurz 
Ping ?

On Thu, 14 Sep 2017 23:56:25 +0200
Greg Kurz  wrote:

> The following program causes a kernel oops:
> 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> main()
> {
> int fd = open("/dev/kvm", O_RDWR);
> ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
> }
> 
> This happens because when using the global KVM fd with
> KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
> called with a NULL kvm argument, which gets dereferenced
> in is_kvmppc_hv_enabled(). Spotted while reading the code.
> 
> Let's use the hv_enabled fallback variable, like everywhere
> else in this function.
> 
> Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
> Cc: sta...@vger.kernel.org # v4.7+
> Signed-off-by: Greg Kurz 
> ---
>  arch/powerpc/kvm/powerpc.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 3480faaf1ef8..ee279c7f4802 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -644,8 +644,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
> ext)
>   break;
>  #endif
>   case KVM_CAP_PPC_HTM:
> - r = cpu_has_feature(CPU_FTR_TM_COMP) &&
> - is_kvmppc_hv_enabled(kvm);
> + r = cpu_has_feature(CPU_FTR_TM_COMP) && hv_enabled;
>   break;
>   default:
>   r = 0;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-10-12 Thread David Gibson 
On Thu, Oct 12, 2017 at 02:51:57PM +0200, Greg Kurz wrote:
> On Thu, 12 Oct 2017 22:27:54 +1100
> Michael Ellerman  wrote:
> 
> > Greg Kurz  writes:
> > > The following program causes a kernel oops:
> > >
> > > #include 
> > > #include 
> > > #include 
> > > #include 
> > > #include 
> > >
> > > main()
> > > {
> > > int fd = open("/dev/kvm", O_RDWR);
> > > ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
> > > }
> > >
> > > This happens because when using the global KVM fd with
> > > KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
> > > called with a NULL kvm argument, which gets dereferenced
> > > in is_kvmppc_hv_enabled(). Spotted while reading the code.
> > >
> > > Let's use the hv_enabled fallback variable, like everywhere
> > > else in this function.
> > >
> > > Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
> > > Cc: sta...@vger.kernel.org # v4.7+
> > > Signed-off-by: Greg Kurz 
> > > ---
> > >  arch/powerpc/kvm/powerpc.c |3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > > index 3480faaf1ef8..ee279c7f4802 100644
> > > --- a/arch/powerpc/kvm/powerpc.c
> > > +++ b/arch/powerpc/kvm/powerpc.c
> > > @@ -644,8 +644,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, 
> > > long ext)
> > >   break;
> > >  #endif
> > >   case KVM_CAP_PPC_HTM:
> > > - r = cpu_has_feature(CPU_FTR_TM_COMP) &&
> > > - is_kvmppc_hv_enabled(kvm);
> > > + r = cpu_has_feature(CPU_FTR_TM_COMP) && hv_enabled;
> > >   break;
> > >   default:
> > >   r = 0;  
> > 
> > Did this go anywhere?
> > 
> > cheers
> 
> I'm afraid not... and I haven't tried to ping Paul yet, since he's
> supposed to be on vacation from what I've been told.

He's back now.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-10-12 Thread Greg Kurz 
On Thu, 12 Oct 2017 22:27:54 +1100
Michael Ellerman  wrote:

> Greg Kurz  writes:
> > The following program causes a kernel oops:
> >
> > #include 
> > #include 
> > #include 
> > #include 
> > #include 
> >
> > main()
> > {
> > int fd = open("/dev/kvm", O_RDWR);
> > ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
> > }
> >
> > This happens because when using the global KVM fd with
> > KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
> > called with a NULL kvm argument, which gets dereferenced
> > in is_kvmppc_hv_enabled(). Spotted while reading the code.
> >
> > Let's use the hv_enabled fallback variable, like everywhere
> > else in this function.
> >
> > Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
> > Cc: sta...@vger.kernel.org # v4.7+
> > Signed-off-by: Greg Kurz 
> > ---
> >  arch/powerpc/kvm/powerpc.c |3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 3480faaf1ef8..ee279c7f4802 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -644,8 +644,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
> > ext)
> > break;
> >  #endif
> > case KVM_CAP_PPC_HTM:
> > -   r = cpu_has_feature(CPU_FTR_TM_COMP) &&
> > -   is_kvmppc_hv_enabled(kvm);
> > +   r = cpu_has_feature(CPU_FTR_TM_COMP) && hv_enabled;
> > break;
> > default:
> > r = 0;  
> 
> Did this go anywhere?
> 
> cheers

I'm afraid not... and I haven't tried to ping Paul yet, since he's
supposed to be on vacation from what I've been told.

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-10-12 Thread Michael Ellerman 
Greg Kurz  writes:
> The following program causes a kernel oops:
>
> #include 
> #include 
> #include 
> #include 
> #include 
>
> main()
> {
> int fd = open("/dev/kvm", O_RDWR);
> ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
> }
>
> This happens because when using the global KVM fd with
> KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
> called with a NULL kvm argument, which gets dereferenced
> in is_kvmppc_hv_enabled(). Spotted while reading the code.
>
> Let's use the hv_enabled fallback variable, like everywhere
> else in this function.
>
> Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
> Cc: sta...@vger.kernel.org # v4.7+
> Signed-off-by: Greg Kurz 
> ---
>  arch/powerpc/kvm/powerpc.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 3480faaf1ef8..ee279c7f4802 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -644,8 +644,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
> ext)
>   break;
>  #endif
>   case KVM_CAP_PPC_HTM:
> - r = cpu_has_feature(CPU_FTR_TM_COMP) &&
> - is_kvmppc_hv_enabled(kvm);
> + r = cpu_has_feature(CPU_FTR_TM_COMP) && hv_enabled;
>   break;
>   default:
>   r = 0;

Did this go anywhere?

cheers

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-09-18 Thread Thomas Huth 
On 15.09.2017 10:59, David Gibson wrote:
> On Fri, Sep 15, 2017 at 07:52:49AM +0200, Greg Kurz wrote:
>> Dang! The mail relay at OVH has blacklisted Paul's address :-\
>>
>> : host smtp.samba.org[144.76.82.148] said: 550-blacklisted 
>> at
>> zen.spamhaus.org 550 https://www.spamhaus.org/sbl/query/SBL370982 (in 
>> reply
>> to RCPT TO command)
>>
>> Cc'ing Paul at ozlabs.org
>>
>> On Fri, 15 Sep 2017 10:48:39 +1000
>> David Gibson  wrote:
>>
>>> On Thu, Sep 14, 2017 at 11:56:25PM +0200, Greg Kurz wrote:
 The following program causes a kernel oops:

 #include 
 #include 
 #include 
 #include 
 #include 

 main()
 {
 int fd = open("/dev/kvm", O_RDWR);
 ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
 }

 This happens because when using the global KVM fd with
 KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
 called with a NULL kvm argument, which gets dereferenced
 in is_kvmppc_hv_enabled(). Spotted while reading the code.

 Let's use the hv_enabled fallback variable, like everywhere
 else in this function.

 Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
 Cc: sta...@vger.kernel.org # v4.7+
 Signed-off-by: Greg Kurz   
>>>
>>> I don't think this is right.  I'm pretty sure you want to fall back to
>>> hv_enabled *only when* kvm is NULL.  Otherwise if you have a PR guest
>>> on an HV capable machine, this will give the wrong answer, when called
>>> for that specific VM.
>>>
>>
>> Hmmm... this is what we get with this patch applied:
>>
>> open("/dev/kvm", O_RDWR)= 3
>> ioctl(3, KVM_CHECK_EXTENSION, 0x84) = 1 <== if HV is present
>> ioctl(3, KVM_CREATE_VM, 0x1)= 4 <== HV
>> ioctl(4, KVM_CHECK_EXTENSION, 0x84) = 1
>> ioctl(3, KVM_CREATE_VM, 0x2)= 5 <== PR
>> ioctl(5, KVM_CHECK_EXTENSION, 0x84) = 0
>>
>> The hv_enabled variable is set as follows:
>>
>>  /* Assume we're using HV mode when the HV module is loaded */
>>  int hv_enabled = kvmppc_hv_ops ? 1 : 0;
>>
>>  if (kvm) {
>>  /*
>>   * Hooray - we know which VM type we're running on. Depend on
>>   * that rather than the guess above.
>>   */
>>  hv_enabled = is_kvmppc_hv_enabled(kvm);
>>  }
>>
>> so we're good. :)
> 
> Oh, sorry, missed that bit.  In that case.
> 
> Reviewed-by: David Gibson 

LGTM, too:

Reviewed-by: Thomas Huth 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-09-17 Thread David Gibson 
On Fri, Sep 15, 2017 at 07:52:49AM +0200, Greg Kurz wrote:
> Dang! The mail relay at OVH has blacklisted Paul's address :-\
> 
> : host smtp.samba.org[144.76.82.148] said: 550-blacklisted 
> at
> zen.spamhaus.org 550 https://www.spamhaus.org/sbl/query/SBL370982 (in 
> reply
> to RCPT TO command)
> 
> Cc'ing Paul at ozlabs.org
> 
> On Fri, 15 Sep 2017 10:48:39 +1000
> David Gibson  wrote:
> 
> > On Thu, Sep 14, 2017 at 11:56:25PM +0200, Greg Kurz wrote:
> > > The following program causes a kernel oops:
> > > 
> > > #include 
> > > #include 
> > > #include 
> > > #include 
> > > #include 
> > > 
> > > main()
> > > {
> > > int fd = open("/dev/kvm", O_RDWR);
> > > ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
> > > }
> > > 
> > > This happens because when using the global KVM fd with
> > > KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
> > > called with a NULL kvm argument, which gets dereferenced
> > > in is_kvmppc_hv_enabled(). Spotted while reading the code.
> > > 
> > > Let's use the hv_enabled fallback variable, like everywhere
> > > else in this function.
> > > 
> > > Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
> > > Cc: sta...@vger.kernel.org # v4.7+
> > > Signed-off-by: Greg Kurz   
> > 
> > I don't think this is right.  I'm pretty sure you want to fall back to
> > hv_enabled *only when* kvm is NULL.  Otherwise if you have a PR guest
> > on an HV capable machine, this will give the wrong answer, when called
> > for that specific VM.
> > 
> 
> Hmmm... this is what we get with this patch applied:
> 
> open("/dev/kvm", O_RDWR)= 3
> ioctl(3, KVM_CHECK_EXTENSION, 0x84) = 1 <== if HV is present
> ioctl(3, KVM_CREATE_VM, 0x1)= 4 <== HV
> ioctl(4, KVM_CHECK_EXTENSION, 0x84) = 1
> ioctl(3, KVM_CREATE_VM, 0x2)= 5 <== PR
> ioctl(5, KVM_CHECK_EXTENSION, 0x84) = 0
> 
> The hv_enabled variable is set as follows:
> 
>   /* Assume we're using HV mode when the HV module is loaded */
>   int hv_enabled = kvmppc_hv_ops ? 1 : 0;
> 
>   if (kvm) {
>   /*
>* Hooray - we know which VM type we're running on. Depend on
>* that rather than the guess above.
>*/
>   hv_enabled = is_kvmppc_hv_enabled(kvm);
>   }
> 
> so we're good. :)

Oh, sorry, missed that bit.  In that case.

Reviewed-by: David Gibson 



> The last sentence in the commit message is maybe^wprobably not comprehensive
> enough...
> 
> What about the following ?
> 
> The hv_enabled variable is initialized to 1 if HV is loaded or 0 otherwise.
> In the case KVM_CHECK_EXTENSION is used with a VM fd, hv_enabled is updated
> to is_kvmppc_hv_enabled(kvm). Let's use it here, like everywhere else in this
> function.
> 
> > > ---
> > >  arch/powerpc/kvm/powerpc.c |3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > > index 3480faaf1ef8..ee279c7f4802 100644
> > > --- a/arch/powerpc/kvm/powerpc.c
> > > +++ b/arch/powerpc/kvm/powerpc.c
> > > @@ -644,8 +644,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, 
> > > long ext)
> > >   break;
> > >  #endif
> > >   case KVM_CAP_PPC_HTM:
> > > - r = cpu_has_feature(CPU_FTR_TM_COMP) &&
> > > - is_kvmppc_hv_enabled(kvm);
> > > + r = cpu_has_feature(CPU_FTR_TM_COMP) && hv_enabled;
> > >   break;
> > >   default:
> > >   r = 0;
> > >   
> > 
> 



-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-09-15 Thread Greg Kurz 
On Fri, 15 Sep 2017 07:52:49 +0200
Greg Kurz  wrote:

> Dang! The mail relay at OVH has blacklisted Paul's address :-\
> 
> : host smtp.samba.org[144.76.82.148] said: 550-blacklisted 
> at
> zen.spamhaus.org 550 https://www.spamhaus.org/sbl/query/SBL370982 (in 
> reply
> to RCPT TO command)
> 

Dumb me! It's the opposite... OVH is blacklisted by smtp.samba.org :-\

Sigh.


pgpbGVYSAUr7G.pgp
Description: OpenPGP digital signature

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-09-14 Thread Greg Kurz 
Dang! The mail relay at OVH has blacklisted Paul's address :-\

: host smtp.samba.org[144.76.82.148] said: 550-blacklisted at
zen.spamhaus.org 550 https://www.spamhaus.org/sbl/query/SBL370982 (in reply
to RCPT TO command)

Cc'ing Paul at ozlabs.org

On Fri, 15 Sep 2017 10:48:39 +1000
David Gibson  wrote:

> On Thu, Sep 14, 2017 at 11:56:25PM +0200, Greg Kurz wrote:
> > The following program causes a kernel oops:
> > 
> > #include 
> > #include 
> > #include 
> > #include 
> > #include 
> > 
> > main()
> > {
> > int fd = open("/dev/kvm", O_RDWR);
> > ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
> > }
> > 
> > This happens because when using the global KVM fd with
> > KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
> > called with a NULL kvm argument, which gets dereferenced
> > in is_kvmppc_hv_enabled(). Spotted while reading the code.
> > 
> > Let's use the hv_enabled fallback variable, like everywhere
> > else in this function.
> > 
> > Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
> > Cc: sta...@vger.kernel.org # v4.7+
> > Signed-off-by: Greg Kurz   
> 
> I don't think this is right.  I'm pretty sure you want to fall back to
> hv_enabled *only when* kvm is NULL.  Otherwise if you have a PR guest
> on an HV capable machine, this will give the wrong answer, when called
> for that specific VM.
> 

Hmmm... this is what we get with this patch applied:

open("/dev/kvm", O_RDWR)= 3
ioctl(3, KVM_CHECK_EXTENSION, 0x84) = 1 <== if HV is present
ioctl(3, KVM_CREATE_VM, 0x1)= 4 <== HV
ioctl(4, KVM_CHECK_EXTENSION, 0x84) = 1
ioctl(3, KVM_CREATE_VM, 0x2)= 5 <== PR
ioctl(5, KVM_CHECK_EXTENSION, 0x84) = 0

The hv_enabled variable is set as follows:

/* Assume we're using HV mode when the HV module is loaded */
int hv_enabled = kvmppc_hv_ops ? 1 : 0;

if (kvm) {
/*
 * Hooray - we know which VM type we're running on. Depend on
 * that rather than the guess above.
 */
hv_enabled = is_kvmppc_hv_enabled(kvm);
}

so we're good. :)

The last sentence in the commit message is maybe^wprobably not comprehensive
enough...

What about the following ?

The hv_enabled variable is initialized to 1 if HV is loaded or 0 otherwise.
In the case KVM_CHECK_EXTENSION is used with a VM fd, hv_enabled is updated
to is_kvmppc_hv_enabled(kvm). Let's use it here, like everywhere else in this
function.

> > ---
> >  arch/powerpc/kvm/powerpc.c |3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 3480faaf1ef8..ee279c7f4802 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -644,8 +644,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
> > ext)
> > break;
> >  #endif
> > case KVM_CAP_PPC_HTM:
> > -   r = cpu_has_feature(CPU_FTR_TM_COMP) &&
> > -   is_kvmppc_hv_enabled(kvm);
> > +   r = cpu_has_feature(CPU_FTR_TM_COMP) && hv_enabled;
> > break;
> > default:
> > r = 0;
> >   
> 



pgpCqjd5b_a7V.pgp
Description: OpenPGP digital signature

Re: [PATCH] KVM: PPC: fix oops when checking KVM_CAP_PPC_HTM

2017-09-14 Thread David Gibson 
On Thu, Sep 14, 2017 at 11:56:25PM +0200, Greg Kurz wrote:
> The following program causes a kernel oops:
> 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> main()
> {
> int fd = open("/dev/kvm", O_RDWR);
> ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_HTM);
> }
> 
> This happens because when using the global KVM fd with
> KVM_CHECK_EXTENSION, kvm_vm_ioctl_check_extension() gets
> called with a NULL kvm argument, which gets dereferenced
> in is_kvmppc_hv_enabled(). Spotted while reading the code.
> 
> Let's use the hv_enabled fallback variable, like everywhere
> else in this function.
> 
> Fixes: 23528bb21ee2 ("KVM: PPC: Introduce KVM_CAP_PPC_HTM")
> Cc: sta...@vger.kernel.org # v4.7+
> Signed-off-by: Greg Kurz 

I don't think this is right.  I'm pretty sure you want to fall back to
hv_enabled *only when* kvm is NULL.  Otherwise if you have a PR guest
on an HV capable machine, this will give the wrong answer, when called
for that specific VM.

> ---
>  arch/powerpc/kvm/powerpc.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 3480faaf1ef8..ee279c7f4802 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -644,8 +644,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
> ext)
>   break;
>  #endif
>   case KVM_CAP_PPC_HTM:
> - r = cpu_has_feature(CPU_FTR_TM_COMP) &&
> - is_kvmppc_hv_enabled(kvm);
> + r = cpu_has_feature(CPU_FTR_TM_COMP) && hv_enabled;
>   break;
>   default:
>   r = 0;
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature