| Message ID | 20250125-virtgpu-mixed-page-size-v2-1-c40c555cf276@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/virtio: Align host mapping request to maximum platform page size | expand |
Hi Am 25.01.25 um 22:08 schrieb Sasha Finkelstein via B4 Relay: > From: Sasha Finkelstein <fnkl.kernel@gmail.com> > > This allows running different page sizes between host and guest on > platforms that support mixed page sizes. > > Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com> > --- > Changes in v2: > - Aligned all object sizes to MAX_PAGE_SIZE too. > - Link to v1: https://lore.kernel.org/r/20250109-virtgpu-mixed-page-size-v1-1-c8fe1e1859f3@gmail.com > --- > drivers/gpu/drm/virtio/virtgpu_drv.h | 6 ++++++ > drivers/gpu/drm/virtio/virtgpu_gem.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_object.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_vram.c | 4 ++-- > 5 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > index 64c236169db88acd6ba9afd20a1ab16c667490c4..b73844d6535e45402dc46898035eaa7492de935d 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > @@ -61,6 +61,12 @@ > /* See virtio_gpu_ctx_create. One additional character for NULL terminator. */ > #define DEBUG_NAME_MAX_LEN 65 > > +#if defined(__powerpc64__) || defined(__aarch64__) || defined(__mips__) || defined(__loongarch__) > +#define MAX_PAGE_SIZE SZ_64K > +#else > +#define MAX_PAGE_SIZE PAGE_SIZE > +#endif This is per-architecture code and does not belong in a DRM driver. Best regards Thomas > + > struct virtio_gpu_object_params { > unsigned long size; > bool dumb; > diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c > index 7db48d17ee3a8a9c638a8c6f9e58f35bd004b453..8e625ccae308f46b11a390a0987fd5ea55ccbf8d 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_gem.c > +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c > @@ -73,7 +73,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv, > > pitch = args->width * 4; > args->size = pitch * args->height; > - args->size = ALIGN(args->size, PAGE_SIZE); > + args->size = ALIGN(args->size, MAX_PAGE_SIZE); > > params.format = virtio_gpu_translate_format(DRM_FORMAT_HOST_XRGB8888); > params.width = args->width; > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > index e4f76f31555049369000a50c0cb1d5edab68536b..e39ae008ac5f734ec52e9703413958b4ea4be7d6 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > @@ -167,7 +167,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, > params.size = rc->size; > /* allocate a single page size object */ > if (params.size == 0) > - params.size = PAGE_SIZE; > + params.size = MAX_PAGE_SIZE; > > fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0); > if (!fence) > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c > index c7e74cf130221bbed3aa447e416065b03bf3e2b4..5aab82028293b7d73bcc4e686e7e2ecba6108b7b 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > @@ -190,7 +190,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, > > *bo_ptr = NULL; > > - params->size = roundup(params->size, PAGE_SIZE); > + params->size = roundup(params->size, MAX_PAGE_SIZE); > shmem_obj = drm_gem_shmem_create(vgdev->ddev, params->size); > if (IS_ERR(shmem_obj)) > return PTR_ERR(shmem_obj); > diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c > index 25df81c027837c248a746e41856b5aa7e216b8d5..18e773b036f7c8190d4e20ad3f2c85c36e7e295c 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_vram.c > +++ b/drivers/gpu/drm/virtio/virtgpu_vram.c > @@ -150,8 +150,8 @@ static int virtio_gpu_vram_map(struct virtio_gpu_object *bo) > return -EINVAL; > > spin_lock(&vgdev->host_visible_lock); > - ret = drm_mm_insert_node(&vgdev->host_visible_mm, &vram->vram_node, > - bo->base.base.size); > + ret = drm_mm_insert_node_generic(&vgdev->host_visible_mm, &vram->vram_node, > + bo->base.base.size, MAX_PAGE_SIZE, 0, 0); > spin_unlock(&vgdev->host_visible_lock); > > if (ret) > > --- > base-commit: 643e2e259c2b25a2af0ae4c23c6e16586d9fd19c > change-id: 20250109-virtgpu-mixed-page-size-282b8f4a02fc > >
On Mon, 27 Jan 2025 at 08:31, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> This is per-architecture code and does not belong in a DRM driver.
I agree that this is not quite the place, and ideally i'd use MAX_PAGE_SIZE
from https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/
however that series has not landed yet. Would a todo comment
with instructions to replace it with that when that lands be ok?
On 1/26/25 00:08, Sasha Finkelstein via B4 Relay wrote: > --- a/drivers/gpu/drm/virtio/virtgpu_vram.c > +++ b/drivers/gpu/drm/virtio/virtgpu_vram.c > @@ -150,8 +150,8 @@ static int virtio_gpu_vram_map(struct virtio_gpu_object *bo) > return -EINVAL; > > spin_lock(&vgdev->host_visible_lock); > - ret = drm_mm_insert_node(&vgdev->host_visible_mm, &vram->vram_node, > - bo->base.base.size); > + ret = drm_mm_insert_node_generic(&vgdev->host_visible_mm, &vram->vram_node, > + bo->base.base.size, MAX_PAGE_SIZE, 0, 0); > spin_unlock(&vgdev->host_visible_lock); The BO size is already aligned, no need to align it second time. Anyways, we first should wait for a reply from Rob RE potential impact of this change on Freedreno and other non-x86 drivers and non-nctx contexts. Otherwise, the proper solution would be to pass info about host's page size to guest using extended virtio protocol. This is very doable if you have time to work on this useful feature and want to contribute/learn more.
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 64c236169db88acd6ba9afd20a1ab16c667490c4..b73844d6535e45402dc46898035eaa7492de935d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -61,6 +61,12 @@ /* See virtio_gpu_ctx_create. One additional character for NULL terminator. */ #define DEBUG_NAME_MAX_LEN 65 +#if defined(__powerpc64__) || defined(__aarch64__) || defined(__mips__) || defined(__loongarch__) +#define MAX_PAGE_SIZE SZ_64K +#else +#define MAX_PAGE_SIZE PAGE_SIZE +#endif + struct virtio_gpu_object_params { unsigned long size; bool dumb; diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c index 7db48d17ee3a8a9c638a8c6f9e58f35bd004b453..8e625ccae308f46b11a390a0987fd5ea55ccbf8d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_gem.c +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c @@ -73,7 +73,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv, pitch = args->width * 4; args->size = pitch * args->height; - args->size = ALIGN(args->size, PAGE_SIZE); + args->size = ALIGN(args->size, MAX_PAGE_SIZE); params.format = virtio_gpu_translate_format(DRM_FORMAT_HOST_XRGB8888); params.width = args->width; diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index e4f76f31555049369000a50c0cb1d5edab68536b..e39ae008ac5f734ec52e9703413958b4ea4be7d6 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -167,7 +167,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, params.size = rc->size; /* allocate a single page size object */ if (params.size == 0) - params.size = PAGE_SIZE; + params.size = MAX_PAGE_SIZE; fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0); if (!fence) diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index c7e74cf130221bbed3aa447e416065b03bf3e2b4..5aab82028293b7d73bcc4e686e7e2ecba6108b7b 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -190,7 +190,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, *bo_ptr = NULL; - params->size = roundup(params->size, PAGE_SIZE); + params->size = roundup(params->size, MAX_PAGE_SIZE); shmem_obj = drm_gem_shmem_create(vgdev->ddev, params->size); if (IS_ERR(shmem_obj)) return PTR_ERR(shmem_obj); diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c index 25df81c027837c248a746e41856b5aa7e216b8d5..18e773b036f7c8190d4e20ad3f2c85c36e7e295c 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vram.c +++ b/drivers/gpu/drm/virtio/virtgpu_vram.c @@ -150,8 +150,8 @@ static int virtio_gpu_vram_map(struct virtio_gpu_object *bo) return -EINVAL; spin_lock(&vgdev->host_visible_lock); - ret = drm_mm_insert_node(&vgdev->host_visible_mm, &vram->vram_node, - bo->base.base.size); + ret = drm_mm_insert_node_generic(&vgdev->host_visible_mm, &vram->vram_node, + bo->base.base.size, MAX_PAGE_SIZE, 0, 0); spin_unlock(&vgdev->host_visible_lock); if (ret)