2020-08-13 06:23:56

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: [PATCH v2 0/5] Fixes and improvements for Xen pvdrm

From: Oleksandr Andrushchenko <[email protected]>

Hello,

This series contains an assorted set of fixes and improvements for
the Xen para-virtualized display driver and grant device driver which
I have collected over the last couple of months:

1. Minor fixes to grant device driver and drm/xen-front.

2. New format (YUYV) added to the list of the PV DRM supported formats
which allows the driver to be used in zero-copying use-cases when
a camera device is the source of the dma-bufs.

3. Synchronization with the latest para-virtualized protocol definition
in Xen [1].

4. SGT offset is now propagated to the backend: while importing a dmabuf
it is possible that the data of the buffer is put with offset which is
indicated by the SGT offset. This is needed for some GPUs which have
non-zero offset.

Thank you,
Oleksandr Andrushchenko

[1] https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=c27a184225eab54d20435c8cab5ad0ef384dc2c0

Changes since v1:
=================

1. Removed patch which adds EDID to PV DRM as it needs more time for review:
"5. Version 2 of the Xen displif protocol adds XENDISPL_OP_GET_EDID
request which allows frontends to request EDID structure per
connector. This request is optional and if not supported by the
backend then visible area is still defined by the relevant
XenStore's "resolution" property.
If backend provides EDID with XENDISPL_OP_GET_EDID request then
its values must take precedence over the resolutions defined in
XenStore."
I will send this as a dedicated patch.

2. Added missing CC stable for the patches with fixes

Oleksandr Andrushchenko (5):
xen/gntdev: Fix dmabuf import with non-zero sgt offset
drm/xen-front: Fix misused IS_ERR_OR_NULL checks
drm/xen-front: Add YUYV to supported formats
xen: Sync up with the canonical protocol definition in Xen
drm/xen-front: Pass dumb buffer data offset to the backend

drivers/gpu/drm/xen/xen_drm_front.c | 10 +--
drivers/gpu/drm/xen/xen_drm_front.h | 2 +-
drivers/gpu/drm/xen/xen_drm_front_conn.c | 1 +
drivers/gpu/drm/xen/xen_drm_front_gem.c | 11 +--
drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +-
drivers/xen/gntdev-dmabuf.c | 8 +++
include/xen/interface/io/displif.h | 91 +++++++++++++++++++++++-
7 files changed, 111 insertions(+), 14 deletions(-)

--
2.17.1


2020-08-13 06:25:16

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: [PATCH v2 2/5] drm/xen-front: Fix misused IS_ERR_OR_NULL checks

From: Oleksandr Andrushchenko <[email protected]>

The patch c575b7eeb89f: "drm/xen-front: Add support for Xen PV
display frontend" from Apr 3, 2018, leads to the following static
checker warning:

drivers/gpu/drm/xen/xen_drm_front_gem.c:140 xen_drm_front_gem_create()
warn: passing zero to 'ERR_CAST'

drivers/gpu/drm/xen/xen_drm_front_gem.c
133 struct drm_gem_object *xen_drm_front_gem_create(struct drm_device *dev,
134 size_t size)
135 {
136 struct xen_gem_object *xen_obj;
137
138 xen_obj = gem_create(dev, size);
139 if (IS_ERR_OR_NULL(xen_obj))
140 return ERR_CAST(xen_obj);

Fix this and the rest of misused places with IS_ERR_OR_NULL in the
driver.

Fixes: c575b7eeb89f: "drm/xen-front: Add support for Xen PV display frontend"

Signed-off-by: Oleksandr Andrushchenko <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Reviewed-by: Dan Carpenter <[email protected]>
Cc: <[email protected]>
---
drivers/gpu/drm/xen/xen_drm_front.c | 4 ++--
drivers/gpu/drm/xen/xen_drm_front_gem.c | 8 ++++----
drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c
index 1fd458e877ca..51818e76facd 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.c
+++ b/drivers/gpu/drm/xen/xen_drm_front.c
@@ -400,8 +400,8 @@ static int xen_drm_drv_dumb_create(struct drm_file *filp,
args->size = args->pitch * args->height;

obj = xen_drm_front_gem_create(dev, args->size);
- if (IS_ERR_OR_NULL(obj)) {
- ret = PTR_ERR_OR_ZERO(obj);
+ if (IS_ERR(obj)) {
+ ret = PTR_ERR(obj);
goto fail;
}

diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
index f0b85e094111..4ec8a49241e1 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -83,7 +83,7 @@ static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)

size = round_up(size, PAGE_SIZE);
xen_obj = gem_create_obj(dev, size);
- if (IS_ERR_OR_NULL(xen_obj))
+ if (IS_ERR(xen_obj))
return xen_obj;

if (drm_info->front_info->cfg.be_alloc) {
@@ -117,7 +117,7 @@ static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)
*/
xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
xen_obj->pages = drm_gem_get_pages(&xen_obj->base);
- if (IS_ERR_OR_NULL(xen_obj->pages)) {
+ if (IS_ERR(xen_obj->pages)) {
ret = PTR_ERR(xen_obj->pages);
xen_obj->pages = NULL;
goto fail;
@@ -136,7 +136,7 @@ struct drm_gem_object *xen_drm_front_gem_create(struct drm_device *dev,
struct xen_gem_object *xen_obj;

xen_obj = gem_create(dev, size);
- if (IS_ERR_OR_NULL(xen_obj))
+ if (IS_ERR(xen_obj))
return ERR_CAST(xen_obj);

return &xen_obj->base;
@@ -194,7 +194,7 @@ xen_drm_front_gem_import_sg_table(struct drm_device *dev,

size = attach->dmabuf->size;
xen_obj = gem_create_obj(dev, size);
- if (IS_ERR_OR_NULL(xen_obj))
+ if (IS_ERR(xen_obj))
return ERR_CAST(xen_obj);

ret = gem_alloc_pages_array(xen_obj, size);
diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
index 78096bbcd226..ef11b1e4de39 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_kms.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
@@ -60,7 +60,7 @@ fb_create(struct drm_device *dev, struct drm_file *filp,
int ret;

fb = drm_gem_fb_create_with_funcs(dev, filp, mode_cmd, &fb_funcs);
- if (IS_ERR_OR_NULL(fb))
+ if (IS_ERR(fb))
return fb;

gem_obj = fb->obj[0];
--
2.17.1

2020-08-13 06:25:27

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: [PATCH v2 1/5] xen/gntdev: Fix dmabuf import with non-zero sgt offset

From: Oleksandr Andrushchenko <[email protected]>

It is possible that the scatter-gather table during dmabuf import has
non-zero offset of the data, but user-space doesn't expect that.
Fix this by failing the import, so user-space doesn't access wrong data.

Fixes: bf8dc55b1358 ("xen/gntdev: Implement dma-buf import functionality")

Signed-off-by: Oleksandr Andrushchenko <[email protected]>
Acked-by: Juergen Gross <[email protected]>
Cc: <[email protected]>
---
drivers/xen/gntdev-dmabuf.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c
index 75d3bb948bf3..b1b6eebafd5d 100644
--- a/drivers/xen/gntdev-dmabuf.c
+++ b/drivers/xen/gntdev-dmabuf.c
@@ -613,6 +613,14 @@ dmabuf_imp_to_refs(struct gntdev_dmabuf_priv *priv, struct device *dev,
goto fail_detach;
}

+ /* Check that we have zero offset. */
+ if (sgt->sgl->offset) {
+ ret = ERR_PTR(-EINVAL);
+ pr_debug("DMA buffer has %d bytes offset, user-space expects 0\n",
+ sgt->sgl->offset);
+ goto fail_unmap;
+ }
+
/* Check number of pages that imported buffer has. */
if (attach->dmabuf->size != gntdev_dmabuf->nr_pages << PAGE_SHIFT) {
ret = ERR_PTR(-EINVAL);
--
2.17.1

2020-08-13 07:06:29

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Fixes and improvements for Xen pvdrm

On 13.08.20 08:32, Oleksandr Andrushchenko wrote:
> Juergen, Boris,
>
> can we please merge these via Xen Linux tree as I have collected enough Ack/R-b?
>
> The series has DRM patches, but those anyway are Xen related, so I think
>
> this should be fine from DRI point of view.

Yes, fine with me.


Juergen

>
> Thank you,
>
> Oleksandr
>
> On 8/13/20 9:21 AM, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <[email protected]>
>>
>> Hello,
>>
>> This series contains an assorted set of fixes and improvements for
>> the Xen para-virtualized display driver and grant device driver which
>> I have collected over the last couple of months:
>>
>> 1. Minor fixes to grant device driver and drm/xen-front.
>>
>> 2. New format (YUYV) added to the list of the PV DRM supported formats
>> which allows the driver to be used in zero-copying use-cases when
>> a camera device is the source of the dma-bufs.
>>
>> 3. Synchronization with the latest para-virtualized protocol definition
>> in Xen [1].
>>
>> 4. SGT offset is now propagated to the backend: while importing a dmabuf
>> it is possible that the data of the buffer is put with offset which is
>> indicated by the SGT offset. This is needed for some GPUs which have
>> non-zero offset.
>>
>> Thank you,
>> Oleksandr Andrushchenko
>>
>> [1] https://urldefense.com/v3/__https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=c27a184225eab54d20435c8cab5ad0ef384dc2c0__;!!GF_29dbcQIUBPA!iAHOdk4M167VNM1AypMGVmyKJu-iqC9e5cXyu6N595Np3iyIZDnZl0MIBX3IROJSD1GSMX_GfQ$ [xenbits[.]xen[.]org]
>>
>> Changes since v1:
>> =================
>>
>> 1. Removed patch which adds EDID to PV DRM as it needs more time for review:
>> "5. Version 2 of the Xen displif protocol adds XENDISPL_OP_GET_EDID
>> request which allows frontends to request EDID structure per
>> connector. This request is optional and if not supported by the
>> backend then visible area is still defined by the relevant
>> XenStore's "resolution" property.
>> If backend provides EDID with XENDISPL_OP_GET_EDID request then
>> its values must take precedence over the resolutions defined in
>> XenStore."
>> I will send this as a dedicated patch.
>>
>> 2. Added missing CC stable for the patches with fixes
>>
>> Oleksandr Andrushchenko (5):
>> xen/gntdev: Fix dmabuf import with non-zero sgt offset
>> drm/xen-front: Fix misused IS_ERR_OR_NULL checks
>> drm/xen-front: Add YUYV to supported formats
>> xen: Sync up with the canonical protocol definition in Xen
>> drm/xen-front: Pass dumb buffer data offset to the backend
>>
>> drivers/gpu/drm/xen/xen_drm_front.c | 10 +--
>> drivers/gpu/drm/xen/xen_drm_front.h | 2 +-
>> drivers/gpu/drm/xen/xen_drm_front_conn.c | 1 +
>> drivers/gpu/drm/xen/xen_drm_front_gem.c | 11 +--
>> drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +-
>> drivers/xen/gntdev-dmabuf.c | 8 +++
>> include/xen/interface/io/displif.h | 91 +++++++++++++++++++++++-
>> 7 files changed, 111 insertions(+), 14 deletions(-)
>>
>

2020-08-13 07:24:04

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Fixes and improvements for Xen pvdrm


On 8/13/20 10:05 AM, Jürgen Groß wrote:
> On 13.08.20 08:32, Oleksandr Andrushchenko wrote:
>> Juergen, Boris,
>>
>> can we please merge these via Xen Linux tree as I have collected enough Ack/R-b?
>>
>> The series has DRM patches, but those anyway are Xen related, so I think
>>
>> this should be fine from DRI point of view.
>
> Yes, fine with me.
Great, thank you
>
>
> Juergen
>
>>
>> Thank you,
>>
>> Oleksandr
>>
>> On 8/13/20 9:21 AM, Oleksandr Andrushchenko wrote:
>>> From: Oleksandr Andrushchenko <[email protected]>
>>>
>>> Hello,
>>>
>>> This series contains an assorted set of fixes and improvements for
>>> the Xen para-virtualized display driver and grant device driver which
>>> I have collected over the last couple of months:
>>>
>>> 1. Minor fixes to grant device driver and drm/xen-front.
>>>
>>> 2. New format (YUYV) added to the list of the PV DRM supported formats
>>> which allows the driver to be used in zero-copying use-cases when
>>> a camera device is the source of the dma-bufs.
>>>
>>> 3. Synchronization with the latest para-virtualized protocol definition
>>> in Xen [1].
>>>
>>> 4. SGT offset is now propagated to the backend: while importing a dmabuf
>>> it is possible that the data of the buffer is put with offset which is
>>> indicated by the SGT offset. This is needed for some GPUs which have
>>> non-zero offset.
>>>
>>> Thank you,
>>> Oleksandr Andrushchenko
>>>
>>> [1] https://urldefense.com/v3/__https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=c27a184225eab54d20435c8cab5ad0ef384dc2c0__;!!GF_29dbcQIUBPA!iAHOdk4M167VNM1AypMGVmyKJu-iqC9e5cXyu6N595Np3iyIZDnZl0MIBX3IROJSD1GSMX_GfQ$ [xenbits[.]xen[.]org]
>>>
>>> Changes since v1:
>>> =================
>>>
>>> 1. Removed patch which adds EDID to PV DRM as it needs more time for review:
>>> "5. Version 2 of the Xen displif protocol adds XENDISPL_OP_GET_EDID
>>> request which allows frontends to request EDID structure per
>>> connector. This request is optional and if not supported by the
>>> backend then visible area is still defined by the relevant
>>> XenStore's "resolution" property.
>>> If backend provides EDID with XENDISPL_OP_GET_EDID request then
>>> its values must take precedence over the resolutions defined in
>>> XenStore."
>>> I will send this as a dedicated patch.
>>>
>>> 2. Added missing CC stable for the patches with fixes
>>>
>>> Oleksandr Andrushchenko (5):
>>>     xen/gntdev: Fix dmabuf import with non-zero sgt offset
>>>     drm/xen-front: Fix misused IS_ERR_OR_NULL checks
>>>     drm/xen-front: Add YUYV to supported formats
>>>     xen: Sync up with the canonical protocol definition in Xen
>>>     drm/xen-front: Pass dumb buffer data offset to the backend
>>>
>>>    drivers/gpu/drm/xen/xen_drm_front.c      | 10 +--
>>>    drivers/gpu/drm/xen/xen_drm_front.h      |  2 +-
>>>    drivers/gpu/drm/xen/xen_drm_front_conn.c |  1 +
>>>    drivers/gpu/drm/xen/xen_drm_front_gem.c  | 11 +--
>>>    drivers/gpu/drm/xen/xen_drm_front_kms.c  |  2 +-
>>>    drivers/xen/gntdev-dmabuf.c              |  8 +++
>>>    include/xen/interface/io/displif.h       | 91 +++++++++++++++++++++++-
>>>    7 files changed, 111 insertions(+), 14 deletions(-)
>>>
>>
>

2020-08-13 08:35:12

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Fixes and improvements for Xen pvdrm

Juergen, Boris,

can we please merge these via Xen Linux tree as I have collected enough Ack/R-b?

The series has DRM patches, but those anyway are Xen related, so I think

this should be fine from DRI point of view.

Thank you,

Oleksandr

On 8/13/20 9:21 AM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <[email protected]>
>
> Hello,
>
> This series contains an assorted set of fixes and improvements for
> the Xen para-virtualized display driver and grant device driver which
> I have collected over the last couple of months:
>
> 1. Minor fixes to grant device driver and drm/xen-front.
>
> 2. New format (YUYV) added to the list of the PV DRM supported formats
> which allows the driver to be used in zero-copying use-cases when
> a camera device is the source of the dma-bufs.
>
> 3. Synchronization with the latest para-virtualized protocol definition
> in Xen [1].
>
> 4. SGT offset is now propagated to the backend: while importing a dmabuf
> it is possible that the data of the buffer is put with offset which is
> indicated by the SGT offset. This is needed for some GPUs which have
> non-zero offset.
>
> Thank you,
> Oleksandr Andrushchenko
>
> [1] https://urldefense.com/v3/__https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=c27a184225eab54d20435c8cab5ad0ef384dc2c0__;!!GF_29dbcQIUBPA!iAHOdk4M167VNM1AypMGVmyKJu-iqC9e5cXyu6N595Np3iyIZDnZl0MIBX3IROJSD1GSMX_GfQ$ [xenbits[.]xen[.]org]
>
> Changes since v1:
> =================
>
> 1. Removed patch which adds EDID to PV DRM as it needs more time for review:
> "5. Version 2 of the Xen displif protocol adds XENDISPL_OP_GET_EDID
> request which allows frontends to request EDID structure per
> connector. This request is optional and if not supported by the
> backend then visible area is still defined by the relevant
> XenStore's "resolution" property.
> If backend provides EDID with XENDISPL_OP_GET_EDID request then
> its values must take precedence over the resolutions defined in
> XenStore."
> I will send this as a dedicated patch.
>
> 2. Added missing CC stable for the patches with fixes
>
> Oleksandr Andrushchenko (5):
> xen/gntdev: Fix dmabuf import with non-zero sgt offset
> drm/xen-front: Fix misused IS_ERR_OR_NULL checks
> drm/xen-front: Add YUYV to supported formats
> xen: Sync up with the canonical protocol definition in Xen
> drm/xen-front: Pass dumb buffer data offset to the backend
>
> drivers/gpu/drm/xen/xen_drm_front.c | 10 +--
> drivers/gpu/drm/xen/xen_drm_front.h | 2 +-
> drivers/gpu/drm/xen/xen_drm_front_conn.c | 1 +
> drivers/gpu/drm/xen/xen_drm_front_gem.c | 11 +--
> drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +-
> drivers/xen/gntdev-dmabuf.c | 8 +++
> include/xen/interface/io/displif.h | 91 +++++++++++++++++++++++-
> 7 files changed, 111 insertions(+), 14 deletions(-)
>

2020-08-13 15:06:04

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Fixes and improvements for Xen pvdrm

On 13.08.20 08:21, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <[email protected]>

Series pushed to:

xen/tip.git for-linus-5.9


Juergen

2020-08-13 15:12:37

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Fixes and improvements for Xen pvdrm


On 8/13/20 6:02 PM, Jürgen Groß wrote:
> On 13.08.20 08:21, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <[email protected]>
>
> Series pushed to:
>
> xen/tip.git for-linus-5.9
>
The top patch has strange title though:

"Subject: [PATCH v2 5/5] drm/xen-front: Pass dumb buffer data offset to the backend"

>
> Juergen

Thank you,

Oleksandr

2020-08-13 15:16:31

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Fixes and improvements for Xen pvdrm

On 13.08.20 17:10, Oleksandr Andrushchenko wrote:
>
> On 8/13/20 6:02 PM, Jürgen Groß wrote:
>> On 13.08.20 08:21, Oleksandr Andrushchenko wrote:
>>> From: Oleksandr Andrushchenko <[email protected]>
>>
>> Series pushed to:
>>
>> xen/tip.git for-linus-5.9
>>
> The top patch has strange title though:
>
> "Subject: [PATCH v2 5/5] drm/xen-front: Pass dumb buffer data offset to the backend"

Oh, indeed. I had to repair it manually as it seems some mail system
(probably on my end) mangled it a little bit.

Will repair it.


Juergen

2020-08-13 19:08:06

by Oleksandr Andrushchenko

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Fixes and improvements for Xen pvdrm


On 8/13/20 6:13 PM, Jürgen Groß wrote:
> On 13.08.20 17:10, Oleksandr Andrushchenko wrote:
>>
>> On 8/13/20 6:02 PM, Jürgen Groß wrote:
>>> On 13.08.20 08:21, Oleksandr Andrushchenko wrote:
>>>> From: Oleksandr Andrushchenko <[email protected]>
>>>
>>> Series pushed to:
>>>
>>> xen/tip.git for-linus-5.9
>>>
>> The top patch has strange title though:
>>
>> "Subject: [PATCH v2 5/5] drm/xen-front: Pass dumb buffer data offset to the backend"
>
> Oh, indeed. I had to repair it manually as it seems some mail system
> (probably on my end) mangled it a little bit.
>
> Will repair it.
>
Now everything is great,

Thank you!

>
> Juergen