Re: [PATCH] LoongArch: Support mem=SIZE kernel parameter

[Yanteng Si <si.yanteng@linux.dev>]

在 7/3/25 9:36 AM, Ming Wang 写道:
> Hi Yanteng,
>
> Thanks for reviewing the patch and for your insightful question.
>
> On 7/2/25 10:11, Yanteng Si wrote:
>> 在 7/1/25 5:04 PM, Ming Wang 写道:
>>> The LoongArch mem= parameter parser was previously limited to the
>>> mem=SIZE@START format. This was inconvenient for the common use case
>>> of simply capping the total system memory, as it forced users to
>>> manually specify a start address. It was also inconsistent with the
>>> behavior on other architectures.
>>>
>>> This patch enhances the parser in early_parse_mem() to also support the
>>> more user-friendly mem=SIZE format. The implementation now checks for
>>> the presence of the '@' symbol to determine the user's intent:
>>>
>>> - If mem=SIZE is provided (no '@'), the kernel now calls
>>>    memblock_enforce_memory_limit(). This trims memory from the top down
>>>    to the specified size.
>>> - If mem=SIZE@START is used, the original behavior is retained for
>>>    backward compatibility. This allows for defining specific memory
>>>    banks.
>>>
>>> This change introduces an important usage rule reflected in the code's
>>> comments: the mem=SIZE format should only be specified once on the
>>> kernel command line. It acts as a single, global cap on total memory. In
>>> contrast, the mem=SIZE@START format can be used multiple times to
>>> define several distinct memory regions.
>>>
>>> Signed-off-by: Ming Wang <wangming01@loongson.cn>
>>> ---
>>>   arch/loongarch/kernel/setup.c | 18 ++++++++++--------
>>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/ setup.c
>>> index b99fbb388fe0..af59ba180dc2 100644
>>> --- a/arch/loongarch/kernel/setup.c
>>> +++ b/arch/loongarch/kernel/setup.c
>>> @@ -191,6 +191,16 @@ static int __init early_parse_mem(char *p)
>>>           return -EINVAL;
>>>       }
>>> +    start = 0;
>>> +    size = memparse(p, &p);
>>> +    if (*p == '@')    /* Every mem=... should contain '@' */
>>> +        start = memparse(p + 1, &p);
>>> +    else {            /* Only one mem=... is allowed if no '@' */
>>> +        usermem = 1;
>>> +        memblock_enforce_memory_limit(size);
>>> +        return 0;
>>> +    }
>>> +
>>>       /*
>>>        * If a user specifies memory size, we
>>>        * blow away any automatically generated
>>> @@ -201,14 +211,6 @@ static int __init early_parse_mem(char *p)
>>>           memblock_remove(memblock_start_of_DRAM(),
>>>               memblock_end_of_DRAM() - memblock_start_of_DRAM());
>>>       }
>>> -    start = 0;
>>> -    size = memparse(p, &p);
>>> -    if (*p == '@')
>>> -        start = memparse(p + 1, &p);
>>> -    else {
>>> -        pr_err("Invalid format!\n");
>>> -        return -EINVAL;
>>> -    }
>> I don't understand. Isn't it better to modify the else{} directly here?
>>
> You've raised a very good point. The reason for moving the parsing logic to the top, rather than just modifying the original else block, is to handle the fundamentally different behaviors required for mem=SIZE versus mem=SIZE@START. The key lies in thisexisting block of code which handles the mem=SIZE@START case:
>
> ```
> /*
> * If a user specifies memory size, we
> * blow away any automatically generated
> * size.
> */
> if (usermem == 0) {
>     usermem = 1;
>     memblock_remove(memblock_start_of_DRAM(),
>             memblock_end_of_DRAM() - memblock_start_of_DRAM());
> }
> ```
>
> This code is destructive. As the comment says, it "blows away" the entire memory map discovered from the firmware (UEFI/ACPI). After this call, memblock is essentially empty, waiting for new regions to be added via memblock_add(). This is the correct behavior for mem=SIZE@START.
>
> However, the new mem=SIZE functionality is meant to be non-destructive. It should take the existing firmware-provided memory map and simply trim it down to the desired size. The function memblock_enforce_memory_limit() is designed for this purpose—it operates on the current state of memblock.
>
> If we were to keep the parsing logic at the end and only modify the else block, the destructive memblock_remove() call would have already executed for both cases. By that point, calling memblock_enforce_memory_limit() would be meaningless, as there would be no memory regions left in memblock to limit.
>
> Therefore, the patch moves the parsing logic to the very beginning to create a clean separation:
> 1. It first checks if the format is mem=SIZE (no '@').
> 2. If it is, it performs the non-destructive limit on the intact memory map and returns immediately, completely bypassing the destructive memblock_remove() logic.
> 3. If the format is mem=SIZE@START, it falls through to the original destructive path as before.

I have an idea: what if we move the destructive code into the if block?


Thanks,

Yanteng

>
> I hope this explanation clarifies why the code structure was changed this way. It's crucial to ensure the non-destructive path is handled before any memory map information is lost.
>
> Best regards,
> Ming
>
>
>> Thanks,
>> Yanteng
>>
>