From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4CC29C83001 for ; Thu, 30 Apr 2020 08:13:59 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5039B2073E for ; Thu, 30 Apr 2020 08:13:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.b="aRdqN+ob" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5039B2073E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 49CSnw06c8zDrDl for ; Thu, 30 Apr 2020 18:13:56 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=intel.com (client-ip=2a00:1450:4864:20::641; helo=mail-ej1-x641.google.com; envelope-from=dan.j.williams@intel.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.a=rsa-sha256 header.s=20150623 header.b=aRdqN+ob; dkim-atps=neutral Received: from mail-ej1-x641.google.com (mail-ej1-x641.google.com [IPv6:2a00:1450:4864:20::641]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 49CSm06B0NzDr7s for ; Thu, 30 Apr 2020 18:12:09 +1000 (AEST) Received: by mail-ej1-x641.google.com with SMTP id rh22so3911759ejb.12 for ; Thu, 30 Apr 2020 01:12:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=aPyH7cBulXkr69UeTYsLw8J8jsMaQ8KU1AV45QxO0GM=; b=aRdqN+obvpVV2A+fgMLJy4RekybxRVDiG7lp8OF4bnZx2Ywvd3iCU/W+kNS4ZeZMFO s2KHcqmsaIITIji0juOY7H6rW7Fc6dWrzMOI3EIosswh9jq3YYccKaX2xhcXTvz1nd5V aBV+pihhUiUdyrUO5BWYQElkaX6/osTUoFcDUd3gIMpaDgbgaWZ7XiKQfHXqUsjsTevr xcbBoOAzO8ZeJrHgjs2FlN5xkisQM2/NaUdROIGlFFS2pMP9xFv8klIZrmKxkYMDnngC ZkScurbP2qpL3FH2QS0j799ldY6mMF4VeS1bCDMrE9KedwDuMF2flKOw3iR6FucwjAe8 uO/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=aPyH7cBulXkr69UeTYsLw8J8jsMaQ8KU1AV45QxO0GM=; b=RQhio8HZZkPuExQNfxP9sHFQtvKyVIpaHfnmxH40/7FM9aBN3KKG9OYsJI1lGveu8s USbAtlZJhAQS9LC+NOoqpY99ISmV4dMQrLrTSzSKGA2TjICqx5fmDeB6B7ibFfwowQ/r Ct/fcr1wDgcHL9RIBpdmCCZs66qQ2oT1UXxQzDnqk2gZro6JSJ3Ju/kR7X9U7ud/S7mc pGib0X5bXMYazJEtlUqCD2qSNmlgGueynT9Ev9Y+r8HZUejvTnUHTzpYr+37PlrXnf1s /AIKRhm0YbL8MscApah7RxvphZZudLugpqQVMm6KMEw/ex6Z1n8b/K5xKc8sILvy+smM IdnQ== X-Gm-Message-State: AGi0PuY9/oLz9dFnULxHH5d06mPgblsKE4mXXjrVXFIQkwpurzmFeK8O Pc9i4D5EPw6xqGPpeQlkoNQutET8An/h3pZ15j1uTg== X-Google-Smtp-Source: APiQypKwCBG4S6M4oBR8TWLxl5ZHi3zkP+L9vXXaCbuE040rlsosdGPooBbFU8EQOuJGeo1PTMYPgjG4Ewt4MDGe5kI= X-Received: by 2002:a17:906:7750:: with SMTP id o16mr1646531ejn.12.1588234325407; Thu, 30 Apr 2020 01:12:05 -0700 (PDT) MIME-Version: 1.0 References: <20200429160803.109056-1-david@redhat.com> <20200429160803.109056-3-david@redhat.com> In-Reply-To: From: Dan Williams Date: Thu, 30 Apr 2020 01:11:54 -0700 Message-ID: Subject: Re: [PATCH v1 2/3] mm/memory_hotplug: Introduce MHP_DRIVER_MANAGED To: David Hildenbrand Content-Type: text/plain; charset="UTF-8" X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: virtio-dev@lists.oasis-open.org, linux-hyperv@vger.kernel.org, Michal Hocko , Pavel Tatashin , Baoquan He , Linux MM , Wei Yang , linux-s390 , linux-nvdimm , Linux Kernel Mailing List , Dave Hansen , virtualization@lists.linux-foundation.org, Linux ACPI , "Michael S . Tsirkin" , Eric Biederman , Pankaj Gupta , xen-devel , Andrew Morton , Michal Hocko , linuxppc-dev Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Thu, Apr 30, 2020 at 12:20 AM David Hildenbrand wrote: > > On 29.04.20 18:08, David Hildenbrand wrote: > > Some paravirtualized devices that add memory via add_memory() and > > friends (esp. virtio-mem) don't want to create entries in > > /sys/firmware/memmap/ - primarily to hinder kexec from adding this > > memory to the boot memmap of the kexec kernel. > > > > In fact, such memory is never exposed via the firmware (e.g., e820), but > > only via the device, so exposing this memory via /sys/firmware/memmap/ is > > wrong: > > "kexec needs the raw firmware-provided memory map to setup the > > parameter segment of the kernel that should be booted with > > kexec. Also, the raw memory map is useful for debugging. For > > that reason, /sys/firmware/memmap is an interface that provides > > the raw memory map to userspace." [1] > > > > We want to let user space know that memory which is always detected, > > added, and managed via a (device) driver - like memory managed by > > virtio-mem - is special. It cannot be used for placing kexec segments > > and the (device) driver is responsible for re-adding memory that > > (eventually shrunk/grown/defragmented) memory after a reboot/kexec. It > > should e.g., not be added to a fixed up firmware memmap. However, it should > > be dumped by kdump. > > > > Also, such memory could behave differently than an ordinary DIMM - e.g., > > memory managed by virtio-mem can have holes inside added memory resource, > > which should not be touched, especially for writing. > > > > Let's expose that memory as "System RAM (driver managed)" e.g., via > > /pro/iomem. > > > > We don't have to worry about firmware_map_remove() on the removal path. > > If there is no entry, it will simply return with -EINVAL. > > > > [1] https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-firmware-memmap > > > > Cc: Andrew Morton > > Cc: Michal Hocko > > Cc: Pankaj Gupta > > Cc: Wei Yang > > Cc: Baoquan He > > Cc: Eric Biederman > > Signed-off-by: David Hildenbrand > > --- > > include/linux/memory_hotplug.h | 8 ++++++++ > > mm/memory_hotplug.c | 20 ++++++++++++++++---- > > 2 files changed, 24 insertions(+), 4 deletions(-) > > > > diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h > > index bf0e3edb8688..cc538584b39e 100644 > > --- a/include/linux/memory_hotplug.h > > +++ b/include/linux/memory_hotplug.h > > @@ -68,6 +68,14 @@ struct mhp_params { > > pgprot_t pgprot; > > }; > > > > +/* Flags used for add_memory() and friends. */ > > + > > +/* > > + * Don't create entries in /sys/firmware/memmap/ and expose memory as > > + * "System RAM (driver managed)" in e.g., /proc/iomem > > + */ > > +#define MHP_DRIVER_MANAGED 1 > > + > > /* > > * Zone resizing functions > > * > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > > index ebdf6541d074..cfa0721280aa 100644 > > --- a/mm/memory_hotplug.c > > +++ b/mm/memory_hotplug.c > > @@ -98,11 +98,11 @@ void mem_hotplug_done(void) > > u64 max_mem_size = U64_MAX; > > > > /* add this memory to iomem resource */ > > -static struct resource *register_memory_resource(u64 start, u64 size) > > +static struct resource *register_memory_resource(u64 start, u64 size, > > + const char *resource_name) > > { > > struct resource *res; > > unsigned long flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > > - char *resource_name = "System RAM"; > > > > /* > > * Make sure value parsed from 'mem=' only restricts memory adding > > @@ -1058,7 +1058,8 @@ int __ref add_memory_resource(int nid, struct resource *res, > > BUG_ON(ret); > > > > /* create new memmap entry */ > > - firmware_map_add_hotplug(start, start + size, "System RAM"); > > + if (!(flags & MHP_DRIVER_MANAGED)) > > + firmware_map_add_hotplug(start, start + size, "System RAM"); > > > > /* device_online() will take the lock when calling online_pages() */ > > mem_hotplug_done(); > > @@ -1081,10 +1082,21 @@ int __ref add_memory_resource(int nid, struct resource *res, > > /* requires device_hotplug_lock, see add_memory_resource() */ > > int __ref __add_memory(int nid, u64 start, u64 size, unsigned long flags) > > { > > + const char *resource_name = "System RAM"; > > struct resource *res; > > int ret; > > > > - res = register_memory_resource(start, size); > > + /* > > + * Indicate that memory managed by a driver is special. It's always > > + * detected and added via a driver, should not be given to the kexec > > + * kernel for booting when manually crafting the firmware memmap, and > > + * no kexec segments should be placed on it. However, kdump should > > + * dump this memory. > > + */ > > + if (flags & MHP_DRIVER_MANAGED) > > + resource_name = "System RAM (driver managed)"; > > + > > + res = register_memory_resource(start, size, resource_name); > > if (IS_ERR(res)) > > return PTR_ERR(res); > > > > > > BTW, I was wondering if this is actually also something that > drivers/dax/kmem.c wants to use for adding memory. > > Just because we decided to use some DAX memory in the current kernel as > system ram, doesn't mean we should make that decision for the kexec > kernel (e.g., using it as initial memory, placing kexec binaries onto > it, etc.). This is also not what we would observe during a real reboot. Agree. > I can see that the "System RAM" resource will show up as child resource > under the device e.g., in /proc/iomem. > > However, entries in /sys/firmware/memmap/ are created as "System RAM". True. Do you think this rename should just be limited to what type /sys/firmware/memmap/ emits? I have the concern, but no proof currently, that there are /proc/iomem walkers that explicitly look for "System RAM", but might be thrown off by "System RAM (driver managed)". I was not aware of /sys/firmware/memmap until about 5 minutes ago.