Re: [Patch v2 7/9] iommu/amd: copy old tables and do not update dev tables before driver init

[Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>]

On Fri, Nov 06, 2015 at 08:10:49PM +0800, Baoquan He wrote:
> Signed-off-by: Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Missing patch description.

> ---
>  drivers/iommu/amd_iommu.c      | 19 +++++------
>  drivers/iommu/amd_iommu_init.c | 71 ++++++++++++++++++++++++------------------
>  2 files changed, 49 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 48bcd83..90c6205 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -1992,14 +1992,15 @@ static void do_attach(struct iommu_dev_data *dev_data,
>  	/* Update data structures */
>  	dev_data->domain = domain;
>  	list_add(&dev_data->list, &domain->dev_list);
> -	set_dte_entry(dev_data->devid, domain, ats);
> +	if (!translation_pre_enabled()) {
> +		set_dte_entry(dev_data->devid, domain, ats);
> +		/* Flush the DTE entry */
> +		device_flush_dte(dev_data);
> +	}

Hmm, this patch adds a lot of special cases into the AMD IOMMU code to
make sure the domain is attached at driver init time.

Can we change the code to generally defer domain attachment to driver
init time? There is a set_dma_mask call-back in the dma-ops that can be
used for that.

This would limit the special cases to device table initialization and
iommu enable time.

> +	if ( !translation_pre_enabled()) {
> +		if (flags & ACPI_DEVFLAG_INITPASS)
> +			set_dev_entry_bit(devid, DEV_ENTRY_INIT_PASS);
> +		if (flags & ACPI_DEVFLAG_EXTINT)
> +			set_dev_entry_bit(devid, DEV_ENTRY_EINT_PASS);
> +		if (flags & ACPI_DEVFLAG_NMI)
> +			set_dev_entry_bit(devid, DEV_ENTRY_NMI_PASS);
> +		if (flags & ACPI_DEVFLAG_SYSMGT1)
> +			set_dev_entry_bit(devid, DEV_ENTRY_SYSMGT1);
> +		if (flags & ACPI_DEVFLAG_SYSMGT2)
> +			set_dev_entry_bit(devid, DEV_ENTRY_SYSMGT2);
> +		if (flags & ACPI_DEVFLAG_LINT0)
> +			set_dev_entry_bit(devid, DEV_ENTRY_LINT0_PASS);
> +		if (flags & ACPI_DEVFLAG_LINT1)
> +			set_dev_entry_bit(devid, DEV_ENTRY_LINT1_PASS);
> +
> +		amd_iommu_apply_erratum_63(devid);
> +	}

This bothers, will the flags still get set when translation is
pre-enabled?


	Joerg