linux-kernel - Re: [PATCH v3] powerpc32: memset: only use dcbz once cache is enabled

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] [day] [month] [year] [list]

Message-ID: <1442274735.6342.1.camel@ellerman.id.au>
Date:	Tue, 15 Sep 2015 09:52:15 +1000
From:	Michael Ellerman <mpe@...erman.id.au>
To:	Scott Wood <scottwood@...escale.com>
Cc:	Christophe LEROY <christophe.leroy@....fr>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Paul Mackerras <paulus@...ba.org>, sojkam1@....cvut.cz,
	linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v3] powerpc32: memset: only use dcbz once cache is
 enabled

On Mon, 2015-09-14 at 11:13 -0500, Scott Wood wrote:
> On Mon, 2015-09-14 at 17:44 +0200, Christophe LEROY wrote:
> > Le 14/09/2015 17:20, Scott Wood a écrit :
> > > On Mon, 2015-09-14 at 08:21 +0200, Christophe Leroy wrote:
> > > > diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
> > > > index da5847d..68a59d4 100644
> > > > --- a/arch/powerpc/lib/copy_32.S
> > > > +++ b/arch/powerpc/lib/copy_32.S
> > > > @@ -73,8 +73,13 @@ CACHELINE_MASK = (L1_CACHE_BYTES-1)
> > > > @@ -122,6 +127,16 @@ _GLOBAL(memset)
> > > >        bdnz    8b
> > > >        blr
> > > >   
> > > > +/* Simple version of memset used during early boot until cache is 
> > > > enabled
> > > > */
> > > > +simple_memset:
> > > > +     cmplwi  cr0,r5,0
> > > > +     addi    r6,r3,-1
> > > > +     beqlr
> > > > +     mtctr   r5
> > > > +1:   stbu    r4,1(r6)
> > > > +     bdnz    1b
> > > > +     blr
> > > Instead couldn't you use the generic memset at label 2: and patch the "bne
> > > 2f"?
> > 
> > Yes I could but it means adding a global symbol there at the "bne 2f". I 
> > thought it was what Michael didn't like in my v1 of memcpy().
> > What name could I give to that symbol ? Something like 
> > memcpy_nocache_patch: ?
> > What would be the best ? Having b 2f, and replacing it with bne 2f ? Or 
> > have b 2f just above and replace it by nop once cache is up ?
> 
> I'm not sure why a global symbol for an instruction to be patched would be a 
> big deal (similar to kvm_emul.S)...  I thought Michael just wanted existing 
> infrastructure to be used if it's practical to do so.

Yeah, I don't mind adding a global if you need it.

I'm not wedded to either approach, I'm just trying to find the least intrusive
option that uses the most existing code, and is the most maintainable into the
future.

cheers



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/