Re: [PULL] gvt-fixes
On 2024.02.01 16:44:05 +0200, Joonas Lahtinen wrote:
> Hi Zhenyu,
>
> I'm getting the following:
>
> dim: ff833b32ccc4 ("drm/i915: Replace dead 01.org link"): mandatory review
> missing.
> dim: ERROR: issues in commits detected, aborting
>
> Can you fix the commit?
>
Sorry, I missed to add review tag which had actually been done..
Here's the new generated one.
The following changes since commit f9f031dd21a7ce13a13862fa5281d32e1029c70f:
drm/i915/psr: Only allow PSR in LPSP mode on HSW non-ULT (2024-01-25 10:44:13
+0200)
are available in the Git repository at:
https://github.com/intel/gvt-linux tags/gvt-fixes-2024-02-05
for you to fetch changes up to 44e4192f88978e32e4ac08b27141f3767366f79b:
MAINTAINERS: Update Zhi Wang's email address (2024-02-05 11:16:26 +0800)
gvt-fixes-2024-02-05
- Fix broken gvt doc link (Zhenyu)
- Fix one uninitialized variable bug in warning (Dan)
- Update Zhi's new email address in MAINTAINERS file. (Zhi)
Dan Carpenter (1):
drm/i915/gvt: Fix uninitialized variable in handle_mmio()
Zhenyu Wang (1):
drm/i915: Replace dead 01.org link
Zhi Wang (1):
MAINTAINERS: Update Zhi Wang's email address
MAINTAINERS | 4 ++--
drivers/gpu/drm/i915/Kconfig| 2 +-
drivers/gpu/drm/i915/gvt/handlers.c | 3 +--
drivers/gpu/drm/i915/intel_gvt.c| 2 +-
4 files changed, 5 insertions(+), 6 deletions(-)
signature.asc
Description: PGP signature
[PULL] gvt-fixes
Hi, Joonas Here is another gvt-fixes pull which contains fixes on doc link and one uninitialized variable in warning message, also update about Zhi's new mail address in MAINTAINERS. Thanks. --- The following changes since commit f9f031dd21a7ce13a13862fa5281d32e1029c70f: drm/i915/psr: Only allow PSR in LPSP mode on HSW non-ULT (2024-01-25 10:44:13 +0200) are available in the Git repository at: https://github.com/intel/gvt-linux tags/gvt-fixes-2024-01-31 for you to fetch changes up to 88569fa2c3bc83d77a96580da94dd35edee0f893: MAINTAINERS: Update Zhi Wang's email address (2024-01-31 17:19:15 +0800) gvt-fixes-2024-01-31 - Fix broken gvt doc link (Zhenyu) - Fix one uninitialized variable bug in warning (Dan) - Update Zhi's new email address in MAINTAINERS file. (Zhi) Dan Carpenter (1): drm/i915/gvt: Fix uninitialized variable in handle_mmio() Zhenyu Wang (1): drm/i915: Replace dead 01.org link Zhi Wang (1): MAINTAINERS: Update Zhi Wang's email address MAINTAINERS | 4 ++-- drivers/gpu/drm/i915/Kconfig| 2 +- drivers/gpu/drm/i915/gvt/handlers.c | 3 +-- drivers/gpu/drm/i915/intel_gvt.c| 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) signature.asc Description: PGP signature
Re: [PATCH] drm/i915/gvt: Fix uninitialized variable in handle_mmio()
On 2024.01.26 11:41:47 +0300, Dan Carpenter wrote:
> This code prints the wrong variable in the warning message. It should
> print "i" instead of "info->offset". On the first iteration "info" is
> uninitialized leading to a crash and on subsequent iterations it prints
> the previous offset instead of the current one.
>
> Fixes: e0f74ed4634d ("i915/gvt: Separate the MMIO tracking table from GVT-g")
> Signed-off-by: Dan Carpenter
> ---
> drivers/gpu/drm/i915/gvt/handlers.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index 90f6c1ece57d..efcb00472be2 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -2849,8 +2849,7 @@ static int handle_mmio(struct intel_gvt_mmio_table_iter
> *iter, u32 offset,
> for (i = start; i < end; i += 4) {
> p = intel_gvt_find_mmio_info(gvt, i);
> if (p) {
> - WARN(1, "dup mmio definition offset %x\n",
> - info->offset);
> + WARN(1, "dup mmio definition offset %x\n", i);
>
> /* We return -EEXIST here to make GVT-g load fail.
>* So duplicated MMIO can be found as soon as
> --
> 2.43.0
>
Thanks for the fix.
Reviewed-by: Zhenyu Wang
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH v2 1/4] i915: make inject_virtual_interrupt() void
On 2023.11.22 13:48:22 +0100, Christian Brauner wrote:
> The single caller of inject_virtual_interrupt() ignores the return value
> anyway. This allows us to simplify eventfd_signal() in follow-up
> patches.
>
> Signed-off-by: Christian Brauner
> ---
> drivers/gpu/drm/i915/gvt/interrupt.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c
> b/drivers/gpu/drm/i915/gvt/interrupt.c
> index de3f5903d1a7..9665876b4b13 100644
> --- a/drivers/gpu/drm/i915/gvt/interrupt.c
> +++ b/drivers/gpu/drm/i915/gvt/interrupt.c
> @@ -422,7 +422,7 @@ static void init_irq_map(struct intel_gvt_irq *irq)
> #define MSI_CAP_DATA(offset) (offset + 8)
> #define MSI_CAP_EN 0x1
>
> -static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
> +static void inject_virtual_interrupt(struct intel_vgpu *vgpu)
> {
> unsigned long offset = vgpu->gvt->device_info.msi_cap_offset;
> u16 control, data;
> @@ -434,10 +434,10 @@ static int inject_virtual_interrupt(struct intel_vgpu
> *vgpu)
>
> /* Do not generate MSI if MSIEN is disabled */
> if (!(control & MSI_CAP_EN))
> - return 0;
> + return;
>
> if (WARN(control & GENMASK(15, 1), "only support one MSI format\n"))
> - return -EINVAL;
> + return;
>
> trace_inject_msi(vgpu->id, addr, data);
>
> @@ -451,10 +451,10 @@ static int inject_virtual_interrupt(struct intel_vgpu
> *vgpu)
>* returned and don't inject interrupt into guest.
>*/
> if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
> - return -ESRCH;
> - if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger, 1) != 1)
> - return -EFAULT;
> - return 0;
> + return;
> + if (!vgpu->msi_trigger)
> + return;
> + eventfd_signal(vgpu->msi_trigger, 1);
> }
I think it's a little simpler to write as
if (vgpu->msi_trigger)
eventfd_signal(vgpu->msi_trigger, 1);
Looks fine with me.
Reviewed-by: Zhenyu Wang
Thanks!
>
> static void propagate_event(struct intel_gvt_irq *irq,
>
> --
> 2.42.0
>
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH 1/4] drm/i915/gvt: remove unused to_gvt() and reduce includes
On 2023.10.11 10:04:09 +0300, Jani Nikula wrote: > On Wed, 11 Oct 2023, Zhenyu Wang wrote: > > On 2023.10.04 15:54:11 +0300, Jani Nikula wrote: > >> On Tue, 26 Sep 2023, Jani Nikula wrote: > >> > gvt.h has no need to include i915_drv.h once the unused to_gvt() has > >> > been removed. > >> > > >> > Signed-off-by: Jani Nikula > >> > >> Zhenyu, Zhi, ping? > >> > > > > Sorry for late reply, as last week was full holiday here. > > > > Reviewed-by: Zhenyu Wang > > > > I don't think I need to do extra pick and pull request for this or > > let me know if you has question. > > Did you pick them up to gvt-next or shall I pick them up to > drm-intel-next? > > If the former, I think I'd actually like a pull request, because > otherwise the trees will be out-of-sync for a long time. > Sorry, I mean it's fine for me if you directly pick them for drm-intel-next. thanks signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 1/4] drm/i915/gvt: remove unused to_gvt() and reduce includes
On 2023.10.04 15:54:11 +0300, Jani Nikula wrote:
> On Tue, 26 Sep 2023, Jani Nikula wrote:
> > gvt.h has no need to include i915_drv.h once the unused to_gvt() has
> > been removed.
> >
> > Signed-off-by: Jani Nikula
>
> Zhenyu, Zhi, ping?
>
Sorry for late reply, as last week was full holiday here.
Reviewed-by: Zhenyu Wang
I don't think I need to do extra pick and pull request for this or
let me know if you has question.
Thanks!
>
>
>
> > ---
> > drivers/gpu/drm/i915/gvt/gvt.h | 7 +--
> > 1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > index 53a0a42a50db..3a0624fe63bf 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > @@ -39,7 +39,7 @@
> >
> > #include
> >
> > -#include "i915_drv.h"
> > +#include "gt/intel_gt.h"
> > #include "intel_gvt.h"
> >
> > #include "debug.h"
> > @@ -368,11 +368,6 @@ struct intel_gvt {
> > struct dentry *debugfs_root;
> > };
> >
> > -static inline struct intel_gvt *to_gvt(struct drm_i915_private *i915)
> > -{
> > - return i915->gvt;
> > -}
> > -
> > enum {
> > /* Scheduling trigger by timer */
> > INTEL_GVT_REQUEST_SCHED = 0,
>
> --
> Jani Nikula, Intel
signature.asc
Description: PGP signature
[Intel-gfx] [PATCH] drm/i915: Replace dead 01.org link
01.org is dead so replace old gvt link with current wiki page. Signed-off-by: Zhenyu Wang --- MAINTAINERS | 2 +- drivers/gpu/drm/i915/Kconfig | 2 +- drivers/gpu/drm/i915/intel_gvt.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index d516295978a4..805d33a107aa 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10436,7 +10436,7 @@ M: Zhi Wang L: intel-gvt-...@lists.freedesktop.org L: intel-gfx@lists.freedesktop.org S: Supported -W: https://01.org/igvt-g +W: https://github.com/intel/gvt-linux/wiki T: git https://github.com/intel/gvt-linux.git F: drivers/gpu/drm/i915/gvt/ diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 01b5a8272a27..854255966d3d 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -140,7 +140,7 @@ config DRM_I915_GVT_KVMGT Note that this driver only supports newer device from Broadwell on. For further information and setup guide, you can visit: - http://01.org/igvt-g. + https://github.com/intel/gvt-linux/wiki. If in doubt, say "N". diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c index e98b6d69a91a..9b6d87c8b583 100644 --- a/drivers/gpu/drm/i915/intel_gvt.c +++ b/drivers/gpu/drm/i915/intel_gvt.c @@ -41,7 +41,7 @@ * To virtualize GPU resources GVT-g driver depends on hypervisor technology * e.g KVM/VFIO/mdev, Xen, etc. to provide resource access trapping capability * and be virtualized within GVT-g device module. More architectural design - * doc is available on https://01.org/group/2230/documentation-list. + * doc is available on https://github.com/intel/gvt-linux/wiki. */ static LIST_HEAD(intel_gvt_devices); -- 2.40.1
[Intel-gfx] [PULL] gvt-fixes
Hi, Here is one gvt fix for bug in AUX CH register message length get. Please help to pick. Thanks! -- The following changes since commit e354f67733115b4453268f61e6e072e9b1ea7a2f: drm/i915: Fix an error handling path in igt_write_huge() (2023-07-25 08:38:12 +0100) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-fixes-2023-08-02 for you to fetch changes up to 46d14e17095237007b59f56aae2d81ae2dcb0f93: drm/i915/gvt: Fix bug in getting msg length in AUX CH registers handler (2023-08-01 11:21:09 +0800) gvt-fixes-2023-08-02 - Fix bug to get AUX CH register message length (Yan) Yan Zhao (1): drm/i915/gvt: Fix bug in getting msg length in AUX CH registers handler drivers/gpu/drm/i915/gvt/edid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Fix bug in getting msg length in AUX CH registers handler
On 2023.07.31 19:20:33 +0800, Yan Zhao wrote:
> Msg length should be obtained from value written to AUX_CH_CTL register
> rather than from enum type of the register.
>
> Commit 0cad796a2269 ("drm/i915: Use REG_BIT() & co. for AUX CH registers")
> incorrectly calculates the msg_length from reg type and yields below
> warning in intel_gvt_i2c_handle_aux_ch_write():
> "i915 :00:02.0: drm_WARN_ON(msg_length != 4)".
>
> Fixes: 0cad796a2269 ("drm/i915: Use REG_BIT() & co. for AUX CH registers")
> Signed-off-by: Yan Zhao
> ---
Thanks for the fix!
Reviewed-by: Zhenyu Wang
> drivers/gpu/drm/i915/gvt/edid.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/edid.c b/drivers/gpu/drm/i915/gvt/edid.c
> index 2a0438f12a14..af9afdb53c7f 100644
> --- a/drivers/gpu/drm/i915/gvt/edid.c
> +++ b/drivers/gpu/drm/i915/gvt/edid.c
> @@ -491,7 +491,7 @@ void intel_gvt_i2c_handle_aux_ch_write(struct intel_vgpu
> *vgpu,
> return;
> }
>
> - msg_length = REG_FIELD_GET(DP_AUX_CH_CTL_MESSAGE_SIZE_MASK, reg);
> + msg_length = REG_FIELD_GET(DP_AUX_CH_CTL_MESSAGE_SIZE_MASK, value);
>
> // check the msg in DATA register.
> msg = vgpu_vreg(vgpu, offset + 4);
> --
> 2.17.1
>
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH RESEND] drm/i915/gvt: remove unused variable gma_bottom in command parser
On 2023.05.31 12:27:11 +0300, Jani Nikula wrote:
> On Wed, 31 May 2023, Zhenyu Wang wrote:
> > On 2023.05.31 02:04:11 +, Zhi Wang wrote:
> >> Remove unused variable gma_bottom in scan_workload() and scan_wa_ctx().
> >> commit be1da7070aea ("drm/i915/gvt: vGPU command scanner") introduces
> >> gma_bottom in several functions to calculate the size of the command
> >> buffer. However, some of them are set but actually unused.
> >>
> >> When compiling the code with ccflags -Wunused-but-set-variable, gcc
> >> throws warnings.
> >>
> >> Remove unused variables to avoid the gcc warnings. Tested via compiling
> >> the code with ccflags -Wunused-but-set-variable.
> >>
> >> Fixes: be1da7070aea ("drm/i915/gvt: vGPU command scanner")
> >> Suggested-by: Jani Nikula
> >> Cc: Zhenyu Wang
> >> Cc: intel-gvt-...@lists.freedesktop.org
> >> Signed-off-by: Zhi Wang
> >> ---
> >
> > Good with me. I think I also caught this before but never send the change..
> > Reviewed-by: Zhenyu Wang
>
> I'd like to pick this up via drm-intel-next if that's all right.
>
Sorry for missed this. Pls feel free to do so. I'll sync to check anything left
in gvt tree.
Thanks.
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH RESEND] drm/i915/gvt: remove unused variable gma_bottom in command parser
On 2023.05.31 02:04:11 +, Zhi Wang wrote:
> Remove unused variable gma_bottom in scan_workload() and scan_wa_ctx().
> commit be1da7070aea ("drm/i915/gvt: vGPU command scanner") introduces
> gma_bottom in several functions to calculate the size of the command
> buffer. However, some of them are set but actually unused.
>
> When compiling the code with ccflags -Wunused-but-set-variable, gcc
> throws warnings.
>
> Remove unused variables to avoid the gcc warnings. Tested via compiling
> the code with ccflags -Wunused-but-set-variable.
>
> Fixes: be1da7070aea ("drm/i915/gvt: vGPU command scanner")
> Suggested-by: Jani Nikula
> Cc: Zhenyu Wang
> Cc: intel-gvt-...@lists.freedesktop.org
> Signed-off-by: Zhi Wang
> ---
Good with me. I think I also caught this before but never send the change..
Reviewed-by: Zhenyu Wang
> drivers/gpu/drm/i915/gvt/cmd_parser.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index 3c4ae1da0d41..05f9348b7a9d 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -2833,7 +2833,7 @@ static int command_scan(struct parser_exec_state *s,
>
> static int scan_workload(struct intel_vgpu_workload *workload)
> {
> - unsigned long gma_head, gma_tail, gma_bottom;
> + unsigned long gma_head, gma_tail;
> struct parser_exec_state s;
> int ret = 0;
>
> @@ -2843,7 +2843,6 @@ static int scan_workload(struct intel_vgpu_workload
> *workload)
>
> gma_head = workload->rb_start + workload->rb_head;
> gma_tail = workload->rb_start + workload->rb_tail;
> - gma_bottom = workload->rb_start + _RING_CTL_BUF_SIZE(workload->rb_ctl);
>
> s.buf_type = RING_BUFFER_INSTRUCTION;
> s.buf_addr_type = GTT_BUFFER;
> @@ -2874,7 +2873,7 @@ static int scan_workload(struct intel_vgpu_workload
> *workload)
> static int scan_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx)
> {
>
> - unsigned long gma_head, gma_tail, gma_bottom, ring_size, ring_tail;
> + unsigned long gma_head, gma_tail, ring_size, ring_tail;
> struct parser_exec_state s;
> int ret = 0;
> struct intel_vgpu_workload *workload = container_of(wa_ctx,
> @@ -2891,7 +2890,6 @@ static int scan_wa_ctx(struct intel_shadow_wa_ctx
> *wa_ctx)
> PAGE_SIZE);
> gma_head = wa_ctx->indirect_ctx.guest_gma;
> gma_tail = wa_ctx->indirect_ctx.guest_gma + ring_tail;
> - gma_bottom = wa_ctx->indirect_ctx.guest_gma + ring_size;
>
> s.buf_type = RING_BUFFER_INSTRUCTION;
> s.buf_addr_type = GTT_BUFFER;
> --
> 2.25.1
>
signature.asc
Description: PGP signature
[Intel-gfx] [PULL] gvt-next-fixes
Hi, Here's one single change for gvt to use idr based dmabuf object reference instead of old adhoc code. We've verified no regression for internal test. Thanks. -- The following changes since commit d944eafed618a8507270b324ad9d5405bb7f0b3e: drm/i915: Check pipe source size when using skl+ scalers (2023-04-24 14:48:42 +0300) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-next-fixes-2023-04-28 for you to fetch changes up to 004040fdb55fa55463730c95d1384cb67f9396c3: drm/i915/gvt: Make use of idr_find and idr_for_each_entry in dmabuf (2023-04-28 15:21:17 +0800) gvt-next-fixes-2023-04-28 - Use idr based dmabuf object reference (Cai Huoqing) Cai Huoqing (1): drm/i915/gvt: Make use of idr_find and idr_for_each_entry in dmabuf drivers/gpu/drm/i915/gvt/dmabuf.c | 58 --- drivers/gpu/drm/i915/gvt/dmabuf.h | 1 - drivers/gpu/drm/i915/gvt/gvt.h| 1 - drivers/gpu/drm/i915/gvt/vgpu.c | 1 - 4 files changed, 12 insertions(+), 49 deletions(-) signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: Make use of idr_find and idr_for_each_entry in dmabuf
On 2023.03.03 22:07:18 +0800, Cai Huoqing wrote:
> This patch uses the already existing IDR mechanism to simplify
> and improve the dmabuf code.
>
> Using 'vgpu.object_idr' directly instead of 'dmabuf_obj_list_head'
> or 'dmabuf.list', because the dmabuf_obj can be found by 'idr_find'
> or 'idr_for_each_entry'.
>
> Signed-off-by: Cai Huoqing
> ---
> v1->v2:
> 1.Use idr_find to get the target one and free it instead of free all
> dma objs.
> 2.Revert the original code 'ret' related
> 3.Add '&& !idr_is_empty()' like the original code '&& !list_empty()'
Looks good to me. I'll send it for some regression test before upstream.
Reviewed-by: Zhenyu Wang
Thanks!
>
> v1 link:
>
> https://lore.kernel.org/lkml/20230302115318.79487-1-cai.huoq...@linux.dev/
>
> drivers/gpu/drm/i915/gvt/dmabuf.c | 58 +++
> drivers/gpu/drm/i915/gvt/dmabuf.h | 1 -
> drivers/gpu/drm/i915/gvt/gvt.h| 1 -
> drivers/gpu/drm/i915/gvt/vgpu.c | 1 -
> 4 files changed, 12 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
> b/drivers/gpu/drm/i915/gvt/dmabuf.c
> index 6834f9fe40cf..cf619b1ed3ad 100644
> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> @@ -133,21 +133,15 @@ static void dmabuf_gem_object_free(struct kref *kref)
> struct intel_vgpu_dmabuf_obj *obj =
> container_of(kref, struct intel_vgpu_dmabuf_obj, kref);
> struct intel_vgpu *vgpu = obj->vgpu;
> - struct list_head *pos;
> struct intel_vgpu_dmabuf_obj *dmabuf_obj;
>
> if (vgpu && test_bit(INTEL_VGPU_STATUS_ACTIVE, vgpu->status) &&
> - !list_empty(>dmabuf_obj_list_head)) {
> - list_for_each(pos, >dmabuf_obj_list_head) {
> - dmabuf_obj = list_entry(pos, struct
> intel_vgpu_dmabuf_obj, list);
> - if (dmabuf_obj == obj) {
> - list_del(pos);
> - idr_remove(>object_idr,
> -dmabuf_obj->dmabuf_id);
> - kfree(dmabuf_obj->info);
> - kfree(dmabuf_obj);
> - break;
> - }
> + !idr_is_empty(>object_idr)) {
> + dmabuf_obj = idr_find(>object_idr, obj->dmabuf_id);
> + if (dmabuf_obj) {
> + idr_remove(>object_idr, obj->dmabuf_id);
> + kfree(dmabuf_obj->info);
> + kfree(dmabuf_obj);
> }
> } else {
> /* Free the orphan dmabuf_objs here */
> @@ -340,13 +334,12 @@ static struct intel_vgpu_dmabuf_obj *
> pick_dmabuf_by_info(struct intel_vgpu *vgpu,
> struct intel_vgpu_fb_info *latest_info)
> {
> - struct list_head *pos;
> struct intel_vgpu_fb_info *fb_info;
> struct intel_vgpu_dmabuf_obj *dmabuf_obj = NULL;
> struct intel_vgpu_dmabuf_obj *ret = NULL;
> + int id;
>
> - list_for_each(pos, >dmabuf_obj_list_head) {
> - dmabuf_obj = list_entry(pos, struct intel_vgpu_dmabuf_obj,
> list);
> + idr_for_each_entry(>object_idr, dmabuf_obj, id) {
> if (!dmabuf_obj->info)
> continue;
>
> @@ -366,24 +359,6 @@ pick_dmabuf_by_info(struct intel_vgpu *vgpu,
> return ret;
> }
>
> -static struct intel_vgpu_dmabuf_obj *
> -pick_dmabuf_by_num(struct intel_vgpu *vgpu, u32 id)
> -{
> - struct list_head *pos;
> - struct intel_vgpu_dmabuf_obj *dmabuf_obj = NULL;
> - struct intel_vgpu_dmabuf_obj *ret = NULL;
> -
> - list_for_each(pos, >dmabuf_obj_list_head) {
> - dmabuf_obj = list_entry(pos, struct intel_vgpu_dmabuf_obj,
> list);
> - if (dmabuf_obj->dmabuf_id == id) {
> - ret = dmabuf_obj;
> - break;
> - }
> - }
> -
> - return ret;
> -}
> -
> static void update_fb_info(struct vfio_device_gfx_plane_info *gvt_dmabuf,
> struct intel_vgpu_fb_info *fb_info)
> {
> @@ -477,11 +452,6 @@ int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void
> *args)
>
> update_fb_info(gfx_plane_info, _info);
>
> - INIT_LIST_HEAD(_obj->list);
> - mutex_lock(>dmabuf_lock);
> - list_add_tail(_obj->list, >dmabuf_obj_list_head);
> - mutex_unlock(>dmabuf_lock);
> -
> gvt_dbg_dpy("vgpu%d: %s new dmabuf_obj ref %d, id %d\n", vgpu->id,
> __func__, kref_read(_obj-&g
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Make use of idr_find and idr_for_each_entry in dmabuf
On 2023.03.02 19:53:18 +0800, Cai Huoqing wrote:
> This patch uses the already existing IDR mechanism to simplify
> and improve the dmabuf code.
>
> Using 'vgpu.object_idr' directly instead of 'dmabuf_obj_list_head'
> or 'dmabuf.list', because the dmabuf_obj can be found by 'idr_find'
> or 'idr_for_each_entry'.
>
> Signed-off-by: Cai Huoqing
> ---
> drivers/gpu/drm/i915/gvt/dmabuf.c | 69 +++
> drivers/gpu/drm/i915/gvt/dmabuf.h | 1 -
> drivers/gpu/drm/i915/gvt/gvt.h| 1 -
> drivers/gpu/drm/i915/gvt/vgpu.c | 1 -
> 4 files changed, 16 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
> b/drivers/gpu/drm/i915/gvt/dmabuf.c
> index 6834f9fe40cf..7933bd843ae8 100644
> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> @@ -133,21 +133,15 @@ static void dmabuf_gem_object_free(struct kref *kref)
> struct intel_vgpu_dmabuf_obj *obj =
> container_of(kref, struct intel_vgpu_dmabuf_obj, kref);
> struct intel_vgpu *vgpu = obj->vgpu;
> - struct list_head *pos;
> struct intel_vgpu_dmabuf_obj *dmabuf_obj;
> + int id;
>
> - if (vgpu && test_bit(INTEL_VGPU_STATUS_ACTIVE, vgpu->status) &&
> - !list_empty(>dmabuf_obj_list_head)) {
> - list_for_each(pos, >dmabuf_obj_list_head) {
> - dmabuf_obj = list_entry(pos, struct
> intel_vgpu_dmabuf_obj, list);
> - if (dmabuf_obj == obj) {
> - list_del(pos);
> - idr_remove(>object_idr,
> -dmabuf_obj->dmabuf_id);
> - kfree(dmabuf_obj->info);
> - kfree(dmabuf_obj);
> - break;
> - }
> + if (vgpu && test_bit(INTEL_VGPU_STATUS_ACTIVE, vgpu->status)) {
> + idr_for_each_entry(>object_idr, dmabuf_obj, id) {
> + idr_remove(>object_idr, id);
> + kfree(dmabuf_obj->info);
> + kfree(dmabuf_obj);
This is wrong, it is not to free all dmabuf objects, but just for target one.
> + break;
> }
> } else {
> /* Free the orphan dmabuf_objs here */
> @@ -340,13 +334,11 @@ static struct intel_vgpu_dmabuf_obj *
> pick_dmabuf_by_info(struct intel_vgpu *vgpu,
> struct intel_vgpu_fb_info *latest_info)
> {
> - struct list_head *pos;
> struct intel_vgpu_fb_info *fb_info;
> struct intel_vgpu_dmabuf_obj *dmabuf_obj = NULL;
> - struct intel_vgpu_dmabuf_obj *ret = NULL;
> + int id;
>
> - list_for_each(pos, >dmabuf_obj_list_head) {
> - dmabuf_obj = list_entry(pos, struct intel_vgpu_dmabuf_obj,
> list);
> + idr_for_each_entry(>object_idr, dmabuf_obj, id) {
> if (!dmabuf_obj->info)
> continue;
>
> @@ -357,31 +349,11 @@ pick_dmabuf_by_info(struct intel_vgpu *vgpu,
> (fb_info->drm_format_mod == latest_info->drm_format_mod) &&
> (fb_info->drm_format == latest_info->drm_format) &&
> (fb_info->width == latest_info->width) &&
> - (fb_info->height == latest_info->height)) {
> - ret = dmabuf_obj;
> - break;
> - }
Maybe just keep original code's use of extra ret to not include this cumbersome
diff?
> - }
> -
> - return ret;
> -}
> -
> -static struct intel_vgpu_dmabuf_obj *
> -pick_dmabuf_by_num(struct intel_vgpu *vgpu, u32 id)
> -{
> - struct list_head *pos;
> - struct intel_vgpu_dmabuf_obj *dmabuf_obj = NULL;
> - struct intel_vgpu_dmabuf_obj *ret = NULL;
> -
> - list_for_each(pos, >dmabuf_obj_list_head) {
> - dmabuf_obj = list_entry(pos, struct intel_vgpu_dmabuf_obj,
> list);
> - if (dmabuf_obj->dmabuf_id == id) {
> - ret = dmabuf_obj;
> - break;
> - }
> + (fb_info->height == latest_info->height))
> + return dmabuf_obj;
> }
>
> - return ret;
> + return dmabuf_obj;
> }
>
> static void update_fb_info(struct vfio_device_gfx_plane_info *gvt_dmabuf,
> @@ -477,11 +449,6 @@ int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void
> *args)
>
> update_fb_info(gfx_plane_info, _info);
>
> - INIT_LIST_HEAD(_obj->list);
> - mutex_lock(>dmabuf_lock);
> - list_add_tail(_obj->list, >dmabuf_obj_list_head);
> - mutex_unlock(>dmabuf_lock);
> -
> gvt_dbg_dpy("vgpu%d: %s new dmabuf_obj ref %d, id %d\n", vgpu->id,
> __func__, kref_read(_obj->kref), ret);
>
> @@ -508,7 +475,7 @@ int intel_vgpu_get_dmabuf(struct intel_vgpu *vgpu,
> unsigned int dmabuf_id)
>
> mutex_lock(>dmabuf_lock);
>
> - dmabuf_obj = pick_dmabuf_by_num(vgpu, dmabuf_id);
> + dmabuf_obj =
[Intel-gfx] [PULL] gvt-next-fixes
Hi, This is what are on gvt tree now for next kernel, including fixes for gvt debugfs, one kconfig symbol fix for menu presentation and misc typo fixes. Please check details below. This is generated against current drm-intel-next-fixes. Thanks! -- The following changes since commit 8038510b1fe443ffbc0e356db5f47cbb8678a594: drm/i915: Fix system suspend without fbdev being initialized (2023-02-15 17:33:07 +0200) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-next-fixes-2023-02-23 for you to fetch changes up to 0b93efca3659f6d55ed31cff6722dca5f6e4d6e2: drm/i915: move a Kconfig symbol to unbreak the menu presentation (2023-02-23 13:46:39 +0800) gvt-next-fixes-2023-02-23 - use debugfs attribute for gvt debugfs entries (Deepak R Varma) - fix memory leak in vgpu destroy for debugfs_lookup() then remove (Greg KH) - fix DRM_I915_GVT kconfig symbol to unbreak menu presentation (Randy Dunlap) - fix typos (Deepak R Varma, Colin Ian King) Colin Ian King (1): i915/gvt: Fix spelling mistake "vender" -> "vendor" Deepak R Varma (2): drm/i915/gvt: Avoid full proxy f_ops for debugfs attributes drm/i915/gvt: Remove extra semicolon Greg Kroah-Hartman (1): i915: fix memory leak with using debugfs_lookup() Randy Dunlap (1): drm/i915: move a Kconfig symbol to unbreak the menu presentation drivers/gpu/drm/i915/Kconfig| 6 +++--- drivers/gpu/drm/i915/gvt/debugfs.c | 16 drivers/gpu/drm/i915/gvt/firmware.c | 2 +- drivers/gpu/drm/i915/gvt/kvmgt.c| 2 +- drivers/gpu/drm/i915/gvt/vgpu.c | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH][next] i915/gvt: Fix spelling mistake "vender" -> "vendor"
On 2023.02.02 12:50:18 +, Colin Ian King wrote:
> There is a spelling mistake in a literal string. Fix it.
>
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/i915/gvt/firmware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/firmware.c
> b/drivers/gpu/drm/i915/gvt/firmware.c
> index dce93738e98a..4dd52ac2043e 100644
> --- a/drivers/gpu/drm/i915/gvt/firmware.c
> +++ b/drivers/gpu/drm/i915/gvt/firmware.c
> @@ -171,7 +171,7 @@ static int verify_firmware(struct intel_gvt *gvt,
> mem = (fw->data + h->cfg_space_offset);
>
> id = *(u16 *)(mem + PCI_VENDOR_ID);
> - VERIFY("vender id", id, pdev->vendor);
> + VERIFY("vendor id", id, pdev->vendor);
>
> id = *(u16 *)(mem + PCI_DEVICE_ID);
> VERIFY("device id", id, pdev->device);
> --
Thanks, Colin.
Acked-by: Zhenyu Wang
>
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH] i915: fix memory leak with using debugfs_lookup()
On 2023.02.06 15:23:55 -0500, Rodrigo Vivi wrote: > On Thu, Feb 02, 2023 at 03:13:09PM +0100, Greg Kroah-Hartman wrote: > > When calling debugfs_lookup() the result must have dput() called on it, > > otherwise the memory will leak over time. To make things simpler, just > > call debugfs_lookup_and_remove() instead which handles all of the logic > > at once. > > > > Cc: Zhenyu Wang > > Cc: Zhi Wang > > Cc: Jani Nikula > > Cc: Joonas Lahtinen > > Cc: Rodrigo Vivi > > Cc: Tvrtko Ursulin > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: intel-gvt-...@lists.freedesktop.org > > Cc: intel-gfx@lists.freedesktop.org > > Cc: dri-de...@lists.freedesktop.org > > Signed-off-by: Greg Kroah-Hartman > > > Reviewed-by: Rodrigo Vivi > > Zhenyu or Zhi, could you please propagate this through your gvt branch? > Sorry I missed this one after I migrated my mail stuff onto new machine.. Looks good to me. Reviewed-by: Zhenyu Wang > > drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > > b/drivers/gpu/drm/i915/gvt/kvmgt.c > > index 8ae7039b3683..de675d799c7d 100644 > > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > > @@ -699,7 +699,7 @@ static void intel_vgpu_close_device(struct vfio_device > > *vfio_dev) > > > > clear_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status); > > > > - debugfs_remove(debugfs_lookup(KVMGT_DEBUGFS_FILENAME, vgpu->debugfs)); > > + debugfs_lookup_and_remove(KVMGT_DEBUGFS_FILENAME, vgpu->debugfs); > > > > kvm_page_track_unregister_notifier(vgpu->vfio_device.kvm, > >>track_node); > > -- > > 2.39.1 > > signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH] drm/i915: move a Kconfig symbol to unbreak the menu presentation
On 2023.02.16 22:32:33 -0800, Randy Dunlap wrote: > Hi, > > On 2/16/23 17:52, Zhenyu Wang wrote: > > On 2023.02.14 20:45:33 -0800, Randy Dunlap wrote: > >> Inserting a Kconfig symbol that does not have a dependency (DRM_I915_GVT) > >> into a list of other symbols that do have a dependency (on DRM_I915) > >> breaks the driver menu presentation in 'make *config'. > >> > > > > I'm not sure what's the actual failure in presentation, I'm not quite > > familiar > > with Kconfig, could you help to elaborate? > > > > thanks! > > For menuconfig and nconfig, it's a subtle difference. The following menu > items are indented more after the patch (i.e., they are not indented enough > before the patch): > > │Enable KVM host support Intel GVT-g graphics virtualization > │ > │ [*] Enable Intel PXP support > │ > │ drm/i915 Debugging ---> > │ > │ drm/i915 Profile Guided Optimisation ---> > > Same menu items for gconfig: they should all be subordinate (so indented) > to the main > Intel 8xx/9xx/G3x/G4x/HD Graphics > menu. > > For xconfig, it's worse. "drm/i915 Debugging" and "drm/i915 Profile Guided > Optimisation" > are shown on the left side window, while are of the other i915 options are > shown in the > right side window (before the patch). > After the patch, all subordinate options are listed in the right side window > under the > main "Intel 8xx/9xx/G3x/G4x/HD Graphics" menu item. > > See attached photos for comparisons. > I wasn't awared of the wrong indentation. Thanks a lot, Randy! Acked-by: Zhenyu Wang signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH] drm/i915: move a Kconfig symbol to unbreak the menu presentation
On 2023.02.14 20:45:33 -0800, Randy Dunlap wrote:
> Inserting a Kconfig symbol that does not have a dependency (DRM_I915_GVT)
> into a list of other symbols that do have a dependency (on DRM_I915)
> breaks the driver menu presentation in 'make *config'.
>
I'm not sure what's the actual failure in presentation, I'm not quite familiar
with Kconfig, could you help to elaborate?
thanks!
> Relocate the DRM_I915_GVT symbol so that it does not cause this
> problem.
>
> Fixes: 8b750bf74418 ("drm/i915/gvt: move the gvt code into kvmgt.ko")
> Signed-off-by: Randy Dunlap
> Cc: Christoph Hellwig
> Cc: Zhi Wang
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
> Cc: Tvrtko Ursulin
> Cc: Zhenyu Wang
> Cc: intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-...@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org
> ---
> drivers/gpu/drm/i915/Kconfig |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff -- a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -118,9 +118,6 @@ config DRM_I915_USERPTR
>
> If in doubt, say "Y".
>
> -config DRM_I915_GVT
> - bool
> -
> config DRM_I915_GVT_KVMGT
> tristate "Enable KVM host support Intel GVT-g graphics virtualization"
> depends on DRM_I915
> @@ -172,3 +169,6 @@ menu "drm/i915 Unstable Evolution"
> depends on DRM_I915
> source "drivers/gpu/drm/i915/Kconfig.unstable"
> endmenu
> +
> +config DRM_I915_GVT
> + bool
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH 2/2] drm/i915/gvt: Avoid full proxy f_ops for vgpu_status debug attributes
On 2023.01.19 17:05:56 -0500, Rodrigo Vivi wrote: > > It still doesn't apply in drm-intel-next. > Could you please take it through your branch? > sure, I'll pick it. signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 03/27] drm/i915/gvt: Incorporate KVM memslot info into check for 2MiB GTT entry
On 2023.01.11 17:55:04 +, Sean Christopherson wrote:
> On Mon, Jan 09, 2023, Yan Zhao wrote:
> > On Fri, Jan 06, 2023 at 11:01:53PM +, Sean Christopherson wrote:
> > > On Fri, Jan 06, 2023, Yan Zhao wrote:
> > > > On Thu, Jan 05, 2023 at 05:40:32PM +, Sean Christopherson wrote:
> > > > > On Thu, Jan 05, 2023, Yan Zhao wrote:
> > > > > I'm totally fine if KVMGT's ABI is that VFIO is the source of truth
> > > > > for mappings
> > > > > and permissions, and that the only requirement for KVM memslots is
> > > > > that GTT page
> > > > > tables need to be visible in KVM's memslots. But if that's the ABI,
> > > > > then
> > > > > intel_gvt_is_valid_gfn() should be probing VFIO, not KVM (commit
> > > > > cc753fbe1ac4
> > > > > ("drm/i915/gvt: validate gfn before set shadow page entry").
> > > > >
> > > > > In other words, pick either VFIO or KVM. Checking that X is valid
> > > > > according to
> > > > > KVM and then mapping X through VFIO is confusing and makes
> > > > > assumptions about how
> > > > > userspace configures KVM and VFIO. It works because QEMU always
> > > > > configures KVM
> > > > > and VFIO as expected, but IMO it's unnecessarily fragile and again
> > > > > confusing for
> > > > > unaware readers because the code is technically flawed.
> > > > >
> > > > Agreed.
> > > > Then after some further thought, I think maybe we can just remove
> > > > intel_gvt_is_valid_gfn() in KVMGT, because
> > > >
> > > > (1) both intel_gvt_is_valid_gfn() in emulate_ggtt_mmio_write() and
> > > > ppgtt_populate_spt() are not for page track purpose, but to validate
> > > > bogus
> > > > GFN.
> > > > (2) gvt_pin_guest_page() with gfn and size can do the validity checking,
> > > > which is called in intel_gvt_dma_map_guest_page(). So, we can move the
> > > > mapping of scratch page to the error path after
> > > > intel_gvt_dma_map_guest_page().
> > >
> > > IIUC, that will re-introduce the problem commit cc753fbe1ac4
> > > ("drm/i915/gvt: validate
> > > gfn before set shadow page entry") solved by poking into KVM. Lack of
> > > pre-validation
> > > means that bogus GFNs will trigger error messages, e.g.
> > >
> > > gvt_vgpu_err("vfio_pin_pages failed for iova %pad, ret
> > > %d\n",
> > >_iova, ret);
> > >
> > > and
> > >
> > > gvt_vgpu_err("fail to populate guest ggtt entry\n");
> >
> > Thanks for pointing it out.
> > I checked this commit message and found below original intentions to
> > introduce
> > pre-validation:
>
> ...
>
> > (I actually found that the original code will print "invalid entry type"
> > warning which indicates it's broken for a while due to lack of test in
> > this invalid gfn path)
> >
> >
> > > One thought would be to turn those printks into tracepoints to eliminate
> > > unwanted
> > > noise, and to prevent the guest from spamming the host kernel log by
> > > programming
> > > garbage into the GTT (gvt_vgpu_err() isn't ratelimited).
> > As those printks would not happen in normal conditions and printks may have
> > some advantages to discover the attack or bug, could we just convert
> > gvt_vgpu_err() to be ratelimited ?
>
> That's ultimately a decision that needs to be made by the GVT maintainers, as
> the
> answer depends on the use case. E.g. if most users of KVMGT run a single VM
> and
> the guest user is also the host admin, then pr_err_ratelimited() is likely an
> acceptable/preferable choice as there's a decent chance a human will see the
> errors
> in the host kernel logs and be able to take action.
>
> But if there's unlikely to be a human monitoring the host logs, and/or the
> guest
> user is unrelated to the host admin, then a ratelimited printk() is less
> useful.
> E.g. if there's no one monitoring the logs, then losing messages due to
> ratelimiting provides a worse debug experience overall than having to manually
> enable tracepoints. And if there may be many tens of VMs (seems unlikely?),
> then
> ratelimited printk() is even less useful because errors for a specific VM may
> be
> lost, i.e. the printk() can't be relied upon in any way to detect issues.
>
> FWIW, in KVM proper, use of printk() to capture guest "errors" is strongly
> discourage
> for exactly these reasons.
Current KVMGT usage is mostly in controlled mode, either user is own host admin,
or host admin would pre-configure specific limited number of VMs for KVMGT use.
I think printk on error should be fine, we don't need rate limit, and adding
extra trace monitor for admin might not be necessary. So I'm towards to keep to
use current error message.
thanks
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Remove extra semicolon
On 2023.01.14 21:12:39 +0530, Deepak R Varma wrote: > Remove the extra semicolon at end. Issue identified using > semicolon.cocci Coccinelle semantic patch. > > Signed-off-by: Deepak R Varma > --- > drivers/gpu/drm/i915/gvt/vgpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c > index a5497440484f..08ad1bd651f1 100644 > --- a/drivers/gpu/drm/i915/gvt/vgpu.c > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c > @@ -323,7 +323,7 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu, > ret = idr_alloc(>vgpu_idr, vgpu, IDLE_VGPU_IDR + 1, GVT_MAX_VGPU, > GFP_KERNEL); > if (ret < 0) > - goto out_unlock;; > + goto out_unlock; > > vgpu->id = ret; > vgpu->sched_ctl.weight = conf->weight; > -- Thanks for catching that! Reviewed-by: Zhenyu Wang signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 2/2] drm/i915/gvt: Avoid full proxy f_ops for vgpu_status debug attributes
On 2023.01.18 11:44:55 -0500, Rodrigo Vivi wrote:
> On Wed, Jan 18, 2023 at 10:18:11AM +0530, Deepak R Varma wrote:
> > On Tue, Jan 17, 2023 at 02:29:37PM -0500, Rodrigo Vivi wrote:
> > > On Mon, Jan 16, 2023 at 01:44:46PM +0800, Zhenyu Wang wrote:
> > > > On 2023.01.10 13:49:57 -0500, Rodrigo Vivi wrote:
> > > > > On Wed, Jan 11, 2023 at 12:00:12AM +0530, Deepak R Varma wrote:
> > > > > > Using DEFINE_SIMPLE_ATTRIBUTE macro with the debugfs_create_file()
> > > > > > function adds the overhead of introducing a proxy file operation
> > > > > > functions to wrap the original read/write inside file removal
> > > > > > protection
> > > > > > functions. This adds significant overhead in terms of introducing
> > > > > > and
> > > > > > managing the proxy factory file operations structure and function
> > > > > > wrapping at runtime.
> > > > > > As a replacement, a combination of DEFINE_DEBUGFS_ATTRIBUTE macro
> > > > > > paired
> > > > > > with debugfs_create_file_unsafe() is suggested to be used instead.
> > > > > > The
> > > > > > DEFINE_DEBUGFS_ATTRIBUTE utilises debugfs_file_get() and
> > > > > > debugfs_file_put() wrappers to protect the original read and write
> > > > > > function calls for the debug attributes. There is no need for any
> > > > > > runtime proxy file operations to be managed by the debugfs core.
> > > > > > Following coccicheck make command helped identify this change:
> > > > > >
> > > > > > make coccicheck M=drivers/gpu/drm/i915/ MODE=patch
> > > > > > COCCI=./scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > > > > >
> > > > > > Signed-off-by: Deepak R Varma
> > > > >
> > > > > I believe these 2 gvt cases could be done in one patch.
> > > > > But anyways,
> > > > >
> > > > > Reviewed-by: Rodrigo Vivi
> > > > >
> > > > > for both patches... and will leave these 2 patches for gvt folks
> > > > > to apply. Unless they ack and I apply in the drm-intel along with the
> > > > > other ones.
> > > > >
> > > >
> > > > yeah, they're fine with me, feel free to apply them directly.
> > > >
> > > > Acked-by: Zhenyu Wang
> > >
> > > Unfortunately I got some conflicts when trying to apply on drm-intel-next.
> > >
> > > We probably need a new version, and probably through gvt branches it
> > > will be easier to handle conflicts if they appear.
> >
> > Hello Rodrigo,
> > Sure. I will send in a new version. I am current using linux-next git repo
> > as my
> > remote origin [tag 20230113]. Are there any specific instruction/location
> > from
> > where I should access the gvt branch?
>
> https://github.com/intel/gvt-linux.git
>
> but with the linux-next your patch is probably right for them.
>
yeah, I think so as currently from last pull request I don't have
other updates in gvt tree, maybe it's just d-i-n hasn't included
recent gvt change.
I saw Deepak sent a new one, feel free to apply. Let me know if
there's still any issue.
thanks!
> > > >
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/gvt/debugfs.c | 6 +++---
> > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/gvt/debugfs.c
> > > > > > b/drivers/gpu/drm/i915/gvt/debugfs.c
> > > > > > index 03f081c3d9a4..baccbf1761b7 100644
> > > > > > --- a/drivers/gpu/drm/i915/gvt/debugfs.c
> > > > > > +++ b/drivers/gpu/drm/i915/gvt/debugfs.c
> > > > > > @@ -165,7 +165,7 @@ static int vgpu_status_get(void *data, u64 *val)
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -DEFINE_SIMPLE_ATTRIBUTE(vgpu_status_fops, vgpu_status_get, NULL,
> > > > > > "0x%llx\n");
> > > > > > +DEFINE_DEBUGFS_ATTRIBUTE(vgpu_status_fops, vgpu_status_get, NULL,
> > > > > > "0x%llx\n");
> > > > > >
> > > > > > /**
> > > > > > * intel_gvt_debugfs_add_vgpu - register debugfs entries for a vGPU
> > > > > > @@ -182,8 +182,8 @@ void intel_gvt_debugfs_add_vgpu(struct
> > > > > > intel_vgpu *vgpu)
> > > > > > _mmio_diff_fops);
> > > > > > debugfs_create_file_unsafe("scan_nonprivbb", 0644,
> > > > > > vgpu->debugfs, vgpu,
> > > > > >_scan_nonprivbb_fops);
> > > > > > - debugfs_create_file("status", 0644, vgpu->debugfs, vgpu,
> > > > > > - _status_fops);
> > > > > > + debugfs_create_file_unsafe("status", 0644, vgpu->debugfs, vgpu,
> > > > > > + _status_fops);
> > > > > > }
> > > > > >
> > > > > > /**
> > > > > > --
> > > > > > 2.34.1
> > > > > >
> > > > > >
> > > > > >
> > >
> > >
> >
> >
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH 2/2] drm/i915/gvt: Avoid full proxy f_ops for vgpu_status debug attributes
On 2023.01.10 13:49:57 -0500, Rodrigo Vivi wrote:
> On Wed, Jan 11, 2023 at 12:00:12AM +0530, Deepak R Varma wrote:
> > Using DEFINE_SIMPLE_ATTRIBUTE macro with the debugfs_create_file()
> > function adds the overhead of introducing a proxy file operation
> > functions to wrap the original read/write inside file removal protection
> > functions. This adds significant overhead in terms of introducing and
> > managing the proxy factory file operations structure and function
> > wrapping at runtime.
> > As a replacement, a combination of DEFINE_DEBUGFS_ATTRIBUTE macro paired
> > with debugfs_create_file_unsafe() is suggested to be used instead. The
> > DEFINE_DEBUGFS_ATTRIBUTE utilises debugfs_file_get() and
> > debugfs_file_put() wrappers to protect the original read and write
> > function calls for the debug attributes. There is no need for any
> > runtime proxy file operations to be managed by the debugfs core.
> > Following coccicheck make command helped identify this change:
> >
> > make coccicheck M=drivers/gpu/drm/i915/ MODE=patch
> > COCCI=./scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> >
> > Signed-off-by: Deepak R Varma
>
> I believe these 2 gvt cases could be done in one patch.
> But anyways,
>
> Reviewed-by: Rodrigo Vivi
>
> for both patches... and will leave these 2 patches for gvt folks
> to apply. Unless they ack and I apply in the drm-intel along with the other
> ones.
>
yeah, they're fine with me, feel free to apply them directly.
Acked-by: Zhenyu Wang
thanks!
> > ---
> > drivers/gpu/drm/i915/gvt/debugfs.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/debugfs.c
> > b/drivers/gpu/drm/i915/gvt/debugfs.c
> > index 03f081c3d9a4..baccbf1761b7 100644
> > --- a/drivers/gpu/drm/i915/gvt/debugfs.c
> > +++ b/drivers/gpu/drm/i915/gvt/debugfs.c
> > @@ -165,7 +165,7 @@ static int vgpu_status_get(void *data, u64 *val)
> > return 0;
> > }
> >
> > -DEFINE_SIMPLE_ATTRIBUTE(vgpu_status_fops, vgpu_status_get, NULL,
> > "0x%llx\n");
> > +DEFINE_DEBUGFS_ATTRIBUTE(vgpu_status_fops, vgpu_status_get, NULL,
> > "0x%llx\n");
> >
> > /**
> > * intel_gvt_debugfs_add_vgpu - register debugfs entries for a vGPU
> > @@ -182,8 +182,8 @@ void intel_gvt_debugfs_add_vgpu(struct intel_vgpu *vgpu)
> > _mmio_diff_fops);
> > debugfs_create_file_unsafe("scan_nonprivbb", 0644, vgpu->debugfs, vgpu,
> >_scan_nonprivbb_fops);
> > - debugfs_create_file("status", 0644, vgpu->debugfs, vgpu,
> > - _status_fops);
> > + debugfs_create_file_unsafe("status", 0644, vgpu->debugfs, vgpu,
> > + _status_fops);
> > }
> >
> > /**
> > --
> > 2.34.1
> >
> >
> >
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PULL] gvt-fixes
On 2023.01.04 06:34:28 -0500, Rodrigo Vivi wrote: > On Wed, Jan 04, 2023 at 04:05:13PM +0800, Zhenyu Wang wrote: > > > > Hi, > > > > Here's accumulated current gvt-fixes. Several of them handle > > for error or destroy path issues and one from Zhi to fix up > > left vgpu status tracking. > > > > thanks! > > --- > > The following changes since commit 6217e9f05a74df48c77ee68993d587cdfdb1feb7: > > I'm kind of confused on your baseline here. > > It is including a strange merge commit in the middle of the commits: > Zhenyu Wang ??? M?? Merge tag 'drm-intel-fixes-2022-12-30' into > gvt-fixes > commit c063c8c07864246ba3831b017cea8d3096e236a8 > > Please rebase on top of v6.2-rc2 so we have the same base here. > (and please remind to sign-off-by when pushing the commits) > oh, I tried to sycn up by back merge with vfio iommufd of gvt changes to apply Zhi's patch properly, so it should stand on that tag..but anyway I just refresh with rebase and fixed two invalid r-b tags. Please pull below one. thanks! --- The following changes since commit 88603b6dc419445847923fcb7fe5080067a30f98: Linux 6.2-rc2 (2023-01-01 13:53:16 -0800) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-fixes-2023-01-05 for you to fetch changes up to 4a61648af68f5ba4884f0e3b494ee1cabc4b6620: drm/i915/gvt: fix double free bug in split_2MB_gtt_entry (2023-01-04 23:21:19 +0800) gvt-fixes-2023-01-05 - Fix one missed unpin in error of intel_vgpu_shadow_mm_pin() - Fix two debugfs destroy oops issues for vgpu and gvt entries - Fix one potential double free issue in gtt shadow pt code - Fix to use atomic bit flag for vgpu status Dan Carpenter (1): drm/i915: unpin on error in intel_vgpu_shadow_mm_pin() Zheng Wang (1): drm/i915/gvt: fix double free bug in split_2MB_gtt_entry Zhenyu Wang (2): drm/i915/gvt: fix gvt debugfs destroy drm/i915/gvt: fix vgpu debugfs clean in remove Zhi Wang (1): drm/i915/gvt: use atomic operations to change the vGPU status drivers/gpu/drm/i915/gvt/debugfs.c | 36 +++- drivers/gpu/drm/i915/gvt/dmabuf.c| 3 ++- drivers/gpu/drm/i915/gvt/gtt.c | 21 +++-- drivers/gpu/drm/i915/gvt/gvt.h | 15 ++- drivers/gpu/drm/i915/gvt/interrupt.c | 2 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 35 +-- drivers/gpu/drm/i915/gvt/scheduler.c | 4 +++- drivers/gpu/drm/i915/gvt/vgpu.c | 12 +--- 8 files changed, 80 insertions(+), 48 deletions(-) signature.asc Description: PGP signature
[Intel-gfx] [PULL] gvt-fixes
Hi, Here's accumulated current gvt-fixes. Several of them handle for error or destroy path issues and one from Zhi to fix up left vgpu status tracking. thanks! --- The following changes since commit 6217e9f05a74df48c77ee68993d587cdfdb1feb7: drm/i915/dsi: fix MIPI_BKLT_EN_1 native GPIO index (2022-12-30 04:28:46 -0500) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-fixes-2023-01-04 for you to fetch changes up to 601fd0f6b2a4c776a21ab8300fe0de0726937623: drm/i915/gvt: fix double free bug in split_2MB_gtt_entry (2023-01-04 15:20:09 +0800) gvt-fixes-2023-01-04 - Fix one missed unpin in error of intel_vgpu_shadow_mm_pin() - Fix two debugfs destroy oops issues for vgpu and gvt entries - Fix one potential double free issue in gtt shadow pt code - Fix to use atomic bit flag for vgpu status Dan Carpenter (1): drm/i915: unpin on error in intel_vgpu_shadow_mm_pin() Zheng Wang (1): drm/i915/gvt: fix double free bug in split_2MB_gtt_entry Zhenyu Wang (3): drm/i915/gvt: fix gvt debugfs destroy drm/i915/gvt: fix vgpu debugfs clean in remove Merge tag 'drm-intel-fixes-2022-12-30' into gvt-fixes Zhi Wang (1): drm/i915/gvt: use atomic operations to change the vGPU status drivers/gpu/drm/i915/gvt/debugfs.c | 36 +++- drivers/gpu/drm/i915/gvt/dmabuf.c| 3 ++- drivers/gpu/drm/i915/gvt/gtt.c | 21 +++-- drivers/gpu/drm/i915/gvt/gvt.h | 15 ++- drivers/gpu/drm/i915/gvt/interrupt.c | 2 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 35 +-- drivers/gpu/drm/i915/gvt/scheduler.c | 4 +++- drivers/gpu/drm/i915/gvt/vgpu.c | 12 +--- 8 files changed, 80 insertions(+), 48 deletions(-) signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH v5] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry
On 2022.12.20 17:40:14 +0800, Zheng Wang wrote:
> If intel_gvt_dma_map_guest_page failed, it will call
> ppgtt_invalidate_spt, which will finally free the spt. But the
> caller function ppgtt_populate_spt_by_guest_entry does not notice
> that, it will free spt again in its error path.
indent
>
> Fix this by undoing the mapping of DMA address and freeing sub_spt.
> Besides, leave the handle of spt destroy to caller function instead of
> callee function when error occurs.
>
> Fixes: b901b252b6cf ("drm/i915/gvt: Add 2M huge gtt support")
> Signed-off-by: Zheng Wang
> ---
> v5:
> - remove unnecessary switch-case code for there is only one particular case,
> correct the unmap target from parent_spt to sub_spt.add more details in
> commit message. All suggested by Zhenyu
>
> v4:
> - fix by undo the mapping of DMA address and free sub_spt suggested by Zhi
>
> v3:
> - correct spelling mistake and remove unused variable suggested by Greg
>
> v2: https://lore.kernel.org/all/20221006165845.1735393-1-zyytlz...@163.com/
>
> v1: https://lore.kernel.org/all/20220928033340.1063949-1-zyytlz...@163.com/
> ---
> drivers/gpu/drm/i915/gvt/gtt.c | 18 ++
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 51e5e8fb505b..4d478a59eb7d 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1209,10 +1209,8 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
> for_each_shadow_entry(sub_spt, _se, sub_index) {
> ret = intel_gvt_dma_map_guest_page(vgpu, start_gfn + sub_index,
> PAGE_SIZE, _addr);
> - if (ret) {
> - ppgtt_invalidate_spt(spt);
> - return ret;
> - }
> + if (ret)
> + goto err;
> sub_se.val64 = se->val64;
>
> /* Copy the PAT field from PDE. */
> @@ -1231,6 +1229,18 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
> ops->set_pfn(se, sub_spt->shadow_page.mfn);
> ppgtt_set_shadow_entry(spt, se, index);
> return 0;
> +err:
> + /* Undone the existing mappings of DMA addr. */
We need a verb here for Undo.
> + for_each_present_shadow_entry(sub_spt, _se, sub_index) {
> + gvt_vdbg_mm("invalidate 4K entry\n");
> + ppgtt_invalidate_pte(sub_spt, _se);
> + }
> + /* Release the new allocated spt. */
> + trace_spt_change(sub_spt->vgpu->id, "release", sub_spt,
> + sub_spt->guest_page.gfn, sub_spt->shadow_page.type);
> + ppgtt_free_spt(sub_spt);
> + sub_spt = NULL;
Not need to reset local variable that has no use then.
I'll handle these trivial fixes during the merge.
Reviewed-by: Zhenyu Wang
thanks
> + return ret;
> }
>
> static int split_64KB_gtt_entry(struct intel_vgpu *vgpu,
> --
> 2.25.1
>
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH] [next] i915/gvt: Replace one-element array with flexible-array member
On 2022.12.20 16:41:15 +1300, Paulo Miguel Almeida wrote:
> One-element arrays are deprecated, and we are replacing them with
> flexible array members instead. So, replace one-element array with
> flexible-array member in struct gvt_firmware_header and refactor the
> rest of the code accordingly.
>
> Additionally, previous implementation was allocating 8 bytes more than
> required to represent firmware_header + cfg_space data + mmio data.
>
> This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
> routines on memcpy() and help us make progress towards globally
> enabling -fstrict-flex-arrays=3 [1].
>
> Link: https://github.com/KSPP/linux/issues/79
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
> Signed-off-by: Paulo Miguel Almeida
> ---
> To make reviewing this patch easier, I'm pasting before/after struct
> sizes.
>
> pahole -C gvt_firmware_header before/drivers/gpu/drm/i915/gvt/firmware.o
> struct gvt_firmware_header {
> u64magic;/* 0 8 */
> u32crc32;/* 8 4 */
> u32version; /*12 4 */
> u64cfg_space_size; /*16 8 */
> u64cfg_space_offset; /*24 8 */
> u64mmio_size;/*32 8 */
> u64mmio_offset; /*40 8 */
> unsigned char data[1]; /*48 1 */
>
> /* size: 56, cachelines: 1, members: 8 */
> /* padding: 7 */
> /* last cacheline: 56 bytes */
> };
>
> pahole -C gvt_firmware_header after/drivers/gpu/drm/i915/gvt/firmware.o
> struct gvt_firmware_header {
> u64magic;/* 0 8 */
> u32crc32;/* 8 4 */
> u32version; /*12 4 */
> u64cfg_space_size; /*16 8 */
> u64cfg_space_offset; /*24 8 */
> u64mmio_size;/*32 8 */
> u64mmio_offset; /*40 8 */
> unsigned char data[]; /*48 0 */
>
> /* size: 48, cachelines: 1, members: 8 */
> /* last cacheline: 48 bytes */
> };
>
> As you can see the additional byte of the fake-flexible array (data[1])
> forced the compiler to pad the struct but those bytes aren't actually used
> as first & last bytes (of both cfg_space and mmio) are controlled by the
> <>_size and <>_offset members present in the gvt_firmware_header struct.
> ---
> drivers/gpu/drm/i915/gvt/firmware.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/firmware.c
> b/drivers/gpu/drm/i915/gvt/firmware.c
> index a683c22d5b64..dce93738e98a 100644
> --- a/drivers/gpu/drm/i915/gvt/firmware.c
> +++ b/drivers/gpu/drm/i915/gvt/firmware.c
> @@ -45,7 +45,7 @@ struct gvt_firmware_header {
> u64 cfg_space_offset; /* offset in the file */
> u64 mmio_size;
> u64 mmio_offset;/* offset in the file */
> - unsigned char data[1];
> + unsigned char data[];
> };
>
> #define dev_to_drm_minor(d) dev_get_drvdata((d))
> @@ -77,7 +77,7 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt)
> unsigned long size, crc32_start;
> int ret;
>
> - size = sizeof(*h) + info->mmio_size + info->cfg_space_size;
> + size = offsetof(struct gvt_firmware_header, data) + info->mmio_size +
> info->cfg_space_size;
> firmware = vzalloc(size);
> if (!firmware)
> return -ENOMEM;
> --
Looks good to me.
Reviewed-by: Zhenyu Wang
Thanks!
signature.asc
Description: PGP signature
Re: [Intel-gfx] [RESEND PATCH v4] drm/i915/gvt: fix double free bug in split_2MB_gtt_entry
On 2022.12.19 20:52:04 +0800, Zheng Wang wrote:
> If intel_gvt_dma_map_guest_page failed, it will call
> ppgtt_invalidate_spt, which will finally free the spt. But the caller does
> not notice that, it will free spt again in error path.
>
It's not clear from this description which caller is actually wrong,
better to clarify the problem in ppgtt_populate_spt_by_guest_entry() function.
> Fix this by undoing the mapping of DMA address and freeing sub_spt.
>
> Fixes: b901b252b6cf ("drm/i915/gvt: Add 2M huge gtt support")
> Signed-off-by: Zheng Wang
> ---
> v4:
> - fix by undo the mapping of DMA address and free sub_spt suggested by Zhi
>
> v3:
> - correct spelling mistake and remove unused variable suggested by Greg
>
> v2: https://lore.kernel.org/all/20221006165845.1735393-1-zyytlz...@163.com/
>
> v1: https://lore.kernel.org/all/20220928033340.1063949-1-zyytlz...@163.com/
> ---
> drivers/gpu/drm/i915/gvt/gtt.c | 53 +-
> 1 file changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 51e5e8fb505b..b472e021e5a4 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1192,11 +1192,11 @@ static int split_2MB_gtt_entry(struct intel_vgpu
> *vgpu,
> {
> const struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
> struct intel_vgpu_ppgtt_spt *sub_spt;
> - struct intel_gvt_gtt_entry sub_se;
> + struct intel_gvt_gtt_entry sub_se, e;
> unsigned long start_gfn;
> dma_addr_t dma_addr;
> - unsigned long sub_index;
> - int ret;
> + unsigned long sub_index, parent_index;
> + int ret, ret1;
>
> gvt_dbg_mm("Split 2M gtt entry, index %lu\n", index);
>
> @@ -1209,10 +1209,8 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
> for_each_shadow_entry(sub_spt, _se, sub_index) {
> ret = intel_gvt_dma_map_guest_page(vgpu, start_gfn + sub_index,
> PAGE_SIZE, _addr);
> - if (ret) {
> - ppgtt_invalidate_spt(spt);
> - return ret;
> - }
> + if (ret)
> + goto err;
I think it's fine to remove this and leave to upper caller, but again please
describe the behavior change in commit message as well, e.g to fix the sanity
of spt destroy that leaving previous invalidate and free of spt to caller
function
instead of within callee function.
> sub_se.val64 = se->val64;
>
> /* Copy the PAT field from PDE. */
> @@ -1231,6 +1229,47 @@ static int split_2MB_gtt_entry(struct intel_vgpu *vgpu,
> ops->set_pfn(se, sub_spt->shadow_page.mfn);
> ppgtt_set_shadow_entry(spt, se, index);
> return 0;
> +err:
> + /* Undone the existing mappings of DMA addr. */
> + for_each_present_shadow_entry(spt, , parent_index) {
sub_spt? We're undoing what's mapped for sub_spt right?
> + switch (e.type) {
> + case GTT_TYPE_PPGTT_PTE_4K_ENTRY:
> + gvt_vdbg_mm("invalidate 4K entry\n");
> + ppgtt_invalidate_pte(spt, );
> + break;
> + case GTT_TYPE_PPGTT_PTE_64K_ENTRY:
> + /* We don't setup 64K shadow entry so far. */
> + WARN(1, "suspicious 64K gtt entry\n");
> + continue;
> + case GTT_TYPE_PPGTT_PTE_2M_ENTRY:
> + gvt_vdbg_mm("invalidate 2M entry\n");
> + continue;
> + case GTT_TYPE_PPGTT_PTE_1G_ENTRY:
> + WARN(1, "GVT doesn't support 1GB page\n");
> + continue;
> + case GTT_TYPE_PPGTT_PML4_ENTRY:
> + case GTT_TYPE_PPGTT_PDP_ENTRY:
> + case GTT_TYPE_PPGTT_PDE_ENTRY:
I don't think this all entry type makes sense, as here we just split
2M entry for multiple 4K PTE entry.
> + gvt_vdbg_mm("invalidate PMUL4/PDP/PDE entry\n");
> + ret1 = ppgtt_invalidate_spt_by_shadow_entry(
> + spt->vgpu, );
> + if (ret1) {
> + gvt_vgpu_err("fail: shadow page %p shadow entry
> 0x%llx type %d\n",
> + spt, e.val64, e.type);
> + goto free_spt;
> + }
for above reason, I don't think this is valid.
> + break;
> + default:
> + GEM_BUG_ON(1);
> + }
> + }
> + /* Release the new alloced apt. */
> +free_spt:
> + trace_spt_change(sub_spt->vgpu->id, "release", sub_spt,
> + sub_spt->guest_page.gfn, sub_spt->shadow_page.type);
> + ppgtt_free_spt(sub_spt);
> + sub_spt = NULL;
> + return ret;
> }
>
> static int split_64KB_gtt_entry(struct intel_vgpu *vgpu,
> --
> 2.25.1
>
Re: [Intel-gfx] [PATCH] drm/i915: unpin on error in intel_vgpu_shadow_mm_pin()
On 2022.11.15 16:15:18 +0300, Dan Carpenter wrote:
> Call intel_vgpu_unpin_mm() on this error path.
>
> Fixes: 418741480809 ("drm/i915/gvt: Adding ppgtt to GVT GEM context after
> shadow pdps settled.")
> Signed-off-by: Dan Carpenter
> ---
> drivers/gpu/drm/i915/gvt/scheduler.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index d6fe94cd0fdb..8342d95f56cb 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -696,6 +696,7 @@ intel_vgpu_shadow_mm_pin(struct intel_vgpu_workload
> *workload)
>
> if (workload->shadow_mm->type != INTEL_GVT_MM_PPGTT ||
> !workload->shadow_mm->ppgtt_mm.shadowed) {
> + intel_vgpu_unpin_mm(workload->shadow_mm);
> gvt_vgpu_err("workload shadow ppgtt isn't ready\n");
> return -EINVAL;
> }
> --
Thanks, Dan. Looks fine to me.
Reviewed-by: Zhenyu Wang
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PULL] gvt-fixes
ping for this fix pull... On 2022.11.11 17:02:08 +0800, Zhenyu Wang wrote: > Hi, > > Here's two fixes from Sean for 6.1 kernel, which is to fix kvm > reference in gvt. No regression found in our test. > > Thanks! > --- > The following changes since commit f0c4d9fc9cc9462659728d168387191387e903cc: > > Linux 6.1-rc4 (2022-11-06 15:07:11 -0800) > > are available in the Git repository at: > > https://github.com/intel/gvt-linux.git tags/gvt-fixes-2022-11-11 > > for you to fetch changes up to 3c9fd44b9330adc5006653566f3d386784b2080e: > > drm/i915/gvt: Unconditionally put reference to KVM when detaching vGPU > (2022-11-11 13:21:52 +0800) > > > gvt-fixes-2022-11-11 > > - kvm reference fix from Sean > > > Sean Christopherson (2): > drm/i915/gvt: Get reference to KVM iff attachment to VM is successful > drm/i915/gvt: Unconditionally put reference to KVM when detaching vGPU > > drivers/gpu/drm/i915/gvt/kvmgt.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > signature.asc Description: PGP signature
Re: [Intel-gfx] [PULL] gvt-next
On 2022.11.17 03:37:17 +, Vivi, Rodrigo wrote: > On Thu, 2022-11-17 at 11:02 +0800, Zhenyu Wang wrote: > > On 2022.11.15 10:36:59 -0500, Rodrigo Vivi wrote: > > > On Fri, Nov 11, 2022 at 04:59:03PM +0800, Zhenyu Wang wrote: > > > > Hi, > > > > > > > > Here's current accumulated changes in gvt-next. Sorry that I > > > > delayed > > > > to refresh them on time for upstream...It contains mostly kernel > > > > doc, > > > > typo fixes and small code cleanups, as details below. > > > > > > > > btw, one gvt change for next > > > > https://patchwork.freedesktop.org/patch/58/ > > > > is still pending, I need a backmerge from linus tree e.g with > > > > recent vfio/mdev > > > > consolidate change with gvt and Jason's fix for destroy device, > > > > to apply Zhi's > > > > change cleanly. Pls help on that. > > > > > > Based on what I could verify the commiter signature is really not > there. It looks like you later forced a rebase and while > rebasing you forgot to re-sign everything. > Hi, pls re-pull this one. thanks! --- The following changes since commit a6ebd538364b1e9e6048faaafbc0188172ed50c3: drm/i915/sdvo: Fix debug print (2022-10-28 14:46:21 +0300) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-next-2022-11-17 for you to fetch changes up to 04ec334e1a0381c3305da4d277cef9250769ca43: drm/i915/gvt: Remove the unused function get_pt_type() (2022-11-17 14:07:09 +0800) gvt-next-2022-11-17 - kernel doc fixes - remove vgpu->released sanity check - small clean up Colin Ian King (1): drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported" Jiapeng Chong (4): drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Remove the unused function get_pt_type() Julia Lawall (1): drm/i915/gvt: fix typo in comment Mauro Carvalho Chehab (1): drm/i915: gvt: fix kernel-doc trivial warnings Paulo Miguel Almeida (1): i915/gvt: remove hardcoded value on crc32_start calculation Zhi Wang (1): drm/i915/gvt: remove the vgpu->released and its sanity check wangjianli (1): drm/i915: fix repeated words in comments drivers/gpu/drm/i915/gvt/aperture_gm.c | 4 ++-- drivers/gpu/drm/i915/gvt/cfg_space.c| 2 +- drivers/gpu/drm/i915/gvt/dmabuf.h | 2 +- drivers/gpu/drm/i915/gvt/firmware.c | 2 +- drivers/gpu/drm/i915/gvt/gtt.c | 9 ++--- drivers/gpu/drm/i915/gvt/gvt.h | 2 -- drivers/gpu/drm/i915/gvt/handlers.c | 4 ++-- drivers/gpu/drm/i915/gvt/kvmgt.c| 4 drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +- drivers/gpu/drm/i915/gvt/page_track.c | 2 +- drivers/gpu/drm/i915/gvt/vgpu.c | 6 +++--- 11 files changed, 14 insertions(+), 25 deletions(-) signature.asc Description: PGP signature
Re: [Intel-gfx] [PULL] gvt-next
On 2022.11.17 03:37:17 +, Vivi, Rodrigo wrote:
> On Thu, 2022-11-17 at 11:02 +0800, Zhenyu Wang wrote:
> > On 2022.11.15 10:36:59 -0500, Rodrigo Vivi wrote:
> > > On Fri, Nov 11, 2022 at 04:59:03PM +0800, Zhenyu Wang wrote:
> > > > Hi,
> > > >
> > > > Here's current accumulated changes in gvt-next. Sorry that I
> > > > delayed
> > > > to refresh them on time for upstream...It contains mostly kernel
> > > > doc,
> > > > typo fixes and small code cleanups, as details below.
> > > >
> > > > btw, one gvt change for next
> > > > https://patchwork.freedesktop.org/patch/58/
> > > > is still pending, I need a backmerge from linus tree e.g with
> > > > recent vfio/mdev
> > > > consolidate change with gvt and Jason's fix for destroy device,
> > > > to apply Zhi's
> > > > change cleanly. Pls help on that.
> > > >
> > > > Thanks!
> > > > ---
> > > > The following changes since commit
> > > > a6ebd538364b1e9e6048faaafbc0188172ed50c3:
> > > >
> > > > ?? drm/i915/sdvo: Fix debug print (2022-10-28 14:46:21 +0300)
> > > >
> > > > are available in the Git repository at:
> > > >
> > > > ?? https://github.com/intel/gvt-linux.git??tags/gvt-next-2022-11-11
> > > >
> > > > for you to fetch changes up to
> > > > 50468ca2e2e1ce882f060a8c263f678affe112db:
> > > >
> > > > ?? drm/i915/gvt: Remove the unused function get_pt_type() (2022-
> > > > 11-08 15:34:06 +0800)
> > > >
> > > >
> > > > gvt-next-2022-11-11
> > > >
> > > > - kernel doc fixes
> > > > - remove vgpu->released sanity check
> > > > - small clean up
> > > >
> > > >
> > > > Colin Ian King (1):
> > > > ?? drm/i915/reg: Fix spelling mistake "Unsupport" ->
> > > > "Unsupported"
> > >
> > > dim: d7e4e9579f01 ("drm/i915/reg: Fix spelling mistake "Unsupport"
> > > -> "Unsupported""): committer Signed-off-by missing.
> > >
> > > could you please fix this before we can merge this pr?
> > >
> >
> > That should still be .mailmap issue for Colin's email...
> > But could we improve our dim script to grep mailmap in that case? So
> > if s-o-b mail is valid
> > in mailmap, we should still allow it, right?
>
> Based on what I could verify the commiter signature is really not
> there. It looks like you later forced a rebase and while
> rebasing you forgot to re-sign everything.
>
Oops! Sorry for that. I'll redo this.
> We need to fix this in your tree to avoid propagating that to other
> trees later:
>
> From tig view:
> Commit: Zhenyu Wang
> CommitDate: Tue Nov 8 15:04:53 2022 +0800
>
> drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported"
>
> There is a spelling mistake in a gvt_vgpu_err error message. Fix
> it.
>
> Fixes: 695fbc08d80f ("drm/i915/gvt: replace the gvt_err with
> gvt_vgpu_err")
> Signed-off-by: Colin Ian King
> Signed-off-by: Zhi Wang
> Link:
> http://patchwork.freedesktop.org/patch/msgid/20220315202449.2952845-1-colin.i.k...@gmail.com
> Reviewed-by: Zhi Wang
>
> no signature from you.
>
> >
> > > >
> > > > Jiapeng Chong (4):
> > > > ?? drm/i915/gvt: Fix kernel-doc
> > > > ?? drm/i915/gvt: Fix kernel-doc
> > > > ?? drm/i915/gvt: Fix kernel-doc
> > > > ?? drm/i915/gvt: Remove the unused function get_pt_type()
> > > >
> > > > Julia Lawall (1):
> > > > ?? drm/i915/gvt: fix typo in comment
> > > >
> > > > Mauro Carvalho Chehab (1):
> > > > ?? drm/i915: gvt: fix kernel-doc trivial warnings
> > > >
> > > > Paulo Miguel Almeida (1):
> > > > ?? i915/gvt: remove hardcoded value on crc32_start calculation
> > > >
> > > > Zhi Wang (1):
> > > > ?? drm/i915/gvt: remove the vgpu->released and its sanity
> > > > check
> > > >
> > > > wangjianli (1):
> > > > ?? drm/i915: fix repeated words in comments
> > > >
> > > > ??drivers/gpu/drm/i915/gvt/aperture_gm.c?? | 4 ++--
> > > > ??drivers/gpu/drm/i915/gvt/cfg_space.c?? | 2 +-
> > > > ??drivers/gpu/drm/i915/gvt/dmabuf.h | 2 +-
> > > > ??drivers/gpu/drm/i915/gvt/firmware.c | 2 +-
> > > > ??drivers/gpu/drm/i915/gvt/gtt.c?? | 9 ++---
> > > > ??drivers/gpu/drm/i915/gvt/gvt.h?? | 2 --
> > > > ??drivers/gpu/drm/i915/gvt/handlers.c | 4 ++--
> > > > ??drivers/gpu/drm/i915/gvt/kvmgt.c?? | 4
> > > > ??drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +-
> > > > ??drivers/gpu/drm/i915/gvt/page_track.c | 2 +-
> > > > ??drivers/gpu/drm/i915/gvt/vgpu.c | 6 +++---
> > > > ??11 files changed, 14 insertions(+), 25 deletions(-)
> > >
> > >
>
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PULL] gvt-next
On 2022.11.15 10:36:59 -0500, Rodrigo Vivi wrote:
> On Fri, Nov 11, 2022 at 04:59:03PM +0800, Zhenyu Wang wrote:
> > Hi,
> >
> > Here's current accumulated changes in gvt-next. Sorry that I delayed
> > to refresh them on time for upstream...It contains mostly kernel doc,
> > typo fixes and small code cleanups, as details below.
> >
> > btw, one gvt change for next https://patchwork.freedesktop.org/patch/58/
> > is still pending, I need a backmerge from linus tree e.g with recent
> > vfio/mdev
> > consolidate change with gvt and Jason's fix for destroy device, to apply
> > Zhi's
> > change cleanly. Pls help on that.
> >
> > Thanks!
> > ---
> > The following changes since commit a6ebd538364b1e9e6048faaafbc0188172ed50c3:
> >
> > drm/i915/sdvo: Fix debug print (2022-10-28 14:46:21 +0300)
> >
> > are available in the Git repository at:
> >
> > https://github.com/intel/gvt-linux.git tags/gvt-next-2022-11-11
> >
> > for you to fetch changes up to 50468ca2e2e1ce882f060a8c263f678affe112db:
> >
> > drm/i915/gvt: Remove the unused function get_pt_type() (2022-11-08
> > 15:34:06 +0800)
> >
> >
> > gvt-next-2022-11-11
> >
> > - kernel doc fixes
> > - remove vgpu->released sanity check
> > - small clean up
> >
> >
> > Colin Ian King (1):
> > drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported"
>
> dim: d7e4e9579f01 ("drm/i915/reg: Fix spelling mistake "Unsupport" ->
> "Unsupported""): committer Signed-off-by missing.
>
> could you please fix this before we can merge this pr?
>
That should still be .mailmap issue for Colin's email...
But could we improve our dim script to grep mailmap in that case? So if s-o-b
mail is valid
in mailmap, we should still allow it, right?
> >
> > Jiapeng Chong (4):
> > drm/i915/gvt: Fix kernel-doc
> > drm/i915/gvt: Fix kernel-doc
> > drm/i915/gvt: Fix kernel-doc
> > drm/i915/gvt: Remove the unused function get_pt_type()
> >
> > Julia Lawall (1):
> > drm/i915/gvt: fix typo in comment
> >
> > Mauro Carvalho Chehab (1):
> > drm/i915: gvt: fix kernel-doc trivial warnings
> >
> > Paulo Miguel Almeida (1):
> > i915/gvt: remove hardcoded value on crc32_start calculation
> >
> > Zhi Wang (1):
> > drm/i915/gvt: remove the vgpu->released and its sanity check
> >
> > wangjianli (1):
> > drm/i915: fix repeated words in comments
> >
> > drivers/gpu/drm/i915/gvt/aperture_gm.c | 4 ++--
> > drivers/gpu/drm/i915/gvt/cfg_space.c| 2 +-
> > drivers/gpu/drm/i915/gvt/dmabuf.h | 2 +-
> > drivers/gpu/drm/i915/gvt/firmware.c | 2 +-
> > drivers/gpu/drm/i915/gvt/gtt.c | 9 ++---
> > drivers/gpu/drm/i915/gvt/gvt.h | 2 --
> > drivers/gpu/drm/i915/gvt/handlers.c | 4 ++--
> > drivers/gpu/drm/i915/gvt/kvmgt.c| 4
> > drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +-
> > drivers/gpu/drm/i915/gvt/page_track.c | 2 +-
> > drivers/gpu/drm/i915/gvt/vgpu.c | 6 +++---
> > 11 files changed, 14 insertions(+), 25 deletions(-)
>
>
signature.asc
Description: PGP signature
[Intel-gfx] [PULL] gvt-fixes
Hi, Here's two fixes from Sean for 6.1 kernel, which is to fix kvm reference in gvt. No regression found in our test. Thanks! --- The following changes since commit f0c4d9fc9cc9462659728d168387191387e903cc: Linux 6.1-rc4 (2022-11-06 15:07:11 -0800) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-fixes-2022-11-11 for you to fetch changes up to 3c9fd44b9330adc5006653566f3d386784b2080e: drm/i915/gvt: Unconditionally put reference to KVM when detaching vGPU (2022-11-11 13:21:52 +0800) gvt-fixes-2022-11-11 - kvm reference fix from Sean Sean Christopherson (2): drm/i915/gvt: Get reference to KVM iff attachment to VM is successful drm/i915/gvt: Unconditionally put reference to KVM when detaching vGPU drivers/gpu/drm/i915/gvt/kvmgt.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) signature.asc Description: PGP signature
[Intel-gfx] [PULL] gvt-next
Hi, Here's current accumulated changes in gvt-next. Sorry that I delayed to refresh them on time for upstream...It contains mostly kernel doc, typo fixes and small code cleanups, as details below. btw, one gvt change for next https://patchwork.freedesktop.org/patch/58/ is still pending, I need a backmerge from linus tree e.g with recent vfio/mdev consolidate change with gvt and Jason's fix for destroy device, to apply Zhi's change cleanly. Pls help on that. Thanks! --- The following changes since commit a6ebd538364b1e9e6048faaafbc0188172ed50c3: drm/i915/sdvo: Fix debug print (2022-10-28 14:46:21 +0300) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-next-2022-11-11 for you to fetch changes up to 50468ca2e2e1ce882f060a8c263f678affe112db: drm/i915/gvt: Remove the unused function get_pt_type() (2022-11-08 15:34:06 +0800) gvt-next-2022-11-11 - kernel doc fixes - remove vgpu->released sanity check - small clean up Colin Ian King (1): drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported" Jiapeng Chong (4): drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Remove the unused function get_pt_type() Julia Lawall (1): drm/i915/gvt: fix typo in comment Mauro Carvalho Chehab (1): drm/i915: gvt: fix kernel-doc trivial warnings Paulo Miguel Almeida (1): i915/gvt: remove hardcoded value on crc32_start calculation Zhi Wang (1): drm/i915/gvt: remove the vgpu->released and its sanity check wangjianli (1): drm/i915: fix repeated words in comments drivers/gpu/drm/i915/gvt/aperture_gm.c | 4 ++-- drivers/gpu/drm/i915/gvt/cfg_space.c| 2 +- drivers/gpu/drm/i915/gvt/dmabuf.h | 2 +- drivers/gpu/drm/i915/gvt/firmware.c | 2 +- drivers/gpu/drm/i915/gvt/gtt.c | 9 ++--- drivers/gpu/drm/i915/gvt/gvt.h | 2 -- drivers/gpu/drm/i915/gvt/handlers.c | 4 ++-- drivers/gpu/drm/i915/gvt/kvmgt.c| 4 drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +- drivers/gpu/drm/i915/gvt/page_track.c | 2 +- drivers/gpu/drm/i915/gvt/vgpu.c | 6 +++--- 11 files changed, 14 insertions(+), 25 deletions(-) signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 0/2] drm/i915/gvt: Fix for KVM refcounting bug
On 2022.11.11 00:22:23 +, Sean Christopherson wrote: > Bug fix and cleanup related to KVM refcounting. Found by inspection while > attempting to clean up KVM's page-tracker APIs. > > Compile tested only! > > Sean Christopherson (2): > drm/i915/gvt: Get reference to KVM iff attachment to VM is successful > drm/i915/gvt: Unconditionally put reference to KVM when detaching vGPU > > drivers/gpu/drm/i915/gvt/kvmgt.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > Thanks, Sean! and Kevin's review. Pushed this to fixes tree, will issue regression test before going upstream. signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 2/2] drm/i915/gvt: Remove the unused function get_pt_type()
On 2022.09.26 14:40:43 +0800, Jiapeng Chong wrote:
> The function get_pt_type is defined in the gtt.c file, but not
> called elsewhere, so delete this unused function.
>
> drivers/gpu/drm/i915/gvt/gtt.c:285:19: warning: unused function 'get_pt_type'.
>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2277
> Reported-by: Abaci Robot
> Signed-off-by: Jiapeng Chong
> ---
> drivers/gpu/drm/i915/gvt/gtt.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index ce0eb03709c3..6cd4c1d386a5 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -282,11 +282,6 @@ static inline int get_next_pt_type(int type)
> return gtt_type_table[type].next_pt_type;
> }
>
> -static inline int get_pt_type(int type)
> -{
> - return gtt_type_table[type].pt_type;
> -}
> -
> static inline int get_entry_type(int type)
> {
> return gtt_type_table[type].entry_type;
> --
> 2.20.1.7.g153144c
>
Reviewed-by: Zhenyu Wang
thanks!
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH] drm/i915: fix repeated words in comments
On 2022.10.22 14:13:27 +0800, wangjianli wrote: > Delete the redundant word 'the'. > > Signed-off-by: wangjianli > --- > drivers/gpu/drm/i915/gvt/gtt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c > index b4f69364f9a1..7b29ef36941a 100644 > --- a/drivers/gpu/drm/i915/gvt/gtt.c > +++ b/drivers/gpu/drm/i915/gvt/gtt.c > @@ -2785,7 +2785,7 @@ int intel_gvt_init_gtt(struct intel_gvt *gvt) > * intel_gvt_clean_gtt - clean up mm components of a GVT device > * @gvt: GVT device > * > - * This function is called at the driver unloading stage, to clean up the > + * This function is called at the driver unloading stage, to clean up > * the mm components of a GVT device. > * > */ > -- > 2.36.1 > Reviewed-by: Zhenyu Wang thanks signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH] [next] i915/gvt: remove hardcoded value on crc32_start calculation
On 2022.10.30 16:36:28 +1300, Paulo Miguel Almeida wrote: > struct gvt_firmware_header has a crc32 member in which all members that > come after the that field are used to calculate it. The previous > implementation added the value '4' (crc32's u32 size) to calculate the > crc32_start offset which came across as a bit cryptic until you take a > deeper look at the struct. > > This patch changes crc32_start offset to the 'version' member which is > the first member of the struct gvt_firmware_header after crc32. > > It's worth mentioning that doing a build before/after this patch results > in no binary output differences. > > Signed-off-by: Paulo Miguel Almeida > --- > drivers/gpu/drm/i915/gvt/firmware.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gvt/firmware.c > b/drivers/gpu/drm/i915/gvt/firmware.c > index 54fe442238c6..a683c22d5b64 100644 > --- a/drivers/gpu/drm/i915/gvt/firmware.c > +++ b/drivers/gpu/drm/i915/gvt/firmware.c > @@ -104,7 +104,7 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt) > > memcpy(p, gvt->firmware.mmio, info->mmio_size); > > - crc32_start = offsetof(struct gvt_firmware_header, crc32) + 4; > + crc32_start = offsetof(struct gvt_firmware_header, version); > h->crc32 = crc32_le(0, firmware + crc32_start, size - crc32_start); > > firmware_attr.size = size; > -- > 2.25.4 > Reviewed-by: Zhenyu Wang thanks! signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH v2 08/15] drm/i915/gvt: Use the new device life cycle helpers
On 2022.09.01 22:37:40 +0800, Kevin Tian wrote:
> Move vfio_device to the start of intel_vgpu as required by the new
> helpers.
>
> Change intel_gvt_create_vgpu() to use intel_vgpu as the first param
> as other vgpu helpers do.
>
> Signed-off-by: Kevin Tian
> Reviewed-by: Jason Gunthorpe
> ---
Looks good to me.
Reviewed-by: Zhenyu Wang
> drivers/gpu/drm/i915/gvt/gvt.h | 5 ++-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 52 ++--
> drivers/gpu/drm/i915/gvt/vgpu.c | 33
> 3 files changed, 50 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 705689e64011..89fab7896fc6 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -172,6 +172,7 @@ struct intel_vgpu_submission {
> #define KVMGT_DEBUGFS_FILENAME "kvmgt_nr_cache_entries"
>
> struct intel_vgpu {
> + struct vfio_device vfio_device;
> struct intel_gvt *gvt;
> struct mutex vgpu_lock;
> int id;
> @@ -211,7 +212,6 @@ struct intel_vgpu {
>
> u32 scan_nonprivbb;
>
> - struct vfio_device vfio_device;
> struct vfio_region *region;
> int num_regions;
> struct eventfd_ctx *intx_trigger;
> @@ -494,8 +494,7 @@ void intel_gvt_clean_vgpu_types(struct intel_gvt *gvt);
>
> struct intel_vgpu *intel_gvt_create_idle_vgpu(struct intel_gvt *gvt);
> void intel_gvt_destroy_idle_vgpu(struct intel_vgpu *vgpu);
> -struct intel_vgpu *intel_gvt_create_vgpu(struct intel_gvt *gvt,
> - struct intel_vgpu_type *type);
> +int intel_gvt_create_vgpu(struct intel_vgpu *vgpu, struct intel_vgpu_type
> *type);
> void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu);
> void intel_gvt_release_vgpu(struct intel_vgpu *vgpu);
> void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index e3cd58946477..41bba40feef8 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1546,7 +1546,33 @@ static const struct attribute_group
> *intel_vgpu_groups[] = {
> NULL,
> };
>
> +static int intel_vgpu_init_dev(struct vfio_device *vfio_dev)
> +{
> + struct mdev_device *mdev = to_mdev_device(vfio_dev->dev);
> + struct device *pdev = mdev_parent_dev(mdev);
> + struct intel_gvt *gvt = kdev_to_i915(pdev)->gvt;
> + struct intel_vgpu_type *type;
> + struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
> +
> + type = >types[mdev_get_type_group_id(mdev)];
> + if (!type)
> + return -EINVAL;
> +
> + vgpu->gvt = gvt;
> + return intel_gvt_create_vgpu(vgpu, type);
> +}
> +
> +static void intel_vgpu_release_dev(struct vfio_device *vfio_dev)
> +{
> + struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
> +
> + intel_gvt_destroy_vgpu(vgpu);
> + vfio_free_device(vfio_dev);
> +}
> +
> static const struct vfio_device_ops intel_vgpu_dev_ops = {
> + .init = intel_vgpu_init_dev,
> + .release= intel_vgpu_release_dev,
> .open_device= intel_vgpu_open_device,
> .close_device = intel_vgpu_close_device,
> .read = intel_vgpu_read,
> @@ -1558,35 +1584,28 @@ static const struct vfio_device_ops
> intel_vgpu_dev_ops = {
>
> static int intel_vgpu_probe(struct mdev_device *mdev)
> {
> - struct device *pdev = mdev_parent_dev(mdev);
> - struct intel_gvt *gvt = kdev_to_i915(pdev)->gvt;
> - struct intel_vgpu_type *type;
> struct intel_vgpu *vgpu;
> int ret;
>
> - type = >types[mdev_get_type_group_id(mdev)];
> - if (!type)
> - return -EINVAL;
> -
> - vgpu = intel_gvt_create_vgpu(gvt, type);
> + vgpu = vfio_alloc_device(intel_vgpu, vfio_device, >dev,
> + _vgpu_dev_ops);
> if (IS_ERR(vgpu)) {
> gvt_err("failed to create intel vgpu: %ld\n", PTR_ERR(vgpu));
> return PTR_ERR(vgpu);
> }
>
> - vfio_init_group_dev(>vfio_device, >dev,
> - _vgpu_dev_ops);
> -
> dev_set_drvdata(>dev, vgpu);
> ret = vfio_register_emulated_iommu_dev(>vfio_device);
> - if (ret) {
> - intel_gvt_destroy_vgpu(vgpu);
> - return ret;
> - }
> + if (ret)
> + goto out_put_vdev;
>
> gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n",
>dev_name(mdev_dev(mdev)));
>
Re: [Intel-gfx] [PATCH] drm/i915/gvt: fix double-free bug in split_2MB_gtt_entry.
On 2022.09.06 19:36:56 +0800, Zheng Hacker wrote:
> Hi Greg,
>
> Alex has explained how we figured out the patch. We did analyze the
> code and found it possible to reach the vulnerability code. But we
> have no physical device in hand to test the driver. So we'd like to
> discuss with developers to see if the issue exists or not.
>
> Best regards,
> Zheng Wang.
>
> Greg KH ???2022???9???5? 16:04?
> >
> > On Mon, Sep 05, 2022 at 03:46:09PM +0800, Zheng Hacker wrote:
> > > I rewrote the letter. Hope it works.
> > >
> > > There is a double-free security bug in split_2MB_gtt_entry.
> > >
> > > Here is a calling chain :
> > > ppgtt_populate_spt->ppgtt_populate_shadow_entry->split_2MB_gtt_entry.
> > > If intel_gvt_dma_map_guest_page failed, it will call
> > > ppgtt_invalidate_spt, which will finally call ppgtt_free_spt and
> > > kfree(spt). But the caller does not notice that, and it will call
> > > ppgtt_free_spt again in error path.
> > >
It's a little mess in code so in theory it might be possible but
intel_gvt_dma_map_guest_page won't fail in practise...
> > > Fix this by returning the result of ppgtt_invalidate_spt to
> > > split_2MB_gtt_entry.
> > >
I don't see why changing ret value can fix this issue, as it doesn't change
any behavior e.g caller of ppgtt_populate_spt to handle possible different
error return.
As current code looks assuming that ppgtt_invalidate_spt would free spt in good
case,
I think the real cleanup should split that assumption and handle free in error
case properly.
> > > Signed-off-by: Zheng Wang
This misses proper email address.
thanks
> > >
> > > ---
> > > drivers/gpu/drm/i915/gvt/gtt.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c
> > > b/drivers/gpu/drm/i915/gvt/gtt.c
> > > index ce0eb03709c3..9f14fded8c0c 100644
> > > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > > @@ -1215,7 +1215,7 @@ static int split_2MB_gtt_entry(struct intel_vgpu
> > > *vgpu,
> > > ret = intel_gvt_dma_map_guest_page(vgpu, start_gfn +
> > > sub_index,
> > >PAGE_SIZE, _addr);
> > > if (ret) {
> > > - ppgtt_invalidate_spt(spt);
> > > + ret = ppgtt_invalidate_spt(spt);
> > > return ret;
> >
> > But now you just lost the original error, shouldn't this succeed even if
> > intel_gvt_dma_map_guest_page() failed?
> >
> > And how are you causing intel_gvt_dma_map_guest_page() to fail in a real
> > system?
> >
> > thanks,
> >
> > greg k-h
signature.asc
Description: PGP signature
Re: [Intel-gfx] [Bug 216388] New: On Host, kernel errors in KVM, on guests, it shows CPU stalls
On 2022.08.22 17:50:33 +, Sean Christopherson wrote: > +GVT folks > > On Sun, Aug 21, 2022, bugzilla-dae...@kernel.org wrote: > > https://bugzilla.kernel.org/show_bug.cgi?id=216388 > > > > Bug ID: 216388 > >Summary: On Host, kernel errors in KVM, on guests, it shows CPU > > stalls > >Product: Virtualization > >Version: unspecified > > Kernel Version: 5.19.0 / 5.19.1 / 5.19.2 > > Hardware: All > > OS: Linux > > Tree: Mainline > > Status: NEW > > Severity: high > > Priority: P1 > > Component: kvm > > Assignee: virtualization_...@kernel-bugs.osdl.org > > Reporter: nan...@eskimo.com > > Regression: No > > > > Created attachment 301614 > > --> https://bugzilla.kernel.org/attachment.cgi?id=301614=edit > > The configuration file used to Comile this kernel. > > > > This behavior has persisted across 5.19.0, 5.19.1, and 5.19.2. While the > > kernel I am taking this example from is tainted (owing to using Intel > > development drivers for GPU virtualization), it is also occurring on > > non-tainted kernels on servers with no development or third party modules > > installed. > > > > INFO: task CPU 2/KVM:2343 blocked for more than 1228 seconds. > > [207177.050049] Tainted: G UI 5.19.2 #1 > > [207177.050050] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > > this message. > > [207177.050051] task:CPU 2/KVM state:D stack:0 pid: 2343 ppid: > > 1 > > flags:0x0002 > > [207177.050054] Call Trace: > > [207177.050055] > > [207177.050056] __schedule+0x359/0x1400 > > [207177.050060] ? kvm_mmu_page_fault+0x1ee/0x980 > > [207177.050062] ? kvm_set_msr_common+0x31f/0x1060 > > [207177.050065] schedule+0x5f/0x100 > > [207177.050066] schedule_preempt_disabled+0x15/0x30 > > [207177.050068] __mutex_lock.constprop.0+0x4e2/0x750 > > [207177.050070] ? aa_file_perm+0x124/0x4f0 > > [207177.050071] __mutex_lock_slowpath+0x13/0x20 > > [207177.050072] mutex_lock+0x25/0x30 > > [207177.050075] intel_vgpu_emulate_mmio_read+0x5d/0x3b0 [kvmgt] > > This isn't a KVM problem, it's a KVMGT problem (despite the name, KVMGT is > very > much not KVM). > > > [207177.050084] intel_vgpu_rw+0xb8/0x1c0 [kvmgt] > > [207177.050091] intel_vgpu_read+0x20d/0x250 [kvmgt] > > [207177.050097] vfio_device_fops_read+0x1f/0x40 > > [207177.050100] vfs_read+0x9b/0x160 > > [207177.050102] __x64_sys_pread64+0x93/0xd0 > > [207177.050104] do_syscall_64+0x58/0x80 > > [207177.050106] ? kvm_on_user_return+0x84/0xe0 > > [207177.050107] ? fire_user_return_notifiers+0x37/0x70 > > [207177.050109] ? exit_to_user_mode_prepare+0x41/0x200 > > [207177.050111] ? syscall_exit_to_user_mode+0x1b/0x40 > > [207177.050112] ? do_syscall_64+0x67/0x80 > > [207177.050114] ? irqentry_exit+0x54/0x70 > > [207177.050115] ? sysvec_call_function_single+0x4b/0xa0 > > [207177.050116] entry_SYSCALL_64_after_hwframe+0x63/0xcd > > [207177.050118] RIP: 0033:0x7ff51131293f > > [207177.050119] RSP: 002b:7ff4ddffa260 EFLAGS: 0293 ORIG_RAX: > > 0011 > > [207177.050121] RAX: ffda RBX: 5599a6835420 RCX: > > 7ff51131293f > > [207177.050122] RDX: 0004 RSI: 7ff4ddffa2a8 RDI: > > 0027 > > [207177.050123] RBP: 0004 R08: R09: > > > > [207177.050124] R10: 00065f10 R11: 0293 R12: > > 00065f10 > > [207177.050124] R13: 5599a6835330 R14: 0004 R15: > > 00065f10 > > [207177.050126] > > > > I am seeing this on Intel i7-6700k, i7-6850k, and i7-9700k platforms. One recent regression fix on Comet Lake is https://patchwork.freedesktop.org/patch/496987/, it's on the way to 6.0-rc and would be pushed to 5.19 stable as well. But looks this report impacts on more platforms? We'll double check. Thanks > > > > This did not happen on 5.17 kernels, and 5.18 kernels never ran stable > > enough on my platforms to actually run them for more than a few minutes. > > > > Likewise 6.0-rc1 has not been stable enough to run in production. > > After > > less than three hours running on my workstation it locked hard with even the > > magic sys-request key being unresponsive and only power cycling the machine > > got > > it back. > > > > The operating system in use for the host on all machines is Ubuntu > > 22.04. > > > > Guests vary with Ubuntu 22.04 being the most common but also Mint, > > Debian, > > Manjaro, Centos, Fedora, ScientificLinux, Zorin, and Windows being in use. > > > > I see the same issue manifest on platforms running only Ubuntu guests > > as > > with guests of varying operating systems. > > > > The configuration file I used to compile this kernel is attached. I > > compiled it with gcc 12.1.0. > > > > This behavior does not manifest itself
[Intel-gfx] [PULL] gvt-fixes (resend)
(resend after fixing sign-off after rebase) Hi, Here's one gvt-fixes pull for 6.0-rc. Major one is Cometlake regression fix for mmio table rework, and others are left kernel doc fixes not pushed yet. Thanks! -- The following changes since commit a7a47a5dfa9a9692a41764ee9ab4054f12924a42: drm/i915/reset: Add additional steps for Wa_22011802037 for execlist backend (2022-07-25 15:57:54 +0100) are available in the Git repository at: https://github.com/intel/gvt-linux tags/gvt-fixes-2022-08-22 for you to fetch changes up to b75ef35bb57791a5d675699ed4a40c870d1da12f: drm/i915/gvt: Fix Comet Lake (2022-08-22 11:33:12 +0800) gvt-fixes-2022-08-22 - CometLake regression fix in mmio table rework (Alex) - misc kernel doc and typo fixes Alex Williamson (1): drm/i915/gvt: Fix Comet Lake Colin Ian King (1): drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported" Jiapeng Chong (3): drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Fix kernel-doc Julia Lawall (1): drm/i915/gvt: fix typo in comment drivers/gpu/drm/i915/gvt/aperture_gm.c | 4 ++-- drivers/gpu/drm/i915/gvt/gtt.c | 2 +- drivers/gpu/drm/i915/gvt/handlers.c | 4 ++-- drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +- drivers/gpu/drm/i915/intel_gvt_mmio_table.c | 3 ++- 5 files changed, 8 insertions(+), 7 deletions(-) signature.asc Description: PGP signature
Re: [Intel-gfx] [PULL] gvt-fixes
On 2022.08.18 15:43:32 +, Vivi, Rodrigo wrote:
> On Thu, 2022-08-18 at 17:27 +0300, Jani Nikula wrote:
>
> On Thu, 18 Aug 2022, Jani Nikula wrote:
>
> On Wed, 17 Aug 2022, "Colin King (gmail)"
> wrote:
>
> On 17/08/2022 21:07, Vivi, Rodrigo wrote:
>
> On Tue, 2022-08-16 at 12:43 +0800, Zhenyu Wang wrote:
>
> On 2022.08.16 12:05:08 +0800, Zhenyu Wang wrote:
>
> On 2022.08.15 19:32:45 -0400, Rodrigo Vivi wrote:
>
> On Mon, Aug 15, 2022 at 10:38:55AM +0800, Zhenyu
> Wang wrote:
>
> oh, surprise! I just found Colin's email is actually
> defined in
> .mailmap,
> so all his commits in kernel are changed for @intel.com
> address as in
> mailmap...
>
>
> Colin, would you mind to get the Sign-off-by in the patches
> the
> same
> as your authorship so the tools don't get confused?
> (starting with modifying in tree this already merged patch)
>
>
> Since my patches are generally trivial janitorial fixed done in my
> spare
> time I'm going to get the .mailmap changed to use my gmail email
> address
> rather than my Intel one (since I don't do kernel work in my
> current role).
>
> This should clean up the confusion. Apologies.
>
>
> Everyone hold your horses.
>
> I think our tooling should handle the mailmap stuff. The commit *is*
> fine, it's just that the when we check it, we let mailmap alter it. We
> should check the commit without mailmap modifications.
>
>
> In this case, it's actually not about Colin's Signed-off-by or mailmap
> at all! Like the error message from dim says, "committer Signed-off-by
> missing". Committer, not author!
>
> $ git show -s tags/gvt-fixes-2022-08-15^ --pretty=fuller
> commit d6632370536d0b80be3bfc90dd67e1f693335a75
> Author: Colin Ian King
> AuthorDate: Tue Mar 15 20:24:49 2022 +
> Commit: Zhenyu Wang
> CommitDate: Mon Aug 15 10:51:15 2022 +0800
>
> drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported"
>
> There is a spelling mistake in a gvt_vgpu_err error message. Fix it.
>
> Fixes: 695fbc08d80f ("drm/i915/gvt: replace the gvt_err with
> gvt_vgpu_err")
> Signed-off-by: Colin Ian King
> Signed-off-by: Zhi Wang
> Link: http://patchwork.freedesktop.org/patch/msgid/
> 20220315202449.2952845-1-colin.i.k...@gmail.com
> Reviewed-by: Zhi Wang
>
> Committed by Zhenyu, Signed-off-by Zhi. Maybe caused by rebase by Zhenyu
> after being committed by Zhi?
>
>
> Probably easier if you could rebase it again signing it then?
>
Oops, sorry about that, will re-send.
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PULL] gvt-fixes
On 2022.08.16 12:05:08 +0800, Zhenyu Wang wrote:
> On 2022.08.15 19:32:45 -0400, Rodrigo Vivi wrote:
> > On Mon, Aug 15, 2022 at 10:38:55AM +0800, Zhenyu Wang wrote:
> > >
> > > Hi,
> > >
> > > Here's one gvt-fixes pull for 6.0-rc. Major one is Cometlake regression
> > > fix for mmio table rework, and others are left kernel doc fixes not
> > > pushed yet.
> > >
> > > Thanks
> > > --
> > > The following changes since commit
> > > a7a47a5dfa9a9692a41764ee9ab4054f12924a42:
> > >
> > > drm/i915/reset: Add additional steps for Wa_22011802037 for execlist
> > > backend (2022-07-25 15:57:54 +0100)
> > >
> > > are available in the Git repository at:
> > >
> > > https://github.com/intel/gvt-linux.git tags/gvt-fixes-2022-08-15
> > >
> > > for you to fetch changes up to 394f0560a76298842defd1d95bd64b203a5fdcc4:
> > >
> > > drm/i915/gvt: Fix Comet Lake (2022-08-15 10:54:03 +0800)
> > >
> > >
> > > gvt-fixes-2022-08-15
> > >
> > > - CometLake regression fix in mmio table rework (Alex)
> > > - misc kernel doc and typo fixes
> > >
> > >
> > > Alex Williamson (1):
> > > drm/i915/gvt: Fix Comet Lake
> > >
> > > Colin Ian King (1):
> > > drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported"
> >
> > dim: d6632370536d ("drm/i915/reg: Fix spelling mistake "Unsupport" ->
> > "Unsupported""): committer Signed-off-by missing.
> >
> > is it possible to fix this in your tree?
>
> Sorry about that. Let me re-generate.
oh, surprise! I just found Colin's email is actually defined in .mailmap,
so all his commits in kernel are changed for @intel.com address as in mailmap...
So maybe I can't change that?
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PULL] gvt-fixes
On 2022.08.15 19:32:45 -0400, Rodrigo Vivi wrote:
> On Mon, Aug 15, 2022 at 10:38:55AM +0800, Zhenyu Wang wrote:
> >
> > Hi,
> >
> > Here's one gvt-fixes pull for 6.0-rc. Major one is Cometlake regression
> > fix for mmio table rework, and others are left kernel doc fixes not pushed
> > yet.
> >
> > Thanks
> > --
> > The following changes since commit a7a47a5dfa9a9692a41764ee9ab4054f12924a42:
> >
> > drm/i915/reset: Add additional steps for Wa_22011802037 for execlist
> > backend (2022-07-25 15:57:54 +0100)
> >
> > are available in the Git repository at:
> >
> > https://github.com/intel/gvt-linux.git tags/gvt-fixes-2022-08-15
> >
> > for you to fetch changes up to 394f0560a76298842defd1d95bd64b203a5fdcc4:
> >
> > drm/i915/gvt: Fix Comet Lake (2022-08-15 10:54:03 +0800)
> >
> >
> > gvt-fixes-2022-08-15
> >
> > - CometLake regression fix in mmio table rework (Alex)
> > - misc kernel doc and typo fixes
> >
> >
> > Alex Williamson (1):
> > drm/i915/gvt: Fix Comet Lake
> >
> > Colin Ian King (1):
> > drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported"
>
> dim: d6632370536d ("drm/i915/reg: Fix spelling mistake "Unsupport" ->
> "Unsupported""): committer Signed-off-by missing.
>
> is it possible to fix this in your tree?
Sorry about that. Let me re-generate.
signature.asc
Description: PGP signature
[Intel-gfx] [PULL] gvt-fixes
Hi, Here's one gvt-fixes pull for 6.0-rc. Major one is Cometlake regression fix for mmio table rework, and others are left kernel doc fixes not pushed yet. Thanks -- The following changes since commit a7a47a5dfa9a9692a41764ee9ab4054f12924a42: drm/i915/reset: Add additional steps for Wa_22011802037 for execlist backend (2022-07-25 15:57:54 +0100) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-fixes-2022-08-15 for you to fetch changes up to 394f0560a76298842defd1d95bd64b203a5fdcc4: drm/i915/gvt: Fix Comet Lake (2022-08-15 10:54:03 +0800) gvt-fixes-2022-08-15 - CometLake regression fix in mmio table rework (Alex) - misc kernel doc and typo fixes Alex Williamson (1): drm/i915/gvt: Fix Comet Lake Colin Ian King (1): drm/i915/reg: Fix spelling mistake "Unsupport" -> "Unsupported" Jiapeng Chong (3): drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Fix kernel-doc drm/i915/gvt: Fix kernel-doc Julia Lawall (1): drm/i915/gvt: fix typo in comment drivers/gpu/drm/i915/gvt/aperture_gm.c | 4 ++-- drivers/gpu/drm/i915/gvt/gtt.c | 2 +- drivers/gpu/drm/i915/gvt/handlers.c | 4 ++-- drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +- drivers/gpu/drm/i915/intel_gvt_mmio_table.c | 3 ++- 5 files changed, 8 insertions(+), 7 deletions(-) signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH] i915/gvt: Fix Comet Lake
On 2022.08.10 15:55:48 -0600, Alex Williamson wrote:
> Prior to the commit below the GAMT_CHKN_BIT_REG address was setup for
> devices matching (D_KBL | D_CFL), where intel_gvt_get_device_type()
> returns D_CFL for either Coffee Lake or Comet Lake. Include the missed
> platform.`
>
> Link:
> https://lore.kernel.org/all/20220808142711.02d16782.alex.william...@redhat.com
> Fixes: e0f74ed4634d ("i915/gvt: Separate the MMIO tracking table from GVT-g")
> Signed-off-by: Alex Williamson
> ---
> drivers/gpu/drm/i915/intel_gvt_mmio_table.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> index 157e166672d7..5595639d0033 100644
> --- a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> +++ b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c
> @@ -1076,7 +1076,8 @@ static int iterate_skl_plus_mmio(struct
> intel_gvt_mmio_table_iter *iter)
> MMIO_D(GEN8_HDC_CHICKEN1);
> MMIO_D(GEN9_WM_CHICKEN3);
>
> - if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
> + if (IS_KABYLAKE(dev_priv) ||
> + IS_COFFEELAKE(dev_priv) || IS_COMETLAKE(dev_priv))
> MMIO_D(GAMT_CHKN_BIT_REG);
> if (!IS_BROXTON(dev_priv))
> MMIO_D(GEN9_CTX_PREEMPT_REG);
>
Thanks for catching this!
Reviewed-by: Zhenyu Wang
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH v2 07/39] drm/i915: gvt: fix kernel-doc trivial warnings
fn: the gfn of guest page > * > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c > index 46da19b3225d..8e71cda19995 100644 > --- a/drivers/gpu/drm/i915/gvt/vgpu.c > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c > @@ -205,7 +205,7 @@ static void intel_gvt_update_vgpu_types(struct intel_gvt > *gvt) > } > > /** > - * intel_gvt_active_vgpu - activate a virtual GPU > + * intel_gvt_activate_vgpu - activate a virtual GPU > * @vgpu: virtual GPU > * > * This function is called when user wants to activate a virtual GPU. > @@ -219,7 +219,7 @@ void intel_gvt_activate_vgpu(struct intel_vgpu *vgpu) > } > > /** > - * intel_gvt_deactive_vgpu - deactivate a virtual GPU > + * intel_gvt_deactivate_vgpu - deactivate a virtual GPU > * @vgpu: virtual GPU > * > * This function is called when user wants to deactivate a virtual GPU. > @@ -348,7 +348,7 @@ struct intel_vgpu *intel_gvt_create_idle_vgpu(struct > intel_gvt *gvt) > } > > /** > - * intel_gvt_destroy_vgpu - destroy an idle virtual GPU > + * intel_gvt_destroy_idle_vgpu - destroy an idle virtual GPU > * @vgpu: virtual GPU > * > * This function is called when user wants to destroy an idle virtual GPU. > -- Acked-by: Zhenyu Wang Thanks! I'll put in queue with other gvt fixes. signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 04/32] drm/i915: gvt: fix kernel-doc trivial warnings
On 2022.07.11 21:24:49 +0100, Mauro Carvalho Chehab wrote: > Some functions seem to have been renamed without updating the kernel-doc > markup causing warnings. Also, struct intel_vgpu_dmabuf_obj is not > properly documented, but has a kerneld-doc markup. > > Fix those warnings: > drivers/gpu/drm/i915/gvt/aperture_gm.c:308: warning: expecting > prototype for inte_gvt_free_vgpu_resource(). Prototype was for > intel_vgpu_free_resource() instead > drivers/gpu/drm/i915/gvt/aperture_gm.c:344: warning: expecting > prototype for intel_alloc_vgpu_resource(). Prototype was for > intel_vgpu_alloc_resource() instead > drivers/gpu/drm/i915/gvt/cfg_space.c:257: warning: expecting prototype > for intel_vgpu_emulate_cfg_read(). Prototype was for > intel_vgpu_emulate_cfg_write() instead > drivers/gpu/drm/i915/gvt/dmabuf.h:61: warning: Function parameter or > member 'vgpu' not described in 'intel_vgpu_dmabuf_obj' > drivers/gpu/drm/i915/gvt/dmabuf.h:61: warning: Function parameter or > member 'info' not described in 'intel_vgpu_dmabuf_obj' > drivers/gpu/drm/i915/gvt/dmabuf.h:61: warning: Function parameter or > member 'dmabuf_id' not described in 'intel_vgpu_dmabuf_obj' > drivers/gpu/drm/i915/gvt/dmabuf.h:61: warning: Function parameter or > member 'kref' not described in 'intel_vgpu_dmabuf_obj' > drivers/gpu/drm/i915/gvt/dmabuf.h:61: warning: Function parameter or > member 'initref' not described in 'intel_vgpu_dmabuf_obj' > drivers/gpu/drm/i915/gvt/dmabuf.h:61: warning: Function parameter or > member 'list' not described in 'intel_vgpu_dmabuf_obj' > drivers/gpu/drm/i915/gvt/handlers.c:3066: warning: expecting prototype > for intel_t_default_mmio_write(). Prototype was for > intel_vgpu_default_mmio_write() instead > drivers/gpu/drm/i915/gvt/mmio_context.c:560: warning: expecting > prototype for intel_gvt_switch_render_mmio(). Prototype was for > intel_gvt_switch_mmio() instead > drivers/gpu/drm/i915/gvt/page_track.c:131: warning: expecting prototype > for intel_vgpu_enable_page_track(). Prototype was for > intel_vgpu_disable_page_track() instead > drivers/gpu/drm/i915/gvt/vgpu.c:215: warning: expecting prototype for > intel_gvt_active_vgpu(). Prototype was for intel_gvt_activate_vgpu() instead > drivers/gpu/drm/i915/gvt/vgpu.c:230: warning: expecting prototype for > intel_gvt_deactive_vgpu(). Prototype was for intel_gvt_deactivate_vgpu() > instead > drivers/gpu/drm/i915/gvt/vgpu.c:358: warning: expecting prototype for > intel_gvt_destroy_vgpu(). Prototype was for intel_gvt_destroy_idle_vgpu() > instead > > Signed-off-by: Mauro Carvalho Chehab > --- Hi, thanks for this, but there're already several fixes in queue right now, e.g https://patchwork.freedesktop.org/series/104302/ and https://patchwork.freedesktop.org/series/104640/, but looks there're other uncaught issues. I'd like to submit current in queue first, then maybe you could update for others? signature.asc Description: PGP signature
[Intel-gfx] [PULL] gvt-fixes
Hi, Here's one gvt fix for 5.19, from Dan for shmem_pin_map() return check bug. Thanks! --- The following changes since commit d72d69abfdb6e0375981cfdda8eb45143f12c77d: drm/i915/gvt: Make DRM_I915_GVT depend on X86 (2022-01-13 18:13:12 +) are available in the Git repository at: https://github.com/intel/gvt-linux.git tags/gvt-fixes-2022-07-11 for you to fetch changes up to e87197fbd137c888fd6c871c72fe7e89445dd015: drm/i915/gvt: IS_ERR() vs NULL bug in intel_gvt_update_reg_whitelist() (2022-07-11 13:05:05 +0800) gvt-fixes-2022-07-11 - Fix return value for shmem_pin_map() Dan Carpenter (1): drm/i915/gvt: IS_ERR() vs NULL bug in intel_gvt_update_reg_whitelist() drivers/gpu/drm/i915/gvt/cmd_parser.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Fix kernel-doc
On 2022.06.02 15:35:19 +0800, Jiapeng Chong wrote: > Fix the following W=1 kernel warnings: > > drivers/gpu/drm/i915/gvt/aperture_gm.c:308: warning: expecting prototype > for inte_gvt_free_vgpu_resource(). Prototype was for > intel_vgpu_free_resource() instead. > > drivers/gpu/drm/i915/gvt/aperture_gm.c:344: warning: expecting prototype > for intel_alloc_vgpu_resource(). Prototype was for > intel_vgpu_alloc_resource() instead. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/gpu/drm/i915/gvt/aperture_gm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/aperture_gm.c > b/drivers/gpu/drm/i915/gvt/aperture_gm.c > index 557f3314291a..3b81a6d35a7b 100644 > --- a/drivers/gpu/drm/i915/gvt/aperture_gm.c > +++ b/drivers/gpu/drm/i915/gvt/aperture_gm.c > @@ -298,7 +298,7 @@ static int alloc_resource(struct intel_vgpu *vgpu, > } > > /** > - * inte_gvt_free_vgpu_resource - free HW resource owned by a vGPU > + * intel_vgpu_free_resource() - free HW resource owned by a vGPU > * @vgpu: a vGPU > * > * This function is used to free the HW resource owned by a vGPU. > @@ -328,7 +328,7 @@ void intel_vgpu_reset_resource(struct intel_vgpu *vgpu) > } > > /** > - * intel_alloc_vgpu_resource - allocate HW resource for a vGPU > + * intel_vgpu_alloc_resource() - allocate HW resource for a vGPU > * @vgpu: vGPU > * @param: vGPU creation params > * > -- > 2.20.1.7.g153144c > Acked-by: Zhenyu Wang Thanks! signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 2/2] drm/i915/gvt: Fix kernel-doc
On 2022.05.24 16:37:32 +0800, Jiapeng Chong wrote: > Fix the following W=1 kernel warnings: > > drivers/gpu/drm/i915/gvt/mmio_context.c:560: warning: expecting > prototype for intel_gvt_switch_render_mmio(). Prototype was for > intel_gvt_switch_mmio() instead. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c > b/drivers/gpu/drm/i915/gvt/mmio_context.c > index c85bafe7539e..1c6e941c9666 100644 > --- a/drivers/gpu/drm/i915/gvt/mmio_context.c > +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c > @@ -546,7 +546,7 @@ static void switch_mmio(struct intel_vgpu *pre, > } > > /** > - * intel_gvt_switch_render_mmio - switch mmio context of specific engine > + * intel_gvt_switch_mmio - switch mmio context of specific engine > * @pre: the last vGPU that own the engine > * @next: the vGPU to switch to > * @engine: the engine > -- > 2.20.1.7.g153144c > Acked-by: Zhenyu Wang Thanks! signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Fix kernel-doc
On 2022.05.24 16:37:33 +0800, Jiapeng Chong wrote: > Fix the following W=1 kernel warnings: > > drivers/gpu/drm/i915/gvt/handlers.c:3066: warning: expecting prototype > for intel_t_default_mmio_write(). Prototype was for > intel_vgpu_default_mmio_write() instead. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/gpu/drm/i915/gvt/handlers.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > b/drivers/gpu/drm/i915/gvt/handlers.c > index beea5895e499..9c8dde079cb4 100644 > --- a/drivers/gpu/drm/i915/gvt/handlers.c > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > @@ -3052,7 +3052,7 @@ int intel_vgpu_default_mmio_read(struct intel_vgpu > *vgpu, unsigned int offset, > } > > /** > - * intel_t_default_mmio_write - default MMIO write handler > + * intel_vgpu_default_mmio_write() - default MMIO write handler > * @vgpu: a vGPU > * @offset: access offset > * @p_data: write data buffer > -- > 2.20.1.7.g153144c > Sorry for late reply! Acked-by: Zhenyu Wang signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH] drm/i915/gvt: fix typo in comment
On 2022.05.21 13:10:59 +0200, Julia Lawall wrote:
> Spelling mistake (triple letters) in comment.
> Detected with the help of Coccinelle.
>
> Signed-off-by: Julia Lawall
>
> ---
> drivers/gpu/drm/i915/gvt/gtt.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 9c5cc2800975..c919f14c4fcb 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -2341,7 +2341,7 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu
> *vgpu, unsigned int off,
> gvt_vgpu_err("fail to populate guest ggtt entry\n");
> /* guest driver may read/write the entry when partial
>* update the entry in this situation p2m will fail
> - * settting the shadow entry to point to a scratch page
> + * setting the shadow entry to point to a scratch page
>*/
> ops->set_pfn(, gvt->gtt.scratch_mfn);
> } else
>
Sorry for late reply!
Acked-by: Zhenyu Wang
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH] drm/i915/gvt: IS_ERR() vs NULL bug in intel_gvt_update_reg_whitelist()
On 2022.07.08 10:55:52 +0200, Andrzej Hajda wrote:
> On 08.07.2022 10:41, Dan Carpenter wrote:
> > The shmem_pin_map() function returns NULL, it doesn't return error
> > pointers.
> >
> > Fixes: 97ea656521c8 ("drm/i915/gvt: Parse default state to update reg
> > whitelist")
> > Signed-off-by: Dan Carpenter
>
> Reviewed-by: Andrzej Hajda
>
Acked-by: Zhenyu Wang
Thanks! Will push to -fixes.
>
> > ---
> > drivers/gpu/drm/i915/gvt/cmd_parser.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > index b9eb75a2b400..1c35a41620ae 100644
> > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > @@ -3117,9 +3117,9 @@ void intel_gvt_update_reg_whitelist(struct intel_vgpu
> > *vgpu)
> > continue;
> > vaddr = shmem_pin_map(engine->default_state);
> > - if (IS_ERR(vaddr)) {
> > - gvt_err("failed to map %s->default state, err:%zd\n",
> > - engine->name, PTR_ERR(vaddr));
> > + if (!vaddr) {
> > + gvt_err("failed to map %s->default state\n",
> > + engine->name);
> > return;
> > }
>
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH v3 1/2] vfio: Replace the DMA unmapping notifier with a callback
On 2022.07.04 21:59:03 -0300, Jason Gunthorpe wrote:
> Instead of having drivers register the notifier with explicit code just
> have them provide a dma_unmap callback op in their driver ops and rely on
> the core code to wire it up.
>
> Suggested-by: Christoph Hellwig
> Reviewed-by: Christoph Hellwig
> Reviewed-by: Kevin Tian
> Reviewed-by: Tony Krowiak
> Reviewed-by: Eric Farman
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/gpu/drm/i915/gvt/gvt.h| 1 -
> drivers/gpu/drm/i915/gvt/kvmgt.c | 75 ---
gvt change looks fine to me.
Reviewed-by: Zhenyu Wang
> drivers/s390/cio/vfio_ccw_ops.c | 41 ++--
> drivers/s390/cio/vfio_ccw_private.h | 2 -
> drivers/s390/crypto/vfio_ap_ops.c | 53 ++-
> drivers/s390/crypto/vfio_ap_private.h | 3 -
> drivers/vfio/vfio.c | 129 +-
> drivers/vfio/vfio.h | 3 +
> include/linux/vfio.h | 21 +
> 9 files changed, 88 insertions(+), 240 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index aee1a45da74bcb..705689e6401197 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -226,7 +226,6 @@ struct intel_vgpu {
> unsigned long nr_cache_entries;
> struct mutex cache_lock;
>
> - struct notifier_block iommu_notifier;
> atomic_t released;
>
> struct kvm_page_track_notifier_node track_node;
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index e2f6c56ab3420c..ecd5bb37b63a2a 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -729,34 +729,25 @@ int intel_gvt_set_edid(struct intel_vgpu *vgpu, int
> port_num)
> return ret;
> }
>
> -static int intel_vgpu_iommu_notifier(struct notifier_block *nb,
> - unsigned long action, void *data)
> +static void intel_vgpu_dma_unmap(struct vfio_device *vfio_dev, u64 iova,
> + u64 length)
> {
> - struct intel_vgpu *vgpu =
> - container_of(nb, struct intel_vgpu, iommu_notifier);
> -
> - if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) {
> - struct vfio_iommu_type1_dma_unmap *unmap = data;
> - struct gvt_dma *entry;
> - unsigned long iov_pfn, end_iov_pfn;
> + struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
> + struct gvt_dma *entry;
> + u64 iov_pfn = iova >> PAGE_SHIFT;
> + u64 end_iov_pfn = iov_pfn + length / PAGE_SIZE;
>
> - iov_pfn = unmap->iova >> PAGE_SHIFT;
> - end_iov_pfn = iov_pfn + unmap->size / PAGE_SIZE;
> + mutex_lock(>cache_lock);
> + for (; iov_pfn < end_iov_pfn; iov_pfn++) {
> + entry = __gvt_cache_find_gfn(vgpu, iov_pfn);
> + if (!entry)
> + continue;
>
> - mutex_lock(>cache_lock);
> - for (; iov_pfn < end_iov_pfn; iov_pfn++) {
> - entry = __gvt_cache_find_gfn(vgpu, iov_pfn);
> - if (!entry)
> - continue;
> -
> - gvt_dma_unmap_page(vgpu, entry->gfn, entry->dma_addr,
> -entry->size);
> - __gvt_cache_remove_entry(vgpu, entry);
> - }
> - mutex_unlock(>cache_lock);
> + gvt_dma_unmap_page(vgpu, entry->gfn, entry->dma_addr,
> +entry->size);
> + __gvt_cache_remove_entry(vgpu, entry);
> }
> -
> - return NOTIFY_OK;
> + mutex_unlock(>cache_lock);
> }
>
> static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
> @@ -783,36 +774,20 @@ static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu)
> static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
> {
> struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
> - unsigned long events;
> - int ret;
> -
> - vgpu->iommu_notifier.notifier_call = intel_vgpu_iommu_notifier;
>
> - events = VFIO_IOMMU_NOTIFY_DMA_UNMAP;
> - ret = vfio_register_notifier(vfio_dev, VFIO_IOMMU_NOTIFY, ,
> - >iommu_notifier);
> - if (ret != 0) {
> - gvt_vgpu_err("vfio_register_notifier for iommu failed: %d\n",
> - ret);
> - goto out;
> - }
> -
> - ret = -EEXIST;
> if (vgpu->attached)
> - goto undo_iommu;
> + return -EEXIST;
>
> - ret
Re: [Intel-gfx] [PATCH v9 1/3] i915/gvt: Separate the MMIO tracking table from GVT-g
On 2022.04.07 03:19:43 -0400, Zhi Wang wrote: > From: Zhi Wang > > To support the new mdev interfaces and the re-factor patches from > Christoph, which moves the GVT-g code into a dedicated module, the GVT-g > MMIO tracking table needs to be separated from GVT-g. > Looks fine to me. Thanks! Reviewed-by: Zhenyu Wang signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH v2] drm/i915/tgl: Simply subplatform detection
On 2022.02.22 14:21:33 +, Tvrtko Ursulin wrote:
>
> On 22/02/2022 14:14, Jos?? Roberto de Souza wrote:
> > In the past we had a need to differentiate TGL U and TGL Y, there
> > was a different voltage swing table for each subplatform and some PCI
> > ids of this subplatforms are shared but it turned out that it was a
> > specification mistake and the voltage swing table was indeed the same
> > but we went ahead with that patch because we needed to differentiate
> > TGL U and Y from TGL H and by that time TGL H was embargoed so that
> > was the perfect way to land it upstream.
> >
> > Now the embargo for TGL H is long past and now we even have
> > INTEL_TGL_12_GT1_IDS with all TGL H ids, so we can drop this PCI root
> > check and only rely in the PCI ids to differentiate TGL U and Y from
> > TGL H that actually has code differences.
> >
> > Besides the simplification this will fix issues in virtualization
> > environments where the PCI root is virtualized and don't have the same
> > id as actual hardware.
> >
> > v2:
> > - add and set INTEL_SUBPLATFORM_UY
>
> LGTM, thanks for the tweak!
>
> Looks mechanical enough so:
>
> Reviewed-by: Tvrtko Ursulin
>
Add Yu, who has been testing this under GPU passthrough case as well,
which now release our effort for root pci device info passthrough issue.
Thanks!
>
> > Cc: Fred Gao
> > Cc: Tvrtko Ursulin
> > Signed-off-by: Jos?? Roberto de Souza
> > ---
> > .../drm/i915/display/intel_ddi_buf_trans.c| 2 +-
> > drivers/gpu/drm/i915/i915_drv.h | 11 +++-
> > drivers/gpu/drm/i915/i915_reg.h | 6 -
> > drivers/gpu/drm/i915/intel_device_info.c | 26 +--
> > drivers/gpu/drm/i915/intel_device_info.h | 3 +++
> > drivers/gpu/drm/i915/intel_step.c | 2 +-
> > 6 files changed, 16 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> > b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> > index 0c32210bf5031..934a9f9e7dabb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> > @@ -1321,7 +1321,7 @@ tgl_get_combo_buf_trans_dp(struct intel_encoder
> > *encoder,
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > if (crtc_state->port_clock > 27) {
> > - if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv)) {
> > + if (IS_TGL_UY(dev_priv)) {
> > return
> > intel_get_buf_trans(_uy_combo_phy_trans_dp_hbr2,
> >n_entries);
> > } else {
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 1c2f4ae4ebf98..51417e9b740f4 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1147,11 +1147,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> > #define IS_ICL_WITH_PORT_F(dev_priv) \
> > IS_SUBPLATFORM(dev_priv, INTEL_ICELAKE, INTEL_SUBPLATFORM_PORTF)
> > -#define IS_TGL_U(dev_priv) \
> > - IS_SUBPLATFORM(dev_priv, INTEL_TIGERLAKE, INTEL_SUBPLATFORM_ULT)
> > -
> > -#define IS_TGL_Y(dev_priv) \
> > - IS_SUBPLATFORM(dev_priv, INTEL_TIGERLAKE, INTEL_SUBPLATFORM_ULX)
> > +#define IS_TGL_UY(dev_priv) \
> > + IS_SUBPLATFORM(dev_priv, INTEL_TIGERLAKE, INTEL_SUBPLATFORM_UY)
> > #define IS_SKL_GRAPHICS_STEP(p, since, until) (IS_SKYLAKE(p) &&
> > IS_GRAPHICS_STEP(p, since, until))
> > @@ -1170,11 +1167,11 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> > IS_DISPLAY_STEP(__i915, since, until))
> > #define IS_TGL_UY_GRAPHICS_STEP(__i915, since, until) \
> > - ((IS_TGL_U(__i915) || IS_TGL_Y(__i915)) && \
> > + (IS_TGL_UY(__i915) && \
> > IS_GRAPHICS_STEP(__i915, since, until))
> > #define IS_TGL_GRAPHICS_STEP(__i915, since, until) \
> > - (IS_TIGERLAKE(__i915) && !(IS_TGL_U(__i915) || IS_TGL_Y(__i915)) && \
> > + (IS_TIGERLAKE(__i915) && !IS_TGL_UY(__i915)) && \
> > IS_GRAPHICS_STEP(__i915, since, until))
> > #define IS_RKL_DISPLAY_STEP(p, since, until) \
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 2b8a3086ed35a..30aa1d99f2244 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -8823,12 +8823,6 @@ enum skl_power_gate {
> > #define DSB_ENABLE (1 << 31)
> > #define DSB_STATUS (1 << 0)
> > -#define TGL_ROOT_DEVICE_ID 0x9A00
> > -#define TGL_ROOT_DEVICE_MASK 0xFF00
> > -#define TGL_ROOT_DEVICE_SKU_MASK 0xF
> > -#define TGL_ROOT_DEVICE_SKU_ULX0x2
> > -#define TGL_ROOT_DEVICE_SKU_ULT0x4
> > -
> > #define CLKREQ_POLICY _MMIO(0x101038)
> > #define CLKREQ_POLICY_MEM_UP_OVRDREG_BIT(1)
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> >
Re: [Intel-gfx] [PATCH v8 10/16] i915/gvt: remove spaces in pr_debug "gvt: core:" etc prefixes
On 2021.09.15 10:39:51 -0600, Jim Cromie wrote:
> Taking embedded spaces out of existing prefixes makes them better
> class-prefixes; simplifying the extra quoting needed otherwise:
>
> $> echo format "^gvt: core:" +p >control
>
> Dropping the internal spaces means any trailing space in a query will
> more clearly terminate the prefix being searched for.
>
> Consider a generic drm-debug example:
>
> # turn off ATOMIC reports
> echo format "^drm:atomic: " -p > control
>
> # turn off all ATOMIC:* reports, including any sub-categories
> echo format "^drm:atomic:" -p > control
>
> # turn on ATOMIC:FAIL: reports
> echo format "^drm:atomic:fail: " +p > control
>
> Removing embedded spaces in the class-prefixes simplifies the
> corresponding match-prefix. This means that "quoted" match-prefixes
> are only needed when the trailing space is desired, in order to
> exclude explicitly sub-categorized pr-debugs; in this example,
> "drm:atomic:fail:".
>
> RFC: maybe the prefix catenation should paste in the " " class-prefix
> terminator explicitly. A pr_debug_() flavor could exclude the " ",
> allowing ad-hoc sub-categorization by appending for example, "fail:"
> to "drm:atomic:" without the default " " insertion.
>
> Signed-off-by: Jim Cromie
> ---
> v8:
> . fix patchwork CI warning
> ---
> drivers/gpu/drm/i915/gvt/debug.h | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/debug.h
> b/drivers/gpu/drm/i915/gvt/debug.h
> index c6027125c1ec..bbecc279e077 100644
> --- a/drivers/gpu/drm/i915/gvt/debug.h
> +++ b/drivers/gpu/drm/i915/gvt/debug.h
> @@ -36,30 +36,30 @@ do {
> \
> } while (0)
>
> #define gvt_dbg_core(fmt, args...) \
> - pr_debug("gvt: core: "fmt, ##args)
> + pr_debug("gvt:core: " fmt, ##args)
>
> #define gvt_dbg_irq(fmt, args...) \
> - pr_debug("gvt: irq: "fmt, ##args)
> + pr_debug("gvt:irq: " fmt, ##args)
>
> #define gvt_dbg_mm(fmt, args...) \
> - pr_debug("gvt: mm: "fmt, ##args)
> + pr_debug("gvt:mm: " fmt, ##args)
>
> #define gvt_dbg_mmio(fmt, args...) \
> - pr_debug("gvt: mmio: "fmt, ##args)
> + pr_debug("gvt:mmio: " fmt, ##args)
>
> #define gvt_dbg_dpy(fmt, args...) \
> - pr_debug("gvt: dpy: "fmt, ##args)
> + pr_debug("gvt:dpy: " fmt, ##args)
>
> #define gvt_dbg_el(fmt, args...) \
> - pr_debug("gvt: el: "fmt, ##args)
> + pr_debug("gvt:el: " fmt, ##args)
>
> #define gvt_dbg_sched(fmt, args...) \
> - pr_debug("gvt: sched: "fmt, ##args)
> + pr_debug("gvt:sched: " fmt, ##args)
>
> #define gvt_dbg_render(fmt, args...) \
> - pr_debug("gvt: render: "fmt, ##args)
> + pr_debug("gvt:render: " fmt, ##args)
>
> #define gvt_dbg_cmd(fmt, args...) \
> - pr_debug("gvt: cmd: "fmt, ##args)
> + pr_debug("gvt:cmd: " fmt, ##args)
>
> #endif
> --
Looks good to me. Thanks!
Reviewed-by: Zhenyu Wang
signature.asc
Description: PGP signature
[Intel-gfx] [PULL] gvt-fixes
Hi, Here's one ww lock fini fix from Zhi which resolved recent regression with i915 change. Thanks -- The following changes since commit 71de496cc489b6bae2f51f89da7f28849bf2836e: drm/i915/dp: Drop redundant debug print (2021-08-26 07:31:52 -0400) are available in the Git repository at: https://github.com/intel/gvt-linux tags/gvt-fixes-2021-09-18 for you to fetch changes up to d168cd797982db9db617113644c87b8f5f3cf27e: drm/i915/gvt: fix the usage of ww lock in gvt scheduler. (2021-09-13 21:59:31 +0800) gvt-fixes-2021-09-18 - ww locking fix from Zhi Zhi A Wang (1): drm/i915/gvt: fix the usage of ww lock in gvt scheduler. drivers/gpu/drm/i915/gvt/scheduler.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) signature.asc Description: PGP signature
Re: [Intel-gfx] refactor the i915 GVT support
On 2021.08.20 12:56:34 -0700, Luis Chamberlain wrote:
> On Fri, Aug 20, 2021 at 04:17:24PM +0200, Christoph Hellwig wrote:
> > On Thu, Aug 19, 2021 at 04:29:29PM +0800, Zhenyu Wang wrote:
> > > I'm working on below patch to resolve this. But I met a weird issue in
> > > case when building i915 as module and also kvmgt module, it caused
> > > busy wait on request_module("kvmgt") when boot, it doesn't happen if
> > > building i915 into kernel. I'm not sure what could be the reason?
> >
> > Luis, do you know if there is a problem with a request_module from
> > a driver ->probe routine that is probably called by a module_init
> > function itself?
>
> Generally no, but you can easily foot yourself in the feet by creating
> cross dependencies and not dealing with them properly. I'd make sure
> to keep module initialization as simple as possible, and run whatever
> takes more time asynchronously, then use a state machine to allow
> you to verify where you are in the initialization phase or query it
> or wait for a completion with a timeout.
>
> It seems the code in question is getting some spring cleaning, and its
> unclear where the code is I can inspect. If there's a tree somewhere I
> can take a peak I'd be happy to review possible oddities that may stick
> out.
I tried to put current patches under test here:
https://github.com/intel/gvt-linux/tree/gvt-staging
The issue can be produced with CONFIG_DRM_I915=m and
CONFIG_DRM_I915_GVT_KVMGT=m.
>
> My goto model for these sorts of problems is to abstract the issue
> *outside* of the driver in question and implement new selftests to
> try to reproduce. This serves two purposes, 1) helps with testing
> 2) may allow you to see the problem more clearly.
>
I'll see if can abstract that.
Thanks, Luis.
signature.asc
Description: PGP signature
Re: [Intel-gfx] refactor the i915 GVT support
On 2021.08.20 16:17:24 +0200, Christoph Hellwig wrote:
> On Thu, Aug 19, 2021 at 04:29:29PM +0800, Zhenyu Wang wrote:
> > I'm working on below patch to resolve this. But I met a weird issue in
> > case when building i915 as module and also kvmgt module, it caused
> > busy wait on request_module("kvmgt") when boot, it doesn't happen if
> > building i915 into kernel. I'm not sure what could be the reason?
>
> Luis, do you know if there is a problem with a request_module from
> a driver ->probe routine that is probably called by a module_init
> function itself?
>
> In the meantime I'll try to reproduce it locally, but I always had a
> hard time getting useful results out of a modular i915, especially
> when combined with module paramters. (no blame on i915, just the problem
> with modules needed early on).
>
> >
> > > But the problem I see is that after moving gvt device model (gvt/*.c
> > > except kvmgt.c) into kvmgt module, we'll have issue with initial mmio
> > > state which current gvt relies on, that is in design supposed to get
> > > initial HW state before i915 driver has taken any operation. Before
> > > we can ensure that, I think we may only remove MPT part first but
> > > still keep gvt device model as part of i915 with config. I'll try to
> > > split that out.
> >
> > Sorry I misread the code that as we always request kvmgt module when
> > gvt init, so it should still apply original method that this isn't a
> > problem. Our current validation result has shown no regression as well.
>
> What does initial mmio state mean? This is something new to me. But
> as you said in this mail unless I missed something very big it should
> work the same as before.
>
It's gvt internal track for all gfx mmio state, and yes with your current
change it should still work as before.
> > -static inline void intel_context_unpin(struct intel_context *ce)
> > +static inline void _intel_context_unpin(struct intel_context *ce)
> > {
> > if (!ce->ops->sched_disable) {
> > __intel_context_do_unpin(ce, 1);
> > @@ -150,6 +150,7 @@ static inline void intel_context_unpin(struct
> > intel_context *ce)
> > }
> > }
> > }
> > +void intel_context_unpin(struct intel_context *ce);
>
> Looking at intel_context_unpin/_intel_context_unpin is there really
> a need to have this inline to start with? It don't see much the compiler
> could optimize by inlining it.
I'll send patch to i915 for this, and get more comments there.
thanks
signature.asc
Description: PGP signature
Re: [Intel-gfx] refactor the i915 GVT support
On 2021.08.19 17:43:43 +0300, Joonas Lahtinen wrote:
> Quoting Zhenyu Wang (2021-08-19 11:29:29)
> > On 2021.08.17 13:22:03 +0800, Zhenyu Wang wrote:
> > > > On 2021.08.16 19:34:58 +0200, Christoph Hellwig wrote:
> > > > > Any updates on this? I'd really hate to miss this merge window.
> > > >
> > > > I'm still waiting for our validation team's report on this. I'm afraid
> > > > it might be missing for next version as i915 merge window is mostly
> > > > till rc5...and for any change outside of gvt, it still needs to be
> > > > acked by i915 maintainers.
> > >
> > > Looks our validation team did have problem against recent i915 change.
> > > If you like to try, we have a gvt-staging branch on
> > > https://github.com/intel/gvt-linux which is generated against drm-tip
> > > with gvt changes for testing, currently it's broken.
> > >
> > > One issue is with i915 export that intel_context_unpin has been
> > > changed into static inline function. Another is that intel_gvt.c
> > > should be part of i915 for gvt interface instead of depending on KVMGT
> > > config.
> >
> > I'm working on below patch to resolve this. But I met a weird issue in
> > case when building i915 as module and also kvmgt module, it caused
> > busy wait on request_module("kvmgt") when boot, it doesn't happen if
> > building i915 into kernel. I'm not sure what could be the reason?
> >
> > > But the problem I see is that after moving gvt device model (gvt/*.c
> > > except kvmgt.c) into kvmgt module, we'll have issue with initial mmio
> > > state which current gvt relies on, that is in design supposed to get
> > > initial HW state before i915 driver has taken any operation.
>
> As mentioned in some past discussions, I think it would be best rely on
> golden MMIO located in /lib/firmware or elsewhere. This way we will better
> isolate the guest system from host system updates/changes.
>
> This should also hopefully allow enabling kvmgt module after i915 has
> already loaded, as the initialization would not be conditional to
> capture the MMIO.
>
I think the concern is that even for same GEN hw there could be many
variant platforms e.g APL with gen9, etc. To verify golden states for
them all might take too much effort...
>
> > > Before
> > > we can ensure that, I think we may only remove MPT part first but
> > > still keep gvt device model as part of i915 with config. I'll try to
> > > split that out.
> >
> > Sorry I misread the code that as we always request kvmgt module when
> > gvt init, so it should still apply original method that this isn't a
> > problem. Our current validation result has shown no regression as well.
> >
> > ---8<---
> > From 58ff84572f1a0f9d79ca1d7ec0cff5ecbe78d280 Mon Sep 17 00:00:00 2001
> > From: Zhenyu Wang
> > Date: Thu, 19 Aug 2021 16:36:33 +0800
> > Subject: [PATCH] TESTONLY:drm/i915/gvt: potential fix for refactor against
> > current tip
> >
> > ---
> > drivers/gpu/drm/i915/Makefile | 4 +++-
> > drivers/gpu/drm/i915/gt/intel_context.c | 5 +
> > drivers/gpu/drm/i915/gt/intel_context.h | 3 ++-
> > drivers/gpu/drm/i915/i915_trace.h | 1 +
> > 4 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index c4f953837f72..2248574428a1 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -296,7 +296,9 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \
> >
> > # virtual gpu code
> > i915-y += i915_vgpu.o
> > -i915-$(CONFIG_DRM_I915_GVT_KVMGT) += intel_gvt.o
> > +ifneq ($(CONFIG_DRM_I915_GVT_KVMGT),)
> > +i915-y += intel_gvt.o
> > +endif
> >
> > kvmgt-y += gvt/kvmgt.o \
> > gvt/gvt.o \
> > diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
> > b/drivers/gpu/drm/i915/gt/intel_context.c
> > index 745e84c72c90..20e7522fed84 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_context.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> > @@ -328,6 +328,11 @@ void __intel_context_do_unpin(struct intel_context
> > *ce, int sub)
> > intel_context_put(ce);
> > }
> >
> > +void intel_context_unpin(struct intel_context *ce)
> > +{
> > + _intel_context_unpin(ce);
> > +}
> > +
> > static void __intel_context_retire(struct i915_active *active)
> > {
> > struct intel_context *ce =
Re: [Intel-gfx] refactor the i915 GVT support
On 2021.08.17 13:22:03 +0800, Zhenyu Wang wrote:
> > On 2021.08.16 19:34:58 +0200, Christoph Hellwig wrote:
> > > Any updates on this? I'd really hate to miss this merge window.
> >
> > I'm still waiting for our validation team's report on this. I'm afraid
> > it might be missing for next version as i915 merge window is mostly
> > till rc5...and for any change outside of gvt, it still needs to be
> > acked by i915 maintainers.
>
> Looks our validation team did have problem against recent i915 change.
> If you like to try, we have a gvt-staging branch on
> https://github.com/intel/gvt-linux which is generated against drm-tip
> with gvt changes for testing, currently it's broken.
>
> One issue is with i915 export that intel_context_unpin has been
> changed into static inline function. Another is that intel_gvt.c
> should be part of i915 for gvt interface instead of depending on KVMGT
> config.
I'm working on below patch to resolve this. But I met a weird issue in
case when building i915 as module and also kvmgt module, it caused
busy wait on request_module("kvmgt") when boot, it doesn't happen if
building i915 into kernel. I'm not sure what could be the reason?
> But the problem I see is that after moving gvt device model (gvt/*.c
> except kvmgt.c) into kvmgt module, we'll have issue with initial mmio
> state which current gvt relies on, that is in design supposed to get
> initial HW state before i915 driver has taken any operation. Before
> we can ensure that, I think we may only remove MPT part first but
> still keep gvt device model as part of i915 with config. I'll try to
> split that out.
Sorry I misread the code that as we always request kvmgt module when
gvt init, so it should still apply original method that this isn't a
problem. Our current validation result has shown no regression as well.
---8<---
From 58ff84572f1a0f9d79ca1d7ec0cff5ecbe78d280 Mon Sep 17 00:00:00 2001
From: Zhenyu Wang
Date: Thu, 19 Aug 2021 16:36:33 +0800
Subject: [PATCH] TESTONLY:drm/i915/gvt: potential fix for refactor against
current tip
---
drivers/gpu/drm/i915/Makefile | 4 +++-
drivers/gpu/drm/i915/gt/intel_context.c | 5 +
drivers/gpu/drm/i915/gt/intel_context.h | 3 ++-
drivers/gpu/drm/i915/i915_trace.h | 1 +
4 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index c4f953837f72..2248574428a1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -296,7 +296,9 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \
# virtual gpu code
i915-y += i915_vgpu.o
-i915-$(CONFIG_DRM_I915_GVT_KVMGT) += intel_gvt.o
+ifneq ($(CONFIG_DRM_I915_GVT_KVMGT),)
+i915-y += intel_gvt.o
+endif
kvmgt-y += gvt/kvmgt.o \
gvt/gvt.o \
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c
b/drivers/gpu/drm/i915/gt/intel_context.c
index 745e84c72c90..20e7522fed84 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -328,6 +328,11 @@ void __intel_context_do_unpin(struct intel_context *ce,
int sub)
intel_context_put(ce);
}
+void intel_context_unpin(struct intel_context *ce)
+{
+ _intel_context_unpin(ce);
+}
+
static void __intel_context_retire(struct i915_active *active)
{
struct intel_context *ce = container_of(active, typeof(*ce), active);
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h
b/drivers/gpu/drm/i915/gt/intel_context.h
index c41098950746..f942cbf6300a 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -131,7 +131,7 @@ static inline void intel_context_sched_disable_unpin(struct
intel_context *ce)
__intel_context_do_unpin(ce, 2);
}
-static inline void intel_context_unpin(struct intel_context *ce)
+static inline void _intel_context_unpin(struct intel_context *ce)
{
if (!ce->ops->sched_disable) {
__intel_context_do_unpin(ce, 1);
@@ -150,6 +150,7 @@ static inline void intel_context_unpin(struct intel_context
*ce)
}
}
}
+void intel_context_unpin(struct intel_context *ce);
void intel_context_enter_engine(struct intel_context *ce);
void intel_context_exit_engine(struct intel_context *ce);
diff --git a/drivers/gpu/drm/i915/i915_trace.h
b/drivers/gpu/drm/i915/i915_trace.h
index 806ad688274b..2c6a8bcef7c1 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -17,6 +17,7 @@
#undef TRACE_SYSTEM
#define TRACE_SYSTEM i915
+#undef TRACE_INCLUDE_FILE
#define TRACE_INCLUDE_FILE i915_trace
/* watermark/fifo updates */
--
2.32.0
---8<---
signature.asc
Description: PGP signature
Re: [Intel-gfx] refactor the i915 GVT support
On 2021.08.17 09:08:55 +0800, Zhenyu Wang wrote: > On 2021.08.16 19:34:58 +0200, Christoph Hellwig wrote: > > On Wed, Aug 04, 2021 at 01:26:06PM +0800, Zhenyu Wang wrote: > > > On 2021.08.03 11:30:58 -0300, Jason Gunthorpe wrote: > > > > On Tue, Aug 03, 2021 at 05:43:15PM +0800, Zhenyu Wang wrote: > > > > > Acked-by: Zhenyu Wang > > > > > > > > > > Thanks a lot for this effort! > > > > > > > > Great, do we have a submission plan for this? how much does it clash > > > > with my open_device/etc patch? ie does the whole thing have to go > > > > through the vfio tree? > > > > > > > > > > I think Alex would determine when to merge open_device series, gvt part > > > can be through vfio tree without problem. For this refactor, I would first > > > merge for gvt staging to do more regression testing before sending through > > > i915 tree. > > > > Any updates on this? I'd really hate to miss this merge window. > > I'm still waiting for our validation team's report on this. I'm afraid > it might be missing for next version as i915 merge window is mostly > till rc5...and for any change outside of gvt, it still needs to be > acked by i915 maintainers. Looks our validation team did have problem against recent i915 change. If you like to try, we have a gvt-staging branch on https://github.com/intel/gvt-linux which is generated against drm-tip with gvt changes for testing, currently it's broken. One issue is with i915 export that intel_context_unpin has been changed into static inline function. Another is that intel_gvt.c should be part of i915 for gvt interface instead of depending on KVMGT config. But the problem I see is that after moving gvt device model (gvt/*.c except kvmgt.c) into kvmgt module, we'll have issue with initial mmio state which current gvt relies on, that is in design supposed to get initial HW state before i915 driver has taken any operation. Before we can ensure that, I think we may only remove MPT part first but still keep gvt device model as part of i915 with config. I'll try to split that out. signature.asc Description: PGP signature
Re: [Intel-gfx] refactor the i915 GVT support
On 2021.08.16 19:34:58 +0200, Christoph Hellwig wrote: > On Wed, Aug 04, 2021 at 01:26:06PM +0800, Zhenyu Wang wrote: > > On 2021.08.03 11:30:58 -0300, Jason Gunthorpe wrote: > > > On Tue, Aug 03, 2021 at 05:43:15PM +0800, Zhenyu Wang wrote: > > > > Acked-by: Zhenyu Wang > > > > > > > > Thanks a lot for this effort! > > > > > > Great, do we have a submission plan for this? how much does it clash > > > with my open_device/etc patch? ie does the whole thing have to go > > > through the vfio tree? > > > > > > > I think Alex would determine when to merge open_device series, gvt part > > can be through vfio tree without problem. For this refactor, I would first > > merge for gvt staging to do more regression testing before sending through > > i915 tree. > > Any updates on this? I'd really hate to miss this merge window. I'm still waiting for our validation team's report on this. I'm afraid it might be missing for next version as i915 merge window is mostly till rc5...and for any change outside of gvt, it still needs to be acked by i915 maintainers. signature.asc Description: PGP signature
Re: [Intel-gfx] i915 timeouts delaying boot under GVT
On 2021.08.13 08:31:28 +0200, Christoph Hellwig wrote:
> Hi all,
>
> when botting a current 4.14-rc tree in a VM using GVT-g (with the host
> also running a current 4.14-rc tree), I see bunch of long timeouts
> followed by i915 errors:
>
> [4.252066] i915 :00:03.0: [drm] VGT balloon successfully
> [5.095190] i915 :00:03.0: [drm] *ERROR* Failed to disable SAGV (-110)
> [ 15.334559] [drm:drm_atomic_helper_wait_for_flip_done] *ERROR*
> [CRTC:51:pipe
> A] flip_done timed out
> [ 15.346934] [drm] Initialized i915 1.6.0 20201103 for :00:03.0 on minor
> 0
>
> I did a hackjob to track them down and just if out the offending code,
> which speeds up the boot by ~11 seconds but is probably dangerous as hell:
Hi, Christoph, what platform is this? And what's your guest i915 config?
I'll try to reproduce on our side. Thanks for reporting this.
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 2d5d21740c25..ee82fd67f386 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -10696,7 +10696,7 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
>* - switch over to the vblank wait helper in the core after that since
>* we don't need out special handling any more.
>*/
> - drm_atomic_helper_wait_for_flip_done(dev, >base);
> +// drm_atomic_helper_wait_for_flip_done(dev, >base);
>
> for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> if (new_crtc_state->uapi.async_flip)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 45fefa0ed160..f03ce729cc4b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3753,7 +3753,7 @@ intel_disable_sagv(struct drm_i915_private *dev_priv)
> if (!intel_has_sagv(dev_priv))
> return 0;
>
> - if (dev_priv->sagv_status == I915_SAGV_DISABLED)
> + if (1 || dev_priv->sagv_status == I915_SAGV_DISABLED)
> return 0;
>
> drm_dbg_kms(_priv->drm, "Disabling SAGV\n");
signature.asc
Description: PGP signature
[Intel-gfx] [PULL] gvt-fixes
Hi, Here's one regression fix for windows VM hang issue on recent drivers. Thanks -- The following changes since commit c90b4503ccf42d9d367e843c223df44aa550e82a: drm/i915/gvt: Clear d3_entered on elsp cmd submission. (2021-07-08 16:42:34 +0800) are available in the Git repository at: https://github.com/intel/gvt-linux tags/gvt-fixes-2021-08-10 for you to fetch changes up to 699aa57b35672c3b2f230e2b7e5d0ab8c2bde80a: drm/i915/gvt: Fix cached atomics setting for Windows VM (2021-08-09 14:42:09 +0800) gvt-fixes-2021-08-10 - Fix windows VM hang issue for atomics workaround (Zhenyu) Zhenyu Wang (1): drm/i915/gvt: Fix cached atomics setting for Windows VM drivers/gpu/drm/i915/gvt/handlers.c | 1 + drivers/gpu/drm/i915/gvt/mmio_context.c | 2 ++ 2 files changed, 3 insertions(+) signature.asc Description: PGP signature
[Intel-gfx] [PATCH v2] drm/i915/gvt: Fix cached atomics setting for Windows VM
We've seen recent regression with host and windows VM running
simultaneously that cause gpu hang or even crash. Finally bisect to
commit 58586680ffad ("drm/i915: Disable atomics in L3 for gen9"),
which seems cached atomics behavior difference caused regression
issue.
This tries to add new scratch register handler and add those in mmio
save/restore list for context switch. No gpu hang produced with this one.
Cc: sta...@vger.kernel.org # 5.12+
Cc: "Xu, Terrence"
Cc: "Bloomfield, Jon"
Cc: "Ekstrand, Jason"
Fixes: 58586680ffad ("drm/i915: Disable atomics in L3 for gen9")
Signed-off-by: Zhenyu Wang
---
drivers/gpu/drm/i915/gvt/handlers.c | 1 +
drivers/gpu/drm/i915/gvt/mmio_context.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
b/drivers/gpu/drm/i915/gvt/handlers.c
index 06024d321a1a..cde0a477fb49 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -3149,6 +3149,7 @@ static int init_bdw_mmio_info(struct intel_gvt *gvt)
MMIO_DFH(_MMIO(0xb100), D_BDW, F_CMD_ACCESS, NULL, NULL);
MMIO_DFH(_MMIO(0xb10c), D_BDW, F_CMD_ACCESS, NULL, NULL);
MMIO_D(_MMIO(0xb110), D_BDW);
+ MMIO_D(GEN9_SCRATCH_LNCF1, D_BDW_PLUS);
MMIO_F(_MMIO(0x24d0), 48, F_CMD_ACCESS | F_CMD_WRITE_PATCH, 0, 0,
D_BDW_PLUS, NULL, force_nonpriv_write);
diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c
b/drivers/gpu/drm/i915/gvt/mmio_context.c
index b8ac80765461..f776c470914d 100644
--- a/drivers/gpu/drm/i915/gvt/mmio_context.c
+++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
@@ -105,6 +105,8 @@ static struct engine_mmio gen9_engine_mmio_list[]
__cacheline_aligned = {
{RCS0, COMMON_SLICE_CHICKEN2, 0x, true}, /* 0x7014 */
{RCS0, GEN9_CS_DEBUG_MODE1, 0x, false}, /* 0x20ec */
{RCS0, GEN8_L3SQCREG4, 0, false}, /* 0xb118 */
+ {RCS0, GEN9_SCRATCH1, 0, false}, /* 0xb11c */
+ {RCS0, GEN9_SCRATCH_LNCF1, 0, false}, /* 0xb008 */
{RCS0, GEN7_HALF_SLICE_CHICKEN1, 0x, true}, /* 0xe100 */
{RCS0, HALF_SLICE_CHICKEN2, 0x, true}, /* 0xe180 */
{RCS0, HALF_SLICE_CHICKEN3, 0x, true}, /* 0xe184 */
--
2.32.0.rc2
Re: [Intel-gfx] refactor the i915 GVT support
On 2021.08.03 11:30:58 -0300, Jason Gunthorpe wrote: > On Tue, Aug 03, 2021 at 05:43:15PM +0800, Zhenyu Wang wrote: > > Acked-by: Zhenyu Wang > > > > Thanks a lot for this effort! > > Great, do we have a submission plan for this? how much does it clash > with my open_device/etc patch? ie does the whole thing have to go > through the vfio tree? > I think Alex would determine when to merge open_device series, gvt part can be through vfio tree without problem. For this refactor, I would first merge for gvt staging to do more regression testing before sending through i915 tree. Thanks signature.asc Description: PGP signature
Re: [Intel-gfx] refactor the i915 GVT support
On 2021.07.29 09:20:22 +0200, Christoph Hellwig wrote: > On Wed, Jul 28, 2021 at 02:59:25PM -0300, Jason Gunthorpe wrote: > > On Wed, Jul 28, 2021 at 01:38:58PM +, Wang, Zhi A wrote: > > > > > I guess those APIs you were talking about are KVM-only. For other > > > hypervisors, e.g. Xen, ARCN cannot use the APIs you mentioned. Not > > > sure if you have already noticed that VFIO is KVM-only right now. > > > > There is very little hard connection between VFIO and KVM, so no, I > > don't think that is completely true. > > The only connection is the SET_KVM notifier as far as I can tell. > Which is used by a total of two drivers, including i915/gvt. That > being said gvt does not only use vfio, but also does quite a few > direct cals to KVM. yeah, we mostly combined VFIO into hypervisor specific thing before, e.g interface to set vgpu edid, etc. along with kvm specific calls. > > > In an event, an in-tree version of other hypervisor support for GVT > > needs to go through enabling VFIO support so that the existing API > > multiplexers we have can be used properly, not adding a shim layer > > trying to recreate VFIO inside a GPU driver. > > Yes. And if we go back to actually looking at the series a lot of > it just removes entirely pointless indirect calls that go to generic > code and not even the kvm code, or questionable data structure designs. > If we were to support another upstream hypervisor we'd just need to > union a few fields in struct intel_gpu and maybe introduce a few > methods. Preferably in a way that avoids expensive indirect calls > in the fast path. ok, agree on that. Acked-by: Zhenyu Wang Thanks a lot for this effort! signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH 01/21] drm/i915/gvt: integrate into the main Makefile
On 2021.07.21 17:53:35 +0200, Christoph Hellwig wrote: > Remove the separately included Makefile and just use the relative > reference from the main i915 Makefile as for source files in other > subdirectories. > We agreed to make gvt mostly self-contained in its own directory before. Although I don't think we would change files much later, but still need to get ack from i915 maintainers. Thanks > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/i915/Makefile | 29 - > drivers/gpu/drm/i915/gvt/Makefile | 9 - > drivers/gpu/drm/i915/gvt/trace.h | 2 +- > 3 files changed, 25 insertions(+), 15 deletions(-) > delete mode 100644 drivers/gpu/drm/i915/gvt/Makefile > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 4f22cac1c49b..2153f67705b8 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -289,11 +289,30 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \ > > # virtual gpu code > i915-y += i915_vgpu.o > - > -ifeq ($(CONFIG_DRM_I915_GVT),y) > -i915-y += intel_gvt.o > -include $(src)/gvt/Makefile > -endif > +i915-$(CONFIG_DRM_I915_GVT) += \ > + intel_gvt.o \ > + gvt/gvt.o \ > + gvt/aperture_gm.o \ > + gvt/handlers.o \ > + gvt/vgpu.o \ > + gvt/trace_points.o \ > + gvt/firmware.o \ > + gvt/interrupt.o \ > + gvt/gtt.o \ > + gvt/cfg_space.o \ > + gvt/opregion.o \ > + gvt/mmio.o \ > + gvt/display.o \ > + gvt/edid.o \ > + gvt/execlist.o \ > + gvt/scheduler.o \ > + gvt/sched_policy.o \ > + gvt/mmio_context.o \ > + gvt/cmd_parser.o \ > + gvt/debugfs.o \ > + gvt/fb_decoder.o \ > + gvt/dmabuf.o \ > + gvt/page_track.o > > obj-$(CONFIG_DRM_I915) += i915.o > obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o > diff --git a/drivers/gpu/drm/i915/gvt/Makefile > b/drivers/gpu/drm/i915/gvt/Makefile > deleted file mode 100644 > index ea8324abc784.. > --- a/drivers/gpu/drm/i915/gvt/Makefile > +++ /dev/null > @@ -1,9 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0 > -GVT_DIR := gvt > -GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o > firmware.o \ > - interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \ > - execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o > debugfs.o \ > - fb_decoder.o dmabuf.o page_track.o > - > -ccflags-y+= -I $(srctree)/$(src) -I > $(srctree)/$(src)/$(GVT_DIR)/ > -i915-y += $(addprefix $(GVT_DIR)/, > $(GVT_SOURCE)) > diff --git a/drivers/gpu/drm/i915/gvt/trace.h > b/drivers/gpu/drm/i915/gvt/trace.h > index 6d787750d279..348f57f8301d 100644 > --- a/drivers/gpu/drm/i915/gvt/trace.h > +++ b/drivers/gpu/drm/i915/gvt/trace.h > @@ -379,5 +379,5 @@ TRACE_EVENT(render_mmio, > #undef TRACE_INCLUDE_PATH > #define TRACE_INCLUDE_PATH . > #undef TRACE_INCLUDE_FILE > -#define TRACE_INCLUDE_FILE trace > +#define TRACE_INCLUDE_FILE gvt/trace > #include > -- > 2.30.2 > > ___ > intel-gvt-dev mailing list > intel-gvt-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev signature.asc Description: PGP signature
Re: [Intel-gfx] refactor the i915 GVT support
On 2021.07.21 17:53:34 +0200, Christoph Hellwig wrote: > 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. > The reason we isolated hypervisor specific code from core vgpu emulation is to make multiple hypervisor support possible. Yes, we do have Xen support but never got way into upstream...And we also have third party hypervisors which leverage gvt function through current hypervisor interface. Sorry I may not have more time to check in details for now, but some of them look fine to me. I'll review more after vacation or let Zhi check details. Thanks! > 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-gvt > > Diffstat: > b/drivers/gpu/drm/i915/Kconfig | 31 > b/drivers/gpu/drm/i915/Makefile| 30 > b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c |4 > 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.c | 100 -- > b/drivers/gpu/drm/i915/gvt/gvt.h | 132 ++- > b/drivers/gpu/drm/i915/gvt/interrupt.c | 38 - > b/drivers/gpu/drm/i915/gvt/kvmgt.c | 634 > - > 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.h |7 > b/drivers/gpu/drm/i915/i915_params.c |2 > b/drivers/gpu/drm/i915/intel_gvt.c | 64 + > b/drivers/gpu/drm/i915/intel_gvt.h |4 > drivers/gpu/drm/i915/gvt/Makefile |9 > drivers/gpu/drm/i915/gvt/hypercall.h | 82 -- > drivers/gpu/drm/i915/gvt/mpt.h | 400 -- > 25 files changed, 541 insertions(+), 1413 deletions(-) signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] 5.14-rc2 warnings with kvmgvt
On 2021.07.21 13:10:49 +0200, Christoph Hellwig wrote: > Hi all, > > I'm trying to test some changes for the gvt code, but even with a baseline > 5.14-rc2 host and guest the 915 driver does not seem overly happy: > I think we also got bug report on those display related warnings, should be some issue with our virtual display model that doesn't work nicely with more i915 display pipe/port check or exercises have been added...But I believe you should still get virtual framebuffer up and show, right? > [5.693099] i915 :00:04.0: [drm] Virtual GPU for Intel GVT-g detected. > [5.694841] i915 :00:04.0: [drm] VT-d active for gfx access > [5.696411] i915 :00:04.0: [drm] iGVT-g active, disabling use of > stolen memory > [5.711317] i915 :00:04.0: BAR 6: can't assign [??? 0x flags > 0x2000] (bogus alignm) > [5.712847] i915 :00:04.0: [drm] Failed to find VBIOS tables (VBT) > [5.714343] i915 :00:04.0: vgaarb: changed VGA decodes: > olddecodes=io+mem,decodes=none:owns=iom > [5.716466] i915 :00:04.0: Direct firmware load for > i915/kbl_dmc_ver1_04.bin failed with error2 > [5.718021] i915 :00:04.0: [drm] Failed to load DMC firmware > i915/kbl_dmc_ver1_04.bin. Disabli. > [5.719914] i915 :00:04.0: [drm] DMC firmware homepage: > https://git.kernel.org/pub/scm/linux/k5 > [5.733269] i915 :00:04.0: [drm] failed to retrieve link info, > disabling eDP > [5.735841] i915 :00:04.0: [drm] *ERROR* crtc 51: Can't calculate > constants, dotclock = 0! > [5.737354] [ cut here ] > [5.738141] i915 :00:04.0: > drm_WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev)) > [5.738165] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_vblank.c:728 > drm_crtc_vblank_helper_get_0 > [5.738745] Modules linked in: > [5.738745] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc2+ #22 > [5.738745] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > 1.14.0-2 04/01/2014 > [5.738745] RIP: > 0010:drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x335/0x350 > [5.738745] Code: 4c 8b 6f 50 4d 85 ed 75 03 4c 8b 2f e8 34 10 26 00 48 c7 > c1 20 54 0d 83 4c 89 ea0 > [5.738745] RSP: :c9013a90 EFLAGS: 00010086 > [5.738745] RAX: RBX: 81c3c5b0 RCX: > > [5.738745] RDX: 0003 RSI: fffe RDI: > > [5.738745] RBP: c9013b00 R08: 83bb3e28 R09: > 0003 > [5.738745] R10: 834b3e40 R11: 3fff R12: > > [5.738745] R13: 888100e982f0 R14: 8881053f0340 R15: > 888105592178 > [5.738745] FS: () GS:88813bc0() > knlGS: > [5.738745] CS: 0010 DS: ES: CR0: 80050033 > [5.738745] CR2: CR3: 03462000 CR4: > 06f0 > [5.738745] Call Trace: > [5.738745] drm_get_last_vbltimestamp+0xa5/0xb0 > [5.738745] drm_reset_vblank_timestamp+0x56/0xc0 > [5.738745] drm_crtc_vblank_on+0x81/0x140 > [5.738745] intel_crtc_vblank_on+0x2b/0xe0 > [5.738745] intel_modeset_setup_hw_state+0xa9c/0x1ab0 > [5.738745] ? ww_mutex_lock+0x2b/0x90 > [5.738745] intel_modeset_init_nogem+0x3c5/0x1310 > [5.738745] ? intel_irq_postinstall+0x1aa/0x520 > [5.738745] i915_driver_probe+0x695/0xd30 > [5.738745] ? _raw_spin_unlock_irqrestore+0x33/0x50 > [5.738745] pci_device_probe+0xcd/0x140 > [5.738745] really_probe.part.0+0x99/0x270 > [5.738745] __driver_probe_device+0x8b/0x120 > [5.738745] driver_probe_device+0x19/0x90 > [5.738745] __driver_attach+0x79/0x120 > [5.738745] ? __device_attach_driver+0x90/0x90 > [5.738745] bus_for_each_dev+0x78/0xc0 > [5.738745] bus_add_driver+0x109/0x1b0 > [5.738745] driver_register+0x86/0xd0 > [5.738745] ? ttm_init+0x18/0x18 > [5.738745] i915_init+0x58/0x72 > [5.738745] do_one_initcall+0x56/0x2e0 > [5.738745] ? rcu_read_lock_sched_held+0x3a/0x70 > [5.738745] kernel_init_freeable+0x186/0x1ce > [5.738745] ? rest_init+0x250/0x250 > [5.738745] kernel_init+0x11/0x110 > [5.738745] ret_from_fork+0x22/0x30 > [5.738745] irq event stamp: 8200428 > [5.738745] hardirqs last enabled at (8200427): [] > _raw_spin_unlock_irqrestore+0 > [5.738745] hardirqs last disabled at (8200428): [] > _raw_spin_lock_irq+0x41/0x50 > [5.738745] softirqs last enabled at (8199086): [] > irq_exit_rcu+0x108/0x140 > [5.738745] softirqs last disabled at (8199079): [] > irq_exit_rcu+0x108/0x140 > [5.738745] ---[ end trace e99e0812b8ee9c5d ]--- > [5.786472] i915 :00:04.0: [drm] VGT ballooning configuration: > [5.787531] i915 :00:04.0: [drm] Mappable graphic memory: base > 0x31c7000 size 65536KiB > [5.788865] i915 :00:04.0: [drm] Unmappable graphic memory: base >
[Intel-gfx] [PATCH] drm/i915/gvt: Fix cached atomics setting for Windows VM
We've seen recent regression with host and windows VM running
simultaneously that cause gpu hang or even crash. Finally bisect to
58586680ffad ("drm/i915: Disable atomics in L3 for gen9"), which seems
cached atomics behavior difference caused regression issue.
This trys to add new scratch register handler and add those in mmio
save/restore list for context switch. No gpu hang produced with this one.
Cc: sta...@vger.kernel.org # 5.12+
Cc: "Xu, Terrence"
Fixes: 58586680ffad ("drm/i915: Disable atomics in L3 for gen9")
Signed-off-by: Zhenyu Wang
---
drivers/gpu/drm/i915/gvt/handlers.c | 1 +
drivers/gpu/drm/i915/gvt/mmio_context.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
b/drivers/gpu/drm/i915/gvt/handlers.c
index 98eb48c24c46..345b4be5ebad 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -3134,6 +3134,7 @@ static int init_bdw_mmio_info(struct intel_gvt *gvt)
MMIO_DFH(_MMIO(0xb100), D_BDW, F_CMD_ACCESS, NULL, NULL);
MMIO_DFH(_MMIO(0xb10c), D_BDW, F_CMD_ACCESS, NULL, NULL);
MMIO_D(_MMIO(0xb110), D_BDW);
+ MMIO_D(GEN9_SCRATCH_LNCF1, D_BDW_PLUS);
MMIO_F(_MMIO(0x24d0), 48, F_CMD_ACCESS | F_CMD_WRITE_PATCH, 0, 0,
D_BDW_PLUS, NULL, force_nonpriv_write);
diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c
b/drivers/gpu/drm/i915/gvt/mmio_context.c
index b8ac80765461..f776c470914d 100644
--- a/drivers/gpu/drm/i915/gvt/mmio_context.c
+++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
@@ -105,6 +105,8 @@ static struct engine_mmio gen9_engine_mmio_list[]
__cacheline_aligned = {
{RCS0, COMMON_SLICE_CHICKEN2, 0x, true}, /* 0x7014 */
{RCS0, GEN9_CS_DEBUG_MODE1, 0x, false}, /* 0x20ec */
{RCS0, GEN8_L3SQCREG4, 0, false}, /* 0xb118 */
+ {RCS0, GEN9_SCRATCH1, 0, false}, /* 0xb11c */
+ {RCS0, GEN9_SCRATCH_LNCF1, 0, false}, /* 0xb008 */
{RCS0, GEN7_HALF_SLICE_CHICKEN1, 0x, true}, /* 0xe100 */
{RCS0, HALF_SLICE_CHICKEN2, 0x, true}, /* 0xe180 */
{RCS0, HALF_SLICE_CHICKEN3, 0x, true}, /* 0xe184 */
--
2.32.0.rc2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Convert from atomic_t to refcount_t on intel_vgpu_ppgtt_spt->refcount
On 2021.07.16 18:41:38 +0800, Xiyu Yang wrote:
> refcount_t type and corresponding API can protect refcounters from
> accidental underflow and overflow and further use-after-free situations
>
Thanks for the patch. Is there any specific problem you run with current code?
Any shadow ppgtt error?
> Signed-off-by: Xiyu Yang
> Signed-off-by: Xin Tan
> ---
> drivers/gpu/drm/i915/gvt/gtt.c | 11 ++-
> drivers/gpu/drm/i915/gvt/gtt.h | 3 ++-
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index cc2c05e18206..62f3daff5a36 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -841,7 +841,7 @@ static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt(
> }
>
> spt->vgpu = vgpu;
> - atomic_set(>refcount, 1);
> + refcount_set(>refcount, 1);
> INIT_LIST_HEAD(>post_shadow_list);
>
> /*
> @@ -927,18 +927,19 @@ static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt_gfn(
>
> static inline void ppgtt_get_spt(struct intel_vgpu_ppgtt_spt *spt)
> {
> - int v = atomic_read(>refcount);
> + int v = refcount_read(>refcount);
>
> trace_spt_refcount(spt->vgpu->id, "inc", spt, v, (v + 1));
> - atomic_inc(>refcount);
> + refcount_inc(>refcount);
> }
>
> static inline int ppgtt_put_spt(struct intel_vgpu_ppgtt_spt *spt)
> {
> - int v = atomic_read(>refcount);
> + int v = refcount_read(>refcount);
>
> trace_spt_refcount(spt->vgpu->id, "dec", spt, v, (v - 1));
> - return atomic_dec_return(>refcount);
> + refcount_dec(>refcount);
> + return refcount_read(>refcount);
> }
>
> static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt);
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
> index 3bf45672ef98..944c2d0739df 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.h
> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
> @@ -38,6 +38,7 @@
> #include
> #include
> #include
> +#include
>
> #include "gt/intel_gtt.h"
>
> @@ -243,7 +244,7 @@ struct intel_vgpu_oos_page {
>
> /* Represent a vgpu shadow page table. */
> struct intel_vgpu_ppgtt_spt {
> - atomic_t refcount;
> + refcount_t refcount;
> struct intel_vgpu *vgpu;
>
> struct {
> --
> 2.7.4
>
> ___
> intel-gvt-dev mailing list
> intel-gvt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/13] vfio/gvt: Fix open/close when multiple device FDs are open
On 2021.07.14 21:20:41 -0300, Jason Gunthorpe wrote:
> The user can open multiple device FDs if it likes, however the open
> function calls vfio_register_notifier() on device global state. Calling
> vfio_register_notifier() twice will trigger a WARN_ON from
> notifier_chain_register() and the first close will wrongly delete the
> notifier and more.
>
> Since these really want the new open/close_device() semantics just change
> the function over.
>
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/gpu/drm/i915/gvt/kvmgt.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 1ac98f8aba31e6..7efa386449d104 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -885,7 +885,7 @@ static int intel_vgpu_group_notifier(struct
> notifier_block *nb,
> return NOTIFY_OK;
> }
>
> -static int intel_vgpu_open(struct mdev_device *mdev)
> +static int intel_vgpu_open_device(struct mdev_device *mdev)
> {
> struct intel_vgpu *vgpu = mdev_get_drvdata(mdev);
> struct kvmgt_vdev *vdev = kvmgt_vdev(vgpu);
> @@ -1004,7 +1004,7 @@ static void __intel_vgpu_release(struct intel_vgpu
> *vgpu)
> vgpu->handle = 0;
> }
>
> -static void intel_vgpu_release(struct mdev_device *mdev)
> +static void intel_vgpu_close_device(struct mdev_device *mdev)
> {
> struct intel_vgpu *vgpu = mdev_get_drvdata(mdev);
>
> @@ -1753,8 +1753,8 @@ static struct mdev_parent_ops intel_vgpu_ops = {
> .create = intel_vgpu_create,
> .remove = intel_vgpu_remove,
>
> - .open = intel_vgpu_open,
> - .release= intel_vgpu_release,
> + .open_device= intel_vgpu_open_device,
> + .close_device = intel_vgpu_close_device,
>
> .read = intel_vgpu_read,
> .write = intel_vgpu_write,
Looks ok to me. Thanks!
Reviewed-by: Zhenyu Wang
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] gvt-fixes
Hi, Here's one fix of shadow ppgtt invalidation with proper vGPU D3 state tracking. Thanks -- The following changes since commit 62fb9874f5da54fdb243003b386128037319b219: Linux 5.13 (2021-06-27 15:21:11 -0700) are available in the Git repository at: https://github.com/intel/gvt-linux tags/gvt-fixes-2021-07-15 for you to fetch changes up to c90b4503ccf42d9d367e843c223df44aa550e82a: drm/i915/gvt: Clear d3_entered on elsp cmd submission. (2021-07-08 16:42:34 +0800) gvt-fixes-2021-07-15 - Fix shadow ppgtt invalidation with proper D3 state tracking (Colin) Colin Xu (1): drm/i915/gvt: Clear d3_entered on elsp cmd submission. drivers/gpu/drm/i915/gvt/handlers.c | 15 +++ 1 file changed, 15 insertions(+) signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/gem: Use list_entry to access list members
On 2021.05.23 10:23:04 -0700, Guenter Roeck wrote:
> Use list_entry() instead of container_of() to access list members.
> Also drop unnecessary and misleading NULL checks on the result of
> list_entry().
>
> Signed-off-by: Guenter Roeck
> ---
> v2: Checkpatch fixes:
> - Fix alignment
> - Replace comparison against NULL with !
>
> drivers/gpu/drm/i915/gvt/dmabuf.c | 18 +-
> 1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c
> b/drivers/gpu/drm/i915/gvt/dmabuf.c
> index d4f883f35b95..e3f488681484 100644
> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> @@ -148,8 +148,7 @@ static void dmabuf_gem_object_free(struct kref *kref)
>
> if (vgpu && vgpu->active && !list_empty(>dmabuf_obj_list_head)) {
> list_for_each(pos, >dmabuf_obj_list_head) {
> - dmabuf_obj = container_of(pos,
> - struct intel_vgpu_dmabuf_obj, list);
> + dmabuf_obj = list_entry(pos, struct
> intel_vgpu_dmabuf_obj, list);
> if (dmabuf_obj == obj) {
> list_del(pos);
> intel_gvt_hypervisor_put_vfio_device(vgpu);
> @@ -357,10 +356,8 @@ pick_dmabuf_by_info(struct intel_vgpu *vgpu,
> struct intel_vgpu_dmabuf_obj *ret = NULL;
>
> list_for_each(pos, >dmabuf_obj_list_head) {
> - dmabuf_obj = container_of(pos, struct intel_vgpu_dmabuf_obj,
> - list);
> - if ((dmabuf_obj == NULL) ||
> - (dmabuf_obj->info == NULL))
> + dmabuf_obj = list_entry(pos, struct intel_vgpu_dmabuf_obj,
> list);
> + if (!dmabuf_obj->info)
> continue;
>
> fb_info = (struct intel_vgpu_fb_info *)dmabuf_obj->info;
> @@ -387,11 +384,7 @@ pick_dmabuf_by_num(struct intel_vgpu *vgpu, u32 id)
> struct intel_vgpu_dmabuf_obj *ret = NULL;
>
> list_for_each(pos, >dmabuf_obj_list_head) {
> - dmabuf_obj = container_of(pos, struct intel_vgpu_dmabuf_obj,
> - list);
> - if (!dmabuf_obj)
> - continue;
> -
> + dmabuf_obj = list_entry(pos, struct intel_vgpu_dmabuf_obj,
> list);
> if (dmabuf_obj->dmabuf_id == id) {
> ret = dmabuf_obj;
> break;
> @@ -600,8 +593,7 @@ void intel_vgpu_dmabuf_cleanup(struct intel_vgpu *vgpu)
>
> mutex_lock(>dmabuf_lock);
> list_for_each_safe(pos, n, >dmabuf_obj_list_head) {
> - dmabuf_obj = container_of(pos, struct intel_vgpu_dmabuf_obj,
> - list);
> + dmabuf_obj = list_entry(pos, struct intel_vgpu_dmabuf_obj,
> list);
> dmabuf_obj->vgpu = NULL;
>
> idr_remove(>object_idr, dmabuf_obj->dmabuf_id);
> --
Sorry for late reply! Looks good to me.
Reviewed-by: Zhenyu Wang
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Add missing macro name changes
On 2021.05.21 11:12:29 -0700, Anusha Srivatsa wrote:
> Propagate changes to macros name containing CSR_*
> to DMC_* from display side.
>
> Cc: intel-gvt-...@lists.freedesktop.org
> Cc: Jani Nikula
> Signed-off-by: Anusha Srivatsa
> ---
> drivers/gpu/drm/i915/gvt/handlers.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index dda320749c65..33496397a74f 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -3342,9 +3342,9 @@ static int init_skl_mmio_info(struct intel_gvt *gvt)
> MMIO_D(_MMIO(_PLANE_SURF_3_A), D_SKL_PLUS);
> MMIO_D(_MMIO(_PLANE_SURF_3_B), D_SKL_PLUS);
>
> - MMIO_D(CSR_SSP_BASE, D_SKL_PLUS);
> - MMIO_D(CSR_HTP_SKL, D_SKL_PLUS);
> - MMIO_D(CSR_LAST_WRITE, D_SKL_PLUS);
> + MMIO_D(DMC_SSP_BASE, D_SKL_PLUS);
> + MMIO_D(DMC_HTP_SKL, D_SKL_PLUS);
> + MMIO_D(DMC_LAST_WRITE, D_SKL_PLUS);
>
> MMIO_DFH(BDW_SCRATCH1, D_SKL_PLUS, F_CMD_ACCESS, NULL, NULL);
>
> @@ -3655,7 +3655,7 @@ void intel_gvt_clean_mmio_info(struct intel_gvt *gvt)
> * otherwise, need to update cmd_reg_handler in cmd_parser.c
> */
> static struct gvt_mmio_block mmio_blocks[] = {
> - {D_SKL_PLUS, _MMIO(CSR_MMIO_START_RANGE), 0x3000, NULL, NULL},
> + {D_SKL_PLUS, _MMIO(DMC_MMIO_START_RANGE), 0x3000, NULL, NULL},
> {D_ALL, _MMIO(MCHBAR_MIRROR_BASE_SNB), 0x4, NULL, NULL},
> {D_ALL, _MMIO(VGT_PVINFO_PAGE), VGT_PVINFO_SIZE,
> pvinfo_mmio_read, pvinfo_mmio_write},
> --
Sorry that I missed your fix before sending mine...
Reviewed-by: Zhenyu Wang
Thanks!
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 gvt broke drm-tip; Fix ASAP
On 2021.05.22 21:19:38 +0200, Thomas Zimmermann wrote: > Hi, > > after creating drm-tip today as part of [1], building drm-tip is now broken > with the error message shown below. > > Some register constants appear to be missing from the GVT code. Please fix > ASAP. > Thanks, Thomas. Looks DMC rename missed gvt part. We need to ask CI to have at least build test with gvt. signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gvt: remove local storage of debugfs file
On 2021.05.19 10:31:18 +0200, Greg Kroah-Hartman wrote: > On Wed, May 19, 2021 at 04:03:13PM +0800, Zhenyu Wang wrote: > > On 2021.05.18 18:28:53 +0200, Greg Kroah-Hartman wrote: > > > On Tue, May 18, 2021 at 06:17:05PM +0200, Greg Kroah-Hartman wrote: > > > > There is no need to keep the dentry around for the debugfs kvmgt cache > > > > file, as we can just look it up when we want to remove it later on. > > > > Simplify the structure by removing the dentry and relying on debugfs > > > > to find the dentry to remove when we want to. > > > > > > > > By doing this change, we remove the last in-kernel user that was storing > > > > the result of debugfs_create_long(), so that api can be cleaned up. > > > > > > > > Cc: Zhenyu Wang > > > > Cc: Zhi Wang > > > > Cc: Jani Nikula > > > > Cc: Joonas Lahtinen > > > > Cc: Rodrigo Vivi > > > > Cc: David Airlie > > > > Cc: Daniel Vetter > > > > Cc: intel-gvt-...@lists.freedesktop.org > > > > Cc: intel-gfx@lists.freedesktop.org > > > > Cc: dri-de...@lists.freedesktop.org > > > > Signed-off-by: Greg Kroah-Hartman > > > > --- > > > > drivers/gpu/drm/i915/gvt/kvmgt.c | 11 +-- > > > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > > > Note, I can take this through my debugfs tree if wanted, that way I can > > > clean up the debugfs_create_long() api at the same time. Otherwise it's > > > fine, I can wait until next -rc1 for that to happen. > > > > > > > It's fine with me to go through debugfs tree. Just double check that recent > > kvmgt change would not cause conflict with this as well. > > How can I check that? I'll be glad to take this through my tree, we can > handle the merge issues later for 5.14-rc1 :) > Current kvmgt change in merge queue is just https://patchwork.freedesktop.org/patch/433536/?series=89995=2 It applies fine with debugfs change. signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gvt: remove local storage of debugfs file
On 2021.05.18 18:28:53 +0200, Greg Kroah-Hartman wrote: > On Tue, May 18, 2021 at 06:17:05PM +0200, Greg Kroah-Hartman wrote: > > There is no need to keep the dentry around for the debugfs kvmgt cache > > file, as we can just look it up when we want to remove it later on. > > Simplify the structure by removing the dentry and relying on debugfs > > to find the dentry to remove when we want to. > > > > By doing this change, we remove the last in-kernel user that was storing > > the result of debugfs_create_long(), so that api can be cleaned up. > > > > Cc: Zhenyu Wang > > Cc: Zhi Wang > > Cc: Jani Nikula > > Cc: Joonas Lahtinen > > Cc: Rodrigo Vivi > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: intel-gvt-...@lists.freedesktop.org > > Cc: intel-gfx@lists.freedesktop.org > > Cc: dri-de...@lists.freedesktop.org > > Signed-off-by: Greg Kroah-Hartman > > --- > > drivers/gpu/drm/i915/gvt/kvmgt.c | 11 +-- > > 1 file changed, 5 insertions(+), 6 deletions(-) > > Note, I can take this through my debugfs tree if wanted, that way I can > clean up the debugfs_create_long() api at the same time. Otherwise it's > fine, I can wait until next -rc1 for that to happen. > It's fine with me to go through debugfs tree. Just double check that recent kvmgt change would not cause conflict with this as well. Thanks signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] gvt-fixes
Hi, This is to fix GVT config workaround introduced during -rc1 via vfio/mdev change, which exposed dependency issue explicitly that made current GVT config nasty. So this is to fix dependency issue and get back original config sanity. Thanks -- The following changes since commit e4527420ed087f99c6aa2ac22c6d3458c7dc1a94: drm/i915: Use correct downstream caps for check Src-Ctl mode for PCON (2021-05-12 20:53:08 +0300) are available in the Git repository at: https://github.com/intel/gvt-linux tags/gvt-fixes-2021-05-19 for you to fetch changes up to 145e06b58f8625becc61792a0554726314297a85: drm/i915/gvt: Move mdev attribute groups into kvmgt module (2021-05-17 16:37:09 +0800) gvt-fixes-2021-05-19 - Fix workaround in -rc1 for GVT config (Zhenyu) Zhenyu Wang (1): drm/i915/gvt: Move mdev attribute groups into kvmgt module drivers/gpu/drm/i915/Kconfig | 1 - drivers/gpu/drm/i915/gvt/gvt.c | 124 +-- drivers/gpu/drm/i915/gvt/gvt.h | 3 - drivers/gpu/drm/i915/gvt/hypercall.h | 2 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 122 +++--- drivers/gpu/drm/i915/gvt/mpt.h | 4 +- 6 files changed, 118 insertions(+), 138 deletions(-) signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gvt: remove local storage of debugfs file
On 2021.05.18 18:17:05 +0200, Greg Kroah-Hartman wrote:
> There is no need to keep the dentry around for the debugfs kvmgt cache
> file, as we can just look it up when we want to remove it later on.
> Simplify the structure by removing the dentry and relying on debugfs
> to find the dentry to remove when we want to.
>
> By doing this change, we remove the last in-kernel user that was storing
> the result of debugfs_create_long(), so that api can be cleaned up.
>
> Cc: Zhenyu Wang
> Cc: Zhi Wang
> Cc: Jani Nikula
> Cc: Joonas Lahtinen
> Cc: Rodrigo Vivi
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: intel-gvt-...@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Greg Kroah-Hartman
> ---
> drivers/gpu/drm/i915/gvt/kvmgt.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 65ff43cfc0f7..433c2a448f2d 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -88,6 +88,7 @@ struct kvmgt_pgfn {
> struct hlist_node hnode;
> };
>
> +#define KVMGT_DEBUGFS_FILENAME "kvmgt_nr_cache_entries"
> struct kvmgt_guest_info {
> struct kvm *kvm;
> struct intel_vgpu *vgpu;
> @@ -95,7 +96,6 @@ struct kvmgt_guest_info {
> #define NR_BKT (1 << 18)
> struct hlist_head ptable[NR_BKT];
> #undef NR_BKT
> - struct dentry *debugfs_cache_entries;
> };
>
> struct gvt_dma {
> @@ -1843,16 +1843,15 @@ static int kvmgt_guest_init(struct mdev_device *mdev)
> info->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
> kvm_page_track_register_notifier(kvm, >track_node);
>
> - info->debugfs_cache_entries = debugfs_create_ulong(
> - "kvmgt_nr_cache_entries",
> - 0444, vgpu->debugfs,
> - >nr_cache_entries);
> + debugfs_create_ulong(KVMGT_DEBUGFS_FILENAME, 0444, vgpu->debugfs,
> + >nr_cache_entries);
> return 0;
> }
>
> static bool kvmgt_guest_exit(struct kvmgt_guest_info *info)
> {
> - debugfs_remove(info->debugfs_cache_entries);
> + debugfs_remove(debugfs_lookup(KVMGT_DEBUGFS_FILENAME,
> + info->vgpu->debugfs));
>
> kvm_page_track_unregister_notifier(info->kvm, >track_node);
> kvm_put_kvm(info->kvm);
> --
Reviewed-by: Zhenyu Wang
Thanks!
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/gvt: Move mdev attribute groups into kvmgt module
On 2021.05.13 09:02:49 -0300, Jason Gunthorpe wrote: > On Thu, May 13, 2021 at 12:56:47PM +0800, Zhenyu Wang wrote: > > On 2021.05.12 09:47:39 -0300, Jason Gunthorpe wrote: > > > On Wed, May 12, 2021 at 10:31:41AM +0800, Zhenyu Wang wrote: > > > > > > > > This need to go into the vfio tree in some way, either directly > > > > > through it, via rc or otherwise > > > > > > > > As this is only for i915/gvt, once drm/i915 backmerge with linus master, > > > > it should still go through normal i915/gvt merge path. > > > > > > Don't do this, you will create conflicts with ongoing vfio work. > > > > > > > Sure, there always could be conflict, which means you need to rebase onto > > this cleanup. Would that a problem? > > Yes. > > > I'd want to fix current workaround in 5.13-rc. > > This doesn't seem like a rc candiate to me, but going to -rc is also > fine. > > > Merging i915/gvt only change through vfio doesn't make sense to me, > > You need to do it to avoid major conflicts for stuff that will go into > the vfio tree this cycle. Looks apply to vfio/for-linus is fine, and vfio/next missed gvt change to apply...but not conflict with any new stuff. Alex, pls let me know if you have any concern of this. > > VFIO drivers should not be outside drivers/vfio/ in the first place, > and this shows why. > Well, I can't agree, otherwise that'll be dependency nightmare for device driver writer. ;) signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
As kvmgt module contains all handling for VFIO/mdev, leaving mdev attribute
groups in gvt module caused dependency issue. Although it was there for possible
other hypervisor usage, that turns out never to be true. So this moves all mdev
handling into kvmgt module completely to resolve dependency issue.
With this fix, no config workaround is required. So revert previous workaround
commits: adaeb718d46f ("vfio/gvt: fix DRM_I915_GVT dependency on VFIO_MDEV")
and 07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV").
Cc: Arnd Bergmann
Cc: Jason Gunthorpe
Cc: Alex Williamson
Signed-off-by: Zhenyu Wang
---
drivers/gpu/drm/i915/Kconfig | 1 -
drivers/gpu/drm/i915/gvt/gvt.c | 124 +--
drivers/gpu/drm/i915/gvt/gvt.h | 3 -
drivers/gpu/drm/i915/gvt/hypercall.h | 2 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 122 --
drivers/gpu/drm/i915/gvt/mpt.h | 4 +-
6 files changed, 118 insertions(+), 138 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 69f57ca9c68d..93f4d059fc89 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -102,7 +102,6 @@ config DRM_I915_GVT
bool "Enable Intel GVT-g graphics virtualization host support"
depends on DRM_I915
depends on 64BIT
- depends on VFIO_MDEV=y || VFIO_MDEV=DRM_I915
default n
help
Choose this option if you want to enable Intel GVT-g graphics
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index e7c2babcee8b..cbac409f6c8a 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -46,118 +46,6 @@ static const char * const supported_hypervisors[] = {
[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
};
-static struct intel_vgpu_type *
-intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id)
-{
- if (WARN_ON(type_group_id >= gvt->num_types))
- return NULL;
- return >types[type_group_id];
-}
-
-static ssize_t available_instances_show(struct mdev_type *mtype,
- struct mdev_type_attribute *attr,
- char *buf)
-{
- struct intel_vgpu_type *type;
- unsigned int num = 0;
- void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
-
- type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
- if (!type)
- num = 0;
- else
- num = type->avail_instance;
-
- return sprintf(buf, "%u\n", num);
-}
-
-static ssize_t device_api_show(struct mdev_type *mtype,
- struct mdev_type_attribute *attr, char *buf)
-{
- return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
-}
-
-static ssize_t description_show(struct mdev_type *mtype,
- struct mdev_type_attribute *attr, char *buf)
-{
- struct intel_vgpu_type *type;
- void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
-
- type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
- if (!type)
- return 0;
-
- return sprintf(buf, "low_gm_size: %dMB\nhigh_gm_size: %dMB\n"
- "fence: %d\nresolution: %s\n"
- "weight: %d\n",
- BYTES_TO_MB(type->low_gm_size),
- BYTES_TO_MB(type->high_gm_size),
- type->fence, vgpu_edid_str(type->resolution),
- type->weight);
-}
-
-static MDEV_TYPE_ATTR_RO(available_instances);
-static MDEV_TYPE_ATTR_RO(device_api);
-static MDEV_TYPE_ATTR_RO(description);
-
-static struct attribute *gvt_type_attrs[] = {
- _type_attr_available_instances.attr,
- _type_attr_device_api.attr,
- _type_attr_description.attr,
- NULL,
-};
-
-static struct attribute_group *gvt_vgpu_type_groups[] = {
- [0 ... NR_MAX_INTEL_VGPU_TYPES - 1] = NULL,
-};
-
-static bool intel_get_gvt_attrs(struct attribute_group
***intel_vgpu_type_groups)
-{
- *intel_vgpu_type_groups = gvt_vgpu_type_groups;
- return true;
-}
-
-static int intel_gvt_init_vgpu_type_groups(struct intel_gvt *gvt)
-{
- int i, j;
- struct intel_vgpu_type *type;
- struct attribute_group *group;
-
- for (i = 0; i < gvt->num_types; i++) {
- type = >types[i];
-
- group = kzalloc(sizeof(struct attribute_group), GFP_KERNEL);
- if (WARN_ON(!group))
- goto unwind;
-
- group->name = type->name;
- group->attrs = gvt_type_attrs;
- gvt_vgpu_type_groups[i] = group;
- }
-
- return 0;
-
-unwind:
- for (j = 0; j < i; j++) {
- group = gvt_vgpu_type_groups[
Re: [Intel-gfx] [PATCH 1/3] drm/i915/gvt: Move mdev attribute groups into kvmgt module
On 2021.05.12 09:47:39 -0300, Jason Gunthorpe wrote: > On Wed, May 12, 2021 at 10:31:41AM +0800, Zhenyu Wang wrote: > > > > This need to go into the vfio tree in some way, either directly > > > through it, via rc or otherwise > > > > As this is only for i915/gvt, once drm/i915 backmerge with linus master, > > it should still go through normal i915/gvt merge path. > > Don't do this, you will create conflicts with ongoing vfio work. > Sure, there always could be conflict, which means you need to rebase onto this cleanup. Would that a problem? I'd want to fix current workaround in 5.13-rc. Merging i915/gvt only change through vfio doesn't make sense to me, I will handle that instead of Alex. Thanks signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/gvt: Move mdev attribute groups into kvmgt module
On 2021.05.11 12:54:46 -0300, Jason Gunthorpe wrote:
> On Tue, May 11, 2021 at 04:33:30PM +0800, Zhenyu Wang wrote:
> > As kvmgt module contains all handling for VFIO/mdev, leaving mdev attribute
> > groups in gvt module caused dependency issue. Although it was there for
> > possible
> > other hypervisor usage, that turns out never to be true. So this moves all
> > mdev
> > handling into kvmgt module completely to resolve dependency issue.
> >
> > Cc: Arnd Bergmann
> > Cc: Jason Gunthorpe
> > Cc: Alex Williamson
> > Signed-off-by: Zhenyu Wang
> > ---
> > drivers/gpu/drm/i915/gvt/gvt.c | 124 +
> > drivers/gpu/drm/i915/gvt/gvt.h | 3 -
> > drivers/gpu/drm/i915/gvt/hypercall.h | 2 +-
> > drivers/gpu/drm/i915/gvt/kvmgt.c | 129 +--
> > drivers/gpu/drm/i915/gvt/mpt.h | 4 +-
> > 5 files changed, 126 insertions(+), 136 deletions(-)
>
> There is no reason to make this into three patches, just make the one
> line change to kconfig here.
>
> > +static struct intel_vgpu_type *
> > +intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id)
> > +{
> > + if (WARN_ON(type_group_id >= gvt->num_types))
> > + return NULL;
> > + return >types[type_group_id];
> > +}
>
> The WARN_ON can't happen, all this error handling is code should be
> deleted, just use the simple
>
> struct intel_gvt *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
> struct intel_vgpu_type *type = >types[mtype_get_type_group_id(mtype)]
>
> sequence like the other mdev drivers
>
> > +static ssize_t available_instances_show(struct mdev_type *mtype,
> > + struct mdev_type_attribute *attr,
> > + char *buf)
> > +{
> > + struct intel_vgpu_type *type;
> > + unsigned int num = 0;
> > + void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
>
> Use proper types not 'void *'
> > +
> > +static int intel_gvt_init_vgpu_type_groups(struct intel_gvt *gvt)
> > +{
> > + int i, j;
> > + struct intel_vgpu_type *type;
> > + struct attribute_group *group;
> > +
> > + for (i = 0; i < gvt->num_types; i++) {
> > + type = >types[i];
> > +
> > + group = kzalloc(sizeof(struct attribute_group), GFP_KERNEL);
> > + if (WARN_ON(!group))
> > + goto unwind;
>
> WARN_ON at allocation failure is not good
Thanks for the comments! I left those during code move, will clean them up.
>
> This need to go into the vfio tree in some way, either directly
> through it, via rc or otherwise
>
As this is only for i915/gvt, once drm/i915 backmerge with linus master,
it should still go through normal i915/gvt merge path.
Thanks
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] Fix I915_GVT dependency
This is to fix I915_GVT dependency mess to move all vfio/dev handling into kvmgt module, so gvt device model core won't need to depend on vfio/mdev. Currently apply against linus master with merge from vfio tree. This targets for 5.13 fix. thanks Zhenyu Wang (3): drm/i915/gvt: Move mdev attribute groups into kvmgt module Revert "vfio/gvt: fix DRM_I915_GVT dependency on VFIO_MDEV" Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV" drivers/gpu/drm/i915/Kconfig | 1 - drivers/gpu/drm/i915/gvt/gvt.c | 124 + drivers/gpu/drm/i915/gvt/gvt.h | 3 - drivers/gpu/drm/i915/gvt/hypercall.h | 2 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 129 +-- drivers/gpu/drm/i915/gvt/mpt.h | 4 +- 6 files changed, 126 insertions(+), 137 deletions(-) -- 2.31.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV"
This reverts commit 07e543f4f9d116d6b4240644191dee6388ef4a85. As I915_GVT dependency issue is resolved, revert this. Cc: Jason Gunthorpe Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 8f15bfb5faac..93f4d059fc89 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -102,7 +102,6 @@ config DRM_I915_GVT bool "Enable Intel GVT-g graphics virtualization host support" depends on DRM_I915 depends on 64BIT - depends on VFIO_MDEV default n help Choose this option if you want to enable Intel GVT-g graphics -- 2.31.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] Revert "vfio/gvt: fix DRM_I915_GVT dependency on VFIO_MDEV"
This reverts commit adaeb718d46f6b42a3fc1dffd4f946f26b33779a. As I915_GVT dependency is resolved, revert this temporary hack. Cc: Arnd Bergmann Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 69f57ca9c68d..8f15bfb5faac 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -102,7 +102,7 @@ config DRM_I915_GVT bool "Enable Intel GVT-g graphics virtualization host support" depends on DRM_I915 depends on 64BIT - depends on VFIO_MDEV=y || VFIO_MDEV=DRM_I915 + depends on VFIO_MDEV default n help Choose this option if you want to enable Intel GVT-g graphics -- 2.31.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915/gvt: Move mdev attribute groups into kvmgt module
As kvmgt module contains all handling for VFIO/mdev, leaving mdev attribute
groups in gvt module caused dependency issue. Although it was there for possible
other hypervisor usage, that turns out never to be true. So this moves all mdev
handling into kvmgt module completely to resolve dependency issue.
Cc: Arnd Bergmann
Cc: Jason Gunthorpe
Cc: Alex Williamson
Signed-off-by: Zhenyu Wang
---
drivers/gpu/drm/i915/gvt/gvt.c | 124 +
drivers/gpu/drm/i915/gvt/gvt.h | 3 -
drivers/gpu/drm/i915/gvt/hypercall.h | 2 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 129 +--
drivers/gpu/drm/i915/gvt/mpt.h | 4 +-
5 files changed, 126 insertions(+), 136 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index e7c2babcee8b..cbac409f6c8a 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -46,118 +46,6 @@ static const char * const supported_hypervisors[] = {
[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
};
-static struct intel_vgpu_type *
-intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id)
-{
- if (WARN_ON(type_group_id >= gvt->num_types))
- return NULL;
- return >types[type_group_id];
-}
-
-static ssize_t available_instances_show(struct mdev_type *mtype,
- struct mdev_type_attribute *attr,
- char *buf)
-{
- struct intel_vgpu_type *type;
- unsigned int num = 0;
- void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
-
- type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
- if (!type)
- num = 0;
- else
- num = type->avail_instance;
-
- return sprintf(buf, "%u\n", num);
-}
-
-static ssize_t device_api_show(struct mdev_type *mtype,
- struct mdev_type_attribute *attr, char *buf)
-{
- return sprintf(buf, "%s\n", VFIO_DEVICE_API_PCI_STRING);
-}
-
-static ssize_t description_show(struct mdev_type *mtype,
- struct mdev_type_attribute *attr, char *buf)
-{
- struct intel_vgpu_type *type;
- void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
-
- type = intel_gvt_find_vgpu_type(gvt, mtype_get_type_group_id(mtype));
- if (!type)
- return 0;
-
- return sprintf(buf, "low_gm_size: %dMB\nhigh_gm_size: %dMB\n"
- "fence: %d\nresolution: %s\n"
- "weight: %d\n",
- BYTES_TO_MB(type->low_gm_size),
- BYTES_TO_MB(type->high_gm_size),
- type->fence, vgpu_edid_str(type->resolution),
- type->weight);
-}
-
-static MDEV_TYPE_ATTR_RO(available_instances);
-static MDEV_TYPE_ATTR_RO(device_api);
-static MDEV_TYPE_ATTR_RO(description);
-
-static struct attribute *gvt_type_attrs[] = {
- _type_attr_available_instances.attr,
- _type_attr_device_api.attr,
- _type_attr_description.attr,
- NULL,
-};
-
-static struct attribute_group *gvt_vgpu_type_groups[] = {
- [0 ... NR_MAX_INTEL_VGPU_TYPES - 1] = NULL,
-};
-
-static bool intel_get_gvt_attrs(struct attribute_group
***intel_vgpu_type_groups)
-{
- *intel_vgpu_type_groups = gvt_vgpu_type_groups;
- return true;
-}
-
-static int intel_gvt_init_vgpu_type_groups(struct intel_gvt *gvt)
-{
- int i, j;
- struct intel_vgpu_type *type;
- struct attribute_group *group;
-
- for (i = 0; i < gvt->num_types; i++) {
- type = >types[i];
-
- group = kzalloc(sizeof(struct attribute_group), GFP_KERNEL);
- if (WARN_ON(!group))
- goto unwind;
-
- group->name = type->name;
- group->attrs = gvt_type_attrs;
- gvt_vgpu_type_groups[i] = group;
- }
-
- return 0;
-
-unwind:
- for (j = 0; j < i; j++) {
- group = gvt_vgpu_type_groups[j];
- kfree(group);
- }
-
- return -ENOMEM;
-}
-
-static void intel_gvt_cleanup_vgpu_type_groups(struct intel_gvt *gvt)
-{
- int i;
- struct attribute_group *group;
-
- for (i = 0; i < gvt->num_types; i++) {
- group = gvt_vgpu_type_groups[i];
- gvt_vgpu_type_groups[i] = NULL;
- kfree(group);
- }
-}
-
static const struct intel_gvt_ops intel_gvt_ops = {
.emulate_cfg_read = intel_vgpu_emulate_cfg_read,
.emulate_cfg_write = intel_vgpu_emulate_cfg_write,
@@ -169,8 +57,6 @@ static const struct intel_gvt_ops intel_gvt_ops = {
.vgpu_reset = intel_gvt_reset_vgpu,
.vgpu_activate = intel_gvt_activate_vgpu,
.vgpu_deactivate = intel_gvt_deactivate_vgpu,
-
[Intel-gfx] [PULL] gvt-next-fixes
Hi, Here's just another left fix for possible divide error in vgpu display rate calculation by Colin. btw, I'll need a backmerge of linus tree or maybe wait till rc1 to apply gvt mdev dependency fix from https://patchwork.freedesktop.org/series/89479/ Thanks -- The following changes since commit e65a4d378480101f222e8f6978c22e590c1fb7b5: Merge tag 'gvt-next-fixes-2021-04-21' of https://github.com/intel/gvt-linux into drm-intel-next-fixes (2021-04-21 13:22:30 +0300) are available in the Git repository at: https://github.com/intel/gvt-linux tags/gvt-next-fixes-2021-04-29 for you to fetch changes up to d385c16173f28a18866abf54c764200c276dace0: drm/i915/gvt: Prevent divided by zero when calculating refresh rate (2021-04-29 17:00:09 +0800) gvt-next-fixes-2021-04-29 - Fix possible divide error in vgpu display rate calculation (Colin) Colin Xu (1): drm/i915/gvt: Prevent divided by zero when calculating refresh rate drivers/gpu/drm/i915/gvt/handlers.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
On 2021.04.28 14:21:41 -0300, Jason Gunthorpe wrote:
> On Wed, Apr 28, 2021 at 01:32:43PM +0800, Zhenyu Wang wrote:
> > On 2021.04.27 09:12:35 -0300, Jason Gunthorpe wrote:
> > > On Tue, Apr 27, 2021 at 10:45:06AM +0800, Zhenyu Wang wrote:
> > > > On 2021.04.26 10:39:24 -0300, Jason Gunthorpe wrote:
> > > > > On Mon, Apr 26, 2021 at 05:41:42PM +0800, Zhenyu Wang wrote:
> > > > > > @@ -1667,19 +1773,26 @@ static struct mdev_parent_ops
> > > > > > intel_vgpu_ops = {
> > > > > >
> > > > > > static int kvmgt_host_init(struct device *dev, void *gvt, const
> > > > > > void *ops)
> > > > > > {
> > > > > > - struct attribute_group **kvm_vgpu_type_groups;
> > > > > > + int ret;
> > > > > > +
> > > > > > + ret = intel_gvt_init_vgpu_type_groups((struct intel_gvt *)gvt);
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > >
> > > > > > intel_gvt_ops = ops;
> > > > > > - if (!intel_gvt_ops->get_gvt_attrs(_vgpu_type_groups))
> > > > > > - return -EFAULT;
> > > > > > - intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
> > > > > > + intel_vgpu_ops.supported_type_groups = gvt_vgpu_type_groups;
> > > > >
> > > > > This patch is an improvement, but this fictional dynamic behavior is
> > > > > still wrong. The supported_type_groups directly flows from the
> > > > > vgpu_types array in vgpu.c and it should not be split up like this
> > > > >
> > > > > The code copies the rodata vgpu_types into dynamic memory gvt->types
> > > > > then copies that dynamic memory into a dynamic gvt_vgpu_type_groups,
> > > > > which makes very little sense at all.
> > > >
> > > > vgpu_types is static for we want fixed vgpu mdev type, but actual
> > > > supported
> > > > types depend on HW resources e.g aperture mem, fence, etc, that's
> > > > dynamic for
> > > > gvt->types, so gvt_vgpu_type_groups is dynamic from gvt->types.
> > >
> > > Well, that's worse then, intel_vgpu_ops.supported_type_groups is a
> > > static global, it cannot depend on HW variable calculations.
> >
> > sorry, maybe I didn't describe this properly, gvt_vgpu_type_groups is
> > static global, but the actual supported types is determined from
> > gvt->types and initialized before mdev device register.
>
> No, I got it, I saw the code too.
>
> Think about what happens if there are two GPUs in the system, you get
> two gvt's and with different properties and you are trying to squeeze
> that into a single static global - it doesn't work.
>
Well, that's not what this change trys to resolve and on gvt supported platform
that's not an available hardware configuration for IGD. So I think this is fine.
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
On 2021.04.27 09:12:35 -0300, Jason Gunthorpe wrote:
> On Tue, Apr 27, 2021 at 10:45:06AM +0800, Zhenyu Wang wrote:
> > On 2021.04.26 10:39:24 -0300, Jason Gunthorpe wrote:
> > > On Mon, Apr 26, 2021 at 05:41:42PM +0800, Zhenyu Wang wrote:
> > > > @@ -1667,19 +1773,26 @@ static struct mdev_parent_ops intel_vgpu_ops = {
> > > >
> > > > static int kvmgt_host_init(struct device *dev, void *gvt, const void
> > > > *ops)
> > > > {
> > > > - struct attribute_group **kvm_vgpu_type_groups;
> > > > + int ret;
> > > > +
> > > > + ret = intel_gvt_init_vgpu_type_groups((struct intel_gvt *)gvt);
> > > > + if (ret)
> > > > + return ret;
> > > >
> > > > intel_gvt_ops = ops;
> > > > - if (!intel_gvt_ops->get_gvt_attrs(_vgpu_type_groups))
> > > > - return -EFAULT;
> > > > - intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
> > > > + intel_vgpu_ops.supported_type_groups = gvt_vgpu_type_groups;
> > >
> > > This patch is an improvement, but this fictional dynamic behavior is
> > > still wrong. The supported_type_groups directly flows from the
> > > vgpu_types array in vgpu.c and it should not be split up like this
> > >
> > > The code copies the rodata vgpu_types into dynamic memory gvt->types
> > > then copies that dynamic memory into a dynamic gvt_vgpu_type_groups,
> > > which makes very little sense at all.
> >
> > vgpu_types is static for we want fixed vgpu mdev type, but actual supported
> > types depend on HW resources e.g aperture mem, fence, etc, that's dynamic
> > for
> > gvt->types, so gvt_vgpu_type_groups is dynamic from gvt->types.
>
> Well, that's worse then, intel_vgpu_ops.supported_type_groups is a
> static global, it cannot depend on HW variable calculations.
>
sorry, maybe I didn't describe this properly, gvt_vgpu_type_groups is
static global, but the actual supported types is determined from
gvt->types and initialized before mdev device register.
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] Revert "vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV"
On 2021.04.26 14:40:17 -0300, Jason Gunthorpe wrote:
> On Mon, Apr 26, 2021 at 10:55:55AM -0600, Alex Williamson wrote:
> > On Mon, 26 Apr 2021 17:41:43 +0800
> > Zhenyu Wang wrote:
> >
> > > This reverts commit 07e543f4f9d116d6b4240644191dee6388ef4a85.
> >
> > 07e543f4f9d1 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV")
> >
> > > With all mdev handing moved to kvmgt module, only kvmgt should depend
> > > on VFIO_MDEV. So revert this one.
> > >
> > > Cc: Arnd Bergmann
> > > Cc: Jason Gunthorpe
> > > Cc: Alex Williamson
> > > Signed-off-by: Zhenyu Wang
> > > drivers/gpu/drm/i915/Kconfig | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> > > index 8f15bfb5faac..93f4d059fc89 100644
> > > +++ b/drivers/gpu/drm/i915/Kconfig
> > > @@ -102,7 +102,6 @@ config DRM_I915_GVT
> > > bool "Enable Intel GVT-g graphics virtualization host support"
> > > depends on DRM_I915
> > > depends on 64BIT
> > > - depends on VFIO_MDEV
> > > default n
> > > help
> > > Choose this option if you want to enable Intel GVT-g graphics
> >
> > I take it that this retracts your ack from
> > https://lore.kernel.org/dri-devel/20210425032356.gh1...@zhen-hp.sh.intel.com/
> > I'll drop it from consideration for pushing through my tree unless
> > indicated otherwise. Thanks,
>
> In any event you'll need either Arnd's patch or this patch in your
> tree to avoid randconfig problems.
>
> At this point I would take Arnd's and leave this to go next merge
> window.
>
I'm ok with that, so won't block your vfio pull for merge window.
I'll send gvt fixes pull in next RC.
Thanks
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: Move mdev attribute groups into kvmgt module
On 2021.04.26 10:39:24 -0300, Jason Gunthorpe wrote:
> On Mon, Apr 26, 2021 at 05:41:42PM +0800, Zhenyu Wang wrote:
> > @@ -1667,19 +1773,26 @@ static struct mdev_parent_ops intel_vgpu_ops = {
> >
> > static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
> > {
> > - struct attribute_group **kvm_vgpu_type_groups;
> > + int ret;
> > +
> > + ret = intel_gvt_init_vgpu_type_groups((struct intel_gvt *)gvt);
> > + if (ret)
> > + return ret;
> >
> > intel_gvt_ops = ops;
> > - if (!intel_gvt_ops->get_gvt_attrs(_vgpu_type_groups))
> > - return -EFAULT;
> > - intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
> > + intel_vgpu_ops.supported_type_groups = gvt_vgpu_type_groups;
>
> This patch is an improvement, but this fictional dynamic behavior is
> still wrong. The supported_type_groups directly flows from the
> vgpu_types array in vgpu.c and it should not be split up like this
>
> The code copies the rodata vgpu_types into dynamic memory gvt->types
> then copies that dynamic memory into a dynamic gvt_vgpu_type_groups,
> which makes very little sense at all.
vgpu_types is static for we want fixed vgpu mdev type, but actual supported
types depend on HW resources e.g aperture mem, fence, etc, that's dynamic for
gvt->types, so gvt_vgpu_type_groups is dynamic from gvt->types.
>
> vgpu_types should be moved to kvmgt and everything should be static,
> like every other mdev driver. Copy the 'type' information from the
> gpu_types static when the mdev is created.
>
I don't think that's necessary, as it's not static for gvt at all.
The logic to handle type resource change can still be in vgpu.c
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
