Re: [PATCH] xen: introduce XENFEAT_direct_mapped and XENFEAT_not_direct_mapped

[Jan Beulich]

On 25.02.2021 02:22, Stefano Stabellini wrote:
> --- a/xen/include/public/features.h
> +++ b/xen/include/public/features.h
> @@ -114,6 +114,13 @@
>   */
>  #define XENFEAT_linux_rsdp_unrestricted   15
>  
> +/*
> + * A direct-mapped (or 1:1 mapped) domain is a domain for which its
> + * local pages have gfn == mfn.
> + */
> +#define XENFEAT_not_direct_mapped   16
> +#define XENFEAT_direct_mapped   17

Why two new values? Absence of XENFEAT_direct_mapped requires
implying not-direct-mapped by the consumer anyway, doesn't it?

Further, quoting xen/mm.h: "For a non-translated guest which
is aware of Xen, gfn == mfn." This to me implies that PV would
need to get XENFEAT_direct_mapped set; not sure whether this
simply means x86'es is_domain_direct_mapped() is wrong, but if
it is, uses elsewhere in the code would likely need changing.

Also, nit: Please keep the right sides aligned with #define-s
higher up in the file.

Jan

Re: [PATCH 0/2] hvmloader: drop usage of system headers

[Jan Beulich]

On 24.02.2021 21:08, Andrew Cooper wrote:
> On 24/02/2021 10:26, Roger Pau Monne wrote:
>> Hello,
>>
>> Following two patches aim to make hvmloader standalone, so that it don't
>> try to use system headers. It shouldn't result in any functional
>> change.
>>
>> Thanks, Roger.
> 
> After some experimentation in the arch container
> 
> Given foo.c as:
> 
> #include 
> 
> extern uint64_t bar;
> uint64_t foo(void)
> {
>     return bar;
> }
> 
> int main(void)
> {
>     return 0;
> }
> 
> The preprocessed form with `gcc -m32 -E` is:
> 
> # 1 "foo.c"
> # 1 ""
> # 1 ""
> # 31 ""
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 32 "" 2
> # 1 "foo.c"
> # 1 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint.h" 1 3 4
> # 9 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint.h" 3 4
> # 1 "/usr/include/stdint.h" 1 3 4
> # 26 "/usr/include/stdint.h" 3 4
> # 1 "/usr/include/bits/libc-header-start.h" 1 3 4
> # 33 "/usr/include/bits/libc-header-start.h" 3 4
> # 1 "/usr/include/features.h" 1 3 4
> # 450 "/usr/include/features.h" 3 4
> # 1 "/usr/include/sys/cdefs.h" 1 3 4
> # 452 "/usr/include/sys/cdefs.h" 3 4
> # 1 "/usr/include/bits/wordsize.h" 1 3 4
> # 453 "/usr/include/sys/cdefs.h" 2 3 4
> # 1 "/usr/include/bits/long-double.h" 1 3 4
> # 454 "/usr/include/sys/cdefs.h" 2 3 4
> # 451 "/usr/include/features.h" 2 3 4
> # 474 "/usr/include/features.h" 3 4
> # 1 "/usr/include/gnu/stubs.h" 1 3 4
> 
> # 1 "/usr/include/gnu/stubs-32.h" 1 3 4
> # 8 "/usr/include/gnu/stubs.h" 2 3 4
> # 475 "/usr/include/features.h" 2 3 4
> # 34 "/usr/include/bits/libc-header-start.h" 2 3 4
> # 27 "/usr/include/stdint.h" 2 3 4
> # 1 "/usr/include/bits/types.h" 1 3 4
> # 27 "/usr/include/bits/types.h" 3 4
> # 1 "/usr/include/bits/wordsize.h" 1 3 4
> # 28 "/usr/include/bits/types.h" 2 3 4
> # 1 "/usr/include/bits/timesize.h" 1 3 4
> # 29 "/usr/include/bits/types.h" 2 3 4
> 
> # 31 "/usr/include/bits/types.h" 3 4
> typedef unsigned char __u_char;
> ...
> 
> while the freestanding form with `gcc -ffreestanding -m32 -E` is:
> 
> # 1 "foo.c"
> # 1 ""
> # 1 ""
> # 1 "foo.c"
> # 1 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint.h" 1 3 4
> # 11 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint.h" 3 4
> # 1 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint-gcc.h" 1 3 4
> # 34 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint-gcc.h" 3 4
> 
> # 34 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint-gcc.h" 3 4
> typedef signed char int8_t;
> ...
> 
> 
> I can compile and link trivial programs using -m32 and stdint.h without
> any issue.
> 
> Clearly something more subtle is going on with our choice of options
> when compiling hvmloader, but it certainly looks like stdint.h is fine
> to use in the way we want to use it.

Why "more subtle"? All we're lacking is -ffreestanding. The
question is whether it is an acceptably risky thing to do at
this point in the release cycle to add the option.

Jan

[linux-linus test] 159646: regressions - FAIL

[osstest service owner]

flight 159646 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/159646/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-xsm7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. 
vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-examine   6 xen-install  fail REGR. vs. 152332
 test-amd64-i386-libvirt   7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332
 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-coresched-i386-xl  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-libvirt-xsm   7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-freebsd10-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-pvshim 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-raw7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-freebsd10-i386  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-shadow 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. 
vs. 152332
 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332
 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332
 test-arm64-arm64-xl-xsm  10 host-ping-check-xen  fail REGR. vs. 152332
 test-arm64-arm64-libvirt-xsm 10 host-ping-check-xen  fail REGR. vs. 152332
 test-arm64-arm64-examine  8 reboot   fail REGR. vs. 152332
 test-amd64-amd64-i386-pvgrub 19 guest-localmigrate/x10   fail REGR. vs. 152332
 test-amd64-amd64-amd64-pvgrub 20 guest-stop  fail REGR. vs. 152332
 test-arm64-arm64-xl-seattle  10 host-ping-check-xen  fail REGR. vs. 152332
 test-arm64-arm64-xl-credit2   8 xen-boot fail REGR. vs. 152332

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl  11 leak-check/basis(11)fail blocked in 152332
 test-arm64-arm64-xl-credit1  11 leak-check/basis(11)fail blocked in 152332
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 152332
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 152332
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152332
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-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-amd64-libvirt-vhd 14 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-libvirt 15 migrate-support-check  

Re: [PATCH] xen-netback: correct success/error reporting for the SKB-with-fraglist case

[Jan Beulich]

On 24.02.2021 17:39, Paul Durrant wrote:
> On 23/02/2021 16:29, Jan Beulich wrote:
>> When re-entering the main loop of xenvif_tx_check_gop() a 2nd time, the
>> special considerations for the head of the SKB no longer apply. Don't
>> mistakenly report ERROR to the frontend for the first entry in the list,
>> even if - from all I can tell - this shouldn't matter much as the overall
>> transmit will need to be considered failed anyway.
>>
>> Signed-off-by: Jan Beulich 
>>
>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -499,7 +499,7 @@ check_frags:
>>   * the header's copy failed, and they are
>>   * sharing a slot, send an error
>>   */
>> -if (i == 0 && sharedslot)
>> +if (i == 0 && !first_shinfo && sharedslot)
>>  xenvif_idx_release(queue, pending_idx,
>> XEN_NETIF_RSP_ERROR);
>>  else
>>
> 
> I think this will DTRT, but to my mind it would make more sense to clear 
> 'sharedslot' before the 'goto check_frags' at the bottom of the function.

That was my initial idea as well, but
- I think it is for a reason that the variable is "const".
- There is another use of it which would then instead need further
  amending (and which I believe is at least part of the reason for
  the variable to be "const").

Jan

[libvirt test] 159656: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-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-arm64-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-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-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a

version targeted for testing:
 libvirt  a0cef16787930c810263f1edd057e038cb6406e3
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  230 days
Failing since151818  2020-07-11 04:18:52 Z  229 days  222 attempts
Testing same since   159656  2021-02-25 04:18:57 Z0 days1 attempts


People who touched revisions under test:
  Adolfo Jayme Barrientos 
  Aleksandr Alekseev 
  Andika Triwidada 
  Andrea Bolognani 
  Balázs Meskó 
  Barrett Schonefeld 
  Bastien Orivel 
  BiaoXiang Ye 
  Bihong Yu 
  Binfeng Wu 
  Boris Fiuczynski 
  Brian Turek 
  Bruno Haible 
  Christian Ehrhardt 
  Christian Schoenebeck 
  Cole Robinson 
  Collin Walling 
  Cornelia Huck 
  Cédric Bosdonnat 
  Côme Borsoi 
  Daniel Henrique Barboza 
  Daniel Letai 
  Daniel P. Berrange 
  Daniel P. Berrangé 
  Dmytro Linkin 
  Eiichi Tsukata 
  Erik Skultety 
  Fabian Affolter 
  Fabian Freyer 
  Fangge Jin 
  Farhan Ali 
  Fedora Weblate Translation 
  gongwei 
  Guoyi Tu
  Göran Uddeborg 
  Halil Pasic 
  Han Han 
  Hao Wang 
  Helmut Grohne 
  Ian Wienand 
  Jakob Meng 
  Jamie Strandboge 
  Jamie Strandboge 
  Jan Kuparinen 
  Jean-Baptiste Holcroft 
  Jianan Gao 
  Jim Fehlig 
  Jin Yan 
  Jiri Denemark 
  John Ferlan 
  Jonathan Watt 
  Jonathon Jongsma 
  Julio Faracco 
  Ján Tomko 
  Kashyap Chamarthy 
  Kevin Locke 
  Kristina Hanicova 
  Laine Stump 
  Laszlo Ersek 
  Liao Pingfang 
  Lin Ma 
  Lin Ma 
  Lin Ma 
  Marc Hartmayer 
  Marc-André Lureau 
  Marek Marczykowski-Górecki 
  Markus Schade 
  Martin Kletzander 
  Masayoshi Mizuma 
  Matt Coleman 
  Matt Coleman 
  Mauro Matteo Cascella 
  Meina Li 
  Michal Privoznik 
  Michał Smyk 
  Milo Casagrande 
  Moshe Levi 
  Muha Aliss 
  Neal Gompa 
  Nick Shyrokovskiy 
  Nickys Music Group 
  Nico Pache 
  Nikolay Shirokovskiy 
  Olaf Hering 
  Olesya Gerasimenko 
  Orion Poplawski 
  Pany 
  Patrick Magauran 
  Paulo de Rezende Pinatti 
  Pavel Hrdina 
  Peng Liang 
  Peter Krempa 
  Pino Toscano 
  Pino Toscano 
  Piotr Drąg 
  Prathamesh Chavan 
  Ricky Tigg 
  Roman Bogorodskiy 
  Roman Bolshakov 
  Ryan Gahagan 
  Ryan Schmidt 
  Sam Hartman 
  Scott Shambarger 
  Sebastian Mitterle 
  Shalini Chellathurai Saroja 
  Shaojun Yang 
  Shi Lei 
  Simon Gaiser 
  Stefan Bader 
  Stefan Berger 
  Stefan Berger 
  Szymon Scholz 
  Thomas Huth 
  Tim Wiederhake 
  Tomáš Golembiovský 
  Tomáš Janoušek 
  Tuguoyi 
  Ville Skyttä 
  Wang Xin 
  Weblate 
  Yalei Li <274268...@qq.com>
  Yalei Li 
  Yang Hang 
  Yanqiu Zhang 
  Yi Li 
  Yi Wang 
  Yuri Chornoivan 
  Zheng Chuan 
  zhenwei pi 
  Zhenyu Zheng 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  fail
 build-arm64-libvirt 

[4.15] Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Jan Beulich]

On 24.02.2021 18:17, Andrew Cooper wrote:
> On 23/02/2021 07:53, Jan Beulich wrote:
>> On 22.02.2021 17:36, Andrew Cooper wrote:
>>> On 19/02/2021 08:09, Jan Beulich wrote:
 --- a/xen/arch/x86/Makefile
 +++ b/xen/arch/x86/Makefile
 @@ -123,8 +123,13 @@ ifneq ($(efi-y),)
  # Check if the compiler supports the MS ABI.
  export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o 
 efi/check.o 2>/dev/null && echo y)
  # Check if the linker supports PE.
 -XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep 
 --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
 +EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 
 --strip-debug
 +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o 
 efi/check.efi efi/check.o 2>/dev/null && echo y))
  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 +# Check if the linker produces fixups in PE by default (we need to 
 disable it doing so for now).
 +XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
 + $(shell $(LD) $(EFI_LDFLAGS) 
 --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
 + echo --disable-reloc-section))
>>> Why does --strip-debug move?
>> -S and --strip-debug are the same. I'm simply accumulating in
>> EFI_LDFLAGS all that's needed for the use in the probing construct.
> 
> Oh ok.
> 
> It occurs to me that EFI_LDFLAGS now only gets started in an ifneq
> block, but appended to later on while unprotected.  That said, I'm
> fairly sure it is only consumed inside a different ifeq section, so I
> think there is a reasonable quantity of tidying which ought to be done here.
> 
>> Also I meanwhile have a patch to retain debug info, for which this
>> movement turns out to be a prereq. (I've yet to test that the
>> produced binary actually works, and what's more I first need to get
>> a couple of changes accepted into binutils for the linker to actually
>> cope.)
>>
>>> What's wrong with $(call ld-option ...) ?  Actually, lots of this block
>>> of code looks to be opencoding of standard constructs.
>> It looks like ld-option could indeed be used here (there are marginal
>> differences which are likely acceptable), despite its brief comment
>> talking of just "flag" (singular, plus not really covering e.g. input
>> files).
>>
>> But:
>> - It working differently than cc-option makes it inconsistent to
>>   use (the setting of XEN_BUILD_EFI can't very well be switched to
>>   use cc-option); because of this I'm not surprised that we have
>>   only exactly one use right now in the tree.
>> - While XEN_BUILD_PE wants to be set to "y", for XEN_NO_PE_FIXUPS
>>   another transformation would then be necessary to translate "y"
>>   into "--disable-reloc-section".
>> - Do you really suggest to re-do this at this point in the release
>>   cycle?
> 
> I'm looking to prevent this almost-incompressible mess from getting worse.
> 
> But I suppose you want this to backport, so I suppose it ought to be
> minimally invasive.
> 
> Acked-by: Andrew Cooper 

Since getting Andrew's ack has taken across the firm-freeze boundary,
may I ask for a release-ack here? As noted this change (alongside
the earlier one) will want backporting, perhaps even to security-
support-only branches.

Jan

Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Jan Beulich]

On 24.02.2021 18:17, Andrew Cooper wrote:
> On 23/02/2021 07:53, Jan Beulich wrote:
>> On 22.02.2021 17:36, Andrew Cooper wrote:
>>> On 19/02/2021 08:09, Jan Beulich wrote:
 --- a/xen/arch/x86/Makefile
 +++ b/xen/arch/x86/Makefile
 @@ -123,8 +123,13 @@ ifneq ($(efi-y),)
  # Check if the compiler supports the MS ABI.
  export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o 
 efi/check.o 2>/dev/null && echo y)
  # Check if the linker supports PE.
 -XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep 
 --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
 +EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 
 --strip-debug
 +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o 
 efi/check.efi efi/check.o 2>/dev/null && echo y))
  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 +# Check if the linker produces fixups in PE by default (we need to 
 disable it doing so for now).
 +XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
 + $(shell $(LD) $(EFI_LDFLAGS) 
 --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
 + echo --disable-reloc-section))
>>> Why does --strip-debug move?
>> -S and --strip-debug are the same. I'm simply accumulating in
>> EFI_LDFLAGS all that's needed for the use in the probing construct.
> 
> Oh ok.
> 
> It occurs to me that EFI_LDFLAGS now only gets started in an ifneq
> block, but appended to later on while unprotected.  That said, I'm
> fairly sure it is only consumed inside a different ifeq section, so I
> think there is a reasonable quantity of tidying which ought to be done here.

Yes, in particular it wants to move out of Makefile (so it
won't get executed multiple times).

>> Also I meanwhile have a patch to retain debug info, for which this
>> movement turns out to be a prereq. (I've yet to test that the
>> produced binary actually works, and what's more I first need to get
>> a couple of changes accepted into binutils for the linker to actually
>> cope.)
>>
>>> What's wrong with $(call ld-option ...) ?  Actually, lots of this block
>>> of code looks to be opencoding of standard constructs.
>> It looks like ld-option could indeed be used here (there are marginal
>> differences which are likely acceptable), despite its brief comment
>> talking of just "flag" (singular, plus not really covering e.g. input
>> files).
>>
>> But:
>> - It working differently than cc-option makes it inconsistent to
>>   use (the setting of XEN_BUILD_EFI can't very well be switched to
>>   use cc-option); because of this I'm not surprised that we have
>>   only exactly one use right now in the tree.
>> - While XEN_BUILD_PE wants to be set to "y", for XEN_NO_PE_FIXUPS
>>   another transformation would then be necessary to translate "y"
>>   into "--disable-reloc-section".
>> - Do you really suggest to re-do this at this point in the release
>>   cycle?
> 
> I'm looking to prevent this almost-incompressible mess from getting worse.
> 
> But I suppose you want this to backport, so I suppose it ought to be
> minimally invasive.

Backporting - yes, definitely. And hence minimally invasive would
indeed be helpful.

> Acked-by: Andrew Cooper 

Thanks.

> This logic all actually needs moving into Kconfig so we can also go
> about fixing the other bugs we have such as having Multiboot headers in
> xen.efi pointing at unusable entrypoints.

My objections to doing such stuff in Kconfig have remained
unresponded to, iirc. Plus doing this in Kconfig would help on its
own - we'd also need to further split which object files get linked
into which binary. (In fact a patch in the 4.16 series I have now
to use linker produced base relocations and to retain debug info I
do away with prelink-efi.o, for becoming identical to prelink.o.)

Jan

[qemu-mainline test] 159642: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat fail REGR. vs. 152631
 test-amd64-amd64-xl-qcow2   21 guest-start/debian.repeat fail REGR. vs. 152631
 test-armhf-armhf-xl-vhd 17 guest-start/debian.repeat fail REGR. vs. 152631

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

version targeted for testing:
 qemuu7ef8134565dccf9186d5eabd7dbb4ecae6dead87
baseline version:
 qemuu1d806cef0e38b5db8347a8e12f214d543204a314

Last test of basis   152631  2020-08-20 09:07:46 Z  188 days
Failing since152659  2020-08-21 14:07:39 Z  187 days  362 attempts
Testing same since   159563  2021-02-22 23:37:57 Z2 days4 attempts


425 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm 

Re: Linux DomU freezes and dies under heavy memory shuffling

[Elliott Mitchell]

On Wed, Feb 24, 2021 at 08:30:45PM -0800, Roman Shaposhnik wrote:
> Right -- but that's not what distro builders use, right? I mean they do
> the whole sdeb -> deb business.
> 
> In fact, to stay as faithful as possible -- I'd love to:
>1. unpack SDEB
>2. add a single patch to the set of sources
>3. repack SDEB back
>4. do whatever it is they do to go SDEB -> DEB

Oh, that close to the original distribution package.  For
Debian-derivatives, install the package "dpkg-dev".

Generally the distribution will have a page somewhere where you can get
the files, but often it is handiest to run `apt-get source ` (I
believe `apt source ` also works, but I'm used to `apt-get`).
This will grab the tarballs for the source and unpack them.

Go into the unpacked directory and run `dpkg-buildpackage -b`
(optionally, patch first).  This creates the package in the starting
directory.

The tarballs left behind in the starting directory can be nuked or saved.
If saved, the build directory can be recreated by running
`dpkg-source -x _.dsc`.  This lets you reset the
build directory to original state.


-- 
(\___(\___(\__  --=> 8-) EHM <=--  __/)___/)___/)
 \BS (| ehem+sig...@m5p.com  PGP 87145445 |)   /
  \_CS\   |  _  -O #include  O-   _  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: Linux DomU freezes and dies under heavy memory shuffling

[Roman Shaposhnik]

On Wed, Feb 24, 2021 at 7:44 PM Elliott Mitchell  wrote:
>
> On Wed, Feb 24, 2021 at 07:06:25PM -0800, Roman Shaposhnik wrote:
> > I'm slightly confused about this patch -- it seems to me that it needs
> > to be applied to the guest kernel, correct?
> >
> > If that's the case -- the challenge I have is that I need to re-build
> > the Canonical (Ubuntu) distro kernel with this patch -- this seems
> > a bit daunting at first (I mean -- I'm pretty good at rebuilding kernels
> > I just never do it with the vendor ones ;-)).
> >
> > So... if there's anyone here who has any suggestions on how to do that
> > -- I'd appreciate pointers.
>
> Generally Debian-derivatives ship the kernel source they use as packages
> named "linux-source-." (guessing you need
> linux-source-5.4?).  They ship their configurations as packages
> "linux-config-.", but they also ship their configuration
> with their kernels as /boot/config-.
>
> If you're trying to create a proper packaged kernel, the Linux kernel
> Make target "bindeb-pkg" will create an appropriate .deb file.

Right -- but that's not what distro builders use, right? I mean they do
the whole sdeb -> deb business.

In fact, to stay as faithful as possible -- I'd love to:
   1. unpack SDEB
   2. add a single patch to the set of sources
   3. repack SDEB back
   4. do whatever it is they do to go SDEB -> DEB

Thanks,
Roman.

Re: Linux DomU freezes and dies under heavy memory shuffling

[Elliott Mitchell]

On Wed, Feb 24, 2021 at 07:06:25PM -0800, Roman Shaposhnik wrote:
> I'm slightly confused about this patch -- it seems to me that it needs
> to be applied to the guest kernel, correct?
> 
> If that's the case -- the challenge I have is that I need to re-build
> the Canonical (Ubuntu) distro kernel with this patch -- this seems
> a bit daunting at first (I mean -- I'm pretty good at rebuilding kernels
> I just never do it with the vendor ones ;-)).
> 
> So... if there's anyone here who has any suggestions on how to do that
> -- I'd appreciate pointers.

Generally Debian-derivatives ship the kernel source they use as packages
named "linux-source-." (guessing you need
linux-source-5.4?).  They ship their configurations as packages
"linux-config-.", but they also ship their configuration
with their kernels as /boot/config-.

If you're trying to create a proper packaged kernel, the Linux kernel
Make target "bindeb-pkg" will create an appropriate .deb file.

If you wish to extract a Debian package, they're some tarballs and a
marker file wrapped in a ar archive.  You're likely interested in
control.tar.?z* and data.tar.?z*.  Older packages used gzip-format
(.tar.gz), newer packages use xz-format (.tar.xz).

If you want to extract current Ubuntu kernel source on a different
distribution (or even an unrelated flavor of Unix), likely you would
want `ar p linux-source-5.4.deb data.tar.xz | unxz -c | tar -xf -`.


-- 
(\___(\___(\__  --=> 8-) EHM <=--  __/)___/)___/)
 \BS (| ehem+sig...@m5p.com  PGP 87145445 |)   /
  \_CS\   |  _  -O #include  O-   _  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: Linux DomU freezes and dies under heavy memory shuffling

[Roman Shaposhnik]

Hi Jürgen!

sorry for the belated reply -- I wanted to externalize the VM before I
do -- but let me at least reply to you:

On Tue, Feb 23, 2021 at 5:17 AM Jürgen Groß  wrote:
>
> On 18.02.21 06:21, Roman Shaposhnik wrote:
> > On Wed, Feb 17, 2021 at 12:29 AM Jürgen Groß  > > wrote:
> >
> > On 17.02.21 09:12, Roman Shaposhnik wrote:
> >  > Hi Jürgen, thanks for taking a look at this. A few comments below:
> >  >
> >  > On Tue, Feb 16, 2021 at 10:47 PM Jürgen Groß  > > wrote:
> >  >>
> >  >> On 16.02.21 21:34, Stefano Stabellini wrote:
> >  >>> + x86 maintainers
> >  >>>
> >  >>> It looks like the tlbflush is getting stuck?
> >  >>
> >  >> I have seen this case multiple times on customer systems now, but
> >  >> reproducing it reliably seems to be very hard.
> >  >
> >  > It is reliably reproducible under my workload but it take a long time
> >  > (~3 days of the workload running in the lab).
> >
> > This is by far the best reproduction rate I have seen up to now.
> >
> > The next best reproducer seems to be a huge installation with several
> > hundred hosts and thousands of VMs with about 1 crash each week.
> >
> >  >
> >  >> I suspected fifo events to be blamed, but just yesterday I've been
> >  >> informed of another case with fifo events disabled in the guest.
> >  >>
> >  >> One common pattern seems to be that up to now I have seen this
> > effect
> >  >> only on systems with Intel Gold cpus. Can it be confirmed to be true
> >  >> in this case, too?
> >  >
> >  > I am pretty sure mine isn't -- I can get you full CPU specs if
> > that's useful.
> >
> > Just the output of "grep model /proc/cpuinfo" should be enough.
> >
> >
> > processor: 3
> > vendor_id: GenuineIntel
> > cpu family: 6
> > model: 77
> > model name: Intel(R) Atom(TM) CPU  C2550  @ 2.40GHz
> > stepping: 8
> > microcode: 0x12d
> > cpu MHz: 1200.070
> > cache size: 1024 KB
> > physical id: 0
> > siblings: 4
> > core id: 3
> > cpu cores: 4
> > apicid: 6
> > initial apicid: 6
> > fpu: yes
> > fpu_exception: yes
> > cpuid level: 11
> > wp: yes
> > flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat
> > pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp
> > lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology
> > nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx est
> > tm2 ssse3 cx16 xtpr pdcm sse4_1 sse4_2 movbe popcnt tsc_deadline_timer
> > aes rdrand lahf_lm 3dnowprefetch cpuid_fault epb pti ibrs ibpb stibp
> > tpr_shadow vnmi flexpriority ept vpid tsc_adjust smep erms dtherm ida
> > arat md_clear
> > vmx flags: vnmi preemption_timer invvpid ept_x_only flexpriority
> > tsc_offset vtpr mtf vapic ept vpid unrestricted_guest
> > bugs: cpu_meltdown spectre_v1 spectre_v2 mds msbds_only
> > bogomips: 4800.19
> > clflush size: 64
> > cache_alignment: 64
> > address sizes: 36 bits physical, 48 bits virtual
> > power management:
> >
> >  >
> >  >> In case anybody has a reproducer (either in a guest or dom0) with a
> >  >> setup where a diagnostic kernel can be used, I'd be _very_
> > interested!
> >  >
> >  > I can easily add things to Dom0 and DomU. Whether that will
> > disrupt the
> >  > experiment is, of course, another matter. Still please let me
> > know what
> >  > would be helpful to do.
> >
> > Is there a chance to switch to an upstream kernel in the guest? I'd like
> > to add some diagnostic code to the kernel and creating the patches will
> > be easier this way.
> >
> >
> > That's a bit tough -- the VM is based on stock Ubuntu and if I upgrade
> > the kernel I'll have fiddle with a lot things to make workload
> > functional again.
> >
> > However, I can install debug kernel (from Ubuntu, etc. etc.)
> >
> > Of course, if patching the kernel is the only way to make progress --
> > lets try that -- please let me know.
>
> I have found a nice upstream patch, which - with some modifications - I
> plan to give our customer as a workaround.
>
> The patch is for kernel 4.12, but chances are good it will apply to a
> 4.15 kernel, too.

I'm slightly confused about this patch -- it seems to me that it needs
to be applied to the guest kernel, correct?

If that's the case -- the challenge I have is that I need to re-build
the Canonical (Ubuntu) distro kernel with this patch -- this seems
a bit daunting at first (I mean -- I'm pretty good at rebuilding kernels
I just never do it with the vendor ones ;-)).

So... if there's anyone here who has any suggestions on how to do that
-- I'd appreciate pointers.

> I have been able to gather some more data.
>
> I have contacted the author of the upstream kernel patch I've been using
> for our customer (and that helped, by the way).
>
> It seems as if the problem is occurring when running as a guest at least
> under Xen, 

Re: [PATCH] virtio-gpu: Respect graphics update interval for EDID

[Akihiko Odaki]

2021年2月24日(水) 20:17 Gerd Hoffmann :
>
> On Tue, Feb 23, 2021 at 01:50:51PM +0900, Akihiko Odaki wrote:
> > 2021年2月22日(月) 19:57 Gerd Hoffmann :
> > >
> > > On Sun, Feb 21, 2021 at 10:34:14PM +0900, Akihiko Odaki wrote:
> > > > This change introduces an additional member, refresh_rate to
> > > > qemu_edid_info in include/hw/display/edid.h.
> > > >
> > > > This change also isolates the graphics update interval from the
> > > > display update interval. The guest will update the frame buffer
> > > > in the graphics update interval, but displays can be updated in a
> > > > dynamic interval, for example to save update costs aggresively
> > > > (vnc) or to respond to user-generated events (sdl).
> > > > It stabilizes the graphics update interval and prevents the guest
> > > > from being confused.
> > >
> > > Hmm.  What problem you are trying to solve here?
> > >
> > > The update throttle being visible by the guest was done intentionally,
> > > so the guest can throttle the display updates too in case nobody is
> > > watching those display updated anyway.
> >
> > Indeed, we are throttling the update for vnc to avoid some worthless
> > work. But typically a guest cannot respond to update interval changes
> > so often because real display devices the guest is designed for does
> > not change the update interval in that way.
>
> What is the problem you are seeing?
>
> Some guest software raising timeout errors when they see only
> one vblank irq every 3 seconds?  If so: which software is this?
> Any chance we can fix this on the guest side?
>
> > That is why we have to
> > tell the guest a stable update interval even if it results in wasted
> > frames.
>
> Because of the wasted frames I'd like this to be an option you can
> enable when needed.  For the majority of use cases this seems to be
> no problem ...

I see blinks with GNOME on Wayland on Ubuntu 20.04 and virtio-gpu with
the EDID change included in this patch. The only devices inspecting
the variable, xenfb and modified virtio-gpu, do not yield vblank irq,
but they report the refresh rate to the guest, and the guest
proactively requests them to switch the surface.

I suspect Linux's kernel mode setting causes blinks and other guests
have similar problems.

>
> Also: the EDID changes should go to a separate patch.

That makes sense. I'll isolate it to a seperate patch in a series.

Regards,
Akihiko Odaki

>
> take care,
>   Gerd
>

[PATCH] xen: introduce XENFEAT_direct_mapped and XENFEAT_not_direct_mapped

[Stefano Stabellini]

Introduce two feature flags to tell the domain whether it is
direct-mapped or not. It allows the guest kernel to make informed
decisions on things such as swiotlb-xen enablement.

Currently, only Dom0 on ARM is direct-mapped, see:
xen/include/asm-arm/domain.h:is_domain_direct_mapped
xen/include/asm-x86/domain.h:is_domain_direct_mapped

However, given that it is theoretically possible to have direct-mapped
domains on x86 too, the two new feature flags are arch-neutral.

Signed-off-by: Stefano Stabellini 
CC: jbeul...@suse.com
CC: andrew.coop...@citrix.com
CC: jul...@xen.org
---
 xen/common/kernel.c   | 4 
 xen/include/public/features.h | 7 +++
 2 files changed, 11 insertions(+)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 7a345ae45e..6ca1377dec 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -560,6 +560,10 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
  (1U << XENFEAT_hvm_callback_vector) |
  (has_pirq(d) ? (1U << XENFEAT_hvm_pirqs) : 0);
 #endif
+if ( is_domain_direct_mapped(d) )
+fi.submap |= (1U << XENFEAT_direct_mapped);
+else
+fi.submap |= (1U << XENFEAT_not_direct_mapped);
 break;
 default:
 return -EINVAL;
diff --git a/xen/include/public/features.h b/xen/include/public/features.h
index 1613b2aab8..18e3e61df0 100644
--- a/xen/include/public/features.h
+++ b/xen/include/public/features.h
@@ -114,6 +114,13 @@
  */
 #define XENFEAT_linux_rsdp_unrestricted   15
 
+/*
+ * A direct-mapped (or 1:1 mapped) domain is a domain for which its
+ * local pages have gfn == mfn.
+ */
+#define XENFEAT_not_direct_mapped   16
+#define XENFEAT_direct_mapped   17
+
 #define XENFEAT_NR_SUBMAPS 1
 
 #endif /* __XEN_PUBLIC_FEATURES_H__ */
-- 
2.17.1

[xen-unstable test] 159626: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-xtf-amd64-amd64-3  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-1  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-4  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-2  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-5  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-xl-xsm8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  8 xen-boot  fail REGR. vs. 159475
 build-i3866 xen-buildfail REGR. vs. 159475
 test-amd64-amd64-examine  4 memdisk-try-append   fail REGR. vs. 159475
 build-i386-prev   6 xen-buildfail REGR. vs. 159475

Tests which did not succeed, but are not blocking:
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-pvshim 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemut-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-i386-examine   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  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-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-livepatch 1 build-check(1)   blocked  n/a
 test-amd64-i386-migrupgrade   1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-qemut-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-qemut-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 159475
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 159475
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 159475
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 159475
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 159475
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 159475
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 159475
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 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-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-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
 

[xen-unstable bisection] complete test-amd64-i386-qemut-rhel6hvm-intel

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-qemut-rhel6hvm-intel
testid xen-boot

Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  xen git://xenbits.xen.org/xen.git
  Bug introduced:  4dc1815991420b809ce18dddfdf9c0af48944204
  Bug not present: 2d824791504f4119f04f95bafffec2e37d319c25
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/159650/


  commit 4dc1815991420b809ce18dddfdf9c0af48944204
  Author: Jan Beulich 
  Date:   Fri Feb 19 17:19:56 2021 +0100
  
  x86/PV: harden guest memory accesses against speculative abuse
  
  Inspired by
  
https://lore.kernel.org/lkml/f12e7d3cecf41b2c29734ea45a393be21d4a8058.1597848273.git.jpoim...@redhat.com/
  and prior work in that area of x86 Linux, suppress speculation with
  guest specified pointer values by suitably masking the addresses to
  non-canonical space in case they fall into Xen's virtual address range.
  
  Introduce a new Kconfig control.
  
  Note that it is necessary in such code to avoid using "m" kind operands:
  If we didn't, there would be no guarantee that the register passed to
  guest_access_mask_ptr is also the (base) one used for the memory access.
  
  As a minor unrelated change in get_unsafe_asm() the unnecessary "itype"
  parameter gets dropped and the XOR on the fixup path gets changed to be
  a 32-bit one in all cases: This way we avoid pointless REX.W or operand
  size overrides, or writes to partial registers.
  
  Requested-by: Andrew Cooper 
  Signed-off-by: Jan Beulich 
  Reviewed-by: Roger Pau Monné 
  Release-Acked-by: Ian Jackson 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable/test-amd64-i386-qemut-rhel6hvm-intel.xen-boot.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/xen-unstable/test-amd64-i386-qemut-rhel6hvm-intel.xen-boot
 --summary-out=tmp/159650.bisection-summary --basis-template=159475 
--blessings=real,real-bisect,real-retry xen-unstable 
test-amd64-i386-qemut-rhel6hvm-intel xen-boot
Searching for failure / basis pass:
 159602 fail [host=huxelrebe1] / 159475 [host=chardonnay0] 159453 [host=fiano1] 
159424 [host=elbling0] 159396 [host=fiano0] 159362 [host=chardonnay1] 159335 
[host=elbling1] 159315 [host=albana0] 159202 [host=albana1] 159134 
[host=fiano1] 159036 [host=chardonnay0] 159013 [host=fiano0] 158957 
[host=elbling0] 158922 [host=elbling1] 158873 [host=chardonnay1] 158835 
[host=chardonnay0] 158811 [host=fiano1] 158787 [host=albana1] 158755 
[host=elbling1] 158719 [host=chardonnay1] 158711 [host=fiano0] 1586\
 99 [host=chardonnay1] 158628 [host=elbling1] 158617 [host=chardonnay1] 158607 
[host=chardonnay1] 158601 [host=albana0] 158591 [host=elbling1] 158581 ok.
Failure / basis pass flights: 159602 / 158581
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Latest c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
f894c3d8e705fea9cb3244fa61684bfd8bdd1b2a
Basis pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
5e317896342d553f0b55f72948bbf93a0f1147d3
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/linux-pvops.git#c3038e718a19fc596f7b1baba0f83d5146dc7784-c3038e718a19fc596f7b1baba0f83d5146dc7784
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/qemu-xen-traditional.git#3d273dd05e51e5a1ffba3d98c7437ee84e8f8764-3d273dd05e51e5a1ffba3d98c7437ee84e8f8764
 git://xenbits.xen.org/qemu-xen.git#7ea428895af2840d85c524f0bd11a38\
 aac308308-7ea428895af2840d85c524f0bd11a38aac308308 
git://xenbits.xen.org/xen.git#5e317896342d553f0b55f72948bbf93a0f1147d3-f894c3d8e705fea9cb3244fa61684bfd8bdd1b2a
Loaded 5001 nodes in revision graph
Searching for test results:
 158581 pass c3038e718a19fc596f7b1baba0f83d5146dc7784 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 

Re: [RFC PATCH 00/10] Preemption in hypervisor (ARM only)

[Andrew Cooper]

On 24/02/2021 23:58, Volodymyr Babchuk wrote:
> And I am not mentioning x86 support there...

x86 uses per-pCPU stacks, not per-vCPU stacks.

Transcribing from an old thread which happened in private as part of an
XSA discussion, concerning the implications of trying to change this.

~Andrew

-8<-

Here is a partial list off the top of my head of the practical problems
you're going to have to solve.

Introduction of new SpectreRSB vulnerable gadgets.  I'm really close to
being able to drop RSB stuffing and recover some performance in Xen.

CPL0 entrypoints need updating across schedule.  SYSCALL entry would
need to become a stub per vcpu, rather than the current stub per pcpu.
This requires reintroducing a writeable mapping to the TSS (doable) and
a shadow stack switch of active stacks (This corner case is so broken it
looks to be a blocker for CET-SS support in Linux, and is resulting in
some conversation about tweaking Shstk's in future processors).

All per-cpu variables stop working.  You'd need to rewrite Xen to use
%gs for TLS which will have churn in the PV logic, and introduce the x86
architectural corner cases of running with an invalid %gs.  Xen has been
saved from a large number of privilege escalation vulnerabilities in
common with Linux and Windows by the fact that we don't use %gs, so
anyone trying to do this is going to have to come up with some concrete
way of proving that the corner cases are covered.

Re: [RFC PATCH 00/10] Preemption in hypervisor (ARM only)

[Volodymyr Babchuk]

Julien Grall writes:

> On Wed, 24 Feb 2021 at 20:58, Volodymyr Babchuk
>  wrote:
>>
>>
>> Hi Julien,
>>
>> Julien Grall writes:
>>
>>> On 23/02/2021 12:06, Volodymyr Babchuk wrote:
 Hi Julien,
>>>
>>> Hi Volodymyr,
>>>
 Julien Grall writes:
> On 23/02/2021 02:34, Volodymyr Babchuk wrote:
> ... just rescheduling the vCPU. It will also give the opportunity for
> the guest to handle interrupts.
>
> If you don't return to the guest, then risk to get an RCU sched stall
> on that the vCPU (some hypercalls can take really really long).
 Ah yes, you are right. I'd only wish that hypervisor saved context
 of
 hypercall on it's side...
 I have example of OP-TEE before my eyes. They have special return
 code
 "task was interrupted" and they use separate call "continue execution of
 interrupted task", which takes opaque context handle as a
 parameter. With this approach state of interrupted call never leaks to > 
 rest of the system.
>>>
>>> Feel free to suggest a new approach for the hypercals.
>>>
>>
>> I believe, I suggested it right above. There are some corner cases, that
>> should be addressed, of course.
>
> If we wanted a clean break, then possibly yes.  But I meant one that doesn't
> break all the existing users and doesn't put Xen at risk.
>
> I don't believe your approach fulfill it.

Of course, we can't touch any hypercalls that are part of stable
ABI. But if I got this right, domctls and sysctls are not stable, so one
can change theirs behavior quite drastically in major releases.

>>
>
>> This approach itself have obvious
>> problems: code that executes hypercall is responsible for preemption,
>> preemption checks are infrequent (because they are costly by
>> themselves), hypercall execution state is stored in guest-controlled
>> area, we rely on guest's good will to continue the hypercall.
>
> Why is it a problem to rely on guest's good will? The hypercalls
> should be preempted at a boundary that is safe to continue.
 Yes, and it imposes restrictions on how to write hypercall
 handler.
 In other words, there are much more places in hypercall handler code
 where it can be preempted than where hypercall continuation can be
 used. For example, you can preempt hypercall that holds a mutex, but of
 course you can't create an continuation point in such place.
>>>
>>> I disagree, you can create continuation point in such place. Although
>>> it will be more complex because you have to make sure you break the
>>> work in a restartable place.
>>
>> Maybe there is some misunderstanding. You can't create hypercall
>> continuation point in a place where you are holding mutex. Because,
>> there is absolutely not guarantee that guest will restart the
>> hypercall.
>
> I don't think we are disagreeing here. My point is you should rarely
> need to hold a mutex for a long period, so you could break your work
> in smaller chunk. In which cases, you can use hypercall continuation.

Let's say in this way: generally you can hold a mutex much longer than
you can hold a spinlock. And nothing catastrophic will happen if you are
preempted while holding a mutex. Better to avoid, this of course.

>
>>
>> But you can preempt vCPU while holding mutex, because xen owns scheduler
>> and it can guarantee that vCPU will be scheduled eventually to continue
>> the work and release mutex.
>
> The problem is the "eventually". If you are accounting the time spent
> in the hypervisor to the vCPU A, then there is a possibility that it
> has exhausted its time slice. In which case, your vCPU A may be
> sleeping for a while with a mutex held.
>
> If another vCPU B needs the mutex, it will either have to wait
> potentially for a long time or we need to force vCPU A to run on
> borrowed time.

Yes, of course.

>>
>>> I would also like to point out that preemption also have some drawbacks.
>>> With RT in mind, you have to deal with priority inversion (e.g. a
>>> lower priority vCPU held a mutex that is required by an higher
>>> priority).
>>
>> Of course. This is not as simple as "just call scheduler when we want
>> to".
>
> Your e-mail made it sounds like it was easy to add preemption in
> Xen. ;)

I'm sorry for that :)
Actually, there is lots of work to do. It appeared to me that "current"
needs to be reworked, preempt_enable/disable needs to be reworked, per-cpu
variables also should be reworked. And this is just to ensure
consistency of already existing code.

And I am not mentioning x86 support there...

>>> Outside of RT, you have to be careful where mutex are held. In your
>>> earlier answer, you suggested to held mutex for the memory
>>> allocation. If you do that, then it means a domain A can block
>>> allocation for domain B as it helds the mutex.
>>
>> As long as we do not exit to a EL1 with mutex being held, domain A can't
>> block anything. Of course, we have to deal with priority inversion, but
>> 

Re: [RFC PATCH 00/10] Preemption in hypervisor (ARM only)

[Volodymyr Babchuk]

Hi Andrew,

Andrew Cooper writes:

> On 23/02/2021 02:34, Volodymyr Babchuk wrote:
>> Hello community,
>>
>> Subject of this cover letter is quite self-explanatory. This patch
>> series implements PoC for preemption in hypervisor mode.
>>
>> This is the sort of follow-up to recent discussion about latency
>> ([1]).
>>
>> Motivation
>> ==
>>
>> It is well known that Xen is not preemptable. On other words, it is
>> impossible to switch vCPU contexts while running in hypervisor
>> mode. Only one place where scheduling decision can be made and one
>> vCPU can be replaced with another is the exit path from the hypervisor
>> mode. The one exception are Idle vCPUs, which never leaves the
>> hypervisor mode for obvious reasons.
>>
>> This leads to a number of problems. This list is not comprehensive. It
>> lists only things that I or my colleagues encountered personally.
>>
>> Long-running hypercalls. Due to nature of some hypercalls they can
>> execute for arbitrary long time. Mostly those are calls that deal with
>> long list of similar actions, like memory pages processing. To deal
>> with this issue Xen employs most horrific technique called "hypercall
>> continuation". When code that handles hypercall decides that it should
>> be preempted, it basically updates the hypercall parameters, and moves
>> guest PC one instruction back. This causes guest to re-execute the
>> hypercall with altered parameters, which will allow hypervisor to
>> continue hypercall execution later. This approach itself have obvious
>> problems: code that executes hypercall is responsible for preemption,
>> preemption checks are infrequent (because they are costly by
>> themselves), hypercall execution state is stored in guest-controlled
>> area, we rely on guest's good will to continue the hypercall. All this
>> imposes restrictions on which hypercalls can be preempted, when they
>> can be preempted and how to write hypercall handlers. Also, it
>> requires very accurate coding and already led to at least one
>> vulnerability - XSA-318. Some hypercalls can not be preempted at all,
>> like the one mentioned in [1].
>>
>> Absence of hypervisor threads/vCPUs. Hypervisor owns only idle vCPUs,
>> which are supposed to run when the system is idle. If hypervisor needs
>> to execute own tasks that are required to run right now, it have no
>> other way than to execute them on current vCPU. But scheduler does not
>> know that hypervisor executes hypervisor task and accounts spent time
>> to a domain. This can lead to domain starvation.
>>
>> Also, absence of hypervisor threads leads to absence of high-level
>> synchronization primitives like mutexes, conditional variables,
>> completions, etc. This leads to two problems: we need to use spinlocks
>> everywhere and we have problems when porting device drivers from linux
>> kernel.
>
> You cannot reenter a guest, even to deliver interrupts, if pre-empted at
> an arbitrary point in a hypercall.  State needs unwinding suitably.
>

Yes, Julien pointed this to me already. So, looks like hypercall
continuations are still needed.

> Xen's non-preemptible-ness is designed to specifically force you to not
> implement long-running hypercalls which would interfere with timely
> interrupt handling in the general case.

What if long-running hypercalls are still required? There are other
options, like async calls, for example.

> Hypervisor/virt properties are different to both a kernel-only-RTOS, and
> regular usespace.  This was why I gave you some specific extra scenarios
> to do latency testing with, so you could make a fair comparison of
> "extra overhead caused by Xen" separate from "overhead due to
> fundamental design constraints of using virt".

I can't see any fundamental constraints there. I see how virtualization
architecture can influence context switch time: how many actions you
need to switch one vCPU to another. I have in mind low level things
there: reprogram MMU to use another set of tables, reprogram your
interrupt controller, timer, etc. Of course, you can't get latency lower
that context switch time. This is the only fundamental constraint I can
see.

But all other things are debatable.

As for latency testing, I'm not interested in absolute times per se. I
already determined that time needed to switch vCPU context on my machine
is about 9us. It is fine for me. I am interested in a (semi-)guaranteed
time of reaction. And Xen is doing quite well in most cases. But there
are other cases, when long-lasting hypercalls cause spikes in time of
reaction.

> Preemption like this will make some benchmarks look better, but it also
> introduces the ability to create fundamental problems, like preventing
> any interrupt delivery into a VM for seconds of wallclock time while
> each vcpu happens to be in a long-running hypercall.
>
> If you want timely interrupt handling, you either need to partition your
> workloads by the long-running-ness of their hypercalls, or not have
> long-running 

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

[osstest service owner]

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

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  60390ccb8b9b2dbf85010f8b47779bb231aa2533
baseline version:
 xen  5d94433a66df29ce314696a13bdd324ec0e342fe

Last test of basis   159600  2021-02-23 20:01:30 Z1 days
Failing since159624  2021-02-24 12:01:29 Z0 days3 attempts
Testing same since   159644  2021-02-24 19:01:47 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Roger Pau Monné 

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
   5d94433a66..60390ccb8b  60390ccb8b9b2dbf85010f8b47779bb231aa2533 -> smoke

Re: [RFC PATCH 00/10] Preemption in hypervisor (ARM only)

[Julien Grall]

On Wed, 24 Feb 2021 at 20:58, Volodymyr Babchuk
 wrote:
>
>
> Hi Julien,
>
> Julien Grall writes:
>
> > On 23/02/2021 12:06, Volodymyr Babchuk wrote:
> >> Hi Julien,
> >
> > Hi Volodymyr,
> >
> >> Julien Grall writes:
> >>> On 23/02/2021 02:34, Volodymyr Babchuk wrote:
> >>> ... just rescheduling the vCPU. It will also give the opportunity for
> >>> the guest to handle interrupts.
> >>>
> >>> If you don't return to the guest, then risk to get an RCU sched stall
> >>> on that the vCPU (some hypercalls can take really really long).
> >> Ah yes, you are right. I'd only wish that hypervisor saved context
> >> of
> >> hypercall on it's side...
> >> I have example of OP-TEE before my eyes. They have special return
> >> code
> >> "task was interrupted" and they use separate call "continue execution of
> >> interrupted task", which takes opaque context handle as a
> >> parameter. With this approach state of interrupted call never leaks to > 
> >> rest of the system.
> >
> > Feel free to suggest a new approach for the hypercals.
> >
>
> I believe, I suggested it right above. There are some corner cases, that
> should be addressed, of course.

If we wanted a clean break, then possibly yes.  But I meant one that doesn't
break all the existing users and doesn't put Xen at risk.

I don't believe your approach fulfill it.

>
> >>>
>  This approach itself have obvious
>  problems: code that executes hypercall is responsible for preemption,
>  preemption checks are infrequent (because they are costly by
>  themselves), hypercall execution state is stored in guest-controlled
>  area, we rely on guest's good will to continue the hypercall.
> >>>
> >>> Why is it a problem to rely on guest's good will? The hypercalls
> >>> should be preempted at a boundary that is safe to continue.
> >> Yes, and it imposes restrictions on how to write hypercall
> >> handler.
> >> In other words, there are much more places in hypercall handler code
> >> where it can be preempted than where hypercall continuation can be
> >> used. For example, you can preempt hypercall that holds a mutex, but of
> >> course you can't create an continuation point in such place.
> >
> > I disagree, you can create continuation point in such place. Although
> > it will be more complex because you have to make sure you break the
> > work in a restartable place.
>
> Maybe there is some misunderstanding. You can't create hypercall
> continuation point in a place where you are holding mutex. Because,
> there is absolutely not guarantee that guest will restart the
> hypercall.

I don't think we are disagreeing here. My point is you should rarely
need to hold a mutex for a long period, so you could break your work
in smaller chunk. In which cases, you can use hypercall continuation.

>
> But you can preempt vCPU while holding mutex, because xen owns scheduler
> and it can guarantee that vCPU will be scheduled eventually to continue
> the work and release mutex.

The problem is the "eventually". If you are accounting the time spent
in the hypervisor to the vCPU A, then there is a possibility that it
has exhausted its time slice. In which case, your vCPU A may be
sleeping for a while with a mutex held.

If another vCPU B needs the mutex, it will either have to wait
potentially for a long time or we need to force vCPU A to run on
borrowed time.

>
> > I would also like to point out that preemption also have some drawbacks.
> > With RT in mind, you have to deal with priority inversion (e.g. a
> > lower priority vCPU held a mutex that is required by an higher
> > priority).
>
> Of course. This is not as simple as "just call scheduler when we want
> to".

Your e-mail made it sounds like it was easy to add preemption in Xen. ;)

>
> > Outside of RT, you have to be careful where mutex are held. In your
> > earlier answer, you suggested to held mutex for the memory
> > allocation. If you do that, then it means a domain A can block
> > allocation for domain B as it helds the mutex.
>
> As long as we do not exit to a EL1 with mutex being held, domain A can't
> block anything. Of course, we have to deal with priority inversion, but
> malicious domain can't cause DoS.

It is not really a priority inversion problem outside of RT because
all the tasks will have the same priority. It is more a time
accounting problem because each vCPU may have a different number of
credits.

> >>> I am really surprised that this is the only changes necessary in
> >>> Xen. For a first approach, we may want to be conservative when the
> >>> preemption is happening as I am not convinced that all the places are
> >>> safe to preempt.
> >>>
> >> Well, I can't say that I ran extensive tests, but I played with this
> >> for
> >> some time and it seemed quite stable. Of course, I had some problems
> >> with RTDS...
> >> As I see it, Xen is already supports SMP, so all places where races
> >> are
> >> possible should already be covered by spinlocks or taken into account by
> >> some other 

Re: [RFC PATCH 00/10] Preemption in hypervisor (ARM only)

[Volodymyr Babchuk]

Hi Julien,

Julien Grall writes:

> On 23/02/2021 12:06, Volodymyr Babchuk wrote:
>> Hi Julien,
>
> Hi Volodymyr,
>
>> Julien Grall writes:
>>> On 23/02/2021 02:34, Volodymyr Babchuk wrote:
>>> ... just rescheduling the vCPU. It will also give the opportunity for
>>> the guest to handle interrupts.
>>>
>>> If you don't return to the guest, then risk to get an RCU sched stall
>>> on that the vCPU (some hypercalls can take really really long).
>> Ah yes, you are right. I'd only wish that hypervisor saved context
>> of
>> hypercall on it's side...
>> I have example of OP-TEE before my eyes. They have special return
>> code
>> "task was interrupted" and they use separate call "continue execution of
>> interrupted task", which takes opaque context handle as a
>> parameter. With this approach state of interrupted call never leaks to > 
>> rest of the system.
>
> Feel free to suggest a new approach for the hypercals.
>

I believe, I suggested it right above. There are some corner cases, that
should be addressed, of course.

>>>
 This approach itself have obvious
 problems: code that executes hypercall is responsible for preemption,
 preemption checks are infrequent (because they are costly by
 themselves), hypercall execution state is stored in guest-controlled
 area, we rely on guest's good will to continue the hypercall.
>>>
>>> Why is it a problem to rely on guest's good will? The hypercalls
>>> should be preempted at a boundary that is safe to continue.
>> Yes, and it imposes restrictions on how to write hypercall
>> handler.
>> In other words, there are much more places in hypercall handler code
>> where it can be preempted than where hypercall continuation can be
>> used. For example, you can preempt hypercall that holds a mutex, but of
>> course you can't create an continuation point in such place.
>
> I disagree, you can create continuation point in such place. Although
> it will be more complex because you have to make sure you break the
> work in a restartable place.

Maybe there is some misunderstanding. You can't create hypercall
continuation point in a place where you are holding mutex. Because,
there is absolutely not guarantee that guest will restart the
hypercall.

But you can preempt vCPU while holding mutex, because xen owns scheduler
and it can guarantee that vCPU will be scheduled eventually to continue
the work and release mutex.

> I would also like to point out that preemption also have some drawbacks.
> With RT in mind, you have to deal with priority inversion (e.g. a
> lower priority vCPU held a mutex that is required by an higher
> priority).

Of course. This is not as simple as "just call scheduler when we want
to".

> Outside of RT, you have to be careful where mutex are held. In your
> earlier answer, you suggested to held mutex for the memory
> allocation. If you do that, then it means a domain A can block
> allocation for domain B as it helds the mutex.

As long as we do not exit to a EL1 with mutex being held, domain A can't
block anything. Of course, we have to deal with priority inversion, but
malicious domain can't cause DoS.

> This can lead to quite serious problem if domain A cannot run (because
> it exhausted its credit) for a long time.
>

I believe, this problem is related to a priority inversion problem and
they should be addressed together.

>> 
 All this
 imposes restrictions on which hypercalls can be preempted, when they
 can be preempted and how to write hypercall handlers. Also, it
 requires very accurate coding and already led to at least one
 vulnerability - XSA-318. Some hypercalls can not be preempted at all,
 like the one mentioned in [1].
 Absence of hypervisor threads/vCPUs. Hypervisor owns only idle
 vCPUs,
 which are supposed to run when the system is idle. If hypervisor needs
 to execute own tasks that are required to run right now, it have no
 other way than to execute them on current vCPU. But scheduler does not
 know that hypervisor executes hypervisor task and accounts spent time
 to a domain. This can lead to domain starvation.
 Also, absence of hypervisor threads leads to absence of high-level
 synchronization primitives like mutexes, conditional variables,
 completions, etc. This leads to two problems: we need to use spinlocks
 everywhere and we have problems when porting device drivers from linux
 kernel.
 Proposed solution
 =
 It is quite obvious that to fix problems above we need to allow
 preemption in hypervisor mode. I am not familiar with x86 side, but
 for the ARM it was surprisingly easy to implement. Basically, vCPU
 context in hypervisor mode is determined by its stack at general
 purpose registers. And __context_switch() function perfectly switches
 them when running in hypervisor mode. So there are no hard
 restrictions, why it should be called only in leave_hypervisor() path.
 The 

[ovmf test] 159640: all pass - PUSHED

[osstest service owner]

flight 159640 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/159640/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 35f87da8a2debd443ac842db0a3794b17914a8f4
baseline version:
 ovmf 739a506b18c4f694b8d5d2f3424a329c45d737ba

Last test of basis   159619  2021-02-24 08:09:48 Z0 days
Testing same since   159640  2021-02-24 17:10:46 Z0 days1 attempts


People who touched revisions under test:
  Abner Chang 
  Fan Wang 
  Jiaxin Wu 
  Siyuan Fu 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  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/osstest/ovmf.git
   739a506b18..35f87da8a2  35f87da8a2debd443ac842db0a3794b17914a8f4 -> 
xen-tested-master

Re: [PATCH 0/2] hvmloader: drop usage of system headers

[Andrew Cooper]

On 24/02/2021 10:26, Roger Pau Monne wrote:
> Hello,
>
> Following two patches aim to make hvmloader standalone, so that it don't
> try to use system headers. It shouldn't result in any functional
> change.
>
> Thanks, Roger.

After some experimentation in the arch container

Given foo.c as:

#include 

extern uint64_t bar;
uint64_t foo(void)
{
    return bar;
}

int main(void)
{
    return 0;
}

The preprocessed form with `gcc -m32 -E` is:

# 1 "foo.c"
# 1 ""
# 1 ""
# 31 ""
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 32 "" 2
# 1 "foo.c"
# 1 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint.h" 1 3 4
# 9 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint.h" 3 4
# 1 "/usr/include/stdint.h" 1 3 4
# 26 "/usr/include/stdint.h" 3 4
# 1 "/usr/include/bits/libc-header-start.h" 1 3 4
# 33 "/usr/include/bits/libc-header-start.h" 3 4
# 1 "/usr/include/features.h" 1 3 4
# 450 "/usr/include/features.h" 3 4
# 1 "/usr/include/sys/cdefs.h" 1 3 4
# 452 "/usr/include/sys/cdefs.h" 3 4
# 1 "/usr/include/bits/wordsize.h" 1 3 4
# 453 "/usr/include/sys/cdefs.h" 2 3 4
# 1 "/usr/include/bits/long-double.h" 1 3 4
# 454 "/usr/include/sys/cdefs.h" 2 3 4
# 451 "/usr/include/features.h" 2 3 4
# 474 "/usr/include/features.h" 3 4
# 1 "/usr/include/gnu/stubs.h" 1 3 4

# 1 "/usr/include/gnu/stubs-32.h" 1 3 4
# 8 "/usr/include/gnu/stubs.h" 2 3 4
# 475 "/usr/include/features.h" 2 3 4
# 34 "/usr/include/bits/libc-header-start.h" 2 3 4
# 27 "/usr/include/stdint.h" 2 3 4
# 1 "/usr/include/bits/types.h" 1 3 4
# 27 "/usr/include/bits/types.h" 3 4
# 1 "/usr/include/bits/wordsize.h" 1 3 4
# 28 "/usr/include/bits/types.h" 2 3 4
# 1 "/usr/include/bits/timesize.h" 1 3 4
# 29 "/usr/include/bits/types.h" 2 3 4

# 31 "/usr/include/bits/types.h" 3 4
typedef unsigned char __u_char;
...

while the freestanding form with `gcc -ffreestanding -m32 -E` is:

# 1 "foo.c"
# 1 ""
# 1 ""
# 1 "foo.c"
# 1 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint.h" 1 3 4
# 11 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint.h" 3 4
# 1 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint-gcc.h" 1 3 4
# 34 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint-gcc.h" 3 4

# 34 "/usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/stdint-gcc.h" 3 4
typedef signed char int8_t;
...


I can compile and link trivial programs using -m32 and stdint.h without
any issue.

Clearly something more subtle is going on with our choice of options
when compiling hvmloader, but it certainly looks like stdint.h is fine
to use in the way we want to use it.

~Andrew

[linux-linus test] 159610: regressions - FAIL

[osstest service owner]

flight 159610 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/159610/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-xsm7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. 
vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-examine   6 xen-install  fail REGR. vs. 152332
 test-amd64-i386-libvirt   7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332
 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-coresched-i386-xl  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-libvirt-xsm   7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-freebsd10-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-pvshim 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-raw7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-freebsd10-i386  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-shadow 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. 
vs. 152332
 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332
 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332
 test-arm64-arm64-xl-xsm  10 host-ping-check-xen  fail REGR. vs. 152332
 test-arm64-arm64-xl-credit1  10 host-ping-check-xen  fail REGR. vs. 152332
 test-arm64-arm64-xl   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-examine 13 examine-iommufail REGR. vs. 152332
 test-amd64-amd64-amd64-pvgrub 20 guest-stop  fail REGR. vs. 152332
 test-amd64-amd64-i386-pvgrub 20 guest-stop   fail REGR. vs. 152332
 test-arm64-arm64-xl-seattle   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-xl-credit2  10 host-ping-check-xen  fail REGR. vs. 152332
 test-arm64-arm64-libvirt-xsm  8 xen-boot fail REGR. vs. 152332

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

Re: [PATCH][4.15] x86: mirror compat argument translation area for 32-bit PV

[Andrew Cooper]

On 23/02/2021 07:13, Jan Beulich wrote:
> On 22.02.2021 17:47, Andrew Cooper wrote:
>> On 22/02/2021 14:22, Jan Beulich wrote:
>>> On 22.02.2021 15:14, Andrew Cooper wrote:
 On 22/02/2021 10:27, Jan Beulich wrote:
> Now that we guard the entire Xen VA space against speculative abuse
> through hypervisor accesses to guest memory, the argument translation
> area's VA also needs to live outside this range, at least for 32-bit PV
> guests. To avoid extra is_hvm_*() conditionals, use the alternative VA
> uniformly.
>
> While this could be conditionalized upon CONFIG_PV32 &&
> CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS, omitting such extra conditionals
> keeps the code more legible imo.
>
> Fixes: 4dc181599142 ("x86/PV: harden guest memory accesses against 
> speculative abuse")
> Signed-off-by: Jan Beulich 
>
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -1727,6 +1727,11 @@ void init_xen_l4_slots(l4_pgentry_t *l4t
> (ROOT_PAGETABLE_FIRST_XEN_SLOT + slots -
>  l4_table_offset(XEN_VIRT_START)) * sizeof(*l4t));
>  }
> +
> +/* Slot 511: Per-domain mappings mirror. */
> +if ( !is_pv_64bit_domain(d) )
> +l4t[l4_table_offset(PERDOMAIN2_VIRT_START)] =
> +l4e_from_page(d->arch.perdomain_l3_pg, __PAGE_HYPERVISOR_RW);
 This virtual address is inside the extended directmap.
>>> No. That one covers only the range excluding the last L4 slot.
>>>
   You're going to
 need to rearrange more things than just this, to make it safe.
>>> I specifically picked that entry because I don't think further
>>> arrangements are needed.
>> map_domain_page() will blindly hand this virtual address if an
>> appropriate mfn is passed, because there are no suitability checks.
>>
>> The error handling isn't great, but at least any attempt to use that
>> pointer would fault, which is now no longer the case.
>>
>> LA57 machines can have RAM or NVDIMMs in a range which will tickle this
>> bug.  In fact, they can have MFNs which would wrap around 0 into guest
>> space.
> This latter fact would be a far worse problem than accesses through
> the last L4 entry, populated or not. However, I don't really follow
> your concern: There are ample cases where functions assume to be
> passed sane arguments. A pretty good (imo) comparison here is with
> mfn_to_page(), which also will assume a sane MFN (i.e. one with a
> representable (in frame_table[]) value. If there was a bug, it
> would be either the caller taking an MFN out of thin air, or us
> introducing MFNs we can't cover in any of direct map, frame table,
> or M2P. But afaict there is guarding against the latter (look for
> the "Ignoring inaccessible memory range" loge messages in setup.c).

I'm not trying to say that this patch has introduced the bug.

But we should absolutely have defence in depth where appropriate.  I
don't mind if it is an unrelated change, but 4.15 does start trying to
introduce support for IceLake and I think this qualifies as a reasonable
precaution to add.

> In any event - imo any such bug would need fixing there, rather
> than being an argument against the change here.
>
> Also, besides your objection going quite a bit too far for my taste,
> I miss any form of an alternative suggestion. Do you want the mirror
> range to be put below the canonical boundary? Taking in mind your
> wrapping consideration, about _any_ VA would be unsuitable.

Honestly, I want the XLAT area to disappear entirely.  This is partly
PTSD from the acquire_resource fixes, but was an opinion held from
before that series as well, and I'm convinced that the result without
XLAT will be clearer and faster code.

Obviously, that's not an option at this point in 4.15.


I'd forgotten that the lower half of the address space was available,
and I do prefer that idea.  We don't need to put everything adjacent
together in the upper half.

~Andrew

[PATCH v1] xen: ACPI: Get rid of ACPICA message printing

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

The ACPI_DEBUG_PRINT() macro is used in a few places in
xen-acpi-cpuhotplug.c and xen-acpi-memhotplug.c for printing debug
messages, but that is questionable, because that macro belongs to
ACPICA and it should not be used elsewhere.  In addition,
ACPI_DEBUG_PRINT() requires special enabling to allow it to actually
print the message and the _COMPONENT symbol generally needed for
that is not defined in any of the files in question.

For this reason, replace all of the ACPI_DEBUG_PRINT() instances in
the Xen code with acpi_handle_debug() (with the additional benefit
that the source object can be identified more easily after this
change) and drop the ACPI_MODULE_NAME() definitions that are only
used by the ACPICA message printing macros from that code.

Signed-off-by: Rafael J. Wysocki 
---
 drivers/xen/xen-acpi-cpuhotplug.c |   12 +---
 drivers/xen/xen-acpi-memhotplug.c |   16 +++-
 2 files changed, 12 insertions(+), 16 deletions(-)

Index: linux-pm/drivers/xen/xen-acpi-cpuhotplug.c
===
--- linux-pm.orig/drivers/xen/xen-acpi-cpuhotplug.c
+++ linux-pm/drivers/xen/xen-acpi-cpuhotplug.c
@@ -242,10 +242,10 @@ static void acpi_processor_hotplug_notif
switch (event) {
case ACPI_NOTIFY_BUS_CHECK:
case ACPI_NOTIFY_DEVICE_CHECK:
-   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+   acpi_handle_debug(handle,
"Processor driver received %s event\n",
(event == ACPI_NOTIFY_BUS_CHECK) ?
-   "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK"));
+   "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK");
 
if (!is_processor_present(handle))
break;
@@ -269,8 +269,8 @@ static void acpi_processor_hotplug_notif
break;
 
case ACPI_NOTIFY_EJECT_REQUEST:
-   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "received ACPI_NOTIFY_EJECT_REQUEST\n"));
+   acpi_handle_debug(handle,
+ "received ACPI_NOTIFY_EJECT_REQUEST\n");
 
if (acpi_bus_get_device(handle, )) {
pr_err(PREFIX "Device don't exist, dropping EJECT\n");
@@ -290,8 +290,7 @@ static void acpi_processor_hotplug_notif
break;
 
default:
-   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "Unsupported event [0x%x]\n", event));
+   acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event);
 
/* non-hotplug event; possibly handled by other handler */
goto out;
@@ -440,7 +439,6 @@ static void __exit xen_acpi_processor_ex
 
 module_init(xen_acpi_processor_init);
 module_exit(xen_acpi_processor_exit);
-ACPI_MODULE_NAME("xen-acpi-cpuhotplug");
 MODULE_AUTHOR("Liu Jinsong ");
 MODULE_DESCRIPTION("Xen Hotplug CPU Driver");
 MODULE_LICENSE("GPL");
Index: linux-pm/drivers/xen/xen-acpi-memhotplug.c
===
--- linux-pm.orig/drivers/xen/xen-acpi-memhotplug.c
+++ linux-pm/drivers/xen/xen-acpi-memhotplug.c
@@ -227,13 +227,13 @@ static void acpi_memory_device_notify(ac
 
switch (event) {
case ACPI_NOTIFY_BUS_CHECK:
-   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-   "\nReceived BUS CHECK notification for device\n"));
+   acpi_handle_debug(handle,
+   "Received BUS CHECK notification for device\n");
fallthrough;
case ACPI_NOTIFY_DEVICE_CHECK:
if (event == ACPI_NOTIFY_DEVICE_CHECK)
-   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-   "\nReceived DEVICE CHECK notification for device\n"));
+   acpi_handle_debug(handle,
+   "Received DEVICE CHECK notification for device\n");
 
if (acpi_memory_get_device(handle, _device)) {
pr_err(PREFIX "Cannot find driver data\n");
@@ -244,8 +244,8 @@ static void acpi_memory_device_notify(ac
break;
 
case ACPI_NOTIFY_EJECT_REQUEST:
-   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-   "\nReceived EJECT REQUEST notification for device\n"));
+   acpi_handle_debug(handle,
+   "Received EJECT REQUEST notification for device\n");
 
acpi_scan_lock_acquire();
if (acpi_bus_get_device(handle, )) {
@@ -269,8 +269,7 @@ static void acpi_memory_device_notify(ac
break;
 
default:
-   ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "Unsupported event [0x%x]\n", event));
+   acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event);
/* non-hotplug event; possibly handled by other handler */
return;
  

Re: [RFC PATCH 01/10] sched: core: save IRQ state during locking

[Andrew Cooper]

On 23/02/2021 02:34, Volodymyr Babchuk wrote:
> With XEN preemption enabled, scheduler functions can be called with
> IRQs disabled (for example, at end of IRQ handler), so we should
> save and restore IRQ state in schedulers code.
>
> Signed-off-by: Volodymyr Babchuk 

These functions need to fixed not to be _irq() variants in the first place.

The only reason they're like that is so the schedule softirq/irq can
edit the runqueues.  I seem to recall Dario having a plan to switch the
runqueues to using a lockless update mechanism, which IIRC removes any
need for any of the scheduler locks to be irqs-off.

~Andrew

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 159600

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 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  81b2b328a26c1b89c275898d12e8ab26c0673dad
baseline version:
 xen  5d94433a66df29ce314696a13bdd324ec0e342fe

Last test of basis   159600  2021-02-23 20:01:30 Z0 days
Testing same since   159624  2021-02-24 12:01:29 Z0 days2 attempts


People who touched revisions under test:
  Jan Beulich 
  Roger Pau Monné 

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



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

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

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

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


Not pushing.


commit 81b2b328a26c1b89c275898d12e8ab26c0673dad
Author: Roger Pau Monné 
Date:   Wed Feb 24 12:48:13 2021 +0100

hvmloader: use Xen private header for elf structs

Do not use the system provided elf.h, and instead use elfstructs.h
from libelf.

Signed-off-by: Roger Pau Monné 
Acked-by: Jan Beulich 
Reviewed-by: Ian Jackson 
Release-Acked-by: Ian Jackson 

commit 10bb8aa0d5d029bd56da4a2a92e1e42bef880210
Author: Jan Beulich 
Date:   Wed Feb 24 12:47:34 2021 +0100

build: remove more absolute paths from dependency tracking files

d6b12add90da ("DEPS handling: Remove absolute paths from references to
cwd") took care of massaging the dependencies of the output file, but
for our passing of -MP to the compiler to take effect the same needs to
be done on the "phony" rules that the compiler emits.

Signed-off-by: Jan Beulich 
Reviewed-by: Ian Jackson 
Release-Acked-by: Ian Jackson 
(qemu changes not included)

[qemu-mainline test] 159606: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat fail REGR. vs. 152631
 test-amd64-amd64-xl-qcow2   21 guest-start/debian.repeat fail REGR. vs. 152631
 test-armhf-armhf-xl-vhd 17 guest-start/debian.repeat fail REGR. vs. 152631

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

version targeted for testing:
 qemuu7ef8134565dccf9186d5eabd7dbb4ecae6dead87
baseline version:
 qemuu1d806cef0e38b5db8347a8e12f214d543204a314

Last test of basis   152631  2020-08-20 09:07:46 Z  188 days
Failing since152659  2020-08-21 14:07:39 Z  187 days  361 attempts
Testing same since   159563  2021-02-22 23:37:57 Z1 days3 attempts


425 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm 

Re: [RFC PATCH 00/10] Preemption in hypervisor (ARM only)

[Andrew Cooper]

On 23/02/2021 02:34, Volodymyr Babchuk wrote:
> Hello community,
>
> Subject of this cover letter is quite self-explanatory. This patch
> series implements PoC for preemption in hypervisor mode.
>
> This is the sort of follow-up to recent discussion about latency
> ([1]).
>
> Motivation
> ==
>
> It is well known that Xen is not preemptable. On other words, it is
> impossible to switch vCPU contexts while running in hypervisor
> mode. Only one place where scheduling decision can be made and one
> vCPU can be replaced with another is the exit path from the hypervisor
> mode. The one exception are Idle vCPUs, which never leaves the
> hypervisor mode for obvious reasons.
>
> This leads to a number of problems. This list is not comprehensive. It
> lists only things that I or my colleagues encountered personally.
>
> Long-running hypercalls. Due to nature of some hypercalls they can
> execute for arbitrary long time. Mostly those are calls that deal with
> long list of similar actions, like memory pages processing. To deal
> with this issue Xen employs most horrific technique called "hypercall
> continuation". When code that handles hypercall decides that it should
> be preempted, it basically updates the hypercall parameters, and moves
> guest PC one instruction back. This causes guest to re-execute the
> hypercall with altered parameters, which will allow hypervisor to
> continue hypercall execution later. This approach itself have obvious
> problems: code that executes hypercall is responsible for preemption,
> preemption checks are infrequent (because they are costly by
> themselves), hypercall execution state is stored in guest-controlled
> area, we rely on guest's good will to continue the hypercall. All this
> imposes restrictions on which hypercalls can be preempted, when they
> can be preempted and how to write hypercall handlers. Also, it
> requires very accurate coding and already led to at least one
> vulnerability - XSA-318. Some hypercalls can not be preempted at all,
> like the one mentioned in [1].
>
> Absence of hypervisor threads/vCPUs. Hypervisor owns only idle vCPUs,
> which are supposed to run when the system is idle. If hypervisor needs
> to execute own tasks that are required to run right now, it have no
> other way than to execute them on current vCPU. But scheduler does not
> know that hypervisor executes hypervisor task and accounts spent time
> to a domain. This can lead to domain starvation.
>
> Also, absence of hypervisor threads leads to absence of high-level
> synchronization primitives like mutexes, conditional variables,
> completions, etc. This leads to two problems: we need to use spinlocks
> everywhere and we have problems when porting device drivers from linux
> kernel.

You cannot reenter a guest, even to deliver interrupts, if pre-empted at
an arbitrary point in a hypercall.  State needs unwinding suitably.

Xen's non-preemptible-ness is designed to specifically force you to not
implement long-running hypercalls which would interfere with timely
interrupt handling in the general case.

Hypervisor/virt properties are different to both a kernel-only-RTOS, and
regular usespace.  This was why I gave you some specific extra scenarios
to do latency testing with, so you could make a fair comparison of
"extra overhead caused by Xen" separate from "overhead due to
fundamental design constraints of using virt".


Preemption like this will make some benchmarks look better, but it also
introduces the ability to create fundamental problems, like preventing
any interrupt delivery into a VM for seconds of wallclock time while
each vcpu happens to be in a long-running hypercall.

If you want timely interrupt handling, you either need to partition your
workloads by the long-running-ness of their hypercalls, or not have
long-running hypercalls.

I remain unconvinced that preemption is an sensible fix to the problem
you're trying to solve.

~Andrew

Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Andrew Cooper]

On 23/02/2021 07:53, Jan Beulich wrote:
> On 22.02.2021 17:36, Andrew Cooper wrote:
>> On 19/02/2021 08:09, Jan Beulich wrote:
>>> --- a/xen/arch/x86/Makefile
>>> +++ b/xen/arch/x86/Makefile
>>> @@ -123,8 +123,13 @@ ifneq ($(efi-y),)
>>>  # Check if the compiler supports the MS ABI.
>>>  export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o 
>>> efi/check.o 2>/dev/null && echo y)
>>>  # Check if the linker supports PE.
>>> -XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep 
>>> --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>> +EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 
>>> --strip-debug
>>> +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o 
>>> efi/check.efi efi/check.o 2>/dev/null && echo y))
>>>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>>> +# Check if the linker produces fixups in PE by default (we need to disable 
>>> it doing so for now).
>>> +XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
>>> + $(shell $(LD) $(EFI_LDFLAGS) 
>>> --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
>>> + echo --disable-reloc-section))
>> Why does --strip-debug move?
> -S and --strip-debug are the same. I'm simply accumulating in
> EFI_LDFLAGS all that's needed for the use in the probing construct.

Oh ok.

It occurs to me that EFI_LDFLAGS now only gets started in an ifneq
block, but appended to later on while unprotected.  That said, I'm
fairly sure it is only consumed inside a different ifeq section, so I
think there is a reasonable quantity of tidying which ought to be done here.

> Also I meanwhile have a patch to retain debug info, for which this
> movement turns out to be a prereq. (I've yet to test that the
> produced binary actually works, and what's more I first need to get
> a couple of changes accepted into binutils for the linker to actually
> cope.)
>
>> What's wrong with $(call ld-option ...) ?  Actually, lots of this block
>> of code looks to be opencoding of standard constructs.
> It looks like ld-option could indeed be used here (there are marginal
> differences which are likely acceptable), despite its brief comment
> talking of just "flag" (singular, plus not really covering e.g. input
> files).
>
> But:
> - It working differently than cc-option makes it inconsistent to
>   use (the setting of XEN_BUILD_EFI can't very well be switched to
>   use cc-option); because of this I'm not surprised that we have
>   only exactly one use right now in the tree.
> - While XEN_BUILD_PE wants to be set to "y", for XEN_NO_PE_FIXUPS
>   another transformation would then be necessary to translate "y"
>   into "--disable-reloc-section".
> - Do you really suggest to re-do this at this point in the release
>   cycle?

I'm looking to prevent this almost-incompressible mess from getting worse.

But I suppose you want this to backport, so I suppose it ought to be
minimally invasive.

Acked-by: Andrew Cooper 

This logic all actually needs moving into Kconfig so we can also go
about fixing the other bugs we have such as having Multiboot headers in
xen.efi pointing at unusable entrypoints.

~Andrew

[xen-unstable-smoke bisection] complete build-amd64

[osstest service owner]

branch xen-unstable-smoke
xenbranch xen-unstable-smoke
job build-amd64
testid xen-build

Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  xen git://xenbits.xen.org/xen.git
  Bug introduced:  81b2b328a26c1b89c275898d12e8ab26c0673dad
  Bug not present: 10bb8aa0d5d029bd56da4a2a92e1e42bef880210
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/159638/


  commit 81b2b328a26c1b89c275898d12e8ab26c0673dad
  Author: Roger Pau Monné 
  Date:   Wed Feb 24 12:48:13 2021 +0100
  
  hvmloader: use Xen private header for elf structs
  
  Do not use the system provided elf.h, and instead use elfstructs.h
  from libelf.
  
  Signed-off-by: Roger Pau Monné 
  Acked-by: Jan Beulich 
  Reviewed-by: Ian Jackson 
  Release-Acked-by: Ian Jackson 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable-smoke/build-amd64.xen-build.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/xen-unstable-smoke/build-amd64.xen-build 
--summary-out=tmp/159638.bisection-summary --basis-template=159600 
--blessings=real,real-bisect,real-retry xen-unstable-smoke build-amd64 xen-build
Searching for failure / basis pass:
 159624 fail [host=himrod2] / 159600 ok.
Failure / basis pass flights: 159624 / 159600
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Latest 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
81b2b328a26c1b89c275898d12e8ab26c0673dad
Basis pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
5d94433a66df29ce314696a13bdd324ec0e342fe
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/qemu-xen-traditional.git#3d273dd05e51e5a1ffba3d98c7437ee84e8f8764-3d273dd05e51e5a1ffba3d98c7437ee84e8f8764
 
git://xenbits.xen.org/qemu-xen.git#7ea428895af2840d85c524f0bd11a38aac308308-7ea428895af2840d85c524f0bd11a38aac308308
 
git://xenbits.xen.org/xen.git#5d94433a66df29ce314696a13bdd324ec0e342fe-81b2b328a26c1b89c275898d12e8ab26c0673dad
Loaded 5001 nodes in revision graph
Searching for test results:
 159600 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
5d94433a66df29ce314696a13bdd324ec0e342fe
 159624 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
81b2b328a26c1b89c275898d12e8ab26c0673dad
 159627 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
5d94433a66df29ce314696a13bdd324ec0e342fe
 159630 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
81b2b328a26c1b89c275898d12e8ab26c0673dad
 159631 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
10bb8aa0d5d029bd56da4a2a92e1e42bef880210
 159632 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
81b2b328a26c1b89c275898d12e8ab26c0673dad
 159634 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
10bb8aa0d5d029bd56da4a2a92e1e42bef880210
 159636 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
81b2b328a26c1b89c275898d12e8ab26c0673dad
 159637 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
10bb8aa0d5d029bd56da4a2a92e1e42bef880210
 159638 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
81b2b328a26c1b89c275898d12e8ab26c0673dad
Searching for interesting versions
 Result found: flight 159600 (pass), for basis pass
 For basis failure, parent search stopping at 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
10bb8aa0d5d029bd56da4a2a92e1e42bef880210, results HASH(0x55aef3ccb2a8) 
HASH(0x55aef3cc4f68) HASH(0x55aef3cceb38) For basis failure, parent search 
stopping at 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
5d94433a66df29ce314696a13bdd324ec0e342fe, results HASH(0x55aef3cc24e0) 
HASH(0x55aef3cc2ae0) Result found: flight 159624 (fail), for \
 basis failure (at ancestor ~79)
 Repro found: flight 159627 (pass), for basis pass
 Repro found: flight 159630 (fail), for basis failure
 0 revisions at 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
7ea428895af2840d85c524f0bd11a38aac308308 
10bb8aa0d5d029bd56da4a2a92e1e42bef880210
No revisions left to test, checking graph state.
 Result found: flight 159631 (pass), for last pass
 Result found: flight 159632 (fail), for first failure
 Repro found: 

[ovmf test] 159619: all pass - PUSHED

[osstest service owner]

flight 159619 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/159619/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 739a506b18c4f694b8d5d2f3424a329c45d737ba
baseline version:
 ovmf 68e5ecc4d208fe900530fdbe6b44dfc85bef60a8

Last test of basis   159598  2021-02-23 19:09:45 Z0 days
Testing same since   159619  2021-02-24 08:09:48 Z0 days1 attempts


People who touched revisions under test:
  Abner Chang 
  Leif Lindholm 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  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/osstest/ovmf.git
   68e5ecc4d2..739a506b18  739a506b18c4f694b8d5d2f3424a329c45d737ba -> 
xen-tested-master

io-apic issue on Ryzen 1800X

[Frédéric Pierret]

Hi,
Just to inform you that after talking with Andrew, we identified that latest 
workaround for recent Intel laptop (this series of patches 
https://github.com/QubesOS/qubes-vmm-xen/commit/075e6b1) is failing on my 
system having AMD Ryzen 1800X CPU and Asrock X370 Gaming K4 as motherboard.

Related Qubes issue tracking this: 
https://github.com/QubesOS/qubes-issues/issues/6423

Best regards,
Frédéric



OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] xen-netback: correct success/error reporting for the SKB-with-fraglist case

[Paul Durrant]

On 23/02/2021 16:29, Jan Beulich wrote:

When re-entering the main loop of xenvif_tx_check_gop() a 2nd time, the
special considerations for the head of the SKB no longer apply. Don't
mistakenly report ERROR to the frontend for the first entry in the list,
even if - from all I can tell - this shouldn't matter much as the overall
transmit will need to be considered failed anyway.

Signed-off-by: Jan Beulich 

--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -499,7 +499,7 @@ check_frags:
 * the header's copy failed, and they are
 * sharing a slot, send an error
 */
-   if (i == 0 && sharedslot)
+   if (i == 0 && !first_shinfo && sharedslot)
xenvif_idx_release(queue, pending_idx,
   XEN_NETIF_RSP_ERROR);
else



I think this will DTRT, but to my mind it would make more sense to clear 
'sharedslot' before the 'goto check_frags' at the bottom of the function.


  Paul

Re: [PATCH] drm/xen: adjust Kconfig

[Daniel Vetter]

On Wed, Feb 24, 2021 at 8:55 AM Oleksandr Andrushchenko
 wrote:
>
> Hello, Jan!
>
> On 2/23/21 6:41 PM, Jan Beulich wrote:
> > By having selected DRM_XEN, I was assuming I would build the frontend
> > driver. As it turns out this is a dummy option, and I have really not
> > been building this (because I had DRM disabled). Make it a promptless
> > one, moving the "depends on" to the other, real option, and "select"ing
> > the dummy one.
> >
> > Signed-off-by: Jan Beulich 
> Reviewed-by: Oleksandr Andrushchenko 

Since you're maintainer/committer, I'm assuming you'll also merge
this? Always confusing when there's an r-b but nothing about whether
the patch will get merged or not.
-Daniel

> > --- a/drivers/gpu/drm/xen/Kconfig
> > +++ b/drivers/gpu/drm/xen/Kconfig
> > @@ -1,15 +1,11 @@
> >   # SPDX-License-Identifier: GPL-2.0-only
> >   config DRM_XEN
> > - bool "DRM Support for Xen guest OS"
> > - depends on XEN
> > - help
> > -   Choose this option if you want to enable DRM support
> > -   for Xen.
> > + bool
> >
> >   config DRM_XEN_FRONTEND
> >   tristate "Para-virtualized frontend driver for Xen guest OS"
> > - depends on DRM_XEN
> > - depends on DRM
> > + depends on XEN && DRM
> > + select DRM_XEN
> >   select DRM_KMS_HELPER
> >   select VIDEOMODE_HELPERS
> >   select XEN_XENBUS_FRONTEND
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/2] hvmloader: drop usage of system headers

[Stefano Stabellini]

Hi Roger,

These seems to be genuine breakages:

https://gitlab.com/xen-project/patchew/xen/-/pipelines/260986219

FYI keep an eye on the patchew gitlab-ci as you should be able to see
the alpine linux tests pass once your series is fixed.

Cheers,

Stefano


On Wed, 24 Feb 2021, no-re...@patchew.org wrote:
> Hi,
> 
> Patchew automatically ran gitlab-ci pipeline with this patch (series) 
> applied, but the job failed. Maybe there's a bug in the patches?
> 
> You can find the link to the pipeline near the end of the report below:
> 
> Type: series
> Message-id: 20210224102641.89455-1-roger@citrix.com
> Subject: [PATCH 0/2] hvmloader: drop usage of system headers
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> sleep 10
> patchew gitlab-pipeline-check -p xen-project/patchew/xen
> === TEST SCRIPT END ===
> 
> warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
> warning: redirecting to https://gitlab.com/xen-project/patchew/xen.git/
> From https://gitlab.com/xen-project/patchew/xen
>  * [new tag]   
> patchew/20210224102641.89455-1-roger@citrix.com -> 
> patchew/20210224102641.89455-1-roger@citrix.com
> Switched to a new branch 'test'
> 471a8f9278 hvmloader: do not include system headers for type declarations
> f826f3a198 hvmloader: use Xen private header for elf structs
> 
> === OUTPUT BEGIN ===
> [2021-02-24 11:27:25] Looking up pipeline...
> [2021-02-24 11:27:27] Found pipeline 260986219:
> 
> https://gitlab.com/xen-project/patchew/xen/-/pipelines/260986219
> 
> [2021-02-24 11:27:27] Waiting for pipeline to finish...
> [2021-02-24 11:42:41] Still waiting...
> [2021-02-24 11:57:46] Still waiting...
> [2021-02-24 12:12:51] Still waiting...
> [2021-02-24 12:20:56] Pipeline failed
> [2021-02-24 12:20:58] Job 'qemu-smoke-x86-64-clang-pvh' in stage 'test' is 
> skipped
> [2021-02-24 12:20:58] Job 'qemu-smoke-x86-64-gcc-pvh' in stage 'test' is 
> skipped
> [2021-02-24 12:20:58] Job 'qemu-smoke-x86-64-clang' in stage 'test' is skipped
> [2021-02-24 12:20:58] Job 'qemu-smoke-x86-64-gcc' in stage 'test' is skipped
> [2021-02-24 12:20:58] Job 'qemu-smoke-arm64-gcc' in stage 'test' is skipped
> [2021-02-24 12:20:58] Job 'qemu-alpine-arm64-gcc' in stage 'test' is skipped
> [2021-02-24 12:20:58] Job 'build-each-commit-gcc' in stage 'test' is skipped
> [2021-02-24 12:20:58] Job 'alpine-3.12-clang-debug' in stage 'build' is failed
> [2021-02-24 12:20:58] Job 'alpine-3.12-clang' in stage 'build' is failed
> [2021-02-24 12:20:58] Job 'alpine-3.12-gcc-debug' in stage 'build' is failed
> [2021-02-24 12:20:58] Job 'alpine-3.12-gcc' in stage 'build' is failed
> === OUTPUT END ===
> 
> Test command exited with code: 1

Re: [for-4.15][RESEND PATCH v4 2/2] xen/iommu: x86: Clear the root page-table before freeing the page-tables

[Jan Beulich]

On 24.02.2021 16:58, Jan Beulich wrote:
> On 24.02.2021 10:43, Julien Grall wrote:
>> --- a/xen/drivers/passthrough/x86/iommu.c
>> +++ b/xen/drivers/passthrough/x86/iommu.c
>> @@ -149,6 +149,13 @@ int arch_iommu_domain_init(struct domain *d)
>>  
>>  void arch_iommu_domain_destroy(struct domain *d)
>>  {
>> +/*
>> + * There should be not page-tables left allocated by the time the
>> + * domain is destroyed. Note that arch_iommu_domain_destroy() is
>> + * called unconditionally, so pgtables may be unitialized.
>> + */
>> +ASSERT(dom_iommu(d)->platform_ops == NULL ||
> 
> Since you've used the preferred ! instead of "== 0" /
> "== NULL" in the ASSERT()s you add further up, may I ask that
> you do so here as well?

Oh, and I meant to provide
Reviewed-by: Jan Beulich 
preferably with that cosmetic adjustment (and ideally also with
"uninitialized" in the comment, as I notice only now).

Jan

Re: [PATCH for-4.15] elfstructs: add relocation defines for i386

[Roger Pau Monné]

On Wed, Feb 24, 2021 at 04:01:13PM +0100, Jan Beulich wrote:
> On 24.02.2021 15:58, Roger Pau Monne wrote:
> > Those are need by the rombios relocation code in hvmloader. Fixes the
> > following build error:
> > 
> > 32bitbios_support.c: In function 'relocate_32bitbios':
> > 32bitbios_support.c:130:18: error: 'R_386_PC32' undeclared (first use in 
> > this function); did you mean 'R_X86_64_PC32'?
> >  case R_386_PC32:
> >   ^~
> >   R_X86_64_PC32
> > 32bitbios_support.c:130:18: note: each undeclared identifier is reported 
> > only once for each function it appears in
> > 32bitbios_support.c:134:18: error: 'R_386_32' undeclared (first use in this 
> > function)
> >  case R_386_32:
> >   ^~~~
> > 
> > Only add the two defines that are actually used, which seems to match
> > what we do for amd64.
> > 
> > Fixes: 81b2b328a26c1b ('hvmloader: use Xen private header for elf structs')
> > Signed-off-by: Roger Pau Monné 
> 
> In principle
> Reviewed-by: Jan Beulich 
> 
> But ...
> 
> > --- a/xen/include/xen/elfstructs.h
> > +++ b/xen/include/xen/elfstructs.h
> > @@ -348,6 +348,9 @@ typedef struct {
> >  #define ELF32_R_TYPE(i)((unsigned char) (i))
> >  #define ELF32_R_INFO(s,t)  (((s) << 8) + (unsigned char)(t))
> >  
> > +#define R_386_32   1/* Direct 32 bit  */
> > +#define R_386_PC32 2/* PC relative 32 bit */
> > +
> >  typedef struct {
> > Elf64_Addr  r_offset;   /* where to do it */
> > Elf64_Xword r_info; /* index & type of relocation */
> 
> ... I'm heavily inclined to move this a few lines down to where
> the other relocation types get defined, and add a respective
> comment.

I've placed it together with the other 32bit ELF relocation structures
and macros, but I see the rest of the relocation defines are a bit
below, feel free to move it. For a comment:

/*
 * Relocation definitions required by the rombios hvmloader relocation
 * code.
 */

Does seem fine?

Thanks, Roger.

Re: [PATCH for-4.15] elfstructs: add relocation defines for i386

[Ian Jackson]

Roger Pau Monne writes ("[PATCH for-4.15] elfstructs: add relocation defines 
for i386"):
> Those are need by the rombios relocation code in hvmloader. Fixes the
> following build error:
> 
> 32bitbios_support.c: In function 'relocate_32bitbios':
> 32bitbios_support.c:130:18: error: 'R_386_PC32' undeclared (first use in this 
> function); did you mean 'R_X86_64_PC32'?
>  case R_386_PC32:
>   ^~
>   R_X86_64_PC32
> 32bitbios_support.c:130:18: note: each undeclared identifier is reported only 
> once for each function it appears in
> 32bitbios_support.c:134:18: error: 'R_386_32' undeclared (first use in this 
> function)
>  case R_386_32:
>   ^~~~
> 
> Only add the two defines that are actually used, which seems to match
> what we do for amd64.
> 
> Fixes: 81b2b328a26c1b ('hvmloader: use Xen private header for elf structs')
> Signed-off-by: Roger Pau Monné 

Release-Acked-by: Ian Jackson 

Re: [PATCH for-4.15] elfstructs: add relocation defines for i386

[Jan Beulich]

On 24.02.2021 15:58, Roger Pau Monne wrote:
> Those are need by the rombios relocation code in hvmloader. Fixes the
> following build error:
> 
> 32bitbios_support.c: In function 'relocate_32bitbios':
> 32bitbios_support.c:130:18: error: 'R_386_PC32' undeclared (first use in this 
> function); did you mean 'R_X86_64_PC32'?
>  case R_386_PC32:
>   ^~
>   R_X86_64_PC32
> 32bitbios_support.c:130:18: note: each undeclared identifier is reported only 
> once for each function it appears in
> 32bitbios_support.c:134:18: error: 'R_386_32' undeclared (first use in this 
> function)
>  case R_386_32:
>   ^~~~
> 
> Only add the two defines that are actually used, which seems to match
> what we do for amd64.
> 
> Fixes: 81b2b328a26c1b ('hvmloader: use Xen private header for elf structs')
> Signed-off-by: Roger Pau Monné 

In principle
Reviewed-by: Jan Beulich 

But ...

> --- a/xen/include/xen/elfstructs.h
> +++ b/xen/include/xen/elfstructs.h
> @@ -348,6 +348,9 @@ typedef struct {
>  #define ELF32_R_TYPE(i)  ((unsigned char) (i))
>  #define ELF32_R_INFO(s,t)(((s) << 8) + (unsigned char)(t))
>  
> +#define R_386_32   1/* Direct 32 bit  */
> +#define R_386_PC32 2/* PC relative 32 bit */
> +
>  typedef struct {
>   Elf64_Addr  r_offset;   /* where to do it */
>   Elf64_Xword r_info; /* index & type of relocation */

... I'm heavily inclined to move this a few lines down to where
the other relocation types get defined, and add a respective
comment.

Jan

[PATCH for-4.15] elfstructs: add relocation defines for i386

[Roger Pau Monne]

Those are need by the rombios relocation code in hvmloader. Fixes the
following build error:

32bitbios_support.c: In function 'relocate_32bitbios':
32bitbios_support.c:130:18: error: 'R_386_PC32' undeclared (first use in this 
function); did you mean 'R_X86_64_PC32'?
 case R_386_PC32:
  ^~
  R_X86_64_PC32
32bitbios_support.c:130:18: note: each undeclared identifier is reported only 
once for each function it appears in
32bitbios_support.c:134:18: error: 'R_386_32' undeclared (first use in this 
function)
 case R_386_32:
  ^~~~

Only add the two defines that are actually used, which seems to match
what we do for amd64.

Fixes: 81b2b328a26c1b ('hvmloader: use Xen private header for elf structs')
Signed-off-by: Roger Pau Monné 
---
 xen/include/xen/elfstructs.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/xen/elfstructs.h b/xen/include/xen/elfstructs.h
index 726ca8f60d..d1054ae380 100644
--- a/xen/include/xen/elfstructs.h
+++ b/xen/include/xen/elfstructs.h
@@ -348,6 +348,9 @@ typedef struct {
 #define ELF32_R_TYPE(i)((unsigned char) (i))
 #define ELF32_R_INFO(s,t)  (((s) << 8) + (unsigned char)(t))
 
+#define R_386_32   1/* Direct 32 bit  */
+#define R_386_PC32 2/* PC relative 32 bit */
+
 typedef struct {
Elf64_Addr  r_offset;   /* where to do it */
Elf64_Xword r_info; /* index & type of relocation */
-- 
2.30.1

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations [and 1 more messages]

[Jan Beulich]

On 24.02.2021 15:33, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH 2/2] hvmloader: do not include system 
> headers for type declarations"):
>> At what point do we just declare Alpine broken?  These are all
>> freestanding headers, an explicitly available for use, in the way we use
>> them.
> 
> There is IMO nothing wrong with Alpine here.  Alpine amd64 simply does
> not support compilation of 32-bit x86 userland binaries.
> 
> But that's OK for us.  Xen doe not require the execution of any 32-bit
> userland binaries.  hvmloader is not a userland binary.
> 
> As Roger said on irc
> 
> 13:35  but requiring a compiler that supports generating
>i386 code doens't imply that we also have a libc for it?
>
>> There are substantial portability costs for making changes like this,
>> which takes us from standards compliant C to GCC-isms-only.
> 
> Since we are defining our out standalone environment for hvmloader, we
> are in the position of the C *implementor*.  Compilers have features
> (like __builtin_va*) that are helpful for implementing standard C
> features like stdarg.h and indeed stdint.h.
> 
> Or to put it another way, GCC does not, by itself, provide (in
> Standard C terms) a "freestanding implementation".  Arguably GCC ought
> to provide stdint.h et al but in practice it doing so causes more
> trouble as it gets in the way of the implentors of hosted
> implementations.

But gcc _does_ provide a stdint.h.

> Jan Beulich writes ("Re: [PATCH 2/2] hvmloader: do not include system headers 
> for type declarations"):
>> On 24.02.2021 12:07, Ian Jackson wrote:
>>> This code is only ever going to be for 32-bit x86, so I think the way
>>> Roger did it is fine.
>>
>> It is technically correct at this point in time, from all we can
>> tell. I can't see any reason though why a compiler might not
>> support wider int or, in particular, long long.
> 
> Our requirement for hvmloader is that we have an ILP32 LL64 compiler
> which generates 32-bit x86 machine code.  That is what "gcc -m32"
> means.

I'm not sure about the last statement; I'm pretty sure we don't
check that we have such a compiler (in tools/configure).

>  Whether future compiler(s) might exist which can provide ILP32
> LLP64 (and what type uint64_t is on such a compiler) is not relevant.
> 
>>> Doing it the other way, to cope with this file being used with
>>> compiler settings where the above set of types is wrong, would also
>>> imply more complex definitions of INT32_MIN et al.
>>
>> Well, that's only as far as the use of number suffixes goes. The
>> values used won't change, as these constants describe fixed width
>> types.
> 
> So the definitions would need to contain casts.

Which they can't, as that would make them unusable in preprocessor
directives.

 Like the hypervisor, we should prefer using __SIZE_TYPE__
 when available.
>>>
>>> I disagree.
>>
>> May I ask why? There is a reason providing of these types did get
>> added to (at least) gcc.
> 
> __SIZE_TYPE__ is provided by the compiler to the libc implementor.  It
> is one of those facilities like __builtin_va*.  The bulk of the code
> in hvmloader should not use this kind of thing.  It should use plain
> size_t.
> 
> As for the new header in hvmloader, it does not matter whether it uses
> __SIZE_TYPE__ or some other type which is known to be 32-bit, since
> this code is definitely only ever for 32-bit x86.

>From a compiler perspective, "32-bit" and "x86" needs further pairing
with an OS, as it's the OS which defines the ABI. This is why further
up I did say "It is technically correct at this point in time, from
all we can tell" - we imply that all OSes we want to be able to build
on provide a suitable ABI, so we can use their compilers.

>> One argument against this would be above mentioned independence
>> on any ABI the compiler would be built for, but I'd buy that only
>> if above we indeed used __attribute__((__mode__())), as that's
>> the only way to achieve such independence.
> 
> We don't want or need to support building hvmloader with a differnet
> ABI.
> 
 Nit: Perhaps better omit the unnecessary inner parentheses?
>>>
>>> We should definitely keep the inner parentheses.  I don't want to
>>> start carefully reasoning about precisely which inner parentheses are
>>> necesary for macro argument parsing correctness.
>>
>> Can you give me an example of when the inner parentheses would be
>> needed? I don't think they're needed no matter whether (taking the
>> example here) __builtin_va_...() were functions or macros. They
>> would of course be needed if the identifiers were part of
>> expressions beyond the mere function invocation.
> 
> You mention the situation where the parentheses would be needed
> yourself.

Okay, if that would have been your example, then since there are
no further expressions involved here you agree parentheses aren't
needed here?

JAn

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations [and 1 more messages]

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH 2/2] hvmloader: do not include system headers 
for type declarations"):
> At what point do we just declare Alpine broken?  These are all
> freestanding headers, an explicitly available for use, in the way we use
> them.

There is IMO nothing wrong with Alpine here.  Alpine amd64 simply does
not support compilation of 32-bit x86 userland binaries.

But that's OK for us.  Xen doe not require the execution of any 32-bit
userland binaries.  hvmloader is not a userland binary.

As Roger said on irc

13:35  but requiring a compiler that supports generating
   i386 code doens't imply that we also have a libc for it?
   
> There are substantial portability costs for making changes like this,
> which takes us from standards compliant C to GCC-isms-only.

Since we are defining our out standalone environment for hvmloader, we
are in the position of the C *implementor*.  Compilers have features
(like __builtin_va*) that are helpful for implementing standard C
features like stdarg.h and indeed stdint.h.

Or to put it another way, GCC does not, by itself, provide (in
Standard C terms) a "freestanding implementation".  Arguably GCC ought
to provide stdint.h et al but in practice it doing so causes more
trouble as it gets in the way of the implentors of hosted
implementations.

The conclusion is simply that we must provide for ourselves any
apsects of a "freestanding implementation" that we care about.  (And
then we get to decide for ourselves how much the internal API should
look like Standard C.  Defining the Standard C type names is
definitely IMO advisable as it makes the bulk of the code sensible to
read.)

Jan Beulich writes ("Re: [PATCH 2/2] hvmloader: do not include system headers 
for type declarations"):
> On 24.02.2021 12:07, Ian Jackson wrote:
> > This code is only ever going to be for 32-bit x86, so I think the way
> > Roger did it is fine.
> 
> It is technically correct at this point in time, from all we can
> tell. I can't see any reason though why a compiler might not
> support wider int or, in particular, long long.

Our requirement for hvmloader is that we have an ILP32 LL64 compiler
which generates 32-bit x86 machine code.  That is what "gcc -m32"
means.  Whether future compiler(s) might exist which can provide ILP32
LLP64 (and what type uint64_t is on such a compiler) is not relevant.

> > Doing it the other way, to cope with this file being used with
> > compiler settings where the above set of types is wrong, would also
> > imply more complex definitions of INT32_MIN et al.
> 
> Well, that's only as far as the use of number suffixes goes. The
> values used won't change, as these constants describe fixed width
> types.

So the definitions would need to contain casts.

> >> Like the hypervisor, we should prefer using __SIZE_TYPE__
> >> when available.
> > 
> > I disagree.
> 
> May I ask why? There is a reason providing of these types did get
> added to (at least) gcc.

__SIZE_TYPE__ is provided by the compiler to the libc implementor.  It
is one of those facilities like __builtin_va*.  The bulk of the code
in hvmloader should not use this kind of thing.  It should use plain
size_t.

As for the new header in hvmloader, it does not matter whether it uses
__SIZE_TYPE__ or some other type which is known to be 32-bit, since
this code is definitely only ever for 32-bit x86.

> One argument against this would be above mentioned independence
> on any ABI the compiler would be built for, but I'd buy that only
> if above we indeed used __attribute__((__mode__())), as that's
> the only way to achieve such independence.

We don't want or need to support building hvmloader with a differnet
ABI.

> >> Nit: Perhaps better omit the unnecessary inner parentheses?
> > 
> > We should definitely keep the inner parentheses.  I don't want to
> > start carefully reasoning about precisely which inner parentheses are
> > necesary for macro argument parsing correctness.
> 
> Can you give me an example of when the inner parentheses would be
> needed? I don't think they're needed no matter whether (taking the
> example here) __builtin_va_...() were functions or macros. They
> would of course be needed if the identifiers were part of
> expressions beyond the mere function invocation.

You mention the situation where the parentheses would be needed
yourself.

> We've been trying to eliminate such in the hypervisor part of the
> tree,

Really ?  Well I think that is in exceedingly poor taste.  I can't
claim that it's objectively wrong and this is a question of style so I
won't insist on it.

> The primary reason why I've been advocating to avoid them is that,
> as long as they're not needed for anything, they harm readability
> and increase the risk of mistakes like the one that had led to
> XSA-316.

I looked again at XSA-316.  I don't want to open another can of worms.
It will suffice to say that I don't share your view on the root cause.

Ian.

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 159600

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 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  81b2b328a26c1b89c275898d12e8ab26c0673dad
baseline version:
 xen  5d94433a66df29ce314696a13bdd324ec0e342fe

Last test of basis   159600  2021-02-23 20:01:30 Z0 days
Testing same since   159624  2021-02-24 12:01:29 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Roger Pau Monné 

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



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

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

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

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


Not pushing.


commit 81b2b328a26c1b89c275898d12e8ab26c0673dad
Author: Roger Pau Monné 
Date:   Wed Feb 24 12:48:13 2021 +0100

hvmloader: use Xen private header for elf structs

Do not use the system provided elf.h, and instead use elfstructs.h
from libelf.

Signed-off-by: Roger Pau Monné 
Acked-by: Jan Beulich 
Reviewed-by: Ian Jackson 
Release-Acked-by: Ian Jackson 

commit 10bb8aa0d5d029bd56da4a2a92e1e42bef880210
Author: Jan Beulich 
Date:   Wed Feb 24 12:47:34 2021 +0100

build: remove more absolute paths from dependency tracking files

d6b12add90da ("DEPS handling: Remove absolute paths from references to
cwd") took care of massaging the dependencies of the output file, but
for our passing of -MP to the compiler to take effect the same needs to
be done on the "phony" rules that the compiler emits.

Signed-off-by: Jan Beulich 
Reviewed-by: Ian Jackson 
Release-Acked-by: Ian Jackson 
(qemu changes not included)

Re: [for-4.15][RESEND PATCH v4 1/2] xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying

[Jan Beulich]

On 24.02.2021 10:43, Julien Grall wrote:
> From: Julien Grall 
> 
> The new x86 IOMMU page-tables allocator will release the pages when
> relinquishing the domain resources. However, this is not sufficient
> when the domain is dying because nothing prevents page-table to be
> allocated.
> 
> As the domain is dying, it is not necessary to continue to modify the
> IOMMU page-tables as they are going to be destroyed soon.
> 
> At the moment, page-table allocates will only happen when iommu_map().
> So after this change there will be no more page-table allocation
> happening.

While I'm still not happy about this asymmetry, I'm willing to accept
it in the interest of getting the underlying issue addressed. May I
ask though that you add something like "... because we don't use
superpage mappings yet when not sharing page tables"?

But there are two more minor things:

> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -267,6 +267,12 @@ int iommu_free_pgtables(struct domain *d)
>  struct page_info *pg;
>  unsigned int done = 0;
>  
> +if ( !is_iommu_enabled(d) )
> +return 0;

Why is this addition needed? Hitting a not yet initialize spin lock
is - afaict - no worse than a not yet initialized list, so it would
seem to me that this can't be the reason. No other reason looks to
be called out by the description.

> +/* After this barrier, no more IOMMU mapping can happen */
> +spin_barrier(>arch.mapping_lock);

On the v3 discussion I thought you did agree to change the wording
of the comment to something like "no new IOMMU mappings can be
inserted"?

Jan

[xen-unstable test] 159602: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-ws16-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-debianhvm-amd64  8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-shadow 8 xen-boot fail REGR. vs. 159475
 test-xtf-amd64-amd64-1  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-amd64-i386-qemut-rhel6hvm-intel  8 xen-boot fail REGR. vs. 159475
 test-xtf-amd64-amd64-3  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-debianhvm-amd64  8 xen-boot fail REGR. vs. 159475
 test-xtf-amd64-amd64-4  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-2  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-xtf-amd64-amd64-5  19 xtf/test-pv32pae-selftest fail REGR. vs. 159475
 test-amd64-i386-libvirt-xsm   8 xen-boot fail REGR. vs. 159475
 test-amd64-coresched-i386-xl  8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-raw8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-xsm8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-migrupgrade  13 xen-boot/dst_hostfail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-ovmf-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-xl8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-qemut-rhel6hvm-amd  8 xen-boot   fail REGR. vs. 159475
 test-amd64-i386-libvirt-pair 12 xen-boot/src_hostfail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-libvirt-pair 13 xen-boot/dst_hostfail REGR. vs. 159475
 test-amd64-i386-qemuu-rhel6hvm-amd  8 xen-boot   fail REGR. vs. 159475
 test-amd64-i386-xl-qemut-win7-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-freebsd10-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-freebsd10-i386  8 xen-boot   fail REGR. vs. 159475
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 159475
 test-amd64-i386-libvirt   8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-livepatch 8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-qemuu-rhel6hvm-intel  8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-xl-qemuu-ws16-amd64  8 xen-boot  fail REGR. vs. 159475
 test-amd64-i386-pair 12 xen-boot/src_hostfail REGR. vs. 159475
 test-amd64-i386-pair 13 xen-boot/dst_hostfail REGR. vs. 159475
 test-amd64-i386-xl-pvshim 8 xen-boot fail REGR. vs. 159475
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 
159475
 test-amd64-i386-xl-qemuu-win7-amd64  8 xen-boot  fail REGR. vs. 159475

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeat  fail pass in 159559

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

Re: [PATCH v2 0/8] x86/PV: avoid speculation abuse through guest accessors

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH v2 0/8] x86/PV: avoid speculation abuse through 
guest accessors"):
> On 24.02.2021 14:08, Ian Jackson wrote:
> > For 7, I remember what I think was an IRC conversation where someone
> > (you, I think) said you had examined the generated asm and it was
> > unchanged.
> 
> It was in email, and I've inspected only some example of the
> generated asm, not all instances. I would hope that was
> sufficient, but since I'm not entirely certain ...
> 
> > If I have remembered that correctly, then for 7 as well:
> > Release-Acked-by: Ian Jackson 
> 
> ... I'll better wait for explicit confirmation of this.

I think that's convincing enough.  Thank you.

Release-Acked-by: Ian Jackson 

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations

[Andrew Cooper]

On 24/02/2021 11:07, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH 2/2] hvmloader: do not include system headers 
> for type declarations"):
>> Like the hypervisor, we should prefer using __SIZE_TYPE__
>> when available.
> I disagree.

size_t is obnoxious in the C spec.  It might not be the largest native
word size on the processor, and in some 64bit environments, it really is
32 bits.

It cannot be correctly derived from a standard type, and must come from
the compiler, because it critical that it matches what the compiler
generates for the sizeof operator.

Posix being fairly sane prohibits environments where the maximum object
size is smaller than the address size, which is why aliasing it to
unsigned long works in the common case.

~Andrew

Re: [PATCH v2 0/8] x86/PV: avoid speculation abuse through guest accessors

[Jan Beulich]

On 24.02.2021 14:08, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH v2 0/8] x86/PV: avoid speculation abuse 
> through guest accessors"):
>> On 19.02.2021 16:50, Ian Jackson wrote:
>>> Jan Beulich writes ("[PATCH v2 0/8] x86/PV: avoid speculation abuse through 
>>> guest accessors"):
 4: rename {get,put}_user() to {get,put}_guest()
 5: gdbsx: convert "user" to "guest" accesses
 6: rename copy_{from,to}_user() to copy_{from,to}_guest_pv()
 7: move stac()/clac() from {get,put}_unsafe_asm() ...
 8: PV: use get_unsafe() instead of copy_from_unsafe()
>>>
>>> These have not got a maintainer review yet.  To grant a release-ack
>>> I'd like an explanation of the downsides and upsides of taking this
>>> series in 4.15 ?
>>>
>>> You say "consistency" but in practical terms, what will happen if the
>>> code is not "conxistent" in this sense ?
>>>
>>> I'd also like to hear from aother hypervisor maintainer.
>>
>> Meanwhile they have been reviewed by Roger. Are you willing to
>> give them, perhaps with the exception of 7, a release ack as
>> well?
> 
> Sorry, yes.
> 
> I found these explanations convincing  Thank you.
> 
> For all except 7,
> Release-Acked-by: Ian Jackson 

Thanks.

> For 7, I remember what I think was an IRC conversation where someone
> (you, I think) said you had examined the generated asm and it was
> unchanged.

It was in email, and I've inspected only some example of the
generated asm, not all instances. I would hope that was
sufficient, but since I'm not entirely certain ...

> If I have remembered that correctly, then for 7 as well:
> Release-Acked-by: Ian Jackson 

... I'll better wait for explicit confirmation of this.

Jan

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations

[Jan Beulich]

On 24.02.2021 13:01, Roger Pau Monné wrote:
> On Wed, Feb 24, 2021 at 11:51:39AM +0100, Jan Beulich wrote:
>> On 24.02.2021 11:26, Roger Pau Monne wrote:
>>> --- /dev/null
>>> +++ b/tools/firmware/hvmloader/types.h
>>> @@ -0,0 +1,47 @@
>>> +#ifndef _HVMLOADER_TYPES_H_
>>> +#define _HVMLOADER_TYPES_H_
>>> +
>>> +typedef unsigned char uint8_t;
>>> +typedef signed char int8_t;
>>> +
>>> +typedef unsigned short uint16_t;
>>> +typedef signed short int16_t;
>>> +
>>> +typedef unsigned int uint32_t;
>>> +typedef signed int int32_t;
>>> +
>>> +typedef unsigned long long uint64_t;
>>> +typedef signed long long int64_t;
>>
>> I wonder if we weren't better of not making assumptions on
>> short / int / long long, and instead use
>> __attribute__((__mode__(...))) or, if available, the compiler
>> provided __{,U}INT*_TYPE__.
> 
> Oh, didn't know about all this fancy stuff.
> 
> Clang doens't seem to support the same mode attributes, for example
> QImode is unknown, and that's just on one version of clang that I
> happened to test on.

Oh, these modes have been available even in gcc 3.x. I thought
Clang was claiming to be 4.4(?) compatible.

> Using __{,U}INT*_TYPE__ does seem to be supported on the clang version
> I've tested with, so that might be an option if it's supported
> everywhere we care about. If we still need to keep the current typedef
> chunk for fallback purposes then I see no real usefulness of using
> __{,U}INT*_TYPE__.

Fair point. And they're available from 4.5 onwards only. So
just __SIZE_TYPE__ has been available for long enough. As said
in reply to Ian I think we at least want to use that one (and
I guess in the hypervisor we may want to drop the fallback).

Jan

Re: [PATCH v2 0/8] x86/PV: avoid speculation abuse through guest accessors

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH v2 0/8] x86/PV: avoid speculation abuse through 
guest accessors"):
> On 19.02.2021 16:50, Ian Jackson wrote:
> > Jan Beulich writes ("[PATCH v2 0/8] x86/PV: avoid speculation abuse through 
> > guest accessors"):
> >> 4: rename {get,put}_user() to {get,put}_guest()
> >> 5: gdbsx: convert "user" to "guest" accesses
> >> 6: rename copy_{from,to}_user() to copy_{from,to}_guest_pv()
> >> 7: move stac()/clac() from {get,put}_unsafe_asm() ...
> >> 8: PV: use get_unsafe() instead of copy_from_unsafe()
> > 
> > These have not got a maintainer review yet.  To grant a release-ack
> > I'd like an explanation of the downsides and upsides of taking this
> > series in 4.15 ?
> > 
> > You say "consistency" but in practical terms, what will happen if the
> > code is not "conxistent" in this sense ?
> > 
> > I'd also like to hear from aother hypervisor maintainer.
> 
> Meanwhile they have been reviewed by Roger. Are you willing to
> give them, perhaps with the exception of 7, a release ack as
> well?

Sorry, yes.

I found these explanations convincing  Thank you.

For all except 7,
Release-Acked-by: Ian Jackson 

For 7, I remember what I think was an IRC conversation where someone
(you, I think) said you had examined the generated asm and it was
unchanged.

If I have remembered that correctly, then for 7 as well:
Release-Acked-by: Ian Jackson 

If I have misremembered please do say.

Ian.

[PATCH AUTOSEL 5.10 51/56] xen-blkback: fix error handling in xen_blkbk_map()

[Sasha Levin]

From: Jan Beulich 

[ Upstream commit 871997bc9e423f05c7da7c9178e62dde5df2a7f8 ]

The function uses a goto-based loop, which may lead to an earlier error
getting discarded by a later iteration. Exit this ad-hoc loop when an
error was encountered.

The out-of-memory error path additionally fails to fill a structure
field looked at by xen_blkbk_unmap_prepare() before inspecting the
handle which does get properly set (to BLKBACK_INVALID_HANDLE).

Since the earlier exiting from the ad-hoc loop requires the same field
filling (invalidation) as that on the out-of-memory path, fold both
paths. While doing so, drop the pr_alert(), as extra log messages aren't
going to help the situation (the kernel will log oom conditions already
anyway).

This is XSA-365.

Signed-off-by: Jan Beulich 
Reviewed-by: Juergen Gross 
Reviewed-by: Julien Grall 
Signed-off-by: Juergen Gross 
Signed-off-by: Sasha Levin 
---
 drivers/block/xen-blkback/blkback.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/block/xen-blkback/blkback.c 
b/drivers/block/xen-blkback/blkback.c
index 9ebf53903d7bf..9301de1386436 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -794,8 +794,13 @@ static int xen_blkbk_map(struct xen_blkif_ring *ring,
pages[i]->persistent_gnt = persistent_gnt;
} else {
if (gnttab_page_cache_get(>free_pages,
- [i]->page))
-   goto out_of_memory;
+ [i]->page)) {
+   gnttab_page_cache_put(>free_pages,
+ pages_to_gnt,
+ segs_to_map);
+   ret = -ENOMEM;
+   goto out;
+   }
addr = vaddr(pages[i]->page);
pages_to_gnt[segs_to_map] = pages[i]->page;
pages[i]->persistent_gnt = NULL;
@@ -882,17 +887,18 @@ static int xen_blkbk_map(struct xen_blkif_ring *ring,
}
segs_to_map = 0;
last_map = map_until;
-   if (map_until != num)
+   if (!ret && map_until != num)
goto again;
 
-   return ret;
-
-out_of_memory:
-   pr_alert("%s: out of memory\n", __func__);
-   gnttab_page_cache_put(>free_pages, pages_to_gnt, segs_to_map);
-   for (i = last_map; i < num; i++)
+out:
+   for (i = last_map; i < num; i++) {
+   /* Don't zap current batch's valid persistent grants. */
+   if(i >= last_map + segs_to_map)
+   pages[i]->persistent_gnt = NULL;
pages[i]->handle = BLKBACK_INVALID_HANDLE;
-   return -ENOMEM;
+   }
+
+   return ret;
 }
 
 static int xen_blkbk_map_seg(struct pending_req *pending_req)
-- 
2.27.0

[PATCH AUTOSEL 5.11 62/67] xen-blkback: fix error handling in xen_blkbk_map()

[Sasha Levin]

From: Jan Beulich 

[ Upstream commit 871997bc9e423f05c7da7c9178e62dde5df2a7f8 ]

The function uses a goto-based loop, which may lead to an earlier error
getting discarded by a later iteration. Exit this ad-hoc loop when an
error was encountered.

The out-of-memory error path additionally fails to fill a structure
field looked at by xen_blkbk_unmap_prepare() before inspecting the
handle which does get properly set (to BLKBACK_INVALID_HANDLE).

Since the earlier exiting from the ad-hoc loop requires the same field
filling (invalidation) as that on the out-of-memory path, fold both
paths. While doing so, drop the pr_alert(), as extra log messages aren't
going to help the situation (the kernel will log oom conditions already
anyway).

This is XSA-365.

Signed-off-by: Jan Beulich 
Reviewed-by: Juergen Gross 
Reviewed-by: Julien Grall 
Signed-off-by: Juergen Gross 
Signed-off-by: Sasha Levin 
---
 drivers/block/xen-blkback/blkback.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/block/xen-blkback/blkback.c 
b/drivers/block/xen-blkback/blkback.c
index 9ebf53903d7bf..9301de1386436 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -794,8 +794,13 @@ static int xen_blkbk_map(struct xen_blkif_ring *ring,
pages[i]->persistent_gnt = persistent_gnt;
} else {
if (gnttab_page_cache_get(>free_pages,
- [i]->page))
-   goto out_of_memory;
+ [i]->page)) {
+   gnttab_page_cache_put(>free_pages,
+ pages_to_gnt,
+ segs_to_map);
+   ret = -ENOMEM;
+   goto out;
+   }
addr = vaddr(pages[i]->page);
pages_to_gnt[segs_to_map] = pages[i]->page;
pages[i]->persistent_gnt = NULL;
@@ -882,17 +887,18 @@ static int xen_blkbk_map(struct xen_blkif_ring *ring,
}
segs_to_map = 0;
last_map = map_until;
-   if (map_until != num)
+   if (!ret && map_until != num)
goto again;
 
-   return ret;
-
-out_of_memory:
-   pr_alert("%s: out of memory\n", __func__);
-   gnttab_page_cache_put(>free_pages, pages_to_gnt, segs_to_map);
-   for (i = last_map; i < num; i++)
+out:
+   for (i = last_map; i < num; i++) {
+   /* Don't zap current batch's valid persistent grants. */
+   if(i >= last_map + segs_to_map)
+   pages[i]->persistent_gnt = NULL;
pages[i]->handle = BLKBACK_INVALID_HANDLE;
-   return -ENOMEM;
+   }
+
+   return ret;
 }
 
 static int xen_blkbk_map_seg(struct pending_req *pending_req)
-- 
2.27.0

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations

[Roger Pau Monné]

On Wed, Feb 24, 2021 at 12:11:45PM +, Andrew Cooper wrote:
> On 24/02/2021 10:26, Roger Pau Monne wrote:
> > Instead create a private types.h header that contains the set of types
> > that are required by hvmloader. Replace include occurrences of std*
> > headers with type.h. Note that including types.h directly is not
> > required in util.c because it already includes util.h which in turn
> > includes the newly created types.h.
> >
> > Signed-off-by: Roger Pau Monné 
> 
> At what point do we just declare Alpine broken?  These are all
> freestanding headers, an explicitly available for use, in the way we use
> them.

The headers are available in Alpine, but they seem to be specifically
tied to the native bitness of the OS, which is causing us the issues.

So they are freestanding AFAICT, it's just that the bitness they use
is not portable.

Thanks, Roger.

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations

[Andrew Cooper]

On 24/02/2021 10:26, Roger Pau Monne wrote:
> Instead create a private types.h header that contains the set of types
> that are required by hvmloader. Replace include occurrences of std*
> headers with type.h. Note that including types.h directly is not
> required in util.c because it already includes util.h which in turn
> includes the newly created types.h.
>
> Signed-off-by: Roger Pau Monné 

At what point do we just declare Alpine broken?  These are all
freestanding headers, an explicitly available for use, in the way we use
them.

There are substantial portability costs for making changes like this,
which takes us from standards compliant C to GCC-isms-only.

~Andrew

[libvirt test] 159613: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-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-arm64-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-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-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a

version targeted for testing:
 libvirt  fa58f571ee0b2ab72f8a5a19d35298d6de6469b3
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  229 days
Failing since151818  2020-07-11 04:18:52 Z  228 days  221 attempts
Testing same since   159613  2021-02-24 04:18:55 Z0 days1 attempts


People who touched revisions under test:
  Adolfo Jayme Barrientos 
  Aleksandr Alekseev 
  Andika Triwidada 
  Andrea Bolognani 
  Balázs Meskó 
  Barrett Schonefeld 
  Bastien Orivel 
  BiaoXiang Ye 
  Bihong Yu 
  Binfeng Wu 
  Boris Fiuczynski 
  Brian Turek 
  Bruno Haible 
  Christian Ehrhardt 
  Christian Schoenebeck 
  Cole Robinson 
  Collin Walling 
  Cornelia Huck 
  Cédric Bosdonnat 
  Côme Borsoi 
  Daniel Henrique Barboza 
  Daniel Letai 
  Daniel P. Berrange 
  Daniel P. Berrangé 
  Dmytro Linkin 
  Eiichi Tsukata 
  Erik Skultety 
  Fabian Affolter 
  Fabian Freyer 
  Fangge Jin 
  Farhan Ali 
  Fedora Weblate Translation 
  gongwei 
  Guoyi Tu
  Göran Uddeborg 
  Halil Pasic 
  Han Han 
  Hao Wang 
  Helmut Grohne 
  Ian Wienand 
  Jakob Meng 
  Jamie Strandboge 
  Jamie Strandboge 
  Jan Kuparinen 
  Jean-Baptiste Holcroft 
  Jianan Gao 
  Jim Fehlig 
  Jin Yan 
  Jiri Denemark 
  John Ferlan 
  Jonathan Watt 
  Jonathon Jongsma 
  Julio Faracco 
  Ján Tomko 
  Kashyap Chamarthy 
  Kevin Locke 
  Kristina Hanicova 
  Laine Stump 
  Laszlo Ersek 
  Liao Pingfang 
  Lin Ma 
  Lin Ma 
  Lin Ma 
  Marc Hartmayer 
  Marc-André Lureau 
  Marek Marczykowski-Górecki 
  Markus Schade 
  Martin Kletzander 
  Masayoshi Mizuma 
  Matt Coleman 
  Matt Coleman 
  Mauro Matteo Cascella 
  Meina Li 
  Michal Privoznik 
  Michał Smyk 
  Milo Casagrande 
  Moshe Levi 
  Muha Aliss 
  Neal Gompa 
  Nick Shyrokovskiy 
  Nickys Music Group 
  Nico Pache 
  Nikolay Shirokovskiy 
  Olaf Hering 
  Olesya Gerasimenko 
  Orion Poplawski 
  Pany 
  Patrick Magauran 
  Paulo de Rezende Pinatti 
  Pavel Hrdina 
  Peter Krempa 
  Pino Toscano 
  Pino Toscano 
  Piotr Drąg 
  Prathamesh Chavan 
  Ricky Tigg 
  Roman Bogorodskiy 
  Roman Bolshakov 
  Ryan Gahagan 
  Ryan Schmidt 
  Sam Hartman 
  Scott Shambarger 
  Sebastian Mitterle 
  Shalini Chellathurai Saroja 
  Shaojun Yang 
  Shi Lei 
  Simon Gaiser 
  Stefan Bader 
  Stefan Berger 
  Stefan Berger 
  Szymon Scholz 
  Thomas Huth 
  Tim Wiederhake 
  Tomáš Golembiovský 
  Tomáš Janoušek 
  Tuguoyi 
  Ville Skyttä 
  Wang Xin 
  Weblate 
  Yalei Li <274268...@qq.com>
  Yalei Li 
  Yang Hang 
  Yanqiu Zhang 
  Yi Li 
  Yi Wang 
  Yuri Chornoivan 
  Zheng Chuan 
  zhenwei pi 
  Zhenyu Zheng 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  fail
 build-arm64-libvirt  fail

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations

[Roger Pau Monné]

On Wed, Feb 24, 2021 at 11:51:39AM +0100, Jan Beulich wrote:
> On 24.02.2021 11:26, Roger Pau Monne wrote:
> > --- /dev/null
> > +++ b/tools/firmware/hvmloader/types.h
> > @@ -0,0 +1,47 @@
> > +#ifndef _HVMLOADER_TYPES_H_
> > +#define _HVMLOADER_TYPES_H_
> > +
> > +typedef unsigned char uint8_t;
> > +typedef signed char int8_t;
> > +
> > +typedef unsigned short uint16_t;
> > +typedef signed short int16_t;
> > +
> > +typedef unsigned int uint32_t;
> > +typedef signed int int32_t;
> > +
> > +typedef unsigned long long uint64_t;
> > +typedef signed long long int64_t;
> 
> I wonder if we weren't better of not making assumptions on
> short / int / long long, and instead use
> __attribute__((__mode__(...))) or, if available, the compiler
> provided __{,U}INT*_TYPE__.

Oh, didn't know about all this fancy stuff.

Clang doens't seem to support the same mode attributes, for example
QImode is unknown, and that's just on one version of clang that I
happened to test on.

Using __{,U}INT*_TYPE__ does seem to be supported on the clang version
I've tested with, so that might be an option if it's supported
everywhere we care about. If we still need to keep the current typedef
chunk for fallback purposes then I see no real usefulness of using
__{,U}INT*_TYPE__.

> > +#define INT8_MIN(-0x7f-1)
> > +#define INT16_MIN   (-0x7fff-1)
> > +#define INT32_MIN   (-0x7fff-1)
> > +#define INT64_MIN   (-0x7fffll-1)
> > +
> > +#define INT8_MAX0x7f
> > +#define INT16_MAX   0x7fff
> > +#define INT32_MAX   0x7fff
> > +#define INT64_MAX   0x7fffll
> > +
> > +#define UINT8_MAX   0xff
> > +#define UINT16_MAX  0x
> > +#define UINT32_MAX  0xu
> > +#define UINT64_MAX  0xull
> 
> At least if going the above outlined route, I think we'd then
> also be better off not #define-ing any of these which we don't
> really use. Afaics it's really only UINTPTR_MAX which needs
> providing.

I've assumed that for consistency we would like to provide those
already. I can switch them to using __{U}INT*_{MAX,MIN}__ if we agree
that it's supported on all compilers we care about, but I would rather
not drop them. I think those might be useful in the future, and having
them already does no harm.

> > +typedef uint32_t size_t;
> 
> Like the hypervisor, we should prefer using __SIZE_TYPE__
> when available.
> 
> > +typedef uint32_t uintptr_t;
> 
> Again - use __UINTPTR_TYPE__ or, like Xen,
> __attribute__((__mode__(__pointer__))).

Let me run a gitlab test suite using the __{,U}INT*_TYPE__ stuff and
if that's fine everywhere we test then I think we can go for that if
you prefer over the current proposal?

I still think that coding them like I've done above should be fine as
we don't expect hvmloader to ever be built in a mode different than
i386?

Thanks, Roger.

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations

[Jan Beulich]

On 24.02.2021 12:07, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH 2/2] hvmloader: do not include system headers 
> for type declarations"):
>> On 24.02.2021 11:26, Roger Pau Monne wrote:
>>> --- /dev/null
>>> +++ b/tools/firmware/hvmloader/types.h
>>> @@ -0,0 +1,47 @@
>>> +#ifndef _HVMLOADER_TYPES_H_
>>> +#define _HVMLOADER_TYPES_H_
>>> +
>>> +typedef unsigned char uint8_t;
>>> +typedef signed char int8_t;
>>> +
>>> +typedef unsigned short uint16_t;
>>> +typedef signed short int16_t;
>>> +
>>> +typedef unsigned int uint32_t;
>>> +typedef signed int int32_t;
>>> +
>>> +typedef unsigned long long uint64_t;
>>> +typedef signed long long int64_t;
>>
>> I wonder if we weren't better of not making assumptions on
>> short / int / long long, and instead use
>> __attribute__((__mode__(...))) or, if available, the compiler
>> provided __{,U}INT*_TYPE__.
> 
> This code is only ever going to be for 32-bit x86, so I think the way
> Roger did it is fine.

It is technically correct at this point in time, from all we can
tell. I can't see any reason though why a compiler might not
support wider int or, in particular, long long. hvmloader, unlike
most of the rest of the tools, is a freestanding binary and hence
not tied to any particular ABI the compiler used may have been
built for.

> Doing it the other way, to cope with this file being used with
> compiler settings where the above set of types is wrong, would also
> imply more complex definitions of INT32_MIN et al.

Well, that's only as far as the use of number suffixes goes. The
values used won't change, as these constants describe fixed width
types.

>>> +typedef uint32_t size_t;
> 
> I would be inclined to provide ssize_t too but maybe hvmloader will
> never need it.
> 
>> Like the hypervisor, we should prefer using __SIZE_TYPE__
>> when available.
> 
> I disagree.

May I ask why? There is a reason providing of these types did get
added to (at least) gcc.

One argument against this would be above mentioned independence
on any ABI the compiler would be built for, but I'd buy that only
if above we indeed used __attribute__((__mode__())), as that's
the only way to achieve such independence.

IOW imo if we stick to what is there now for {,u}int_t, we
should use __SIZE_TYPE__ here. If we used the mode attribute
approach there, using uint32_t here would indeed be better.

>>> +typedef uint32_t uintptr_t;
>>
>> Again - use __UINTPTR_TYPE__ or, like Xen,
>> __attribute__((__mode__(__pointer__))).
> 
> I disagree.

The same question / considerations apply here then.

>>> +#define bool _Bool
>>> +#define true 1
>>> +#define false 0
>>> +#define __bool_true_false_are_defined   1
>>> +
>>> +typedef __builtin_va_list va_list;
>>> +#define va_copy(dest, src)__builtin_va_copy((dest), (src))
>>> +#define va_start(ap, last)__builtin_va_start((ap), (last))
>>
>> Nit: Perhaps better omit the unnecessary inner parentheses?
> 
> We should definitely keep the inner parentheses.  I don't want to
> start carefully reasoning about precisely which inner parentheses are
> necesary for macro argument parsing correctness.

Can you give me an example of when the inner parentheses would be
needed? I don't think they're needed no matter whether (taking the
example here) __builtin_va_...() were functions or macros. They
would of course be needed if the identifiers were part of
expressions beyond the mere function invocation. We've been trying
to eliminate such in the hypervisor part of the tree, and since
hvmloader is more closely related to the hypervisor than the tools
(see also its maintainership), I think we would want to do so
here, too. But of course if there are cases where such
parentheses are really needed, we'd want (need) to change our
approach in hypervisor code as well.

The primary reason why I've been advocating to avoid them is that,
as long as they're not needed for anything, they harm readability
and increase the risk of mistakes like the one that had led to
XSA-316.

Jan

Re: [PATCH] virtio-gpu: Respect graphics update interval for EDID

[Gerd Hoffmann]

On Tue, Feb 23, 2021 at 01:50:51PM +0900, Akihiko Odaki wrote:
> 2021年2月22日(月) 19:57 Gerd Hoffmann :
> >
> > On Sun, Feb 21, 2021 at 10:34:14PM +0900, Akihiko Odaki wrote:
> > > This change introduces an additional member, refresh_rate to
> > > qemu_edid_info in include/hw/display/edid.h.
> > >
> > > This change also isolates the graphics update interval from the
> > > display update interval. The guest will update the frame buffer
> > > in the graphics update interval, but displays can be updated in a
> > > dynamic interval, for example to save update costs aggresively
> > > (vnc) or to respond to user-generated events (sdl).
> > > It stabilizes the graphics update interval and prevents the guest
> > > from being confused.
> >
> > Hmm.  What problem you are trying to solve here?
> >
> > The update throttle being visible by the guest was done intentionally,
> > so the guest can throttle the display updates too in case nobody is
> > watching those display updated anyway.
> 
> Indeed, we are throttling the update for vnc to avoid some worthless
> work. But typically a guest cannot respond to update interval changes
> so often because real display devices the guest is designed for does
> not change the update interval in that way.

What is the problem you are seeing?

Some guest software raising timeout errors when they see only
one vblank irq every 3 seconds?  If so: which software is this?
Any chance we can fix this on the guest side?

> That is why we have to
> tell the guest a stable update interval even if it results in wasted
> frames.

Because of the wasted frames I'd like this to be an option you can
enable when needed.  For the majority of use cases this seems to be
no problem ...

Also: the EDID changes should go to a separate patch.

take care,
  Gerd

Re: [PATCH v2 0/8] x86/PV: avoid speculation abuse through guest accessors

[Jan Beulich]

On 19.02.2021 16:50, Ian Jackson wrote:
> Jan Beulich writes ("[PATCH v2 0/8] x86/PV: avoid speculation abuse through 
> guest accessors"):
>> Re-sending primarily for the purpose of getting a release ack, an
>> explicit release nak, or an indication of there not being a need,
>> all for at least the first three patches here (which are otherwise
>> ready to go in). I've dropped the shadow part of the series from
>> this re-submission, because it has all got reviewed by Tim already
>> and is intended for 4.16 only anyway. I'm re-including the follow
>> up patches getting the code base in consistent shape again, as I
>> continue to think this consistency goal is at least worth a
>> consideration towards a freeze exception.
>>
>> 1: split __{get,put}_user() into "guest" and "unsafe" variants
>> 2: split __copy_{from,to}_user() into "guest" and "unsafe" variants
>> 3: PV: harden guest memory accesses against speculative abuse
> 
> These three:
> 
> Release-Acked-by: Ian Jackson 
> 
> On the grounds that this is probably severe enough to be a blocking
> issue for 4.15.
> 
>> 4: rename {get,put}_user() to {get,put}_guest()
>> 5: gdbsx: convert "user" to "guest" accesses
>> 6: rename copy_{from,to}_user() to copy_{from,to}_guest_pv()
>> 7: move stac()/clac() from {get,put}_unsafe_asm() ...
>> 8: PV: use get_unsafe() instead of copy_from_unsafe()
> 
> These have not got a maintainer review yet.  To grant a release-ack
> I'd like an explanation of the downsides and upsides of taking this
> series in 4.15 ?
> 
> You say "consistency" but in practical terms, what will happen if the
> code is not "conxistent" in this sense ?
> 
> I'd also like to hear from aother hypervisor maintainer.

Meanwhile they have been reviewed by Roger. Are you willing to
give them, perhaps with the exception of 7, a release ack as
well?

Jan

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH 2/2] hvmloader: do not include system headers 
for type declarations"):
> On 24.02.2021 11:26, Roger Pau Monne wrote:
> > --- /dev/null
> > +++ b/tools/firmware/hvmloader/types.h
> > @@ -0,0 +1,47 @@
> > +#ifndef _HVMLOADER_TYPES_H_
> > +#define _HVMLOADER_TYPES_H_
> > +
> > +typedef unsigned char uint8_t;
> > +typedef signed char int8_t;
> > +
> > +typedef unsigned short uint16_t;
> > +typedef signed short int16_t;
> > +
> > +typedef unsigned int uint32_t;
> > +typedef signed int int32_t;
> > +
> > +typedef unsigned long long uint64_t;
> > +typedef signed long long int64_t;
> 
> I wonder if we weren't better of not making assumptions on
> short / int / long long, and instead use
> __attribute__((__mode__(...))) or, if available, the compiler
> provided __{,U}INT*_TYPE__.

This code is only ever going to be for 32-bit x86, so I think the way
Roger did it is fine.

Doing it the other way, to cope with this file being used with
compiler settings where the above set of types is wrong, would also
imply more complex definitions of INT32_MIN et al.

> > +#define INT8_MIN(-0x7f-1)
> > +#define INT16_MIN   (-0x7fff-1)
> > +#define INT32_MIN   (-0x7fff-1)
> > +#define INT64_MIN   (-0x7fffll-1)
> > +
> > +#define INT8_MAX0x7f
> > +#define INT16_MAX   0x7fff
> > +#define INT32_MAX   0x7fff
> > +#define INT64_MAX   0x7fffll
> > +
> > +#define UINT8_MAX   0xff
> > +#define UINT16_MAX  0x
> > +#define UINT32_MAX  0xu
> > +#define UINT64_MAX  0xull
> 
> At least if going the above outlined route, I think we'd then
> also be better off not #define-ing any of these which we don't
> really use. Afaics it's really only UINTPTR_MAX which needs
> providing.

I disagree.  Providing the full set now gets them all properly
reviewe and reduces the burden on future work.

> > +typedef uint32_t size_t;

I would be inclined to provide ssize_t too but maybe hvmloader will
never need it.

> Like the hypervisor, we should prefer using __SIZE_TYPE__
> when available.

I disagree.

> > +typedef uint32_t uintptr_t;
> 
> Again - use __UINTPTR_TYPE__ or, like Xen,
> __attribute__((__mode__(__pointer__))).

I disagree.

> > +#define bool _Bool
> > +#define true 1
> > +#define false 0
> > +#define __bool_true_false_are_defined   1
> > +
> > +typedef __builtin_va_list va_list;
> > +#define va_copy(dest, src)__builtin_va_copy((dest), (src))
> > +#define va_start(ap, last)__builtin_va_start((ap), (last))
> 
> Nit: Perhaps better omit the unnecessary inner parentheses?

We should definitely keep the inner parentheses.  I don't want to
start carefully reasoning about precisely which inner parentheses are
necesary for macro argument parsing correctness.

Ian.

Re: [for-4.15][RESEND PATCH v4 0/2] xen/iommu: Collection of bug fixes for IOMMU teardown

[Ian Jackson]

Julien Grall writes ("[for-4.15][RESEND PATCH v4 0/2] xen/iommu: Collection of 
bug fixes for IOMMU teardown"):
> This series is a collection of bug fixes for the IOMMU teardown code.
> All of them are candidate for 4.15 as they can either leak memory or
> lead to host crash/host corruption.
> 
> This is sent directly on xen-devel because all the issues were either
> introduced in 4.15 or happen in the domain creation code.

Thanks.

Release-Acked-by: Ian Jackson 

I'd appreciate it if reviewers would double-check the comments and
commit messages which Julien has helpfully provided, to check that the
assertions made are true.  It seems this is quite complex...

Ian.

Re: [PATCH 0/2] hvmloader: drop usage of system headers

[Ian Jackson]

Roger Pau Monne writes ("[PATCH 0/2] hvmloader: drop usage of system headers"):
> Following two patches aim to make hvmloader standalone, so that it don't
> try to use system headers. It shouldn't result in any functional
> change.

Both patches:

Reviewed-by: Ian Jackson 

Given its status as a build fix,

Release-Acked-by: Ian Jackson 

Thanks,
Ian.

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations

[Jan Beulich]

On 24.02.2021 11:26, Roger Pau Monne wrote:
> --- /dev/null
> +++ b/tools/firmware/hvmloader/types.h
> @@ -0,0 +1,47 @@
> +#ifndef _HVMLOADER_TYPES_H_
> +#define _HVMLOADER_TYPES_H_
> +
> +typedef unsigned char uint8_t;
> +typedef signed char int8_t;
> +
> +typedef unsigned short uint16_t;
> +typedef signed short int16_t;
> +
> +typedef unsigned int uint32_t;
> +typedef signed int int32_t;
> +
> +typedef unsigned long long uint64_t;
> +typedef signed long long int64_t;

I wonder if we weren't better of not making assumptions on
short / int / long long, and instead use
__attribute__((__mode__(...))) or, if available, the compiler
provided __{,U}INT*_TYPE__.

> +#define INT8_MIN(-0x7f-1)
> +#define INT16_MIN   (-0x7fff-1)
> +#define INT32_MIN   (-0x7fff-1)
> +#define INT64_MIN   (-0x7fffll-1)
> +
> +#define INT8_MAX0x7f
> +#define INT16_MAX   0x7fff
> +#define INT32_MAX   0x7fff
> +#define INT64_MAX   0x7fffll
> +
> +#define UINT8_MAX   0xff
> +#define UINT16_MAX  0x
> +#define UINT32_MAX  0xu
> +#define UINT64_MAX  0xull

At least if going the above outlined route, I think we'd then
also be better off not #define-ing any of these which we don't
really use. Afaics it's really only UINTPTR_MAX which needs
providing.

> +typedef uint32_t size_t;

Like the hypervisor, we should prefer using __SIZE_TYPE__
when available.

> +typedef uint32_t uintptr_t;

Again - use __UINTPTR_TYPE__ or, like Xen,
__attribute__((__mode__(__pointer__))).

> +#define bool _Bool
> +#define true 1
> +#define false 0
> +#define __bool_true_false_are_defined   1
> +
> +typedef __builtin_va_list va_list;
> +#define va_copy(dest, src)__builtin_va_copy((dest), (src))
> +#define va_start(ap, last)__builtin_va_start((ap), (last))

Nit: Perhaps better omit the unnecessary inner parentheses?

Jan

Re: [PATCH 1/2] hvmloader: use Xen private header for elf structs

[Jan Beulich]

On 24.02.2021 11:26, Roger Pau Monne wrote:
> Do not use the system provided elf.h, and instead use elfstructs.h
> from libelf.
> 
> Signed-off-by: Roger Pau Monné 

Acked-by: Jan Beulich 

[PATCH 2/2] hvmloader: do not include system headers for type declarations

[Roger Pau Monne]

Instead create a private types.h header that contains the set of types
that are required by hvmloader. Replace include occurrences of std*
headers with type.h. Note that including types.h directly is not
required in util.c because it already includes util.h which in turn
includes the newly created types.h.

Signed-off-by: Roger Pau Monné 
---
 tools/firmware/hvmloader/32bitbios_support.c |  2 +-
 tools/firmware/hvmloader/config.h|  3 +-
 tools/firmware/hvmloader/hypercall.h |  2 +-
 tools/firmware/hvmloader/mp_tables.c |  2 +-
 tools/firmware/hvmloader/option_rom.h|  2 +-
 tools/firmware/hvmloader/pir_types.h |  2 +-
 tools/firmware/hvmloader/smbios.c|  2 +-
 tools/firmware/hvmloader/smbios_types.h  |  2 +-
 tools/firmware/hvmloader/types.h | 47 
 tools/firmware/hvmloader/util.c  |  1 -
 tools/firmware/hvmloader/util.h  |  5 +--
 11 files changed, 56 insertions(+), 14 deletions(-)
 create mode 100644 tools/firmware/hvmloader/types.h

diff --git a/tools/firmware/hvmloader/32bitbios_support.c 
b/tools/firmware/hvmloader/32bitbios_support.c
index e726946a7b..32b5c4c4ad 100644
--- a/tools/firmware/hvmloader/32bitbios_support.c
+++ b/tools/firmware/hvmloader/32bitbios_support.c
@@ -20,7 +20,7 @@
  * this program; If not, see .
  */
 
-#include 
+#include "types.h"
 #include 
 #ifdef __sun__
 #include 
diff --git a/tools/firmware/hvmloader/config.h 
b/tools/firmware/hvmloader/config.h
index 844120bc87..510d5b5c79 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -1,8 +1,7 @@
 #ifndef __HVMLOADER_CONFIG_H__
 #define __HVMLOADER_CONFIG_H__
 
-#include 
-#include 
+#include "types.h"
 
 enum virtual_vga { VGA_none, VGA_std, VGA_cirrus, VGA_pt };
 extern enum virtual_vga virtual_vga;
diff --git a/tools/firmware/hvmloader/hypercall.h 
b/tools/firmware/hvmloader/hypercall.h
index 5368c30720..788f699565 100644
--- a/tools/firmware/hvmloader/hypercall.h
+++ b/tools/firmware/hvmloader/hypercall.h
@@ -31,7 +31,7 @@
 #ifndef __HVMLOADER_HYPERCALL_H__
 #define __HVMLOADER_HYPERCALL_H__
 
-#include 
+#include "types.h"
 #include 
 #include "config.h"
 
diff --git a/tools/firmware/hvmloader/mp_tables.c 
b/tools/firmware/hvmloader/mp_tables.c
index d207ecbf00..76790a9a1e 100644
--- a/tools/firmware/hvmloader/mp_tables.c
+++ b/tools/firmware/hvmloader/mp_tables.c
@@ -27,7 +27,7 @@
  * this program; If not, see .
  */
 
-#include 
+#include "types.h"
 #include "config.h"
 
 /* number of non-processor MP table entries */
diff --git a/tools/firmware/hvmloader/option_rom.h 
b/tools/firmware/hvmloader/option_rom.h
index 0fefe0812a..7988aa29ec 100644
--- a/tools/firmware/hvmloader/option_rom.h
+++ b/tools/firmware/hvmloader/option_rom.h
@@ -1,7 +1,7 @@
 #ifndef __HVMLOADER_OPTION_ROM_H__
 #define __HVMLOADER_OPTION_ROM_H__
 
-#include 
+#include "types.h"
 
 struct option_rom_header {
 uint8_t signature[2]; /* "\x55\xaa" */
diff --git a/tools/firmware/hvmloader/pir_types.h 
b/tools/firmware/hvmloader/pir_types.h
index 9f9259c2e1..9efcdcf94b 100644
--- a/tools/firmware/hvmloader/pir_types.h
+++ b/tools/firmware/hvmloader/pir_types.h
@@ -23,7 +23,7 @@
 #ifndef PIR_TYPES_H
 #define PIR_TYPES_H
 
-#include 
+#include "types.h"
 
 #define NR_PIR_SLOTS 6
 
diff --git a/tools/firmware/hvmloader/smbios.c 
b/tools/firmware/hvmloader/smbios.c
index 97a054e9e3..5821c85c30 100644
--- a/tools/firmware/hvmloader/smbios.c
+++ b/tools/firmware/hvmloader/smbios.c
@@ -19,7 +19,7 @@
  * Authors: Andrew D. Ball 
  */
 
-#include 
+#include "types.h"
 #include 
 #include 
 #include "smbios_types.h"
diff --git a/tools/firmware/hvmloader/smbios_types.h 
b/tools/firmware/hvmloader/smbios_types.h
index 7c648ece71..439c3fb247 100644
--- a/tools/firmware/hvmloader/smbios_types.h
+++ b/tools/firmware/hvmloader/smbios_types.h
@@ -25,7 +25,7 @@
 #ifndef SMBIOS_TYPES_H
 #define SMBIOS_TYPES_H
 
-#include 
+#include "types.h"
 
 /* SMBIOS entry point -- must be written to a 16-bit aligned address
between 0xf and 0xf. 
diff --git a/tools/firmware/hvmloader/types.h b/tools/firmware/hvmloader/types.h
new file mode 100644
index 00..3d765f2c60
--- /dev/null
+++ b/tools/firmware/hvmloader/types.h
@@ -0,0 +1,47 @@
+#ifndef _HVMLOADER_TYPES_H_
+#define _HVMLOADER_TYPES_H_
+
+typedef unsigned char uint8_t;
+typedef signed char int8_t;
+
+typedef unsigned short uint16_t;
+typedef signed short int16_t;
+
+typedef unsigned int uint32_t;
+typedef signed int int32_t;
+
+typedef unsigned long long uint64_t;
+typedef signed long long int64_t;
+
+#define INT8_MIN(-0x7f-1)
+#define INT16_MIN   (-0x7fff-1)
+#define INT32_MIN   (-0x7fff-1)
+#define INT64_MIN   (-0x7fffll-1)
+
+#define INT8_MAX0x7f
+#define INT16_MAX   0x7fff
+#define INT32_MAX   0x7fff
+#define INT64_MAX   

[PATCH 1/2] hvmloader: use Xen private header for elf structs

[Roger Pau Monne]

Do not use the system provided elf.h, and instead use elfstructs.h
from libelf.

Signed-off-by: Roger Pau Monné 
---
 tools/firmware/hvmloader/32bitbios_support.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/32bitbios_support.c 
b/tools/firmware/hvmloader/32bitbios_support.c
index 114135022e..e726946a7b 100644
--- a/tools/firmware/hvmloader/32bitbios_support.c
+++ b/tools/firmware/hvmloader/32bitbios_support.c
@@ -21,7 +21,7 @@
  */
 
 #include 
-#include 
+#include 
 #ifdef __sun__
 #include 
 #endif
-- 
2.30.1

[PATCH 0/2] hvmloader: drop usage of system headers

[Roger Pau Monne]

Hello,

Following two patches aim to make hvmloader standalone, so that it don't
try to use system headers. It shouldn't result in any functional
change.

Thanks, Roger.

Roger Pau Monne (2):
  hvmloader: use Xen private header for elf structs
  hvmloader: do not include system headers for type declarations

 tools/firmware/hvmloader/32bitbios_support.c |  4 +-
 tools/firmware/hvmloader/config.h|  3 +-
 tools/firmware/hvmloader/hypercall.h |  2 +-
 tools/firmware/hvmloader/mp_tables.c |  2 +-
 tools/firmware/hvmloader/option_rom.h|  2 +-
 tools/firmware/hvmloader/pir_types.h |  2 +-
 tools/firmware/hvmloader/smbios.c|  2 +-
 tools/firmware/hvmloader/smbios_types.h  |  2 +-
 tools/firmware/hvmloader/types.h | 47 
 tools/firmware/hvmloader/util.c  |  1 -
 tools/firmware/hvmloader/util.h  |  5 +--
 11 files changed, 57 insertions(+), 15 deletions(-)
 create mode 100644 tools/firmware/hvmloader/types.h

-- 
2.30.1

Re: [PATCH for-next] configure: probe for gcc -m32 integer sizes

[Roger Pau Monné]

On Tue, Feb 23, 2021 at 05:08:43PM -0800, Stefano Stabellini wrote:
> The hvmloader build on Alpine Linux x86_64 currenly fails:
> 
> 
> hvmloader.c: In function 'init_vm86_tss':
> hvmloader.c:202:39: error: left shift count >= width of type 
> [-Werror=shift-count-overflow]
>   202 |   ((uint64_t)TSS_SIZE << 32) | virt_to_phys(tss));
> 
> util.c: In function 'get_cpu_mhz':
> util.c:824:15: error: conversion from 'long long unsigned int' to 'uint64_t' 
> {aka 'long unsigned int'} changes value from
> '429496729600' to '0' [-Werror=overflow]
>   824 | cpu_khz = 100ull << 32;
> 
> 
> The root cause of the issue is that gcc -m32 picks up headers meant for
> 64-bit builds.

I'm working on getting hvmloader to build standalone without using any
system headers, which I think it's a worthwhile change to do rather
than this configure bodge. Will post the series now.

Thanks, Roger.

Re: [RFC PATCH 00/10] Preemption in hypervisor (ARM only)

[Julien Grall]

On 23/02/2021 12:06, Volodymyr Babchuk wrote:


Hi Julien,


Hi Volodymyr,


Julien Grall writes:

On 23/02/2021 02:34, Volodymyr Babchuk wrote:
... just rescheduling the vCPU. It will also give the opportunity for
the guest to handle interrupts.

If you don't return to the guest, then risk to get an RCU sched stall
on that the vCPU (some hypercalls can take really really long).


Ah yes, you are right. I'd only wish that hypervisor saved context of
hypercall on it's side...

I have example of OP-TEE before my eyes. They have special return code
"task was interrupted" and they use separate call "continue execution of
interrupted task", which takes opaque context handle as a
parameter. With this approach state of interrupted call never leaks to > rest 
of the system.


Feel free to suggest a new approach for the hypercals.




This approach itself have obvious
problems: code that executes hypercall is responsible for preemption,
preemption checks are infrequent (because they are costly by
themselves), hypercall execution state is stored in guest-controlled
area, we rely on guest's good will to continue the hypercall.


Why is it a problem to rely on guest's good will? The hypercalls
should be preempted at a boundary that is safe to continue.


Yes, and it imposes restrictions on how to write hypercall
handler.
In other words, there are much more places in hypercall handler code
where it can be preempted than where hypercall continuation can be
used. For example, you can preempt hypercall that holds a mutex, but of
course you can't create an continuation point in such place.


I disagree, you can create continuation point in such place. Although it 
will be more complex because you have to make sure you break the work in 
a restartable place.


I would also like to point out that preemption also have some drawbacks.
With RT in mind, you have to deal with priority inversion (e.g. a lower 
priority vCPU held a mutex that is required by an higher priority).


Outside of RT, you have to be careful where mutex are held. In your 
earlier answer, you suggested to held mutex for the memory allocation. 
If you do that, then it means a domain A can block allocation for domain 
B as it helds the mutex.


This can lead to quite serious problem if domain A cannot run (because 
it exhausted its credit) for a long time.





All this
imposes restrictions on which hypercalls can be preempted, when they
can be preempted and how to write hypercall handlers. Also, it
requires very accurate coding and already led to at least one
vulnerability - XSA-318. Some hypercalls can not be preempted at all,
like the one mentioned in [1].
Absence of hypervisor threads/vCPUs. Hypervisor owns only idle
vCPUs,
which are supposed to run when the system is idle. If hypervisor needs
to execute own tasks that are required to run right now, it have no
other way than to execute them on current vCPU. But scheduler does not
know that hypervisor executes hypervisor task and accounts spent time
to a domain. This can lead to domain starvation.
Also, absence of hypervisor threads leads to absence of high-level
synchronization primitives like mutexes, conditional variables,
completions, etc. This leads to two problems: we need to use spinlocks
everywhere and we have problems when porting device drivers from linux
kernel.
Proposed solution
=
It is quite obvious that to fix problems above we need to allow
preemption in hypervisor mode. I am not familiar with x86 side, but
for the ARM it was surprisingly easy to implement. Basically, vCPU
context in hypervisor mode is determined by its stack at general
purpose registers. And __context_switch() function perfectly switches
them when running in hypervisor mode. So there are no hard
restrictions, why it should be called only in leave_hypervisor() path.
The obvious question is: when we should to try to preempt running
vCPU?  And answer is: when there was an external event. This means
that we should try to preempt only when there was an interrupt request
where we are running in hypervisor mode. On ARM, in this case function
do_trap_irq() is called. Problem is that IRQ handler can be called
when vCPU is already in atomic state (holding spinlock, for
example). In this case we should try to preempt right after leaving
atomic state. This is basically all the idea behind this PoC.
Now, about the series composition.
Patches
sched: core: save IRQ state during locking
sched: rt: save IRQ state during locking
sched: credit2: save IRQ state during locking
preempt: use atomic_t to for preempt_count
arm: setup: disable preemption during startup
arm: context_switch: allow to run with IRQs already disabled
prepare the groundwork for the rest of PoC. It appears that not all
code is ready to be executed in IRQ state, and schedule() now can be
called at end of do_trap_irq(), which technically is considered IRQ
handler state. Also, it is unwise to try preempt things when we are
still 

[xen-unstable-coverity test] 159620: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xen  5d94433a66df29ce314696a13bdd324ec0e342fe
baseline version:
 xen  87a067fd8f4d4f7c6be02c3d38145115ac542017

Last test of basis   159515  2021-02-21 09:18:27 Z3 days
Testing same since   159620  2021-02-24 09:19:25 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Roger Pau Monne 
  Roger Pau Monné 
  Tamas K Lengyel 

jobs:
 coverity-amd64   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
   87a067fd8f..5d94433a66  5d94433a66df29ce314696a13bdd324ec0e342fe -> 
coverity-tested/smoke

[for-4.15][RESEND PATCH v4 2/2] xen/iommu: x86: Clear the root page-table before freeing the page-tables

[Julien Grall]

From: Julien Grall 

The new per-domain IOMMU page-table allocator will now free the
page-tables when domain's resources are relinquished. However, the
per-domain IOMMU structure will still contain a dangling pointer to
the root page-table.

Xen may access the IOMMU page-tables afterwards at least in the case of
PV domain:

(XEN) Xen call trace:
(XEN)[] R iommu.c#addr_to_dma_page_maddr+0x12e/0x1d8
(XEN)[] F iommu.c#intel_iommu_unmap_page+0x5d/0xf8
(XEN)[] F iommu_unmap+0x9c/0x129
(XEN)[] F iommu_legacy_unmap+0x26/0x63
(XEN)[] F mm.c#cleanup_page_mappings+0x139/0x144
(XEN)[] F put_page+0x4b/0xb3
(XEN)[] F put_page_from_l1e+0x136/0x13b
(XEN)[] F devalidate_page+0x256/0x8dc
(XEN)[] F mm.c#_put_page_type+0x236/0x47e
(XEN)[] F mm.c#put_pt_page+0x6f/0x80
(XEN)[] F mm.c#put_page_from_l2e+0x8a/0xcf
(XEN)[] F devalidate_page+0x3a3/0x8dc
(XEN)[] F mm.c#_put_page_type+0x236/0x47e
(XEN)[] F mm.c#put_pt_page+0x6f/0x80
(XEN)[] F mm.c#put_page_from_l3e+0x8a/0xcf
(XEN)[] F devalidate_page+0x56c/0x8dc
(XEN)[] F mm.c#_put_page_type+0x236/0x47e
(XEN)[] F mm.c#put_pt_page+0x6f/0x80
(XEN)[] F mm.c#put_page_from_l4e+0x69/0x6d
(XEN)[] F devalidate_page+0x6a0/0x8dc
(XEN)[] F mm.c#_put_page_type+0x236/0x47e
(XEN)[] F put_page_type_preemptible+0x13/0x15
(XEN)[] F domain.c#relinquish_memory+0x1ff/0x4e9
(XEN)[] F domain_relinquish_resources+0x2b6/0x36a
(XEN)[] F domain_kill+0xb8/0x141
(XEN)[] F do_domctl+0xb6f/0x18e5
(XEN)[] F pv_hypercall+0x2f0/0x55f
(XEN)[] F lstar_enter+0x112/0x120

This will result to a use after-free and possibly an host crash or
memory corruption.

It would not be possible to free the page-tables further down in
domain_relinquish_resources() because cleanup_page_mappings() will only
be called when the last reference on the page dropped. This may happen
much later if another domain still hold a reference.

After all the PCI devices have been de-assigned, nobody should use the
IOMMU page-tables and it is therefore pointless to try to modify them.

So we can simply clear any reference to the root page-table in the
per-domain IOMMU structure. This requires to introduce a new callback of
the method will depend on the IOMMU driver used.

Take the opportunity to add an ASSERT() in arch_iommu_domain_destroy()
to check if we freed all the IOMMU page tables.

Fixes: 3eef6d07d722 ("x86/iommu: convert VT-d code to use new page table 
allocator")
Signed-off-by: Julien Grall 

---
Changes in v4:
- Move the patch later in the series as we need to prevent
iommu_map() to allocate memory first.
- Add an ASSERT() in arch_iommu_domain_destroy().

Changes in v3:
- Move the patch earlier in the series
- Reword the commit message

Changes in v2:
- Introduce clear_root_pgtable()
- Move the patch later in the series
---
 xen/drivers/passthrough/amd/pci_amd_iommu.c | 12 +++-
 xen/drivers/passthrough/vtd/iommu.c | 12 +++-
 xen/drivers/passthrough/x86/iommu.c | 13 +
 xen/include/xen/iommu.h |  1 +
 4 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c 
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 42b5a5a9bec4..085fe2f5771e 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -381,9 +381,18 @@ static int amd_iommu_assign_device(struct domain *d, u8 
devfn,
 return reassign_device(pdev->domain, d, devfn, pdev);
 }
 
+static void amd_iommu_clear_root_pgtable(struct domain *d)
+{
+struct domain_iommu *hd = dom_iommu(d);
+
+spin_lock(>arch.mapping_lock);
+hd->arch.amd.root_table = NULL;
+spin_unlock(>arch.mapping_lock);
+}
+
 static void amd_iommu_domain_destroy(struct domain *d)
 {
-dom_iommu(d)->arch.amd.root_table = NULL;
+ASSERT(!dom_iommu(d)->arch.amd.root_table);
 }
 
 static int amd_iommu_add_device(u8 devfn, struct pci_dev *pdev)
@@ -565,6 +574,7 @@ static const struct iommu_ops __initconstrel _iommu_ops = {
 .remove_device = amd_iommu_remove_device,
 .assign_device  = amd_iommu_assign_device,
 .teardown = amd_iommu_domain_destroy,
+.clear_root_pgtable = amd_iommu_clear_root_pgtable,
 .map_page = amd_iommu_map_page,
 .unmap_page = amd_iommu_unmap_page,
 .iotlb_flush = amd_iommu_flush_iotlb_pages,
diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index b549a71530d5..475efb3be3bd 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1726,6 +1726,15 @@ out:
 return ret;
 }
 
+static void iommu_clear_root_pgtable(struct domain *d)
+{
+struct domain_iommu *hd = dom_iommu(d);
+
+spin_lock(>arch.mapping_lock);
+hd->arch.vtd.pgd_maddr = 0;
+spin_unlock(>arch.mapping_lock);
+}
+
 static void iommu_domain_teardown(struct domain *d)
 {
   

[for-4.15][RESEND PATCH v4 0/2] xen/iommu: Collection of bug fixes for IOMMU teardown

[Julien Grall]

From: Julien Grall 

Hi all,

This series is a collection of bug fixes for the IOMMU teardown code.
All of them are candidate for 4.15 as they can either leak memory or
lead to host crash/host corruption.

This is sent directly on xen-devel because all the issues were either
introduced in 4.15 or happen in the domain creation code.

Major changes since v3:
- Remove patch #3 "xen/iommu: x86: Harden the IOMMU page-table
allocator" as it is not strictly necessary for 4.15.
- Re-order the patches to avoid on a follow-up patch to fix
completely the issue.

Major changes since v2:
- patch #1 "xen/x86: p2m: Don't map the special pages in the IOMMU
page-tables" has been removed. This requires Jan's patch [1] to
fully mitigate memory leaks.

Cheers,

[1] <90271e69-c07e-a32c-5531-a79b10ef0...@suse.com>


*** BLURB HERE ***

Julien Grall (2):
  xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying
  xen/iommu: x86: Clear the root page-table before freeing the
page-tables

 xen/drivers/passthrough/amd/iommu_map.c | 12 +++
 xen/drivers/passthrough/amd/pci_amd_iommu.c | 12 ++-
 xen/drivers/passthrough/vtd/iommu.c | 24 -
 xen/drivers/passthrough/x86/iommu.c | 19 
 xen/include/xen/iommu.h |  1 +
 5 files changed, 66 insertions(+), 2 deletions(-)

-- 
2.17.1

[for-4.15][RESEND PATCH v4 1/2] xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying

[Julien Grall]

From: Julien Grall 

The new x86 IOMMU page-tables allocator will release the pages when
relinquishing the domain resources. However, this is not sufficient
when the domain is dying because nothing prevents page-table to be
allocated.

As the domain is dying, it is not necessary to continue to modify the
IOMMU page-tables as they are going to be destroyed soon.

At the moment, page-table allocates will only happen when iommu_map().
So after this change there will be no more page-table allocation
happening.

In order to observe d->is_dying correctly, we need to rely on per-arch
locking, so the check to ignore IOMMU mapping is added on the per-driver
map_page() callback.

Signed-off-by: Julien Grall 

---

As discussed in v3, this is only covering 4.15. We can discuss
post-4.15 how to catch page-table allocations if another caller (e.g.
iommu_unmap() if we ever decide to support superpages) start to use the
page-table allocator.

Changes in v4:
- Move the patch to the top of the queue
- Reword the commit message

Changes in v3:
- Patch added. This is a replacement of "xen/iommu: iommu_map: Don't
crash the domain if it is dying"
---
 xen/drivers/passthrough/amd/iommu_map.c | 12 
 xen/drivers/passthrough/vtd/iommu.c | 12 
 xen/drivers/passthrough/x86/iommu.c |  6 ++
 3 files changed, 30 insertions(+)

diff --git a/xen/drivers/passthrough/amd/iommu_map.c 
b/xen/drivers/passthrough/amd/iommu_map.c
index d3a8b1aec766..560af54b765b 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -285,6 +285,18 @@ int amd_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t 
mfn,
 
 spin_lock(>arch.mapping_lock);
 
+/*
+ * IOMMU mapping request can be safely ignored when the domain is dying.
+ *
+ * hd->arch.mapping_lock guarantees that d->is_dying will be observed
+ * before any page tables are freed (see iommu_free_pgtables()).
+ */
+if ( d->is_dying )
+{
+spin_unlock(>arch.mapping_lock);
+return 0;
+}
+
 rc = amd_iommu_alloc_root(d);
 if ( rc )
 {
diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index d136fe36883b..b549a71530d5 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1762,6 +1762,18 @@ static int __must_check intel_iommu_map_page(struct 
domain *d, dfn_t dfn,
 
 spin_lock(>arch.mapping_lock);
 
+/*
+ * IOMMU mapping request can be safely ignored when the domain is dying.
+ *
+ * hd->arch.mapping_lock guarantees that d->is_dying will be observed
+ * before any page tables are freed (see iommu_free_pgtables())
+ */
+if ( d->is_dying )
+{
+spin_unlock(>arch.mapping_lock);
+return 0;
+}
+
 pg_maddr = addr_to_dma_page_maddr(d, dfn_to_daddr(dfn), 1);
 if ( !pg_maddr )
 {
diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index cea1032b3d02..c6b03624fe28 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -267,6 +267,12 @@ int iommu_free_pgtables(struct domain *d)
 struct page_info *pg;
 unsigned int done = 0;
 
+if ( !is_iommu_enabled(d) )
+return 0;
+
+/* After this barrier, no more IOMMU mapping can happen */
+spin_barrier(>arch.mapping_lock);
+
 while ( (pg = page_list_remove_head(>arch.pgtables.list)) )
 {
 free_domheap_page(pg);
-- 
2.17.1

Re: [for-4.15][PATCH v4 0/2] xen/iommu: Collection of bug fixes for IOMMU teardown

[Julien Grall]

Hi all,

Please ignore this version. I forgot to CC the maintainers on it. I will 
resend a new series.


Cheers,

On 23/02/2021 12:34, Julien Grall wrote:

From: Julien Grall 

Hi all,

This series is a collection of bug fixes for the IOMMU teardown code.
All of them are candidate for 4.15 as they can either leak memory or
lead to host crash/host corruption.

This is sent directly on xen-devel because all the issues were either
introduced in 4.15 or happen in the domain creation code.

Major changes since v3:
 - Remove patch #3 "xen/iommu: x86: Harden the IOMMU page-table
 allocator" as it is not strictly necessary for 4.15.
 - Re-order the patches to avoid on a follow-up patch to fix
 completely the issue.

Major changes since v2:
 - patch #1 "xen/x86: p2m: Don't map the special pages in the IOMMU
 page-tables" has been removed. This requires Jan's patch [1] to
 fully mitigate memory leaks.

Cheers,

[1] <90271e69-c07e-a32c-5531-a79b10ef0...@suse.com>

Julien Grall (2):
   xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying
   xen/iommu: x86: Clear the root page-table before freeing the
 page-tables

  xen/drivers/passthrough/amd/iommu_map.c | 12 +++
  xen/drivers/passthrough/amd/pci_amd_iommu.c | 12 ++-
  xen/drivers/passthrough/vtd/iommu.c | 24 -
  xen/drivers/passthrough/x86/iommu.c | 19 
  xen/include/xen/iommu.h |  1 +
  5 files changed, 66 insertions(+), 2 deletions(-)



--
Julien Grall

Re: [PATCH RFC v3 4/4] x86/time: re-arrange struct calibration_rendezvous

[Jan Beulich]

On 09.02.2021 13:57, Jan Beulich wrote:
> To reduce latency on time_calibration_tsc_rendezvous()'s last loop
> iteration, separate fields written on the last iteration enough from the
> crucial field read by all CPUs on the last iteration such that they end
> up in distinct cache lines. Prefetch this field on an earlier iteration.

I've now measured the effects of this, at least to some degree. On my
single-socket 18-core Skylake system this reduces the average loop
exit time on CPU0 (from the TSC write on the last iteration to until
after the main loop) from around 32k cycles to around 28k (albeit the
values measured on separate runs vary quite significantly).

About the same effect (maybe a little less, but within error bounds)
can be had without any re-arrangement of the struct's layout, by
simply reading r->master_tsc_stamp into a local variable at the end
of each loop iteration. I'll make v4 use this less convoluted model
instead.

Jan

> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1655,10 +1655,20 @@ static void tsc_check_reliability(void)
>   * All CPUS snapshot their local TSC and extrapolation of system time.
>   */
>  struct calibration_rendezvous {
> -cpumask_t cpu_calibration_map;
>  atomic_t semaphore;
>  s_time_t master_stime;
> -uint64_t master_tsc_stamp, max_tsc_stamp;
> +cpumask_t cpu_calibration_map;
> +/*
> + * All CPUs want to read master_tsc_stamp on the last iteration.  If
> + * cpu_calibration_map isn't large enough to push the field into a cache
> + * line different from the one used by semaphore (written by all CPUs on
> + * every iteration) and master_stime (written by CPU0 on the last
> + * iteration), align the field suitably.
> + */
> +uint64_t __aligned(BITS_TO_LONGS(NR_CPUS) * sizeof(long) +
> +   sizeof(atomic_t) + sizeof(s_time_t) < SMP_CACHE_BYTES
> +   ? SMP_CACHE_BYTES : 1) master_tsc_stamp;
> +uint64_t max_tsc_stamp;
>  };
>  
>  static void
> @@ -1709,6 +1719,8 @@ static void time_calibration_tsc_rendezv
>  
>  if ( i == 0 )
>  write_tsc(r->master_tsc_stamp);
> +else
> +prefetch(>master_tsc_stamp);
>  
>  while ( atomic_read(>semaphore) != (2*total_cpus - 1) )
>  cpu_relax();
> @@ -1731,6 +1743,8 @@ static void time_calibration_tsc_rendezv
>  
>  if ( i == 0 )
>  write_tsc(r->master_tsc_stamp);
> +else
> +prefetch(>master_tsc_stamp);
>  
>  atomic_inc(>semaphore);
>  while ( atomic_read(>semaphore) > total_cpus )
> 
>