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=-9.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,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 6DA9CC56202 for ; Wed, 21 Oct 2020 18:59:54 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 88D3C2416E for ; Wed, 21 Oct 2020 18:59:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.b="LlHb740U" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 88D3C2416E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A83EF6B005D; Wed, 21 Oct 2020 14:59:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A5B3E6B0062; Wed, 21 Oct 2020 14:59:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9734E6B0068; Wed, 21 Oct 2020 14:59:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0059.hostedemail.com [216.40.44.59]) by kanga.kvack.org (Postfix) with ESMTP id 6983E6B005D for ; Wed, 21 Oct 2020 14:59:52 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 0BD5A1CDA for ; Wed, 21 Oct 2020 18:59:52 +0000 (UTC) X-FDA: 77396847024.01.talk85_1c09a462724a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin01.hostedemail.com (Postfix) with ESMTP id CDA641004C7B4 for ; Wed, 21 Oct 2020 18:59:51 +0000 (UTC) X-HE-Tag: talk85_1c09a462724a X-Filterd-Recvd-Size: 14959 Received: from mail-ed1-f68.google.com (mail-ed1-f68.google.com [209.85.208.68]) by imf40.hostedemail.com (Postfix) with ESMTP for ; Wed, 21 Oct 2020 18:59:50 +0000 (UTC) Received: by mail-ed1-f68.google.com with SMTP id cq12so3676406edb.2 for ; Wed, 21 Oct 2020 11:59:50 -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:content-transfer-encoding; bh=GDRf8ABUu+TZaWK0lYE+MAlDPbiqs8UpaJTToDFItts=; b=LlHb740UPxtxMP1zvZIGPA/lPoD9hh7cNYpX8bNSMxy2cki0zar6/kZRNh0S/E0qPa ZpW4b/leM0+bY946rUTzSmq/7g3ri2RkNzbtHgOg4PsM+KUBcOCcVeKNeanRm84wcu/W XOLQ77AHYidJJUWKsJxnqTUekRBiQRbwXSBG2jlE4qdoEJfy409qyTvxRYnPU5bIQb+U bf906IePyxRHDp6qAfLZ/tVAqrc0wSIsu+4XCZIej2gYYhK1Kj6IeIArk+F37RNo6Hm5 mBLTs8l1AL21QQMBFirH7aP1YUylC1F8+nuxuBwX/Huc7Cd864psezEXx9wpo6JKELw9 5LxA== 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:content-transfer-encoding; bh=GDRf8ABUu+TZaWK0lYE+MAlDPbiqs8UpaJTToDFItts=; b=fI6xKUoG9OtpY9SwarzrZxyYpaKiDMIYayigooNqAf/ymEYYOPxIr+0nqoVrX7JKA2 dKJbMHmWhANN1q7Zlf9stOzfMhM7rgxOrEEXxR0Uady7DVAbqfIy0d6VDAU6na/rojWN AF0Vj8LLNNpdjzRLX6Ve25ul7DN8wAnLHka8dQsas8UKtmWSu5OMEYidX3yoFUkroQow /wHlD3K4CXGDPhTybxL9XKsSmb40sithcinM/VvsjIdKtFg54267ovsxQjs2AQ04xN7Q I31JrhdXpabtPLahNWdiwHU0lxjWHyOuD45GPnLFphax14z1TO1WzJxlIoQuO6eh5vLW cotw== X-Gm-Message-State: AOAM533L4TrB1SxczU2lptFDhazUiYWBb12yq1x8hIBkbKVKzaYQB7eh v0AUSCodr/wtHOWuICSnbqp57Cffw3XM1PpmJ3upHw== X-Google-Smtp-Source: ABdhPJxkwinHUuUKpXZZrs2mV0S9ZFr8FhcGow++fntkKzEh91M9Md+jRtPcnjhtSGe98DL/L2wAwzmghcS9V3WHrNo= X-Received: by 2002:aa7:cd1a:: with SMTP id b26mr4390509edw.97.1603306788939; Wed, 21 Oct 2020 11:59:48 -0700 (PDT) MIME-Version: 1.0 References: <20201021085655.1192025-1-daniel.vetter@ffwll.ch> <20201021085655.1192025-15-daniel.vetter@ffwll.ch> In-Reply-To: <20201021085655.1192025-15-daniel.vetter@ffwll.ch> From: Dan Williams Date: Wed, 21 Oct 2020 11:59:37 -0700 Message-ID: Subject: Re: [PATCH v3 14/16] resource: Move devmem revoke code to resource framework To: Daniel Vetter Cc: DRI Development , LKML , KVM list , Linux MM , Linux ARM , linux-samsung-soc , "Linux-media@vger.kernel.org" , linux-s390 , Greg Kroah-Hartman , Daniel Vetter , Jason Gunthorpe , Kees Cook , Andrew Morton , John Hubbard , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Jan Kara , Arnd Bergmann , David Hildenbrand , "Rafael J. Wysocki" , Daniel Vetter Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Oct 21, 2020 at 1:57 AM Daniel Vetter wrot= e: > > We want all iomem mmaps to consistently revoke ptes when the kernel > takes over and CONFIG_IO_STRICT_DEVMEM is enabled. This includes the > pci bar mmaps available through procfs and sysfs, which currently do > not revoke mappings. > > To prepare for this, move the code from the /dev/kmem driver to > kernel/resource.c. > > Reviewed-by: Greg Kroah-Hartman > Signed-off-by: Daniel Vetter > Cc: Jason Gunthorpe > Cc: Kees Cook > Cc: Dan Williams > Cc: Andrew Morton > Cc: John Hubbard > Cc: J=C3=A9r=C3=B4me Glisse > Cc: Jan Kara > Cc: Dan Williams > Cc: linux-mm@kvack.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-samsung-soc@vger.kernel.org > Cc: linux-media@vger.kernel.org > Cc: Arnd Bergmann > Cc: Greg Kroah-Hartman > Cc: Daniel Vetter > Cc: David Hildenbrand > Cc: "Rafael J. Wysocki" > Signed-off-by: Daniel Vetter > -- > v3: > - add barrier for consistency and document why we don't have to check > for NULL (Jason) > --- > drivers/char/mem.c | 85 +--------------------------------- > include/linux/ioport.h | 6 +-- > kernel/resource.c | 101 ++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 102 insertions(+), 90 deletions(-) > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > index 5502f56f3655..53338aad8d28 100644 > --- a/drivers/char/mem.c > +++ b/drivers/char/mem.c > @@ -31,9 +31,6 @@ > #include > #include > #include > -#include > -#include > -#include > > #ifdef CONFIG_IA64 > # include > @@ -809,42 +806,6 @@ static loff_t memory_lseek(struct file *file, loff_t= offset, int orig) > return ret; > } > > -static struct inode *devmem_inode; > - > -#ifdef CONFIG_IO_STRICT_DEVMEM > -void revoke_devmem(struct resource *res) > -{ > - /* pairs with smp_store_release() in devmem_init_inode() */ > - struct inode *inode =3D smp_load_acquire(&devmem_inode); > - > - /* > - * Check that the initialization has completed. Losing the race > - * is ok because it means drivers are claiming resources before > - * the fs_initcall level of init and prevent /dev/mem from > - * establishing mappings. > - */ > - if (!inode) > - return; > - > - /* > - * The expectation is that the driver has successfully marked > - * the resource busy by this point, so devmem_is_allowed() > - * should start returning false, however for performance this > - * does not iterate the entire resource range. > - */ > - if (devmem_is_allowed(PHYS_PFN(res->start)) && > - devmem_is_allowed(PHYS_PFN(res->end))) { > - /* > - * *cringe* iomem=3Drelaxed says "go ahead, what's the > - * worst that can happen?" > - */ > - return; > - } > - > - unmap_mapping_range(inode->i_mapping, res->start, resource_size(r= es), 1); > -} > -#endif > - > static int open_port(struct inode *inode, struct file *filp) > { > int rc; > @@ -864,7 +825,7 @@ static int open_port(struct inode *inode, struct file= *filp) > * revocations when drivers want to take over a /dev/mem mapped > * range. > */ > - filp->f_mapping =3D inode->i_mapping; > + filp->f_mapping =3D iomem_get_mapping(); > > return 0; > } > @@ -995,48 +956,6 @@ static char *mem_devnode(struct device *dev, umode_t= *mode) > > static struct class *mem_class; > > -static int devmem_fs_init_fs_context(struct fs_context *fc) > -{ > - return init_pseudo(fc, DEVMEM_MAGIC) ? 0 : -ENOMEM; > -} > - > -static struct file_system_type devmem_fs_type =3D { > - .name =3D "devmem", > - .owner =3D THIS_MODULE, > - .init_fs_context =3D devmem_fs_init_fs_context, > - .kill_sb =3D kill_anon_super, > -}; > - > -static int devmem_init_inode(void) > -{ > - static struct vfsmount *devmem_vfs_mount; > - static int devmem_fs_cnt; > - struct inode *inode; > - int rc; > - > - rc =3D simple_pin_fs(&devmem_fs_type, &devmem_vfs_mount, &devmem_= fs_cnt); > - if (rc < 0) { > - pr_err("Cannot mount /dev/mem pseudo filesystem: %d\n", r= c); > - return rc; > - } > - > - inode =3D alloc_anon_inode(devmem_vfs_mount->mnt_sb); > - if (IS_ERR(inode)) { > - rc =3D PTR_ERR(inode); > - pr_err("Cannot allocate inode for /dev/mem: %d\n", rc); > - simple_release_fs(&devmem_vfs_mount, &devmem_fs_cnt); > - return rc; > - } > - > - /* > - * Publish /dev/mem initialized. > - * Pairs with smp_load_acquire() in revoke_devmem(). > - */ > - smp_store_release(&devmem_inode, inode); > - > - return 0; > -} > - > static int __init chr_dev_init(void) > { > int minor; > @@ -1058,8 +977,6 @@ static int __init chr_dev_init(void) > */ > if ((minor =3D=3D DEVPORT_MINOR) && !arch_has_dev_port()) > continue; > - if ((minor =3D=3D DEVMEM_MINOR) && devmem_init_inode() != =3D 0) > - continue; > > device_create(mem_class, NULL, MKDEV(MEM_MAJOR, minor), > NULL, devlist[minor].name); > diff --git a/include/linux/ioport.h b/include/linux/ioport.h > index 6c2b06fe8beb..8ffb61b36606 100644 > --- a/include/linux/ioport.h > +++ b/include/linux/ioport.h > @@ -302,11 +302,7 @@ struct resource *devm_request_free_mem_region(struct= device *dev, > struct resource *request_free_mem_region(struct resource *base, > unsigned long size, const char *name); > > -#ifdef CONFIG_IO_STRICT_DEVMEM > -void revoke_devmem(struct resource *res); > -#else > -static inline void revoke_devmem(struct resource *res) { }; > -#endif > +extern struct address_space *iomem_get_mapping(void); > > #endif /* __ASSEMBLY__ */ > #endif /* _LINUX_IOPORT_H */ > diff --git a/kernel/resource.c b/kernel/resource.c > index 841737bbda9e..a92fed5b9997 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -18,12 +18,15 @@ > #include > #include > #include > +#include > #include > #include > #include > #include > #include > +#include > #include > +#include > #include > > > @@ -1112,6 +1115,58 @@ resource_size_t resource_alignment(struct resource= *res) > > static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait); > > +static struct inode *iomem_inode; > + > +#ifdef CONFIG_IO_STRICT_DEVMEM > +static void revoke_iomem(struct resource *res) > +{ > + /* pairs with smp_store_release() in iomem_init_inode() */ > + struct inode *inode =3D smp_load_acquire(&iomem_inode); > + > + /* > + * Check that the initialization has completed. Losing the race > + * is ok because it means drivers are claiming resources before > + * the fs_initcall level of init and prevent /dev/mem from How about: s,/dev/mem,iomem_get_mapping() users, ...now that this facility is generalized? > + * establishing mappings. > + */ > + if (!inode) > + return; > + > + /* > + * The expectation is that the driver has successfully marked > + * the resource busy by this point, so devmem_is_allowed() > + * should start returning false, however for performance this > + * does not iterate the entire resource range. > + */ > + if (devmem_is_allowed(PHYS_PFN(res->start)) && > + devmem_is_allowed(PHYS_PFN(res->end))) { > + /* > + * *cringe* iomem=3Drelaxed says "go ahead, what's the > + * worst that can happen?" > + */ > + return; > + } > + > + unmap_mapping_range(inode->i_mapping, res->start, resource_size(r= es), 1); > +} > +struct address_space *iomem_get_mapping(void) > +{ > + /* > + * This function is only called from file open paths, hence guara= nteed > + * that fs_initcalls have completed and no need to check for NULL= . But > + * since revoke_iomem can be called before the initcall we still = need > + * the barrier to appease checkers. > + */ > + return smp_load_acquire(&iomem_inode)->i_mapping; > +} > +#else > +static void revoke_iomem(struct resource *res) {} > +struct address_space *iomem_get_mapping(void) > +{ > + return NULL; > +} > +#endif > + > /** > * __request_region - create a new busy resource region > * @parent: parent resource descriptor > @@ -1179,7 +1234,7 @@ struct resource * __request_region(struct resource = *parent, > write_unlock(&resource_lock); > > if (res && orig_parent =3D=3D &iomem_resource) > - revoke_devmem(res); > + revoke_iomem(res); > > return res; > } > @@ -1713,4 +1768,48 @@ static int __init strict_iomem(char *str) > return 1; > } > > +static int iomem_fs_init_fs_context(struct fs_context *fc) > +{ > + return init_pseudo(fc, DEVMEM_MAGIC) ? 0 : -ENOMEM; > +} > + > +static struct file_system_type iomem_fs_type =3D { > + .name =3D "iomem", > + .owner =3D THIS_MODULE, > + .init_fs_context =3D iomem_fs_init_fs_context, > + .kill_sb =3D kill_anon_super, > +}; > + > +static int __init iomem_init_inode(void) > +{ > + static struct vfsmount *iomem_vfs_mount; > + static int iomem_fs_cnt; > + struct inode *inode; > + int rc; > + > + rc =3D simple_pin_fs(&iomem_fs_type, &iomem_vfs_mount, &iomem_fs_= cnt); > + if (rc < 0) { > + pr_err("Cannot mount iomem pseudo filesystem: %d\n", rc); > + return rc; > + } > + > + inode =3D alloc_anon_inode(iomem_vfs_mount->mnt_sb); > + if (IS_ERR(inode)) { > + rc =3D PTR_ERR(inode); > + pr_err("Cannot allocate inode for iomem: %d\n", rc); > + simple_release_fs(&iomem_vfs_mount, &iomem_fs_cnt); > + return rc; > + } > + > + /* > + * Publish /dev/mem initialized. Similar potential fixup: "Publish iomem revocation inode initialized" Other than that: Reviewed-by: Dan Williams