Re: [RFC/PATCH 1/1] virtio: Introduce MMIO ops
On 30.04.20 13:11, Srivatsa Vaddagiri wrote: * Will Deacon [2020-04-30 11:41:50]: On Thu, Apr 30, 2020 at 04:04:46PM +0530, Srivatsa Vaddagiri wrote: If CONFIG_VIRTIO_MMIO_OPS is defined, then I expect this to be unconditionally set to 'magic_qcom_ops' that uses hypervisor-supported interface for IO (for example: message_queue_send() and message_queue_recevie() hypercalls). Hmm, but then how would such a kernel work as a guest under all the spec-compliant hypervisors out there? Ok I see your point and yes for better binary compatibility, the ops have to be set based on runtime detection of hypervisor capabilities. Ok. I guess the other option is to standardize on a new virtio transport (like ivshmem2-virtio)? I haven't looked at that, but I suppose it depends on what your hypervisor folks are willing to accomodate. I believe ivshmem2_virtio requires hypervisor to support PCI device emulation (for life-cycle management of VMs), which our hypervisor may not support. A simple shared memory and doorbell or message-queue based transport will work for us. As written in our private conversation, a mapping of the ivshmem2 device discovery to platform mechanism (device tree etc.) and maybe even the register access for doorbell and life-cycle management to something hypercall-like would be imaginable. What would count more from virtio perspective is a common mapping on a shared memory transport. That said, I also warned about all the features that PCI already defined (such as message-based interrupts) which you may have to add when going a different way for the shared memory device. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [virtio-dev] Re: [PATCH 5/5] virtio: Add bounce DMA ops
On 29.04.20 12:45, Michael S. Tsirkin wrote: On Wed, Apr 29, 2020 at 12:26:43PM +0200, Jan Kiszka wrote: On 29.04.20 12:20, Michael S. Tsirkin wrote: On Wed, Apr 29, 2020 at 03:39:53PM +0530, Srivatsa Vaddagiri wrote: That would still not work I think where swiotlb is used for pass-thr devices (when private memory is fine) as well as virtio devices (when shared memory is required). So that is a separate question. When there are multiple untrusted devices, at the moment it looks like a single bounce buffer is used. Which to me seems like a security problem, I think we should protect untrusted devices from each other. Definitely. That's the model we have for ivshmem-virtio as well. Jan Want to try implementing that? The desire is definitely there, currently "just" not the time. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/5] virtio: Add bounce DMA ops
On 29.04.20 12:20, Michael S. Tsirkin wrote: On Wed, Apr 29, 2020 at 03:39:53PM +0530, Srivatsa Vaddagiri wrote: That would still not work I think where swiotlb is used for pass-thr devices (when private memory is fine) as well as virtio devices (when shared memory is required). So that is a separate question. When there are multiple untrusted devices, at the moment it looks like a single bounce buffer is used. Which to me seems like a security problem, I think we should protect untrusted devices from each other. Definitely. That's the model we have for ivshmem-virtio as well. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] swiotlb: Allow swiotlb to live at pre-defined address
On 26.03.20 17:29, Alexander Graf wrote:
The swiotlb is a very convenient fallback mechanism for bounce buffering of
DMAable data. It is usually used for the compatibility case where devices
can only DMA to a "low region".
However, in some scenarios this "low region" may be bound even more
heavily. For example, there are embedded system where only an SRAM region
is shared between device and CPU. There are also heterogeneous computing
scenarios where only a subset of RAM is cache coherent between the
components of the system. There are partitioning hypervisors, where
a "control VM" that implements device emulation has limited view into a
partition's memory for DMA capabilities due to safety concerns.
This patch adds a command line driven mechanism to move all DMA memory into
a predefined shared memory region which may or may not be part of the
physical address layout of the Operating System.
Ideally, the typical path to set this configuration would be through Device
Tree or ACPI, but neither of the two mechanisms is standardized yet. Also,
in the x86 MicroVM use case, we have neither ACPI nor Device Tree, but
instead configure the system purely through kernel command line options.
I'm sure other people will find the functionality useful going forward
though and extend it to be triggered by DT/ACPI in the future.
Signed-off-by: Alexander Graf
---
Documentation/admin-guide/kernel-parameters.txt | 3 +-
Documentation/x86/x86_64/boot-options.rst | 4 ++-
kernel/dma/swiotlb.c| 46 +++--
3 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index c07815d230bc..d085d55c3cbe 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4785,11 +4785,12 @@
it if 0 is given (See
Documentation/admin-guide/cgroup-v1/memory.rst)
swiotlb=[ARM,IA-64,PPC,MIPS,X86]
- Format: { | force | noforce }
+ Format: { | force | noforce | addr= }
-- Number of I/O TLB slabs
force -- force using of bounce buffers even if they
wouldn't be automatically used by the kernel
noforce -- Never use bounce buffers (for debugging)
+ addr= -- Try to allocate SWIOTLB at defined
address
switches= [HW,M68k]
diff --git a/Documentation/x86/x86_64/boot-options.rst
b/Documentation/x86/x86_64/boot-options.rst
index 2b98efb5ba7f..ca46c57b68c9 100644
--- a/Documentation/x86/x86_64/boot-options.rst
+++ b/Documentation/x86/x86_64/boot-options.rst
@@ -297,11 +297,13 @@ iommu options only relevant to the AMD GART hardware
IOMMU:
iommu options only relevant to the software bounce buffering (SWIOTLB) IOMMU
implementation:
-swiotlb=[,force]
+swiotlb=[,force][,addr=]
Prereserve that many 128K pages for the software IO bounce buffering.
force
Force all IO through the software TLB.
+ addr=
+Try to allocate SWIOTLB at defined address
Settings for the IBM Calgary hardware IOMMU currently found in IBM
pSeries and xSeries machines
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index c19379fabd20..83da0caa2f93 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -46,6 +46,7 @@
#include
#include
#include
+#include
#define CREATE_TRACE_POINTS
#include
@@ -102,6 +103,12 @@ unsigned int max_segment;
#define INVALID_PHYS_ADDR (~(phys_addr_t)0)
static phys_addr_t *io_tlb_orig_addr;
+/*
+ * The TLB phys addr may be defined on the command line. Store it here if it
is.
+ */
+static phys_addr_t io_tlb_addr = INVALID_PHYS_ADDR;
+
+
/*
* Protect the above data structures in the map and unmap calls
*/
@@ -119,11 +126,23 @@ setup_io_tlb_npages(char *str)
}
if (*str == ',')
++str;
- if (!strcmp(str, "force")) {
+ if (!strncmp(str, "force", 5)) {
swiotlb_force = SWIOTLB_FORCE;
- } else if (!strcmp(str, "noforce")) {
+ str += 5;
+ } else if (!strncmp(str, "noforce", 7)) {
swiotlb_force = SWIOTLB_NO_FORCE;
io_tlb_nslabs = 1;
+ str += 7;
+ }
+
+ if (*str == ',')
+ ++str;
+ if (!strncmp(str, "addr=", 5)) {
+ char *addrstr = str + 5;
+
+ io_tlb_addr = kstrtoul(addrstr, 0, );
+ if (addrstr == str)
+ io_tlb_addr = INVALID_PHYS_ADDR;
}
return 0;
@@ -239,6 +258,25 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long
nslabs, int verbose)
return 0;
}
+static int __init swiotlb_init_io(int verbose, unsigned long bytes)
+{
+ unsigned __iomem char *vstart;
Re: [PATCH] iommu/vt-d: Don't register bus-notifier under
On 2017-10-10 09:21, Joerg Roedel wrote: > On Mon, Oct 09, 2017 at 06:58:13PM +0200, Jan Kiszka wrote: >>> extern int dmar_table_init(void); >>> extern int dmar_dev_scope_init(void); >>> +extern void dmar_register_bus_notifier(void); >>> extern int dmar_parse_dev_scope(void *start, void *end, int *cnt, >>> struct dmar_dev_scope **devices, u16 segment); >>> extern void *dmar_alloc_dev_scope(void *start, void *end, int *cnt); >>> >> >> Silences the warning, but locking in the init paths still smells fishy >> to me. > > Yes, its certainly not optimal, but the code that runs in there also > runs at iommu hotplug time, so we can't just remove the locking there > entirely. It may not just be "sub-optimal" but rather borken: "One example: dmar_table_init is not consistently protected by dmar_global_lock." Jan > > On the other side the warning you reported is a false-positive, it can > never dead-lock because the reverse lock-order happens only at > initialization time, but I don't know how to silence it otherwise. > > > Regards, > > Joerg > -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/vt-d: Don't register bus-notifier under
On 2017-10-06 15:08, Joerg Roedel wrote:
> Hey Jan,
>
> On Wed, Sep 27, 2017 at 04:19:15PM +0200, Jan Kiszka wrote:
>> If we could drop the dmar_global_lock around bus_register_notifier in
>> dmar_dev_scope_init, the issue above would likely be resolved.
>
> That is true. Can you please try this patch?
>
>>From dd7685f84d3954f9361bfb4290fb8a0dd033097a Mon Sep 17 00:00:00 2001
> From: Joerg Roedel <jroe...@suse.de>
> Date: Fri, 6 Oct 2017 15:00:53 +0200
> Subject: [PATCH] iommu/vt-d: Don't register bus-notifier under
> dmar_global_lock
>
> The notifier function will take the dmar_global_lock too, so
> lockdep complains about inverse locking order when the
> notifier is registered under the dmar_global_lock.
>
> Reported-by: Jan Kiszka <jan.kis...@siemens.com>
> Fixes: 59ce0515cdaf ('iommu/vt-d: Update DRHD/RMRR/ATSR device scope caches
> when PCI hotplug happens')
> Signed-off-by: Joerg Roedel <jroe...@suse.de>
> ---
> drivers/iommu/dmar.c| 7 +--
> drivers/iommu/intel-iommu.c | 10 ++
> include/linux/dmar.h| 1 +
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index 57c920c1372d..1ea7cd537873 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -801,13 +801,16 @@ int __init dmar_dev_scope_init(void)
> dmar_free_pci_notify_info(info);
> }
> }
> -
> - bus_register_notifier(_bus_type, _pci_bus_nb);
> }
>
> return dmar_dev_scope_status;
> }
>
> +void dmar_register_bus_notifier(void)
> +{
> + bus_register_notifier(_bus_type, _pci_bus_nb);
> +}
> +
>
> int __init dmar_table_init(void)
> {
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 6784a05dd6b2..934cef924461 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -4752,6 +4752,16 @@ int __init intel_iommu_init(void)
> goto out_free_dmar;
> }
>
> + up_write(_global_lock);
> +
> + /*
> + * The bus notifier takes the dmar_global_lock, so lockdep will
> + * complain later when we register it under the lock.
> + */
> + dmar_register_bus_notifier();
> +
> + down_write(_global_lock);
> +
> if (no_iommu || dmar_disabled) {
> /*
>* We exit the function here to ensure IOMMU's remapping and
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index e8ffba1052d3..e2433bc50210 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -112,6 +112,7 @@ static inline bool dmar_rcu_check(void)
>
> extern int dmar_table_init(void);
> extern int dmar_dev_scope_init(void);
> +extern void dmar_register_bus_notifier(void);
> extern int dmar_parse_dev_scope(void *start, void *end, int *cnt,
> struct dmar_dev_scope **devices, u16 segment);
> extern void *dmar_alloc_dev_scope(void *start, void *end, int *cnt);
>
Silences the warning, but locking in the init paths still smells fishy
to me.
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: intel-dmar: possible circular locking dependency detected
On 2017-09-27 15:21, Jan Kiszka wrote:
> On 2017-09-27 14:14, Jan Kiszka wrote:
>> Hi,
>>
>> while I'm triggering this with a still out-of-tree module from the
>> Jailhouse project, the potential deadlock appears to me being unrelated
>> to it. Please have a look:
>>
>> ==
>> WARNING: possible circular locking dependency detected
>> 4.14.0-rc2-dbg+ #176 Tainted: G O
>> --
>> jailhouse/6105 is trying to acquire lock:
>> dmar_pci_bus_notifier+0x4f/0xcb
>>
>> but task is already holding lock:
>> __blocking_notifier_call_chain+0x31/0x65
>>
>> which lock already depends on the new lock.
>>
>>
>> the existing dependency chain (in reverse order) is:
>>
>> -> #1 (&(>bus_notifier)->rwsem){}:
>>__lock_acquire+0xed7/0x113b
>>lock_acquire+0x148/0x1f6
>>down_write+0x3b/0x6a
>>blocking_notifier_chain_register+0x33/0x53
>>bus_register_notifier+0x1c/0x1e
>>dmar_dev_scope_init+0x2c6/0x2db
>>intel_iommu_init+0xec/0x11c2
>>pci_iommu_init+0x17/0x41
>>do_one_initcall+0x90/0x143
>>kernel_init_freeable+0x1cc/0x256
>>kernel_init+0xe/0xf8
>>ret_from_fork+0x2a/0x40
>>
>> -> #0 (dmar_global_lock){}:
>>check_prev_add+0x112/0x65f
>>__lock_acquire+0xed7/0x113b
>>lock_acquire+0x148/0x1f6
>>down_write+0x3b/0x6a
>>dmar_pci_bus_notifier+0x4f/0xcb
>>notifier_call_chain+0x3c/0x5e
>>__blocking_notifier_call_chain+0x4c/0x65
>>blocking_notifier_call_chain+0x14/0x16
>>device_add+0x40c/0x522
>>pci_device_add+0x1c0/0x1ce
>>pci_scan_single_device+0x92/0x9d
>>pci_scan_slot+0x59/0x10a
>>jailhouse_pci_do_all_devices+0x74/0x263 [jailhouse]
>>jailhouse_pci_virtual_root_devices_add+0x40/0x42 [jailhouse]
>>jailhouse_cmd_enable+0x4fd/0x5e8 [jailhouse]
>>jailhouse_ioctl+0x28/0x70 [jailhouse]
>>vfs_ioctl+0x18/0x34
>>do_vfs_ioctl+0x51b/0x5e3
>>SyS_ioctl+0x50/0x7b
>>entry_SYSCALL_64_fastpath+0x1f/0xbe
>>
>> other info that might help us debug this:
>>
>> Possible unsafe locking scenario:
>>
>>CPU0CPU1
>>
>> lock(&(>bus_notifier)->rwsem);
>>lock(dmar_global_lock);
>>lock(&(>bus_notifier)->rwsem);
>> lock(dmar_global_lock);
>>
>> *** DEADLOCK ***
>>
>> 2 locks held by jailhouse/6105:
>> jailhouse_cmd_enable+0x130/0x5e8 [jailhouse]
>> __blocking_notifier_call_chain+0x31/0x65
>>
>> stack backtrace:
>> CPU: 1 PID: 6105 Comm: jailhouse Tainted: G O
>> 4.14.0-rc2-dbg+ #176
>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
>> rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
>> Call Trace:
>> dump_stack+0x85/0xbe
>> print_circular_bug+0x389/0x398
>> ? add_lock_to_list.isra.23+0x96/0x96
>> check_prev_add+0x112/0x65f
>> ? kernel_text_address+0x1c/0x6a
>> ? add_lock_to_list.isra.23+0x96/0x96
>> __lock_acquire+0xed7/0x113b
>> ? __lock_acquire+0xed7/0x113b
>> lock_acquire+0x148/0x1f6
>> ? dmar_pci_bus_notifier+0x4f/0xcb
>> down_write+0x3b/0x6a
>> ? dmar_pci_bus_notifier+0x4f/0xcb
>> dmar_pci_bus_notifier+0x4f/0xcb
>> notifier_call_chain+0x3c/0x5e
>> __blocking_notifier_call_chain+0x4c/0x65
>> blocking_notifier_call_chain+0x14/0x16
>> device_add+0x40c/0x522
>> pci_device_add+0x1c0/0x1ce
>> pci_scan_single_device+0x92/0x9d
>> pci_scan_slot+0x59/0x10a
>> jailhouse_pci_do_all_devices+0x74/0x263 [jailhouse]
>> jailhouse_pci_virtual_root_devices_add+0x40/0x42 [jailhouse]
>> jailhouse_cmd_enable+0x4fd/0x5e8 [jailhouse]
>> jailhouse_ioctl+0x28/0x70 [jailhouse]
>> vfs_ioctl+0x18/0x34
>> do_vfs_ioctl+0x51b/0x5e3
>> ? kmem_cache_free+0x15b/0x1fa
>> ? entry_SYSCALL_64_fastpath+0x5/0xbe
>> ? trace_hardirqs_on_caller+0x180/0x19c
>> SyS_ioctl+0x50/0x7b
>> entry_SYSCALL_64_fastpath+0x1f/0xbe
>> RIP: 0033:0x7f8b3b110d87
>> RSP: 002b:7ffc44b70088 EFLAGS: 0206 ORIG_RAX: 0010
>> RAX: ffda RBX: 0046 RCX: 7f8b3b110d87
>> RDX: 00604010 RSI: 4008 RD
Re: intel-dmar: possible circular locking dependency detected
On 2017-09-27 14:14, Jan Kiszka wrote:
> Hi,
>
> while I'm triggering this with a still out-of-tree module from the
> Jailhouse project, the potential deadlock appears to me being unrelated
> to it. Please have a look:
>
> ==
> WARNING: possible circular locking dependency detected
> 4.14.0-rc2-dbg+ #176 Tainted: G O
> --
> jailhouse/6105 is trying to acquire lock:
> dmar_pci_bus_notifier+0x4f/0xcb
>
> but task is already holding lock:
> __blocking_notifier_call_chain+0x31/0x65
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (&(>bus_notifier)->rwsem){}:
>__lock_acquire+0xed7/0x113b
>lock_acquire+0x148/0x1f6
>down_write+0x3b/0x6a
>blocking_notifier_chain_register+0x33/0x53
>bus_register_notifier+0x1c/0x1e
>dmar_dev_scope_init+0x2c6/0x2db
>intel_iommu_init+0xec/0x11c2
>pci_iommu_init+0x17/0x41
>do_one_initcall+0x90/0x143
>kernel_init_freeable+0x1cc/0x256
>kernel_init+0xe/0xf8
>ret_from_fork+0x2a/0x40
>
> -> #0 (dmar_global_lock){}:
>check_prev_add+0x112/0x65f
>__lock_acquire+0xed7/0x113b
>lock_acquire+0x148/0x1f6
>down_write+0x3b/0x6a
>dmar_pci_bus_notifier+0x4f/0xcb
>notifier_call_chain+0x3c/0x5e
>__blocking_notifier_call_chain+0x4c/0x65
>blocking_notifier_call_chain+0x14/0x16
>device_add+0x40c/0x522
>pci_device_add+0x1c0/0x1ce
>pci_scan_single_device+0x92/0x9d
>pci_scan_slot+0x59/0x10a
>jailhouse_pci_do_all_devices+0x74/0x263 [jailhouse]
>jailhouse_pci_virtual_root_devices_add+0x40/0x42 [jailhouse]
>jailhouse_cmd_enable+0x4fd/0x5e8 [jailhouse]
>jailhouse_ioctl+0x28/0x70 [jailhouse]
>vfs_ioctl+0x18/0x34
>do_vfs_ioctl+0x51b/0x5e3
>SyS_ioctl+0x50/0x7b
>entry_SYSCALL_64_fastpath+0x1f/0xbe
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
>CPU0CPU1
>
> lock(&(>bus_notifier)->rwsem);
>lock(dmar_global_lock);
>lock(&(>bus_notifier)->rwsem);
> lock(dmar_global_lock);
>
> *** DEADLOCK ***
>
> 2 locks held by jailhouse/6105:
> jailhouse_cmd_enable+0x130/0x5e8 [jailhouse]
> __blocking_notifier_call_chain+0x31/0x65
>
> stack backtrace:
> CPU: 1 PID: 6105 Comm: jailhouse Tainted: G O
> 4.14.0-rc2-dbg+ #176
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
> Call Trace:
> dump_stack+0x85/0xbe
> print_circular_bug+0x389/0x398
> ? add_lock_to_list.isra.23+0x96/0x96
> check_prev_add+0x112/0x65f
> ? kernel_text_address+0x1c/0x6a
> ? add_lock_to_list.isra.23+0x96/0x96
> __lock_acquire+0xed7/0x113b
> ? __lock_acquire+0xed7/0x113b
> lock_acquire+0x148/0x1f6
> ? dmar_pci_bus_notifier+0x4f/0xcb
> down_write+0x3b/0x6a
> ? dmar_pci_bus_notifier+0x4f/0xcb
> dmar_pci_bus_notifier+0x4f/0xcb
> notifier_call_chain+0x3c/0x5e
> __blocking_notifier_call_chain+0x4c/0x65
> blocking_notifier_call_chain+0x14/0x16
> device_add+0x40c/0x522
> pci_device_add+0x1c0/0x1ce
> pci_scan_single_device+0x92/0x9d
> pci_scan_slot+0x59/0x10a
> jailhouse_pci_do_all_devices+0x74/0x263 [jailhouse]
> jailhouse_pci_virtual_root_devices_add+0x40/0x42 [jailhouse]
> jailhouse_cmd_enable+0x4fd/0x5e8 [jailhouse]
> jailhouse_ioctl+0x28/0x70 [jailhouse]
> vfs_ioctl+0x18/0x34
> do_vfs_ioctl+0x51b/0x5e3
> ? kmem_cache_free+0x15b/0x1fa
> ? entry_SYSCALL_64_fastpath+0x5/0xbe
> ? trace_hardirqs_on_caller+0x180/0x19c
> SyS_ioctl+0x50/0x7b
> entry_SYSCALL_64_fastpath+0x1f/0xbe
> RIP: 0033:0x7f8b3b110d87
> RSP: 002b:7ffc44b70088 EFLAGS: 0206 ORIG_RAX: 0010
> RAX: ffda RBX: 0046 RCX: 7f8b3b110d87
> RDX: 00604010 RSI: 4008 RDI: 0003
> RBP: 00604010 R08: 7f8b3b3ade80 R09: 000885d0
> R10: 7ffc44b6fe40 R11: 0206 R12: 25d4
> R13: R14: 7ffc44b714a4 R15:
>
> Thanks,
> Jan
>
Oh, just realized that I already sent this report earlier this year [1]
but didn't receive any feedback so far.
Jan
[1] https://lkml.org/lkml/2017/7/24/238
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
intel-dmar: possible circular locking dependency detected
Hi,
while I'm triggering this with a still out-of-tree module from the
Jailhouse project, the potential deadlock appears to me being unrelated
to it. Please have a look:
==
WARNING: possible circular locking dependency detected
4.14.0-rc2-dbg+ #176 Tainted: G O
--
jailhouse/6105 is trying to acquire lock:
dmar_pci_bus_notifier+0x4f/0xcb
but task is already holding lock:
__blocking_notifier_call_chain+0x31/0x65
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&(>bus_notifier)->rwsem){}:
__lock_acquire+0xed7/0x113b
lock_acquire+0x148/0x1f6
down_write+0x3b/0x6a
blocking_notifier_chain_register+0x33/0x53
bus_register_notifier+0x1c/0x1e
dmar_dev_scope_init+0x2c6/0x2db
intel_iommu_init+0xec/0x11c2
pci_iommu_init+0x17/0x41
do_one_initcall+0x90/0x143
kernel_init_freeable+0x1cc/0x256
kernel_init+0xe/0xf8
ret_from_fork+0x2a/0x40
-> #0 (dmar_global_lock){}:
check_prev_add+0x112/0x65f
__lock_acquire+0xed7/0x113b
lock_acquire+0x148/0x1f6
down_write+0x3b/0x6a
dmar_pci_bus_notifier+0x4f/0xcb
notifier_call_chain+0x3c/0x5e
__blocking_notifier_call_chain+0x4c/0x65
blocking_notifier_call_chain+0x14/0x16
device_add+0x40c/0x522
pci_device_add+0x1c0/0x1ce
pci_scan_single_device+0x92/0x9d
pci_scan_slot+0x59/0x10a
jailhouse_pci_do_all_devices+0x74/0x263 [jailhouse]
jailhouse_pci_virtual_root_devices_add+0x40/0x42 [jailhouse]
jailhouse_cmd_enable+0x4fd/0x5e8 [jailhouse]
jailhouse_ioctl+0x28/0x70 [jailhouse]
vfs_ioctl+0x18/0x34
do_vfs_ioctl+0x51b/0x5e3
SyS_ioctl+0x50/0x7b
entry_SYSCALL_64_fastpath+0x1f/0xbe
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0CPU1
lock(&(>bus_notifier)->rwsem);
lock(dmar_global_lock);
lock(&(>bus_notifier)->rwsem);
lock(dmar_global_lock);
*** DEADLOCK ***
2 locks held by jailhouse/6105:
jailhouse_cmd_enable+0x130/0x5e8 [jailhouse]
__blocking_notifier_call_chain+0x31/0x65
stack backtrace:
CPU: 1 PID: 6105 Comm: jailhouse Tainted: G O
4.14.0-rc2-dbg+ #176
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
Call Trace:
dump_stack+0x85/0xbe
print_circular_bug+0x389/0x398
? add_lock_to_list.isra.23+0x96/0x96
check_prev_add+0x112/0x65f
? kernel_text_address+0x1c/0x6a
? add_lock_to_list.isra.23+0x96/0x96
__lock_acquire+0xed7/0x113b
? __lock_acquire+0xed7/0x113b
lock_acquire+0x148/0x1f6
? dmar_pci_bus_notifier+0x4f/0xcb
down_write+0x3b/0x6a
? dmar_pci_bus_notifier+0x4f/0xcb
dmar_pci_bus_notifier+0x4f/0xcb
notifier_call_chain+0x3c/0x5e
__blocking_notifier_call_chain+0x4c/0x65
blocking_notifier_call_chain+0x14/0x16
device_add+0x40c/0x522
pci_device_add+0x1c0/0x1ce
pci_scan_single_device+0x92/0x9d
pci_scan_slot+0x59/0x10a
jailhouse_pci_do_all_devices+0x74/0x263 [jailhouse]
jailhouse_pci_virtual_root_devices_add+0x40/0x42 [jailhouse]
jailhouse_cmd_enable+0x4fd/0x5e8 [jailhouse]
jailhouse_ioctl+0x28/0x70 [jailhouse]
vfs_ioctl+0x18/0x34
do_vfs_ioctl+0x51b/0x5e3
? kmem_cache_free+0x15b/0x1fa
? entry_SYSCALL_64_fastpath+0x5/0xbe
? trace_hardirqs_on_caller+0x180/0x19c
SyS_ioctl+0x50/0x7b
entry_SYSCALL_64_fastpath+0x1f/0xbe
RIP: 0033:0x7f8b3b110d87
RSP: 002b:7ffc44b70088 EFLAGS: 0206 ORIG_RAX: 0010
RAX: ffda RBX: 0046 RCX: 7f8b3b110d87
RDX: 00604010 RSI: 4008 RDI: 0003
RBP: 00604010 R08: 7f8b3b3ade80 R09: 000885d0
R10: 7ffc44b6fe40 R11: 0206 R12: 25d4
R13: R14: 7ffc44b714a4 R15:
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
intel-iommu: possible circular locking dependency?
Hi,
this trigger for this is out-of-tree [1], but my understanding so far is
that this is an in-tree issue:
[ 14.841871] ==
[ 14.841874] WARNING: possible circular locking dependency detected
[ 14.841878] 4.13.0-rc2-dbg+ #174 Tainted: G O
[ 14.841881] --
[ 14.841884] jailhouse/6120 is trying to acquire lock:
[ 14.841887] (dmar_global_lock){..}, at: []
dmar_pci_bus_notifier+0x4f/0xcb
[ 14.841907]
[ 14.841907] but task is already holding lock:
[ 14.841910] (&(>bus_notifier)->rwsem){..}, at:
[] __blocking_notifier_call_chain+0x31/0x65
[ 14.841925]
[ 14.841925] which lock already depends on the new lock.
[ 14.841925]
[ 14.841930]
[ 14.841930] the existing dependency chain (in reverse order) is:
[ 14.841933]
[ 14.841933] -> #1 (&(>bus_notifier)->rwsem){..}:
[ 14.841943]lock_acquire+0x148/0x1f6
[ 14.841951]down_write+0x3b/0x6a
[ 14.841954]blocking_notifier_chain_register+0x33/0x53
[ 14.841960]bus_register_notifier+0x1c/0x1e
[ 14.841972]dmar_dev_scope_init+0x2c6/0x2db
[ 14.841976]intel_iommu_init+0xeb/0x12cb
[ 14.841983]pci_iommu_init+0x17/0x41
[ 14.841989]do_one_initcall+0x90/0x143
[ 14.841994]kernel_init_freeable+0x1cc/0x256
[ 14.841999]kernel_init+0xe/0xf8
[ 14.842004]ret_from_fork+0x2a/0x40
[ 14.842008]
[ 14.842008] -> #0 (dmar_global_lock){..}:
[ 14.842017]__lock_acquire+0xfe4/0x1521
[ 14.842021]lock_acquire+0x148/0x1f6
[ 14.842026]down_write+0x3b/0x6a
[ 14.842031]dmar_pci_bus_notifier+0x4f/0xcb
[ 14.842036]notifier_call_chain+0x3c/0x5e
[ 14.842041]__blocking_notifier_call_chain+0x4c/0x65
[ 14.842046]blocking_notifier_call_chain+0x14/0x16
[ 14.842051]device_add+0x40c/0x522
[ 14.842062]pci_device_add+0x1c0/0x1ce
[ 14.842066]pci_scan_single_device+0x92/0x9d
[ 14.842071]pci_scan_slot+0x59/0xff
[ 14.842078]jailhouse_pci_do_all_devices+0x74/0x263 [jailhouse]
[ 14.842084]jailhouse_pci_virtual_root_devices_add+0x40/0x42
[jailhouse]
[ 14.842090]jailhouse_cmd_enable+0x4fd/0x5e3 [jailhouse]
[ 14.842096]jailhouse_ioctl+0x28/0x70 [jailhouse]
[ 14.842106]vfs_ioctl+0x18/0x34
[ 14.842111]do_vfs_ioctl+0x512/0x5da
[ 14.842116]SyS_ioctl+0x50/0x7b
[ 14.842121]entry_SYSCALL_64_fastpath+0x1f/0xbe
[ 14.842125]
[ 14.842125] other info that might help us debug this:
[ 14.842125]
[ 14.842134] Possible unsafe locking scenario:
[ 14.842134]
[ 14.842140]CPU0CPU1
[ 14.842144]
[ 14.842148] lock(&(>bus_notifier)->rwsem);
[ 14.842155]lock(dmar_global_lock);
[ 14.842160]
lock(&(>bus_notifier)->rwsem);
[ 14.842166] lock(dmar_global_lock);
[ 14.842170]
[ 14.842170] *** DEADLOCK ***
[ 14.842170]
[ 14.842179] 2 locks held by jailhouse/6120:
[ 14.842183] #0: (jailhouse_lock){+.+.+.}, at: []
jailhouse_cmd_enable+0x130/0x5e3 [jailhouse]
[ 14.842194] #1: (&(>bus_notifier)->rwsem){..}, at:
[] __blocking_notifier_call_chain+0x31/0x65
[ 14.842204]
[ 14.842204] stack backtrace:
[ 14.842211] CPU: 0 PID: 6120 Comm: jailhouse Tainted: G O
4.13.0-rc2-dbg+ #174
[ 14.842217] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014
[ 14.842225] Call Trace:
[ 14.842232] dump_stack+0x85/0xbe
[ 14.842237] print_circular_bug+0x29b/0x2a9
[ 14.842243] __lock_acquire+0xfe4/0x1521
[ 14.842248] ? save_trace+0x3b/0x9b
[ 14.842253] lock_acquire+0x148/0x1f6
[ 14.842257] ? lock_acquire+0x148/0x1f6
[ 14.842262] ? dmar_pci_bus_notifier+0x4f/0xcb
[ 14.842268] down_write+0x3b/0x6a
[ 14.842272] ? dmar_pci_bus_notifier+0x4f/0xcb
[ 14.842277] dmar_pci_bus_notifier+0x4f/0xcb
[ 14.842282] notifier_call_chain+0x3c/0x5e
[ 14.842287] __blocking_notifier_call_chain+0x4c/0x65
[ 14.842292] blocking_notifier_call_chain+0x14/0x16
[ 14.842297] device_add+0x40c/0x522
[ 14.842302] pci_device_add+0x1c0/0x1ce
[ 14.842306] pci_scan_single_device+0x92/0x9d
[ 14.842311] pci_scan_slot+0x59/0xff
[ 14.842316] jailhouse_pci_do_all_devices+0x74/0x263 [jailhouse]
[ 14.842322] jailhouse_pci_virtual_root_devices_add+0x40/0x42 [jailhouse]
[ 14.842329] jailhouse_cmd_enable+0x4fd/0x5e3 [jailhouse]
[ 14.842337] jailhouse_ioctl+0x28/0x70 [jailhouse]
[ 14.842342] vfs_ioctl+0x18/0x34
[ 14.842346] do_vfs_ioctl+0x512/0x5da
[ 14.842352] ? kmem_cache_free+0x15b/0x1fa
[ 14.842357] ? entry_SYSCALL_64_fastpath+0x5/0xbe
[ 14.842362] ? trace_hardirqs_on_caller+0x180/0x19c
[ 14.842367] SyS_ioctl+0x50/0x7b
Re: [RFC] iommu/vt-d: Use the SIRTP when enabling remapping
On 2014-09-09 23:11, Nathan Zimmer wrote: The previous change (iommu/vt-d: Don't store SIRTP request) to this area caused a crash in our simulator. In particular is seems that by the time a UART interrupt is sent through the system, we don't see interrupt remapping to be enabled. So the interrupt does not get translated to a logical interrupt and crashes. OR'ing the SIRTP request to make sure it is seen but hopefully not sticky. This seems like a clean fix, at least on our simulator; if you don't agree, our simulator guy will take a closer look at our iommu model. Check the VT-d spec (6.7, Set Interrupt Remapping Table Pointer Operation): Software must always follow the interrupt-remapping table pointer set operation with a global invalidate of the IEC to ensure hardware references the new structures *before* enabling interrupt remapping. There is also (command register description): If multiple control fields in this register need to be modified, software must serialize the modifications through multiple writes to this register. This, in combination with the command registers description of bits 24 and 25 strongly suggests that your model is broken. Found testing on our simulator, not real hardware. Funnily, the original issue was found by the QEMU model of VT-d that used to take the last cited sentence very strictly (I asked to remove it due to the preexisting Linux versions). Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/vt-d: Don't store SIRTP request
Don't store the SIRTP request bit in the register state. It will otherwise become sticky and could request an Interrupt Remap Table Pointer update on each command register write. Found while starting to emulate IR in QEMU, not by observing problems on real hardware. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- drivers/iommu/intel_irq_remapping.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c index 0df41f6..a872874 100644 --- a/drivers/iommu/intel_irq_remapping.c +++ b/drivers/iommu/intel_irq_remapping.c @@ -438,8 +438,7 @@ static void iommu_set_irq_remapping(struct intel_iommu *iommu, int mode) (addr) | IR_X2APIC_MODE(mode) | INTR_REMAP_TABLE_REG_SIZE); /* Set interrupt-remapping table pointer */ - iommu-gcmd |= DMA_GCMD_SIRTP; - writel(iommu-gcmd, iommu-reg + DMAR_GCMD_REG); + writel(iommu-gcmd | DMA_GCMD_SIRTP, iommu-reg + DMAR_GCMD_REG); IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, readl, (sts DMA_GSTS_IRTPS), sts); -- 1.8.1.1.298.ge7eed54 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
