Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Michael Ellerman <mpe@ellerman.id.au>
To:     bharata@linux.ibm.com, Nicholas Piggin <npiggin@gmail.com>
Cc:     Qian Cai <cai@lca.pw>, aneesh.kumar@linux.ibm.com,
        linux-kernel@vger.kernel.org, linux-next@vger.kernel.org,
        linuxppc-dev@lists.ozlabs.org, sfr@canb.auug.org.au
Subject: Re: [PATCH v3 0/3] Off-load TLB invalidations to host for !GTSE
In-Reply-To: <20200717042854.GL7902@in.ibm.com>
References: <20200703053608.12884-1-bharata@linux.ibm.com> <20200716172713.GA4565@lca.pw> <1594950229.jn9ipe6td1.astroid@bobo.none> <1594953143.b8px5ir35m.astroid@bobo.none> <20200717042854.GL7902@in.ibm.com>
Date:   Sat, 18 Jul 2020 22:40:34 +1000
Message-ID: <87pn8tt2rh.fsf@mpe.ellerman.id.au>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Bharata B Rao <bharata@linux.ibm.com> writes:
> On Fri, Jul 17, 2020 at 12:44:00PM +1000, Nicholas Piggin wrote:
>> Excerpts from Nicholas Piggin's message of July 17, 2020 12:08 pm:
>> > Excerpts from Qian Cai's message of July 17, 2020 3:27 am:
>> >> On Fri, Jul 03, 2020 at 11:06:05AM +0530, Bharata B Rao wrote:
>> >>> Hypervisor may choose not to enable Guest Translation Shootdown Enable
>> >>> (GTSE) option for the guest. When GTSE isn't ON, the guest OS isn't
>> >>> permitted to use instructions like tblie and tlbsync directly, but is
>> >>> expected to make hypervisor calls to get the TLB flushed.
>> >>> 
>> >>> This series enables the TLB flush routines in the radix code to
>> >>> off-load TLB flushing to hypervisor via the newly proposed hcall
>> >>> H_RPT_INVALIDATE. 
>> >>> 
>> >>> To easily check the availability of GTSE, it is made an MMU feature.
>> >>> The OV5 handling and H_REGISTER_PROC_TBL hcall are changed to
>> >>> handle GTSE as an optionally available feature and to not assume GTSE
>> >>> when radix support is available.
>> >>> 
>> >>> The actual hcall implementation for KVM isn't included in this
>> >>> patchset and will be posted separately.
>> >>> 
>> >>> Changes in v3
>> >>> =============
>> >>> - Fixed a bug in the hcall wrapper code where we were missing setting
>> >>>   H_RPTI_TYPE_NESTED while retrying the failed flush request with
>> >>>   a full flush for the nested case.
>> >>> - s/psize_to_h_rpti/psize_to_rpti_pgsize
>> >>> 
>> >>> v2: https://lore.kernel.org/linuxppc-dev/20200626131000.5207-1-bharata@linux.ibm.com/T/#t
>> >>> 
>> >>> Bharata B Rao (2):
>> >>>   powerpc/mm: Enable radix GTSE only if supported.
>> >>>   powerpc/pseries: H_REGISTER_PROC_TBL should ask for GTSE only if
>> >>>     enabled
>> >>> 
>> >>> Nicholas Piggin (1):
>> >>>   powerpc/mm/book3s64/radix: Off-load TLB invalidations to host when
>> >>>     !GTSE
>> >> 
>> >> Reverting the whole series fixed random memory corruptions during boot on
>> >> POWER9 PowerNV systems below.
>> > 
>> > If I s/mmu_has_feature(MMU_FTR_GTSE)/(1)/g in radix_tlb.c, then the .o
>> > disasm is the same as reverting my patch.
>> > 
>> > Feature bits not being set right? PowerNV should be pretty simple, seems
>> > to do the same as FTR_TYPE_RADIX.
>> 
>> Might need this fix
>> 
>> ---
>> 
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index 9cc49f265c86..54c9bcea9d4e 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -163,7 +163,7 @@ static struct ibm_pa_feature {
>>  	{ .pabyte = 0,  .pabit = 6, .cpu_features  = CPU_FTR_NOEXECUTE },
>>  	{ .pabyte = 1,  .pabit = 2, .mmu_features  = MMU_FTR_CI_LARGE_PAGE },
>>  #ifdef CONFIG_PPC_RADIX_MMU
>> -	{ .pabyte = 40, .pabit = 0, .mmu_features  = MMU_FTR_TYPE_RADIX },
>> +	{ .pabyte = 40, .pabit = 0, .mmu_features  = (MMU_FTR_TYPE_RADIX | MMU_FTR_GTSE) },
>>  #endif
>>  	{ .pabyte = 1,  .pabit = 1, .invert = 1, .cpu_features = CPU_FTR_NODSISRALIGN },
>>  	{ .pabyte = 5,  .pabit = 0, .cpu_features  = CPU_FTR_REAL_LE,
>
> Michael - Let me know if this should be folded into 1/3 and the complete
> series resent.

No it's already merged.

Please send a proper patch with a Fixes: tag and a change log that
explains the details.

cheers