[Xen-devel] [PATCH for-4.13 v3 0/3] xen: Build fixes related to Python3

[Xen-devel] [PATCH for-4.13 v3 0/3] xen: Build fixes related to Python3

[Andrew Cooper]

Patch 3 is a fix for a problem reported on IRC.  It is a very-nice-to-have
considering our attempt to make Xen 4.13 Py3-clean.

While testing patch 3, it became apparent that XSM/Flask isn't Py3-clean, and
this is a blocker.  It is addressed in patch 1.

Patch 2 addresses a bug spotted by Gitlab while testing v1 of this series.  It
isn't strictly a Py3 bug, but is a build system robustness fix.

v3 of this series fixes all Travis and Gitlab identified issues:
  https://travis-ci.org/andyhhp/xen/builds/622092503
  https://gitlab.com/xen-project/people/andyhhp/xen/pipelines/101417861

Andrew Cooper (3):
  xen/flask: Drop the gen-policy.py script
  xen/banner: Drop the fig-to-oct.py script
  xen/build: Automatically locate a suitable python interpreter

 xen/Makefile                 |  7 ++++++-
 xen/tools/fig-to-oct.py      | 18 ------------------
 xen/tools/process-banner.sed | 14 ++++++++++++++
 xen/xsm/flask/Makefile       |  6 ++----
 xen/xsm/flask/flask-policy.S | 20 ++++++++++++++++++++
 xen/xsm/flask/gen-policy.py  | 23 -----------------------
 6 files changed, 42 insertions(+), 46 deletions(-)
 delete mode 100644 xen/tools/fig-to-oct.py
 create mode 100755 xen/tools/process-banner.sed
 create mode 100644 xen/xsm/flask/flask-policy.S
 delete mode 100644 xen/xsm/flask/gen-policy.py

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v3 1/3] xen/flask: Drop the gen-policy.py script

[Andrew Cooper]

The script is Python 2 specific, and fails with string/binary issues with
Python 3:

  Traceback (most recent call last):
    File "gen-policy.py", line 14, in <module>
      for char in sys.stdin.read():
    File "/usr/lib/python3.5/codecs.py", line 321, in decode
      (result, consumed) = self._buffer_decode(data, self.errors, final)
  UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte

Fixing the script to be compatible isn't hard, but using python here is
wasteful.  Drop the script entirely, and write a short flask-policy.S instead.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
CC: Juergen Gross <jgross@suse.com>

v2:
 * Fix tabs vs spaces issues
v3:
 * Use % rather than @ for progbits/object, for Arm32 build.
 * Spotted by https://travis-ci.org/andyhhp/xen/builds/622085138

For 4.13.  This is a blocker to our intent to by Py3-clean in this release.

Discovered entirely accidently when testing the final patch.
---
 xen/xsm/flask/Makefile       |  6 ++----
 xen/xsm/flask/flask-policy.S | 20 ++++++++++++++++++++
 xen/xsm/flask/gen-policy.py  | 23 -----------------------
 3 files changed, 22 insertions(+), 27 deletions(-)
 create mode 100644 xen/xsm/flask/flask-policy.S
 delete mode 100644 xen/xsm/flask/gen-policy.py

diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index f5ffab1226..7c3f381287 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,7 +27,8 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
 $(AV_H_FILES): $(AV_H_DEPEND)
 	$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
 
-obj-$(CONFIG_XSM_FLASK_POLICY) += policy.o
+obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
+flask-policy.o: policy.bin
 
 FLASK_BUILD_DIR := $(CURDIR)
 POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
@@ -36,9 +37,6 @@ policy.bin: FORCE
 	$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common -C $(XEN_ROOT)/tools/flask/policy FLASK_BUILD_DIR=$(FLASK_BUILD_DIR)
 	cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
 
-policy.c: policy.bin gen-policy.py
-	$(PYTHON) gen-policy.py < $< > $@
-
 .PHONY: clean
 clean::
 	rm -f $(ALL_H_FILES) *.o $(DEPS_RM) policy.* $(POLICY_SRC)
diff --git a/xen/xsm/flask/flask-policy.S b/xen/xsm/flask/flask-policy.S
new file mode 100644
index 0000000000..81bfc09ec2
--- /dev/null
+++ b/xen/xsm/flask/flask-policy.S
@@ -0,0 +1,20 @@
+        .section .init.rodata, "a", %progbits
+
+/* const unsigned char xsm_flask_init_policy[] __initconst */
+        .align 4
+        .global xsm_flask_init_policy
+xsm_flask_init_policy:
+        .incbin "policy.bin"
+.Lend:
+
+        .type xsm_flask_init_policy, %object
+        .size xsm_flask_init_policy, . - xsm_flask_init_policy
+
+/* const unsigned int __initconst xsm_flask_init_policy_size */
+        .align 4
+        .global xsm_flask_init_policy_size
+xsm_flask_init_policy_size:
+        .long .Lend - xsm_flask_init_policy
+
+        .type xsm_flask_init_policy_size, %object
+        .size xsm_flask_init_policy_size, . - xsm_flask_init_policy_size
diff --git a/xen/xsm/flask/gen-policy.py b/xen/xsm/flask/gen-policy.py
deleted file mode 100644
index c7501e4614..0000000000
--- a/xen/xsm/flask/gen-policy.py
+++ /dev/null
@@ -1,23 +0,0 @@
-#!/usr/bin/env python
-import sys
-
-policy_size = 0
-
-sys.stdout.write("""
-/* This file is autogenerated by gen_policy.py */
-#include <xen/init.h>
-#include <xsm/xsm.h>
-
-const unsigned char xsm_flask_init_policy[] __initconst = {
-""")
-
-for char in sys.stdin.read():
-    sys.stdout.write(" 0x%02x," % ord(char))
-    policy_size = policy_size + 1
-    if policy_size % 13 == 0:
-        sys.stdout.write("\n")
-
-sys.stdout.write("""
-};
-const unsigned int __initconst xsm_flask_init_policy_size = %d;
-""" % policy_size)
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v3 2/3] xen/banner: Drop the fig-to-oct.py script

[Andrew Cooper]

The script is 664 rather than 775, so the banner conversion doesn't actually
work if $(PYTHON) is empty:

  /bin/sh: tools/fig-to-oct.py: Permission denied
  make[3]: *** [include/xen/compile.h] Error 126
  make[3]: Leaving directory `/builds/xen-project/people/andyhhp/xen/xen'

Fixing this is easy, but using python here is wasteful.  compile.h doesn't
need XEN_BANNER rendering in octal, and text is much more simple to handle.
Replace fig-to-oct.py with a smaller sed script.  This could be a shell
one-liner, but it is much more simple to comment sensibly, and doesn't need to
include the added cognative load of makefile and shell escaping.

While changing this logic, take the opportunity to optimise the banner
space (and time on the serial port) by dropping trailing whitespace, which is
84 characters for current staging.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien@xen.org>
CC: Juergen Gross <jgross@suse.com>

v2:
 * New
v3:
 * Fix error: backslash-newline at end of file [-Werror]
 * Implement as a sed script.

This (v3) is how happy in the CI:
  https://gitlab.com/xen-project/people/andyhhp/xen/pipelines/101409945

Spotted by Gitlab CI, caused by `which` not being present in some of the
CentOS containers.  While this is more of a container bug than anything else,
it does highlight that the build ought to cope.
---
 xen/Makefile                 |  2 +-
 xen/tools/fig-to-oct.py      | 18 ------------------
 xen/tools/process-banner.sed | 14 ++++++++++++++
 3 files changed, 15 insertions(+), 19 deletions(-)
 delete mode 100644 xen/tools/fig-to-oct.py
 create mode 100755 xen/tools/process-banner.sed

diff --git a/xen/Makefile b/xen/Makefile
index 99701e3165..949ca6eb03 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -176,7 +176,7 @@ include/xen/compile.h: include/xen/compile.h.in .banner
 	    -e 's!@@changeset@@!$(shell tools/scmversion $(XEN_ROOT) || echo "unavailable")!g' \
 	    < include/xen/compile.h.in > $@.new
 	@cat .banner
-	@$(PYTHON) tools/fig-to-oct.py < .banner >> $@.new
+	@sed -rf tools/process-banner.sed < .banner >> $@.new
 	@mv -f $@.new $@
 
 include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s
diff --git a/xen/tools/fig-to-oct.py b/xen/tools/fig-to-oct.py
deleted file mode 100644
index db4fd32159..0000000000
--- a/xen/tools/fig-to-oct.py
+++ /dev/null
@@ -1,18 +0,0 @@
-#!/usr/bin/env python
-import sys
-
-chars_per_line = 18
-chars_so_far = 0
-
-sys.stdout.write('"')
-
-for char in sys.stdin.read():
-
-    sys.stdout.write("\\%03o" % ord(char))
-    chars_so_far = chars_so_far + 1
-
-    if chars_so_far == chars_per_line:
-        chars_so_far = 0
-        sys.stdout.write('" \\\n"')
-
-sys.stdout.write('"\n')
diff --git a/xen/tools/process-banner.sed b/xen/tools/process-banner.sed
new file mode 100755
index 0000000000..56c76558bc
--- /dev/null
+++ b/xen/tools/process-banner.sed
@@ -0,0 +1,14 @@
+#!/bin/sed -rf
+# Process a text input, to turn it into a C string for the XEN_BANNER macro.
+
+# Strip trailing whitespace.
+s_ *$__
+
+# Escape backslashes.
+s_\\_\\\\_g
+
+# Enclose the line in "...\n".
+s_(.*)_"\1\\n"_
+
+# Trailing \ on all but the final line.
+$!s_$_ \\_
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v3 3/3] xen/build: Automatically locate a suitable python interpreter

[Andrew Cooper]

Needing to pass PYTHON=python3 into hypervisor builds is irritating and
unnecessary.  Locate a suitable interpreter automatically, defaulting to Py3
if it is available.

Reported-by: Steven Haigh <netwiz@crc.id.au>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien@xen.org>
CC: Steven Haigh <netwiz@crc.id.au>
CC: Juergen Gross <jgross@suse.com>

v2:
 * Cope with `which` not being present in the system.
 * Only evaulate the shell command one, rather than once per $(PTHON) usage

For 4.13.  This is a very-nice-to-have WRT our Py3-clean intention.
---
 xen/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/Makefile b/xen/Makefile
index 949ca6eb03..f36a5bc6c0 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -13,6 +13,11 @@ export XEN_BUILD_TIME	?= $(shell LC_ALL=C date +%T)
 export XEN_BUILD_HOST	?= $(shell hostname)
 export XEN_CONFIG_EXPERT ?= n
 
+# Best effort attempt to find a python interpreter, defaulting to Python 3 if
+# available.  Fall back to just `python` if `which` is nowhere to be found.
+PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null) python)
+export PYTHON		?= $(PYTHON_INTERPRETER)
+
 export BASEDIR := $(CURDIR)
 export XEN_ROOT := $(BASEDIR)/..
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 1/3] xen/flask: Drop the gen-policy.py script

[George Dunlap]

On Sat, Dec 7, 2019 at 9:17 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> The script is Python 2 specific, and fails with string/binary issues with
> Python 3:
>
>   Traceback (most recent call last):
>     File "gen-policy.py", line 14, in <module>
>       for char in sys.stdin.read():
>     File "/usr/lib/python3.5/codecs.py", line 321, in decode
>       (result, consumed) = self._buffer_decode(data, self.errors, final)
>   UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte
>
> Fixing the script to be compatible isn't hard, but using python here is
> wasteful.  Drop the script entirely, and write a short flask-policy.S instead.

It might be helpful for casual reviewers to have a slightly better
explanation of what the change is; namely:

- The end goal is to have a .o file exporting one variable containing
the contents of policy.bin, and another containing its size.

- gen-policy.py generates a C file which contains the bytes of
policy.bin (and its size).  This means running python, and then a c
compiler.

- The replacement use a .S file to "include" the binary directly.
This involves only running an assembler, but has the same end effect.

This looks like a good change; but I don't know assembler well enough
to give it an R-b.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 3/3] xen/build: Automatically locate a suitable python interpreter

[George Dunlap]

On 12/7/19 9:16 PM, Andrew Cooper wrote:
> Needing to pass PYTHON=python3 into hypervisor builds is irritating and
> unnecessary.  Locate a suitable interpreter automatically, defaulting to Py3
> if it is available.
> 
> Reported-by: Steven Haigh <netwiz@crc.id.au>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Wei Liu <wl@xen.org>
> CC: Julien Grall <julien@xen.org>
> CC: Steven Haigh <netwiz@crc.id.au>
> CC: Juergen Gross <jgross@suse.com>
> 
> v2:
>  * Cope with `which` not being present in the system.
>  * Only evaulate the shell command one, rather than once per $(PTHON) usage
> 
> For 4.13.  This is a very-nice-to-have WRT our Py3-clean intention.
> ---
>  xen/Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/xen/Makefile b/xen/Makefile
> index 949ca6eb03..f36a5bc6c0 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -13,6 +13,11 @@ export XEN_BUILD_TIME	?= $(shell LC_ALL=C date +%T)
>  export XEN_BUILD_HOST	?= $(shell hostname)
>  export XEN_CONFIG_EXPERT ?= n
>  
> +# Best effort attempt to find a python interpreter, defaulting to Python 3 if
> +# available.  Fall back to just `python` if `which` is nowhere to be found.
> +PYTHON_INTERPRETER	:= $(word 1,$(shell which python3 python python2 2>/dev/null) python)
> +export PYTHON		?= $(PYTHON_INTERPRETER)

Acked-by: George Dunlap <george.dunlap@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 2/3] xen/banner: Drop the fig-to-oct.py script

[George Dunlap]

On 12/7/19 9:16 PM, Andrew Cooper wrote:
> The script is 664 rather than 775, so the banner conversion doesn't actually
> work if $(PYTHON) is empty:
> 
>   /bin/sh: tools/fig-to-oct.py: Permission denied
>   make[3]: *** [include/xen/compile.h] Error 126
>   make[3]: Leaving directory `/builds/xen-project/people/andyhhp/xen/xen'
> 
> Fixing this is easy, but using python here is wasteful.  compile.h doesn't
> need XEN_BANNER rendering in octal, and text is much more simple to handle.
> Replace fig-to-oct.py with a smaller sed script.  This could be a shell
> one-liner, but it is much more simple to comment sensibly, and doesn't need to
> include the added cognative load of makefile and shell escaping.
> 
> While changing this logic, take the opportunity to optimise the banner
> space (and time on the serial port) by dropping trailing whitespace, which is
> 84 characters for current staging.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

It's more work than I would have put into it. :-)  But since it's
already done:

Acked-by: George Dunlap <george.dunlap@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 1/3] xen/flask: Drop the gen-policy.py script

[Jan Beulich]

On 07.12.2019 22:16, Andrew Cooper wrote:
> --- /dev/null
> +++ b/xen/xsm/flask/flask-policy.S
> @@ -0,0 +1,20 @@
> +        .section .init.rodata, "a", %progbits
> +
> +/* const unsigned char xsm_flask_init_policy[] __initconst */
> +        .align 4

I'm afraid .align is not universal enough to be used here - iirc
some architectures have it alias .p2align rather than (how e.g.
x86 behaves) .balign. Looks like .p2align is available in all
binutils versions we claim to be able to be built with. (I'm
not sure the one here is needed anyway, but the one below we
surely want.)

> +        .global xsm_flask_init_policy
> +xsm_flask_init_policy:
> +        .incbin "policy.bin"
> +.Lend:
> +
> +        .type xsm_flask_init_policy, %object
> +        .size xsm_flask_init_policy, . - xsm_flask_init_policy
> +
> +/* const unsigned int __initconst xsm_flask_init_policy_size */
> +        .align 4
> +        .global xsm_flask_init_policy_size
> +xsm_flask_init_policy_size:
> +        .long .Lend - xsm_flask_init_policy

Similarly .long isn't really universal (various arches override
it in gas). Aiui .dc.l is intended to be portable (despite still
carrying the 'l' in its name, and despite even this one getting
overridden by two arches). But perhaps best to ask on the
binutils list.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 1/3] xen/flask: Drop the gen-policy.py script

[Andrew Cooper]

On 09/12/2019 13:38, Jan Beulich wrote:
> On 07.12.2019 22:16, Andrew Cooper wrote:
>> --- /dev/null
>> +++ b/xen/xsm/flask/flask-policy.S
>> @@ -0,0 +1,20 @@
>> +        .section .init.rodata, "a", %progbits
>> +
>> +/* const unsigned char xsm_flask_init_policy[] __initconst */
>> +        .align 4
> I'm afraid .align is not universal enough to be used here - iirc
> some architectures have it alias .p2align rather than (how e.g.
> x86 behaves) .balign. Looks like .p2align is available in all
> binutils versions we claim to be able to be built with. (I'm
> not sure the one here is needed anyway, but the one below we
> surely want.)

I can switch to p2align, but...

>
>> +        .global xsm_flask_init_policy
>> +xsm_flask_init_policy:
>> +        .incbin "policy.bin"
>> +.Lend:
>> +
>> +        .type xsm_flask_init_policy, %object
>> +        .size xsm_flask_init_policy, . - xsm_flask_init_policy
>> +
>> +/* const unsigned int __initconst xsm_flask_init_policy_size */
>> +        .align 4
>> +        .global xsm_flask_init_policy_size
>> +xsm_flask_init_policy_size:
>> +        .long .Lend - xsm_flask_init_policy
> Similarly .long isn't really universal (various arches override
> it in gas). Aiui .dc.l is intended to be portable (despite still
> carrying the 'l' in its name, and despite even this one getting
> overridden by two arches). But perhaps best to ask on the
> binutils list.

... this is not a clear or obvious way to go, not least because it makes
a different expectation that int will never change from being 32 bits. 
At least .long will work even if it becomes longer in a future toolchain.

What is used here doesn't need to be universal - it only needs to work
for the architectures we support.

If hand writing an asm file isn't considered good enough, then the other
options are a C file with inline asm incbin, or `objdump
--rename-section`.  The latter one would require a few changes elsewhere
in the code, but only for linkage purposes.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 1/3] xen/flask: Drop the gen-policy.py script

[Daniel De Graaf]

On 12/7/19 4:16 PM, Andrew Cooper wrote:
> The script is Python 2 specific, and fails with string/binary issues with
> Python 3:
> 
>    Traceback (most recent call last):
>      File "gen-policy.py", line 14, in <module>
>        for char in sys.stdin.read():
>      File "/usr/lib/python3.5/codecs.py", line 321, in decode
>        (result, consumed) = self._buffer_decode(data, self.errors, final)
>    UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte
> 
> Fixing the script to be compatible isn't hard, but using python here is
> wasteful.  Drop the script entirely, and write a short flask-policy.S instead.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

With either .align or .p2align as appropriate for more assemblers.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 1/3] xen/flask: Drop the gen-policy.py script

[Jan Beulich]

On 09.12.2019 18:01, Andrew Cooper wrote:
> On 09/12/2019 13:38, Jan Beulich wrote:
>> On 07.12.2019 22:16, Andrew Cooper wrote:
>>> --- /dev/null
>>> +++ b/xen/xsm/flask/flask-policy.S
>>> @@ -0,0 +1,20 @@
>>> +        .section .init.rodata, "a", %progbits
>>> +
>>> +/* const unsigned char xsm_flask_init_policy[] __initconst */
>>> +        .align 4
>> I'm afraid .align is not universal enough to be used here - iirc
>> some architectures have it alias .p2align rather than (how e.g.
>> x86 behaves) .balign. Looks like .p2align is available in all
>> binutils versions we claim to be able to be built with. (I'm
>> not sure the one here is needed anyway, but the one below we
>> surely want.)
> 
> I can switch to p2align, but...
> 
>>
>>> +        .global xsm_flask_init_policy
>>> +xsm_flask_init_policy:
>>> +        .incbin "policy.bin"
>>> +.Lend:
>>> +
>>> +        .type xsm_flask_init_policy, %object
>>> +        .size xsm_flask_init_policy, . - xsm_flask_init_policy
>>> +
>>> +/* const unsigned int __initconst xsm_flask_init_policy_size */
>>> +        .align 4
>>> +        .global xsm_flask_init_policy_size
>>> +xsm_flask_init_policy_size:
>>> +        .long .Lend - xsm_flask_init_policy
>> Similarly .long isn't really universal (various arches override
>> it in gas). Aiui .dc.l is intended to be portable (despite still
>> carrying the 'l' in its name, and despite even this one getting
>> overridden by two arches). But perhaps best to ask on the
>> binutils list.
> 
> ... this is not a clear or obvious way to go, not least because it makes
> a different expectation that int will never change from being 32 bits. 
> At least .long will work even if it becomes longer in a future toolchain.

There are a few targets where .long (and .int) appear to produce 2-byte
values (at the first glance, i.e. without checking very closely).

> What is used here doesn't need to be universal - it only needs to work
> for the architectures we support.

But it also would better not break silently for some future port. How
about an equivalent to Linux'es _ASM_PTR() (say ASM_WORD()), which each
architecture has to supply explicitly?

> If hand writing an asm file isn't considered good enough, then the other
> options are a C file with inline asm incbin, or `objdump
> --rename-section`.  The latter one would require a few changes elsewhere
> in the code, but only for linkage purposes.

I'm entirely fine with an assembler source here, it just needs a little
more polishing imo.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH for-4.13 v4 1/3] xen/flask: Drop the gen-policy.py script

[Andrew Cooper]

The script is Python 2 specific, and fails with string/binary issues with
Python 3:

  Traceback (most recent call last):
    File "gen-policy.py", line 14, in <module>
      for char in sys.stdin.read():
    File "/usr/lib/python3.5/codecs.py", line 321, in decode
      (result, consumed) = self._buffer_decode(data, self.errors, final)
  UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte

Fixing the script to be compatible isn't hard, but using python here is
wasteful.  Drop the script entirely, and write an equivelent flask-policy.S
instead.  This removes the need for a $(PYTHON) and $(CC) pass.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
CC: Juergen Gross <jgross@suse.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>

v2:
 * Fix tabs vs spaces issues
v3:
 * Use % rather than @ for progbits/object, for Arm32 build.
 * Spotted by https://travis-ci.org/andyhhp/xen/builds/622085138
v4:
 * Introduce and use ASM_INT() to declare something compatible with `int`
 * Drop .align

For 4.13.  This is a blocker to our intent to by Py3-clean in this release.

Discovered entirely accidently when testing the final patch.
---
 xen/include/asm-arm/asm_defns.h |  6 ++++++
 xen/include/asm-x86/asm_defns.h |  6 ++++++
 xen/xsm/flask/Makefile          |  6 ++----
 xen/xsm/flask/flask-policy.S    | 16 ++++++++++++++++
 xen/xsm/flask/gen-policy.py     | 23 -----------------------
 5 files changed, 30 insertions(+), 27 deletions(-)
 create mode 100644 xen/xsm/flask/flask-policy.S
 delete mode 100644 xen/xsm/flask/gen-policy.py

diff --git a/xen/include/asm-arm/asm_defns.h b/xen/include/asm-arm/asm_defns.h
index 3f21def0ab..b4fbcdae1d 100644
--- a/xen/include/asm-arm/asm_defns.h
+++ b/xen/include/asm-arm/asm_defns.h
@@ -21,6 +21,12 @@
 label:  .asciz msg;                             \
 .popsection
 
+#define ASM_INT(label, val)                 \
+    .p2align 2;                             \
+label: .long (val);                         \
+    .size label, . - label;                 \
+    .type label, %object
+
 #endif /* __ARM_ASM_DEFNS_H__ */
 /*
  * Local variables:
diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index c4f49a35d3..370f239c50 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -386,4 +386,10 @@ static always_inline void stac(void)
 4:  .p2align 2                            ; \
     .popsection
 
+#define ASM_INT(label, val)                 \
+    .p2align 2;                             \
+label: .long (val);                         \
+    .size label, . - label;                 \
+    .type label, @object
+
 #endif /* __X86_ASM_DEFNS_H__ */
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index f5ffab1226..7c3f381287 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,7 +27,8 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
 $(AV_H_FILES): $(AV_H_DEPEND)
 	$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
 
-obj-$(CONFIG_XSM_FLASK_POLICY) += policy.o
+obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
+flask-policy.o: policy.bin
 
 FLASK_BUILD_DIR := $(CURDIR)
 POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
@@ -36,9 +37,6 @@ policy.bin: FORCE
 	$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common -C $(XEN_ROOT)/tools/flask/policy FLASK_BUILD_DIR=$(FLASK_BUILD_DIR)
 	cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
 
-policy.c: policy.bin gen-policy.py
-	$(PYTHON) gen-policy.py < $< > $@
-
 .PHONY: clean
 clean::
 	rm -f $(ALL_H_FILES) *.o $(DEPS_RM) policy.* $(POLICY_SRC)
diff --git a/xen/xsm/flask/flask-policy.S b/xen/xsm/flask/flask-policy.S
new file mode 100644
index 0000000000..e9308aa175
--- /dev/null
+++ b/xen/xsm/flask/flask-policy.S
@@ -0,0 +1,16 @@
+#include <asm/asm_defns.h>
+
+        .section .init.rodata, "a", %progbits
+
+/* const unsigned char xsm_flask_init_policy[] __initconst */
+        .global xsm_flask_init_policy
+xsm_flask_init_policy:
+        .incbin "policy.bin"
+.Lend:
+
+        .type xsm_flask_init_policy, %object
+        .size xsm_flask_init_policy, . - xsm_flask_init_policy
+
+/* const unsigned int __initconst xsm_flask_init_policy_size */
+ 	.global xsm_flask_init_policy_size
+        ASM_INT(xsm_flask_init_policy_size, .Lend - xsm_flask_init_policy)
diff --git a/xen/xsm/flask/gen-policy.py b/xen/xsm/flask/gen-policy.py
deleted file mode 100644
index c7501e4614..0000000000
--- a/xen/xsm/flask/gen-policy.py
+++ /dev/null
@@ -1,23 +0,0 @@
-#!/usr/bin/env python
-import sys
-
-policy_size = 0
-
-sys.stdout.write("""
-/* This file is autogenerated by gen_policy.py */
-#include <xen/init.h>
-#include <xsm/xsm.h>
-
-const unsigned char xsm_flask_init_policy[] __initconst = {
-""")
-
-for char in sys.stdin.read():
-    sys.stdout.write(" 0x%02x," % ord(char))
-    policy_size = policy_size + 1
-    if policy_size % 13 == 0:
-        sys.stdout.write("\n")
-
-sys.stdout.write("""
-};
-const unsigned int __initconst xsm_flask_init_policy_size = %d;
-""" % policy_size)
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13 v4 1/3] xen/flask: Drop the gen-policy.py script

[Jürgen Groß]

On 10.12.19 13:17, Andrew Cooper wrote:
> The script is Python 2 specific, and fails with string/binary issues with
> Python 3:
> 
>    Traceback (most recent call last):
>      File "gen-policy.py", line 14, in <module>
>        for char in sys.stdin.read():
>      File "/usr/lib/python3.5/codecs.py", line 321, in decode
>        (result, consumed) = self._buffer_decode(data, self.errors, final)
>    UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte
> 
> Fixing the script to be compatible isn't hard, but using python here is
> wasteful.  Drop the script entirely, and write an equivelent flask-policy.S
> instead.  This removes the need for a $(PYTHON) and $(CC) pass.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 3/3] xen/build: Automatically locate a suitable python interpreter

[Jürgen Groß]

On 07.12.19 22:16, Andrew Cooper wrote:
> Needing to pass PYTHON=python3 into hypervisor builds is irritating and
> unnecessary.  Locate a suitable interpreter automatically, defaulting to Py3
> if it is available.
> 
> Reported-by: Steven Haigh <netwiz@crc.id.au>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13 v4 1/3] xen/flask: Drop the gen-policy.py script

[Jan Beulich]

On 10.12.2019 13:17, Andrew Cooper wrote:
> The script is Python 2 specific, and fails with string/binary issues with
> Python 3:
> 
>   Traceback (most recent call last):
>     File "gen-policy.py", line 14, in <module>
>       for char in sys.stdin.read():
>     File "/usr/lib/python3.5/codecs.py", line 321, in decode
>       (result, consumed) = self._buffer_decode(data, self.errors, final)
>   UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte
> 
> Fixing the script to be compatible isn't hard, but using python here is
> wasteful.  Drop the script entirely, and write an equivelent flask-policy.S
> instead.  This removes the need for a $(PYTHON) and $(CC) pass.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13 v4 1/3] xen/flask: Drop the gen-policy.py script

[Julien Grall]

Hi,

On 10/12/2019 12:17, Andrew Cooper wrote:
> The script is Python 2 specific, and fails with string/binary issues with
> Python 3:
> 
>    Traceback (most recent call last):
>      File "gen-policy.py", line 14, in <module>
>        for char in sys.stdin.read():
>      File "/usr/lib/python3.5/codecs.py", line 321, in decode
>        (result, consumed) = self._buffer_decode(data, self.errors, final)
>    UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte
> 
> Fixing the script to be compatible isn't hard, but using python here is
> wasteful.  Drop the script entirely, and write an equivelent flask-policy.S
> instead.  This removes the need for a $(PYTHON) and $(CC) pass.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
> CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> CC: Juergen Gross <jgross@suse.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wl@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> 
> v2:
>   * Fix tabs vs spaces issues
> v3:
>   * Use % rather than @ for progbits/object, for Arm32 build.

How to make developper life more exciting...

Acked-by: Julien Grall <jgrall@xen.org>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel