[Intel-gfx] [PULL] GVT-g device model core

[Zhenyu Wang]

On 2016.10.17 09:33:19 +0200, Daniel Vetter wrote:
> On Mon, Oct 17, 2016 at 09:30:45AM +0200, Daniel Vetter wrote:
> > On Fri, Oct 14, 2016 at 06:30:30PM +0800, Zhenyu Wang wrote:
> > > 
> > > Hi,
> > > 
> > > This is first pull request to merge GVT-g device model in i915
> > > which contains core GVT-g device model work to virtualize GPU
> > > resources. This tries to add feature of Intel GVT-g technology
> > > for full GPU virtualization. This version will support KVM based
> > > virtualization solution named as KVMGT.
> > > 
> > > More background is on official project home: https://01.org/igvt-g
> > > 
> > > To manage mediated device between virtual GPU and physical device it
> > > will rely on VFIO/mdev framework, this version has not included GVT-g
> > > device model integration work for VFIO/mdev yet as VFIO community is
> > > still under work to refine code base. Currently we're basing on
> > > VFIO/mdev v8 patch series (http://www.spinics.net/lists/kvm/msg138515.html)
> > > and doing more testings on that.
> > > 
> > > There're also several KVM change dependences. KVM maintainer has
> > > merged two and we will ensure left hits KVM tree before sending new
> > > pull request to enable that.
> > > 
> > > p.s, There would require some coordinate work for VFIO/mdev. We will
> > > send device model work for that after Alex merged mdev framework in
> > > VFIO tree. Alex has promised to merge that in early of Nov.
> > > 
> > > Let me know if there's any issue with this our first pull request.
> > 
> > Ok applied, but a few things to keep in mind before your next pull
> > request:
> > 
> > - Dont rebase everything 5 seconds before sending out the pull request.
> >   That just invalidates all the testing you've done, so not a good idea.
> >   In general try to avoid rebases as much as possible, and only rebase to
> >   take out a truly embarassing mistake. And then only rebase up to the
> >   patch that needs a hotfix, not your entire tree.
> > 
> > - Similar, don't base your pull requests upon a random commit of the day
> >   (that's why I noticed you rebased). Instead pick something meaningful,
> >   like a tag I (or Dave Airlie or Linus Torvalds) push out. Or another
> >   good option is to base it right on top of the last merge commit from
> >   gvt. Once you've picked a baseline, don't change it (except when you
> >   have a good reason). And if you need a patch from upstream also don't
> >   rebase, just send out a pull request with your current patch pile, and
> >   then continue applying more stuff on top once I merged that.
> >

Sorry for that although we liked to include only core GVT-g device
model in first pull request, we do have continual testing internally
to cover GVT-g features. Will do as you said.

> > - One technical nit on the integration: My idea was that i915 core code
> >   only calls a few specific functions and structures exposed through
> >   intel_gvt.h. But that file now seems to include gvt-internal headers,
> >   which is a bit a mess. Please clean that up in the next pull request:
> > 
> >   * Anything that core i915 code or headers needs must be moved into
> >     intel_gvt.h.
> >   * Everything else, including the 2 gvt includes we now have (gvt/gvt.h
> >     and i915_pvinfo.h) should only be included from code in
> >     drm/i915/gvt.h. So either sprinkle include directives over your source
> >     files for everything, or make gvt/gvt.h the main gvt header that pulls
> >     in everything.
> > 
> >   The idea here is similar to drm core vs. i915: drm core headers never
> >   pull in i915 headers, and all communication happens through the
> >   well-defined interfaces in drm core header files. I think our goal with
> >   gvt should be similar, with all the interfaces being in intel_gvt.h.
> >   Otherwise I fear the submaintainer model will be a bit painful, if we
> >   don't aim for strict separation here.
> >

yeah, that's a little messy, we will try to clean it up.

> > - There's not yet a MAINTAINERS entry for i915/gvt with gvt mailing lists,
> >   git repos and your name on it. Please fix that in the next pull request,
> >   too.

Will add that.

> > 
> > - gvt seems to lack kernel-doc entirely. I think we need at least an
> >   overview file and interface documentation for the stuff in
> >   intel_gvt.[hc]. Please run
> > 
> > 	$ make hmtldocs
> > 
> >   to make sure it all looks pretty (you need to add stanzas in
> >   Documenation/gpu/i915.rst to include things). Another item for the next
> >   pull request please.
> 
> Quick addition: Since this will be a patch touching i915 core code pls
> submit it to intel-gfx for review. You can then apply it to your tree once
> it's reviewed (or Joonas or someone can commit directly to
> drm-intel-next-queued).
>

Will fix kernel-doc too.

> And another item:
> - Please add me to the moderation whitelist of igt-g-dev, I don't want to
>   be spammed by moderation mails every time I reply to your pull requests
>   ;-)
> 

Will ping 01.org admin for that.

> > Also, this is the first time ever I've taken a pull request, so some
> > learning involved on my side too. Please bear with me ;-)
> > 

It's new thing for us to learn obviously. Thanks for the advice!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20161017/d1c69aee/attachment.sig>