From: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> To: Lu Baolu <baolu.lu@linux.intel.com> Cc: "David Woodhouse" <dwmw2@infradead.org>, "Joerg Roedel" <joro@8bytes.org>, iommu@lists.linux-foundation.org, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, "Michał Wajdeczko" <michal.wajdeczko@intel.com>, "Janusz Krzysztofik" <janusz.krzysztofik@linux.intel.com> Subject: Re: [RFC PATCH] iommu/vt-d: Fix IOMMU field not populated on device hot re-plug Date: Mon, 02 Sep 2019 10:37:41 +0200 [thread overview] Message-ID: <1769080.0GM3UzqXcv@jkrzyszt-desk.ger.corp.intel.com> (raw) In-Reply-To: <ccb1434d-281c-abae-0726-7fd924041315@linux.intel.com> Hi Baolu, On Thursday, August 29, 2019 11:08:18 AM CEST Lu Baolu wrote: > Hi, > > On 8/29/19 3:58 PM, Janusz Krzysztofik wrote: > > Hi Baolu, > > > > On Thursday, August 29, 2019 3:43:31 AM CEST Lu Baolu wrote: > >> Hi Janusz, > >> > >> On 8/28/19 10:17 PM, Janusz Krzysztofik wrote: > >>>> We should avoid kernel panic when a intel_unmap() is called against > >>>> a non-existent domain. > >>> Does that mean you suggest to replace > >>> BUG_ON(!domain); > >>> with something like > >>> if (WARN_ON(!domain)) > >>> return; > >>> and to not care of orphaned mappings left allocated? Is there a way to > > inform > >>> users that their active DMA mappings are no longer valid and they > > shouldn't > >>> call dma_unmap_*()? > >>> > >>>> But we shouldn't expect the IOMMU driver not > >>>> cleaning up the domain info when a device remove notification comes and > >>>> wait until all file descriptors being closed, right? > >>> Shouldn't then the IOMMU driver take care of cleaning up resources still > >>> allocated on device remove before it invalidates and forgets their > > pointers? > >>> > >> > >> You are right. We need to wait until all allocated resources (iova and > >> mappings) to be released. > >> > >> How about registering a callback for BUS_NOTIFY_UNBOUND_DRIVER, and > >> removing the domain info when the driver detachment completes? > > > > Device core calls BUS_NOTIFY_UNBOUND_DRIVER on each driver unbind, regardless > > of a device being removed or not. As long as the device is not unplugged and > > the BUS_NOTIFY_REMOVED_DEVICE notification not generated, an unbound driver is > > not a problem here. > > Morever, BUS_NOTIFY_UNBOUND_DRIVER is called even before > > BUS_NOTIFY_REMOVED_DEVICE so that wouldn't help anyway. > > Last but not least, bus events are independent of the IOMMU driver use via > > DMA-API it exposes. > > Fair enough. > > > > > If keeping data for unplugged devices and reusing it on device re-plug is not > > acceptable then maybe the IOMMU driver should perform reference counting of > > its internal resources occupied by DMA-API users and perform cleanups on last > > release? > > I am not saying that keeping data is not acceptable. I just want to > check whether there are any other solutions. Then reverting 458b7c8e0dde and applying this patch still resolves the issue for me. No errors appear when mappings are unmapped on device close after the device has been removed, and domain info preserved on device removal is successfully reused on device re-plug. Is there anything else I can do to help? Thanks, Janusz > > Best regards, > Baolu >
WARNING: multiple messages have this Message-ID (diff)
From: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> To: Lu Baolu <baolu.lu@linux.intel.com> Cc: linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, "Janusz Krzysztofik" <janusz.krzysztofik@linux.intel.com>, "David Woodhouse" <dwmw2@infradead.org>, intel-gfx@lists.freedesktop.org, "Michał Wajdeczko" <michal.wajdeczko@intel.com> Subject: Re: [RFC PATCH] iommu/vt-d: Fix IOMMU field not populated on device hot re-plug Date: Mon, 02 Sep 2019 10:37:41 +0200 [thread overview] Message-ID: <1769080.0GM3UzqXcv@jkrzyszt-desk.ger.corp.intel.com> (raw) In-Reply-To: <ccb1434d-281c-abae-0726-7fd924041315@linux.intel.com> Hi Baolu, On Thursday, August 29, 2019 11:08:18 AM CEST Lu Baolu wrote: > Hi, > > On 8/29/19 3:58 PM, Janusz Krzysztofik wrote: > > Hi Baolu, > > > > On Thursday, August 29, 2019 3:43:31 AM CEST Lu Baolu wrote: > >> Hi Janusz, > >> > >> On 8/28/19 10:17 PM, Janusz Krzysztofik wrote: > >>>> We should avoid kernel panic when a intel_unmap() is called against > >>>> a non-existent domain. > >>> Does that mean you suggest to replace > >>> BUG_ON(!domain); > >>> with something like > >>> if (WARN_ON(!domain)) > >>> return; > >>> and to not care of orphaned mappings left allocated? Is there a way to > > inform > >>> users that their active DMA mappings are no longer valid and they > > shouldn't > >>> call dma_unmap_*()? > >>> > >>>> But we shouldn't expect the IOMMU driver not > >>>> cleaning up the domain info when a device remove notification comes and > >>>> wait until all file descriptors being closed, right? > >>> Shouldn't then the IOMMU driver take care of cleaning up resources still > >>> allocated on device remove before it invalidates and forgets their > > pointers? > >>> > >> > >> You are right. We need to wait until all allocated resources (iova and > >> mappings) to be released. > >> > >> How about registering a callback for BUS_NOTIFY_UNBOUND_DRIVER, and > >> removing the domain info when the driver detachment completes? > > > > Device core calls BUS_NOTIFY_UNBOUND_DRIVER on each driver unbind, regardless > > of a device being removed or not. As long as the device is not unplugged and > > the BUS_NOTIFY_REMOVED_DEVICE notification not generated, an unbound driver is > > not a problem here. > > Morever, BUS_NOTIFY_UNBOUND_DRIVER is called even before > > BUS_NOTIFY_REMOVED_DEVICE so that wouldn't help anyway. > > Last but not least, bus events are independent of the IOMMU driver use via > > DMA-API it exposes. > > Fair enough. > > > > > If keeping data for unplugged devices and reusing it on device re-plug is not > > acceptable then maybe the IOMMU driver should perform reference counting of > > its internal resources occupied by DMA-API users and perform cleanups on last > > release? > > I am not saying that keeping data is not acceptable. I just want to > check whether there are any other solutions. Then reverting 458b7c8e0dde and applying this patch still resolves the issue for me. No errors appear when mappings are unmapped on device close after the device has been removed, and domain info preserved on device removal is successfully reused on device re-plug. Is there anything else I can do to help? Thanks, Janusz > > Best regards, > Baolu > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2019-09-02 8:38 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-22 14:29 [RFC PATCH] iommu/vt-d: Fix IOMMU field not populated on device hot re-plug Janusz Krzysztofik 2019-08-22 14:29 ` Janusz Krzysztofik 2019-08-22 15:34 ` ✓ Fi.CI.BAT: success for " Patchwork 2019-08-23 1:51 ` [RFC PATCH] " Lu Baolu 2019-08-23 1:51 ` Lu Baolu 2019-08-26 8:15 ` Janusz Krzysztofik 2019-08-26 8:15 ` Janusz Krzysztofik 2019-08-26 8:29 ` Lu Baolu 2019-08-26 8:29 ` Lu Baolu 2019-08-27 9:35 ` Janusz Krzysztofik 2019-08-27 9:35 ` Janusz Krzysztofik 2019-08-27 9:35 ` Janusz Krzysztofik 2019-08-28 0:56 ` Lu Baolu 2019-08-28 0:56 ` Lu Baolu 2019-08-28 14:17 ` Janusz Krzysztofik 2019-08-28 14:17 ` Janusz Krzysztofik 2019-08-29 1:43 ` Lu Baolu 2019-08-29 1:43 ` Lu Baolu 2019-08-29 1:43 ` Lu Baolu 2019-08-29 7:58 ` Janusz Krzysztofik 2019-08-29 7:58 ` Janusz Krzysztofik 2019-08-29 7:58 ` Janusz Krzysztofik 2019-08-29 9:08 ` Lu Baolu 2019-08-29 9:08 ` Lu Baolu 2019-09-02 8:37 ` Janusz Krzysztofik [this message] 2019-09-02 8:37 ` Janusz Krzysztofik 2019-09-03 1:29 ` Lu Baolu 2019-09-03 1:29 ` Lu Baolu 2019-09-03 7:41 ` Janusz Krzysztofik 2019-09-03 7:41 ` Janusz Krzysztofik 2019-10-01 15:01 ` Janusz Krzysztofik 2019-10-01 15:01 ` Janusz Krzysztofik 2019-10-08 2:27 ` Lu Baolu 2019-10-08 2:27 ` Lu Baolu 2019-10-08 2:27 ` Lu Baolu 2019-10-11 6:54 ` Lu Baolu 2019-10-11 6:54 ` Lu Baolu 2019-10-11 6:54 ` Lu Baolu 2019-10-11 10:27 ` Janusz Krzysztofik 2019-10-11 10:27 ` Janusz Krzysztofik 2019-10-11 10:27 ` Janusz Krzysztofik 2019-08-23 8:47 ` ✓ Fi.CI.IGT: success for " Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1769080.0GM3UzqXcv@jkrzyszt-desk.ger.corp.intel.com \ --to=janusz.krzysztofik@linux.intel.com \ --cc=baolu.lu@linux.intel.com \ --cc=dwmw2@infradead.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=iommu@lists.linux-foundation.org \ --cc=joro@8bytes.org \ --cc=linux-kernel@vger.kernel.org \ --cc=michal.wajdeczko@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.