All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Monakov <amonakov@ispras.ru>
To: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: linux-kernel@vger.kernel.org, Joerg Roedel <joro@8bytes.org>,
	iommu@lists.linux-foundation.org
Subject: Re: [PATCH] iommu/amd: Fix event counter availability check
Date: Mon, 15 Jun 2020 23:48:52 +0300 (MSK)	[thread overview]
Message-ID: <alpine.LNX.2.20.13.2006152346320.21123@monopod.intra.ispras.ru> (raw)
In-Reply-To: <dba1e37a-1ed7-ef7f-7252-2ebd1d6bde8c@amd.com>

On Mon, 1 Jun 2020, Suravee Suthikulpanit wrote:

> Alexander
> 
> On 6/1/20 4:01 PM, Alexander Monakov wrote:
> > On Mon, 1 Jun 2020, Suravee Suthikulpanit wrote:
> > 
> > > > Moving init_iommu_perf_ctr just after iommu_flush_all_caches resolves
> > > > the issue. This is the earliest point in amd_iommu_init_pci where the
> > > > call succeeds on my laptop.
> > > 
> > > According to your description, it should just need to be anywhere after
> > > the
> > > pci_enable_device() is called for the IOMMU device, isn't it? So, on your
> > > system, what if we just move the init_iommu_perf_ctr() here:
> > 
> > No, this doesn't work, as I already said in the paragraph you are responding
> > to. See my last sentence in the quoted part.
> > 
> > So the implication is init_device_table_dma together with subsequent cache
> > flush is also setting up something that is necessary for counters to be
> > writable.
> > 
> > Alexander
> > 
> 
> Instead of blindly moving the code around to a spot that would just work,
> I am trying to understand what might be required here. In this case,
> the init_device_table_dma()should not be needed. I suspect it's the IOMMU
> invalidate all command that's also needed here.
> 
> I'm also checking with the HW and BIOS team. Meanwhile, could you please give
> the following change a try:

Hello. Can you give any update please?

Alexander

> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 5b81fd16f5fa..b07458cc1b0b 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1875,6 +1875,8 @@ static int __init amd_iommu_init_pci(void)
>                 ret = iommu_init_pci(iommu);
>                 if (ret)
>                         break;
> +               iommu_flush_all_caches(iommu);
> +               init_iommu_perf_ctr(iommu);
>         }
> 
>         /*
> --
> 2.17.1
> 
> Thanks,
> Suravee
> 

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Monakov <amonakov@ispras.ru>
To: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org
Subject: Re: [PATCH] iommu/amd: Fix event counter availability check
Date: Mon, 15 Jun 2020 23:48:52 +0300 (MSK)	[thread overview]
Message-ID: <alpine.LNX.2.20.13.2006152346320.21123@monopod.intra.ispras.ru> (raw)
In-Reply-To: <dba1e37a-1ed7-ef7f-7252-2ebd1d6bde8c@amd.com>

On Mon, 1 Jun 2020, Suravee Suthikulpanit wrote:

> Alexander
> 
> On 6/1/20 4:01 PM, Alexander Monakov wrote:
> > On Mon, 1 Jun 2020, Suravee Suthikulpanit wrote:
> > 
> > > > Moving init_iommu_perf_ctr just after iommu_flush_all_caches resolves
> > > > the issue. This is the earliest point in amd_iommu_init_pci where the
> > > > call succeeds on my laptop.
> > > 
> > > According to your description, it should just need to be anywhere after
> > > the
> > > pci_enable_device() is called for the IOMMU device, isn't it? So, on your
> > > system, what if we just move the init_iommu_perf_ctr() here:
> > 
> > No, this doesn't work, as I already said in the paragraph you are responding
> > to. See my last sentence in the quoted part.
> > 
> > So the implication is init_device_table_dma together with subsequent cache
> > flush is also setting up something that is necessary for counters to be
> > writable.
> > 
> > Alexander
> > 
> 
> Instead of blindly moving the code around to a spot that would just work,
> I am trying to understand what might be required here. In this case,
> the init_device_table_dma()should not be needed. I suspect it's the IOMMU
> invalidate all command that's also needed here.
> 
> I'm also checking with the HW and BIOS team. Meanwhile, could you please give
> the following change a try:

Hello. Can you give any update please?

Alexander

> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 5b81fd16f5fa..b07458cc1b0b 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1875,6 +1875,8 @@ static int __init amd_iommu_init_pci(void)
>                 ret = iommu_init_pci(iommu);
>                 if (ret)
>                         break;
> +               iommu_flush_all_caches(iommu);
> +               init_iommu_perf_ctr(iommu);
>         }
> 
>         /*
> --
> 2.17.1
> 
> Thanks,
> Suravee
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  parent reply	other threads:[~2020-06-15 20:49 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-29 20:07 [PATCH] iommu/amd: Fix event counter availability check Alexander Monakov
2020-05-29 20:07 ` Alexander Monakov
2020-05-31  7:22 ` Alexander Monakov
2020-05-31  7:22   ` Alexander Monakov
2020-06-01  2:48   ` Paul Menzel
2020-06-01  2:48     ` Paul Menzel
2021-02-21 13:44     ` Paul Menzel
2021-02-21 13:44       ` Paul Menzel
2020-06-02 23:51   ` Shuah Khan
2020-06-02 23:51     ` Shuah Khan
2020-06-03  6:54     ` Alexander Monakov
2020-06-03  6:54       ` Alexander Monakov
2021-02-26 21:44       ` Paul Menzel
2021-02-26 21:44         ` Paul Menzel
2021-02-26 21:55         ` Shuah Khan
2021-02-26 21:55           ` Shuah Khan
2020-06-01  7:37 ` Suravee Suthikulpanit
2020-06-01  7:37   ` Suravee Suthikulpanit
2020-06-01  9:01   ` Alexander Monakov
2020-06-01  9:01     ` Alexander Monakov
2020-06-01 15:10     ` Suravee Suthikulpanit
2020-06-01 15:10       ` Suravee Suthikulpanit
2020-06-01 16:09       ` Alexander Monakov
2020-06-01 16:09         ` Alexander Monakov
2020-06-15 20:48       ` Alexander Monakov [this message]
2020-06-15 20:48         ` Alexander Monakov
2020-06-16  9:35         ` Suravee Suthikulpanit
2020-06-16  9:35           ` Suravee Suthikulpanit
2020-06-30 19:22           ` Alexander Monakov
2020-06-30 19:22             ` Alexander Monakov
2020-09-17 17:55           ` Alexander Monakov
2020-09-17 17:55             ` Alexander Monakov
2021-02-21 13:49             ` Paul Menzel
2021-02-21 13:49               ` Paul Menzel
2021-02-22 17:59               ` Suravee Suthikulpanit
2021-02-22 17:59                 ` Suravee Suthikulpanit
2021-02-24 20:35                 ` Paul Menzel
2021-02-24 20:35                   ` Paul Menzel

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=alpine.LNX.2.20.13.2006152346320.21123@monopod.intra.ispras.ru \
    --to=amonakov@ispras.ru \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=suravee.suthikulpanit@amd.com \
    /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.