Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-03-05 Thread Oleksandr Andrushchenko 

On 03/06/2018 09:26 AM, Daniel Vetter wrote:

On Mon, Mar 05, 2018 at 03:46:07PM +0200, Oleksandr Andrushchenko wrote:

On 03/05/2018 11:32 AM, Daniel Vetter wrote:

On Wed, Feb 21, 2018 at 10:03:41AM +0200, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

Implement GEM handling depending on driver mode of operation:
depending on the requirements for the para-virtualized environment, namely
requirements dictated by the accompanying DRM/(v)GPU drivers running in both
host and guest environments, number of operating modes of para-virtualized
display driver are supported:
   - display buffers can be allocated by either frontend driver or backend
   - display buffers can be allocated to be contiguous in memory or not

Note! Frontend driver itself has no dependency on contiguous memory for
its operation.

1. Buffers allocated by the frontend driver.

The below modes of operation are configured at compile-time via
frontend driver's kernel configuration.

1.1. Front driver configured to use GEM CMA helpers
   This use-case is useful when used with accompanying DRM/vGPU driver in
   guest domain which was designed to only work with contiguous buffers,
   e.g. DRM driver based on GEM CMA helpers: such drivers can only import
   contiguous PRIME buffers, thus requiring frontend driver to provide
   such. In order to implement this mode of operation para-virtualized
   frontend driver can be configured to use GEM CMA helpers.

1.2. Front driver doesn't use GEM CMA
   If accompanying drivers can cope with non-contiguous memory then, to
   lower pressure on CMA subsystem of the kernel, driver can allocate
   buffers from system memory.

Note! If used with accompanying DRM/(v)GPU drivers this mode of operation
may require IOMMU support on the platform, so accompanying DRM/vGPU
hardware can still reach display buffer memory while importing PRIME
buffers from the frontend driver.

2. Buffers allocated by the backend

This mode of operation is run-time configured via guest domain configuration
through XenStore entries.

For systems which do not provide IOMMU support, but having specific
requirements for display buffers it is possible to allocate such buffers
at backend side and share those with the frontend.
For example, if host domain is 1:1 mapped and has DRM/GPU hardware expecting
physically contiguous memory, this allows implementing zero-copying
use-cases.

Note! Configuration options 1.1 (contiguous display buffers) and 2 (backend
allocated buffers) are not supported at the same time.

Signed-off-by: Oleksandr Andrushchenko 

Some suggestions below for some larger cleanup work.
-Daniel


---
   drivers/gpu/drm/xen/Kconfig |  13 +
   drivers/gpu/drm/xen/Makefile|   6 +
   drivers/gpu/drm/xen/xen_drm_front.h |  74 ++
   drivers/gpu/drm/xen/xen_drm_front_drv.c |  80 ++-
   drivers/gpu/drm/xen/xen_drm_front_drv.h |   1 +
   drivers/gpu/drm/xen/xen_drm_front_gem.c | 360 

   drivers/gpu/drm/xen/xen_drm_front_gem.h |  46 
   drivers/gpu/drm/xen/xen_drm_front_gem_cma.c |  93 +++
   8 files changed, 667 insertions(+), 6 deletions(-)
   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.c
   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.h
   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c

diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
index 4cca160782ab..4f4abc91f3b6 100644
--- a/drivers/gpu/drm/xen/Kconfig
+++ b/drivers/gpu/drm/xen/Kconfig
@@ -15,3 +15,16 @@ config DRM_XEN_FRONTEND
help
  Choose this option if you want to enable a para-virtualized
  frontend DRM/KMS driver for Xen guest OSes.
+
+config DRM_XEN_FRONTEND_CMA
+   bool "Use DRM CMA to allocate dumb buffers"
+   depends on DRM_XEN_FRONTEND
+   select DRM_KMS_CMA_HELPER
+   select DRM_GEM_CMA_HELPER
+   help
+ Use DRM CMA helpers to allocate display buffers.
+ This is useful for the use-cases when guest driver needs to
+ share or export buffers to other drivers which only expect
+ contiguous buffers.
+ Note: in this mode driver cannot use buffers allocated
+ by the backend.
diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
index 4fcb0da1a9c5..12376ec78fbc 100644
--- a/drivers/gpu/drm/xen/Makefile
+++ b/drivers/gpu/drm/xen/Makefile
@@ -8,4 +8,10 @@ drm_xen_front-objs := xen_drm_front.o \
  xen_drm_front_shbuf.o \
  xen_drm_front_cfg.o
+ifeq ($(CONFIG_DRM_XEN_FRONTEND_CMA),y)
+   drm_xen_front-objs += xen_drm_front_gem_cma.o
+else
+   drm_xen_front-objs += xen_drm_front_gem.o
+endif
+
   obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o
diff --git a/drivers/gpu/drm/xen/xen_drm_front.h 
b/drivers/gpu/drm/xen/xen_drm_front.h
index

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-03-05 Thread Oleksandr Andrushchenko 

On 03/06/2018 09:26 AM, Daniel Vetter wrote:

On Mon, Mar 05, 2018 at 03:46:07PM +0200, Oleksandr Andrushchenko wrote:

On 03/05/2018 11:32 AM, Daniel Vetter wrote:

On Wed, Feb 21, 2018 at 10:03:41AM +0200, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

Implement GEM handling depending on driver mode of operation:
depending on the requirements for the para-virtualized environment, namely
requirements dictated by the accompanying DRM/(v)GPU drivers running in both
host and guest environments, number of operating modes of para-virtualized
display driver are supported:
   - display buffers can be allocated by either frontend driver or backend
   - display buffers can be allocated to be contiguous in memory or not

Note! Frontend driver itself has no dependency on contiguous memory for
its operation.

1. Buffers allocated by the frontend driver.

The below modes of operation are configured at compile-time via
frontend driver's kernel configuration.

1.1. Front driver configured to use GEM CMA helpers
   This use-case is useful when used with accompanying DRM/vGPU driver in
   guest domain which was designed to only work with contiguous buffers,
   e.g. DRM driver based on GEM CMA helpers: such drivers can only import
   contiguous PRIME buffers, thus requiring frontend driver to provide
   such. In order to implement this mode of operation para-virtualized
   frontend driver can be configured to use GEM CMA helpers.

1.2. Front driver doesn't use GEM CMA
   If accompanying drivers can cope with non-contiguous memory then, to
   lower pressure on CMA subsystem of the kernel, driver can allocate
   buffers from system memory.

Note! If used with accompanying DRM/(v)GPU drivers this mode of operation
may require IOMMU support on the platform, so accompanying DRM/vGPU
hardware can still reach display buffer memory while importing PRIME
buffers from the frontend driver.

2. Buffers allocated by the backend

This mode of operation is run-time configured via guest domain configuration
through XenStore entries.

For systems which do not provide IOMMU support, but having specific
requirements for display buffers it is possible to allocate such buffers
at backend side and share those with the frontend.
For example, if host domain is 1:1 mapped and has DRM/GPU hardware expecting
physically contiguous memory, this allows implementing zero-copying
use-cases.

Note! Configuration options 1.1 (contiguous display buffers) and 2 (backend
allocated buffers) are not supported at the same time.

Signed-off-by: Oleksandr Andrushchenko 

Some suggestions below for some larger cleanup work.
-Daniel


---
   drivers/gpu/drm/xen/Kconfig |  13 +
   drivers/gpu/drm/xen/Makefile|   6 +
   drivers/gpu/drm/xen/xen_drm_front.h |  74 ++
   drivers/gpu/drm/xen/xen_drm_front_drv.c |  80 ++-
   drivers/gpu/drm/xen/xen_drm_front_drv.h |   1 +
   drivers/gpu/drm/xen/xen_drm_front_gem.c | 360 

   drivers/gpu/drm/xen/xen_drm_front_gem.h |  46 
   drivers/gpu/drm/xen/xen_drm_front_gem_cma.c |  93 +++
   8 files changed, 667 insertions(+), 6 deletions(-)
   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.c
   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.h
   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c

diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
index 4cca160782ab..4f4abc91f3b6 100644
--- a/drivers/gpu/drm/xen/Kconfig
+++ b/drivers/gpu/drm/xen/Kconfig
@@ -15,3 +15,16 @@ config DRM_XEN_FRONTEND
help
  Choose this option if you want to enable a para-virtualized
  frontend DRM/KMS driver for Xen guest OSes.
+
+config DRM_XEN_FRONTEND_CMA
+   bool "Use DRM CMA to allocate dumb buffers"
+   depends on DRM_XEN_FRONTEND
+   select DRM_KMS_CMA_HELPER
+   select DRM_GEM_CMA_HELPER
+   help
+ Use DRM CMA helpers to allocate display buffers.
+ This is useful for the use-cases when guest driver needs to
+ share or export buffers to other drivers which only expect
+ contiguous buffers.
+ Note: in this mode driver cannot use buffers allocated
+ by the backend.
diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
index 4fcb0da1a9c5..12376ec78fbc 100644
--- a/drivers/gpu/drm/xen/Makefile
+++ b/drivers/gpu/drm/xen/Makefile
@@ -8,4 +8,10 @@ drm_xen_front-objs := xen_drm_front.o \
  xen_drm_front_shbuf.o \
  xen_drm_front_cfg.o
+ifeq ($(CONFIG_DRM_XEN_FRONTEND_CMA),y)
+   drm_xen_front-objs += xen_drm_front_gem_cma.o
+else
+   drm_xen_front-objs += xen_drm_front_gem.o
+endif
+
   obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o
diff --git a/drivers/gpu/drm/xen/xen_drm_front.h 
b/drivers/gpu/drm/xen/xen_drm_front.h
index 9ed5bfb248d0..c6f52c892434 100644
---

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-03-05 Thread Daniel Vetter 
On Mon, Mar 05, 2018 at 03:46:07PM +0200, Oleksandr Andrushchenko wrote:
> On 03/05/2018 11:32 AM, Daniel Vetter wrote:
> > On Wed, Feb 21, 2018 at 10:03:41AM +0200, Oleksandr Andrushchenko wrote:
> > > From: Oleksandr Andrushchenko 
> > > 
> > > Implement GEM handling depending on driver mode of operation:
> > > depending on the requirements for the para-virtualized environment, namely
> > > requirements dictated by the accompanying DRM/(v)GPU drivers running in 
> > > both
> > > host and guest environments, number of operating modes of para-virtualized
> > > display driver are supported:
> > >   - display buffers can be allocated by either frontend driver or backend
> > >   - display buffers can be allocated to be contiguous in memory or not
> > > 
> > > Note! Frontend driver itself has no dependency on contiguous memory for
> > > its operation.
> > > 
> > > 1. Buffers allocated by the frontend driver.
> > > 
> > > The below modes of operation are configured at compile-time via
> > > frontend driver's kernel configuration.
> > > 
> > > 1.1. Front driver configured to use GEM CMA helpers
> > >   This use-case is useful when used with accompanying DRM/vGPU driver 
> > > in
> > >   guest domain which was designed to only work with contiguous 
> > > buffers,
> > >   e.g. DRM driver based on GEM CMA helpers: such drivers can only 
> > > import
> > >   contiguous PRIME buffers, thus requiring frontend driver to provide
> > >   such. In order to implement this mode of operation para-virtualized
> > >   frontend driver can be configured to use GEM CMA helpers.
> > > 
> > > 1.2. Front driver doesn't use GEM CMA
> > >   If accompanying drivers can cope with non-contiguous memory then, to
> > >   lower pressure on CMA subsystem of the kernel, driver can allocate
> > >   buffers from system memory.
> > > 
> > > Note! If used with accompanying DRM/(v)GPU drivers this mode of operation
> > > may require IOMMU support on the platform, so accompanying DRM/vGPU
> > > hardware can still reach display buffer memory while importing PRIME
> > > buffers from the frontend driver.
> > > 
> > > 2. Buffers allocated by the backend
> > > 
> > > This mode of operation is run-time configured via guest domain 
> > > configuration
> > > through XenStore entries.
> > > 
> > > For systems which do not provide IOMMU support, but having specific
> > > requirements for display buffers it is possible to allocate such buffers
> > > at backend side and share those with the frontend.
> > > For example, if host domain is 1:1 mapped and has DRM/GPU hardware 
> > > expecting
> > > physically contiguous memory, this allows implementing zero-copying
> > > use-cases.
> > > 
> > > Note! Configuration options 1.1 (contiguous display buffers) and 2 
> > > (backend
> > > allocated buffers) are not supported at the same time.
> > > 
> > > Signed-off-by: Oleksandr Andrushchenko 
> > Some suggestions below for some larger cleanup work.
> > -Daniel
> > 
> > > ---
> > >   drivers/gpu/drm/xen/Kconfig |  13 +
> > >   drivers/gpu/drm/xen/Makefile|   6 +
> > >   drivers/gpu/drm/xen/xen_drm_front.h |  74 ++
> > >   drivers/gpu/drm/xen/xen_drm_front_drv.c |  80 ++-
> > >   drivers/gpu/drm/xen/xen_drm_front_drv.h |   1 +
> > >   drivers/gpu/drm/xen/xen_drm_front_gem.c | 360 
> > > 
> > >   drivers/gpu/drm/xen/xen_drm_front_gem.h |  46 
> > >   drivers/gpu/drm/xen/xen_drm_front_gem_cma.c |  93 +++
> > >   8 files changed, 667 insertions(+), 6 deletions(-)
> > >   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.c
> > >   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.h
> > >   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c
> > > 
> > > diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
> > > index 4cca160782ab..4f4abc91f3b6 100644
> > > --- a/drivers/gpu/drm/xen/Kconfig
> > > +++ b/drivers/gpu/drm/xen/Kconfig
> > > @@ -15,3 +15,16 @@ config DRM_XEN_FRONTEND
> > >   help
> > > Choose this option if you want to enable a para-virtualized
> > > frontend DRM/KMS driver for Xen guest OSes.
> > > +
> > > +config DRM_XEN_FRONTEND_CMA
> > > + bool "Use DRM CMA to allocate dumb buffers"
> > > + depends on DRM_XEN_FRONTEND
> > > + select DRM_KMS_CMA_HELPER
> > > + select DRM_GEM_CMA_HELPER
> > > + help
> > > +   Use DRM CMA helpers to allocate display buffers.
> > > +   This is useful for the use-cases when guest driver needs to
> > > +   share or export buffers to other drivers which only expect
> > > +   contiguous buffers.
> > > +   Note: in this mode driver cannot use buffers allocated
> > > +   by the backend.
> > > diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
> > > index 4fcb0da1a9c5..12376ec78fbc 100644
> > > --- a/drivers/gpu/drm/xen/Makefile
> > > +++

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-03-05 Thread Daniel Vetter 
On Mon, Mar 05, 2018 at 03:46:07PM +0200, Oleksandr Andrushchenko wrote:
> On 03/05/2018 11:32 AM, Daniel Vetter wrote:
> > On Wed, Feb 21, 2018 at 10:03:41AM +0200, Oleksandr Andrushchenko wrote:
> > > From: Oleksandr Andrushchenko 
> > > 
> > > Implement GEM handling depending on driver mode of operation:
> > > depending on the requirements for the para-virtualized environment, namely
> > > requirements dictated by the accompanying DRM/(v)GPU drivers running in 
> > > both
> > > host and guest environments, number of operating modes of para-virtualized
> > > display driver are supported:
> > >   - display buffers can be allocated by either frontend driver or backend
> > >   - display buffers can be allocated to be contiguous in memory or not
> > > 
> > > Note! Frontend driver itself has no dependency on contiguous memory for
> > > its operation.
> > > 
> > > 1. Buffers allocated by the frontend driver.
> > > 
> > > The below modes of operation are configured at compile-time via
> > > frontend driver's kernel configuration.
> > > 
> > > 1.1. Front driver configured to use GEM CMA helpers
> > >   This use-case is useful when used with accompanying DRM/vGPU driver 
> > > in
> > >   guest domain which was designed to only work with contiguous 
> > > buffers,
> > >   e.g. DRM driver based on GEM CMA helpers: such drivers can only 
> > > import
> > >   contiguous PRIME buffers, thus requiring frontend driver to provide
> > >   such. In order to implement this mode of operation para-virtualized
> > >   frontend driver can be configured to use GEM CMA helpers.
> > > 
> > > 1.2. Front driver doesn't use GEM CMA
> > >   If accompanying drivers can cope with non-contiguous memory then, to
> > >   lower pressure on CMA subsystem of the kernel, driver can allocate
> > >   buffers from system memory.
> > > 
> > > Note! If used with accompanying DRM/(v)GPU drivers this mode of operation
> > > may require IOMMU support on the platform, so accompanying DRM/vGPU
> > > hardware can still reach display buffer memory while importing PRIME
> > > buffers from the frontend driver.
> > > 
> > > 2. Buffers allocated by the backend
> > > 
> > > This mode of operation is run-time configured via guest domain 
> > > configuration
> > > through XenStore entries.
> > > 
> > > For systems which do not provide IOMMU support, but having specific
> > > requirements for display buffers it is possible to allocate such buffers
> > > at backend side and share those with the frontend.
> > > For example, if host domain is 1:1 mapped and has DRM/GPU hardware 
> > > expecting
> > > physically contiguous memory, this allows implementing zero-copying
> > > use-cases.
> > > 
> > > Note! Configuration options 1.1 (contiguous display buffers) and 2 
> > > (backend
> > > allocated buffers) are not supported at the same time.
> > > 
> > > Signed-off-by: Oleksandr Andrushchenko 
> > Some suggestions below for some larger cleanup work.
> > -Daniel
> > 
> > > ---
> > >   drivers/gpu/drm/xen/Kconfig |  13 +
> > >   drivers/gpu/drm/xen/Makefile|   6 +
> > >   drivers/gpu/drm/xen/xen_drm_front.h |  74 ++
> > >   drivers/gpu/drm/xen/xen_drm_front_drv.c |  80 ++-
> > >   drivers/gpu/drm/xen/xen_drm_front_drv.h |   1 +
> > >   drivers/gpu/drm/xen/xen_drm_front_gem.c | 360 
> > > 
> > >   drivers/gpu/drm/xen/xen_drm_front_gem.h |  46 
> > >   drivers/gpu/drm/xen/xen_drm_front_gem_cma.c |  93 +++
> > >   8 files changed, 667 insertions(+), 6 deletions(-)
> > >   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.c
> > >   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.h
> > >   create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c
> > > 
> > > diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
> > > index 4cca160782ab..4f4abc91f3b6 100644
> > > --- a/drivers/gpu/drm/xen/Kconfig
> > > +++ b/drivers/gpu/drm/xen/Kconfig
> > > @@ -15,3 +15,16 @@ config DRM_XEN_FRONTEND
> > >   help
> > > Choose this option if you want to enable a para-virtualized
> > > frontend DRM/KMS driver for Xen guest OSes.
> > > +
> > > +config DRM_XEN_FRONTEND_CMA
> > > + bool "Use DRM CMA to allocate dumb buffers"
> > > + depends on DRM_XEN_FRONTEND
> > > + select DRM_KMS_CMA_HELPER
> > > + select DRM_GEM_CMA_HELPER
> > > + help
> > > +   Use DRM CMA helpers to allocate display buffers.
> > > +   This is useful for the use-cases when guest driver needs to
> > > +   share or export buffers to other drivers which only expect
> > > +   contiguous buffers.
> > > +   Note: in this mode driver cannot use buffers allocated
> > > +   by the backend.
> > > diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
> > > index 4fcb0da1a9c5..12376ec78fbc 100644
> > > --- a/drivers/gpu/drm/xen/Makefile
> > > +++ b/drivers/gpu/drm/xen/Makefile
> > > @@ -8,4 +8,10 @@

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-03-05 Thread Oleksandr Andrushchenko 

On 03/05/2018 11:32 AM, Daniel Vetter wrote:

On Wed, Feb 21, 2018 at 10:03:41AM +0200, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

Implement GEM handling depending on driver mode of operation:
depending on the requirements for the para-virtualized environment, namely
requirements dictated by the accompanying DRM/(v)GPU drivers running in both
host and guest environments, number of operating modes of para-virtualized
display driver are supported:
  - display buffers can be allocated by either frontend driver or backend
  - display buffers can be allocated to be contiguous in memory or not

Note! Frontend driver itself has no dependency on contiguous memory for
its operation.

1. Buffers allocated by the frontend driver.

The below modes of operation are configured at compile-time via
frontend driver's kernel configuration.

1.1. Front driver configured to use GEM CMA helpers
  This use-case is useful when used with accompanying DRM/vGPU driver in
  guest domain which was designed to only work with contiguous buffers,
  e.g. DRM driver based on GEM CMA helpers: such drivers can only import
  contiguous PRIME buffers, thus requiring frontend driver to provide
  such. In order to implement this mode of operation para-virtualized
  frontend driver can be configured to use GEM CMA helpers.

1.2. Front driver doesn't use GEM CMA
  If accompanying drivers can cope with non-contiguous memory then, to
  lower pressure on CMA subsystem of the kernel, driver can allocate
  buffers from system memory.

Note! If used with accompanying DRM/(v)GPU drivers this mode of operation
may require IOMMU support on the platform, so accompanying DRM/vGPU
hardware can still reach display buffer memory while importing PRIME
buffers from the frontend driver.

2. Buffers allocated by the backend

This mode of operation is run-time configured via guest domain configuration
through XenStore entries.

For systems which do not provide IOMMU support, but having specific
requirements for display buffers it is possible to allocate such buffers
at backend side and share those with the frontend.
For example, if host domain is 1:1 mapped and has DRM/GPU hardware expecting
physically contiguous memory, this allows implementing zero-copying
use-cases.

Note! Configuration options 1.1 (contiguous display buffers) and 2 (backend
allocated buffers) are not supported at the same time.

Signed-off-by: Oleksandr Andrushchenko 

Some suggestions below for some larger cleanup work.
-Daniel


---
  drivers/gpu/drm/xen/Kconfig |  13 +
  drivers/gpu/drm/xen/Makefile|   6 +
  drivers/gpu/drm/xen/xen_drm_front.h |  74 ++
  drivers/gpu/drm/xen/xen_drm_front_drv.c |  80 ++-
  drivers/gpu/drm/xen/xen_drm_front_drv.h |   1 +
  drivers/gpu/drm/xen/xen_drm_front_gem.c | 360 
  drivers/gpu/drm/xen/xen_drm_front_gem.h |  46 
  drivers/gpu/drm/xen/xen_drm_front_gem_cma.c |  93 +++
  8 files changed, 667 insertions(+), 6 deletions(-)
  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.c
  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.h
  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c

diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
index 4cca160782ab..4f4abc91f3b6 100644
--- a/drivers/gpu/drm/xen/Kconfig
+++ b/drivers/gpu/drm/xen/Kconfig
@@ -15,3 +15,16 @@ config DRM_XEN_FRONTEND
help
  Choose this option if you want to enable a para-virtualized
  frontend DRM/KMS driver for Xen guest OSes.
+
+config DRM_XEN_FRONTEND_CMA
+   bool "Use DRM CMA to allocate dumb buffers"
+   depends on DRM_XEN_FRONTEND
+   select DRM_KMS_CMA_HELPER
+   select DRM_GEM_CMA_HELPER
+   help
+ Use DRM CMA helpers to allocate display buffers.
+ This is useful for the use-cases when guest driver needs to
+ share or export buffers to other drivers which only expect
+ contiguous buffers.
+ Note: in this mode driver cannot use buffers allocated
+ by the backend.
diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
index 4fcb0da1a9c5..12376ec78fbc 100644
--- a/drivers/gpu/drm/xen/Makefile
+++ b/drivers/gpu/drm/xen/Makefile
@@ -8,4 +8,10 @@ drm_xen_front-objs := xen_drm_front.o \
  xen_drm_front_shbuf.o \
  xen_drm_front_cfg.o
  
+ifeq ($(CONFIG_DRM_XEN_FRONTEND_CMA),y)

+   drm_xen_front-objs += xen_drm_front_gem_cma.o
+else
+   drm_xen_front-objs += xen_drm_front_gem.o
+endif
+
  obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o
diff --git a/drivers/gpu/drm/xen/xen_drm_front.h 
b/drivers/gpu/drm/xen/xen_drm_front.h
index 9ed5bfb248d0..c6f52c892434 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.h
+++ b/drivers/gpu/drm/xen/xen_drm_front.h
@@ -34,6 +34,80 @@

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-03-05 Thread Oleksandr Andrushchenko 

On 03/05/2018 11:32 AM, Daniel Vetter wrote:

On Wed, Feb 21, 2018 at 10:03:41AM +0200, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

Implement GEM handling depending on driver mode of operation:
depending on the requirements for the para-virtualized environment, namely
requirements dictated by the accompanying DRM/(v)GPU drivers running in both
host and guest environments, number of operating modes of para-virtualized
display driver are supported:
  - display buffers can be allocated by either frontend driver or backend
  - display buffers can be allocated to be contiguous in memory or not

Note! Frontend driver itself has no dependency on contiguous memory for
its operation.

1. Buffers allocated by the frontend driver.

The below modes of operation are configured at compile-time via
frontend driver's kernel configuration.

1.1. Front driver configured to use GEM CMA helpers
  This use-case is useful when used with accompanying DRM/vGPU driver in
  guest domain which was designed to only work with contiguous buffers,
  e.g. DRM driver based on GEM CMA helpers: such drivers can only import
  contiguous PRIME buffers, thus requiring frontend driver to provide
  such. In order to implement this mode of operation para-virtualized
  frontend driver can be configured to use GEM CMA helpers.

1.2. Front driver doesn't use GEM CMA
  If accompanying drivers can cope with non-contiguous memory then, to
  lower pressure on CMA subsystem of the kernel, driver can allocate
  buffers from system memory.

Note! If used with accompanying DRM/(v)GPU drivers this mode of operation
may require IOMMU support on the platform, so accompanying DRM/vGPU
hardware can still reach display buffer memory while importing PRIME
buffers from the frontend driver.

2. Buffers allocated by the backend

This mode of operation is run-time configured via guest domain configuration
through XenStore entries.

For systems which do not provide IOMMU support, but having specific
requirements for display buffers it is possible to allocate such buffers
at backend side and share those with the frontend.
For example, if host domain is 1:1 mapped and has DRM/GPU hardware expecting
physically contiguous memory, this allows implementing zero-copying
use-cases.

Note! Configuration options 1.1 (contiguous display buffers) and 2 (backend
allocated buffers) are not supported at the same time.

Signed-off-by: Oleksandr Andrushchenko 

Some suggestions below for some larger cleanup work.
-Daniel


---
  drivers/gpu/drm/xen/Kconfig |  13 +
  drivers/gpu/drm/xen/Makefile|   6 +
  drivers/gpu/drm/xen/xen_drm_front.h |  74 ++
  drivers/gpu/drm/xen/xen_drm_front_drv.c |  80 ++-
  drivers/gpu/drm/xen/xen_drm_front_drv.h |   1 +
  drivers/gpu/drm/xen/xen_drm_front_gem.c | 360 
  drivers/gpu/drm/xen/xen_drm_front_gem.h |  46 
  drivers/gpu/drm/xen/xen_drm_front_gem_cma.c |  93 +++
  8 files changed, 667 insertions(+), 6 deletions(-)
  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.c
  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.h
  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c

diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
index 4cca160782ab..4f4abc91f3b6 100644
--- a/drivers/gpu/drm/xen/Kconfig
+++ b/drivers/gpu/drm/xen/Kconfig
@@ -15,3 +15,16 @@ config DRM_XEN_FRONTEND
help
  Choose this option if you want to enable a para-virtualized
  frontend DRM/KMS driver for Xen guest OSes.
+
+config DRM_XEN_FRONTEND_CMA
+   bool "Use DRM CMA to allocate dumb buffers"
+   depends on DRM_XEN_FRONTEND
+   select DRM_KMS_CMA_HELPER
+   select DRM_GEM_CMA_HELPER
+   help
+ Use DRM CMA helpers to allocate display buffers.
+ This is useful for the use-cases when guest driver needs to
+ share or export buffers to other drivers which only expect
+ contiguous buffers.
+ Note: in this mode driver cannot use buffers allocated
+ by the backend.
diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
index 4fcb0da1a9c5..12376ec78fbc 100644
--- a/drivers/gpu/drm/xen/Makefile
+++ b/drivers/gpu/drm/xen/Makefile
@@ -8,4 +8,10 @@ drm_xen_front-objs := xen_drm_front.o \
  xen_drm_front_shbuf.o \
  xen_drm_front_cfg.o
  
+ifeq ($(CONFIG_DRM_XEN_FRONTEND_CMA),y)

+   drm_xen_front-objs += xen_drm_front_gem_cma.o
+else
+   drm_xen_front-objs += xen_drm_front_gem.o
+endif
+
  obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o
diff --git a/drivers/gpu/drm/xen/xen_drm_front.h 
b/drivers/gpu/drm/xen/xen_drm_front.h
index 9ed5bfb248d0..c6f52c892434 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.h
+++ b/drivers/gpu/drm/xen/xen_drm_front.h
@@ -34,6 +34,80 @@
  
  struct xen_drm_front_drm_pipeline;
  
+/*

+

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-03-05 Thread Daniel Vetter 
On Wed, Feb 21, 2018 at 10:03:41AM +0200, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Implement GEM handling depending on driver mode of operation:
> depending on the requirements for the para-virtualized environment, namely
> requirements dictated by the accompanying DRM/(v)GPU drivers running in both
> host and guest environments, number of operating modes of para-virtualized
> display driver are supported:
>  - display buffers can be allocated by either frontend driver or backend
>  - display buffers can be allocated to be contiguous in memory or not
> 
> Note! Frontend driver itself has no dependency on contiguous memory for
> its operation.
> 
> 1. Buffers allocated by the frontend driver.
> 
> The below modes of operation are configured at compile-time via
> frontend driver's kernel configuration.
> 
> 1.1. Front driver configured to use GEM CMA helpers
>  This use-case is useful when used with accompanying DRM/vGPU driver in
>  guest domain which was designed to only work with contiguous buffers,
>  e.g. DRM driver based on GEM CMA helpers: such drivers can only import
>  contiguous PRIME buffers, thus requiring frontend driver to provide
>  such. In order to implement this mode of operation para-virtualized
>  frontend driver can be configured to use GEM CMA helpers.
> 
> 1.2. Front driver doesn't use GEM CMA
>  If accompanying drivers can cope with non-contiguous memory then, to
>  lower pressure on CMA subsystem of the kernel, driver can allocate
>  buffers from system memory.
> 
> Note! If used with accompanying DRM/(v)GPU drivers this mode of operation
> may require IOMMU support on the platform, so accompanying DRM/vGPU
> hardware can still reach display buffer memory while importing PRIME
> buffers from the frontend driver.
> 
> 2. Buffers allocated by the backend
> 
> This mode of operation is run-time configured via guest domain configuration
> through XenStore entries.
> 
> For systems which do not provide IOMMU support, but having specific
> requirements for display buffers it is possible to allocate such buffers
> at backend side and share those with the frontend.
> For example, if host domain is 1:1 mapped and has DRM/GPU hardware expecting
> physically contiguous memory, this allows implementing zero-copying
> use-cases.
> 
> Note! Configuration options 1.1 (contiguous display buffers) and 2 (backend
> allocated buffers) are not supported at the same time.
> 
> Signed-off-by: Oleksandr Andrushchenko 

Some suggestions below for some larger cleanup work.
-Daniel

> ---
>  drivers/gpu/drm/xen/Kconfig |  13 +
>  drivers/gpu/drm/xen/Makefile|   6 +
>  drivers/gpu/drm/xen/xen_drm_front.h |  74 ++
>  drivers/gpu/drm/xen/xen_drm_front_drv.c |  80 ++-
>  drivers/gpu/drm/xen/xen_drm_front_drv.h |   1 +
>  drivers/gpu/drm/xen/xen_drm_front_gem.c | 360 
> 
>  drivers/gpu/drm/xen/xen_drm_front_gem.h |  46 
>  drivers/gpu/drm/xen/xen_drm_front_gem_cma.c |  93 +++
>  8 files changed, 667 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.c
>  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.h
>  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c
> 
> diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
> index 4cca160782ab..4f4abc91f3b6 100644
> --- a/drivers/gpu/drm/xen/Kconfig
> +++ b/drivers/gpu/drm/xen/Kconfig
> @@ -15,3 +15,16 @@ config DRM_XEN_FRONTEND
>   help
> Choose this option if you want to enable a para-virtualized
> frontend DRM/KMS driver for Xen guest OSes.
> +
> +config DRM_XEN_FRONTEND_CMA
> + bool "Use DRM CMA to allocate dumb buffers"
> + depends on DRM_XEN_FRONTEND
> + select DRM_KMS_CMA_HELPER
> + select DRM_GEM_CMA_HELPER
> + help
> +   Use DRM CMA helpers to allocate display buffers.
> +   This is useful for the use-cases when guest driver needs to
> +   share or export buffers to other drivers which only expect
> +   contiguous buffers.
> +   Note: in this mode driver cannot use buffers allocated
> +   by the backend.
> diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
> index 4fcb0da1a9c5..12376ec78fbc 100644
> --- a/drivers/gpu/drm/xen/Makefile
> +++ b/drivers/gpu/drm/xen/Makefile
> @@ -8,4 +8,10 @@ drm_xen_front-objs := xen_drm_front.o \
> xen_drm_front_shbuf.o \
> xen_drm_front_cfg.o
>  
> +ifeq ($(CONFIG_DRM_XEN_FRONTEND_CMA),y)
> + drm_xen_front-objs += xen_drm_front_gem_cma.o
> +else
> + drm_xen_front-objs += xen_drm_front_gem.o
> +endif
> +
>  obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o
> diff --git a/drivers/gpu/drm/xen/xen_drm_front.h 
> b/drivers/gpu/drm/xen/xen_drm_front.h
> index 9ed5bfb248d0..c6f52c892434 100644
> ---

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-03-05 Thread Daniel Vetter 
On Wed, Feb 21, 2018 at 10:03:41AM +0200, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Implement GEM handling depending on driver mode of operation:
> depending on the requirements for the para-virtualized environment, namely
> requirements dictated by the accompanying DRM/(v)GPU drivers running in both
> host and guest environments, number of operating modes of para-virtualized
> display driver are supported:
>  - display buffers can be allocated by either frontend driver or backend
>  - display buffers can be allocated to be contiguous in memory or not
> 
> Note! Frontend driver itself has no dependency on contiguous memory for
> its operation.
> 
> 1. Buffers allocated by the frontend driver.
> 
> The below modes of operation are configured at compile-time via
> frontend driver's kernel configuration.
> 
> 1.1. Front driver configured to use GEM CMA helpers
>  This use-case is useful when used with accompanying DRM/vGPU driver in
>  guest domain which was designed to only work with contiguous buffers,
>  e.g. DRM driver based on GEM CMA helpers: such drivers can only import
>  contiguous PRIME buffers, thus requiring frontend driver to provide
>  such. In order to implement this mode of operation para-virtualized
>  frontend driver can be configured to use GEM CMA helpers.
> 
> 1.2. Front driver doesn't use GEM CMA
>  If accompanying drivers can cope with non-contiguous memory then, to
>  lower pressure on CMA subsystem of the kernel, driver can allocate
>  buffers from system memory.
> 
> Note! If used with accompanying DRM/(v)GPU drivers this mode of operation
> may require IOMMU support on the platform, so accompanying DRM/vGPU
> hardware can still reach display buffer memory while importing PRIME
> buffers from the frontend driver.
> 
> 2. Buffers allocated by the backend
> 
> This mode of operation is run-time configured via guest domain configuration
> through XenStore entries.
> 
> For systems which do not provide IOMMU support, but having specific
> requirements for display buffers it is possible to allocate such buffers
> at backend side and share those with the frontend.
> For example, if host domain is 1:1 mapped and has DRM/GPU hardware expecting
> physically contiguous memory, this allows implementing zero-copying
> use-cases.
> 
> Note! Configuration options 1.1 (contiguous display buffers) and 2 (backend
> allocated buffers) are not supported at the same time.
> 
> Signed-off-by: Oleksandr Andrushchenko 

Some suggestions below for some larger cleanup work.
-Daniel

> ---
>  drivers/gpu/drm/xen/Kconfig |  13 +
>  drivers/gpu/drm/xen/Makefile|   6 +
>  drivers/gpu/drm/xen/xen_drm_front.h |  74 ++
>  drivers/gpu/drm/xen/xen_drm_front_drv.c |  80 ++-
>  drivers/gpu/drm/xen/xen_drm_front_drv.h |   1 +
>  drivers/gpu/drm/xen/xen_drm_front_gem.c | 360 
> 
>  drivers/gpu/drm/xen/xen_drm_front_gem.h |  46 
>  drivers/gpu/drm/xen/xen_drm_front_gem_cma.c |  93 +++
>  8 files changed, 667 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.c
>  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem.h
>  create mode 100644 drivers/gpu/drm/xen/xen_drm_front_gem_cma.c
> 
> diff --git a/drivers/gpu/drm/xen/Kconfig b/drivers/gpu/drm/xen/Kconfig
> index 4cca160782ab..4f4abc91f3b6 100644
> --- a/drivers/gpu/drm/xen/Kconfig
> +++ b/drivers/gpu/drm/xen/Kconfig
> @@ -15,3 +15,16 @@ config DRM_XEN_FRONTEND
>   help
> Choose this option if you want to enable a para-virtualized
> frontend DRM/KMS driver for Xen guest OSes.
> +
> +config DRM_XEN_FRONTEND_CMA
> + bool "Use DRM CMA to allocate dumb buffers"
> + depends on DRM_XEN_FRONTEND
> + select DRM_KMS_CMA_HELPER
> + select DRM_GEM_CMA_HELPER
> + help
> +   Use DRM CMA helpers to allocate display buffers.
> +   This is useful for the use-cases when guest driver needs to
> +   share or export buffers to other drivers which only expect
> +   contiguous buffers.
> +   Note: in this mode driver cannot use buffers allocated
> +   by the backend.
> diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
> index 4fcb0da1a9c5..12376ec78fbc 100644
> --- a/drivers/gpu/drm/xen/Makefile
> +++ b/drivers/gpu/drm/xen/Makefile
> @@ -8,4 +8,10 @@ drm_xen_front-objs := xen_drm_front.o \
> xen_drm_front_shbuf.o \
> xen_drm_front_cfg.o
>  
> +ifeq ($(CONFIG_DRM_XEN_FRONTEND_CMA),y)
> + drm_xen_front-objs += xen_drm_front_gem_cma.o
> +else
> + drm_xen_front-objs += xen_drm_front_gem.o
> +endif
> +
>  obj-$(CONFIG_DRM_XEN_FRONTEND) += drm_xen_front.o
> diff --git a/drivers/gpu/drm/xen/xen_drm_front.h 
> b/drivers/gpu/drm/xen/xen_drm_front.h
> index 9ed5bfb248d0..c6f52c892434 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front.h
> +++

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-28 Thread Oleksandr Andrushchenko 

On 02/28/2018 09:46 PM, Boris Ostrovsky wrote:

On 02/27/2018 01:52 AM, Oleksandr Andrushchenko wrote:

On 02/27/2018 01:47 AM, Boris Ostrovsky wrote:

On 02/23/2018 10:35 AM, Oleksandr Andrushchenko wrote:

On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:

On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:

+    ret = gem_alloc_pages_array(xen_obj, size);
+    if (ret < 0) {
+    gem_free_pages_array(xen_obj);
+    goto fail;
+    }
+
+    ret = alloc_xenballooned_pages(xen_obj->num_pages,
+    xen_obj->pages);

Why are you allocating balloon pages?

in this use-case we map pages provided by the backend
(yes, I know this can be a problem from both security
POV and that DomU can die holding pages of Dom0 forever:
but still it is a configuration option, so user decides
if her use-case needs this and takes responsibility for
such a decision).

Perhaps I am missing something here but when you say "I know this can be
a problem from both security POV ..." then there is something wrong with
your solution.

well, in this scenario there are actually 2 concerns:
1. If DomU dies the pages/grants from Dom0/DomD cannot be
reclaimed back
2. Misbehaving guest may send too many requests to the
backend exhausting grant references and memory of Dom0/DomD
(this is the only concern from security POV). Please see [1]

But, we are focusing on embedded use-cases,
so those systems we use are not that "dynamic" with respect to 2).
Namely: we have fixed number of domains and their functionality
is well known, so we can do rather precise assumption on resource
usage. This is why I try to warn on such a use-case and rely on
the end user who understands the caveats


How will dom0/backend know whether or not to trust the front end (and
thus whether or not to provide provide pages to it)? Will there be
something in xenstore, for example, to indicate such trusted frontends?
Exactly, there is a dedicated xl configuration option available [1] for 
vdispl:


"be-alloc=BOOLEAN
Indicates if backend can be a buffer provider/allocator for this domain. 
See display protocol for details."


Thus, one can configure this per domain for trusted ones in corresponding
xl configuration files

-boris



I'll probably add more precise description of this use-case
clarifying what is that security POV, so there is no confusion

Hope this explanation answers your questions

-boris


Please see description of the buffering modes in xen_drm_front.h
specifically for backend allocated buffers:
  
***


   * 2. Buffers allocated by the backend
  
***


   *
   * This mode of operation is run-time configured via guest domain
configuration
   * through XenStore entries.
   *
   * For systems which do not provide IOMMU support, but having specific
   * requirements for display buffers it is possible to allocate such
buffers
   * at backend side and share those with the frontend.
   * For example, if host domain is 1:1 mapped and has DRM/GPU hardware
expecting
   * physically contiguous memory, this allows implementing zero-copying
   * use-cases.


-boris


+    if (ret < 0) {
+    DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
+    xen_obj->num_pages, ret);
+    goto fail;
+    }
+
+    return xen_obj;
+    }
+    /*
+ * need to allocate backing pages now, so we can share those
+ * with the backend
+ */
+    xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
+    xen_obj->pages = drm_gem_get_pages(_obj->base);
+    if (IS_ERR_OR_NULL(xen_obj->pages)) {
+    ret = PTR_ERR(xen_obj->pages);
+    xen_obj->pages = NULL;
+    goto fail;
+    }
+
+    return xen_obj;
+
+fail:
+    DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
+    return ERR_PTR(ret);
+}
+


Thank you,
Oleksandr

[1]
https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03100.html


[1] https://xenbits.xen.org/docs/4.10-testing/man/xl.cfg.5.html

   Indicates if backend can be a buffer provider/allocator for this
   domain. See display protocol for details.

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-28 Thread Oleksandr Andrushchenko 

On 02/28/2018 09:46 PM, Boris Ostrovsky wrote:

On 02/27/2018 01:52 AM, Oleksandr Andrushchenko wrote:

On 02/27/2018 01:47 AM, Boris Ostrovsky wrote:

On 02/23/2018 10:35 AM, Oleksandr Andrushchenko wrote:

On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:

On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:

+    ret = gem_alloc_pages_array(xen_obj, size);
+    if (ret < 0) {
+    gem_free_pages_array(xen_obj);
+    goto fail;
+    }
+
+    ret = alloc_xenballooned_pages(xen_obj->num_pages,
+    xen_obj->pages);

Why are you allocating balloon pages?

in this use-case we map pages provided by the backend
(yes, I know this can be a problem from both security
POV and that DomU can die holding pages of Dom0 forever:
but still it is a configuration option, so user decides
if her use-case needs this and takes responsibility for
such a decision).

Perhaps I am missing something here but when you say "I know this can be
a problem from both security POV ..." then there is something wrong with
your solution.

well, in this scenario there are actually 2 concerns:
1. If DomU dies the pages/grants from Dom0/DomD cannot be
reclaimed back
2. Misbehaving guest may send too many requests to the
backend exhausting grant references and memory of Dom0/DomD
(this is the only concern from security POV). Please see [1]

But, we are focusing on embedded use-cases,
so those systems we use are not that "dynamic" with respect to 2).
Namely: we have fixed number of domains and their functionality
is well known, so we can do rather precise assumption on resource
usage. This is why I try to warn on such a use-case and rely on
the end user who understands the caveats


How will dom0/backend know whether or not to trust the front end (and
thus whether or not to provide provide pages to it)? Will there be
something in xenstore, for example, to indicate such trusted frontends?
Exactly, there is a dedicated xl configuration option available [1] for 
vdispl:


"be-alloc=BOOLEAN
Indicates if backend can be a buffer provider/allocator for this domain. 
See display protocol for details."


Thus, one can configure this per domain for trusted ones in corresponding
xl configuration files

-boris



I'll probably add more precise description of this use-case
clarifying what is that security POV, so there is no confusion

Hope this explanation answers your questions

-boris


Please see description of the buffering modes in xen_drm_front.h
specifically for backend allocated buffers:
  
***


   * 2. Buffers allocated by the backend
  
***


   *
   * This mode of operation is run-time configured via guest domain
configuration
   * through XenStore entries.
   *
   * For systems which do not provide IOMMU support, but having specific
   * requirements for display buffers it is possible to allocate such
buffers
   * at backend side and share those with the frontend.
   * For example, if host domain is 1:1 mapped and has DRM/GPU hardware
expecting
   * physically contiguous memory, this allows implementing zero-copying
   * use-cases.


-boris


+    if (ret < 0) {
+    DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
+    xen_obj->num_pages, ret);
+    goto fail;
+    }
+
+    return xen_obj;
+    }
+    /*
+ * need to allocate backing pages now, so we can share those
+ * with the backend
+ */
+    xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
+    xen_obj->pages = drm_gem_get_pages(_obj->base);
+    if (IS_ERR_OR_NULL(xen_obj->pages)) {
+    ret = PTR_ERR(xen_obj->pages);
+    xen_obj->pages = NULL;
+    goto fail;
+    }
+
+    return xen_obj;
+
+fail:
+    DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
+    return ERR_PTR(ret);
+}
+


Thank you,
Oleksandr

[1]
https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03100.html


[1] https://xenbits.xen.org/docs/4.10-testing/man/xl.cfg.5.html

   Indicates if backend can be a buffer provider/allocator for this
   domain. See display protocol for details.

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-28 Thread Boris Ostrovsky 
On 02/27/2018 01:52 AM, Oleksandr Andrushchenko wrote:
> On 02/27/2018 01:47 AM, Boris Ostrovsky wrote:
>> On 02/23/2018 10:35 AM, Oleksandr Andrushchenko wrote:
>>> On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:
 On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:
>>
> +    ret = gem_alloc_pages_array(xen_obj, size);
> +    if (ret < 0) {
> +    gem_free_pages_array(xen_obj);
> +    goto fail;
> +    }
> +
> +    ret = alloc_xenballooned_pages(xen_obj->num_pages,
> +    xen_obj->pages);
 Why are you allocating balloon pages?
>>> in this use-case we map pages provided by the backend
>>> (yes, I know this can be a problem from both security
>>> POV and that DomU can die holding pages of Dom0 forever:
>>> but still it is a configuration option, so user decides
>>> if her use-case needs this and takes responsibility for
>>> such a decision).
>>
>> Perhaps I am missing something here but when you say "I know this can be
>> a problem from both security POV ..." then there is something wrong with
>> your solution.
> well, in this scenario there are actually 2 concerns:
> 1. If DomU dies the pages/grants from Dom0/DomD cannot be
> reclaimed back
> 2. Misbehaving guest may send too many requests to the
> backend exhausting grant references and memory of Dom0/DomD
> (this is the only concern from security POV). Please see [1]
>
> But, we are focusing on embedded use-cases,
> so those systems we use are not that "dynamic" with respect to 2).
> Namely: we have fixed number of domains and their functionality
> is well known, so we can do rather precise assumption on resource
> usage. This is why I try to warn on such a use-case and rely on
> the end user who understands the caveats


How will dom0/backend know whether or not to trust the front end (and
thus whether or not to provide provide pages to it)? Will there be
something in xenstore, for example, to indicate such trusted frontends?

-boris


>
> I'll probably add more precise description of this use-case
> clarifying what is that security POV, so there is no confusion
>
> Hope this explanation answers your questions
>> -boris
>>
>>> Please see description of the buffering modes in xen_drm_front.h
>>> specifically for backend allocated buffers:
>>>  
>>> ***
>>>
>>>   * 2. Buffers allocated by the backend
>>>  
>>> ***
>>>
>>>   *
>>>   * This mode of operation is run-time configured via guest domain
>>> configuration
>>>   * through XenStore entries.
>>>   *
>>>   * For systems which do not provide IOMMU support, but having specific
>>>   * requirements for display buffers it is possible to allocate such
>>> buffers
>>>   * at backend side and share those with the frontend.
>>>   * For example, if host domain is 1:1 mapped and has DRM/GPU hardware
>>> expecting
>>>   * physically contiguous memory, this allows implementing zero-copying
>>>   * use-cases.
>>>
 -boris

> +    if (ret < 0) {
> +    DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
> +    xen_obj->num_pages, ret);
> +    goto fail;
> +    }
> +
> +    return xen_obj;
> +    }
> +    /*
> + * need to allocate backing pages now, so we can share those
> + * with the backend
> + */
> +    xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
> +    xen_obj->pages = drm_gem_get_pages(_obj->base);
> +    if (IS_ERR_OR_NULL(xen_obj->pages)) {
> +    ret = PTR_ERR(xen_obj->pages);
> +    xen_obj->pages = NULL;
> +    goto fail;
> +    }
> +
> +    return xen_obj;
> +
> +fail:
> +    DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
> +    return ERR_PTR(ret);
> +}
> +
>
> Thank you,
> Oleksandr
>
> [1]
> https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03100.html
>

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-28 Thread Boris Ostrovsky 
On 02/27/2018 01:52 AM, Oleksandr Andrushchenko wrote:
> On 02/27/2018 01:47 AM, Boris Ostrovsky wrote:
>> On 02/23/2018 10:35 AM, Oleksandr Andrushchenko wrote:
>>> On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:
 On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:
>>
> +    ret = gem_alloc_pages_array(xen_obj, size);
> +    if (ret < 0) {
> +    gem_free_pages_array(xen_obj);
> +    goto fail;
> +    }
> +
> +    ret = alloc_xenballooned_pages(xen_obj->num_pages,
> +    xen_obj->pages);
 Why are you allocating balloon pages?
>>> in this use-case we map pages provided by the backend
>>> (yes, I know this can be a problem from both security
>>> POV and that DomU can die holding pages of Dom0 forever:
>>> but still it is a configuration option, so user decides
>>> if her use-case needs this and takes responsibility for
>>> such a decision).
>>
>> Perhaps I am missing something here but when you say "I know this can be
>> a problem from both security POV ..." then there is something wrong with
>> your solution.
> well, in this scenario there are actually 2 concerns:
> 1. If DomU dies the pages/grants from Dom0/DomD cannot be
> reclaimed back
> 2. Misbehaving guest may send too many requests to the
> backend exhausting grant references and memory of Dom0/DomD
> (this is the only concern from security POV). Please see [1]
>
> But, we are focusing on embedded use-cases,
> so those systems we use are not that "dynamic" with respect to 2).
> Namely: we have fixed number of domains and their functionality
> is well known, so we can do rather precise assumption on resource
> usage. This is why I try to warn on such a use-case and rely on
> the end user who understands the caveats


How will dom0/backend know whether or not to trust the front end (and
thus whether or not to provide provide pages to it)? Will there be
something in xenstore, for example, to indicate such trusted frontends?

-boris


>
> I'll probably add more precise description of this use-case
> clarifying what is that security POV, so there is no confusion
>
> Hope this explanation answers your questions
>> -boris
>>
>>> Please see description of the buffering modes in xen_drm_front.h
>>> specifically for backend allocated buffers:
>>>  
>>> ***
>>>
>>>   * 2. Buffers allocated by the backend
>>>  
>>> ***
>>>
>>>   *
>>>   * This mode of operation is run-time configured via guest domain
>>> configuration
>>>   * through XenStore entries.
>>>   *
>>>   * For systems which do not provide IOMMU support, but having specific
>>>   * requirements for display buffers it is possible to allocate such
>>> buffers
>>>   * at backend side and share those with the frontend.
>>>   * For example, if host domain is 1:1 mapped and has DRM/GPU hardware
>>> expecting
>>>   * physically contiguous memory, this allows implementing zero-copying
>>>   * use-cases.
>>>
 -boris

> +    if (ret < 0) {
> +    DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
> +    xen_obj->num_pages, ret);
> +    goto fail;
> +    }
> +
> +    return xen_obj;
> +    }
> +    /*
> + * need to allocate backing pages now, so we can share those
> + * with the backend
> + */
> +    xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
> +    xen_obj->pages = drm_gem_get_pages(_obj->base);
> +    if (IS_ERR_OR_NULL(xen_obj->pages)) {
> +    ret = PTR_ERR(xen_obj->pages);
> +    xen_obj->pages = NULL;
> +    goto fail;
> +    }
> +
> +    return xen_obj;
> +
> +fail:
> +    DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
> +    return ERR_PTR(ret);
> +}
> +
>
> Thank you,
> Oleksandr
>
> [1]
> https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03100.html
>

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-26 Thread Oleksandr Andrushchenko 

On 02/27/2018 01:47 AM, Boris Ostrovsky wrote:

On 02/23/2018 10:35 AM, Oleksandr Andrushchenko wrote:

On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:

On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:

+static struct xen_gem_object *gem_create(struct drm_device *dev,
size_t size)
+{
+struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+struct xen_gem_object *xen_obj;
+int ret;
+
+size = round_up(size, PAGE_SIZE);
+xen_obj = gem_create_obj(dev, size);
+if (IS_ERR_OR_NULL(xen_obj))
+return xen_obj;
+
+if (drm_info->cfg->be_alloc) {
+/*
+ * backend will allocate space for this buffer, so
+ * only allocate array of pointers to pages
+ */
+xen_obj->be_alloc = true;

If be_alloc is a flag (which I am not sure about) --- should it be set
to true *after* you've successfully allocated your things?

this is a configuration option telling about the way
the buffer gets allocated: either by the frontend or
backend (be_alloc -> buffer allocated by the backend)


I can see how drm_info->cfg->be_alloc might be a configuration option
but xen_obj->be_alloc is set here and that's not how configuration
options typically behave.

you are right, I will put be_alloc down the code and will slightly
rework error handling for this function



+ret = gem_alloc_pages_array(xen_obj, size);
+if (ret < 0) {
+gem_free_pages_array(xen_obj);
+goto fail;
+}
+
+ret = alloc_xenballooned_pages(xen_obj->num_pages,
+xen_obj->pages);

Why are you allocating balloon pages?

in this use-case we map pages provided by the backend
(yes, I know this can be a problem from both security
POV and that DomU can die holding pages of Dom0 forever:
but still it is a configuration option, so user decides
if her use-case needs this and takes responsibility for
such a decision).


Perhaps I am missing something here but when you say "I know this can be
a problem from both security POV ..." then there is something wrong with
your solution.

well, in this scenario there are actually 2 concerns:
1. If DomU dies the pages/grants from Dom0/DomD cannot be
reclaimed back
2. Misbehaving guest may send too many requests to the
backend exhausting grant references and memory of Dom0/DomD
(this is the only concern from security POV). Please see [1]

But, we are focusing on embedded use-cases,
so those systems we use are not that "dynamic" with respect to 2).
Namely: we have fixed number of domains and their functionality
is well known, so we can do rather precise assumption on resource
usage. This is why I try to warn on such a use-case and rely on
the end user who understands the caveats

I'll probably add more precise description of this use-case
clarifying what is that security POV, so there is no confusion

Hope this explanation answers your questions

-boris


Please see description of the buffering modes in xen_drm_front.h
specifically for backend allocated buffers:
  
***

  * 2. Buffers allocated by the backend
  
***

  *
  * This mode of operation is run-time configured via guest domain
configuration
  * through XenStore entries.
  *
  * For systems which do not provide IOMMU support, but having specific
  * requirements for display buffers it is possible to allocate such
buffers
  * at backend side and share those with the frontend.
  * For example, if host domain is 1:1 mapped and has DRM/GPU hardware
expecting
  * physically contiguous memory, this allows implementing zero-copying
  * use-cases.


-boris


+if (ret < 0) {
+DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
+xen_obj->num_pages, ret);
+goto fail;
+}
+
+return xen_obj;
+}
+/*
+ * need to allocate backing pages now, so we can share those
+ * with the backend
+ */
+xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
+xen_obj->pages = drm_gem_get_pages(_obj->base);
+if (IS_ERR_OR_NULL(xen_obj->pages)) {
+ret = PTR_ERR(xen_obj->pages);
+xen_obj->pages = NULL;
+goto fail;
+}
+
+return xen_obj;
+
+fail:
+DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
+return ERR_PTR(ret);
+}
+


Thank you,
Oleksandr

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03100.html

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-26 Thread Oleksandr Andrushchenko 

On 02/27/2018 01:47 AM, Boris Ostrovsky wrote:

On 02/23/2018 10:35 AM, Oleksandr Andrushchenko wrote:

On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:

On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:

+static struct xen_gem_object *gem_create(struct drm_device *dev,
size_t size)
+{
+struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+struct xen_gem_object *xen_obj;
+int ret;
+
+size = round_up(size, PAGE_SIZE);
+xen_obj = gem_create_obj(dev, size);
+if (IS_ERR_OR_NULL(xen_obj))
+return xen_obj;
+
+if (drm_info->cfg->be_alloc) {
+/*
+ * backend will allocate space for this buffer, so
+ * only allocate array of pointers to pages
+ */
+xen_obj->be_alloc = true;

If be_alloc is a flag (which I am not sure about) --- should it be set
to true *after* you've successfully allocated your things?

this is a configuration option telling about the way
the buffer gets allocated: either by the frontend or
backend (be_alloc -> buffer allocated by the backend)


I can see how drm_info->cfg->be_alloc might be a configuration option
but xen_obj->be_alloc is set here and that's not how configuration
options typically behave.

you are right, I will put be_alloc down the code and will slightly
rework error handling for this function



+ret = gem_alloc_pages_array(xen_obj, size);
+if (ret < 0) {
+gem_free_pages_array(xen_obj);
+goto fail;
+}
+
+ret = alloc_xenballooned_pages(xen_obj->num_pages,
+xen_obj->pages);

Why are you allocating balloon pages?

in this use-case we map pages provided by the backend
(yes, I know this can be a problem from both security
POV and that DomU can die holding pages of Dom0 forever:
but still it is a configuration option, so user decides
if her use-case needs this and takes responsibility for
such a decision).


Perhaps I am missing something here but when you say "I know this can be
a problem from both security POV ..." then there is something wrong with
your solution.

well, in this scenario there are actually 2 concerns:
1. If DomU dies the pages/grants from Dom0/DomD cannot be
reclaimed back
2. Misbehaving guest may send too many requests to the
backend exhausting grant references and memory of Dom0/DomD
(this is the only concern from security POV). Please see [1]

But, we are focusing on embedded use-cases,
so those systems we use are not that "dynamic" with respect to 2).
Namely: we have fixed number of domains and their functionality
is well known, so we can do rather precise assumption on resource
usage. This is why I try to warn on such a use-case and rely on
the end user who understands the caveats

I'll probably add more precise description of this use-case
clarifying what is that security POV, so there is no confusion

Hope this explanation answers your questions

-boris


Please see description of the buffering modes in xen_drm_front.h
specifically for backend allocated buffers:
  
***

  * 2. Buffers allocated by the backend
  
***

  *
  * This mode of operation is run-time configured via guest domain
configuration
  * through XenStore entries.
  *
  * For systems which do not provide IOMMU support, but having specific
  * requirements for display buffers it is possible to allocate such
buffers
  * at backend side and share those with the frontend.
  * For example, if host domain is 1:1 mapped and has DRM/GPU hardware
expecting
  * physically contiguous memory, this allows implementing zero-copying
  * use-cases.


-boris


+if (ret < 0) {
+DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
+xen_obj->num_pages, ret);
+goto fail;
+}
+
+return xen_obj;
+}
+/*
+ * need to allocate backing pages now, so we can share those
+ * with the backend
+ */
+xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
+xen_obj->pages = drm_gem_get_pages(_obj->base);
+if (IS_ERR_OR_NULL(xen_obj->pages)) {
+ret = PTR_ERR(xen_obj->pages);
+xen_obj->pages = NULL;
+goto fail;
+}
+
+return xen_obj;
+
+fail:
+DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
+return ERR_PTR(ret);
+}
+


Thank you,
Oleksandr

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03100.html

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-26 Thread Boris Ostrovsky 
On 02/23/2018 10:35 AM, Oleksandr Andrushchenko wrote:
> On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:
>> On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:
>>> +static struct xen_gem_object *gem_create(struct drm_device *dev,
>>> size_t size)
>>> +{
>>> +struct xen_drm_front_drm_info *drm_info = dev->dev_private;
>>> +struct xen_gem_object *xen_obj;
>>> +int ret;
>>> +
>>> +size = round_up(size, PAGE_SIZE);
>>> +xen_obj = gem_create_obj(dev, size);
>>> +if (IS_ERR_OR_NULL(xen_obj))
>>> +return xen_obj;
>>> +
>>> +if (drm_info->cfg->be_alloc) {
>>> +/*
>>> + * backend will allocate space for this buffer, so
>>> + * only allocate array of pointers to pages
>>> + */
>>> +xen_obj->be_alloc = true;
>> If be_alloc is a flag (which I am not sure about) --- should it be set
>> to true *after* you've successfully allocated your things?
> this is a configuration option telling about the way
> the buffer gets allocated: either by the frontend or
> backend (be_alloc -> buffer allocated by the backend)


I can see how drm_info->cfg->be_alloc might be a configuration option
but xen_obj->be_alloc is set here and that's not how configuration
options typically behave.


>>
>>> +ret = gem_alloc_pages_array(xen_obj, size);
>>> +if (ret < 0) {
>>> +gem_free_pages_array(xen_obj);
>>> +goto fail;
>>> +}
>>> +
>>> +ret = alloc_xenballooned_pages(xen_obj->num_pages,
>>> +xen_obj->pages);
>> Why are you allocating balloon pages?
> in this use-case we map pages provided by the backend
> (yes, I know this can be a problem from both security
> POV and that DomU can die holding pages of Dom0 forever:
> but still it is a configuration option, so user decides
> if her use-case needs this and takes responsibility for
> such a decision).


Perhaps I am missing something here but when you say "I know this can be
a problem from both security POV ..." then there is something wrong with
your solution.

-boris

>
> Please see description of the buffering modes in xen_drm_front.h
> specifically for backend allocated buffers:
>  
> ***
>
>  * 2. Buffers allocated by the backend
>  
> ***
>
>  *
>  * This mode of operation is run-time configured via guest domain
> configuration
>  * through XenStore entries.
>  *
>  * For systems which do not provide IOMMU support, but having specific
>  * requirements for display buffers it is possible to allocate such
> buffers
>  * at backend side and share those with the frontend.
>  * For example, if host domain is 1:1 mapped and has DRM/GPU hardware
> expecting
>  * physically contiguous memory, this allows implementing zero-copying
>  * use-cases.
>
>>
>> -boris
>>
>>> +if (ret < 0) {
>>> +DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
>>> +xen_obj->num_pages, ret);
>>> +goto fail;
>>> +}
>>> +
>>> +return xen_obj;
>>> +}
>>> +/*
>>> + * need to allocate backing pages now, so we can share those
>>> + * with the backend
>>> + */
>>> +xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
>>> +xen_obj->pages = drm_gem_get_pages(_obj->base);
>>> +if (IS_ERR_OR_NULL(xen_obj->pages)) {
>>> +ret = PTR_ERR(xen_obj->pages);
>>> +xen_obj->pages = NULL;
>>> +goto fail;
>>> +}
>>> +
>>> +return xen_obj;
>>> +
>>> +fail:
>>> +DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
>>> +return ERR_PTR(ret);
>>> +}
>>> +
>>>
>

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-26 Thread Boris Ostrovsky 
On 02/23/2018 10:35 AM, Oleksandr Andrushchenko wrote:
> On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:
>> On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:
>>> +static struct xen_gem_object *gem_create(struct drm_device *dev,
>>> size_t size)
>>> +{
>>> +struct xen_drm_front_drm_info *drm_info = dev->dev_private;
>>> +struct xen_gem_object *xen_obj;
>>> +int ret;
>>> +
>>> +size = round_up(size, PAGE_SIZE);
>>> +xen_obj = gem_create_obj(dev, size);
>>> +if (IS_ERR_OR_NULL(xen_obj))
>>> +return xen_obj;
>>> +
>>> +if (drm_info->cfg->be_alloc) {
>>> +/*
>>> + * backend will allocate space for this buffer, so
>>> + * only allocate array of pointers to pages
>>> + */
>>> +xen_obj->be_alloc = true;
>> If be_alloc is a flag (which I am not sure about) --- should it be set
>> to true *after* you've successfully allocated your things?
> this is a configuration option telling about the way
> the buffer gets allocated: either by the frontend or
> backend (be_alloc -> buffer allocated by the backend)


I can see how drm_info->cfg->be_alloc might be a configuration option
but xen_obj->be_alloc is set here and that's not how configuration
options typically behave.


>>
>>> +ret = gem_alloc_pages_array(xen_obj, size);
>>> +if (ret < 0) {
>>> +gem_free_pages_array(xen_obj);
>>> +goto fail;
>>> +}
>>> +
>>> +ret = alloc_xenballooned_pages(xen_obj->num_pages,
>>> +xen_obj->pages);
>> Why are you allocating balloon pages?
> in this use-case we map pages provided by the backend
> (yes, I know this can be a problem from both security
> POV and that DomU can die holding pages of Dom0 forever:
> but still it is a configuration option, so user decides
> if her use-case needs this and takes responsibility for
> such a decision).


Perhaps I am missing something here but when you say "I know this can be
a problem from both security POV ..." then there is something wrong with
your solution.

-boris

>
> Please see description of the buffering modes in xen_drm_front.h
> specifically for backend allocated buffers:
>  
> ***
>
>  * 2. Buffers allocated by the backend
>  
> ***
>
>  *
>  * This mode of operation is run-time configured via guest domain
> configuration
>  * through XenStore entries.
>  *
>  * For systems which do not provide IOMMU support, but having specific
>  * requirements for display buffers it is possible to allocate such
> buffers
>  * at backend side and share those with the frontend.
>  * For example, if host domain is 1:1 mapped and has DRM/GPU hardware
> expecting
>  * physically contiguous memory, this allows implementing zero-copying
>  * use-cases.
>
>>
>> -boris
>>
>>> +if (ret < 0) {
>>> +DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
>>> +xen_obj->num_pages, ret);
>>> +goto fail;
>>> +}
>>> +
>>> +return xen_obj;
>>> +}
>>> +/*
>>> + * need to allocate backing pages now, so we can share those
>>> + * with the backend
>>> + */
>>> +xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
>>> +xen_obj->pages = drm_gem_get_pages(_obj->base);
>>> +if (IS_ERR_OR_NULL(xen_obj->pages)) {
>>> +ret = PTR_ERR(xen_obj->pages);
>>> +xen_obj->pages = NULL;
>>> +goto fail;
>>> +}
>>> +
>>> +return xen_obj;
>>> +
>>> +fail:
>>> +DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
>>> +return ERR_PTR(ret);
>>> +}
>>> +
>>>
>

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-23 Thread Oleksandr Andrushchenko 

On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:

On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:

+static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)
+{
+   struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+   struct xen_gem_object *xen_obj;
+   int ret;
+
+   size = round_up(size, PAGE_SIZE);
+   xen_obj = gem_create_obj(dev, size);
+   if (IS_ERR_OR_NULL(xen_obj))
+   return xen_obj;
+
+   if (drm_info->cfg->be_alloc) {
+   /*
+* backend will allocate space for this buffer, so
+* only allocate array of pointers to pages
+*/
+   xen_obj->be_alloc = true;

If be_alloc is a flag (which I am not sure about) --- should it be set
to true *after* you've successfully allocated your things?

this is a configuration option telling about the way
the buffer gets allocated: either by the frontend or
backend (be_alloc -> buffer allocated by the backend)



+   ret = gem_alloc_pages_array(xen_obj, size);
+   if (ret < 0) {
+   gem_free_pages_array(xen_obj);
+   goto fail;
+   }
+
+   ret = alloc_xenballooned_pages(xen_obj->num_pages,
+   xen_obj->pages);

Why are you allocating balloon pages?

in this use-case we map pages provided by the backend
(yes, I know this can be a problem from both security
POV and that DomU can die holding pages of Dom0 forever:
but still it is a configuration option, so user decides
if her use-case needs this and takes responsibility for
such a decision).

Please see description of the buffering modes in xen_drm_front.h
specifically for backend allocated buffers:
 ***
 * 2. Buffers allocated by the backend
 ***
 *
 * This mode of operation is run-time configured via guest domain 
configuration

 * through XenStore entries.
 *
 * For systems which do not provide IOMMU support, but having specific
 * requirements for display buffers it is possible to allocate such buffers
 * at backend side and share those with the frontend.
 * For example, if host domain is 1:1 mapped and has DRM/GPU hardware 
expecting

 * physically contiguous memory, this allows implementing zero-copying
 * use-cases.



-boris


+   if (ret < 0) {
+   DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
+   xen_obj->num_pages, ret);
+   goto fail;
+   }
+
+   return xen_obj;
+   }
+   /*
+* need to allocate backing pages now, so we can share those
+* with the backend
+*/
+   xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
+   xen_obj->pages = drm_gem_get_pages(_obj->base);
+   if (IS_ERR_OR_NULL(xen_obj->pages)) {
+   ret = PTR_ERR(xen_obj->pages);
+   xen_obj->pages = NULL;
+   goto fail;
+   }
+
+   return xen_obj;
+
+fail:
+   DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
+   return ERR_PTR(ret);
+}
+

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-23 Thread Oleksandr Andrushchenko 

On 02/23/2018 05:26 PM, Boris Ostrovsky wrote:

On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:

+static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)
+{
+   struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+   struct xen_gem_object *xen_obj;
+   int ret;
+
+   size = round_up(size, PAGE_SIZE);
+   xen_obj = gem_create_obj(dev, size);
+   if (IS_ERR_OR_NULL(xen_obj))
+   return xen_obj;
+
+   if (drm_info->cfg->be_alloc) {
+   /*
+* backend will allocate space for this buffer, so
+* only allocate array of pointers to pages
+*/
+   xen_obj->be_alloc = true;

If be_alloc is a flag (which I am not sure about) --- should it be set
to true *after* you've successfully allocated your things?

this is a configuration option telling about the way
the buffer gets allocated: either by the frontend or
backend (be_alloc -> buffer allocated by the backend)



+   ret = gem_alloc_pages_array(xen_obj, size);
+   if (ret < 0) {
+   gem_free_pages_array(xen_obj);
+   goto fail;
+   }
+
+   ret = alloc_xenballooned_pages(xen_obj->num_pages,
+   xen_obj->pages);

Why are you allocating balloon pages?

in this use-case we map pages provided by the backend
(yes, I know this can be a problem from both security
POV and that DomU can die holding pages of Dom0 forever:
but still it is a configuration option, so user decides
if her use-case needs this and takes responsibility for
such a decision).

Please see description of the buffering modes in xen_drm_front.h
specifically for backend allocated buffers:
 ***
 * 2. Buffers allocated by the backend
 ***
 *
 * This mode of operation is run-time configured via guest domain 
configuration

 * through XenStore entries.
 *
 * For systems which do not provide IOMMU support, but having specific
 * requirements for display buffers it is possible to allocate such buffers
 * at backend side and share those with the frontend.
 * For example, if host domain is 1:1 mapped and has DRM/GPU hardware 
expecting

 * physically contiguous memory, this allows implementing zero-copying
 * use-cases.



-boris


+   if (ret < 0) {
+   DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
+   xen_obj->num_pages, ret);
+   goto fail;
+   }
+
+   return xen_obj;
+   }
+   /*
+* need to allocate backing pages now, so we can share those
+* with the backend
+*/
+   xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
+   xen_obj->pages = drm_gem_get_pages(_obj->base);
+   if (IS_ERR_OR_NULL(xen_obj->pages)) {
+   ret = PTR_ERR(xen_obj->pages);
+   xen_obj->pages = NULL;
+   goto fail;
+   }
+
+   return xen_obj;
+
+fail:
+   DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
+   return ERR_PTR(ret);
+}
+

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-23 Thread Boris Ostrovsky 
On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:
> +static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)
> +{
> + struct xen_drm_front_drm_info *drm_info = dev->dev_private;
> + struct xen_gem_object *xen_obj;
> + int ret;
> +
> + size = round_up(size, PAGE_SIZE);
> + xen_obj = gem_create_obj(dev, size);
> + if (IS_ERR_OR_NULL(xen_obj))
> + return xen_obj;
> +
> + if (drm_info->cfg->be_alloc) {
> + /*
> +  * backend will allocate space for this buffer, so
> +  * only allocate array of pointers to pages
> +  */
> + xen_obj->be_alloc = true;

If be_alloc is a flag (which I am not sure about) --- should it be set
to true *after* you've successfully allocated your things?

> + ret = gem_alloc_pages_array(xen_obj, size);
> + if (ret < 0) {
> + gem_free_pages_array(xen_obj);
> + goto fail;
> + }
> +
> + ret = alloc_xenballooned_pages(xen_obj->num_pages,
> + xen_obj->pages);

Why are you allocating balloon pages?

-boris

> + if (ret < 0) {
> + DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
> + xen_obj->num_pages, ret);
> + goto fail;
> + }
> +
> + return xen_obj;
> + }
> + /*
> +  * need to allocate backing pages now, so we can share those
> +  * with the backend
> +  */
> + xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
> + xen_obj->pages = drm_gem_get_pages(_obj->base);
> + if (IS_ERR_OR_NULL(xen_obj->pages)) {
> + ret = PTR_ERR(xen_obj->pages);
> + xen_obj->pages = NULL;
> + goto fail;
> + }
> +
> + return xen_obj;
> +
> +fail:
> + DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
> + return ERR_PTR(ret);
> +}
> +
>

Re: [PATCH 8/9] drm/xen-front: Implement GEM operations

2018-02-23 Thread Boris Ostrovsky 
On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:
> +static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)
> +{
> + struct xen_drm_front_drm_info *drm_info = dev->dev_private;
> + struct xen_gem_object *xen_obj;
> + int ret;
> +
> + size = round_up(size, PAGE_SIZE);
> + xen_obj = gem_create_obj(dev, size);
> + if (IS_ERR_OR_NULL(xen_obj))
> + return xen_obj;
> +
> + if (drm_info->cfg->be_alloc) {
> + /*
> +  * backend will allocate space for this buffer, so
> +  * only allocate array of pointers to pages
> +  */
> + xen_obj->be_alloc = true;

If be_alloc is a flag (which I am not sure about) --- should it be set
to true *after* you've successfully allocated your things?

> + ret = gem_alloc_pages_array(xen_obj, size);
> + if (ret < 0) {
> + gem_free_pages_array(xen_obj);
> + goto fail;
> + }
> +
> + ret = alloc_xenballooned_pages(xen_obj->num_pages,
> + xen_obj->pages);

Why are you allocating balloon pages?

-boris

> + if (ret < 0) {
> + DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
> + xen_obj->num_pages, ret);
> + goto fail;
> + }
> +
> + return xen_obj;
> + }
> + /*
> +  * need to allocate backing pages now, so we can share those
> +  * with the backend
> +  */
> + xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
> + xen_obj->pages = drm_gem_get_pages(_obj->base);
> + if (IS_ERR_OR_NULL(xen_obj->pages)) {
> + ret = PTR_ERR(xen_obj->pages);
> + xen_obj->pages = NULL;
> + goto fail;
> + }
> +
> + return xen_obj;
> +
> +fail:
> + DRM_ERROR("Failed to allocate buffer with size %zu\n", size);
> + return ERR_PTR(ret);
> +}
> +
>