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@linux.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.