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: Fri, 10 Oct 2014 16:23:52 +0800 Message-ID: <54379798.1060301@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> <20140929121625.GL19278@nuc-i3427.alporthouse.com> <54295335.8090203@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTP id CC07D6E408 for ; Fri, 10 Oct 2014 01:30:07 -0700 (PDT) In-Reply-To: <54295335.8090203@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Jike Song Cc: Daniel , "intel-gfx@lists.freedesktop.org" <"intel-gfx@lists.freedesktop.org>, "@intel.com>, Vetter@freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 9/29/2014 8:40 PM, Jike Song wrote: > On 09/29/2014 08:16 PM, Chris Wilson wrote: >> On Mon, Sep 29, 2014 at 07:44:56PM +0800, 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. >> >> You only need to be careful during vgpu detection. After that you know >> everything is safe. If you do the detection during intel_uncore_init(), >> or similar, you can use raw mmio access and explict sparse annotations >> to do the right thing. >> -Chris > > Hi Chris, > > Thanks for your suggestion, however, it addressed issue #1 but not issue > #2. > I'd like to give a detailed explanation :) > > For Guest i915 driver, when accessing a MMIO reg, it goes: > > whatever I915_READ/readl or direct dereferencing(addr) > addr is translated to gpa(guest physical address) by guest > paging structure > hypervisor trapped the gpa access and forward it to vgt > vgt_emulate_read32(vgt instance of guest, gpa) > > > The real problem is, when running as the host gpu driver, i915 MMIO/GTT > accesses are: > > 1) every difficult to trap, say the domain 0 of Xen hypervisor; or > 2) impossible to trap, say KVM(In KVM, the host i915 is running in > the very same > privilege level and root mode as KVM hypervisor.) > > So, for Host i915 driver, when accessing a MMIO reg, it goes: > > I915_READ(addr) > gen6_read32(addr) > offset = addr - dev_priv->regs > vgt_mmio_readl(the vgt instance of host, offset) > .. some processing .. > if (passed-throuth) > readl(offset + dev_priv->regs) > else > pa = BAR0 + offset > vgt_emulate_read32(vgt instance of host, pa) > > > The vgt_if corresponds to the PVINFO page, not passed-through(actually > it doesn't physically exist), > should fall into the "else" above. > > Given that ... Yes we can dereference pointers for guest here, but > consider that we'll > deal with host in the future ... > Hi Chris, Any comments on this issue? The host side patches were sent out by Jike last week. As you can see in his patch, the I915_READ/WRITEs are redefined, and the MMIO accesses are mediated to vgt, by offset of the regs, instead of by read/write (dev_priv->regs + offset) for host i915. So, in order to let i915 in the host side access the PVINFO page successfully, could we still use the I915_READ/WRTE, and keep the vgpu_active flag when detecting virtual gpu? :) Thanks Yu > > -- > Thanks, > Jike > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >