RE: [RFC v1 0/2] vfio/pci: expose device's PASID capability to VMs

2020-02-07 Thread Liu, Yi L
Hi Alex,

Any comment on this series?

Regards,
Yi Liu

> From: Liu, Yi L
> Sent: Wednesday, January 29, 2020 8:19 PM
> To: alex.william...@redhat.com; eric.au...@redhat.com
> Subject: [RFC v1 0/2] vfio/pci: expose device's PASID capability to VMs
> 
> Shared Virtual Addressing (SVA), a.k.a, Shared Virtual Memory (SVM) on Intel
> platforms allows address space sharing between device DMA and applications. 
> SVA
> can reduce programming complexity and enhance security.
> 
> To enable SVA, device needs to have PASID capability, which is a key 
> capability for
> SVA. This patchset exposes the device's PASID capability to guest instead of 
> hiding it
> from guest.
> 
> The second patch emulates PASID capability for VFs (Virtual Function) since 
> VFs don't
> implement such capability per PCIe spec. This patch emulates such capability 
> and
> expose to VM if the capability is enabled in PF (Physical Function).
> 
> However, there is an open for PASID emulation. If PF driver disables PASID 
> capability
> at runtime, then it may be an issue. e.g. PF should not disable PASID 
> capability if
> there is guest using this capability on any VF related to this PF. To solve 
> it, may need
> to introduce a generic communication framework between vfio-pci driver and PF
> drivers. Please feel free to give your suggestions on it.
> 
> Liu Yi L (2):
>   vfio/pci: Expose PCIe PASID capability to guest
>   vfio/pci: Emulate PASID/PRI capability for VFs
> 
>  drivers/vfio/pci/vfio_pci_config.c | 321
> -
>  1 file changed, 318 insertions(+), 3 deletions(-)
> 
> --
> 2.7.4

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


Re: warning from domain_get_iommu

2020-02-07 Thread Lu Baolu

Hi Jerry,

On 2020/2/7 17:34, Jerry Snitselaar wrote:

On Thu Feb 06 20, Jerry Snitselaar wrote:

On Tue Feb 04 20, Jerry Snitselaar wrote:
I'm working on getting a system to reproduce this, and verify it also 
occurs

with 5.5, but I have a report of a case where the kdump kernel gives
warnings like the following on a hp dl360 gen9:

[    2.830589] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) 
Driver

[    2.832615] ehci-pci: EHCI PCI platform driver
[    2.834190] ehci-pci :00:1a.0: EHCI Host Controller
[    2.835974] ehci-pci :00:1a.0: new USB bus registered, 
assigned bus number 1

[    2.838276] ehci-pci :00:1a.0: debug port 2
[    2.839700] WARNING: CPU: 0 PID: 1 at 
drivers/iommu/intel-iommu.c:598 domain_get_iommu+0x55/0x60

[    2.840671] Modules linked in:
[    2.840671] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
4.18.0-170.el8.kdump2.x86_64 #1
[    2.840671] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 
Gen9, BIOS P89 07/21/2019

[    2.840671] RIP: 0010:domain_get_iommu+0x55/0x60
[    2.840671] Code: c2 01 eb 0b 48 83 c0 01 8b 34 87 85 f6 75 0b 48 
63 c8 48 39 c2 75 ed 31 c0 c3 48 c1 e1 03 48 8b 05 70 f3 91 01 48 8b 
04 08 c3 <0f> 0b 31 c0 c3 31 c9 eb eb 66 90 0f 1f 44 00 00 41 55 40 
0f b6 f6

[    2.840671] RSP: 0018:c90dfab8 EFLAGS: 00010202
[    2.840671] RAX: 88ec7f1c8000 RBX: 006c7c867000 RCX: 

[    2.840671] RDX: fff0 RSI:  RDI: 
88ec7f1c8000
[    2.840671] RBP: 88ec6f7000b0 R08: 88ec7f19d000 R09: 
88ec7cbfcd00
[    2.840671] R10: 0095 R11: c90df928 R12: 

[    2.840671] R13: 88ec7f1c8000 R14: 1000 R15: 

[    2.840671] FS:  () GS:88ec7f60() 
knlGS:

[    2.840671] CS:  0010 DS:  ES:  CR0: 80050033
[    2.840671] CR2: 7ff3e1713000 CR3: 006c7de0a004 CR4: 
001606b0

[    2.840671] Call Trace:
[    2.840671]  __intel_map_single+0x62/0x140
[    2.840671]  intel_alloc_coherent+0xa6/0x130
[    2.840671]  dma_pool_alloc+0xd8/0x1e0
[    2.840671]  e_qh_alloc+0x55/0x130
[    2.840671]  ehci_setup+0x284/0x7b0
[    2.840671]  ehci_pci_setup+0xa3/0x530
[    2.840671]  usb_add_hcd+0x2b6/0x800
[    2.840671]  usb_hcd_pci_probe+0x375/0x460
[    2.840671]  local_pci_probe+0x41/0x90
[    2.840671]  pci_device_probe+0x105/0x1b0
[    2.840671]  driver_probe_device+0x12d/0x460
[    2.840671]  device_driver_attach+0x50/0x60
[    2.840671]  __driver_attach+0x61/0x130
[    2.840671]  ? device_driver_attach+0x60/0x60
[    2.840671]  bus_for_each_dev+0x77/0xc0
[    2.840671]  ? klist_add_tail+0x3b/0x70
[    2.840671]  bus_add_driver+0x14d/0x1e0
[    2.840671]  ? ehci_hcd_init+0xaa/0xaa
[    2.840671]  ? do_early_param+0x91/0x91
[    2.840671]  driver_register+0x6b/0xb0
[    2.840671]  ? ehci_hcd_init+0xaa/0xaa
[    2.840671]  do_one_initcall+0x46/0x1c3
[    2.840671]  ? do_early_param+0x91/0x91
[    2.840671]  kernel_init_freeable+0x1af/0x258
[    2.840671]  ? rest_init+0xaa/0xaa
[    2.840671]  kernel_init+0xa/0xf9
[    2.840671]  ret_from_fork+0x35/0x40
[    2.840671] ---[ end trace e87b0d9a1c8135c4 ]---
[    3.010848] ehci-pci :00:1a.0: Using iommu dma mapping
[    3.012551] ehci-pci :00:1a.0: 32bit DMA uses non-identity 
mapping
[    3.018537] ehci-pci :00:1a.0: cache line size of 64 is not 
supported

[    3.021188] ehci-pci :00:1a.0: irq 18, io mem 0x93002000
[    3.029006] ehci-pci :00:1a.0: USB 2.0 started, EHCI 1.00
[    3.030918] usb usb1: New USB device found, idVendor=1d6b, 
idProduct=0002, bcdDevice= 4.18
[    3.033491] usb usb1: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1

[    3.035900] usb usb1: Product: EHCI Host Controller
[    3.037423] usb usb1: Manufacturer: Linux 
4.18.0-170.el8.kdump2.x86_64 ehci_hcd

[    3.039691] usb usb1: SerialNumber: :00:1a.0

It looks like the device finishes initializing once it figures out it
needs dma mapping instead of the default
passthrough. intel_alloc_coherent calls iommu_need_mapping, before it
calls __intel_map_single, so I'm not sure why it is tripping over the
WARN_ON in domain_get_iommu.

one thing I noticed while looking at this is that domain_get_iommu can
return NULL. So should there be something like the following in
__intel_map_single after the domain_get_iommu call?

if (!iommu)
 goto error;

It is possible to deref the null pointer later otherwise.

Regards,
Jerry


I reproduced the warning with a 5.5 kernel on an Intel NUC5i5MYBE.


Hi Baolu,

I think I understand what is happening here. With the kdump boot
translation is pre-enabled, so in intel_iommu_add_device things are
getting set to DEFER_DEVICE_DOMAIN_INFO. When intel_alloc_coherent
calls iommu_need_mapping it returns true, but doesn't do the dma
domain switch because of DEFER_DEVICE_DOMAIN_INFO. Then
__intel_map_single gets called and it calls deferred_attach_domain,
which sets the domain to the group d

Re: [PATCH] iommu/intel-iommu: set as DUMMY_DEVICE_DOMAIN_INFO if no IOMMU

2020-02-07 Thread Lu Baolu

Hi,

On 2020/2/7 15:32, Jian-Hong Pan wrote:

Lu Baolu  於 2020年2月6日 週四 下午6:49寫道:


Hi,

On 2020/2/5 18:06, Jian-Hong Pan wrote:

Lu Baolu  於 2020年2月5日 週三 上午9:28寫道:


Hi,

On 2020/2/4 17:25, Jian-Hong Pan wrote:

Lu Baolu  於 2020年2月4日 週二 下午2:11寫道:


Hi,

On 2020/2/3 17:10, Jian-Hong Pan wrote:

If the device has no IOMMU, it still invokes iommu_need_mapping during
intel_alloc_coherent. However, iommu_need_mapping can only check the
device is DUMMY_DEVICE_DOMAIN_INFO or not. This patch marks the device
is a DUMMY_DEVICE_DOMAIN_INFO if the device has no IOMMU.

Signed-off-by: Jian-Hong Pan 
---
 drivers/iommu/intel-iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 35a4a3abedc6..878bc986a015 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5612,8 +5612,10 @@ static int intel_iommu_add_device(struct device *dev)
 int ret;

 iommu = device_to_iommu(dev, &bus, &devfn);
- if (!iommu)
+ if (!iommu) {
+ dev->archdata.iommu = DUMMY_DEVICE_DOMAIN_INFO;


Is this a DMA capable device?


Do you mean is the device in DMA Remapping table?
Dump DMAR from ACPI table.  The device is not in the table.
So, it does not support DMAR, Intel IOMMU.

Or, should device_to_iommu be invoked in iommu_need_mapping to check
IOMMU feature again?



Normally intel_iommu_add_device() should only be called for PCI devices
and APCI name space devices (reported in ACPI/DMAR table). In both
cases, device_to_iommu() should always return a corresponding iommu. I
just tried to understand why it failed for your case.


We found all of the DMAR featured devices's PCI Segment Number is **.
But the devices locating under segment/domain *0001* hit the issue,
until the patch is applied.

Because of different segment numbers, none of iommu will be matched by
for_each_active_iommu(iommu, drhd) loop in function device_to_iommu()
and it will return NULL.  So, intel_iommu_add_device() returns no
device.

I can share the DMAR:
/*
   * Intel ACPI Component Architecture
   * AML/ASL+ Disassembler version 20200110 (64-bit version)
   * Copyright (c) 2000 - 2020 Intel Corporation
   *
   * Disassembly of dmar.dat, Wed Jan 22 11:41:50 2020
   *
   * ACPI Data Table [DMAR]
   *
   * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue
   */

[000h    4]Signature : "DMAR"[DMA Remapping table]
[004h 0004   4] Table Length : 00A8
[008h 0008   1] Revision : 01
[009h 0009   1] Checksum : 5E
[00Ah 0010   6]   Oem ID : "INTEL "
[010h 0016   8] Oem Table ID : "EDK2"
[018h 0024   4] Oem Revision : 0002
[01Ch 0028   4]  Asl Compiler ID : ""
[020h 0032   4]Asl Compiler Revision : 0113

[024h 0036   1]   Host Address Width : 26
[025h 0037   1]Flags : 05
[026h 0038  10] Reserved : 00 00 00 00 00 00 00 00 00 00

[030h 0048   2]Subtable Type :  [Hardware Unit Definition]
[032h 0050   2]   Length : 0018

[034h 0052   1]Flags : 00
[035h 0053   1] Reserved : 00
[036h 0054   2]   PCI Segment Number : 
[038h 0056   8]Register Base Address : FED9

[040h 0064   1]Device Scope Type : 01 [PCI Endpoint Device]
[041h 0065   1] Entry Length : 08
[042h 0066   2] Reserved : 
[044h 0068   1]   Enumeration ID : 00
[045h 0069   1]   PCI Bus Number : 00

[046h 0070   2] PCI Path : 02,00


[048h 0072   2]Subtable Type :  [Hardware Unit Definition]
[04Ah 0074   2]   Length : 0020

[04Ch 0076   1]Flags : 01
[04Dh 0077   1] Reserved : 00
[04Eh 0078   2]   PCI Segment Number : 
[050h 0080   8]Register Base Address : FED91000

[058h 0088   1]Device Scope Type : 03 [IOAPIC Device]
[059h 0089   1] Entry Length : 08
[05Ah 0090   2] Reserved : 
[05Ch 0092   1]   Enumeration ID : 02
[05Dh 0093   1]   PCI Bus Number : 00

[05Eh 0094   2] PCI Path : 1E,07


[060h 0096   1]Device Scope Type : 04 [Message-capable HPET Device]
[061h 0097   1] Entry Length : 08
[062h 0098   2] Reserved : 
[064h 0100   1]   Enumeration ID : 00
[065h 0101   1]   PCI Bus Number : 00

[066h 0102   2] PCI Path : 1E,06


[068h 0104   2]Subtable Type : 0001 [Reserved Memory Region]
[06Ah 0106   2]   Length : 0020

[06Ch 0108   2] Reserved : 
[06Eh 0110   2]

Re: [RFC v3 2/8] vfio/type1: Make per-application (VM) PASID quota tunable

2020-02-07 Thread Jacob Pan
On Wed, 29 Jan 2020 04:11:46 -0800
"Liu, Yi L"  wrote:

> From: Liu Yi L 
> 
> The PASID quota is per-application (VM) according to vfio's PASID
> management rule. For better flexibility, quota shall be user tunable
> . This patch provides a VFIO based user interface for which quota can
> be adjusted. However, quota cannot be adjusted downward below the
> number of outstanding PASIDs.
> 
> This patch only makes the per-VM PASID quota tunable. While for the
> way to tune the default PASID quota, it may require a new vfio module
> option or other way. This may be another patchset in future.
> 
One issue we need to solve is how to share PASIDs at the system
level, e.g. Both VMs and baremetal drivers could use PASIDs.

This patch is granting quota to a guest w/o knowing the remaining
system capacity. So guest PASID allocation could fail even within its
quota.

The solution I am thinking is to enforce quota at IOASID common
code, since IOASID APIs already used to manage system-wide allocation.
How about the following changes to IOASID?
1. introduce quota in ioasid_set (could have a soft limit for better
sharing)

2. introduce an API to create a set with quota before allocation, e.g.
ioasid_set_id = ioasid_alloc_set(size, token)
set_id will be used for ioasid_alloc() instead of token.

3. introduce API to adjust set quota ioasid_adjust_set_size(set_id,
size)

4. API to check remaining PASIDs ioasid_get_capacity(set_id); //return
system capacity if set_id == 0;

5. API to set system capacity, ioasid_set_capacity(nr_pasids), e.g. if
system has 20 bit PASIDs, IOMMU driver needs to call
ioasid_set_capacity(1<<20) during boot.

6. Optional set level APIs. e.g. ioasid_free_set(set_id), frees all
IOASIDs in the set.

With these APIs, this patch could query PASID capacity at both system
and set level and adjust quota within range. i.e.
1. IOMMU vendor driver(or other driver to use PASID w/o IOMMU) sets
system wide capacity during boot.
2. VFIO Call ioasid_alloc_set() when allocating vfio_mm(), set default
quota
3. Adjust quota per set with ioasid_adjust_set_size() as the tunable in
this patch.

Thoughts?

> Previous discussions:
> https://patchwork.kernel.org/patch/11209429/
> 
> Cc: Kevin Tian 
> CC: Jacob Pan 
> Cc: Alex Williamson 
> Cc: Eric Auger 
> Cc: Jean-Philippe Brucker 
> Signed-off-by: Liu Yi L 
> ---
>  drivers/vfio/vfio_iommu_type1.c | 33
> + include/uapi/linux/vfio.h   |
> 22 ++ 2 files changed, 55 insertions(+)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c index e836d04..1cf75f5 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -2243,6 +2243,27 @@ static int vfio_iommu_type1_pasid_free(struct
> vfio_iommu *iommu, return ret;
>  }
>  
> +static int vfio_iommu_type1_set_pasid_quota(struct vfio_iommu *iommu,
> + u32 quota)
> +{
> + struct vfio_mm *vmm = iommu->vmm;
> + int ret = 0;
> +
> + mutex_lock(&iommu->lock);
> + mutex_lock(&vmm->pasid_lock);
> + if (vmm->pasid_count > quota) {
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> + vmm->pasid_quota = quota;
> + ret = quota;
> +
> +out_unlock:
> + mutex_unlock(&vmm->pasid_lock);
> + mutex_unlock(&iommu->lock);
> + return ret;
> +}
> +
>  static long vfio_iommu_type1_ioctl(void *iommu_data,
>  unsigned int cmd, unsigned long
> arg) {
> @@ -2389,6 +2410,18 @@ static long vfio_iommu_type1_ioctl(void
> *iommu_data, default:
>   return -EINVAL;
>   }
> + } else if (cmd == VFIO_IOMMU_SET_PASID_QUOTA) {
> + struct vfio_iommu_type1_pasid_quota quota;
> +
> + minsz = offsetofend(struct
> vfio_iommu_type1_pasid_quota,
> + quota);
> +
> + if (copy_from_user("a, (void __user *)arg,
> minsz))
> + return -EFAULT;
> +
> + if (quota.argsz < minsz)
> + return -EINVAL;
> + return vfio_iommu_type1_set_pasid_quota(iommu,
> quota.quota); }
>  
>   return -ENOTTY;
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 298ac80..d4bf415 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -835,6 +835,28 @@ struct vfio_iommu_type1_pasid_request {
>   */
>  #define VFIO_IOMMU_PASID_REQUEST _IO(VFIO_TYPE, VFIO_BASE +
> 22) 
> +/**
> + * @quota: the new pasid quota which a userspace application (e.g.
> VM)
> + * is configured.
> + */
> +struct vfio_iommu_type1_pasid_quota {
> + __u32   argsz;
> + __u32   flags;
> + __u32   quota;
> +};
> +
> +/**
> + * VFIO_IOMMU_SET_PASID_QUOTA - _IOW(VFIO_TYPE, VFIO_BASE + 23,
> + *   struct
> vfio_iommu_type1_pasid_quota)
> + *
> + * Availability of this feature depends on PASID support in the
> device,
> + * its bus, t

Re: warning from domain_get_iommu

2020-02-07 Thread Jerry Snitselaar

On Thu Feb 06 20, Jerry Snitselaar wrote:

On Tue Feb 04 20, Jerry Snitselaar wrote:

I'm working on getting a system to reproduce this, and verify it also occurs
with 5.5, but I have a report of a case where the kdump kernel gives
warnings like the following on a hp dl360 gen9:

[2.830589] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[2.832615] ehci-pci: EHCI PCI platform driver
[2.834190] ehci-pci :00:1a.0: EHCI Host Controller
[2.835974] ehci-pci :00:1a.0: new USB bus registered, assigned bus 
number 1
[2.838276] ehci-pci :00:1a.0: debug port 2
[2.839700] WARNING: CPU: 0 PID: 1 at drivers/iommu/intel-iommu.c:598 
domain_get_iommu+0x55/0x60
[2.840671] Modules linked in:
[2.840671] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
4.18.0-170.el8.kdump2.x86_64 #1
[2.840671] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9, BIOS 
P89 07/21/2019
[2.840671] RIP: 0010:domain_get_iommu+0x55/0x60
[2.840671] Code: c2 01 eb 0b 48 83 c0 01 8b 34 87 85 f6 75 0b 48 63 c8 48 39 c2 
75 ed 31 c0 c3 48 c1 e1 03 48 8b 05 70 f3 91 01 48 8b 04 08 c3 <0f> 0b 31 c0 c3 
31 c9 eb eb 66 90 0f 1f 44 00 00 41 55 40 0f b6 f6
[2.840671] RSP: 0018:c90dfab8 EFLAGS: 00010202
[2.840671] RAX: 88ec7f1c8000 RBX: 006c7c867000 RCX: 
[2.840671] RDX: fff0 RSI:  RDI: 88ec7f1c8000
[2.840671] RBP: 88ec6f7000b0 R08: 88ec7f19d000 R09: 88ec7cbfcd00
[2.840671] R10: 0095 R11: c90df928 R12: 
[2.840671] R13: 88ec7f1c8000 R14: 1000 R15: 
[2.840671] FS:  () GS:88ec7f60() 
knlGS:
[2.840671] CS:  0010 DS:  ES:  CR0: 80050033
[2.840671] CR2: 7ff3e1713000 CR3: 006c7de0a004 CR4: 001606b0
[2.840671] Call Trace:
[2.840671]  __intel_map_single+0x62/0x140
[2.840671]  intel_alloc_coherent+0xa6/0x130
[2.840671]  dma_pool_alloc+0xd8/0x1e0
[2.840671]  e_qh_alloc+0x55/0x130
[2.840671]  ehci_setup+0x284/0x7b0
[2.840671]  ehci_pci_setup+0xa3/0x530
[2.840671]  usb_add_hcd+0x2b6/0x800
[2.840671]  usb_hcd_pci_probe+0x375/0x460
[2.840671]  local_pci_probe+0x41/0x90
[2.840671]  pci_device_probe+0x105/0x1b0
[2.840671]  driver_probe_device+0x12d/0x460
[2.840671]  device_driver_attach+0x50/0x60
[2.840671]  __driver_attach+0x61/0x130
[2.840671]  ? device_driver_attach+0x60/0x60
[2.840671]  bus_for_each_dev+0x77/0xc0
[2.840671]  ? klist_add_tail+0x3b/0x70
[2.840671]  bus_add_driver+0x14d/0x1e0
[2.840671]  ? ehci_hcd_init+0xaa/0xaa
[2.840671]  ? do_early_param+0x91/0x91
[2.840671]  driver_register+0x6b/0xb0
[2.840671]  ? ehci_hcd_init+0xaa/0xaa
[2.840671]  do_one_initcall+0x46/0x1c3
[2.840671]  ? do_early_param+0x91/0x91
[2.840671]  kernel_init_freeable+0x1af/0x258
[2.840671]  ? rest_init+0xaa/0xaa
[2.840671]  kernel_init+0xa/0xf9
[2.840671]  ret_from_fork+0x35/0x40
[2.840671] ---[ end trace e87b0d9a1c8135c4 ]---
[3.010848] ehci-pci :00:1a.0: Using iommu dma mapping
[3.012551] ehci-pci :00:1a.0: 32bit DMA uses non-identity mapping
[3.018537] ehci-pci :00:1a.0: cache line size of 64 is not supported
[3.021188] ehci-pci :00:1a.0: irq 18, io mem 0x93002000
[3.029006] ehci-pci :00:1a.0: USB 2.0 started, EHCI 1.00
[3.030918] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002, 
bcdDevice= 4.18
[3.033491] usb usb1: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[3.035900] usb usb1: Product: EHCI Host Controller
[3.037423] usb usb1: Manufacturer: Linux 4.18.0-170.el8.kdump2.x86_64 
ehci_hcd
[3.039691] usb usb1: SerialNumber: :00:1a.0

It looks like the device finishes initializing once it figures out it
needs dma mapping instead of the default
passthrough. intel_alloc_coherent calls iommu_need_mapping, before it
calls __intel_map_single, so I'm not sure why it is tripping over the
WARN_ON in domain_get_iommu.

one thing I noticed while looking at this is that domain_get_iommu can
return NULL. So should there be something like the following in
__intel_map_single after the domain_get_iommu call?

if (!iommu)
 goto error;

It is possible to deref the null pointer later otherwise.

Regards,
Jerry


I reproduced the warning with a 5.5 kernel on an Intel NUC5i5MYBE.


Hi Baolu,

I think I understand what is happening here. With the kdump boot
translation is pre-enabled, so in intel_iommu_add_device things are
getting set to DEFER_DEVICE_DOMAIN_INFO. When intel_alloc_coherent
calls iommu_need_mapping it returns true, but doesn't do the dma
domain switch because of DEFER_DEVICE_DOMAIN_INFO. Then
__intel_map_single gets called and it calls deferred_attach_domain,
which sets the domain to the group domain, which in this case is the
identity domain. Then it calls domain_get_i

Re: [PATCH 3/3] iommu/uapi: Add helper function for size lookup

2020-02-07 Thread Jean-Philippe Brucker
On Mon, Feb 03, 2020 at 02:41:02PM -0800, Jacob Pan wrote:
> Yeah, that would work as well. I just feel IOMMU UAPI is unlikely to get
> updated frequently, should be much less than adding new capabilities.
> I think argsz could be viewed as the version field set by the
> user, minsz is what kernel current code supports.
> 
> So let me summarize the options we have
> 1. Disallow adding new members to each structure other than reuse
> padding bits or adding union members at the end.
> 2. Allow extension of the structures beyond union, but union size has
> to be fixed with reserved spaces
> 3. Adopt VFIO argsz scheme, I don't think we need version for each
> struct anymore. argsz implies the version that user is using assuming
> UAPI data is extension only.
> 
> Jean, Eric, any comments? My preference is #1. In the apocalyptic event
> when we run out of padding, perhaps we can introduce a new API_v2 :)

I agree, new extensions will most likely want to extend the vendor
specific structures at the end rather than introduce new common fields, so
I prefer #1 which avoids fixing the union size.

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