Tamas K Lengyel [Fri, 11 Mar 2022 20:43:55 +0000 (20:43 +0000)]
x86/monitor: Add new monitor event to catch all vmexits
Add monitor event that hooks the vmexit handler allowing for both sync and
async monitoring of events. With async monitoring an event is placed on the
monitor ring for each exit and the rest of the vmexit handler resumes normally.
If there are additional monitor events configured those will also place their
respective events on the monitor ring.
With the sync version an event is placed on the monitor ring but the handler
does not get resumed, thus the sync version is only useful when the VM is not
expected to resume normally after the vmexit. Our use-case is primarily with
the sync version with VM forks where the fork gets reset after sync vmexit
event, thus the rest of the vmexit handler can be safely skipped. This is
very useful when we want to avoid Xen crashing the VM under any circumstance,
for example during fuzzing. Collecting all vmexit information regardless of
the root cause makes it easier to reason about the state of the VM on the
monitor side, hence we opt to receive all events, even for external interrupt
and NMI exits and let the monitor agent decide how to proceed.
Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
---
v5: wrap vmexit fields in arch.vmx structures in the public vm_event ABI
Tamas K Lengyel [Tue, 26 Apr 2022 18:25:34 +0000 (14:25 -0400)]
x86/mem_sharing: make fork_reset more configurable
Alow specify distinct parts of the fork VM to be reset. This is useful when a
fuzzing operation involves mapping in only a handful of pages that are known
ahead of time. Throwing these pages away just to be re-copied immediately is
expensive, thus allowing to specify partial resets can speed things up.
Also allow resetting to be initiated from vm_event responses as an
optiomization.
Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
v5: split toolstack part to separate patch and use ASSERT_UNREACHABLE
v4: No change
v3: Rebase on simpler approach after dropping empty_p2m feature
v2: address review comments and add more sanity checking
David Vrabel [Tue, 26 Apr 2022 08:33:01 +0000 (10:33 +0200)]
page_alloc: assert IRQs are enabled in heap alloc/free
Heap pages can only be safely allocated and freed with interrupts
enabled as they may require a TLB flush which may send IPIs (on x86).
Normally spinlock debugging would catch calls from the incorrect
context, but not from stop_machine_run() action functions as these are
called with spin lock debugging disabled.
Enhance the assertions in alloc_xenheap_pages() and
alloc_domheap_pages() to check interrupts are enabled. For consistency
the same asserts are used when freeing heap pages.
As an exception, when only 1 PCPU is online, allocations are permitted
with interrupts disabled as any TLB flushes would be local only. This
is necessary during early boot.
Signed-off-by: David Vrabel <dvrabel@amazon.co.uk> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Daniel P. Smith [Tue, 26 Apr 2022 08:30:31 +0000 (10:30 +0200)]
xsm/flask: code style formatting
This is a quick code style cleanup patch for xsm/flask. The files flask_op.c
and hooks.c are Xen specific, thus full code style rules were applied. The
remaining files are from Linux and therefore only trailing whitespace was
remove from those files.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Jan Beulich [Tue, 26 Apr 2022 08:25:54 +0000 (10:25 +0200)]
IOMMU: make domctl handler tolerate NULL domain
Besides the reporter's issue of hitting a NULL deref when !CONFIG_GDBSX,
XEN_DOMCTL_test_assign_device can legitimately end up having NULL passed
here, when the domctl was passed DOMID_INVALID.
Fixes: 71e617a6b8f6 ("use is_iommu_enabled() where appropriate...") Reported-by: Cheyenne Wills <cheyenne.wills@gmail.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Juergen Gross <jgross@suse.com>
Today iommu_do_domctl() is being called from arch_do_domctl() in the
"default:" case of a switch statement. This has led already to crashes
due to unvalidated parameters.
Fix that by moving the call of iommu_do_domctl() to the main switch
statement of do_domctl().
Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> # Arm
tools/libs/light: don't set errno to a negative value
Setting errno to a negative value makes no sense.
Fixes: e78e8b9bb649 ("libxl: Add interface for querying hypervisor about PCI topology") Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
tools/libs/guest: don't set errno to a negative value
Setting errno to a negative error value makes no sense.
Fixes: cb99a64029c9 ("libxc: arm: allow passing a device tree blob to the guest") Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
tools/libs/ctrl: don't set errno to a negative value
The claimed reason for setting errno to -1 is wrong. On x86
xc_domain_pod_target() will set errno to a sane value in the error
case.
Fixes: ff1745d5882b ("tools: libxl: do not set the PoD target on ARM") Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
tools/libs/evtchn: don't set errno to negative values
Setting errno to a negative value makes no sense.
Fixes: 6b6500b3cbaa ("tools/libs/evtchn: Add support for restricting a handle") Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Bobby Eshleman [Tue, 28 Sep 2021 20:30:29 +0000 (13:30 -0700)]
xen: Split x86/debugger.h into common and arch specific parts
With all the non-CONFIG_CRASH_DEBUG functionality moved elsewhere, split
x86/debugger.h in two, with the stubs and explanation moved to xen/debugger.h.
In particular, this means that arches only need to provide an $arch/debugger.h
if they implement CONFIG_CRASH_DEBUG, and ARM's stub can be deleted.
Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Julien Grall <jgrall@amazon.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Wed, 20 Apr 2022 13:40:45 +0000 (14:40 +0100)]
x86/debugger: Misc cleanup prior to splitting
* Remove inappropriate semicolon from debugger_trap_immediate().
* Try to explain what debugger_trap_fatal() is doing, and write it in a more
legible way.
* Drop unnecessary includes. This includes common/domain.c which doesn't use
any debugger functionality, even prior to this cleanup.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Wed, 20 Apr 2022 12:48:05 +0000 (13:48 +0100)]
x86/gdbstub: Clean up includes
common/gdbstub.c wants struct gdb_context but only gets it transitively
through asm/debugger.h. None of */gdbstub.c should include asm/debugger.h so
include xen/gdbstub.h instead.
Forward declare struct cpu_user_regs in xen/gdbstub.h so it doesn't depend on
the include order to compile.
x86/setup.c doesn't need xen/gdbstub.h at all, so drop it.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Wed, 20 Apr 2022 00:38:32 +0000 (01:38 +0100)]
x86/gdbsx: Move domain_pause_for_debugger() into gdbsx
domain_pause_for_debugger() is guest debugging (CONFIG_GDBSX) not host
debugging (CONFIG_CRASH_DEBUG).
Move it into the new gdbsx.c to drop the (incorrect) ifdefary, and provide a
static inline in the !CONFIG_GDBSX case so callers can optimise away
everything rather than having to emit a call to an empty function.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Bobby Eshleman [Tue, 28 Sep 2021 20:30:26 +0000 (13:30 -0700)]
x86/gdbsx: Rename debug.c to gdbsx.c
debug.c contains only dbg_rw_mem(). Rename it to gdbsx.c.
Move gdbsx_guest_mem_io(), and the prior setup of iop->remain, from domctl.c
to gdbsx.c, merging it with dbg_rw_mem().
Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Bobby Eshleman [Tue, 28 Sep 2021 20:30:25 +0000 (13:30 -0700)]
x86/debugger: Remove debugger_trap_entry()
debugger_trap_entry() is unrelated to the other contents of debugger.h. It is
a no-op for everything other than #DB/#BP, and for those it invokes guest
debugging (CONFIG_GDBSX) not host debugging (CONFIG_CRASH_DEBUG).
The reason it is a no-op for gdbstub is related to the fact that it's
description is inappropriate for any kind of useful debugging. In normal
debugging, gdb only sees things which manifest as signals; it doesn't see
things which the kernel resolves itself (some #PF, #NM, etc). Furthermore,
without a mechanism to invoke pv_inject_event(), the current infrastructure
will livelock on faults from guest context.
As such, there is no plausible future matching it's description. Any work to
do something better than the current nothing will have to design something
more coherent.
Therefore, simplify everything by expanding debugger_trap_entry() into its two
non-empty locations, fixing bugs with their positioning (vs early exceptions
and curr not being safe to deference) and for #DB, deferring the pause until
the changes in %dr6 are saved to v->arch.dr6 so the debugger can actually see
which condition triggered. This also removes some logically dead code from
do_trap(), where the compiler can't prove that #DB/#BP are handled by
different codepaths.
Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Thu, 21 Apr 2022 14:23:37 +0000 (15:23 +0100)]
xen/build: Fix MAP rule when called in isolation
Now that `make MAP` might rebuild $(TARGET), it needs removing from
no-dot-config-targets.
Otherwise the build eventually fails with:
CPP arch/x86/asm-macros.i
arch/x86/asm-macros.c:1:10: fatal error: asm/asm-defns.h: No such file or
directory
1 | #include <asm/asm-defns.h>
| ^~~~~~~~~~~~~~~~~
Fixes: e1e72198213b ("xen/build: Fix dependency for the MAP rule") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Fri, 22 Apr 2022 12:56:23 +0000 (14:56 +0200)]
x86/build: make linking work again with ld incapable of generating build ID
The retaining of .note.* in a PT_NOTE segment requires a matching
program header to be present in the first place. Drop the respective
conditional and adjust mkelf32 to deal with (ignore) the potentially
present but empty extra segment (but have the new code be generic by
dropping any excess trailing entirely empty segments).
Fixes: dedb0aa42c6d ("x86/build: use --orphan-handling linker option if available") Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Fri, 22 Apr 2022 12:54:59 +0000 (14:54 +0200)]
AMD/IOMMU: drop stray TLB flush
I think this flush was overlooked when flushing was moved out of the
core (un)mapping functions. The flush the caller is required to invoke
anyway will satisfy the needs resulting from the splitting of a
superpage.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Fri, 22 Apr 2022 12:54:16 +0000 (14:54 +0200)]
IOMMU: have vendor code announce supported page sizes
Generic code will use this information to determine what order values
can legitimately be passed to the ->{,un}map_page() hooks. For now all
ops structures simply get to announce 4k mappings (as base page size),
and there is (and always has been) an assumption that this matches the
CPU's MMU base page size (eventually we will want to permit IOMMUs with
a base page size smaller than the CPU MMU's).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Rahul Singh <rahul.singh@arm.com>
Jan Beulich [Fri, 22 Apr 2022 12:53:13 +0000 (14:53 +0200)]
VT-d: limit page table population in domain_pgd_maddr()
I have to admit that I never understood why domain_pgd_maddr() wants to
populate all page table levels for DFN 0. I can only assume that despite
the comment there what is needed is population just down to the smallest
possible nr_pt_levels that the loop later in the function may need to
run to. Hence what is needed is the minimum of all possible
iommu->nr_pt_levels, to then be passed into addr_to_dma_page_maddr()
instead of literal 1.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Fri, 22 Apr 2022 12:52:40 +0000 (14:52 +0200)]
VT-d: have callers specify the target level for page table walks
In order to be able to insert/remove super-pages we need to allow
callers of the walking function to specify at which point to stop the
walk.
For intel_iommu_lookup_page() integrate the last level access into
the main walking function.
dma_pte_clear_one() gets only partly adjusted for now: Error handling
and order parameter get put in place, but the order parameter remains
ignored (just like intel_iommu_map_page()'s order part of the flags).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Fri, 22 Apr 2022 12:51:37 +0000 (14:51 +0200)]
AMD/IOMMU: have callers specify the target level for page table walks
In order to be able to insert/remove super-pages we need to allow
callers of the walking function to specify at which point to stop the
walk. (For now at least gcc will instantiate just a variant of the
function with the parameter eliminated, so effectively no change to
generated code as far as the parameter addition goes.)
Instead of merely adjusting a BUG_ON() condition, convert it into an
error return - there's no reason to crash the entire host in that case.
Leave an assertion though for spotting issues early in debug builds.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Add a minimal ARM32 smoke test based on qemu-system-arm, as provided by
the test-artifacts qemu container. The minimal test simply boots Xen
(built from previous build stages) and Dom0.
The test needs a working kernel and minimal initrd for dom0. Instead of
building our own kernel and initrd, which would mean maintaining one or
two more builting scripts under automation/, we borrow a kernel and
initrd from distros.
For the kernel we pick the Debian Bullseye kernel, which has everything
we need already built-in. However, we cannot use the Debian Bullseye
initrd because it is 22MB and the large size causes QEMU to core dump.
Instead, use the tiny busybox-based rootfs provided by Alpine Linux,
which is really minimal: just 2.5MB. Note that we cannot use the Alpine
Linux kernel because that doesn't boot on Xen.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> Reviewed-by: Michal Orzel <michal.orzel@arm.com>
Andrew Cooper [Thu, 14 Apr 2022 09:33:01 +0000 (10:33 +0100)]
x86/build: Rework binary conversion for boot/{cmdline,reloc}.c
There is no need to opencode .got.plt size check; it can be done with linker
asserts instead. Extend the checking to all dynamic linkage sections, and
drop the $(OBJDUMP) pass.
Furthermore, instead of removing .got.plt specifically, take only .text when
converting to a flat binary. This makes the process invariant of .text's
position relative to the start of the binary, which avoids needing to discard
all sections, and removes the need to work around sections that certain
linkers are unhappy discarding.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
David Vrabel [Wed, 20 Apr 2022 08:55:01 +0000 (10:55 +0200)]
x86/mm: avoid inadvertently degrading a TLB flush to local only
If the direct map is incorrectly modified with interrupts disabled,
the required TLB flushes are degraded to flushing the local CPU only.
This could lead to very hard to diagnose problems as different CPUs will
end up with different views of memory. Although, no such issues have yet
been identified.
Change the check in the flush_area() macro to look at system_state
instead. This defers the switch from local to all later in the boot
(see xen/arch/x86/setup.c:__start_xen()). This is fine because
additional PCPUs are not brought up until after the system state is
SYS_STATE_smp_boot.
Signed-off-by: David Vrabel <dvrabel@amazon.co.uk> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Wed, 20 Apr 2022 08:54:26 +0000 (10:54 +0200)]
VT-d: refuse to use IOMMU with reserved CAP.ND value
The field taking the value 7 (resulting in 18-bit DIDs when using the
calculation in cap_ndoms(), when the DID fields are only 16 bits wide)
is reserved. Instead of misbehaving in case we would encounter such an
IOMMU, refuse to use it.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Wed, 20 Apr 2022 08:53:57 +0000 (10:53 +0200)]
VT-d: plug memory leaks in iommu_alloc()
While 97af062b89d5 ("IOMMU/x86: maintain a per-device pseudo domain ID")
took care of not making things worse, plugging pre-existing leaks wasn't
the purpose of that change; they're not security relevant after all.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Wed, 20 Apr 2022 08:52:13 +0000 (10:52 +0200)]
IOMMU/x86: drop locking from quarantine_init() hooks
Prior extension of these functions to enable per-device quarantine page
tables already didn't add more locking there, but merely left in place
what had been there before. But really locking is unnecessary here:
We're running with pcidevs_lock held (i.e. multiple invocations of the
same function [or their teardown equivalents] are impossible, and hence
there are no "local" races), while all consuming of the data being
populated here can't race anyway due to happening sequentially
afterwards, and unlike ordinary domains' page tables quarantine ones
are never modified once fully constructed. See also the comment in
struct arch_pci_dev.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
include/public: add command result definitions to vscsiif.h
The result field of struct vscsiif_response is lacking a detailed
definition. Today the Linux kernel internal scsi definitions are being
used, which is not a sane interface for a PV device driver.
Add macros to change that by using today's values in the XEN namespace.
Peng Fan [Tue, 19 Apr 2022 04:39:26 +0000 (12:39 +0800)]
xen/arm: Add i.MX lpuart driver
The i.MX LPUART Documentation:
https://www.nxp.com/webapp/Download?colCode=IMX8QMIEC
Chatper 13.6 Low Power Universal Asynchronous Receiver/
Transmitter (LPUART)
Tested-by: Henry Wang <Henry.Wang@arm.com> Signed-off-by: Peng Fan <peng.fan@nxp.com> Reviewed-by: Michal Orzel <michal.orzel@arm.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Michal Orzel [Thu, 14 Apr 2022 09:58:43 +0000 (11:58 +0200)]
xen/arm: Make use of DT_MATCH_TIMER in make_timer_node
DT_MATCH_TIMER stores the compatible timer ids and as such should be
used in all the places where we need to refer to them. make_timer_node
explicitly lists the same ids as the ones defined in DT_MATCH_TIMER so
make use of this macro instead.
Signed-off-by: Michal Orzel <michal.orzel@arm.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Modify the gdbsx_guest_mem_io() interface to take the already known
domain pointer as parameter instead of the domid. This enables to
remove some more code further down the call tree.
Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
A hypervisor built without CONFIG_GDBSX will crash in case the
XEN_DOMCTL_gdbsx_guestmemio domctl is being called, as the call will
end up in iommu_do_domctl() with d == NULL:
It used to be permitted to pass DOMID_IDLE to dbg_rw_mem(), which is why the
special case skipping the domid checks exists. Now that it is only permitted
to pass proper domids, remove the special case, making 'd' always valid.
Reported-by: Cheyenne Wills <cheyenne.wills@gmail.com> Fixes: e726a82ca0dc ("xen: make gdbsx support configurable") Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Wed, 13 Apr 2022 10:36:03 +0000 (12:36 +0200)]
IOMMU: log appropriate SBDF
To handle phantom devices, several functions are passed separate "devfn"
arguments besides a PCI device. In such cases we want to log the phantom
device's coordinates instead of the main one's. (Note that not all of
the instances being changed are fallout from the referenced commit.)
Fixes: 1ee1441835f4 ("print: introduce a format specifier for pci_sbdf_t") Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Wed, 13 Apr 2022 10:35:17 +0000 (12:35 +0200)]
AMD/IOMMU: replace a few PCI_BDF2()
struct pci_dev has the wanted value directly available; use it. Note
that this fixes a - imo benign - mistake in reassign_device(): The unity
map removal ought to be based on the passed in devfn (as is the case on
the establishing side). This is benign because the mappings would be
removed anyway a little later, when the "main" device gets processed.
While there also limit the scope of two variables in that function.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Anthony PERARD [Wed, 13 Apr 2022 10:33:21 +0000 (12:33 +0200)]
build: adding out-of-tree support to the xen build
This implement out-of-tree support, there's two ways to create an
out-of-tree build tree (after that, `make` in that new directory
works):
make O=build
mkdir build; cd build; make -f ../Makefile
also works with an absolute path for both.
This implementation only works if the source tree is clean, as we use
VPATH.
This patch copies most new code with handling out-of-tree build from
Linux v5.12.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com> Acked-by: Julien Grall <jgrall@amazon.com> Tested-by: Julien Grall <jgrall@amazon.com> Acked-by: Ross Lagerwall <ross.lagerwall@citrix.com> # livepatch
Jan Beulich [Mon, 11 Apr 2022 10:30:09 +0000 (12:30 +0200)]
x86/boot: obtain video info from boot loader
With MB2 the boot loader may provide this information, allowing us to
obtain it without needing to enter real mode (assuming we don't need to
set a new mode from "vga=", but can instead inherit the one the
bootloader may have established).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Mon, 11 Apr 2022 10:29:14 +0000 (12:29 +0200)]
x86/boot: make "vga=current" work with graphics modes
GrUB2 can be told to leave the screen in the graphics mode it has been
using (or any other one), via "set gfxpayload=keep" (or suitable
variants thereof). In this case we can avoid doing another mode switch
ourselves. This in particular avoids possibly setting the screen to a
less desirable mode: On one of my test systems the set of modes
reported available by the VESA BIOS depends on whether the interposed
KVM switch has that machine set as the active one. If it's not active,
only modes up to 1024x768 get reported, while when active 1280x1024
modes are also included. For things to always work with an explicitly
specified mode (via the "vga=" option), that mode therefore needs be a
1024x768 one.
For some reason this only works for me with "multiboot2" (and
"module2"); "multiboot" (and "module") still forces the screen into text
mode, despite my reading of the sources suggesting otherwise.
For starters I'm limiting this to graphics modes; I do think this ought
to also work for text modes, but
- I can't tell whether GrUB2 can set any text mode other than 80x25
(I've only found plain "text" to be valid as a "gfxpayload" setting),
- I'm uncertain whether supporting that is worth it, since I'm uncertain
how many people would be running their systems/screens in text mode,
- I'd like to limit the amount of code added to the realmode trampoline.
For starters I'm also limiting mode information retrieval to raw BIOS
accesses. This will allow things to work (in principle) also with other
boot environments where a graphics mode can be left in place. The
downside is that this then still is dependent upon switching back to
real mode, so retrieving the needed information from multiboot info is
likely going to be desirable down the road.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Henry Wang <Henry.Wang@arm.com>
Michal Orzel [Mon, 11 Apr 2022 07:03:00 +0000 (09:03 +0200)]
xen: Populate xen.lds.h and make use of its macros
Populate header file xen.lds.h with the first portion of macros storing
constructs common to x86 and arm linker scripts. Replace the original
constructs with these helpers.
No functional improvements to x86 linker script.
Making use of common macros improves arm linker script with:
- explicit list of debug sections that otherwise are seen as "orphans"
by the linker. This will allow to fix issues after enabling linker
option --orphan-handling one day,
- extended list of discarded section to include: .discard, destructors
related sections, .fini_array which can reference .text.exit,
- sections not related to debugging that are placed by ld.lld. Even
though we do not support linking with LLD on Arm, these sections do
not cause problem to GNU ld,
As we are replacing hardcoded boundary specified as an argument to ALIGN
function with POINTER_ALIGN, this changes the alignment in HYPFS_PARAM
construct for arm32 from 8 to 4. It is fine as there are no 64bit values
used in struct param_hypfs.
Please note that this patch does not aim to perform the full sync up
between the linker scripts. It creates a base for further work.
Signed-off-by: Michal Orzel <michal.orzel@arm.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Julien Grall <jgrall@amazon.com>
Michal Orzel [Mon, 11 Apr 2022 07:02:59 +0000 (09:02 +0200)]
xen: Introduce a header to store common linker scripts content
Both x86 and arm linker scripts share quite a lot of common content.
It is difficult to keep syncing them up, thus introduce a new header
in include/xen called xen.lds.h to store the internals mutual to all
the linker scripts.
Include this header in linker scripts for x86 and arm.
This patch serves as an intermediate step before populating xen.lds.h
and making use of its content in the linker scripts later on.
Signed-off-by: Michal Orzel <michal.orzel@arm.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <jgrall@amazon.com>
Add Dom0less to SUPPORT.md to clarify its support status. The feature is
mature enough and small enough to make it security supported.
Clarify that dom0less DomUs memory is not scrubbed at boot when
bootscrub=on or bootscrub=off are passed as Xen command line parameters,
and no XSAs will be issued for that.
Jason Andryuk [Fri, 8 Apr 2022 12:51:52 +0000 (14:51 +0200)]
x86/irq: skip unmap_domain_pirq XSM during destruction
xsm_unmap_domain_irq was seen denying unmap_domain_pirq when called from
complete_domain_destroy as an RCU callback. The source context was an
unexpected, random domain. Since this is a xen-internal operation,
going through the XSM hook is inapproriate.
Check d->is_dying and skip the XSM hook when set since this is a cleanup
operation for a domain being destroyed.
Suggested-by: Roger Pau Monné <roger.pau@citrix.com> Signed-off-by: Jason Andryuk <jandryuk@gmail.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Fri, 8 Apr 2022 12:50:29 +0000 (14:50 +0200)]
x86/P2M: p2m.c is HVM-only
This only requires moving p2m_percpu_rwlock elsewhere (ultimately I
think all P2M locking should go away as well when !HVM, but this looks
to require further code juggling). The two other unguarded functions are
already unneeded (by virtue of DCE) when !HVM.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Fri, 8 Apr 2022 12:48:45 +0000 (14:48 +0200)]
paged_pages field is MEM_PAGING-only
Conditionalize it and its uses accordingly.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Fri, 8 Apr 2022 12:47:56 +0000 (14:47 +0200)]
shr_pages field is MEM_SHARING-only
Conditionalize it and its uses accordingly. The main goal though is to
demonstrate that x86's p2m_teardown() is now empty when !HVM, which in
particular means the last remaining use of p2m_lock() in this cases goes
away.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Fri, 8 Apr 2022 12:47:11 +0000 (14:47 +0200)]
x86/p2m: re-arrange {,__}put_gfn()
All explicit callers of __put_gfn() are in HVM-only code and hold a valid
P2M pointer in their hands. Move the paging_mode_translate() check out of
there into put_gfn(), renaming __put_gfn() and making its GFN parameter
type-safe.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Fri, 8 Apr 2022 12:46:30 +0000 (14:46 +0200)]
x86/P2M: derive HVM-only variant from __get_gfn_type_access()
Introduce an inline wrapper dealing with the non-translated-domain case,
while stripping that logic from the main function, which gets renamed to
p2m_get_gfn_type_access(). HVM-only callers can then directly use the
main function.
Along with renaming the main function also make its and the new inline
helper's GFN parameters type-safe.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Fri, 8 Apr 2022 12:45:37 +0000 (14:45 +0200)]
x86/P2M: p2m_get_page_from_gfn() is HVM-only
This function is the wrong layer to go through for PV guests. It happens
to work, but produces results which aren't fully consistent with
get_page_from_gfn(). The latter function, however, cannot be used in
map_domain_gfn() as it may not be the host P2M we mean to act on.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Fri, 8 Apr 2022 12:44:05 +0000 (14:44 +0200)]
x86/P2M: split out init/teardown functions
Mostly just code movement, and certainly no functional change intended.
In p2m_final_teardown() the calls to p2m_teardown_{alt,nested}p2m() need
to be guarded by an is_hvm_domain() check now, though. This matches
p2m_init(). And p2m_is_logdirty_range() also gets moved inside the (so
far) adjacent #ifdef.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Fri, 8 Apr 2022 12:41:51 +0000 (14:41 +0200)]
x86/P2M: PoD, altp2m, and nested-p2m are HVM-only
There's no need to initialize respective data for PV domains. Note that
p2m_teardown_{alt,nested}p2m() will handle the lack-of-initialization
case fine.
As a result, despite PV domains having a host P2M associated with them
and hence using XENMEM_get_pod_target on such may not be a real problem,
calling p2m_pod_set_mem_target() for a PV domain is surely wrong, even
if benign at present. Add a guard there as well.
In p2m_pod_demand_populate() the situation is a little different: This
function is reachable only for HVM domains anyway, but following from
other PoD functions only ever acting on the host P2M (and hence PoD
entries only ever existing in host P2Ms), assert and bail from there for
non-host-P2Ms.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Fri, 8 Apr 2022 12:39:43 +0000 (14:39 +0200)]
x86/mm: split set_identity_p2m_entry() into PV and HVM parts
..., moving the former into the new physmap.c. Also call the new
functions directly from arch_iommu_hwdom_init() and
vpci_make_msix_hole(), as the PV/HVM split is explicit there.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Track whether symbols belong to ignored sections in order to avoid
applying relocations referencing those symbols. The address of such
symbols won't be resolved and thus the relocation will likely fail or
write garbage to the destination.
Return an error in that case, as leaving unresolved relocations would
lead to malfunctioning payload code.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Tested-by: Bjoern Doebel <doebel@amazon.de> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
A side effect of ignoring such sections is that symbols belonging to
them won't be resolved, and that could make relocations belonging to
other sections that reference those symbols fail.
For example it's likely to have an empty .altinstr_replacement with
symbols pointing to it, and marking the section as ignored will
prevent the symbols from being resolved, which in turn will cause any
relocations against them to fail.
In order to solve this do not ignore sections with 0 size, only ignore
sections that don't have the SHF_ALLOC flag set.
Special case such empty sections in move_payload so they are not taken
into account in order to decide whether a livepatch can be safely
re-applied after a revert.
Fixes: 98b728a7b2 ('livepatch: Disallow applying after an revert') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Tested-by: Bjoern Doebel <doebel@amazon.de> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Anthony PERARD [Thu, 7 Apr 2022 15:58:44 +0000 (17:58 +0200)]
build: shuffle main Makefile
Reorganize a bit the Makefile ahead of patch
"build: adding out-of-tree support to the xen build"
We are going to want to calculate all the $(*srctree) and $(*objtree)
once, when we can calculate them. This can happen within the
"$(root-make-done)" guard, in an out-of-tree build scenario, so move
those variable there.
$(XEN_ROOT) is going to depends on the value of $(abs_srctree) so
needs to move as well. "Kbuild.include" also depends on $(srctree).
Next, "Config.mk" depends on $(XEN_ROOT) and $(TARGET_*ARCH) depends
on "Config.mk" so those needs to move as well.
This should only be code movement without functional changes.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Anthony PERARD [Thu, 7 Apr 2022 15:57:44 +0000 (17:57 +0200)]
build: specify source tree in include/ for prerequisite
When doing an out-of-tree build, and thus setting VPATH,
GNU Make 3.81 on Ubuntu Trusty complains about Circular dependency of
include/Makefile and include/xlat.lst and drop them. The build fails
later due to headers malformed.
This might be due to bug #13529
"Incorrect circular dependancy"
https://savannah.gnu.org/bugs/?13529
which was fixed in 3.82.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Anthony PERARD [Thu, 7 Apr 2022 15:56:53 +0000 (17:56 +0200)]
build: rework "headers*.chk" prerequisite in include/
Listing public headers when out-of-tree build are involved becomes
more annoying where every path to every headers needs to start with
"$(srctree)/$(src)", or $(wildcard ) will not work. This means more
repetition. ( "$(srcdir)" is a shortcut for "$(srctree)/$(src)" )
This patch attempt to reduce the amount of duplication and make better
use of make's meta programming capability. The filters are now listed
in a variable and don't have to repeat the path to the headers files
as this is added later as needed.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Anthony PERARD [Thu, 7 Apr 2022 15:56:00 +0000 (17:56 +0200)]
build: replace $(BASEDIR) and use $(srctree)
$(srctree) is a better description for the source directory than
$(BASEDIR) that has been used for both source and build directory
(which where the same).
This adds $(srctree) to a few path where make's VPATH=$(srctree) won't
apply. And replace $(BASEDIR) by $(srctree).
Introduce "$(srcdir)" as a shortcut for "$(srctree)/$(src)" as the
later is used often enough.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com> # XSM
Andrew Cooper [Wed, 6 Apr 2022 21:40:20 +0000 (22:40 +0100)]
x86/cpuid: Clobber CPUID leaves 0x800000{1d..20} in policies
c/s 1a914256dca5 increased the AMD max leaf from 0x8000001c to 0x80000021, but
did not adjust anything in the calculate_*_policy() chain. As a result, on
hardware supporting these leaves, we read the real hardware values into the
raw policy, then copy into host, and all the way into the PV/HVM default
policies.
All 4 of these leaves have enable bits (first two by TopoExt, next by SEV,
next by PQOS), so any software following the rules is fine and will leave them
alone. However, leaf 0x8000001d takes a subleaf input and at least two
userspace utilities have been observed to loop indefinitely under Xen (clearly
waiting for eax to report "no more cache levels").
Such userspace is buggy, but Xen's behaviour isn't great either.
In the short term, clobber all information in these leaves. This is a giant
bodge, but there are complexities with implementing all of these leaves
properly.
Fixes: 1a914256dca5 ("x86/cpuid: support LFENCE always serialising CPUID bit") Link: https://github.com/QubesOS/qubes-issues/issues/7392 Reported-by: fosslinux <fosslinux@aussies.space> Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Thu, 7 Apr 2022 10:31:16 +0000 (12:31 +0200)]
VT-d: avoid infinite recursion on domain_context_mapping_one() error path
Despite the comment there infinite recursion was still possible, by
flip-flopping between two domains. This is because prev_dom is derived
from the DID found in the context entry, which was already updated by
the time error recovery is invoked. Simply introduce yet another mode
flag to prevent rolling back an in-progress roll-back of a prior
mapping attempt.
Also drop the existing recursion prevention for having been dead anyway:
Earlier in the function we already bail when prev_dom == domain.
Fixes: 8f41e481b485 ("VT-d: re-assign devices directly") Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Thu, 7 Apr 2022 10:30:19 +0000 (12:30 +0200)]
VT-d: avoid NULL deref on domain_context_mapping_one() error paths
First there's a printk() which actually wrongly uses pdev in the first
place: We want to log the coordinates of the (perhaps fake) device
acted upon, which may not be pdev.
Then it was quite pointless for eb19326a328d ("VT-d: prepare for per-
device quarantine page tables (part I)") to add a domid_t parameter to
domain_context_unmap_one(): It's only used to pass back here via
me_wifi_quirk() -> map_me_phantom_function(). Drop the parameter again.
Finally there's the invocation of domain_context_mapping_one(), which
needs to be passed the correct domain ID. Avoid taking that path when
pdev is NULL and the quarantine state is what would need restoring to.
This means we can't security-support non-PCI-Express devices with RMRRs
(if such exist in practice) any longer; note that as of trhe 1st of the
two commits referenced below assigning them to DomU-s is unsupported
anyway.
Fixes: 8f41e481b485 ("VT-d: re-assign devices directly") Fixes: 14dd241aad8a ("IOMMU/x86: use per-device page tables for quarantining")
Coverity ID: 1503784 Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Thu, 7 Apr 2022 10:29:03 +0000 (12:29 +0200)]
VT-d: don't needlessly look up DID
If get_iommu_domid() in domain_context_unmap_one() fails, we better
wouldn't clear the context entry in the first place, as we're then unable
to issue the corresponding flush. However, we have no need to look up the
DID in the first place: What needs flushing is very specifically the DID
that was in the context entry before our clearing of it.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
platform/cpufreq: add public defines for CPUFREQ_SHARED_TYPE_
The values set in the shared_type field of xen_processor_performance
have so far relied on Xen and Linux having the same
CPUFREQ_SHARED_TYPE_ defines, as those have never been part of the
public interface.
Formalize by adding the defines for the allowed values in the public
header, while renaming them to use the XEN_CPUPERF_SHARED_TYPE_ prefix
for clarity.
Set the Xen internal defines for CPUFREQ_SHARED_TYPE_ using the newly
introduced XEN_CPUPERF_SHARED_TYPE_ public defines in order to avoid
unnecessary code churn. While there also drop
CPUFREQ_SHARED_TYPE_NONE as it's unused.
Fixes: 2fa7bee0a0 ('Get ACPI Px from dom0 and choose Px controller') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Thu, 7 Apr 2022 06:34:58 +0000 (08:34 +0200)]
x86/boot: fold branches in video handling code
Using Jcc to branch around a JMP is necessary only in pre-386 code,
where Jcc is limited to disp8. Use the opposite Jcc directly in two
places. Since it's adjacent, also convert an ORB to TESTB.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Thu, 7 Apr 2022 06:34:07 +0000 (08:34 +0200)]
x86/boot: simplify mode_table
There's no point in writing 80x25 text mode information via multiple
insns all storing immediate values. The data can simply be included
first thing in the vga_modes table, allowing the already present
REP MOVSB to take care of everything in one go.
While touching this also correct a related but stale comment.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Thu, 7 Apr 2022 06:33:09 +0000 (08:33 +0200)]
x86/EFI: retrieve EDID
When booting directly from EFI, obtaining this information from EFI is
the only possible way. And even when booting with a boot loader
interposed, it's more clean not to use legacy BIOS calls for this
purpose. (The downside being that there are no "capabilities" that we
can retrieve the EFI way.)
To achieve this we need to propagate the handle used to obtain the
EFI_GRAPHICS_OUTPUT_PROTOCOL instance for further obtaining an
EFI_EDID_*_PROTOCOL instance, which has been part of the spec since 2.5.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> # Arm, common Acked-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Bertrand Marquis <bertrand.marquis@arm.com> #arm
Jan Beulich [Thu, 7 Apr 2022 06:29:33 +0000 (08:29 +0200)]
x86/P2M: introduce p2m_{add,remove}_page()
Rename guest_physmap_add_entry() to p2m_add_page(); make
guest_physmap_remove_page() a trivial wrapper around p2m_remove_page().
This way callers can use suitable pairs of functions (previously
violated by hvm/grant_table.c).
In HVM-specific code further avoid going through the guest_physmap_*()
layer, and instead use the two new/renamed functions directly.
Ultimately the goal is to have guest_physmap_...() functions cover all
types of guests, but p2m_...() dealing only with translated ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Tue, 5 Apr 2022 12:24:18 +0000 (14:24 +0200)]
IOMMU/x86: use per-device page tables for quarantining
Devices with RMRRs / unity mapped regions, due to it being unspecified
how/when these memory regions may be accessed, may not be left
disconnected from the mappings of these regions (as long as it's not
certain that the device has been fully quiesced). Hence even the page
tables used when quarantining such devices need to have mappings of
those regions. This implies installing page tables in the first place
even when not in scratch-page quarantining mode.
This is CVE-2022-26361 / part of XSA-400.
While for the purpose here it would be sufficient to have devices with
RMRRs / unity mapped regions use per-device page tables, extend this to
all devices (in scratch-page quarantining mode). This allows the leaf
pages to be mapped r/w, thus covering also memory writes (rather than
just reads) issued by non-quiescent devices.
Set up quarantine page tables as late as possible, yet early enough to
not encounter failure during de-assign. This means setup generally
happens in assign_device(), while (for now) the one in deassign_device()
is there mainly to be on the safe side.
As to the removal of QUARANTINE_SKIP() from domain_context_unmap_one():
I think this was never really needed there, as the function explicitly
deals with finding a non-present context entry. Leaving it there would
require propagating pgd_maddr into the function (like was done by "VT-d:
prepare for per-device quarantine page tables" for
domain_context_mapping_one()).
In VT-d's DID allocation function don't require the IOMMU lock to be
held anymore: All involved code paths hold pcidevs_lock, so this way we
avoid the need to acquire the IOMMU lock around the new call to
context_set_domain_id().
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Tue, 5 Apr 2022 12:19:42 +0000 (14:19 +0200)]
IOMMU/x86: drop TLB flushes from quarantine_init() hooks
The page tables just created aren't hooked up yet anywhere, so there's
nothing that could be present in any TLB, and hence nothing to flush.
Dropping this flush is, at least on the VT-d side, a prereq to per-
device domain ID use when quarantining devices, as dom_io isn't going
to be assigned a DID anymore: The warning in get_iommu_did() would
trigger.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Tue, 5 Apr 2022 12:19:10 +0000 (14:19 +0200)]
IOMMU/x86: maintain a per-device pseudo domain ID
In order to subsequently enable per-device quarantine page tables, we'll
need domain-ID-like identifiers to be inserted in the respective device
(AMD) or context (Intel) table entries alongside the per-device page
table root addresses.
Make use of "real" domain IDs occupying only half of the value range
coverable by domid_t.
Note that in VT-d's iommu_alloc() I didn't want to introduce new memory
leaks in case of error, but existing ones don't get plugged - that'll be
the subject of a later change.
The VT-d changes are slightly asymmetric, but this way we can avoid
assigning pseudo domain IDs to devices which would never be mapped while
still avoiding to add a new parameter to domain_context_unmap().
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Tue, 5 Apr 2022 12:18:48 +0000 (14:18 +0200)]
VT-d: prepare for per-device quarantine page tables (part II)
Replace the passing of struct domain * by domid_t in preparation of
per-device quarantine page tables also requiring per-device pseudo
domain IDs, which aren't going to be associated with any struct domain
instances.
No functional change intended (except for slightly adjusted log message
text).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Tue, 5 Apr 2022 12:18:26 +0000 (14:18 +0200)]
VT-d: prepare for per-device quarantine page tables (part I)
Arrange for domain ID and page table root to be passed around, the latter in
particular to domain_pgd_maddr() such that taking it from the per-domain
fields can be overridden.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Tue, 5 Apr 2022 12:18:04 +0000 (14:18 +0200)]
AMD/IOMMU: re-assign devices directly
Devices with unity map ranges, due to it being unspecified how/when
these memory ranges may get accessed, may not be left disconnected from
their unity mappings (as long as it's not certain that the device has
been fully quiesced). Hence rather than tearing down the old root page
table pointer and then establishing the new one, re-assignment needs to
be done in a single step.
This is CVE-2022-26360 / part of XSA-400.
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Similarly quarantining scratch-page mode relies on page tables to be
continuously wired up.
To avoid complicating things more than necessary, treat all devices
mostly equally, i.e. regardless of their association with any unity map
ranges. The main difference is when it comes to updating DTEs, which need
to be atomic when there are unity mappings. Yet atomicity can only be
achieved with CMPXCHG16B, availability of which we can't take for given.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Tue, 5 Apr 2022 12:17:42 +0000 (14:17 +0200)]
VT-d: re-assign devices directly
Devices with RMRRs, due to it being unspecified how/when the specified
memory regions may get accessed, may not be left disconnected from their
respective mappings (as long as it's not certain that the device has
been fully quiesced). Hence rather than unmapping the old context and
then mapping the new one, re-assignment needs to be done in a single
step.
This is CVE-2022-26359 / part of XSA-400.
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Similarly quarantining scratch-page mode relies on page tables to be
continuously wired up.
To avoid complicating things more than necessary, treat all devices
mostly equally, i.e. regardless of their association with any RMRRs. The
main difference is when it comes to updating context entries, which need
to be atomic when there are RMRRs. Yet atomicity can only be achieved
with CMPXCHG16B, availability of which we can't take for given.
The seemingly complicated choice of non-negative return values for
domain_context_mapping_one() is to limit code churn: This way callers
passing NULL for pdev don't need fiddling with.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Tue, 5 Apr 2022 12:17:21 +0000 (14:17 +0200)]
VT-d: drop ownership checking from domain_context_mapping_one()
Despite putting in quite a bit of effort it was not possible to
establish why exactly this code exists (beyond possibly sanity
checking). Instead of a subsequent change further complicating this
logic, simply get rid of it.
Take the opportunity and move the respective unmap_vtd_domain_page() out
of the locked region.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
This is to make more obvious that nothing outside of domain_iommu(d)
actually changes or is otherwise needed by the function.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Tue, 5 Apr 2022 12:16:10 +0000 (14:16 +0200)]
VT-d: fix add/remove ordering when RMRRs are in use
In the event that the RMRR mappings are essential for device operation,
they should be established before updating the device's context entry,
while they should be torn down only after the device's context entry was
successfully cleared.
Also switch to %pd in related log messages.
Fixes: fa88cfadf918 ("vt-d: Map RMRR in intel_iommu_add_device() if the device has RMRR") Fixes: 8b99f4400b69 ("VT-d: fix RMRR related error handling") Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Tue, 5 Apr 2022 12:15:33 +0000 (14:15 +0200)]
VT-d: fix (de)assign ordering when RMRRs are in use
In the event that the RMRR mappings are essential for device operation,
they should be established before updating the device's context entry,
while they should be torn down only after the device's context entry was
successfully updated.
Also adjust a related log message.
This is CVE-2022-26358 / part of XSA-400.
Fixes: 8b99f4400b69 ("VT-d: fix RMRR related error handling") Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Tue, 5 Apr 2022 12:12:27 +0000 (14:12 +0200)]
VT-d: correct ordering of operations in cleanup_domid_map()
The function may be called without any locks held (leaving aside the
domctl one, which we surely don't want to depend on here), so needs to
play safe wrt other accesses to domid_map[] and domid_bitmap[]. This is
to avoid context_set_domain_id()'s writing of domid_map[] to be reset to
zero right away in the case of it racing the freeing of a DID.
For the interaction with context_set_domain_id() and did_to_domain_id()
see the code comment.
{check_,}cleanup_domid_map() are called with pcidevs_lock held or during
domain cleanup only (and pcidevs_lock is also held around
context_set_domain_id()), i.e. racing calls with the same (dom, iommu)
tuple cannot occur.
domain_iommu_domid(), besides its use by cleanup_domid_map(), has its
result used only to control flushing, and hence a stale result would
only lead to a stray extra flush.
This is CVE-2022-26357 / XSA-399.
Fixes: b9c20c78789f ("VT-d: per-iommu domain-id") Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
projects / people / tklengyel / xen.git / log