Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Laurent Dufour <ldufour@linux.ibm.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	mpe@ellerman.id.au, benh@kernel.crashing.org, paulus@samba.org,
	npiggin@gmail.com
Cc: linuxppc-dev@lists.ozlabs.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] powperc/mm: read TLB Block Invalidate Characteristics
Date: Fri, 13 Sep 2019 15:55:39 +0200
Message-ID: <5b5dfa47-43a0-5035-d620-addca4549bf7@linux.ibm.com> (raw)
In-Reply-To: <e809e33f-58df-ec99-44e8-9b1ae0b6bfe0@linux.ibm.com>

Le 12/09/2019 à 16:16, Aneesh Kumar K.V a écrit :
> On 8/30/19 5:37 PM, Laurent Dufour wrote:
>> The PAPR document specifies the TLB Block Invalidate Characteristics which
>> is telling which couple base page size / page size is supported by the
>> H_BLOCK_REMOVE hcall.
>>
>> A new set of feature is added to the mmu_psize_def structure to record per
>> base page size which page size is supported by H_BLOCK_REMOVE.
>>
>> A new init service is added to read the characteristics. The size of the
>> buffer is set to twice the number of known page size, plus 10 bytes to
>> ensure we have enough place.
>>
> 
> 
> So this is not really the base page size/actual page size combination. This 
> is related to H_BLOCK_REMOVE hcall, block size supported by that HCALL and 
> what page size combination is supported with that specific block size.

I agree

> 
> We should add that TLB block invalidate characteristics format in this patch.

Sure, will do that in a comment inside the code.

> 
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>> ---
>>   arch/powerpc/include/asm/book3s/64/mmu.h |   3 +
>>   arch/powerpc/platforms/pseries/lpar.c    | 107 +++++++++++++++++++++++
>>   2 files changed, 110 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h 
>> b/arch/powerpc/include/asm/book3s/64/mmu.h
>> index 23b83d3593e2..675895dfe39f 100644
>> --- a/arch/powerpc/include/asm/book3s/64/mmu.h
>> +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
>> @@ -12,11 +12,14 @@
>>    *    sllp  : is a bit mask with the value of SLB L || LP to be or'ed
>>    *            directly to a slbmte "vsid" value
>>    *    penc  : is the HPTE encoding mask for the "LP" field:
>> + *    hblk  : H_BLOCK_REMOVE supported block size for this page size in
>> + *            segment who's base page size is that page size.
>>    *
>>    */
>>   struct mmu_psize_def {
>>       unsigned int    shift;    /* number of bits */
>>       int        penc[MMU_PAGE_COUNT];    /* HPTE encoding */
>> +    int        hblk[MMU_PAGE_COUNT];    /* H_BLOCK_REMOVE support */
>>       unsigned int    tlbiel;    /* tlbiel supported for that page size */
>>       unsigned long    avpnm;    /* bits to mask out in AVPN in the HPTE */
>>       union {
>> diff --git a/arch/powerpc/platforms/pseries/lpar.c 
>> b/arch/powerpc/platforms/pseries/lpar.c
>> index 4f76e5f30c97..375e19b3cf53 100644
>> --- a/arch/powerpc/platforms/pseries/lpar.c
>> +++ b/arch/powerpc/platforms/pseries/lpar.c
>> @@ -1311,6 +1311,113 @@ static void do_block_remove(unsigned long number, 
>> struct ppc64_tlb_batch *batch,
>>           (void)call_block_remove(pix, param, true);
>>   }
>> +static inline void __init set_hblk_bloc_size(int bpsize, int psize,
>> +                         unsigned int block_size)
>> +{
>> +    struct mmu_psize_def *def = &mmu_psize_defs[bpsize];
>> +
>> +    if (block_size > def->hblk[psize])
>> +        def->hblk[psize] = block_size;
>> +}
>> +
>> +static inline void __init check_lp_set_hblk(unsigned int lp,
>> +                        unsigned int block_size)
>> +{
>> +    unsigned int bpsize, psize;
>> +
>> +
>> +    /* First, check the L bit, if not set, this means 4K */
>> +    if ((lp & 0x80) == 0) {
> 
> 
> What is that 0x80? We should have #define for most of those.

I will make that more explicit through a define

> 
>> +        set_hblk_bloc_size(MMU_PAGE_4K, MMU_PAGE_4K, block_size);
>> +        return;
>> +    }
>> +
>> +    /* PAPR says to look at bits 2-7 (0 = MSB) */
>> +    lp &= 0x3f;
> 
> Also convert that to #define?

Really ? The comment above is explicitly saying that we are looking at bits 
2-7. A define will obfuscate that.

> 
>> +    for (bpsize = 0; bpsize < MMU_PAGE_COUNT; bpsize++) {
>> +        struct mmu_psize_def *def =  &mmu_psize_defs[bpsize];
>> +
>> +        for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
>> +            if (def->penc[psize] == lp) {
>> +                set_hblk_bloc_size(bpsize, psize, block_size);
>> +                return;
>> +            }
>> +        }
>> +    }
>> +}
>> +
>> +#define SPLPAR_TLB_BIC_TOKEN        50
>> +#define SPLPAR_TLB_BIC_MAXLENGTH    (MMU_PAGE_COUNT*2 + 10)
>> +static int __init read_tlbbi_characteristics(void)
>> +{
>> +    int call_status;
>> +    unsigned char local_buffer[SPLPAR_TLB_BIC_MAXLENGTH];
>> +    int len, idx, bpsize;
>> +
>> +    if (!firmware_has_feature(FW_FEATURE_BLOCK_REMOVE)) {
>> +        pr_info("H_BLOCK_REMOVE is not supported");
>> +        return 0;
>> +    }
>> +
>> +    memset(local_buffer, 0, SPLPAR_TLB_BIC_MAXLENGTH);
>> +
>> +    spin_lock(&rtas_data_buf_lock);
>> +    memset(rtas_data_buf, 0, RTAS_DATA_BUF_SIZE);
>> +    call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
>> +                NULL,
>> +                SPLPAR_TLB_BIC_TOKEN,
>> +                __pa(rtas_data_buf),
>> +                RTAS_DATA_BUF_SIZE);
>> +    memcpy(local_buffer, rtas_data_buf, SPLPAR_TLB_BIC_MAXLENGTH);
>> +    local_buffer[SPLPAR_TLB_BIC_MAXLENGTH - 1] = '\0';
>> +    spin_unlock(&rtas_data_buf_lock);
>> +
>> +    if (call_status != 0) {
>> +        pr_warn("%s %s Error calling get-system-parameter (0x%x)\n",
>> +            __FILE__, __func__, call_status);
>> +        return 0;
>> +    }
>> +
>> +    /*
>> +     * The first two (2) bytes of the data in the buffer are the length of
>> +     * the returned data, not counting these first two (2) bytes.
>> +     */
>> +    len = local_buffer[0] * 256 + local_buffer[1] + 2;
>> +    if (len >= SPLPAR_TLB_BIC_MAXLENGTH) {
>> +        pr_warn("%s too large returned buffer %d", __func__, len);
>> +        return 0;
>> +    }
>> +
>> +    idx = 2;
>> +    while (idx < len) {
>> +        unsigned int block_size = local_buffer[idx++];
>> +        unsigned int npsize;
>> +
>> +        if (!block_size)
>> +            break;
>> +
>> +        block_size = 1 << block_size;
>> +        if (block_size != 8)
>> +            /* We only support 8 bytes size TLB invalidate buffer */
>> +            pr_warn("Unsupported H_BLOCK_REMOVE block size : %d\n",
>> +                block_size);
>> +
>> +        for (npsize = local_buffer[idx++];  npsize > 0; npsize--)
>> +            check_lp_set_hblk((unsigned int) local_buffer[idx++],
>> +                      block_size);
>> +    }
>> +
>> +    for (bpsize = 0; bpsize < MMU_PAGE_COUNT; bpsize++)
>> +        for (idx = 0; idx < MMU_PAGE_COUNT; idx++)
>> +            if (mmu_psize_defs[bpsize].hblk[idx])
>> +                pr_info("H_BLOCK_REMOVE supports base psize:%d psize:%d 
>> block size:%d",
>> +                    bpsize, idx,
>> +                    mmu_psize_defs[bpsize].hblk[idx]);
>> +
>> +    return 0;
>> +}
>> +machine_arch_initcall(pseries, read_tlbbi_characteristics);
>> +
> 
> Why a machine_arch_initcall() ? Can't we do this similar to how we do 
> segment-page-size parsing from device tree? Also this should be hash 
> translation mode specific.

Because that code is specific to the pseries architecture. the hash 
translation is not pseries specific.

Indeed the change in mmu_psize_defs is not too generic. The hblk 
characteristics should remain static to the lpar.c file where it is used.

> 
>>   /*
>>    * Take a spinlock around flushes to avoid bouncing the hypervisor tlbie
>>    * lock.
>>
> 



  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-30 12:07 [PATCH 0/3] powerpc/mm: Conditionally call H_BLOCK_REMOVE Laurent Dufour
2019-08-30 12:07 ` [PATCH 1/3] powerpc/mm: Initialize the HPTE encoding values Laurent Dufour
2019-09-12 13:37   ` Aneesh Kumar K.V
2019-09-13 10:56     ` Laurent Dufour
2019-08-30 12:07 ` [PATCH 2/3] powperc/mm: read TLB Block Invalidate Characteristics Laurent Dufour
2019-09-12 14:16   ` Aneesh Kumar K.V
2019-09-13 13:55     ` Laurent Dufour [this message]
2019-09-12 14:44   ` Aneesh Kumar K.V
2019-09-12 19:26     ` Laurent Dufour
2019-09-13  2:00       ` Aneesh Kumar K.V
2019-09-13  9:10         ` Laurent Dufour
2019-08-30 12:07 ` [PATCH 3/3] powerpc/mm: call H_BLOCK_REMOVE when supported Laurent Dufour
2019-09-12 14:20   ` Aneesh Kumar K.V
2019-09-13 13:16     ` Laurent Dufour
2019-09-12 13:44 ` [PATCH 0/3] powerpc/mm: Conditionally call H_BLOCK_REMOVE Aneesh Kumar K.V
2019-09-13 11:09   ` Laurent Dufour

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5b5dfa47-43a0-5035-d620-addca4549bf7@linux.ibm.com \
    --to=ldufour@linux.ibm.com \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org linux-mm@archiver.kernel.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox