[Xen-devel] XPTI patches for 4.10 don't apply

[Andy Smith]

Hi,

Maybe this is a silly question but which tag are the 4.10 XPTI
commits from https://xenbits.xen.org/xsa/xsa254/README.pti against?
They don't apply to RELEASE-4.10.0.

Cheers,
Andy

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

[Xen-devel] [xen-4.9-testing test] 118167: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-libvirt-raw  7 xen-boot fail REGR. vs. 117931
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
117931

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop   fail blocked in 117931
 test-amd64-i386-xl-qemuu-ws16-amd64 18 guest-start/win.repeat fail blocked in 
117931
 test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail like 117868
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 117868
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 117868
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 117931
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 117931
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 117931
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  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  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 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-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 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-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-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

version targeted for testing:
 xen  dc7d46580d9c633a59be1c3776f79c01dd0cb98b
baseline version:
 xen  0c3d52410094c2504ede126eaa05a80e99c4b4c7

Last test of basis   117931  2018-01-13 05:25:04 Z6 days
Testing same since   118167  2018-01-17 16:50:01 Z1 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Jan H. Schönherr 
  Roger Pau Monné 

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

Re: [Xen-devel] [RFC 02/11] acpi: arm: API to query estimated size of hardware domain's IORT

[Manish Jaggi]

On 01/17/2018 12:22 AM, Julien Grall wrote:

Hi Manish,


Hi Julien,
Thanks for reviewing this patch.

On 02/01/18 09:28, manish.ja...@linaro.org wrote:

From: Manish Jaggi 

  Code to query estimated IORT size for hardware domain.


Please avoid indenting the commit message.

ok.


  IORT for hardware domain is generated using the requesterId and 
deviceId map.


  Signed-off-by: Manish Jaggi 
---
  xen/arch/arm/domain_build.c |  12 -
  xen/drivers/acpi/arm/Makefile   |   1 +
  xen/drivers/acpi/arm/gen-iort.c | 101 


  xen/include/acpi/gen-iort.h |   6 +++
  4 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c74f4dd69d..f5d5e3d271 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -14,6 +14,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -1799,7 +1800,7 @@ static int acpi_create_fadt(struct domain *d, 
struct membank tbl_add[])
    static int estimate_acpi_efi_size(struct domain *d, struct 
kernel_info *kinfo)

  {
-    size_t efi_size, acpi_size, madt_size;
+    size_t efi_size, acpi_size, madt_size, iort_size;


Rather than introduce a variable for 10 instructions, you can rename 
madt_size so it can be re-used. I would be ok for this to be in the 
same patch (providing a proper commit message).

Why would you want to replace iort_size with madt_size ?
What is the harm if adding a variable makes the code more verbose.
I am not able to appreciate your point here.



  u64 addr;
  struct acpi_table_rsdp *rsdp_tbl;
  struct acpi_table_header *table;
@@ -1840,6 +1841,15 @@ static int estimate_acpi_efi_size(struct 
domain *d, struct kernel_info *kinfo)

  acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
    acpi_size += ROUNDUP(sizeof(struct acpi_table_rsdp), 8);
+
+    if( estimate_iort_size(_size) )


Coding style.


+    {
+    printk("Unable to get hwdom iort size\n");
+    return -EINVAL;
+    }
+
+    acpi_size += ROUNDUP(iort_size, 8);
+
  d->arch.efi_acpi_len = PAGE_ALIGN(ROUNDUP(efi_size, 8)
    + ROUNDUP(acpi_size, 8));
  diff --git a/xen/drivers/acpi/arm/Makefile 
b/xen/drivers/acpi/arm/Makefile

index 046fad5e3d..13f1a9159f 100644
--- a/xen/drivers/acpi/arm/Makefile
+++ b/xen/drivers/acpi/arm/Makefile
@@ -1 +1,2 @@
  obj-y = ridmap.o
+obj-y += gen-iort.o
diff --git a/xen/drivers/acpi/arm/gen-iort.c 
b/xen/drivers/acpi/arm/gen-iort.c

new file mode 100644
index 00..3fc32959c6
--- /dev/null
+++ b/xen/drivers/acpi/arm/gen-iort.c
@@ -0,0 +1,101 @@
+/*
+ * xen/drivers/acpi/arm/gen-iort.c
+ *
+ * Code to generate IORT for hardware domain using the requesterId
+ * and deviceId map.
+ *
+ * Manish Jaggi 
+ * Copyright (c) 2018 Linaro.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.


The license is wrong (see patch #1).

Please see my comment in patch #1.
This license is used from an existing file in xen.
So there are a lot of wrong licenses in xen code.



+ *
+ * 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 
+
+/*
+ * Size of hardware domains iort is calulcated based on the number of


s/iort/IORT/
s/calulcated/calculated/

Thanks.



+ * mappings in the requesterId - deviceId mapping list.
+ */
+int estimate_iort_size(size_t *iort_size)
+{
+    int count = 0;
+    int pcirc_count = 0;
+    int itsg_count = 0;
+    uint64_t *pcirc_array;
+    uint64_t *itsg_array;


What is the rationale to store the address directly rather than "void 
*"? This would avoid the cast in the code.



+    struct rid_deviceid_map *rmap;
+


A bit more documention of this function would be appreciated. For 
instance, the rationale between browsing the list twice for allocation.


I actually do think this might be avoidable by storing a bit more 
information from the IORT. From the table you can easily deduced the 
number of root complex and ITS group. They could be store with the 
rest of information.



I will add more documentation that will explain why it is used this way.
For the rest of the function, please be careful on the coding style. I 
am not going to point them all, but I expect you to fix them.



+    list_for_each_entry(rmap, _deviceid_map_list, entry)
+    count++;
+
+    pcirc_array = xzalloc_bytes(sizeof(uint64_t)*count);
+    if ( !pcirc_array )
+    return -ENOMEM;
+
+    itsg_array = 

Re: [Xen-devel] [RFC 01/11] acpi: arm: Public API for populating and query based on requesterid

[Manish Jaggi]

On 01/17/2018 12:01 AM, Julien Grall wrote:

Hi Manish,


Hi Julien,
Thanks for reviewing the patch.


I sent the previous e-mail too soon.

On 02/01/18 09:27, manish.ja...@linaro.org wrote:

From: Manish Jaggi 

  Public API to populate and query map between requester id and
  streamId/DeviceID. IORT is parsed one time (outside this patch)
  and two lists are created one for mapping between reuesterId and 
streamid

  and another between requesterID and deviceID.

  These lists eliminate the need to reparse IORT for querying streamid
  or deviceid using requesterid.

  Signed-off-by: Manish Jaggi 
---
  xen/drivers/acpi/Makefile |   1 +
  xen/drivers/acpi/arm/Makefile |   1 +


We have a directory arch/arm/acpi/. So please move all your code there.

The current files in arch/arm/acpi hold only boot time/low level code.
IMHO creating drivers/acpi/arm makes more sense.
Linux also has iort code in drivers/acpi/arm.



  xen/drivers/acpi/arm/ridmap.c | 124 
++

  xen/include/acpi/ridmap.h |  77 ++


No need to make this header available in common. That should go under 
asm-arm/acpi/

ok



  4 files changed, 203 insertions(+)

diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
index 444b11d583..80a074e007 100644
--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -1,6 +1,7 @@
  subdir-y += tables
  subdir-y += utilities
  subdir-$(CONFIG_X86) += apei
+subdir-$(CONFIG_ARM) += arm
    obj-bin-y += tables.init.o
  obj-$(CONFIG_NUMA) += numa.o
diff --git a/xen/drivers/acpi/arm/Makefile 
b/xen/drivers/acpi/arm/Makefile

new file mode 100644
index 00..046fad5e3d
--- /dev/null
+++ b/xen/drivers/acpi/arm/Makefile
@@ -0,0 +1 @@
+obj-y = ridmap.o
diff --git a/xen/drivers/acpi/arm/ridmap.c 
b/xen/drivers/acpi/arm/ridmap.c

new file mode 100644
index 00..2c3a8876ea
--- /dev/null
+++ b/xen/drivers/acpi/arm/ridmap.c
@@ -0,0 +1,124 @@
+/*
+ * xen/drivers/acpi/arm/ridmap.c
+ *
+ * Public API to populate and query map between requester id and
+ * streamId/DeviceID


I don't care whether you use deviceID or DeviceID but please stay 
consistent with the naming.

ok



+ *
+ * Manish Jaggi 
+ * Copyright (c) 2018 Linaro.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.


Xen is GPLv2 only and hence the copyright wrong. You want to use:

This program is free software; you can redistribute it and/or modify it
under the terms and conditions of the GNU General Public License,
version 2, as published by the Free Software Foundation.


I picked this copyright from xen/arch/arm/traps.c.

 * xen/arch/arm/traps.c
 *
 * ARM Trap handlers
 *
 * Copyright (c) 2011 Citrix Systems.
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 2 of the Licese, or
 * (at your option) any later version.

So IIUYC, traps.c copyright is also wrong.
How do we plan to fix all other files in xen code which use the same 
copyright.

+ *
+ * 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 
+
+struct list_head rid_streamid_map_list;
+struct list_head rid_deviceid_map_list;


Please drop _list. This is pointless to know that when you can 
discover it.


I think it is not pointless.
There is a point here. :)
 _list is added to show that it is a list to make it more verbose.
Without _list the name could mean a single mapping as well.

If you care to see
xen/common/rangeset.c:27:    struct list_head range_list;

I hope you can appreciate the point.


Also, can you explain the rationale of using an unsorted list over 
another structure? 
Since rid - streamId mapping also requires pcirc_node so it would 
require two level of sorting.

First sort based on pcirc_node and next on basis of rid.
Does it makes sense to have all that complex code here ?
 as this API will be used only once per pci device
Along that please give an idea how often and where the query API will 
be used.

ok
BTW, this is called from pci_for_each_dma_alias code flow.



+
+void init_ridmaps(void)


This likely need to be __init.

ok.



+{
+    INIT_LIST_HEAD(_deviceid_map_list);
+    INIT_LIST_HEAD(_streamid_map_list);
+}


This function is not necessary. Declaring 
LIST_HEAD(rid_streamid_map_list) will do the trick.

ok.

+
+int add_rid_streamid_map(struct acpi_iort_node *pcirc_node,


Ditto.

Ditto 

[Xen-devel] [xen-4.7-testing test] 118171: regressions - trouble: broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-debianhvm-amd64broken
 test-amd64-i386-xl-qemut-debianhvm-amd64 4 host-install(4) broken REGR. vs. 
117937
 test-xtf-amd64-amd64-5 49 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 117937

Tests which did not succeed, but are not blocking:
 test-xtf-amd64-amd64-4  49 xtf/test-hvm64-lbr-tsx-vmentry fail like 117873
 test-xtf-amd64-amd64-2  49 xtf/test-hvm64-lbr-tsx-vmentry fail like 117873
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 117937
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 117937
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 117937
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 117937
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 117937
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 117937
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 117937
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 117937
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 117937
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 117937
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 117937
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 117937
 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-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-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-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-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-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-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-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  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-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  e19d0af4ee2ae9e42a85db639fd6848e72f5658b
baseline version:
 xen  23077989038c076597be3d7852f51b80486cd157

Last test of basis   117937  2018-01-13 11:20:36 Z5 days

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 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
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  56498d2cf9d3c5f7d3d894a89f7d66ed81548e01
baseline version:
 xen  380c152033d2a58fe7dc43bf26c3c8b56bfa5d43

Last test of basis   118210  2018-01-18 12:01:06 Z0 days
Testing same since   118219  2018-01-19 01:01:22 Z0 days1 attempts


People who touched revisions under test:
  Stefano Stabellini 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   380c152033..56498d2cf9  56498d2cf9d3c5f7d3d894a89f7d66ed81548e01 -> smoke

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

[Xen-devel] [xen-4.10-testing test] 118165: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 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-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-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-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  7cccd6f748ec724cf9408cec6b3ec8e54a8a2c1f
baseline version:
 xen  cdb1fb4921e2a8636c143e92f74cf0c8d5fce9b1

Last test of basis   117870  2018-01-11 20:44:51 Z7 days
Testing same since   118165  2018-01-17 16:19:04 Z1 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Jan H. Schönherr 
  Roger Pau 

Re: [Xen-devel] [PATCH] fix potential null dereference

[Stefano Stabellini]

On Thu, 18 Jan 2018, Andrew Cooper wrote:
> On 18/01/2018 22:11, Stefano Stabellini wrote:
> > On Tue, 16 Jan 2018, Jan Beulich wrote:
> > On 15.01.18 at 19:51,  wrote:
> >>> On Mon, 15 Jan 2018, Jan Beulich wrote:
> >>> On 13.01.18 at 07:21,  wrote:
> > On 01/12/2018 11:40 PM, Stefano Stabellini wrote:
> >> handles can theoretically be NULL, check for it explicitly before
> >> dereferencing it.
> > I doubt handles could be NULL if LocateHandle succeed. This seems to be 
> > confirmed by the spec (Page 208 in UEFI spec 2.7).
> >
> > So I am not entirely convince we should add yet another check in the 
> > code. An ASSERT might be better.
>  Indeed if there is a platform where NULL is coming back in the
>  success case, that platform should be named as a justification
>  in the commit message. Otherwise I don't see the value of this
>  change.
> >>> Truthfully, it is mostly to silence Coverity. We can all appreciate when
> >>> static analysts cannot find defects in the code.
> >> So what does Coverity dislike here (the more that this is on a
> >> boot path, i.e. not exploitable by guests at all in the first place)?
> >> Merely the NULL pointer? What if the interface gave back a
> >> pointer with a value of 0x123456789abcdef?
> > Coverity complains "Dereferencing null pointer handles". The code path
> > to get to that point is the following, as explained by Coverity:
> >
> >   - handles is set to NULL at the beginning of efi_get_gop
> >   - status is not EFI_BUFFER_TOO_SMALL, AllocatePool is not called, handles 
> > is still NULL
> >   - later we call efi_bs->HandleProtocol(handles[i], "Dereferencing null 
> > pointer handles"
> >
> > Given that in the first call to LocateHandle we pass size == 0, by the
> > spec I don't see how it can return anything other than EFI_BUFFER_TOO_SMALL.
> >
> > In other words, if LocateHandle complies, then the code is correct. I'll
> > mark it as won't fix.
> 
> EFI is full of UB because of needing to pass and cast pointers with
> *(void **), and Coverity has a hard time following these (as a direct
> consequence of the compilers having a hard time following them).
> 
> As for the spec however, that is irrelevant.  The point is that there
> are billions of other integers which could be returned which aren't
> TOO_SMALL and OK, and without the EFI firmware source code to inspect,
> Coverity can't know.
> 
> Also, I'd like to see any EFI anywhere which successfully implements the
> spec...

I guess the question is: do we want to make Xen more resilient against
possible firmware malfunctioning? In this specific case, a bug in the
firmware causes Xen to crash at boot, while with this patch Xen
could try to run past it. There are pros and cons both ways, and I
don't have a strong opinion on the approach to take in general. However,
in this specific case, given the simplicity of this patch, I would be
tempted to make the change.___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] fix potential null dereference

[Andrew Cooper]

On 18/01/2018 22:11, Stefano Stabellini wrote:
> On Tue, 16 Jan 2018, Jan Beulich wrote:
> On 15.01.18 at 19:51,  wrote:
>>> On Mon, 15 Jan 2018, Jan Beulich wrote:
>>> On 13.01.18 at 07:21,  wrote:
> On 01/12/2018 11:40 PM, Stefano Stabellini wrote:
>> handles can theoretically be NULL, check for it explicitly before
>> dereferencing it.
> I doubt handles could be NULL if LocateHandle succeed. This seems to be 
> confirmed by the spec (Page 208 in UEFI spec 2.7).
>
> So I am not entirely convince we should add yet another check in the 
> code. An ASSERT might be better.
 Indeed if there is a platform where NULL is coming back in the
 success case, that platform should be named as a justification
 in the commit message. Otherwise I don't see the value of this
 change.
>>> Truthfully, it is mostly to silence Coverity. We can all appreciate when
>>> static analysts cannot find defects in the code.
>> So what does Coverity dislike here (the more that this is on a
>> boot path, i.e. not exploitable by guests at all in the first place)?
>> Merely the NULL pointer? What if the interface gave back a
>> pointer with a value of 0x123456789abcdef?
> Coverity complains "Dereferencing null pointer handles". The code path
> to get to that point is the following, as explained by Coverity:
>
>   - handles is set to NULL at the beginning of efi_get_gop
>   - status is not EFI_BUFFER_TOO_SMALL, AllocatePool is not called, handles 
> is still NULL
>   - later we call efi_bs->HandleProtocol(handles[i], "Dereferencing null 
> pointer handles"
>
> Given that in the first call to LocateHandle we pass size == 0, by the
> spec I don't see how it can return anything other than EFI_BUFFER_TOO_SMALL.
>
> In other words, if LocateHandle complies, then the code is correct. I'll
> mark it as won't fix.

EFI is full of UB because of needing to pass and cast pointers with
*(void **), and Coverity has a hard time following these (as a direct
consequence of the compilers having a hard time following them).

As for the spec however, that is irrelevant.  The point is that there
are billions of other integers which could be returned which aren't
TOO_SMALL and OK, and without the EFI firmware source code to inspect,
Coverity can't know.

Also, I'd like to see any EFI anywhere which successfully implements the
spec...

~Andrew

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

Re: [Xen-devel] [PATCH] fix potential null dereference

[Stefano Stabellini]

On Tue, 16 Jan 2018, Jan Beulich wrote:
> >>> On 15.01.18 at 19:51,  wrote:
> > On Mon, 15 Jan 2018, Jan Beulich wrote:
> >> >>> On 13.01.18 at 07:21,  wrote:
> >> > On 01/12/2018 11:40 PM, Stefano Stabellini wrote:
> >> >> handles can theoretically be NULL, check for it explicitly before
> >> >> dereferencing it.
> >> > 
> >> > I doubt handles could be NULL if LocateHandle succeed. This seems to be 
> >> > confirmed by the spec (Page 208 in UEFI spec 2.7).
> >> > 
> >> > So I am not entirely convince we should add yet another check in the 
> >> > code. An ASSERT might be better.
> >> 
> >> Indeed if there is a platform where NULL is coming back in the
> >> success case, that platform should be named as a justification
> >> in the commit message. Otherwise I don't see the value of this
> >> change.
> > 
> > Truthfully, it is mostly to silence Coverity. We can all appreciate when
> > static analysts cannot find defects in the code.
> 
> So what does Coverity dislike here (the more that this is on a
> boot path, i.e. not exploitable by guests at all in the first place)?
> Merely the NULL pointer? What if the interface gave back a
> pointer with a value of 0x123456789abcdef?

Coverity complains "Dereferencing null pointer handles". The code path
to get to that point is the following, as explained by Coverity:

  - handles is set to NULL at the beginning of efi_get_gop
  - status is not EFI_BUFFER_TOO_SMALL, AllocatePool is not called, handles is 
still NULL
  - later we call efi_bs->HandleProtocol(handles[i], "Dereferencing null 
pointer handles"

Given that in the first call to LocateHandle we pass size == 0, by the
spec I don't see how it can return anything other than EFI_BUFFER_TOO_SMALL.

In other words, if LocateHandle complies, then the code is correct. I'll
mark it as won't fix.

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

Re: [Xen-devel] [PATCH] remove dead code in allocate_memory

[Stefano Stabellini]

On Mon, 15 Jan 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 01/13/2018 12:29 AM, Stefano Stabellini wrote:
> > lowmem is unchanged until after this loop, there is no point in testing
> > for its value. Moreover, Coverity complains about dead code.
> > 
> > Remove the lowmem test in the first loop.
> 
> While I understand Coverity complains about dead code, in that particular case
> I don't think removing this test is the right thing to do.
> 
> It is very easy to toggle lowmen to false in the declaration as this was done
> before ab5b00a "xen/arm: domain_build: allocate lowmem for dom0 as much as
> possible". One example would be keep low memory free for other purpose.
 
Fair enough


> > 
> > CID: 1381832
> > Signed-off-by: Stefano Stabellini 
> > 
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 155c952..fa58906 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -277,7 +277,7 @@ static void allocate_memory(struct domain *d, struct
> > kernel_info *kinfo)
> >*/
> >   while ( order >= min_low_order )
> >   {
> > -for ( bits = order ; bits <= (lowmem ? 32 : PADDR_BITS); bits++ )
> > +for ( bits = order ; bits <= 32; bits++ )
> >   {
> >   pg = alloc_domheap_pages(d, order, MEMF_bits(bits));
> >   if ( pg != NULL )
> > 
> 

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

[Xen-devel] [PATCH v2] don't pass r12 as reference

[Stefano Stabellini]

r12 and x16 are of different sizes; when passing r12 as a reference to
do_trap_hypercall on arm64, we end up dereferencing it as a pointer to a
64bit value, but actually it isn't.

Instead, use a temporary variable to pass r12, and write back the result
after the call to do_trap_hypercall.

CID: 1457708
Signed-off-by: Stefano Stabellini 

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 013c160..67a68fc 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2123,6 +2123,9 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
 do_trap_smc(regs, hsr);
 break;
 case HSR_EC_HVC32:
+{
+register_t nr;
+
 GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
 perfc_incr(trap_hvc32);
 #ifndef NDEBUG
@@ -2131,8 +2134,11 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
 #endif
 if ( hsr.iss == 0 )
 return do_trap_hvc_smccc(regs);
-do_trap_hypercall(regs, (register_t *)>r12, hsr.iss);
+nr = regs->r12;
+do_trap_hypercall(regs, , hsr.iss);
+regs->r12 = (uint32_t)nr;
 break;
+}
 #ifdef CONFIG_ARM_64
 case HSR_EC_HVC64:
 GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));

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

Re: [Xen-devel] XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Stefano Stabellini]

On Thu, 18 Jan 2018, Julien Grall wrote:
> (+ Security team)
> 
> Hi Stefano,
> 
> On 17/01/18 21:47, Stefano Stabellini wrote:
> > On Wed, 17 Jan 2018, Stefano Stabellini wrote:
> > > On Wed, 17 Jan 2018, Lars Kurth wrote:
> > > >Regarding README.source, this is covering file and contain the
> > > > same mention as in the commit message. As this is a single function.
> > > > Isn't the commit message
> > > >enough?
> > > > 
> > > > 
> > > >  From a legal viewpoint it is enough.
> > > 
> > > If that is enough from a legal viewpoint, then it is enough for me.
> > > 
> > > However, from a legal viewpoint, I thought we needed to explicitly
> > > mention all the original signed-off-bys because Julien is not actually
> > > the copyright holder for that function, hence, we need to add the
> > > signed-off-bys of all the missing copyright holders.
> > 
> > Actually, reading again the Developer’s Certificate of Origin, it
> > states:
> > 
> > "The contribution is based upon previous work that, to the best of my
> > knowledge, is covered under an appropriate open source license and I have
> > the right under that license to submit that work with modifications, whether
> > created in whole or in part by me, under the same open source license
> > (unless I am permitted to submit under a different license), as indicated in
> > the file"
> > 
> > so I think Lars is right. In that case, there is no need to resubmit
> > this series, I'll commit to staging as is. If tests go well, I'll
> > backport it to the stable trees.
> Thank you! I have created branches with patches backported up to Xen 4.8. With
> minor changes:
> 
>- Xen 4.10: No changes
>- Xen 4.9:
>   * minor conflict in some files
>   * compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
>- Xen 4.8:
>   * conflict in some files (one medium as the number of "features" is
> different)
>   * compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
>   
> The branches can be found on xenbits [1] : xsa-254-sp2-X.XX where X.XX is the
> version of Xen.
> 
> Xen 4.7 and earlier does not have cpufeature/cpuerrata infrastructure and will
> require backport. The only difficulty here should be finding the list of
> commits required.
> 
> Also, we probably want to update the XSA pointing to the patches. So if
> someone wants to backport to Xen 4.7 (or earlier) they can. Any opinions?

Thank you, Julien. Ideally, I would like to do the backports after
OSSTest passes its tests on those changes. In practice, for the sake of
mitigating SP2 as soon as possible, tomorrow (Friday) I might do the
backports anyway, if OSSTest is still behind on other problems.

I don't think that backporting cpufeature/cpuerrata to 4.7 should be too
convoluted, I'll give that a go as well.

Once done, I'll provide the list of commits to the xen security list so
that the XSA advisory can be updated appropriately.

Cheers,

Stefano


> Cheers,
> 
> [1] https://xenbits.xen.org/git-http/people/julieng/xen-unstable.git___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [qemu-mainline test] 118159: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 117930
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 117930
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 117930
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 117930
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 117930
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 117930
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-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-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 qemuu8e5dc9ba49743b46d955ec7dacb04e42ae7ada7c
baseline version:
 qemuu7398166ddf7c6dbbc9cae6ac69bb2feda14b40ac

Last test of basis   117930  2018-01-12 23:48:34 Z5 days
Failing since118034  2018-01-15 10:18:58 Z3 days5 attempts
Testing same since   118117  2018-01-16 21:47:31 Z1 days2 attempts


People who touched revisions under test:
  Alex Bennée 
  Alexey Perevalov 
  Alistair Francis 
  Andrey Smirnov 
  Cornelia Huck 
  Daniel P. Berrange 
  Dr. David Alan Gilbert 
  Eric Blake 
  Fam Zheng 
  Frediano Ziglio 
  Gerd Hoffmann 
  Haozhong Zhang 
  Jan Dakinevich 
  Jindrich Makovicka 
  Juan Quintela 
  

Re: [Xen-devel] [RFC 11/11] Add to_pci_dev macro

[Julien Grall]

Hi Manish,

Again please use scripts/get_maintainers.pl.

On 02/01/18 09:28, manish.ja...@linaro.org wrote:

From: Manish Jaggi 

This patch adds to_pci_dev macro


Why? Who is going to use it? If it is a patch in your series, then 
likely this patch should be before you use it.


Any in case, a commit message a bit more develop will be help.



Signed-off-by: Manish Jaggi 
---
  xen/include/xen/pci.h | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 43f21251a5..4c7ff4dd10 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -92,8 +92,11 @@ struct pci_dev {
  #define PT_FAULT_THRESHOLD 10
  } fault;
  u64 vf_rlen[6];
+struct device dev;


That's common code. Please look at adding this in arch_pci_dev. But I 
would need a bit more context why you need that.



  };
  
+#define to_pci_dev(p) container_of(p, struct pci_dev, dev)

+#define pci_domain_nr(dev) dev->seg
  #define for_each_pdev(domain, pdev) \
  list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
  



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC 09/11] Xen IORT Changes

[Julien Grall]

Hi Manish,

On 02/01/18 09:28, manish.ja...@linaro.org wrote:

From: Manish Jaggi 

This patch adds xen specific changes to iort.c


When I see the diff below, it is not changed but fully rewrite.

The original file contains 1279 lines, but you remove 978 lines and add 
71. In that context, I see no value to re-use Linux code.




Signed-off-by: Manish Jaggi 
---
  xen/arch/arm/setup.c  |2 +
  xen/drivers/acpi/arm/Makefile |1 +
  xen/drivers/acpi/arm/iort.c   | 1040 +++--
  xen/include/acpi/acpi_iort.h  |6 +-
  4 files changed, 71 insertions(+), 978 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 16a3b1be8e..7ada48920f 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -37,6 +37,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -800,6 +801,7 @@ void __init start_xen(unsigned long boot_phys_offset,
  
  tasklet_subsys_init();
  
+acpi_iort_init();


Please no. I can't see any reason to call acpi_iort_init() in common 
code. Instead this should be an initcall().


If you think differently, please explain why.

  
  xsm_dt_init();
  
diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile

index 13f1a9159f..5d16161016 100644
--- a/xen/drivers/acpi/arm/Makefile
+++ b/xen/drivers/acpi/arm/Makefile
@@ -1,2 +1,3 @@
  obj-y = ridmap.o
  obj-y += gen-iort.o
+obj-y += iort.o
diff --git a/xen/drivers/acpi/arm/iort.c b/xen/drivers/acpi/arm/iort.c
index de56394dd1..a47ee2df4c 100644
--- a/xen/drivers/acpi/arm/iort.c
+++ b/xen/drivers/acpi/arm/iort.c
@@ -14,17 +14,20 @@
   * This file implements early detection/parsing of I/O mapping
   * reported to OS through firmware via I/O Remapping Table (IORT)
   * IORT document number: ARM DEN 0049A
+ *
+ * Imported from Linux 4.14.0
+ * Xen Modifications : Manish Jaggi 
   */
  
  #define pr_fmt(fmt)	"ACPI: IORT: " fmt
  
-#include 

-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
  
  #define IORT_TYPE_MASK(type)	(1 << (type))

  #define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
@@ -36,6 +39,22 @@
  #define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX   0x2
  #endif
  
+/* Redefine WARN macros */

+#undef WARN
+#undef WARN_ON
+#define WARN(condition, format...) ({  \
+   int __ret_warn_on = !!(condition);  \
+   if (unlikely(__ret_warn_on))\
+   printk(format); \
+   unlikely(__ret_warn_on);\
+})
+#define WARN_TAINT(cond, taint, format...) WARN(cond, format)
+#define WARN_ON(cond)  (!!cond)


Again, I would have appreciated if you look at the comment I made on 
Sameer series.



+
+
+#define MAX_ERRNO  4095
+#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned 
long)-MAX_ERRNO)


IS_ERR_VALUE already exists in Xen (see include/xen/err.h).

For the rest of the code, I will wait your answer on my first comment in 
that e-mail.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] AMD / SVM vm_event support

[Tamas K Lengyel]

On Thu, Jan 18, 2018 at 6:19 AM, Razvan Cojocaru
 wrote:
> Hello,
>
> We're looking at potentially working with vm_event on SVM hosts. To that
> end, we've removed a few cpu_has_vmx tests and saw that some things just
> work.
>
> That is, unfortunately, not the case for EPT events.
>
> There's this old thread about NPT support:
>
> https://lists.gt.net/xen/devel/339571
>
> but it looks like nothing came of it.
>
> Has there been more Xen development work, or at least discussion on this
> topic that we are not aware of?

I'm not aware of any work in this area (there were some patches to
implement PV memory events but those never got merged either).

>
> As for the things that do work, any objections to submitting patches
> that allows an introspection application to subscribe to what's
> available on SVM as well, with the proper capabilities set in
> arch_monitor_get_capabilities()?

From my side this would be a welcome addition!

Tamas

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

Re: [Xen-devel] [RFC 08/11] Add ACPI_IORT config

[Julien Grall]

Hi  Manish,

The usual scripts/get_maintainers.pl.

On 02/01/18 09:28, manish.ja...@linaro.org wrote:

From: Manish Jaggi 

Add ACPI_IORT config

Singed-off-by: Manish Jaggi 
---
  xen/arch/arm/Kconfig | 5 +
  xen/drivers/acpi/Kconfig | 3 +++
  2 files changed, 8 insertions(+)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index f58019d6ed..d4767d6ea3 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -42,6 +42,11 @@ config ACPI
  Advanced Configuration and Power Interface (ACPI) support for Xen is
  an alternative to device tree on ARM64.
  
+config ACPI_IORT

+   bool
+   prompt "ACPI IORT Support" if EXPERT = "y"


No need for EXPERT here. It will get picked up by ACPI.

Also, I don't think it is useful to let the user disabling IORT. You 
either need all ACPI or not. It will get into trouble if IORT is not 
handled in Xen.


Note that I am happy to see the config ACPI_IORT here. But not exposed 
to user's choice.



+   depends on ACPI
+
  config HAS_GICV3
bool
  
diff --git a/xen/drivers/acpi/Kconfig b/xen/drivers/acpi/Kconfig

index b64d3731fb..15ae98140c 100644
--- a/xen/drivers/acpi/Kconfig
+++ b/xen/drivers/acpi/Kconfig
@@ -5,5 +5,8 @@ config ACPI
  config ACPI_LEGACY_TABLES_LOOKUP
bool
  
+config ACPI_IORT

+   bool


I am not sure to understand why you define ACPI_IORT again here. It is 
already done above?


However, I don't think it is necessary to have a separate patch just for 
adding the Kconfig. You can fold into the patch that is first using it. 
BTW, I would have expected this to be patch #1 and used to gate 
compilation for any of those file.



+
  config NUMA
bool



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC 07/11] Add kernel helper functions

[Julien Grall]

Hi Manish,

Please use scripts/get_maitainers.pl to CC relevant maintainers.

On 02/01/18 09:28, manish.ja...@linaro.org wrote:

From: Manish Jaggi 

Add kalloc kfree functions from linux kernel.


This does not description all the functions you added. But I am really 
not convinced this is the right place to do that.


This file is included in many places that should not use kmalloc & co 
but instead Xen version.


If you still want to add it in an header, I think it would make sense to 
create a header that will contain linuxism. So it get included only 
where it is needed.




Signed-off-by: Manish Jaggi 
---
  xen/include/xen/kernel.h | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 548b64da9f..78517f6caa 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -7,6 +7,16 @@
  
  #include 
  
+/* Xen: Define compatibility functions */

+#define FW_BUG "[Firmware Bug]: "
+#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
+#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
+
+/* Alias to Xen allocation helpers */
+#define kfree xfree
+#define kmalloc(size, flags)_xmalloc(size, sizeof(void *))
+#define kzalloc(size, flags)_xzalloc(size, sizeof(void *))
+
  /*
   * min()/max() macros that also do
   * strict type-checking.. See the



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC 03/11] acpi: arm: Code to generate Hardware Domains IORT

[Julien Grall]

Hi,

On 02/01/18 09:28, manish.ja...@linaro.org wrote:

From: Manish Jaggi 



First of all, I would expect a commit message explaining roughly the 
logic. I am only going to skim through this patch and will wait a proper 
description to fully read it.


A general comment, please respect the coding style. Most of error can be 
avoided by using the coding style from the first line you write. And you 
are also saving bandwidth review.



Singed-off-by: Manish Jaggi 
---
  xen/arch/arm/domain_build.c |  28 +
  xen/drivers/acpi/arm/gen-iort.c | 253 +++-
  xen/include/acpi/gen-iort.h |   1 +
  xen/include/asm-arm/acpi.h  |   1 +
  4 files changed, 282 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f5d5e3d271..9831943147 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1654,6 +1654,8 @@ static int acpi_create_xsdt(struct domain *d, struct 
membank tbl_add[])
 ACPI_SIG_FADT, tbl_add[TBL_FADT].start);
  acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
 ACPI_SIG_MADT, tbl_add[TBL_MADT].start);
+acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
+   ACPI_SIG_IORT, tbl_add[TBL_IORT].start);
  xsdt->table_offset_entry[entry_count] = tbl_add[TBL_STAO].start;
  
  xsdt->header.length = table_size;

@@ -1704,6 +1706,28 @@ static int acpi_create_stao(struct domain *d, struct 
membank tbl_add[])
  return 0;
  }
  
+static int acpi_create_iort(struct domain *d, struct membank tbl_add[])

+{
+struct acpi_table_iort *hwdom_table;
+unsigned int size = 0;
+
+tbl_add[TBL_IORT].start = d->arch.efi_acpi_gpa
+  + acpi_get_table_offset(tbl_add, TBL_IORT);
+hwdom_table = d->arch.efi_acpi_table
+  + acpi_get_table_offset(tbl_add, TBL_IORT);


This code (and probably other bits of this series) seems to assume that 
IORT is always present. This may not be true, think of a platform with 
no MSI controller or Xen built with no ITS support.


So you have to figure out whether you need to generate the IORT table 
for the hardware domain.



+
+if ( prepare_iort(hwdom_table, ) )
+{
+printk("Failed to write IORT table\n");
+return -EINVAL;
+}
+printk("%s %d %d \r\n", __func__, __LINE__, size);


This sounds like a debug message.


+
+tbl_add[TBL_IORT].size = size;
+printk("%s %d %d \r\n", __func__, __LINE__, size);


Ditto.

Newline here.


+return 0;
+}
+
  static int acpi_create_madt(struct domain *d, struct membank tbl_add[])
  {
  struct acpi_table_header *table = NULL;
@@ -1899,6 +1923,10 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  if ( rc != 0 )
  return rc;
  
+rc = acpi_create_iort(d, tbl_add);

+if ( rc != 0 )
+return rc;
+
  rc = acpi_create_xsdt(d, tbl_add);
  if ( rc != 0 )
  return rc;
diff --git a/xen/drivers/acpi/arm/gen-iort.c b/xen/drivers/acpi/arm/gen-iort.c
index 3fc32959c6..f368000753 100644
--- a/xen/drivers/acpi/arm/gen-iort.c
+++ b/xen/drivers/acpi/arm/gen-iort.c
@@ -21,6 +21,257 @@
  #include 
  #include 
  
+/*

+ * Structure of Hardware domain's IORT
+ * ---
+ *
+ * Below is the structure of the IORT which this code generates.
+ *
+ * [IORT Header]
+ * [ITS Group 1 ]
+ * [ITS Group N ]
+ * [PCIRC Node 1]
+ * [PCIRC IDMAP entry 1]
+ * [PCIRC IDMAP entry N]
+ * [PCIRC Node N]
+ *
+ * requesterId- deviceId mapping list poplated by parsing IORT is used


s/d-/d -/
s/poplated/populated/


+ * to create nodes and idmaps.
+ * We have one small problem, how to resolve the its grooup node offset from


s/grooup/group/


+ * the firmware iort to the ones in hardware domains IORT.


s/iort/IORT/


+ *
+ * Since the ITS group node pointer stored with the rid-devid map is used
+ * to populate the ITS Group nodes in the hardware domains' IORT.
+ * We create another map to save the offset of the ITS group node written
+ * in the hardware domain IORT with the ITS node pointer in the firmware IORT.
+ *
+ * This offset is later used when writing pcirc idmaps output_reference.
+ */
+struct its_node_offset_map
+{
+struct acpi_iort_node *its_node;
+unsigned int offset;
+struct list_head entry;
+};


newline.


+struct list_head its_map_list;
If you use LIST_HEAD(its_map_list) you don't need to do 
LIST_HEAD_INIT(...) below.


Also, static in front. Unless this should be exported and therefore have 
a corresponding line in the header.



+
+int set_its_node_offset(struct acpi_iort_node *its_node, unsigned int offset)


Ditto. I am not going to point all of them. So I expect you to fix them 
by next version.



+{
+struct its_node_offset_map *its_map;


Newline.


+

Re: [Xen-devel] [PATCH 1/9] Update shim.config

[Andrew Cooper]

On 18/01/18 18:28, Wei Liu wrote:
> On Thu, Jan 18, 2018 at 06:26:02PM +, Andrew Cooper wrote:
>> On 18/01/18 18:16, Wei Liu wrote:
>>> To avoid having it changed every time the shim is built.
>>>
>>> Signed-off-by: Wei Liu 
>> One downside of having a this config in-tree is that it needs
>> regenerating every time the Kconfig logic changes.
>>
>> OTOH, I did deliberately wire up all the config options, so
>>
>> make -C tools/firmware/xen-dir shim-olddefconfig
>>
>> or shim-menuconfig etc do work as expected.
>>
>> Do we know why this line is disappearing?  I presume it is some change
>> in the Kconfig logic, but I can't spot which changeset.
> I guess that's due to "xen/x86: make VGA support selectable".
>
> The shim.config probably is generated before that patch.

Can you identify that in the commit message?  With that, Reviewed-by:
Andrew Cooper 

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

Re: [Xen-devel] [PATCH 1/9] Update shim.config

[Wei Liu]

On Thu, Jan 18, 2018 at 06:26:02PM +, Andrew Cooper wrote:
> On 18/01/18 18:16, Wei Liu wrote:
> > To avoid having it changed every time the shim is built.
> >
> > Signed-off-by: Wei Liu 
> 
> One downside of having a this config in-tree is that it needs
> regenerating every time the Kconfig logic changes.
> 
> OTOH, I did deliberately wire up all the config options, so
> 
> make -C tools/firmware/xen-dir shim-olddefconfig
> 
> or shim-menuconfig etc do work as expected.
> 
> Do we know why this line is disappearing?  I presume it is some change
> in the Kconfig logic, but I can't spot which changeset.

I guess that's due to "xen/x86: make VGA support selectable".

The shim.config probably is generated before that patch.

Wei.

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

Re: [Xen-devel] [PATCH 4/9] Remove sched=null from shim cmdline and doc

[Andrew Cooper]

On 18/01/18 18:16, Wei Liu wrote:
> We use the default scheduler (credit1 as of writing). The NULL
> scheduler still has bugs to fix.
>
> Signed-off-by: Wei Liu 

Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH 3/9] x86/guest: clean up guest/xen.h

[Andrew Cooper]

On 18/01/18 18:16, Wei Liu wrote:
> Remove extraneous semicolon. Add blank lines.
>
> Signed-off-by: Wei Liu 

Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH 1/9] Update shim.config

[Andrew Cooper]

On 18/01/18 18:16, Wei Liu wrote:
> To avoid having it changed every time the shim is built.
>
> Signed-off-by: Wei Liu 

One downside of having a this config in-tree is that it needs
regenerating every time the Kconfig logic changes.

OTOH, I did deliberately wire up all the config options, so

make -C tools/firmware/xen-dir shim-olddefconfig

or shim-menuconfig etc do work as expected.

Do we know why this line is disappearing?  I presume it is some change
in the Kconfig logic, but I can't spot which changeset.

~Andrew

> ---
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> ---
>  tools/firmware/xen-dir/shim.config | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/tools/firmware/xen-dir/shim.config 
> b/tools/firmware/xen-dir/shim.config
> index 78b965f4c7..d5bd516632 100644
> --- a/tools/firmware/xen-dir/shim.config
> +++ b/tools/firmware/xen-dir/shim.config
> @@ -68,7 +68,6 @@ CONFIG_HAS_EHCI=y
>  CONFIG_HAS_CPUFREQ=y
>  CONFIG_HAS_PASSTHROUGH=y
>  CONFIG_HAS_PCI=y
> -# CONFIG_VGA is not set
>  CONFIG_DEFCONFIG_LIST="$ARCH_DEFCONFIG"
>  CONFIG_ARCH_SUPPORTS_INT128=y
>  


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

[Xen-devel] [PATCH 5/9] x86: relocate pvh_info

[Wei Liu]

Modify early boot code to relocate pvh info as well, so that we can be
sure __va in __start_xen works.

Signed-off-by: Wei Liu 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/arch/x86/boot/Makefile   |  7 +-
 xen/arch/x86/boot/build32.mk |  3 +++
 xen/arch/x86/boot/defs.h |  3 +++
 xen/arch/x86/boot/head.S | 25 -
 xen/arch/x86/boot/reloc.c| 53 +++-
 5 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index c6246c85d2..9fe5b309c5 100644
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -7,10 +7,15 @@ CMDLINE_DEPS = $(DEFS_H_DEPS) video.h
 RELOC_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/include/xen/multiboot.h \
 $(BASEDIR)/include/xen/multiboot2.h
 
+ifeq ($(CONFIG_PVH_GUEST),y)
+RELOC_DEPS += $(BASEDIR)/include/public/arch-x86/hvm/start_info.h
+RELOC_EXTRA = CONFIG_PVH_GUEST=y
+endif
+
 head.o: cmdline.S reloc.S
 
 cmdline.S: cmdline.c $(CMDLINE_DEPS)
$(MAKE) -f build32.mk $@ CMDLINE_DEPS="$(CMDLINE_DEPS)"
 
 reloc.S: reloc.c $(RELOC_DEPS)
-   $(MAKE) -f build32.mk $@ RELOC_DEPS="$(RELOC_DEPS)"
+   $(MAKE) -f build32.mk $@ RELOC_DEPS="$(RELOC_DEPS)" $(RELOC_EXTRA)
diff --git a/xen/arch/x86/boot/build32.mk b/xen/arch/x86/boot/build32.mk
index 48c7407c00..028ac19b96 100644
--- a/xen/arch/x86/boot/build32.mk
+++ b/xen/arch/x86/boot/build32.mk
@@ -36,5 +36,8 @@ CFLAGS := $(filter-out -flto,$(CFLAGS))
 cmdline.o: cmdline.c $(CMDLINE_DEPS)
 
 reloc.o: reloc.c $(RELOC_DEPS)
+ifeq ($(CONFIG_PVH_GUEST),y)
+reloc.o: CFLAGS += -DCONFIG_PVH_GUEST
+endif
 
 .PRECIOUS: %.bin %.lnk
diff --git a/xen/arch/x86/boot/defs.h b/xen/arch/x86/boot/defs.h
index 6abdc15446..05921a64a3 100644
--- a/xen/arch/x86/boot/defs.h
+++ b/xen/arch/x86/boot/defs.h
@@ -51,6 +51,9 @@ typedef unsigned short u16;
 typedef unsigned int u32;
 typedef unsigned long long u64;
 typedef unsigned int size_t;
+typedef u8 uint8_t;
+typedef u32 uint32_t;
+typedef u64 uint64_t;
 
 #define U16_MAX((u16)(~0U))
 #define UINT_MAX   (~0U)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 0f652cea11..2f94c286d5 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -414,6 +414,7 @@ __pvh_start:
 
 /* Set trampoline_phys to use mfn 1 to avoid having a mapping at VA 0 
*/
 movw$0x1000, sym_esi(trampoline_phys)
+xor %eax, %eax /* Needed by reloc.c */
 jmp trampoline_setup
 
 #endif /* CONFIG_PVH_GUEST */
@@ -578,18 +579,20 @@ trampoline_setup:
 /* Get bottom-most low-memory stack address. */
 add $TRAMPOLINE_SPACE,%ecx
 
-#ifdef CONFIG_PVH_GUEST
-cmpb$0, sym_fs(pvh_boot)
-jne 1f
-#endif
-
-/* Save the Multiboot info struct (after relocation) for later use. */
+/* Save Multiboot / PVH info struct (after relocation) for later use. 
*/
 push%ecx/* Bottom-most low-memory stack address. */
-push%ebx/* Multiboot information address. */
-push%eax/* Multiboot magic. */
+push%ebx/* Multiboot / PVH information address. */
+push%eax/* Magic number. */
 callreloc
-mov %eax,sym_fs(multiboot_ptr)
+#ifdef CONFIG_PVH_GUEST
+cmp $0,sym_fs(pvh_boot)
+je  1f
+mov %eax,sym_fs(pvh_start_info_pa)
+jmp 2f
+#endif
 1:
+mov %eax,sym_fs(multiboot_ptr)
+2:
 
 /*
  * Now trampoline_phys points to the following structure (lowest 
address
@@ -598,12 +601,12 @@ trampoline_setup:
  * ++
  * | TRAMPOLINE_STACK_SPACE |
  * ++
- * |mbi data|
+ * | Data (MBI / PVH)   |
  * +- - - - - - - - - - - - +
  * |TRAMPOLINE_SPACE|
  * ++
  *
- * mbi data grows downwards from the highest address of 
TRAMPOLINE_SPACE
+ * Data grows downwards from the highest address of TRAMPOLINE_SPACE
  * region to the end of the trampoline. The rest of TRAMPOLINE_SPACE is
  * reserved for trampoline code and data.
  */
diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
index b992678b5e..b01e148772 100644
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -14,8 +14,8 @@
 
 /*
  * This entry point is entered from xen/arch/x86/boot/head.S with:
- *   - 0x4(%esp) = MULTIBOOT_MAGIC,
- *   - 0x8(%esp) = MULTIBOOT_INFORMATION_ADDRESS,
+ *   - 0x4(%esp) = MAGIC,
+ *   - 0x8(%esp) = INFORMATION_ADDRESS,
  *   - 0xc(%esp) = TOPMOST_LOW_MEMORY_STACK_ADDRESS.
  */
 asm (
@@ -29,6 +29,10 @@ asm (
 #include "../../../include/xen/multiboot.h"
 #include 

[Xen-devel] [PATCH 6/9] Revert "x86/boot: Map more than the first 16MB"

[Wei Liu]

This reverts commit 7d6f958d9d18c54017f5ef6e299a08037f035747.

Now we have PVH info relocation support, this change is no longer
needed.

Signed-off-by: Wei Liu 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/arch/x86/boot/x86_64.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 42636cf334..cf47e019f5 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -114,10 +114,11 @@ GLOBAL(__page_tables_start)
 GLOBAL(l2_identmap)
 .quad sym_offs(l1_identmap) + __PAGE_HYPERVISOR
 idx = 1
-.rept 4 * L2_PAGETABLE_ENTRIES - 1
+.rept 7
 .quad (idx << L2_PAGETABLE_SHIFT) | PAGE_HYPERVISOR | _PAGE_PSE
 idx = idx + 1
 .endr
+.fill 4 * L2_PAGETABLE_ENTRIES - 8, 8, 0
 .size l2_identmap, . - l2_identmap
 
 /*
-- 
2.11.0


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

[Xen-devel] [PATCH 9/9] x86/shim: pass through vcpu runstate to L0 Xen

[Wei Liu]

So that we can have accurate stolen time accounting. Idea taken from
Vixen work from Amazon.

Signed-off-by: Wei Liu 
---
Cc: Roger Pau Monné 
Cc: Anthony Liguori 
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/common/domain.c   | 10 ++
 xen/include/asm-x86/guest/hypercall.h |  7 +++
 2 files changed, 17 insertions(+)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 558318e852..189ffac9b1 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -45,6 +45,7 @@
 
 #ifdef CONFIG_X86
 #include 
+#include 
 #endif
 
 /* Linux config option: propageted to domain0 */
@@ -1425,6 +1426,15 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, 
XEN_GUEST_HANDLE_PARAM(void) arg)
 if ( !guest_handle_okay(area.addr.h, 1) )
 break;
 
+#if CONFIG_X86
+if ( pv_shim )
+{
+rc = xen_hypercall_vcpu_op(VCPUOP_register_runstate_memory_area,
+   vcpuid, );
+break;
+}
+#endif
+
 rc = 0;
 runstate_guest(v) = area.addr.h;
 
diff --git a/xen/include/asm-x86/guest/hypercall.h 
b/xen/include/asm-x86/guest/hypercall.h
index e9e626b474..07b9302263 100644
--- a/xen/include/asm-x86/guest/hypercall.h
+++ b/xen/include/asm-x86/guest/hypercall.h
@@ -192,6 +192,13 @@ static inline long xen_hypercall_shutdown(unsigned int 
reason)
 return 0;
 }
 
+static inline int xen_hypercall_vcpu_op(unsigned int cmd, unsigned int vcpu,
+void *arg)
+{
+ASSERT_UNREACHABLE();
+return 0;
+}
+
 #endif /* CONFIG_XEN_GUEST */
 #endif /* __X86_XEN_HYPERCALL_H__ */
 
-- 
2.11.0


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

[Xen-devel] [PATCH 2/9] libxl: remove whitespaces introduced in 62982da926

[Wei Liu]

Signed-off-by: Wei Liu 
---
Cc: Ian Jackson 
---
 tools/libxl/libxl_dom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index b03386409f..e1a3e747fc 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1069,7 +1069,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
 }
 }
 }
-
+
 if (state->pv_ramdisk.path && strlen(state->pv_ramdisk.path)) {
 if (state->pv_ramdisk.mapped) {
 rc = xc_dom_module_mem(dom, state->pv_ramdisk.data,
@@ -1183,7 +1183,7 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
 
 xc_dom_loginit(ctx->xch);
 
-/* 
+/*
  * If PVH and we have a shim override, use the shim cmdline.
  * If PVH and no shim override, use the pv cmdline.
  * If not PVH, use info->cmdline.
-- 
2.11.0


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

[Xen-devel] [PATCH 8/9] xen/consoled: discard NUL from guest

[Wei Liu]

According to [0], some program sends NUL for padding purpose. We can
discard them.

https://www.gnu.org/software/termutils/manual/termcap-1.3/html_mono/termcap.html#SEC7

Reported-by: Sarah Newman 
Signed-off-by: Wei Liu 
---
Cc: Sarah Newman 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Ian Jackson 

A bit RFC. Awaiting test results.
---
 xen/drivers/char/consoled.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/char/consoled.c b/xen/drivers/char/consoled.c
index 552abf5766..6fcb2aa115 100644
--- a/xen/drivers/char/consoled.c
+++ b/xen/drivers/char/consoled.c
@@ -72,7 +72,9 @@ size_t consoled_guest_rx(void)
 {
 char c = cons_ring->out[MASK_XENCONS_IDX(cons++, cons_ring->out)];
 
-buf[idx++] = c;
+/* Discard NUL. See the "Padding" section of termcap manual. */
+if ( c != '\0' )
+buf[idx++] = c;
 recv++;
 
 if ( idx >= BUF_SZ )
-- 
2.11.0


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

[Xen-devel] [PATCH 7/9] libxl: lower shim related message to level info

[Wei Liu]

Signed-off-by: Wei Liu 
---
Cc: Ian Jackson 
---
 tools/libxl/libxl_dom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index e1a3e747fc..b880341d7e 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1035,7 +1035,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
 
 /* We've loaded the shim, so load the kernel as a secondary module 
*/
 if (state->pv_kernel.mapped) {
-LOG(WARN, "xc_dom_module_mem, cmdline %s",
+LOG(INFO, "xc_dom_module_mem, cmdline %s",
 state->pv_cmdline);
 rc = xc_dom_module_mem(dom, state->pv_kernel.data,
state->pv_kernel.size, 
state->pv_cmdline);
@@ -1044,7 +1044,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
 goto out;
 }
 } else {
-LOG(WARN, "xc_dom_module_file, path %s cmdline %s",
+LOG(INFO, "xc_dom_module_file, path %s cmdline %s",
 state->pv_kernel.path, state->pv_cmdline);
 rc = xc_dom_module_file(dom, state->pv_kernel.path, 
state->pv_cmdline);
 if (rc) {
-- 
2.11.0


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

[Xen-devel] [PATCH 0/9] Fix and improvements to pvshim

[Wei Liu]

First batch of patches to fix problems I know of.

Wei Liu (9):
  Update shim.config
  libxl: remove whitespaces introduced in 62982da926
  x86/guest: clean up guest/xen.h
  Remove sched=null from shim cmdline and doc
  x86: relocate pvh_info
  Revert "x86/boot: Map more than the first 16MB"
  libxl: lower shim related message to level info
  xen/consoled: discard NUL from guest
  x86/shim: pass through vcpu runstate to L0 Xen

 docs/man/xl.cfg.pod.5.in  |  2 +-
 tools/firmware/xen-dir/shim.config|  1 -
 tools/libxl/libxl_dom.c   |  8 +++---
 tools/libxl/libxl_internal.h  |  2 +-
 xen/arch/x86/boot/Makefile|  7 -
 xen/arch/x86/boot/build32.mk  |  3 ++
 xen/arch/x86/boot/defs.h  |  3 ++
 xen/arch/x86/boot/head.S  | 25 +
 xen/arch/x86/boot/reloc.c | 53 +++
 xen/arch/x86/boot/x86_64.S|  3 +-
 xen/common/domain.c   | 10 +++
 xen/drivers/char/consoled.c   |  4 ++-
 xen/include/asm-x86/guest/hypercall.h |  7 +
 xen/include/asm-x86/guest/xen.h   | 12 +---
 14 files changed, 109 insertions(+), 31 deletions(-)

-- 
2.11.0


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

[Xen-devel] [PATCH 1/9] Update shim.config

[Wei Liu]

To avoid having it changed every time the shim is built.

Signed-off-by: Wei Liu 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 tools/firmware/xen-dir/shim.config | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/firmware/xen-dir/shim.config 
b/tools/firmware/xen-dir/shim.config
index 78b965f4c7..d5bd516632 100644
--- a/tools/firmware/xen-dir/shim.config
+++ b/tools/firmware/xen-dir/shim.config
@@ -68,7 +68,6 @@ CONFIG_HAS_EHCI=y
 CONFIG_HAS_CPUFREQ=y
 CONFIG_HAS_PASSTHROUGH=y
 CONFIG_HAS_PCI=y
-# CONFIG_VGA is not set
 CONFIG_DEFCONFIG_LIST="$ARCH_DEFCONFIG"
 CONFIG_ARCH_SUPPORTS_INT128=y
 
-- 
2.11.0


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

[Xen-devel] [PATCH 3/9] x86/guest: clean up guest/xen.h

[Wei Liu]

Remove extraneous semicolon. Add blank lines.

Signed-off-by: Wei Liu 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/include/asm-x86/guest/xen.h | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/xen/include/asm-x86/guest/xen.h b/xen/include/asm-x86/guest/xen.h
index 11243fe60d..31326442f7 100644
--- a/xen/include/asm-x86/guest/xen.h
+++ b/xen/include/asm-x86/guest/xen.h
@@ -49,7 +49,8 @@ DECLARE_PER_CPU(struct vcpu_info *, vcpu_info);
 #define xen_guest 0
 #define pv_console 0
 
-static inline void probe_hypervisor(void) {};
+static inline void probe_hypervisor(void) {}
+
 static inline void hypervisor_setup(void)
 {
 ASSERT_UNREACHABLE();
@@ -63,20 +64,23 @@ static inline void hypervisor_fixup_e820(struct e820map 
*e820)
 {
 ASSERT_UNREACHABLE();
 }
+
 static inline const unsigned long *hypervisor_reserved_pages(unsigned int 
*size)
 {
 ASSERT_UNREACHABLE();
 return NULL;
-};
+}
+
 static inline uint32_t hypervisor_cpuid_base(void)
 {
 ASSERT_UNREACHABLE();
 return 0;
-};
+}
+
 static inline void hypervisor_resume(void)
 {
 ASSERT_UNREACHABLE();
-};
+}
 
 #endif /* CONFIG_XEN_GUEST */
 #endif /* __X86_GUEST_XEN_H__ */
-- 
2.11.0


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

Re: [Xen-devel] [PATCH v9 05/11] x86/hvm: Permit guests direct access to MSR_{SPEC_CTRL, PRED_CMD}

[Boris Ostrovsky]

On 01/18/2018 10:46 AM, Andrew Cooper wrote:
> For performance reasons, HVM guests should have direct access to these MSRs
> when possible.
>
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: Jun Nakajima 
> CC: Kevin Tian 
> CC: Boris Ostrovsky 
> CC: Suravee Suthikulpanit 
>
> v7:
>  * Drop excess brackets
> v9:
>  * Re-implement it light of Intels new spec.  Drop R-by's.
>  * Spelling fixes

Reviewed-by: Boris Ostrovsky 

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

[Xen-devel] [linux-linus test] 118154: trouble: broken/fail/pass

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-libvirt-raw broken
 test-armhf-armhf-libvirt-raw  4 host-install(4)broken REGR. vs. 118112

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 118112
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 118112
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 118112
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 118112
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 118112
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 118112
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 118112
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 118112
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 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-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail 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-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux8cbab92dff778e516064c13113ca15d4869ec883
baseline version:
 linux41aa5e5d712ba3a5f4fac0bbd6d976d70f9aed06

Last test of basis   118112  2018-01-16 20:52:02 Z1 days
Testing same since   118154  2018-01-17 09:59:00 Z1 days1 attempts


People who touched revisions under test:
  Alaa Hleihel 
  Alexei Starovoitov 
  Arnd Bergmann 
  Benjamin Beichler 
  Colin Ian King 
  Cong Wang 
  Dan Carpenter 
  Daniel Borkmann 

Re: [Xen-devel] [PATCH] xen: append EXTRA_CFLAGS to CFLAGS for expert builds

[Andrew Cooper]

On 18/01/18 17:04, Doug Goldstein wrote:
> Allow a user to supply extra CFLAGS via the EXTRA_CFLAGS environment
> variable.  This is not a configuration that is supported but is only
> aimed to help support testing and troubleshooting when you need to make
> changes.
>
> Signed-off-by: Doug Goldstein 

Reviewed-by: Andrew Cooper 



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

Re: [Xen-devel] [RFC PATCH] arm64: vgic-v3: Add ITS doorbell region in Dom0 stage-2

[Julien Grall]

Hi,

On 18/01/18 17:25, Manish Jaggi wrote:



On 01/18/2018 10:22 PM, Julien Grall wrote:

Hi Manish,

Hi Julien,


On 18/01/18 06:15, mja...@caviumnetworks.com wrote:

From: Manish Jaggi 

This patch introduces a function vgic_map_translation_space for mapping
ITS 64K translater space for doorbells in dom0 stage-2.
It is required as dom0 PCI devices behined SMMU  wont be able to 
write MSIs.


s/behined/behind/

also there a two spaces after SMMU. One should be enough.


ok


This function is called from vgic_v3_its_init_domain only for 
hardware domain.


This is really confusing. You mix dom0 and hardware domain in the same 
commit. Please use only "hardware domain".
I stressed this here as this function is only called for hardware 
domain, if you see line 1550


So why are you using the word Dom0 everywhere else? I keep asking you to 
use the term "Hardware Domain".



|if ( is_hardware_domain(d) ) is checked.|





This patch is also required for testing
[RFC 00/11] acpi: arm: IORT Support for Xen [1]
+ [2]  [RFC v4 0/8] SMMUv3 driver
  [1] 
https://lists.xenproject.org/archives/html/xen-devel/2018-01/msg7.html

[2] https://lists.xen.org/archives/html/xen-devel/2017-12/msg01294.html


Anything after "This patch..." does not seem to be relevant in the 
commit. You might want to add that after --- so it get stripped after 
committing.

ok




Signed-off-by: Manish Jaggi 
---
  xen/arch/arm/vgic-v3-its.c | 26 ++
  1 file changed, 26 insertions(+)

diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index 72a5c70656..a8f8079149 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -1511,6 +1511,30 @@ unsigned int vgic_v3_its_count(const struct 
domain *d)

  return ret;
  }
  +static int vgic_map_translation_space(struct domain *d, paddr_t 
guest_addr)


This function is quite confusing. What is guest_addr? It looks like it 
is the base address of the ITS but it is not very clear here.

ok I will change the name.



+{
+    u64 addr, size;


This should be paddr_t.


+    int ret;


Newline here please.


+    addr = guest_addr + SZ_64K;
+    size = SZ_64K;
+
+    ret = map_mmio_regions(d,
+   paddr_to_pfn(addr & PAGE_MASK),
+   DIV_ROUND_UP(size, PAGE_SIZE),
+   paddr_to_pfn(addr & PAGE_MASK));


You are assuming a 1:1 mapping in the domain. Nowhere you explain that 
assumption nor that it will not work for guest. At least you need an 
ASSERT(is_domain_direct_mapped(d)).


Furthermore, as we discussed yesterday I would expect a big fat 
comment that this need to be revisited when DomU support will be added 
as it may not be safe to map the Doorbell in page-tables used by the 
processor.
I think you didnt read the commit log properly, this function is *only* 
called for hardware domain. So there is no question of DomU.

Request you to please see the code again.


I read properly the commit log... and my point still stands. This 
function is name vgic_map_translation_space. How a developer would guess 
that it can't be used for other domain than the hardware domain and 
direct mapped? More that you pass a domain pointer...



  /*
   * For a hardware domain, this will iterate over the host ITSes
   * and map one virtual ITS per host ITS at the same address.
@@ -1541,6 +1565,8 @@ int vgic_v3_its_init_domain(struct domain *d)
  return ret;
  else
  d->arch.vgic.has_its = true;
+
+    vgic_map_translation_space(d, hw_its->addr);

Your new function may return an error. Therefore you should test it.
yes, I thought we will focus more is this is the right place to add this 
test code.


This is not difficult to check error return in a first draft. Please 
stop sending patch that are full of coding style error or half done 
(without even mentioning it).


This is rather annoying and a way to miss that afterwards.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH] arm64: vgic-v3: Add ITS doorbell region in Dom0 stage-2

[Manish Jaggi]

On 01/18/2018 10:22 PM, Julien Grall wrote:

Hi Manish,

Hi Julien,


On 18/01/18 06:15, mja...@caviumnetworks.com wrote:

From: Manish Jaggi 

This patch introduces a function vgic_map_translation_space for mapping
ITS 64K translater space for doorbells in dom0 stage-2.
It is required as dom0 PCI devices behined SMMU  wont be able to 
write MSIs.


s/behined/behind/

also there a two spaces after SMMU. One should be enough.


ok


This function is called from vgic_v3_its_init_domain only for 
hardware domain.


This is really confusing. You mix dom0 and hardware domain in the same 
commit. Please use only "hardware domain".
I stressed this here as this function is only called for hardware 
domain, if you see line 1550


|if ( is_hardware_domain(d) ) is checked.|





This patch is also required for testing
[RFC 00/11] acpi: arm: IORT Support for Xen [1]
+ [2]  [RFC v4 0/8] SMMUv3 driver
  [1] 
https://lists.xenproject.org/archives/html/xen-devel/2018-01/msg7.html

[2] https://lists.xen.org/archives/html/xen-devel/2017-12/msg01294.html


Anything after "This patch..." does not seem to be relevant in the 
commit. You might want to add that after --- so it get stripped after 
committing.

ok




Signed-off-by: Manish Jaggi 
---
  xen/arch/arm/vgic-v3-its.c | 26 ++
  1 file changed, 26 insertions(+)

diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index 72a5c70656..a8f8079149 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -1511,6 +1511,30 @@ unsigned int vgic_v3_its_count(const struct 
domain *d)

  return ret;
  }
  +static int vgic_map_translation_space(struct domain *d, paddr_t 
guest_addr)


This function is quite confusing. What is guest_addr? It looks like it 
is the base address of the ITS but it is not very clear here.

ok I will change the name.



+{
+    u64 addr, size;


This should be paddr_t.


+    int ret;


Newline here please.


+    addr = guest_addr + SZ_64K;
+    size = SZ_64K;
+
+    ret = map_mmio_regions(d,
+   paddr_to_pfn(addr & PAGE_MASK),
+   DIV_ROUND_UP(size, PAGE_SIZE),
+   paddr_to_pfn(addr & PAGE_MASK));


You are assuming a 1:1 mapping in the domain. Nowhere you explain that 
assumption nor that it will not work for guest. At least you need an 
ASSERT(is_domain_direct_mapped(d)).


Furthermore, as we discussed yesterday I would expect a big fat 
comment that this need to be revisited when DomU support will be added 
as it may not be safe to map the Doorbell in page-tables used by the 
processor.
I think you didnt read the commit log properly, this function is *only* 
called for hardware domain. So there is no question of DomU.

Request you to please see the code again.

|int vgic_v3_its_init_domain(struct domain *d) { ... if ( 
is_hardware_domain(d) ) { struct host_its *hw_its; 
list_for_each_entry(hw_its, _its_list, entry) { ... 
vgic_map_translation_space(d, hw_its->addr); } } ... } |







+
+    if ( ret )
+    {
+    printk(XENLOG_ERR "Unable to map to dom%d access to"
+   " 0x%"PRIx64" - 0x%"PRIx64"\n",
+   d->domain_id,
+   addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
+    }
+
+    return ret;
+
+}
+
  /*
   * For a hardware domain, this will iterate over the host ITSes
   * and map one virtual ITS per host ITS at the same address.
@@ -1541,6 +1565,8 @@ int vgic_v3_its_init_domain(struct domain *d)
  return ret;
  else
  d->arch.vgic.has_its = true;
+
+    vgic_map_translation_space(d, hw_its->addr);

Your new function may return an error. Therefore you should test it.
yes, I thought we will focus more is this is the right place to add this 
test code.



  }
  }



Cheers,



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

[Xen-devel] [xen-4.6-testing test] 118166: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 117116
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 117116
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 117116
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 117116
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 117116
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 117116
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 117116
 test-armhf-armhf-xl-rtds 12 guest-start  fail  like 117116
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 117116
 test-xtf-amd64-amd64-1   73 xtf/test-pv32pae-xsa-194 fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-2   73 xtf/test-pv32pae-xsa-194 fail   never pass
 test-xtf-amd64-amd64-4   73 xtf/test-pv32pae-xsa-194 fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-3   73 xtf/test-pv32pae-xsa-194 fail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-xtf-amd64-amd64-5   73 xtf/test-pv32pae-xsa-194 fail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-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-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail 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-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass

version targeted for testing:
 xen  44ad7f6895da9861042d7a41e635d42d83cb2660
baseline version:
 xen  f94c11d2ff3219b571c8b0c6197dccf21cb4537b

Last test of basis   117116  2017-12-13 18:11:15 Z   35 days
Testing same since   118166  2018-01-17 16:49:59 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64-xtf  pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt   

Re: [Xen-devel] [PATCH] xen: append EXTRA_CFLAGS to CFLAGS for expert builds

[Ian Jackson]

Doug Goldstein writes ("[PATCH] xen: append EXTRA_CFLAGS to CFLAGS for expert 
builds"):
> Allow a user to supply extra CFLAGS via the EXTRA_CFLAGS environment
> variable.  This is not a configuration that is supported but is only
> aimed to help support testing and troubleshooting when you need to make
> changes.

Something like this is IMO essential.  I don't care what the variable
is called.

Ian.
(not a hypervisor maintainer)

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

[Xen-devel] [PATCH] xen: append EXTRA_CFLAGS to CFLAGS for expert builds

[Doug Goldstein]

Allow a user to supply extra CFLAGS via the EXTRA_CFLAGS environment
variable.  This is not a configuration that is supported but is only
aimed to help support testing and troubleshooting when you need to make
changes.

Signed-off-by: Doug Goldstein 
---
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 
CC: Roger Pau Monné 
---
 xen/Rules.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 3cf40754a6..63b559035f 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -75,6 +75,8 @@ ALL_OBJS := $(ALL_OBJS-y)
 CFLAGS-y += -MMD -MF $(@D)/.$(@F).d
 
 CFLAGS += $(CFLAGS-y)
+# allow extra CFLAGS externally via EXTRA_CFLAGS
+CFLAGS += $(EXTRA_CFLAGS)
 
 # Most CFLAGS are safe for assembly files:
 #  -std=gnu{89,99} gets confused by #-prefixed end-of-line comments
-- 
2.13.6


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

Re: [Xen-devel] [RFC PATCH] arm64: vgic-v3: Add ITS doorbell region in Dom0 stage-2

[Julien Grall]

Hi Manish,

On 18/01/18 06:15, mja...@caviumnetworks.com wrote:

From: Manish Jaggi 

This patch introduces a function vgic_map_translation_space for mapping
ITS 64K translater space for doorbells in dom0 stage-2.
It is required as dom0 PCI devices behined SMMU  wont be able to write MSIs.


s/behined/behind/

also there a two spaces after SMMU. One should be enough.



This function is called from vgic_v3_its_init_domain only for hardware domain.


This is really confusing. You mix dom0 and hardware domain in the same 
commit. Please use only "hardware domain".




This patch is also required for testing
[RFC 00/11] acpi: arm: IORT Support for Xen [1]
+ [2]  [RFC v4 0/8] SMMUv3 driver
  
[1] https://lists.xenproject.org/archives/html/xen-devel/2018-01/msg7.html

[2] https://lists.xen.org/archives/html/xen-devel/2017-12/msg01294.html


Anything after "This patch..." does not seem to be relevant in the 
commit. You might want to add that after --- so it get stripped after 
committing.




Signed-off-by: Manish Jaggi 
---
  xen/arch/arm/vgic-v3-its.c | 26 ++
  1 file changed, 26 insertions(+)

diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index 72a5c70656..a8f8079149 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -1511,6 +1511,30 @@ unsigned int vgic_v3_its_count(const struct domain *d)
  return ret;
  }
  
+static int vgic_map_translation_space(struct domain *d, paddr_t guest_addr)


This function is quite confusing. What is guest_addr? It looks like it 
is the base address of the ITS but it is not very clear here.



+{
+u64 addr, size;


This should be paddr_t.


+int ret;


Newline here please.


+addr = guest_addr + SZ_64K;
+size = SZ_64K;
+
+ret = map_mmio_regions(d,
+   paddr_to_pfn(addr & PAGE_MASK),
+   DIV_ROUND_UP(size, PAGE_SIZE),
+   paddr_to_pfn(addr & PAGE_MASK));


You are assuming a 1:1 mapping in the domain. Nowhere you explain that 
assumption nor that it will not work for guest. At least you need an 
ASSERT(is_domain_direct_mapped(d)).


Furthermore, as we discussed yesterday I would expect a big fat comment 
that this need to be revisited when DomU support will be added as it may 
not be safe to map the Doorbell in page-tables used by the processor.



+
+if ( ret )
+{
+printk(XENLOG_ERR "Unable to map to dom%d access to"
+   " 0x%"PRIx64" - 0x%"PRIx64"\n",
+   d->domain_id,
+   addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
+}
+
+return ret;
+
+}
+
  /*
   * For a hardware domain, this will iterate over the host ITSes
   * and map one virtual ITS per host ITS at the same address.
@@ -1541,6 +1565,8 @@ int vgic_v3_its_init_domain(struct domain *d)
  return ret;
  else
  d->arch.vgic.has_its = true;
+
+vgic_map_translation_space(d, hw_its->addr);

Your new function may return an error. Therefore you should test it.


  }
  }
  



Cheers,

--
Julien Grall

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

Re: [Xen-devel] Ping: [PATCH v2 1/2] x86/PoD: correctly handle non-order-0 decrease-reservation requests

[Julien Grall]

Hi Jan,

On 18/01/18 15:59, Jan Beulich wrote:

On 20.12.17 at 10:34,  wrote:

p2m_pod_decrease_reservation() at the moment only returns a boolean
value: true for "nothing more to do", false for "something more to do".
If it returns false, decrease_reservation() will loop over the entire
range, calling guest_remove_page() for each page.

Unfortunately, in the case p2m_pod_decrease_reservation() succeeds
partially, some of the memory in the range will be not-present; at which
point guest_remove_page() will return an error, and the entire operation
will fail.

Fix this by:
1. Having p2m_pod_decrease_reservation() return exactly the number of
gpfn pages it has handled (i.e., replaced with 'not present').
2. Making guest_remove_page() return -ENOENT in the case that the gpfn
in question was already empty (and in no other cases).
3. When looping over guest_remove_page(), expect the number of -ENOENT
failures to be no larger than the number of pages
p2m_pod_decrease_reservation() removed.

Signed-off-by: Jan Beulich 
Signed-off-by: George Dunlap 
---
v2: Re-written description (by George). Add comments (as suggested
 by George). Formatting.

--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -388,10 +388,10 @@ int guest_physmap_mark_populate_on_deman
  return -ENOSYS;
  }
  
-int p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn,

- unsigned int order)
+unsigned long p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn,
+   unsigned int order)
  {
-return -ENOSYS;
+return 0;
  }
  
  static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a)


Stefano, Julien?


Sorry, it fell through the cracks.

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

[Xen-devel] [PATCH v3] x86/hvm: re-work viridian APIC assist code

[Paul Durrant]

It appears there is a case where Windows enables the APIC assist
enlightenment[1] but does not use it. This scenario is perfectly valid
according to the documentation, but causes the state machine in Xen to
become confused leading to a domain_crash() such as the following:

(XEN) d4: VIRIDIAN GUEST_OS_ID: vendor: 1 os: 4 major: 6 minor: 1 sp: 0
  build: 1db0
(XEN) d4: VIRIDIAN HYPERCALL: enabled: 1 pfn: 3
(XEN) d4v0: VIRIDIAN VP_ASSIST_PAGE: enabled: 1 pfn: 3fffe
(XEN) domain_crash called from viridian.c:452
(XEN) Domain 4 (vcpu#0) crashed on cpu#1:

The following sequence of events is an example of how this can happen:

 - On return to guest vlapic_has_pending_irq() finds a bit set in the IRR.
 - vlapic_ack_pending_irq() calls viridian_start_apic_assist() which latches
   the vector, sets the bit in the ISR and clears it from the IRR.
 - The guest then processes the interrupt but EOIs it normally, therefore
   clearing the bit in the ISR.
 - On next return to guest vlapic_has_pending_irq() calls
   viridian_complete_apic_assist(), which discovers the assist bit still set
   in the shared page and therefore leaves the latched vector in place, but
   also finds another bit set in the IRR.
 - vlapic_ack_pending_irq() is then called but, because the ISR is was
   cleared by the EOI, another call is made to viridian_start_apic_assist()
   and this then calls domain_crash() because it finds the latched vector
   has not been cleared.

Having re-visited the code I also conclude that Xen's implementation of the
enlightenment is currently wrong and we are not properly following the
specification.

The specification says:

"The hypervisor sets the “No EOI required” bit when it injects a virtual
 interrupt if the following conditions are satisfied:

 - The virtual interrupt is edge-triggered, and
 - There are no lower priority interrupts pending.

 If, at a later time, a lower priority interrupt is requested, the
 hypervisor clears the “No EOI required” such that a subsequent EOI causes
 an intercept.
 In case of nested interrupts, the EOI intercept is avoided only for the
 highest priority interrupt. This is necessary since no count is maintained
 for the number of EOIs performed by the OS. Therefore only the first EOI
 can be avoided and since the first EOI clears the “No EOI Required” bit,
 the next EOI generates an intercept."

Thus it is quite legitimate to set the "No EOI required" bit and then
subsequently take a higher priority interrupt without clearing the bit.
Thus the avoided EOI will then relate to that subsequent interrupt rather
than the highest priority interrupt when the bit was set. Hence latching
the vector when setting the bit is not entirely useful and somewhat
misleading.

This patch re-works the APIC assist code to simply track when the "No EOI
required" bit is set and test if it has been cleared by the guest (i.e.
'completing' the APIC assist), thus indicating a 'missed EOI'. Missed EOIs
need to be dealt with in two places:

 - In vlapic_has_pending_irq(), to avoid comparing the IRR against a stale
   ISR, and
 - In vlapic_EOI_set() because a missed EOI for a higher priority vector
   should be dealt with before the actual EOI for the lower priority
   vector.

Furthermore, because the guest is at liberty to ignore the "No EOI required"
bit (which lead the crash detailed above) vlapic_EOI_set() must also make
sure the bit is cleared to avoid confusing the state machine.

Lastly the previous code did not properly emulate an EOI if a missed EOI
was discovered in vlapic_has_pending_irq(); it merely cleared the bit in
the ISR. The new code instead calls vlapic_EOI_set().

[1] See section 10.3.5 of Microsoft's "Hypervisor Top Level Functional
Specification v5.0b".

NOTE: The changes to the save/restore code are safe because the layout
  of struct hvm_viridian_vcpu_context is unchanged and the new
  interpretation of the (previously so named) vp_assist_vector field
  as the boolean pending flag maintains the correct semantics.

Signed-off-by: Paul Durrant 
Reviewed-by: Jan Beulich 
---
Cc: Andrew Cooper 

v3:
 - Cosmetic fixes requested by Jan

v2:
 - Extend patch to completely re-work APIC assist code
---
 xen/arch/x86/hvm/viridian.c| 36 
 xen/arch/x86/hvm/vlapic.c  | 75 --
 xen/include/asm-x86/hvm/viridian.h |  8 ++--
 xen/include/public/arch-x86/hvm/save.h |  2 +-
 4 files changed, 76 insertions(+), 45 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index f0fa59d7d5..70aab520bc 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -433,46 +433,44 @@ static void teardown_vp_assist(struct vcpu *v)
 put_page_and_type(page);
 }
 
-void viridian_start_apic_assist(struct vcpu *v, int vector)
+void viridian_apic_assist_set(struct vcpu *v)
 {
 uint32_t *va = 

Re: [Xen-devel] [PATCH v2] x86/hvm: re-work viridian APIC assist code

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 18 January 2018 16:21
> To: Paul Durrant 
> Cc: Andrew Cooper ; xen-
> de...@lists.xenproject.org
> Subject: Re: [PATCH v2] x86/hvm: re-work viridian APIC assist code
> 
> >>> On 18.01.18 at 16:10,  wrote:
> > -int viridian_complete_apic_assist(struct vcpu *v)
> > +bool viridian_apic_assist_completed(struct vcpu *v)
> >  {
> >  uint32_t *va = v->arch.hvm_vcpu.viridian.vp_assist.va;
> > -int vector;
> >
> >  if ( !va )
> > -return 0;
> > +return false;
> >
> > -if ( *va & 1u )
> > -return 0; /* Interrupt not yet processed by the guest. */
> > -
> > -vector = v->arch.hvm_vcpu.viridian.vp_assist.vector;
> > -v->arch.hvm_vcpu.viridian.vp_assist.vector = 0;
> > +if ( v->arch.hvm_vcpu.viridian.vp_assist.pending &&
> > + !(*va & 1u) )
> > +{
> > +/* An EOI has been avoided */
> > +v->arch.hvm_vcpu.viridian.vp_assist.pending = false;
> > +return true;
> > +}
> 
> Especially with such a non-atomic update, please remind me: Despite
> them having struct vcpu * parameters, these functions are all only
> ever called with v == current? If the answer is yes, then
> Reviewed-by: Jan Beulich 

Thanks. The answer is yes. They are only called from the vlapic code for the 
current vlapic AFAICT.

> with one more cosmetic remark:
> 
> > @@ -120,9 +120,9 @@ void viridian_time_ref_count_thaw(struct domain
> *d);
> >  void viridian_vcpu_deinit(struct vcpu *v);
> >  void viridian_domain_deinit(struct domain *d);
> >
> > -void viridian_start_apic_assist(struct vcpu *v, int vector);
> > -int viridian_complete_apic_assist(struct vcpu *v);
> > -void viridian_abort_apic_assist(struct vcpu *v);
> > +void viridian_set_apic_assist(struct vcpu *v);
> > +bool viridian_apic_assist_completed(struct vcpu *v);
> > +void viridian_clear_apic_assist(struct vcpu *v);
> 
> Could I talk you into naming them all viridian_apic_assist_...()?
> 

Sure. I'll send a v3 with that fixed and your R-b.

  Paul

> Jan


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

Re: [Xen-devel] [PATCH v2] x86/hvm: re-work viridian APIC assist code

[Jan Beulich]

>>> On 18.01.18 at 16:10,  wrote:
> -int viridian_complete_apic_assist(struct vcpu *v)
> +bool viridian_apic_assist_completed(struct vcpu *v)
>  {
>  uint32_t *va = v->arch.hvm_vcpu.viridian.vp_assist.va;
> -int vector;
>  
>  if ( !va )
> -return 0;
> +return false;
>  
> -if ( *va & 1u )
> -return 0; /* Interrupt not yet processed by the guest. */
> -
> -vector = v->arch.hvm_vcpu.viridian.vp_assist.vector;
> -v->arch.hvm_vcpu.viridian.vp_assist.vector = 0;
> +if ( v->arch.hvm_vcpu.viridian.vp_assist.pending &&
> + !(*va & 1u) )
> +{
> +/* An EOI has been avoided */
> +v->arch.hvm_vcpu.viridian.vp_assist.pending = false;
> +return true;
> +}

Especially with such a non-atomic update, please remind me: Despite
them having struct vcpu * parameters, these functions are all only
ever called with v == current? If the answer is yes, then
Reviewed-by: Jan Beulich 
with one more cosmetic remark:

> @@ -120,9 +120,9 @@ void viridian_time_ref_count_thaw(struct domain *d);
>  void viridian_vcpu_deinit(struct vcpu *v);
>  void viridian_domain_deinit(struct domain *d);
>  
> -void viridian_start_apic_assist(struct vcpu *v, int vector);
> -int viridian_complete_apic_assist(struct vcpu *v);
> -void viridian_abort_apic_assist(struct vcpu *v);
> +void viridian_set_apic_assist(struct vcpu *v);
> +bool viridian_apic_assist_completed(struct vcpu *v);
> +void viridian_clear_apic_assist(struct vcpu *v);

Could I talk you into naming them all viridian_apic_assist_...()?

Jan


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

Re: [Xen-devel] [PATCH v2] x86/efi: fix build with linkers that support both coff-x86-64 and pe-x86-64

[Doug Goldstein]

On 1/17/18 6:56 AM, Roger Pau Monne wrote:
> When using a linker that supports both formats the following error
> will be triggered:
> 
> efi/buildid.o: file not recognized: File format is ambiguous
> efi/buildid.o: matching formats: coff-x86-64 pe-x86-64
> 
> Solve this by specifying the efi/buildid.o format to pe-x86-64.
> 
> Signed-off-by: Roger Pau Monné 
> ---

Reviewed-by: Doug Goldstein 
-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v9 01/11] x86/thunk: Fix GEN_INDIRECT_THUNK comment

[Jan Beulich]

>>> On 18.01.18 at 16:46,  wrote:
> This is a rebasing error in c/s 858cba0d4c6b "x86: Introduce alternative
> indirect thunks" hidden by other changes in the same sentence.
> 
> The name with dots rather than underscores was the prerelease GCC ABI.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich 



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

[Xen-devel] Ping: [PATCH v2 1/2] x86/PoD: correctly handle non-order-0 decrease-reservation requests

[Jan Beulich]

>>> On 20.12.17 at 10:34,  wrote:
> p2m_pod_decrease_reservation() at the moment only returns a boolean
> value: true for "nothing more to do", false for "something more to do".
> If it returns false, decrease_reservation() will loop over the entire
> range, calling guest_remove_page() for each page.
> 
> Unfortunately, in the case p2m_pod_decrease_reservation() succeeds
> partially, some of the memory in the range will be not-present; at which
> point guest_remove_page() will return an error, and the entire operation
> will fail.
> 
> Fix this by:
> 1. Having p2m_pod_decrease_reservation() return exactly the number of
>gpfn pages it has handled (i.e., replaced with 'not present').
> 2. Making guest_remove_page() return -ENOENT in the case that the gpfn
>in question was already empty (and in no other cases).
> 3. When looping over guest_remove_page(), expect the number of -ENOENT
>failures to be no larger than the number of pages
>p2m_pod_decrease_reservation() removed.
> 
> Signed-off-by: Jan Beulich 
> Signed-off-by: George Dunlap 
> ---
> v2: Re-written description (by George). Add comments (as suggested
> by George). Formatting.
> 
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -388,10 +388,10 @@ int guest_physmap_mark_populate_on_deman
>  return -ENOSYS;
>  }
>  
> -int p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn,
> - unsigned int order)
> +unsigned long p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn,
> +   unsigned int order)
>  {
> -return -ENOSYS;
> +return 0;
>  }
>  
>  static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a)

Stefano, Julien?

Jan


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

[Xen-devel] [PATCH v9 10/11] x86/cpuid: Offer Indirect Branch Controls to guests

[Andrew Cooper]

With all infrastructure in place, it is now safe to let guests see and use
these features.

Signed-off-by: Andrew Cooper 
Acked-by: Jan Beulich 
Acked-by: Wei Liu 
---
v9:
 * Split patch in half with the libxc hunk moving earlier, and rebasing over
   the changed nature of STIBP
---
 xen/include/public/arch-x86/cpufeatureset.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/include/public/arch-x86/cpufeatureset.h 
b/xen/include/public/arch-x86/cpufeatureset.h
index 0f21fed..fa81af1 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -237,13 +237,13 @@ XEN_CPUFEATURE(EFRO,  7*32+10) /*   APERF/MPERF 
Read Only interface */
 
 /* AMD-defined CPU features, CPUID level 0x8008.ebx, word 8 */
 XEN_CPUFEATURE(CLZERO,8*32+ 0) /*A  CLZERO instruction */
-XEN_CPUFEATURE(IBPB,  8*32+12) /*   IBPB support only (no IBRS, used 
by AMD) */
+XEN_CPUFEATURE(IBPB,  8*32+12) /*A  IBPB support only (no IBRS, used 
by AMD) */
 
 /* Intel-defined CPU features, CPUID level 0x0007:0.edx, word 9 */
 XEN_CPUFEATURE(AVX512_4VNNIW, 9*32+ 2) /*A  AVX512 Neural Network Instructions 
*/
 XEN_CPUFEATURE(AVX512_4FMAPS, 9*32+ 3) /*A  AVX512 Multiply Accumulation 
Single Precision */
-XEN_CPUFEATURE(IBRSB, 9*32+26) /*   IBRS and IBPB support (used by 
Intel) */
-XEN_CPUFEATURE(STIBP, 9*32+27) /*!  STIBP */
+XEN_CPUFEATURE(IBRSB, 9*32+26) /*A  IBRS and IBPB support (used by 
Intel) */
+XEN_CPUFEATURE(STIBP, 9*32+27) /*A! STIBP */
 
 #endif /* XEN_CPUFEATURE */
 
-- 
2.1.4


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

[Xen-devel] [PATCH v9 04/11] x86/migrate: Move MSR_SPEC_CTRL on migrate

[Andrew Cooper]

Signed-off-by: Andrew Cooper 
Reviewed-by: Wei Liu 
Reviewed-by: Jan Beulich 
---
 xen/arch/x86/domctl.c  | 2 ++
 xen/arch/x86/hvm/hvm.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 2585d4e..1a15a34 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1292,6 +1292,7 @@ long arch_do_domctl(
 struct xen_domctl_vcpu_msr msr = {};
 struct vcpu *v;
 static const uint32_t msrs_to_send[] = {
+MSR_SPEC_CTRL,
 MSR_INTEL_MISC_FEATURES_ENABLES,
 };
 uint32_t nr_msrs = ARRAY_SIZE(msrs_to_send);
@@ -1415,6 +1416,7 @@ long arch_do_domctl(
 
 switch ( msr.index )
 {
+case MSR_SPEC_CTRL:
 case MSR_INTEL_MISC_FEATURES_ENABLES:
 if ( guest_wrmsr(v, msr.index, msr.value) != X86EMUL_OKAY )
 break;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index db282b5..ed36598 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1323,6 +1323,7 @@ static int hvm_load_cpu_xsave_states(struct domain *d, 
hvm_domain_context_t *h)
 
 #define HVM_CPU_MSR_SIZE(cnt) offsetof(struct hvm_msr, msr[cnt])
 static const uint32_t msrs_to_send[] = {
+MSR_SPEC_CTRL,
 MSR_INTEL_MISC_FEATURES_ENABLES,
 };
 static unsigned int __read_mostly msr_count_max = ARRAY_SIZE(msrs_to_send);
@@ -1458,6 +1459,7 @@ static int hvm_load_cpu_msrs(struct domain *d, 
hvm_domain_context_t *h)
 {
 int rc;
 
+case MSR_SPEC_CTRL:
 case MSR_INTEL_MISC_FEATURES_ENABLES:
 rc = guest_wrmsr(v, ctxt->msr[i].index, ctxt->msr[i].val);
 
-- 
2.1.4


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

[Xen-devel] [PATCH v9 06/11] x86/entry: Organise the use of MSR_SPEC_CTRL at each entry/exit point

[Andrew Cooper]

We need to be able to either set or clear IBRS in Xen context, as well as
restore appropriate guest values in guest context.  See the documentation in
asm-x86/spec_ctrl_asm.h for details.

Writes to %cr3 are slower when SPEC_CTRL.IBRS is set, so the
SPEC_CTRL_{ENTRY/EXIT}* positioning is important, and optimised for the
expected common case of Xen not using IBRS itself.

There is a semi-unrelated bugfix, where various asm_defn.h macros have a
hidden dependency on PAGE_SIZE, which results in an assembler error if used in
a .macro definition.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v7:
 * Spelling fixes
 * Reposition the semicolon fix.
v9:
 * Rebase over XPTI.  Moderate changes in the exit-to-PV paths.
 * Swap cmpw for testb when looking for cpl0
---
 xen/arch/x86/hvm/svm/entry.S|   8 +-
 xen/arch/x86/hvm/vmx/entry.S|  11 ++
 xen/arch/x86/setup.c|   1 +
 xen/arch/x86/smpboot.c  |   2 +
 xen/arch/x86/x86_64/asm-offsets.c   |   6 +
 xen/arch/x86/x86_64/compat/entry.S  |  14 +++
 xen/arch/x86/x86_64/entry.S |  34 ++
 xen/include/asm-x86/asm_defns.h |   3 +
 xen/include/asm-x86/current.h   |   6 +
 xen/include/asm-x86/nops.h  |   6 +
 xen/include/asm-x86/spec_ctrl.h |   9 ++
 xen/include/asm-x86/spec_ctrl_asm.h | 227 
 12 files changed, 326 insertions(+), 1 deletion(-)
 create mode 100644 xen/include/asm-x86/spec_ctrl_asm.h

diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
index df86da0..fb1048b 100644
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -79,6 +79,9 @@ UNLIKELY_END(svm_trace)
 or   $X86_EFLAGS_MBS,%rax
 mov  %rax,VMCB_rflags(%rcx)
 
+/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
+SPEC_CTRL_EXIT_TO_GUEST /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
+
 pop  %r15
 pop  %r14
 pop  %r13
@@ -101,8 +104,11 @@ UNLIKELY_END(svm_trace)
 SAVE_ALL
 
 GET_CURRENT(bx)
-mov  VCPU_svm_vmcb(%rbx),%rcx
 
+SPEC_CTRL_ENTRY_FROM_VMEXIT /* Req: b=curr %rsp=regs/cpuinfo, Clob: 
acd */
+/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
+
+mov  VCPU_svm_vmcb(%rbx),%rcx
 movb $0,VCPU_svm_vmcb_in_sync(%rbx)
 mov  VMCB_rax(%rcx),%rax
 mov  %rax,UREGS_rax(%rsp)
diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S
index b2f98be..21e959f 100644
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -38,6 +38,9 @@ ENTRY(vmx_asm_vmexit_handler)
 movb $1,VCPU_vmx_launched(%rbx)
 mov  %rax,VCPU_hvm_guest_cr2(%rbx)
 
+SPEC_CTRL_ENTRY_FROM_VMEXIT /* Req: b=curr %rsp=regs/cpuinfo, Clob: 
acd */
+/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
+
 mov  %rsp,%rdi
 call vmx_vmexit_handler
 
@@ -68,6 +71,10 @@ UNLIKELY_END(realmode)
 call vmx_vmenter_helper
 test %al, %al
 jz .Lvmx_vmentry_restart
+
+/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
+SPEC_CTRL_EXIT_TO_GUEST /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
+
 mov  VCPU_hvm_guest_cr2(%rbx),%rax
 
 pop  %r15
@@ -99,6 +106,10 @@ UNLIKELY_END(realmode)
 .Lvmx_vmentry_fail:
 sti
 SAVE_ALL
+
+SPEC_CTRL_ENTRY_FROM_PV /* Req: %rsp=regs/cpuinfo Clob: acd */
+/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
+
 call vmx_vmentry_failure
 BUG  /* vmx_vmentry_failure() shouldn't return. */
 
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b6888c7..41afd2a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -678,6 +678,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 set_processor_id(0);
 set_current(INVALID_VCPU); /* debug sanity. */
 idle_vcpu[0] = current;
+init_shadow_spec_ctrl_state();
 
 percpu_init_areas();
 
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 2cdd431..196ee7e 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -309,6 +310,7 @@ void start_secondary(void *unused)
 set_current(idle_vcpu[cpu]);
 this_cpu(curr_vcpu) = idle_vcpu[cpu];
 rdmsrl(MSR_EFER, this_cpu(efer));
+init_shadow_spec_ctrl_state();
 
 /*
  * Just as during early bootstrap, it is convenient here to disable
diff --git a/xen/arch/x86/x86_64/asm-offsets.c 
b/xen/arch/x86/x86_64/asm-offsets.c
index b1a4310..17f1d77 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -88,6 +88,7 @@ void __dummy__(void)
 OFFSET(VCPU_kernel_ss, struct vcpu, arch.pv_vcpu.kernel_ss);
 OFFSET(VCPU_iopl, struct vcpu, arch.pv_vcpu.iopl);
 

[Xen-devel] [PATCH v9 05/11] x86/hvm: Permit guests direct access to MSR_{SPEC_CTRL, PRED_CMD}

[Andrew Cooper]

For performance reasons, HVM guests should have direct access to these MSRs
when possible.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Jun Nakajima 
CC: Kevin Tian 
CC: Boris Ostrovsky 
CC: Suravee Suthikulpanit 

v7:
 * Drop excess brackets
v9:
 * Re-implement it light of Intels new spec.  Drop R-by's.
 * Spelling fixes
---
 xen/arch/x86/domctl.c  | 19 +++
 xen/arch/x86/hvm/svm/svm.c |  5 +
 xen/arch/x86/hvm/vmx/vmx.c | 17 +
 3 files changed, 41 insertions(+)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 1a15a34..a9adae3 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -53,6 +53,7 @@ static int update_domain_cpuid_info(struct domain *d,
 struct cpuid_policy *p = d->arch.cpuid;
 const struct cpuid_leaf leaf = { ctl->eax, ctl->ebx, ctl->ecx, ctl->edx };
 int old_vendor = p->x86_vendor;
+unsigned int old_7d0 = p->feat.raw[0].d, old_e8b = p->extd.raw[8].b;
 bool call_policy_changed = false; /* Avoid for_each_vcpu() unnecessarily */
 
 /*
@@ -218,6 +219,14 @@ static int update_domain_cpuid_info(struct domain *d,
 
 d->arch.pv_domain.cpuidmasks->_7ab0 = mask;
 }
+
+/*
+ * If the IBRS/IBPB policy has changed, we need to recalculate the MSR
+ * interception bitmaps.
+ */
+call_policy_changed = (is_hvm_domain(d) &&
+   ((old_7d0 ^ p->feat.raw[0].d) &
+cpufeat_mask(X86_FEATURE_IBRSB)));
 break;
 
 case 0xa:
@@ -292,6 +301,16 @@ static int update_domain_cpuid_info(struct domain *d,
 d->arch.pv_domain.cpuidmasks->e1cd = mask;
 }
 break;
+
+case 0x8008:
+/*
+ * If the IBRB policy has changed, we need to recalculate the MSR
+ * interception bitmaps.
+ */
+call_policy_changed = (is_hvm_domain(d) &&
+   ((old_e8b ^ p->extd.raw[8].b) &
+cpufeat_mask(X86_FEATURE_IBPB)));
+break;
 }
 
 if ( call_policy_changed )
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 9d9ad77..249ede0 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -617,6 +617,7 @@ static void svm_cpuid_policy_changed(struct vcpu *v)
 {
 struct arch_svm_struct *arch_svm = >arch.hvm_svm;
 struct vmcb_struct *vmcb = arch_svm->vmcb;
+const struct cpuid_policy *cp = v->domain->arch.cpuid;
 u32 bitmap = vmcb_get_exception_intercepts(vmcb);
 
 if ( opt_hvm_fep ||
@@ -626,6 +627,10 @@ static void svm_cpuid_policy_changed(struct vcpu *v)
 bitmap &= ~(1U << TRAP_invalid_op);
 
 vmcb_set_exception_intercepts(vmcb, bitmap);
+
+/* Give access to MSR_PRED_CMD if the guest has been told about it. */
+svm_intercept_msr(v, MSR_PRED_CMD,
+  cp->extd.ibpb ? MSR_INTERCEPT_NONE : MSR_INTERCEPT_RW);
 }
 
 static void svm_sync_vmcb(struct vcpu *v)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index e036303..1546c2a 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -656,6 +656,8 @@ void vmx_update_exception_bitmap(struct vcpu *v)
 
 static void vmx_cpuid_policy_changed(struct vcpu *v)
 {
+const struct cpuid_policy *cp = v->domain->arch.cpuid;
+
 if ( opt_hvm_fep ||
  (v->domain->arch.cpuid->x86_vendor != boot_cpu_data.x86_vendor) )
 v->arch.hvm_vmx.exception_bitmap |= (1U << TRAP_invalid_op);
@@ -665,6 +667,21 @@ static void vmx_cpuid_policy_changed(struct vcpu *v)
 vmx_vmcs_enter(v);
 vmx_update_exception_bitmap(v);
 vmx_vmcs_exit(v);
+
+/*
+ * We can safely pass MSR_SPEC_CTRL through to the guest, even if STIBP
+ * isn't enumerated in hardware, as SPEC_CTRL_STIBP is ignored.
+ */
+if ( cp->feat.ibrsb )
+vmx_clear_msr_intercept(v, MSR_SPEC_CTRL, VMX_MSR_RW);
+else
+vmx_set_msr_intercept(v, MSR_SPEC_CTRL, VMX_MSR_RW);
+
+/* MSR_PRED_CMD is safe to pass through if the guest knows about it. */
+if ( cp->feat.ibrsb || cp->extd.ibpb )
+vmx_clear_msr_intercept(v, MSR_PRED_CMD,  VMX_MSR_RW);
+else
+vmx_set_msr_intercept(v, MSR_PRED_CMD,  VMX_MSR_RW);
 }
 
 int vmx_guest_x86_mode(struct vcpu *v)
-- 
2.1.4


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

[Xen-devel] [PATCH v9 07/11] x86/boot: Calculate the most appropriate BTI mitigation to use

[Andrew Cooper]

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v7:
 * static, and tweak comment
---
 docs/misc/xen-command-line.markdown |   6 ++-
 xen/arch/x86/spec_ctrl.c| 104 ++--
 2 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index f73990f..b4a7ecd 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -246,7 +246,7 @@ enough. Setting this to a high value may cause boot 
failure, particularly if
 the NMI watchdog is also enabled.
 
 ### bti (x86)
-> `= List of [ thunk=retpoline|lfence|jmp ]`
+> `= List of [ thunk=retpoline|lfence|jmp, ibrs= ]`
 
 Branch Target Injection controls.  By default, Xen will pick the most
 appropriate BTI mitigations based on compiled in support, loaded microcode,
@@ -261,6 +261,10 @@ locations.  The default thunk is `retpoline` (generally 
preferred for Intel
 hardware), with the alternatives being `jmp` (a `jmp *%reg` gadget, minimal
 overhead), and `lfence` (an `lfence; jmp *%reg` gadget, preferred for AMD).
 
+On hardware supporting IBRS, the `ibrs=` option can be used to force or
+prevent Xen using the feature itself.  If Xen is not using IBRS itself,
+functionality is still set up so IBRS can be virtualised for guests.
+
 ### xenheap\_megabytes (arm32)
 > `= `
 
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 89e7287..7b0daaf 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 
@@ -31,11 +32,12 @@ static enum ind_thunk {
 THUNK_LFENCE,
 THUNK_JMP,
 } opt_thunk __initdata = THUNK_DEFAULT;
+static int opt_ibrs __initdata = -1;
 
 static int __init parse_bti(const char *s)
 {
 const char *ss;
-int rc = 0;
+int val, rc = 0;
 
 do {
 ss = strchr(s, ',');
@@ -55,6 +57,8 @@ static int __init parse_bti(const char *s)
 else
 rc = -EINVAL;
 }
+else if ( (val = parse_boolean("ibrs", s, ss)) >= 0 )
+opt_ibrs = val;
 else
 rc = -EINVAL;
 
@@ -91,24 +95,82 @@ static void __init print_details(enum ind_thunk thunk)
 printk(XENLOG_DEBUG "  Compiled-in support: INDIRECT_THUNK\n");
 
 printk(XENLOG_INFO
-   "BTI mitigations: Thunk %s\n",
+   "BTI mitigations: Thunk %s, Others:%s\n",
thunk == THUNK_NONE  ? "N/A" :
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE? "LFENCE" :
-   thunk == THUNK_JMP   ? "JMP" : "?");
+   thunk == THUNK_JMP   ? "JMP" : "?",
+   boot_cpu_has(X86_FEATURE_XEN_IBRS_SET)? " IBRS+" :
+   boot_cpu_has(X86_FEATURE_XEN_IBRS_CLEAR)  ? " IBRS-"  : "");
+}
+
+/* Calculate whether Retpoline is known-safe on this CPU. */
+static bool __init retpoline_safe(void)
+{
+unsigned int ucode_rev = this_cpu(ucode_cpu_info).cpu_sig.rev;
+
+if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+return true;
+
+if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ boot_cpu_data.x86 != 6 )
+return false;
+
+switch ( boot_cpu_data.x86_model )
+{
+case 0x17: /* Penryn */
+case 0x1d: /* Dunnington */
+case 0x1e: /* Nehalem */
+case 0x1f: /* Auburndale / Havendale */
+case 0x1a: /* Nehalem EP */
+case 0x2e: /* Nehalem EX */
+case 0x25: /* Westmere */
+case 0x2c: /* Westmere EP */
+case 0x2f: /* Westmere EX */
+case 0x2a: /* SandyBridge */
+case 0x2d: /* SandyBridge EP/EX */
+case 0x3a: /* IvyBridge */
+case 0x3e: /* IvyBridge EP/EX */
+case 0x3c: /* Haswell */
+case 0x3f: /* Haswell EX/EP */
+case 0x45: /* Haswell D */
+case 0x46: /* Haswell H */
+return true;
+
+/*
+ * Broadwell processors are retpoline-safe after specific microcode
+ * versions.
+ */
+case 0x3d: /* Broadwell */
+return ucode_rev >= 0x28;
+case 0x47: /* Broadwell H */
+return ucode_rev >= 0x1b;
+case 0x4f: /* Broadwell EP/EX */
+return ucode_rev >= 0xb25;
+case 0x56: /* Broadwell D */
+return false; /* TBD. */
+
+/*
+ * Skylake and later processors are not retpoline-safe.
+ */
+default:
+return false;
+}
 }
 
 void __init init_speculation_mitigations(void)
 {
 enum ind_thunk thunk = THUNK_DEFAULT;
+bool ibrs = false;
 
 /*
  * Has the user specified any custom BTI mitigations?  If so, follow their
  * instructions exactly and disable all heuristics.
  */
-if ( opt_thunk != THUNK_DEFAULT )
+if ( opt_thunk != THUNK_DEFAULT || opt_ibrs != -1 )
 {
 thunk = opt_thunk;
+ibrs  = !!opt_ibrs;
 }
 else
 {
@@ -124,7 +186,21 @@ void __init init_speculation_mitigations(void)
  

[Xen-devel] [PATCH v9 01/11] x86/thunk: Fix GEN_INDIRECT_THUNK comment

[Andrew Cooper]

This is a rebasing error in c/s 858cba0d4c6b "x86: Introduce alternative
indirect thunks" hidden by other changes in the same sentence.

The name with dots rather than underscores was the prerelease GCC ABI.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v9: New.
---
 xen/arch/x86/indirect-thunk.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/indirect-thunk.S b/xen/arch/x86/indirect-thunk.S
index 7d34707..e03fc14 100644
--- a/xen/arch/x86/indirect-thunk.S
+++ b/xen/arch/x86/indirect-thunk.S
@@ -31,7 +31,7 @@
 .endm
 
 /*
- * Build the __x86.indirect_thunk.* symbols.  Execution lands on an
+ * Build the __x86_indirect_thunk_* symbols.  Execution lands on an
  * alternative patch point which implements one of the above THUNK_*'s
  */
 .macro GEN_INDIRECT_THUNK reg:req
-- 
2.1.4


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

[Xen-devel] [PATCH v9 03/11] x86/msr: Emulation of MSR_{SPEC_CTRL, PRED_CMD} for guests

[Andrew Cooper]

As per the spec currently available here:

https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf

MSR_ARCH_CAPABILITIES will only come into existence on new hardware, but is
implemented as a straight #GP for now to avoid being leaky when new hardware
arrives.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v9:
 * Alter the STIBP handling to match Intel's latest spec
 * Drop spec_ctrl.guest as it is no longer needed
---
 xen/arch/x86/msr.c  | 45 +
 xen/include/asm-x86/msr-index.h |  2 ++
 xen/include/asm-x86/msr.h   | 10 +
 3 files changed, 57 insertions(+)

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 187f862..7875d9c 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -120,11 +120,22 @@ int init_vcpu_msr_policy(struct vcpu *v)
 
 int guest_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
 {
+const struct cpuid_policy *cp = v->domain->arch.cpuid;
 const struct msr_domain_policy *dp = v->domain->arch.msr;
 const struct msr_vcpu_policy *vp = v->arch.msr;
 
 switch ( msr )
 {
+case MSR_PRED_CMD:
+/* Write-only */
+goto gp_fault;
+
+case MSR_SPEC_CTRL:
+if ( !cp->feat.ibrsb )
+goto gp_fault;
+*val = vp->spec_ctrl.raw;
+break;
+
 case MSR_INTEL_PLATFORM_INFO:
 if ( !dp->plaform_info.available )
 goto gp_fault;
@@ -132,6 +143,10 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, 
uint64_t *val)
_MSR_PLATFORM_INFO_CPUID_FAULTING;
 break;
 
+case MSR_ARCH_CAPABILITIES:
+/* Not implemented yet. */
+goto gp_fault;
+
 case MSR_INTEL_MISC_FEATURES_ENABLES:
 if ( !vp->misc_features_enables.available )
 goto gp_fault;
@@ -153,14 +168,44 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t 
val)
 {
 const struct vcpu *curr = current;
 struct domain *d = v->domain;
+const struct cpuid_policy *cp = d->arch.cpuid;
 struct msr_domain_policy *dp = d->arch.msr;
 struct msr_vcpu_policy *vp = v->arch.msr;
 
 switch ( msr )
 {
 case MSR_INTEL_PLATFORM_INFO:
+case MSR_ARCH_CAPABILITIES:
+/* Read-only */
 goto gp_fault;
 
+case MSR_SPEC_CTRL:
+if ( !cp->feat.ibrsb )
+goto gp_fault; /* MSR available? */
+
+/*
+ * Note: SPEC_CTRL_STIBP is specified as safe to use (i.e. ignored)
+ * when STIBP isn't enumerated in hardware.
+ */
+
+if ( val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP) )
+goto gp_fault; /* Rsvd bit set? */
+
+vp->spec_ctrl.raw = val;
+break;
+
+case MSR_PRED_CMD:
+if ( !cp->feat.ibrsb && !cp->extd.ibpb )
+goto gp_fault; /* MSR available? */
+
+/*
+ * The only defined behaviour is when writing PRED_CMD_IBPB.  In
+ * practice, real hardware accepts any value without faulting.
+ */
+if ( v == curr && (val & PRED_CMD_IBPB) )
+wrmsrl(MSR_PRED_CMD, PRED_CMD_IBPB);
+break;
+
 case MSR_INTEL_MISC_FEATURES_ENABLES:
 {
 uint64_t rsvd = ~0ull;
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index a0aacfa..23ad743 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -39,6 +39,8 @@
 #define MSR_PRED_CMD   0x0049
 #define PRED_CMD_IBPB  (_AC(1, ULL) << 0)
 
+#define MSR_ARCH_CAPABILITIES  0x010a
+
 /* Intel MSRs. Some also available on other CPUs */
 #define MSR_IA32_PERFCTR0  0x00c1
 #define MSR_IA32_A_PERFCTR00x04c1
diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
index 2fbed02..928f1cc 100644
--- a/xen/include/asm-x86/msr.h
+++ b/xen/include/asm-x86/msr.h
@@ -223,6 +223,16 @@ struct msr_domain_policy
 /* MSR policy object for per-vCPU MSRs */
 struct msr_vcpu_policy
 {
+/* 0x0048 - MSR_SPEC_CTRL */
+struct {
+/*
+ * Only the bottom two bits are defined, so no need to waste space
+ * with uint64_t at the moment, but use uint32_t for the convenience
+ * of the assembly code.
+ */
+uint32_t raw;
+} spec_ctrl;
+
 /* 0x0140  MSR_INTEL_MISC_FEATURES_ENABLES */
 struct {
 bool available; /* This MSR is non-architectural */
-- 
2.1.4


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

[Xen-devel] [PATCH v9 00/11] x86: Mitigations for SP2/CVE-2017-5715/Branch Target Injection

[Andrew Cooper]

This series is availabe in git form from:

  
http://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=shortlog;h=refs/heads/sp2-mitigations-v9

A copy of Intel's spec for IBRS/IBPB can be found here:

  
https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf

In addition to this software series, you will need the following:

  1) A compiler which understands -mindirect-branch=thunk-external and
 -mindirect-branch-register.  A GCC patch series implementing this should
 be available imminently.  In the meantime, a development branch can be
 obtained from:

 https://github.com/hjl-tools/gcc/commits/hjl/indirect/gcc-7-branch/master

  2) New microcode from Intel and AMD.  These provide new MSRs for Xen to use,
 and virtualise for guest kernels to use.

There are some limitations, even with the work presented here.

  1) vCPU-to-vCPU SP2 attacks can only be mitigated at the hypervisor level
 with IBPB support, which for internal pipeline reasons, we do not expect
 to be made available on older processors.  For now, I will leave these
 details to the hardware vendors.

  2) Hardware lacking SMEP is in a worse position than hardware with SMEP.  If
 you have SMEP (Intel IvyBridge and later, Some AMD Fam16h and all Fam17h
 and later), make absolutely sure it is enabled in the BIOS and working.

  3) On hardware lacking SMEP support, it is still an open question how to
 protect against RSB-to-SMM speculation.  Native operating systems can fix
 this by prohibiting userspace from mmap()'ing addresses which alias the
 SMM range, but Xen has no feasible way of enforcing this restriction on
 PV guests, even if we could tolerate the ABI breakage.  (However, see the
 forthcoming SP3 mitigation series for alternatives for un trusted PV
 guests).

~Andrew

Changes from v8:
  * Large rework of STIBP handling following Intel publishing a new spec
  * Rebase over the XPTI patches

Andrew Cooper (11):
  x86/thunk: Fix GEN_INDIRECT_THUNK comment
  x86/cpuid: Handling of IBRS/IBPB, STIBP and IBRS for guests
  x86/msr: Emulation of MSR_{SPEC_CTRL,PRED_CMD} for guests
  x86/migrate: Move MSR_SPEC_CTRL on migrate
  x86/hvm: Permit guests direct access to MSR_{SPEC_CTRL,PRED_CMD}
  x86/entry: Organise the use of MSR_SPEC_CTRL at each entry/exit point
  x86/boot: Calculate the most appropriate BTI mitigation to use
  x86/entry: Clobber the Return Stack Buffer/Return Address Stack on entry to 
Xen
  x86/ctxt: Issue a speculation barrier between vcpu contexts
  x86/cpuid: Offer Indirect Branch Controls to guests
  x86/idle: Clear SPEC_CTRL while idle

 docs/misc/xen-command-line.markdown |  13 +-
 tools/libxc/xc_cpuid_x86.c  |   4 +-
 xen/arch/x86/acpi/cpu_idle.c|  21 +++
 xen/arch/x86/cpu/mwait-idle.c   |   7 +
 xen/arch/x86/cpuid.c|  28 +++
 xen/arch/x86/domain.c   |  31 
 xen/arch/x86/domctl.c   |  21 +++
 xen/arch/x86/hvm/hvm.c  |   2 +
 xen/arch/x86/hvm/svm/entry.S|   8 +-
 xen/arch/x86/hvm/svm/svm.c  |   5 +
 xen/arch/x86/hvm/vmx/entry.S|  11 ++
 xen/arch/x86/hvm/vmx/vmx.c  |  17 ++
 xen/arch/x86/indirect-thunk.S   |   2 +-
 xen/arch/x86/msr.c  |  45 +
 xen/arch/x86/setup.c|   1 +
 xen/arch/x86/smpboot.c  |   2 +
 xen/arch/x86/spec_ctrl.c| 142 ++-
 xen/arch/x86/x86_64/asm-offsets.c   |   6 +
 xen/arch/x86/x86_64/compat/entry.S  |  14 ++
 xen/arch/x86/x86_64/entry.S |  34 
 xen/include/asm-x86/asm_defns.h |   3 +
 xen/include/asm-x86/cpufeatures.h   |   2 +
 xen/include/asm-x86/current.h   |   6 +
 xen/include/asm-x86/msr-index.h |   2 +
 xen/include/asm-x86/msr.h   |  10 ++
 xen/include/asm-x86/nops.h  |   7 +
 xen/include/asm-x86/spec_ctrl.h |  45 +
 xen/include/asm-x86/spec_ctrl_asm.h | 267 
 xen/include/public/arch-x86/cpufeatureset.h |   6 +-
 29 files changed, 751 insertions(+), 11 deletions(-)
 create mode 100644 xen/include/asm-x86/spec_ctrl_asm.h

-- 
2.1.4


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

[Xen-devel] [PATCH] x86: slightly reduce Meltdown band-aid overhead

[Jan Beulich]

I'm not sure why I didn't do this right away: By avoiding to make any
of the cloned directmap PTEs global, there's no need to fiddle with
CR4.PGE on any of the entry paths. Only the exit paths need to flush
global mappings.

The reduced flushing, however, implies that we now need to have
interrupts off on all entry paths until after the page table switch, so
that flush IPIs can't arrive with the restricted page tables still
active, but only a non-global flush happening with the CR3 loads. Along
those lines the "sync" IPI after L4 entry updates now needs to become a
real (and global) flush IPI, so that inside Xen we'll also pick up such
changes.

Take the opportunity and also do a GET_CURRENT() -> __GET_CURRENT()
transition the original patch missed.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3804,18 +3804,14 @@ long do_mmu_update(
 {
 /*
  * Force other vCPU-s of the affected guest to pick up L4 entry
- * changes (if any). Issue a flush IPI with empty operation mask to
- * facilitate this (including ourselves waiting for the IPI to
- * actually have arrived). Utilize the fact that FLUSH_VA_VALID is
- * meaningless without FLUSH_CACHE, but will allow to pass the no-op
- * check in flush_area_mask().
+ * changes (if any).
  */
 unsigned int cpu = smp_processor_id();
 cpumask_t *mask = per_cpu(scratch_cpumask, cpu);
 
 cpumask_andnot(mask, pt_owner->domain_dirty_cpumask, cpumask_of(cpu));
 if ( !cpumask_empty(mask) )
-flush_area_mask(mask, ZERO_BLOCK_PTR, FLUSH_VA_VALID);
+flush_mask(mask, FLUSH_TLB_GLOBAL);
 }
 
 perfc_add(num_page_updates, i);
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -726,6 +726,7 @@ static int clone_mapping(const void *ptr
 }
 
 pl1e += l1_table_offset(linear);
+flags &= ~_PAGE_GLOBAL;
 
 if ( l1e_get_flags(*pl1e) & _PAGE_PRESENT )
 {
@@ -1009,8 +1010,17 @@ void __init smp_prepare_cpus(unsigned in
 if ( rc )
 panic("Error %d setting up PV root page table\n", rc);
 if ( per_cpu(root_pgt, 0) )
+{
 get_cpu_info()->pv_cr3 = __pa(per_cpu(root_pgt, 0));
 
+/*
+ * All entry points which may need to switch page tables have to start
+ * with interrupts off. Re-write what pv_trap_init() has put there.
+ */
+_set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_irq_gate, 3,
+  _direct_trap);
+}
+
 set_nr_sockets();
 
 socket_cpumask = xzalloc_array(cpumask_t *, nr_sockets);
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -189,7 +189,7 @@ ENTRY(compat_post_handle_exception)
 
 /* See lstar_enter for entry register state. */
 ENTRY(cstar_enter)
-sti
+/* sti could live here when we don't switch page tables below. */
 CR4_PV32_RESTORE
 movq  8(%rsp),%rax /* Restore %rax. */
 movq  $FLAT_KERNEL_SS,8(%rsp)
@@ -206,11 +206,12 @@ ENTRY(cstar_enter)
 jz.Lcstar_cr3_okay
 mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 neg   %rcx
-write_cr3 rcx, rdi, rsi
+mov   %rcx, %cr3
 movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lcstar_cr3_okay:
+sti
 
-GET_CURRENT(bx)
+__GET_CURRENT(bx)
 movq  VCPU_domain(%rbx),%rcx
 cmpb  $0,DOMAIN_is_32bit_pv(%rcx)
 jeswitch_to_kernel
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -135,7 +135,7 @@ UNLIKELY_END(exit_cr3)
  * %ss must be saved into the space left by the trampoline.
  */
 ENTRY(lstar_enter)
-sti
+/* sti could live here when we don't switch page tables below. */
 movq  8(%rsp),%rax /* Restore %rax. */
 movq  $FLAT_KERNEL_SS,8(%rsp)
 pushq %r11
@@ -151,9 +151,10 @@ ENTRY(lstar_enter)
 jz.Llstar_cr3_okay
 mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 neg   %rcx
-write_cr3 rcx, rdi, rsi
+mov   %rcx, %cr3
 movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Llstar_cr3_okay:
+sti
 
 __GET_CURRENT(bx)
 testb $TF_kernel_mode,VCPU_thread_flags(%rbx)
@@ -236,7 +237,7 @@ process_trap:
 jmp  test_all_events
 
 ENTRY(sysenter_entry)
-sti
+/* sti could live here when we don't switch page tables below. */
 pushq $FLAT_USER_SS
 pushq $0
 pushfq
@@ -254,9 +255,10 @@ GLOBAL(sysenter_eflags_saved)
 jz.Lsyse_cr3_okay
 mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 neg   %rcx
-write_cr3 rcx, rdi, rsi
+mov   %rcx, %cr3
 movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lsyse_cr3_okay:
+sti
 
 __GET_CURRENT(bx)
 cmpb  $0,VCPU_sysenter_disables_events(%rbx)
@@ -300,9 +302,10 @@ ENTRY(int80_direct_trap)
 jz.Lint80_cr3_okay
 

[Xen-devel] [PATCH v2] x86/hvm: re-work viridian APIC assist code

[Paul Durrant]

It appears there is a case where Windows enables the APIC assist
enlightenment[1] but does not use it. This scenario is perfectly valid
according to the documentation, but causes the state machine in Xen to
become confused leading to a domain_crash() such as the following:

(XEN) d4: VIRIDIAN GUEST_OS_ID: vendor: 1 os: 4 major: 6 minor: 1 sp: 0
  build: 1db0
(XEN) d4: VIRIDIAN HYPERCALL: enabled: 1 pfn: 3
(XEN) d4v0: VIRIDIAN VP_ASSIST_PAGE: enabled: 1 pfn: 3fffe
(XEN) domain_crash called from viridian.c:452
(XEN) Domain 4 (vcpu#0) crashed on cpu#1:

The following sequence of events is an example of how this can happen:

 - On return to guest vlapic_has_pending_irq() finds a bit set in the IRR.
 - vlapic_ack_pending_irq() calls viridian_start_apic_assist() which latches
   the vector, sets the bit in the ISR and clears it from the IRR.
 - The guest then processes the interrupt but EOIs it normally, therefore
   clearing the bit in the ISR.
 - On next return to guest vlapic_has_pending_irq() calls
   viridian_complete_apic_assist(), which discovers the assist bit still set
   in the shared page and therefore leaves the latched vector in place, but
   also finds another bit set in the IRR.
 - vlapic_ack_pending_irq() is then called but, because the ISR is was
   cleared by the EOI, another call is made to viridian_start_apic_assist()
   and this then calls domain_crash() because it finds the latched vector
   has not been cleared.

Having re-visited the code I also conclude that Xen's implementation of the
enlightenment is currently wrong and we are not properly following the
specification.

The specification says:

"The hypervisor sets the “No EOI required” bit when it injects a virtual
 interrupt if the following conditions are satisfied:

 - The virtual interrupt is edge-triggered, and
 - There are no lower priority interrupts pending.

 If, at a later time, a lower priority interrupt is requested, the
 hypervisor clears the “No EOI required” such that a subsequent EOI causes
 an intercept.
 In case of nested interrupts, the EOI intercept is avoided only for the
 highest priority interrupt. This is necessary since no count is maintained
 for the number of EOIs performed by the OS. Therefore only the first EOI
 can be avoided and since the first EOI clears the “No EOI Required” bit,
 the next EOI generates an intercept."

Thus it is quite legitimate to set the "No EOI required" bit and then
subsequently take a higher priority interrupt without clearing the bit.
Thus the avoided EOI will then relate to that subsequent interrupt rather
than the highest priority interrupt when the bit was set. Hence latching
the vector when setting the bit is not entirely useful and somewhat
misleading.

This patch re-works the APIC assist code to simply track when the "No EOI
required" bit is set and test if it has been cleared by the guest (i.e.
'completing' the APIC assist), thus indicating a 'missed EOI'. Missed EOIs
need to be dealt with in two places:

 - In vlapic_has_pending_irq(), to avoid comparing the IRR against a stale
   ISR, and
 - In vlapic_EOI_set() because a missed EOI for a higher priority vector
   should be dealt with before the actual EOI for the lower priority
   vector.

Furthermore, because the guest is at liberty to ignore the "No EOI required"
bit (which lead the crash detailed above) vlapic_EOI_set() must also make
sure the bit is cleared to avoid confusing the state machine.

Lastly the previous code did not properly emulate an EOI if a missed EOI
was discovered in vlapic_has_pending_irq(); it merely cleared the bit in
the ISR. The new code instead calls vlapic_EOI_set().

[1] See section 10.3.5 of Microsoft's "Hypervisor Top Level Functional
Specification v5.0b".

NOTE: The changes to the save/restore code are safe because the layout
  of struct hvm_viridian_vcpu_context is unchanged and the new
  interpretation of the (previously so named) vp_assist_vector field
  as the boolean pending flag maintains the correct semantics.

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

v2:
 - Extend patch to completely re-work APIC assist code
---
 xen/arch/x86/hvm/viridian.c| 36 
 xen/arch/x86/hvm/vlapic.c  | 75 --
 xen/include/asm-x86/hvm/viridian.h |  8 ++--
 xen/include/public/arch-x86/hvm/save.h |  2 +-
 4 files changed, 76 insertions(+), 45 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index f0fa59d7d5..cc40af1fb5 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -433,46 +433,44 @@ static void teardown_vp_assist(struct vcpu *v)
 put_page_and_type(page);
 }
 
-void viridian_start_apic_assist(struct vcpu *v, int vector)
+void viridian_set_apic_assist(struct vcpu *v)
 {
 uint32_t *va = v->arch.hvm_vcpu.viridian.vp_assist.va;
 
 if ( !va )

Re: [Xen-devel] [PATCH v2 07/13] iommu: Make decision about needing IOMMU for hardware domains in advance

[Oleksandr Tyshchenko]

Hi, Roger

On Thu, Jan 18, 2018 at 2:09 PM, Roger Pau Monné  wrote:
> Sorry for the delay in the reply.
No problem.

>
> On Tue, Jul 25, 2017 at 08:26:49PM +0300, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko 
>>
>> The hardware domains require IOMMU to be used in the most cases and
>> a decision to use it is made at hardware domain construction time.
>> But, it is not the best moment for the non-shared IOMMUs due to
>> the necessity of retrieving all mapping which could happen in a period
>> of time between IOMMU per-domain initialization and this moment.
>>
>> So, make a decision about needing IOMMU a bit earlier, in 
>> iommu_domain_init().
>> Having "d->need_iommu" flag set at the early stage we won't skip
>> any IOMMU mapping updates. And as the result the existing in 
>> iommu_hwdom_init()
>> code that goes through the list of the pages and tries to retrieve mapping
>> for non-shared IOMMUs won't be needed anymore and can be just dropped.
>
> If I understand this correctly the approach looks fine to me, and it's
> inline with what I'm doing for PVHv2 Dom0. Ie: the IOMMU is
> initialized _before_ populating the memory map, so that when pages are
> added to the p2m they are also added to the IOMMU page tables if
> required.
>
> This avoids having to iterate over the list of domain pages in
> iommu_hwdom_init, because it's empty at the point iommu_hwdom_init is
> called for a PVHv2 Dom0.
>
>> Signed-off-by: Oleksandr Tyshchenko 
>> CC: Jan Beulich 
>> CC: Julien Grall 
>>
>> ---
>> Changes in v1:
>>-
>>
>> Changes in v2:
>>- This is the result of reworking old patch:
>>  [PATCH v1 08/10] iommu: Split iommu_hwdom_init() into arch specific 
>> parts
>> ---
>>  xen/drivers/passthrough/iommu.c | 44 
>> ++---
>>  1 file changed, 10 insertions(+), 34 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/iommu.c 
>> b/xen/drivers/passthrough/iommu.c
>> index 19c87d1..f5e5b7e 100644
>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -52,7 +52,7 @@ custom_param("iommu", parse_iommu_param);
>>  bool_t __initdata iommu_enable = 1;
>>  bool_t __read_mostly iommu_enabled;
>>  bool_t __read_mostly force_iommu;
>> -bool_t __hwdom_initdata iommu_dom0_strict;
>> +bool_t __read_mostly iommu_dom0_strict;
>>  bool_t __read_mostly iommu_verbose;
>>  bool_t __read_mostly iommu_workaround_bios_bug;
>>  bool_t __read_mostly iommu_igfx = 1;
>> @@ -141,6 +141,15 @@ int iommu_domain_init(struct domain *d, bool use_iommu)
>>  if ( !iommu_enabled )
>>  return 0;
>>
>> +if ( is_hardware_domain(d) )
>> +{
>> +if ( (paging_mode_translate(d) && !iommu_passthrough) ||
>> +  iommu_dom0_strict )
>> +use_iommu = 1;
>> +else
>> +use_iommu = 0;
>> +}
>> +
>>  hd->platform_ops = iommu_get_ops();
>>  ret = hd->platform_ops->init(d, use_iommu);
>>  if ( ret )
>> @@ -161,8 +170,6 @@ static void __hwdom_init check_hwdom_reqs(struct domain 
>> *d)
>>  if ( iommu_passthrough )
>>  panic("Dom0 uses paging translated mode, dom0-passthrough must not 
>> be "
>>"enabled\n");
>> -
>> -iommu_dom0_strict = 1;
>>  }
>>
>>  void __hwdom_init iommu_hwdom_init(struct domain *d)
>> @@ -175,37 +182,6 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>>  return;
>>
>>  register_keyhandler('o', _dump_p2m_table, "dump iommu p2m table", 
>> 0);
>> -d->need_iommu = !!iommu_dom0_strict;
>
> Where is this set now? You seem to remove setting need_iommu here, but
> I don't see it being set anywhere else. Am I missing something?

d->need_iommu is set in iommu_domain_init(). This was done by previous
patch [1].
For your convenience you can see how the whole function looks like [2].

[1]
[PATCH v2 06/13] iommu: Add extra use_iommu argument to iommu_domain_init()
https://marc.info/?l=xen-devel=150100368126600=2

[2] 
https://github.com/otyshchenko1/xen/blob/non_shared_iommu_v2/xen/drivers/passthrough/iommu.c#L158

>
> Thanks, Roger.

-- 
Regards,

Oleksandr Tyshchenko

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

Re: [Xen-devel] [PATCH v4 2/4] x86emul: Support vpclmulqdq

[Jan Beulich]

>>> On 03.01.18 at 09:26,  wrote:
> The previous vpclmulqdq only support AVX128.
> Icelake added AVX256 support.

And 512-bit support as well; you only don't add support for the EVEX
encoding forms in your patch.

The patch itself looks fine, but please clarify its testing status.

> @@ -6168,6 +6169,7 @@ x86_emulate(
>  simd_0f_imm8_avx:
>  host_and_vcpu_must_have(avx);
>  }
> +simd_0f_imm8_ymm:
>  get_fpu(X86EMUL_FPU_ymm, );
>  }
>  else if ( vex.pfx )

One remark as I see this: Personally I don't see any of this series go
in ahead of my larger emulator series (or at least large parts of it),
considering that it's been pending for much longer. I'm sorry for this
meaning that you also won't be able to report this task as completed
any time soon. Other maintainers may have a different opinion here,
though.

Jan


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

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 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
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  380c152033d2a58fe7dc43bf26c3c8b56bfa5d43
baseline version:
 xen  e871e80c38547d9faefc6604532ba3e985e65873

Last test of basis   118105  2018-01-16 17:04:09 Z1 days
Failing since118110  2018-01-16 19:02:12 Z1 days   20 attempts
Testing same since   118210  2018-01-18 12:01:06 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anthony Liguori 
  Bob Moore 
  David Scott 
  Doug Goldstein 
  George Dunlap 
  Ian Jackson 
  Jan Beulich 
  Jon Ludlam 
  Jonathan Ludlam 
  Juergen Gross 
  Julien Grall 
  Lv Zheng 
  Michael Young 
  Rafael J. Wysocki 
  Roger Pau Monne 
  Roger Pau Monné 
  Sergey Dyasli 
  Stefano Stabellini 
  Wei Liu 
  Yi Sun 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   e871e80c38..380c152033  380c152033d2a58fe7dc43bf26c3c8b56bfa5d43 -> smoke

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

[Xen-devel] AMD / SVM vm_event support

[Razvan Cojocaru]

Hello,

We're looking at potentially working with vm_event on SVM hosts. To that
end, we've removed a few cpu_has_vmx tests and saw that some things just
work.

That is, unfortunately, not the case for EPT events.

There's this old thread about NPT support:

https://lists.gt.net/xen/devel/339571

but it looks like nothing came of it.

Has there been more Xen development work, or at least discussion on this
topic that we are not aware of?

As for the things that do work, any objections to submitting patches
that allows an introspection application to subscribe to what's
available on SVM as well, with the proper capabilities set in
arch_monitor_get_capabilities()?


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH v2 07/13] iommu: Make decision about needing IOMMU for hardware domains in advance

[Roger Pau Monné]

Sorry for the delay in the reply.

On Tue, Jul 25, 2017 at 08:26:49PM +0300, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko 
> 
> The hardware domains require IOMMU to be used in the most cases and
> a decision to use it is made at hardware domain construction time.
> But, it is not the best moment for the non-shared IOMMUs due to
> the necessity of retrieving all mapping which could happen in a period
> of time between IOMMU per-domain initialization and this moment.
> 
> So, make a decision about needing IOMMU a bit earlier, in iommu_domain_init().
> Having "d->need_iommu" flag set at the early stage we won't skip
> any IOMMU mapping updates. And as the result the existing in 
> iommu_hwdom_init()
> code that goes through the list of the pages and tries to retrieve mapping
> for non-shared IOMMUs won't be needed anymore and can be just dropped.

If I understand this correctly the approach looks fine to me, and it's
inline with what I'm doing for PVHv2 Dom0. Ie: the IOMMU is
initialized _before_ populating the memory map, so that when pages are
added to the p2m they are also added to the IOMMU page tables if
required.

This avoids having to iterate over the list of domain pages in
iommu_hwdom_init, because it's empty at the point iommu_hwdom_init is
called for a PVHv2 Dom0.

> Signed-off-by: Oleksandr Tyshchenko 
> CC: Jan Beulich 
> CC: Julien Grall 
> 
> ---
> Changes in v1:
>-
> 
> Changes in v2:
>- This is the result of reworking old patch:
>  [PATCH v1 08/10] iommu: Split iommu_hwdom_init() into arch specific parts
> ---
>  xen/drivers/passthrough/iommu.c | 44 
> ++---
>  1 file changed, 10 insertions(+), 34 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index 19c87d1..f5e5b7e 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -52,7 +52,7 @@ custom_param("iommu", parse_iommu_param);
>  bool_t __initdata iommu_enable = 1;
>  bool_t __read_mostly iommu_enabled;
>  bool_t __read_mostly force_iommu;
> -bool_t __hwdom_initdata iommu_dom0_strict;
> +bool_t __read_mostly iommu_dom0_strict;
>  bool_t __read_mostly iommu_verbose;
>  bool_t __read_mostly iommu_workaround_bios_bug;
>  bool_t __read_mostly iommu_igfx = 1;
> @@ -141,6 +141,15 @@ int iommu_domain_init(struct domain *d, bool use_iommu)
>  if ( !iommu_enabled )
>  return 0;
>  
> +if ( is_hardware_domain(d) )
> +{
> +if ( (paging_mode_translate(d) && !iommu_passthrough) ||
> +  iommu_dom0_strict )
> +use_iommu = 1;
> +else
> +use_iommu = 0;
> +}
> +
>  hd->platform_ops = iommu_get_ops();
>  ret = hd->platform_ops->init(d, use_iommu);
>  if ( ret )
> @@ -161,8 +170,6 @@ static void __hwdom_init check_hwdom_reqs(struct domain 
> *d)
>  if ( iommu_passthrough )
>  panic("Dom0 uses paging translated mode, dom0-passthrough must not 
> be "
>"enabled\n");
> -
> -iommu_dom0_strict = 1;
>  }
>  
>  void __hwdom_init iommu_hwdom_init(struct domain *d)
> @@ -175,37 +182,6 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>  return;
>  
>  register_keyhandler('o', _dump_p2m_table, "dump iommu p2m table", 
> 0);
> -d->need_iommu = !!iommu_dom0_strict;

Where is this set now? You seem to remove setting need_iommu here, but
I don't see it being set anywhere else. Am I missing something?

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v4] kexec-tools: Perform run-time linking of libxenctrl.so

[Daniel Kiper]

On Wed, Jan 17, 2018 at 10:39:01AM -0600, Eric DeVolder wrote:
> When kexec is utilized in a Xen environment, it has an explicit
> run-time dependency on libxenctrl.so. This dependency occurs
> during the configure stage and when building kexec-tools.

[...]

As I saw Simon applied the patch. Great! Sadly I have just
realized that I have missed two things. Please look below.

> The patch creates a wrapper function around xc_interface_open()
> and xc_interface_close() to perform the dlopen() and dlclose().

You say about dlclose() here...

diff --git a/kexec/kexec-xen.h b/kexec/kexec-xen.h
new file mode 100644
index 000..ffb8743
--- /dev/null
+++ b/kexec/kexec-xen.h

[...]

> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
> +extern void *xc_dlhandle;

...and here but it is never called. Is it by design or by mistake?
If by design please add a comment saying why into relevant place,
e.g. __xc_interface_close()? If by mistake please fix it.

[...]

> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
> +extern void *xc_dlhandle;

I am still not happy with xc_dlhandle being extern. IMO it should be static.
I have missed that during last review. Sorry about that. I have a feeling
that you can easily fix it.

Define __xc_dlsym() in kexec/kexec-xen.c just before __xc_interface_open() like 
below:

void *__xc_dlsym(const char *symbol)
{
  return dlsym(xc_dlhandle, symbol);
}

> +/* Wrappers around xc_interface_open/close() to insert dlopen/dlclose() */
> +xc_interface *__xc_interface_open(xentoollog_logger *logger,
> +   xentoollog_logger *dombuild_logger,
> +   unsigned open_flags);
> +int __xc_interface_close(xc_interface *xch);
> +
> +/* GCC expression statements for evaluating dlsym() */
> +#define __xc_call(dtype, name, args...) \
> +( \
> + { dtype value; \
> + typedef dtype (*func_t)(xc_interface *, ...); \
> + func_t func = dlsym(xc_dlhandle, #name); \

And then replace dlsym(xc_dlhandle, #name) with __xc_dlsym(#name)...

> + value = func(args); \
> + value; } \
> +)
> +#define __xc_data(dtype, name) \
> +( \
> + { dtype *value = (dtype *)dlsym(xc_dlhandle, #name); value; } \

...and here. Additionally, you can use __xc_dlsym() instead of dlsym()
in other places in your patch. Latter is not a must...

Daniel

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

Re: [Xen-devel] [PATCH 4/5] xen/arm64: Add skeleton to harden the branch predictor aliasing attacks

[Julien Grall]

Hi Stefano,

On 17/01/18 18:26, Stefano Stabellini wrote:

On Tue, 16 Jan 2018, Julien Grall wrote:

diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 7de68361ff..23ebf367ea 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -1,6 +1,7 @@
  #ifndef __ARM_CPUERRATA_H__
  #define __ARM_CPUERRATA_H__
  
+#include 


This doesn't seem to be necessary?


You are right. My original plan was based on Linux which use per-cpu 
variable. But I scratched that as it was possible to simplify the code.


I will send a patch to remove it.

Cheers,

--
Julien Grall

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

[Xen-devel] [xen-unstable-smoke test] 118206: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  e730f8e41e8537f1db9770b9464f9523c28857b9
baseline version:
 xen  e871e80c38547d9faefc6604532ba3e985e65873

Last test of basis   118105  2018-01-16 17:04:09 Z1 days
Failing since118110  2018-01-16 19:02:12 Z1 days   19 attempts
Testing same since   118194  2018-01-18 00:01:12 Z0 days5 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anthony Liguori 
  Bob Moore 
  George Dunlap 
  Ian Jackson 
  Jan Beulich 
  Jon Ludlam 
  Jonathan Ludlam 
  Julien Grall 
  Lv Zheng 
  Rafael J. Wysocki 
  Roger Pau Monne 
  Roger Pau Monné 
  Sergey Dyasli 
  Stefano Stabellini 
  Wei Liu 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 test-arm64-arm64-xl-xsm  pass
 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


Not pushing.

(No revision log; it would be 1042 lines long.)

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

Re: [Xen-devel] [PATCH v10 00/11] Enable Memory Bandwidth Allocation in Xen

[Wei Liu]

On Thu, Jan 18, 2018 at 11:31:51AM +, Wei Liu wrote:
> On Thu, Jan 18, 2018 at 11:10:50AM +, Wei Liu wrote:
> > On Tue, Jan 09, 2018 at 08:31:52AM +0800, Yi Sun wrote:
> > > On 18-01-08 12:25:02, Wei Liu wrote:
> > > > On Mon, Jan 08, 2018 at 12:28:58PM +0800, Yi Sun wrote:
> > > > > On 17-12-20 02:11:32, Jan Beulich wrote:
> > > > > > >>> On 19.12.17 at 01:42,  wrote:
> > > > > > > We plan to bring a new PSR (Platform Shared Resource) feature 
> > > > > > > called
> > > > > > > Intel Memory Bandwidth Allocation (MBA) to Xen.
> > > > > > > 
> > > > > > > Besides the MBA enabling, we change some interfaces to make them 
> > > > > > > more
> > > > > > > general but not only for CAT.
> > > > > > > 
> > > > > > > The first 5 patches of V9 haven been merged. To fix a few issues, 
> > > > > > > the
> > > > > > > V10 is submitted.
> > > > > > 
> > > > > > I've committed the three hypervisor patches; I've not done anything
> > > > > > with the eight tools ones, as one of them needs to be (re-)acked. 
> > > > > > I'll
> > > > > > leave that part to one of you.
> > > > > > 
> > > > > > Jan
> > > > > 
> > > > > Hi, Ian and Wei,
> > > > > 
> > > > > Could you please help to check tools/ patches? Thanks!
> > > > > 
> > > > 
> > > > I just acked the one patch that missed an ack. I will try to get around
> > > > to commit them later this week.
> > > > 
> > > Thank you! :)
> > 
> > Hi Yi
> > 
> > I tried to apply patch 4 to 11 today but there were conflicts. Can you
> > rebase your series on top of staging?
> > 
> > Sorry for the inconvenience. We've mostly been dealing with security
> > issues and the tree seemed to have changed a bit with all the recent
> > fixes applied.
> > 
> 
> Actually, before you set off to do anything, let me try again. There
> could a problem with my email client. I will let you know the result.

Alright, it is done. Your patches are now in staging. Please let me know
if there is any problem.

Wei.

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

Re: [Xen-devel] [PATCH 4/5] x86/pv: Drop support for paging out the LDT

[Jan Beulich]

>>> On 18.01.18 at 12:00,  wrote:
> On 18/01/18 10:57, Andrew Cooper wrote:
>> On 18/01/18 10:38, George Dunlap wrote:
>>> On Thu, Jan 18, 2018 at 7:59 AM, Jan Beulich  wrote:
>>> On 12.01.18 at 19:37,  wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1942,11 +1942,8 @@ int domain_relinquish_resources(struct domain *d)
>  {
>  for_each_vcpu ( d, v )
>  {
> -/*
> - * Relinquish GDT mappings. No need for explicit 
> unmapping of
> - * the LDT as it automatically gets squashed with the 
> guest
> - * mappings.
> - */
> +/* Relinquish GDT/LDT mappings. */
> +pv_destroy_ldt(v);
>  pv_destroy_gdt(v);
 The domain is dead at this point, so the order doesn't matter much,
 but strictly speaking you should destroy the GDT before destroying
 the LDT (just like LDT _loads_ always need to come _after_ GDT
 adjustments).
>> By that logic, I've got it the correct way round (which is they way I
>> intended).  Allocation and freeing of the LDT is nested within the scope
>> of the GDT.

Ah, I see how I didn't properly express what I mean. The idea
behind the remark was that the GDT might still have a reference
to the LDT, which would become stale with the LDT gone. The
better thing to compare with would be construction of an LDT
versus insertion of the respective descriptor into the GDT.

 Everything else here looks fine, but the initial comment may need
 further discussion. For example we may want to consider a
 two-stage phasing out of the feature, with a couple of years in
 between: Make the functionality dependent upon a default-off
 command line option for the time being, and issue a bright warning
 when someone actually enables it (telling them to tell us).
>>> One of the problems we have is that people seem to wait for 2-3 years
>>> after a release has been made to start updating to it.  So we'd have
>>> to leave such a warning for probably 5 years minimum.
>> In almost any other case, I'd agree, and would be the first to suggest this.
>>
>> However, This patch is strictly necessary for a more complete XPTI,
>> which is how I stumbled onto the issue in my KAISER series to begin
>> with.  It is the only codepath where we ever poke at a remote critical
>> data structure, which is why
> 
> Sorry - sent too early.
> 
> ... which is why isolating the LDT in a per-cpu doesn't work in
> combination with this algorithm.

Right, in the context of that series I can see it likely becoming
unavoidable to remove this functionality (aiui it could be kept,
but would become more complicated). Not having heard back
on the incompleteness discussion on stage 1 yet, I can't really
conclude at this point whether we will actually _need_ (not
just want) this full series making things per-CPU (taken together
with there so far not being any promise of it to help recover
performance).

Jan


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

Re: [Xen-devel] [PATCH v10 00/11] Enable Memory Bandwidth Allocation in Xen

[Wei Liu]

On Thu, Jan 18, 2018 at 11:10:50AM +, Wei Liu wrote:
> On Tue, Jan 09, 2018 at 08:31:52AM +0800, Yi Sun wrote:
> > On 18-01-08 12:25:02, Wei Liu wrote:
> > > On Mon, Jan 08, 2018 at 12:28:58PM +0800, Yi Sun wrote:
> > > > On 17-12-20 02:11:32, Jan Beulich wrote:
> > > > > >>> On 19.12.17 at 01:42,  wrote:
> > > > > > We plan to bring a new PSR (Platform Shared Resource) feature called
> > > > > > Intel Memory Bandwidth Allocation (MBA) to Xen.
> > > > > > 
> > > > > > Besides the MBA enabling, we change some interfaces to make them 
> > > > > > more
> > > > > > general but not only for CAT.
> > > > > > 
> > > > > > The first 5 patches of V9 haven been merged. To fix a few issues, 
> > > > > > the
> > > > > > V10 is submitted.
> > > > > 
> > > > > I've committed the three hypervisor patches; I've not done anything
> > > > > with the eight tools ones, as one of them needs to be (re-)acked. I'll
> > > > > leave that part to one of you.
> > > > > 
> > > > > Jan
> > > > 
> > > > Hi, Ian and Wei,
> > > > 
> > > > Could you please help to check tools/ patches? Thanks!
> > > > 
> > > 
> > > I just acked the one patch that missed an ack. I will try to get around
> > > to commit them later this week.
> > > 
> > Thank you! :)
> 
> Hi Yi
> 
> I tried to apply patch 4 to 11 today but there were conflicts. Can you
> rebase your series on top of staging?
> 
> Sorry for the inconvenience. We've mostly been dealing with security
> issues and the tree seemed to have changed a bit with all the recent
> fixes applied.
> 

Actually, before you set off to do anything, let me try again. There
could a problem with my email client. I will let you know the result.

Wei.

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

Re: [Xen-devel] [PATCH v2 3/6] xen/pvshim: identity pin shim vCPUs to pCPUs

[Roger Pau Monné]

On Thu, Jan 18, 2018 at 04:22:11AM -0700, Jan Beulich wrote:
> >>> On 18.01.18 at 12:11,  wrote:
> > Jan Beulich writes ("Re: [PATCH v2 3/6] xen/pvshim: identity pin shim vCPUs 
> > to pCPUs"):
> >> On 17.01.18 at 17:29,  wrote:
> >> > Since VCPUOP_{up/down} already identity maps vCPU hotplug to pCPU
> >> > hotplug also identity pin the vCPUs to the pCPUs in the scheduler.
> >> > This prevents vCPU migration and should improve performance.
> >> > 
> >> > While there also use __cpumask_set_cpu instead of cpumask_set_cpu,
> >> > there's no need to use the locked variant.
> >> > 
> >> > Signed-off-by: Roger Pau Monné 
> >> > Reviewed-by: Wei Liu 
> >> 
> >> Acked-by: Jan Beulich 
> > 
> > Should this be in the security response comet branch ?
> 
> I believe so, but better ask Roger and Wei.

It should be in the 4.10 comment branch only (not the 4.8), since it's
a shim patch.

In any case, it's not a bug fix, just a performance improvement but
still nice to have IMHO.

Roger.

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

Re: [Xen-devel] [PATCH] x86/hvm: more sure APIC assist is aborted if guest EOIs APIC

[Paul Durrant]

> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> Of Paul Durrant
> Sent: 18 January 2018 10:37
> To: 'Jan Beulich' 
> Cc: Andrew Cooper ; xen-
> de...@lists.xenproject.org
> Subject: Re: [Xen-devel] [PATCH] x86/hvm: more sure APIC assist is aborted
> if guest EOIs APIC
> 
> > -Original Message-
> > From: Jan Beulich [mailto:jbeul...@suse.com]
> > Sent: 18 January 2018 10:17
> > To: Paul Durrant 
> > Cc: Andrew Cooper ; xen-
> > de...@lists.xenproject.org
> > Subject: RE: [PATCH] x86/hvm: more sure APIC assist is aborted if guest
> EOIs
> > APIC
> >
> > >>> On 18.01.18 at 10:55,  wrote:
> > > Ok. I'll re-work it so that abort is passed a vector and warns on 
> > > mismatch.
> >
> > You mean on the new path only, I suppose? The old path should
> > not trigger warnings (aiui it would always do so).
> 
> I think, in the existing abort case, the latched vector should always be the
> lowest pending bit in the ISR.
> 

Actually reading through the code and the spec. again, I'm not convinced by the 
logic even with the EOI fix. It looks to me like vlapic_has_pending_irq() could 
leave an existing assist in place and return a higher priority irr value. This 
guest would then service the higher priority interrupt, skip the EOI because 
the assist bit is set, but then the next call to vlapic_has_pending_irq() would 
erroneously clear the lower priority vector from the ISR because we originally 
latched the lower priority vector.
As the spec. points out... only the first EOI can avoided, thus I'm not sure 
trying to track the vector in the viridian code is worth it. It looks like the 
correct thing to do as test whether an EOI was avoided and then clear the 
highest priority vector in the ISR (since, if we hadn't squashed it, that would 
have been the first EOI). We can test when an EOI is avoided because the bit in 
the shared page would have transitioned from 1 to 0.

  Paul

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

Re: [Xen-devel] [PATCH 5/5] xen/arm64: Implement branch predictor hardening for affected Cortex-A CPUs

[Julien Grall]

Hi,

On 17/01/18 17:11, Stefano Stabellini wrote:

On Wed, 17 Jan 2018, Julien Grall wrote:

Hi Stefano,

On 17/01/18 00:42, Stefano Stabellini wrote:

On Tue, 16 Jan 2018, Julien Grall wrote:

   #define MIDR_RANGE(model, min, max) \
@@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[]
= {
  (1 << MIDR_VARIANT_SHIFT) | 2),
   },
   #endif
+#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
+{
+.capability = ARM_HARDEN_BRANCH_PREDICTOR,
+MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
+.enable = enable_psci_bp_hardening,
+},
+{
+.capability = ARM_HARDEN_BRANCH_PREDICTOR,
+MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
+.enable = enable_psci_bp_hardening,
+},
+{
+.capability = ARM_HARDEN_BRANCH_PREDICTOR,
+MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
+.enable = enable_psci_bp_hardening,
+},
+{
+.capability = ARM_HARDEN_BRANCH_PREDICTOR,
+MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
+.enable = enable_psci_bp_hardening,
+},


We need to add a basic description in the desc field as it is printed by
update_cpu_capabilities.


desc field is not mandatory, and in that case I think the print would be
confusing. At the difference of the other errata, we have more check in
install_bp_hardening_vec that may result to skip the hardening.

The errata here is covering all variant/revision of A75 models for safety
reason. Arm has introduced a new field ID_AA64PFR0_EL1.CSV2 to tell whether a
branch predictor trained in one context will affect speculative execution in
another context. This field is checked in install_bp_hardening_vec so you
avoid to harden the vector tables and small performance hit.

IHMO, it would be better to have a per-CPU message in install_bp_hardening_vec
announcing whether the vector tables has been harden and the kind of
hardening.

What do you think?


I understand what you are saying and I agree with you. Maybe we should
change update_cpu_capabilities to print "detected need for workaround:
blah" but you don't have to do it in this series.


"need for workaround..." is not entirely true for the branch predictor 
hardening. It is more a "may need". I still prefer the per-CPU message

"CPU%u will %s on guest exit"

%u is the CPU number. %s will be the name of the call/instruction to 
execute.


The rationale behind is branch predictor hardening may be different on 
each CPU (think big.LITTLE) so code executed will be different. This 
differ from the other errata that will be applied for all CPUs.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 6/7] firmware/shim: fix build process to use POSIX find options

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v3 6/7] firmware/shim: fix build process to use 
POSIX find options"):
> The -printf find option is not POSIX compatible, so replace it with
> another rune.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH v3 7/7] xen/pvshim: switch shim.c to use typesafe mfn_to_page and virt_to_mfn

[Wei Liu]

On Thu, Jan 18, 2018 at 11:02:52AM +, Roger Pau Monne wrote:
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné 
> Requested-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 3/6] xen/pvshim: identity pin shim vCPUs to pCPUs

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH v2 3/6] xen/pvshim: identity pin shim vCPUs to 
pCPUs"):
> On 17.01.18 at 17:29,  wrote:
> > Since VCPUOP_{up/down} already identity maps vCPU hotplug to pCPU
> > hotplug also identity pin the vCPUs to the pCPUs in the scheduler.
> > This prevents vCPU migration and should improve performance.
> > 
> > While there also use __cpumask_set_cpu instead of cpumask_set_cpu,
> > there's no need to use the locked variant.
> > 
> > Signed-off-by: Roger Pau Monné 
> > Reviewed-by: Wei Liu 
> 
> Acked-by: Jan Beulich 

Should this be in the security response comet branch ?

Ian.

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

Re: [Xen-devel] [PATCH 6/6] firmware/shim: fix build process to use POSIX find options

[Ian Jackson]

Roger Pau Monné writes ("Re: [Xen-devel] [PATCH 6/6] firmware/shim: fix build 
process to use POSIX find options"):
> What about using:
> 
> find $(XEN_ROOT)/$(d)/ -type d | sed 's,^$(XEN_ROOT)/$(d)/,,g' | xargs mkdir 
> -p
> 
> This AFAICT works fine, and should be the one involving less forks
> since the whole output is processed at once.

Much nicer, thanks.  That IMO makes it easy to see what's going on.

Ian.

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

Re: [Xen-devel] [PATCH v10 00/11] Enable Memory Bandwidth Allocation in Xen

[Wei Liu]

On Tue, Jan 09, 2018 at 08:31:52AM +0800, Yi Sun wrote:
> On 18-01-08 12:25:02, Wei Liu wrote:
> > On Mon, Jan 08, 2018 at 12:28:58PM +0800, Yi Sun wrote:
> > > On 17-12-20 02:11:32, Jan Beulich wrote:
> > > > >>> On 19.12.17 at 01:42,  wrote:
> > > > > We plan to bring a new PSR (Platform Shared Resource) feature called
> > > > > Intel Memory Bandwidth Allocation (MBA) to Xen.
> > > > > 
> > > > > Besides the MBA enabling, we change some interfaces to make them more
> > > > > general but not only for CAT.
> > > > > 
> > > > > The first 5 patches of V9 haven been merged. To fix a few issues, the
> > > > > V10 is submitted.
> > > > 
> > > > I've committed the three hypervisor patches; I've not done anything
> > > > with the eight tools ones, as one of them needs to be (re-)acked. I'll
> > > > leave that part to one of you.
> > > > 
> > > > Jan
> > > 
> > > Hi, Ian and Wei,
> > > 
> > > Could you please help to check tools/ patches? Thanks!
> > > 
> > 
> > I just acked the one patch that missed an ack. I will try to get around
> > to commit them later this week.
> > 
> Thank you! :)

Hi Yi

I tried to apply patch 4 to 11 today but there were conflicts. Can you
rebase your series on top of staging?

Sorry for the inconvenience. We've mostly been dealing with security
issues and the tree seemed to have changed a bit with all the recent
fixes applied.

Wei.

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

Re: [Xen-devel] [PATCH] ocaml: fix arm build

[Wei Liu]

On Thu, Jan 18, 2018 at 10:44:58AM +, Wei Liu wrote:
> On Wed, Jan 17, 2018 at 04:43:54PM +, Wei Liu wrote:
> > ARM doesn't have emulation_flags in the arch_domainconfig.
> > 
> > Signed-off-by: Wei Liu 
> > ---
> > Cc: Ian Jackson 
> > Cc: Julien Grall 
> > Cc: Andrew Cooper 
> > ---
> >  tools/ocaml/libs/xc/xenctrl_stubs.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c 
> > b/tools/ocaml/libs/xc/xenctrl_stubs.c
> > index 0b5a2361c0..fd128778b3 100644
> > --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> > +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> > @@ -175,11 +175,15 @@ CAMLprim value stub_xc_domain_create(value xch, value 
> > ssidref,
> > caml_failwith("Unhandled: ARM");
> > break;
> >  
> > +#if defined(__i386__) || defined(__x86_64__)
> > case 1: /* X86 - emulation flags in the block */
> 
> The label will be moved outside of the #ifdef.

In the interest of unblocking things I have pushed a version of this
patch to staging.

Wei.

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

[Xen-devel] [PATCH v3 4/7] xen/pvshim: re-order replace_va_mapping code

[Roger Pau Monne]

No functional change.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Wei Liu 
Acked-by: Jan Beulich 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
Changes since v1:
 - Fix double ; and space.
---
 xen/arch/x86/pv/shim.c | 21 ++---
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index 702249719e..aa5d416b75 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -117,21 +117,12 @@ uint64_t pv_shim_mem(uint64_t avail)
 static void __init replace_va_mapping(struct domain *d, l4_pgentry_t *l4start,
   unsigned long va, unsigned long mfn)
 {
-struct page_info *page;
-l4_pgentry_t *pl4e;
-l3_pgentry_t *pl3e;
-l2_pgentry_t *pl2e;
-l1_pgentry_t *pl1e;
-
-pl4e = l4start + l4_table_offset(va);
-pl3e = l4e_to_l3e(*pl4e);
-pl3e += l3_table_offset(va);
-pl2e = l3e_to_l2e(*pl3e);
-pl2e += l2_table_offset(va);
-pl1e = l2e_to_l1e(*pl2e);
-pl1e += l1_table_offset(va);
-
-page = mfn_to_page(l1e_get_pfn(*pl1e));
+l4_pgentry_t *pl4e = l4start + l4_table_offset(va);
+l3_pgentry_t *pl3e = l4e_to_l3e(*pl4e) + l3_table_offset(va);
+l2_pgentry_t *pl2e = l3e_to_l2e(*pl3e) + l2_table_offset(va);
+l1_pgentry_t *pl1e = l2e_to_l1e(*pl2e) + l1_table_offset(va);
+struct page_info *page = mfn_to_page(l1e_get_pfn(*pl1e));
+
 put_page_and_type(page);
 
 *pl1e = l1e_from_pfn(mfn, (!is_pv_32bit_domain(d) ? L1_PROT
-- 
2.15.1


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

Re: [Xen-devel] [PATCH v3 7/7] xen/pvshim: switch shim.c to use typesafe mfn_to_page and virt_to_mfn

[Andrew Cooper]

On 18/01/18 11:02, Roger Pau Monne wrote:
> No functional change intended.
>
> Signed-off-by: Roger Pau Monné 
> Requested-by: Andrew Cooper 
> ---
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Wei Liu 

Acked-by: Andrew Cooper 

Thanks.

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

[Xen-devel] [PATCH v3 5/7] xen/pvshim: fix coding style issues

[Roger Pau Monne]

Fix a couple of coding style issues.

No code or functional change.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Wei Liu 
Acked-by: Jan Beulich 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/arch/x86/pv/shim.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index aa5d416b75..903562e8e4 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -264,14 +264,14 @@ int pv_shim_shutdown(uint8_t reason)
_console_pfn));
 
 /* Pause the other vcpus before starting the migration. */
-for_each_vcpu(d, v)
+for_each_vcpu ( d, v )
 if ( v != current )
 vcpu_pause_by_systemcontroller(v);
 
 rc = xen_hypercall_shutdown(SHUTDOWN_suspend);
 if ( rc )
 {
-for_each_vcpu(d, v)
+for_each_vcpu ( d, v )
 if ( v != current )
 vcpu_unpause_by_systemcontroller(v);
 
@@ -347,7 +347,7 @@ int pv_shim_shutdown(uint8_t reason)
  */
 write_start_info(d);
 
-for_each_vcpu(d, v)
+for_each_vcpu ( d, v )
 {
 /* Unmap guest vcpu_info pages. */
 unmap_vcpu_info(v);
@@ -428,7 +428,7 @@ static long pv_shim_event_channel_op(int cmd, 
XEN_GUEST_HANDLE_PARAM(void) arg)
  */
 rc = xen_hypercall_event_channel_op(EVTCHNOP_alloc_unbound, );
 if ( rc )
-   break;
+break;
 
 /* Force L1 to use the event channel port allocated on L0. */
 rc = evtchn_bind_virq(, alloc.port);
@@ -477,7 +477,7 @@ static long pv_shim_event_channel_op(int cmd, 
XEN_GUEST_HANDLE_PARAM(void) arg)
 {
 rc = xen_hypercall_event_channel_op(EVTCHNOP_bind_vcpu, );
 if ( !rc )
- evtchn_assign_vcpu(d, vcpu.port, vcpu.vcpu);
+evtchn_assign_vcpu(d, vcpu.port, vcpu.vcpu);
 }
 
 break;
@@ -596,9 +596,9 @@ void pv_shim_inject_evtchn(unsigned int port)
 {
 if ( port_is_valid(guest, port) )
 {
- struct evtchn *chn = evtchn_from_port(guest, port);
+struct evtchn *chn = evtchn_from_port(guest, port);
 
- evtchn_port_set_pending(guest, chn->notify_vcpu_id, chn);
+evtchn_port_set_pending(guest, chn->notify_vcpu_id, chn);
 }
 }
 
@@ -633,7 +633,7 @@ static long pv_shim_grant_table_op(unsigned int cmd,
 }
 if ( compat )
 #define XLAT_gnttab_setup_table_HNDL_frame_list(d, s)
-XLAT_gnttab_setup_table(, );
+XLAT_gnttab_setup_table(, );
 #undef XLAT_gnttab_setup_table_HNDL_frame_list
 
 nat.status = GNTST_okay;
@@ -728,7 +728,7 @@ static long pv_shim_grant_table_op(unsigned int cmd,
 
 if ( compat )
 #define XLAT_gnttab_setup_table_HNDL_frame_list(d, s)
-XLAT_gnttab_setup_table(, );
+XLAT_gnttab_setup_table(, );
 #undef XLAT_gnttab_setup_table_HNDL_frame_list
 
 if ( unlikely(compat ? __copy_to_guest(uop, , 1)
-- 
2.15.1


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

[Xen-devel] [PATCH v3 3/7] xen/pvshim: identity pin shim vCPUs to pCPUs

[Roger Pau Monne]

Since VCPUOP_{up/down} already identity maps vCPU hotplug to pCPU
hotplug also identity pin the vCPUs to the pCPUs in the scheduler.
This prevents vCPU migration and should improve performance.

While there also use __cpumask_set_cpu instead of cpumask_set_cpu,
there's no need to use the locked variant.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Wei Liu 
Acked-by: Jan Beulich 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Ian Jackson 
---
Changes since v1:
 - Clarify commit message.
---
Should be backported to the 4.10.0-shim-comet branch.
---
 xen/arch/x86/dom0_build.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 17cb1272c1..555660b853 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -140,9 +140,8 @@ struct vcpu *__init dom0_setup_vcpu(struct domain *d,
 {
 if ( pv_shim )
 {
-
-cpumask_setall(v->cpu_hard_affinity);
-cpumask_setall(v->cpu_soft_affinity);
+__cpumask_set_cpu(vcpu_id, v->cpu_hard_affinity);
+__cpumask_set_cpu(vcpu_id, v->cpu_soft_affinity);
 }
 else
 {
-- 
2.15.1


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

[Xen-devel] [PATCH v3 7/7] xen/pvshim: switch shim.c to use typesafe mfn_to_page and virt_to_mfn

[Roger Pau Monne]

No functional change intended.

Signed-off-by: Roger Pau Monné 
Requested-by: Andrew Cooper 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
---
 xen/arch/x86/pv/shim.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index 903562e8e4..d5383dcfc7 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -37,6 +37,11 @@
 
 #include 
 
+#undef mfn_to_page
+#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn))
+#undef virt_to_mfn
+#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+
 #ifndef CONFIG_PV_SHIM_EXCLUSIVE
 bool pv_shim;
 boolean_param("pv-shim", pv_shim);
@@ -115,17 +120,17 @@ uint64_t pv_shim_mem(uint64_t avail)
 #define COMPAT_L1_PROT (_PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED)
 
 static void __init replace_va_mapping(struct domain *d, l4_pgentry_t *l4start,
-  unsigned long va, unsigned long mfn)
+  unsigned long va, mfn_t mfn)
 {
 l4_pgentry_t *pl4e = l4start + l4_table_offset(va);
 l3_pgentry_t *pl3e = l4e_to_l3e(*pl4e) + l3_table_offset(va);
 l2_pgentry_t *pl2e = l3e_to_l2e(*pl3e) + l2_table_offset(va);
 l1_pgentry_t *pl1e = l2e_to_l1e(*pl2e) + l1_table_offset(va);
-struct page_info *page = mfn_to_page(l1e_get_pfn(*pl1e));
+struct page_info *page = mfn_to_page(l1e_get_mfn(*pl1e));
 
 put_page_and_type(page);
 
-*pl1e = l1e_from_pfn(mfn, (!is_pv_32bit_domain(d) ? L1_PROT
+*pl1e = l1e_from_mfn(mfn, (!is_pv_32bit_domain(d) ? L1_PROT
   : COMPAT_L1_PROT));
 }
 
@@ -165,8 +170,9 @@ void __init pv_shim_setup_dom(struct domain *d, 
l4_pgentry_t *l4start,
 (si) = param;  
\
 if ( va )  
\
 {  
\
-share_xen_page_with_guest(mfn_to_page(param), d, XENSHARE_writable);   
\
-replace_va_mapping(d, l4start, va, param); 
\
+share_xen_page_with_guest(mfn_to_page(_mfn(param)), d, 
\
+  XENSHARE_writable);  
\
+replace_va_mapping(d, l4start, va, _mfn(param));   
\
 dom0_update_physmap(d, PFN_DOWN((va) - va_start), param, vphysmap);
\
 }  
\
 else   
\
@@ -186,17 +192,17 @@ void __init pv_shim_setup_dom(struct domain *d, 
l4_pgentry_t *l4start,
 {
 /* Allocate a new page for DomU's PV console */
 void *page = alloc_xenheap_pages(0, MEMF_bits(32));
-uint64_t console_mfn;
+mfn_t console_mfn;
 
 ASSERT(page);
 clear_page(page);
 console_mfn = virt_to_mfn(page);
-si->console.domU.mfn = console_mfn;
+si->console.domU.mfn = mfn_x(console_mfn);
 share_xen_page_with_guest(mfn_to_page(console_mfn), d,
   XENSHARE_writable);
 replace_va_mapping(d, l4start, console_va, console_mfn);
 dom0_update_physmap(d, (console_va - va_start) >> PAGE_SHIFT,
-console_mfn, vphysmap);
+mfn_x(console_mfn), vphysmap);
 consoled_set_ring_addr(page);
 }
 pv_hypercall_table_replace(__HYPERVISOR_event_channel_op,
@@ -232,7 +238,7 @@ static void write_start_info(struct domain *d)
 BUG_ON(xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_EVTCHN, ));
 si->console.domU.evtchn = param;
 if ( pv_console )
-si->console.domU.mfn = virt_to_mfn(consoled_get_ring_addr());
+si->console.domU.mfn = mfn_x(virt_to_mfn(consoled_get_ring_addr()));
 else if ( xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_PFN,
   >console.domU.mfn) )
 BUG();
@@ -334,7 +340,7 @@ int pv_shim_shutdown(uint8_t reason)
 if ( d->arch.pirq_eoi_map != NULL )
 {
 unmap_domain_page_global(d->arch.pirq_eoi_map);
-put_page_and_type(mfn_to_page(d->arch.pirq_eoi_map_mfn));
+put_page_and_type(mfn_to_page(_mfn(d->arch.pirq_eoi_map_mfn)));
 d->arch.pirq_eoi_map = NULL;
 d->arch.pirq_eoi_map_mfn = 0;
 d->arch.auto_unmask = 0;
-- 
2.15.1


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

[Xen-devel] [PATCH v3 6/7] firmware/shim: fix build process to use POSIX find options

[Roger Pau Monne]

The -printf find option is not POSIX compatible, so replace it with
another rune.

Signed-off-by: Roger Pau Monné 
---
Cc: Ian Jackson 
Cc: Wei Liu 
---
Changes since v1:
 - Drop the exec rune and instead process the whole output at once.
---
 tools/firmware/xen-dir/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/xen-dir/Makefile b/tools/firmware/xen-dir/Makefile
index adf6c31e8d..53eb3b6543 100644
--- a/tools/firmware/xen-dir/Makefile
+++ b/tools/firmware/xen-dir/Makefile
@@ -21,7 +21,8 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
$(foreach d, $(LINK_DIRS), \
 (mkdir -p $(D)/$(d); \
  cd $(D)/$(d); \
- find $(XEN_ROOT)/$(d)/ -type d -printf "./%P\n" |  xargs 
mkdir -p);)
+ find $(XEN_ROOT)/$(d)/ -type d |\
+   sed 's,^$(XEN_ROOT)/$(d)/,,g' | xargs mkdir -p);)
$(foreach d, $(LINK_DIRS), \
(cd $(XEN_ROOT); \
 find $(d) ! -type l -type f \
-- 
2.15.1


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

[Xen-devel] [PATCH v3 2/7] xen/pvh: place the trampoline starting at MFN 1

[Roger Pau Monne]

Since PVH guest jump straight into trampoline_setup trampoline_phys is
not initialized, thus the trampoline is relocated to address 0.

This works, but has the undesirable effect of having VA 0 mapped to
MFN 0, which means NULL pointed dereferences no longer trigger a page
fault.

In order to solve this, place the trampoline starting at MFN 1 and
reserve the memory used by it.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Wei Liu 
Acked-by: Jan Beulich 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Ian Jackson 
---
Changes since v2:
 - Clarify comments.

Changes since v1:
 - Expand comment in head.S.
 - Add a BUG_ON to check trampoline_phys value in the PVH case.
---
Should be backported to the 4.10.0-shim-comet branch.
---
 xen/arch/x86/boot/head.S | 3 +++
 xen/arch/x86/mm.c| 9 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 4fe5a776b1..0f652cea11 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -411,6 +411,9 @@ __pvh_start:
 /* Skip bootloader setup and bios setup, go straight to trampoline */
 movb$1, sym_esi(pvh_boot)
 movb$1, sym_esi(skip_realmode)
+
+/* Set trampoline_phys to use mfn 1 to avoid having a mapping at VA 0 
*/
+movw$0x1000, sym_esi(trampoline_phys)
 jmp trampoline_setup
 
 #endif /* CONFIG_PVH_GUEST */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 1147a1afb1..275e68c417 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -292,9 +292,14 @@ void __init arch_init_memory(void)
 /*
  * First 1MB of RAM is historically marked as I/O.  If we booted PVH,
  * reclaim the space.  Irrespective, leave MFN 0 as special for the sake
- * of 0 being a very common default value.
+ * of 0 being a very common default value. Also reserve the RAM needed by
+ * the trampoline on PVH starting at MFN 1.
  */
-for ( i = 0; i < (pvh_boot ? 1 : 0x100); i++ )
+BUG_ON(pvh_boot && trampoline_phys != 0x1000);
+for ( i = 0;
+  i < (pvh_boot ? (1 + PFN_UP(trampoline_end - trampoline_start))
+: 0x100);
+  i++ )
 share_xen_page_with_guest(mfn_to_page(_mfn(i)),
   dom_io, XENSHARE_writable);
 
-- 
2.15.1


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

[Xen-devel] [PATCH v3 1/7] xen/pvshim: map vcpu_info earlier for APs

[Roger Pau Monne]

Or else init_percpu_time is going to dereference a NULL pointer when
trying to access vcpu_info.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Wei Liu 
Acked-by: Jan Beulich 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Ian Jackson 
Cc: Wei Liu 
---
Should be backported to the 4.10.0-shim-comet branch.
---
 xen/arch/x86/smpboot.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 63ca053b35..2cdd431b5f 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -349,6 +349,9 @@ void start_secondary(void *unused)
 else
 microcode_resume_cpu(cpu);
 
+if ( xen_guest )
+hypervisor_ap_setup();
+
 smp_callin();
 
 init_percpu_time();
@@ -376,9 +379,6 @@ void start_secondary(void *unused)
 cpumask_set_cpu(cpu, _online_map);
 unlock_vector_lock();
 
-if ( xen_guest )
-hypervisor_ap_setup();
-
 /* We can take interrupts now: we're officially "up". */
 local_irq_enable();
 mtrr_ap_init();
-- 
2.15.1


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

[Xen-devel] [PATCH v3 0/7] xen/pvshim: fixes for staging

[Roger Pau Monne]

Hello,

The following series contain some code and style fixes for the pvshim
(comet) code that has been merged into staging. IMHO patches 1-3 should
be backported to the stable comet branch.

A branch with the series is also available at:

git://xenbits.xen.org/people/royger/xen.git pvshim_fixes_v3

Thanks, Roger.

Roger Pau Monne (7):
  xen/pvshim: map vcpu_info earlier for APs
  xen/pvh: place the trampoline starting at MFN 1
  xen/pvshim: identity pin shim vCPUs to pCPUs
  xen/pvshim: re-order replace_va_mapping code
  xen/pvshim: fix coding style issues
  firmware/shim: fix build process to use POSIX find options
  xen/pvshim: switch shim.c to use typesafe mfn_to_page and virt_to_mfn

 tools/firmware/xen-dir/Makefile |  3 +-
 xen/arch/x86/boot/head.S|  3 ++
 xen/arch/x86/dom0_build.c   |  5 ++--
 xen/arch/x86/mm.c   |  9 --
 xen/arch/x86/pv/shim.c  | 63 -
 xen/arch/x86/smpboot.c  |  6 ++--
 6 files changed, 47 insertions(+), 42 deletions(-)

-- 
2.15.1


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

Re: [Xen-devel] [PATCH 4/5] x86/pv: Drop support for paging out the LDT

[Andrew Cooper]

On 18/01/18 10:57, Andrew Cooper wrote:
> On 18/01/18 10:38, George Dunlap wrote:
>> On Thu, Jan 18, 2018 at 7:59 AM, Jan Beulich  wrote:
>> On 12.01.18 at 19:37,  wrote:
 Windows is the only OS which pages out kernel datastructures, so chances 
 are
 good that this is a vestigial remnant of the PV Windows XP experiment.
>>> This is based on what? How do you know there are no other OSes
>>> doing so, including perhaps ones which none of us has ever heard
>>> of?
> The chances of there being a production OS ported to PV that we've never
> head of 0.
>
>>>  The diffstat of the change here is certainly nice, but as always
>>> with something being removed from the ABI I'm rather hesitant.
>>>
 --- a/xen/arch/x86/domain.c
 +++ b/xen/arch/x86/domain.c
 @@ -1942,11 +1942,8 @@ int domain_relinquish_resources(struct domain *d)
  {
  for_each_vcpu ( d, v )
  {
 -/*
 - * Relinquish GDT mappings. No need for explicit 
 unmapping of
 - * the LDT as it automatically gets squashed with the 
 guest
 - * mappings.
 - */
 +/* Relinquish GDT/LDT mappings. */
 +pv_destroy_ldt(v);
  pv_destroy_gdt(v);
>>> The domain is dead at this point, so the order doesn't matter much,
>>> but strictly speaking you should destroy the GDT before destroying
>>> the LDT (just like LDT _loads_ always need to come _after_ GDT
>>> adjustments).
> By that logic, I've got it the correct way round (which is they way I
> intended).  Allocation and freeing of the LDT is nested within the scope
> of the GDT.
>
>>> Everything else here looks fine, but the initial comment may need
>>> further discussion. For example we may want to consider a
>>> two-stage phasing out of the feature, with a couple of years in
>>> between: Make the functionality dependent upon a default-off
>>> command line option for the time being, and issue a bright warning
>>> when someone actually enables it (telling them to tell us).
>> One of the problems we have is that people seem to wait for 2-3 years
>> after a release has been made to start updating to it.  So we'd have
>> to leave such a warning for probably 5 years minimum.
> In almost any other case, I'd agree, and would be the first to suggest this.
>
> However, This patch is strictly necessary for a more complete XPTI,
> which is how I stumbled onto the issue in my KAISER series to begin
> with.  It is the only codepath where we ever poke at a remote critical
> data structure, which is why

Sorry - sent too early.

... which is why isolating the LDT in a per-cpu doesn't work in
combination with this algorithm.

~Andrew

>
> Also as noted in the commit message, it is broken even in the case you
> wanted to sensibly page the LDT, which further backs up the exception
> that it was only for Windows XP, and has never ever encountered a
> production system.
>
> ~Andrew


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

Re: [Xen-devel] [PATCH 4/5] x86/pv: Drop support for paging out the LDT

[Andrew Cooper]

On 18/01/18 10:38, George Dunlap wrote:
> On Thu, Jan 18, 2018 at 7:59 AM, Jan Beulich  wrote:
> On 12.01.18 at 19:37,  wrote:
>>> Windows is the only OS which pages out kernel datastructures, so chances are
>>> good that this is a vestigial remnant of the PV Windows XP experiment.
>> This is based on what? How do you know there are no other OSes
>> doing so, including perhaps ones which none of us has ever heard
>> of?

The chances of there being a production OS ported to PV that we've never
head of 0.

>>  The diffstat of the change here is certainly nice, but as always
>> with something being removed from the ABI I'm rather hesitant.
>>
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -1942,11 +1942,8 @@ int domain_relinquish_resources(struct domain *d)
>>>  {
>>>  for_each_vcpu ( d, v )
>>>  {
>>> -/*
>>> - * Relinquish GDT mappings. No need for explicit unmapping 
>>> of
>>> - * the LDT as it automatically gets squashed with the guest
>>> - * mappings.
>>> - */
>>> +/* Relinquish GDT/LDT mappings. */
>>> +pv_destroy_ldt(v);
>>>  pv_destroy_gdt(v);
>> The domain is dead at this point, so the order doesn't matter much,
>> but strictly speaking you should destroy the GDT before destroying
>> the LDT (just like LDT _loads_ always need to come _after_ GDT
>> adjustments).

By that logic, I've got it the correct way round (which is they way I
intended).  Allocation and freeing of the LDT is nested within the scope
of the GDT.

>>
>> Everything else here looks fine, but the initial comment may need
>> further discussion. For example we may want to consider a
>> two-stage phasing out of the feature, with a couple of years in
>> between: Make the functionality dependent upon a default-off
>> command line option for the time being, and issue a bright warning
>> when someone actually enables it (telling them to tell us).
> One of the problems we have is that people seem to wait for 2-3 years
> after a release has been made to start updating to it.  So we'd have
> to leave such a warning for probably 5 years minimum.

In almost any other case, I'd agree, and would be the first to suggest this.

However, This patch is strictly necessary for a more complete XPTI,
which is how I stumbled onto the issue in my KAISER series to begin
with.  It is the only codepath where we ever poke at a remote critical
data structure, which is why

Also as noted in the commit message, it is broken even in the case you
wanted to sensibly page the LDT, which further backs up the exception
that it was only for Windows XP, and has never ever encountered a
production system.

~Andrew

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

Re: [Xen-devel] [PATCH 4/5] x86/pv: Drop support for paging out the LDT

[Jan Beulich]

>>> On 18.01.18 at 11:38,  wrote:
> On Thu, Jan 18, 2018 at 7:59 AM, Jan Beulich  wrote:
> On 12.01.18 at 19:37,  wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -1942,11 +1942,8 @@ int domain_relinquish_resources(struct domain *d)
>>>  {
>>>  for_each_vcpu ( d, v )
>>>  {
>>> -/*
>>> - * Relinquish GDT mappings. No need for explicit unmapping 
>>> of
>>> - * the LDT as it automatically gets squashed with the guest
>>> - * mappings.
>>> - */
>>> +/* Relinquish GDT/LDT mappings. */
>>> +pv_destroy_ldt(v);
>>>  pv_destroy_gdt(v);
>>
>> The domain is dead at this point, so the order doesn't matter much,
>> but strictly speaking you should destroy the GDT before destroying
>> the LDT (just like LDT _loads_ always need to come _after_ GDT
>> adjustments).
>>
>> Everything else here looks fine, but the initial comment may need
>> further discussion. For example we may want to consider a
>> two-stage phasing out of the feature, with a couple of years in
>> between: Make the functionality dependent upon a default-off
>> command line option for the time being, and issue a bright warning
>> when someone actually enables it (telling them to tell us).
> 
> One of the problems we have is that people seem to wait for 2-3 years
> after a release has been made to start updating to it.  So we'd have
> to leave such a warning for probably 5 years minimum.

That's a reasonable time frame imo for phasing out something that's
a de-facto part of an ABI.

Jan


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

Re: [Xen-devel] [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Julien Grall]

On 17/01/18 14:31, Lars Kurth wrote:

Hi Julien,


Hi Lars,

On 17 Jan 2018, at 12:31, Julien Grall > wrote:





If you took the code from Linux, you need to add the original
Signed-off-by from the Linux commit. Aside from that:


There are multiple commits touching this function. So I followed 
what we did in similar situation. By that I mean, mentioning the 
code was taken from Linux and not gathered the signed-off-by.


If you really want, I can gather all the signed-off-by of the commit 
touching this function.
If there are a lot of then, you may want to consider adding 
a README.source file instead. There are a few examples in tree and 
they are also mentioned in CONTRIBUTING files


I am not sure to understand your suggestion here. I spotted only one 
CONTRIBUTING file and it only list the license.


I was suggesting to create a README.source in the xen arm tree seomwhere 
where you can list imports.


Regarding README.source, this is covering file and contain the same 
mention as in the commit message. As this is a single function. Isn't 
the commit message enough?


 From a legal viewpoint it is enough.
The reason why we created the README.source file is because it is very 
easy to miss code imports when they are mentioned in commit messages.


I understand that.



Normally this isn't a problem: only if we ever have to relicense the 
code or if someone does code archeology
When we relicensed the ACPI builder, it created all sorts of problems as 
outlined in 
https://www.slideshare.net/xen_com_mgr/ossa17-mixed-license-foss-projects


Lastly, we do have quite a bit of code in Xen coming from Linux (or 
other project). A lot of them are not listed in 
README.source/CONTRIBUTING. But only mention in the commit message 
(not necessarily with Signed-off-by tag).


That is true: which is why I have started fixing these, whenever I found 
them and moved such information to README.source.



So I am quite interested to know what is the normal procedure here.


I think:
* Doing this via Signed-off-by tag is sufficient for a one-off-import
* I would prefer if people added import related info README.source files 
(and also in the commit message)


Does this make sense?


I think it makes sense. Do you expect to see the README.source 
per-directory? Or one at top level?


Cheers,



Lars


--
Julien Grall

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

Re: [Xen-devel] [PATCH] ocaml: fix arm build

[Wei Liu]

On Wed, Jan 17, 2018 at 04:43:54PM +, Wei Liu wrote:
> ARM doesn't have emulation_flags in the arch_domainconfig.
> 
> Signed-off-by: Wei Liu 
> ---
> Cc: Ian Jackson 
> Cc: Julien Grall 
> Cc: Andrew Cooper 
> ---
>  tools/ocaml/libs/xc/xenctrl_stubs.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c 
> b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index 0b5a2361c0..fd128778b3 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -175,11 +175,15 @@ CAMLprim value stub_xc_domain_create(value xch, value 
> ssidref,
>   caml_failwith("Unhandled: ARM");
>   break;
>  
> +#if defined(__i386__) || defined(__x86_64__)
>   case 1: /* X86 - emulation flags in the block */

The label will be moved outside of the #ifdef.

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

Re: [Xen-devel] [PATCH 4/5] x86/pv: Drop support for paging out the LDT

[George Dunlap]

On Thu, Jan 18, 2018 at 7:59 AM, Jan Beulich  wrote:
 On 12.01.18 at 19:37,  wrote:
>> Windows is the only OS which pages out kernel datastructures, so chances are
>> good that this is a vestigial remnant of the PV Windows XP experiment.
>
> This is based on what? How do you know there are no other OSes
> doing so, including perhaps ones which none of us has ever heard
> of? The diffstat of the change here is certainly nice, but as always
> with something being removed from the ABI I'm rather hesitant.
>
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -1942,11 +1942,8 @@ int domain_relinquish_resources(struct domain *d)
>>  {
>>  for_each_vcpu ( d, v )
>>  {
>> -/*
>> - * Relinquish GDT mappings. No need for explicit unmapping 
>> of
>> - * the LDT as it automatically gets squashed with the guest
>> - * mappings.
>> - */
>> +/* Relinquish GDT/LDT mappings. */
>> +pv_destroy_ldt(v);
>>  pv_destroy_gdt(v);
>
> The domain is dead at this point, so the order doesn't matter much,
> but strictly speaking you should destroy the GDT before destroying
> the LDT (just like LDT _loads_ always need to come _after_ GDT
> adjustments).
>
> Everything else here looks fine, but the initial comment may need
> further discussion. For example we may want to consider a
> two-stage phasing out of the feature, with a couple of years in
> between: Make the functionality dependent upon a default-off
> command line option for the time being, and issue a bright warning
> when someone actually enables it (telling them to tell us).

One of the problems we have is that people seem to wait for 2-3 years
after a release has been made to start updating to it.  So we'd have
to leave such a warning for probably 5 years minimum.

 -George

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

Re: [Xen-devel] [PATCH] x86/hvm: more sure APIC assist is aborted if guest EOIs APIC

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 18 January 2018 10:17
> To: Paul Durrant 
> Cc: Andrew Cooper ; xen-
> de...@lists.xenproject.org
> Subject: RE: [PATCH] x86/hvm: more sure APIC assist is aborted if guest EOIs
> APIC
> 
> >>> On 18.01.18 at 10:55,  wrote:
> > Ok. I'll re-work it so that abort is passed a vector and warns on mismatch.
> 
> You mean on the new path only, I suppose? The old path should
> not trigger warnings (aiui it would always do so).

I think, in the existing abort case, the latched vector should always be the 
lowest pending bit in the ISR.

  Paul

> 
> Jan


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

Re: [Xen-devel] [PATCH v2] libxl: put RSDP for PVH guest near 4GB

[Wei Liu]

On Thu, Jan 18, 2018 at 11:31:32AM +0100, Juergen Gross wrote:
> Wei,
> 
> On 01/12/17 15:14, Juergen Gross wrote:
> > Instead of locating the RSDP table below 1MB put it just below 4GB
> > like the rest of the ACPI tables in case of PVH guests. This will
> > avoid punching more holes than necessary into the memory map.
> > 
> > Signed-off-by: Juergen Gross 
> > Acked-by: Wei Liu 
> 
> Mind applying this one?

Don't worry, it is in my queue.

Will come to this and other patches I accumulated soon.

Wei.

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

Re: [Xen-devel] [PATCH v2] libxl: put RSDP for PVH guest near 4GB

[Juergen Gross]

Wei,

On 01/12/17 15:14, Juergen Gross wrote:
> Instead of locating the RSDP table below 1MB put it just below 4GB
> like the rest of the ACPI tables in case of PVH guests. This will
> avoid punching more holes than necessary into the memory map.
> 
> Signed-off-by: Juergen Gross 
> Acked-by: Wei Liu 

Mind applying this one?


Juergen

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

Re: [Xen-devel] [PATCH] x86/hvm: more sure APIC assist is aborted if guest EOIs APIC

[Jan Beulich]

>>> On 18.01.18 at 10:55,  wrote:
> Ok. I'll re-work it so that abort is passed a vector and warns on mismatch.

You mean on the new path only, I suppose? The old path should
not trigger warnings (aiui it would always do so).

Jan


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

Re: [Xen-devel] [PATCH 6/6] firmware/shim: fix build process to use POSIX find options

[Roger Pau Monné]

On Wed, Jan 17, 2018 at 05:58:43PM +, Andrew Cooper wrote:
> On 17/01/18 17:32, Roger Pau Monné wrote:
> > On Wed, Jan 17, 2018 at 04:24:27PM +, Ian Jackson wrote:
> >> Roger Pau Monne writes ("[PATCH 6/6] firmware/shim: fix build process to 
> >> use POSIX find options"):
> >>> The -printf find option is not POSIX compatible, so replace it with
> >>> another rune.
> >> ...
> >>> cd $(D)/$(d); \
> >>> -   find $(XEN_ROOT)/$(d)/ -type d -printf "./%P\n" |  xargs 
> >>> mkdir -p);)
> >>> +   find $(XEN_ROOT)/$(d)/ -type d -exec sh -c \
> >>> +   "echo {} | sed 's,^$(XEN_ROOT)/$(d)/,,g' | xargs mkdir 
> >>> -p" \;);)
> >> This is now a pretty nasty shell construct.
> >>
> >> If you're going to use sed, you could just
> >>find ... -print | sed ... | xargs mkdir -p
> > I think I will go with this one...

What about using:

find $(XEN_ROOT)/$(d)/ -type d | sed 's,^$(XEN_ROOT)/$(d)/,,g' | xargs mkdir -p

This AFAICT works fine, and should be the one involving less forks
since the whole output is processed at once.

Thanks, Roger.

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