linux-kernel - Re: [PATCH v4 07/24] EDAC/amd64: Define function to dehash address

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YikhDoGD+V7OdBq4@yaz-ubuntu>
Date:   Wed, 9 Mar 2022 21:50:06 +0000
From:   Yazen Ghannam <yazen.ghannam@....com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org,
        mchehab@...nel.org, tony.luck@...el.com, james.morse@....com,
        rric@...nel.org, Smita.KoralahalliChannabasappa@....com
Subject: Re: [PATCH v4 07/24] EDAC/amd64: Define function to dehash address

On Fri, Feb 11, 2022 at 11:47:31PM +0100, Borislav Petkov wrote:

...

> > --- a/drivers/edac/amd64_edac.c
> > +++ b/drivers/edac/amd64_edac.c
> > @@ -1077,7 +1077,7 @@ struct addr_ctx {
> >  	u8 map_num;
> >  	u8 intlv_addr_bit;
> >  	u8 cs_id;
> > -	bool hash_enabled;
> > +	int (*dehash_addr)(struct addr_ctx *ctx);
> 
> A function pointer in a context struct?!
> 
> > @@ -1357,18 +1372,9 @@ static int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr
> >  		goto out_err;
> >  	}
> >  
> > -	if (ctx.hash_enabled) {
> > -		/* Save some parentheses and grab ls-bit at the end. */
> > -		hashed_bit =	(ctx.ret_addr >> 12) ^
> > -				(ctx.ret_addr >> 18) ^
> > -				(ctx.ret_addr >> 21) ^
> > -				(ctx.ret_addr >> 30) ^
> > -				ctx.cs_id;
> > -
> > -		hashed_bit &= BIT(0);
> > -
> > -		if (hashed_bit != ((ctx.ret_addr >> ctx.intlv_addr_bit) & BIT(0)))
> > -			ctx.ret_addr ^= BIT(ctx.intlv_addr_bit);
> > +	if (ctx.dehash_addr && ctx.dehash_addr(&ctx)) {
> 
> So you can just as well do:
> 
> 	if (ctx->intlv_mode == 8)
> 		dehash_addr();
> 
> And dehash_addr() can inside determine whether df2 or df3.
> 
> Btw, that 8 looks like magic. It should be a #define.
> 
> What you have now looks a bit weird with those function pointers lumped
> together with those other members of addr_ctx. Dunno, maybe it'll make
> more sense when I read the rest first...
>

Yeah, I think I got carried away with the function pointers. :P

The functions in the context struct are set based on interleaving mode rather
than Data Fabric version. Which is why it doesn't work to include them in the
"df_ops".

But I can do something like you suggest. There will be an unconditional call
to dehash_addr(). Helper functions will be called for specific interleave
modes. Otherwise, it'll return 0 if hashing isn't used.

Thanks,
Yazen