[PATCH 5/9] drm/i915/gvt: Use standard pte bit definition

[Zhi Wang]

I see now. Reviewed-by: Zhi Wang <zhi.a.wang at intel.com>


On 12/25/17 18:53, Zhi Wang wrote:
> I'm OK with using PTE bit definition from kernel. I'm just worried 
> about the PAGE_SHIFT, since PAGE_SHIFT can be changed in CPU PTE 
> definition but in GPU PTE definition, it's fixed.
>
> On 12/25/17 18:37, Du, Changbin wrote:
>> On Mon, Dec 25, 2017 at 06:41:07PM +0800, Zhi Wang wrote:
>>> But PAGE_SHFIT might be changed on CPU side, right? On GPU side, it 
>>> won't be
>>> changed in any case. So I suggest you keep I915_GTT_PAGE_SHIFT and 
>>> I'm OK to
>>> another re-using of definitions.
>>>
>> If you checked the i915 gtt code, you can see i915 directly use some 
>> PTE bit
>> definition from kernel. So if i915 does it, we can.
>>> On 12/25/17 18:29, Du, Changbin wrote:
>>>> On Mon, Dec 25, 2017 at 06:24:16PM +0800, Zhi Wang wrote:
>>>>> Please use I915_GTT_PAGE_SHIFT.
>>>>>
>>>> Ideally, I just want to remove the I915_GTT_PAGE_SHIFT. GTT has 
>>>> same format
>>>> as x86 PT. We can use standard macro as i915 does. (But I see i915 
>>>> also has
>>>> some duplicated macros)
>>>>
>>>>> Thanks,
>>>>> Zhi.
>>>>>
>>>>> On 12/25/17 17:11, changbin.du at intel.com wrote:
>>>>>> From: Changbin Du <changbin.du at intel.com>
>>>>>>
>>>>>> GTT entry has similar format with the CPU PTE. We'd prefer named 
>>>>>> macro
>>>>>> instead of hardcode.
>>>>>>
>>>>>> Signed-off-by: Changbin Du <changbin.du at intel.com>
>>>>>> ---
>>>>>>     drivers/gpu/drm/i915/gvt/gtt.c | 22 +++++++++++-----------
>>>>>>     1 file changed, 11 insertions(+), 11 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c 
>>>>>> b/drivers/gpu/drm/i915/gvt/gtt.c
>>>>>> index fbb0aa2..86636689 100644
>>>>>> --- a/drivers/gpu/drm/i915/gvt/gtt.c
>>>>>> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
>>>>>> @@ -346,11 +346,11 @@ static unsigned long 
>>>>>> gen8_gtt_get_pfn(struct intel_gvt_gtt_entry *e)
>>>>>>         unsigned long pfn;
>>>>>>         if (e->type == GTT_TYPE_PPGTT_PTE_1G_ENTRY)
>>>>>> -        pfn = (e->val64 & ADDR_1G_MASK) >> 12;
>>>>>> +        pfn = (e->val64 & ADDR_1G_MASK) >> PAGE_SHIFT;
>>>>>>         else if (e->type == GTT_TYPE_PPGTT_PTE_2M_ENTRY)
>>>>>> -        pfn = (e->val64 & ADDR_2M_MASK) >> 12;
>>>>>> +        pfn = (e->val64 & ADDR_2M_MASK) >> PAGE_SHIFT;
>>>>>>         else
>>>>>> -        pfn = (e->val64 & ADDR_4K_MASK) >> 12;
>>>>>> +        pfn = (e->val64 & ADDR_4K_MASK) >> PAGE_SHIFT;
>>>>>>         return pfn;
>>>>>>     }
>>>>>> @@ -358,16 +358,16 @@ static void gen8_gtt_set_pfn(struct 
>>>>>> intel_gvt_gtt_entry *e, unsigned long pfn)
>>>>>>     {
>>>>>>         if (e->type == GTT_TYPE_PPGTT_PTE_1G_ENTRY) {
>>>>>>             e->val64 &= ~ADDR_1G_MASK;
>>>>>> -        pfn &= (ADDR_1G_MASK >> 12);
>>>>>> +        pfn &= (ADDR_1G_MASK >> PAGE_SHIFT);
>>>>>>         } else if (e->type == GTT_TYPE_PPGTT_PTE_2M_ENTRY) {
>>>>>>             e->val64 &= ~ADDR_2M_MASK;
>>>>>> -        pfn &= (ADDR_2M_MASK >> 12);
>>>>>> +        pfn &= (ADDR_2M_MASK >> PAGE_SHIFT);
>>>>>>         } else {
>>>>>>             e->val64 &= ~ADDR_4K_MASK;
>>>>>> -        pfn &= (ADDR_4K_MASK >> 12);
>>>>>> +        pfn &= (ADDR_4K_MASK >> PAGE_SHIFT);
>>>>>>         }
>>>>>> -    e->val64 |= (pfn << 12);
>>>>>> +    e->val64 |= (pfn << PAGE_SHIFT);
>>>>>>     }
>>>>>>     static bool gen8_gtt_test_pse(struct intel_gvt_gtt_entry *e)
>>>>>> @@ -377,7 +377,7 @@ static bool gen8_gtt_test_pse(struct 
>>>>>> intel_gvt_gtt_entry *e)
>>>>>>             return false;
>>>>>>         e->type = get_entry_type(e->type);
>>>>>> -    if (!(e->val64 & BIT(7)))
>>>>>> +    if (!(e->val64 & _PAGE_PSE))
>>>>>>             return false;
>>>>>>         e->type = get_pse_type(e->type);
>>>>>> @@ -395,17 +395,17 @@ static bool gen8_gtt_test_present(struct 
>>>>>> intel_gvt_gtt_entry *e)
>>>>>>                 || e->type == GTT_TYPE_PPGTT_ROOT_L4_ENTRY)
>>>>>>             return (e->val64 != 0);
>>>>>>         else
>>>>>> -        return (e->val64 & BIT(0));
>>>>>> +        return (e->val64 & _PAGE_PRESENT);
>>>>>>     }
>>>>>>     static void gtt_entry_clear_present(struct 
>>>>>> intel_gvt_gtt_entry *e)
>>>>>>     {
>>>>>> -    e->val64 &= ~BIT(0);
>>>>>> +    e->val64 &= ~_PAGE_PRESENT;
>>>>>>     }
>>>>>>     static void gtt_entry_set_present(struct intel_gvt_gtt_entry *e)
>>>>>>     {
>>>>>> -    e->val64 |= BIT(0);
>>>>>> +    e->val64 |= _PAGE_PRESENT;
>>>>>>     }
>>>>>>     /*
>>>>>>
>>>>> _______________________________________________
>>>>> intel-gvt-dev mailing list
>>>>> intel-gvt-dev at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
>>>>
>>