It took me a bit of time to understand why __DEFINE_TRAP_ENTRY is
storing the original stack pointer in r11. It is working in pair with
return_traps_entry where sp will be restored from r11.
This is fine because per the AAPCS r11 must be preserved by the
subroutine. So in return_from_trap, r11 will still contain the original
stack pointer.
Add some documentation in the code to point the 2 sides to each other.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
In order to avoid aliasing attacks against the branch predictor on
Cortex A-15, let's invalidate the BTB on guest exit, which can only be
done by invalidating the icache (with ACTLR[0] being set).
We use the same hack as for A12/A17 to perform the vector decoding.
This is based on Linux patch from the kpti branch in [1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
In order to avoid aliasing attackes agains the branch predictor, let's
invalidate the BTB on guest exist. This is made complicated by the fact
that we cannot take a branch invalidating the BTB.
This is based on the fourth version posted by Marc Zyngier on Linux-arm
mailing list (see [1]).
This is part of XSA-254.
[1] https://www.spinics.net/lists/arm-kernel/msg632062.html
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Aliasing attacked against CPU branch predictors can allow an attacker to
redirect speculative control flow on some CPUs and potentially divulge
information from one context to another.
This patch adds initiatial skeleton code behind a new Kconfig option
to enable implementation-specific mitigations against these attacks
for CPUs that are affected.
Most of mitigations will have to be applied when entering to the
hypervisor from the guest context.
Because the attack is against branch predictor, it is not possible to
safely use branch instruction before the mitigation is applied.
Therefore this has to be done in the vector entry before jump to the
helper handling a given exception.
However, on arm32, each vector contain a single instruction. This means
that the hardened vector tables may rely on the state of registers that
does not hold when in the hypervisor (e.g SP is 8 bytes aligned).
Therefore hypervisor code running with guest vectors table should be
minimized and always have IRQs and SErrors masked to reduce the risk to
use them.
This patch provides an infrastructure to switch vector tables before
entering to the guest and when leaving it.
Note that alternative could have been used, but older Xen (4.8 or
earlier) doesn't have support. So avoid using alternative to ease
backporting.
This is part of XSA-254.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
At the moment, the reset vector is defined as .word 0 (e.g andeq r0, r0,
r0).
This is rather unintuitive and will result to execute the trap
undefined. Instead introduce trap helpers for reset and will generate an
error message in the unlikely case that reset will be called.
This is part of XSA-254.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Cortex-A17 and A12 MIDR will be used in a follow-up patch for hardening
the branch predictor.
This is part of XSA-254.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The only difference between all the DEFINE_TRAP_ENTRY_* macros are the
interrupts (Asynchronous Abort, IRQ, FIQ) unmasked.
Rather than duplicating the code, introduce __DEFINE_TRAP_ENTRY macro
that will take the list of interrupts to unmask.
This is part of XSA-254.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Support both a "disable everything" and a "disable all RSB overwriting"
sub-option.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
For a release build, bloat-o-meter reports:
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-5111 (-5111)
function old new delta
x86_emulate 126458 121347 -5111
or in other words, a 4% redunction in code size from this change alone.
The use of __LINE__ is a concern with livepatching, but any livepatch touching
this file is overwhemlingly likely to alter x86_emulate() anyway.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
ARM now unconditionally selects HAS_ALTERNATIVE, which has caused the
!HAS_ALTERNATIVE code in include/asm-arm/alternative.h to bitrot to the point
of failing to compile.
Expand all the CONFIG_HAS_ALTERNATIVE references in ARM code.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Put the opcode into an array and use memcpy. This allows the compiled code to
be written with two movs, rather than 10 mov $imm8's. Also, drop trailing
whitespace in the file.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Current upstream gas silently assumes 32-bit operand size for most
operations where the size can't be inferred from an involved register
(my own one doesn't anymore, which is how I've noticed this). It is pure
luck that the 3 bytes following pvh_boot are currently padding ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Note that this avoids emulating the behavior of VCVTPS2PH found on at
least some Intel CPUs, which update MXCSR even when the memory write
faults.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Most users of decode_register() can be replaced with decode_gpr() right away.
For the few sites which do care about possibly using the legacy byteop
encoding, rename decode_register() to _decode_gpr() (to match its non-legacy
counterpart), and adjust its 'int highbyte_regs' parameter to the more correct
'bool legacy'.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
* Rename to decode_gpr() to be more specific as to its purpose
* Drop the highbyte encoding handling, as no users currently care, and it
unlikely that future users would care.
* Change to a static inline, returning an unsigned long pointer.
Doing so highlights that the "invalid gpr" paths in hvm_mov_{to,from}_cr()
were actually unreachable. All callers already passed in-range GPRs, and
out-of-range GPRs would have hit the BUG() previously.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The positions of GPRs inside struct cpu_user_regs doesn't follow any
particular order, so as compiled, decode_register() becomes a jump table to 16
blocks which calculate the appropriate offset, at a total of 207 bytes.
Instead, pre-compute the offsets at build time and use pointer arithmetic to
calculate the result. By observation, most callers in x86_emulate() inline
and constant-propagate the highbyte_regs value of 0.
The splitting of the general and legacy byte-op cases means that we will now
hit an ASSERT if any code path tries to use the legacy byte-op encoding with a
REX prefix.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This makes the code cleaner because there's no need to declare the
exception_table in assembly, and also fixes the following error when
using clang's integrated assembler:
entry.S:834:15: error: unexpected token in '.rept' directive
.rept 32 - ((. - exception_table) / 8)
^
entry.S:836:14: error: unmatched '.endr' directive
.endr
^
This should be a non-functional change.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
toggle_guest_mode() is only ever being called for 64-bit PV vCPU-s -
replace the 32-bit PV conditional by an ASSERT().
Introduce a local helper without 32-bit PV conditional, to be used by
both pre-existing functions.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
As its name says, it handles a single GMFN only anyway. Note that ARM
needs no adjustment, as it doesn't handle the two types at all.
Also take the opportunity and clean up the handling of XENMAPSPACE_gmfn
a little: There's no point in going through "idx" when capturing the MFN.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
A command line such as "cpuid=no-ibrsb,no-stibp" tickles a bug in
parse_boolean() because the separating comma fails the NUL case.
Instead, check for slen == nlen which accounts for the boundary (if any)
passed via the 'e' parameter.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
At the moment we re-generate the Dom0 GICv3 DT node, by creating the
"reg" property from scratch using our previously parsed and
translated(!) host addresses. However we then write the *absolute*
addresses into the new node, not considering possible "range" mappings
in any of the GIC's parent nodes. So whenever one of the parents has a
non-empty ranges property, Dom0 will wrongly translate the addresses.
Properly incorporating the ranges properties sounds tedious, so let's
just copy the first part of the reg property instead (as we do for GICv2),
since the addresses for Dom0 are identical to those from the hardware.
The mainline kernel DT for the Espressobin board with an Marvell 3720 SoC
has the GIC in such an translated bus, so this patch allows this board
to boot properly (after adding support for the SoC's UART).
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
There are firmware tables out describing the ITS but does not support
LPIs. This will result to a data abort when trying to initialize ITS.
While this can be consider a bug in the Device-Tree, same configuration
boots on Linux. So gate the ITS initialization with the support of LPIs
in the distributor.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
There are Device Tree (e.g for the Foundation Model) out that describes the
ITS but LPIs is not supported by the platform. Booting with such DT will
result to an early Data Abort. The same DT is booting fine with a
baremetal Linux because ITS will be initialized only when LPIs is
supported.
While this is a bug in the DT, I think Xen should be boot with the same
hardware level support (e.g ITS will not be used) as with a baremetal
Linux.
The slight problem is Xen is relying on gicv3_its_host_has_its() to know
if ITS can be used. The list is populated by gicv3_its_{dt,acpi}_init().
It would be theoretically possible to gate those with a check of
GICD_TYPER.LPIS because we don't know yet whether the HW is an actual
GICv3/GICv4.
Looking at the callers of gicv3_its_host_has_its(), they will only be
done after gicv3_its_init() is called. Therefore move the parsing of ITS
information from firmware tables later on.
Note that gicv3_its_init() has been moved at the end of the file to
avoid forward declaration.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Switch it, and the arch infrastructure, to return bool. Drop the unnecessary
rc variable, and remove a redundant assertion from send_global_virq().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
__GET_CURRENT() is dangerous to use, as is easy to confuse with GET_CURRENT(),
but strictly depends on the regster parameter already having the STACK_END
value in it. Also, there is no reason to special case accesses of
current_vcpu differently to other cpuinfo fields.
Expand __GET_CURRENT() in its current users, and remove the macro.
Take the opportunity to replace the GET_CURRENT() in the cstar path which
doesn't need to recalculate STACK_END.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
xl: libxl.c:339: libxl_defbool_val: Assertion `!libxl_defbool_is_default(db)' failed.
This happens because initiate_domain_create() checks for type != HVM, then
pokes at the hvm union. Check for == HVM instead so the union access is
correctly guarded.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
c/s 94450e36bf removed XEN_DOMCTL_getmemlist entirely, but missed adjusting
the XSM side of things. As far as I can tell, 'pagelist' wasn't even offered
to dom0 in default policy.
Also, drop the stale struct xen_domctl_getmemlist which was missed from the
same changeset.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
On selftests failure, print a very visible warning instead of crashing
over an ASSERT.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Adjust to print extra information in the case of a failure
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Switch the PV message to match the wording of the PVH side, use the same
number of ***'s, explicitly identify PV vs PVH, set the log level at INFO, and
print the real domid (which won't be 0 in pv-shim mode).
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
c/s 4ddf474e2 "tools/xen-mceinj: Pass in GPA when injecting through
MSR_MCI_ADDR" removed the remaining user of hypercall.
It has been listed as broken, deprecated and wont-fix since XSA-74, so take
this opportunity to remove it completely.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
On contemporary hardware, setting IBRS/STIBP has a performance impact on
adjacent hyperthreads. It is therefore recommended to clear the setting
before becoming idle, to avoid an idle core preventing adjacent userspace
execution from running at full performance.
Care must be taken to ensure there are no ret or indirect branch instructions
between spec_ctrl_{enter,exit}_idle() invocations, which are forced always
inline. Care must also be taken to avoid using spec_ctrl_enter_idle() between
flushing caches and becoming idle, in cases where that matters.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Issuing an IBPB command flushes the Branch Target Buffer, so that any poison
left by one vcpu won't remain when beginning to execute the next.
The cost of IBPB is substantial, and skipped on transition to idle, as Xen's
idle code is robust already. All transitions into vcpu context are fully
serialising in practice (and under consideration for being retroactively
declared architecturally serialising), so a cunning attacker cannot use SP1 to
try and skip the flush.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
See the logic and comments in init_speculation_mitigations() for further
details.
There are two controls for RSB overwriting, because in principle there are
cases where it might be safe to forego rsb_native (Off the top of my head,
SMEP active, no 32bit PV guests at all, no use of vmevent/paging subsystems
for HVM guests, but I make no guarantees that this list of restrictions is
exhaustive).
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This patch is deliberately arranged to be easy to revert if/when alternatives
patching becomes NMI/#MC safe.
For safety, there must be a dispatch serialising instruction in (what is
logically) DO_SPEC_CTRL_ENTRY so that, in the case that Xen needs IBRS set in
context, an attacker can't speculate around the WRMSR and reach an indirect
branch within the speculation window.
Using conditionals opens this attack vector up, so the else clause gets an
LFENCE to force the pipeline to catch up before continuing. This also covers
the safety of RSB conditional, as execution it is guaranteed to either hit the
WRMSR or LFENCE.
One downside of not using alternatives is that there unconditionally an LFENCE
in the IST path in cases where we are not using the features from IBRS-capable
microcode.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
ret instructions are speculated directly to values recorded in the Return
Stack Buffer/Return Address Stack, as there is no uncertainty in well-formed
code. Guests can take advantage of this in two ways:
1) If they can find a path in Xen which executes more ret instructions than
call instructions. (At least one in the waitqueue infrastructure,
probably others.)
2) Use the fact that the RSB/RAS in hardware is actually a circular stack
without a concept of empty. (When it logically empties, stale values
will start being used.)
To mitigate, overwrite the RSB on entry to Xen with gadgets which will capture
and contain rogue speculation.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
We need to be able to either set or clear IBRS in Xen context, as well as
restore appropriate guest values in guest context. See the documentation in
asm-x86/spec_ctrl_asm.h for details.
With the contemporary microcode, writes to %cr3 are slower when SPEC_CTRL.IBRS
is set. Therefore, the positioning of SPEC_CTRL_{ENTRY/EXIT}* is important.
Ideally, the IBRS_SET/IBRS_CLEAR hunks might be positioned either side of the
%cr3 change, but that is rather more complicated to arrange, and could still
result in a guest controlled value in SPEC_CTRL during the %cr3 change,
negating the saving if the guest chose to have IBRS set.
Therefore, we optimise for the pre-Skylake case (being far more common in the
field than Skylake and later, at the moment), where we have a Xen-preferred
value of IBRS clear when switching %cr3.
There is a semi-unrelated bugfix, where various asm_defn.h macros have a
hidden dependency on PAGE_SIZE, which results in an assembler error if used in
a .macro definition.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Intel specifies IBRS/IBPB (combined, in a single bit) and STIBP as a separate
bit. AMD specifies IBPB alone in a 3rd bit.
AMD's IBPB is a subset of Intel's combined IBRS/IBPB. For performance
reasons, administrators might wish to express "IBPB only" even on Intel
hardware, so we allow the AMD bit to be used for this purpose.
The behaviour of STIBP is more complicated.
It is our current understanding that STIBP will be advertised on HT-capable
hardware irrespective of whether HT is enabled, but not advertised on
HT-incapable hardware. However, for ease of virtualisation, STIBP's
functionality is ignored rather than reserved by microcode/hardware on
HT-incapable hardware.
For guest safety, we treat STIBP as special, always override the toolstack
choice, and always advertise STIBP if IBRS is available. This removes the
corner case where STIBP is not advertised, but the guest is running on
HT-capable hardware where it does matter.
Finally as a bugfix, update the libxc CPUID logic to understand the e8b
feature leaf, which has the side effect of also offering CLZERO to guests on
applicable hardware.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Both options are independently choseable in KConfig, but currently a DEBUG
build without FRAME_POINTER is left to the compilers default choice, not the
users choice.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Julien Grall