Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 10/11/2021 12:04, Lu Baolu wrote:
> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 07:12, Lu Baolu wrote:
>>> Hi Tvrtko,
>>>
>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>
>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>>> and probe presence of iommu domain per device to accurately reflect its
>>>> status.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>> ---
>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the 
>>>> intel_iommu
>>>> driver. But it certainly appears the setup can assign some iommu ops 
>>>> (and
>>>> assign the discrete i915 to iommu group) when those two are set to off.
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index e967cd08f23e..9fb38a54f1fe 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -1763,26 +1763,27 @@ static inline bool run_as_guest(void)
>>>   #define HAS_D12_PLANE_MINIMIZATION(dev_priv) 
>>> (IS_ROCKETLAKE(dev_priv) || \
>>>                             IS_ALDERLAKE_S(dev_priv))
>>>
>>> -static inline bool intel_vtd_active(void)
>>> +static inline bool intel_vtd_active(struct drm_i915_private *i915)
>>>   {
>>> -#ifdef CONFIG_INTEL_IOMMU
>>> -    if (intel_iommu_gfx_mapped)
>>> +    if (iommu_get_domain_for_dev(i915->drm.dev))
>>>           return true;
>>> -#endif
>>>
>>>       /* Running as a guest, we assume the host is enforcing VT'd */
>>>       return run_as_guest();
>>>   }
>>>
>>> Have you verified this change? I am afraid that
>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>> intel_iommu_gfx_mapped == 0.
>>
>> Yes it seems to work as is:
>>
>> default:
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>
>> intel_iommu=igfx_off:
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>
>> On my system dri device 0 is integrated graphics and 1 is discrete.
> 
> The drm device 0 has a dedicated iommu. When the user request igfx not
> mapped, the VT-d implementation will turn it off to save power. But for
> shared iommu, you definitely will get it enabled.

Sorry I am not following, what exactly do you mean? Is there a platform 
with integrated graphics without a dedicated iommu, in which case 
intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and 
iommu_get_domain_for_dev returning non-NULL?

Regards,

Tvrtko