Re: [Xen-devel] Design session report: Live-Updating Xen

2019-07-15 Thread Sarah Newman

On 7/15/19 8:48 PM, Juergen Gross wrote:

On 15.07.19 21:31, Sarah Newman wrote:

On 7/15/19 11:57 AM, Foerster, Leonard wrote:
...

A key cornerstone for Live-update is guest transparent live migration

...

-> for live migration: domid is a problem in this case
    -> randomize and pray does not work on smaller fleets
    -> this is not a problem for live-update
    -> BUT: as a community we shoudl make this restriction go away


Andrew Cooper pointed out to me that manually assigning domain IDs is supported in much of the code already. If guest transparent live migration 
gets merged, we'll look at passing in a domain ID to xl, which would be good enough for us. I don't know about the other toolstacks.


The main problem is the case where on the target host the domid of the
migrated domain is already in use by another domain. So you either need
a domid allocator spanning all hosts or the change of domid during
migration must be hidden from the guest for guest transparent migration.


Yes. There are some cluster management systems which use xl rather than xapi.
They could be extended to manage domain IDs if it's too difficult to allow
the domain ID to change during migration.

--Sarah

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

Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support

2019-07-15 Thread Juergen Gross

On 15.07.19 19:39, Andrew Cooper wrote:

On 15/07/2019 18:28, Andy Lutomirski wrote:

On Mon, Jul 15, 2019 at 9:34 AM Andi Kleen  wrote:

Juergen Gross  writes:


The long term plan has been to replace Xen PV guests by PVH. The first
victim of that plan are now 32-bit PV guests, as those are used only
rather seldom these days. Xen on x86 requires 64-bit support and with
Grub2 now supporting PVH officially since version 2.04 there is no
need to keep 32-bit PV guest support alive in the Linux kernel.
Additionally Meltdown mitigation is not available in the kernel running
as 32-bit PV guest, so dropping this mode makes sense from security
point of view, too.

Normally we have a deprecation period for feature removals like this.
You would make the kernel print a warning for some releases, and when
no user complains you can then remove. If a user complains you can't.


As I understand it, the kernel rules do allow changes like this even
if there's a complaint: this is a patch that removes what is
effectively hardware support.  If the maintenance cost exceeds the
value, then removal is fair game.  (Obviously we weight the value to
preserving compatibility quite highly, but in this case, Xen dropped
32-bit hardware support a long time ago.  If the Xen hypervisor says
that 32-bit PV guest support is deprecated, it's deprecated.)

That being said, a warning might not be a bad idea.  What's the
current status of this in upstream Xen?


So personally, I'd prefer to see support stay, but at the end of the day
it is Juergen's choice as the maintainer of the code.


Especially on the security front we are unsafe with 32-bit PV Linux.
And making it safe will make it so slow that the needed effort is not
spent very well.


Juergen

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

Re: [Xen-devel] [PATCH 1/2] x86/xen: remove 32-bit Xen PV guest support

2019-07-15 Thread Juergen Gross

On 15.07.19 17:44, Boris Ostrovsky wrote:



diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index 084de77a109e..d42737f31304 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -1,5 +1,5 @@
  # SPDX-License-Identifier: GPL-2.0
-OBJECT_FILES_NON_STANDARD_xen-asm_$(BITS).o := y
+OBJECT_FILES_NON_STANDARD_xen-asm_64.o := y
  
  ifdef CONFIG_FUNCTION_TRACER

  # Do not profile debug and lowlevel utilities
@@ -34,7 +34,7 @@ obj-$(CONFIG_XEN_PV)  += mmu_pv.o
  obj-$(CONFIG_XEN_PV)  += irq.o
  obj-$(CONFIG_XEN_PV)  += multicalls.o
  obj-$(CONFIG_XEN_PV)  += xen-asm.o
-obj-$(CONFIG_XEN_PV)   += xen-asm_$(BITS).o
+obj-$(CONFIG_XEN_PV)   += xen-asm_64.o


We should be able to merge xen-asm_64.S into xen-asm.S, shouldn't we?


Yes, probably a good idea to add that.


@@ -1312,15 +1290,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
  
  	/* keep using Xen gdt for now; no urgent need to change it */
  
-#ifdef CONFIG_X86_32

-   pv_info.kernel_rpl = 1;
-   if (xen_feature(XENFEAT_supervisor_mode_kernel))
-   pv_info.kernel_rpl = 0;
-#else
pv_info.kernel_rpl = 0;


Is kernel_rpl needed anymore?


Yes, this can be dropped, together with get_kernel_rpl().


Juergen

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

Re: [Xen-devel] Design session report: Live-Updating Xen

2019-07-15 Thread Juergen Gross

On 15.07.19 21:31, Sarah Newman wrote:

On 7/15/19 11:57 AM, Foerster, Leonard wrote:
...

A key cornerstone for Live-update is guest transparent live migration

...

-> for live migration: domid is a problem in this case
    -> randomize and pray does not work on smaller fleets
    -> this is not a problem for live-update
    -> BUT: as a community we shoudl make this restriction go away


Andrew Cooper pointed out to me that manually assigning domain IDs is 
supported in much of the code already. If guest transparent live 
migration gets merged, we'll look at passing in a domain ID to xl, which 
would be good enough for us. I don't know about the other toolstacks.


The main problem is the case where on the target host the domid of the
migrated domain is already in use by another domain. So you either need
a domid allocator spanning all hosts or the change of domid during
migration must be hidden from the guest for guest transparent migration.


Juergen

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

[Xen-devel] [qemu-mainline test] 139014: regressions - FAIL

2019-07-15 Thread osstest service owner
flight 139014 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139014/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 138977
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 138977

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 138977
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 138977
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 138977
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 138977
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 138977
 test-amd64-i386-xl-pvshim12 guest-start  fail   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-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-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-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-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-credit1  13 migrate-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-amd64-amd64-libvirt-vhd 12 migrate-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-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-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-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  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  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-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-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-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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 qemuu46cd24e7ed38191b5ab5c40a836d6c5b6b604f8a
baseline version:
 qemuu1316b1ddc8a05e418c8134243f8bff8cccbbccb1

Last test of basis   138977  2019-07-14 03:43:52 Z1 days
Testing same since   139014  2019-07-15 09:06:23 Z0 days1 attempts


People who touched revisions under test:
  Dr. David Alan Gilbert 
  Igor Mammedov 
  Michael S. Tsirkin 
  Pankaj Gupta 
  Peter Maydell 
  Stefan Hajnoczi 
  Wolfgang Bumiller 

jobs:
 build-amd64-xsm  pass
 

[Xen-devel] [xen-unstable test] 139010: tolerable FAIL

2019-07-15 Thread osstest service owner
flight 139010 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139010/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-examine 11 examine-serial/bootloaderfail  like 138915
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 138986
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 138986
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 138986
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 138986
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 138986
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 138986
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 138986
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 138986
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 138986
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 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-i386-libvirt  13 migrate-support-checkfail   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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail 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-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  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-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-libvirt 13 migrate-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-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 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-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   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-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass

version targeted for testing:
 xen  38eeb3864de40aa568c48f9f26271c141c62b50b
baseline version:
 xen  38eeb3864de40aa568c48f9f26271c141c62b50b

Last test of basis   139010  2019-07-15 07:01:24 Z0 days
Testing same since  (not found) 0 

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

2019-07-15 Thread osstest service owner
flight 139011 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139011/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf eebc135ffb210c6da7133145ba9e5423cafc13d4
baseline version:
 ovmf 70565e64227dfa254d8a0703dd60dc74bd8b5e6e

Last test of basis   138998  2019-07-14 17:21:25 Z1 days
Testing same since   139011  2019-07-15 07:47:26 Z0 days1 attempts


People who touched revisions under test:
  Cole Robinson 

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
   70565e6422..eebc135ffb  eebc135ffb210c6da7133145ba9e5423cafc13d4 -> 
xen-tested-master

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

[Xen-devel] [freebsd-master test] 139016: all pass - PUSHED

2019-07-15 Thread osstest service owner
flight 139016 freebsd-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139016/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 freebsd  163feb2e45cd088e65da1ff395dc3293065a4603
baseline version:
 freebsd  5c4a9b0e32c1f9c47d5b687d6036bb03c3cc071c

Last test of basis   138886  2019-07-10 09:19:38 Z5 days
Failing since138921  2019-07-12 09:19:32 Z3 days2 attempts
Testing same since   139016  2019-07-15 09:19:57 Z0 days1 attempts


People who touched revisions under test:
  ae 
  alc 
  avg 
  chuck 
  cy 
  dim 
  dougm 
  ian 
  imp 
  jhibbits 
  kib 
  luporl 
  markj 
  np 
  philip 
  phk 
  rrs 
  scottl 
  seanc 
  sjg 
  tijl 
  tuexen 
  vmaffione 

jobs:
 build-amd64-freebsd-againpass
 build-amd64-freebsd  pass
 build-amd64-xen-freebsd  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/freebsd.git
   5c4a9b0e32c..163feb2e45c  163feb2e45cd088e65da1ff395dc3293065a4603 -> 
tested/master

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

[Xen-devel] [linux-linus test] 139003: regressions - FAIL

2019-07-15 Thread osstest service owner
flight 139003 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139003/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. 
vs. 133580
 test-amd64-amd64-i386-pvgrub  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-libvirt  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
133580
 test-amd64-amd64-xl-xsm   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-qemuu-nested-intel  7 xen-boot  fail REGR. vs. 133580
 test-amd64-amd64-xl-pvhv2-intel  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 133580
 test-amd64-amd64-examine  8 reboot   fail REGR. vs. 133580
 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-amd64-xl-qemut-win7-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-amd64-xl-pvshim7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemut-win10-i386  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-multivcpu  7 xen-bootfail REGR. vs. 133580
 test-amd64-amd64-amd64-pvgrub  7 xen-bootfail REGR. vs. 133580
 test-amd64-amd64-xl-qemut-debianhvm-amd64  7 xen-bootfail REGR. vs. 133580
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-amd64-pygrub   7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-credit2   7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-libvirt-xsm  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. 
vs. 133580
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-amd64-xl-qemuu-ovmf-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-amd64-xl-pvhv2-amd  7 xen-bootfail REGR. vs. 133580
 test-amd64-amd64-qemuu-nested-amd  7 xen-bootfail REGR. vs. 133580
 test-amd64-amd64-xl-shadow7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-win10-i386  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemut-ws16-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-amd64-libvirt-vhd  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  7 xen-bootfail REGR. vs. 133580
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-amd64-xl-credit1   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host   fail REGR. vs. 133580
 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host   fail REGR. vs. 133580
 

[Xen-devel] [linux-4.19 test] 139004: regressions - FAIL

2019-07-15 Thread osstest service owner
flight 139004 linux-4.19 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139004/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs. 129313
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail REGR. vs. 
129313
 build-armhf-pvops 6 kernel-build fail REGR. vs. 129313

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt  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-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-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop 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-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail 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-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux3bd837bfe431839a378e9d421af05b2e22a6d329
baseline version:
 linux84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d

Last test of basis   129313  2018-11-02 05:39:08 Z  255 days
Failing since129412  2018-11-04 14:10:15 Z  253 days  159 attempts
Testing same since   139004  2019-07-14 23:35:54 Z0 days1 attempts


2271 people touched revisions under test,
not listing them all

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

Re: [Xen-devel] Design session report: Live-Updating Xen

2019-07-15 Thread Sarah Newman

On 7/15/19 11:57 AM, Foerster, Leonard wrote:
...

A key cornerstone for Live-update is guest transparent live migration

...

-> for live migration: domid is a problem in this case
-> randomize and pray does not work on smaller fleets
-> this is not a problem for live-update
-> BUT: as a community we shoudl make this restriction go away


Andrew Cooper pointed out to me that manually assigning domain IDs is supported in much of the code already. If guest transparent live migration gets 
merged, we'll look at passing in a domain ID to xl, which would be good enough for us. I don't know about the other toolstacks.


--Sarah

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

Re: [Xen-devel] [PATCH 05/17] xen/arm64: head: Introduce print_reg

2019-07-15 Thread Volodymyr Babchuk
Hi Julien,

Julien Grall writes:

> At the moment, the user should save x30/lr if it cares about it.
>
> Follow-up patches will introduce more use of putn in place where lr
> should be preserved.
>
> Furthermore, any user of putn should also move the value to register x0
> if it was stored in a different register.
>
> For convenience, a new macro is introduced to print a given register.
> The macro will take care for us to move the value to x0 and also
> preserve lr.
>
> Lastly the new macro is used to replace all the callsite of putn. This
> will simplify rework/review later on.
>
> Note that CurrentEL is now stored in x5 instead of x4 because the latter
> will be clobbered by the macro print_reg.
>
> Signed-off-by: Julien Grall 
> ---
>  xen/arch/arm/arm64/head.S | 29 ++---
>  1 file changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 84e26582c4..9142b4a774 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -90,8 +90,25 @@
>  blputs; \
>  mov   lr, x3  ; \
>  RODATA_STR(98, _s)
> +
> +/*
> + * Macro to print the value of register \xb
> + *
> + * Clobbers x0 - x4
> + */

Despite its name, this macro can't print x4. I would recommend adding at
least comment about this. Static assertion would be even better, but
looks like we don't have them for asm code.

> +.macro print_reg xb
> +mov   x4, lr
> +mov   x0, \xb
> +blputn
> +mov   lr, x4
> +.endm
> +
>  #else /* CONFIG_EARLY_PRINTK */
>  #define PRINT(s)
> +
> +.macro print_reg xb
> +.endm
> +
>  #endif /* !CONFIG_EARLY_PRINTK */
>
>  /* Load the physical address of a symbol into xb */
> @@ -304,22 +321,20 @@ GLOBAL(init_secondary)
>  #ifdef CONFIG_EARLY_PRINTK
>  ldr   x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */
>  PRINT("- CPU ")
> -mov   x0, x24
> -blputn
> +print_reg x24
>  PRINT(" booting -\r\n")
>  #endif
>
>  common_start:
>
>  PRINT("- Current EL ")
> -mrs   x4, CurrentEL
> -mov   x0, x4
> -blputn
> +mrs   x5, CurrentEL
> +print_reg x5
>  PRINT(" -\r\n")
>
>  /* Are we in EL2 */
> -cmp   x4, #PSR_MODE_EL2t
> -ccmp  x4, #PSR_MODE_EL2h, #0x4, ne
> +cmp   x5, #PSR_MODE_EL2t
> +ccmp  x5, #PSR_MODE_EL2h, #0x4, ne
>  b.eq  el2 /* Yes */
>
>  /* OK, we're boned. */


--
Best regards, Volodymyr Babchuk
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] Design session report: Further defences for speculative sidechannels

2019-07-15 Thread Andrew Cooper
Some numpty (me) forgot to organise a scribe for the session, so these
notes are from memory.  As a result, all aspects are up for argumen^W
debate.

We started by discussing Core-aware scheduling, and covering the point
that while in principle it makes an attackers life easier, it doesn't
make a difference in practice.  A determined attacker won't be put off
by the extra stats required to determine co-residency, and there are
plenty of research papers published with various techniques in this area.

We also discussed synchronised scheduling.  Part of this was an
explanation of why it is believed to be safe alternative to disabling
HT.  Part was a discussion of the performance aspects and why the perf
hit is substantially worse when virtualised than on native, due to a
lack of virtualised IPI support on affected hardware.

Some discussion went on MSR_ARCH_CAPS virtualisation support, and
support for eIBRS.  This work is already on a TODO list, and no concerns
were raised.

The area which took up a large part of the discussion was the current
state of "l1tf_barrier".  In current staging, we are compiling code
which takes all of the performance hit, and gains none of the safety.

This is because we fought the optimiser, and the optimiser won.

The use of inline assembly is causing the optimiser to out-of-line the
protected predicates, which causes the LFENCE to be emitted in the
instruction stream *before* the Jcc trying to be protected, which
renders the protections useless.

Annotating all predicates with always_inline is believed to resolve the
issue, but this is almost impossible to spot during review, as all it
takes is one intermediate non-always_inline predicate to break the
security all over again.

An alternative to the current approach was raised, which involves using
a compiler extension.  Linux has used compiler extensions for several
releases now, and forms the basis of several KSPP features/mitigations.

It is expected that we would be able to express the required protections
 using a compiler plugin rather than with inline assembly, which
provides a much cleaner argument concerning correctness, and makes it
less likely that errors will occur due to fighting with the optimiser.

~Andrew

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

Re: [Xen-devel] [PATCH] xen/arm: merge make_timer_node and make_timer_domU_node

2019-07-15 Thread Julien Grall

Hi Viktor,

On 20/06/2019 11:38, Viktor Mitin wrote:

Functions make_timer_node and make_timer_domU_node are quite similar.
The only difference between Dom0 and DomU timer DT node
is the timer interrupts used.  All the rest code should be the same.


This is a bit confusing to read. First you say there are only "one difference" 
but then the second part leads to think there are more difference.


The commit message should describe what are the differences and which version 
you are keeping.



So it is better to merge them to avoid discrepancy.

Tested dom0 boot with rcar h3 sk board.
How about domU support? Also, do you have the clock-frequency property in your 
DT timer node?




Suggested-by: Julien Grall 
Signed-off-by: Viktor Mitin 
---
  xen/arch/arm/domain_build.c | 66 -
  1 file changed, 21 insertions(+), 45 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 7fb828cae2..610dd3e8e7 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -976,6 +976,8 @@ static int __init make_timer_node(const struct domain *d, 
void *fdt)
  gic_interrupt_t intrs[3];
  u32 clock_frequency;
  bool clock_valid;
+bool d0 = is_hardware_domain(d);


Please avoid to use the term "d0"/"dom0" whenever it is possible. While in 
practice the hardware domain is always dom0 on Arm, I want to avoid the mixing.



+uint32_t ip_val;
  
  dt_dprintk("Create timer node\n");


I am not sure where to comment, but the compatible will be different for DomU 
now. I would actually prefer if we keep the domU version for the compatible as 
it is simpler.


  
@@ -1004,22 +1006,36 @@ static int __init make_timer_node(const struct domain *d, void *fdt)

  /* The timer IRQ is emulated by Xen. It always exposes an active-low
   * level-sensitive interrupt */
  
-irq = timer_get_irq(TIMER_PHYS_SECURE_PPI);

+irq = d0
+? timer_get_irq(TIMER_PHYS_SECURE_PPI)
+: GUEST_TIMER_PHYS_S_PPI;


Rather than using ternary everywhere, how about introducing an array where the 
value will be stored.


So the code would look like:

if ( is_hardware_domain(...) )
{
  timer_irq[...] = ...;
  timer_irq[...] = ...;
}
else
{
}

[]

set_interrupt(timer_irq[...]);

Have a look at time.c as we define handy value (enum timer_ppi and 
MAX_TIMER_PPI).


  dt_dprintk("  Secure interrupt %u\n", irq);
  set_interrupt(intrs[0], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
  
-irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI);

+irq = d0
+? timer_get_irq(TIMER_PHYS_NONSECURE_PPI)
+: GUEST_TIMER_PHYS_NS_PPI;
  dt_dprintk("  Non secure interrupt %u\n", irq);
  set_interrupt(intrs[1], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
  
-irq = timer_get_irq(TIMER_VIRT_PPI);

+irq = d0
+? timer_get_irq(TIMER_VIRT_PPI)
+: GUEST_TIMER_VIRT_PPI;
  dt_dprintk("  Virt interrupt %u\n", irq);
  set_interrupt(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
  
-res = fdt_property_interrupts(fdt, intrs, 3);

+res = fdt_property(fdt, "interrupts", intrs, sizeof (intrs[0]) * 3);
  if ( res )
  return res;
  
+ip_val = d0

+   ? dt_interrupt_controller->phandle
+   : GUEST_PHANDLE_GIC;
+
+res = fdt_property_cell(fdt, "interrupt-parent", ip_val);


I would actually prefer if we extend fdt_property_interrupts to deal with other 
domain than the hwdom. This would avoid the function.



+if (res)


NIT: Coding style

if ( ... )

Cheers,

--
Julien Grall

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

Re: [Xen-devel] Design Session report: Toolstacks

2019-07-15 Thread Amit Shah
On Mon, 2019-07-15 at 14:59 +0100, George Dunlap wrote:
> Looking through the notes, it seems like the first part of this
> discussion, re hypervisor upgrade / downgrade & libraries, was partly
> covered in the distro session, in which Debian's Xen version co-
> install
> was discussed and found useful even if we had a hypervisor , Ian
> Jackson
> agreed to post Debian's co-install patches.

Yea.

That's also useful for the live-update project, where we want to (in
the future) use the userspace tools and libraries depending on which
Xen version is in use at a time.

It doesn't need to be much smarter than symlinks for binaries at the
first go, and the symlinks updated each time a live-update operation
succeeds.

Beyond that, for libraries, we'll have to do much smarter things
eventually.



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

Re: [Xen-devel] Design session report: Xen on Distros

2019-07-15 Thread Amit Shah
On Mon, 2019-07-15 at 14:52 +, Jan Beulich wrote:
> On 15.07.2019 16:42, George Dunlap wrote:
> > On 7/15/19 3:23 PM, Jan Beulich wrote:
> > > On 15.07.2019 16:11, George Dunlap wrote:
> > > > There was a long discussion about security patches, with the
> > > > general
> > > > proposal being that we should cut a point release for every
> > > > security issue.
> > > 
> > > Interesting. Looks like in politics that until a decision fits
> > > people
> > > they keep re-raising the point. Iirc on a prior meeting
> > > (Budapest?)
> > > we had settled on continuing with the current scheme. Were there
> > > any
> > > new arguments towards this alternative model?
> > 
> > Well I don't know if there were any new arguments because I don't
> > immediately remember the old discussion.  Do we have a summary of
> > the
> > discussion in Budapest, with its conclusions, anywhere?
> 
> I don't recall if suitable notes were taken back then; as indicated
> I'm not even sure which meeting it was at.
> 
> > The basic idea was that:
> > 
> > 1. Most distros / packagers are going to want to do an immediate
> > release
> > anyway.
> > 
> > 2. Distros generally seemed to be rebasing on top of staging as
> > soon as
> > the XSA went out anyway (and ISTR this being the recommeneded
> > course of
> > action)
> > 
> > So for all intents and purposes, we have something which is, in
> > fact, a
> > release; all it's missing is a signed tag and a tarball.
> > 
> > Obviously there are testing implications that would need to be
> > sorted
> > out before this could become a reality.
> > 
> > In any case, the ball is in the court of "VOLUNTEER" to write up a
> > concrete proposal which could be discussed.  You'll be able to
> > raise all
> > your concerns at that point if you want (although having a sketch
> > would
> > of course be helpful for whoever is writing such a proposal).
> 
> Sure - I realized soon after having sent the initial reply that
> perhaps
> this was the wrong context in the first place to raise my question.

In any case, I'd like to know why it doesn't make sense for Xen to have
a point release frequently, and not have a point release after an XSA
above some severity level (pick one - high/critical/important).  As
George mentioned, distros have to do it anyway, and the upstream
project not doing it only makes it more difficult for all distros
involved.

Not sure of the politics involved though, and what can of worms this
opens.


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

Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support

2019-07-15 Thread Juergen Gross

On 15.07.19 19:28, Andy Lutomirski wrote:

On Mon, Jul 15, 2019 at 9:34 AM Andi Kleen  wrote:


Juergen Gross  writes:


The long term plan has been to replace Xen PV guests by PVH. The first
victim of that plan are now 32-bit PV guests, as those are used only
rather seldom these days. Xen on x86 requires 64-bit support and with
Grub2 now supporting PVH officially since version 2.04 there is no
need to keep 32-bit PV guest support alive in the Linux kernel.
Additionally Meltdown mitigation is not available in the kernel running
as 32-bit PV guest, so dropping this mode makes sense from security
point of view, too.


Normally we have a deprecation period for feature removals like this.
You would make the kernel print a warning for some releases, and when
no user complains you can then remove. If a user complains you can't.



As I understand it, the kernel rules do allow changes like this even
if there's a complaint: this is a patch that removes what is
effectively hardware support.  If the maintenance cost exceeds the
value, then removal is fair game.  (Obviously we weight the value to
preserving compatibility quite highly, but in this case, Xen dropped
32-bit hardware support a long time ago.  If the Xen hypervisor says
that 32-bit PV guest support is deprecated, it's deprecated.)

That being said, a warning might not be a bad idea.  What's the
current status of this in upstream Xen?


Xen still supports that.

We have asked downstream for their opinion about dropping 32-bit PV
guest support in the kernel about 1 year ago and the common answer was:
no problem, but for users still wanting 32 bit guests we should wait
until PVH support is available in all related products. Grub2 was the
last one missing and as grub2 has released a version with PVH support
I posted this small series now.


Juergen

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

Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support

2019-07-15 Thread Andrew Cooper
On 15/07/2019 18:28, Andy Lutomirski wrote:
> On Mon, Jul 15, 2019 at 9:34 AM Andi Kleen  wrote:
>> Juergen Gross  writes:
>>
>>> The long term plan has been to replace Xen PV guests by PVH. The first
>>> victim of that plan are now 32-bit PV guests, as those are used only
>>> rather seldom these days. Xen on x86 requires 64-bit support and with
>>> Grub2 now supporting PVH officially since version 2.04 there is no
>>> need to keep 32-bit PV guest support alive in the Linux kernel.
>>> Additionally Meltdown mitigation is not available in the kernel running
>>> as 32-bit PV guest, so dropping this mode makes sense from security
>>> point of view, too.
>> Normally we have a deprecation period for feature removals like this.
>> You would make the kernel print a warning for some releases, and when
>> no user complains you can then remove. If a user complains you can't.
>>
> As I understand it, the kernel rules do allow changes like this even
> if there's a complaint: this is a patch that removes what is
> effectively hardware support.  If the maintenance cost exceeds the
> value, then removal is fair game.  (Obviously we weight the value to
> preserving compatibility quite highly, but in this case, Xen dropped
> 32-bit hardware support a long time ago.  If the Xen hypervisor says
> that 32-bit PV guest support is deprecated, it's deprecated.)
>
> That being said, a warning might not be a bad idea.  What's the
> current status of this in upstream Xen?

So personally, I'd prefer to see support stay, but at the end of the day
it is Juergen's choice as the maintainer of the code.

Xen itself has been exclusively 64-bit since Xen 4.3 (released in 2013).

Over time, various features like SMEP/SMAP have been making 32bit PV
guests progressively slower, because ring 1 is supervisor rather than
user.  Things have got even worse with IBRS, to the point at which 32bit
PV guests are starting to run like treacle.

There are no current plans to remove support for 32bit PV guests from
Xen, but it is very much in the category of "you shouldn't be using this
mode any more".

~Andrew

P.S. I don't see 64bit PV guest support going anywhere, because there
are still a number of open performance questions due to the inherent
differences between syscall and vmexit, and the difference EPT/NPT
tables make on cross-domain mappings.

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

Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support

2019-07-15 Thread Andy Lutomirski
On Mon, Jul 15, 2019 at 9:34 AM Andi Kleen  wrote:
>
> Juergen Gross  writes:
>
> > The long term plan has been to replace Xen PV guests by PVH. The first
> > victim of that plan are now 32-bit PV guests, as those are used only
> > rather seldom these days. Xen on x86 requires 64-bit support and with
> > Grub2 now supporting PVH officially since version 2.04 there is no
> > need to keep 32-bit PV guest support alive in the Linux kernel.
> > Additionally Meltdown mitigation is not available in the kernel running
> > as 32-bit PV guest, so dropping this mode makes sense from security
> > point of view, too.
>
> Normally we have a deprecation period for feature removals like this.
> You would make the kernel print a warning for some releases, and when
> no user complains you can then remove. If a user complains you can't.
>

As I understand it, the kernel rules do allow changes like this even
if there's a complaint: this is a patch that removes what is
effectively hardware support.  If the maintenance cost exceeds the
value, then removal is fair game.  (Obviously we weight the value to
preserving compatibility quite highly, but in this case, Xen dropped
32-bit hardware support a long time ago.  If the Xen hypervisor says
that 32-bit PV guest support is deprecated, it's deprecated.)

That being said, a warning might not be a bad idea.  What's the
current status of this in upstream Xen?

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

Re: [Xen-devel] [PATCH v2] xen/doc: update ARM warning about testing gcov on arm

2019-07-15 Thread Andrew Cooper
On 15/07/2019 18:26, Julien Grall wrote:
> Hi,
>
> On 10/07/2019 05:57, Viktor Mitin wrote:
>> Update ARM code coverage warning about testing gcov on arm
>>
>> Signed-off-by: Viktor Mitin 
>
> Acked-by: Julien Grall 

Docs parts Acked-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH v2] xen/doc: update ARM warning about testing gcov on arm

2019-07-15 Thread Julien Grall

Hi,

On 10/07/2019 05:57, Viktor Mitin wrote:

Update ARM code coverage warning about testing gcov on arm

Signed-off-by: Viktor Mitin 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Julien Grall

Hi,

On 15/07/2019 10:17, Paul Durrant wrote:

The _PGC_allocated flag is set on a page when it is assigned to a domain
along with an initial reference count of at least 1. To clear this
'allocation' reference it is necessary to test-and-clear _PGC_allocated and
then only drop the reference if the test-and-clear succeeds. This is open-
coded in many places. It is also unsafe to test-and-clear _PGC_allocated
unless the caller holds an additional reference.

This patch adds a helper function, put_page_alloc_ref(), to replace all the
open-coded test-and-clear/put_page occurrences and incorporates in that a
BUG_ON() an additional page reference not being held.

Signed-off-by: Paul Durrant 


For the Arm bits:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

[Xen-devel] [PATCH] x86/suspend: Don't save/restore %cr8

2019-07-15 Thread Andrew Cooper
%cr8 is an alias of APIC_TASKPRI, which is handled by
lapic_{suspend,resume}() with the rest of the Local APIC state.  Saving
and restoring the TPR state in isolation is not a clever idea.

Drop it all.

While editing wakeup_prot.S, trim its include list to just the headers
which are used, which is precicely none of them.

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

This is a Xen mirror to my Linux patch of the same effect:
https://lore.kernel.org/lkml/20190715151641.29210-1-andrew.coop...@citrix.com/T/#u

With a bit of care, I'm pretty sure the whole of wakeup_prot.S can
disappear, but -ETIME right now.

I've confirmed that after resume TPR retains its value of 0x10.  However, all
attempts to debug the internals of lapic_suspend/resume have eluded me,
including manually poking the UART.  Again, -ETIME to investigate further.
---
 xen/arch/x86/acpi/wakeup_prot.S | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/xen/arch/x86/acpi/wakeup_prot.S b/xen/arch/x86/acpi/wakeup_prot.S
index 361751d290..4a92627436 100644
--- a/xen/arch/x86/acpi/wakeup_prot.S
+++ b/xen/arch/x86/acpi/wakeup_prot.S
@@ -1,13 +1,5 @@
 .file __FILE__
 .text
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
 .code64
 
 #define GREG(x) %r##x
@@ -40,9 +32,6 @@ ENTRY(do_suspend_lowlevel)
 pushfq;
 popqSAVED_GREG(flags)
 
-mov %cr8, GREG(ax)
-mov GREG(ax), REF(saved_cr8)
-
 mov %ss, REF(saved_ss)
 
 sgdtREF(saved_gdt)
@@ -90,9 +79,6 @@ ENTRY(__ret_point)
 pushq   %rax
 lretq
 1:
-mov REF(saved_cr8), %rax
-mov %rax, %cr8
-
 pushq   SAVED_GREG(flags)
 popfq
 
@@ -149,4 +135,3 @@ saved_ldt:  .quad   0,0
 
 saved_cr0:  .quad   0
 saved_cr3:  .quad   0
-saved_cr8:  .quad   0
-- 
2.11.0


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

Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support

2019-07-15 Thread Andi Kleen
Juergen Gross  writes:

> The long term plan has been to replace Xen PV guests by PVH. The first
> victim of that plan are now 32-bit PV guests, as those are used only
> rather seldom these days. Xen on x86 requires 64-bit support and with
> Grub2 now supporting PVH officially since version 2.04 there is no
> need to keep 32-bit PV guest support alive in the Linux kernel.
> Additionally Meltdown mitigation is not available in the kernel running
> as 32-bit PV guest, so dropping this mode makes sense from security
> point of view, too.

Normally we have a deprecation period for feature removals like this.
You would make the kernel print a warning for some releases, and when
no user complains you can then remove. If a user complains you can't.

-Andi

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

Re: [Xen-devel] [PATCH v1 02/33] drm/xen: drop use of drmP.h

2019-07-15 Thread Sam Ravnborg
On Mon, Jul 01, 2019 at 08:05:24AM +0200, Sam Ravnborg wrote:
> Hi Oleksandr
> 
> > > --- a/drivers/gpu/drm/xen/xen_drm_front.h
> > > +++ b/drivers/gpu/drm/xen/xen_drm_front.h
> > > @@ -11,13 +11,19 @@
> > >   #ifndef __XEN_DRM_FRONT_H_
> > >   #define __XEN_DRM_FRONT_H_
> > > -#include 
> > > -#include 
> > > -
> > >   #include 
> > > +#include 
> > > +#include 
> > > +#include 
> > no need to include twice
> > with that fixed:
> > Acked-by: Oleksandr Andrushchenko 

Applied, thanks.

Sam

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

Re: [Xen-devel] [PATCH v1 2/5] xen: sched: null: don't assign down vcpus to pcpus

2019-07-15 Thread George Dunlap
On 8/25/18 1:21 AM, Dario Faggioli wrote:
> If a pCPU has been/is being offlined, we don't want it to be neither
> assigned to any pCPU, nor in the wait list.

I take it the first `pCPU` should be `vCPU`?

Also, English grammar agreement is funny: `neither` needs to agree with
`nor`, but the two do *not* agree with the original verb.  That is, the
sentence should say:

"...we want it to be neither assigned to pCPU, nor in the wait list".

Both here and in the comment.

The other thing is, from a "developmental purity" point of view, I think
this series technically has a regression in the middle: cpu offline /
online stops working between patch 2 and patch 4.  But I'm inclined in
this case not to worry too much. :-)

Everything else looks good.

 -George

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

Re: [Xen-devel] [PATCH v2 4/4] iommu / pci: re-implement XEN_DOMCTL_get_device_group...

2019-07-15 Thread Paul Durrant
> -Original Message-
> From: Roger Pau Monne 
> Sent: 15 July 2019 16:46
> To: Paul Durrant 
> Cc: xen-devel@lists.xenproject.org; Jan Beulich 
> Subject: Re: [Xen-devel] [PATCH v2 4/4] iommu / pci: re-implement 
> XEN_DOMCTL_get_device_group...
> 
> On Mon, Jul 15, 2019 at 01:37:10PM +0100, Paul Durrant wrote:
> > ... using the new iommu_group infrastructure.
> >
> > Because 'sibling' devices are now members of the same iommu_group,
> > implement the domctl by looking up the iommu_group of the pdev with the
> > matching SBDF and then finding all the assigned pdevs that are in the
> > group.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> > Cc: Jan Beulich 
> >
> > v2:
> >  - Re-implement in the absence of a per-group devs list.
> >  - Make use of pci_sbdf_t.
> > ---
> >  xen/drivers/passthrough/groups.c | 44 ++
> >  xen/drivers/passthrough/pci.c| 51 
> > ++--
> >  xen/include/xen/iommu.h  |  2 ++
> >  3 files changed, 48 insertions(+), 49 deletions(-)
> >
> > diff --git a/xen/drivers/passthrough/groups.c 
> > b/xen/drivers/passthrough/groups.c
> > index 1a2f461c87..6d8064e4f4 100644
> > --- a/xen/drivers/passthrough/groups.c
> > +++ b/xen/drivers/passthrough/groups.c
> > @@ -12,8 +12,12 @@
> >   * GNU General Public License for more details.
> >   */
> >
> > +#include 
> >  #include 
> > +#include 
> >  #include 
> > +#include 
> > +#include 
> >
> >  struct iommu_group {
> >  unsigned int id;
> > @@ -81,6 +85,46 @@ int iommu_group_assign(struct pci_dev *pdev, void *arg)
> >  return 0;
> >  }
> >
> > +int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf,
> > +   XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs)
> 
> max_sdevs should be unsigned AFAICT, but it seems to be completely
> unused. I think you want to do...
> 
> > +{
> > +struct iommu_group *grp = NULL;
> > +struct pci_dev *pdev;
> > +unsigned int i = 0;
> > +
> > +pcidevs_lock();
> > +
> > +for_each_pdev ( d, pdev )
> > +{
> > +if ( pdev->sbdf.sbdf == sbdf.sbdf )
> > +{
> > +grp = pdev->grp;
> > +break;
> > +}
> > +}
> > +
> > +if ( !grp )
> > +goto out;
> > +
> > +for_each_pdev ( d, pdev )
> > +{
> 
> if ( i == max_sdevs )
> {
> pcidevs_unlock();
> return -ENOSPC;
> }

Oh, I'm sure I used to have that... I don't know how it got dropped. It 
certainly needs to be there.

  Paul

> 
> Thanks, Roger.

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

Re: [Xen-devel] [PATCH v1 1/5] xen: sched: null: refactor core around vcpu_deassign()

2019-07-15 Thread George Dunlap
On 8/25/18 1:21 AM, Dario Faggioli wrote:
> vcpu_deassign() has only one caller: _vcpu_remove().
> Let's consolidate the two functions into one.
> 
> No functional change intended.
> 
> Signed-off-by: Dario Faggioli 

Acked-by: George Dunlap 

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

Re: [Xen-devel] [PATCH v2 4/4] iommu / pci: re-implement XEN_DOMCTL_get_device_group...

2019-07-15 Thread Roger Pau Monné
On Mon, Jul 15, 2019 at 01:37:10PM +0100, Paul Durrant wrote:
> ... using the new iommu_group infrastructure.
> 
> Because 'sibling' devices are now members of the same iommu_group,
> implement the domctl by looking up the iommu_group of the pdev with the
> matching SBDF and then finding all the assigned pdevs that are in the
> group.
> 
> Signed-off-by: Paul Durrant 
> ---
> Cc: Jan Beulich 
> 
> v2:
>  - Re-implement in the absence of a per-group devs list.
>  - Make use of pci_sbdf_t.
> ---
>  xen/drivers/passthrough/groups.c | 44 ++
>  xen/drivers/passthrough/pci.c| 51 
> ++--
>  xen/include/xen/iommu.h  |  2 ++
>  3 files changed, 48 insertions(+), 49 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/groups.c 
> b/xen/drivers/passthrough/groups.c
> index 1a2f461c87..6d8064e4f4 100644
> --- a/xen/drivers/passthrough/groups.c
> +++ b/xen/drivers/passthrough/groups.c
> @@ -12,8 +12,12 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include 
>  #include 
> +#include 
>  #include 
> +#include 
> +#include 
>  
>  struct iommu_group {
>  unsigned int id;
> @@ -81,6 +85,46 @@ int iommu_group_assign(struct pci_dev *pdev, void *arg)
>  return 0;
>  }
>  
> +int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf,
> +   XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs)

max_sdevs should be unsigned AFAICT, but it seems to be completely
unused. I think you want to do...

> +{
> +struct iommu_group *grp = NULL;
> +struct pci_dev *pdev;
> +unsigned int i = 0;
> +
> +pcidevs_lock();
> +
> +for_each_pdev ( d, pdev )
> +{
> +if ( pdev->sbdf.sbdf == sbdf.sbdf )
> +{
> +grp = pdev->grp;
> +break;
> +}
> +}
> +
> +if ( !grp )
> +goto out;
> +
> +for_each_pdev ( d, pdev )
> +{

if ( i == max_sdevs )
{
pcidevs_unlock();
return -ENOSPC;
}

Thanks, Roger.

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

Re: [Xen-devel] [PATCH 1/2] x86/xen: remove 32-bit Xen PV guest support

2019-07-15 Thread Boris Ostrovsky

> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
> index 084de77a109e..d42737f31304 100644
> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -OBJECT_FILES_NON_STANDARD_xen-asm_$(BITS).o := y
> +OBJECT_FILES_NON_STANDARD_xen-asm_64.o := y
>  
>  ifdef CONFIG_FUNCTION_TRACER
>  # Do not profile debug and lowlevel utilities
> @@ -34,7 +34,7 @@ obj-$(CONFIG_XEN_PV)+= mmu_pv.o
>  obj-$(CONFIG_XEN_PV) += irq.o
>  obj-$(CONFIG_XEN_PV) += multicalls.o
>  obj-$(CONFIG_XEN_PV) += xen-asm.o
> -obj-$(CONFIG_XEN_PV) += xen-asm_$(BITS).o
> +obj-$(CONFIG_XEN_PV) += xen-asm_64.o

We should be able to merge xen-asm_64.S into xen-asm.S, shouldn't we?


> @@ -1312,15 +1290,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
>  
>   /* keep using Xen gdt for now; no urgent need to change it */
>  
> -#ifdef CONFIG_X86_32
> - pv_info.kernel_rpl = 1;
> - if (xen_feature(XENFEAT_supervisor_mode_kernel))
> - pv_info.kernel_rpl = 0;
> -#else
>   pv_info.kernel_rpl = 0;

Is kernel_rpl needed anymore?


-boris



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

Re: [Xen-devel] [PATCH v2 3/4] iommu: introduce iommu_groups

2019-07-15 Thread Paul Durrant
> -Original Message-
> From: Roger Pau Monne 
> Sent: 15 July 2019 16:35
> To: Paul Durrant 
> Cc: xen-devel@lists.xenproject.org; Stefano Stabellini 
> ; Wei Liu ;
> Konrad Rzeszutek Wilk ; George Dunlap 
> ; Andrew
> Cooper ; Ian Jackson ; Tim 
> (Xen.org) ;
> Julien Grall ; Jan Beulich 
> Subject: Re: [Xen-devel] [PATCH v2 3/4] iommu: introduce iommu_groups
> 
> On Mon, Jul 15, 2019 at 01:37:09PM +0100, Paul Durrant wrote:
> > Some devices may share a single PCIe initiator id, e.g. if they are actually
> > legacy PCI devices behind a bridge, and hence DMA from such devices will
> > be subject to the same address translation in the IOMMU. Hence these devices
> > should be treated as a unit for the purposes of assignment. There are also
> > other reasons why multiple devices should be treated as a unit, e.g. those
> > subject to a shared RMRR or those downstream of a bridge that does not
> > support ACS.
> >
> > This patch introduces a new struct iommu_group to act as a container for
> > devices that should be treated as a unit, and builds a list of them as
> > PCI devices are scanned. The iommu_ops already implement a method,
> > get_device_group_id(), that is seemingly intended to return the initiator
> > id for a given SBDF so use this as the mechanism for group assignment in
> > the first instance. Assignment based on shared RMRR or lack of ACS will be
> > dealt with in subsequent patches, as will modifications to the device
> > assignment code.
> >
> > Signed-off-by: Paul Durrant 
> 
> LGTM, just two comments below.
> 
> > ---
> > Cc: Jan Beulich 
> > Cc: Andrew Cooper 
> > Cc: George Dunlap 
> > Cc: Ian Jackson 
> > Cc: Julien Grall 
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Stefano Stabellini 
> > Cc: Tim Deegan 
> > Cc: Wei Liu 
> >
> > v2:
> >  - Move code into new drivers/passthrough/groups.c
> >  - Drop the group index.
> >  - Handle failure to get group id.
> >  - Drop the group devs list.
> > ---
> >  xen/drivers/passthrough/Makefile|  1 +
> >  xen/drivers/passthrough/groups.c| 91 
> > +
> >  xen/drivers/passthrough/x86/iommu.c |  8 +++-
> >  xen/include/xen/iommu.h |  7 +++
> >  xen/include/xen/pci.h   |  2 +
> >  5 files changed, 108 insertions(+), 1 deletion(-)
> >  create mode 100644 xen/drivers/passthrough/groups.c
> >
> > diff --git a/xen/drivers/passthrough/Makefile 
> > b/xen/drivers/passthrough/Makefile
> > index d50ab188c8..8a77110179 100644
> > --- a/xen/drivers/passthrough/Makefile
> > +++ b/xen/drivers/passthrough/Makefile
> > @@ -4,6 +4,7 @@ subdir-$(CONFIG_X86) += x86
> >  subdir-$(CONFIG_ARM) += arm
> >
> >  obj-y += iommu.o
> > +obj-$(CONFIG_HAS_PCI) += groups.o
> >  obj-$(CONFIG_HAS_PCI) += pci.o
> >  obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
> >
> > diff --git a/xen/drivers/passthrough/groups.c 
> > b/xen/drivers/passthrough/groups.c
> > new file mode 100644
> > index 00..1a2f461c87
> > --- /dev/null
> > +++ b/xen/drivers/passthrough/groups.c
> > @@ -0,0 +1,91 @@
> > +/*
> > + * Copyright (c) 2019 Citrix Systems Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that 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.
> > + */
> > +
> > +#include 
> > +#include 
> > +
> > +struct iommu_group {
> > +unsigned int id;
> > +};
> > +
> > +static struct radix_tree_root iommu_groups;
> > +
> > +void __init iommu_groups_init(void)
> > +{
> > +radix_tree_init(_groups);
> > +}
> > +
> > +static struct iommu_group *alloc_iommu_group(unsigned int id)
> > +{
> > +struct iommu_group *grp = xzalloc(struct iommu_group);
> > +
> > +if ( !grp )
> > +return NULL;
> > +
> > +grp->id = id;
> > +
> > +if ( radix_tree_insert(_groups, id, grp) )
> > +{
> > +xfree(grp);
> > +grp = NULL;
> > +}
> > +
> > +return grp;
> > +}
> > +
> > +static struct iommu_group *get_iommu_group(unsigned int id)
> > +{
> > +struct iommu_group *grp = radix_tree_lookup(_groups, id);
> > +
> > +if ( !grp )
> > +grp = alloc_iommu_group(id);
> > +
> > +return grp;
> > +}
> > +
> > +int iommu_group_assign(struct pci_dev *pdev, void *arg)
> 
> I'm not sure I see the point of the arg parameter, AFAICT it's
> completely unused.

It needs to be there because it needs to conform to the all device iterator 
function callback prototype. It is indeed unused though.

> 
> > +{
> > +const struct iommu_ops *ops = iommu_get_ops();
> > +unsigned int id;
> > +struct iommu_group *grp;
> > +
> > +if ( 

Re: [Xen-devel] [PATCH v2 3/4] iommu: introduce iommu_groups

2019-07-15 Thread Roger Pau Monné
On Mon, Jul 15, 2019 at 01:37:09PM +0100, Paul Durrant wrote:
> Some devices may share a single PCIe initiator id, e.g. if they are actually
> legacy PCI devices behind a bridge, and hence DMA from such devices will
> be subject to the same address translation in the IOMMU. Hence these devices
> should be treated as a unit for the purposes of assignment. There are also
> other reasons why multiple devices should be treated as a unit, e.g. those
> subject to a shared RMRR or those downstream of a bridge that does not
> support ACS.
> 
> This patch introduces a new struct iommu_group to act as a container for
> devices that should be treated as a unit, and builds a list of them as
> PCI devices are scanned. The iommu_ops already implement a method,
> get_device_group_id(), that is seemingly intended to return the initiator
> id for a given SBDF so use this as the mechanism for group assignment in
> the first instance. Assignment based on shared RMRR or lack of ACS will be
> dealt with in subsequent patches, as will modifications to the device
> assignment code.
> 
> Signed-off-by: Paul Durrant 

LGTM, just two comments below.

> ---
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Julien Grall 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Cc: Wei Liu 
> 
> v2:
>  - Move code into new drivers/passthrough/groups.c
>  - Drop the group index.
>  - Handle failure to get group id.
>  - Drop the group devs list.
> ---
>  xen/drivers/passthrough/Makefile|  1 +
>  xen/drivers/passthrough/groups.c| 91 
> +
>  xen/drivers/passthrough/x86/iommu.c |  8 +++-
>  xen/include/xen/iommu.h |  7 +++
>  xen/include/xen/pci.h   |  2 +
>  5 files changed, 108 insertions(+), 1 deletion(-)
>  create mode 100644 xen/drivers/passthrough/groups.c
> 
> diff --git a/xen/drivers/passthrough/Makefile 
> b/xen/drivers/passthrough/Makefile
> index d50ab188c8..8a77110179 100644
> --- a/xen/drivers/passthrough/Makefile
> +++ b/xen/drivers/passthrough/Makefile
> @@ -4,6 +4,7 @@ subdir-$(CONFIG_X86) += x86
>  subdir-$(CONFIG_ARM) += arm
>  
>  obj-y += iommu.o
> +obj-$(CONFIG_HAS_PCI) += groups.o
>  obj-$(CONFIG_HAS_PCI) += pci.o
>  obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
>  
> diff --git a/xen/drivers/passthrough/groups.c 
> b/xen/drivers/passthrough/groups.c
> new file mode 100644
> index 00..1a2f461c87
> --- /dev/null
> +++ b/xen/drivers/passthrough/groups.c
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright (c) 2019 Citrix Systems Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that 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.
> + */
> +
> +#include 
> +#include 
> +
> +struct iommu_group {
> +unsigned int id;
> +};
> +
> +static struct radix_tree_root iommu_groups;
> +
> +void __init iommu_groups_init(void)
> +{
> +radix_tree_init(_groups);
> +}
> +
> +static struct iommu_group *alloc_iommu_group(unsigned int id)
> +{
> +struct iommu_group *grp = xzalloc(struct iommu_group);
> +
> +if ( !grp )
> +return NULL;
> +
> +grp->id = id;
> +
> +if ( radix_tree_insert(_groups, id, grp) )
> +{
> +xfree(grp);
> +grp = NULL;
> +}
> +
> +return grp;
> +}
> +
> +static struct iommu_group *get_iommu_group(unsigned int id)
> +{
> +struct iommu_group *grp = radix_tree_lookup(_groups, id);
> +
> +if ( !grp )
> +grp = alloc_iommu_group(id);
> +
> +return grp;
> +}
> +
> +int iommu_group_assign(struct pci_dev *pdev, void *arg)

I'm not sure I see the point of the arg parameter, AFAICT it's
completely unused.

> +{
> +const struct iommu_ops *ops = iommu_get_ops();
> +unsigned int id;
> +struct iommu_group *grp;
> +
> +if ( !ops->get_device_group_id )
> +return 0;
> +
> +id = ops->get_device_group_id(pdev->seg, pdev->bus, pdev->devfn);

I think I would prefer id to be of signed type here, then when you
pass it to get_iommu_group it's already made unsigned.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2 2/4] pci: add all-device iterator function...

2019-07-15 Thread Roger Pau Monné
On Mon, Jul 15, 2019 at 01:37:08PM +0100, Paul Durrant wrote:
> ...and use it for setup_hwdom_pci_devices() and dump_pci_devices().
> 
> The unlock/process-pending-softirqs/lock sequence that was in
> _setup_hwdom_pci_devices() is now done in the generic iterator function,
> which does mean it is also done (unnecessarily) in the case of
> dump_pci_devices(), since run_all_nonirq_keyhandlers() will call
> process_pending_softirqs() before invoking each key handler anyway, but
> this is not performance critical code.
> 
> The " segment  " headline that was in _dump_pci_devices() has
> been dropped because it is non-trivial to deal with it when using a
> generic all-device iterator and, since the segment number is included
> in every log line anyway, it didn't add much value anyway.
> 
> Signed-off-by: Paul Durrant 

Reviewed-by: Roger Pau Monné 

Just some trivial comments.

Thanks.

> ---
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Julien Grall 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Cc: Wei Liu 
> 
> v2:
>  - New in v2.
> ---
>  xen/drivers/passthrough/pci.c | 120 
> +++---
>  xen/include/xen/pci.h |   1 +
>  2 files changed, 68 insertions(+), 53 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index e88689425d..179cb7e17e 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1134,54 +1134,78 @@ static void __hwdom_init setup_one_hwdom_device(const 
> struct setup_hwdom *ctxt,
> ctxt->d->domain_id, err);
>  }
>  
> -static int __hwdom_init _setup_hwdom_pci_devices(struct pci_seg *pseg, void 
> *arg)
> +static int __hwdom_init setup_hwdom_pci_device(struct pci_dev *pdev, void 
> *arg)
>  {
>  struct setup_hwdom *ctxt = arg;
> -int bus, devfn;
> +struct domain *d = ctxt->d;
>  
> -for ( bus = 0; bus < 256; bus++ )
> +if ( !pdev->domain )
>  {
> -for ( devfn = 0; devfn < 256; devfn++ )
> +pdev->domain = d;
> +list_add(>domain_list, >pdev_list);
> +setup_one_hwdom_device(ctxt, pdev);
> +}
> +else if ( pdev->domain == dom_xen )
> +{
> +pdev->domain = d;
> +setup_one_hwdom_device(ctxt, pdev);
> +pdev->domain = dom_xen;
> +}
> +else if ( pdev->domain != d )
> +printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n",
> +   pdev->domain->domain_id, pdev->seg, pdev->bus,

You can use %pd here to print the domain.

> +   PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> +
> +return 0;
> +}
> +
> +struct psdi_ctxt {
> +int (*cb)(struct pci_dev *, void *);
> +void *arg;
> +};
> +
> +static int pci_segment_devices_iterate(struct pci_seg *pseg, void *arg)
> +{
> +struct psdi_ctxt *ctxt = arg;
> +int bus, devfn;

unsigned for both the above.

> +int rc = 0;
> +
> +/*
> + * We don't iterate by walking pseg->alldevs_list here because that
> + * would make the pcidevs_unlock()/lock() sequence below unsafe.
> + */
> +for ( bus = 0; !rc && bus < 256; bus++ )
> +for ( devfn = 0; !rc && devfn < 256; devfn++ )
>  {
>  struct pci_dev *pdev = pci_get_pdev(pseg->nr, bus, devfn);
>  
>  if ( !pdev )
>  continue;
>  
> -if ( !pdev->domain )
> -{
> -pdev->domain = ctxt->d;
> -list_add(>domain_list, >d->pdev_list);
> -setup_one_hwdom_device(ctxt, pdev);
> -}
> -else if ( pdev->domain == dom_xen )
> -{
> -pdev->domain = ctxt->d;
> -setup_one_hwdom_device(ctxt, pdev);
> -pdev->domain = dom_xen;
> -}
> -else if ( pdev->domain != ctxt->d )
> -printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n",
> -   pdev->domain->domain_id, pseg->nr, bus,
> -   PCI_SLOT(devfn), PCI_FUNC(devfn));
> +rc = ctxt->cb(pdev, ctxt->arg);
>  
> -if ( iommu_verbose )
> -{
> -pcidevs_unlock();
> -process_pending_softirqs();
> -pcidevs_lock();
> -}
> -}
> -
> -if ( !iommu_verbose )
> -{
> +/*
> + * Err on the safe side and assume the callback has taken
> + * a significant amount of time.
> + */
>  pcidevs_unlock();
>  process_pending_softirqs();
>  pcidevs_lock();
>  }
> -}
>  
> -return 0;
> +return rc;
> +}
> +
> +int pci_pdevs_iterate(int (*cb)(struct pci_dev *, void *), void *arg)
> +{
> +struct psdi_ctxt ctxt = { .cb = cb, .arg = arg };
> +int rc;
> +
> +pcidevs_lock();
> +rc = pci_segments_iterate(pci_segment_devices_iterate, );
> +  

Re: [Xen-devel] [PATCH v7 5/5] x86/xen: Add "nopv" support for HVM guest

2019-07-15 Thread Boris Ostrovsky
On 7/11/19 8:02 AM, Zhenzhong Duan wrote:
> PVH guest needs PV extentions to work, so "nopv" parameter should be
> ignored for PVH but not for HVM guest.
>
> If PVH guest boots up via the Xen-PVH boot entry, xen_pvh is set early,
> we know it's PVH guest and ignore "nopv" parameter directly.
>
> If PVH guest boots up via the normal boot entry same as HVM guest, it's
> hard to distinguish PVH and HVM guest at that time. In this case, we
> have to panic early if PVH is detected and nopv is enabled to avoid a
> worse situation later.
>
> Move xen_platform_hvm() after xen_hvm_guest_late_init() to avoid compile
> error.
>
> Signed-off-by: Zhenzhong Duan 
> Cc: Boris Ostrovsky 
> Cc: Juergen Gross 
> Cc: Stefano Stabellini 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 


Reviewed-by: Boris Ostrovsky 



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

Re: [Xen-devel] [PATCH 1/2] tools/xenstore: start with empty data base

2019-07-15 Thread George Dunlap
On Tue, Jan 10, 2017 at 4:14 PM Juergen Gross  wrote:
>
> Today xenstored tries to open a tdb data base file on disk when it is
> started. As this is problematic in most cases the scripts used to start
> xenstored ensure xenstored won't find such a file in order to start
> with an empty xenstore.

Sorry to respond to such an old thread, just trying to record this in
a useful place: in the distros design session at the recent XenSummit,
it turned out that:
1. Most distros had to mount a tmpfs for the xenstore database to
prevent xenstore from destroying disk performance
2. xenstored already has an `-I` option which creates a memory-only database

At which point it was asked: Why is this option not the default?

This patch seems to imply that the main reason at the moment for an
external db is debugging; in which case, it does seem like the default
should be in-memory, with an external db as a debugging option.

 -George

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

[Xen-devel] [PATCH v3 1/2] x86/traps: guard top-of-stack reads

2019-07-15 Thread Jan Beulich
Nothing guarantees that the original frame's stack pointer points at
readable memory. Avoid a (likely nested) crash by attaching exception
recovery to the read (making it a single read at the same time). Don't
even invoke _show_trace() in case of a non-readable top slot.

Signed-off-by: Jan Beulich 
Reviewed-by: Andrew Cooper 
---
v2: Name asm() arguments. Use explicit "fault" variable.

--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -486,17 +486,31 @@ static void _show_trace(unsigned long sp
  
  static void show_trace(const struct cpu_user_regs *regs)
  {
-unsigned long *sp = ESP_BEFORE_EXCEPTION(regs);
+unsigned long *sp = ESP_BEFORE_EXCEPTION(regs), tos = 0;
+bool fault = false;
  
  printk("Xen call trace:\n");
  
+/* Guarded read of the stack top. */
+asm ( "1: mov %[data], %[tos]; 2:\n"
+  ".pushsection .fixup,\"ax\"\n"
+  "3: movb $1, %[fault]; jmp 2b\n"
+  ".popsection\n"
+  _ASM_EXTABLE(1b, 3b)
+  : [tos] "+r" (tos), [fault] "+qm" (fault) : [data] "m" (*sp) );
+
  /*
   * If RIP looks sensible, or the top of the stack doesn't, print RIP at
   * the top of the stack trace.
   */
  if ( is_active_kernel_text(regs->rip) ||
- !is_active_kernel_text(*sp) )
+ !is_active_kernel_text(tos) )
  printk("   [<%p>] %pS\n", _p(regs->rip), _p(regs->rip));
+else if ( fault )
+{
+printk("   [Fault on access]\n");
+return;
+}
  /*
   * Else RIP looks bad but the top of the stack looks good.  Perhaps we
   * followed a wild function pointer? Lets assume the top of the stack is a
@@ -505,7 +519,7 @@ static void show_trace(const struct cpu_
   */
  else
  {
-printk("   [<%p>] %pS\n", _p(*sp), _p(*sp));
+printk("   [<%p>] %pS\n", _p(tos), _p(tos));
  sp++;
  }
  

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

[Xen-devel] [PATCH v3 2/2] x86/traps: widen condition for logging top-of-stack

2019-07-15 Thread Jan Beulich
Despite -fno-omit-frame-pointer the compiler may omit the frame pointer,
often for relatively simple leaf functions. (To give a specific example,
the case I've run into this with is _pci_hide_device() and gcc 8.
Interestingly the even more simple neighboring iommu_has_feature() does
get a frame pointer set up, around just a single instruction. But this
may be a result of the size-of-asm() effects discussed elsewhere.)

Log the top-of-stack value if it looks valid _or_ if RIP looks invalid.

Also annotate all stack trace entries with a marker, to indicate their
origin:
R: register state
F: frame pointer based
S: raw stack contents

Signed-off-by: Jan Beulich 
---
v3: Tag stack entries consistently, but differently than in v2.
v2: Re-base over changes to earlier patch.

--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -433,7 +433,7 @@ static void _show_trace(unsigned long sp
  {
  addr = *stack++;
  if ( is_active_kernel_text(addr) )
-printk("   [<%p>] %pS\n", _p(addr), _p(addr));
+printk("   [<%p>] S %pS\n", _p(addr), _p(addr));
  }
  }
  
@@ -476,7 +476,7 @@ static void _show_trace(unsigned long sp
  addr  = frame[1];
  }
  
-printk("   [<%p>] %pS\n", _p(addr), _p(addr));
+printk("   [<%p>] F %pS\n", _p(addr), _p(addr));
  
  low = (unsigned long)[2];
  }
@@ -505,21 +505,26 @@ static void show_trace(const struct cpu_
   */
  if ( is_active_kernel_text(regs->rip) ||
   !is_active_kernel_text(tos) )
-printk("   [<%p>] %pS\n", _p(regs->rip), _p(regs->rip));
-else if ( fault )
+printk("   [<%p>] R %pS\n", _p(regs->rip), _p(regs->rip));
+
+if ( fault )
  {
  printk("   [Fault on access]\n");
  return;
  }
+
  /*
- * Else RIP looks bad but the top of the stack looks good.  Perhaps we
- * followed a wild function pointer? Lets assume the top of the stack is a
+ * If RIP looks bad or the top of the stack looks good, log the top of
+ * stack as well.  Perhaps we followed a wild function pointer, or we're
+ * in a function without frame pointer, or in a function prologue before
+ * the frame pointer gets set up?  Let's assume the top of the stack is a
   * return address; print it and skip past so _show_trace() doesn't print
   * it again.
   */
-else
+if ( !is_active_kernel_text(regs->rip) ||
+ is_active_kernel_text(tos) )
  {
-printk("   [<%p>] %pS\n", _p(tos), _p(tos));
+printk("   [<%p>] S %pS\n", _p(tos), _p(tos));
  sp++;
  }
  

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

Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5

2019-07-15 Thread Andrew Cooper
On 15/07/2019 15:19, Jan Beulich wrote:
> On 15.07.2019 15:49, Andrew Cooper wrote:
>> On 15/07/2019 11:35, Jan Beulich wrote:
>>> With non-empty CONFIG_DOM0_MEM clang5 produces
>>>
>>> dom0_build.c:344:24: error: use of logical '&&' with constant operand 
>>> [-Werror,-Wconstant-logical-operand]
>>>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>>  ^  ~~
>>> dom0_build.c:344:24: note: use '&' for a bitwise operation
>>>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>>  ^~
>>>  &
>>> dom0_build.c:344:24: note: remove constant to silence this warning
>>>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>> ~^
>>> 1 error generated.
>>>
>>> Obviously neither of the two suggestions are an option here. Oddly
>>> enough swapping the operands of the && helps, while e.g. casting or
>>> parenthesizing doesn't. Another workable variant looks to be the use of
>>> !! on the constant.
>>>
>>> Signed-off-by: Jan Beulich 
>>> ---
>>> I'm open to going the !! or yet some different route. No matter which
>>> one we choose, I'm afraid it is going to remain guesswork what newer
>>> (and future) versions of clang will choke on.
>>>
>>> --- a/xen/arch/x86/dom0_build.c
>>> +++ b/xen/arch/x86/dom0_build.c
>>> @@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag
>>>unsigned long avail = 0, nr_pages, min_pages, max_pages;
>>>bool need_paging;
>>>
>>> -if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>> +if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set )
>>>parse_dom0_mem(CONFIG_DOM0_MEM);
>> First and foremost, there is an identical construct on the ARM side,
>> which wants an equivalent adjustment.
> Oh, indeed. I should have remembered ...
>
>> As to the change, I'm going to suggest what I suggested instead of this
>> the first time around, which is strlen(CONFIG_DOM0_MEM) to make the
>> logic easier to follow.
> How does use of strlen() make anything easier? I think it is a pretty
> common pattern to check the first character for nul if all one is
> after is to know whether a string is empty.

Only for things which are obviously a string.  CONFIG_DOM0_MEM isn't.

In this case, you have a name which would most obviously be an integer,
which is compiling in a context where an array reference is valid, which
is confusing to read.

As strlen() is implemented with a builtin, it should be evaluated by the
compiler, given that CONFIG_DOM0_MEM is a constant, but hopefully won't
trigger this warning.

~Andrew

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

[Xen-devel] [PATCH v3 0/2] x86/traps: improve show_trace()'s top-of-stack handling

2019-07-15 Thread Jan Beulich
1: guard top-of-stack reads
2: widen condition for logging top-of-stack

The issue patch 2 fixes (a curious lack of an intermediate call stack
entry) was observed in practice; patch 1 is a result of me just looking
at the code.

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

Re: [Xen-devel] Design session report: Xen on Distros

2019-07-15 Thread Jan Beulich
On 15.07.2019 16:42, George Dunlap wrote:
> On 7/15/19 3:23 PM, Jan Beulich wrote:
>> On 15.07.2019 16:11, George Dunlap wrote:
>>> There was a long discussion about security patches, with the general
>>> proposal being that we should cut a point release for every security issue.
>>
>> Interesting. Looks like in politics that until a decision fits people
>> they keep re-raising the point. Iirc on a prior meeting (Budapest?)
>> we had settled on continuing with the current scheme. Were there any
>> new arguments towards this alternative model?
> 
> Well I don't know if there were any new arguments because I don't
> immediately remember the old discussion.  Do we have a summary of the
> discussion in Budapest, with its conclusions, anywhere?

I don't recall if suitable notes were taken back then; as indicated
I'm not even sure which meeting it was at.

> The basic idea was that:
> 
> 1. Most distros / packagers are going to want to do an immediate release
> anyway.
> 
> 2. Distros generally seemed to be rebasing on top of staging as soon as
> the XSA went out anyway (and ISTR this being the recommeneded course of
> action)
> 
> So for all intents and purposes, we have something which is, in fact, a
> release; all it's missing is a signed tag and a tarball.
> 
> Obviously there are testing implications that would need to be sorted
> out before this could become a reality.
> 
> In any case, the ball is in the court of "VOLUNTEER" to write up a
> concrete proposal which could be discussed.  You'll be able to raise all
> your concerns at that point if you want (although having a sketch would
> of course be helpful for whoever is writing such a proposal).

Sure - I realized soon after having sent the initial reply that perhaps
this was the wrong context in the first place to raise my question.

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

Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5

2019-07-15 Thread Jan Beulich
On 15.07.2019 16:44, Tamas K Lengyel wrote:
> On Mon, Jul 15, 2019 at 8:13 AM Jan Beulich  wrote:
>>
>> On 15.07.2019 15:41, Tamas K Lengyel wrote:
>>> On Mon, Jul 15, 2019 at 4:36 AM Jan Beulich  wrote:

 With non-empty CONFIG_DOM0_MEM clang5 produces

 dom0_build.c:344:24: error: use of logical '&&' with constant operand 
 [-Werror,-Wconstant-logical-operand]
if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
   ^  ~~
 dom0_build.c:344:24: note: use '&' for a bitwise operation
if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
   ^~
   &
 dom0_build.c:344:24: note: remove constant to silence this warning
if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
  ~^
 1 error generated.

 Obviously neither of the two suggestions are an option here. Oddly
 enough swapping the operands of the && helps, while e.g. casting or
 parenthesizing doesn't. Another workable variant looks to be the use of
 !! on the constant.

 Signed-off-by: Jan Beulich 
 ---
 I'm open to going the !! or yet some different route. No matter which
 one we choose, I'm afraid it is going to remain guesswork what newer
 (and future) versions of clang will choke on.
>>>
>>> Is disabling the check itself not an option? Seems to me to be a more
>>> sensible option then hacking around it.
>>
>> How would you envision to disable the check? It's there for a reason
>> after all.
> 
> By passing -Wno-constant-logical-operand to the compiler. It looks
> like a check for a non-common situation which evidently Xen uses, so
> what's the point of keeping it but hacking around with it with tricks
> that are fragile?

Oh, so you meant disabling the compiler warning. That may be an option,
but I wouldn't routinely suggest such as often besides unhelpful
instances there are also helpful ones.

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

Re: [Xen-devel] Design session report: Xen on Distros

2019-07-15 Thread George Dunlap
On 7/15/19 3:42 PM, George Dunlap wrote:
> On 7/15/19 3:23 PM, Jan Beulich wrote:
>> On 15.07.2019 16:11, George Dunlap wrote:
>>> There was a long discussion about security patches, with the general
>>> proposal being that we should cut a point release for every security issue.
>>
>> Interesting. Looks like in politics that until a decision fits people
>> they keep re-raising the point. Iirc on a prior meeting (Budapest?)
>> we had settled on continuing with the current scheme. Were there any
>> new arguments towards this alternative model?

If we end up deciding against doing this again, it might be worth
creating a ticket somewhere to try to capture the arguments on each
side, so that we don't end up re-hashing the same issues in another few
years.

 -George

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

Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5

2019-07-15 Thread Tamas K Lengyel
On Mon, Jul 15, 2019 at 8:13 AM Jan Beulich  wrote:
>
> On 15.07.2019 15:41, Tamas K Lengyel wrote:
> > On Mon, Jul 15, 2019 at 4:36 AM Jan Beulich  wrote:
> >>
> >> With non-empty CONFIG_DOM0_MEM clang5 produces
> >>
> >> dom0_build.c:344:24: error: use of logical '&&' with constant operand 
> >> [-Werror,-Wconstant-logical-operand]
> >>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
> >>  ^  ~~
> >> dom0_build.c:344:24: note: use '&' for a bitwise operation
> >>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
> >>  ^~
> >>  &
> >> dom0_build.c:344:24: note: remove constant to silence this warning
> >>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
> >> ~^
> >> 1 error generated.
> >>
> >> Obviously neither of the two suggestions are an option here. Oddly
> >> enough swapping the operands of the && helps, while e.g. casting or
> >> parenthesizing doesn't. Another workable variant looks to be the use of
> >> !! on the constant.
> >>
> >> Signed-off-by: Jan Beulich 
> >> ---
> >> I'm open to going the !! or yet some different route. No matter which
> >> one we choose, I'm afraid it is going to remain guesswork what newer
> >> (and future) versions of clang will choke on.
> >
> > Is disabling the check itself not an option? Seems to me to be a more
> > sensible option then hacking around it.
>
> How would you envision to disable the check? It's there for a reason
> after all.

By passing -Wno-constant-logical-operand to the compiler. It looks
like a check for a non-common situation which evidently Xen uses, so
what's the point of keeping it but hacking around with it with tricks
that are fragile?

Tamas

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

Re: [Xen-devel] Design session report: Xen on Distros

2019-07-15 Thread George Dunlap
On 7/15/19 3:23 PM, Jan Beulich wrote:
> On 15.07.2019 16:11, George Dunlap wrote:
>> There was a long discussion about security patches, with the general
>> proposal being that we should cut a point release for every security issue.
> 
> Interesting. Looks like in politics that until a decision fits people
> they keep re-raising the point. Iirc on a prior meeting (Budapest?)
> we had settled on continuing with the current scheme. Were there any
> new arguments towards this alternative model?

Well I don't know if there were any new arguments because I don't
immediately remember the old discussion.  Do we have a summary of the
discussion in Budapest, with its conclusions, anywhere?

The basic idea was that:

1. Most distros / packagers are going to want to do an immediate release
anyway.

2. Distros generally seemed to be rebasing on top of staging as soon as
the XSA went out anyway (and ISTR this being the recommeneded course of
action)

So for all intents and purposes, we have something which is, in fact, a
release; all it's missing is a signed tag and a tarball.

Obviously there are testing implications that would need to be sorted
out before this could become a reality.

In any case, the ball is in the court of "VOLUNTEER" to write up a
concrete proposal which could be discussed.  You'll be able to raise all
your concerns at that point if you want (although having a sketch would
of course be helpful for whoever is writing such a proposal).

 -George

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

[Xen-devel] [xen-4.10-testing test] 138996: regressions - trouble: blocked/fail/pass/starved

2019-07-15 Thread osstest service owner
flight 138996 xen-4.10-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/138996/

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-migrupgrade  1 build-check(1)   blocked  n/a
 test-amd64-i386-migrupgrade   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 138376
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail like 138376
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-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 13 migrate-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-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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail 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-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-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  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-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-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-amd64-i386-xl-qemuu-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-qemut-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-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail 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-amd64-xl-qemuu-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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-arm64-arm64-xl-thunderx  2 hosts-allocate

[Xen-devel] [PATCH v2] x86: use POPCNT for hweight() when available

2019-07-15 Thread Jan Beulich
This is faster than using the software implementation, and the insn is
available on all half-way recent hardware. Therefore convert
generic_hweight() to out-of-line functions (without affecting Arm)
and use alternatives patching to replace the function calls.

Note that the approach doesn#t work for clang, due to it not recognizing
-ffixed-*.

Suggested-by: Andrew Cooper 
Signed-off-by: Jan Beulich 
---
v2: Also suppress UB sanitizer instrumentation. Reduce macroization in
 hweight.c. Exclude clang builds.
---
Note: Using "g" instead of "X" as the dummy constraint in hweight64()
   and hweight32(), other than expected, produces slightly better
   code with gcc 8.

--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -31,6 +31,10 @@ obj-y += emul-i8254.o
  obj-y += extable.o
  obj-y += flushtlb.o
  obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
+# clang doesn't appear to know of -ffixed-*
+hweight-$(gcc) := hweight.o
+hweight-$(clang) :=
+obj-y += $(hweight-y)
  obj-y += hypercall.o
  obj-y += i387.o
  obj-y += i8259.o
@@ -251,6 +255,10 @@ boot/mkelf32: boot/mkelf32.c
  efi/mkreloc: efi/mkreloc.c
$(HOSTCC) $(HOSTCFLAGS) -g -o $@ $<
  
+nocov-y += hweight.o
+noubsan-y += hweight.o
+hweight.o: CFLAGS += $(foreach reg,cx dx si 8 9 10 11,-ffixed-r$(reg))
+
  .PHONY: clean
  clean::
rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32
--- /dev/null
+++ b/xen/arch/x86/hweight.c
@@ -0,0 +1,21 @@
+#define generic_hweight64 _hweight64
+#define generic_hweight32 _hweight32
+#define generic_hweight16 _hweight16
+#define generic_hweight8  _hweight8
+
+#include 
+
+#undef inline
+#define inline always_inline
+
+#include 
+
+#undef generic_hweight8
+#undef generic_hweight16
+#undef generic_hweight32
+#undef generic_hweight64
+
+unsigned int generic_hweight8 (unsigned int x) { return _hweight8 (x); }
+unsigned int generic_hweight16(unsigned int x) { return _hweight16(x); }
+unsigned int generic_hweight32(unsigned int x) { return _hweight32(x); }
+unsigned int generic_hweight64(uint64_t x) { return _hweight64(x); }
--- a/xen/include/asm-x86/bitops.h
+++ b/xen/include/asm-x86/bitops.h
@@ -475,9 +475,36 @@ static inline int fls(unsigned int x)
   *
   * The Hamming Weight of a number is the total number of bits set in it.
   */
+#ifndef __clang__
+/* POPCNT encodings with %{r,e}di input and %{r,e}ax output: */
+#define POPCNT_64 ".byte 0xF3, 0x48, 0x0F, 0xB8, 0xC7"
+#define POPCNT_32 ".byte 0xF3, 0x0F, 0xB8, 0xC7"
+
+#define hweight_(n, x, insn, setup, cout, cin) ({ \
+unsigned int res_;\
+/*\
+ * For the function call the POPCNT input register needs to be marked \
+ * modified as well. Set up a local variable of appropriate type  \
+ * for this purpose.  \
+ */   \
+typeof((uint##n##_t)(x) + 0U) val_ = (x); \
+alternative_io(setup "; call generic_hweight" #n, \
+   insn, X86_FEATURE_POPCNT,  \
+   ASM_OUTPUT2([res] "=a" (res_), [val] cout (val_)), \
+   [src] cin (val_)); \
+res_; \
+})
+#define hweight64(x) hweight_(64, x, POPCNT_64, "", "+D", "g")
+#define hweight32(x) hweight_(32, x, POPCNT_32, "", "+D", "g")
+#define hweight16(x) hweight_(16, x, "movzwl %w[src], %[val]; " POPCNT_32, \
+  "mov %[src], %[val]", "=", "rm")
+#define hweight8(x)  hweight_( 8, x, "movzbl %b[src], %[val]; " POPCNT_32, \
+  "mov %[src], %[val]", "=", "rm")
+#else
  #define hweight64(x) generic_hweight64(x)
  #define hweight32(x) generic_hweight32(x)
  #define hweight16(x) generic_hweight16(x)
  #define hweight8(x) generic_hweight8(x)
+#endif
  
  #endif /* _X86_BITOPS_H */
x86: use POPCNT for hweight() when available

This is faster than using the software implementation, and the insn is
available on all half-way recent hardware. Therefore convert
generic_hweight() to out-of-line functions (without affecting Arm)
and use alternatives patching to replace the function calls.

Note that the approach doesn#t work for clang, due to it not recognizing
-ffixed-*.

Suggested-by: Andrew Cooper 
Signed-off-by: Jan Beulich 
---
v2: Also suppress UB sanitizer instrumentation. Reduce macroization in
hweight.c. Exclude clang builds.
---
Note: Using "g" instead of "X" as the dummy constraint in hweight64()
  and hweight32(), other than expected, produces slightly better
  code with gcc 8.

--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -31,6 +31,10 @@ obj-y += emul-i8254.o
 obj-y += extable.o
 obj-y += flushtlb.o
 

Re: [Xen-devel] [PATCH v2 1/4] iommu / x86: move call to scan_pci_devices() out of vendor code

2019-07-15 Thread Roger Pau Monné
On Mon, Jul 15, 2019 at 01:37:07PM +0100, Paul Durrant wrote:
> It's not vendor specific so it doesn't really belong there.
> 
> Scanning the PCI topology also really doesn't have much to do with IOMMU
> initialization. It doesn't depend on there even being an IOMMU. This patch
> moves to the call to the beginning of iommu_hardware_setup() but only
> places it there because the topology information would be otherwise unused.
> 
> Subsequent patches will actually make use of the PCI topology during
> (x86) IOMMU initialization.
> 
> Signed-off-by: Paul Durrant 

Reviewed-by: Roger Pau Monné 

I would even consider moving the call to scan_pci_devices into
pci_segments_init instead of doing it in the IOMMU code, as you
suggest above.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH

2019-07-15 Thread Roger Pau Monné
On Mon, Jul 15, 2019 at 12:50:29PM +0100, Andrew Cooper wrote:
> On 15/07/2019 12:46, Roger Pau Monné wrote:
> >> +;
> >> +; Jump to the main routine of the pre-SEC code
> >> +; skiping the 16-bit part of the routine and
> >> +; into the 32-bit flat mode part
> >> +;
> >> +OneTimeCallRet TransitionFromReal16To32BitFlat
> > Since PVH already starts in flat 32bit mode, I'm not sure I see the
> > point of this routine, since it seems to be used exclusively to switch
> > from 16 to 32b flat mode. The comment mentions skipping that part, but
> > I'm not sure I see how that's achieved.
> 
> Its some OVMF local magic.  This means "jmp
> end_of_TransitionFromReal16To32BitFlat", which is the correct place to
> go, but the code really is misleading to read.

Oh right, it's OneTimeCallRet. I guess this is obvious if you are
familiar with OVMF code, which I'm not. Expanding the comment to
mention that jumping to the end of the routine is achieved by using
OneTimeCallRet would have helped me, but this might be too verbose.

Thanks, Roger.

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

Re: [Xen-devel] Design session report: Xen on Distros

2019-07-15 Thread Jan Beulich
On 15.07.2019 16:11, George Dunlap wrote:
> There was a long discussion about security patches, with the general
> proposal being that we should cut a point release for every security issue.

Interesting. Looks like in politics that until a decision fits people
they keep re-raising the point. Iirc on a prior meeting (Budapest?)
we had settled on continuing with the current scheme. Were there any
new arguments towards this alternative model?

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

Re: [Xen-devel] [PATCH v3 09/35] OvmfPkg/OvmfXen: use a TimerLib instance that depends only on the CPU

2019-07-15 Thread Roger Pau Monné
On Thu, Jul 04, 2019 at 03:42:07PM +0100, Anthony PERARD wrote:
> ACPI Timer does not work in a PVH guest, but local APIC works on both

This is not accurate. It's not that the ACPI timer doesn't work, it's
just that it's not present. The PM_TMR_BLK should be set to 0 to
indicate the lack of PM timer support, or else there's something
broken.

> PVH and HVM.
> 
> Note that the use of SecPeiDxeTimerLibCpu might be an issue with a
> driver of type DXE_RUNTIME_DRIVER. I've attemptde to find out which of
  ^ attempted
> the DXE_RUNTIME_DRIVER uses the TimerLib at runtime. I've done that by
> replacing the TimerLib evaluation in
> [LibraryClasses.common.DXE_RUNTIME_DRIVER] by a different one and
> check every module that uses it (with the --report-file=report build
  ^ checking
> option).
> 
> ResetSystemRuntimeDxe is calling the TimerLib API at runtime to do the
> operation "EfiResetCold", so this may never complete if the OS have
> disabled the Local APIC Timer.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5

2019-07-15 Thread Jan Beulich
On 15.07.2019 15:49, Andrew Cooper wrote:
> On 15/07/2019 11:35, Jan Beulich wrote:
>> With non-empty CONFIG_DOM0_MEM clang5 produces
>>
>> dom0_build.c:344:24: error: use of logical '&&' with constant operand 
>> [-Werror,-Wconstant-logical-operand]
>>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>  ^  ~~
>> dom0_build.c:344:24: note: use '&' for a bitwise operation
>>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>  ^~
>>  &
>> dom0_build.c:344:24: note: remove constant to silence this warning
>>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>> ~^
>> 1 error generated.
>>
>> Obviously neither of the two suggestions are an option here. Oddly
>> enough swapping the operands of the && helps, while e.g. casting or
>> parenthesizing doesn't. Another workable variant looks to be the use of
>> !! on the constant.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> I'm open to going the !! or yet some different route. No matter which
>> one we choose, I'm afraid it is going to remain guesswork what newer
>> (and future) versions of clang will choke on.
>>
>> --- a/xen/arch/x86/dom0_build.c
>> +++ b/xen/arch/x86/dom0_build.c
>> @@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag
>>unsigned long avail = 0, nr_pages, min_pages, max_pages;
>>bool need_paging;
>>
>> -if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>> +if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set )
>>parse_dom0_mem(CONFIG_DOM0_MEM);
> 
> First and foremost, there is an identical construct on the ARM side,
> which wants an equivalent adjustment.

Oh, indeed. I should have remembered ...

> As to the change, I'm going to suggest what I suggested instead of this
> the first time around, which is strlen(CONFIG_DOM0_MEM) to make the
> logic easier to follow.

How does use of strlen() make anything easier? I think it is a pretty
common pattern to check the first character for nul if all one is
after is to know whether a string is empty.

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

Re: [Xen-devel] [PATCH v3 24/35] OvmfPkg/XenPlatformPei: Rework memory detection

2019-07-15 Thread Roger Pau Monné
On Thu, Jul 04, 2019 at 03:42:22PM +0100, Anthony PERARD wrote:
> When running as a Xen PVH guest, there is no CMOS to read the memory
> size from.  Rework GetSystemMemorySize(Below|Above)4gb() so they can
> works without CMOS by reading the e820 table.
> 
> Rework XenPublishRamRegions for PVH, handle the Reserve type and explain
> about the ACPI type. MTRR settings aren't modified anymore, on HVM, it's
> already done by hvmloader, on PVH it is supposed to have sane default.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
> Signed-off-by: Anthony PERARD 
> Acked-by: Laszlo Ersek 
> ---
> 
> Notes:
> Comment for Xen people:
> About MTRR, should we redo the setting in OVMF? Even if in both case of
> PVH and HVM, something would have setup the default type to write back
> and handle a few other ranges like PCI hole, hvmloader for HVM or and
> libxc I think for PVH.

That's a tricky question. Ideally we would like the firmware (OVMF) to
take care of that, because it already has code to do so. Problem here
is that PVH can also be booted without firmware, in which case it
needs the hypervisor to have setup some sane initial MTRR state.

The statement in the PVH document about initial MTRR state is vague
enough that allows Xen to boot into the guest with a minimal MTRR
state, that can for example not contain UC regions for the MMIO
regions of passed through devices, hence I think OVMF should be in
charge of creating a more complete MTRR state if possible.

Is this something OVMF already has logic for?

Also accounting for the MMIO regions of devices?

> (For PVH, it's in the spec as well
> https://xenbits.xenproject.org/docs/unstable/misc/pvh.html#mtrr )
> 
>  OvmfPkg/XenPlatformPei/Platform.h  |  6 +++
>  OvmfPkg/XenPlatformPei/MemDetect.c | 71 ++
>  OvmfPkg/XenPlatformPei/Xen.c   | 47 ++--
>  3 files changed, 111 insertions(+), 13 deletions(-)
> 
> diff --git a/OvmfPkg/XenPlatformPei/Platform.h 
> b/OvmfPkg/XenPlatformPei/Platform.h
> index db9a62572f..e8e0b835a5 100644
> --- a/OvmfPkg/XenPlatformPei/Platform.h
> +++ b/OvmfPkg/XenPlatformPei/Platform.h
> @@ -114,6 +114,12 @@ XenPublishRamRegions (
>VOID
>);
>  
> +EFI_STATUS
> +XenGetE820Map (
> +  EFI_E820_ENTRY64 **Entries,
> +  UINT32 *Count
> +  );
> +
>  extern EFI_BOOT_MODE mBootMode;
>  
>  extern UINT8 mPhysMemAddressWidth;
> diff --git a/OvmfPkg/XenPlatformPei/MemDetect.c 
> b/OvmfPkg/XenPlatformPei/MemDetect.c
> index cb7dd93ad6..3e33e7f414 100644
> --- a/OvmfPkg/XenPlatformPei/MemDetect.c
> +++ b/OvmfPkg/XenPlatformPei/MemDetect.c
> @@ -96,6 +96,47 @@ Q35TsegMbytesInitialization (
>mQ35TsegMbytes = ExtendedTsegMbytes;
>  }
>  
> +STATIC
> +UINT64
> +GetHighestSystemMemoryAddress (
> +  BOOLEAN   Below4gb
> +  )
> +{
> +  EFI_E820_ENTRY64*E820Map;
> +  UINT32  E820EntriesCount;
> +  EFI_E820_ENTRY64*Entry;
> +  EFI_STATUS  Status;
> +  UINT32  Loop;
> +  UINT64  HighestAddress;
> +  UINT64  EntryEnd;
> +
> +  HighestAddress = 0;
> +
> +  Status = XenGetE820Map (, );

You could maybe initialize this as a global to avoid having to issue
a hypercall each time you need to get something from the memory map.

> +  ASSERT_EFI_ERROR (Status);
> +
> +  for (Loop = 0; Loop < E820EntriesCount; Loop++) {
> +Entry = E820Map + Loop;
> +EntryEnd = Entry->BaseAddr + Entry->Length;
> +
> +if (Entry->Type == EfiAcpiAddressRangeMemory &&
> +EntryEnd > HighestAddress) {
> +
> +  if (Below4gb && (EntryEnd <= BASE_4GB)) {
> +HighestAddress = EntryEnd;
> +  } else if (!Below4gb && (EntryEnd >= BASE_4GB)) {
> +HighestAddress = EntryEnd;
> +  }
> +}
> +  }
> +
> +  //
> +  // Round down the end address.
> +  //
> +  HighestAddress &= ~(UINT64)EFI_PAGE_MASK;
> +
> +  return HighestAddress;

You could do the rounding on the return statement.

> +}
>  
>  UINT32
>  GetSystemMemorySizeBelow4gb (
> @@ -105,6 +146,19 @@ GetSystemMemorySizeBelow4gb (
>UINT8 Cmos0x34;
>UINT8 Cmos0x35;
>  
> +  //
> +  // In PVH case, there is no CMOS, we have to calculate the memory size
> +  // from parsing the E820
> +  //
> +  if (XenPvhDetected ()) {

IIRC on HVM you can also get the memory map from the hypercall, in
which case you could use the same code path for both HVM and PVH.

> +UINT64  HighestAddress;
> +
> +HighestAddress = GetHighestSystemMemoryAddress (TRUE);
> +ASSERT (HighestAddress > 0 && HighestAddress <= BASE_4GB);
> +
> +return HighestAddress;

The name of the function here is GetSystemMemorySizeBelow4gb, but you
are returning the highest memory address in the range, is this
expected?

ie: highest address != memory size

On HVM there are quite some holes in the memory map, and nothing
guarantees there are no memory regions after the holes or non-RAM
regions.

> +  }
> +
>//
>// CMOS 0x34/0x35 specifies the system memory above 16 

Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5

2019-07-15 Thread Jan Beulich
On 15.07.2019 15:41, Tamas K Lengyel wrote:
> On Mon, Jul 15, 2019 at 4:36 AM Jan Beulich  wrote:
>>
>> With non-empty CONFIG_DOM0_MEM clang5 produces
>>
>> dom0_build.c:344:24: error: use of logical '&&' with constant operand 
>> [-Werror,-Wconstant-logical-operand]
>>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>  ^  ~~
>> dom0_build.c:344:24: note: use '&' for a bitwise operation
>>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>  ^~
>>  &
>> dom0_build.c:344:24: note: remove constant to silence this warning
>>   if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>> ~^
>> 1 error generated.
>>
>> Obviously neither of the two suggestions are an option here. Oddly
>> enough swapping the operands of the && helps, while e.g. casting or
>> parenthesizing doesn't. Another workable variant looks to be the use of
>> !! on the constant.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> I'm open to going the !! or yet some different route. No matter which
>> one we choose, I'm afraid it is going to remain guesswork what newer
>> (and future) versions of clang will choke on.
> 
> Is disabling the check itself not an option? Seems to me to be a more
> sensible option then hacking around it.

How would you envision to disable the check? It's there for a reason
after all.

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

[Xen-devel] Design session report: Xen on Distros

2019-07-15 Thread George Dunlap
Much of this covered things discussed elsewhere:
* Allowing multiple versions of the tools to be installed at the same time
* Getting rid of external builds

There was a long discussion about security patches, with the general
proposal being that we should cut a point release for every security issue.

One random thing was that xenstored apparently has an 'in-memory-only'
option.  Since xenstored can't actually be restarted ATM, and most
distros seemed to put xenstored in a tmpfs for performance reasons, this
should probably be the default.

https://hackmd.io/vmacVBYbQiORJ9H4_a9Ivw

# Xen on Distros design session

* qemu / libxc dependency loop
* build system needs "extras" turned off
* xenstored / tmpfs / memory-only option?
* Disabling auto-download in build system
* WGET=/bin/false
* Multiple version of Xen / tools?
* Debian has co-install
* Change some installation paths
* /usr/lib/xen/4.11/...
* /usr/bin/xl is a shell script
* libfsimage is special
* Don't need to downgrade to older tools
* Gentoo has a ~~dumpster fire~~ something
* A hack which stops the package manager to allow you to reboot
the box halfway through
* Security issues
* Building from stable branch / staging branch
* Doing a "point release" every XSA?
* "Release from staging" is effectively a low-quality release
* Idea: Always immediately release from staging?


# Actions
* [ ] Ian: Post a git branch of Debian co-install to xen-devel
* [ ] George: Post systemd / selinux / xenstored patch
* [ ] George, Ian: private osstest runs
* [ ] VOLUNTEER: Propose / argue for a point release per XSA
* [ ] VOLUNTEER: Improve release automation

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

Re: [Xen-devel] [PATCH 00/60] xen: add core scheduling support

2019-07-15 Thread Sergey Dyasli
On 05/07/2019 14:56, Dario Faggioli wrote:
> On Fri, 2019-07-05 at 14:17 +0100, Sergey Dyasli wrote:
>> 1) This crash is quite likely to happen:
>>
>> [2019-07-04 18:22:46 UTC] (XEN) [ 3425.220660] Watchdog timer detects
>> that CPU2 is stuck!
>> [2019-07-04 18:22:46 UTC] (XEN) [ 3425.226293] [ Xen-4.13.0-
>> 8.0.6-d  x86_64  debug=y   Not tainted ]
>> [...]
>> [2019-07-04 18:22:47 UTC] (XEN) [ 3425.501989] Xen call trace:
>> [2019-07-04 18:22:47 UTC] (XEN) [
>> 3425.505278][] vcpu_sleep_sync+0x50/0x71
>> [2019-07-04 18:22:47 UTC] (XEN) [
>> 3425.511518][] vcpu_pause+0x21/0x23
>> [2019-07-04 18:22:47 UTC] (XEN) [
>> 3425.517326][]
>> vcpu_set_periodic_timer+0x27/0x73
>> [2019-07-04 18:22:47 UTC] (XEN) [
>> 3425.524258][] do_vcpu_op+0x2c9/0x668
>> [2019-07-04 18:22:47 UTC] (XEN) [
>> 3425.530238][] compat_vcpu_op+0x250/0x390
>> [2019-07-04 18:22:47 UTC] (XEN) [
>> 3425.536566][] pv_hypercall+0x364/0x564
>> [2019-07-04 18:22:47 UTC] (XEN) [
>> 3425.542719][] do_entry_int82+0x26/0x2d
>> [2019-07-04 18:22:47 UTC] (XEN) [
>> 3425.548876][] entry_int82+0xbb/0xc0
>>
> Mmm... vcpu_set_periodic_timer?
> 
> What kernel is this and when does this crash happen?

Hi Dario,

I can easily reproduce this crash using a Debian 7 PV VM (2 vCPUs, 2GB RAM)
which has the following kernel:

# uname -a

Linux localhost 3.2.0-4-amd64 #1 SMP Debian 3.2.78-1 x86_64 GNU/Linux

All I need to do is suspend and resume the VM.

Thanks,
Sergey

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

[Xen-devel] Design Session report: Toolstacks

2019-07-15 Thread George Dunlap
Looking through the notes, it seems like the first part of this
discussion, re hypervisor upgrade / downgrade & libraries, was partly
covered in the distro session, in which Debian's Xen version co-install
was discussed and found useful even if we had a hypervisor , Ian Jackson
agreed to post Debian's co-install patches.

There was a general agreement to improve xl's json handling (including
accepting json to xl create), and a discussion about other improvements;
various people who proposed them seem likely to come back with patches.

 -George

https://hackmd.io/0vZaSrKBT2iKWzpVMxDVvQ

# Xen toolstacks

Issues:
* Hypervisor upgrade / downgrade
* xl not fully-featured
* Remote access

Potential places:
* ~~On top of libxl~~
* Or libxc (symlink swizzling scheme)
* Doesn't work w/ statically linked
* Doesn't require HV to deal w/ backwards ABI compatibility
* On the hypervisor
* Need to keep working for a full release cycle
* A daemon

* xl create w/ json blob
* xl does some processing before passing to libxl

Hypershell has effective golang libxl bindings (but need to be
recompiled against new versions of libxl)

Other wishlist:
* More events (e.g., backend / frontend disconnect)
* Or, have xl / libxl clean up disconnected entries & notify when done
* Feature parity on other dom0 operating systems







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

Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5

2019-07-15 Thread Andrew Cooper
On 15/07/2019 11:35, Jan Beulich wrote:
> With non-empty CONFIG_DOM0_MEM clang5 produces
>
> dom0_build.c:344:24: error: use of logical '&&' with constant operand 
> [-Werror,-Wconstant-logical-operand]
>  if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
> ^  ~~
> dom0_build.c:344:24: note: use '&' for a bitwise operation
>  if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
> ^~
> &
> dom0_build.c:344:24: note: remove constant to silence this warning
>  if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>~^
> 1 error generated.
>
> Obviously neither of the two suggestions are an option here. Oddly
> enough swapping the operands of the && helps, while e.g. casting or
> parenthesizing doesn't. Another workable variant looks to be the use of
> !! on the constant.
>
> Signed-off-by: Jan Beulich 
> ---
> I'm open to going the !! or yet some different route. No matter which
> one we choose, I'm afraid it is going to remain guesswork what newer
> (and future) versions of clang will choke on.
>
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag
>   unsigned long avail = 0, nr_pages, min_pages, max_pages;
>   bool need_paging;
>   
> -if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
> +if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set )
>   parse_dom0_mem(CONFIG_DOM0_MEM);

First and foremost, there is an identical construct on the ARM side,
which wants an equivalent adjustment.

As to the change, I'm going to suggest what I suggested instead of this
the first time around, which is strlen(CONFIG_DOM0_MEM) to make the
logic easier to follow.

~Andrew

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

Re: [Xen-devel] [PATCH] x86/dom0-build: fix build with clang5

2019-07-15 Thread Tamas K Lengyel
On Mon, Jul 15, 2019 at 4:36 AM Jan Beulich  wrote:
>
> With non-empty CONFIG_DOM0_MEM clang5 produces
>
> dom0_build.c:344:24: error: use of logical '&&' with constant operand 
> [-Werror,-Wconstant-logical-operand]
>  if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
> ^  ~~
> dom0_build.c:344:24: note: use '&' for a bitwise operation
>  if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
> ^~
> &
> dom0_build.c:344:24: note: remove constant to silence this warning
>  if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>~^
> 1 error generated.
>
> Obviously neither of the two suggestions are an option here. Oddly
> enough swapping the operands of the && helps, while e.g. casting or
> parenthesizing doesn't. Another workable variant looks to be the use of
> !! on the constant.
>
> Signed-off-by: Jan Beulich 
> ---
> I'm open to going the !! or yet some different route. No matter which
> one we choose, I'm afraid it is going to remain guesswork what newer
> (and future) versions of clang will choke on.

Is disabling the check itself not an option? Seems to me to be a more
sensible option then hacking around it.

Tamas

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

[Xen-devel] Design session notes: Build system gripe session

2019-07-15 Thread George Dunlap
Below are the raw notes.  General agreements seem to be:

* Removing external builds from xen build system is an advantage for the
main users (developers, distros / packagers)
* Out-of-tree build is useful for lots of circumstances; symlink trick
used to work around its lack causes lots of problems

There didn't seem to be anyone volunteering to hack at the build system,
but with enough interest in seeing it change, and a general agreement on
the direction we want to go, perhaps someone will step up when they get
a chance.

 -George

https://hackmd.io/mAGT5bU9T6-aXJVj88deYw

# Build system gripe session

***Link available from the design-sessions session description page***

Topics include:

 * Moving "external" builds to a separate part of the tree, not built by
default
 * Adding more "external" builds; `pvgrub` for instance
 * Making it easy to build with no internet access
 * Using `git submodules`


Issues:
1. out of tree build not supported -- shim build still half broken
2. Xen build re-evaluates compiler support for every translation unit.
(Switch to newer Kconfig.)
3. meta virtualization build system (yocto) needs to pull simlink
tricks, xen's build system stomp on that
4. containerize.sh is broken by symlink tricks in the build system
5. External projects get pulled in. Some think they should be removed,
some think more should be added. Solved by raisin but nobody used it



For out-of-tree build, for xen, use Kbuild.

Would be nice to be able to do (without having to clean or fix or patch):
```
make CONFIG=release.config O=output/release
make CONFIG=shim.config O=output/shim
```

Different users have differnt requirements: developers, users, distro
packagers.

For everyday users we should point people to their distros.

Raisin tried to be devstack, which is not developers want or trust.

People in favour of removing more stuff from xen.git. Should start with
a document describing how to build a xen system -- pull this tree, use
this rune etc etc. Or use Android's build system?

Git runes in tree should disappear.

Need transitional plan for osstest.

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

[Xen-devel] [PATCH v2 4/4] iommu / pci: re-implement XEN_DOMCTL_get_device_group...

2019-07-15 Thread Paul Durrant
... using the new iommu_group infrastructure.

Because 'sibling' devices are now members of the same iommu_group,
implement the domctl by looking up the iommu_group of the pdev with the
matching SBDF and then finding all the assigned pdevs that are in the
group.

Signed-off-by: Paul Durrant 
---
Cc: Jan Beulich 

v2:
 - Re-implement in the absence of a per-group devs list.
 - Make use of pci_sbdf_t.
---
 xen/drivers/passthrough/groups.c | 44 ++
 xen/drivers/passthrough/pci.c| 51 ++--
 xen/include/xen/iommu.h  |  2 ++
 3 files changed, 48 insertions(+), 49 deletions(-)

diff --git a/xen/drivers/passthrough/groups.c b/xen/drivers/passthrough/groups.c
index 1a2f461c87..6d8064e4f4 100644
--- a/xen/drivers/passthrough/groups.c
+++ b/xen/drivers/passthrough/groups.c
@@ -12,8 +12,12 @@
  * GNU General Public License for more details.
  */
 
+#include 
 #include 
+#include 
 #include 
+#include 
+#include 
 
 struct iommu_group {
 unsigned int id;
@@ -81,6 +85,46 @@ int iommu_group_assign(struct pci_dev *pdev, void *arg)
 return 0;
 }
 
+int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf,
+   XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs)
+{
+struct iommu_group *grp = NULL;
+struct pci_dev *pdev;
+unsigned int i = 0;
+
+pcidevs_lock();
+
+for_each_pdev ( d, pdev )
+{
+if ( pdev->sbdf.sbdf == sbdf.sbdf )
+{
+grp = pdev->grp;
+break;
+}
+}
+
+if ( !grp )
+goto out;
+
+for_each_pdev ( d, pdev )
+{
+if ( xsm_get_device_group(XSM_HOOK, pdev->sbdf.sbdf) ||
+ pdev->grp != grp )
+continue;
+
+if ( unlikely(copy_to_guest_offset(buf, i++, >sbdf.sbdf, 1)) )
+{
+pcidevs_unlock();
+return -EFAULT;
+}
+}
+
+ out:
+pcidevs_unlock();
+
+return i;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 179cb7e17e..3a5e90c176 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1563,53 +1563,6 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, 
u8 devfn)
 return ret;
 }
 
-static int iommu_get_device_group(
-struct domain *d, u16 seg, u8 bus, u8 devfn,
-XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs)
-{
-const struct domain_iommu *hd = dom_iommu(d);
-struct pci_dev *pdev;
-int group_id, sdev_id;
-u32 bdf;
-int i = 0;
-const struct iommu_ops *ops = hd->platform_ops;
-
-if ( !iommu_enabled || !ops || !ops->get_device_group_id )
-return 0;
-
-group_id = ops->get_device_group_id(seg, bus, devfn);
-
-pcidevs_lock();
-for_each_pdev( d, pdev )
-{
-if ( (pdev->seg != seg) ||
- ((pdev->bus == bus) && (pdev->devfn == devfn)) )
-continue;
-
-if ( xsm_get_device_group(XSM_HOOK, (seg << 16) | (pdev->bus << 8) | 
pdev->devfn) )
-continue;
-
-sdev_id = ops->get_device_group_id(seg, pdev->bus, pdev->devfn);
-if ( (sdev_id == group_id) && (i < max_sdevs) )
-{
-bdf = 0;
-bdf |= (pdev->bus & 0xff) << 16;
-bdf |= (pdev->devfn & 0xff) << 8;
-
-if ( unlikely(copy_to_guest_offset(buf, i, , 1)) )
-{
-pcidevs_unlock();
-return -1;
-}
-i++;
-}
-}
-
-pcidevs_unlock();
-
-return i;
-}
-
 void iommu_dev_iotlb_flush_timeout(struct domain *d, struct pci_dev *pdev)
 {
 pcidevs_lock();
@@ -1666,11 +1619,11 @@ int iommu_do_pci_domctl(
 max_sdevs = domctl->u.get_device_group.max_sdevs;
 sdevs = domctl->u.get_device_group.sdev_array;
 
-ret = iommu_get_device_group(d, seg, bus, devfn, sdevs, max_sdevs);
+ret = iommu_get_device_group(d, PCI_SBDF3(seg, bus, devfn), sdevs,
+ max_sdevs);
 if ( ret < 0 )
 {
 dprintk(XENLOG_ERR, "iommu_get_device_group() failed!\n");
-ret = -EFAULT;
 domctl->u.get_device_group.num_sdevs = 0;
 }
 else
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index c93f580fdc..ac764b41f9 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -321,6 +321,8 @@ extern struct page_list_head iommu_pt_cleanup_list;
 
 void iommu_groups_init(void);
 int iommu_group_assign(struct pci_dev *pdev, void *arg);
+int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf,
+   XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs);
 
 #endif /* CONFIG_HAS_PCI */
 
-- 
2.11.0


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

[Xen-devel] [PATCH v2 2/4] pci: add all-device iterator function...

2019-07-15 Thread Paul Durrant
...and use it for setup_hwdom_pci_devices() and dump_pci_devices().

The unlock/process-pending-softirqs/lock sequence that was in
_setup_hwdom_pci_devices() is now done in the generic iterator function,
which does mean it is also done (unnecessarily) in the case of
dump_pci_devices(), since run_all_nonirq_keyhandlers() will call
process_pending_softirqs() before invoking each key handler anyway, but
this is not performance critical code.

The " segment  " headline that was in _dump_pci_devices() has
been dropped because it is non-trivial to deal with it when using a
generic all-device iterator and, since the segment number is included
in every log line anyway, it didn't add much value anyway.

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

v2:
 - New in v2.
---
 xen/drivers/passthrough/pci.c | 120 +++---
 xen/include/xen/pci.h |   1 +
 2 files changed, 68 insertions(+), 53 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index e88689425d..179cb7e17e 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1134,54 +1134,78 @@ static void __hwdom_init setup_one_hwdom_device(const 
struct setup_hwdom *ctxt,
ctxt->d->domain_id, err);
 }
 
-static int __hwdom_init _setup_hwdom_pci_devices(struct pci_seg *pseg, void 
*arg)
+static int __hwdom_init setup_hwdom_pci_device(struct pci_dev *pdev, void *arg)
 {
 struct setup_hwdom *ctxt = arg;
-int bus, devfn;
+struct domain *d = ctxt->d;
 
-for ( bus = 0; bus < 256; bus++ )
+if ( !pdev->domain )
 {
-for ( devfn = 0; devfn < 256; devfn++ )
+pdev->domain = d;
+list_add(>domain_list, >pdev_list);
+setup_one_hwdom_device(ctxt, pdev);
+}
+else if ( pdev->domain == dom_xen )
+{
+pdev->domain = d;
+setup_one_hwdom_device(ctxt, pdev);
+pdev->domain = dom_xen;
+}
+else if ( pdev->domain != d )
+printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n",
+   pdev->domain->domain_id, pdev->seg, pdev->bus,
+   PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+
+return 0;
+}
+
+struct psdi_ctxt {
+int (*cb)(struct pci_dev *, void *);
+void *arg;
+};
+
+static int pci_segment_devices_iterate(struct pci_seg *pseg, void *arg)
+{
+struct psdi_ctxt *ctxt = arg;
+int bus, devfn;
+int rc = 0;
+
+/*
+ * We don't iterate by walking pseg->alldevs_list here because that
+ * would make the pcidevs_unlock()/lock() sequence below unsafe.
+ */
+for ( bus = 0; !rc && bus < 256; bus++ )
+for ( devfn = 0; !rc && devfn < 256; devfn++ )
 {
 struct pci_dev *pdev = pci_get_pdev(pseg->nr, bus, devfn);
 
 if ( !pdev )
 continue;
 
-if ( !pdev->domain )
-{
-pdev->domain = ctxt->d;
-list_add(>domain_list, >d->pdev_list);
-setup_one_hwdom_device(ctxt, pdev);
-}
-else if ( pdev->domain == dom_xen )
-{
-pdev->domain = ctxt->d;
-setup_one_hwdom_device(ctxt, pdev);
-pdev->domain = dom_xen;
-}
-else if ( pdev->domain != ctxt->d )
-printk(XENLOG_WARNING "Dom%d owning %04x:%02x:%02x.%u?\n",
-   pdev->domain->domain_id, pseg->nr, bus,
-   PCI_SLOT(devfn), PCI_FUNC(devfn));
+rc = ctxt->cb(pdev, ctxt->arg);
 
-if ( iommu_verbose )
-{
-pcidevs_unlock();
-process_pending_softirqs();
-pcidevs_lock();
-}
-}
-
-if ( !iommu_verbose )
-{
+/*
+ * Err on the safe side and assume the callback has taken
+ * a significant amount of time.
+ */
 pcidevs_unlock();
 process_pending_softirqs();
 pcidevs_lock();
 }
-}
 
-return 0;
+return rc;
+}
+
+int pci_pdevs_iterate(int (*cb)(struct pci_dev *, void *), void *arg)
+{
+struct psdi_ctxt ctxt = { .cb = cb, .arg = arg };
+int rc;
+
+pcidevs_lock();
+rc = pci_segments_iterate(pci_segment_devices_iterate, );
+pcidevs_unlock();
+
+return rc;
 }
 
 void __hwdom_init setup_hwdom_pci_devices(
@@ -1189,9 +1213,7 @@ void __hwdom_init setup_hwdom_pci_devices(
 {
 struct setup_hwdom ctxt = { .d = d, .handler = handler };
 
-pcidevs_lock();
-pci_segments_iterate(_setup_hwdom_pci_devices, );
-pcidevs_unlock();
+pci_pdevs_iterate(setup_hwdom_pci_device, );
 }
 
 #ifdef CONFIG_ACPI
@@ -1294,24 +1316,18 @@ bool_t pcie_aer_get_firmware_first(const struct pci_dev 
*pdev)
 }
 #endif
 
-static 

[Xen-devel] [PATCH v2 3/4] iommu: introduce iommu_groups

2019-07-15 Thread Paul Durrant
Some devices may share a single PCIe initiator id, e.g. if they are actually
legacy PCI devices behind a bridge, and hence DMA from such devices will
be subject to the same address translation in the IOMMU. Hence these devices
should be treated as a unit for the purposes of assignment. There are also
other reasons why multiple devices should be treated as a unit, e.g. those
subject to a shared RMRR or those downstream of a bridge that does not
support ACS.

This patch introduces a new struct iommu_group to act as a container for
devices that should be treated as a unit, and builds a list of them as
PCI devices are scanned. The iommu_ops already implement a method,
get_device_group_id(), that is seemingly intended to return the initiator
id for a given SBDF so use this as the mechanism for group assignment in
the first instance. Assignment based on shared RMRR or lack of ACS will be
dealt with in subsequent patches, as will modifications to the device
assignment code.

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

v2:
 - Move code into new drivers/passthrough/groups.c
 - Drop the group index.
 - Handle failure to get group id.
 - Drop the group devs list.
---
 xen/drivers/passthrough/Makefile|  1 +
 xen/drivers/passthrough/groups.c| 91 +
 xen/drivers/passthrough/x86/iommu.c |  8 +++-
 xen/include/xen/iommu.h |  7 +++
 xen/include/xen/pci.h   |  2 +
 5 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 xen/drivers/passthrough/groups.c

diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
index d50ab188c8..8a77110179 100644
--- a/xen/drivers/passthrough/Makefile
+++ b/xen/drivers/passthrough/Makefile
@@ -4,6 +4,7 @@ subdir-$(CONFIG_X86) += x86
 subdir-$(CONFIG_ARM) += arm
 
 obj-y += iommu.o
+obj-$(CONFIG_HAS_PCI) += groups.o
 obj-$(CONFIG_HAS_PCI) += pci.o
 obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
 
diff --git a/xen/drivers/passthrough/groups.c b/xen/drivers/passthrough/groups.c
new file mode 100644
index 00..1a2f461c87
--- /dev/null
+++ b/xen/drivers/passthrough/groups.c
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2019 Citrix Systems Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that 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.
+ */
+
+#include 
+#include 
+
+struct iommu_group {
+unsigned int id;
+};
+
+static struct radix_tree_root iommu_groups;
+
+void __init iommu_groups_init(void)
+{
+radix_tree_init(_groups);
+}
+
+static struct iommu_group *alloc_iommu_group(unsigned int id)
+{
+struct iommu_group *grp = xzalloc(struct iommu_group);
+
+if ( !grp )
+return NULL;
+
+grp->id = id;
+
+if ( radix_tree_insert(_groups, id, grp) )
+{
+xfree(grp);
+grp = NULL;
+}
+
+return grp;
+}
+
+static struct iommu_group *get_iommu_group(unsigned int id)
+{
+struct iommu_group *grp = radix_tree_lookup(_groups, id);
+
+if ( !grp )
+grp = alloc_iommu_group(id);
+
+return grp;
+}
+
+int iommu_group_assign(struct pci_dev *pdev, void *arg)
+{
+const struct iommu_ops *ops = iommu_get_ops();
+unsigned int id;
+struct iommu_group *grp;
+
+if ( !ops->get_device_group_id )
+return 0;
+
+id = ops->get_device_group_id(pdev->seg, pdev->bus, pdev->devfn);
+if ( id < 0 )
+return -ENODATA;
+
+grp = get_iommu_group(id);
+if ( !grp )
+return -ENOMEM;
+
+if ( iommu_verbose )
+printk(XENLOG_INFO "Assign %04x:%02x:%02x.%u -> IOMMU group %x\n",
+   pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
+   PCI_FUNC(pdev->devfn), grp->id);
+
+pdev->grp = grp;
+
+return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index a7438c9c25..90fc750456 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -43,7 +43,13 @@ int __init iommu_hardware_setup(void)
 /* x2apic setup may have previously initialised the struct. */
 ASSERT(iommu_ops.init == iommu_init_ops->ops->init);
 
-return iommu_init_ops->setup();
+rc = iommu_init_ops->setup();
+if ( rc )
+return rc;
+
+iommu_groups_init();
+
+return pci_pdevs_iterate(iommu_group_assign, NULL);
 }
 
 

[Xen-devel] [PATCH v2 1/4] iommu / x86: move call to scan_pci_devices() out of vendor code

2019-07-15 Thread Paul Durrant
It's not vendor specific so it doesn't really belong there.

Scanning the PCI topology also really doesn't have much to do with IOMMU
initialization. It doesn't depend on there even being an IOMMU. This patch
moves to the call to the beginning of iommu_hardware_setup() but only
places it there because the topology information would be otherwise unused.

Subsequent patches will actually make use of the PCI topology during
(x86) IOMMU initialization.

Signed-off-by: Paul Durrant 
---
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
Cc: Kevin Tian 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 

v2:
 - Expanded commit comment.
 - Moved PCI scan to before IOMMU initialization, rather than after it.
---
 xen/drivers/passthrough/amd/pci_amd_iommu.c | 3 ++-
 xen/drivers/passthrough/vtd/iommu.c | 4 
 xen/drivers/passthrough/x86/iommu.c | 6 ++
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c 
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 4afbcd1609..3338a8e0e8 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -180,7 +180,8 @@ static int __init iov_detect(void)
 
 if ( !amd_iommu_perdev_intremap )
 printk(XENLOG_WARNING "AMD-Vi: Using global interrupt remap table is 
not recommended (see XSA-36)!\n");
-return scan_pci_devices();
+
+return 0;
 }
 
 int amd_iommu_alloc_root(struct domain_iommu *hd)
diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 8b27d7e775..b0e3bf26b5 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2372,10 +2372,6 @@ static int __init vtd_setup(void)
 P(iommu_hap_pt_share, "Shared EPT tables");
 #undef P
 
-ret = scan_pci_devices();
-if ( ret )
-goto error;
-
 ret = init_vtd_hw();
 if ( ret )
 goto error;
diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index 0fa6dcc3fd..a7438c9c25 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -28,9 +28,15 @@ struct iommu_ops __read_mostly iommu_ops;
 
 int __init iommu_hardware_setup(void)
 {
+int rc;
+
 if ( !iommu_init_ops )
 return -ENODEV;
 
+rc = scan_pci_devices();
+if ( rc )
+return rc;
+
 if ( !iommu_ops.init )
 iommu_ops = *iommu_init_ops->ops;
 else
-- 
2.11.0


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

Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support

2019-07-15 Thread Juergen Gross

On 15.07.19 14:32, Peter Zijlstra wrote:

On Mon, Jul 15, 2019 at 01:37:37PM +0200, Juergen Gross wrote:

The long term plan has been to replace Xen PV guests by PVH. The first
victim of that plan are now 32-bit PV guests, as those are used only
rather seldom these days. Xen on x86 requires 64-bit support and with
Grub2 now supporting PVH officially since version 2.04 there is no
need to keep 32-bit PV guest support alive in the Linux kernel.
Additionally Meltdown mitigation is not available in the kernel running
as 32-bit PV guest, so dropping this mode makes sense from security
point of view, too.

Juergen Gross (2):
   x86/xen: remove 32-bit Xen PV guest support
   x86/paravirt: remove 32-bit support from PARAVIRT_XXL


Hooray!



Always a pleasure to cheer the community up by sending Xen patches. :-D


Juergen

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

[Xen-devel] [PATCH v2 0/4] iommu groups + cleanup

2019-07-15 Thread Paul Durrant
This series is a mixture of tidying and some preparatory work for grouping
PCI devices for the purposes of assignment.

Paul Durrant (4):
  iommu / x86: move call to scan_pci_devices() out of vendor code
  pci: add all-device iterator function...
  iommu: introduce iommu_groups
  iommu / pci: re-implement XEN_DOMCTL_get_device_group...

 xen/drivers/passthrough/Makefile|   1 +
 xen/drivers/passthrough/amd/pci_amd_iommu.c |   3 +-
 xen/drivers/passthrough/groups.c| 135 ++
 xen/drivers/passthrough/pci.c   | 171 +++-
 xen/drivers/passthrough/vtd/iommu.c |   4 -
 xen/drivers/passthrough/x86/iommu.c |  14 ++-
 xen/include/xen/iommu.h |   9 ++
 xen/include/xen/pci.h   |   3 +
 8 files changed, 232 insertions(+), 108 deletions(-)
 create mode 100644 xen/drivers/passthrough/groups.c
---
v2:
 - Drop iommu_get_ops() move and add all-device iterator

Cc: Andrew Cooper 
Cc: Brian Woods 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Kevin Tian 
Cc: Konrad Rzeszutek Wilk 
Cc: "Roger Pau Monné" 
Cc: Stefano Stabellini 
Cc: Suravee Suthikulpanit 
Cc: Tim Deegan 
Cc: Wei Liu 
-- 
2.11.0


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

Re: [Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support

2019-07-15 Thread Peter Zijlstra
On Mon, Jul 15, 2019 at 01:37:37PM +0200, Juergen Gross wrote:
> The long term plan has been to replace Xen PV guests by PVH. The first
> victim of that plan are now 32-bit PV guests, as those are used only
> rather seldom these days. Xen on x86 requires 64-bit support and with
> Grub2 now supporting PVH officially since version 2.04 there is no
> need to keep 32-bit PV guest support alive in the Linux kernel.
> Additionally Meltdown mitigation is not available in the kernel running
> as 32-bit PV guest, so dropping this mode makes sense from security
> point of view, too.
> 
> Juergen Gross (2):
>   x86/xen: remove 32-bit Xen PV guest support
>   x86/paravirt: remove 32-bit support from PARAVIRT_XXL

Hooray!

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

[Xen-devel] [linux-linus bisection] complete test-amd64-amd64-xl-qemuu-ovmf-amd64

2019-07-15 Thread osstest service owner
branch xen-unstable
xenbranch xen-unstable
job test-amd64-amd64-xl-qemuu-ovmf-amd64
testid xen-boot

Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  linux 
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
  Bug introduced:  a2d79c7174aeb43b13020dd53d85a7aefdd9f3e5
  Bug not present: 3ab4436f688c2d2f221793953cd05435ca84261c
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/139018/


  (Revision log too long, omitted.)


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-linus/test-amd64-amd64-xl-qemuu-ovmf-amd64.xen-boot.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/linux-linus/test-amd64-amd64-xl-qemuu-ovmf-amd64.xen-boot
 --summary-out=tmp/139018.bisection-summary --basis-template=133580 
--blessings=real,real-bisect linux-linus test-amd64-amd64-xl-qemuu-ovmf-amd64 
xen-boot
Searching for failure / basis pass:
 138962 fail [host=baroque0] / 138849 [host=italia0] 138813 [host=italia1] 
138780 [host=godello0] 138754 [host=pinot1] 138735 [host=baroque1] 138710 
[host=godello1] 138680 [host=albana1] 138661 [host=fiano0] 138639 
[host=rimava1] 138612 [host=debina1] 138584 [host=albana0] 138488 
[host=italia0] 138386 [host=chardonnay1] 138245 [host=chardonnay0] 138073 
[host=elbling1] 137986 [host=elbling0] 137896 [host=debina0] 137739 
[host=fiano0] 137686 [host=italia0] 137589 [host=albana1] 137484 [host=debina\
 1] 137388 [host=elbling1] 137283 [host=baroque1] 137191 [host=rimava1] 137125 
ok.
Failure / basis pass flights: 138962 / 137125
(tree with no url: minios)
Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git
Latest a2d79c7174aeb43b13020dd53d85a7aefdd9f3e5 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
91cc60bafc7d6e49b7bc85990f895d6228f51364 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
9cca02d8ffc23e9688a971d858e4ffdff5389b11 
30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 
b541287c3600713feaaaf7608cd405e7b2e4efd0
Basis pass 3ab4436f688c2d2f221793953cd05435ca84261c 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
5a9e23ceb991f3bd0eea74d6b67f9102f65ea6bc 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
9cca02d8ffc23e9688a971d858e4ffdff5389b11 
0932c20560574696cf87ddd12623e8c423ee821b 
81646cea826fa322831fffb43f81e7e0866dc124
Generating revisions with ./adhoc-revtuple-generator  
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git#3ab4436f688c2d2f221793953cd05435ca84261c-a2d79c7174aeb43b13020dd53d85a7aefdd9f3e5
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/osstest/ovmf.git#5a9e23ceb991f3bd0eea74d6b67f9102f65ea6bc-91cc60bafc7d6e49b7bc85990f895d6228f51364
 git://xenbits.xen.org/qemu-xen-traditional.\
 
git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798
 
git://xenbits.xen.org/qemu-xen.git#9cca02d8ffc23e9688a971d858e4ffdff5389b11-9cca02d8ffc23e9688a971d858e4ffdff5389b11
 
git://xenbits.xen.org/osstest/seabios.git#0932c20560574696cf87ddd12623e8c423ee821b-30f1e41f04fb4c715d27f987f003cfc31c9ff4f3
 
git://xenbits.xen.org/xen.git#81646cea826fa322831fffb43f81e7e0866dc124-b541287c3600713feaaaf7608cd405e7b2e4efd0
adhoc-revtuple-generator: tree discontiguous: linux-2.6
Loaded 3002 nodes in revision graph
Searching for test results:
 137098 [host=albana0]
 137125 pass 3ab4436f688c2d2f221793953cd05435ca84261c 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
5a9e23ceb991f3bd0eea74d6b67f9102f65ea6bc 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
9cca02d8ffc23e9688a971d858e4ffdff5389b11 
0932c20560574696cf87ddd12623e8c423ee821b 
81646cea826fa322831fffb43f81e7e0866dc124
 137191 [host=rimava1]
 137283 [host=baroque1]
 137388 [host=elbling1]
 137484 [host=debina1]
 137589 [host=albana1]
 137739 [host=fiano0]
 137686 [host=italia0]
 137896 [host=debina0]
 137986 [host=elbling0]
 138073 [host=elbling1]
 138245 [host=chardonnay0]
 138386 [host=chardonnay1]
 138488 [host=italia0]
 138584 [host=albana0]
 138612 [host=debina1]
 138639 [host=rimava1]
 138661 [host=fiano0]
 138680 [host=albana1]
 138710 

Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Jan Beulich
On 15.07.2019 13:09, Paul Durrant wrote:
>> From: Jan Beulich 
>> Sent: 15 July 2019 11:44
>>
>> Well, the problem is that I don't feel well adjusting what a native
>> English speaking person has written.
> 
> Ok. How about...
> 
> "This patch adds a helper function, put_page_alloc_ref(), to replace
> all the open-coded test-and-clear/put_page occurrences. That helper
> function incorporates a check that an additional page reference is
> held and will BUG() if it is not."

Sounds good. I'll record this for merging in if no other need for a
v3 arises.

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

Re: [Xen-devel] [PATCH v2] MAINTAINERS: Make myself libxl golang binding maintainer

2019-07-15 Thread Andrew Cooper
On 15/07/2019 13:01, George Dunlap wrote:
> Signed-off-by: George Dunlap 

Acked-by: Andrew Cooper 

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

[Xen-devel] [PATCH v2] MAINTAINERS: Make myself libxl golang binding maintainer

2019-07-15 Thread George Dunlap
Signed-off-by: George Dunlap 
---
v2:
- Put in alphabetical order
- Replace spaces with tabs

CC: Ian Jackson 
CC: Wei Liu 
CC: Andrew Cooper 
CC: Jan Beulich 
CC: Tim Deegan 
CC: Konrad Wilk 
CC: Stefano Stabellini 
CC: Julien Grall 
---
 MAINTAINERS | 5 +
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b8e4daae41..f18567b56a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -234,6 +234,11 @@ S: Supported
 F: xen/arch/x86/debug.c
 F: tools/debugger/gdbsx/
 
+GOLANG BINDINGS
+M: George Dunlap 
+S: Maintained
+F: tools/golang
+
 INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
 M: Gang Wei 
 M: Shane Wang 
-- 
2.20.1


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

Re: [Xen-devel] [PATCH] MAINTAINERS: Make myself libxl golang binding maintainer

2019-07-15 Thread George Dunlap
On 7/10/19 3:51 AM, Jan Beulich wrote:
> On 09.07.2019 22:13, George Dunlap wrote:
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -343,6 +343,11 @@ M:  Marek Marczykowski-Górecki 
>> 
>>   S: Supported
>>   F: tools/python
>>   
>> +GOLANG BINDINGS
>> +M: George Dunlap 
>> +S: Maintained
>> +F: tools/golang
>> +
>>   QEMU-DM
>>   M: Ian Jackson 
>>   S: Supported
> 
> This doesn't look to be the alphabetically correct slot to insert
> this section. Yet instead of moving it perhaps its title should
> be "libxl golang bindings"?

Didn't realize this was in alphabetical order.

I was going to say 'libxl golang bindings', but the actual directory in
question is, like the python bindings it's next to, generic; someone
could if they wanted to write golang bindings for any of the other
libraries, and they'd be in this directory; and I'd probably be the
right person to review them if they went in.

> Also please use tabs like surrounding sections do.

Ack.  v2 on the way.

 -George

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

Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH

2019-07-15 Thread Andrew Cooper
On 15/07/2019 12:46, Roger Pau Monné wrote:
>> diff --git a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm 
>> b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
>> new file mode 100644
>> index 00..2a17fed52f
>> --- /dev/null
>> +++ b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
>> @@ -0,0 +1,49 @@
>> +;--
>> +; @file
>> +; An entry point use by Xen when a guest is started in PVH mode.
>> +;
>> +; Copyright (c) 2019, Citrix Systems, Inc.
>> +;
>> +; SPDX-License-Identifier: BSD-2-Clause-Patent
>> +;
>> +;--
>> +
>> +BITS32
>> +
>> +xenPVHMain:
>> +;
>> +; 'BP' to indicate boot-strap processor
>> +;
>> +mov di, 'BP'
>> +
>> +;
>> +; ESP will be used as initial value of the EAX register
>> +; in Main.asm
>> +;
>> +xor esp, esp
>> +
>> +mov ebx, ADDR_OF(gdtr)
>> +lgdt[ebx]
>> +
>> +mov eax, SEC_DEFAULT_CR0
>> +mov cr0, eax
>> +
>> +jmp LINEAR_CODE_SEL:ADDR_OF(.jmpToNewCodeSeg)
>> +.jmpToNewCodeSeg:
>> +
>> +mov eax, SEC_DEFAULT_CR4
>> +mov cr4, eax
>> +
>> +mov ax, LINEAR_SEL
>> +mov ds, ax
>> +mov es, ax
>> +mov fs, ax
>> +mov gs, ax
>> +mov ss, ax
>> +
>> +;
>> +; Jump to the main routine of the pre-SEC code
>> +; skiping the 16-bit part of the routine and
>> +; into the 32-bit flat mode part
>> +;
>> +OneTimeCallRet TransitionFromReal16To32BitFlat
> Since PVH already starts in flat 32bit mode, I'm not sure I see the
> point of this routine, since it seems to be used exclusively to switch
> from 16 to 32b flat mode. The comment mentions skipping that part, but
> I'm not sure I see how that's achieved.

Its some OVMF local magic.  This means "jmp
end_of_TransitionFromReal16To32BitFlat", which is the correct place to
go, but the code really is misleading to read.

~Andrew

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

Re: [Xen-devel] [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH

2019-07-15 Thread Roger Pau Monné
On Thu, Jul 04, 2019 at 03:42:04PM +0100, Anthony PERARD wrote:
> Add a new entry point for Xen PVH that enter directly in 32bits.
> 
> Information on the expected state of the machine when this entry point
> is used can be found at:
> https://xenbits.xenproject.org/docs/unstable/misc/pvh.html
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
> Signed-off-by: Anthony PERARD 

Thanks for doing this! My knowledge of nasm is very limited, so some
of the above comments might be completely wrong.

> ---
> 
> Notes:
> v3:
> - rebased, SPDX
> - remove `cli' as via PVH the interrupts are guaranteed to be off
> - rewrite some comments
> 
>  .../XenResetVector/Ia16/ResetVectorVtf0.asm   | 81 +++
>  OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm| 49 +++
>  OvmfPkg/XenResetVector/XenResetVector.nasmb   |  1 +
>  3 files changed, 131 insertions(+)
>  create mode 100644 OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
>  create mode 100644 OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
> 
> diff --git a/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm 
> b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
> new file mode 100644
> index 00..958195bc5e
> --- /dev/null
> +++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
> @@ -0,0 +1,81 @@
> +;--
> +; @file
> +; First code executed by processor after resetting.
> +;
> +; Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.

Extraneous  tag?

> +; Copyright (c) 2019, Citrix Systems, Inc.
> +;
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +;--
> +
> +BITS16
> +
> +ALIGN   16

Do you need the BITS and ALIGN here?

Isn't it enough with the BITS 32 below for the entry point, since DB
is already explicitly sized?

> +
> +;
> +; Pad the image size to 4k when page tables are in VTF0
> +;
> +; If the VTF0 image has page tables built in, then we need to make
> +; sure the end of VTF0 is 4k above where the page tables end.
> +;
> +; This is required so the page tables will be 4k aligned when VTF0 is
> +; located just below 0x1 (4GB) in the firmware device.
> +;
> +%ifdef ALIGN_TOP_TO_4K_FOR_PAGING
> +TIMES (0x1000 - ($ - EndOfPageTables) - (fourGigabytes - 
> xenPVHEntryPoint)) DB 0

What's the meaning of 0x1000 here?

> +%endif
> +
> +BITS32
> +xenPVHEntryPoint:
> +;
> +; Entry point to use when running as a Xen PVH guest. (0xffd0)

Shouldn't this positioning be set on the linker script instead?

> +;
> +; Description of the expected state of the machine when this entry point is
> +; used can be found at:
> +; https://xenbits.xenproject.org/docs/unstable/misc/pvh.html
> +;
> +jmp xenPVHMain
> +
> +BITS16
> +ALIGN   16

Is it really needed to specify both?

I would assume that setting BITS 16 will already set a suitable
alignment.

> +
> +applicationProcessorEntryPoint:
> +;
> +; Application Processors entry point
> +;
> +; GenFv generates code aligned on a 4k boundary which will jump to this
> +; location.  (0xffe0)  This allows the Local APIC Startup IPI to be

Also, if xenPVHEntryPoint is at 0x...d0, how can
applicationProcessorEntryPoint be at 0x...e0, I guess there's some
other code I'm missing that either adds padding between both, or
places them in different sections on the resulting binary image?

> +; used to wake up the application processors.
> +;
> +jmp EarlyApInitReal16
> +
> +ALIGN   8
> +
> +DD  0

Can you remove this DD...

> +
> +;
> +; The VTF signature
> +;
> +; VTF-0 means that the VTF (Volume Top File) code does not require
> +; any fixups.
> +;
> +vtfSignature:
> +DB  'V', 'T', 'F', 0

And instead do DB 0, 0, 0, 0, 'V',...?

In any case I'm not sure of the point of setting align to 8 and then
writing 32bits of 0s (but again maybe I'm just misreading the code).

Maybe you just want to set align to 32 and write the vtf signature?

> +
> +ALIGN   16
> +
> +resetVector:
> +;
> +; Reset Vector
> +;
> +; This is where the processor will begin execution
> +;
> +nop
> +nop
> +jmp EarlyBspInitReal16
> +
> +ALIGN   16
> +
> +fourGigabytes:
> +
> diff --git a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm 
> b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
> new file mode 100644
> index 00..2a17fed52f
> --- /dev/null
> +++ b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
> @@ -0,0 +1,49 @@
> +;--
> +; @file
> +; An entry point use by Xen when a guest is started in PVH mode.
> +;
> +; Copyright (c) 2019, Citrix Systems, Inc.
> +;
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +;--
> +
> +BITS32
> +
> +xenPVHMain:
> +;
> +; 'BP' to indicate boot-strap processor
> +;
> +mov di, 'BP'
> +
> + 

[Xen-devel] [PATCH 1/2] x86/xen: remove 32-bit Xen PV guest support

2019-07-15 Thread Juergen Gross
Xen is requiring 64-bit machines today. There is no need to carry the
burden of 32-bit PV guest support in the kernel any longer, as new
guests can be either HVM or PVH, or they can use a 64 bit kernel.

Remove the 32-bit Xen PV support from the kernel.

Signed-off-by: Juergen Gross 
---
 arch/x86/entry/entry_32.S  |  93 
 arch/x86/include/asm/proto.h   |   2 +-
 arch/x86/include/asm/segment.h |   2 +-
 arch/x86/include/asm/traps.h   |   2 +-
 arch/x86/xen/Kconfig   |   3 +-
 arch/x86/xen/Makefile  |   4 +-
 arch/x86/xen/apic.c|  17 ---
 arch/x86/xen/enlighten_pv.c|  45 +-
 arch/x86/xen/mmu_pv.c  | 326 -
 arch/x86/xen/p2m.c |   4 -
 arch/x86/xen/setup.c   |  44 +-
 arch/x86/xen/smp_pv.c  |  19 +--
 arch/x86/xen/xen-asm.S |  14 --
 arch/x86/xen/xen-asm_32.S  | 207 --
 arch/x86/xen/xen-head.S|   6 -
 arch/x86/xen/xen-ops.h |   5 -
 drivers/xen/Kconfig|   4 +-
 17 files changed, 42 insertions(+), 755 deletions(-)
 delete mode 100644 arch/x86/xen/xen-asm_32.S

diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 7b23431be5cb..d4464af28212 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -787,16 +787,6 @@ GLOBAL(__begin_SYSENTER_singlestep_region)
  * will ignore all of the single-step traps generated in this range.
  */
 
-#ifdef CONFIG_XEN_PV
-/*
- * Xen doesn't set %esp to be precisely what the normal SYSENTER
- * entry point expects, so fix it up before using the normal path.
- */
-ENTRY(xen_sysenter_target)
-   addl$5*4, %esp  /* remove xen-provided frame */
-   jmp .Lsysenter_past_esp
-#endif
-
 /*
  * 32-bit SYSENTER entry.
  *
@@ -1249,89 +1239,6 @@ ENTRY(spurious_interrupt_bug)
jmp common_exception
 END(spurious_interrupt_bug)
 
-#ifdef CONFIG_XEN_PV
-ENTRY(xen_hypervisor_callback)
-   pushl   $-1 /* orig_ax = -1 => not a system 
call */
-   SAVE_ALL
-   ENCODE_FRAME_POINTER
-   TRACE_IRQS_OFF
-
-   /*
-* Check to see if we got the event in the critical
-* region in xen_iret_direct, after we've reenabled
-* events and checked for pending events.  This simulates
-* iret instruction's behaviour where it delivers a
-* pending interrupt when enabling interrupts:
-*/
-   movlPT_EIP(%esp), %eax
-   cmpl$xen_iret_start_crit, %eax
-   jb  1f
-   cmpl$xen_iret_end_crit, %eax
-   jae 1f
-
-   jmp xen_iret_crit_fixup
-
-ENTRY(xen_do_upcall)
-1: mov %esp, %eax
-   callxen_evtchn_do_upcall
-#ifndef CONFIG_PREEMPT
-   callxen_maybe_preempt_hcall
-#endif
-   jmp ret_from_intr
-ENDPROC(xen_hypervisor_callback)
-
-/*
- * Hypervisor uses this for application faults while it executes.
- * We get here for two reasons:
- *  1. Fault while reloading DS, ES, FS or GS
- *  2. Fault while executing IRET
- * Category 1 we fix up by reattempting the load, and zeroing the segment
- * register if the load fails.
- * Category 2 we fix up by jumping to do_iret_error. We cannot use the
- * normal Linux return path in this case because if we use the IRET hypercall
- * to pop the stack frame we end up in an infinite loop of failsafe callbacks.
- * We distinguish between categories by maintaining a status value in EAX.
- */
-ENTRY(xen_failsafe_callback)
-   pushl   %eax
-   movl$1, %eax
-1: mov 4(%esp), %ds
-2: mov 8(%esp), %es
-3: mov 12(%esp), %fs
-4: mov 16(%esp), %gs
-   /* EAX == 0 => Category 1 (Bad segment)
-  EAX != 0 => Category 2 (Bad IRET) */
-   testl   %eax, %eax
-   popl%eax
-   lea 16(%esp), %esp
-   jz  5f
-   jmp iret_exc
-5: pushl   $-1 /* orig_ax = -1 => not a system 
call */
-   SAVE_ALL
-   ENCODE_FRAME_POINTER
-   jmp ret_from_exception
-
-.section .fixup, "ax"
-6: xorl%eax, %eax
-   movl%eax, 4(%esp)
-   jmp 1b
-7: xorl%eax, %eax
-   movl%eax, 8(%esp)
-   jmp 2b
-8: xorl%eax, %eax
-   movl%eax, 12(%esp)
-   jmp 3b
-9: xorl%eax, %eax
-   movl%eax, 16(%esp)
-   jmp 4b
-.previous
-   _ASM_EXTABLE(1b, 6b)
-   _ASM_EXTABLE(2b, 7b)
-   _ASM_EXTABLE(3b, 8b)
-   _ASM_EXTABLE(4b, 9b)
-ENDPROC(xen_failsafe_callback)
-#endif /* CONFIG_XEN_PV */
-
 #ifdef CONFIG_XEN_PVHVM
 BUILD_INTERRUPT3(xen_hvm_callback_vector, HYPERVISOR_CALLBACK_VECTOR,
 xen_evtchn_do_upcall)
diff --git a/arch/x86/include/asm/proto.h b/arch/x86/include/asm/proto.h
index 6e81788a30c1..e5a13a138b01 100644
--- a/arch/x86/include/asm/proto.h
+++ b/arch/x86/include/asm/proto.h
@@ -25,7 +25,7 @@ void entry_SYSENTER_compat(void);
 void 

[Xen-devel] [PATCH 2/2] x86/paravirt: remove 32-bit support from PARAVIRT_XXL

2019-07-15 Thread Juergen Gross
The last 32-bit user of stuff under CONFIG_PARAVIRT_XXL is gone.

Remove 32-bit specific parts.

Signed-off-by: Juergen Gross 
---
 arch/x86/entry/vdso/vdso32/vclock_gettime.c |   1 +
 arch/x86/include/asm/paravirt.h | 105 
 arch/x86/include/asm/paravirt_types.h   |  20 --
 arch/x86/include/asm/pgtable-3level_types.h |   5 --
 arch/x86/kernel/cpu/common.c|   8 ---
 arch/x86/kernel/paravirt.c  |  17 -
 arch/x86/kernel/paravirt_patch_32.c |  36 +-
 7 files changed, 15 insertions(+), 177 deletions(-)

diff --git a/arch/x86/entry/vdso/vdso32/vclock_gettime.c 
b/arch/x86/entry/vdso/vdso32/vclock_gettime.c
index 9242b28418d5..36f4ce1405cb 100644
--- a/arch/x86/entry/vdso/vdso32/vclock_gettime.c
+++ b/arch/x86/entry/vdso/vdso32/vclock_gettime.c
@@ -17,6 +17,7 @@
 #undef CONFIG_ILLEGAL_POINTER_VALUE
 #undef CONFIG_SPARSEMEM_VMEMMAP
 #undef CONFIG_NR_CPUS
+#undef CONFIG_PARAVIRT_XXL
 
 #define CONFIG_X86_32 1
 #define CONFIG_PGTABLE_LEVELS 2
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index c25c38a05c1c..60dfa93313a9 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -139,7 +139,6 @@ static inline void __write_cr4(unsigned long x)
PVOP_VCALL1(cpu.write_cr4, x);
 }
 
-#ifdef CONFIG_X86_64
 static inline unsigned long read_cr8(void)
 {
return PVOP_CALL0(unsigned long, cpu.read_cr8);
@@ -149,7 +148,6 @@ static inline void write_cr8(unsigned long x)
 {
PVOP_VCALL1(cpu.write_cr8, x);
 }
-#endif
 
 static inline void arch_safe_halt(void)
 {
@@ -283,12 +281,10 @@ static inline void load_TLS(struct thread_struct *t, 
unsigned cpu)
PVOP_VCALL2(cpu.load_tls, t, cpu);
 }
 
-#ifdef CONFIG_X86_64
 static inline void load_gs_index(unsigned int gs)
 {
PVOP_VCALL1(cpu.load_gs_index, gs);
 }
-#endif
 
 static inline void write_ldt_entry(struct desc_struct *dt, int entry,
   const void *desc)
@@ -375,50 +371,28 @@ static inline pte_t __pte(pteval_t val)
 {
pteval_t ret;
 
-   if (sizeof(pteval_t) > sizeof(long))
-   ret = PVOP_CALLEE2(pteval_t, mmu.make_pte, val, (u64)val >> 32);
-   else
-   ret = PVOP_CALLEE1(pteval_t, mmu.make_pte, val);
+   ret = PVOP_CALLEE1(pteval_t, mmu.make_pte, val);
 
return (pte_t) { .pte = ret };
 }
 
 static inline pteval_t pte_val(pte_t pte)
 {
-   pteval_t ret;
-
-   if (sizeof(pteval_t) > sizeof(long))
-   ret = PVOP_CALLEE2(pteval_t, mmu.pte_val,
-  pte.pte, (u64)pte.pte >> 32);
-   else
-   ret = PVOP_CALLEE1(pteval_t, mmu.pte_val, pte.pte);
-
-   return ret;
+   return PVOP_CALLEE1(pteval_t, mmu.pte_val, pte.pte);
 }
 
 static inline pgd_t __pgd(pgdval_t val)
 {
pgdval_t ret;
 
-   if (sizeof(pgdval_t) > sizeof(long))
-   ret = PVOP_CALLEE2(pgdval_t, mmu.make_pgd, val, (u64)val >> 32);
-   else
-   ret = PVOP_CALLEE1(pgdval_t, mmu.make_pgd, val);
+   ret = PVOP_CALLEE1(pgdval_t, mmu.make_pgd, val);
 
return (pgd_t) { ret };
 }
 
 static inline pgdval_t pgd_val(pgd_t pgd)
 {
-   pgdval_t ret;
-
-   if (sizeof(pgdval_t) > sizeof(long))
-   ret =  PVOP_CALLEE2(pgdval_t, mmu.pgd_val,
-   pgd.pgd, (u64)pgd.pgd >> 32);
-   else
-   ret =  PVOP_CALLEE1(pgdval_t, mmu.pgd_val, pgd.pgd);
-
-   return ret;
+   return PVOP_CALLEE1(pgdval_t, mmu.pgd_val, pgd.pgd);
 }
 
 #define  __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
@@ -435,79 +409,48 @@ static inline pte_t ptep_modify_prot_start(struct 
vm_area_struct *vma, unsigned
 static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, 
unsigned long addr,
   pte_t *ptep, pte_t old_pte, pte_t 
pte)
 {
-
-   if (sizeof(pteval_t) > sizeof(long))
-   /* 5 arg words */
-   pv_ops.mmu.ptep_modify_prot_commit(vma, addr, ptep, pte);
-   else
-   PVOP_VCALL4(mmu.ptep_modify_prot_commit,
-   vma, addr, ptep, pte.pte);
+   PVOP_VCALL4(mmu.ptep_modify_prot_commit, vma, addr, ptep, pte.pte);
 }
 
 static inline void set_pte(pte_t *ptep, pte_t pte)
 {
-   if (sizeof(pteval_t) > sizeof(long))
-   PVOP_VCALL3(mmu.set_pte, ptep, pte.pte, (u64)pte.pte >> 32);
-   else
-   PVOP_VCALL2(mmu.set_pte, ptep, pte.pte);
+   PVOP_VCALL2(mmu.set_pte, ptep, pte.pte);
 }
 
 static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
  pte_t *ptep, pte_t pte)
 {
-   if (sizeof(pteval_t) > sizeof(long))
-   /* 5 arg words */
-   pv_ops.mmu.set_pte_at(mm, addr, ptep, pte);
-   else
-   PVOP_VCALL4(mmu.set_pte_at, mm, addr, ptep, pte.pte);
+   

[Xen-devel] [PATCH 0/2] Remove 32-bit Xen PV guest support

2019-07-15 Thread Juergen Gross
The long term plan has been to replace Xen PV guests by PVH. The first
victim of that plan are now 32-bit PV guests, as those are used only
rather seldom these days. Xen on x86 requires 64-bit support and with
Grub2 now supporting PVH officially since version 2.04 there is no
need to keep 32-bit PV guest support alive in the Linux kernel.
Additionally Meltdown mitigation is not available in the kernel running
as 32-bit PV guest, so dropping this mode makes sense from security
point of view, too.

Juergen Gross (2):
  x86/xen: remove 32-bit Xen PV guest support
  x86/paravirt: remove 32-bit support from PARAVIRT_XXL

 arch/x86/entry/entry_32.S   |  93 
 arch/x86/entry/vdso/vdso32/vclock_gettime.c |   1 +
 arch/x86/include/asm/paravirt.h | 105 +
 arch/x86/include/asm/paravirt_types.h   |  20 --
 arch/x86/include/asm/pgtable-3level_types.h |   5 -
 arch/x86/include/asm/proto.h|   2 +-
 arch/x86/include/asm/segment.h  |   2 +-
 arch/x86/include/asm/traps.h|   2 +-
 arch/x86/kernel/cpu/common.c|   8 -
 arch/x86/kernel/paravirt.c  |  17 --
 arch/x86/kernel/paravirt_patch_32.c |  36 +--
 arch/x86/xen/Kconfig|   3 +-
 arch/x86/xen/Makefile   |   4 +-
 arch/x86/xen/apic.c |  17 --
 arch/x86/xen/enlighten_pv.c |  45 +---
 arch/x86/xen/mmu_pv.c   | 326 +++-
 arch/x86/xen/p2m.c  |   4 -
 arch/x86/xen/setup.c|  44 +---
 arch/x86/xen/smp_pv.c   |  19 +-
 arch/x86/xen/xen-asm.S  |  14 --
 arch/x86/xen/xen-asm_32.S   | 207 --
 arch/x86/xen/xen-head.S |   6 -
 arch/x86/xen/xen-ops.h  |   5 -
 drivers/xen/Kconfig |   4 +-
 24 files changed, 57 insertions(+), 932 deletions(-)
 delete mode 100644 arch/x86/xen/xen-asm_32.S

-- 
2.16.4


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

Re: [Xen-devel] [GSoC-2019] About the crossbar and xen

2019-07-15 Thread Julien Grall

Hi,

On 12/07/2019 19:32, Hunyue Yau z wrote:

On Friday, July 12, 2019 16:13:32 Julien Grall wrote:

Hi,

On 7/11/19 7:29 PM, Hunyue Yau wrote:

[This mail incorporates comments raised on IRC. I have made some of this
mHi,ore verbose to provide context to people that haven't seen the IRC
comments.]

Thank you for the summary!


This will be a bunch of facts on the am5. Someone else will have relate it
back to Xen.

1 - The WUGen is a hardware block on the MPU block that can turn
interrupts
into wake up events if the MPU is in certain deeper sleep states. This
applies only to certain interrupts. We can confirm this by looking at the
DT's register address. Per the TRM, they are registers for the MPU's PRCM
(Power/Reset/Clock Management). In short, this block makes interrupts a
possible wake up source.

2 - Earlier kernels did not expose that HW block. See this patch that
introduced the WUGen -
https://github.com/torvalds/linux/commit/7136d457f365ecc93ddffcdd42ab49a84
73f260b I suspect looking at the before part of the patch should provide
clues on how to handle the WUGen.


3 - This may be redundant but more definitions (in the human sense) here:
https://www.mjmwired.net/kernel/Documentation/devicetree/bindings/interrup
t-controller/ti,omap4-wugen-mpu

4 - For the UART case, I suspect the flow Dennis pointed out is about
right. However, that may be different depending on the interrupt source.

Unknowns from my point of view -

a - Does Xen virtualize power management? If so, this may have have an
impact. I would not recommend adding PM virtualization in GSoC. It is
tugging on a very long string.


Xen does not virtualize power management at the moment. I agree that
this is too big for the scope of the GSoC.


b - If Xen does not virtualize that, someone needs to decide how much to
leave to the guess.

c - I wonder if we can do a half way hack where the kernel sets up the PM
but Xen hooks to get the interrupt. The HW will do its PM thing and Xen
can process the interrupt.


Hmmm, the question here is whether the UART would be usuable before Dom0
is setting up the PM? If not, then, we would need to deal with it in Xen.


Guesses/possible hacks -
- For the interrupts we care about, the cross bar can route things to the
MPU unconditionally. This would break the other HW blocks but most of
them aren't needed for boot.


Sorry for my ignorance, which HW blocks are you talking about?


The HW blocks I am referring to are stuff like EVE, IPU, and DSP. Initially, we
can even ignore the PRUSS. This should leave just sending interrupts to the
MPU. As I understand it, there is no current support for those right now
anyways. I think EVE/IPU/DSP require a working cmem driver which is not fully
upstreamed. BBAI does use them but that requires a specific kernel.

PRUSS would be nice (aka the PRU stuff) eventually as the bits are upstream but
not critical for now.


Thank you for the clarification. My knowledge on the board is limited, so I will 
take yours comments as granted :).


Cheers,





Cheers,




--
Julien Grall

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

[Xen-devel] [xen-4.9-testing test] 138992: regressions - trouble: fail/pass/starved

2019-07-15 Thread osstest service owner
flight 138992 xen-4.9-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/138992/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-prev   6 xen-build  fail in 138772 REGR. vs. 132889
 build-amd64-prev  6 xen-build  fail in 138772 REGR. vs. 132889

Tests which are failing intermittently (not blocking):
 test-amd64-i386-freebsd10-amd64 14 guest-saverestore fail in 138772 pass in 
138992
 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-saverestore fail in 138772 pass 
in 138992
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail in 138919 pass in 
138772
 test-amd64-amd64-xl-qemut-win7-amd64 13 guest-saverestore fail in 138919 pass 
in 138992
 test-amd64-i386-xl-qemut-win7-amd64 13 guest-saverestore fail in 138919 pass 
in 138992
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 17 guest-stop fail in 138919 
pass in 138992
 test-amd64-i386-freebsd10-i386 17 guest-localmigrate/x10 fail in 138951 pass 
in 138992
 test-amd64-i386-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail in 138951 
pass in 138992
 test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail pass in 
138842
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail pass in 
138919
 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail pass in 
138919
 test-amd64-i386-freebsd10-amd64 17 guest-localmigrate/x10  fail pass in 138951

Tests which did not succeed, but are not blocking:
 test-amd64-i386-migrupgrade   1 build-check(1)   blocked in 138772 n/a
 test-amd64-amd64-migrupgrade  1 build-check(1)   blocked in 138772 n/a
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop   fail blocked in 132889
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop  fail blocked in 132889
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop   fail blocked in 132889
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop  fail in 138842 like 132889
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop  fail in 138919 like 132889
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail in 138951 
like 132889
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 132889
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 132889
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-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-amd64-i386-libvirt-xsm  13 migrate-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-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 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-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-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-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 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-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass

Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Paul Durrant
> -Original Message-
> From: Jan Beulich 
> Sent: 15 July 2019 11:44
> To: Paul Durrant 
> Cc: JulienGrall ; Andrew Cooper 
> ; George Dunlap
> ; Ian Jackson ; Roger Pau 
> Monne
> ; VolodymyrBabchuk ; 
> Stefano Stabellini
> ; xen-devel@lists.xenproject.org; Konrad Rzeszutek 
> Wilk
> ; Tamas K Lengyel ; Tim 
> (Xen.org) ; Wei Liu
> 
> Subject: Re: [PATCH v2] xen/mm.h: add helper function to test-and-clear 
> _PGC_allocated
> 
> On 15.07.2019 11:50, Paul Durrant wrote:
> >> From: Jan Beulich 
> >> Sent: 15 July 2019 10:24
> >>
> >> On 15.07.2019 11:17, Paul Durrant wrote:
> >>> The _PGC_allocated flag is set on a page when it is assigned to a domain
> >>> along with an initial reference count of at least 1. To clear this
> >>> 'allocation' reference it is necessary to test-and-clear _PGC_allocated 
> >>> and
> >>> then only drop the reference if the test-and-clear succeeds. This is open-
> >>> coded in many places. It is also unsafe to test-and-clear _PGC_allocated
> >>> unless the caller holds an additional reference.
> >>>
> >>> This patch adds a helper function, put_page_alloc_ref(), to replace all 
> >>> the
> >>> open-coded test-and-clear/put_page occurrences and incorporates in that a
> >>> BUG_ON() an additional page reference not being held.
> >>
> >> This last sentence reads somewhat strange to me - are there words
> >> missing and/or mis-ordered?
> >
> > Perhaps it reads better if 'BUG_ON()' is substituted with 'BUG() on'?
> > I just wanted to express that there was a new check in the helper
> > function that the necessary additional reference is held.
> 
> But then still more like "... incorporates in a BUG() on that an
> additional ..."? At which point it imo could as well be "...
> incorporates in a BUG_ON() that an additional ..." (i.e. just a
> word order change from your original sentence). (There's then
> perhaps also an "is" missing later in the sentence.)
> 
> >>> Signed-off-by: Paul Durrant 
> >>
> >> With the commit message aspect clarified
> >
> > I am happy for you to re-word it if you feel it is not clear.
> 
> Well, the problem is that I don't feel well adjusting what a native
> English speaking person has written.

Ok. How about...

"This patch adds a helper function, put_page_alloc_ref(), to replace all the 
open-coded test-and-clear/put_page occurrences. That helper function 
incorporates a check that an additional page reference is held and will BUG() 
if it is not."

?

  Paul

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

Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Jan Beulich
On 15.07.2019 11:50, Paul Durrant wrote:
>> From: Jan Beulich 
>> Sent: 15 July 2019 10:24
>>
>> On 15.07.2019 11:17, Paul Durrant wrote:
>>> The _PGC_allocated flag is set on a page when it is assigned to a domain
>>> along with an initial reference count of at least 1. To clear this
>>> 'allocation' reference it is necessary to test-and-clear _PGC_allocated and
>>> then only drop the reference if the test-and-clear succeeds. This is open-
>>> coded in many places. It is also unsafe to test-and-clear _PGC_allocated
>>> unless the caller holds an additional reference.
>>>
>>> This patch adds a helper function, put_page_alloc_ref(), to replace all the
>>> open-coded test-and-clear/put_page occurrences and incorporates in that a
>>> BUG_ON() an additional page reference not being held.
>>
>> This last sentence reads somewhat strange to me - are there words
>> missing and/or mis-ordered?
> 
> Perhaps it reads better if 'BUG_ON()' is substituted with 'BUG() on'?
> I just wanted to express that there was a new check in the helper
> function that the necessary additional reference is held.

But then still more like "... incorporates in a BUG() on that an
additional ..."? At which point it imo could as well be "...
incorporates in a BUG_ON() that an additional ..." (i.e. just a
word order change from your original sentence). (There's then
perhaps also an "is" missing later in the sentence.)

>>> Signed-off-by: Paul Durrant 
>>
>> With the commit message aspect clarified
> 
> I am happy for you to re-word it if you feel it is not clear.

Well, the problem is that I don't feel well adjusting what a native
English speaking person has written.

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

Re: [Xen-devel] [PATCH V4 9/9] xen/blkback: use helper in vbd_sz()

2019-07-15 Thread Roger Pau Monné
On Mon, Jul 08, 2019 at 11:47:11AM -0700, Chaitanya Kulkarni wrote:
> This patch updates the vbd_sz() macro with newly introduced helper
> function to read the nr_sects from block device's hd_parts with the
> help of part_nr_sects_read().
> 
> Signed-off-by: Chaitanya Kulkarni 

Acked-by: Roger Pau Monné 

Thanks, Roger.

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

[Xen-devel] [PATCH] x86/dom0-build: fix build with clang5

2019-07-15 Thread Jan Beulich
With non-empty CONFIG_DOM0_MEM clang5 produces

dom0_build.c:344:24: error: use of logical '&&' with constant operand 
[-Werror,-Wconstant-logical-operand]
 if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
^  ~~
dom0_build.c:344:24: note: use '&' for a bitwise operation
 if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
^~
&
dom0_build.c:344:24: note: remove constant to silence this warning
 if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
   ~^
1 error generated.

Obviously neither of the two suggestions are an option here. Oddly
enough swapping the operands of the && helps, while e.g. casting or
parenthesizing doesn't. Another workable variant looks to be the use of
!! on the constant.

Signed-off-by: Jan Beulich 
---
I'm open to going the !! or yet some different route. No matter which
one we choose, I'm afraid it is going to remain guesswork what newer
(and future) versions of clang will choke on.

--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag
  unsigned long avail = 0, nr_pages, min_pages, max_pages;
  bool need_paging;
  
-if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
+if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set )
  parse_dom0_mem(CONFIG_DOM0_MEM);
  
  for_each_node_mask ( node, dom0_nodes )
x86/dom0-build: fix build with clang5

With non-empty CONFIG_DOM0_MEM clang5 produces

dom0_build.c:344:24: error: use of logical '&&' with constant operand 
[-Werror,-Wconstant-logical-operand]
if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
   ^  ~~
dom0_build.c:344:24: note: use '&' for a bitwise operation
if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
   ^~
   &
dom0_build.c:344:24: note: remove constant to silence this warning
if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
  ~^
1 error generated.

Obviously neither of the two suggestions are an option here. Oddly
enough swapping the operands of the && helps, while e.g. casting or
parenthesizing doesn't. Another workable variant looks to be the use of
!! on the constant.

Signed-off-by: Jan Beulich 
---
I'm open to going the !! or yet some different route. No matter which
one we choose, I'm afraid it is going to remain guesswork what newer
(and future) versions of clang will choke on.

--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -341,7 +341,7 @@ unsigned long __init dom0_compute_nr_pag
 unsigned long avail = 0, nr_pages, min_pages, max_pages;
 bool need_paging;
 
-if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
+if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set )
 parse_dom0_mem(CONFIG_DOM0_MEM);
 
 for_each_node_mask ( node, dom0_nodes )
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Ping: [PATCH v2] timers: limit heap size

2019-07-15 Thread Roger Pau Monné
On Fri, Jul 05, 2019 at 04:06:06PM +, Jan Beulich wrote:
> >>> On 05.06.19 at 08:51,  wrote:
> > First and foremost make timer_softirq_action() avoid growing the heap
> > if its new size can't be stored without truncation. 64k entries is a
> > lot, and I don't think we're at risk of actually running into the issue,
> > but I also think it's better not to allow for hard to debug problems to
> > occur in the first place.
> > 
> > Furthermore also adjust the code such the size/limit fields becoming
> > unsigned int would at least work from a mere sizing point of view. For
> > this also switch various uses of plain int to unsigned int.
> > 
> > Signed-off-by: Jan Beulich 

Thanks:

Reviewed-by: Roger Pau Monné 

I however wonder whether all this heap timer management plus the extra
list is really the best option, using a balanced tree seems like a
better option here.

Roger.

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

Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Paul Durrant
> -Original Message-
> From: Jan Beulich 
> Sent: 15 July 2019 10:24
> To: Paul Durrant 
> Cc: xen-devel@lists.xenproject.org; Julien Grall ; 
> Andrew Cooper
> ; Roger Pau Monne ; 
> VolodymyrBabchuk
> ; George Dunlap ; Ian 
> Jackson
> ; Stefano Stabellini ; Konrad 
> Rzeszutek Wilk
> ; Tamas K Lengyel ; Tim 
> (Xen.org) ; Wei Liu
> 
> Subject: Re: [PATCH v2] xen/mm.h: add helper function to test-and-clear 
> _PGC_allocated
> 
> On 15.07.2019 11:17, Paul Durrant wrote:
> > The _PGC_allocated flag is set on a page when it is assigned to a domain
> > along with an initial reference count of at least 1. To clear this
> > 'allocation' reference it is necessary to test-and-clear _PGC_allocated and
> > then only drop the reference if the test-and-clear succeeds. This is open-
> > coded in many places. It is also unsafe to test-and-clear _PGC_allocated
> > unless the caller holds an additional reference.
> >
> > This patch adds a helper function, put_page_alloc_ref(), to replace all the
> > open-coded test-and-clear/put_page occurrences and incorporates in that a
> > BUG_ON() an additional page reference not being held.
> 
> This last sentence reads somewhat strange to me - are there words
> missing and/or mis-ordered?

Perhaps it reads better if 'BUG_ON()' is substituted with 'BUG() on'? I just 
wanted to express that there was a new check in the helper function that the 
necessary additional reference is held.

> 
> > Signed-off-by: Paul Durrant 
> 
> With the commit message aspect clarified

I am happy for you to re-word it if you feel it is not clear. With the extra 
comment in the helper function in v2 then perhaps it is not really necessary to 
have any additional explanation in the commit comment anyway?

> Acked-by: Jan Beulich 

Thanks,

  Paul

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

Re: [Xen-devel] Notes from summit design session on branch management

2019-07-15 Thread George Dunlap
On Fri, Jul 12, 2019 at 10:41 PM Ian Jackson  wrote:
>
> Here are the photos I took of the flipchart:
>   https://xenbits.xen.org/people/iwj/2019/summit-ci-branch-workshop/

FYI, I can see the directory, but when I click on the individual
images, I get this:

You don't have permission to access
/people/iwj/2019/summit-ci-branch-workshop/IMG_20190711_115507.jpg on
this server.

 -George

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

Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Jan Beulich
On 15.07.2019 11:17, Paul Durrant wrote:
> The _PGC_allocated flag is set on a page when it is assigned to a domain
> along with an initial reference count of at least 1. To clear this
> 'allocation' reference it is necessary to test-and-clear _PGC_allocated and
> then only drop the reference if the test-and-clear succeeds. This is open-
> coded in many places. It is also unsafe to test-and-clear _PGC_allocated
> unless the caller holds an additional reference.
> 
> This patch adds a helper function, put_page_alloc_ref(), to replace all the
> open-coded test-and-clear/put_page occurrences and incorporates in that a
> BUG_ON() an additional page reference not being held.

This last sentence reads somewhat strange to me - are there words
missing and/or mis-ordered?

> Signed-off-by: Paul Durrant 

With the commit message aspect clarified
Acked-by: Jan Beulich 

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

Re: [Xen-devel] [PATCH] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Paul Durrant
> -Original Message-
> From: Jan Beulich 
> Sent: 15 July 2019 10:18
> To: Paul Durrant 
> Cc: JulienGrall ; Andrew Cooper 
> ; George Dunlap
> ; Ian Jackson ; Roger Pau 
> Monne
> ; Volodymyr Babchuk ; 
> Stefano Stabellini
> ; xen-devel@lists.xenproject.org; Konrad Rzeszutek 
> Wilk
> ; Tamas K Lengyel ; Tim 
> (Xen.org) ; Wei Liu
> 
> Subject: Re: [Xen-devel] [PATCH] xen/mm.h: add helper function to 
> test-and-clear _PGC_allocated
> 
> On 15.07.2019 10:45, Paul Durrant wrote:
> >> From: Jan Beulich 
> >> Sent: 10 July 2019 23:53
> >>
> >> On 10.07.2019 18:17, Paul Durrant wrote:
> >>> @@ -418,13 +417,7 @@ static void hvm_free_ioreq_mfn(struct 
> >>> hvm_ioreq_server *s, bool buf)
> >>>unmap_domain_page_global(iorp->va);
> >>>iorp->va = NULL;
> >>>
> >>> -/*
> >>> - * Check whether we need to clear the allocation reference before
> >>> - * dropping the explicit references taken by get_page_and_type().
> >>> - */
> >>> -if ( test_and_clear_bit(_PGC_allocated, >count_info) )
> >>> -put_page(page);
> >>> -
> >>> +clear_assignment_reference(page);
> >>>put_page_and_type(page);
> >>>}
> >>
> >> Is there a specific reason you drop the comment? It doesn't become
> >> less relevant than when it was added, does it?
> >
> > Not sure, since what's actually going on is now internal to the function.
> > If I change the function name to clear_allocation_reference() then I
> > think the comment probably becomes extraneous.
> 
> Well, the perspective I'm taking is that the ordering constraint
> wrt put_page_and_type() doesn't go away and is a relevant part of
> what the comment talks about.

Ok. Would you be happy fixing the comment to your taste on commit then, as I'm 
not sure exactly what you want to say?

  Paul

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

Re: [Xen-devel] [PATCH] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Jan Beulich
On 15.07.2019 10:45, Paul Durrant wrote:
>> From: Jan Beulich 
>> Sent: 10 July 2019 23:53
>>
>> On 10.07.2019 18:17, Paul Durrant wrote:
>>> @@ -418,13 +417,7 @@ static void hvm_free_ioreq_mfn(struct hvm_ioreq_server 
>>> *s, bool buf)
>>>unmap_domain_page_global(iorp->va);
>>>iorp->va = NULL;
>>>
>>> -/*
>>> - * Check whether we need to clear the allocation reference before
>>> - * dropping the explicit references taken by get_page_and_type().
>>> - */
>>> -if ( test_and_clear_bit(_PGC_allocated, >count_info) )
>>> -put_page(page);
>>> -
>>> +clear_assignment_reference(page);
>>>put_page_and_type(page);
>>>}
>>
>> Is there a specific reason you drop the comment? It doesn't become
>> less relevant than when it was added, does it?
> 
> Not sure, since what's actually going on is now internal to the function.
> If I change the function name to clear_allocation_reference() then I
> think the comment probably becomes extraneous.

Well, the perspective I'm taking is that the ordering constraint
wrt put_page_and_type() doesn't go away and is a relevant part of
what the comment talks about.

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

[Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Paul Durrant
The _PGC_allocated flag is set on a page when it is assigned to a domain
along with an initial reference count of at least 1. To clear this
'allocation' reference it is necessary to test-and-clear _PGC_allocated and
then only drop the reference if the test-and-clear succeeds. This is open-
coded in many places. It is also unsafe to test-and-clear _PGC_allocated
unless the caller holds an additional reference.

This patch adds a helper function, put_page_alloc_ref(), to replace all the
open-coded test-and-clear/put_page occurrences and incorporates in that a
BUG_ON() an additional page reference not being held.

Signed-off-by: Paul Durrant 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Volodymyr Babchuk 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Tim Deegan 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 
Cc: Tamas K Lengyel 
Cc: George Dunlap 

v2:
 - Re-name clear_assignment_reference() to put_page_alloc_ref()
 - Swap ASSERT() for BUG_ON()
 - Add an extra comment explaining what put_page_alloc_ref() is doing
---
 xen/arch/arm/domain.c |  4 +---
 xen/arch/x86/domain.c |  3 +--
 xen/arch/x86/hvm/ioreq.c  | 11 ++-
 xen/arch/x86/mm.c |  3 +--
 xen/arch/x86/mm/mem_sharing.c |  9 +++--
 xen/arch/x86/mm/p2m-pod.c |  4 +---
 xen/arch/x86/mm/p2m.c |  3 +--
 xen/common/grant_table.c  |  3 +--
 xen/common/memory.c   |  5 ++---
 xen/common/xenoprof.c |  3 +--
 xen/include/xen/mm.h  | 14 ++
 11 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 4f44d5c742..941bbff4fe 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -926,9 +926,7 @@ static int relinquish_memory(struct domain *d, struct 
page_list_head *list)
  */
 continue;
 
-if ( test_and_clear_bit(_PGC_allocated, >count_info) )
-put_page(page);
-
+put_page_alloc_ref(page);
 put_page(page);
 
 if ( hypercall_preempt_check() )
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 147f96a09e..e791d86892 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1939,8 +1939,7 @@ static int relinquish_memory(
 BUG();
 }
 
-if ( test_and_clear_bit(_PGC_allocated, >count_info) )
-put_page(page);
+put_page_alloc_ref(page);
 
 /*
  * Forcibly invalidate top-most, still valid page tables at this point
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 7a80cfb28b..a79cabb680 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -398,8 +398,7 @@ static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, 
bool buf)
 return 0;
 
  fail:
-if ( test_and_clear_bit(_PGC_allocated, >count_info) )
-put_page(page);
+put_page_alloc_ref(page);
 put_page_and_type(page);
 
 return -ENOMEM;
@@ -418,13 +417,7 @@ static void hvm_free_ioreq_mfn(struct hvm_ioreq_server *s, 
bool buf)
 unmap_domain_page_global(iorp->va);
 iorp->va = NULL;
 
-/*
- * Check whether we need to clear the allocation reference before
- * dropping the explicit references taken by get_page_and_type().
- */
-if ( test_and_clear_bit(_PGC_allocated, >count_info) )
-put_page(page);
-
+put_page_alloc_ref(page);
 put_page_and_type(page);
 }
 
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index df2c0130f1..138662e777 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -498,8 +498,7 @@ void share_xen_page_with_guest(struct page_info *page, 
struct domain *d,
 
 void free_shared_domheap_page(struct page_info *page)
 {
-if ( test_and_clear_bit(_PGC_allocated, >count_info) )
-put_page(page);
+put_page_alloc_ref(page);
 if ( !test_and_clear_bit(_PGC_xen_heap, >count_info) )
 ASSERT_UNREACHABLE();
 page->u.inuse.type_info = 0;
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index f16a3f5324..58d9157fa8 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1000,8 +1000,7 @@ static int share_pages(struct domain *sd, gfn_t sgfn, 
shr_handle_t sh,
 mem_sharing_page_unlock(firstpg);
 
 /* Free the client page */
-if(test_and_clear_bit(_PGC_allocated, >count_info))
-put_page(cpage);
+put_page_alloc_ref(cpage);
 put_page(cpage);
 
 /* We managed to free a domain page. */
@@ -1082,8 +1081,7 @@ int mem_sharing_add_to_physmap(struct domain *sd, 
unsigned long sgfn, shr_handle
 ret = -EOVERFLOW;
 goto err_unlock;
 }
-if ( test_and_clear_bit(_PGC_allocated, >count_info) )
-put_page(cpage);
+put_page_alloc_ref(cpage);
 put_page(cpage);
 }
 }
@@ -1177,8 +1175,7 @@ int 

[Xen-devel] [PATCH v3] xen/pv: Fix a boot up hang revealed by int3 self test

2019-07-15 Thread Zhenzhong Duan
Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call()
selftest") is used to ensure there is a gap setup in int3 exception stack
which could be used for inserting call return address.

This gap is missed in XEN PV int3 exception entry path, then below panic
triggered:

[0.772876] general protection fault:  [#1] SMP NOPTI
[0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11
[0.772893] RIP: e030:int3_magic+0x0/0x7
[0.772905] RSP: 3507:82203e98 EFLAGS: 0246
[0.773334] Call Trace:
[0.773334]  alternative_instructions+0x3d/0x12e
[0.773334]  check_bugs+0x7c9/0x887
[0.773334]  ? __get_locked_pte+0x178/0x1f0
[0.773334]  start_kernel+0x4ff/0x535
[0.773334]  ? set_init_arg+0x55/0x55
[0.773334]  xen_start_kernel+0x571/0x57a

For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with
%rcx/%r11 on the stack. To convert back to "normal" looking exceptions,
the xen thunks do 'xen_*: pop %rcx; pop %r11; jmp *'.

E.g. Extracting 'xen_pv_trap xenint3' we have:
xen_xenint3:
 pop %rcx;
 pop %r11;
 jmp xenint3

As xenint3 and int3 entry code are same except xenint3 doesn't generate
a gap, we can fix it by using int3 and drop useless xenint3.

Signed-off-by: Zhenzhong Duan 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: Stefano Stabellini 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Andrew Cooper 
---
 bootup test pass with PV guest.

 v3: set ist_okay to false for int3 per PeterZ
 add Andrew's comments to patch description

 v2: fix up description.
---
 arch/x86/entry/entry_64.S| 1 -
 arch/x86/include/asm/traps.h | 2 +-
 arch/x86/xen/enlighten_pv.c  | 2 +-
 arch/x86/xen/xen-asm_64.S| 1 -
 4 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 0ea4831..35a66fc 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1176,7 +1176,6 @@ idtentry stack_segmentdo_stack_segment
has_error_code=1
 #ifdef CONFIG_XEN_PV
 idtentry xennmido_nmi  has_error_code=0
 idtentry xendebug  do_debughas_error_code=0
-idtentry xenint3   do_int3 has_error_code=0
 #endif
 
 idtentry general_protectiondo_general_protection   has_error_code=1
diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 7d6f3f3..f2bd284 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -40,7 +40,7 @@
 asmlinkage void xen_divide_error(void);
 asmlinkage void xen_xennmi(void);
 asmlinkage void xen_xendebug(void);
-asmlinkage void xen_xenint3(void);
+asmlinkage void xen_int3(void);
 asmlinkage void xen_overflow(void);
 asmlinkage void xen_bounds(void);
 asmlinkage void xen_invalid_op(void);
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 4722ba2..30c14cb 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -596,12 +596,12 @@ struct trap_array_entry {
 
 static struct trap_array_entry trap_array[] = {
{ debug,   xen_xendebug,true },
-   { int3,xen_xenint3, true },
{ double_fault,xen_double_fault,true },
 #ifdef CONFIG_X86_MCE
{ machine_check,   xen_machine_check,   true },
 #endif
{ nmi, xen_xennmi,  true },
+   { int3,xen_int3,false },
{ overflow,xen_overflow,false },
 #ifdef CONFIG_IA32_EMULATION
{ entry_INT80_compat,  xen_entry_INT80_compat,  false },
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index 1e9ef0b..ebf610b 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -32,7 +32,6 @@ xen_pv_trap divide_error
 xen_pv_trap debug
 xen_pv_trap xendebug
 xen_pv_trap int3
-xen_pv_trap xenint3
 xen_pv_trap xennmi
 xen_pv_trap overflow
 xen_pv_trap bounds
-- 
1.8.3.1


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

Re: [Xen-devel] [PATCH v2] xen/pv: Fix a boot up hang revealed by int3 self test

2019-07-15 Thread Andrew Cooper
On 15/07/2019 07:54, Jan Beulich wrote:
> On 15.07.2019 07:05, Zhenzhong Duan wrote:
>> On 2019/7/12 22:06, Andrew Cooper wrote:
>>> On 11/07/2019 03:15, Zhenzhong Duan wrote:
 Commit 7457c0da024b ("x86/alternatives: Add int3_emulate_call()
 selftest") is used to ensure there is a gap setup in exception stack
 which could be used for inserting call return address.

 This gap is missed in XEN PV int3 exception entry path, then below panic
 triggered:

 [    0.772876] general protection fault:  [#1] SMP NOPTI
 [    0.772886] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0+ #11
 [    0.772893] RIP: e030:int3_magic+0x0/0x7
 [    0.772905] RSP: 3507:82203e98 EFLAGS: 0246
 [    0.773334] Call Trace:
 [    0.773334]  alternative_instructions+0x3d/0x12e
 [    0.773334]  check_bugs+0x7c9/0x887
 [    0.773334]  ? __get_locked_pte+0x178/0x1f0
 [    0.773334]  start_kernel+0x4ff/0x535
 [    0.773334]  ? set_init_arg+0x55/0x55
 [    0.773334]  xen_start_kernel+0x571/0x57a

 As xenint3 and int3 entry code are same except xenint3 doesn't generate
 a gap, we can fix it by using int3 and drop useless xenint3.
>>> For 64bit PV guests, Xen's ABI enters the kernel with using SYSRET, with
>>> %rcx/%r11 on the stack.
>>>
>>> To convert back to "normal" looking exceptions, the xen thunks do `pop
>>> %rcx; pop %r11; jmp do_*`...
>> I will add this to commit message.
 diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
 index 0ea4831..35a66fc 100644
 --- a/arch/x86/entry/entry_64.S
 +++ b/arch/x86/entry/entry_64.S
 @@ -1176,7 +1176,6 @@ idtentry stack_segment    do_stack_segment    
 has_error_code=1
   #ifdef CONFIG_XEN_PV
   idtentry xennmi    do_nmi    has_error_code=0
   idtentry xendebug    do_debug    has_error_code=0
 -idtentry xenint3    do_int3    has_error_code=0
   #endif
>>> What is confusing is why there are 3 extra magic versions here.  At a
>>> guess, I'd say something to do with IST settings (given the vectors),
>>> but I don't see why #DB/#BP would need to be different in the first
>>> place.  (NMI sure, but that is more to do with the crazy hoops needing
>>> to be jumped through to keep native functioning safely.)
>> xenint3 will be removed in this patch safely as it don't use IST now.
>>
>> But debug and nmi need paranoid_entry which will read MSR_GS_BASE to 
>> determine
>>
>> if swapgs is needed. I guess PV guesting running in ring3 will #GP with 
>> swapgs?
> Not only that (Xen could trap and emulate swapgs if that was needed) - 64-bit
> PV kernel mode always gets entered with kernel GS base already set. Hence
> finding out whether to switch the GS base is specifically not something that
> any exception entry point would need to do (and it should actively try to
> avoid it, for performance reasons).

Indeed.  The SWAPGS PVOP is implemented as a nop for x86 PV, to simply
the entry assembly (rather than doubling up all entry vectors).

~Andrew

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

[Xen-devel] Ping²: [PATCH 4/4] x86/PV: remove unnecessary toggle_guest_pt() overhead

2019-07-15 Thread Jan Beulich
>>> On 27.05.19 at 11:25,  wrote:
 On 13.03.19 at 13:39,  wrote:
> > While the mere updating of ->pv_cr3 and ->root_pgt_changed aren't overly
> > expensive (but still needed only for the toggle_guest_mode() path), the
> > effect of the latter on the exit-to-guest path is not insignificant.
> > Move the logic into toggle_guest_mode().
> > 
> > Signed-off-by: Jan Beulich 
> 
> I think I did address the one concern you had.
> 
> Jan

https://lists.xenproject.org/archives/html/xen-devel/2019-04/msg00306.html
Ping?

> > --- a/xen/arch/x86/pv/domain.c
> > +++ b/xen/arch/x86/pv/domain.c
> > @@ -349,18 +349,10 @@ bool __init xpti_pcid_enabled(void)
> >  
> >  static void _toggle_guest_pt(struct vcpu *v)
> >  {
> > -const struct domain *d = v->domain;
> > -struct cpu_info *cpu_info = get_cpu_info();
> >  unsigned long cr3;
> >  
> >  v->arch.flags ^= TF_kernel_mode;
> >  update_cr3(v);
> > -if ( d->arch.pv.xpti )
> > -{
> > -cpu_info->root_pgt_changed = true;
> > -cpu_info->pv_cr3 = __pa(this_cpu(root_pgt)) |
> > -   (d->arch.pv.pcid ? get_pcid_bits(v, true) : 0);
> > -}
> >  
> >  /*
> >   * Don't flush user global mappings from the TLB. Don't tick TLB clock.
> > @@ -368,15 +360,11 @@ static void _toggle_guest_pt(struct vcpu
> >   * In shadow mode, though, update_cr3() may need to be accompanied by a
> >   * TLB flush (for just the incoming PCID), as the top level page table 
> > may
> >   * have changed behind our backs. To be on the safe side, suppress the
> > - * no-flush unconditionally in this case. The XPTI CR3 write, if 
> > enabled,
> > - * will then need to be a flushing one too.
> > + * no-flush unconditionally in this case.
> >   */
> >  cr3 = v->arch.cr3;
> > -if ( shadow_mode_enabled(d) )
> > -{
> > +if ( shadow_mode_enabled(v->domain) )
> >  cr3 &= ~X86_CR3_NOFLUSH;
> > -cpu_info->pv_cr3 &= ~X86_CR3_NOFLUSH;
> > -}
> >  write_cr3(cr3);
> >  
> >  if ( !(v->arch.flags & TF_kernel_mode) )
> > @@ -392,6 +380,8 @@ static void _toggle_guest_pt(struct vcpu
> >  
> >  void toggle_guest_mode(struct vcpu *v)
> >  {
> > +const struct domain *d = v->domain;
> > +
> >  ASSERT(!is_pv_32bit_vcpu(v));
> >  
> >  /* %fs/%gs bases can only be stale if WR{FS,GS}BASE are usable. */
> > @@ -405,6 +395,21 @@ void toggle_guest_mode(struct vcpu *v)
> >  asm volatile ( "swapgs" );
> >  
> >  _toggle_guest_pt(v);
> > +
> > +if ( d->arch.pv.xpti )
> > +{
> > +struct cpu_info *cpu_info = get_cpu_info();
> > +
> > +cpu_info->root_pgt_changed = true;
> > +cpu_info->pv_cr3 = __pa(this_cpu(root_pgt)) |
> > +   (d->arch.pv.pcid ? get_pcid_bits(v, true) : 0);
> > +/*
> > + * As in _toggle_guest_pt() the XPTI CR3 write needs to be a TLB-
> > + * flushing one too for shadow mode guests.
> > + */
> > +if ( shadow_mode_enabled(d) )
> > +cpu_info->pv_cr3 &= ~X86_CR3_NOFLUSH;
> > +}
> >  }
> >  
> >  void toggle_guest_pt(struct vcpu *v)
> > 
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] Ping: [PATCH v2] x86/HVM: p2m_ram_ro is incompatible with device pass-through

2019-07-15 Thread Jan Beulich
On 03.07.2019 13:36, Jan Beulich wrote:
> The write-discard property of the type can't be represented in IOMMU
> page table entries. Make sure the respective checks / tracking can't
> race, by utilizing the domain lock. The other sides of the sharing/
> paging/log-dirty exclusion checks should subsequently perhaps also be
> put under that lock then.
> 
> Take the opportunity and also convert neighboring bool_t to bool in
> struct hvm_domain.
> 
> Signed-off-by: Jan Beulich 

Alongside Paul's R-b could I get an ack or otherwise from you?

Thanks, Jan

> ---
> v2: Don't set p2m_ram_ro_used when failing the request.
> 
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -255,16 +255,33 @@ static int set_mem_type(struct domain *d
>
>mem_type = array_index_nospec(data->mem_type, ARRAY_SIZE(memtype));
>
> -if ( mem_type == HVMMEM_ioreq_server )
> +switch ( mem_type )
>{
>unsigned int flags;
>
> +case HVMMEM_ioreq_server:
>if ( !hap_enabled(d) )
>return -EOPNOTSUPP;
>
>/* Do not change to HVMMEM_ioreq_server if no ioreq server mapped. 
> */
>if ( !p2m_get_ioreq_server(d, ) )
>return -EINVAL;
> +
> +break;
> +
> +case HVMMEM_ram_ro:
> +/* p2m_ram_ro can't be represented in IOMMU mappings. */
> +domain_lock(d);
> +if ( has_iommu_pt(d) )
> +rc = -EXDEV;
> +else
> +d->arch.hvm.p2m_ram_ro_used = true;
> +domain_unlock(d);
> +
> +if ( rc )
> +return rc;
> +
> +break;
>}
>
>while ( iter < data->nr )
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1448,17 +1448,36 @@ static int assign_device(struct domain *
>if ( !iommu_enabled || !hd->platform_ops )
>return 0;
>
> -/* Prevent device assign if mem paging or mem sharing have been
> - * enabled for this domain */
> -if ( unlikely(d->arch.hvm.mem_sharing_enabled ||
> -  vm_event_check_ring(d->vm_event_paging) ||
> +domain_lock(d);
> +
> +/*
> + * Prevent device assignment if any of
> + * - mem paging
> + * - mem sharing
> + * - the p2m_ram_ro type
> + * - global log-dirty mode
> + * are in use by this domain.
> + */
> +if ( unlikely(vm_event_check_ring(d->vm_event_paging) ||
> +#ifdef CONFIG_HVM
> +  (is_hvm_domain(d) &&
> +   (d->arch.hvm.mem_sharing_enabled ||
> +d->arch.hvm.p2m_ram_ro_used)) ||
> +#endif
>  p2m_get_hostp2m(d)->global_logdirty) )
> +{
> +domain_unlock(d);
>return -EXDEV;
> +}
>
>if ( !pcidevs_trylock() )
> +{
> +domain_unlock(d);
>return -ERESTART;
> +}
>
>rc = iommu_construct(d);
> +domain_unlock(d);
>if ( rc )
>{
>pcidevs_unlock();
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -156,10 +156,11 @@ struct hvm_domain {
>
>struct viridian_domain *viridian;
>
> -bool_t hap_enabled;
> -bool_t mem_sharing_enabled;
> -bool_t qemu_mapcache_invalidate;
> -bool_t is_s3_suspended;
> +bool   hap_enabled;
> +bool   mem_sharing_enabled;
> +bool   p2m_ram_ro_used;
> +bool   qemu_mapcache_invalidate;
> +bool   is_s3_suspended;
>
>/*
> * TSC value that VCPUs use to calculate their tsc_offset value.
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
> 

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

[Xen-devel] Ping³: [PATCH 1/2] core-parking: interact with runtime SMT-disabling

2019-07-15 Thread Jan Beulich
On 03.07.2019 13:29, Jan Beulich wrote:
 On 27.05.19 at 11:36,  wrote:
> On 12.04.19 at 13:41,  wrote:
>> On 11.04.19 at 21:06,  wrote:
 On 11/04/2019 13:45, Jan Beulich wrote:
> When disabling SMT at runtime, secondary threads should no longer be
> candidates for bringing back up in response to _PUD ACPI events. Purge
> them from the tracking array.
>
> Doing so involves adding locking to guard accounting data in the core
> parking code. While adding the declaration for the lock take the liberty
> to drop two unnecessary forward function declarations.
>
> Signed-off-by: Jan Beulich 

 I can certainly appreciate these arguments, but surely the converse is
 true.  When SMT-enable is used, the newly-onlined threads are now
 eligible to be parked.
>>>
>>> And nothing will keep them from getting parked.
>>>
 At the moment, this looks asymetric.
>>>
>>> It does, but that's a result of core_parking.c only recording CPUs
>>> it has parked, not ones it could park.
>>
>> Did my responses address your concerns?
>>
>> Jan

Ping?

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

Re: [Xen-devel] [PATCH v7] x86/emulate: Send vm_event from emulate

2019-07-15 Thread Alexandru Stefan ISAILA


On 12.07.2019 04:28, Jan Beulich wrote:
> On 11.07.2019 19:13, Tamas K Lengyel wrote:
>>> @@ -629,6 +697,14 @@ static void *hvmemul_map_linear_addr(
>>>
>>>ASSERT(p2mt == p2m_ram_logdirty || !p2m_is_readonly(p2mt));
>>>}
>>> +
>>> +if ( curr->arch.vm_event &&
>>> +curr->arch.vm_event->send_event &&
>>
>> Why not fold these checks into hvm_emulate_send_vm_event since..
> 
> I had asked for at least the first of the checks to be pulled
> out of the function, for the common case to be affected as
> little as possible.
> 
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -3224,6 +3224,14 @@ static enum hvm_translation_result __hvm_copy(
>>>return HVMTRANS_bad_gfn_to_mfn;
>>>}
>>>
>>> +if ( unlikely(v->arch.vm_event) &&
>>> +v->arch.vm_event->send_event &&
>>
>> .. you seem to just repeat them here again?
> 
> I agree that the duplication makes no sense.
> 

The first check is on the hvmemul_map_linear_addr() path and the second 
is on hvmemul_insn_fetch() path. There are 2 distinct ways to reach that 
if and therefore the check is not duplicated.

Thanks,
Alex
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/hvm: make hvmemul_virtual_to_linear()'s reps parameter optional

2019-07-15 Thread Paul Durrant
> -Original Message-
> From: Jan Beulich 
> Sent: 01 July 2019 13:00
> To: xen-devel@lists.xenproject.org; xen-devel@lists.xenproject.org
> Cc: Andrew Cooper ; Wei Liu ; Roger 
> Pau Monne
> ; Paul Durrant 
> Subject: [PATCH] x86/hvm: make hvmemul_virtual_to_linear()'s reps parameter 
> optional
> 
> A majority of callers wants just a single iteration handled. Allow to
> express this by passing in a NULL pointer, instead of setting up a local
> variable just to hold the "1" to pass in here.
> 
> Signed-off-by: Jan Beulich 

Reviewed-by: Paul Durrant 

> ---
> Note that this conflicts with additions/changes made by "x86emul:
> further work". Whatever goes in later will need re-basing.
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -788,14 +788,14 @@ static int hvmemul_virtual_to_linear(
>   enum x86_segment seg,
>   unsigned long offset,
>   unsigned int bytes_per_rep,
> -unsigned long *reps,
> +unsigned long *reps_p,
>   enum hvm_access_type access_type,
>   struct hvm_emulate_ctxt *hvmemul_ctxt,
>   unsigned long *linear)
>   {
>   struct segment_register *reg;
>   int okay;
> -unsigned long max_reps = 4096;
> +unsigned long reps = 1;
> 
>   if ( seg == x86_seg_none )
>   {
> @@ -803,62 +803,72 @@ static int hvmemul_virtual_to_linear(
>   return X86EMUL_OKAY;
>   }
> 
> -/*
> - * If introspection has been enabled for this domain, and we're emulating
> - * becase a vm_reply asked us to (i.e. not doing regular IO) reps should
> - * be at most 1, since optimization might otherwise cause a single
> - * vm_event being triggered for repeated writes to a whole page.
> - */
> -if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) &&
> - current->arch.vm_event->emulate_flags != 0 )
> -   max_reps = 1;
> +if ( reps_p )
> +{
> +unsigned long max_reps = 4096;
> 
> -/*
> - * Clip repetitions to avoid overflow when multiplying by @bytes_per_rep.
> - * The chosen maximum is very conservative but it's what we use in
> - * hvmemul_linear_to_phys() so there is no point in using a larger value.
> - */
> -*reps = min_t(unsigned long, *reps, max_reps);
> +/*
> + * If introspection has been enabled for this domain, and we're
> + * emulating because a vm_reply asked us to (i.e. not doing regular 
> IO)
> + * reps should be at most 1, since optimization might otherwise 
> cause a
> + * single vm_event being triggered for repeated writes to a whole 
> page.
> + */
> +if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) &&
> + current->arch.vm_event->emulate_flags != 0 )
> +   max_reps = 1;
> +
> +/*
> + * Clip repetitions to avoid overflow when multiplying by
> + * @bytes_per_rep. The chosen maximum is very conservative but it's
> + * what we use in hvmemul_linear_to_phys() so there is no point in
> + * using a larger value.
> + */
> +reps = *reps_p = min_t(unsigned long, *reps_p, max_reps);
> +}
> 
>   reg = hvmemul_get_seg_reg(seg, hvmemul_ctxt);
>   if ( IS_ERR(reg) )
>   return -PTR_ERR(reg);
> 
> -if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1) )
> +if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (reps > 1) )
>   {
>   /*
>* x86_emulate() clips the repetition count to ensure we don't wrap
>* the effective-address index register. Hence this assertion holds.
>*/
> -ASSERT(offset >= ((*reps - 1) * bytes_per_rep));
> +ASSERT(offset >= ((reps - 1) * bytes_per_rep));
>   okay = hvm_virtual_to_linear_addr(
> -seg, reg, offset - (*reps - 1) * bytes_per_rep,
> -*reps * bytes_per_rep, access_type,
> +seg, reg, offset - (reps - 1) * bytes_per_rep,
> +reps * bytes_per_rep, access_type,
>   hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt), linear);
> -*linear += (*reps - 1) * bytes_per_rep;
> +*linear += (reps - 1) * bytes_per_rep;
>   if ( hvmemul_ctxt->ctxt.addr_size != 64 )
>   *linear = (uint32_t)*linear;
>   }
>   else
>   {
>   okay = hvm_virtual_to_linear_addr(
> -seg, reg, offset, *reps * bytes_per_rep, access_type,
> +seg, reg, offset, reps * bytes_per_rep, access_type,
>   hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt), linear);
>   }
> 
>   if ( okay )
>   return X86EMUL_OKAY;
> 
> -/* If this is a string operation, emulate each iteration separately. */
> -if ( *reps != 1 )
> -return X86EMUL_UNHANDLEABLE;
> +if ( reps_p )
> +{
> +/* If this is a string operation, emulate each iteration separately. 
> */
> +if ( reps != 1 )
> +return 

[Xen-devel] Ping²: [PATCH v2] timers: limit heap size

2019-07-15 Thread Jan Beulich
On 05.07.2019 18:06, Jan Beulich wrote:
 On 05.06.19 at 08:51,  wrote:
>> First and foremost make timer_softirq_action() avoid growing the heap
>> if its new size can't be stored without truncation. 64k entries is a
>> lot, and I don't think we're at risk of actually running into the issue,
>> but I also think it's better not to allow for hard to debug problems to
>> occur in the first place.
>>
>> Furthermore also adjust the code such the size/limit fields becoming
>> unsigned int would at least work from a mere sizing point of view. For
>> this also switch various uses of plain int to unsigned int.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> v2: Log (once) when heap limit would have been exceeded.
>>
>> --- a/xen/common/timer.c
>> +++ b/xen/common/timer.c
>> @@ -63,9 +63,9 @@ static struct heap_metadata *heap_metada
>>   }
>>   
>>   /* Sink down element @pos of @heap. */
>> -static void down_heap(struct timer **heap, int pos)
>> +static void down_heap(struct timer **heap, unsigned int pos)
>>   {
>> -int sz = heap_metadata(heap)->size, nxt;
>> +unsigned int sz = heap_metadata(heap)->size, nxt;
>>   struct timer *t = heap[pos];
>>   
>>   while ( (nxt = (pos << 1)) <= sz )
>> @@ -84,7 +84,7 @@ static void down_heap(struct timer **hea
>>   }
>>   
>>   /* Float element @pos up @heap. */
>> -static void up_heap(struct timer **heap, int pos)
>> +static void up_heap(struct timer **heap, unsigned int pos)
>>   {
>>   struct timer *t = heap[pos];
>>   
>> @@ -103,8 +103,8 @@ static void up_heap(struct timer **heap,
>>   /* Delete @t from @heap. Return TRUE if new top of heap. */
>>   static int remove_from_heap(struct timer **heap, struct timer *t)
>>   {
>> -int sz = heap_metadata(heap)->size;
>> -int pos = t->heap_offset;
>> +unsigned int sz = heap_metadata(heap)->size;
>> +unsigned int pos = t->heap_offset;
>>   
>>   if ( unlikely(pos == sz) )
>>   {
>> @@ -130,7 +130,7 @@ static int remove_from_heap(struct timer
>>   /* Add new entry @t to @heap. Return TRUE if new top of heap. */
>>   static int add_to_heap(struct timer **heap, struct timer *t)
>>   {
>> -int sz = heap_metadata(heap)->size;
>> +unsigned int sz = heap_metadata(heap)->size;
>>   
>>   /* Fail if the heap is full. */
>>   if ( unlikely(sz == heap_metadata(heap)->limit) )
>> @@ -463,9 +463,17 @@ static void timer_softirq_action(void)
>>   if ( unlikely(ts->list != NULL) )
>>   {
>>   /* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
>> -int old_limit = heap_metadata(heap)->limit;
>> -int new_limit = ((old_limit + 1) << 4) - 1;
>> -struct timer **newheap = xmalloc_array(struct timer *, new_limit +
>> 1);
>> +unsigned int old_limit = heap_metadata(heap)->limit;
>> +unsigned int new_limit = ((old_limit + 1) << 4) - 1;
>> +struct timer **newheap = NULL;
>> +
>> +/* Don't grow the heap beyond what is representable in its
>> metadata. */
>> +if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
>> + new_limit + 1 )
>> +newheap = xmalloc_array(struct timer *, new_limit + 1);
>> +else
>> +printk_once(XENLOG_WARNING "CPU%u: timer heap limit reached\n",
>> +smp_processor_id());
>>   if ( newheap != NULL )
>>   {
>>   spin_lock_irq(>lock);
>> @@ -544,7 +549,7 @@ static void dump_timerq(unsigned char ke
>>   struct timers *ts;
>>   unsigned long  flags;
>>   s_time_t   now = NOW();
>> -inti, j;
>> +unsigned int   i, j;
>>   
>>   printk("Dumping timer queues:\n");
>>   
>> @@ -556,7 +561,7 @@ static void dump_timerq(unsigned char ke
>>   spin_lock_irqsave(>lock, flags);
>>   for ( j = 1; j <= heap_metadata(ts->heap)->size; j++ )
>>   dump_timer(ts->heap[j], now);
>> -for ( t = ts->list, j = 0; t != NULL; t = t->list_next, j++ )
>> +for ( t = ts->list; t != NULL; t = t->list_next )
>>   dump_timer(t, now);
>>   spin_unlock_irqrestore(>lock, flags);
>>   }
>>
>>
>>
>>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
> 

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

[Xen-devel] Ping: [PATCH] x86/hvm: make hvmemul_virtual_to_linear()'s reps parameter optional

2019-07-15 Thread Jan Beulich
On 01.07.2019 13:59, Jan Beulich wrote:
> A majority of callers wants just a single iteration handled. Allow to
> express this by passing in a NULL pointer, instead of setting up a local
> variable just to hold the "1" to pass in here.
> 
> Signed-off-by: Jan Beulich 
> ---
> Note that this conflicts with additions/changes made by "x86emul:
> further work". Whatever goes in later will need re-basing.
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -788,14 +788,14 @@ static int hvmemul_virtual_to_linear(
>enum x86_segment seg,
>unsigned long offset,
>unsigned int bytes_per_rep,
> -unsigned long *reps,
> +unsigned long *reps_p,
>enum hvm_access_type access_type,
>struct hvm_emulate_ctxt *hvmemul_ctxt,
>unsigned long *linear)
>{
>struct segment_register *reg;
>int okay;
> -unsigned long max_reps = 4096;
> +unsigned long reps = 1;
>
>if ( seg == x86_seg_none )
>{
> @@ -803,62 +803,72 @@ static int hvmemul_virtual_to_linear(
>return X86EMUL_OKAY;
>}
>
> -/*
> - * If introspection has been enabled for this domain, and we're emulating
> - * becase a vm_reply asked us to (i.e. not doing regular IO) reps should
> - * be at most 1, since optimization might otherwise cause a single
> - * vm_event being triggered for repeated writes to a whole page.
> - */
> -if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) &&
> - current->arch.vm_event->emulate_flags != 0 )
> -   max_reps = 1;
> +if ( reps_p )
> +{
> +unsigned long max_reps = 4096;
>
> -/*
> - * Clip repetitions to avoid overflow when multiplying by @bytes_per_rep.
> - * The chosen maximum is very conservative but it's what we use in
> - * hvmemul_linear_to_phys() so there is no point in using a larger value.
> - */
> -*reps = min_t(unsigned long, *reps, max_reps);
> +/*
> + * If introspection has been enabled for this domain, and we're
> + * emulating because a vm_reply asked us to (i.e. not doing regular 
> IO)
> + * reps should be at most 1, since optimization might otherwise 
> cause a
> + * single vm_event being triggered for repeated writes to a whole 
> page.
> + */
> +if ( unlikely(current->domain->arch.mem_access_emulate_each_rep) &&
> + current->arch.vm_event->emulate_flags != 0 )
> +   max_reps = 1;
> +
> +/*
> + * Clip repetitions to avoid overflow when multiplying by
> + * @bytes_per_rep. The chosen maximum is very conservative but it's
> + * what we use in hvmemul_linear_to_phys() so there is no point in
> + * using a larger value.
> + */
> +reps = *reps_p = min_t(unsigned long, *reps_p, max_reps);
> +}
>
>reg = hvmemul_get_seg_reg(seg, hvmemul_ctxt);
>if ( IS_ERR(reg) )
>return -PTR_ERR(reg);
>
> -if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1) )
> +if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (reps > 1) )
>{
>/*
> * x86_emulate() clips the repetition count to ensure we don't wrap
> * the effective-address index register. Hence this assertion 
> holds.
> */
> -ASSERT(offset >= ((*reps - 1) * bytes_per_rep));
> +ASSERT(offset >= ((reps - 1) * bytes_per_rep));
>okay = hvm_virtual_to_linear_addr(
> -seg, reg, offset - (*reps - 1) * bytes_per_rep,
> -*reps * bytes_per_rep, access_type,
> +seg, reg, offset - (reps - 1) * bytes_per_rep,
> +reps * bytes_per_rep, access_type,
>hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt), linear);
> -*linear += (*reps - 1) * bytes_per_rep;
> +*linear += (reps - 1) * bytes_per_rep;
>if ( hvmemul_ctxt->ctxt.addr_size != 64 )
>*linear = (uint32_t)*linear;
>}
>else
>{
>okay = hvm_virtual_to_linear_addr(
> -seg, reg, offset, *reps * bytes_per_rep, access_type,
> +seg, reg, offset, reps * bytes_per_rep, access_type,
>hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt), linear);
>}
>
>if ( okay )
>return X86EMUL_OKAY;
>
> -/* If this is a string operation, emulate each iteration separately. */
> -if ( *reps != 1 )
> -return X86EMUL_UNHANDLEABLE;
> +if ( reps_p )
> +{
> +/* If this is a string operation, emulate each iteration separately. 
> */
> +if ( reps != 1 )
> +return X86EMUL_UNHANDLEABLE;
> +
> +*reps_p = 0;
> +}
>
>/*
> * Leave exception injection to the caller for non-user segments: We
> * neither know the exact error code to be used, nor can we easily
> * determine the 

Re: [Xen-devel] [PATCH] xen/mm.h: add helper function to test-and-clear _PGC_allocated

2019-07-15 Thread Paul Durrant
> -Original Message-
> From: Jan Beulich 
> Sent: 10 July 2019 23:53
> To: Paul Durrant 
> Cc: xen-devel@lists.xenproject.org; Julien Grall ; 
> Andrew Cooper
> ; Roger Pau Monne ; 
> Volodymyr Babchuk
> ; George Dunlap ; Ian 
> Jackson
> ; Stefano Stabellini ; Konrad 
> Rzeszutek Wilk
> ; Tamas K Lengyel ; Tim 
> (Xen.org) ; Wei Liu
> 
> Subject: Re: [Xen-devel] [PATCH] xen/mm.h: add helper function to 
> test-and-clear _PGC_allocated
> 
> On 10.07.2019 18:17, Paul Durrant wrote:
> > @@ -418,13 +417,7 @@ static void hvm_free_ioreq_mfn(struct hvm_ioreq_server 
> > *s, bool buf)
> >   unmap_domain_page_global(iorp->va);
> >   iorp->va = NULL;
> >
> > -/*
> > - * Check whether we need to clear the allocation reference before
> > - * dropping the explicit references taken by get_page_and_type().
> > - */
> > -if ( test_and_clear_bit(_PGC_allocated, >count_info) )
> > -put_page(page);
> > -
> > +clear_assignment_reference(page);
> >   put_page_and_type(page);
> >   }
> 
> Is there a specific reason you drop the comment? It doesn't become
> less relevant than when it was added, does it?

Not sure, since what's actually going on is now internal to the function. If I 
change the function name to clear_allocation_reference() then I think the 
comment probably becomes extraneous.

> 
> > --- a/xen/include/xen/mm.h
> > +++ b/xen/include/xen/mm.h
> > @@ -658,4 +658,15 @@ static inline void 
> > share_xen_page_with_privileged_guests(
> >   share_xen_page_with_guest(page, dom_xen, flags);
> >   }
> >
> > +static inline void clear_assignment_reference(struct page_info *page)
> 
> I think the function should have 'page' in it's name. Perhaps
> page_deassign() / page_dealloc() are also misleading, but how
> about page_put_alloc() or page_put_alloc_ref()?
> 

Ok, I think page_put_alloc_ref() is most descriptive (particularly w.r.t. the 
above discussion).

> > +{
> > +/*
> > + * It is unsafe to clear _PGC_allocated without holding an additional
> > + * reference.
> > + */
> > +ASSERT((page->count_info & PGC_count_mask) > 1);
> 
> While this isn't really in line with our goal of wanting to limit
> damage also in release builds, I agree that there's no really good
> alternative here. Crashing the owner of the page wouldn't help
> much, and bailing from the function wouldn't necessarily be better
> either. Hence I think this would better be BUG_ON().

Ok.

> 
> > +if ( test_and_clear_bit(_PGC_allocated, >count_info) )
> > +put_page(page);
> > +}
> 
> On the whole I have to admit I'm not entirely convinced the "open-
> coding" as you call it (to me it's not really open-coding as long as
> there is no helper) is such a bad thing here: Without the helper it
> is slightly more obvious at the use sites what's actually going on.
> But maybe that's indeed just me.

I still think a helper is better, but I'll add a comment to describe what it is 
doing.

  Paul

> 
> Jan

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

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

2019-07-15 Thread osstest service owner
flight 138998 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/138998/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 70565e64227dfa254d8a0703dd60dc74bd8b5e6e
baseline version:
 ovmf 55b9bbf40a1cf9788dd6a7b093851076851fc670

Last test of basis   138966  2019-07-13 22:12:19 Z1 days
Testing same since   138998  2019-07-14 17:21:25 Z0 days1 attempts


People who touched revisions under test:
  Jordan Justen 

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
   55b9bbf40a..70565e6422  70565e64227dfa254d8a0703dd60dc74bd8b5e6e -> 
xen-tested-master

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

  1   2   >