[Qemu-devel] KVM-PR is broken with current QEMU

[Qemu-devel] KVM-PR is broken with current QEMU

[Thomas Huth]

 Hi,

when I try to run my guest in KVM-PR mode, current QEMU refuses to start:

  $ sudo qemu-system-ppc64 -M pseries,accel=kvm,kvm-type=PR \
                           -nographic -vga none -cpu POWER8
  qemu: fatal: Unknown MMU model 851972

... followed by a useless register dump. I've bisected the issue, and it
seems like the problem has been introduced by this commit here:

  commit 4322e8ced5aaac7191958f09622d199fe61e2d87
  ppc: Fix 64K pages support in full emulation

Seems like KVM PR is using the "degraded" ISA variants (without the 1TB
segments), but the new POWERPC_MMU_64K flag has not been added to those.
Has this been done on purpose, or was this just by accident?
I can make KVM PR working again with the following patch:

diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 2864105..36694cb 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -81,6 +81,7 @@ enum powerpc_mmu_t {
                              | POWERPC_MMU_AMR | 0x00000003,
     /* Architecture 2.06 "degraded" (no 1T segments)           */
     POWERPC_MMU_2_06a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
+                             | POWERPC_MMU_64K
                              | 0x00000003,
     /* Architecture 2.07 variant                               */
     POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
@@ -88,6 +89,7 @@ enum powerpc_mmu_t {
                              | POWERPC_MMU_AMR | 0x00000004,
     /* Architecture 2.07 "degraded" (no 1T segments)           */
     POWERPC_MMU_2_07a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
+                             | POWERPC_MMU_64K
                              | 0x00000004,
 };

However, not sure whether this is the right fix ... Cédric, Ben, any ideas?

 Thomas

Re: [Qemu-devel] KVM-PR is broken with current QEMU

[Cédric Le Goater]

On 09/20/2016 01:44 PM, Thomas Huth wrote:
>  Hi,
> 
> when I try to run my guest in KVM-PR mode, current QEMU refuses to start:
> 
>   $ sudo qemu-system-ppc64 -M pseries,accel=kvm,kvm-type=PR \
>                            -nographic -vga none -cpu POWER8
>   qemu: fatal: Unknown MMU model 851972
> 
> ... followed by a useless register dump. I've bisected the issue, and it
> seems like the problem has been introduced by this commit here:
> 
>   commit 4322e8ced5aaac7191958f09622d199fe61e2d87
>   ppc: Fix 64K pages support in full emulation
> 
> Seems like KVM PR is using the "degraded" ISA variants (without the 1TB
> segments), but the new POWERPC_MMU_64K flag has not been added to those.
> Has this been done on purpose, or was this just by accident?

The "degraded" architecture support has some history behind it :

 commit 126a79300971 added it
 commit aa4bb5875231 removed it.
 commit ba3ecda05e93 readded it.
 commit 4322e8ced5aa forgot about it again

> I can make KVM PR working again with the following patch:

I think this is correct. Let's wait for Ben to chime in :)

Thanks,

C.
 
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index 2864105..36694cb 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -81,6 +81,7 @@ enum powerpc_mmu_t {
>                               | POWERPC_MMU_AMR | 0x00000003,
>      /* Architecture 2.06 "degraded" (no 1T segments)           */
>      POWERPC_MMU_2_06a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
> +                             | POWERPC_MMU_64K
>                               | 0x00000003,
>      /* Architecture 2.07 variant                               */
>      POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> @@ -88,6 +89,7 @@ enum powerpc_mmu_t {
>                               | POWERPC_MMU_AMR | 0x00000004,
>      /* Architecture 2.07 "degraded" (no 1T segments)           */
>      POWERPC_MMU_2_07a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
> +                             | POWERPC_MMU_64K
>                               | 0x00000004,
>  };
> 
> However, not sure whether this is the right fix ... Cédric, Ben, any ideas?

Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU

[Cédric Le Goater]

On 09/20/2016 02:24 PM, Cédric Le Goater wrote:
> On 09/20/2016 01:44 PM, Thomas Huth wrote:
>>  Hi,
>>
>> when I try to run my guest in KVM-PR mode, current QEMU refuses to start:
>>
>>   $ sudo qemu-system-ppc64 -M pseries,accel=kvm,kvm-type=PR \
>>                            -nographic -vga none -cpu POWER8
>>   qemu: fatal: Unknown MMU model 851972
>>
>> ... followed by a useless register dump. I've bisected the issue, and it
>> seems like the problem has been introduced by this commit here:
>>
>>   commit 4322e8ced5aaac7191958f09622d199fe61e2d87
>>   ppc: Fix 64K pages support in full emulation
>>
>> Seems like KVM PR is using the "degraded" ISA variants (without the 1TB
>> segments), but the new POWERPC_MMU_64K flag has not been added to those.
>> Has this been done on purpose, or was this just by accident?
> 
> The "degraded" architecture support has some history behind it :
> 
>  commit 126a79300971 added it
>  commit aa4bb5875231 removed it.
>  commit ba3ecda05e93 readded it.
>  commit 4322e8ced5aa forgot about it again
> 
>> I can make KVM PR working again with the following patch:
> 
> I think this is correct. Let's wait for Ben to chime in :)

So I confirm the bug and the fix. 

There are other issues after in the guest (kernel crashing). But I think
these are related to TM which is not supported in KVM-PR. I am not sure
where we are on that point.

Thanks,

C. 

>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>> index 2864105..36694cb 100644
>> --- a/target-ppc/cpu-qom.h
>> +++ b/target-ppc/cpu-qom.h
>> @@ -81,6 +81,7 @@ enum powerpc_mmu_t {
>>                               | POWERPC_MMU_AMR | 0x00000003,
>>      /* Architecture 2.06 "degraded" (no 1T segments)           */
>>      POWERPC_MMU_2_06a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
>> +                             | POWERPC_MMU_64K
>>                               | 0x00000003,
>>      /* Architecture 2.07 variant                               */
>>      POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
>> @@ -88,6 +89,7 @@ enum powerpc_mmu_t {
>>                               | POWERPC_MMU_AMR | 0x00000004,
>>      /* Architecture 2.07 "degraded" (no 1T segments)           */
>>      POWERPC_MMU_2_07a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
>> +                             | POWERPC_MMU_64K
>>                               | 0x00000004,
>>  };
>>
>> However, not sure whether this is the right fix ... Cédric, Ben, any ideas?
> 
> 
> 
> 

Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU

[Thomas Huth]

On 20.09.2016 16:04, Cédric Le Goater wrote:
> On 09/20/2016 02:24 PM, Cédric Le Goater wrote:
>> On 09/20/2016 01:44 PM, Thomas Huth wrote:
>>>  Hi,
>>>
>>> when I try to run my guest in KVM-PR mode, current QEMU refuses to start:
>>>
>>>   $ sudo qemu-system-ppc64 -M pseries,accel=kvm,kvm-type=PR \
>>>                            -nographic -vga none -cpu POWER8
>>>   qemu: fatal: Unknown MMU model 851972
>>>
>>> ... followed by a useless register dump. I've bisected the issue, and it
>>> seems like the problem has been introduced by this commit here:
>>>
>>>   commit 4322e8ced5aaac7191958f09622d199fe61e2d87
>>>   ppc: Fix 64K pages support in full emulation
>>>
>>> Seems like KVM PR is using the "degraded" ISA variants (without the 1TB
>>> segments), but the new POWERPC_MMU_64K flag has not been added to those.
>>> Has this been done on purpose, or was this just by accident?
>>
>> The "degraded" architecture support has some history behind it :
>>
>>  commit 126a79300971 added it
>>  commit aa4bb5875231 removed it.
>>  commit ba3ecda05e93 readded it.
>>  commit 4322e8ced5aa forgot about it again
>>
>>> I can make KVM PR working again with the following patch:
>>
>> I think this is correct. Let's wait for Ben to chime in :)
> 
> So I confirm the bug and the fix. 
> 
> There are other issues after in the guest (kernel crashing). But I think
> these are related to TM which is not supported in KVM-PR. I am not sure
> where we are on that point.

There was a patch some months ago:

https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00046.html

... but I think it has never been included, as far as I can see.

 Thomas

Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU

[Cédric Le Goater]

On 09/20/2016 04:24 PM, Thomas Huth wrote:
> On 20.09.2016 16:04, Cédric Le Goater wrote:
>> On 09/20/2016 02:24 PM, Cédric Le Goater wrote:
>>> On 09/20/2016 01:44 PM, Thomas Huth wrote:
>>>>  Hi,
>>>>
>>>> when I try to run my guest in KVM-PR mode, current QEMU refuses to start:
>>>>
>>>>   $ sudo qemu-system-ppc64 -M pseries,accel=kvm,kvm-type=PR \
>>>>                            -nographic -vga none -cpu POWER8
>>>>   qemu: fatal: Unknown MMU model 851972
>>>>
>>>> ... followed by a useless register dump. I've bisected the issue, and it
>>>> seems like the problem has been introduced by this commit here:
>>>>
>>>>   commit 4322e8ced5aaac7191958f09622d199fe61e2d87
>>>>   ppc: Fix 64K pages support in full emulation
>>>>
>>>> Seems like KVM PR is using the "degraded" ISA variants (without the 1TB
>>>> segments), but the new POWERPC_MMU_64K flag has not been added to those.
>>>> Has this been done on purpose, or was this just by accident?
>>>
>>> The "degraded" architecture support has some history behind it :
>>>
>>>  commit 126a79300971 added it
>>>  commit aa4bb5875231 removed it.
>>>  commit ba3ecda05e93 readded it.
>>>  commit 4322e8ced5aa forgot about it again
>>>
>>>> I can make KVM PR working again with the following patch:
>>>
>>> I think this is correct. Let's wait for Ben to chime in :)
>>
>> So I confirm the bug and the fix. 
>>
>> There are other issues after in the guest (kernel crashing). But I think
>> these are related to TM which is not supported in KVM-PR. I am not sure
>> where we are on that point.
> 
> There was a patch some months ago:
> 
> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00046.html
> 
> ... but I think it has never been included, as far as I can see.

and with that patch, the guest fully boots. But David had some concerns
on the way it is done. It would be nice to put some cycle on this. 

Thanks,

C.

Re: [Qemu-devel] KVM-PR is broken with current QEMU

[Benjamin Herrenschmidt]

On Tue, 2016-09-20 at 13:44 +0200, Thomas Huth wrote:
> 
> Seems like KVM PR is using the "degraded" ISA variants (without the
> 1TB
> segments), but the new POWERPC_MMU_64K flag has not been added to
> those.
> Has this been done on purpose, or was this just by accident?
> I can make KVM PR working again with the following patch:
> 
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index 2864105..36694cb 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -81,6 +81,7 @@ enum powerpc_mmu_t {
>                               | POWERPC_MMU_AMR | 0x00000003,
>      /* Architecture 2.06 "degraded" (no 1T segments)           */
>      POWERPC_MMU_2_06a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
> +                             | POWERPC_MMU_64K
>                               | 0x00000003,
>      /* Architecture 2.07 variant                               */
>      POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> @@ -88,6 +89,7 @@ enum powerpc_mmu_t {
>                               | POWERPC_MMU_AMR | 0x00000004,
>      /* Architecture 2.07 "degraded" (no 1T segments)           */
>      POWERPC_MMU_2_07a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
> +                             | POWERPC_MMU_64K
>                               | 0x00000004,
>  };
> 
> However, not sure whether this is the right fix ... Cédric, Ben, any
> ideas?

Oh I thought I had removed the degraded variants ... Definitely looks
like an accident. I *think* PR KVM supports 64K pages, no ? If not,
then we shouldn't enable the flag.. somebody needs to check the kernel.

Cheers,
Ben.

Re: [Qemu-devel] KVM-PR is broken with current QEMU

[Thomas Huth]

On 20.09.2016 23:45, Benjamin Herrenschmidt wrote:
> On Tue, 2016-09-20 at 13:44 +0200, Thomas Huth wrote:
>>
>> Seems like KVM PR is using the "degraded" ISA variants (without the
>> 1TB
>> segments), but the new POWERPC_MMU_64K flag has not been added to
>> those.
>> Has this been done on purpose, or was this just by accident?
>> I can make KVM PR working again with the following patch:
>>
>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>> index 2864105..36694cb 100644
>> --- a/target-ppc/cpu-qom.h
>> +++ b/target-ppc/cpu-qom.h
>> @@ -81,6 +81,7 @@ enum powerpc_mmu_t {
>>                               | POWERPC_MMU_AMR | 0x00000003,
>>      /* Architecture 2.06 "degraded" (no 1T segments)           */
>>      POWERPC_MMU_2_06a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
>> +                             | POWERPC_MMU_64K
>>                               | 0x00000003,
>>      /* Architecture 2.07 variant                               */
>>      POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
>> @@ -88,6 +89,7 @@ enum powerpc_mmu_t {
>>                               | POWERPC_MMU_AMR | 0x00000004,
>>      /* Architecture 2.07 "degraded" (no 1T segments)           */
>>      POWERPC_MMU_2_07a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
>> +                             | POWERPC_MMU_64K
>>                               | 0x00000004,
>>  };
>>
>> However, not sure whether this is the right fix ... Cédric, Ben, any
>> ideas?
> 
> Oh I thought I had removed the degraded variants ... Definitely looks
> like an accident. I *think* PR KVM supports 64K pages, no ? If not,
> then we shouldn't enable the flag.. somebody needs to check the kernel.

I've now added some debug printf statements to kvm_fixup_page_sizes() in
QEMU, and it seems that at least my current downstream kernel does not
report support for 64K page sizes. So the right fix is likely to disable
the POWERPC_MMU_64K bit there in env->mmu_model if the kernel does not
report support for 64K pages. I'll try to come up with a patch...

But actually, now I wonder why my kernel does not support this page
size. There was a kernel patch from Paul (a4a0f2524acc2c6 - "KVM: PPC:
Book3S PR: Allow guest to use 64k pages") which added this page size to
KVM-PR, so IMHO it should work ... Seem like I need to do some more
debugging here...

 Thomas

Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU

[Thomas Huth]

On 20.09.2016 16:39, Cédric Le Goater wrote:
> On 09/20/2016 04:24 PM, Thomas Huth wrote:
>> On 20.09.2016 16:04, Cédric Le Goater wrote:
[...]
>>> There are other issues after in the guest (kernel crashing). But I think
>>> these are related to TM which is not supported in KVM-PR. I am not sure
>>> where we are on that point.
>>
>> There was a patch some months ago:
>>
>> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00046.html
>>
>> ... but I think it has never been included, as far as I can see.
> 
> and with that patch, the guest fully boots. But David had some concerns
> on the way it is done. It would be nice to put some cycle on this. 

Looking at the mail thread, I think TM should be currently disabled for
both, KVM-PR and TCG, i.e. only enabled for KVM-HV. The TM support in
TCG is just fake, since TBEGIN always fails.
Once we've got proper TM support in TCG, this can be easily changed
within QEMU. And once we've got TM support in KVM-PR, I think we should
also introduce a capability flag to KVM which can be used to inform QEMU
about this.

So I think Anton's patch currently just lacks the check for TCG.
Anton, if you've got some spare minutes, could you maybe send an updated
version of that patch?

 Thanks,
  Thomas

Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

On Wed, Sep 21, 2016 at 10:22:11AM +0200, Thomas Huth wrote:
> On 20.09.2016 16:39, Cédric Le Goater wrote:
> > On 09/20/2016 04:24 PM, Thomas Huth wrote:
> >> On 20.09.2016 16:04, Cédric Le Goater wrote:
> [...]
> >>> There are other issues after in the guest (kernel crashing). But I think
> >>> these are related to TM which is not supported in KVM-PR. I am not sure
> >>> where we are on that point.
> >>
> >> There was a patch some months ago:
> >>
> >> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00046.html
> >>
> >> ... but I think it has never been included, as far as I can see.
> > 
> > and with that patch, the guest fully boots. But David had some concerns
> > on the way it is done. It would be nice to put some cycle on this. 
> 
> Looking at the mail thread, I think TM should be currently disabled for
> both, KVM-PR and TCG, i.e. only enabled for KVM-HV. The TM support in
> TCG is just fake, since TBEGIN always fails.

Right.  So there's two questions here

1) Is qemu correctly advertising availability of TM in the device
tree?

If not we need to fix that, which might involve adding a kernel
capability for the PR case.

2) Is the kvm unit test properly checking for availability of TM
before executing?

> Once we've got proper TM support in TCG, this can be easily changed
> within QEMU. And once we've got TM support in KVM-PR, I think we should
> also introduce a capability flag to KVM which can be used to inform QEMU
> about this.
> 
> So I think Anton's patch currently just lacks the check for TCG.
> Anton, if you've got some spare minutes, could you maybe send an updated
> version of that patch?

Sorry, which patch of Anton's?

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU

[Thomas Huth]

[-- Attachment #1: Type: text/plain, Size: 2247 bytes --]

On Thu, 22 Sep 2016 11:57:15 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Sep 21, 2016 at 10:22:11AM +0200, Thomas Huth wrote:
> > On 20.09.2016 16:39, Cédric Le Goater wrote:
> > > On 09/20/2016 04:24 PM, Thomas Huth wrote:
> > >> On 20.09.2016 16:04, Cédric Le Goater wrote:
> > [...]
> > >>> There are other issues after in the guest (kernel crashing). But I think
> > >>> these are related to TM which is not supported in KVM-PR. I am not sure
> > >>> where we are on that point.
> > >>
> > >> There was a patch some months ago:
> > >>
> > >> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00046.html
> > >>
> > >> ... but I think it has never been included, as far as I can see.
> > > 
> > > and with that patch, the guest fully boots. But David had some concerns
> > > on the way it is done. It would be nice to put some cycle on this. 
> > 
> > Looking at the mail thread, I think TM should be currently disabled for
> > both, KVM-PR and TCG, i.e. only enabled for KVM-HV. The TM support in
> > TCG is just fake, since TBEGIN always fails.
> 
> Right.  So there's two questions here
> 
> 1) Is qemu correctly advertising availability of TM in the device
> tree?

If I've got that right, it's currently always advertising TM, even if
it's not really available (in TCG mode and PR mode).

> If not we need to fix that, which might involve adding a kernel
> capability for the PR case.
> 
> 2) Is the kvm unit test properly checking for availability of TM
> before executing?

Not yet. That's why it would be good to get a proper way for testing
for the availability of TM --> i.e. something like Anton's patch.

> > Once we've got proper TM support in TCG, this can be easily changed
> > within QEMU. And once we've got TM support in KVM-PR, I think we should
> > also introduce a capability flag to KVM which can be used to inform QEMU
> > about this.
> > 
> > So I think Anton's patch currently just lacks the check for TCG.
> > Anton, if you've got some spare minutes, could you maybe send an updated
> > version of that patch?
> 
> Sorry, which patch of Anton's?

This one:
https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00415.html

 Thomas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] KVM-PR is broken with current QEMU

[Thomas Huth]

On Wed, 21 Sep 2016 07:45:35 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Tue, 2016-09-20 at 13:44 +0200, Thomas Huth wrote:
> > 
> > Seems like KVM PR is using the "degraded" ISA variants (without the
> > 1TB
> > segments), but the new POWERPC_MMU_64K flag has not been added to
> > those.
> > Has this been done on purpose, or was this just by accident?
> > I can make KVM PR working again with the following patch:
> > 
> > diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> > index 2864105..36694cb 100644
> > --- a/target-ppc/cpu-qom.h
> > +++ b/target-ppc/cpu-qom.h
> > @@ -81,6 +81,7 @@ enum powerpc_mmu_t {
> >                               | POWERPC_MMU_AMR | 0x00000003,
> >      /* Architecture 2.06 "degraded" (no 1T segments)           */
> >      POWERPC_MMU_2_06a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
> > +                             | POWERPC_MMU_64K
> >                               | 0x00000003,
> >      /* Architecture 2.07 variant                               */
> >      POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> > @@ -88,6 +89,7 @@ enum powerpc_mmu_t {
> >                               | POWERPC_MMU_AMR | 0x00000004,
> >      /* Architecture 2.07 "degraded" (no 1T segments)           */
> >      POWERPC_MMU_2_07a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
> > +                             | POWERPC_MMU_64K
> >                               | 0x00000004,
> >  };
> > 
> > However, not sure whether this is the right fix ... Cédric, Ben, any
> > ideas?
> 
> Oh I thought I had removed the degraded variants ... Definitely looks
> like an accident. I *think* PR KVM supports 64K pages, no ? If not,
> then we shouldn't enable the flag.. somebody needs to check the kernel.

Yes, it supports 64k pages - but only on POWER8, not on POWER8E or
POWER8NVL yet. I've posted a patch to fix this here:

https://patchwork.ozlabs.org/patch/672841/

 Thomas

Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU

[Thomas Huth]

[-- Attachment #1: Type: text/plain, Size: 3163 bytes --]

On Thu, 22 Sep 2016 07:30:52 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On Thu, 22 Sep 2016 11:57:15 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Wed, Sep 21, 2016 at 10:22:11AM +0200, Thomas Huth wrote:
> > > On 20.09.2016 16:39, Cédric Le Goater wrote:
> > > > On 09/20/2016 04:24 PM, Thomas Huth wrote:
> > > >> On 20.09.2016 16:04, Cédric Le Goater wrote:
> > > [...]
> > > >>> There are other issues after in the guest (kernel crashing). But I think
> > > >>> these are related to TM which is not supported in KVM-PR. I am not sure
> > > >>> where we are on that point.
> > > >>
> > > >> There was a patch some months ago:
> > > >>
> > > >> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00046.html
> > > >>
> > > >> ... but I think it has never been included, as far as I can see.
> > > > 
> > > > and with that patch, the guest fully boots. But David had some concerns
> > > > on the way it is done. It would be nice to put some cycle on this. 
> > > 
> > > Looking at the mail thread, I think TM should be currently disabled for
> > > both, KVM-PR and TCG, i.e. only enabled for KVM-HV. The TM support in
> > > TCG is just fake, since TBEGIN always fails.
> > 
> > Right.  So there's two questions here
> > 
> > 1) Is qemu correctly advertising availability of TM in the device
> > tree?
> 
> If I've got that right, it's currently always advertising TM, even if
> it's not really available (in TCG mode and PR mode).
> 
> > If not we need to fix that, which might involve adding a kernel
> > capability for the PR case.
> > 
> > 2) Is the kvm unit test properly checking for availability of TM
> > before executing?
> 
> Not yet. That's why it would be good to get a proper way for testing
> for the availability of TM --> i.e. something like Anton's patch.
> 
> > > Once we've got proper TM support in TCG, this can be easily changed
> > > within QEMU. And once we've got TM support in KVM-PR, I think we should
> > > also introduce a capability flag to KVM which can be used to inform QEMU
> > > about this.
> > > 
> > > So I think Anton's patch currently just lacks the check for TCG.
> > > Anton, if you've got some spare minutes, could you maybe send an updated
> > > version of that patch?
> > 
> > Sorry, which patch of Anton's?
> 
> This one:
> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00415.html

Actually, looking at that whole pa-feature code in QEMU, I think
there's some more work to do here: Everything that is not using
mmu_model==POWERPC_MMU_2_06 is automatically getting pa_features_207.
This is sometimes completely wrong, for example when running with
KVM-PR, the mmu_model for POWER7 is POWERPC_MMU_2_06a instead.
Or when running with TCG, I think it's also perfectly legal to run the
pseries machine with a POWER5+ or PPC970 CPU - and we certainly do not
want to use pa_features_207 there.

So if you like, I can try to come up with a small patch series that
cleans up this mess - and I could also include an updated versions of
Anton's patch there unless he wants to redo the changes on his own...?

 Thomas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU

[Cédric Le Goater]

On 09/22/2016 09:18 AM, Thomas Huth wrote:
> On Thu, 22 Sep 2016 07:30:52 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On Thu, 22 Sep 2016 11:57:15 +1000
>> David Gibson <david@gibson.dropbear.id.au> wrote:
>>
>>> On Wed, Sep 21, 2016 at 10:22:11AM +0200, Thomas Huth wrote:
>>>> On 20.09.2016 16:39, Cédric Le Goater wrote:
>>>>> On 09/20/2016 04:24 PM, Thomas Huth wrote:
>>>>>> On 20.09.2016 16:04, Cédric Le Goater wrote:
>>>> [...]
>>>>>>> There are other issues after in the guest (kernel crashing). But I think
>>>>>>> these are related to TM which is not supported in KVM-PR. I am not sure
>>>>>>> where we are on that point.
>>>>>>
>>>>>> There was a patch some months ago:
>>>>>>
>>>>>> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00046.html
>>>>>>
>>>>>> ... but I think it has never been included, as far as I can see.
>>>>>
>>>>> and with that patch, the guest fully boots. But David had some concerns
>>>>> on the way it is done. It would be nice to put some cycle on this. 
>>>>
>>>> Looking at the mail thread, I think TM should be currently disabled for
>>>> both, KVM-PR and TCG, i.e. only enabled for KVM-HV. The TM support in
>>>> TCG is just fake, since TBEGIN always fails.
>>>
>>> Right.  So there's two questions here
>>>
>>> 1) Is qemu correctly advertising availability of TM in the device
>>> tree?
>>
>> If I've got that right, it's currently always advertising TM, even if
>> it's not really available (in TCG mode and PR mode).
>>
>>> If not we need to fix that, which might involve adding a kernel
>>> capability for the PR case.
>>>
>>> 2) Is the kvm unit test properly checking for availability of TM
>>> before executing?
>>
>> Not yet. That's why it would be good to get a proper way for testing
>> for the availability of TM --> i.e. something like Anton's patch.
>>
>>>> Once we've got proper TM support in TCG, this can be easily changed
>>>> within QEMU. And once we've got TM support in KVM-PR, I think we should
>>>> also introduce a capability flag to KVM which can be used to inform QEMU
>>>> about this.
>>>>
>>>> So I think Anton's patch currently just lacks the check for TCG.
>>>> Anton, if you've got some spare minutes, could you maybe send an updated
>>>> version of that patch?
>>>
>>> Sorry, which patch of Anton's?
>>
>> This one:
>> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00415.html
> 
> Actually, looking at that whole pa-feature code in QEMU, I think
> there's some more work to do here: Everything that is not using
> mmu_model==POWERPC_MMU_2_06 is automatically getting pa_features_207.
> This is sometimes completely wrong, for example when running with
> KVM-PR, the mmu_model for POWER7 is POWERPC_MMU_2_06a instead.
> Or when running with TCG, I think it's also perfectly legal to run the
> pseries machine with a POWER5+ or PPC970 CPU - and we certainly do not
> want to use pa_features_207 there.
> 
> So if you like, I can try to come up with a small patch series that
> cleans up this mess - and I could also include an updated versions of
> Anton's patch there unless he wants to redo the changes on his own...?
> 
>  Thomas
> 

That would be nice. I just gave a quick try on a f24/le kvm-pr running 
under a f24/le kvm-hv running under powernv. Only your couple of patches 
plus Anton's are needed to make it work.


Thanks,

C.

Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU

[Anton Blanchard]

Hi Thomas,

> So if you like, I can try to come up with a small patch series that
> cleans up this mess - and I could also include an updated versions of
> Anton's patch there unless he wants to redo the changes on his own...?

Thanks for looking at this. I'm travelling (stuck in an airport at the
moment) and wont be able to get to this for a few days. If you could
incorporate my fixes that would be great!

From memory we were waiting on KVM_CAP_PPC_HTM, which thanks to Sam is
now upstream in 23528bb21ee2

Anton