Re: [PATCH v1 8/8] ACPI: Use recently introduced uuid_le_cmp_p{p}() helpers

[Andy Shevchenko]

On Thu, 2017-04-27 at 14:46 +0200, Borislav Petkov wrote:
> On Fri, Apr 21, 2017 at 11:22:31PM +0200, Rafael J. Wysocki wrote:
> > > Â#ifdef CONFIG_ACPI_APEI_PCIEAER
> > > -		else if (!uuid_le_cmp(*(uuid_le *)gdata-
> > > >section_type,
> > > -				ÂÂÂÂÂÂCPER_SEC_PCIE)) {
> > > +		else if (!uuid_le_cmp_p(sec_type, CPER_SEC_PCIE))
> > > {
> > > Â			struct cper_sec_pcie *pcie_err;
> > > Â			pcie_err = (struct cper_sec_pcie
> > > *)(gdata+1);
> > > Â			if (sev == GHES_SEV_RECOVERABLE &&
> > > 
> > 
> > But this one is for Boris.
> 
> I don't see anything wrong with it upon a brief inspection.

Lukas pointed to this:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725

> 
> What could be improved here, though, is if the whole uuid_* types
> handling be changed so that gcc doesn't generate yucky code. Because
> here's what it does now, regardless of this patch:
> 
> ÂÂÂÂÂÂÂÂ.file 16 "./include/linux/uuid.h"
> ÂÂÂÂÂÂÂÂ.loc 16 63 0
> ÂÂÂÂÂÂÂÂleaqÂÂÂÂ16(%rsp), %rsiÂÂ#,
> ÂÂÂÂÂÂÂÂmovlÂÂÂÂ$16, %edxÂÂÂÂÂÂÂ#,
> ÂÂÂÂÂÂÂÂmovqÂÂÂÂ%r15, %rdiÂÂÂÂÂÂ# gdata,
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$84, 16(%rsp)ÂÂÂ#, MEM[(structÂÂ*)&u2]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$-23, 17(%rsp)ÂÂ#, MEM[(structÂÂ*)&u2 + 1B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$-107, 18(%rsp) #, MEM[(structÂÂ*)&u2 + 2B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$-39, 19(%rsp)ÂÂ#, MEM[(structÂÂ*)&u2 + 3B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$-63, 20(%rsp)ÂÂ#, MEM[(structÂÂ*)&u2 + 4B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$-69, 21(%rsp)ÂÂ#, MEM[(structÂÂ*)&u2 + 5B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$15, 22(%rsp)ÂÂÂ#, MEM[(structÂÂ*)&u2 + 6B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$67, 23(%rsp)ÂÂÂ#, MEM[(structÂÂ*)&u2 + 7B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$-83, 24(%rsp)ÂÂ#, MEM[(structÂÂ*)&u2 + 8B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$-111, 25(%rsp) #, MEM[(structÂÂ*)&u2 + 9B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$-76, 26(%rsp)ÂÂ#, MEM[(structÂÂ*)&u2 + 10B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$77, 27(%rsp)ÂÂÂ#, MEM[(structÂÂ*)&u2 + 11B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$-53, 28(%rsp)ÂÂ#, MEM[(structÂÂ*)&u2 + 12B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$60, 29(%rsp)ÂÂÂ#, MEM[(structÂÂ*)&u2 + 13B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$111, 30(%rsp)ÂÂ#, MEM[(structÂÂ*)&u2 + 14B]
> ÂÂÂÂÂÂÂÂmovbÂÂÂÂ$53, 31(%rsp)ÂÂÂ#, MEM[(structÂÂ*)&u2 + 15B]
> ÂÂÂÂÂÂÂÂcallÂÂÂÂmemcmpÂÂ#
> 
> So it is basically building that UUID byte by byte before calling
> memcmp.
> 
> And I'm wondering if those 16-byte arrays could be replaced with
> 
> typedef struct {
> ÂÂÂÂÂÂÂÂu64 a, b;
> } u128;
> 
> from the crypto code.
> 
> And whether the code generated by gcc would look much saner. Because
> the
> CPU can handle two qwords much better/faster than 16 u8s.
> 
> Anyway, in case someone feels bored...
> 
> --Â
> Regards/Gruss,
> ÂÂÂÂBoris.
> 
> SUSE Linux GmbH, GF: Felix ImendÃrffer, Jane Smithard, Graham Norton,
> HRB 21284 (AG NÃrnberg)

-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy