Message-ID: <26a7ba50-e1cd-4c33-8b91-232413790786@arm.com>
Date: Wed, 26 Feb 2025 10:18:55 +0000
From: Ryan Roberts <ryan.roberts@....com>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Will Deacon <will@...nel.org>, Catalin Marinas <catalin.marinas@....com>,
 Mark Rutland <mark.rutland@....com>, Luiz Capitulino <luizcap@...hat.com>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] arm64/mm: Fix Boot panic on Ampere Altra

On 26/02/2025 10:07, Ard Biesheuvel wrote:
> On Wed, 26 Feb 2025 at 10:45, Ryan Roberts <ryan.roberts@....com> wrote:
>>
>> On 26/02/2025 09:04, Ard Biesheuvel wrote:
>>> On Wed, 26 Feb 2025 at 09:55, Ryan Roberts <ryan.roberts@....com> wrote:
>>>>
>>>> On 26/02/2025 08:33, Ard Biesheuvel wrote:
>>>>> On Wed, 26 Feb 2025 at 09:07, Ryan Roberts <ryan.roberts@....com> wrote:
>>>>>>
>>>>>> On 26/02/2025 06:59, Ard Biesheuvel wrote:
>>>>>>> On Wed, 26 Feb 2025 at 01:10, Will Deacon <will@...nel.org> wrote:
>>>>>>>>
>>>>>>>> On Tue, Feb 25, 2025 at 07:05:35PM +0100, Ard Biesheuvel wrote:
>>>>>>>>> Apologies for the breakage, and thanks for the fix.
>>>>>>>>>
>>>>>>>>> I have to admit that I was a bit overzealous here: there is no point
>>>>>>>>> yet in using the sanitised value, given that we don't actually
>>>>>>>>> override the PA range in the first place.
>>>>>>
>>>>>> But unless I've misunderstood something, parange is overridden; Commit
>>>>>> 62cffa496aac (the same one we are fixing) adds an override to force parange to
>>>>>> 48 bits when arm64.nolva is specified for LPA2 systems (see mmfr2_varange_filter()).
>>>>>>
>>>>>> I thought it would be preferable to honour that override, hence my use of
>>>>>> arm64_apply_feature_override() in the fix. Are you saying we don't need to worry
>>>>>> about that case?
>>>>>>
>>>>>
>>>>> I wouldn't think so (but I'm glad you brought it up because this
>>>>> didn't occur to me at all tbh)
>>>>>
>>>>> With arm64.nolva, both the VA and PA ranges will be reduced, and so
>>>>> the range of the linear map will be 47 bits. So if the PA range is
>>>>> being reduced from 52 to 48, it will still exceed the size of the
>>>>> linear map, and so it should make no difference in this particular
>>>>> case.
>>>>
>>>> OK, so I think you're saying it'll happen to work correctly even if we ignore
>>>> that override? That sounds a bit fragile to me. Surely we should be consistent
>>>> and either always honour the override or remove the override in the first place?
>>>>
>>>
>>> I'm trying to walk a fine line here between consistent use of the
>>> override, and fixing something that I broke in a nasty way all the way
>>> back to 6.12.
>>>
>>> So yes, I agree that it would be better to use the override
>>> consistently, and this is what we should do going forward. But the
>>> purpose of the original patch was mainly to ensure that we
>>> consistently program the MMU either in LPA2 or in non-LPA2 mode, and
>>> not in a mix of the two. The linear mapping randomization change was
>>> kind of secondary.
>>>
>>> So perhaps this should be two patches:
>>> - backing out the use of the sanitised register, as proposed by Will,
>>> with cc:stable
>>> - a follow up based on your proposal, which can be backported later if
>>> needed, but without tagging it for stable.
>>
>> I suspect I'm misunderstanding something crucial about the way the linear map
>> randomization works (TBH the details of the signed comparisons went a little
>> over my head).
>>
>> But my rough understanding is that there are 2 values you want to compare; the
>> size of the linear map and the PA range. If the PA range is significantly bigger
>> than the linear map size then we conclude we have enough room to randomize.
> 
> It is the other way around: the linear map should be able to cover all
> system RAM that may appear anywhere in the PA space, and so if the PA
> space is very large (and memory hotplug is enabled), no randomization
> is possible, as it may end up mapping RAM that appears later past the
> top of the VA space. Note that the same is fundamentally true for
> running a 39-bit VA kernel on hardware that has a PA range that
> exceeds that.
> 
> As for the signed arithmetic: the randomization works by substracting
> from memstart_addr (aka PHYS_OFFSET). Normally, if system RAM starts
> at 1G, memstart_addr will be 1G, and the linear map will associate
> PAGE_OFFSET with PA 1G.
> 
> To randomize the linear mapping, the system RAM has to be moved up in
> the linear map (as this is the only direction it can be moved), and so
> PAGE_OFFSET needs to map to a lower value, and so memstart_addr may
> wrap around and become negative.
> 
>> (I
>> think this assumes that VA size is always GTE PA size). But if linear map size
>> is based on an overridden VA and overridden PA size (which I assume it must be,
>> given the pgtables will be limitted to the overridden VA size) and PA size is
>> not overridden, isn't it possible to wrongly conclude that there is enough room
>> to randomize when there really is not?
>>
> 
> No, if the linear map is only 47 bits, and the PA space is either 52
> or 48 bits, no randomization is possible. We might even run out of
> space without randomization, but this is an existing problem for
> non-LPA2 too.

Ahh that all makes much more sense now! I'm happy to go with the patch Will
proposed.

Thanks,
Ryan

Powered by blists - more mailing lists