On 2/11/22 1:39 AM, Edgecombe, Rick P wrote: > CC intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Thread: > https://lore.kernel.org/lkml/a5bb32b8-8bd7-ac98-5c4c-5af604ac8256@xxxxxxxxx/ > > On Wed, 2022-02-09 at 08:58 -0800, Dave Hansen wrote: >> On 1/30/22 13:18, Rick Edgecombe wrote: >>> >>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c >>> b/drivers/gpu/drm/i915/gvt/gtt.c >>> index 99d1781fa5f0..75ce4e823902 100644 >>> --- a/drivers/gpu/drm/i915/gvt/gtt.c >>> +++ b/drivers/gpu/drm/i915/gvt/gtt.c >>> @@ -1210,7 +1210,7 @@ static int split_2MB_gtt_entry(struct >>> intel_vgpu *vgpu, >>> } >>> >>> /* Clear dirty field. */ >>> - se->val64 &= ~_PAGE_DIRTY; >>> + se->val64 &= ~_PAGE_DIRTY_BITS; >>> >>> ops->clear_pse(se); >>> ops->clear_ips(se); >> >> Are these x86 CPU page table values? I see ->val64 being used like >> this: >> >> e->val64 &= ~GEN8_PAGE_PRESENT; >> and >> se.val64 |= GEN8_PAGE_PRESENT | GEN8_PAGE_RW; >> >> where we also have: >> >> #define GEN8_PAGE_PRESENT BIT_ULL(0) >> #define GEN8_PAGE_RW BIT_ULL(1) >> >> Which tells me that these are probably *close* to the CPU's page >> tables. >> But, I honestly don't know which format they are. I don't know if >> _PAGE_COW is still a software bit in that format or not. >> >> Either way, I don't think we should be messing with i915 device page >> tables. >> >> Or, are these somehow magically shared with the CPU in some way I >> don't >> know about? >> >> [ If these are device-only page tables, it would probably be nice to >> stop using _PAGE_FOO for them. It would avoid confusion like this. >> ] > > The two Reviewed-by tags are giving me pause, but as far as I can tell > this should not be setting _PAGE_DIRTY_BITS. This code seems to be > shadowing guest page tables, and the change would clear the COW > software bit in the guest page tables. So, yes, I think this should be > dropped. > Hi: According to the PRM https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-lkf-vol06-memory_views.pdf p.28, the GPU page table is IA-like and there will be scenarios when IA and gpu sharing a page table. That's why they are sharing part of the definitions. But the dirty bits will be ignored in the HW which GVT-g supports. The code should copy the bits from the guest PDPE 2M entry and then cleans some unused bits. So the _PAGE_DIRTY_ is misused here. I would suggest you can remove that line in your patch and I will clean this function after your patches got merged. Thanks, Zhi.
