lkml.org 
[lkml]   [2020]   [Jun]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] iommu/amd: Use 'unsigned long' for domain->pt_root
On Thu, Jun 25, 2020 at 04:52:27PM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Using atomic64_t can be quite expensive, so use unsigned long instead.
> This is safe because the write becomes visible atomically.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> drivers/iommu/amd/amd_iommu_types.h | 2 +-
> drivers/iommu/amd/iommu.c | 10 ++++++++--
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 30a5d412255a..f6f102282dda 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -468,7 +468,7 @@ struct protection_domain {
> iommu core code */
> spinlock_t lock; /* mostly used to lock the page table*/
> u16 id; /* the domain id written to the device table */
> - atomic64_t pt_root; /* pgtable root and pgtable mode */
> + unsigned long pt_root; /* pgtable root and pgtable mode */
> int glx; /* Number of levels for GCR3 table */
> u64 *gcr3_tbl; /* Guest CR3 table */
> unsigned long flags; /* flags to find out type of domain */
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 5286ddcfc2f9..b0e1dc58244e 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -156,7 +156,7 @@ static struct protection_domain *to_pdomain(struct iommu_domain *dom)
> static void amd_iommu_domain_get_pgtable(struct protection_domain *domain,
> struct domain_pgtable *pgtable)
> {
> - u64 pt_root = atomic64_read(&domain->pt_root);
> + unsigned long pt_root = domain->pt_root;

The pt_root might be reload later in case of register pressure where the
compiler decides to not store it as a stack variable, so it needs
smp_rmb() here to match to the smp_wmb() in
amd_iommu_domain_set_pt_root() to make the load visiable to all CPUs.

Then, smp_rmb/wmb() wouldn't be able to deal with data races, so it
needs,

unsigned long pt_root = READ_ONCE(domain->pt_root);

>
> pgtable->root = (u64 *)(pt_root & PAGE_MASK);
> pgtable->mode = pt_root & 7; /* lowest 3 bits encode pgtable mode */
> @@ -164,7 +164,13 @@ static void amd_iommu_domain_get_pgtable(struct protection_domain *domain,
>
> static void amd_iommu_domain_set_pt_root(struct protection_domain *domain, u64 root)
> {
> - atomic64_set(&domain->pt_root, root);
> + domain->pt_root = root;

WRITE_ONCE(domain->pt_root, root);

> +
> + /*
> + * The new value needs to be gobally visible in case pt_root gets
> + * cleared, so that the page-table can be safely freed.
> + */
> + smp_wmb();
> }
>
> static void amd_iommu_domain_clr_pt_root(struct protection_domain *domain)
> --
> 2.27.0
>

\
 
 \ /
  Last update: 2020-06-25 17:38    [W:0.085 / U:0.880 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site