Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts

2018-03-22 Thread Jason Wang 



On 2018年03月22日 11:10, Michael S. Tsirkin wrote:

On Thu, Mar 22, 2018 at 09:52:18AM +0800, Jason Wang wrote:

On 2018年03月20日 12:26, Jonathan Helman wrote:

On Mar 19, 2018, at 7:31 PM, Jason Wang  wrote:



On 2018年03月20日 06:14, Jonathan Helman wrote:

Export the number of successful and failed hugetlb page
allocations via the virtio balloon driver. These 2 counts
come directly from the vm_events HTLB_BUDDY_PGALLOC and
HTLB_BUDDY_PGALLOC_FAIL.

Signed-off-by: Jonathan Helman

Reviewed-by: Jason Wang

Thanks.


---
   drivers/virtio/virtio_balloon.c | 6 ++
   include/uapi/linux/virtio_balloon.h | 4 +++-
   2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index dfe5684..6b237e3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,6 +272,12 @@ static unsigned int update_balloon_stats(struct 
virtio_balloon *vb)
pages_to_bytes(events[PSWPOUT]));
update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT, events[PGMAJFAULT]);
update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
+#ifdef CONFIG_HUGETLB_PAGE
+   update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGALLOC,
+   events[HTLB_BUDDY_PGALLOC]);
+   update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGFAIL,
+   events[HTLB_BUDDY_PGALLOC_FAIL]);
+#endif
   #endif
update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
pages_to_bytes(i.freeram));
diff --git a/include/uapi/linux/virtio_balloon.h 
b/include/uapi/linux/virtio_balloon.h
index 4e8b830..40297a3 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -53,7 +53,9 @@ struct virtio_balloon_config {
   #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
   #define VIRTIO_BALLOON_S_AVAIL6   /* Available memory as in /proc */
   #define VIRTIO_BALLOON_S_CACHES   7   /* Disk caches */
-#define VIRTIO_BALLOON_S_NR   8
+#define VIRTIO_BALLOON_S_HTLB_PGALLOC  8  /* Hugetlb page allocations */
+#define VIRTIO_BALLOON_S_HTLB_PGFAIL   9  /* Hugetlb page allocation failures 
*/
+#define VIRTIO_BALLOON_S_NR   10
 /*
* Memory statistics structure.

Not for this patch, but it looks to me that exporting such nr through uapi is 
fragile.

Sorry, can you explain what you mean here?

Jon

Spec said "Within an output buffer submitted to the statsq, the device MUST
ignore entries with tag values that it does not recognize". So exporting
VIRTIO_BALLOON_S_NR seems useless and device implementation can not depend
on such number in uapi.

Thanks

Suggestions? I don't like to break build for people ...



Didn't have a good idea. But maybe we should keep VIRTIO_BALLOON_S_NR 
unchanged, and add a comment here.


Thanks
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/23] drm: Eliminate plane->fb/crtc usage for atomic drivers

2018-03-22 Thread Noralf Trønnes 


Den 22.03.2018 19.49, skrev Ville Syrjälä:

On Thu, Mar 22, 2018 at 05:51:35PM +0100, Noralf Trønnes wrote:

tinydrm is also using plane->fb:

$ grep -r "plane\.fb" drivers/gpu/drm/tinydrm/
drivers/gpu/drm/tinydrm/repaper.c:  if (tdev->pipe.plane.fb != fb)
drivers/gpu/drm/tinydrm/mipi-dbi.c: if (tdev->pipe.plane.fb != fb)
drivers/gpu/drm/tinydrm/mipi-dbi.c: struct drm_framebuffer *fb =
mipi->tinydrm.pipe.plane.fb;

Oh dear, and naturally it's the most annoying one of the bunch. So we
want to take the plane lock in the fb.dirty() hook to look at the fb,
but mipi-dbi.c calls that directly from the bowels of its
.atomic_enable() hook. So that would deadlock pretty neatly if the
commit happens while already holding the lock.

So we'd either need to plumb an acquire context into fb.dirty(),
or maybe have tinydrm provide a lower level lockless dirty() hook
that gets called by both (there are just too many layers in this
particular cake to immediately see where such a hook would be best
placed). Or maybe there's some other solution I didn't think of.

Looking at this I'm also left wondering how this is supposed
handle fb.dirty() getting called concurrently with a modeset.
The dirty_mutex only seems to offer any protection for
fb.dirty() against another fb.dirty()...



I have been waiting for the 'page-flip with damage'[1] work to come to
fruition so I could handle all flushing during atomic commit.
The idea is to use the same damage rect for a generic dirtyfb callback.

Maybe a temporary tinydrm specific solution is possible until that work
lands, but I don't know enough about how to implement such a dirtyfb
callback.

I imagine it could look something like this:

 struct tinydrm_device {
+    struct drm_clip_rect dirtyfb_rect;
 };

static int tinydrm_fb_dirty(struct drm_framebuffer *fb,
                struct drm_file *file_priv,
                unsigned int flags, unsigned int color,
                struct drm_clip_rect *clips,
                unsigned int num_clips)
{
    struct tinydrm_device *tdev = fb->dev->dev_private;
    struct drm_framebuffer *planefb;

    /* Grab whatever lock needed to check this */
    planefb = tdev->pipe.plane.state->fb;

    /* fbdev can flush even when we're not interested */
    if (fb != planefb)
        return 0;

    /* Protect dirtyfb_rect with a lock */
    tinydrm_merge_clips(>dirty_rectfb, clips, num_clips, flags,
                fb->width, fb->height);

    /*
     * Somehow do an atomic commit that results in the atomic update
     * callback being called
     */
    ret = drm_atomic_commit(state);
...
}

static void mipi_dbi_flush(struct drm_framebuffer *fb,
               struct drm_clip_rect *clip)
{
    /* Flush out framebuffer */
}

void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
              struct drm_plane_state *old_state)
{
    struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
    struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
    struct drm_framebuffer *fb = pipe->plane.state->fb;
    struct drm_crtc *crtc = >pipe.crtc;

    if (!fb || (fb == old_state->fb && !tdev->dirty_rect.x2))
        goto out_vblank;

    /* Don't flush if the controller isn't initialized yet */
    if (!mipi->enabled)
        goto out_vblank;

    if (fb != old_state->fb) { /* Page flip or new, flush all */
        mipi_dbi_flush(fb, NULL);
    } else { /* dirtyfb ioctl */
        mipi_dbi_flush(fb, >dirtyfb_rect);
        memset(>dirtyfb_rect, 0, sizeof(tdev->dirtyfb_rect));
    }

out_vblank:
    if (crtc->state->event) {
        spin_lock_irq(>dev->event_lock);
        drm_crtc_send_vblank_event(crtc, crtc->state->event);
        spin_unlock_irq(>dev->event_lock);
        crtc->state->event = NULL;
    }
}

This is called from the driver pipe enable callback after the controller
has been initialized:

 void mipi_dbi_pipe_enable(struct drm_simple_display_pipe *pipe,
           struct drm_crtc_state *crtc_state)
 {
 struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
 struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
-     struct drm_framebuffer *fb = pipe->plane.fb;
+    struct drm_framebuffer *fb = pipe->plane.state->fb;

 DRM_DEBUG_KMS("\n");

 mipi->enabled = true;
-     if (fb)
-         fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
+    mipi_dbi_flush(fb, NULL);
 tinydrm_enable_backlight(mipi->backlight);
 }

I can make patches if this makes any sense and you can help me with the
dirtyfb implementation.

Noralf.

[1] 
https://lists.freedesktop.org/archives/dri-devel/2017-December/161003.html


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/23] drm: Eliminate plane->fb/crtc usage for atomic drivers

2018-03-22 Thread Ville Syrjälä 
On Thu, Mar 22, 2018 at 05:51:35PM +0100, Noralf Trønnes wrote:
> tinydrm is also using plane->fb:
> 
> $ grep -r "plane\.fb" drivers/gpu/drm/tinydrm/
> drivers/gpu/drm/tinydrm/repaper.c:  if (tdev->pipe.plane.fb != fb)
> drivers/gpu/drm/tinydrm/mipi-dbi.c: if (tdev->pipe.plane.fb != fb)
> drivers/gpu/drm/tinydrm/mipi-dbi.c: struct drm_framebuffer *fb = 
> mipi->tinydrm.pipe.plane.fb;

Oh dear, and naturally it's the most annoying one of the bunch. So we
want to take the plane lock in the fb.dirty() hook to look at the fb,
but mipi-dbi.c calls that directly from the bowels of its
.atomic_enable() hook. So that would deadlock pretty neatly if the
commit happens while already holding the lock.

So we'd either need to plumb an acquire context into fb.dirty(),
or maybe have tinydrm provide a lower level lockless dirty() hook
that gets called by both (there are just too many layers in this
particular cake to immediately see where such a hook would be best
placed). Or maybe there's some other solution I didn't think of.

Looking at this I'm also left wondering how this is supposed
handle fb.dirty() getting called concurrently with a modeset.
The dirty_mutex only seems to offer any protection for
fb.dirty() against another fb.dirty()...

-- 
Ville Syrjälä
Intel OTC
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/23] drm: Eliminate plane->fb/crtc usage for atomic drivers

2018-03-22 Thread Emil Velikov 
On 22 March 2018 at 18:03, Harry Wentland  wrote:
> On 2018-03-22 01:54 PM, Emil Velikov wrote:
>> Hi Ville,
>>
>> On 22 March 2018 at 15:22, Ville Syrjala  
>> wrote:
>>> From: Ville Syrjälä 
>>>
>>> I really just wanted to fix i915 to re-enable its planes afer load
>>> detection (a two line patch). This is what I actually ended up with
>>> after I ran into a framebuffer refcount leak with said two line patch.
>>>
>>> I've tested this on a few i915 boxes and so far it's looking
>>> good. Everything else is just compile tested.
>>>
>> Mostly thinking out loud:
>>
>> Wondering if one cannot somehow (re)move plane->fb/crtc altogether.
>> Otherwise drivers will reintroduce similar code, despite the WARNs and
>> beefy documentation :-\
>
> Wouldn't that require an atomic conversion of all remaining drivers?
>
That or maybe move into plane->legacy->{fb,crtc}. Feel free to swap
'legacy' with flashier name.

Hmm back in 2015 we had a GSoC that updated BOCHS and CIRRUS drivers,
but they never got merged.
Don't recall the details - from memory the conversion seemed fine, but
there was either shortage on review/other.

Might be worth reviving that... regardless it's getting a bit off-topic.
-Emil

[1] 
https://www.google-melange.com/archive/gsoc/2015/orgs/xorg/projects/johnhunter.html
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/23] drm: Eliminate plane->fb/crtc usage for atomic drivers

2018-03-22 Thread Emil Velikov 
Hi Ville,

On 22 March 2018 at 15:22, Ville Syrjala  wrote:
> From: Ville Syrjälä 
>
> I really just wanted to fix i915 to re-enable its planes afer load
> detection (a two line patch). This is what I actually ended up with
> after I ran into a framebuffer refcount leak with said two line patch.
>
> I've tested this on a few i915 boxes and so far it's looking
> good. Everything else is just compile tested.
>
Mostly thinking out loud:

Wondering if one cannot somehow (re)move plane->fb/crtc altogether.
Otherwise drivers will reintroduce similar code, despite the WARNs and
beefy documentation :-\

HTH
Emil
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v2 20/23] drm/virtio: Stop updating plane->crtc

2018-03-22 Thread Ville Syrjala 
From: Ville Syrjälä 

We want to get rid of plane->crtc on atomic drivers. Stop setting it.

v2: s/fb/crtc/ in the commit message (Gerd)

Cc: David Airlie 
Cc: Gerd Hoffmann 
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c 
b/drivers/gpu/drm/virtio/virtgpu_display.c
index 8cc8c34d67f5..42e842ceb53c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -302,8 +302,6 @@ static int vgdev_output_init(struct virtio_gpu_device 
*vgdev, int index)
drm_crtc_init_with_planes(dev, crtc, primary, cursor,
  _gpu_crtc_funcs, NULL);
drm_crtc_helper_add(crtc, _gpu_crtc_helper_funcs);
-   primary->crtc = crtc;
-   cursor->crtc = crtc;
 
drm_connector_init(dev, connector, _gpu_connector_funcs,
   DRM_MODE_CONNECTOR_VIRTUAL);
-- 
2.16.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 00/23] drm: Eliminate plane->fb/crtc usage for atomic drivers

2018-03-22 Thread Noralf Trønnes 


Den 22.03.2018 16.22, skrev Ville Syrjala:

From: Ville Syrjälä 

I really just wanted to fix i915 to re-enable its planes afer load
detection (a two line patch). This is what I actually ended up with
after I ran into a framebuffer refcount leak with said two line patch.

I've tested this on a few i915 boxes and so far it's looking
good. Everything else is just compile tested.

Entire series available here:
git://github.com/vsyrjala/linux.git plane_fb_crtc_nuke

Cc: Alex Deucher 
Cc: amd-...@lists.freedesktop.org
Cc: Benjamin Gaignard 
Cc: Boris Brezillon 
Cc: ch...@chris-wilson.co.uk
Cc: "Christian König" 
Cc: Daniel Vetter 
Cc: Dave Airlie 
Cc: David Airlie 
Cc: "David (ChunMing) Zhou" 
Cc: Eric Anholt 
Cc: freedr...@lists.freedesktop.org
Cc: Gerd Hoffmann 
Cc: Harry Wentland 
Cc: Inki Dae 
Cc: Joonyoung Shim 
Cc: Kyungmin Park 
Cc: linux-arm-...@vger.kernel.org
Cc: Maarten Lankhorst 
Cc: martin.pe...@free.fr
Cc: Rob Clark 
Cc: Seung-Woo Kim 
Cc: Shawn Guo 
Cc: Sinclair Yeh 
Cc: Thomas Hellstrom 
Cc: Vincent Abriou 
Cc: virtualization@lists.linux-foundation.org
Cc: VMware Graphics 

Ville Syrjälä (23):
   Revert "drm/atomic-helper: Fix leak in disable_all"
   drm/atomic-helper: Make drm_atomic_helper_disable_all() update the
 plane->fb pointers
   drm: Clear crtc->primary->crtc when disabling the crtc via setcrtc()
   drm/atomic-helper: WARN if legacy plane fb pointers are bogus when
 committing duplicated state
   drm: Add local 'plane' variable for primary/cursor planes
   drm: Adjust whitespace for legibility
   drm: Make the fb refcount handover less magic
   drm: Use plane->state->fb over plane->fb
   drm/i915: Stop consulting plane->fb
   drm/msm: Stop consulting plane->fb
   drm/sti: Stop consulting plane->fb
   drm/vmwgfx: Stop consulting plane->fb
   drm/zte: Stop consulting plane->fb
   drm/atmel-hlcdc: Stop using plane->fb
   drm: Stop updating plane->crtc/fb/old_fb on atomic drivers
   drm/amdgpu/dc: Stop updating plane->fb
   drm/i915: Stop updating plane->fb/crtc
   drm/exynos: Stop updating plane->crtc
   drm/msm: Stop updating plane->fb/crtc
   drm/virtio: Stop updating plane->fb
   drm/vc4: Stop updating plane->fb/crtc
   drm/i915: Restore planes after load detection
   drm/i915: Make force_load_detect effective even w/ DMI quirks/hotplug


tinydrm is also using plane->fb:

$ grep -r "plane\.fb" drivers/gpu/drm/tinydrm/
drivers/gpu/drm/tinydrm/repaper.c:  if (tdev->pipe.plane.fb != fb)
drivers/gpu/drm/tinydrm/mipi-dbi.c: if (tdev->pipe.plane.fb != fb)
drivers/gpu/drm/tinydrm/mipi-dbi.c: struct drm_framebuffer *fb = 
mipi->tinydrm.pipe.plane.fb;

drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c: pipe->plane.fb = fb;
drivers/gpu/drm/tinydrm/ili9225.c:  if (tdev->pipe.plane.fb != fb)
drivers/gpu/drm/tinydrm/st7586.c:   if (tdev->pipe.plane.fb != fb)

Noralf.


  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 -
  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 12 +---
  drivers/gpu/drm/drm_atomic.c  | 55 ++--
  drivers/gpu/drm/drm_atomic_helper.c   | 79 ++-
  drivers/gpu/drm/drm_crtc.c| 51 ++-
  drivers/gpu/drm/drm_fb_helper.c   |  7 --
  drivers/gpu/drm/drm_framebuffer.c |  5 --
  drivers/gpu/drm/drm_plane.c   | 64 +++---
  drivers/gpu/drm/drm_plane_helper.c|  4 +-
  drivers/gpu/drm/exynos/exynos_drm_plane.c |  2 -
  drivers/gpu/drm/i915/intel_crt.c  |  6 ++
  drivers/gpu/drm/i915/intel_display.c  |  9 +--
  drivers/gpu/drm/i915/intel_fbdev.c|  2 +-
  drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |  3 +-
  drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c|  2 -
  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c|  2 -
  drivers/gpu/drm/sti/sti_plane.c   |  9 +--
  drivers/gpu/drm/vc4/vc4_crtc.c|  3 -
  drivers/gpu/drm/virtio/virtgpu_display.c  |  2 -
  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |  6 +-
  drivers/gpu/drm/zte/zx_vou.c  |  2 +-
  include/drm/drm_atomic.h  |  3 -
  22 files changed, 143 insertions(+), 187 deletions(-)



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org

Re: [PATCH 20/23] drm/virtio: Stop updating plane->fb

2018-03-22 Thread Gerd Hoffmann 
> We want to get rid of plane->fb on atomic drivers. Stop setting it.

> - primary->crtc = crtc;
> - cursor->crtc = crtc;

commit msg vs. patch content mismatch here ...

cheers,
  Gerd

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 20/23] drm/virtio: Stop updating plane->fb

2018-03-22 Thread Ville Syrjala 
From: Ville Syrjälä 

We want to get rid of plane->fb on atomic drivers. Stop setting it.

Cc: David Airlie 
Cc: Gerd Hoffmann 
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c 
b/drivers/gpu/drm/virtio/virtgpu_display.c
index 8cc8c34d67f5..42e842ceb53c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -302,8 +302,6 @@ static int vgdev_output_init(struct virtio_gpu_device 
*vgdev, int index)
drm_crtc_init_with_planes(dev, crtc, primary, cursor,
  _gpu_crtc_funcs, NULL);
drm_crtc_helper_add(crtc, _gpu_crtc_helper_funcs);
-   primary->crtc = crtc;
-   cursor->crtc = crtc;
 
drm_connector_init(dev, connector, _gpu_connector_funcs,
   DRM_MODE_CONNECTOR_VIRTUAL);
-- 
2.16.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 00/23] drm: Eliminate plane->fb/crtc usage for atomic drivers

2018-03-22 Thread Ville Syrjala 
From: Ville Syrjälä 

I really just wanted to fix i915 to re-enable its planes afer load
detection (a two line patch). This is what I actually ended up with
after I ran into a framebuffer refcount leak with said two line patch.

I've tested this on a few i915 boxes and so far it's looking
good. Everything else is just compile tested.

Entire series available here:
git://github.com/vsyrjala/linux.git plane_fb_crtc_nuke

Cc: Alex Deucher 
Cc: amd-...@lists.freedesktop.org
Cc: Benjamin Gaignard 
Cc: Boris Brezillon 
Cc: ch...@chris-wilson.co.uk
Cc: "Christian König" 
Cc: Daniel Vetter 
Cc: Dave Airlie 
Cc: David Airlie 
Cc: "David (ChunMing) Zhou" 
Cc: Eric Anholt 
Cc: freedr...@lists.freedesktop.org
Cc: Gerd Hoffmann 
Cc: Harry Wentland 
Cc: Inki Dae 
Cc: Joonyoung Shim 
Cc: Kyungmin Park 
Cc: linux-arm-...@vger.kernel.org
Cc: Maarten Lankhorst 
Cc: martin.pe...@free.fr
Cc: Rob Clark 
Cc: Seung-Woo Kim 
Cc: Shawn Guo 
Cc: Sinclair Yeh 
Cc: Thomas Hellstrom 
Cc: Vincent Abriou 
Cc: virtualization@lists.linux-foundation.org
Cc: VMware Graphics 

Ville Syrjälä (23):
  Revert "drm/atomic-helper: Fix leak in disable_all"
  drm/atomic-helper: Make drm_atomic_helper_disable_all() update the
plane->fb pointers
  drm: Clear crtc->primary->crtc when disabling the crtc via setcrtc()
  drm/atomic-helper: WARN if legacy plane fb pointers are bogus when
committing duplicated state
  drm: Add local 'plane' variable for primary/cursor planes
  drm: Adjust whitespace for legibility
  drm: Make the fb refcount handover less magic
  drm: Use plane->state->fb over plane->fb
  drm/i915: Stop consulting plane->fb
  drm/msm: Stop consulting plane->fb
  drm/sti: Stop consulting plane->fb
  drm/vmwgfx: Stop consulting plane->fb
  drm/zte: Stop consulting plane->fb
  drm/atmel-hlcdc: Stop using plane->fb
  drm: Stop updating plane->crtc/fb/old_fb on atomic drivers
  drm/amdgpu/dc: Stop updating plane->fb
  drm/i915: Stop updating plane->fb/crtc
  drm/exynos: Stop updating plane->crtc
  drm/msm: Stop updating plane->fb/crtc
  drm/virtio: Stop updating plane->fb
  drm/vc4: Stop updating plane->fb/crtc
  drm/i915: Restore planes after load detection
  drm/i915: Make force_load_detect effective even w/ DMI quirks/hotplug

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 -
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 12 +---
 drivers/gpu/drm/drm_atomic.c  | 55 ++--
 drivers/gpu/drm/drm_atomic_helper.c   | 79 ++-
 drivers/gpu/drm/drm_crtc.c| 51 ++-
 drivers/gpu/drm/drm_fb_helper.c   |  7 --
 drivers/gpu/drm/drm_framebuffer.c |  5 --
 drivers/gpu/drm/drm_plane.c   | 64 +++---
 drivers/gpu/drm/drm_plane_helper.c|  4 +-
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  2 -
 drivers/gpu/drm/i915/intel_crt.c  |  6 ++
 drivers/gpu/drm/i915/intel_display.c  |  9 +--
 drivers/gpu/drm/i915/intel_fbdev.c|  2 +-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |  3 +-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c|  2 -
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c|  2 -
 drivers/gpu/drm/sti/sti_plane.c   |  9 +--
 drivers/gpu/drm/vc4/vc4_crtc.c|  3 -
 drivers/gpu/drm/virtio/virtgpu_display.c  |  2 -
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |  6 +-
 drivers/gpu/drm/zte/zx_vou.c  |  2 +-
 include/drm/drm_atomic.h  |  3 -
 22 files changed, 143 insertions(+), 187 deletions(-)

-- 
2.16.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH 1/4] iommu: Add virtio-iommu driver

2018-03-22 Thread Tian, Kevin 
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: Wednesday, March 21, 2018 10:24 PM
> 
> On 21/03/18 13:14, Jean-Philippe Brucker wrote:
> > On 21/03/18 06:43, Tian, Kevin wrote:
> > [...]
> >>> +
> >>> +#include 
> >>> +
> >>> +#define MSI_IOVA_BASE0x800
> >>> +#define MSI_IOVA_LENGTH  0x10
> >>
> >> this is ARM specific, and according to virtio-iommu spec isn't it
> >> better probed on the endpoint instead of hard-coding here?
> >
> > These values are arbitrary, not really ARM-specific even if ARM is the
> > only user yet: we're just reserving a random IOVA region for mapping
> MSIs.
> > It is hard-coded because of the way iommu-dma.c works, but I don't
> quite
> > remember why that allocation isn't dynamic.
> 
> The host kernel needs to have *some* MSI region in place before the
> guest can start configuring interrupts, otherwise it won't know what
> address to give to the underlying hardware. However, as soon as the host
> kernel has picked a region, host userspace needs to know that it can no
> longer use addresses in that region for DMA-able guest memory. It's a
> lot easier when the address is fixed in hardware and the host userspace
> will never be stupid enough to try and VFIO_IOMMU_DMA_MAP it, but in
> the
> more general case where MSI writes undergo IOMMU address translation
> so
> it's an arbitrary IOVA, this has the potential to conflict with stuff
> like guest memory hotplug.
> 
> What we currently have is just the simplest option, with the host kernel
> just picking something up-front and pretending to host userspace that
> it's a fixed hardware address. There's certainly scope for it to be a
> bit more dynamic in the sense of adding an interface to let userspace
> move it around (before attaching any devices, at least), but I don't
> think it's feasible for the host kernel to second-guess userspace enough
> to make it entirely transparent like it is in the DMA API domain case.
> 
> Of course, that's all assuming the host itself is using a virtio-iommu
> (e.g. in a nested virt or emulation scenario). When it's purely within a
> guest then an MSI reservation shouldn't matter so much, since the guest
> won't be anywhere near the real hardware configuration anyway.
> 
> Robin.

Curious since anyway we are defining a new iommu architecture
is it possible to avoid those ARM-specific burden completely? 

Thanks
Kevin

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [virtio-dev] [RFC] virtio-iommu version 0.6

2018-03-22 Thread Tian, Kevin 
> From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com]
> Sent: Wednesday, March 21, 2018 9:14 PM
> 
> Hi Kevin,
> 
> Thanks for the comments
> 
> On 19/03/18 10:03, Tian, Kevin wrote:
> > BYPASS feature bit is not covered in "2.3.1/2.3.2/2.3.3"". Is it
> > intended?
> 
> In my opinion BYPASS is a bit different from the other features: while the
> others are needed for correctness, this one is optional and even if the
> guest supports BYPASS, it should be allowed not to accept it. For security
> reasons it may not want to let endpoints access the whole guest-physical
> address space.

ok, possibly because I'm not familiar with virtio spec convention.
My original feeling is that each feature bit will have a behavior
description regarding to whether device reports and whether 
driver accepts. If no need to cover optional feature, then it's fine
to me. :-)

> 
> > --2.6.2 Device Requirements: Device operations--
> >
> > "If the VIRTIO_IOMMU_F_INPUT_RANGE feature is offered,
> > the device MUST truncate the range described by virt_start
> > and virt_end in requests to ft in the range described by
> > input_range."
> >
> > "If the VIRTIO_IOMMU_F_DOMAIN_BITS is offered, the device
> > MUST ignore bits above domain_bits in field domain of requests."
> >
> > shouldn't device return error upon above situation instead
> > of continuing operation with modified value?
> 
> The intent was to allow device and driver to pass metadata in the top bits
> of pointers and domain IDs, perhaps for a future extension or for
> debugging. I'm not convinced it's useful anymore (and do wonder what my
> initial reasoning was...) Such extension would be determined by a new
> feature bit and the device would know that the fields contain additional
> info, if the driver accepted that feature.
> 
> > --2.6.4 DETACH request--
> >
> > " Detach an endpoint from its domain. When this request
> > completes, the endpoint cannot access any mapping from that
> > domain anymore "
> >
> > Does it make sense to mention BYPASS situation upon this request?
> 
> Sure, I'll add something to clarify that situation
> 
> > --2.6.8.2 Property RESV_MEM --
> >
> > "VIRTIO_IOMMU_RESV_MEM_T_RESERVED (0) Accesses to
> > virtual addresses in this region are not translated by the device.
> > They may either be aborted by the device (or the underlying
> > IOMMU), bypass it, or never even reach it"
> >
> > in 3.3 hardware device assignment you described an example
> > that a reserved range is stolen by host to map the MSI
> > doorbell... such map behavior is not described above.
> 
> Right, we can say that accesses to the region may be used for host
> translation
> 
> > Then comes a question for VIRTIO_IOMMU_RESV_MEM_T_MSI.
> > I know there were quite some discussion around this flag before,
> > but my mental picture now still is a bit difficult to understand its
> > usage based on examples in implementation notes:
> >
> > - for x86, you describe it as indicating MSI bypass for
> > oxfeex. However guest doesn't need to know this fact. Only
> > requirement is to treat it as reserved range (as on bare metal)
> > then T_RESERVED is sufficient for this purpose>
> > - for ARM, either let guest or let host to choose a virtual
> > address for mapping to MSI doorbell. the former doesn't require
> > a reserved range. for the latter also T_RESERVED is enough as
> > the example in hardware device assignment section.
> 
> It might be nicer to have the host decide it, but when the physical IOMMU
> is ARM SMMU, nesting translation complicates things, because the guest
> *has* to create a mapping:

confirm one thing first. v0.6 doesn't support binding to guest IOVA
page table yet. So based on current map/unmap interface, there is 
no such complicity right? stage-1 is just a shadowed translation (IOVA
->HPA) to guest side (IOVA->GPA) with nesting disabled. In that case
the default behavior is host-reserved style.

Then comes nested scenario:

> 
> * The guest is in charge of stage-1. It creates IOVA->GPA mapping for the
>   MSI doorbell. The GPA is mapped to the physical MSI doorbell at
>   stage-2 by the host.

Is it a must that above GPA is mapped to physical MSI doorbell?

Ideally:

1) Host reserves some IOVA range for mapping MSI doorbell
2) the range is reported to user space
3) Qemu reported the range as reserved on endpoints, marked
as T_IDENTITY (a new type to be introduced), meaning guest
needs setup identity mapping in stage-1
4) Then device should be able to ring physical MSI doorbell
5) I'm not sure whether guest still needs to allocate its own
IOVA and mapping to vGIC doorbell in such case...

> 
> * when it writes that IOVA in the virtual MSI-X tables, the host updates
>   the physical MSI-X tables using an ioctl (it can't map the physical
>   tables into the guest, because MSI-X vector data also contain physical
>   device info that shouldn't be known by the guest.)
> 
> So we need support for software-mapped MSIs in the guest anyway. In
>