Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr
On Sun, Feb 24, 2019 at 09:25:18PM +0800, Pingfan Liu wrote: > Maybe I misunderstood you, but does "requested range failed" mean that > user specify the range? If yes, then it should be the duty of user as > you said later, not the duty of kernel" No, it should say that it selected a different range only when the user didn't specify it. Which would mean that the user didn't care about the range - she/he only wanted to have *any* crashkernel range reserved. I.e., crashkernel=X invocation. > We do not know the memory layout of a system, maybe a system with > memory less than 4GB. So it is better to try all the range of system > memory. Ok. If 4G fails, you set high and then try again. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr
On Fri, Feb 22, 2019 at 02:00:26PM +0100, Borislav Petkov wrote: > On Fri, Feb 22, 2019 at 09:42:41AM +0100, Joerg Roedel wrote: > > The current default of 256MB was found by experiments on a bigger > > number of machines, to create a reasonable default that is at least > > likely to be sufficient of an average machine. > > Exactly, and this is what makes sense. > > The code should try the requested reservation and if it fails, it should > try high allocation with default swiotlb size because we need to reserve > *some* range. Right, makes sense. While at it, maybe it is time to move the default allocation policy to 'high' again. The change was reverted six years ago because it broke old kexec tools, but those are probably out-of-service now. I think this change would make the whole crashdump allocation process less fragile. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr
On 02/25/19 at 12:00pm, Joerg Roedel wrote: > On Fri, Feb 22, 2019 at 02:00:26PM +0100, Borislav Petkov wrote: > > On Fri, Feb 22, 2019 at 09:42:41AM +0100, Joerg Roedel wrote: > > > The current default of 256MB was found by experiments on a bigger > > > number of machines, to create a reasonable default that is at least > > > likely to be sufficient of an average machine. > > > > Exactly, and this is what makes sense. > > > > The code should try the requested reservation and if it fails, it should > > try high allocation with default swiotlb size because we need to reserve > > *some* range. > > Right, makes sense. While at it, maybe it is time to move the default > allocation policy to 'high' again. The change was reverted six years ago > because it broke old kexec tools, but those are probably out-of-service > now. I think this change would make the whole crashdump allocation > process less fragile. One concern about this is for average cases, one do not need so much memory for kdump. For example in RHEL we use crashkernel=auto to automatically reserve kdump kernel memory, and for x86 the reserved size is like below now: 1G-64G:160M,64G-1T:256M,1T-:512M That means for a machine with less than 64G memory we only allocate 160M, it works for most machines in our lab. If we move to high as default, it will allocate 160M high + 256M low. It is too much for people who is good with the default 160M. Especially for virtual machine with less memory (but > 4G) To make the process less fragile maybe we can remove the 896M limitation and only try <4G then go to high. Thanks Dave ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr
On Mon, Feb 25, 2019 at 07:12:16PM +0800, Dave Young wrote: > If we move to high as default, it will allocate 160M high + 256M low. It We won't move to high by default - we will *fall* back to high if the default allocation fails. > To make the process less fragile maybe we can remove the 896M limitation > and only try <4G then go to high. Sure, the more robust for the user, the better. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove block layer bounce buffering for MMC v2
On Tue, 12 Feb 2019 at 08:25, Christoph Hellwig wrote: > > Hi everyone, > > this series converts the remaining MMC host drivers to properly kmap the > scatterlist entries it does PIO operations on, and then goes on to > remove the usage of block layer bounce buffering (which I plan to remove > eventually) from the MMC layer. > > As a bonus I've converted various drivers to the proper scatterlist > helpers so that at least in theory they are ready for chained > scatterlists. > > All the changes are compile tested only as I don't have any of the > hardware, so a careful review would be appreciated. > > Changes since v1: > - fix a missing kunmap_atomic in mvsdio > - fix a stray whitespace in s3cmci > - add new sg_kmap_atomic and sg_kunmap_atomic helpers > - set the DMA and block layer dma boundary > - use pointer arithmetics to reduce the amount of changes in >various drivers > This looks good to me, however the lack of feedback/tests worries me a bit. So, unless you think it's a bad idea, I intend to apply this when v5.1 rc1 is out, which allows a lengthy test period in linux-next. Make sense? Kind regards Uffe ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 19/22] vfio-pci: Register an iommu fault handler
Hi Vincent,
On 2/25/19 3:22 PM, Vincent Stehlé wrote:
> Hi Eric,
>
> On Mon, Feb 18, 2019 at 02:55:00PM +0100, Eric Auger wrote:
>> This patch registers a fault handler which records faults in
>> a circular buffer and then signals an eventfd. This buffer is
>> exposed within the fault region.
>>
>> Signed-off-by: Eric Auger
>> ---
>> drivers/vfio/pci/vfio_pci.c | 49 +
>> drivers/vfio/pci/vfio_pci_private.h | 1 +
>> 2 files changed, 50 insertions(+)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>> index aaf63e5ca2b6..019c9fd380a5 100644
>> --- a/drivers/vfio/pci/vfio_pci.c
>> +++ b/drivers/vfio/pci/vfio_pci.c
> (..)
>> static int vfio_pci_init_fault_region(struct vfio_pci_device *vdev)
>> {
>> struct vfio_region_fault_prod *header;
>> @@ -276,6 +317,13 @@ static int vfio_pci_init_fault_region(struct
>> vfio_pci_device *vdev)
>> header = (struct vfio_region_fault_prod *)vdev->fault_pages;
>> header->version = -1;
>> header->offset = PAGE_SIZE;
>> +
>> +ret = iommu_register_device_fault_handler(&vdev->pdev->dev,
>> +vfio_pci_iommu_dev_fault_handler,
>> +vdev);
>> +if (ret)
>> +goto out;
>> +
>> return 0;
>> out:
>> kfree(vdev->fault_pages);
>
> This patch calls iommu_register_device_fault_handler from
> vfio_pci_init_fault_region, leading to the following call stack:
>
> iommu_register_device_fault_handler
> vfio_pci_init_fault_region
> vfio_pci_enable
> vfio_pci_open
> vfio_group_get_device_fd
>
>> @@ -1420,6 +1468,7 @@ static void vfio_pci_remove(struct pci_dev *pdev)
>> vfio_iommu_group_put(pdev->dev.iommu_group, &pdev->dev);
>> kfree(vdev->region);
>> kfree(vdev->fault_pages);
>> +iommu_unregister_device_fault_handler(&pdev->dev);
>> mutex_destroy(&vdev->ioeventfds_lock);
>> kfree(vdev);
>
> And then this patch calls iommu_unregister_device_fault_handler from
> vfio_pci_remove, and not from vfio_pci_release.
Yes you're fully right. Thank you for the time spent debugging the
issue. this is a left-over from the previous version and indeed the
unregistration should be called from the release ops.
By the way, I will package a qemu version for testing this week. Sorry
for the delay.
Thanks
Eric
>
> I think this means a device cannot be used twice in a row without unloading
> the
> module.
>
> Here is an example sequence:
>
> 1. modprobe vfio-pci
> 2. Userspace uses VFIO, calls ioctl(VFIO_GROUP_GET_DEVICE_FD)
> 2.1. iommu_register_device_fault_handler is called
> 3. Userspace exits
> 3.1. vfio_pci_release is called,
> but iommu_unregister_device_fault_handler is not called
> 4. Userspace uses VFIO agin, calls ioctl(VFIO_GROUP_GET_DEVICE_FD) again
> 4.1. iommu_register_device_fault_handler is called again,
> notices a fault handler is already there,
> returns -EBUSY
>
> Unloading the vfio-pci module will call vfio_pci_remove.
>
> Maybe iommu_unregister_device_fault_handler should be called from
> vfio_pci_release instead of vfio_pci_remove?
>
> Best regards,
> Vincent.
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 0/7] Add virtio-iommu driver
Hi Jean, On 15.01.2019 13:19, Jean-Philippe Brucker wrote: > Implement the virtio-iommu driver, following specification v0.9 [1]. > > This is a simple rebase onto Linux v5.0-rc2. We now use the > dev_iommu_fwspec_get() helper introduced in v5.0 instead of accessing > dev->iommu_fwspec, but there aren't any functional change from v6 [2]. > > Our current goal for virtio-iommu is to get a paravirtual IOMMU working > on Arm, and enable device assignment to guest userspace. In this > use-case the mappings are static, and don't require optimal performance, > so this series tries to keep things simple. However there is plenty more > to do for features and optimizations, and having this base in v5.1 would > be good. Given that most of the changes are to drivers/iommu, I believe > the driver and future changes should go via the IOMMU tree. > > You can find Linux driver and kvmtool device on v0.9.2 branches [3], > module and x86 support on virtio-iommu/devel. Also tested with Eric's > QEMU device [4]. Please note that the series depends on Robin's > probe-deferral fix [5], which will hopefully land in v5.0. > > [1] Virtio-iommu specification v0.9, sources and pdf > git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9 > http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf > > [2] [PATCH v6 0/7] Add virtio-iommu driver > > https://lists.linuxfoundation.org/pipermail/iommu/2018-December/032127.html > > [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9.2 > git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9.2 > > [4] [RFC v9 00/17] VIRTIO-IOMMU device > https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html > > [5] [PATCH] iommu/of: Fix probe-deferral > https://www.spinics.net/lists/arm-kernel/msg698371.html > > Jean-Philippe Brucker (7): >dt-bindings: virtio-mmio: Add IOMMU description >dt-bindings: virtio: Add virtio-pci-iommu node >of: Allow the iommu-map property to omit untranslated devices >PCI: OF: Initialize dev->fwnode appropriately >iommu: Add virtio-iommu driver >iommu/virtio: Add probe request >iommu/virtio: Add event queue > > .../devicetree/bindings/virtio/iommu.txt | 66 + > .../devicetree/bindings/virtio/mmio.txt | 30 + > MAINTAINERS |7 + > drivers/iommu/Kconfig | 11 + > drivers/iommu/Makefile|1 + > drivers/iommu/virtio-iommu.c | 1158 + > drivers/of/base.c | 10 +- > drivers/pci/of.c |7 + > include/uapi/linux/virtio_ids.h |1 + > include/uapi/linux/virtio_iommu.h | 161 +++ > 10 files changed, 1449 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt > create mode 100644 drivers/iommu/virtio-iommu.c > create mode 100644 include/uapi/linux/virtio_iommu.h > I have tested the whole series and Eric's QEMU patchset [4] for virtio-net-pci and virtio-blk-pci devices and faced no issues on ThunderX2. The multiqueue mode for both devices is working fine too so: Tested-by: Tomasz Nowicki Thanks, Tomasz ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 19/22] vfio-pci: Register an iommu fault handler
Hi Eric,
On Mon, Feb 18, 2019 at 02:55:00PM +0100, Eric Auger wrote:
> This patch registers a fault handler which records faults in
> a circular buffer and then signals an eventfd. This buffer is
> exposed within the fault region.
>
> Signed-off-by: Eric Auger
> ---
> drivers/vfio/pci/vfio_pci.c | 49 +
> drivers/vfio/pci/vfio_pci_private.h | 1 +
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index aaf63e5ca2b6..019c9fd380a5 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
(..)
> static int vfio_pci_init_fault_region(struct vfio_pci_device *vdev)
> {
> struct vfio_region_fault_prod *header;
> @@ -276,6 +317,13 @@ static int vfio_pci_init_fault_region(struct
> vfio_pci_device *vdev)
> header = (struct vfio_region_fault_prod *)vdev->fault_pages;
> header->version = -1;
> header->offset = PAGE_SIZE;
> +
> + ret = iommu_register_device_fault_handler(&vdev->pdev->dev,
> + vfio_pci_iommu_dev_fault_handler,
> + vdev);
> + if (ret)
> + goto out;
> +
> return 0;
> out:
> kfree(vdev->fault_pages);
This patch calls iommu_register_device_fault_handler from
vfio_pci_init_fault_region, leading to the following call stack:
iommu_register_device_fault_handler
vfio_pci_init_fault_region
vfio_pci_enable
vfio_pci_open
vfio_group_get_device_fd
> @@ -1420,6 +1468,7 @@ static void vfio_pci_remove(struct pci_dev *pdev)
> vfio_iommu_group_put(pdev->dev.iommu_group, &pdev->dev);
> kfree(vdev->region);
> kfree(vdev->fault_pages);
> + iommu_unregister_device_fault_handler(&pdev->dev);
> mutex_destroy(&vdev->ioeventfds_lock);
> kfree(vdev);
And then this patch calls iommu_unregister_device_fault_handler from
vfio_pci_remove, and not from vfio_pci_release.
I think this means a device cannot be used twice in a row without unloading the
module.
Here is an example sequence:
1. modprobe vfio-pci
2. Userspace uses VFIO, calls ioctl(VFIO_GROUP_GET_DEVICE_FD)
2.1. iommu_register_device_fault_handler is called
3. Userspace exits
3.1. vfio_pci_release is called,
but iommu_unregister_device_fault_handler is not called
4. Userspace uses VFIO agin, calls ioctl(VFIO_GROUP_GET_DEVICE_FD) again
4.1. iommu_register_device_fault_handler is called again,
notices a fault handler is already there,
returns -EBUSY
Unloading the vfio-pci module will call vfio_pci_remove.
Maybe iommu_unregister_device_fault_handler should be called from
vfio_pci_release instead of vfio_pci_remove?
Best regards,
Vincent.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH V5 0/3] x86/Hyper-V/IOMMU: Add Hyper-V IOMMU driver to support x2apic mode
From: Tianyu Lan Sent: Friday, February 22, 2019 4:12 AM > > On the bare metal, enabling X2APIC mode requires interrupt remapping > function which helps to deliver irq to cpu with 32-bit APIC ID. > Hyper-V doesn't provide interrupt remapping function so far and Hyper-V > MSI protocol already supports to deliver interrupt to the CPU whose > virtual processor index is more than 255. IO-APIC interrupt still has > 8-bit APIC ID limitation. > > This patchset is to add Hyper-V stub IOMMU driver in order to enable > X2APIC mode successfully in Hyper-V Linux guest. The driver returns X2APIC > interrupt remapping capability when X2APIC mode is available. X2APIC > destination mode is set to physical by PATCH 1 when X2APIC is available. > Hyper-V IOMMU driver will scan cpu 0~255 and set cpu into IO-APIC MAX cpu > affinity cpumask if its APIC ID is 8-bit. Driver creates a Hyper-V irq domain > to limit IO-APIC interrupts' affinity and make sure cpus assigned with IO-APIC > interrupt are in the scope of IO-APIC MAX cpu affinity. > > Lan Tianyu (3): > x86/Hyper-V: Set x2apic destination mode to physical when x2apic is > available > HYPERV/IOMMU: Add Hyper-V stub IOMMU driver > MAINTAINERS: Add Hyper-V IOMMU driver into Hyper-V CORE AND DRIVERS > scope > Joerg -- What's your take on this patch set now that it has settled down? If you are good with it, from the Microsoft standpoint we're hoping that it can get into linux-next this week (given the extra week due to 5.0-rc8). Thanks, Michael Kelley ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/13] dt-binding: mediatek: Get rid of mediatek, larb for multimedia HW
On Mon, Dec 31, 2018 at 8:51 PM Yong Wu wrote: > > After adding device_link between the consumer with the smi-larbs, > if the consumer call its owner pm_runtime_get(_sync), the > pm_runtime_get(_sync) of smi-larb and smi-common will be called > automatically. Thus, the consumer don't need the property. > > And IOMMU also know which larb this consumer connects with from > iommu id in the "iommus=" property. > > Signed-off-by: Yong Wu Reviewed-by: Evan Green ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 03/13] iommu/mediatek: Add probe_defer for smi-larb
On Mon, Dec 31, 2018 at 8:52 PM Yong Wu wrote: > > The iommu consumer should use device_link to connect with the > smi-larb(supplier). then the smi-larb should run before the iommu > consumer. Here we delay the iommu driver until the smi driver is > ready, then all the iommu consumer always is after the smi driver. > > When there is no this patch, if some consumer drivers run before > smi-larb, the supplier link_status is DL_DEV_NO_DRIVER(0) in the > device_link_add, then device_links_driver_bound will use WARN_ON > to complain that the link_status of supplier is not right. > I don't quite have enough knowledge of device links to figure out if this is a) the right fix, b) a deficiency in the device_links code, or c) neither. Perhaps someone else could chime in on this one. -Evan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 06/13] media: mtk-jpeg: Get rid of mtk_smi_larb_get/put
On Mon, Dec 31, 2018 at 8:52 PM Yong Wu wrote: > > MediaTek IOMMU has already added device_link between the consumer > and smi-larb device. If the jpg device call the pm_runtime_get_sync, > the smi-larb's pm_runtime_get_sync also be called automatically. > > CC: Rick Chang > Signed-off-by: Yong Wu Reviewed-by: Evan Green ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 05/13] memory: mtk-smi: Add device-link between smi-larb and smi-common
On Mon, Dec 31, 2018 at 8:52 PM Yong Wu wrote: > > Normally, If the smi-larb HW need work, we should enable the smi-common > HW power and clock firstly. > This patch adds device-link between the smi-larb dev and the smi-common > dev. then If pm_runtime_get_sync(smi-larb-dev), the pm_runtime_get_sync > (smi-common-dev) will be called automatically. > > Since smi is built-in driver like IOMMU and never unbound, > DL_FLAG_AUTOREMOVE_* is not needed. > > CC: Matthias Brugger > Suggested-by: Tomasz Figa > Signed-off-by: Yong Wu > --- > drivers/memory/mtk-smi.c | 16 +++- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > index 9688341..30930e4 100644 > --- a/drivers/memory/mtk-smi.c > +++ b/drivers/memory/mtk-smi.c > @@ -271,6 +271,7 @@ static int mtk_smi_larb_probe(struct platform_device > *pdev) > struct device *dev = &pdev->dev; > struct device_node *smi_node; > struct platform_device *smi_pdev; > + struct device_link *link; > > larb = devm_kzalloc(dev, sizeof(*larb), GFP_KERNEL); > if (!larb) > @@ -310,6 +311,12 @@ static int mtk_smi_larb_probe(struct platform_device > *pdev) > if (!platform_get_drvdata(smi_pdev)) > return -EPROBE_DEFER; > larb->smi_common_dev = &smi_pdev->dev; > + link = device_link_add(dev, larb->smi_common_dev, > + DL_FLAG_PM_RUNTIME); Doesn't this need to be torn down in remove()? You mention that it's built-in and never removed, but it does seem to have a remove() function that tears down everything else, so it seemed a shame to start leaking now. Maybe the AUTOREMOVE flag would do it. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/13] driver core: Remove the link if there is no driver with AUTO flag
On Mon, Dec 31, 2018 at 8:52 PM Yong Wu wrote: > > DL_FLAG_AUTOREMOVE_CONSUMER/SUPPLIER means "Remove the link > automatically on consumer/supplier driver unbind", that means we should > remove whole the device_link when there is no this driver no matter what > the ref_count of the link is. > > CC: Greg Kroah-Hartman > Signed-off-by: Yong Wu > --- > The ref_count of our device_link normally is over 1. When the consumer > device driver is removed, whole the device_link should be removed. > Thus, I add this patch. > --- I will admit to reading about device links for the first time while reviewing this patch, but I don't really get this. Why use a kref at all if we're just going to ignore its value? For instance, I see that if you call device_link_add() with the same supplier and consumer, it uses the kref to return the same link. That machinery is broken with your change. Although I don't see any uses of it, you might also expect a supplier or consumer could do a kref_get() on the link it got back from device_link_add(), and have a reasonable expectation that the link wouldn't be freed out from under it. This would also be broken. Can you explain why your device_links normally have a reference count >1, and why those additional references can't be cleaned up in an orderly fashion? (To be honest, I don't really understand the case for the AUTOREMOVE flags at all. Is there some case where the party that set up the link can't tear it down? Or is this a way to devm_ify the link, where devm itself doesn't work because the links themselves stall out that mechanism?) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 11/13] iommu/mediatek: Use builtin_platform_driver
On Mon, Dec 31, 2018 at 8:53 PM Yong Wu wrote:
>
> MediaTek IOMMU should wait for smi larb which need wait for the
> power domain(mtk-scpsys.c) and the multimedia ccf who both are
> module init. Thus, subsys_initcall for MediaTek IOMMU is not helpful.
> Switch to builtin_platform_driver.
>
> Meanwhile, the ".remove" can be removed. Move its content to
> ".shutdown".
>
> Signed-off-by: Yong Wu
> ---
> drivers/iommu/mtk_iommu.c| 23 ++-
> drivers/iommu/mtk_iommu_v1.c | 16 ++--
> 2 files changed, 4 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 735ae8d..2798b12 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -690,7 +690,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> return component_master_add_with_match(dev, &mtk_iommu_com_ops,
> match);
> }
>
> -static int mtk_iommu_remove(struct platform_device *pdev)
> +static void mtk_iommu_shutdown(struct platform_device *pdev)
> {
> struct mtk_iommu_data *data = platform_get_drvdata(pdev);
>
> @@ -703,12 +703,6 @@ static int mtk_iommu_remove(struct platform_device *pdev)
> clk_disable_unprepare(data->bclk);
> devm_free_irq(&pdev->dev, data->irq, data);
> component_master_del(&pdev->dev, &mtk_iommu_com_ops);
> - return 0;
> -}
> -
> -static void mtk_iommu_shutdown(struct platform_device *pdev)
> -{
> - mtk_iommu_remove(pdev);
Is there a reason all these things are happening in shutdown()? Don't
we normally just not clean things up and let the machine turn off?
Normally I'm a big advocate of proper symmetric teardown, so it hurts
a little to ask.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 04/13] iommu/mediatek: Add device_link between the consumer and the larb devices
On Mon, Dec 31, 2018 at 8:52 PM Yong Wu wrote:
>
> MediaTek IOMMU don't have its power-domain. all the consumer connect
> with smi-larb, then connect with smi-common.
>
> M4U
> |
> smi-common
> |
> -
> | |...
> | |
> larb1 larb2
> | |
> vdec venc
>
> When the consumer works, it should enable the smi-larb's power which
> also need enable the smi-common's power firstly.
>
> Thus, First of all, use the device link connect the consumer and the
> smi-larbs. then add device link between the smi-larb and smi-common.
>
> This patch adds device_link between the consumer and the larbs.
>
> Suggested-by: Tomasz Figa
> Signed-off-by: Yong Wu
> ---
> drivers/iommu/mtk_iommu.c| 15 +--
> drivers/iommu/mtk_iommu_v1.c | 14 --
> 2 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 202e41b..735ae8d 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -247,6 +247,7 @@ static void mtk_iommu_config(struct mtk_iommu_data *data,
> struct mtk_smi_larb_iommu*larb_mmu;
> unsigned int larbid, portid;
> struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> + struct device_link *link;
> int i;
>
> for (i = 0; i < fwspec->num_ids; ++i) {
> @@ -257,10 +258,20 @@ static void mtk_iommu_config(struct mtk_iommu_data
> *data,
> dev_dbg(dev, "%s iommu port: %d\n",
> enable ? "enable" : "disable", portid);
>
> - if (enable)
> + if (enable) {
> larb_mmu->mmu |= MTK_SMI_MMU_EN(portid);
> - else
> + /* Link the consumer with the larb device(supplier) */
> + link = device_link_add(dev, larb_mmu->dev,
> + DL_FLAG_PM_RUNTIME |
> + DL_FLAG_AUTOREMOVE_CONSUMER);
> + if (!link) {
> + dev_err(dev, "Unable to link %s\n",
> + dev_name(larb_mmu->dev));
> + return;
The return is a little odd here, given that you don't return an error
and this function doesn't do anything else. If it's non-fatal that
this link didn't get set up, then remove the return. If it is fatal,
then return an error code.
> + }
> + } else {
> larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid);
> + }
> }
> }
>
> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index 9386aee..022bad9 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c
> @@ -201,6 +201,7 @@ static void mtk_iommu_config(struct mtk_iommu_data *data,
> struct mtk_smi_larb_iommu*larb_mmu;
> unsigned int larbid, portid;
> struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> + struct device_link *link;
> int i;
>
> for (i = 0; i < fwspec->num_ids; ++i) {
> @@ -211,10 +212,19 @@ static void mtk_iommu_config(struct mtk_iommu_data
> *data,
> dev_dbg(dev, "%s iommu port: %d\n",
> enable ? "enable" : "disable", portid);
>
> - if (enable)
> + if (enable) {
> larb_mmu->mmu |= MTK_SMI_MMU_EN(portid);
> - else
> + link = device_link_add(dev, larb_mmu->dev,
> + DL_FLAG_PM_RUNTIME |
> + DL_FLAG_AUTOREMOVE_CONSUMER);
> + if (!link) {
> + dev_err(dev, "Unable to link %s\n",
> + dev_name(larb_mmu->dev));
> + return;
Same for this one.
> + }
> + } else {
> larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid);
> + }
> }
> }
>
> --
> 1.9.1
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 13/13] arm64: dts: mediatek: Get rid of mediatek, larb for MM nodes
On Mon, Dec 31, 2018 at 8:53 PM Yong Wu wrote: > > After adding device_link between the IOMMU consumer and smi, > the mediatek,larb is unnecessary now. > > CC: Matthias Brugger > Signed-off-by: Yong Wu Reviewed-by: Evan Green ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 07/13] media: mtk-mdp: Get rid of mtk_smi_larb_get/put
On Mon, Dec 31, 2018 at 8:52 PM Yong Wu wrote: > > MediaTek IOMMU has already added the device_link between the consumer > and smi-larb device. If the mdp device call the pm_runtime_get_sync, > the smi-larb's pm_runtime_get_sync also be called automatically. > > CC: Minghsiu Tsai > Signed-off-by: Yong Wu Reviewed-by: Evan Green ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 09/13] drm/mediatek: Get rid of mtk_smi_larb_get/put
On Mon, Dec 31, 2018 at 8:52 PM Yong Wu wrote: > > MediaTek IOMMU has already added the device_link between the consumer > and smi-larb device. If the drm device call the pm_runtime_get_sync, > the smi-larb's pm_runtime_get_sync also be called automatically. > > CC: CK Hu > CC: Philipp Zabel > Signed-off-by: Yong Wu Nice cleanup. Reviewed-by: Evan Green ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 10/13] memory: mtk-smi: Get rid of mtk_smi_larb_get/put
On Mon, Dec 31, 2018 at 8:53 PM Yong Wu wrote: > > After adding device_link between the iommu consumer and smi-larb, > the pm_runtime_get(_sync) of smi-larb and smi-common will be called > automatically. we can get rid of mtk_smi_larb_get/put. > > CC: Matthias Brugger > Signed-off-by: Yong Wu It's possible that there's so little left in mediatek/smi.h that it could be deleted entirely and its last remaining bits absorbed into other locations. But maybe it's nice to keep those last little bits used by mtk-smi.c away from the iommu definitions. So it seems fine as-is to me. Reviewed-by: Evan Green ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/13] arm: dts: mediatek: Get rid of mediatek, larb for MM nodes
On Mon, Dec 31, 2018 at 8:53 PM Yong Wu wrote: > > After adding device_link between the IOMMU consumer and smi, > the mediatek,larb is unnecessary now. > > CC: Matthias Brugger > Signed-off-by: Yong Wu Reviewed-by: Evan Green ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 08/13] media: mtk-vcodec: Get rid of mtk_smi_larb_get/put
On Mon, Dec 31, 2018 at 8:52 PM Yong Wu wrote: > > MediaTek IOMMU has already added the device_link between the consumer > and smi-larb device. If the vcodec device call the pm_runtime_get_sync, > the smi-larb's pm_runtime_get_sync also be called automatically. > > CC: Tiffany Lin > Signed-off-by: Yong Wu Reviewed-by: Evan Green ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
