2019-08-30 12:10:49

by Laurent Dufour

[permalink] [raw]
Subject: [PATCH 0/3] powerpc/mm: Conditionally call H_BLOCK_REMOVE

Since the commit ba2dd8a26baa ("powerpc/pseries/mm: call H_BLOCK_REMOVE"),
the call to H_BLOCK_REMOVE is always done if the feature is exhibited.

On some system, the hypervisor may not support all the combination of
segment base page size and page size. When this happens the hcall is
returning H_PARAM, which is triggering a BUG_ON check leading to a panic.

The PAPR document is specifying a TLB Block Invalidate Characteristics item
detailing which couple base page size, page size the hypervisor is
supporting through H_BLOCK_REMOVE. Furthermore, the characteristics are
also providing the size of the block the hcall could process.

Supporting various block size seems not needed as all systems I was able to
play with was support an 8 addresses block size, which is the maximum
through the hcall. Supporting various size may complexify the algorithm in
call_block_remove() so unless this is required, this is not done.

In the case of block size different from 8, a warning message is displayed
at boot time and that block size will be ignored checking for the
H_BLOCK_REMOVE support.

Due to the minimal amount of hardware showing a limited set of
H_BLOCK_REMOVE supported page size, I don't think there is a need to push
this series to the stable mailing list.

The first patch is initializing the penc values for each page size to an
invalid value to be able to detect those which have been initialized as 0
is a valid value.

The second patch is reading the characteristic through the hcall
ibm,get-system-parameter and record the supported block size for each page
size.

The third patch is changing the check used to detect the H_BLOCK_REMOVE
availability to take care of the base page size and page size couple.

Laurent Dufour (3):
powerpc/mm: Initialize the HPTE encoding values
powperc/mm: read TLB Block Invalidate Characteristics
powerpc/mm: call H_BLOCK_REMOVE when supported

arch/powerpc/include/asm/book3s/64/mmu.h | 3 +
arch/powerpc/mm/book3s64/hash_utils.c | 8 +-
arch/powerpc/platforms/pseries/lpar.c | 118 ++++++++++++++++++++++-
3 files changed, 125 insertions(+), 4 deletions(-)

--
2.23.0


2019-09-12 13:48:44

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: [PATCH 0/3] powerpc/mm: Conditionally call H_BLOCK_REMOVE

On 8/30/19 5:37 PM, Laurent Dufour wrote:
> Since the commit ba2dd8a26baa ("powerpc/pseries/mm: call H_BLOCK_REMOVE"),
> the call to H_BLOCK_REMOVE is always done if the feature is exhibited.
>
> On some system, the hypervisor may not support all the combination of
> segment base page size and page size. When this happens the hcall is
> returning H_PARAM, which is triggering a BUG_ON check leading to a panic.
>
> The PAPR document is specifying a TLB Block Invalidate Characteristics item
> detailing which couple base page size, page size the hypervisor is
> supporting through H_BLOCK_REMOVE. Furthermore, the characteristics are
> also providing the size of the block the hcall could process.
>
> Supporting various block size seems not needed as all systems I was able to
> play with was support an 8 addresses block size, which is the maximum
> through the hcall. Supporting various size may complexify the algorithm in
> call_block_remove() so unless this is required, this is not done.
>
> In the case of block size different from 8, a warning message is displayed
> at boot time and that block size will be ignored checking for the
> H_BLOCK_REMOVE support.
>
> Due to the minimal amount of hardware showing a limited set of
> H_BLOCK_REMOVE supported page size, I don't think there is a need to push
> this series to the stable mailing list.
>
> The first patch is initializing the penc values for each page size to an
> invalid value to be able to detect those which have been initialized as 0
> is a valid value.
>
> The second patch is reading the characteristic through the hcall
> ibm,get-system-parameter and record the supported block size for each page
> size.
>
> The third patch is changing the check used to detect the H_BLOCK_REMOVE
> availability to take care of the base page size and page size couple.
>

So ibm,segment-page-sizes indicates wether we support a combination of
base page size and actual page size. You are suggesting that the value
reported by that is not correct? Can you also share the early part of
dmesg as below.

[ 0.000000] hash-mmu: Page sizes from device-tree:
[ 0.000000] hash-mmu: base_shift=12: shift=12, sllp=0x0000,
avpnm=0x00000000, tlbiel=1, penc=0
[ 0.000000] hash-mmu: base_shift=12: shift=16, sllp=0x0000,
avpnm=0x00000000, tlbiel=1, penc=7
[ 0.000000] hash-mmu: base_shift=12: shift=24, sllp=0x0000,
avpnm=0x00000000, tlbiel=1, penc=56
[ 0.000000] hash-mmu: base_shift=16: shift=16, sllp=0x0110,
avpnm=0x00000000, tlbiel=1, penc=1
[ 0.000000] hash-mmu: base_shift=16: shift=24, sllp=0x0110,
avpnm=0x00000000, tlbiel=1, penc=8
[ 0.000000] hash-mmu: base_shift=24: shift=24, sllp=0x0100,
avpnm=0x00000001, tlbiel=0, penc=0
[ 0.000000] hash-mmu: base_shift=34: shift=34, sllp=0x0120,
avpnm=0x000007ff, tlbiel=0, penc=3

That shows different base page size and actual page size combination.


> Laurent Dufour (3):
> powerpc/mm: Initialize the HPTE encoding values
> powperc/mm: read TLB Block Invalidate Characteristics
> powerpc/mm: call H_BLOCK_REMOVE when supported
>
> arch/powerpc/include/asm/book3s/64/mmu.h | 3 +
> arch/powerpc/mm/book3s64/hash_utils.c | 8 +-
> arch/powerpc/platforms/pseries/lpar.c | 118 ++++++++++++++++++++++-
> 3 files changed, 125 insertions(+), 4 deletions(-)
>


-aneesh

2019-09-13 11:33:41

by Laurent Dufour

[permalink] [raw]
Subject: Re: [PATCH 0/3] powerpc/mm: Conditionally call H_BLOCK_REMOVE

Le 12/09/2019 à 15:44, Aneesh Kumar K.V a écrit :
> On 8/30/19 5:37 PM, Laurent Dufour wrote:
>> Since the commit ba2dd8a26baa ("powerpc/pseries/mm: call H_BLOCK_REMOVE"),
>> the call to H_BLOCK_REMOVE is always done if the feature is exhibited.
>>
>> On some system, the hypervisor may not support all the combination of
>> segment base page size and page size. When this happens the hcall is
>> returning H_PARAM, which is triggering a BUG_ON check leading to a panic.
>>
>> The PAPR document is specifying a TLB Block Invalidate Characteristics item
>> detailing which couple base page size, page size the hypervisor is
>> supporting through H_BLOCK_REMOVE. Furthermore, the characteristics are
>> also providing the size of the block the hcall could process.
>>
>> Supporting various block size seems not needed as all systems I was able to
>> play with was support an 8 addresses block size, which is the maximum
>> through the hcall. Supporting various size may complexify the algorithm in
>> call_block_remove() so unless this is required, this is not done.
>>
>> In the case of block size different from 8, a warning message is displayed
>> at boot time and that block size will be ignored checking for the
>> H_BLOCK_REMOVE support.
>>
>> Due to the minimal amount of hardware showing a limited set of
>> H_BLOCK_REMOVE supported page size, I don't think there is a need to push
>> this series to the stable mailing list.
>>
>> The first patch is initializing the penc values for each page size to an
>> invalid value to be able to detect those which have been initialized as 0
>> is a valid value.
>>
>> The second patch is reading the characteristic through the hcall
>> ibm,get-system-parameter and record the supported block size for each page
>> size.
>>
>> The third patch is changing the check used to detect the H_BLOCK_REMOVE
>> availability to take care of the base page size and page size couple.
>>
>
> So ibm,segment-page-sizes indicates wether we support a combination of base
> page size and actual page size. You are suggesting that the value reported
> by that is not correct? Can you also share the early part of dmesg as below.

I'm not saying that the value reported by ibm,segment-page-sizes are
incorrect, I'm saying that some couple are not supported by the hcall
H_BLOCK_REMOVE.

May be should I change the second sentence by

On some system, the hypervisor may not support all the combination of
segment base page size and page size for the hcall H_BLOCK_REMOVE. When
this happens the hcall is returning H_PARAM, which is triggering a BUG_ON
check leading to a panic.

Is that clear enough now ?

>
> [    0.000000] hash-mmu: Page sizes from device-tree:
> [    0.000000] hash-mmu: base_shift=12: shift=12, sllp=0x0000,
> avpnm=0x00000000, tlbiel=1, penc=0
> [    0.000000] hash-mmu: base_shift=12: shift=16, sllp=0x0000,
> avpnm=0x00000000, tlbiel=1, penc=7
> [    0.000000] hash-mmu: base_shift=12: shift=24, sllp=0x0000,
> avpnm=0x00000000, tlbiel=1, penc=56
> [    0.000000] hash-mmu: base_shift=16: shift=16, sllp=0x0110,
> avpnm=0x00000000, tlbiel=1, penc=1
> [    0.000000] hash-mmu: base_shift=16: shift=24, sllp=0x0110,
> avpnm=0x00000000, tlbiel=1, penc=8
> [    0.000000] hash-mmu: base_shift=24: shift=24, sllp=0x0100,
> avpnm=0x00000001, tlbiel=0, penc=0
> [    0.000000] hash-mmu: base_shift=34: shift=34, sllp=0x0120,
> avpnm=0x000007ff, tlbiel=0, penc=3
>
> That shows different base page size and actual page size combination.
>
>
>> Laurent Dufour (3):
>>    powerpc/mm: Initialize the HPTE encoding values
>>    powperc/mm: read TLB Block Invalidate Characteristics
>>    powerpc/mm: call H_BLOCK_REMOVE when supported
>>
>>   arch/powerpc/include/asm/book3s/64/mmu.h |   3 +
>>   arch/powerpc/mm/book3s64/hash_utils.c    |   8 +-
>>   arch/powerpc/platforms/pseries/lpar.c    | 118 ++++++++++++++++++++++-
>>   3 files changed, 125 insertions(+), 4 deletions(-)
>>
>
>
> -aneesh