Re: [PATCH 4/4] iommu/vt-d: Remove lazy allocation of domains
Hi James,
On 3/7/19 6:21 PM, James Sewart wrote:
Hey Lu,
On 7 Mar 2019, at 06:31, Lu Baolu wrote:
Hi James,
On 3/7/19 2:08 AM, James Sewart wrote:
- /*
-* For each rmrr
-* for each dev attached to rmrr
-* do
-* locate drhd for dev, alloc domain for dev
-* allocate free domain
-* allocate page table entries for rmrr
-* if context not allocated for bus
-* allocate and init context
-* set present in root table for this bus
-* init context with domain, translation etc
-*endfor
-* endfor
-*/
- pr_info("Setting RMRR:\n");
- for_each_rmrr_units(rmrr) {
- /* some BIOS lists non-exist devices in DMAR table. */
- for_each_active_dev_scope(rmrr->devices, rmrr->devices_cnt,
- i, dev) {
- ret = iommu_prepare_rmrr_dev(rmrr, dev);
- if (ret)
- pr_err("Mapping reserved region failed\n");
- }
- }
-
- iommu_prepare_isa();
Why do you want to remove this segment of code?
This will only work if the lazy allocation of domains exists, these
mappings will disappear once a default domain is attached to a device and
then remade by iommu_group_create_direct_mappings. This code is redundant
and removing it allows us to remove all the lazy allocation logic.
No exactly.
We need to setup the rmrr mapping before enabling dma remapping,
otherwise, there will be a window after dma remapping enabling and
rmrr getting mapped, during which people might see dma faults.
Do you think this patch instead should be a refactoring to simplify this
initial domain setup before the default domain takes over? It seems like
duplicated effort to have both lazy allocated domains and externally managed
domains. We could allocate a domain here for any devices with RMRR and call
get_resv_regions to avoid duplicating RMRR loop.
Agree. We should replace the lazy allocated domains with default domain
in a clean way. Actually, your patches look great to me. But I do think
we need further cleanups. The rmrr code is one example, and the identity
domain (si_domain) is another.
Do you mind if I work on top of your patches for further cleanups and
sign off a v2 together with you?
Sure, sounds good. I’ll fixup patch 3 and have a go at integrating
iommu_prepare_isa into get_resv_regions. This should make the initial
domain logic here quite concise.
Here attached three extra patches which I think should be added before
PATCH 3/4, and some further cleanup changes which you can merge with
PATCH 4/4.
0006-iommu-Add-ops-entry-for-vendor-specific-default-doma.patch
0007-iommu-vt-d-Add-is_identity_map-ops-entry.patch
These two patches aim to add a generic method for vendor specific iommu
drivers to specify the type of the default domain for a device. Intel
iommu driver will register an ops for this since it already has its own
identity map adjudicator for a long time.
0008-iommu-vt-d-Enable-DMA-remapping-after-rmrr-mapped.patch
This aims to address the requirement of rmrr identity map before
enabling DMA remapping. With default domain mechanism, the default
domain will be allocated and attached to the device in bus_set_iommu()
during boot. Move enabling DMA remapping engines after bus_set_iommu()
will fix the rmrr issue.
0009-return-si_domain-directly.patch
I suggest that we should keep current si_domain implementation since
changing si_domain is not the purpose of this patch set. Please merge
this with PATCH 3/4 if you like it.
0010-iommu-vt-d-remove-floopy-workaround.patch
0011-iommu-vt-d-remove-prepare-rmrr-helpers.patch
0012-iommu-vt-d-remove-prepare-identity-map-during-boot.patch
Above patches are further cleanups as the result of switching to default
domain. Please consider to merge them with PATCH 4/4.
I have done some sanity checks on my local machine against all patches.
I can do more tests after you submit a v2.
Best regards,
Lu Baolu
>From d1625f1e8461cef23bc8697ec51382c47b92fa5a Mon Sep 17 00:00:00 2001
From: Lu Baolu
Date: Thu, 7 Mar 2019 10:50:27 +0800
Subject: [PATCH 06/12] iommu: Add ops entry for vendor specific default domain
type
This adds an iommu ops entry for iommu vendor driver to specify
the type of the default domain for each device. This is needed
for the vendor driver, which already has its own logic to
determine the use of identity domain for a long time, when it
switches to apply default domain.
Signed-off-by: Lu Baolu
---
drivers/iommu/iommu.c | 12 +---
include/linux/iommu.h | 4
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 374327018a11..4101f38a7844 100644
--- a/drivers/iommu/iommu.c
+++
Re: [PATCH 3/4] iommu/vt-d: Allow IOMMU_DOMAIN_DMA and IOMMU_DOMAIN_IDENTITY to be allocated
On 3/4/19 3:46 PM, James Sewart wrote:
> +static inline int domain_is_initialised(struct dmar_domain *domain)
> +{
> + return domain->flags & DOMAIN_FLAG_INITIALISED;
> +}
Maybe check it in intel_iommu_domain_free(), eh?
Thanks,
Dmitry
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] iommu/vt-d: Remove lazy allocation of domains
Hi,
On 3/7/19 6:21 PM, James Sewart wrote:
Hey Lu,
On 7 Mar 2019, at 06:31, Lu Baolu wrote:
Hi James,
On 3/7/19 2:08 AM, James Sewart wrote:
- /*
-* For each rmrr
-* for each dev attached to rmrr
-* do
-* locate drhd for dev, alloc domain for dev
-* allocate free domain
-* allocate page table entries for rmrr
-* if context not allocated for bus
-* allocate and init context
-* set present in root table for this bus
-* init context with domain, translation etc
-*endfor
-* endfor
-*/
- pr_info("Setting RMRR:\n");
- for_each_rmrr_units(rmrr) {
- /* some BIOS lists non-exist devices in DMAR table. */
- for_each_active_dev_scope(rmrr->devices, rmrr->devices_cnt,
- i, dev) {
- ret = iommu_prepare_rmrr_dev(rmrr, dev);
- if (ret)
- pr_err("Mapping reserved region failed\n");
- }
- }
-
- iommu_prepare_isa();
Why do you want to remove this segment of code?
This will only work if the lazy allocation of domains exists, these
mappings will disappear once a default domain is attached to a device and
then remade by iommu_group_create_direct_mappings. This code is redundant
and removing it allows us to remove all the lazy allocation logic.
No exactly.
We need to setup the rmrr mapping before enabling dma remapping,
otherwise, there will be a window after dma remapping enabling and
rmrr getting mapped, during which people might see dma faults.
Do you think this patch instead should be a refactoring to simplify this
initial domain setup before the default domain takes over? It seems like
duplicated effort to have both lazy allocated domains and externally managed
domains. We could allocate a domain here for any devices with RMRR and call
get_resv_regions to avoid duplicating RMRR loop.
Agree. We should replace the lazy allocated domains with default domain
in a clean way. Actually, your patches look great to me. But I do think
we need further cleanups. The rmrr code is one example, and the identity
domain (si_domain) is another.
Do you mind if I work on top of your patches for further cleanups and
sign off a v2 together with you?
Sure, sounds good. I’ll fixup patch 3 and have a go at integrating
iommu_prepare_isa into get_resv_regions. This should make the initial
domain logic here quite concise.
Very appreciated! Thank you!
Best regards,
Lu Baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 9/9] vfio/type1: Handle different mdev isolation type
On Thu, 7 Mar 2019 00:44:54 -0800 Neo Jia wrote: > On Fri, Feb 22, 2019 at 10:19:27AM +0800, Lu Baolu wrote: > > This adds the support to determine the isolation type > > of a mediated device group by checking whether it has > > an iommu device. If an iommu device exists, an iommu > > domain will be allocated and then attached to the iommu > > device. Otherwise, keep the same behavior as it is. > > > > Cc: Ashok Raj > > Cc: Jacob Pan > > Cc: Kevin Tian > > Signed-off-by: Sanjay Kumar > > Signed-off-by: Liu Yi L > > Signed-off-by: Lu Baolu > > Reviewed-by: Jean-Philippe Brucker > > --- > > drivers/vfio/vfio_iommu_type1.c | 48 - > > 1 file changed, 41 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/vfio/vfio_iommu_type1.c > > b/drivers/vfio/vfio_iommu_type1.c > > index ccc4165474aa..f1392c582a3c 100644 > > --- a/drivers/vfio/vfio_iommu_type1.c > > +++ b/drivers/vfio/vfio_iommu_type1.c > > @@ -1368,13 +1368,40 @@ static void vfio_iommu_detach_group(struct > > vfio_domain *domain, > > iommu_detach_group(domain->domain, group->iommu_group); > > } > > > > Hi Baolu, > > To allow IOMMU-awared mdev, I think you need to modify the > vfio_iommu_type1_pin_pages and vfio_iommu_type1_unpin_pages to remove the > iommu->external_domain check. > > Could you please include that in your patch? If not, I can send out a separate > patch to address that issue. I figured it was intentional that an IOMMU backed mdev would not use the pin/unpin interface and therefore the exiting -EINVAL returns would be correct. Can you elaborate on the use case for still requiring the mdev pin/unpin interface for these devices? Thanks, Alex ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH/RFC] driver core: Postpone DMA tear-down until after devres release
On Thu, Mar 07, 2019 at 02:52:55PM +, Robin Murphy wrote: > Hi John, > > On 07/03/2019 14:45, John Garry wrote: > [...] > > Hi guys, > > > > Any idea what happened to this fix? > > It's been in -next for a while (commit 376991db4b64) - I assume it will land > shortly and hit stable thereafter, at which point somebody gets to sort out > the manual backport past 4.20. It's already in Linus's tree, I'll go try to do the stable backporting now... greg k-h ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH/RFC] driver core: Postpone DMA tear-down until after devres release
On 07/03/2019 14:52, Robin Murphy wrote: Hi John, Hi Robin, ok, fine. It's a pain bisecting another rmmod issue with it... Cheers, On 07/03/2019 14:45, John Garry wrote: [...] Hi guys, Any idea what happened to this fix? It's been in -next for a while (commit 376991db4b64) - I assume it will land shortly and hit stable thereafter, at which point somebody gets to sort out the manual backport past 4.20. Robin. I have this on 5.0: [0.00] Linux version 5.0.0 (john@htsatcamb-server) (gcc version 4.8.5 (Linaro GCC 4.8-2015.06)) #121 SMP PREEMPT Thu Mar 7 14:28:39 GMT 2019 [0.00] Kernel command line: BOOT_IMAGE=/john/Image rdinit=/init crashkernel=256M@32M earlycon console=ttyAMA0,115200 acpi=force pcie_aspm=off scsi_mod.use_blk_mq=y no_console_suspend pcie-hisi.disable=1 ... [ 26.806856] pci_bus 000c:20: 2-byte config write to 000c:20:00.0 offset 0x44 may corrupt adjacent RW1C bits [ 26.817521] pcieport 0002:f8:00.0: can't derive routing for PCI INT B [ 26.837167] pci_bus 000c:20: 2-byte config write to 000c:20:00.0 offset 0x44 may corrupt adjacent RW1C bits [ 26.850091] serial 0002:f9:00.1: PCI INT B: no GSI [ 26.879364] serial 0002:f9:00.1: enabling device ( -> 0003) [ 26.913131] 0002:f9:00.1: ttyS3 at I/O 0x1008 (irq = 0, base_baud = 115200) is a ST16650V2 [ 26.992897] rtc-efi rtc-efi: setting system clock to 2019-03-07T14:41:48 UTC (1551969708) [ 27.009380] ALSA device list: [ 27.015326] No soundcards found. [ 27.022549] Freeing unused kernel memory: 1216K [ 27.055567] Run /init as init process root@(none)$ cd /sys/devices/platform/HISI0162:01 root@(none)$ echo HISI0162:01 > driver/unbind [ 36.488040] hisi_sas_v2_hw HISI0162:01: dev[9:1] is gone [ 36.561077] hisi_sas_v2_hw HISI0162:01: dev[8:1] is gone [ 36.621061] hisi_sas_v2_hw HISI0162:01: dev[7:1] is gone [ 36.693074] hisi_sas_v2_hw HISI0162:01: dev[6:1] is gone [ 36.753066] hisi_sas_v2_hw HISI0162:01: dev[5:1] is gone [ 36.764276] sd 0:0:3:0: [sdd] Synchronizing SCSI cache [ 36.821106] hisi_sas_v2_hw HISI0162:01: dev[4:1] is gone [ 36.889048] hisi_sas_v2_hw HISI0162:01: dev[3:1] is gone [ 36.993002] hisi_sas_v2_hw HISI0162:01: dev[1:1] is gone [ 37.004276] sd 0:0:0:0: [sda] Synchronizing SCSI cache [ 37.014768] sd 0:0:0:0: [sda] Stopping disk [ 37.709094] hisi_sas_v2_hw HISI0162:01: dev[2:5] is gone [ 37.721231] hisi_sas_v2_hw HISI0162:01: dev[0:2] is gone [ 37.803774] BUG: Bad page state in process sh pfn:11356 [ 37.81] page:7e44d580 count:1 mapcount:0 mapping: index:0x0 [ 37.830525] flags: 0xfffc0001000(reserved) [ 37.839443] raw: 0fffc0001000 7e44d588 7e44d588 [ 37.854998] raw: 0001 [ 37.870552] page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set [ 37.883485] bad because of flags: 0x1000(reserved) [ 37.893098] Modules linked in: [ 37.899221] CPU: 5 PID: 2691 Comm: sh Not tainted 5.0.0 #121 [ 37.910578] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018 [ 37.928924] Call trace: [ 37.933825] dump_backtrace+0x0/0x150 [ 37.941166] show_stack+0x14/0x1c [ 37.947808] dump_stack+0x8c/0xb0 [ 37.954451] bad_page+0xe4/0x144 [ 37.960918] free_pages_check_bad+0x7c/0x84 [ 37.969307] __free_pages_ok+0x284/0x290 [ 37.977173] __free_pages+0x30/0x44 [ 37.984163] __dma_direct_free_pages+0x68/0x6c [ 37.993076] dma_direct_free+0x24/0x38 [ 38.000591] dma_free_attrs+0x84/0xc0 [ 38.007930] dmam_release+0x20/0x28 [ 38.014924] release_nodes+0x128/0x1f8 [ 38.022439] devres_release_all+0x34/0x4c [ 38.030478] device_release_driver_internal+0x190/0x208 [ 38.040963] device_release_driver+0x14/0x1c [ 38.049526] unbind_store+0xbc/0xf4 [ 38.056517] drv_attr_store+0x20/0x30 [ 38.063859] sysfs_kf_write+0x44/0x4c [ 38.071200] kernfs_fop_write+0xd0/0x1c4 [ 38.079065] __vfs_write+0x2c/0x158 [ 38.086055] vfs_write+0xa8/0x19c [ 38.092696] ksys_write+0x44/0xa0 [ 38.099338] __arm64_sys_write+0x1c/0x24 [ 38.107203] el0_svc_common+0xb0/0x100 [ 38.114718] el0_svc_handler+0x70/0x88 [ 38.122232] el0_svc+0x8/0x7c0 [ 38.128356] Disabling lock debugging due to kernel taint [ 38.139019] BUG: Bad page state in process sh pfn:11355 [ 38.149682] page:7e44d540 count:0 mapcount:0 mapping: index:0x0 [ 38.165760] flags: 0xfffc0001000(reserved) [ 38.174676] raw: 0fffc0001000 7e44d548 7e44d548 [ 38.190230] raw: [ 38.205783] page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set [ 38.218716] bad because of flags: 0x1000(reserved) [ 38.228329] Modules linked in: [ 38.234451] CPU: 5 PID: 2691 Comm: sh Tainted: GB 5.0.0 #121 [ 38.248604] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon
Re: [PATCH/RFC] driver core: Postpone DMA tear-down until after devres release
Hi John, On 07/03/2019 14:45, John Garry wrote: [...] Hi guys, Any idea what happened to this fix? It's been in -next for a while (commit 376991db4b64) - I assume it will land shortly and hit stable thereafter, at which point somebody gets to sort out the manual backport past 4.20. Robin. I have this on 5.0: [ 0.00] Linux version 5.0.0 (john@htsatcamb-server) (gcc version 4.8.5 (Linaro GCC 4.8-2015.06)) #121 SMP PREEMPT Thu Mar 7 14:28:39 GMT 2019 [ 0.00] Kernel command line: BOOT_IMAGE=/john/Image rdinit=/init crashkernel=256M@32M earlycon console=ttyAMA0,115200 acpi=force pcie_aspm=off scsi_mod.use_blk_mq=y no_console_suspend pcie-hisi.disable=1 ... [ 26.806856] pci_bus 000c:20: 2-byte config write to 000c:20:00.0 offset 0x44 may corrupt adjacent RW1C bits [ 26.817521] pcieport 0002:f8:00.0: can't derive routing for PCI INT B [ 26.837167] pci_bus 000c:20: 2-byte config write to 000c:20:00.0 offset 0x44 may corrupt adjacent RW1C bits [ 26.850091] serial 0002:f9:00.1: PCI INT B: no GSI [ 26.879364] serial 0002:f9:00.1: enabling device ( -> 0003) [ 26.913131] 0002:f9:00.1: ttyS3 at I/O 0x1008 (irq = 0, base_baud = 115200) is a ST16650V2 [ 26.992897] rtc-efi rtc-efi: setting system clock to 2019-03-07T14:41:48 UTC (1551969708) [ 27.009380] ALSA device list: [ 27.015326] No soundcards found. [ 27.022549] Freeing unused kernel memory: 1216K [ 27.055567] Run /init as init process root@(none)$ cd /sys/devices/platform/HISI0162:01 root@(none)$ echo HISI0162:01 > driver/unbind [ 36.488040] hisi_sas_v2_hw HISI0162:01: dev[9:1] is gone [ 36.561077] hisi_sas_v2_hw HISI0162:01: dev[8:1] is gone [ 36.621061] hisi_sas_v2_hw HISI0162:01: dev[7:1] is gone [ 36.693074] hisi_sas_v2_hw HISI0162:01: dev[6:1] is gone [ 36.753066] hisi_sas_v2_hw HISI0162:01: dev[5:1] is gone [ 36.764276] sd 0:0:3:0: [sdd] Synchronizing SCSI cache [ 36.821106] hisi_sas_v2_hw HISI0162:01: dev[4:1] is gone [ 36.889048] hisi_sas_v2_hw HISI0162:01: dev[3:1] is gone [ 36.993002] hisi_sas_v2_hw HISI0162:01: dev[1:1] is gone [ 37.004276] sd 0:0:0:0: [sda] Synchronizing SCSI cache [ 37.014768] sd 0:0:0:0: [sda] Stopping disk [ 37.709094] hisi_sas_v2_hw HISI0162:01: dev[2:5] is gone [ 37.721231] hisi_sas_v2_hw HISI0162:01: dev[0:2] is gone [ 37.803774] BUG: Bad page state in process sh pfn:11356 [ 37.81] page:7e44d580 count:1 mapcount:0 mapping: index:0x0 [ 37.830525] flags: 0xfffc0001000(reserved) [ 37.839443] raw: 0fffc0001000 7e44d588 7e44d588 [ 37.854998] raw: 0001 [ 37.870552] page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set [ 37.883485] bad because of flags: 0x1000(reserved) [ 37.893098] Modules linked in: [ 37.899221] CPU: 5 PID: 2691 Comm: sh Not tainted 5.0.0 #121 [ 37.910578] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018 [ 37.928924] Call trace: [ 37.933825] dump_backtrace+0x0/0x150 [ 37.941166] show_stack+0x14/0x1c [ 37.947808] dump_stack+0x8c/0xb0 [ 37.954451] bad_page+0xe4/0x144 [ 37.960918] free_pages_check_bad+0x7c/0x84 [ 37.969307] __free_pages_ok+0x284/0x290 [ 37.977173] __free_pages+0x30/0x44 [ 37.984163] __dma_direct_free_pages+0x68/0x6c [ 37.993076] dma_direct_free+0x24/0x38 [ 38.000591] dma_free_attrs+0x84/0xc0 [ 38.007930] dmam_release+0x20/0x28 [ 38.014924] release_nodes+0x128/0x1f8 [ 38.022439] devres_release_all+0x34/0x4c [ 38.030478] device_release_driver_internal+0x190/0x208 [ 38.040963] device_release_driver+0x14/0x1c [ 38.049526] unbind_store+0xbc/0xf4 [ 38.056517] drv_attr_store+0x20/0x30 [ 38.063859] sysfs_kf_write+0x44/0x4c [ 38.071200] kernfs_fop_write+0xd0/0x1c4 [ 38.079065] __vfs_write+0x2c/0x158 [ 38.086055] vfs_write+0xa8/0x19c [ 38.092696] ksys_write+0x44/0xa0 [ 38.099338] __arm64_sys_write+0x1c/0x24 [ 38.107203] el0_svc_common+0xb0/0x100 [ 38.114718] el0_svc_handler+0x70/0x88 [ 38.122232] el0_svc+0x8/0x7c0 [ 38.128356] Disabling lock debugging due to kernel taint [ 38.139019] BUG: Bad page state in process sh pfn:11355 [ 38.149682] page:7e44d540 count:0 mapcount:0 mapping: index:0x0 [ 38.165760] flags: 0xfffc0001000(reserved) [ 38.174676] raw: 0fffc0001000 7e44d548 7e44d548 [ 38.190230] raw: [ 38.205783] page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set [ 38.218716] bad because of flags: 0x1000(reserved) [ 38.228329] Modules linked in: [ 38.234451] CPU: 5 PID: 2691 Comm: sh Tainted: G B 5.0.0 #121 [ 38.248604] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018 [ 38.266949] Call trace: [ 38.271844] dump_backtrace+0x0/0x150
Re: [PATCH/RFC] driver core: Postpone DMA tear-down until after devres release
On 11/02/2019 10:22, Robin Murphy wrote:
On 08/02/2019 18:55, Geert Uytterhoeven wrote:
Hi Robin,
On Fri, Feb 8, 2019 at 6:55 PM Robin Murphy wrote:
On 08/02/2019 16:40, Joerg Roedel wrote:
On Thu, Feb 07, 2019 at 08:36:53PM +0100, Geert Uytterhoeven wrote:
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 8ac10af17c0043a3..d62487d024559620 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -968,9 +968,9 @@ static void __device_release_driver(struct
device *dev, struct device *parent)
drv->remove(dev);
device_links_driver_cleanup(dev);
-arch_teardown_dma_ops(dev);
devres_release_all(dev);
+arch_teardown_dma_ops(dev);
dev->driver = NULL;
dev_set_drvdata(dev, NULL);
if (dev->pm_domain && dev->pm_domain->dismiss)
Thanks for the fix! Should it also be tagged for stable and get a Fixes
FTR, Greg has added it to driver-core-testing, with a CC to stable.
So I see, great!
tag? I know it only triggers with a fix in v5.0-rc, but still...
I think so:
Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time
for platform/amba/pci bus devices")
Thanks! It won't backport cleanly due to commit dc3c05504d38849f
("dma-mapping: remove dma_deconfigure") in v4.20, though.
Ah yes - backports beyond that should simply be a case of moving the
dma_deconfigure() wrapper in the same manner.
Hi guys,
Any idea what happened to this fix?
I have this on 5.0:
[0.00] Linux version 5.0.0 (john@htsatcamb-server) (gcc version
4.8.5 (Linaro GCC 4.8-2015.06)) #121 SMP PREEMPT Thu Mar 7 14:28:39 GMT 2019
[0.00] Kernel command line: BOOT_IMAGE=/john/Image rdinit=/init
crashkernel=256M@32M earlycon console=ttyAMA0,115200 acpi=force
pcie_aspm=off scsi_mod.use_blk_mq=y no_console_suspend pcie-hisi.disable=1
...
[ 26.806856] pci_bus 000c:20: 2-byte config write to 000c:20:00.0
offset 0x44 may corrupt adjacent RW1C bits
[ 26.817521] pcieport 0002:f8:00.0: can't derive routing for PCI INT B
[ 26.837167] pci_bus 000c:20: 2-byte config write to 000c:20:00.0
offset 0x44 may corrupt adjacent RW1C bits
[ 26.850091] serial 0002:f9:00.1: PCI INT B: no GSI
[ 26.879364] serial 0002:f9:00.1: enabling device ( -> 0003)
[ 26.913131] 0002:f9:00.1: ttyS3 at I/O 0x1008 (irq = 0, base_baud =
115200) is a ST16650V2
[ 26.992897] rtc-efi rtc-efi: setting system clock to
2019-03-07T14:41:48 UTC (1551969708)
[ 27.009380] ALSA device list:
[ 27.015326] No soundcards found.
[ 27.022549] Freeing unused kernel memory: 1216K
[ 27.055567] Run /init as init process
root@(none)$ cd /sys/devices/platform/HISI0162:01
root@(none)$ echo HISI0162:01 > driver/unbind
[ 36.488040] hisi_sas_v2_hw HISI0162:01: dev[9:1] is gone
[ 36.561077] hisi_sas_v2_hw HISI0162:01: dev[8:1] is gone
[ 36.621061] hisi_sas_v2_hw HISI0162:01: dev[7:1] is gone
[ 36.693074] hisi_sas_v2_hw HISI0162:01: dev[6:1] is gone
[ 36.753066] hisi_sas_v2_hw HISI0162:01: dev[5:1] is gone
[ 36.764276] sd 0:0:3:0: [sdd] Synchronizing SCSI cache
[ 36.821106] hisi_sas_v2_hw HISI0162:01: dev[4:1] is gone
[ 36.889048] hisi_sas_v2_hw HISI0162:01: dev[3:1] is gone
[ 36.993002] hisi_sas_v2_hw HISI0162:01: dev[1:1] is gone
[ 37.004276] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[ 37.014768] sd 0:0:0:0: [sda] Stopping disk
[ 37.709094] hisi_sas_v2_hw HISI0162:01: dev[2:5] is gone
[ 37.721231] hisi_sas_v2_hw HISI0162:01: dev[0:2] is gone
[ 37.803774] BUG: Bad page state in process sh pfn:11356
[ 37.81] page:7e44d580 count:1 mapcount:0
mapping: index:0x0
[ 37.830525] flags: 0xfffc0001000(reserved)
[ 37.839443] raw: 0fffc0001000 7e44d588 7e44d588
[ 37.854998] raw: 0001
[ 37.870552] page dumped because: PAGE_FLAGS_CHECK_AT_FREE flag(s) set
[ 37.883485] bad because of flags: 0x1000(reserved)
[ 37.893098] Modules linked in:
[ 37.899221] CPU: 5 PID: 2691 Comm: sh Not tainted 5.0.0 #121
[ 37.910578] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon
D05 IT21 Nemo 2.0 RC0 04/18/2018
[ 37.928924] Call trace:
[ 37.933825] dump_backtrace+0x0/0x150
[ 37.941166] show_stack+0x14/0x1c
[ 37.947808] dump_stack+0x8c/0xb0
[ 37.954451] bad_page+0xe4/0x144
[ 37.960918] free_pages_check_bad+0x7c/0x84
[ 37.969307] __free_pages_ok+0x284/0x290
[ 37.977173] __free_pages+0x30/0x44
[ 37.984163] __dma_direct_free_pages+0x68/0x6c
[ 37.993076] dma_direct_free+0x24/0x38
[ 38.000591] dma_free_attrs+0x84/0xc0
[ 38.007930] dmam_release+0x20/0x28
[ 38.014924] release_nodes+0x128/0x1f8
[ 38.022439] devres_release_all+0x34/0x4c
[ 38.030478] device_release_driver_internal+0x190/0x208
[ 38.040963] device_release_driver+0x14/0x1c
[ 38.049526] unbind_store+0xbc/0xf4
[ 38.056517] drv_attr_store+0x20/0x30
[
Re: [PATCH v4 03/22] iommu: introduce device fault report API
On 06/03/2019 23:46, Jacob Pan wrote:
> On Tue, 5 Mar 2019 15:03:41 +
> Jean-Philippe Brucker wrote:
>
>> On 18/02/2019 13:54, Eric Auger wrote:
>> [...]> +/**
>> > + * iommu_register_device_fault_handler() - Register a device fault
>> > handler
>> > + * @dev: the device
>> > + * @handler: the fault handler
>> > + * @data: private data passed as argument to the handler
>> > + *
>> > + * When an IOMMU fault event is received, call this handler with
>> > the fault event
>> > + * and data as argument. The handler should return 0 on success.
>> > If the fault is
>> > + * recoverable (IOMMU_FAULT_PAGE_REQ), the handler can also
>> > complete
>> > + * the fault by calling iommu_page_response() with one of the
>> > following
>> > + * response code:
>> > + * - IOMMU_PAGE_RESP_SUCCESS: retry the translation
>> > + * - IOMMU_PAGE_RESP_INVALID: terminate the fault
>> > + * - IOMMU_PAGE_RESP_FAILURE: terminate the fault and stop
>> > reporting
>> > + * page faults if possible.
>>
>> The comment refers to function and values that haven't been defined
>> yet. Either the page_response() patch should come before, or we need
>> to split this patch.
>>
>> Something I missed before: if the handler fails (returns != 0) it
>> should complete the fault by calling iommu_page_response(), if we're
>> not doing it in iommu_report_device_fault(). It should be indicated
>> in this comment. It's safe for the handler to call page_response()
>> since we're not holding fault_param->lock when calling the handler.
>>
> If the page request fault is to be reported to a guest, the report
> function cannot wait for the completion status. As long as the fault is
> injected into the guest, the handler should complete with success. If
> the PRQ report fails, IMHO, the caller of iommu_report_device_fault()
> should send page_response, perhaps after clean up all partial response
> of the group too.
Ok, the caller (IOMMU driver) sending the page_response if
iommu_report_device_fault() fails does make more sense. Agreed on the
partial cleanup as well, we don't keep track of them here, but I need to
add that to the io-pgfault layer. However some cleanup should probably
happen in here...
>> > + /* we only report device fault if there is a handler
>> > registered */
>> > + mutex_lock(>iommu_param->lock);
>> > + if (!dev->iommu_param->fault_param ||
>> > + !dev->iommu_param->fault_param->handler) {
>> > + ret = -EINVAL;
>> > + goto done_unlock;
>> > + }
>> > + fparam = dev->iommu_param->fault_param;
>> > + if (evt->fault.type == IOMMU_FAULT_PAGE_REQ &&
>> > + evt->fault.prm.flags &
>> > IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE) {
>> > + evt_pending = kmemdup(evt, sizeof(struct
>> > iommu_fault_event),
>> > + GFP_KERNEL);
>> > + if (!evt_pending) {
>> > + ret = -ENOMEM;
>> > + goto done_unlock;
>> > + }
>> > + mutex_lock(>lock);
>> > + list_add_tail(_pending->list, >faults);
>> > + mutex_unlock(>lock);
>> > + }
>> > + ret = fparam->handler(evt, fparam->data);
... if ret != 0, removing and freeing the pending event seems more
appropriate here than asking our caller to do it
Thanks,
Jean
>> > +done_unlock:
>> > + mutex_unlock(>iommu_param->lock);
>> > + return ret;
>> > +}
>> > +EXPORT_SYMBOL_GPL(iommu_report_device_fault);
>> [...]
>
> [Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] [v3] dma-mapping: work around clang bug
Clang has a rather annoying behavior of checking for integer
arithmetic problems in code paths that are discarded by gcc
before that perfoms the same checks.
For DMA_BIT_MASK(64), this leads to a warning despite the
result of the macro being completely sensible:
arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type
[-Werror,-Wshift-count-overflow]
.coherent_dma_mask = DMA_BIT_MASK(64),
The best workaround I could come up with is to shift the
value twice, which makes the macro way less readable but
always has the same result.
Link: https://bugs.llvm.org/show_bug.cgi?id=38789
Reviewed-by: Geert Uytterhoeven
Reviewed-by: Robin Murphy
Signed-off-by: Arnd Bergmann
---
v3: use (2ull << n-1) instead of ((1ull << n-1) << 1)
special-case 0 instead of 64
v2: fix off-by-one error
---
include/linux/dma-mapping.h | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 75e60be91e5f..5788d60c2223 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -138,7 +138,11 @@ struct dma_map_ops {
extern const struct dma_map_ops dma_virt_ops;
extern const struct dma_map_ops dma_dummy_ops;
-#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
+/*
+ * Shifting '2' instead of '1' because of
+ * https://bugs.llvm.org/show_bug.cgi?id=38789
+ */
+#define DMA_BIT_MASK(n)(((n) == 0) ? 0ULL : (2ULL<<((n)-1))-1)
#define DMA_MASK_NONE 0x0ULL
--
2.20.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] iommu/vt-d: Remove lazy allocation of domains
Hey Lu,
> On 7 Mar 2019, at 06:31, Lu Baolu wrote:
>
> Hi James,
>
> On 3/7/19 2:08 AM, James Sewart wrote:
>> -/*
>> - * For each rmrr
>> - * for each dev attached to rmrr
>> - * do
>> - * locate drhd for dev, alloc domain for dev
>> - * allocate free domain
>> - * allocate page table entries for rmrr
>> - * if context not allocated for bus
>> - * allocate and init context
>> - * set present in root table for this bus
>> - * init context with domain, translation etc
>> - *endfor
>> - * endfor
>> - */
>> -pr_info("Setting RMRR:\n");
>> -for_each_rmrr_units(rmrr) {
>> -/* some BIOS lists non-exist devices in DMAR table. */
>> -for_each_active_dev_scope(rmrr->devices,
>> rmrr->devices_cnt,
>> - i, dev) {
>> -ret = iommu_prepare_rmrr_dev(rmrr, dev);
>> -if (ret)
>> -pr_err("Mapping reserved region
>> failed\n");
>> -}
>> -}
>> -
>> -iommu_prepare_isa();
> Why do you want to remove this segment of code?
This will only work if the lazy allocation of domains exists, these
mappings will disappear once a default domain is attached to a device and
then remade by iommu_group_create_direct_mappings. This code is redundant
and removing it allows us to remove all the lazy allocation logic.
>>> No exactly.
>>>
>>> We need to setup the rmrr mapping before enabling dma remapping,
>>> otherwise, there will be a window after dma remapping enabling and
>>> rmrr getting mapped, during which people might see dma faults.
>> Do you think this patch instead should be a refactoring to simplify this
>> initial domain setup before the default domain takes over? It seems like
>> duplicated effort to have both lazy allocated domains and externally managed
>> domains. We could allocate a domain here for any devices with RMRR and call
>> get_resv_regions to avoid duplicating RMRR loop.
>
> Agree. We should replace the lazy allocated domains with default domain
> in a clean way. Actually, your patches look great to me. But I do think
> we need further cleanups. The rmrr code is one example, and the identity
> domain (si_domain) is another.
>
> Do you mind if I work on top of your patches for further cleanups and
> sign off a v2 together with you?
Sure, sounds good. I’ll fixup patch 3 and have a go at integrating
iommu_prepare_isa into get_resv_regions. This should make the initial
domain logic here quite concise.
>
> Best regards,
> Lu Baolu
Cheers,
James.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] [v2] dma-mapping: work around clang bug
On 2019-03-07 9:28 am, Arnd Bergmann wrote: On Thu, Mar 7, 2019 at 10:17 AM Robin Murphy wrote: On 2019-03-07 8:52 am, Arnd Bergmann wrote: -#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */ +#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1) I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter in most cases, but it could potentially happen at runtime where callers use a non-constant argument. However, it also means we don't need to special-case 64 any more (since that's there to avoid the same thing anyway), so we could simply flip that to handle 0 instead. Yes, good idea. FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)", but that may not be to everyone's taste. I like that. So shall we do this? /* * Shifting '2' instead of '1' because of * https://bugs.llvm.org/show_bug.cgi?id=38789 */ #define DMA_BIT_MASK(n)(((n) == 0) ? 0ULL : ((2ULL<<((n)-1)))-1) Neat - it was too early in the morning for me to think of a succinct way to comment it, but that's great. I suspect there may be a redundant set of parentheses around the shift still, but other than that, Reviewed-by: Robin Murphy Cheers, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] [v2] dma-mapping: work around clang bug
On Thu, Mar 7, 2019 at 10:28 AM Arnd Bergmann wrote:
> On Thu, Mar 7, 2019 at 10:17 AM Robin Murphy wrote:
> > On 2019-03-07 8:52 am, Arnd Bergmann wrote:
> > >
> > > -#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> > > +/* double shift to work around
> > > https://bugs.llvm.org/show_bug.cgi?id=38789 */
> > > +#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL :
> > > ((1ULL<<((n)-1))<<1)-1)
> >
> > I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter
> > in most cases, but it could potentially happen at runtime where callers
> > use a non-constant argument. However, it also means we don't need to
> > special-case 64 any more (since that's there to avoid the same thing
> > anyway), so we could simply flip that to handle 0 instead.
>
> Yes, good idea.
>
> > FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)",
> > but that may not be to everyone's taste.
>
> I like that. So shall we do this?
>
> /*
> * Shifting '2' instead of '1' because of
> * https://bugs.llvm.org/show_bug.cgi?id=38789
> */
> #define DMA_BIT_MASK(n)(((n) == 0) ? 0ULL : ((2ULL<<((n)-1)))-1)
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] [v2] dma-mapping: work around clang bug
On Thu, Mar 7, 2019 at 10:17 AM Robin Murphy wrote: > On 2019-03-07 8:52 am, Arnd Bergmann wrote: > > > > -#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) > > +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 > > */ > > +#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1) > > I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter > in most cases, but it could potentially happen at runtime where callers > use a non-constant argument. However, it also means we don't need to > special-case 64 any more (since that's there to avoid the same thing > anyway), so we could simply flip that to handle 0 instead. Yes, good idea. > FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)", > but that may not be to everyone's taste. I like that. So shall we do this? /* * Shifting '2' instead of '1' because of * https://bugs.llvm.org/show_bug.cgi?id=38789 */ #define DMA_BIT_MASK(n)(((n) == 0) ? 0ULL : ((2ULL<<((n)-1)))-1) Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] [v2] dma-mapping: work around clang bug
Hi Arnd,
On 2019-03-07 8:52 am, Arnd Bergmann wrote:
Clang has a rather annoying behavior of checking for integer
arithmetic problems in code paths that are discarded by gcc
before that perfoms the same checks.
For DMA_BIT_MASK(64), this leads to a warning despite the
result of the macro being completely sensible:
arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type
[-Werror,-Wshift-count-overflow]
.coherent_dma_mask = DMA_BIT_MASK(64),
The best workaround I could come up with is to shift the
value twice, which makes the macro way less readable but
always has the same result.
Link: https://bugs.llvm.org/show_bug.cgi?id=38789
Signed-off-by: Arnd Bergmann
---
v2: fix off-by-one error
---
include/linux/dma-mapping.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 75e60be91e5f..9e438fe6b130 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -138,7 +138,8 @@ struct dma_map_ops {
extern const struct dma_map_ops dma_virt_ops;
extern const struct dma_map_ops dma_dummy_ops;
-#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
+/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
+#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
I think that now makes DMA_BIT_MASK(0) undefined - that shouldn't matter
in most cases, but it could potentially happen at runtime where callers
use a non-constant argument. However, it also means we don't need to
special-case 64 any more (since that's there to avoid the same thing
anyway), so we could simply flip that to handle 0 instead.
FWIW I'd be very tempted to fold in the second shift as "2ULL<<((n)-1)",
but that may not be to everyone's taste.
Robin.
#define DMA_MASK_NONE 0x0ULL
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] [v2] dma-mapping: work around clang bug
Clang has a rather annoying behavior of checking for integer
arithmetic problems in code paths that are discarded by gcc
before that perfoms the same checks.
For DMA_BIT_MASK(64), this leads to a warning despite the
result of the macro being completely sensible:
arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type
[-Werror,-Wshift-count-overflow]
.coherent_dma_mask = DMA_BIT_MASK(64),
The best workaround I could come up with is to shift the
value twice, which makes the macro way less readable but
always has the same result.
Link: https://bugs.llvm.org/show_bug.cgi?id=38789
Signed-off-by: Arnd Bergmann
---
v2: fix off-by-one error
---
include/linux/dma-mapping.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 75e60be91e5f..9e438fe6b130 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -138,7 +138,8 @@ struct dma_map_ops {
extern const struct dma_map_ops dma_virt_ops;
extern const struct dma_map_ops dma_dummy_ops;
-#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
+/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
+#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<((n)-1))<<1)-1)
#define DMA_MASK_NONE 0x0ULL
--
2.20.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-mapping: work around clang bug
On Thu, Mar 7, 2019 at 9:28 AM Geert Uytterhoeven wrote:
> On Thu, Mar 7, 2019 at 9:01 AM Arnd Bergmann wrote:
> > Clang has a rather annoying behavior of checking for integer
> > arithmetic problems in code paths that are discarded by gcc
> > before that perfoms the same checks.
> >
> > For DMA_BIT_MASK(64), this leads to a warning despite the
> > result of the macro being completely sensible:
> >
> > arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type
> > [-Werror,-Wshift-count-overflow]
> > .coherent_dma_mask = DMA_BIT_MASK(64),
> >
> > The best workaround I could come up with is to shift the
> > value twice, which makes the macro way less readable but
> > always has the same result.
> >
> > Link: https://bugs.llvm.org/show_bug.cgi?id=38789
> > Signed-off-by: Arnd Bergmann
> > ---
> > include/linux/dma-mapping.h | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> > index 75e60be91e5f..380d3a95d02e 100644
> > --- a/include/linux/dma-mapping.h
> > +++ b/include/linux/dma-mapping.h
> > @@ -138,7 +138,8 @@ struct dma_map_ops {
> > extern const struct dma_map_ops dma_virt_ops;
> > extern const struct dma_map_ops dma_dummy_ops;
> >
> > -#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> > +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789
> > */
> > +#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : (((1ULL<<((n)-1))-1)
> > << 1))
>
> The second "-1" should be done on the final result, not on the
> intermediate value.
Ah, of course. I'll send an update patch in a bit, sorry about this.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 9/9] vfio/type1: Handle different mdev isolation type
On Fri, Feb 22, 2019 at 10:19:27AM +0800, Lu Baolu wrote:
> This adds the support to determine the isolation type
> of a mediated device group by checking whether it has
> an iommu device. If an iommu device exists, an iommu
> domain will be allocated and then attached to the iommu
> device. Otherwise, keep the same behavior as it is.
>
> Cc: Ashok Raj
> Cc: Jacob Pan
> Cc: Kevin Tian
> Signed-off-by: Sanjay Kumar
> Signed-off-by: Liu Yi L
> Signed-off-by: Lu Baolu
> Reviewed-by: Jean-Philippe Brucker
> ---
> drivers/vfio/vfio_iommu_type1.c | 48 -
> 1 file changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index ccc4165474aa..f1392c582a3c 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -1368,13 +1368,40 @@ static void vfio_iommu_detach_group(struct
> vfio_domain *domain,
> iommu_detach_group(domain->domain, group->iommu_group);
> }
>
Hi Baolu,
To allow IOMMU-awared mdev, I think you need to modify the
vfio_iommu_type1_pin_pages and vfio_iommu_type1_unpin_pages to remove the
iommu->external_domain check.
Could you please include that in your patch? If not, I can send out a separate
patch to address that issue.
Thanks,
Neo
> +static bool vfio_bus_is_mdev(struct bus_type *bus)
> +{
> + struct bus_type *mdev_bus;
> + bool ret = false;
> +
> + mdev_bus = symbol_get(mdev_bus_type);
> + if (mdev_bus) {
> + ret = (bus == mdev_bus);
> + symbol_put(mdev_bus_type);
> + }
> +
> + return ret;
> +}
> +
> +static int vfio_mdev_iommu_device(struct device *dev, void *data)
> +{
> + struct device **old = data, *new;
> +
> + new = vfio_mdev_get_iommu_device(dev);
> + if (!new || (*old && *old != new))
> + return -EINVAL;
> +
> + *old = new;
> +
> + return 0;
> +}
> +
> static int vfio_iommu_type1_attach_group(void *iommu_data,
>struct iommu_group *iommu_group)
> {
> struct vfio_iommu *iommu = iommu_data;
> struct vfio_group *group;
> struct vfio_domain *domain, *d;
> - struct bus_type *bus = NULL, *mdev_bus;
> + struct bus_type *bus = NULL;
> int ret;
> bool resv_msi, msi_remap;
> phys_addr_t resv_msi_base;
> @@ -1409,23 +1436,30 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> if (ret)
> goto out_free;
>
> - mdev_bus = symbol_get(mdev_bus_type);
> + if (vfio_bus_is_mdev(bus)) {
> + struct device *iommu_device = NULL;
>
> - if (mdev_bus) {
> - if ((bus == mdev_bus) && !iommu_present(bus)) {
> - symbol_put(mdev_bus_type);
> + group->mdev_group = true;
> +
> + /* Determine the isolation type */
> + ret = iommu_group_for_each_dev(iommu_group, _device,
> +vfio_mdev_iommu_device);
> + if (ret || !iommu_device) {
> if (!iommu->external_domain) {
> INIT_LIST_HEAD(>group_list);
> iommu->external_domain = domain;
> - } else
> + } else {
> kfree(domain);
> + }
>
> list_add(>next,
>>external_domain->group_list);
> mutex_unlock(>lock);
> +
> return 0;
> }
> - symbol_put(mdev_bus_type);
> +
> + bus = iommu_device->bus;
> }
>
> domain->domain = iommu_domain_alloc(bus);
> --
> 2.17.1
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-mapping: work around clang bug
Hi Arnd,
On Thu, Mar 7, 2019 at 9:01 AM Arnd Bergmann wrote:
> Clang has a rather annoying behavior of checking for integer
> arithmetic problems in code paths that are discarded by gcc
> before that perfoms the same checks.
>
> For DMA_BIT_MASK(64), this leads to a warning despite the
> result of the macro being completely sensible:
>
> arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type
> [-Werror,-Wshift-count-overflow]
> .coherent_dma_mask = DMA_BIT_MASK(64),
>
> The best workaround I could come up with is to shift the
> value twice, which makes the macro way less readable but
> always has the same result.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=38789
> Signed-off-by: Arnd Bergmann
> ---
> include/linux/dma-mapping.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 75e60be91e5f..380d3a95d02e 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -138,7 +138,8 @@ struct dma_map_ops {
> extern const struct dma_map_ops dma_virt_ops;
> extern const struct dma_map_ops dma_dummy_ops;
>
> -#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> +/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
> +#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : (((1ULL<<((n)-1))-1)
> << 1))
The second "-1" should be done on the final result, not on the
intermediate value.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] dma-mapping: work around clang bug
Clang has a rather annoying behavior of checking for integer
arithmetic problems in code paths that are discarded by gcc
before that perfoms the same checks.
For DMA_BIT_MASK(64), this leads to a warning despite the
result of the macro being completely sensible:
arch/arm/plat-iop/adma.c:146:24: error: shift count >= width of type
[-Werror,-Wshift-count-overflow]
.coherent_dma_mask = DMA_BIT_MASK(64),
The best workaround I could come up with is to shift the
value twice, which makes the macro way less readable but
always has the same result.
Link: https://bugs.llvm.org/show_bug.cgi?id=38789
Signed-off-by: Arnd Bergmann
---
include/linux/dma-mapping.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 75e60be91e5f..380d3a95d02e 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -138,7 +138,8 @@ struct dma_map_ops {
extern const struct dma_map_ops dma_virt_ops;
extern const struct dma_map_ops dma_dummy_ops;
-#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
+/* double shift to work around https://bugs.llvm.org/show_bug.cgi?id=38789 */
+#define DMA_BIT_MASK(n)(((n) == 64) ? ~0ULL : (((1ULL<<((n)-1))-1) <<
1))
#define DMA_MASK_NONE 0x0ULL
--
2.20.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
