Re: [PATCH 1/4] Rip out simple_strtoll()

2023-06-09 Thread kernel test robot
Hi Demi,

kernel test robot noticed the following build errors:

[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on lee-leds/for-leds-next linus/master v6.4-rc5 
next-20230609]
[cannot apply to xen-tip/linux-next lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Demi-Marie-Obenour/vsscanf-Return-ERANGE-on-integer-overflow/20230610-110026
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:
https://lore.kernel.org/r/20230610025759.1813-1-demi%40invisiblethingslab.com
patch subject: [PATCH 1/4] Rip out simple_strtoll()
config: csky-randconfig-r011-20230610 
(https://download.01.org/0day-ci/archive/20230610/202306101317.yibrl6oz-...@intel.com/config)
compiler: csky-linux-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add lee-mfd 
https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
git fetch lee-mfd for-mfd-next
git checkout lee-mfd/for-mfd-next
b4 shazam 
https://lore.kernel.org/r/20230610025759.1813-1-d...@invisiblethingslab.com
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross 
W=1 O=build_dir ARCH=csky olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross 
W=1 O=build_dir ARCH=csky SHELL=/bin/bash drivers/md/bcache/

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202306101317.yibrl6oz-...@intel.com/

All errors (new ones prefixed by >>):

   drivers/md/bcache/util.c: In function 'bch_strtoll_h':
>> drivers/md/bcache/util.c:28:18: error: implicit declaration of function 
>> 'simple_strtoll'; did you mean 'simple_strtoull'? 
>> [-Werror=implicit-function-declaration]
  28 | type i = simple_ ## name(cp, &e, 10);   \
 |  ^~~
   drivers/md/bcache/util.c:82:1: note: in expansion of macro 'STRTO_H'
  82 | STRTO_H(strtoll, long long)
 | ^~~
   cc1: some warnings being treated as errors


vim +28 drivers/md/bcache/util.c

cafe563591446c Kent Overstreet 2013-03-23  22  
cafe563591446c Kent Overstreet 2013-03-23  23  #define STRTO_H(name, type)  
\
169ef1cf6171d3 Kent Overstreet 2013-03-28  24  int bch_ ## name ## _h(const 
char *cp, type *res)\
cafe563591446c Kent Overstreet 2013-03-23  25  {
\
cafe563591446c Kent Overstreet 2013-03-23  26   int u = 0;  
\
cafe563591446c Kent Overstreet 2013-03-23  27   char *e;
\
cafe563591446c Kent Overstreet 2013-03-23 @28   type i = simple_ ## 
name(cp, &e, 10);   \
cafe563591446c Kent Overstreet 2013-03-23  29   
\
cafe563591446c Kent Overstreet 2013-03-23  30   switch (tolower(*e)) {  
\
cafe563591446c Kent Overstreet 2013-03-23  31   default:
\
cafe563591446c Kent Overstreet 2013-03-23  32   return -EINVAL; 
\
cafe563591446c Kent Overstreet 2013-03-23  33   case 'y':   
\
cafe563591446c Kent Overstreet 2013-03-23  34   case 'z':   
\
cafe563591446c Kent Overstreet 2013-03-23  35   u++;
\
df561f6688fef7 Gustavo A. R. Silva 2020-08-23  36   fallthrough;
\
cafe563591446c Kent Overstreet 2013-03-23  37   case 'e':   
\
cafe563591446c Kent Overstreet 2013-03-23  38   u++;
\
df561f6688fef7 Gustavo A. R. Silva 2020-08-23  39   fallthrough;
\
cafe563591446c Kent Overstreet 2013-03-23  40   case 'p':   
\
cafe563591446c Kent Overstreet 2013-03-23  41   

Re: [PATCH 1/4] Rip out simple_strtoll()

2023-06-09 Thread kernel test robot
Hi Demi,

kernel test robot noticed the following build errors:

[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on lee-leds/for-leds-next linus/master v6.4-rc5 
next-20230609]
[cannot apply to xen-tip/linux-next lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Demi-Marie-Obenour/vsscanf-Return-ERANGE-on-integer-overflow/20230610-110026
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:
https://lore.kernel.org/r/20230610025759.1813-1-demi%40invisiblethingslab.com
patch subject: [PATCH 1/4] Rip out simple_strtoll()
config: arm-randconfig-r046-20230610 
(https://download.01.org/0day-ci/archive/20230610/202306101334.ldzersko-...@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 
8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
git remote add lee-mfd 
https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
git fetch lee-mfd for-mfd-next
git checkout lee-mfd/for-mfd-next
b4 shazam 
https://lore.kernel.org/r/20230610025759.1813-1-d...@invisiblethingslab.com
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 
O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 
O=build_dir ARCH=arm SHELL=/bin/bash drivers/md/bcache/

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202306101334.ldzersko-...@intel.com/

All errors (new ones prefixed by >>):

>> drivers/md/bcache/util.c:82:1: error: call to undeclared function 
>> 'simple_strtoll'; ISO C99 and later do not support implicit function 
>> declarations [-Werror,-Wimplicit-function-declaration]
   STRTO_H(strtoll, long long)
   ^
   drivers/md/bcache/util.c:28:11: note: expanded from macro 'STRTO_H'
   type i = simple_ ## name(cp, &e, 10);   \
^
   :22:1: note: expanded from here
   simple_strtoll
   ^
   1 error generated.


vim +/simple_strtoll +82 drivers/md/bcache/util.c

cafe563591446c Kent Overstreet 2013-03-23  79  
cafe563591446c Kent Overstreet 2013-03-23  80  STRTO_H(strtoint, int)
cafe563591446c Kent Overstreet 2013-03-23  81  STRTO_H(strtouint, unsigned int)
cafe563591446c Kent Overstreet 2013-03-23 @82  STRTO_H(strtoll, long long)
cafe563591446c Kent Overstreet 2013-03-23  83  STRTO_H(strtoull, unsigned long 
long)
cafe563591446c Kent Overstreet 2013-03-23  84  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki



[linux-linus test] 181337: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181337 linux-linus real [real]
flight 181357 linux-linus real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/181337/
http://logs.test-lab.xenproject.org/osstest/logs/181357/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 180278

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-examine  8 reboot   fail  like 180278
 test-armhf-armhf-xl-multivcpu  8 xen-boot fail like 180278
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278
 test-armhf-armhf-xl-credit2   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278
 test-armhf-armhf-libvirt  8 xen-boot fail  like 180278
 test-armhf-armhf-xl-arndale   8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-raw  8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-qcow2  8 xen-bootfail like 180278
 test-armhf-armhf-xl   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-vhd   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-rtds  8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 linux33f2b5785a2b6b0ed1948aafee60d3abb12f1e3a
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z   54 days
Failing since180281  2023-04-17 06:24:36 Z   53 days  101 attempts
Testing same since   181337  2023-06-09 09:15:16 Z0 days1 attempts


2659 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-amd64-cores

Re: [PATCH 4/4] Strict XenStore entry parsing

2023-06-09 Thread Matthew Wilcox
On Fri, Jun 09, 2023 at 10:57:59PM -0400, Demi Marie Obenour wrote:
> This uses the newly-introduced strict version of sscanf().

I can see that.  Why does it do that?

Documentation/process/5.Posting.rst

(in general, there is a lack of detail across all four of these patches
justifying why any of this work is being done.  it isn't obvious to me
why skipping leading whitespace is bad in this context)




[qemu-mainline test] 181345: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181345 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181345/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 180691
 build-arm64   6 xen-buildfail REGR. vs. 180691
 build-amd64   6 xen-buildfail REGR. vs. 180691
 build-i3866 xen-buildfail REGR. vs. 180691
 build-amd64-xsm   6 xen-buildfail REGR. vs. 180691
 build-i386-xsm6 xen-buildfail REGR. vs. 180691
 build-armhf   6 xen-buildfail REGR. vs. 180691

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-vhd1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-raw   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-coresched-amd64-xl  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-de

[PATCH 3/4] Add strict version of vsscanf()

2023-06-09 Thread Demi Marie Obenour
Signed-off-by: Demi Marie Obenour 
---
 include/linux/kernel.h |  4 
 lib/vsprintf.c | 43 +++---
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 
0d91e0af01250c1d82f4a2ea562d2619b9cc6e9c..b348b84ce9c4e95031f67e0cbac5de8deca69aac
 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -227,8 +227,12 @@ const char *kvasprintf_const(gfp_t gfp, const char *fmt, 
va_list args);
 
 extern __scanf(2, 3)
 int sscanf(const char *, const char *, ...);
+extern __scanf(2, 3)
+int sscanf_strict(const char *, const char *, ...);
 extern __scanf(2, 0)
 int vsscanf(const char *, const char *, va_list);
+extern __scanf(2, 0)
+int vsscanf_strict(const char *, const char *, va_list);
 
 extern int no_hash_pointers_enable(char *str);
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 
9846d2385f5b9e8f3945a5664d81047e97cf10d5..2dae357b367e1da8b1004ed6e85e051a045ca36b
 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3414,6 +3414,8 @@ EXPORT_SYMBOL_GPL(bprintf);
 
 #endif /* CONFIG_BINARY_PRINTF */
 
+static int vsscanf_internal(const char *buf, const char *fmt, va_list args, 
bool strict);
+
 /**
  * vsscanf - Unformat a buffer into a list of arguments
  * @buf:   input buffer
@@ -3421,6 +3423,23 @@ EXPORT_SYMBOL_GPL(bprintf);
  * @args:  arguments
  */
 int vsscanf(const char *buf, const char *fmt, va_list args)
+{
+   return vsscanf_internal(buf, fmt, args, false);
+}
+
+/**
+ * vsscanf_strict - Unformat a buffer into a list of arguments, but
+ *  do not skip spaces.
+ * @buf:   input buffer
+ * @fmt:   format of buffer
+ * @args:  arguments
+ */
+int vsscanf_strict(const char *buf, const char *fmt, va_list args)
+{
+   return vsscanf_internal(buf, fmt, args, true);
+}
+
+static int vsscanf_internal(const char *buf, const char *fmt, va_list args, 
bool strict)
 {
const char *str = buf;
char *next;
@@ -3530,8 +3549,10 @@ int vsscanf(const char *buf, const char *fmt, va_list 
args)
char *s = (char *)va_arg(args, char *);
if (field_width == -1)
field_width = SHRT_MAX;
-   /* first, skip leading white space in buffer */
-   str = skip_spaces(str);
+   if (!strict) {
+   /* first, skip leading white space in buffer */
+   str = skip_spaces(str);
+   }
 
/* now copy until next white space */
while (*str && !isspace(*str) && field_width--)
@@ -3621,7 +3642,8 @@ int vsscanf(const char *buf, const char *fmt, va_list 
args)
/* have some sort of integer conversion.
 * first, skip white space in buffer.
 */
-   str = skip_spaces(str);
+   if (!strict)
+   str = skip_spaces(str);
 
digit = *str;
if (is_sign && digit == '-') {
@@ -3721,6 +3743,9 @@ int vsscanf(const char *buf, const char *fmt, va_list 
args)
str = next;
}
 
+   if (strict && *str)
+   return -EINVAL;
+
return num;
 }
 EXPORT_SYMBOL(vsscanf);
@@ -3743,3 +3768,15 @@ int sscanf(const char *buf, const char *fmt, ...)
return i;
 }
 EXPORT_SYMBOL(sscanf);
+int sscanf_strict(const char *buf, const char *fmt, ...)
+{
+   va_list args;
+   int i;
+
+   va_start(args, fmt);
+   i = vsscanf_strict(buf, fmt, args);
+   va_end(args);
+
+   return i;
+}
+EXPORT_SYMBOL(sscanf_strict);
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab




[PATCH 4/4] Strict XenStore entry parsing

2023-06-09 Thread Demi Marie Obenour
This uses the newly-introduced strict version of sscanf().

Signed-off-by: Demi Marie Obenour 
---
 drivers/xen/xenbus/xenbus_xs.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index 
12e02eb01f5991b31db451cc57037205359b347f..88e94269c9221d16d1a97e59399058e870675729
 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -569,16 +569,20 @@ int xenbus_scanf(struct xenbus_transaction t,
 const char *dir, const char *node, const char *fmt, ...)
 {
va_list ap;
-   int ret;
+   int ret = 0;
+   unsigned int len;
char *val;
 
-   val = xenbus_read(t, dir, node, NULL);
+   val = xenbus_read(t, dir, node, &len);
if (IS_ERR(val))
return PTR_ERR(val);
+   if (strlen(val) != len)
+   goto bad;
 
va_start(ap, fmt);
-   ret = vsscanf(val, fmt, ap);
+   ret = vsscanf_strict(val, fmt, ap);
va_end(ap);
+bad:
kfree(val);
/* Distinctive errno. */
if (ret == 0)
@@ -636,15 +640,18 @@ int xenbus_gather(struct xenbus_transaction t, const char 
*dir, ...)
while (ret == 0 && (name = va_arg(ap, char *)) != NULL) {
const char *fmt = va_arg(ap, char *);
void *result = va_arg(ap, void *);
+   unsigned len;
char *p;
 
-   p = xenbus_read(t, dir, name, NULL);
+   p = xenbus_read(t, dir, name, &len);
if (IS_ERR(p)) {
ret = PTR_ERR(p);
break;
}
-   if (fmt) {
-   if (sscanf(p, fmt, result) == 0)
+   if (strlen(p) != len)
+   ret = -EINVAL;
+   else if (fmt) {
+   if (sscanf_strict(p, fmt, result) <= 0)
ret = -EINVAL;
kfree(p);
} else
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab




[PATCH 1/4] Rip out simple_strtoll()

2023-06-09 Thread Demi Marie Obenour
It is not used anywhere but its own unit tests.

Signed-off-by: Demi Marie Obenour 
---
 Documentation/dev-tools/checkpatch.rst |  9 -
 Documentation/process/deprecated.rst   |  5 ++---
 .../translations/it_IT/process/deprecated.rst  |  9 -
 .../translations/sp_SP/process/deprecated.rst  | 14 +++---
 include/linux/kstrtox.h|  1 -
 lib/kstrtox.c  |  2 +-
 lib/test_scanf.c   | 10 --
 lib/vsprintf.c | 14 --
 8 files changed, 18 insertions(+), 46 deletions(-)

diff --git a/Documentation/dev-tools/checkpatch.rst 
b/Documentation/dev-tools/checkpatch.rst
index 
c3389c6f38381f038ed5d9a884f2d333a749f8a2..0ae0ca80beb0c0171e8c04306cb5b9ccbc9fa713
 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -290,11 +290,10 @@ API usage
 See: 
https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-on
 
   **CONSIDER_KSTRTO**
-The simple_strtol(), simple_strtoll(), simple_strtoul(), and
-simple_strtoull() functions explicitly ignore overflows, which
-may lead to unexpected results in callers.  The respective kstrtol(),
-kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the
-correct replacements.
+The simple_strtol(), simple_strtoul(), and simple_strtoull() functions
+explicitly ignore overflows, which may lead to unexpected results in
+callers.  The respective kstrtol(), kstrtoll(), kstrtoul(), and kstrtoull()
+functions tend to be the correct replacements.
 
 See: 
https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull
 
diff --git a/Documentation/process/deprecated.rst 
b/Documentation/process/deprecated.rst
index 
f91b8441f2ef70576c5bad079e631e4077eabed6..b2f677eb6d6b12df63003b00960ef27b1656e4c3
 100644
--- a/Documentation/process/deprecated.rst
+++ b/Documentation/process/deprecated.rst
@@ -109,10 +109,9 @@ For more details, also see array3_size() and 
flex_array_size(),
 as well as the related check_mul_overflow(), check_add_overflow(),
 check_sub_overflow(), and check_shl_overflow() family of functions.
 
-simple_strtol(), simple_strtoll(), simple_strtoul(), simple_strtoull()
+simple_strtol(), simple_strtoul(), simple_strtoull()
 --
-The simple_strtol(), simple_strtoll(),
-simple_strtoul(), and simple_strtoull() functions
+The simple_strtol(), simple_strtoul(), and simple_strtoull() functions
 explicitly ignore overflows, which may lead to unexpected results
 in callers. The respective kstrtol(), kstrtoll(),
 kstrtoul(), and kstrtoull() functions tend to be the
diff --git a/Documentation/translations/it_IT/process/deprecated.rst 
b/Documentation/translations/it_IT/process/deprecated.rst
index 
ba0ed7dc154c95e0e973cb51cc36b95e786079a8..cdc0aa107bce7f4018ad241a02b4f701974e52b4
 100644
--- a/Documentation/translations/it_IT/process/deprecated.rst
+++ b/Documentation/translations/it_IT/process/deprecated.rst
@@ -118,12 +118,11 @@ Per maggiori dettagli fate riferimento a array3_size() e 
flex_array_size(), ma
 anche le funzioni della famiglia check_mul_overflow(), check_add_overflow(),
 check_sub_overflow(), e check_shl_overflow().
 
-simple_strtol(), simple_strtoll(), simple_strtoul(), simple_strtoull()
+simple_strtol(), simple_strtoul(), simple_strtoull()
 --
-Le funzioni simple_strtol(), simple_strtoll(),
-simple_strtoul(), e simple_strtoull() ignorano volutamente
-i possibili overflow, e questo può portare il chiamante a generare risultati
-inaspettati. Le rispettive funzioni kstrtol(), kstrtoll(),
+Le funzioni simple_strtol(), simple_strtoul(), e simple_strtoull() ignorano
+volutamente i possibili overflow, e questo può portare il chiamante a generare
+risultati inaspettati. Le rispettive funzioni kstrtol(), kstrtoll(),
 kstrtoul(), e kstrtoull() sono da considerarsi le corrette
 sostitute; tuttavia va notato che queste richiedono che la stringa sia
 terminata con il carattere NUL o quello di nuova riga.
diff --git a/Documentation/translations/sp_SP/process/deprecated.rst 
b/Documentation/translations/sp_SP/process/deprecated.rst
index 
d52120e0d75354d0d32c33d631f9f364eba32f82..f0ba93e7188f02ae98a8533e2dfee4c82cf78c5c
 100644
--- a/Documentation/translations/sp_SP/process/deprecated.rst
+++ b/Documentation/translations/sp_SP/process/deprecated.rst
@@ -117,14 +117,14 @@ como también la familia de funciones relacionadas 
check_mul_overflow(),
 check_add_overflow(), check_sub_overflow(), y check_shl_overflow().
 
 
-simple_strtol(), simple_strtoll(), simple_strtoul(), simple_strtoull()
+simple_strtol(), simple_strtoul(), simple_strtoull()
 

[PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow

2023-06-09 Thread Demi Marie Obenour
Userspace sets errno to ERANGE, but the kernel can't do that.

Signed-off-by: Demi Marie Obenour 
---
 include/linux/limits.h  |  1 +
 include/linux/mfd/wl1273-core.h |  3 --
 include/vdso/limits.h   |  3 ++
 lib/vsprintf.c  | 80 -
 4 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/include/linux/limits.h b/include/linux/limits.h
index 
f6bcc936901071f496e3e85bb6e1d93905b12e32..8f7fd85b41fb46e6992d9e5912da00424119227a
 100644
--- a/include/linux/limits.h
+++ b/include/linux/limits.h
@@ -8,6 +8,7 @@
 
 #define SIZE_MAX   (~(size_t)0)
 #define SSIZE_MAX  ((ssize_t)(SIZE_MAX >> 1))
+#define SSIZE_MIN  (-SSIZE_MAX - 1)
 #define PHYS_ADDR_MAX  (~(phys_addr_t)0)
 
 #define U8_MAX ((u8)~0U)
diff --git a/include/linux/mfd/wl1273-core.h b/include/linux/mfd/wl1273-core.h
index 
c28cf76d5c31ee1c94a9319a2e2d318bf00283a6..b81a229135ed9f756c749122a8341816031c8311
 100644
--- a/include/linux/mfd/wl1273-core.h
+++ b/include/linux/mfd/wl1273-core.h
@@ -204,9 +204,6 @@
 WL1273_IS2_TRI_OPT | \
 WL1273_IS2_RATE_48K)
 
-#define SCHAR_MIN (-128)
-#define SCHAR_MAX 127
-
 #define WL1273_FR_EVENTBIT(0)
 #define WL1273_BL_EVENTBIT(1)
 #define WL1273_RDS_EVENT   BIT(2)
diff --git a/include/vdso/limits.h b/include/vdso/limits.h
index 
0197888ad0e00b2f853d3f25ffa764f61cca7385..0cad0a2490e5efc194d874025eb3e3b846a5c7b4
 100644
--- a/include/vdso/limits.h
+++ b/include/vdso/limits.h
@@ -2,6 +2,9 @@
 #ifndef __VDSO_LIMITS_H
 #define __VDSO_LIMITS_H
 
+#define UCHAR_MAX  ((unsigned char)~0U)
+#define SCHAR_MAX  ((signed char)(UCHAR_MAX >> 1))
+#define SCHAR_MIN  ((signed char)(-SCHAR_MAX - 1))
 #define USHRT_MAX  ((unsigned short)~0U)
 #define SHRT_MAX   ((short)(USHRT_MAX >> 1))
 #define SHRT_MIN   ((short)(-SHRT_MAX - 1))
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 
a60d348efb276d66ca07fe464883408df7fdab97..9846d2385f5b9e8f3945a5664d81047e97cf10d5
 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -59,7 +59,7 @@
 bool no_hash_pointers __ro_after_init;
 EXPORT_SYMBOL_GPL(no_hash_pointers);
 
-static noinline unsigned long long simple_strntoull(const char *startp, size_t 
max_chars, char **endp, unsigned int base)
+static noinline unsigned long long simple_strntoull(const char *startp, size_t 
max_chars, char **endp, unsigned int base, bool *overflow)
 {
const char *cp;
unsigned long long result = 0ULL;
@@ -71,6 +71,8 @@ static noinline unsigned long long simple_strntoull(const 
char *startp, size_t m
if (prefix_chars < max_chars) {
rv = _parse_integer_limit(cp, base, &result, max_chars - 
prefix_chars);
/* FIXME */
+   if (overflow)
+   *overflow = !!(rv & KSTRTOX_OVERFLOW);
cp += (rv & ~KSTRTOX_OVERFLOW);
} else {
/* Field too short for prefix + digit, skip over without 
converting */
@@ -94,7 +96,7 @@ static noinline unsigned long long simple_strntoull(const 
char *startp, size_t m
 noinline
 unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int 
base)
 {
-   return simple_strntoull(cp, INT_MAX, endp, base);
+   return simple_strntoull(cp, INT_MAX, endp, base, NULL);
 }
 EXPORT_SYMBOL(simple_strtoull);
 
@@ -130,18 +132,22 @@ long simple_strtol(const char *cp, char **endp, unsigned 
int base)
 EXPORT_SYMBOL(simple_strtol);
 
 static long long simple_strntoll(const char *cp, size_t max_chars, char **endp,
-unsigned int base)
+unsigned int base, bool *overflow)
 {
+   unsigned long long minand;
+   bool negate;
+
/*
 * simple_strntoull() safely handles receiving max_chars==0 in the
 * case cp[0] == '-' && max_chars == 1.
 * If max_chars == 0 we can drop through and pass it to 
simple_strntoull()
 * and the content of *cp is irrelevant.
 */
-   if (*cp == '-' && max_chars > 0)
-   return -simple_strntoull(cp + 1, max_chars - 1, endp, base);
-
-   return simple_strntoull(cp, max_chars, endp, base);
+   negate = *cp == '-' && max_chars > 0;
+   minand = simple_strntoull(cp + negate, max_chars - negate, endp, base, 
overflow);
+   if (minand > (unsigned long long)LONG_MAX + negate)
+   *overflow = true;
+   return negate ? -minand : minand;
 }
 
 static noinline_for_stack
@@ -3427,7 +3433,7 @@ int vsscanf(const char *buf, const char *fmt, va_list 
args)
unsigned long long u;
} val;
s16 field_width;
-   bool is_sign;
+   bool is_sign, overflow;
 
while (*fmt) {
/* skip any white space in format */
@@ -3635,45 +3641,77 @@ int vsscanf(const char *buf, const char *fmt, va_list 
args)
if (is_sign)

[linux-5.4 test] 181336: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181336 linux-5.4 real [real]
flight 181353 linux-5.4 real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/181336/
http://logs.test-lab.xenproject.org/osstest/logs/181353/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit2 18 guest-start/debian.repeat fail REGR. vs. 181182

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-qemuu-freebsd12-amd64 19 guest-localmigrate/x10 fail pass in 
181353-retest
 test-armhf-armhf-xl-multivcpu 14 guest-startfail pass in 181353-retest
 test-amd64-i386-xl-vhd 21 guest-start/debian.repeat fail pass in 181353-retest
 test-armhf-armhf-libvirt-raw 17 guest-start/debian.repeat fail pass in 
181353-retest

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-multivcpu 15 migrate-support-check fail in 181353 never 
pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-check fail in 181353 
never pass
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 181182
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 181182
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 181182
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 181182
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 181182
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 181182
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 181182
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 181182
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 181182
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 181182
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 181182
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 181182
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-che

Re: Asking for help to debug xen efi on Kunpeng machine

2023-06-09 Thread Jiatong Shen
Hello Julien,

Thank you very much for your help!

Best,

Jiatong Shen

On Fri, Jun 9, 2023 at 4:48 PM Julien Grall  wrote:

> Hello,
>
> On 09/06/2023 03:32, Jiatong Shen wrote:
> > Thank you for your answer. Can you teach me how to verify if acpi is
> > enabled?
>
> You usually look at the .config. But I am not sure if this is provided
> by the Debian package. If not, then your best option would be to build
> your own Xen. To select ACPI, you want to use the menuconfig and select
> UNSUPPORTED and ACPI.
>
> Cheers,
>
> --
> Julien Grall
>


-- 

Best Regards,

Jiatong Shen


[xen-unstable-smoke test] 181349: tolerable all pass - PUSHED

2023-06-09 Thread osstest service owner
flight 181349 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181349/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  b4642c32c4d079916d5607ddda0232aae5e1690e
baseline version:
 xen  64a647f8d817c6989edc000613b5afae19f03f99

Last test of basis   181233  2023-06-07 02:04:37 Z2 days
Failing since181246  2023-06-07 11:02:03 Z2 days   28 attempts
Testing same since   181349  2023-06-09 20:00:24 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Christian Lindig 
  George Dunlap 
  Henry Wang  # CHANGELOG
  Juergen Gross 
  Julien Grall 
  Luca Fancellu 
  Michal Orzel 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   64a647f8d8..b4642c32c4  b4642c32c4d079916d5607ddda0232aae5e1690e -> smoke



[xen-unstable-smoke test] 181347: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181347 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181347/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 181233
 build-armhf   6 xen-buildfail REGR. vs. 181233

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 xen  3a82e4501c4ec4d53b764c5b69955997b03d1137
baseline version:
 xen  64a647f8d817c6989edc000613b5afae19f03f99

Last test of basis   181233  2023-06-07 02:04:37 Z2 days
Failing since181246  2023-06-07 11:02:03 Z2 days   27 attempts
Testing same since   181342  2023-06-09 14:01:58 Z0 days3 attempts


People who touched revisions under test:
  Andrew Cooper 
  Christian Lindig 
  George Dunlap 
  Henry Wang  # CHANGELOG
  Juergen Gross 
  Julien Grall 
  Luca Fancellu 
  Michal Orzel 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 

jobs:
 build-arm64-xsm  fail
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 test-arm64-arm64-xl-xsm  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 520 lines long.)



Re: [PATCH v3 00/16] tools/xenstore: more cleanups

2023-06-09 Thread Julien Grall

Hi Juergen,

On 30/05/2023 09:54, Juergen Gross wrote:

Some more cleanups of Xenstore.

Based on top of the previous Xenstore series "tools/xenstore: rework
internal accounting".

Changes in V2:
- rebase
- one small modification of patch 10
- added patches 11-13

Changes in V3:
- rebase
- modified patch 4
- added patches 10, 11 and 13

Juergen Gross (16):
   tools/xenstore: verify command line parameters better
   tools/xenstore: do some cleanup of hashtable.c
   tools/xenstore: modify interface of create_hashtable()
   tools/xenstore: rename hashtable_insert() and let it return 0 on
 success
   tools/xenstore: make some write limit functions static
   tools/xenstore: switch write limiting to use millisecond time base
   tools/xenstore: remove stale TODO file
   tools/xenstore: remove unused events list
   tools/xenstore: remove support of file backed data base


I have committed up to here. I still need to review the rest.

Cheers,

--
Julien Grall



Re: [PATCH v3 11/16] tools/libs/store: make libxenstore independent of utils.h

2023-06-09 Thread Julien Grall

Hi Juergen,

On 30/05/2023 09:54, Juergen Gross wrote:

There is no real need for including tools/xenstore/utils.h from
libxenstore, as only streq() and ARRAY_SIZE() are obtained via that
header.

streq() is just !strcmp(), and ARRAY_SIZE() is brought in via
xen-tools/common-macros.h.

Signed-off-by: Juergen Gross 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall



Re: [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h

2023-06-09 Thread Julien Grall

Hi Juergen,

On 30/05/2023 09:54, Juergen Gross wrote:

Replace the usage of the xenstore private list.h header with the
common xen_list.h one.

Signed-off-by: Juergen Gross 
---
V3:
- new patch
---
  tools/libs/store/xs.c | 56 +--
  1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
index 7a9a8b1656..3813b69ae2 100644
--- a/tools/libs/store/xs.c
+++ b/tools/libs/store/xs.c
@@ -35,13 +35,13 @@
  #include 
  #include "xenstore.h"
  #include "xs_lib.h"
-#include "list.h"
  #include "utils.h"
  
  #include 

+#include 
  
  struct xs_stored_msg {

-   struct list_head list;
+   XEN_TAILQ_ENTRY(struct xs_stored_msg) list;


I have expected us to use to XEN_LIST_*. Can you explain why you didn't 
use them?



struct xsd_sockmsg hdr;
char *body;
  };
@@ -70,7 +70,7 @@ struct xs_handle {
   * A list of fired watch messages, protected by a mutex. Users can
   * wait on the conditional variable until a watch is pending.
   */
-   struct list_head watch_list;
+   XEN_TAILQ_HEAD(, struct xs_stored_msg) watch_list;
pthread_mutex_t watch_mutex;
pthread_cond_t watch_condvar;
  
@@ -84,7 +84,7 @@ struct xs_handle {

   * because we serialise requests. The requester can wait on the
   * conditional variable for its response.
   */
-   struct list_head reply_list;
+   XEN_TAILQ_HEAD(, struct xs_stored_msg) reply_list;
pthread_mutex_t reply_mutex;
pthread_cond_t reply_condvar;
  
@@ -133,8 +133,8 @@ static void *read_thread(void *arg);

  struct xs_handle {
int fd;
Xentoolcore__Active_Handle tc_ah; /* for restrict */
-   struct list_head reply_list;
-   struct list_head watch_list;
+   XEN_TAILQ_HEAD(, struct xs_stored_msg) reply_list;
+   XEN_TAILQ_HEAD(, struct xs_stored_msg) watch_list;
/* Clients can select() on this pipe to wait for a watch to fire. */
int watch_pipe[2];
/* Filtering watch event in unwatch function? */
@@ -180,7 +180,7 @@ int xs_fileno(struct xs_handle *h)
  
  	if ((h->watch_pipe[0] == -1) && (pipe(h->watch_pipe) != -1)) {

/* Kick things off if the watch list is already non-empty. */
-   if (!list_empty(&h->watch_list))
+   if (!XEN_TAILQ_EMPTY(&h->watch_list))
while (write(h->watch_pipe[1], &c, 1) != 1)
continue;
}
@@ -262,8 +262,8 @@ static struct xs_handle *get_handle(const char *connect_to)
if (h->fd == -1)
goto err;
  
-	INIT_LIST_HEAD(&h->reply_list);

-   INIT_LIST_HEAD(&h->watch_list);
+   XEN_TAILQ_INIT(&h->reply_list);
+   XEN_TAILQ_INIT(&h->watch_list);
  
  	/* Watch pipe is allocated on demand in xs_fileno(). */

h->watch_pipe[0] = h->watch_pipe[1] = -1;
@@ -329,12 +329,12 @@ struct xs_handle *xs_open(unsigned long flags)
  static void close_free_msgs(struct xs_handle *h) {
struct xs_stored_msg *msg, *tmsg;
  
-	list_for_each_entry_safe(msg, tmsg, &h->reply_list, list) {

+   XEN_TAILQ_FOREACH_SAFE(msg, &h->reply_list, list, tmsg) {
free(msg->body);
free(msg);
}
  
-	list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {

+   XEN_TAILQ_FOREACH_SAFE(msg, &h->watch_list, list, tmsg) {
free(msg->body);
free(msg);
}
@@ -459,17 +459,17 @@ static void *read_reply(
  
  	mutex_lock(&h->reply_mutex);

  #ifdef USE_PTHREAD
-   while (list_empty(&h->reply_list) && read_from_thread && h->fd != -1)
+   while (XEN_TAILQ_EMPTY(&h->reply_list) && read_from_thread && h->fd != 
-1)
condvar_wait(&h->reply_condvar, &h->reply_mutex);
  #endif
-   if (list_empty(&h->reply_list)) {
+   if (XEN_TAILQ_EMPTY(&h->reply_list)) {
mutex_unlock(&h->reply_mutex);
errno = EINVAL;
return NULL;
}
-   msg = list_top(&h->reply_list, struct xs_stored_msg, list);
-   list_del(&msg->list);
-   assert(list_empty(&h->reply_list));
+   msg = XEN_TAILQ_FIRST(&h->reply_list);
+   XEN_TAILQ_REMOVE(&h->reply_list, msg, list);
+   assert(XEN_TAILQ_EMPTY(&h->reply_list));
mutex_unlock(&h->reply_mutex);
  
  	*type = msg->hdr.type;

@@ -883,7 +883,7 @@ static void xs_maybe_clear_watch_pipe(struct xs_handle *h)
  {
char c;
  
-	if (list_empty(&h->watch_list) && (h->watch_pipe[0] != -1))

+   if (XEN_TAILQ_EMPTY(&h->watch_list) && (h->watch_pipe[0] != -1))
while (read(h->watch_pipe[0], &c, 1) != 1)
continue;
  }
@@ -907,7 +907,7 @@ static char **read_watch_internal(struct xs_handle *h, 
unsigned int *num,
 * we haven't called xs_watch.  Presumably the application
 * will do so later; in the meantime we just block.
 */
-   whil

Re: [PATCH v3 09/16] tools/xenstore: remove support of file backed data base

2023-06-09 Thread Julien Grall

Hi Juergen,

On 30/05/2023 09:54, Juergen Gross wrote:

In order to prepare the replacement of TDB with direct accessible nodes
in memory, remove the support for a file backed data base.

This allows to remove xs_tdb_dump, too.

Signed-off-by: Juergen Gross 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall



Re: [PATCH v3 06/16] tools/xenstore: switch write limiting to use millisecond time base

2023-06-09 Thread Julien Grall

Hi Juergen,

On 30/05/2023 09:54, Juergen Gross wrote:

There is no need to keep struct wrl_timestampt, as it serves the same
purpose as the more simple time base provided by get_now().

Move some more stuff from xenstored_domain.h into xenstored_domain.c
as it is being used nowhere else.

Signed-off-by: Juergen Gross 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall



Re: [PATCH v3 05/16] tools/xenstore: make some write limit functions static

2023-06-09 Thread Julien Grall

Hi Juergen,

On 30/05/2023 09:54, Juergen Gross wrote:

Some wrl_*() functions are only used in xenstored_domain.c, so make
them static. In order to avoid the need of forward declarations, move
the whole function block to the start of the file.

Signed-off-by: Juergen Gross 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall



Re: [PATCH v3 04/16] tools/xenstore: rename hashtable_insert() and let it return 0 on success

2023-06-09 Thread Julien Grall

Hi Juergen,

On 30/05/2023 09:54, Juergen Gross wrote:

Today hashtable_insert() returns 0 in case of an error. Change that to
let it return an errno value in the error case and 0 in case of success.
In order to avoid any missed return value checks or related future
backport errors, rename hashtable_insert() to hashtable_add().

Even if not used today, do the same switch for the return value of
hashtable_expand().

Signed-off-by: Juergen Gross 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall



Re: [PATCH v3 03/16] tools/xenstore: modify interface of create_hashtable()

2023-06-09 Thread Julien Grall

Hi Juergen,

On 30/05/2023 09:54, Juergen Gross wrote:

The minsize parameter of create_hashtable() doesn't have any real use
case for Xenstore, so drop it.

For better talloc_report_full() diagnostic output add a name parameter
to create_hashtable().

Signed-off-by: Juergen Gross 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall



[PATCH v2] docs/misra: new rules addition

2023-06-09 Thread Stefano Stabellini
From: Stefano Stabellini 

For Dir 1.1, a document describing all implementation-defined behaviour
(i.e. gcc-specific behavior) will be added to docs/misra, also including
implementation-specific (gcc-specific) appropriate types for bit-field
relevant to Rule 6.1.

Rule 21.21 is lacking an example on gitlab but the rule is
straightforward: we don't use stdlib at all in Xen.

Signed-off-by: Stefano Stabellini 
---
Changes in v2:
- drop 5.6
- specify additional appropriate types for 6.1
---
 docs/misra/rules.rst | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index d5a6ee8cb6..8be6868496 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -40,6 +40,12 @@ existing codebase are work-in-progress.
  - Summary
  - Notes
 
+   * - `Dir 1.1 
`_
+ - Required
+ - Any implementation-defined behaviour on which the output of the
+   program depends shall be documented and understood
+ -
+
* - `Dir 2.1 
`_
  - Required
  - All source files shall compile without any compilation errors
@@ -57,6 +63,13 @@ existing codebase are work-in-progress.
header file being included more than once
  -
 
+   * - `Dir 4.11 
`_
+ - Required
+ - The validity of values passed to library functions shall be checked
+ - We do not have libraries in Xen (libfdt and others are not
+   considered libraries from MISRA C point of view as they are
+   imported in source form)
+
* - `Dir 4.14 
`_
  - Required
  - The validity of values received from external sources shall be
@@ -133,6 +146,13 @@ existing codebase are work-in-progress.
headers (xen/include/public/) are allowed to retain longer
identifiers for backward compatibility.
 
+   * - `Rule 6.1 
`_
+ - Required
+ - Bit-fields shall only be declared with an appropriate type
+ - In addition to the C99 types, we also consider appropriate types:
+   unsigned char, unsigned short, unsigned long, unsigned long long,
+   enum.
+
* - `Rule 6.2 
`_
  - Required
  - Single-bit named bit fields shall not be of a signed type
@@ -143,6 +163,12 @@ existing codebase are work-in-progress.
  - Octal constants shall not be used
  -
 
+   * - `Rule 7.2 
`_
+ - Required
+ - A "u" or "U" suffix shall be applied to all integer constants
+   that are represented in an unsigned type
+ -
+
* - `Rule 7.3 
`_
  - Required
  - The lowercase character l shall not be used in a literal suffix
@@ -314,6 +340,11 @@ existing codebase are work-in-progress.
used following a subsequent call to the same function
  -
 
+   * - Rule 21.21
+ - Required
+ - The Standard Library function system of  shall not be used
+ -
+
* - `Rule 22.2 
`_
  - Mandatory
  - A block of memory shall only be freed if it was allocated by means of a
-- 
2.25.1




Re: [PATCH] docs/misra: new rules addition

2023-06-09 Thread Stefano Stabellini
On Fri, 9 Jun 2023, Jan Beulich wrote:
> On 08.06.2023 13:02, Roberto Bagnara wrote:
> > On 07/06/23 23:53, Stefano Stabellini wrote:
> >> On Wed, 7 Jun 2023, Jan Beulich wrote:
>  +   * - `Rule 5.6 
>  `_
>  + - Required
>  + - A typedef name shall be a unique identifier
>  + -
> >>>
> >>> Considering that the rule requires uniqueness across the entire code
> >>> base (and hence precludes e.g. two functions having identically named
> >>> local typedefs), I'm a little puzzled this was adopted. I for one
> >>> question that a use like the one mentioned is really at risk of being
> >>> confusing. Instead I think that the need to change at least one of
> >>> the names is at risk of making the code less readable then, even if
> >>> ever so slightly. (All of this said - I don't know if we have any
> >>> violations of this rule.)
> >>
> >> I don't think we have many local typedefs and I think we have only few
> >> violations if I remember right. I'll let Roberto confirm how many. This
> >> rule was considered not a difficult rule (some difficult rules were
> >> found, namely 2.1, 5.5 and 7.4.)
> > 
> > There currently are 30 violations for ARM64:
> > 
> > - some involve a typedef module_name (in the call it was said this should
> >be renamed module_name_t, which will solve the issue);
> > - most concern repeated typedefs (UINT64 and similar) which are repeated
> >in xen/arch/arm/include/asm/arm64/efibind.h
> >and xen/include/acpi/actypes.h
> > - ret_t and phys_addr_t are also redefined in a couple of places.
> > 
> > There are 90 violations for X86_64, for the same reasons, plus
> > 
> > - another set of typedefs for basic types (such as u8);
> > - repeated typedefs for things like guest_l1e_t in the same header file:
> > 
> > xen/arch/x86/include/asm/guest_pt.h:60.39-60.49:
> > for program `xen/.xen-syms.0', the identifier for typedef `guest_l1e_t' is 
> > reused
> > xen/arch/x86/include/asm/guest_pt.h:128.22-128.32:
> > for program `xen/.xen-syms.0', the identifier for typedef `guest_l1e_t' is 
> > reused
> > 
> > The indicated lines read as follows:
> > 
> > typedef struct { guest_intpte_t l1; } guest_l1e_t;
> > typedef l1_pgentry_t guest_l1e_t;
> 
> So this is an example where I don't think we can sensibly do away with the
> re-use of the same typedef name: We use it so we can build the same source
> files multiple ways, dealing with different paging modes guests may be in.

Maybe we can have a quick discussion on Rule 5.6 during the next
meeting.

In the meantime, I'll resend the patch without Rule 5.6 and additing the
extra types in the notes for 6.1.



Re: [PATCH] CI: Add Ocaml to the alpine containers

2023-06-09 Thread Stefano Stabellini
On Fri, 9 Jun 2023, Michal Orzel wrote:
> On 09/06/2023 18:02, Andrew Cooper wrote:
> > 
> > 
> > This gets more coverage of optional parts of the build, and makes it easier 
> > to
> > trial Ocaml related changes in the smoke tests.
> > 
> > Signed-off-by: Andrew Cooper 
> > ---
> > CC: Roger Pau Monné 
> > CC: Stefano Stabellini 
> > CC: Michal Orzel 
> > CC: Anthony PERARD 
> > 
> > Hacked up manually to fix the Ocaml bindings for arm64:
> > 
> >   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/895162017
> > 
> > but this aspect should be done irrespective.
> > ---
> >  automation/build/alpine/3.12-arm64v8.dockerfile | 2 ++
> >  automation/build/alpine/3.12.dockerfile | 2 ++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/automation/build/alpine/3.12-arm64v8.dockerfile 
> > b/automation/build/alpine/3.12-arm64v8.dockerfile
> > index 3f1e6a3fc6df..1be3bf780509 100644
> > --- a/automation/build/alpine/3.12-arm64v8.dockerfile
> > +++ b/automation/build/alpine/3.12-arm64v8.dockerfile
> > @@ -28,6 +28,8 @@ RUN apk --no-cache add \
> >make \
> >musl-dev  \
> >ncurses-dev \
> > +  ocaml \
> > +  ocaml-findlib \
> I can see that in your CI pipeline, this package is missing. Is it then 
> necessary to be added?
> Asking just out of curiosity because other containers have it installed too.
> 
> Apart from that, I can confirm that containers can be built without issues, 
> so:
> Reviewed-by: Michal Orzel 

Acked-by: Stefano Stabellini 

[xen-unstable-smoke test] 181344: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181344 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181344/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 181233
 build-armhf   6 xen-buildfail REGR. vs. 181233

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 xen  3a82e4501c4ec4d53b764c5b69955997b03d1137
baseline version:
 xen  64a647f8d817c6989edc000613b5afae19f03f99

Last test of basis   181233  2023-06-07 02:04:37 Z2 days
Failing since181246  2023-06-07 11:02:03 Z2 days   26 attempts
Testing same since   181342  2023-06-09 14:01:58 Z0 days2 attempts


People who touched revisions under test:
  Andrew Cooper 
  Christian Lindig 
  George Dunlap 
  Henry Wang  # CHANGELOG
  Juergen Gross 
  Julien Grall 
  Luca Fancellu 
  Michal Orzel 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 

jobs:
 build-arm64-xsm  fail
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 test-arm64-arm64-xl-xsm  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 520 lines long.)



Re: [PATCH v2 13/16] xen-blkback: Implement diskseq checks

2023-06-09 Thread Demi Marie Obenour
On Fri, Jun 09, 2023 at 05:13:45PM +0200, Roger Pau Monné wrote:
> On Thu, Jun 08, 2023 at 11:33:26AM -0400, Demi Marie Obenour wrote:
> > On Thu, Jun 08, 2023 at 10:29:18AM +0200, Roger Pau Monné wrote:
> > > On Wed, Jun 07, 2023 at 12:14:46PM -0400, Demi Marie Obenour wrote:
> > > > On Wed, Jun 07, 2023 at 10:20:08AM +0200, Roger Pau Monné wrote:
> > > > > On Tue, Jun 06, 2023 at 01:01:20PM -0400, Demi Marie Obenour wrote:
> > > > > > On Tue, Jun 06, 2023 at 10:25:47AM +0200, Roger Pau Monné wrote:
> > > > > > > On Tue, May 30, 2023 at 04:31:13PM -0400, Demi Marie Obenour 
> > > > > > > wrote:
> > > > > > > Also, you tie this logic to the "physical-device" watch, which
> > > > > > > strictly implies that the "diskseq" node must be written to 
> > > > > > > xenstore
> > > > > > > before the "physical-device" node.  This seems fragile, but I 
> > > > > > > don't
> > > > > > > see much better optiono since the "diskseq" is optional.
> > > > > > 
> > > > > > What about including the diskseq in the "physical-device" node?  
> > > > > > Perhaps
> > > > > > use diskseq@major:minor syntax?
> > > > > 
> > > > > Hm, how would you know whether the blkback instance in the kernel
> > > > > supports the diskseq syntax in physical-device?
> > > > 
> > > > That’s what the next patch is for 🙂.
> > > 
> > > Hm, I think we should separate diskseq support from the notify open
> > > stuff: it's possible a different (non-Linux) backend wants to
> > > implement open notify support but doesn't have diskseq.
> > 
> > I like this idea!  What about having blkback set diskseq to zero?
> > Userspace could then replace it with the actual value.
> 
> I think it would be better if we used a sysfs node, because blkfront
> has no business is knowing whether diskseq is supported by the
> backend or not.
> 
> xenstore should be reserved to features exposed between blkfront and
> blkback if possible.  I know we haven't been very good at this
> however.
> 
> > > > > Can you fetch a disk using a diskseq identifier?
> > > > 
> > > > Not yet, although I have considered adding this ability.  It would be
> > > > one step towards a “diskseqfs” that userspace could use to open a device
> > > > by diskseq.
> > > > 
> > > > > Why I understand that this is an extra safety check in order to assert
> > > > > blkback is opening the intended device, is this attempting to fix some
> > > > > existing issue?
> > > > 
> > > > Yes, it is.  I have a block script (written in C) that validates the
> > > > device it has opened before passing the information to blkback.  It uses
> > > > the diskseq to do this, but for that protection to be complete, blkback
> > > > must also be aware of it.
> > > 
> > > But if your block script opens the device, and keeps it open until
> > > blkback has also taken a reference to it, there's no way such device
> > > could be removed and recreated in the window you point out above, as
> > > there's always a reference on it taken?
> > 
> > This assumes that the block script is not killed in the meantime,
> > which is not a safe assumption due to timeouts and the OOM killer.
> 
> Doesn't seem very reliable to use with delete-on-close either then.

That’s actually the purpose of delete-on-close!  It ensures that if the
block script gets killed, the device is automatically cleaned up.

> > > > > I'm not sure I see how the major:minor numbers would point to a
> > > > > different device than the one specified by the toolstack unless the
> > > > > admin explicitly messes with the devices before blkback has got time
> > > > > to open them.  But then the admin can already do pretty much
> > > > > everything it wants with the system.
> > > > 
> > > > Admins typically refer to e.g. device-mapper devices by name, not by
> > > > major:minor number.  If a device is destroyed and recreated right as the
> > > > block script is running, this race condition can occur.
> > > 
> > > Right, but what about this device recreation happening after the admin
> > > has written the guest config file but before the call to (lib)xl
> > > happens?  blkback would also end up using a different device than
> > > indented, and your proposed approach doesn't fix this.  The only way to
> > > solve this would be to reference devices by UUID (iow: diskseq)
> > > directly in the guest config file.
> > 
> > That would be a good idea, but it is orthogonal to this patch.  My
> > script opens the device and uses various means to check that it did
> > open the correct device.  It then passes the diskseq to blkback.
> 
> How you do this with losetup?  I guess there's an atomic way to setup
> a loop device and get its diskseq?

It can’t be done with losetup.  I use a C program that directly
issues ioctls to /dev/loop-control and /dev/loop*.  Doing this with
device-mapper requires kernel patches that have been submitted but are
not yet upstream.

> > > Then the block script will open the device by diskseq and pass the
> > > major:minor numbers to blkback.
> > 
> > Alternatively, t

Re: [PATCH] CI: Add Ocaml to the alpine containers

2023-06-09 Thread Andrew Cooper
On 09/06/2023 5:21 pm, Michal Orzel wrote:
> On 09/06/2023 18:02, Andrew Cooper wrote:
>> This gets more coverage of optional parts of the build, and makes it easier 
>> to
>> trial Ocaml related changes in the smoke tests.
>>
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Roger Pau Monné 
>> CC: Stefano Stabellini 
>> CC: Michal Orzel 
>> CC: Anthony PERARD 
>>
>> Hacked up manually to fix the Ocaml bindings for arm64:
>>
>>   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/895162017
>>
>> but this aspect should be done irrespective.
>> ---
>>  automation/build/alpine/3.12-arm64v8.dockerfile | 2 ++
>>  automation/build/alpine/3.12.dockerfile | 2 ++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/automation/build/alpine/3.12-arm64v8.dockerfile 
>> b/automation/build/alpine/3.12-arm64v8.dockerfile
>> index 3f1e6a3fc6df..1be3bf780509 100644
>> --- a/automation/build/alpine/3.12-arm64v8.dockerfile
>> +++ b/automation/build/alpine/3.12-arm64v8.dockerfile
>> @@ -28,6 +28,8 @@ RUN apk --no-cache add \
>>make \
>>musl-dev  \
>>ncurses-dev \
>> +  ocaml \
>> +  ocaml-findlib \
> I can see that in your CI pipeline, this package is missing. Is it then 
> necessary to be added?
> Asking just out of curiosity because other containers have it installed too.
>
> Apart from that, I can confirm that containers can be built without issues, 
> so:
> Reviewed-by: Michal Orzel 

Thanks.  It is necessary, yes.

I missed it the first time around, and deployed updated container to
rerun the failed job, rather than pushing a full new branch.

I'm unsure why the opensuse containers have ocaml-ocamlbuild and
ocaml-ocamldoc too, but that can be a mystery for another day.

~Andrew



Re: [PATCH] CI: Add Ocaml to the alpine containers

2023-06-09 Thread Michal Orzel


On 09/06/2023 18:02, Andrew Cooper wrote:
> 
> 
> This gets more coverage of optional parts of the build, and makes it easier to
> trial Ocaml related changes in the smoke tests.
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Roger Pau Monné 
> CC: Stefano Stabellini 
> CC: Michal Orzel 
> CC: Anthony PERARD 
> 
> Hacked up manually to fix the Ocaml bindings for arm64:
> 
>   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/895162017
> 
> but this aspect should be done irrespective.
> ---
>  automation/build/alpine/3.12-arm64v8.dockerfile | 2 ++
>  automation/build/alpine/3.12.dockerfile | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/automation/build/alpine/3.12-arm64v8.dockerfile 
> b/automation/build/alpine/3.12-arm64v8.dockerfile
> index 3f1e6a3fc6df..1be3bf780509 100644
> --- a/automation/build/alpine/3.12-arm64v8.dockerfile
> +++ b/automation/build/alpine/3.12-arm64v8.dockerfile
> @@ -28,6 +28,8 @@ RUN apk --no-cache add \
>make \
>musl-dev  \
>ncurses-dev \
> +  ocaml \
> +  ocaml-findlib \
I can see that in your CI pipeline, this package is missing. Is it then 
necessary to be added?
Asking just out of curiosity because other containers have it installed too.

Apart from that, I can confirm that containers can be built without issues, so:
Reviewed-by: Michal Orzel 

~Michal



Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Shawn Anastasio
On Fri Jun 9, 2023 at 11:07 AM CDT, Julien Grall wrote:
> Thanks for the explanations. To clarify, are you saying that all the 
> files will be GPLv2+ or just some?

My idea was to license any file where I expect there to derivations
from existing code as GPLv2+, and fall back to GPLv2-only for
newly-written files for which I expect there to be no reuse of existing
GPLv2+ code.

> If the latter, then my concern would be that if you need to import 
> GPLv2-only code, then you may need to write your code in a different 
> file. This may become messy to handle and some developer may end up to 
> be confused.

Agreed, and I definitely want to minimize confusion. Ultimately I think
situations like this will be unavoidable, though, since derivations will
likely need to be made from both GPLv2-only and GPLv2+ code.

> Cheers,

Thanks,
Shawn




Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Julien Grall

Hi Shawn,

On 09/06/2023 16:01, Shawn Anastasio wrote:

On Fri Jun 9, 2023 at 5:12 AM CDT, Julien Grall wrote:

Strictly speaking we can refuse any code. That count for license as
well. Anyway, I didn't request a change here. I merely pointed out that
any use of GPLv2+ should be justified because on Arm most of the people
don't pay attention on the license and pick the one from an existing file.


Hi Julien,

The choice of GPLv2+ for many of the files in this patchset was indeed
inherited from old IBM-written Xen code that the files in question were
derived from. I did not realize it was permissible or even desirable to
relicense those to GPLv2-only.

As for the new files, GPLv2+ was chosen to remain consistent and to open
the door for future derivations from GPLv2+ licensed code, either from
the older Xen tree or from the Linux ppc tree, much of which is also
licensed as GPLv2+. If it would reduce friction, these files could be
relicensed to GPLv2-only.


(Before someone points out, I know this is already a problem on other 
part of Xen. But it would be ideal if we avoid spreading this mess on 
new architectures :).


Thanks for the explanations. To clarify, are you saying that all the 
files will be GPLv2+ or just some?


If the latter, then my concern would be that if you need to import 
GPLv2-only code, then you may need to write your code in a different 
file. This may become messy to handle and some developer may end up to 
be confused.


I am not a lawyer though, so you may want to check the implications here.

Cheers,

--
Julien Grall



[PATCH] CI: Add Ocaml to the alpine containers

2023-06-09 Thread Andrew Cooper
This gets more coverage of optional parts of the build, and makes it easier to
trial Ocaml related changes in the smoke tests.

Signed-off-by: Andrew Cooper 
---
CC: Roger Pau Monné 
CC: Stefano Stabellini 
CC: Michal Orzel 
CC: Anthony PERARD 

Hacked up manually to fix the Ocaml bindings for arm64:

  https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/895162017

but this aspect should be done irrespective.
---
 automation/build/alpine/3.12-arm64v8.dockerfile | 2 ++
 automation/build/alpine/3.12.dockerfile | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/automation/build/alpine/3.12-arm64v8.dockerfile 
b/automation/build/alpine/3.12-arm64v8.dockerfile
index 3f1e6a3fc6df..1be3bf780509 100644
--- a/automation/build/alpine/3.12-arm64v8.dockerfile
+++ b/automation/build/alpine/3.12-arm64v8.dockerfile
@@ -28,6 +28,8 @@ RUN apk --no-cache add \
   make \
   musl-dev  \
   ncurses-dev \
+  ocaml \
+  ocaml-findlib \
   patch  \
   python3-dev \
   texinfo \
diff --git a/automation/build/alpine/3.12.dockerfile 
b/automation/build/alpine/3.12.dockerfile
index c847aa82d9e2..72ad3a07ad4a 100644
--- a/automation/build/alpine/3.12.dockerfile
+++ b/automation/build/alpine/3.12.dockerfile
@@ -30,6 +30,8 @@ RUN apk --no-cache add \
   make \
   musl-dev  \
   ncurses-dev \
+  ocaml \
+  ocaml-findlib \
   patch  \
   python3-dev \
   texinfo \

base-commit: 3a82e4501c4ec4d53b764c5b69955997b03d1137
-- 
2.30.2




[qemu-mainline test] 181335: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181335 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181335/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 180691
 build-arm64   6 xen-buildfail REGR. vs. 180691
 build-amd64   6 xen-buildfail REGR. vs. 180691
 build-i3866 xen-buildfail REGR. vs. 180691
 build-amd64-xsm   6 xen-buildfail REGR. vs. 180691
 build-i386-xsm6 xen-buildfail REGR. vs. 180691
 build-armhf   6 xen-buildfail REGR. vs. 180691

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-vhd1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-raw   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-coresched-amd64-xl  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-de

Re: [PATCH 3/3] maintainers: Add PPC64 maintainer

2023-06-09 Thread Shawn Anastasio
On Fri Jun 9, 2023 at 4:04 AM CDT, Jan Beulich wrote:
> On 07.06.2023 17:06, Shawn Anastasio wrote:
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -451,6 +451,9 @@ M:  Wei Liu 
> >  S: Supported
> >  T: git https://xenbits.xenproject.org/git-http/ovmf.git
> >  
> > +PPC64
> > +M: Shawn Anastasio 
> > +
> >  POWER MANAGEMENT
> >  M: Jan Beulich 
> >  S: Supported
>
> Nit: You want to move your insertion further down, to maintain alphabetic
> sorting. You further want at least one F: line; see e.g. RISC-V's entry.

Will fix in v2.

> Jan

Thanks,
Shawn




Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling

2023-06-09 Thread Juergen Gross

On 07.06.23 15:12, Christoph Hellwig wrote:

On Mon, May 22, 2023 at 10:37:09AM +0200, Juergen Gross wrote:

In normal cases PCI passthrough in PV guests requires to start the guest
with e820_host=1. So it should be rather easy to limit allocating the
64MB in PV guests to the cases where the memory map has non-RAM regions
especially in the first 1MB of the memory.

This will cover even hotplug cases. The only case not covered would be a
guest started with e820_host=1 even if no PCI passthrough was planned.
But this should be rather rare (at least I hope so).


So is this an ACK for the patch and can we go ahead with it?


As long as above mentioned check of the E820 map is done, yes.

If you want I can send a diff to be folded into your patch on Monday.



(I'd still like to merge swiotlb-xen into swiotlb eventually, but it's
probably not going to happen this merge window)


Would be nice.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Shawn Anastasio
On Fri Jun 9, 2023 at 4:22 AM CDT, Jan Beulich wrote:
> >  xen/arch/ppc/Kconfig.debug   |   5 +
> >  xen/arch/ppc/Makefile|   3 +-
> >  xen/arch/ppc/boot_of.c   | 122 +++
> >  xen/arch/ppc/configs/openpower_defconfig |   1 +
> >  xen/arch/ppc/early_printk.c  |  36 +++
> >  xen/arch/ppc/include/asm/boot.h  |  31 ++
> >  xen/arch/ppc/include/asm/bug.h   |   6 ++
> >  xen/arch/ppc/include/asm/byteorder.h |  74 ++
> >  xen/arch/ppc/include/asm/cache.h |   6 ++
> >  xen/arch/ppc/include/asm/config.h|   3 +
> >  xen/arch/ppc/include/asm/early_printk.h  |  14 +++
> >  xen/arch/ppc/include/asm/processor.h |  54 +-
> >  xen/arch/ppc/include/asm/string.h|   6 ++
> >  xen/arch/ppc/include/asm/types.h |  64 
> >  xen/arch/ppc/ppc64/asm-offsets.c |  55 ++
> >  xen/arch/ppc/ppc64/head.S|  59 +++
> >  xen/arch/ppc/setup.c |  20 +++-
> >  17 files changed, 555 insertions(+), 4 deletions(-)
> >  create mode 100644 xen/arch/ppc/boot_of.c
>
> Unless required, in new additions we tend to prefer dashes over
> underscores. In filenames it is pretty rare that dashes really need
> avoiding.

Thanks for pointing that out -- I'll fix this in v2.

> > --- a/xen/arch/ppc/Kconfig.debug
> > +++ b/xen/arch/ppc/Kconfig.debug
> > @@ -0,0 +1,5 @@
> > +config EARLY_PRINTK
> > +bool "Enable early printk"
> > +default DEBUG
> > +help
> > +  Enables early printk debug messages
> > \ No newline at end of file
>
> There are many examples of this throughout the patch, which you want to
> take care of.

Ditto.

> > --- /dev/null
> > +++ b/xen/arch/ppc/boot_of.c
> > @@ -0,0 +1,122 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
>
> By default we mean to use ...
>
> > --- /dev/null
> > +++ b/xen/arch/ppc/early_printk.c
> > @@ -0,0 +1,36 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
>
> ... the more modern form of this (GPL-2.0-only). Anything deviating from
> that may want justifying in the description.

Got it -- I'll clean this up as well.

> Jan

Thanks,
Shawn




Re: [PATCH 1/3] xen: Add files needed for minimal Power build

2023-06-09 Thread Shawn Anastasio
On Fri Jun 9, 2023 at 4:15 AM CDT, Jan Beulich wrote:
> > --- /dev/null
> > +++ b/config/ppc64.mk
> > @@ -0,0 +1,5 @@
> > +CONFIG_PPC64 := y
> > +CONFIG_PPC64_64 := y
> > +CONFIG_PPC64_$(XEN_OS) := y
>
> The first of the 64-s here are a little odd; looking at RISC-V's
> counterpart, wouldn't this want to be
>
> CONFIG_PPC := y
> CONFIG_PPC_64 := y
> CONFIG_PPC_$(XEN_OS) := y>

Good point, this was definitely an oversight on my part. I'll clean this
up in v2.

> > --- /dev/null
> > +++ b/xen/arch/ppc/Kconfig
> > @@ -0,0 +1,42 @@
> > +config PPC
> > +   def_bool y
>
> Is this necessary? Iirc PPC is frequently used as a name for 32-bit PPC
> (but then also elsewhere as covering both 32- and 64-bit), so I'm not
> sure we want this without having a need for it.

This was mostly modeled after riscv/Kconfig. The idea was that this
option guards PPC support in general (both 32- and 64-bit) and the
subsequent option specifically opts in to 64-bit.

Given that this effort is only targeting 64-bit mode, though, the
argument could definitely be made that having the two options is
redundant, but I believe that is also the case for the risc-v port.

> > +config PPC64
> > +   def_bool y
> > +   select 64BIT
> > +
> > +config ARCH_DEFCONFIG
> > +   string
> > +   default "arch/ppc/configs/openpower_defconfig"
> > +
> > +menu "Architecture Features"
> > +
> > +source "arch/Kconfig"
> > +
> > +endmenu
> > +
> > +menu "ISA Selection"
> > +
> > +choice
> > +   prompt "Base ISA"
> > +   default POWER_ISA_2_07B if PPC64
> > +   help
> > + This selects the base ISA version that Xen will target.
> > +
> > +config POWER_ISA_2_07B
> > +   bool "POWER ISA 2.07B+"
> > +   help
> > + Target version 2.07B+ of the POWER ISA (POWER8+)
> > +
> > +config POWER_ISA_3_00
> > +   bool "POWER ISA 3.00+"
> > +   help
> > + Target version 3.00+ of the POWER ISA (POWER9+)
>
> What are the + in here meant to indicate? Since this is about a baseline
> ISA, I find such a use (presumably standing for "or newer") ambiguous.

Fair point. The + was intended as an "or newer" but I agree that it's
ambiguous (for instance, it might be interpreted as a part of the ISA's
name). I'll remove it in v2, along with a renaming of "POWER ISA" to
"Power ISA" which I originally overlooked.

> Jan

Thanks,
Shawn




[PATCH v2] tools/ocaml/xc: Fix xc_physinfo() bindings

2023-06-09 Thread Andrew Cooper
The original change doesn't compile on ARM:

  xenctrl_stubs.c: In function 'stub_xc_physinfo':
  xenctrl_stubs.c:821:16: error: unused variable 'arch_cap_flags_tag' 
[-Werror=unused-variable]
821 | int r, arch_cap_flags_tag;
|^~
  cc1: all warnings being treated as errors

but it was buggy too.

First, it tried storing an int in a pointer slot, causing heap corruption.

Next, it is not legitimate to exclude arm32 in the toolstack as it explicitly
can operate an arm64 toolstack and build arm64 domains.  That in turn means
that you can't stash a C uint32_t in an OCaml int.

Rewrite the arch_capabilities handling from scratch.  Break it out into a
separate function, and make the construction of arch_physinfo_cap_flags common
to prevent other indirection bugs.

Reintroduce arm_physinfo_caps with the fields broken out.

Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
Signed-off-by: Andrew Cooper 
Acked-by: Christian Lindig 
---
CC: Christian Lindig 
CC: Edwin Török 
CC: Rob Hoes 
CC: Luca Fancellu 

v2:
 * CAMLreturn().

Tested on x86 locally.  Tested on arm64 with:
  https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/4447272476

and a tweaked form of my local ocaml smoke test for these kinds of things:

  # osmoke
  Xen version: 4.18-unstable
  Physinfo:
threads_per_core 1
SVE len: 0
  Starting GC.compact()
  Done

I didn't get this working for arm32, because that's all Yocto, but the arm64
test is good enough IMO.
---
 tools/ocaml/libs/xc/xenctrl.ml  |  7 +++-
 tools/ocaml/libs/xc/xenctrl.mli |  7 +++-
 tools/ocaml/libs/xc/xenctrl_stubs.c | 52 -
 3 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index bf23ca50bb15..d6c6eb73db44 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -128,10 +128,15 @@ type physinfo_cap_flag =
   | CAP_Gnttab_v1
   | CAP_Gnttab_v2
 
+type arm_physinfo_caps =
+  {
+sve_vl: int;
+  }
+
 type x86_physinfo_cap_flag
 
 type arch_physinfo_cap_flags =
-  | ARM of int
+  | ARM of arm_physinfo_caps
   | X86 of x86_physinfo_cap_flag list
 
 type physinfo =
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index ed1e28ea30a0..3bfc16edba96 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -113,10 +113,15 @@ type physinfo_cap_flag =
   | CAP_Gnttab_v1
   | CAP_Gnttab_v2
 
+type arm_physinfo_caps =
+  {
+sve_vl: int;
+  }
+
 type x86_physinfo_cap_flag
 
 type arch_physinfo_cap_flags =
-  | ARM of int
+  | ARM of arm_physinfo_caps
   | X86 of x86_physinfo_cap_flag list
 
 type physinfo = {
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c 
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index a03da31f6f2c..e4d9070f2d00 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -812,13 +812,46 @@ CAMLprim value stub_xc_send_debug_keys(value xch_val, 
value keys)
CAMLreturn(Val_unit);
 }
 
+CAMLprim value physinfo_arch_caps(const xc_physinfo_t *info)
+{
+   CAMLparam0();
+   CAMLlocal2(arch_cap_flags, arch_obj);
+   int tag = -1;
+
+#if defined(__arm__) || defined(__aarch64__)
+
+   tag = 0; /* tag ARM */
+
+   arch_obj = caml_alloc_tuple(1);
+
+   Store_field(arch_obj, 0,
+   Val_int(MASK_EXTR(info->arch_capabilities,
+ XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK) * 128));
+
+#elif defined(__i386__) || defined(__x86_64__)
+
+   tag = 1; /* tag x86 */
+
+   arch_obj = Tag_cons;
+
+#endif
+
+   if ( tag < 0 )
+   caml_failwith("Unhandled architecture");
+
+   arch_cap_flags = caml_alloc_small(1, tag);
+   Store_field(arch_cap_flags, 0, arch_obj);
+
+   CAMLreturn(arch_cap_flags);
+}
+
 CAMLprim value stub_xc_physinfo(value xch_val)
 {
CAMLparam1(xch_val);
-   CAMLlocal4(physinfo, cap_list, arch_cap_flags, arch_cap_list);
+   CAMLlocal2(physinfo, cap_list);
xc_interface *xch = xch_of_val(xch_val);
xc_physinfo_t c_physinfo;
-   int r, arch_cap_flags_tag;
+   int r;
 
caml_enter_blocking_section();
r = xc_physinfo(xch, &c_physinfo);
@@ -846,20 +879,7 @@ CAMLprim value stub_xc_physinfo(value xch_val)
Store_field(physinfo, 7, caml_copy_nativeint(c_physinfo.scrub_pages));
Store_field(physinfo, 8, cap_list);
Store_field(physinfo, 9, Val_int(c_physinfo.max_cpu_id + 1));
-
-#if defined(__i386__) || defined(__x86_64__)
-   arch_cap_list = Tag_cons;
-
-   arch_cap_flags_tag = 1; /* tag x86 */
-
-   arch_cap_flags = caml_alloc_small(1, arch_cap_flags_tag);
-   Store_field(arch_cap_flags, 0, arch_cap_list);
-   Store_field(physinfo, 10, arch_cap_flags);
-#elif defined(__aarch64__)
-   Store_field(physinfo, 10, Val_int(c_physinfo.arch_capabilities));
-#else
-   cam

Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-06-09 Thread Dave Hansen
On 6/9/23 08:22, Konrad Rzeszutek Wilk wrote:
> Dave, are you OK with the change in where the reserve call is made?

The move makes logical sense.  I'm not 100% confident it won't regress
anything, but the blast radius should be limited to iSCSI.  But, yeah,
I'm OK with it.



Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-06-09 Thread Juergen Gross

On 09.06.23 17:27, Konrad Rzeszutek Wilk wrote:
Usually I put it in my tree (ibft) but since it is so simple and the user is Xen 
it would make more sense to do it via the Xen tree (Juergen).


Works for me.


Juergen



Thx

On Fri, Jun 9, 2023, 11:16 AM Dave Hansen > wrote:


On 6/5/23 03:28, Ross Lagerwall wrote:
 > The result of these changes is that it is possible to boot a diskless
 > Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
 > find the iBFT and consequently, the iSCSI root disk.

Acked-by: Dave Hansen mailto:dave.han...@linux.intel.com>> # for x86

The work in this patch seems pretty evenly split between x86 and iSCSI.
Any preferences on who picks it up?





OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-06-09 Thread Konrad Rzeszutek Wilk
Usually I put it in my tree (ibft) but since it is so simple and the user
is Xen it would make more sense to do it via the Xen tree (Juergen).

Thx

On Fri, Jun 9, 2023, 11:16 AM Dave Hansen  wrote:

> On 6/5/23 03:28, Ross Lagerwall wrote:
> > The result of these changes is that it is possible to boot a diskless
> > Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> > find the iBFT and consequently, the iSCSI root disk.
>
> Acked-by: Dave Hansen  # for x86
>
> The work in this patch seems pretty evenly split between x86 and iSCSI.
> Any preferences on who picks it up?
>


Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-06-09 Thread Konrad Rzeszutek Wilk
Feel free to add my Acked-by.

Dave, are you OK with the change in where the reserve call is made?

Thx

On Fri, Jun 9, 2023, 11:08 AM Juergen Gross  wrote:

> On 05.06.23 12:28, Ross Lagerwall wrote:
> > To facilitate diskless iSCSI boot, the firmware can place a table of
> > configuration details in memory called the iBFT. The presence of this
> > table is not specified, nor is the precise location (and it's not in the
> > E820) so the kernel has to search for a magic marker to find it.
> >
> > When running under Xen, Dom 0 does not have access to the entire host's
> > memory, only certain regions which are identity-mapped which means that
> > the pseudo-physical address in Dom0 == real host physical address.
> > Add the iBFT search bounds as a reserved region which causes it to be
> > identity-mapped in xen_set_identity_and_remap_chunk() which allows Dom0
> > access to the specific physical memory to correctly search for the iBFT
> > magic marker (and later access the full table).
> >
> > This necessitates moving the call to reserve_ibft_region() somewhat
> > later so that it is called after e820__memory_setup() which is when the
> > Xen identity mapping adjustments are applied. The precise location of
> > the call is not too important so I've put it alongside dmi_setup() which
> > does similar scanning of memory for configuration tables.
> >
> > Finally in the iBFT find code, instead of using isa_bus_to_virt() which
> > doesn't do the right thing under Xen, use early_memremap() like the
> > dmi_setup() code does.
> >
> > The result of these changes is that it is possible to boot a diskless
> > Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> > find the iBFT and consequently, the iSCSI root disk.
> >
> > Signed-off-by: Ross Lagerwall 
>
> Reviewed-by: Juergen Gross 
>
>
> Juergen
>
>


[xen-unstable-smoke test] 181342: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181342 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181342/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 181233
 build-armhf   6 xen-buildfail REGR. vs. 181233

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 xen  3a82e4501c4ec4d53b764c5b69955997b03d1137
baseline version:
 xen  64a647f8d817c6989edc000613b5afae19f03f99

Last test of basis   181233  2023-06-07 02:04:37 Z2 days
Failing since181246  2023-06-07 11:02:03 Z2 days   25 attempts
Testing same since   181342  2023-06-09 14:01:58 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Christian Lindig 
  George Dunlap 
  Henry Wang  # CHANGELOG
  Juergen Gross 
  Julien Grall 
  Luca Fancellu 
  Michal Orzel 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 

jobs:
 build-arm64-xsm  fail
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 test-arm64-arm64-xl-xsm  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 520 lines long.)



Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-06-09 Thread Dave Hansen
On 6/5/23 03:28, Ross Lagerwall wrote:
> The result of these changes is that it is possible to boot a diskless
> Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> find the iBFT and consequently, the iSCSI root disk.

Acked-by: Dave Hansen  # for x86

The work in this patch seems pretty evenly split between x86 and iSCSI.
Any preferences on who picks it up?



Re: [PATCH v2 13/16] xen-blkback: Implement diskseq checks

2023-06-09 Thread Roger Pau Monné
On Thu, Jun 08, 2023 at 11:33:26AM -0400, Demi Marie Obenour wrote:
> On Thu, Jun 08, 2023 at 10:29:18AM +0200, Roger Pau Monné wrote:
> > On Wed, Jun 07, 2023 at 12:14:46PM -0400, Demi Marie Obenour wrote:
> > > On Wed, Jun 07, 2023 at 10:20:08AM +0200, Roger Pau Monné wrote:
> > > > On Tue, Jun 06, 2023 at 01:01:20PM -0400, Demi Marie Obenour wrote:
> > > > > On Tue, Jun 06, 2023 at 10:25:47AM +0200, Roger Pau Monné wrote:
> > > > > > On Tue, May 30, 2023 at 04:31:13PM -0400, Demi Marie Obenour wrote:
> > > > > > Also, you tie this logic to the "physical-device" watch, which
> > > > > > strictly implies that the "diskseq" node must be written to xenstore
> > > > > > before the "physical-device" node.  This seems fragile, but I don't
> > > > > > see much better optiono since the "diskseq" is optional.
> > > > > 
> > > > > What about including the diskseq in the "physical-device" node?  
> > > > > Perhaps
> > > > > use diskseq@major:minor syntax?
> > > > 
> > > > Hm, how would you know whether the blkback instance in the kernel
> > > > supports the diskseq syntax in physical-device?
> > > 
> > > That’s what the next patch is for 🙂.
> > 
> > Hm, I think we should separate diskseq support from the notify open
> > stuff: it's possible a different (non-Linux) backend wants to
> > implement open notify support but doesn't have diskseq.
> 
> I like this idea!  What about having blkback set diskseq to zero?
> Userspace could then replace it with the actual value.

I think it would be better if we used a sysfs node, because blkfront
has no business is knowing whether diskseq is supported by the
backend or not.

xenstore should be reserved to features exposed between blkfront and
blkback if possible.  I know we haven't been very good at this
however.

> > > > Can you fetch a disk using a diskseq identifier?
> > > 
> > > Not yet, although I have considered adding this ability.  It would be
> > > one step towards a “diskseqfs” that userspace could use to open a device
> > > by diskseq.
> > > 
> > > > Why I understand that this is an extra safety check in order to assert
> > > > blkback is opening the intended device, is this attempting to fix some
> > > > existing issue?
> > > 
> > > Yes, it is.  I have a block script (written in C) that validates the
> > > device it has opened before passing the information to blkback.  It uses
> > > the diskseq to do this, but for that protection to be complete, blkback
> > > must also be aware of it.
> > 
> > But if your block script opens the device, and keeps it open until
> > blkback has also taken a reference to it, there's no way such device
> > could be removed and recreated in the window you point out above, as
> > there's always a reference on it taken?
> 
> This assumes that the block script is not killed in the meantime,
> which is not a safe assumption due to timeouts and the OOM killer.

Doesn't seem very reliable to use with delete-on-close either then.

> > > > I'm not sure I see how the major:minor numbers would point to a
> > > > different device than the one specified by the toolstack unless the
> > > > admin explicitly messes with the devices before blkback has got time
> > > > to open them.  But then the admin can already do pretty much
> > > > everything it wants with the system.
> > > 
> > > Admins typically refer to e.g. device-mapper devices by name, not by
> > > major:minor number.  If a device is destroyed and recreated right as the
> > > block script is running, this race condition can occur.
> > 
> > Right, but what about this device recreation happening after the admin
> > has written the guest config file but before the call to (lib)xl
> > happens?  blkback would also end up using a different device than
> > indented, and your proposed approach doesn't fix this.  The only way to
> > solve this would be to reference devices by UUID (iow: diskseq)
> > directly in the guest config file.
> 
> That would be a good idea, but it is orthogonal to this patch.  My
> script opens the device and uses various means to check that it did
> open the correct device.  It then passes the diskseq to blkback.

How you do this with losetup?  I guess there's an atomic way to setup
a loop device and get its diskseq?

> > Then the block script will open the device by diskseq and pass the
> > major:minor numbers to blkback.
> 
> Alternatively, the toolstack could write both the diskseq and
> major:minor numbers and be confident that it is referring to the
> correct device, no matter how long ago it got that information.
> This could be quite useful for e.g. one VM exporting a device to
> another VM by calling losetup(8) and expecting a human to make a
> decision based on various properties about the device.  In this
> case there is no upper bound on the race window.

Instead of playing with xenstore nodes, it might be better to simply
have blkback export on sysfs the diskseq of the opened device, and let
the block script check whether that's correct or not.  That implies
less c

Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-06-09 Thread Juergen Gross

On 05.06.23 12:28, Ross Lagerwall wrote:

To facilitate diskless iSCSI boot, the firmware can place a table of
configuration details in memory called the iBFT. The presence of this
table is not specified, nor is the precise location (and it's not in the
E820) so the kernel has to search for a magic marker to find it.

When running under Xen, Dom 0 does not have access to the entire host's
memory, only certain regions which are identity-mapped which means that
the pseudo-physical address in Dom0 == real host physical address.
Add the iBFT search bounds as a reserved region which causes it to be
identity-mapped in xen_set_identity_and_remap_chunk() which allows Dom0
access to the specific physical memory to correctly search for the iBFT
magic marker (and later access the full table).

This necessitates moving the call to reserve_ibft_region() somewhat
later so that it is called after e820__memory_setup() which is when the
Xen identity mapping adjustments are applied. The precise location of
the call is not too important so I've put it alongside dmi_setup() which
does similar scanning of memory for configuration tables.

Finally in the iBFT find code, instead of using isa_bus_to_virt() which
doesn't do the right thing under Xen, use early_memremap() like the
dmi_setup() code does.

The result of these changes is that it is possible to boot a diskless
Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
find the iBFT and consequently, the iSCSI root disk.

Signed-off-by: Ross Lagerwall 


Reviewed-by: Juergen Gross 


Juergen



OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Shawn Anastasio
On Fri Jun 9, 2023 at 5:12 AM CDT, Julien Grall wrote:
> Strictly speaking we can refuse any code. That count for license as 
> well. Anyway, I didn't request a change here. I merely pointed out that 
> any use of GPLv2+ should be justified because on Arm most of the people 
> don't pay attention on the license and pick the one from an existing file.

Hi Julien,

The choice of GPLv2+ for many of the files in this patchset was indeed
inherited from old IBM-written Xen code that the files in question were
derived from. I did not realize it was permissible or even desirable to
relicense those to GPLv2-only.

As for the new files, GPLv2+ was chosen to remain consistent and to open
the door for future derivations from GPLv2+ licensed code, either from
the older Xen tree or from the Linux ppc tree, much of which is also
licensed as GPLv2+. If it would reduce friction, these files could be
relicensed to GPLv2-only.

Thanks,
Shawn



[libvirt test] 181328: tolerable all pass - PUSHED

2023-06-09 Thread osstest service owner
flight 181328 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181328/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 181293
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 181293
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 181293
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  d09b73b5601e7d974698fc62d1fbc64efb7093dc
baseline version:
 libvirt  9b743ee19053db2fc3da8fba1e9cf81915c1e2f4

Last test of basis   181293  2023-06-08 04:18:54 Z1 days
Testing same since   181328  2023-06-09 04:21:22 Z0 days1 attempts


People who touched revisions under test:
  Michal Privoznik 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   pass
 test-armhf-armhf-libvirt-qcow2   pass
 test-arm64-arm64-libvirt-raw pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-i386-libvirt-raw  pass
 test-amd64-amd64-libvirt-vhd pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is a

Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0

2023-06-09 Thread Ross Lagerwall
> From: Xen-devel  on behalf of Konrad 
> Rzeszutek Wilk 
> Sent: Thursday, June 8, 2023 5:38 PM
> To: Ross Lagerwall 
> Cc: linux-ker...@vger.kernel.org ; 
> xen-devel@lists.xenproject.org ; Jan Beulich 
> ; Thomas Gleixner ; Ingo Molnar 
> ; Borislav Petkov ; Dave Hansen 
> ; x...@kernel.org ; Juergen 
> Gross ; Boris Ostrovsky ; Peter 
> Jones ; Konrad Rzeszutek Wilk 
> Subject: Re: [PATCH v3] iscsi_ibft: Fix finding the iBFT under Xen Dom 0 
>  
> It looks fine to me, but could I ask you to triple check that on
> non-Xen it still detects the iBFT?
> 
> Thx!

I have checked again, and yes, it still detects the iBFT in the non-Xen
case and the information in /sys/firmware/ibft looks correct.

Ross

> 
> On Mon, Jun 5, 2023 at 6:28 AM Ross Lagerwall  
> wrote:
> >
> > To facilitate diskless iSCSI boot, the firmware can place a table of
> > configuration details in memory called the iBFT. The presence of this
> > table is not specified, nor is the precise location (and it's not in the
> > E820) so the kernel has to search for a magic marker to find it.
> >
> > When running under Xen, Dom 0 does not have access to the entire host's
> > memory, only certain regions which are identity-mapped which means that
> > the pseudo-physical address in Dom0 == real host physical address.
> > Add the iBFT search bounds as a reserved region which causes it to be
> > identity-mapped in xen_set_identity_and_remap_chunk() which allows Dom0
> > access to the specific physical memory to correctly search for the iBFT
> > magic marker (and later access the full table).
> >
> > This necessitates moving the call to reserve_ibft_region() somewhat
> > later so that it is called after e820__memory_setup() which is when the
> > Xen identity mapping adjustments are applied. The precise location of
> > the call is not too important so I've put it alongside dmi_setup() which
> > does similar scanning of memory for configuration tables.
> >
> > Finally in the iBFT find code, instead of using isa_bus_to_virt() which
> > doesn't do the right thing under Xen, use early_memremap() like the
> > dmi_setup() code does.
> >
> > The result of these changes is that it is possible to boot a diskless
> > Xen + Dom0 running off an iSCSI disk whereas previously it would fail to
> > find the iBFT and consequently, the iSCSI root disk.
> >
> > Signed-off-by: Ross Lagerwall 
> > ---
> >
> > In v3:
> > * Expanded commit message.
> > * Used IBFT_ constants.
> >
> >  arch/x86/kernel/setup.c    |  2 +-
> >  arch/x86/xen/setup.c   | 28 +++-
> >  drivers/firmware/iscsi_ibft_find.c | 26 +-
> >  include/linux/iscsi_ibft.h | 10 +-
> >  4 files changed, 46 insertions(+), 20 deletions(-)
> >
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 16babff771bd..616b80507abd 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -796,7 +796,6 @@ static void __init early_reserve_memory(void)
> >
> > memblock_x86_reserve_range_setup_data();
> >
> > -   reserve_ibft_region();
> > reserve_bios_regions();
> > trim_snb_memory();
> >  }
> > @@ -1032,6 +1031,7 @@ void __init setup_arch(char **cmdline_p)
> > if (efi_enabled(EFI_BOOT))
> > efi_init();
> >
> > +   reserve_ibft_region();
> > dmi_setup();
> >
> > /*
> > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> > index c2be3efb2ba0..8b5cf7bb1f47 100644
> > --- a/arch/x86/xen/setup.c
> > +++ b/arch/x86/xen/setup.c
> > @@ -6,6 +6,7 @@
> >   */
> >
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -764,17 +765,26 @@ char * __init xen_memory_setup(void)
> > BUG_ON(memmap.nr_entries == 0);
> > xen_e820_table.nr_entries = memmap.nr_entries;
> >
> > -   /*
> > -    * Xen won't allow a 1:1 mapping to be created to UNUSABLE
> > -    * regions, so if we're using the machine memory map leave the
> > -    * region as RAM as it is in the pseudo-physical map.
> > -    *
> > -    * UNUSABLE regions in domUs are not handled and will need
> > -    * a patch in the future.
> > -    */
> > -   if (xen_initial_domain())
> > +   if (xen_initial_domain()) {
> > +   /*
> > +    * Xen won't allow a 1:1 mapping to be created to UNUSABLE
> > +    * regions, so if we're using the machine memory map leave 
> > the
> > +    * region as RAM as it is in the pseudo-physical map.
> > +    *
> > +    * UNUSABLE regions in domUs are not handled and will need
> > +    * a patch in the future.
> > +    */
> > xen_ignore_unusable();
> >
> > +#ifdef CONFIG_ISCSI_IBFT_FIND
> > +   /* Reserve 0.5 MiB to 1 MiB region so iBFT can be found */
> > +   xen_e820_table.entries[xen_e820_table.nr_entries].addr = 
> > IBFT_START;
> > +

Re: Xen reliance on non-standard GCC features

2023-06-09 Thread Bertrand Marquis
Hi,

> On 9 Jun 2023, at 15:19, Julien Grall  wrote:
> 
> Hi Jan,
> 
> On 09/06/2023 09:54, Jan Beulich wrote:
>> On 08.06.2023 14:18, Roberto Bagnara wrote:
>>> On 07/06/23 09:39, Jan Beulich wrote:
 On 05.06.2023 15:26, Roberto Bagnara wrote:
> On 05/06/23 11:28, Jan Beulich wrote:
>> On 05.06.2023 07:28, Roberto Bagnara wrote:
> You are right: here are a few examples for U2:
> 
> xen/arch/arm/cpuerrata.c:92.12-92.35:
> empty initializer list (ill-formed for the C99 standard, ISO/IEC 
> 9899:1999 Section 6.7.8: "An empty initialization list." [STD.emptinit]). 
> Tool used is `/usr/bin/aarch64-linux-gnu-gcc-12'
> xen/include/xen/spinlock.h:31.21-31.23: expanded from macro `_LOCK_DEBUG'
> xen/include/xen/spinlock.h:143.57-143.67: expanded from macro 
> `SPIN_LOCK_UNLOCKED'
> xen/include/xen/spinlock.h:144.43-144.60: expanded from macro 
> `DEFINE_SPINLOCK'
 
 I'm afraid this is a bad example, as it goes hand-in-hand with using
 another extension. I don't think using a non-empty initialization list
 is going to work with
 
 union lock_debug { };
>>> 
>>> Yes, this is C99 undefined behavior 58:
>>> "A structure or union is defined as containing no named members (6.7.2.1)."
>>> 
>>> Here is another example:
>>> 
>>> lpae_t pte = {};
>>> 
>>> whereas we have
>>> 
>>> typedef union {
>>>  uint64_t bits;
>>>  lpae_pt_t pt;
>>>  lpae_p2m_t p2m;
>>>  lpae_walk_t walk;
>>> } lpae_t;
>>> 
>>> 
> xen/arch/arm/cpuerrata.c:678.5-678.6:
> empty initializer list (ill-formed for the C99 standard, ISO/IEC 
> 9899:1999 Section 6.7.8: "An empty initialization list." [STD.emptinit]). 
> Tool used is `/usr/bin/aarch64-linux-gnu-gcc-12'
> 
> xen/arch/arm/cpufeature.c:33.5-33.6:
> empty initializer list (ill-formed for the C99 standard, ISO/IEC 
> 9899:1999 Section 6.7.8: "An empty initialization list." [STD.emptinit]). 
> Tool used is `/usr/bin/aarch64-linux-gnu-gcc-12'
 
 Both of these are a common idiom we use: The "sentinel" of an array
 of compound type initializer.
>>> 
>>> Wouldn't it be possible writing such sentinels in a standard-compliant
>>> way, like {0} or similar, instead of {}?
>> I would be possible, sure, but the question is whether we want that. Iirc
>> in review comments we've been asking to preferably use {}, for being
>> shorter / less clutter without resulting in any ambiguity.
>>> U6) Empty declarations.
> 
> Examples:
> 
> xen/arch/arm/arm64/lib/find_next_bit.c:57.29:
> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
> `/usr/bin/aarch64-linux-gnu-gcc-12'
> 
> xen/arch/arm/arm64/lib/find_next_bit.c:103.34:
> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
> `/usr/bin/aarch64-linux-gnu-gcc-12'
 
 Looks like these could be taken care of by finally purging our
 EXPORT_SYMBOL() stub.
 
> xen/arch/arm/include/asm/vreg.h:143.26:
> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
> `/usr/bin/aarch64-linux-gnu-gcc-12'
> 
> xen/arch/arm/include/asm/vreg.h:144.26:
> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
> `/usr/bin/aarch64-linux-gnu-gcc-12'
 
 I'm having trouble spotting anything suspicious there.
>>> 
>>> The macro expands to definitions of inline functions
>>> and after the macro invocation there is a ";".
>>> 
>>> The preprocessed code is then:
>>> 
>>> static inline void foo() { ... }
>>> ;
>>> 
>>> where the final ";" is an empty declaration not allowed by
>>> the C99 language standard.
>> Oh, I see.
>>> Removing the ";" after the macro invocation is a possible solution,
>>> but other possibilities exist if this is strongly unwanted.
>> We have other macros to instantiate functions, and there no stray
>> semicolons are used. I think this wants doing the same way here, but it
>> being Arm code the ultimate say is with the Arm maintainers.
> 
> I don't think there is a reason to keep the ";" after. So I would be fine if 
> this is removed.

+1

Cheers
Bertrand




Re: RHEL 8.8 and derivatives: suspend and live migration issue

2023-06-09 Thread Roger Pau Monné
On Fri, Jun 09, 2023 at 04:07:54PM +0200, David Morel wrote:
> Hello,
> 
> This mail is meant to be more of a summary plus a heads up, I don't think
> there is any action needed on it.
> 
> A user reported [1] a crash when live migrating a VM running Rocky Linux
> 8.8. After investigation it appears to be happening:
> - for RHEL derivatives when migrating from 8.7 to 8.8.
> - on suspend/resume
> - on live migration
> 
> With a callstack we provided, Roger Pau Monné quickly identified the
> issue as being the addition of:
> - x86/idt: Remove address operator on function machine_check() [2]

No, the above reference is incorrect, the issue is caused by the
backport of [0]:

06184325a1cc x86/idt: Annotate alloc_intr_gate() with __init

Without having backported the mentioned Xen commit (x86/xen: Split HVM
vector callback setup and interrupt gate allocation).

Regards, Roger.

[0] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=06184325a1cce27a02a688d98740f90fe06e0133



RHEL 8.8 and derivatives: suspend and live migration issue

2023-06-09 Thread David Morel
Hello,

This mail is meant to be more of a summary plus a heads up, I don't think
there is any action needed on it.

A user reported [1] a crash when live migrating a VM running Rocky Linux
8.8. After investigation it appears to be happening:
- for RHEL derivatives when migrating from 8.7 to 8.8.
- on suspend/resume
- on live migration

With a callstack we provided, Roger Pau Monné quickly identified the
issue as being the addition of:
- x86/idt: Remove address operator on function machine_check() [2]
While missing:
- x86/xen: Split HVM vector callback setup and interrupt gate allocation [3]

Version-wise:
- 4.18.0-466.el8 will start having the issue as it integrate [2].
- 4.18.0-477 are versions used in 8.8 releases, these versions will
  therefore have the issue.
- 4.18.0-488 that can be found in CentOS 8 Stream integrates [3] and
  works fine.

According to the RPM changelog I think the bugzilla entry at RH should
be the number 2187810 [4], but that is not publicly available.

The -488 versions are not yet available for the derivative
distributions. We do not think there is much to do, raising this to
Rocky Linux or Alma directly likely won't change their kernel upgrade
plan.


[1] 
https://xcp-ng.org/forum/topic/7420/live-migrate-of-rocky-linux-8-8-vm-crashes-reboots-vm
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/xen/events/events_base.c?id=fbaed278a3cc72a46aadae667b8c6754b78640a6
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/xen/events/events_base.c?id=a0bb51f2638e0810c347024679239fd10a8f7990
[4] https://bugzilla.redhat.com/show_bug.cgi?id=2187810

-- 
David Morel



Re: Xen reliance on non-standard GCC features

2023-06-09 Thread Julien Grall

Hi Jan,

On 09/06/2023 09:54, Jan Beulich wrote:

On 08.06.2023 14:18, Roberto Bagnara wrote:

On 07/06/23 09:39, Jan Beulich wrote:

On 05.06.2023 15:26, Roberto Bagnara wrote:

On 05/06/23 11:28, Jan Beulich wrote:

On 05.06.2023 07:28, Roberto Bagnara wrote:

You are right: here are a few examples for U2:

xen/arch/arm/cpuerrata.c:92.12-92.35:
empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 Section 6.7.8: 
"An empty initialization list." [STD.emptinit]). Tool used is 
`/usr/bin/aarch64-linux-gnu-gcc-12'
xen/include/xen/spinlock.h:31.21-31.23: expanded from macro `_LOCK_DEBUG'
xen/include/xen/spinlock.h:143.57-143.67: expanded from macro 
`SPIN_LOCK_UNLOCKED'
xen/include/xen/spinlock.h:144.43-144.60: expanded from macro `DEFINE_SPINLOCK'


I'm afraid this is a bad example, as it goes hand-in-hand with using
another extension. I don't think using a non-empty initialization list
is going to work with

union lock_debug { };


Yes, this is C99 undefined behavior 58:
"A structure or union is defined as containing no named members (6.7.2.1)."

Here is another example:

lpae_t pte = {};

whereas we have

typedef union {
  uint64_t bits;
  lpae_pt_t pt;
  lpae_p2m_t p2m;
  lpae_walk_t walk;
} lpae_t;



xen/arch/arm/cpuerrata.c:678.5-678.6:
empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 Section 6.7.8: 
"An empty initialization list." [STD.emptinit]). Tool used is 
`/usr/bin/aarch64-linux-gnu-gcc-12'

xen/arch/arm/cpufeature.c:33.5-33.6:
empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 Section 6.7.8: 
"An empty initialization list." [STD.emptinit]). Tool used is 
`/usr/bin/aarch64-linux-gnu-gcc-12'


Both of these are a common idiom we use: The "sentinel" of an array
of compound type initializer.


Wouldn't it be possible writing such sentinels in a standard-compliant
way, like {0} or similar, instead of {}?


I would be possible, sure, but the question is whether we want that. Iirc
in review comments we've been asking to preferably use {}, for being
shorter / less clutter without resulting in any ambiguity.


U6) Empty declarations.


Examples:

xen/arch/arm/arm64/lib/find_next_bit.c:57.29:
empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 Section 6.7: 
"An empty declaration." [STD.emptdecl]). Tool used is 
`/usr/bin/aarch64-linux-gnu-gcc-12'

xen/arch/arm/arm64/lib/find_next_bit.c:103.34:
empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 Section 6.7: 
"An empty declaration." [STD.emptdecl]). Tool used is 
`/usr/bin/aarch64-linux-gnu-gcc-12'


Looks like these could be taken care of by finally purging our
EXPORT_SYMBOL() stub.


xen/arch/arm/include/asm/vreg.h:143.26:
empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 Section 6.7: 
"An empty declaration." [STD.emptdecl]). Tool used is 
`/usr/bin/aarch64-linux-gnu-gcc-12'

xen/arch/arm/include/asm/vreg.h:144.26:
empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 Section 6.7: 
"An empty declaration." [STD.emptdecl]). Tool used is 
`/usr/bin/aarch64-linux-gnu-gcc-12'


I'm having trouble spotting anything suspicious there.


The macro expands to definitions of inline functions
and after the macro invocation there is a ";".

The preprocessed code is then:

static inline void foo() { ... }
;

where the final ";" is an empty declaration not allowed by
the C99 language standard.


Oh, I see.


Removing the ";" after the macro invocation is a possible solution,
but other possibilities exist if this is strongly unwanted.


We have other macros to instantiate functions, and there no stray
semicolons are used. I think this wants doing the same way here, but it
being Arm code the ultimate say is with the Arm maintainers.


I don't think there is a reason to keep the ";" after. So I would be 
fine if this is removed.


Cheers,

--
Julien Grall



[xen-unstable-smoke test] 181340: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181340 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181340/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 181233
 build-armhf   6 xen-buildfail REGR. vs. 181233

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 xen  6915a120641b5d232762af7882266048cf039ca0
baseline version:
 xen  64a647f8d817c6989edc000613b5afae19f03f99

Last test of basis   181233  2023-06-07 02:04:37 Z2 days
Failing since181246  2023-06-07 11:02:03 Z2 days   24 attempts
Testing same since   181303  2023-06-08 11:00:25 Z1 days   13 attempts


People who touched revisions under test:
  Andrew Cooper 
  Christian Lindig 
  George Dunlap 
  Henry Wang  # CHANGELOG
  Juergen Gross 
  Julien Grall 
  Luca Fancellu 
  Michal Orzel 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 

jobs:
 build-arm64-xsm  fail
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 test-arm64-arm64-xl-xsm  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 504 lines long.)



[PATCH] xen/arm: rename guest_cpuinfo in domain_cpuinfo

2023-06-09 Thread Bertrand Marquis
Rename the guest_cpuinfo structure to domain_cpuinfo as it is not only
used for guests but also for dom0 so domain is a more suitable name.

While there also rename the create_guest_cpuinfo function to
create_domain_cpuinfo to be coherent.

Signed-off-by: Bertrand Marquis 
---
 xen/arch/arm/arm64/vsysreg.c  |  6 ++--
 xen/arch/arm/cpufeature.c | 40 +--
 xen/arch/arm/include/asm/cpufeature.h |  2 +-
 xen/arch/arm/vcpreg.c |  2 +-
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
index fe31f7b3827f..b5d54c569b33 100644
--- a/xen/arch/arm/arm64/vsysreg.c
+++ b/xen/arch/arm/arm64/vsysreg.c
@@ -76,7 +76,7 @@ TVM_REG(CONTEXTIDR_EL1)
 case HSR_SYSREG_##reg:  \
 {   \
 return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr,   \
-  1, guest_cpuinfo.field.bits[offset]); \
+  1, domain_cpuinfo.field.bits[offset]); \
 }
 
 void do_sysreg(struct cpu_user_regs *regs,
@@ -300,7 +300,7 @@ void do_sysreg(struct cpu_user_regs *regs,
 
 case HSR_SYSREG_ID_AA64PFR0_EL1:
 {
-register_t guest_reg_value = guest_cpuinfo.pfr64.bits[0];
+register_t guest_reg_value = domain_cpuinfo.pfr64.bits[0];
 
 if ( is_sve_domain(v->domain) )
 {
@@ -336,7 +336,7 @@ void do_sysreg(struct cpu_user_regs *regs,
  * When the guest has the SVE feature enabled, the whole 
id_aa64zfr0_el1
  * needs to be exposed.
  */
-register_t guest_reg_value = guest_cpuinfo.zfr64.bits[0];
+register_t guest_reg_value = domain_cpuinfo.zfr64.bits[0];
 
 if ( is_sve_domain(v->domain) )
 guest_reg_value = system_cpuinfo.zfr64.bits[0];
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index b53e1a977601..5f4644865505 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -14,7 +14,7 @@
 
 DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
 
-struct cpuinfo_arm __read_mostly guest_cpuinfo;
+struct cpuinfo_arm __read_mostly domain_cpuinfo;
 
 #ifdef CONFIG_ARM_64
 static bool has_sb_instruction(const struct arm_cpu_capabilities *entry)
@@ -191,45 +191,45 @@ void identify_cpu(struct cpuinfo_arm *c)
 /*
  * This function is creating a cpuinfo structure with values modified to mask
  * all cpu features that should not be published to guest.
- * The created structure is then used to provide ID registers values to guests.
+ * The created structure is then used to provide ID registers values to 
domains.
  */
-static int __init create_guest_cpuinfo(void)
+static int __init create_domain_cpuinfo(void)
 {
 /* Use the sanitized cpuinfo as initial guest cpuinfo */
-guest_cpuinfo = system_cpuinfo;
+domain_cpuinfo = system_cpuinfo;
 
 #ifdef CONFIG_ARM_64
 /* Hide MPAM support as xen does not support it */
-guest_cpuinfo.pfr64.mpam = 0;
-guest_cpuinfo.pfr64.mpam_frac = 0;
+domain_cpuinfo.pfr64.mpam = 0;
+domain_cpuinfo.pfr64.mpam_frac = 0;
 
 /* Hide SVE by default */
-guest_cpuinfo.pfr64.sve = 0;
-guest_cpuinfo.zfr64.bits[0] = 0;
+domain_cpuinfo.pfr64.sve = 0;
+domain_cpuinfo.zfr64.bits[0] = 0;
 
 /* Hide MTE support as Xen does not support it */
-guest_cpuinfo.pfr64.mte = 0;
+domain_cpuinfo.pfr64.mte = 0;
 
 /* Hide PAC support as Xen does not support it */
-guest_cpuinfo.isa64.apa = 0;
-guest_cpuinfo.isa64.api = 0;
-guest_cpuinfo.isa64.gpa = 0;
-guest_cpuinfo.isa64.gpi = 0;
+domain_cpuinfo.isa64.apa = 0;
+domain_cpuinfo.isa64.api = 0;
+domain_cpuinfo.isa64.gpa = 0;
+domain_cpuinfo.isa64.gpi = 0;
 #endif
 
 /* Hide AMU support */
 #ifdef CONFIG_ARM_64
-guest_cpuinfo.pfr64.amu = 0;
+domain_cpuinfo.pfr64.amu = 0;
 #endif
-guest_cpuinfo.pfr32.amu = 0;
+domain_cpuinfo.pfr32.amu = 0;
 
 /* Hide RAS support as Xen does not support it */
 #ifdef CONFIG_ARM_64
-guest_cpuinfo.pfr64.ras = 0;
-guest_cpuinfo.pfr64.ras_frac = 0;
+domain_cpuinfo.pfr64.ras = 0;
+domain_cpuinfo.pfr64.ras_frac = 0;
 #endif
-guest_cpuinfo.pfr32.ras = 0;
-guest_cpuinfo.pfr32.ras_frac = 0;
+domain_cpuinfo.pfr32.ras = 0;
+domain_cpuinfo.pfr32.ras_frac = 0;
 
 return 0;
 }
@@ -237,7 +237,7 @@ static int __init create_guest_cpuinfo(void)
  * This function needs to be run after all smp are started to have
  * cpuinfo structures for all cores.
  */
-__initcall(create_guest_cpuinfo);
+__initcall(create_domain_cpuinfo);
 
 /*
  * Local variables:
diff --git a/xen/arch/arm/include/asm/cpufeature.h 
b/xen/arch/arm/include/asm/cpufeature.h
index 03fe684b4d36..894f278a4a5a 100644
--- a/xen/arch/arm/include/asm/cpufeature.h
+++ b/xen/arch/arm/include/asm/cpufeature.h
@@ -461,7 +461,7 @@ static inline void update_system_featu

Re: [PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Jason Andryuk
On Fri, Jun 9, 2023 at 6:11 AM Andrew Cooper  wrote:
>
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>
> Drop the pair from x86-emulate.h which includes common-macros.h.
>
> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jason Andryuk 

I have (and will now be dropping) an equivalent patch for my
forthcoming HWP v4 series.

Thanks,
Jason



[xen-unstable test] 181325: trouble: broken/fail/pass

2023-06-09 Thread osstest service owner
flight 181325 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181325/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-libvirt-qcow2 broken

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-qcow2  5 host-install(5)broken pass in 181290
 test-amd64-i386-xl-vhd 21 guest-start/debian.repeat fail in 181290 pass in 
181325
 test-armhf-armhf-xl-multivcpu 12 debian-installfail pass in 181290
 test-amd64-i386-qemuu-rhel6hvm-amd 15 guest-start.2fail pass in 181290
 test-armhf-armhf-libvirt-raw 12 debian-di-install  fail pass in 181290

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-raw 15 saverestore-support-check fail in 181290 like 
181261
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail in 181290 
like 181261
 test-armhf-armhf-xl-multivcpu 15 migrate-support-check fail in 181290 never 
pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-check fail in 181290 
never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-check fail in 181290 never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-check fail in 181290 never 
pass
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 181290
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 181290
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 181290
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 181290
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 181290
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 181290
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 181290
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 181290
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 181290
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 181290
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   

Re: Xen reliance on non-standard GCC features

2023-06-09 Thread Jan Beulich
On 09.06.2023 12:12, Michal Orzel wrote:
> 
> 
> On 09/06/2023 11:47, Jan Beulich wrote:
>>
>>
>> On 09.06.2023 11:36, Michal Orzel wrote:
>>> On 09/06/2023 10:54, Jan Beulich wrote:
 On 08.06.2023 14:18, Roberto Bagnara wrote:
> On 07/06/23 09:39, Jan Beulich wrote:
>> On 05.06.2023 15:26, Roberto Bagnara wrote:
>>> On 05/06/23 11:28, Jan Beulich wrote:
 On 05.06.2023 07:28, Roberto Bagnara wrote:
> U6) Empty declarations.
>>>
>>> Examples:
>>>
>>> xen/arch/arm/arm64/lib/find_next_bit.c:57.29:
>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>
>>> xen/arch/arm/arm64/lib/find_next_bit.c:103.34:
>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>
>> Looks like these could be taken care of by finally purging our
>> EXPORT_SYMBOL() stub.
>>
>>> xen/arch/arm/include/asm/vreg.h:143.26:
>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>
>>> xen/arch/arm/include/asm/vreg.h:144.26:
>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>
>> I'm having trouble spotting anything suspicious there.
>
> The macro expands to definitions of inline functions
> and after the macro invocation there is a ";".
>
> The preprocessed code is then:
>
> static inline void foo() { ... }
> ;
>
> where the final ";" is an empty declaration not allowed by
> the C99 language standard.

 Oh, I see.

> Removing the ";" after the macro invocation is a possible solution,
> but other possibilities exist if this is strongly unwanted.

 We have other macros to instantiate functions, and there no stray
 semicolons are used. I think this wants doing the same way here, but it
 being Arm code the ultimate say is with the Arm maintainers.
>>> Apart from vreg.h the same applies to TLB_HELPER of arm32/arm64.
>>> I think also TYPE_SAFE would want to be fixed.
>>
>> Indeed. For this last one I wonder though whether it wouldn't be better
>> to continue to permit (really: require) the semicolon at the use sites,
>> by putting the typedef-s last and omitting the semicolon in the macro
>> definitions.
> This would be an error I think since the functions are defined using this type
> so it must be defined first. Unless there is a way to forward typedef.

Well, I didn't make the suggestion without first checking whether that
would (likely) be possible.

> All in all,
> removing semicolon at use sites is simpler.

Simpler - yes. But syntax-wise I think it is best if, except in special
cases, kind-of-statements and kind-of-declarations ended with a semicolon.
Special cases would be in particular ones where macro definition and
macro use are next to one another.

Jan



[xen-unstable-smoke test] 181338: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181338 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181338/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 181233
 build-armhf   6 xen-buildfail REGR. vs. 181233

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 xen  6915a120641b5d232762af7882266048cf039ca0
baseline version:
 xen  64a647f8d817c6989edc000613b5afae19f03f99

Last test of basis   181233  2023-06-07 02:04:37 Z2 days
Failing since181246  2023-06-07 11:02:03 Z2 days   23 attempts
Testing same since   181303  2023-06-08 11:00:25 Z1 days   12 attempts


People who touched revisions under test:
  Andrew Cooper 
  Christian Lindig 
  George Dunlap 
  Henry Wang  # CHANGELOG
  Juergen Gross 
  Julien Grall 
  Luca Fancellu 
  Michal Orzel 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 

jobs:
 build-arm64-xsm  fail
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 test-arm64-arm64-xl-xsm  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 504 lines long.)



Re: [PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Roger Pau Monné
On Fri, Jun 09, 2023 at 11:11:05AM +0100, Andrew Cooper wrote:
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
> 
> Drop the pair from x86-emulate.h which includes common-macros.h.
> 
> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper 

Acked-by: Roger Pau Monné 

Thanks, Roger.



Re: [PATCH v4 0/7] Resolve TYPE_PIIX3_XEN_DEVICE

2023-06-09 Thread Anthony PERARD
On Thu, Jun 08, 2023 at 03:43:32PM -0700, Stefano Stabellini wrote:
> On Mon, 5 Jun 2023, Bernhard Beschow wrote:
> > Am 22. Mai 2023 15:42:03 UTC schrieb Bernhard Beschow :
> > >
> > >
> > >Am 15. Mai 2023 20:52:40 UTC schrieb Stefano Stabellini 
> > >:
> > >>On Sat, 13 May 2023, Bernhard Beschow wrote:
> > >>> Am 21. April 2023 07:38:10 UTC schrieb "Michael S. Tsirkin" 
> > >>> :
> > >>> >On Mon, Apr 03, 2023 at 09:41:17AM +0200, Bernhard Beschow wrote:
> > >>> >> There is currently a dedicated PIIX3 device model for use under Xen. 
> > >>> >> By reusing
> > >>> >> existing PCI API during initialization this device model can be 
> > >>> >> eliminated and
> > >>> >> the plain PIIX3 device model can be used instead.
> > >>> >> 
> > >>> >> Resolving TYPE_PIIX3_XEN_DEVICE results in less code while also 
> > >>> >> making Xen
> > >>> >> agnostic towards the precise south bridge being used in the PC 
> > >>> >> machine. The
> > >>> >> latter might become particularily interesting once PIIX4 becomes 
> > >>> >> usable in the
> > >>> >> PC machine, avoiding the "Frankenstein" use of PIIX4_ACPI in PIIX3.
> > >>> >
> > >>> >xen stuff so I assume that tree?
> > >>> 
> > >>> Ping
> > >>
> > >>I am OK either way. Michael, what do you prefer?
> > >>
> > >>Normally I would suggest for you to pick up the patches. But as it
> > >>happens I'll have to likely send another pull request in a week or two
> > >>and I can add these patches to it.
> > >>
> > >>Let me know your preference and I am happy to follow it.
> > >
> > >Hi Stefano,
> > >
> > >Michael's PR was merged last week. How about including this series into 
> > >your PR then?
> > 
> > Ping
> 
> Sorry for the late reply, it looks like patch #3 breaks the build:

Hi Stefano,

Sorry I forgot to reply to these mails. I've sent a pull request for
this earlier this week (along with other patches I had to send), so the
series should be applied now.

I guess the build issue is due to trying to apply the same patch again.

Cheers,

-- 
Anthony PERARD



Re: [PATCH v2] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Andrew Cooper
On 09/06/2023 11:22 am, Luca Fancellu wrote:
>
>> On 9 Jun 2023, at 11:11, Andrew Cooper  wrote:
>>
>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>>
>> Drop the pair from x86-emulate.h which includes common-macros.h.
>>
>> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for 
>> Arm")
>> Signed-off-by: Andrew Cooper 
> I think you forgot to retain my R-by:
>
> Reviewed-by: Luca Fancellu 

I did.  I'm very sorry.  I need more coffee.

Thanks,

~Andrew



Re: [PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Luca Fancellu


> On 9 Jun 2023, at 11:11, Andrew Cooper  wrote:
> 
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
> 
> Drop the pair from x86-emulate.h which includes common-macros.h.
> 
> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper 

I think you forgot to retain my R-by:

Reviewed-by: Luca Fancellu 



> ---
> CC: Anthony PERARD 
> CC: Juergen Gross 
> CC: Luca Fancellu 
> CC: Jan Beulich 
> CC: Roger Pau Monné 
> CC: Wei Liu 
> 
> v2:
> * Drop from x86-emulate.h too.
> ---
> tools/include/xen-tools/common-macros.h | 1 +
> tools/libs/light/libxl_internal.h   | 2 --
> tools/tests/x86_emulator/x86-emulate.h  | 3 ---
> 3 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/tools/include/xen-tools/common-macros.h 
> b/tools/include/xen-tools/common-macros.h
> index d53b88182560..168691be0e93 100644
> --- a/tools/include/xen-tools/common-macros.h
> +++ b/tools/include/xen-tools/common-macros.h
> @@ -73,6 +73,7 @@
> #endif
> 
> #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> +#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> 
> #ifndef __must_check
> #define __must_check __attribute__((__warn_unused_result__))
> diff --git a/tools/libs/light/libxl_internal.h 
> b/tools/libs/light/libxl_internal.h
> index 8aba3e138909..61f4fe1dec55 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -132,8 +132,6 @@
> 
> #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
> 
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
> #define LIBXL__LOGGING_ENABLED
> 
> #ifdef LIBXL__LOGGING_ENABLED
> diff --git a/tools/tests/x86_emulator/x86-emulate.h 
> b/tools/tests/x86_emulator/x86-emulate.h
> index de1f82668010..aa1ed75ec890 100644
> --- a/tools/tests/x86_emulator/x86-emulate.h
> +++ b/tools/tests/x86_emulator/x86-emulate.h
> @@ -51,9 +51,6 @@
> #define DEFINE_PER_CPU(type, var) type per_cpu_##var
> #define this_cpu(var) per_cpu_##var
> 
> -#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
> #define __init
> #define __maybe_unused __attribute__((__unused__))
> 
> -- 
> 2.30.2
> 



Re: [PATCH v2] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Andrew Cooper
Urgh, well I'm failing at email today.  This is v2.  Everything else in
the patch is as intended.

~Andrew

On 09/06/2023 11:11 am, Andrew Cooper wrote:
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>
> Drop the pair from x86-emulate.h which includes common-macros.h.
>
> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper 
> ---
> CC: Anthony PERARD 
> CC: Juergen Gross 
> CC: Luca Fancellu 
> CC: Jan Beulich 
> CC: Roger Pau Monné 
> CC: Wei Liu 
>
> v2:
>  * Drop from x86-emulate.h too.
> ---
>  tools/include/xen-tools/common-macros.h | 1 +
>  tools/libs/light/libxl_internal.h   | 2 --
>  tools/tests/x86_emulator/x86-emulate.h  | 3 ---
>  3 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/tools/include/xen-tools/common-macros.h 
> b/tools/include/xen-tools/common-macros.h
> index d53b88182560..168691be0e93 100644
> --- a/tools/include/xen-tools/common-macros.h
> +++ b/tools/include/xen-tools/common-macros.h
> @@ -73,6 +73,7 @@
>  #endif
>  
>  #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> +#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>  
>  #ifndef __must_check
>  #define __must_check __attribute__((__warn_unused_result__))
> diff --git a/tools/libs/light/libxl_internal.h 
> b/tools/libs/light/libxl_internal.h
> index 8aba3e138909..61f4fe1dec55 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -132,8 +132,6 @@
>  
>  #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
>  
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
>  #define LIBXL__LOGGING_ENABLED
>  
>  #ifdef LIBXL__LOGGING_ENABLED
> diff --git a/tools/tests/x86_emulator/x86-emulate.h 
> b/tools/tests/x86_emulator/x86-emulate.h
> index de1f82668010..aa1ed75ec890 100644
> --- a/tools/tests/x86_emulator/x86-emulate.h
> +++ b/tools/tests/x86_emulator/x86-emulate.h
> @@ -51,9 +51,6 @@
>  #define DEFINE_PER_CPU(type, var) type per_cpu_##var
>  #define this_cpu(var) per_cpu_##var
>  
> -#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
>  #define __init
>  #define __maybe_unused __attribute__((__unused__))
>  




Re: Xen reliance on non-standard GCC features

2023-06-09 Thread Michal Orzel



On 09/06/2023 11:47, Jan Beulich wrote:
> 
> 
> On 09.06.2023 11:36, Michal Orzel wrote:
>> On 09/06/2023 10:54, Jan Beulich wrote:
>>> On 08.06.2023 14:18, Roberto Bagnara wrote:
 On 07/06/23 09:39, Jan Beulich wrote:
> On 05.06.2023 15:26, Roberto Bagnara wrote:
>> On 05/06/23 11:28, Jan Beulich wrote:
>>> On 05.06.2023 07:28, Roberto Bagnara wrote:
 U6) Empty declarations.
>>
>> Examples:
>>
>> xen/arch/arm/arm64/lib/find_next_bit.c:57.29:
>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>
>> xen/arch/arm/arm64/lib/find_next_bit.c:103.34:
>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>
> Looks like these could be taken care of by finally purging our
> EXPORT_SYMBOL() stub.
>
>> xen/arch/arm/include/asm/vreg.h:143.26:
>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>
>> xen/arch/arm/include/asm/vreg.h:144.26:
>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>
> I'm having trouble spotting anything suspicious there.

 The macro expands to definitions of inline functions
 and after the macro invocation there is a ";".

 The preprocessed code is then:

 static inline void foo() { ... }
 ;

 where the final ";" is an empty declaration not allowed by
 the C99 language standard.
>>>
>>> Oh, I see.
>>>
 Removing the ";" after the macro invocation is a possible solution,
 but other possibilities exist if this is strongly unwanted.
>>>
>>> We have other macros to instantiate functions, and there no stray
>>> semicolons are used. I think this wants doing the same way here, but it
>>> being Arm code the ultimate say is with the Arm maintainers.
>> Apart from vreg.h the same applies to TLB_HELPER of arm32/arm64.
>> I think also TYPE_SAFE would want to be fixed.
> 
> Indeed. For this last one I wonder though whether it wouldn't be better
> to continue to permit (really: require) the semicolon at the use sites,
> by putting the typedef-s last and omitting the semicolon in the macro
> definitions.
This would be an error I think since the functions are defined using this type
so it must be defined first. Unless there is a way to forward typedef. All in 
all,
removing semicolon at use sites is simpler.

~Michal



Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Julien Grall

Hi Andrew,

On 09/06/2023 10:54, Andrew Cooper wrote:

On 09/06/2023 10:46 am, Julien Grall wrote:

On 09/06/2023 10:43, Andrew Cooper wrote:

On 09/06/2023 10:38 am, Jan Beulich wrote:

On 09.06.2023 11:29, Andrew Cooper wrote:

On 09/06/2023 10:22 am, Jan Beulich wrote:

--- /dev/null
+++ b/xen/arch/ppc/boot_of.c
@@ -0,0 +1,122 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */

By default we mean to use ...


--- /dev/null
+++ b/xen/arch/ppc/early_printk.c
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */

... the more modern form of this (GPL-2.0-only). Anything
deviating from
that may want justifying in the description.

GPL-2.0-or-later is fine.

Hmm, I was merely following
https://lists.xen.org/archives/html/xen-devel/2023-06/msg00415.html.
The text at the top of ./COPYING looks to suggest -only, and I'm
unaware of any other place where our default is actually written down.


The license is chosen by the submitter/copyright holder, based on their
preferences/wishes.

It's fine for Xen to say "if you've got no vested interest, we recommend
GPL-2.0-only", but that is strictly a recommendation and no more.

If the submitter chooses GPL-2.0-or-later, that is their prerogative.
We have plenty of GPL-2.0-or-later code in Xen.


 From my past experience, the submitters tend to just copy the license
from an existing file in Xen rather than explicitly choosing it. So I
think it is fair to ask the question because our original and default
license is GPLv2 nor GPLv2+.


Did you read the bit in the cover letter about part of this code being
derived from the out-of-tree port years ago?


Yes I read it... But I didn't check the original license and ...



You're blindly assuming that there is even a choice of license available
to be used.


... I didn't assume anything here. I made a generic statement because 
your e-mail lead to think that all the submitter made a conscious decision.


Note that, from past discussion, we agreed that it would be fine to 
re-license from gplv2+ to gplv2-only without requesting the original 
author. So technically there is a choice.


As a side note, "blindly" is not very inclusive. We may have different 
view, but it doesn't mean yours is better than mine (and vice-versa). 
You could have express your opinion without saying "blindly" and it 
would have come across less rude.



The submitter chooses the license to use.  You can request that they
justify it, but you cannot demand that they change it.
Strictly speaking we can refuse any code. That count for license as 
well. Anyway, I didn't request a change here. I merely pointed out that 
any use of GPLv2+ should be justified because on Arm most of the people 
don't pay attention on the license and pick the one from an existing file.


Cheers,

--
Julien Grall



[PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Andrew Cooper
MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.

Drop the pair from x86-emulate.h which includes common-macros.h.

Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
Signed-off-by: Andrew Cooper 
---
CC: Anthony PERARD 
CC: Juergen Gross 
CC: Luca Fancellu 
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 

v2:
 * Drop from x86-emulate.h too.
---
 tools/include/xen-tools/common-macros.h | 1 +
 tools/libs/light/libxl_internal.h   | 2 --
 tools/tests/x86_emulator/x86-emulate.h  | 3 ---
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/include/xen-tools/common-macros.h 
b/tools/include/xen-tools/common-macros.h
index d53b88182560..168691be0e93 100644
--- a/tools/include/xen-tools/common-macros.h
+++ b/tools/include/xen-tools/common-macros.h
@@ -73,6 +73,7 @@
 #endif
 
 #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
+#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
 
 #ifndef __must_check
 #define __must_check __attribute__((__warn_unused_result__))
diff --git a/tools/libs/light/libxl_internal.h 
b/tools/libs/light/libxl_internal.h
index 8aba3e138909..61f4fe1dec55 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -132,8 +132,6 @@
 
 #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
 
-#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
-
 #define LIBXL__LOGGING_ENABLED
 
 #ifdef LIBXL__LOGGING_ENABLED
diff --git a/tools/tests/x86_emulator/x86-emulate.h 
b/tools/tests/x86_emulator/x86-emulate.h
index de1f82668010..aa1ed75ec890 100644
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -51,9 +51,6 @@
 #define DEFINE_PER_CPU(type, var) type per_cpu_##var
 #define this_cpu(var) per_cpu_##var
 
-#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
-#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
-
 #define __init
 #define __maybe_unused __attribute__((__unused__))
 
-- 
2.30.2




Re: [PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Luca Fancellu


> On 9 Jun 2023, at 11:06, Andrew Cooper  wrote:
> 
> On 08/06/2023 8:37 pm, Luca Fancellu wrote:
>>> On 8 Jun 2023, at 18:40, Andrew Cooper  wrote:
>>> 
>>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>>> 
>>> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for 
>>> Arm")
>> I don’t think this patch is fixing a bug:
>> 
>> ### Fixes:
>> 
>> If your patch fixes a bug in a specific commit, e.g. you found an issue using
>> ``git bisect``, please use the `Fixes:` tag with the first 12 characters of
>> the commit id, and the one line summary.
> 
> That a poor explanation...
> 
> Fixes: is about corrections to the patch, not bugs.
> 
> 56a7aaa16bfe is unlikely to be backported, but if a downstream were to
> backport your SVE patches, Fixes: identify all other patches they need
> to take.
> 
> Fixes: was specifically invented to let tooling (partially) automate the
> task if finding new patches to backport, based on what had already been
> backported.

Ok this makes sense, so that a tool can easily understand where to put the 
focus.

> 
> Concrete bugs are the majority reason for a Fixes tag, sure, but not the
> only reason.  In this case, a downstream absolutely doesn't want to get
> into a position where these macros aren't together in a pair, because it
> there will be a case in the future where it causes a build error.
> 
>>> Signed-off-by: Andrew Cooper 
>> But it makes sense, so 
>> 
>> Reviewed-by: Luca Fancellu 
> 
> Thanks.  As you've already indicated that you're ok with fixing up
> x86-emulate.h in v2, I'll retain this.

sure, thanks for fixing it

> 
> ~Andrew



Re: [PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Andrew Cooper
On 09/06/2023 10:33 am, Andrew Cooper wrote:
> On 09/06/2023 10:31 am, Jan Beulich wrote:
>> On 08.06.2023 19:40, Andrew Cooper wrote:
>>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>> Right, that's one thing which should have been done right away.
>> The other is that both macros should have been purged from
>> tools/tests/x86_emulator/x86-emulate.h (which includes
>> xen-tools/common-macros.h). Luca?
> Hmm - I explicitly checked that, and concluded they didn't...  Happy to
> purge them if I'm wrong.

I think I was looking in an old branch.  Fixed for v2.

~Andrew



Re: [PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Andrew Cooper
On 08/06/2023 8:37 pm, Luca Fancellu wrote:
>> On 8 Jun 2023, at 18:40, Andrew Cooper  wrote:
>>
>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>>
>> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for 
>> Arm")
> I don’t think this patch is fixing a bug:
>
> ### Fixes:
>
> If your patch fixes a bug in a specific commit, e.g. you found an issue using
> ``git bisect``, please use the `Fixes:` tag with the first 12 characters of
> the commit id, and the one line summary.

That a poor explanation...

Fixes: is about corrections to the patch, not bugs.

56a7aaa16bfe is unlikely to be backported, but if a downstream were to
backport your SVE patches, Fixes: identify all other patches they need
to take.

Fixes: was specifically invented to let tooling (partially) automate the
task if finding new patches to backport, based on what had already been
backported.

Concrete bugs are the majority reason for a Fixes tag, sure, but not the
only reason.  In this case, a downstream absolutely doesn't want to get
into a position where these macros aren't together in a pair, because it
there will be a case in the future where it causes a build error.

>> Signed-off-by: Andrew Cooper 
> But it makes sense, so 
>
> Reviewed-by: Luca Fancellu 

Thanks.  As you've already indicated that you're ok with fixing up
x86-emulate.h in v2, I'll retain this.

~Andrew



Re: [PATCH] tools/ocaml/xc: Fix xc_physinfo() bindings

2023-06-09 Thread Christian Lindig



> On 9 Jun 2023, at 10:37, Andrew Cooper  wrote:
> 
> On 09/06/2023 9:17 am, Christian Lindig wrote:
>>> On 8 Jun 2023, at 20:33, Andrew Cooper  wrote:
>>> 
>>> +type arm_physinfo_caps =
>>> +  {
>>> +sve_vl: int;
>>> +  }
>>> +
>> 
>> Does the OCaml side need to know about the structure of this value or is it 
>> enough to pass it around as an abstract value because all logic is on the C 
>> side? I assume the OCaml side needs at least a way to persist the value and 
>> hence needs to know some representation.
> 
> Yes, Ocaml does need to know about the structure.
> 
> [..]
> 
> The vector length is a multiple of 128.  This value is more amenable to
> being understood in a log file by a human.


Acked-by: Christian Lindig 




Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Andrew Cooper
On 09/06/2023 10:46 am, Julien Grall wrote:
> On 09/06/2023 10:43, Andrew Cooper wrote:
>> On 09/06/2023 10:38 am, Jan Beulich wrote:
>>> On 09.06.2023 11:29, Andrew Cooper wrote:
 On 09/06/2023 10:22 am, Jan Beulich wrote:
>> --- /dev/null
>> +++ b/xen/arch/ppc/boot_of.c
>> @@ -0,0 +1,122 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> By default we mean to use ...
>
>> --- /dev/null
>> +++ b/xen/arch/ppc/early_printk.c
>> @@ -0,0 +1,36 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
> ... the more modern form of this (GPL-2.0-only). Anything
> deviating from
> that may want justifying in the description.
 GPL-2.0-or-later is fine.
>>> Hmm, I was merely following
>>> https://lists.xen.org/archives/html/xen-devel/2023-06/msg00415.html.
>>> The text at the top of ./COPYING looks to suggest -only, and I'm
>>> unaware of any other place where our default is actually written down.
>>
>> The license is chosen by the submitter/copyright holder, based on their
>> preferences/wishes.
>>
>> It's fine for Xen to say "if you've got no vested interest, we recommend
>> GPL-2.0-only", but that is strictly a recommendation and no more.
>>
>> If the submitter chooses GPL-2.0-or-later, that is their prerogative.
>> We have plenty of GPL-2.0-or-later code in Xen.
>
> From my past experience, the submitters tend to just copy the license
> from an existing file in Xen rather than explicitly choosing it. So I
> think it is fair to ask the question because our original and default
> license is GPLv2 nor GPLv2+.

Did you read the bit in the cover letter about part of this code being
derived from the out-of-tree port years ago?

You're blindly assuming that there is even a choice of license available
to be used.

The submitter chooses the license to use.  You can request that they
justify it, but you cannot demand that they change it.

~Andrew



Re: Xen reliance on non-standard GCC features

2023-06-09 Thread Jan Beulich
On 09.06.2023 11:36, Michal Orzel wrote:
> On 09/06/2023 10:54, Jan Beulich wrote:
>> On 08.06.2023 14:18, Roberto Bagnara wrote:
>>> On 07/06/23 09:39, Jan Beulich wrote:
 On 05.06.2023 15:26, Roberto Bagnara wrote:
> On 05/06/23 11:28, Jan Beulich wrote:
>> On 05.06.2023 07:28, Roberto Bagnara wrote:
>>> U6) Empty declarations.
>
> Examples:
>
> xen/arch/arm/arm64/lib/find_next_bit.c:57.29:
> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
> `/usr/bin/aarch64-linux-gnu-gcc-12'
>
> xen/arch/arm/arm64/lib/find_next_bit.c:103.34:
> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
> `/usr/bin/aarch64-linux-gnu-gcc-12'

 Looks like these could be taken care of by finally purging our
 EXPORT_SYMBOL() stub.

> xen/arch/arm/include/asm/vreg.h:143.26:
> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
> `/usr/bin/aarch64-linux-gnu-gcc-12'
>
> xen/arch/arm/include/asm/vreg.h:144.26:
> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
> `/usr/bin/aarch64-linux-gnu-gcc-12'

 I'm having trouble spotting anything suspicious there.
>>>
>>> The macro expands to definitions of inline functions
>>> and after the macro invocation there is a ";".
>>>
>>> The preprocessed code is then:
>>>
>>> static inline void foo() { ... }
>>> ;
>>>
>>> where the final ";" is an empty declaration not allowed by
>>> the C99 language standard.
>>
>> Oh, I see.
>>
>>> Removing the ";" after the macro invocation is a possible solution,
>>> but other possibilities exist if this is strongly unwanted.
>>
>> We have other macros to instantiate functions, and there no stray
>> semicolons are used. I think this wants doing the same way here, but it
>> being Arm code the ultimate say is with the Arm maintainers.
> Apart from vreg.h the same applies to TLB_HELPER of arm32/arm64.
> I think also TYPE_SAFE would want to be fixed.

Indeed. For this last one I wonder though whether it wouldn't be better
to continue to permit (really: require) the semicolon at the use sites,
by putting the typedef-s last and omitting the semicolon in the macro
definitions.

Jan



Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Julien Grall

Hi Andrew,

On 09/06/2023 10:43, Andrew Cooper wrote:

On 09/06/2023 10:38 am, Jan Beulich wrote:

On 09.06.2023 11:29, Andrew Cooper wrote:

On 09/06/2023 10:22 am, Jan Beulich wrote:

--- /dev/null
+++ b/xen/arch/ppc/boot_of.c
@@ -0,0 +1,122 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */

By default we mean to use ...


--- /dev/null
+++ b/xen/arch/ppc/early_printk.c
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */

... the more modern form of this (GPL-2.0-only). Anything deviating from
that may want justifying in the description.

GPL-2.0-or-later is fine.

Hmm, I was merely following
https://lists.xen.org/archives/html/xen-devel/2023-06/msg00415.html.
The text at the top of ./COPYING looks to suggest -only, and I'm
unaware of any other place where our default is actually written down.


The license is chosen by the submitter/copyright holder, based on their
preferences/wishes.

It's fine for Xen to say "if you've got no vested interest, we recommend
GPL-2.0-only", but that is strictly a recommendation and no more.

If the submitter chooses GPL-2.0-or-later, that is their prerogative.
We have plenty of GPL-2.0-or-later code in Xen.


From my past experience, the submitters tend to just copy the license 
from an existing file in Xen rather than explicitly choosing it. So I 
think it is fair to ask the question because our original and default 
license is GPLv2 nor GPLv2+.


Cheers,

--
Julien Grall



Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Andrew Cooper
On 09/06/2023 10:38 am, Jan Beulich wrote:
> On 09.06.2023 11:29, Andrew Cooper wrote:
>> On 09/06/2023 10:22 am, Jan Beulich wrote:
 --- /dev/null
 +++ b/xen/arch/ppc/boot_of.c
 @@ -0,0 +1,122 @@
 +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>> By default we mean to use ...
>>>
 --- /dev/null
 +++ b/xen/arch/ppc/early_printk.c
 @@ -0,0 +1,36 @@
 +/* SPDX-License-Identifier: GPL-2.0 */
>>> ... the more modern form of this (GPL-2.0-only). Anything deviating from
>>> that may want justifying in the description.
>> GPL-2.0-or-later is fine.
> Hmm, I was merely following
> https://lists.xen.org/archives/html/xen-devel/2023-06/msg00415.html.
> The text at the top of ./COPYING looks to suggest -only, and I'm
> unaware of any other place where our default is actually written down.

The license is chosen by the submitter/copyright holder, based on their
preferences/wishes.

It's fine for Xen to say "if you've got no vested interest, we recommend
GPL-2.0-only", but that is strictly a recommendation and no more.

If the submitter chooses GPL-2.0-or-later, that is their prerogative. 
We have plenty of GPL-2.0-or-later code in Xen.

~Andrew



Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Julien Grall

Hi,

On 09/06/2023 10:38, Jan Beulich wrote:

On 09.06.2023 11:29, Andrew Cooper wrote:

On 09/06/2023 10:22 am, Jan Beulich wrote:

--- /dev/null
+++ b/xen/arch/ppc/boot_of.c
@@ -0,0 +1,122 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */

By default we mean to use ...


--- /dev/null
+++ b/xen/arch/ppc/early_printk.c
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */

... the more modern form of this (GPL-2.0-only). Anything deviating from
that may want justifying in the description.


GPL-2.0-or-later is fine.


Hmm, I was merely following
https://lists.xen.org/archives/html/xen-devel/2023-06/msg00415.html.
The text at the top of ./COPYING looks to suggest -only, and I'm
unaware of any other place where our default is actually written down.


We had several discussion in the past about using GPLv2+ license in Xen 
(see [1], [2]). It has always been unclear what is the impact on 
contribution with such license. So I think we should stick with GPL-2.0 
for new code unless there is a reason to diverge.


Cheers,

[1] 
https://patchwork.kernel.org/project/xen-devel/patch/1474985810-12289-1-git-send-email-lars.ku...@citrix.com/#19650817
[2] 
https://lore.kernel.org/all/alpine.DEB.2.22.394.2208231140140.15247@ubuntu-linux-20-04-desktop/


--
Julien Grall



Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Jan Beulich
On 09.06.2023 11:29, Andrew Cooper wrote:
> On 09/06/2023 10:22 am, Jan Beulich wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/ppc/boot_of.c
>>> @@ -0,0 +1,122 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> By default we mean to use ...
>>
>>> --- /dev/null
>>> +++ b/xen/arch/ppc/early_printk.c
>>> @@ -0,0 +1,36 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>> ... the more modern form of this (GPL-2.0-only). Anything deviating from
>> that may want justifying in the description.
> 
> GPL-2.0-or-later is fine.

Hmm, I was merely following
https://lists.xen.org/archives/html/xen-devel/2023-06/msg00415.html.
The text at the top of ./COPYING looks to suggest -only, and I'm
unaware of any other place where our default is actually written down.

Jan

> It's only the un-qualified GPL-2.0 form which is deprecated, and should
> be replaced with an -or-later or -only suffix, as chosen by the
> copyright holder.
> 
> ~Andrew




Re: [PATCH] tools/ocaml/xc: Fix xc_physinfo() bindings

2023-06-09 Thread Andrew Cooper
On 09/06/2023 9:17 am, Christian Lindig wrote:
>> On 8 Jun 2023, at 20:33, Andrew Cooper  wrote:
>>
>> +type arm_physinfo_caps =
>> +  {
>> +sve_vl: int;
>> +  }
>> +
>
> Does the OCaml side need to know about the structure of this value or is it 
> enough to pass it around as an abstract value because all logic is on the C 
> side? I assume the OCaml side needs at least a way to persist the value and 
> hence needs to know some representation.

Yes, Ocaml does need to know about the structure.

info->arch_capabilities is a collection of misc info.  It was intended
to be just a bitmap of things, hence why it's a list on the x86 side (We
added fields, then had to revert and I haven't got back around to
reworking that yet).

But now ARM have put a non-bit field field into it, where a toolstack
needs to select a domain_create sve_vl setting between min(0,
physinfo.sve_vl)

The Ocaml code doesn't have an #include  so doesn't
have the masks/constants required to decompose the integer into it's
constitute parts.

>
>>  Store_field(arch_obj, 0,
>> +Val_int(MASK_EXTR(info->arch_capabilities,
>> +  XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK) * 128));
>> +
> What is the “* 128” achieving as part of this encoding?

No functional change from before...  except it was previously hidden in
a different header file which I'm in the process of deleting.

The vector length is a multiple of 128.  This value is more amenable to
being understood in a log file by a human.

~Andrew



Re: [PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Luca Fancellu


> On 9 Jun 2023, at 10:31, Jan Beulich  wrote:
> 
> On 08.06.2023 19:40, Andrew Cooper wrote:
>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
> 
> Right, that's one thing which should have been done right away.
> The other is that both macros should have been purged from
> tools/tests/x86_emulator/x86-emulate.h (which includes
> xen-tools/common-macros.h). Luca?

mmm right I’ve missed that, if Andrew can handle it in this patch I’m ok, if you
want me to send a patch I can do it

> 
> Jan
> 
>> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for 
>> Arm")
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Anthony PERARD 
>> CC: Juergen Gross 
>> CC: Luca Fancellu 
>> ---
>> tools/include/xen-tools/common-macros.h | 1 +
>> tools/libs/light/libxl_internal.h   | 2 --
>> 2 files changed, 1 insertion(+), 2 deletions(-)
>> 
>> diff --git a/tools/include/xen-tools/common-macros.h 
>> b/tools/include/xen-tools/common-macros.h
>> index d53b88182560..168691be0e93 100644
>> --- a/tools/include/xen-tools/common-macros.h
>> +++ b/tools/include/xen-tools/common-macros.h
>> @@ -73,6 +73,7 @@
>> #endif
>> 
>> #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
>> +#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>> 
>> #ifndef __must_check
>> #define __must_check __attribute__((__warn_unused_result__))
>> diff --git a/tools/libs/light/libxl_internal.h 
>> b/tools/libs/light/libxl_internal.h
>> index 8aba3e138909..61f4fe1dec55 100644
>> --- a/tools/libs/light/libxl_internal.h
>> +++ b/tools/libs/light/libxl_internal.h
>> @@ -132,8 +132,6 @@
>> 
>> #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
>> 
>> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>> -
>> #define LIBXL__LOGGING_ENABLED
>> 
>> #ifdef LIBXL__LOGGING_ENABLED
> 



Re: Xen reliance on non-standard GCC features

2023-06-09 Thread Michal Orzel



On 09/06/2023 10:54, Jan Beulich wrote:
> 
> 
> On 08.06.2023 14:18, Roberto Bagnara wrote:
>> On 07/06/23 09:39, Jan Beulich wrote:
>>> On 05.06.2023 15:26, Roberto Bagnara wrote:
 On 05/06/23 11:28, Jan Beulich wrote:
> On 05.06.2023 07:28, Roberto Bagnara wrote:
 You are right: here are a few examples for U2:

 xen/arch/arm/cpuerrata.c:92.12-92.35:
 empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 
 Section 6.7.8: "An empty initialization list." [STD.emptinit]). Tool used 
 is `/usr/bin/aarch64-linux-gnu-gcc-12'
 xen/include/xen/spinlock.h:31.21-31.23: expanded from macro `_LOCK_DEBUG'
 xen/include/xen/spinlock.h:143.57-143.67: expanded from macro 
 `SPIN_LOCK_UNLOCKED'
 xen/include/xen/spinlock.h:144.43-144.60: expanded from macro 
 `DEFINE_SPINLOCK'
>>>
>>> I'm afraid this is a bad example, as it goes hand-in-hand with using
>>> another extension. I don't think using a non-empty initialization list
>>> is going to work with
>>>
>>> union lock_debug { };
>>
>> Yes, this is C99 undefined behavior 58:
>> "A structure or union is defined as containing no named members (6.7.2.1)."
>>
>> Here is another example:
>>
>> lpae_t pte = {};
>>
>> whereas we have
>>
>> typedef union {
>>  uint64_t bits;
>>  lpae_pt_t pt;
>>  lpae_p2m_t p2m;
>>  lpae_walk_t walk;
>> } lpae_t;
>>
>>
 xen/arch/arm/cpuerrata.c:678.5-678.6:
 empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 
 Section 6.7.8: "An empty initialization list." [STD.emptinit]). Tool used 
 is `/usr/bin/aarch64-linux-gnu-gcc-12'

 xen/arch/arm/cpufeature.c:33.5-33.6:
 empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 
 Section 6.7.8: "An empty initialization list." [STD.emptinit]). Tool used 
 is `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>
>>> Both of these are a common idiom we use: The "sentinel" of an array
>>> of compound type initializer.
>>
>> Wouldn't it be possible writing such sentinels in a standard-compliant
>> way, like {0} or similar, instead of {}?
> 
> I would be possible, sure, but the question is whether we want that. Iirc
> in review comments we've been asking to preferably use {}, for being
> shorter / less clutter without resulting in any ambiguity.
> 
>> U6) Empty declarations.

 Examples:

 xen/arch/arm/arm64/lib/find_next_bit.c:57.29:
 empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
 Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
 `/usr/bin/aarch64-linux-gnu-gcc-12'

 xen/arch/arm/arm64/lib/find_next_bit.c:103.34:
 empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
 Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
 `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>
>>> Looks like these could be taken care of by finally purging our
>>> EXPORT_SYMBOL() stub.
>>>
 xen/arch/arm/include/asm/vreg.h:143.26:
 empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
 Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
 `/usr/bin/aarch64-linux-gnu-gcc-12'

 xen/arch/arm/include/asm/vreg.h:144.26:
 empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
 Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
 `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>
>>> I'm having trouble spotting anything suspicious there.
>>
>> The macro expands to definitions of inline functions
>> and after the macro invocation there is a ";".
>>
>> The preprocessed code is then:
>>
>> static inline void foo() { ... }
>> ;
>>
>> where the final ";" is an empty declaration not allowed by
>> the C99 language standard.
> 
> Oh, I see.
> 
>> Removing the ";" after the macro invocation is a possible solution,
>> but other possibilities exist if this is strongly unwanted.
> 
> We have other macros to instantiate functions, and there no stray
> semicolons are used. I think this wants doing the same way here, but it
> being Arm code the ultimate say is with the Arm maintainers.
Apart from vreg.h the same applies to TLB_HELPER of arm32/arm64.
I think also TYPE_SAFE would want to be fixed.

~Michal



Re: [PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Andrew Cooper
On 09/06/2023 10:31 am, Jan Beulich wrote:
> On 08.06.2023 19:40, Andrew Cooper wrote:
>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
> Right, that's one thing which should have been done right away.
> The other is that both macros should have been purged from
> tools/tests/x86_emulator/x86-emulate.h (which includes
> xen-tools/common-macros.h). Luca?

Hmm - I explicitly checked that, and concluded they didn't...  Happy to
purge them if I'm wrong.

~Andrew



Re: [PATCH] tools: Move MASK_INSR to common-macros.h

2023-06-09 Thread Jan Beulich
On 08.06.2023 19:40, Andrew Cooper wrote:
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.

Right, that's one thing which should have been done right away.
The other is that both macros should have been purged from
tools/tests/x86_emulator/x86-emulate.h (which includes
xen-tools/common-macros.h). Luca?

Jan

> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper 
> ---
> CC: Anthony PERARD 
> CC: Juergen Gross 
> CC: Luca Fancellu 
> ---
>  tools/include/xen-tools/common-macros.h | 1 +
>  tools/libs/light/libxl_internal.h   | 2 --
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/include/xen-tools/common-macros.h 
> b/tools/include/xen-tools/common-macros.h
> index d53b88182560..168691be0e93 100644
> --- a/tools/include/xen-tools/common-macros.h
> +++ b/tools/include/xen-tools/common-macros.h
> @@ -73,6 +73,7 @@
>  #endif
>  
>  #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> +#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>  
>  #ifndef __must_check
>  #define __must_check __attribute__((__warn_unused_result__))
> diff --git a/tools/libs/light/libxl_internal.h 
> b/tools/libs/light/libxl_internal.h
> index 8aba3e138909..61f4fe1dec55 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -132,8 +132,6 @@
>  
>  #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
>  
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
>  #define LIBXL__LOGGING_ENABLED
>  
>  #ifdef LIBXL__LOGGING_ENABLED




Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Andrew Cooper
On 09/06/2023 10:22 am, Jan Beulich wrote:
>> --- /dev/null
>> +++ b/xen/arch/ppc/boot_of.c
>> @@ -0,0 +1,122 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> By default we mean to use ...
>
>> --- /dev/null
>> +++ b/xen/arch/ppc/early_printk.c
>> @@ -0,0 +1,36 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
> ... the more modern form of this (GPL-2.0-only). Anything deviating from
> that may want justifying in the description.

GPL-2.0-or-later is fine.

It's only the un-qualified GPL-2.0 form which is deprecated, and should
be replaced with an -or-later or -only suffix, as chosen by the
copyright holder.

~Andrew



Re: [PATCH 2/3] xen/ppc: Implement early serial printk on PaPR/pseries

2023-06-09 Thread Jan Beulich
On 07.06.2023 17:06, Shawn Anastasio wrote:
> On typical Power VMs (e.g. QEMU's -M pseries), a variety of services
> are provided by OpenFirmware, including an early serial console.
> Implement the required interfaces to call into OpenFirmware and write
> to the serial console.
> 
> Since OpenFirmware runs in 32-bit Big Endian mode and Xen runs in
> 64-bit Little Endian mode, a thunk is required to save/restore
> any potentially-clobbered registers as well as to perform the
> required endianness switch. Thankfully, linux already has such
> a routine, which was imported into head.S.
> 
> Support for bare metal (PowerNV) will be implemented in a future
> patch.
> 
> Signed-off-by: Shawn Anastasio 

Just a couple of nits:

>  xen/arch/ppc/Kconfig.debug   |   5 +
>  xen/arch/ppc/Makefile|   3 +-
>  xen/arch/ppc/boot_of.c   | 122 +++
>  xen/arch/ppc/configs/openpower_defconfig |   1 +
>  xen/arch/ppc/early_printk.c  |  36 +++
>  xen/arch/ppc/include/asm/boot.h  |  31 ++
>  xen/arch/ppc/include/asm/bug.h   |   6 ++
>  xen/arch/ppc/include/asm/byteorder.h |  74 ++
>  xen/arch/ppc/include/asm/cache.h |   6 ++
>  xen/arch/ppc/include/asm/config.h|   3 +
>  xen/arch/ppc/include/asm/early_printk.h  |  14 +++
>  xen/arch/ppc/include/asm/processor.h |  54 +-
>  xen/arch/ppc/include/asm/string.h|   6 ++
>  xen/arch/ppc/include/asm/types.h |  64 
>  xen/arch/ppc/ppc64/asm-offsets.c |  55 ++
>  xen/arch/ppc/ppc64/head.S|  59 +++
>  xen/arch/ppc/setup.c |  20 +++-
>  17 files changed, 555 insertions(+), 4 deletions(-)
>  create mode 100644 xen/arch/ppc/boot_of.c

Unless required, in new additions we tend to prefer dashes over
underscores. In filenames it is pretty rare that dashes really need
avoiding.

> --- a/xen/arch/ppc/Kconfig.debug
> +++ b/xen/arch/ppc/Kconfig.debug
> @@ -0,0 +1,5 @@
> +config EARLY_PRINTK
> +bool "Enable early printk"
> +default DEBUG
> +help
> +  Enables early printk debug messages
> \ No newline at end of file

There are many examples of this throughout the patch, which you want to
take care of.

> --- /dev/null
> +++ b/xen/arch/ppc/boot_of.c
> @@ -0,0 +1,122 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */

By default we mean to use ...

> --- /dev/null
> +++ b/xen/arch/ppc/early_printk.c
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

... the more modern form of this (GPL-2.0-only). Anything deviating from
that may want justifying in the description.

Jan



[xen-unstable-smoke test] 181333: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181333 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181333/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 181233
 build-armhf   6 xen-buildfail REGR. vs. 181233

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 xen  6915a120641b5d232762af7882266048cf039ca0
baseline version:
 xen  64a647f8d817c6989edc000613b5afae19f03f99

Last test of basis   181233  2023-06-07 02:04:37 Z2 days
Failing since181246  2023-06-07 11:02:03 Z1 days   22 attempts
Testing same since   181303  2023-06-08 11:00:25 Z0 days   11 attempts


People who touched revisions under test:
  Andrew Cooper 
  Christian Lindig 
  George Dunlap 
  Henry Wang  # CHANGELOG
  Juergen Gross 
  Julien Grall 
  Luca Fancellu 
  Michal Orzel 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 

jobs:
 build-arm64-xsm  fail
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 test-arm64-arm64-xl-xsm  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 504 lines long.)



Re: [PATCH 1/3] xen: Add files needed for minimal Power build

2023-06-09 Thread Jan Beulich
On 07.06.2023 17:06, Shawn Anastasio wrote:
> Add the build system changes required to build for ppc64le (POWER8+).
> Following in the footsteps of the initial riscv port, only building
> the head.o target, which boots to an infinite loop, is supported:
> 
> $ make XEN_TARGET_ARCH=ppc64 -C xen openpower_defconfig
> $ make XEN_TARGET_ARCH=ppc64 SUBSYSTEMS=xen -C xen TARGET=ppc64/head.o
> 
> This port targets POWER8+ CPUs running in Little Endian mode specifically,
> and does not boot on older machines. Additionally, this initial skeleton
> only implements the PaPR/pseries boot protocol which allows it to be
> booted in a standard QEMU virtual machine:
> 
> $ qemu-system-ppc64 -M pseries-5.2 -m 256M -kernel ppc64/head.o
> 
> Where possible, this patch uses header definitions and support routines
> from the original Xen PowerPC port present in Xen 3.2.3. Though we are
> targeting a much newer ISA than that original port did, some of the
> definitions have remained similar enough for reuse.
> 
> Signed-off-by: Shawn Anastasio 

Just a few small remarks, as following Andrew's comments I expect the
patch will shrink quite a bit:

> --- /dev/null
> +++ b/config/ppc64.mk
> @@ -0,0 +1,5 @@
> +CONFIG_PPC64 := y
> +CONFIG_PPC64_64 := y
> +CONFIG_PPC64_$(XEN_OS) := y

The first of the 64-s here are a little odd; looking at RISC-V's
counterpart, wouldn't this want to be

CONFIG_PPC := y
CONFIG_PPC_64 := y
CONFIG_PPC_$(XEN_OS) := y

> --- /dev/null
> +++ b/xen/arch/ppc/Kconfig
> @@ -0,0 +1,42 @@
> +config PPC
> + def_bool y

Is this necessary? Iirc PPC is frequently used as a name for 32-bit PPC
(but then also elsewhere as covering both 32- and 64-bit), so I'm not
sure we want this without having a need for it.

> +config PPC64
> + def_bool y
> + select 64BIT
> +
> +config ARCH_DEFCONFIG
> + string
> + default "arch/ppc/configs/openpower_defconfig"
> +
> +menu "Architecture Features"
> +
> +source "arch/Kconfig"
> +
> +endmenu
> +
> +menu "ISA Selection"
> +
> +choice
> + prompt "Base ISA"
> + default POWER_ISA_2_07B if PPC64
> + help
> +   This selects the base ISA version that Xen will target.
> +
> +config POWER_ISA_2_07B
> + bool "POWER ISA 2.07B+"
> + help
> +   Target version 2.07B+ of the POWER ISA (POWER8+)
> +
> +config POWER_ISA_3_00
> + bool "POWER ISA 3.00+"
> + help
> +   Target version 3.00+ of the POWER ISA (POWER9+)

What are the + in here meant to indicate? Since this is about a baseline
ISA, I find such a use (presumably standing for "or newer") ambiguous.

Jan



[linux-linus test] 181320: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181320 linux-linus real [real]
flight 181332 linux-linus real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/181320/
http://logs.test-lab.xenproject.org/osstest/logs/181332/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 180278
 test-amd64-amd64-libvirt-qcow2 19 guest-start/debian.repeat fail REGR. vs. 
180278

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-vhd 21 guest-start/debian.repeat fail pass in 181332-retest

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-examine  8 reboot   fail  like 180278
 test-armhf-armhf-xl-multivcpu  8 xen-boot fail like 180278
 test-armhf-armhf-xl-arndale   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278
 test-armhf-armhf-xl-credit2   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278
 test-armhf-armhf-libvirt  8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-raw  8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-qcow2  8 xen-bootfail like 180278
 test-armhf-armhf-xl   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-vhd   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-rtds  8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 linux8d15d5e1851b1bbb9cd3289b84c7f32399e06ac5
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z   53 days
Failing since180281  2023-04-17 06:24:36 Z   53 days  100 attempts
Testing same since   181320  2023-06-08 23:09:56 Z0 days1 attempts


2654 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-ar

Re: [PATCH 3/3] maintainers: Add PPC64 maintainer

2023-06-09 Thread Jan Beulich
On 07.06.2023 17:06, Shawn Anastasio wrote:
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -451,6 +451,9 @@ M:Wei Liu 
>  S:   Supported
>  T:   git https://xenbits.xenproject.org/git-http/ovmf.git
>  
> +PPC64
> +M:   Shawn Anastasio 
> +
>  POWER MANAGEMENT
>  M:   Jan Beulich 
>  S:   Supported

Nit: You want to move your insertion further down, to maintain alphabetic
sorting. You further want at least one F: line; see e.g. RISC-V's entry.

Jan



Re: Xen reliance on non-standard GCC features

2023-06-09 Thread Jan Beulich
On 08.06.2023 14:18, Roberto Bagnara wrote:
> On 07/06/23 09:39, Jan Beulich wrote:
>> On 05.06.2023 15:26, Roberto Bagnara wrote:
>>> On 05/06/23 11:28, Jan Beulich wrote:
 On 05.06.2023 07:28, Roberto Bagnara wrote:
>>> You are right: here are a few examples for U2:
>>>
>>> xen/arch/arm/cpuerrata.c:92.12-92.35:
>>> empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7.8: "An empty initialization list." [STD.emptinit]). Tool used 
>>> is `/usr/bin/aarch64-linux-gnu-gcc-12'
>>> xen/include/xen/spinlock.h:31.21-31.23: expanded from macro `_LOCK_DEBUG'
>>> xen/include/xen/spinlock.h:143.57-143.67: expanded from macro 
>>> `SPIN_LOCK_UNLOCKED'
>>> xen/include/xen/spinlock.h:144.43-144.60: expanded from macro 
>>> `DEFINE_SPINLOCK'
>>
>> I'm afraid this is a bad example, as it goes hand-in-hand with using
>> another extension. I don't think using a non-empty initialization list
>> is going to work with
>>
>> union lock_debug { };
> 
> Yes, this is C99 undefined behavior 58:
> "A structure or union is defined as containing no named members (6.7.2.1)."
> 
> Here is another example:
> 
> lpae_t pte = {};
> 
> whereas we have
> 
> typedef union {
>  uint64_t bits;
>  lpae_pt_t pt;
>  lpae_p2m_t p2m;
>  lpae_walk_t walk;
> } lpae_t;
> 
> 
>>> xen/arch/arm/cpuerrata.c:678.5-678.6:
>>> empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7.8: "An empty initialization list." [STD.emptinit]). Tool used 
>>> is `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>
>>> xen/arch/arm/cpufeature.c:33.5-33.6:
>>> empty initializer list (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7.8: "An empty initialization list." [STD.emptinit]). Tool used 
>>> is `/usr/bin/aarch64-linux-gnu-gcc-12'
>>
>> Both of these are a common idiom we use: The "sentinel" of an array
>> of compound type initializer.
> 
> Wouldn't it be possible writing such sentinels in a standard-compliant
> way, like {0} or similar, instead of {}?

I would be possible, sure, but the question is whether we want that. Iirc
in review comments we've been asking to preferably use {}, for being
shorter / less clutter without resulting in any ambiguity.

> U6) Empty declarations.
>>>
>>> Examples:
>>>
>>> xen/arch/arm/arm64/lib/find_next_bit.c:57.29:
>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>
>>> xen/arch/arm/arm64/lib/find_next_bit.c:103.34:
>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>
>> Looks like these could be taken care of by finally purging our
>> EXPORT_SYMBOL() stub.
>>
>>> xen/arch/arm/include/asm/vreg.h:143.26:
>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>>
>>> xen/arch/arm/include/asm/vreg.h:144.26:
>>> empty declaration (ill-formed for the C99 standard, ISO/IEC 9899:1999 
>>> Section 6.7: "An empty declaration." [STD.emptdecl]). Tool used is 
>>> `/usr/bin/aarch64-linux-gnu-gcc-12'
>>
>> I'm having trouble spotting anything suspicious there.
> 
> The macro expands to definitions of inline functions
> and after the macro invocation there is a ";".
> 
> The preprocessed code is then:
> 
> static inline void foo() { ... }
> ;
> 
> where the final ";" is an empty declaration not allowed by
> the C99 language standard.

Oh, I see.

> Removing the ";" after the macro invocation is a possible solution,
> but other possibilities exist if this is strongly unwanted.

We have other macros to instantiate functions, and there no stray
semicolons are used. I think this wants doing the same way here, but it
being Arm code the ultimate say is with the Arm maintainers.

Jan



Re: Asking for help to debug xen efi on Kunpeng machine

2023-06-09 Thread Julien Grall

Hello,

On 09/06/2023 03:32, Jiatong Shen wrote:

Thank you for your answer. Can you teach me how to verify if acpi is
enabled?


You usually look at the .config. But I am not sure if this is provided 
by the Debian package. If not, then your best option would be to build 
your own Xen. To select ACPI, you want to use the menuconfig and select 
UNSUPPORTED and ACPI.


Cheers,

--
Julien Grall



Re: [PATCH] docs/misra: new rules addition

2023-06-09 Thread Jan Beulich
On 08.06.2023 13:02, Roberto Bagnara wrote:
> On 07/06/23 23:53, Stefano Stabellini wrote:
>> On Wed, 7 Jun 2023, Jan Beulich wrote:
 +   * - `Rule 5.6 
 `_
 + - Required
 + - A typedef name shall be a unique identifier
 + -
>>>
>>> Considering that the rule requires uniqueness across the entire code
>>> base (and hence precludes e.g. two functions having identically named
>>> local typedefs), I'm a little puzzled this was adopted. I for one
>>> question that a use like the one mentioned is really at risk of being
>>> confusing. Instead I think that the need to change at least one of
>>> the names is at risk of making the code less readable then, even if
>>> ever so slightly. (All of this said - I don't know if we have any
>>> violations of this rule.)
>>
>> I don't think we have many local typedefs and I think we have only few
>> violations if I remember right. I'll let Roberto confirm how many. This
>> rule was considered not a difficult rule (some difficult rules were
>> found, namely 2.1, 5.5 and 7.4.)
> 
> There currently are 30 violations for ARM64:
> 
> - some involve a typedef module_name (in the call it was said this should
>be renamed module_name_t, which will solve the issue);
> - most concern repeated typedefs (UINT64 and similar) which are repeated
>in xen/arch/arm/include/asm/arm64/efibind.h
>and xen/include/acpi/actypes.h
> - ret_t and phys_addr_t are also redefined in a couple of places.
> 
> There are 90 violations for X86_64, for the same reasons, plus
> 
> - another set of typedefs for basic types (such as u8);
> - repeated typedefs for things like guest_l1e_t in the same header file:
> 
> xen/arch/x86/include/asm/guest_pt.h:60.39-60.49:
> for program `xen/.xen-syms.0', the identifier for typedef `guest_l1e_t' is 
> reused
> xen/arch/x86/include/asm/guest_pt.h:128.22-128.32:
> for program `xen/.xen-syms.0', the identifier for typedef `guest_l1e_t' is 
> reused
> 
> The indicated lines read as follows:
> 
> typedef struct { guest_intpte_t l1; } guest_l1e_t;
> typedef l1_pgentry_t guest_l1e_t;

So this is an example where I don't think we can sensibly do away with the
re-use of the same typedef name: We use it so we can build the same source
files multiple ways, dealing with different paging modes guests may be in.

Jan



Re: [PATCH] tools/ocaml/xc: Fix xc_physinfo() bindings

2023-06-09 Thread Christian Lindig



> On 8 Jun 2023, at 20:33, Andrew Cooper  wrote:
> 
> +type arm_physinfo_caps =
> +  {
> +sve_vl: int;
> +  }
> +


Does the OCaml side need to know about the structure of this value or is it 
enough to pass it around as an abstract value because all logic is on the C 
side? I assume the OCaml side needs at least a way to persist the value and 
hence needs to know some representation. 

>   Store_field(arch_obj, 0,
> + Val_int(MASK_EXTR(info->arch_capabilities,
> +   XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK) * 128));
> +

What is the “* 128” achieving as part of this encoding?

— C


[qemu-mainline test] 181327: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181327 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181327/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 180691
 build-arm64   6 xen-buildfail REGR. vs. 180691
 build-amd64   6 xen-buildfail REGR. vs. 180691
 build-i3866 xen-buildfail REGR. vs. 180691
 build-amd64-xsm   6 xen-buildfail REGR. vs. 180691
 build-i386-xsm6 xen-buildfail REGR. vs. 180691
 build-armhf   6 xen-buildfail REGR. vs. 180691

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-vhd1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-raw   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-coresched-amd64-xl  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-de

[xen-unstable-smoke test] 181330: regressions - FAIL

2023-06-09 Thread osstest service owner
flight 181330 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/181330/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-xsm   6 xen-buildfail REGR. vs. 181233
 build-armhf   6 xen-buildfail REGR. vs. 181233

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 xen  6915a120641b5d232762af7882266048cf039ca0
baseline version:
 xen  64a647f8d817c6989edc000613b5afae19f03f99

Last test of basis   181233  2023-06-07 02:04:37 Z2 days
Failing since181246  2023-06-07 11:02:03 Z1 days   21 attempts
Testing same since   181303  2023-06-08 11:00:25 Z0 days   10 attempts


People who touched revisions under test:
  Andrew Cooper 
  Christian Lindig 
  George Dunlap 
  Henry Wang  # CHANGELOG
  Juergen Gross 
  Julien Grall 
  Luca Fancellu 
  Michal Orzel 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 

jobs:
 build-arm64-xsm  fail
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 test-arm64-arm64-xl-xsm  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 504 lines long.)