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
| ||
|
Message-ID: <e735fa2cde6e9c92dda134634cb3d67b64b23fe9.camel@linux.ibm.com> Date: Tue, 22 Nov 2022 14:10:41 +0100 From: Janis Schoetterl-Glausch <scgl@...ux.ibm.com> To: Thomas Huth <thuth@...hat.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>, Janosch Frank <frankja@...ux.ibm.com>, Claudio Imbrenda <imbrenda@...ux.ibm.com>, Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>, Alexander Gordeev <agordeev@...ux.ibm.com> Cc: David Hildenbrand <david@...hat.com>, Jonathan Corbet <corbet@....net>, kvm@...r.kernel.org, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, linux-s390@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>, Shuah Khan <shuah@...nel.org>, Sven Schnelle <svens@...ux.ibm.com> Subject: Re: [PATCH v3 2/9] Documentation: KVM: s390: Describe KVM_S390_MEMOP_F_CMPXCHG On Tue, 2022-11-22 at 08:47 +0100, Thomas Huth wrote: > On 17/11/2022 23.17, Janis Schoetterl-Glausch wrote: > > Describe the semantics of the new KVM_S390_MEMOP_F_CMPXCHG flag for > > absolute vm write memops which allows user space to perform (storage key > > checked) cmpxchg operations on guest memory. > > > > Signed-off-by: Janis Schoetterl-Glausch <scgl@...ux.ibm.com> > > --- > ... > > Supported flags: > > * ``KVM_S390_MEMOP_F_CHECK_ONLY`` > > * ``KVM_S390_MEMOP_F_SKEY_PROTECTION`` > > + * ``KVM_S390_MEMOP_F_CMPXCHG`` > > + > > +The semantics of the flags common with logical acesses are as for logical > > +accesses. > > + > > +For write accesses, the KVM_S390_MEMOP_F_CMPXCHG might be supported. > > I'd maybe merge this with the last sentence: > > For write accesses, the KVM_S390_MEMOP_F_CMPXCHG flag is supported if > KVM_CAP_S390_MEM_OP_EXTENSION has bit 1 (i.e. bit with value 2) set. Ok. > > ... and speaking of that, I wonder whether it's maybe a good idea to > introduce some #defines for bit 1 / value 2, to avoid the confusion ? Not sure, I don't feel it's too complicated. Where would you define it? Next to the mem_op struct? KVM_S390_MEMOP_EXTENSION_CAP_CMPXCHG? > > > +In this case, instead of doing an unconditional write, the access occurs only > > +if the target location contains the "size" byte long value pointed to by > > +"old_p". This is performed as an atomic cmpxchg. > > I had to read the first sentence twice to understand it ... maybe it's > easier to understand if you move the "size" part to the second sentence: > > In this case, instead of doing an unconditional write, the access occurs > only if the target location contains value pointed to by "old_p". This is > performed as an atomic cmpxchg with the length specified by the "size" > parameter. > > ? Ok. > > > "size" must be a power of two > > +up to and including 16. > > +The value at the target location is written to the location "old_p" points to. > > IMHO something like this would be better: > > The value at the target location is replaced with the value from the > location that "old_p" points to. I'm trying to say the opposite :). I went with this: If the exchange did not take place because the target value doesn't match the old value, KVM_S390_MEMOP_R_NO_XCHG is returned. In this case the value "old_addr" points to is replaced by the target value. > > > +If the exchange did not take place because the target value doesn't match the > > +old value KVM_S390_MEMOP_R_NO_XCHG is returned. > > +The KVM_S390_MEMOP_F_CMPXCHG flag is supported if KVM_CAP_S390_MEM_OP_EXTENSION > > +has bit 1 (i.e. bit with value 2) set. > > Thomas > > PS: Please take my suggestions with a grain of salt ... I'm not a native > speaker either. >
Powered by blists - more mailing lists