[Xen-devel] [linux-3.18 test] 112857: trouble: blocked/broken/fail/pass

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-pvops 3 capture-logs   broken REGR. vs. 112102

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-multivcpu  7 xen-bootfail in 112843 pass in 112857
 test-amd64-amd64-xl-qemut-debianhvm-amd64 16 guest-localmigrate/x10 fail pass 
in 112843
 test-armhf-armhf-xl-credit2   7 xen-boot   fail pass in 112843

Regressions which are regarded as allowable (not blocking):
 build-arm64   2 hosts-allocate broken REGR. vs. 112102
 build-arm64-xsm   2 hosts-allocate broken REGR. vs. 112102
 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 112102

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64-xsm   3 capture-logs  broken blocked in 112102
 build-arm64   3 capture-logs  broken blocked in 112102
 test-amd64-amd64-xl-qemut-win7-amd64 18 guest-start/win.repeat fail blocked in 
112102
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail in 112843 blocked in 
112102
 test-amd64-i386-xl-qemuu-win7-amd64 18 guest-start/win.repeat fail in 112843 
blocked in 112102
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail in 112843 blocked in 
112102
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop  fail in 112843 like 112085
 test-armhf-armhf-xl-credit2 13 migrate-support-check fail in 112843 never pass
 test-armhf-armhf-xl-credit2 14 saverestore-support-check fail in 112843 never 
pass
 test-amd64-amd64-rumprun-amd64 17 rumprun-demo-xenstorels/xenstorels.repeat 
fail like 112102
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 112102
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 112102
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail like 112102
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 112102
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 112102
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 112102
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 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-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   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-armhf-armhf-libvirt 13 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-libvirt-raw 12 migrate-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-amd64-amd64-xl-qemut-win10-i386 10 windows-install

[Xen-devel] [PATCH v8] VT-d: use correct BDF for VF to search VT-d unit

[Chao Gao]

When SR-IOV is enabled, 'Virtual Functions' of a 'Physical Function' are under
the scope of the same VT-d unit as the 'Physical Function'. A 'Physical
Function' can be a 'Traditional Function' or an ARI 'Extended Function'.
And furthermore, 'Extended Functions' on an endpoint are under the scope of
the same VT-d unit as the 'Traditional Functions' on the endpoint. To search
VT-d unit, the BDF of PF or the BDF of a traditional function may be used. And
it depends on whether the PF is an extended function or not.

Current code uses PCI_SLOT() to recognize an ARI 'Extended Funcion'. But it
is conceptually wrong w/o checking whether PF is an extended function and
would lead to match VFs of a RC endpoint to a wrong VT-d unit.

This patch reuses 'is_extfn' field in VF's struct pci_dev_info to indicate
whether the PF of this VF is an extended function. The field helps to use
correct BDF to search VT-d unit.

Reported-by: Crawford, Eric R 
Signed-off-by: Chao Gao 
---
v8:
 - use "conceptually wrong", instead of "a corner case" in commit message
 - check 'is_virtfn' first in acpi_find_matched_drhd_unit()

v7:
 - Drop Eric's tested-by
 - Change commit message to be clearer
 - Re-use VF's is_extfn field
 - access PF's is_extfn field in locked area

---
 xen/drivers/passthrough/pci.c  |  6 ++
 xen/drivers/passthrough/vtd/dmar.c | 12 ++--
 xen/include/xen/pci.h  |  4 
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 27bdb71..2a91320 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -599,6 +599,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
 unsigned int slot = PCI_SLOT(devfn), func = PCI_FUNC(devfn);
 const char *pdev_type;
 int ret;
+bool pf_is_extfn = false;
 
 if (!info)
 pdev_type = "device";
@@ -608,6 +609,8 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
 {
 pcidevs_lock();
 pdev = pci_get_pdev(seg, info->physfn.bus, info->physfn.devfn);
+if ( pdev )
+pf_is_extfn = pdev->info.is_extfn;
 pcidevs_unlock();
 if ( !pdev )
 pci_add_device(seg, info->physfn.bus, info->physfn.devfn,
@@ -707,6 +710,9 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
seg, bus, slot, func, ctrl);
 }
 
+/* VF's 'is_extfn' is used to indicate whether PF is an extended function 
*/
+if ( pdev->info.is_virtfn )
+pdev->info.is_extfn = pf_is_extfn;
 check_pdev(pdev);
 
 ret = 0;
diff --git a/xen/drivers/passthrough/vtd/dmar.c 
b/xen/drivers/passthrough/vtd/dmar.c
index 82040dd..75c9c92 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -211,15 +211,15 @@ struct acpi_drhd_unit *acpi_find_matched_drhd_unit(const 
struct pci_dev *pdev)
 if ( pdev == NULL )
 return NULL;
 
-if ( pdev->info.is_extfn )
+if ( pdev->info.is_virtfn )
 {
-bus = pdev->bus;
-devfn = 0;
+bus = pdev->info.physfn.bus;
+devfn = pdev->info.is_extfn ? 0 : pdev->info.physfn.devfn;
 }
-else if ( pdev->info.is_virtfn )
+else if ( pdev->info.is_extfn )
 {
-bus = pdev->info.physfn.bus;
-devfn = PCI_SLOT(pdev->info.physfn.devfn) ? 0 : 
pdev->info.physfn.devfn;
+bus = pdev->bus;
+devfn = 0;
 }
 else
 {
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 59b6e8a..ea86f9f 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -39,6 +39,10 @@
 #define PCI_SBDF3(s,b,df) s) & 0x) << 16) | PCI_BDF2(b, df))
 
 struct pci_dev_info {
+/*
+ * Considering VF's 'is_extfn' field isn't used, we reuse VF's 'is_extfn'
+ * field to show whether the PF of this VF is an extended function.
+ */
 bool_t is_extfn;
 bool_t is_virtfn;
 struct {
-- 
1.8.3.1


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

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

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 279c01ce13739f0fd8ec3e7652299f6873fc14a9
baseline version:
 ovmf e4d409c6e3b96738fb0c710ecd21bcd79db93381

Last test of basis   112846  2017-08-23 15:56:43 Z1 days
Testing same since   112859  2017-08-24 07:31:08 Z0 days1 attempts


People who touched revisions under test:
  Liming Gao 

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 :

+ branch=ovmf
+ revision=279c01ce13739f0fd8ec3e7652299f6873fc14a9
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 
279c01ce13739f0fd8ec3e7652299f6873fc14a9
+ branch=ovmf
+ revision=279c01ce13739f0fd8ec3e7652299f6873fc14a9
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.9-testing
+ '[' x279c01ce13739f0fd8ec3e7652299f6873fc14a9 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : 

Re: [Xen-devel] [PATCH V2 9/25] tools/libxl: build DMAR table for a guest with one virtual VTD

[Lan Tianyu]

On 2017年08月24日 19:08, Wei Liu wrote:
 If add dmar table for hvmlite, we should combine dmar table with other
 > >> ACPI table and populate into acpi_modules[2]. This is how hvmlite add
 > >> other ACPI tables in libxl__dom_load_acpi().
 > >>
>>> > > 
>>> > > Sure, that sounds plausible.
>>> > > 
>>> > > What I would like to see is to have one entry point to manipulate APCI
>>> > > tables.
>>> > > 
>>> > > Given the patch volume we're seeing now, we expect contributors to drive
>>> > > the discussion forward. If you're not sure, feel free to ask more 
>>> > > questions.
>> > 
>> > I am not sure whether I understood correctly.
>> > 
>> > PVHv2 builds all ACPI table in tool stack and uses acpi_module[0, 1, 2]
>> > to pass related table content.
>> > 
>> > HVM builds ACPI tables in hvmloader and just use acpi_module[0] to pass
>> > additional ACPI firmware or table.
>> > 
>> > These two modes have different way to use acpi_modules[]. So I think we
>> > can't combine them, right?
>> > 
> There might be some misunderstanding.  We probably don't want to
> manipulate the content of the tables in libxl.
> 
>> > For build dmar table, we have introduced construct_dmar() in under
>> > libacpi to build dmar table and PVHv2 also can use it in
>> > libxl__dom_load_acpi().
>> > 
> My major complain is now there are two functions and in two different
> locations, in two different phases of domain construction that would
> manipulate ACPI tables. I would like to have only one.
> 
> The function you're currently modifying libxl__domain_firmware is not
> the right place. It's primary function is to load files from disks.
> 
> You should be able to call the function you introduced in
> libxl__dom_load_acpi, provided appropriate checks are added.

But libxl__dom_load_acpi() isn't called on hvm guest code path. It just
works for PVHv2/HVMlite and have some conflict with hvm guest
configuration(i.e, acpi_module).


int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
   libxl_domain_build_info
*info,
   struct xc_dom_image *dom)
{
int rc = 0;

if ((info->type == LIBXL_DOMAIN_TYPE_HVM) &&
(info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE)) {
rc = libxl__dom_load_acpi(gc, info, dom);
if (rc != 0)
LOGE(ERROR, "libxl_dom_load_acpi failed");
}

return rc;
}



-- 
Best regards
Tianyu Lan

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

[Xen-devel] [xen-unstable test] 112855: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-examine   7 reboot   fail REGR. vs. 112809
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 112809
 build-amd64-xsm   6 xen-buildfail REGR. vs. 112809
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
112809
 test-armhf-armhf-xl-credit2 16 guest-start/debian.repeat fail REGR. vs. 112809
 test-amd64-amd64-xl-qemut-win7-amd64 10 windows-install  fail REGR. vs. 112809
 test-amd64-i386-xl-qemut-win7-amd64 10 windows-install   fail REGR. vs. 112809
 test-amd64-i386-xl-qemut-ws16-amd64 10 windows-install   fail REGR. vs. 112809

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64-pvops 2 hosts-allocate  broken like 112809
 build-arm64   2 hosts-allocate  broken like 112809
 build-arm64-xsm   2 hosts-allocate  broken like 112809
 build-arm64-pvops 3 capture-logsbroken like 112809
 build-arm64   3 capture-logsbroken like 112809
 build-arm64-xsm   3 capture-logsbroken like 112809
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 112809
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 112809
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 112809
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 112809
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 112809
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 112809
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  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-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-check  

Re: [Xen-devel] [PATCH QEMU v4] xen/pt: allow QEMU to request MSI unmasking at bind time

[Stefano Stabellini]

On Thu, 24 Aug 2017, Roger Pau Monne wrote:
> When a MSI interrupt is bound to a guest using
> xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is
> left masked by default.
> 
> This causes problems with guests that first configure interrupts and
> clean the per-entry MSIX table mask bit and afterwards enable MSIX
> globally. In such scenario the Xen internal msixtbl handlers would not
> detect the unmasking of MSIX entries because vectors are not yet
> registered since MSIX is not enabled, and vectors would be left
> masked.
> 
> Introduce a new flag in the gflags field to signal Xen whether a MSI
> interrupt should be unmasked after being bound.
> 
> This also requires to track the mask register for MSI interrupts, so
> QEMU can also notify to Xen whether the MSI interrupt should be bound
> masked or unmasked
> 
> Signed-off-by: Roger Pau Monné 
> Reviewed-by: Jan Beulich 
> Reported-by: Andreas Kinzler 

Acked-by: Stefano Stabellini 

I'll queue it up.


> ---
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: Jan Beulich 
> Cc: qemu-de...@nongnu.org
> ---
> Changes since v2:
>  - Fix coding style.
> 
> Changes since v1:
>  - Track MSI mask bits.
>  - Notify Xen of whether MSI interrupts should be unmasked after
>binding, instead of hardcoding it.
> ---
>  hw/xen/xen_pt.h |  1 +
>  hw/xen/xen_pt_config_init.c | 20 ++--
>  hw/xen/xen_pt_msi.c | 13 ++---
>  3 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 191d9caea1..aa39a9aa5f 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -180,6 +180,7 @@ typedef struct XenPTMSI {
>  uint32_t addr_hi;  /* guest message upper address */
>  uint16_t data; /* guest message data */
>  uint32_t ctrl_offset; /* saved control offset */
> +uint32_t mask; /* guest mask bits */
>  int pirq;  /* guest pirq corresponding */
>  bool initialized;  /* when guest MSI is initialized */
>  bool mapped;   /* when pirq is mapped */
> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> index 1f04ec5eec..a3ce33e78b 100644
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -1315,6 +1315,22 @@ static int 
> xen_pt_msgdata_reg_write(XenPCIPassthroughState *s,
>  return 0;
>  }
>  
> +static int xen_pt_mask_reg_write(XenPCIPassthroughState *s, XenPTReg 
> *cfg_entry,
> + uint32_t *val, uint32_t dev_value,
> + uint32_t valid_mask)
> +{
> +int rc;
> +
> +rc = xen_pt_long_reg_write(s, cfg_entry, val, dev_value, valid_mask);
> +if (rc) {
> +return rc;
> +}
> +
> +s->msi->mask = *val;
> +
> +return 0;
> +}
> +
>  /* MSI Capability Structure reg static information table */
>  static XenPTRegInfo xen_pt_emu_reg_msi[] = {
>  /* Next Pointer reg */
> @@ -1393,7 +1409,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
>  .emu_mask   = 0x,
>  .init   = xen_pt_mask_reg_init,
>  .u.dw.read  = xen_pt_long_reg_read,
> -.u.dw.write = xen_pt_long_reg_write,
> +.u.dw.write = xen_pt_mask_reg_write,
>  },
>  /* Mask reg (if PCI_MSI_FLAGS_MASKBIT set, for 64-bit devices) */
>  {
> @@ -1404,7 +1420,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
>  .emu_mask   = 0x,
>  .init   = xen_pt_mask_reg_init,
>  .u.dw.read  = xen_pt_long_reg_read,
> -.u.dw.write = xen_pt_long_reg_write,
> +.u.dw.write = xen_pt_mask_reg_write,
>  },
>  /* Pending reg (if PCI_MSI_FLAGS_MASKBIT set, for 32-bit devices) */
>  {
> diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
> index ff9a79f5d2..6d1e3bdeb4 100644
> --- a/hw/xen/xen_pt_msi.c
> +++ b/hw/xen/xen_pt_msi.c
> @@ -24,6 +24,7 @@
>  #define XEN_PT_GFLAGS_SHIFT_DM 9
>  #define XEN_PT_GFLAGSSHIFT_DELIV_MODE 12
>  #define XEN_PT_GFLAGSSHIFT_TRG_MODE   15
> +#define XEN_PT_GFLAGSSHIFT_UNMASKED   16
>  
>  #define latch(fld) latch[PCI_MSIX_ENTRY_##fld / sizeof(uint32_t)]
>  
> @@ -155,7 +156,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
> int pirq,
> bool is_msix,
> int msix_entry,
> -   int *old_pirq)
> +   int *old_pirq,
> +   bool masked)
>  {
>  PCIDevice *d = >dev;
>  uint8_t gvec = msi_vector(data);
> @@ -171,6 +173,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
>  table_addr = s->msix->mmio_base_addr;
>  }
>  
> +gflags |= masked ? 0 : (1u << XEN_PT_GFLAGSSHIFT_UNMASKED);
> +
>  rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
> 

[Xen-devel] [xen-4.5-testing test] 112854: tolerable FAIL - PUSHED

[osstest service owner]

flight 112854 xen-4.5-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/112854/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail like 112800
 test-amd64-amd64-xl-rtds  7 xen-boot fail  like 112838
 test-xtf-amd64-amd64-2   59 leak-check/check fail  like 112838
 test-xtf-amd64-amd64-3   59 leak-check/check fail  like 112838
 test-xtf-amd64-amd64-4   59 leak-check/check fail  like 112838
 test-xtf-amd64-amd64-5   59 leak-check/check fail  like 112838
 test-xtf-amd64-amd64-1   59 leak-check/check fail  like 112838
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 112838
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 112838
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 112838
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 112838
 test-xtf-amd64-amd64-2   19 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-2 33 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-2 40 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-2   44 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-4   19 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-5   19 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-4 33 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-5 33 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-4 40 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-4   44 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-5 40 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-xtf-amd64-amd64-5   44 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-1   19 xtf/test-hvm32-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-1 33 xtf/test-hvm32pae-cpuid-faulting fail never pass
 test-amd64-amd64-xl-pvh-intel 15 guest-saverestorefail  never pass
 test-xtf-amd64-amd64-1 40 xtf/test-hvm32pse-cpuid-faulting fail never pass
 test-amd64-amd64-xl-pvh-amd  12 guest-start  fail   never pass
 test-xtf-amd64-amd64-1   44 xtf/test-hvm64-cpuid-faulting fail  never pass
 test-xtf-amd64-amd64-2   58 xtf/test-hvm64-xsa-195   fail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-3   58 xtf/test-hvm64-xsa-195   fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-4   58 xtf/test-hvm64-xsa-195   fail   never pass
 test-xtf-amd64-amd64-5   58 xtf/test-hvm64-xsa-195   fail   never pass
 test-xtf-amd64-amd64-1   58 xtf/test-hvm64-xsa-195   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-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 13 guest-saverestore  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-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-amd64-amd64-xl-qemut-ws16-amd64 13 guest-saverestore  fail 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-vhd  11 guest-start  fail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   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-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install 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
 

[Xen-devel] [linux-linus test] 112853: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl  10 debian-install   fail REGR. vs. 110515
 test-amd64-amd64-xl-xsm  10 debian-install   fail REGR. vs. 110515
 test-amd64-amd64-xl-pvh-intel 10 debian-install  fail REGR. vs. 110515
 test-amd64-amd64-libvirt 10 debian-install   fail REGR. vs. 110515
 test-amd64-amd64-pair16 debian-install/dst_host  fail REGR. vs. 110515
 test-amd64-amd64-libvirt-pair 16 debian-install/dst_host fail REGR. vs. 110515
 test-amd64-i386-libvirt  10 debian-install   fail REGR. vs. 110515
 test-amd64-i386-pair 16 debian-install/dst_host  fail REGR. vs. 110515
 test-amd64-amd64-libvirt-xsm 10 debian-install   fail REGR. vs. 110515
 test-amd64-amd64-xl-multivcpu 10 debian-install  fail REGR. vs. 110515
 test-amd64-amd64-xl-credit2  10 debian-install   fail REGR. vs. 110515
 test-amd64-i386-xl   10 debian-install   fail REGR. vs. 110515
 test-amd64-amd64-xl-pvh-amd  10 debian-install   fail REGR. vs. 110515
 test-amd64-i386-libvirt-pair 16 debian-install/dst_host  fail REGR. vs. 110515
 build-i386-xsm6 xen-buildfail REGR. vs. 110515
 test-armhf-armhf-libvirt-xsm  6 xen-install  fail REGR. vs. 110515
 test-armhf-armhf-libvirt 10 debian-install   fail REGR. vs. 110515
 test-armhf-armhf-xl-cubietruck 10 debian-install fail REGR. vs. 110515
 test-armhf-armhf-xl-multivcpu 10 debian-install  fail REGR. vs. 110515
 test-armhf-armhf-xl-xsm  10 debian-install   fail REGR. vs. 110515
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
110515
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
110515
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
110515
 test-armhf-armhf-xl  10 debian-install   fail REGR. vs. 110515
 test-armhf-armhf-xl-arndale  10 debian-install   fail REGR. vs. 110515
 test-armhf-armhf-xl-credit2  10 debian-install   fail REGR. vs. 110515

Regressions which are regarded as allowable (not blocking):
 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 110515
 build-arm64-xsm   2 hosts-allocate broken REGR. vs. 110515
 build-arm64   2 hosts-allocate broken REGR. vs. 110515
 test-armhf-armhf-xl-rtds 10 debian-install   fail REGR. vs. 110515

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64-pvops 3 capture-logs  broken blocked in 110515
 build-arm64-xsm   3 capture-logs  broken blocked in 110515
 build-arm64   3 capture-logs  broken blocked in 110515
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 110515
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 110515
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 110515
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   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-qemut-win10-i386 10 

Re: [Xen-devel] x86: PIE support and option to extend KASLR randomization

[Steven Rostedt]

On Thu, 24 Aug 2017 14:13:38 -0700
Thomas Garnier  wrote:

> With the fix for function tracing, the hackbench results have an
> average of +0.8 to +1.4% (from +8% to +10% before). With a default
> configuration, the numbers are closer to 0.8%.

Wow, an empty fentry function not "nop"ed out only added 8% to 10%
overhead. I never did the benchmarks of that since I did it before
fentry was introduced, which was with the old "mcount". That gave an
average of 13% overhead in hackbench.

-- Steve

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

Re: [Xen-devel] [stage1-xen (RFC) PATCH 07/10] .circleci/config.yml: Add

[Stefano Stabellini]

On Thu, 24 Aug 2017, Rajiv Ranganath wrote:
> On Thu, Aug 24 2017 at 05:54:05 AM, Stefano Stabellini 
>  wrote:
> > On Mon, 21 Aug 2017, Rajiv Ranganath wrote:
> >> From: Rajiv M Ranganath 
> >
> > Does .circleci need to be in the top directory or could it be under
> > fedora? If possible, I think it would make more sense to introduce it
> > there.
> >
> 
> I would have also preferred the `.circleci/` directory to be under
> `build/fedora/`.
> 
> However, I could not find an option to change this directory. From their
> documentation [1], I get a sense that this path is hardcoded.

Oh well. In that case, we'll keep it in the root directory. Thanks for
checking.

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

[Xen-devel] [libvirt test] 112856: tolerable trouble: blocked/broken/pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 build-arm64-xsm   2 hosts-allocate  broken like 112840
 build-arm64-pvops 2 hosts-allocate  broken like 112840
 build-arm64-xsm   3 capture-logsbroken like 112840
 build-arm64   2 hosts-allocate  broken like 112840
 build-arm64-pvops 3 capture-logsbroken like 112840
 build-arm64   3 capture-logsbroken like 112840
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 112840
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 112840
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 112840
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-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

version targeted for testing:
 libvirt  f60ec522a3c508c749d10e70f29c4ad8c6120b36
baseline version:
 libvirt  a530078cd2d7d9a47a6e96a64f70497fb7b2ff10

Last test of basis   112840  2017-08-23 04:21:20 Z1 days
Testing same since   112856  2017-08-24 04:21:04 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  John Ferlan 
  Nikolay Shirokovskiy 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  broken  
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  broken  
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  blocked 
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopsbroken  
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm blocked 
 test-armhf-armhf-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt blocked 
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   blocked 
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd 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

[Xen-devel] [xen-4.7-testing baseline-only test] 72014: tolerable trouble: blocked/broken/fail/pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 72014 xen-4.7-testing real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/72014/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64-pvops 2 hosts-allocate   broken never pass
 build-arm64-xsm   2 hosts-allocate   broken never pass
 build-arm64   2 hosts-allocate   broken never pass
 build-arm64-xsm   3 capture-logs broken never pass
 build-arm64-pvops 3 capture-logs broken never pass
 build-arm64   3 capture-logs broken never pass
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail like 72004
 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-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-armhf-armhf-xl-midway   13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail 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-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  12 guest-start  fail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 15 guest-saverestorefail  never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-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-amd64-amd64-qemuu-nested-intel 18 capture-logs/l1(18) fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail 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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 17 guest-stop fail never pass

version targeted for testing:
 xen  30d50f8ead03d6524cbc4ed22075980090fbd2ed
baseline version:
 xen  5151257626155d6e331cc9e66d896c84db1611e1

Last test of basis72004  2017-08-23 00:48:33 Z1 days
Testing same since72014  2017-08-24 14:47:31 Z0 days1 attempts

Re: [Xen-devel] x86: PIE support and option to extend KASLR randomization

[Linus Torvalds]

On Thu, Aug 24, 2017 at 2:13 PM, Thomas Garnier  wrote:
>
> My original performance testing was done with an Ubuntu generic
> configuration. This configuration has the CONFIG_FUNCTION_TRACER
> option which was incompatible with PIE. The tracer failed to replace
> the __fentry__ call by a nop slide on each traceable function because
> the instruction was not the one expected. If PIE is enabled, gcc
> generates a difference call instruction based on the GOT without
> checking the visibility options (basically call *__fentry__@GOTPCREL).

Gah.

Don't we actually have *more* address bits for randomization at the
low end, rather than getting rid of -mcmodel=kernel?

Has anybody looked at just moving kernel text by smaller values than
the page size? Yeah, yeah, the kernel has several sections that need
page alignment, but I think we could relocate normal text by just the
cacheline size, and that sounds like it would give several bits of
randomness with little downside.

Or has somebody already looked at it and I just missed it?

   Linus

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

Re: [Xen-devel] [PATCH 1/1] xen-blkback: stop blkback thread of every queue in xen_blkif_disconnect

[annie li]

On 8/23/2017 5:20 PM, Konrad Rzeszutek Wilk wrote:

..snip..

Acked-by: Roger Pau Monné 

Forgot to add, this needs to be backported to stable branches, so:

Annie, could you resend the patch with the tags and an update
to the description to me please?

Done

Thanks
Annie

Cc: sta...@vger.kernel.org

Roger.

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



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

[Xen-devel] [PATCH v2 1/1] xen-blkback: stop blkback thread of every queue in xen_blkif_disconnect

[Annie Li]

In xen_blkif_disconnect, before checking inflight I/O, following code
stops the blkback thread,
if (ring->xenblkd) {
kthread_stop(ring->xenblkd);
wake_up(>shutdown_wq);
}
If there is inflight I/O in any non-last queue, blkback returns -EBUSY
directly, and above code would not be called to stop thread of remaining
queue and processs them. When removing vbd device with lots of disk I/O
load, some queues with inflight I/O still have blkback thread running even
though the corresponding vbd device or guest is gone.
And this could cause some problems, for example, if the backend device type
is file, some loop devices and blkback thread always lingers there forever
after guest is destroyed, and this causes failure of umounting repositories
unless rebooting the dom0.
This patch allows thread of every queue has the chance to get stopped.
Otherwise, only thread of queue previous to(including) first busy one get
stopped, blkthread of remaining queue will still run.  So stop all threads
properly and return -EBUSY if any queue has inflight I/O.

Signed-off-by: Annie Li 
Reviewed-by: Herbert van den Bergh 
Reviewed-by: Bhavesh Davda 
Reviewed-by: Adnan Misherfi 
Acked-by: Roger Pau Monné 
---
  v2: enhance patch description and add Acked-by
---
 drivers/block/xen-blkback/xenbus.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c 
b/drivers/block/xen-blkback/xenbus.c
index 792da68..2adb859 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -244,6 +244,7 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
 {
struct pending_req *req, *n;
unsigned int j, r;
+   bool busy = false;
 
for (r = 0; r < blkif->nr_rings; r++) {
struct xen_blkif_ring *ring = >rings[r];
@@ -261,8 +262,10 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
 * don't have any discard_io or other_io requests. So, checking
 * for inflight IO is enough.
 */
-   if (atomic_read(>inflight) > 0)
-   return -EBUSY;
+   if (atomic_read(>inflight) > 0) {
+   busy = true;
+   continue;
+   }
 
if (ring->irq) {
unbind_from_irqhandler(ring->irq, ring);
@@ -300,6 +303,9 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
WARN_ON(i != (XEN_BLKIF_REQS_PER_PAGE * blkif->nr_ring_pages));
ring->active = false;
}
+   if (busy)
+   return -EBUSY;
+
blkif->nr_ring_pages = 0;
/*
 * blkif->rings was allocated in connect_ring, so we should free it in
-- 
1.9.3


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

Re: [Xen-devel] x86: PIE support and option to extend KASLR randomization

[Thomas Garnier]

On Thu, Aug 17, 2017 at 7:10 AM, Thomas Garnier  wrote:
>
> On Thu, Aug 17, 2017 at 1:09 AM, Ingo Molnar  wrote:
> >
> >
> > * Thomas Garnier  wrote:
> >
> > > > > -model=small/medium assume you are on the low 32-bit. It generates
> > > > > instructions where the virtual addresses have the high 32-bit to be 
> > > > > zero.
> > > >
> > > > How are these assumptions hardcoded by GCC? Most of the instructions 
> > > > should be
> > > > relocatable straight away, as most call/jump/branch instructions are
> > > > RIP-relative.
> > >
> > > I think PIE is capable to use relative instructions well. mcmodel=large 
> > > assumes
> > > symbols can be anywhere.
> >
> > So if the numbers in your changelog and Kconfig text cannot be trusted, 
> > there's
> > this description of the size impact which I suspect is less susceptible to
> > measurement error:
> >
> > + The kernel and modules will generate slightly more assembly (1 to 
> > 2%
> > + increase on the .text sections). The vmlinux binary will be
> > + significantly smaller due to less relocations.
> >
> > ... but describing a 1-2% kernel text size increase as "slightly more 
> > assembly"
> > shows a gratituous disregard to kernel code generation quality! In reality 
> > that's
> > a huge size increase that in most cases will almost directly transfer to a 
> > 1-2%
> > slowdown for kernel intense workloads.
> >
> >
> > Where does that size increase come from, if PIE is capable of using relative
> > instructins well? Does it come from the loss of a generic register and the
> > resulting increase in register pressure, stack spills, etc.?
>
> I will try to gather more information on the size increase. The size
> increase might be smaller with gcc 4.9 given performance was much
> better.

Coming back on this thread as I identified the root cause of the
performance issue.

My original performance testing was done with an Ubuntu generic
configuration. This configuration has the CONFIG_FUNCTION_TRACER
option which was incompatible with PIE. The tracer failed to replace
the __fentry__ call by a nop slide on each traceable function because
the instruction was not the one expected. If PIE is enabled, gcc
generates a difference call instruction based on the GOT without
checking the visibility options (basically call *__fentry__@GOTPCREL).

With the fix for function tracing, the hackbench results have an
average of +0.8 to +1.4% (from +8% to +10% before). With a default
configuration, the numbers are closer to 0.8%.

On the .text size, with gcc 4.9 I see +0.8% on default configuration
and +1.180% on the ubuntu configuration.

Next iteration should have an updated set of performance metrics (will
try to use gcc 6.0 or higher) and incorporate the fix on function
tracing.

Let me know if you have questions and feedback.

>
> >
> > So I'm still unhappy about this all, and about the attitude surrounding it.
> >
> > Thanks,
> >
> > Ingo
>
>
>
>
> --
> Thomas




-- 
Thomas

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

Re: [Xen-devel] [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges

[Dan Williams]

On Thu, Aug 24, 2017 at 9:39 AM, Christoph Hellwig  wrote:
> On Thu, Aug 24, 2017 at 09:31:17AM -0700, Dan Williams wrote:
>> External agent is a DMA device, or a hypervisor like Xen. In the DMA
>> case perhaps we can use the fcntl lease mechanism, I'll investigate.
>> In the Xen case it actually would need to use fiemap() to discover the
>> physical addresses that back the file to setup their M2P tables.
>> Here's the discussion where we discovered that physical address
>> dependency:
>>
>> https://lists.xen.org/archives/html/xen-devel/2017-04/msg00419.html
>
> fiemap does not work to discover physical addresses.  If they want
> to do anything involving physical address they will need a kernel
> driver.

True, it's broken with respect to multi-device filesystems and these
patches do nothing to fix that problem. Ok, I'm fine to let that use
case depend on a kernel driver and just focus on fixing the DMA case.

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

Re: [Xen-devel] [PATCH v2 1/4] xen: credit2: implement utilization cap

[Anshul Makkar]

On 8/18/17 4:50 PM, Dario Faggioli wrote:
  
@@ -474,6 +586,12 @@ static inline struct csched2_runqueue_data *c2rqd(const struct scheduler *ops,

  return _priv(ops)->rqd[c2r(cpu)];
  }
  
+/* Does the domain of this vCPU have a cap? */

+static inline bool has_cap(const struct csched2_vcpu *svc)
+{
+return svc->budget != STIME_MAX;
+}
+
  /*
   * Hyperthreading (SMT) support.
   *
@@ -1515,7 +1633,16 @@ static void reset_credit(const struct scheduler *ops, 
int cpu, s_time_t now,
   * that the credit it has spent so far get accounted.
   */
  if ( svc->vcpu == curr_on_cpu(svc_cpu) )
+{
  burn_credits(rqd, svc, now);
+/*
+ * And, similarly, in case it has run out of budget, as a
+ * consequence of this round of accounting, we also must inform
+ * its pCPU that it's time to park it, and pick up someone else.
+ */
+if ( unlikely(svc->budget <= 0) )
+tickle_cpu(svc_cpu, rqd);
This is for accounting of credit. Why it willl impact the budget. Do you 
intend to refer that

budget of current vcpu expired while doing calculation for credit ??

+}
  
  start_credit = svc->credit;
  
@@ -1571,27 +1698,35 @@ void burn_credits(struct csched2_runqueue_data *rqd,
  
  delta = now - svc->start_time;
  
-if ( likely(delta > 0) )

-{
-SCHED_STAT_CRANK(burn_credits_t2c);
-t2c_update(rqd, delta, svc);
-svc->start_time = now;
-}
-else if ( delta < 0 )
+if ( unlikely(delta <= 0) )
  {

+static void replenish_domain_budget(void* data)
+{
+struct csched2_dom *sdom = data;
+unsigned long flags;
+s_time_t now;
+LIST_HEAD(parked);
+
+spin_lock_irqsave(>budget_lock, flags);
+
+now = NOW();
+
+/*
+ * Let's do the replenishment. Note, though, that a domain may overrun,
+ * which means the budget would have gone below 0 (reasons may be system
+ * overbooking, accounting issues, etc.). It also may happen that we are
+ * handling the replenishment (much) later than we should (reasons may
+ * again be overbooking, or issues with timers).
+ *
+ * Even in cases of overrun or delay, however, we expect that in 99% of
+ * cases, doing just one replenishment will be good enough for being able
+ * to unpark the vCPUs that are waiting for some budget.
+ */
+do_replenish(sdom);
+
+/*
+ * And now, the special cases:
+ * 1) if we are late enough to have skipped (at least) one full period,
+ * what we must do is doing more replenishments. Note that, however,
+ * every time we add tot_budget to the budget, we also move next_repl
+ * away by CSCHED2_BDGT_REPL_PERIOD, to make sure the cap is always
+ * respected.
+ */
+if ( unlikely(sdom->next_repl <= now) )
+{
+do
+do_replenish(sdom);
+while ( sdom->next_repl <= now );
+}
Just a bit confused. Have you seen this kind of scenario. Please can you 
explain it.

Is this condition necessary.

+/*
+ * 2) if we overrun by more than tot_budget, then budget+tot_budget is
+ * still < 0, which means that we can't unpark the vCPUs. Let's bail,
+ * and wait for future replenishments.
+ */
+if ( unlikely(sdom->budget <= 0) )
+{
+spin_unlock_irqrestore(>budget_lock, flags);
+goto out;
+}
"if we overran by more than tot_budget in previous run", make is more 
clear..

+
+/* Since we do more replenishments, make sure we didn't overshot. */
+sdom->budget = min(sdom->budget, sdom->tot_budget);
+
+/*
+ * As above, let's prepare the temporary list, out of the domain's
+ * parked_vcpus list, now that we hold the budget_lock. Then, drop such
+ * lock, and pass the list to the unparking function.
+ */
+list_splice_init(>parked_vcpus, );
+
+spin_unlock_irqrestore(>budget_lock, flags);
+
+unpark_parked_vcpus(sdom->dom->cpupool->sched, );
+
+ out:
+set_timer(sdom->repl_timer, sdom->next_repl);
+}
+
  #ifndef NDEBUG
  static inline void
  csched2_vcpu_check(struct vcpu *vc)
@@ -1658,6 +2035,9 @@ csched2_alloc_vdata(const struct scheduler *ops, struct 
vcpu *vc, void *dd)
  }
  svc->tickled_cpu = -1;
  
+


Rest, looks good to me.

Thanks
Anshul





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

Re: [Xen-devel] [PATCH v2] common/vm_event: Initialize vm_event lists on domain creation

[Tamas K Lengyel]

On Thu, Aug 24, 2017 at 9:24 AM, Jan Beulich  wrote:
 On 24.08.17 at 17:17,  wrote:
>> On Jo, 2017-08-24 at 07:24 -0600, Jan Beulich wrote:
>>> > @@ -500,6 +519,9 @@ bool_t vm_event_check_ring(struct
>>> > vm_event_domain *ved)
>>> >  int __vm_event_claim_slot(struct domain *d, struct vm_event_domain
>>> > *ved,
>>> >bool_t allow_sleep)
>>> >  {
>>> > +if ( !ved )
>>> > +return -ENOSYS;
>>> I don't think ENOSYS is correct here.
>> Can you tell me what is the preferred return value here?
>
> -EOPNOTSUPP is what we commonly use in such cases.
>
>>> > @@ -599,7 +621,6 @@ int vm_event_domctl(struct domain *d,
>>> > xen_domctl_vm_event_op_t *vec,
>>> >  #ifdef CONFIG_HAS_MEM_PAGING
>>> >  case XEN_DOMCTL_VM_EVENT_OP_PAGING:
>>> >  {
>>> > -struct vm_event_domain *ved = >vm_event->paging;
>>> Dropping this local variable (and similar ones below) pointlessly
>>> increases the size of this patch.
>> I have dropped the local var because in case of a XEN_VM_EVENT_ENABLE
>> d->vm_event_... is allocated in the vm_event_enable function below so
>> it will allocate mem for the local variable.
>
> I don't see how that renders the local variable useless. But anyway,
> its the maintainers of that code to judge.

I guess the reason for dropping it is that vm_event_enable will place
the location of the structure into the pointer that was passed in, so
if you are passing in a pointer that was locally declared you would
then still have to copy it back to d->vm_event_ which doesn't make
much sense. IMHO it's fine how it's changed in the patch.

Tamas

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

Re: [Xen-devel] [PATCH v2] common/vm_event: Initialize vm_event lists on domain creation

[Tamas K Lengyel]

On Thu, Aug 24, 2017 at 5:48 AM, Alexandru Isaila
 wrote:
> The patch splits the vm_event into three structures:vm_event_share,
> vm_event_paging, vm_event_monitor. The allocation for the
> structure is moved to vm_event_enable so that it can be
> allocated/init when needed and freed in vm_event_disable.
>
> Signed-off-by: Alexandru Isaila 

Thanks for doing this patch, I think it improves the code a lot!


> @@ -51,8 +51,7 @@ int mem_access_memop(unsigned long cmd,
>  if ( rc )
>  goto out;
>

Why are you removing setting the rc below?

> -rc = -ENODEV;
> -if ( unlikely(!d->vm_event->monitor.ring_page) )
> +if ( !d->vm_event_monitor || unlikely(!d->vm_event_monitor->ring_page) )
>  goto out;
>
>  switch ( mao.op )

...

> @@ -187,39 +194,45 @@ void vm_event_wake(struct domain *d, struct 
> vm_event_domain *ved)
>  vm_event_wake_blocked(d, ved);
>  }
>
> -static int vm_event_disable(struct domain *d, struct vm_event_domain *ved)
> +static int vm_event_disable(struct domain *d, struct vm_event_domain **ved)
>  {

I think you should check for *ved and *ved->ring_page all in one go below.

> -if ( ved->ring_page )
> +if ( !*ved )
> +return 0;
> +
> +if ( (*ved)->ring_page )
>  {
>  struct vcpu *v;
>

Thanks,
Tamas

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

[Xen-devel] [xen-4.6-testing baseline-only test] 72013: regressions - FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 72013 xen-4.6-testing real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/72013/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-credit2   7 xen-boot  fail REGR. vs. 72008
 test-xtf-amd64-amd64-421 xtf/test-hvm32-invlpg~shadow fail REGR. vs. 72008
 test-xtf-amd64-amd64-4 35 xtf/test-hvm32pae-invlpg~shadow fail REGR. vs. 72008
 test-xtf-amd64-amd64-447 xtf/test-hvm64-invlpg~shadow fail REGR. vs. 72008
 test-amd64-i386-qemuu-rhel6hvm-amd 15 leak-check/checkfail REGR. vs. 72008
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
72008

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stopfail blocked in 72008
 test-xtf-amd64-amd64-1   21 xtf/test-hvm32-invlpg~shadow fail   like 72008
 test-xtf-amd64-amd64-1  35 xtf/test-hvm32pae-invlpg~shadow fail like 72008
 test-xtf-amd64-amd64-1   47 xtf/test-hvm64-invlpg~shadow fail   like 72008
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   like 72008
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail   like 72008
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   like 72008
 test-amd64-amd64-qemuu-nested-intel 14 xen-boot/l1 fail like 72008
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 72008
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail like 72008
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-xtf-amd64-amd64-5   70 xtf/test-pv32pae-xsa-194 fail   never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-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-amd64-amd64-xl-pvh-intel 15 guest-saverestorefail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-4   70 xtf/test-pv32pae-xsa-194 fail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-2   70 xtf/test-pv32pae-xsa-194 fail   never pass
 test-xtf-amd64-amd64-1   70 xtf/test-pv32pae-xsa-194 fail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-xtf-amd64-amd64-3   70 xtf/test-pv32pae-xsa-194 fail   never pass
 test-amd64-amd64-xl-pvh-amd  12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
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-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   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-amd64-amd64-xl-qemuu-win10-i386 17 guest-stop fail 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-amd64-i386-xl-qemuu-win10-i386 17 guest-stop  fail never pass

version targeted for testing:
 xen  64c03bbacfb099f464c0fe0850ece71d4007d0ea
baseline version:
 xen  b4660b4d4a35edac715c003c84326de2b0fa4f47

[Xen-devel] [PATCH 2/3] gnttab: Drop the frame parameter from get_paged_frame()

[Andrew Cooper]

It is redundant with the *page parameter.

Signed-off-by: Andrew Cooper 
---
CC: George Dunlap 
CC: Jan Beulich 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 
---
 xen/common/grant_table.c | 50 +---
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 188c477..d8307b7 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -257,13 +257,13 @@ static inline void active_entry_release(struct 
active_grant_entry *act)
 spin_unlock(>lock);
 }
 
-/* Check if the page has been paged out, or needs unsharing.
-   If rc == GNTST_okay, *page contains the page struct with a ref taken.
-   Caller must do put_page(*page).
-   If any error, *page = NULL, *frame = INVALID_MFN, no ref taken. */
-static int get_paged_frame(unsigned long gfn, unsigned long *frame,
-   struct page_info **page, bool readonly,
-   struct domain *rd)
+/*
+ * Check if the page has been paged out, or needs unsharing.
+ * If rc == GNTST_okay, *page contains the page struct with a ref taken.
+ * Caller must do put_page(*page). If any error, *page = NULL, no ref taken.
+ */
+static int get_paged_frame(unsigned long gfn, struct page_info **page,
+   bool readonly, struct domain *rd)
 {
 int rc = GNTST_okay;
 #if defined(P2M_PAGED_TYPES) || defined(P2M_SHARED_TYPES)
@@ -273,7 +273,6 @@ static int get_paged_frame(unsigned long gfn, unsigned long 
*frame,
   (readonly) ? P2M_ALLOC : P2M_UNSHARE);
 if ( !(*page) )
 {
-*frame = mfn_x(INVALID_MFN);
 if ( p2m_is_shared(p2mt) )
 return GNTST_eagain;
 if ( p2m_is_paging(p2mt) )
@@ -283,13 +282,12 @@ static int get_paged_frame(unsigned long gfn, unsigned 
long *frame,
 }
 return GNTST_bad_page;
 }
-*frame = page_to_mfn(*page);
 #else
-*frame = mfn_x(gfn_to_mfn(rd, _gfn(gfn)));
-*page = mfn_valid(_mfn(*frame)) ? mfn_to_page(*frame) : NULL;
+mfn_t mfn = gfn_to_mfn(rd, _gfn(gfn));
+
+*page = mfn_valid(mfn) ? mfn_to_page(mfn_x(mfn)) : NULL;
 if ( (!(*page)) || (!get_page(*page, rd)) )
 {
-*frame = mfn_x(INVALID_MFN);
 *page = NULL;
 rc = GNTST_bad_page;
 }
@@ -804,7 +802,7 @@ map_grant_ref(
 struct grant_table *lgt, *rgt;
 struct vcpu   *led;
 grant_handle_t handle;
-unsigned long  frame = 0;
+unsigned long  frame;
 struct page_info *pg = NULL;
 intrc = GNTST_okay;
 u32old_pin;
@@ -896,13 +894,12 @@ map_grant_ref(
 shared_entry_v1(rgt, op->ref).frame :
 shared_entry_v2(rgt, op->ref).full_page.frame;
 
-rc = get_paged_frame(gfn, , ,
- op->flags & GNTMAP_readonly, rd);
+rc = get_paged_frame(gfn, , op->flags & GNTMAP_readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
 act_set_gfn(act, _gfn(gfn));
 act->domid = ld->domain_id;
-act->frame = frame;
+act->frame = page_to_mfn(pg);
 act->start = 0;
 act->length = PAGE_SIZE;
 act->is_sub_page = false;
@@ -2163,7 +2160,6 @@ acquire_grant_for_copy(
 domid_t trans_domid;
 grant_ref_t trans_gref;
 struct domain *td;
-unsigned long frame;
 uint16_t trans_page_off;
 uint16_t trans_length;
 bool is_sub_page;
@@ -2256,8 +2252,6 @@ acquire_grant_for_copy(
 return rc;
 }
 
-frame = page_to_mfn(*page);
-
 /*
  * We dropped the lock, so we have to check that the grant didn't
  * change, and that nobody else tried to pin/unpin it. If anything
@@ -2265,7 +2259,8 @@ acquire_grant_for_copy(
  */
 if ( rgt->gt_version != 2 ||
  act->pin != old_pin ||
- (old_pin && (act->domid != ldom || act->frame != frame ||
+ (old_pin && (act->domid != ldom ||
+  act->frame != page_to_mfn(*page) ||
   act->start != trans_page_off ||
   act->length != trans_length ||
   act->trans_domain != td ||
@@ -2289,7 +2284,7 @@ acquire_grant_for_copy(
 act->length = trans_length;
 act->trans_domain = td;
 act->trans_gref = trans_gref;
-act->frame = frame;
+act->frame = page_to_mfn(*page);
 act_set_gfn(act, INVALID_GFN);
 /*
  * The actual remote remote grant may or may not be a sub-page,
@@ -2313,7 +2308,7 @@ acquire_grant_for_copy(
 {
 unsigned 

[Xen-devel] [PATCH 3/3] gnttab: Drop the frame field from struct gnttab_copy_buf

[Andrew Cooper]

It is redundant with *page.

Signed-off-by: Andrew Cooper 
---
CC: George Dunlap 
CC: Jan Beulich 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 
---
 xen/common/grant_table.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index d8307b7..bef3b26 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2393,7 +2393,6 @@ struct gnttab_copy_buf {
 
 /* Mapped etc. */
 struct domain *domain;
-unsigned long frame;
 struct page_info *page;
 void *virt;
 bool_t read_only;
@@ -2502,7 +2501,6 @@ static int gnttab_copy_claim_buf(const struct gnttab_copy 
*op,
 >ptr.offset, >len, true);
 if ( rc != GNTST_okay )
 goto out;
-buf->frame = page_to_mfn(buf->page);
 buf->ptr.u.ref = ptr->u.ref;
 buf->have_grant = 1;
 }
@@ -2514,7 +2512,6 @@ static int gnttab_copy_claim_buf(const struct gnttab_copy 
*op,
 PIN_FAIL(out, rc,
  "source frame %"PRI_xen_pfn" invalid.\n", ptr->u.gmfn);
 
-buf->frame = page_to_mfn(buf->page);
 buf->ptr.u.gmfn = ptr->u.gmfn;
 buf->ptr.offset = 0;
 buf->len = PAGE_SIZE;
@@ -2525,14 +2522,15 @@ static int gnttab_copy_claim_buf(const struct 
gnttab_copy *op,
 if ( !get_page_type(buf->page, PGT_writable_page) )
 {
 if ( !buf->domain->is_dying )
-gdprintk(XENLOG_WARNING, "Could not get writable frame %lx\n", 
buf->frame);
+gdprintk(XENLOG_WARNING, "Could not get writable mfn 
%"PRI_mfn"\n",
+ page_to_mfn(buf->page));
 rc = GNTST_general_error;
 goto out;
 }
 buf->have_type = 1;
 }
 
-buf->virt = map_domain_page(_mfn(buf->frame));
+buf->virt = __map_domain_page(buf->page);
 rc = GNTST_okay;
 
  out:
@@ -2576,7 +2574,7 @@ static int gnttab_copy_buf(const struct gnttab_copy *op,
 
 memcpy(dest->virt + op->dest.offset, src->virt + op->source.offset,
op->len);
-gnttab_mark_dirty(dest->domain, dest->frame);
+gnttab_mark_dirty(dest->domain, page_to_mfn(dest->page));
 rc = GNTST_okay;
  out:
 return rc;
-- 
2.1.4


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

[Xen-devel] [PATCH 0/3] gnttab: Further XSA-226 cleanup

[Andrew Cooper]

Andrew Cooper (3):
  gnttab: Drop the frame parameter from acquire_grant_for_copy()
  gnttab: Drop the frame parameter from get_paged_frame()
  gnttab: Drop the frame field from struct gnttab_copy_buf

 xen/common/grant_table.c | 80 ++--
 1 file changed, 37 insertions(+), 43 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH 1/3] gnttab: Drop the frame parameter from acquire_grant_for_copy()

[Andrew Cooper]

It is redundant with the *page parameter.  Rename the grant_frame parameter to
indicate that it is local, and highlight the correctness of the change.

Signed-off-by: Andrew Cooper 
---
CC: George Dunlap 
CC: Jan Beulich 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 
---
 xen/common/grant_table.c | 42 ++
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 36895aa..188c477 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2142,15 +2142,17 @@ static void fixup_status_for_copy_pin(const struct 
active_grant_entry *act,
 gnttab_clear_flag(_GTF_reading, status);
 }
 
-/* Grab a frame number from a grant entry and update the flags and pin
-   count as appropriate. If rc == GNTST_okay, note that this *does*
-   take one ref count on the target page, stored in *page.
-   If there is any error, *page = NULL, no ref taken. */
+/*
+ * Grab a frame number from a grant entry and update the flags and pin
+ * count as appropriate. If rc == GNTST_okay, note that this *does*
+ * take one ref count on the target page, stored in *page.
+ * If there is any error, *page = NULL, no ref taken.
+ */
 static int
 acquire_grant_for_copy(
 struct domain *rd, grant_ref_t gref, domid_t ldom, bool readonly,
-unsigned long *frame, struct page_info **page,
-uint16_t *page_off, uint16_t *length, bool allow_transitive)
+struct page_info **page, uint16_t *page_off, uint16_t *length,
+bool allow_transitive)
 {
 struct grant_table *rgt = rd->grant_table;
 grant_entry_v2_t *sha2;
@@ -2161,7 +2163,7 @@ acquire_grant_for_copy(
 domid_t trans_domid;
 grant_ref_t trans_gref;
 struct domain *td;
-unsigned long grant_frame;
+unsigned long frame;
 uint16_t trans_page_off;
 uint16_t trans_length;
 bool is_sub_page;
@@ -2238,10 +2240,9 @@ acquire_grant_for_copy(
 active_entry_release(act);
 grant_read_unlock(rgt);
 
-rc = acquire_grant_for_copy(td, trans_gref, rd->domain_id,
-readonly, _frame, page,
-_page_off, _length,
-false);
+rc = acquire_grant_for_copy(
+td, trans_gref, rd->domain_id, readonly, page, _page_off,
+_length, false);
 
 grant_read_lock(rgt);
 act = active_entry_acquire(rgt, gref);
@@ -2255,6 +2256,8 @@ acquire_grant_for_copy(
 return rc;
 }
 
+frame = page_to_mfn(*page);
+
 /*
  * We dropped the lock, so we have to check that the grant didn't
  * change, and that nobody else tried to pin/unpin it. If anything
@@ -2262,7 +2265,7 @@ acquire_grant_for_copy(
  */
 if ( rgt->gt_version != 2 ||
  act->pin != old_pin ||
- (old_pin && (act->domid != ldom || act->frame != grant_frame ||
+ (old_pin && (act->domid != ldom || act->frame != frame ||
   act->start != trans_page_off ||
   act->length != trans_length ||
   act->trans_domain != td ||
@@ -2286,7 +2289,7 @@ acquire_grant_for_copy(
 act->length = trans_length;
 act->trans_domain = td;
 act->trans_gref = trans_gref;
-act->frame = grant_frame;
+act->frame = frame;
 act_set_gfn(act, INVALID_GFN);
 /*
  * The actual remote remote grant may or may not be a sub-page,
@@ -2310,7 +2313,7 @@ acquire_grant_for_copy(
 {
 unsigned long gfn = shared_entry_v1(rgt, gref).frame;
 
-rc = get_paged_frame(gfn, _frame, page, readonly, rd);
+rc = get_paged_frame(gfn, , page, readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
 act_set_gfn(act, _gfn(gfn));
@@ -2320,7 +2323,7 @@ acquire_grant_for_copy(
 }
 else if ( !(sha2->hdr.flags & GTF_sub_page) )
 {
-rc = get_paged_frame(sha2->full_page.frame, _frame, page,
+rc = get_paged_frame(sha2->full_page.frame, , page,
  readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
@@ -2331,7 +2334,7 @@ acquire_grant_for_copy(
 }
 else
 {
-rc = get_paged_frame(sha2->sub_page.frame, _frame, page,
+rc = get_paged_frame(sha2->sub_page.frame, , page,
  readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
@@ -2349,7 +2352,7 @@ acquire_grant_for_copy(
 act->length = trans_length;
 act->trans_domain 

Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Andrew Cooper]

On 24/08/17 18:45, Juergen Gross wrote:
> On 24/08/17 17:16, Andrew Cooper wrote:
>> On 24/08/17 16:01, Juergen Gross wrote:
>>> On 24/08/17 16:50, Andrew Cooper wrote:
 This patch was originally a workaround for XSA-226.  Since then, transitive
 grants are believed to be functioning properly, and the defaults have 
 changed
 appropriately.

 However, for those people who chose to use the workaround (especially from 
 an
 attack surface mitigation point of view), retain the ability for the host
 administrator to choose.

 Signed-off-by: Andrew Cooper 
 ---
 CC: Jan Beulich 
 CC: George Dunlap 
 CC: Konrad Rzeszutek Wilk 
 CC: Stefano Stabellini 
 CC: Tim Deegan 
 CC: Wei Liu 
 ---
  docs/misc/xen-command-line.markdown | 13 +++
  xen/common/grant_table.c| 44 
 +++--
  2 files changed, 55 insertions(+), 2 deletions(-)

 diff --git a/docs/misc/xen-command-line.markdown 
 b/docs/misc/xen-command-line.markdown
 index 4002eab..78c7b51 100644
 --- a/docs/misc/xen-command-line.markdown
 +++ b/docs/misc/xen-command-line.markdown
 @@ -868,6 +868,19 @@ Controls EPT related features.
  
  Specify which console gdbstub should use. See **console**.
  
 +### gnttab
 +> `= List of [ max_ver:, transitive ]`
 +
 +> Default: `gnttab=max_ver:2,transitive`
 +
 +Control various aspects of the grant table behaviour available to guests.
 +
 +* `max_ver` Select the maximum grant table version to offer to guests.  
 Valid
 +version are 1 and 2.
 +* `transitive` Permit or disallow the use of transitive grants.  Note 
 that the
 +use of grant table v2 without transitive grants is an ABI breakage from 
 the
 +guests point of view.
>>> So shouldn't there be a way for the guest to query the support of
>>> transient grants?
>> Ideally yes, but how do you suggest doing this in a compatible way?
> An ELF note in the guest kernel indicating it is aware of that
> possibility in order to allow v2 grants for that guest without transient
> grants?
>
>> All Xen downstreams which haven't backported the eventual transitive
>> fixes will have this clobber in place, without any query-ability.
> So the only really compatible way would be to disallow v2 grants. OTOH
> I guess there aren't so many guests using v2 right now...
>
>> The reason this workaround was so effective, and why several large users
>> still wish to clobber them, is because nothing uses transitive grants.
> Right. And only very few guests use v2 grants.

We tried disabling gnttab v2 by default first, which is why the max_ver:
parameter is present in this patch.

Some vendor versions of Linux refused to fall back from gnttab v2 to
gnttab v1 on migrate, even though they are perfectly happy booting in a
v1-only situation.

Also
https://github.com/xenserver/win-xenbus/blob/c2a60d437b42f2349361629f15275e4fabdcc22e/src/xenbus/gnttab.c#L566-L571
which was discovered out of the blue, when all windows guests installed
on older versions of XenServer started turning blue.


Leaving v2 active and creating an ABI breakage turned out to be the
viable way to inhibit the use of transitive grants in cases which needed
to run arbitrary guests.

>
 +
  ### gnttab\_max\_frames
  > `= `
  
 diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
 index 36895aa..f9c313d 100644
 --- a/xen/common/grant_table.c
 +++ b/xen/common/grant_table.c
 @@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", 
 max_nr_grant_frames);
  unsigned int __read_mostly max_grant_frames;
  integer_param("gnttab_max_frames", max_grant_frames);
  
 +static unsigned int __read_mostly opt_gnttab_max_version = 2;
 +static bool __read_mostly opt_transitive_grants = true;
 +
 +static void __init parse_gnttab(char *s)
>>> Do you mind using:
>>>
>>> static int __init parse_gnttab(const char *s)
>>>
>>> in order to comply with my runtime parameter series?
>> If the result will still compile.  What should the semantics be?  (I've
>> had a quick look through your series, but I can't see the semantics
>> stated anywhere obvious)
> The parsing routine must not change the parameter string and should
> return an error (e.g. -EINVAL) in case of an illegal parameter.

Let me see what I can do.

~Andrew

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

[Xen-devel] [xen-4.9-testing test] 112850: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

flight 112850 xen-4.9-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/112850/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
112820

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 12 guest-start  fail REGR. vs. 112820
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stopfail REGR. vs. 112820

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64-xsm   2 hosts-allocate  broken like 112820
 build-arm64-pvops 2 hosts-allocate  broken like 112820
 build-arm64   2 hosts-allocate  broken like 112820
 build-arm64-xsm   3 capture-logs broken never pass
 build-arm64-pvops 3 capture-logs broken never pass
 build-arm64   3 capture-logs broken never pass
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 112820
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 112820
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail 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-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-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-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  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-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail 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-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-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-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail 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-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 xen  5ff1de3e4f56b2dd7c5c7dae8b008f6ee6dc2081
baseline version:
 xen  9bf14bbf990843bfec16a5d69d36cf46c7593d88

Last test of basis   112820  2017-08-22 11:15:27 Z2 days
Testing same since   112850  2017-08-23 16:14:09 Z1 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Doug Goldstein 
  Jan Beulich 

Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Juergen Gross]

On 24/08/17 17:16, Andrew Cooper wrote:
> On 24/08/17 16:01, Juergen Gross wrote:
>> On 24/08/17 16:50, Andrew Cooper wrote:
>>> This patch was originally a workaround for XSA-226.  Since then, transitive
>>> grants are believed to be functioning properly, and the defaults have 
>>> changed
>>> appropriately.
>>>
>>> However, for those people who chose to use the workaround (especially from 
>>> an
>>> attack surface mitigation point of view), retain the ability for the host
>>> administrator to choose.
>>>
>>> Signed-off-by: Andrew Cooper 
>>> ---
>>> CC: Jan Beulich 
>>> CC: George Dunlap 
>>> CC: Konrad Rzeszutek Wilk 
>>> CC: Stefano Stabellini 
>>> CC: Tim Deegan 
>>> CC: Wei Liu 
>>> ---
>>>  docs/misc/xen-command-line.markdown | 13 +++
>>>  xen/common/grant_table.c| 44 
>>> +++--
>>>  2 files changed, 55 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/misc/xen-command-line.markdown 
>>> b/docs/misc/xen-command-line.markdown
>>> index 4002eab..78c7b51 100644
>>> --- a/docs/misc/xen-command-line.markdown
>>> +++ b/docs/misc/xen-command-line.markdown
>>> @@ -868,6 +868,19 @@ Controls EPT related features.
>>>  
>>>  Specify which console gdbstub should use. See **console**.
>>>  
>>> +### gnttab
>>> +> `= List of [ max_ver:, transitive ]`
>>> +
>>> +> Default: `gnttab=max_ver:2,transitive`
>>> +
>>> +Control various aspects of the grant table behaviour available to guests.
>>> +
>>> +* `max_ver` Select the maximum grant table version to offer to guests.  
>>> Valid
>>> +version are 1 and 2.
>>> +* `transitive` Permit or disallow the use of transitive grants.  Note that 
>>> the
>>> +use of grant table v2 without transitive grants is an ABI breakage from the
>>> +guests point of view.
>> So shouldn't there be a way for the guest to query the support of
>> transient grants?
> 
> Ideally yes, but how do you suggest doing this in a compatible way?

An ELF note in the guest kernel indicating it is aware of that
possibility in order to allow v2 grants for that guest without transient
grants?

> All Xen downstreams which haven't backported the eventual transitive
> fixes will have this clobber in place, without any query-ability.

So the only really compatible way would be to disallow v2 grants. OTOH
I guess there aren't so many guests using v2 right now...

> The reason this workaround was so effective, and why several large users
> still wish to clobber them, is because nothing uses transitive grants.

Right. And only very few guests use v2 grants.

> 
>>
>>> +
>>>  ### gnttab\_max\_frames
>>>  > `= `
>>>  
>>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>>> index 36895aa..f9c313d 100644
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", 
>>> max_nr_grant_frames);
>>>  unsigned int __read_mostly max_grant_frames;
>>>  integer_param("gnttab_max_frames", max_grant_frames);
>>>  
>>> +static unsigned int __read_mostly opt_gnttab_max_version = 2;
>>> +static bool __read_mostly opt_transitive_grants = true;
>>> +
>>> +static void __init parse_gnttab(char *s)
>> Do you mind using:
>>
>> static int __init parse_gnttab(const char *s)
>>
>> in order to comply with my runtime parameter series?
> 
> If the result will still compile.  What should the semantics be?  (I've
> had a quick look through your series, but I can't see the semantics
> stated anywhere obvious)

The parsing routine must not change the parameter string and should
return an error (e.g. -EINVAL) in case of an illegal parameter.


Juergen


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

Re: [Xen-devel] [PATCH v4 10/11] public: add XENFEAT_ARM_SMCCC_supported feature

[Julien Grall]

On 21/08/17 21:27, Volodymyr Babchuk wrote:

This feature indicates that hypervisor is compatible with ARM
SMC calling convention. Hypervisor will not inject an undefined
instruction exception if an invalid SMC function were called and
will not crash a domain if an invlalid HVC functions were called.


s/invlalid/invalid/

The last sentence is misleading. Xen will still inject and undefined 
instruction for some SMC/HVC. You may want to rework it to make it clear.




Signed-off-by: Volodymyr Babchuk 
---
 xen/include/public/features.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/public/features.h b/xen/include/public/features.h
index 2110b04..1a989b8 100644
--- a/xen/include/public/features.h
+++ b/xen/include/public/features.h
@@ -102,6 +102,9 @@
 /* Guest can use XENMEMF_vnode to specify virtual node for memory op. */
 #define XENFEAT_memory_op_vnode_supported 13

+/* arm: Hypervisor supports ARM SMC calling convention. */
+#define XENFEAT_ARM_SMCCC_supported   14
+
 #define XENFEAT_NR_SUBMAPS 1

 #endif /* __XEN_PUBLIC_FEATURES_H__ */



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 11/11] arm: enable XENFEAT_ARM_SMCCC_supported feature

[Julien Grall]

On 22/08/17 08:29, Jan Beulich wrote:

On 21.08.17 at 22:27,  wrote:

As Xen now supports SMCCC, we can enable this feature to tell
guests that it is safe to call generic SMCs and HVCs.


I think this and patch 10 should be folded.


+1.




Signed-off-by: Volodymyr Babchuk 
---
 xen/common/kernel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index ce7cb8a..18c4d51 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -332,6 +332,9 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 (1U << XENFEAT_auto_translated_physmap);
 if ( is_hardware_domain(d) )
 fi.submap |= 1U << XENFEAT_dom0;
+#ifdef CONFIG_ARM
+fi.submap |= (1U << XENFEAT_ARM_SMCCC_supported);
+#endif


Pointless parentheses (as other code in context shows you).

With both aspects taken care of
Acked-by: Jan Beulich 

Jan



--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 08/11] arm: PSCI: use definitions provided by asm/smccc.h

[Julien Grall]

Hi Volodymyr,

On 21/08/17 21:27, Volodymyr Babchuk wrote:

smccc.h provides definitions to construct SMC call function number according
to SMCCC. We don't need multiple definitions for one thing, and definitions
in smccc.h are more generic than ones used in psci.h.

So psci.h will only provide function codes, while whole SMC function
identifier will be constructed using generic macros from smccc.h.

PSCI_0_2_FN_xxx was deliberately renamed to PSCI_0_2_FUNC_xxx, because this
is a new entity. It can lead to problems, if we'll just change value of
PSCI_0_2_FN_xxx without renaming it.


I don't think "new entity" is a good reason to rename them. And the 
previous naming was kind of nice to read. More that you still use 
PSCI_0_2_FN{32,64}. We should definitely stay consistent in naming.


So what is the exact problem? Is it because you are worry to miss some 
of them?




This change also affects vsmc.c and seattle.c, because they both use PSCI
function numbers.


This paragraph could be dropped.



Signed-off-by: Volodymyr Babchuk 
---

 * Reworked definitions to minimize their lenght
 * Described why I replaced PSCI_0_2_FN_xxx with PSCI_0_2_FUNC_xxx

---
 xen/arch/arm/platforms/seattle.c |  5 +++--
 xen/arch/arm/psci.c  | 10 -
 xen/arch/arm/vsmc.c  | 24 +++---
 xen/include/asm-arm/psci.h   | 44 ++--
 4 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
index 86dce91..fb2ad13 100644
--- a/xen/arch/arm/platforms/seattle.c
+++ b/xen/arch/arm/platforms/seattle.c
@@ -19,6 +19,7 @@

 #include 
 #include 
+#include 


Again, vsmc.h stands for "virtual SMC". Here you use for the "physical 
SMC". So please don't include vsmc.h here.




 static const char * const seattle_dt_compat[] __initconst =
 {
@@ -33,12 +34,12 @@ static const char * const seattle_dt_compat[] __initconst =
  */
 static void seattle_system_reset(void)
 {
-call_smc(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
+call_smc(PSCI_0_2_FN32(SYSTEM_RESET), 0, 0, 0);
 }

 static void seattle_system_off(void)
 {
-call_smc(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
+call_smc(PSCI_0_2_FN32(SYSTEM_OFF), 0, 0, 0);
 }

 PLATFORM_START(seattle, "SEATTLE")
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 34ee97e..645fe58 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -31,9 +31,9 @@
  * (native-width) function ID.
  */
 #ifdef CONFIG_ARM_64
-#define PSCI_0_2_FN_NATIVE(name)   PSCI_0_2_FN64_##name
+#define PSCI_0_2_FN_NATIVE(name)   PSCI_0_2_FN64(name)
 #else
-#define PSCI_0_2_FN_NATIVE(name)   PSCI_0_2_FN_##name
+#define PSCI_0_2_FN_NATIVE(name)   PSCI_0_2_FN32(name)
 #endif

 uint32_t psci_ver;
@@ -48,13 +48,13 @@ int call_psci_cpu_on(int cpu)
 void call_psci_system_off(void)
 {
 if ( psci_ver > PSCI_VERSION(0, 1) )
-call_smc(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
+call_smc(PSCI_0_2_FN32(SYSTEM_OFF), 0, 0, 0);
 }

 void call_psci_system_reset(void)
 {
 if ( psci_ver > PSCI_VERSION(0, 1) )
-call_smc(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
+call_smc(PSCI_0_2_FN32(SYSTEM_RESET), 0, 0, 0);
 }

 int __init psci_is_smc_method(const struct dt_device_node *psci)
@@ -144,7 +144,7 @@ int __init psci_init_0_2(void)
 }
 }

-psci_ver = call_smc(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
+psci_ver = call_smc(PSCI_0_2_FN32(PSCI_VERSION), 0, 0, 0);

 /* For the moment, we only support PSCI 0.2 and PSCI 1.x */
 if ( psci_ver != PSCI_VERSION(0, 2) && PSCI_VERSION_MAJOR(psci_ver) != 1 )
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 956d4ef..46a2fde 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -102,7 +102,7 @@ static bool handle_psci_0_x(struct cpu_user_regs *regs)
 /* helper function for checking arm mode 32/64 bit */
 static inline int psci_mode_check(struct domain *d, register_t fid)
 {
-return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) );
+return is_64bit_domain(d) || !ARM_SMCCC_IS_64(fid);


I don't see any reason to do the renaming in psci_mode_check given that 
you will remove this function in the next patch.



 }

 /* PSCI 0.2 interface and other Standard Secure Calls */
@@ -112,34 +112,34 @@ static bool handle_sssc(struct cpu_user_regs *regs)

 switch ( ARM_SMCCC_FUNC_NUM(fid) )
 {
-case ARM_SMCCC_FUNC_NUM(PSCI_0_2_FN_PSCI_VERSION):
+case PSCI_0_2_FUNC_PSCI_VERSION:
 perfc_incr(vpsci_version);
 PSCI_SET_RESULT(regs, do_psci_0_2_version());
 return true;
-case ARM_SMCCC_FUNC_NUM(PSCI_0_2_FN_CPU_OFF):
+case PSCI_0_2_FUNC_CPU_OFF:
 perfc_incr(vpsci_cpu_off);
 PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off());
 return true;
-case ARM_SMCCC_FUNC_NUM(PSCI_0_2_FN_MIGRATE_INFO_TYPE):
+case PSCI_0_2_FUNC_MIGRATE_INFO_TYPE:
 perfc_incr(vpsci_migrate_info_type);
 

[Xen-devel] [xen-unstable-smoke test] 112861: tolerable trouble: broken/pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64-pvops 2 hosts-allocate  broken like 112847
 build-arm64   2 hosts-allocate  broken like 112847
 build-arm64-pvops 3 capture-logsbroken like 112847
 build-arm64   3 capture-logsbroken like 112847
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  8f7d1b08a1ed0583ece262b3abf6a0f093b764bb
baseline version:
 xen  98df75f2782e47c47002d57ca5c5832de4e903fc

Last test of basis   112847  2017-08-23 16:01:33 Z1 days
Testing same since   112861  2017-08-24 16:04:16 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Julien Grall 
  Tim Deegan 
  Wei Liu 

jobs:
 build-amd64  pass
 build-arm64  broken  
 build-armhf  pass
 build-amd64-libvirt  pass
 build-arm64-pvopsbroken  
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  broken  
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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

broken-step build-arm64-pvops hosts-allocate
broken-step build-arm64 hosts-allocate
broken-step build-arm64-pvops capture-logs
broken-step build-arm64 capture-logs

Pushing revision :

+ branch=xen-unstable-smoke
+ revision=8f7d1b08a1ed0583ece262b3abf6a0f093b764bb
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
8f7d1b08a1ed0583ece262b3abf6a0f093b764bb
+ branch=xen-unstable-smoke
+ revision=8f7d1b08a1ed0583ece262b3abf6a0f093b764bb
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.9-testing
+ '[' x8f7d1b08a1ed0583ece262b3abf6a0f093b764bb = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : 

Re: [Xen-devel] [PATCH v2 REPOST 11/12] x86/hvm/ioreq: defer mapping gfns until they are actually requsted

[Roger Pau Monné]

On Tue, Aug 22, 2017 at 03:51:05PM +0100, Paul Durrant wrote:
> A subsequent patch will introduce a new scheme to allow an emulator to
> map IOREQ server pages directly from Xen rather than the guest P2M.
> 
> This patch lays the groundwork for that change by deferring mapping of
> gfns until their values are requested by an emulator. To that end, the
> pad field of the xen_dm_op_get_ioreq_server_info structure is re-purposed
> to a flags field and new flag, XEN_DMOP_no_gfns, defined which modifies the
> behaviour of XEN_DMOP_get_ioreq_server_info to allow the caller to avoid
> requesting the gfn values.
> 
> Signed-off-by: Paul Durrant 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Jan Beulich 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> ---
>  tools/libs/devicemodel/core.c   |  8 +
>  tools/libs/devicemodel/include/xendevicemodel.h |  6 ++--
>  xen/arch/x86/hvm/dm.c   |  9 +++--
>  xen/arch/x86/hvm/ioreq.c| 44 
> ++---
>  xen/include/asm-x86/hvm/domain.h|  2 +-
>  xen/include/public/hvm/dm_op.h  | 32 ++
>  6 files changed, 61 insertions(+), 40 deletions(-)
> 
> diff --git a/tools/libs/devicemodel/core.c b/tools/libs/devicemodel/core.c
> index fcb260d29b..907c894e77 100644
> --- a/tools/libs/devicemodel/core.c
> +++ b/tools/libs/devicemodel/core.c
> @@ -188,6 +188,14 @@ int xendevicemodel_get_ioreq_server_info(
>  
>  data->id = id;
>  
> +/*
> + * If the caller is not requesting gfn values then instruct the
> + * hypercall not to retrieve them as this may cause them to be
> + * mapped.
> + */
> +if (!ioreq_gfn && !bufioreq_gfn)
> +data->flags = XEN_DMOP_no_gfns;

Since this is memset to 0 I would use |= here, in case someone adds a
new flag further up.

Also, seeing the code below, don't you need to retry on error in case
you are dealing with an old hypervisor version? (that will choke on pad
being set).

> diff --git a/xen/include/asm-x86/hvm/domain.h 
> b/xen/include/asm-x86/hvm/domain.h
> index 7b93d10209..b8bcd559a5 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -70,7 +70,7 @@ struct hvm_ioreq_server {
>  evtchn_port_t  bufioreq_evtchn;
>  struct rangeset*range[NR_IO_RANGE_TYPES];

I would place bufioreq_handling here in order to have a more compact
structure layout.

>  bool   enabled;
> -bool   bufioreq_atomic;
> +intbufioreq_handling;
>  bool   is_default;
>  };
>  
> diff --git a/xen/include/public/hvm/dm_op.h b/xen/include/public/hvm/dm_op.h
> index 6bbab5fca3..9677bd74e7 100644
> --- a/xen/include/public/hvm/dm_op.h
> +++ b/xen/include/public/hvm/dm_op.h
> @@ -79,28 +79,34 @@ struct xen_dm_op_create_ioreq_server {
>   * XEN_DMOP_get_ioreq_server_info: Get all the information necessary to
>   * access IOREQ Server .
>   *
> - * The emulator needs to map the synchronous ioreq structures and buffered
> - * ioreq ring (if it exists) that Xen uses to request emulation. These are
> - * hosted in the target domain's gmfns  and 
> - * respectively. In addition, if the IOREQ Server is handling buffered
> - * emulation requests, the emulator needs to bind to event channel
> - *  to listen for them. (The event channels used for
> - * synchronous emulation requests are specified in the per-CPU ioreq
> - * structures in ).
> - * If the IOREQ Server is not handling buffered emulation requests then the
> - * values handed back in  and  will both be 0.
> + * If the IOREQ Server is handling buffered emulation requests, the
> + * emulator needs to bind to event channel  to listen for
> + * them. (The event channels used for synchronous emulation requests are
> + * specified in the per-CPU ioreq structures).
> + * In addition, if the XENMEM_acquire_resource memory op cannot be used,
> + * the emulator will need to map the synchronous ioreq structures and
> + * buffered ioreq ring (if it exists) from guest memory. If  does
> + * not contain XEN_DMOP_no_gfns then these pages will be made available and
> + * the frame numbers passed back in gfns  and 
> + * respectively. (If the IOREQ Server is not handling buffered emulation
> + * only  will be valid).
>   */
>  #define XEN_DMOP_get_ioreq_server_info 2
>  
>  struct xen_dm_op_get_ioreq_server_info {
>  /* IN - server id */
>  ioservid_t id;
> -uint16_t pad;
> +/* IN - flags */
> +uint16_t flags;

Don't you need to bump some version or similar to let the consumers
know this field is now available? Or that's done 

Re: [Xen-devel] [PATCH v2 REPOST 10/12] x86/hvm/ioreq: use gfn_t in struct hvm_ioreq_page

[Roger Pau Monné]

On Tue, Aug 22, 2017 at 03:51:04PM +0100, Paul Durrant wrote:
> This patch adjusts the IOREQ server code to use type-safe gfn_t values
> where possible. No functional change.

Oh, forget my comment on the previous patch then.

Thanks.

> Signed-off-by: Paul Durrant 

Reviewed-by: Roger Pau Monné 

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

Re: [Xen-devel] [PATCH v2 REPOST 09/12] x86/hvm/ioreq: simplify code and use consistent naming

[Roger Pau Monné]

On Tue, Aug 22, 2017 at 03:51:03PM +0100, Paul Durrant wrote:
> This patch re-works much of the IOREQ server initialization and teardown
> code:
> 
> - The hvm_map/unmap_ioreq_gfn() functions are expanded to call through
>   to hvm_alloc/free_ioreq_gfn() rather than expecting them to be called
>   separately by outer functions.
> - Several functions now test the validity of the hvm_ioreq_page gfn value
>   to determine whether they need to act. This means can be safely called
>   for the bufioreq page even when it is not used.
> - hvm_add/remove_ioreq_gfn() simply return in the case of the default
>   IOREQ server so callers no longer need to test before calling.
> - hvm_ioreq_server_setup_pages() is renamed to hvm_ioreq_server_map_pages()
>   to mirror the existing hvm_ioreq_server_unmap_pages().
> 
> All of this significantly shortens the code.
> 
> Signed-off-by: Paul Durrant 
> ---
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> ---
>  xen/arch/x86/hvm/ioreq.c | 181 
> ++-
>  1 file changed, 69 insertions(+), 112 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index 5737082238..edfb394c59 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -181,63 +181,76 @@ bool handle_hvm_io_completion(struct vcpu *v)
>  return true;
>  }
>  
> -static int hvm_alloc_ioreq_gfn(struct domain *d, unsigned long *gfn)
> +static unsigned long hvm_alloc_ioreq_gfn(struct hvm_ioreq_server *s)

gfn_t as the return type instead? I see that you are moving it, so I
won't insist (I assume there's also some other refactoring involved in
making this return gfn_t). I see there are also further uses of
unsigned long to store gfns, I'm not going to point those out.

>  {
> +struct domain *d = s->domain;
>  unsigned int i;
> -int rc;
>  
> -rc = -ENOMEM;
> +ASSERT(!s->is_default);
> +
>  for ( i = 0; i < sizeof(d->arch.hvm_domain.ioreq_gfn.mask) * 8; i++ )
>  {
>  if ( test_and_clear_bit(i, >arch.hvm_domain.ioreq_gfn.mask) )
>  {

The braces are not strictly needed anymore, but that's a question of
taste.

> -*gfn = d->arch.hvm_domain.ioreq_gfn.base + i;
> -rc = 0;
> -break;
> +return d->arch.hvm_domain.ioreq_gfn.base + i;
>  }
>  }
>  
> -return rc;
> +return gfn_x(INVALID_GFN);
>  }
>  
> -static void hvm_free_ioreq_gfn(struct domain *d, unsigned long gfn)
> +static void hvm_free_ioreq_gfn(struct hvm_ioreq_server *s,
> +   unsigned long gfn)
>  {
> +struct domain *d = s->domain;
>  unsigned int i = gfn - d->arch.hvm_domain.ioreq_gfn.base;
>  
> -if ( gfn != gfn_x(INVALID_GFN) )
> -set_bit(i, >arch.hvm_domain.ioreq_gfn.mask);
> +ASSERT(!s->is_default);

I would maybe add a gfn != gfn_x(INVALID_GFN) in the ASSERT.

> +
> +set_bit(i, >arch.hvm_domain.ioreq_gfn.mask);
>  }
>  
> -static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, bool buf)
> +static void hvm_unmap_ioreq_gfn(struct hvm_ioreq_server *s, bool buf)

I'm not sure if you need the buf parameter, it seems in all cases you
want to unmap everything when calling hvm_unmap_ioreq_gfn? (same
applies to hvm_remove_ioreq_gfn and hvm_add_ioreq_gfn)

>  static int hvm_ioreq_server_map_pages(struct hvm_ioreq_server *s,
> -  unsigned long ioreq_gfn,
> -  unsigned long bufioreq_gfn)
> -{
> -int rc;
> -
> -rc = hvm_map_ioreq_page(s, false, ioreq_gfn);
> -if ( rc )
> -return rc;
> -
> -if ( bufioreq_gfn != gfn_x(INVALID_GFN) )
> -rc = hvm_map_ioreq_page(s, true, bufioreq_gfn);
> -
> -if ( rc )
> -hvm_unmap_ioreq_page(s, false);
> -
> -return rc;
> -}
> -
> -static int hvm_ioreq_server_setup_pages(struct hvm_ioreq_server *s,
> -bool handle_bufioreq)
> +  bool handle_bufioreq)
>  {
> -struct domain *d = s->domain;
> -unsigned long ioreq_gfn = gfn_x(INVALID_GFN);
> -unsigned long bufioreq_gfn = gfn_x(INVALID_GFN);
> -int rc;
> -
> -if ( s->is_default )
> -{
> -/*
> - * The default ioreq server must handle buffered ioreqs, for
> - * backwards compatibility.
> - */
> -ASSERT(handle_bufioreq);
> -return hvm_ioreq_server_map_pages(s,
> -   d->arch.hvm_domain.params[HVM_PARAM_IOREQ_PFN],
> -   d->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_PFN]);
> -}
> +int rc = -ENOMEM;

No need to set rc, you are just overwriting it in the next line.

Roger.

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

Re: [Xen-devel] [PATCH v4 07/11] arm: traps: handle PSCI calls inside `vsmc.c`

[Julien Grall]

Hi Volodymyr,

On 21/08/17 21:27, Volodymyr Babchuk wrote:

PSCI is part of HVC/SMC interface, so it should be handled in
appropriate place: `vsmc.c`. This patch just moves PSCI
handler calls from `traps.c` to `vsmc.c`.

Older PSCI 0.1 uses SMC function identifiers in range that is
resereved for exisings APIs (ARM DEN 0028B, page 16), while newer


s/resereved/reserved/
s/exisings/existing/


PSCI 0.2 is defined as "standard secure service" with own ranges


0.2 and later

"with its own ranges" I think.


(ARM DEN 0028B, page 18).

Signed-off-by: Volodymyr Babchuk 
Reviewed-by: Oleksandr Andrushchenko 
Reviewed-by: Oleksandr Tyshchenko 
---

 * Fixed mistakes about non-existant PSCI 2.0
 * Added special SMC function number handling for PSCI 0.1
 * Fixed coding style in  handle_psci_0_1()
 * Changed how return do_trap_hvc_smccc() is called from traps.c
 * Renamed SSC to SSSC (Standard Secure Service Calls)

---
 xen/arch/arm/traps.c  | 117 +
 xen/arch/arm/vsmc.c   | 175 --
 xen/include/asm-arm/smccc.h   |   4 +
 xen/include/asm-arm/vsmc.h|   1 +
 xen/include/public/arch-arm/smc.h |   8 ++
 5 files changed, 183 insertions(+), 122 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 4141a89..517e013 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1451,119 +1451,6 @@ static void do_debug_trap(struct cpu_user_regs *regs, 
unsigned int code)
 }
 #endif

-#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
-#define PSCI_ARG(reg, n) get_user_reg(reg, n)
-
-#ifdef CONFIG_ARM_64
-#define PSCI_ARG32(reg, n) (uint32_t)(get_user_reg(reg, n) & 0x)
-#else
-#define PSCI_ARG32(reg, n) PSCI_ARG(reg, n)
-#endif
-
-/* helper function for checking arm mode 32/64 bit */
-static inline int psci_mode_check(struct domain *d, register_t fid)
-{
-return !( is_64bit_domain(d)^( (fid & PSCI_0_2_64BIT) >> 30 ) );
-}
-
-static void do_trap_psci(struct cpu_user_regs *regs)
-{
-register_t fid = PSCI_ARG(regs,0);
-
-/* preloading in case psci_mode_check fails */
-PSCI_SET_RESULT(regs, PSCI_INVALID_PARAMETERS);
-switch( fid )
-{
-case PSCI_cpu_off:
-{
-uint32_t pstate = PSCI_ARG32(regs,1);
-perfc_incr(vpsci_cpu_off);
-PSCI_SET_RESULT(regs, do_psci_cpu_off(pstate));
-}
-break;
-case PSCI_cpu_on:
-{
-uint32_t vcpuid = PSCI_ARG32(regs,1);
-register_t epoint = PSCI_ARG(regs,2);
-perfc_incr(vpsci_cpu_on);
-PSCI_SET_RESULT(regs, do_psci_cpu_on(vcpuid, epoint));
-}
-break;
-case PSCI_0_2_FN_PSCI_VERSION:
-perfc_incr(vpsci_version);
-PSCI_SET_RESULT(regs, do_psci_0_2_version());
-break;
-case PSCI_0_2_FN_CPU_OFF:
-perfc_incr(vpsci_cpu_off);
-PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off());
-break;
-case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
-perfc_incr(vpsci_migrate_info_type);
-PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type());
-break;
-case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
-case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
-perfc_incr(vpsci_migrate_info_up_cpu);
-if ( psci_mode_check(current->domain, fid) )
-PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
-break;
-case PSCI_0_2_FN_SYSTEM_OFF:
-perfc_incr(vpsci_system_off);
-do_psci_0_2_system_off();
-PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
-break;
-case PSCI_0_2_FN_SYSTEM_RESET:
-perfc_incr(vpsci_system_reset);
-do_psci_0_2_system_reset();
-PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
-break;
-case PSCI_0_2_FN_CPU_ON:
-case PSCI_0_2_FN64_CPU_ON:
-perfc_incr(vpsci_cpu_on);
-if ( psci_mode_check(current->domain, fid) )
-{
-register_t vcpuid = PSCI_ARG(regs,1);
-register_t epoint = PSCI_ARG(regs,2);
-register_t cid = PSCI_ARG(regs,3);
-PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
-}
-break;
-case PSCI_0_2_FN_CPU_SUSPEND:
-case PSCI_0_2_FN64_CPU_SUSPEND:
-perfc_incr(vpsci_cpu_suspend);
-if ( psci_mode_check(current->domain, fid) )
-{
-uint32_t pstate = PSCI_ARG32(regs,1);
-register_t epoint = PSCI_ARG(regs,2);
-register_t cid = PSCI_ARG(regs,3);
-PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, 
cid));
-}
-break;
-case PSCI_0_2_FN_AFFINITY_INFO:
-case PSCI_0_2_FN64_AFFINITY_INFO:
-perfc_incr(vpsci_cpu_affinity_info);
-if ( psci_mode_check(current->domain, fid) )
-{
-register_t taff = PSCI_ARG(regs,1);
-uint32_t laff = 

Re: [Xen-devel] [PATCH v4 06/11] arm: smccc: handle SMCs according to SMCCC

[Julien Grall]

Hi Volodymyr,

On 21/08/17 21:27, Volodymyr Babchuk wrote:

SMCCC (SMC Call Convention) describes how to handle both HVCs and SMCs.
SMCCC states that both HVC and SMC are valid conduits to call to different
firmware functions. Thus, for example, PSCI calls can be made both by
SMC or HVC. Also SMCCC defines function number coding for such calls.
Besides functional calls there are query calls, which allows underling
OS determine version, UUID and number of functions provided by service
provider.

This patch adds new file `vsmc.c`, which handles both generic SMCs
and HVC according to SMCCC. At this moment it implements only one
service: Standard Hypervisor Service.

At this time Standard Hypervisor Service only supports query calls,
so caller can ask about hypervisor UID and determine that it is XEN running.

This change allows more generic handling for SMCs and HVCs and it can
be easily extended to support new services and functions.

But, before SMC is forwarded to standard SMCCC handler, it can be routed
to a domain monitor, if one is installed.

Signed-off-by: Volodymyr Babchuk 
Reviewed-by: Oleksandr Andrushchenko 
Reviewed-by: Oleksandr Tyshchenko 
---

 * Reworked UUID handling (due to new UUID type definition)
 * Renamed vsmc_handle_call() to vsmccc_handle_call() to emphasis
   that it handles both SMC and HVC
 * Added comment for inject_undef_exception() usage
 * Used HSR_XXC_IMM_MASK insted of 0xFF
 * Added full stops to comments

---

 xen/arch/arm/Makefile |   1 +
 xen/arch/arm/traps.c  |  18 +
 xen/arch/arm/vsmc.c   | 160 ++
 xen/include/asm-arm/vsmc.h|  30 +++
 xen/include/public/arch-arm/smc.h |  58 ++
 5 files changed, 250 insertions(+), 17 deletions(-)
 create mode 100644 xen/arch/arm/vsmc.c
 create mode 100644 xen/include/asm-arm/vsmc.h
 create mode 100644 xen/include/public/arch-arm/smc.h

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index de00c5e..3d7dde9 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_HAS_GICV3) += vgic-v3.o
 obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o
 obj-y += vm_event.o
 obj-y += vtimer.o
+obj-y += vsmc.o
 obj-y += vpsci.o
 obj-y += vuart.o

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 82cd2b1..4141a89 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 

 #include "decode.h"
@@ -2155,23 +2156,6 @@ static void do_trap_data_abort_guest(struct 
cpu_user_regs *regs,
 inject_dabt_exception(regs, info.gva, hsr.len);
 }

-static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
-{
-int rc = 0;
-
-if ( !check_conditional_instr(regs, hsr) )
-{
-advance_pc(regs, hsr);
-return;
-}
-
-if ( current->domain->arch.monitor.privileged_call_enabled )
-rc = monitor_smc();
-
-if ( rc != 1 )
-inject_undef_exception(regs, hsr);
-}
-


In general we try to avoid code movement in the same patch as new code. 
I am ok with that one, but please avoid this in the future.



 static void enter_hypervisor_head(struct cpu_user_regs *regs)
 {
 if ( guest_mode(regs) )
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
new file mode 100644
index 000..0a81294
--- /dev/null
+++ b/xen/arch/arm/vsmc.c
@@ -0,0 +1,160 @@
+/*
+ * xen/arch/arm/vsmc.c
+ *
+ * Generic handler for SMC and HVC calls according to
+ * ARM SMC calling convention
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Number of functions currently supported by Hypervisor Service. */
+#define XEN_SMCCC_FUNCTION_COUNT 3
+
+static void fill_uuid(struct cpu_user_regs *regs, const xen_uuid_t u)
+{
+#define FILL_UUID(n) \
+set_user_reg(regs, n, (register_t) u[n * 4 + 0] << 24 |
 \
+   u[n * 4 + 1] << 16 |
 \
+   u[n * 4 + 2] << 8  |
 \
+   u[n * 4 + 3] << 0  )


This does not seem to match the spec: "Bytes 0...3 with byte 0 in the 
low-order bits".


Which I understand as byte 0 should be in [0...7], byte 1 [8..15]...


+
+FILL_UUID(0);
+FILL_UUID(1);
+FILL_UUID(2);
+FILL_UUID(3);
+#undef FILL_UUID


This function is really hard to parse and 

Re: [Xen-devel] [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges

[Christoph Hellwig]

On Thu, Aug 24, 2017 at 09:31:17AM -0700, Dan Williams wrote:
> External agent is a DMA device, or a hypervisor like Xen. In the DMA
> case perhaps we can use the fcntl lease mechanism, I'll investigate.
> In the Xen case it actually would need to use fiemap() to discover the
> physical addresses that back the file to setup their M2P tables.
> Here's the discussion where we discovered that physical address
> dependency:
> 
> https://lists.xen.org/archives/html/xen-devel/2017-04/msg00419.html

fiemap does not work to discover physical addresses.  If they want
to do anything involving physical address they will need a kernel
driver.

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

Re: [Xen-devel] [PATCH v2 REPOST 02/12] x86/mm: allow a privileged PV domain to map guest mfns

[Wei Liu]

On Tue, Aug 22, 2017 at 03:50:56PM +0100, Paul Durrant wrote:
> In the case where a PV domain is mapping guest resources then it needs make
> the HYPERVISOR_mmu_update call using DOMID_SELF, rather than the guest
> domid, so that the passed in gmfn values are correctly treated as mfns
> rather than gfns present in the guest p2m.
> 

What would be the callchain like in this case?

I don't quite understand how this fits with the resource mapping code
in this series.

> This patch removes a check which currently disallows mapping of a page when
> the owner of the page tables matches the domain passed to
> HYPERVISOR_mmu_update, but that domain is not the real owner of the page.
> The check was introduced by patch d3c6a215ca9 ("x86: Clean up
> get_page_from_l1e() to correctly distinguish between owner-of-pte and
> owner-of-data-page in all cases") but it's not clear why it was needed.
> 
> Signed-off-by: Paul Durrant 
> ---
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> ---
>  xen/arch/x86/mm.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 0abb1e284f..aaa9ff5197 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -989,12 +989,15 @@ get_page_from_l1e(
> (real_pg_owner != dom_cow) ) )
>  {
>  /*
> - * Let privileged domains transfer the right to map their target
> - * domain's pages. This is used to allow stub-domain pvfb export to
> - * dom0, until pvfb supports granted mappings. At that time this
> - * minor hack can go away.
> + * If the real page owner is not the domain specified in the
> + * hypercall then establish that the specified domain has
> + * mapping privilege over the page owner.
> + * This is used to allow stub-domain pvfb export to dom0. It is
> + * also used to allow a privileged PV domain to map mfns using
> + * DOMID_SELF, which is needed for mapping guest resources such
> + * grant table frames.
>   */
> -if ( (real_pg_owner == NULL) || (pg_owner == l1e_owner) ||
> +if ( (real_pg_owner == NULL) ||
>   xsm_priv_mapping(XSM_TARGET, pg_owner, real_pg_owner) )
>  {
>  gdprintk(XENLOG_WARNING,
> -- 
> 2.11.0
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

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

Re: [Xen-devel] [PATCH v2 REPOST 08/12] x86/hvm/ioreq: move is_default into struct hvm_ioreq_server

[Paul Durrant]

> -Original Message-
> From: Roger Pau Monne
> Sent: 24 August 2017 17:22
> To: Paul Durrant 
> Cc: xen-de...@lists.xenproject.org; Andrew Cooper
> ; Jan Beulich 
> Subject: Re: [Xen-devel] [PATCH v2 REPOST 08/12] x86/hvm/ioreq: move
> is_default into struct hvm_ioreq_server
> 
> On Tue, Aug 22, 2017 at 03:51:02PM +0100, Paul Durrant wrote:
> > Legacy emulators use the 'default' IOREQ server which has slightly
> > different semantics than other, explicitly created, IOREQ servers.
> >
> > Because of this, most of the initialization and teardown code needs to
> > know whether the server is default or not. This is currently achieved
> > by passing an is_default boolean argument to the functions in question,
> > whereas this argument could be avoided by adding a field to the
> > hvm_ioreq_server structure which is also passed as an argument to all
> > the relevant functions.
> >
> > Signed-off-by: Paul Durrant 
> 
> This looks fine, I've also seen that there's a
> hvm_domain.default_ioreq_server, which is used in a bunch of loops in the
> file like:
> 
> if ( s == d->arch.hvm_domain.default_ioreq_server )
> 
> AFAICT those could also be replaced with s->is_default.

Yes, they should be. Thanks for spotting that.

Cheers,

  Paul

> 
> Roger.

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

Re: [Xen-devel] [PATCH v6 4/5] fs, xfs: introduce MAP_DIRECT for creating block-map-atomic file ranges

[Dan Williams]

[ adding Xen ]

On Thu, Aug 24, 2017 at 9:11 AM, Christoph Hellwig  wrote:
> I still can't make any sense of this description.  What is an external
> agent?  Userspace obviously can't ever see a change in the extent
> map, so it can't be meant.

External agent is a DMA device, or a hypervisor like Xen. In the DMA
case perhaps we can use the fcntl lease mechanism, I'll investigate.
In the Xen case it actually would need to use fiemap() to discover the
physical addresses that back the file to setup their M2P tables.
Here's the discussion where we discovered that physical address
dependency:

https://lists.xen.org/archives/html/xen-devel/2017-04/msg00419.html

> It would help a lot if you could come up with a concrete user for this,
> including example code.

Will do.

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

Re: [Xen-devel] [PATCH v4 01/53] xen: add an optional string end parameter to parse_bool()

[Juergen Gross]

On 24/08/17 17:33, Jan Beulich wrote:
 On 23.08.17 at 19:33,  wrote:
>> @@ -163,20 +163,24 @@ void __init cmdline_parse(const char *cmdline)
>>  #endif
>>  }
>>  
>> -int __init parse_bool(const char *s)
>> +int __init parse_bool(const char *s, const char *e)
>>  {
>> -if ( !strcmp("no", s) ||
>> - !strcmp("off", s) ||
>> - !strcmp("false", s) ||
>> - !strcmp("disable", s) ||
>> - !strcmp("0", s) )
>> +unsigned int len;
>> +
>> +len = e ? e - s : strlen(s);
> 
> If you don't mind, I'd like to see ASSERT(e >= s) added to the middle
> part here; should be easily doable while committing.

Sure, NP.


Juergen


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

Re: [Xen-devel] [PATCH v4 19/53] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 24/08/17 17:35, Jan Beulich wrote:
 On 23.08.17 at 19:34,  wrote:
>> --- a/xen/arch/x86/psr.c
>> +++ b/xen/arch/x86/psr.c
>> @@ -418,50 +418,66 @@ static const struct feat_props l2_cat_props = {
>>  .write_msr = l2_cat_write_msr,
>>  };
>>  
>> -static void __init parse_psr_bool(char *s, char *value, char *feature,
>> +static bool __init parse_psr_bool(const char *s, const char *value,
>> +  const char *ss, const char *feature,
>>unsigned int mask)
>>  {
>> -if ( !strcmp(s, feature) )
>> +if ( !strncmp(s, feature, value - s) )
>>  {
>> -if ( !value )
>> +if ( !*value )
>>  opt_psr |= mask;
>>  else
>>  {
>> -int val_int = parse_bool(value, NULL);
>> +int val_int = parse_bool(value + 1, ss);
> 
> Why "+ 1" here?

value points to the delimiter ('\0' or ',') now.


Juergen

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

Re: [Xen-devel] [PATCH v2 REPOST 08/12] x86/hvm/ioreq: move is_default into struct hvm_ioreq_server

[Roger Pau Monné]

On Tue, Aug 22, 2017 at 03:51:02PM +0100, Paul Durrant wrote:
> Legacy emulators use the 'default' IOREQ server which has slightly
> different semantics than other, explicitly created, IOREQ servers.
> 
> Because of this, most of the initialization and teardown code needs to
> know whether the server is default or not. This is currently achieved
> by passing an is_default boolean argument to the functions in question,
> whereas this argument could be avoided by adding a field to the
> hvm_ioreq_server structure which is also passed as an argument to all
> the relevant functions.
> 
> Signed-off-by: Paul Durrant 

This looks fine, I've also seen that there's a hvm_domain.default_ioreq_server, 
which is used in a bunch of loops in the file like:

if ( s == d->arch.hvm_domain.default_ioreq_server )

AFAICT those could also be replaced with s->is_default.

Roger.

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

Re: [Xen-devel] [PATCH v2 REPOST 07/12] x86/hvm/ioreq: use bool rather than bool_t

[Roger Pau Monné]

On Tue, Aug 22, 2017 at 03:51:01PM +0100, Paul Durrant wrote:
> This patch changes use of bool_t to bool in the IOREQ server code. It also
> fixes an incorrect indentation in a continuation line.
> 
> This patch is purely cosmetic. No semantic or functional change.
> 
> Signed-off-by: Paul Durrant 

LGTM:

Reviewed-by: Roger Pau Monné 

Thanks, Roger.

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

Re: [Xen-devel] [PATCH 2/5] xen: move XENMAPSPACE_grant_table code into grant_table.c

[Julien Grall]

Hello,

I was expecting to be CCed on this patch.

On 21/08/17 19:05, Juergen Gross wrote:

The x86 and arm versions of XENMAPSPACE_grant_table handling are nearly
identical. Move the code into a function in grant_table.c and add an
architecture dependant hook to handle the differences.

This at once fixes a bug in the arm code which didn't unlock the grant
table in error case.

Signed-off-by: Juergen Gross 
---
 xen/arch/arm/mm.c | 34 
 xen/arch/x86/mm.c | 41 ++-
 xen/common/grant_table.c  | 38 
 xen/include/asm-arm/grant_table.h |  6 ++
 xen/include/asm-x86/grant_table.h |  5 +
 xen/include/xen/grant_table.h |  3 +++
 6 files changed, 66 insertions(+), 61 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index a810a056d7..6dad167a8e 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 


Why sched.h has been moved earlier? Likely it means one of the header 
doesn't include all its dependency.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 REPOST 05/12] tools/libxenctrl: use new xenforeignmemory API to seed grant table

[Paul Durrant]

> -Original Message-
> From: Roger Pau Monne
> Sent: 24 August 2017 17:03
> To: Paul Durrant 
> Cc: xen-de...@lists.xenproject.org; Wei Liu ; Ian
> Jackson 
> Subject: Re: [Xen-devel] [PATCH v2 REPOST 05/12] tools/libxenctrl: use new
> xenforeignmemory API to seed grant table
> 
> On Tue, Aug 22, 2017 at 03:50:59PM +0100, Paul Durrant wrote:
> > A previous patch added support for priv-mapping guest resources directly
> > (rather than having to foreign-map, which requires P2M modification for
> > HVM guests).
> >
> > This patch makes use of the new API to seed the guest grant table unless
> > the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> > case the old scheme is used.
> >
> > NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params()
> was
> >   actually unnecessary, as the grant table has already been seeded
> >   by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
> >
> > Signed-off-by: Paul Durrant 
> > Acked-by: Marek Marczykowski-Górecki
> 
> > ---
> > Cc: Ian Jackson 
> > Cc: Wei Liu 
> > ---
> >  tools/libxc/include/xc_dom.h|   8 +--
> >  tools/libxc/xc_dom_boot.c   | 102
> 
> >  tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
> >  tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
> >  tools/libxl/libxl_dom.c |   1 -
> >  tools/python/xen/lowlevel/xc/xc.c   |   6 +--
> >  6 files changed, 92 insertions(+), 37 deletions(-)
> >
> > diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> > index ce47058c41..d6ca0a8680 100644
> > --- a/tools/libxc/include/xc_dom.h
> > +++ b/tools/libxc/include/xc_dom.h
> > @@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct
> xc_dom_image *dom, xen_pfn_t pfn,
> >  int xc_dom_boot_image(struct xc_dom_image *dom);
> >  int xc_dom_compat_check(struct xc_dom_image *dom);
> >  int xc_dom_gnttab_init(struct xc_dom_image *dom);
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -   xen_pfn_t console_gmfn,
> > -   xen_pfn_t xenstore_gmfn,
> > -   domid_t console_domid,
> > -   domid_t xenstore_domid);
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > +   bool is_hvm,
> > xen_pfn_t console_gmfn,
> > xen_pfn_t xenstore_gmfn,
> > domid_t console_domid,
> > diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> > index c3b44dd399..fc3174ad7e 100644
> > --- a/tools/libxc/xc_dom_boot.c
> > +++ b/tools/libxc/xc_dom_boot.c
> > @@ -280,11 +280,11 @@ static xen_pfn_t
> xc_dom_gnttab_setup(xc_interface *xch, domid_t domid)
> >  return gmfn;
> >  }
> >
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > -   xen_pfn_t console_gmfn,
> > -   xen_pfn_t xenstore_gmfn,
> > -   domid_t console_domid,
> > -   domid_t xenstore_domid)
> > +static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
> > +  xen_pfn_t console_gmfn,
> > +  xen_pfn_t xenstore_gmfn,
> > +  domid_t console_domid,
> > +  domid_t xenstore_domid)
> >  {
> >
> >  xen_pfn_t gnttab_gmfn;
> > @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch,
> domid_t domid,
> >  return 0;
> >  }
> >
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -   xen_pfn_t console_gpfn,
> > -   xen_pfn_t xenstore_gpfn,
> > -   domid_t console_domid,
> > -   domid_t xenstore_domid)
> > +static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > +  xen_pfn_t console_gpfn,
> > +  xen_pfn_t xenstore_gpfn,
> > +  domid_t console_domid,
> > +  domid_t xenstore_domid)
> >  {
> >  int rc;
> >  xen_pfn_t scratch_gpfn;
> > @@ -380,7 +380,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch,
> domid_t domid,
> >  return -1;
> >  }
> >
> > -rc = xc_dom_gnttab_seed(xch, domid,
> > +rc = compat_gnttab_seed(xch, domid,
> >  console_gpfn, xenstore_gpfn,
> >  console_domid, xenstore_domid);
> >  if (rc != 0)
> > @@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface
> *xch, domid_t domid,
> >  return 0;
> >  }
> >
> > -int xc_dom_gnttab_init(struct xc_dom_image *dom)
> > 

Re: [Xen-devel] [PATCH v2 REPOST 06/12] x86/hvm/ioreq: rename .*pfn and .*gmfn to .*gfn

[Roger Pau Monné]

On Tue, Aug 22, 2017 at 03:51:00PM +0100, Paul Durrant wrote:
> Since IOREQ servers are only relevant to HVM guests and all the names in
> question unequivocally refer to guest frame numbers, name them all .*gfn
> to avoid any confusion.
> 
> This patch is purely cosmetic. No semantic or functional change.
> 
> Signed-off-by: Paul Durrant 

LGTM:

Reviewed-by: Roger Pau Monné 

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v2] x86: enable RCU based table free

[Peter Zijlstra]

On Thu, Aug 24, 2017 at 05:27:21PM +0200, Vitaly Kuznetsov wrote:
> Do you think adding something like
> 
> /*
>  * While x86 architecture in general requires an IPI to perform TLB
>  * shootdown, enablement code for several hypervisors overrides
>  * .flush_tlb_others hook in pv_mmu_ops and implements it by issuing
>  * a hypercall. To keep software pagetable walkers safe in this case we 
>  * switch to RCU based table free (HAVE_RCU_TABLE_FREE). See the comment
>  * below 'ifdef CONFIG_HAVE_RCU_TABLE_FREE' in include/asm-generic/tlb.h
>  * for more details.
>  */
> 
> before __tlb_remove_table would suffice? Or do you see a better place
> for such comment?

Yes, that seems fine. Thanks!

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

Re: [Xen-devel] [PATCH v2 REPOST 05/12] tools/libxenctrl: use new xenforeignmemory API to seed grant table

[Roger Pau Monné]

On Tue, Aug 22, 2017 at 03:50:59PM +0100, Paul Durrant wrote:
> A previous patch added support for priv-mapping guest resources directly
> (rather than having to foreign-map, which requires P2M modification for
> HVM guests).
> 
> This patch makes use of the new API to seed the guest grant table unless
> the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> case the old scheme is used.
> 
> NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params() was
>   actually unnecessary, as the grant table has already been seeded
>   by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
> 
> Signed-off-by: Paul Durrant 
> Acked-by: Marek Marczykowski-Górecki 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> ---
>  tools/libxc/include/xc_dom.h|   8 +--
>  tools/libxc/xc_dom_boot.c   | 102 
> 
>  tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
>  tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
>  tools/libxl/libxl_dom.c |   1 -
>  tools/python/xen/lowlevel/xc/xc.c   |   6 +--
>  6 files changed, 92 insertions(+), 37 deletions(-)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index ce47058c41..d6ca0a8680 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, 
> xen_pfn_t pfn,
>  int xc_dom_boot_image(struct xc_dom_image *dom);
>  int xc_dom_compat_check(struct xc_dom_image *dom);
>  int xc_dom_gnttab_init(struct xc_dom_image *dom);
> -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> -   xen_pfn_t console_gmfn,
> -   xen_pfn_t xenstore_gmfn,
> -   domid_t console_domid,
> -   domid_t xenstore_domid);
> -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> +   bool is_hvm,
> xen_pfn_t console_gmfn,
> xen_pfn_t xenstore_gmfn,
> domid_t console_domid,
> diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> index c3b44dd399..fc3174ad7e 100644
> --- a/tools/libxc/xc_dom_boot.c
> +++ b/tools/libxc/xc_dom_boot.c
> @@ -280,11 +280,11 @@ static xen_pfn_t xc_dom_gnttab_setup(xc_interface *xch, 
> domid_t domid)
>  return gmfn;
>  }
>  
> -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> -   xen_pfn_t console_gmfn,
> -   xen_pfn_t xenstore_gmfn,
> -   domid_t console_domid,
> -   domid_t xenstore_domid)
> +static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
> +  xen_pfn_t console_gmfn,
> +  xen_pfn_t xenstore_gmfn,
> +  domid_t console_domid,
> +  domid_t xenstore_domid)
>  {
>  
>  xen_pfn_t gnttab_gmfn;
> @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
>  return 0;
>  }
>  
> -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> -   xen_pfn_t console_gpfn,
> -   xen_pfn_t xenstore_gpfn,
> -   domid_t console_domid,
> -   domid_t xenstore_domid)
> +static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> +  xen_pfn_t console_gpfn,
> +  xen_pfn_t xenstore_gpfn,
> +  domid_t console_domid,
> +  domid_t xenstore_domid)
>  {
>  int rc;
>  xen_pfn_t scratch_gpfn;
> @@ -380,7 +380,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t 
> domid,
>  return -1;
>  }
>  
> -rc = xc_dom_gnttab_seed(xch, domid,
> +rc = compat_gnttab_seed(xch, domid,
>  console_gpfn, xenstore_gpfn,
>  console_domid, xenstore_domid);
>  if (rc != 0)
> @@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t 
> domid,
>  return 0;
>  }
>  
> -int xc_dom_gnttab_init(struct xc_dom_image *dom)
> +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> +   bool is_hvm, xen_pfn_t console_gmfn,
> +   xen_pfn_t xenstore_gmfn, domid_t console_domid,
> +   domid_t xenstore_domid)
>  {
> -if ( xc_dom_translated(dom) ) {
> -return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
> -  dom->console_pfn, dom->xenstore_pfn,
> -  dom->console_domid, 
> dom->xenstore_domid);
> -} else {
> 

Re: [Xen-devel] [PATCH v2 REPOST 04/12] tools/libxenforeignmemory: add support for resource mapping

[Paul Durrant]

> -Original Message-
> From: Roger Pau Monne
> Sent: 24 August 2017 16:53
> To: Paul Durrant 
> Cc: xen-de...@lists.xenproject.org; Wei Liu ; Ian
> Jackson 
> Subject: Re: [Xen-devel] [PATCH v2 REPOST 04/12]
> tools/libxenforeignmemory: add support for resource mapping
> 
> On Tue, Aug 22, 2017 at 03:50:58PM +0100, Paul Durrant wrote:
> > diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h
> b/tools/libs/foreignmemory/include/xenforeignmemory.h
> > index f4814c390f..e56eb3c4d4 100644
> > --- a/tools/libs/foreignmemory/include/xenforeignmemory.h
> > +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h
> > @@ -138,6 +138,45 @@ int
> xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
> >  int xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
> >domid_t domid);
> >
> > +typedef struct xenforeignmemory_resource_handle
> xenforeignmemory_resource_handle;
> > +
> > +/**
> > + * This function maps a guest resource.
> > + *
> > + * @parm fmem handle to the open foreignmemory interface
> > + * @parm domid the domain id
> > + * @parm type the resource type
> > + * @parm id the type-specific resource identifier
> > + * @parm frame base frame index within the resource
> > + * @parm nr_frames number of frames to map
> > + * @parm paddr pointer to an address passed through to mmap(2)
> > + * @parm prot passed through to mmap(2)
> > + * @parm flags passed through to mmap(2)
> 
> Passing mmap flags directly can be troublesome, POSIX only defines
> MAP_SHARED and MAP_PRIVATE, the rest is OS-specific AFAIK. So we
> either limit this to POSIX only flags (and enforce it), or we replace
> it with some xenforeignmemory specific flags that each OS can map to
> it's equivalents.

Given that foreign memory mapping already passes through flags I guess that, 
for consistency, it would be best to limit use to POSIX-only flags in all cases.

> 
> > --- a/tools/libs/foreignmemory/private.h
> > +++ b/tools/libs/foreignmemory/private.h
> > @@ -42,6 +42,36 @@ void
> *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom,
> >xen_pfn_t *arr, int num);
> >  #endif
> >
> > +struct xenforeignmemory_resource_handle {
> > +domid_t domid;
> > +unsigned int type;
> > +unsigned int id;
> > +unsigned long frame;
> > +unsigned long nr_frames;
> > +void *addr;
> > +int prot;
> > +int flags;
> > +};
> > +
> > +#ifndef __linux__
> 
> The common practice in libxenforeign seems to be to add those dummy
> handlers to each OS-specific file (see osdep_xenforeignmemory_restrict
> for example) instead of doing it in the header file.

Yes, I know, I introduced that code :-) I think this way is actually neater. If 
no-one objects I can add a patch in to clean up xenforeignmemory_restrict() too.

Cheers,

  Paul

> 
> Roger.

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

Re: [Xen-devel] [PATCH v2 REPOST 04/12] tools/libxenforeignmemory: add support for resource mapping

[Roger Pau Monné]

On Tue, Aug 22, 2017 at 03:50:58PM +0100, Paul Durrant wrote:
> diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h 
> b/tools/libs/foreignmemory/include/xenforeignmemory.h
> index f4814c390f..e56eb3c4d4 100644
> --- a/tools/libs/foreignmemory/include/xenforeignmemory.h
> +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h
> @@ -138,6 +138,45 @@ int xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
>  int xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
>domid_t domid);
>  
> +typedef struct xenforeignmemory_resource_handle 
> xenforeignmemory_resource_handle;
> +
> +/**
> + * This function maps a guest resource.
> + *
> + * @parm fmem handle to the open foreignmemory interface
> + * @parm domid the domain id
> + * @parm type the resource type
> + * @parm id the type-specific resource identifier
> + * @parm frame base frame index within the resource
> + * @parm nr_frames number of frames to map
> + * @parm paddr pointer to an address passed through to mmap(2)
> + * @parm prot passed through to mmap(2)
> + * @parm flags passed through to mmap(2)

Passing mmap flags directly can be troublesome, POSIX only defines
MAP_SHARED and MAP_PRIVATE, the rest is OS-specific AFAIK. So we
either limit this to POSIX only flags (and enforce it), or we replace
it with some xenforeignmemory specific flags that each OS can map to
it's equivalents.

> --- a/tools/libs/foreignmemory/private.h
> +++ b/tools/libs/foreignmemory/private.h
> @@ -42,6 +42,36 @@ void *compat_mapforeign_batch(xenforeignmem_handle *fmem, 
> uint32_t dom,
>xen_pfn_t *arr, int num);
>  #endif
>  
> +struct xenforeignmemory_resource_handle {
> +domid_t domid;
> +unsigned int type;
> +unsigned int id;
> +unsigned long frame;
> +unsigned long nr_frames;
> +void *addr;
> +int prot;
> +int flags;
> +};
> +
> +#ifndef __linux__

The common practice in libxenforeign seems to be to add those dummy
handlers to each OS-specific file (see osdep_xenforeignmemory_restrict
for example) instead of doing it in the header file.

Roger.

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

Re: [Xen-devel] [PATCH v5 01/11] x86/pci: introduce hvm_pci_decode_addr

[Jan Beulich]

>>> On 14.08.17 at 16:28,  wrote:
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -256,6 +256,25 @@ void register_g2m_portio_handler(struct domain *d)
>  handler->ops = _portio_ops;
>  }
>  
> +unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr,
> + unsigned int *bus, unsigned int *slot,
> + unsigned int *func)
> +{
> +unsigned long bdf;

Is there a need for this being unsigned long instead of unsigned int?
If not, I'd be fine changing this while committing.

Jan


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

Re: [Xen-devel] [PATCH v4 01/53] xen: add an optional string end parameter to parse_bool()

[Andrew Cooper]

On 23/08/17 18:33, Juergen Gross wrote:
> Add a parameter to parse_bool() to specify the end of the to be
> parsed string. Specifying it as NULL will preserve the current
> behavior to parse until the end of the input string, while passing
> a non-NULL pointer will specify the first character after the input
> string.
>
> This will allow to parse boolean sub-strings without having to
> write a NUL byte into the input string.
>
> Modify all users of parse_bool() to pass NULL for the new parameter.

So I already had plans for parse_bool() during the XSA-226 embaro
period, but couldn't discuss any of them, and this series appeared in
the meantime.

One rather confusing problem we have is that top level booleans behave
differently to sub-booleans.

Top-level booleans support all kinds of ={0,true,yes, ...} qualifiers,
as well as no- prefixes.  sub-booleans may or may not support the
qualifiers, and where they do support the no- prefixes, the same prefix
gets silently eaten for non-boolean suboptions.

I had planned to modify parse_bool() to be

int parse_bool(const char *field, const char *s)
{
...
}

which cases care of considering the "no-" prefix, optionally skips the
field name if it matches exactly, and then performs the current logic on
the remainder of the string.  This way, boolean options should work
consistently wherever they are.

It also means that a lot of custom_params() need simplifying to always
pass intended boolean options to parse_bool().

Could we see about merging this work together, rather than having two
series going and modifying how the parsing works?

~Andrew

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

Re: [Xen-devel] [PATCH v4 19/53] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Jan Beulich]

>>> On 23.08.17 at 19:34,  wrote:
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -418,50 +418,66 @@ static const struct feat_props l2_cat_props = {
>  .write_msr = l2_cat_write_msr,
>  };
>  
> -static void __init parse_psr_bool(char *s, char *value, char *feature,
> +static bool __init parse_psr_bool(const char *s, const char *value,
> +  const char *ss, const char *feature,
>unsigned int mask)
>  {
> -if ( !strcmp(s, feature) )
> +if ( !strncmp(s, feature, value - s) )
>  {
> -if ( !value )
> +if ( !*value )
>  opt_psr |= mask;
>  else
>  {
> -int val_int = parse_bool(value, NULL);
> +int val_int = parse_bool(value + 1, ss);

Why "+ 1" here?

Jan


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

Re: [Xen-devel] [PATCH v4 01/53] xen: add an optional string end parameter to parse_bool()

[Jan Beulich]

>>> On 23.08.17 at 19:33,  wrote:
> @@ -163,20 +163,24 @@ void __init cmdline_parse(const char *cmdline)
>  #endif
>  }
>  
> -int __init parse_bool(const char *s)
> +int __init parse_bool(const char *s, const char *e)
>  {
> -if ( !strcmp("no", s) ||
> - !strcmp("off", s) ||
> - !strcmp("false", s) ||
> - !strcmp("disable", s) ||
> - !strcmp("0", s) )
> +unsigned int len;
> +
> +len = e ? e - s : strlen(s);

If you don't mind, I'd like to see ASSERT(e >= s) added to the middle
part here; should be easily doable while committing.

Jan


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

Re: [Xen-devel] [PATCH XEN v4] x86/pt: add a MSI unmask flag to XEN_DOMCTL_bind_pt_irq

[Jan Beulich]

>>> On 24.08.17 at 17:07,  wrote:
> The flag is part of the gflags, and should be used to request the
> unmask of a MSI interrupt once it's bound.
> 
> This is required for the device model in order to be capable of
> binding MSIX interrupts that have the entry mask bit already unset at
> bind time. Without this fix the interrupts would be left masked.
> 
> Note that this commit introduces a change to the domctl, which
> requires a bump of the interface version. This is done done here

"... is not done here ..." I suppose (I'll try to remember changing
that while committing, unless you tell me I've got it wrong).

> because the interface version has already been bumped in this release
> cycle.
> 
> Signed-off-by: Roger Pau Monné 
> Reported by: Andreas Kinzler 

Reviewed-by: Jan Beulich 


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

Re: [Xen-devel] [PATCH v2] x86: enable RCU based table free

[Vitaly Kuznetsov]

Peter Zijlstra  writes:

> On Thu, Aug 24, 2017 at 11:22:58AM +0200, Vitaly Kuznetsov wrote:
>
>> diff --git a/arch/x86/include/asm/tlb.h b/arch/x86/include/asm/tlb.h
>> index c7797307fc2b..d43a7fcafee9 100644
>> --- a/arch/x86/include/asm/tlb.h
>> +++ b/arch/x86/include/asm/tlb.h
>> @@ -15,4 +15,9 @@
>>  
>>  #include 
>>  
>> +static inline void __tlb_remove_table(void *table)
>> +{
>> +free_page_and_swap_cache(table);
>> +}
>
> Most other archs have this in pgtable.h, only ARM* has it in tlb.h.
>

Sure, I can move it in v3 if nobody objects.

> And should we put a comment on explaining _why_ we have RCU_TABLE_FREE
> enabled?

Do you think adding something like

/*
 * While x86 architecture in general requires an IPI to perform TLB
 * shootdown, enablement code for several hypervisors overrides
 * .flush_tlb_others hook in pv_mmu_ops and implements it by issuing
 * a hypercall. To keep software pagetable walkers safe in this case we 
 * switch to RCU based table free (HAVE_RCU_TABLE_FREE). See the comment
 * below 'ifdef CONFIG_HAVE_RCU_TABLE_FREE' in include/asm-generic/tlb.h
 * for more details.
 */

before __tlb_remove_table would suffice? Or do you see a better place
for such comment?

Actually, after enabling HAVE_RCU_TABLE_FREE on x86 we may consider
switching to this mechanism globally: it seems to have negligible effect
on performace (and all major arches will already have it). One step at a
time, though.

-- 
  Vitaly

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

Re: [Xen-devel] [PATCH v4 12/31] x86/mm: split out readonly MMIO emulation code

[Andrew Cooper]

On 24/08/17 16:16, Jan Beulich wrote:
 On 17.08.17 at 16:44,  wrote:
>> Move the code to pv/emul-mmio-op.c. Fix coding style issues while
>> moving.
>>
>> Note that mmio_ro_emulated_write is needed by both PV and HVM, so it
>> is left in x86/mm.c.
>>
>> Signed-off-by: Wei Liu 
>> ---
>>  xen/arch/x86/mm.c  | 129 
>>  xen/arch/x86/pv/Makefile   |   1 +
>>  xen/arch/x86/pv/emul-mmio-op.c | 166 
>> +
> Again I think just mmio.c would do. Other comments on earlier
> patches apply here as well.

I think it would be wise to merge the ptwr and mmio handling.  At the
moment, we invoke a full lookup pte/decode/try-to-emulate cycle twice in
the #PF handler for PV guests before handing the fault back to the guest.

The correct ops and context can be determined by inspecting the l1e
under %cr2 before calling into any emulation code.

Simplifying this logic before moving it would be the better option.

~Andrew

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

Re: [Xen-devel] [PATCH v2] common/vm_event: Initialize vm_event lists on domain creation

[Jan Beulich]

>>> On 24.08.17 at 17:17,  wrote:
> On Jo, 2017-08-24 at 07:24 -0600, Jan Beulich wrote:
>> > @@ -500,6 +519,9 @@ bool_t vm_event_check_ring(struct
>> > vm_event_domain *ved)
>> >  int __vm_event_claim_slot(struct domain *d, struct vm_event_domain
>> > *ved,
>> >bool_t allow_sleep)
>> >  {
>> > +if ( !ved )
>> > +return -ENOSYS;
>> I don't think ENOSYS is correct here.
> Can you tell me what is the preferred return value here?

-EOPNOTSUPP is what we commonly use in such cases.

>> > @@ -599,7 +621,6 @@ int vm_event_domctl(struct domain *d,
>> > xen_domctl_vm_event_op_t *vec,
>> >  #ifdef CONFIG_HAS_MEM_PAGING
>> >  case XEN_DOMCTL_VM_EVENT_OP_PAGING:
>> >  {
>> > -struct vm_event_domain *ved = >vm_event->paging;
>> Dropping this local variable (and similar ones below) pointlessly
>> increases the size of this patch.
> I have dropped the local var because in case of a XEN_VM_EVENT_ENABLE
> d->vm_event_... is allocated in the vm_event_enable function below so
> it will allocate mem for the local variable.

I don't see how that renders the local variable useless. But anyway,
its the maintainers of that code to judge.

Also - please trim your replies.

Jan


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

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Jan Beulich]

>>> On 24.08.17 at 17:13,  wrote:
> On 24/08/17 17:04, Jan Beulich wrote:
> On 24.08.17 at 16:48,  wrote:
>>> How would the guest know whether using v2 grants is no disadvantage?
>> 
>> As said - it's always going to be a disadvantage. Even if controlling
>> the number of frames per-domain, that same number of frames
>> will always fit more v1 than v2 entries.
> 
> And my patches break the assumption "same number of frames".
> 
>> I don't think the config
>> setting should directly control the number of active grants.
> 
> Why not? In the end that number is the one the guest is interested in.

And the resource use is what the admin is interested in.

> BTW: the number of needed maptrack frames is depending on the number
> of grants only, not on v1 or v2. And the default for the max. number
> of maptrack frames is much higher than the one of the grant frames
> (1024 vs 32).

Of course.

Jan


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

Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Andrew Cooper]

On 24/08/17 16:01, Juergen Gross wrote:
> On 24/08/17 16:50, Andrew Cooper wrote:
>> This patch was originally a workaround for XSA-226.  Since then, transitive
>> grants are believed to be functioning properly, and the defaults have changed
>> appropriately.
>>
>> However, for those people who chose to use the workaround (especially from an
>> attack surface mitigation point of view), retain the ability for the host
>> administrator to choose.
>>
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Jan Beulich 
>> CC: George Dunlap 
>> CC: Konrad Rzeszutek Wilk 
>> CC: Stefano Stabellini 
>> CC: Tim Deegan 
>> CC: Wei Liu 
>> ---
>>  docs/misc/xen-command-line.markdown | 13 +++
>>  xen/common/grant_table.c| 44 
>> +++--
>>  2 files changed, 55 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/misc/xen-command-line.markdown 
>> b/docs/misc/xen-command-line.markdown
>> index 4002eab..78c7b51 100644
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -868,6 +868,19 @@ Controls EPT related features.
>>  
>>  Specify which console gdbstub should use. See **console**.
>>  
>> +### gnttab
>> +> `= List of [ max_ver:, transitive ]`
>> +
>> +> Default: `gnttab=max_ver:2,transitive`
>> +
>> +Control various aspects of the grant table behaviour available to guests.
>> +
>> +* `max_ver` Select the maximum grant table version to offer to guests.  
>> Valid
>> +version are 1 and 2.
>> +* `transitive` Permit or disallow the use of transitive grants.  Note that 
>> the
>> +use of grant table v2 without transitive grants is an ABI breakage from the
>> +guests point of view.
> So shouldn't there be a way for the guest to query the support of
> transient grants?

Ideally yes, but how do you suggest doing this in a compatible way?

All Xen downstreams which haven't backported the eventual transitive
fixes will have this clobber in place, without any query-ability.

The reason this workaround was so effective, and why several large users
still wish to clobber them, is because nothing uses transitive grants.

>
>> +
>>  ### gnttab\_max\_frames
>>  > `= `
>>  
>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>> index 36895aa..f9c313d 100644
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", 
>> max_nr_grant_frames);
>>  unsigned int __read_mostly max_grant_frames;
>>  integer_param("gnttab_max_frames", max_grant_frames);
>>  
>> +static unsigned int __read_mostly opt_gnttab_max_version = 2;
>> +static bool __read_mostly opt_transitive_grants = true;
>> +
>> +static void __init parse_gnttab(char *s)
> Do you mind using:
>
> static int __init parse_gnttab(const char *s)
>
> in order to comply with my runtime parameter series?

If the result will still compile.  What should the semantics be?  (I've
had a quick look through your series, but I can't see the semantics
stated anywhere obvious)

~Andrew

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

Re: [Xen-devel] [PATCH v2] common/vm_event: Initialize vm_event lists on domain creation

[Alexandru Stefan ISAILA]

On Jo, 2017-08-24 at 07:24 -0600, Jan Beulich wrote:
> > 
> > > 
> > > > 
> > > > On 24.08.17 at 13:48,  wrote:
> > The patch splits the vm_event into three structures:vm_event_share,
> > vm_event_paging, vm_event_monitor. The allocation for the
> > structure is moved to vm_event_enable so that it can be
> > allocated/init when needed and freed in vm_event_disable.
> > 
> > Signed-off-by: Alexandru Isaila 
> Missing brief description of changes from v1 here.
> 
> > 
> > @@ -50,32 +50,37 @@ static int vm_event_enable(
> >  int rc;
> >  unsigned long ring_gfn = d->arch.hvm_domain.params[param];
> >  
> > +if ( !(*ved) )
> > +(*ved) = xzalloc(struct vm_event_domain);
> > +if ( !(*ved) )
> In none of the three cases you really need the parentheses around
> *ved.
> 
> > 
> > @@ -187,39 +194,45 @@ void vm_event_wake(struct domain *d, struct 
> > vm_event_domain *ved)
> >  vm_event_wake_blocked(d, ved);
> >  }
> >  
> > -static int vm_event_disable(struct domain *d, struct
> > vm_event_domain *ved)
> > +static int vm_event_disable(struct domain *d, struct
> > vm_event_domain **ved)
> >  {
> > -if ( ved->ring_page )
> > +if ( !*ved )
> > +return 0;
> > +
> > +if ( (*ved)->ring_page )
> >  {
> > [...]
> > +xfree(*ved);
> > +*ved = NULL;
> >  }
> If both if()-s above are really useful, you are leaking *ved when it
> is non-NULL but ->ring_page is NULL.
> 
> > 
> > @@ -500,6 +519,9 @@ bool_t vm_event_check_ring(struct
> > vm_event_domain *ved)
> >  int __vm_event_claim_slot(struct domain *d, struct vm_event_domain
> > *ved,
> >    bool_t allow_sleep)
> >  {
> > +if ( !ved )
> > +return -ENOSYS;
> I don't think ENOSYS is correct here.
Can you tell me what is the preferred return value here?
> 
> > 
> > @@ -510,24 +532,24 @@ int __vm_event_claim_slot(struct domain *d,
> > struct vm_event_domain *ved,
> >  /* Registered with Xen-bound event channel for incoming
> > notifications. */
> >  static void mem_paging_notification(struct vcpu *v, unsigned int
> > port)
> >  {
> > -if ( likely(v->domain->vm_event->paging.ring_page != NULL) )
> > -vm_event_resume(v->domain, >domain->vm_event->paging);
> > +if ( likely(v->domain->vm_event_paging->ring_page != NULL) )
> > +vm_event_resume(v->domain, v->domain->vm_event_paging);
> >  }
> >  #endif
> >  
> >  /* Registered with Xen-bound event channel for incoming
> > notifications. */
> >  static void monitor_notification(struct vcpu *v, unsigned int
> > port)
> >  {
> > -if ( likely(v->domain->vm_event->monitor.ring_page != NULL) )
> > -vm_event_resume(v->domain, >domain->vm_event->monitor);
> > +if ( likely(v->domain->vm_event_monitor->ring_page != NULL) )
> > +vm_event_resume(v->domain, v->domain->vm_event_monitor);
> >  }
> >  
> >  #ifdef CONFIG_HAS_MEM_SHARING
> >  /* Registered with Xen-bound event channel for incoming
> > notifications. */
> >  static void mem_sharing_notification(struct vcpu *v, unsigned int
> > port)
> >  {
> > -if ( likely(v->domain->vm_event->share.ring_page != NULL) )
> > -vm_event_resume(v->domain, >domain->vm_event->share);
> > +if ( likely(v->domain->vm_event_share->ring_page != NULL) )
> > +vm_event_resume(v->domain, v->domain->vm_event_share);
> >  }
> >  #endif
> For all three a local variable holding v->domain would certain help;
> eventually the functions should even be passed struct domain *
> instead of struct vcpu *, I think.
> 
> > 
> > @@ -599,7 +621,6 @@ int vm_event_domctl(struct domain *d,
> > xen_domctl_vm_event_op_t *vec,
> >  #ifdef CONFIG_HAS_MEM_PAGING
> >  case XEN_DOMCTL_VM_EVENT_OP_PAGING:
> >  {
> > -struct vm_event_domain *ved = >vm_event->paging;
> Dropping this local variable (and similar ones below) pointlessly
> increases the size of this patch.
I have dropped the local var because in case of a XEN_VM_EVENT_ENABLE
d->vm_event_... is allocated in the vm_event_enable function below so
it will allocate mem for the local variable.
> 
> > 
> > --- a/xen/drivers/passthrough/pci.c
> > +++ b/xen/drivers/passthrough/pci.c
> > @@ -1363,9 +1363,11 @@ static int assign_device(struct domain *d,
> > u16 seg, u8 bus, u8 devfn, u32 flag)
> >  
> >  /* Prevent device assign if mem paging or mem sharing have
> > been 
> >   * enabled for this domain */
> > +if( !d->vm_event_paging )
> > +return -EXDEV;
> Is this check the wrong way round? And why can't it be combined
> with ...
> 
> > 
> >  if ( unlikely(!need_iommu(d) &&
> >  (d->arch.hvm_domain.mem_sharing_enabled ||
> > - d->vm_event->paging.ring_page ||
> > + d->vm_event_paging->ring_page ||
> >   p2m_get_hostp2m(d)->global_logdirty)) )
> >  return -EXDEV;
> ... this set?
> 
> Jan
Alex
> 
> 
> This email was scanned by Bitdefender

Re: [Xen-devel] [PATCH v4 12/31] x86/mm: split out readonly MMIO emulation code

[Jan Beulich]

>>> On 17.08.17 at 16:44,  wrote:
> Move the code to pv/emul-mmio-op.c. Fix coding style issues while
> moving.
> 
> Note that mmio_ro_emulated_write is needed by both PV and HVM, so it
> is left in x86/mm.c.
> 
> Signed-off-by: Wei Liu 
> ---
>  xen/arch/x86/mm.c  | 129 
>  xen/arch/x86/pv/Makefile   |   1 +
>  xen/arch/x86/pv/emul-mmio-op.c | 166 
> +

Again I think just mmio.c would do. Other comments on earlier
patches apply here as well.

Jan


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

Re: [Xen-devel] [PATCH v4 11/31] x86/mm: split out writable pagetable emulation code

[Jan Beulich]

>>> On 17.08.17 at 16:44,  wrote:
> Move the code to pv/emul-ptwr-op.c. Fix coding style issues while
> moving the code.
> 
> Rename ptwr_emulated_read to pv_emul_ptwr_read and export it via
> pv/mm.h because it is needed by other emulation code.

If other emulated code uses it, renaming the function would better
imply dropping the ptwr infix from it. pv_emulated_read() perhaps?

> Signed-off-by: Wei Liu 
> ---
>  xen/arch/x86/mm.c  | 308 +-
>  xen/arch/x86/pv/Makefile   |   1 +
>  xen/arch/x86/pv/emul-ptwr-op.c | 327 
> +

Would you mind calling this just ptwr.c?

> +/*
> + * Writable Pagetables
> + */
> +
> +struct ptwr_emulate_ctxt {
> +struct x86_emulate_ctxt ctxt;
> +unsigned long cr2;
> +l1_pgentry_t  pte;
> +};
>[...]
> +static int ptwr_emulated_update(unsigned long addr, paddr_t old, paddr_t val,
> +unsigned int bytes, unsigned int do_cmpxchg,
> +struct ptwr_emulate_ctxt *ptwr_ctxt)

I've meanwhile noticed that in prior patches of yours such movement
was needlessly retaining the component prefixes. With you splitting
things into separate files, these aren't really useful anymore - stack
traces will have them disambiguated by being prefixed with their
file names. They merely eat valuable serial line bandwidth / ring
buffer space and clutter the (serial) log. I could accept the structure
tags to stay the way they are, but please shorten the local function
names as much as possible without losing information. That'll likely
mean dropping more than just the ptwr_ prefix.

Jan


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

Re: [Xen-devel] [PATCH 4/5] xen: support different gnttab_max_frames for grant v1 and v2

[Juergen Gross]

On 24/08/17 17:01, Jan Beulich wrote:
 On 24.08.17 at 16:54,  wrote:
>> OTOH I think there should be a default especially on huge hosts
>> allowing to use v2 grants without reducing the number of allowed
>> grants, which doesn't need adding another parameter to the domain
>> config. Or do you want the tools to always set the per-domain value
>> possibly based on a xl.conf value? Then we could remove the
>> gnttab_max_frames command line parameter completely.
> 
> Yes, obsoleting that command line option would seem quite
> desirable to me.

Okay, I can modify my patches in that regard.


Juergen

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

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Juergen Gross]

On 24/08/17 17:04, Jan Beulich wrote:
 On 24.08.17 at 16:48,  wrote:
>> On 24/08/17 16:28, Jan Beulich wrote:
>> On 21.08.17 at 20:05,  wrote:
 Today a guest can get the maximum grant table frame number for the
 currently selected grant table interface version (1 or 2) only. Add
 a new grant table operation to get the limits of both variants in
 order to give the guest an opportunity to select the version serving
 its needs best.

 Background for the need for this new hypercall is that e.g. the Linux
 kernel won't use v2 as long as this will allow less active grants as
 v1. As soon as the kernel can detect it can create as many v2 entries
 as for v1, it will have no reason not to use v2. And this will allow
 Xen placing a pv-domain with such a kernel above the current 16TB
 RAM limit.

 For setting up the grant table a kernel needs the following
 information:
 - current version (kexec case)
 - current size (kexec case)
 - max size v1
 - max size v2
 In order not to have to issue 3 hypercalls (GNTTABOP_query_size,
 GNTTABOP_get_version, GNTTABOP_get_v1_and_v2_max), let the new
 hypercall return all the needed information.
>>>
>>> I'm not sure I follow: v2 is always going to allow less active grants
>>> than v1, as the limit is always derived from the number of frames
>>> allowed for a domain.
>>
>> Right. Patch 3 adds support for different number of allowed frames for
>> v1 and v2. So the system can be configured to allow the same max.
>> number of grants for v1 and v2, or even more v2 grants than v1.
>>
>>> I also don't see a problem with issuing multiple
>>> calls - none of this ought to be performance critical. I would,
>>> however, agree that rather than adding a new hypercall to just
>>> return the max sizes adding one like you suggest would be
>>> preferable. I'm simply not convinced yet that returning the max
>>> sizes is actually necessary.
>>
>> How would the guest know whether using v2 grants is no disadvantage?
> 
> As said - it's always going to be a disadvantage. Even if controlling
> the number of frames per-domain, that same number of frames
> will always fit more v1 than v2 entries.

And my patches break the assumption "same number of frames".

> I don't think the config
> setting should directly control the number of active grants.

Why not? In the end that number is the one the guest is interested in.
BTW: the number of needed maptrack frames is depending on the number
of grants only, not on v1 or v2. And the default for the max. number
of maptrack frames is much higher than the one of the grant frames
(1024 vs 32).

> Or if we did it that way, then the answer to your question would be
> "based on hypervisor version".

Yeah, or based on the answer from the hypervisor regarding my new
info call (if the answer is "-ENOSYS" the guest probably would choose
v1 as today).


Juergen


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

[Xen-devel] [PATCH v4] x86/pt: fix binding of MSIX entries already unmasked

[Roger Pau Monne]

Hello,

The following two patches fix an issue where a MSIX interrupt bound to
a guest when the MSIX entry is already unmasked would be left masked,
and thus the guest would not receive any interrupts from the device.

This is fixed by adding a new flag to the gflags field used in
XEN_DOMCTL_bind_pt_irq so that the caller can request the interrupt to
be unmasked once bound.

Thanks, Roger.

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

[Xen-devel] [PATCH XEN v4] x86/pt: add a MSI unmask flag to XEN_DOMCTL_bind_pt_irq

[Roger Pau Monne]

The flag is part of the gflags, and should be used to request the
unmask of a MSI interrupt once it's bound.

This is required for the device model in order to be capable of
binding MSIX interrupts that have the entry mask bit already unset at
bind time. Without this fix the interrupts would be left masked.

Note that this commit introduces a change to the domctl, which
requires a bump of the interface version. This is done done here
because the interface version has already been bumped in this release
cycle.

Signed-off-by: Roger Pau Monné 
Reported by: Andreas Kinzler 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
Changes since v2:
 - Use _irqrestore.

Changes since v1:
 - Use pirq_spin_lock_irq_desc.
---
 xen/drivers/passthrough/io.c  | 22 +++---
 xen/include/asm-x86/hvm/irq.h |  1 +
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index 19a21bf85a..1d260bd7ba 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -342,13 +342,14 @@ int pt_irq_create_bind(
 uint8_t dest, dest_mode, delivery_mode;
 int dest_vcpu_id;
 const struct vcpu *vcpu;
+uint32_t gflags = pt_irq_bind->u.msi.gflags & ~VMSI_UNMASKED;
 
 if ( !(pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) )
 {
 pirq_dpci->flags = HVM_IRQ_DPCI_MAPPED | HVM_IRQ_DPCI_MACH_MSI |
HVM_IRQ_DPCI_GUEST_MSI;
 pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
-pirq_dpci->gmsi.gflags = pt_irq_bind->u.msi.gflags;
+pirq_dpci->gmsi.gflags = gflags;
 /*
  * 'pt_irq_create_bind' can be called after 'pt_irq_destroy_bind'.
  * The 'pirq_cleanup_check' which would free the structure is only
@@ -401,13 +402,13 @@ int pt_irq_create_bind(
 
 /* If pirq is already mapped as vmsi, update guest data/addr. */
 if ( pirq_dpci->gmsi.gvec != pt_irq_bind->u.msi.gvec ||
- pirq_dpci->gmsi.gflags != pt_irq_bind->u.msi.gflags )
+ pirq_dpci->gmsi.gflags != gflags )
 {
 /* Directly clear pending EOIs before enabling new MSI info. */
 pirq_guest_eoi(info);
 
 pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
-pirq_dpci->gmsi.gflags = pt_irq_bind->u.msi.gflags;
+pirq_dpci->gmsi.gflags = gflags;
 }
 }
 /* Calculate dest_vcpu_id for MSI-type pirq migration. */
@@ -438,6 +439,21 @@ int pt_irq_create_bind(
 pi_update_irte(vcpu ? >arch.hvm_vmx.pi_desc : NULL,
info, pirq_dpci->gmsi.gvec);
 
+if ( pt_irq_bind->u.msi.gflags & VMSI_UNMASKED )
+{
+unsigned long flags;
+struct irq_desc *desc = pirq_spin_lock_irq_desc(info, );
+
+if ( !desc )
+{
+pt_irq_destroy_bind(d, pt_irq_bind);
+return -EINVAL;
+}
+
+guest_mask_msi_irq(desc, false);
+spin_unlock_irqrestore(>lock, flags);
+}
+
 break;
 }
 
diff --git a/xen/include/asm-x86/hvm/irq.h b/xen/include/asm-x86/hvm/irq.h
index 106dc19613..9546c24879 100644
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -136,6 +136,7 @@ struct dev_intx_gsi_link {
 #define VMSI_DM_MASK  0x200
 #define VMSI_DELIV_MASK   0x7000
 #define VMSI_TRIG_MODE0x8000
+#define VMSI_UNMASKED 0x1
 
 #define GFLAGS_SHIFT_RH 8
 #define GFLAGS_SHIFT_DELIV_MODE 12
-- 
2.11.0 (Apple Git-81)


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

[Xen-devel] [PATCH QEMU v4] xen/pt: allow QEMU to request MSI unmasking at bind time

[Roger Pau Monne]

When a MSI interrupt is bound to a guest using
xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is
left masked by default.

This causes problems with guests that first configure interrupts and
clean the per-entry MSIX table mask bit and afterwards enable MSIX
globally. In such scenario the Xen internal msixtbl handlers would not
detect the unmasking of MSIX entries because vectors are not yet
registered since MSIX is not enabled, and vectors would be left
masked.

Introduce a new flag in the gflags field to signal Xen whether a MSI
interrupt should be unmasked after being bound.

This also requires to track the mask register for MSI interrupts, so
QEMU can also notify to Xen whether the MSI interrupt should be bound
masked or unmasked

Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 
Reported-by: Andreas Kinzler 
---
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Jan Beulich 
Cc: qemu-de...@nongnu.org
---
Changes since v2:
 - Fix coding style.

Changes since v1:
 - Track MSI mask bits.
 - Notify Xen of whether MSI interrupts should be unmasked after
   binding, instead of hardcoding it.
---
 hw/xen/xen_pt.h |  1 +
 hw/xen/xen_pt_config_init.c | 20 ++--
 hw/xen/xen_pt_msi.c | 13 ++---
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 191d9caea1..aa39a9aa5f 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -180,6 +180,7 @@ typedef struct XenPTMSI {
 uint32_t addr_hi;  /* guest message upper address */
 uint16_t data; /* guest message data */
 uint32_t ctrl_offset; /* saved control offset */
+uint32_t mask; /* guest mask bits */
 int pirq;  /* guest pirq corresponding */
 bool initialized;  /* when guest MSI is initialized */
 bool mapped;   /* when pirq is mapped */
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 1f04ec5eec..a3ce33e78b 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1315,6 +1315,22 @@ static int 
xen_pt_msgdata_reg_write(XenPCIPassthroughState *s,
 return 0;
 }
 
+static int xen_pt_mask_reg_write(XenPCIPassthroughState *s, XenPTReg 
*cfg_entry,
+ uint32_t *val, uint32_t dev_value,
+ uint32_t valid_mask)
+{
+int rc;
+
+rc = xen_pt_long_reg_write(s, cfg_entry, val, dev_value, valid_mask);
+if (rc) {
+return rc;
+}
+
+s->msi->mask = *val;
+
+return 0;
+}
+
 /* MSI Capability Structure reg static information table */
 static XenPTRegInfo xen_pt_emu_reg_msi[] = {
 /* Next Pointer reg */
@@ -1393,7 +1409,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
 .emu_mask   = 0x,
 .init   = xen_pt_mask_reg_init,
 .u.dw.read  = xen_pt_long_reg_read,
-.u.dw.write = xen_pt_long_reg_write,
+.u.dw.write = xen_pt_mask_reg_write,
 },
 /* Mask reg (if PCI_MSI_FLAGS_MASKBIT set, for 64-bit devices) */
 {
@@ -1404,7 +1420,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
 .emu_mask   = 0x,
 .init   = xen_pt_mask_reg_init,
 .u.dw.read  = xen_pt_long_reg_read,
-.u.dw.write = xen_pt_long_reg_write,
+.u.dw.write = xen_pt_mask_reg_write,
 },
 /* Pending reg (if PCI_MSI_FLAGS_MASKBIT set, for 32-bit devices) */
 {
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index ff9a79f5d2..6d1e3bdeb4 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -24,6 +24,7 @@
 #define XEN_PT_GFLAGS_SHIFT_DM 9
 #define XEN_PT_GFLAGSSHIFT_DELIV_MODE 12
 #define XEN_PT_GFLAGSSHIFT_TRG_MODE   15
+#define XEN_PT_GFLAGSSHIFT_UNMASKED   16
 
 #define latch(fld) latch[PCI_MSIX_ENTRY_##fld / sizeof(uint32_t)]
 
@@ -155,7 +156,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
int pirq,
bool is_msix,
int msix_entry,
-   int *old_pirq)
+   int *old_pirq,
+   bool masked)
 {
 PCIDevice *d = >dev;
 uint8_t gvec = msi_vector(data);
@@ -171,6 +173,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
 table_addr = s->msix->mmio_base_addr;
 }
 
+gflags |= masked ? 0 : (1u << XEN_PT_GFLAGSSHIFT_UNMASKED);
+
 rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
   pirq, gflags, table_addr);
 
@@ -273,8 +277,10 @@ int xen_pt_msi_setup(XenPCIPassthroughState *s)
 int xen_pt_msi_update(XenPCIPassthroughState *s)
 {
 XenPTMSI *msi = s->msi;
+
+/* Current MSI emulation in QEMU only supports 1 vector */
 return msi_msix_update(s, msi_addr64(msi), msi->data, msi->pirq,
-   false, 

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Jan Beulich]

>>> On 24.08.17 at 16:48,  wrote:
> On 24/08/17 16:28, Jan Beulich wrote:
> On 21.08.17 at 20:05,  wrote:
>>> Today a guest can get the maximum grant table frame number for the
>>> currently selected grant table interface version (1 or 2) only. Add
>>> a new grant table operation to get the limits of both variants in
>>> order to give the guest an opportunity to select the version serving
>>> its needs best.
>>>
>>> Background for the need for this new hypercall is that e.g. the Linux
>>> kernel won't use v2 as long as this will allow less active grants as
>>> v1. As soon as the kernel can detect it can create as many v2 entries
>>> as for v1, it will have no reason not to use v2. And this will allow
>>> Xen placing a pv-domain with such a kernel above the current 16TB
>>> RAM limit.
>>>
>>> For setting up the grant table a kernel needs the following
>>> information:
>>> - current version (kexec case)
>>> - current size (kexec case)
>>> - max size v1
>>> - max size v2
>>> In order not to have to issue 3 hypercalls (GNTTABOP_query_size,
>>> GNTTABOP_get_version, GNTTABOP_get_v1_and_v2_max), let the new
>>> hypercall return all the needed information.
>> 
>> I'm not sure I follow: v2 is always going to allow less active grants
>> than v1, as the limit is always derived from the number of frames
>> allowed for a domain.
> 
> Right. Patch 3 adds support for different number of allowed frames for
> v1 and v2. So the system can be configured to allow the same max.
> number of grants for v1 and v2, or even more v2 grants than v1.
> 
>> I also don't see a problem with issuing multiple
>> calls - none of this ought to be performance critical. I would,
>> however, agree that rather than adding a new hypercall to just
>> return the max sizes adding one like you suggest would be
>> preferable. I'm simply not convinced yet that returning the max
>> sizes is actually necessary.
> 
> How would the guest know whether using v2 grants is no disadvantage?

As said - it's always going to be a disadvantage. Even if controlling
the number of frames per-domain, that same number of frames
will always fit more v1 than v2 entries. I don't think the config
setting should directly control the number of active grants. Or if
we did it that way, then the answer to your question would be
"based on hypervisor version".

Jan


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

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Juergen Gross]

On 24/08/17 16:48, Juergen Gross wrote:
> On 24/08/17 16:28, Jan Beulich wrote:
> On 21.08.17 at 20:05,  wrote:
>>> Today a guest can get the maximum grant table frame number for the
>>> currently selected grant table interface version (1 or 2) only. Add
>>> a new grant table operation to get the limits of both variants in
>>> order to give the guest an opportunity to select the version serving
>>> its needs best.
>>>
>>> Background for the need for this new hypercall is that e.g. the Linux
>>> kernel won't use v2 as long as this will allow less active grants as
>>> v1. As soon as the kernel can detect it can create as many v2 entries
>>> as for v1, it will have no reason not to use v2. And this will allow
>>> Xen placing a pv-domain with such a kernel above the current 16TB
>>> RAM limit.
>>>
>>> For setting up the grant table a kernel needs the following
>>> information:
>>> - current version (kexec case)
>>> - current size (kexec case)
>>> - max size v1
>>> - max size v2
>>> In order not to have to issue 3 hypercalls (GNTTABOP_query_size,
>>> GNTTABOP_get_version, GNTTABOP_get_v1_and_v2_max), let the new
>>> hypercall return all the needed information.
>>
>> I'm not sure I follow: v2 is always going to allow less active grants
>> than v1, as the limit is always derived from the number of frames
>> allowed for a domain.
> 
> Right. Patch 3 adds support for different number of allowed frames for

This should read "Patch 4", of course

> v1 and v2. So the system can be configured to allow the same max.
> number of grants for v1 and v2, or even more v2 grants than v1.
> 
>> I also don't see a problem with issuing multiple
>> calls - none of this ought to be performance critical. I would,
>> however, agree that rather than adding a new hypercall to just
>> return the max sizes adding one like you suggest would be
>> preferable. I'm simply not convinced yet that returning the max
>> sizes is actually necessary.
> 
> How would the guest know whether using v2 grants is no disadvantage?
> 
> 
> Juergen
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 


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

Re: [Xen-devel] [PATCH 4/5] xen: support different gnttab_max_frames for grant v1 and v2

[Jan Beulich]

>>> On 24.08.17 at 16:54,  wrote:
> OTOH I think there should be a default especially on huge hosts
> allowing to use v2 grants without reducing the number of allowed
> grants, which doesn't need adding another parameter to the domain
> config. Or do you want the tools to always set the per-domain value
> possibly based on a xl.conf value? Then we could remove the
> gnttab_max_frames command line parameter completely.

Yes, obsoleting that command line option would seem quite
desirable to me.

Jan


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

Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Juergen Gross]

On 24/08/17 16:50, Andrew Cooper wrote:
> This patch was originally a workaround for XSA-226.  Since then, transitive
> grants are believed to be functioning properly, and the defaults have changed
> appropriately.
> 
> However, for those people who chose to use the workaround (especially from an
> attack surface mitigation point of view), retain the ability for the host
> administrator to choose.
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: George Dunlap 
> CC: Konrad Rzeszutek Wilk 
> CC: Stefano Stabellini 
> CC: Tim Deegan 
> CC: Wei Liu 
> ---
>  docs/misc/xen-command-line.markdown | 13 +++
>  xen/common/grant_table.c| 44 
> +++--
>  2 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index 4002eab..78c7b51 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -868,6 +868,19 @@ Controls EPT related features.
>  
>  Specify which console gdbstub should use. See **console**.
>  
> +### gnttab
> +> `= List of [ max_ver:, transitive ]`
> +
> +> Default: `gnttab=max_ver:2,transitive`
> +
> +Control various aspects of the grant table behaviour available to guests.
> +
> +* `max_ver` Select the maximum grant table version to offer to guests.  Valid
> +version are 1 and 2.
> +* `transitive` Permit or disallow the use of transitive grants.  Note that 
> the
> +use of grant table v2 without transitive grants is an ABI breakage from the
> +guests point of view.

So shouldn't there be a way for the guest to query the support of
transient grants?

> +
>  ### gnttab\_max\_frames
>  > `= `
>  
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 36895aa..f9c313d 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", max_nr_grant_frames);
>  unsigned int __read_mostly max_grant_frames;
>  integer_param("gnttab_max_frames", max_grant_frames);
>  
> +static unsigned int __read_mostly opt_gnttab_max_version = 2;
> +static bool __read_mostly opt_transitive_grants = true;
> +
> +static void __init parse_gnttab(char *s)

Do you mind using:

static int __init parse_gnttab(const char *s)

in order to comply with my runtime parameter series?


Juergen

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

Re: [Xen-devel] [PATCH v4 05/11] arm: add SMCCC protocol definitions.

[Julien Grall]

Hi Volodymyr,

Title: No need for the full stop.

On 21/08/17 21:27, Volodymyr Babchuk wrote:

This patch adds generic definitions used in ARM SMC call convention.
Those definitions was taken from linux header arm-smccc.h, extended
and formatted according to XEN coding style.

They can be used by both SMCCC clients (like PSCI) and by SMCCC
servers (like vPSCI or upcoming generic SMCCC handler).

Signed-off-by: Volodymyr Babchuk 
---
 xen/include/asm-arm/smccc.h | 92 +
 1 file changed, 92 insertions(+)
 create mode 100644 xen/include/asm-arm/smccc.h

diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
new file mode 100644
index 000..67da3fb
--- /dev/null
+++ b/xen/include/asm-arm/smccc.h
@@ -0,0 +1,92 @@
+/*
+ * Copyright (c) 2015, Linaro Limited


Linaro? Where does this code come from?


+ * Copyright (c) 2017, EPAM Systems
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#ifndef __ASM_ARM_SMCCC_H__
+#define __ASM_ARM_SMCCC_H__
+
+/*
+ * This file provides common defines for ARM SMC Calling Convention as
+ * specified in
+ * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
+ */
+
+#define ARM_SMCCC_STD_CALL  0U
+#define ARM_SMCCC_FAST_CALL 1U
+#define ARM_SMCCC_TYPE_SHIFT31
+
+#define ARM_SMCCC_SMC_320U
+#define ARM_SMCCC_SMC_641U


I am not sure to understand why you embed SMC in the name? The 
convention is SMC32/HVC32. So I would name


ARM_SMCCC_CONV_{32,64}


+#define ARM_SMCCC_CALL_CONV_SHIFT   30


Also, I would rename to ARM_SMCCC_CONV to fit with the value above.


+
+#define ARM_SMCCC_OWNER_MASK0x3F


Missing U.


+#define ARM_SMCCC_OWNER_SHIFT   24
+
+#define ARM_SMCCC_FUNC_MASK 0x
+
+/* Check if this is fast call */
+#define ARM_SMCCC_IS_FAST_CALL(smc_val) \
+((smc_val) & (ARM_SMCCC_FAST_CALL << ARM_SMCCC_TYPE_SHIFT))
+
+/* Check if this is 64 bit call  */
+#define ARM_SMCCC_IS_64(smc_val)\
+((smc_val) & (ARM_SMCCC_SMC_64 << ARM_SMCCC_CALL_CONV_SHIFT))
+
+/* Get function number from function identifier */
+#define ARM_SMCCC_FUNC_NUM(smc_val) ((smc_val) & ARM_SMCCC_FUNC_MASK)
+
+/* Get service owner number from function identifier */
+#define ARM_SMCCC_OWNER_NUM(smc_val)\
+(((smc_val) >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK)


Can we please use static inline helper for the 4 macros above?


+
+/*
+ * Construct function identifier from call type (fast or standard),
+ * calling convention (32 or 64 bit), service owner and function number
+ */
+#define ARM_SMCCC_CALL_VAL(type, calling_convention, owner, func_num)   \
+(((type) << ARM_SMCCC_TYPE_SHIFT) | \
+ ((calling_convention) << ARM_SMCCC_CALL_CONV_SHIFT) |  \
+ (((owner) & ARM_SMCCC_OWNER_MASK) << ARM_SMCCC_OWNER_SHIFT) |  \
+ ((func_num) & ARM_SMCCC_FUNC_MASK))


The indentation looks wrong here. Also I think the (& *MASK) is not 
necessary here. It would be wrong by the user to pass a wrong value and 
even with the masking you would end up to wrong result.


If you are really worry of wrong result, then you should use 
BUILD_BUG_ON()/BUG_ON().



+
+/* List of known service owners */
+#define ARM_SMCCC_OWNER_ARCH0
+#define ARM_SMCCC_OWNER_CPU 1
+#define ARM_SMCCC_OWNER_SIP 2
+#define ARM_SMCCC_OWNER_OEM 3
+#define ARM_SMCCC_OWNER_STANDARD4
+#define ARM_SMCCC_OWNER_HYPERVISOR  5
+#define ARM_SMCCC_OWNER_TRUSTED_APP 48
+#define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
+#define ARM_SMCCC_OWNER_TRUSTED_OS  50
+#define ARM_SMCCC_OWNER_TRUSTED_OS_END  63
+
+/* List of generic function numbers */
+#define ARM_SMCCC_FUNC_CALL_COUNT   0xFF00
+#define ARM_SMCCC_FUNC_CALL_UID 0xFF01
+#define ARM_SMCCC_FUNC_CALL_REVISION0xFF03
+
+/* Only one error code defined in SMCCC */
+#define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
+
+#endif  /* __ASM_ARM_SMCCC_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:b
+ */



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 10/31] x86/mm: move {un, }adjust_guest_* to pv/mm.h

[Jan Beulich]

>>> On 17.08.17 at 16:44,  wrote:
> Those macros will soon be used in different files. They are PV
> specific so move them to pv/mm.h.

Same comment here regarding where to move them.

Jan


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

Re: [Xen-devel] [PATCH v4 09/31] x86/mm: rename and move update_intpte

[Jan Beulich]

>>> On 17.08.17 at 16:44,  wrote:
> That function is only used by PV guests supporting code, add pv_
> prefix.

Is any code outside of x86/pv/ going to need access to it? I hope not,
in which case it shouldn't be exposed via include/asm-x86/pv/mm.h,
but via a private header in x86/pv/. That non-private header should
have only declarations of things needed by non-PV/HVM-specific code.

Jan


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

Re: [Xen-devel] [PATCH v4 08/31] x86/mm: export get_page_from_mfn

[Jan Beulich]

>>> On 17.08.17 at 16:44,  wrote:
> It will be used by different files later, so export it via
> asm-x86/mm.h.

This is a pretty thin wrapper - wouldn't be better to make it a
static inline?

Jan


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

Re: [Xen-devel] [PATCH 4/5] xen: support different gnttab_max_frames for grant v1 and v2

[Juergen Gross]

On 24/08/17 16:21, Jan Beulich wrote:
 On 21.08.17 at 20:05,  wrote:
>> The number of grants a domain can setup is limited by the maximum
>> number of grant frames it is allowed to use. Today the limit is the
>> same regardless whether the domain uses grant v1 or v2. Using v2
>> will therefor be a disadvantage for the domain as only half the
>> number of grants compared to v1 can be used, because a grant v2 entry
>> is twice as large as the v1 entry. This is the reason for the lack of
>> grant v2 support in the Linux kernel (in fact grant v2 support has
>> been removed from Linux for this reason).
>>
>> OTOH using only grant v1 will limit a pv domain to the low 16TB of
>> memory of the host, as grant v1 entries only use a 32 bit mfn. So
>> if we want to support more than 16TB of memory and be able to use
>> that memory in pv domains, we have to remove the disadvantage of
>> using grant v2 by being able to setup the same number of grants as
>> with v1.
>>
>> In order to achieve this add support for limiting the number of grant
>> frames for v1 and v2 independently from each other. Per default let
>> the v2 number be twice the value of the v1 number. Modify the boot
>> parameter gnttab_max_frames to accept either a single value which
>> will set the v1 limit to that value and the v2 limit to 2*value, or
>> two values separated by a comma to set both limits to dedicated
>> values.
>>
>> Add some sanity checks to make sure the maximum number of frames isn't
>> lower than the initial number, as this leads to rather strange crashes.
>>
>> Signed-off-by: Juergen Gross 
> 
> As discussed elsewhere, this probably rather wants to become a
> per-domain setting then. Looking also at what patch 5 adds, I
> additionally wonder whether we shouldn't allow Dom0 to know
> whether it needs to use v2 at all (or maybe it can derive that
> already).

I'm absolutely in favor of adding a way to make this a per-domain
setting.

OTOH I think there should be a default especially on huge hosts
allowing to use v2 grants without reducing the number of allowed
grants, which doesn't need adding another parameter to the domain
config. Or do you want the tools to always set the per-domain value
possibly based on a xl.conf value? Then we could remove the
gnttab_max_frames command line parameter completely.


Juergen

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

Re: [Xen-devel] [PATCH v4 07/31] x86/mm: move and rename guest_get_eff{, kern}_l1e

[Jan Beulich]

>>> On 17.08.17 at 16:44,  wrote:
> Move them to pv/mm.c and rename them to pv_get_guest_eff_{,kern}_l1e.
> Export them via pv/mm.h.

I think these should remain static inlines. If either is really needed by
more than one C file, it may be best to move it/them to a private
header in x86/pv/. But guest_get_eff_kern_l1e() clearly has just a
single caller.

Jan


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

[Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Andrew Cooper]

This patch was originally a workaround for XSA-226.  Since then, transitive
grants are believed to be functioning properly, and the defaults have changed
appropriately.

However, for those people who chose to use the workaround (especially from an
attack surface mitigation point of view), retain the ability for the host
administrator to choose.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: George Dunlap 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 
---
 docs/misc/xen-command-line.markdown | 13 +++
 xen/common/grant_table.c| 44 +++--
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 4002eab..78c7b51 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -868,6 +868,19 @@ Controls EPT related features.
 
 Specify which console gdbstub should use. See **console**.
 
+### gnttab
+> `= List of [ max_ver:, transitive ]`
+
+> Default: `gnttab=max_ver:2,transitive`
+
+Control various aspects of the grant table behaviour available to guests.
+
+* `max_ver` Select the maximum grant table version to offer to guests.  Valid
+version are 1 and 2.
+* `transitive` Permit or disallow the use of transitive grants.  Note that the
+use of grant table v2 without transitive grants is an ABI breakage from the
+guests point of view.
+
 ### gnttab\_max\_frames
 > `= `
 
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 36895aa..f9c313d 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", max_nr_grant_frames);
 unsigned int __read_mostly max_grant_frames;
 integer_param("gnttab_max_frames", max_grant_frames);
 
+static unsigned int __read_mostly opt_gnttab_max_version = 2;
+static bool __read_mostly opt_transitive_grants = true;
+
+static void __init parse_gnttab(char *s)
+{
+char *ss;
+
+do {
+ss = strchr(s, ',');
+if ( ss )
+*ss = '\0';
+
+if ( !strncmp(s, "max_ver:", 8) )
+{
+long ver = simple_strtol(s + 8, NULL, 10);
+
+if ( ver >= 1 && ver <= 2 )
+opt_gnttab_max_version = ver;
+}
+else
+{
+bool val = !!strncmp(s, "no-", 3);
+
+if ( !val )
+s += 3;
+
+if ( !strcmp(s, "transitive") )
+opt_transitive_grants = val;
+}
+
+s = ss + 1;
+} while ( ss );
+}
+
+custom_param("gnttab", parse_gnttab);
+
 /* The maximum number of grant mappings is defined as a multiplier of the
  * maximum number of grant table entries. This defines the multiplier used.
  * Pretty arbitrary. [POLICY]
@@ -2505,7 +2541,8 @@ static int gnttab_copy_claim_buf(const struct gnttab_copy 
*op,
 current->domain->domain_id,
 buf->read_only,
 >frame, >page,
->ptr.offset, >len, true);
+>ptr.offset, >len,
+opt_transitive_grants);
 if ( rc != GNTST_okay )
 goto out;
 buf->ptr.u.ref = ptr->u.ref;
@@ -3237,7 +3274,10 @@ do_grant_table_op(
 break;
 
 case GNTTABOP_set_version:
-rc = gnttab_set_version(guest_handle_cast(uop, gnttab_set_version_t));
+if ( opt_gnttab_max_version == 1 )
+rc = -ENOSYS; /* Behave as before set_version was introduced. */
+else
+rc = gnttab_set_version(guest_handle_cast(uop, 
gnttab_set_version_t));
 break;
 
 case GNTTABOP_get_status_frames:
-- 
2.1.4


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

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Juergen Gross]

On 24/08/17 16:28, Jan Beulich wrote:
 On 21.08.17 at 20:05,  wrote:
>> Today a guest can get the maximum grant table frame number for the
>> currently selected grant table interface version (1 or 2) only. Add
>> a new grant table operation to get the limits of both variants in
>> order to give the guest an opportunity to select the version serving
>> its needs best.
>>
>> Background for the need for this new hypercall is that e.g. the Linux
>> kernel won't use v2 as long as this will allow less active grants as
>> v1. As soon as the kernel can detect it can create as many v2 entries
>> as for v1, it will have no reason not to use v2. And this will allow
>> Xen placing a pv-domain with such a kernel above the current 16TB
>> RAM limit.
>>
>> For setting up the grant table a kernel needs the following
>> information:
>> - current version (kexec case)
>> - current size (kexec case)
>> - max size v1
>> - max size v2
>> In order not to have to issue 3 hypercalls (GNTTABOP_query_size,
>> GNTTABOP_get_version, GNTTABOP_get_v1_and_v2_max), let the new
>> hypercall return all the needed information.
> 
> I'm not sure I follow: v2 is always going to allow less active grants
> than v1, as the limit is always derived from the number of frames
> allowed for a domain.

Right. Patch 3 adds support for different number of allowed frames for
v1 and v2. So the system can be configured to allow the same max.
number of grants for v1 and v2, or even more v2 grants than v1.

> I also don't see a problem with issuing multiple
> calls - none of this ought to be performance critical. I would,
> however, agree that rather than adding a new hypercall to just
> return the max sizes adding one like you suggest would be
> preferable. I'm simply not convinced yet that returning the max
> sizes is actually necessary.

How would the guest know whether using v2 grants is no disadvantage?


Juergen

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

Re: [Xen-devel] [PATCH v4 04/11] arm: processor.h: add definition for immediate value mask

[Julien Grall]

Hi Volodymyr,

Title: It is a too generic, you may want to rename to: "Define HVC/SMC 
immediate mask"


On 21/08/17 21:27, Volodymyr Babchuk wrote:

This patch adds definition HSR_XXC_IMM_MASK. It can be used to extract


s/adds definition/define/


immediate value for trapped HVC32, HVC64, SMC64, SVC32, SVC64 instructions,
as described at ARM ARM (ARM DDI 0487B.a pages D7-2270, D7-2272).


s/at/in the/



Signed-off-by: Volodymyr Babchuk 
---
 xen/include/asm-arm/processor.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 51ce802..89752a7 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -580,6 +580,9 @@ union hsr {
   HSR_SYSREG_CRN_MASK|HSR_SYSREG_CRM_MASK|\
   HSR_SYSREG_OP2_MASK)

+/* HSR.EC == HSR_{HVC32, HVC64, SMC64, SVC32, SVC64} */
+#define HSR_XXC_IMM_MASK (0x)
+
 /* Physical Address Register */
 #define PAR_F   (_AC(1,U)<<0)




Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 02/11] arm: traps: check if SMC was conditional before handling it

[Julien Grall]

Hi Volodymyr,

On 21/08/17 21:27, Volodymyr Babchuk wrote:

Trapped SMC instruction can fail condition check on ARMv8 arhcitecture


s/arhcitecture/architecture/


(ARM DDI 0487B.a page D7-2271). So we need to check if condition was meet.

Signed-off-by: Volodymyr Babchuk 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH QEMU v2] xen/pt: allow QEMU to request MSI unmasking at bind time

[Roger Pau Monne]

On Thu, Aug 24, 2017 at 07:09:08AM -0600, Jan Beulich wrote:
> >>> On 24.08.17 at 14:19,  wrote:
> > When a MSI interrupt is bound to a guest using
> > xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is
> > left masked by default.
> > 
> > This causes problems with guests that first configure interrupts and
> > clean the per-entry MSIX table mask bit and afterwards enable MSIX
> > globally. In such scenario the Xen internal msixtbl handlers would not
> > detect the unmasking of MSIX entries because vectors are not yet
> > registered since MSIX is not enabled, and vectors would be left
> > masked.
> > 
> > Introduce a new flag in the gflags field to signal Xen whether a MSI
> > interrupt should be unmasked after being bound.
> > 
> > This also requires to track the mask register for MSI interrupts, so
> > QEMU can also notify to Xen whether the MSI interrupt should be bound
> > masked or unmasked
> > 
> > Signed-off-by: Roger Pau Monné 
> > Reported-by: Andreas Kinzler 
> 
> Reviewed-by: Jan Beulich 

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v4 01/11] arm: traps: use generic register accessors in the PSCI code

[Julien Grall]

Hi Volodymyr,

On 21/08/17 21:27, Volodymyr Babchuk wrote:

There are standard functions set_user_reg() and get_user_reg(). We can
use them in PSCI_SET_RESULT()/PSCI_ARG() macroses instead of relying on


s/macroses/macros/ I think.


CONFIG_ARM_64 definition.

Signed-off-by: Volodymyr Babchuk 
---

* Added space into reg,n
* Used 32-bit constant tin PSCI_ARG32

---
xen/arch/arm/traps.c | 40 ++--
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 13efb58..66f12cb 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1450,14 +1450,13 @@ static void do_debug_trap(struct cpu_user_regs *regs, 
unsigned int code)
 }
 #endif

+#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
+#define PSCI_ARG(reg, n) get_user_reg(reg, n)
+
 #ifdef CONFIG_ARM_64
-#define PSCI_RESULT_REG(reg) (reg)->x0
-#define PSCI_ARG(reg,n) (reg)->x##n
-#define PSCI_ARG32(reg,n) (uint32_t)( (reg)->x##n & 0x )
+#define PSCI_ARG32(reg, n) (uint32_t)(get_user_reg(reg, n) & 0x)


Please drop the mask, the cast is sufficient.


 #else
-#define PSCI_RESULT_REG(reg) (reg)->r0
-#define PSCI_ARG(reg,n) (reg)->r##n
-#define PSCI_ARG32(reg,n) PSCI_ARG(reg,n)
+#define PSCI_ARG32(reg, n) PSCI_ARG(reg, n)


Please mention in the commit message that you also modify the coding style.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2] x86: enable RCU based table free

[Peter Zijlstra]

On Thu, Aug 24, 2017 at 11:22:58AM +0200, Vitaly Kuznetsov wrote:

> diff --git a/arch/x86/include/asm/tlb.h b/arch/x86/include/asm/tlb.h
> index c7797307fc2b..d43a7fcafee9 100644
> --- a/arch/x86/include/asm/tlb.h
> +++ b/arch/x86/include/asm/tlb.h
> @@ -15,4 +15,9 @@
>  
>  #include 
>  
> +static inline void __tlb_remove_table(void *table)
> +{
> + free_page_and_swap_cache(table);
> +}

Most other archs have this in pgtable.h, only ARM* has it in tlb.h.

And should we put a comment on explaining _why_ we have RCU_TABLE_FREE
enabled?

>  #endif /* _ASM_X86_TLB_H */
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index 508a708eb9a6..218834a3e9ad 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -56,7 +56,7 @@ void ___pte_free_tlb(struct mmu_gather *tlb, struct page 
> *pte)
>  {
>   pgtable_page_dtor(pte);
>   paravirt_release_pte(page_to_pfn(pte));
> - tlb_remove_page(tlb, pte);
> + tlb_remove_table(tlb, pte);
>  }
>  
>  #if CONFIG_PGTABLE_LEVELS > 2
> @@ -72,21 +72,21 @@ void ___pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd)
>   tlb->need_flush_all = 1;
>  #endif
>   pgtable_pmd_page_dtor(page);
> - tlb_remove_page(tlb, page);
> + tlb_remove_table(tlb, page);
>  }
>  
>  #if CONFIG_PGTABLE_LEVELS > 3
>  void ___pud_free_tlb(struct mmu_gather *tlb, pud_t *pud)
>  {
>   paravirt_release_pud(__pa(pud) >> PAGE_SHIFT);
> - tlb_remove_page(tlb, virt_to_page(pud));
> + tlb_remove_table(tlb, virt_to_page(pud));
>  }
>  
>  #if CONFIG_PGTABLE_LEVELS > 4
>  void ___p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d)
>  {
>   paravirt_release_p4d(__pa(p4d) >> PAGE_SHIFT);
> - tlb_remove_page(tlb, virt_to_page(p4d));
> + tlb_remove_table(tlb, virt_to_page(p4d));
>  }
>  #endif   /* CONFIG_PGTABLE_LEVELS > 4 */
>  #endif   /* CONFIG_PGTABLE_LEVELS > 3 */
> -- 
> 2.13.5
> 

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

Re: [Xen-devel] [PATCH 4/6] xsm: flask: change the interface and default policy for xsm_map_gmfn_foregin

[Daniel De Graaf]

On 08/24/2017 08:39 AM, Jan Beulich wrote:

On 24.08.17 at 13:33,  wrote:

Hi Jan,

2017-08-24 14:37 GMT+08:00 Jan Beulich :

On 24.08.17 at 02:51,  wrote:

2017-08-23 17:55 GMT+08:00 Jan Beulich :

On 22.08.17 at 20:08,  wrote:

--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -525,10 +525,12 @@ static XSM_INLINE int

xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,

  return xsm_default_action(action, d1, d2);
  }

-static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain

*d, struct domain *t)

+static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain

*cd,

+   struct domain *d, struct domain

*t)

  {
  XSM_ASSERT_ACTION(XSM_TARGET);
-return xsm_default_action(action, d, t);
+return xsm_default_action(action, cd, d) ||
+xsm_default_action(action, cd, t);
  }


... you use "or" here and ...


This might be confusing. But think of returning 0 as "allowed", the
only condition where this
statement returns a 0 is when both calls return 0 -- so it's actually
an "and". (Think of de-morgan's law.)

But as Stefano has pointed out, I should preserve the error code.


Ah, right - the code as written suggests boolean return values,
which gives it the wrong look. You really mean ?: instead of ||.
Why do you, btw, pass in current->domain (as cd) instead of
obtaining it here (just like various other hooks do)?


This was my original plan, but I'm not very sure about this, so I turn
to Julien for help, and...
Here is part of the irc log from a discussion with Julien on
#xendevel, where Julien said:

blackskygg: I think you want to pass the current domain in
parameter, i.e having 3 domains argument.
because your solution only works when XSM is not enabled (this is
the dummy callback)
when XSM is enabled, the policy would be specificed by the administrator
he needs to be able to know which domain was doing the configuration.


in flask/hooks.c there are quite a few uses of
avc_current_has_perm() in such cases, so I would think not
handing current->domain through the hook should be fine. But
of course Daniel may tell you I'm completely wrong here.

Jan


This is really just a choice of whatever looks better.  There's a very minor
performance penalty from not calling current->domain over and over, but there
might also be a performance gain if current_has_perm is not inlined and the
call results in smaller code size.

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

Re: [Xen-devel] [PATCH v4 49/53] xen: add hypercall for setting parameters at runtime

[Wei Liu]

On Wed, Aug 23, 2017 at 07:34:42PM +0200, Juergen Gross wrote:
> Add a sysctl hypercall to support setting parameters similar to
> command line parameters, but at runtime. The parameters to set are
> specified as a string, just like the boot parameters.
> 
> Cc: Daniel De Graaf 
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Jan Beulich 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Signed-off-by: Juergen Gross 
> Acked-by: Daniel De Graaf 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 3/5] xen: clean up grant_table.h

[Juergen Gross]

On 24/08/17 16:17, Jan Beulich wrote:
 On 21.08.17 at 20:05,  wrote:
>> @@ -118,6 +154,18 @@ struct grant_mapping {
>>  uint32_t pad;   /* round size to a power of 2 */
>>  };
>>  
>> +/* Number of grant table frames. Caller must hold d's grant table lock. */
>> +static inline unsigned int nr_grant_frames(struct grant_table *gt)
>> +{
>> +return gt->nr_grant_frames;
>> +}
>> +
>> +/* Number of status grant table frames. Caller must hold d's gr. table 
>> lock.*/
>> +static inline unsigned int nr_status_frames(struct grant_table *gt)
>> +{
>> +return gt->nr_status_frames;
>> +}
> 
> For both the parameters want to be constified.

Okay.

> 
>> @@ -250,6 +318,15 @@ static inline void active_entry_release(struct 
>> active_grant_entry *act)
>>  spin_unlock(>lock);
>>  }
>>  
>> +#define GRANT_STATUS_PER_PAGE (PAGE_SIZE / sizeof(grant_status_t))
>> +#define GRANT_PER_PAGE (PAGE_SIZE / sizeof(grant_entry_v2_t))
>> +/* Number of grant table status entries. Caller must hold d's gr. table 
>> lock.*/
>> +static inline unsigned int grant_to_status_frames(int grant_frames)
> 
> unsigned int also for the parameter.

Okay.

> 
>> --- a/xen/include/xen/grant_table.h
>> +++ b/xen/include/xen/grant_table.h
>> @@ -29,65 +29,8 @@
>>  #include 
>>  #include 
>>  
>> -#ifndef DEFAULT_MAX_NR_GRANT_FRAMES /* to allow arch to override */
>> -/* Default maximum size of a grant table. [POLICY] */
>> -#define DEFAULT_MAX_NR_GRANT_FRAMES   32
>> -#endif
>>  /* The maximum size of a grant table. */
>> -extern unsigned int max_grant_frames;
>> -
>> -DECLARE_PERCPU_RWLOCK_GLOBAL(grant_rwlock);
>> -
>> -/* Per-domain grant information. */
>> -struct grant_table {
>> -/*
>> - * Lock protecting updates to grant table state (version, active
>> - * entry list, etc.)
>> - */
>> -percpu_rwlock_t   lock;
>> -/* Table size. Number of frames shared with guest */
>> -unsigned int  nr_grant_frames;
>> -/* Shared grant table (see include/public/grant_table.h). */
>> -union {
>> -void **shared_raw;
>> -struct grant_entry_v1 **shared_v1;
>> -union grant_entry_v2 **shared_v2;
>> -};
>> -/* Number of grant status frames shared with guest (for version 2) */
>> -unsigned int  nr_status_frames;
>> -/* State grant table (see include/public/grant_table.h). */
>> -grant_status_t   **status;
>> -/* Active grant table. */
>> -struct active_grant_entry **active;
>> -/* Mapping tracking table per vcpu. */
>> -struct grant_mapping **maptrack;
>> -unsigned int  maptrack_limit;
>> -/* Lock protecting the maptrack limit */
>> -spinlock_tmaptrack_lock;
>> -/* The defined versions are 1 and 2.  Set to 0 if we don't know
>> -   what version to use yet. */
>> -unsigned  gt_version;
>> -};
>> -
>> -static inline void grant_read_lock(struct grant_table *gt)
>> -{
>> -percpu_read_lock(grant_rwlock, >lock);
>> -}
>> -
>> -static inline void grant_read_unlock(struct grant_table *gt)
>> -{
>> -percpu_read_unlock(grant_rwlock, >lock);
>> -}
>> -
>> -static inline void grant_write_lock(struct grant_table *gt)
>> -{
>> -percpu_write_lock(grant_rwlock, >lock);
>> -}
>> -
>> -static inline void grant_write_unlock(struct grant_table *gt)
>> -{
>> -percpu_write_unlock(grant_rwlock, >lock);
>> -}
>> +extern unsigned int __read_mostly max_grant_frames;
> 
> Why are you adding __read_mostly here? Like all section placement
> annotations it belongs on definitions only, not declarations.

Oops, sorry.

> 
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -307,6 +307,7 @@ struct vm_event_per_domain
>>  };
>>  
>>  struct evtchn_port_ops;
>> +struct grant_table;
> 
> Why is this needed? There's no function with a respective parameter
> in the header here.

You are right, of course. Will remove it again.


Juergen

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

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Jan Beulich]

>>> On 21.08.17 at 20:05,  wrote:
> Today a guest can get the maximum grant table frame number for the
> currently selected grant table interface version (1 or 2) only. Add
> a new grant table operation to get the limits of both variants in
> order to give the guest an opportunity to select the version serving
> its needs best.
> 
> Background for the need for this new hypercall is that e.g. the Linux
> kernel won't use v2 as long as this will allow less active grants as
> v1. As soon as the kernel can detect it can create as many v2 entries
> as for v1, it will have no reason not to use v2. And this will allow
> Xen placing a pv-domain with such a kernel above the current 16TB
> RAM limit.
> 
> For setting up the grant table a kernel needs the following
> information:
> - current version (kexec case)
> - current size (kexec case)
> - max size v1
> - max size v2
> In order not to have to issue 3 hypercalls (GNTTABOP_query_size,
> GNTTABOP_get_version, GNTTABOP_get_v1_and_v2_max), let the new
> hypercall return all the needed information.

I'm not sure I follow: v2 is always going to allow less active grants
than v1, as the limit is always derived from the number of frames
allowed for a domain. I also don't see a problem with issuing multiple
calls - none of this ought to be performance critical. I would,
however, agree that rather than adding a new hypercall to just
return the max sizes adding one like you suggest would be
preferable. I'm simply not convinced yet that returning the max
sizes is actually necessary.

Jan


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

Re: [Xen-devel] [PATCH v4 40/53] xen: check parameter validity when parsing command line

[Wei Liu]

On Wed, Aug 23, 2017 at 07:34:33PM +0200, Juergen Gross wrote:
> Where possible check validity of parameters in _cmdline_parse() and
> issue a warning message in case of an error detected.
> 
> In order to make sure a custom parameter parsing function really
> returns a value (error or success), don't use a void pointer for
> storing the function address, but a proper typed function pointer.
> 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Jan Beulich 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Cc: Wei Liu 
> Signed-off-by: Juergen Gross 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH V2 1/25] DOMCTL: Introduce new DOMCTL commands for vIOMMU support

[Roger Pau Monné]

On Thu, Aug 24, 2017 at 03:03:49PM +0100, Julien Grall wrote:
> Hi,
> 
> On 23/08/17 15:05, Roger Pau Monné wrote:
> > On Wed, Aug 23, 2017 at 11:19:01AM +0100, Julien Grall wrote:
> > > Hi Roger,
> > > 
> > > On 23/08/17 08:22, Roger Pau Monné wrote:
> > > > On Wed, Aug 23, 2017 at 02:06:17PM +0800, Lan Tianyu wrote:
> > > > > Hi Roger:
> > > > >   Thanks for your review.
> > > > > 
> > > > > On 2017年08月22日 22:32, Roger Pau Monné wrote:
> > > > > > On Wed, Aug 09, 2017 at 04:34:02PM -0400, Lan Tianyu wrote:
> > > > > > > +
> > > > > > > +/* vIOMMU capabilities */
> > > > > > > +#define VIOMMU_CAP_IRQ_REMAPPING  (1u << 0)
> > > > > > > +
> > > > > > > +struct xen_domctl_viommu_op {
> > > > > > > +uint32_t cmd;
> > > > > > > +#define XEN_DOMCTL_create_viommu  0
> > > > > > > +#define XEN_DOMCTL_destroy_viommu 1
> > > > > > > +#define XEN_DOMCTL_query_viommu_caps  2
> > > > > > > +union {
> > > > > > > +struct {
> > > > > > > +/* IN - vIOMMU type */
> > > > > > > +uint64_t viommu_type;
> > > > > > > +/*
> > > > > > > + * IN - MMIO base address of vIOMMU. vIOMMU device 
> > > > > > > models
> > > > > > > + * are in charge of to check base_address and length.
> > > > > > > + */
> > > > > > > +uint64_t base_address;
> > > > > > > +/* IN - Length of MMIO region */
> > > > > > > +uint64_t length;
> > > > > > 
> > > > > > It seems weird that you can specify the length, is that something
> > > > > > that a user would like to set? Isn't the length of the IOMMU MMIO
> > > > > > region fixed by the hardware spec?
> > > > > 
> > > > > Different vendor may have different IOMMU register region sizes. (e.g,
> > > > > VTD has one page size for register region). The length field is to 
> > > > > make
> > > > > vIOMMU device model not to abuse address space. Some registers' 
> > > > > offsets
> > > > > are reported by other register and these offsets are emulated by 
> > > > > vIOMMU
> > > > > device model. If it's not necessary, we can remove it and add it when
> > > > > there is real such requirement.
> > > > 
> > > > So from my understanding the size of the IOMMU MMIO region is implicit
> > > > in the IOMMU type that the user chooses. I don't think this field is
> > > > needed.
> > > 
> > > To me, it makes more sense to care both the base and the size rather than
> > > only the former.
> > > 
> > > The toolstack is in charge of the address space and should be aware of the
> > > size of everything. This address space may not be static and it makes 
> > > sense
> > > to give this information to Xen and verify we had the same assumption.
> > 
> > Does this imply that we will have variable size vIOMMU MMIO regions?
> 
> There are existing IOMMU with multiple MMIO regions. This is the case of the
> Nvidia SMMU. Whether we will emulate then is another question. But for
> completeness, I would use address/size.

The proposed implementation does not support multiple MMIO regions
anyway. I'm not going to oppose to this anymore, but I don't see much
value on implementing things just for completeness without having a
real use case, specially when this is a domctl interface that is not
stable, ie: we can always modify it later on without any issues.

Roger.

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

[Xen-devel] [xen-4.7-testing test] 112851: tolerable trouble: blocked/broken/fail/pass - PUSHED

[osstest service owner]

flight 112851 xen-4.7-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/112851/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stopfail REGR. vs. 112793

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64-xsm   2 hosts-allocate  broken like 112793
 build-arm64-xsm   3 capture-logsbroken like 112793
 build-arm64-pvops 2 hosts-allocate  broken like 112793
 build-arm64   2 hosts-allocate  broken like 112793
 build-arm64-pvops 3 capture-logsbroken like 112793
 build-arm64   3 capture-logsbroken like 112793
 test-xtf-amd64-amd64-1  48 xtf/test-hvm64-lbr-tsx-vmentry fail like 112793
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 112793
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 112793
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 112793
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 112793
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 112793
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 112793
 test-amd64-amd64-xl-rtds 10 debian-install   fail  like 112793
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-xl-pvh-amd  12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 15 guest-saverestorefail  never pass
 test-amd64-i386-libvirt  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-amd64-i386-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-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail 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-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-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-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 xen  30d50f8ead03d6524cbc4ed22075980090fbd2ed
baseline version:
 xen  5151257626155d6e331cc9e66d896c84db1611e1

Last test of 

Re: [Xen-devel] [PATCH 4/5] xen: support different gnttab_max_frames for grant v1 and v2

[Jan Beulich]

>>> On 21.08.17 at 20:05,  wrote:
> The number of grants a domain can setup is limited by the maximum
> number of grant frames it is allowed to use. Today the limit is the
> same regardless whether the domain uses grant v1 or v2. Using v2
> will therefor be a disadvantage for the domain as only half the
> number of grants compared to v1 can be used, because a grant v2 entry
> is twice as large as the v1 entry. This is the reason for the lack of
> grant v2 support in the Linux kernel (in fact grant v2 support has
> been removed from Linux for this reason).
> 
> OTOH using only grant v1 will limit a pv domain to the low 16TB of
> memory of the host, as grant v1 entries only use a 32 bit mfn. So
> if we want to support more than 16TB of memory and be able to use
> that memory in pv domains, we have to remove the disadvantage of
> using grant v2 by being able to setup the same number of grants as
> with v1.
> 
> In order to achieve this add support for limiting the number of grant
> frames for v1 and v2 independently from each other. Per default let
> the v2 number be twice the value of the v1 number. Modify the boot
> parameter gnttab_max_frames to accept either a single value which
> will set the v1 limit to that value and the v2 limit to 2*value, or
> two values separated by a comma to set both limits to dedicated
> values.
> 
> Add some sanity checks to make sure the maximum number of frames isn't
> lower than the initial number, as this leads to rather strange crashes.
> 
> Signed-off-by: Juergen Gross 

As discussed elsewhere, this probably rather wants to become a
per-domain setting then. Looking also at what patch 5 adds, I
additionally wonder whether we shouldn't allow Dom0 to know
whether it needs to use v2 at all (or maybe it can derive that
already).

Jan


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

Re: [Xen-devel] [PATCH XEN v2] x86/pt: add a MSI unmask flag to XEN_DOMCTL_bind_pt_irq

[Roger Pau Monne]

On Thu, Aug 24, 2017 at 07:04:55AM -0600, Jan Beulich wrote:
> >>> On 24.08.17 at 14:19,  wrote:
> > @@ -438,6 +439,20 @@ int pt_irq_create_bind(
> >  pi_update_irte(vcpu ? >arch.hvm_vmx.pi_desc : NULL,
> > info, pirq_dpci->gmsi.gvec);
> >  
> > +if ( pt_irq_bind->u.msi.gflags & VMSI_UNMASKED )
> > +{
> > +struct irq_desc *desc = pirq_spin_lock_irq_desc(info, NULL);
> > +
> > +if ( !desc )
> > +{
> > +pt_irq_destroy_bind(d, pt_irq_bind);
> > +return -EINVAL;
> > +}
> > +
> > +guest_mask_msi_irq(desc, false);
> > +spin_unlock_irq(>lock);
> > +}
> 
> In v1 you've used spin_unlock_irqrestore() here - any reason
> you went to the less safe variant? I do understand it is correct,
> but I'd still prefer spin_{,un}lock_irq() to be avoided as much as
> possible.

OK, I saw it was not really needed and switched to the less safe one.
Will switch back, I've also need to resend the QEMU patch to fix a
coding style mistake.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH 3/3] x86/mm: Introduce and use l?e_{get, from}_mfn()

[Wei Liu]

On Thu, Aug 24, 2017 at 02:14:57PM +0100, Andrew Cooper wrote:
> This avoids the explicit boxing/unboxing of mfn_t in relevant codepaths.
> 
> Signed-off-by: Andrew Cooper 


Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 1/3] x86/mm: Replace opencoded forms of l?e_{get, from}_page()

[Wei Liu]

On Thu, Aug 24, 2017 at 02:14:55PM +0100, Andrew Cooper wrote:
> No functional change (confirmed by diffing the disassembly).
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 2/3] x86/mm: Replace opencoded forms of map_l?t_from_l?e()

[Wei Liu]

On Thu, Aug 24, 2017 at 02:14:56PM +0100, Andrew Cooper wrote:
> No functional change (confirmed by diffing the disassembly).
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 3/3] x86/mm: Introduce and use l?e_{get, from}_mfn()

[Tim Deegan]

At 14:14 +0100 on 24 Aug (1503584097), Andrew Cooper wrote:
> This avoids the explicit boxing/unboxing of mfn_t in relevant codepaths.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Tim Deegan 

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