Re: [PATCH v4 0/3] Apple M1 DART IOMMU driver
On Wed, Jul 14, 2021 at 10:51:34PM +0200, Arnd Bergmann wrote: > The question is how we can best allow one but not the other. By only allowing to allocate domains of type IDENTITY and DMA, but fail to allocate UNMANAGED domains. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v2] /dev/iommu uAPI proposal
> From: Shenming Lu > Sent: Thursday, July 15, 2021 2:29 PM > > On 2021/7/15 11:55, Tian, Kevin wrote: > >> From: Shenming Lu > >> Sent: Thursday, July 15, 2021 11:21 AM > >> > >> On 2021/7/9 15:48, Tian, Kevin wrote: > >>> 4.6. I/O page fault > >>> +++ > >>> > >>> uAPI is TBD. Here is just about the high-level flow from host IOMMU > driver > >>> to guest IOMMU driver and backwards. This flow assumes that I/O page > >> faults > >>> are reported via IOMMU interrupts. Some devices report faults via > device > >>> specific way instead of going through the IOMMU. That usage is not > >> covered > >>> here: > >>> > >>> - Host IOMMU driver receives a I/O page fault with raw fault_data {rid, > >>> pasid, addr}; > >>> > >>> - Host IOMMU driver identifies the faulting I/O page table according to > >>> {rid, pasid} and calls the corresponding fault handler with an opaque > >>> object (registered by the handler) and raw fault_data (rid, pasid, > >>> addr); > >>> > >>> - IOASID fault handler identifies the corresponding ioasid and device > >>> cookie according to the opaque object, generates an user fault_data > >>> (ioasid, cookie, addr) in the fault region, and triggers eventfd to > >>> userspace; > >>> > >> > >> Hi, I have some doubts here: > >> > >> For mdev, it seems that the rid in the raw fault_data is the parent > >> device's, > >> then in the vSVA scenario, how can we get to know the mdev(cookie) from > >> the > >> rid and pasid? > >> > >> And from this point of view,would it be better to register the mdev > >> (iommu_register_device()) with the parent device info? > >> > > > > This is what is proposed in this RFC. A successful binding generates a new > > iommu_dev object for each vfio device. For mdev this object includes > > its parent device, the defPASID marking this mdev, and the cookie > > representing it in userspace. Later it is iommu_dev being recorded in > > the attaching_data when the mdev is attached to an IOASID: > > > > struct iommu_attach_data *__iommu_device_attach( > > struct iommu_dev *dev, u32 ioasid, u32 pasid, int flags); > > > > Then when a fault is reported, the fault handler just needs to figure out > > iommu_dev according to {rid, pasid} in the raw fault data. > > > > Yeah, we have the defPASID that marks the mdev and refers to the default > I/O address space, but how about the non-default I/O address spaces? > Is there a case that two different mdevs (on the same parent device) > are used by the same process in the guest, thus have a same pasid route > in the physical IOMMU? It seems that we can't figure out the mdev from > the rid and pasid in this case... > > Did I misunderstand something?... :-) > No. You are right on this case. I don't think there is a way to differentiate one mdev from the other if they come from the same parent and attached by the same guest process. In this case the fault could be reported on either mdev (e.g. the first matching one) to get it fixed in the guest. Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On 2021/7/15 11:55, Tian, Kevin wrote: >> From: Shenming Lu >> Sent: Thursday, July 15, 2021 11:21 AM >> >> On 2021/7/9 15:48, Tian, Kevin wrote: >>> 4.6. I/O page fault >>> +++ >>> >>> uAPI is TBD. Here is just about the high-level flow from host IOMMU driver >>> to guest IOMMU driver and backwards. This flow assumes that I/O page >> faults >>> are reported via IOMMU interrupts. Some devices report faults via device >>> specific way instead of going through the IOMMU. That usage is not >> covered >>> here: >>> >>> - Host IOMMU driver receives a I/O page fault with raw fault_data {rid, >>> pasid, addr}; >>> >>> - Host IOMMU driver identifies the faulting I/O page table according to >>> {rid, pasid} and calls the corresponding fault handler with an opaque >>> object (registered by the handler) and raw fault_data (rid, pasid, >>> addr); >>> >>> - IOASID fault handler identifies the corresponding ioasid and device >>> cookie according to the opaque object, generates an user fault_data >>> (ioasid, cookie, addr) in the fault region, and triggers eventfd to >>> userspace; >>> >> >> Hi, I have some doubts here: >> >> For mdev, it seems that the rid in the raw fault_data is the parent device's, >> then in the vSVA scenario, how can we get to know the mdev(cookie) from >> the >> rid and pasid? >> >> And from this point of view,would it be better to register the mdev >> (iommu_register_device()) with the parent device info? >> > > This is what is proposed in this RFC. A successful binding generates a new > iommu_dev object for each vfio device. For mdev this object includes > its parent device, the defPASID marking this mdev, and the cookie > representing it in userspace. Later it is iommu_dev being recorded in > the attaching_data when the mdev is attached to an IOASID: > > struct iommu_attach_data *__iommu_device_attach( > struct iommu_dev *dev, u32 ioasid, u32 pasid, int flags); > > Then when a fault is reported, the fault handler just needs to figure out > iommu_dev according to {rid, pasid} in the raw fault data. > Yeah, we have the defPASID that marks the mdev and refers to the default I/O address space, but how about the non-default I/O address spaces? Is there a case that two different mdevs (on the same parent device) are used by the same process in the guest, thus have a same pasid route in the physical IOMMU? It seems that we can't figure out the mdev from the rid and pasid in this case... Did I misunderstand something?... :-) Thanks, Shenming ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 17/17] Documentation: Add documentation for VDUSE
在 2021/7/13 下午4:46, Xie Yongji 写道: VDUSE (vDPA Device in Userspace) is a framework to support implementing software-emulated vDPA devices in userspace. This document is intended to clarify the VDUSE design and usage. Signed-off-by: Xie Yongji --- Documentation/userspace-api/index.rst | 1 + Documentation/userspace-api/vduse.rst | 248 ++ 2 files changed, 249 insertions(+) create mode 100644 Documentation/userspace-api/vduse.rst diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst index 0b5eefed027e..c432be070f67 100644 --- a/Documentation/userspace-api/index.rst +++ b/Documentation/userspace-api/index.rst @@ -27,6 +27,7 @@ place where this information is gathered. iommu media/index sysfs-platform_profile + vduse .. only:: subproject and html diff --git a/Documentation/userspace-api/vduse.rst b/Documentation/userspace-api/vduse.rst new file mode 100644 index ..2c0d56d4b2da --- /dev/null +++ b/Documentation/userspace-api/vduse.rst @@ -0,0 +1,248 @@ +== +VDUSE - "vDPA Device in Userspace" +== + +vDPA (virtio data path acceleration) device is a device that uses a +datapath which complies with the virtio specifications with vendor +specific control path. vDPA devices can be both physically located on +the hardware or emulated by software. VDUSE is a framework that makes it +possible to implement software-emulated vDPA devices in userspace. And +to make the device emulation more secure, the emulated vDPA device's +control path is handled in the kernel and only the data path is +implemented in the userspace. + +Note that only virtio block device is supported by VDUSE framework now, +which can reduce security risks when the userspace process that implements +the data path is run by an unprivileged user. The support for other device +types can be added after the security issue of corresponding device driver +is clarified or fixed in the future. + +Start/Stop VDUSE devices + + +VDUSE devices are started as follows: Not native speaker but "created" is probably better. + +1. Create a new VDUSE instance with ioctl(VDUSE_CREATE_DEV) on + /dev/vduse/control. + +2. Setup each virtqueue with ioctl(VDUSE_VQ_SETUP) on /dev/vduse/$NAME. + +3. Begin processing VDUSE messages from /dev/vduse/$NAME. The first + messages will arrive while attaching the VDUSE instance to vDPA bus. + +4. Send the VDPA_CMD_DEV_NEW netlink message to attach the VDUSE + instance to vDPA bus. I think 4 should be done before 3? + +VDUSE devices are stopped as follows: "removed" or "destroyed" is better than "stopped" here. + +1. Send the VDPA_CMD_DEV_DEL netlink message to detach the VDUSE + instance from vDPA bus. + +2. Close the file descriptor referring to /dev/vduse/$NAME. + +3. Destroy the VDUSE instance with ioctl(VDUSE_DESTROY_DEV) on + /dev/vduse/control. + +The netlink messages can be sent via vdpa tool in iproute2 or use the +below sample codes: + +.. code-block:: c + + static int netlink_add_vduse(const char *name, enum vdpa_command cmd) + { + struct nl_sock *nlsock; + struct nl_msg *msg; + int famid; + + nlsock = nl_socket_alloc(); + if (!nlsock) + return -ENOMEM; + + if (genl_connect(nlsock)) + goto free_sock; + + famid = genl_ctrl_resolve(nlsock, VDPA_GENL_NAME); + if (famid < 0) + goto close_sock; + + msg = nlmsg_alloc(); + if (!msg) + goto close_sock; + + if (!genlmsg_put(msg, NL_AUTO_PORT, NL_AUTO_SEQ, famid, 0, 0, cmd, 0)) + goto nla_put_failure; + + NLA_PUT_STRING(msg, VDPA_ATTR_DEV_NAME, name); + if (cmd == VDPA_CMD_DEV_NEW) + NLA_PUT_STRING(msg, VDPA_ATTR_MGMTDEV_DEV_NAME, "vduse"); + + if (nl_send_sync(nlsock, msg)) + goto close_sock; + + nl_close(nlsock); + nl_socket_free(nlsock); + + return 0; + nla_put_failure: + nlmsg_free(msg); + close_sock: + nl_close(nlsock); + free_sock: + nl_socket_free(nlsock); + return -1; + } + +How VDUSE works +--- + +As mentioned above, a VDUSE device is created by ioctl(VDUSE_CREATE_DEV) on +/dev/vduse/control. With this ioctl, userspace can specify some basic configuration +such as device name (uniquely identify a VDUSE device), virtio features, virtio +configuration space, bounce buffer size This bounce buffer size looks questionable. We'd better not expose any implementation details to userspace. I think we can simply start with a module parameter for VDUSE? and
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
在 2021/7/15 下午12:03, Yongji Xie 写道: Which ioctl can be used for this? I mean we can introduce a new ioctl for that in the future. Ok, I see. I wonder if it's better to do something similar to ccw: 1) requires the userspace to update the status bit in the response 2) update the dev->status to the status in the response if no timeout Then userspace could then set NEEDS_RESET if necessary. But NEEDS_RESET does not only happen in this case. Yes, so you need an ioctl for userspace to update the device status (NEEDS_RESET) probably. Yes, but I'd like to do that after the initial version is merged since NEEDS_RESET is not supported now. Right. Thanks ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Patch V2 0/2] iommu/arm-smmu: Fix races in iommu domain/group creation
On 6/18/2021 2:00 AM, Ashish Mhetre wrote: Multiple iommu domains and iommu groups are getting created for the devices sharing same SID. It is expected for devices sharing same SID to be in same iommu group and same iommu domain. This is leading to context faults when one device is accessing IOVA from other device which shouldn't be the case for devices sharing same SID. Fix this by protecting iommu domain and iommu group creation with mutexes. Ashish Mhetre (1): iommu: Fix race condition during default domain allocation Krishna Reddy (1): iommu/arm-smmu: Fix race condition during iommu_group creation drivers/iommu/arm/arm-smmu/arm-smmu.c | 6 +- drivers/iommu/iommu.c | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) Hi, Can you please help in reviewing this V2 change? Please let me know if anymore changes are required. Thanks, Ashish Mhetre ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
On Wed, Jul 14, 2021 at 5:12 PM Jason Wang wrote: > > > 在 2021/7/14 下午2:49, Yongji Xie 写道: > > On Wed, Jul 14, 2021 at 1:45 PM Jason Wang wrote: > >> > >> 在 2021/7/13 下午4:46, Xie Yongji 写道: > >>> This VDUSE driver enables implementing software-emulated vDPA > >>> devices in userspace. The vDPA device is created by > >>> ioctl(VDUSE_CREATE_DEV) on /dev/vduse/control. Then a char device > >>> interface (/dev/vduse/$NAME) is exported to userspace for device > >>> emulation. > >>> > >>> In order to make the device emulation more secure, the device's > >>> control path is handled in kernel. A message mechnism is introduced > >>> to forward some dataplane related control messages to userspace. > >>> > >>> And in the data path, the DMA buffer will be mapped into userspace > >>> address space through different ways depending on the vDPA bus to > >>> which the vDPA device is attached. In virtio-vdpa case, the MMU-based > >>> IOMMU driver is used to achieve that. And in vhost-vdpa case, the > >>> DMA buffer is reside in a userspace memory region which can be shared > >>> to the VDUSE userspace processs via transferring the shmfd. > >>> > >>> For more details on VDUSE design and usage, please see the follow-on > >>> Documentation commit. > >>> > >>> Signed-off-by: Xie Yongji > >>> --- > >>>Documentation/userspace-api/ioctl/ioctl-number.rst |1 + > >>>drivers/vdpa/Kconfig | 10 + > >>>drivers/vdpa/Makefile |1 + > >>>drivers/vdpa/vdpa_user/Makefile|5 + > >>>drivers/vdpa/vdpa_user/vduse_dev.c | 1502 > >>> > >>>include/uapi/linux/vduse.h | 221 +++ > >>>6 files changed, 1740 insertions(+) > >>>create mode 100644 drivers/vdpa/vdpa_user/Makefile > >>>create mode 100644 drivers/vdpa/vdpa_user/vduse_dev.c > >>>create mode 100644 include/uapi/linux/vduse.h > >>> > >>> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst > >>> b/Documentation/userspace-api/ioctl/ioctl-number.rst > >>> index 1409e40e6345..293ca3aef358 100644 > >>> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst > >>> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst > >>> @@ -300,6 +300,7 @@ Code Seq#Include File > >>>Comments > >>>'z' 10-4F drivers/s390/crypto/zcrypt_api.h > >>> conflict! > >>>'|' 00-7F linux/media.h > >>>0x80 00-1F linux/fb.h > >>> +0x81 00-1F linux/vduse.h > >>>0x89 00-06 arch/x86/include/asm/sockios.h > >>>0x89 0B-DF linux/sockios.h > >>>0x89 E0-EF linux/sockios.h > >>> SIOCPROTOPRIVATE range > >>> diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig > >>> index a503c1b2bfd9..6e23bce6433a 100644 > >>> --- a/drivers/vdpa/Kconfig > >>> +++ b/drivers/vdpa/Kconfig > >>> @@ -33,6 +33,16 @@ config VDPA_SIM_BLOCK > >>> vDPA block device simulator which terminates IO request in a > >>> memory buffer. > >>> > >>> +config VDPA_USER > >>> + tristate "VDUSE (vDPA Device in Userspace) support" > >>> + depends on EVENTFD && MMU && HAS_DMA > >>> + select DMA_OPS > >>> + select VHOST_IOTLB > >>> + select IOMMU_IOVA > >>> + help > >>> + With VDUSE it is possible to emulate a vDPA Device > >>> + in a userspace program. > >>> + > >>>config IFCVF > >>>tristate "Intel IFC VF vDPA driver" > >>>depends on PCI_MSI > >>> diff --git a/drivers/vdpa/Makefile b/drivers/vdpa/Makefile > >>> index 67fe7f3d6943..f02ebed33f19 100644 > >>> --- a/drivers/vdpa/Makefile > >>> +++ b/drivers/vdpa/Makefile > >>> @@ -1,6 +1,7 @@ > >>># SPDX-License-Identifier: GPL-2.0 > >>>obj-$(CONFIG_VDPA) += vdpa.o > >>>obj-$(CONFIG_VDPA_SIM) += vdpa_sim/ > >>> +obj-$(CONFIG_VDPA_USER) += vdpa_user/ > >>>obj-$(CONFIG_IFCVF)+= ifcvf/ > >>>obj-$(CONFIG_MLX5_VDPA) += mlx5/ > >>>obj-$(CONFIG_VP_VDPA)+= virtio_pci/ > >>> diff --git a/drivers/vdpa/vdpa_user/Makefile > >>> b/drivers/vdpa/vdpa_user/Makefile > >>> new file mode 100644 > >>> index ..260e0b26af99 > >>> --- /dev/null > >>> +++ b/drivers/vdpa/vdpa_user/Makefile > >>> @@ -0,0 +1,5 @@ > >>> +# SPDX-License-Identifier: GPL-2.0 > >>> + > >>> +vduse-y := vduse_dev.o iova_domain.o > >>> + > >>> +obj-$(CONFIG_VDPA_USER) += vduse.o > >>> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c > >>> b/drivers/vdpa/vdpa_user/vduse_dev.c > >>> new file mode 100644 > >>> index ..c994a4a4660c > >>> --- /dev/null > >>> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > >>> @@ -0,0 +1,1502 @@ > >>> +// SPDX-License-Identifier: GPL-2.0-only > >>> +/* > >>> + * VDUSE: vDPA Device in Userspace > >>> + * > >>> + * Copyright (C) 2020-2021 Bytedance Inc. and/or its affiliates. All > >>> rights reserved. > >>> + * > >>> + * Author: Xie Yongji > >>> + * > >>> + */ > >>> + >
RE: [RFC v2] /dev/iommu uAPI proposal
> From: Shenming Lu > Sent: Thursday, July 15, 2021 11:21 AM > > On 2021/7/9 15:48, Tian, Kevin wrote: > > 4.6. I/O page fault > > +++ > > > > uAPI is TBD. Here is just about the high-level flow from host IOMMU driver > > to guest IOMMU driver and backwards. This flow assumes that I/O page > faults > > are reported via IOMMU interrupts. Some devices report faults via device > > specific way instead of going through the IOMMU. That usage is not > covered > > here: > > > > - Host IOMMU driver receives a I/O page fault with raw fault_data {rid, > > pasid, addr}; > > > > - Host IOMMU driver identifies the faulting I/O page table according to > > {rid, pasid} and calls the corresponding fault handler with an opaque > > object (registered by the handler) and raw fault_data (rid, pasid, > > addr); > > > > - IOASID fault handler identifies the corresponding ioasid and device > > cookie according to the opaque object, generates an user fault_data > > (ioasid, cookie, addr) in the fault region, and triggers eventfd to > > userspace; > > > > Hi, I have some doubts here: > > For mdev, it seems that the rid in the raw fault_data is the parent device's, > then in the vSVA scenario, how can we get to know the mdev(cookie) from > the > rid and pasid? > > And from this point of view,would it be better to register the mdev > (iommu_register_device()) with the parent device info? > This is what is proposed in this RFC. A successful binding generates a new iommu_dev object for each vfio device. For mdev this object includes its parent device, the defPASID marking this mdev, and the cookie representing it in userspace. Later it is iommu_dev being recorded in the attaching_data when the mdev is attached to an IOASID: struct iommu_attach_data *__iommu_device_attach( struct iommu_dev *dev, u32 ioasid, u32 pasid, int flags); Then when a fault is reported, the fault handler just needs to figure out iommu_dev according to {rid, pasid} in the raw fault data. Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On 2021/7/9 15:48, Tian, Kevin wrote: > 4.6. I/O page fault > +++ > > uAPI is TBD. Here is just about the high-level flow from host IOMMU driver > to guest IOMMU driver and backwards. This flow assumes that I/O page faults > are reported via IOMMU interrupts. Some devices report faults via device > specific way instead of going through the IOMMU. That usage is not covered > here: > > - Host IOMMU driver receives a I/O page fault with raw fault_data {rid, > pasid, addr}; > > - Host IOMMU driver identifies the faulting I/O page table according to > {rid, pasid} and calls the corresponding fault handler with an opaque > object (registered by the handler) and raw fault_data (rid, pasid, addr); > > - IOASID fault handler identifies the corresponding ioasid and device > cookie according to the opaque object, generates an user fault_data > (ioasid, cookie, addr) in the fault region, and triggers eventfd to > userspace; > Hi, I have some doubts here: For mdev, it seems that the rid in the raw fault_data is the parent device's, then in the vSVA scenario, how can we get to know the mdev(cookie) from the rid and pasid? And from this point of view,would it be better to register the mdev (iommu_register_device()) with the parent device info? Thanks, Shenming ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 13/17] vdpa: factor out vhost_vdpa_pa_map() and vhost_vdpa_pa_unmap()
在 2021/7/14 下午5:57, Dan Carpenter 写道: On Wed, Jul 14, 2021 at 05:41:54PM +0800, Jason Wang wrote: 在 2021/7/14 下午4:05, Dan Carpenter 写道: On Wed, Jul 14, 2021 at 10:14:32AM +0800, Jason Wang wrote: 在 2021/7/13 下午7:31, Dan Carpenter 写道: On Tue, Jul 13, 2021 at 04:46:52PM +0800, Xie Yongji wrote: @@ -613,37 +618,28 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v, u64 iova, u64 size) } } -static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v, - struct vhost_iotlb_msg *msg) +static int vhost_vdpa_pa_map(struct vhost_vdpa *v, +u64 iova, u64 size, u64 uaddr, u32 perm) { struct vhost_dev *dev = &v->vdev; - struct vhost_iotlb *iotlb = dev->iotlb; struct page **page_list; unsigned long list_size = PAGE_SIZE / sizeof(struct page *); unsigned int gup_flags = FOLL_LONGTERM; unsigned long npages, cur_base, map_pfn, last_pfn = 0; unsigned long lock_limit, sz2pin, nchunks, i; - u64 iova = msg->iova; + u64 start = iova; long pinned; int ret = 0; - if (msg->iova < v->range.first || - msg->iova + msg->size - 1 > v->range.last) - return -EINVAL; This is not related to your patch, but can the "msg->iova + msg->size" addition can have an integer overflow. From looking at the callers it seems like it can. msg comes from: vhost_chr_write_iter() --> dev->msg_handler(dev, &msg); --> vhost_vdpa_process_iotlb_msg() --> vhost_vdpa_process_iotlb_update() Yes. If I'm thinking of the right thing then these are allowed to overflow to 0 because of the " - 1" but not further than that. I believe the check needs to be something like: if (msg->iova < v->range.first || msg->iova - 1 > U64_MAX - msg->size || I guess we don't need - 1 here? The - 1 is important. The highest address is 0x. So it goes start + size = 0 and then start + size - 1 == 0x. Right, so actually msg->iova = 0xfffe, msg->size=2 is valid. I believe so, yes. It's inclusive of 0xfffe and 0x. (Not an expert). I think so, and we probably need to fix vhost_overflow() as well which did: static bool vhost_overflow(u64 uaddr, u64 size) { /* Make sure 64 bit math will not overflow. */ return uaddr > ULONG_MAX || size > ULONG_MAX || uaddr > ULONG_MAX - size; } Thanks regards, dan carpenter ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 00/15] Optimizing iommu_[map/unmap] performance
在 2021/7/15 9:23, Lu Baolu 写道: On 7/14/21 10:24 PM, Georgi Djakov wrote: On 16.06.21 16:38, Georgi Djakov wrote: When unmapping a buffer from an IOMMU domain, the IOMMU framework unmaps the buffer at a granule of the largest page size that is supported by the IOMMU hardware and fits within the buffer. For every block that is unmapped, the IOMMU framework will call into the IOMMU driver, and then the io-pgtable framework to walk the page tables to find the entry that corresponds to the IOVA, and then unmaps the entry. This can be suboptimal in scenarios where a buffer or a piece of a buffer can be split into several contiguous page blocks of the same size. For example, consider an IOMMU that supports 4 KB page blocks, 2 MB page blocks, and 1 GB page blocks, and a buffer that is 4 MB in size is being unmapped at IOVA 0. The current call-flow will result in 4 indirect calls, and 2 page table walks, to unmap 2 entries that are next to each other in the page-tables, when both entries could have been unmapped in one shot by clearing both page table entries in the same call. The same optimization is applicable to mapping buffers as well, so these patches implement a set of callbacks called unmap_pages and map_pages to the io-pgtable code and IOMMU drivers which unmaps or maps an IOVA range that consists of a number of pages of the same page size that is supported by the IOMMU hardware, and allows for manipulating multiple page table entries in the same set of indirect calls. The reason for introducing these callbacks is to give other IOMMU drivers/io-pgtable formats time to change to using the new callbacks, so that the transition to using this approach can be done piecemeal. Hi Will, Did you get a chance to look at this patchset? Most patches are already acked/reviewed and all still applies clean on rc1. I also have the ops->[un]map_pages implementation for the Intel IOMMU driver. I will post them once the iommu/core part get applied. I also implement those callbacks on ARM SMMUV3 based on this series, and use dma_map_benchmark to have a test on the latency of map/unmap as follows, and i think it promotes much on the latency of map/unmap. I will also plan to post the implementations for ARM SMMUV3 after this series are applied. t = 1(thread = 1): before opt(us) after opt(us) g=1(4K size)0.1/1.3 0.1/0.8 g=2(8K size)0.2/1.5 0.2/0.9 g=4(16K size) 0.3/1.9 0.1/1.1 g=8(32K size) 0.5/2.7 0.2/1.4 g=16(64K size) 1.0/4.5 0.2/2.0 g=32(128K size) 1.8/7.9 0.2/3.3 g=64(256K size) 3.7/14.8 0.4/6.1 g=128(512K size)7.1/14.7 0.5/10.4 g=256(1M size) 14.0/53.90.8/19.3 g=512(2M size) 0.2/0.9 0.2/0.9 g=1024(4M size) 0.5/1.5 0.4/1.0 t = 10(thread = 10): before opt(us) after opt(us) g=1(4K size)0.3/7.0 0.1/5.8 g=2(8K size)0.4/6.7 0.3/6.0 g=4(16K size) 0.5/6.3 0.3/5.6 g=8(32K size) 0.5/8.3 0.2/6.3 g=16(64K size) 1.0/17.3 0.3/12.4 g=32(128K size) 1.8/36.0 0.2/24.2 g=64(256K size) 4.3/67.2 1.2/46.4 g=128(512K size)7.8/93.7 1.3/94.2 g=256(1M size) 14.7/280.8 1.8/191.5 g=512(2M size) 3.6/3.2 1.5/2.5 g=1024(4M size) 2.0/3.1 1.8/2.6 Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu . ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Unify iova_to_phys for identity domains
On 7/15/21 1:28 AM, Robin Murphy wrote: If people are going to insist on calling iommu_iova_to_phys() pointlessly and expecting it to work, we can at least do ourselves a favour by handling those cases in the core code, rather than repeatedly across an inconsistent handful of drivers. Signed-off-by: Robin Murphy --- drivers/iommu/amd/io_pgtable.c | 3 --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 --- drivers/iommu/iommu.c | 6 +- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c index bb0ee5c9fde7..182c93a43efd 100644 --- a/drivers/iommu/amd/io_pgtable.c +++ b/drivers/iommu/amd/io_pgtable.c @@ -493,9 +493,6 @@ static phys_addr_t iommu_v1_iova_to_phys(struct io_pgtable_ops *ops, unsigned lo unsigned long offset_mask, pte_pgsize; u64 *pte, __pte; - if (pgtable->mode == PAGE_MODE_NONE) - return iova; - pte = fetch_pte(pgtable, iova, &pte_pgsize); if (!pte || !IOMMU_PTE_PRESENT(*pte)) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 3e87a9cf6da3..c9fdd0d097be 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2481,9 +2481,6 @@ arm_smmu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova) { struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops; - if (domain->type == IOMMU_DOMAIN_IDENTITY) - return iova; - if (!ops) return 0; diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 0f181f76c31b..0d04eafa3fdb 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1317,9 +1317,6 @@ static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain, struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops; - if (domain->type == IOMMU_DOMAIN_IDENTITY) - return iova; - if (!ops) return 0; diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 43a2041d9629..7c16f977b5a6 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2371,7 +2371,11 @@ EXPORT_SYMBOL_GPL(iommu_detach_group); phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova) { - if (unlikely(domain->ops->iova_to_phys == NULL)) + if (domain->type == IOMMU_DOMAIN_IDENTITY) + return iova; + + if (unlikely(domain->ops->iova_to_phys == NULL) || + domain->type == IOMMU_DOMAIN_BLOCKED) return 0; Nit: Logically we only needs ops->iova_to_phys for DMA and UNMANAGED domains, so it looks better if we have if (domain->type == IOMMU_DOMAIN_BLOCKED || unlikely(domain->ops->iova_to_phys == NULL)) return 0; Anyway, Reviewed-by: Lu Baolu Best regards, baolu return domain->ops->iova_to_phys(domain, iova); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 00/15] Optimizing iommu_[map/unmap] performance
On 7/14/21 10:24 PM, Georgi Djakov wrote: On 16.06.21 16:38, Georgi Djakov wrote: When unmapping a buffer from an IOMMU domain, the IOMMU framework unmaps the buffer at a granule of the largest page size that is supported by the IOMMU hardware and fits within the buffer. For every block that is unmapped, the IOMMU framework will call into the IOMMU driver, and then the io-pgtable framework to walk the page tables to find the entry that corresponds to the IOVA, and then unmaps the entry. This can be suboptimal in scenarios where a buffer or a piece of a buffer can be split into several contiguous page blocks of the same size. For example, consider an IOMMU that supports 4 KB page blocks, 2 MB page blocks, and 1 GB page blocks, and a buffer that is 4 MB in size is being unmapped at IOVA 0. The current call-flow will result in 4 indirect calls, and 2 page table walks, to unmap 2 entries that are next to each other in the page-tables, when both entries could have been unmapped in one shot by clearing both page table entries in the same call. The same optimization is applicable to mapping buffers as well, so these patches implement a set of callbacks called unmap_pages and map_pages to the io-pgtable code and IOMMU drivers which unmaps or maps an IOVA range that consists of a number of pages of the same page size that is supported by the IOMMU hardware, and allows for manipulating multiple page table entries in the same set of indirect calls. The reason for introducing these callbacks is to give other IOMMU drivers/io-pgtable formats time to change to using the new callbacks, so that the transition to using this approach can be done piecemeal. Hi Will, Did you get a chance to look at this patchset? Most patches are already acked/reviewed and all still applies clean on rc1. I also have the ops->[un]map_pages implementation for the Intel IOMMU driver. I will post them once the iommu/core part get applied. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/24] dt-bindings: mediatek: mt8195: Add binding for infra IOMMU
On Wed, Jun 30, 2021 at 10:34:42AM +0800, Yong Wu wrote: > In mt8195, we have a new IOMMU that is for INFRA IOMMU. its masters > mainly are PCIe and USB. Different with MM IOMMU, all these masters > connect with IOMMU directly, there is no mediatek,larbs property for > infra IOMMU. > > Another thing is about PCIe ports. currently the function > "of_iommu_configure_dev_id" only support the id number is 1, But our > PCIe have two ports, one is for reading and the other is for writing. > see more about the PCIe patch in this patchset. Thus, I only list > the reading id here and add the other id in our driver. > > Signed-off-by: Yong Wu > --- > .../bindings/iommu/mediatek,iommu.yaml | 14 +- > .../dt-bindings/memory/mt8195-memory-port.h| 18 ++ > include/dt-bindings/memory/mtk-memory-port.h | 2 ++ > 3 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > index 9b04630158c8..6f3ff631c06b 100644 > --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > @@ -79,6 +79,7 @@ properties: >- mediatek,mt8192-m4u # generation two >- mediatek,mt8195-iommu-vdo# generation two >- mediatek,mt8195-iommu-vpp# generation two > + - mediatek,mt8195-iommu-infra # generation two > >- description: mt7623 generation one > items: > @@ -129,7 +130,6 @@ required: >- compatible >- reg >- interrupts > - - mediatek,larbs >- '#iommu-cells' > > allOf: > @@ -161,6 +161,18 @@ allOf: >required: > - power-domains > > + - if: > + not: > +properties: > + compatible: > +items: > + enum: > +- mediatek,mt8195-iommu-infra This is saying all items are 'mediatek,mt8195-iommu-infra'. Other schemas prevent that, but really this should be: compatible: contains: const: mediatek,mt8195-iommu-infra > + > +then: > + required: > +- mediatek,larbs > + > additionalProperties: false > > examples: > diff --git a/include/dt-bindings/memory/mt8195-memory-port.h > b/include/dt-bindings/memory/mt8195-memory-port.h > index 783bcae8cdea..67afad848725 100644 > --- a/include/dt-bindings/memory/mt8195-memory-port.h > +++ b/include/dt-bindings/memory/mt8195-memory-port.h > @@ -387,4 +387,22 @@ > #define M4U_PORT_L28_CAM_DRZS4NO_R1 MTK_M4U_ID(28, 5) > #define M4U_PORT_L28_CAM_TNCSO_R1MTK_M4U_ID(28, 6) > > +/* infra iommu ports */ > +/* PCIe1: read: BIT16; write BIT17. */ > +#define M4U_PORT_INFRA_PCIE1 MTK_IFAIOMMU_PERI_ID(16) > +/* PCIe0: read: BIT18; write BIT19. */ > +#define M4U_PORT_INFRA_PCIE0 MTK_IFAIOMMU_PERI_ID(18) > +#define M4U_PORT_INFRA_SSUSB_P3_RMTK_IFAIOMMU_PERI_ID(20) > +#define M4U_PORT_INFRA_SSUSB_P3_WMTK_IFAIOMMU_PERI_ID(21) > +#define M4U_PORT_INFRA_SSUSB_P2_RMTK_IFAIOMMU_PERI_ID(22) > +#define M4U_PORT_INFRA_SSUSB_P2_WMTK_IFAIOMMU_PERI_ID(23) > +#define M4U_PORT_INFRA_SSUSB_P1_1_R MTK_IFAIOMMU_PERI_ID(24) > +#define M4U_PORT_INFRA_SSUSB_P1_1_W MTK_IFAIOMMU_PERI_ID(25) > +#define M4U_PORT_INFRA_SSUSB_P1_0_R MTK_IFAIOMMU_PERI_ID(26) > +#define M4U_PORT_INFRA_SSUSB_P1_0_W MTK_IFAIOMMU_PERI_ID(27) > +#define M4U_PORT_INFRA_SSUSB2_R MTK_IFAIOMMU_PERI_ID(28) > +#define M4U_PORT_INFRA_SSUSB2_W MTK_IFAIOMMU_PERI_ID(29) > +#define M4U_PORT_INFRA_SSUSB_R MTK_IFAIOMMU_PERI_ID(30) > +#define M4U_PORT_INFRA_SSUSB_W MTK_IFAIOMMU_PERI_ID(31) > + > #endif > diff --git a/include/dt-bindings/memory/mtk-memory-port.h > b/include/dt-bindings/memory/mtk-memory-port.h > index 7d64103209af..2f68a0511a25 100644 > --- a/include/dt-bindings/memory/mtk-memory-port.h > +++ b/include/dt-bindings/memory/mtk-memory-port.h > @@ -12,4 +12,6 @@ > #define MTK_M4U_TO_LARB(id) (((id) >> 5) & 0x1f) > #define MTK_M4U_TO_PORT(id) ((id) & 0x1f) > > +#define MTK_IFAIOMMU_PERI_ID(port) MTK_M4U_ID(0, port) > + > #endif > -- > 2.18.0 > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/24] dt-bindings: mediatek: mt8195: Add binding for MM IOMMU
On Wed, 30 Jun 2021 10:34:41 +0800, Yong Wu wrote: > This patch adds descriptions for mt8195 IOMMU which also use ARM > Short-Descriptor translation table format. > > In mt8195, there are two smi-common HW and IOMMU, one is for vdo(video > output), the other is for vpp(video processing pipe). They connects > with different smi-larbs, then some setting(larbid_remap) is different. > Differentiate them with the compatible string. > > Something like this: > > IOMMU(VDO) IOMMU(VPP) >| | > SMI_COMMON_VDO SMI_COMMON_VPP > --- > | | ... | | ... > larb0 larb2 ...larb1 larb3... > > Another change is that we have a new IOMMU that is for infra master like > PCIe and USB. The infra master don't have the larb and ports, thus we > rename the port header file to mt8195-memory-port.h rather than > mt8195-larb-port.h. > > Also, the IOMMU is not only for MM, thus, we don't call it "m4u" which > means "MultiMedia Memory Management UNIT". thus, use the "iommu" as the > compatiable string. > > Signed-off-by: Yong Wu > --- > .../bindings/iommu/mediatek,iommu.yaml| 7 + > .../dt-bindings/memory/mt8195-memory-port.h | 390 ++ > 2 files changed, 397 insertions(+) > create mode 100644 include/dt-bindings/memory/mt8195-memory-port.h > Reviewed-by: Rob Herring ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] MAINTAINERS: Add Suravee Suthikulpanit as Reviewer for AMD IOMMU (AMD-Vi)
To help review changes related to AMD IOMMU. Signed-off-by: Suravee Suthikulpanit --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index b80e6f7..8022dbd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -933,6 +933,7 @@ F: drivers/video/fbdev/geode/ AMD IOMMU (AMD-VI) M: Joerg Roedel +R: Suravee Suthikulpanit L: iommu@lists.linux-foundation.org S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git -- 1.8.3.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 0/3] Apple M1 DART IOMMU driver
On Wed, Jul 14, 2021 at 8:21 PM Robin Murphy wrote: > > On 2021-06-27 15:34, Sven Peter wrote: > [...] > > In the long term, I'd like to extend the dma-iommu framework itself to > > support iommu pagesizes with a larger granule than the CPU pagesize if that > > is > > something you agree with. > > BTW this isn't something we can fully support in general. IOMMU API > users may expect this to work: > > iommu_map(domain, iova, page_to_phys(p1), PAGE_SIZE, prot); > iommu_map(domain, iova + PAGE_SIZE, page_to_phys(p2), PAGE_SIZE, prot); > > Although they do in principle have visibility of pgsize_bitmap, I still > doubt anyone is really prepared for CPU-page-aligned mappings to fail. > Even at the DMA API level you could hide *some* of it (at the cost of > effectively only having 1/4 of the usable address space), but there are > still cases like where v4l2 has a hard requirement that a page-aligned > scatterlist can be mapped into a contiguous region of DMA addresses. I think that was the same conclusion we had earlier: the dma-mapping interfaces should be possible for large iotlb pages, but any driver directly using the IOMMU API, such as VFIO, would not work. The question is how we can best allow one but not the other. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 0/3] Apple M1 DART IOMMU driver
On 2021-06-27 15:34, Sven Peter wrote: [...] In the long term, I'd like to extend the dma-iommu framework itself to support iommu pagesizes with a larger granule than the CPU pagesize if that is something you agree with. BTW this isn't something we can fully support in general. IOMMU API users may expect this to work: iommu_map(domain, iova, page_to_phys(p1), PAGE_SIZE, prot); iommu_map(domain, iova + PAGE_SIZE, page_to_phys(p2), PAGE_SIZE, prot); Although they do in principle have visibility of pgsize_bitmap, I still doubt anyone is really prepared for CPU-page-aligned mappings to fail. Even at the DMA API level you could hide *some* of it (at the cost of effectively only having 1/4 of the usable address space), but there are still cases like where v4l2 has a hard requirement that a page-aligned scatterlist can be mapped into a contiguous region of DMA addresses. This would be important to later support the thunderbolt DARTs since I would be very uncomfortable to have these running in (software or hardware) bypass mode. Funnily enough that's the one case that would be relatively workable, since untrusted devices are currently subject to bounce-buffering of the entire DMA request, so it doesn't matter so much how the bounce buffer itself is mapped. Even with the possible future optimisation of only bouncing the non-page-aligned start and end parts of a buffer I think it still works (the physical alignment just has to be considered in terms of the IOMMU granule). Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/3] iommu: io-pgtable: add DART pagetable format
Hi, On Tue, Jul 13, 2021, at 21:17, Robin Murphy wrote: > On 2021-06-27 15:34, Sven Peter wrote: > > Apple's DART iommu uses a pagetable format that shares some > > similarities with the ones already implemented by io-pgtable.c. > > Add a new format variant to support the required differences > > so that we don't have to duplicate the pagetable handling code. > > > > Signed-off-by: Sven Peter > > --- > > drivers/iommu/io-pgtable-arm.c | 62 ++ > > drivers/iommu/io-pgtable.c | 1 + > > include/linux/io-pgtable.h | 7 > > 3 files changed, 70 insertions(+) > > > > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c > > index 87def58e79b5..1dd5c45b4b5b 100644 > > --- a/drivers/iommu/io-pgtable-arm.c > > +++ b/drivers/iommu/io-pgtable-arm.c > > @@ -127,6 +127,9 @@ > > #define ARM_MALI_LPAE_MEMATTR_IMP_DEF 0x88ULL > > #define ARM_MALI_LPAE_MEMATTR_WRITE_ALLOC 0x8DULL > > > > +#define APPLE_DART_PTE_PROT_NO_WRITE (1<<7) > > +#define APPLE_DART_PTE_PROT_NO_READ (1<<8) > > + > > /* IOPTE accessors */ > > #define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d)) > > > > @@ -381,6 +384,15 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct > > arm_lpae_io_pgtable *data, > > { > > arm_lpae_iopte pte; > > > > + if (data->iop.fmt == ARM_APPLE_DART) { > > + pte = 0; > > + if (!(prot & IOMMU_WRITE)) > > + pte |= APPLE_DART_PTE_PROT_NO_WRITE; > > + if (!(prot & IOMMU_READ)) > > + pte |= APPLE_DART_PTE_PROT_NO_READ; > > + return pte; > > + } > > + > > if (data->iop.fmt == ARM_64_LPAE_S1 || > > data->iop.fmt == ARM_32_LPAE_S1) { > > pte = ARM_LPAE_PTE_nG; > > @@ -1043,6 +1055,51 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg > > *cfg, void *cookie) > > return NULL; > > } > > > > +static struct io_pgtable * > > +apple_dart_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie) > > +{ > > + struct arm_lpae_io_pgtable *data; > > + int i; > > + > > + if (cfg->oas > 36) > > + return NULL; > > + > > + data = arm_lpae_alloc_pgtable(cfg); > > + if (!data) > > + return NULL; > > + > > + /* > > +* Apple's DART always requires three levels with the first level being > > +* stored in four MMIO registers. We always concatenate the first and > > +* second level so that we only have to setup the MMIO registers once. > > +* This results in an effective two level pagetable. > > +*/ > > Nit: I appreciate the effort to document the weirdness, but this comment > did rather mislead me initially, and now that I (think I) understand how > things work it seems a bit backwards. Could we say something like: > >"The table format itself always uses two levels, but the total VA > space is mapped by four separate tables, making the MMIO registers > an effective "level 1". For simplicity, though, we treat this > equivalently to LPAE stage 2 concatenation at level 2, with the > additional TTBRs each just pointing at consecutive pages." > > ? > Sure, your version is much easier to understand! Thanks. > > + if (data->start_level < 1) > > + return NULL; > > + if (data->start_level == 1 && data->pgd_bits > 2) > > + return NULL; > > + if (data->start_level > 1) > > + data->pgd_bits = 0; > > + data->start_level = 2; > > + cfg->apple_dart_cfg.n_ttbrs = 1 << data->pgd_bits; > > + data->pgd_bits += data->bits_per_level; > > + > > + data->pgd = __arm_lpae_alloc_pages(ARM_LPAE_PGD_SIZE(data), GFP_KERNEL, > > + cfg); > > + if (!data->pgd) > > + goto out_free_data; > > + > > + for (i = 0; i < cfg->apple_dart_cfg.n_ttbrs; ++i) > > + cfg->apple_dart_cfg.ttbr[i] = > > + virt_to_phys(data->pgd + i * ARM_LPAE_GRANULE(data)); > > + > > + return &data->iop; > > + > > +out_free_data: > > + kfree(data); > > + return NULL; > > +} > > + > > struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = { > > .alloc = arm_64_lpae_alloc_pgtable_s1, > > .free = arm_lpae_free_pgtable, > > @@ -1068,6 +1125,11 @@ struct io_pgtable_init_fns > > io_pgtable_arm_mali_lpae_init_fns = { > > .free = arm_lpae_free_pgtable, > > }; > > > > +struct io_pgtable_init_fns io_pgtable_apple_dart_init_fns = { > > + .alloc = apple_dart_alloc_pgtable, > > + .free = arm_lpae_free_pgtable, > > +}; > > + > > #ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST > > > > static struct io_pgtable_cfg *cfg_cookie __initdata; > > diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c > > index 6e9917ce980f..fd8e6bd6caf9 100644 > > --- a/drivers/iommu/io-pgtable.c > > +++ b/drivers/iommu/io-pgtable.c > > @@ -20,6 +20,7 @@ io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = { > > [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns, > > [
[PATCH] iommu: Unify iova_to_phys for identity domains
If people are going to insist on calling iommu_iova_to_phys() pointlessly and expecting it to work, we can at least do ourselves a favour by handling those cases in the core code, rather than repeatedly across an inconsistent handful of drivers. Signed-off-by: Robin Murphy --- drivers/iommu/amd/io_pgtable.c | 3 --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 --- drivers/iommu/iommu.c | 6 +- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c index bb0ee5c9fde7..182c93a43efd 100644 --- a/drivers/iommu/amd/io_pgtable.c +++ b/drivers/iommu/amd/io_pgtable.c @@ -493,9 +493,6 @@ static phys_addr_t iommu_v1_iova_to_phys(struct io_pgtable_ops *ops, unsigned lo unsigned long offset_mask, pte_pgsize; u64 *pte, __pte; - if (pgtable->mode == PAGE_MODE_NONE) - return iova; - pte = fetch_pte(pgtable, iova, &pte_pgsize); if (!pte || !IOMMU_PTE_PRESENT(*pte)) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 3e87a9cf6da3..c9fdd0d097be 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2481,9 +2481,6 @@ arm_smmu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova) { struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops; - if (domain->type == IOMMU_DOMAIN_IDENTITY) - return iova; - if (!ops) return 0; diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 0f181f76c31b..0d04eafa3fdb 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1317,9 +1317,6 @@ static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain, struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops; - if (domain->type == IOMMU_DOMAIN_IDENTITY) - return iova; - if (!ops) return 0; diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 43a2041d9629..7c16f977b5a6 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2371,7 +2371,11 @@ EXPORT_SYMBOL_GPL(iommu_detach_group); phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova) { - if (unlikely(domain->ops->iova_to_phys == NULL)) + if (domain->type == IOMMU_DOMAIN_IDENTITY) + return iova; + + if (unlikely(domain->ops->iova_to_phys == NULL) || + domain->type == IOMMU_DOMAIN_BLOCKED) return 0; return domain->ops->iova_to_phys(domain, iova); -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 0/3] iommu: Enable non-strict DMA on QCom SD/MMC
Hi, On Tue, Jul 13, 2021 at 11:07 AM Robin Murphy wrote: > > On 2021-07-08 15:36, Doug Anderson wrote: > [...] > >> Or document for the users that want performance how to > >> change the setting, so that they can decide. > > > > Pushing this to the users can make sense for a Linux distribution but > > probably less sense for an embedded platform. So I'm happy to make > > some way for a user to override this (like via kernel command line), > > but I also strongly believe there should be a default that users don't > > have to futz with that we think is correct. > > FYI I did make progress on the "punt it to userspace" approach. I'm not > posting it even as an RFC yet because I still need to set up a machine > to try actually testing any of it (it's almost certainly broken > somewhere), but in the end it comes out looking surprisingly not too bad > overall. If you're curious to take a look in the meantime I put it here: > > https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/fq Being able to change this at runtime through sysfs sounds great and it fills all the needs I'm aware of, thanks! In Chrome OS we can just use this with some udev rules and get everything we need. I'm happy to give this a spin when you're ready for extra testing. -Doug ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 00/15] Optimizing iommu_[map/unmap] performance
On 16.06.21 16:38, Georgi Djakov wrote: When unmapping a buffer from an IOMMU domain, the IOMMU framework unmaps the buffer at a granule of the largest page size that is supported by the IOMMU hardware and fits within the buffer. For every block that is unmapped, the IOMMU framework will call into the IOMMU driver, and then the io-pgtable framework to walk the page tables to find the entry that corresponds to the IOVA, and then unmaps the entry. This can be suboptimal in scenarios where a buffer or a piece of a buffer can be split into several contiguous page blocks of the same size. For example, consider an IOMMU that supports 4 KB page blocks, 2 MB page blocks, and 1 GB page blocks, and a buffer that is 4 MB in size is being unmapped at IOVA 0. The current call-flow will result in 4 indirect calls, and 2 page table walks, to unmap 2 entries that are next to each other in the page-tables, when both entries could have been unmapped in one shot by clearing both page table entries in the same call. The same optimization is applicable to mapping buffers as well, so these patches implement a set of callbacks called unmap_pages and map_pages to the io-pgtable code and IOMMU drivers which unmaps or maps an IOVA range that consists of a number of pages of the same page size that is supported by the IOMMU hardware, and allows for manipulating multiple page table entries in the same set of indirect calls. The reason for introducing these callbacks is to give other IOMMU drivers/io-pgtable formats time to change to using the new callbacks, so that the transition to using this approach can be done piecemeal. Hi Will, Did you get a chance to look at this patchset? Most patches are already acked/reviewed and all still applies clean on rc1. Thanks, Georgi ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 03/17] vdpa: Fix code indentation
On Tue, 2021-07-13 at 16:46 +0800, Xie Yongji wrote: > Use tabs to indent the code instead of spaces. There are a lot more of these in this file. $ ./scripts/checkpatch.pl --fix-inplace --strict include/linux/vdpa.h and a little typing gives: --- include/linux/vdpa.h | 50 +- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index 3357ac98878d4..14cd4248e59fd 100644 --- a/include/linux/vdpa.h +++ b/include/linux/vdpa.h @@ -43,17 +43,17 @@ struct vdpa_vq_state_split { * @last_used_idx: used index */ struct vdpa_vq_state_packed { -u16last_avail_counter:1; -u16last_avail_idx:15; -u16last_used_counter:1; -u16last_used_idx:15; + u16 last_avail_counter:1; + u16 last_avail_idx:15; + u16 last_used_counter:1; + u16 last_used_idx:15; }; struct vdpa_vq_state { - union { - struct vdpa_vq_state_split split; - struct vdpa_vq_state_packed packed; - }; + union { + struct vdpa_vq_state_split split; + struct vdpa_vq_state_packed packed; + }; }; struct vdpa_mgmt_dev; @@ -131,7 +131,7 @@ struct vdpa_iova_range { * @vdev: vdpa device * @idx: virtqueue index * @state: pointer to returned state (last_avail_idx) - * @get_vq_notification: Get the notification area for a virtqueue + * @get_vq_notification: Get the notification area for a virtqueue * @vdev: vdpa device * @idx: virtqueue index * Returns the notifcation area @@ -277,13 +277,13 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent, const struct vdpa_config_ops *config, size_t size, const char *name); -#define vdpa_alloc_device(dev_struct, member, parent, config, name) \ - container_of(__vdpa_alloc_device( \ - parent, config, \ - sizeof(dev_struct) + \ - BUILD_BUG_ON_ZERO(offsetof( \ - dev_struct, member)), name), \ - dev_struct, member) +#define vdpa_alloc_device(dev_struct, member, parent, config, name)\ + container_of(__vdpa_alloc_device(parent, config,\ +sizeof(dev_struct) + \ +BUILD_BUG_ON_ZERO(offsetof(dev_struct, \ + member)), \ +name), \ +dev_struct, member) int vdpa_register_device(struct vdpa_device *vdev, int nvqs); void vdpa_unregister_device(struct vdpa_device *vdev); @@ -308,8 +308,8 @@ struct vdpa_driver { int __vdpa_register_driver(struct vdpa_driver *drv, struct module *owner); void vdpa_unregister_driver(struct vdpa_driver *drv); -#define module_vdpa_driver(__vdpa_driver) \ - module_driver(__vdpa_driver, vdpa_register_driver, \ +#define module_vdpa_driver(__vdpa_driver) \ + module_driver(__vdpa_driver, vdpa_register_driver, \ vdpa_unregister_driver) static inline struct vdpa_driver *drv_to_vdpa(struct device_driver *driver) @@ -339,25 +339,25 @@ static inline struct device *vdpa_get_dma_dev(struct vdpa_device *vdev) static inline void vdpa_reset(struct vdpa_device *vdev) { -const struct vdpa_config_ops *ops = vdev->config; + const struct vdpa_config_ops *ops = vdev->config; vdev->features_valid = false; -ops->set_status(vdev, 0); + ops->set_status(vdev, 0); } static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features) { -const struct vdpa_config_ops *ops = vdev->config; + const struct vdpa_config_ops *ops = vdev->config; vdev->features_valid = true; -return ops->set_features(vdev, features); + return ops->set_features(vdev, features); } - -static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned offset, +static inline void vdpa_get_config(struct vdpa_device *vdev, + unsigned int offset, void *buf, unsigned int len) { -const struct vdpa_config_ops *ops = vdev->config; + const struct vdpa_config_ops *ops = vdev->config; /* * Config accesses aren't supposed to trigger before features are set. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/m
[PATCH v2 0/4] iommu/vt-d: Disable igfx iommu superpage on bxt/skl/glk
From: Ville Syrjälä I ran into some kind of fail with VT-d superpage on Geminlake igfx, so without any better ideas let's just disable it. Additionally Skylake/Broxton igfx have known issues with VT-d superpage as well, so let's disable it there as well. This should let us re-enable frame buffer compression (FBC) for some extra power savings when the display is on. v2: disable superpage only for the igfx iommu Cc: David Woodhouse Cc: Lu Baolu Cc: iommu@lists.linux-foundation.org Ville Syrjälä (4): iommu/vt-d: Disable superpage for Geminilake igfx iommu/vt-d: Disable superpage for Broxton igfx iommu/vt-d: Disable superpage for Skylake igfx drm/i915/fbc: Allow FBC + VT-d on SKL/BXT drivers/gpu/drm/i915/display/intel_fbc.c | 16 -- drivers/iommu/intel/iommu.c | 68 +++- 2 files changed, 67 insertions(+), 17 deletions(-) -- 2.31.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 3/4] iommu/vt-d: Disable superpage for Skylake igfx
From: Ville Syrjälä Skylake has known issues with VT-d superpage. Namely frame buffer compression (FBC) can't be safely used when superpage is enabled. Currently we're disabling FBC entirely when VT-d is active, but I think just disabling superpage would be better since FBC can save some power. TODO: would be nice to use the macros from include/drm/i915_pciids.h, but can't do that with DECLARE_PCI_FIXUP_HEADER() Cc: David Woodhouse Cc: Lu Baolu Cc: iommu@lists.linux-foundation.org Signed-off-by: Ville Syrjälä --- drivers/iommu/intel/iommu.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index ef717908647d..ea9c69dc13f5 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -5681,6 +5681,33 @@ static void quirk_skip_igfx_superpage(struct pci_dev *dev) iommu_skip_igfx_superpage = 1; } +/* Skylake igfx has issues with superpage */ +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1906, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1913, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x190E, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1915, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1902, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x190A, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x190B, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1917, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1916, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1921, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x191E, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1912, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x191A, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x191B, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x191D, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1923, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1926, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1927, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x192A, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x192B, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x192D, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1932, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x193A, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x193B, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x193D, quirk_skip_igfx_superpage); + /* Broxton igfx has issues with superpage */ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0A84, quirk_skip_igfx_superpage); DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1A84, quirk_skip_igfx_superpage); -- 2.31.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 4/4] drm/i915/fbc: Allow FBC + VT-d on SKL/BXT
From: Ville Syrjälä With the iommu driver disabling VT-d superpage it should be safe to use FBC on SKL/BXT with VT-d otherwise enabled. Cc: David Woodhouse Cc: Lu Baolu Cc: iommu@lists.linux-foundation.org Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_fbc.c | 16 1 file changed, 16 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 82effb64a3b9..de44f93a33d0 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1448,19 +1448,6 @@ static int intel_sanitize_fbc_option(struct drm_i915_private *dev_priv) return 0; } -static bool need_fbc_vtd_wa(struct drm_i915_private *dev_priv) -{ - /* WaFbcTurnOffFbcWhenHyperVisorIsUsed:skl,bxt */ - if (intel_vtd_active() && - (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))) { - drm_info(&dev_priv->drm, -"Disabling framebuffer compression (FBC) to prevent screen flicker with VT-d enabled\n"); - return true; - } - - return false; -} - /** * intel_fbc_init - Initialize FBC * @dev_priv: the i915 device @@ -1478,9 +1465,6 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) if (!drm_mm_initialized(&dev_priv->mm.stolen)) mkwrite_device_info(dev_priv)->display.has_fbc = false; - if (need_fbc_vtd_wa(dev_priv)) - mkwrite_device_info(dev_priv)->display.has_fbc = false; - dev_priv->params.enable_fbc = intel_sanitize_fbc_option(dev_priv); drm_dbg_kms(&dev_priv->drm, "Sanitized enable_fbc value: %d\n", dev_priv->params.enable_fbc); -- 2.31.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/4] iommu/vt-d: Disable superpage for Geminilake igfx
From: Ville Syrjälä While running "gem_exec_big --r single" from igt-gpu-tools on Geminilake as soon as a 2M mapping is made I tend to get a DMAR write fault. Strangely the faulting address is always a 4K page and usually very far away from the 2M page that got mapped. But if no 2M mappings get used I can't reproduce the fault. I also tried to dump the PTE for the faulting address but it actually looks correct to me (ie. definitely seems to have the write bit set): DMAR: DRHD: handling fault status reg 2 DMAR: [DMA Write] Request device [00:02.0] PASID fault addr 7fa8a78000 [fault reason 05] PTE Write access is not set DMAR: fault 7fa8a78000 (level=1) PTE = 149efc003 So not really sure what's going on and this might just be full on duct tape, but it seems to work here. The machine has now survived a whole day running that test whereas with superpage enabled it fails in less than a minute usually. Credit to Lu Baolu for the mechanism to disable superpage just for the igfx iommu. TODO: would be nice to use the macros from include/drm/i915_pciids.h, but can't do that with DECLARE_PCI_FIXUP_HEADER() Cc: David Woodhouse Cc: Lu Baolu Cc: iommu@lists.linux-foundation.org Signed-off-by: Ville Syrjälä --- drivers/iommu/intel/iommu.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index b04bfb0d9409..08ba412053e3 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -365,6 +365,7 @@ static int intel_iommu_strict; static int intel_iommu_superpage = 1; static int iommu_identity_mapping; static int iommu_skip_te_disable; +static int iommu_skip_igfx_superpage; #define IDENTMAP_GFX 2 #define IDENTMAP_AZALIA4 @@ -674,6 +675,27 @@ static bool domain_update_iommu_snooping(struct intel_iommu *skip) return ret; } +static bool domain_use_super_page(struct dmar_domain *domain) +{ + struct dmar_drhd_unit *drhd; + struct intel_iommu *iommu; + bool ret = true; + + if (!intel_iommu_superpage) + return false; + + rcu_read_lock(); + for_each_active_iommu(iommu, drhd) { + if (drhd->gfx_dedicated && iommu_skip_igfx_superpage) { + ret = false; + break; + } + } + rcu_read_unlock(); + + return ret; +} + static int domain_update_iommu_superpage(struct dmar_domain *domain, struct intel_iommu *skip) { @@ -681,7 +703,7 @@ static int domain_update_iommu_superpage(struct dmar_domain *domain, struct intel_iommu *iommu; int mask = 0x3; - if (!intel_iommu_superpage) + if (!domain_use_super_page(domain)) return 0; /* set iommu_superpage to the smallest common denominator */ @@ -5653,6 +5675,16 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1632, quirk_iommu_igfx); DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163A, quirk_iommu_igfx); DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x163D, quirk_iommu_igfx); +static void quirk_skip_igfx_superpage(struct pci_dev *dev) +{ + pci_info(dev, "Disabling IOMMU superpage for graphics on this chipset\n"); + iommu_skip_igfx_superpage = 1; +} + +/* Geminilake igfx appears to have issues with superpage */ +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x3184, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x3185, quirk_skip_igfx_superpage); + static void quirk_iommu_rwbf(struct pci_dev *dev) { if (risky_device(dev)) -- 2.31.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 2/4] iommu/vt-d: Disable superpage for Broxton igfx
From: Ville Syrjälä Broxton has known issues with VT-d superpage. Namely frame buffer compression (FBC) can't be safely used when superpage is enabled. Currently we're disabling FBC entirely when VT-d is active, but I think just disabling superpage would be better since FBC can save some power. TODO: would be nice to use the macros from include/drm/i915_pciids.h, but can't do that with DECLARE_PCI_FIXUP_HEADER() Cc: David Woodhouse Cc: Lu Baolu Cc: iommu@lists.linux-foundation.org Signed-off-by: Ville Syrjälä --- drivers/iommu/intel/iommu.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 08ba412053e3..ef717908647d 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -5681,6 +5681,13 @@ static void quirk_skip_igfx_superpage(struct pci_dev *dev) iommu_skip_igfx_superpage = 1; } +/* Broxton igfx has issues with superpage */ +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0A84, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1A84, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1A85, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5A84, quirk_skip_igfx_superpage); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5A85, quirk_skip_igfx_superpage); + /* Geminilake igfx appears to have issues with superpage */ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x3184, quirk_skip_igfx_superpage); DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x3185, quirk_skip_igfx_superpage); -- 2.31.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dt-bindings: More dropping redundant minItems/maxItems
On 13/07/2021 13:34:53-0600, Rob Herring wrote: > Another round of removing redundant minItems/maxItems from new schema in > the recent merge window. > > If a property has an 'items' list, then a 'minItems' or 'maxItems' with the > same size as the list is redundant and can be dropped. Note that is DT > schema specific behavior and not standard json-schema behavior. The tooling > will fixup the final schema adding any unspecified minItems/maxItems. > > This condition is partially checked with the meta-schema already, but > only if both 'minItems' and 'maxItems' are equal to the 'items' length. > An improved meta-schema is pending. > > Cc: Stephen Boyd > Cc: Joerg Roedel > Cc: Will Deacon > Cc: Krzysztof Kozlowski > Cc: Miquel Raynal > Cc: Richard Weinberger > Cc: Vignesh Raghavendra > Cc: Alessandro Zummo > Cc: Alexandre Belloni > Cc: Greg Kroah-Hartman > Cc: Sureshkumar Relli > Cc: Brian Norris > Cc: Kamal Dasu > Cc: Linus Walleij > Cc: Sebastian Siewior > Cc: Laurent Pinchart > Cc: linux-...@vger.kernel.org > Cc: iommu@lists.linux-foundation.org > Cc: linux-...@lists.infradead.org > Cc: linux-...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Signed-off-by: Rob Herring Acked-by: Alexandre Belloni > --- > .../devicetree/bindings/clock/brcm,iproc-clocks.yaml | 1 - > .../devicetree/bindings/iommu/rockchip,iommu.yaml | 2 -- > .../bindings/memory-controllers/arm,pl353-smc.yaml| 1 - > Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 > .../devicetree/bindings/rtc/faraday,ftrtc010.yaml | 1 - > Documentation/devicetree/bindings/usb/nxp,isp1760.yaml| 2 -- > 6 files changed, 15 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.yaml > b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.yaml > index 8dc7b404ee12..1174c9aa9934 100644 > --- a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.yaml > +++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.yaml > @@ -50,7 +50,6 @@ properties: > >reg: > minItems: 1 > -maxItems: 3 > items: >- description: base register >- description: power register > diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml > b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml > index d2e28a9e3545..ba9124f721f1 100644 > --- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml > +++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml > @@ -28,14 +28,12 @@ properties: >- description: configuration registers for MMU instance 0 >- description: configuration registers for MMU instance 1 > minItems: 1 > -maxItems: 2 > >interrupts: > items: >- description: interruption for MMU instance 0 >- description: interruption for MMU instance 1 > minItems: 1 > -maxItems: 2 > >clocks: > items: > diff --git > a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml > b/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml > index 7a63c85ef8c5..01c9acf9275d 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml > +++ b/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml > @@ -57,7 +57,6 @@ properties: > >ranges: > minItems: 1 > -maxItems: 3 > description: | >Memory bus areas for interacting with the devices. Reflects >the memory layout with four integer values following: > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > index e5f1a2a5..dd5a64969e37 100644 > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > @@ -84,7 +84,6 @@ properties: > >interrupts: > minItems: 1 > -maxItems: 3 > items: >- description: NAND CTLRDY interrupt >- description: FLASH_DMA_DONE if flash DMA is available > @@ -92,7 +91,6 @@ properties: > >interrupt-names: > minItems: 1 > -maxItems: 3 > items: >- const: nand_ctlrdy >- const: flash_dma_done > @@ -148,8 +146,6 @@ allOf: > then: >properties: > reg-names: > - minItems: 2 > - maxItems: 2 >items: > - const: nand > - const: nand-int-base > @@ -161,8 +157,6 @@ allOf: > then: >properties: > reg-names: > - minItems: 3 > - maxItems: 3 >items: > - const: nand > - const: nand-int-base > @@ -175,8 +169,6 @@ allOf: > then: >properties: > reg-names: > - minItems: 3 > - maxItems: 3 >items: > - const: nand > - const: iproc-idm > diff --git a/Documentation/devicetree/bindings/rtc/faraday,ftrtc010.yaml > b/Document
Re: [PATCH] dt-bindings: More dropping redundant minItems/maxItems
On Tue, Jul 13, 2021 at 01:34:53PM -0600, Rob Herring wrote: > Another round of removing redundant minItems/maxItems from new schema in > the recent merge window. > > If a property has an 'items' list, then a 'minItems' or 'maxItems' with the > same size as the list is redundant and can be dropped. Note that is DT > schema specific behavior and not standard json-schema behavior. The tooling > will fixup the final schema adding any unspecified minItems/maxItems. > > This condition is partially checked with the meta-schema already, but > only if both 'minItems' and 'maxItems' are equal to the 'items' length. > An improved meta-schema is pending. > > Cc: Stephen Boyd > Cc: Joerg Roedel > Cc: Will Deacon > Cc: Krzysztof Kozlowski > Cc: Miquel Raynal > Cc: Richard Weinberger > Cc: Vignesh Raghavendra > Cc: Alessandro Zummo > Cc: Alexandre Belloni > Cc: Greg Kroah-Hartman > Cc: Sureshkumar Relli > Cc: Brian Norris > Cc: Kamal Dasu > Cc: Linus Walleij > Cc: Sebastian Siewior > Cc: Laurent Pinchart > Cc: linux-...@vger.kernel.org > Cc: iommu@lists.linux-foundation.org > Cc: linux-...@lists.infradead.org > Cc: linux-...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Signed-off-by: Rob Herring Reviewed-by: Greg Kroah-Hartman ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Aw: Re: [PATCH v6 00/11] Clean up "mediatek,larb"
> Gesendet: Mittwoch, 14. Juli 2021 um 13:18 Uhr > Von: "Yong Wu" > Hi Frank, > > Thanks for your report. mt7623 use mtk_iommu_v1.c. > > I will try to reproduce this locally. Hi, as far as i have debugged it dev->iommu_group is NULL, so it crashes on first access (dev_info) drivers/iommu/iommu.c: 923 void iommu_group_remove_device(struct device *dev) 924 { 925 printk(KERN_ALERT "DEBUG: Passed %s %d \n",__FUNCTION__,__LINE__); 926 struct iommu_group *group = dev->iommu_group; 927 struct group_device *tmp_device, *device = NULL; 928 929 printk(KERN_ALERT "DEBUG: Passed %s %d 0x%08x\n",__FUNCTION__,__LINE__,(unsigned int)group); 930 dev_info(dev, "Removing from iommu group %d\n", group->id); regards Frank ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: Aw: [PATCH v6 00/11] Clean up "mediatek,larb"
On Wed, 2021-07-14 at 10:59 +0200, Frank Wunderlich wrote: > Hi, > > sorry this (or the 2 depency-series) cause a NULL Pointer deref in > iommu_group_remove_device on mt7623/bpi-r2 Hi Frank, Thanks for your report. mt7623 use mtk_iommu_v1.c. I will try to reproduce this locally. > > i wonder why on bootup a cleanup is run, but have no hint about this. > > since "dts: mtk-mdp: remove mediatek, vpu property from primary MDP device" > all is good, i guess problem comes up while removing larb with DT > > this is backtrace > > [6.274465] PC is at iommu_group_remove_device+0x28/0x148 > [6.279877] LR is at iommu_release_device+0x4c/0x70 > > [6.674347] Backtrace: > [6.676797] [] (iommu_group_remove_device) from [] > (iomm) > [6.686221] r7: r6:c06bf04c r5:c0d7a1ac r4:c21fc010 > [6.691883] [] (iommu_release_device) from [] > (remove_io) > [6.700689] r5: r4: > [6.704265] [] (remove_iommu_group) from [] > (bus_for_eac) > [6.712725] [] (bus_for_each_dev) from [] > (bus_set_iommu) > [6.720753] r6:c331f440 r5:c1406f58 r4:ffea > [6.725370] [] (bus_set_iommu) from [] > (mtk_iommu_probe+) > [6.733484] r7:c32db0b8 r6:c21f9c00 r5:c331f1c0 r4: > [6.739145] [] (mtk_iommu_probe) from [] > (platform_probe) > [6.747176] r10:c21f9c10 r9:c2496f54 r8:c14623b8 r7:c14623b8 r6:c1405b90 > r50 > [6.755012] r4: > [6.757544] [] (platform_probe) from [] > (really_probe.pa) > [6.766006] r7:c14623b8 r6:c1405b90 r5: r4:c21f9c10 > [6.771667] [] (really_probe.part.0) from [] > (really_pro) > [6.779866] r7:c21f9c10 r6:c2549e74 r5:c1405b90 r4:c21f9c10 > [6.785527] [] (really_probe) from [] > (__driver_probe_de) > [6.793984] r5:c1405b90 r4:c21f9c10 > [6.797560] [] (__driver_probe_device) from [] > (driver_p) > [6.806543] r9:c2496f54 r8:0008 r7:c21f9c10 r6:c2549e74 r5:c14c6ec8 > r4:4 > [6.814291] [] (driver_probe_device) from [] > (__device_a) > [6.823448] r9:c2496f54 r8: r7:c21f9c10 r6:c2549e74 r5:c1405b90 > r4:1 > [6.831196] [] (__device_attach_driver) from [] > (bus_for) > [6.840007] r7:c14623b8 r6:c073635c r5:c2549e74 r4: > [6.845669] [] (bus_for_each_drv) from [] > (__device_atta) > [6.854044] r6:0001 r5:c21f9c54 r4:c21f9c10 > [6.858662] [] (__device_attach) from [] > (device_initial) > [6.867207] r6:c21f9c10 r5:c1406f58 r4:c1406ca0 > [6.871825] [] (device_initial_probe) from [] > (bus_probe) > [6.880454] [] (bus_probe_device) from [] > (deferred_prob) > > > bisect shows this commit as breaking: > > Author: Yong Wu > Date: Wed Jul 14 10:56:17 2021 +0800 > > iommu/mediatek: Add probe_defer for smi-larb > > Prepare for adding device_link. > > regards Frank ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 00/11] Clean up "mediatek,larb"
On Wed, 2021-07-14 at 10:56 +0200, Dafna Hirschfeld wrote: > Hi > > Thanks for the patchset. > > I tested it on mt8173 (elm) with chromeos userspace. > Before that patchset, the test: > > tast -verbose run -build=false 10.42.0.175 video.DecodeAccel.h264 > > sometimes passed and sometimes failed with 'context deadline exceeded'. > With this patchset it seems that the test always passes so I added tested-by: > > Tested-by: Dafna Hirschfeld Thanks very much for your quick review and testing:) > > Thanks, > Dafna > > > > > On 14.07.21 04:56, Yong Wu wrote: > > MediaTek IOMMU block diagram always like below: > > > > M4U > > | > > smi-common > > | > >- > >| | ... > >| | > > larb1 larb2 > >| | > > vdec venc > > > > All the consumer connect with smi-larb, then connect with smi-common. > > > > When the consumer works, it should enable the smi-larb's power which also > > need enable the smi-common's power firstly. > > > > Thus, Firstly, use the device link connect the consumer and the > > smi-larbs. then add device link between the smi-larb and smi-common. > > > > After adding the device_link, then "mediatek,larb" property can be removed. > > the iommu consumer don't need call the mtk_smi_larb_get/put to enable > > the power and clock of smi-larb and smi-common. > > > > About the MM dt-binding/dtsi patches, I guess they should go together, thus > > I don't split them for each a MM module and each a SoC. > > > > Base on v5.14-rc1, and a jpeg[1] and mdp[2] patchset. > > > > [1] > > https://lore.kernel.org/linux-mediatek/20210702102304.3346429-1-hsi...@chromium.org/ > > [2] > > https://lore.kernel.org/linux-mediatek/20210709022324.1607884-1-ei...@chromium.org/ > > > > Change notes: > > v6: 1) rebase on v5.14-rc1. > > 2) Fix the issue commented in v5 from Dafna and Hsin-Yi. > > 3) Remove the patches about using pm_runtime_resume_and_get since they > > have > > already been merged by other patches. > > > > v5: > > https://lore.kernel.org/linux-mediatek/20210410091128.31823-1-yong...@mediatek.com/ > > 1) Base v5.12-rc2. > > 2) Remove changing the mtk-iommu to module_platform_driver patch, It > > have already been a > > independent patch. > > > > v4: > > https://lore.kernel.org/linux-mediatek/1590826218-23653-1-git-send-email-yong...@mediatek.com/ > > base on v5.7-rc1. > >1) Move drm PM patch before smi patchs. > >2) Change builtin_platform_driver to module_platform_driver since we may > > need > > build as module. > >3) Rebase many patchset as above. > > > > v3: > > https://lore.kernel.org/linux-iommu/1567503456-24725-1-git-send-email-yong...@mediatek.com/ > > 1) rebase on v5.3-rc1 and the latest mt8183 patchset. > > 2) Use device_is_bound to check whether the driver is ready from > > Matthias. > > 3) Add DL_FLAG_STATELESS flag when calling device_link_add and explain > > the > > reason in the commit message[3/14]. > > 4) Add a display patch[12/14] into this series. otherwise it may affect > > display HW fastlogo even though it don't happen in mt8183. > > > > v2: > > https://lore.kernel.org/linux-iommu/1560171313-28299-1-git-send-email-yong...@mediatek.com/ > > 1) rebase on v5.2-rc1. > > 2) Move adding device_link between the consumer and smi-larb into > > iommu_add_device from Robin. > > 3) add DL_FLAG_AUTOREMOVE_CONSUMER even though the smi is built-in from > > Evan. > > 4) Remove the shutdown callback in iommu. > > > > v1: > > https://lore.kernel.org/linux-iommu/1546318276-18993-1-git-send-email-yong...@mediatek.com/ > > > > Yong Wu (10): > >dt-binding: mediatek: Get rid of mediatek,larb for multimedia HW > >iommu/mediatek: Add probe_defer for smi-larb > >iommu/mediatek: Add device_link between the consumer and the larb > > devices > >media: mtk-jpeg: Get rid of mtk_smi_larb_get/put > >media: mtk-mdp: Get rid of mtk_smi_larb_get/put > >drm/mediatek: Get rid of mtk_smi_larb_get/put > >media: mtk-vcodec: Get rid of mtk_smi_larb_get/put > >memory: mtk-smi: Get rid of mtk_smi_larb_get/put > >arm: dts: mediatek: Get rid of mediatek,larb for MM nodes > >arm64: dts: mediatek: Get rid of mediatek,larb for MM nodes > > > > Yongqiang Niu (1): > >drm/mediatek: Add pm runtime support for ovl and rdma > > > > .../display/mediatek/mediatek,disp.txt| 9 > > .../bindings/media/mediatek-jpeg-decoder.yaml | 9 > > .../bindings/media/mediatek-jpeg-encoder.yaml | 9 > > .../bindings/media/mediatek-mdp.txt | 8 > > .../bindings/media/mediatek-vcodec.txt| 4 -- > > arch/arm/boot/dts/mt2701.dtsi | 2 - > > arch/arm/boot/dts/mt7623n.dtsi| 5 -- > > arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16 --- > > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 6
Re: [PATCH v1] iommu/tegra-smmu: Change debugfs directory name
14.07.2021 13:52, Joerg Roedel пишет: > On Mon, Jul 12, 2021 at 03:13:41AM +0300, Dmitry Osipenko wrote: >> -err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev)); >> +err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, "smmu"); > > Are you sure no one is relying on the old name so that this change > breaks ABI? IIUC, Thierry and Jon have a testing suite that uses the old name, but it shouldn't be a problem to support the new name in addition to the old name since it's internal/private testing suite. The reason I'm proposing this change is because it's not obvious where the SMMU debug is located when you look at the debugfs directory, it takes some effort to find it. > Also this change means that there is always be only one SMMU > per system. Is that guaranteed too? A single SMMU is guaranteed here. Only latest Tegra SoCs have two SMMUs and those are ARM SMMUs, while this is a legacy Tegra SMMU here. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 06/11] drm/mediatek: Add pm runtime support for ovl and rdma
On Wed, 2021-07-14 at 10:44 +0200, Dafna Hirschfeld wrote: > > On 14.07.21 04:56, Yong Wu wrote: > > From: Yongqiang Niu > > > > Prepare for smi cleaning up "mediatek,larb". > > > > Display use the dispsys device to call pm_rumtime_get_sync before. > > This patch add pm_runtime_xx with ovl and rdma device whose nodes has > > "iommus" property, then display could help pm_runtime_get for smi via > > ovl or rdma device. > > > > CC: CK Hu > > Signed-off-by: Yongqiang Niu > > Signed-off-by: Yong Wu > > (Yong: Use pm_runtime_resume_and_get instead of pm_runtime_get_sync) > > Acked-by: Chun-Kuang Hu > > --- > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 9 - > > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 9 - > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 12 +++- > > 3 files changed, 27 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > index fa9d79963cd3..ea5760f856ec 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > @@ -11,6 +11,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #include "mtk_disp_drv.h" > > @@ -414,15 +415,21 @@ static int mtk_disp_ovl_probe(struct platform_device > > *pdev) > > return ret; > > } > > > > + pm_runtime_enable(dev); > > + > > ret = component_add(dev, &mtk_disp_ovl_component_ops); > > - if (ret) > > + if (ret) { > > + pm_runtime_disable(dev); > > dev_err(dev, "Failed to add component: %d\n", ret); > > + } > > > > return ret; > > } > > > > static int mtk_disp_ovl_remove(struct platform_device *pdev) > > { > > + pm_runtime_disable(&pdev->dev); > > + > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > index 705f28ceb4dd..0f31d1c8e37c 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > @@ -9,6 +9,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #include "mtk_disp_drv.h" > > @@ -327,9 +328,13 @@ static int mtk_disp_rdma_probe(struct platform_device > > *pdev) > > > > platform_set_drvdata(pdev, priv); > > > > + pm_runtime_enable(dev); > > + > > ret = component_add(dev, &mtk_disp_rdma_component_ops); > > - if (ret) > > + if (ret) { > > + pm_runtime_disable(dev); > > dev_err(dev, "Failed to add component: %d\n", ret); > > + } > > > > return ret; > > } > > @@ -338,6 +343,8 @@ static int mtk_disp_rdma_remove(struct platform_device > > *pdev) > > { > > component_del(&pdev->dev, &mtk_disp_rdma_component_ops); > > > > + pm_runtime_disable(&pdev->dev); > > + > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > index 474efb844249..08e3f352377d 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > @@ -557,9 +557,15 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc > > *crtc, > > return; > > } > > > > + ret = pm_runtime_resume_and_get(comp->dev); > > + if (ret < 0) > > + DRM_DEV_ERROR(comp->dev, "Failed to enable power domain: %d\n", > > + ret); > > shouldn't the code return in case of failure here? After confirm with yongqiang, We will fix this in next version. Thanks. > > Thanks, > Dafna > > > + > > ret = mtk_crtc_ddp_hw_init(mtk_crtc); > > if (ret) { > > mtk_smi_larb_put(comp->larb_dev); > > + pm_runtime_put(comp->dev); > > return; > > } > > > > @@ -572,7 +578,7 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc > > *crtc, > > { > > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > > struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0]; > > - int i; > > + int i, ret; > > > > DRM_DEBUG_DRIVER("%s %d\n", __func__, crtc->base.id); > > if (!mtk_crtc->enabled) > > @@ -596,6 +602,10 @@ static void mtk_drm_crtc_atomic_disable(struct > > drm_crtc *crtc, > > drm_crtc_vblank_off(crtc); > > mtk_crtc_ddp_hw_fini(mtk_crtc); > > mtk_smi_larb_put(comp->larb_dev); > > + ret = pm_runtime_put(comp->dev); > > + if (ret < 0) > > + DRM_DEV_ERROR(comp->dev, "Failed to disable power domain: %d\n", > > + ret); > > > > mtk_crtc->enabled = false; > > } > > > > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 03/11] iommu/mediatek: Add device_link between the consumer and the larb devices
On Wed, 2021-07-14 at 10:26 +0200, Dafna Hirschfeld wrote: > > On 14.07.21 04:56, Yong Wu wrote: > > MediaTek IOMMU-SMI diagram is like below. 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. > > > > When device_link_add, I add the flag DL_FLAG_STATELESS to avoid calling > > pm_runtime_xx to keep the original status of clocks. It can avoid two > > issues: > > 1) Display HW show fastlogo abnormally reported in [1]. At the beggining, > > all the clocks are enabled before entering kernel, but the clocks for > > display HW(always in larb0) will be gated after clk_enable and clk_disable > > called from device_link_add(->pm_runtime_resume) and rpm_idle. The clock > > operation happened before display driver probe. At that time, the display > > HW will be abnormal. > > > > 2) A deadlock issue reported in [2]. Use DL_FLAG_STATELESS to skip > > pm_runtime_xx to avoid the deadlock. > > > > Corresponding, DL_FLAG_AUTOREMOVE_CONSUMER can't be added, then > > device_link_removed should be added explicitly. > > > > [1] > > https://lore.kernel.org/linux-mediatek/1564213888.22908.4.camel@mhfsdcap03/ > > [2] https://lore.kernel.org/patchwork/patch/1086569/ > > > > Suggested-by: Tomasz Figa > > Signed-off-by: Yong Wu > > --- > > drivers/iommu/mtk_iommu.c| 22 ++ > > drivers/iommu/mtk_iommu_v1.c | 20 +++- > > 2 files changed, 41 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > index a02dde094788..ee742900cf4b 100644 > > --- a/drivers/iommu/mtk_iommu.c > > +++ b/drivers/iommu/mtk_iommu.c > > @@ -571,22 +571,44 @@ static struct iommu_device > > *mtk_iommu_probe_device(struct device *dev) > > { > > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > > struct mtk_iommu_data *data; > > + struct device_link *link; > > + struct device *larbdev; > > + unsigned int larbid; > > > > if (!fwspec || fwspec->ops != &mtk_iommu_ops) > > return ERR_PTR(-ENODEV); /* Not a iommu client device */ > > > > data = dev_iommu_priv_get(dev); > > > > + /* > > +* Link the consumer device with the smi-larb device(supplier) > > +* The device in each a larb is a independent HW. thus only link > > +* one larb here. > > +*/ > > + larbid = MTK_M4U_TO_LARB(fwspec->ids[0]); > > + larbdev = data->larb_imu[larbid].dev; > > + link = device_link_add(dev, larbdev, > > + DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS); > > + if (!link) > > + dev_err(dev, "Unable to link %s\n", dev_name(larbdev)); > shoudn't ERR_PTR be returned in case of failure? In the previous design, this is not a fatal error. the consumer device could probe continuously even though it fail here..Returning here may let the issue be caught earlier, I will add this in next version. if (!link) { ... return ERR_PTR(EINVAL); } > > Thanks, > Dafna > > > return &data->iommu; > > } > > > > static void mtk_iommu_release_device(struct device *dev) > > { > > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > > + struct mtk_iommu_data *data; > > + struct device *larbdev; > > + unsigned int larbid; > > > > if (!fwspec || fwspec->ops != &mtk_iommu_ops) > > return; > > > > + data = dev_iommu_priv_get(dev); > > + larbid = MTK_M4U_TO_LARB(fwspec->ids[0]); > > + larbdev = data->larb_imu[larbid].dev; > > + device_link_remove(dev, larbdev); > > + > > iommu_fwspec_free(dev); > > } > > > > diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c > > index d9365a3d8dc9..d2a7c66b8239 100644 > > --- a/drivers/iommu/mtk_iommu_v1.c > > +++ b/drivers/iommu/mtk_iommu_v1.c > > @@ -424,7 +424,9 @@ static struct iommu_device > > *mtk_iommu_probe_device(struct device *dev) > > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > > struct of_phandle_args iommu_spec; > > struct mtk_iommu_data *data; > > - int err, idx = 0; > > + int err, idx = 0, larbid; > > + struct device_link *link; > > + struct device *larbdev; > > > > while (!of_parse_phandle_with_args(dev->of_node, "iommus", > >"#iommu-cells", > > @@ -445,6 +447,14 @@ static struct iommu_device > > *mtk_iommu_probe_device(struct device *dev) > > > > data = dev_iommu_priv_get(dev); > > > > + /* Link the
Re: [PATCH v1] iommu/tegra-smmu: Change debugfs directory name
On Mon, Jul 12, 2021 at 03:13:41AM +0300, Dmitry Osipenko wrote: > - err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, dev_name(dev)); > + err = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, "smmu"); Are you sure no one is relying on the old name so that this change breaks ABI? Also this change means that there is always be only one SMMU per system. Is that guaranteed too? Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 0/3] iommu: Enable non-strict DMA on QCom SD/MMC
On Wed, Jul 14, 2021 at 11:29:08AM +0100, Robin Murphy wrote: > As mentioned yesterday, already done! I'm hoping to be able to post the > patches next week after some testing :) Great, looking forward to your patches :-) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 6/6] dma-iommu: Pass iova len for IOVA domain init
Pass the max opt iova len to init the IOVA domain, if set. Signed-off-by: John Garry --- drivers/iommu/dma-iommu.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index b540b586fe37..eee9f5f87935 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -335,6 +335,8 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, struct iommu_dma_cookie *cookie = domain->iova_cookie; unsigned long order, base_pfn; struct iova_domain *iovad; + size_t max_opt_dma_size; + unsigned long iova_len = 0; if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE) return -EINVAL; @@ -368,7 +370,16 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, return 0; } - init_iova_domain(iovad, 1UL << order, base_pfn, 0); + + max_opt_dma_size = iommu_group_get_max_opt_dma_size(dev->iommu_group); + if (max_opt_dma_size) { + unsigned long shift = __ffs(1UL << order); + + iova_len = max_opt_dma_size >> shift; + iova_len = roundup_pow_of_two(iova_len); + } + + init_iova_domain(iovad, 1UL << order, base_pfn, iova_len); if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) { -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 5/6] iova: Add iova_len argument to init_iova_domain()
Add max opt argument to init_iova_domain(), and use it to set the rcaches range. Also fix up all users to set this value (at 0, meaning use default). Signed-off-by: John Garry --- drivers/gpu/drm/tegra/drm.c | 2 +- drivers/gpu/host1x/dev.c | 2 +- drivers/iommu/dma-iommu.c| 2 +- drivers/iommu/iova.c | 18 +- drivers/staging/media/ipu3/ipu3-dmamap.c | 2 +- drivers/staging/media/tegra-vde/iommu.c | 2 +- drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +- include/linux/iova.h | 5 +++-- 8 files changed, 22 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index f96c237b2242..c5fb2396ac81 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -1164,7 +1164,7 @@ static int host1x_drm_probe(struct host1x_device *dev) order = __ffs(tegra->domain->pgsize_bitmap); init_iova_domain(&tegra->carveout.domain, 1UL << order, -carveout_start >> order); +carveout_start >> order, 0); tegra->carveout.shift = iova_shift(&tegra->carveout.domain); tegra->carveout.limit = carveout_end >> tegra->carveout.shift; diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index fbb6447b8659..3cd02ffbd50e 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -278,7 +278,7 @@ static struct iommu_domain *host1x_iommu_attach(struct host1x *host) end = geometry->aperture_end & host->info->dma_mask; order = __ffs(host->domain->pgsize_bitmap); - init_iova_domain(&host->iova, 1UL << order, start >> order); + init_iova_domain(&host->iova, 1UL << order, start >> order, 0); host->iova_end = end; domain = host->domain; diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 4772278aa5da..b540b586fe37 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -368,7 +368,7 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, return 0; } - init_iova_domain(iovad, 1UL << order, base_pfn); + init_iova_domain(iovad, 1UL << order, base_pfn, 0); if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) { diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c index 07ce73fdd8c1..0c26aeada1ac 100644 --- a/drivers/iommu/iova.c +++ b/drivers/iommu/iova.c @@ -23,7 +23,7 @@ static bool iova_rcache_insert(struct iova_domain *iovad, static unsigned long iova_rcache_get(struct iova_domain *iovad, unsigned long size, unsigned long limit_pfn); -static void init_iova_rcaches(struct iova_domain *iovad); +static void init_iova_rcaches(struct iova_domain *iovad, unsigned long iova_len); static void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain *iovad); static void free_iova_rcaches(struct iova_domain *iovad); static void fq_destroy_all_entries(struct iova_domain *iovad); @@ -48,7 +48,7 @@ static struct iova *to_iova(struct rb_node *node) void init_iova_domain(struct iova_domain *iovad, unsigned long granule, - unsigned long start_pfn) + unsigned long start_pfn, unsigned long iova_len) { /* * IOVA granularity will normally be equal to the smallest @@ -71,7 +71,7 @@ init_iova_domain(struct iova_domain *iovad, unsigned long granule, rb_link_node(&iovad->anchor.node, NULL, &iovad->rbroot.rb_node); rb_insert_color(&iovad->anchor.node, &iovad->rbroot); cpuhp_state_add_instance_nocalls(CPUHP_IOMMU_IOVA_DEAD, &iovad->cpuhp_dead); - init_iova_rcaches(iovad); + init_iova_rcaches(iovad, iova_len); } EXPORT_SYMBOL_GPL(init_iova_domain); @@ -876,14 +876,22 @@ static void iova_magazine_push(struct iova_magazine *mag, unsigned long pfn) mag->pfns[mag->size++] = pfn; } -static void init_iova_rcaches(struct iova_domain *iovad) +static unsigned long iova_len_to_rcache_max(unsigned long iova_len) +{ + return order_base_2(iova_len) + 1; +} + +static void init_iova_rcaches(struct iova_domain *iovad, unsigned long iova_len) { struct iova_cpu_rcache *cpu_rcache; struct iova_rcache *rcache; unsigned int cpu; int i; - iovad->rcache_max_size = IOVA_RANGE_CACHE_MAX_SIZE; + if (iova_len) + iovad->rcache_max_size = iova_len_to_rcache_max(iova_len); + else + iovad->rcache_max_size = IOVA_RANGE_CACHE_MAX_SIZE; iovad->rcaches = kcalloc(iovad->rcache_max_size, sizeof(*iovad->rcaches), GFP_KERNEL); diff --git a/drivers/staging/media/ipu3/ipu3-dmamap.c b/drivers/staging/me
[PATCH v4 4/6] iommu: Allow max opt DMA len be set for a group via sysfs
Add support to allow the maximum optimised DMA len be set for an IOMMU group via sysfs. This much the same with the method to change the default domain type for a group. Signed-off-by: John Garry --- .../ABI/testing/sysfs-kernel-iommu_groups | 16 ++ drivers/iommu/iommu.c | 51 ++- include/linux/iommu.h | 6 +++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups index eae2f1c1e11e..c5a15b768dcc 100644 --- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups +++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups @@ -59,3 +59,19 @@ Description: /sys/kernel/iommu_groups//type shows the type of default system could lead to catastrophic effects (the users might need to reboot the machine to get it to normal state). So, it's expected that the users understand what they're doing. + +What: /sys/kernel/iommu_groups//max_opt_dma_size +Date: July 2021 +KernelVersion: v5.15 +Contact: John Garry +Description: /sys/kernel/iommu_groups//max_opt_dma_size shows the + max optimised DMA size for the default IOMMU domain associated + with the group. + Each IOMMU domain has an IOVA domain. The IOVA domain caches + IOVAs upto a certain size as a performance optimisation. + This sysfs file allows the range of the IOVA domain caching be + set, such that larger than default IOVAs may be cached. + A value of 0 means that the default caching range is chosen. + A privileged user could request the kernel the change the range + by writing to this file. For this to happen, the same rules + and procedure applies as in changing the default domain type. diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index d8198a9aff4e..38ec1c56e00b 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -45,6 +45,7 @@ struct iommu_group { struct iommu_domain *default_domain; struct iommu_domain *domain; struct list_head entry; + size_t max_opt_dma_size; }; struct group_device { @@ -86,6 +87,9 @@ static int iommu_create_device_direct_mappings(struct iommu_group *group, static struct iommu_group *iommu_group_get_for_dev(struct device *dev); static ssize_t iommu_group_store_type(struct iommu_group *group, const char *buf, size_t count); +static ssize_t iommu_group_store_max_opt_dma_size(struct iommu_group *group, + const char *buf, + size_t count); #define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) \ struct iommu_group_attribute iommu_group_attr_##_name =\ @@ -554,6 +558,12 @@ static ssize_t iommu_group_show_type(struct iommu_group *group, return strlen(type); } +static ssize_t iommu_group_show_max_opt_dma_size(struct iommu_group *group, +char *buf) +{ + return sprintf(buf, "%zu\n", group->max_opt_dma_size); +} + static IOMMU_GROUP_ATTR(name, S_IRUGO, iommu_group_show_name, NULL); static IOMMU_GROUP_ATTR(reserved_regions, 0444, @@ -562,6 +572,9 @@ static IOMMU_GROUP_ATTR(reserved_regions, 0444, static IOMMU_GROUP_ATTR(type, 0644, iommu_group_show_type, iommu_group_store_type); +static IOMMU_GROUP_ATTR(max_opt_dma_size, 0644, iommu_group_show_max_opt_dma_size, + iommu_group_store_max_opt_dma_size); + static void iommu_group_release(struct kobject *kobj) { struct iommu_group *group = to_iommu_group(kobj); @@ -648,6 +661,10 @@ struct iommu_group *iommu_group_alloc(void) if (ret) return ERR_PTR(ret); + ret = iommu_group_create_file(group, &iommu_group_attr_max_opt_dma_size); + if (ret) + return ERR_PTR(ret); + pr_debug("Allocated group %d\n", group->id); return group; @@ -2279,6 +2296,11 @@ struct iommu_domain *iommu_get_dma_domain(struct device *dev) return dev->iommu_group->default_domain; } +size_t iommu_group_get_max_opt_dma_size(struct iommu_group *group) +{ + return group->max_opt_dma_size; +} + /* * IOMMU groups are really the natural working unit of the IOMMU, but * the IOMMU API works on domains and devices. Bridge that gap by @@ -3045,12 +3067,14 @@ EXPORT_SYMBOL_GPL(iommu_sva_get_pasid); * hasn't changed after the caller has called this function) * @type: The type of the new default domain that gets associated with the group * @new: Allocate new default domain, keeping same type when no type passed + * @max_opt_dma_size: If set, set the IOMMU group max_opt_dma_size when success * * Returns 0 on suc
[PATCH v4 3/6] iommu: Allow iommu_change_dev_def_domain() realloc default domain for same type
Allow iommu_change_dev_def_domain() to create a new default domain, keeping the same as current when type is unset. Also remove comment about function purpose, which will become stale. Signed-off-by: John Garry --- drivers/iommu/iommu.c | 54 ++- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 8a815ac261f0..d8198a9aff4e 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -3036,6 +3036,7 @@ u32 iommu_sva_get_pasid(struct iommu_sva *handle) } EXPORT_SYMBOL_GPL(iommu_sva_get_pasid); + /* * Changes the default domain of an iommu group that has *only* one device * @@ -3043,16 +3044,13 @@ EXPORT_SYMBOL_GPL(iommu_sva_get_pasid); * @prev_dev: The device in the group (this is used to make sure that the device * hasn't changed after the caller has called this function) * @type: The type of the new default domain that gets associated with the group + * @new: Allocate new default domain, keeping same type when no type passed * * Returns 0 on success and error code on failure * - * Note: - * 1. Presently, this function is called only when user requests to change the - *group's default domain type through /sys/kernel/iommu_groups//type - *Please take a closer look if intended to use for other purposes. */ static int iommu_change_dev_def_domain(struct iommu_group *group, - struct device *prev_dev, int type) + struct device *prev_dev, int type, bool new) { struct iommu_domain *prev_dom; struct group_device *grp_dev; @@ -3102,28 +3100,32 @@ static int iommu_change_dev_def_domain(struct iommu_group *group, goto out; } - dev_def_dom = iommu_get_def_domain_type(dev); - if (!type) { + if (new && !type) { + type = prev_dom->type; + } else { + dev_def_dom = iommu_get_def_domain_type(dev); + if (!type) { + /* +* If the user hasn't requested any specific type of domain and +* if the device supports both the domains, then default to the +* domain the device was booted with +*/ + type = dev_def_dom ? : iommu_def_domain_type; + } else if (dev_def_dom && type != dev_def_dom) { + dev_err_ratelimited(prev_dev, "Device cannot be in %s domain\n", + iommu_domain_type_str(type)); + ret = -EINVAL; + goto out; + } + /* -* If the user hasn't requested any specific type of domain and -* if the device supports both the domains, then default to the -* domain the device was booted with +* Switch to a new domain only if the requested domain type is different +* from the existing default domain type */ - type = dev_def_dom ? : iommu_def_domain_type; - } else if (dev_def_dom && type != dev_def_dom) { - dev_err_ratelimited(prev_dev, "Device cannot be in %s domain\n", - iommu_domain_type_str(type)); - ret = -EINVAL; - goto out; - } - - /* -* Switch to a new domain only if the requested domain type is different -* from the existing default domain type -*/ - if (prev_dom->type == type) { - ret = 0; - goto out; + if (prev_dom->type == type) { + ret = 0; + goto out; + } } /* Sets group->default_domain to the newly allocated domain */ @@ -3267,7 +3269,7 @@ static int iommu_group_store_type_cb(const char *buf, else return -EINVAL; - return iommu_change_dev_def_domain(group, dev, type); + return iommu_change_dev_def_domain(group, dev, type, false); } static ssize_t iommu_group_store_type(struct iommu_group *group, -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 2/6] iova: Allow rcache range upper limit to be flexible
Some LLDs may request DMA mappings whose IOVA length exceeds that of the current rcache upper limit. This means that allocations for those IOVAs will never be cached, and always must be allocated and freed from the RB tree per DMA mapping cycle. This has a significant effect on performance, more so since commit 4e89dce72521 ("iommu/iova: Retry from last rb tree node if iova search fails"), as discussed at [0]. As a first step towards allowing the rcache range upper limit be configured, hold this value in the IOVA rcache structure, and allocate the rcaches separately. [0] https://lore.kernel.org/linux-iommu/20210129092120.1482-1-thunder.leiz...@huawei.com/ Signed-off-by: John Garry --- drivers/iommu/dma-iommu.c | 2 +- drivers/iommu/iova.c | 23 +-- include/linux/iova.h | 4 ++-- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 98ba927aee1a..4772278aa5da 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -434,7 +434,7 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain, * rounding up anything cacheable to make sure that can't happen. The * order of the unadjusted size will still match upon freeing. */ - if (iova_len < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1))) + if (iova_len < (1 << (iovad->rcache_max_size - 1))) iova_len = roundup_pow_of_two(iova_len); dma_limit = min_not_zero(dma_limit, dev->bus_dma_limit); diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c index b6cf5f16123b..07ce73fdd8c1 100644 --- a/drivers/iommu/iova.c +++ b/drivers/iommu/iova.c @@ -15,6 +15,8 @@ /* The anchor node sits above the top of the usable address space */ #define IOVA_ANCHOR~0UL +#define IOVA_RANGE_CACHE_MAX_SIZE 6/* log of max cached IOVA range size (in pages) */ + static bool iova_rcache_insert(struct iova_domain *iovad, unsigned long pfn, unsigned long size); @@ -881,7 +883,14 @@ static void init_iova_rcaches(struct iova_domain *iovad) unsigned int cpu; int i; - for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) { + iovad->rcache_max_size = IOVA_RANGE_CACHE_MAX_SIZE; + + iovad->rcaches = kcalloc(iovad->rcache_max_size, +sizeof(*iovad->rcaches), GFP_KERNEL); + if (!iovad->rcaches) + return; + + for (i = 0; i < iovad->rcache_max_size; ++i) { rcache = &iovad->rcaches[i]; spin_lock_init(&rcache->lock); rcache->depot_size = 0; @@ -956,7 +965,7 @@ static bool iova_rcache_insert(struct iova_domain *iovad, unsigned long pfn, { unsigned int log_size = order_base_2(size); - if (log_size >= IOVA_RANGE_CACHE_MAX_SIZE) + if (log_size >= iovad->rcache_max_size) return false; return __iova_rcache_insert(iovad, &iovad->rcaches[log_size], pfn); @@ -1012,7 +1021,7 @@ static unsigned long iova_rcache_get(struct iova_domain *iovad, { unsigned int log_size = order_base_2(size); - if (log_size >= IOVA_RANGE_CACHE_MAX_SIZE) + if (log_size >= iovad->rcache_max_size) return 0; return __iova_rcache_get(&iovad->rcaches[log_size], limit_pfn - size); @@ -1028,7 +1037,7 @@ static void free_iova_rcaches(struct iova_domain *iovad) unsigned int cpu; int i, j; - for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) { + for (i = 0; i < iovad->rcache_max_size; ++i) { rcache = &iovad->rcaches[i]; for_each_possible_cpu(cpu) { cpu_rcache = per_cpu_ptr(rcache->cpu_rcaches, cpu); @@ -1039,6 +1048,8 @@ static void free_iova_rcaches(struct iova_domain *iovad) for (j = 0; j < rcache->depot_size; ++j) iova_magazine_free(rcache->depot[j]); } + + kfree(iovad->rcaches); } /* @@ -1051,7 +1062,7 @@ static void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain *iovad) unsigned long flags; int i; - for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) { + for (i = 0; i < iovad->rcache_max_size; ++i) { rcache = &iovad->rcaches[i]; cpu_rcache = per_cpu_ptr(rcache->cpu_rcaches, cpu); spin_lock_irqsave(&cpu_rcache->lock, flags); @@ -1070,7 +1081,7 @@ static void free_global_cached_iovas(struct iova_domain *iovad) unsigned long flags; int i, j; - for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) { + for (i = 0; i < iovad->rcache_max_size; ++i) { rcache = &iovad->rcaches[i]; spin_lock_irqsave(&rcache->lock, flags); for (j = 0; j < rcache->depot_size; ++j) { diff --git a/include/linux/iova.h b/include/linux/iova.h index 71d8a2de6635..9974e1d3e2bc 100644 --- a/i
[PATCH v4 1/6] iommu: Refactor iommu_group_store_type()
Function iommu_group_store_type() supports changing the default domain of an IOMMU group. Many conditions need to be satisfied and steps taken for this action to be successful. Satisfying these conditions and steps will be required for setting other IOMMU group attributes, so factor into a common part and a part specific to update the IOMMU group attribute. No functional change intended. Some code comments are tidied up also. Signed-off-by: John Garry --- drivers/iommu/iommu.c | 73 +++ 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 5419c4b9f27a..8a815ac261f0 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -3166,20 +3166,23 @@ static int iommu_change_dev_def_domain(struct iommu_group *group, } /* - * Changing the default domain through sysfs requires the users to ubind the - * drivers from the devices in the iommu group. Return failure if this doesn't - * meet. + * Changing the default domain or any other IOMMU group attribute through sysfs + * requires the users to unbind the drivers from the devices in the IOMMU group. + * Return failure if this precondition is not met. * * We need to consider the race between this and the device release path. * device_lock(dev) is used here to guarantee that the device release path * will not be entered at the same time. */ -static ssize_t iommu_group_store_type(struct iommu_group *group, - const char *buf, size_t count) +static ssize_t iommu_group_store_common(struct iommu_group *group, + const char *buf, size_t count, + int (*cb)(const char *buf, + struct iommu_group *group, + struct device *dev)) { struct group_device *grp_dev; struct device *dev; - int ret, req_type; + int ret; if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO)) return -EACCES; @@ -3187,25 +3190,16 @@ static ssize_t iommu_group_store_type(struct iommu_group *group, if (WARN_ON(!group)) return -EINVAL; - if (sysfs_streq(buf, "identity")) - req_type = IOMMU_DOMAIN_IDENTITY; - else if (sysfs_streq(buf, "DMA")) - req_type = IOMMU_DOMAIN_DMA; - else if (sysfs_streq(buf, "auto")) - req_type = 0; - else - return -EINVAL; - /* * Lock/Unlock the group mutex here before device lock to -* 1. Make sure that the iommu group has only one device (this is a +* 1. Make sure that the IOMMU group has only one device (this is a *prerequisite for step 2) * 2. Get struct *dev which is needed to lock device */ mutex_lock(&group->mutex); if (iommu_group_device_count(group) != 1) { mutex_unlock(&group->mutex); - pr_err_ratelimited("Cannot change default domain: Group has more than one device\n"); + pr_err_ratelimited("Cannot change IOMMU group default domain attribute: Group has more than one device\n"); return -EINVAL; } @@ -3217,16 +3211,16 @@ static ssize_t iommu_group_store_type(struct iommu_group *group, /* * Don't hold the group mutex because taking group mutex first and then * the device lock could potentially cause a deadlock as below. Assume -* two threads T1 and T2. T1 is trying to change default domain of an -* iommu group and T2 is trying to hot unplug a device or release [1] VF -* of a PCIe device which is in the same iommu group. T1 takes group -* mutex and before it could take device lock assume T2 has taken device -* lock and is yet to take group mutex. Now, both the threads will be -* waiting for the other thread to release lock. Below, lock order was -* suggested. +* two threads, T1 and T2. T1 is trying to change default domain +* attribute of an IOMMU group and T2 is trying to hot unplug a device +* or release [1] VF of a PCIe device which is in the same IOMMU group. +* T1 takes the group mutex and before it could take device lock T2 may +* have taken device lock and is yet to take group mutex. Now, both the +* threads will be waiting for the other thread to release lock. Below, +* lock order was suggested. * device_lock(dev); * mutex_lock(&group->mutex); -* iommu_change_dev_def_domain(); +* cb->iommu_change_dev_def_domain(); [example cb] * mutex_unlock(&group->mutex); * device_unlock(dev); * @@ -3240,7 +3234,7 @@ static ssize_t iommu_group_store_type(struct iommu_group *group, */ mutex_unlock
[PATCH v4 0/6] iommu: Allow IOVA rcache range be configured
For streaming DMA mappings involving an IOMMU and whose IOVA len regularly exceeds the IOVA rcache upper limit (meaning that they are not cached), performance can be reduced. This may be much more pronounced from commit 4e89dce72521 ("iommu/iova: Retry from last rb tree node if iova search fails"), as discussed at [0]. IOVAs which cannot be cached are highly involved in the IOVA ageing issue, as discussed at [1]. This series allows the IOVA rcache range be configured, so that we may cache all IOVAs per domain, thus improving performance. A new IOMMU group sysfs file is added - max_opt_dma_size - which is used indirectly to configure the IOVA rcache range: /sys/kernel/iommu_groups/X/max_opt_dma_size This file is updated same as how the IOMMU group default domain type is updated, i.e. must unbind the only device in the group first. The inspiration here comes from block layer request queue sysfs "optimal_io_size" file, in /sys/block/sdX/queue/optimal_io_size Some figures for storage scenario (when increasing IOVA rcache range to cover all DMA mapping sizes from the LLD): v5.13-rc1 baseline: 1200K IOPS With series:1800K IOPS All above are for IOMMU strict mode. Non-strict mode gives ~1800K IOPS in all scenarios. [0] https://lore.kernel.org/linux-iommu/20210129092120.1482-1-thunder.leiz...@huawei.com/ [1] https://lore.kernel.org/linux-iommu/1607538189-237944-1-git-send-email-john.ga...@huawei.com/ Note that I cc'ed maintainers/reviewers only for the changes associated with patch #5 since it just touches their code in only a minor way. John Garry (6): iommu: Refactor iommu_group_store_type() iova: Allow rcache range upper limit to be flexible iommu: Allow iommu_change_dev_def_domain() realloc default domain for same type iommu: Allow max opt DMA len be set for a group via sysfs iova: Add iova_len argument to init_iova_domain() dma-iommu: Pass iova len for IOVA domain init .../ABI/testing/sysfs-kernel-iommu_groups | 16 ++ drivers/gpu/drm/tegra/drm.c | 2 +- drivers/gpu/host1x/dev.c | 2 +- drivers/iommu/dma-iommu.c | 15 +- drivers/iommu/iommu.c | 172 -- drivers/iommu/iova.c | 39 +++- drivers/staging/media/ipu3/ipu3-dmamap.c | 2 +- drivers/staging/media/tegra-vde/iommu.c | 2 +- drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +- include/linux/iommu.h | 6 + include/linux/iova.h | 9 +- 11 files changed, 194 insertions(+), 73 deletions(-) -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3] iommu/rockchip: Fix physical address decoding
On Mon, Jul 12, 2021 at 12:12:32PM +0200, Benjamin Gaignard wrote: > Restore bits 39 to 32 at correct position. > It reverses the operation done in rk_dma_addr_dte_v2(). > > Fixes: c55356c534aa ("iommu: rockchip: Add support for iommu v2") > > Reported-by: Dan Carpenter > Signed-off-by: Benjamin Gaignard > --- > version 3: > - change commit header to match with IOMMU tree convention Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/vt-d: Fix clearing real DMA device's scalable-mode context entries
On Mon, Jul 12, 2021 at 03:17:12PM +0800, Lu Baolu wrote: > drivers/iommu/intel/iommu.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/vt-d: Global devTLB flush when present context entry changed
On Mon, Jul 12, 2021 at 03:13:15PM +0800, Lu Baolu wrote: > --- > drivers/iommu/intel/iommu.c | 31 ++- > 1 file changed, 22 insertions(+), 9 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 0/3] iommu: Enable non-strict DMA on QCom SD/MMC
On 2021-07-14 11:15, Joerg Roedel wrote: Hi Robin, On Fri, Jul 09, 2021 at 02:56:47PM +0100, Robin Murphy wrote: As I mentioned before, conceptually I think this very much belongs in sysfs as a user decision. We essentially have 4 levels of "strictness": 1: DMA domain with bounce pages 2: DMA domain 3: DMA domain with flush queue 4: Identity domain Together with reasonable defaults (influenced by compile-time options) it seems to be a good thing to configure at runtime via sysfs. We already have CONFIG_IOMMU_DEFAULT_PASSTHROUGH, which can probably be extended to be an option list: - CONFIG_IOMMU_DEFAULT_PASSTHROUGH: Trusted devices are identity mapped - CONFIG_IOMMU_DEFAULT_DMA_STRICT: Trusted devices are DMA mapped with strict flush behavior on unmap - CONFIG_IOMMU_DEFAULT_DMA_LAZY: Trusted devices are DMA mapped with flush queues for performance Indeed, I got focused on the sysfs angle, but rearranging the Kconfig default that way to match makes a lot of sense, and is another thing which should fall out really easily from my domain type rework, so I'll add that to my branch now before I forget again. Untrusted devices always get into the DMA domain with bounce pages by default. The defaults can be changed at runtime via sysfs. We already have basic support for runtime switching of the default domain, so that can be re-used. As mentioned yesterday, already done! I'm hoping to be able to post the patches next week after some testing :) Cheers, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Tue, Jul 13, 2021 at 07:57:40PM -0400, Konrad Rzeszutek Wilk wrote: > The SWIOTLB does have support to do late initialization (xen-pcifront > does that for example - so if you add devices that can't do 64-bit it > will allocate something like 4MB). That sounds like a way to evaluate. I suggest to allocate the SWIOTLB memory at boot and when the IOMMUs are initialized we re-evaluate what we ended up with and free the SWIOTLB memory if its not needed. If that turns out to be wrong during runtime (e.g. because a device is switched to a passthrough default domain at runtime), we allocate a small aperture for this device like the above mentioned 4MB. (A boot option to always keep the aperture around might also be helpful for some setups) Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 0/3] iommu: Enable non-strict DMA on QCom SD/MMC
Hi Robin, On Fri, Jul 09, 2021 at 02:56:47PM +0100, Robin Murphy wrote: > As I mentioned before, conceptually I think this very much belongs in sysfs > as a user decision. We essentially have 4 levels of "strictness": > > 1: DMA domain with bounce pages > 2: DMA domain > 3: DMA domain with flush queue > 4: Identity domain Together with reasonable defaults (influenced by compile-time options) it seems to be a good thing to configure at runtime via sysfs. We already have CONFIG_IOMMU_DEFAULT_PASSTHROUGH, which can probably be extended to be an option list: - CONFIG_IOMMU_DEFAULT_PASSTHROUGH: Trusted devices are identity mapped - CONFIG_IOMMU_DEFAULT_DMA_STRICT: Trusted devices are DMA mapped with strict flush behavior on unmap - CONFIG_IOMMU_DEFAULT_DMA_LAZY: Trusted devices are DMA mapped with flush queues for performance Untrusted devices always get into the DMA domain with bounce pages by default. The defaults can be changed at runtime via sysfs. We already have basic support for runtime switching of the default domain, so that can be re-used. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 13/17] vdpa: factor out vhost_vdpa_pa_map() and vhost_vdpa_pa_unmap()
On Wed, Jul 14, 2021 at 05:41:54PM +0800, Jason Wang wrote: > > 在 2021/7/14 下午4:05, Dan Carpenter 写道: > > On Wed, Jul 14, 2021 at 10:14:32AM +0800, Jason Wang wrote: > > > 在 2021/7/13 下午7:31, Dan Carpenter 写道: > > > > On Tue, Jul 13, 2021 at 04:46:52PM +0800, Xie Yongji wrote: > > > > > @@ -613,37 +618,28 @@ static void vhost_vdpa_unmap(struct vhost_vdpa > > > > > *v, u64 iova, u64 size) > > > > > } > > > > >} > > > > > -static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v, > > > > > -struct vhost_iotlb_msg *msg) > > > > > +static int vhost_vdpa_pa_map(struct vhost_vdpa *v, > > > > > + u64 iova, u64 size, u64 uaddr, u32 perm) > > > > >{ > > > > > struct vhost_dev *dev = &v->vdev; > > > > > - struct vhost_iotlb *iotlb = dev->iotlb; > > > > > struct page **page_list; > > > > > unsigned long list_size = PAGE_SIZE / sizeof(struct page *); > > > > > unsigned int gup_flags = FOLL_LONGTERM; > > > > > unsigned long npages, cur_base, map_pfn, last_pfn = 0; > > > > > unsigned long lock_limit, sz2pin, nchunks, i; > > > > > - u64 iova = msg->iova; > > > > > + u64 start = iova; > > > > > long pinned; > > > > > int ret = 0; > > > > > - if (msg->iova < v->range.first || > > > > > - msg->iova + msg->size - 1 > v->range.last) > > > > > - return -EINVAL; > > > > This is not related to your patch, but can the "msg->iova + msg->size" > > > > addition can have an integer overflow. From looking at the callers it > > > > seems like it can. msg comes from: > > > > vhost_chr_write_iter() > > > > --> dev->msg_handler(dev, &msg); > > > > --> vhost_vdpa_process_iotlb_msg() > > > >--> vhost_vdpa_process_iotlb_update() > > > > > > Yes. > > > > > > > > > > If I'm thinking of the right thing then these are allowed to overflow to > > > > 0 because of the " - 1" but not further than that. I believe the check > > > > needs to be something like: > > > > > > > > if (msg->iova < v->range.first || > > > > msg->iova - 1 > U64_MAX - msg->size || > > > > > > I guess we don't need - 1 here? > > The - 1 is important. The highest address is 0x. So it goes > > start + size = 0 and then start + size - 1 == 0x. > > > Right, so actually > > msg->iova = 0xfffe, msg->size=2 is valid. I believe so, yes. It's inclusive of 0xfffe and 0x. (Not an expert). regards, dan carpenter ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 13/17] vdpa: factor out vhost_vdpa_pa_map() and vhost_vdpa_pa_unmap()
在 2021/7/14 下午4:05, Dan Carpenter 写道: On Wed, Jul 14, 2021 at 10:14:32AM +0800, Jason Wang wrote: 在 2021/7/13 下午7:31, Dan Carpenter 写道: On Tue, Jul 13, 2021 at 04:46:52PM +0800, Xie Yongji wrote: @@ -613,37 +618,28 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v, u64 iova, u64 size) } } -static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v, - struct vhost_iotlb_msg *msg) +static int vhost_vdpa_pa_map(struct vhost_vdpa *v, +u64 iova, u64 size, u64 uaddr, u32 perm) { struct vhost_dev *dev = &v->vdev; - struct vhost_iotlb *iotlb = dev->iotlb; struct page **page_list; unsigned long list_size = PAGE_SIZE / sizeof(struct page *); unsigned int gup_flags = FOLL_LONGTERM; unsigned long npages, cur_base, map_pfn, last_pfn = 0; unsigned long lock_limit, sz2pin, nchunks, i; - u64 iova = msg->iova; + u64 start = iova; long pinned; int ret = 0; - if (msg->iova < v->range.first || - msg->iova + msg->size - 1 > v->range.last) - return -EINVAL; This is not related to your patch, but can the "msg->iova + msg->size" addition can have an integer overflow. From looking at the callers it seems like it can. msg comes from: vhost_chr_write_iter() --> dev->msg_handler(dev, &msg); --> vhost_vdpa_process_iotlb_msg() --> vhost_vdpa_process_iotlb_update() Yes. If I'm thinking of the right thing then these are allowed to overflow to 0 because of the " - 1" but not further than that. I believe the check needs to be something like: if (msg->iova < v->range.first || msg->iova - 1 > U64_MAX - msg->size || I guess we don't need - 1 here? The - 1 is important. The highest address is 0x. So it goes start + size = 0 and then start + size - 1 == 0x. Right, so actually msg->iova = 0xfffe, msg->size=2 is valid. Thanks I guess we could move the - 1 to the other side? msg->iova > U64_MAX - msg->size + 1 || regards, dan carpenter ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
在 2021/7/14 下午2:49, Yongji Xie 写道: On Wed, Jul 14, 2021 at 1:45 PM Jason Wang wrote: 在 2021/7/13 下午4:46, Xie Yongji 写道: This VDUSE driver enables implementing software-emulated vDPA devices in userspace. The vDPA device is created by ioctl(VDUSE_CREATE_DEV) on /dev/vduse/control. Then a char device interface (/dev/vduse/$NAME) is exported to userspace for device emulation. In order to make the device emulation more secure, the device's control path is handled in kernel. A message mechnism is introduced to forward some dataplane related control messages to userspace. And in the data path, the DMA buffer will be mapped into userspace address space through different ways depending on the vDPA bus to which the vDPA device is attached. In virtio-vdpa case, the MMU-based IOMMU driver is used to achieve that. And in vhost-vdpa case, the DMA buffer is reside in a userspace memory region which can be shared to the VDUSE userspace processs via transferring the shmfd. For more details on VDUSE design and usage, please see the follow-on Documentation commit. Signed-off-by: Xie Yongji --- Documentation/userspace-api/ioctl/ioctl-number.rst |1 + drivers/vdpa/Kconfig | 10 + drivers/vdpa/Makefile |1 + drivers/vdpa/vdpa_user/Makefile|5 + drivers/vdpa/vdpa_user/vduse_dev.c | 1502 include/uapi/linux/vduse.h | 221 +++ 6 files changed, 1740 insertions(+) create mode 100644 drivers/vdpa/vdpa_user/Makefile create mode 100644 drivers/vdpa/vdpa_user/vduse_dev.c create mode 100644 include/uapi/linux/vduse.h diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst index 1409e40e6345..293ca3aef358 100644 --- a/Documentation/userspace-api/ioctl/ioctl-number.rst +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst @@ -300,6 +300,7 @@ Code Seq#Include File Comments 'z' 10-4F drivers/s390/crypto/zcrypt_api.h conflict! '|' 00-7F linux/media.h 0x80 00-1F linux/fb.h +0x81 00-1F linux/vduse.h 0x89 00-06 arch/x86/include/asm/sockios.h 0x89 0B-DF linux/sockios.h 0x89 E0-EF linux/sockios.h SIOCPROTOPRIVATE range diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig index a503c1b2bfd9..6e23bce6433a 100644 --- a/drivers/vdpa/Kconfig +++ b/drivers/vdpa/Kconfig @@ -33,6 +33,16 @@ config VDPA_SIM_BLOCK vDPA block device simulator which terminates IO request in a memory buffer. +config VDPA_USER + tristate "VDUSE (vDPA Device in Userspace) support" + depends on EVENTFD && MMU && HAS_DMA + select DMA_OPS + select VHOST_IOTLB + select IOMMU_IOVA + help + With VDUSE it is possible to emulate a vDPA Device + in a userspace program. + config IFCVF tristate "Intel IFC VF vDPA driver" depends on PCI_MSI diff --git a/drivers/vdpa/Makefile b/drivers/vdpa/Makefile index 67fe7f3d6943..f02ebed33f19 100644 --- a/drivers/vdpa/Makefile +++ b/drivers/vdpa/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_VDPA) += vdpa.o obj-$(CONFIG_VDPA_SIM) += vdpa_sim/ +obj-$(CONFIG_VDPA_USER) += vdpa_user/ obj-$(CONFIG_IFCVF)+= ifcvf/ obj-$(CONFIG_MLX5_VDPA) += mlx5/ obj-$(CONFIG_VP_VDPA)+= virtio_pci/ diff --git a/drivers/vdpa/vdpa_user/Makefile b/drivers/vdpa/vdpa_user/Makefile new file mode 100644 index ..260e0b26af99 --- /dev/null +++ b/drivers/vdpa/vdpa_user/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 + +vduse-y := vduse_dev.o iova_domain.o + +obj-$(CONFIG_VDPA_USER) += vduse.o diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c new file mode 100644 index ..c994a4a4660c --- /dev/null +++ b/drivers/vdpa/vdpa_user/vduse_dev.c @@ -0,0 +1,1502 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * VDUSE: vDPA Device in Userspace + * + * Copyright (C) 2020-2021 Bytedance Inc. and/or its affiliates. All rights reserved. + * + * Author: Xie Yongji + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "iova_domain.h" + +#define DRV_AUTHOR "Yongji Xie " +#define DRV_DESC "vDPA Device in Userspace" +#define DRV_LICENSE "GPL v2" + +#define VDUSE_DEV_MAX (1U << MINORBITS) +#define VDUSE_MAX_BOUNCE_SIZE (64 * 1024 * 1024) +#define VDUSE_IOVA_SIZE (128 * 1024 * 1024) +#define VDUSE_REQUEST_TIMEOUT 30 + +struct vduse_virtqueue { + u16 index; + u16 num_max; + u32 num; + u64 desc_addr; + u64 driver_addr; + u64 device_addr; + struct vdpa_vq_state state; + bool ready; + bool kic
Aw: [PATCH v6 00/11] Clean up "mediatek,larb"
Hi, sorry this (or the 2 depency-series) cause a NULL Pointer deref in iommu_group_remove_device on mt7623/bpi-r2 i wonder why on bootup a cleanup is run, but have no hint about this. since "dts: mtk-mdp: remove mediatek, vpu property from primary MDP device" all is good, i guess problem comes up while removing larb with DT this is backtrace [6.274465] PC is at iommu_group_remove_device+0x28/0x148 [6.279877] LR is at iommu_release_device+0x4c/0x70 [6.674347] Backtrace: [6.676797] [] (iommu_group_remove_device) from [] (iomm) [6.686221] r7: r6:c06bf04c r5:c0d7a1ac r4:c21fc010 [6.691883] [] (iommu_release_device) from [] (remove_io) [6.700689] r5: r4: [6.704265] [] (remove_iommu_group) from [] (bus_for_eac) [6.712725] [] (bus_for_each_dev) from [] (bus_set_iommu) [6.720753] r6:c331f440 r5:c1406f58 r4:ffea [6.725370] [] (bus_set_iommu) from [] (mtk_iommu_probe+) [6.733484] r7:c32db0b8 r6:c21f9c00 r5:c331f1c0 r4: [6.739145] [] (mtk_iommu_probe) from [] (platform_probe) [6.747176] r10:c21f9c10 r9:c2496f54 r8:c14623b8 r7:c14623b8 r6:c1405b90 r50 [6.755012] r4: [6.757544] [] (platform_probe) from [] (really_probe.pa) [6.766006] r7:c14623b8 r6:c1405b90 r5: r4:c21f9c10 [6.771667] [] (really_probe.part.0) from [] (really_pro) [6.779866] r7:c21f9c10 r6:c2549e74 r5:c1405b90 r4:c21f9c10 [6.785527] [] (really_probe) from [] (__driver_probe_de) [6.793984] r5:c1405b90 r4:c21f9c10 [6.797560] [] (__driver_probe_device) from [] (driver_p) [6.806543] r9:c2496f54 r8:0008 r7:c21f9c10 r6:c2549e74 r5:c14c6ec8 r4:4 [6.814291] [] (driver_probe_device) from [] (__device_a) [6.823448] r9:c2496f54 r8: r7:c21f9c10 r6:c2549e74 r5:c1405b90 r4:1 [6.831196] [] (__device_attach_driver) from [] (bus_for) [6.840007] r7:c14623b8 r6:c073635c r5:c2549e74 r4: [6.845669] [] (bus_for_each_drv) from [] (__device_atta) [6.854044] r6:0001 r5:c21f9c54 r4:c21f9c10 [6.858662] [] (__device_attach) from [] (device_initial) [6.867207] r6:c21f9c10 r5:c1406f58 r4:c1406ca0 [6.871825] [] (device_initial_probe) from [] (bus_probe) [6.880454] [] (bus_probe_device) from [] (deferred_prob) bisect shows this commit as breaking: Author: Yong Wu Date: Wed Jul 14 10:56:17 2021 +0800 iommu/mediatek: Add probe_defer for smi-larb Prepare for adding device_link. regards Frank ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
在 2021/7/14 下午2:47, Greg KH 写道: On Wed, Jul 14, 2021 at 02:02:50PM +0800, Jason Wang wrote: 在 2021/7/14 下午1:54, Michael S. Tsirkin 写道: On Wed, Jul 14, 2021 at 01:45:39PM +0800, Jason Wang wrote: +static int vduse_dev_msg_sync(struct vduse_dev *dev, + struct vduse_dev_msg *msg) +{ + int ret; + + init_waitqueue_head(&msg->waitq); + spin_lock(&dev->msg_lock); + msg->req.request_id = dev->msg_unique++; + vduse_enqueue_msg(&dev->send_list, msg); + wake_up(&dev->waitq); + spin_unlock(&dev->msg_lock); + + wait_event_killable_timeout(msg->waitq, msg->completed, + VDUSE_REQUEST_TIMEOUT * HZ); + spin_lock(&dev->msg_lock); + if (!msg->completed) { + list_del(&msg->list); + msg->resp.result = VDUSE_REQ_RESULT_FAILED; + } + ret = (msg->resp.result == VDUSE_REQ_RESULT_OK) ? 0 : -EIO; I think we should mark the device as malfunction when there is a timeout and forbid any userspace operations except for the destroy aftwards for safety. This looks like if one tried to run gdb on the program the behaviour will change completely because kernel wants it to respond within specific time. Looks like a receipe for heisenbugs. Let's not build interfaces with arbitrary timeouts like that. Interruptible wait exists for this very reason. The problem is. Do we want userspace program like modprobe to be stuck for indefinite time and expect the administrator to kill that? Why would modprobe be stuck for forever? Is this on the module probe path? Yes, it is called in the device probing path where the kernel forwards the device configuration request to userspace and wait for its response. If it turns out to be tricky, we can implement the whole device inside the kernel and leave only the datapath in the userspace (as what TUN did). Thanks ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 00/11] Clean up "mediatek,larb"
Hi Thanks for the patchset. I tested it on mt8173 (elm) with chromeos userspace. Before that patchset, the test: tast -verbose run -build=false 10.42.0.175 video.DecodeAccel.h264 sometimes passed and sometimes failed with 'context deadline exceeded'. With this patchset it seems that the test always passes so I added tested-by: Tested-by: Dafna Hirschfeld Thanks, Dafna On 14.07.21 04:56, Yong Wu wrote: MediaTek IOMMU block diagram always like below: M4U | smi-common | - | | ... | | larb1 larb2 | | vdec venc All the consumer connect with smi-larb, then connect with smi-common. When the consumer works, it should enable the smi-larb's power which also need enable the smi-common's power firstly. Thus, Firstly, use the device link connect the consumer and the smi-larbs. then add device link between the smi-larb and smi-common. After adding the device_link, then "mediatek,larb" property can be removed. the iommu consumer don't need call the mtk_smi_larb_get/put to enable the power and clock of smi-larb and smi-common. About the MM dt-binding/dtsi patches, I guess they should go together, thus I don't split them for each a MM module and each a SoC. Base on v5.14-rc1, and a jpeg[1] and mdp[2] patchset. [1] https://lore.kernel.org/linux-mediatek/20210702102304.3346429-1-hsi...@chromium.org/ [2] https://lore.kernel.org/linux-mediatek/20210709022324.1607884-1-ei...@chromium.org/ Change notes: v6: 1) rebase on v5.14-rc1. 2) Fix the issue commented in v5 from Dafna and Hsin-Yi. 3) Remove the patches about using pm_runtime_resume_and_get since they have already been merged by other patches. v5: https://lore.kernel.org/linux-mediatek/20210410091128.31823-1-yong...@mediatek.com/ 1) Base v5.12-rc2. 2) Remove changing the mtk-iommu to module_platform_driver patch, It have already been a independent patch. v4: https://lore.kernel.org/linux-mediatek/1590826218-23653-1-git-send-email-yong...@mediatek.com/ base on v5.7-rc1. 1) Move drm PM patch before smi patchs. 2) Change builtin_platform_driver to module_platform_driver since we may need build as module. 3) Rebase many patchset as above. v3: https://lore.kernel.org/linux-iommu/1567503456-24725-1-git-send-email-yong...@mediatek.com/ 1) rebase on v5.3-rc1 and the latest mt8183 patchset. 2) Use device_is_bound to check whether the driver is ready from Matthias. 3) Add DL_FLAG_STATELESS flag when calling device_link_add and explain the reason in the commit message[3/14]. 4) Add a display patch[12/14] into this series. otherwise it may affect display HW fastlogo even though it don't happen in mt8183. v2: https://lore.kernel.org/linux-iommu/1560171313-28299-1-git-send-email-yong...@mediatek.com/ 1) rebase on v5.2-rc1. 2) Move adding device_link between the consumer and smi-larb into iommu_add_device from Robin. 3) add DL_FLAG_AUTOREMOVE_CONSUMER even though the smi is built-in from Evan. 4) Remove the shutdown callback in iommu. v1: https://lore.kernel.org/linux-iommu/1546318276-18993-1-git-send-email-yong...@mediatek.com/ Yong Wu (10): dt-binding: mediatek: Get rid of mediatek,larb for multimedia HW iommu/mediatek: Add probe_defer for smi-larb iommu/mediatek: Add device_link between the consumer and the larb devices media: mtk-jpeg: Get rid of mtk_smi_larb_get/put media: mtk-mdp: Get rid of mtk_smi_larb_get/put drm/mediatek: Get rid of mtk_smi_larb_get/put media: mtk-vcodec: Get rid of mtk_smi_larb_get/put memory: mtk-smi: Get rid of mtk_smi_larb_get/put arm: dts: mediatek: Get rid of mediatek,larb for MM nodes arm64: dts: mediatek: Get rid of mediatek,larb for MM nodes Yongqiang Niu (1): drm/mediatek: Add pm runtime support for ovl and rdma .../display/mediatek/mediatek,disp.txt| 9 .../bindings/media/mediatek-jpeg-decoder.yaml | 9 .../bindings/media/mediatek-jpeg-encoder.yaml | 9 .../bindings/media/mediatek-mdp.txt | 8 .../bindings/media/mediatek-vcodec.txt| 4 -- arch/arm/boot/dts/mt2701.dtsi | 2 - arch/arm/boot/dts/mt7623n.dtsi| 5 -- arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16 --- arch/arm64/boot/dts/mediatek/mt8183.dtsi | 6 --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 9 +++- drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 9 +++- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 19 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 36 +-- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.c| 5 +- drivers/iommu/mtk_iommu.c | 24 +- drivers/iommu/mtk_iommu_v1.c | 22 - .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 45 +- .../media/platform/
Re: [PATCH v6 01/11] dt-binding: mediatek: Get rid of mediatek, larb for multimedia HW
On 14.07.21 10:13, Dafna Hirschfeld wrote: Hi, thanks for the patch On 14.07.21 04:56, 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: Rob Herring Reviewed-by: Evan Green --- .../bindings/display/mediatek/mediatek,disp.txt | 9 - .../devicetree/bindings/media/mediatek-jpeg-decoder.yaml | 9 - .../devicetree/bindings/media/mediatek-jpeg-encoder.yaml | 9 - On which repo are these patches based on ? In linux-next the file mediatek-jpeg-encoder.yaml don't exist Thanks, Dafna sorry, I see you reference the patch that convert to yaml in the cover letter. Thanks, Dafna Documentation/devicetree/bindings/media/mediatek-mdp.txt | 8 .../devicetree/bindings/media/mediatek-vcodec.txt | 4 5 files changed, 39 deletions(-) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt index fbb59c9ddda6..867bd82e2f03 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt @@ -61,8 +61,6 @@ Required properties (DMA function blocks): "mediatek,-disp-rdma" "mediatek,-disp-wdma" the supported chips are mt2701, mt8167 and mt8173. -- larb: Should contain a phandle pointing to the local arbiter device as defined - in Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml - iommus: Should point to the respective IOMMU block with master port as argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details. @@ -91,7 +89,6 @@ ovl0: ovl@1400c000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_OVL0>; iommus = <&iommu M4U_PORT_DISP_OVL0>; - mediatek,larb = <&larb0>; }; ovl1: ovl@1400d000 { @@ -101,7 +98,6 @@ ovl1: ovl@1400d000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_OVL1>; iommus = <&iommu M4U_PORT_DISP_OVL1>; - mediatek,larb = <&larb4>; }; rdma0: rdma@1400e000 { @@ -111,7 +107,6 @@ rdma0: rdma@1400e000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_RDMA0>; iommus = <&iommu M4U_PORT_DISP_RDMA0>; - mediatek,larb = <&larb0>; mediatek,rdma-fifosize = <8192>; }; @@ -122,7 +117,6 @@ rdma1: rdma@1400f000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_RDMA1>; iommus = <&iommu M4U_PORT_DISP_RDMA1>; - mediatek,larb = <&larb4>; }; rdma2: rdma@1401 { @@ -132,7 +126,6 @@ rdma2: rdma@1401 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_RDMA2>; iommus = <&iommu M4U_PORT_DISP_RDMA2>; - mediatek,larb = <&larb4>; }; wdma0: wdma@14011000 { @@ -142,7 +135,6 @@ wdma0: wdma@14011000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_WDMA0>; iommus = <&iommu M4U_PORT_DISP_WDMA0>; - mediatek,larb = <&larb0>; }; wdma1: wdma@14012000 { @@ -152,7 +144,6 @@ wdma1: wdma@14012000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_WDMA1>; iommus = <&iommu M4U_PORT_DISP_WDMA1>; - mediatek,larb = <&larb4>; }; color0: color@14013000 { diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml index 9b87f036f178..052e752157b4 100644 --- a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml @@ -42,13 +42,6 @@ properties: power-domains: maxItems: 1 - mediatek,larb: - $ref: '/schemas/types.yaml#/definitions/phandle' - description: | - Must contain the local arbiters in the current Socs, see - Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml - for details. - iommus: maxItems: 2 description: | @@ -63,7 +56,6 @@ required: - clocks - clock-names - power-domains - - mediatek,larb - iommus additionalProperties: false @@ -83,7 +75,6 @@ examples: clock-names = "jpgdec-smi", "jpgdec"; power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>; - mediatek,larb = <&larb2>; iommus = <&iommu MT2701_M4U_PORT_JPGDEC_WDMA>, <&iommu MT2701_M4U_PORT_JPGDEC_BSDMA>; }; diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml b/Docum
Re: [PATCH v6 06/11] drm/mediatek: Add pm runtime support for ovl and rdma
On 14.07.21 04:56, Yong Wu wrote: From: Yongqiang Niu Prepare for smi cleaning up "mediatek,larb". Display use the dispsys device to call pm_rumtime_get_sync before. This patch add pm_runtime_xx with ovl and rdma device whose nodes has "iommus" property, then display could help pm_runtime_get for smi via ovl or rdma device. CC: CK Hu Signed-off-by: Yongqiang Niu Signed-off-by: Yong Wu (Yong: Use pm_runtime_resume_and_get instead of pm_runtime_get_sync) Acked-by: Chun-Kuang Hu --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 9 - drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 9 - drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 12 +++- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index fa9d79963cd3..ea5760f856ec 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include "mtk_disp_drv.h" @@ -414,15 +415,21 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev) return ret; } + pm_runtime_enable(dev); + ret = component_add(dev, &mtk_disp_ovl_component_ops); - if (ret) + if (ret) { + pm_runtime_disable(dev); dev_err(dev, "Failed to add component: %d\n", ret); + } return ret; } static int mtk_disp_ovl_remove(struct platform_device *pdev) { + pm_runtime_disable(&pdev->dev); + return 0; } diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c index 705f28ceb4dd..0f31d1c8e37c 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "mtk_disp_drv.h" @@ -327,9 +328,13 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev) platform_set_drvdata(pdev, priv); + pm_runtime_enable(dev); + ret = component_add(dev, &mtk_disp_rdma_component_ops); - if (ret) + if (ret) { + pm_runtime_disable(dev); dev_err(dev, "Failed to add component: %d\n", ret); + } return ret; } @@ -338,6 +343,8 @@ static int mtk_disp_rdma_remove(struct platform_device *pdev) { component_del(&pdev->dev, &mtk_disp_rdma_component_ops); + pm_runtime_disable(&pdev->dev); + return 0; } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 474efb844249..08e3f352377d 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -557,9 +557,15 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, return; } + ret = pm_runtime_resume_and_get(comp->dev); + if (ret < 0) + DRM_DEV_ERROR(comp->dev, "Failed to enable power domain: %d\n", + ret); shouldn't the code return in case of failure here? Thanks, Dafna + ret = mtk_crtc_ddp_hw_init(mtk_crtc); if (ret) { mtk_smi_larb_put(comp->larb_dev); + pm_runtime_put(comp->dev); return; } @@ -572,7 +578,7 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc *crtc, { struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0]; - int i; + int i, ret; DRM_DEBUG_DRIVER("%s %d\n", __func__, crtc->base.id); if (!mtk_crtc->enabled) @@ -596,6 +602,10 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc *crtc, drm_crtc_vblank_off(crtc); mtk_crtc_ddp_hw_fini(mtk_crtc); mtk_smi_larb_put(comp->larb_dev); + ret = pm_runtime_put(comp->dev); + if (ret < 0) + DRM_DEV_ERROR(comp->dev, "Failed to disable power domain: %d\n", + ret); mtk_crtc->enabled = false; } ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dt-bindings: More dropping redundant minItems/maxItems
Hi Rob, Thank you for the patch. On Tue, Jul 13, 2021 at 01:34:53PM -0600, Rob Herring wrote: > Another round of removing redundant minItems/maxItems from new schema in > the recent merge window. > > If a property has an 'items' list, then a 'minItems' or 'maxItems' with the > same size as the list is redundant and can be dropped. Note that is DT > schema specific behavior and not standard json-schema behavior. The tooling > will fixup the final schema adding any unspecified minItems/maxItems. > > This condition is partially checked with the meta-schema already, but > only if both 'minItems' and 'maxItems' are equal to the 'items' length. > An improved meta-schema is pending. > > Cc: Stephen Boyd > Cc: Joerg Roedel > Cc: Will Deacon > Cc: Krzysztof Kozlowski > Cc: Miquel Raynal > Cc: Richard Weinberger > Cc: Vignesh Raghavendra > Cc: Alessandro Zummo > Cc: Alexandre Belloni > Cc: Greg Kroah-Hartman > Cc: Sureshkumar Relli > Cc: Brian Norris > Cc: Kamal Dasu > Cc: Linus Walleij > Cc: Sebastian Siewior > Cc: Laurent Pinchart > Cc: linux-...@vger.kernel.org > Cc: iommu@lists.linux-foundation.org > Cc: linux-...@lists.infradead.org > Cc: linux-...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Signed-off-by: Rob Herring Reviewed-by: Laurent Pinchart > --- > .../devicetree/bindings/clock/brcm,iproc-clocks.yaml | 1 - > .../devicetree/bindings/iommu/rockchip,iommu.yaml | 2 -- > .../bindings/memory-controllers/arm,pl353-smc.yaml| 1 - > Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 > .../devicetree/bindings/rtc/faraday,ftrtc010.yaml | 1 - > Documentation/devicetree/bindings/usb/nxp,isp1760.yaml| 2 -- > 6 files changed, 15 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.yaml > b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.yaml > index 8dc7b404ee12..1174c9aa9934 100644 > --- a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.yaml > +++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.yaml > @@ -50,7 +50,6 @@ properties: > >reg: > minItems: 1 > -maxItems: 3 > items: >- description: base register >- description: power register > diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml > b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml > index d2e28a9e3545..ba9124f721f1 100644 > --- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml > +++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml > @@ -28,14 +28,12 @@ properties: >- description: configuration registers for MMU instance 0 >- description: configuration registers for MMU instance 1 > minItems: 1 > -maxItems: 2 > >interrupts: > items: >- description: interruption for MMU instance 0 >- description: interruption for MMU instance 1 > minItems: 1 > -maxItems: 2 > >clocks: > items: > diff --git > a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml > b/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml > index 7a63c85ef8c5..01c9acf9275d 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml > +++ b/Documentation/devicetree/bindings/memory-controllers/arm,pl353-smc.yaml > @@ -57,7 +57,6 @@ properties: > >ranges: > minItems: 1 > -maxItems: 3 > description: | >Memory bus areas for interacting with the devices. Reflects >the memory layout with four integer values following: > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > index e5f1a2a5..dd5a64969e37 100644 > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > @@ -84,7 +84,6 @@ properties: > >interrupts: > minItems: 1 > -maxItems: 3 > items: >- description: NAND CTLRDY interrupt >- description: FLASH_DMA_DONE if flash DMA is available > @@ -92,7 +91,6 @@ properties: > >interrupt-names: > minItems: 1 > -maxItems: 3 > items: >- const: nand_ctlrdy >- const: flash_dma_done > @@ -148,8 +146,6 @@ allOf: > then: >properties: > reg-names: > - minItems: 2 > - maxItems: 2 >items: > - const: nand > - const: nand-int-base > @@ -161,8 +157,6 @@ allOf: > then: >properties: > reg-names: > - minItems: 3 > - maxItems: 3 >items: > - const: nand > - const: nand-int-base > @@ -175,8 +169,6 @@ allOf: > then: >properties: > reg-names: > - minItems: 3 > - maxItems: 3 >items: > - const: nand > - const: iproc-idm > diff --git a/Documentation/devicetr
Re: [PATCH v6 09/11] memory: mtk-smi: Get rid of mtk_smi_larb_get/put
On 14.07.21 04:56, 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 Reviewed-by: Evan Green Acked-by: Krzysztof Kozlowski Acked-by: Matthias Brugger Reviewed-by: Dafna Hirschfeld --- drivers/memory/mtk-smi.c | 14 -- include/soc/mediatek/smi.h | 20 2 files changed, 34 deletions(-) diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c index c5fb51f73b34..7c61c924e220 100644 --- a/drivers/memory/mtk-smi.c +++ b/drivers/memory/mtk-smi.c @@ -134,20 +134,6 @@ static void mtk_smi_clk_disable(const struct mtk_smi *smi) clk_disable_unprepare(smi->clk_apb); } -int mtk_smi_larb_get(struct device *larbdev) -{ - int ret = pm_runtime_resume_and_get(larbdev); - - return (ret < 0) ? ret : 0; -} -EXPORT_SYMBOL_GPL(mtk_smi_larb_get); - -void mtk_smi_larb_put(struct device *larbdev) -{ - pm_runtime_put_sync(larbdev); -} -EXPORT_SYMBOL_GPL(mtk_smi_larb_put); - static int mtk_smi_larb_bind(struct device *dev, struct device *master, void *data) { diff --git a/include/soc/mediatek/smi.h b/include/soc/mediatek/smi.h index 15e3397cec58..11f7d6b59642 100644 --- a/include/soc/mediatek/smi.h +++ b/include/soc/mediatek/smi.h @@ -19,26 +19,6 @@ struct mtk_smi_larb_iommu { unsigned char bank[32]; }; -/* - * mtk_smi_larb_get: Enable the power domain and clocks for this local arbiter. - * It also initialize some basic setting(like iommu). - * mtk_smi_larb_put: Disable the power domain and clocks for this local arbiter. - * Both should be called in non-atomic context. - * - * Returns 0 if successful, negative on failure. - */ -int mtk_smi_larb_get(struct device *larbdev); -void mtk_smi_larb_put(struct device *larbdev); - -#else - -static inline int mtk_smi_larb_get(struct device *larbdev) -{ - return 0; -} - -static inline void mtk_smi_larb_put(struct device *larbdev) { } - #endif #endif ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 08/11] media: mtk-vcodec: Get rid of mtk_smi_larb_get/put
On 14.07.21 04:56, 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 CC: Irui Wang Signed-off-by: Yong Wu Reviewed-by: Evan Green Acked-by: Tiffany Lin Reviewed-by: Dafna Hirschfeld --- .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 37 +++- .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 -- .../platform/mtk-vcodec/mtk_vcodec_enc.c | 1 - .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 44 +++ 4 files changed, 10 insertions(+), 75 deletions(-) diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c index 6038db96f71c..d0bf9aa3b29d 100644 --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c @@ -8,14 +8,12 @@ #include #include #include -#include #include "mtk_vcodec_dec_pm.h" #include "mtk_vcodec_util.h" int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) { - struct device_node *node; struct platform_device *pdev; struct mtk_vcodec_pm *pm; struct mtk_vcodec_clk *dec_clk; @@ -26,18 +24,7 @@ int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) pm = &mtkdev->pm; pm->mtkdev = mtkdev; dec_clk = &pm->vdec_clk; - node = of_parse_phandle(pdev->dev.of_node, "mediatek,larb", 0); - if (!node) { - mtk_v4l2_err("of_parse_phandle mediatek,larb fail!"); - return -1; - } - pdev = of_find_device_by_node(node); - of_node_put(node); - if (WARN_ON(!pdev)) { - return -1; - } - pm->larbvdec = &pdev->dev; pdev = mtkdev->plat_dev; pm->dev = &pdev->dev; @@ -47,14 +34,11 @@ int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) dec_clk->clk_info = devm_kcalloc(&pdev->dev, dec_clk->clk_num, sizeof(*clk_info), GFP_KERNEL); - if (!dec_clk->clk_info) { - ret = -ENOMEM; - goto put_device; - } + if (!dec_clk->clk_info) + return -ENOMEM; } else { mtk_v4l2_err("Failed to get vdec clock count"); - ret = -EINVAL; - goto put_device; + return -EINVAL; } for (i = 0; i < dec_clk->clk_num; i++) { @@ -63,29 +47,24 @@ int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) "clock-names", i, &clk_info->clk_name); if (ret) { mtk_v4l2_err("Failed to get clock name id = %d", i); - goto put_device; + return ret; } clk_info->vcodec_clk = devm_clk_get(&pdev->dev, clk_info->clk_name); if (IS_ERR(clk_info->vcodec_clk)) { mtk_v4l2_err("devm_clk_get (%d)%s fail", i, clk_info->clk_name); - ret = PTR_ERR(clk_info->vcodec_clk); - goto put_device; + return PTR_ERR(clk_info->vcodec_clk); } } pm_runtime_enable(&pdev->dev); return 0; -put_device: - put_device(pm->larbvdec); - return ret; } void mtk_vcodec_release_dec_pm(struct mtk_vcodec_dev *dev) { pm_runtime_disable(dev->pm.dev); - put_device(dev->pm.larbvdec); } int mtk_vcodec_dec_pw_on(struct mtk_vcodec_pm *pm) @@ -122,11 +101,6 @@ void mtk_vcodec_dec_clock_on(struct mtk_vcodec_pm *pm) } } - ret = mtk_smi_larb_get(pm->larbvdec); - if (ret) { - mtk_v4l2_err("mtk_smi_larb_get larbvdec fail %d", ret); - goto error; - } return; error: @@ -139,7 +113,6 @@ void mtk_vcodec_dec_clock_off(struct mtk_vcodec_pm *pm) struct mtk_vcodec_clk *dec_clk = &pm->vdec_clk; int i = 0; - mtk_smi_larb_put(pm->larbvdec); for (i = dec_clk->clk_num - 1; i >= 0; i--) clk_disable_unprepare(dec_clk->clk_info[i].vcodec_clk); } diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h index c6c7672fecfb..64b73dd880ce 100644 --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h @@ -189,10 +189,7 @@ struct mtk_vcodec_clk { */ struct mtk_vcodec_pm { struct mtk_vcodec_clk vdec_clk; - struct device *larbvdec; - struct mtk_vcodec_clk venc_clk; - struct device *larbvenc; struct device *dev; struct mtk_vcodec_dev *mtkdev; }; diff --git a/drivers
Re: [PATCH v6 07/11] drm/mediatek: Get rid of mtk_smi_larb_get/put
On 14.07.21 04:56, 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 Reviewed-by: Evan Green Acked-by: Chun-Kuang Hu Reviewed-by: Dafna Hirschfeld --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 9 -- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 36 ++--- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +-- 4 files changed, 3 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 08e3f352377d..d046abcf66ce 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -10,7 +10,6 @@ #include #include -#include #include #include @@ -551,12 +550,6 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, DRM_DEBUG_DRIVER("%s %d\n", __func__, crtc->base.id); - ret = mtk_smi_larb_get(comp->larb_dev); - if (ret) { - DRM_ERROR("Failed to get larb: %d\n", ret); - return; - } - ret = pm_runtime_resume_and_get(comp->dev); if (ret < 0) DRM_DEV_ERROR(comp->dev, "Failed to enable power domain: %d\n", @@ -564,7 +557,6 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, ret = mtk_crtc_ddp_hw_init(mtk_crtc); if (ret) { - mtk_smi_larb_put(comp->larb_dev); pm_runtime_put(comp->dev); return; } @@ -601,7 +593,6 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc *crtc, drm_crtc_vblank_off(crtc); mtk_crtc_ddp_hw_fini(mtk_crtc); - mtk_smi_larb_put(comp->larb_dev); ret = pm_runtime_put(comp->dev); if (ret < 0) DRM_DEV_ERROR(comp->dev, "Failed to disable power domain: %d\n", diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c index 75bc00e17fc4..7d240218d4c7 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c @@ -449,37 +449,15 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm, return ret; } -static int mtk_ddp_get_larb_dev(struct device_node *node, struct mtk_ddp_comp *comp, - struct device *dev) -{ - struct device_node *larb_node; - struct platform_device *larb_pdev; - - larb_node = of_parse_phandle(node, "mediatek,larb", 0); - if (!larb_node) { - dev_err(dev, "Missing mediadek,larb phandle in %pOF node\n", node); - return -EINVAL; - } - - larb_pdev = of_find_device_by_node(larb_node); - if (!larb_pdev) { - dev_warn(dev, "Waiting for larb device %pOF\n", larb_node); - of_node_put(larb_node); - return -EPROBE_DEFER; - } - of_node_put(larb_node); - comp->larb_dev = &larb_pdev->dev; - - return 0; -} - int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp, enum mtk_ddp_comp_id comp_id) { struct platform_device *comp_pdev; enum mtk_ddp_comp_type type; struct mtk_ddp_comp_dev *priv; +#if IS_REACHABLE(CONFIG_MTK_CMDQ) int ret; +#endif if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX) return -EINVAL; @@ -495,16 +473,6 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp, } comp->dev = &comp_pdev->dev; - /* Only DMA capable components need the LARB property */ - if (type == MTK_DISP_OVL || - type == MTK_DISP_OVL_2L || - type == MTK_DISP_RDMA || - type == MTK_DISP_WDMA) { - ret = mtk_ddp_get_larb_dev(node, comp, comp->dev); - if (ret) - return ret; - } - if (type == MTK_DISP_BLS || type == MTK_DISP_CCORR || type == MTK_DISP_COLOR || diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h index bb914d976cf5..1b582262b682 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h @@ -70,7 +70,6 @@ struct mtk_ddp_comp_funcs { struct mtk_ddp_comp { struct device *dev; int irq; - struct device *larb_dev; enum mtk_ddp_comp_id id; const struct mtk_ddp_comp_funcs *funcs; }; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index b46bdb8985da..0d5ef3d8d081 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -577,11 +577,8 @@ static int mtk_drm_probe(struct platform_de
Re: [PATCH v6 05/11] media: mtk-mdp: Get rid of mtk_smi_larb_get/put
On 14.07.21 04:56, 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 CC: Houlong Wei Signed-off-by: Yong Wu Reviewed-by: Evan Green Reviewed-by: Houlong Wei Reviewed-by: Dafna Hirschfeld --- drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 46 +-- drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 2 - drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 1 - 3 files changed, 1 insertion(+), 48 deletions(-) diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c index de2d425efdd1..5e0ea83a9f7f 100644 --- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c @@ -13,7 +13,6 @@ #include #include #include -#include #include #include "mtk_mdp_comp.h" @@ -57,13 +56,6 @@ int mtk_mdp_comp_power_on(struct mtk_mdp_comp *comp) { int status, err; - if (comp->larb_dev) { - err = mtk_smi_larb_get(comp->larb_dev); - if (err) - dev_err(comp->dev, - "failed to get larb, err %d.\n", err); - } - err = pm_runtime_get_sync(comp->dev); if (err < 0) { dev_err(comp->dev, "failed to runtime get, err %d.\n", err); @@ -146,9 +138,6 @@ void mtk_mdp_comp_clock_off(struct mtk_mdp_comp *comp) continue; clk_disable_unprepare(comp->clk[i]); } - - if (comp->larb_dev) - mtk_smi_larb_put(comp->larb_dev); } /* @@ -236,9 +225,6 @@ static const struct component_ops mtk_mdp_component_ops = { int mtk_mdp_comp_init(struct mtk_mdp_comp *comp, struct device *dev) { - struct device_node *larb_node; - struct platform_device *larb_pdev; - int ret; int i; struct device_node *node = dev->of_node; enum mtk_mdp_comp_type comp_type = @@ -252,8 +238,7 @@ int mtk_mdp_comp_init(struct mtk_mdp_comp *comp, struct device *dev) if (IS_ERR(comp->clk[i])) { if (PTR_ERR(comp->clk[i]) != -EPROBE_DEFER) dev_err(dev, "Failed to get clock\n"); - ret = PTR_ERR(comp->clk[i]); - goto err; + return PTR_ERR(comp->clk[i]); } /* Only RDMA needs two clocks */ @@ -261,36 +246,7 @@ int mtk_mdp_comp_init(struct mtk_mdp_comp *comp, struct device *dev) break; } - /* Only DMA capable components need the LARB property */ - comp->larb_dev = NULL; - if (comp_type != MTK_MDP_RDMA && - comp_type != MTK_MDP_WDMA && - comp_type != MTK_MDP_WROT) - return 0; - - larb_node = of_parse_phandle(node, "mediatek,larb", 0); - if (!larb_node) { - dev_err(dev, - "Missing mediadek,larb phandle in %pOF node\n", node); - ret = -EINVAL; - goto err; - } - - larb_pdev = of_find_device_by_node(larb_node); - if (!larb_pdev) { - dev_warn(dev, "Waiting for larb device %pOF\n", larb_node); - of_node_put(larb_node); - ret = -EPROBE_DEFER; - goto err; - } - of_node_put(larb_node); - - comp->larb_dev = &larb_pdev->dev; - return 0; - -err: - return ret; } static int mtk_mdp_comp_probe(struct platform_device *pdev) diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h index 5201c47f7baa..2bd229cc7eae 100644 --- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h @@ -11,13 +11,11 @@ * struct mtk_mdp_comp - the MDP's function component data * @node: list node to track sibing MDP components * @clk: clocks required for component - * @larb_dev: SMI device required for component * @dev: component's device */ struct mtk_mdp_comp { struct list_headnode; struct clk *clk[2]; - struct device *larb_dev; struct device *dev; }; diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c index e1fb39231248..be7d35b3e3ff 100644 --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c @@ -18,7 +18,6 @@ #include #include #include -#include #include "mtk_mdp_comp.h" #include "mtk_mdp_core.h" ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 04/11] media: mtk-jpeg: Get rid of mtk_smi_larb_get/put
On 14.07.21 04:56, 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. After removing the larb_get operations, then mtk_jpeg_clk_init is also unnecessary. Remove it too. CC: Rick Chang CC: Xia Jiang Signed-off-by: Yong Wu Reviewed-by: Evan Green Acked-by: Rick Chang Reviewed-by: Dafna Hirschfeld --- .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 45 +-- .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 2 - 2 files changed, 2 insertions(+), 45 deletions(-) diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c index a89c7b206eef..4fea2c512434 100644 --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c @@ -22,7 +22,6 @@ #include #include #include -#include #include "mtk_jpeg_enc_hw.h" #include "mtk_jpeg_dec_hw.h" @@ -1055,10 +1054,6 @@ static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg) { int ret; - ret = mtk_smi_larb_get(jpeg->larb); - if (ret) - dev_err(jpeg->dev, "mtk_smi_larb_get larbvdec fail %d\n", ret); - ret = clk_bulk_prepare_enable(jpeg->variant->num_clks, jpeg->variant->clks); if (ret) @@ -1069,7 +1064,6 @@ static void mtk_jpeg_clk_off(struct mtk_jpeg_dev *jpeg) { clk_bulk_disable_unprepare(jpeg->variant->num_clks, jpeg->variant->clks); - mtk_smi_larb_put(jpeg->larb); } static irqreturn_t mtk_jpeg_enc_done(struct mtk_jpeg_dev *jpeg) @@ -1284,35 +1278,6 @@ static struct clk_bulk_data mtk_jpeg_clocks[] = { { .id = "jpgenc" }, }; -static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg) -{ - struct device_node *node; - struct platform_device *pdev; - int ret; - - node = of_parse_phandle(jpeg->dev->of_node, "mediatek,larb", 0); - if (!node) - return -EINVAL; - pdev = of_find_device_by_node(node); - if (WARN_ON(!pdev)) { - of_node_put(node); - return -EINVAL; - } - of_node_put(node); - - jpeg->larb = &pdev->dev; - - ret = devm_clk_bulk_get(jpeg->dev, jpeg->variant->num_clks, - jpeg->variant->clks); - if (ret) { - dev_err(&pdev->dev, "failed to get jpeg clock:%d\n", ret); - put_device(&pdev->dev); - return ret; - } - - return 0; -} - static void mtk_jpeg_job_timeout_work(struct work_struct *work) { struct mtk_jpeg_dev *jpeg = container_of(work, struct mtk_jpeg_dev, @@ -1333,11 +1298,6 @@ static void mtk_jpeg_job_timeout_work(struct work_struct *work) v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx); } -static inline void mtk_jpeg_clk_release(struct mtk_jpeg_dev *jpeg) -{ - put_device(jpeg->larb); -} - static int mtk_jpeg_probe(struct platform_device *pdev) { struct mtk_jpeg_dev *jpeg; @@ -1376,7 +1336,8 @@ static int mtk_jpeg_probe(struct platform_device *pdev) goto err_req_irq; } - ret = mtk_jpeg_clk_init(jpeg); + ret = devm_clk_bulk_get(jpeg->dev, jpeg->variant->num_clks, + jpeg->variant->clks); if (ret) { dev_err(&pdev->dev, "Failed to init clk, err %d\n", ret); goto err_clk_init; @@ -1442,7 +1403,6 @@ static int mtk_jpeg_probe(struct platform_device *pdev) v4l2_device_unregister(&jpeg->v4l2_dev); err_dev_register: - mtk_jpeg_clk_release(jpeg); err_clk_init: @@ -1460,7 +1420,6 @@ static int mtk_jpeg_remove(struct platform_device *pdev) video_device_release(jpeg->vdev); v4l2_m2m_release(jpeg->m2m_dev); v4l2_device_unregister(&jpeg->v4l2_dev); - mtk_jpeg_clk_release(jpeg); return 0; } diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h index 595f7f10c9fd..3e4811a41ba2 100644 --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h @@ -85,7 +85,6 @@ struct mtk_jpeg_variant { * @alloc_ctx:videobuf2 memory allocator's context * @vdev: video device node for jpeg mem2mem mode * @reg_base: JPEG registers mapping - * @larb: SMI device * @job_timeout_work: IRQ timeout structure * @variant: driver variant to be used */ @@ -99,7 +98,6 @@ struct mtk_jpeg_dev { void*alloc_ctx; struct video_device *vdev; void __iomem*reg_base; - struct device *larb; struct delayed_work job_timeout_work; const struct mtk_jpeg_variant *variant; }; _
Re: [PATCH v6 03/11] iommu/mediatek: Add device_link between the consumer and the larb devices
On 14.07.21 04:56, Yong Wu wrote: MediaTek IOMMU-SMI diagram is like below. 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. When device_link_add, I add the flag DL_FLAG_STATELESS to avoid calling pm_runtime_xx to keep the original status of clocks. It can avoid two issues: 1) Display HW show fastlogo abnormally reported in [1]. At the beggining, all the clocks are enabled before entering kernel, but the clocks for display HW(always in larb0) will be gated after clk_enable and clk_disable called from device_link_add(->pm_runtime_resume) and rpm_idle. The clock operation happened before display driver probe. At that time, the display HW will be abnormal. 2) A deadlock issue reported in [2]. Use DL_FLAG_STATELESS to skip pm_runtime_xx to avoid the deadlock. Corresponding, DL_FLAG_AUTOREMOVE_CONSUMER can't be added, then device_link_removed should be added explicitly. [1] https://lore.kernel.org/linux-mediatek/1564213888.22908.4.camel@mhfsdcap03/ [2] https://lore.kernel.org/patchwork/patch/1086569/ Suggested-by: Tomasz Figa Signed-off-by: Yong Wu --- drivers/iommu/mtk_iommu.c| 22 ++ drivers/iommu/mtk_iommu_v1.c | 20 +++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index a02dde094788..ee742900cf4b 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -571,22 +571,44 @@ static struct iommu_device *mtk_iommu_probe_device(struct device *dev) { struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); struct mtk_iommu_data *data; + struct device_link *link; + struct device *larbdev; + unsigned int larbid; if (!fwspec || fwspec->ops != &mtk_iommu_ops) return ERR_PTR(-ENODEV); /* Not a iommu client device */ data = dev_iommu_priv_get(dev); + /* +* Link the consumer device with the smi-larb device(supplier) +* The device in each a larb is a independent HW. thus only link +* one larb here. +*/ + larbid = MTK_M4U_TO_LARB(fwspec->ids[0]); + larbdev = data->larb_imu[larbid].dev; + link = device_link_add(dev, larbdev, + DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS); + if (!link) + dev_err(dev, "Unable to link %s\n", dev_name(larbdev)); shoudn't ERR_PTR be returned in case of failure? Thanks, Dafna return &data->iommu; } static void mtk_iommu_release_device(struct device *dev) { struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); + struct mtk_iommu_data *data; + struct device *larbdev; + unsigned int larbid; if (!fwspec || fwspec->ops != &mtk_iommu_ops) return; + data = dev_iommu_priv_get(dev); + larbid = MTK_M4U_TO_LARB(fwspec->ids[0]); + larbdev = data->larb_imu[larbid].dev; + device_link_remove(dev, larbdev); + iommu_fwspec_free(dev); } diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c index d9365a3d8dc9..d2a7c66b8239 100644 --- a/drivers/iommu/mtk_iommu_v1.c +++ b/drivers/iommu/mtk_iommu_v1.c @@ -424,7 +424,9 @@ static struct iommu_device *mtk_iommu_probe_device(struct device *dev) struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); struct of_phandle_args iommu_spec; struct mtk_iommu_data *data; - int err, idx = 0; + int err, idx = 0, larbid; + struct device_link *link; + struct device *larbdev; while (!of_parse_phandle_with_args(dev->of_node, "iommus", "#iommu-cells", @@ -445,6 +447,14 @@ static struct iommu_device *mtk_iommu_probe_device(struct device *dev) data = dev_iommu_priv_get(dev); + /* Link the consumer device with the smi-larb device(supplier) */ + larbid = mt2701_m4u_to_larb(fwspec->ids[0]); + larbdev = data->larb_imu[larbid].dev; + link = device_link_add(dev, larbdev, + DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS); + if (!link) + dev_err(dev, "Unable to link %s\n", dev_name(larbdev)); + return &data->iommu; } @@ -465,10 +475,18 @@ static void mtk_iommu_probe_finalize(struct device *dev) static void mtk_iommu_release_device(struct device *dev) { struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); + struct mtk_iommu_data *data; + struct device *larbdev; + un
Re: [PATCH v6 01/11] dt-binding: mediatek: Get rid of mediatek, larb for multimedia HW
Hi, thanks for the patch On 14.07.21 04:56, 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: Rob Herring Reviewed-by: Evan Green --- .../bindings/display/mediatek/mediatek,disp.txt | 9 - .../devicetree/bindings/media/mediatek-jpeg-decoder.yaml | 9 - .../devicetree/bindings/media/mediatek-jpeg-encoder.yaml | 9 - On which repo are these patches based on ? In linux-next the file mediatek-jpeg-encoder.yaml don't exist Thanks, Dafna Documentation/devicetree/bindings/media/mediatek-mdp.txt | 8 .../devicetree/bindings/media/mediatek-vcodec.txt| 4 5 files changed, 39 deletions(-) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt index fbb59c9ddda6..867bd82e2f03 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt @@ -61,8 +61,6 @@ Required properties (DMA function blocks): "mediatek,-disp-rdma" "mediatek,-disp-wdma" the supported chips are mt2701, mt8167 and mt8173. -- larb: Should contain a phandle pointing to the local arbiter device as defined - in Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml - iommus: Should point to the respective IOMMU block with master port as argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details. @@ -91,7 +89,6 @@ ovl0: ovl@1400c000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_OVL0>; iommus = <&iommu M4U_PORT_DISP_OVL0>; - mediatek,larb = <&larb0>; }; ovl1: ovl@1400d000 { @@ -101,7 +98,6 @@ ovl1: ovl@1400d000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_OVL1>; iommus = <&iommu M4U_PORT_DISP_OVL1>; - mediatek,larb = <&larb4>; }; rdma0: rdma@1400e000 { @@ -111,7 +107,6 @@ rdma0: rdma@1400e000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_RDMA0>; iommus = <&iommu M4U_PORT_DISP_RDMA0>; - mediatek,larb = <&larb0>; mediatek,rdma-fifosize = <8192>; }; @@ -122,7 +117,6 @@ rdma1: rdma@1400f000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_RDMA1>; iommus = <&iommu M4U_PORT_DISP_RDMA1>; - mediatek,larb = <&larb4>; }; rdma2: rdma@1401 { @@ -132,7 +126,6 @@ rdma2: rdma@1401 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_RDMA2>; iommus = <&iommu M4U_PORT_DISP_RDMA2>; - mediatek,larb = <&larb4>; }; wdma0: wdma@14011000 { @@ -142,7 +135,6 @@ wdma0: wdma@14011000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_WDMA0>; iommus = <&iommu M4U_PORT_DISP_WDMA0>; - mediatek,larb = <&larb0>; }; wdma1: wdma@14012000 { @@ -152,7 +144,6 @@ wdma1: wdma@14012000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_WDMA1>; iommus = <&iommu M4U_PORT_DISP_WDMA1>; - mediatek,larb = <&larb4>; }; color0: color@14013000 { diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml index 9b87f036f178..052e752157b4 100644 --- a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml @@ -42,13 +42,6 @@ properties: power-domains: maxItems: 1 - mediatek,larb: -$ref: '/schemas/types.yaml#/definitions/phandle' -description: | - Must contain the local arbiters in the current Socs, see - Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml - for details. - iommus: maxItems: 2 description: | @@ -63,7 +56,6 @@ required: - clocks - clock-names - power-domains - - mediatek,larb - iommus additionalProperties: false @@ -83,7 +75,6 @@ examples: clock-names = "jpgdec-smi", "jpgdec"; power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>; - mediatek,larb = <&larb2>; iommus = <&iommu MT2701_M4U_PORT_JPGDEC_WDMA>, <&iommu MT2701_M4U_PORT_JPGDEC_BSDMA>; }; diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml b/Documentation/devicetree/bindings/media/medi
Re: [PATCH v9 13/17] vdpa: factor out vhost_vdpa_pa_map() and vhost_vdpa_pa_unmap()
On Wed, Jul 14, 2021 at 10:14:32AM +0800, Jason Wang wrote: > > 在 2021/7/13 下午7:31, Dan Carpenter 写道: > > On Tue, Jul 13, 2021 at 04:46:52PM +0800, Xie Yongji wrote: > > > @@ -613,37 +618,28 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v, > > > u64 iova, u64 size) > > > } > > > } > > > -static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v, > > > -struct vhost_iotlb_msg *msg) > > > +static int vhost_vdpa_pa_map(struct vhost_vdpa *v, > > > + u64 iova, u64 size, u64 uaddr, u32 perm) > > > { > > > struct vhost_dev *dev = &v->vdev; > > > - struct vhost_iotlb *iotlb = dev->iotlb; > > > struct page **page_list; > > > unsigned long list_size = PAGE_SIZE / sizeof(struct page *); > > > unsigned int gup_flags = FOLL_LONGTERM; > > > unsigned long npages, cur_base, map_pfn, last_pfn = 0; > > > unsigned long lock_limit, sz2pin, nchunks, i; > > > - u64 iova = msg->iova; > > > + u64 start = iova; > > > long pinned; > > > int ret = 0; > > > - if (msg->iova < v->range.first || > > > - msg->iova + msg->size - 1 > v->range.last) > > > - return -EINVAL; > > This is not related to your patch, but can the "msg->iova + msg->size" > > addition can have an integer overflow. From looking at the callers it > > seems like it can. msg comes from: > >vhost_chr_write_iter() > >--> dev->msg_handler(dev, &msg); > >--> vhost_vdpa_process_iotlb_msg() > > --> vhost_vdpa_process_iotlb_update() > > > Yes. > > > > > > If I'm thinking of the right thing then these are allowed to overflow to > > 0 because of the " - 1" but not further than that. I believe the check > > needs to be something like: > > > > if (msg->iova < v->range.first || > > msg->iova - 1 > U64_MAX - msg->size || > > > I guess we don't need - 1 here? The - 1 is important. The highest address is 0x. So it goes start + size = 0 and then start + size - 1 == 0x. I guess we could move the - 1 to the other side? msg->iova > U64_MAX - msg->size + 1 || regards, dan carpenter ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu