linux-kernel - Re: [RFC PATCH v3 1/6] arch: introduce set_direct_map_valid

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: <440b2682-dbfb-4a5c-927c-2397413a7f9c@suse.cz>
Date: Mon, 11 Nov 2024 13:12:27 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: David Hildenbrand <david@...hat.com>, Patrick Roy <roypat@...zon.co.uk>,
 tabba@...gle.com, quic_eberman@...cinc.com, seanjc@...gle.com,
 pbonzini@...hat.com, jthoughton@...gle.com, ackerleytng@...gle.com,
 vannapurve@...gle.com, rppt@...nel.org
Cc: graf@...zon.com, jgowans@...zon.com, derekmn@...zon.com,
 kalyazin@...zon.com, xmarcalx@...zon.com, linux-mm@...ck.org,
 corbet@....net, catalin.marinas@....com, will@...nel.org,
 chenhuacai@...nel.org, kernel@...0n.name, paul.walmsley@...ive.com,
 palmer@...belt.com, aou@...s.berkeley.edu, hca@...ux.ibm.com,
 gor@...ux.ibm.com, agordeev@...ux.ibm.com, borntraeger@...ux.ibm.com,
 svens@...ux.ibm.com, gerald.schaefer@...ux.ibm.com, tglx@...utronix.de,
 mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
 hpa@...or.com, luto@...nel.org, peterz@...radead.org, rostedt@...dmis.org,
 mhiramat@...nel.org, mathieu.desnoyers@...icios.com, shuah@...nel.org,
 kvm@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 loongarch@...ts.linux.dev, linux-riscv@...ts.infradead.org,
 linux-s390@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 linux-kselftest@...r.kernel.org
Subject: Re: [RFC PATCH v3 1/6] arch: introduce set_direct_map_valid_noflush()

On 10/31/24 10:57, David Hildenbrand wrote:
> On 30.10.24 14:49, Patrick Roy wrote:
>> From: "Mike Rapoport (Microsoft)" <rppt@...nel.org>
>> 
>> From: Mike Rapoport (Microsoft) <rppt@...nel.org>
>> 
>> Add an API that will allow updates of the direct/linear map for a set of
>> physically contiguous pages.
>> 
>> It will be used in the following patches.
>> 
>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@...nel.org>
>> Signed-off-by: Patrick Roy <roypat@...zon.co.uk>
> 
> 
> [...]
> 
>>   #ifdef CONFIG_DEBUG_PAGEALLOC
>>   void __kernel_map_pages(struct page *page, int numpages, int enable)
>>   {
>> diff --git a/include/linux/set_memory.h b/include/linux/set_memory.h
>> index e7aec20fb44f1..3030d9245f5ac 100644
>> --- a/include/linux/set_memory.h
>> +++ b/include/linux/set_memory.h
>> @@ -34,6 +34,12 @@ static inline int set_direct_map_default_noflush(struct page *page)
>>   	return 0;
>>   }
>>   
>> +static inline int set_direct_map_valid_noflush(struct page *page,
>> +					       unsigned nr, bool valid)
> 
> I recall that "unsigned" is frowned upon; "unsigned int".
> 
>> +{
>> +	return 0;
>> +}
> 
> Can we add some kernel doc for this?
> 
> In particular
> 
> (a) What does it mean when we return 0? That it worked? Then, this

Seems so.

>      dummy function looks wrong. Or this it return the

That's !CONFIG_ARCH_HAS_SET_DIRECT_MAP and other functions around do it the
same way. Looks like the current callers can only exist with the CONFIG_
enabled in the first place.

>      number of processed entries? Then we'd have a possible "int" vs.
>      "unsigned int" inconsistency.
> 
> (b) What are the semantics when we fail halfway through the operation
>      when processing nr > 1? Is it "all or nothing"?

Looking at x86 implementation it seems like it can just bail out in the
middle, but then I'm not sure if it can really fail in the middle, hmm...