linux-kernel - Re: refactor the i915 GVT support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0f52ab3e-f50a-041f-f37d-4d2f3febe49b@intel.com>
Date:   Fri, 1 Oct 2021 13:01:41 +0000
From:   "Wang, Zhi A" <zhi.a.wang@...el.com>
To:     Jason Gunthorpe <jgg@...dia.com>
CC:     Luis Chamberlain <mcgrof@...nel.org>, Jessica Yu <jeyu@...nel.org>,
        "De Marchi, Lucas" <lucas.demarchi@...el.com>,
        Zhenyu Wang <zhenyuw@...ux.intel.com>,
        Christoph Hellwig <hch@....de>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        "intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Gerd Hoffmann <kraxel@...hat.com>,
        "Vivi, Rodrigo" <rodrigo.vivi@...el.com>,
        "intel-gvt-dev@...ts.freedesktop.org" 
        <intel-gvt-dev@...ts.freedesktop.org>,
        "Nikula, Jani" <jani.nikula@...el.com>
Subject: Re: refactor the i915 GVT support

On 9/29/21 6:55 PM, Jason Gunthorpe wrote:
> On Wed, Sep 29, 2021 at 06:27:16PM +0000, Wang, Zhi A wrote:
>> On 9/28/21 3:05 PM, Jason Gunthorpe wrote:
>>> On Tue, Sep 28, 2021 at 02:35:06PM +0000, Wang, Zhi A wrote:
>>>
>>>> Yes. I was thinking of the possibility of putting off some work later so
>>>> that we don't need to make a lot of changes. GVT-g needs to take a
>>>> snapshot of GPU registers as the initial virtual states for other vGPUs,
>>>> which requires the initialization happens at a certain early time of
>>>> initialization of i915. I was thinking maybe we can take other patches
>>>> from Christoph like "de-virtualize*" except this one because currently
>>>> we have to maintain a TEST-ONLY patch on our tree to prevent i915 built
>>>> as kernel module.
>>> How about just capture these registers in the main module/device and
>>> not try so hard to isolate it to the gvt stuff?
>> Hi Jason:
>>
>> Thanks for the idea. I am not sure i915 guys would take this idea since
>> that it's only for GVT-g, i915 doesn't use this at all. We need to take
>> a snapshot of both PCI configuration space and MMIO registers before
>> i915 driver starts to touch the HW.
> Given the code is already linked into i915 I don't see there is much
> to object to here. It can remain conditional on the kernel parameter
> as today.
>
> As a general philosophy this would all be much less strange if the
> mdev .ko is truely optional. It should be cleanly seperate from its
> base device and never request_module'd..
>
> In this case auxiliary device might be a good option, have i915 create
> one and the mdev module be loaded against it.
>
> In the mean time is there some shortcut to get this series to move
> ahead? Is patch 4 essential to the rest of the series?
>
> A really awful hack would be to push the pci_driver_register into a
> WQ so that the request_module is guarenteed to not be part of the
> module_init callchain.

Hi Jason and folks:

Thanks so much for the ideas. That sounds great and I was keeping 
thinking how to make progress on this. How about we do like this: We 
don't do request_module("kvmgt") in i915.ko, which resolves the circular 
module dependency. We keep the code of doing snapshot of registers in 
intel_gvt.c. When i915.enable_gvt=1, we do the snapshot. Then we export 
functions for kvmgt.ko in intel_gvt.c to check if gvt in i915 is enabled 
or not and get the snapshots.

How does that sounds? I just need to write another patch and put it on 
top of Christoph's series.

Thanks,

Zhi.

>> Also I was thinking if moving gvt into kvmgt.ko is the right direction.
>> It seems the module loading system in kernel is not designed for "module
>> A loading module B, which needs symbols from module A, in the
>> initialization path of module A".
> Of course not, that is a circular module dependency, it should not be
> that way. The SW layers need to be clean and orderly - meaning the
> i915 module needs to have the minimal amount of code to support the
> mdev module.
>
> Jason