Message-ID: <DM4PR11MB554905AC416FBD055251DE43CA8D9@DM4PR11MB5549.namprd11.prod.outlook.com>
Date:   Thu, 4 Nov 2021 14:51:28 +0000
From:   "Wang, Zhi A" <zhi.a.wang@...el.com>
To:     Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Christoph Hellwig <hch@....de>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        "Vivi, Rodrigo" <rodrigo.vivi@...el.com>,
        Zhenyu Wang <zhenyuw@...ux.intel.com>
CC:     Jason Gunthorpe <jgg@...dia.com>,
        "intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
        "intel-gvt-dev@...ts.freedesktop.org" 
        <intel-gvt-dev@...ts.freedesktop.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: refactor the i915 GVT support and move to the modern mdev API v2

Hi Joonas and Christoph:

We were testing the patch series since Monday and planning to reply after we get the test result. Mostly, we are concerned about patch 6 and how it would affect the test result. Patch 6 changes the timing of loading GVT-g. According to the discussion in the last email, this will break our design of golden MMIO snapshot. Also moving GVT-g code into kvmgt.ko requires the discussion of defining and shrinking the interfaces between i915 and kvmgt.  I guess the ideal way to take Christoph's patch is:

1) We have to figure out how to deal with golden MMIO snapshot. It's a little bit hard to take the re-factor patch before settling this down. In the previous discussion, we would like to find a way to do the snapshot in intel_gvt.c
2) Shrink and refine the exported interfaces because of moving the code into kvmgt.ko
3) Get patches reviewed and merged.

For 1) I was thinking to separated the MMIO handler table from handlers.c and let it build different data structures depends on where it got referenced.
2) That's might require some more discussion.

Is it possible to separate the refactor part from the using new mdev API stuff? So that the design opens in the re-factor patches wouldn’t block the process of mdev API improvement?

Thanks,
Zhi.

-----Original Message-----
From: Joonas Lahtinen <joonas.lahtinen@...ux.intel.com> 
Sent: Thursday, November 4, 2021 2:59 PM
To: Christoph Hellwig <hch@....de>; Jani Nikula <jani.nikula@...ux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@...el.com>; Zhenyu Wang <zhenyuw@...ux.intel.com>; Wang, Zhi A <zhi.a.wang@...el.com>
Cc: Jason Gunthorpe <jgg@...dia.com>; intel-gfx@...ts.freedesktop.org; intel-gvt-dev@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org; linux-kernel@...r.kernel.org
Subject: Re: refactor the i915 GVT support and move to the modern mdev API v2

Hi Zhenyu and Zhi,

Can you have somebody from the GVT team to review the patches that are fully contained in gvt/ ?

I also started discussion on patch 6 which is about defining the interface between the modules. I remember there is prior work to shrink the interface. Do you have links to such patches?

The minimal we should do is to eliminate the double underscore prefixed functions. But I would prefer to have the symbol exports by default so that we can enable the functionality just by loading the module.

Regards, Joonas

Quoting Christoph Hellwig (2021-11-02 09:05:32)
> Hi all,
> 
> the GVT code in the i915 is a bit of a mess right now due to strange 
> abstractions and lots of indirect calls.  This series refactors 
> various bits to clean that up.  The main user visible change is that 
> almost all of the GVT code moves out of the main i915 driver and into 
> the kvmgt module.
> 
> Tested on my Thinkpad with a Kaby Lake CPU and integrated graphics.
> 
> Git tree:
> 
>     git://git.infradead.org/users/hch/misc.git i915-gvt
> 
> Gitweb:
> 
>     
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/i915-g
> vt
> 
> Changes since v1:
>  - rebased on Linux 5.15
>  - allow the kvmgvt module to be loaded at any time and thus solve
>    the deadlock when both i915 amd kvmgvt are modular
>  - include the conversion to the modern mdev API
> 
> Note that I do expect to rebased this again against 5.16-rc1 once 
> released, but I'd like to get this out for review ASAP.
> 
> Diffstat:
>  b/drivers/gpu/drm/i915/Kconfig          |   33 
>  b/drivers/gpu/drm/i915/Makefile         |   31 
>  b/drivers/gpu/drm/i915/gvt/cfg_space.c  |   89 --
>  b/drivers/gpu/drm/i915/gvt/cmd_parser.c |    4 
>  b/drivers/gpu/drm/i915/gvt/dmabuf.c     |   36 -
>  b/drivers/gpu/drm/i915/gvt/execlist.c   |   12 
>  b/drivers/gpu/drm/i915/gvt/gtt.c        |   55 +
>  b/drivers/gpu/drm/i915/gvt/gvt.h        |  125 ++-
>  b/drivers/gpu/drm/i915/gvt/interrupt.c  |   38 +
>  b/drivers/gpu/drm/i915/gvt/kvmgt.c      | 1099 +++++++++++++++-----------------
>  b/drivers/gpu/drm/i915/gvt/mmio.c       |    4 
>  b/drivers/gpu/drm/i915/gvt/opregion.c   |  148 ----
>  b/drivers/gpu/drm/i915/gvt/page_track.c |    8 
>  b/drivers/gpu/drm/i915/gvt/scheduler.c  |   37 -
>  b/drivers/gpu/drm/i915/gvt/trace.h      |    2 
>  b/drivers/gpu/drm/i915/gvt/vgpu.c       |   22 
>  b/drivers/gpu/drm/i915/i915_drv.c       |    7 
>  b/drivers/gpu/drm/i915/i915_drv.h       |    1 
>  b/drivers/gpu/drm/i915/i915_trace.h     |    1 
>  b/drivers/gpu/drm/i915/intel_gvt.c      |  162 +++-
>  b/drivers/gpu/drm/i915/intel_gvt.h      |   17 
>  drivers/gpu/drm/i915/gvt/Makefile       |    9 
>  drivers/gpu/drm/i915/gvt/gvt.c          |  340 ---------
>  drivers/gpu/drm/i915/gvt/hypercall.h    |   82 --
>  drivers/gpu/drm/i915/gvt/mpt.h          |  400 -----------
>  25 files changed, 929 insertions(+), 1833 deletions(-)

Powered by blists - more mailing lists