[Xen-devel] [xen-unstable test] 132380: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  08b908ba63dee8bc313983c5e412852cbcbcda85
baseline version:
 xen  1912f1220cf87aee28349469893f101980714a05

Last test of basis   132230  2019-01-21 02:07:39 Z2 days
Testing same since   132380  2019-01-22 15:38:36 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anthony PERARD 
  Ian Jackson 
  Jan Beulich 
  Razvan Cojocaru 
  Roger Pau Monné 
  Tamas K Lengyel 

jobs:
 build-amd64-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  pass
 build-i386-libvirt   pass
 build-amd64-prev pass
 build-i386-prev  pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops  

Re: [Xen-devel] [PATCH v4 1/1] cameraif: add ABI for para-virtual camera

[Oleksandr Andrushchenko]

Any comments from Xen community?
Konrad?

On 1/15/19 4:44 PM, Hans Verkuil wrote:
> Hi Oleksandr,
>
> Just two remaining comments:
>
> On 1/15/19 10:38 AM, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko 
>>
>> This is the ABI for the two halves of a para-virtualized
>> camera driver which extends Xen's reach multimedia capabilities even
>> farther enabling it for video conferencing, In-Vehicle Infotainment,
>> high definition maps etc.
>>
>> The initial goal is to support most needed functionality with the
>> final idea to make it possible to extend the protocol if need be:
>>
>> 1. Provide means for base virtual device configuration:
>>   - pixel formats
>>   - resolutions
>>   - frame rates
>> 2. Support basic camera controls:
>>   - contrast
>>   - brightness
>>   - hue
>>   - saturation
>> 3. Support streaming control
>>
>> Signed-off-by: Oleksandr Andrushchenko 
>> ---
>>   xen/include/public/io/cameraif.h | 1364 ++
>>   1 file changed, 1364 insertions(+)
>>   create mode 100644 xen/include/public/io/cameraif.h
>>
>> diff --git a/xen/include/public/io/cameraif.h 
>> b/xen/include/public/io/cameraif.h
>> new file mode 100644
>> index ..246eb2457f40
>> --- /dev/null
>> +++ b/xen/include/public/io/cameraif.h
>> @@ -0,0 +1,1364 @@
> 
>
>> +/*
>> + 
>> **
>> + * EVENT CODES
>> + 
>> **
>> + */
>> +#define XENCAMERA_EVT_FRAME_AVAIL  0x00
>> +#define XENCAMERA_EVT_CTRL_CHANGE  0x01
>> +
>> +/* Resolution has changed. */
>> +#define XENCAMERA_EVT_CFG_FLG_RESOL(1 << 0)
> I think this flag is a left-over from v2 and should be removed.
>
> 
>
>> + * Request number of buffers to be used:
>> + * 01 2   3octet
>> + * +++++
>> + * |   id| _OP_BUF_REQUEST|   reserved | 4
>> + * +++++
>> + * | reserved  | 8
>> + * +++++
>> + * |num_bufs| reserved | 12
>> + * +++++
>> + * | reserved  | 16
>> + * +++++
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +++++
>> + * | reserved  | 64
>> + * +++++
>> + *
>> + * num_bufs - uint8_t, desired number of buffers to be used. This is
>> + *   limited to the value configured in XenStore.max-buffers.
>> + *   Passing zero num_bufs in this request (after streaming has stopped
>> + *   and all buffers destroyed) unblocks camera configuration changes.
> I think the phrase 'unblocks camera configuration changes' is confusing.
>
> In v3 this sentence came after the third note below, and so it made sense
> in that context, but now the order has been reversed and it became hard to
> understand.
>
> I'm not sure what the best approach is to fix this. One option is to remove
> the third note and integrate it somehow in the sentence above. Or perhaps
> do away with the 'notes' at all and just write a more extensive documentation
> for this op. I leave that up to you.
>
>> + *
>> + * See response format for this request.
>> + *
>> + * Notes:
>> + *  - frontend must check the corresponding response in order to see
>> + *if the values reported back by the backend do match the desired ones
>> + *and can be accepted.
>> + *  - frontend may send multiple XENCAMERA_OP_BUF_REQUEST requests before
>> + *sending XENCAMERA_OP_STREAM_START request to update or tune the
>> + *configuration.
>> + *  - after this request camera configuration cannot be changed, unless
> camera configuration -> the camera configuration
>
>> + *streaming is stopped and buffers destroyed
>> + */
> Regards,
>
>   Hans
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 6/6] x86: xen: no need to check return value of debugfs_create functions

[Juergen Gross]

On 22/01/2019 15:35, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.
> 
> Cc: Boris Ostrovsky 
> Cc: Juergen Gross 
> Cc: Stefano Stabellini 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: "H. Peter Anvin" 
> Cc: 
> Cc: 
> Signed-off-by: Greg Kroah-Hartman 

Reviewed-by: Juergen Gross 


Juergen

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

[Xen-devel] [seabios test] 132395: tolerable FAIL - PUSHED

[osstest service owner]

flight 132395 seabios real [real]
http://logs.test-lab.xenproject.org/osstest/logs/132395/

Failures :-/ but no regressions.

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

version targeted for testing:
 seabios  34fe8660ec42e18f768fb1f5e645c7a59620e2ed
baseline version:
 seabios  d62ca8c9c53f375b69a85da0aafe5aaced79642f

Last test of basis   132271  2019-01-21 12:30:10 Z1 days
Testing same since   132395  2019-01-22 19:43:51 Z0 days1 attempts


People who touched revisions under test:
  Kevin O'Connor 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/seabios.git
   d62ca8c..34fe866  34fe8660ec42e18f768fb1f5e645c7a59620e2ed -> 
xen-tested-master

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

[Xen-devel] [PATCH] xen-block: handle resize callback

[Paul Durrant]

Some frontend drivers will handle dynamic resizing of PV disks, so set up
the BlockDevOps resize_cb() method during xen_block_realize() to allow
this to be done.

Signed-off-by: Paul Durrant 
---
Cc: Stefan Hajnoczi 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Kevin Wolf 
Cc: Max Reitz 
---
 hw/block/dataplane/xen-block.c |  4 +---
 hw/block/trace-events  |  1 +
 hw/block/xen-block.c   | 26 ++
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index d0d8905a33..c6a15da024 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -50,7 +50,6 @@ struct XenBlockDataPlane {
 unsigned int nr_ring_ref;
 void *sring;
 int64_t file_blk;
-int64_t file_size;
 int protocol;
 blkif_back_rings_t rings;
 int more_work;
@@ -189,7 +188,7 @@ static int xen_block_parse_request(XenBlockRequest *request)
request->req.seg[i].first_sect + 1) * dataplane->file_blk;
 request->size += len;
 }
-if (request->start + request->size > dataplane->file_size) {
+if (request->start + request->size > blk_getlength(dataplane->blk)) {
 error_report("error: access beyond end of file");
 goto err;
 }
@@ -638,7 +637,6 @@ XenBlockDataPlane *xen_block_dataplane_create(XenDevice 
*xendev,
 dataplane->xendev = xendev;
 dataplane->file_blk = conf->logical_block_size;
 dataplane->blk = conf->blk;
-dataplane->file_size = blk_getlength(dataplane->blk);
 
 QLIST_INIT(&dataplane->inflight);
 QLIST_INIT(&dataplane->freelist);
diff --git a/hw/block/trace-events b/hw/block/trace-events
index d0851953c5..8020f9226a 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -126,6 +126,7 @@ xen_block_realize(const char *type, uint32_t disk, uint32_t 
partition) "%s d%up%
 xen_block_connect(const char *type, uint32_t disk, uint32_t partition) "%s 
d%up%u"
 xen_block_disconnect(const char *type, uint32_t disk, uint32_t partition) "%s 
d%up%u"
 xen_block_unrealize(const char *type, uint32_t disk, uint32_t partition) "%s 
d%up%u"
+xen_block_size(const char *type, uint32_t disk, uint32_t partition, int64_t 
sectors) "%s d%up%u %"PRIi64
 xen_disk_realize(void) ""
 xen_disk_unrealize(void) ""
 xen_cdrom_realize(void) ""
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index a636487b3e..8ea28381ea 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -144,6 +144,24 @@ static void xen_block_unrealize(XenDevice *xendev, Error 
**errp)
 }
 }
 
+static void xen_block_resize_cb(void *opaque)
+{
+XenBlockDevice *blockdev = opaque;
+const char *type = object_get_typename(OBJECT(blockdev));
+XenBlockVdev *vdev = &blockdev->props.vdev;
+BlockConf *conf = &blockdev->props.conf;
+XenDevice *xendev = XEN_DEVICE(blockdev);
+int64_t sectors = blk_getlength(conf->blk) / conf->logical_block_size;
+
+trace_xen_block_size(type, vdev->disk, vdev->partition, sectors);
+
+xen_device_backend_printf(xendev, "sectors", "%"PRIi64, sectors);
+}
+
+static const BlockDevOps xen_block_dev_ops = {
+.resize_cb = xen_block_resize_cb,
+};
+
 static void xen_block_realize(XenDevice *xendev, Error **errp)
 {
 XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev);
@@ -180,7 +198,7 @@ static void xen_block_realize(XenDevice *xendev, Error 
**errp)
 }
 
 if (!blkconf_apply_backend_options(conf, blockdev->info & VDISK_READONLY,
-   false, errp)) {
+   true, errp)) {
 return;
 }
 
@@ -197,6 +215,7 @@ static void xen_block_realize(XenDevice *xendev, Error 
**errp)
 return;
 }
 
+blk_set_dev_ops(conf->blk, &xen_block_dev_ops, blockdev);
 blk_set_guest_block_size(conf->blk, conf->logical_block_size);
 
 if (conf->discard_granularity > 0) {
@@ -215,9 +234,8 @@ static void xen_block_realize(XenDevice *xendev, Error 
**errp)
 
 xen_device_backend_printf(xendev, "sector-size", "%u",
   conf->logical_block_size);
-xen_device_backend_printf(xendev, "sectors", "%"PRIi64,
-  blk_getlength(conf->blk) /
-  conf->logical_block_size);
+
+xen_block_dev_ops.resize_cb(blockdev);
 
 blockdev->dataplane =
 xen_block_dataplane_create(xendev, conf, blockdev->props.iothread);
-- 
2.20.1.2.gb21ebb6


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

[Xen-devel] [PATCH for-4.12] amd/iommu: fix present bit checking when clearing PTE

[Roger Pau Monne]

The current check for the present bit is wrong, since the present bit
is located in the low part of the entry.

Fixes: e8afe1124cc1 ("iommu: elide flushing for higher order map/unmap 
operations")
Signed-off-by: Roger Pau Monné 
---
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
Cc: Juergen Gross 
Cc: Paul Durrant 
---
 xen/drivers/passthrough/amd/iommu_map.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_map.c 
b/xen/drivers/passthrough/amd/iommu_map.c
index 99ac0a6862..67329b0c95 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -39,15 +39,13 @@ static unsigned int clear_iommu_pte_present(unsigned long 
l1_mfn,
 unsigned long dfn)
 {
 uint64_t *table, *pte;
-uint32_t entry;
 unsigned int flush_flags;
 
 table = map_domain_page(_mfn(l1_mfn));
 
 pte = (table + pfn_to_pde_idx(dfn, 1));
-entry = *pte >> 32;
 
-flush_flags = get_field_from_reg_u32(entry, IOMMU_PTE_PRESENT_MASK,
+flush_flags = get_field_from_reg_u32(*pte, IOMMU_PTE_PRESENT_MASK,
  IOMMU_PTE_PRESENT_SHIFT) ?
  IOMMU_FLUSHF_modified : 0;
 
-- 
2.20.1


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

[Xen-devel] [linux-3.18 bisection] complete test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow
testid xen-boot

Tree: linux 
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  linux 
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
  Bug introduced:  7b8052e19304865477e03a0047062d977309a22f
  Bug not present: d255d18a34a8d53ccc4a019dc07e17b6e8cf6bd1
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/132423/


  commit 7b8052e19304865477e03a0047062d977309a22f
  Author: Jan Beulich 
  Date:   Mon Oct 19 04:23:29 2015 -0600
  
  igb: fix NULL derefs due to skipped SR-IOV enabling
  
  [ Upstream commit be06998f96ecb93938ad2cce46c4289bf7cf45bc ]
  
  The combined effect of commits 6423fc3416 ("igb: do not re-init SR-IOV
  during probe") and ceee3450b3 ("igb: make sure SR-IOV init uses the
  right number of queues") causes VFs no longer getting set up, leading
  to NULL pointer dereferences due to the adapter's ->vf_data being NULL
  while ->vfs_allocated_count is non-zero. The first commit not only
  neglected the side effect of igb_sriov_reinit() that the second commit
  tried to account for, but also that of setting IGB_FLAG_HAS_MSIX,
  without which igb_enable_sriov() is effectively a no-op. Calling
  igb_{,re}set_interrupt_capability() as done here seems to address this,
  but I'm not sure whether this is better than sinply reverting the other
  two commits.
  
  Signed-off-by: Jan Beulich 
  Tested-by: Aaron Brown 
  Signed-off-by: Jeff Kirsher 
  Signed-off-by: Sasha Levin 


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


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/linux-3.18/test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow.xen-boot
 --summary-out=tmp/132423.bisection-summary --basis-template=128858 
--blessings=real,real-bisect linux-3.18 
test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow xen-boot
Searching for failure / basis pass:
 132290 fail [host=debina0] / 128858 [host=fiano0] 128841 [host=baroque1] 
128807 [host=albana1] 128691 ok.
Failure / basis pass flights: 132290 / 128691
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: linux 
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Latest 9b5eed105a45ac0557af113b4096132ae7e3e47f 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
de5b678ca4dcdfa83e322491d478d66df56c1986 
1912f1220cf87aee28349469893f101980714a05
Basis pass 0d63979c1bc9c85578be4c589768a13dc0a7c5eb 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 
de5b678ca4dcdfa83e322491d478d66df56c1986 
85b00385827e4e061b2ff38b549c03d0f1e66b6a
Generating revisions with ./adhoc-revtuple-generator  
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git#0d63979c1bc9c85578be4c589768a13dc0a7c5eb-9b5eed105a45ac0557af113b4096132ae7e3e47f
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/qemu-xen-traditional.git#9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149-d0d8ad39ecb51cd7497cd524484fe09f50876798
 git://xenbits.xen.org/qemu-xen.git\
 
#de5b678ca4dcdfa83e322491d478d66df56c1986-de5b678ca4dcdfa83e322491d478d66df56c1986
 
git://xenbits.xen.org/xen.git#85b00385827e4e061b2ff38b549c03d0f1e66b6a-1912f1220cf87aee28349469893f101980714a05
Loaded 3004 nodes in revision graph
Searching for test results:
 128691 pass 0d63979c1bc9c85578be4c589768a13dc0a7c5eb 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 
de5b678ca4dcdfa83e322491d478d66df56c1986 
85b00385827e4e061b2ff38b549c03d0f1e66b6a
 128807 [host=albana1]
 128858 [host=fiano0]
 128841 [host=baroque1]
 129760 fail irrelevant
 129845 fail irrelevant
 129971 pass 0d63979c1bc9c85578be4c589768a13dc0a7c5eb 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 
de5b678ca4dcdfa83e322491d478d66df56c1986 
85b00385827e4e061b2ff38b549c03d0f1e66b6a
 129975 fail irrelevant
 129977 pass irrelevant
 129985 pass irrelevant
 129990 fail irrelevant
 129993 pass irrelevant
 12 pass irrelevant
 130001

Re: [Xen-devel] [PATCH for-4.12] amd/iommu: fix present bit checking when clearing PTE

[Paul Durrant]

> -Original Message-
> From: Roger Pau Monne [mailto:roger@citrix.com]
> Sent: 23 January 2019 09:48
> To: xen-devel@lists.xenproject.org
> Cc: Roger Pau Monne ; Suravee Suthikulpanit
> ; Brian Woods ;
> Juergen Gross ; Paul Durrant 
> Subject: [PATCH for-4.12] amd/iommu: fix present bit checking when
> clearing PTE
> 
> The current check for the present bit is wrong, since the present bit
> is located in the low part of the entry.
> 
> Fixes: e8afe1124cc1 ("iommu: elide flushing for higher order map/unmap
> operations")
> Signed-off-by: Roger Pau Monné 

No idea how I managed to get that wrong. I had a bad feeling at the time about 
not accessing the pte as a uint32_t[2] array.

Reviewed-by: Paul Durrant 

> ---
> Cc: Suravee Suthikulpanit 
> Cc: Brian Woods 
> Cc: Juergen Gross 
> Cc: Paul Durrant 
> ---
>  xen/drivers/passthrough/amd/iommu_map.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/amd/iommu_map.c
> b/xen/drivers/passthrough/amd/iommu_map.c
> index 99ac0a6862..67329b0c95 100644
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -39,15 +39,13 @@ static unsigned int clear_iommu_pte_present(unsigned
> long l1_mfn,
>  unsigned long dfn)
>  {
>  uint64_t *table, *pte;
> -uint32_t entry;
>  unsigned int flush_flags;
> 
>  table = map_domain_page(_mfn(l1_mfn));
> 
>  pte = (table + pfn_to_pde_idx(dfn, 1));
> -entry = *pte >> 32;
> 
> -flush_flags = get_field_from_reg_u32(entry, IOMMU_PTE_PRESENT_MASK,
> +flush_flags = get_field_from_reg_u32(*pte, IOMMU_PTE_PRESENT_MASK,
>   IOMMU_PTE_PRESENT_SHIFT) ?
>   IOMMU_FLUSHF_modified : 0;
> 
> --
> 2.20.1

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

[Xen-devel] [RFC] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Andrii Anisov]

From: Andrii Anisov 

Taking decission by `need_iommu_pt_sync()` make us never kicking
`iommu_iotlb_flush()` for IOMMUs which do share TLB with CPU.
So check `has_iommu_pt()` instead.

Signed-off-by: Andrii Anisov 

---

Julien,

Could you please look at this, IMO there is a mistake here.
x86 uses `need_iommu_pt_sync()` to make decission if iommu's map/unmap should 
be additionally called.
But ARM has no non-shared pt support in the mainline, so using 
`need_iommu_pt_sync()` seems to be odd.

 xen/arch/arm/p2m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 2394f97..059a391 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1019,7 +1019,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
  !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) )
 p2m_free_entry(p2m, orig_pte, level);
 
-if ( need_iommu_pt_sync(p2m->domain) &&
+if ( has_iommu_pt(p2m->domain) &&
  (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
 {
 unsigned int flush_flags = 0;
-- 
2.7.4


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

[Xen-devel] [xen-unstable-coverity test] 132424: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xen  08b908ba63dee8bc313983c5e412852cbcbcda85
baseline version:
 xen  1912f1220cf87aee28349469893f101980714a05

Last test of basis   132169  2019-01-20 09:22:47 Z3 days
Testing same since   132424  2019-01-23 09:19:14 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anthony PERARD 
  Ian Jackson 
  Jan Beulich 
  Razvan Cojocaru 
  Roger Pau Monné 
  Tamas K Lengyel 

jobs:
 coverity-amd64   pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   1912f1220c..08b908ba63  08b908ba63dee8bc313983c5e412852cbcbcda85 -> 
coverity-tested/smoke

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

Re: [Xen-devel] Organising a workshop to solve safety certification related questions

[Lars Kurth]

Hi all,

it looks as if March 25/26 in Frankfurt or Cambridge is the best option. For 
Matt, this would mean that he can only attend the first day, but I believe this 
would be OK. Maybe Robin can attend the second day, instead of Matt. Before we 
finalise the dates, I will need to secure the meeting space. I will be able to 
do this in the next few days and will send an update as soon as this is done.

Note that we had a few people on this list which have replied to me privately. 
Please let me know privately or publicly whether March 25/26 would be suitable 
for you. We can in parallel work on the agenda.
 
Best Regards
Lars

> On 16 Jan 2019, at 13:09, Lars Kurth  wrote:
> 
> 
> 
>> On 16 Jan 2019, at 12:16, George Dunlap > > wrote:
>> 
>> On 1/8/19 5:59 PM, Lars Kurth wrote:
>>> What I need is 
>>> - Raise your hands if you are interested 
>>> - Let me know of date / location restrictions
>>> - We could try and so some of this via video conference: would you be able 
>>> to attend if we did open the meeting up to some remote participation
>> 
>> I'm interested.  All the dates mentioned should work for me.
>> 
>> -George
> 
> Hi all,
> 
> to summarise!
> 
> We have a good number of people and organisations interested from pretty one 
> everyone on the list, but it seems the dates won't work for most people. 
> Location wise: Germany (Frankfurt) and/or UK (Cambridge) work for most, 
> except for representatives from Dornerworks and Starlab, who would dial in 
> for some of the meetings 
> There seems to be a slight bias for Cambridge, as we have most of our 
> maintainers there. 
> 
> Automotive vendors would be happy to align with automotive meetings/events 
> (even in Japan), but that won't work for the committers as they won't 
> normally be able to travel. 
> I also have two organisations which could potentially host in Cambridge and 
> one in Germany (Frankfurt). But the venue depends partly on the dates. This 
> tells me, that we should choose either Frankfurt or Cambridge for the event.
> 
> In terms of numbers we are roughly looking at 10-12 who could attend 
> physically, but it could be more
> 
> To move forward, I thought I would expend the time horizon a little bit via 
> the following doodle poll: https://doodle.com/poll/anvfr2hk2t8gy9a8 
> 
> Note that you can specify suboptimal dates by clicking twice: also, if you 
> have any constraints on location, etc. feel free to make use of the 
> commenting feature.
> 
> I will be in the US mid-March and thus excluded these dates. I also excluded 
> March 28/29: because of Brexit, it is possible that there would be some 
> travel chaos at least in the UK. 
> 
> Regards
> Lars
> 
> 
> 

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

Re: [Xen-devel] [PATCH for-4.12] amd/iommu: fix present bit checking when clearing PTE

[Juergen Gross]

On 23/01/2019 10:47, Roger Pau Monne wrote:
> The current check for the present bit is wrong, since the present bit
> is located in the low part of the entry.
> 
> Fixes: e8afe1124cc1 ("iommu: elide flushing for higher order map/unmap 
> operations")
> Signed-off-by: Roger Pau Monné 

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH] iommu: specify page_count rather than page_order to iommu_map/unmap()...

[Jan Beulich]

>>> On 22.01.19 at 19:23,  wrote:
> On 22/01/2019 10:46, Jan Beulich wrote:
>>
>>> Regardless of the 
>>> alignment though, the fact that order comes from a hypercall argument and 
>>> may 
>>> not match any of the orders supported by the IOMMU implementation makes me 
>>> think that using a page count is better.
>> Splitting up guest requests is orthogonal to whether a count or an
>> order is more suitable as a parameter.
> 
> No - this is most certainly not true.
> 
> Any arbitrary mapping can be expressed with a single map call, given a
> start/count.  This is not true of a start/order pair, so start/count is
> strictly more expressive.
> 
> Furthermore, I've already given the following concrete options as to why
> start/count is better than start/order:  Reduced caller looping,

That depends on how many callers are actually affected. Hence my
request for concrete examples. I'm about to look into what Roger's
latest reply is meaning in this regard.

> reduced TLB flushing in the current implementation,

I'm afraid I lack the connection to the amount of TLB flushing done.

> and the fact we literally have hypercalls using this mechanism who's
> API is stable.

Hmm, would you mind helping me with this? All the memop-s are
using order values as input. XEN_DOMCTL_memory_mapping is
not a stable interface. What else do we have that I can't seem
to recall?

Jan



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

Re: [Xen-devel] [PATCH] iommu: specify page_count rather than page_order to iommu_map/unmap()...

[Jan Beulich]

>>> On 22.01.19 at 18:02,  wrote:
> On Mon, Jan 21, 2019 at 04:27:38AM -0700, Jan Beulich wrote:
>> >>> On 18.01.19 at 17:03,  wrote:
>> > ...and remove alignment assertions.
>> > 
>> > Testing shows that certain callers of iommu_legacy_map/unmap() specify
>> > order > 0 ranges that are not order aligned thus causing one of the
>> > IS_ALIGNED() assertions to fire.
>> 
>> As said before - without a much better explanation of why the current
>> order-based model is unsuitable (so far I've been provided only vague
>> pointers into "somewhere in PVH Dom0 boot code" iirc) to understand
>> why it's undesirable to simply make those call sites obey to the current
>> requirements, I'm not happy to see us go this route.
> 
> The current PVH dom0 builder will try to always use the biggest
> possible order to populate the physmap.
> 
> However, the memory map used by dom0 is not under our control, so it's
> quite likely that a RAM region starts at a 4K only aligned address.
> dom0 builder will then find the next 2M or 1G aligned address and
> populate the space between the current address and the next aligned
> address using an order as high as possible. In the above scenario,
> it's perfectly fine to populate a 4K aligned entry using an order of 5
> for example, in order to reach the next 2M or 1G aligned address.

When filling the gap between an entry aligned no better than 4k,
you unavoidably will need to allocate (and map) at least one
order-0 chunk. If you started with an order-5 one, in the next
iteration the alignment would still be no better than 4k (and hence
you'd never reach an alignment of 2M or 1G)..

> Not removing the asserts would imply that in the above example the
> dom0 builder has to iterate over all the 4K pages and make repeated
> guest_physmap_add_page calls with order 0. This is sub-optimal,
> creates a non-trivial overhead to the Dom0 builder, and also promotes
> the open-coding of loops around guest_physmap_add_page.

It would need to make one request with order-0, and then individual
requests with higher orders to incrementally increase alignment. I've
already indicated that I think the function should accept all orders as
inputs, irrespective of underlying hardware capabilities, if it doesn't
already. But that's still not the same as accepting arbitrary counts as
inputs.

Jan



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

[Xen-devel] [PATCH 1/3] treewide: Lift switch variables out of switches

[Kees Cook]

Variables declared in a switch statement before any case statements
cannot be initialized, so move all instances out of the switches.
After this, future always-initialized stack variables will work
and not throw warnings like this:

fs/fcntl.c: In function ‘send_sigio_to_task’:
fs/fcntl.c:738:13: warning: statement will never be executed 
[-Wswitch-unreachable]
   siginfo_t si;
 ^~

Signed-off-by: Kees Cook 
---
 arch/x86/xen/enlighten_pv.c   |  7 ---
 drivers/char/pcmcia/cm4000_cs.c   |  2 +-
 drivers/char/ppdev.c  | 20 ---
 drivers/gpu/drm/drm_edid.c|  4 ++--
 drivers/gpu/drm/i915/intel_display.c  |  2 +-
 drivers/gpu/drm/i915/intel_pm.c   |  4 ++--
 drivers/net/ethernet/intel/e1000/e1000_main.c |  3 ++-
 drivers/tty/n_tty.c   |  3 +--
 drivers/usb/gadget/udc/net2280.c  |  5 ++---
 fs/fcntl.c|  3 ++-
 mm/shmem.c|  5 +++--
 net/core/skbuff.c |  4 ++--
 net/ipv6/ip6_gre.c|  4 ++--
 net/ipv6/ip6_tunnel.c |  4 ++--
 net/openvswitch/flow_netlink.c|  7 +++
 security/tomoyo/common.c  |  3 ++-
 security/tomoyo/condition.c   |  7 ---
 security/tomoyo/util.c|  4 ++--
 18 files changed, 45 insertions(+), 46 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index c54a493e139a..a79d4b548a08 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -907,14 +907,15 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
 static int xen_write_msr_safe(unsigned int msr, unsigned low, unsigned high)
 {
int ret;
+#ifdef CONFIG_X86_64
+   unsigned which;
+   u64 base;
+#endif
 
ret = 0;
 
switch (msr) {
 #ifdef CONFIG_X86_64
-   unsigned which;
-   u64 base;
-
case MSR_FS_BASE:   which = SEGBASE_FS; goto set;
case MSR_KERNEL_GS_BASE:which = SEGBASE_GS_USER; goto set;
case MSR_GS_BASE:   which = SEGBASE_GS_KERNEL; goto set;
diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c
index 7a4eb86aedac..7211dc0e6f4f 100644
--- a/drivers/char/pcmcia/cm4000_cs.c
+++ b/drivers/char/pcmcia/cm4000_cs.c
@@ -663,6 +663,7 @@ static void monitor_card(struct timer_list *t)
 {
struct cm4000_dev *dev = from_timer(dev, t, timer);
unsigned int iobase = dev->p_dev->resource[0]->start;
+   unsigned char flags0;
unsigned short s;
struct ptsreq ptsreq;
int i, atrc;
@@ -731,7 +732,6 @@ static void monitor_card(struct timer_list *t)
}
 
switch (dev->mstate) {
-   unsigned char flags0;
case M_CARDOFF:
DEBUGP(4, dev, "M_CARDOFF\n");
flags0 = inb(REG_FLAGS0(iobase));
diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index 1ae77b41050a..d77c97e4f996 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -359,14 +359,19 @@ static int pp_do_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
struct pp_struct *pp = file->private_data;
struct parport *port;
void __user *argp = (void __user *)arg;
+   struct ieee1284_info *info;
+   unsigned char reg;
+   unsigned char mask;
+   int mode;
+   s32 time32[2];
+   s64 time64[2];
+   struct timespec64 ts;
+   int ret;
 
/* First handle the cases that don't take arguments. */
switch (cmd) {
case PPCLAIM:
{
-   struct ieee1284_info *info;
-   int ret;
-
if (pp->flags & PP_CLAIMED) {
dev_dbg(&pp->pdev->dev, "you've already got it!\n");
return -EINVAL;
@@ -517,15 +522,6 @@ static int pp_do_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
 
port = pp->pdev->port;
switch (cmd) {
-   struct ieee1284_info *info;
-   unsigned char reg;
-   unsigned char mask;
-   int mode;
-   s32 time32[2];
-   s64 time64[2];
-   struct timespec64 ts;
-   int ret;
-
case PPRSTATUS:
reg = parport_read_status(port);
if (copy_to_user(argp, ®, sizeof(reg)))
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b506e3622b08..8f93956c1628 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3942,12 +3942,12 @@ static void drm_edid_to_eld(struct drm_connector 
*connector, struct edid *edid)
}
 
for_each_cea_db(cea, i, start, end) {
+   int sad_count;
+
db = &cea[i];

[Xen-devel] [PATCH 3/3] lib: Introduce test_stackinit module

[Kees Cook]

Adds test for stack initialization coverage. We have several build options
that control the level of stack variable initialization. This test lets us
visualize which options cover which cases, and provide tests for options
that are currently not available (padding initialization).

All options pass the explicit initialization cases and the partial
initializers (even with padding):

test_stackinit: u8_zero ok
test_stackinit: u16_zero ok
test_stackinit: u32_zero ok
test_stackinit: u64_zero ok
test_stackinit: char_array_zero ok
test_stackinit: small_hole_zero ok
test_stackinit: big_hole_zero ok
test_stackinit: packed_zero ok
test_stackinit: small_hole_dynamic_partial ok
test_stackinit: big_hole_dynamic_partial ok
test_stackinit: packed_static_partial ok
test_stackinit: small_hole_static_partial ok
test_stackinit: big_hole_static_partial ok

The results of the other tests (which contain no explicit initialization),
change based on the build's configured compiler instrumentation.

No options:

test_stackinit: small_hole_static_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_static_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_dynamic_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_dynamic_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_runtime_partial FAIL (uninit bytes: 23)
test_stackinit: big_hole_runtime_partial FAIL (uninit bytes: 127)
test_stackinit: small_hole_runtime_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_runtime_all FAIL (uninit bytes: 61)
test_stackinit: u8 FAIL (uninit bytes: 1)
test_stackinit: u16 FAIL (uninit bytes: 2)
test_stackinit: u32 FAIL (uninit bytes: 4)
test_stackinit: u64 FAIL (uninit bytes: 8)
test_stackinit: char_array FAIL (uninit bytes: 16)
test_stackinit: small_hole FAIL (uninit bytes: 24)
test_stackinit: big_hole FAIL (uninit bytes: 128)
test_stackinit: user FAIL (uninit bytes: 32)
test_stackinit: failures: 16

CONFIG_GCC_PLUGIN_STRUCTLEAK=y
This only tries to initialize structs with __user markings:

test_stackinit: small_hole_static_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_static_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_dynamic_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_dynamic_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_runtime_partial FAIL (uninit bytes: 23)
test_stackinit: big_hole_runtime_partial FAIL (uninit bytes: 127)
test_stackinit: small_hole_runtime_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_runtime_all FAIL (uninit bytes: 61)
test_stackinit: u8 FAIL (uninit bytes: 1)
test_stackinit: u16 FAIL (uninit bytes: 2)
test_stackinit: u32 FAIL (uninit bytes: 4)
test_stackinit: u64 FAIL (uninit bytes: 8)
test_stackinit: char_array FAIL (uninit bytes: 16)
test_stackinit: small_hole FAIL (uninit bytes: 24)
test_stackinit: big_hole FAIL (uninit bytes: 128)
test_stackinit: user ok
test_stackinit: failures: 15

CONFIG_GCC_PLUGIN_STRUCTLEAK=y
CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL=y
This initializes all structures passed by reference (scalars and strings
remain uninitialized, but padding is wiped):

test_stackinit: small_hole_static_all ok
test_stackinit: big_hole_static_all ok
test_stackinit: small_hole_dynamic_all ok
test_stackinit: big_hole_dynamic_all ok
test_stackinit: small_hole_runtime_partial ok
test_stackinit: big_hole_runtime_partial ok
test_stackinit: small_hole_runtime_all ok
test_stackinit: big_hole_runtime_all ok
test_stackinit: u8 FAIL (uninit bytes: 1)
test_stackinit: u16 FAIL (uninit bytes: 2)
test_stackinit: u32 FAIL (uninit bytes: 4)
test_stackinit: u64 FAIL (uninit bytes: 8)
test_stackinit: char_array FAIL (uninit bytes: 16)
test_stackinit: small_hole ok
test_stackinit: big_hole ok
test_stackinit: user ok
test_stackinit: failures: 5

CONFIG_GCC_PLUGIN_STACKINIT=y
This initializes all variables, but has no special padding handling:

test_stackinit: small_hole_static_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_static_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_dynamic_all FAIL (uninit bytes: 3)
test_stackinit: big_hole_dynamic_all FAIL (uninit bytes: 61)
test_stackinit: small_hole_runtime_partial ok
test_stackinit: big_hole_runtime_partial ok
test_stackinit: small_hole_runtime_all ok
test_stackinit: big_hole_runtime_all ok
test_stackinit: u8 ok
test_stackinit: u16 ok
test_stackinit: u32 ok
test_stackinit: u64 ok
test_stackinit: char_array ok
test_stackinit: small_hole ok
test_stackinit: big_hole ok
test_stackinit: user ok
test_stackinit: failures: 4

Signed-off-by: Kees Cook 
---
 lib/Kconfig.debug|   9 ++
 lib/Makefile |   1 +
 lib/test_stackinit.c | 327 +++
 3 files changed, 337 insertions(+)
 create mode 100644 lib/test_stackinit.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d4df5b24d75e..09788af9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2001,6 +2001,15 @@ config TEST_OBJAGG
 
  If unsure, say N.
 
+config TEST_STACKINIT
+   tristate "Test level of stack variabl

[Xen-devel] [PATCH 0/3] gcc-plugins: Introduce stackinit plugin

[Kees Cook]

This adds a new plugin "stackinit" that attempts to perform unconditional
initialization of all stack variables[1]. It has wider effects than
GCC_PLUGIN_STRUCTLEAK_BYREF_ALL=y since BYREF_ALL does not consider
non-structures. A notable weakness is that padding bytes in many cases
remain uninitialized since GCC treats these bytes as "undefined". I'm
hoping we can improve the compiler (or the plugin) to cover that too.
(It's worth noting that BYREF_ALL actually does handle the padding --
I think this is due to the different method of detecting if initialization
is needed.)

Included is a tree-wide change to move switch variables up and out of
their switch and into the top-level variable declarations.

Included is a set of test cases for evaluating stack initialization,
which checks for padding, different types, etc.

Feedback welcome! :)

-Kees

[1] 
https://lkml.kernel.org/r/CA+55aFykZL+cSBJjBBts7ebEFfyGPdMzTmLSxKnT_29=j94...@mail.gmail.com

Kees Cook (3):
  treewide: Lift switch variables out of switches
  gcc-plugins: Introduce stackinit plugin
  lib: Introduce test_stackinit module

 arch/x86/xen/enlighten_pv.c   |   7 +-
 drivers/char/pcmcia/cm4000_cs.c   |   2 +-
 drivers/char/ppdev.c  |  20 +-
 drivers/gpu/drm/drm_edid.c|   4 +-
 drivers/gpu/drm/i915/intel_display.c  |   2 +-
 drivers/gpu/drm/i915/intel_pm.c   |   4 +-
 drivers/net/ethernet/intel/e1000/e1000_main.c |   3 +-
 drivers/tty/n_tty.c   |   3 +-
 drivers/usb/gadget/udc/net2280.c  |   5 +-
 fs/fcntl.c|   3 +-
 lib/Kconfig.debug |   9 +
 lib/Makefile  |   1 +
 lib/test_stackinit.c  | 327 ++
 mm/shmem.c|   5 +-
 net/core/skbuff.c |   4 +-
 net/ipv6/ip6_gre.c|   4 +-
 net/ipv6/ip6_tunnel.c |   4 +-
 net/openvswitch/flow_netlink.c|   7 +-
 scripts/Makefile.gcc-plugins  |   6 +
 scripts/gcc-plugins/Kconfig   |   9 +
 scripts/gcc-plugins/gcc-common.h  |  11 +-
 scripts/gcc-plugins/stackinit_plugin.c|  79 +
 security/tomoyo/common.c  |   3 +-
 security/tomoyo/condition.c   |   7 +-
 security/tomoyo/util.c|   4 +-
 25 files changed, 484 insertions(+), 49 deletions(-)
 create mode 100644 lib/test_stackinit.c
 create mode 100644 scripts/gcc-plugins/stackinit_plugin.c

-- 
2.17.1


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

[Xen-devel] [PATCH 2/3] gcc-plugins: Introduce stackinit plugin

[Kees Cook]

This attempts to duplicate the proposed gcc option -finit-local-vars[1]
in an effort to implement the "always initialize local variables" kernel
development goal[2].

Enabling CONFIG_GCC_PLUGIN_STACKINIT should stop all "uninitialized
stack variable" flaws as long as they don't depend on being zero. :)

[1] https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00615.html
[2] 
https://lkml.kernel.org/r/CA+55aFykZL+cSBJjBBts7ebEFfyGPdMzTmLSxKnT_29=j94...@mail.gmail.com

Signed-off-by: Kees Cook 
---
 scripts/Makefile.gcc-plugins   |  6 ++
 scripts/gcc-plugins/Kconfig|  9 +++
 scripts/gcc-plugins/gcc-common.h   | 11 +++-
 scripts/gcc-plugins/stackinit_plugin.c | 79 ++
 4 files changed, 102 insertions(+), 3 deletions(-)
 create mode 100644 scripts/gcc-plugins/stackinit_plugin.c

diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 35042d96cf5d..2483121d781c 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -12,6 +12,12 @@ export DISABLE_LATENT_ENTROPY_PLUGIN
 
 gcc-plugin-$(CONFIG_GCC_PLUGIN_SANCOV) += sancov_plugin.so
 
+gcc-plugin-$(CONFIG_GCC_PLUGIN_STACKINIT)  += stackinit_plugin.so
+ifdef CONFIG_GCC_PLUGIN_STACKINIT
+DISABLE_STACKINIT_PLUGIN += -fplugin-arg-stackinit_plugin-disable
+endif
+export DISABLE_STACKINIT_PLUGIN
+
 gcc-plugin-$(CONFIG_GCC_PLUGIN_STRUCTLEAK) += structleak_plugin.so
 gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE)  \
+= -fplugin-arg-structleak_plugin-verbose
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index d45f7f36b859..b117fe83f1d3 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -66,6 +66,15 @@ config GCC_PLUGIN_LATENT_ENTROPY
   * https://grsecurity.net/
   * https://pax.grsecurity.net/
 
+config GCC_PLUGIN_STACKINIT
+   bool "Initialize all stack variables to zero by default"
+   depends on GCC_PLUGINS
+   depends on !GCC_PLUGIN_STRUCTLEAK
+   help
+ This plugin zero-initializes all stack variables. This is more
+ comprehensive than GCC_PLUGIN_STRUCTLEAK, and attempts to
+ duplicate the proposed -finit-local-vars gcc build flag.
+
 config GCC_PLUGIN_STRUCTLEAK
bool "Force initialization of variables containing userspace addresses"
# Currently STRUCTLEAK inserts initialization out of live scope of
diff --git a/scripts/gcc-plugins/gcc-common.h b/scripts/gcc-plugins/gcc-common.h
index 552d5efd7cb7..f690b4deeabd 100644
--- a/scripts/gcc-plugins/gcc-common.h
+++ b/scripts/gcc-plugins/gcc-common.h
@@ -76,6 +76,14 @@
 #include "c-common.h"
 #endif
 
+#if BUILDING_GCC_VERSION > 4005
+#include "c-tree.h"
+#else
+/* should come from c-tree.h if only it were installed for gcc 4.5... */
+#define C_TYPE_FIELDS_READONLY(TYPE) TREE_LANG_FLAG_1(TYPE)
+extern bool global_bindings_p (void);
+#endif
+
 #if BUILDING_GCC_VERSION <= 4008
 #include "tree-flow.h"
 #else
@@ -158,9 +166,6 @@ void dump_gimple_stmt(pretty_printer *, gimple, int, int);
 #define TYPE_NAME_POINTER(node) IDENTIFIER_POINTER(TYPE_NAME(node))
 #define TYPE_NAME_LENGTH(node) IDENTIFIER_LENGTH(TYPE_NAME(node))
 
-/* should come from c-tree.h if only it were installed for gcc 4.5... */
-#define C_TYPE_FIELDS_READONLY(TYPE) TREE_LANG_FLAG_1(TYPE)
-
 static inline tree build_const_char_string(int len, const char *str)
 {
tree cstr, elem, index, type;
diff --git a/scripts/gcc-plugins/stackinit_plugin.c 
b/scripts/gcc-plugins/stackinit_plugin.c
new file mode 100644
index ..41055cd7098e
--- /dev/null
+++ b/scripts/gcc-plugins/stackinit_plugin.c
@@ -0,0 +1,79 @@
+/* SPDX-License: GPLv2 */
+/*
+ * This will zero-initialize local stack variables. (Though structure
+ * padding may remain uninitialized in certain cases.)
+ *
+ * Implements Florian Weimer's "-finit-local-vars" gcc patch as a plugin:
+ * https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00615.html
+ *
+ * Plugin skeleton code thanks to PaX Team.
+ *
+ * Options:
+ * -fplugin-arg-stackinit_plugin-disable
+ */
+
+#include "gcc-common.h"
+
+__visible int plugin_is_GPL_compatible;
+
+static struct plugin_info stackinit_plugin_info = {
+   .version= "20190122",
+   .help   = "disable\tdo not activate plugin\n",
+};
+
+static void finish_decl(void *event_data, void *data)
+{
+   tree decl = (tree)event_data;
+   tree type;
+
+   if (TREE_CODE (decl) != VAR_DECL)
+   return;
+
+   if (DECL_EXTERNAL (decl))
+   return;
+
+   if (DECL_INITIAL (decl) != NULL_TREE)
+   return;
+
+   if (global_bindings_p ())
+   return;
+
+   type = TREE_TYPE (decl);
+   if (AGGREGATE_TYPE_P (type))
+   DECL_INITIAL (decl) = build_constructor (type, NULL);
+   else
+   DECL_INITIAL (decl) = fold_convert (type, integer_zero_node);
+}
+
+__visible int plugin_init(struct plugin_name_args *plu

Re: [Xen-devel] [PATCH v4 1/3] docs: Improve documentation and parsing for efi=

[Jan Beulich]

>>> On 21.01.19 at 20:02,  wrote:
> Update parse_efi_param() to use parse_boolean() for "rs", so it behaves
> like other Xen booleans.
> 
> However, change "attr=uc" to not be a boolean.  "no-attr=uc" is ambiguous and
> shouldn't be accepted, but accept "attr=no" as an acceptable alternative.
> 
> Update the command line documentation for consistency.
> 
> Signed-off-by: Andrew Cooper 
> Release-acked-by: Juergen Gross 

Reviewed-by: Jan Beulich 



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

Re: [Xen-devel] [PATCH v4 2/3] xen/dom0: Deprecate iommu_hwdom_inclusive and leave it disabled by default

[Jan Beulich]

>>> On 21.01.19 at 20:02,  wrote:
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -38,7 +38,7 @@ bool_t __read_mostly iommu_intremap = 1;
>  
>  bool __hwdom_initdata iommu_hwdom_strict;
>  bool __read_mostly iommu_hwdom_passthrough;
> -int8_t __hwdom_initdata iommu_hwdom_inclusive = -1;
> +int8_t __hwdom_initdata iommu_hwdom_inclusive;

I guess this could become bool now, but either way
Reviewed-by: Jan Beulich 

Jan



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

Re: [Xen-devel] [PATCH 1/3] x86/pvh-dom0: Remove unnecessary function pointer call from modify_identity_mmio()

[Jan Beulich]

>>> On 21.01.19 at 16:52,  wrote:
> On Mon, Jan 21, 2019 at 03:37:19PM +, Andrew Cooper wrote:
>> Function pointer calls are far more expensive in a post-Spectre world, and
>> this one doesn't need to be.
>> 
>> No functional change.
>> 
>> Signed-off-by: Andrew Cooper 
> 
> Reviewed-by: Wei Liu 

Acked-by: Jan Beulich 

Arguably a compiler could be intelligent enough to use direct calls
despite the ?: construct, though.

Jan



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

Re: [Xen-devel] [PATCH for-4.12 0/3] x86/pvh: Misc fixes and cleanup

[Juergen Gross]

On 21/01/2019 16:37, Andrew Cooper wrote:
> These are all minor fixes from some recent PVH work.  They are all limited to
> the boot path, and low-risk nice-to-have's for 4.12.
> 
> Andrew Cooper (3):
>   x86/pvh-dom0: Remove unnecessary function pointer call from 
> modify_identity_mmio()
>   x86/pvh: Fixes to convert_pvh_info()
>   x86/pvh: Print the PVH start info more concisely
> 
>  xen/arch/x86/guest/pvh-boot.c | 48 
> +--
>  xen/arch/x86/hvm/dom0_build.c |  4 ++--
>  2 files changed, 30 insertions(+), 22 deletions(-)
> 

For the series:

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [RFC] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Julien Grall]

(+ Paul)

Hello,

On 23/01/2019 10:12, Andrii Anisov wrote:

From: Andrii Anisov 

Taking decission by `need_iommu_pt_sync()` make us never kicking


s/decission/decision/


`iommu_iotlb_flush()` for IOMMUs which do share TLB with CPU.


I am not aware of platform where we share the TLB with the CPU. Do you mean 
sharing the P2M?



So check `has_iommu_pt()` instead.

Signed-off-by: Andrii Anisov 

---

Julien,

Could you please look at this, IMO there is a mistake here.
x86 uses `need_iommu_pt_sync()` to make decission if iommu's map/unmap should 
be additionally called.
But ARM has no non-shared pt support in the mainline, so using 
`need_iommu_pt_sync()` seems to be odd.

  xen/arch/arm/p2m.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 2394f97..059a391 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1019,7 +1019,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
   !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) )
  p2m_free_entry(p2m, orig_pte, level);
  
-if ( need_iommu_pt_sync(p2m->domain) &&

+if ( has_iommu_pt(p2m->domain) &&


I think this makes sense because we want to flush the TLB when the P2M is 
shared. Although, I would like to hear Paul's opinion here.



   (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
  {
  unsigned int flush_flags = 0; >


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 2/3] x86/pvh: Fixes to convert_pvh_info()

[Jan Beulich]

>>> On 21.01.19 at 16:37,  wrote:
> --- a/xen/arch/x86/guest/pvh-boot.c
> +++ b/xen/arch/x86/guest/pvh-boot.c
> @@ -38,12 +38,20 @@ static const char *__initdata pvh_loader = "PVH 
> Directboot";
>  static void __init convert_pvh_info(multiboot_info_t **mbi,
>  module_t **mod)
>  {
> -const struct hvm_start_info *pvh_info = __va(pvh_start_info_pa);
> +struct hvm_start_info *pvh_info = __va(pvh_start_info_pa);
>  const struct hvm_modlist_entry *entry;
>  unsigned int i;
>  
>  if ( pvh_info->magic != XEN_HVM_START_MAGIC_VALUE )
> -panic("Magic value is wrong: %x\n", pvh_info->magic);
> +panic("PVH magic value is wrong: %x\n", pvh_info->magic);
> +
> +/* Check consistency between the modlist and number of modules. */
> +if ( (pvh_info->modlist_paddr == 0) != (pvh_info->nr_modules == 0) )
> +{
> +printk("PVH module mismatch: pa %08"PRIx64", nr %u - Ignoring\n",
> +   pvh_info->modlist_paddr, pvh_info->nr_modules);
> +pvh_info->modlist_paddr = pvh_info->nr_modules = 0;
> +}

While we don't consume memmap_{paddr,entries} (yet), wouldn't
it make sense to also check those for similar consistency?

Furthermore I'm not convinced the check above is correct: I don't
see anything wrong with a random modlist_paddr as long as
nr_modules is zero. In particular it is not uncommon for placement
implementations to assign the next sequential address to the next
item to process before looking at or iterating over the number of
associated entries.

Jan



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

Re: [Xen-devel] [PATCH 3/3] x86/pvh: Print the PVH start info more concisely

[Jan Beulich]

>>> On 21.01.19 at 16:37,  wrote:
> The current rendering of PVH start info in unnecessarily verbose, and doesn't
> clearly separate decimal and hex numbers.

As expressed on earlier occasions, I think blindly adding 0x in
all cases goes too far. When context makes sufficiently clear
that it's a hex number (like when addresses get printed) I don't
see the need. Nevertheless, knowing you disagree,
Acked-by: Jan Beulich 
with two instances of one further question below.

> --- a/xen/arch/x86/guest/pvh-boot.c
> +++ b/xen/arch/x86/guest/pvh-boot.c
> @@ -123,28 +123,29 @@ void __init pvh_print_info(void)
>  const struct hvm_modlist_entry *entry;
>  unsigned int i;
>  
> -ASSERT(pvh_info->magic == XEN_HVM_START_MAGIC_VALUE);
> -
> -printk("PVH start info: (pa %08x)\n", pvh_start_info_pa);
> -printk("  version:%u\n", pvh_info->version);
> -printk("  flags:  %#"PRIx32"\n", pvh_info->flags);
> -printk("  nr_modules: %u\n", pvh_info->nr_modules);
> -printk("  modlist_pa: %016"PRIx64"\n", pvh_info->modlist_paddr);
> -printk("  cmdline_pa: %016"PRIx64"\n", pvh_info->cmdline_paddr);
> +printk("PVH start info: (pa 0x%08x)\n", pvh_start_info_pa);
> +printk("  version %u, flags %#x\n", pvh_info->version, pvh_info->flags);
> +
> +printk("  cmdline 0x%08"PRIx64, pvh_info->cmdline_paddr);
>  if ( pvh_info->cmdline_paddr )
> -printk("  cmdline:'%s'\n", (char 
> *)__va(pvh_info->cmdline_paddr));
> -printk("  rsdp_pa:%016"PRIx64"\n", pvh_info->rsdp_paddr);
> +printk(" '%s'", (char *)__va(pvh_info->cmdline_paddr));

Is the cast here really necessary?

> +printk("\n");
> +
> +printk("  rsdp0x%08"PRIx64"\n", pvh_info->rsdp_paddr);
> +
> +printk("  modlist 0x%08"PRIx64", nr %u\n",
> +   pvh_info->modlist_paddr, pvh_info->nr_modules);
>  
>  entry = __va(pvh_info->modlist_paddr);
>  for ( i = 0; i < pvh_info->nr_modules; i++ )
>  {
> -printk("mod[%u].pa: %016"PRIx64"\n", i, entry[i].paddr);
> -printk("mod[%u].size:   %016"PRIu64"\n", i, entry[i].size);
> -printk("mod[%u].cmdline_pa: %016"PRIx64"\n",
> -   i, entry[i].cmdline_paddr);
> +printk("mod%u pa 0x%08"PRIx64", sz 0x%08"PRIx64", cmdline 0x%08" 
> PRIx64,
> +   i, entry[i].paddr, entry[i].size, entry[i].cmdline_paddr);
> +
>  if ( entry[i].cmdline_paddr )
> -printk("mod[%1u].cmdline:'%s'\n", i,
> -   (char *)__va(entry[i].cmdline_paddr));
> +printk(" '%s'\n", (char *)__va(entry[i].cmdline_paddr));

Same here then.

Jan



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

Re: [Xen-devel] [RFC] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Paul Durrant]

> -Original Message-
> From: Julien Grall [mailto:julien.gr...@arm.com]
> Sent: 23 January 2019 11:34
> To: Andrii Anisov ; xen-
> de...@lists.xenproject.org
> Cc: Stefano Stabellini ; Andrii Anisov
> ; Paul Durrant 
> Subject: Re: [RFC] arm/p2m: call iommu iotlb flush if iommu exists and
> enabled
> 
> (+ Paul)
> 
> Hello,
> 
> On 23/01/2019 10:12, Andrii Anisov wrote:
> > From: Andrii Anisov 
> >
> > Taking decission by `need_iommu_pt_sync()` make us never kicking
> 
> s/decission/decision/
> 
> > `iommu_iotlb_flush()` for IOMMUs which do share TLB with CPU.
> 
> I am not aware of platform where we share the TLB with the CPU. Do you
> mean
> sharing the P2M?
> 
> > So check `has_iommu_pt()` instead.
> >
> > Signed-off-by: Andrii Anisov 
> >
> > ---
> >
> > Julien,
> >
> > Could you please look at this, IMO there is a mistake here.
> > x86 uses `need_iommu_pt_sync()` to make decission if iommu's map/unmap
> should be additionally called.
> > But ARM has no non-shared pt support in the mainline, so using
> `need_iommu_pt_sync()` seems to be odd.
> >
> >   xen/arch/arm/p2m.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > index 2394f97..059a391 100644
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -1019,7 +1019,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
> >!mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) )
> >   p2m_free_entry(p2m, orig_pte, level);
> >
> > -if ( need_iommu_pt_sync(p2m->domain) &&
> > +if ( has_iommu_pt(p2m->domain) &&
> 
> I think this makes sense because we want to flush the TLB when the P2M is
> shared. Although, I would like to hear Paul's opinion here.

Yes, this was a mistake when moving from the old macros and need_iommu. Andrii 
is correct that need_iommu_pt_sync() is supposed to gate whether an explicit 
map/unmap is needed. The need for flush should only depend on has_iommu_pt(). 
There is also iommu_use_hap_pt() which is actually defined as has_iommu_pt(), 
but I wonder whether that should just go away for ARM since the page tables are 
always shared.

  Paul

> 
> >(lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
> >   {
> >   unsigned int flush_flags = 0; >
> 
> Cheers,
> 
> --
> Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH SpectreV1+L1TF v4 02/11] is_hvm/pv_domain: block speculation

[Norbert Manthey]

When checking for being an hvm domain, or PV domain, we have to make
sure that speculation cannot bypass that check, and eventually access
data that should not end up in cache for the current domain type.

Signed-off-by: Norbert Manthey 

---
 xen/include/xen/sched.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -892,7 +892,8 @@ void watchdog_domain_destroy(struct domain *d);
 
 static inline bool is_pv_domain(const struct domain *d)
 {
-return IS_ENABLED(CONFIG_PV) ? d->guest_type == guest_type_pv : false;
+return IS_ENABLED(CONFIG_PV)
+   ? evaluate_nospec(d->guest_type == guest_type_pv) : false;
 }
 
 static inline bool is_pv_vcpu(const struct vcpu *v)
@@ -923,7 +924,8 @@ static inline bool is_pv_64bit_vcpu(const struct vcpu *v)
 #endif
 static inline bool is_hvm_domain(const struct domain *d)
 {
-return IS_ENABLED(CONFIG_HVM) ? d->guest_type == guest_type_hvm : false;
+return IS_ENABLED(CONFIG_HVM)
+   ? evaluate_nospec(d->guest_type == guest_type_hvm) : false;
 }
 
 static inline bool is_hvm_vcpu(const struct vcpu *v)
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH SpectreV1+L1TF v4 05/11] common/grant_table: block speculative out-of-bound accesses

[Norbert Manthey]

Guests can issue grant table operations and provide guest controlled
data to them. This data is also used for memory loads. To avoid
speculative out-of-bound accesses, we use the array_index_nospec macro
where applicable. However, there are also memory accesses that cannot
be protected by a single array protection, or multiple accesses in a
row. To protect these, an lfence instruction is placed between the
actual range check and the access via the newly introduced macro
block_speculation.

This commit is part of the SpectreV1+L1TF mitigation patch series.

Signed-off-by: Norbert Manthey 

---
 xen/common/grant_table.c | 23 +--
 xen/include/xen/nospec.h |  9 +
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -963,6 +964,9 @@ map_grant_ref(
 PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref %#x for d%d\n",
  op->ref, rgt->domain->domain_id);
 
+/* Make sure the above check is not bypassed speculatively */
+op->ref = array_index_nospec(op->ref, nr_grant_entries(rgt));
+
 act = active_entry_acquire(rgt, op->ref);
 shah = shared_entry_header(rgt, op->ref);
 status = rgt->gt_version == 1 ? &shah->flags : &status_entry(rgt, op->ref);
@@ -1268,7 +1272,8 @@ unmap_common(
 }
 
 smp_rmb();
-map = &maptrack_entry(lgt, op->handle);
+map = &maptrack_entry(lgt, array_index_nospec(op->handle,
+  lgt->maptrack_limit));
 
 if ( unlikely(!read_atomic(&map->flags)) )
 {
@@ -2026,6 +2031,9 @@ gnttab_prepare_for_transfer(
 goto fail;
 }
 
+/* Make sure the above check is not bypassed speculatively */
+ref = array_index_nospec(ref, nr_grant_entries(rgt));
+
 sha = shared_entry_header(rgt, ref);
 
 scombo.word = *(u32 *)&sha->flags;
@@ -2223,7 +2231,8 @@ gnttab_transfer(
 okay = gnttab_prepare_for_transfer(e, d, gop.ref);
 spin_lock(&e->page_alloc_lock);
 
-if ( unlikely(!okay) || unlikely(e->is_dying) )
+/* Make sure this check is not bypassed speculatively */
+if ( evaluate_nospec(unlikely(!okay) || unlikely(e->is_dying)) )
 {
 bool_t drop_dom_ref = !domain_adjust_tot_pages(e, -1);
 
@@ -2408,6 +2417,9 @@ acquire_grant_for_copy(
 PIN_FAIL(gt_unlock_out, GNTST_bad_gntref,
  "Bad grant reference %#x\n", gref);
 
+/* Make sure the above check is not bypassed speculatively */
+gref = array_index_nospec(gref, nr_grant_entries(rgt));
+
 act = active_entry_acquire(rgt, gref);
 shah = shared_entry_header(rgt, gref);
 if ( rgt->gt_version == 1 )
@@ -2826,6 +2838,9 @@ static int gnttab_copy_buf(const struct gnttab_copy *op,
  op->dest.offset, dest->ptr.offset,
  op->len, dest->len);
 
+/* Make sure the above checks are not bypassed speculatively */
+block_speculation();
+
 memcpy(dest->virt + op->dest.offset, src->virt + op->source.offset,
op->len);
 gnttab_mark_dirty(dest->domain, dest->mfn);
@@ -3215,6 +3230,10 @@ swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b)
 if ( ref_a == ref_b )
 goto out;
 
+/* Make sure the above check is not bypassed speculatively */
+ref_a = array_index_nospec(ref_a, nr_grant_entries(d->grant_table));
+ref_b = array_index_nospec(ref_b, nr_grant_entries(d->grant_table));
+
 act_a = active_entry_acquire(gt, ref_a);
 if ( act_a->pin )
 PIN_FAIL(out, GNTST_eagain, "ref a %#x busy\n", ref_a);
diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -87,6 +87,15 @@ static inline bool lfence_true(void) { return true; }
 #define evaluate_nospec(condition) ({ bool res = (condition); rmb(); res; })
 #endif
 
+/*
+ * allow to block speculative execution in generic code
+ */
+#ifdef CONFIG_X86
+#define block_speculation() rmb()
+#else
+#define block_speculation()
+#endif
+
 #endif /* XEN_NOSPEC_H */
 
 /*
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH SpectreV1+L1TF v4 03/11] config: introduce L1TF_LFENCE option

[Norbert Manthey]

This commit introduces the configuration option L1TF_LFENCE that allows
to control the implementation of the protection of privilege checks via
lfence instructions. The following four alternatives are provided:

 - not injecting lfence instructions
 - inject an lfence instruction for both outcomes of the conditional
 - inject an lfence instruction only if the conditional would evaluate
   to true, so that this case cannot be entered under speculation
 - evaluating the condition and store the result into a local variable.
   before using this value, inject an lfence instruction.

The different options allow to control the level of protection vs the
slowdown the addtional lfence instructions would introduce. The default
value is set to protecting both branches.

For non-x86 platforms, the protection is disabled by default.

Signed-off-by: Norbert Manthey 

---
 xen/arch/x86/Kconfig | 24 
 xen/include/xen/nospec.h | 12 ++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -176,6 +176,30 @@ config PV_SHIM_EXCLUSIVE
  firmware, and will not function correctly in other scenarios.
 
  If unsure, say N.
+
+choice
+   prompt "Default L1TF Branch Protection?"
+
+   config L1TF_LFENCE_BOTH
+   bool "Protect both branches of certain conditionals" if HVM
+   ---help---
+ Inject an lfence instruction after the condition to be
+ evaluated for both outcomes of the condition
+   config L1TF_LFENCE_TRUE
+   bool "Protect true branch of certain conditionals" if HVM
+   ---help---
+ Protect only the path where the condition is evaluated to true
+   config L1TF_LFENCE_INTERMEDIATE
+   bool "Protect before using certain conditionals value" if HVM
+   ---help---
+ Inject an lfence instruction after evaluating the condition
+ but before forwarding this value from a local variable
+   config L1TF_LFENCE_NONE
+   bool "No conditional protection"
+   ---help---
+ Do not inject lfences for conditional evaluations
+endchoice
+
 endmenu
 
 source "common/Kconfig"
diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -68,10 +68,18 @@ static inline bool lfence_true(void) { return true; }
 #endif
 
 /*
- * protect evaluation of conditional with respect to speculation
+ * allow to protect evaluation of conditional with respect to speculation on 
x86
  */
-#define evaluate_nospec(condition)  \
+#if defined(CONFIG_L1TF_LFENCE_NONE) || !defined(CONFIG_X86)
+#define evaluate_nospec(condition) (condition)
+#elif defined(CONFIG_L1TF_LFENCE_BOTH)
+#define evaluate_nospec(condition) \
 (((condition) && lfence_true()) || !lfence_true())
+#elif defined(CONFIG_L1TF_LFENCE_TRUE)
+#define evaluate_nospec(condition) ((condition) && lfence_true())
+#elif defined(CONFIG_L1TF_LFENCE_INTERMEDIATE)
+#define evaluate_nospec(condition) ({ bool res = (condition); rmb(); res; })
+#endif
 
 #endif /* XEN_NOSPEC_H */
 
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] SpectreV1+L1TF Patch Series

[Norbert Manthey]

Dear all,

This patch series attempts to mitigate the issue that have been raised in the
XSA-289 (https://xenbits.xen.org/xsa/advisory-289.html). To block speculative
execution on Intel hardware, an lfence instruction is required to make sure
that selected checks are not bypassed. Speculative out-of-bound accesses can
be prevented by using the array_index_nospec macro.

The lfence instruction should be added on x86 platforms only. To not affect
platforms that are not affected by the L1TF vulnerability, the lfence
instruction is patched in via alternative patching on Intel CPUs only.
Furthermore, the compile time configuration allows to choose how to protect the
evaluation of conditions with the lfence instruction.

Best,
Norbert




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH SpectreV1+L1TF v4 01/11] is_control_domain: block speculation

[Norbert Manthey]

Checks of domain properties, such as is_hardware_domain or is_hvm_domain,
might be bypassed by speculatively executing these instructions. A reason
for bypassing these checks is that these macros access the domain
structure via a pointer, and check a certain field. Since this memory
access is slow, the CPU assumes a returned value and continues the
execution.

In case an is_control_domain check is bypassed, for example during a
hypercall, data that should only be accessible by the control domain could
be loaded into the cache.

Since the L1TF vulnerability of Intel CPUs, loading hypervisor data into
L1 cache is problemetic, because when hyperthreading is used as well, a
guest running on the sibling core can leak this potentially secret data.

To prevent these speculative accesses, we block speculation after
accessing the domain property field by adding lfence instructions. This
way, the CPU continues executing and loading data only once the condition
is actually evaluated.

As the macros are typically used in if statements, the lfence has to come
in a compatible way. Therefore, a function that returns true after an
lfence instruction is introduced. To protect both branches after a
conditional, an lfence instruction has to be added for the two branches.
As the L1TF vulnerability is only present on the x86 architecture, the
macros will not use the lfence instruction on other architectures.

Introducing the lfence instructions catches a lot of potential leaks with
a simple unintrusive code change. During performance testing, we did not
notice performance effects.

Signed-off-by: Norbert Manthey 

---
 xen/include/xen/nospec.h | 15 +++
 xen/include/xen/sched.h  |  5 +++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -58,6 +58,21 @@ static inline unsigned long array_index_mask_nospec(unsigned 
long index,
 (typeof(_i)) (_i & _mask);  \
 })
 
+/*
+ * allow to insert a read memory barrier into conditionals
+ */
+#ifdef CONFIG_X86
+static inline bool lfence_true(void) { rmb(); return true; }
+#else
+static inline bool lfence_true(void) { return true; }
+#endif
+
+/*
+ * protect evaluation of conditional with respect to speculation
+ */
+#define evaluate_nospec(condition)  \
+(((condition) && lfence_true()) || !lfence_true())
+
 #endif /* XEN_NOSPEC_H */
 
 /*
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -882,10 +883,10 @@ void watchdog_domain_destroy(struct domain *d);
  *(that is, this would not be suitable for a driver domain)
  *  - There is never a reason to deny the hardware domain access to this
  */
-#define is_hardware_domain(_d) ((_d) == hardware_domain)
+#define is_hardware_domain(_d) evaluate_nospec((_d) == hardware_domain)
 
 /* This check is for functionality specific to a control domain */
-#define is_control_domain(_d) ((_d)->is_privileged)
+#define is_control_domain(_d) evaluate_nospec((_d)->is_privileged)
 
 #define VM_ASSIST(d, t) (test_bit(VMASST_TYPE_ ## t, &(d)->vm_assist))
 
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH SpectreV1+L1TF v4 06/11] common/memory: block speculative out-of-bound accesses

[Norbert Manthey]

The get_page_from_gfn method returns a pointer to a page that belongs
to a gfn. Before returning the pointer, the gfn is checked for being
valid. Under speculation, these checks can be bypassed, so that
the function get_page is still executed partially. Consequently, the
function page_get_owner_and_reference might be executed partially as
well. In this function, the computed pointer is accessed, resulting in
a speculative out-of-bound address load. As the gfn can be controlled by
a guest, this access is problematic.

To mitigate the root cause, an lfence instruction is added via the
evaluate_nospec macro. To make the protection generic, we do not
introduce the lfence instruction for this single check, but add it to
the mfn_valid function. This way, other potentially problematic accesses
are protected as well.

This commit is part of the SpectreV1+L1TF mitigation patch series.

Signed-off-by: Norbert Manthey 

---
 xen/common/pdx.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/common/pdx.c b/xen/common/pdx.c
--- a/xen/common/pdx.c
+++ b/xen/common/pdx.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Parameters for PFN/MADDR compression. */
 unsigned long __read_mostly max_pdx;
@@ -33,10 +34,10 @@ unsigned long __read_mostly pdx_group_valid[BITS_TO_LONGS(
 
 bool __mfn_valid(unsigned long mfn)
 {
-return likely(mfn < max_page) &&
-   likely(!(mfn & pfn_hole_mask)) &&
-   likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
-   pdx_group_valid));
+return evaluate_nospec(likely(mfn < max_page) &&
+   likely(!(mfn & pfn_hole_mask)) &&
+   likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
+   pdx_group_valid)));
 }
 
 /* Sets all bits from the most-significant 1-bit down to the LSB */
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH SpectreV1+L1TF v4 04/11] x86/hvm: block speculative out-of-bound accesses

[Norbert Manthey]

There are multiple arrays in the HVM interface that are accessed
with indices that are provided by the guest. To avoid speculative
out-of-bound accesses, we use the array_index_nospec macro.

When blocking speculative out-of-bound accesses, we can classify arrays
into dynamic arrays and static arrays. Where the former are allocated
during run time, the size of the latter is known during compile time.
On static arrays, compiler might be able to block speculative accesses
in the future.

We introduce another macro that uses the ARRAY_SIZE macro to block
speculative accesses. For arrays that are statically accessed, this macro
can be used instead of the usual macro. Using this macro results in more
readable code, and allows to modify the way this case is handled in a
single place.

This commit is part of the SpectreV1+L1TF mitigation patch series.

Reported-by: Pawel Wieczorkiewicz 
Signed-off-by: Norbert Manthey 

---
 xen/arch/x86/hvm/hvm.c   | 27 ++-
 xen/include/xen/nospec.h |  6 ++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2102,7 +2103,7 @@ int hvm_mov_from_cr(unsigned int cr, unsigned int gpr)
 case 2:
 case 3:
 case 4:
-val = curr->arch.hvm.guest_cr[cr];
+val = array_access_nospec(curr->arch.hvm.guest_cr, cr);
 break;
 case 8:
 val = (vlapic_get_reg(vcpu_vlapic(curr), APIC_TASKPRI) & 0xf0) >> 4;
@@ -3448,13 +3449,15 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t 
*msr_content)
 if ( !d->arch.cpuid->basic.mtrr )
 goto gp_fault;
 index = msr - MSR_MTRRfix16K_8;
-*msr_content = fixed_range_base[index + 1];
+*msr_content = fixed_range_base[array_index_nospec(index + 1,
+   ARRAY_SIZE(v->arch.hvm.mtrr.fixed_ranges))];
 break;
 case MSR_MTRRfix4K_C...MSR_MTRRfix4K_F8000:
 if ( !d->arch.cpuid->basic.mtrr )
 goto gp_fault;
 index = msr - MSR_MTRRfix4K_C;
-*msr_content = fixed_range_base[index + 3];
+*msr_content = fixed_range_base[array_index_nospec(index + 3,
+   ARRAY_SIZE(v->arch.hvm.mtrr.fixed_ranges))];
 break;
 case MSR_IA32_MTRR_PHYSBASE(0)...MSR_IA32_MTRR_PHYSMASK(MTRR_VCNT_MAX - 1):
 if ( !d->arch.cpuid->basic.mtrr )
@@ -3463,7 +3466,8 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t 
*msr_content)
 if ( (index / 2) >=
  MASK_EXTR(v->arch.hvm.mtrr.mtrr_cap, MTRRcap_VCNT) )
 goto gp_fault;
-*msr_content = var_range_base[index];
+*msr_content = var_range_base[array_index_nospec(index,
+  MASK_EXTR(v->arch.hvm.mtrr.mtrr_cap, MTRRcap_VCNT))];
 break;
 
 case MSR_IA32_XSS:
@@ -4026,7 +4030,8 @@ static int hvmop_set_evtchn_upcall_vector(
 if ( op.vector < 0x10 )
 return -EINVAL;
 
-if ( op.vcpu >= d->max_vcpus || (v = d->vcpu[op.vcpu]) == NULL )
+if ( op.vcpu >= d->max_vcpus ||
+(v = d->vcpu[array_index_nospec(op.vcpu, d->max_vcpus)]) == NULL )
 return -ENOENT;
 
 printk(XENLOG_G_INFO "%pv: upcall vector %02x\n", v, op.vector);
@@ -4114,6 +4119,12 @@ static int hvmop_set_param(
 if ( a.index >= HVM_NR_PARAMS )
 return -EINVAL;
 
+/*
+ * Make sure the guest controlled value a.index is bounded even during
+ * speculative execution.
+ */
+a.index = array_index_nospec(a.index, HVM_NR_PARAMS);
+
 d = rcu_lock_domain_by_any_id(a.domid);
 if ( d == NULL )
 return -ESRCH;
@@ -4380,6 +4391,12 @@ static int hvmop_get_param(
 if ( a.index >= HVM_NR_PARAMS )
 return -EINVAL;
 
+/*
+ * Make sure the guest controlled value a.index is bounded even during
+ * speculative execution.
+ */
+a.index = array_index_nospec(a.index, HVM_NR_PARAMS);
+
 d = rcu_lock_domain_by_any_id(a.domid);
 if ( d == NULL )
 return -ESRCH;
diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -59,6 +59,12 @@ static inline unsigned long array_index_mask_nospec(unsigned 
long index,
 })
 
 /*
+ * array_access_nospec - allow nospec access for static size arrays
+ */
+#define array_access_nospec(array, index)   \
+(array)[array_index_nospec(index, ARRAY_SIZE(array))]
+
+/*
  * allow to insert a read memory barrier into conditionals
  */
 #ifdef CONFIG_X86
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



___
Xen-de

[Xen-devel] [PATCH SpectreV1+L1TF v4 09/11] x86/vioapic: block speculative out-of-bound accesses

[Norbert Manthey]

When interacting with io apic, a guest can specify values that are used
as index to structures, and whose values are not compared against
upper bounds to prevent speculative out-of-bound accesses. This change
prevents these speculative accesses.

This commit is part of the SpectreV1+L1TF mitigation patch series.

Signed-off-by: Norbert Manthey 

---
 xen/arch/x86/hvm/vioapic.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
--- a/xen/arch/x86/hvm/vioapic.c
+++ b/xen/arch/x86/hvm/vioapic.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -66,6 +67,9 @@ static struct hvm_vioapic *gsi_vioapic(const struct domain *d,
 {
 unsigned int i;
 
+/* Make sure the compiler does not optimize the initialization */
+OPTIMIZER_HIDE_VAR(pin);
+
 for ( i = 0; i < d->arch.hvm.nr_vioapics; i++ )
 {
 struct hvm_vioapic *vioapic = domain_vioapic(d, i);
@@ -117,7 +121,8 @@ static uint32_t vioapic_read_indirect(const struct 
hvm_vioapic *vioapic)
 break;
 }
 
-redir_content = vioapic->redirtbl[redir_index].bits;
+redir_content = vioapic->redirtbl[array_index_nospec(redir_index,
+   vioapic->nr_pins)].bits;
 result = (vioapic->ioregsel & 1) ? (redir_content >> 32)
  : redir_content;
 break;
@@ -212,7 +217,12 @@ static void vioapic_write_redirent(
 struct hvm_irq *hvm_irq = hvm_domain_irq(d);
 union vioapic_redir_entry *pent, ent;
 int unmasked = 0;
-unsigned int gsi = vioapic->base_gsi + idx;
+unsigned int gsi;
+
+/* Make sure no out-of-bound value for idx can be used */
+idx = array_index_nospec(idx, vioapic->nr_pins);
+
+gsi = vioapic->base_gsi + idx;
 
 spin_lock(&d->arch.hvm.irq_lock);
 
@@ -378,7 +388,8 @@ static inline int pit_channel0_enabled(void)
 
 static void vioapic_deliver(struct hvm_vioapic *vioapic, unsigned int pin)
 {
-uint16_t dest = vioapic->redirtbl[pin].fields.dest_id;
+uint16_t dest = vioapic->redirtbl
+   [pin = array_index_nospec(pin, 
vioapic->nr_pins)].fields.dest_id;
 uint8_t dest_mode = vioapic->redirtbl[pin].fields.dest_mode;
 uint8_t delivery_mode = vioapic->redirtbl[pin].fields.delivery_mode;
 uint8_t vector = vioapic->redirtbl[pin].fields.vector;
@@ -463,7 +474,7 @@ static void vioapic_deliver(struct hvm_vioapic *vioapic, 
unsigned int pin)
 
 void vioapic_irq_positive_edge(struct domain *d, unsigned int irq)
 {
-unsigned int pin;
+unsigned int pin = 0; /* See gsi_vioapic */
 struct hvm_vioapic *vioapic = gsi_vioapic(d, irq, &pin);
 union vioapic_redir_entry *ent;
 
@@ -560,7 +571,7 @@ int vioapic_get_vector(const struct domain *d, unsigned int 
gsi)
 
 int vioapic_get_trigger_mode(const struct domain *d, unsigned int gsi)
 {
-unsigned int pin;
+unsigned int pin = 0; /* See gsi_vioapic */
 const struct hvm_vioapic *vioapic = gsi_vioapic(d, gsi, &pin);
 
 if ( !vioapic )
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH SpectreV1+L1TF v4 10/11] x86/hvm/hpet: block speculative out-of-bound accesses

[Norbert Manthey]

When interacting with hpet, read and write operations can be executed
during instruction emulation, where the guest controls the data that
is used. As it is hard to predict the number of instructions that are
executed speculatively, we prevent out-of-bound accesses by using the
array_index_nospec function for guest specified addresses that should
be used for hpet operations.

This commit is part of the SpectreV1+L1TF mitigation patch series.

Signed-off-by: Norbert Manthey 

---
 xen/arch/x86/hvm/hpet.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define domain_vhpet(x) (&(x)->arch.hvm.pl_time->vhpet)
 #define vcpu_vhpet(x)   (domain_vhpet((x)->domain))
@@ -124,15 +125,17 @@ static inline uint64_t hpet_read64(HPETState *h, unsigned 
long addr,
 case HPET_Tn_CFG(0):
 case HPET_Tn_CFG(1):
 case HPET_Tn_CFG(2):
-return h->hpet.timers[HPET_TN(CFG, addr)].config;
+return array_access_nospec(h->hpet.timers, HPET_TN(CFG, addr)).config;
 case HPET_Tn_CMP(0):
 case HPET_Tn_CMP(1):
 case HPET_Tn_CMP(2):
-return hpet_get_comparator(h, HPET_TN(CMP, addr), guest_time);
+return hpet_get_comparator(h, array_index_nospec(HPET_TN(CMP, addr),
+   ARRAY_SIZE(h->hpet.timers)),
+  guest_time);
 case HPET_Tn_ROUTE(0):
 case HPET_Tn_ROUTE(1):
 case HPET_Tn_ROUTE(2):
-return h->hpet.timers[HPET_TN(ROUTE, addr)].fsb;
+return array_access_nospec(h->hpet.timers, HPET_TN(ROUTE, addr)).fsb;
 }
 
 return 0;
@@ -438,7 +441,7 @@ static int hpet_write(
 case HPET_Tn_CFG(0):
 case HPET_Tn_CFG(1):
 case HPET_Tn_CFG(2):
-tn = HPET_TN(CFG, addr);
+tn = array_index_nospec(HPET_TN(CFG, addr), 
ARRAY_SIZE(h->hpet.timers));
 
 h->hpet.timers[tn].config =
 hpet_fixup_reg(new_val, old_val,
@@ -480,7 +483,7 @@ static int hpet_write(
 case HPET_Tn_CMP(0):
 case HPET_Tn_CMP(1):
 case HPET_Tn_CMP(2):
-tn = HPET_TN(CMP, addr);
+tn = array_index_nospec(HPET_TN(CMP, addr), 
ARRAY_SIZE(h->hpet.timers));
 if ( timer_is_periodic(h, tn) &&
  !(h->hpet.timers[tn].config & HPET_TN_SETVAL) )
 {
@@ -523,7 +526,7 @@ static int hpet_write(
 case HPET_Tn_ROUTE(0):
 case HPET_Tn_ROUTE(1):
 case HPET_Tn_ROUTE(2):
-tn = HPET_TN(ROUTE, addr);
+tn = array_index_nospec(HPET_TN(ROUTE, addr), 
ARRAY_SIZE(h->hpet.timers));
 h->hpet.timers[tn].fsb = new_val;
 break;
 
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

Re: [Xen-devel] [PATCH 1/3] treewide: Lift switch variables out of switches

[Greg KH]

On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
> 
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed 
> [-Wswitch-unreachable]
>siginfo_t si;
>  ^~

That's a pain, so this means we can't have any new variables in { }
scope except for at the top of a function?

That's going to be a hard thing to keep from happening over time, as
this is valid C :(

greg k-h

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

[Xen-devel] [PATCH SpectreV1+L1TF v4 08/11] xen/evtchn: block speculative out-of-bound accesses

[Norbert Manthey]

Guests can issue event channel interaction with guest specified data.
To avoid speculative out-of-bound accesses, we use the nospec macros.

This commit is part of the SpectreV1+L1TF mitigation patch series.

Signed-off-by: Norbert Manthey 

---
 xen/common/event_channel.c | 25 -
 xen/common/event_fifo.c| 16 +---
 xen/include/xen/event.h|  5 +++--
 3 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -368,8 +368,14 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, 
evtchn_port_t port)
 if ( virq_is_global(virq) && (vcpu != 0) )
 return -EINVAL;
 
+   /*
+* Make sure the guest controlled value virq is bounded even during
+* speculative execution.
+*/
+virq = array_index_nospec(virq, ARRAY_SIZE(v->virq_to_evtchn));
+
 if ( (vcpu < 0) || (vcpu >= d->max_vcpus) ||
- ((v = d->vcpu[vcpu]) == NULL) )
+ ((v = d->vcpu[array_index_nospec(vcpu, d->max_vcpus)]) == NULL) )
 return -ENOENT;
 
 spin_lock(&d->event_lock);
@@ -419,7 +425,7 @@ static long evtchn_bind_ipi(evtchn_bind_ipi_t *bind)
 long   rc = 0;
 
 if ( (vcpu < 0) || (vcpu >= d->max_vcpus) ||
- (d->vcpu[vcpu] == NULL) )
+ (d->vcpu[array_index_nospec(vcpu, d->max_vcpus)] == NULL) )
 return -ENOENT;
 
 spin_lock(&d->event_lock);
@@ -816,6 +822,12 @@ int set_global_virq_handler(struct domain *d, uint32_t 
virq)
 if (!virq_is_global(virq))
 return -EINVAL;
 
+   /*
+* Make sure the guest controlled value virq is bounded even during
+* speculative execution.
+*/
+virq = array_index_nospec(virq, ARRAY_SIZE(global_virq_handlers));
+
 if (global_virq_handlers[virq] == d)
 return 0;
 
@@ -931,7 +943,8 @@ long evtchn_bind_vcpu(unsigned int port, unsigned int 
vcpu_id)
 struct evtchn *chn;
 long   rc = 0;
 
-if ( (vcpu_id >= d->max_vcpus) || (d->vcpu[vcpu_id] == NULL) )
+if ( (vcpu_id >= d->max_vcpus) ||
+ (d->vcpu[array_index_nospec(vcpu_id, d->max_vcpus)] == NULL) )
 return -ENOENT;
 
 spin_lock(&d->event_lock);
@@ -969,8 +982,10 @@ long evtchn_bind_vcpu(unsigned int port, unsigned int 
vcpu_id)
 unlink_pirq_port(chn, d->vcpu[chn->notify_vcpu_id]);
 chn->notify_vcpu_id = vcpu_id;
 pirq_set_affinity(d, chn->u.pirq.irq,
-  cpumask_of(d->vcpu[vcpu_id]->processor));
-link_pirq_port(port, chn, d->vcpu[vcpu_id]);
+  cpumask_of(d->vcpu[array_index_nospec(vcpu_id,
+
d->max_vcpus)]->processor));
+link_pirq_port(port, chn, d->vcpu[array_index_nospec(vcpu_id,
+ d->max_vcpus)]);
 break;
 default:
 rc = -EINVAL;
diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -33,7 +33,8 @@ static inline event_word_t *evtchn_fifo_word_from_port(const 
struct domain *d,
  */
 smp_rmb();
 
-p = port / EVTCHN_FIFO_EVENT_WORDS_PER_PAGE;
+p = array_index_nospec(port / EVTCHN_FIFO_EVENT_WORDS_PER_PAGE,
+   d->evtchn_fifo->num_evtchns);
 w = port % EVTCHN_FIFO_EVENT_WORDS_PER_PAGE;
 
 return d->evtchn_fifo->event_array[p] + w;
@@ -516,14 +517,23 @@ int evtchn_fifo_init_control(struct evtchn_init_control 
*init_control)
 gfn = init_control->control_gfn;
 offset  = init_control->offset;
 
-if ( vcpu_id >= d->max_vcpus || !d->vcpu[vcpu_id] )
+if ( vcpu_id >= d->max_vcpus ||
+ !d->vcpu[array_index_nospec(vcpu_id, d->max_vcpus)] )
 return -ENOENT;
-v = d->vcpu[vcpu_id];
+
+v = d->vcpu[array_index_nospec(vcpu_id, d->max_vcpus)];
 
 /* Must not cross page boundary. */
 if ( offset > (PAGE_SIZE - sizeof(evtchn_fifo_control_block_t)) )
 return -EINVAL;
 
+/*
+ * Make sure the guest controlled value offset is bounded even during
+ * speculative execution.
+ */
+offset = array_index_nospec(offset,
+  PAGE_SIZE - sizeof(evtchn_fifo_control_block_t));
+
 /* Must be 8-bytes aligned. */
 if ( offset & (8 - 1) )
 return -EINVAL;
diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
--- a/xen/include/xen/event.h
+++ b/xen/include/xen/event.h
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /*
@@ -96,7 +97,7 @@ void arch_evtchn_inject(struct vcpu *v);
  * The first bucket is directly accessed via d->evtchn.
  */
 #define group_from_port(d, p) \
-((d)->evtchn_group[(p) / EVTCHNS_PER_GROUP])
+array_access_nospec((d)->evtchn_group, (p) / EVTCHNS_PER_GROUP)
 #define bucket_from_port(d, p) \
 ((group_from_port(d, p))[((p) % EVTCHNS_PER_GROUP) /

[Xen-devel] [PATCH SpectreV1+L1TF v4 11/11] x86/CPUID: block speculative out-of-bound accesses

[Norbert Manthey]

During instruction emulation, the cpuid instruction is emulated with
data that is controlled by the guest. As speculation might pass bound
checks, we have to ensure that no out-of-bound loads are possible.

To not rely on the compiler to perform value propagation, instead of
using the array_index_nospec macro, we replace the variable with the
constant to be propagated instead.

This commit is part of the SpectreV1+L1TF mitigation patch series.

Signed-off-by: Norbert Manthey 
Reviewed-by: Jan Beulich 

---
 xen/arch/x86/cpuid.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -1,6 +1,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -629,7 +630,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
 if ( subleaf >= ARRAY_SIZE(p->cache.raw) )
 return;
 
-*res = p->cache.raw[subleaf];
+*res = array_access_nospec(p->cache.raw, subleaf);
 break;
 
 case 0x7:
@@ -638,25 +639,25 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
  ARRAY_SIZE(p->feat.raw) - 1) )
 return;
 
-*res = p->feat.raw[subleaf];
+*res = array_access_nospec(p->feat.raw, subleaf);
 break;
 
 case 0xb:
 if ( subleaf >= ARRAY_SIZE(p->topo.raw) )
 return;
 
-*res = p->topo.raw[subleaf];
+*res = array_access_nospec(p->topo.raw, subleaf);
 break;
 
 case XSTATE_CPUID:
 if ( !p->basic.xsave || subleaf >= ARRAY_SIZE(p->xstate.raw) )
 return;
 
-*res = p->xstate.raw[subleaf];
+*res = array_access_nospec(p->xstate.raw, subleaf);
 break;
 
 default:
-*res = p->basic.raw[leaf];
+*res = array_access_nospec(p->basic.raw, leaf);
 break;
 }
 break;
@@ -680,7 +681,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
  ARRAY_SIZE(p->extd.raw) - 1) )
 return;
 
-*res = p->extd.raw[leaf & 0x];
+*res = array_access_nospec(p->extd.raw, leaf & 0x);
 break;
 
 default:
@@ -847,7 +848,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
 if ( is_pv_domain(d) && is_hardware_domain(d) &&
  guest_kernel_mode(v, regs) && cpu_has_monitor &&
  regs->entry_vector == TRAP_gp_fault )
-*res = raw_cpuid_policy.basic.raw[leaf];
+*res = raw_cpuid_policy.basic.raw[5];
 break;
 
 case 0x7:
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

[Xen-devel] [PATCH SpectreV1+L1TF v4 07/11] nospec: enable lfence on Intel

[Norbert Manthey]

While the lfence instruction was added for all x86 platform in the
beginning, it's useful to not block platforms that are not affected
by the L1TF vulnerability. Therefore, the lfence instruction should
only be introduced, in case the current CPU is an Intel CPU that is
capable of hyper threading. This combination of features is added
to the features that activate the alternative.

This commit is part of the SpectreV1+L1TF mitigation patch series.

Signed-off-by: Norbert Manthey 

---
 xen/include/xen/nospec.h | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -7,6 +7,7 @@
 #ifndef XEN_NOSPEC_H
 #define XEN_NOSPEC_H
 
+#include 
 #include 
 
 /**
@@ -68,7 +69,10 @@ static inline unsigned long array_index_mask_nospec(unsigned 
long index,
  * allow to insert a read memory barrier into conditionals
  */
 #ifdef CONFIG_X86
-static inline bool lfence_true(void) { rmb(); return true; }
+static inline bool lfence_true(void) {
+alternative("", "lfence", X86_VENDOR_INTEL);
+return true;
+}
 #else
 static inline bool lfence_true(void) { return true; }
 #endif
@@ -91,7 +95,7 @@ static inline bool lfence_true(void) { return true; }
  * allow to block speculative execution in generic code
  */
 #ifdef CONFIG_X86
-#define block_speculation() rmb()
+#define block_speculation() alternative("", "lfence", X86_VENDOR_INTEL)
 #else
 #define block_speculation()
 #endif
-- 
2.7.4




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B



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

Re: [Xen-devel] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jann Horn]

On Wed, Jan 23, 2019 at 1:04 PM Greg KH  wrote:
> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> > Variables declared in a switch statement before any case statements
> > cannot be initialized, so move all instances out of the switches.
> > After this, future always-initialized stack variables will work
> > and not throw warnings like this:
> >
> > fs/fcntl.c: In function ‘send_sigio_to_task’:
> > fs/fcntl.c:738:13: warning: statement will never be executed 
> > [-Wswitch-unreachable]
> >siginfo_t si;
> >  ^~
>
> That's a pain, so this means we can't have any new variables in { }
> scope except for at the top of a function?

AFAICS this only applies to switch statements (because they jump to a
case and don't execute stuff at the start of the block), not blocks
after if/while/... .

> That's going to be a hard thing to keep from happening over time, as
> this is valid C :(

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

Re: [Xen-devel] [PATCH 1/3] treewide: Lift switch variables out of switches

[Ard Biesheuvel]

On Wed, 23 Jan 2019 at 13:09, Jann Horn  wrote:
>
> On Wed, Jan 23, 2019 at 1:04 PM Greg KH  wrote:
> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> > > Variables declared in a switch statement before any case statements
> > > cannot be initialized, so move all instances out of the switches.
> > > After this, future always-initialized stack variables will work
> > > and not throw warnings like this:
> > >
> > > fs/fcntl.c: In function ‘send_sigio_to_task’:
> > > fs/fcntl.c:738:13: warning: statement will never be executed 
> > > [-Wswitch-unreachable]
> > >siginfo_t si;
> > >  ^~
> >
> > That's a pain, so this means we can't have any new variables in { }
> > scope except for at the top of a function?
>
> AFAICS this only applies to switch statements (because they jump to a
> case and don't execute stuff at the start of the block), not blocks
> after if/while/... .
>

I guess that means it may apply to other cases where you do a 'goto'
into the middle of a for() loop, for instance (at the first
iteration), which is also a valid pattern.

Is there any way to tag these assignments so the diagnostic disregards them?

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

Re: [Xen-devel] [PATCH 3/3] x86/pvh: Print the PVH start info more concisely

[Andrew Cooper]

On 23/01/2019 11:42, Jan Beulich wrote:
>
>> --- a/xen/arch/x86/guest/pvh-boot.c
>> +++ b/xen/arch/x86/guest/pvh-boot.c
>> @@ -123,28 +123,29 @@ void __init pvh_print_info(void)
>>  const struct hvm_modlist_entry *entry;
>>  unsigned int i;
>>  
>> -ASSERT(pvh_info->magic == XEN_HVM_START_MAGIC_VALUE);
>> -
>> -printk("PVH start info: (pa %08x)\n", pvh_start_info_pa);
>> -printk("  version:%u\n", pvh_info->version);
>> -printk("  flags:  %#"PRIx32"\n", pvh_info->flags);
>> -printk("  nr_modules: %u\n", pvh_info->nr_modules);
>> -printk("  modlist_pa: %016"PRIx64"\n", pvh_info->modlist_paddr);
>> -printk("  cmdline_pa: %016"PRIx64"\n", pvh_info->cmdline_paddr);
>> +printk("PVH start info: (pa 0x%08x)\n", pvh_start_info_pa);
>> +printk("  version %u, flags %#x\n", pvh_info->version, pvh_info->flags);
>> +
>> +printk("  cmdline 0x%08"PRIx64, pvh_info->cmdline_paddr);
>>  if ( pvh_info->cmdline_paddr )
>> -printk("  cmdline:'%s'\n", (char 
>> *)__va(pvh_info->cmdline_paddr));
>> -printk("  rsdp_pa:%016"PRIx64"\n", pvh_info->rsdp_paddr);
>> +printk(" '%s'", (char *)__va(pvh_info->cmdline_paddr));
> Is the cast here really necessary?

Yes.  Omitting it causes -Wformat to object to passing void* into
something expecting char*.

~Andrew

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

Re: [Xen-devel] [RFC] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Andrii Anisov]

On 23.01.19 13:33, Julien Grall wrote:

Do you mean sharing the P2M?


For sure!

--
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [RFC] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Andrii Anisov]

Hello Paul,

On 23.01.19 13:45, Paul Durrant wrote:

Yes, this was a mistake when moving from the old macros and need_iommu. Andrii 
is correct that need_iommu_pt_sync() is supposed to gate whether an explicit 
map/unmap is needed. The need for flush should only depend on has_iommu_pt().

So I fix the commit message and send V2 for XEN 4.12.


There is also iommu_use_hap_pt() which is actually defined as has_iommu_pt(), 
but I wonder whether that should just go away for ARM since the page tables are 
always shared.

Actually, the mainline currently supports only SMMU, which does share the page 
table.
But it is not always true, we are using HW, where PT is not shared. And going 
to upstream that support one day.

--
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [RFC] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Paul Durrant]

> -Original Message-
> From: Andrii Anisov [mailto:andrii.ani...@gmail.com]
> Sent: 23 January 2019 12:40
> To: Paul Durrant ; 'Julien Grall'
> ; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Andrii Anisov
> 
> Subject: Re: [RFC] arm/p2m: call iommu iotlb flush if iommu exists and
> enabled
> 
> Hello Paul,
> 
> On 23.01.19 13:45, Paul Durrant wrote:
> > Yes, this was a mistake when moving from the old macros and need_iommu.
> Andrii is correct that need_iommu_pt_sync() is supposed to gate whether an
> explicit map/unmap is needed. The need for flush should only depend on
> has_iommu_pt().
> So I fix the commit message and send V2 for XEN 4.12.

Cool.

> 
> > There is also iommu_use_hap_pt() which is actually defined as
> has_iommu_pt(), but I wonder whether that should just go away for ARM
> since the page tables are always shared.
> Actually, the mainline currently supports only SMMU, which does share the
> page table.
> But it is not always true, we are using HW, where PT is not shared. And
> going to upstream that support one day.

Oh, ok. Leave it in then :-)

  Paul

> 
> --
> Sincerely,
> Andrii Anisov.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 for-4.12] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Andrii Anisov]

From: Andrii Anisov 

Taking decision by `need_iommu_pt_sync()` make us never kicking
`iommu_iotlb_flush()` for IOMMUs which do share P2M with CPU.
So check `has_iommu_pt()` instead.

Signed-off-by: Andrii Anisov 
---
Changes in v2:
Fixed a missprint and a nit in a commit message

 xen/arch/arm/p2m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 2394f97..059a391 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1019,7 +1019,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
  !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) )
 p2m_free_entry(p2m, orig_pte, level);
 
-if ( need_iommu_pt_sync(p2m->domain) &&
+if ( has_iommu_pt(p2m->domain) &&
  (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
 {
 unsigned int flush_flags = 0;
-- 
2.7.4


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

Re: [Xen-devel] [PATCH v2 for-4.12] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Paul Durrant]

> -Original Message-
> From: Andrii Anisov [mailto:andrii.ani...@gmail.com]
> Sent: 23 January 2019 12:50
> To: xen-devel@lists.xenproject.org
> Cc: Julien Grall ; Stefano Stabellini
> ; Paul Durrant ; Juergen
> Gross ; Andrii Anisov 
> Subject: [PATCH v2 for-4.12] arm/p2m: call iommu iotlb flush if iommu
> exists and enabled
> 
> From: Andrii Anisov 
> 
> Taking decision by `need_iommu_pt_sync()` make us never kicking
> `iommu_iotlb_flush()` for IOMMUs which do share P2M with CPU.
> So check `has_iommu_pt()` instead.
> 
> Signed-off-by: Andrii Anisov 

Reviewed-by: Paul Durrant 

> ---
> Changes in v2:
> Fixed a missprint and a nit in a commit message
> 
>  xen/arch/arm/p2m.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2394f97..059a391 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1019,7 +1019,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
>   !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) )
>  p2m_free_entry(p2m, orig_pte, level);
> 
> -if ( need_iommu_pt_sync(p2m->domain) &&
> +if ( has_iommu_pt(p2m->domain) &&
>   (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
>  {
>  unsigned int flush_flags = 0;
> --
> 2.7.4


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

Re: [Xen-devel] [PATCH v2 for-4.12] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Juergen Gross]

On 23/01/2019 13:50, Andrii Anisov wrote:
> From: Andrii Anisov 
> 
> Taking decision by `need_iommu_pt_sync()` make us never kicking
> `iommu_iotlb_flush()` for IOMMUs which do share P2M with CPU.
> So check `has_iommu_pt()` instead.
> 
> Signed-off-by: Andrii Anisov 

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 01/11] is_control_domain: block speculation

[Jan Beulich]

>>> On 23.01.19 at 12:51,  wrote:
> --- a/xen/include/xen/nospec.h
> +++ b/xen/include/xen/nospec.h
> @@ -58,6 +58,21 @@ static inline unsigned long 
> array_index_mask_nospec(unsigned long index,
>  (typeof(_i)) (_i & _mask);  \
>  })
>  
> +/*
> + * allow to insert a read memory barrier into conditionals
> + */

Please obey to the comment style set forth in ./CODING_STYLE.

> +#ifdef CONFIG_X86
> +static inline bool lfence_true(void) { rmb(); return true; }
> +#else
> +static inline bool lfence_true(void) { return true; }
> +#endif

This is a generic header, hence functions defined here should have
universally applicable names. "lfence", however, is an x86 term
(naming a particular instruction). I can't think of really good
alternatives, but how about one of arch_nospec_true() /
arch_fence_nospec_true() / arch_nospec_fence_true()?

Furthermore, rather than adding Kconfig control and alternatives
patching later in the series (as per the cover letter), it should be
that way from the beginning. Remember that any series may go
in piecemeal, not all in one go.

Jan



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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 03/11] config: introduce L1TF_LFENCE option

[Jan Beulich]

>>> On 23.01.19 at 12:51,  wrote:
> This commit introduces the configuration option L1TF_LFENCE that allows
> to control the implementation of the protection of privilege checks via
> lfence instructions. The following four alternatives are provided:
> 
>  - not injecting lfence instructions
>  - inject an lfence instruction for both outcomes of the conditional
>  - inject an lfence instruction only if the conditional would evaluate
>to true, so that this case cannot be entered under speculation

I'd really like to see justification for this variant, as the asymmetric
handling doesn't look overly helpful. It's also not clear to me how
someone configuring Xen should tell whether this would be a safe
selection to make. I'm tempted to request that this option become
EXPERT dependent.

>  - evaluating the condition and store the result into a local variable.
>before using this value, inject an lfence instruction.
> 
> The different options allow to control the level of protection vs the
> slowdown the addtional lfence instructions would introduce. The default
> value is set to protecting both branches.
> 
> For non-x86 platforms, the protection is disabled by default.

At least the "by default" is wrong here.

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -176,6 +176,30 @@ config PV_SHIM_EXCLUSIVE
> firmware, and will not function correctly in other scenarios.
>  
> If unsure, say N.
> +
> +choice
> + prompt "Default L1TF Branch Protection?"
> +
> + config L1TF_LFENCE_BOTH
> + bool "Protect both branches of certain conditionals" if HVM
> + ---help---
> +   Inject an lfence instruction after the condition to be
> +   evaluated for both outcomes of the condition
> + config L1TF_LFENCE_TRUE
> + bool "Protect true branch of certain conditionals" if HVM
> + ---help---
> +   Protect only the path where the condition is evaluated to true
> + config L1TF_LFENCE_INTERMEDIATE
> + bool "Protect before using certain conditionals value" if HVM
> + ---help---
> +   Inject an lfence instruction after evaluating the condition
> +   but before forwarding this value from a local variable
> + config L1TF_LFENCE_NONE
> + bool "No conditional protection"
> + ---help---
> +   Do not inject lfences for conditional evaluations
> +endchoice

I guess we should settle on some default for this choice. The
description talks about a default, but I don't see it set here (and
I don't think relying merely on the order is a good idea).

> --- a/xen/include/xen/nospec.h
> +++ b/xen/include/xen/nospec.h
> @@ -68,10 +68,18 @@ static inline bool lfence_true(void) { return true; }
>  #endif
>  
>  /*
> - * protect evaluation of conditional with respect to speculation
> + * allow to protect evaluation of conditional with respect to speculation on 
> x86
>   */
> -#define evaluate_nospec(condition)  \
> +#if defined(CONFIG_L1TF_LFENCE_NONE) || !defined(CONFIG_X86)
> +#define evaluate_nospec(condition) (condition)
> +#elif defined(CONFIG_L1TF_LFENCE_BOTH)
> +#define evaluate_nospec(condition) \

I'm puzzled by this line changing - do you really need to move the
backslash here?

Jan



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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 01/11] is_control_domain: block speculation

[Julien Grall]

Hi,

On 23/01/2019 13:07, Jan Beulich wrote:

On 23.01.19 at 12:51,  wrote:

--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -58,6 +58,21 @@ static inline unsigned long array_index_mask_nospec(unsigned 
long index,
  (typeof(_i)) (_i & _mask);  \
  })
  
+/*

+ * allow to insert a read memory barrier into conditionals
+ */


Please obey to the comment style set forth in ./CODING_STYLE.


+#ifdef CONFIG_X86
+static inline bool lfence_true(void) { rmb(); return true; }
+#else
+static inline bool lfence_true(void) { return true; }
+#endif


This is a generic header, hence functions defined here should have
universally applicable names. "lfence", however, is an x86 term
(naming a particular instruction). I can't think of really good
alternatives, but how about one of arch_nospec_true() /
arch_fence_nospec_true() / arch_nospec_fence_true()?


We seems to use more the term "barrier" in common code over "fence". So how 
about arch_barrier_nospec_true/arch_nospec_barrier_true?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 01/11] is_control_domain: block speculation

[Jan Beulich]

>>> On 23.01.19 at 12:51,  wrote:
> --- a/xen/include/xen/nospec.h
> +++ b/xen/include/xen/nospec.h
> @@ -58,6 +58,21 @@ static inline unsigned long 
> array_index_mask_nospec(unsigned long index,
>  (typeof(_i)) (_i & _mask);  \
>  })
>  
> +/*
> + * allow to insert a read memory barrier into conditionals
> + */
> +#ifdef CONFIG_X86
> +static inline bool lfence_true(void) { rmb(); return true; }
> +#else
> +static inline bool lfence_true(void) { return true; }
> +#endif
> +
> +/*
> + * protect evaluation of conditional with respect to speculation
> + */
> +#define evaluate_nospec(condition)  \
> +(((condition) && lfence_true()) || !lfence_true())

It may be just me, but I think

#define evaluate_nospec(condition)  \
((condition) ? lfence_true() : !lfence_true())

would better express the two-way nature of this.

Jan



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

Re: [Xen-devel] [PATCH 1/3] treewide: Lift switch variables out of switches

[William Kucharski]

> On Jan 23, 2019, at 5:09 AM, Jann Horn  wrote:
> 
> AFAICS this only applies to switch statements (because they jump to a
> case and don't execute stuff at the start of the block), not blocks
> after if/while/... .

It bothers me that we are going out of our way to deprecate valid C constructs
in favor of placing the declarations elsewhere.

As current compiler warnings would catch any reference before initialization
usage anyway, it seems like we are letting a compiler warning rather than the
language standard dictate syntax.

Certainly if we want to make it a best practice coding style issue we can, and
then an appropriate note explaining why should be added to
Documentation/process/coding-style.rst.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 03/11] config: introduce L1TF_LFENCE option

[Julien Grall]

Hi,

On 23/01/2019 11:51, Norbert Manthey wrote:

diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -68,10 +68,18 @@ static inline bool lfence_true(void) { return true; }
  #endif
  
  /*

- * protect evaluation of conditional with respect to speculation
+ * allow to protect evaluation of conditional with respect to speculation on 
x86
   */
-#define evaluate_nospec(condition)  \
+#if defined(CONFIG_L1TF_LFENCE_NONE) || !defined(CONFIG_X86)
+#define evaluate_nospec(condition) (condition)
+#elif defined(CONFIG_L1TF_LFENCE_BOTH)
+#define evaluate_nospec(condition) \
  (((condition) && lfence_true()) || !lfence_true())
+#elif defined(CONFIG_L1TF_LFENCE_TRUE)
+#define evaluate_nospec(condition) ((condition) && lfence_true())
+#elif defined(CONFIG_L1TF_LFENCE_INTERMEDIATE)
+#define evaluate_nospec(condition) ({ bool res = (condition); rmb(); res; }) > 
+#endif


None of the configs are defined on Arm, so can this be moved in arch-x86?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 for-4.12] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Julien Grall]

On 23/01/2019 12:50, Andrii Anisov wrote:

From: Andrii Anisov 

Taking decision by `need_iommu_pt_sync()` make us never kicking
`iommu_iotlb_flush()` for IOMMUs which do share P2M with CPU.
So check `has_iommu_pt()` instead.


Please mention the bug was introduced by commit 91d4eca7ad "mm / iommu: split 
need_iommu() into has_iommu_pt() and need_iommu_pt_sync()".


With that:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.12] xen/arm: gic: Make sure the number of interrupt lines is valid before using it

[Julien Grall]

(+ Juergen)

Hi Juergen,

On 22/01/2019 23:22, Stefano Stabellini wrote:

On Fri, 30 Nov 2018, Julien Grall wrote:

GICv2 and GICv3 supports up to 1020 interrupts. However, the value computed
from GICD_TYPER.ITLinesNumber can be up to 1024. On GICv3, we will end up to
write in reserved registers that are right after the IROUTERs one as the
value is not capped early enough.

Cap the number of interrupts as soon as we compute it so we know we can
safely using it afterwards.

Signed-off-by: Julien Grall 
Reported-by: Jan-Peter Larsson 


Reviewed-by: Stefano Stabellini 


Would it be possible to give an RAB for this patch?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 for-4.12] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Andrii Anisov]

Juergen,

On 23.01.19 15:26, Julien Grall wrote:

Please mention the bug was introduced by commit 91d4eca7ad "mm / iommu: split 
need_iommu() into has_iommu_pt() and need_iommu_pt_sync()".

Should I send v3?.


With that:

Acked-by: Julien Grall 


--
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [PATCH v2 for-4.12] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Julien Grall]

On 23/01/2019 13:32, Andrii Anisov wrote:

Juergen,

On 23.01.19 15:26, Julien Grall wrote:
Please mention the bug was introduced by commit 91d4eca7ad "mm / iommu: split 
need_iommu() into has_iommu_pt() and need_iommu_pt_sync()".

Should I send v3?.


No need, I will fix it on commit.

Cheers,




With that:

Acked-by: Julien Grall 




--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 for-4.12] arm/p2m: call iommu iotlb flush if iommu exists and enabled

[Andrii Anisov]

On 23.01.19 15:33, Julien Grall wrote:

On 23.01.19 15:26, Julien Grall wrote:

No need, I will fix it on commit.


Thank you.

--
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [PATCH for-4.12] xen/arm: gic: Make sure the number of interrupt lines is valid before using it

[Juergen Gross]

On 23/01/2019 14:29, Julien Grall wrote:
> (+ Juergen)
> 
> Hi Juergen,
> 
> On 22/01/2019 23:22, Stefano Stabellini wrote:
>> On Fri, 30 Nov 2018, Julien Grall wrote:
>>> GICv2 and GICv3 supports up to 1020 interrupts. However, the value
>>> computed
>>> from GICD_TYPER.ITLinesNumber can be up to 1024. On GICv3, we will
>>> end up to
>>> write in reserved registers that are right after the IROUTERs one as the
>>> value is not capped early enough.
>>>
>>> Cap the number of interrupts as soon as we compute it so we know we can
>>> safely using it afterwards.
>>>
>>> Signed-off-by: Julien Grall 
>>> Reported-by: Jan-Peter Larsson 
>>
>> Reviewed-by: Stefano Stabellini 
> 
> Would it be possible to give an RAB for this patch?

Release-acked-by: Juergen Gross 


Juergen


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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 05/11] common/grant_table: block speculative out-of-bound accesses

[Jan Beulich]

>>> On 23.01.19 at 12:51,  wrote:
> @@ -1268,7 +1272,8 @@ unmap_common(
>  }
>  
>  smp_rmb();
> -map = &maptrack_entry(lgt, op->handle);
> +map = &maptrack_entry(lgt, array_index_nospec(op->handle,
> +  lgt->maptrack_limit));

It might be better to move this into maptrack_entry() itself, or
make a maptrack_entry_nospec() clone (as several but not all
uses may indeed not be in need of the extra protection). At
least the ones in steal_maptrack_handle() and
put_maptrack_handle() also look potentially suspicious.

> @@ -2223,7 +2231,8 @@ gnttab_transfer(
>  okay = gnttab_prepare_for_transfer(e, d, gop.ref);
>  spin_lock(&e->page_alloc_lock);
>  
> -if ( unlikely(!okay) || unlikely(e->is_dying) )
> +/* Make sure this check is not bypassed speculatively */
> +if ( evaluate_nospec(unlikely(!okay) || unlikely(e->is_dying)) )
>  {
>  bool_t drop_dom_ref = !domain_adjust_tot_pages(e, -1);

What is it that makes this particular if() different from other
surrounding ones? In particular the version dependent code (a few
lines down from here as well as elsewhere) look to be easily
divertable onto the wrong branch, then causing out of bounds
speculative accesses due to the different (version dependent)
shared entry sizes.

> @@ -3215,6 +3230,10 @@ swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b)
>  if ( ref_a == ref_b )
>  goto out;
>  
> +/* Make sure the above check is not bypassed speculatively */
> +ref_a = array_index_nospec(ref_a, nr_grant_entries(d->grant_table));
> +ref_b = array_index_nospec(ref_b, nr_grant_entries(d->grant_table));

I think this wants to move up ahead of the if() in context, and the
comment be changed to plural.

> --- a/xen/include/xen/nospec.h
> +++ b/xen/include/xen/nospec.h
> @@ -87,6 +87,15 @@ static inline bool lfence_true(void) { return true; }
>  #define evaluate_nospec(condition) ({ bool res = (condition); rmb(); res; 
> })
>  #endif
>  
> +/*
> + * allow to block speculative execution in generic code
> + */

Comment style again.

> +#ifdef CONFIG_X86
> +#define block_speculation() rmb()
> +#else
> +#define block_speculation()
> +#endif

Why does this not simply resolve to what currently is named lfence_true()
(perhaps with a cast to void)? And why does this not depend on the
Kconfig setting?

Jan



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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 03/11] config: introduce L1TF_LFENCE option

[Jan Beulich]

>>> On 23.01.19 at 14:24,  wrote:
> On 23/01/2019 11:51, Norbert Manthey wrote:
>> --- a/xen/include/xen/nospec.h
>> +++ b/xen/include/xen/nospec.h
>> @@ -68,10 +68,18 @@ static inline bool lfence_true(void) { return true; }
>>   #endif
>>   
>>   /*
>> - * protect evaluation of conditional with respect to speculation
>> + * allow to protect evaluation of conditional with respect to speculation 
>> on x86
>>*/
>> -#define evaluate_nospec(condition)  \
>> +#if defined(CONFIG_L1TF_LFENCE_NONE) || !defined(CONFIG_X86)
>> +#define evaluate_nospec(condition) (condition)
>> +#elif defined(CONFIG_L1TF_LFENCE_BOTH)
>> +#define evaluate_nospec(condition) \
>>   (((condition) && lfence_true()) || !lfence_true())
>> +#elif defined(CONFIG_L1TF_LFENCE_TRUE)
>> +#define evaluate_nospec(condition) ((condition) && lfence_true())
>> +#elif defined(CONFIG_L1TF_LFENCE_INTERMEDIATE)
>> +#define evaluate_nospec(condition) ({ bool res = (condition); rmb(); res; })
> +#endif
> 
> None of the configs are defined on Arm, so can this be moved in arch-x86?

To be honest I'd like to avoid introducing asm/nospec.h for the time
being.

Jan



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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 01/11] is_control_domain: block speculation

[Jan Beulich]

>>> On 23.01.19 at 14:20,  wrote:
> On 23/01/2019 13:07, Jan Beulich wrote:
> On 23.01.19 at 12:51,  wrote:
>>> --- a/xen/include/xen/nospec.h
>>> +++ b/xen/include/xen/nospec.h
>>> @@ -58,6 +58,21 @@ static inline unsigned long 
>>> array_index_mask_nospec(unsigned long index,
>>>   (typeof(_i)) (_i & _mask);  \
>>>   })
>>>   
>>> +/*
>>> + * allow to insert a read memory barrier into conditionals
>>> + */
>> 
>> Please obey to the comment style set forth in ./CODING_STYLE.
>> 
>>> +#ifdef CONFIG_X86
>>> +static inline bool lfence_true(void) { rmb(); return true; }
>>> +#else
>>> +static inline bool lfence_true(void) { return true; }
>>> +#endif
>> 
>> This is a generic header, hence functions defined here should have
>> universally applicable names. "lfence", however, is an x86 term
>> (naming a particular instruction). I can't think of really good
>> alternatives, but how about one of arch_nospec_true() /
>> arch_fence_nospec_true() / arch_nospec_fence_true()?
> 
> We seems to use more the term "barrier" in common code over "fence". So how 
> about arch_barrier_nospec_true/arch_nospec_barrier_true?

That would be fine with me as well.

Jan



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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 03/11] config: introduce L1TF_LFENCE option

[Julien Grall]

Hi Jan,

On 23/01/2019 13:39, Jan Beulich wrote:

On 23.01.19 at 14:24,  wrote:

On 23/01/2019 11:51, Norbert Manthey wrote:

--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -68,10 +68,18 @@ static inline bool lfence_true(void) { return true; }
   #endif
   
   /*

- * protect evaluation of conditional with respect to speculation
+ * allow to protect evaluation of conditional with respect to speculation on 
x86
*/
-#define evaluate_nospec(condition)  \
+#if defined(CONFIG_L1TF_LFENCE_NONE) || !defined(CONFIG_X86)
+#define evaluate_nospec(condition) (condition)
+#elif defined(CONFIG_L1TF_LFENCE_BOTH)
+#define evaluate_nospec(condition) \
   (((condition) && lfence_true()) || !lfence_true())
+#elif defined(CONFIG_L1TF_LFENCE_TRUE)
+#define evaluate_nospec(condition) ((condition) && lfence_true())
+#elif defined(CONFIG_L1TF_LFENCE_INTERMEDIATE)
+#define evaluate_nospec(condition) ({ bool res = (condition); rmb(); res; })

+#endif

None of the configs are defined on Arm, so can this be moved in arch-x86?


To be honest I'd like to avoid introducing asm/nospec.h for the time
being.


How about adding them in system.h as we did for array_index_mask_nospec?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Greg KH  wrote:
> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>> Variables declared in a switch statement before any case statements
>> cannot be initialized, so move all instances out of the switches.
>> After this, future always-initialized stack variables will work
>> and not throw warnings like this:
>> 
>> fs/fcntl.c: In function ‘send_sigio_to_task’:
>> fs/fcntl.c:738:13: warning: statement will never be executed 
>> [-Wswitch-unreachable]
>>siginfo_t si;
>>  ^~
>
> That's a pain, so this means we can't have any new variables in { }
> scope except for at the top of a function?
>
> That's going to be a hard thing to keep from happening over time, as
> this is valid C :(

Not all valid C is meant to be used! ;)

Anyway, I think you're mistaking the limitation to arbitrary blocks
while it's only about the switch block IIUC.

Can't have:

switch (i) {
int j;
case 0:
/* ... */
}

because it can't be turned into:

switch (i) {
int j = 0; /* not valid C */
case 0:
/* ... */
}

but can have e.g.:

switch (i) {
case 0:
{
int j = 0;
/* ... */
}
}

I think Kees' approach of moving such variable declarations to the
enclosing block scope is better than adding another nesting block.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

Re: [Xen-devel] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Jani Nikula  wrote:
> On Wed, 23 Jan 2019, Greg KH  wrote:
>> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>>> Variables declared in a switch statement before any case statements
>>> cannot be initialized, so move all instances out of the switches.
>>> After this, future always-initialized stack variables will work
>>> and not throw warnings like this:
>>> 
>>> fs/fcntl.c: In function ‘send_sigio_to_task’:
>>> fs/fcntl.c:738:13: warning: statement will never be executed 
>>> [-Wswitch-unreachable]
>>>siginfo_t si;
>>>  ^~
>>
>> That's a pain, so this means we can't have any new variables in { }
>> scope except for at the top of a function?
>>
>> That's going to be a hard thing to keep from happening over time, as
>> this is valid C :(
>
> Not all valid C is meant to be used! ;)
>
> Anyway, I think you're mistaking the limitation to arbitrary blocks
> while it's only about the switch block IIUC.
>
> Can't have:
>
>   switch (i) {
>   int j;
>   case 0:
>   /* ... */
>   }
>
> because it can't be turned into:
>
>   switch (i) {
>   int j = 0; /* not valid C */
>   case 0:
>   /* ... */
>   }
>
> but can have e.g.:
>
>   switch (i) {
>   case 0:
>   {
>   int j = 0;
>   /* ... */
>   }
>   }
>
> I think Kees' approach of moving such variable declarations to the
> enclosing block scope is better than adding another nesting block.

PS. The patch is

Reviewed-by: Jani Nikula 

and the drivers/gpu/drm/i915/* parts are

Acked-by: Jani Nikula 

for merging via whichever tree is appropriate. (There'll be minor
conflicts with in-flight work in our -next tree, but no biggie.)


-- 
Jani Nikula, Intel Open Source Graphics Center

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

Re: [Xen-devel] [PATCH 1/2] x86: respect memory size limiting via mem= parameter

[William Kucharski]

> On Jan 22, 2019, at 1:06 AM, Juergen Gross  wrote:
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index b9a667d36c55..7fc2a87110a3 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -96,10 +96,16 @@ void mem_hotplug_done(void)
>   cpus_read_unlock();
> }
> 
> +u64 max_mem_size = -1;

This may be pedantic, but I'd rather see U64_MAX used here.


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

Re: [Xen-devel] [PATCH 1/2] x86: respect memory size limiting via mem= parameter

[Juergen Gross]

On 23/01/2019 15:35, William Kucharski wrote:
> 
> 
>> On Jan 22, 2019, at 1:06 AM, Juergen Gross  wrote:
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index b9a667d36c55..7fc2a87110a3 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -96,10 +96,16 @@ void mem_hotplug_done(void)
>>  cpus_read_unlock();
>> }
>>
>> +u64 max_mem_size = -1;
> 
> This may be pedantic, but I'd rather see U64_MAX used here.

Fine with me. Will change.


Juergen


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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 03/11] config: introduce L1TF_LFENCE option

[Jan Beulich]

>>> On 23.01.19 at 14:44,  wrote:
> On 23/01/2019 13:39, Jan Beulich wrote:
> On 23.01.19 at 14:24,  wrote:
>>> On 23/01/2019 11:51, Norbert Manthey wrote:
 --- a/xen/include/xen/nospec.h
 +++ b/xen/include/xen/nospec.h
 @@ -68,10 +68,18 @@ static inline bool lfence_true(void) { return true; }
#endif

/*
 - * protect evaluation of conditional with respect to speculation
 + * allow to protect evaluation of conditional with respect to speculation 
 on x86
 */
 -#define evaluate_nospec(condition)  \
 +#if defined(CONFIG_L1TF_LFENCE_NONE) || !defined(CONFIG_X86)
 +#define evaluate_nospec(condition) (condition)
 +#elif defined(CONFIG_L1TF_LFENCE_BOTH)
 +#define evaluate_nospec(condition)
  \
(((condition) && lfence_true()) || !lfence_true())
 +#elif defined(CONFIG_L1TF_LFENCE_TRUE)
 +#define evaluate_nospec(condition) ((condition) && lfence_true())
 +#elif defined(CONFIG_L1TF_LFENCE_INTERMEDIATE)
 +#define evaluate_nospec(condition) ({ bool res = (condition); rmb(); res; 
 })
>>> +#endif
>>>
>>> None of the configs are defined on Arm, so can this be moved in arch-x86?
>> 
>> To be honest I'd like to avoid introducing asm/nospec.h for the time
>> being.
> 
> How about adding them in system.h as we did for array_index_mask_nospec?

To tell you the truth, that's where Norbert had it originally.
I think that's not the right place though (also for
array_index_mask_nospec()). But I'll listen to a majority
thinking differently, at least as far as what is currently
lfence_true() goes. evaluate_nospec(), otoh, belongs where
it is now, I think.

Jan



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

Re: [Xen-devel] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Edwin Zimmerman]

On Wed, 23 Jan 2019, Jani Nikula  wrote:
> On Wed, 23 Jan 2019, Greg KH  wrote:
> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> >> Variables declared in a switch statement before any case statements
> >> cannot be initialized, so move all instances out of the switches.
> >> After this, future always-initialized stack variables will work
> >> and not throw warnings like this:
> >>
> >> fs/fcntl.c: In function ‘send_sigio_to_task’:
> >> fs/fcntl.c:738:13: warning: statement will never be executed 
> >> [-Wswitch-unreachable]
> >>siginfo_t si;
> >>  ^~
> >
> > That's a pain, so this means we can't have any new variables in { }
> > scope except for at the top of a function?
> >
> > That's going to be a hard thing to keep from happening over time, as
> > this is valid C :(
> 
> Not all valid C is meant to be used! ;)

Very true.  The other thing to keep in mind is the burden of enforcing a 
prohibition on a valid C construct like this.  
It seems to me that patch reviewers and maintainers have enough to do without 
forcing them to watch for variable
declarations in switch statements.  Automating this prohibition, should it be 
accepted, seems like a good idea to me.

-Edwin Zimmerman


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

[Xen-devel] [PATCH] xen/sched: Introduce domain_vcpu() helper

[Andrew Cooper]

The progression of multi-vcpu support in Xen (originally a single pointer,
then an embedded d->vcpu[] array, then a dynamically allocated array) has
resulted in a large quantity of ad-hoc code for looking a vcpu up by id, and a
large number of ways that the toolstack can cause Xen to trip over a NULL
pointer.  Some of this has been addressed in Xen 4.12, and work is ongoing.

Another property of looking a vcpu up by id is frequently done in unprivileged
hypercall context, making it an attractive target for speculative sidechannel
attacks.

Introduce a helper to do the lookup correctly, and without speculative
interference.  For performance reasons, it is useful not to have an smp_rmb()
in this helper on ARM, and luckily this is safe to do, because of the
serialisation offered by the global domheap lock.

As a minor change noticed when checking the safety of this construct, sanity
check during boot that idle->max_vcpus is a suitable upper bound for
idle->vcpu[].

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
CC: Roger Pau Monné 
CC: Stefano Stabellini 
CC: Julien Grall 
CC: Juergen Gross 
CC: Norbert Manthey 

Some notes:
 * This patch really is from 2014, and formed my first attempt to fix the NULL
   dereference problem, and eventally started getting upstream as the domain
   creation changes.  It is posted now to help with the XSA-289 series.
 * The reasoning about the safey of not having an smp_rmb() is only true for
   Xen 4.12.  It does not hold for older versions of Xen.
---
 xen/common/schedule.c   |  1 +
 xen/include/xen/sched.h | 26 ++
 2 files changed, 27 insertions(+)

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index a957c5e..fd58762 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1815,6 +1815,7 @@ void __init scheduler_init(void)
 
 idle_domain = domain_create(DOMID_IDLE, NULL, false);
 BUG_ON(IS_ERR(idle_domain));
+BUG_ON(nr_cpu_ids > ARRAY_SIZE(idle_vcpu));
 idle_domain->vcpu = idle_vcpu;
 idle_domain->max_vcpus = nr_cpu_ids;
 if ( vcpu_create(idle_domain, 0, 0) == NULL )
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 4956a77..a761db0 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -836,6 +837,31 @@ static inline int 
domain_pause_by_systemcontroller_nosync(struct domain *d)
 void domain_pause_except_self(struct domain *d);
 void domain_unpause_except_self(struct domain *d);
 
+/*
+ * For each allocated vcpu, d->vcpu[X]->vcpu_id == X
+ *
+ * During construction, all vcpus in d->vcpu[] are allocated sequentially, and
+ * in ascending order.  Therefore, if d->vcpu[N] exists (e.g. derived from
+ * current), all vcpus with an id less than N also exist.
+ *
+ * SMP considerations: The idle domain is constructed before APs are started.
+ * All other domains have d->vcpu[] allocated and d->max_vcpus set before the
+ * domain is made visible in the domlist, which is serialised on the global
+ * domlist_update_lock.
+ *
+ * Therefore, all observations of d->max_vcpus vs d->vcpu[] will be consistent
+ * despite the lack of smp_* barriers, either by being on the same CPU as the
+ * one which issued the writes, or because of barrier properties of the domain
+ * having been inserted into the domlist.
+ */
+static inline struct vcpu *domain_vcpu(const struct domain *d,
+   unsigned int vcpu_id)
+{
+unsigned int idx = array_index_nospec(vcpu_id, d->max_vcpus);
+
+return idx >= d->max_vcpus ? NULL : d->vcpu[idx];
+}
+
 void cpu_init(void);
 
 struct scheduler;
-- 
2.1.4


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

Re: [Xen-devel] [PATCH] xen/sched: Introduce domain_vcpu() helper

[Andrew Cooper]

On 23/01/2019 14:59, Andrew Cooper wrote:
> The progression of multi-vcpu support in Xen (originally a single pointer,
> then an embedded d->vcpu[] array, then a dynamically allocated array) has
> resulted in a large quantity of ad-hoc code for looking a vcpu up by id, and a
> large number of ways that the toolstack can cause Xen to trip over a NULL
> pointer.  Some of this has been addressed in Xen 4.12, and work is ongoing.
>
> Another property of looking a vcpu up by id is frequently done in unprivileged
> hypercall context, making it an attractive target for speculative sidechannel
> attacks.
>
> Introduce a helper to do the lookup correctly, and without speculative
> interference.  For performance reasons, it is useful not to have an smp_rmb()
> in this helper on ARM, and luckily this is safe to do, because of the
> serialisation offered by the global domheap lock.
>
> As a minor change noticed when checking the safety of this construct, sanity
> check during boot that idle->max_vcpus is a suitable upper bound for
> idle->vcpu[].
>
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: Wei Liu 
> CC: Roger Pau Monné 
> CC: Stefano Stabellini 
> CC: Julien Grall 
> CC: Juergen Gross 
> CC: Norbert Manthey 

And in my haste, I forgot to tag this as "for 4.12".

~Andrew

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

Re: [Xen-devel] [PATCH] xen/sched: Introduce domain_vcpu() helper

[Juergen Gross]

On 23/01/2019 15:59, Andrew Cooper wrote:
> The progression of multi-vcpu support in Xen (originally a single pointer,
> then an embedded d->vcpu[] array, then a dynamically allocated array) has
> resulted in a large quantity of ad-hoc code for looking a vcpu up by id, and a
> large number of ways that the toolstack can cause Xen to trip over a NULL
> pointer.  Some of this has been addressed in Xen 4.12, and work is ongoing.
> 
> Another property of looking a vcpu up by id is frequently done in unprivileged
> hypercall context, making it an attractive target for speculative sidechannel
> attacks.
> 
> Introduce a helper to do the lookup correctly, and without speculative
> interference.  For performance reasons, it is useful not to have an smp_rmb()
> in this helper on ARM, and luckily this is safe to do, because of the
> serialisation offered by the global domheap lock.
> 
> As a minor change noticed when checking the safety of this construct, sanity
> check during boot that idle->max_vcpus is a suitable upper bound for
> idle->vcpu[].
> 
> Signed-off-by: Andrew Cooper 

Release-acked-by: Juergen Gross 


Juergen

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 131842
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail REGR. vs. 131842

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 131842
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 131842
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 131842
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 131842
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 131842
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail 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-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:
 qemuu952bc8b3c2cbba78261923a1e8ca55cda261dee9
baseline version:
 qemuu147923b1a901a0370f83a0f4c58ec1baffef22f0

Last test of basis   131842  2019-01-09 00:37:22 Z   14 days
Failing since131892  2019-01-09 23:37:00 Z   13 days   13 attempts
Testing same since   132403  2019-01-22 21:07:04 Z0 days1 attempts


People who touched revisions under test:
  Aaron Lindsay 
  Aaron Lindsay 
  Aaron Lindsay 
  Aleksandar Markovic 
  Alex Bennée 
  Alex Williamson 
  Alexander Graf 
  Alexandro Sanchez Bach 
  Alexey Kardashevskiy 
  Alistair Francis 
  Andrew Jeffery 
  Anthony PERARD 
  BALATON Zoltan 
  Borislav Petkov 
  Christian Borntraeger 
  Cleber Rosa 
  Collin Walling 
  Cornelia Huck 
  Cédric Le Goater 
  Daniel P. Berrangé 
  David Gibson 
  David Hildenbrand 
  Dongli Zhang 
  Eduardo Habkost 
  Eduardo Otubo 
  Eric Auger 
  Eric Blake 
  Fam Zheng 
  Fei Li 
  Frediano Ziglio 
  Fredrik Noring 
  Gerd Hoffmann 
  Greg Kurz 
  Guenter Roeck 
  Igor Mammedov 
  Janosch Frank 
  Jian Wang 
  Joel Stanley 
  John Snow 
  Juan Quintela 
  Kamal Heib 
  Kashyap Chamarthy 
  Laurent Vivier 
  Laurent Vivier 
  Li Feng 
  Li Qiang 
  Marc-André Lureau 
  Marcel Apfelbaum 
  Mark Cave-Ayland 
  Michael Clark 
  Michael Roth 
  Michael S. Tsirkin 
  Palmer Dabbelt 
  Paolo Bonzini 
  Paul Durrant 
  Peng Hao 
  Peter Maydell 
  Philippe Mathieu-Daudé 
  Philippe Mathieu-Daudé 
  Pierre Morel 
  Prasad J Pandit 
  Priit Laes 
  Remy Noel 
  Richard Henderson 
  Richard W.M. Jones 
  Roman Bolshakov 
  Samuel Thibault 
  Shameer Kolothum 
  Sreejith Mohanan 
  

Re: [Xen-devel] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Edwin Zimmerman  wrote:
> On Wed, 23 Jan 2019, Jani Nikula  wrote:
>> On Wed, 23 Jan 2019, Greg KH  wrote:
>> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>> >> Variables declared in a switch statement before any case statements
>> >> cannot be initialized, so move all instances out of the switches.
>> >> After this, future always-initialized stack variables will work
>> >> and not throw warnings like this:
>> >>
>> >> fs/fcntl.c: In function ‘send_sigio_to_task’:
>> >> fs/fcntl.c:738:13: warning: statement will never be executed 
>> >> [-Wswitch-unreachable]
>> >>siginfo_t si;
>> >>  ^~
>> >
>> > That's a pain, so this means we can't have any new variables in { }
>> > scope except for at the top of a function?
>> >
>> > That's going to be a hard thing to keep from happening over time, as
>> > this is valid C :(
>> 
>> Not all valid C is meant to be used! ;)
>
> Very true.  The other thing to keep in mind is the burden of enforcing
> a prohibition on a valid C construct like this.  It seems to me that
> patch reviewers and maintainers have enough to do without forcing them
> to watch for variable declarations in switch statements.  Automating
> this prohibition, should it be accepted, seems like a good idea to me.

Considering that the treewide diffstat to fix this is:

 18 files changed, 45 insertions(+), 46 deletions(-)

and using the gcc plugin in question will trigger the switch-unreachable
warning, I think we're good. There'll probably be the occasional
declarations that pass through, and will get fixed afterwards.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-shadow7 xen-boot fail REGR. vs. 129313
 test-amd64-amd64-amd64-pvgrub  7 xen-bootfail REGR. vs. 129313
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. 
vs. 129313
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. 
vs. 129313
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 129313
 test-amd64-amd64-i386-pvgrub  7 xen-boot fail REGR. vs. 129313
 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host   fail REGR. vs. 129313
 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host   fail REGR. vs. 129313
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 129313
 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 129313
 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 129313
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 129313
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 129313
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 129313
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 129313
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 129313
 test-amd64-amd64-examine  8 reboot   fail REGR. vs. 129313
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail REGR. vs. 
129313
 test-armhf-armhf-xl-credit2   7 xen-boot fail REGR. vs. 129313
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 
129313

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

Re: [Xen-devel] Problem with IOMEM and domain reboot

[Andrii Anisov]

Hello Wei,

On 13.02.18 14:24, Wei Liu wrote:

On Mon, Feb 12, 2018 at 07:22:27PM +0200, Oleksandr Grytsov wrote:

Is it done by design or there is an issue with parse_json?
If it is done by design then the solution proposed by you (update_config
hook)
will solve this problem. But handling default value in parse json looks
more correct.


I need to figure out what is going on before I can answer these
questions. :-)


Sorry for my ignorance, is this issue fixed in coming 4.12?
If not, is there any chance to get it finished for 4.12?

--
Sincerely,
Andrii Anisov.

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

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

[osstest service owner]

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

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

version targeted for testing:
 xen  30f5047b2c4e577436b505ba7627f34c3be02014
baseline version:
 xen  08b908ba63dee8bc313983c5e412852cbcbcda85

Last test of basis   132342  2019-01-22 12:00:34 Z1 days
Testing same since   132436  2019-01-23 14:00:50 Z0 days1 attempts


People who touched revisions under test:
  Andrii Anisov 
  Julien Grall 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  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
   08b908ba63..30f5047b2c  30f5047b2c4e577436b505ba7627f34c3be02014 -> smoke

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

Re: [Xen-devel] [PATCH] xen/sched: Introduce domain_vcpu() helper

[Jan Beulich]

>>> On 23.01.19 at 15:59,  wrote:
> +static inline struct vcpu *domain_vcpu(const struct domain *d,
> +   unsigned int vcpu_id)
> +{
> +unsigned int idx = array_index_nospec(vcpu_id, d->max_vcpus);
> +
> +return idx >= d->max_vcpus ? NULL : d->vcpu[idx];
> +}

For an out of bounds incoming vcpu_id, isn't it the case that
idx then would be zero? In which case you'd return d->vcpu[0]
instead of NULL?

Jan



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

Re: [Xen-devel] [PATCH] drm: Split out drm_probe_helper.h

[Sam Ravnborg]

Hi Daniel.

On Thu, Jan 17, 2019 at 10:03:34PM +0100, Daniel Vetter wrote:
> Having the probe helper stuff (which pretty much everyone needs) in
> the drm_crtc_helper.h file (which atomic drivers should never need) is
> confusing. Split them out.
> 
> To make sure I actually achieved the goal here I went through all
> drivers. And indeed, all atomic drivers are now free of
> drm_crtc_helper.h includes.

How are the plans to get this patchset merged?
There were dependencies on onging drmP.h removal in i915 IIRC?
I guess my "Minimize drmP.h dependencies" patch-set also have a role in this.

This does not hold up any work of mine, mainly wanted to make
sure this was not lost between all the other patches.

Sam

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

Re: [Xen-devel] [RFC PATCH v2 1/2] xen: credit2: rb-tree for runqueues

[Praveen Kumar]

Hi Dario,
Thanks for your comments.

On Fri, Jan 18, 2019 at 8:38 PM Dario Faggioli  wrote:
>
> On Sun, 2018-12-23 at 19:51 +0530, Praveen Kumar wrote:
> > diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> > index 623a325ceb..2463a25f87 100644
> > --- a/xen/common/sched_credit2.c
> > +++ b/xen/common/sched_credit2.c
> > @@ -471,7 +472,7 @@ custom_param("credit2_runqueue",
> > parse_credit2_runqueue);
> >  struct csched2_runqueue_data {
> >  spinlock_t lock;   /* Lock for this
> > runqueue */
> >
> > -struct list_head runq; /* Ordered list of runnable
> > vms   */
> > +struct rb_root runq;   /* Runqueue is an
> > rbtree  */
> >
> I wouldn't change the comment. It's useful to know that the idea is to
> use this field to keep a list of runnable vcpus, and that we want it to
> be ordered, which is what the comment currently says.
>
> It's pointless to state that we're using an rb-tree, because once can
> tell that, by just looking at the data type.
>
> Actually, the comment says "Ordered list of runnable vms", while I
> think it would be better if it said "Ordered list of runnable vcpus".
>
> Changing "vms" to "vcpus" does not belong in this patch, strictly
> speaking, but if you want to do that, I could live with that. If you
> don't want to do it, then don't, and just leave the comment alone.
>
> > @@ -604,6 +605,28 @@ static inline bool has_cap(const struct
> > csched2_vcpu *svc)
> >  return svc->budget != STIME_MAX;
> >  }
> >
> > +static void rb_runq_insert(struct rb_root *root,
> > +   struct csched2_vcpu *svc,
> > +   int *pos)
> > +{
> > +struct csched2_vcpu *entry = NULL;
> >
> I'd call this (something like) 'parent_svc'.
>
> It makes it easier to understand what's actually happening in the loop
> below.
>
> > +struct rb_node **node = &root->rb_node;
> > +struct rb_node *parent = NULL;
> > +
> > +while (*node) {
> > +parent = *node;
> > +entry = rb_entry(parent, struct csched2_vcpu, runq_elem);
> > +if ( svc->credit < entry->credit )
> > +node = &parent->rb_left;
> > +else
> > +node = &parent->rb_right;
> > +
> > +(*pos)++;
> > +}
>
> > @@ -1789,6 +1803,7 @@ static void park_vcpu(struct csched2_vcpu *svc)
> >   * In both cases, we also add it to the list of parked vCPUs of
> > the domain.
> >   */
> >  __set_bit(_VPF_parked, &v->pause_flags);
> > +
> >  if ( vcpu_on_runq(svc) )
> >  {
> >  runq_remove(svc);
> >
> Unrelated change. Don't do it. :-)
>
> > @@ -2087,6 +2102,8 @@ csched2_vcpu_sleep(const struct scheduler *ops,
> > struct vcpu *vc)
> >  }
> >  else if ( vcpu_on_runq(svc) )
> >  {
> > +printk(XENLOG_WARNING "%s : %d : vcpu on runq rb !\n",
> > __func__, __LINE__);
> > +
> >
> So, this, and all the various other lines similar to this are/have been
> useful for debugging, I guess?
>
> That is ok, but I don't think they should land upstream. If you think
> they (or maybe some of them) should, then make it/them proper debugging
> output.
>
> And, probably, do that in its own patch, as it would not be related to
> what the data structure used for the runqueues is.
>
> But again, IMO, you can get rid of all of them.
>
> >  ASSERT(svc->rqd == c2rqd(ops, vc->processor));
> >  update_load(ops, svc->rqd, svc, -1, NOW());
> >  runq_remove(svc);
>
> > @@ -2764,8 +2783,10 @@ csched2_vcpu_migrate(
> >  if ( unlikely(!cpumask_test_cpu(new_cpu,
> > cpupool_domain_cpumask(d))) )
> >  {
> >  ASSERT(system_state == SYS_STATE_suspend);
> > +
> >  if ( vcpu_on_runq(svc) )
> >  {
> >
> Stray new-line again.
>
> > @@ -3206,17 +3227,18 @@ csched2_runtime(const struct scheduler *ops,
> > int cpu,
> >   * 2) If there's someone waiting whose credit is positive,
> >   *run until your credit ~= his.
> >   */
> > -if ( ! list_empty(runq) )
> > +if ( ! RB_EMPTY_ROOT(runq) )
> >  {
> > -struct csched2_vcpu *swait = runq_elem(runq->next);
> > +// Find the left most element, which is the most probable
> > candidate
> > +struct rb_node *node = rb_last(runq);
> >
> Comment style. And I think I'd say "Check the rightmost element in the
> tree, which is the one with the highest credits"
>
> > +struct csched2_vcpu *swait = runq_elem(node);
> >
> I think we can do:
>
>   struct csched2_vcpu *swait = runq_elem(rb_last(runq));
>
> Yeah, matter of taste, mostly. Still...
>
> Anyway, if you keep the code like this, no blanks in-between
> definitions. And you instead want one between the last definition and
> the if below.
>
> >  if ( ! is_idle_vcpu(swait->vcpu)
> >   && swait->credit > 0 )
> >  {
> >  rt_credit = snext->credit - swait->credit;
> >  }
> >  }
> > -
> >  /*
> >   * The

Re: [Xen-devel] [PATCH] xen/sched: Introduce domain_vcpu() helper

[Andrew Cooper]

On 23/01/2019 17:01, Jan Beulich wrote:
 On 23.01.19 at 15:59,  wrote:
>> +static inline struct vcpu *domain_vcpu(const struct domain *d,
>> +   unsigned int vcpu_id)
>> +{
>> +unsigned int idx = array_index_nospec(vcpu_id, d->max_vcpus);
>> +
>> +return idx >= d->max_vcpus ? NULL : d->vcpu[idx];
>> +}
> For an out of bounds incoming vcpu_id, isn't it the case that
> idx then would be zero? In which case you'd return d->vcpu[0]
> instead of NULL?

Speculatively, yes.  array_index_nospec() works by forcing speculative
mis-accesses to operate as if it request had been for index 0.

What matters from a data-leaking perspective is whether d->vcpu[idx],
when executed speculative, ends up being out-of-bounds or not.  i.e.
whether it is distinguishable from a path which can architecturally be
taken.

~Andrew

P.S. index 0 is actually better than NULL on any hardware lacking SMAP,
because you won't potentially use guest-controlled data from 0 during
the subsequent speculation.

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

Re: [Xen-devel] [PATCH 0/2] Dangling fixes for ARM iommu

[Andrii Anisov]

Hello Julien,

On 22.01.19 16:31, Julien Grall wrote:

IMO, the pure fixes, like patch 2, and the first hunk of patch 1 should be OK 
for 4.12.


The first hunk of patch 1 aside, we never supported the new IOMMU bindings nor 
unsharing the P2M. So what do you actually fix?

Supporting unshared P2M will require more work given because the code relies on 
mfn_to_gmfn that is not implemented on Arm. So accepting this patch standalone 
would be misleading as the rest of the series is not going to be merged in Xen 
4.12.

I'm not saying I want something from non-shared p2m push into 4.12.
I would strip the first patch to its first chunk.


Similarly, we don't support new IOMMU bindings. The patch #2 alone is going to 
add more trouble as now Dom0 would not be able to use the IOMMU if it were not 
hidden.

Are you sure we should pass IOMMU devices or related info to Dom0 if they are 
not supported by hypervisor?
The commit message states "We don't passthrough IOMMU device to DOM0 even if it is 
not used by Xen.", and you agreed that some time ago. Could you please clarify why 
you have changed your mind?

--
Sincerely,
Andrii Anisov.

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

[Xen-devel] [PATCH for-4.12] iommu: fix order of arguments in iommu_map call at iommu_hwdom_init

[Roger Pau Monne]

The order of the page_order and the flags parameters are inverted in
the call to iommu_map made in iommu_hwdom_init.

Fixes: e8afe1124cc1 ("iommu: elide flushing for higher order map/unmap 
operations")
Signed-off-by: Roger Pau Monné 
---
Cc: Jan Beulich 
Cc: Paul Durrant 
---
 xen/drivers/passthrough/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 98e6fc35e2..acd17ac2dc 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -226,7 +226,7 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
   == PGT_writable_page) )
 mapping |= IOMMUF_writable;
 
-ret = iommu_map(d, _dfn(dfn), _mfn(mfn), mapping, 0,
+ret = iommu_map(d, _dfn(dfn), _mfn(mfn), 0, mapping,
 &flush_flags);
 
 if ( !rc )
-- 
2.20.1


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

Re: [Xen-devel] [PATCH 0/2] Dangling fixes for ARM iommu

[Julien Grall]

On 23/01/2019 17:53, Andrii Anisov wrote:

Hello Julien,


Hi,


On 22.01.19 16:31, Julien Grall wrote:
IMO, the pure fixes, like patch 2, and the first hunk of patch 1 should be OK 
for 4.12.


The first hunk of patch 1 aside, we never supported the new IOMMU bindings nor 
unsharing the P2M. So what do you actually fix?


Supporting unshared P2M will require more work given because the code relies 
on mfn_to_gmfn that is not implemented on Arm. So accepting this patch 
standalone would be misleading as the rest of the series is not going to be 
merged in Xen 4.12.

I'm not saying I want something from non-shared p2m push into 4.12.


I know and this is not what I implied in my previous e-mail. I pointed out that 
the rest of the series is not planned for Xen 4.12, hence it does not make sense 
to get that patch merged.



I would strip the first patch to its first chunk.


I can queue the first hunk for Xen 4.13.



Similarly, we don't support new IOMMU bindings. The patch #2 alone is going to 
add more trouble as now Dom0 would not be able to use the IOMMU if it were not 
hidden.
Are you sure we should pass IOMMU devices or related info to Dom0 if they are 
not supported by hypervisor?
The commit message states "We don't passthrough IOMMU device to DOM0 even if it 
is not used by Xen.", and you agreed that some time ago. Could you please 
clarify why you have changed your mind?


This statement is only correct if the IOMMU has been actively blacklisted by 
Xen. In the case of the SMMU driver, this is done by arm_smmu_dt_init(). The 
function is only called when the IOMMU has been enabled.


So if you pass "iommu=disabled" to Xen commandline, the SMMU will be accessible 
by Dom0.


When there are no drivers for the IOMMU used, then you end up to assign the 
IOMMU to Dom0. This means that Dom0 can use them.


It does not make sense to remove the properties "iommus", "iommu-map", 
"iommu-map-mask" if Dom0 is already touching the IOMMU. It is either everything 
or nothing.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] Xen-unstable PVHdom0: Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at iommu.c:324

[Roger Pau Monné]

On Wed, Jan 23, 2019 at 12:39:21AM +0100, Sander Eikelenboom wrote:
> On 22/01/2019 17:14, Roger Pau Monné wrote:
> > On Sun, Jan 20, 2019 at 11:09:25PM +0100, Sander Eikelenboom wrote:
> >> On 18/01/2019 18:56, Roger Pau Monné wrote:
> >>> On Fri, Jan 18, 2019 at 03:17:57PM +0100, Sander Eikelenboom wrote:
>  On 18/01/2019 13:50, Roger Pau Monné wrote:
> > On Fri, Jan 18, 2019 at 01:03:04PM +0100, Sander Eikelenboom wrote:
> >> Hi Roger,
> >>
> >> I gave PVH dom0 a spin, see how far I would get.
> >
> > Thanks!
> >
> >> With current xen-unstable unfortunately not that far, i got the splat 
> >> below.
> >
> > Yes, this was already reported:
> >
> > https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg01030.html
> >> If you need more info, would like me to test a patch (or some other 
> >> git tree/branch), 
> >> I will be happy to give it a spin !
> >
> > Paul is working on a fix, but in the meantime just removing the
> > assertions should be fine:
> >
> > ---8<---
> > diff --git a/xen/drivers/passthrough/iommu.c 
> > b/xen/drivers/passthrough/iommu.c
> > index bd1af35a13..98e6fc35e2 100644
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -321,9 +321,6 @@ int iommu_map(struct domain *d, dfn_t dfn, mfn_t 
> > mfn,
> >  if ( !iommu_enabled || !hd->platform_ops )
> >  return 0;
> >  
> > -ASSERT(IS_ALIGNED(dfn_x(dfn), (1ul << page_order)));
> > -ASSERT(IS_ALIGNED(mfn_x(mfn), (1ul << page_order)));
> > -
> >  for ( i = 0; i < (1ul << page_order); i++ )
> >  {
> >  rc = hd->platform_ops->map_page(d, dfn_add(dfn, i), 
> > mfn_add(mfn, i),
> >
> 
>  I gave that a spin and i now get a seemingly endless stream of 
>  IO_PAGE_FAULTs
> >>>
> >>> You shouldn't get those page faults since they are for addresses that
> >>> belong to a reserved region, and that should be mapped into the p2m.
> >>> I've just tested on my AMD box and I'm also seeing errors (albeit
> >>> different ones), so I guess something broke since I last fixed PVH
> >>> Dom0 to boot on AMD hardware.
> >>>
> >>> I've also tested commit:
> >>>
> >>> commit fad6ba64a8c98bebb9374f390cc255fac05237ab (HEAD)
> >>> Author: Roger Pau Monné 
> >>> Date:   Fri Nov 30 12:10:00 2018 +0100
> >>> amd/iommu: skip host bridge devices when updating IOMMU page tables
> >>>
> >>> And it works on my AMD box and I'm able to boot as a PVH Dom0. Can you
> >>> give this commit a spin?
> >>>
> >>> Thanks, Roger.
> >>>
> >>
> >> Hi Roger,
> >>
> >> Tested that commit, but that didn't help.
> > 
> > Thanks! Sorry for the delay, I got sidetracked with something else.
> 
> No problem, it's not too urgent and probably a busy time with the remaining 
> 4.12 stuff.
>  
> > Can you please post the serial log when using the above commit?
> 
> Sure, I attached a log of:
>  - fad6ba64a8c98bebb9374f390cc255fac05237ab  dom0 PVH unsuccesful boot
>  - fad6ba64a8c98bebb9374f390cc255fac05237ab  dom0 PVsuccesful boot

Thanks. So you get the same IO page faults.

I don't seem to be able to reproduce this behaviour on my AMD box, but
that might be just luck. I've been finding some issues today related
to the IOMMU, could you give the following patch a spin and paste the
serial log that you get.

Thanks, Roger.
---8<---
diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index e40d7a7d7b..4fd75d4105 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -241,10 +241,11 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
 
 if ( !hwdom_iommu_map(d, pfn, max_pfn) )
 continue;
-
+#if 0
 if ( paging_mode_translate(d) )
 rc = set_identity_p2m_entry(d, pfn, p2m_access_rw, 0);
 else
+#endif
 rc = iommu_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
IOMMUF_readable | IOMMUF_writable, &flush_flags);
 


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

Re: [Xen-devel] [PATCH 0/2] Dangling fixes for ARM iommu

[Andrii Anisov]

On 23.01.19 20:14, Julien Grall wrote:

I can queue the first hunk for Xen 4.13.

For 4.13? Well I hope we will manage to upstream the whole series until then.


This statement is only correct if the IOMMU has been actively blacklisted by 
Xen. In the case of the SMMU driver, this is done by arm_smmu_dt_init(). The 
function is only called when the IOMMU has been enabled.

So if you pass "iommu=disabled" to Xen commandline, the SMMU will be accessible 
by Dom0.

When there are no drivers for the IOMMU used, then you end up to assign the 
IOMMU to Dom0. This means that Dom0 can use them.

It does not make sense to remove the properties "iommus", "iommu-map", 
"iommu-map-mask" if Dom0 is already touching the IOMMU. It is either everything or nothing.

So that patch should be totally rewritten. At least to support iommu=disabled.

--
Sincerely,
Andrii Anisov.

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

Re: [Xen-devel] [PATCH 0/2] Dangling fixes for ARM iommu

[Julien Grall]

On 23/01/2019 18:29, Andrii Anisov wrote:


On 23.01.19 20:14, Julien Grall wrote:

I can queue the first hunk for Xen 4.13.

For 4.13? Well I hope we will manage to upstream the whole series until then.


If you plan to resend it separately, I can apply to the next branch regardless 
the rest of the series.


It does not make sense to remove the properties "iommus", "iommu-map", 
"iommu-map-mask" if Dom0 is already touching the IOMMU. It is either 
everything or nothing.

So that patch should be totally rewritten. At least to support iommu=disabled.


And the case where IOMMU is enabled but no IOMMU driver is present in Xen.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Kees Cook]

On Thu, Jan 24, 2019 at 4:44 AM Jani Nikula  wrote:
>
> On Wed, 23 Jan 2019, Edwin Zimmerman  wrote:
> > On Wed, 23 Jan 2019, Jani Nikula  wrote:
> >> On Wed, 23 Jan 2019, Greg KH  wrote:
> >> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> >> >> Variables declared in a switch statement before any case statements
> >> >> cannot be initialized, so move all instances out of the switches.
> >> >> After this, future always-initialized stack variables will work
> >> >> and not throw warnings like this:
> >> >>
> >> >> fs/fcntl.c: In function ‘send_sigio_to_task’:
> >> >> fs/fcntl.c:738:13: warning: statement will never be executed 
> >> >> [-Wswitch-unreachable]
> >> >>siginfo_t si;
> >> >>  ^~
> >> >
> >> > That's a pain, so this means we can't have any new variables in { }
> >> > scope except for at the top of a function?

Just in case this wasn't clear: no, it's just the switch statement
before the first "case". I cannot imagine how bad it would be if we
couldn't have block-scoped variables! Heh. :)

> >> >
> >> > That's going to be a hard thing to keep from happening over time, as
> >> > this is valid C :(
> >>
> >> Not all valid C is meant to be used! ;)
> >
> > Very true.  The other thing to keep in mind is the burden of enforcing
> > a prohibition on a valid C construct like this.  It seems to me that
> > patch reviewers and maintainers have enough to do without forcing them
> > to watch for variable declarations in switch statements.  Automating
> > this prohibition, should it be accepted, seems like a good idea to me.
>
> Considering that the treewide diffstat to fix this is:
>
>  18 files changed, 45 insertions(+), 46 deletions(-)
>
> and using the gcc plugin in question will trigger the switch-unreachable
> warning, I think we're good. There'll probably be the occasional
> declarations that pass through, and will get fixed afterwards.

Yeah, that was my thinking as well: it's a rare use, and we get a
warning when it comes up.

Thanks!

-- 
Kees Cook

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

Re: [Xen-devel] [Intel-wired-lan] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jeff Kirsher]

On Wed, 2019-01-23 at 03:03 -0800, Kees Cook wrote:
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
> 
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed [-
> Wswitch-unreachable]
>siginfo_t si;
>  ^~
> 
> Signed-off-by: Kees Cook 

Acked-by: Jeff Kirsher 

For the e1000 changes.

> ---
>  arch/x86/xen/enlighten_pv.c   |  7 ---
>  drivers/char/pcmcia/cm4000_cs.c   |  2 +-
>  drivers/char/ppdev.c  | 20 -
> --
>  drivers/gpu/drm/drm_edid.c|  4 ++--
>  drivers/gpu/drm/i915/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c   |  4 ++--
>  drivers/net/ethernet/intel/e1000/e1000_main.c |  3 ++-
>  drivers/tty/n_tty.c   |  3 +--
>  drivers/usb/gadget/udc/net2280.c  |  5 ++---
>  fs/fcntl.c|  3 ++-
>  mm/shmem.c|  5 +++--
>  net/core/skbuff.c |  4 ++--
>  net/ipv6/ip6_gre.c|  4 ++--
>  net/ipv6/ip6_tunnel.c |  4 ++--
>  net/openvswitch/flow_netlink.c|  7 +++
>  security/tomoyo/common.c  |  3 ++-
>  security/tomoyo/condition.c   |  7 ---
>  security/tomoyo/util.c|  4 ++--
>  18 files changed, 45 insertions(+), 46 deletions(-)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Matthew Wilcox]

On Wed, Jan 23, 2019 at 04:17:30PM +0200, Jani Nikula wrote:
> Can't have:
> 
>   switch (i) {
>   int j;
>   case 0:
>   /* ... */
>   }
> 
> because it can't be turned into:
> 
>   switch (i) {
>   int j = 0; /* not valid C */
>   case 0:
>   /* ... */
>   }
> 
> but can have e.g.:
> 
>   switch (i) {
>   case 0:
>   {
>   int j = 0;
>   /* ... */
>   }
>   }
> 
> I think Kees' approach of moving such variable declarations to the
> enclosing block scope is better than adding another nesting block.

Another nesting level would be bad, but I think this is OK:

switch (i) {
case 0: {
int j = 0;
/* ... */
}
case 1: {
void *p = q;
/* ... */
}
}

I can imagine Kees' patch might have a bad effect on stack consumption,
unless GCC can be relied on to be smart enough to notice the
non-overlapping liveness of the vriables and use the same stack slots
for both.

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

[Xen-devel] [libvirt test] 132411: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 132318
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 132318
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  4fb769f5e02833ecf6ad495af3f3c705364e2d2c
baseline version:
 libvirt  fb0d0d6c5492a427bc4d1813257f6ad77bb0ea70

Last test of basis   132318  2019-01-22 04:18:53 Z1 days
Testing same since   132411  2019-01-23 04:18:45 Z0 days1 attempts


People who touched revisions under test:
  Daniel P. Berrangé 
  Ján Tomko 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd pass



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

Logs, config files, etc. are available at
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/libvirt.git
   fb0d0d6c54..4fb769f5e0  4fb769f5e02833ecf6ad495af3f3c705364e2d2c -> 
xen-tested-master

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

Re: [Xen-devel] Xen-unstable PVHdom0: Assertion 'IS_ALIGNED(dfn_x(dfn), (1ul << page_order))' failed at iommu.c:324

[Sander Eikelenboom]

On 23/01/2019 19:25, Roger Pau Monné wrote:
> On Wed, Jan 23, 2019 at 12:39:21AM +0100, Sander Eikelenboom wrote:
>> On 22/01/2019 17:14, Roger Pau Monné wrote:
>>> On Sun, Jan 20, 2019 at 11:09:25PM +0100, Sander Eikelenboom wrote:
 On 18/01/2019 18:56, Roger Pau Monné wrote:
> On Fri, Jan 18, 2019 at 03:17:57PM +0100, Sander Eikelenboom wrote:
>> On 18/01/2019 13:50, Roger Pau Monné wrote:
>>> On Fri, Jan 18, 2019 at 01:03:04PM +0100, Sander Eikelenboom wrote:
 Hi Roger,

 I gave PVH dom0 a spin, see how far I would get.
>>>
>>> Thanks!
>>>
 With current xen-unstable unfortunately not that far, i got the splat 
 below.
>>>
>>> Yes, this was already reported:
>>>
>>> https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg01030.html
 If you need more info, would like me to test a patch (or some other 
 git tree/branch), 
 I will be happy to give it a spin !
>>>
>>> Paul is working on a fix, but in the meantime just removing the
>>> assertions should be fine:
>>>
>>> ---8<---
>>> diff --git a/xen/drivers/passthrough/iommu.c 
>>> b/xen/drivers/passthrough/iommu.c
>>> index bd1af35a13..98e6fc35e2 100644
>>> --- a/xen/drivers/passthrough/iommu.c
>>> +++ b/xen/drivers/passthrough/iommu.c
>>> @@ -321,9 +321,6 @@ int iommu_map(struct domain *d, dfn_t dfn, mfn_t 
>>> mfn,
>>>  if ( !iommu_enabled || !hd->platform_ops )
>>>  return 0;
>>>  
>>> -ASSERT(IS_ALIGNED(dfn_x(dfn), (1ul << page_order)));
>>> -ASSERT(IS_ALIGNED(mfn_x(mfn), (1ul << page_order)));
>>> -
>>>  for ( i = 0; i < (1ul << page_order); i++ )
>>>  {
>>>  rc = hd->platform_ops->map_page(d, dfn_add(dfn, i), 
>>> mfn_add(mfn, i),
>>>
>>
>> I gave that a spin and i now get a seemingly endless stream of 
>> IO_PAGE_FAULTs
>
> You shouldn't get those page faults since they are for addresses that
> belong to a reserved region, and that should be mapped into the p2m.
> I've just tested on my AMD box and I'm also seeing errors (albeit
> different ones), so I guess something broke since I last fixed PVH
> Dom0 to boot on AMD hardware.
>
> I've also tested commit:
>
> commit fad6ba64a8c98bebb9374f390cc255fac05237ab (HEAD)
> Author: Roger Pau Monné 
> Date:   Fri Nov 30 12:10:00 2018 +0100
> amd/iommu: skip host bridge devices when updating IOMMU page tables
>
> And it works on my AMD box and I'm able to boot as a PVH Dom0. Can you
> give this commit a spin?
>
> Thanks, Roger.
>

 Hi Roger,

 Tested that commit, but that didn't help.
>>>
>>> Thanks! Sorry for the delay, I got sidetracked with something else.
>>
>> No problem, it's not too urgent and probably a busy time with the remaining 
>> 4.12 stuff.
>>  
>>> Can you please post the serial log when using the above commit?
>>
>> Sure, I attached a log of:
>>  - fad6ba64a8c98bebb9374f390cc255fac05237ab  dom0 PVH unsuccesful boot
>>  - fad6ba64a8c98bebb9374f390cc255fac05237ab  dom0 PVsuccesful boot
> 
> Thanks. So you get the same IO page faults.
> 
> I don't seem to be able to reproduce this behaviour on my AMD box, but
> that might be just luck. I've been finding some issues today related
> to the IOMMU, could you give the following patch a spin and paste the
> serial log that you get.

Hi Roger,

Sure, on top of what ?
- fad6ba64a8c98bebb9374f390cc255fac05237ab ?
- xen-unstable ?
- xen-unstable + Paul's patch ?

--
Sander

> Thanks, Roger.
> ---8<---
> diff --git a/xen/drivers/passthrough/x86/iommu.c 
> b/xen/drivers/passthrough/x86/iommu.c
> index e40d7a7d7b..4fd75d4105 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -241,10 +241,11 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain 
> *d)
>  
>  if ( !hwdom_iommu_map(d, pfn, max_pfn) )
>  continue;
> -
> +#if 0
>  if ( paging_mode_translate(d) )
>  rc = set_identity_p2m_entry(d, pfn, p2m_access_rw, 0);
>  else
> +#endif
>  rc = iommu_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
> IOMMUF_readable | IOMMUF_writable, &flush_flags);
>  
> 


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

Re: [Xen-devel] [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Kees Cook]

On Thu, Jan 24, 2019 at 8:18 AM Matthew Wilcox  wrote:
>
> On Wed, Jan 23, 2019 at 04:17:30PM +0200, Jani Nikula wrote:
> > Can't have:
> >
> >   switch (i) {
> >   int j;
> >   case 0:
> >   /* ... */
> >   }
> >
> > because it can't be turned into:
> >
> >   switch (i) {
> >   int j = 0; /* not valid C */
> >   case 0:
> >   /* ... */
> >   }
> >
> > but can have e.g.:
> >
> >   switch (i) {
> >   case 0:
> >   {
> >   int j = 0;
> >   /* ... */
> >   }
> >   }
> >
> > I think Kees' approach of moving such variable declarations to the
> > enclosing block scope is better than adding another nesting block.
>
> Another nesting level would be bad, but I think this is OK:
>
> switch (i) {
> case 0: {
> int j = 0;
> /* ... */
> }
> case 1: {
> void *p = q;
> /* ... */
> }
> }
>
> I can imagine Kees' patch might have a bad effect on stack consumption,
> unless GCC can be relied on to be smart enough to notice the
> non-overlapping liveness of the vriables and use the same stack slots
> for both.

GCC is reasonable at this. The main issue, though, was most of these
places were using the variables in multiple case statements, so they
couldn't be limited to a single block (or they'd need to be manually
repeated in each block, which is even more ugly, IMO).

Whatever the consensus, I'm happy to tweak the patch.

Thanks!

-- 
Kees Cook

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

Re: [Xen-devel] [RFC] virtio_ring: check dma_mem for xen_domain

[Stefano Stabellini]

On Tue, 22 Jan 2019, h...@infradead.org wrote:
> On Tue, Jan 22, 2019 at 11:59:31AM -0800, Stefano Stabellini wrote:
> > >   if (!virtio_has_iommu_quirk(vdev))
> > >   return true;
> > >  
> > > @@ -260,7 +262,7 @@ static bool vring_use_dma_api(struct virtio_device 
> > > *vdev)
> > >* the DMA API if we're a Xen guest, which at least allows
> > >* all of the sensible Xen configurations to work correctly.
> > >*/
> > > - if (xen_domain())
> > > + if (xen_domain() && !dma_dev->dma_mem)
> > >   return true;
> > >  
> > >   return false;
> > 
> > I can see you spotted a real issue, but this is not the right fix. We
> > just need something a bit more flexible than xen_domain(): there are
> > many kinds of Xen domains on different architectures, we basically want
> > to enable this (return true from vring_use_dma_api) only when the xen
> > swiotlb is meant to be used. Does the appended patch fix the issue you
> > have?
> 
> The problem generally is the other way around - if dma_dev->dma_mem
> is set the device decription in the device tree explicitly requires
> using this memory, so we must _always_ use the DMA API.
> 
> The problem is just that that rproc driver absuses the DMA API
> in horrible ways.

If vring_use_dma_api is actually supposed to return true when
dma_dev->dma_mem is set, then both Peng's patch and the patch I wrote
are not fixing the real issue here.

I don't know enough about remoteproc to know where the problem actually
lies though.

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

Re: [Xen-devel] [RFC] virtio_ring: check dma_mem for xen_domain

[h...@infradead.org]

On Wed, Jan 23, 2019 at 01:04:33PM -0800, Stefano Stabellini wrote:
> If vring_use_dma_api is actually supposed to return true when
> dma_dev->dma_mem is set, then both Peng's patch and the patch I wrote
> are not fixing the real issue here.
> 
> I don't know enough about remoteproc to know where the problem actually
> lies though.

The problem is the following:

Devices can declare a specific memory region that they want to use when
the driver calls dma_alloc_coherent for the device, this is done using
the shared-dma-pool DT attribute, which comes in two variants that
would be a little to much to explain here.

remoteproc makes use of that because apparently the device can
only communicate using that region.  But it then feeds back memory
obtained with dma_alloc_coherent into the virtio code.  For that
it calls vmalloc_to_page on the dma_alloc_coherent, which is a huge
no-go for the ĐMA API and only worked accidentally on a few platform,
and apparently arm64 just changed a few internals that made it stop
working for remoteproc.

The right answer is to not use the DMA API to allocate memory from
a device-speficic region, but to tie the driver directly into the
DT reserved memory API in a way that allows it to easilt obtain
a struct device for it.

This is orthogonal to another issue, and that is that hardware
virtio devices really always need to use the DMA API, otherwise
we'll bypass such features as the device specific DMA pools,
DMA offsets, cache flushing, etc, etc.

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

Re: [Xen-devel] [PATCH v3 15/15] argo: validate hypercall arg structures via compat machinery

[Christopher Clark]

On Mon, Jan 21, 2019 at 4:03 AM Jan Beulich  wrote:
>
> >>> On 20.01.19 at 22:18,  wrote:
> > On Thu, Jan 17, 2019 at 3:25 AM Jan Beulich  wrote:
> >>
> >> >>> On 17.01.19 at 08:22,  wrote:
> >
> > 3. A challenge with using the "struct" form, following from the result
> > of point 2, occurs when it's a XEN_GUEST_HANDLE field within the struct.
> > It's not obvious how to declare that field using the "struct" form
> > rather than the "type" form.
> > This affects the argo_iov struct.
>
> Structures containing handles are intentionally not covered
> by the CHECK_* machinery, because handles necessarily
> need translation due to their different widths in 32- and
> 64-bit modes on x86.

ack.

>
> > 4. Macros to perform "struct form" checks cannot be repeated.
> >
> > When using the "struct" form, it's problem when the struct contains two
> > fields of the same compat-translated type.
> >
> > eg. consider the "struct form" version of xen_argo_send_addr, which has
> > two fields of struct xen_argo_addr:
> >
> > typedef struct xen_argo_send_addr
> > {
> > struct xen_argo_addr src;
> > struct xen_argo_addr dst;
> > } xen_argo_send_addr_t;
> >
> > which then generates this in the compat header:
> >
> > #define CHECK_argo_send_addr \
> > CHECK_SIZE_(struct, argo_send_addr); \
> > CHECK_argo_addr; \
> > CHECK_argo_addr
> >
> > and the second macro invocation of CHECK_argo_addr just breaks, with the
> > build failing due to redefinition of a symbol that is already defined.
>
> Hmm, this looks like something that indeed wants fixing.

I have a patch to fix that, that it turns out I will not need but can
post separately if this is still wanted -- copied here for illustration.
(apologies in advance if this gets mail-client mangled here).

diff --git a/xen/tools/get-fields.sh b/xen/tools/get-fields.sh
index 45a0e2e..14c6859 100644
--- a/xen/tools/get-fields.sh
+++ b/xen/tools/get-fields.sh
@@ -438,7 +438,7 @@ build_check ()
 {
echo
echo "#define CHECK_$1 \\"
-   local level=1 fields= kind= id= arrlvl=1 token
+   local level=1 fields= kind= id= arrlvl=1 token suppress_dups=
for token in $2
do
case "$token" in
@@ -470,8 +470,12 @@ build_check ()
[\,\;])
if [ $level = 2 -a -n "$(echo $id | $SED
's,^_pad[[:digit:]]*,,')" ]
then
-   check_field $kind $1 $id "$fields"
-   test "$token" != ";" || fields= id=
+if [ "${suppress_dups#*|$kind $1|}" = "${suppress_dups}" ]
+then
+check_field $kind $1 $id "$fields"
+[ -z "$fields" ] ||
suppress_dups="${suppress_dups:-|}$kind $1|"
+test "$token" != ";" || fields= id=
+fi
fi
;;
esac

On Tue, Jan 22, 2019 at 3:08 AM Jan Beulich  wrote:
>
> >>> On 21.01.19 at 13:03,  wrote:
>  On 20.01.19 at 22:18,  wrote:
> >> The "no repeated checks" problem also occurs when another separate
> >> struct contains a field of a type that has already been checked:
> >> whichever CHECK is performed second will break.
> >>
> >> eg.
> >> typedef struct xen_argo_ring_data_ent
> >> {
> >> struct xen_argo_addr ring;
> >> uint16_t flags;
> >> uint16_t pad;
> >> uint32_t space_required;
> >> uint32_t max_message_size;
> >> } xen_argo_ring_data_ent_t;
> >>
> >> also has a field of type xen_argo_addr, which produces CHECK_argo_addr,
> >> which then fails because that was already tested in
> >> CHECK_argo_send_addr.
> >
> > Hmm, I think the mcinfo example above contradicts this, because
> > struct mcinfo_common is used by multiple other structures.
>
> Due to
>
> CHECK_mcinfo_common;
> # undef xen_mcinfo_common
> # undef CHECK_mcinfo_common
> # define CHECK_mcinfo_common struct mcinfo_common
>
> which I think would be easy enough to use in your case as well
> (until we could perhaps get around and address the underlying
> issue, albeit it's not really clear to me how that should be done).

ack, this technique works for the Argo data structures, so I've
applied it, dropped the previous macro overrides, moved the checks
into the common/argo.c file and dropped common/compat/argo.c,
with each check being added at the same time as the structs go in
through the series.

Christopher

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

[Xen-devel] [examine test] 132441: trouble: broken/fail

[osstest service owner]

flight 132441 examine real [real]
http://logs.test-lab.xenproject.org/osstest/logs/132441/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 examine-laxton1  broken
 examine-rochester0   broken
 examine-rochester1   broken
 examine-arndale-bluewater 2 hosts-allocate broken REGR. vs. 131533
 examine-arndale-metrocentre   2 hosts-allocate broken REGR. vs. 131533
 examine-arndale-lakeside  2 hosts-allocate broken REGR. vs. 131533
 examine-arndale-westfield 2 hosts-allocate broken REGR. vs. 131533

Tests which did not succeed, but are not blocking:
 examine-rochester01 build-check(1)   blocked  n/a
 examine-rochester11 build-check(1)   blocked  n/a
 examine-laxton1   1 build-check(1)   blocked  n/a
 examine-albana0   2 hosts-allocate  broken like 131533
 examine-debina1   2 hosts-allocate  broken like 131533
 examine-albana1   2 hosts-allocate  broken like 131533
 examine-baroque1  2 hosts-allocate  broken like 131533
 examine-debina0   2 hosts-allocate  broken like 131533
 examine-godello1  2 hosts-allocate  broken like 131533
 examine-rimava1   2 hosts-allocate  broken like 131533
 examine-cubietruck-gleizes2 hosts-allocate  broken like 131533
 examine-joubertin02 hosts-allocate  broken like 131533
 examine-cubietruck-metzinger  2 hosts-allocate  broken like 131533
 examine-pinot02 hosts-allocate  broken like 131533
 examine-baroque0  2 hosts-allocate  broken like 131533
 examine-elbling1  2 hosts-allocate  broken like 131533
 examine-fiano02 hosts-allocate  broken like 131533
 examine-cubietruck-picasso2 hosts-allocate  broken like 131533
 examine-pinot12 hosts-allocate  broken like 131533
 examine-fiano12 hosts-allocate  broken like 131533
 examine-elbling0  2 hosts-allocate  broken like 131533
 examine-chardonnay0   2 hosts-allocate  broken like 131533
 examine-cubietruck-braque 2 hosts-allocate  broken like 131533
 examine-chardonnay1   2 hosts-allocate  broken like 131533
 examine-godello0  2 hosts-allocate  broken like 131533
 examine-italia0   2 hosts-allocate  broken like 131533

baseline version:
 flight   131533

jobs:
 examine-albana0  fail
 examine-albana1  fail
 examine-baroque0 fail
 examine-baroque1 fail
 examine-arndale-bluewaterfail
 examine-cubietruck-braquefail
 examine-chardonnay0  fail
 examine-chardonnay1  fail
 examine-debina0  fail
 examine-debina1  fail
 examine-elbling0 fail
 examine-elbling1 fail
 examine-fiano0   fail
 examine-fiano1   fail
 examine-cubietruck-gleizes   fail
 examine-godello0 fail
 examine-godello1 fail
 examine-italia0  fail
 examine-joubertin0   fail
 examine-arndale-lakeside fail
 examine-laxton1  broken  
 examine-arndale-metrocentre  fail
 examine-cubietruck-metzinger fail
 examine-cubietruck-picasso   fail
 examine-pinot0   fail
 examine-pinot1   fail
 examine-rimava1  fail
 examine-rochester0   broken  
 examine-rochester1   broken  
 examine-arndale-westfield   

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

[osstest service owner]

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

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

version targeted for testing:
 xen  e3b418ac491919127211b4d3c692d431061d7c09
baseline version:
 xen  30f5047b2c4e577436b505ba7627f34c3be02014

Last test of basis   132436  2019-01-23 14:00:50 Z0 days
Testing same since   132450  2019-01-23 19:00:41 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  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
   30f5047b2c..e3b418ac49  e3b418ac491919127211b4d3c692d431061d7c09 -> smoke

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

Re: [Xen-devel] [PATCH for-4.12 2/8] xen/arm: p2m: Provide an helper to generate the VTTBR

[Stefano Stabellini]

On Wed, 28 Nov 2018, Julien Grall wrote:
> A follow-up patch will need to generate the VTTBR in a few places.
> 
> Signed-off-by: Julien Grall 
> ---
>  xen/arch/arm/p2m.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 6c76298ebc..8ebf1e8dba 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -47,6 +47,11 @@ static const paddr_t level_masks[] =
>  static const uint8_t level_orders[] =
>  { ZEROETH_ORDER, FIRST_ORDER, SECOND_ORDER, THIRD_ORDER };
>  
> +static uint64_t generate_vttbr(uint16_t vmid, mfn_t root_mfn)
> +{
> +return (mfn_to_maddr(root_mfn) | ((uint64_t)vmid << 48));

Outer brackets are not necessary. Regardless:

Reviewed-by: Stefano Stabellini 


> +}
> +
>  /* Unlock the flush and do a P2M TLB flush if necessary */
>  void p2m_write_unlock(struct p2m_domain *p2m)
>  {
> @@ -1147,7 +1152,7 @@ static int p2m_alloc_table(struct domain *d)
>  
>  p2m->root = page;
>  
> -p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid << 48);
> +p2m->vttbr = generate_vttbr(p2m->vmid, page_to_mfn(p2m->root));
>  
>  /*
>   * Make sure that all TLBs corresponding to the new VMID are flushed
> -- 
> 2.11.0
> 

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