linux-kernel - Re: [PATCH v2 3/4] arm64/mm: Elide tlbi in contpte

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7e987f17-ffcb-45e0-8588-2d569d90f776@redhat.com>
Date: Mon, 3 Mar 2025 10:57:21 +0100
From: David Hildenbrand <david@...hat.com>
To: Mikołaj Lenczewski <miko.lenczewski@....com>
Cc: ryan.roberts@....com, suzuki.poulose@....com,
 yang@...amperecomputing.com, catalin.marinas@....com, will@...nel.org,
 joro@...tes.org, jean-philippe@...aro.org, mark.rutland@....com,
 joey.gouly@....com, oliver.upton@...ux.dev, james.morse@....com,
 broonie@...nel.org, maz@...nel.org, akpm@...ux-foundation.org, jgg@...pe.ca,
 nicolinc@...dia.com, mshavit@...gle.com, jsnitsel@...hat.com,
 smostafa@...gle.com, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, iommu@...ts.linux.dev
Subject: Re: [PATCH v2 3/4] arm64/mm: Elide tlbi in contpte_convert() under
 BBML2

On 03.03.25 10:49, Mikołaj Lenczewski wrote:
> Hi David,
> 
> Thanks for taking the time to review.
> 
> On Mon, Mar 03, 2025 at 10:17:12AM +0100, David Hildenbrand wrote:
>> On 28.02.25 19:24, Mikołaj Lenczewski wrote:
>>> If we support bbml2 without conflict aborts, we can avoid the final
>>> flush and have hardware manage the tlb entries for us. Avoiding flushes
>>> is a win.
>>>
>>> Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@....com>
>>> Reviewed-by: Ryan Roberts <ryan.roberts@....com>
>>> ---
>>>    arch/arm64/mm/contpte.c | 3 ---
>>>    1 file changed, 3 deletions(-)
>>>
>>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
>>> index 145530f706a9..77ed03b30b72 100644
>>> --- a/arch/arm64/mm/contpte.c
>>> +++ b/arch/arm64/mm/contpte.c
>>> @@ -72,9 +72,6 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
>>>    		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>>>    	__set_ptes(mm, start_addr, start_ptep, pte, CONT_PTES);
>>> -
>>> -	if (system_supports_bbml2_noabort())
>>> -		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>>>    }
>>>    void __contpte_try_fold(struct mm_struct *mm, unsigned long addr,
>>
>> What's the point of not squashing this into #2? :)
>>
>> If this split was requested during earlier review, at least seeing patch #2
>> on its own confused me.
> 
> This split is a holdover from an earlier patchset, where it was still
> unknown whether the removal of the second flush was permitted with
> BBML2. Partly this was due to us being worried about conflict aborts
> after the removal, and partly this was because the "delay" is a separate
> optimisation that we could apply even if it turned out the final patch
> was not architecturally sound.
> 
> Now that we do not handle conflict aborts (preferring only systems that
> handle BBML2 without ever raising aborts), the first issue is not a
> problem. The reasoning behind the second patch is also a little bit
> outdated, but I can see the logical split between a tlbi reorder, and
> the removal of the tlbi. If this is truly redundant though, I would be
> happy to squash the two into a single patch.

Thanks for the information.

Does patch #2 (reordering the tlbi) have any benefit on its own? I read 
"other threads will not see an invalid pagetable entry", but I am not 
sure that is correct. A concurrent HW page table walker would still find 
the invalid PTE? It's just a matter of TLB state.

If there is no benefit in having patch #2 independently, I'd just squash 
them. Reordering to then remove is more complicated than just removing 
it IMHO.

-- 
Cheers,

David / dhildenb