diff mbox series

[v4,1/9] KVM: s390: Extend MEM_OP ioctl by storage key checked cmpxchg

Message ID 20221213165405.2953539-2-scgl@linux.ibm.com
State New
Headers
Series KVM: s390: Extend MEM_OP ioctl by storage key checked cmpxchg |

Commit Message

Janis Schoetterl-Glausch Dec. 13, 2022, 4:53 p.m. UTC 
  User space can use the MEM_OP ioctl to make storage key checked reads
and writes to the guest, however, it has no way of performing atomic,
key checked, accesses to the guest.
Extend the MEM_OP ioctl in order to allow for this, by adding a cmpxchg
mode. For now, support this mode for absolute accesses only.

This mode can be use, for example, to set the device-state-change
indicator and the adapter-local-summary indicator atomically.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
 include/uapi/linux/kvm.h |   7 +++
 arch/s390/kvm/gaccess.h  |   3 ++
 arch/s390/kvm/gaccess.c  | 102 +++++++++++++++++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.c |  39 ++++++++++++++-
 4 files changed, 149 insertions(+), 2 deletions(-)

Comments

kernel test robot Dec. 14, 2022, 2:23 a.m. UTC | #1 
Hi Janis,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on 739ad2e4e15b585a0eaf98b7bdee62b2dd9588c9]

url:    https://github.com/intel-lab-lkp/linux/commits/Janis-Schoetterl-Glausch/KVM-s390-Extend-MEM_OP-ioctl-by-storage-key-checked-cmpxchg/20221214-005540
base:   739ad2e4e15b585a0eaf98b7bdee62b2dd9588c9
patch link:    https://lore.kernel.org/r/20221213165405.2953539-2-scgl%40linux.ibm.com
patch subject: [PATCH v4 1/9] KVM: s390: Extend MEM_OP ioctl by storage key checked cmpxchg
config: s390-randconfig-r004-20221213
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/4e0991bd47ba30c7588e042da7a84d84b9f84056
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Janis-Schoetterl-Glausch/KVM-s390-Extend-MEM_OP-ioctl-by-storage-key-checked-cmpxchg/20221214-005540
        git checkout 4e0991bd47ba30c7588e042da7a84d84b9f84056
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   s390-linux-ld: arch/s390/kvm/gaccess.o: in function `__cmpxchg_user_key':
>> arch/s390/include/asm/uaccess.h:410: undefined reference to `__ashlti3'
>> s390-linux-ld: arch/s390/include/asm/uaccess.h:411: undefined reference to `__ashlti3'
   s390-linux-ld: arch/s390/include/asm/uaccess.h:458: undefined reference to `__ashlti3'
   s390-linux-ld: arch/s390/include/asm/uaccess.h:459: undefined reference to `__ashlti3'


vim +410 arch/s390/include/asm/uaccess.h

739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  396  
4148575abe1e14 Heiko Carstens           2022-11-02  397  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
4148575abe1e14 Heiko Carstens           2022-11-02  398  					      __uint128_t old, __uint128_t new,
4148575abe1e14 Heiko Carstens           2022-11-02  399  					      unsigned long key, int size)
4148575abe1e14 Heiko Carstens           2022-11-02  400  {
4148575abe1e14 Heiko Carstens           2022-11-02  401  	int rc = 0;
4148575abe1e14 Heiko Carstens           2022-11-02  402  
4148575abe1e14 Heiko Carstens           2022-11-02  403  	switch (size) {
4148575abe1e14 Heiko Carstens           2022-11-02  404  	case 1: {
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  405  		unsigned int prev, shift, mask, _old, _new;
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  406  		unsigned long count;
4148575abe1e14 Heiko Carstens           2022-11-02  407  
4148575abe1e14 Heiko Carstens           2022-11-02  408  		shift = (3 ^ (address & 3)) << 3;
4148575abe1e14 Heiko Carstens           2022-11-02  409  		address ^= address & 3;
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16 @410  		_old = (old & 0xff) << shift;
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16 @411  		_new = (new & 0xff) << shift;
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  412  		mask = ~(0xff << shift);
4148575abe1e14 Heiko Carstens           2022-11-02  413  		asm volatile(
4148575abe1e14 Heiko Carstens           2022-11-02  414  			"	spka	0(%[key])\n"
4148575abe1e14 Heiko Carstens           2022-11-02  415  			"	sacf	256\n"
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  416  			"	llill	%[count],%[max_loops]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  417  			"0:	l	%[prev],%[address]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  418  			"1:	nr	%[prev],%[mask]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  419  			"	xilf	%[mask],0xffffffff\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  420  			"	or	%[new],%[prev]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  421  			"	or	%[prev],%[tmp]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  422  			"2:	lr	%[tmp],%[prev]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  423  			"3:	cs	%[prev],%[new],%[address]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  424  			"4:	jnl	5f\n"
4148575abe1e14 Heiko Carstens           2022-11-02  425  			"	xr	%[tmp],%[prev]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  426  			"	xr	%[new],%[tmp]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  427  			"	nr	%[tmp],%[mask]\n"
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  428  			"	jnz	5f\n"
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  429  			"	brct	%[count],2b\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  430  			"5:	sacf	768\n"
4148575abe1e14 Heiko Carstens           2022-11-02  431  			"	spka	%[default_key]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  432  			EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev])
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  433  			EX_TABLE_UA_LOAD_REG(1b, 5b, %[rc], %[prev])
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  434  			EX_TABLE_UA_LOAD_REG(3b, 5b, %[rc], %[prev])
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  435  			EX_TABLE_UA_LOAD_REG(4b, 5b, %[rc], %[prev])
4148575abe1e14 Heiko Carstens           2022-11-02  436  			: [rc] "+&d" (rc),
4148575abe1e14 Heiko Carstens           2022-11-02  437  			  [prev] "=&d" (prev),
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  438  			  [address] "+Q" (*(int *)address),
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  439  			  [tmp] "+&d" (_old),
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  440  			  [new] "+&d" (_new),
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  441  			  [mask] "+&d" (mask),
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  442  			  [count] "=a" (count)
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  443  			: [key] "%[count]" (key << 4),
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  444  			  [default_key] "J" (PAGE_DEFAULT_KEY),
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  445  			  [max_loops] "J" (CMPXCHG_USER_KEY_MAX_LOOPS)
4148575abe1e14 Heiko Carstens           2022-11-02  446  			: "memory", "cc");
4148575abe1e14 Heiko Carstens           2022-11-02  447  		*(unsigned char *)uval = prev >> shift;
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  448  		if (!count)
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  449  			rc = -EAGAIN;
4148575abe1e14 Heiko Carstens           2022-11-02  450  		return rc;
4148575abe1e14 Heiko Carstens           2022-11-02  451  	}
4148575abe1e14 Heiko Carstens           2022-11-02  452  	case 2: {
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  453  		unsigned int prev, shift, mask, _old, _new;
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  454  		unsigned long count;
4148575abe1e14 Heiko Carstens           2022-11-02  455  
4148575abe1e14 Heiko Carstens           2022-11-02  456  		shift = (2 ^ (address & 2)) << 3;
4148575abe1e14 Heiko Carstens           2022-11-02  457  		address ^= address & 2;
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  458  		_old = (old & 0xffff) << shift;
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  459  		_new = (new & 0xffff) << shift;
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  460  		mask = ~(0xffff << shift);
4148575abe1e14 Heiko Carstens           2022-11-02  461  		asm volatile(
4148575abe1e14 Heiko Carstens           2022-11-02  462  			"	spka	0(%[key])\n"
4148575abe1e14 Heiko Carstens           2022-11-02  463  			"	sacf	256\n"
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  464  			"	llill	%[count],%[max_loops]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  465  			"0:	l	%[prev],%[address]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  466  			"1:	nr	%[prev],%[mask]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  467  			"	xilf	%[mask],0xffffffff\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  468  			"	or	%[new],%[prev]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  469  			"	or	%[prev],%[tmp]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  470  			"2:	lr	%[tmp],%[prev]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  471  			"3:	cs	%[prev],%[new],%[address]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  472  			"4:	jnl	5f\n"
4148575abe1e14 Heiko Carstens           2022-11-02  473  			"	xr	%[tmp],%[prev]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  474  			"	xr	%[new],%[tmp]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  475  			"	nr	%[tmp],%[mask]\n"
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  476  			"	jnz	5f\n"
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  477  			"	brct	%[count],2b\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  478  			"5:	sacf	768\n"
4148575abe1e14 Heiko Carstens           2022-11-02  479  			"	spka	%[default_key]\n"
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  480  			EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev])
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  481  			EX_TABLE_UA_LOAD_REG(1b, 5b, %[rc], %[prev])
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  482  			EX_TABLE_UA_LOAD_REG(3b, 5b, %[rc], %[prev])
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  483  			EX_TABLE_UA_LOAD_REG(4b, 5b, %[rc], %[prev])
4148575abe1e14 Heiko Carstens           2022-11-02  484  			: [rc] "+&d" (rc),
4148575abe1e14 Heiko Carstens           2022-11-02  485  			  [prev] "=&d" (prev),
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  486  			  [address] "+Q" (*(int *)address),
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  487  			  [tmp] "+&d" (_old),
51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  488  			  [new] "+&d" (_new),
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  489  			  [mask] "+&d" (mask),
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  490  			  [count] "=a" (count)
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  491  			: [key] "%[count]" (key << 4),
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  492  			  [default_key] "J" (PAGE_DEFAULT_KEY),
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  493  			  [max_loops] "J" (CMPXCHG_USER_KEY_MAX_LOOPS)
4148575abe1e14 Heiko Carstens           2022-11-02  494  			: "memory", "cc");
4148575abe1e14 Heiko Carstens           2022-11-02  495  		*(unsigned short *)uval = prev >> shift;
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  496  		if (!count)
739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  497  			rc = -EAGAIN;
4148575abe1e14 Heiko Carstens           2022-11-02  498  		return rc;
4148575abe1e14 Heiko Carstens           2022-11-02  499  	}
4148575abe1e14 Heiko Carstens           2022-11-02  500  	case 4:	{
4148575abe1e14 Heiko Carstens           2022-11-02  501  		unsigned int prev = old;
4148575abe1e14 Heiko Carstens           2022-11-02  502  
4148575abe1e14 Heiko Carstens           2022-11-02  503  		asm volatile(
4148575abe1e14 Heiko Carstens           2022-11-02  504  			"	spka	0(%[key])\n"
4148575abe1e14 Heiko Carstens           2022-11-02  505  			"	sacf	256\n"
4148575abe1e14 Heiko Carstens           2022-11-02  506  			"0:	cs	%[prev],%[new],%[address]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  507  			"1:	sacf	768\n"
4148575abe1e14 Heiko Carstens           2022-11-02  508  			"	spka	%[default_key]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  509  			EX_TABLE_UA_LOAD_REG(0b, 1b, %[rc], %[prev])
4148575abe1e14 Heiko Carstens           2022-11-02  510  			EX_TABLE_UA_LOAD_REG(1b, 1b, %[rc], %[prev])
4148575abe1e14 Heiko Carstens           2022-11-02  511  			: [rc] "+&d" (rc),
4148575abe1e14 Heiko Carstens           2022-11-02  512  			  [prev] "+&d" (prev),
4148575abe1e14 Heiko Carstens           2022-11-02  513  			  [address] "+Q" (*(int *)address)
4148575abe1e14 Heiko Carstens           2022-11-02  514  			: [new] "d" ((unsigned int)new),
4148575abe1e14 Heiko Carstens           2022-11-02  515  			  [key] "a" (key << 4),
4148575abe1e14 Heiko Carstens           2022-11-02  516  			  [default_key] "J" (PAGE_DEFAULT_KEY)
4148575abe1e14 Heiko Carstens           2022-11-02  517  			: "memory", "cc");
4148575abe1e14 Heiko Carstens           2022-11-02  518  		*(unsigned int *)uval = prev;
4148575abe1e14 Heiko Carstens           2022-11-02  519  		return rc;
4148575abe1e14 Heiko Carstens           2022-11-02  520  	}
4148575abe1e14 Heiko Carstens           2022-11-02  521  	case 8: {
4148575abe1e14 Heiko Carstens           2022-11-02  522  		unsigned long prev = old;
4148575abe1e14 Heiko Carstens           2022-11-02  523  
4148575abe1e14 Heiko Carstens           2022-11-02  524  		asm volatile(
4148575abe1e14 Heiko Carstens           2022-11-02  525  			"	spka	0(%[key])\n"
4148575abe1e14 Heiko Carstens           2022-11-02  526  			"	sacf	256\n"
4148575abe1e14 Heiko Carstens           2022-11-02  527  			"0:	csg	%[prev],%[new],%[address]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  528  			"1:	sacf	768\n"
4148575abe1e14 Heiko Carstens           2022-11-02  529  			"	spka	%[default_key]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  530  			EX_TABLE_UA_LOAD_REG(0b, 1b, %[rc], %[prev])
4148575abe1e14 Heiko Carstens           2022-11-02  531  			EX_TABLE_UA_LOAD_REG(1b, 1b, %[rc], %[prev])
4148575abe1e14 Heiko Carstens           2022-11-02  532  			: [rc] "+&d" (rc),
4148575abe1e14 Heiko Carstens           2022-11-02  533  			  [prev] "+&d" (prev),
4148575abe1e14 Heiko Carstens           2022-11-02  534  			  [address] "+QS" (*(long *)address)
4148575abe1e14 Heiko Carstens           2022-11-02  535  			: [new] "d" ((unsigned long)new),
4148575abe1e14 Heiko Carstens           2022-11-02  536  			  [key] "a" (key << 4),
4148575abe1e14 Heiko Carstens           2022-11-02  537  			  [default_key] "J" (PAGE_DEFAULT_KEY)
4148575abe1e14 Heiko Carstens           2022-11-02  538  			: "memory", "cc");
4148575abe1e14 Heiko Carstens           2022-11-02  539  		*(unsigned long *)uval = prev;
4148575abe1e14 Heiko Carstens           2022-11-02  540  		return rc;
4148575abe1e14 Heiko Carstens           2022-11-02  541  	}
4148575abe1e14 Heiko Carstens           2022-11-02  542  	case 16: {
4148575abe1e14 Heiko Carstens           2022-11-02  543  		__uint128_t prev = old;
4148575abe1e14 Heiko Carstens           2022-11-02  544  
4148575abe1e14 Heiko Carstens           2022-11-02  545  		asm volatile(
4148575abe1e14 Heiko Carstens           2022-11-02  546  			"	spka	0(%[key])\n"
4148575abe1e14 Heiko Carstens           2022-11-02  547  			"	sacf	256\n"
4148575abe1e14 Heiko Carstens           2022-11-02  548  			"0:	cdsg	%[prev],%[new],%[address]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  549  			"1:	sacf	768\n"
4148575abe1e14 Heiko Carstens           2022-11-02  550  			"	spka	%[default_key]\n"
4148575abe1e14 Heiko Carstens           2022-11-02  551  			EX_TABLE_UA_LOAD_REGPAIR(0b, 1b, %[rc], %[prev])
4148575abe1e14 Heiko Carstens           2022-11-02  552  			EX_TABLE_UA_LOAD_REGPAIR(1b, 1b, %[rc], %[prev])
4148575abe1e14 Heiko Carstens           2022-11-02  553  			: [rc] "+&d" (rc),
4148575abe1e14 Heiko Carstens           2022-11-02  554  			  [prev] "+&d" (prev),
4148575abe1e14 Heiko Carstens           2022-11-02  555  			  [address] "+QS" (*(__int128_t *)address)
4148575abe1e14 Heiko Carstens           2022-11-02  556  			: [new] "d" (new),
4148575abe1e14 Heiko Carstens           2022-11-02  557  			  [key] "a" (key << 4),
4148575abe1e14 Heiko Carstens           2022-11-02  558  			  [default_key] "J" (PAGE_DEFAULT_KEY)
4148575abe1e14 Heiko Carstens           2022-11-02  559  			: "memory", "cc");
4148575abe1e14 Heiko Carstens           2022-11-02  560  		*(__uint128_t *)uval = prev;
4148575abe1e14 Heiko Carstens           2022-11-02  561  		return rc;
4148575abe1e14 Heiko Carstens           2022-11-02  562  	}
4148575abe1e14 Heiko Carstens           2022-11-02  563  	}
4148575abe1e14 Heiko Carstens           2022-11-02  564  	__cmpxchg_user_key_called_with_bad_pointer();
4148575abe1e14 Heiko Carstens           2022-11-02  565  	return rc;
4148575abe1e14 Heiko Carstens           2022-11-02  566  }
4148575abe1e14 Heiko Carstens           2022-11-02  567
Thomas Huth Dec. 14, 2022, 9:19 a.m. UTC | #2 
On 13/12/2022 17.53, Janis Schoetterl-Glausch wrote:
> User space can use the MEM_OP ioctl to make storage key checked reads
> and writes to the guest, however, it has no way of performing atomic,
> key checked, accesses to the guest.
> Extend the MEM_OP ioctl in order to allow for this, by adding a cmpxchg
> mode. For now, support this mode for absolute accesses only.
> 
> This mode can be use, for example, to set the device-state-change
> indicator and the adapter-local-summary indicator atomically.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
>   include/uapi/linux/kvm.h |   7 +++
>   arch/s390/kvm/gaccess.h  |   3 ++
>   arch/s390/kvm/gaccess.c  | 102 +++++++++++++++++++++++++++++++++++++++
>   arch/s390/kvm/kvm-s390.c |  39 ++++++++++++++-
>   4 files changed, 149 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 0d5d4419139a..f106db1af5ee 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -588,6 +588,8 @@ struct kvm_s390_mem_op {
>   		struct {
>   			__u8 ar;	/* the access register number */
>   			__u8 key;	/* access key, ignored if flag unset */
> +			__u8 pad1[6];	/* ignored */
> +			__u64 old_addr;	/* ignored if flag unset */
>   		};
>   		__u32 sida_offset; /* offset into the sida */
>   		__u8 reserved[32]; /* ignored */
> @@ -604,6 +606,11 @@ struct kvm_s390_mem_op {
>   #define KVM_S390_MEMOP_F_CHECK_ONLY		(1ULL << 0)
>   #define KVM_S390_MEMOP_F_INJECT_EXCEPTION	(1ULL << 1)
>   #define KVM_S390_MEMOP_F_SKEY_PROTECTION	(1ULL << 2)
> +#define KVM_S390_MEMOP_F_CMPXCHG		(1ULL << 3)
> +/* flags specifying extension support */
> +#define KVM_S390_MEMOP_EXTENSION_CAP_CMPXCHG 0x2
> +/* Non program exception return codes (pgm codes are 16 bit) */
> +#define KVM_S390_MEMOP_R_NO_XCHG		(1 << 16)
>   
>   /* for KVM_INTERRUPT */
>   struct kvm_interrupt {
> diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
> index 9408d6cc8e2c..92a3b9fb31ec 100644
> --- a/arch/s390/kvm/gaccess.h
> +++ b/arch/s390/kvm/gaccess.h
> @@ -206,6 +206,9 @@ int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
>   int access_guest_real(struct kvm_vcpu *vcpu, unsigned long gra,
>   		      void *data, unsigned long len, enum gacc_mode mode);
>   
> +int cmpxchg_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, int len,
> +			       __uint128_t *old, __uint128_t new, u8 access_key);
> +
>   /**
>    * write_guest_with_key - copy data from kernel space to guest space
>    * @vcpu: virtual cpu
> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index 0243b6e38d36..6165e761a637 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c
> @@ -1161,6 +1161,108 @@ int access_guest_real(struct kvm_vcpu *vcpu, unsigned long gra,
>   	return rc;
>   }
>   
> +/**
> + * cmpxchg_guest_abs_with_key() - Perform cmpxchg on guest absolute address.
> + * @kvm: Virtual machine instance.
> + * @gpa: Absolute guest address of the location to be changed.
> + * @len: Operand length of the cmpxchg, required: 1 <= len <= 16. Providing a
> + *       non power of two will result in failure.
> + * @old_addr: Pointer to old value. If the location at @gpa contains this value, the
> + *         exchange will succeed. After calling cmpxchg_guest_abs_with_key() *@old
> + *         contains the value at @gpa before the attempt to exchange the value.
> + * @new: The value to place at @gpa.
> + * @access_key: The access key to use for the guest access.
> + *
> + * Atomically exchange the value at @gpa by @new, if it contains *@old.
> + * Honors storage keys.
> + *
> + * Return: * 0: successful exchange
> + *         * 1: exchange unsuccessful
> + *         * a program interruption code indicating the reason cmpxchg could
> + *           not be attempted
> + *         * -EINVAL: address misaligned or len not power of two
> + *         * -EAGAIN: transient failure (len 1 or 2)
> + *         * -EOPNOTSUPP: read-only memslot (should never occur)
> + */
> +int cmpxchg_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, int len,
> +			       __uint128_t *old_addr, __uint128_t new,
> +			       u8 access_key)
> +{
> +	gfn_t gfn = gpa >> PAGE_SHIFT;
> +	struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
> +	bool writable;
> +	hva_t hva;
> +	int ret;
> +
> +	if (!IS_ALIGNED(gpa, len))
> +		return -EINVAL;
> +
> +	hva = gfn_to_hva_memslot_prot(slot, gfn, &writable);
> +	if (kvm_is_error_hva(hva))
> +		return PGM_ADDRESSING;
> +	/*
> +	 * Check if it's a read-only memslot, even though that cannot occur
> +	 * since those are unsupported.
> +	 * Don't try to actually handle that case.
> +	 */
> +	if (!writable)
> +		return -EOPNOTSUPP;
> +
> +	hva += offset_in_page(gpa);
> +	switch (len) {
> +	case 1: {
> +		u8 old;
> +
> +		ret = cmpxchg_user_key((u8 *)hva, &old, *old_addr, new, access_key);
> +		ret = ret < 0 ? ret : old != *old_addr;
> +		*old_addr = old;
> +		break;
> +	}
> +	case 2: {
> +		u16 old;
> +
> +		ret = cmpxchg_user_key((u16 *)hva, &old, *old_addr, new, access_key);
> +		ret = ret < 0 ? ret : old != *old_addr;
> +		*old_addr = old;
> +		break;
> +	}
> +	case 4: {
> +		u32 old;
> +
> +		ret = cmpxchg_user_key((u32 *)hva, &old, *old_addr, new, access_key);
> +		ret = ret < 0 ? ret : old != *old_addr;
> +		*old_addr = old;
> +		break;
> +	}
> +	case 8: {
> +		u64 old;
> +
> +		ret = cmpxchg_user_key((u64 *)hva, &old, *old_addr, new, access_key);
> +		ret = ret < 0 ? ret : old != *old_addr;
> +		*old_addr = old;
> +		break;
> +	}
> +	case 16: {
> +		__uint128_t old;
> +
> +		ret = cmpxchg_user_key((__uint128_t *)hva, &old, *old_addr, new, access_key);
> +		ret = ret < 0 ? ret : old != *old_addr;
> +		*old_addr = old;
> +		break;
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +	mark_page_dirty_in_slot(kvm, slot, gfn);
> +	/*
> +	 * Assume that the fault is caused by protection, either key protection
> +	 * or user page write protection.
> +	 */
> +	if (ret == -EFAULT)
> +		ret = PGM_PROTECTION;
> +	return ret;
> +}
> +
>   /**
>    * guest_translate_address_with_key - translate guest logical into guest absolute address
>    * @vcpu: virtual cpu
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 45d4b8182b07..47bcf2cb4345 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -576,7 +576,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>   	case KVM_CAP_S390_VCPU_RESETS:
>   	case KVM_CAP_SET_GUEST_DEBUG:
>   	case KVM_CAP_S390_DIAG318:
> -	case KVM_CAP_S390_MEM_OP_EXTENSION:
>   		r = 1;
>   		break;
>   	case KVM_CAP_SET_GUEST_DEBUG2:
> @@ -590,6 +589,14 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>   	case KVM_CAP_S390_MEM_OP:
>   		r = MEM_OP_MAX_SIZE;
>   		break;
> +	case KVM_CAP_S390_MEM_OP_EXTENSION:
> +		/*
> +		 * Flag bits indicating which extensions are supported.
> +		 * The first extension doesn't use a flag, but pretend it does,
> +		 * this way that can be changed in the future.
> +		 */
> +		r = KVM_S390_MEMOP_EXTENSION_CAP_CMPXCHG | 1;
> +		break;
>   	case KVM_CAP_NR_VCPUS:
>   	case KVM_CAP_MAX_VCPUS:
>   	case KVM_CAP_MAX_VCPU_ID:
> @@ -2714,12 +2721,19 @@ static bool access_key_invalid(u8 access_key)
>   static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
>   {
>   	void __user *uaddr = (void __user *)mop->buf;
> +	void __user *old_addr = (void __user *)mop->old_addr;
> +	union {
> +		__uint128_t quad;
> +		char raw[sizeof(__uint128_t)];
> +	} old = { .quad = 0}, new = { .quad = 0 };
> +	unsigned int off_in_quad = sizeof(new) - mop->size;
>   	u64 supported_flags;
>   	void *tmpbuf = NULL;
>   	int r, srcu_idx;
>   
>   	supported_flags = KVM_S390_MEMOP_F_SKEY_PROTECTION
> -			  | KVM_S390_MEMOP_F_CHECK_ONLY;
> +			  | KVM_S390_MEMOP_F_CHECK_ONLY
> +			  | KVM_S390_MEMOP_F_CMPXCHG;
>   	if (mop->flags & ~supported_flags || !mop->size)
>   		return -EINVAL;
>   	if (mop->size > MEM_OP_MAX_SIZE)
> @@ -2741,6 +2755,19 @@ static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
>   	} else {
>   		mop->key = 0;
>   	}
> +	if (mop->flags & KVM_S390_MEMOP_F_CMPXCHG) {
> +		/*
> +		 * This validates off_in_quad. Checking that size is a power
> +		 * of two is not necessary, as cmpxchg_guest_abs_with_key
> +		 * takes care of that
> +		 */
> +		if (mop->size > sizeof(new))
> +			return -EINVAL;

I'd maybe add a check for mop->op == KVM_S390_MEMOP_ABSOLUTE_WRITE here, 
since calling the _READ function with the F_CMPXCHG flag set does not make 
too much sense.

Anyway, patch looks good to me, so with or without that additional check:
Reviewed-by: Thomas Huth <thuth@redhat.com>

> +		if (copy_from_user(&new.raw[off_in_quad], uaddr, mop->size))
> +			return -EFAULT;
> +		if (copy_from_user(&old.raw[off_in_quad], old_addr, mop->size))
> +			return -EFAULT;
> +	}
>   	if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
>   		tmpbuf = vmalloc(mop->size);
>   		if (!tmpbuf)
> @@ -2771,6 +2798,14 @@ static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
>   	case KVM_S390_MEMOP_ABSOLUTE_WRITE: {
>   		if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
>   			r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, mop->key);
> +		} else if (mop->flags & KVM_S390_MEMOP_F_CMPXCHG) {
> +			r = cmpxchg_guest_abs_with_key(kvm, mop->gaddr, mop->size,
> +						       &old.quad, new.quad, mop->key);
> +			if (r == 1) {
> +				r = KVM_S390_MEMOP_R_NO_XCHG;
> +				if (copy_to_user(old_addr, &old.raw[off_in_quad], mop->size))
> +					r = -EFAULT;
> +			}
>   		} else {
>   			if (copy_from_user(tmpbuf, uaddr, mop->size)) {
>   				r = -EFAULT;
Janis Schoetterl-Glausch Dec. 14, 2022, 1:12 p.m. UTC | #3 
On Wed, 2022-12-14 at 10:19 +0100, Thomas Huth wrote:
> On 13/12/2022 17.53, Janis Schoetterl-Glausch wrote:
> > User space can use the MEM_OP ioctl to make storage key checked reads
> > and writes to the guest, however, it has no way of performing atomic,
> > key checked, accesses to the guest.
> > Extend the MEM_OP ioctl in order to allow for this, by adding a cmpxchg
> > mode. For now, support this mode for absolute accesses only.
> > 
> > This mode can be use, for example, to set the device-state-change
> > indicator and the adapter-local-summary indicator atomically.
> > 
> > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> > ---
> >   include/uapi/linux/kvm.h |   7 +++
> >   arch/s390/kvm/gaccess.h  |   3 ++
> >   arch/s390/kvm/gaccess.c  | 102 +++++++++++++++++++++++++++++++++++++++
> >   arch/s390/kvm/kvm-s390.c |  39 ++++++++++++++-
> >   4 files changed, 149 insertions(+), 2 deletions(-)
> > 
[...]
> > 
> > @@ -2714,12 +2721,19 @@ static bool access_key_invalid(u8 access_key)
> >   static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
> >   {
> >   	void __user *uaddr = (void __user *)mop->buf;
> > +	void __user *old_addr = (void __user *)mop->old_addr;
> > +	union {
> > +		__uint128_t quad;
> > +		char raw[sizeof(__uint128_t)];
> > +	} old = { .quad = 0}, new = { .quad = 0 };
> > +	unsigned int off_in_quad = sizeof(new) - mop->size;
> >   	u64 supported_flags;
> >   	void *tmpbuf = NULL;
> >   	int r, srcu_idx;
> >   
> >   	supported_flags = KVM_S390_MEMOP_F_SKEY_PROTECTION
> > -			  | KVM_S390_MEMOP_F_CHECK_ONLY;
> > +			  | KVM_S390_MEMOP_F_CHECK_ONLY
> > +			  | KVM_S390_MEMOP_F_CMPXCHG;
> >   	if (mop->flags & ~supported_flags || !mop->size)
> >   		return -EINVAL;
> >   	if (mop->size > MEM_OP_MAX_SIZE)
> > @@ -2741,6 +2755,19 @@ static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
> >   	} else {
> >   		mop->key = 0;
> >   	}
> > +	if (mop->flags & KVM_S390_MEMOP_F_CMPXCHG) {
> > +		/*
> > +		 * This validates off_in_quad. Checking that size is a power
> > +		 * of two is not necessary, as cmpxchg_guest_abs_with_key
> > +		 * takes care of that
> > +		 */
> > +		if (mop->size > sizeof(new))
> > +			return -EINVAL;
> 
> I'd maybe add a check for mop->op == KVM_S390_MEMOP_ABSOLUTE_WRITE here, 
> since calling the _READ function with the F_CMPXCHG flag set does not make 
> too much sense.

Good point.
> 
> Anyway, patch looks good to me, so with or without that additional check:
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks!
> 
> > +		if (copy_from_user(&new.raw[off_in_quad], uaddr, mop->size))
> > +			return -EFAULT;
> > +		if (copy_from_user(&old.raw[off_in_quad], old_addr, mop->size))
> > +			return -EFAULT;
> > +	}
> >   	if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
> >   		tmpbuf = vmalloc(mop->size);
> >   		if (!tmpbuf)
> > @@ -2771,6 +2798,14 @@ static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
> >   	case KVM_S390_MEMOP_ABSOLUTE_WRITE: {
> >   		if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
> >   			r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, mop->key);
> > +		} else if (mop->flags & KVM_S390_MEMOP_F_CMPXCHG) {
> > +			r = cmpxchg_guest_abs_with_key(kvm, mop->gaddr, mop->size,
> > +						       &old.quad, new.quad, mop->key);
> > +			if (r == 1) {
> > +				r = KVM_S390_MEMOP_R_NO_XCHG;
> > +				if (copy_to_user(old_addr, &old.raw[off_in_quad], mop->size))
> > +					r = -EFAULT;
> > +			}
> >   		} else {
> >   			if (copy_from_user(tmpbuf, uaddr, mop->size)) {
> >   				r = -EFAULT;
>
Janis Schoetterl-Glausch Dec. 19, 2022, 9:24 p.m. UTC | #4 
On Wed, 2022-12-14 at 10:23 +0800, kernel test robot wrote:
> Hi Janis,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on 739ad2e4e15b585a0eaf98b7bdee62b2dd9588c9]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Janis-Schoetterl-Glausch/KVM-s390-Extend-MEM_OP-ioctl-by-storage-key-checked-cmpxchg/20221214-005540
> base:   739ad2e4e15b585a0eaf98b7bdee62b2dd9588c9
> patch link:    https://lore.kernel.org/r/20221213165405.2953539-2-scgl%40linux.ibm.com
> patch subject: [PATCH v4 1/9] KVM: s390: Extend MEM_OP ioctl by storage key checked cmpxchg
> config: s390-randconfig-r004-20221213
> compiler: s390-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/4e0991bd47ba30c7588e042da7a84d84b9f84056
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Janis-Schoetterl-Glausch/KVM-s390-Extend-MEM_OP-ioctl-by-storage-key-checked-cmpxchg/20221214-005540
>         git checkout 4e0991bd47ba30c7588e042da7a84d84b9f84056
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    s390-linux-ld: arch/s390/kvm/gaccess.o: in function `__cmpxchg_user_key':
> > > arch/s390/include/asm/uaccess.h:410: undefined reference to `__ashlti3'
> > > s390-linux-ld: arch/s390/include/asm/uaccess.h:411: undefined reference to `__ashlti3'
>    s390-linux-ld: arch/s390/include/asm/uaccess.h:458: undefined reference to `__ashlti3'
>    s390-linux-ld: arch/s390/include/asm/uaccess.h:459: undefined reference to `__ashlti3'
> 
> 
> vim +410 arch/s390/include/asm/uaccess.h
> 
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  396  
> 4148575abe1e14 Heiko Carstens           2022-11-02  397  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
> 4148575abe1e14 Heiko Carstens           2022-11-02  398  					      __uint128_t old, __uint128_t new,
> 4148575abe1e14 Heiko Carstens           2022-11-02  399  					      unsigned long key, int size)
> 4148575abe1e14 Heiko Carstens           2022-11-02  400  {
> 4148575abe1e14 Heiko Carstens           2022-11-02  401  	int rc = 0;
> 4148575abe1e14 Heiko Carstens           2022-11-02  402  
> 4148575abe1e14 Heiko Carstens           2022-11-02  403  	switch (size) {
> 4148575abe1e14 Heiko Carstens           2022-11-02  404  	case 1: {
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  405  		unsigned int prev, shift, mask, _old, _new;
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  406  		unsigned long count;
> 4148575abe1e14 Heiko Carstens           2022-11-02  407  
> 4148575abe1e14 Heiko Carstens           2022-11-02  408  		shift = (3 ^ (address & 3)) << 3;
> 4148575abe1e14 Heiko Carstens           2022-11-02  409  		address ^= address & 3;
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16 @410  		_old = (old & 0xff) << shift;
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16 @411  		_new = (new & 0xff) << shift;

Not sure what it is in this config that causes gcc to emit this symbol instead of a shift instruction, but casting old/new to 32 bit fixes
the error.

> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  412  		mask = ~(0xff << shift);
> 4148575abe1e14 Heiko Carstens           2022-11-02  413  		asm volatile(
> 4148575abe1e14 Heiko Carstens           2022-11-02  414  			"	spka	0(%[key])\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  415  			"	sacf	256\n"
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  416  			"	llill	%[count],%[max_loops]\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  417  			"0:	l	%[prev],%[address]\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  418  			"1:	nr	%[prev],%[mask]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  419  			"	xilf	%[mask],0xffffffff\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  420  			"	or	%[new],%[prev]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  421  			"	or	%[prev],%[tmp]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  422  			"2:	lr	%[tmp],%[prev]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  423  			"3:	cs	%[prev],%[new],%[address]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  424  			"4:	jnl	5f\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  425  			"	xr	%[tmp],%[prev]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  426  			"	xr	%[new],%[tmp]\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  427  			"	nr	%[tmp],%[mask]\n"
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  428  			"	jnz	5f\n"
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  429  			"	brct	%[count],2b\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  430  			"5:	sacf	768\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  431  			"	spka	%[default_key]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  432  			EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev])
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  433  			EX_TABLE_UA_LOAD_REG(1b, 5b, %[rc], %[prev])
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  434  			EX_TABLE_UA_LOAD_REG(3b, 5b, %[rc], %[prev])
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  435  			EX_TABLE_UA_LOAD_REG(4b, 5b, %[rc], %[prev])
> 4148575abe1e14 Heiko Carstens           2022-11-02  436  			: [rc] "+&d" (rc),
> 4148575abe1e14 Heiko Carstens           2022-11-02  437  			  [prev] "=&d" (prev),
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  438  			  [address] "+Q" (*(int *)address),
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  439  			  [tmp] "+&d" (_old),
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  440  			  [new] "+&d" (_new),
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  441  			  [mask] "+&d" (mask),
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  442  			  [count] "=a" (count)
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  443  			: [key] "%[count]" (key << 4),
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  444  			  [default_key] "J" (PAGE_DEFAULT_KEY),
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  445  			  [max_loops] "J" (CMPXCHG_USER_KEY_MAX_LOOPS)
> 4148575abe1e14 Heiko Carstens           2022-11-02  446  			: "memory", "cc");
> 4148575abe1e14 Heiko Carstens           2022-11-02  447  		*(unsigned char *)uval = prev >> shift;
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  448  		if (!count)
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  449  			rc = -EAGAIN;
> 4148575abe1e14 Heiko Carstens           2022-11-02  450  		return rc;
> 4148575abe1e14 Heiko Carstens           2022-11-02  451  	}
> 4148575abe1e14 Heiko Carstens           2022-11-02  452  	case 2: {
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  453  		unsigned int prev, shift, mask, _old, _new;
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  454  		unsigned long count;
> 4148575abe1e14 Heiko Carstens           2022-11-02  455  
> 4148575abe1e14 Heiko Carstens           2022-11-02  456  		shift = (2 ^ (address & 2)) << 3;
> 4148575abe1e14 Heiko Carstens           2022-11-02  457  		address ^= address & 2;
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  458  		_old = (old & 0xffff) << shift;
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  459  		_new = (new & 0xffff) << shift;

Same here.

> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  460  		mask = ~(0xffff << shift);
> 4148575abe1e14 Heiko Carstens           2022-11-02  461  		asm volatile(
> 4148575abe1e14 Heiko Carstens           2022-11-02  462  			"	spka	0(%[key])\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  463  			"	sacf	256\n"
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  464  			"	llill	%[count],%[max_loops]\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  465  			"0:	l	%[prev],%[address]\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  466  			"1:	nr	%[prev],%[mask]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  467  			"	xilf	%[mask],0xffffffff\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  468  			"	or	%[new],%[prev]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  469  			"	or	%[prev],%[tmp]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  470  			"2:	lr	%[tmp],%[prev]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  471  			"3:	cs	%[prev],%[new],%[address]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  472  			"4:	jnl	5f\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  473  			"	xr	%[tmp],%[prev]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  474  			"	xr	%[new],%[tmp]\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  475  			"	nr	%[tmp],%[mask]\n"
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  476  			"	jnz	5f\n"
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  477  			"	brct	%[count],2b\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  478  			"5:	sacf	768\n"
> 4148575abe1e14 Heiko Carstens           2022-11-02  479  			"	spka	%[default_key]\n"
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  480  			EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev])
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  481  			EX_TABLE_UA_LOAD_REG(1b, 5b, %[rc], %[prev])
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  482  			EX_TABLE_UA_LOAD_REG(3b, 5b, %[rc], %[prev])
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  483  			EX_TABLE_UA_LOAD_REG(4b, 5b, %[rc], %[prev])
> 4148575abe1e14 Heiko Carstens           2022-11-02  484  			: [rc] "+&d" (rc),
> 4148575abe1e14 Heiko Carstens           2022-11-02  485  			  [prev] "=&d" (prev),
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  486  			  [address] "+Q" (*(int *)address),
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  487  			  [tmp] "+&d" (_old),
> 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16  488  			  [new] "+&d" (_new),
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  489  			  [mask] "+&d" (mask),
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  490  			  [count] "=a" (count)
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  491  			: [key] "%[count]" (key << 4),
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  492  			  [default_key] "J" (PAGE_DEFAULT_KEY),
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  493  			  [max_loops] "J" (CMPXCHG_USER_KEY_MAX_LOOPS)
> 4148575abe1e14 Heiko Carstens           2022-11-02  494  			: "memory", "cc");
> 4148575abe1e14 Heiko Carstens           2022-11-02  495  		*(unsigned short *)uval = prev >> shift;
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  496  		if (!count)
> 739ad2e4e15b58 Janis Schoetterl-Glausch 2022-11-17  497  			rc = -EAGAIN;
> 4148575abe1e14 Heiko Carstens           2022-11-02  498  		return rc;

[...]
Heiko Carstens Jan. 4, 2023, 5:27 p.m. UTC | #5 
On Mon, Dec 19, 2022 at 10:24:33PM +0100, Janis Schoetterl-Glausch wrote:
> On Wed, 2022-12-14 at 10:23 +0800, kernel test robot wrote:
> > Hi Janis,
> > 
> > Thank you for the patch! Yet something to improve:
> > All errors (new ones prefixed by >>):
> > 
> >    s390-linux-ld: arch/s390/kvm/gaccess.o: in function `__cmpxchg_user_key':
> > > > arch/s390/include/asm/uaccess.h:410: undefined reference to `__ashlti3'
> > > > s390-linux-ld: arch/s390/include/asm/uaccess.h:411: undefined reference to `__ashlti3'
> >    s390-linux-ld: arch/s390/include/asm/uaccess.h:458: undefined reference to `__ashlti3'
> >    s390-linux-ld: arch/s390/include/asm/uaccess.h:459: undefined reference to `__ashlti3'
> > 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16 @410  		_old = (old & 0xff) << shift;
> > 51098f0eb22e2f Janis Schoetterl-Glausch 2022-11-16 @411  		_new = (new & 0xff) << shift;
> 
> Not sure what it is in this config that causes gcc to emit this
> symbol instead of a shift instruction, but casting old/new to 32 bit
> fixes the error.

Right.. now we have the same fun with 128 bit arithmetics that we had
with 64 bit arithmetics on 32 bit. I really missed that :)

Fixed the way you proposed it:
https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/?h=cmpxchg_user_key&id=b33d59fb37ddcb6ee65d4fa23cc3d58793d13c5b
diff mbox series

Patch

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 0d5d4419139a..f106db1af5ee 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -588,6 +588,8 @@  struct kvm_s390_mem_op {
 		struct {
 			__u8 ar;	/* the access register number */
 			__u8 key;	/* access key, ignored if flag unset */
+			__u8 pad1[6];	/* ignored */
+			__u64 old_addr;	/* ignored if flag unset */
 		};
 		__u32 sida_offset; /* offset into the sida */
 		__u8 reserved[32]; /* ignored */
@@ -604,6 +606,11 @@  struct kvm_s390_mem_op {
 #define KVM_S390_MEMOP_F_CHECK_ONLY		(1ULL << 0)
 #define KVM_S390_MEMOP_F_INJECT_EXCEPTION	(1ULL << 1)
 #define KVM_S390_MEMOP_F_SKEY_PROTECTION	(1ULL << 2)
+#define KVM_S390_MEMOP_F_CMPXCHG		(1ULL << 3)
+/* flags specifying extension support */
+#define KVM_S390_MEMOP_EXTENSION_CAP_CMPXCHG 0x2
+/* Non program exception return codes (pgm codes are 16 bit) */
+#define KVM_S390_MEMOP_R_NO_XCHG		(1 << 16)
 
 /* for KVM_INTERRUPT */
 struct kvm_interrupt {
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index 9408d6cc8e2c..92a3b9fb31ec 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -206,6 +206,9 @@  int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
 int access_guest_real(struct kvm_vcpu *vcpu, unsigned long gra,
 		      void *data, unsigned long len, enum gacc_mode mode);
 
+int cmpxchg_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, int len,
+			       __uint128_t *old, __uint128_t new, u8 access_key);
+
 /**
  * write_guest_with_key - copy data from kernel space to guest space
  * @vcpu: virtual cpu
diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 0243b6e38d36..6165e761a637 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -1161,6 +1161,108 @@  int access_guest_real(struct kvm_vcpu *vcpu, unsigned long gra,
 	return rc;
 }
 
+/**
+ * cmpxchg_guest_abs_with_key() - Perform cmpxchg on guest absolute address.
+ * @kvm: Virtual machine instance.
+ * @gpa: Absolute guest address of the location to be changed.
+ * @len: Operand length of the cmpxchg, required: 1 <= len <= 16. Providing a
+ *       non power of two will result in failure.
+ * @old_addr: Pointer to old value. If the location at @gpa contains this value, the
+ *         exchange will succeed. After calling cmpxchg_guest_abs_with_key() *@old
+ *         contains the value at @gpa before the attempt to exchange the value.
+ * @new: The value to place at @gpa.
+ * @access_key: The access key to use for the guest access.
+ *
+ * Atomically exchange the value at @gpa by @new, if it contains *@old.
+ * Honors storage keys.
+ *
+ * Return: * 0: successful exchange
+ *         * 1: exchange unsuccessful
+ *         * a program interruption code indicating the reason cmpxchg could
+ *           not be attempted
+ *         * -EINVAL: address misaligned or len not power of two
+ *         * -EAGAIN: transient failure (len 1 or 2)
+ *         * -EOPNOTSUPP: read-only memslot (should never occur)
+ */
+int cmpxchg_guest_abs_with_key(struct kvm *kvm, gpa_t gpa, int len,
+			       __uint128_t *old_addr, __uint128_t new,
+			       u8 access_key)
+{
+	gfn_t gfn = gpa >> PAGE_SHIFT;
+	struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
+	bool writable;
+	hva_t hva;
+	int ret;
+
+	if (!IS_ALIGNED(gpa, len))
+		return -EINVAL;
+
+	hva = gfn_to_hva_memslot_prot(slot, gfn, &writable);
+	if (kvm_is_error_hva(hva))
+		return PGM_ADDRESSING;
+	/*
+	 * Check if it's a read-only memslot, even though that cannot occur
+	 * since those are unsupported.
+	 * Don't try to actually handle that case.
+	 */
+	if (!writable)
+		return -EOPNOTSUPP;
+
+	hva += offset_in_page(gpa);
+	switch (len) {
+	case 1: {
+		u8 old;
+
+		ret = cmpxchg_user_key((u8 *)hva, &old, *old_addr, new, access_key);
+		ret = ret < 0 ? ret : old != *old_addr;
+		*old_addr = old;
+		break;
+	}
+	case 2: {
+		u16 old;
+
+		ret = cmpxchg_user_key((u16 *)hva, &old, *old_addr, new, access_key);
+		ret = ret < 0 ? ret : old != *old_addr;
+		*old_addr = old;
+		break;
+	}
+	case 4: {
+		u32 old;
+
+		ret = cmpxchg_user_key((u32 *)hva, &old, *old_addr, new, access_key);
+		ret = ret < 0 ? ret : old != *old_addr;
+		*old_addr = old;
+		break;
+	}
+	case 8: {
+		u64 old;
+
+		ret = cmpxchg_user_key((u64 *)hva, &old, *old_addr, new, access_key);
+		ret = ret < 0 ? ret : old != *old_addr;
+		*old_addr = old;
+		break;
+	}
+	case 16: {
+		__uint128_t old;
+
+		ret = cmpxchg_user_key((__uint128_t *)hva, &old, *old_addr, new, access_key);
+		ret = ret < 0 ? ret : old != *old_addr;
+		*old_addr = old;
+		break;
+	}
+	default:
+		return -EINVAL;
+	}
+	mark_page_dirty_in_slot(kvm, slot, gfn);
+	/*
+	 * Assume that the fault is caused by protection, either key protection
+	 * or user page write protection.
+	 */
+	if (ret == -EFAULT)
+		ret = PGM_PROTECTION;
+	return ret;
+}
+
 /**
  * guest_translate_address_with_key - translate guest logical into guest absolute address
  * @vcpu: virtual cpu
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 45d4b8182b07..47bcf2cb4345 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -576,7 +576,6 @@  int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_S390_VCPU_RESETS:
 	case KVM_CAP_SET_GUEST_DEBUG:
 	case KVM_CAP_S390_DIAG318:
-	case KVM_CAP_S390_MEM_OP_EXTENSION:
 		r = 1;
 		break;
 	case KVM_CAP_SET_GUEST_DEBUG2:
@@ -590,6 +589,14 @@  int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_S390_MEM_OP:
 		r = MEM_OP_MAX_SIZE;
 		break;
+	case KVM_CAP_S390_MEM_OP_EXTENSION:
+		/*
+		 * Flag bits indicating which extensions are supported.
+		 * The first extension doesn't use a flag, but pretend it does,
+		 * this way that can be changed in the future.
+		 */
+		r = KVM_S390_MEMOP_EXTENSION_CAP_CMPXCHG | 1;
+		break;
 	case KVM_CAP_NR_VCPUS:
 	case KVM_CAP_MAX_VCPUS:
 	case KVM_CAP_MAX_VCPU_ID:
@@ -2714,12 +2721,19 @@  static bool access_key_invalid(u8 access_key)
 static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
 {
 	void __user *uaddr = (void __user *)mop->buf;
+	void __user *old_addr = (void __user *)mop->old_addr;
+	union {
+		__uint128_t quad;
+		char raw[sizeof(__uint128_t)];
+	} old = { .quad = 0}, new = { .quad = 0 };
+	unsigned int off_in_quad = sizeof(new) - mop->size;
 	u64 supported_flags;
 	void *tmpbuf = NULL;
 	int r, srcu_idx;
 
 	supported_flags = KVM_S390_MEMOP_F_SKEY_PROTECTION
-			  | KVM_S390_MEMOP_F_CHECK_ONLY;
+			  | KVM_S390_MEMOP_F_CHECK_ONLY
+			  | KVM_S390_MEMOP_F_CMPXCHG;
 	if (mop->flags & ~supported_flags || !mop->size)
 		return -EINVAL;
 	if (mop->size > MEM_OP_MAX_SIZE)
@@ -2741,6 +2755,19 @@  static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
 	} else {
 		mop->key = 0;
 	}
+	if (mop->flags & KVM_S390_MEMOP_F_CMPXCHG) {
+		/*
+		 * This validates off_in_quad. Checking that size is a power
+		 * of two is not necessary, as cmpxchg_guest_abs_with_key
+		 * takes care of that
+		 */
+		if (mop->size > sizeof(new))
+			return -EINVAL;
+		if (copy_from_user(&new.raw[off_in_quad], uaddr, mop->size))
+			return -EFAULT;
+		if (copy_from_user(&old.raw[off_in_quad], old_addr, mop->size))
+			return -EFAULT;
+	}
 	if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
 		tmpbuf = vmalloc(mop->size);
 		if (!tmpbuf)
@@ -2771,6 +2798,14 @@  static int kvm_s390_vm_mem_op(struct kvm *kvm, struct kvm_s390_mem_op *mop)
 	case KVM_S390_MEMOP_ABSOLUTE_WRITE: {
 		if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) {
 			r = check_gpa_range(kvm, mop->gaddr, mop->size, GACC_STORE, mop->key);
+		} else if (mop->flags & KVM_S390_MEMOP_F_CMPXCHG) {
+			r = cmpxchg_guest_abs_with_key(kvm, mop->gaddr, mop->size,
+						       &old.quad, new.quad, mop->key);
+			if (r == 1) {
+				r = KVM_S390_MEMOP_R_NO_XCHG;
+				if (copy_to_user(old_addr, &old.raw[off_in_quad], mop->size))
+					r = -EFAULT;
+			}
 		} else {
 			if (copy_from_user(tmpbuf, uaddr, mop->size)) {
 				r = -EFAULT;