* [PATCH] iommu: amd: Fix IOMMU perf counter clobbering during init
@ 2020-01-14 15:12 Shuah Khan
2020-01-17 10:08 ` Joerg Roedel
0 siblings, 1 reply; 5+ messages in thread
From: Shuah Khan @ 2020-01-14 15:12 UTC (permalink / raw)
To: joro; +Cc: Shuah Khan, iommu, linux-kernel
init_iommu_perf_ctr() clobbers the register when it checks write access
to IOMMU perf counters and fails to restore when they are writable.
Add save and restore to fix it.
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
drivers/iommu/amd_iommu_init.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 568c52317757..c0ad4f293522 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1655,27 +1655,37 @@ static int iommu_pc_get_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
static void init_iommu_perf_ctr(struct amd_iommu *iommu)
{
struct pci_dev *pdev = iommu->dev;
- u64 val = 0xabcd, val2 = 0;
+ u64 val = 0xabcd, val2 = 0, save_reg = 0;
if (!iommu_feature(iommu, FEATURE_PC))
return;
amd_iommu_pc_present = true;
+ /* save the value to restore, if writable */
+ if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &save_reg, false))
+ goto pc_false;
+
/* Check if the performance counters can be written to */
if ((iommu_pc_get_set_reg(iommu, 0, 0, 0, &val, true)) ||
(iommu_pc_get_set_reg(iommu, 0, 0, 0, &val2, false)) ||
- (val != val2)) {
- pci_err(pdev, "Unable to write to IOMMU perf counter.\n");
- amd_iommu_pc_present = false;
- return;
- }
+ (val != val2))
+ goto pc_false;
+
+ /* restore */
+ if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &save_reg, true))
+ goto pc_false;
pci_info(pdev, "IOMMU performance counters supported\n");
val = readl(iommu->mmio_base + MMIO_CNTR_CONF_OFFSET);
iommu->max_banks = (u8) ((val >> 12) & 0x3f);
iommu->max_counters = (u8) ((val >> 7) & 0xf);
+
+pc_false:
+ pci_err(pdev, "Unable to read/write to IOMMU perf counter.\n");
+ amd_iommu_pc_present = false;
+ return;
}
static ssize_t amd_iommu_show_cap(struct device *dev,
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu: amd: Fix IOMMU perf counter clobbering during init
2020-01-14 15:12 [PATCH] iommu: amd: Fix IOMMU perf counter clobbering during init Shuah Khan
@ 2020-01-17 10:08 ` Joerg Roedel
2020-01-21 2:10 ` Suravee Suthikulpanit
0 siblings, 1 reply; 5+ messages in thread
From: Joerg Roedel @ 2020-01-17 10:08 UTC (permalink / raw)
To: Shuah Khan, Suravee Suthikulpanit; +Cc: iommu, linux-kernel
Adding Suravee, who wrote the IOMMU Perf Counter code.
On Tue, Jan 14, 2020 at 08:12:20AM -0700, Shuah Khan wrote:
> init_iommu_perf_ctr() clobbers the register when it checks write access
> to IOMMU perf counters and fails to restore when they are writable.
>
> Add save and restore to fix it.
>
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> ---
> drivers/iommu/amd_iommu_init.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
Suravee, can you please review this patch?
>
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 568c52317757..c0ad4f293522 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1655,27 +1655,37 @@ static int iommu_pc_get_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
> static void init_iommu_perf_ctr(struct amd_iommu *iommu)
> {
> struct pci_dev *pdev = iommu->dev;
> - u64 val = 0xabcd, val2 = 0;
> + u64 val = 0xabcd, val2 = 0, save_reg = 0;
>
> if (!iommu_feature(iommu, FEATURE_PC))
> return;
>
> amd_iommu_pc_present = true;
>
> + /* save the value to restore, if writable */
> + if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &save_reg, false))
> + goto pc_false;
> +
> /* Check if the performance counters can be written to */
> if ((iommu_pc_get_set_reg(iommu, 0, 0, 0, &val, true)) ||
> (iommu_pc_get_set_reg(iommu, 0, 0, 0, &val2, false)) ||
> - (val != val2)) {
> - pci_err(pdev, "Unable to write to IOMMU perf counter.\n");
> - amd_iommu_pc_present = false;
> - return;
> - }
> + (val != val2))
> + goto pc_false;
> +
> + /* restore */
> + if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &save_reg, true))
> + goto pc_false;
>
> pci_info(pdev, "IOMMU performance counters supported\n");
>
> val = readl(iommu->mmio_base + MMIO_CNTR_CONF_OFFSET);
> iommu->max_banks = (u8) ((val >> 12) & 0x3f);
> iommu->max_counters = (u8) ((val >> 7) & 0xf);
> +
> +pc_false:
> + pci_err(pdev, "Unable to read/write to IOMMU perf counter.\n");
> + amd_iommu_pc_present = false;
> + return;
> }
>
> static ssize_t amd_iommu_show_cap(struct device *dev,
> --
> 2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu: amd: Fix IOMMU perf counter clobbering during init
2020-01-17 10:08 ` Joerg Roedel
@ 2020-01-21 2:10 ` Suravee Suthikulpanit
2020-01-21 15:32 ` Shuah Khan
0 siblings, 1 reply; 5+ messages in thread
From: Suravee Suthikulpanit @ 2020-01-21 2:10 UTC (permalink / raw)
To: Joerg Roedel, Shuah Khan; +Cc: iommu, linux-kernel
On 1/17/2020 5:08 PM, Joerg Roedel wrote:
> Adding Suravee, who wrote the IOMMU Perf Counter code.
>
> On Tue, Jan 14, 2020 at 08:12:20AM -0700, Shuah Khan wrote:
>> init_iommu_perf_ctr() clobbers the register when it checks write access
>> to IOMMU perf counters and fails to restore when they are writable.
>>
>> Add save and restore to fix it.
>>
>> Signed-off-by: Shuah Khan<skhan@linuxfoundation.org>
>> ---
>> drivers/iommu/amd_iommu_init.c | 22 ++++++++++++++++------
>> 1 file changed, 16 insertions(+), 6 deletions(-)
> Suravee, can you please review this patch?
>
This looks ok. Does this fix certain issues? Or is this just for sanity.
Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Thanks,
Suravee
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu: amd: Fix IOMMU perf counter clobbering during init
2020-01-21 2:10 ` Suravee Suthikulpanit
@ 2020-01-21 15:32 ` Shuah Khan
2020-01-23 21:27 ` Shuah Khan
0 siblings, 1 reply; 5+ messages in thread
From: Shuah Khan @ 2020-01-21 15:32 UTC (permalink / raw)
To: Suravee Suthikulpanit, Joerg Roedel; +Cc: iommu, linux-kernel, Shuah Khan
On 1/20/20 7:10 PM, Suravee Suthikulpanit wrote:
> On 1/17/2020 5:08 PM, Joerg Roedel wrote:
>> Adding Suravee, who wrote the IOMMU Perf Counter code.
>>
>> On Tue, Jan 14, 2020 at 08:12:20AM -0700, Shuah Khan wrote:
>>> init_iommu_perf_ctr() clobbers the register when it checks write access
>>> to IOMMU perf counters and fails to restore when they are writable.
>>>
>>> Add save and restore to fix it.
>>>
>>> Signed-off-by: Shuah Khan<skhan@linuxfoundation.org>
>>> ---
>>> drivers/iommu/amd_iommu_init.c | 22 ++++++++++++++++------
>>> 1 file changed, 16 insertions(+), 6 deletions(-)
>> Suravee, can you please review this patch?
>>
>
> This looks ok. Does this fix certain issues? Or is this just for sanity.
I didn't notice any problems. Counters aren't writable on my system.
However, it certainly looks like a bog since registers aren't restored
like in other places in this file where such checks are done on other
registers.
I see 2 banks and 4 counters on my system. Is it sufficient to check
the first bank and first counter? In other words, if the first one
isn't writable, are all counters non-writable?
Should we read the config first and then, try to see if any of the
counters are writable? I have a patch that does that, I can send it
out for review.
>
> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Thanks for the review.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu: amd: Fix IOMMU perf counter clobbering during init
2020-01-21 15:32 ` Shuah Khan
@ 2020-01-23 21:27 ` Shuah Khan
0 siblings, 0 replies; 5+ messages in thread
From: Shuah Khan @ 2020-01-23 21:27 UTC (permalink / raw)
To: Suravee Suthikulpanit, Joerg Roedel; +Cc: iommu, linux-kernel, Shuah Khan
On 1/21/20 8:32 AM, Shuah Khan wrote:
> On 1/20/20 7:10 PM, Suravee Suthikulpanit wrote:
>> On 1/17/2020 5:08 PM, Joerg Roedel wrote:
>>> Adding Suravee, who wrote the IOMMU Perf Counter code.
>>>
>>> On Tue, Jan 14, 2020 at 08:12:20AM -0700, Shuah Khan wrote:
>>>> init_iommu_perf_ctr() clobbers the register when it checks write access
>>>> to IOMMU perf counters and fails to restore when they are writable.
>>>>
>>>> Add save and restore to fix it.
>>>>
>>>> Signed-off-by: Shuah Khan<skhan@linuxfoundation.org>
>>>> ---
>>>> drivers/iommu/amd_iommu_init.c | 22 ++++++++++++++++------
>>>> 1 file changed, 16 insertions(+), 6 deletions(-)
>>> Suravee, can you please review this patch?
>>>
>>
>> This looks ok. Does this fix certain issues? Or is this just for sanity.
>
> I didn't notice any problems. Counters aren't writable on my system.
> However, it certainly looks like a bog since registers aren't restored
> like in other places in this file where such checks are done on other
> registers.
>
> I see 2 banks and 4 counters on my system. Is it sufficient to check
> the first bank and first counter? In other words, if the first one
> isn't writable, are all counters non-writable?
>
> Should we read the config first and then, try to see if any of the
> counters are writable? I have a patch that does that, I can send it
> out for review.
>
>>
>> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Joerg,
Please don't pull this in. I introduced a bug in this patch. It always
returns amd_iommu_pc_present false even when it can write to the
counters. My bad.
I will send v2.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-01-23 21:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 15:12 [PATCH] iommu: amd: Fix IOMMU perf counter clobbering during init Shuah Khan
2020-01-17 10:08 ` Joerg Roedel
2020-01-21 2:10 ` Suravee Suthikulpanit
2020-01-21 15:32 ` Shuah Khan
2020-01-23 21:27 ` Shuah Khan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).