Re: [Intel-gfx] [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends

2020-09-20 Thread Daniel Vetter
On Sun, Sep 20, 2020 at 08:23:26AM +0200, Thomas Gleixner wrote:
> On Sat, Sep 19 2020 at 12:37, Daniel Vetter wrote:
> > On Sat, Sep 19, 2020 at 12:35 PM Daniel Vetter  wrote:
> >> I think it should be the case, but I want to double check: Will
> >> copy_*_user be allowed within a kmap_temporary section? This would
> >> allow us to ditch an absolute pile of slowpaths.
> >
> > (coffee just kicked in) copy_*_user is ofc allowed, but if you hit a
> > page fault you get a short read/write. This looks like it would remove
> > the need to handle these in a slowpath, since page faults can now be
> > served in this new kmap_temporary sections. But this sounds too good
> > to be true, so I'm wondering what I'm missing.
> 
> In principle we could allow pagefaults, but not with the currently
> proposed interface which can be called from any context. Obviously if
> called from atomic context it can't handle user page faults.
 
Yeah that's clear, but does the implemention need to disable pagefaults
unconditionally?

> In theory we could make a variant which does not disable pagefaults, but
> that's what kmap() already provides.

Currently we have a bunch of code which roughly does

kmap_atomic();
copy_*_user();
kunmap_atomic();

if (short_copy_user) {
kmap();
copy_*_user(remaining_stuff);
kunmap();
}

And the only reason is that kmap is a notch slower, hence the fastpath. If
we get a kmap which is fast and allows pagefaults (only in contexts that
allow pagefaults already ofc) then we can ditch a pile of kmap users.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends

2020-09-20 Thread Thomas Gleixner
On Sun, Sep 20 2020 at 08:41, Thomas Gleixner wrote:
> On Sat, Sep 19 2020 at 10:18, Linus Torvalds wrote:
>> Maybe I've missed something.  Is it because the new interface still
>> does "pagefault_disable()" perhaps?
>>
>> But does it even need the pagefault_disable() at all? Yes, the
>> *atomic* one obviously needed it. But why does this new one need to
>> disable page faults?
>
> It disables pagefaults because it can be called from atomic and
> non-atomic context. That was the point to get rid of
>
>  if (preeemptible())
>   kmap();
>  else
> kmap_atomic();
>
> If it does not disable pagefaults, then it's just a lightweight variant
> of kmap() for short lived mappings.

Actually most usage sites of kmap atomic do not need page faults to be
disabled at all. As Daniel pointed out the implicit pagefault disable
enforces copy_from_user_inatomic() even in places which are clearly
preemptible task context.

As we need to look at each instance anyway we can add the PF disable
explicitely to the very few places which actually need it.

Thanks,

tglx

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends

2020-09-20 Thread Linus Torvalds
On Sun, Sep 20, 2020 at 1:49 AM Thomas Gleixner  wrote:
>
> Actually most usage sites of kmap atomic do not need page faults to be
> disabled at all.

Right. I think the pagefault disabling has (almost) nothing at all to
do with the kmap() itself - it comes from the "atomic" part, not the
"kmap" part.

I say *almost*, because there is one issue that needs some thought:
the amount of kmap nesting.

The kmap_atomic() interface - and your local/temporary/whatever
versions of it - depends very much inherently on being strictly
nesting. In fact, it depends so much on it that maybe that should be
part of the new name?

It's very wrong to do

addr1 = kmap_atomic();
addr2 = kmap_atomic();
..do something with addr 1..
kunmap_atomic(addr1);
.. do something with addr 2..
kunmap_atomic(addr2);

because the way we allocate the slots is by using a percpu-atomic
inc-return (and we deallocate using dec).

So it's fundamentally a stack.

And that's perfectly fine for page faults - if they do any kmaps,
those will obviously nest.

So the only issue with page faults might be that the stack grows
_larger_. And that might need some thought. We already make the kmap
stack bigger for CONFIG_DEBUG_HIGHMEM, and it's possibly that if we
allow page faults we need to make the kmap stack bigger still.

Btw, looking at the stack code, Ithink your new implementation of it
is a bit scary:

   static inline int kmap_atomic_idx_push(void)
   {
  -   int idx = __this_cpu_inc_return(__kmap_atomic_idx) - 1;
  +   int idx = current->kmap_ctrl.idx++;

and now that 'current->kmap_ctrl.idx' is not atomic wrt

 (a) NMI's (this may be ok, maybe we never do kmaps in NMIs, and with
nesting I think it's fine anyway - the NMI will undo whatever it did)

 (b) the prev/next switch

And that (b) part worries me. You do the kmap_switch_temporary() to
switch the entries, but you do that *separately* from actually
switching 'current' to the new value.

So kmap_switch_temporary() looks safe, but I don't think it actually
is. Because while it first unmaps the old entries and then remaps the
new ones, an interrupt can come in, and at that point it matters what
is *CURRENT*.

And regardless of whether 'current' is 'prev' or 'next', that
kmap_switch_temporary() loop may be doing the wrong thing, depending
on which one had the deeper stack. The interrupt will be using
whatever "current->kmap_ctrl.idx" is, but that might overwrite entries
that are in the process of being restored (if current is still 'prev',
but kmap_switch_temporary() is in the "restore @next's kmaps" pgase),
or it might stomp on entries that have been pte_clear()'ed by the
'prev' thing.

I dunno. The latter may be one of those "it works anyway, it
overwrites things we don't care about", but the former will most
definitely not work.

And it will be completely impossible to debug, because it will depend
on an interrupt that uses kmap_local/atomic/whatever() coming in
_just_ at the right point in the scheduler, and only when the
scheduler has been entered with the right number of kmap entries on
the prev/next stack.

And no developer will ever see this with any amount of debug code
enabled, because it will only hit on legacy platforms that do this
kmap anyway.

So honestly, that code scares me. I think it's buggy. And even if it
"happens to work", it does so for all the wrong reasons, and is very
fragile.

So I would suggest:

 - continue to use an actual per-cpu kmap_atomic_idx

 - make the switching code save the old idx, then unmap the old
entries one by one (while doing the proper "pop" action), and then map
the new entries one by one (while doing the proper "push" action).

which would mean that the only index that is actually ever *USED* is
the percpu one, and it's always up-to-date and pushed/popped for
individual entries, rather than this - imho completely bogus -
optimization where you use "p->kmap_ctrl.idx" directly and very very
unsafely.

Alternatively, that process counter would need about a hundred lines
of commentary about exactly why it's safe. Because I don't think it
is.

   Linus
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends

2020-09-20 Thread Thomas Gleixner
On Sun, Sep 20 2020 at 10:23, Daniel Vetter wrote:

> On Sun, Sep 20, 2020 at 08:23:26AM +0200, Thomas Gleixner wrote:
>> On Sat, Sep 19 2020 at 12:37, Daniel Vetter wrote:
>> > On Sat, Sep 19, 2020 at 12:35 PM Daniel Vetter  wrote:
>> >> I think it should be the case, but I want to double check: Will
>> >> copy_*_user be allowed within a kmap_temporary section? This would
>> >> allow us to ditch an absolute pile of slowpaths.
>> >
>> > (coffee just kicked in) copy_*_user is ofc allowed, but if you hit a
>> > page fault you get a short read/write. This looks like it would remove
>> > the need to handle these in a slowpath, since page faults can now be
>> > served in this new kmap_temporary sections. But this sounds too good
>> > to be true, so I'm wondering what I'm missing.
>> 
>> In principle we could allow pagefaults, but not with the currently
>> proposed interface which can be called from any context. Obviously if
>> called from atomic context it can't handle user page faults.
>  
> Yeah that's clear, but does the implemention need to disable pagefaults
> unconditionally?

As I wrote in the other reply it's not required and the final interface
will neither disable preemption nor pagefaults (except for the atomic
wrapper around it).

Thanks,

tglx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends

2020-09-20 Thread Thomas Gleixner
On Sun, Sep 20 2020 at 09:57, Linus Torvalds wrote:
> On Sun, Sep 20, 2020 at 1:49 AM Thomas Gleixner  wrote:
> Btw, looking at the stack code, Ithink your new implementation of it
> is a bit scary:
>
>static inline int kmap_atomic_idx_push(void)
>{
>   -   int idx = __this_cpu_inc_return(__kmap_atomic_idx) - 1;
>   +   int idx = current->kmap_ctrl.idx++;
>
> and now that 'current->kmap_ctrl.idx' is not atomic wrt
>
>  (a) NMI's (this may be ok, maybe we never do kmaps in NMIs, and with
> nesting I think it's fine anyway - the NMI will undo whatever it did)

Right. Nesting should be a non issue, but I don't think we have
kmap_atomic() in NMI context.

>  (b) the prev/next switch
>
> And that (b) part worries me. You do the kmap_switch_temporary() to
> switch the entries, but you do that *separately* from actually
> switching 'current' to the new value.
>
> So kmap_switch_temporary() looks safe, but I don't think it actually
> is. Because while it first unmaps the old entries and then remaps the
> new ones, an interrupt can come in, and at that point it matters what
> is *CURRENT*.
>
> And regardless of whether 'current' is 'prev' or 'next', that
> kmap_switch_temporary() loop may be doing the wrong thing, depending
> on which one had the deeper stack. The interrupt will be using
> whatever "current->kmap_ctrl.idx" is, but that might overwrite entries
> that are in the process of being restored (if current is still 'prev',
> but kmap_switch_temporary() is in the "restore @next's kmaps" pgase),
> or it might stomp on entries that have been pte_clear()'ed by the
> 'prev' thing.

Duh yes. Never thought about that.

> Alternatively, that process counter would need about a hundred lines
> of commentary about exactly why it's safe. Because I don't think it
> is.

I think the more obvious solution is to split the whole exercise:


  schedule()
 prepare_switch()
unmap()

switch_to()

finish_switch()
map()

That's safe because neither the unmap() nor the map() code changes
kmap_ctrl.idx. So if there is an interrupt coming in between unmap() and
switch_to() then a kmap_local() there will use the next entry. So we
could even do the unmap() with interrupts enabled (preemption disabled).
Same for the map() part.

To explain that we need only a few lines of commentry methinks.

Thanks,

tglx

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends

2020-09-20 Thread Linus Torvalds
On Sun, Sep 20, 2020 at 10:40 AM Thomas Gleixner  wrote:
>
> I think the more obvious solution is to split the whole exercise:
>
>   schedule()
>  prepare_switch()
> unmap()
>
> switch_to()
>
> finish_switch()
> map()

Yeah, that looks much easier to explain. Ack.

   Linus
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends

2020-09-20 Thread Linus Torvalds
On Sun, Sep 20, 2020 at 10:42 AM Linus Torvalds
 wrote:
>
> Yeah, that looks much easier to explain. Ack.

Btw, one thing that might be a good idea at least initially is to add
a check for p->kmap_ctrl.idx being zero at fork, exit and maybe
syscall return time (but that last one may be too cumbersome to really
worry about).

The kmap_atomic() interface basically has a lot of coverage for leaked
as part of all the "might_sleep()" checks sprinkled around,  The new
kmap_temporary/local/whatever wouldn't have that kind of incidental
debug checking, and any leaked kmap indexes would be rather hard to
debug (much) later when they cause index overflows or whatever.

Linus
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault handler

2020-09-20 Thread Zbigniew Kempczyński
On !llc platforms when buffer is mapped with WB using GEM_MMAP_OFFSET
pagefault handler clears its dirty flag. This has consequences
buffer is not flushed on exec before moving to render domain.
So make sure we keep dirty flag for buffers mapped to write until
it leaves cpu domain.

Signed-off-by: Zbigniew Kempczyński 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c 
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 3d69e51f3e4d..3f61a5ccbdd5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -239,6 +239,7 @@ static vm_fault_t vm_fault_cpu(struct vm_fault *vmf)
struct i915_mmap_offset *mmo = area->vm_private_data;
struct drm_i915_gem_object *obj = mmo->obj;
resource_size_t iomap;
+   u16 prev_write_domain;
int err;
 
/* Sanity check that we allow writing into this object */
@@ -246,6 +247,8 @@ static vm_fault_t vm_fault_cpu(struct vm_fault *vmf)
 area->vm_flags & VM_WRITE))
return VM_FAULT_SIGBUS;
 
+   prev_write_domain = obj->write_domain;
+
err = i915_gem_object_pin_pages(obj);
if (err)
goto out;
@@ -268,6 +271,10 @@ static vm_fault_t vm_fault_cpu(struct vm_fault *vmf)
 
i915_gem_object_unpin_pages(obj);
 
+   /* On !llc we need to flush before moving to render domain */
+   if (!obj->cache_coherent && prev_write_domain == I915_GEM_DOMAIN_CPU)
+   obj->cache_dirty = true;
+
 out:
return i915_error_to_vmf_fault(err);
 }
-- 
2.26.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault handler

2020-09-20 Thread Patchwork
== Series Details ==

Series: drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault handler
URL   : https://patchwork.freedesktop.org/series/81881/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
bd0e70d971fe drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault 
handler
-:51: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch 
author ''

total: 0 errors, 1 warnings, 0 checks, 25 lines checked


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault handler

2020-09-20 Thread Patchwork
== Series Details ==

Series: drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault handler
URL   : https://patchwork.freedesktop.org/series/81881/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9025 -> Patchwork_18537


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_18537 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_18537, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/index.html

Possible new issues
---

  Here are the unknown changes that may have been introduced in Patchwork_18537:

### IGT changes ###

 Possible regressions 

  * igt@kms_chamelium@common-hpd-after-suspend:
- fi-icl-u2:  [PASS][1] -> [TIMEOUT][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-icl-u2/igt@kms_chamel...@common-hpd-after-suspend.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-icl-u2/igt@kms_chamel...@common-hpd-after-suspend.html

  
Known issues


  Here are the changes found in Patchwork_18537 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@kms_chamelium@hdmi-crc-fast:
- fi-kbl-7500u:   [PASS][3] -> [FAIL][4] ([i915#227])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-7500u/igt@kms_chamel...@hdmi-crc-fast.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-kbl-7500u/igt@kms_chamel...@hdmi-crc-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-bsw-kefka:   [PASS][5] -> [DMESG-WARN][6] ([i915#1982]) +1 similar 
issue
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-bsw-kefka/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-bsw-kefka/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html

  * igt@vgem_basic@unload:
- fi-kbl-x1275:   [PASS][7] -> [DMESG-WARN][8] ([i915#62] / [i915#92] / 
[i915#95])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-x1275/igt@vgem_ba...@unload.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-kbl-x1275/igt@vgem_ba...@unload.html

  
 Possible fixes 

  * {igt@core_hotunplug@unbind-rebind}:
- fi-kbl-x1275:   [DMESG-WARN][9] ([i915#62] / [i915#92]) -> [PASS][10]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-x1275/igt@core_hotunp...@unbind-rebind.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-kbl-x1275/igt@core_hotunp...@unbind-rebind.html

  * igt@i915_module_load@reload:
- fi-apl-guc: [DMESG-WARN][11] ([i915#1635] / [i915#1982]) -> 
[PASS][12]
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-apl-guc/igt@i915_module_l...@reload.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-apl-guc/igt@i915_module_l...@reload.html

  * igt@kms_busy@basic@flip:
- fi-kbl-x1275:   [DMESG-WARN][13] ([i915#62] / [i915#92] / [i915#95]) 
-> [PASS][14]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-x1275/igt@kms_busy@ba...@flip.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-kbl-x1275/igt@kms_busy@ba...@flip.html

  * igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7500u:   [DMESG-WARN][15] ([i915#2203]) -> [PASS][16]
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-7500u/igt@kms_chamel...@common-hpd-after-suspend.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-kbl-7500u/igt@kms_chamel...@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-bsw-n3050:   [DMESG-WARN][17] ([i915#1982]) -> [PASS][18]
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-bsw-n3050/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-bsw-n3050/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html
- fi-icl-u2:  [DMESG-WARN][19] ([i915#1982]) -> [PASS][20]
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-icl-u2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18537/fi-icl-u2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_flip@basic-flip-vs-dpms@d-dsi1:
- {fi-tgl-dsi}:   [DMESG-WARN][21] ([i915#1982]) -> [PASS][22] +1 
similar issue
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-d...@d-dsi1.html
   [22]: 
https://intel-gfx-ci.01.org/tree

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault handler (rev2)

2020-09-20 Thread Patchwork
== Series Details ==

Series: drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault 
handler (rev2)
URL   : https://patchwork.freedesktop.org/series/81881/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
3d2651f10f09 drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault 
handler
-:51: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch 
author ''

total: 0 errors, 1 warnings, 0 checks, 25 lines checked


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v1 01/12] drm/i915: introduced vgpu pv capability

2020-09-20 Thread Zhang, Xiaolin
On Thu, 2020-09-10 at 16:10 +0300, Jani Nikula wrote:
> On Sat, 05 Sep 2020, Xiaolin Zhang  wrote:
> > to enable vgpu pv feature, pv capability is introduced for guest by
> > new pv_caps member in struct i915_virtual_gpu and for host GVT by
> > new pv_caps register in struct vgt_if.
> > 
> > both of them are used to control different pv feature support in
> > each
> > domain and the final pv caps runtime negotiated between guest and
> > host.
> > 
> > it also adds VGT_CAPS_PV capability BIT useb by guest to query host
> > GVTg
> > whether support any PV feature or not.
> > 
> > Signed-off-by: Xiaolin Zhang 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c |  3 ++
> >  drivers/gpu/drm/i915/i915_drv.h |  1 +
> >  drivers/gpu/drm/i915/i915_pvinfo.h  |  5 ++-
> >  drivers/gpu/drm/i915/i915_vgpu.c| 63
> > -
> >  drivers/gpu/drm/i915/i915_vgpu.h| 10 ++
> >  5 files changed, 80 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 7842199..fd1e0fc 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -48,6 +48,7 @@
> >  #include "i915_trace.h"
> >  #include "intel_pm.h"
> >  #include "intel_sideband.h"
> > +#include "i915_vgpu.h"
> 
> Please keep includes sorted.
Sure. thanks for review. 
> 
> >  
> >  static inline struct drm_i915_private *node_to_i915(struct
> > drm_info_node *node)
> >  {
> > @@ -60,6 +61,8 @@ static int i915_capabilities(struct seq_file *m,
> > void *data)
> > struct drm_printer p = drm_seq_file_printer(m);
> >  
> > seq_printf(m, "pch: %d\n", INTEL_PCH_TYPE(i915));
> > +   if (intel_vgpu_active(i915))
> > +   seq_printf(m, "vgpu pv_caps: 0x%x\n", i915-
> > >vgpu.pv_caps);
> >  
> > intel_device_info_print_static(INTEL_INFO(i915), &p);
> > intel_device_info_print_runtime(RUNTIME_INFO(i915), &p);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index a455752..16d1b51 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -808,6 +808,7 @@ struct i915_virtual_gpu {
> > struct mutex lock; /* serialises sending of g2v_notify command
> > pkts */
> > bool active;
> > u32 caps;
> > +   u32 pv_caps;
> >  };
> >  
> >  struct intel_cdclk_config {
> > diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h
> > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > index 683e97a..8b0dc25 100644
> > --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > @@ -57,6 +57,7 @@ enum vgt_g2v_type {
> >  #define VGT_CAPS_FULL_PPGTTBIT(2)
> >  #define VGT_CAPS_HWSP_EMULATIONBIT(3)
> >  #define VGT_CAPS_HUGE_GTT  BIT(4)
> > +#define VGT_CAPS_PVBIT(5)
> >  
> >  struct vgt_if {
> > u64 magic;  /* VGT_MAGIC */
> > @@ -109,7 +110,9 @@ struct vgt_if {
> > u32 execlist_context_descriptor_lo;
> > u32 execlist_context_descriptor_hi;
> >  
> > -   u32  rsv7[0x200 - 24];/* pad to one page */
> > +   u32 pv_caps;
> > +
> > +   u32  rsv7[0x200 - 25];/* pad to one page */
> >  } __packed;
> >  
> >  #define vgtif_offset(x) (offsetof(struct vgt_if, x))
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> > b/drivers/gpu/drm/i915/i915_vgpu.c
> > index 70fca72..10960125 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.c
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> > @@ -98,7 +98,13 @@ void intel_vgpu_detect(struct drm_i915_private
> > *dev_priv)
> >  
> > dev_priv->vgpu.active = true;
> > mutex_init(&dev_priv->vgpu.lock);
> > -   drm_info(&dev_priv->drm, "Virtual GPU for Intel GVT-g
> > detected.\n");
> > +
> > +   if (!intel_vgpu_detect_pv_caps(dev_priv, shared_area)) {
> > +   DRM_INFO("Virtual GPU for Intel GVT-g detected.\n");
> > +   goto out;
> > +   }
> > +
> > +   DRM_INFO("Virtual GPU for Intel GVT-g detected with PV
> > Optimized.\n");
> >  
> >  out:
> > pci_iounmap(pdev, shared_area);
> > @@ -134,6 +140,18 @@ bool intel_vgpu_has_huge_gtt(struct
> > drm_i915_private *dev_priv)
> > return dev_priv->vgpu.caps & VGT_CAPS_HUGE_GTT;
> >  }
> >  
> > +static bool intel_vgpu_check_pv_cap(struct drm_i915_private
> > *dev_priv,
> > +   enum pv_caps cap)
> > +{
> > +   return (dev_priv->vgpu.active && (dev_priv->vgpu.caps &
> > VGT_CAPS_PV)
> > +   && (dev_priv->vgpu.pv_caps & cap));
> > +}
> > +
> > +static bool intel_vgpu_has_pv_caps(struct drm_i915_private
> > *dev_priv)
> > +{
> > +   return dev_priv->vgpu.caps & VGT_CAPS_PV;
> > +}
> > +
> >  struct _balloon_info_ {
> > /*
> >  * There are up to 2 regions per mappable/unmappable graphic
> > @@ -336,3 +354,46 @@ int intel_vgt_balloon(struct i915_ggtt *ggtt)
> > drm_err(&dev_priv->drm, "VGT balloon fail\n");
> > return ret;
> >  }
> > +
> > +/*
> > + * i915 vgpu PV support for Linux
> > + */
> > +
> 

Re: [Intel-gfx] [PATCH v1 02/12] drm/i915: vgpu shared memory setup for pv support

2020-09-20 Thread Zhang, Xiaolin
On Thu, 2020-09-10 at 16:16 +0300, Jani Nikula wrote:
> On Sat, 05 Sep 2020, Xiaolin Zhang  wrote:
> > To support vgpu pv features, a common shared memory is setup used
> > for
> > communication and data exchange between guest and host GVTg to
> > reduce
> > data access overhead and trap cost.
> > 
> > guest i915 will allocate this common memory (1 page size) and then
> > pass
> > it's physical address to host GVTg through PVINFO register so that
> > host
> > GVTg can access this shared guest page meory without trap cost with
> > hyperviser's facility.
> > 
> > guest i915 will send VGT_G2V_SHARED_PAGE_SETUP notification to host
> > GVTg
> > once shared memory setup succcessfully finished.
> > 
> > the layout of the shared_page also defined as well, the first part
> > is the
> > PV vervsion information used for compabilty support.
> > 
> > Signed-off-by: Xiaolin Zhang 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c|  2 +
> >  drivers/gpu/drm/i915/i915_drv.h|  4 +-
> >  drivers/gpu/drm/i915/i915_pvinfo.h |  5 +-
> >  drivers/gpu/drm/i915/i915_vgpu.c   | 94
> > ++
> >  drivers/gpu/drm/i915/i915_vgpu.h   | 14 ++
> >  5 files changed, 117 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 00292a8..5fbb4ab 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1071,6 +1071,8 @@ static void i915_driver_release(struct
> > drm_device *dev)
> >  
> > disable_rpm_wakeref_asserts(rpm);
> >  
> > +   intel_vgpu_destroy(dev_priv);
> > +
> > i915_gem_driver_release(dev_priv);
> >  
> > intel_memory_regions_driver_release(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 16d1b51..3cde2c5f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -809,7 +809,9 @@ struct i915_virtual_gpu {
> > bool active;
> > u32 caps;
> > u32 pv_caps;
> > -};
> > +
> > +   struct i915_virtual_gpu_pv *pv;
> > +} __packed;
> 
> I'm unsure why this struct should be packed.
Thanks your point it out. it is not necessary. will remove this next
version. 
> 
> >  
> >  struct intel_cdclk_config {
> > unsigned int cdclk, vco, ref, bypass;
> > diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h
> > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > index 8b0dc25..1d44876 100644
> > --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > @@ -48,6 +48,7 @@ enum vgt_g2v_type {
> > VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> > VGT_G2V_EXECLIST_CONTEXT_CREATE,
> > VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> > +   VGT_G2V_SHARED_PAGE_REGISTER,
> > VGT_G2V_MAX,
> >  };
> >  
> > @@ -112,7 +113,9 @@ struct vgt_if {
> >  
> > u32 pv_caps;
> >  
> > -   u32  rsv7[0x200 - 25];/* pad to one page */
> > +   u64 shared_page_gpa;
> > +
> > +   u32  rsv7[0x200 - 27];/* pad to one page */
> >  } __packed;
> >  
> >  #define vgtif_offset(x) (offsetof(struct vgt_if, x))
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> > b/drivers/gpu/drm/i915/i915_vgpu.c
> > index 10960125..8b2b451 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.c
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> > @@ -110,6 +110,17 @@ void intel_vgpu_detect(struct drm_i915_private
> > *dev_priv)
> > pci_iounmap(pdev, shared_area);
> >  }
> >  
> > +void intel_vgpu_destroy(struct drm_i915_private *i915)
> > +{
> > +   struct i915_virtual_gpu_pv *pv = i915->vgpu.pv;
> > +
> > +   if (!intel_vgpu_active(i915) || !pv)
> > +   return;
> > +
> > +   __free_page(virt_to_page(pv->shared_page));
> > +   kfree(pv);
> > +}
> > +
> >  void intel_vgpu_register(struct drm_i915_private *i915)
> >  {
> > /*
> > @@ -360,6 +371,83 @@ int intel_vgt_balloon(struct i915_ggtt *ggtt)
> >   */
> >  
> >  /*
> > + * shared_page setup for VGPU PV features
> > + */
> > +static int intel_vgpu_setup_shared_page(struct drm_i915_private
> > *i915,
> > +   void __iomem *shared_area)
> > +{
> > +   void __iomem *addr;
> > +   struct i915_virtual_gpu_pv *pv;
> > +   struct gvt_shared_page *base;
> > +   u64 gpa;
> > +   u16 ver_maj, ver_min;
> > +   int ret = 0;
> > +
> > +   /* We allocate 1 page shared between guest and GVT for data
> > exchange.
> > +*   ___
> > +*  |version|
> > +*  |___PAGE/8
> > +*  |   |
> > +*  |___PAGE/4
> > +*  |   |
> > +*  |   |
> > +*  |   |
> > +*  |___PAGE/2
> > +*  |   |
> > +*  |   |
> > +*  |   |
> > +*  |  

Re: [Intel-gfx] [PATCH v1 03/12] drm/i915: vgpu pv command buffer transport protocol

2020-09-20 Thread Zhang, Xiaolin
On Thu, 2020-09-10 at 16:20 +0300, Jani Nikula wrote:
> On Sat, 05 Sep 2020, Xiaolin Zhang  wrote:
> > based on the common shared memory, vgpu pv command transport buffer
> > (CTB)
> > protocol is implemented which is a simple pv command buffer ring
> > with pv
> > command descriptor used to perform guest-2-gvt single direction
> > commucation
> > between guest and host GVTg.
> > 
> > with this CTB, guest can send PV command with PV data to host to
> > perform PV
> > commands in host side.
> > 
> > Signed-off-by: Xiaolin Zhang 
> > ---
> >  drivers/gpu/drm/i915/i915_pvinfo.h |   1 +
> >  drivers/gpu/drm/i915/i915_vgpu.c   | 195
> > -
> >  drivers/gpu/drm/i915/i915_vgpu.h   |  53 ++
> >  3 files changed, 247 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h
> > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > index 1d44876..ded93c5 100644
> > --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > @@ -49,6 +49,7 @@ enum vgt_g2v_type {
> > VGT_G2V_EXECLIST_CONTEXT_CREATE,
> > VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> > VGT_G2V_SHARED_PAGE_REGISTER,
> > +   VGT_G2V_PV_SEND_TRIGGER,
> > VGT_G2V_MAX,
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> > b/drivers/gpu/drm/i915/i915_vgpu.c
> > index 8b2b451..e856eff 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.c
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> > @@ -370,6 +370,183 @@ int intel_vgt_balloon(struct i915_ggtt *ggtt)
> >   * i915 vgpu PV support for Linux
> >   */
> >  
> > +/**
> > + * wait_for_desc_update - Wait for the command buffer descriptor
> > update.
> > + * @desc:  buffer descriptor
> > + * @fence: response fence
> > + * @status:placeholder for status
> > + *
> > + * GVTg will update command buffer descriptor with new fence and
> > status
> > + * after processing the command identified by the fence. Wait for
> > + * specified fence and then read from the descriptor status of the
> > + * command.
> > + *
> > + * Return:
> > + * *   0 response received (status is valid)
> > + * *   -ETIMEDOUT no response within hardcoded timeout
> > + */
> > +static int wait_for_desc_update(struct vgpu_pv_ct_buffer_desc
> > *desc,
> > +   u32 fence, u32 *status)
> > +{
> > +   int err;
> > +
> > +#define done (READ_ONCE(desc->fence) == fence)
> > +   err = wait_for_us(done, 5);
> > +   if (err)
> > +   err = wait_for(done, 10);
> > +#undef done
> > +
> > +   if (unlikely(err)) {
> > +   DRM_ERROR("CT: fence %u failed; reported fence=%u\n",
> > +   fence, desc->fence);
> 
> drm_err() please.
Sure. will change all the similar code. thanks. 
> 
> > +   }
> > +
> > +   *status = desc->status;
> 
> Please have a blank line before the return. Recommended throughout
> the
> series.
Sure, will do it in whole series. thanks. 
> 
> > +   return err;
> > +}
> > +
> > +/**
> > + * CTB Guest to GVT request
> > + *
> > + * Format of the CTB Guest to GVT request message is as follows::
> > + *
> > + *  ++-+-+-+-+
> > + *  |   msg[0]   |   [1]   |   [2]   |   ...   |  [n-1]  |
> > + *  ++-+-+-+-+
> > + *  |   MESSAGE  |   MESSAGE PAYLOAD |
> > + *  +   HEADER   +-+-+-+-+
> > + *  ||0|1|   ...   |n|
> > + *  ++=+=+=+=+
> > + *  |  len >= 1  |  FENCE  | request specific data   |
> > + *  +--+-+-+-+-+-+
> > + *
> > + *   ^-len---^
> > + */
> > +static int pv_command_buffer_write(struct i915_virtual_gpu_pv *pv,
> > +   const u32 *action, u32 len /* in dwords */, u32 fence)
> > +{
> > +   struct vgpu_pv_ct_buffer_desc *desc = pv->ctb.desc;
> > +   u32 head = desc->head / 4;  /* in dwords */
> > +   u32 tail = desc->tail / 4;  /* in dwords */
> > +   u32 size = desc->size / 4;  /* in dwords */
> > +   u32 used;   /* in dwords */
> > +   u32 header;
> > +   u32 *cmds = pv->ctb.cmds;
> > +   unsigned int i;
> > +
> > +   GEM_BUG_ON(desc->size % 4);
> > +   GEM_BUG_ON(desc->head % 4);
> > +   GEM_BUG_ON(desc->tail % 4);
> > +   GEM_BUG_ON(tail >= size);
> > +
> > +/* tail == head condition indicates empty */
> > +   if (tail < head)
> > +   used = (size - head) + tail;
> > +   else
> > +   used = tail - head;
> > +
> > +   /* make sure there is a space including extra dw for the fence
> > */
> > +   if (unlikely(used + len + 1 >= size))
> > +   return -ENOSPC;
> > +
> > +   /*
> > +* Write the message. The format is the following:
> > +* DW0: header (including action code)
> > +* DW1: fence
> > +* DW2+: action data
> > +*/
> > +   header = (len << PV_CT_MSG_LEN_SHIFT

Re: [Intel-gfx] [PATCH v1 01/12] drm/i915: introduced vgpu pv capability

2020-09-20 Thread Zhang, Xiaolin
On Thu, 2020-09-10 at 16:10 +0300, Jani Nikula wrote:
> On Sat, 05 Sep 2020, Xiaolin Zhang  wrote:
> > to enable vgpu pv feature, pv capability is introduced for guest by
> > new pv_caps member in struct i915_virtual_gpu and for host GVT by
> > new pv_caps register in struct vgt_if.
> > 
> > both of them are used to control different pv feature support in
> > each
> > domain and the final pv caps runtime negotiated between guest and
> > host.
> > 
> > it also adds VGT_CAPS_PV capability BIT useb by guest to query host
> > GVTg
> > whether support any PV feature or not.
> > 
> > Signed-off-by: Xiaolin Zhang 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c |  3 ++
> >  drivers/gpu/drm/i915/i915_drv.h |  1 +
> >  drivers/gpu/drm/i915/i915_pvinfo.h  |  5 ++-
> >  drivers/gpu/drm/i915/i915_vgpu.c| 63
> > -
> >  drivers/gpu/drm/i915/i915_vgpu.h| 10 ++
> >  5 files changed, 80 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 7842199..fd1e0fc 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -48,6 +48,7 @@
> >  #include "i915_trace.h"
> >  #include "intel_pm.h"
> >  #include "intel_sideband.h"
> > +#include "i915_vgpu.h"
> >  
> >  static inline struct drm_i915_private *node_to_i915(struct
> > drm_info_node *node)
> >  {
> > @@ -60,6 +61,8 @@ static int i915_capabilities(struct seq_file *m,
> > void *data)
> > struct drm_printer p = drm_seq_file_printer(m);
> >  
> > seq_printf(m, "pch: %d\n", INTEL_PCH_TYPE(i915));
> > +   if (intel_vgpu_active(i915))
> > +   seq_printf(m, "vgpu pv_caps: 0x%x\n", i915-
> > >vgpu.pv_caps);
> 
> I think the placement here over-emphasizes the importance of the
> caps. Maybe you also want to print something if vgpu isn't active?
thanks comment. will consider how to print this. 
> 
> >  
> > intel_device_info_print_static(INTEL_INFO(i915), &p);
> > intel_device_info_print_runtime(RUNTIME_INFO(i915), &p);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index a455752..16d1b51 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -808,6 +808,7 @@ struct i915_virtual_gpu {
> > struct mutex lock; /* serialises sending of g2v_notify command
> > pkts */
> > bool active;
> > u32 caps;
> > +   u32 pv_caps;
> >  };
> >  
> >  struct intel_cdclk_config {
> > diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h
> > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > index 683e97a..8b0dc25 100644
> > --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > @@ -57,6 +57,7 @@ enum vgt_g2v_type {
> >  #define VGT_CAPS_FULL_PPGTTBIT(2)
> >  #define VGT_CAPS_HWSP_EMULATIONBIT(3)
> >  #define VGT_CAPS_HUGE_GTT  BIT(4)
> > +#define VGT_CAPS_PVBIT(5)
> >  
> >  struct vgt_if {
> > u64 magic;  /* VGT_MAGIC */
> > @@ -109,7 +110,9 @@ struct vgt_if {
> > u32 execlist_context_descriptor_lo;
> > u32 execlist_context_descriptor_hi;
> >  
> > -   u32  rsv7[0x200 - 24];/* pad to one page */
> > +   u32 pv_caps;
> > +
> > +   u32  rsv7[0x200 - 25];/* pad to one page */
> >  } __packed;
> >  
> >  #define vgtif_offset(x) (offsetof(struct vgt_if, x))
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> > b/drivers/gpu/drm/i915/i915_vgpu.c
> > index 70fca72..10960125 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.c
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> > @@ -98,7 +98,13 @@ void intel_vgpu_detect(struct drm_i915_private
> > *dev_priv)
> >  
> > dev_priv->vgpu.active = true;
> > mutex_init(&dev_priv->vgpu.lock);
> > -   drm_info(&dev_priv->drm, "Virtual GPU for Intel GVT-g
> > detected.\n");
> > +
> > +   if (!intel_vgpu_detect_pv_caps(dev_priv, shared_area)) {
> > +   DRM_INFO("Virtual GPU for Intel GVT-g detected.\n");
> > +   goto out;
> 
> Seems clearer without the goto. It's not like one is an error path,
> right?
> 
> > +   }
> > +
> > +   DRM_INFO("Virtual GPU for Intel GVT-g detected with PV
> > Optimized.\n");
> 
> Please retain use of drm_info().
> 
> >  
> >  out:
> > pci_iounmap(pdev, shared_area);
> > @@ -134,6 +140,18 @@ bool intel_vgpu_has_huge_gtt(struct
> > drm_i915_private *dev_priv)
> > return dev_priv->vgpu.caps & VGT_CAPS_HUGE_GTT;
> >  }
> >  
> > +static bool intel_vgpu_check_pv_cap(struct drm_i915_private
> > *dev_priv,
> > +   enum pv_caps cap)
> 
> The indentation is off here, and all over the place, as reported by
> checkpatch. Please address them everywhere.
> 
> > +{
> > +   return (dev_priv->vgpu.active && (dev_priv->vgpu.caps &
> > VGT_CAPS_PV)
> > +   && (dev_priv->vgpu.pv_caps & cap));
> > +}
> > +
> > +static bool intel_vgpu_has_pv_caps(struct drm_i915_private
> > *dev_priv)
> > +{
> > +   return dev

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault handler (rev2)

2020-09-20 Thread Patchwork
== Series Details ==

Series: drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault 
handler (rev2)
URL   : https://patchwork.freedesktop.org/series/81881/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9025 -> Patchwork_18538


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/index.html

Known issues


  Here are the changes found in Patchwork_18538 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@i915_pm_rpm@basic-pci-d3-state:
- fi-bsw-kefka:   [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-bsw-kefka/igt@i915_pm_...@basic-pci-d3-state.html

  * igt@vgem_basic@unload:
- fi-kbl-x1275:   [PASS][3] -> [DMESG-WARN][4] ([i915#62] / [i915#92])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-x1275/igt@vgem_ba...@unload.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-kbl-x1275/igt@vgem_ba...@unload.html

  
 Possible fixes 

  * {igt@core_hotunplug@unbind-rebind}:
- fi-kbl-x1275:   [DMESG-WARN][5] ([i915#62] / [i915#92]) -> [PASS][6]
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-x1275/igt@core_hotunp...@unbind-rebind.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-kbl-x1275/igt@core_hotunp...@unbind-rebind.html

  * igt@i915_module_load@reload:
- fi-apl-guc: [DMESG-WARN][7] ([i915#1635] / [i915#1982]) -> 
[PASS][8]
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-apl-guc/igt@i915_module_l...@reload.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-apl-guc/igt@i915_module_l...@reload.html

  * igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7500u:   [DMESG-WARN][9] ([i915#2203]) -> [PASS][10]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-7500u/igt@kms_chamel...@common-hpd-after-suspend.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-kbl-7500u/igt@kms_chamel...@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-icl-u2:  [DMESG-WARN][11] ([i915#1982]) -> [PASS][12]
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-icl-u2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-icl-u2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_flip@basic-flip-vs-dpms@d-dsi1:
- {fi-tgl-dsi}:   [DMESG-WARN][13] ([i915#1982]) -> [PASS][14]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-d...@d-dsi1.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-d...@d-dsi1.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2:
- fi-skl-guc: [DMESG-WARN][15] ([i915#2203]) -> [PASS][16]
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vbl...@c-hdmi-a2.html

  
 Warnings 

  * igt@i915_pm_rpm@module-reload:
- fi-kbl-x1275:   [DMESG-FAIL][17] ([i915#62]) -> [DMESG-FAIL][18] 
([i915#62] / [i915#95])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-x1275/igt@i915_pm_...@module-reload.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-kbl-x1275/igt@i915_pm_...@module-reload.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
- fi-kbl-x1275:   [DMESG-WARN][19] ([i915#62] / [i915#92] / [i915#95]) 
-> [DMESG-WARN][20] ([i915#62] / [i915#92]) +3 similar issues
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/fi-kbl-x1275/igt@kms_pipe_crc_ba...@compare-crc-sanitycheck-pipe-c.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/fi-kbl-x1275/igt@kms_pipe_crc_ba...@compare-crc-sanitycheck-pipe-c.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379


Participating hosts (45 -

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault handler (rev2)

2020-09-20 Thread Patchwork
== Series Details ==

Series: drm/i915: Ensure bo is dirty when WB-mapped on !llc in pagefault 
handler (rev2)
URL   : https://patchwork.freedesktop.org/series/81881/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9025_full -> Patchwork_18538_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_18538_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_18538_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_18538_full:

### IGT changes ###

 Possible regressions 

  * igt@kms_flip@2x-nonexisting-fb-interruptible@ac-vga1-hdmi-a1:
- shard-hsw:  NOTRUN -> [INCOMPLETE][1] +1 similar issue
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-hsw4/igt@kms_flip@2x-nonexisting-fb-interrupti...@ac-vga1-hdmi-a1.html

  
Known issues


  Here are the changes found in Patchwork_18538_full that come from known 
issues:

### IGT changes ###

 Issues hit 

  * igt@gem_ctx_persistence@engines-mixed-process@bcs0:
- shard-skl:  [PASS][2] -> [FAIL][3] ([i915#2374])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/shard-skl9/igt@gem_ctx_persistence@engines-mixed-proc...@bcs0.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-skl5/igt@gem_ctx_persistence@engines-mixed-proc...@bcs0.html

  * igt@gem_ctx_persistence@legacy-engines-mixed@vebox:
- shard-kbl:  [PASS][4] -> [DMESG-WARN][5] ([i915#1982])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/shard-kbl3/igt@gem_ctx_persistence@legacy-engines-mi...@vebox.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-kbl4/igt@gem_ctx_persistence@legacy-engines-mi...@vebox.html

  * igt@gem_exec_reloc@basic-many-active@rcs0:
- shard-glk:  [PASS][6] -> [FAIL][7] ([i915#2389]) +3 similar issues
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/shard-glk2/igt@gem_exec_reloc@basic-many-act...@rcs0.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-glk4/igt@gem_exec_reloc@basic-many-act...@rcs0.html

  * igt@gem_huc_copy@huc-copy:
- shard-tglb: [PASS][8] -> [SKIP][9] ([i915#2190])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/shard-tglb7/igt@gem_huc_c...@huc-copy.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-tglb6/igt@gem_huc_c...@huc-copy.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
- shard-hsw:  [PASS][10] -> [FAIL][11] ([i915#96])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/shard-hsw1/igt@kms_cursor_leg...@2x-long-cursor-vs-flip-legacy.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-hsw1/igt@kms_cursor_leg...@2x-long-cursor-vs-flip-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- shard-skl:  [PASS][12] -> [DMESG-WARN][13] ([i915#1982]) +5 
similar issues
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/shard-skl7/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-skl2/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@pipe-b-torture-bo:
- shard-iclb: [PASS][14] -> [DMESG-WARN][15] ([i915#128])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/shard-iclb4/igt@kms_cursor_leg...@pipe-b-torture-bo.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-iclb7/igt@kms_cursor_leg...@pipe-b-torture-bo.html

  * igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled:
- shard-skl:  [PASS][16] -> [FAIL][17] ([i915#177] / [i915#52] / 
[i915#54])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/shard-skl5/igt@kms_draw_...@draw-method-rgb565-pwrite-untiled.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-skl7/igt@kms_draw_...@draw-method-rgb565-pwrite-untiled.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
- shard-tglb: [PASS][18] -> [DMESG-WARN][19] ([i915#1982]) +2 
similar issues
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9025/shard-tglb3/igt@kms_frontbuffer_track...@fbc-1p-primscrn-spr-indfb-fullscreen.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18538/shard-tglb8/igt@kms_frontbuffer_track...@fbc-1p-primscrn-spr-indfb-fullscreen.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-wc:
- shard-skl:  [PASS][20] -> [FAIL][21] ([i915#49]) +1 similar issue
   [20]: 
https://intel-gfx-ci.01.