linux-kernel - Re: [PATCH] fpga fr br: fix warning for unexpected version number

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]

Message-ID: <alpine.DEB.2.20.1704051521090.2392@mgerlach-VirtualBox>
Date:   Wed, 5 Apr 2017 15:25:24 -0700 (PDT)
From:   matthew.gerlach@...ux.intel.com
To:     Moritz Fischer <mdf@...nel.org>
cc:     Alan Tull <atull@...nel.org>, linux-fpga@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] fpga fr br: fix warning for unexpected version number



On Wed, 5 Apr 2017, Moritz Fischer wrote:

> Hi Matthew,
>

Hi Moritz,


> On Wed, Apr 5, 2017 at 12:05 PM,  <matthew.gerlach@...ux.intel.com> wrote:
>> From: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
>>
>> The value in the version register of the altera freeze bridge
>> controller changed from the beta value of 2 to the
>> value of 0xad000003 in the official release of the IP.
>> This patch supports the old and new version numbers
>> without printing an warning.
>>
>> Signed-off-by: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
>> ---
>>  drivers/fpga/altera-freeze-bridge.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c
>> index 8dcd9fb..bdfd5eb 100644
>> --- a/drivers/fpga/altera-freeze-bridge.c
>> +++ b/drivers/fpga/altera-freeze-bridge.c
>> @@ -28,6 +28,7 @@
>>  #define FREEZE_CSR_REG_VERSION                 12
>>
>>  #define FREEZE_CSR_SUPPORTED_VERSION           2
>> +#define FREEZE_CSR_OFFICIAL_VERSION            0xad000003
>>
>>  #define FREEZE_CSR_STATUS_FREEZE_REQ_DONE      BIT(0)
>>  #define FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE    BIT(1)
>> @@ -241,10 +242,12 @@ static int altera_freeze_br_probe(struct platform_device *pdev)
>>                 priv->enable = 1;
>>
>>         revision = readl(priv->base_addr + FREEZE_CSR_REG_VERSION);
>> -       if (revision != FREEZE_CSR_SUPPORTED_VERSION)
>> +       if ((revision != FREEZE_CSR_SUPPORTED_VERSION) &&
>> +           (revision != FREEZE_CSR_OFFICIAL_VERSION))
>>                 dev_warn(dev,
>> -                        "%s Freeze Controller unexpected revision %d != %d\n",
>> -                        __func__, revision, FREEZE_CSR_SUPPORTED_VERSION);
>> +                        "%s unexpected revision 0x%x != 0x%x != 0x%x\n",
>> +                        __func__, revision, FREEZE_CSR_SUPPORTED_VERSION,
>> +                        FREEZE_CSR_OFFICIAL_VERSION);
>
> Maybe you actually wanna bail out if you read a random other value
> instead of what you
> expect instead of printing a warning.

I thought about making it an error if the version didn't match, but it was 
a dev_warn() before and that allowed folks to use the "old" driver with 
the new IP version.

>>
>>         return fpga_bridge_register(dev, FREEZE_BRIDGE_NAME,
>>                                     &altera_freeze_br_br_ops, priv);
>> --
>> 2.7.4
>>
>
> Cheers,
>
> Moritz
>