Re: [PATCH v4 2/7] accel/ivpu: Add Intel VPU MMU support
Hi, On 18.12.2022 10:13, Oded Gabbay wrote: > On Thu, Dec 8, 2022 at 1:08 PM Jacek Lawrynowicz > wrote: >> >> VPU Memory Management Unit is based on ARM MMU-600. >> It allows the creation of multiple virtual address spaces for >> the device and map noncontinuous host memory (there is no dedicated >> memory on the VPU). >> >> Address space is implemented as a struct ivpu_mmu_context, it has an ID, >> drm_mm allocator for VPU addresses and struct ivpu_mmu_pgtable that >> holds actual 3-level, 4KB page table. >> Context with ID 0 (global context) is created upon driver initialization >> and it's mainly used for mapping memory required to execute >> the firmware. >> Contexts with non-zero IDs are user contexts allocated each time >> the devices is open()-ed and they map command buffers and other >> workload-related memory. >> Workloads executing in a given contexts have access only >> to the memory mapped in this context. >> >> This patch is has to main files: > This patch has two main files: OK >> - ivpu_mmu_context.c handles MMU page tables and memory mapping >> - ivpu_mmu.c implements a driver that programs the MMU device >> >> Co-developed-by: Karol Wachowski >> Signed-off-by: Karol Wachowski >> Co-developed-by: Krystian Pradzynski >> Signed-off-by: Krystian Pradzynski >> Signed-off-by: Jacek Lawrynowicz >> --- >> drivers/accel/ivpu/Makefile | 4 +- >> drivers/accel/ivpu/ivpu_drv.c | 83 ++- >> drivers/accel/ivpu/ivpu_drv.h | 6 + >> drivers/accel/ivpu/ivpu_hw_mtl.c | 10 + >> drivers/accel/ivpu/ivpu_mmu.c | 875 ++ >> drivers/accel/ivpu/ivpu_mmu.h | 50 ++ >> drivers/accel/ivpu/ivpu_mmu_context.c | 385 >> drivers/accel/ivpu/ivpu_mmu_context.h | 49 ++ >> include/uapi/drm/ivpu_drm.h | 4 + >> 9 files changed, 1463 insertions(+), 3 deletions(-) >> create mode 100644 drivers/accel/ivpu/ivpu_mmu.c >> create mode 100644 drivers/accel/ivpu/ivpu_mmu.h >> create mode 100644 drivers/accel/ivpu/ivpu_mmu_context.c >> create mode 100644 drivers/accel/ivpu/ivpu_mmu_context.h >> >> diff --git a/drivers/accel/ivpu/Makefile b/drivers/accel/ivpu/Makefile >> index 28330c04e52f..37b8bf1d3247 100644 >> --- a/drivers/accel/ivpu/Makefile >> +++ b/drivers/accel/ivpu/Makefile >> @@ -3,6 +3,8 @@ >> >> intel_vpu-y := \ >> ivpu_drv.o \ >> - ivpu_hw_mtl.o >> + ivpu_hw_mtl.o \ >> + ivpu_mmu.o \ >> + ivpu_mmu_context.o >> >> obj-$(CONFIG_DRM_ACCEL_IVPU) += intel_vpu.o >> \ No newline at end of file >> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c >> index 8fbccb8d888b..a22d41ca5a4b 100644 >> --- a/drivers/accel/ivpu/ivpu_drv.c >> +++ b/drivers/accel/ivpu/ivpu_drv.c >> @@ -15,6 +15,8 @@ >> >> #include "ivpu_drv.h" >> #include "ivpu_hw.h" >> +#include "ivpu_mmu.h" >> +#include "ivpu_mmu_context.h" >> >> #ifndef DRIVER_VERSION_STR >> #define DRIVER_VERSION_STR __stringify(DRM_IVPU_DRIVER_MAJOR) "." \ >> @@ -37,23 +39,38 @@ MODULE_PARM_DESC(pll_max_ratio, "Maximum PLL ratio used >> to set VPU frequency"); >> >> struct ivpu_file_priv *ivpu_file_priv_get(struct ivpu_file_priv *file_priv) >> { >> + struct ivpu_device *vdev = file_priv->vdev; >> + >> kref_get(_priv->ref); >> + >> + ivpu_dbg(vdev, KREF, "file_priv get: ctx %u refcount %u\n", >> +file_priv->ctx.id, kref_read(_priv->ref)); >> + >> return file_priv; >> } >> >> static void file_priv_release(struct kref *ref) >> { >> struct ivpu_file_priv *file_priv = container_of(ref, struct >> ivpu_file_priv, ref); >> + struct ivpu_device *vdev = file_priv->vdev; >> >> + ivpu_dbg(vdev, FILE, "file_priv release: ctx %u\n", >> file_priv->ctx.id); >> + >> + ivpu_mmu_user_context_fini(vdev, _priv->ctx); >> + WARN_ON(xa_erase_irq(>context_xa, file_priv->ctx.id) != >> file_priv); >> kfree(file_priv); >> } >> >> void ivpu_file_priv_put(struct ivpu_file_priv **link) >> { >> struct ivpu_file_priv *file_priv = *link; >> + struct ivpu_device *vdev = file_priv->vdev; >> >> WARN_ON(!file_priv); >> >> + ivpu_dbg(vdev, KREF, "file_priv put: ctx %u refcount %u\n", >> +file_priv->ctx.id, kref_read(_priv->ref)); >> + >> *link = NULL; >> kref_put(_priv->ref, file_priv_release); >> } >> @@ -88,6 +105,9 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, >> void *data, struct drm_f >> case DRM_IVPU_PARAM_CONTEXT_PRIORITY: >> args->value = file_priv->priority; >> break; >> + case DRM_IVPU_PARAM_CONTEXT_ID: >> + args->value = file_priv->ctx.id; > Why is this needed ? Why does the user need to know its context ID ? This is not really needed by the user space, we only use this as a debug feature. >> + break; >> default: >> ret = -EINVAL; >> break; >> @@
Re: [PATCH v4 2/7] accel/ivpu: Add Intel VPU MMU support
On Thu, Dec 8, 2022 at 1:08 PM Jacek Lawrynowicz wrote: > > VPU Memory Management Unit is based on ARM MMU-600. > It allows the creation of multiple virtual address spaces for > the device and map noncontinuous host memory (there is no dedicated > memory on the VPU). > > Address space is implemented as a struct ivpu_mmu_context, it has an ID, > drm_mm allocator for VPU addresses and struct ivpu_mmu_pgtable that > holds actual 3-level, 4KB page table. > Context with ID 0 (global context) is created upon driver initialization > and it's mainly used for mapping memory required to execute > the firmware. > Contexts with non-zero IDs are user contexts allocated each time > the devices is open()-ed and they map command buffers and other > workload-related memory. > Workloads executing in a given contexts have access only > to the memory mapped in this context. > > This patch is has to main files: This patch has two main files: > - ivpu_mmu_context.c handles MMU page tables and memory mapping > - ivpu_mmu.c implements a driver that programs the MMU device > > Co-developed-by: Karol Wachowski > Signed-off-by: Karol Wachowski > Co-developed-by: Krystian Pradzynski > Signed-off-by: Krystian Pradzynski > Signed-off-by: Jacek Lawrynowicz > --- > drivers/accel/ivpu/Makefile | 4 +- > drivers/accel/ivpu/ivpu_drv.c | 83 ++- > drivers/accel/ivpu/ivpu_drv.h | 6 + > drivers/accel/ivpu/ivpu_hw_mtl.c | 10 + > drivers/accel/ivpu/ivpu_mmu.c | 875 ++ > drivers/accel/ivpu/ivpu_mmu.h | 50 ++ > drivers/accel/ivpu/ivpu_mmu_context.c | 385 > drivers/accel/ivpu/ivpu_mmu_context.h | 49 ++ > include/uapi/drm/ivpu_drm.h | 4 + > 9 files changed, 1463 insertions(+), 3 deletions(-) > create mode 100644 drivers/accel/ivpu/ivpu_mmu.c > create mode 100644 drivers/accel/ivpu/ivpu_mmu.h > create mode 100644 drivers/accel/ivpu/ivpu_mmu_context.c > create mode 100644 drivers/accel/ivpu/ivpu_mmu_context.h > > diff --git a/drivers/accel/ivpu/Makefile b/drivers/accel/ivpu/Makefile > index 28330c04e52f..37b8bf1d3247 100644 > --- a/drivers/accel/ivpu/Makefile > +++ b/drivers/accel/ivpu/Makefile > @@ -3,6 +3,8 @@ > > intel_vpu-y := \ > ivpu_drv.o \ > - ivpu_hw_mtl.o > + ivpu_hw_mtl.o \ > + ivpu_mmu.o \ > + ivpu_mmu_context.o > > obj-$(CONFIG_DRM_ACCEL_IVPU) += intel_vpu.o > \ No newline at end of file > diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c > index 8fbccb8d888b..a22d41ca5a4b 100644 > --- a/drivers/accel/ivpu/ivpu_drv.c > +++ b/drivers/accel/ivpu/ivpu_drv.c > @@ -15,6 +15,8 @@ > > #include "ivpu_drv.h" > #include "ivpu_hw.h" > +#include "ivpu_mmu.h" > +#include "ivpu_mmu_context.h" > > #ifndef DRIVER_VERSION_STR > #define DRIVER_VERSION_STR __stringify(DRM_IVPU_DRIVER_MAJOR) "." \ > @@ -37,23 +39,38 @@ MODULE_PARM_DESC(pll_max_ratio, "Maximum PLL ratio used > to set VPU frequency"); > > struct ivpu_file_priv *ivpu_file_priv_get(struct ivpu_file_priv *file_priv) > { > + struct ivpu_device *vdev = file_priv->vdev; > + > kref_get(_priv->ref); > + > + ivpu_dbg(vdev, KREF, "file_priv get: ctx %u refcount %u\n", > +file_priv->ctx.id, kref_read(_priv->ref)); > + > return file_priv; > } > > static void file_priv_release(struct kref *ref) > { > struct ivpu_file_priv *file_priv = container_of(ref, struct > ivpu_file_priv, ref); > + struct ivpu_device *vdev = file_priv->vdev; > > + ivpu_dbg(vdev, FILE, "file_priv release: ctx %u\n", > file_priv->ctx.id); > + > + ivpu_mmu_user_context_fini(vdev, _priv->ctx); > + WARN_ON(xa_erase_irq(>context_xa, file_priv->ctx.id) != > file_priv); > kfree(file_priv); > } > > void ivpu_file_priv_put(struct ivpu_file_priv **link) > { > struct ivpu_file_priv *file_priv = *link; > + struct ivpu_device *vdev = file_priv->vdev; > > WARN_ON(!file_priv); > > + ivpu_dbg(vdev, KREF, "file_priv put: ctx %u refcount %u\n", > +file_priv->ctx.id, kref_read(_priv->ref)); > + > *link = NULL; > kref_put(_priv->ref, file_priv_release); > } > @@ -88,6 +105,9 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, > void *data, struct drm_f > case DRM_IVPU_PARAM_CONTEXT_PRIORITY: > args->value = file_priv->priority; > break; > + case DRM_IVPU_PARAM_CONTEXT_ID: > + args->value = file_priv->ctx.id; Why is this needed ? Why does the user need to know its context ID ? > + break; > default: > ret = -EINVAL; > break; > @@ -120,22 +140,59 @@ static int ivpu_open(struct drm_device *dev, struct > drm_file *file) > { > struct ivpu_device *vdev = to_ivpu_device(dev); > struct ivpu_file_priv *file_priv; > + u32 ctx_id; > + void *old; > + int ret; > + > +
[PATCH v4 2/7] accel/ivpu: Add Intel VPU MMU support
VPU Memory Management Unit is based on ARM MMU-600. It allows the creation of multiple virtual address spaces for the device and map noncontinuous host memory (there is no dedicated memory on the VPU). Address space is implemented as a struct ivpu_mmu_context, it has an ID, drm_mm allocator for VPU addresses and struct ivpu_mmu_pgtable that holds actual 3-level, 4KB page table. Context with ID 0 (global context) is created upon driver initialization and it's mainly used for mapping memory required to execute the firmware. Contexts with non-zero IDs are user contexts allocated each time the devices is open()-ed and they map command buffers and other workload-related memory. Workloads executing in a given contexts have access only to the memory mapped in this context. This patch is has to main files: - ivpu_mmu_context.c handles MMU page tables and memory mapping - ivpu_mmu.c implements a driver that programs the MMU device Co-developed-by: Karol Wachowski Signed-off-by: Karol Wachowski Co-developed-by: Krystian Pradzynski Signed-off-by: Krystian Pradzynski Signed-off-by: Jacek Lawrynowicz --- drivers/accel/ivpu/Makefile | 4 +- drivers/accel/ivpu/ivpu_drv.c | 83 ++- drivers/accel/ivpu/ivpu_drv.h | 6 + drivers/accel/ivpu/ivpu_hw_mtl.c | 10 + drivers/accel/ivpu/ivpu_mmu.c | 875 ++ drivers/accel/ivpu/ivpu_mmu.h | 50 ++ drivers/accel/ivpu/ivpu_mmu_context.c | 385 drivers/accel/ivpu/ivpu_mmu_context.h | 49 ++ include/uapi/drm/ivpu_drm.h | 4 + 9 files changed, 1463 insertions(+), 3 deletions(-) create mode 100644 drivers/accel/ivpu/ivpu_mmu.c create mode 100644 drivers/accel/ivpu/ivpu_mmu.h create mode 100644 drivers/accel/ivpu/ivpu_mmu_context.c create mode 100644 drivers/accel/ivpu/ivpu_mmu_context.h diff --git a/drivers/accel/ivpu/Makefile b/drivers/accel/ivpu/Makefile index 28330c04e52f..37b8bf1d3247 100644 --- a/drivers/accel/ivpu/Makefile +++ b/drivers/accel/ivpu/Makefile @@ -3,6 +3,8 @@ intel_vpu-y := \ ivpu_drv.o \ - ivpu_hw_mtl.o + ivpu_hw_mtl.o \ + ivpu_mmu.o \ + ivpu_mmu_context.o obj-$(CONFIG_DRM_ACCEL_IVPU) += intel_vpu.o \ No newline at end of file diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c index 8fbccb8d888b..a22d41ca5a4b 100644 --- a/drivers/accel/ivpu/ivpu_drv.c +++ b/drivers/accel/ivpu/ivpu_drv.c @@ -15,6 +15,8 @@ #include "ivpu_drv.h" #include "ivpu_hw.h" +#include "ivpu_mmu.h" +#include "ivpu_mmu_context.h" #ifndef DRIVER_VERSION_STR #define DRIVER_VERSION_STR __stringify(DRM_IVPU_DRIVER_MAJOR) "." \ @@ -37,23 +39,38 @@ MODULE_PARM_DESC(pll_max_ratio, "Maximum PLL ratio used to set VPU frequency"); struct ivpu_file_priv *ivpu_file_priv_get(struct ivpu_file_priv *file_priv) { + struct ivpu_device *vdev = file_priv->vdev; + kref_get(_priv->ref); + + ivpu_dbg(vdev, KREF, "file_priv get: ctx %u refcount %u\n", +file_priv->ctx.id, kref_read(_priv->ref)); + return file_priv; } static void file_priv_release(struct kref *ref) { struct ivpu_file_priv *file_priv = container_of(ref, struct ivpu_file_priv, ref); + struct ivpu_device *vdev = file_priv->vdev; + ivpu_dbg(vdev, FILE, "file_priv release: ctx %u\n", file_priv->ctx.id); + + ivpu_mmu_user_context_fini(vdev, _priv->ctx); + WARN_ON(xa_erase_irq(>context_xa, file_priv->ctx.id) != file_priv); kfree(file_priv); } void ivpu_file_priv_put(struct ivpu_file_priv **link) { struct ivpu_file_priv *file_priv = *link; + struct ivpu_device *vdev = file_priv->vdev; WARN_ON(!file_priv); + ivpu_dbg(vdev, KREF, "file_priv put: ctx %u refcount %u\n", +file_priv->ctx.id, kref_read(_priv->ref)); + *link = NULL; kref_put(_priv->ref, file_priv_release); } @@ -88,6 +105,9 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_f case DRM_IVPU_PARAM_CONTEXT_PRIORITY: args->value = file_priv->priority; break; + case DRM_IVPU_PARAM_CONTEXT_ID: + args->value = file_priv->ctx.id; + break; default: ret = -EINVAL; break; @@ -120,22 +140,59 @@ static int ivpu_open(struct drm_device *dev, struct drm_file *file) { struct ivpu_device *vdev = to_ivpu_device(dev); struct ivpu_file_priv *file_priv; + u32 ctx_id; + void *old; + int ret; + + ret = xa_alloc_irq(>context_xa, _id, NULL, vdev->context_xa_limit, GFP_KERNEL); + if (ret) { + ivpu_err(vdev, "Failed to allocate context id: %d\n", ret); + return ret; + } file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL); - if (!file_priv) - return -ENOMEM; + if (!file_priv) { + ret = -ENOMEM; +