From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C959B353A8F for ; Wed, 26 Aug 2026 20:30:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787776228; cv=none; b=jytYwx2ue5bU2NcQYZLCEeAU1paqD1tNu8IvyOnTydso74sEIT1YVIMf4TasCFj9NmFvMC0rKpWXkh4oe8cLAjXvfupfFUabPqfo5PXCenuh9Ux6hSTnlKRF8f5/nOMhBNy/3CMW/+oS1zNDLUI8jD7M5LuLTknyXHkcRD7ipis= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787776228; c=relaxed/simple; bh=0kgi22xNJcVO36+dXKeosv/aiOf0RymZDzU7O1fom8k=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KG1cZvJAICDYWwtRq8g4QPymXqY2KiW8zdu0bFe9I+GqasmZaBfpvB+FaSIniTDNHjf82QnxZSLnrlKi5B+B87vxJ0R0WKpWroikn5FvQJd4EMORJeY46CdRn8nkL2pac9gbhbamWuW5yY2R+SajzulXW/U5jDsCGxf3KQ1m2D4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=T6/oEUK+; arc=none smtp.client-ip=209.85.214.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="T6/oEUK+" Received: by mail-pl1-f170.google.com with SMTP id d9443c01a7336-2d3b445a84fso7395ad.1 for ; Wed, 26 Aug 2026 13:30:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1787776226; x=1788381026; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to:content-type; bh=tviHa9iHC75nfLCPdWJnLGm9+adY8MzlYYxeJUkpU14=; b=T6/oEUK+s3C6+y6eTlV40UEFKemiQ07wosxior7fbZ7e22fWIOdSaKabI9xZIxeTqn 7eOexLBdLG9dFyx7/as4bnsGtmR8SHlGbM7iM9dTb6uvveRi3XirKYJG4J5nLcjEbZOk rut1RPoeB2yf+YLXJPsZc0w1gFq8kXqZnslnB4zmcmTJ5qtt+ymmphsD4r7/hA3G1d4T KsDOftPcoC+MuBI3S0cwMNcCLKkQQBuTdxpj+4JeB7D2RBtBnPy+Nbostj1b5WeoZdEh /6VxFOA2cLwJfZtyjnXxu1uNUDg0ay9ziJiQTV5WSTGSzioj1O9ymlYRPyRGDUDUhhy3 6q3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787776226; x=1788381026; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:from :date:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=tviHa9iHC75nfLCPdWJnLGm9+adY8MzlYYxeJUkpU14=; b=qDm6VENMMLB2jySOnk8G8FPiysB0PN2frXZOCrEn+TdzwqMEe66R+HdAUiX+E0ziR4 ds0f+ireM6gmCQdOZBFiO+N2hXUf1ISVH2/IR+j18VP+rB1UbxBNNsdj5HatddAfdiBf EJohusDBeX6FJXLsJH9Aj9qAH8jP6wKgLcbjoCw7EXdI9gVHcVeOe/azgz1gNQK4MkwK 99f/6bIH1uqXEF0ELCMTsXyCFCTmDsli/bOYBtH072sseK970Ebocch8KNKcFZMI+Tv1 vHHM0En5VLYi9EzN47LruJdocBubxE3eHkitfALDLlYMsMx+jdkpeNG05vMJK+1KRsJj 4Baw== X-Forwarded-Encrypted: i=1; AHgh+RrId5WF2/1d/VlT/lHRUYqUmFTLfxPsQCPVs7MHQzf5yhowc7RFWfOjFSmgtnsI6UXFRa+mR29/bPbzF/o=@vger.kernel.org X-Gm-Message-State: AFuF++ksUQqF5TzFy9TMCrFmDfBsxbyem5V/t3EKCCPJVVApQaFAVCnb xPCweQ80Tx4k2+NaGloY03yGctEoCm3QlDISasgZ+hS2iq5SsP0Q0P6K9q/0X7E7d2Y8bthHqiZ v6OrpOQ== X-Gm-Gg: AR+sD11ua3bMDWdNcrKbs3pYX6CTqmuReq4fGN4r3J+ZTTZoxgW8os+hOJ6DpeUSd4N hdun+ZWGCEzy3vG0/aMgDrXty4ZfztED4+0wkmORnb+K0kE3YF76WZDWZmi4MLCGR2JXhMUueB8 tee0C+Ithk5KauWEwuEsA6dRUsrYm91MmecPpbDwt8U7X2fIRoQM0F1fkJlbYqzpuraxiUS8tAl sZwp/Hod0+uLK2ll4UBPnp4+t/8OY8TtKMOI1Ej5P5ZrtLdEV1NHJ6Lb7c3pQ4fTNVtpxsDdUAk zpcyZya7DVksWhC8pa4A8vtuY1WPqLV8leZiJp9S0nL3pYl3YzqFwhpXPro3G6w1fNF2V6vGVwL sZU0GfQDdz1RnxxBAazHY431nqJ03v/ULExFSXcKfQiocMmZpnMkGyI97yEAl1IFXb05y/S3v2L 3MrpPxzyEy2YHIPIcuEv1Qca14pFl2VBUbvgJIPqY1cSPTg+itj213r4RdnRG4WAwA4MTh3uQS1 ZUbrqhXtmevACcO/dQa2mGFJK3ypfN5JcoDGeDt/xUFZqWTo8WDu1UsU2U= X-Received: by 2002:a17:903:b0e:b0:2bd:3bfd:74f1 with SMTP id d9443c01a7336-2d72a67b81amr2346875ad.2.1787776225466; Wed, 26 Aug 2026 13:30:25 -0700 (PDT) Received: from google.com (210.87.127.34.bc.googleusercontent.com. [34.127.87.210]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-cc1befce3b8sm1387309a12.5.2026.08.26.13.30.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Aug 2026 13:30:24 -0700 (PDT) Date: Wed, 26 Aug 2026 20:30:19 +0000 From: Samiullah Khawaja To: Baolu Lu Cc: David Woodhouse , Joerg Roedel , Will Deacon , Jason Gunthorpe , Robin Murphy , Kevin Tian , Alex Williamson , Shuah Khan , iommu@lists.linux.dev, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Pratyush Yadav , Pasha Tatashin , David Matlack , Andrew Morton , Pranjal Shrivastava , Vipin Sharma Subject: Re: [PATCH v4 08/18] iommu/vt-d: Clear unpreserved context entries during shutdown Message-ID: References: <20260808022723.3893618-1-skhawaja@google.com> <20260808022723.3893618-9-skhawaja@google.com> <446684d8-44d0-47c5-9301-172c55efd950@linux.intel.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <446684d8-44d0-47c5-9301-172c55efd950@linux.intel.com> On Wed, Aug 26, 2026 at 04:05:51PM +0800, Baolu Lu wrote: >On 8/8/26 10:27, Samiullah Khawaja wrote: >>During normal shutdown the iommu translation is disabled. Since the root >>table is preserved during live update, it needs to be cleaned up and the >>context entries of the unpreserved devices and root entries for the >>unpreserved context tables need to be cleared. > >The key assumption here seems to be that, during a live-update kexec, >most devices do not go through the normal iommu release path. Otherwise, >their context entries should already be torn down in the >iommu_release_device path. Also there is another part that the root table entries of unpreserved context tables also need to be removed. So even if the devices went through the release path, the context tables used by those devices are not removed by the Intel IOMMU driver. > >Could you please confirm this assumption and add some short note in the >comments or commit message? Yes, I can add a short note in the comment here and also in the commit message. > >> >>Signed-off-by: Samiullah Khawaja >>--- >> drivers/iommu/intel/iommu.c | 15 +++- >> drivers/iommu/intel/iommu.h | 5 ++ >> drivers/iommu/intel/liveupdate.c | 140 +++++++++++++++++++++++++++++++ >> 3 files changed, 158 insertions(+), 2 deletions(-) >> >>+ [snip] >>+static void clear_unpreserved_context(struct device_domain_info *info, u8 bus, u8 devfn) >>+{ >>+ struct context_entry *context; >>+ >>+ /* >>+ * This cleanup is done during shutdown, so it should be fine to only >>+ * clear the entries here and issue one global invalidation later to >>+ * invalidate all cleared entries. >>+ * >>+ * Note that the device IOTLB invalidation for unpreserved devices is >>+ * skipped this way, but that should not be needed as the devices are >>+ * quiesced at this point. This should improve the performance of the >>+ * cleanup process and avoids any invalidation timeouts because drivers >>+ * might have moved devices to D3 state. >>+ */ >>+ context = iommu_context_addr(info->iommu, bus, devfn, 0); >>+ if (context) { >>+ context_clear_entry(context); >>+ __iommu_flush_cache(info->iommu, context, sizeof(*context)); > >For tearing down a present context entry, please follow the VT-d >recommended sequence: > >- clear only the Present bit, >- flush the updated entry to memory if necessary, >- issue the required cache invalidations, Do we still need the individual cache invalidations if we issue global invalidations (context, pasid-cache and iotlb), after clearing all entries, as they would be done if a new root table was being setup? Looking at the VT-d specs (Invalidation of Translation Caches), each invalidation type (cache, pasid and iotlb) defines granularity in both register and queue based interface. And the granularity indicates that Global invalidations clear the cached entries for that specific type. For example following text is used for each cache type (in queue interface): Context-cache: Global Invalidation (01b): All context-cache entries cached at the remapping hardware are invalidated. Pasid-cache: Global Invalidation (11b): All PASID-cache entries are invalidated. Iotlb: Global Invalidation (01b): - All IOTLB entries are invalidated. - All paging-structure-cache entries are invalidated. A similar note about using Global invalidation is suggested in the specs when setting up root table (Set Root Table Pointer Operation). ... software must perform a global invalidate of the contextcache, PASID-cache (if applicable), and IOTLB, in that order. This is required to ensure hardware references only the remapping structures referenced by the new root table pointer and not stale cached entries. Also please note that this is happening during dmar unit teardown and system shutdown, and while the context table entries in root table are being cleared, the memory is not freed until the global invalidation is issued. Since this is during shutdown, issuing global invalidations instead of multiple individual invalidations for devices and aliases is simpler and would likely also have shutdown time improvements and reduce the blackout time during liveupdate. Please let me know if my global invalidations and granularity understanding is not correct. I added a comment at the top of this function to explain this, let me know if you want me to expand it with more details. >- then clear the remaining fields of the entry. > >>+ } >>+} >>+ >>+static int clear_unpreserved_alias_cb(struct pci_dev *pdev, u16 alias, void *data) >>+{ >>+ struct device_domain_info *info = data; >>+ >>+ clear_unpreserved_context(info, PCI_BUS_NUM(alias), alias & 0xff); >>+ return 0; >>+} >>+ >>+static int clear_unpreserve_context_entry_fn(struct device *dev, >>+ struct iommu_device *iommu_dev, >>+ void *arg) >>+{ >>+ struct device_domain_info *info; >>+ struct context_entry *context; >>+ >>+ info = dev_iommu_priv_get(dev); >>+ if (!info) >>+ return 0; >>+ >>+ if (!dev_is_pci(dev) || !dev_iommu_preserved_state(dev)) >>+ goto out_unpreserved; >>+ >>+ /* >>+ * PRE use cases are not supported with Live Update and a preservation >>+ * attempt on such domains returns an error. But Intel IOMMU driver >>+ * enables PRE by default on all devices that support it. For preserved >>+ * entries, the PRE needs to be disabled so preserved PCI devices do not >>+ * generate PRQs, during kexec, as translations are kept enabled during >>+ * live update. There is no need to disable these for DMA aliases. >>+ */ >>+ if (sm_supported(info->iommu)) { >>+ context = iommu_context_addr(info->iommu, info->bus, info->devfn, 0); > >Nit: please add a brief comment explaining why locking is not needed at >this call site. Will do in next revision. > >>+ if (context) { >>+ context_clear_sm_pre(context); > >For cache invalidation considerations when changing the PRE bit in a >present context entry, please follow the VT-d spec guidance (Table 28, >“Guidance to Software for Invalidations”). Please see note above the regarding global invalidations. > >>+ __iommu_flush_cache(info->iommu, context, sizeof(*context)); >> + }> + } >>+ >>+ return 0; >>+ >>+out_unpreserved: >>+ if (dev_is_pci(dev)) >>+ pci_for_each_dma_alias(to_pci_dev(dev), >>+ clear_unpreserved_alias_cb, info); >>+ else >>+ clear_unpreserved_context(info, info->bus, info->devfn); >>+ >>+ return 0; >>+} >>+ >>+/** >>+ * clear_unpreserved_context_entries() - Clear context entries for unpreserved devices >>+ * @iommu: Target IOMMU >>+ * >>+ * Clear the context entries of unpreserved devices during shutdown before kexec. >>+ */ >>+void clear_unpreserved_context_entries(struct intel_iommu *iommu) >>+{ >>+ struct iommu_dev_iter iter = { >>+ .fn = clear_unpreserve_context_entry_fn, >>+ .iommu = &iommu->iommu, >>+ .arg = NULL, >>+ >>+ }; >>+ >>+ /* >>+ * Clear context entries for unpreserved devices. >>+ * >>+ * Note that the error can be ignored as the iterator function does not >>+ * fail. >>+ */ >>+ iommu_for_each_dev(&iter); >>+ >>+ /* Clear reference to unpreserved context tables */ >>+ clear_unpreserved_context_root_entries(iommu, >>+ iommu_preserved_state(&iommu->iommu)); >>+ >>+ /* >>+ * Some devices might not have teardown/detached properly depending on >>+ * whether a proper device remove is done before kexec is triggered. >>+ * Also unpreserved context tables and entries are removed during >>+ * shutdown. So issue global invalidations to remove references to >>+ * unpreserved tables and entries. >>+ */ >>+ iommu->flush.flush_context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL); >>+ if (sm_supported(iommu)) >>+ qi_flush_pasid_cache(iommu, 0, QI_PC_GLOBAL, 0); >>+ iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH); Global invalidations are issued here after doing all the clear work. >>+} >>+ >> static void unpreserve_iommu_context_tables(struct intel_iommu *iommu, >> struct iommu_hw_ser *ser) >> { > >Thanks, >baolu Thanks for looking at this. Sami