linux-kernel - Re: [PATCH v3 4/5] s390/mm: implement MEM_PREPARE_ONLINE/MEM_FINISH

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: <18d06a3d-b75f-4ac9-8791-8391a3f60575@redhat.com>
Date:   Mon, 27 Nov 2023 17:58:51 +0100
From:   David Hildenbrand <david@...hat.com>
To:     Sumanth Korikkar <sumanthk@...ux.ibm.com>
Cc:     linux-mm <linux-mm@...ck.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Oscar Salvador <osalvador@...e.de>,
        Michal Hocko <mhocko@...e.com>,
        "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>,
        Anshuman Khandual <anshuman.khandual@....com>,
        Gerald Schaefer <gerald.schaefer@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        linux-s390 <linux-s390@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 4/5] s390/mm: implement
 MEM_PREPARE_ONLINE/MEM_FINISH_OFFLINE notifiers

On 27.11.23 17:12, Sumanth Korikkar wrote:
> On Mon, Nov 27, 2023 at 04:11:05PM +0100, David Hildenbrand wrote:
>>> diff --git a/drivers/s390/char/sclp_cmd.c b/drivers/s390/char/sclp_cmd.c
>>> index 355e63e44e95..30b829e4c052 100644
>>> --- a/drivers/s390/char/sclp_cmd.c
>>> +++ b/drivers/s390/char/sclp_cmd.c
>>> @@ -18,6 +18,7 @@
>>>    #include <linux/mm.h>
>>>    #include <linux/mmzone.h>
>>>    #include <linux/memory.h>
>>> +#include <linux/memory_hotplug.h>
>>>    #include <linux/module.h>
>>>    #include <asm/ctlreg.h>
>>>    #include <asm/chpid.h>
>>> @@ -26,6 +27,7 @@
>>>    #include <asm/sclp.h>
>>>    #include <asm/numa.h>
>>>    #include <asm/facility.h>
>>> +#include <asm/page-states.h>
>>>    #include "sclp.h"
>>> @@ -319,6 +321,7 @@ static bool contains_standby_increment(unsigned long start, unsigned long end)
>>>    static int sclp_mem_notifier(struct notifier_block *nb,
>>>    			     unsigned long action, void *data)
>>>    {
>>> +	struct memory_block *memory_block;
>>>    	unsigned long start, size;
>>>    	struct memory_notify *arg;
>>>    	unsigned char id;
>>> @@ -340,18 +343,29 @@ static int sclp_mem_notifier(struct notifier_block *nb,
>>>    		if (contains_standby_increment(start, start + size))
>>>    			rc = -EPERM;
>>>    		break;
>>> -	case MEM_GOING_ONLINE:
>>> +	case MEM_PREPARE_ONLINE:
>>> +		memory_block = find_memory_block(pfn_to_section_nr(arg->start_pfn));
>>> +		if (!memory_block) {
>>> +			rc = -EINVAL;
>>> +			goto out;
>>> +		}
>>>    		rc = sclp_mem_change_state(start, size, 1);
>>> +		if (rc || !memory_block->altmap)
>>> +			goto out;
>>> +		/*
>>> +		 * Set CMMA state to nodat here, since the struct page memory
>>> +		 * at the beginning of the memory block will not go through the
>>> +		 * buddy allocator later.
>>> +		 */
>>> +		__arch_set_page_nodat((void *)__va(start), memory_block->altmap->free);
>>
>> Looking up the memory block and grabbing the altmap from there is a bit
>> unfortunate.
>>
>> Why can't we do that when adding the altmap? Will the hypervisor scream at
>> us?
>>
> calling __arch_set_page_nodat() before making memory block accessible
> will lead to crash. Hence, we think this is the only safe location to
> place it.
> 
>> ... would we want to communicate any altmap start+size via the memory
>> notifier instead?
> 
> Passing start, size  of memory range via memory notifier looks correct
> approach to me, as we try to make the specified range accessible.
> 
> If we want to pass altmap size (nr_vmemmap_pages), then we might need a
> new field in struct memory_notify, which would prevent access of
> memory_block->altmap->free in the notifier.
> 
> Do you want to take this approach instead?
> 
> If yes, Then I could add a new field nr_vmemmap_pages in struct
> memory_notify and place it in PATCH : introduce
> MEM_PREPARE_ONLINE/MEM_FINISH_OFFLINE notifiers.

Yes, see my other mail. That's probably cleanest!

-- 
Cheers,

David / dhildenb