linux-kernel - Re: [PATCH 1/2] Revert "iommu/amd: Fix performance counter initialization"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 13 Apr 2021 20:36:11 +0700
From:   "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
To:     Shuah Khan <skhan@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org
Cc:     joro@...tes.org, will@...nel.org, jsnitsel@...hat.com,
        pmenzel@...gen.mpg.de, Jon.Grimm@....com,
        Tj <ml.linux@...oe.vision>,
        Alexander Monakov <amonakov@...ras.ru>,
        David Coe <david.coe@...e.co.uk>
Subject: Re: [PATCH 1/2] Revert "iommu/amd: Fix performance counter
 initialization"

Shuah,

On 4/10/2021 12:06 AM, Shuah Khan wrote:
>> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
>> index 321f5906e6ed..648cdfd03074 100644
>> --- a/drivers/iommu/amd/init.c
>> +++ b/drivers/iommu/amd/init.c
>> ....
>> @@ -1729,39 +1728,17 @@ static void __init init_iommu_perf_ctr(struct amd_iommu *iommu)
>>       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) ||
>> -        iommu_pc_get_set_reg(iommu, 0, 0, 8, &save_src, false))
>> -        goto pc_false;
>> -
>> -    /*
>> -     * Disable power gating by programing the performance counter
>> -     * source to 20 (i.e. counts the reads and writes from/to IOMMU
>> -     * Reserved Register [MMIO Offset 1FF8h] that are ignored.),
>> -     * which never get incremented during this init phase.
>> -     * (Note: The event is also deprecated.)
>> -     */
>> -    val = 20;
>> -    if (iommu_pc_get_set_reg(iommu, 0, 0, 8, &val, true))
>> +    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 */
>> -    val = 0xabcd;
>> -    for (retry = 5; retry; retry--) {
>> -        if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &val, true) ||
>> -            iommu_pc_get_set_reg(iommu, 0, 0, 0, &val2, false) ||
>> -            val2)
>> -            break;
>> -
>> -        /* Wait about 20 msec for power gating to disable and retry. */
>> -        msleep(20);
>> -    }
>> -
>> -    /* restore */
>> -    if (iommu_pc_get_set_reg(iommu, 0, 0, 0, &save_reg, true) ||
>> -        iommu_pc_get_set_reg(iommu, 0, 0, 8, &save_src, true))
>> +    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))
> 
> Probably don't need parentheses around 'val != val2'

This is the result from git revert. Also, the logic is removed in patch 2/2.

Suravee