From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yu, Zhang" Subject: Re: [PATCH 1/8] drm/i915: Introduce a PV INFO page structure for Intel GVT-g. Date: Mon, 29 Sep 2014 20:08:13 +0800 Message-ID: <54294BAD.7060105@linux.intel.com> References: <1411152428-7226-1-git-send-email-jike.song@intel.com> <1411152428-7226-2-git-send-email-jike.song@intel.com> <20140919072546.GB21738@nuc-i3427.alporthouse.com> <54294638.9060103@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 590A36E112 for ; Mon, 29 Sep 2014 05:09:25 -0700 (PDT) In-Reply-To: <54294638.9060103@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jike Song , Chris Wilson Cc: intel-gfx@lists.freedesktop.org, daniel.vetter@intel.com List-Id: intel-gfx@lists.freedesktop.org On 9/29/2014 7:44 PM, Jike Song wrote: > On 09/19/2014 03:25 PM, Chris Wilson wrote: >> Now, given that these are simply trapped memory access, wouldn't it be >> simply to have: >> >> struct i915_virtual_gpu { >> struct vgt_if *if; >> } vgu; >> >> static inline bool intel_vgpu_active(struct drm_i915_private *i915) { >> return i915->vgpu.if; } >> >> then you have constructs like >> void i915_check_vgpu(struct drm_i915_private *i915) >> { >> struct vgt_if *if; >> >> if = i915->regs + VGT_PVINFO_PAGE; >> if (if->magic != VGT_MAGIC) >> return; >> >> if (INTEL_VGT_IF_VERSION != >> INTEL_VGT_IF_VERSION_ENCODE(if->version_major, >> if->version_minor)) >> return; >> >> >> i915->vgpu.if = if; >> } >> >> And later, for example: >> >> if (intel_vgpu_active(dev_priv)) >> dev_priv->num_fence_regs = dev_priv->vgpu.if->fence_num; >> > > Hi Chris, sorry that I didn't understand you correctly. After discussion > with Yu today, I realized that unfortunately, the vgt_if can't be > dereferenced directly. > > There are several reasons: > > - dereferencing a MMIO address will be complained by sparse(1) > > - for Guest VM, such accesses will be trapped by hypervisor, and > hence emulated correctly; However this doesn't work for Host(e.g. > Domain 0 of Xen, the Linux host KVM resides in). For host, we used > a proactive mechanism to redirect i915 MMIO accesses to vgt, > the GPU device-model, for the sake of central management & sharing > among VMs, including host. > > Given that, though technically your code works for Guest, but after the > integration of host support of iGVT, we still need to use > I915_READ/I915_WRITE > then. The host patches is soon to posted for your review :) > > I should have realized that earlier, sorry! > Hi Chris, Sorry, I also should have noticed this issue earlier. To my understanding, the reason you proposed to use the "struct vgt_if *if" in struct vgpu, to replace the previous vgpu_active, is to simplify the mmio accesses in our patches. This suggestion works fine from the guest & native point of view. However, just like Jike's mail said, this change may not work for the host side, which also need to visit the PVINFO page from time to time. So, could we still keep the vgpu_active flag when detecting virtual gpu, and read the mmio registers in PVINFO structure by I915_READ? Thanks Yu > >> -Chris >> > > -- > Thanks, > Jike > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >