[PATCH 1/1] iommu/vt-d: Check identity map for hot-added devices

2019-02-24 Thread Lu Baolu
The Intel IOMMU driver will put devices into a static identity
mapped domain during boot if the kernel parameter "iommu=pt" is
used. That means the IOMMU hardware will translate a DMA address
into the same memory address.

Unfortunately, hot-added devices are not subject to this. That
results in some devices not working properly after hot added. A
quick way to reproduce this issue is to boot a system with

iommu=pt

and, remove then readd the pci device with

echo 1 > /sys/bus/pci/devices/[pci_source_id]/remove
echo 1 > /sys/bus/pci/rescan

You will find the identity mapped domain was replaced with a
normal domain.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Fenghua Yu 
Cc: sta...@vger.kernel.org
Reported-by: Jis Ben 
Signed-off-by: Lu Baolu 
Tested-by: James Dong 
---
 drivers/iommu/intel-iommu.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index af23cfc2a05e..4f77657a9c25 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4564,16 +4564,19 @@ static int device_notifier(struct notifier_block *nb,
if (iommu_dummy(dev))
return 0;
 
-   if (action != BUS_NOTIFY_REMOVED_DEVICE)
-   return 0;
-
-   domain = find_domain(dev);
-   if (!domain)
-   return 0;
+   if (action == BUS_NOTIFY_REMOVED_DEVICE) {
+   domain = find_domain(dev);
+   if (!domain)
+   return 0;
 
-   dmar_remove_one_dev_info(dev);
-   if (!domain_type_is_vm_or_si(domain) && list_empty(>devices))
-   domain_exit(domain);
+   dmar_remove_one_dev_info(dev);
+   if (!domain_type_is_vm_or_si(domain) &&
+   list_empty(>devices))
+   domain_exit(domain);
+   } else if (action == BUS_NOTIFY_ADD_DEVICE) {
+   if (iommu_should_identity_map(dev, 1))
+   domain_add_dev_info(si_domain, dev);
+   }
 
return 0;
 }
-- 
2.17.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr

2019-02-24 Thread Dave Young
On 02/24/19 at 09:25pm, Pingfan Liu wrote:
> On Fri, Feb 22, 2019 at 9:00 PM Borislav Petkov  wrote:
> >
> > On Fri, Feb 22, 2019 at 09:42:41AM +0100, Joerg Roedel wrote:
> > > The current default of 256MB was found by experiments on a bigger
> > > number of machines, to create a reasonable default that is at least
> > > likely to be sufficient of an average machine.
> >
> > Exactly, and this is what makes sense.
> >
> > The code should try the requested reservation and if it fails, it should
> > try high allocation with default swiotlb size because we need to reserve
> > *some* range.
> >
> > If that reservation succeeds, we should say something along the lines of
> >
> > "... requested range failed, reserved  range instead."
> >
> Maybe I misunderstood you, but does "requested range failed" mean that
> user specify the range? If yes, then it should be the duty of user as
> you said later, not the duty of kernel"

If you go with the changes in your current patch it is needed to say
something like:
"crashkernel: can not find free memory under 4G, reserve XM@.. instead" 

Also need to print the reserved low memory area in case ,high being used.

But for 896M -> 4G, the 896M faulure is not necessary to show in dmesg,
it is some in kernel logic.

Thanks
Dave
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v6 0/3] iommu/io-pgtable-arm-v7s: Use DMA32 zone for page tables

2019-02-24 Thread Nicolas Boichat
On Thu, Feb 14, 2019 at 1:12 AM Vlastimil Babka  wrote:
>
> On 1/22/19 11:51 PM, Nicolas Boichat wrote:
> > Hi Andrew,
> >
> > On Fri, Jan 11, 2019 at 6:21 PM Joerg Roedel  wrote:
> >>
> >> On Wed, Jan 02, 2019 at 01:51:45PM +0800, Nicolas Boichat wrote:
> >> > Does anyone have any further comment on this series? If not, which
> >> > maintainer is going to pick this up? I assume Andrew Morton?
> >>
> >> Probably, yes. I don't like to carry the mm-changes in iommu-tree, so
> >> this should go through mm.
> >
> > Gentle ping on this series, it seems like it's better if it goes
> > through your tree.
> >
> > Series still applies cleanly on linux-next, but I'm happy to resend if
> > that helps.
>
> Ping, Andrew?

Another gentle ping, I still don't see these patches in mmot[ms]. Thanks.

> > Thanks!
> >
> >> Regards,
> >>
> >> Joerg
> >
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/io-pgtable-arm-v7s: only kmemleak_ignore L2 tables

2019-02-24 Thread Nicolas Boichat
Joerg: Just to make sure, is this patch in your queue? Thanks.

On Thu, Jan 31, 2019 at 2:21 AM Will Deacon  wrote:
>
> On Mon, Jan 28, 2019 at 05:43:01PM +0800, Nicolas Boichat wrote:
> > L1 tables are allocated with __get_dma_pages, and therefore already
> > ignored by kmemleak.
> >
> > Without this, the kernel would print this error message on boot,
> > when the first L1 table is allocated:
> >
> > [2.810533] kmemleak: Trying to color unknown object at 
> > 0xffd652388000 as Black
> > [2.818190] CPU: 5 PID: 39 Comm: kworker/5:0 Tainted: G S
> > 4.19.16 #8
> > [2.831227] Workqueue: events deferred_probe_work_func
> > [2.836353] Call trace:
> > ...
> > [2.852532]  paint_ptr+0xa0/0xa8
> > [2.855750]  kmemleak_ignore+0x38/0x6c
> > [2.859490]  __arm_v7s_alloc_table+0x168/0x1f4
> > [2.863922]  arm_v7s_alloc_pgtable+0x114/0x17c
> > [2.868354]  alloc_io_pgtable_ops+0x3c/0x78
> > ...
> >
> > Fixes: e5fc9753b1a8314 ("iommu/io-pgtable: Add ARMv7 short descriptor 
> > support")
> > Signed-off-by: Nicolas Boichat 
> > ---
> >
> > I only tested this on top of my other series
> > (https://patchwork.kernel.org/patch/10720495/), but I think the same fix
> > applies. I'm still a bit confused as to why this only shows up now, as IIUC,
> > the kmemleak_ignore call was always wrong with L1 tables.
>
> Yes, I managed to reproduce this on top of -rc4 (see below). I suspect you
> /are/ the intersection of people using v7s w/ kmemleak, so this has just
> lingered and never been hit until now.
>
> For the patch (assuming this is going via Joerg):
>
> Acked-by: Will Deacon 
>
> Will
>
> --->8
>
> [0.124473] kmemleak: Trying to color unknown object at 0x842d8000 
> as Black
> [0.125312] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
> 5.0.0-rc4-00012-g40b114779944 #1
> [0.126181] Hardware name: linux,dummy-virt (DT)
> [0.126680] Call trace:
> [0.126950]  dump_backtrace+0x0/0x140
> [0.127346]  show_stack+0x14/0x20
> [0.127706]  dump_stack+0x90/0xb4
> [0.128066]  paint_ptr+0x94/0xa8
> [0.128417]  kmemleak_ignore+0x54/0x60
> [0.128991]  __arm_v7s_alloc_table+0x6c/0x240
> [0.129661]  arm_v7s_alloc_pgtable+0x10c/0x188
> [0.130359]  alloc_io_pgtable_ops+0x44/0xb0
> [0.131006]  arm_v7s_do_selftests+0x84/0x4bc
> [0.131663]  do_one_initcall+0x74/0x178
> [0.132253]  kernel_init_freeable+0x188/0x220
> [0.132923]  kernel_init+0x10/0x100
> [0.133460]  ret_from_fork+0x10/0x18
> [0.142102] arm-v7s io-pgtable: self test ok
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH] iommu/vt-d: Handle hotplug devices' default identity mapping setting

2019-02-24 Thread James Dong via iommu
Baolu:

Yes, it is a generic issue for hotplug devices with current Intel IOMMU driver,
as reported in this thread as well.

The patch you provided does the job in our case. Please update this thread once
your patch is merged. Thanks.

Best Regards,
James

On 2/23/19 12:56 AM, Lu Baolu wrote
>
> @@ -4807,16 +4807,19 @@ static int device_notifier(struct notifier_block *nb,
>   if (iommu_dummy(dev))
>   return 0;
> 
>-  if (action != BUS_NOTIFY_REMOVED_DEVICE)
>-  return 0;
>-
>-  domain = find_domain(dev);
>-  if (!domain)
>-  return 0;
>+  if (action == BUS_NOTIFY_REMOVED_DEVICE) {
>+  domain = find_domain(dev);
>+  if (!domain)
>+  return 0;
> 
>-  dmar_remove_one_dev_info(dev);
>-  if (!domain_type_is_vm_or_si(domain) && list_empty(>devices))
>-  domain_exit(domain);
>+  dmar_remove_one_dev_info(dev);
>+  if (!domain_type_is_vm_or_si(domain) &&
>+  list_empty(>devices))
>+  domain_exit(domain);
>+  } else if (action == BUS_NOTIFY_ADD_DEVICE) {
>+  if (iommu_should_identity_map(dev, 1))
>+  domain_add_dev_info(si_domain, dev);
>+  }


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr

2019-02-24 Thread Pingfan Liu
On Fri, Feb 22, 2019 at 9:00 PM Borislav Petkov  wrote:
>
> On Fri, Feb 22, 2019 at 09:42:41AM +0100, Joerg Roedel wrote:
> > The current default of 256MB was found by experiments on a bigger
> > number of machines, to create a reasonable default that is at least
> > likely to be sufficient of an average machine.
>
> Exactly, and this is what makes sense.
>
> The code should try the requested reservation and if it fails, it should
> try high allocation with default swiotlb size because we need to reserve
> *some* range.
>
> If that reservation succeeds, we should say something along the lines of
>
> "... requested range failed, reserved  range instead."
>
Maybe I misunderstood you, but does "requested range failed" mean that
user specify the range? If yes, then it should be the duty of user as
you said later, not the duty of kernel"

> And then in Documentation/admin-guide/kernel-parameters.txt above the
> crashkernel= explanations, the allocation strategy of best effort should
> be explained in short. That the kernel will try to allocate high if the
> requested allocation didn't succeed and that the user can tweak the
> allocation with the below options.
>
Yes, it should be improved.

> Bottom line is: the kernel should assist the user and try harder to
> allocate *some* range for a crash kernel when there's no detailed
> specification what that range should be.
>
> *If* the user adds ,low, high, then the kernel should try only that
> specified range because the assumption is that the user knows what she's
> doing.
>
> But if the user simply wants a range for a crash kernel without stating
> where that range should be in particular and it's placement is a don't
> care - as long as there is a range - then the kernel should simply try
> high, etc.
>
We do not know the memory layout of a system, maybe a system with
memory less than 4GB. So it is better to try all the range of system
memory

Thanks,
Pingfan

> Makes sense?
>
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/vt-d: Handle hotplug devices' default identity mapping setting

2019-02-24 Thread Lu Baolu

Hi James,

On 2/22/19 3:36 PM, James Dong wrote:

Baolu:

Sorry that my last reply email seems not text format. Resend it now.

Thanks for your comments and your patch. Please find below our responses to
each of your comments:


What does "I/O operation won't work" exactly mean here? Do you see any
IOMMU fault message? Or, something doesn't work as expected?


Yes, DMAR fault messages as following came out:
[  354.939896] DMAR: DMAR:[DMA Read] Request device [03:00.1]fault addr 
1fdfe8
[  354.939896] DMAR:[fault reason 02] Present bit in context entry is clear



Do you mind checking this?

index 6ecdcf8fc8c0..f62f30bc1339 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2632,6 +2632,9 @@ static struct dmar_domain
*find_or_alloc_domain(struct device *dev, int gaw)
  goto out;
  }

+   if (!iommu_should_identity_map(dev, 0))
+   return si_domain;
+
  /* Allocate and initialize new domain for the device */
  domain = alloc_domain(0);
  if (!domain)


Tried this patch, and the same DMAR fault message came out.


Thank you!



Guess it is because of the iommu code path for hotplug devices. If a hotplug
device is rescanned after removal, iommu_bus_notifier will be called as part
of the notifier chains to handle BUS_NOTIFY_ADD_DEVICE event. Along the code
path, intel_iommu_ops->add_device() created an iommu group for this hotplug
device, but failed to create an iommu domain because of the default domain
type IOMMU_DOMAIN_IDENTITY imposed by current IOMMU command line option got
declined by intel_iommu_ops->domain_alloc().


The Intel IOMMU driver hasn't switched to default domain yet although
it's in the pipe line. So, there should be no domain allocated when a
group is allocated for the device.

The problem is we need to check whether a hot-added device requires
identity map instead of allocating a normal domain blindly.



Since si_domain is type of "struct dmar_domain", which is platform dependent,
it is hard to make this change in intel_iommu_ops->domain_alloc().

In your patch, function find_or_alloc_domain() is not in the code path of
BUS_NOTIFY_ADD_DEVICE event notifier chain.

Please let us know if your have more concerns and suggestions.


Can you please try the patch attached? I think this is a generic issue
as I described in the commit message.

Best regards,
Lu Baolu
>From d942e60557fc7ea6fee535fb9a0a7d334d65b636 Mon Sep 17 00:00:00 2001
From: Lu Baolu 
Date: Sun, 24 Feb 2019 10:01:03 +0800
Subject: [PATCH 1/1] iommu/vt-d: Check identity map for hot-added devices

The Intel IOMMU driver will put devices into a static identity
mapped domain during boot if the kernel parameter "iommu=pt" is
used. That means the IOMMU hardware will translate a DMA address
into the same memory address.

Unfortunately, a hot-added device doesn't subject to this. That
results in some devices not working properly after hot added. A
quick way to reproduce this issue is to boot a system with

iommu=pt

and, remove then readd the pci device with

echo 1 > /sys/bus/pci/devices/[pci_source_id]/remove
echo 1 > /sys/bus/pci/rescan

You will find the identity mapped domain was replaced with a
normal domain.

Cc: Ashok Raj 
Cc: Jacob Pan 
Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel-iommu.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 6ecdcf8fc8c0..730ee29d561b 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3001,9 +3001,9 @@ static int iommu_should_identity_map(struct device *dev, int startup)
 	}
 
 	/*
-	 * At boot time, we don't yet know if devices will be 64-bit capable.
-	 * Assume that they will — if they turn out not to be, then we can
-	 * take them out of the 1:1 domain later.
+	 * At boot time or hot added, we don't yet know if devices will be
+	 * 64-bit capable. Assume that they will — if they turn out not to
+	 * be, then we can take them out of the 1:1 domain later.
 	 */
 	if (!startup) {
 		/*
@@ -4807,16 +4807,19 @@ static int device_notifier(struct notifier_block *nb,
 	if (iommu_dummy(dev))
 		return 0;
 
-	if (action != BUS_NOTIFY_REMOVED_DEVICE)
-		return 0;
-
-	domain = find_domain(dev);
-	if (!domain)
-		return 0;
+	if (action == BUS_NOTIFY_REMOVED_DEVICE) {
+		domain = find_domain(dev);
+		if (!domain)
+			return 0;
 
-	dmar_remove_one_dev_info(dev);
-	if (!domain_type_is_vm_or_si(domain) && list_empty(>devices))
-		domain_exit(domain);
+		dmar_remove_one_dev_info(dev);
+		if (!domain_type_is_vm_or_si(domain) &&
+		list_empty(>devices))
+			domain_exit(domain);
+	} else if (action == BUS_NOTIFY_ADD_DEVICE) {
+		if (iommu_should_identity_map(dev, 1))
+			domain_add_dev_info(si_domain, dev);
+	}
 
 	return 0;
 }
-- 
2.17.1

___
iommu mailing list
iommu@lists.linux-foundation.org