lkml.org 
[lkml]   [2018]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH V2] arm64: tlbflush: avoid writing RES0 bits
From
Date
Hi Will,

On 03/27/2018 12:36 PM, Will Deacon wrote:
> Hi Shanker,
>
> On Tue, Mar 27, 2018 at 09:53:16AM -0500, Shanker Donthineni wrote:
>> On 03/27/2018 06:34 AM, Robin Murphy wrote:
>>> On 27/03/18 04:21, Philip Elcan wrote:
>>>> Several of the bits of the TLBI register operand are RES0 per the ARM
>>>> ARM, so TLBI operations should avoid writing non-zero values to these
>>>> bits.
>>>>
>>>> This patch adds a macro __TLBI_VADDR(addr, asid) that creates the
>>>> operand register in the correct format and honors the RES0 bits.
>>>>
>>>> Signed-off-by: Philip Elcan <pelcan@codeaurora.org>
>>>> ---
>>>>   arch/arm64/include/asm/tlbflush.h | 23 ++++++++++++++++-------
>>>>   1 file changed, 16 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
>>>> index 9e82dd7..b1205e9 100644
>>>> --- a/arch/arm64/include/asm/tlbflush.h
>>>> +++ b/arch/arm64/include/asm/tlbflush.h
>>>> @@ -60,6 +60,15 @@
>>>>           __tlbi(op, (arg) | USER_ASID_FLAG);                \
>>>>   } while (0)
>>>>   +/* This macro creates a properly formatted VA operand for the TLBI */
>>>> +#define __TLBI_VADDR(addr, asid)                \
>>>> +    ({                            \
>>>> +        unsigned long __ta = (addr) >> 12;        \
>>>> +        __ta &= GENMASK_ULL(43, 0);            \
>>>> +        __ta |= (unsigned long)(asid) << 48;        \
>>>> +        __ta;                        \
>>>> +    })
>>>
>>> I'd be inclined to make this a static inline function rather than a
>>> macro, since it doesn't need to do any wacky type-dodging, but either
>>> way the overall change now looks appropriate;
>>>
>>> Acked-by: Robin Murphy <robin.murphy@arm.com>
>>>
>>
>> Tested-by: Shanker Donthineni <shankerd@codeaurora.org>
>
> [...]
>
>>>> @@ -154,8 +163,8 @@ static inline void __flush_tlb_range(struct vm_area_struct *vma,
>>>>           return;
>>>>       }
>>>>   -    start = asid | (start >> 12);
>>>> -    end = asid | (end >> 12);
>>>> +    start = __TLBI_VADDR(start, asid);
>>>> +    end = __TLBI_VADDR(end, asid);
>
> Can you test this bit too, please? ;)
>

I've verified the basic boot functionality on QDF2400 platform. But I can see now
after your comments, it leads to TLB conflicts because of ASID is truncated to zero
due to two times 48bit shift.

Thanks for catching this one.

@@ -146,7 +155,7 @@ static inline void __flush_tlb_range(structvm_area_struct *
unsigned long start, unsigned long end,
bool last_level)
{
- unsigned long asid = ASID(vma->vm_mm) << 48;
+ unsigned long asid = ASID(vma->vm_mm);
unsigned long addr;

if ((end - start) > MAX_TLB_RANGE) {
@@ -154,8 +163,8 @@ static inline void __flush_tlb_range(struct
vm_area_struct *
return;
}

- start = asid | (start >> 12);
- end = asid | (end >> 12);
+ start = __TLBI_VADDR(start, asid);
+ end = __TLBI_VADDR(end, asid);


> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

--
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
\
 
 \ /
  Last update: 2018-03-28 03:03    [W:1.819 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site