linux-kernel - Re: [PATCH 03/21] x86/mm: factor out phys_pgd

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] [day] [month] [year] [list]

Message-ID: <DDSLXKU87HTE.G0XUZ5BG5M8K@google.com>
Date: Sun, 26 Oct 2025 22:29:23 +0000
From: Brendan Jackman <jackmanb@...gle.com>
To: Borislav Petkov <bp@...en8.de>, Brendan Jackman <jackmanb@...gle.com>
Cc: Andy Lutomirski <luto@...nel.org>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, 
	"Liam R. Howlett" <Liam.Howlett@...cle.com>, Suren Baghdasaryan <surenb@...gle.com>, 
	Michal Hocko <mhocko@...e.com>, Johannes Weiner <hannes@...xchg.org>, Zi Yan <ziy@...dia.com>, 
	Axel Rasmussen <axelrasmussen@...gle.com>, Yuanchu Xie <yuanchu@...gle.com>, 
	Roman Gushchin <roman.gushchin@...ux.dev>, <peterz@...radead.org>, 
	<dave.hansen@...ux.intel.com>, <mingo@...hat.com>, <tglx@...utronix.de>, 
	<akpm@...ux-foundation.org>, <david@...hat.com>, <derkling@...gle.com>, 
	<junaids@...gle.com>, <linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>, 
	<reijiw@...gle.com>, <rientjes@...gle.com>, <rppt@...nel.org>, 
	<vbabka@...e.cz>, <x86@...nel.org>, <yosry.ahmed@...ux.dev>
Subject: Re: [PATCH 03/21] x86/mm: factor out phys_pgd_init()

On Sat Oct 25, 2025 at 11:48 AM UTC, Borislav Petkov wrote:
> On Wed, Sep 24, 2025 at 02:59:38PM +0000, Brendan Jackman wrote:
>> +static unsigned long __meminit
>> +__kernel_physical_mapping_init(unsigned long paddr_start,
>> +			       unsigned long paddr_end,
>> +			       unsigned long page_size_mask,
>> +			       pgprot_t prot, bool init)
>> +{
>> +	bool pgd_changed;
>
> I have to say, that pgd_changed is yuck but I don't have a better idea and
> this has happened a long time ago anyway.
>
> How about you have the caller pass in false:
>
> 	bool pgd_changed = false;
>
> and then callee sets it to true when it does so?

Sure.

Per Dave's feedback I am still slightly hopeful I can find a way to
come in and refactor this code so that it's gets cleaner for you guys
and then ASI becomes a natural addition. So far I don't come up with
anything in init_64.c but I'm still planning to stare at set_memory.c a
while longer and see if anything comes to mind. So maybe we'll be able
to reduce the yuck factor a bit.

>> +	unsigned long paddr_last;
>
> The tip-tree preferred ordering of variable declarations at the
> beginning of a function is reverse fir tree order::
>
> 	struct long_struct_name *descriptive_name;
> 	unsigned long foo, bar;
> 	unsigned int tmp;
> 	int ret;
>
> The above is faster to parse than the reverse ordering::
>
> 	int ret;
> 	unsigned int tmp;
> 	unsigned long foo, bar;
> 	struct long_struct_name *descriptive_name;
>
> And even more so than random ordering::
>
> 	unsigned long foo, bar;
> 	int ret;
> 	struct long_struct_name *descriptive_name;
> 	unsigned int tmp;

Ack

>> +
>> +	paddr_last = phys_pgd_init(init_mm.pgd, paddr_start, paddr_end, page_size_mask,
>> +				   prot, init, &pgd_changed);
>> +	if (pgd_changed)
>> +		sync_global_pgds((unsigned long)__va(paddr_start),
>> +				 (unsigned long)__va(paddr_end) - 1);
>> +
>> +	return paddr_last;
>> +}
>>  
>>  /*
>>   * Create page table mapping for the physical memory for specific physical
>> 
>> -- 
>> 2.50.1
>>