On 2020/1/10 上午1:38, Jonathan Cameron wrote:
On Mon, 16 Dec 2019 11:08:15 +0800
Zhangfei Gao <zhangfei.gao@xxxxxxxxxx> wrote:
From: Kenneth Lee <liguozhu@xxxxxxxxxxxxx>
Uacce (Unified/User-space-access-intended Accelerator Framework) targets to
provide Shared Virtual Addressing (SVA) between accelerators and processes.
So accelerator can access any data structure of the main cpu.
This differs from the data sharing between cpu and io device, which share
only data content rather than address.
Since unified address, hardware and user space of process can share the
same virtual address in the communication.
Uacce create a chrdev for every registration, the queue is allocated to
the process when the chrdev is opened. Then the process can access the
hardware resource by interact with the queue file. By mmap the queue
file space to user space, the process can directly put requests to the
hardware without syscall to the kernel space.
The IOMMU core only tracks mm<->device bonds at the moment, because it
only needs to handle IOTLB invalidation and PASID table entries. However
uacce needs a finer granularity since multiple queues from the same
device can be bound to an mm. When the mm exits, all bound queues must
be stopped so that the IOMMU can safely clear the PASID table entry and
reallocate the PASID.
An intermediate struct uacce_mm links uacce devices and queues.
Note that an mm may be bound to multiple devices but an uacce_mm
structure only ever belongs to a single device, because we don't need
anything more complex (if multiple devices are bound to one mm, then
we'll create one uacce_mm for each bond).
uacce_device --+-- uacce_mm --+-- uacce_queue
| '-- uacce_queue
|
'-- uacce_mm --+-- uacce_queue
+-- uacce_queue
'-- uacce_queue
Signed-off-by: Kenneth Lee <liguozhu@xxxxxxxxxxxxx>
Signed-off-by: Zaibo Xu <xuzaibo@xxxxxxxxxx>
Signed-off-by: Zhou Wang <wangzhou1@xxxxxxxxxxxxx>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@xxxxxxxxxx>
Signed-off-by: Zhangfei Gao <zhangfei.gao@xxxxxxxxxx>
Hi,
Two small things I'd missed previously. Fix those and for
what it's worth
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
Thanks Jonathan
---
Documentation/ABI/testing/sysfs-driver-uacce | 37 ++
drivers/misc/Kconfig | 1 +
drivers/misc/Makefile | 1 +
drivers/misc/uacce/Kconfig | 13 +
drivers/misc/uacce/Makefile | 2 +
drivers/misc/uacce/uacce.c | 628 +++++++++++++++++++++++++++
include/linux/uacce.h | 161 +++++++
include/uapi/misc/uacce/uacce.h | 38 ++
8 files changed, 881 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-uacce
create mode 100644 drivers/misc/uacce/Kconfig
create mode 100644 drivers/misc/uacce/Makefile
create mode 100644 drivers/misc/uacce/uacce.c
create mode 100644 include/linux/uacce.h
create mode 100644 include/uapi/misc/uacce/uacce.h
...
+
+What: /sys/class/uacce/<dev_name>/available_instances
+Date: Dec 2019
+KernelVersion: 5.6
+Contact: linux-accelerators@xxxxxxxxxxxxxxxx
+Description: Available instances left of the device
+ Return -ENODEV if uacce_ops get_available_instances is not provided
+
See below. It doesn't "return" it prints it currently.
Will update to
'unknown' if uacce_ops get_available_instances is not provided
...
+static int uacce_fops_mmap(struct file *filep, struct vm_area_struct *vma)
+{
+ struct uacce_queue *q = filep->private_data;
+ struct uacce_device *uacce = q->uacce;
+ struct uacce_qfile_region *qfr;
+ enum uacce_qfrt type = UACCE_MAX_REGION;
+ int ret = 0;
+
+ if (vma->vm_pgoff < UACCE_MAX_REGION)
+ type = vma->vm_pgoff;
+ else
+ return -EINVAL;
+
+ qfr = kzalloc(sizeof(*qfr), GFP_KERNEL);
+ if (!qfr)
+ return -ENOMEM;
+
+ vma->vm_flags |= VM_DONTCOPY | VM_DONTEXPAND | VM_WIPEONFORK;
+ vma->vm_ops = &uacce_vm_ops;
+ vma->vm_private_data = q;
+ qfr->type = type;
+
+ mutex_lock(&uacce_mutex);
+
+ if (q->state != UACCE_Q_INIT && q->state != UACCE_Q_STARTED) {
+ ret = -EINVAL;
+ goto out_with_lock;
+ }
+
+ if (q->qfrs[type]) {
+ ret = -EEXIST;
+ goto out_with_lock;
+ }
+
+ switch (type) {
+ case UACCE_QFRT_MMIO:
+ if (!uacce->ops->mmap) {
+ ret = -EINVAL;
+ goto out_with_lock;
+ }
+
+ ret = uacce->ops->mmap(q, vma, qfr);
+ if (ret)
+ goto out_with_lock;
+
+ break;
+
+ case UACCE_QFRT_DUS:
+ if (uacce->flags & UACCE_DEV_SVA) {
+ if (!uacce->ops->mmap) {
+ ret = -EINVAL;
+ goto out_with_lock;
+ }
+
+ ret = uacce->ops->mmap(q, vma, qfr);
+ if (ret)
+ goto out_with_lock;
+ }
Slightly odd corner case, but what stops us getting here with
the UACCE_DEV_SVA flag not set? If that happened I'd expect to
return an error but looks like we return 0.
The check with flag UACCE_DEV_SVA can be removed here, non-sva also has
dus region.
We have removed the check when we add non-sva support.
...
+static ssize_t available_instances_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct uacce_device *uacce = to_uacce_device(dev);
+ int val = -ENODEV;
+
+ if (uacce->ops->get_available_instances)
+ val = uacce->ops->get_available_instances(uacce);
+
+ return sprintf(buf, "%d\n", val);
It's unusual to pass an error value back as a string.
I'd expect some logic like..
if (val < 0)
return val;
return sprintf(buf, "%d\n", val);
Note this is the documented behavior "returns -ENODEV".
If return -ENODEV,
cat /sys/class/uacce/hisi_zip-0/available_instances
cat: /sys/class/uacce/hisi_zip-0/available_instances: No such device
I think print "unknown" maybe better, like cpufreq.c
if (uacce->ops->get_available_instances)
return sprintf(buf, "%d\n",
uacce->ops->get_available_instances(uacce));
return sprintf(buf, "unknown\n");
Thanks
