Skip to content

xen: RMRR fix

Tiejun Chen xen-devel 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v8 Latest17 patches 88 repliesIn review
  2. v7 RFC13 patches 166 repliesSuperseded

Patches in v8 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/17tools/hvmloader: link errno.h from xen internal.gitignore tools/firmware/hvmloader/Makefile+7 -10 replies
2/17introduce XEN_DOMCTL_set_rdmdocs/man/xl.cfg.pod.5 docs/misc/vtd.txt tools/libxc/include/xenctrl.h +12 more+196 -014 replies
3/17introduce XENMEM_reserved_device_memory_mapxen/common/compat/memory.c xen/common/memory.c xen/drivers/passthrough/iommu.c +6 more+163 -25 replies
4/17update the existing hypercall to support XEN_DOMCTL_set_rdmxen/common/compat/memory.c xen/common/memory.c xen/drivers/passthrough/vtd/dmar.c +3 more+148 -3922 replies
5/17tools/libxc: introduce hypercall for xc_reserved_device_memory_maptools/libxc/include/xenctrl.h tools/libxc/xc_domain.c+35 -04 replies
6/17tools/libxc: check if modules space is overlapping with reserved device memorytools/libxc/xc_hvm_build_x86.c+79 -153 replies
7/17hvmloader/util: get reserved device memory mapstools/firmware/hvmloader/util.c tools/firmware/hvmloader/util.h+61 -08 replies
8/17hvmloader/mmio: reconcile guest mmio with reserved device memorytools/firmware/hvmloader/pci.c tools/firmware/hvmloader/util.c tools/firmware/hvmloader/util.h+64 -14 replies
9/17hvmloader/ram: check if guest memory is out of reserved device memory mapstools/firmware/hvmloader/e820.c+168 -05 replies
10/17hvmloader/mem_hole_alloc: skip any overlap with reserved device memorytools/firmware/hvmloader/util.c+21 -15 replies
11/17xen/x86/p2m: reject populating for reserved device memory mappingxen/arch/x86/mm/p2m.c xen/include/asm-x86/p2m.h+66 -22 replies
12/17xen/x86/ept: handle reserved device memory in ept_handle_violationxen/arch/x86/hvm/vmx/vmx.c+18 -03 replies
13/17xen/mem_access: don't allow accessing reserved device memoryxen/common/mem_access.c+41 -04 replies
14/17xen/x86/p2m: introduce set_identity_p2m_entryxen/arch/x86/mm/p2m.c xen/include/asm-x86/p2m.h+32 -02 replies
15/17xen:vtd: create RMRR mappingxen/drivers/passthrough/vtd/iommu.c+9 -42 replies
16/17xen/vtd: group assigned device with RMRRxen/drivers/passthrough/vtd/dmar.c xen/drivers/passthrough/vtd/dmar.h xen/drivers/passthrough/vtd/iommu.c+91 -73 replies
17/17xen/vtd: re-enable USB device assignment if enable pci_forcexen/drivers/passthrough/vtd/dmar.h xen/drivers/passthrough/vtd/iommu.c xen/drivers/passthrough/vtd/utils.c+27 -41 reply

Reviews trailers per patch

6 of 17 patches have a Reviewed-by or Acked-by.

PatchJan BeulichKevin Tian
1/17A-b
2/17
3/17A-b
4/17
5/17R-b
6/17R-b
7/17
8/17
9/17
10/17
11/17
12/17
13/17
14/17R-b
15/17A-b
16/17
17/17

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v8 thread

Cover letter [v8][PATCH 00/17] xen: RMRR fix

v8:

A brief summary:

* Rebased on the latest
* We skip creating p2m table associated to RMRR range as Jan and Tim commented And especially, this is identified to setting these tables as INVALID since as you know all p2m tables already are initialized as INVALID. Actually I also tried to reset them as INVALID again, everything is still fine but some warning logs show we're resetting these Already-Invalided p2m entries so just keep skipping these entries.
* Add something into mem_access to skip these RMRR ranges to make sure we're in the same page.
* Provide a new approach to control if
1. we want to check/reserve all RMRR ranges
2. Or just some RMRR ranges specific to those devices we want to PT
This brings a new parameter to enable/disable this, and new domCTL to get SBDF associated to those PT devices.
* Add a new check point in mem_hole_alloc() since we may use this to allocate some ranges living in runtime cycle, like igd_opregion_pgbase.
* Some code itself improvements/corrections.

The details:

0001-tools-hvmloader-link-errno.h-from-xen-internal.patch

This is a new patch but Acked by Jan.

0002-introduce-XEN_DOMCTL_set_rdm.patch

This is a new patch to introduce that parameter and domCTL. Please refer to the patch head description.

0003-introduce-XENMEM_reserved_device_memory_map.patch

This is slightly rebased to make sure all codes should be in PT case, and this is from Jan and Acked by Kevin. To me, I just rebase on the latest.

0004-update-the-existing-hypercall-to-support-XEN_DOMCTL_.patch

This is a new patch since after we introduce that parameter and domCTL, we need to rework that existing hypercall from #4 patch.

Most should be discussed between Jan and me, but I changed something slightly again since I found we were missing some scenarios, like the hypercall caller don't know construct how much buffers to carry forward all necessary entries at first, and additionally, we also need to expose a return value from iommu ops as some caller expects.

So I have to ask Jan take a look at this again. And maybe we can squash this into #4 eventually.

0005-tools-libxc-introduce-hypercall-for-xc_reserved_devi.patch 0006-tools-libxc-check-if-modules-space-is-overlapping-wi.patch 0007-hvmloader-util-get-reserved-device-memory-maps.patch

Just rebase and cleanup.

0008-hvmloader-mmio-reconcile-guest-mmio-with-reserved-de.patch

Just rebase and cleanup, and some code corrections as Jan comments.

0009-hvmloader-ram-check-if-guest-memory-is-out-of-reserv.patch

Just rebase and cleanup, and some code improvements.

0010-hvmloader-mem_hole_alloc-skip-any-overlap-with-reser.patch

This is a new used to address igd_opregion_pgbase as Yang mentioned to me.

0011-xen-x86-p2m-reject-populating-for-reserved-device-me.patch

Refactor some codes to handle such a case. Note please refer to a description above.

0012-xen-x86-ept-handle-reserved-device-memory-in-ept_han.patch

Just improve comments.

0013-xen-mem_access-don-t-allow-accessing-reserved-device.patch

This is a new to sync mem_access after our action in memory populating.

0014-xen-x86-p2m-introduce-set_identity_p2m_entry.patch 0015-xen-vtd-create-RMRR-mapping.patch 0016-xen-vtd-group-assigned-device-with-RMRR.patch

There's nothing to change.

0017-xen-vtd-re-enable-USB-device-assignment-if-enable-pc.patch

Just rebase.

How to reproduce this issu:

* In shared-ept case with Xen.
* Target owns RMRR.
* Do IGD passthrough with Windows guest OS: gfx_passthru=1 pci=["00:02.0"]
* Please use qemu-xen-traditional.

My test machine is BDW with Windows 7.

v7:

This series of patches try to reconcile those remaining problems but just post as RFC to ask for any comments to refine everything.

The current whole scheme is as follows:

1. Reconcile guest mmio with RMRR in pci_setup
2. Reconcile guest RAM with RMRR in e820 table

Then in theory guest wouldn't access any RMRR range.

3. Just initialize all RMRR ranges as p2m_access_n in p2m table:
gfn:mfn:p2m_access_n

Here I think we shouldn't set 1:1 to expose RMRR to guest if guest may never have a device assignment. It can prevent from leaking RMRR.

4. We reset those mappings as 1:1:p2m_mmio_direct:p2m_ram_rw once we have a device assignment.

5. Before we take real device assignment, any access to RMRR may issue ept_handle_violation because of p2m_access_n. Then we just call update_guest_eip() to return.

6. After a device assignment, guest may maliciously access RMRR ranges although we already reserve in e820 table. In the worst-case scenario just that device can't work well. But this behavior should be same as native so I think we shouldn't do anything here.

7. Its not necessary to introduce any flag in ept_set_entry.

First of all, hypervisor/dom0 should be trusted. Any user should make sure they never override any valid RMRR tables without any check. So our original set_identity_p2m_entry() tries to set as follows:

- gfn space unoccupied -> insert mapping; success.
- gfn space already occupied by 1:1 RMRR mapping -> do nothing; success.
- gfn space already occupied by other mapping -> fail.

Now in our case we add a rule:
- if p2m_access_n is set we also set this mapping.

Another reason is that ept_set_entry is called in many scenarios to support its own management, I think we shouldn't corrupt this mechanism and its also difficult to cover all points.

8. We need to take a consideration grouping all devices that have same RMRR range to make sure they're just assigned to one VM.

----------------------------------------------------------------
Jan Beulich (1):
introduce XENMEM_reserved_device_memory_map

Tiejun Chen (16):
tools/hvmloader: link errno.h from xen internal
introduce XEN_DOMCTL_set_rdm
update the existing hypercall to support XEN_DOMCTL_set_rdm tools/libxc: introduce hypercall for xc_reserved_device_memory_map tools/libxc: check if modules space is overlapping with reserved device memory hvmloader/util: get reserved device memory maps
hvmloader/mmio: reconcile guest mmio with reserved device memory hvmloader/ram: check if guest memory is out of reserved device memory maps hvmloader/mem_hole_alloc: skip any overlap with reserved device memory xen/x86/p2m: reject populating for reserved device memory mapping xen/x86/ept: handle reserved device memory in ept_handle_violation xen/mem_access: don't allow accessing reserved device memory xen/x86/p2m: introduce set_identity_p2m_entry
xen:vtd: create RMRR mapping
xen/vtd: group assigned device with RMRR
xen/vtd: re-enable USB device assignment if enable pci_force

 .gitignore                           |   1 +
 docs/man/xl.cfg.pod.5                |   6 ++++++
 docs/misc/vtd.txt                    |  15 ++++++++++++++
 tools/firmware/hvmloader/Makefile    |   7 ++++++-
 tools/firmware/hvmloader/e820.c      | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/firmware/hvmloader/pci.c       |  54 +++++++++++++++++++++++++++++++++++++++++++++++++-
 tools/firmware/hvmloader/util.c      |  90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 tools/firmware/hvmloader/util.h      |   4 ++++
 tools/libxc/include/xenctrl.h        |  11 +++++++++++
 tools/libxc/xc_domain.c              |  58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/libxc/xc_hvm_build_x86.c       |  94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------
 tools/libxl/libxl_create.c           |   3 +++
 tools/libxl/libxl_dm.c               |  47 ++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h         |   4 ++++
 tools/libxl/libxl_types.idl          |   2 ++
 tools/libxl/libxlu_pci.c             |   2 ++
 tools/libxl/xl_cmdimpl.c             |  10 ++++++++++
 xen/arch/x86/hvm/vmx/vmx.c           |  18 +++++++++++++++++
 xen/arch/x86/mm/p2m.c                |  87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 xen/common/compat/memory.c           |  97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 xen/common/mem_access.c              |  41 ++++++++++++++++++++++++++++++++++++++
 xen/common/memory.c                  |  94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 xen/drivers/passthrough/iommu.c      |  10 ++++++++++
 xen/drivers/passthrough/pci.c        |  39 ++++++++++++++++++++++++++++++++++++
 xen/drivers/passthrough/vtd/dmar.c   |  69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 xen/drivers/passthrough/vtd/dmar.h   |   3 +++
 xen/drivers/passthrough/vtd/extern.h |   1 +
 xen/drivers/passthrough/vtd/iommu.c  |  94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 xen/drivers/passthrough/vtd/utils.c  |  18 +++++++++++++++++
 xen/include/asm-x86/hvm/domain.h     |   4 ++++
 xen/include/asm-x86/p2m.h            |  13 ++++++++++++
 xen/include/public/domctl.h          |  21 ++++++++++++++++++++
 xen/include/public/memory.h          |  29 ++++++++++++++++++++++++++-
 xen/include/xen/iommu.h              |   4 ++++
 xen/include/xen/pci.h                |   2 ++
 xen/include/xlat.lst                 |   3 ++-
 xen/xsm/flask/hooks.c                |   1 +
 37 files changed, 1187 insertions(+), 37 deletions(-)

Thanks
Tiejun