linux-kernel - Re: [PATCH 6/8] x86/platform/uv: Decode UVsystab Info

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: <d0675e8f-80a2-bba4-888c-90feda085a14@hpe.com>
Date:   Thu, 5 Sep 2019 09:43:57 -0700
From:   Mike Travis <mike.travis@....com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Borislav Petkov <bp@...en8.de>,
        Christoph Hellwig <hch@...radead.org>,
        Dimitri Sivanich <dimitri.sivanich@....com>,
        Russ Anderson <russ.anderson@....com>,
        Hedi Berriche <hedi.berriche@....com>,
        Steve Wahl <steve.wahl@....com>, x86@...nel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 6/8] x86/platform/uv: Decode UVsystab Info



On 9/5/2019 7:47 AM, Mike Travis wrote:
> Also, nit:
> 
>> --- linux.orig/arch/x86/kernel/apic/x2apic_uv_x.c
>> +++ linux/arch/x86/kernel/apic/x2apic_uv_x.c
>> @@ -1303,7 +1303,8 @@ static int __init decode_uv_systab(void)
>>       struct uv_systab *st;
>>       int i;
>> -    if (uv_hub_info->hub_revision < UV4_HUB_REVISION_BASE)
>> +    /* Select only UV4 (hubbed or hubless) and higher */
>> +    if (is_uv_hubbed(-2) < uv(4) && is_uv_hubless(-2) < uv(4))
>>           return 0;    /* No extended UVsystab required */
>>       st = uv_systab;
>> @@ -1554,8 +1555,19 @@ static __init int uv_system_init_hubless
>>       /* Init kernel/BIOS interface */
>>       rc = uv_bios_init();
>> +    if (rc < 0) {
>> +        pr_err("UV: BIOS init error:%d\n", rc);
> 
> Why isn't that function printing an error?
> 
> 
>> +        return rc;
>> +    }
>> +
>> +    /* Process UVsystab */
>> +    rc = decode_uv_systab();
>> +    if (rc < 0) {
>> +        pr_err("UV: UVsystab decode error:%d\n", rc);
> 
> Same here, have the function itself print the error, makes this type of
> stuff much cleaner.

Turns out both functions already print an error message for each 
instance of an error.  The only redundancy is the caller also printing 
an error with just the numeric error code.  Shall I remove that?