[libvirt test] 161927: regressions - FAIL

2021-05-13 Thread osstest service owner
flight 161927 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/161927/

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  156315cff4ddee560121328a530b308e1786d73b
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  307 days
Failing since151818  2020-07-11 04:18:52 Z  306 days  299 attempts
Testing same since   161927  2021-05-13 04:19:57 Z0 days1 attempts


People who touched revisions under test:
  Adolfo Jayme Barrientos 
  Aleksandr Alekseev 
  Aleksei Zakharov 
  Andika Triwidada 
  Andrea Bolognani 
  Balázs Meskó 
  Barrett Schonefeld 
  Bastian Germann 
  Bastien Orivel 
  BiaoXiang Ye 
  Bihong Yu 
  Binfeng Wu 
  Boris Fiuczynski 
  Brian Turek 
  Bruno Haible 
  Chris Mayo 
  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 
  Eric Farman 
  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 
  Hela Basa 
  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 
  Luke Yue 
  Luyao Zhong 
  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 
  SeongHyun Jo 
  Shalini Chellathurai Saroja 
  Shaojun Yang 
  Shi Lei 
  simmon 
  Simon Gaiser 
  Stefan Bader 
  Stefan Berger 
  Stefan Berger 
  Szymon Scholz 
  Thomas Huth 
  Tim Wiederhake 
  Tomáš Golembiovský 
  Tomáš Janoušek 
  Tuguoyi 
  Ville Skyttä 
  Wang Xin 
  WangJian 
  Weblate 
  Yalei Li <274268...@qq.com>
  Yalei Li 
  Yang Hang 
  Yanqiu Zhang 
  Yaroslav Kargin 
  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   

Hand over of the Xen shared info page

2021-05-13 Thread Anastasiia Lukianenko
Hi all,

The problem described below concerns cases when a shared info page
needs to be handed over from one entity in the system to another, for
example, there is a bootloader or any other code that may run before
the guest OS' kernel.
Normally, to map the shared info page guests allocate a memory page
from their RAM and map the shared info on top of it. Specifically we
use XENMAPSPACE_shared_info memory space in  XENMEM_add_to_physmap
hypercall.  As the info page exists throughout the guest existence this
doesn't hurt the guest, but when the page gets out of accounting, e.g.
after bootloader jumps to Linux and the page is not handed over to it,
the mapped page becomes a problem.
Consider the case with U-boot bootloader which already has Xen support.
The U-boot’s Xen guest implementation allocates a shared info page
between Xen and the guest domain and U-boot uses domain's RAM address
space to create and map the shared info page by using
XENMEM_add_to_physmap hypercall [1].

After U-boot transfers control to the operating system (Linux, Android,
etc), the shared info page is still mapped in domain’s address space,
e.g. its RAM. So, after we leave U-boot, this page becomes just an
ordinary memory page from Linux POV while it is still a shared info
page from Xen POV. This can lead to undefined behavior, errors etc as
Xen can write something to the shared info page and when Linux tries to
use it - data corruption may happen.
This happens because there is no unmap function in Xen API to remove an
existing shared info page mapping. We could use only hypercall
XENMEM_remove_from_physmap which eventually will create a hole in the
domain's RAM address space which may also lead to guest’s crash while
accessing that memory.

We noticed this problem and the workaround was implemented using the
special GUEST_MAGIC memory region [2].

Now we want to make a proper solution based on GUEST_MAGIC_BASE, which
does not belong to the guest’s RAM address space [3]. Using the example
of how offsets for the console and xenstore are implemented, we can add
a new shared_info offset and increase the number of magic pages [4] and
implement related functionality, so there is a similar API to query
that magic page location as it is done for console PFN and others.
This approach would allow the use of the XENMEM_remove_from_physmap
hypercall without creating gaps in the RAM address space for Xen guest
OS [5].

[1] - 
https://github.com/u-boot/u-boot/blob/master/drivers/xen/hypervisor.c#L141
[2] - 
https://github.com/xen-troops/u-boot/commit/f759b151116af204a5ab02ace82c09300cd6233a
[3] - 
https://github.com/xen-project/xen/blob/master/xen/include/public/arch-arm.h#L432
[4] - 
https://github.com/xen-project/xen/blob/25849c8b16f2a5b7fcd0a823e80a5f1b590291f9/tools/libs/guest/xg_dom_arm.c#L29
[5] - 
https://github.com/xen-troops/u-boot/blob/android-master/drivers/xen/hypervisor.c#L162

Regards,
Anastasiia Lukianenko


[linux-5.4 test] 161918: tolerable FAIL - PUSHED

2021-05-13 Thread osstest service owner
flight 161918 linux-5.4 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/161918/

Failures :-/ but no regressions.

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

Last test of basis   161832  2021-05-07 09:10:55 Z5 days
Testing same since   161905  2021-05-11 12:11:22 Z1 days3 attempts


People who touched revisions under test:
  Adrian Hunter 
  Alan Stern 
  Alex Deucher 
  Alexander Lobakin 
  Alexander Shishkin 
  Aneesh Kumar K.V 
  Anirudh Rayabharam 
  Anson Jacob 
  Ard Biesheuvel 
  Aric Cyr 
  Arnd Bergmann 
  Artur Petrosyan 
  Arun Easi 
  Avri Altman 
  Bart Van Assche 
  Benjamin Block 
  Bill Wendling 
  B

Re: Hand over of the Xen shared info page

2021-05-13 Thread Olaf Hering
Am Thu, 13 May 2021 08:03:48 +
schrieb Anastasiia Lukianenko :

> shared info page needs to be handed over from one entity in the system to 
> another

The same issue exists with kexec. Not sure why it was not addressed as you 
proposed, "soft_reset" was implemented instead.

Good luck.


Olaf


pgpIOu1jkf9oW.pgp
Description: Digitale Signatur von OpenPGP


Re: Hand over of the Xen shared info page

2021-05-13 Thread Julien Grall




On 13/05/2021 09:03, Anastasiia Lukianenko wrote:

Hi all,


Hi,


The problem described below concerns cases when a shared info page
needs to be handed over from one entity in the system to another, for
example, there is a bootloader or any other code that may run before
the guest OS' kernel.
Normally, to map the shared info page guests allocate a memory page
from their RAM and map the shared info on top of it. Specifically we
use XENMAPSPACE_shared_info memory space in  XENMEM_add_to_physmap
hypercall.  As the info page exists throughout the guest existence this
doesn't hurt the guest, but when the page gets out of accounting, e.g.
after bootloader jumps to Linux and the page is not handed over to it,
the mapped page becomes a problem.
Consider the case with U-boot bootloader which already has Xen support.
The U-boot’s Xen guest implementation allocates a shared info page
between Xen and the guest domain and U-boot uses domain's RAM address
space to create and map the shared info page by using
XENMEM_add_to_physmap hypercall [1].

After U-boot transfers control to the operating system (Linux, Android,
etc), the shared info page is still mapped in domain’s address space,
e.g. its RAM. So, after we leave U-boot, this page becomes just an
ordinary memory page from Linux POV while it is still a shared info
page from Xen POV. This can lead to undefined behavior, errors etc as
Xen can write something to the shared info page and when Linux tries to
use it - data corruption may happen.
This happens because there is no unmap function in Xen API to remove an
existing shared info page mapping. We could use only hypercall
XENMEM_remove_from_physmap which eventually will create a hole in the
domain's RAM address space which may also lead to guest’s crash while
accessing that memory.


The hypercall XENMEM_remove_from_physmap is the correct hypercall here 
and work as intented. It is not Xen business to keep track what was the 
original page (it may have been RAM, device...).


The problem here is the hypercall XENMEM_add_to_physmap is misused in 
U-boot. When you give an address for the mapping you are telling Xen 
"Here a free region to map the share paged". IOW, Xen will throw away 
whatever was before because that was you asked.


If you want to map in place of the RAM page, then the correct approach 
is to:

  1) Request Xen to remove the RAM page from the P2M
  2) Map the shared page
  /* Use it */
  3) Unmap the shared page
  4) Allocate the memory

You can avoid 1) and 4) by finding free region in the address space.



We noticed this problem and the workaround was implemented using the
special GUEST_MAGIC memory region [2].

Now we want to make a proper solution based on GUEST_MAGIC_BASE, which
does not belong to the guest’s RAM address space [3]. Using the example
of how offsets for the console and xenstore are implemented, we can add
a new shared_info offset and increase the number of magic pages [4] and
implement related functionality, so there is a similar API to query
that magic page location as it is done for console PFN and others.


They are not the same type. The console PFN points memory already 
populated in the guest address space.


For the domain shared page, this is memory belonging to Xen that you 
will map in your address space. A domain can map it anywhere it wants.



This approach would allow the use of the XENMEM_remove_from_physmap
hypercall without creating gaps in the RAM address space for Xen guest
OS [5].


See above to prevent the gap. I appreciate this means a superpage may 
get shattered.


The alternative is for U-boot to go through the DT and infer which 
regions are free (IOW any region not described).


Cheers,

--
Julien Grall



regression in Xen 4.15, unable to boot xenlinux dom0

2021-05-13 Thread Olaf Hering
I have access to a ProLiant BL465c G5, which can boot a xenlinux based dom0 
kernel, like the one included in SLE11SP4. But it fails to do that with 
staging-4.15 and staging:

...
[0.197199] node 0 link 0: io port [1000, 3fff]
[0.197199] node 0 link 2: io port [4000, ]
(XEN) emul-priv-op.c:1015:d0v0 RDMSR 0xc001001a unimplemented
[0.197199] general protection fault:  [#1] SMP 
[0.197199] CPU 0 
[0.197199] Modules linked in:
[0.197199] Supported: Yes
[0.197199] 
[0.197199] Pid: 1, comm: swapper Not tainted 3.0.101-63-xen #1 HP ProLiant 
BL465c G5  
[0.197199] RIP: e030:[]  [] 
early_fill_mp_bus_info+0x344/0x7f9


I have attached the full logs from staging-4.14 and staging-4.15.


Olaf


xen_414.xenlinux.txt.gz
Description: application/gzip


xen_415.xenlinux.txt.gz
Description: application/gzip


pgpBmYBLtGNis.pgp
Description: Digitale Signatur von OpenPGP


Re: regression in Xen 4.15, unable to boot xenlinux dom0

2021-05-13 Thread Roger Pau Monné
On Thu, May 13, 2021 at 11:21:36AM +0200, Olaf Hering wrote:
> I have access to a ProLiant BL465c G5, which can boot a xenlinux based dom0 
> kernel, like the one included in SLE11SP4. But it fails to do that with 
> staging-4.15 and staging:
> 
> ...
> [0.197199] node 0 link 0: io port [1000, 3fff]
> [0.197199] node 0 link 2: io port [4000, ]
> (XEN) emul-priv-op.c:1015:d0v0 RDMSR 0xc001001a unimplemented
> [0.197199] general protection fault:  [#1] SMP 
> [0.197199] CPU 0 
> [0.197199] Modules linked in:
> [0.197199] Supported: Yes
> [0.197199] 
> [0.197199] Pid: 1, comm: swapper Not tainted 3.0.101-63-xen #1 HP 
> ProLiant BL465c G5  
> [0.197199] RIP: e030:[]  [] 
> early_fill_mp_bus_info+0x344/0x7f9
> 
> 
> I have attached the full logs from staging-4.14 and staging-4.15.

Can you boot with dom0=msr-relaxed on the Xen command line?

Roger.



Re: regression in Xen 4.15, unable to boot xenlinux dom0

2021-05-13 Thread Olaf Hering
Am Thu, 13 May 2021 11:49:58 +0200
schrieb Roger Pau Monné :

> Can you boot with dom0=msr-relaxed on the Xen command line?

Yes, that helps - I just discovered this cmdline option.


Olaf


pgprxX69rpcQm.pgp
Description: Digitale Signatur von OpenPGP


[PATCH 0/8] xen: harden frontends against malicious backends

2021-05-13 Thread Juergen Gross
Xen backends of para-virtualized devices can live in dom0 kernel, dom0
user land, or in a driver domain. This means that a backend might
reside in a less trusted environment than the Xen core components, so
a backend should not be able to do harm to a Xen guest (it can still
mess up I/O data, but it shouldn't be able to e.g. crash a guest by
other means or cause a privilege escalation in the guest).

Unfortunately many frontends in the Linux kernel are fully trusting
their respective backends. This series is starting to fix the most
important frontends: console, disk and network.

It was discussed to handle this as a security problem, but the topic
was discussed in public before, so it isn't a real secret.

Juergen Gross (8):
  xen: sync include/xen/interface/io/ring.h with Xen's newest version
  xen/blkfront: read response from backend only once
  xen/blkfront: don't take local copy of a request from the ring page
  xen/blkfront: don't trust the backend response data blindly
  xen/netfront: read response from backend only once
  xen/netfront: don't read data from request on the ring page
  xen/netfront: don't trust the backend response data blindly
  xen/hvc: replace BUG_ON() with negative return value

 drivers/block/xen-blkfront.c| 118 +-
 drivers/net/xen-netfront.c  | 184 ++---
 drivers/tty/hvc/hvc_xen.c   |  15 +-
 include/xen/interface/io/ring.h | 278 ++--
 4 files changed, 369 insertions(+), 226 deletions(-)

-- 
2.26.2




[PATCH 6/8] xen/netfront: don't read data from request on the ring page

2021-05-13 Thread Juergen Gross
In order to avoid a malicious backend being able to influence the local
processing of a request build the request locally first and then copy
it to the ring page. Any reading from the request needs to be done on
the local instance.

Signed-off-by: Juergen Gross 
---
 drivers/net/xen-netfront.c | 75 ++
 1 file changed, 36 insertions(+), 39 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index f91e41ece554..261c35be0147 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -435,7 +435,8 @@ struct xennet_gnttab_make_txreq {
struct netfront_queue *queue;
struct sk_buff *skb;
struct page *page;
-   struct xen_netif_tx_request *tx; /* Last request */
+   struct xen_netif_tx_request *tx;  /* Last request on ring page */
+   struct xen_netif_tx_request tx_local; /* Last request local copy*/
unsigned int size;
 };
 
@@ -463,30 +464,27 @@ static void xennet_tx_setup_grant(unsigned long gfn, 
unsigned int offset,
queue->grant_tx_page[id] = page;
queue->grant_tx_ref[id] = ref;
 
-   tx->id = id;
-   tx->gref = ref;
-   tx->offset = offset;
-   tx->size = len;
-   tx->flags = 0;
+   info->tx_local.id = id;
+   info->tx_local.gref = ref;
+   info->tx_local.offset = offset;
+   info->tx_local.size = len;
+   info->tx_local.flags = 0;
+
+   *tx = info->tx_local;
 
info->tx = tx;
-   info->size += tx->size;
+   info->size += info->tx_local.size;
 }
 
 static struct xen_netif_tx_request *xennet_make_first_txreq(
-   struct netfront_queue *queue, struct sk_buff *skb,
-   struct page *page, unsigned int offset, unsigned int len)
+   struct xennet_gnttab_make_txreq *info,
+   unsigned int offset, unsigned int len)
 {
-   struct xennet_gnttab_make_txreq info = {
-   .queue = queue,
-   .skb = skb,
-   .page = page,
-   .size = 0,
-   };
+   info->size = 0;
 
-   gnttab_for_one_grant(page, offset, len, xennet_tx_setup_grant, &info);
+   gnttab_for_one_grant(info->page, offset, len, xennet_tx_setup_grant, 
info);
 
-   return info.tx;
+   return info->tx;
 }
 
 static void xennet_make_one_txreq(unsigned long gfn, unsigned int offset,
@@ -499,35 +497,27 @@ static void xennet_make_one_txreq(unsigned long gfn, 
unsigned int offset,
xennet_tx_setup_grant(gfn, offset, len, data);
 }
 
-static struct xen_netif_tx_request *xennet_make_txreqs(
-   struct netfront_queue *queue, struct xen_netif_tx_request *tx,
-   struct sk_buff *skb, struct page *page,
+static void xennet_make_txreqs(
+   struct xennet_gnttab_make_txreq *info,
+   struct page *page,
unsigned int offset, unsigned int len)
 {
-   struct xennet_gnttab_make_txreq info = {
-   .queue = queue,
-   .skb = skb,
-   .tx = tx,
-   };
-
/* Skip unused frames from start of page */
page += offset >> PAGE_SHIFT;
offset &= ~PAGE_MASK;
 
while (len) {
-   info.page = page;
-   info.size = 0;
+   info->page = page;
+   info->size = 0;
 
gnttab_foreach_grant_in_range(page, offset, len,
  xennet_make_one_txreq,
- &info);
+ info);
 
page++;
offset = 0;
-   len -= info.size;
+   len -= info->size;
}
-
-   return info.tx;
 }
 
 /*
@@ -580,10 +570,14 @@ static int xennet_xdp_xmit_one(struct net_device *dev,
 {
struct netfront_info *np = netdev_priv(dev);
struct netfront_stats *tx_stats = this_cpu_ptr(np->tx_stats);
+   struct xennet_gnttab_make_txreq info = {
+   .queue = queue,
+   .skb = NULL,
+   .page = virt_to_page(xdpf->data),
+   };
int notify;
 
-   xennet_make_first_txreq(queue, NULL,
-   virt_to_page(xdpf->data),
+   xennet_make_first_txreq(&info,
offset_in_page(xdpf->data),
xdpf->len);
 
@@ -647,6 +641,7 @@ static netdev_tx_t xennet_start_xmit(struct sk_buff *skb, 
struct net_device *dev
unsigned int len;
unsigned long flags;
struct netfront_queue *queue = NULL;
+   struct xennet_gnttab_make_txreq info = { };
unsigned int num_queues = dev->real_num_tx_queues;
u16 queue_index;
struct sk_buff *nskb;
@@ -704,14 +699,16 @@ static netdev_tx_t xennet_start_xmit(struct sk_buff *skb, 
struct net_device *dev
}
 
/* First request for the linear area. */
-   first_tx = tx = xennet_make_first_txreq(queue, skb,
-   page, offset, len);
+   info.q

[PATCH 2/8] xen/blkfront: read response from backend only once

2021-05-13 Thread Juergen Gross
In order to avoid problems in case the backend is modifying a response
on the ring page while the frontend has already seen it, just read the
response into a local buffer in one go and then operate on that buffer
only.

Signed-off-by: Juergen Gross 
---
 drivers/block/xen-blkfront.c | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 10df39a8b18d..a8b56c153330 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1557,7 +1557,7 @@ static bool blkif_completion(unsigned long *id,
 static irqreturn_t blkif_interrupt(int irq, void *dev_id)
 {
struct request *req;
-   struct blkif_response *bret;
+   struct blkif_response bret;
RING_IDX i, rp;
unsigned long flags;
struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)dev_id;
@@ -1574,8 +1574,9 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
for (i = rinfo->ring.rsp_cons; i != rp; i++) {
unsigned long id;
 
-   bret = RING_GET_RESPONSE(&rinfo->ring, i);
-   id   = bret->id;
+   RING_COPY_RESPONSE(&rinfo->ring, i, &bret);
+   id = bret.id;
+
/*
 * The backend has messed up and given us an id that we would
 * never have given to it (we stamp it up to BLK_RING_SIZE -
@@ -1583,39 +1584,39 @@ static irqreturn_t blkif_interrupt(int irq, void 
*dev_id)
 */
if (id >= BLK_RING_SIZE(info)) {
WARN(1, "%s: response to %s has incorrect id (%ld)\n",
-info->gd->disk_name, op_name(bret->operation), id);
+info->gd->disk_name, op_name(bret.operation), id);
/* We can't safely get the 'struct request' as
 * the id is busted. */
continue;
}
req  = rinfo->shadow[id].request;
 
-   if (bret->operation != BLKIF_OP_DISCARD) {
+   if (bret.operation != BLKIF_OP_DISCARD) {
/*
 * We may need to wait for an extra response if the
 * I/O request is split in 2
 */
-   if (!blkif_completion(&id, rinfo, bret))
+   if (!blkif_completion(&id, rinfo, &bret))
continue;
}
 
if (add_id_to_freelist(rinfo, id)) {
WARN(1, "%s: response to %s (id %ld) couldn't be 
recycled!\n",
-info->gd->disk_name, op_name(bret->operation), id);
+info->gd->disk_name, op_name(bret.operation), id);
continue;
}
 
-   if (bret->status == BLKIF_RSP_OKAY)
+   if (bret.status == BLKIF_RSP_OKAY)
blkif_req(req)->error = BLK_STS_OK;
else
blkif_req(req)->error = BLK_STS_IOERR;
 
-   switch (bret->operation) {
+   switch (bret.operation) {
case BLKIF_OP_DISCARD:
-   if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
+   if (unlikely(bret.status == BLKIF_RSP_EOPNOTSUPP)) {
struct request_queue *rq = info->rq;
printk(KERN_WARNING "blkfront: %s: %s op 
failed\n",
-  info->gd->disk_name, 
op_name(bret->operation));
+  info->gd->disk_name, 
op_name(bret.operation));
blkif_req(req)->error = BLK_STS_NOTSUPP;
info->feature_discard = 0;
info->feature_secdiscard = 0;
@@ -1625,15 +1626,15 @@ static irqreturn_t blkif_interrupt(int irq, void 
*dev_id)
break;
case BLKIF_OP_FLUSH_DISKCACHE:
case BLKIF_OP_WRITE_BARRIER:
-   if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
+   if (unlikely(bret.status == BLKIF_RSP_EOPNOTSUPP)) {
printk(KERN_WARNING "blkfront: %s: %s op 
failed\n",
-  info->gd->disk_name, 
op_name(bret->operation));
+  info->gd->disk_name, 
op_name(bret.operation));
blkif_req(req)->error = BLK_STS_NOTSUPP;
}
-   if (unlikely(bret->status == BLKIF_RSP_ERROR &&
+   if (unlikely(bret.status == BLKIF_RSP_ERROR &&
 rinfo->shadow[id].req.u.rw.nr_segments == 
0)) {
printk(KERN_WARNING "blkfront: %s: emp

[PATCH 1/8] xen: sync include/xen/interface/io/ring.h with Xen's newest version

2021-05-13 Thread Juergen Gross
Sync include/xen/interface/io/ring.h with Xen's newest version in
order to get the RING_COPY_RESPONSE() and RING_RESPONSE_PROD_OVERFLOW()
macros.

Note that this will correct the wrong license info by adding the
missing original copyright notice.

Signed-off-by: Juergen Gross 
---
 include/xen/interface/io/ring.h | 278 ++--
 1 file changed, 156 insertions(+), 122 deletions(-)

diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
index 2af7a1cd6658..b39cdbc522ec 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -1,21 +1,53 @@
-/* SPDX-License-Identifier: GPL-2.0 */
 /**
  * ring.h
  *
  * Shared producer-consumer ring macros.
  *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
  * Tim Deegan and Andrew Warfield November 2004.
  */
 
 #ifndef __XEN_PUBLIC_IO_RING_H__
 #define __XEN_PUBLIC_IO_RING_H__
 
+/*
+ * When #include'ing this header, you need to provide the following
+ * declaration upfront:
+ * - standard integers types (uint8_t, uint16_t, etc)
+ * They are provided by stdint.h of the standard headers.
+ *
+ * In addition, if you intend to use the FLEX macros, you also need to
+ * provide the following, before invoking the FLEX macros:
+ * - size_t
+ * - memcpy
+ * - grant_ref_t
+ * These declarations are provided by string.h of the standard headers,
+ * and grant_table.h from the Xen public headers.
+ */
+
 #include 
 
 typedef unsigned int RING_IDX;
 
 /* Round a 32-bit unsigned constant down to the nearest power of two. */
-#define __RD2(_x)  (((_x) & 0x0002) ? 0x2 : ((_x) & 0x1))
+#define __RD2(_x)  (((_x) & 0x0002) ? 0x2  : ((_x) & 0x1))
 #define __RD4(_x)  (((_x) & 0x000c) ? __RD2((_x)>>2)<<2: __RD2(_x))
 #define __RD8(_x)  (((_x) & 0x00f0) ? __RD4((_x)>>4)<<4: __RD4(_x))
 #define __RD16(_x) (((_x) & 0xff00) ? __RD8((_x)>>8)<<8: __RD8(_x))
@@ -27,82 +59,79 @@ typedef unsigned int RING_IDX;
  * A ring contains as many entries as will fit, rounded down to the nearest
  * power of two (so we can mask with (size-1) to loop around).
  */
-#define __CONST_RING_SIZE(_s, _sz) \
-   (__RD32(((_sz) - offsetof(struct _s##_sring, ring)) /   \
-   sizeof(((struct _s##_sring *)0)->ring[0])))
-
+#define __CONST_RING_SIZE(_s, _sz) \
+(__RD32(((_sz) - offsetof(struct _s##_sring, ring)) / \
+   sizeof(((struct _s##_sring *)0)->ring[0])))
 /*
  * The same for passing in an actual pointer instead of a name tag.
  */
-#define __RING_SIZE(_s, _sz)   \
-   (__RD32(((_sz) - (long)&(_s)->ring + (long)(_s)) / 
sizeof((_s)->ring[0])))
+#define __RING_SIZE(_s, _sz) \
+(__RD32(((_sz) - (long)(_s)->ring + (long)(_s)) / sizeof((_s)->ring[0])))
 
 /*
  * Macros to make the correct C datatypes for a new kind of ring.
  *
  * To make a new ring datatype, you need to have two message structures,
- * let's say struct request, and struct response already defined.
+ * let's say request_t, and response_t already defined.
  *
  * In a header where you want the ring datatype declared, you then do:
  *
- * DEFINE_RING_TYPES(mytag, struct request, struct response);
+ * DEFINE_RING_TYPES(mytag, request_t, response_t);
  *
  * These expand out to give you a set of types, as you can see below.
  * The most important of these are:
  *
- * struct mytag_sring  - The shared ring.
- * struct mytag_front_ring - The 'front' half of the ring.
- * struct mytag_back_ring  - The 'back' half of the ring.
+ * mytag_sring_t  - The shared ring.
+ * mytag_front_ring_t - The 'front' half of the ring.
+ * mytag_back_ring_t  - The 'back' half of the ring.
  *
  * To initialize a ring in your code you need to know the location and size
  * of the shared memory area (P

[PATCH 3/8] xen/blkfront: don't take local copy of a request from the ring page

2021-05-13 Thread Juergen Gross
In order to avoid a malicious backend being able to influence the local
copy of a request build the request locally first and then copy it to
the ring page instead of doing it the other way round as today.

Signed-off-by: Juergen Gross 
---
 drivers/block/xen-blkfront.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index a8b56c153330..c6a05de4f15f 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -546,7 +546,7 @@ static unsigned long blkif_ring_get_request(struct 
blkfront_ring_info *rinfo,
rinfo->shadow[id].status = REQ_WAITING;
rinfo->shadow[id].associated_id = NO_ASSOCIATED_ID;
 
-   (*ring_req)->u.rw.id = id;
+   rinfo->shadow[id].req.u.rw.id = id;
 
return id;
 }
@@ -554,11 +554,12 @@ static unsigned long blkif_ring_get_request(struct 
blkfront_ring_info *rinfo,
 static int blkif_queue_discard_req(struct request *req, struct 
blkfront_ring_info *rinfo)
 {
struct blkfront_info *info = rinfo->dev_info;
-   struct blkif_request *ring_req;
+   struct blkif_request *ring_req, *final_ring_req;
unsigned long id;
 
/* Fill out a communications ring structure. */
-   id = blkif_ring_get_request(rinfo, req, &ring_req);
+   id = blkif_ring_get_request(rinfo, req, &final_ring_req);
+   ring_req = &rinfo->shadow[id].req;
 
ring_req->operation = BLKIF_OP_DISCARD;
ring_req->u.discard.nr_sectors = blk_rq_sectors(req);
@@ -569,8 +570,8 @@ static int blkif_queue_discard_req(struct request *req, 
struct blkfront_ring_inf
else
ring_req->u.discard.flag = 0;
 
-   /* Keep a private copy so we can reissue requests when recovering. */
-   rinfo->shadow[id].req = *ring_req;
+   /* Copy the request to the ring page. */
+   *final_ring_req = *ring_req;
 
return 0;
 }
@@ -703,6 +704,7 @@ static int blkif_queue_rw_req(struct request *req, struct 
blkfront_ring_info *ri
 {
struct blkfront_info *info = rinfo->dev_info;
struct blkif_request *ring_req, *extra_ring_req = NULL;
+   struct blkif_request *final_ring_req, *final_extra_ring_req;
unsigned long id, extra_id = NO_ASSOCIATED_ID;
bool require_extra_req = false;
int i;
@@ -747,7 +749,8 @@ static int blkif_queue_rw_req(struct request *req, struct 
blkfront_ring_info *ri
}
 
/* Fill out a communications ring structure. */
-   id = blkif_ring_get_request(rinfo, req, &ring_req);
+   id = blkif_ring_get_request(rinfo, req, &final_ring_req);
+   ring_req = &rinfo->shadow[id].req;
 
num_sg = blk_rq_map_sg(req->q, req, rinfo->shadow[id].sg);
num_grant = 0;
@@ -798,7 +801,9 @@ static int blkif_queue_rw_req(struct request *req, struct 
blkfront_ring_info *ri
ring_req->u.rw.nr_segments = num_grant;
if (unlikely(require_extra_req)) {
extra_id = blkif_ring_get_request(rinfo, req,
- &extra_ring_req);
+ 
&final_extra_ring_req);
+   extra_ring_req = &rinfo->shadow[extra_id].req;
+
/*
 * Only the first request contains the scatter-gather
 * list.
@@ -840,10 +845,10 @@ static int blkif_queue_rw_req(struct request *req, struct 
blkfront_ring_info *ri
if (setup.segments)
kunmap_atomic(setup.segments);
 
-   /* Keep a private copy so we can reissue requests when recovering. */
-   rinfo->shadow[id].req = *ring_req;
+   /* Copy request(s) to the ring page. */
+   *final_ring_req = *ring_req;
if (unlikely(require_extra_req))
-   rinfo->shadow[extra_id].req = *extra_ring_req;
+   *final_extra_ring_req = *extra_ring_req;
 
if (new_persistent_gnts)
gnttab_free_grant_references(setup.gref_head);
-- 
2.26.2




[PATCH 7/8] xen/netfront: don't trust the backend response data blindly

2021-05-13 Thread Juergen Gross
Today netfront will trust the backend to send only sane response data.
In order to avoid privilege escalations or crashes in case of malicious
backends verify the data to be within expected limits. Especially make
sure that the response always references an outstanding request.

Note that only the tx queue needs special id handling, as for the rx
queue the id is equal to the index in the ring page.

Introduce a new indicator for the device whether it is broken and let
the device stop working when it is set. Set this indicator in case the
backend sets any weird data.

Signed-off-by: Juergen Gross 
---
 drivers/net/xen-netfront.c | 71 +++---
 1 file changed, 67 insertions(+), 4 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 261c35be0147..ccd6d1389b0a 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -154,6 +154,8 @@ struct netfront_queue {
 
struct page_pool *page_pool;
struct xdp_rxq_info xdp_rxq;
+
+   bool tx_pending[NET_TX_RING_SIZE];
 };
 
 struct netfront_info {
@@ -173,6 +175,9 @@ struct netfront_info {
bool netback_has_xdp_headroom;
bool netfront_xdp_enabled;
 
+   /* Is device behaving sane? */
+   bool broken;
+
atomic_t rx_gso_checksum_fixup;
 };
 
@@ -363,7 +368,7 @@ static int xennet_open(struct net_device *dev)
unsigned int i = 0;
struct netfront_queue *queue = NULL;
 
-   if (!np->queues)
+   if (!np->queues || np->broken)
return -ENODEV;
 
for (i = 0; i < num_queues; ++i) {
@@ -391,11 +396,17 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue)
unsigned short id;
struct sk_buff *skb;
bool more_to_do;
+   const struct device *dev = &queue->info->netdev->dev;
 
BUG_ON(!netif_carrier_ok(queue->info->netdev));
 
do {
prod = queue->tx.sring->rsp_prod;
+   if (RING_RESPONSE_PROD_OVERFLOW(&queue->tx, prod)) {
+   dev_alert(dev, "Illegal number of responses %u\n",
+ prod - queue->tx.rsp_cons);
+   goto err;
+   }
rmb(); /* Ensure we see responses up to 'rp'. */
 
for (cons = queue->tx.rsp_cons; cons != prod; cons++) {
@@ -406,12 +417,25 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue)
continue;
 
id  = txrsp.id;
+   if (id >= RING_SIZE(&queue->tx)) {
+   dev_alert(dev,
+ "Response has incorrect id (%u)\n",
+ id);
+   goto err;
+   }
+   if (!queue->tx_pending[id]) {
+   dev_alert(dev,
+ "Response for inactive request\n");
+   goto err;
+   }
+
+   queue->tx_pending[id] = false;
skb = queue->tx_skbs[id].skb;
if (unlikely(gnttab_query_foreign_access(
queue->grant_tx_ref[id]) != 0)) {
-   pr_alert("%s: warning -- grant still in use by 
backend domain\n",
-__func__);
-   BUG();
+   dev_alert(dev,
+ "Grant still in use by backend 
domain\n");
+   goto err;
}
gnttab_end_foreign_access_ref(
queue->grant_tx_ref[id], GNTMAP_readonly);
@@ -429,6 +453,12 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue)
} while (more_to_do);
 
xennet_maybe_wake_tx(queue);
+
+   return;
+
+ err:
+   queue->info->broken = true;
+   dev_alert(dev, "Disabled for further use\n");
 }
 
 struct xennet_gnttab_make_txreq {
@@ -472,6 +502,13 @@ static void xennet_tx_setup_grant(unsigned long gfn, 
unsigned int offset,
 
*tx = info->tx_local;
 
+   /*
+* The request is not in its final form, as size and flags might be
+* modified later, but even if a malicious backend will send a response
+* now, nothing bad regarding security could happen.
+*/
+   queue->tx_pending[id] = true;
+
info->tx = tx;
info->size += info->tx_local.size;
 }
@@ -605,6 +642,8 @@ static int xennet_xdp_xmit(struct net_device *dev, int n,
int nxmit = 0;
int i;
 
+   if (unlikely(np->broken))
+   return -ENODEV;
if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
return -EINVAL;
 
@@ -649,6 +688,8 @@ static netdev_tx_t xennet_start_xmit(struct sk_buff *skb, 
struct net_device *dev
/* D

[PATCH 4/8] xen/blkfront: don't trust the backend response data blindly

2021-05-13 Thread Juergen Gross
Today blkfront will trust the backend to send only sane response data.
In order to avoid privilege escalations or crashes in case of malicious
backends verify the data to be within expected limits. Especially make
sure that the response always references an outstanding request.

Introduce a new state of the ring BLKIF_STATE_ERROR which will be
switched to in case an inconsistency is being detected.

Signed-off-by: Juergen Gross 
---
 drivers/block/xen-blkfront.c | 62 +++-
 1 file changed, 47 insertions(+), 15 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index c6a05de4f15f..aa0f159829b4 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -80,6 +80,7 @@ enum blkif_state {
BLKIF_STATE_DISCONNECTED,
BLKIF_STATE_CONNECTED,
BLKIF_STATE_SUSPENDED,
+   BLKIF_STATE_ERROR,
 };
 
 struct grant {
@@ -89,6 +90,7 @@ struct grant {
 };
 
 enum blk_req_status {
+   REQ_PROCESSING,
REQ_WAITING,
REQ_DONE,
REQ_ERROR,
@@ -543,7 +545,7 @@ static unsigned long blkif_ring_get_request(struct 
blkfront_ring_info *rinfo,
 
id = get_id_from_freelist(rinfo);
rinfo->shadow[id].request = req;
-   rinfo->shadow[id].status = REQ_WAITING;
+   rinfo->shadow[id].status = REQ_PROCESSING;
rinfo->shadow[id].associated_id = NO_ASSOCIATED_ID;
 
rinfo->shadow[id].req.u.rw.id = id;
@@ -572,6 +574,7 @@ static int blkif_queue_discard_req(struct request *req, 
struct blkfront_ring_inf
 
/* Copy the request to the ring page. */
*final_ring_req = *ring_req;
+   rinfo->shadow[id].status = REQ_WAITING;
 
return 0;
 }
@@ -847,8 +850,11 @@ static int blkif_queue_rw_req(struct request *req, struct 
blkfront_ring_info *ri
 
/* Copy request(s) to the ring page. */
*final_ring_req = *ring_req;
-   if (unlikely(require_extra_req))
+   rinfo->shadow[id].status = REQ_WAITING;
+   if (unlikely(require_extra_req)) {
*final_extra_ring_req = *extra_ring_req;
+   rinfo->shadow[extra_id].status = REQ_WAITING;
+   }
 
if (new_persistent_gnts)
gnttab_free_grant_references(setup.gref_head);
@@ -1420,8 +1426,8 @@ static enum blk_req_status blkif_rsp_to_req_status(int 
rsp)
 static int blkif_get_final_status(enum blk_req_status s1,
  enum blk_req_status s2)
 {
-   BUG_ON(s1 == REQ_WAITING);
-   BUG_ON(s2 == REQ_WAITING);
+   BUG_ON(s1 < REQ_DONE);
+   BUG_ON(s2 < REQ_DONE);
 
if (s1 == REQ_ERROR || s2 == REQ_ERROR)
return BLKIF_RSP_ERROR;
@@ -1454,7 +1460,7 @@ static bool blkif_completion(unsigned long *id,
s->status = blkif_rsp_to_req_status(bret->status);
 
/* Wait the second response if not yet here. */
-   if (s2->status == REQ_WAITING)
+   if (s2->status < REQ_DONE)
return false;
 
bret->status = blkif_get_final_status(s->status,
@@ -1574,10 +1580,16 @@ static irqreturn_t blkif_interrupt(int irq, void 
*dev_id)
spin_lock_irqsave(&rinfo->ring_lock, flags);
  again:
rp = rinfo->ring.sring->rsp_prod;
+   if (RING_RESPONSE_PROD_OVERFLOW(&rinfo->ring, rp)) {
+   pr_alert("%s: illegal number of responses %u\n",
+info->gd->disk_name, rp - rinfo->ring.rsp_cons);
+   goto err;
+   }
rmb(); /* Ensure we see queued responses up to 'rp'. */
 
for (i = rinfo->ring.rsp_cons; i != rp; i++) {
unsigned long id;
+   unsigned int op;
 
RING_COPY_RESPONSE(&rinfo->ring, i, &bret);
id = bret.id;
@@ -1588,14 +1600,28 @@ static irqreturn_t blkif_interrupt(int irq, void 
*dev_id)
 * look in get_id_from_freelist.
 */
if (id >= BLK_RING_SIZE(info)) {
-   WARN(1, "%s: response to %s has incorrect id (%ld)\n",
-info->gd->disk_name, op_name(bret.operation), id);
-   /* We can't safely get the 'struct request' as
-* the id is busted. */
-   continue;
+   pr_alert("%s: response has incorrect id (%ld)\n",
+info->gd->disk_name, id);
+   goto err;
}
+   if (rinfo->shadow[id].status != REQ_WAITING) {
+   pr_alert("%s: response references no pending request\n",
+info->gd->disk_name);
+   goto err;
+   }
+
+   rinfo->shadow[id].status = REQ_PROCESSING;
req  = rinfo->shadow[id].request;
 
+   op = rinfo->shadow[id].req.operation;
+   if (op == BLKIF_OP_INDIRECT)
+   op = rinfo->shadow[

[PATCH 5/8] xen/netfront: read response from backend only once

2021-05-13 Thread Juergen Gross
In order to avoid problems in case the backend is modifying a response
on the ring page while the frontend has already seen it, just read the
response into a local buffer in one go and then operate on that buffer
only.

Signed-off-by: Juergen Gross 
---
 drivers/net/xen-netfront.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 44275908d61a..f91e41ece554 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -399,13 +399,13 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue)
rmb(); /* Ensure we see responses up to 'rp'. */
 
for (cons = queue->tx.rsp_cons; cons != prod; cons++) {
-   struct xen_netif_tx_response *txrsp;
+   struct xen_netif_tx_response txrsp;
 
-   txrsp = RING_GET_RESPONSE(&queue->tx, cons);
-   if (txrsp->status == XEN_NETIF_RSP_NULL)
+   RING_COPY_RESPONSE(&queue->tx, cons, &txrsp);
+   if (txrsp.status == XEN_NETIF_RSP_NULL)
continue;
 
-   id  = txrsp->id;
+   id  = txrsp.id;
skb = queue->tx_skbs[id].skb;
if (unlikely(gnttab_query_foreign_access(
queue->grant_tx_ref[id]) != 0)) {
@@ -814,7 +814,7 @@ static int xennet_get_extras(struct netfront_queue *queue,
 RING_IDX rp)
 
 {
-   struct xen_netif_extra_info *extra;
+   struct xen_netif_extra_info extra;
struct device *dev = &queue->info->netdev->dev;
RING_IDX cons = queue->rx.rsp_cons;
int err = 0;
@@ -830,24 +830,22 @@ static int xennet_get_extras(struct netfront_queue *queue,
break;
}
 
-   extra = (struct xen_netif_extra_info *)
-   RING_GET_RESPONSE(&queue->rx, ++cons);
+   RING_COPY_RESPONSE(&queue->rx, ++cons, &extra);
 
-   if (unlikely(!extra->type ||
-extra->type >= XEN_NETIF_EXTRA_TYPE_MAX)) {
+   if (unlikely(!extra.type ||
+extra.type >= XEN_NETIF_EXTRA_TYPE_MAX)) {
if (net_ratelimit())
dev_warn(dev, "Invalid extra type: %d\n",
-   extra->type);
+   extra.type);
err = -EINVAL;
} else {
-   memcpy(&extras[extra->type - 1], extra,
-  sizeof(*extra));
+   memcpy(&extras[extra.type - 1], &extra, sizeof(extra));
}
 
skb = xennet_get_rx_skb(queue, cons);
ref = xennet_get_rx_ref(queue, cons);
xennet_move_rx_slot(queue, skb, ref);
-   } while (extra->flags & XEN_NETIF_EXTRA_FLAG_MORE);
+   } while (extra.flags & XEN_NETIF_EXTRA_FLAG_MORE);
 
queue->rx.rsp_cons = cons;
return err;
@@ -905,7 +903,7 @@ static int xennet_get_responses(struct netfront_queue 
*queue,
struct sk_buff_head *list,
bool *need_xdp_flush)
 {
-   struct xen_netif_rx_response *rx = &rinfo->rx;
+   struct xen_netif_rx_response *rx = &rinfo->rx, rx_local;
int max = XEN_NETIF_NR_SLOTS_MIN + (rx->status <= RX_COPY_THRESHOLD);
RING_IDX cons = queue->rx.rsp_cons;
struct sk_buff *skb = xennet_get_rx_skb(queue, cons);
@@ -989,7 +987,8 @@ static int xennet_get_responses(struct netfront_queue 
*queue,
break;
}
 
-   rx = RING_GET_RESPONSE(&queue->rx, cons + slots);
+   RING_COPY_RESPONSE(&queue->rx, cons + slots, &rx_local);
+   rx = &rx_local;
skb = xennet_get_rx_skb(queue, cons + slots);
ref = xennet_get_rx_ref(queue, cons + slots);
slots++;
@@ -1044,10 +1043,11 @@ static int xennet_fill_frags(struct netfront_queue 
*queue,
struct sk_buff *nskb;
 
while ((nskb = __skb_dequeue(list))) {
-   struct xen_netif_rx_response *rx =
-   RING_GET_RESPONSE(&queue->rx, ++cons);
+   struct xen_netif_rx_response rx;
skb_frag_t *nfrag = &skb_shinfo(nskb)->frags[0];
 
+   RING_COPY_RESPONSE(&queue->rx, ++cons, &rx);
+
if (skb_shinfo(skb)->nr_frags == MAX_SKB_FRAGS) {
unsigned int pull_to = NETFRONT_SKB_CB(skb)->pull_to;
 
@@ -1062,7 +1062,7 @@ static int xennet_fill_frags(struct netfront_queue *queue,
 
skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
skb_frag_page(nfrag),
-   rx->off

[PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value

2021-05-13 Thread Juergen Gross
Xen frontends shouldn't BUG() in case of illegal data received from
their backends. So replace the BUG_ON()s when reading illegal data from
the ring page with negative return values.

Signed-off-by: Juergen Gross 
---
 drivers/tty/hvc/hvc_xen.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 92c9a476defc..30d7ffb1e04c 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -86,6 +86,11 @@ static int __write_console(struct xencons_info *xencons,
cons = intf->out_cons;
prod = intf->out_prod;
mb();   /* update queue values before going on */
+
+   if (WARN_ONCE((prod - cons) > sizeof(intf->out),
+ "Illegal ring page indices"))
+   return -EINVAL;
+
BUG_ON((prod - cons) > sizeof(intf->out));
 
while ((sent < len) && ((prod - cons) < sizeof(intf->out)))
@@ -114,7 +119,10 @@ static int domU_write_console(uint32_t vtermno, const char 
*data, int len)
 */
while (len) {
int sent = __write_console(cons, data, len);
-   
+
+   if (sent < 0)
+   return sent;
+
data += sent;
len -= sent;
 
@@ -138,7 +146,10 @@ static int domU_read_console(uint32_t vtermno, char *buf, 
int len)
cons = intf->in_cons;
prod = intf->in_prod;
mb();   /* get pointers before reading ring */
-   BUG_ON((prod - cons) > sizeof(intf->in));
+
+   if (WARN_ONCE((prod - cons) > sizeof(intf->in),
+ "Illegal ring page indices"))
+   return -EINVAL;
 
while (cons != prod && recv < len)
buf[recv++] = intf->in[MASK_XENCONS_IDX(cons++, intf->in)];
-- 
2.26.2




Re: [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value

2021-05-13 Thread Juergen Gross

On 13.05.21 12:16, Christophe Leroy wrote:



Le 13/05/2021 à 12:03, Juergen Gross a écrit :

Xen frontends shouldn't BUG() in case of illegal data received from
their backends. So replace the BUG_ON()s when reading illegal data from
the ring page with negative return values.

Signed-off-by: Juergen Gross 
---
  drivers/tty/hvc/hvc_xen.c | 15 +--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 92c9a476defc..30d7ffb1e04c 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -86,6 +86,11 @@ static int __write_console(struct xencons_info 
*xencons,

  cons = intf->out_cons;
  prod = intf->out_prod;
  mb();    /* update queue values before going on */
+
+    if (WARN_ONCE((prod - cons) > sizeof(intf->out),
+  "Illegal ring page indices"))
+    return -EINVAL;
+
  BUG_ON((prod - cons) > sizeof(intf->out));


Why keep the BUG_ON() ?


Oh, failed to delete it. Thanks for noticing.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature


regression in recent pvops kernels, dom0 crashes early

2021-05-13 Thread Olaf Hering
Recent pvops dom0 kernels fail to boot on this particular ProLiant BL465c G5 
box.
It happens to work with every Xen and a 4.4 based sle12sp3 kernel, but fails 
with every Xen and a 4.12 based sle12sp4 (and every newer) kernel.

Any idea what is going on?


(XEN) Freed 256kB init memory.
(XEN) mm.c:1758:d0 Bad L1 flags 80
(XEN) traps.c:458:d0 Unhandled invalid opcode fault/trap [#6] on VCPU 0 
[ec=]
(XEN) domain_crash_sync called from entry.S: fault at 82d08022a2a0 
create_bounce_frame+0x133/0x143
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) [ Xen-4.4.20170405T152638.6bf0560e12-9.xen44  x86_64  debug=y  Not 
tainted ]



(XEN) Freed 656kB init memory
(XEN) mm.c:2165:d0v0 Bad L1 flags 80
(XEN) d0v0 Unhandled invalid opcode fault/trap [#6, ec=]
(XEN) domain_crash_sync called from entry.S: fault at 82d04031a016 
x86_64/entry.S#create_bounce_frame+0x15d/0x177
(XEN) Domain 0 (vcpu#0) crashed on cpu#5:
(XEN) [ Xen-4.15.20210504T145803.280d472f4f-6.xen415  x86_64  debug=y  Not 
tainted ]


I can probably cycle through all kernels between 4.4 and 4.12 to see where it 
broke.


Olaf


xen_404.sle12sp4.txt.gz
Description: application/gzip


xen_415.tw.txt.gz
Description: application/gzip


pgpPJvwq0XL2v.pgp
Description: Digitale Signatur von OpenPGP


Re: [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value

2021-05-13 Thread Greg Kroah-Hartman
On Thu, May 13, 2021 at 12:03:02PM +0200, Juergen Gross wrote:
> Xen frontends shouldn't BUG() in case of illegal data received from
> their backends. So replace the BUG_ON()s when reading illegal data from
> the ring page with negative return values.
> 
> Signed-off-by: Juergen Gross 
> ---
>  drivers/tty/hvc/hvc_xen.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 92c9a476defc..30d7ffb1e04c 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -86,6 +86,11 @@ static int __write_console(struct xencons_info *xencons,
>   cons = intf->out_cons;
>   prod = intf->out_prod;
>   mb();   /* update queue values before going on */
> +
> + if (WARN_ONCE((prod - cons) > sizeof(intf->out),
> +   "Illegal ring page indices"))
> + return -EINVAL;

How nice, you just rebooted on panic-on-warn systems :(

> +
>   BUG_ON((prod - cons) > sizeof(intf->out));

Why keep this line?

Please just fix this up properly, if userspace can trigger this, then
both the WARN_ON() and BUG_ON() are not correct and need to be correctly
handled.


>  
>   while ((sent < len) && ((prod - cons) < sizeof(intf->out)))
> @@ -114,7 +119,10 @@ static int domU_write_console(uint32_t vtermno, const 
> char *data, int len)
>*/
>   while (len) {
>   int sent = __write_console(cons, data, len);
> - 
> +
> + if (sent < 0)
> + return sent;
> +
>   data += sent;
>   len -= sent;
>  
> @@ -138,7 +146,10 @@ static int domU_read_console(uint32_t vtermno, char 
> *buf, int len)
>   cons = intf->in_cons;
>   prod = intf->in_prod;
>   mb();   /* get pointers before reading ring */
> - BUG_ON((prod - cons) > sizeof(intf->in));
> +
> + if (WARN_ONCE((prod - cons) > sizeof(intf->in),
> +   "Illegal ring page indices"))
> + return -EINVAL;

Same here, you still just paniced a machine :(

thanks,

greg k-h



Re: [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value

2021-05-13 Thread Juergen Gross

On 13.05.21 12:25, Greg Kroah-Hartman wrote:

On Thu, May 13, 2021 at 12:03:02PM +0200, Juergen Gross wrote:

Xen frontends shouldn't BUG() in case of illegal data received from
their backends. So replace the BUG_ON()s when reading illegal data from
the ring page with negative return values.

Signed-off-by: Juergen Gross 
---
  drivers/tty/hvc/hvc_xen.c | 15 +--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 92c9a476defc..30d7ffb1e04c 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -86,6 +86,11 @@ static int __write_console(struct xencons_info *xencons,
cons = intf->out_cons;
prod = intf->out_prod;
mb();   /* update queue values before going on */
+
+   if (WARN_ONCE((prod - cons) > sizeof(intf->out),
+ "Illegal ring page indices"))
+   return -EINVAL;


How nice, you just rebooted on panic-on-warn systems :(


+
BUG_ON((prod - cons) > sizeof(intf->out));


Why keep this line?


Failed to delete it, sorry.



Please just fix this up properly, if userspace can trigger this, then
both the WARN_ON() and BUG_ON() are not correct and need to be correctly
handled.


It can be triggered by the console backend, but I agree a WARN isn't the
way to go here.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature


Re: [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value

2021-05-13 Thread Christophe Leroy




Le 13/05/2021 à 12:03, Juergen Gross a écrit :

Xen frontends shouldn't BUG() in case of illegal data received from
their backends. So replace the BUG_ON()s when reading illegal data from
the ring page with negative return values.

Signed-off-by: Juergen Gross 
---
  drivers/tty/hvc/hvc_xen.c | 15 +--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 92c9a476defc..30d7ffb1e04c 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -86,6 +86,11 @@ static int __write_console(struct xencons_info *xencons,
cons = intf->out_cons;
prod = intf->out_prod;
mb();   /* update queue values before going on */
+
+   if (WARN_ONCE((prod - cons) > sizeof(intf->out),
+ "Illegal ring page indices"))
+   return -EINVAL;
+
BUG_ON((prod - cons) > sizeof(intf->out));


Why keep the BUG_ON() ?


  
  	while ((sent < len) && ((prod - cons) < sizeof(intf->out)))

@@ -114,7 +119,10 @@ static int domU_write_console(uint32_t vtermno, const char 
*data, int len)
 */
while (len) {
int sent = __write_console(cons, data, len);
-   
+
+   if (sent < 0)
+   return sent;
+
data += sent;
len -= sent;
  
@@ -138,7 +146,10 @@ static int domU_read_console(uint32_t vtermno, char *buf, int len)

cons = intf->in_cons;
prod = intf->in_prod;
mb();   /* get pointers before reading ring */
-   BUG_ON((prod - cons) > sizeof(intf->in));
+
+   if (WARN_ONCE((prod - cons) > sizeof(intf->in),
+ "Illegal ring page indices"))
+   return -EINVAL;
  
  	while (cons != prod && recv < len)

buf[recv++] = intf->in[MASK_XENCONS_IDX(cons++, intf->in)];





Re: regression in recent pvops kernels, dom0 crashes early

2021-05-13 Thread Andrew Cooper
On 13/05/2021 11:24, Olaf Hering wrote:
> Recent pvops dom0 kernels fail to boot on this particular ProLiant BL465c G5 
> box.
> It happens to work with every Xen and a 4.4 based sle12sp3 kernel, but fails 
> with every Xen and a 4.12 based sle12sp4 (and every newer) kernel.
>
> Any idea what is going on?
>
> 
> (XEN) Freed 256kB init memory.
> (XEN) mm.c:1758:d0 Bad L1 flags 80
> (XEN) traps.c:458:d0 Unhandled invalid opcode fault/trap [#6] on VCPU 0 
> [ec=]
> (XEN) domain_crash_sync called from entry.S: fault at 82d08022a2a0 
> create_bounce_frame+0x133/0x143
> (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
> (XEN) [ Xen-4.4.20170405T152638.6bf0560e12-9.xen44  x86_64  debug=y  Not 
> tainted ]
> 
>
> 
> (XEN) Freed 656kB init memory
> (XEN) mm.c:2165:d0v0 Bad L1 flags 80
> (XEN) d0v0 Unhandled invalid opcode fault/trap [#6, ec=]
> (XEN) domain_crash_sync called from entry.S: fault at 82d04031a016 
> x86_64/entry.S#create_bounce_frame+0x15d/0x177
> (XEN) Domain 0 (vcpu#0) crashed on cpu#5:
> (XEN) [ Xen-4.15.20210504T145803.280d472f4f-6.xen415  x86_64  debug=y  
> Not tainted ]
> 
>
> I can probably cycle through all kernels between 4.4 and 4.12 to see where it 
> broke.

"Unhandled invalid opcode fault/trap" is "Xen tried to raise #UD with
the guest, and it hasn't set up a handler yet".  The Bad L1 flags
earlier means there was an attempted edit to a pagetable which was
rejected by Xen.

These two things aren't obviously related by a single action in Xen, so
I expect the pagetable modification failed, and the guest fell into a
bad error path.


If I'm counting bits correctly, that is Xen rejecting the use of the NX
bit, which is suspicious.  Do you have the full Xen boot log on this
box?  I wonder if we've some problem clobbing the XD-disable bit.

~Andrew




Re: regression in recent pvops kernels, dom0 crashes early

2021-05-13 Thread Olaf Hering
Am Thu, 13 May 2021 13:11:10 +0100
schrieb Andrew Cooper :

> If I'm counting bits correctly, that is Xen rejecting the use of the NX
> bit, which is suspicious.  Do you have the full Xen boot log on this
> box?  I wonder if we've some problem clobbing the XD-disable bit.


Yes, it was attached.
Is there any other Xen cmdline knob to enable more debug?

Olaf


pgpNwiMIVNxSZ.pgp
Description: Digitale Signatur von OpenPGP


Re: regression in recent pvops kernels, dom0 crashes early

2021-05-13 Thread Olaf Hering
Am Thu, 13 May 2021 13:11:10 +0100
schrieb Andrew Cooper :

> If I'm counting bits correctly, that is Xen rejecting the use of the NX
> bit, which is suspicious.

I tried 'dom0=pvh,debug':

...
(XEN) mcheck_poll: Machine check polling timer started.
(XEN) Running stub recovery selftests...
(XEN) Fixup #UD[]: 82d07fffe040 [82d07fffe040] -> 82d040394a17
(XEN) Fixup #GP[]: 82d07fffe041 [82d07fffe041] -> 82d040394a17
(XEN) Fixup #SS[]: 82d07fffe040 [82d07fffe040] -> 82d040394a17
(XEN) Fixup #BP[]: 82d07fffe041 [82d07fffe041] -> 82d040394a17
(XEN) HPET: 0 timers usable for broadcast (3 total)
(XEN) Warning: NX (Execute Disable) protection not active
(XEN) Dom0 has maximum 864 PIRQs
(XEN)
(XEN) 
(XEN) Panic on CPU 0:
(XEN) Presently, iommu must be enabled for PVH hardware domain
(XEN) 
...

The other logs have:
(XEN) Warning: NX (Execute Disable) protection not active

Olaf


pgpOfLWE00c84.pgp
Description: Digitale Signatur von OpenPGP


Re: regression in recent pvops kernels, dom0 crashes early

2021-05-13 Thread Andrew Cooper
On 13/05/2021 13:22, Olaf Hering wrote:
> Am Thu, 13 May 2021 13:11:10 +0100
> schrieb Andrew Cooper :
>
>> If I'm counting bits correctly, that is Xen rejecting the use of the NX
>> bit, which is suspicious.  Do you have the full Xen boot log on this
>> box?  I wonder if we've some problem clobbing the XD-disable bit.
>
> Yes, it was attached.
> Is there any other Xen cmdline knob to enable more debug?

Urgh sorry - I've not had enough coffee yet today.

Warning: NX (Execute Disable) protection not active

And this is an AMD box not an Intel box, so no XD-disable nonsense (that
I'm aware of).

So, the two options are:
1) This box legitimately doesn't have NX, and the dom0 kernel is buggy
for trying to use it.
2) This box does actually have NX, Xen has failed to turn it on, and
dom0 (through non CPUID means) thinks that NX is usable.

Can we first establish whether this box really does, or does not, have NX ?

~Andrew




Re: regression in recent pvops kernels, dom0 crashes early

2021-05-13 Thread Olaf Hering
Am Thu, 13 May 2021 13:29:32 +0100
schrieb Andrew Cooper :

> Can we first establish whether this box really does, or does not, have NX ?

According to lscpu of a native boot: no.

Architecture:  x86_64
CPU op-mode(s):32-bit, 64-bit
Byte Order:Little Endian
CPU(s):8
On-line CPU(s) list:   0-7
Thread(s) per core:1
Core(s) per socket:4
Socket(s): 2
NUMA node(s):  2
Vendor ID: AuthenticAMD
CPU family:16
Model: 2
Model name:Quad-Core AMD Opteron(tm) Processor 2356
Stepping:  3
CPU MHz:   2300.057
BogoMIPS:  4600.11
Virtualization:AMD-V
L1d cache: 64K
L1i cache: 64K
L2 cache:  512K
L3 cache:  2048K
NUMA node0 CPU(s): 0,2,4,6
NUMA node1 CPU(s): 1,3,5,7
Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall mmxex
t fxsr_opt pdpe1gb rdtscp lm 3dnowext 3dnow constant_tsc rep_good nopl 
nonstop_tsc cpuid extd_apicid pni monitor cx16 popcnt lahf_lm cmp_
legacy svm extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch osvw ibs 
hw_pstate vmmcall npt lbrv svm_lock

Olaf


pgpNJX0nb6CI8.pgp
Description: Digitale Signatur von OpenPGP


Re: regression in recent pvops kernels, dom0 crashes early

2021-05-13 Thread Olaf Hering
Am Thu, 13 May 2021 13:29:32 +0100
schrieb Andrew Cooper :

> Warning: NX (Execute Disable) protection not active

There was a knob in the BIOS, it was set to "Disabled" for some reason.
Once enabled, the flag is seen and the dom0 starts fine.

If Xen is booted with 'cpuid=no-nx', the dom0 crashes again.

Thanks for the help, Andrew.


Olaf


pgpWbJPZKkY1H.pgp
Description: Digitale Signatur von OpenPGP


Re: regression in recent pvops kernels, dom0 crashes early

2021-05-13 Thread Andrew Cooper
On 13/05/2021 14:00, Olaf Hering wrote:
> Am Thu, 13 May 2021 13:29:32 +0100
> schrieb Andrew Cooper :
>
>> Warning: NX (Execute Disable) protection not active
> There was a knob in the BIOS, it was set to "Disabled" for some reason.
> Once enabled, the flag is seen and the dom0 starts fine.
>
> If Xen is booted with 'cpuid=no-nx', the dom0 crashes again.
>
> Thanks for the help, Andrew.

Well - I wouldn't say we're quite done yet.

Clearly between sle12sp3 and sle12sp4 you've picked up a regression
where Linux decides to use NX despite its absence.

If NX is a mandatory feature now, then dom0 ought to error out cleanly
stating this fact.

~Andrew



[xen-unstable test] 161926: tolerable trouble: fail/pass/starved - PUSHED

2021-05-13 Thread osstest service owner
flight 161926 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/161926/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 161898
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 161898
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 161898
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 161898
 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeatfail  like 161898
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 161898
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 161898
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 161898
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 161898
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 161898
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 161898
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-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-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-dom0pvh-xl-amd  3 hosts-allocate   starved  n/a
 test-amd64-i386-qemut-rhel6hvm-amd  3 hosts-allocate   starved n/a
 test-amd64-amd64-xl-pvhv2-amd  3 hosts-allocate   starved  n/a
 test-amd64-amd64-qemuu-nested-amd  3 hosts-allocate   starved  n/a
 test-amd64-coresched-amd64-xl  3 hosts-allocate   starved  n/a
 test-amd64-coresched-i386-xl  3 hosts-allocate   starved  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  3 hosts-allocate   starved n/a

version targeted for testing:
 xen  43d4cc7d36503bcc3aa2aa6ebea2b7912808f254
baseline version:
 xen  982c89ed527bc5b0ffae5da9fd33f9d

[linux-linus test] 161922: regressions - FAIL

2021-05-13 Thread osstest service owner
flight 161922 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/161922/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-ws16-amd64  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-xsm7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 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-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-libvirt   7 xen-install  fail REGR. vs. 152332
 test-amd64-coresched-i386-xl  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-examine   6 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-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-qemut-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 
152332
 test-amd64-i386-xl7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-libvirt-xsm   7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-freebsd10-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-raw7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-pvshim 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-shadow 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-freebsd10-i386  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 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-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-arm64-arm64-examine  8 reboot   fail REGR. vs. 152332
 test-arm64-arm64-xl-credit2   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-libvirt-xsm  8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-xl-credit1   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-xl   8 xen-boot fail REGR. vs. 152332
 test-amd64-amd64-libvirt-vhd 13 guest-start  fail REGR. vs. 152332
 test-arm64-arm64-xl-xsm   8 xen-boot fail 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-thunderx  8 xen-boot fail REGR. vs. 152332
 test-amd64-amd64-xl-qcow213 guest-start  fail REGR. vs. 152332
 test-arm64-arm64-xl-seattle   8 xen-boot fail REGR. vs. 152332
 test-armhf-armhf-xl-vhd  13 guest-start  fail REGR. vs. 152332

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-qemuu-freebsd12-amd64 16 guest-saverestore fail in 161911 
pass in 161922
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass 
in 161911

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-xl-qemut-ws16-amd64 19 guest-stopfail 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-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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-check 

Re: [PATCH RFCv2 09/15] xen/arm32: mm: Re-implement setup_xenheap_mappings() using map_pages_to_xen()

2021-05-13 Thread Julien Grall

Hi Stefano,

On 12/05/2021 23:07, Stefano Stabellini wrote:

On Sun, 25 Apr 2021, Julien Grall wrote:

From: Julien Grall 

Now that map_pages_to_xen() has been extended to support 2MB mappings,
we can replace the create_mappings() call by map_pages_to_xen() call.

Signed-off-by: Julien Grall 

---
 Changes in v2:
 - New patch

 TODOs:
 - add support for contiguous mapping
---
  xen/arch/arm/mm.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 5c17cafff847..19ecf73542ce 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -806,7 +806,12 @@ void mmu_init_secondary_cpu(void)
  void __init setup_xenheap_mappings(unsigned long base_mfn,
 unsigned long nr_mfns)
  {
-create_mappings(xen_second, XENHEAP_VIRT_START, base_mfn, nr_mfns, MB(32));
+int rc;
+
+rc = map_pages_to_xen(XENHEAP_VIRT_START, base_mfn, nr_mfns,
+  PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
+if ( rc )
+panic("Unable to setup the xenheap mappings.\n");
  
  /* Record where the xenheap is, for translation routines. */

  xenheap_virt_end = XENHEAP_VIRT_START + nr_mfns * PAGE_SIZE;


I get the following build error:

mm.c: In function ‘setup_xenheap_mappings’:
mm.c:811:47: error: incompatible type for argument 2 of ‘map_pages_to_xen’
  rc = map_pages_to_xen(XENHEAP_VIRT_START, base_mfn, nr_mfns,
^~~~
In file included from mm.c:24:0:
/local/repos/xen-upstream/xen/include/xen/mm.h:89:5: note: expected ‘mfn_t {aka 
struct }’ but argument is of type ‘long unsigned int’
  int map_pages_to_xen(
  ^~~~

I think base_mfn needs to be converted to mfn_t


You are right. I think my scripts are build testing arm32 with debug=n. 
I will fix it and respin.


Cheers,

--
Julien Grall



Re: [PATCH RFCv2 10/15] xen/arm: mm: Allocate xen page tables in domheap rather than xenheap

2021-05-13 Thread Julien Grall

Hi Stefano,

On 12/05/2021 23:44, Stefano Stabellini wrote:

On Sun, 25 Apr 2021, Julien Grall wrote:

From: Julien Grall 

xen_{un,}map_table() already uses the helper to map/unmap pages
on-demand (note this is currently a NOP on arm64). So switching to
domheap don't have any disavantage.

But this as the benefit:
 - to keep the page tables unmapped if an arch decided to do so
 - reduce xenheap use on arm32 which can be pretty small

Signed-off-by: Julien Grall 


Thanks for the patch. It looks OK but let me ask a couple of questions
to clarify my doubts.

This change should have no impact to arm64, right?

For arm32, I wonder why we were using map_domain_page before in
xen_map_table: it wasn't necessary, was it? In fact, one could even say
that it was wrong?
In xen_map_table() we need to be able to map pages from Xen binary, 
xenheap... On arm64, we would be able to use mfn_to_virt() because 
everything is mapped in Xen. But that's not the case on arm32. So we 
need a way to map anything easily.


The only difference between xenheap and domheap are the former is always 
mapped while the latter may not be. So one can also view a xenheap page 
as a glorified domheap.


I also don't really want to create yet another interface to map pages 
(we have vmap(), map_domain_page(), map_domain_global_page()...). So, 
when I implemented xen_map_table() a couple of years ago, I came to the 
conclusion that this is a convenient (ab)use of the interface.


Cheers,

--
Julien Grall



Re: Discussion of Xenheap problems on AArch64

2021-05-13 Thread Julien Grall




On 11/05/2021 02:11, Henry Wang wrote:

Hi Julien,


Hi Henry,




From: Julien Grall 
Hi Henry,

On 07/05/2021 05:06, Henry Wang wrote:

From: Julien Grall 
On 28/04/2021 10:28, Henry Wang wrote:

[...]


when I continue booting Xen, I got following error log:

(XEN) CPU:0
(XEN) PC: 002b5a5c alloc_boot_pages+0x94/0x98
(XEN) LR: 002ca3bc
(XEN) SP: 002ffde0
(XEN) CPSR:   63c9 MODE:64-bit EL2h (Hypervisor, handler)
(XEN)
(XEN)   VTCR_EL2: 8000
(XEN)  VTTBR_EL2: 
(XEN)
(XEN)  SCTLR_EL2: 30cd183d
(XEN)HCR_EL2: 0038
(XEN)  TTBR0_EL2: 8413c000
(XEN)
(XEN)ESR_EL2: f201
(XEN)  HPFAR_EL2: 
(XEN)FAR_EL2: 
(XEN)
(XEN) Xen call trace:
(XEN)[<002b5a5c>] alloc_boot_pages+0x94/0x98 (PC)
(XEN)[<002ca3bc>] setup_frametable_mappings+0xa4/0x108

(LR)

(XEN)[<002ca3bc>] setup_frametable_mappings+0xa4/0x108
(XEN)[<002cb988>] start_xen+0x344/0xbcc
(XEN)[<002001c0>]

arm64/head.o#primary_switched+0x10/0x30

(XEN)
(XEN) 
(XEN) Panic on CPU 0:
(XEN) Xen BUG at page_alloc.c:432
(XEN) 


This is happening without my patch series applied, right? If so, what
happen if you apply it?


No, I am afraid this is with your patch series applied, and that is why I
am a little bit confused about the error log...


You are hitting the BUG() at the end of alloc_boot_pages(). This is hit 
because the boot allocator couldn't allocate memory for your request.


Would you be able to apply the following diff and paste the output here?

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index ace6333c18ea..dbb736fdb275 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -329,6 +329,8 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe)
 if ( pe <= ps )
 return;

+printk("%s: ps %"PRI_paddr" pe %"PRI_paddr"\n", __func__, ps, pe);
+
 first_valid_mfn = mfn_min(maddr_to_mfn(ps), first_valid_mfn);

 bootmem_region_add(ps >> PAGE_SHIFT, pe >> PAGE_SHIFT);
@@ -395,6 +397,8 @@ mfn_t __init alloc_boot_pages(unsigned long nr_pfns, 
unsigned long pfn_align)

 unsigned long pg, _e;
 unsigned int i = nr_bootmem_regions;

+printk("%s: nr_pfns %lu pfn_align %lu\n", __func__, nr_pfns, 
pfn_align);

+
 BUG_ON(!nr_bootmem_regions);

 while ( i-- )

Cheers,

--
Julien Grall



[qemu-mainline test] 161924: regressions - FAIL

2021-05-13 Thread osstest service owner
flight 161924 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/161924/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-freebsd11-amd64 16 guest-saverestore fail REGR. vs. 
152631
 test-amd64-amd64-qemuu-freebsd12-amd64 16 guest-saverestore fail REGR. vs. 
152631
 test-amd64-i386-freebsd10-i386 16 guest-saverestore  fail REGR. vs. 152631
 test-amd64-i386-freebsd10-amd64 16 guest-saverestore fail REGR. vs. 152631
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 15 guest-saverestore fail 
REGR. vs. 152631
 test-amd64-i386-xl-qemuu-debianhvm-amd64 15 guest-saverestore fail REGR. vs. 
152631
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 15 guest-saverestore fail REGR. 
vs. 152631
 test-amd64-i386-xl-qemuu-ovmf-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 15 guest-saverestore fail REGR. vs. 
152631
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 15 guest-saverestore fail 
REGR. vs. 152631
 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-amd64-xl-qemuu-ovmf-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 15 guest-saverestore fail REGR. 
vs. 152631
 test-amd64-amd64-qemuu-nested-intel 20 debian-hvm-install/l1/l2 fail REGR. vs. 
152631
 test-amd64-i386-xl-qemuu-win7-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-i386-xl-qemuu-ws16-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-amd64-xl-qemuu-ws16-amd64 15 guest-saverestore fail REGR. vs. 152631

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

Regressed XSA-286, was [xen-unstable test] 161917: regressions - FAIL

2021-05-13 Thread Andrew Cooper
On 13/05/2021 04:56, osstest service owner wrote:
> flight 161917 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/161917/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-arm64-arm64-examine  8 reboot   fail REGR. vs. 
> 161898
>  test-arm64-arm64-xl-thunderx  8 xen-boot fail REGR. vs. 
> 161898
>  test-arm64-arm64-xl-credit1   8 xen-boot fail REGR. vs. 
> 161898
>  test-arm64-arm64-xl-credit2   8 xen-boot fail REGR. vs. 
> 161898
>  test-arm64-arm64-xl   8 xen-boot fail REGR. vs. 
> 161898

I reported these on IRC, and Julien/Stefano have already committed a fix.

> Tests which are failing intermittently (not blocking):
>  test-xtf-amd64-amd64-3 92 xtf/test-pv32pae-xsa-286 fail in 161909 pass in 
> 161917

While noticing the ARM issue above, I also spotted this one by chance. 
There are two issues.

First, I have reverted bed7e6cad30 and edcfce55917.  The XTF test is
correct, and they really do reintroduce XSA-286.  It is a miracle of
timing that we don't need an XSA/CVE against Xen 4.15.

Given that I was unhappy with the changes in the first place, I don't
particularly want to see an attempt to resurrect them.  I did not find
the claim that they were a perf improvement in the first place very
convincing, and the XTF test demonstrates that the reasoning about their
safety was incorrect.


Second, the unexplained OSSTest behaviour.

When I repro'd this on pinot1, test-pv32pae-xsa-286 failing was totally
deterministic and repeatable (I tried 100 times because the test is a
fraction of a second).

>From the log trawling which Ian already did, the first recorded failure
was flight 160912 on April 11th.  All failures (12, but this number is a
few flights old now) were on pinot*.

What would be interesting to see is whether there have been any passes
on pinot since 160912.

I can't see any reason why the test would be reliable for me, but
unreliable for OSSTest, so I'm wondering whether it is actually
reliable, and something is wrong with the stickiness heuristic.

~Andrew




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

2021-05-13 Thread osstest service owner
flight 161937 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/161937/

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  cb199cc7de987cfda4659fccf51059f210f6ad34
baseline version:
 xen  43d4cc7d36503bcc3aa2aa6ebea2b7912808f254

Last test of basis   161923  2021-05-12 21:01:32 Z0 days
Testing same since   161937  2021-05-13 18:01:32 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 

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
   43d4cc7d36..cb199cc7de  cb199cc7de987cfda4659fccf51059f210f6ad34 -> smoke



[linux-linus test] 161936: regressions - FAIL

2021-05-13 Thread osstest service owner
flight 161936 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/161936/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-ws16-amd64  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-xsm7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 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-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332
 test-amd64-i386-libvirt   7 xen-install  fail REGR. vs. 152332
 test-amd64-coresched-i386-xl  7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-examine   6 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-qemuu-debianhvm-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-qemut-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-install   fail 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-qemuu-rhel6hvm-amd  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-freebsd10-amd64  7 xen-install   fail REGR. vs. 152332
 test-amd64-i386-xl-raw7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-pvshim 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-xl-shadow 7 xen-install  fail REGR. vs. 152332
 test-amd64-i386-freebsd10-i386  7 xen-installfail REGR. vs. 152332
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 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-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-arm64-arm64-examine  8 reboot   fail REGR. vs. 152332
 test-arm64-arm64-xl-credit2   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-libvirt-xsm  8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-xl-credit1   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-xl   8 xen-boot fail REGR. vs. 152332
 test-amd64-amd64-libvirt-vhd 13 guest-start  fail REGR. vs. 152332
 test-arm64-arm64-xl-xsm   8 xen-boot 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-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 152332
 test-arm64-arm64-xl-thunderx  8 xen-boot fail REGR. vs. 152332
 test-amd64-amd64-xl-qcow213 guest-start  fail REGR. vs. 152332
 test-arm64-arm64-xl-seattle   8 xen-boot fail REGR. vs. 152332
 test-armhf-armhf-xl-vhd  13 guest-start  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-xl-qemut-ws16-amd64 19 guest-stopfail 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-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-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-multivcpu 15 migrate-support-check 

Re: [PATCH RFCv2 10/15] xen/arm: mm: Allocate xen page tables in domheap rather than xenheap

2021-05-13 Thread Stefano Stabellini
On Thu, 13 May 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 12/05/2021 23:44, Stefano Stabellini wrote:
> > On Sun, 25 Apr 2021, Julien Grall wrote:
> > > From: Julien Grall 
> > > 
> > > xen_{un,}map_table() already uses the helper to map/unmap pages
> > > on-demand (note this is currently a NOP on arm64). So switching to
> > > domheap don't have any disavantage.
> > > 
> > > But this as the benefit:
> > >  - to keep the page tables unmapped if an arch decided to do so
> > >  - reduce xenheap use on arm32 which can be pretty small
> > > 
> > > Signed-off-by: Julien Grall 
> > 
> > Thanks for the patch. It looks OK but let me ask a couple of questions
> > to clarify my doubts.
> > 
> > This change should have no impact to arm64, right?
> > 
> > For arm32, I wonder why we were using map_domain_page before in
> > xen_map_table: it wasn't necessary, was it? In fact, one could even say
> > that it was wrong?
> In xen_map_table() we need to be able to map pages from Xen binary, xenheap...
> On arm64, we would be able to use mfn_to_virt() because everything is mapped
> in Xen. But that's not the case on arm32. So we need a way to map anything
> easily.
> 
> The only difference between xenheap and domheap are the former is always
> mapped while the latter may not be. So one can also view a xenheap page as a
> glorified domheap.
> 
> I also don't really want to create yet another interface to map pages (we have
> vmap(), map_domain_page(), map_domain_global_page()...). So, when I
> implemented xen_map_table() a couple of years ago, I came to the conclusion
> that this is a convenient (ab)use of the interface.

Got it. Repeating to check if I see the full picture. On ARM64 there are
no changes. On ARM32, at runtime there are no changes mapping/unmapping
pages because xen_map_table is already mapping all pages using domheap,
even xenheap pages are mapped as domheap; so this patch makes no
difference in mapping/unmapping, correct?

The only difference is that on arm32 we are using domheap to allocate
the pages, which is a different (larger) pool.



Re: [PATCH RFCv2 08/15] xen/arm32: mm: Check if the virtual address is shared before updating it

2021-05-13 Thread Stefano Stabellini
On Wed, 12 May 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 12/05/2021 23:00, Stefano Stabellini wrote:
> > On Sun, 25 Apr 2021, Julien Grall wrote:
> > > From: Julien Grall 
> > > 
> > > Only the first 2GB of the virtual address space is shared between all
> > > the page-tables on Arm32.
> > > 
> > > There is a long outstanding TODO in xen_pt_update() stating that the
> > > function is can only work with shared mapping. Nobody has ever called
> > ^ remove
> > 
> > > the function with private mapping, however as we add more callers
> > > there is a risk to mess things up.
> > > 
> > > Introduce a new define to mark the ened of the shared mappings and use
> >   ^end
> > 
> > > it in xen_pt_update() to verify if the address is correct.
> > > 
> > > Note that on Arm64, all the mappings are shared. Some compiler may
> > > complain about an always true check, so the new define is not introduced
> > > for arm64 and the code is protected with an #ifdef.
> >   On arm64 we could maybe define SHARED_VIRT_END to an arbitrarely large
> > value, such as:
> > 
> > #define SHARED_VIRT_END (1UL<<48)
> > 
> > or:
> > 
> > #define SHARED_VIRT_END DIRECTMAP_VIRT_END
> > 
> > ?
> 
> I thought about it but I didn't want to define to a random value... I felt not
> define it was better.

Yeah, I see your point: any restrictions in addressing (e.g. 48bits)
are physical address restrictions. Here we are talking about virtual
address restriction, and I don't think there are actually any
restrictions there?  We could validly map something at
0x___. So even (1<<48) which makes sense at the physical
level, doesn't make sense in terms of virtual addresses.


> > > Signed-off-by: Julien Grall 
> > > 
> > > ---
> > >  Changes in v2:
> > >  - New patch
> > > ---
> > >   xen/arch/arm/mm.c| 11 +--
> > >   xen/include/asm-arm/config.h |  4 
> > >   2 files changed, 13 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > > index 8fac24d80086..5c17cafff847 100644
> > > --- a/xen/arch/arm/mm.c
> > > +++ b/xen/arch/arm/mm.c
> > > @@ -1275,11 +1275,18 @@ static int xen_pt_update(unsigned long virt,
> > >* For arm32, page-tables are different on each CPUs. Yet, they
> > > share
> > >* some common mappings. It is assumed that only common mappings
> > >* will be modified with this function.
> > > - *
> > > - * XXX: Add a check.
> > >*/
> > >   const mfn_t root = virt_to_mfn(THIS_CPU_PGTABLE);
> > >   +#ifdef SHARED_VIRT_END
> > > +if ( virt > SHARED_VIRT_END ||
> > > + (SHARED_VIRT_END - virt) < nr_mfns )
> > 
> > The following would be sufficient, right?
> > 
> >  if ( virt + nr_mfns > SHARED_VIRT_END )
> 
> This would not protect against an overflow. So I think it is best if we keep
> my version.

But there can be no overflow with the way SHARED_VIRT_END is defined.
Even if SHARED_VIRT_END was defined at (1<<48) there can be no overflow.
Only if we defined SHARED_VIRT_END as 0x___ we would
have an overflow, but you wrote above that your preference is not to do
that.



Re: [PATCH RFCv2 02/15] xen/arm: lpae: Use the generic helpers to defined the Xen PT helpers

2021-05-13 Thread Stefano Stabellini
On Wed, 12 May 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 12/05/2021 22:30, Stefano Stabellini wrote:
> > On Wed, 12 May 2021, Julien Grall wrote:
> > > > > +#define LPAE_SHIFT  LPAE_SHIFT_GS(PAGE_SHIFT)
> > > > > +#define LPAE_ENTRIESLPAE_ENTRIES_GS(PAGE_SHIFT)
> > > > > +#define LPAE_ENTRY_MASK LPAE_ENTRY_MASK_GS(PAGE_SHIFT)
> > > > > 
> > > > > +#define LEVEL_SHIFT(lvl)LEVEL_SHIFT_GS(PAGE_SHIFT, lvl)
> > > > > +#define LEVEL_ORDER(lvl)LEVEL_ORDER_GS(PAGE_SHIFT, lvl)
> > > > > +#define LEVEL_SIZE(lvl) LEVEL_SIZE_GS(PAGE_SHIFT, lvl)
> > > > > +#define LEVEL_MASK(lvl) (~(LEVEL_SIZE(lvl) - 1))
> > > > 
> > > > I would avoid adding these 4 macros. It would be OK if they were just
> > > > used within this file but lpae.h is a header: they could end up be used
> > > > anywhere in the xen/ code and they have a very generic name. My
> > > > suggestion would be to skip them and just do:
> > > 
> > > Those macros will be used in follow-up patches. They are pretty useful to
> > > avoid introduce static array with the different information for each
> > > level.
> > > 
> > > Would prefix them with XEN_ be better?
> > 
> > Maybe. The concern I have is that there are multiple page granularities
> > (4kb, 16kb, etc) and multiple page sizes (4kb, 2mb, etc). If I just see
> > LEVEL_ORDER it is not immediately obvious what granularity and what size
> > we are talking about.
> 
> I am a bit puzzled with your answer. AFAIU, you are happy with the existing
> macros (THIRD_*, SECOND_*) but not with the new macros.
>
> In reality, there is no difference because THIRD_* doesn't tell you the exact
> size but only "this is a level 3 mapping".
> 
> So can you clarify what you are after? IOW is it reworking the current naming
> scheme?

You are right -- there is no real difference between THIRD_*, SECOND_*
and LEVEL_*.

The original reason for my comments is that I hadn't read the following
patches, and the definition of LEVEL_* macros is simple, they could be
open coded. It looked like they were only going to be used to make the
definition of THIRD_*, SECOND_* a bit easier. So, at first, I was
wondering if they were needed at all.

Secondly, I realized that they were going to be used in *.c files by
other patches. That's why they are there. But I started thinking whether
we should find a way to make it a bit clearer that they are for Xen
pages, currently at 4KB granularity. THIRD_*, SECOND_*, etc. are already
generic names which don't convey the granularity or whether they are Xen
pages at all. But LEVEL_* seem even more generic.

As I mentioned, I don't have any good suggestions for changes to make
here, so unless you can come up with a good idea let's keep it as is.



Re: [PATCH RFCv2 11/15] xen/arm: mm: Allow page-table allocation from the boot allocator

2021-05-13 Thread Stefano Stabellini
On Sun, 25 Apr 2021, Julien Grall wrote:
> From: Julien Grall 
> 
> At the moment, page-table can only be allocated from domheap. This means
> it is not possible to create mapping in the page-tables via
> map_pages_to_xen() if page-table needs to be allocated.
> 
> In order to avoid open-coding page-tables update in early boot, we need
> to be able to allocate page-tables much earlier. Thankfully, we have the
> boot allocator for those cases.
> 
> create_xen_table() is updated to cater early boot allocation by using
> alloc_boot_pages().
> 
> Note, this is not sufficient to bootstrap the page-tables (i.e mapping
> before any memory is actually mapped). This will be addressed
> separately.
> 
> Signed-off-by: Julien Grall 
> Signed-off-by: Julien Grall 

Reviewed-by: Stefano Stabellini 


> ---
> Changes in v2:
> - New patch
> ---
>  xen/arch/arm/mm.c | 20 ++--
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index ae5a07ea956b..d090fdfd5994 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1011,19 +1011,27 @@ static void xen_unmap_table(const lpae_t *table)
>  
>  static int create_xen_table(lpae_t *entry)
>  {
> -struct page_info *pg;
> +mfn_t mfn;
>  void *p;
>  lpae_t pte;
>  
> -pg = alloc_domheap_page(NULL, 0);
> -if ( pg == NULL )
> -return -ENOMEM;
> +if ( system_state != SYS_STATE_early_boot )
> +{
> +struct page_info *pg = alloc_domheap_page(NULL, 0);
> +
> +if ( pg == NULL )
> +return -ENOMEM;
> +
> +mfn = page_to_mfn(pg);
> +}
> +else
> +mfn = alloc_boot_pages(1, 1);
>  
> -p = xen_map_table(page_to_mfn(pg));
> +p = xen_map_table(mfn);
>  clear_page(p);
>  xen_unmap_table(p);
>  
> -pte = mfn_to_xen_entry(page_to_mfn(pg), MT_NORMAL);
> +pte = mfn_to_xen_entry(mfn, MT_NORMAL);
>  pte.pt.table = 1;
>  write_pte(entry, pte);
>  
> -- 
> 2.17.1
> 



Re: [PATCH RFCv2 08/15] xen/arm32: mm: Check if the virtual address is shared before updating it

2021-05-13 Thread Julien Grall
On Thu, 13 May 2021, 23:32 Stefano Stabellini, 
wrote:

> On Wed, 12 May 2021, Julien Grall wrote:
> > Hi Stefano,
> >
> > On 12/05/2021 23:00, Stefano Stabellini wrote:
> > > On Sun, 25 Apr 2021, Julien Grall wrote:
> > > > From: Julien Grall 
> > > >
> > > > Only the first 2GB of the virtual address space is shared between all
> > > > the page-tables on Arm32.
> > > >
> > > > There is a long outstanding TODO in xen_pt_update() stating that the
> > > > function is can only work with shared mapping. Nobody has ever called
> > > ^ remove
> > >
> > > > the function with private mapping, however as we add more callers
> > > > there is a risk to mess things up.
> > > >
> > > > Introduce a new define to mark the ened of the shared mappings and
> use
> > >   ^end
> > >
> > > > it in xen_pt_update() to verify if the address is correct.
> > > >
> > > > Note that on Arm64, all the mappings are shared. Some compiler may
> > > > complain about an always true check, so the new define is not
> introduced
> > > > for arm64 and the code is protected with an #ifdef.
> > >   On arm64 we could maybe define SHARED_VIRT_END to an arbitrarely
> large
> > > value, such as:
> > >
> > > #define SHARED_VIRT_END (1UL<<48)
> > >
> > > or:
> > >
> > > #define SHARED_VIRT_END DIRECTMAP_VIRT_END
> > >
> > > ?
> >
> > I thought about it but I didn't want to define to a random value... I
> felt not
> > define it was better.
>
> Yeah, I see your point: any restrictions in addressing (e.g. 48bits)
> are physical address restrictions. Here we are talking about virtual
> address restriction, and I don't think there are actually any
> restrictions there?  We could validly map something at
> 0x___. So even (1<<48) which makes sense at the physical
> level, doesn't make sense in terms of virtual addresses.
>

The limit for the virtual address is 2^64.


>
> > > > Signed-off-by: Julien Grall 
> > > >
> > > > ---
> > > >  Changes in v2:
> > > >  - New patch
> > > > ---
> > > >   xen/arch/arm/mm.c| 11 +--
> > > >   xen/include/asm-arm/config.h |  4 
> > > >   2 files changed, 13 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > > > index 8fac24d80086..5c17cafff847 100644
> > > > --- a/xen/arch/arm/mm.c
> > > > +++ b/xen/arch/arm/mm.c
> > > > @@ -1275,11 +1275,18 @@ static int xen_pt_update(unsigned long virt,
> > > >* For arm32, page-tables are different on each CPUs. Yet, they
> > > > share
> > > >* some common mappings. It is assumed that only common
> mappings
> > > >* will be modified with this function.
> > > > - *
> > > > - * XXX: Add a check.
> > > >*/
> > > >   const mfn_t root = virt_to_mfn(THIS_CPU_PGTABLE);
> > > >   +#ifdef SHARED_VIRT_END
> > > > +if ( virt > SHARED_VIRT_END ||
> > > > + (SHARED_VIRT_END - virt) < nr_mfns )
> > >
> > > The following would be sufficient, right?
> > >
> > >  if ( virt + nr_mfns > SHARED_VIRT_END )
> >
> > This would not protect against an overflow. So I think it is best if we
> keep
> > my version.
>
> But there can be no overflow with the way SHARED_VIRT_END is defined.

Even if SHARED_VIRT_END was defined at (1<<48) there can be no overflow.
> Only if we defined SHARED_VIRT_END as 0x___ we would
> have an overflow, but you wrote above that your preference is not to do
> that.
>

You can have an overflow regardless the value of SHARED_VIRT_END.

Imagine virt = 2^64 - 1 and nr_mfs = 1. The addition would result to 0.

As a consequence the check would pass when it should not.

One can argue that the caller will always provide sane values. However
given the simplicity of the check, this is not worth the trouble if a
caller is buggy.

Now, the problem with SHARED_VIRT_END equals to 2^64 - 1 is not the
overflow but the compiler that may throw an error/warning for always true
check. Hence the reason of not defining SHARED_VIRT_END on arm64.

Cheers,


[PATCH v2 1/4] usb: early: Avoid using DbC if already enabled

2021-05-13 Thread Connor Davis
Check if the debug capability is enabled in early_xdbc_parse_parameter,
and if it is, return with an error. This avoids collisions with whatever
enabled the DbC prior to linux starting.

Signed-off-by: Connor Davis 
---
 drivers/usb/early/xhci-dbc.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
index be4ecbabdd58..ca67fddc2d36 100644
--- a/drivers/usb/early/xhci-dbc.c
+++ b/drivers/usb/early/xhci-dbc.c
@@ -642,6 +642,16 @@ int __init early_xdbc_parse_parameter(char *s)
}
xdbc.xdbc_reg = (struct xdbc_regs __iomem *)(xdbc.xhci_base + offset);
 
+   if (readl(&xdbc.xdbc_reg->control) & CTRL_DBC_ENABLE) {
+   pr_notice("xhci debug capability already in use\n");
+   early_iounmap(xdbc.xhci_base, xdbc.xhci_length);
+   xdbc.xdbc_reg = NULL;
+   xdbc.xhci_base = NULL;
+   xdbc.xhci_length = 0;
+
+   return -ENODEV;
+   }
+
return 0;
 }
 
-- 
2.31.1




[PATCH v2 0/4] Support xen-driven USB3 debug capability

2021-05-13 Thread Connor Davis
Hi all,

This goal of this series is to allow the USB3 debug capability (DbC) to be
safely used by xen while linux runs as dom0.

The first patch prevents the early DbC driver from using an
already-running DbC.

The second exports xen_dbgp_reset_prep and xen_dbgp_external_startup
functions when CONFIG_XEN_DOM0 is enabled so they may be used by the
xHCI driver.

The third ensures that xen_dbgp_reset_prep/xen_dbgp_external_startup
return consistent values in failure cases. This inconsistency illustrated
another issue: dbgp_reset_prep returned the value of xen_dbgp_reset_prep
if it was nonzero, but callers of dbgp_reset_prep interpret nonzero
as "keep using the debug port" and would eventually (needlessly) call
dbgp_external_startup. Patch three _should_ fix this issue, but
I don't have any EHCI hardware available to test unfortunately.

The last uses the xen_dbgp_* functions to notify xen of unsafe periods
(e.g. reset and D3hot transition).

Thanks,
Connor

--
Changes since v1:
 - Added patch for dbgp return value fixes
 - Return -EPERM when !xen_initial_domain() in xen_dbgp_op
 - Moved #ifdef-ary out of xhci.c into xhci-dbgcap.h

--
Connor Davis (4):
  usb: early: Avoid using DbC if already enabled
  xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled
  usb: dbgp: Fix return values for reset prep and startup
  usb: xhci: Notify xen when DbC is unsafe to use

 drivers/usb/early/ehci-dbgp.c  |  9 ---
 drivers/usb/early/xhci-dbc.c   | 10 
 drivers/usb/host/xhci-dbgcap.h | 19 ++
 drivers/usb/host/xhci.c| 47 ++
 drivers/usb/host/xhci.h|  1 +
 drivers/xen/dbgp.c |  4 +--
 include/linux/usb/ehci-dbgp.h  | 14 ++
 7 files changed, 94 insertions(+), 10 deletions(-)


base-commit: 88b06399c9c766c283e070b022b5ceafa4f63f19
-- 
2.31.1




[PATCH v2 2/4] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled

2021-05-13 Thread Connor Davis
Export xen_dbgp_reset_prep and xen_dbgp_external_startup
when CONFIG_XEN_DOM0 is defined. This allows use of these symbols
even if CONFIG_EARLY_PRINK_DBGP is defined.

Signed-off-by: Connor Davis 
Acked-by: Juergen Gross 
---
 drivers/xen/dbgp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/dbgp.c b/drivers/xen/dbgp.c
index cfb5de31d860..fef32dd1a5dc 100644
--- a/drivers/xen/dbgp.c
+++ b/drivers/xen/dbgp.c
@@ -44,7 +44,7 @@ int xen_dbgp_external_startup(struct usb_hcd *hcd)
return xen_dbgp_op(hcd, PHYSDEVOP_DBGP_RESET_DONE);
 }
 
-#ifndef CONFIG_EARLY_PRINTK_DBGP
+#if defined(CONFIG_XEN_DOM0) || !defined(CONFIG_EARLY_PRINTK_DBGP)
 #include 
 EXPORT_SYMBOL_GPL(xen_dbgp_reset_prep);
 EXPORT_SYMBOL_GPL(xen_dbgp_external_startup);
-- 
2.31.1




[PATCH v2 3/4] usb: dbgp: Fix return values for reset prep and startup

2021-05-13 Thread Connor Davis
Callers of dbgp_reset_prep treat a 0 return value as "stop using
the debug port", which means they don't make any subsequent calls to
dbgp_reset_prep or dbgp_external_startup.

To ensure the callers' interpretation is correct, first return -EPERM
from xen_dbgp_op if !xen_initial_domain(). This ensures that
both xen_dbgp_reset_prep and xen_dbgp_external_startup return 0
iff the PHYSDEVOP_DBGP_RESET_{PREPARE,DONE} hypercalls succeed. Also
update xen_dbgp_reset_prep and xen_dbgp_external_startup to return
-EPERM when !CONFIG_XEN_DOM0 for consistency.

Next, return nonzero from dbgp_reset_prep if xen_dbgp_reset_prep returns
0. The nonzero value ensures that callers of dbgp_reset_prep will
subsequently call dbgp_external_startup when it is safe to do so.

Also invert the return values from dbgp_external_startup for
consistency with dbgp_reset_prep (this inversion has no functional
change since no callers actually check the value).

Signed-off-by: Connor Davis 
---
 drivers/usb/early/ehci-dbgp.c |  9 ++---
 drivers/xen/dbgp.c|  2 +-
 include/linux/usb/ehci-dbgp.h | 14 +-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
index 45b42d8f6453..ff993d330c01 100644
--- a/drivers/usb/early/ehci-dbgp.c
+++ b/drivers/usb/early/ehci-dbgp.c
@@ -970,8 +970,8 @@ int dbgp_reset_prep(struct usb_hcd *hcd)
int ret = xen_dbgp_reset_prep(hcd);
u32 ctrl;
 
-   if (ret)
-   return ret;
+   if (!ret)
+   return 1;
 
dbgp_not_safe = 1;
if (!ehci_debug)
@@ -995,7 +995,10 @@ EXPORT_SYMBOL_GPL(dbgp_reset_prep);
 
 int dbgp_external_startup(struct usb_hcd *hcd)
 {
-   return xen_dbgp_external_startup(hcd) ?: _dbgp_external_startup();
+   if (!xen_dbgp_external_startup(hcd))
+   return 1;
+
+   return !_dbgp_external_startup();
 }
 EXPORT_SYMBOL_GPL(dbgp_external_startup);
 #endif /* USB */
diff --git a/drivers/xen/dbgp.c b/drivers/xen/dbgp.c
index fef32dd1a5dc..d54f98380e6f 100644
--- a/drivers/xen/dbgp.c
+++ b/drivers/xen/dbgp.c
@@ -15,7 +15,7 @@ static int xen_dbgp_op(struct usb_hcd *hcd, int op)
struct physdev_dbgp_op dbgp;
 
if (!xen_initial_domain())
-   return 0;
+   return -EPERM;
 
dbgp.op = op;
 
diff --git a/include/linux/usb/ehci-dbgp.h b/include/linux/usb/ehci-dbgp.h
index 62ab3805172d..c0e98557efc0 100644
--- a/include/linux/usb/ehci-dbgp.h
+++ b/include/linux/usb/ehci-dbgp.h
@@ -56,28 +56,32 @@ extern int xen_dbgp_external_startup(struct usb_hcd *);
 #else
 static inline int xen_dbgp_reset_prep(struct usb_hcd *hcd)
 {
-   return 1; /* Shouldn't this be 0? */
+   return -EPERM;
 }
 
 static inline int xen_dbgp_external_startup(struct usb_hcd *hcd)
 {
-   return -1;
+   return -EPERM;
 }
 #endif
 
 #ifdef CONFIG_EARLY_PRINTK_DBGP
-/* Call backs from ehci host driver to ehci debug driver */
+/*
+ * Call backs from ehci host driver to ehci debug driver.
+ * Returns 0 if the debug port should stopped being used,
+ * nonzero otherwise.
+ */
 extern int dbgp_external_startup(struct usb_hcd *);
 extern int dbgp_reset_prep(struct usb_hcd *);
 #else
 static inline int dbgp_reset_prep(struct usb_hcd *hcd)
 {
-   return xen_dbgp_reset_prep(hcd);
+   return !xen_dbgp_reset_prep(hcd);
 }
 
 static inline int dbgp_external_startup(struct usb_hcd *hcd)
 {
-   return xen_dbgp_external_startup(hcd);
+   return !xen_dbgp_external_startup(hcd);
 }
 #endif
 
-- 
2.31.1




Re: [PATCH RFCv2 08/15] xen/arm32: mm: Check if the virtual address is shared before updating it

2021-05-13 Thread Stefano Stabellini
On Thu, 13 May 2021, Julien Grall wrote:
> On Thu, 13 May 2021, 23:32 Stefano Stabellini,  wrote:
>   On Wed, 12 May 2021, Julien Grall wrote:
>   > Hi Stefano,
>   >
>   > On 12/05/2021 23:00, Stefano Stabellini wrote:
>   > > On Sun, 25 Apr 2021, Julien Grall wrote:
>   > > > From: Julien Grall 
>   > > >
>   > > > Only the first 2GB of the virtual address space is shared between 
> all
>   > > > the page-tables on Arm32.
>   > > >
>   > > > There is a long outstanding TODO in xen_pt_update() stating that 
> the
>   > > > function is can only work with shared mapping. Nobody has ever 
> called
>   > >             ^ remove
>   > >
>   > > > the function with private mapping, however as we add more callers
>   > > > there is a risk to mess things up.
>   > > >
>   > > > Introduce a new define to mark the ened of the shared mappings 
> and use
>   > >                                       ^end
>   > >
>   > > > it in xen_pt_update() to verify if the address is correct.
>   > > >
>   > > > Note that on Arm64, all the mappings are shared. Some compiler may
>   > > > complain about an always true check, so the new define is not 
> introduced
>   > > > for arm64 and the code is protected with an #ifdef.
>   > >   On arm64 we could maybe define SHARED_VIRT_END to an arbitrarely 
> large
>   > > value, such as:
>   > >
>   > > #define SHARED_VIRT_END (1UL<<48)
>   > >
>   > > or:
>   > >
>   > > #define SHARED_VIRT_END DIRECTMAP_VIRT_END
>   > >
>   > > ?
>   >
>   > I thought about it but I didn't want to define to a random value... I 
> felt not
>   > define it was better.
> 
>   Yeah, I see your point: any restrictions in addressing (e.g. 48bits)
>   are physical address restrictions. Here we are talking about virtual
>   address restriction, and I don't think there are actually any
>   restrictions there?  We could validly map something at
>   0x___. So even (1<<48) which makes sense at the physical
>   level, doesn't make sense in terms of virtual addresses.
> 
> 
> The limit for the virtual address is 2^64.
> 
> 
> 
>   > > > Signed-off-by: Julien Grall 
>   > > >
>   > > > ---
>   > > >      Changes in v2:
>   > > >          - New patch
>   > > > ---
>   > > >   xen/arch/arm/mm.c            | 11 +--
>   > > >   xen/include/asm-arm/config.h |  4 
>   > > >   2 files changed, 13 insertions(+), 2 deletions(-)
>   > > >
>   > > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>   > > > index 8fac24d80086..5c17cafff847 100644
>   > > > --- a/xen/arch/arm/mm.c
>   > > > +++ b/xen/arch/arm/mm.c
>   > > > @@ -1275,11 +1275,18 @@ static int xen_pt_update(unsigned long 
> virt,
>   > > >        * For arm32, page-tables are different on each CPUs. Yet, 
> they
>   > > > share
>   > > >        * some common mappings. It is assumed that only common 
> mappings
>   > > >        * will be modified with this function.
>   > > > -     *
>   > > > -     * XXX: Add a check.
>   > > >        */
>   > > >       const mfn_t root = virt_to_mfn(THIS_CPU_PGTABLE);
>   > > >   +#ifdef SHARED_VIRT_END
>   > > > +    if ( virt > SHARED_VIRT_END ||
>   > > > +         (SHARED_VIRT_END - virt) < nr_mfns )
>   > >
>   > > The following would be sufficient, right?
>   > >
>   > >      if ( virt + nr_mfns > SHARED_VIRT_END )
>   >
>   > This would not protect against an overflow. So I think it is best if 
> we keep
>   > my version.
> 
>   But there can be no overflow with the way SHARED_VIRT_END is defined.
> 
>   Even if SHARED_VIRT_END was defined at (1<<48) there can be no overflow.
>   Only if we defined SHARED_VIRT_END as 0x___ we would
>   have an overflow, but you wrote above that your preference is not to do
>   that.
> 
> 
> You can have an overflow regardless the value of SHARED_VIRT_END.
> 
> Imagine virt = 2^64 - 1 and nr_mfs = 1. The addition would result to 0.
> 
> As a consequence the check would pass when it should not.

Yes you are right, I don't know how I missed it!


> One can argue that the caller will always provide sane values. However given 
> the simplicity of the check, this is not worth the trouble if
> a caller is buggy.
> 
> Now, the problem with SHARED_VIRT_END equals to 2^64 - 1 is not the overflow 
> but the compiler that may throw an error/warning for always
> true check. Hence the reason of not defining SHARED_VIRT_END on arm64.

OK, all checks out.

Reviewed-by: Stefano Stabellini 

[qemu-mainline test] 161938: regressions - FAIL

2021-05-13 Thread osstest service owner
flight 161938 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/161938/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-freebsd11-amd64 16 guest-saverestore fail REGR. vs. 
152631
 test-amd64-amd64-qemuu-freebsd12-amd64 16 guest-saverestore fail REGR. vs. 
152631
 test-amd64-i386-freebsd10-i386 16 guest-saverestore  fail REGR. vs. 152631
 test-amd64-i386-freebsd10-amd64 16 guest-saverestore fail REGR. vs. 152631
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 15 guest-saverestore fail 
REGR. vs. 152631
 test-amd64-i386-xl-qemuu-debianhvm-amd64 15 guest-saverestore fail REGR. vs. 
152631
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 15 guest-saverestore fail REGR. 
vs. 152631
 test-amd64-i386-xl-qemuu-ovmf-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 15 guest-saverestore fail REGR. vs. 
152631
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 15 guest-saverestore fail 
REGR. vs. 152631
 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-amd64-xl-qemuu-ovmf-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-amd64-qemuu-nested-intel 20 debian-hvm-install/l1/l2 fail REGR. vs. 
152631
 test-amd64-i386-xl-qemuu-win7-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-i386-xl-qemuu-ws16-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-amd64-xl-qemuu-ws16-amd64 15 guest-saverestore fail REGR. vs. 152631
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. 
vs. 152631

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

Uses of /hypervisor memory range (was: FreeBSD/Xen/ARM issues)

2021-05-13 Thread Elliott Mitchell
Upon thinking about it, this seems appropriate to bring to the attention
of the Xen development list since it seems to have wider implications.


On Wed, May 12, 2021 at 11:08:39AM +0100, Julien Grall wrote:
> On 12/05/2021 03:37, Elliott Mitchell wrote:
> > 
> > What about the approach to the grant-table/xenpv memory situation?
> > 
> > As stated for a 768MB VM, Xen suggested a 16MB range.  I'm unsure whether
> > that is strictly meant for grant-table use or is meant for any foreign
> > memory mappings (Julien?).
> 
> An OS is free to use it as it wants. However, there is no promise that:
>1) The region will not shrink
>2) The region will stay where it is

Issue is what is the intended use of the memory range allocated to
/hypervisor in the device-tree on ARM?  What do the Xen developers plan
for?  What is expected?


With FreeBSD, Julien Grall's attempt 5 years ago at getting Xen/ARM
support treated the grant table as distinct from other foreign memory
mappings.  Yet for the current code (which is oriented towards x86) it is
rather easier to treat all foreign mappings the same.

Limiting foreign mappings to a total of 16MB for a 768MB domain is tight.
Was the /hypervisor range intended *strictly* for mapping grant-tables?
Was it intended for the /hypervisor range to dynamically scale with the
size of the domain?  Was it intended for /hypervisor to grow over the
years as hardware got cheaper?

Might it be better to deprecate the /hypervisor range and have domains
allocate any available address space for foreign mappings?

Should the FreeBSD implementation be treating grant tables as distinct
from other foreign mappings?  (is treating them the same likely to
induce buggy behavior on x86?)


-- 
(\___(\___(\__  --=> 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: [PATCH for-next 5/6] xen: Add files needed for minimal riscv build

2021-05-13 Thread Connor Davis



On 3/12/21 10:09 AM, Jan Beulich wrote:

On 25.02.2021 16:24, Connor Davis wrote:

--- a/xen/include/public/hvm/save.h
+++ b/xen/include/public/hvm/save.h
@@ -106,6 +106,8 @@ DECLARE_HVM_SAVE_TYPE(END, 0, struct hvm_save_end);
  #include "../arch-x86/hvm/save.h"
  #elif defined(__arm__) || defined(__aarch64__)
  #include "../arch-arm/hvm/save.h"
+#elif defined(__riscv)
+#include "../arch-riscv/hvm/save.h"

Does the compiler not also provide __riscv__? If it does, using it
here (and elsewhere) would fit better with the existing logic.


No only __riscv is defined


Thanks,

Connor




[PATCH v4 0/2] Introducing Hyperlaunch capability design (formerly: DomB mode of dom0less)

2021-05-13 Thread Christopher Clark
We are submitting for inclusion in the Xen documentation:

- the Hyperlaunch design document, and
- the Hyperlaunch device tree design document

to describe a new method for launching the Xen hypervisor.

The Hyperlaunch feature builds upon prior dom0less work,
to bring a flexible and security-minded means to launch a
variety of VM configurations as part of the startup of Xen.

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


Daniel P. Smith (2):
  docs/designs/launch: hyperlaunch design document
  docs/designs/launch: hyperlaunch device tree

 .../designs/launch/hyperlaunch-devicetree.rst |  343 ++
 docs/designs/launch/hyperlaunch.rst   | 1004 +
 2 files changed, 1347 insertions(+)
 create mode 100644 docs/designs/launch/hyperlaunch-devicetree.rst
 create mode 100644 docs/designs/launch/hyperlaunch.rst

-- 
2.25.1




[PATCH v4 2/2] docs/designs/launch: Hyperlaunch device tree

2021-05-13 Thread Christopher Clark
From: "Daniel P. Smith" 

Adds a design document for Hyperlaunch device tree structure.

Signed-off-by: Christopher Clark 
Signed-off by: Daniel P. Smith 
---
 .../designs/launch/hyperlaunch-devicetree.rst | 343 ++
 1 file changed, 343 insertions(+)
 create mode 100644 docs/designs/launch/hyperlaunch-devicetree.rst

diff --git a/docs/designs/launch/hyperlaunch-devicetree.rst 
b/docs/designs/launch/hyperlaunch-devicetree.rst
new file mode 100644
index 00..f97d357407
--- /dev/null
+++ b/docs/designs/launch/hyperlaunch-devicetree.rst
@@ -0,0 +1,343 @@
+-
+Xen Hyperlaunch Device Tree Bindings
+-
+
+The Xen Hyperlaunch device tree adopts the dom0less device tree structure and
+extends it to meet the requirements for the Hyperlaunch capability. The primary
+difference is the introduction of the ``hypervisor`` node that is under the
+``/chosen`` node. The move to a dedicated node was driven by:
+
+1. Reduces the need to walk over nodes that are not of interest, e.g. only
+   nodes of interest should be in ``/chosen/hypervisor``
+
+2. Allows for the domain construction information to easily be sanitized by
+   simple removing the ``/chosen/hypervisor`` node.
+
+Example Configuration
+-
+
+Below are two example device tree definitions for the hypervisor node. The
+first is an example of a multiboot-based configuration for x86 and the second
+is a module-based configuration for Arm.
+
+Multiboot x86 Configuration:
+
+
+::
+
+hypervisor {
+#address-cells = <1>;
+#size-cells = <0>;
+compatible = “hypervisor,xen”
+ 
+// Configuration container
+config {
+compatible = "xen,config";
+ 
+module {
+compatible = "module,microcode", "multiboot,module";
+mb-index = <1>;
+};
+ 
+module {
+compatible = "module,xsm-policy", "multiboot,module";
+mb-index = <2>;
+};
+};
+ 
+// Boot Domain definition
+domain {
+compatible = "xen,domain";
+ 
+domid = <0x7FF5>;
+ 
+// FUNCTION_NONE(0)
+// FUNCTION_BOOT(1 << 0)
+// FUNCTION_CRASH   (1 << 1)
+// FUNCTION_CONSOLE (1 << 2)
+// FUNCTION_XENSTORE(1 << 30)
+// FUNCTION_LEGACY_DOM0 (1 << 31)
+functions = <0x0001>;
+ 
+memory = <0x0 0x2>;
+cpus = <1>;
+module {
+compatible = "module,kernel", "multiboot,module";
+mb-index = <3>;
+};
+ 
+module {
+compatible = "module,ramdisk", "multiboot,module";
+mb-index = <4>;
+};
+module {
+compatible = "module,config", "multiboot,module";
+mb-index = <5>;
+};
+ 
+// Classic Dom0 definition
+domain {
+compatible = "xen,domain";
+ 
+domid = <0>;
+ 
+// PERMISSION_NONE  (0)
+// PERMISSION_CONTROL   (1 << 0)
+// PERMISSION_HARDWARE  (1 << 1)
+permissions = <3>;
+ 
+// FUNCTION_NONE(0)
+// FUNCTION_BOOT(1 << 0)
+// FUNCTION_CRASH   (1 << 1)
+// FUNCTION_CONSOLE (1 << 2)
+// FUNCTION_XENSTORE(1 << 30)
+// FUNCTION_LEGACY_DOM0 (1 << 31)
+functions = <0xC006>;
+ 
+// MODE_PARAVIRTUALIZED (1 << 0) /* PV | PVH/HVM */
+// MODE_ENABLE_DEVICE_MODEL (1 << 1) /* HVM | PVH */
+// MODE_LONG(1 << 2) /* 64 BIT | 32 BIT */
+mode = <5>; /* 64 BIT, PV */
+ 
+// UUID
+domain-uuid = [B3 FB 98 FB 8F 9F 67 A3];
+ 
+cpus = <1>;
+memory = <0x0 0x2>;
+security-id = “dom0_t;
+ 
+module {
+compatible = "module,kernel", "multiboot,module";
+mb-index = <6>;
+bootargs = "console=hvc0";
+};
+module {
+compatible = "module,ramdisk", "multiboot,module";
+mb-index = <7>;
+};
+};
+
+The multiboot modules supplied when using the above config would be, in order:
+
+* (the above config, compiled)
+* CPU microcode
+* XSM policy
+* kernel for boot domain
+* ramdisk for boot domain
+* boot domain configuration file
+* kernel for the classic dom0 domain
+* ramdisk for the classic dom0 domain
+
+Module Arm Configuration:
+"
+
+::
+
+hypervisor {
+compatible = “hypervisor,xen”
+ 
+// Configuration container
+config {
+compatible = "xen,config";
+ 
+  

[PATCH v4 1/2] docs/designs/launch: Hyperlaunch design document

2021-05-13 Thread Christopher Clark
From: "Daniel P. Smith" 

Adds a design document for Hyperlaunch, formerly DomB mode of dom0less.

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

---
Changes since v3:
* Rename the Landscape table
* Changed Crash Domain to Recovery Domain
  * amended text to indicate that this will be new rather than existing Xen
functionality
  * including update to the configuration, permission, function table
* Add definitions for “recovery domain” and “crash environment”, describing
  the different functionalities
  * some design issues deferred
* Added section to explain the motivations for the separation between VM
  creation (by the hypervisor) and VM configuration (by the boot domain)
* Adjusted the description of the current process for creating a domain
* Added recommendation for UEFI boot to use GRUB.efi to load via multiboot2
  method.
* Added Document Structure section
* Added section on Communication of Domain Configuration

 docs/designs/launch/hyperlaunch.rst | 1004 +++
 1 file changed, 1004 insertions(+)
 create mode 100644 docs/designs/launch/hyperlaunch.rst

diff --git a/docs/designs/launch/hyperlaunch.rst 
b/docs/designs/launch/hyperlaunch.rst
new file mode 100644
index 00..30fce8c9c3
--- /dev/null
+++ b/docs/designs/launch/hyperlaunch.rst
@@ -0,0 +1,1004 @@
+###
+Hyperlaunch Design Document
+###
+
+.. sectnum:: :depth: 4
+
+This post is a Request for Comment on the included v4 of a design document that
+describes Hyperlaunch: a new method of launching the Xen hypervisor, relating
+to dom0less and work from the Hyperlaunch project. We invite discussion of this
+on this list, at the monthly Xen Community Calls, and at dedicated meetings on
+this topic in the Xen Working Group which will be announced in advance on the
+Xen Development mailing list.
+
+
+.. contents:: :depth: 3
+
+
+Introduction
+
+
+This document describes the design and motivation for the funded development of
+a new, flexible system for launching the Xen hypervisor and virtual machines
+named: "Hyperlaunch".
+
+The design enables seamless transition for existing systems that require a
+dom0, and provides a new general capability to build and launch alternative
+configurations of virtual machines, including support for static partitioning
+and accelerated start of VMs during host boot, while adhering to the principles
+of least privilege. It incorporates the existing dom0less functionality,
+extended to fold in the new developments from the Hyperlaunch project, with
+support for both x86 and Arm platform architectures, building upon and
+replacing the earlier 'late hardware domain' feature for disaggregation of
+dom0.
+
+Hyperlaunch is designed to be flexible and reusable across multiple use cases,
+and our aim is to ensure that it is capable, widely exercised, comprehensively
+tested, and well understood by the Xen community.
+
+Document Structure
+==
+
+This is the primary design document for Hyperlaunch, to provide an overview of
+the feature. Separate additional documents will cover specific aspects of
+Hyperlaunch in further detail, including:
+
+  - The Device Tree specification for Hyperlaunch metadata
+  - New Domain Roles for Xen and the Xen Security Modules (XSM) policy
+  - Passthrough of PCI devices with Hyperlaunch
+
+Approach
+
+
+Born out of improving support for Dynamic Root of Trust for Measurement (DRTM),
+the Hyperlaunch project is focused on restructuring the system launch of Xen.
+The Hyperlaunch design provides a security architecture that builds on the
+principles of Least Privilege and Strong Isolation, achieving this through the
+disaggregation of system functions. It enables this with the introduction of a
+boot domain that works in conjunction with the hypervisor to provide the
+ability to launch multiple domains as part of host boot while maintaining a
+least privilege implementation.
+
+While the Hyperlaunch project inception was and continues to be driven by a
+focus on security through disaggregation, there are multiple use cases with a
+non-security focus that require or benefit from the ability to launch multiple
+domains at host boot. This was proven by the need that drove the implementation
+of the dom0less capability in the Arm branch of Xen.
+
+Hyperlaunch is designed to be flexible and reusable across multiple use cases,
+and our aim is to ensure that it is capable, widely exercised, comprehensively
+tested, and provides a robust foundation for current and emerging system launch
+requirements of the Xen community.
+
+
+Objectives
+--
+
+* In general strive to maintain compatibility with existing Xen behavior
+* A default build of the hypervisor should be capable of booting both 
legacy-compatible and new styles of launch:
+
+* classic Xen boot: starting a single, privileged Dom0
+* classic Xen boot with late hard

[PATCH v2 1/5] xen/char: Default HAS_NS16550 to y only for X86 and ARM

2021-05-13 Thread Connor Davis
Defaulting to yes only for X86 and ARM reduces the requirements
for a minimal build when porting new architectures.

Signed-off-by: Connor Davis 
---
 xen/drivers/char/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index b572305657..b15b0c8d6a 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -1,6 +1,6 @@
 config HAS_NS16550
bool "NS16550 UART driver" if ARM
-   default y
+   default y if (ARM || X86)
help
  This selects the 16550-series UART support. For most systems, say Y.
 
-- 
2.31.1




[PATCH v2 0/5] Minimal build for RISCV

2021-05-13 Thread Connor Davis
Hi all,

This series introduces a minimal build for RISCV. It is based on Bobby's
previous work from last year[0]. I have worked to rebase onto current Xen,
as well as update the various header files borrowed from Linux.

This series provides the patches necessary to get a minimal build
working. The build is "minimal" in the sense that 1) it uses a
minimal config and 2) files, functions, and variables are included if
and only if they are required for a successful build based on the
config. It doesn't run at all, as the functions just have stub
implementations.

My hope is that this can serve as a useful example for future ports as
well as inform the community of exactly what is imposed by common code
onto new architectures.

The first 3 patches are mods to non-RISCV bits that enable building a
config with:

  !CONFIG_HAS_NS16550
  !CONFIG_HAS_PASSTHROUGH
  !CONFIG_GRANT_TABLE

respectively. The fourth patch adds the RISCV files, and the last patch
adds a docker container for doing the build. To build from the docker
container (after creating it locally), you can run the following:

  $ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen 

The sources taken from Linux are documented in arch/riscv/README.sources.
There were also some files copied from arm:

  asm-arm/softirq.h
  asm-arm/random.h
  asm-arm/nospec.h
  asm-arm/numa.h
  asm-arm/p2m.h
  asm-arm/delay.h
  asm-arm/debugger.h
  asm-arm/desc.h
  asm-arm/guest_access.h
  asm-arm/hardirq.h
  lib/find_next_bit.c

I imagine some of these will want some consolidation, but I put them
under the respective RISCV directories for now.

[0] 
https://lore.kernel.org/xen-devel/cover.1579615303.git.bobbyeshle...@gmail.com/

Thanks,
Connor

--
Changes since v1:
  - Dropped "xen/sched: Fix build when NR_CPUS == 1" since this was
fixed for 4.15
  - Moved #ifdef-ary around iommu_enabled to iommu.h
  - Moved struct grant_table declaration above ifdef CONFIG_GRANT_TABLE
instead of defining an empty struct when !CONFIG_GRANT_TABLE

Connor Davis (5):
  xen/char: Default HAS_NS16550 to y only for X86 and ARM
  xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH
  xen: Fix build when !CONFIG_GRANT_TABLE
  xen: Add files needed for minimal riscv build
  automation: add container for riscv64 builds

 automation/build/archlinux/riscv64.dockerfile |  33 ++
 automation/scripts/containerize   |   1 +
 config/riscv64.mk |   7 +
 xen/Makefile  |   8 +-
 xen/arch/riscv/Kconfig|  54 +++
 xen/arch/riscv/Kconfig.debug  |   0
 xen/arch/riscv/Makefile   |  57 +++
 xen/arch/riscv/README.source  |  19 +
 xen/arch/riscv/Rules.mk   |  13 +
 xen/arch/riscv/arch.mk|   7 +
 xen/arch/riscv/configs/riscv64_defconfig  |  12 +
 xen/arch/riscv/delay.c|  16 +
 xen/arch/riscv/domain.c   | 144 +++
 xen/arch/riscv/domctl.c   |  36 ++
 xen/arch/riscv/guestcopy.c|  57 +++
 xen/arch/riscv/head.S |   6 +
 xen/arch/riscv/irq.c  |  78 
 xen/arch/riscv/lib/Makefile   |   1 +
 xen/arch/riscv/lib/find_next_bit.c| 284 +
 xen/arch/riscv/mm.c   |  93 +
 xen/arch/riscv/p2m.c  | 144 +++
 xen/arch/riscv/percpu.c   |  17 +
 xen/arch/riscv/platforms/Kconfig  |  31 ++
 xen/arch/riscv/riscv64/asm-offsets.c  |  31 ++
 xen/arch/riscv/setup.c|  27 ++
 xen/arch/riscv/shutdown.c |  28 ++
 xen/arch/riscv/smp.c  |  35 ++
 xen/arch/riscv/smpboot.c  |  34 ++
 xen/arch/riscv/sysctl.c   |  33 ++
 xen/arch/riscv/time.c |  35 ++
 xen/arch/riscv/traps.c|  35 ++
 xen/arch/riscv/vm_event.c |  39 ++
 xen/arch/riscv/xen.lds.S  | 113 ++
 xen/common/memory.c   |  10 +
 xen/drivers/char/Kconfig  |   2 +-
 xen/include/asm-riscv/altp2m.h|  39 ++
 xen/include/asm-riscv/asm.h   |  77 
 xen/include/asm-riscv/asm_defns.h |  24 ++
 xen/include/asm-riscv/atomic.h| 204 ++
 xen/include/asm-riscv/bitops.h| 331 +++
 xen/include/asm-riscv/bug.h   |  54 +++
 xen/include/asm-riscv/byteorder.h |  16 +
 xen/include/asm-riscv/cache.h |  24 ++
 xen/include/asm-riscv/cmpxchg.h   | 382 ++
 xen/include/asm-riscv/compiler_types.h|  32 ++
 xen/include/asm-riscv/config.h| 110 +
 xen/include/asm-riscv/cpufeature.h|  17 +
 xen/include/asm-riscv/csr.h 

[PATCH v2 2/5] xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH

2021-05-13 Thread Connor Davis
The variables iommu_enabled and iommu_dont_flush_iotlb are defined in
drivers/passthrough/iommu.c and are referenced in common code, which
causes the link to fail when !CONFIG_HAS_PASSTHROUGH.

Guard references to these variables in common code so that xen
builds when !CONFIG_HAS_PASSTHROUGH.

Signed-off-by: Connor Davis 
---
 xen/common/memory.c | 10 ++
 xen/include/xen/iommu.h |  8 +++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index b5c70c4b85..72a6b70cb5 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -294,7 +294,9 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
 p2m_type_t p2mt;
 #endif
 mfn_t mfn;
+#ifdef CONFIG_HAS_PASSTHROUGH
 bool *dont_flush_p, dont_flush;
+#endif
 int rc;
 
 #ifdef CONFIG_X86
@@ -385,13 +387,17 @@ int guest_remove_page(struct domain *d, unsigned long 
gmfn)
  * Since we're likely to free the page below, we need to suspend
  * xenmem_add_to_physmap()'s suppressing of IOMMU TLB flushes.
  */
+#ifdef CONFIG_HAS_PASSTHROUGH
 dont_flush_p = &this_cpu(iommu_dont_flush_iotlb);
 dont_flush = *dont_flush_p;
 *dont_flush_p = false;
+#endif
 
 rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
 
+#ifdef CONFIG_HAS_PASSTHROUGH
 *dont_flush_p = dont_flush;
+#endif
 
 /*
  * With the lack of an IOMMU on some platforms, domains with DMA-capable
@@ -839,11 +845,13 @@ int xenmem_add_to_physmap(struct domain *d, struct 
xen_add_to_physmap *xatp,
 xatp->gpfn += start;
 xatp->size -= start;
 
+#ifdef CONFIG_HAS_PASSTHROUGH
 if ( is_iommu_enabled(d) )
 {
this_cpu(iommu_dont_flush_iotlb) = 1;
extra.ppage = &pages[0];
 }
+#endif
 
 while ( xatp->size > done )
 {
@@ -868,6 +876,7 @@ int xenmem_add_to_physmap(struct domain *d, struct 
xen_add_to_physmap *xatp,
 }
 }
 
+#ifdef CONFIG_HAS_PASSTHROUGH
 if ( is_iommu_enabled(d) )
 {
 int ret;
@@ -894,6 +903,7 @@ int xenmem_add_to_physmap(struct domain *d, struct 
xen_add_to_physmap *xatp,
 if ( unlikely(ret) && rc >= 0 )
 rc = ret;
 }
+#endif
 
 return rc;
 }
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 460755df29..d878a93269 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -51,9 +51,15 @@ static inline bool_t dfn_eq(dfn_t x, dfn_t y)
 return dfn_x(x) == dfn_x(y);
 }
 
-extern bool_t iommu_enable, iommu_enabled;
+extern bool_t iommu_enable;
 extern bool force_iommu, iommu_quarantine, iommu_verbose;
 
+#ifdef CONFIG_HAS_PASSTHROUGH
+extern bool_t iommu_enabled;
+#else
+#define iommu_enabled false
+#endif
+
 #ifdef CONFIG_X86
 extern enum __packed iommu_intremap {
/*
-- 
2.31.1




[PATCH v2 3/5] xen: Fix build when !CONFIG_GRANT_TABLE

2021-05-13 Thread Connor Davis
Move struct grant_table; in grant_table.h above
ifdef CONFIG_GRANT_TABLE. This fixes the following:

/build/xen/include/xen/grant_table.h:84:50: error: 'struct grant_table'
declared inside parameter list will not be visible outside of this
definition or declaration [-Werror]
   84 | static inline int mem_sharing_gref_to_gfn(struct grant_table *gt,
  |

Signed-off-by: Connor Davis 
---
 xen/include/xen/grant_table.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 63b6dc78f4..9f8b7e66c1 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -28,9 +28,10 @@
 #include 
 #include 
 
-#ifdef CONFIG_GRANT_TABLE
 struct grant_table;
 
+#ifdef CONFIG_GRANT_TABLE
+
 extern unsigned int opt_max_grant_frames;
 
 /* Create/destroy per-domain grant table context. */
-- 
2.31.1




[PATCH v2 5/5] automation: add container for riscv64 builds

2021-05-13 Thread Connor Davis
Add a container for cross-compiling xen to riscv64.
This just includes the cross-compiler and necessary packages for
building xen itself (packages for tools, stubdoms, etc., can be
added later).

To build xen in the container run the following:

$ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen

Signed-off-by: Connor Davis 
---
 automation/build/archlinux/riscv64.dockerfile | 33 +++
 automation/scripts/containerize   |  1 +
 2 files changed, 34 insertions(+)
 create mode 100644 automation/build/archlinux/riscv64.dockerfile

diff --git a/automation/build/archlinux/riscv64.dockerfile 
b/automation/build/archlinux/riscv64.dockerfile
new file mode 100644
index 00..505b623c01
--- /dev/null
+++ b/automation/build/archlinux/riscv64.dockerfile
@@ -0,0 +1,33 @@
+FROM archlinux
+LABEL maintainer.name="The Xen Project" \
+  maintainer.email="xen-devel@lists.xenproject.org"
+
+# Packages needed for the build
+RUN pacman --noconfirm --needed -Syu \
+base-devel \
+gcc \
+git
+
+# Packages needed for QEMU
+RUN pacman --noconfirm --needed -Syu \
+pixman \
+python \
+sh
+
+# There is a regression in GDB that causes an assertion error
+# when setting breakpoints, use this commit until it is fixed!
+RUN git clone --recursive -j$(nproc) --progress 
https://github.com/riscv/riscv-gnu-toolchain && \
+cd riscv-gnu-toolchain/riscv-gdb && \
+git checkout 1dd588507782591478882a891f64945af9e2b86c && \
+cd  .. && \
+./configure --prefix=/opt/riscv && \
+make linux -j$(nproc) && \
+rm -R /riscv-gnu-toolchain
+
+# Add compiler path
+ENV PATH=/opt/riscv/bin/:${PATH}
+ENV CROSS_COMPILE=riscv64-unknown-linux-gnu-
+
+RUN useradd --create-home user
+USER user
+WORKDIR /build
diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index b7c81559fb..59edf0ba40 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -26,6 +26,7 @@ BASE="registry.gitlab.com/xen-project/xen"
 case "_${CONTAINER}" in
 _alpine) CONTAINER="${BASE}/alpine:3.12" ;;
 _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
+_riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;;
 _centos7) CONTAINER="${BASE}/centos:7" ;;
 _centos72) CONTAINER="${BASE}/centos:7.2" ;;
 _fedora) CONTAINER="${BASE}/fedora:29";;
-- 
2.31.1




RE: Discussion of Xenheap problems on AArch64

2021-05-13 Thread Henry Wang
> From: Julien Grall 
Hi Julien,

> 
> On 11/05/2021 02:11, Henry Wang wrote:
> > Hi Julien,
> Hi Henry,
> >
> >> From: Julien Grall 
> >> Hi Henry,
> >>
> >> On 07/05/2021 05:06, Henry Wang wrote:
>  From: Julien Grall 
>  On 28/04/2021 10:28, Henry Wang wrote:
> >> [...]
> >>
> >>> when I continue booting Xen, I got following error log:
> >>>
> >>> (XEN) Xen call trace:
> >>> (XEN)[<002b5a5c>] alloc_boot_pages+0x94/0x98 (PC)
> >>> (XEN)[<002ca3bc>] setup_frametable_mappings+0xa4/0x108
> >> (LR)
> >>> (XEN)[<002ca3bc>] setup_frametable_mappings+0xa4/0x108
> >>> (XEN)[<002cb988>] start_xen+0x344/0xbcc
> >>> (XEN)[<002001c0>]
> >> arm64/head.o#primary_switched+0x10/0x30
> >>> (XEN)
> >>> (XEN) 
> >>> (XEN) Panic on CPU 0:
> >>> (XEN) Xen BUG at page_alloc.c:432
> >>> (XEN) 
> >>
> >> This is happening without my patch series applied, right? If so, what
> >> happen if you apply it?
> >
> > No, I am afraid this is with your patch series applied, and that is why I
> > am a little bit confused about the error log...
> 
> You are hitting the BUG() at the end of alloc_boot_pages(). This is hit
> because the boot allocator couldn't allocate memory for your request.
> 
> Would you be able to apply the following diff and paste the output here?

Thank you, of course yes, please see below output attached :)

> 
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index ace6333c18ea..dbb736fdb275 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -329,6 +329,8 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe)
>   if ( pe <= ps )
>   return;
> 
> +printk("%s: ps %"PRI_paddr" pe %"PRI_paddr"\n", __func__, ps, pe);
  ^ FYI: I have to change this 
PRI_paddr to PRIpaddr
 to make compiler happy

> +
>   first_valid_mfn = mfn_min(maddr_to_mfn(ps), first_valid_mfn);
> 
>   bootmem_region_add(ps >> PAGE_SHIFT, pe >> PAGE_SHIFT);
> @@ -395,6 +397,8 @@ mfn_t __init alloc_boot_pages(unsigned long nr_pfns,
> unsigned long pfn_align)
>   unsigned long pg, _e;
>   unsigned int i = nr_bootmem_regions;
> 
> +printk("%s: nr_pfns %lu pfn_align %lu\n", __func__, nr_pfns,
> pfn_align);
> +
>   BUG_ON(!nr_bootmem_regions);
> 
>   while ( i-- )
> 

I also added some printk to make sure the dtb is parsed correctly, and for the
Error case, I get following log:

(XEN) --banks=2
(XEN) --start=8000
(XEN) --size=7F00
(XEN) --start=F9
(XEN) --size=8000
(XEN) Checking for initrd in /chosen
(XEN) RAM: 8000 - feff
(XEN) RAM: 00f9 - 00f97fff
(XEN)
(XEN) MODULE[0]: 8400 - 841464c8 Xen
(XEN) MODULE[1]: 841464c8 - 84148c9b Device Tree
(XEN) MODULE[2]: 8008 - 8108 Kernel
(XEN)  RESVD[0]: 8000 - 8001
(XEN)
(XEN) Command line: noreboot dom0_mem=1024M console=dtuart 
dtuart=serial0 bootscrub=0
(XEN) PFN compression on bits 21...22
(XEN) init_boot_pages: ps 8001 pe 8008
(XEN) init_boot_pages: ps 8108 pe 8400
(XEN) init_boot_pages: ps 84149000 pe ff00
(XEN) alloc_boot_pages: nr_pfns 1 pfn_align 1
(XEN) alloc_boot_pages: nr_pfns 1 pfn_align 1
(XEN) alloc_boot_pages: nr_pfns 1 pfn_align 1
(XEN) init_boot_pages: ps 00f9 pe 00f98000
(XEN) alloc_boot_pages: nr_pfns 909312 pfn_align 8192
(XEN) Xen BUG at page_alloc.c:436

To compare with the maximum start address (f8) of second part mem
where xen boots correctly, I also attached the log for your information:

(XEN) --banks=2
(XEN) --start=8000
(XEN) --size=7F00
(XEN) --start=F8
(XEN) --size=8000
(XEN) Checking for initrd in /chosen
(XEN) RAM: 8000 - feff
(XEN) RAM: 00f8 - 00f87fff
(XEN)
(XEN) MODULE[0]: 8400 - 841464c8 Xen
(XEN) MODULE[1]: 841464c8 - 84148c9b Device Tree
(XEN) MODULE[2]: 8008 - 8108 Kernel
(XEN)  RESVD[0]: 8000 - 8001
(XEN)
(XEN) Command line: noreboot dom0_mem=1024M console=dtuart
dtuart=serial0 bootscrub=0
(XEN) PFN compression on bits 20...22
(XEN) init_boot_pages: ps 8001 pe 8008
(XEN) init_boot_pages: ps 8108 pe 8400
(XEN) init_boot_pages: ps 84149000 pe ff00
(XEN) alloc_boot_pages: nr_pfns 1 pfn_align 1
(XEN) alloc_boot_pages: nr_pfns 1 pfn_align 1
(XEN) alloc_boot_pages: nr_pfns 1 pfn_align 1
(XEN) init_boot_pages: ps 00f8 pe 00f88000
(XEN) all

Re: [PATCH v2 0/5] Minimal build for RISCV

2021-05-13 Thread Alistair Francis
On Fri, May 14, 2021 at 2:18 PM Connor Davis  wrote:
>
> Hi all,
>
> This series introduces a minimal build for RISCV. It is based on Bobby's
> previous work from last year[0]. I have worked to rebase onto current Xen,
> as well as update the various header files borrowed from Linux.
>
> This series provides the patches necessary to get a minimal build
> working. The build is "minimal" in the sense that 1) it uses a
> minimal config and 2) files, functions, and variables are included if
> and only if they are required for a successful build based on the
> config. It doesn't run at all, as the functions just have stub
> implementations.
>
> My hope is that this can serve as a useful example for future ports as
> well as inform the community of exactly what is imposed by common code
> onto new architectures.
>
> The first 3 patches are mods to non-RISCV bits that enable building a
> config with:
>
>   !CONFIG_HAS_NS16550
>   !CONFIG_HAS_PASSTHROUGH
>   !CONFIG_GRANT_TABLE
>
> respectively. The fourth patch adds the RISCV files, and the last patch
> adds a docker container for doing the build. To build from the docker
> container (after creating it locally), you can run the following:
>
>   $ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen
>
> The sources taken from Linux are documented in arch/riscv/README.sources.
> There were also some files copied from arm:
>
>   asm-arm/softirq.h
>   asm-arm/random.h
>   asm-arm/nospec.h
>   asm-arm/numa.h
>   asm-arm/p2m.h
>   asm-arm/delay.h
>   asm-arm/debugger.h
>   asm-arm/desc.h
>   asm-arm/guest_access.h
>   asm-arm/hardirq.h
>   lib/find_next_bit.c
>
> I imagine some of these will want some consolidation, but I put them
> under the respective RISCV directories for now.

Awesome!

Do you have a public branch I could pull these from to try out?

Alistair

>
> [0] 
> https://lore.kernel.org/xen-devel/cover.1579615303.git.bobbyeshle...@gmail.com/
>
> Thanks,
> Connor
>
> --
> Changes since v1:
>   - Dropped "xen/sched: Fix build when NR_CPUS == 1" since this was
> fixed for 4.15
>   - Moved #ifdef-ary around iommu_enabled to iommu.h
>   - Moved struct grant_table declaration above ifdef CONFIG_GRANT_TABLE
> instead of defining an empty struct when !CONFIG_GRANT_TABLE
>
> Connor Davis (5):
>   xen/char: Default HAS_NS16550 to y only for X86 and ARM
>   xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH
>   xen: Fix build when !CONFIG_GRANT_TABLE
>   xen: Add files needed for minimal riscv build
>   automation: add container for riscv64 builds
>
>  automation/build/archlinux/riscv64.dockerfile |  33 ++
>  automation/scripts/containerize   |   1 +
>  config/riscv64.mk |   7 +
>  xen/Makefile  |   8 +-
>  xen/arch/riscv/Kconfig|  54 +++
>  xen/arch/riscv/Kconfig.debug  |   0
>  xen/arch/riscv/Makefile   |  57 +++
>  xen/arch/riscv/README.source  |  19 +
>  xen/arch/riscv/Rules.mk   |  13 +
>  xen/arch/riscv/arch.mk|   7 +
>  xen/arch/riscv/configs/riscv64_defconfig  |  12 +
>  xen/arch/riscv/delay.c|  16 +
>  xen/arch/riscv/domain.c   | 144 +++
>  xen/arch/riscv/domctl.c   |  36 ++
>  xen/arch/riscv/guestcopy.c|  57 +++
>  xen/arch/riscv/head.S |   6 +
>  xen/arch/riscv/irq.c  |  78 
>  xen/arch/riscv/lib/Makefile   |   1 +
>  xen/arch/riscv/lib/find_next_bit.c| 284 +
>  xen/arch/riscv/mm.c   |  93 +
>  xen/arch/riscv/p2m.c  | 144 +++
>  xen/arch/riscv/percpu.c   |  17 +
>  xen/arch/riscv/platforms/Kconfig  |  31 ++
>  xen/arch/riscv/riscv64/asm-offsets.c  |  31 ++
>  xen/arch/riscv/setup.c|  27 ++
>  xen/arch/riscv/shutdown.c |  28 ++
>  xen/arch/riscv/smp.c  |  35 ++
>  xen/arch/riscv/smpboot.c  |  34 ++
>  xen/arch/riscv/sysctl.c   |  33 ++
>  xen/arch/riscv/time.c |  35 ++
>  xen/arch/riscv/traps.c|  35 ++
>  xen/arch/riscv/vm_event.c |  39 ++
>  xen/arch/riscv/xen.lds.S  | 113 ++
>  xen/common/memory.c   |  10 +
>  xen/drivers/char/Kconfig  |   2 +-
>  xen/include/asm-riscv/altp2m.h|  39 ++
>  xen/include/asm-riscv/asm.h   |  77 
>  xen/include/asm-riscv/asm_defns.h |  24 ++
>  xen/include/asm-riscv/atomic.h| 204 ++
>  xen/include/asm-riscv/bitops.h| 331 +++
>  xen/include/asm-riscv/bug.h   |  54 +++
>  xen/include/asm-riscv/byteorder.h 

Re: [PATCH v2 1/5] xen/char: Default HAS_NS16550 to y only for X86 and ARM

2021-05-13 Thread Elliott Mitchell
On Thu, May 13, 2021 at 10:17:08PM -0600, Connor Davis wrote:
> Defaulting to yes only for X86 and ARM reduces the requirements
> for a minimal build when porting new architectures.
> 
> Signed-off-by: Connor Davis 
> ---
>  xen/drivers/char/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
> index b572305657..b15b0c8d6a 100644
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -1,6 +1,6 @@
>  config HAS_NS16550
>   bool "NS16550 UART driver" if ARM
> - default y
> + default y if (ARM || X86)
>   help
> This selects the 16550-series UART support. For most systems, say Y.

Are you sure this is necessary?  I've been working on something else
recently, but did you confirm this with a full build?

If you observe the line directly above that one, `_if_ARM_`.  I'm pretty
sure this driver will refuse to show up in a RISC-V build.


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





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

2021-05-13 Thread osstest service owner
flight 161939 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/161939/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl  broken
 test-armhf-armhf-xl   5 host-install(5)broken REGR. vs. 161926
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. 
vs. 161926

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 18 guest-localmigrate   fail REGR. vs. 161926

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

version targeted for testing:
 xen  cb199cc7de987cfda4659fccf51059f210f6ad34
baseline version:
 xen  43d4cc7d36503bcc3aa2aa6ebea2b7912808f254

Last test of basis   161926  2021-05-13 03:59:53 Z1 days
Testing same since   161939  2021-05-13 21:07:48 Z0 days1 attempts


People who touched revisions under test:
  Andrew 

Re: [PATCH v2 0/4] Support xen-driven USB3 debug capability

2021-05-13 Thread Greg Kroah-Hartman
On Thu, May 13, 2021 at 06:56:47PM -0600, Connor Davis wrote:
> Hi all,
> 
> This goal of this series is to allow the USB3 debug capability (DbC) to be
> safely used by xen while linux runs as dom0.

Patch 2/4 does not seem to be showing up anywhere, did it get lost?

thanks,

greg k-h