Re: [PATCH v9 17/17] Documentation: Add documentation for VDUSE

2021-07-14 Thread Jason Wang


在 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?



  

Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace

2021-07-14 Thread Jason Wang


在 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

2021-07-14 Thread Ashish Mhetre via iommu




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

2021-07-14 Thread Yongji Xie
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

2021-07-14 Thread Tian, Kevin
> 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

2021-07-14 Thread Shenming Lu
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-07-14 Thread Jason Wang


在 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 = >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, );
 --> 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-07-14 Thread chenxiang (M)



在 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

2021-07-14 Thread Lu Baolu

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, _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

2021-07-14 Thread 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.

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

2021-07-14 Thread Rob Herring
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

2021-07-14 Thread Rob Herring
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)

2021-07-14 Thread Suravee Suthikulpanit via iommu
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

2021-07-14 Thread Arnd Bergmann
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

2021-07-14 Thread Robin Murphy

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

2021-07-14 Thread Sven Peter via iommu
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 >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] = _pgtable_arm_64_lpae_s1_init_fns,
> > 

[PATCH] iommu: Unify iova_to_phys for identity domains

2021-07-14 Thread Robin Murphy
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, _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

2021-07-14 Thread Doug Anderson
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

2021-07-14 Thread Georgi Djakov

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

2021-07-14 Thread Joe Perches
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

[PATCH v2 0/4] iommu/vt-d: Disable igfx iommu superpage on bxt/skl/glk

2021-07-14 Thread Ville Syrjala
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

2021-07-14 Thread Ville Syrjala
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

2021-07-14 Thread Ville Syrjala
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(_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(_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(_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

2021-07-14 Thread Ville Syrjala
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

2021-07-14 Thread Ville Syrjala
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

2021-07-14 Thread Alexandre Belloni
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 
> 

Re: [PATCH] dt-bindings: More dropping redundant minItems/maxItems

2021-07-14 Thread Greg Kroah-Hartman
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"

2021-07-14 Thread Frank Wunderlich
> 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"

2021-07-14 Thread Yong Wu
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"

2021-07-14 Thread Yong Wu
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

2021-07-14 Thread Dmitry Osipenko
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(>iommu, dev, NULL, dev_name(dev));
>> +err = iommu_device_sysfs_add(>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

2021-07-14 Thread Yong Wu
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, _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(>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, _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(>dev, _disp_rdma_component_ops);
> >   
> > +   pm_runtime_disable(>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

2021-07-14 Thread Yong Wu
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 != _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 >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 != _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 

Re: [PATCH v1] iommu/tegra-smmu: Change debugfs directory name

2021-07-14 Thread Joerg Roedel
On Mon, Jul 12, 2021 at 03:13:41AM +0300, Dmitry Osipenko wrote:
> - err = iommu_device_sysfs_add(>iommu, dev, NULL, dev_name(dev));
> + err = iommu_device_sysfs_add(>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

2021-07-14 Thread Joerg Roedel
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

2021-07-14 Thread John Garry
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()

2021-07-14 Thread John Garry
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(>carveout.domain, 1UL << order,
-carveout_start >> order);
+carveout_start >> order, 0);
 
tegra->carveout.shift = iova_shift(>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(>iova, 1UL << order, start >> order);
+   init_iova_domain(>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(>anchor.node, NULL, >rbroot.rb_node);
rb_insert_color(>anchor.node, >rbroot);
cpuhp_state_add_instance_nocalls(CPUHP_IOMMU_IOVA_DEAD, 
>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/media/ipu3/ipu3-dmamap.c
index 8a19b0024152..dad8789873e8 

[PATCH v4 4/6] iommu: Allow max opt DMA len be set for a group via sysfs

2021-07-14 Thread John Garry
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, 
_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 success 

[PATCH v4 3/6] iommu: Allow iommu_change_dev_def_domain() realloc default domain for same type

2021-07-14 Thread John Garry
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

2021-07-14 Thread John Garry
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 = >rcaches[i];
spin_lock_init(>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, >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(>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 = >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 = >rcaches[i];
cpu_rcache = per_cpu_ptr(rcache->cpu_rcaches, cpu);
spin_lock_irqsave(_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 = >rcaches[i];
spin_lock_irqsave(>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/include/linux/iova.h
+++ b/include/linux/iova.h
@@ -25,7 +25,6 

[PATCH v4 1/6] iommu: Refactor iommu_group_store_type()

2021-07-14 Thread John Garry
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(>mutex);
if (iommu_group_device_count(group) != 1) {
mutex_unlock(>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(>mutex);
-*  iommu_change_dev_def_domain();
+*  cb->iommu_change_dev_def_domain(); [example cb]
 *  mutex_unlock(>mutex);
 * device_unlock(dev);
 *
@@ -3240,7 +3234,7 @@ static ssize_t iommu_group_store_type(struct iommu_group 
*group,
 */
mutex_unlock(>mutex);
 
-   /* 

[PATCH v4 0/6] iommu: Allow IOVA rcache range be configured

2021-07-14 Thread John Garry
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

2021-07-14 Thread Joerg Roedel
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

2021-07-14 Thread Joerg Roedel
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

2021-07-14 Thread Joerg Roedel
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

2021-07-14 Thread Robin Murphy

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

2021-07-14 Thread Joerg Roedel
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

2021-07-14 Thread Joerg Roedel
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()

2021-07-14 Thread 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 = >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, );
> > > > --> 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-07-14 Thread Jason Wang


在 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 = >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, );
--> 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-07-14 Thread Jason Wang


在 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 

Aw: [PATCH v6 00/11] Clean up "mediatek,larb"

2021-07-14 Thread Frank Wunderlich
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-07-14 Thread Jason Wang


在 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(>waitq);
+   spin_lock(>msg_lock);
+   msg->req.request_id = dev->msg_unique++;
+   vduse_enqueue_msg(>send_list, msg);
+   wake_up(>waitq);
+   spin_unlock(>msg_lock);
+
+   wait_event_killable_timeout(msg->waitq, msg->completed,
+   VDUSE_REQUEST_TIMEOUT * HZ);
+   spin_lock(>msg_lock);
+   if (!msg->completed) {
+   list_del(>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"

2021-07-14 Thread Dafna Hirschfeld

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 +-
  

Re: [PATCH v6 01/11] dt-binding: mediatek: Get rid of mediatek, larb for multimedia HW

2021-07-14 Thread Dafna Hirschfeld



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 = < MT8173_POWER_DOMAIN_MM>;
  clocks = < CLK_MM_DISP_OVL0>;
  iommus = < M4U_PORT_DISP_OVL0>;
-    mediatek,larb = <>;
  };
  ovl1: ovl@1400d000 {
@@ -101,7 +98,6 @@ ovl1: ovl@1400d000 {
  power-domains = < MT8173_POWER_DOMAIN_MM>;
  clocks = < CLK_MM_DISP_OVL1>;
  iommus = < M4U_PORT_DISP_OVL1>;
-    mediatek,larb = <>;
  };
  rdma0: rdma@1400e000 {
@@ -111,7 +107,6 @@ rdma0: rdma@1400e000 {
  power-domains = < MT8173_POWER_DOMAIN_MM>;
  clocks = < CLK_MM_DISP_RDMA0>;
  iommus = < M4U_PORT_DISP_RDMA0>;
-    mediatek,larb = <>;
  mediatek,rdma-fifosize = <8192>;
  };
@@ -122,7 +117,6 @@ rdma1: rdma@1400f000 {
  power-domains = < MT8173_POWER_DOMAIN_MM>;
  clocks = < CLK_MM_DISP_RDMA1>;
  iommus = < M4U_PORT_DISP_RDMA1>;
-    mediatek,larb = <>;
  };
  rdma2: rdma@1401 {
@@ -132,7 +126,6 @@ rdma2: rdma@1401 {
  power-domains = < MT8173_POWER_DOMAIN_MM>;
  clocks = < CLK_MM_DISP_RDMA2>;
  iommus = < M4U_PORT_DISP_RDMA2>;
-    mediatek,larb = <>;
  };
  wdma0: wdma@14011000 {
@@ -142,7 +135,6 @@ wdma0: wdma@14011000 {
  power-domains = < MT8173_POWER_DOMAIN_MM>;
  clocks = < CLK_MM_DISP_WDMA0>;
  iommus = < M4U_PORT_DISP_WDMA0>;
-    mediatek,larb = <>;
  };
  wdma1: wdma@14012000 {
@@ -152,7 +144,6 @@ wdma1: wdma@14012000 {
  power-domains = < MT8173_POWER_DOMAIN_MM>;
  clocks = < CLK_MM_DISP_WDMA1>;
  iommus = < M4U_PORT_DISP_WDMA1>;
-    mediatek,larb = <>;
  };
  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 = < MT2701_POWER_DOMAIN_ISP>;
-  mediatek,larb = <>;
    iommus = < MT2701_M4U_PORT_JPGDEC_WDMA>,
 < MT2701_M4U_PORT_JPGDEC_BSDMA>;
  };
diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml 
b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
index fcd9b829e036..8bfdfdfaba59 100644
--- a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
+++ 

Re: [PATCH v6 06/11] drm/mediatek: Add pm runtime support for ovl and rdma

2021-07-14 Thread Dafna Hirschfeld




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, _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(>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, _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(>dev, _disp_rdma_component_ops);
  
+	pm_runtime_disable(>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

2021-07-14 Thread Laurent Pinchart
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 

Re: [PATCH v6 09/11] memory: mtk-smi: Get rid of mtk_smi_larb_get/put

2021-07-14 Thread Dafna Hirschfeld




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

2021-07-14 Thread Dafna Hirschfeld




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 = >pm;
pm->mtkdev = mtkdev;
dec_clk = >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 = >dev;
pdev = mtkdev->plat_dev;
pm->dev = >dev;
  
@@ -47,14 +34,11 @@ int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev)

dec_clk->clk_info = devm_kcalloc(>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, _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(>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(>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 = >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/media/platform/mtk-vcodec/mtk_vcodec_enc.c 

Re: [PATCH v6 07/11] drm/mediatek: Get rid of mtk_smi_larb_get/put

2021-07-14 Thread Dafna Hirschfeld




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 = _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 = _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_device 

Re: [PATCH v6 05/11] media: mtk-mdp: Get rid of mtk_smi_larb_get/put

2021-07-14 Thread Dafna Hirschfeld




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 = _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

2021-07-14 Thread Dafna Hirschfeld




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 = >dev;
-
-   ret = devm_clk_bulk_get(jpeg->dev, jpeg->variant->num_clks,
-   jpeg->variant->clks);
-   if (ret) {
-   dev_err(>dev, "failed to get jpeg clock:%d\n", ret);
-   put_device(>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(>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(>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(>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

2021-07-14 Thread Dafna Hirschfeld




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 != _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 >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 != _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 >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;
+   unsigned int larbid;
 

Re: [PATCH v6 01/11] dt-binding: mediatek: Get rid of mediatek, larb for multimedia HW

2021-07-14 Thread Dafna Hirschfeld

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 = < MT8173_POWER_DOMAIN_MM>;
clocks = < CLK_MM_DISP_OVL0>;
iommus = < M4U_PORT_DISP_OVL0>;
-   mediatek,larb = <>;
  };
  
  ovl1: ovl@1400d000 {

@@ -101,7 +98,6 @@ ovl1: ovl@1400d000 {
power-domains = < MT8173_POWER_DOMAIN_MM>;
clocks = < CLK_MM_DISP_OVL1>;
iommus = < M4U_PORT_DISP_OVL1>;
-   mediatek,larb = <>;
  };
  
  rdma0: rdma@1400e000 {

@@ -111,7 +107,6 @@ rdma0: rdma@1400e000 {
power-domains = < MT8173_POWER_DOMAIN_MM>;
clocks = < CLK_MM_DISP_RDMA0>;
iommus = < M4U_PORT_DISP_RDMA0>;
-   mediatek,larb = <>;
mediatek,rdma-fifosize = <8192>;
  };
  
@@ -122,7 +117,6 @@ rdma1: rdma@1400f000 {

power-domains = < MT8173_POWER_DOMAIN_MM>;
clocks = < CLK_MM_DISP_RDMA1>;
iommus = < M4U_PORT_DISP_RDMA1>;
-   mediatek,larb = <>;
  };
  
  rdma2: rdma@1401 {

@@ -132,7 +126,6 @@ rdma2: rdma@1401 {
power-domains = < MT8173_POWER_DOMAIN_MM>;
clocks = < CLK_MM_DISP_RDMA2>;
iommus = < M4U_PORT_DISP_RDMA2>;
-   mediatek,larb = <>;
  };
  
  wdma0: wdma@14011000 {

@@ -142,7 +135,6 @@ wdma0: wdma@14011000 {
power-domains = < MT8173_POWER_DOMAIN_MM>;
clocks = < CLK_MM_DISP_WDMA0>;
iommus = < M4U_PORT_DISP_WDMA0>;
-   mediatek,larb = <>;
  };
  
  wdma1: wdma@14012000 {

@@ -152,7 +144,6 @@ wdma1: wdma@14012000 {
power-domains = < MT8173_POWER_DOMAIN_MM>;
clocks = < CLK_MM_DISP_WDMA1>;
iommus = < M4U_PORT_DISP_WDMA1>;
-   mediatek,larb = <>;
  };
  
  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 = < MT2701_POWER_DOMAIN_ISP>;
-  mediatek,larb = <>;
iommus = < MT2701_M4U_PORT_JPGDEC_WDMA>,
 < MT2701_M4U_PORT_JPGDEC_BSDMA>;
  };
diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml 
b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
index fcd9b829e036..8bfdfdfaba59 100644
--- a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
+++ 

Re: [PATCH v9 13/17] vdpa: factor out vhost_vdpa_pa_map() and vhost_vdpa_pa_unmap()

2021-07-14 Thread 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 = >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, );
> >--> 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

Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace

2021-07-14 Thread 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   

Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace

2021-07-14 Thread 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(>waitq);
> > > > +   spin_lock(>msg_lock);
> > > > +   msg->req.request_id = dev->msg_unique++;
> > > > +   vduse_enqueue_msg(>send_list, msg);
> > > > +   wake_up(>waitq);
> > > > +   spin_unlock(>msg_lock);
> > > > +
> > > > +   wait_event_killable_timeout(msg->waitq, msg->completed,
> > > > +   VDUSE_REQUEST_TIMEOUT * HZ);
> > > > +   spin_lock(>msg_lock);
> > > > +   if (!msg->completed) {
> > > > +   list_del(>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?
___
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-07-14 Thread Jason Wang


在 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(>waitq);
+   spin_lock(>msg_lock);
+   msg->req.request_id = dev->msg_unique++;
+   vduse_enqueue_msg(>send_list, msg);
+   wake_up(>waitq);
+   spin_unlock(>msg_lock);
+
+   wait_event_killable_timeout(msg->waitq, msg->completed,
+   VDUSE_REQUEST_TIMEOUT * HZ);
+   spin_lock(>msg_lock);
+   if (!msg->completed) {
+   list_del(>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?


Thanks



  Let userspace have its
own code to set and use timers. This does mean that userspace will
likely have to change a bit to support this driver, such is life.



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