Re: [PATCH 1/8] drm/i915: Introduce a PV INFO page structure for Intel GVT-g.

["Yu, Zhang" <yu.c.zhang@linux.intel.com>]

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
>
>