[PATCH] drm/i915/gvt: validate gfn before set shadow page entry
Zhi Wang
zhi.a.wang at intel.com
Fri Jan 5 05:06:21 UTC 2018
Reviewed-by: Zhi Wang <zhi.a.wang at intel.com>
On 01/04/18 10:46, Yuan, Hang wrote:
> Any comments? Thanks.
>
>> -----Original Message-----
>> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On
>> Behalf Of hang.yuan at intel.com
>> Sent: Friday, December 22, 2017 6:07 PM
>> To: intel-gvt-dev at lists.freedesktop.org
>> Cc: Yuan, Hang <hang.yuan at intel.com>
>> Subject: [PATCH] drm/i915/gvt: validate gfn before set shadow page entry
>>
>> From: Hang Yuan <hang.yuan at intel.com>
>>
>> GVT may receive partial write on one guest PTE update. Validate gfn not to
>> translate incomplete gfn. This avoids some unnecessary error messages
>> incurred by the incomplete gfn translating. Also fix the bug that the whole
>> PPGTT shadow page update is aborted on any invalid gfn entry.
>>
>> gfn validation relys on hypervisor's help. Add one MPT module function to
>> provide the function.
>>
>> Signed-off-by: Hang Yuan <hang.yuan at intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/gtt.c | 24 +++++++++++++++++++-----
>> drivers/gpu/drm/i915/gvt/hypercall.h | 1 +
>> drivers/gpu/drm/i915/gvt/kvmgt.c | 16 ++++++++++++++++
>> drivers/gpu/drm/i915/gvt/mpt.h | 17 +++++++++++++++++
>> 4 files changed, 53 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
>> index 71a0f2b..3c154c0 100644
>> --- a/drivers/gpu/drm/i915/gvt/gtt.c
>> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
>> @@ -997,9 +997,11 @@ static inline void
>> ppgtt_generate_shadow_entry(struct intel_gvt_gtt_entry *se, static int
>> ppgtt_populate_shadow_page(struct intel_vgpu_ppgtt_spt *spt) {
>> struct intel_vgpu *vgpu = spt->vgpu;
>> + struct intel_gvt *gvt = vgpu->gvt;
>> + struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops;
>> struct intel_vgpu_ppgtt_spt *s;
>> struct intel_gvt_gtt_entry se, ge;
>> - unsigned long i;
>> + unsigned long gfn, i;
>> int ret;
>>
>> trace_spt_change(spt->vgpu->id, "born", spt, @@ -1007,9 +1009,10
>> @@ static int ppgtt_populate_shadow_page(struct intel_vgpu_ppgtt_spt
>> *spt)
>>
>> if (gtt_type_is_pte_pt(spt->shadow_page.type)) {
>> for_each_present_guest_entry(spt, &ge, i) {
>> - ret = gtt_entry_p2m(vgpu, &ge, &se);
>> - if (ret)
>> - goto fail;
>> + gfn = ops->get_pfn(&ge);
>> + if (!intel_gvt_hypervisor_is_valid_gfn(vgpu, gfn) ||
>> + gtt_entry_p2m(vgpu, &ge, &se))
>> + ops->set_pfn(&se, gvt->gtt.scratch_mfn);
>> ppgtt_set_shadow_entry(spt, &se, i);
>> }
>> return 0;
>> @@ -1903,7 +1906,7 @@ static int emulate_gtt_mmio_write(struct
>> intel_vgpu *vgpu, unsigned int off,
>> struct intel_vgpu_mm *ggtt_mm = vgpu->gtt.ggtt_mm;
>> struct intel_gvt_gtt_pte_ops *ops = gvt->gtt.pte_ops;
>> unsigned long g_gtt_index = off >> info->gtt_entry_size_shift;
>> - unsigned long gma;
>> + unsigned long gma, gfn;
>> struct intel_gvt_gtt_entry e, m;
>> int ret;
>>
>> @@ -1922,6 +1925,16 @@ static int emulate_gtt_mmio_write(struct
>> intel_vgpu *vgpu, unsigned int off,
>> bytes);
>>
>> if (ops->test_present(&e)) {
>> + gfn = ops->get_pfn(&e);
>> +
>> + /* one PTE update may be issued in multiple writes and the
>> + * first write may not construct a valid gfn
>> + */
>> + if (!intel_gvt_hypervisor_is_valid_gfn(vgpu, gfn)) {
>> + ops->set_pfn(&m, gvt->gtt.scratch_mfn);
>> + goto out;
>> + }
>> +
>> ret = gtt_entry_p2m(vgpu, &e, &m);
>> if (ret) {
>> gvt_vgpu_err("fail to translate guest gtt entry\n");
>> @@ -1936,6 +1949,7 @@ static int emulate_gtt_mmio_write(struct
>> intel_vgpu *vgpu, unsigned int off,
>> ops->set_pfn(&m, gvt->gtt.scratch_mfn);
>> }
>>
>> +out:
>> ggtt_set_shadow_entry(ggtt_mm, &m, g_gtt_index);
>> gtt_invalidate(gvt->dev_priv);
>> ggtt_set_guest_entry(ggtt_mm, &e, g_gtt_index); diff --git
>> a/drivers/gpu/drm/i915/gvt/hypercall.h
>> b/drivers/gpu/drm/i915/gvt/hypercall.h
>> index a1bd82f..f8e77e1 100644
>> --- a/drivers/gpu/drm/i915/gvt/hypercall.h
>> +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
>> @@ -58,6 +58,7 @@ struct intel_gvt_mpt {
>> int (*set_opregion)(void *vgpu);
>> int (*get_vfio_device)(void *vgpu);
>> void (*put_vfio_device)(void *vgpu);
>> + bool (*is_valid_gfn)(unsigned long handle, unsigned long gfn);
>> };
>>
>> extern struct intel_gvt_mpt xengt_mpt;
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index f86983d..c55a5c0 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -1570,6 +1570,21 @@ static unsigned long kvmgt_virt_to_pfn(void
>> *addr)
>> return PFN_DOWN(__pa(addr));
>> }
>>
>> +static bool kvmgt_is_valid_gfn(unsigned long handle, unsigned long gfn)
>> +{
>> + struct kvmgt_guest_info *info;
>> + struct kvm *kvm;
>> +
>> + if (!handle_valid(handle))
>> + return false;
>> +
>> + info = (struct kvmgt_guest_info *)handle;
>> + kvm = info->kvm;
>> +
>> + return kvm_is_visible_gfn(kvm, gfn);
>> +
>> +}
>> +
>> struct intel_gvt_mpt kvmgt_mpt = {
>> .host_init = kvmgt_host_init,
>> .host_exit = kvmgt_host_exit,
>> @@ -1585,6 +1600,7 @@ struct intel_gvt_mpt kvmgt_mpt = {
>> .set_opregion = kvmgt_set_opregion,
>> .get_vfio_device = kvmgt_get_vfio_device,
>> .put_vfio_device = kvmgt_put_vfio_device,
>> + .is_valid_gfn = kvmgt_is_valid_gfn,
>> };
>> EXPORT_SYMBOL_GPL(kvmgt_mpt);
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/mpt.h
>> b/drivers/gpu/drm/i915/gvt/mpt.h index ca8005a..81aff4e 100644
>> --- a/drivers/gpu/drm/i915/gvt/mpt.h
>> +++ b/drivers/gpu/drm/i915/gvt/mpt.h
>> @@ -339,4 +339,21 @@ static inline void
>> intel_gvt_hypervisor_put_vfio_device(struct intel_vgpu *vgpu)
>> intel_gvt_host.mpt->put_vfio_device(vgpu);
>> }
>>
>> +/**
>> + * intel_gvt_hypervisor_is_valid_gfn - check if a visible gfn
>> + * @vgpu: a vGPU
>> + * @gfn: guest PFN
>> + *
>> + * Returns:
>> + * true on valid gfn, false on not.
>> + */
>> +static inline bool intel_gvt_hypervisor_is_valid_gfn(
>> + struct intel_vgpu *vgpu, unsigned long gfn) {
>> + if (!intel_gvt_host.mpt->is_valid_gfn)
>> + return true;
>> +
>> + return intel_gvt_host.mpt->is_valid_gfn(vgpu->handle, gfn); }
>> +
>> #endif /* _GVT_MPT_H_ */
>> --
>> 2.7.4
>>
>> _______________________________________________
>> intel-gvt-dev mailing list
>> intel-gvt-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
>
More information about the intel-gvt-dev
mailing list