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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 705C3C433FE for ; Wed, 16 Nov 2022 12:14:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238601AbiKPMOO (ORCPT ); Wed, 16 Nov 2022 07:14:14 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33838 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232951AbiKPMNy (ORCPT ); Wed, 16 Nov 2022 07:13:54 -0500 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 23B464B988 for ; Wed, 16 Nov 2022 04:08:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1668600516; x=1700136516; h=message-id:date:mime-version:cc:subject:to:references: from:in-reply-to:content-transfer-encoding; bh=ROaSIFRIO9vsgedLMOHZiWNscCfhR6myclt50kra5OA=; b=KPy2rWqd4O1LmiWl9mBiBF9TRq8mbaKavx53RWUvI6MeFwkD6K8JwJyr B9IQkhe5wEmUyo0cqE1upZ6G8aHQeGdUeKqm5nTUgaxSinp7OveVDk61+ ao3izyzF110JEFQPIHHG+iOgFnDgsTUZnjxeBIwWoiI3XzNKp/xd/7E8A 9EPNI+kImRxO4aIfc+oiTdY1bgpVFlAmDZ1/dt9fixY06Q+BeyiZSEYBi r0mVwBzhUD/U4hZiTaMGLZk4p/Z8KuAAA+WqaRBCy5bOOQ0VtUbl53uhb wJ+hyYUIt89TDm1T6mMPxrBYLoPFNGsGBTAFNJK8+ot3gljUW/lLaS37E Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10532"; a="312534528" X-IronPort-AV: E=Sophos;i="5.96,167,1665471600"; d="scan'208";a="312534528" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Nov 2022 04:08:35 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10532"; a="641620162" X-IronPort-AV: E=Sophos;i="5.96,167,1665471600"; d="scan'208";a="641620162" Received: from blu2-mobl3.ccr.corp.intel.com (HELO [10.254.212.114]) ([10.254.212.114]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Nov 2022 04:08:33 -0800 Message-ID: Date: Wed, 16 Nov 2022 20:08:30 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Cc: baolu.lu@linux.intel.com, Joerg Roedel , Will Deacon , Robin Murphy , "Liu, Yi L" , "Pan, Jacob jun" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v3 4/7] iommu/vt-d: Fold dmar_remove_one_dev_info() into its caller Content-Language: en-US To: "Tian, Kevin" , "iommu@lists.linux.dev" References: <20221114014049.3959-1-baolu.lu@linux.intel.com> <20221114014049.3959-5-baolu.lu@linux.intel.com> From: Baolu Lu In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2022/11/16 17:15, Tian, Kevin wrote: >> From: Baolu Lu >> Sent: Wednesday, November 16, 2022 4:03 PM >> >> On 2022/11/16 13:35, Tian, Kevin wrote: >>>> From: Baolu Lu >>>> Sent: Wednesday, November 16, 2022 12:36 PM >>>> >>>> On 11/16/22 11:53 AM, Tian, Kevin wrote: >>>>>> From: Lu Baolu >>>>>> Sent: Monday, November 14, 2022 9:41 AM >>>>>> @@ -4562,7 +4538,10 @@ static void >> intel_iommu_release_device(struct >>>>>> device *dev) >>>>>> { >>>>>> struct device_domain_info *info = dev_iommu_priv_get(dev); >>>>>> >>>>>> - dmar_remove_one_dev_info(dev); >>>>>> + iommu_disable_pci_caps(info); >>>>>> + domain_context_clear(info); >>>>>> + device_block_translation(dev); >>>>> clear context after blocking translation. >>>> Unfortunately domain_context_clear() needs reference to info->domain >>>> (for domain id when flushing cache), which is cleared in >>>> device_block_translation(). >>>> >>> this sounds an ordering problem. clearing context should be after >>> blocking translation in concept. >> >> At present, when the default domain is attached to the device, we first >> populate the pasid table entry, and then populate the device context >> entry. Above code is just the reverse operation. >> >> Can you see any practical problems caused by this sequence? If so, it >> seems that we should carefully consider whether such problems already >> exist. >> > > there is no problem with existing code. Just after this patch the order > looks weird based on the literal name of those functions. > > domain_context_clear() is a big hammer to disable the context entry, > implying translation must be blocked. Then calling another block > translation afterwards becomes unnecessary. > > Probably it should be split into two functions with one requiring > info->domain called before block translation and the rest which > actually clears the context entry being the last step? This is what the existing code does. Perhaps I should drop this patch, or only rename iommu_disable_dev_iotlb() to iommu_disable_pci_caps(). Best regards, baolu