linux-kernel - Re: [PATCH v7 12/12] powerpc/vas: Define copy/paste interfaces

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]

Date:   Mon, 28 Aug 2017 21:45:29 +1000
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
Cc:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        mikey@...ling.org, stewart@...ux.vnet.ibm.com, apopple@....ibm.com,
        hbabu@...ibm.com, oohall@...il.com, linuxppc-dev@...abs.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 12/12] powerpc/vas: Define copy/paste interfaces

Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com> writes:

> Michael Ellerman [mpe@...erman.id.au] wrote:
>> Hi Suka,
>> 
>> A few more things ...
>> 
>> Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com> writes:
>> 
>> > diff --git a/arch/powerpc/platforms/powernv/copy-paste.h b/arch/powerpc/platforms/powernv/copy-paste.h
>> > new file mode 100644
>> > index 0000000..7783bb8
>> > --- /dev/null
>> > +++ b/arch/powerpc/platforms/powernv/copy-paste.h
>> > @@ -0,0 +1,74 @@
>> > +/*
>> > + * Copyright 2016 IBM Corp.
>> > + *
>> > + * This program is free software; you can redistribute it and/or
>> > + * modify it under the terms of the GNU General Public License
>> > + * as published by the Free Software Foundation; either version
>> > + * 2 of the License, or (at your option) any later version.
>> > + */
>> > +
>> > +/*
>> > + * Macros taken from tools/testing/selftests/powerpc/context_switch/cp_abort.c
>> > + */
>> 
>> These are both out of date, they're changed in v3.0B.
>> 
>> > +#define PASTE(RA, RB, L, RC) \
>> > +	.long (0x7c00070c | (RA) << (31-15) | (RB) << (31-20) \
>> > +			  | (L) << (31-10) | (RC) << (31-31))
>> 
>> You should define PPC_PASTE() in ppc-opcode.h
>> 
>> We already have PPC_INST_PASTE, so use that.
>> 
>> L and RC are gone.
>
> Ok. I thought they would come back later, but of course we can update
> these kernel-only calls then.

Possible, but if they do we can update them then.

>> > +#define CR0_SHIFT	28
>> > +#define CR0_MASK	0xF
>> 
>> Not used.
>
> Will need them now to return value in cr0?

Yes.

>> > +/*
>> > + * Copy/paste instructions:
>> > + *
>> > + *	copy RA,RB,L
>> > + *		Copy contents of address (RA) + effective_address(RB)
>> > + *		to internal copy-buffer.
>> > + *
>> > + *		L == 1 indicates this is the first copy.
>> > + *
>> > + *		L == 0 indicates its a continuation of a prior first copy.
>> > + *
>> > + *	paste RA,RB,L
>> > + *		Paste contents of internal copy-buffer to the address
>> > + *		(RA) + effective_address(RB)
>> > + *
>> > + *		L == 0 indicates its a continuation of a prior paste. i.e.
>> > + *		don't wait for the completion or update status.
>> > + *
>> > + *		L == 1 indicates this is the last paste in the group (i.e.
>> > + *		wait for the group to complete and update status in CR0).
>> > + *
>> > + *	For Power9, the L bit must be 'true' in both copy and paste.
>> > + */
>> > +
>> > +static inline int vas_copy(void *crb, int offset, int first)
>> > +{
>> > +	WARN_ON_ONCE(!first);
>> 
>> Please change the API to not require unused parameters.
>> 
>> Same for offset.
>
> Ok, Haren's NX patches will need to drop those parameters as well.

That's fine, I'm merging them all via my tree. I can fix that up.

cheers