From: Wan Zongshun <vw-6ukY98dZOFrYtjvyW6yDsg@public.gmane.org>
To: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Cc: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] iommu/amd: Fix unity mapping initialization race
Date: Sun, 10 Jul 2016 19:40:53 +0800 [thread overview]
Message-ID: <57823445.1020808@iommu.org> (raw)
In-Reply-To: <1467820838-5059-1-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
On 2016年07月07日 00:00, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> There is a race condition in the AMD IOMMU init code that
> causes requested unity mappings to be blocked by the IOMMU
> for a short period of time. This results on boot failures
> and IO_PAGE_FAULTs on some machines.
>
> Fix this by making sure the unity mappings are installed
> before all other DMA is blocked.
>
> Fixes: aafd8ba0ca74 ('iommu/amd: Implement add_device and remove_device')
> Cc: stable@vger.kernel.org # v4.2+
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> drivers/iommu/amd_iommu_init.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index d091def..59741ea 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1568,13 +1568,23 @@ static int __init amd_iommu_init_pci(void)
> break;
> }
>
> + /*
> + * Order is important here to make sure any unity map requirements are
> + * fulfilled. The unity mappings are created and written to the device
> + * table during the amd_iommu_init_api() call.
> + *
> + * After that we call init_device_table_dma() to make sure any
> + * uninitialized DTE will block DMA, and in the end we flush the caches
> + * of all IOMMUs to make sure the changes to the device table are
> + * active.
> + */
Joerg,
Do you mean we need enable the V and TV bits to DTE entry after all DTEs
tables were initialized completely?
I checked this function 'init_device_table_dma', and find it just set
V and TV bit, to set translation info valid and DTE bits127:1 valid.
So I just think all things it should to do are to allow DMA access,
GPA-to-SPA translation should be active, why you add function comments
below is to not allow DMA access and suppress all page faults?
/*
* Init the device table to not allow DMA access for devices and
* suppress all page faults
*/
static void init_device_table_dma(void)
> + ret = amd_iommu_init_api();
> +
> init_device_table_dma();
>
> for_each_iommu(iommu)
> iommu_flush_all_caches(iommu);
>
> - ret = amd_iommu_init_api();
> -
> if (!ret)
> print_iommu_info();
>
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
WARNING: multiple messages have this Message-ID (diff)
From: Wan Zongshun <vw@iommu.org>
To: Joerg Roedel <joro@8bytes.org>, iommu@lists.linux-foundation.org
Cc: Joerg Roedel <jroedel@suse.de>,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] iommu/amd: Fix unity mapping initialization race
Date: Sun, 10 Jul 2016 19:40:53 +0800 [thread overview]
Message-ID: <57823445.1020808@iommu.org> (raw)
In-Reply-To: <1467820838-5059-1-git-send-email-joro@8bytes.org>
On 2016年07月07日 00:00, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> There is a race condition in the AMD IOMMU init code that
> causes requested unity mappings to be blocked by the IOMMU
> for a short period of time. This results on boot failures
> and IO_PAGE_FAULTs on some machines.
>
> Fix this by making sure the unity mappings are installed
> before all other DMA is blocked.
>
> Fixes: aafd8ba0ca74 ('iommu/amd: Implement add_device and remove_device')
> Cc: stable@vger.kernel.org # v4.2+
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> drivers/iommu/amd_iommu_init.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index d091def..59741ea 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1568,13 +1568,23 @@ static int __init amd_iommu_init_pci(void)
> break;
> }
>
> + /*
> + * Order is important here to make sure any unity map requirements are
> + * fulfilled. The unity mappings are created and written to the device
> + * table during the amd_iommu_init_api() call.
> + *
> + * After that we call init_device_table_dma() to make sure any
> + * uninitialized DTE will block DMA, and in the end we flush the caches
> + * of all IOMMUs to make sure the changes to the device table are
> + * active.
> + */
Joerg,
Do you mean we need enable the V and TV bits to DTE entry after all DTEs
tables were initialized completely?
I checked this function 'init_device_table_dma', and find it just set
V and TV bit, to set translation info valid and DTE bits127:1 valid.
So I just think all things it should to do are to allow DMA access,
GPA-to-SPA translation should be active, why you add function comments
below is to not allow DMA access and suppress all page faults?
/*
* Init the device table to not allow DMA access for devices and
* suppress all page faults
*/
static void init_device_table_dma(void)
> + ret = amd_iommu_init_api();
> +
> init_device_table_dma();
>
> for_each_iommu(iommu)
> iommu_flush_all_caches(iommu);
>
> - ret = amd_iommu_init_api();
> -
> if (!ret)
> print_iommu_info();
>
>
next prev parent reply other threads:[~2016-07-10 11:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-06 16:00 [PATCH] iommu/amd: Fix unity mapping initialization race Joerg Roedel
[not found] ` <1467820838-5059-1-git-send-email-joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2016-07-10 11:40 ` Wan Zongshun [this message]
2016-07-10 11:40 ` Wan Zongshun
[not found] ` <57823445.1020808-6ukY98dZOFrYtjvyW6yDsg@public.gmane.org>
2016-07-11 7:19 ` Joerg Roedel
2016-07-11 7:19 ` Joerg Roedel
2016-07-11 9:25 ` Wan Zongshun
2016-07-11 9:39 ` Joerg Roedel
2016-07-11 10:05 ` Wan ZongShun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57823445.1020808@iommu.org \
--to=vw-6uky98dzofrytjvyw6ydsg@public.gmane.org \
--cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
--cc=joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org \
--cc=jroedel-l3A5Bk7waGM@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.