From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6B456C433F5 for ; Thu, 11 Nov 2021 15:18:23 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2557E610D0 for ; Thu, 11 Nov 2021 15:18:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 2557E610D0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 553A96E463; Thu, 11 Nov 2021 15:18:22 +0000 (UTC) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id A72BF6E463; Thu, 11 Nov 2021 15:18:20 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10164"; a="212962424" X-IronPort-AV: E=Sophos;i="5.87,226,1631602800"; d="scan'208";a="212962424" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Nov 2021 07:18:20 -0800 X-IronPort-AV: E=Sophos;i="5.87,226,1631602800"; d="scan'208";a="589998163" Received: from hscahill-mobl.ger.corp.intel.com (HELO [10.213.223.189]) ([10.213.223.189]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Nov 2021 07:18:18 -0800 To: Robin Murphy , Lu Baolu , Intel-gfx@lists.freedesktop.org References: <20211109121759.170915-1-tvrtko.ursulin@linux.intel.com> <6e8c55a7-45b6-57ab-35f7-d522401efccb@linux.intel.com> <4d1a0ab9-e0d8-2ed9-1fc4-9ffaf2f19bef@linux.intel.com> <7b2e1427-69cf-8f5d-0c15-73c4e602953d@linux.intel.com> <2a1ae709-19f8-7983-b171-98ec2f3f010a@linux.intel.com> <4c5ab72f-aaff-8b92-7471-44dd907cf2f6@linux.intel.com> <215fa7de-4ed7-1da5-724e-006e36286c08@linux.intel.com> <9463fda7-d215-6c14-3ca7-a2ff94349c3e@arm.com> From: Tvrtko Ursulin Organization: Intel Corporation UK Plc Message-ID: Date: Thu, 11 Nov 2021 15:18:17 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <9463fda7-d215-6c14-3ca7-a2ff94349c3e@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: dri-devel@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On 10/11/2021 14:37, Robin Murphy wrote: > On 2021-11-10 14:11, Tvrtko Ursulin wrote: >> >> On 10/11/2021 12:35, Lu Baolu wrote: >>> On 2021/11/10 20:08, Tvrtko Ursulin wrote: >>>> >>>> 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 >>>>>>>> >>>>>>>> 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 >>>>>>>> Cc: Lu Baolu >>>>>>>> --- >>>>>>>> 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? >>> >>> Your code always work for an igfx with a dedicated iommu. This might be >>> always true on today's platforms. But from driver's point of view, we >>> should not make such assumption. >>> >>> For example, if the iommu implementation decides not to turn off the >>> graphic iommu (perhaps due to some hw quirk or for graphic >>> virtualization), your code will be broken. >> >> If I got it right, this would go back to your earlier recommendation >> to have the check look like this: >> >> static bool intel_vtd_active(struct drm_i915_private *i915) >> { >>          struct iommu_domain *domain; >> >>          domain = iommu_get_domain_for_dev(i915->drm.dev); >>          if (domain && (domain->type & __IOMMU_DOMAIN_PAGING)) >>                  return true; >>      ... >> >> This would be okay as a first step? >> >> Elsewhere in the thread Robin suggested looking at the dec->dma_ops >> and comparing against iommu_dma_ops. These two solution would be >> effectively the same? > > Effectively, yes. See iommu_setup_dma_ops() - the only way to end up > with iommu_dma_ops is if a managed translation domain is present; if the > IOMMU is present but the default domain type has been set to passthrough > (either globally or forced for the given device) it will do nothing and > leave you with dma-direct, while if the IOMMU has been ignored entirely > then it should never even be called. Thus it neatly encapsulates what > you're after here. One concern I have is whether the pass-through mode truly does nothing or addresses perhaps still go through the dmar hardware just with no translation? If latter then most like for like change is actually exactly what the first version of my patch did. That is replace intel_iommu_gfx_mapped with a plain non-NULL check on iommu_get_domain_for_dev. Regards, Tvrtko