[Xen-devel] [xen-unstable test] 149068: regressions - trouble: fail/pass/starved

2020-03-27 Thread osstest service owner
flight 149068 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/149068/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-dom0pvh-xl-intel 16 guest-localmigrate  fail REGR. vs. 148925

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 148925
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 148925
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 148925
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail like 
148925
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 148925
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 148925
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 148925
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 148925
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 148925
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64  2 hosts-allocate  starved n/a
 test-amd64-amd64-dom0pvh-xl-amd  2 hosts-allocate   starved  n/a

version targeted for testing:
 xen  88a1a11daeb93c0f16d9c4d5cb30f1f563c1817c
baseline version:
 xen  60d6ba1916dce0622a53b00dbae3c01d0761057e

Last test of basis   148925  2020-03-23 17:36:41 Z4 days
Failing since148980  2020-03-24 16:19:46 Z3 days3 attempts
Testing same since   149068  2020-03-26 23:52:18 Z1 days1 attempts


[Xen-devel] [seabios test] 149072: regressions - FAIL

2020-03-27 Thread osstest service owner
flight 149072 seabios real [real]
http://logs.test-lab.xenproject.org/osstest/logs/149072/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
148666

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 148666
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 148666
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 148666
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop  fail starved in 148666

version targeted for testing:
 seabios  de88a9628426e82f1cee4b61b06e67e6787301b1
baseline version:
 seabios  066a9956097b54530888b88ab9aa1ea02e42af5a

Last test of basis   148666  2020-03-17 13:39:45 Z   10 days
Failing since148690  2020-03-18 06:43:59 Z9 days   12 attempts
Testing same since   148794  2020-03-20 23:39:57 Z7 days8 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Matt DeVillier 
  Paul Menzel 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm pass
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  pass
 test-amd64-amd64-qemuu-nested-amdfail
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-amd64-amd64-xl-qemuu-ws16-amd64 fail
 test-amd64-i386-xl-qemuu-ws16-amd64  fail
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrictpass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict pass
 test-amd64-amd64-qemuu-nested-intel  fail
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  pass



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

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

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

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


Not pushing.


commit de88a9628426e82f1cee4b61b06e67e6787301b1
Author: Paul Menzel 
Date:   Wed Mar 4 14:51:27 2020 +0100

std/tcg: Replace zero-length array with flexible-array member

GCC 10 gives the warnings below:

In file included from out/ccode32flat.o.tmp.c:54:
./src/tcgbios.c: In function 'tpm20_write_EfiSpecIdEventStruct':
./src/tcgbios.c:290:30: warning: array subscript '() + 
4294967295' is outside the bounds of an interior zero-length array 'struct 
TCG_EfiSpecIdEventAlgorithmSize[0]' [-Wzero-length-bounds]
  290 | event.hdr.digestSizes[count].algorithmId = 
be16_to_cpu(sel->hashAlg);
  | ~^~~
In file included from ./src/tcgbios.c:22,
 from out/ccode32flat.o.tmp.c:54:
./src/std/tcg.h:527:7: note: while referencing 'digestSizes'
  527 | } digestSizes[0];
  |   ^~~
In file included from out/ccode32flat.o.tmp.c:54:

Re: [Xen-devel] PCIe IOMMU ACS support

2020-03-27 Thread Roman Shaposhnik
On Fri, Mar 27, 2020 at 2:12 AM Paul Durrant  wrote:
>
> > -Original Message-
> > From: Roman Shaposhnik 
> > Sent: 26 March 2020 22:03
> > To: Roger Pau Monné 
> > Cc: xen-devel@lists.xenproject.org; Jan Beulich ; Paul 
> > Durrant ;
> > Kevin Tian ; Andrew Cooper 
> > Subject: Re: [Xen-devel] PCIe IOMMU ACS support
> >
> > On Wed, Mar 25, 2020 at 4:05 AM Roger Pau Monné  
> > wrote:
> > >
> > > Adding the PCI and IOMMU maintainers.
> > >
> > > On Mon, Mar 23, 2020 at 01:55:01PM -0700, Roman Shaposhnik wrote:
> > > > Hi!
> > > >
> > > > I was going through how Xen support PCIe IOMMU ACS and
> > > > all I could find is this:
> > > > 
> > > > https://github.com/xen-project/xen/blob/master/xen/drivers/passthrough/pci.c#L608
> > > > which looks to me as an attempt of enabling ACS opportunistically,
> > > > but still proceeding forward even if it fails.
> > >
> > > That's correct AFAICT. Xen will try to enable some features, but will
> > > proceed normally if ACS is not available, or if some of the features
> > > are not implemented.
> > >
> > > Are you looking to ensure that all devices on the system have a
> > > certain feature enabled?
> >
> > My primary objective was to get some visibility into how Xen would
> > prevent two PCIe devices behind a common bridge from doing p2p
> > transactions (thus violating VM boundaries if those devices are
> > assigned to different domains).
> >
> > It looks like Xen simply trusts the hardware.
> >
> > > Can you provide some more details about what you expect of ACS
> > > handling?
> >
> > I was actually surprised not to see IOMMU groups in the style of what
> > VFIO https://www.kernel.org/doc/Documentation/vfio.txt
> >
>
> I did write a doc some time ago to present the issues facing Xen w.r.t. IOMMU 
> and device pass-through. Hopefully you can see it at 
> https://docs.google.com/document/d/12-z6JD41J_oNrCg_c0yAxGWg5ADBQ8_bSiP_NH6Hqwo/edit?usp=sharing

Paul, this is *exactly* what I was asking about -- thanks for the link.

I guess the only question I have left is whether there was any follow up
regarding what you sketched out in the "IOMMU drivers" section?

Thanks,
Roman.



[Xen-devel] [xen-unstable-smoke test] 149110: tolerable all pass - PUSHED

2020-03-27 Thread osstest service owner
flight 149110 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/149110/

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  a87676c4a32f94d79fcaf5b4e0eb59e880e0f032
baseline version:
 xen  fe746c26c0d23c61dbc7eb1918addb1c9a3729bf

Last test of basis   149096  2020-03-27 14:00:46 Z0 days
Testing same since   149110  2020-03-27 18:05:39 Z0 days1 attempts


People who touched revisions under test:
  Julien Grall 
  Paul Durrant 
  Paul Durrant 

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
   fe746c26c0..a87676c4a3  a87676c4a32f94d79fcaf5b4e0eb59e880e0f032 -> smoke



[Xen-devel] [PATCH] sched/core: Fix bug when moving a domain between cpupools

2020-03-27 Thread Jeff Kubascik
For each UNIT, sched_set_affinity is called before unit->priv is updated
to the new cpupool private UNIT data structure. The issue is
sched_set_affinity will call the adjust_affinity method of the cpupool.
If defined, the new cpupool may use unit->priv (e.g. credit), which at
this point still references the old cpupool private UNIT data structure.

This change fixes the bug by moving the switch of unit->priv earler in
the function.

Signed-off-by: Jeff Kubascik 
---
Hello,

I've been working on updating the arinc653 scheduler to support
multicore for a few months now. In the process of testing, I came across
this obscure bug in the core scheduler code that took me a few weeks to
track down. This bug resulted in the credit scheduler writing past the
end of the arinc653 private UNIT data structure into the TLSF allocator
bhdr structure of the adjacent region. This required some deep diving
into the TLSF allocator code to trace the bug back to this point.

Sincerely,
Jeff Kubascik
---
 xen/common/sched/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 7e8e7d2c39..ea572a345a 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -686,6 +686,7 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
 unsigned int unit_p = new_p;
 
 unitdata = unit->priv;
+unit->priv = unit_priv[unit_idx];
 
 for_each_sched_unit_vcpu ( unit, v )
 {
@@ -707,7 +708,6 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
  */
 spin_unlock_irq(lock);
 
-unit->priv = unit_priv[unit_idx];
 if ( !d->is_dying )
 sched_move_irqs(unit);
 
-- 
2.17.1




[Xen-devel] [ovmf test] 149071: all pass - PUSHED

2020-03-27 Thread osstest service owner
flight 149071 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/149071/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 695d90b9b156573d0dafb20afecea09dc9a914f4
baseline version:
 ovmf f52b30e73ddee9a3a609a6e5aa87e79cf4f50879

Last test of basis   149048  2020-03-26 08:15:52 Z1 days
Testing same since   149071  2020-03-27 01:46:00 Z0 days1 attempts


People who touched revisions under test:
  Ard Biesheuvel 
  Ashish Singhal 
  Gaurav Jain 
  Guomin Jiang 
  Hao A Wu 
  Laszlo Ersek 
  Sami Mujawar 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   f52b30e73d..695d90b9b1  695d90b9b156573d0dafb20afecea09dc9a914f4 -> 
xen-tested-master



[Xen-devel] [PATCH 0/3] xen/x86: Simplify ioapic_init()

2020-03-27 Thread Julien Grall
From: Julien Grall 

Hi all,

The main goal of this small series is to simplify ioapic_init().

Cheers,

Julien Grall (3):
  xen/x86: ioapic: Use true/false in bad_ioapic_register()
  xen/x86: ioapic: Rename init_ioapic_mappings() to init_ioapic()
  xen/x86: ioapic: Simplify ioapic_init()

 xen/arch/x86/apic.c   |  2 +-
 xen/arch/x86/io_apic.c| 67 +--
 xen/include/asm-x86/io_apic.h |  2 +-
 3 files changed, 34 insertions(+), 37 deletions(-)

-- 
2.17.1




[Xen-devel] [PATCH 2/3] xen/x86: ioapic: Rename init_ioapic_mappings() to init_ioapic()

2020-03-27 Thread Julien Grall
From: Julien Grall 

The function init_ioapic_mappings() is doing more than initialization
mappings. It is also initialization the number of IRQs/GSIs supported.

So rename the function to init_ioapic(). This will allow us to re-use
the name in a follow-up patch.

Signed-off-by: Julien Grall 
---
 xen/arch/x86/apic.c   | 2 +-
 xen/arch/x86/io_apic.c| 2 +-
 xen/include/asm-x86/io_apic.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index cde67cd87e..c7a54cafc3 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -978,7 +978,7 @@ __next:
 boot_cpu_physical_apicid = get_apic_id();
 x86_cpu_to_apicid[0] = get_apic_id();
 
-init_ioapic_mappings();
+init_ioapic();
 }
 
 /*
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 9868933287..9a11ee8342 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2537,7 +2537,7 @@ static __init bool bad_ioapic_register(unsigned int idx)
 return false;
 }
 
-void __init init_ioapic_mappings(void)
+void __init init_ioapic(void)
 {
 unsigned long ioapic_phys;
 unsigned int i, idx = FIX_IO_APIC_BASE_0;
diff --git a/xen/include/asm-x86/io_apic.h b/xen/include/asm-x86/io_apic.h
index 998905186b..8c0af4bdd3 100644
--- a/xen/include/asm-x86/io_apic.h
+++ b/xen/include/asm-x86/io_apic.h
@@ -180,7 +180,7 @@ extern int io_apic_get_version (int ioapic);
 extern int io_apic_get_redir_entries (int ioapic);
 extern int io_apic_set_pci_routing (int ioapic, int pin, int irq, int 
edge_level, int active_high_low);
 
-extern void init_ioapic_mappings(void);
+extern void init_ioapic(void);
 
 extern void ioapic_suspend(void);
 extern void ioapic_resume(void);
-- 
2.17.1




[Xen-devel] [PATCH 3/3] xen/x86: ioapic: Simplify ioapic_init()

2020-03-27 Thread Julien Grall
From: Julien Grall 

Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
the fixmap.

Therefore the whole logic to allocate a fake page for some IO APICs is
unnecessary.

With the logic removed, the code can be simplified a lot as we don't
need to go through all the IO APIC if SMP has not been detected or a
bogus zero IO-APIC address has been detected.

To avoid another level of tabulation, the simplification is now moved in
its own function.

Signed-off-by: Julien Grall 
---
 xen/arch/x86/io_apic.c | 63 --
 1 file changed, 30 insertions(+), 33 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 9a11ee8342..3d52e4daf1 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2537,34 +2537,25 @@ static __init bool bad_ioapic_register(unsigned int idx)
 return false;
 }
 
-void __init init_ioapic(void)
+static void __init init_ioapic_mappings(void)
 {
-unsigned long ioapic_phys;
 unsigned int i, idx = FIX_IO_APIC_BASE_0;
-union IO_APIC_reg_01 reg_01;
 
-if ( smp_found_config )
-nr_irqs_gsi = 0;
 for ( i = 0; i < nr_ioapics; i++ )
 {
-if ( smp_found_config )
-{
-ioapic_phys = mp_ioapics[i].mpc_apicaddr;
-if ( !ioapic_phys )
-{
-printk(KERN_ERR "WARNING: bogus zero IO-APIC address "
-   "found in MPTABLE, disabling IO/APIC support!\n");
-smp_found_config = false;
-skip_ioapic_setup = true;
-goto fake_ioapic_page;
-}
-}
-else
+union IO_APIC_reg_01 reg_01;
+unsigned long ioapic_phys = mp_ioapics[i].mpc_apicaddr;
+
+ioapic_phys = mp_ioapics[i].mpc_apicaddr;
+if ( !ioapic_phys )
 {
- fake_ioapic_page:
-ioapic_phys = __pa(alloc_xenheap_page());
-clear_page(__va(ioapic_phys));
+printk(KERN_ERR
+   "WARNING: bogus zero IO-APIC address found in MPTABLE, 
disabling IO/APIC support!\n");
+smp_found_config = false;
+skip_ioapic_setup = true;
+break;
 }
+
 set_fixmap_nocache(idx, ioapic_phys);
 apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
 __fix_to_virt(idx), ioapic_phys);
@@ -2576,18 +2567,24 @@ void __init init_ioapic(void)
 continue;
 }
 
-if ( smp_found_config )
-{
-/* The number of IO-APIC IRQ registers (== #pins): */
-reg_01.raw = io_apic_read(i, 1);
-nr_ioapic_entries[i] = reg_01.bits.entries + 1;
-nr_irqs_gsi += nr_ioapic_entries[i];
-
-if ( rangeset_add_singleton(mmio_ro_ranges,
-ioapic_phys >> PAGE_SHIFT) )
-printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
-   ioapic_phys);
-}
+/* The number of IO-APIC IRQ registers (== #pins): */
+reg_01.raw = io_apic_read(i, 1);
+nr_ioapic_entries[i] = reg_01.bits.entries + 1;
+nr_irqs_gsi += nr_ioapic_entries[i];
+
+if ( rangeset_add_singleton(mmio_ro_ranges,
+ioapic_phys >> PAGE_SHIFT) )
+printk(KERN_ERR "Failed to mark IO-APIC page %lx read-only\n",
+   ioapic_phys);
+}
+}
+
+void __init init_ioapic(void)
+{
+if ( smp_found_config )
+{
+nr_irqs_gsi = 0;
+init_ioapic_mappings();
 }
 
 nr_irqs_gsi = max(nr_irqs_gsi, highest_gsi() + 1);
-- 
2.17.1




[Xen-devel] [PATCH 1/3] xen/x86: ioapic: Use true/false in bad_ioapic_register()

2020-03-27 Thread Julien Grall
From: Julien Grall 

bad_ioapic_register() is return a bool, so we should switch to
true/false.

Signed-off-by: Julien Grall 
---
 xen/arch/x86/io_apic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index e98e08e9c8..9868933287 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2531,10 +2531,10 @@ static __init bool bad_ioapic_register(unsigned int idx)
 {
 printk(KERN_WARNING "I/O APIC %#x registers return all ones, 
skipping!\n",
mp_ioapics[idx].mpc_apicaddr);
-return 1;
+return true;
 }
 
-return 0;
+return false;
 }
 
 void __init init_ioapic_mappings(void)
-- 
2.17.1




[Xen-devel] [PATCH 4/5] common/domain: add a domain context record for shared_info...

2020-03-27 Thread Paul Durrant
... and update xen-ctx to dump some information describing the record.

NOTE: To allow a sensible definition of the record in public/save.h
  this patch also adds a definition of the Xen ABI's de-facto page
  size into public/xen.h.

Signed-off-by: Paul Durrant 
---
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Stefano Stabellini 
---
 tools/misc/xen-ctx.c  |  8 ++
 xen/common/domain.c   | 55 +++
 xen/include/public/save.h | 10 ++-
 xen/include/public/xen.h  |  3 +++
 4 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/tools/misc/xen-ctx.c b/tools/misc/xen-ctx.c
index c31dd5d8e9..8f9692843b 100644
--- a/tools/misc/xen-ctx.c
+++ b/tools/misc/xen-ctx.c
@@ -57,6 +57,13 @@ static void dump_header(void)
h.magic, h.version);
 }
 
+static void dump_shared_info(void)
+{
+DOMAIN_SAVE_TYPE(SHARED_INFO) s;
+READ(s);
+printf("SHARED_INFO: field_width %u\n", s.field_width);
+}
+
 static void dump_end(void)
 {
 DOMAIN_SAVE_TYPE(END) e;
@@ -124,6 +131,7 @@ int main(int argc, char **argv)
 switch (desc.typecode)
 {
 case DOMAIN_SAVE_CODE(HEADER): dump_header(); break;
+case DOMAIN_SAVE_CODE(SHARED_INFO): dump_shared_info(); break;
 case DOMAIN_SAVE_CODE(END): dump_end(); return 0;
 default:
 printf("Unknown type %u: skipping\n", desc.typecode);
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 3dcd73f67c..484f6bde13 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1646,6 +1647,60 @@ int continue_hypercall_on_cpu(
 return 0;
 }
 
+static int save_shared_info(const struct vcpu *v, struct domain_context *c,
+bool dry_run)
+{
+struct domain *d = v->domain;
+struct domain_shared_info_context ctxt = {};
+
+if ( !dry_run )
+{
+memcpy(ctxt.buffer, d->shared_info, PAGE_SIZE);
+ctxt.field_width = has_32bit_shinfo(d) ? 4 : 8;
+}
+
+return DOMAIN_SAVE_ENTRY(SHARED_INFO, c, v, , sizeof(ctxt));
+}
+
+static int load_shared_info(struct vcpu *v, struct domain_context *c)
+{
+struct domain *d = v->domain;
+struct domain_shared_info_context ctxt;
+unsigned int i;
+int rc;
+
+rc = DOMAIN_LOAD_ENTRY(SHARED_INFO, c, v, , sizeof(ctxt), true);
+if ( rc )
+return rc;
+
+for ( i = 0; i < ARRAY_SIZE(ctxt.pad); i++ )
+if ( ctxt.pad[i] )
+return -EINVAL;
+
+switch ( ctxt.field_width )
+{
+case 4:
+d->arch.has_32bit_shinfo = 1;
+break;
+
+case 8:
+d->arch.has_32bit_shinfo = 0;
+break;
+
+default:
+rc = -EINVAL;
+break;
+}
+
+if ( !rc )
+memcpy(d->shared_info, ctxt.buffer, PAGE_SIZE);
+
+return rc;
+}
+
+DOMAIN_REGISTER_SAVE_RESTORE(SHARED_INFO, false, save_shared_info,
+ load_shared_info);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/public/save.h b/xen/include/public/save.h
index 84981cd0f6..ff804a7dbf 100644
--- a/xen/include/public/save.h
+++ b/xen/include/public/save.h
@@ -69,6 +69,14 @@ struct domain_save_header {
 };
 DECLARE_DOMAIN_SAVE_TYPE(HEADER, 1, struct domain_save_header);
 
-#define DOMAIN_SAVE_CODE_MAX 1
+struct domain_shared_info_context {
+uint8_t buffer[XEN_PAGE_SIZE];
+uint8_t field_width;
+uint8_t pad[7];
+};
+
+DECLARE_DOMAIN_SAVE_TYPE(SHARED_INFO, 2, struct domain_shared_info_context);
+
+#define DOMAIN_SAVE_CODE_MAX 2
 
 #endif /* __XEN_PUBLIC_SAVE_H__ */
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 75b1619d0d..cbf603f289 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -37,6 +37,9 @@
 #error "Unsupported architecture"
 #endif
 
+/* The Xen ABI assumes a page size of 4k. */
+#define XEN_PAGE_SIZE 4096
+
 #ifndef __ASSEMBLY__
 /* Guest handles for primitive C types. */
 DEFINE_XEN_GUEST_HANDLE(char);
-- 
2.20.1




[Xen-devel] [PATCH 5/5] tools/libxc: make use of domain context SHARED_INFO record...

2020-03-27 Thread Paul Durrant
... in the save/restore code.

This patch replaces direct mapping of the shared_info_frame (retrieved
using XEN_DOMCTL_getdomaininfo) with save/load of the domain context
SHARED_INFO record.

No modifications are made to the definition of the migration stream at
this point. Subsequent patches will define a record in the libxc domain
image format for passing domain context and convert the save/restore code
to use that.

Signed-off-by: Paul Durrant 
---
Cc: Ian Jackson 
Cc: Wei Liu 
---
 tools/libxc/xc_sr_common.h |  7 +++-
 tools/libxc/xc_sr_common_x86.c | 58 ++
 tools/libxc/xc_sr_common_x86.h |  4 +++
 tools/libxc/xc_sr_common_x86_pv.c  | 52 +++
 tools/libxc/xc_sr_common_x86_pv.h  |  3 ++
 tools/libxc/xc_sr_restore_x86_pv.c | 40 -
 tools/libxc/xc_sr_save_x86_pv.c| 26 ++
 tools/libxc/xg_save_restore.h  |  1 +
 8 files changed, 142 insertions(+), 49 deletions(-)

diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 5dd51ccb15..df21b46dc7 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -287,6 +287,11 @@ struct xc_sr_context
 {
 struct /* x86 */
 {
+struct {
+void *buffer;
+unsigned int len;
+} domain_context;
+
 struct /* x86 PV guest. */
 {
 /* 4 or 8; 32 or 64 bit domain */
@@ -314,7 +319,7 @@ struct xc_sr_context
 /* The guest pfns containing the p2m leaves */
 xen_pfn_t *p2m_pfns;
 
-/* Read-only mapping of guests shared info page */
+/* Pointer to shared_info (located in context buffer)  */
 shared_info_any_t *shinfo;
 
 /* p2m generation count for verifying validity of local p2m. */
diff --git a/tools/libxc/xc_sr_common_x86.c b/tools/libxc/xc_sr_common_x86.c
index 011684df97..7d297b75b5 100644
--- a/tools/libxc/xc_sr_common_x86.c
+++ b/tools/libxc/xc_sr_common_x86.c
@@ -42,6 +42,64 @@ int handle_x86_tsc_info(struct xc_sr_context *ctx, struct 
xc_sr_record *rec)
 return 0;
 }
 
+int x86_get_context(struct xc_sr_context *ctx, uint32_t mask)
+{
+xc_interface *xch = ctx->xch;
+int rc;
+
+if ( ctx->x86.domain_context.buffer )
+{
+ERROR("Domain context already present");
+return -1;
+}
+
+rc = xc_domain_getcontext(xch, ctx->domid, mask, NULL, 0);
+if ( rc < 0 )
+{
+PERROR("Unable to get size of domain context");
+return -1;
+}
+
+ctx->x86.domain_context.buffer = malloc(rc);
+if ( ctx->x86.domain_context.buffer == NULL )
+{
+PERROR("Unable to allocate memory for domain context");
+return -1;
+}
+
+rc = xc_domain_getcontext(xch, ctx->domid, mask,
+  ctx->x86.domain_context.buffer, rc);
+if ( rc < 0 )
+{
+PERROR("Unable to get domain context");
+return -1;
+}
+
+ctx->x86.domain_context.len = rc;
+
+return 0;
+}
+
+int x86_set_context(struct xc_sr_context *ctx, uint32_t mask)
+{
+xc_interface *xch = ctx->xch;
+
+if ( !ctx->x86.domain_context.buffer )
+{
+ERROR("Domain context not present");
+return -1;
+}
+
+return xc_domain_setcontext(xch, ctx->domid, mask,
+ctx->x86.domain_context.buffer,
+ctx->x86.domain_context.len);
+}
+
+void x86_cleanup(struct xc_sr_context *ctx)
+{
+free(ctx->x86.domain_context.buffer);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxc/xc_sr_common_x86.h b/tools/libxc/xc_sr_common_x86.h
index ebc4355bd1..1746094081 100644
--- a/tools/libxc/xc_sr_common_x86.h
+++ b/tools/libxc/xc_sr_common_x86.h
@@ -14,6 +14,10 @@ int write_x86_tsc_info(struct xc_sr_context *ctx);
  */
 int handle_x86_tsc_info(struct xc_sr_context *ctx, struct xc_sr_record *rec);
 
+int x86_get_context(struct xc_sr_context *ctx, uint32_t mask);
+int x86_set_context(struct xc_sr_context *ctx, uint32_t mask);
+void x86_cleanup(struct xc_sr_context *ctx);
+
 #endif
 /*
  * Local variables:
diff --git a/tools/libxc/xc_sr_common_x86_pv.c 
b/tools/libxc/xc_sr_common_x86_pv.c
index d3d425cb82..3e6f130e56 100644
--- a/tools/libxc/xc_sr_common_x86_pv.c
+++ b/tools/libxc/xc_sr_common_x86_pv.c
@@ -182,6 +182,58 @@ int x86_pv_map_m2p(struct xc_sr_context *ctx)
 return rc;
 }
 
+int x86_pv_get_shinfo(struct xc_sr_context *ctx)
+{
+unsigned int off = 0;
+struct domain_save_descriptor *desc;
+int rc;
+
+rc = x86_get_context(ctx, DOMAIN_SAVE_MASK(SHARED_INFO));
+if ( rc )
+return rc;
+
+do {
+if ( ctx->x86.domain_context.len - off < sizeof(*desc) )
+{
+return -1;
+}
+desc = ctx->x86.domain_context.buffer + off;
+off += sizeof(*desc);
+
+switch (desc->typecode)
+{
+case 

[Xen-devel] [PATCH 0/5] domain context infrastructure

2020-03-27 Thread Paul Durrant
Paul Durrant (5):
  xen/common: introduce a new framework for save/restore of 'domain'
context
  xen/common/domctl: introduce XEN_DOMCTL_get/setdomaincontext
  tools/misc: add xen-ctx to present domain context
  common/domain: add a domain context record for shared_info...
  tools/libxc: make use of domain context SHARED_INFO record...

 .gitignore  |   1 +
 tools/flask/policy/modules/xen.if   |   4 +-
 tools/libxc/include/xenctrl.h   |  11 ++
 tools/libxc/xc_domain.c |  52 ++
 tools/libxc/xc_sr_common.h  |   7 +-
 tools/libxc/xc_sr_common_x86.c  |  58 ++
 tools/libxc/xc_sr_common_x86.h  |   4 +
 tools/libxc/xc_sr_common_x86_pv.c   |  52 ++
 tools/libxc/xc_sr_common_x86_pv.h   |   3 +
 tools/libxc/xc_sr_restore_x86_pv.c  |  40 ++---
 tools/libxc/xc_sr_save_x86_pv.c |  26 +--
 tools/libxc/xg_save_restore.h   |   1 +
 tools/misc/Makefile |   4 +
 tools/misc/xen-ctx.c| 152 
 xen/common/Makefile |   1 +
 xen/common/domain.c |  55 ++
 xen/common/domctl.c | 115 
 xen/common/save.c   | 262 
 xen/include/public/domctl.h |  41 -
 xen/include/public/save.h   |  82 +
 xen/include/public/xen.h|   3 +
 xen/include/xen/save.h  | 115 
 xen/xsm/flask/hooks.c   |   6 +
 xen/xsm/flask/policy/access_vectors |   4 +
 24 files changed, 1047 insertions(+), 52 deletions(-)
 create mode 100644 tools/misc/xen-ctx.c
 create mode 100644 xen/common/save.c
 create mode 100644 xen/include/public/save.h
 create mode 100644 xen/include/xen/save.h
---
Cc: Andrew Cooper 
Cc: Daniel De Graaf 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Stefano Stabellini 
Cc: Wei Liu 
-- 
2.20.1




[Xen-devel] [PATCH 2/5] xen/common/domctl: introduce XEN_DOMCTL_get/setdomaincontext

2020-03-27 Thread Paul Durrant
These domctls provide a mechanism to get and set domain context from
the toolstack.

Signed-off-by: Paul Durrant 
---
Cc: Daniel De Graaf 
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Stefano Stabellini 
---
 tools/flask/policy/modules/xen.if   |   4 +-
 tools/libxc/include/xenctrl.h   |  11 +++
 tools/libxc/xc_domain.c |  52 +
 xen/common/domctl.c | 115 
 xen/include/public/domctl.h |  41 +-
 xen/xsm/flask/hooks.c   |   6 ++
 xen/xsm/flask/policy/access_vectors |   4 +
 7 files changed, 230 insertions(+), 3 deletions(-)

diff --git a/tools/flask/policy/modules/xen.if 
b/tools/flask/policy/modules/xen.if
index 8eb2293a52..2bc9db4f64 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -53,7 +53,7 @@ define(`create_domain_common', `
allow $1 $2:domain2 { set_cpu_policy settsc setscheduler setclaim
set_vnumainfo get_vnumainfo cacheflush
psr_cmt_op psr_alloc soft_reset
-   resource_map get_cpu_policy };
+   resource_map get_cpu_policy setcontext };
allow $1 $2:security check_context;
allow $1 $2:shadow enable;
allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage 
mmuext_op updatemp };
@@ -97,7 +97,7 @@ define(`migrate_domain_out', `
allow $1 $2:hvm { gethvmc getparam };
allow $1 $2:mmu { stat pageinfo map_read };
allow $1 $2:domain { getaddrsize getvcpucontext pause destroy };
-   allow $1 $2:domain2 gettsc;
+   allow $1 $2:domain2 { gettsc getcontext };
allow $1 $2:shadow { enable disable logdirty };
 ')
 
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index fc6e57a1a0..5c0d0d27a4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -867,6 +867,17 @@ int xc_domain_hvm_setcontext(xc_interface *xch,
  uint8_t *hvm_ctxt,
  uint32_t size);
 
+int xc_domain_getcontext(xc_interface *xch,
+ uint32_t domid,
+ uint32_t mask,
+ uint8_t *ctxt_buf,
+ uint32_t size);
+int xc_domain_setcontext(xc_interface *xch,
+ uint32_t domid,
+ uint32_t mask,
+ uint8_t *ctxt_buf,
+ uint32_t size);
+
 /**
  * This function will return guest IO ABI protocol
  *
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 71829c2bce..15bcf671fc 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -537,6 +537,58 @@ int xc_domain_hvm_setcontext(xc_interface *xch,
 return ret;
 }
 
+int xc_domain_getcontext(xc_interface *xch,
+ uint32_t domid,
+ uint32_t mask,
+ uint8_t *ctxt_buf,
+ uint32_t size)
+{
+int ret;
+DECLARE_DOMCTL;
+DECLARE_HYPERCALL_BOUNCE(ctxt_buf, size, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
+
+if ( xc_hypercall_bounce_pre(xch, ctxt_buf) )
+return -1;
+
+domctl.cmd = XEN_DOMCTL_getdomaincontext;
+domctl.domain = domid;
+domctl.u.domaincontext.mask = mask;
+domctl.u.domaincontext.size = size;
+set_xen_guest_handle(domctl.u.domaincontext.buffer, ctxt_buf);
+
+ret = do_domctl(xch, );
+
+xc_hypercall_bounce_post(xch, ctxt_buf);
+
+return !ret ? domctl.u.domaincontext.size : -1;
+}
+
+int xc_domain_setcontext(xc_interface *xch,
+ uint32_t domid,
+ uint32_t mask,
+ uint8_t *ctxt_buf,
+ uint32_t size)
+{
+int ret;
+DECLARE_DOMCTL;
+DECLARE_HYPERCALL_BOUNCE(ctxt_buf, size, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+if ( xc_hypercall_bounce_pre(xch, ctxt_buf) )
+return -1;
+
+domctl.cmd = XEN_DOMCTL_setdomaincontext;
+domctl.domain = domid;
+domctl.u.domaincontext.mask = mask;
+domctl.u.domaincontext.size = size;
+set_xen_guest_handle(domctl.u.domaincontext.buffer, ctxt_buf);
+
+ret = do_domctl(xch, );
+
+xc_hypercall_bounce_post(xch, ctxt_buf);
+
+return ret;
+}
+
 int xc_vcpu_getcontext(xc_interface *xch,
uint32_t domid,
uint32_t vcpu,
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index a69b3b59a8..9c39519b04 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -358,6 +359,111 @@ static struct vnuma_info *vnuma_init(const struct 
xen_domctl_vnuma *uinfo,
 return ERR_PTR(ret);
 }
 
+struct domctl_context
+{
+void *buffer;
+size_t len;
+size_t cur;
+};
+
+static int 

[Xen-devel] [PATCH 3/5] tools/misc: add xen-ctx to present domain context

2020-03-27 Thread Paul Durrant
This tools is analogous to 'xen-hvmctx' which presents HVM context.
Subsequent patches will add 'dump' functions when new records are
introduced.

Signed-off-by: Paul Durrant 
---
Cc: Ian Jackson 
Cc: Wei Liu 
---
 .gitignore   |   1 +
 tools/misc/Makefile  |   4 ++
 tools/misc/xen-ctx.c | 144 +++
 3 files changed, 149 insertions(+)
 create mode 100644 tools/misc/xen-ctx.c

diff --git a/.gitignore b/.gitignore
index 4ca679ddbc..72b807141f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -206,6 +206,7 @@ tools/misc/cpuperf/cpuperf-xen
 tools/misc/xc_shadow
 tools/misc/xen_cpuperf
 tools/misc/xen-cpuid
+tools/misc/xen-ctx
 tools/misc/xen-detect
 tools/misc/xen-diag
 tools/misc/xen-tmem-list-parse
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index 63947bfadc..6347bb24e9 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -30,6 +30,7 @@ INSTALL_SBIN   += xenpm
 INSTALL_SBIN   += xenwatchdogd
 INSTALL_SBIN   += xen-livepatch
 INSTALL_SBIN   += xen-diag
+INSTALL_SBIN   += xen-ctx
 INSTALL_SBIN += $(INSTALL_SBIN-y)
 
 # Everything to be installed in a private bin/
@@ -108,6 +109,9 @@ xen-livepatch: xen-livepatch.o
 xen-diag: xen-diag.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
+xen-ctx: xen-ctx.o
+   $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+
 xen-lowmemd: xen-lowmemd.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl) 
$(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
 
diff --git a/tools/misc/xen-ctx.c b/tools/misc/xen-ctx.c
new file mode 100644
index 00..c31dd5d8e9
--- /dev/null
+++ b/tools/misc/xen-ctx.c
@@ -0,0 +1,144 @@
+/*
+ * xen-ctx.c
+ *
+ * Print out domain save records in a human-readable way.
+ *
+ * Copyright Amazon.com Inc. or its affiliates.
+ *
+ * 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+static void *buf = NULL;
+static size_t len, off;
+
+#define READ(_x) do {   \
+if ( len - off < sizeof (_x) )  \
+{   \
+fprintf(stderr, "Error: need another %lu bytes, only %lu available",\
+sizeof(_x), len - off); \
+exit(1);\
+}   \
+memcpy(&(_x), buf + off, sizeof (_x));  \
+off += sizeof (_x); \
+} while (0)
+
+static void dump_header(void)
+{
+DOMAIN_SAVE_TYPE(HEADER) h;
+READ(h);
+printf("HEADER: magic %#x, version %u\n",
+   h.magic, h.version);
+}
+
+static void dump_end(void)
+{
+DOMAIN_SAVE_TYPE(END) e;
+READ(e);
+printf("END\n");
+}
+
+int main(int argc, char **argv)
+{
+uint32_t domid;
+unsigned int entry;
+xc_interface *xch;
+int rc;
+
+if ( argc != 2 || !argv[1] || (rc = atoi(argv[1])) < 0 )
+{
+fprintf(stderr, "usage: %s \n", argv[0]);
+exit(1);
+}
+domid = rc;
+
+xch = xc_interface_open(0,0,0);
+if ( !xch )
+{
+fprintf(stderr, "Error: can't open libxc handle\n");
+exit(1);
+}
+
+rc = xc_domain_getcontext(xch, domid, 0, 0, 0);
+if ( rc < 0 )
+{
+fprintf(stderr, "Error: can't get record length for dom %u: %s\n",
+domid, strerror(errno));
+exit(1);
+}
+len = rc;
+
+buf = malloc(len);
+if ( !buf )
+{
+fprintf(stderr, "Error: can't allocate %lu bytes\n", len);
+exit(1);
+}
+
+rc = 

[Xen-devel] [PATCH 1/5] xen/common: introduce a new framework for save/restore of 'domain' context

2020-03-27 Thread Paul Durrant
Domain context is state held in the hypervisor that does not come under
the category of 'HVM state' but is instead 'PV state' that is common
between PV guests and enlightened HVM guests (i.e. those that have PV
drivers) such as event channel state, grant entry state, etc.

To allow enlightened HVM guests to be migrated without their co-operation
it will be necessary to transfer such state along with the domain's
memory image, architectural state, etc. This framework is introduced for
that purpose.

This patch adds the new public header and the low level implementation,
entered via the domain_save() or domain_load() functions. Subsequent
patches will introduce other parts of the framwork, and code that will
make use of it within the current version of the libxc migration stream.

Signed-off-by: Paul Durrant 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Stefano Stabellini 
Cc: Wei Liu 
---
 xen/common/Makefile   |   1 +
 xen/common/save.c | 262 ++
 xen/include/public/save.h |  74 +++
 xen/include/xen/save.h| 115 +
 4 files changed, 452 insertions(+)
 create mode 100644 xen/common/save.c
 create mode 100644 xen/include/public/save.h
 create mode 100644 xen/include/xen/save.h

diff --git a/xen/common/Makefile b/xen/common/Makefile
index e8cde65370..90553ba5d7 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -37,6 +37,7 @@ obj-y += radix-tree.o
 obj-y += rbtree.o
 obj-y += rcupdate.o
 obj-y += rwlock.o
+obj-y += save.o
 obj-y += shutdown.o
 obj-y += softirq.o
 obj-y += sort.o
diff --git a/xen/common/save.c b/xen/common/save.c
new file mode 100644
index 00..bef99452d8
--- /dev/null
+++ b/xen/common/save.c
@@ -0,0 +1,262 @@
+/*
+ * save.c: Save and restore PV guest state common to all domain types.
+ *
+ * Copyright Amazon.com Inc. or its affiliates.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see .
+ */
+
+#include 
+
+struct domain_context {
+bool log;
+struct domain_save_descriptor desc;
+domain_copy_entry copy;
+void *priv;
+};
+
+static struct {
+const char *name;
+bool per_vcpu;
+domain_save_handler save;
+domain_load_handler load;
+} handlers[DOMAIN_SAVE_CODE_MAX + 1];
+
+void __init domain_register_save_type(unsigned int tc, const char *name,
+  bool per_vcpu,
+  domain_save_handler save,
+  domain_load_handler load)
+{
+BUG_ON(tc > ARRAY_SIZE(handlers));
+
+ASSERT(!handlers[tc].save);
+ASSERT(!handlers[tc].load);
+
+handlers[tc].name = name;
+handlers[tc].per_vcpu = per_vcpu;
+handlers[tc].save = save;
+handlers[tc].load = load;
+}
+
+int domain_save_entry(struct domain_context *c, unsigned int tc,
+  const char *name, const struct vcpu *v, void *src,
+  size_t src_len)
+{
+int rc;
+
+if ( c->log && tc != DOMAIN_SAVE_CODE(HEADER) &&
+ tc != DOMAIN_SAVE_CODE(END) )
+gdprintk(XENLOG_INFO, "%pv save: %s (%lu)\n", v, name, src_len);
+
+if ( !IS_ALIGNED(src_len, 8) )
+return -EINVAL;
+
+BUG_ON(tc != c->desc.typecode);
+BUG_ON(v->vcpu_id != c->desc.instance);
+c->desc.length = src_len;
+
+rc = c->copy(c->priv, >desc, sizeof(c->desc));
+if ( rc )
+return rc;
+
+return c->copy(c->priv, src, src_len);
+}
+
+int domain_save(struct domain *d, domain_copy_entry copy, void *priv,
+unsigned long mask, bool dry_run)
+{
+struct domain_context c = {
+.copy = copy,
+.priv = priv,
+.log = !dry_run,
+};
+struct domain_save_header h = {
+.magic = DOMAIN_SAVE_MAGIC,
+.version = DOMAIN_SAVE_VERSION,
+};
+struct domain_save_header e;
+unsigned int i;
+int rc;
+
+ASSERT(d != current->domain);
+
+if ( d->is_dying )
+return -EINVAL;
+
+domain_pause(d);
+
+c.desc.typecode = DOMAIN_SAVE_CODE(HEADER);
+
+rc = DOMAIN_SAVE_ENTRY(HEADER, , d->vcpu[0], , sizeof(h));
+if ( rc )
+goto out;
+
+for ( i = 0; i < ARRAY_SIZE(handlers); i++ )
+{
+domain_save_handler save = handlers[i].save;
+
+if ( (mask && !test_bit(i, )) || !save )
+continue;
+
+memset(, 0, sizeof(c.desc));
+c.desc.typecode = i;
+
+

[Xen-devel] [OSSTEST PATCH] ts-examine-hostprops-save: Save for commissioning flights too (!)

2020-03-27 Thread Ian Jackson
Signed-off-by: Ian Jackson 
---
 ts-examine-hostprops-save | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ts-examine-hostprops-save b/ts-examine-hostprops-save
index e50ea7fb..3995a7a7 100755
--- a/ts-examine-hostprops-save
+++ b/ts-examine-hostprops-save
@@ -31,8 +31,8 @@ logm("setting host properties and flags");
 
 # NB: in order to aid debug only attempt to save the host props on flights
 # with intended real blessing, for the rest just do a dry run.
-our $dry_run = $blessing ne "real";
-logm("not saving host props/flags with intended blessing $blessing != real")
+our $dry_run = $blessing !~ qr{^real$|^commission-};
+logm("not saving host props/flags with intended blessing $blessing")
 if $dry_run;
 
 foreach my $k (sort keys %r) {
-- 
2.11.0




Re: [Xen-devel] [PATCH v5 01/10] x86emul: support AVX512_BF16 insns

2020-03-27 Thread Andrew Cooper
On 24/03/2020 12:30, Jan Beulich wrote:
> --- a/tools/tests/x86_emulator/evex-disp8.c
> +++ b/tools/tests/x86_emulator/evex-disp8.c
> @@ -550,6 +550,12 @@ static const struct test avx512_4vnniw_5
>  INSN(p4dpwssds, f2, 0f38, 53, el_4, d, vl),
>  };
>  
> +static const struct test avx512_bf16_all[] = {
> +INSN(vcvtne2ps2bf16, f2, 0f38, 72, vl, d, vl),
> +INSN(vcvtneps2bf16,  f3, 0f38, 72, vl, d, vl),
> +INSN(vdpbf16ps,  f3, 0f38, 52, vl, d, vl),
> +};
> +
>  static const struct test avx512_bitalg_all[] = {
>  INSN(popcnt,  66, 0f38, 54, vl, bw, vl),
>  INSN(pshufbitqmb, 66, 0f38, 8f, vl,  b, vl),
> @@ -984,6 +990,7 @@ void evex_disp8_test(void *instr, struct
>  RUN(avx512pf, 512);
>  RUN(avx512_4fmaps, 512);
>  RUN(avx512_4vnniw, 512);
> +RUN(avx512_bf16, all);
>  RUN(avx512_bitalg, all);
>  RUN(avx512_ifma, all);
>  RUN(avx512_vbmi, all);
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -4516,6 +4516,80 @@ int main(int argc, char **argv)
>  else
>  printf("skipped\n");
>  
> +if ( stack_exec && cpu_has_avx512_bf16 )
> +{
> +decl_insn(vcvtne2ps2bf16);
> +decl_insn(vcvtneps2bf16);
> +decl_insn(vdpbf16ps);
> +static const struct {
> +float f[16];
> +} in1 = {{
> +1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16
> +}}, in2 = {{
> +1, -2, 3, -4, 5, -6, 7, -8, 9, -10, 11, -12, 13, -14, 15, -16
> +}}, out = {{
> +1 * 1 + 2 * 2, 3 * 3 + 4 * 4,
> +5 * 5 + 6 * 6, 7 * 7 + 8 * 8,
> +9 * 9 + 10 * 10, 11 * 11 + 12 * 12,
> +13 * 13 + 14 * 14, 15 * 15 + 16 * 16,
> +1 * 1 - 2 * 2, 3 * 3 - 4 * 4,
> +5 * 5 - 6 * 6, 7 * 7 - 8 * 8,
> +9 * 9 - 10 * 10, 11 * 11 - 12 * 12,
> +13 * 13 - 14 * 14, 15 * 15 - 16 * 16
> +}};
> +
> +printf("%-40s", "Testing vcvtne2ps2bf16 64(%ecx),%zmm1,%zmm2...");
> +asm volatile ( "vmovups %1, %%zmm1\n"
> +   put_insn(vcvtne2ps2bf16,
> +/* vcvtne2ps2bf16 64(%0), %%zmm1, %%zmm2 */
> +".byte 0x62, 0xf2, 0x77, 0x48, 0x72, 0x51, 
> 0x01")
> +   :: "c" (NULL), "m" (in2) );
> +set_insn(vcvtne2ps2bf16);
> +regs.ecx = (unsigned long) - 64;
> +rc = x86_emulate(, );
> +if ( rc != X86EMUL_OKAY || !check_eip(vcvtne2ps2bf16) )
> +goto fail;
> +printf("pending\n");
> +
> +printf("%-40s", "Testing vcvtneps2bf16 64(%ecx),%ymm3...");
> +asm volatile ( put_insn(vcvtneps2bf16,
> +/* vcvtneps2bf16 64(%0), %%ymm3 */
> +".byte 0x62, 0xf2, 0x7e, 0x48, 0x72, 0x59, 
> 0x01")
> +   :: "c" (NULL) );
> +set_insn(vcvtneps2bf16);
> +rc = x86_emulate(, );
> +if ( rc != X86EMUL_OKAY || !check_eip(vcvtneps2bf16) )
> +goto fail;
> +asm ( "vmovdqa %%ymm2, %%ymm5\n\t"
> +  "vpcmpeqd %%zmm3, %%zmm5, %%k0\n\t"
> +  "kmovw %%k0, %0"
> +  : "=g" (rc) : "m" (out) );
> +if ( rc != 0x )
> +goto fail;
> +printf("pending\n");
> +
> +printf("%-40s", "Testing vdpbf16ps 128(%ecx),%zmm2,%zmm4...");
> +asm volatile ( "vmovdqa %%ymm3, %0\n\t"
> +   "vmovdqa %%ymm3, %1\n"
> +   put_insn(vdpbf16ps,
> +/* vdpbf16ps 128(%2), %%zmm2, %%zmm4 */
> +".byte 0x62, 0xf2, 0x6e, 0x48, 0x52, 0x61, 
> 0x02")
> +   : "=" (res[0]), "=" (res[8])
> +   : "c" (NULL)
> +   : "memory" );
> +set_insn(vdpbf16ps);
> +regs.ecx = (unsigned long)res - 128;
> +rc = x86_emulate(, );
> +if ( rc != X86EMUL_OKAY || !check_eip(vdpbf16ps) )
> +goto fail;
> +asm ( "vcmpeqps %1, %%zmm4, %%k0\n\t"
> +  "kmovw %%k0, %0"
> +  : "=g" (rc) : "m" (out) );
> +if ( rc != 0x )
> +goto fail;
> +printf("okay\n");
> +}

I've just tried this out on an SDP.

Testing vcvtne2ps2bf16 64(%ecx),%zmm1,%zmm2...pending
Testing vcvtneps2bf16 64(%ecx),%ymm3... pending
Testing vdpbf16ps 128(%ecx),%zmm2,%zmm4...okay
...
Testing avx512_bf16/all disp8 handling...okay

What is the "pending" supposed to signify?  I can see that these three
are linked, and that is fine, but at the point we've checked the
intermediate results, it should be "okay", no?

~Andrew



[Xen-devel] [OSSTEST PATCH 6/6] ts-logs-capture: Fish some logs out of guest filesystem

2020-03-27 Thread Ian Jackson
This involves shutting the guests down.  We use this shell rune
because xl doesn't provide a good way to ensure there are no guests
running.

Signed-off-by: Ian Jackson 
---
 ts-logs-capture | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/ts-logs-capture b/ts-logs-capture
index 6be77676..c67856cd 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -261,10 +261,40 @@ sub fetch_logs_guest ($) {
 }
 }
 
+sub shutdown_guests () {
+target_cmd_root($ho, <<'END', 180);
+set -x
+(
+( exec 2>/dev/null; sleep 30 ; echo y ) &
+( xl shutdown -a -F -w ; echo y ) &
+) | (
+read x
+xl list | awk '!/^Domain-0 |^Name / {print $2}' \
+| xargs -t -r -n1 xl destroy ||:
+)
+END
+}
+
+sub extract_logs_guest ($) {
+my ($gho) = @_;
+if (!eval {
+   guest_find_lv($gho);
+   target_cmd_root($ho, "umount /mnt ||:");
+   target_cmd_root($ho, "mount -r ".$gho->{Lvdev}." /mnt");
+   try_fetch_logs($ho, \@general_logs, '/mnt', "$gho->{Guest}-");
+   target_cmd_root($ho, "umount /mnt ||:");
+   1;
+}) {
+   logm("failure extracting logs out of guest fs: $@");
+}
+}
+
 power_state($ho,1);
 find_guests();
 fetch_xenctx_guest($_) foreach @guests;
 serial_fetch_logs($ho);
 fetch_logs_host();
 fetch_logs_guest($_) foreach @guests;
+shutdown_guests();
+extract_logs_guest($_) foreach @allguests;
 logm("logs captured to $stash");
-- 
2.11.0




[Xen-devel] [OSSTEST PATCH 3/6] ts-logs-capture: Break logs up into general logs and host logs

2020-03-27 Thread Ian Jackson
We are going to fetch logs out of guests.  @general_logs will contain
the relevant patterns.  Right now we just introduce the variable and
split the list.  The categorisation is roughly right...

Signed-off-by: Ian Jackson 
---
 ts-logs-capture | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/ts-logs-capture b/ts-logs-capture
index 88b19658..ae37d492 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -135,8 +135,7 @@ sub try_cmd_output_save ($;$) {
 close $fh or die $!;
 }
 
-sub fetch_logs_host () {
-my $logs= [qw(
+our @general_logs = qw(
   /var/log/kern.log*
   /var/log/syslog*
   /var/log/daemon.log*
@@ -149,6 +148,10 @@ sub fetch_logs_host () {
   /var/log/installer/syslog*
   /var/log/installer/partman*
 
+);
+
+sub fetch_logs_host () {
+my $logs= [@general_logs, qw(
   /var/log/xen/xend.log*
   /var/log/xen/xend-debug.log*
   /var/log/xen/xen-hotplug.log*
-- 
2.11.0




[Xen-devel] [OSSTEST PATCH] README.dev: Suggest -P for commissioning flights

2020-03-27 Thread Ian Jackson
Signed-off-by: Ian Jackson 
---
 README.dev | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/README.dev b/README.dev
index e32889b7..2cbca109 100644
--- a/README.dev
+++ b/README.dev
@@ -115,7 +115,7 @@ and boot Xen:
  $ hn=mudcake
  $ flight=`./make-hosts-flight play xen-unstable blessed-commission-$hn 
commission-$hn $basis`; echo $flight
  113155
- $ ./mg-execute-flight -Bcommission-$hn -eian.jack...@citrix.com $flight
+ $ ./mg-execute-flight -P -Bcommission-$hn -eian.jack...@citrix.com $flight
 
 This will email the specified address.  The examination should pass,
 completely.  If it does not then you may need to change the BIOS
@@ -132,7 +132,7 @@ If that works, a more thorough test:
 
  $ basis=113124   # pick last good xen-unstable or osstest flight
  $ flight=`./cs-adjust-flight new:commission-$hn copy $basis`; echo $flight
- $ ./mg-execute-flight -Bcommission-$hn -eian.jack...@citrix.com -f$basis 
$flight
+ $ ./mg-execute-flight -P -Bcommission-$hn -eian.jack...@citrix.com -f$basis 
$flight
 
 This should show no regressions.  (Or, at least, none that are a cause
 for concern.)
-- 
2.11.0




[Xen-devel] [OSSTEST PATCH 4/6] ts-logs-capture: Move some general logs onto @general_logs

2020-03-27 Thread Ian Jackson
Now @general_logs contains logs we want from guests as well as hosts.

Signed-off-by: Ian Jackson 
---
 ts-logs-capture | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/ts-logs-capture b/ts-logs-capture
index ae37d492..418155ce 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -148,6 +148,11 @@ our @general_logs = qw(
   /var/log/installer/syslog*
   /var/log/installer/partman*
 
+  /boot/config*
+
+  /home/osstest/osstest-confirm-booted.log
+
+  /var/core/*.core
 );
 
 sub fetch_logs_host () {
@@ -172,14 +177,8 @@ sub fetch_logs_host () {
 
   /var/log/xen-tools/*
 
-  /boot/config*
-
   /etc/xen/*
 
-  /home/osstest/osstest-confirm-booted.log
-
-  /var/core/*.core
-
   )];
 if (!try_fetch_logs($ho, $logs)) {
 logm("log fetching failed, trying hard host reboot...");
-- 
2.11.0




[Xen-devel] [OSSTEST PATCH 5/6] ts-logs-capture: try_fetch_logs: Honour $fs_ and $out_prefix

2020-03-27 Thread Ian Jackson
This allows us to add some stuff to add to each pattern, and each
filename.  This will be useful in a moment.

None of the call sites pass this yet.

Signed-off-by: Ian Jackson 
---
 ts-logs-capture | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/ts-logs-capture b/ts-logs-capture
index 418155ce..6be77676 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -68,11 +68,13 @@ END
 @guests = grep { defined $_->{Domid} } @allguests;
 }
 
-sub try_fetch_logs ($$) {
-my ($lho, $logfilepats) = @_;
+sub try_fetch_logs ($$;$$) {
+my ($lho, $logfilepats, $fs_prefix,$out_prefix) = @_;
+$fs_prefix //= '';
+$out_prefix //= '';
 my $ok= 0;
 foreach my $logfilepat (@$logfilepats) {
-my $logfileslist= $logfilepat;
+my $logfileslist= $fs_prefix.$logfilepat;
 if ($logfileslist =~ m/[*?]/) {
 if (!eval {
 $logfileslist=
@@ -91,8 +93,9 @@ END
 }
 foreach my $logfile (split / /, $logfileslist) {
 my $llogfile= $logfile;
+   $llogfile =~ s,^\Q$fs_prefix\E,,;
 $llogfile =~ s,/,-,g;
-$llogfile= hostnamepath($lho)."--$llogfile";
+$llogfile= hostnamepath($lho)."-$out_prefix-$llogfile";
 logm("fetching $logfile to $llogfile");
 if (!eval {
 target_getfile_root($lho,60, $logfile,"$stash/$llogfile");
-- 
2.11.0




[Xen-devel] [OSSTEST PATCH 2/6] ts-logs-capture: Introduce @allguests containing even non-running

2020-03-27 Thread Ian Jackson
Nothing looks at this yet.

Signed-off-by: Ian Jackson 
---
 ts-logs-capture | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ts-logs-capture b/ts-logs-capture
index d16372f2..88b19658 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -39,7 +39,7 @@ if (!$ho) {
 exit 0;
 }
 
-our @guests;
+our (@allguests, @guests);
 
 sub find_guests () {
 my $sth= $dbh_tests->prepare({Domid} } @allguests;
 }
 
 sub try_fetch_logs ($$) {
-- 
2.11.0




[Xen-devel] [OSSTEST PATCH 1/6] TestSupport: export guest_find_lv

2020-03-27 Thread Ian Jackson
We'll need this in a moment.

Signed-off-by: Ian Jackson 
---
 Osstest/TestSupport.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 1c13e2af..5fb78468 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -114,7 +114,7 @@ BEGIN {
   prepareguest_part_xencfg
   guest_umount_lv guest_await guest_await_dhcp_tcp
   guest_checkrunning $guest_state_running_re
-  target_check_ip guest_find_ether
+  target_check_ip guest_find_ether guest_find_lv
   guest_find_domid guest_check_up guest_check_up_quick
   guest_get_state guest_await_reboot
   guest_await_shutdown guest_await_destroy guest_destroy
-- 
2.11.0




[Xen-devel] [libvirt test] 149074: regressions - FAIL

2020-03-27 Thread osstest service owner
flight 149074 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/149074/

Regressions :-(

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

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

version targeted for testing:
 libvirt  0fb83c33369667d9e8e7b85a61cfbff6e73a70f8
baseline version:
 libvirt  a1cd25b919509be2645dbe6f952d5263e0d4e4e5

Last test of basis   146182  2020-01-17 06:00:23 Z   70 days
Failing since146211  2020-01-18 04:18:52 Z   69 days   66 attempts
Testing same since   149074  2020-03-27 04:18:45 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Arnaud Patard 
  Boris Fiuczynski 
  Christian Ehrhardt 
  Collin Walling 
  Daniel Henrique Barboza 
  Daniel P. Berrangé 
  Daniel Veillard 
  Dario Faggioli 
  Erik Skultety 
  Gaurav Agrawal 
  Han Han 
  Jim Fehlig 
  Jiri Denemark 
  Jonathon Jongsma 
  Julio Faracco 
  Ján Tomko 
  Laine Stump 
  Lin Ma 
  Marc-André Lureau 
  Marek Marczykowski-Górecki 
  Mauro S. M. Rodrigues 
  Michal Privoznik 
  Nikolay Shirokovskiy 
  Pavel Hrdina 
  Pavel Mores 
  Peter Krempa 
  Pino Toscano 
  Rafael Fonseca 
  Richard W.M. Jones 
  Rikard Falkeborn 
  Ryan Moeller 
  Sahid Orentino Ferdjaoui 
  Sebastian Mitterle 
  Seeteena Thoufeek 
  Stefan Berger 
  Stefan Berger 
  Stefan Hajnoczi 
  Thomas Huth 
  Wu Qingliang 
  Your Name 
  Zhang Bo 
  zhenwei pi 
  Zhimin Feng 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  fail
 build-arm64-libvirt  fail
 build-armhf-libvirt  fail
 build-i386-libvirt   fail
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   blocked 
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked 
 test-amd64-amd64-libvirt-xsm blocked 
 test-arm64-arm64-libvirt-xsm blocked 
 test-amd64-i386-libvirt-xsm  blocked 
 test-amd64-amd64-libvirt blocked 
 test-arm64-arm64-libvirt blocked 
 test-armhf-armhf-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-libvirt-pairblocked 
 test-amd64-i386-libvirt-pair blocked 
 test-arm64-arm64-libvirt-qcow2   blocked 
 test-armhf-armhf-libvirt-raw blocked 
 test-amd64-amd64-libvirt-vhd blocked 



[Xen-devel] [xen-unstable-smoke test] 149096: tolerable all pass - PUSHED

2020-03-27 Thread osstest service owner
flight 149096 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/149096/

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  fe746c26c0d23c61dbc7eb1918addb1c9a3729bf
baseline version:
 xen  c72154e88c6c177ce9b9ec3c5388b5bfcce42f34

Last test of basis   149087  2020-03-27 11:02:08 Z0 days
Testing same since   149096  2020-03-27 14:00:46 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Julien Grall 
  Roger Pau Monne 
  Roger Pau Monné 
  Wei Liu 

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
   c72154e88c..fe746c26c0  fe746c26c0d23c61dbc7eb1918addb1c9a3729bf -> smoke



Re: [Xen-devel] [PATCH v8 2/2] docs/designs: Add a design document for migration of xenstore data

2020-03-27 Thread Paul Durrant
> -Original Message-
> From: Julien Grall 
> Sent: 27 March 2020 16:58
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Paul Durrant ; Andrew Cooper 
> ; George Dunlap
> ; Ian Jackson ; Jan 
> Beulich
> ; Konrad Rzeszutek Wilk ; Stefano 
> Stabellini
> ; Wei Liu 
> Subject: Re: [PATCH v8 2/2] docs/designs: Add a design document for migration 
> of xenstore data
> 
> 
> 
> On 27/03/2020 13:46, Paul Durrant wrote:
> > +The semantics of this are similar to the domain issuing
> > +TRANSACTION_START and receiving the specified  as the response.
> > +The main difference is that the transaction will be immediately marked as
> > +'conflicting' such that when the domain isses TRANSACTION_END T|, it will
> 
> NIT: s/isses/issues/

Oh yes.

> 
> Acked-by: Julien Grall 
> 
> I can fix the typo while committing the patch series.
> 

Thanks,

  Paul

> Cheers,
> 
> --
> Julien Grall




Re: [Xen-devel] [PATCH v8 2/2] docs/designs: Add a design document for migration of xenstore data

2020-03-27 Thread Julien Grall




On 27/03/2020 13:46, Paul Durrant wrote:

+The semantics of this are similar to the domain issuing
+TRANSACTION_START and receiving the specified  as the response.
+The main difference is that the transaction will be immediately marked as
+'conflicting' such that when the domain isses TRANSACTION_END T|, it will


NIT: s/isses/issues/

Acked-by: Julien Grall 

I can fix the typo while committing the patch series.

Cheers,

--
Julien Grall



Re: [Xen-devel] [PATCH v8 1/2] docs/designs: Add a design document for non-cooperative live migration

2020-03-27 Thread Julien Grall

Hi Paul,

On 27/03/2020 13:46, Paul Durrant wrote:

From: Paul Durrant 

It has become apparent to some large cloud providers that the current
model of cooperative migration of guests under Xen is not usable as it
relies on software running inside the guest, which is likely beyond the
provider's control.
This patch introduces a proposal for non-cooperative live migration,
designed not to rely on any guest-side software.

Signed-off-by: Paul Durrant 


Acked-by: Julien Grall 

Cheers,


---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Wei Liu 

v8:
  - Addressed comments from Julien on v6 that I missed

v6:
  - Addressed comments from Julien

v5:
  - Note that PV domain are not just expected to co-operate, they are
required to

v4:
  - Fix issues raised by Wei

v2:
  - Use the term 'non-cooperative' instead of 'transparent'
  - Replace 'trust in' with 'reliance on' when referring to guest-side
software
---
  docs/designs/non-cooperative-migration.md | 280 ++
  1 file changed, 280 insertions(+)
  create mode 100644 docs/designs/non-cooperative-migration.md

diff --git a/docs/designs/non-cooperative-migration.md 
b/docs/designs/non-cooperative-migration.md
new file mode 100644
index 00..4b876d809f
--- /dev/null
+++ b/docs/designs/non-cooperative-migration.md
@@ -0,0 +1,280 @@
+# Non-Cooperative Migration of Guests on Xen
+
+## Background
+
+The normal model of migration in Xen is driven by the guest because it was
+originally implemented for PV guests, where the guest must be aware it is
+running under Xen and is hence expected to co-operate. This model dates from
+an era when it was assumed that the host administrator had control of at
+least the privileged software running in the guest (i.e. the guest kernel)
+which may still be true in an enterprise deployment but is not generally
+true in a cloud environment. The aim of this design is to provide a model
+which is purely host driven, requiring no co-operation from the software
+running in the guest, and is thus suitable for cloud scenarios.
+
+PV guests are out of scope for this project because, as is outlined above,
+they have a symbiotic relationship with the hypervisor and therefore a
+certain level of co-operation is required.
+
+x86 HVM guests can already be migrated on Xen without guest co-operation
+but only if they don’t have PV drivers installed[1] or are not in ACPI
+power state S0. The reason for not expecting co-operation if the guest is
+any sort of suspended state is obvious, but the reason co-operation is
+expected if PV drivers are installed is due to the nature of PV protocols.
+
+## Xenstore Nodes and Domain ID
+
+The PV driver model consists of a *frontend* and a *backend*. The frontend
+runs inside the guest domain and the backend runs inside a *service domain*
+which may or may not be domain 0. The frontend and backend typically pass
+data via memory pages which are shared between the two domains, but this
+channel of communication is generally established using xenstore (the store
+protocol itself being an exception to this for obvious chicken-and-egg
+reasons).
+
+Typical protocol establishment is based on use of two separate xenstore
+*areas*. If we consider PV drivers for the *netif* protocol (i.e. class vif)
+and assume the guest has domid X, the service domain has domid Y, and the
+vif has index Z then the frontend area will reside under the parent node:
+
+`/local/domain/Y/device/vif/Z`
+
+All backends, by convention, typically reside under parent node:
+
+`/local/domain/X/backend`
+
+and the normal backend area for vif Z would be:
+
+`/local/domain/X/backend/vif/Y/Z`
+
+but this should not be assumed.
+
+The toolstack will place two nodes in the frontend area to explicitly locate
+the backend:
+
+* `backend`: the fully qualified xenstore path of the backend area
+* `backend-id`: the domid of the service domain
+
+and similarly two nodes in the backend area to locate the frontend area:
+
+* `frontend`: the fully qualified xenstore path of the frontend area
+* `frontend-id`: the domid of the guest domain
+
+
+The guest domain only has write permission to the frontend area and
+similarly the service domain only has write permission to the backend area,
+but both ends have read permission to both areas.
+
+Under both frontend and backend areas is a node called *state*. This is key
+to protocol establishment. Upon PV device creation the toolstack will set
+the value of both state nodes to 1 (XenbusStateInitialising[2]). This
+should cause enumeration of appropriate devices in both the guest and
+service domains. The backend device, once it has written any necessary
+protocol specific information into the xenstore backend area (to be read
+by the frontend driver) will update the backend state node to 2
+(XenbusStateInitWait). From this point on PV protocols differ slightly; the

Re: [Xen-devel] [PATCH v1] libxl: remove limit for default number of event channels

2020-03-27 Thread Julien Grall

Hi,

On 27/03/2020 14:37, Olaf Hering wrote:

On Fri, Mar 27, Ian Jackson wrote:


This seems likely to be right, but: what is the default in Xen ?  Is
it sufficiently tight to stop a guest using too many resources ?


The value of d->max_evtchns will be either 4k or 128k.
AFAICS no extra resources are allocated with the changed value.


Event channels are allocated using buckets. Everytime you will a bucket, 
a new bucket will be allocated.


By increasing the limit, you effectively increase the number of buckets 
than can be allocated.


So while I agree that the default allocation will be the same, you 
effectively allow the guest to use more resource in Xen. Therefore I 
don't think this is acceptable to lift the limits for all the guests.


Cheers,

--
Julien Grall



Re: [Xen-devel] [PATCH v1] libxl: remove limit for default number of event channels

2020-03-27 Thread Jan Beulich
On 27.03.2020 15:37, Olaf Hering wrote:
> On Fri, Mar 27, Ian Jackson wrote:
> 
>> This seems likely to be right, but: what is the default in Xen ?  Is
>> it sufficiently tight to stop a guest using too many resources ?
> 
> The value of d->max_evtchns will be either 4k or 128k.
> AFAICS no extra resources are allocated with the changed value.

Of course there are, as soon as the guest uses the event channels.
Did you see Jürgen's patch sent earlier this week, aiming to
address the same issue of larger vCPU counts being a problem
("tools/libxl: make default of max event channels dependant on
vcpus")? See the discussion there.

Jan



Re: [Xen-devel] [PATCH v1] libxl: remove limit for default number of event channels

2020-03-27 Thread Olaf Hering
On Fri, Mar 27, Ian Jackson wrote:

> This seems likely to be right, but: what is the default in Xen ?  Is
> it sufficiently tight to stop a guest using too many resources ?

The value of d->max_evtchns will be either 4k or 128k.
AFAICS no extra resources are allocated with the changed value.

Olaf


signature.asc
Description: PGP signature


Re: [Xen-devel] [PATCH v1] libxl: remove limit for default number of event channels

2020-03-27 Thread Ian Jackson
Olaf Hering writes ("[PATCH v1] libxl: remove limit for default number of event 
channels"):
> The imposed limit of 1023 is too low for a three digit value of vcpus.
> Remove the arbitrary value of 1023 and let Xen decide about the upper limit.

This seems likely to be right, but: what is the default in Xen ?  Is
it sufficiently tight to stop a guest using too many resources ?

Ian.



[Xen-devel] [PATCH v1] libxl: remove limit for default number of event channels

2020-03-27 Thread Olaf Hering
The imposed limit of 1023 is too low for a three digit value of vcpus.
Remove the arbitrary value of 1023 and let Xen decide about the upper limit.

Signed-off-by: Olaf Hering 
---
 docs/man/xl.cfg.5.pod.in   | 8 +++-
 tools/libxl/libxl_create.c | 2 +-
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 0e9e58a41a..ac3fe5f35a 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -1332,11 +1332,9 @@ L to know how to enable vuart console.
 Limit the guest to using at most N event channels (PV interrupts).
 Guests use hypervisor resources for each event channel they use.
 
-The default of 1023 should be sufficient for typical guests.  The
-maximum value depends on what the guest supports.  Guests supporting the
-FIFO-based event channel ABI support up to 131,071 event channels.
-Other guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit
-x86).
+The maximum value depends on what the guest supports.  Guests supporting the
+FIFO-based event channel ABI support up to 131,071 event channels.  Other
+guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit x86).
 
 =item B
 
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e7cb2dbc2b..17c128bc07 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -226,7 +226,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
 b_info->iomem[i].gfn = b_info->iomem[i].start;
 
 if (!b_info->event_channels)
-b_info->event_channels = 1023;
+b_info->event_channels = -1U;
 
 libxl__arch_domain_build_info_setdefault(gc, b_info);
 libxl_defbool_setdefault(_info->dm_restrict, false);



Re: [Xen-devel] [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

2020-03-27 Thread Julien Grall

Hi Jan,

On 27/03/2020 13:50, Jan Beulich wrote:

On 22.03.2020 17:14, jul...@xen.org wrote:

--- a/xen/arch/x86/hvm/domain.c
+++ b/xen/arch/x86/hvm/domain.c
@@ -296,8 +296,10 @@ int arch_set_info_hvm_guest(struct vcpu *v, const 
vcpu_hvm_context_t *ctx)
  if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
  {
  /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
-struct page_info *page = get_page_from_gfn(v->domain,
- v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
+struct page_info *page;
+
+page = get_page_from_gfn(v->domain,
+ gaddr_to_gfn(v->arch.hvm.guest_cr[3]),


My earlier comment on this remains - I thing this conversion makes
the problem this expression has more hidden than with the shift.
This would better use a gfn_from_cr3() helper (or whatever it'll
be that it gets named). Same elsewhere in this patch then.


I will have a closer look the *cr3 helpers and reply with some suggestions.




@@ -2363,7 +2364,7 @@ int hvm_set_cr3(unsigned long value, bool may_defer)
  {
  /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
  HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value);
-page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT,
+page = get_page_from_gfn(v->domain, cr3_to_gfn(value),


Oh, seeing this I recall Paul did point out the above already.


@@ -508,7 +508,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control 
*init_control)
  {
  struct domain *d = current->domain;
  uint32_t vcpu_id;
-uint64_t gfn;
+gfn_t gfn;
  uint32_t offset;
  struct vcpu *v;
  int rc;
@@ -516,7 +516,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control 
*init_control)
  init_control->link_bits = EVTCHN_FIFO_LINK_BITS;
  
  vcpu_id = init_control->vcpu;

-gfn = init_control->control_gfn;
+gfn = _gfn(init_control->control_gfn);


There's silent truncation here now for Arm32, afaict. Are we really
okay with this?


Well, the truncation was already silently happening as we call 
get_page_from_gfn() in map_guest_page(). So it is not worse than the 
current situation.


Although, there are a slight advantage with the new code as you can more 
easily spot potential truncation. Indeed, you could add some type check 
in _gfn().


Cheers,

--
Julien Grall



[Xen-devel] [xen-unstable-smoke test] 149087: tolerable all pass - PUSHED

2020-03-27 Thread osstest service owner
flight 149087 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/149087/

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  c72154e88c6c177ce9b9ec3c5388b5bfcce42f34
baseline version:
 xen  88a1a11daeb93c0f16d9c4d5cb30f1f563c1817c

Last test of basis   149063  2020-03-26 20:01:22 Z0 days
Testing same since   149087  2020-03-27 11:02:08 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Juergen Gross 

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
   88a1a11dae..c72154e88c  c72154e88c6c177ce9b9ec3c5388b5bfcce42f34 -> smoke



Re: [Xen-devel] [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

2020-03-27 Thread Jan Beulich
On 22.03.2020 17:14, jul...@xen.org wrote:
> --- a/xen/arch/x86/hvm/domain.c
> +++ b/xen/arch/x86/hvm/domain.c
> @@ -296,8 +296,10 @@ int arch_set_info_hvm_guest(struct vcpu *v, const 
> vcpu_hvm_context_t *ctx)
>  if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
>  {
>  /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
> -struct page_info *page = get_page_from_gfn(v->domain,
> - v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
> +struct page_info *page;
> +
> +page = get_page_from_gfn(v->domain,
> + gaddr_to_gfn(v->arch.hvm.guest_cr[3]),

My earlier comment on this remains - I thing this conversion makes
the problem this expression has more hidden than with the shift.
This would better use a gfn_from_cr3() helper (or whatever it'll
be that it gets named). Same elsewhere in this patch then.

> @@ -2363,7 +2364,7 @@ int hvm_set_cr3(unsigned long value, bool may_defer)
>  {
>  /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
>  HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value);
> -page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT,
> +page = get_page_from_gfn(v->domain, cr3_to_gfn(value),

Oh, seeing this I recall Paul did point out the above already.

> @@ -508,7 +508,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control 
> *init_control)
>  {
>  struct domain *d = current->domain;
>  uint32_t vcpu_id;
> -uint64_t gfn;
> +gfn_t gfn;
>  uint32_t offset;
>  struct vcpu *v;
>  int rc;
> @@ -516,7 +516,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control 
> *init_control)
>  init_control->link_bits = EVTCHN_FIFO_LINK_BITS;
>  
>  vcpu_id = init_control->vcpu;
> -gfn = init_control->control_gfn;
> +gfn = _gfn(init_control->control_gfn);

There's silent truncation here now for Arm32, afaict. Are we really
okay with this?

Jan



Re: [Xen-devel] [PATCH] automation: update openSUSE Tumbleweed building dependencies

2020-03-27 Thread Andrew Cooper
On 26/03/2020 17:29, Dario Faggioli wrote:
> We need python3 (and the respective -devel package), these days.
>
> Signed-off-by: Dario Faggioli 
> ---
> Cc: Doug Goldstein 
> ---
>  .../build/suse/opensuse-tumbleweed.dockerfile  |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile 
> b/automation/build/suse/opensuse-tumbleweed.dockerfile
> index 2676a87c85..e80d773a79 100644
> --- a/automation/build/suse/opensuse-tumbleweed.dockerfile
> +++ b/automation/build/suse/opensuse-tumbleweed.dockerfile
> @@ -54,8 +54,8 @@ RUN zypper install -y --no-recommends \
>  pandoc \
>  patch \
>  pkg-config \
> -python \
> -python-devel \
> +python3 \
> +python3-devel \

These containers get used for older Xen branches as well, so you
generally can't take deps out like this (Until we stop building the
oldest branches which need the dependency).

Will tumbleweed cope with both packages installed concurrently?

~Andrew



[Xen-devel] [PATCH v8 1/2] docs/designs: Add a design document for non-cooperative live migration

2020-03-27 Thread Paul Durrant
From: Paul Durrant 

It has become apparent to some large cloud providers that the current
model of cooperative migration of guests under Xen is not usable as it
relies on software running inside the guest, which is likely beyond the
provider's control.
This patch introduces a proposal for non-cooperative live migration,
designed not to rely on any guest-side software.

Signed-off-by: Paul Durrant 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Wei Liu 

v8:
 - Addressed comments from Julien on v6 that I missed

v6:
 - Addressed comments from Julien

v5:
 - Note that PV domain are not just expected to co-operate, they are
   required to

v4:
 - Fix issues raised by Wei

v2:
 - Use the term 'non-cooperative' instead of 'transparent'
 - Replace 'trust in' with 'reliance on' when referring to guest-side
   software
---
 docs/designs/non-cooperative-migration.md | 280 ++
 1 file changed, 280 insertions(+)
 create mode 100644 docs/designs/non-cooperative-migration.md

diff --git a/docs/designs/non-cooperative-migration.md 
b/docs/designs/non-cooperative-migration.md
new file mode 100644
index 00..4b876d809f
--- /dev/null
+++ b/docs/designs/non-cooperative-migration.md
@@ -0,0 +1,280 @@
+# Non-Cooperative Migration of Guests on Xen
+
+## Background
+
+The normal model of migration in Xen is driven by the guest because it was
+originally implemented for PV guests, where the guest must be aware it is
+running under Xen and is hence expected to co-operate. This model dates from
+an era when it was assumed that the host administrator had control of at
+least the privileged software running in the guest (i.e. the guest kernel)
+which may still be true in an enterprise deployment but is not generally
+true in a cloud environment. The aim of this design is to provide a model
+which is purely host driven, requiring no co-operation from the software
+running in the guest, and is thus suitable for cloud scenarios.
+
+PV guests are out of scope for this project because, as is outlined above,
+they have a symbiotic relationship with the hypervisor and therefore a
+certain level of co-operation is required.
+
+x86 HVM guests can already be migrated on Xen without guest co-operation
+but only if they don’t have PV drivers installed[1] or are not in ACPI
+power state S0. The reason for not expecting co-operation if the guest is
+any sort of suspended state is obvious, but the reason co-operation is
+expected if PV drivers are installed is due to the nature of PV protocols.
+
+## Xenstore Nodes and Domain ID
+
+The PV driver model consists of a *frontend* and a *backend*. The frontend
+runs inside the guest domain and the backend runs inside a *service domain*
+which may or may not be domain 0. The frontend and backend typically pass
+data via memory pages which are shared between the two domains, but this
+channel of communication is generally established using xenstore (the store
+protocol itself being an exception to this for obvious chicken-and-egg
+reasons).
+
+Typical protocol establishment is based on use of two separate xenstore
+*areas*. If we consider PV drivers for the *netif* protocol (i.e. class vif)
+and assume the guest has domid X, the service domain has domid Y, and the
+vif has index Z then the frontend area will reside under the parent node:
+
+`/local/domain/Y/device/vif/Z`
+
+All backends, by convention, typically reside under parent node:
+
+`/local/domain/X/backend`
+
+and the normal backend area for vif Z would be:
+
+`/local/domain/X/backend/vif/Y/Z`
+
+but this should not be assumed.
+
+The toolstack will place two nodes in the frontend area to explicitly locate
+the backend:
+
+* `backend`: the fully qualified xenstore path of the backend area
+* `backend-id`: the domid of the service domain
+
+and similarly two nodes in the backend area to locate the frontend area:
+
+* `frontend`: the fully qualified xenstore path of the frontend area
+* `frontend-id`: the domid of the guest domain
+
+
+The guest domain only has write permission to the frontend area and
+similarly the service domain only has write permission to the backend area,
+but both ends have read permission to both areas.
+
+Under both frontend and backend areas is a node called *state*. This is key
+to protocol establishment. Upon PV device creation the toolstack will set
+the value of both state nodes to 1 (XenbusStateInitialising[2]). This
+should cause enumeration of appropriate devices in both the guest and
+service domains. The backend device, once it has written any necessary
+protocol specific information into the xenstore backend area (to be read
+by the frontend driver) will update the backend state node to 2
+(XenbusStateInitWait). From this point on PV protocols differ slightly; the
+following illustration is true of the netif protocol.
+
+Upon seeing a backend state value of 2, the 

[Xen-devel] [PATCH v8 0/2] docs: Migration design documents

2020-03-27 Thread Paul Durrant
Paul Durrant (2):
  docs/designs: Add a design document for non-cooperative live migration
  docs/designs: Add a design document for migration of xenstore data

 docs/designs/non-cooperative-migration.md | 280 ++
 docs/designs/xenstore-migration.md| 256 
 docs/misc/xenstore.txt|   6 +-
 3 files changed, 539 insertions(+), 3 deletions(-)
 create mode 100644 docs/designs/non-cooperative-migration.md
 create mode 100644 docs/designs/xenstore-migration.md

-- 
2.20.1




[Xen-devel] [PATCH v8 2/2] docs/designs: Add a design document for migration of xenstore data

2020-03-27 Thread Paul Durrant
From: Paul Durrant 

This patch details proposes extra migration data and xenstore protocol
extensions to support non-cooperative live migration of guests.

NOTE: doc/misc/xenstore.txt is also amended to replace the  term
  for the INTRODUCE operation with the , since this is what
  it actually is.

Signed-off-by: Paul Durrant 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Wei Liu 

v8:
 - Addressed further comments form Julien

v7:
 - Addressed further comments from Julien
 - Switched migration records to defined structures instead of tuples

v6:
 - Addressed comments from Julien

v5:
 - Add QUIESCE
 - Make semantics of  in GET_DOMAIN_WATCHES more clear

v4:
 - Drop the restrictions on special paths

v3:
 - New in v3
---
 docs/designs/xenstore-migration.md | 256 +
 docs/misc/xenstore.txt |   6 +-
 2 files changed, 259 insertions(+), 3 deletions(-)
 create mode 100644 docs/designs/xenstore-migration.md

diff --git a/docs/designs/xenstore-migration.md 
b/docs/designs/xenstore-migration.md
new file mode 100644
index 00..97695f3ec9
--- /dev/null
+++ b/docs/designs/xenstore-migration.md
@@ -0,0 +1,256 @@
+# Xenstore Migration
+
+## Background
+
+The design for *Non-Cooperative Migration of Guests*[1] explains that extra
+save records are required in the migrations stream to allow a guest running
+PV drivers to be migrated without its co-operation. Moreover the save
+records must include details of registered xenstore watches as well as
+content; information that cannot currently be recovered from `xenstored`,
+and hence some extension to the xenstore protocol[2] will also be required.
+
+The *libxenlight Domain Image Format* specification[3] already defines a
+record type `EMULATOR_XENSTORE_DATA` but this is not suitable for
+transferring xenstore data pertaining to the domain directly as it is
+specified such that keys are relative to the path
+`/local/domain/$dm_domid/device-model/$domid`. Thus it is necessary to
+define at least one new save record type.
+
+## Proposal
+
+### New Save Record
+
+A new mandatory record type should be defined within the libxenlight Domain
+Image Format:
+
+`0x0007: DOMAIN_XENSTORE_DATA`
+
+An arbitrary number of these records may be present in the migration
+stream and may appear in any order. The format of each record should be as
+follows:
+
+
+```
+0   1   2   3   4   5   6   7octet
++---+---+---+---+---+---+---+---+
+| type  | record specific data  |
++---+   |
+...
++---+
+```
+
+where type is one of the following values
+
+
+| Field  | Description  |
+||--|
+| `type` | 0x: invalid  |
+|| 0x0001: NODE_DATA|
+|| 0x0002: WATCH_DATA   |
+|| 0x0003: TRANSACTION_DATA |
+|| 0x0004 - 0x: reserved for future use |
+
+
+and data is one of the record data formats described in the following
+sections.
+
+
+NOTE: The record data does not contain an overall length because the
+libxenlight record header specifies the length.
+
+
+**NODE_DATA**
+
+
+Each NODE_DATA record specifies a single node in xenstore and is formatted
+as follows:
+
+
+```
+0   1   2   3 octet
++---+---+---+---+
+| NODE_DATA |
++---+
+| path length   |
++---+
+| path data |
+...
+| pad (0 to 3 octets)   |
++---+
+| perm count (N)|
++---+
+| perm0 |
++---+
+...
++---+
+| permN |
++---+
+| value length  |
++---+
+| value data|
+...
+| pad (0 to 3 octets)   |
++---+
+```
+
+where perm0..N are formatted as follows:
+
+
+```
+0   1   2   3 octet
++---+---+---+---+
+| perm  | pad   | domid |
++---+
+```
+
+
+path length and value length are specified in octets (excluding the NUL
+terminator of the path). perm should be one of the ASCII values `w`, `r`,
+`b` or `n` as described in [2]. All pad values should be 0.
+All paths should be absolute (i.e. start with `/`) and as described in
+[2].
+
+
+**WATCH_DATA**
+
+
+Each WATCH_DATA record specifies a registered watch and is 

Re: [Xen-devel] [PATCH] SVM: split _np_enable VMCB field

2020-03-27 Thread Andrew Cooper
On 26/03/2020 14:00, Jan Beulich wrote:
> The nest paging enable is actually just a single bit within the 64-bit
> VMCB field, which is particularly relevant for uses like the one in
> nsvm_vcpu_vmentry().

Lucky for us, these are configuration options, not returned data, so at
least the field won't change behind our back.  Also, it should be fixed
for the lifetime of the domain.

>  Split the field, adding definitions for a few other
> bits at the same time. To be able to generate accessors for bitfields,
> VMCB_ACCESSORS() needs the type part broken out, as typeof() can't be
> applied to bitfields. Unfortunately this means specification of the same
> type in two distinct places.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 



Re: [Xen-devel] [PATCH] automation/gitlab: add https transport support to Debian images

2020-03-27 Thread Wei Liu
On Fri, Mar 27, 2020 at 12:49:47PM +0100, Roger Pau Monne wrote:
> The LLVM repos have switched from http to https, and trying to access
> using http will get redirected to https. Add the apt-transport-https
> package to the x86 Debian containers that use the LLVM repos, in order
> to support the https transport method.
> 
> Note that on Arm we only test with gcc, so don't add the package for
> the Debian Arm container.
> 
> This fixes the following error seen on the QEMU smoke tests:
> 
> E: The method driver /usr/lib/apt/methods/https could not be found.
> 
> Signed-off-by: Roger Pau Monné 

Acked-by: Wei Liu 

> ---
> Should I try to push the updated containers myself?

Yes if you can do that, that would be great.

Wei.



Re: [Xen-devel] [PATCH 16/17] xen/mm: Convert {s, g}et_gpfn_from_mfn() to use typesafe MFN

2020-03-27 Thread Jan Beulich
On 22.03.2020 17:14, jul...@xen.org wrote:
> @@ -983,19 +984,20 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m)
>  /* check for 1GB super page */
>  if ( l3e_get_flags(l3e[i3]) & _PAGE_PSE )
>  {
> -mfn = l3e_get_pfn(l3e[i3]);
> -ASSERT(mfn_valid(_mfn(mfn)));
> +mfn = l3e_get_mfn(l3e[i3]);
> +ASSERT(mfn_valid(mfn));
>  /* we have to cover 512x512 4K pages */
>  for ( i2 = 0; 
>i2 < (L2_PAGETABLE_ENTRIES * L1_PAGETABLE_ENTRIES);
>i2++)
>  {
> -m2pfn = get_gpfn_from_mfn(mfn+i2);
> +m2pfn = get_pfn_from_mfn(mfn_add(mfn, i2));
>  if ( m2pfn != (gfn + i2) )
>  {
>  pmbad++;
> -P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx -> 
> gfn %#lx\n",
> -   gfn + i2, mfn + i2, m2pfn);
> +P2M_PRINTK("mismatch: gfn %#lx -> mfn %"PRI_mfn" 
> gfn %#lx\n",
> +   gfn + i2, mfn_x(mfn_add(mfn, i2)),

As in the earlier patch, "mfn_x(mfn) + i2" would be shorter and
hence imo preferable, especially in printk() and alike invocations.

I would also prefer if you left %#lx alone, with the 2nd best
option being to also use PRI_gfn alongside PRI_mfn. Primarily
I'd like to avoid having a mixture.

Same (for both) at least one more time further down.

> @@ -974,7 +974,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, 
> mfn_t mfn,
>  P2M_DEBUG("old gfn=%#lx -> mfn %#lx\n",
>gfn_x(ogfn) , mfn_x(omfn));
>  if ( mfn_eq(omfn, mfn_add(mfn, i)) )
> -p2m_remove_page(p2m, gfn_x(ogfn), mfn_x(mfn_add(mfn, i)),
> +p2m_remove_page(p2m, gfn_x(ogfn), mfn_add(mfn, i),
>  0);

Pull this up then onto the now shorter prior line?

> @@ -2843,53 +2843,53 @@ void audit_p2m(struct domain *d,
>  spin_lock(>page_alloc_lock);
>  page_list_for_each ( page, >page_list )
>  {
> -mfn = mfn_x(page_to_mfn(page));
> +mfn = page_to_mfn(page);
>  
> -P2M_PRINTK("auditing guest page, mfn=%#lx\n", mfn);
> +P2M_PRINTK("auditing guest page, mfn=%"PRI_mfn"\n", mfn_x(mfn));
>  
>  od = page_get_owner(page);
>  
>  if ( od != d )
>  {
> -P2M_PRINTK("mfn %"PRI_mfn" owner %pd != %pd\n", mfn, od, d);
> +P2M_PRINTK("mfn %"PRI_mfn" owner %pd != %pd\n", mfn_x(mfn), od, 
> d);
>  continue;
>  }
>  
> -gfn = get_gpfn_from_mfn(mfn);
> +gfn = get_pfn_from_mfn(mfn);
>  if ( gfn == INVALID_M2P_ENTRY )
>  {
>  orphans_count++;
> -P2M_PRINTK("orphaned guest page: mfn=%#lx has invalid gfn\n",
> -   mfn);
> +P2M_PRINTK("orphaned guest page: mfn=%"PRI_mfn" has invalid 
> gfn\n",
> +   mfn_x(mfn));
>  continue;
>  }
>  
>  if ( SHARED_M2P(gfn) )
>  {
> -P2M_PRINTK("shared mfn (%lx) on domain page list!\n",
> -mfn);
> +P2M_PRINTK("shared mfn (%"PRI_mfn") on domain page list!\n",
> +   mfn_x(mfn));
>  continue;
>  }
>  
>  p2mfn = get_gfn_type_access(p2m, gfn, , , 0, NULL);
> -if ( mfn_x(p2mfn) != mfn )
> +if ( !mfn_eq(p2mfn, mfn) )
>  {
>  mpbad++;
> -P2M_PRINTK("map mismatch mfn %#lx -> gfn %#lx -> mfn %#lx"
> +P2M_PRINTK("map mismatch mfn %"PRI_mfn" -> gfn %#lx -> mfn 
> %"PRI_mfn""
> " (-> gfn %#lx)\n",
> -   mfn, gfn, mfn_x(p2mfn),
> +   mfn_x(mfn), gfn, mfn_x(p2mfn),
> (mfn_valid(p2mfn)
> -? get_gpfn_from_mfn(mfn_x(p2mfn))
> +? get_pfn_from_mfn(p2mfn)
>  : -1u));

I realize this is an entirely unrelated change, but the -1u here
is standing out too much to not mention it: Could I talk you into
making this gfn_x(INVALID_GFN) at this occasion?

> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -500,9 +500,10 @@ extern paddr_t mem_hotplug;
>   */
>  extern bool machine_to_phys_mapping_valid;
>  
> -static inline void set_gpfn_from_mfn(unsigned long mfn, unsigned long pfn)
> +static inline void set_pfn_from_mfn(mfn_t mfn_, unsigned long pfn)
>  {
> -const struct domain *d = page_get_owner(mfn_to_page(_mfn(mfn)));
> +const unsigned long mfn = mfn_x(mfn_);

I think it would be better overall if the parameter was named
"mfn" and there was no local variable altogether. This would
bring 

[Xen-devel] [PATCH v4 0/2] x86/nvmx: fixes for interrupt injection

2020-03-27 Thread Roger Pau Monne
Hello,

osstest identified a regression caused by my earlier attempt to fix
interrupt injection when using nested VMX. This series aims to fix the
regression, and should unblock several osstest branches.

The following report is from osstest with this series applied:

http://logs.test-lab.xenproject.org/osstest/logs/149084/

Note the last patch (2/2) is the one that actually fixes the issue. Xen
will always use the Ack on exit feature so patche 1/2 don't change the
functionality when running a nested Xen, as it always requires SVI to be
updated.

Thanks, Roger.

Roger Pau Monne (2):
  x86/nvmx: split updating RVI from SVI in nvmx_update_apicv
  x86/nvmx: update exit bitmap when using virtual interrupt delivery

 xen/arch/x86/hvm/vmx/intr.c   | 21 +++---
 xen/arch/x86/hvm/vmx/vvmx.c   | 36 +++
 xen/include/asm-x86/hvm/vmx/vmx.h |  2 ++
 3 files changed, 42 insertions(+), 17 deletions(-)

-- 
2.26.0




[Xen-devel] [PATCH v4 2/2] x86/nvmx: update exit bitmap when using virtual interrupt delivery

2020-03-27 Thread Roger Pau Monne
Force an update of the EOI exit bitmap in nvmx_update_apicv, because
the one performed in vmx_intr_assist might not be reached if the
interrupt is intercepted by nvmx_intr_intercept returning true.

Extract the code to update the exit bitmap from vmx_intr_assist into a
helper and use it in nvmx_update_apicv.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Kevin Tian 
---
Changes since v2:
 - Only update the EOI exit bitmap if GUEST_INTR_STATUS is changed.

Changes since v1:
 - Reword commit message.
---
 xen/arch/x86/hvm/vmx/intr.c   | 21 +
 xen/arch/x86/hvm/vmx/vvmx.c   |  3 +++
 xen/include/asm-x86/hvm/vmx/vmx.h |  2 ++
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
index 49a1295f09..000e14af49 100644
--- a/xen/arch/x86/hvm/vmx/intr.c
+++ b/xen/arch/x86/hvm/vmx/intr.c
@@ -224,6 +224,18 @@ static int nvmx_intr_intercept(struct vcpu *v, struct 
hvm_intack intack)
 return 0;
 }
 
+void vmx_sync_exit_bitmap(struct vcpu *v)
+{
+const unsigned int n = ARRAY_SIZE(v->arch.hvm.vmx.eoi_exit_bitmap);
+unsigned int i;
+
+while ( (i = find_first_bit(>arch.hvm.vmx.eoi_exitmap_changed, n)) < n )
+{
+clear_bit(i, >arch.hvm.vmx.eoi_exitmap_changed);
+__vmwrite(EOI_EXIT_BITMAP(i), v->arch.hvm.vmx.eoi_exit_bitmap[i]);
+}
+}
+
 void vmx_intr_assist(void)
 {
 struct hvm_intack intack;
@@ -318,7 +330,6 @@ void vmx_intr_assist(void)
   intack.source != hvm_intsrc_vector )
 {
 unsigned long status;
-unsigned int i, n;
 
/*
 * intack.vector is the highest priority vector. So we set 
eoi_exit_bitmap
@@ -379,13 +390,7 @@ void vmx_intr_assist(void)
 intack.vector;
 __vmwrite(GUEST_INTR_STATUS, status);
 
-n = ARRAY_SIZE(v->arch.hvm.vmx.eoi_exit_bitmap);
-while ( (i = find_first_bit(>arch.hvm.vmx.eoi_exitmap_changed,
-n)) < n )
-{
-clear_bit(i, >arch.hvm.vmx.eoi_exitmap_changed);
-__vmwrite(EOI_EXIT_BITMAP(i), v->arch.hvm.vmx.eoi_exit_bitmap[i]);
-}
+vmx_sync_exit_bitmap(v);
 
 pt_intr_post(v, intack);
 }
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index d63f417f9c..125cb87493 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1420,7 +1420,10 @@ static void nvmx_update_apicv(struct vcpu *v)
 }
 
 if ( status )
+{
 __vmwrite(GUEST_INTR_STATUS, status);
+vmx_sync_exit_bitmap(v);
+}
 }
 
 static void virtual_vmexit(struct cpu_user_regs *regs)
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h 
b/xen/include/asm-x86/hvm/vmx/vmx.h
index b334e1ec94..111ccd7e61 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -610,6 +610,8 @@ void update_guest_eip(void);
 void vmx_pi_per_cpu_init(unsigned int cpu);
 void vmx_pi_desc_fixup(unsigned int cpu);
 
+void vmx_sync_exit_bitmap(struct vcpu *v);
+
 #ifdef CONFIG_HVM
 void vmx_pi_hooks_assign(struct domain *d);
 void vmx_pi_hooks_deassign(struct domain *d);
-- 
2.26.0




[Xen-devel] [PATCH v4 1/2] x86/nvmx: split updating RVI from SVI in nvmx_update_apicv

2020-03-27 Thread Roger Pau Monne
Updating SVI is required when an interrupt has been injected using the
Ack on exit VMEXIT feature, so that the in service interrupt in the
GUEST_INTR_STATUS matches the vector that is signaled in
VM_EXIT_INTR_INFO.

Updating RVI however is not tied to the Ack on exit feature, as it
signals the next vector to be injected, and hence should always be
updated to the next pending vector, regardless of whether Ack on exit
is enabled.

When not using the Ack on exit feature preserve the previous vector in
SVI, so that it's not lost when RVI is updated to contain the pending
vector to inject.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Kevin Tian 
---
Changes since v3:
 - Return early if interrupt source is not lapic.

Changes since v2:
 - Return early if the exit reason != EXTERNAL_INTERRUPT.
 - Reduce the number of vmwrites by accumulating the changes to a
   local variable which is flushed at the end of the function.
 - Attempt to preserve the exiting SVI if Ack on exit is not enabled.
---
 xen/arch/x86/hvm/vmx/vvmx.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 1753005c91..d63f417f9c 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1384,28 +1384,43 @@ static void nvmx_update_apicv(struct vcpu *v)
 struct nestedvmx *nvmx = _2_nvmx(v);
 unsigned long reason = get_vvmcs(v, VM_EXIT_REASON);
 unsigned long intr_info = get_vvmcs(v, VM_EXIT_INTR_INFO);
+unsigned long status;
+int rvi;
 
-if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
- nvmx->intr.source == hvm_intsrc_lapic &&
- (intr_info & INTR_INFO_VALID_MASK) )
+if ( reason != EXIT_REASON_EXTERNAL_INTERRUPT ||
+ nvmx->intr.source != hvm_intsrc_lapic )
+return;
+
+if ( intr_info & INTR_INFO_VALID_MASK )
 {
-uint16_t status;
-uint32_t rvi, ppr;
-uint32_t vector = intr_info & 0xff;
+uint32_t ppr;
+unsigned int vector = intr_info & INTR_INFO_VECTOR_MASK;
 struct vlapic *vlapic = vcpu_vlapic(v);
 
+/*
+ * Update SVI to record the current in service interrupt that's
+ * signaled in EXIT_INTR_INFO.
+ */
 vlapic_ack_pending_irq(v, vector, 1);
 
 ppr = vlapic_set_ppr(vlapic);
 WARN_ON((ppr & 0xf0) != (vector & 0xf0));
 
 status = vector << VMX_GUEST_INTR_STATUS_SVI_OFFSET;
-rvi = vlapic_has_pending_irq(v);
-if ( rvi != -1 )
-status |= rvi & VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK;
+}
+else
+   /* Keep previous SVI if there's any. */
+   __vmread(GUEST_INTR_STATUS, );
 
-__vmwrite(GUEST_INTR_STATUS, status);
+rvi = vlapic_has_pending_irq(v);
+if ( rvi != -1 )
+{
+status &= ~VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK;
+status |= rvi & VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK;
 }
+
+if ( status )
+__vmwrite(GUEST_INTR_STATUS, status);
 }
 
 static void virtual_vmexit(struct cpu_user_regs *regs)
-- 
2.26.0




Re: [Xen-devel] [PATCH 15/17] xen/x86: p2m: Rework printk format in audit_p2m()

2020-03-27 Thread Jan Beulich
On 22.03.2020 17:14, jul...@xen.org wrote:
> From: Julien Grall 
> 
> One of the printk format in audit_p2m() may be difficult to read as it
> is not clear what is the first number.
> 
> Furthermore, the format can now take advantage of %pd.
> 
> Signed-off-by: Julien Grall 

Acked-by: Jan Beulich 



Re: [Xen-devel] [PATCH 14/17] xen/x86: mm: Re-implement set_gpfn_from_mfn() as a static inline function

2020-03-27 Thread Jan Beulich
On 22.03.2020 17:14, jul...@xen.org wrote:
> From: Julien Grall 
> 
> set_gpfn_from_mfn() is currently implement in a 2 part macros. The
> second macro is only called within the first macro, so they can be
> folded together.
> 
> Furthermore, this is now converted to a static inline making the code
> more readable and safer.
> 
> Signed-off-by: Julien Grall 

Reviewed-by: Jan Beulich 



Re: [Xen-devel] [PATCH 7/7] x86/ucode/intel: Fold structures together

2020-03-27 Thread Andrew Cooper
On 26/03/2020 15:05, Jan Beulich wrote:
> On 26.03.2020 15:50, Andrew Cooper wrote:
>> On a perhaps tangential note, what (if anything) are you plans regarding
>> backport here?
>>
>> These defines are ok for a transitional period across a series (and
>> probably means I'll need to get the AMD side ready to be committed at
>> the same time), but I don't think we'd want them in the code for the
>> longterm.
>>
>> I personally wasn't overly concerned about backports, but if you are, we
>> should probably take this into consideration for the fixes.
> Till now I didn't see a strong reason why backporting might be
> needed (or even just wanted).

The gratuitous memory allocation fixes would be the most compelling (and
even then, not massively so), but they're sufficiently interlaced with
the rest of the cleanup that I wasn't expecting backports to be a
pleasant idea.

~Andrew



Re: [Xen-devel] [PATCH] x86/ucode: Drop the sanity check for interrupts being disabled

2020-03-27 Thread Jan Beulich
On 27.03.2020 13:19, Andrew Cooper wrote:
> Of the substantial number of things which can go wrong during microcode load,
> this is not one.  Loading occurs entirely within the boundary of a single
> WRMSR instruction.  Its certainly not a BUG()-worthy condition.
> 
> Xen has legitimate reasons to not want interrupts enabled at this point, but
> that is to do with organising the system rendezvous.  As these are private low
> level helpers invoked only from the microcode core logic, forgo the check
> entirely.
> 
> While dropping system.h, clean up the processor.h include which was an
> oversight in the previous header cleanup.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 



Re: [Xen-devel] [PATCH] x86/ucode/amd: Fix buffer overrun with equiv table handling

2020-03-27 Thread Jan Beulich
On 27.03.2020 12:59, Andrew Cooper wrote:
> find_equiv_cpu_id() loops until it finds a 0 installed_cpu entry.  Well formed
> AMD microcode containers have this property.

With this, would you mind adding "potential" to the subject?

> Extend the checking in install_equiv_cpu_table() to reject tables which don't
> have a sentinal at the end.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 




[Xen-devel] [PATCH v2 3/7] x86/ucode/intel: Remove gratuitous memory allocations from cpu_request_microcode()

2020-03-27 Thread Andrew Cooper
cpu_request_microcode() needs to scan its container and duplicate one blob,
but the get_next_ucode_from_buffer() helper duplicates every blob in turn.
Furthermore, the length checking is only safe from overflow in 64bit builds.

Delete get_next_ucode_from_buffer() and alter the purpose of the saved
variable to simply point somewhere in buf until we're ready to return.

This is only a modest reduction in absolute code size (-144), but avoids
making memory allocations for every blob in the container.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

v2:
 * Rebase over struct microcode_patch re-work
 * Reinstate printk() for bad data
---
 xen/arch/x86/cpu/microcode/intel.c | 65 +++---
 1 file changed, 25 insertions(+), 40 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index 77539a00be..2b48959573 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -315,67 +315,52 @@ static int apply_microcode(const struct microcode_patch 
*patch)
 return 0;
 }
 
-static long get_next_ucode_from_buffer(struct microcode_intel **mc,
-   const uint8_t *buf, unsigned long size,
-   unsigned long offset)
-{
-struct microcode_header_intel *mc_header;
-unsigned long total_size;
-
-/* No more data */
-if ( offset >= size )
-return 0;
-mc_header = (struct microcode_header_intel *)(buf + offset);
-total_size = get_totalsize(mc_header);
-
-if ( (offset + total_size) > size )
-{
-printk(KERN_ERR "microcode: error! Bad data in microcode data file\n");
-return -EINVAL;
-}
-
-*mc = xmemdup_bytes(mc_header, total_size);
-if ( *mc == NULL )
-return -ENOMEM;
-
-return offset + total_size;
-}
-
 static struct microcode_patch *cpu_request_microcode(const void *buf,
  size_t size)
 {
-long offset = 0;
 int error = 0;
-struct microcode_intel *mc, *saved = NULL;
+const struct microcode_patch *saved = NULL;
 struct microcode_patch *patch = NULL;
 
-while ( (offset = get_next_ucode_from_buffer(, buf, size, offset)) > 0 )
+while ( size )
 {
-error = microcode_sanity_check(mc);
-if ( error )
+const struct microcode_patch *mc;
+unsigned int blob_size;
+
+if ( size < MC_HEADER_SIZE ||   /* Insufficient space for header? 
*/
+ (mc = buf)->hdr.hdrver != 1 || /* Unrecognised header version?   
*/
+ mc->hdr.ldrver != 1 || /* Unrecognised loader version?   
*/
+ size < (blob_size =/* Insufficient space for patch?  
*/
+ get_totalsize(>hdr)) )
 {
-xfree(mc);
+error = -EINVAL;
+printk(XENLOG_WARNING "microcode: Bad data in container\n");
 break;
 }
 
+error = microcode_sanity_check(mc);
+if ( error )
+break;
+
 /*
  * If the new update covers current CPU, compare updates and store the
  * one with higher revision.
  */
 if ( (microcode_update_match(mc) != MIS_UCODE) &&
  (!saved || (mc->hdr.rev > saved->hdr.rev)) )
-{
-xfree(saved);
 saved = mc;
-}
-else
-xfree(mc);
+
+buf  += blob_size;
+size -= blob_size;
 }
-if ( offset < 0 )
-error = offset;
 
 if ( saved )
-patch = saved;
+{
+patch = xmemdup_bytes(saved, get_totalsize(>hdr));
+
+if ( !patch )
+error = -ENOMEM;
+}
 
 if ( error && !patch )
 patch = ERR_PTR(error);
-- 
2.11.0




[Xen-devel] [PATCH v2 7/7] x86/ucode/intel: Fold structures together

2020-03-27 Thread Andrew Cooper
With all the necessary cleanup now in place, fold struct
microcode_header_intel into struct microcode_patch and drop the struct
microcode_intel temporary ifdef-ary.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

v2:
 * Rebase over struct microcode_patch re-work
---
 xen/arch/x86/cpu/microcode/intel.c | 56 ++
 1 file changed, 20 insertions(+), 36 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index 1358a25032..9a8ef62e2b 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -32,17 +32,12 @@
 
 #define pr_debug(x...) ((void)0)
 
-struct microcode_header_intel {
+struct microcode_patch {
 unsigned int hdrver;
 unsigned int rev;
-union {
-struct {
-uint16_t year;
-uint8_t day;
-uint8_t month;
-};
-unsigned int date;
-};
+uint16_t year;
+uint8_t  day;
+uint8_t  month;
 unsigned int sig;
 unsigned int cksum;
 unsigned int ldrver;
@@ -56,18 +51,11 @@ struct microcode_header_intel {
 unsigned int datasize;
 unsigned int totalsize;
 unsigned int reserved[3];
-};
-
-struct microcode_patch {
-struct microcode_header_intel hdr;
 
 /* Microcode payload.  Format is propriety and encrypted. */
 uint8_t data[];
 };
 
-/* Temporary, until the microcode_* structure are disentangled. */
-#define microcode_intel microcode_patch
-
 /* microcode format is extended from prescott processors */
 struct extended_sigtable {
 unsigned int count;
@@ -81,16 +69,16 @@ struct extended_sigtable {
 };
 
 #define PPRO_UCODE_DATASIZE 2000
-#define MC_HEADER_SIZE  (sizeof(struct microcode_header_intel))
+#define MC_HEADER_SIZE  offsetof(struct microcode_patch, data)
 
 static uint32_t get_datasize(const struct microcode_patch *patch)
 {
-return patch->hdr.datasize ?: PPRO_UCODE_DATASIZE;
+return patch->datasize ?: PPRO_UCODE_DATASIZE;
 }
 
 static uint32_t get_totalsize(const struct microcode_patch *patch)
 {
-return patch->hdr.totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
+return patch->totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
 }
 
 /*
@@ -102,8 +90,8 @@ static uint32_t get_totalsize(const struct microcode_patch 
*patch)
 static const struct extended_sigtable *get_ext_sigtable(
 const struct microcode_patch *patch)
 {
-if ( patch->hdr.totalsize > (MC_HEADER_SIZE + patch->hdr.datasize) )
-return (const void *)>data[patch->hdr.datasize];
+if ( patch->totalsize > (MC_HEADER_SIZE + patch->datasize) )
+return (const void *)>data[patch->datasize];
 
 return NULL;
 }
@@ -224,7 +212,7 @@ static int microcode_sanity_check(const struct 
microcode_patch *patch)
  * Checksum each indiviudal extended signature as if it had been in the
  * main header.
  */
-sum = patch->hdr.sig + patch->hdr.pf + patch->hdr.cksum;
+sum = patch->sig + patch->pf + patch->cksum;
 for ( i = 0; i < ext->count; ++i )
 if ( sum != (ext->sigs[i].sig + ext->sigs[i].pf + ext->sigs[i].cksum) )
 {
@@ -246,7 +234,7 @@ static enum microcode_match_result microcode_update_match(
 ASSERT(!microcode_sanity_check(mc));
 
 /* Check the main microcode signature. */
-if ( signature_matches(cpu_sig, mc->hdr.sig, mc->hdr.pf) )
+if ( signature_matches(cpu_sig, mc->sig, mc->pf) )
 goto found;
 
 /* If there is an extended signature table, check each of them. */
@@ -258,7 +246,7 @@ static enum microcode_match_result microcode_update_match(
 return MIS_UCODE;
 
  found:
-return mc->hdr.rev > cpu_sig->rev ? NEW_UCODE : OLD_UCODE;
+return mc->rev > cpu_sig->rev ? NEW_UCODE : OLD_UCODE;
 }
 
 static bool match_cpu(const struct microcode_patch *patch)
@@ -284,7 +272,7 @@ static enum microcode_match_result compare_patch(
 ASSERT(microcode_update_match(old) != MIS_UCODE);
 ASSERT(microcode_update_match(new) != MIS_UCODE);
 
-return (new->hdr.rev > old->hdr.rev) ? NEW_UCODE : OLD_UCODE;
+return new->rev > old->rev ? NEW_UCODE : OLD_UCODE;
 }
 
 static int apply_microcode(const struct microcode_patch *patch)
@@ -292,7 +280,6 @@ static int apply_microcode(const struct microcode_patch 
*patch)
 uint64_t msr_content;
 unsigned int cpu = smp_processor_id();
 struct cpu_signature *sig = _cpu(cpu_sig);
-const struct microcode_intel *mc_intel;
 uint32_t rev, old_rev = sig->rev;
 
 if ( !patch )
@@ -301,12 +288,10 @@ static int apply_microcode(const struct microcode_patch 
*patch)
 if ( !match_cpu(patch) )
 return -EINVAL;
 
-mc_intel = patch;
-
 BUG_ON(local_irq_is_enabled());
 
 /* write microcode via MSR 0x79 */
-wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->data);
+wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)patch->data);
 wrmsrl(MSR_IA32_UCODE_REV, 

[Xen-devel] [PATCH v2 5/7] x86/ucode/intel: Clean up microcode_update_match()

2020-03-27 Thread Andrew Cooper
Implement a new get_ext_sigtable() helper to abstract the logic for
identifying whether an extended signature table exists.  As part of this,
rename microcode_intel.bits to data and change its type so it can be usefully
used in combination with the datasize header field.

Also, replace the sigmatch() macro with a static inline with a more useful
API, and an explanation of why it is safe to drop one of the previous
conditionals.

No practical change in behaviour.

Signed-off-by: Andrew Cooper 
Reviewed-by: Jan Beulich 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

v2:
 * Rebase over struct microcode_patch re-work
 * Fix maches => matches typo
 * Retain constness on cast through void *
---
 xen/arch/x86/cpu/microcode/intel.c | 77 +-
 1 file changed, 51 insertions(+), 26 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index be2f4871dc..9d8d5bfc6e 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -60,7 +60,9 @@ struct microcode_header_intel {
 
 struct microcode_patch {
 struct microcode_header_intel hdr;
-unsigned int bits[0];
+
+/* Microcode payload.  Format is propriety and encrypted. */
+uint8_t data[];
 };
 
 /* Temporary, until the microcode_* structure are disentangled. */
@@ -96,8 +98,41 @@ static uint32_t get_totalsize(const struct microcode_patch 
*patch)
 return patch->hdr.totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
 }
 
-#define sigmatch(s1, s2, p1, p2) \
-(((s1) == (s2)) && (((p1) & (p2)) || (((p1) == 0) && ((p2) == 0
+/*
+ * A piece of microcode has an extended signature table if there is space
+ * between the end of data[] and the total size.  (This logic also works
+ * appropriately for Pentium Pro/II microcode, which has 0 for both size
+ * fields, and no extended signature table.)
+ */
+static const struct extended_sigtable *get_ext_sigtable(
+const struct microcode_patch *patch)
+{
+if ( patch->hdr.totalsize > (MC_HEADER_SIZE + patch->hdr.datasize) )
+return (const void *)>data[patch->hdr.datasize];
+
+return NULL;
+}
+
+/*
+ * A piece of microcode is applicable for a CPU if:
+ *  1) the signatures (CPUID.1.EAX - Family/Model/Stepping) match, and
+ *  2) The Platform Flags bitmap intersect.
+ *
+ * A CPU will have a single Platform Flag bit, while the microcode may be
+ * common to multiple platforms and have multiple bits set.
+ *
+ * Note: The Pentium Pro/II microcode didn't use platform flags, and should
+ * treat 0 as a match.  However, Xen being 64bit means that the CPU signature
+ * won't match, allowing us to simplify the logic.
+ */
+static bool signature_matches(const struct cpu_signature *cpu_sig,
+  unsigned int ucode_sig, unsigned int ucode_pf)
+{
+if ( cpu_sig->sig != ucode_sig )
+return false;
+
+return cpu_sig->pf & ucode_pf;
+}
 
 #define exttable_size(et) ((et)->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE)
 
@@ -219,36 +254,26 @@ static int microcode_sanity_check(const struct 
microcode_patch *mc)
 static enum microcode_match_result microcode_update_match(
 const struct microcode_patch *mc)
 {
-const struct microcode_header_intel *mc_header = >hdr;
-const struct extended_sigtable *ext_header;
-const struct extended_signature *ext_sig;
+const struct extended_sigtable *ext;
 unsigned int i;
 struct cpu_signature *cpu_sig = _cpu(cpu_sig);
-unsigned int sig = cpu_sig->sig;
-unsigned int pf = cpu_sig->pf;
-unsigned int rev = cpu_sig->rev;
-unsigned long data_size = get_datasize(mc);
-const void *end = (const void *)mc_header + get_totalsize(mc);
 
 ASSERT(!microcode_sanity_check(mc));
-if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
-return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
 
-ext_header = (const void *)(mc_header + 1) + data_size;
-ext_sig = (const void *)(ext_header + 1);
+/* Check the main microcode signature. */
+if ( signature_matches(cpu_sig, mc->hdr.sig, mc->hdr.pf) )
+goto found;
 
-/*
- * Make sure there is enough space to hold an extended header and enough
- * array elements.
- */
-if ( end <= (const void *)ext_sig )
-return MIS_UCODE;
-
-for ( i = 0; i < ext_header->count; i++ )
-if ( sigmatch(sig, ext_sig[i].sig, pf, ext_sig[i].pf) )
-return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
+/* If there is an extended signature table, check each of them. */
+if ( (ext = get_ext_sigtable(mc)) != NULL )
+for ( i = 0; i < ext->count; ++i )
+if ( signature_matches(cpu_sig, ext->sigs[i].sig, ext->sigs[i].pf) 
)
+goto found;
 
 return MIS_UCODE;
+
+ found:
+return mc->hdr.rev > cpu_sig->rev ? NEW_UCODE : OLD_UCODE;
 }
 
 static bool match_cpu(const struct microcode_patch *patch)
@@ -296,7 +321,7 @@ static int 

[Xen-devel] [PATCH v2 2/7] x86/ucode/intel: Adjust microcode_sanity_check() to not take void *

2020-03-27 Thread Andrew Cooper
microcode_sanity_check()'s callers actually call it with a mixture of
microcode_intel(/patch) and microcode_header_intel pointers, which is fragile.

Rework it to take struct microcode_patch *, which in turn requires
microcode_update_match()'s type to be altered.

No functional change - compiled binary is identical.

Signed-off-by: Andrew Cooper 
Reviewed-by: Jan Beulich 
---
CC: Wei Liu 
CC: Roger Pau Monné 

v2:
 * Rebase over struct microcode_patch re-work
---
 xen/arch/x86/cpu/microcode/intel.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index a69f7fe1de..77539a00be 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -118,9 +118,9 @@ static int collect_cpu_info(struct cpu_signature *csig)
 return 0;
 }
 
-static int microcode_sanity_check(const void *mc)
+static int microcode_sanity_check(const struct microcode_patch *mc)
 {
-const struct microcode_header_intel *mc_header = mc;
+const struct microcode_header_intel *mc_header = >hdr;
 const struct extended_sigtable *ext_header = NULL;
 const struct extended_signature *ext_sig;
 unsigned long total_size, data_size, ext_table_size;
@@ -152,7 +152,7 @@ static int microcode_sanity_check(const void *mc)
"Small exttable size in microcode data file\n");
 return -EINVAL;
 }
-ext_header = mc + MC_HEADER_SIZE + data_size;
+ext_header = (void *)mc + MC_HEADER_SIZE + data_size;
 if ( ext_table_size != exttable_size(ext_header) )
 {
 printk(KERN_ERR "microcode: error! "
@@ -210,8 +210,9 @@ static int microcode_sanity_check(const void *mc)
 
 /* Check an update against the CPU signature and current update revision */
 static enum microcode_match_result microcode_update_match(
-const struct microcode_header_intel *mc_header)
+const struct microcode_patch *mc)
 {
+const struct microcode_header_intel *mc_header = >hdr;
 const struct extended_sigtable *ext_header;
 const struct extended_signature *ext_sig;
 unsigned int i;
@@ -222,7 +223,7 @@ static enum microcode_match_result microcode_update_match(
 unsigned long data_size = get_datasize(mc_header);
 const void *end = (const void *)mc_header + get_totalsize(mc_header);
 
-ASSERT(!microcode_sanity_check(mc_header));
+ASSERT(!microcode_sanity_check(mc));
 if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
 return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
 
@@ -248,7 +249,7 @@ static bool match_cpu(const struct microcode_patch *patch)
 if ( !patch )
 return false;
 
-return microcode_update_match(>hdr) == NEW_UCODE;
+return microcode_update_match(patch) == NEW_UCODE;
 }
 
 static void free_patch(struct microcode_patch *patch)
@@ -263,8 +264,8 @@ static enum microcode_match_result compare_patch(
  * Both patches to compare are supposed to be applicable to local CPU.
  * Just compare the revision number.
  */
-ASSERT(microcode_update_match(>hdr) != MIS_UCODE);
-ASSERT(microcode_update_match(>hdr) != MIS_UCODE);
+ASSERT(microcode_update_match(old) != MIS_UCODE);
+ASSERT(microcode_update_match(new) != MIS_UCODE);
 
 return (new->hdr.rev > old->hdr.rev) ? NEW_UCODE : OLD_UCODE;
 }
@@ -361,7 +362,7 @@ static struct microcode_patch *cpu_request_microcode(const 
void *buf,
  * If the new update covers current CPU, compare updates and store the
  * one with higher revision.
  */
-if ( (microcode_update_match(>hdr) != MIS_UCODE) &&
+if ( (microcode_update_match(mc) != MIS_UCODE) &&
  (!saved || (mc->hdr.rev > saved->hdr.rev)) )
 {
 xfree(saved);
-- 
2.11.0




[Xen-devel] [PATCH v2 6/7] x86/ucode/intel: Clean up microcode_sanity_check()

2020-03-27 Thread Andrew Cooper
Rewrite the size checks in a way which doesn't depend on Xen being compiled as
64bit.

Introduce a check missing from the old code, that total_size is a multiple of
1024 bytes, and drop unnecessary defines/macros/structures.

No practical change in behaviour.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

v2:
 * Rebase over struct microcode_patch re-work
 * Retain constness on cast through void *
 * Reinstate printk()s for bad data
---
 xen/arch/x86/cpu/microcode/intel.c | 147 +
 1 file changed, 66 insertions(+), 81 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index 9d8d5bfc6e..1358a25032 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -69,24 +69,19 @@ struct microcode_patch {
 #define microcode_intel microcode_patch
 
 /* microcode format is extended from prescott processors */
-struct extended_signature {
-unsigned int sig;
-unsigned int pf;
-unsigned int cksum;
-};
-
 struct extended_sigtable {
 unsigned int count;
 unsigned int cksum;
 unsigned int reserved[3];
-struct extended_signature sigs[0];
+struct {
+unsigned int sig;
+unsigned int pf;
+unsigned int cksum;
+} sigs[];
 };
 
 #define PPRO_UCODE_DATASIZE 2000
 #define MC_HEADER_SIZE  (sizeof(struct microcode_header_intel))
-#define EXT_HEADER_SIZE (sizeof(struct extended_sigtable))
-#define EXT_SIGNATURE_SIZE  (sizeof(struct extended_signature))
-#define DWSIZE  (sizeof(u32))
 
 static uint32_t get_datasize(const struct microcode_patch *patch)
 {
@@ -134,8 +129,6 @@ static bool signature_matches(const struct cpu_signature 
*cpu_sig,
 return cpu_sig->pf & ucode_pf;
 }
 
-#define exttable_size(et) ((et)->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE)
-
 static int collect_cpu_info(struct cpu_signature *csig)
 {
 uint64_t msr_content;
@@ -160,93 +153,85 @@ static int collect_cpu_info(struct cpu_signature *csig)
 return 0;
 }
 
-static int microcode_sanity_check(const struct microcode_patch *mc)
+/*
+ * Sanity check a blob which is expected to be a microcode patch.  The 48 byte
+ * header is of a known format, and together with totalsize are within the
+ * bounds of the container.  Everything else is unchecked.
+ */
+static int microcode_sanity_check(const struct microcode_patch *patch)
 {
-const struct microcode_header_intel *mc_header = >hdr;
-const struct extended_sigtable *ext_header = NULL;
-const struct extended_signature *ext_sig;
-unsigned long total_size, data_size, ext_table_size;
-unsigned int ext_sigcount = 0, i;
-uint32_t sum, orig_sum;
-
-total_size = get_totalsize(mc);
-data_size = get_datasize(mc);
-if ( (data_size + MC_HEADER_SIZE) > total_size )
+const struct extended_sigtable *ext;
+const uint32_t *ptr;
+unsigned int total_size = get_totalsize(patch);
+unsigned int data_size = get_datasize(patch);
+unsigned int i, ext_size;
+uint32_t sum;
+
+/*
+ * Total size must be a multiple of 1024 bytes.  Data size and the header
+ * must fit within it.
+ */
+if ( (total_size & 1023) ||
+ data_size > (total_size - MC_HEADER_SIZE) )
 {
-printk(KERN_ERR "microcode: error! "
-   "Bad data size in microcode data file\n");
+printk(XENLOG_WARNING "microcode: Bad size\n");
 return -EINVAL;
 }
 
-if ( (mc_header->ldrver != 1) || (mc_header->hdrver != 1) )
-{
-printk(KERN_ERR "microcode: error! "
-   "Unknown microcode update format\n");
+/* Checksum the main header and data. */
+for ( sum = 0, ptr = (const uint32_t *)patch;
+  ptr < (const uint32_t *)>data[data_size]; ++ptr )
+sum += *ptr;
+
+if ( sum != 0 )
 return -EINVAL;
-}
-ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
-if ( ext_table_size )
+
+/* Look to see if there is an extended signature table. */
+ext_size = total_size - data_size - MC_HEADER_SIZE;
+
+/* No extended signature table?  All done. */
+if ( ext_size == 0 )
 {
-if ( (ext_table_size < EXT_HEADER_SIZE) ||
- ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE) )
-{
-printk(KERN_ERR "microcode: error! "
-   "Small exttable size in microcode data file\n");
-return -EINVAL;
-}
-ext_header = (void *)mc + MC_HEADER_SIZE + data_size;
-if ( ext_table_size != exttable_size(ext_header) )
-{
-printk(KERN_ERR "microcode: error! "
-   "Bad exttable size in microcode data file\n");
-return -EFAULT;
-}
-ext_sigcount = ext_header->count;
+printk(XENLOG_WARNING "microcode: Bad checksum\n");
+return 0;
 }
 
-/* check extended table checksum */
-if 

[Xen-devel] [PATCH v2 0/7] x86/ucode: Cleanup and fixes - Part 3/n (Intel)

2020-03-27 Thread Andrew Cooper
This supercedes the remnants of the Part 1 series, using Jan's suggested
alternative for making struct microcode_patch opaque.

Andrew Cooper (7):
  x86/ucode: Remove unnecessary indirection in struct microcode_patch
  x86/ucode/intel: Adjust microcode_sanity_check() to not take void *
  x86/ucode/intel: Remove gratuitous memory allocations from 
cpu_request_microcode()
  x86/ucode/intel: Reimplement get_{data,total}size() helpers
  x86/ucode/intel: Clean up microcode_update_match()
  x86/ucode/intel: Clean up microcode_sanity_check()
  x86/ucode/intel: Fold structures together

 xen/arch/x86/cpu/microcode/amd.c |  30 ++-
 xen/arch/x86/cpu/microcode/core.c|   3 +-
 xen/arch/x86/cpu/microcode/intel.c   | 362 +--
 xen/arch/x86/cpu/microcode/private.h |  11 +-
 4 files changed, 187 insertions(+), 219 deletions(-)

-- 
2.11.0




[Xen-devel] [PATCH v2 4/7] x86/ucode/intel: Reimplement get_{data, total}size() helpers

2020-03-27 Thread Andrew Cooper
Every caller actually passes a struct microcode_header_intel *, but it is more
helpful to us longterm to take struct microcode_patch *.  Implement the
helpers with proper types, and leave a comment explaining the Pentium Pro/II
behaviour with empty {data,total}size fields.

No functional change.

Signed-off-by: Andrew Cooper 
Reviewed-by: Jan Beulich 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

v2:
 * Rebase over struct microcode_patch re-work
 * Drop leading underscore
---
 xen/arch/x86/cpu/microcode/intel.c | 37 ++---
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index 2b48959573..be2f4871dc 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -46,6 +46,12 @@ struct microcode_header_intel {
 unsigned int sig;
 unsigned int cksum;
 unsigned int ldrver;
+
+/*
+ * Microcode for the Pentium Pro and II had all further fields in the
+ * header reserved, had a fixed datasize of 2000 and totalsize of 2048,
+ * and didn't use platform flags despite the availability of the MSR.
+ */
 unsigned int pf;
 unsigned int datasize;
 unsigned int totalsize;
@@ -74,20 +80,21 @@ struct extended_sigtable {
 struct extended_signature sigs[0];
 };
 
-#define DEFAULT_UCODE_DATASIZE  (2000)
+#define PPRO_UCODE_DATASIZE 2000
 #define MC_HEADER_SIZE  (sizeof(struct microcode_header_intel))
-#define DEFAULT_UCODE_TOTALSIZE (DEFAULT_UCODE_DATASIZE + MC_HEADER_SIZE)
 #define EXT_HEADER_SIZE (sizeof(struct extended_sigtable))
 #define EXT_SIGNATURE_SIZE  (sizeof(struct extended_signature))
 #define DWSIZE  (sizeof(u32))
-#define get_totalsize(mc) \
-(((struct microcode_intel *)mc)->hdr.totalsize ? \
- ((struct microcode_intel *)mc)->hdr.totalsize : \
- DEFAULT_UCODE_TOTALSIZE)
 
-#define get_datasize(mc) \
-(((struct microcode_intel *)mc)->hdr.datasize ? \
- ((struct microcode_intel *)mc)->hdr.datasize : DEFAULT_UCODE_DATASIZE)
+static uint32_t get_datasize(const struct microcode_patch *patch)
+{
+return patch->hdr.datasize ?: PPRO_UCODE_DATASIZE;
+}
+
+static uint32_t get_totalsize(const struct microcode_patch *patch)
+{
+return patch->hdr.totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
+}
 
 #define sigmatch(s1, s2, p1, p2) \
 (((s1) == (s2)) && (((p1) & (p2)) || (((p1) == 0) && ((p2) == 0
@@ -127,8 +134,8 @@ static int microcode_sanity_check(const struct 
microcode_patch *mc)
 unsigned int ext_sigcount = 0, i;
 uint32_t sum, orig_sum;
 
-total_size = get_totalsize(mc_header);
-data_size = get_datasize(mc_header);
+total_size = get_totalsize(mc);
+data_size = get_datasize(mc);
 if ( (data_size + MC_HEADER_SIZE) > total_size )
 {
 printk(KERN_ERR "microcode: error! "
@@ -220,8 +227,8 @@ static enum microcode_match_result microcode_update_match(
 unsigned int sig = cpu_sig->sig;
 unsigned int pf = cpu_sig->pf;
 unsigned int rev = cpu_sig->rev;
-unsigned long data_size = get_datasize(mc_header);
-const void *end = (const void *)mc_header + get_totalsize(mc_header);
+unsigned long data_size = get_datasize(mc);
+const void *end = (const void *)mc_header + get_totalsize(mc);
 
 ASSERT(!microcode_sanity_check(mc));
 if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
@@ -331,7 +338,7 @@ static struct microcode_patch *cpu_request_microcode(const 
void *buf,
  (mc = buf)->hdr.hdrver != 1 || /* Unrecognised header version?   
*/
  mc->hdr.ldrver != 1 || /* Unrecognised loader version?   
*/
  size < (blob_size =/* Insufficient space for patch?  
*/
- get_totalsize(>hdr)) )
+ get_totalsize(mc)) )
 {
 error = -EINVAL;
 printk(XENLOG_WARNING "microcode: Bad data in container\n");
@@ -356,7 +363,7 @@ static struct microcode_patch *cpu_request_microcode(const 
void *buf,
 
 if ( saved )
 {
-patch = xmemdup_bytes(saved, get_totalsize(>hdr));
+patch = xmemdup_bytes(saved, get_totalsize(saved));
 
 if ( !patch )
 error = -ENOMEM;
-- 
2.11.0




[Xen-devel] [PATCH v2 1/7] x86/ucode: Remove unnecessary indirection in struct microcode_patch

2020-03-27 Thread Andrew Cooper
Currently, each cpu_request_microcode() allocates a struct microcode_patch,
which is a single pointer to a separate allocated structure.  This is
wasteful.

Fixing this is complicated because the common microcode_free_patch() code is
responsible for freeing struct microcode_patch, despite this being asymmetric
with how it is allocated.

Make struct microcode_patch fully opaque to the common logic.  This involves
moving the responsibility for freeing struct microcode_patch fully into the
free_patch() hook.

In each vendor logic, use some temporary ifdef-ary (cleaned up in subsequent
changes) to reduce the churn as much as possible, and forgo allocating the
intermediate pointer in cpu_request_microcode().

Suggested-by: Jan Beulich 
Signed-off-by: Jan Beulich 
Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

v2:
 * New
---
 xen/arch/x86/cpu/microcode/amd.c | 30 --
 xen/arch/x86/cpu/microcode/core.c|  3 +--
 xen/arch/x86/cpu/microcode/intel.c   | 31 ---
 xen/arch/x86/cpu/microcode/private.h | 11 +++
 4 files changed, 28 insertions(+), 47 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 122b8309af..1b9373f0d9 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -60,13 +60,16 @@ struct __packed microcode_header_amd {
 #define UCODE_EQUIV_CPU_TABLE_TYPE 0x
 #define UCODE_UCODE_TYPE   0x0001
 
-struct microcode_amd {
+struct microcode_patch {
 void *mpb;
 size_t mpb_size;
 struct equiv_cpu_entry *equiv_cpu_table;
 size_t equiv_cpu_table_size;
 };
 
+/* Temporary, until the microcode_* structure are disentangled. */
+#define microcode_amd microcode_patch
+
 struct mpbhdr {
 uint32_t type;
 uint32_t len;
@@ -177,13 +180,11 @@ static enum microcode_match_result microcode_fits(
 
 static bool match_cpu(const struct microcode_patch *patch)
 {
-return patch && (microcode_fits(patch->mc_amd) == NEW_UCODE);
+return patch && (microcode_fits(patch) == NEW_UCODE);
 }
 
-static void free_patch(void *mc)
+static void free_patch(struct microcode_patch *mc_amd)
 {
-struct microcode_amd *mc_amd = mc;
-
 if ( mc_amd )
 {
 xfree(mc_amd->equiv_cpu_table);
@@ -206,12 +207,12 @@ static enum microcode_match_result compare_header(
 static enum microcode_match_result compare_patch(
 const struct microcode_patch *new, const struct microcode_patch *old)
 {
-const struct microcode_header_amd *new_header = new->mc_amd->mpb;
-const struct microcode_header_amd *old_header = old->mc_amd->mpb;
+const struct microcode_header_amd *new_header = new->mpb;
+const struct microcode_header_amd *old_header = old->mpb;
 
 /* Both patches to compare are supposed to be applicable to local CPU. */
-ASSERT(microcode_fits(new->mc_amd) != MIS_UCODE);
-ASSERT(microcode_fits(old->mc_amd) != MIS_UCODE);
+ASSERT(microcode_fits(new) != MIS_UCODE);
+ASSERT(microcode_fits(old) != MIS_UCODE);
 
 return compare_header(new_header, old_header);
 }
@@ -230,7 +231,7 @@ static int apply_microcode(const struct microcode_patch 
*patch)
 if ( !match_cpu(patch) )
 return -EINVAL;
 
-hdr = patch->mc_amd->mpb;
+hdr = patch->mpb;
 
 BUG_ON(local_irq_is_enabled());
 
@@ -554,14 +555,7 @@ static struct microcode_patch *cpu_request_microcode(const 
void *buf,
 {
 mc_amd->mpb = saved;
 mc_amd->mpb_size = saved_size;
-patch = xmalloc(struct microcode_patch);
-if ( patch )
-patch->mc_amd = mc_amd;
-else
-{
-free_patch(mc_amd);
-error = -ENOMEM;
-}
+patch = mc_amd;
 }
 else
 free_patch(mc_amd);
diff --git a/xen/arch/x86/cpu/microcode/core.c 
b/xen/arch/x86/cpu/microcode/core.c
index 61150e04c8..b3e5913d49 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -245,8 +245,7 @@ static struct microcode_patch *parse_blob(const char *buf, 
size_t len)
 
 static void microcode_free_patch(struct microcode_patch *microcode_patch)
 {
-microcode_ops->free_patch(microcode_patch->mc);
-xfree(microcode_patch);
+microcode_ops->free_patch(microcode_patch);
 }
 
 /* Return true if cache gets updated. Otherwise, return false */
diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index 78455aa0ae..a69f7fe1de 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -52,11 +52,14 @@ struct microcode_header_intel {
 unsigned int reserved[3];
 };
 
-struct microcode_intel {
+struct microcode_patch {
 struct microcode_header_intel hdr;
 unsigned int bits[0];
 };
 
+/* Temporary, until the microcode_* structure are disentangled. */
+#define microcode_intel microcode_patch
+
 /* microcode format is extended from prescott processors */
 

[Xen-devel] [linux-5.4 test] 149052: regressions - trouble: fail/pass/starved

2020-03-27 Thread osstest service owner
flight 149052 linux-5.4 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/149052/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
146121

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 12 guest-start  fail REGR. vs. 146121

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-dom0pvh-xl-intel 15 guest-saverestore  fail baseline untested
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-dom0pvh-xl-amd  2 hosts-allocate   starved  n/a

version targeted for testing:
 linux462afcd6e7ea94a7027a96a3bb12d0140b0b4216
baseline version:
 linux122179cb7d648a6f36b20dd6bf34f953cb384c30

Last test of basis   146121  2020-01-15 17:42:04 Z   71 days
Failing since146178  2020-01-17 02:59:07 Z   70 days   96 attempts
Testing same since   149052  2020-03-26 11:07:11 Z1 days1 attempts


[Xen-devel] [PATCH] x86/ucode: Drop the sanity check for interrupts being disabled

2020-03-27 Thread Andrew Cooper
Of the substantial number of things which can go wrong during microcode load,
this is not one.  Loading occurs entirely within the boundary of a single
WRMSR instruction.  Its certainly not a BUG()-worthy condition.

Xen has legitimate reasons to not want interrupts enabled at this point, but
that is to do with organising the system rendezvous.  As these are private low
level helpers invoked only from the microcode core logic, forgo the check
entirely.

While dropping system.h, clean up the processor.h include which was an
oversight in the previous header cleanup.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

An ASSERT() would be one option, but I'd prefer to remove the include.
---
 xen/arch/x86/cpu/microcode/amd.c   | 4 
 xen/arch/x86/cpu/microcode/intel.c | 4 
 2 files changed, 8 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 01854be92c..9fe1a3c941 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -20,8 +20,6 @@
 
 #include 
 #include 
-#include 
-#include 
 
 #include "private.h"
 
@@ -237,8 +235,6 @@ static int apply_microcode(const struct microcode_patch 
*patch)
 
 hdr = patch->mpb;
 
-BUG_ON(local_irq_is_enabled());
-
 hw_err = wrmsr_safe(MSR_AMD_PATCHLOADER, (unsigned long)hdr);
 
 /* get patch id after patching */
diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index bd33f5bc9b..b8b28060f5 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -25,8 +25,6 @@
 #include 
 
 #include 
-#include 
-#include 
 
 #include "private.h"
 
@@ -304,8 +302,6 @@ static int apply_microcode(const struct microcode_patch 
*patch)
 if ( !match_cpu(patch) )
 return -EINVAL;
 
-BUG_ON(local_irq_is_enabled());
-
 /* write microcode via MSR 0x79 */
 wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)patch->data);
 
-- 
2.11.0




[Xen-devel] [PATCH] x86/ucode/amd: Fix buffer overrun with equiv table handling

2020-03-27 Thread Andrew Cooper
find_equiv_cpu_id() loops until it finds a 0 installed_cpu entry.  Well formed
AMD microcode containers have this property.

Extend the checking in install_equiv_cpu_table() to reject tables which don't
have a sentinal at the end.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 

All of this logic needs rewriting, but this is the minimally invasive version
for backport.
---
 xen/arch/x86/cpu/microcode/amd.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index b8b83d248d..9fe1a3c941 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -310,6 +310,7 @@ static int install_equiv_cpu_table(
 size_t *offset)
 {
 const struct mpbhdr *mpbuf = data + *offset + 4;
+const struct equiv_cpu_entry *eq;
 
 *offset += mpbuf->len + CONT_HDR_SIZE; /* add header length */
 
@@ -319,7 +320,9 @@ static int install_equiv_cpu_table(
 return -EINVAL;
 }
 
-if ( mpbuf->len == 0 )
+if ( mpbuf->len == 0 || mpbuf->len % sizeof(*eq) ||
+ (eq = (const void *)mpbuf->data,
+  eq[(mpbuf->len / sizeof(*eq)) - 1].installed_cpu) )
 {
 printk(KERN_ERR "microcode: Wrong microcode equivalent cpu table 
length\n");
 return -EINVAL;
-- 
2.11.0




[Xen-devel] [PATCH] automation/gitlab: add https transport support to Debian images

2020-03-27 Thread Roger Pau Monne
The LLVM repos have switched from http to https, and trying to access
using http will get redirected to https. Add the apt-transport-https
package to the x86 Debian containers that use the LLVM repos, in order
to support the https transport method.

Note that on Arm we only test with gcc, so don't add the package for
the Debian Arm container.

This fixes the following error seen on the QEMU smoke tests:

E: The method driver /usr/lib/apt/methods/https could not be found.

Signed-off-by: Roger Pau Monné 
---
Cc: Wei Liu 
---
Should I try to push the updated containers myself?
---
 automation/build/debian/stretch-i386.dockerfile  | 1 +
 automation/build/debian/stretch.dockerfile   | 1 +
 automation/build/debian/unstable-i386.dockerfile | 1 +
 automation/build/debian/unstable.dockerfile  | 1 +
 4 files changed, 4 insertions(+)

diff --git a/automation/build/debian/stretch-i386.dockerfile 
b/automation/build/debian/stretch-i386.dockerfile
index 4797ae3442..7b6f8eff69 100644
--- a/automation/build/debian/stretch-i386.dockerfile
+++ b/automation/build/debian/stretch-i386.dockerfile
@@ -45,6 +45,7 @@ RUN apt-get update && \
 wget \
 git \
 nasm \
+apt-transport-https \
 && \
 apt-get autoremove -y && \
 apt-get clean && \
diff --git a/automation/build/debian/stretch.dockerfile 
b/automation/build/debian/stretch.dockerfile
index cfbb2e9b0b..32742f7f39 100644
--- a/automation/build/debian/stretch.dockerfile
+++ b/automation/build/debian/stretch.dockerfile
@@ -44,6 +44,7 @@ RUN apt-get update && \
 git \
 nasm \
 gnupg \
+apt-transport-https \
 && \
 apt-get autoremove -y && \
 apt-get clean && \
diff --git a/automation/build/debian/unstable-i386.dockerfile 
b/automation/build/debian/unstable-i386.dockerfile
index 1a73b3b1ec..86ff3585df 100644
--- a/automation/build/debian/unstable-i386.dockerfile
+++ b/automation/build/debian/unstable-i386.dockerfile
@@ -45,6 +45,7 @@ RUN apt-get update && \
 wget \
 git \
 nasm \
+apt-transport-https \
 && \
 apt-get autoremove -y && \
 apt-get clean && \
diff --git a/automation/build/debian/unstable.dockerfile 
b/automation/build/debian/unstable.dockerfile
index 2a834f6719..d0aa5ad2bb 100644
--- a/automation/build/debian/unstable.dockerfile
+++ b/automation/build/debian/unstable.dockerfile
@@ -44,6 +44,7 @@ RUN apt-get update && \
 git \
 nasm \
 gnupg \
+apt-transport-https \
 && \
 apt-get autoremove -y && \
 apt-get clean && \
-- 
2.26.0




Re: [Xen-devel] [PATCH 13/17] xen/x86: p2m: Reflow P2M_PRINTK()s in p2m_pt_audit_p2m()

2020-03-27 Thread Jan Beulich
On 22.03.2020 17:14, jul...@xen.org wrote:
> From: Julien Grall 
> 
> We tend to avoid splitting message on multiple line, so it is easier to
> find it.
> 
> Signed-off-by: Julien Grall 

Acked-by: Jan Beulich 



Re: [Xen-devel] [PATCH 12/17] xen/x86: p2m: Remove duplicate error message in p2m_pt_audit_p2m()

2020-03-27 Thread Jan Beulich
On 22.03.2020 17:14, jul...@xen.org wrote:
> From: Julien Grall 
> 
> p2m_pt_audit_p2m() has one place where the same message may be printed
> twice via printk and P2M_PRINTK.
> 
> Remove the one printed using printk to stay consistent with the rest of
> the code.
> 
> Signed-off-by: Julien Grall 

Acked-by: Jan Beulich 



Re: [Xen-devel] [PATCH 11/17] xen/x86: nested_ept: Fix typo in the message in nept_translate_l2ga()

2020-03-27 Thread Jan Beulich
On 22.03.2020 17:14, jul...@xen.org wrote:
> From: Julien Grall 
> 
> Signed-off-by: Julien Grall 

Acked-by: Jan Beulich 



Re: [Xen-devel] [PATCH 10/17] xen/x86: pv: Use maddr_to_mfn(...) instead of the open-coding version

2020-03-27 Thread Jan Beulich
On 22.03.2020 17:14, jul...@xen.org wrote:
> From: Julien Grall 
> 
> _mfn(addr >> PAGE_SHIFT) is equivalent to maddr_to_mfn(addr).
> 
> Signed-off-by: Julien Grall 

Acked-by: Jan Beulich 




[Xen-devel] [linux-linus test] 149049: regressions - trouble: fail/pass/starved

2020-03-27 Thread osstest service owner
flight 149049 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/149049/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
133580
 test-armhf-armhf-libvirt 19 leak-check/check fail REGR. vs. 133580

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

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 133580
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 133580
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 133580
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 133580
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-dom0pvh-xl-intel 15 guest-saverestore fail never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-dom0pvh-xl-amd  2 hosts-allocate   starved  n/a

version targeted for testing:
 linux1b649e0bcae71c118c1333e02249a7510ba7f70a
baseline version:
 linux

[Xen-devel] Fwd: [ANNOUNCE] Call for agenda items for April 2020 Community Call @ 15:00 UTC

2020-03-27 Thread George Dunlap
Forgot to cc xen-devel

Begin forwarded message:

From: George Dunlap mailto:george.dun...@citrix.com>>
Subject: [ANNOUNCE] Call for agenda items for April 2020 Community Call @ 15:00 
UTC
Date: March 26, 2020 at 6:54:31 PM GMT

Hi all,

The proposed agenda is in 
https://cryptpad.fr/pad/#/2/pad/edit/l2490xsQrrv3jPyMazPN8MMW/ and you can edit 
to add items. Alternatively, you can reply to this mail directly

Agenda items appreciated a few days before the call: please put your name 
besides items if you edit the document

Note the following administrative conventions for the call
* Unless agreed in the pervious meeting otherwise, the call is on the 1st 
Thursday of each month at 1600 British Time (either GMT or BST)
* I usually send out a meeting reminder a few days before with a provisional 
agenda

* If you want to be CC'ed please add or remove yourself from the sign-up-sheet 
at https://cryptpad.fr/pad/#/2/pad/edit/D9vGzihPxxAOe6RFPz0sRCf+/

Best Regards
George



== Dial-in Information ==
## Meeting time
15:00 - 16:00 UTC
Further International meeting times: 
https://www.timeanddate.com/worldclock/meetingdetails.html?year=2020=4=2=15=0=0=1234=37=224=179


## Dial in details
Web: https://www.gotomeet.me/GeorgeDunlap

You can also dial in using your phone.
Access Code: 168-682-109

China (Toll Free): 4008 811084
Germany: +49 692 5736 7317
Poland (Toll Free): 00 800 1124759
Ukraine (Toll Free): 0 800 50 1733
United Kingdom: +44 330 221 0088
United States: +1 (571) 317-3129
Spain: +34 932 75 2004


More phone numbers
Australia: +61 2 9087 3604
Austria: +43 7 2081 5427
Argentina (Toll Free): 0 800 444 3375
Bahrain (Toll Free): 800 81 111
Belarus (Toll Free): 8 820 0011 0400
Belgium: +32 28 93 7018
Brazil (Toll Free): 0 800 047 4906
Bulgaria (Toll Free): 00800 120 4417
Canada: +1 (647) 497-9391
Chile (Toll Free): 800 395 150
Colombia (Toll Free): 01 800 518 4483
Czech Republic (Toll Free): 800 500448
Denmark: +45 32 72 03 82
Finland: +358 923 17 0568
France: +33 170 950 594
Greece (Toll Free): 00 800 4414 3838
Hong Kong (Toll Free): 30713169906-886-965
Hungary (Toll Free): (06) 80 986 255
Iceland (Toll Free): 800 7204
India (Toll Free): 18002669272
Indonesia (Toll Free): 007 803 020 5375
Ireland: +353 15 360 728
Israel (Toll Free): 1 809 454 830
Italy: +39 0 247 92 13 01
Japan (Toll Free): 0 120 663 800
Korea, Republic of (Toll Free): 00798 14 207 4914
Luxembourg (Toll Free): 800 85158
Malaysia (Toll Free): 1 800 81 6854
Mexico (Toll Free): 01 800 522 1133
Netherlands: +31 207 941 377
New Zealand: +64 9 280 6302
Norway: +47 21 93 37 51
Panama (Toll Free): 00 800 226 7928
Peru (Toll Free): 0 800 77023
Philippines (Toll Free): 1 800 1110 1661
Portugal (Toll Free): 800 819 575
Romania (Toll Free): 0 800 410 029
Russian Federation (Toll Free): 8 800 100 6203
Saudi Arabia (Toll Free): 800 844 3633
Singapore (Toll Free): 18007231323
South Africa (Toll Free): 0 800 555 447
Sweden: +46 853 527 827
Switzerland: +41 225 4599 78
Taiwan (Toll Free): 0 800 666 854
Thailand (Toll Free): 001 800 011 023
Turkey (Toll Free): 00 800 4488 23683
United Arab Emirates (Toll Free): 800 044 40439
Uruguay (Toll Free): 0004 019 1018
Viet Nam (Toll Free): 122 80 481
​​​

First GoToMeeting? Let's do a quick system check:

https://link.gotomeeting.com/system-check



Re: [Xen-devel] [PATCH 09/17] xen/x86: Reduce the number of use of l*e_{from, get}_pfn()

2020-03-27 Thread Jan Beulich
On 22.03.2020 17:14, jul...@xen.org wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -1138,7 +1138,7 @@ static int
>  get_page_from_l2e(
>  l2_pgentry_t l2e, mfn_t l2mfn, struct domain *d, unsigned int flags)
>  {
> -unsigned long mfn = l2e_get_pfn(l2e);
> +mfn_t mfn = l2e_get_mfn(l2e);
>  int rc;
>  
>  if ( unlikely((l2e_get_flags(l2e) & L2_DISALLOW_MASK)) )
> @@ -1150,7 +1150,7 @@ get_page_from_l2e(
>  
>  ASSERT(!(flags & PTF_preemptible));
>  
> -rc = get_page_and_type_from_mfn(_mfn(mfn), PGT_l1_page_table, d, flags);
> +rc = get_page_and_type_from_mfn(mfn, PGT_l1_page_table, d, flags);

To bring this better in line with the L3 and L4 counterparts,
could you please drop the local variable instead? Then
Reviewed-by: Jan Beulich 

Jan



Re: [Xen-devel] [PATCH] xen/softirq: adjust comment

2020-03-27 Thread Jan Beulich
On 27.03.2020 11:31, Juergen Gross wrote:
> With commit cef21210fb133 ("rcu: don't process callbacks when holding
> a rcu_read_lock()") the comment in process_pending_softirqs() about
> not entering the scheduler should have been moved.
> 
> Signed-off-by: Juergen Gross 

Acked-by: Jan Beulich 



[Xen-devel] [PATCH] xen/softirq: adjust comment

2020-03-27 Thread Juergen Gross
With commit cef21210fb133 ("rcu: don't process callbacks when holding
a rcu_read_lock()") the comment in process_pending_softirqs() about
not entering the scheduler should have been moved.

Signed-off-by: Juergen Gross 
---
 xen/common/softirq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/softirq.c b/xen/common/softirq.c
index eba65c5fc0..063e93cbe3 100644
--- a/xen/common/softirq.c
+++ b/xen/common/softirq.c
@@ -56,6 +56,7 @@ static void __do_softirq(unsigned long ignore_mask)
 
 void process_pending_softirqs(void)
 {
+/* Do not enter scheduler as it can preempt the calling context. */
 unsigned long ignore_mask = (1ul << SCHEDULE_SOFTIRQ) |
 (1ul << SCHED_SLAVE_SOFTIRQ);
 
@@ -64,7 +65,6 @@ void process_pending_softirqs(void)
 ignore_mask |= 1ul << RCU_SOFTIRQ;
 
 ASSERT(!in_irq() && local_irq_is_enabled());
-/* Do not enter scheduler as it can preempt the calling context. */
 __do_softirq(ignore_mask);
 }
 
-- 
2.16.4




Re: [Xen-devel] [PATCH v8 3/5] xen: don't process rcu callbacks when holding a rcu_read_lock()

2020-03-27 Thread Igor Druzhinin
On 27/03/2020 08:35, Jan Beulich wrote:
> On 27.03.2020 09:10, Jürgen Groß wrote:
>> On 27.03.20 00:24, Igor Druzhinin wrote:
>>> On 26/03/2020 09:19, Juergen Gross wrote:
 Some keyhandlers are calling process_pending_softirqs() while holding
 a rcu_read_lock(). This is wrong, as process_pending_softirqs() might
 activate rcu calls which should not happen inside a rcu_read_lock().

 For that purpose modify process_pending_softirqs() to not allow rcu
 callback processing when a rcu_read_lock() is being held.

 Signed-off-by: Juergen Gross 
 Reviewed-by: Jan Beulich 
 ---
 V3:
 - add RCU_SOFTIRQ to ignore in process_pending_softirqs_norcu()
    (Roger Pau Monné)

 V5:
 - block rcu processing depending on rch_read_lock() being held or not
    (Jan Beulich)
>>>
>>> Juergen,
>>>
>>> Our BVT revealed a likely problem with this commit in that form.
>>> Since 12509bbeb9e ("rwlocks: call preempt_disable() when taking a rwlock")
>>> preemption is disabled after taking cpu_maps which will block RCU
>>> callback processing inside rcu_barrier itself. This will result in
>>
>> Why would that block RCU callback processing?
>>
>> RCU callbacks should be blocked only if a rcu lock is being held.
>>
>> Did I miss something in my patches?
> 
> Igor, are you perhaps running without "rcu: add assertions to debug
> build"? I think this actually fixes what you describe. Without it
> rcu_barrier(), in its second loop, calling process_pending_softirqs(),
> would cause the RCU softirq to not be invoked anymore with preemption
> disabled. Of course the title of this change doesn't reflect this at
> all.

Yes, that explains it - I indeed skipped that patch from backporting to
our tree. Thanks, for the quick catch!

Igor




Re: [Xen-devel] [PATCH v3 3/4] x86/nvmx: split updating RVI from SVI in nvmx_update_apicv

2020-03-27 Thread Roger Pau Monné
On Fri, Mar 27, 2020 at 02:21:46AM +, Tian, Kevin wrote:
> > From: Roger Pau Monne 
> > Sent: Thursday, March 26, 2020 11:27 PM
> > 
> > Updating SVI is required when an interrupt has been injected using the
> > Ack on exit VMEXIT feature, so that the in service interrupt in the
> > GUEST_INTR_STATUS matches the vector that is signaled in
> > VM_EXIT_INTR_INFO.
> > 
> > Updating RVI however is not tied to the Ack on exit feature, as it
> > signals the next vector to be injected, and hence should always be
> > updated to the next pending vector, regardless of whether Ack on exit
> > is enabled.
> > 
> > When not using the Ack on exit feature preserve the previous vector in
> > SVI, so that it's not lost when RVI is updated to contain the pending
> > vector to inject.
> > 
> > Signed-off-by: Roger Pau Monné 
> 
> Reviewed-by: Kevin Tian , with one small comment:
> 
> > ---
> > Changes since v2:
> >  - Return early if the exit reason != EXTERNAL_INTERRUPT.
> >  - Reduce the number of vmwrites by accumulating the changes to a
> >local variable which is flushed at the end of the function.
> >  - Attempt to preserve the exiting SVI if Ack on exit is not enabled.
> > ---
> >  xen/arch/x86/hvm/vmx/vvmx.c | 33 -
> >  1 file changed, 24 insertions(+), 9 deletions(-)
> > 
> > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> > index 1753005c91..39fb553590 100644
> > --- a/xen/arch/x86/hvm/vmx/vvmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> > @@ -1384,28 +1384,43 @@ static void nvmx_update_apicv(struct vcpu *v)
> >  struct nestedvmx *nvmx = _2_nvmx(v);
> >  unsigned long reason = get_vvmcs(v, VM_EXIT_REASON);
> >  unsigned long intr_info = get_vvmcs(v, VM_EXIT_INTR_INFO);
> > +unsigned long status;
> > +int rvi;
> > 
> > -if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
> > - nvmx->intr.source == hvm_intsrc_lapic &&
> > +if ( reason != EXIT_REASON_EXTERNAL_INTERRUPT )
> > +return;
> 
> can we also exit if source is not lapic? as we discussed in another
> thread, the whole logic here is only for lapic not others...

Right, in any case the code below will only update GUEST_INTR_STATUS
(either SVI or RVI) for lapic interrupts, but returning early if the
source is not the lapic will do no harm AFAICT.

Will send v4 with this changed.

Thanks, Roger.



Re: [Xen-devel] PCIe IOMMU ACS support

2020-03-27 Thread Paul Durrant
> -Original Message-
> From: Roman Shaposhnik 
> Sent: 26 March 2020 22:03
> To: Roger Pau Monné 
> Cc: xen-devel@lists.xenproject.org; Jan Beulich ; Paul 
> Durrant ;
> Kevin Tian ; Andrew Cooper 
> Subject: Re: [Xen-devel] PCIe IOMMU ACS support
> 
> On Wed, Mar 25, 2020 at 4:05 AM Roger Pau Monné  wrote:
> >
> > Adding the PCI and IOMMU maintainers.
> >
> > On Mon, Mar 23, 2020 at 01:55:01PM -0700, Roman Shaposhnik wrote:
> > > Hi!
> > >
> > > I was going through how Xen support PCIe IOMMU ACS and
> > > all I could find is this:
> > > 
> > > https://github.com/xen-project/xen/blob/master/xen/drivers/passthrough/pci.c#L608
> > > which looks to me as an attempt of enabling ACS opportunistically,
> > > but still proceeding forward even if it fails.
> >
> > That's correct AFAICT. Xen will try to enable some features, but will
> > proceed normally if ACS is not available, or if some of the features
> > are not implemented.
> >
> > Are you looking to ensure that all devices on the system have a
> > certain feature enabled?
> 
> My primary objective was to get some visibility into how Xen would
> prevent two PCIe devices behind a common bridge from doing p2p
> transactions (thus violating VM boundaries if those devices are
> assigned to different domains).
> 
> It looks like Xen simply trusts the hardware.
> 
> > Can you provide some more details about what you expect of ACS
> > handling?
> 
> I was actually surprised not to see IOMMU groups in the style of what
> VFIO https://www.kernel.org/doc/Documentation/vfio.txt
> 

I did write a doc some time ago to present the issues facing Xen w.r.t. IOMMU 
and device pass-through. Hopefully you can see it at 
https://docs.google.com/document/d/12-z6JD41J_oNrCg_c0yAxGWg5ADBQ8_bSiP_NH6Hqwo/edit?usp=sharing

  Paul




Re: [Xen-devel] [PATCH v8 3/5] xen: don't process rcu callbacks when holding a rcu_read_lock()

2020-03-27 Thread Jürgen Groß

On 27.03.20 09:35, Jan Beulich wrote:

On 27.03.2020 09:10, Jürgen Groß wrote:

On 27.03.20 00:24, Igor Druzhinin wrote:

On 26/03/2020 09:19, Juergen Gross wrote:

Some keyhandlers are calling process_pending_softirqs() while holding
a rcu_read_lock(). This is wrong, as process_pending_softirqs() might
activate rcu calls which should not happen inside a rcu_read_lock().

For that purpose modify process_pending_softirqs() to not allow rcu
callback processing when a rcu_read_lock() is being held.

Signed-off-by: Juergen Gross 
Reviewed-by: Jan Beulich 
---
V3:
- add RCU_SOFTIRQ to ignore in process_pending_softirqs_norcu()
    (Roger Pau Monné)

V5:
- block rcu processing depending on rch_read_lock() being held or not
    (Jan Beulich)


Juergen,

Our BVT revealed a likely problem with this commit in that form.
Since 12509bbeb9e ("rwlocks: call preempt_disable() when taking a rwlock")
preemption is disabled after taking cpu_maps which will block RCU
callback processing inside rcu_barrier itself. This will result in


Why would that block RCU callback processing?

RCU callbacks should be blocked only if a rcu lock is being held.

Did I miss something in my patches?


Igor, are you perhaps running without "rcu: add assertions to debug
build"? I think this actually fixes what you describe. Without it
rcu_barrier(), in its second loop, calling process_pending_softirqs(),
would cause the RCU softirq to not be invoked anymore with preemption
disabled. Of course the title of this change doesn't reflect this at
all.


Right. This explains why I don't see the hang on my test system.



Jürgen, as an aside, while looking at the code again, I think the
comment near the end of process_pending_softirqs() would now rather
belong at its very beginning; should have spotted this while
reviewing.


Oh, indeed. Will send a patch.


Juergen



Re: [Xen-devel] [PATCH v8 3/5] xen: don't process rcu callbacks when holding a rcu_read_lock()

2020-03-27 Thread Jan Beulich
On 27.03.2020 09:10, Jürgen Groß wrote:
> On 27.03.20 00:24, Igor Druzhinin wrote:
>> On 26/03/2020 09:19, Juergen Gross wrote:
>>> Some keyhandlers are calling process_pending_softirqs() while holding
>>> a rcu_read_lock(). This is wrong, as process_pending_softirqs() might
>>> activate rcu calls which should not happen inside a rcu_read_lock().
>>>
>>> For that purpose modify process_pending_softirqs() to not allow rcu
>>> callback processing when a rcu_read_lock() is being held.
>>>
>>> Signed-off-by: Juergen Gross 
>>> Reviewed-by: Jan Beulich 
>>> ---
>>> V3:
>>> - add RCU_SOFTIRQ to ignore in process_pending_softirqs_norcu()
>>>    (Roger Pau Monné)
>>>
>>> V5:
>>> - block rcu processing depending on rch_read_lock() being held or not
>>>    (Jan Beulich)
>>
>> Juergen,
>>
>> Our BVT revealed a likely problem with this commit in that form.
>> Since 12509bbeb9e ("rwlocks: call preempt_disable() when taking a rwlock")
>> preemption is disabled after taking cpu_maps which will block RCU
>> callback processing inside rcu_barrier itself. This will result in
> 
> Why would that block RCU callback processing?
> 
> RCU callbacks should be blocked only if a rcu lock is being held.
> 
> Did I miss something in my patches?

Igor, are you perhaps running without "rcu: add assertions to debug
build"? I think this actually fixes what you describe. Without it
rcu_barrier(), in its second loop, calling process_pending_softirqs(),
would cause the RCU softirq to not be invoked anymore with preemption
disabled. Of course the title of this change doesn't reflect this at
all.

Jürgen, as an aside, while looking at the code again, I think the
comment near the end of process_pending_softirqs() would now rather
belong at its very beginning; should have spotted this while
reviewing.

Jan



Re: [Xen-devel] [PATCH v8 3/5] xen: don't process rcu callbacks when holding a rcu_read_lock()

2020-03-27 Thread Jürgen Groß

On 27.03.20 00:24, Igor Druzhinin wrote:

On 26/03/2020 09:19, Juergen Gross wrote:

Some keyhandlers are calling process_pending_softirqs() while holding
a rcu_read_lock(). This is wrong, as process_pending_softirqs() might
activate rcu calls which should not happen inside a rcu_read_lock().

For that purpose modify process_pending_softirqs() to not allow rcu
callback processing when a rcu_read_lock() is being held.

Signed-off-by: Juergen Gross 
Reviewed-by: Jan Beulich 
---
V3:
- add RCU_SOFTIRQ to ignore in process_pending_softirqs_norcu()
   (Roger Pau Monné)

V5:
- block rcu processing depending on rch_read_lock() being held or not
   (Jan Beulich)


Juergen,

Our BVT revealed a likely problem with this commit in that form.
Since 12509bbeb9e ("rwlocks: call preempt_disable() when taking a rwlock")
preemption is disabled after taking cpu_maps which will block RCU
callback processing inside rcu_barrier itself. This will result in


Why would that block RCU callback processing?

RCU callbacks should be blocked only if a rcu lock is being held.

Did I miss something in my patches?


all system hang on boot after 540d4d60378 ("cpu: sync any remaining
RCU callbacks before CPU up/down").



Juergen



Re: [Xen-devel] [PATCH v3] SVM: Add union intstat_t for offset 68h in vmcb struct

2020-03-27 Thread Jan Beulich
On 26.03.2020 14:56, Andrew Cooper wrote:
> On 26/03/2020 13:44, Pu Wen wrote:
>> According to chapter "Appendix B Layout of VMCB" in the new version
>> (v3.32) AMD64 APM[1], bit 1 of the VMCB offset 68h is defined as
>> GUEST_INTERRUPT_MASK.
>>
>> In current xen codes, it use whole u64 interrupt_shadow to setup
>> interrupt shadow, which will misuse other bit in VMCB offset 68h
>> as part of interrupt_shadow, causing svm_get_interrupt_shadow() to
>> mistake the guest having interrupts enabled as being in an interrupt
>> shadow.  This has been observed to cause SeaBIOS to hang on boot.
>>
>> Add union intstat_t for VMCB offset 68h and fix codes to only use
>> bit 0 as intr_shadow according to the new APM description.
>>
>> Reference:
>> [1] https://www.amd.com/system/files/TechDocs/24593.pdf
>>
>> Signed-off-by: Pu Wen 
> 
> Reviewed-by: Andrew Cooper 
> 
> Although thinking about it, renaming irq_stat to irq_state would
> probably be a good move.  I can fix this on commit - no need to send a v4.

Assuming you meant intstat -> intstate and int_stat -> int_state,
but in any event I don't think you've actually made the change
while committing.

Jan