RE: [PATCH v2 03/12] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()
> From: Baolu Lu
> Sent: Wednesday, June 15, 2022 9:10 PM
>
> On 2022/6/15 14:22, Tian, Kevin wrote:
> >> From: Baolu Lu
> >> Sent: Tuesday, June 14, 2022 3:21 PM
> >>
> >> On 2022/6/14 14:49, Tian, Kevin wrote:
> From: Lu Baolu
> Sent: Tuesday, June 14, 2022 10:51 AM
>
> The disable_dmar_iommu() is called when IOMMU initialization fails or
> the IOMMU is hot-removed from the system. In both cases, there is no
> need to clear the IOMMU translation data structures for devices.
>
> On the initialization path, the device probing only happens after the
> IOMMU is initialized successfully, hence there're no translation data
> structures.
> >>> Out of curiosity. With kexec the IOMMU may contain stale mappings
> >>> from the old kernel. Then is it meaningful to disable IOMMU after the
> >>> new kernel fails to initialize it properly?
> >>
> >> For kexec kernel, if the IOMMU is detected to be pre-enabled, the IOMMU
> >> driver will try to copy tables from the old kernel. If copying table
> >> fails, the IOMMU driver will disable IOMMU and do the normal
> >> initialization.
> >>
> >
> > What about an error occurred after copying table in the initialization
> > path? The new kernel will be in a state assuming iommu is disabled
> > but it is still enabled using an old mapping for certain devices...
> >
>
> If copying table failed, the translation will be disabled and a clean
> root table will be used.
>
> if (translation_pre_enabled(iommu)) {
> pr_info("Translation already enabled - trying to copy
> translation structures\n");
>
> ret = copy_translation_tables(iommu);
> if (ret) {
> /*
> * We found the IOMMU with translation
> * enabled - but failed to copy over the
> * old root-entry table. Try to proceed
> * by disabling translation now and
> * allocating a clean root-entry table.
> * This might cause DMAR faults, but
> * probably the dump will still succeed.
> */
> pr_err("Failed to copy translation tables from previous
> kernel for %s\n",
> iommu->name);
> iommu_disable_translation(iommu);
> clear_translation_pre_enabled(iommu);
> } else {
> pr_info("Copied translation tables from previous kernel
> for %s\n",
> iommu->name);
> }
> }
>
I meant copying table succeeds but another error occurs in the
remaining path of initialization...
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 03/12] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()
On 2022/6/15 14:22, Tian, Kevin wrote:
From: Baolu Lu
Sent: Tuesday, June 14, 2022 3:21 PM
On 2022/6/14 14:49, Tian, Kevin wrote:
From: Lu Baolu
Sent: Tuesday, June 14, 2022 10:51 AM
The disable_dmar_iommu() is called when IOMMU initialization fails or
the IOMMU is hot-removed from the system. In both cases, there is no
need to clear the IOMMU translation data structures for devices.
On the initialization path, the device probing only happens after the
IOMMU is initialized successfully, hence there're no translation data
structures.
Out of curiosity. With kexec the IOMMU may contain stale mappings
from the old kernel. Then is it meaningful to disable IOMMU after the
new kernel fails to initialize it properly?
For kexec kernel, if the IOMMU is detected to be pre-enabled, the IOMMU
driver will try to copy tables from the old kernel. If copying table
fails, the IOMMU driver will disable IOMMU and do the normal
initialization.
What about an error occurred after copying table in the initialization
path? The new kernel will be in a state assuming iommu is disabled
but it is still enabled using an old mapping for certain devices...
If copying table failed, the translation will be disabled and a clean
root table will be used.
if (translation_pre_enabled(iommu)) {
pr_info("Translation already enabled - trying to copy
translation structures\n");
ret = copy_translation_tables(iommu);
if (ret) {
/*
* We found the IOMMU with translation
* enabled - but failed to copy over the
* old root-entry table. Try to proceed
* by disabling translation now and
* allocating a clean root-entry table.
* This might cause DMAR faults, but
* probably the dump will still succeed.
*/
pr_err("Failed to copy translation tables from previous
kernel for %s\n",
iommu->name);
iommu_disable_translation(iommu);
clear_translation_pre_enabled(iommu);
} else {
pr_info("Copied translation tables from previous kernel
for %s\n",
iommu->name);
}
}
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2 03/12] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()
> From: Baolu Lu > Sent: Tuesday, June 14, 2022 3:21 PM > > On 2022/6/14 14:49, Tian, Kevin wrote: > >> From: Lu Baolu > >> Sent: Tuesday, June 14, 2022 10:51 AM > >> > >> The disable_dmar_iommu() is called when IOMMU initialization fails or > >> the IOMMU is hot-removed from the system. In both cases, there is no > >> need to clear the IOMMU translation data structures for devices. > >> > >> On the initialization path, the device probing only happens after the > >> IOMMU is initialized successfully, hence there're no translation data > >> structures. > > Out of curiosity. With kexec the IOMMU may contain stale mappings > > from the old kernel. Then is it meaningful to disable IOMMU after the > > new kernel fails to initialize it properly? > > For kexec kernel, if the IOMMU is detected to be pre-enabled, the IOMMU > driver will try to copy tables from the old kernel. If copying table > fails, the IOMMU driver will disable IOMMU and do the normal > initialization. > What about an error occurred after copying table in the initialization path? The new kernel will be in a state assuming iommu is disabled but it is still enabled using an old mapping for certain devices... ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 03/12] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()
On 2022/6/14 14:49, Tian, Kevin wrote: From: Lu Baolu Sent: Tuesday, June 14, 2022 10:51 AM The disable_dmar_iommu() is called when IOMMU initialization fails or the IOMMU is hot-removed from the system. In both cases, there is no need to clear the IOMMU translation data structures for devices. On the initialization path, the device probing only happens after the IOMMU is initialized successfully, hence there're no translation data structures. Out of curiosity. With kexec the IOMMU may contain stale mappings from the old kernel. Then is it meaningful to disable IOMMU after the new kernel fails to initialize it properly? For kexec kernel, if the IOMMU is detected to be pre-enabled, the IOMMU driver will try to copy tables from the old kernel. If copying table fails, the IOMMU driver will disable IOMMU and do the normal initialization. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2 03/12] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()
> From: Lu Baolu
> Sent: Tuesday, June 14, 2022 10:51 AM
>
> The disable_dmar_iommu() is called when IOMMU initialization fails or
> the IOMMU is hot-removed from the system. In both cases, there is no
> need to clear the IOMMU translation data structures for devices.
>
> On the initialization path, the device probing only happens after the
> IOMMU is initialized successfully, hence there're no translation data
> structures.
Out of curiosity. With kexec the IOMMU may contain stale mappings
from the old kernel. Then is it meaningful to disable IOMMU after the
new kernel fails to initialize it properly?
>
> On the hot-remove path, there is no real use case where the IOMMU is
> hot-removed, but the devices that it manages are still alive in the
> system. The translation data structures were torn down during device
> release, hence there's no need to repeat it in IOMMU hot-remove path
> either. This removes the unnecessary code and only leaves a check.
>
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/intel/pasid.h | 1 +
> drivers/iommu/intel/iommu.c | 21 +++--
> 2 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
> index 583ea67fc783..2afbb2afe8cc 100644
> --- a/drivers/iommu/intel/pasid.h
> +++ b/drivers/iommu/intel/pasid.h
> @@ -39,6 +39,7 @@
> * only and pass-through transfer modes.
> */
> #define FLPT_DEFAULT_DID 1
> +#define NUM_RESERVED_DID 2
>
> /*
> * The SUPERVISOR_MODE flag indicates a first level translation which
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index ff6018f746e0..695aed474e5d 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1715,23 +1715,16 @@ static int iommu_init_domains(struct
> intel_iommu *iommu)
>
> static void disable_dmar_iommu(struct intel_iommu *iommu)
> {
> - struct device_domain_info *info, *tmp;
> - unsigned long flags;
> -
> if (!iommu->domain_ids)
> return;
>
> - spin_lock_irqsave(_domain_lock, flags);
> - list_for_each_entry_safe(info, tmp, _domain_list, global) {
> - if (info->iommu != iommu)
> - continue;
> -
> - if (!info->dev || !info->domain)
> - continue;
> -
> - __dmar_remove_one_dev_info(info);
> - }
> - spin_unlock_irqrestore(_domain_lock, flags);
> + /*
> + * All iommu domains must have been detached from the devices,
> + * hence there should be no domain IDs in use.
> + */
> + if (WARN_ON(bitmap_weight(iommu->domain_ids,
> cap_ndoms(iommu->cap))
> + != NUM_RESERVED_DID))
> + return;
>
> if (iommu->gcmd & DMA_GCMD_TE)
> iommu_disable_translation(iommu);
> --
> 2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
