date:20220726

[xen-4.16-testing test] 171867: tolerable FAIL - PUSHED

2022-07-26 Thread osstest service owner

flight 171867 xen-4.16-testing real [real]
flight 171876 xen-4.16-testing real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/171867/
http://logs.test-lab.xenproject.org/osstest/logs/171876/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install 
fail pass in 171876-retest

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 171637
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 171637
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 171637
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 171637
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 171637
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 171637
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 171637
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 171637
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 171637
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 171637
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 171637
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 171637
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail 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-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  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-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-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 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-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 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-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 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-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-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
 test-armhf-armhf-xl-vhd  14

Re: [PATCH 3/4] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

2022-07-26 Thread Xenia Ragiadakou




On 7/27/22 03:10, Stefano Stabellini wrote:

On Tue, 26 Jul 2022, Jan Beulich wrote:

On 26.07.2022 02:33, Stefano Stabellini wrote:

On Mon, 25 Jul 2022, Xenia Ragiadakou wrote:

On 7/25/22 09:32, Jan Beulich wrote:

On 24.07.2022 19:20, Xenia Ragiadakou wrote:

On 7/7/22 10:55, Jan Beulich wrote:

On 07.07.2022 09:27, Xenia Ragiadakou wrote:

On 7/6/22 11:51, Jan Beulich wrote:

On 06.07.2022 10:43, Xenia Ragiadakou wrote:

On 7/6/22 10:10, Jan Beulich wrote:

On 05.07.2022 23:02, Xenia Ragiadakou wrote:

The function idle_loop() is referenced only in domain.c.
Change its linkage from external to internal by adding the
storage-class
specifier static to its definitions.

Since idle_loop() is referenced only in inline assembly, add
the 'used'
attribute to suppress unused-function compiler warning.


While I see that Julien has already acked the patch, I'd like to
point
out that using __used here is somewhat bogus. Imo the better
approach
is to properly make visible to the compiler that the symbol is
used by
the asm(), by adding a fake(?) input.


I 'm afraid I do not understand what do you mean by "adding a
fake(?)
input". Can you please elaborate a little on your suggestion?


Once the asm() in question takes the function as an input, the
compiler
will know that the function has a user (unless, of course, it finds
a
way to elide the asm() itself). The "fake(?)" was because I'm not
deeply
enough into Arm inline assembly to know whether the input could then
also be used as an instruction operand (which imo would be
preferable) -
if it can't (e.g. because there's no suitable constraint or operand
modifier), it still can be an input just to inform the compiler.


According to the following statement, your approach is the recommended
one:
"To prevent the compiler from removing global data or functions which
are referenced from inline assembly statements, you can:
-use __attribute__((used)) with the global data or functions.
-pass the reference to global data or functions as operands to inline
assembly statements.
Arm recommends passing the reference to global data or functions as
operands to inline assembly statements so that if the final image does
not require the inline assembly statements and the referenced global
data or function, then they can be removed."

IIUC, you are suggesting to change
asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" )
into
asm volatile ("mov sp,%0; b %1" : : "r" (stack), "S" (fn) : "memory"
);


Yes, except that I can't judge about the "S" constraint.



This constraint does not work for arm32. Any other thoughts?

Another way, as Jan suggested, is to pass the function as a 'fake'
(unused) input.
I 'm suspecting something like the following could work
asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack), "X" (fn) :
"memory")
What do you think?


Well, yes, X should always be a fallback option. But I said already when
you suggested S that I can't judge about its use, so I guess I'm the
wrong one to ask. Even more so that you only say "does not work", without
any details ...



The question is addressed to anyone familiar with arm inline assembly.
I added the arm maintainers as primary recipients to this email to make this
perfectly clear.

When cross-compiling Xen on x86 for arm32 with
asm volatile ("mov sp,%0; b %1" : : "r" (stack), "S" (fn) : "memory" );
compilation fails with the error: impossible constraint in ‘asm'


Unfortunately looking at the GCC manual pages [1], it doesn't seem to be
possible. The problem is that the definition of "S" changes between
aarch64 and arm (the 32-bit version).

For aarch64:

S   An absolute symbolic address or a label reference

This is what we want. For arm instead:

S   A symbol in the text segment of the current file

This is not useful for what we need to do here. As far as I can tell,
there is no other way in GCC assembly inline for arm to do this.

So we have 2 choices: we use the __used keyword as Xenia did in this
patch. Or we move the implementation of switch_stack_and_jump in
assembly. I attempted a prototype of the latter to see how it would come
out, see below.

I don't like it very much. I prefer the version with __used that Xenia
had in this patch. But I am OK either way.


You forgot the imo better intermediate option of using the "X" constraint.


I couldn't get "X" to compile in any way (not even for arm64). Do you
have a concrete example that you think should work using "X" as
constraint?


I proposed the X constraint for the case that the function is used as a 
fake (unused) input operand to the inline asm.
For instance, in the example below, the function is listed in the input 
operands but there is not corresponding reference to it.


asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack), "X" (fn) : 
"memory" );


--
Xenia

[xen-4.15-testing test] 171866: tolerable FAIL - PUSHED

2022-07-26 Thread osstest service owner

flight 171866 xen-4.15-testing real [real]
flight 171875 xen-4.15-testing real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/171866/
http://logs.test-lab.xenproject.org/osstest/logs/171875/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-xl-xsm   8 xen-bootfail pass in 171875-retest

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds18 guest-start/debian.repeat fail REGR. vs. 171609

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm 15 migrate-support-check fail in 171875 never pass
 test-arm64-arm64-xl-xsm 16 saverestore-support-check fail in 171875 never pass
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 171609
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 171609
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 171609
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 171609
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 171609
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 171609
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 171609
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 171609
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 171609
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 171609
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 171609
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 171609
 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  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  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-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail 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-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-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-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-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-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-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 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-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

[xen-4.14-testing test] 171865: tolerable FAIL - PUSHED

2022-07-26 Thread osstest service owner

flight 171865 xen-4.14-testing real [real]
flight 171874 xen-4.14-testing real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/171865/
http://logs.test-lab.xenproject.org/osstest/logs/171874/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-qcow2  8 xen-boot  fail pass in 171874-retest

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail in 171874 
like 171604
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-check fail in 171874 never 
pass
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 171604
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 171604
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 171604
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 171604
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 171604
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 171604
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 171604
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 171604
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 171604
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 171604
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 171604
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 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  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  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-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-amd64-i386-libvirt-raw  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-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-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  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  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail 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-armhf-armhf-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-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 15

Re: [IMAGEBUILDER PATCH] uboot-script-gen: allow fit generation with no dom0 kernel

2022-07-26 Thread Stefano Stabellini

On Mon, 25 Jul 2022, Smith, Jackson wrote:
> Hi Stefano,
> 
> My colleague Jason Lei and I would like to submit a patch to imagebuilder.
> 
> It seems that generating a .fit with a true dom0less configuration fails 
> because an extraneous comma is included in the its file.
> 
> We believe this change resolves the issue.

Hi Jackson, thanks for your contribution!

Yes, I see the problem: the code assumes there is a dom0 kernel. If
there is no dom0 kernel then load_files will be wrongly starting with a
","

I would be happy to commit your patch. I assume I can add your
Signed-off-by to it, right?

Signed-off-by: Jackson Smith 

Signed-off-by is the "Developer Certificate of Origin" which means:
https://developercertificate.org/



> 
> Remove extraneous comma in generated its file when no DOM0_KERNEL is 
> specified.
> 
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index 8f08cd6..6f94fce 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -676,7 +676,12 @@ create_its_file_xen()
>  i=$(( $i + 1 ))
>  continue
>  fi
> -load_files+=", \"domU${i}_kernel\""
> +   if test -z "$load_files"
> +   then
> +   load_files+="\"domU${i}_kernel\""
> +   else
> +   load_files+=", \"domU${i}_kernel\""
> +   fi
>  cat >> "$its_file" <<- EOF
>  domU${i}_kernel {
>  description = "domU${i} kernel binary";
> 
>

Re: [PATCH 3/4] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

2022-07-26 Thread Stefano Stabellini

On Tue, 26 Jul 2022, Jan Beulich wrote:
> On 26.07.2022 02:33, Stefano Stabellini wrote:
> > On Mon, 25 Jul 2022, Xenia Ragiadakou wrote:
> >> On 7/25/22 09:32, Jan Beulich wrote:
> >>> On 24.07.2022 19:20, Xenia Ragiadakou wrote:
>  On 7/7/22 10:55, Jan Beulich wrote:
> > On 07.07.2022 09:27, Xenia Ragiadakou wrote:
> >> On 7/6/22 11:51, Jan Beulich wrote:
> >>> On 06.07.2022 10:43, Xenia Ragiadakou wrote:
>  On 7/6/22 10:10, Jan Beulich wrote:
> > On 05.07.2022 23:02, Xenia Ragiadakou wrote:
> >> The function idle_loop() is referenced only in domain.c.
> >> Change its linkage from external to internal by adding the
> >> storage-class
> >> specifier static to its definitions.
> >>
> >> Since idle_loop() is referenced only in inline assembly, add
> >> the 'used'
> >> attribute to suppress unused-function compiler warning.
> >
> > While I see that Julien has already acked the patch, I'd like to
> > point
> > out that using __used here is somewhat bogus. Imo the better
> > approach
> > is to properly make visible to the compiler that the symbol is
> > used by
> > the asm(), by adding a fake(?) input.
> 
>  I 'm afraid I do not understand what do you mean by "adding a
>  fake(?)
>  input". Can you please elaborate a little on your suggestion?
> >>>
> >>> Once the asm() in question takes the function as an input, the
> >>> compiler
> >>> will know that the function has a user (unless, of course, it finds
> >>> a
> >>> way to elide the asm() itself). The "fake(?)" was because I'm not
> >>> deeply
> >>> enough into Arm inline assembly to know whether the input could then
> >>> also be used as an instruction operand (which imo would be
> >>> preferable) -
> >>> if it can't (e.g. because there's no suitable constraint or operand
> >>> modifier), it still can be an input just to inform the compiler.
> >>
> >> According to the following statement, your approach is the recommended
> >> one:
> >> "To prevent the compiler from removing global data or functions which
> >> are referenced from inline assembly statements, you can:
> >> -use __attribute__((used)) with the global data or functions.
> >> -pass the reference to global data or functions as operands to inline
> >> assembly statements.
> >> Arm recommends passing the reference to global data or functions as
> >> operands to inline assembly statements so that if the final image does
> >> not require the inline assembly statements and the referenced global
> >> data or function, then they can be removed."
> >>
> >> IIUC, you are suggesting to change
> >> asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" )
> >> into
> >> asm volatile ("mov sp,%0; b %1" : : "r" (stack), "S" (fn) : "memory"
> >> );
> >
> > Yes, except that I can't judge about the "S" constraint.
> >
> 
>  This constraint does not work for arm32. Any other thoughts?
> 
>  Another way, as Jan suggested, is to pass the function as a 'fake'
>  (unused) input.
>  I 'm suspecting something like the following could work
>  asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack), "X" (fn) :
>  "memory")
>  What do you think?
> >>>
> >>> Well, yes, X should always be a fallback option. But I said already when
> >>> you suggested S that I can't judge about its use, so I guess I'm the
> >>> wrong one to ask. Even more so that you only say "does not work", without
> >>> any details ...
> >>>
> >>
> >> The question is addressed to anyone familiar with arm inline assembly.
> >> I added the arm maintainers as primary recipients to this email to make 
> >> this
> >> perfectly clear.
> >>
> >> When cross-compiling Xen on x86 for arm32 with
> >> asm volatile ("mov sp,%0; b %1" : : "r" (stack), "S" (fn) : "memory" );
> >> compilation fails with the error: impossible constraint in ‘asm'
> > 
> > Unfortunately looking at the GCC manual pages [1], it doesn't seem to be
> > possible. The problem is that the definition of "S" changes between
> > aarch64 and arm (the 32-bit version).
> > 
> > For aarch64:
> > 
> > S   An absolute symbolic address or a label reference
> > 
> > This is what we want. For arm instead:
> > 
> > S   A symbol in the text segment of the current file
> > 
> > This is not useful for what we need to do here. As far as I can tell,
> > there is no other way in GCC assembly inline for arm to do this.
> > 
> > So we have 2 choices: we use the __used keyword as Xenia did in this
> > patch. Or we move the implementation of switch_stack_and_jump in
> > assembly. I attempted a prototype of the latter to see how it would come
> > out, see below.
> > 
> > I don't like it very much. I prefer the version with __used that Xenia
> > had in this

Re: [PATCH v2] x86/xen: Add support for HVMOP_set_evtchn_upcall_vector

2022-07-26 Thread Boris Ostrovsky




On 7/26/22 8:56 AM, Jane Malalane wrote:
  
+/* Setup per-vCPU vector-type callbacks and trick toolstack to think

+ * we are enlightened. If this setup is unavailable, fallback to the
+ * global vector-type callback. */



Comment style.



+static __init void xen_init_setup_upcall_vector(void)
+{
+   unsigned int cpu = 0;



Unnecessary variable.



+
+   if (!xen_have_vector_callback)
+   return;
+
+   if ((cpuid_eax(xen_cpuid_base() + 4) & XEN_HVM_CPUID_UPCALL_VECTOR) &&
+   !xen_set_upcall_vector(cpu) && !xen_set_callback_via(1))
+   xen_percpu_upcall = true;
+   else if (xen_feature(XENFEAT_hvm_callback_vector))
+   xen_setup_callback_vector();
+   else
+   xen_have_vector_callback = false;
+}
+
+int xen_set_upcall_vector(unsigned int cpu)
+{
+   int rc;
+   xen_hvm_evtchn_upcall_vector_t op = {
+   .vector = HYPERVISOR_CALLBACK_VECTOR,
+   .vcpu = per_cpu(xen_vcpu_id, cpu),
+   };
+
+   rc = HYPERVISOR_hvm_op(HVMOP_set_evtchn_upcall_vector, );
+   if (rc)
+   return rc;
+
+   if (!cpu)



A comment (e.g. "Let toolstack know that we are enlightened." or something 
along these lines) would be useful here.


-boris



+   rc = xen_set_callback_via(1);
+

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

2022-07-26 Thread osstest service owner

flight 171870 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/171870/

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  b1f0183e5067fbcb87517795e27929982b2404fb
baseline version:
 xen  a9949efb288fd6e21bbaf9d5826207c7c41cda27

Last test of basis   171864  2022-07-26 13:01:53 Z0 days
Testing same since   171870  2022-07-26 17:00:31 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 

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
   a9949efb28..b1f0183e50  b1f0183e5067fbcb87517795e27929982b2404fb -> smoke

Xen Security Advisory 408 v3 (CVE-2022-33745) - insufficient TLB flush for x86 PV guests in shadow mode

2022-07-26 Thread Xen . org security team

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Xen Security Advisory CVE-2022-33745 / XSA-408
   version 3

insufficient TLB flush for x86 PV guests in shadow mode

UPDATES IN VERSION 3


Update hash for metadata file.

ISSUE DESCRIPTION
=

For migration as well as to work around kernels unaware of L1TF (see
XSA-273), PV guests may be run in shadow paging mode.  To address
XSA-401, code was moved inside a function in Xen.  This code movement
missed a variable changing meaning / value between old and new code
positions.  The now wrong use of the variable did lead to a wrong TLB
flush condition, omitting flushes where such are necessary.

IMPACT
==

The known (observed) impact would be a Denial of Service (DoS) affecting
the entire host, due to running out of memory.  Privilege escalation and
information leaks cannot be ruled out.

VULNERABLE SYSTEMS
==

All versions of Xen with the XSA-401 fixes applied are vulnerable.

Only x86 PV guests can trigger this vulnerability, and only when running
in shadow mode.  Shadow mode would be in use when migrating guests or as
a workaround for XSA-273 (L1TF).

MITIGATION
==

Not running x86 PV guests will avoid the vulnerability.

CREDITS
===

This issue was discovered by Charles Arnold of SUSE.

RESOLUTION
==

Applying the appropriate attached patch resolves this issue.

Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball.  Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.

xsa408.patch   xen-unstable - Xen 4.14.x
xsa408-4.13.patch  Xen 4.13.x

$ sha256sum xsa408*
9411b563c71445d2c95e36aba9d71fa3b9341f0230e4b3e2549a63292df11669  xsa408.meta
f49cb67842c7576f1d59b965331956a9fa1f529a8e2da3531d7ebc4eb3f079b3  xsa408.patch
26871efbd3f834dd4af4fbab6f2cb09a83c509e49894f025ee656071419ed995  
xsa408-4.13.patch
$

DEPLOYMENT DURING EMBARGO
=

Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.

(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-BEGIN PGP SIGNATURE-

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmLgPzsMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZT5cIAKtisZZvdcSolZ+RHFzAdVEP2lbEW2TyoG6oy0st
kMsV/ZSabthow9PiUp48DoZOXSIh/7hn2qyXqx5X0VYjiWOISVRCldm5g4p0+tA/
GN6FztbRR1GargLkvtuWj38K9E7HIqfBRFLbtJD6X97NFSAPeNNZg8nqQPqwkhK+
yeGBjPPO5pTjNwsRt91A1qEttTPjbBpipEcit/qjqqCBxX6NT/pYSE5Ltn2OHm38
eYM25X901rJl0rPsyOeUN312FAL0bEunKVKJbiNcHVBZoR37YoJ5HE5trDxoxPrz
XYJdR7gzcB028lbGU4jt9FVHdYCh0htWpdpdWci4A3DCH7U=
=C02g
-END PGP SIGNATURE-


xsa408.meta
Description: Binary data


xsa408.patch
Description: Binary data


xsa408-4.13.patch
Description: Binary data

[xen-unstable test] 171862: tolerable FAIL - PUSHED

2022-07-26 Thread osstest service owner

flight 171862 xen-unstable real [real]
flight 171869 xen-unstable real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/171862/
http://logs.test-lab.xenproject.org/osstest/logs/171869/

Failures :-/ but no regressions.

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

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds18 guest-start/debian.repeat fail REGR. vs. 171859

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 171859
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 171859
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 171859
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 171859
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 171859
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 171859
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 171859
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 171859
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 171859
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 171859
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 171859
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 171859
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-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-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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-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-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-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-amd64-i386-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-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-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-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-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
 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  15 migrate-support-checkfail   never pass

[PATCH v2 2/2] automation: arm64: Create a test job for testing static allocation on qemu

2022-07-26 Thread Xenia Ragiadakou

Enable CONFIG_STATIC_MEMORY in the existing arm64 build.

Create a new test job, called qemu-smoke-arm64-gcc-staticmem.

Adjust qemu-smoke-arm64.sh script to accomodate the static memory test as a
new test variant. The test variant is determined based on the first argument
passed to the script. For testing static memory, the argument is 'static-mem'.

The test configures DOM1 with a static memory region and adds a check in the
init script.
The check consists in comparing the contents of the /proc/device-tree
memory entry with the static memory range with which DOM1 was configured.
If the memory layout is correct, a message gets printed by DOM1.

At the end of the qemu run, the script searches for the specific message
in the logs and fails if not found.

Signed-off-by: Xenia Ragiadakou 
---

Changes in v2:
- enable CONFIG_STATIC_MEMORY for all arm64 builds
- use the existing qemu-smoke-arm64.sh with an argument passed for
  distinguishing between tests, instead of creating a new script
- test does not rely on kernel logs but prints a message itself on success
- remove the part that enables custom configs for arm64 builds
- add comments
- adapt commit message

 automation/gitlab-ci/test.yaml | 18 +++
 automation/scripts/build   |  8 +
 automation/scripts/qemu-smoke-arm64.sh | 42 --
 3 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 26bdbcc3ea..6f9f64a8da 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -80,6 +80,24 @@ qemu-smoke-arm64-gcc:
   tags:
 - arm64
 
+qemu-smoke-arm64-gcc-staticmem:
+  extends: .test-jobs-common
+  variables:
+CONTAINER: debian:unstable-arm64v8
+  script:
+- ./automation/scripts/qemu-smoke-arm64.sh static-mem 2>&1 | tee 
qemu-smoke-arm64.log
+  needs:
+- debian-unstable-gcc-arm64
+- kernel-5.9.9-arm64-export
+- qemu-system-aarch64-6.0.0-arm64-export
+  artifacts:
+paths:
+  - smoke.serial
+  - '*.log'
+when: always
+  tags:
+- arm64
+
 qemu-smoke-arm32-gcc:
   extends: .test-jobs-common
   variables:
diff --git a/automation/scripts/build b/automation/scripts/build
index 21b3bc57c8..1941763315 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -18,6 +18,14 @@ else
 make -j$(nproc) -C xen defconfig
 fi
 
+if [[ "${XEN_TARGET_ARCH}" = "arm64" ]]; then
+echo "
+CONFIG_EXPERT=y
+CONFIG_UNSUPPORTED=y
+CONFIG_STATIC_MEMORY=y" > xen/.config
+make -j$(nproc) -C xen olddefconfig
+fi
+
 # Save the config file before building because build failure causes the script
 # to exit early -- bash is invoked with -e.
 cp xen/.config xen-config
diff --git a/automation/scripts/qemu-smoke-arm64.sh 
b/automation/scripts/qemu-smoke-arm64.sh
index 53086a5ac7..48513a7832 100755
--- a/automation/scripts/qemu-smoke-arm64.sh
+++ b/automation/scripts/qemu-smoke-arm64.sh
@@ -2,6 +2,15 @@
 
 set -ex
 
+test_variant=$1
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+# Memory range that is statically allocated to DOM1
+domu_base="5000"
+domu_size="1000"
+passed="static-mem test passed"
+fi
+
 # Install QEMU
 export DEBIAN_FRONTENT=noninteractive
 apt-get -qy update
@@ -42,8 +51,22 @@ echo "#!/bin/sh
 
 mount -t proc proc /proc
 mount -t sysfs sysfs /sys
-mount -t devtmpfs devtmpfs /dev
-/bin/sh" > initrd/init
+mount -t devtmpfs devtmpfs /dev" > initrd/init
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+# Verify that DOM1 booted with the expected memory layout by checking the
+# contents of the memory entry in /proc/device-tree.
+echo "
+current=\$(hexdump -e '16/1 \"%02x\"' 
/proc/device-tree/memory@${domu_base}/reg 2>/dev/null)
+expected=\"$(printf \"%016x%016x\" 0x${domu_base} 0x${domu_size})\"
+if [[ ${expected} == \"\${current}\" ]]; then
+   echo \"${passed}\"
+fi" >> initrd/init
+fi
+
+echo "
+/bin/sh" >> initrd/init
+
 chmod +x initrd/init
 cd initrd
 find . | cpio --create --format='newc' | gzip > ../binaries/initrd
@@ -68,6 +91,12 @@ DOMU_MEM[0]="256"
 LOAD_CMD="tftpb"
 UBOOT_SOURCE="boot.source"
 UBOOT_SCRIPT="boot.scr"' > binaries/config
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+echo "
+DOMU_STATIC_MEM[0]=\"0x${domu_base} 0x${domu_size}\"" >> binaries/config
+fi
+
 rm -rf imagebuilder
 git clone https://gitlab.com/ViryaOS/imagebuilder
 bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c 
binaries/config
@@ -89,5 +118,12 @@ timeout -k 1 240 \
 -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin |& tee smoke.serial
 
 set -e
-(grep -q "^BusyBox" smoke.serial && grep -q "DOM1: BusyBox" smoke.serial) || 
exit 1
+grep -q "^BusyBox" smoke.serial || exit 1
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+grep -q "DOM1: ${passed}" smoke.serial || exit 1
+else
+grep -q "DOM1: BusyBox" smoke.serial || exit 1
+fi
+
 exit 0
-- 
2.34.1

[PATCH v2 0/2] Create a test job for testing static memory on qemu

2022-07-26 Thread Xenia Ragiadakou

This patch series
- removes all the references to the XEN_CONFIG_EXPERT environmental variable
which is not used anymore
- creates a trivial arm64 test job that boots xen on qemu with dom0 and a
direct mapped dom0less domu with static memory and verifies that domu's memory
ranges are the same as the static memory ranges with which it was configured

The static memory test relies on the existing qemu-smoke-arm64.sh script.
This script uses the kernel-5.9.9 from the test-artifacts container as domu
kernel. This particular kernel does not work with dom0less enhanced enabled.
More specifically, domu crashes when it attempts to dereference the xenstore
interface which is still uninitialized,
So, qemu-smoke-arm64-gcc test, as well as its static memory version, fails.

To be able to test, I had to disable dom0less enhanced by adding the following
commands to the script.

sed -i 's/xen,enhanced "enabled"/xen,enhanced "disabled"/g' binaries/boot.source
mkimage -A arm64 -T script -C none -d binaries/boot.source binaries/boot.scr

These commands are not part of the patch.
Since dom0less enhanced is enabled by default, a newer kernel version would
be more appropriate for testing dom0less.

Xenia Ragiadakou (2):
  automation: Remove XEN_CONFIG_EXPERT leftovers
  automation: arm64: Create a test job for testing static allocation on
qemu

 automation/build/README.md |  3 --
 automation/gitlab-ci/test.yaml | 18 +++
 automation/scripts/build   | 12 ++--
 automation/scripts/containerize|  1 -
 automation/scripts/qemu-smoke-arm64.sh | 42 --
 5 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] automation: Remove XEN_CONFIG_EXPERT leftovers

2022-07-26 Thread Xenia Ragiadakou

The EXPERT config option cannot anymore be selected via the environmental
variable XEN_CONFIG_EXPERT. Remove stale references to XEN_CONFIG_EXPERT
from the automation code.

Signed-off-by: Xenia Ragiadakou 
Reviewed-by: Stefano Stabellini 
---

Changes in v2:
- add Stefano's R-b tag

 automation/build/README.md  | 3 ---
 automation/scripts/build| 4 ++--
 automation/scripts/containerize | 1 -
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/automation/build/README.md b/automation/build/README.md
index 2137957408..00305eed03 100644
--- a/automation/build/README.md
+++ b/automation/build/README.md
@@ -65,9 +65,6 @@ understands.
 - CONTAINER_NO_PULL: If set to 1, the script will not pull from docker hub.
   This is useful when testing container locally.
 
-- XEN_CONFIG_EXPERT: If this is defined in your shell it will be
-  automatically passed through to the container.
-
 If your docker host has Linux kernel > 4.11, and you want to use containers
 that run old glibc (for example, CentOS 6 or SLES11SP4), you may need to add
 
diff --git a/automation/scripts/build b/automation/scripts/build
index 281f8b1fcc..21b3bc57c8 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -91,6 +91,6 @@ for cfg in `ls ${cfg_dir}`; do
 echo "Building $cfg"
 make -j$(nproc) -C xen clean
 rm -f xen/.config
-make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} 
XEN_CONFIG_EXPERT=y defconfig
-make -j$(nproc) -C xen XEN_CONFIG_EXPERT=y
+make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
+make -j$(nproc) -C xen
 done
diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index 8992c67278..9d4beca4fa 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -101,7 +101,6 @@ exec ${docker_cmd} run \
 -v "${CONTAINER_PATH}":/build:rw${selinux} \
 -v "${HOME}/.ssh":/root/.ssh:ro \
 ${SSH_AUTH_DIR:+-v "${SSH_AUTH_DIR}":/tmp/ssh-agent${selinux}} \
-${XEN_CONFIG_EXPERT:+-e XEN_CONFIG_EXPERT=${XEN_CONFIG_EXPERT}} \
 ${CONTAINER_ARGS} \
 -${termint}i --rm -- \
 ${CONTAINER} \
-- 
2.34.1

[qemu-mainline test] 171863: tolerable FAIL - PUSHED

2022-07-26 Thread osstest service owner

flight 171863 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/171863/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 171749
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 171749
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 171749
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 171749
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 171749
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 171749
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 171749
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 171749
 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-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-i386-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-xsm  15 migrate-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-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-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 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-amd64-amd64-libvirt 15 migrate-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-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-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 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
 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-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 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-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-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-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

version targeted for testing:
 qemuu616a6459d878999b37c7cfbf1ed8d4ca84b3751f
baseline version:
 qemuu5288bee45fbd33203b61f8c76e41b15bb5913e6e

Last test of basis   171749  2022-07-21 21:10:51 Z4 days
Testing same since   171863  2022-07-26 10:07:00 Z0 days1 attempts


People who touched revisions under test:
  Alexander Bulekov 
  Bin Meng 
  Jason A. Donenfeld

Re: [PATCH] page-alloc: fix initialization of cross-node regions

2022-07-26 Thread Julien Grall


Hi Jan,

On 25/07/2022 17:18, Jan Beulich wrote:

On 25.07.2022 18:05, Julien Grall wrote:

(Sorry for the formatting)


No issues seen.


Good to know. I sent it from my phone and the gmail app used to mangle 
e-mails.





On Mon, 25 Jul 2022, 14:10 Jan Beulich,  wrote:


Quite obviously to determine the split condition successive pages'
attributes need to be evaluated, not always those of the initial page.

Fixes: 72b02bc75b47 ("xen/heap: pass order to free_heap_pages() in heap
init")
Signed-off-by: Jan Beulich 
---
Part of the problem was already introduced in 24a53060bd37 ("xen/heap:
Split init_heap_pages() in two"), but there it was still benign.



Is this because range will never cross numa node? How about the fake NUMA
node?


No (afaict), because pages were still freed one by one (and hence node
boundaries still wouldn't end up in the middle of a buddy).


So I agree that free_heap_pages() would be called with one page at the 
time. However, I think _init_heap_pages() would end up to be called with 
the full range.


So we would initialize the first node but not the others (if the range 
spans over multiple ones). Therefore, I think free_heap_pages() could 
dereference a NULL pointer.


Anyway, I would not expect anyone to only backport the patch to split 
_init_heap_pages() and... in any case you already committed it (which is 
fine given this is a major regression).


Cheers,

--
Julien Grall

Re: [PATCH] arm/vgic-v3: fix virq offset in the rank

2022-07-26 Thread Julien Grall


On 26/07/2022 18:54, Luca Fancellu wrote:


Hi Julien,


Hi Luca,


/* Get the index in the rank */
- offset &= virq & INTERRUPT_RANK_MASK;
+ offset = virq & INTERRUPT_RANK_MASK;


AFAICT, vgic_fetch_irouter() has the same problem. Can you update it here as 
well?


I think that function is ok, because in there we have:

/* There is exactly 1 vIRQ per IROUTER */
offset = offset  / NR_BYTES_PER_IROUTER;

/* Get the index in the rank */
offset = offset & INTERRUPT_RANK_MASK;

Which is basically offset = (offset  / NR_BYTES_PER_IROUTER) & 
INTERRUPT_RANK_MASK;

Like in the counterpart (updated by this patch) vgic_store_irouter who has:

/* There is 1 vIRQ per IROUTER */
virq = offset / NR_BYTES_PER_IROUTER;

[…]

/* Get the index in the rank */
offset = virq & INTERRUPT_RANK_MASK;

Which is the same as above


You are right. So the patch looks correct to me.

Although, I would still like the commit message to be clarified.

Cheers,

--
Julien Grall

Re: [PATCH v1 01/18] kconfig: allow configuration of maximum modules

2022-07-26 Thread Julien Grall


Hi Daniel,

On 19/07/2022 17:36, Daniel P. Smith wrote:


On 7/15/22 15:16, Julien Grall wrote:

Hi Daniel,

On 06/07/2022 22:04, Daniel P. Smith wrote:

For x86 the number of allowable multiboot modules varies between the
different
entry points, non-efi boot, pvh boot, and efi boot. In the case of
both Arm and
x86 this value is fixed to values based on generalized assumptions. With
hyperlaunch for x86 and dom0less on Arm, use of static sizes results
in large
allocations compiled into the hypervisor that will go unused by many
use cases.

This commit introduces a Kconfig variable that is set with sane
defaults based
on configuration selection. This variable is in turned used as the
array size
for the cases where a static allocated array of boot modules is declared.

Signed-off-by: Daniel P. Smith 
Reviewed-by: Christopher Clark 


I am not entirely sure where this reviewed-by is coming from. Is this
from internal review?


Yes.


If yes, my recommendation would be to provide the reviewed-by on the
mailing list. Ideally, the review should also be done in the open, but I
understand some company wish to do a fully internal review first.


Since this capability is being jointly developed by Christopher and I,
with myself being the author of code, Christopher reviewed the code as
the co-developer. He did so as a second pair of eyes for any obvious
mistakes and to concur that the implementation was in line with the
approach the two of us architected. Perhaps a SoB line might be more
appropriate than an R-b line.


At least from a committer perspective, this helps me to know whether the
reviewed-by still apply. An example would be if you send a v2, I would
not be able to know whether Christoffer still agreed on the change.


If an SoB line is more appropriate, then on the next version I can
switch it


Thanks for the explanation. To me "signed-off-by" means the person wrote 
some code (or sent the patches) code. So from above, it sounds more like 
Christoffer did a review.


So I think it is more suitable for him to provide a reviewed-by. For 
follow-up, my preference would be Christoffer to provide the reviewed-by 
on the ML.


If it is too much overhead, I would suggest to log the latest version 
Christoffer reviewed-by in the changelog. I usually do:


Changes in vX:
  - Add Christoffer's reviewed-by

Or if he will reviewing every version, just mention it in the cover letter.



Please explain in the commit message why the number of modules was
bumped from 5 to 9.


The number of modules were inconsistent between the different entry
points into __start_xen(). By switching to a Kconfig variable, whose
default was set to the largest value used across the entry points,
results in change for the locations using another value.


Ok. Can you add something like: "For x86, the number of modules is not 
consistent across the code base. Use the maximum"?




See below for +1 explanation.


     static void __init edd_put_string(u8 *dst, size_t n, const char *src)
   {
diff --git a/xen/arch/x86/guest/xen/pvh-boot.c
b/xen/arch/x86/guest/xen/pvh-boot.c
index 498625eae0..834b1ad16b 100644
--- a/xen/arch/x86/guest/xen/pvh-boot.c
+++ b/xen/arch/x86/guest/xen/pvh-boot.c
@@ -32,7 +32,7 @@ bool __initdata pvh_boot;
   uint32_t __initdata pvh_start_info_pa;
     static multiboot_info_t __initdata pvh_mbi;
-static module_t __initdata pvh_mbi_mods[8];
+static module_t __initdata pvh_mbi_mods[CONFIG_NR_BOOTMOD + 1];


What's the +1 for?


I should clarify in the commit message, but the value set in
CONFIG_NR_BOOTMOD is the max modules that Xen would accept from a
bootloader. Xen startup code expects to be able to append Xen itself as
the array. The +1 allocates an additional entry to store Xen in the
array should a bootloader actually pass CONFIG_NR_BOOTMOD modules to
Xen. There is an existing comment floating in one of these locations
that explained it.


This makes sense. So every use of CONFIG_NR_BOOTMOD would end up to 
require +1. Is that correct?


If yes, then I think it would be better to require CONFIG_NR_BOOTMOD to 
be at minimum 1. This would reduce the risk to have different array size 
again. That said, this is x86 code, so the call is for the x86 maintainers.





   static const char *__initdata pvh_loader = "PVH Directboot";
     static void __init convert_pvh_info(multiboot_info_t **mbi,
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index f08b07b8de..2aa1e28c8f 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1020,9 +1020,9 @@ void __init noreturn __start_xen(unsigned long
mbi_p)
   panic("dom0 kernel not specified. Check bootloader
configuration\n");
     /* Check that we don't have a silly number of modules. */
-    if ( mbi->mods_count > sizeof(module_map) * 8 )
+    if ( mbi->mods_count > CONFIG_NR_BOOTMODS )
   {
-    mbi->mods_count = sizeof(module_map) * 8;
+    mbi->mods_count = CONFIG_NR_BOOTMODS;
   printk("Excessive multiboot modules - using the

Re: [PATCH] arm/vgic-v3: fix virq offset in the rank

2022-07-26 Thread Luca Fancellu


Hi Julien,

>> /* Get the index in the rank */
>> - offset &= virq & INTERRUPT_RANK_MASK;
>> + offset = virq & INTERRUPT_RANK_MASK;
> 
> AFAICT, vgic_fetch_irouter() has the same problem. Can you update it here as 
> well?

I think that function is ok, because in there we have:

/* There is exactly 1 vIRQ per IROUTER */
offset = offset  / NR_BYTES_PER_IROUTER;

/* Get the index in the rank */
offset = offset & INTERRUPT_RANK_MASK;

Which is basically offset = (offset  / NR_BYTES_PER_IROUTER) & 
INTERRUPT_RANK_MASK;

Like in the counterpart (updated by this patch) vgic_store_irouter who has:

/* There is 1 vIRQ per IROUTER */
virq = offset / NR_BYTES_PER_IROUTER;

[…]

/* Get the index in the rank */
offset = virq & INTERRUPT_RANK_MASK;

Which is the same as above

Cheers,
Luca

Re: [PATCH] arm/vgic-v3: fix virq offset in the rank

2022-07-26 Thread Julien Grall


Hi,

Title: I would suggest to mention the irouter in the title.

On 15/07/2022 11:46, Hongda Deng wrote:

When vgic performs irouter registers emulation, to get the target vCPU
via virq conveniently, Xen doesn't store the irouter value directly,
instead it will use the value (affinities) in irouter to calculate the
target vCPU, and then save the target vCPU in irq rank->vCPU[offset].


NIT: The field is technically called "vcpu".


But there seems to be a bug in the way the offset is calculated when
vgic tries to store irouter.


NIT: I would drop this sentence because below you suggest this is a bug.



When vgic tries to get the target vcpu, it first calculates the target


Typo: When the vGIC...


vcpu index via
   int target = read_atomic(>vcpu[virq & INTERRUPT_RANK_MASK]);
and then get the target vcpu via
   v->domain->vcpu[target];

When vgic tries to store irouter for one virq, the target vcpu index
in the rank is got via
   offset &= virq & INTERRUPT_RANK_MASK;
finally it gets the target vcpu via
   d->vcpu[read_atomic(>vcpu[offset])];

There is a difference between them when gets the target vcpu index in
the rank.


Typo: I think you mean 'getting' rather than 'gets'.



Here (virq & INTERRUPT_RANK_MASK) would already get the  target vcpu
index in the rank, so we don't need the '&' before '=' when calculate
the offset.


This is a bit confusing to read. Through the commit message you give mix 
information about the issue. "don't need" to me means this is pointless 
but harmless. But then a bit below, you write this is a bug.


I would suggest to change the order with the next paragraph (it may need 
some tweaks) and replace the "don't need" with something more assertive.



For example, the target vcpu index in the rank should be 6 for virq 38,
but vgic will get offset=0 when vgic stores the irouter for this virq,
and finally vgic will access wrong target vcpu index in the rank when
it updates the irouter.

Fixes: 5d495f4349b5 ("xen/arm: vgic: Optimize the way to store the target vCPU in 
the rank")
Signed-off-by: Hongda Deng 
---
  xen/arch/arm/vgic-v3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index e4ba9a6476..7fb99a9ff2 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -135,7 +135,7 @@ static void vgic_store_irouter(struct domain *d, struct 
vgic_irq_rank *rank,
  ASSERT(virq >= 32);
  
  /* Get the index in the rank */

-offset &= virq & INTERRUPT_RANK_MASK;
+offset = virq & INTERRUPT_RANK_MASK;


AFAICT, vgic_fetch_irouter() has the same problem. Can you update it 
here as well?


  
  new_vcpu = vgic_v3_irouter_to_vcpu(d, irouter);

  old_vcpu = d->vcpu[read_atomic(>vcpu[offset])];


Cheers,

--
Julien Grall

Re: [PATCH v3] livepatch: create-diff-object: Check that the section has a secsym

2022-07-26 Thread Sarah Newman


On 7/25/22 23:25, Jan Beulich wrote:

On 25.07.2022 19:13, Sarah Newman wrote:

A STT_SECTION symbol is not needed if if it is not used as a relocation
target. Therefore, a section, in this case a debug section, may not have
a secsym associated with it.

Signed-off-by: Bill Wendling 


Hmm - this wasn't here before. Does this then suggest the patch also
wants to be marked From: Bill?


I don't know what the etiquette is here since this was a commit originally committed to kpatch, I just added that back because the xen patch 
submission guidelines said to preserve original tags.





Origin: https://github.com/dynup/kpatch.git ba3defa06073
Signed-off-by: Sarah Newman 
Reviewed-by: Ross Lagerwall 


Sigh. I had given R-b on v1 as well. Actually I had meant to commit this
yesterday (with all adjustments made), but as it turns out committers
can't commit to that tree. So it'll be up to Ross or Konrad to actually
take care of this.

Jan



My apologies, I simply missed that.

I am doing this in my spare time. This is the first time I've gone through this 
process in a couple of years and have only done it a few times total.

Thanks, Sarah

Re: [PATCH 2/8] xen/evtchn: modify evtchn_alloc_unbound to allocate specified port

2022-07-26 Thread Julien Grall





On 21/07/2022 16:37, Rahul Singh wrote:

Hi Julien,


Hi Rahul,


On 21 Jul 2022, at 2:29 pm, Julien Grall  wrote:

On 21/07/2022 13:50, Rahul Singh wrote:

Hi Julien,


Hi Rahul,


On 20 Jul 2022, at 12:16 pm, Julien Grall  wrote:

Hi Rahul,

On 20/07/2022 10:59, Rahul Singh wrote:

On 13 Jul 2022, at 1:29 pm, Julien Grall  wrote:



On 13/07/2022 13:12, Bertrand Marquis wrote:

On 13 Jul 2022, at 12:31, Julien Grall  wrote:

I can't
see why it would be wrong to have a more tight limit on static ports
than on traditional ("dynamic") ones. Even if only to make sure so
many dynamic ones are left.


This is similar to Xen forbidding to close a static port: it is not the 
hypervisor business to check that there are enough event channel ports freed 
for dynamic allocation.

On other side we need to be cautious not to add too much complexity in the code 
by trying to make things always magically work.
If you want Xen to be accessible to non expert by magically working all the 
time, there would be a lot of work to do.


It is not clear to me whether you are referring to a developper or admin here.

On the admin side, we need to make sure they have an easy way to configure 
event channels. One knob is always going to easier than two knobs.

On the developper side, this could be resolved by better documentation in the 
code/interface.

Cheers,

To conclude the discussion, If everyone agree I will add the below patch or 
similar in the next version to restrict the
max number of evtchn supported as suggested.


I am fine if the limit for domU is fixed by Xen for now. However, for dom0, 
4096 is potentially too low if you have many PV drivers (each backend will need 
a few event channels). So I don't think this wants to be fixed by Xen.

Agree.


I am not entirely sure we want to limit the number of event channels for dom0. 
But if you want to, then this will have to be done via a command line option 
(or device-tree property).

We need to support the static event channel for dom0 also, in that case, we 
need to restrict the max number of evtchn for dom0 to mitigate the security 
issue.


It sounds like there are some misundertanding or I am missing some context. The 
static event channels will be allocated at boot, so the worse that can happen 
is it will be slower to boot.

My point regarding fifo was more in the generic case of allowing the caller to 
select the port. This would be a concern in the context of non-cooperative 
live-migration. An easy way is to restrict the number of ports. For you, this 
is just an increase in boot time.

Furthermore, there is an issue for dom0less domUs because we don't limit the 
number of port by default. This means that a domU can allocate a large amount 
of memory in Xen (we need some per-event channel state). Hence why I suggested 
to update max_evtchn_channel.


Thanks for the clarification.



If the admin set the value greater than 4096 (or what we agreed on) and static 
event channel support is enabled we will print the warning to the user related 
to fill
the hole issue for FIFO ABI.


See above. I don't see the need for a warning. The admin will notice that it is 
slower to boot.


Ok. I will not add the warning. Just to confirm again is that okay If I add new 
command line option "max_event_channels”  in
next version for dom0 to restrict the max number of evtchn.


Personally I am fine with a command line option to *globally* restrict 
the number of events channel. But Jan seemed to have some reservation. 
Quoting what he wrote previously:


"Imo there would need to be a very good reason to limit Dom0's port range.
"

Cheers,

--
Julien Grall

Re: [PATCH] xen/arm: smmuv1: remove iommu group when deassign a device

2022-07-26 Thread Julien Grall


Hi,

On 21/07/2022 16:53, Rahul Singh wrote:

On 28 Jun 2022, at 6:23 pm, Mykyta Poturai  wrote:
With this patch I get the same results, here is the error message:

(XEN) smmu: /iommu@5140: Unexpected global fault, this could be serious
(XEN) smmu: /iommu@5140:GFSR 0x0001, GFSYNR0 0x0004, GFSYNR1 
0x1055, GFSYNR2 0x



As you mentioned earlier, this fault is observed because GPU continues to 
access memory when the context is
already invalidated, and therefore triggers the "Invalid context fault".  This 
is a different issue and is not related to this patch.

@Julien are you okay if I will send the below patch for official review as this 
issue pending for a long time?


I am OK in principle. I will do a proper review on you send a formal patch.

Cheers,

--
Julien Grall

Re: [PATCH 3/8] mm: enforce return value checking on get_page()

2022-07-26 Thread Julien Grall


Hi Jan,

On 26/07/2022 17:04, Jan Beulich wrote:

It's hard to imagine a case where an error may legitimately be ignored
here. It's bad enough that in at least one case (set_shadow_status())
the return value was checked only by way of ASSERT()ing.

Signed-off-by: Jan Beulich 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall

access usb mass storage from multiple DomUs

2022-07-26 Thread A Sudheer

Hi all,

i am able to passthrough usb mass storage to DomUs but only one DomU at a
time.

For passthrough mode, added below parameters in cfg file and is working.
usbctrl=['type=auto, version=2, ports=6']
usbdev=['hostbus=2, hostaddr=2']

Is there a way to share the mass storage between multiple DomUs at a time ?
How do i configure to access usb mass storage in multiple DomU
simultaneously.

Thanks
Sudheer

[PATCH 8/8] x86/mm: re-arrange type check around _get_page_type()'s TLB flush

2022-07-26 Thread Jan Beulich

Checks dependent on only d and x can be pulled out, thus allowing to
skip the flush mask calculation.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3020,7 +3020,10 @@ static int _get_page_type(struct page_in
 if ( d && shadow_mode_enabled(d) )
 shadow_prepare_page_type_change(d, page);
 
-if ( (x ^ type) & PGT_type_mask )
+if ( ((x ^ type) & PGT_type_mask) &&
+ /* Shadow mode: track only writable pages. */
+ (!shadow_mode_enabled(d) ||
+  ((x & PGT_type_mask) == PGT_writable_page)) )
 {
 /*
  * On type change we check to flush stale TLB entries. It is
@@ -3035,10 +3038,7 @@ static int _get_page_type(struct page_in
 /* Don't flush if the timestamp is old enough */
 tlbflush_filter(mask, page->tlbflush_timestamp);
 
-if ( unlikely(!cpumask_empty(mask)) &&
- /* Shadow mode: track only writable pages. */
- (!shadow_mode_enabled(d) ||
-  ((x & PGT_type_mask) == PGT_writable_page)) )
+if ( unlikely(!cpumask_empty(mask)) )
 {
 perfc_incr(need_flush_tlb_flush);
 /*

[PATCH 7/8] x86/mm: adjust type check around _get_page_type()'s TLB flush

2022-07-26 Thread Jan Beulich

While "type" can include PGT_pae_xen_l2, "x" can't, as the bit is
cleared upon de-validation (see also the respective assertion earlier in
the function).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3020,7 +3020,7 @@ static int _get_page_type(struct page_in
 if ( d && shadow_mode_enabled(d) )
 shadow_prepare_page_type_change(d, page);
 
-if ( (x & PGT_type_mask) != type )
+if ( (x ^ type) & PGT_type_mask )
 {
 /*
  * On type change we check to flush stale TLB entries. It is

[PATCH 5/8] x86/shadow: don't open-code shadow_remove_all_shadows()

2022-07-26 Thread Jan Beulich

Let's use the existing inline wrapper instead of repeating respective
commentary at every site.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -389,7 +389,7 @@ static int oos_remove_write_access(struc
  * the page.  If that doesn't work either, the guest is granting
  * his pagetables and must be killed after all.
  * This will flush the tlb, so we can return with no worries. */
-sh_remove_shadows(d, gmfn, 0 /* Be thorough */, 1 /* Must succeed */);
+shadow_remove_all_shadows(d, gmfn);
 return 1;
 }
 
--- a/xen/arch/x86/mm/shadow/hvm.c
+++ b/xen/arch/x86/mm/shadow/hvm.c
@@ -783,7 +783,7 @@ sh_remove_all_shadows_and_parents(struct
 /* Even harsher: this is a HVM page that we thing is no longer a pagetable.
  * Unshadow it, and recursively unshadow pages that reference it. */
 {
-sh_remove_shadows(d, gmfn, 0, 1);
+shadow_remove_all_shadows(d, gmfn);
 /* XXX TODO:
  * Rework this hashtable walker to return a linked-list of all
  * the shadows it modified, then do breadth-first recursion
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -2632,7 +2632,7 @@ static int cf_check sh_page_fault(
 SHADOW_PRINTK("user-mode fault to PT, unshadowing mfn %#lx\n",
   mfn_x(gmfn));
 perfc_incr(shadow_fault_emulate_failed);
-sh_remove_shadows(d, gmfn, 0 /* thorough */, 1 /* must succeed */);
+shadow_remove_all_shadows(d, gmfn);
 trace_shadow_emulate_other(TRC_SHADOW_EMULATE_UNSHADOW_USER,
   va, gfn);
 goto done;
@@ -2718,7 +2718,7 @@ static int cf_check sh_page_fault(
 v->arch.paging.last_write_emul_ok = 0;
 }
 #endif
-sh_remove_shadows(d, gmfn, 0 /* thorough */, 1 /* must succeed */);
+shadow_remove_all_shadows(d, gmfn);
 trace_shadow_emulate_other(TRC_SHADOW_EMULATE_UNSHADOW_EVTINJ,
va, gfn);
 return EXCRET_fault_fixed;

[PATCH 6/8] x86/shadow: drop CONFIG_HVM conditionals from sh_update_cr3()

2022-07-26 Thread Jan Beulich

Now that we're not building multi.c anymore for 2 and 3 guest levels
when !HVM, there's no point in having these conditionals anymore. (As
somewhat a special case, the last of the removed conditionals really
builds on shadow_mode_external() always returning false when !HVM.) This
way the code becomes a tiny bit more readable.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3228,7 +3228,7 @@ static void cf_check sh_update_cr3(struc
 {
 struct domain *d = v->domain;
 mfn_t gmfn;
-#if GUEST_PAGING_LEVELS == 3 && defined(CONFIG_HVM)
+#if GUEST_PAGING_LEVELS == 3
 const guest_l3e_t *gl3e;
 unsigned int i, guest_idx;
 #endif
@@ -3279,7 +3279,7 @@ static void cf_check sh_update_cr3(struc
 #endif
 gmfn = pagetable_get_mfn(v->arch.guest_table);
 
-#if GUEST_PAGING_LEVELS == 3 && defined(CONFIG_HVM)
+#if GUEST_PAGING_LEVELS == 3
 /*
  * On PAE guests we don't use a mapping of the guest's own top-level
  * table.  We cache the current state of that table and shadow that,
@@ -3321,8 +3321,6 @@ static void cf_check sh_update_cr3(struc
   !VM_ASSIST(d, m2p_strict) )
 fill_ro_mpt(smfn);
 }
-#elif !defined(CONFIG_HVM)
-ASSERT_UNREACHABLE();
 #elif GUEST_PAGING_LEVELS == 3
 /* PAE guests have four shadow_table entries, based on the
  * current values of the guest's four l3es. */
@@ -3373,8 +3371,6 @@ static void cf_check sh_update_cr3(struc
 #error This should never happen
 #endif
 
-
-#ifdef CONFIG_HVM
 ///
 /// v->arch.paging.shadow.l3table
 ///
@@ -3400,7 +3396,6 @@ static void cf_check sh_update_cr3(struc
 }
 }
 #endif /* SHADOW_PAGING_LEVELS == 3 */
-#endif /* CONFIG_HVM */
 
 ///
 /// v->arch.cr3
@@ -3419,8 +3414,6 @@ static void cf_check sh_update_cr3(struc
 }
 #endif
 
-
-#ifdef CONFIG_HVM
 ///
 /// v->arch.hvm.hw_cr[3]
 ///
@@ -3437,7 +3430,6 @@ static void cf_check sh_update_cr3(struc
 #endif
 hvm_update_guest_cr3(v, noflush);
 }
-#endif /* CONFIG_HVM */
 
 /* Fix up the linear pagetable mappings */
 sh_update_linear_entries(v);

[PATCH 4/8] x86/shadow: exclude HVM-only code from sh_remove_shadows() when !HVM

2022-07-26 Thread Jan Beulich

In my (debug) build this amounts to well over 500 bytes of dead code.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -2240,10 +2240,12 @@ void sh_remove_shadows(struct domain *d,
 }   \
 } while (0)
 
+#ifdef CONFIG_HVM
 DO_UNSHADOW(SH_type_l2_32_shadow);
 DO_UNSHADOW(SH_type_l1_32_shadow);
 DO_UNSHADOW(SH_type_l2_pae_shadow);
 DO_UNSHADOW(SH_type_l1_pae_shadow);
+#endif
 DO_UNSHADOW(SH_type_l4_64_shadow);
 DO_UNSHADOW(SH_type_l3_64_shadow);
 DO_UNSHADOW(SH_type_l2h_64_shadow);

[PATCH 3/8] mm: enforce return value checking on get_page()

2022-07-26 Thread Jan Beulich

It's hard to imagine a case where an error may legitimately be ignored
here. It's bad enough that in at least one case (set_shadow_status())
the return value was checked only by way of ASSERT()ing.

Signed-off-by: Jan Beulich 

--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -61,7 +61,7 @@
 struct page_info;
 
 void put_page(struct page_info *);
-bool get_page(struct page_info *, const struct domain *);
+bool __must_check get_page(struct page_info *, const struct domain *);
 struct domain *__must_check page_get_owner_and_reference(struct page_info *);
 
 /* Boot-time allocator. Turns into generic allocator after bootstrap. */

[PATCH 2/8] x86/shadow: properly handle get_page() failing

2022-07-26 Thread Jan Beulich

We should not blindly (in a release build) insert the new entry in the
hash if a reference to the guest page cannot be obtained, or else an
excess reference would be put when removing the hash entry again. Crash
the domain in that case instead. The sole caller doesn't further care
about the state of the guest page: All it does is return the
corresponding shadow page (which was obtained successfully before) to
its caller.

To compensate we further need to adjust hash removal: Since the shadow
page already has had its backlink set, domain cleanup code would try to
destroy the shadow, and hence still cause a put_page() without
corresponding get_page(). Leverage that the failed get_page() leads to
no hash insertion, making shadow_hash_delete() no longer assume it will
find the requested entry. Instead return back whether the entry was
found. This way delete_shadow_status() can avoid calling put_page() in
the problem scenario.

For the other caller of shadow_hash_delete() simply reinstate the
otherwise dropped assertion at the call site.

While touching the conditionals in {set,delete}_shadow_status() anyway,
also switch around their two pre-existing parts, to have the cheap one
first (frequently allowing to avoid evaluation of the expensive - due to
evaluate_nospec() - one altogether).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -1581,7 +1581,7 @@ void shadow_hash_insert(struct domain *d
 sh_hash_audit_bucket(d, key);
 }
 
-void shadow_hash_delete(struct domain *d, unsigned long n, unsigned int t,
+bool shadow_hash_delete(struct domain *d, unsigned long n, unsigned int t,
 mfn_t smfn)
 /* Excise the mapping (n,t)->smfn from the hash table */
 {
@@ -1606,10 +1606,8 @@ void shadow_hash_delete(struct domain *d
 {
 /* Need to search for the one we want */
 x = d->arch.paging.shadow.hash_table[key];
-while ( 1 )
+while ( x )
 {
-ASSERT(x); /* We can't have hit the end, since our target is
-* still in the chain somehwere... */
 if ( next_shadow(x) == sp )
 {
 x->next_shadow = sp->next_shadow;
@@ -1617,10 +1615,14 @@ void shadow_hash_delete(struct domain *d
 }
 x = next_shadow(x);
 }
+if ( !x )
+return false;
 }
 set_next_shadow(sp, NULL);
 
 sh_hash_audit_bucket(d, key);
+
+return true;
 }
 
 typedef int (*hash_vcpu_callback_t)(struct vcpu *v, mfn_t smfn, mfn_t 
other_mfn);
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -132,7 +132,8 @@ delete_fl1_shadow_status(struct domain *
 SHADOW_PRINTK("gfn=%"SH_PRI_gfn", type=%08x, smfn=%"PRI_mfn"\n",
gfn_x(gfn), SH_type_fl1_shadow, mfn_x(smfn));
 ASSERT(mfn_to_page(smfn)->u.sh.head);
-shadow_hash_delete(d, gfn_x(gfn), SH_type_fl1_shadow, smfn);
+if ( !shadow_hash_delete(d, gfn_x(gfn), SH_type_fl1_shadow, smfn) )
+ASSERT_UNREACHABLE();
 }
 
 
--- a/xen/arch/x86/mm/shadow/private.h
+++ b/xen/arch/x86/mm/shadow/private.h
@@ -375,7 +375,7 @@ shadow_size(unsigned int shadow_type)
 mfn_t shadow_hash_lookup(struct domain *d, unsigned long n, unsigned int t);
 void  shadow_hash_insert(struct domain *d,
  unsigned long n, unsigned int t, mfn_t smfn);
-void  shadow_hash_delete(struct domain *d,
+bool  shadow_hash_delete(struct domain *d,
  unsigned long n, unsigned int t, mfn_t smfn);
 
 /* shadow promotion */
@@ -773,18 +773,19 @@ static inline void
 set_shadow_status(struct domain *d, mfn_t gmfn, u32 shadow_type, mfn_t smfn)
 /* Put a shadow into the hash table */
 {
-int res;
-
 SHADOW_PRINTK("d%d gmfn=%lx, type=%08x, smfn=%lx\n",
   d->domain_id, mfn_x(gmfn), shadow_type, mfn_x(smfn));
 
 ASSERT(mfn_to_page(smfn)->u.sh.head);
 
 /* 32-bit PV guests don't own their l4 pages so can't get_page them */
-if ( !is_pv_32bit_domain(d) || shadow_type != SH_type_l4_64_shadow )
+if ( (shadow_type != SH_type_l4_64_shadow || !is_pv_32bit_domain(d)) &&
+ !get_page(mfn_to_page(gmfn), d) )
 {
-res = get_page(mfn_to_page(gmfn), d);
-ASSERT(res == 1);
+printk(XENLOG_G_ERR "%pd: cannot get page for MFN %" PRI_mfn "\n",
+   d, mfn_x(gmfn));
+domain_crash(d);
+return;
 }
 
 shadow_hash_insert(d, mfn_x(gmfn), shadow_type, smfn);
@@ -797,9 +798,9 @@ delete_shadow_status(struct domain *d, m
 SHADOW_PRINTK("d%d gmfn=%"PRI_mfn", type=%08x, smfn=%"PRI_mfn"\n",
   d->domain_id, mfn_x(gmfn), shadow_type, mfn_x(smfn));
 ASSERT(mfn_to_page(smfn)->u.sh.head);
-shadow_hash_delete(d, mfn_x(gmfn), shadow_type, smfn);
-/* 32-bit PV guests don't own their l4 pages; see set_shadow_status */
-if ( !is_pv_32bit_domain(d) || shadow_type != SH_type_l4_64_shadow )
+if (

[PATCH 1/8] x86/shadow: drop shadow_prepare_page_type_change()'s 3rd parameter

2022-07-26 Thread Jan Beulich

As of 8cc5036bc385 ("x86/pv: Fix ABAC cmpxchg() race in
_get_page_type()") this no longer needs passing separately - the type
can now be read from struct page_info, as the call now happens after its
writing.

While there also constify the 2nd parameter.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/include/asm/shadow.h
+++ b/xen/arch/x86/include/asm/shadow.h
@@ -84,8 +84,8 @@ void shadow_final_teardown(struct domain
 void sh_remove_shadows(struct domain *d, mfn_t gmfn, int fast, int all);
 
 /* Adjust shadows ready for a guest page to change its type. */
-void shadow_prepare_page_type_change(struct domain *d, struct page_info *page,
- unsigned long new_type);
+void shadow_prepare_page_type_change(struct domain *d,
+ const struct page_info *page);
 
 /* Discard _all_ mappings from the domain's shadows. */
 void shadow_blow_tables_per_domain(struct domain *d);
@@ -113,8 +113,7 @@ static inline void sh_remove_shadows(str
  int fast, int all) {}
 
 static inline void shadow_prepare_page_type_change(struct domain *d,
-   struct page_info *page,
-   unsigned long new_type) {}
+   const struct page_info 
*page) {}
 
 static inline void shadow_blow_tables_per_domain(struct domain *d) {}
 
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3018,7 +3018,7 @@ static int _get_page_type(struct page_in
 struct domain *d = page_get_owner(page);
 
 if ( d && shadow_mode_enabled(d) )
-shadow_prepare_page_type_change(d, page, type);
+shadow_prepare_page_type_change(d, page);
 
 if ( (x & PGT_type_mask) != type )
 {
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -2265,8 +2265,8 @@ void sh_remove_shadows(struct domain *d,
 paging_unlock(d);
 }
 
-void shadow_prepare_page_type_change(struct domain *d, struct page_info *page,
- unsigned long new_type)
+void shadow_prepare_page_type_change(struct domain *d,
+ const struct page_info *page)
 {
 if ( !(page->count_info & PGC_page_table) )
 return;
@@ -2278,7 +2278,7 @@ void shadow_prepare_page_type_change(str
  * pages are allowed to become writeable.
  */
 if ( (page->shadow_flags & SHF_oos_may_write) &&
- new_type == PGT_writable_page )
+ (page->u.inuse.type_info & PGT_type_mask) == PGT_writable_page )
 return;
 #endif

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

2022-07-26 Thread osstest service owner

flight 171864 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/171864/

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  a9949efb288fd6e21bbaf9d5826207c7c41cda27
baseline version:
 xen  5707470bf3103ebae43697a7ac2faced6cd35f92

Last test of basis   171861  2022-07-26 07:01:41 Z0 days
Testing same since   171864  2022-07-26 13:01:53 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 

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
   5707470bf3..a9949efb28  a9949efb288fd6e21bbaf9d5826207c7c41cda27 -> smoke

[PATCH 0/8] x86: XSA-40{1,2,8} follow-up

2022-07-26 Thread Jan Beulich

Perhaps not entirely unexpected the work there has turned up a few
further items which want dealing with. Most if not all of these
aren't interdependent, so could probably be looked at (and go in)
in (about) any order.

There's one more change I'd like to make, but I didn't get around
to making (trying to make) yet: PGC_page_table being a shadow-only
concept, it can apparently simply be 0 when !SHADOW_PAGING.

1: shadow: drop shadow_prepare_page_type_change()'s 3rd parameter
2: shadow: properly handle get_page() failing
3: mm: enforce return value checking on get_page()
4: shadow: exclude HVM-only code from sh_remove_shadows() when !HVM
5: shadow: don't open-code shadow_remove_all_shadows()
6: shadow: drop CONFIG_HVM conditionals from sh_update_cr3()
7: mm: adjust type check around _get_page_type()'s TLB flush
8: mm: re-arrange type check around _get_page_type()'s TLB flush

Jan

Re: [PATCH v2] x86/msr: fix X2APIC_LAST

2022-07-26 Thread Jan Beulich

On 26.07.2022 17:42, Andrew Cooper wrote:
> On 26/07/2022 16:33, Jan Beulich wrote:
>> On 26.07.2022 17:28, Edwin Török wrote:
>>> The latest Intel manual now says the X2APIC reserved range is only
>>> 0x800 to 0x8ff (NOT 0xbff).
>>> This changed between SDM 68 (Nov 2018) and SDM 69 (Jan 2019).
>>> The AMD manual documents 0x800-0x8ff too.
>>>
>>> There are non-X2APIC MSRs in the 0x900-0xbff range now:
>>> e.g. 0x981 is IA32_TME_CAPABILITY, an architectural MSR.
>>>
>>> The new MSR in this range appears to have been introduced in Icelake,
>>> so this commit should be backported to Xen versions supporting Icelake.
>>>
>>> Backport: 4.13+
>> FAOD nevertheless it'll be applied only back to 4.15.
> 
> It shouldn't go back before 4.16, because otherwise we start exposing a
> bunch of MSRs (including undocumented ones on Haswell/Broadwell) which
> were previously disallowed.

Hmm, I'm confused - how would the limiting of this range cause more
MSRs to be exposed in 4.15?

Jan

Re: [PATCH v2] x86/msr: fix X2APIC_LAST

2022-07-26 Thread Jan Beulich

On 26.07.2022 17:28, Edwin Török wrote:
> The latest Intel manual now says the X2APIC reserved range is only
> 0x800 to 0x8ff (NOT 0xbff).
> This changed between SDM 68 (Nov 2018) and SDM 69 (Jan 2019).
> The AMD manual documents 0x800-0x8ff too.
> 
> There are non-X2APIC MSRs in the 0x900-0xbff range now:
> e.g. 0x981 is IA32_TME_CAPABILITY, an architectural MSR.
> 
> The new MSR in this range appears to have been introduced in Icelake,
> so this commit should be backported to Xen versions supporting Icelake.
> 
> Backport: 4.13+

FAOD nevertheless it'll be applied only back to 4.15.

> Signed-off-by: Edwin Török 

Reviewed-by: Jan Beulich

Re: [PATCH V7 07/11] vpci/header: emulate PCI_COMMAND register for guests

2022-07-26 Thread Jan Beulich

On 19.07.2022 19:42, Oleksandr Tyshchenko wrote:
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -443,11 +443,27 @@ static int modify_bars(const struct pci_dev *pdev, 
> uint16_t cmd, bool rom_only)
>  return 0;
>  }
>  
> +/* TODO: Add proper emulation for all bits of the command register. */
>  static void cf_check cmd_write(
>  const struct pci_dev *pdev, unsigned int reg, uint32_t cmd, void *data)
>  {
>  uint16_t current_cmd = pci_conf_read16(pdev->sbdf, reg);
>  
> +if ( !is_hardware_domain(pdev->domain) )
> +{
> +struct vpci_header *header = data;
> +
> +header->guest_cmd = cmd;
> +#ifdef CONFIG_HAS_PCI_MSI
> +if ( pdev->vpci->msi->enabled || pdev->vpci->msix->enabled )
> +/*
> + * Guest wants to enable INTx, but it can't be enabled
> + * if MSI/MSI-X enabled.
> + */
> +cmd |= PCI_COMMAND_INTX_DISABLE;
> +#endif
> +}
> +
>  /*
>   * Let Dom0 play with all the bits directly except for the memory
>   * decoding one.
> @@ -464,6 +480,19 @@ static void cf_check cmd_write(
>  pci_conf_write16(pdev->sbdf, reg, cmd);
>  }
>  
> +static uint32_t cf_check cmd_read(
> +const struct pci_dev *pdev, unsigned int reg, void *data)
> +{
> +if ( !is_hardware_domain(pdev->domain) )
> +{
> +struct vpci_header *header = data;
> +
> +return header->guest_cmd;
> +}
> +
> +return pci_conf_read16(pdev->sbdf, reg);
> +}

This function wants the same leading comment as cmd_write(). I also
think you better wouldn't give the guest the impression that r/o bits
can actually be written to (but getting this right may well fall
under the TODO).

Jan

[PATCH v2] x86/msr: fix X2APIC_LAST

2022-07-26 Thread Edwin Török

The latest Intel manual now says the X2APIC reserved range is only
0x800 to 0x8ff (NOT 0xbff).
This changed between SDM 68 (Nov 2018) and SDM 69 (Jan 2019).
The AMD manual documents 0x800-0x8ff too.

There are non-X2APIC MSRs in the 0x900-0xbff range now:
e.g. 0x981 is IA32_TME_CAPABILITY, an architectural MSR.

The new MSR in this range appears to have been introduced in Icelake,
so this commit should be backported to Xen versions supporting Icelake.

Backport: 4.13+

Signed-off-by: Edwin Török 
---

Notes:
Changed since v1:
* include version of Intel SDM where the change occured
* remove opencoded MSR_X2APIC_FIRST + 0xff

 xen/arch/x86/hvm/vmx/vmx.c   | 4 ++--
 xen/arch/x86/include/asm/msr-index.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 47554cc004..17e103188a 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3397,7 +3397,7 @@ void vmx_vlapic_msr_changed(struct vcpu *v)
 if ( cpu_has_vmx_apic_reg_virt )
 {
 for ( msr = MSR_X2APIC_FIRST;
-  msr <= MSR_X2APIC_FIRST + 0xff; msr++ )
+  msr <= MSR_X2APIC_LAST; msr++ )
 vmx_clear_msr_intercept(v, msr, VMX_MSR_R);
 
 vmx_set_msr_intercept(v, MSR_X2APIC_PPR, VMX_MSR_R);
@@ -3418,7 +3418,7 @@ void vmx_vlapic_msr_changed(struct vcpu *v)
 if ( !(v->arch.hvm.vmx.secondary_exec_control &
SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE) )
 for ( msr = MSR_X2APIC_FIRST;
-  msr <= MSR_X2APIC_FIRST + 0xff; msr++ )
+  msr <= MSR_X2APIC_LAST; msr++ )
 vmx_set_msr_intercept(v, msr, VMX_MSR_RW);
 
 vmx_update_secondary_exec_control(v);
diff --git a/xen/arch/x86/include/asm/msr-index.h 
b/xen/arch/x86/include/asm/msr-index.h
index 8cab8736d8..1a928ea6af 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -148,7 +148,7 @@
 #define MSR_INTERRUPT_SSP_TABLE 0x06a8
 
 #define MSR_X2APIC_FIRST0x0800
-#define MSR_X2APIC_LAST 0x0bff
+#define MSR_X2APIC_LAST 0x08ff
 
 #define MSR_X2APIC_TPR  0x0808
 #define MSR_X2APIC_PPR  0x080a
-- 
2.34.1

Re: [PATCH V7 08/11] vpci/header: reset the command register when adding devices

2022-07-26 Thread Jan Beulich

On 19.07.2022 19:42, Oleksandr Tyshchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Reset the command register when assigning a PCI device to a guest:
> according to the PCI spec the PCI_COMMAND register is typically all 0's
> after reset, but this might not be true for the guest as it needs
> to respect host's settings.
> For that reason, do not write 0 to the PCI_COMMAND register directly,
> but go through the corresponding emulation layer (cmd_write), which
> will take care about the actual bits written.
> 
> Signed-off-by: Oleksandr Andrushchenko 
> ---
> Since v6:
> - use cmd_write directly without introducing emulate_cmd_reg
> - update commit message with more description on all 0's in PCI_COMMAND

I agree with the change, but it's imo enough that you also need to sign
off on the patch (and this likely also applies elsewhere in the series).

Jan

Re: [PATCH V7 00/11] PCI devices passthrough on Arm, part 3

2022-07-26 Thread Oleksandr Tyshchenko


On 26.07.22 16:47, Rahul Singh wrote:
> Hi Oleksandr,


Hello Rahul



>
>> On 19 Jul 2022, at 6:42 pm, Oleksandr Tyshchenko  wrote:
>>
>> From: Oleksandr Tyshchenko 
>>
>> Hi, all!
>>
>> You can find previous discussion at [1].
>>
>> 1. This patch series is focusing on vPCI and adds support for non-identity
>> PCI BAR mappings which is required while passing through a PCI device to
>> a guest. The highlights are:
>>
>> - Add relevant vpci register handlers when assigning PCI device to a domain
>>   and remove those when de-assigning. This allows having different
>>   handlers for different domains, e.g. hwdom and other guests.
>>
>> - Emulate guest BAR register values based on physical BAR values.
>>   This allows creating a guest view of the registers and emulates
>>   size and properties probe as it is done during PCI device enumeration by
>>   the guest.
>>
>> - Instead of handling a single range set, that contains all the memory
>>   regions of all the BARs and ROM, have them per BAR.
>>
>> - Take into account guest's BAR view and program its p2m accordingly:
>>   gfn is guest's view of the BAR and mfn is the physical BAR value as set
>>   up by the host bridge in the hardware domain.
>>   This way hardware domain sees physical BAR values and guest sees
>>   emulated ones.
>>
>> 2. The series also adds support for virtual PCI bus topology for guests:
>> - We emulate a single host bridge for the guest, so segment is always 0.
>> - The implementation is limited to 32 devices which are allowed on
>>a single PCI bus.
>> - The virtual bus number is set to 0, so virtual devices are seen
>>as embedded endpoints behind the root complex.
>>
>> 3. The series has been updated due to the new PCI(vPCI) locking scheme 
>> implemented
>> in the prereq series which is also on the review now [2].
>>
>> 4. For unprivileged guests vpci_{read|write} has been re-worked
>> to not passthrough accesses to the registers not explicitly handled
>> by the corresponding vPCI handlers: without that passthrough
>> to guests is completely unsafe as Xen allows them full access to
>> the registers. During development this can be reverted for debugging 
>> purposes.
>>
>> !!! OT: please note, Oleksandr Andrushchenko who is the author of all this 
>> stuff
>> has managed to address allmost all review comments given for v6 and pushed 
>> the updated
>> version to the github (23.02.22).
>> So after receiving his agreement I just picked it up and did the following 
>> before
>> pushing V7:
>> - rebased on the recent staging (resolving a few conflicts)
>> - updated according to the recent changes (added cf_check specifiers where 
>> appropriate, etc)
>> and performed minor adjustments
>> - made sure that both current and prereq series [2] didn't really break x86 
>> by testing
>> PVH Dom0 (vPCI) and PV Dom0 + HVM DomU (PCI passthrough to DomU) using Qemu
>> - my colleague Volodymyr Babchuk (who was involved in the prereq series) 
>> rechecked that
>> both series worked on Arm using real HW
>>
>> You can also find the series at [3].
>>
>> [1] 
>> https://urldefense.com/v3/__https://lore.kernel.org/xen-devel/20220204063459.680961-1-andr2...@gmail.com/__;!!GF_29dbcQIUBPA!1P9LeytJC7d3tnSuQCjk7YqIqfZPpGlrc6ES1l1sUAPbfGbeYg2YM477xiUy0oTU9ys7qv9MHD6GNDWCeHHG_qsr-NY$
>>  [lore[.]kernel[.]org]
>> [2] 
>> https://urldefense.com/v3/__https://lore.kernel.org/xen-devel/20220718211521.664729-1-volodymyr_babc...@epam.com/__;!!GF_29dbcQIUBPA!1P9LeytJC7d3tnSuQCjk7YqIqfZPpGlrc6ES1l1sUAPbfGbeYg2YM477xiUy0oTU9ys7qv9MHD6GNDWCeHHGbScTNb4$
>>  [lore[.]kernel[.]org]
>> [3] 
>> https://urldefense.com/v3/__https://github.com/otyshchenko1/xen/commits/vpci7__;!!GF_29dbcQIUBPA!1P9LeytJC7d3tnSuQCjk7YqIqfZPpGlrc6ES1l1sUAPbfGbeYg2YM477xiUy0oTU9ys7qv9MHD6GNDWCeHHGhpAmrcM$
>>  [github[.]com]
>>
> I tested the whole series on ARM N1SDP board everything works as expected.


Sounds great!


>
> So for the whole series:
> Tested-by: Rahul Singh 


Thank you for testing!


>
> Regards,
> Rahul

-- 
Regards,

Oleksandr Tyshchenko

Re: [PATCH V7 10/11] xen/arm: translate virtual PCI bus topology for guests

2022-07-26 Thread Jan Beulich

On 19.07.2022 19:42, Oleksandr Tyshchenko wrote:
> --- a/xen/arch/arm/vpci.c
> +++ b/xen/arch/arm/vpci.c
> @@ -41,6 +41,16 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t 
> *info,
>  /* data is needed to prevent a pointer cast on 32bit */
>  unsigned long data;
>  
> +/*
> + * For the passed through devices we need to map their virtual SBDF
> + * to the physical PCI device being passed through.
> + */
> +if ( !bridge && !vpci_translate_virtual_device(v->domain, ) )
> +{
> +*r = ~0ul;
> +return 1;
> +}

I'm probably simply lacking specific Arm-side knowledge, but it strikes
me as odd that the need for translation would be dependent upon "bridge".

> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -158,6 +158,32 @@ static void vpci_remove_virtual_device(const struct 
> pci_dev *pdev)
>  }
>  }
>  
> +/*
> + * Find the physical device which is mapped to the virtual device
> + * and translate virtual SBDF to the physical one.
> + */
> +bool vpci_translate_virtual_device(struct domain *d, pci_sbdf_t *sbdf)
> +{
> +struct pci_dev *pdev;

const wherever possible please (i.e. likely also for the first function
parameter).

> --- a/xen/include/xen/vpci.h
> +++ b/xen/include/xen/vpci.h
> @@ -276,6 +276,7 @@ static inline bool __must_check 
> vpci_process_pending(struct vcpu *v)
>  /* Notify vPCI that device is assigned/de-assigned to/from guest. */
>  int vpci_assign_device(struct pci_dev *pdev);
>  void vpci_deassign_device(struct pci_dev *pdev);
> +bool vpci_translate_virtual_device(struct domain *d, pci_sbdf_t *sbdf);
>  #else
>  static inline int vpci_assign_device(struct pci_dev *pdev)
>  {
> @@ -285,6 +286,12 @@ static inline int vpci_assign_device(struct pci_dev 
> *pdev)
>  static inline void vpci_deassign_device(struct pci_dev *pdev)
>  {
>  };
> +
> +static inline bool vpci_translate_virtual_device(struct domain *d,
> + pci_sbdf_t *sbdf)
> +{
> +return false;
> +}

Please don't add stubs which aren't really needed (which, afaict, is the
case for this one).

Jan

Re: [PATCH V7 08/11] vpci/header: reset the command register when adding devices

2022-07-26 Thread Rahul Singh

HI Oleksandr,

> On 19 Jul 2022, at 6:42 pm, Oleksandr Tyshchenko  wrote:
> 
> From: Oleksandr Andrushchenko 
> 
> Reset the command register when assigning a PCI device to a guest:
> according to the PCI spec the PCI_COMMAND register is typically all 0's
> after reset, but this might not be true for the guest as it needs
> to respect host's settings.
> For that reason, do not write 0 to the PCI_COMMAND register directly,
> but go through the corresponding emulation layer (cmd_write), which
> will take care about the actual bits written.
> 
> Signed-off-by: Oleksandr Andrushchenko  

Reviewed-by: Rahul Singh 

Regards,
Rahul

Re: [PATCH] x86/msr: fix X2APIC_LAST

2022-07-26 Thread Jan Beulich

On 26.07.2022 16:43, Edwin Török wrote:
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -148,7 +148,7 @@
>  #define MSR_INTERRUPT_SSP_TABLE 0x06a8
>  
>  #define MSR_X2APIC_FIRST0x0800
> -#define MSR_X2APIC_LAST 0x0bff
> +#define MSR_X2APIC_LAST 0x08ff

May I ask that then the now open-coded values of MSR_X2APIC_LAST
(two instances in vmx.c using MSR_X2APIC_FIRST + 0xff) be replaced
at the same time?

Jan

Re: [PATCH] xen/check-endbr.sh: Explain the purpose of the script

2022-07-26 Thread Jan Beulich

On 26.07.2022 16:23, Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich

Re: [PATCH v1 11/18] x86: initial conversion to domain builder

2022-07-26 Thread Jan Beulich

On 06.07.2022 23:04, Daniel P. Smith wrote:
> This commit is the first step in adopting domain builder. It goes through the
> dom0 creation and construction functions, converting them over to consume
> struct boot_domaain and changes the startup sequence to use the domain builder
> to create and construct dom0.
> 
> Signed-off-by: Daniel P. Smith 
> Reviewed-by: Christopher Clark 
> ---
>  xen/arch/x86/dom0_build.c |  30 +++
>  xen/arch/x86/hvm/dom0_build.c |  10 +--
>  xen/arch/x86/include/asm/dom0_build.h |   8 +-
>  xen/arch/x86/include/asm/setup.h  |   5 +-
>  xen/arch/x86/pv/dom0_build.c  |  39 -
>  xen/arch/x86/setup.c  | 114 +++---
>  6 files changed, 109 insertions(+), 97 deletions(-)
> 
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index e44f7f3c43..216c9e3590 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -6,6 +6,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -556,31 +557,32 @@ int __init dom0_setup_permissions(struct domain *d)
>  return rc;
>  }
>  
> -int __init construct_dom0(
> -struct domain *d, const struct boot_module *image,
> -struct boot_module *initrd, char *cmdline)
> +int __init construct_domain(struct boot_domain *bd)
>  {
> -int rc;
> +int rc = 0;
>  
>  /* Sanity! */
> -BUG_ON(!pv_shim && d->domain_id != 0);
> -BUG_ON(d->vcpu[0] == NULL);
> -BUG_ON(d->vcpu[0]->is_initialised);
> +BUG_ON(!pv_shim && bd->domid != 0);
> +BUG_ON(bd->domain->vcpu[0] == NULL);
> +BUG_ON(bd->domain->vcpu[0]->is_initialised);
>  
>  process_pending_softirqs();
>  
> -if ( is_hvm_domain(d) )
> -rc = dom0_construct_pvh(d, image, initrd, cmdline);
> -else if ( is_pv_domain(d) )
> -rc = dom0_construct_pv(d, image, initrd, cmdline);
> -else
> -panic("Cannot construct Dom0. No guest interface available\n");
> +if ( builder_is_initdom(bd) )
> +{
> +if ( is_hvm_domain(bd->domain) )
> +rc = dom0_construct_pvh(bd);
> +else if ( is_pv_domain(bd->domain) )
> +rc = dom0_construct_pv(bd);
> +else
> +panic("Cannot construct Dom0. No guest interface available\n");
> +}

Isn't there an "else" missing here, even if just to ASSERT_UNREACHABLE()
and set rc to, say, -EOPNOTSUPP?

> @@ -311,11 +310,12 @@ int __init dom0_construct_pv(
>  unsigned long count;
>  struct page_info *page = NULL;
>  start_info_t *si;
> +struct domain *d = bd->domain;
>  struct vcpu *v = d->vcpu[0];
> -void *image_base = bootstrap_map(image);
> -unsigned long image_len = image->size;
> -void *image_start = image_base + image->arch->headroom;
> -unsigned long initrd_len = initrd ? initrd->size : 0;
> +void *image_base = bootstrap_map(bd->kernel);
> +unsigned long image_len = bd->kernel->size;
> +void *image_start = image_base + bd->kernel->arch->headroom;
> +unsigned long initrd_len = bd->ramdisk ? bd->ramdisk->size : 0;
>  l4_pgentry_t *l4tab = NULL, *l4start = NULL;
>  l3_pgentry_t *l3tab = NULL, *l3start = NULL;
>  l2_pgentry_t *l2tab = NULL, *l2start = NULL;
> @@ -355,7 +355,7 @@ int __init dom0_construct_pv(
>  d->max_pages = ~0U;
>  
>  if ( (rc =
> -  bzimage_parse(image_base, _start, image->arch->headroom,
> +  bzimage_parse(image_base, _start, bd->kernel->arch->headroom,
>   _len)) != 0 )
>  return rc;
>  
> @@ -545,7 +545,7 @@ int __init dom0_construct_pv(
>  initrd_pfn = vinitrd_start ?
>   (vinitrd_start - v_start) >> PAGE_SHIFT :
>   domain_tot_pages(d);
> -initrd_mfn = mfn = mfn_x(initrd->mfn);
> +initrd_mfn = mfn = mfn_x(bd->ramdisk->mfn);
>  count = PFN_UP(initrd_len);
>  if ( d->arch.physaddr_bitsize &&
>   ((mfn + count - 1) >> (d->arch.physaddr_bitsize - PAGE_SHIFT)) )
> @@ -560,13 +560,13 @@ int __init dom0_construct_pv(
>  free_domheap_pages(page, order);
>  page += 1UL << order;
>  }
> -memcpy(page_to_virt(page), maddr_to_virt(initrd->start),
> +memcpy(page_to_virt(page), maddr_to_virt(bd->ramdisk->start),
> initrd_len);
> -mpt_alloc = initrd->start;
> +mpt_alloc = bd->ramdisk->start;
>  init_domheap_pages(mpt_alloc,
> mpt_alloc + PAGE_ALIGN(initrd_len));
> -bootmodule_update_mfn(initrd, page_to_mfn(page));
> -initrd_mfn = mfn_x(initrd->mfn);
> +bootmodule_update_mfn(bd->ramdisk, page_to_mfn(page));
> +initrd_mfn = mfn_x(bd->ramdisk->mfn);
>  }
>  else
>  {
> @@ -574,7 +574,7 @@ int __init dom0_construct_pv(
>  if (

Re: [PATCH V7 11/11] xen/arm: account IO handlers for emulated PCI MSI-X

2022-07-26 Thread Rahul Singh

Hi Oleksandr,

> On 19 Jul 2022, at 6:42 pm, Oleksandr Tyshchenko  wrote:
> 
> From: Oleksandr Andrushchenko 
> 
> At the moment, we always allocate an extra 16 slots for IO handlers
> (see MAX_IO_HANDLER). So while adding IO trap handlers for the emulated
> MSI-X registers we need to explicitly tell that we have additional IO
> handlers, so those are accounted.
> 
> Signed-off-by: Oleksandr Andrushchenko 
> Acked-by: Julien Grall 

Reviewed-by: Rahul Singh 

Regards,
Rahul

Re: [PATCH V7 04/11] rangeset: add RANGESETF_no_print flag

2022-07-26 Thread Rahul Singh

Hi Oleksandr,

> On 19 Jul 2022, at 6:42 pm, Oleksandr Tyshchenko  wrote:
> 
> From: Oleksandr Andrushchenko 
> 
> There are range sets which should not be printed, so introduce a flag
> which allows marking those as such. Implement relevant logic to skip
> such entries while printing.
> 
> While at it also simplify the definition of the flags by directly
> defining those without helpers.
> 
> Suggested-by: Jan Beulich 
> Signed-off-by: Oleksandr Andrushchenko 
> Reviewed-by: Jan Beulich 

Reviewed-by: Rahul Singh 
 
Regards,
Rahul

Re: [PATCH v1 10/18] x86: introduce the domain builder

2022-07-26 Thread Jan Beulich

On 06.07.2022 23:04, Daniel P. Smith wrote:
> This commit introduces the domain builder configuration FDT parser along with
> the domain builder core for domain creation. To enable domain builder to be a
> cross architecture internal API, a new arch domain creation call is introduced
> for use by the domain builder.
> 
> Signed-off-by: Daniel P. Smith 
> Reviewed-by: Christopher Clark 
> ---
>  xen/arch/x86/setup.c   |   9 +
>  xen/common/Makefile|   1 +
>  xen/common/domain-builder/Makefile |   2 +
>  xen/common/domain-builder/core.c   |  96 ++
>  xen/common/domain-builder/fdt.c| 295 +
>  xen/common/domain-builder/fdt.h|   7 +
>  xen/include/xen/bootinfo.h |  16 ++
>  xen/include/xen/domain_builder.h   |   1 +
>  8 files changed, 427 insertions(+)

With this diffstat - why the x86: prefix in the subject?

Also note the naming inconsistency: domain-builder/ (preferred) vs
domain_builder.h (adjustment would require touching earlier patches).

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1,4 +1,6 @@
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -826,6 +828,13 @@ static struct domain *__init create_dom0(const struct 
> boot_info *bi)
>  return d;
>  }
>  
> +void __init arch_create_dom(
> +const struct boot_info *bi, struct boot_domain *bd)
> +{
> +if ( builder_is_initdom(bd) )
> +create_dom0(bi);
> +}

You're not removing any code in exchange - is Dom0 now being built twice?
Or is the function above effectively dead code?

> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -72,6 +72,7 @@ extra-y := symbols-dummy.o
>  obj-$(CONFIG_COVERAGE) += coverage/
>  obj-y += sched/
>  obj-$(CONFIG_UBSAN) += ubsan/
> +obj-y += domain-builder/

At least as long as all of this is still experimental I would really like
to see a way to disable all of it via Kconfig.

> --- /dev/null
> +++ b/xen/common/domain-builder/core.c
> @@ -0,0 +1,96 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include "fdt.h"
> +
> +static struct domain_builder __initdata builder;
> +
> +void __init builder_init(struct boot_info *info)
> +{
> +struct boot_domain *d = NULL;
> +
> +info->builder = 
> +
> +if ( IS_ENABLED(CONFIG_BUILDER_FDT) )
> +{
> +/* fdt is required to be module 0 */
> +switch ( check_fdt(info, __va(info->mods[0].start)) )

Besides requiring fixed order looking inflexible to me, what guarantees
there is at least one module? (Perhaps there is, but once again -
without seeing where this function is being called from, how am I to
judge?)

> +{
> +case 0:
> +printk("Domain Builder: initialized from config\n");
> +info->builder->fdt_enabled = true;
> +return;
> +case -EINVAL:
> +info->builder->fdt_enabled = false;
> +break;

Aiui this is the case where no FDT is present. I'd strongly suggest
to use a less common / ambiguous error code to cover that case. Maybe
-ENODEV or -EOPNOTSUPP or ...

> +case -ENODATA:

... -ENODATA, albeit you having that here suggests this has some
other specific meaning already.

> +default:
> +panic("%s: error occured processing DTB\n", __func__);
> +}
> +}
> +
> +/*
> + * No FDT config support or an FDT wasn't present, do an initial
> + * domain construction
> + */
> +printk("Domain Builder: falling back to initial domain build\n");
> +info->builder->nr_doms = 1;
> +d = >builder->domains[0];
> +
> +d->mode = opt_dom0_pvh ? 0 : BUILD_MODE_PARAVIRTUALIZED;
> +
> +d->kernel = >mods[0];
> +d->kernel->kind = BOOTMOD_KERNEL;
> +
> +d->permissions = BUILD_PERMISSION_CONTROL | BUILD_PERMISSION_HARDWARE;
> +d->functions = BUILD_FUNCTION_CONSOLE | BUILD_FUNCTION_XENSTORE |
> + BUILD_FUNCTION_INITIAL_DOM;

Nit: Indentation.

> +d->kernel->arch->headroom = bzimage_headroom(bootstrap_map(d->kernel),
> +   d->kernel->size);

bzimage isn't an arch-agnostic concept afaict, so I don't see this
function legitimately being called from here.

And nit again: Indentation. (And at least one more further down.)

> +bootstrap_map(NULL);
> +
> +if ( d->kernel->string.len )
> +d->kernel->string.kind = BOOTSTR_CMDLINE;
> +}
> +
> +uint32_t __init builder_create_domains(struct boot_info *info)
> +{
> +uint32_t build_count = 0, functions_built = 0;
> +int i;
> +
> +for ( i = 0; i < info->builder->nr_doms; i++ )
> +{
> +struct boot_domain *d = >builder->domains[i];

Can variables of this type please not be named "d", but e.g. "bd"?

> +if ( ! IS_ENABLED(CONFIG_MULTIDOM_BUILDER) &&
> + ! builder_is_initdom(d) &&

Nit: Stray blanks after ! .

> --- /dev/null
> +++

[PATCH] x86/msr: fix X2APIC_LAST

2022-07-26 Thread Edwin Török

The latest Intel manual now says the X2APIC reserved range is only
0x800 to 0x8ff (NOT 0xbff). The AMD manual documents 0x800-0x8ff too.

There are non-X2APIC MSRs in the 0x900-0xbff range now:
e.g. 0x981 is IA32_TME_CAPABILITY, an architectural MSR.

The new MSR in this range appears to have been introduced in Icelake,
so this commit should be backported to Xen versions supporting Icelake.

Backport: 4.13+

Signed-off-by: Edwin Török 
---
 xen/arch/x86/include/asm/msr-index.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/msr-index.h 
b/xen/arch/x86/include/asm/msr-index.h
index 8cab8736d8..1a928ea6af 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -148,7 +148,7 @@
 #define MSR_INTERRUPT_SSP_TABLE 0x06a8
 
 #define MSR_X2APIC_FIRST0x0800
-#define MSR_X2APIC_LAST 0x0bff
+#define MSR_X2APIC_LAST 0x08ff
 
 #define MSR_X2APIC_TPR  0x0808
 #define MSR_X2APIC_PPR  0x080a
-- 
2.34.1

[PATCH] xen/check-endbr.sh: Explain the purpose of the script

2022-07-26 Thread Andrew Cooper

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
---
 xen/tools/check-endbr.sh | 9 +
 1 file changed, 9 insertions(+)

diff --git a/xen/tools/check-endbr.sh b/xen/tools/check-endbr.sh
index b97684ac25e9..bf153a570db4 100755
--- a/xen/tools/check-endbr.sh
+++ b/xen/tools/check-endbr.sh
@@ -2,6 +2,15 @@
 #
 # Usage ./$0 xen-syms
 #
+# When CET-IBT (Control-flow Enforcement Technology, Indirect Branch Tracking)
+# is active, ENDBR instructions mark legal indirect branch targets in the
+# .text section.
+#
+# However x86 is a variable length instruction set so the same byte pattern
+# can exist embedded in other instructions, or crossing multiple instructions.
+# This script searches .text for any problematic byte patterns which aren't
+# legitimate ENDBR instructions.
+#
 set -e
 
 # Pretty-print parameters a little for message
-- 
2.11.0

Re: [PATCH v1 09/18] x86: introduce abstractions for domain builder

2022-07-26 Thread Jan Beulich

On 06.07.2022 23:04, Daniel P. Smith wrote:
> --- /dev/null
> +++ b/xen/arch/x86/include/asm/bootdomain.h
> @@ -0,0 +1,30 @@
> +#ifndef __ARCH_X86_BOOTDOMAIN_H__
> +#define __ARCH_X86_BOOTDOMAIN_H__
> +
> +struct memsize {
> +long nr_pages;
> +unsigned int percent;
> +bool minus;
> +};
> +
> +static inline bool memsize_gt_zero(const struct memsize *sz)
> +{
> +return !sz->minus && sz->nr_pages;
> +}
> +
> +static inline unsigned long get_memsize(
> +const struct memsize *sz, unsigned long avail)
> +{
> +unsigned long pages;
> +
> +pages = sz->nr_pages + sz->percent * avail / 100;
> +return sz->minus ? avail - pages : pages;
> +}

For both functions I think you should retain the __init, just in case
the compiler decides against actually inlining them (according to my
observations Clang frequently won't).

> +struct arch_domain_mem {
> +struct memsize mem_size;
> +struct memsize mem_min;
> +struct memsize mem_max;
> +};

How come this is introduced here without the three respective Dom0
variables being replaced by an instance of this struct? At which
point a further question would be: What about dom0_mem_set?

> --- /dev/null
> +++ b/xen/include/xen/bootdomain.h
> @@ -0,0 +1,52 @@
> +#ifndef __XEN_BOOTDOMAIN_H__
> +#define __XEN_BOOTDOMAIN_H__
> +
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +struct domain;

Why the forward decl? There's no function being declared here, and
this is not C++.

> +struct boot_domain {
> +#define BUILD_PERMISSION_NONE  (0)
> +#define BUILD_PERMISSION_CONTROL   (1 << 0)
> +#define BUILD_PERMISSION_HARDWARE  (1 << 1)
> +uint32_t permissions;

Why a fixed width type? And why no 'u' suffixes on the 1s being left
shifted above? (Same further down from here.)

> +#define BUILD_FUNCTION_NONE(0)
> +#define BUILD_FUNCTION_BOOT(1 << 0)
> +#define BUILD_FUNCTION_CRASH   (1 << 1)
> +#define BUILD_FUNCTION_CONSOLE (1 << 2)
> +#define BUILD_FUNCTION_STUBDOM (1 << 3)
> +#define BUILD_FUNCTION_XENSTORE(1 << 30)
> +#define BUILD_FUNCTION_INITIAL_DOM (1 << 31)
> +uint32_t functions;
> +/* On | Off*/
> +#define BUILD_MODE_PARAVIRTUALIZED (1 << 0) /* PV | PVH/HVM */
> +#define BUILD_MODE_ENABLE_DEVICE_MODEL (1 << 1) /* HVM| PVH */
> +#define BUILD_MODE_LONG(1 << 2) /* 64 BIT | 32 BIT  */

I guess bitness would better not be a boolean-like value (and "LONG"
is kind of odd anyway) - see RISC-V having provisions right away for
128-bit mode.

> --- /dev/null
> +++ b/xen/include/xen/domain_builder.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef XEN_DOMAIN_BUILDER_H
> +#define XEN_DOMAIN_BUILDER_H
> +
> +#include 
> +#include 
> +
> +#include 
> +
> +struct domain_builder {
> +bool fdt_enabled;
> +#define BUILD_MAX_BOOT_DOMAINS 64
> +uint16_t nr_doms;
> +struct boot_domain domains[BUILD_MAX_BOOT_DOMAINS];
> +
> +struct arch_domain_builder *arch;
> +};
> +
> +static inline bool builder_is_initdom(struct boot_domain *bd)

const wherever possible, please.

> +{
> +return bd->functions & BUILD_FUNCTION_INITIAL_DOM;
> +}
> +
> +static inline bool builder_is_ctldom(struct boot_domain *bd)
> +{
> +return (bd->functions & BUILD_FUNCTION_INITIAL_DOM ||
> +bd->permissions & BUILD_PERMISSION_CONTROL );

Please parenthesize the operands of &, |, or ^ inside && or ||.

> +}
> +
> +static inline bool builder_is_hwdom(struct boot_domain *bd)
> +{
> +return (bd->functions & BUILD_FUNCTION_INITIAL_DOM ||
> +bd->permissions & BUILD_PERMISSION_HARDWARE );
> +}
> +
> +static inline struct domain *builder_get_hwdom(struct boot_info *info)
> +{
> +int i;

unsigned int please when the value can't go negative.

> +for ( i = 0; i < info->builder->nr_doms; i++ )
> +{
> +struct boot_domain *d = >builder->domains[i];
> +
> +if ( builder_is_hwdom(d) )
> +return d->domain;
> +}
> +
> +return NULL;
> +}
> +
> +void builder_init(struct boot_info *info);
> +uint32_t builder_create_domains(struct boot_info *info);

Both for these and for the inline functions - how is one to judge they
are (a) needed and (b) fit their purpose without seeing even a single
caller. And for the prototypes not even the implementation is there:
What's wrong with adding those at the time they're actually implemented
(and hopefully also used)?

Jan

Re: [PATCH] x86/tboot: Drop mfn_in_guarded_stack()

2022-07-26 Thread Jan Beulich

On 26.07.2022 14:25, Andrew Cooper wrote:
> To support CET Shadow Stacks, guard pages changed from being holes to being
> read-only.  As such, they can be read.
> 
> Moreover, they should be included in the integrity check.

As long as they're non-present mappings, I don't think they should be
included here, so - not being a native speaker - I'm not sure about
"moreover".

> Fixes: 60016604739b ("x86/shstk: Rework the stack layout to support shadow 
> stacks")
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 

You should have included the three TXT reviewers: I would have been
curious who, if anyone, would actually have responded.

Jan

Re: [PATCH] x86/vpmu: Fix build following vmfork addition

2022-07-26 Thread Jan Beulich

On 26.07.2022 15:13, Andrew Cooper wrote:
> GCC complains:
> 
>   arch/x86/cpu/vpmu.c:351:15: error: conflicting types for 'vpmu_save_force'; 
> have 'void(void *)' with implied 'nocf_check' attribute
> 351 | void cf_check vpmu_save_force(void *arg)
> |   ^~~
>   In file included from ./arch/x86/include/asm/domain.h:10,
>from ./include/xen/domain.h:8,
>from ./include/xen/sched.h:11,
>from ./include/xen/event.h:12,
>from arch/x86/cpu/vpmu.c:23:
>   ./arch/x86/include/asm/vpmu.h:117:6: note: previous declaration of 
> 'vpmu_save_force' with type 'void(void *)'
> 117 | void vpmu_save_force(void *arg);
> |  ^~~
> 
> Adjust the declaraion.
> 
> Fixes: 755087eb9b10 ("xen/mem_sharing: support forks with active vPMU state")
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 

Maybe worth mentioning that is is a non-default gcc?

Jan

Re: [PATCH V7 00/11] PCI devices passthrough on Arm, part 3

2022-07-26 Thread Rahul Singh

Hi Oleksandr,

> On 19 Jul 2022, at 6:42 pm, Oleksandr Tyshchenko  wrote:
> 
> From: Oleksandr Tyshchenko 
> 
> Hi, all!
> 
> You can find previous discussion at [1].
> 
> 1. This patch series is focusing on vPCI and adds support for non-identity
> PCI BAR mappings which is required while passing through a PCI device to
> a guest. The highlights are:
> 
> - Add relevant vpci register handlers when assigning PCI device to a domain
>  and remove those when de-assigning. This allows having different
>  handlers for different domains, e.g. hwdom and other guests.
> 
> - Emulate guest BAR register values based on physical BAR values.
>  This allows creating a guest view of the registers and emulates
>  size and properties probe as it is done during PCI device enumeration by
>  the guest.
> 
> - Instead of handling a single range set, that contains all the memory
>  regions of all the BARs and ROM, have them per BAR.
> 
> - Take into account guest's BAR view and program its p2m accordingly:
>  gfn is guest's view of the BAR and mfn is the physical BAR value as set
>  up by the host bridge in the hardware domain.
>  This way hardware domain sees physical BAR values and guest sees
>  emulated ones.
> 
> 2. The series also adds support for virtual PCI bus topology for guests:
> - We emulate a single host bridge for the guest, so segment is always 0.
> - The implementation is limited to 32 devices which are allowed on
>   a single PCI bus.
> - The virtual bus number is set to 0, so virtual devices are seen
>   as embedded endpoints behind the root complex.
> 
> 3. The series has been updated due to the new PCI(vPCI) locking scheme 
> implemented
> in the prereq series which is also on the review now [2].
> 
> 4. For unprivileged guests vpci_{read|write} has been re-worked
> to not passthrough accesses to the registers not explicitly handled
> by the corresponding vPCI handlers: without that passthrough
> to guests is completely unsafe as Xen allows them full access to
> the registers. During development this can be reverted for debugging purposes.
> 
> !!! OT: please note, Oleksandr Andrushchenko who is the author of all this 
> stuff
> has managed to address allmost all review comments given for v6 and pushed 
> the updated
> version to the github (23.02.22). 
> So after receiving his agreement I just picked it up and did the following 
> before
> pushing V7:
> - rebased on the recent staging (resolving a few conflicts)
> - updated according to the recent changes (added cf_check specifiers where 
> appropriate, etc)
> and performed minor adjustments
> - made sure that both current and prereq series [2] didn't really break x86 
> by testing
> PVH Dom0 (vPCI) and PV Dom0 + HVM DomU (PCI passthrough to DomU) using Qemu
> - my colleague Volodymyr Babchuk (who was involved in the prereq series) 
> rechecked that
> both series worked on Arm using real HW
> 
> You can also find the series at [3].
> 
> [1] 
> https://lore.kernel.org/xen-devel/20220204063459.680961-1-andr2...@gmail.com/
> [2] 
> https://lore.kernel.org/xen-devel/20220718211521.664729-1-volodymyr_babc...@epam.com/
> [3] https://github.com/otyshchenko1/xen/commits/vpci7
> 

I tested the whole series on ARM N1SDP board everything works as expected.

So for the whole series:
Tested-by: Rahul Singh 

Regards,
Rahul

Re: [PATCH v2] x86/xen: Add support for HVMOP_set_evtchn_upcall_vector

2022-07-26 Thread Jane Malalane

On 26/07/2022 13:56, Jane Malalane wrote:
> Implement support for the HVMOP_set_evtchn_upcall_vector hypercall in
> order to set the per-vCPU event channel vector callback on Linux and
> use it in preference of HVM_PARAM_CALLBACK_IRQ.
> 
> If the per-VCPU vector setup is successful on BSP, use this method
> for the APs. If not, fallback to the global vector-type callback.
> 
> Also register callback_irq at per-vCPU event channel setup to trick
> toolstack to think the domain is enlightened.
> 
> Suggested-by: "Roger Pau Monné" 
> Signed-off-by: Jane Malalane 
> ---
> CC: Juergen Gross 
> CC: Boris Ostrovsky 
> CC: Thomas Gleixner 
> CC: Ingo Molnar 
> CC: Borislav Petkov 
> CC: Dave Hansen 
> CC: x...@kernel.org
> CC: "H. Peter Anvin" 
> CC: Stefano Stabellini 
> CC: Oleksandr Tyshchenko 
> CC: Jane Malalane 
> CC: Maximilian Heyne 
> CC: Jan Beulich 
> CC: Colin Ian King 
> CC: xen-devel@lists.xenproject.org
> 
> v2:
>   * remove no_vector_callback
>   * make xen_have_vector_callback a bool
>   * rename xen_ack_upcall to xen_percpu_upcall
>   * fail to bring CPU up on init instead of crashing kernel
>   * add and use xen_set_upcall_vector where suitable
>   * xen_setup_upcall_vector -> xen_init_setup_upcall_vector for clarity
> ---
>   arch/x86/include/asm/xen/cpuid.h   |  2 ++
>   arch/x86/include/asm/xen/events.h  |  3 ++-
>   arch/x86/xen/enlighten.c   |  2 +-
>   arch/x86/xen/enlighten_hvm.c   | 24 ++-
>   arch/x86/xen/suspend_hvm.c | 10 +++-
>   drivers/xen/events/events_base.c   | 49 
> ++
>   include/xen/hvm.h  |  2 ++
>   include/xen/interface/hvm/hvm_op.h | 15 
>   8 files changed, 93 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/include/asm/xen/cpuid.h 
> b/arch/x86/include/asm/xen/cpuid.h
> index 78e667a31d6c..6daa9b0c8d11 100644
> --- a/arch/x86/include/asm/xen/cpuid.h
> +++ b/arch/x86/include/asm/xen/cpuid.h
> @@ -107,6 +107,8 @@
>* ID field from 8 to 15 bits, allowing to target APIC IDs up 32768.
>*/
>   #define XEN_HVM_CPUID_EXT_DEST_ID  (1u << 5)
> +/* Per-vCPU event channel upcalls */
> +#define XEN_HVM_CPUID_UPCALL_VECTOR(1u << 6)
>   
>   /*
>* Leaf 6 (0x4x05)
> diff --git a/arch/x86/include/asm/xen/events.h 
> b/arch/x86/include/asm/xen/events.h
> index 068d9b067c83..62bdceb594f1 100644
> --- a/arch/x86/include/asm/xen/events.h
> +++ b/arch/x86/include/asm/xen/events.h
> @@ -23,7 +23,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
>   /* No need for a barrier -- XCHG is a barrier on x86. */
>   #define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
>   
> -extern int xen_have_vector_callback;
> +extern bool xen_have_vector_callback;
>   
>   /*
>* Events delivered via platform PCI interrupts are always
> @@ -34,4 +34,5 @@ static inline bool xen_support_evtchn_rebind(void)
>   return (!xen_hvm_domain() || xen_have_vector_callback);
>   }
>   
> +extern bool xen_percpu_upcall;
>   #endif /* _ASM_X86_XEN_EVENTS_H */
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 30c6e986a6cd..b8db2148c07d 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -51,7 +51,7 @@ EXPORT_SYMBOL_GPL(xen_start_info);
>   
>   struct shared_info xen_dummy_shared_info;
>   
> -__read_mostly int xen_have_vector_callback;
> +__read_mostly bool xen_have_vector_callback = true;
>   EXPORT_SYMBOL_GPL(xen_have_vector_callback);
>   
>   /*
> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> index 8b71b1dd7639..198d3cd3e9a5 100644
> --- a/arch/x86/xen/enlighten_hvm.c
> +++ b/arch/x86/xen/enlighten_hvm.c
> @@ -7,6 +7,8 @@
>   
>   #include 
>   #include 
> +#include 
> +#include 
>   #include 
>   
>   #include 
> @@ -30,6 +32,9 @@
>   
>   static unsigned long shared_info_pfn;
>   
> +__ro_after_init bool xen_percpu_upcall;
> +EXPORT_SYMBOL_GPL(xen_percpu_upcall);
> +
>   void xen_hvm_init_shared_info(void)
>   {
>   struct xen_add_to_physmap xatp;
> @@ -125,6 +130,9 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_xen_hvm_callback)
>   {
>   struct pt_regs *old_regs = set_irq_regs(regs);
>   
> + if (xen_percpu_upcall)
> + ack_APIC_irq();
> +
>   inc_irq_stat(irq_hv_callback_count);
>   
>   xen_hvm_evtchn_do_upcall();
> @@ -168,6 +176,15 @@ static int xen_cpu_up_prepare_hvm(unsigned int cpu)
>   if (!xen_have_vector_callback)
>   return 0;
>   
> + if (xen_percpu_upcall) {
> + rc = xen_set_upcall_vector(cpu);
> + if (rc) {
> + WARN(1, "HVMOP_set_evtchn_upcall_vector"
> +  " for CPU %d failed: %d\n", cpu, rc);
> + return rc;
> + }
> + }
> +
>   if (xen_feature(XENFEAT_hvm_safe_pvclock))
>   xen_setup_timer(cpu);
>   
> @@ -188,8 +205,6 @@ static int xen_cpu_dead_hvm(unsigned int cpu)
>   return 0;
>   }
>   
> -static bool

[PATCH] x86/vpmu: Fix build following vmfork addition

2022-07-26 Thread Andrew Cooper

GCC complains:

  arch/x86/cpu/vpmu.c:351:15: error: conflicting types for 'vpmu_save_force'; 
have 'void(void *)' with implied 'nocf_check' attribute
351 | void cf_check vpmu_save_force(void *arg)
|   ^~~
  In file included from ./arch/x86/include/asm/domain.h:10,
   from ./include/xen/domain.h:8,
   from ./include/xen/sched.h:11,
   from ./include/xen/event.h:12,
   from arch/x86/cpu/vpmu.c:23:
  ./arch/x86/include/asm/vpmu.h:117:6: note: previous declaration of 
'vpmu_save_force' with type 'void(void *)'
117 | void vpmu_save_force(void *arg);
|  ^~~

Adjust the declaraion.

Fixes: 755087eb9b10 ("xen/mem_sharing: support forks with active vPMU state")
Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Tamas K Lengyel 
---
 xen/arch/x86/include/asm/vpmu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/vpmu.h b/xen/arch/x86/include/asm/vpmu.h
index 8a3ae115623e..05e1fbfccfcf 100644
--- a/xen/arch/x86/include/asm/vpmu.h
+++ b/xen/arch/x86/include/asm/vpmu.h
@@ -114,7 +114,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs);
 void vpmu_initialise(struct vcpu *v);
 void vpmu_destroy(struct vcpu *v);
 void vpmu_save(struct vcpu *v);
-void vpmu_save_force(void *arg);
+void cf_check vpmu_save_force(void *arg);
 int vpmu_load(struct vcpu *v, bool_t from_guest);
 void vpmu_dump(struct vcpu *v);
 
-- 
2.11.0

[PATCH v2] x86/xen: Add support for HVMOP_set_evtchn_upcall_vector

2022-07-26 Thread Jane Malalane

Implement support for the HVMOP_set_evtchn_upcall_vector hypercall in
order to set the per-vCPU event channel vector callback on Linux and
use it in preference of HVM_PARAM_CALLBACK_IRQ.

If the per-VCPU vector setup is successful on BSP, use this method
for the APs. If not, fallback to the global vector-type callback.

Also register callback_irq at per-vCPU event channel setup to trick
toolstack to think the domain is enlightened.

Suggested-by: "Roger Pau Monné" 
Signed-off-by: Jane Malalane 
---
CC: Juergen Gross 
CC: Boris Ostrovsky 
CC: Thomas Gleixner 
CC: Ingo Molnar 
CC: Borislav Petkov 
CC: Dave Hansen 
CC: x...@kernel.org
CC: "H. Peter Anvin" 
CC: Stefano Stabellini 
CC: Oleksandr Tyshchenko 
CC: Jane Malalane 
CC: Maximilian Heyne 
CC: Jan Beulich 
CC: Colin Ian King 
CC: xen-devel@lists.xenproject.org

v2:
 * remove no_vector_callback
 * make xen_have_vector_callback a bool
 * rename xen_ack_upcall to xen_percpu_upcall
 * fail to bring CPU up on init instead of crashing kernel
 * add and use xen_set_upcall_vector where suitable
 * xen_setup_upcall_vector -> xen_init_setup_upcall_vector for clarity
---
 arch/x86/include/asm/xen/cpuid.h   |  2 ++
 arch/x86/include/asm/xen/events.h  |  3 ++-
 arch/x86/xen/enlighten.c   |  2 +-
 arch/x86/xen/enlighten_hvm.c   | 24 ++-
 arch/x86/xen/suspend_hvm.c | 10 +++-
 drivers/xen/events/events_base.c   | 49 ++
 include/xen/hvm.h  |  2 ++
 include/xen/interface/hvm/hvm_op.h | 15 
 8 files changed, 93 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/xen/cpuid.h b/arch/x86/include/asm/xen/cpuid.h
index 78e667a31d6c..6daa9b0c8d11 100644
--- a/arch/x86/include/asm/xen/cpuid.h
+++ b/arch/x86/include/asm/xen/cpuid.h
@@ -107,6 +107,8 @@
  * ID field from 8 to 15 bits, allowing to target APIC IDs up 32768.
  */
 #define XEN_HVM_CPUID_EXT_DEST_ID  (1u << 5)
+/* Per-vCPU event channel upcalls */
+#define XEN_HVM_CPUID_UPCALL_VECTOR(1u << 6)
 
 /*
  * Leaf 6 (0x4x05)
diff --git a/arch/x86/include/asm/xen/events.h 
b/arch/x86/include/asm/xen/events.h
index 068d9b067c83..62bdceb594f1 100644
--- a/arch/x86/include/asm/xen/events.h
+++ b/arch/x86/include/asm/xen/events.h
@@ -23,7 +23,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
 /* No need for a barrier -- XCHG is a barrier on x86. */
 #define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
 
-extern int xen_have_vector_callback;
+extern bool xen_have_vector_callback;
 
 /*
  * Events delivered via platform PCI interrupts are always
@@ -34,4 +34,5 @@ static inline bool xen_support_evtchn_rebind(void)
return (!xen_hvm_domain() || xen_have_vector_callback);
 }
 
+extern bool xen_percpu_upcall;
 #endif /* _ASM_X86_XEN_EVENTS_H */
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 30c6e986a6cd..b8db2148c07d 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -51,7 +51,7 @@ EXPORT_SYMBOL_GPL(xen_start_info);
 
 struct shared_info xen_dummy_shared_info;
 
-__read_mostly int xen_have_vector_callback;
+__read_mostly bool xen_have_vector_callback = true;
 EXPORT_SYMBOL_GPL(xen_have_vector_callback);
 
 /*
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 8b71b1dd7639..198d3cd3e9a5 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -7,6 +7,8 @@
 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include 
@@ -30,6 +32,9 @@
 
 static unsigned long shared_info_pfn;
 
+__ro_after_init bool xen_percpu_upcall;
+EXPORT_SYMBOL_GPL(xen_percpu_upcall);
+
 void xen_hvm_init_shared_info(void)
 {
struct xen_add_to_physmap xatp;
@@ -125,6 +130,9 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_xen_hvm_callback)
 {
struct pt_regs *old_regs = set_irq_regs(regs);
 
+   if (xen_percpu_upcall)
+   ack_APIC_irq();
+
inc_irq_stat(irq_hv_callback_count);
 
xen_hvm_evtchn_do_upcall();
@@ -168,6 +176,15 @@ static int xen_cpu_up_prepare_hvm(unsigned int cpu)
if (!xen_have_vector_callback)
return 0;
 
+   if (xen_percpu_upcall) {
+   rc = xen_set_upcall_vector(cpu);
+   if (rc) {
+   WARN(1, "HVMOP_set_evtchn_upcall_vector"
+" for CPU %d failed: %d\n", cpu, rc);
+   return rc;
+   }
+   }
+
if (xen_feature(XENFEAT_hvm_safe_pvclock))
xen_setup_timer(cpu);
 
@@ -188,8 +205,6 @@ static int xen_cpu_dead_hvm(unsigned int cpu)
return 0;
 }
 
-static bool no_vector_callback __initdata;
-
 static void __init xen_hvm_guest_init(void)
 {
if (xen_pv_domain())
@@ -211,9 +226,6 @@ static void __init xen_hvm_guest_init(void)
 
xen_panic_handler_init();
 
-   if (!no_vector_callback && xen_feature(XENFEAT_hvm_callback_vector))
-   xen_have_vector_callback = 1;
-
xen_hvm_smp_init();

Re: [PATCH] x86/xen: Add support for HVMOP_set_evtchn_upcall_vector

2022-07-26 Thread Jane Malalane

On 25/07/2022 21:46, Boris Ostrovsky wrote:
> 
> On 7/25/22 6:03 AM, Jane Malalane wrote:
>> On 18/07/2022 14:59, Boris Ostrovsky wrote:
>>> On 7/18/22 4:56 AM, Andrew Cooper wrote:
 On 15/07/2022 14:10, Boris Ostrovsky wrote:
> On 7/15/22 5:50 AM, Andrew Cooper wrote:
>> On 15/07/2022 09:18, Jane Malalane wrote:
>>> On 14/07/2022 00:27, Boris Ostrovsky wrote:
>  xen_hvm_smp_init();
>  WARN_ON(xen_cpuhp_setup(xen_cpu_up_prepare_hvm,
> xen_cpu_dead_hvm));
> diff --git a/arch/x86/xen/suspend_hvm.c 
> b/arch/x86/xen/suspend_hvm.c
> index 9d548b0c772f..be66e027ef28 100644
> --- a/arch/x86/xen/suspend_hvm.c
> +++ b/arch/x86/xen/suspend_hvm.c
> @@ -5,6 +5,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "xen-ops.h"
> @@ -14,6 +15,23 @@ void xen_hvm_post_suspend(int 
> suspend_cancelled)
>  xen_hvm_init_shared_info();
>  xen_vcpu_restore();
>  }
> -    xen_setup_callback_vector();
> +    if (xen_ack_upcall) {
> +    unsigned int cpu;
> +
> +    for_each_online_cpu(cpu) {
> +    xen_hvm_evtchn_upcall_vector_t op = {
> +    .vector = HYPERVISOR_CALLBACK_VECTOR,
> +    .vcpu = per_cpu(xen_vcpu_id, cpu),
> +    };
> +
> +
> BUG_ON(HYPERVISOR_hvm_op(HVMOP_set_evtchn_upcall_vector,
> + ));
> +    /* Trick toolstack to think we are enlightened. */
> +    if (!cpu)
> +    BUG_ON(xen_set_callback_via(1));
 What are you trying to make the toolstack aware of? That we have 
 *a*
 callback (either global or percpu)?
>>> Yes, specifically for the check in 
>>> libxl__domain_pvcontrol_available.
>> And others.
>>
>> This is all a giant bodge, but basically a lot of tooling uses the
>> non-zero-ness of the CALLBACK_VIA param to determine whether the 
>> VM has
>> Xen-aware drivers loaded or not.
>>
>> The value 1 is a CALLBACK_VIA value which encodes GSI 1, and the only
>> reason this doesn't explode everywhere is because the
>> evtchn_upcall_vector registration takes priority over GSI delivery.
>>
>> This is decades of tech debt piled on top of tech debt.
> Feels like it (setting the callback parameter) is something that the
> hypervisor should do --- no need to expose guests to this.
 Sensible or not, it is the ABI.

 Linux still needs to work (nicely) with older Xen's in the world, 
 and we
 can't just retrofit a change in the hypervisor which says "btw, this 
 ABI
 we've just changed now has a side effect of modifying a field that you
 also logically own".
>>>
>>> The hypercall has been around for a while so I understand ABI concerns
>>> there but XEN_HVM_CPUID_UPCALL_VECTOR was introduced only a month ago.
>>> Why not tie presence of this bit to no longer having to explicitly set
>>> the callback field?
>>>
>> Any other opinions on this?
>>
>> (i.e., calling xen_set_callback_via(1) after
>> HVMOP_set_evtchn_upcall_vector OR not exposing this to guests and
>> instead having Xen call this function (in hvmop_set_evtchn_upcall_vector
>> maybe) and tieing its presense to XEN_HVM_CPUID_UPCALL_VECTOR which was
>> recently added)
> 
> 
> CPUID won't help here, I wasn't thinking clearly.
> 
> 
> Can we wrap the HVMOP_set_evtchn_upcall_vector hypercall in a function 
> that will decide whether or not to also do xen_set_callback_via(1)?
> 
Okay. Will do this in a v2.

Thanks,

Jane.

Xen Security Advisory 408 v2 (CVE-2022-33745) - insufficient TLB flush for x86 PV guests in shadow mode

2022-07-26 Thread Xen . org security team

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Xen Security Advisory CVE-2022-33745 / XSA-408
   version 2

insufficient TLB flush for x86 PV guests in shadow mode

UPDATES IN VERSION 2


Added metadata

Public release.

ISSUE DESCRIPTION
=

For migration as well as to work around kernels unaware of L1TF (see
XSA-273), PV guests may be run in shadow paging mode.  To address
XSA-401, code was moved inside a function in Xen.  This code movement
missed a variable changing meaning / value between old and new code
positions.  The now wrong use of the variable did lead to a wrong TLB
flush condition, omitting flushes where such are necessary.

IMPACT
==

The known (observed) impact would be a Denial of Service (DoS) affecting
the entire host, due to running out of memory.  Privilege escalation and
information leaks cannot be ruled out.

VULNERABLE SYSTEMS
==

All versions of Xen with the XSA-401 fixes applied are vulnerable.

Only x86 PV guests can trigger this vulnerability, and only when running
in shadow mode.  Shadow mode would be in use when migrating guests or as
a workaround for XSA-273 (L1TF).

MITIGATION
==

Not running x86 PV guests will avoid the vulnerability.

CREDITS
===

This issue was discovered by Charles Arnold of SUSE.

RESOLUTION
==

Applying the appropriate attached patch resolves this issue.

Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball.  Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.

xsa408.patch   xen-unstable - Xen 4.14.x
xsa408-4.13.patch  Xen 4.13.x

$ sha256sum xsa408*
7349445d53b68bc8e2be2aea9fa20409a9b87e0d6b78fc2515093a65668444a0  xsa408.meta
f49cb67842c7576f1d59b965331956a9fa1f529a8e2da3531d7ebc4eb3f079b3  xsa408.patch
26871efbd3f834dd4af4fbab6f2cb09a83c509e49894f025ee656071419ed995  
xsa408-4.13.patch
$

DEPLOYMENT DURING EMBARGO
=

Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.

(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-BEGIN PGP SIGNATURE-

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmLfyP4MHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZSkAIAM3XDzBdUXux7ONc9nztSMGPBdWosC5f0SycveSq
adplJeShw50aFYLxpZzqfCBAX/Jh0ooF+7gHnjVMuKKkg8vu5SfBpSGRdmva6jpc
qNXoNyIc21PdNH4PVCKDQnO8Dq8wPSCnPpMZbFwk2uz7QGN5BKU/GM6XQrmXA3wz
3XYIcVVR377MdDuR8UQKyCSAG0JPr6SiozygRFHykGjg9NABWZwGyod64C9xBAyu
K8CGTx12bAJEVcqJbGAVSEU6J5iKdWjSLHwy43ZOcAFvfiCAlolBOPlfjJTllYdQ
Yhv0wQtOwsIDjQU6vbUtMsckuNEmfMPTEkRHPOpp46dPuVk=
=33sr
-END PGP SIGNATURE-


xsa408.meta
Description: Binary data


xsa408.patch
Description: Binary data


xsa408-4.13.patch
Description: Binary data

[PATCH] x86/tboot: Drop mfn_in_guarded_stack()

2022-07-26 Thread Andrew Cooper

To support CET Shadow Stacks, guard pages changed from being holes to being
read-only.  As such, they can be read.

Moreover, they should be included in the integrity check.

Fixes: 60016604739b ("x86/shstk: Rework the stack layout to support shadow 
stacks")
Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
---
 xen/arch/x86/tboot.c | 33 +
 1 file changed, 1 insertion(+), 32 deletions(-)

diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c
index fe1abfdf08ff..03098450f788 100644
--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -242,29 +242,6 @@ static void tboot_gen_domain_integrity(const uint8_t 
key[TB_KEY_SIZE],
 memset(, 0, sizeof(ctx));
 }
 
-/*
- * For stack overflow detection in debug build, a guard page is set up.
- * This fn is used to detect whether a page is in the guarded pages for
- * the above reason.
- */
-static int mfn_in_guarded_stack(unsigned long mfn)
-{
-void *p;
-int i;
-
-for ( i = 0; i < nr_cpu_ids; i++ )
-{
-if ( !stack_base[i] )
-continue;
-p = (void *)((unsigned long)stack_base[i] + STACK_SIZE -
- PRIMARY_STACK_SIZE - PAGE_SIZE);
-if ( mfn == virt_to_mfn(p) )
-return -1;
-}
-
-return 0;
-}
-
 static void tboot_gen_xenheap_integrity(const uint8_t key[TB_KEY_SIZE],
 vmac_t *mac)
 {
@@ -288,15 +265,7 @@ static void tboot_gen_xenheap_integrity(const uint8_t 
key[TB_KEY_SIZE],
 continue; /* skip tboot and its page tables */
 
 if ( is_page_in_use(page) && is_special_page(page) )
-{
-void *pg;
-
-if ( mfn_in_guarded_stack(mfn) )
-continue; /* skip guard stack, see memguard_guard_stack() in 
mm.c */
-
-pg = mfn_to_virt(mfn);
-vmac_update((uint8_t *)pg, PAGE_SIZE, );
-}
+vmac_update(mfn_to_virt(mfn), PAGE_SIZE, );
 }
 *mac = vmac(NULL, 0, nonce, NULL, );
 
-- 
2.11.0

Re: [PATCH] x86/pv: Inject #GP for implicit grant unmaps

2022-07-26 Thread Jan Beulich

On 26.07.2022 13:51, Andrew Cooper wrote:
> On 26/07/2022 07:29, Jan Beulich wrote:
>> On 25.07.2022 19:50, Andrew Cooper wrote:
>>> This is a debug behaviour to identify buggy kernels.  Crashing the domain is
>>> the most unhelpful thing to do, because it discards the relevant context.
>>>
>>> Instead, inject #GP[0] like other permission errors in x86.  In particular,
>>> this lets the kernel provide a backtrace that's actually helpful to a
>>> developer trying to figure out what's going wrong.
>>>
>>> As a bugfix, this always injects #GP[0] to current, not l1e_owner.  It is 
>>> not
>>> l1e_owner's fault if dom0 using superpowers triggers an implicit unmap.
>>>
>>> Signed-off-by: Andrew Cooper 
>> Acked-by: Jan Beulich 
>>
>> Albeit preferably with ...
>>
>>> --- a/xen/arch/x86/mm.c
>>> +++ b/xen/arch/x86/mm.c
>>> @@ -1232,7 +1232,7 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct 
>>> domain *l1e_owner)
>>>  gdprintk(XENLOG_WARNING,
>>>   "Attempt to implicitly unmap a granted PTE %" PRIpte "\n",
>>>   l1e_get_intpte(l1e));
>>> -domain_crash(l1e_owner);
>>> +pv_inject_hw_exception(TRAP_gp_fault, 0);
>>>  }
>>>  #endif
>> ... the gdprintk() adjusted to also log l1e_owner.
> 
> Ok, how about this incremental?
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index b3393385ffb6..74054fb5f4ee 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -1229,9 +1229,9 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct
> domain *l1e_owner)
>  if ( (l1e_get_flags(l1e) & _PAGE_GNTTAB) &&
>   !l1e_owner->is_shutting_down && !l1e_owner->is_dying )
>  {
> -    gdprintk(XENLOG_WARNING,
> - "Attempt to implicitly unmap a granted PTE %" PRIpte "\n",
> - l1e_get_intpte(l1e));
> +    gprintk(XENLOG_WARNING,
> +    "Attempt to implicitly %pd's gntmap PTE %" PRIpte "\n",
> +    l1e_owner, l1e_get_intpte(l1e));

DYM to drop "unmap"? With it restored (or anything similar to that
effect), fine with me.

Jan

Re: [PATCH] x86/pv: Inject #GP for implicit grant unmaps

2022-07-26 Thread Andrew Cooper

On 26/07/2022 07:29, Jan Beulich wrote:
> On 25.07.2022 19:50, Andrew Cooper wrote:
>> This is a debug behaviour to identify buggy kernels.  Crashing the domain is
>> the most unhelpful thing to do, because it discards the relevant context.
>>
>> Instead, inject #GP[0] like other permission errors in x86.  In particular,
>> this lets the kernel provide a backtrace that's actually helpful to a
>> developer trying to figure out what's going wrong.
>>
>> As a bugfix, this always injects #GP[0] to current, not l1e_owner.  It is not
>> l1e_owner's fault if dom0 using superpowers triggers an implicit unmap.
>>
>> Signed-off-by: Andrew Cooper 
> Acked-by: Jan Beulich 
>
> Albeit preferably with ...
>
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -1232,7 +1232,7 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct domain 
>> *l1e_owner)
>>  gdprintk(XENLOG_WARNING,
>>   "Attempt to implicitly unmap a granted PTE %" PRIpte "\n",
>>   l1e_get_intpte(l1e));
>> -domain_crash(l1e_owner);
>> +pv_inject_hw_exception(TRAP_gp_fault, 0);
>>  }
>>  #endif
> ... the gdprintk() adjusted to also log l1e_owner.

Ok, how about this incremental?

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index b3393385ffb6..74054fb5f4ee 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1229,9 +1229,9 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct
domain *l1e_owner)
 if ( (l1e_get_flags(l1e) & _PAGE_GNTTAB) &&
  !l1e_owner->is_shutting_down && !l1e_owner->is_dying )
 {
-    gdprintk(XENLOG_WARNING,
- "Attempt to implicitly unmap a granted PTE %" PRIpte "\n",
- l1e_get_intpte(l1e));
+    gprintk(XENLOG_WARNING,
+    "Attempt to implicitly %pd's gntmap PTE %" PRIpte "\n",
+    l1e_owner, l1e_get_intpte(l1e));
 pv_inject_hw_exception(TRAP_gp_fault, 0);
 }
 #endif

The printk() needs to not be omitted in release builds which happen to
have this logic compiled in.

~Andrew

Re: Random crash during guest start on x86

2022-07-26 Thread Jan Beulich

On 26.07.2022 12:36, Bertrand Marquis wrote:
> Would it make sense then for me to try a newer linux kernel first ?

While it's almost always worth a try, I can't really tell. That's
precisely the kind of question that maintainers are in a better
position to answer.

Jan

Re: [PATCH 14/36] cpuidle: Fix rcu_idle_*() usage

2022-07-26 Thread Gautham R. Shenoy

On Wed, Jun 08, 2022 at 04:27:37PM +0200, Peter Zijlstra wrote:
> The whole disable-RCU, enable-IRQS dance is very intricate since
> changing IRQ state is traced, which depends on RCU.
> 
> Add two helpers for the cpuidle case that mirror the entry code.
> 
> Signed-off-by: Peter Zijlstra (Intel) 

Reviewed-by: Gautham R. Shenoy 

> ---
>  arch/arm/mach-imx/cpuidle-imx6q.c|4 +--
>  arch/arm/mach-imx/cpuidle-imx6sx.c   |4 +--
>  arch/arm/mach-omap2/cpuidle34xx.c|4 +--
>  arch/arm/mach-omap2/cpuidle44xx.c|8 +++---
>  drivers/acpi/processor_idle.c|   18 --
>  drivers/cpuidle/cpuidle-big_little.c |4 +--
>  drivers/cpuidle/cpuidle-mvebu-v7.c   |4 +--
>  drivers/cpuidle/cpuidle-psci.c   |4 +--
>  drivers/cpuidle/cpuidle-riscv-sbi.c  |4 +--
>  drivers/cpuidle/cpuidle-tegra.c  |8 +++---
>  drivers/cpuidle/cpuidle.c|   11 
>  include/linux/cpuidle.h  |   37 +---
>  kernel/sched/idle.c  |   45 
> ++-
>  kernel/time/tick-broadcast.c |6 +++-
>  14 files changed, 90 insertions(+), 71 deletions(-)
> 
> --- a/arch/arm/mach-imx/cpuidle-imx6q.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6q.c
> @@ -24,9 +24,9 @@ static int imx6q_enter_wait(struct cpuid
>   imx6_set_lpm(WAIT_UNCLOCKED);
>   raw_spin_unlock(_lock);
>  
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
>   cpu_do_idle();
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>  
>   raw_spin_lock(_lock);
>   if (num_idle_cpus-- == num_online_cpus())
> --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> @@ -47,9 +47,9 @@ static int imx6sx_enter_wait(struct cpui
>   cpu_pm_enter();
>   cpu_cluster_pm_enter();
>  
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
>   cpu_suspend(0, imx6sx_idle_finish);
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>  
>   cpu_cluster_pm_exit();
>   cpu_pm_exit();
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -133,9 +133,9 @@ static int omap3_enter_idle(struct cpuid
>   }
>  
>   /* Execute ARM wfi */
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
>   omap_sram_idle();
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>  
>   /*
>* Call idle CPU PM enter notifier chain to restore
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -105,9 +105,9 @@ static int omap_enter_idle_smp(struct cp
>   }
>   raw_spin_unlock_irqrestore(_lock, flag);
>  
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
>   omap4_enter_lowpower(dev->cpu, cx->cpu_state);
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>  
>   raw_spin_lock_irqsave(_lock, flag);
>   if (cx->mpu_state_vote == num_online_cpus())
> @@ -186,10 +186,10 @@ static int omap_enter_idle_coupled(struc
>   }
>   }
>  
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
>   omap4_enter_lowpower(dev->cpu, cx->cpu_state);
>   cpu_done[dev->cpu] = true;
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>  
>   /* Wakeup CPU1 only if it is not offlined */
>   if (dev->cpu == 0 && cpumask_test_cpu(1, cpu_online_mask)) {
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -607,7 +607,7 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
>   * @cx: Target state context
>   * @index: index of target state
>   */
> -static int acpi_idle_enter_bm(struct cpuidle_driver *drv,
> +static noinstr int acpi_idle_enter_bm(struct cpuidle_driver *drv,
>  struct acpi_processor *pr,
>  struct acpi_processor_cx *cx,
>  int index)
> @@ -626,6 +626,8 @@ static int acpi_idle_enter_bm(struct cpu
>*/
>   bool dis_bm = pr->flags.bm_control;
>  
> + instrumentation_begin();
> +
>   /* If we can skip BM, demote to a safe state. */
>   if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
>   dis_bm = false;
> @@ -647,11 +649,11 @@ static int acpi_idle_enter_bm(struct cpu
>   raw_spin_unlock(_lock);
>   }
>  
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
>  
>   acpi_idle_do_entry(cx);
>  
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>  
>   /* Re-enable bus master arbitration */
>   if (dis_bm) {
> @@ -661,11 +663,13 @@ static int acpi_idle_enter_bm(struct cpu
>   raw_spin_unlock(_lock);
>   }
>  
> + instrumentation_end();
> +
>   return index;
>  }
>  
> -static int acpi_idle_enter(struct cpuidle_device *dev,
> -struct cpuidle_driver *drv, int index)
> +static noinstr int acpi_idle_enter(struct cpuidle_device *dev,
> +struct cpuidle_driver

RE: Enable audio virtualization in Xen

2022-07-26 Thread SHARMA, JYOTIRMOY

[AMD Official Use Only - General]

Hi Christopher,

Thank you for the quick response. Please find answers below.

>> Does audio playback work OK from your Ubuntu dom0?
Yes.

>> Do you know which version of Ubuntu you are using? ('cat /etc/lsb-release')
Ubuntu 22.04 LTS.

>> Do you know which version of Xen you are using? ('xl info' in dom0 should 
>> help)
4.16.2-pre. 

>> Do you know which version of Qemu you have installed in dom0?
QEMU emulator version 6.1.1.

>> Did you build and install Xen from source code, or are you using binary 
>> packages of Xen and its tools?
We built and installed Xen from source code (git clone 
https://xenbits.xen.org/git-http/xen.git).

>> Are you using the xl tools, or libvirt tools for configuring and running 
>> your guest? -- ie. how do you start your domU VM?
We are using xl tools (sudo xl -v create ).

>> When your domU is running, please could you run 'ps auxwww' in dom0 and 
>> obtain the process information about the qemu instance that is running, so 
>> that we can see what command line arguments have been supplied to it

This is the log we get right after booting dom 0:

root 723 0.0 0.2 243792 14468 ? Sl 15:51 0:00 
/usr/local/lib/xen/bin/qemu-system-i386 -xen-domid 0 -xen-attach -name dom0 
-nographic -M xenpv -daemonize -monitor /dev/null -serial /dev/null -parallel 
/dev/null -pidfile /var/run/xen/qemu-dom0.pid

This log is seen after we launch dom U (sudo xl -v create ):

root 2152 20.7 2.3 2858204 133496 ? Ssl 15:53 0:09 
/usr/local/lib/xen/bin/qemu-system-i386 -xen-domid 1 -no-shutdown -chardev 
socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-1,server=on,wait=off -mon 
chardev=libxl-cmd,mode=control -chardev 
socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-1,server=on,wait=off 
-mon chardev=libxenstat-cmd,mode=control -nodefaults -no-user-config -name 
domu-ubuntu.hvm -vnc 127.0.0.1:0,to=99 -display sdl,gl=on -sdl -device 
virtio-vga-gl -boot order=dc -usb -usbdevice tablet -smp 2,maxcpus=2 -net none 
-machine xenfv,suppress-vmdesc=on -m 2040 -drive 
file=/home/amd/u2004_ubuntu.qcow2,if=ide,index=0,media=disk,format=qcow2,cache=writeback

>> A little more information about what you're running will help with providing 
>> guidance here. The xl man page indicates that there is a "soundhw" option in 
>> the VM configuration file for passing sound hardware configution through to 
>> qemu, so if you are using the xl toolstack, you could try adding this to the 
>> config file: soundhw="hda"

I tried giving soundhw="hda" option in the hvm config file. However, I do not 
hear any sound when I play a wave file in dom U. Here is the 'ps auxwww' output 
after making this change:

root 2568 14.8 2.3 2770864 134720 ? Ssl 15:58 0:09 
/usr/local/lib/xen/bin/qemu-system-i386 -xen-domid 2 -no-shutdown -chardev 
socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-2,server=on,wait=off -mon 
chardev=libxl-cmd,mode=control -chardev 
socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-2,server=on,wait=off 
-mon chardev=libxenstat-cmd,mode=control -nodefaults -no-user-config -name 
domu-ubuntu-audio.hvm -vnc 127.0.0.1:0,to=99 -display sdl,gl=on -sdl -device 
virtio-vga-gl -boot order=dc -usb -usbdevice tablet -soundhw hda -smp 
2,maxcpus=2 -net none -machine xenfv,suppress-vmdesc=on -m 2040 -drive 
file=/home/amd/u2004_ubuntu.qcow2,if=ide,index=0,media=disk,format=qcow2,cache=writeback

Also, I tried giving soundhw="all" in the config file, however this throws 
following error:

libxl: error: libxl_dm.c:3130:device_model_spawn_outcome: Domain 4:domain 4 
device model: spawn failed (rc=-3)
libxl: error: libxl_dm.c:3350:device_model_postconfig_done: Domain 4:Post DM 
startup configs failed, rc=-3
libxl: error: libxl_create.c:1867:domcreate_devmodel_started: Domain 4:device 
model did not start: -3
libxl: error: libxl_aoutils.c:646:libxl__kill_xs_path: Device Model already 
exited
libxl: error: libxl_domain.c:1183:libxl__destroy_domid: Domain 4:Non-existant 
domain
libxl: error: libxl_domain.c:1137:domain_destroy_callback: Domain 4:Unable to 
destroy guest
libxl: error: libxl_domain.c:1064:domain_destroy_cb: Domain 4:Destruction of 
domain failed

Can you please let me know what parameters I should use in the config file to 
play audio from dom U?

Regards,
Jyotirmoy

-Original Message-
From: Christopher Clark  
Sent: Tuesday, July 26, 2022 2:44 AM
To: SHARMA, JYOTIRMOY 
Cc: xen-devel@lists.xenproject.org; xen-us...@lists.xenproject.org
Subject: Re: Enable audio virtualization in Xen

[CAUTION: External Email]

On Mon, Jul 25, 2022 at 4:45 AM SHARMA, JYOTIRMOY  
wrote:
>
> [AMD Official Use Only - General]
>
>
> Hi all,

Hi Jyotirmoy,

I have add the xen-users list to CC since this thread may be useful to that 
forum.

> I am using ubuntu as dom 0 and also dom U (HVM). I want to play audio from 
> “dom U” Ubuntu.

I think that it should be possible to enable what you are attempting to do. ie. 
audio output from a HVM Ubuntu guest VM.

Some questions to support

Re: Random crash during guest start on x86

2022-07-26 Thread Bertrand Marquis

Hi Jan,

> On 26 Jul 2022, at 11:29, Jan Beulich  wrote:
> 
> On 26.07.2022 09:51, Bertrand Marquis wrote:
>> 
>> 
>>> On 25 Jul 2022, at 17:16, Jan Beulich  wrote:
>>> 
>>> On 25.07.2022 17:51, Bertrand Marquis wrote:
 On our CI we have randomly a crash during guest boot on x86.
>>> 
>>> Afaict of a PV guest.
>>> 
 We are running on qemu x86_64 using Xen staging.
>>> 
>>> Which may introduce unusual timing. An issue never hit on actual hardware
>>> _may_ (but doesn't have to be) one in qemu itself.
>>> 
 The crash is happening randomly (something like 1 out of 20 times).
 
 This is always happening on the first guest we start, we never got it 
 after first guest was successfully started.
 
 Please tell me if you need any other info.
 
 Here is the guest kernel log:
 [...]
 [ 6.679020] general protection fault, maybe for address 0x8800:  [#1] 
 PREEMPT SMP NOPTI
 [ 6.679020] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.17.6 #1
 [ 6.679020] RIP: e030:error_entry+0xaf/0xe0
 [ 6.679020] Code: 29 89 c8 48 39 84 24 88 00 00 00 74 15 48 81 bc 24 88 00 
 00 00 63 10 e0 81 75 03 0f 01 f8 90 90 90 c3 48 89 8c 24 88 00 00 00 <0f> 
 01 f8 90 90 90 eb 11 0f 20 d8 90 90 90 90 90 48 25 ff e7 ff ff
>>> 
>>> This is SWAPGS, which supposedly a PV guest should never hit. Data further
>>> down suggests the kernel is still in the process of patching alternatives,
>>> which may be the reason for the insn to still be there (being at a point
>>> where exceptions are still unexpected).
>> 
>> So the exception path is using alternative code ? Sounds logic with the 
>> error output.
>> But does explain the original error.
> 
> SWAPGS sits pretty early on all kernel entry paths. If any instance of it
> is subject to alternatives patching, then prior to patching such paths
> may not be taken when running as PV guest under Xen.
> 
 [ 6.679020] RSP: e02b:82803a90 EFLAGS: 0046
 [ 6.679020] RAX: 8800 RBX:  RCX: 
 81e00fa7
 [ 6.679020] RDX:  RSI: 81e009f8 RDI: 
 00eb
 [ 6.679020] RBP:  R08:  R09: 
 
 [ 6.679020] R10:  R11:  R12: 
 
 [ 6.679020] R13:  R14:  R15: 
 
 [ 6.679020] FS: () GS:88801f20() 
 knlGS:
 [ 6.679020] CS: 1e030 DS:  ES:  CR0: 80050033
 [ 6.679020] CR2:  CR3: 0280c000 CR4: 
 00050660
 [ 6.679020] Call Trace:
 [ 6.679020] 
 [ 6.679020] RIP: e030:native_irq_return_iret+0x0/0x2
 [ 6.679020] Code: 41 5d 41 5c 5d 5b 41 5b 41 5a 41 59 41 58 58 59 5a 5e 5f 
 48 83 c4 08 eb 0a 0f 1f 00 90 66 0f 1f 44 00 00 f6 44 24 20 04 75 02 <48> 
 cf 57 0f 01 f8 eb 12 0f 20 df 90 90 90 90 90 48 81 e7 ff e7 ff
 [ 6.679020] RSP: e02b:82803b48 EFLAGS: 0046 ORIG_RAX: 
 e030
 [ 6.679020] RAX: 8800 RBX: 82803be0 RCX: 
 81e00f95
 [ 6.679020] RDX: 81e00f94 RSI: 81e00f95 RDI: 
 00eb
 [ 6.679020] RBP: 00eb R08: 90001f0f R09: 
 0007
 [ 6.679020] R10: 81e00f94 R11: 8285a6c0 R12: 
 
 [ 6.679020] R13: 81e00f94 R14: 0006 R15: 
 0006
 [ 6.679020] ? asm_exc_general_protection+0x8/0x30
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1c/0x27
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1c/0x27
 [ 6.679020] RIP: e030:insn_get_opcode.part.0+0xab/0x180
 [ 6.679020] Code: 00 00 8b 43 4c a9 c0 07 08 00 0f 84 bf 00 00 00 c6 43 1c 
 01 31 c0 5b 5d c3 83 e2 03 be 01 00 00 00 eb b7 89 ef e8 65 e4 ff ff <89> 
 43 4c a8 30 75 21 e9 8e 00 00 00 0f b6 7b 03 40 84 ff 75 73 8b
 [ 6.679020] RSP: e02b:82803b70 EFLAGS: 0246
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
 [ 6.679020] insn_get_modrm+0x6c/0x120
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
 [ 6.679020] insn_get_sib+0x40/0x80
 [ 6.679020] insn_get_displacement+0x82/0x100
 [ 6.679020] insn_decode+0xf8/0x100
 [ 6.679020] optimize_nops+0x60/0x1e0
 [ 6.679020] ? rcu_nmi_exit+0x2b/0x140
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
 [ 6.679020] ? native_iret+0x3/0x7
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1c/0x27
 [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
 [ 6.679020] apply_alternatives+0x165/0x2e0
>>> 
>>> I have to admit that I'm a little lost with these "modern"

Re: Random crash during guest start on x86

2022-07-26 Thread Jan Beulich

On 26.07.2022 09:51, Bertrand Marquis wrote:
> 
> 
>> On 25 Jul 2022, at 17:16, Jan Beulich  wrote:
>>
>> On 25.07.2022 17:51, Bertrand Marquis wrote:
>>> On our CI we have randomly a crash during guest boot on x86.
>>
>> Afaict of a PV guest.
>>
>>> We are running on qemu x86_64 using Xen staging.
>>
>> Which may introduce unusual timing. An issue never hit on actual hardware
>> _may_ (but doesn't have to be) one in qemu itself.
>>
>>> The crash is happening randomly (something like 1 out of 20 times).
>>>
>>> This is always happening on the first guest we start, we never got it after 
>>> first guest was successfully started.
>>>
>>> Please tell me if you need any other info.
>>>
>>> Here is the guest kernel log:
>>> [...]
>>> [ 6.679020] general protection fault, maybe for address 0x8800:  [#1] 
>>> PREEMPT SMP NOPTI
>>> [ 6.679020] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.17.6 #1
>>> [ 6.679020] RIP: e030:error_entry+0xaf/0xe0
>>> [ 6.679020] Code: 29 89 c8 48 39 84 24 88 00 00 00 74 15 48 81 bc 24 88 00 
>>> 00 00 63 10 e0 81 75 03 0f 01 f8 90 90 90 c3 48 89 8c 24 88 00 00 00 <0f> 
>>> 01 f8 90 90 90 eb 11 0f 20 d8 90 90 90 90 90 48 25 ff e7 ff ff
>>
>> This is SWAPGS, which supposedly a PV guest should never hit. Data further
>> down suggests the kernel is still in the process of patching alternatives,
>> which may be the reason for the insn to still be there (being at a point
>> where exceptions are still unexpected).
> 
> So the exception path is using alternative code ? Sounds logic with the error 
> output.
> But does explain the original error.

SWAPGS sits pretty early on all kernel entry paths. If any instance of it
is subject to alternatives patching, then prior to patching such paths
may not be taken when running as PV guest under Xen.

>>> [ 6.679020] RSP: e02b:82803a90 EFLAGS: 0046
>>> [ 6.679020] RAX: 8800 RBX:  RCX: 
>>> 81e00fa7
>>> [ 6.679020] RDX:  RSI: 81e009f8 RDI: 
>>> 00eb
>>> [ 6.679020] RBP:  R08:  R09: 
>>> 
>>> [ 6.679020] R10:  R11:  R12: 
>>> 
>>> [ 6.679020] R13:  R14:  R15: 
>>> 
>>> [ 6.679020] FS: () GS:88801f20() 
>>> knlGS:
>>> [ 6.679020] CS: 1e030 DS:  ES:  CR0: 80050033
>>> [ 6.679020] CR2:  CR3: 0280c000 CR4: 
>>> 00050660
>>> [ 6.679020] Call Trace:
>>> [ 6.679020] 
>>> [ 6.679020] RIP: e030:native_irq_return_iret+0x0/0x2
>>> [ 6.679020] Code: 41 5d 41 5c 5d 5b 41 5b 41 5a 41 59 41 58 58 59 5a 5e 5f 
>>> 48 83 c4 08 eb 0a 0f 1f 00 90 66 0f 1f 44 00 00 f6 44 24 20 04 75 02 <48> 
>>> cf 57 0f 01 f8 eb 12 0f 20 df 90 90 90 90 90 48 81 e7 ff e7 ff
>>> [ 6.679020] RSP: e02b:82803b48 EFLAGS: 0046 ORIG_RAX: 
>>> e030
>>> [ 6.679020] RAX: 8800 RBX: 82803be0 RCX: 
>>> 81e00f95
>>> [ 6.679020] RDX: 81e00f94 RSI: 81e00f95 RDI: 
>>> 00eb
>>> [ 6.679020] RBP: 00eb R08: 90001f0f R09: 
>>> 0007
>>> [ 6.679020] R10: 81e00f94 R11: 8285a6c0 R12: 
>>> 
>>> [ 6.679020] R13: 81e00f94 R14: 0006 R15: 
>>> 0006
>>> [ 6.679020] ? asm_exc_general_protection+0x8/0x30
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1c/0x27
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1c/0x27
>>> [ 6.679020] RIP: e030:insn_get_opcode.part.0+0xab/0x180
>>> [ 6.679020] Code: 00 00 8b 43 4c a9 c0 07 08 00 0f 84 bf 00 00 00 c6 43 1c 
>>> 01 31 c0 5b 5d c3 83 e2 03 be 01 00 00 00 eb b7 89 ef e8 65 e4 ff ff <89> 
>>> 43 4c a8 30 75 21 e9 8e 00 00 00 0f b6 7b 03 40 84 ff 75 73 8b
>>> [ 6.679020] RSP: e02b:82803b70 EFLAGS: 0246
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>>> [ 6.679020] insn_get_modrm+0x6c/0x120
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>>> [ 6.679020] insn_get_sib+0x40/0x80
>>> [ 6.679020] insn_get_displacement+0x82/0x100
>>> [ 6.679020] insn_decode+0xf8/0x100
>>> [ 6.679020] optimize_nops+0x60/0x1e0
>>> [ 6.679020] ? rcu_nmi_exit+0x2b/0x140
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>>> [ 6.679020] ? native_iret+0x3/0x7
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1c/0x27
>>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>>> [ 6.679020] apply_alternatives+0x165/0x2e0
>>
>> I have to admit that I'm a little lost with these "modern" stack traces,
>> where contexts apparently switch without being clearly annotated. It is
>> looking a little as if a #GP fault was happening somewhere here (hence
>> the

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

2022-07-26 Thread osstest service owner

flight 171861 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/171861/

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  5707470bf3103ebae43697a7ac2faced6cd35f92
baseline version:
 xen  f1c719d5cd8ab4d5d4c8df139b9df3c2c47221d1

Last test of basis   171854  2022-07-25 14:03:15 Z0 days
Testing same since   171861  2022-07-26 07:01:41 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Luca Fancellu 

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
   f1c719d5cd..5707470bf3  5707470bf3103ebae43697a7ac2faced6cd35f92 -> smoke

Re: [PATCH v3 00/10] Add Xue - console over USB 3 Debug Capability

2022-07-26 Thread Marek Marczykowski-Górecki

On Tue, Jul 26, 2022 at 08:18:40AM +0200, Jan Beulich wrote:
> On 26.07.2022 05:23, Marek Marczykowski-Górecki wrote:
> > This is integration of https://github.com/connojd/xue into mainline Xen.
> > This patch series includes several patches that I made in the process, some 
> > are
> > very loosely related.
> > 
> > The driver developed by Connor supports console via USB3 debug capability. 
> > The
> > capability is designed to operate mostly independently of normal XHCI 
> > driver,
> > so this patch series allows dom0 to drive standard USB3 controller part, 
> > while
> > Xen uses DbC for console output.
> > 
> > Changes since RFC:
> >  - move the driver to xue.c, remove non-Xen parts, remove now unneeded 
> > abstraction
> >  - adjust for Xen code style
> >  - build for x86 only
> >  - drop patch hidding the device from dom0
> > Changes since v1:
> >  - drop ehci patch - already applied
> >  - adjust for review comments from Jan (see changelogs in individual 
> > patches)
> > Changes since v2:
> >  - add runtime option to share (or not) the controller with dom0 or other 
> > domains
> >  - add RX support
> >  - several smaller changes according to review comments
> > 
> > Cc: Andrew Cooper 
> > Cc: George Dunlap 
> > Cc: Jan Beulich 
> > Cc: Julien Grall 
> > Cc: Stefano Stabellini 
> > Cc: Wei Liu 
> > Cc: "Roger Pau Monné" 
> > Cc: Paul Durrant 
> > Cc: Kevin Tian 
> > Cc: Connor Davis 
> > 
> > Connor Davis (1):
> >   drivers/char: Add support for USB3 DbC debugger
> 
> I notice this patch was sent twice, about 4 min ahead of the full set.
> I take it that the set having come close together is the definitive
> one, and that one extra copy is the one to be ignored.

Yes, I noticed too late I forgot to drop Connor's broken address, that's
the only difference.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab


signature.asc
Description: PGP signature

[xen-unstable test] 171859: tolerable FAIL

2022-07-26 Thread osstest service owner

flight 171859 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/171859/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-libvirt   7 xen-install  fail in 171856 pass in 171859
 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-install fail in 171856 pass in 171859
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail in 171856 
pass in 171859
 test-arm64-arm64-libvirt-raw 13 guest-start  fail in 171856 pass in 171859
 test-amd64-i386-examine   6 xen-installfail pass in 171856
 test-arm64-arm64-xl-credit1  19 guest-start.2  fail pass in 171856

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 171856
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 171856
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 171856
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 171856
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 171856
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 171856
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 171856
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 171856
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 171856
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 171856
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 171856
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 171856
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-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-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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-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-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  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-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-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-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-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
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass

Question to mem-path support at QEMU for Xen

2022-07-26 Thread Huang Rui

Hi Anthony and other Qemu/Xen guys,

We are trying to enable venus on Xen virtualization platform. And we would
like to use the backend memory with memory-backend-memfd,id=mem1,size=4G
options on QEMU, however, the QEMU will tell us the "-mem-path" is not
supported with Xen. I verified the same function on KVM.

qemu-system-i386: -mem-path not supported with Xen

So may I know whether Xen has any limitation that support
memory-backend-memfd in QEMU or just implementation is not done yet?

Looking forward to your reply!

Thanks a lot,
Ray

Re: Random crash during guest start on x86

2022-07-26 Thread Bertrand Marquis




> On 25 Jul 2022, at 17:16, Jan Beulich  wrote:
> 
> On 25.07.2022 17:51, Bertrand Marquis wrote:
>> On our CI we have randomly a crash during guest boot on x86.
> 
> Afaict of a PV guest.
> 
>> We are running on qemu x86_64 using Xen staging.
> 
> Which may introduce unusual timing. An issue never hit on actual hardware
> _may_ (but doesn't have to be) one in qemu itself.
> 
>> The crash is happening randomly (something like 1 out of 20 times).
>> 
>> This is always happening on the first guest we start, we never got it after 
>> first guest was successfully started.
>> 
>> Please tell me if you need any other info.
>> 
>> Here is the guest kernel log:
>> [...]
>> [ 6.679020] general protection fault, maybe for address 0x8800:  [#1] 
>> PREEMPT SMP NOPTI
>> [ 6.679020] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.17.6 #1
>> [ 6.679020] RIP: e030:error_entry+0xaf/0xe0
>> [ 6.679020] Code: 29 89 c8 48 39 84 24 88 00 00 00 74 15 48 81 bc 24 88 00 
>> 00 00 63 10 e0 81 75 03 0f 01 f8 90 90 90 c3 48 89 8c 24 88 00 00 00 <0f> 01 
>> f8 90 90 90 eb 11 0f 20 d8 90 90 90 90 90 48 25 ff e7 ff ff
> 
> This is SWAPGS, which supposedly a PV guest should never hit. Data further
> down suggests the kernel is still in the process of patching alternatives,
> which may be the reason for the insn to still be there (being at a point
> where exceptions are still unexpected).

So the exception path is using alternative code ? Sounds logic with the error 
output.
But does explain the original error.

> 
>> [ 6.679020] RSP: e02b:82803a90 EFLAGS: 0046
>> [ 6.679020] RAX: 8800 RBX:  RCX: 81e00fa7
>> [ 6.679020] RDX:  RSI: 81e009f8 RDI: 00eb
>> [ 6.679020] RBP:  R08:  R09: 
>> [ 6.679020] R10:  R11:  R12: 
>> [ 6.679020] R13:  R14:  R15: 
>> [ 6.679020] FS: () GS:88801f20() 
>> knlGS:
>> [ 6.679020] CS: 1e030 DS:  ES:  CR0: 80050033
>> [ 6.679020] CR2:  CR3: 0280c000 CR4: 00050660
>> [ 6.679020] Call Trace:
>> [ 6.679020] 
>> [ 6.679020] RIP: e030:native_irq_return_iret+0x0/0x2
>> [ 6.679020] Code: 41 5d 41 5c 5d 5b 41 5b 41 5a 41 59 41 58 58 59 5a 5e 5f 
>> 48 83 c4 08 eb 0a 0f 1f 00 90 66 0f 1f 44 00 00 f6 44 24 20 04 75 02 <48> cf 
>> 57 0f 01 f8 eb 12 0f 20 df 90 90 90 90 90 48 81 e7 ff e7 ff
>> [ 6.679020] RSP: e02b:82803b48 EFLAGS: 0046 ORIG_RAX: 
>> e030
>> [ 6.679020] RAX: 8800 RBX: 82803be0 RCX: 81e00f95
>> [ 6.679020] RDX: 81e00f94 RSI: 81e00f95 RDI: 00eb
>> [ 6.679020] RBP: 00eb R08: 90001f0f R09: 0007
>> [ 6.679020] R10: 81e00f94 R11: 8285a6c0 R12: 
>> [ 6.679020] R13: 81e00f94 R14: 0006 R15: 0006
>> [ 6.679020] ? asm_exc_general_protection+0x8/0x30
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1c/0x27
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1c/0x27
>> [ 6.679020] RIP: e030:insn_get_opcode.part.0+0xab/0x180
>> [ 6.679020] Code: 00 00 8b 43 4c a9 c0 07 08 00 0f 84 bf 00 00 00 c6 43 1c 
>> 01 31 c0 5b 5d c3 83 e2 03 be 01 00 00 00 eb b7 89 ef e8 65 e4 ff ff <89> 43 
>> 4c a8 30 75 21 e9 8e 00 00 00 0f b6 7b 03 40 84 ff 75 73 8b
>> [ 6.679020] RSP: e02b:82803b70 EFLAGS: 0246
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>> [ 6.679020] insn_get_modrm+0x6c/0x120
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>> [ 6.679020] insn_get_sib+0x40/0x80
>> [ 6.679020] insn_get_displacement+0x82/0x100
>> [ 6.679020] insn_decode+0xf8/0x100
>> [ 6.679020] optimize_nops+0x60/0x1e0
>> [ 6.679020] ? rcu_nmi_exit+0x2b/0x140
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>> [ 6.679020] ? native_iret+0x3/0x7
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1c/0x27
>> [ 6.679020] ? restore_regs_and_return_to_kernel+0x1b/0x27
>> [ 6.679020] apply_alternatives+0x165/0x2e0
> 
> I have to admit that I'm a little lost with these "modern" stack traces,
> where contexts apparently switch without being clearly annotated. It is
> looking a little as if a #GP fault was happening somewhere here (hence
> the asm_exc_general_protection further up), but I cannot work out where
> (what insn) that would have come from.
> 
> You may want to add some debugging code to the hypervisor to tell you
> where exactly that #GP (if there is one in the first place) originates
> from. With that it may then become a little more clear what's actually
> going on (and why the behavior is random).

I will check what I

Re: [PATCH v9 6/8] xen/arm: unpopulate memory when domain is static

2022-07-26 Thread Jan Beulich

On 26.07.2022 04:57, Penny Zheng wrote:
>> -Original Message-
>> From: Jan Beulich 
>> Sent: Monday, July 25, 2022 11:36 PM
>>
>> On 20.07.2022 07:46, Penny Zheng wrote:
>>> Today when a domain unpopulates the memory on runtime, they will
>>> always hand the memory back to the heap allocator. And it will be a
>>> problem if domain is static.
>>>
>>> Pages as guest RAM for static domain shall be reserved to only this
>>> domain and not be used for any other purposes, so they shall never go
>>> back to heap allocator.
>>>
>>> This commit puts reserved pages on the new list resv_page_list only
>>> after having taken them off the "normal" list, when the last ref dropped.
>>
>> I guess this wording somehow relates to ...
>>
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -2674,10 +2674,14 @@ void free_domstatic_page(struct page_info
>>> *page)
>>>
>>>  drop_dom_ref = !domain_adjust_tot_pages(d, -1);
>>>
>>> -spin_unlock_recursive(>page_alloc_lock);
>>> -
>>>  free_staticmem_pages(page, 1, scrub_debug);
>>>
>>> +/* Add page on the resv_page_list *after* it has been freed. */
>>> +if ( !drop_dom_ref )
>>> +page_list_add_tail(page, >resv_page_list);
>>
>> ... the conditional used here. I cannot, however, figure why there is this
>> conditional (and said part of the description also doesn't help me figure it
>> out).
>>
> 
> I was thinking that if drop_dom_ref true, then later the whole domain struct
> will be released, then there is no need to add the page to d->resv_page_list

If that was the intention, then it should have been spelled out imo. But I
think it's not a good idea to skip the list addition in that one special
case, when keeping things uniform is actually cheaper _and_ more consistent.
In the end I expect sooner or later there'll be a desire to restart DomU-s
which have crashed, which would include re-using their designated static
memory. Then you want to avoid leaking pages like it would happen with your
approach.

Jan

[libvirt test] 171860: regressions - FAIL

2022-07-26 Thread osstest service owner

flight 171860 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/171860/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-amd64-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777
 build-armhf-libvirt   6 libvirt-buildfail REGR. vs. 151777

Tests which did not succeed, but are not blocking:
 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-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  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-raw   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-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

version targeted for testing:
 libvirt  30a067d94c87fd0d634486ebb2d2df98ce1d227c
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  746 days
Failing since151818  2020-07-11 04:18:52 Z  745 days  727 attempts
Testing same since   171860  2022-07-26 04:20:28 Z0 days1 attempts


People who touched revisions under test:
Adolfo Jayme Barrientos 
  Aleksandr Alekseev 
  Aleksei Zakharov 
  Amneesh Singh 
  Andika Triwidada 
  Andrea Bolognani 
  Andrew Melnychenko 
  Ani Sinha 
  Balázs Meskó 
  Barrett Schonefeld 
  Bastian Germann 
  Bastien Orivel 
  BiaoXiang Ye 
  Bihong Yu 
  Binfeng Wu 
  Bjoern Walk 
  Boris Fiuczynski 
  Brad Laue 
  Brian Turek 
  Bruno Haible 
  Chris Mayo 
  Christian Borntraeger 
  Christian Ehrhardt 
  Christian Kirbach 
  Christian Schoenebeck 
  Christophe Fergeau 
  Claudio Fontana 
  Cole Robinson 
  Collin Walling 
  Cornelia Huck 
  Cédric Bosdonnat 
  Côme Borsoi 
  Daniel Henrique Barboza 
  Daniel Letai 
  Daniel P. Berrange 
  Daniel P. Berrangé 
  David Michael 
  Didik Supriadi 
  dinglimin 
  Divya Garg 
  Dmitrii Shcherbakov 
  Dmytro Linkin 
  Eiichi Tsukata 
  Emilio Herrera 
  Eric Farman 
  Erik Skultety 
  Eugenio Pérez 
  Fabian Affolter 
  Fabian Freyer 
  Fabiano Fidêncio 
  Fangge Jin 
  Farhan Ali 
  Fedora Weblate Translation 
  Florian Schmidt 
  Franck Ridel 
  Gavi Teitz 
  gongwei 
  Guoyi Tu
  Göran Uddeborg 
  Halil Pasic 
  Han Han 
  Hao Wang 
  Haonan Wang 
  Hela Basa 
  Helmut Grohne 
  Hiroki Narukawa 
  Hyman Huang(黄勇) 
  Ian Wienand 
  Ioanna Alifieraki 
  Ivan Teterevkov 
  Jakob Meng 
  Jamie Strandboge 
  Jamie Strandboge 
  Jan Kuparinen 
  jason lee 
  Jean-Baptiste Holcroft 
  Jia Zhou 
  Jianan Gao 
  Jim Fehlig 
  Jin Yan 
  Jing Qi 
  Jinsheng Zhang 
  Jiri Denemark 
  Joachim Falk 
  John Ferlan 
  John Levon 
  John Levon 
  Jonathan Watt 
  Jonathon Jongsma 
  Julio Faracco 
  Justin Gatzen 
  Ján Tomko 
  Kashyap Chamarthy 
  Kevin Locke 
  Kim InSoo 
  Koichi Murase 
  Kristina Hanicova 
  Laine Stump 
  Laszlo Ersek 
  Lee Yarwood 
  Lei Yang 
  Lena Voytek 
  Liang Yan 
  Liang Yan 
  Liao Pingfang 
  Lin Ma 
  Lin Ma 
  Lin Ma 
  Liu Yiding 
  Lubomir Rintel 
  Luke Yue 
  Luyao Zhong 
  luzhipeng 
  Marc Hartmayer 
  Marc-André Lureau 
  Marek Marczykowski-Górecki 
  Mark Mielke 
  Markus Schade 
  Martin Kletzander 
  Martin Pitt 
  Masayoshi Mizuma 
  Matej Cepl 
  Matt Coleman 
  Matt Coleman 
  Mauro Matteo Cascella 
  Max Goodhart 
  Maxim Nestratov 
  Meina Li 
  Michal Privoznik 
  Michał Smyk 
  Milo Casagrande 
  minglei.liu 
  Moshe Levi 
  Moteen Shah 
  Moteen Shah 
  Muha Aliss 
  Nathan 
  Neal Gompa 
  Nick Chevsky 
  Nick Shyrokovskiy 
  Nickys Music Group 
  Nico Pache 
  Nicolas Lécureuil 
  Nicolas Lécureuil 
  Nikolay Shirokovskiy 
  Nikolay Shirokovskiy 
  Nikolay Shirokovskiy 
  Niteesh Dubey 
  Olaf Hering 
  Olesya Gerasimenko 
  Or Ozeri 
  Orion Poplawski 
  Pany 
  Paolo Bonzini 
  Patrick Magauran

Re: [PATCH] x86/pv: Inject #GP for implicit grant unmaps

2022-07-26 Thread Jan Beulich

On 25.07.2022 19:50, Andrew Cooper wrote:
> This is a debug behaviour to identify buggy kernels.  Crashing the domain is
> the most unhelpful thing to do, because it discards the relevant context.
> 
> Instead, inject #GP[0] like other permission errors in x86.  In particular,
> this lets the kernel provide a backtrace that's actually helpful to a
> developer trying to figure out what's going wrong.
> 
> As a bugfix, this always injects #GP[0] to current, not l1e_owner.  It is not
> l1e_owner's fault if dom0 using superpowers triggers an implicit unmap.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich 

Albeit preferably with ...

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -1232,7 +1232,7 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct domain 
> *l1e_owner)
>  gdprintk(XENLOG_WARNING,
>   "Attempt to implicitly unmap a granted PTE %" PRIpte "\n",
>   l1e_get_intpte(l1e));
> -domain_crash(l1e_owner);
> +pv_inject_hw_exception(TRAP_gp_fault, 0);
>  }
>  #endif

... the gdprintk() adjusted to also log l1e_owner.

Jan

Re: [PATCH v3] livepatch: create-diff-object: Check that the section has a secsym

2022-07-26 Thread Jan Beulich

On 25.07.2022 19:13, Sarah Newman wrote:
> A STT_SECTION symbol is not needed if if it is not used as a relocation
> target. Therefore, a section, in this case a debug section, may not have
> a secsym associated with it.
> 
> Signed-off-by: Bill Wendling 

Hmm - this wasn't here before. Does this then suggest the patch also
wants to be marked From: Bill?

> Origin: https://github.com/dynup/kpatch.git ba3defa06073
> Signed-off-by: Sarah Newman 
> Reviewed-by: Ross Lagerwall 

Sigh. I had given R-b on v1 as well. Actually I had meant to commit this
yesterday (with all adjustments made), but as it turns out committers
can't commit to that tree. So it'll be up to Ross or Konrad to actually
take care of this.

Jan

Re: [PATCH 3/4] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

2022-07-26 Thread Jan Beulich

On 26.07.2022 02:33, Stefano Stabellini wrote:
> On Mon, 25 Jul 2022, Xenia Ragiadakou wrote:
>> On 7/25/22 09:32, Jan Beulich wrote:
>>> On 24.07.2022 19:20, Xenia Ragiadakou wrote:
 On 7/7/22 10:55, Jan Beulich wrote:
> On 07.07.2022 09:27, Xenia Ragiadakou wrote:
>> On 7/6/22 11:51, Jan Beulich wrote:
>>> On 06.07.2022 10:43, Xenia Ragiadakou wrote:
 On 7/6/22 10:10, Jan Beulich wrote:
> On 05.07.2022 23:02, Xenia Ragiadakou wrote:
>> The function idle_loop() is referenced only in domain.c.
>> Change its linkage from external to internal by adding the
>> storage-class
>> specifier static to its definitions.
>>
>> Since idle_loop() is referenced only in inline assembly, add
>> the 'used'
>> attribute to suppress unused-function compiler warning.
>
> While I see that Julien has already acked the patch, I'd like to
> point
> out that using __used here is somewhat bogus. Imo the better
> approach
> is to properly make visible to the compiler that the symbol is
> used by
> the asm(), by adding a fake(?) input.

 I 'm afraid I do not understand what do you mean by "adding a
 fake(?)
 input". Can you please elaborate a little on your suggestion?
>>>
>>> Once the asm() in question takes the function as an input, the
>>> compiler
>>> will know that the function has a user (unless, of course, it finds
>>> a
>>> way to elide the asm() itself). The "fake(?)" was because I'm not
>>> deeply
>>> enough into Arm inline assembly to know whether the input could then
>>> also be used as an instruction operand (which imo would be
>>> preferable) -
>>> if it can't (e.g. because there's no suitable constraint or operand
>>> modifier), it still can be an input just to inform the compiler.
>>
>> According to the following statement, your approach is the recommended
>> one:
>> "To prevent the compiler from removing global data or functions which
>> are referenced from inline assembly statements, you can:
>> -use __attribute__((used)) with the global data or functions.
>> -pass the reference to global data or functions as operands to inline
>> assembly statements.
>> Arm recommends passing the reference to global data or functions as
>> operands to inline assembly statements so that if the final image does
>> not require the inline assembly statements and the referenced global
>> data or function, then they can be removed."
>>
>> IIUC, you are suggesting to change
>> asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" )
>> into
>> asm volatile ("mov sp,%0; b %1" : : "r" (stack), "S" (fn) : "memory"
>> );
>
> Yes, except that I can't judge about the "S" constraint.
>

 This constraint does not work for arm32. Any other thoughts?

 Another way, as Jan suggested, is to pass the function as a 'fake'
 (unused) input.
 I 'm suspecting something like the following could work
 asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack), "X" (fn) :
 "memory")
 What do you think?
>>>
>>> Well, yes, X should always be a fallback option. But I said already when
>>> you suggested S that I can't judge about its use, so I guess I'm the
>>> wrong one to ask. Even more so that you only say "does not work", without
>>> any details ...
>>>
>>
>> The question is addressed to anyone familiar with arm inline assembly.
>> I added the arm maintainers as primary recipients to this email to make this
>> perfectly clear.
>>
>> When cross-compiling Xen on x86 for arm32 with
>> asm volatile ("mov sp,%0; b %1" : : "r" (stack), "S" (fn) : "memory" );
>> compilation fails with the error: impossible constraint in ‘asm'
> 
> Unfortunately looking at the GCC manual pages [1], it doesn't seem to be
> possible. The problem is that the definition of "S" changes between
> aarch64 and arm (the 32-bit version).
> 
> For aarch64:
> 
> S   An absolute symbolic address or a label reference
> 
> This is what we want. For arm instead:
> 
> S   A symbol in the text segment of the current file
> 
> This is not useful for what we need to do here. As far as I can tell,
> there is no other way in GCC assembly inline for arm to do this.
> 
> So we have 2 choices: we use the __used keyword as Xenia did in this
> patch. Or we move the implementation of switch_stack_and_jump in
> assembly. I attempted a prototype of the latter to see how it would come
> out, see below.
> 
> I don't like it very much. I prefer the version with __used that Xenia
> had in this patch. But I am OK either way.

You forgot the imo better intermediate option of using the "X" constraint.

Jan

Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE

2022-07-26 Thread Michel Lespinasse

On Wed, Jun 08, 2022 at 04:27:27PM +0200, Peter Zijlstra wrote:
> Commit c227233ad64c ("intel_idle: enable interrupts before C1 on
> Xeons") wrecked intel_idle in two ways:
> 
>  - must not have tracing in idle functions
>  - must return with IRQs disabled
> 
> Additionally, it added a branch for no good reason.
> 
> Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on Xeons")
> Signed-off-by: Peter Zijlstra (Intel) 

After this change was introduced, I am seeing "WARNING: suspicious RCU
usage" when booting a kernel with debug options compiled in. Please
see the attached dmesg output. The issue starts with commit 32d4fd5751ea
and is still present in v5.19-rc8.

I'm not sure, is this too late to fix or revert in v5.19 final ?

Thanks,

--
Michel "walken" Lespinasse
[0.00] microcode: microcode updated early to revision 0x2006d05, date = 
2021-11-13
[0.00] Linux version 5.19.0-rc8-test-3-ge3a8d97e6a35 (walken@zeus) 
(gcc (Debian 8.3.0-6) 8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #1 SMP 
PREEMPT_DYNAMIC Mon Jul 25 00:32:16 PDT 2022
[0.00] Command line: 
BOOT_IMAGE=/vmlinuz-5.19.0-rc8-test-3-ge3a8d97e6a35 
root=/dev/mapper/budai--vg-root ro consoleblank=600 quiet
[0.00] KERNEL supported cpus:
[0.00]   Intel GenuineIntel
[0.00]   AMD AuthenticAMD
[0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point 
registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x008: 'MPX bounds registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x010: 'MPX CSR'
[0.00] x86/fpu: Supporting XSAVE feature 0x020: 'AVX-512 opmask'
[0.00] x86/fpu: Supporting XSAVE feature 0x040: 'AVX-512 Hi256'
[0.00] x86/fpu: Supporting XSAVE feature 0x080: 'AVX-512 ZMM_Hi256'
[0.00] x86/fpu: xstate_offset[2]:  576, xstate_sizes[2]:  256
[0.00] x86/fpu: xstate_offset[3]:  832, xstate_sizes[3]:   64
[0.00] x86/fpu: xstate_offset[4]:  896, xstate_sizes[4]:   64
[0.00] x86/fpu: xstate_offset[5]:  960, xstate_sizes[5]:   64
[0.00] x86/fpu: xstate_offset[6]: 1024, xstate_sizes[6]:  512
[0.00] x86/fpu: xstate_offset[7]: 1536, xstate_sizes[7]: 1024
[0.00] x86/fpu: Enabled xstate features 0xff, context size is 2560 
bytes, using 'compacted' format.
[0.00] signal: max sigframe size: 3632
[0.00] BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x0003efff] usable
[0.00] BIOS-e820: [mem 0x0003f000-0x0003] reserved
[0.00] BIOS-e820: [mem 0x0004-0x0009] usable
[0.00] BIOS-e820: [mem 0x000a-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0x62a8efff] usable
[0.00] BIOS-e820: [mem 0x62a8f000-0x6aef4fff] reserved
[0.00] BIOS-e820: [mem 0x6aef5000-0x6b0c2fff] ACPI data
[0.00] BIOS-e820: [mem 0x6b0c3000-0x6be0dfff] ACPI NVS
[0.00] BIOS-e820: [mem 0x6be0e000-0x6c7f1fff] reserved
[0.00] BIOS-e820: [mem 0x6c7f2000-0x6c8e5fff] type 20
[0.00] BIOS-e820: [mem 0x6c8e6000-0x6c8e6fff] reserved
[0.00] BIOS-e820: [mem 0x6c8e7000-0x6c9a7fff] type 20
[0.00] BIOS-e820: [mem 0x6c9a8000-0x6e6f] usable
[0.00] BIOS-e820: [mem 0x6e70-0x8fff] reserved
[0.00] BIOS-e820: [mem 0xfd00-0xfe010fff] reserved
[0.00] BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
[0.00] BIOS-e820: [mem 0xfed0-0xfed00fff] reserved
[0.00] BIOS-e820: [mem 0xfed2-0xfed44fff] reserved
[0.00] BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
[0.00] BIOS-e820: [mem 0xff00-0x] reserved
[0.00] BIOS-e820: [mem 0x0001-0x00107fff] usable
[0.00] NX (Execute Disable) protection: active
[0.00] efi: EFI v2.70 by American Megatrends
[0.00] efi: TPMFinalLog=0x6bd97000 ACPI=0x6b0c2000 ACPI 2.0=0x6b0c2014 
SMBIOS=0x6c47b000 SMBIOS 3.0=0x6c47a000 MEMATTR=0x5d04d018 ESRT=0x604e0718 
[0.00] SMBIOS 3.0.0 present.
[0.00] DMI: LENOVO 30BFS44D00/1036, BIOS S03KT51A 01/17/2022
[0.00] tsc: Detected 3700.000 MHz processor
[0.00] tsc: Detected 3699.850 MHz TSC
[0.000278] e820: update [mem 0x-0x0fff] usable ==> reserved
[0.000281] e820: remove [mem 0x000a-0x000f] usable
[0.000289] last_pfn = 0x108 max_arch_pfn = 0x4
[0.000397] x86/PAT: Configuration [0-7]: WB  WC  UC- UC  WB  WP  UC- WT  
[0.001626] e820: update [mem

Re: [PATCH v3 00/10] Add Xue - console over USB 3 Debug Capability

2022-07-26 Thread Jan Beulich

On 26.07.2022 05:23, Marek Marczykowski-Górecki wrote:
> This is integration of https://github.com/connojd/xue into mainline Xen.
> This patch series includes several patches that I made in the process, some 
> are
> very loosely related.
> 
> The driver developed by Connor supports console via USB3 debug capability. The
> capability is designed to operate mostly independently of normal XHCI driver,
> so this patch series allows dom0 to drive standard USB3 controller part, while
> Xen uses DbC for console output.
> 
> Changes since RFC:
>  - move the driver to xue.c, remove non-Xen parts, remove now unneeded 
> abstraction
>  - adjust for Xen code style
>  - build for x86 only
>  - drop patch hidding the device from dom0
> Changes since v1:
>  - drop ehci patch - already applied
>  - adjust for review comments from Jan (see changelogs in individual patches)
> Changes since v2:
>  - add runtime option to share (or not) the controller with dom0 or other 
> domains
>  - add RX support
>  - several smaller changes according to review comments
> 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Jan Beulich 
> Cc: Julien Grall 
> Cc: Stefano Stabellini 
> Cc: Wei Liu 
> Cc: "Roger Pau Monné" 
> Cc: Paul Durrant 
> Cc: Kevin Tian 
> Cc: Connor Davis 
> 
> Connor Davis (1):
>   drivers/char: Add support for USB3 DbC debugger

I notice this patch was sent twice, about 4 min ahead of the full set.
I take it that the set having come close together is the definitive
one, and that one extra copy is the one to be ignored.

Jan

Re: [PATCH v4 2/2] xen/arm: add FF-A mediator

2022-07-26 Thread Jens Wiklander

Hi Julien,

On Fri, Jul 8, 2022 at 3:41 PM Julien Grall  wrote:
>
> Hi Jens,
>
> I haven't checked whether the FFA driver is complaint with the spec. I
> mainly checked whether the code makes sense from Xen PoV.
>
> This is a fairly long patch to review. So I will split my review in
> multiple session/e-mail.
>
> On 22/06/2022 14:42, Jens Wiklander wrote:
> > Adds a FF-A version 1.1 [1] mediator to communicate with a Secure
> > Partition in secure world.
> >
> > The implementation is the bare minimum to be able to communicate with
> > OP-TEE running as an SPMC at S-EL1.
> >
> > This is loosely based on the TEE mediator framework and the OP-TEE
> > mediator.
> >
> > [1] https://developer.arm.com/documentation/den0077/latest
> > Signed-off-by: Jens Wiklander 
> > ---
> >   SUPPORT.md|7 +
> >   tools/libs/light/libxl_arm.c  |3 +
> >   tools/libs/light/libxl_types.idl  |1 +
> >   tools/xl/xl_parse.c   |3 +
>
> These changes would need an ack from the toolstack maintainers.

OK, I'll keep them in CC.

>
> >   xen/arch/arm/Kconfig  |   11 +
> >   xen/arch/arm/Makefile |1 +
> >   xen/arch/arm/domain.c |   10 +
> >   xen/arch/arm/domain_build.c   |1 +
> >   xen/arch/arm/ffa.c| 1683 +
> >   xen/arch/arm/include/asm/domain.h |4 +
> >   xen/arch/arm/include/asm/ffa.h|   71 ++
> >   xen/arch/arm/vsmc.c   |   17 +-
> >   xen/include/public/arch-arm.h |2 +
> >   13 files changed, 1811 insertions(+), 3 deletions(-)
> >   create mode 100644 xen/arch/arm/ffa.c
> >   create mode 100644 xen/arch/arm/include/asm/ffa.h
> >
> > diff --git a/SUPPORT.md b/SUPPORT.md
> > index 70e98964cbc0..215bb3c9043b 100644
> > --- a/SUPPORT.md
> > +++ b/SUPPORT.md
> > @@ -785,6 +785,13 @@ that covers the DMA of the device to be passed through.
> >
> >   No support for QEMU backends in a 16K or 64K domain.
> >
> > +### ARM: Firmware Framework for Arm A-profile (FF-A) Mediator
> > +
> > +Status, Arm64: Tech Preview
> > +
> > +There are still some code paths where a vCPU may hog a pCPU longer than
> > +necessary. The FF-A mediator is not yet implemented for Arm32.
> > +
> >   ### ARM: Guest Device Tree support
> >
> >   Status: Supported
> > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> > index eef1de093914..a985609861c7 100644
> > --- a/tools/libs/light/libxl_arm.c
> > +++ b/tools/libs/light/libxl_arm.c
> > @@ -101,6 +101,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> >   return ERROR_FAIL;
> >   }
> >
> > +config->arch.ffa_enabled =
> > +libxl_defbool_val(d_config->b_info.arch_arm.ffa_enabled);
> > +
> >   return 0;
> >   }
> >
> > diff --git a/tools/libs/light/libxl_types.idl 
> > b/tools/libs/light/libxl_types.idl
> > index 2a42da2f7d78..bf4544bef399 100644
> > --- a/tools/libs/light/libxl_types.idl
> > +++ b/tools/libs/light/libxl_types.idl
> > @@ -646,6 +646,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> >
> >   ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
> >  ("vuart", libxl_vuart_type),
> > +   ("ffa_enabled", libxl_defbool),
>
> This needs to be accompagnied with a define LIBXL_HAVE_* in
> tools/include/libxl.h. Please see the examples in the header.

OK, I'll add something. I'm not entirely sure how this is used so I'm
afraid it will be a bit of Cargo Cult programming from my side.

>
> > ])),
> >   ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
> > ])),
> > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> > index b98c0de378b6..e0e99ed8d2b1 100644
> > --- a/tools/xl/xl_parse.c
> > +++ b/tools/xl/xl_parse.c
> > @@ -2746,6 +2746,9 @@ skip_usbdev:
> >   exit(-ERROR_FAIL);
> >   }
> >   }
> > +libxl_defbool_setdefault(_info->arch_arm.ffa_enabled, false);
> > +xlu_cfg_get_defbool(config, "ffa_enabled",
> > +_info->arch_arm.ffa_enabled, 0);
>
> This option needs to be documented in docs/man/xl.cfg.5.pod.in.

OK

>
> >
> >   parse_vkb_list(config, d_config);
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index be9eff014120..e57e1d3757e2 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -139,6 +139,17 @@ config TEE
> >
> >   source "arch/arm/tee/Kconfig"
> >
> > +config FFA
> > + bool "Enable FF-A mediator support" if EXPERT
> > + default n
> > + depends on ARM_64
> > + help
> > +   This option enables a minimal FF-A mediator. The mediator is
> > +   generic as it follows the FF-A specification [1], but it only
> > +   implements a small subset of the specification.
>
> Where would a user be able to find which subset of the specification
> that Xen implements?

I'll add a bit

Re: [PATCH] page-alloc: fix initialization of cross-node regions

2022-07-26 Thread Jan Beulich

On 25.07.2022 20:54, Andrew Cooper wrote:
> On 25/07/2022 14:10, Jan Beulich wrote:
>> Quite obviously to determine the split condition successive pages'
>> attributes need to be evaluated, not always those of the initial page.
>>
>> Fixes: 72b02bc75b47 ("xen/heap: pass order to free_heap_pages() in heap 
>> init")
>> Signed-off-by: Jan Beulich 
>> ---
>> Part of the problem was already introduced in 24a53060bd37 ("xen/heap:
>> Split init_heap_pages() in two"), but there it was still benign.
> 
> This also fixes the crash that XenRT found on loads of hardware, which
> looks something like:
> 
> (XEN) NUMA: Allocated memnodemap from 105bc81000 - 105bc92000
> (XEN) NUMA: Using 8 for the hash shift.
> (XEN) Early fatal page fault at e008:82d04022ae1e
> (cr2=00b8, ec=0002)
> (XEN) [ Xen-4.17.0  x86_64  debug=y  Not tainted ]
> (XEN) CPU:    0
> (XEN) RIP:    e008:[]
> common/page_alloc.c#free_heap_pages+0x2dd/0x850
> ...
> (XEN) Xen call trace:
> (XEN)    [] R
> common/page_alloc.c#free_heap_pages+0x2dd/0x850
> (XEN)    [] F
> common/page_alloc.c#init_heap_pages+0x55f/0x720
> (XEN)    [] F end_boot_allocator+0x187/0x1e7
> (XEN)    [] F __start_xen+0x1a06/0x2779
> (XEN)    [] F __high_start+0x94/0xa0
> 
> Debugging shows that it's always a block which crosses node 0 and 1,
> where avail[1] has yet to be initialised.
> 
> What I'm confused by is how this manages to manifest broken swiotlb
> issues without Xen crashing.

I didn't debug this in detail since I had managed to spot the issue
by staring at the offending patch, but from the observations some
of node 1's memory was actually accounted to node 0 (incl off-by-
65535 node_need_scrub[] values for both nodes), so I would guess
avail[1] simply wasn't accessed before being set up in my case.

Jan

81 matches

Mail list logo