linux-kernel - Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000 chip

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.21.9999.1903251418440.8028@viisi.sifive.com>
Date:   Mon, 25 Mar 2019 14:26:52 -0700 (PDT)
From:   Paul Walmsley <paul.walmsley@...ive.com>
To:     Borislav Petkov <bp@...en8.de>
cc:     Paul Walmsley <paul.walmsley@...ive.com>,
        Yash Shah <yash.shah@...ive.com>,
        linux-riscv@...ts.infradead.org, linux-edac@...r.kernel.org,
        palmer@...ive.com, linux-kernel@...r.kernel.org,
        robh+dt@...nel.org, mark.rutland@....com, aou@...s.berkeley.edu,
        mchehab@...nel.org, devicetree@...r.kernel.org,
        sachin.ghadi@...ive.com
Subject: Re: [PATCH 2/2] edac: sifive: Add EDAC driver for SiFive FU540-C000
 chip

On Mon, 25 Mar 2019, Borislav Petkov wrote:

> On Sun, Mar 24, 2019 at 05:23:27PM -0700, Paul Walmsley wrote:
> > > +	if (!IS_ENABLED(CONFIG_EDAC_DEBUG))
> > > +		return;
> > 
> > Can all of these debugfs functions be wrapped with an #if ... #endif such 
> > that, if CONFIG_EDAC_DEBUG is not set, they will all be stripped out by 
> > the preprocessor?
> 
> Why would you make the code more ugly with ifdeffery?
> 
> Do you have any serious code size constraints so that you absolutely
> need to remove a couple of KBs?

We'll definitely take the RAM savings that a few #ifdefs will deliver to 
us.  They add up.  We're selling chips for embedded use cases, not just 
big-iron x86 systems.

Other EDAC drivers have far more #ifdef lines than the single set that I'm 
proposing, so I don't understand why you're singling this driver out for 
criticism.  Consider:

./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_L2C
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_OCRAM
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_ETHERNET
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_NAND
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_DMA
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_USB
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_QSPI
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDMMC
./altera_edac.c:#ifdef CONFIG_EDAC_ALTERA_SDRAM
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG
./synopsys_edac.c:#ifdef CONFIG_EDAC_DEBUG


- Paul