Despite the best intentions at the time, the current recommendation
lead to very long identifiers, bordering on the max limit we've chosen
for certification.
One observation is that we do have static analysis which will
highlight if duplicate guards are created accidentally.
Therefore, relax the recommendations and in particular remove the
specific tie to the directory structure. This has the other advantage
of being more similar to other projects.
This will hopefully mean there's less churn getting the tree in shape,
and a random contributor is more likely to pick an appropriate guard
given no specific knowledge of Xen.
As always, it's something reviewers and maintainers should be aware
of, and to advise on.
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
... when there really are only a few instructions in line.
In some cases, reformat to reduce left-hand margine space.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
... when there really are only a few instructions in line.
In some cases, reformat to reduce left-hand margine space.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Compilers estimate the size of an asm() block for inlining purposes.
Constructs with embedded metadata (BUG_FRAME, ALTERNATIVE, EXTABLE, etc)
appear large, depsite often only being a handful of instructions. asm
inline() overrides the estimation to identify the block as being small.
This has a substantial impact on inlining decisions, expected to be for the
better given that the compiler has a more accurate picture to work with.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
For asm() statements, clobbers are disjoint from inputs and outputs.
Therefore, run_in_exception_handler() specifying an input and clobbering
BUG_FN_REG forces the compiler to set up fn in the wrong regsiter, just to
have the code block move it into the correct register.
Instead, use `register asm()` to tell the compiler which register fn really
wants to be in. This results in better code generation.
Bloat-o-meter reports:
ARM64:
Function old new delta
dump_registers 356 348 -8
ARM32:
ns16550_poll 52 48 -4
dump_registers 432 428 -4
The other instruction dropped in ARM64's dump_registers() is an alignment nop.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
This reverts commit 6065a05adf.
The discussed "proper fix" has now been implemented, and the #DF path no
longer writes out-of-bounds. Restore the proper #DF IST pointer.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
_show_registers() prints the data selectors from struct cpu_user_regs, but
these fields are sometimes out-of-bounds. See commit 6065a05adf
("x86/traps: 'Fix' safety of read_registers() in #DF path").
There are 3 callers of _show_registers():
1. vcpu_show_registers(), which always operates on a scheduled-out vCPU,
where v->arch.user_regs (or aux_regs on the stack) is always in-bounds.
2. show_registers() where regs is always an on-stack frame. regs is copied
into a local variable first (which is an OoB read for constructs such as
WARN()), before being modified (so no OoB write).
3. do_double_fault(), where regs is adjacent to the stack guard page, and
written into directly. This is an out of bounds read and write, with a
bodge to avoid the writes hitting the guard page.
Include the data segment selectors in struct extra_state, and use those fields
instead of the fields in regs. This resolves the OoB write on the #DF path.
Resolve the OoB read in show_registers() by doing a partial memcpy() rather
than full structure copy. This is temporary until we've finished untangling
the vm86 fields fully.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
... in preference to the crs[8] array. This avoids abusing crs[5..7] for the
fs/gs bases, giving them proper named fields instead, and avoids storage for
cr1 which is unused in the x86 architecture.
In show_registers(), remove a redundant read_cr2(). read_registers() already
did the same, and it is only the PV path which needs to override with
arch_get_cr2().
In vcpu_show_registers(), express the gsb/gss decision using SWAP(). The
determination is going to get even more complicated under FRED.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Since commit 434596bbd4 ("x86/smpboot: Write the top-of-stack block in
cpu_smpboot_alloc()"), smp_processor_id() is unconditionally usable on APs.
Drop the global variable.
Also drop the parameter from start_secondary(). It was introduced as unused
in commit e9ac3bbcca ("Move initial stack-pointer adjustment into assembly
bootstrap code.") in 2005. At the time, the caller was a shared codepath with
__start_xen() with a parameter on the stack, but that never mattered for
start_secondary() which ultimately reset_stack_and_jump()'s out of context.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
It is easier to express feature word 17 in terms of word 16 + [32, 64) as
that's how the layout is given in documentation.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The Indirect Target Selection speculative vulnerability means that indirect
branches (including RETs) are unsafe when in the first half of a cacheline.
This means it's not safe for logic using the stubs to write raw 0xc3's.
Introduce place_ret() which, for now, writes a raw 0xc3 but will contain
additional logic when return thunks are in use.
stub_selftest() doesn't strictly need to be converted as they only run on
boot, but doing so gets us a partial test of place_ret() too.
No functional change.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The Indirect Target Selection speculative vulnerability means that indirect
branches (including RETs) are unsafe when in the first half of a cacheline.
clear_bhb_loops() has a precise layout of branches. The alignment for
performance cause the RETs to always be in an unsafe position, and converting
those to return thunks changes the branching pattern. While such a conversion
is believed to be safe, clear_bhb_loops() is also a performance-relevant
fastpath, so (mis)align the RETs to be in a safe position.
No functional change.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
In order to protect against ITS, Xen needs to start using return thunks.
Therefore the advice in XSA-466 becomes relevant, and the hypercall_page needs
to be removed.
Implement early_hypercall(), with infrastructure to figure out the correct
instruction on first use. Use ALTERNATIVE()s to result in inline hypercalls,
including the ALT_NOT() form so we only need a single synthetic feature bit.
No overall change.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Use the top bit of a->cpuid to express inverted polarity. This requires
stripping the top bit back out when performing the sanity checks.
Despite only being used once, create a replace boolean to express the decision
more clearly in _apply_alternatives().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Rule 13.2 states: "The value of an expression and its persistent side effects
shall be the same under all permitted evaluation orders".
Eclair complains about a Rule 13.2 violations because validate_far_branch()
assigns to rc, and the entirety of commit_far_branch() is also assigned to rc.
I'm unsure that the complaint is accurate, but rewriting commit_far_branch()
to use the comma operator seems to make Eclair happy.
Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The text for CONFIG_INDIRECT_THUNK isn't really correct, and was already stale
by the time speculative vulnerabilities hit the headlines in 2018. It is
specifically an out-of-line-ing mechanism, and retpoline is one of several
safety sequences used.
Some of this boilerplate has been copied into all other options, and isn't
interesting for the target audience given that they're all in a "Speculative
Hardening" menu.
Reword it to be more concise.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
This loop is for_each_set_bit() in disguise.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
* For vpic_get_priority(), introduce a common ror8() helper in plain C. One
thing that I can't persuade the compiler to realise is that a non-zero
value rotated is still non-zero, so use __builtin_clz() to help the
optimiser out.
* vpic_ioport_write() can be simplified to just for_each_set_bit(), which
avoids spilling pending to the stack each loop iteration. Changing pending
from unsigned int to uint8_t isn't even strictly necessary given the
underlying types of vpic->isr and vpic->irr, but done so clarity.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This condition is called VMFail(valid) in the SDM.
No functional change.
Fixes: fc3db01db6 ("x86/vmx: Rework VMX wrappers using `asm goto()`")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The new toolchain baseline understands the RTM instructions.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The type used for pagetable attributes/permissions is currently unsigned int,
but needs to become architecture dependent as PPC needs unsigned long.
Introduce mm-types.h to house pte_attr_t.
Given the new toolchain baseline, we can use __has_include() now to remove the
need for boilerplate on most architectures.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
They're only modified by set_in_mcu_opt_ctrl() which is an __init function.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Building Xen with Clang-17 yields the following warning:
ld: warning: orphan section `.debug_str_offsets' from `prelink.o' being placed in section `.debug_str_offsets'
ld: ./.xen.efi.0xffff82d040000000.0:/4: section below image base
ld: ./.xen.efi.0xffff82d040000000.1:/4: section below image base
ld: warning: orphan section `.debug_str_offsets' from `prelink.o' being placed in section `.debug_str_offsets'
ld: xen.efi:/4: section below image base
Set the alignment to 4 as it holds 4-byte values, despite the fact that Clang
appears to only use 1.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
With altcall handled separately, the special case in _apply_alternatives() is
unused and can be dropped. The force parameter (used to signify the seal
pass) can be removed too.
In turn, nmi_apply_alternatives() no longer needs to call
_apply_alternatives() on the second pass.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
With all the infrastructure in place, switch from using ALTERNATIVE() to
simply populating .alt_call_sites.
Before, _apply_alternatives() would devirtualise in two passes; the first
being opportunistic, and the second (signified by the force parameter) sealing
any call with a still-NULL function pointer.
Now, all devirtualising is performed together, at the point in time of the
second pass previously. The call to seal_endbr64() needs delaying until after
apply_alt_calls() is complete, or we have a narrow window with real indirect
branches and no ENDBR64 instructions.
Under the hood, the following changes are happening:
Section Old size New size Change (%)
.alt_call_sites 0 0x00730 +0x0730
.altinstructions 0x1350a 0x11fe0 -0x152a (-7%)
.altinstr_replacement 0x015f2 0x00e35 -0x07bd (-23%)
The changes aren't quite equal because inlining is affected by the smaller
asm() block. Nevertheless, the metadata is held in 1/3 of the space, and
there are no CALL instructions held in the replacement section any more.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The new altcall scheme uses an .alt_call_sites section. Wire this up in very
much the same way as the .altinstructions section, although there is less
sanity checking necessary.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Encoding altcalls as regular alternatives leads to an unreasonable amount of
complexity in _apply_alternatives().
Introduce apply_alt_calls(), and an .alt_call_sites section which simply
tracks the source address (relative, to save on space). That's literally all
that is needed in order to devirtualise the function pointers.
apply_alt_calls() is mostly as per _apply_alternatives(), except the size is
known to be 6 bytes. Drop the logic for JMP *RIPREL, as there's no support
for tailcall optimisations, nor a feasbile plan on how to introduce support.
Pad with a redundant prefix to avoid needing a separate NOP on the end.
Wire it up in nmi_apply_alternatives(), although the section is empty at this
juncture so nothing happens in practice.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
We are going to need to reposition the call in a change with several moving
parts.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
nmi_apply_alternatives() is soon going to need to dispatch to multiple
functions, and a force parameter is not a good way of passing information.
Introduce ALT_INSNS and ALT_CALLS to pass in at the top level to select the
operation(s) desired. They represent what will happen when we've separated
the altcalls out of the general alternative instructions infrastructure,
although in the short term we still need to synthesise the force parameter for
_apply_alternatives().
Move two externs to reduce their scope a little.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The alternatives APIs are not great; rename alternative_branches() to be more
precise. Centralise the declaration in xen/alternative-call.h, in the
expectation that x86 won't be the only user in the long term.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
... in preparation for changing how they're implemented.
Update the MISRA deviations with the new path.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
alternative.h doesn't need lib.h now that macros.h exists. Futhermore, STR()
is already the prevailing style, so convert the final __stringify() to drop
stringify.h too.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Unlike the WRMSR side, we can't use asm goto() unconditionally, because our
toolchain baseline doesn't support asm goto with outputs.
However, the code generation improvements are substantial enough to warrant
the duplicate implementations.
Detect support, and use asm goto() when available.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
With the common code moved fully onto xen/byteorder.h, clean up the dregs.
It turns out that msi.h has not needed byteorder.h since the use of
__{BIG,LITTLE}_ENDIAN_BITFIELD was dropped in commit d58f3941ce ("x86/MSI:
use standard C types in structures/unions").
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
With the common code moved fully onto xen/byteorder.h, clean up the dregs.
The use of byteorder.h in io.h appears to have been copy&paste from ARM. It's
not needed, but macros and types are.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
With the common code moved fully onto xen/byteorder.h, clean up the dregs.
Sort includes in some files while swapping over to xen/byteorder.h.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
cpu_to_le64() is about to become a macro, at which point the #ifdef in the
middle of it becomes undefined behaviour.
Use a local variable to prepare strtab, where the #ifdef is fine to use.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
This seems to have been quite an oversight in Xen's NUMA support, albeit it
probably because NUMA was arch-specific at first.
This is unlikely to be useful for CONFIG_SEPARATE_XENHEAP builds, but that's
only ARM32 right now, and unlikely to be interested in NUMA.
Take the opportunity to sort the includes.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This also moves executables too.
I'm not sure why xilinx-smoke-dom0-x86_64.sh was overriding PATH too, as
/usr/local is clearly in PATH given the other tests, but drop that too.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
All the x86 hardware runners are out of date, to varying degrees, in terms of
microcode.
Microcode must be uncompressed and prepended to the initrd in order for Xen to
be able to use it during early boot.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
The argo artefact is now a cpio archive and can be appended to dom?-rootfs
directly, rather than being part of of the overlay. Drop the
copy_dom?_files() functions.
xen-argo.ko has moved into the standard location, so update the insmod path.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Right now, we have initrd.cpio.gz as domU, and initrd.tar.gz as the base for
dom0.
Rename initrd.cpio.gz to domU-rootfs.cpio.gz, and xen-rootfs.cpio.gz to
dom0-rootfs.cpio.gz to make it clearer which is which. Rename the VM from
test to domU.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
This should have been dropped too, as the authoritative copy lives in another
repo now.
Fixes: babe11b46c ("CI: Drop alpine-3.18-rootfs-export and use test-artefacts")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Linux 6.6.56 was already added to test-artifacts for the argo testing, and
this removes one moving part while cleaning things up.
Drop the associated export job, and dockerfile.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
AMD have updated the SB-7033 advisory to include Zen5 CPUs. Extend the digest
check to cover Zen5 too.
In practice, cover everything until further notice.
Observant readers may be wondering where the update to the digest list is. At
the time of writing, no Zen5 patches are available via a verifiable channel.
Link: https://www.amd.com/en/resources/product-security/bulletin/amd-sb-7033.html
Fixes: 630e8875ab ("x86/ucode: Perform extra SHA2 checks on AMD Fam17h/19h microcode")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Bloat-o-meter reports:
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-29 (-29)
Function old new delta
_probe_mask_msr 99 94 -5
init_amd 2418 2394 -24
but this under-reports because .fixup doesn't contain sized/typed symbols.
This also drops two "mov -EFAULT, %reg; jmp ...;" sequences too, so the net
saving is -50.
wrmsr_amd_safe()'s return value is only checked against 0 (if at all), and
because of this, the compiler can now avoid manifesting the 0/-EFAULT
constants entirely, and the %[fault] label simply lands on the right basic
block.
Convert to Xen style while rewriting.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
MISRA C:2012 Amendment 3 (which is under consideration, but not used by Xen
yet) is more particular about having the storage class specifier first.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
The fixup section is only used by x86, and we're working to remove it there
too. Logic in the fixup section is unconnected to it's origin site, and
interferes with backtraces/etc.
Remove the section from the architectures which don't use it.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
With the new toolchain baseline, we can make use of asm goto() in certain
places, and the VMXON invocation is one example.
This removes the logic to set up rc (including a fixup section where bactraces
have no connection to the invoking function), the logic to decode it,
including the default case which was dead but not visibly-so to the compiler.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This logic was inserted by commit 447f613c54 ("lzo: update LZO compression
to current upstream version") but was only relevant for the TMEM logic, so
should have been deleted in commit c492e19fdd ("xen: remove tmem from
hypervisor")
Fixes: c492e19fdd ("xen: remove tmem from hypervisor")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
In a 64bit-clean environment, blowfish fails:
make[6]: Leaving directory
'/builddir/build/BUILD/xen-4.19.1/tools/tests/x86_emulator'
In file included from /usr/include/features.h:535,
from /usr/include/bits/libc-header-start.h:33,
from /usr/include/stdint.h:26,
from
/usr/lib/gcc/x86_64-xenserver-linux/12/include/stdint.h:9,
from blowfish.c:18:
/usr/include/gnu/stubs.h:7:11: fatal error: gnu/stubs-32.h: No such
file or directory
7 | # include <gnu/stubs-32.h>
| ^~~~~~~~~~~~~~~~
compilation terminated.
make[6]: *** [testcase.mk:15: blowfish.bin] Error 1
because of lack of glibc-i386-devel or equivelent. It's non-fatal, but
reduces the content in test_x86_emulator, which we do care about running.
Instead, convert all emulator testcases to being freestanding builds, resuing
the tools/firmware/include/ headers.
This in turn requires making firmware's stdint.h compatible with 64bit builds.
We now have compiler types for every standard type we use.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
We have checks in both xen/compiler.h, and Config.mk. Both are incomplete.
The check in Config.mk sees $(CC) in system and cross-compiler form, so cannot
express anything more than the global baseline. Change it to simply 5.1.
In xen/compiler.h, rewrite the expression for clarity/brevity.
Include a GCC 12.2 check for RISCV, and include a Clang 11 baseline check.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
These are all loops over a scalar value, and don't need to call general bitop
helpers behind the scenes.
Clamp data to the width of the access in dispatch_mmio_write(), rather than
doing so in every handler.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The bitmap_for_each() expression only inspects the bottom 8 bits of targets.
Change it's type to uint8_t and use for_each_set_bit() which is more efficient
over scalars.
GICD_SGI_TARGET_LIST_MASK is 2 bits wide. Two cases discard the prior
calculation of targets, and one case exits early.
Therefore, move the GICD_SGI_TARGET_MASK calculation into the only case which
wants it, and use MASK_EXTR() to simplify the expression.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
The switch() statement is over bits 24:25 (unshifted) of the guest provided
value. This makes case 0x3: dead, and not an implementation of the 4th
possible state.
A guest which writes (0x3 << 24) | (0xff << 16) to this register will skip the
early exit, then enter bitmap_for_each() with targets not bound by nr_vcpus.
If the guest has fewer than 8 vCPUs, bitmap_for_each() will read off the end
of d->vcpu[] and use the resulting vcpu pointer to ultimately derive irq, and
perform out-of-bounds writes.
Fix this by changing case 0x3 to default.
Fixes: 08c688ca64 ("ARM: new VGIC: Add SGIR register handler")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Manually break the false dependency for the benefit of cases such as
bitmap_weight() which is a reasonable hotpath.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
I was mistaken about when ASM_CALL_CONSTRAINT is applicable. It is not
applicable for plain pushes/pops, so remove it from the flags logic.
Clarify the description of ASM_CALL_CONSTRAINT to be explicit about unwinding
using framepointers.
Fixes: 0754534b8a ("x86/elf: Improve code generation in elf_core_save_regs()")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Petr reports:
(XEN) MMIO emulation failed (1): d12v1 64bit @ 0010:fffff8057ba7dfbf -> 45 0f 20 c2 ...
during introspection.
This is MOV %cr8, which is wired up for hvm_mov_{to,from}_cr(); the VMExit
fastpaths, but not for the full emulation slowpaths.
Xen's handling of %cr8 turns out to be quite wrong. At a minimum, we need
storage for %cr8 separate to APIC_TPR, and to alter intercepts based on
whether the vLAPIC is enabled or not. But that's more work than there is time
for in the short term, so make a stopgap fix.
Extend hvmemul_{read,write}_cr() with %cr8 cases. Unlike hvm_mov_to_cr(),
hardware hasn't filtered out invalid values (#GP checks are ahead of
intercepts), so introduce X86_CR8_VALID_MASK.
Reported-by: Petr Beneš <w1benny@gmail.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In hvmemul_read_cr(), make the TRACE()/X86EMUL_OKAY path common in preparation
for adding a %cr8 case. Use a local 'val' variable instead of always
operating on a deferenced pointer.
In both, calculate curr once.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Drop CentOS 7 entirely. It's way to old now.
Ubuntu 22.04 is the oldest Ubuntu with a suitable version of Clang, so swap
the 16.04 clang builds for 22.04.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Something I overlooked when last cleaning up exception handling is that a TSS
is not necessary if IST isn't configured, and IST isn't necessary until we're
running guest code.
Introduce early_traps_init(), and rearrange the existing logic between this
and traps_init() later on boot, to allow defering TSS and IST setup.
In early_traps_init(), load the IDT and invalidate TR/LDTR; this sufficient
system-table setup to make exception handling work. The setup of the BSPs
per-cpu variables stay early too; they're used on certain error paths.
Move load_system_tables() later into traps_init(). Note that it already
contains enable_each_ist(), so this call is simply dropped.
This removes some complexity prior to having exception support, and lays the
groundwork to not even allocate a TSS when using FRED.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
... and rename to traps_init() for consistency. Move the declaration from
asm/system.h into asm/traps.h.
This also involves moving init_ler() and variables. Move the declaration of
ler_msr from asm/msr.h to asm/traps.h.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Move the declaration from asm/system.h into asm/traps.h.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
cpu_init() doesn't particularly belong in trap_init(). This brings the BSP
more in line with the APs.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The expression for one parameter of find_memory() is already complicated and
about to become moreso. Break it out into a new variable, and express it in
an easier-to-follow way.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
This needs to be ahead of everything. Right now, it is after xen/init.h being
included for -DINIT_SECTIONS_ONLY
# 1 "./include/xen/compiler.h" 1
# 83 "./include/xen/compiler.h"
# 1 "./include/xen/init.h" 1
# 62 "./include/xen/init.h"
typedef int (*initcall_t)(void);
typedef void (*exitcall_t)(void);
# 72 "./include/xen/init.h"
void do_presmp_initcalls(void);
void do_initcalls(void);
# 84 "./include/xen/compiler.h" 2
# 122 "./include/xen/compiler.h"
#pragma GCC visibility push(hidden)
Fixes: 84c4461b7d ("Force out-of-line instances of inline functions into .init.text in init-only code")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
GCC 4.1.2 is from 2007, and Binutils 2.16 is a similar vintage. Clang 3.5 is
from 2014. Supporting toolchains this old is a massive development and
testing burden.
Set a minimum baseline of GCC 5.1 across the board, along with Binutils 2.25
which is the same age. These were chosen *3 years ago* as Linux's minimum
requirements because even back then, they were ubiquitous in distros. Choose
Clang/LLVM 11 as a baseline for similar reasons; the Linux commit making this
change two years ago cites a laudry list of code generation bugs.
This will allow us to retire a lot of compatiblity logic, and start using new
features previously unavailable because of no viable compatibility option.
Merge the ARM 32bit and 64bit sections now they're the same.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
The current CI failures turn out to be a latent bug triggered by a narrow set
of properties of the initrd and the host memory map, which CI encountered by
chance.
One step during boot involves constructing directmap mappings for modules.
With some probing at the point of creation, it is observed that there's a 4k
mapping missing towards the end of the initrd.
(XEN) === Mapped Mod1 [0000000394001000, 00000003be1ff6dc] to Directmap
(XEN) Probing paddr 394001000, va ffff830394001000
(XEN) Probing paddr 3be1ff6db, va ffff8303be1ff6db
(XEN) Probing paddr 3bdffffff, va ffff8303bdffffff
(XEN) Probing paddr 3be001000, va ffff8303be001000
(XEN) Probing paddr 3be000000, va ffff8303be000000
(XEN) Early fatal page fault at e008:ffff82d04032014c (cr2=ffff8303be000000, ec=0000)
The conditions for this bug appear to be map_pages_to_xen() call with a start
address of exactly 4k beyond a 2M boundary, some number of full 2M pages, then
a tail needing 4k pages.
Anyway, the condition for spotting superpage boundaries in map_pages_to_xen()
is wrong. The IS_ALIGNED() macro expects a power of two for the alignment
argument, and subtracts 1 itself.
Fixing this causes the failing case to now boot.
Fixes: 97fb6fcf26 ("x86/mm: introduce helpers to detect super page alignment")
Debugged-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
... which is more consise than the opencoded form, and more efficient when
compiled.
Furthermore, now that find_{first,next}_bit() are no longer in use, the
seg_reg_{accessed,dirty} fields aren't forced to be unsigned long, although
they do need to remain unsigned int because of __set_bit() elsewhere.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Regular data access into the trampoline is via the directmap.
As now discussed quite extensively in asm/trampoline.h, the trampoline is
arranged so that only the AP and S3 paths need an identity mapping, and that
they fit within a single page.
Right now, PFN_UP(trampoline_end - trampoline_start) is 2, causing more than
expected of the trampoline to be mapped. Cut it down just the single page it
ought to be.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
We have since gained a better location for it to live.
Fix up the includes while doing so. I don't recall why we had kernel.h but
it's definitely stale now.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
A CALL with 0 displacement is handled specially, and is why this logic
functions even with CET Shadow Stacks active. Nevertheless a RIP-relative LEA
is the more normal way of doing this in 64bit code.
The retrieval of flags modifies the stack pointer so needs to state a
dependency on the stack pointer. Despite it's name, ASM_CALL_CONSTRAINT is
the way to do this.
read_sreg() forces the answer through a register, causing code generation of
the form:
mov %gs, %eax
mov %eax, %eax
mov %rax, 0x140(%rsi)
Encode the reads directly with a memory operand. This results in a 16bit
store instead of an 64bit store, but the backing memory is zeroed.
While cleaning this up, drop one piece of trailing whitespace.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Use %pd, and state what the unknown is. As it's an enum, it's a signed type.
Also drop one piece of trailing whitespace.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
When PV is enabled, entry_int80 needs to be DPL3, not DPL0.
This, combined with a QEMU bug which incorrectly calculates the error
code (fix submitted separately), causes the XSA-259 PoC to fail with:
--- Xen Test Framework ---
Environment: PV 64bit (Long mode 4 levels)
XSA-259 PoC
Error: Unexpected fault 0x800d0802, #GP[IDT[256]]
Test result: ERROR
Fixes: 3da2149cf4 ("x86/IDT: Generate bsp_idt[] at build time")
Reported-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The use of "legitimately" mixes the concepts of "it was designed to do this"
and "it was correct to do this".
The latter in particular can go stale. "intended" is a better way of phrasing
this.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
When a watchdog fires, the domain is crashed and can't dump any state.
Xen allows a domain to have two separate watchdogs. Therefore, for a
domain running multiple watchdogs (e.g. one based around network, one
for disk), it is important for diagnostics to know which watchdog
fired.
As the printk() is in a timer callback, this is a bit awkward to
arrange, but there are 12 spare bits in the bottom of the domain
pointer owing to its alignment.
Reuse these bits to encode the watchdog id too, so the one which fired
is identified when the domain is crashed.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
As soon as the the domain is in the domlist, it can be queried via various
means, ahead of being fully constructed. Ensure it has the toolstack-given
UUID prior to becoming visible.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
We've already scanned features by the time init_e820() is called. Remove the
cpuid() calls.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The exact behaviour of LVTERR interrupt generation is implementation
specific.
* Newer Intel CPUs generate an interrupt when pending_esr becomes
nonzero.
* Older Intel and all AMD CPUs generate an interrupt when any
individual bit in pending_esr becomes nonzero.
Neither vendor documents their behaviour very well. Xen implements
the per-bit behaviour and has done since support was added.
Importantly, the per-bit behaviour can be expressed using the atomic
operations available in the x86 architecture, whereas the
former (interrupt only on pending_esr becoming nonzero) cannot.
With vlapic->hw.pending_esr held outside of the main LAPIC register page,
it's much easier to use atomic operations.
Use xchg() in vlapic_reg_write(), and *set_bit() in vlapic_error().
The only interesting change is that vlapic_error() now needs to take a
single bit only, rather than a mask, but this fine for all current
callers and forseable changes.
No change from a guests perspective.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Xen currently presents APIC_ESR to guests as a simple read/write register.
This is incorrect. The SDM states:
The ESR is a write/read register. Before attempt to read from the ESR,
software should first write to it. (The value written does not affect the
values read subsequently; only zero may be written in x2APIC mode.) This
write clears any previously logged errors and updates the ESR with any
errors detected since the last write to the ESR.
Introduce a new pending_esr field in hvm_hw_lapic.
Update vlapic_error() to accumulate errors here, and extend vlapic_reg_write()
to discard the written value and transfer pending_esr into APIC_ESR. Reads
are still as before.
Importantly, this means that guests no longer destroys the ESR value it's
looking for in the LVTERR handler when following the SDM instructions.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
... to be more concise, and to match our other entrypoints into Xen.
In acpi_sleep_prepare(), calculate bootsym_phys() once, which GCC seems
unwilling to of it's own accord.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
There are two issues. First, pi_test_and_clear_on() pulls the cache-line to
the CPU and dirties it even if there's nothing outstanding, and second,
bitmap_for_each() is O(256) when O(8) would do, and would avoid multiple
atomic updates to the same IRR word.
Rewrite it from scratch, explaining what's going on at each step.
Bloat-o-meter reports 177 -> 145 (net -32), but real improvement is the
removal of calls to __find_{first,next}_bit() hidden behind bitmap_for_each().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
struct domain is always a page aligned allocation. Update it's type to
reflect this, so we can safely reuse the lower bits in the pointer for
auxiliary information.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
With most of pv_trap_init() being done at build time, opening of NMI_SOFTIRQ
can be a regular initcall, simplifying trap_init().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Now that bsp_idt[] is constructed at build time, we do not need to manually
initialise it in init_idt_traps() and trap_init().
When swapping the early pagefault handler for the normal one, switch to using
_update_gate_addr_lower() as we do on the kexec path for NMI and #MC.
This in turn allows us to drop set_{intr,swint}_gate() and the underlying
infrastructure. It also lets us drop autogen_entrypoints[] and that
underlying infrastructure.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper