This was copy&paste from x86. It causes the padding between functions to be:
a000026dd64: 90909090 adrp x16, 9ff2147d000 <start-0xded83000>
rather than:
a000026dd64: 00000000 udf #0
No functional change.
Fixes: 5da0d3123c ("arm: entry.S and head.S")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
This has been a plain C read for as far back as I can trace in history.
Research into invented-loads has flagged it as a possible vulnerability.
After analysis, it is believed to be a bug only, not a security vulnerability.
The analysis suggests that we can read one value out of the guest, operate on
another, and that this could be an in-guest privliege escalation. Any entity
in the guest able to modify the pagetables already has full privilege, so
while Xen can potentially malfunction, the effects don't cross a privilege
boundary.
The analysis also suggests that this is worse for shadow guests because we may
put the TOCTOU entry in the shadows, but this is inaccurate. What we put in
the shadows is still translated under the P2M and refers to guest physical
address space.
Either way, the code fits the pattern for invented loads, and it is a risk;
harden the accesses.
Link: https://github.com/xoreaxeaxeax/schrodingers-toctou/blob/main/observer-effect/audits/audit-xen-ptwalk-RELEASE-4.21.1.md#86-per-candidate-finding
Fixes: 49f7c7364e ("Replace shadow pagetable code with shadow2.")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Treat "argo" on the command line as a positive boolean, rather than requiring
the user to pass "argo=1/on/enable/true".
Move both opt_argo* variables into __ro_after_init. They're set during
command line parsing and never modified thereafter.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
vgic_v{2,3}_setup_hw()'s callers are __init, so they should be too.
vgic_v{2,3}_hw and gic_v2_hw_data are written once during init and
unmodified thereafter, so make them __ro_after_init. Reposition
'bool enabled' in these structures to fit in the tail padding, removing
8 bytes from their size when paddr_t is 8B.
While at it, drop dead vgic_v3_setup_hw() dummy implementation
from vgic/vgic.c. GICV3 depends on !NEW_VGIC.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Julien Grall <julien@xen.org>
In both setup_{k7,p6}_watchdog(), EvtSel0 is first zeroed, then written with
everything but the enable bit, then written with the enable bit.
setup_p4_watchdog() is slightly more complicated, owing to what
appears to be a bug introduced by commit 2a2bd8de16 ("Clean up NMI
watchdog handler."), which causes a second bit to be temporarily
different too.
The middle of the three writes is useless in all cases. Drop it.
While doing this, rename the 'counter' parameter for
setup_p6_watchdog(). It is the event which is passed in; the counter
is always counter 0.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Right now it's only checked in setup_p4_watchdog(), and not in
setup_p6_watchdog().
Perform the check in the common Intel path in setup_apic_nmi_watchdog(), and
pass misc_enable as a parameter into setup_p4_watchdog() to avoid reading it
twice.
Fixes: 0dfba864fb ("NMI watchdog support in Xen.")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
* Drop trailing whitespace
* Sort includes, dropping asm/mc146818rtc.h and asm/div64.h as unused
* Brace position, and types
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Model 3 was the first 64bit-capable P4. Xen won't be booting on anything
older, so remove the condition.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
FRED has formalised the event_data field. It already has more uses than
given (e.g. the NMI Source Bitmap), and further uses are expected in the
future.
Collapse the union into a single field called 'data' as the struct has event
in it's name, as well as 'event' being the common name for the variable.
Refer to the FRED spec rather than keeping an out-of-date list of uses.
Adjust all users of the old names.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
These refer to the segments themsevles, not to the registers, even if there is
a tight coupling between the two.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
x86_segment enumerates segments, not segment registers. Adjust the comment to
make this clearer.
Fix a stray tab with the closing #endif.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
This name is misleading.
It's not possible to configure NMI or not from the event select register; that
comes from the APIC configuration for performance events.
This name is "the thing we want to count for the NMI watchdog", but that's
clearer to follow when it simply names the event. Drop the indirection.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
With Oprofile support dropped, there are no more users of these. Drop them.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
It used to be the case that cpu_data[] inherited the BSP's cpuid_level until
the AP had calculated it itself. Following the rework, cpuid_level has a
placeholder 1 until it is calculated properly.
setup_apic_nmi_watchdog() happens to be called on the BSP after SMP bring-up,
meaning that the first call is on CPU1. It is also positioned in the window
where cpu_data[] is wrong.
As a result, setup_p6_watchdog()'s one-time calculation of the performance
counter width falls back into Pentium compatibility mode assuming 32bit
counters. This causes a watchdog NMI which is delayed a little (e.g. from an
SMI), to appear as if it hadn't overflowed, and therefore be considered as not
a watchdog NMI. On systems where unknown NMIs are treated as fatal, this
results in a spurious crash.
Switch setup_p6_watchdog() to use boot_cpu_data.cpuid_level, which is how this
is checked almost everywhere else.
core2_vpmu_init() used the same pattern to look at leaf 0xa. Despite being
init code and only running on the BSP, {boot,current}_cpu_data are different
objects, so switch it over to checking boot_cpu_data.cpuid_level too.
Fixes: 7126b7f806 ("x86/CPU: re-work populating of cpu_data[]")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
... and use it for ideal_nops and toolchain_nops_are_ideal; both of which are
invariant after arch_init_ideal_nops() has run.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The {k8,p6}_nops[] arrays are both 80-byte structures indexing 45-byte
structures. Furthermore, perhaps unusually for C, the source layout is an
obvious hint about the triangular nature of the structure.
Therefore, we can replace the pointer chase with some simple arithmetic.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
xen/livepatch.h is a fairly heavyweight header pulling in public/sysctl.h, and
a reasonable number of users care only for the init_or_livepatch_* tags.
They're arguably more init than livepatch anyway, and by moving them to
init.h, we can remove a number of includes.
The include in vsprintf was leftover from early versions of the work. In the
version committed, d5ccf4482e ("x86, xsplice: Print payload's symbol name
and payload name in backtraces"), symbol_lookup() had been adjusted to handle
the livepatch symbol names properly.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
xen/livepatch.h includes public/sysctl.h twice, which can be deduplicated.
xen/livepatch.h also includes asm/livepatch.h meaning that each livepatch.c
does not need to include both.
Comment the #else and #endif cases to aid legibility.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
elfstructs.h needs the stdint.h types. Two headers arrange this manually, but
elf.h and livepatch.h do not, which breaks source files whose headers are
properly sorted.
elfstructs.h is used by tools too, so use stdint directly outside of Xen.
Clean up trailing whitespace.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
The Store_field(result, 16, arch_config) sits inside an #ifdef x86, meaning
that on other architectures the pointer is not filled in. Ocaml code then
falls over a NULL(ish) pointer (really the Val_unit used to initialise
'result') when the layout in the heap doesn't match the type system.
Rearrange alloc_domaininfo() to avoid this. Similarly to
physinfo_arch_caps(), raise an exception if the architecture code hasn't
filled in an appropriate tag. Move the setup of arch_domainconfig to be
common logic.
In order to simplify the addition of other architectures, remove the
arch_config variable (reusing tmp as it's touched exactly once), and rename
x86_arch_config to be arch_config so each architecture can fill in a suitable
one without needing more local variables.
Fixes: 9d683b5e37 ("tools/ocaml: Expose arch_config in domaininfo")
Fixes: 81838c9067 ("ocaml: fix arm build")
Reported-by: Julian Vetter <julian.vetter@vates.tech>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
The current order is derived from expanding XLAT_cpu_user_regs() which used
the x86_32 cpu_user_regs order. This is correct for one the blocks, but not
the others.
In order to work with the prefetcher, rather than against it, loads want to be
in field order. Make this so for the three paths reading an x86_64
cpu_user_regs.
No functional change.
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Frediano Ziglio <frediano.ziglio@citrix.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
As explained in the comment partially in context, and contrary to what was
said in commit 8af337dfb8 ("x86/entry: Use POP_GPRS and remove
RESTORE_ALL"), the restore_all_xen() path cannot use default offsets for
SPEC_CTRL_COND_VERW.
Getting this wrong is surprisingly benign. VERW doesn't fault for any limit
or descriptor reasons.
However, in SVM vCPU context LDTR is the guest's not Xen's. When the segment
selector VERW uses happens to be an LDT selector, the CPU accesses the guest
LDTR in Xen context:
(XEN) ----[ Xen-4.23.0 x86_64 debug=y Not tainted ]----
(XEN) CPU: 14
(XEN) RIP: e008:[<ffff82d0402007f2>] x86_64/entry.S#restore_all_xen+0x72/0x80
(XEN) RFLAGS: 0000000000010002 CONTEXT: hypervisor (d3v1)
...
(XEN) Xen code around <ffff82d0402007f2> (x86_64/entry.S#restore_all_xen+0x72/0x80):
(XEN) 5f f6 44 24 7c 08 74 05 <0f> 00 6c 24 44 48 83 c4 08 48 cf 0f 1f 00 fc 0f
...
(XEN) Xen call trace:
(XEN) [<ffff82d0402007f2>] R x86_64/entry.S#restore_all_xen+0x72/0x80
(XEN) [<ffff82d0402e79f6>] F nestedhap_fix_p2m+0x5f/0xc9
(XEN) [<ffff82d0402e7c8f>] F nestedhvm_hap_nested_page_fault+0x11e/0x22e
(XEN) [<ffff82d0402cd59d>] F hvm_hap_nested_page_fault+0x1b8/0x5d2
(XEN) [<ffff82d0402ad7de>] F svm_vmexit_handler+0xbe9/0x18b3
(XEN) [<ffff82d040202542>] F svm_asm_do_resume+0x162/0x172
(XEN)
(XEN) Pagetable walk from 000000000000fff8:
(XEN) L4[0x000] = 0000000000000000 ffffffffffffffff
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 14:
(XEN) FATAL PAGE FAULT
(XEN) [error_code=0000]
(XEN) Faulting linear address: 000000000000fff8
(XEN) ****************************************
In this case, the guest's LDT obviously has a base of 0 and limit of 0xffff
for the segmentation checks to pass and a memory access to be attempted.
Fixes: 8af337dfb8 ("x86/entry: Use POP_GPRS and remove RESTORE_ALL")
Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
pv_map_ldt_shadow_page() takes the whole guest PTE, adds _PAGE_RW to it, then
installs it into Xen's pagetables. It has had this behaviour ever since LDT
support was added in 2003.
However, it allows the guest to control the software available bits and
cacheability. This happens to be benign right now, but is bad form.
Use only the guest frame, and construct the mapping as regular RW frame, and
notably includes NX. This is how the GDT logic already works.
Fixes: 005c272397 ("Finished virtualisation of x86 LDT")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Just because vcpu_id is within d->max_vcpus doesn't mean d->vcpu[vcpu_id] is
non-NULL. Use the proper accessor to look up a vcpu object.
Reported-by: Michal Orzel <michal.orzel@amd.com>
Fixes: acf65e5a25 ("xen/arm: vgic-v3: Emulate correctly the re-distributor")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Julien Grall <julien@xen.org>
The APM description of the AltMovCR8 feature bit is:
"LOCK MOV CR0 means MOV CR8"
Adjust the decode logic to behave like this.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
For reasons which escape me, rec_pfns are allocated separately to the rest of
the batch allocations.
Allocate them all together. This will allow for future simplifications to be
performed in an incremental mannor.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
PUSH_AND_CLEAR_GPRS is shorter than SAVE_ALL in terms of emitted code.
lstar_enter() and cstar_enter() do not need CLD. The SYSCALL instruction
sanitises flags based on MSR_SYSCALL_MASK. For all other cases, place the CLD
instruction next to STAC/CLAC so the flag handling is together.
Get rid of the compat=1 special case for PV32. It's not obviously a win, and
PV32 is getting increasingly rare these days.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
POP_GPRS is shorter than RESTORE_ALL in terms of emitted code.
By separating the popping of entry_vector/error_code off the stack,
SPEC_CTRL_COND_VERW doesn't need custom displacements.
Get rid of the compat=1 special case for PV32. It's not obviously a win, and
PV32 is getting increasingly rare these days.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
PUSH_AND_CLEAR_GPRS is shorter than SAVE_ALL in terms of emitted code.
SAVE_ALL hides a CLD instruction which is not needed anywhere here.
* VMX VMExit always clears flags. VMX VMentry failure never switched away
from the host flags, so they're still good.
* SVM VMRUN always saves and restores the host flags.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This is slightly stale mentioning SWAPGS and not LKGS. However, take the
opportunity to make the comment more general and less likely to bitrot.
It would be a serious vulnerability for operating systems generally if CPL3
could modify GS_SHADOW at all.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Given the need to calculate the base anyway, it's easier to do so entirely on
the stack.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This was reported to the security team, demonstrating that code execution
could be achieved, but without a demonstration of a priviliege boundary being
bypassed.
Still, it's a high risk path with no obvious evidence of having been used even
for debugging, so remove the environment variable and use the build time
$(libdir)/xenfsimage path only.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
These existed to work around a bug in older GCC when using struct-casting for
variable initialisation. However, our baseline toolchain is new enough to not
suffer this bug.
Removing these resolves two MISRA Rule 9.2 violations which exist in release
builds of Xen only, where "= { ... }" is disallowed for simple scalar
initialisation.
The BUILD_BUG_ON() in xenmem_add_to_physmap() cannot stay as it is, because
INVALID_GFN is not an Integer Constant Expression. Replace it BUILD_ERROR()
which is the nearest available alternative.
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>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Simply define shadow_audit_enable to be false for the !SHADOW_AUDIT case.
Change the variable to be a bool, and __read_mostly as it's only enabled by
debugkey.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Adjust kexec and kimage to more closely adhere to Xen style.
Sort the includes, dropping duplicates (kexec.h) and unused (ctype.h and
kernel.h).
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
The v1 interface was declared obsolete in Xen 4.4 (2013) when kexec in Xen was
overhauled.
The only known user of the v1 interface was the classic-xen fork of Linux.
Linux PVOps does not interact with Xen kexec directly, delegating it entirely
to userspace (i.e. kexec-tools). Xen support in kexec-tools was part of this
work, and uses the "new" interface.
As such, there's no sensible way to test changes to the old interface any
more.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Returning from an NMI which hits guest context needs special casing in FRED
mode just like it does in IDT mode.
Break nmi_exit_to_guest() out of handle_ist_exception(), and use it in
entry_FRED_R3() also.
Expand the comment a little, and invert the conditional jump to
compat_restore_all_guest() to avoid needing an #else clause for CONFIG_PV32.
Fixes: 87cfcbe9f0 ("x86/pv: Guest exception handling in FRED mode")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Relase-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Prior to commit 9c20d3c591 ("x86/IDT: Make idt_tables[] be per_cpu(idt)"),
the global idt_tables[] was always safe to use for CPUs in any state.
However, not-yet-onlined CPUs (e.g. MADT with more entries than exist in
practice) or offlined CPUs (e.g. xen-hptool) have their per-cpu pointer
poisoned to detect incorrect uses. machine_kexec() trips over the posion when
clobbering #MC entry paths. Update the comment to discuss parked CPUs, and to
remove discussion which isn't particularly relevant to the actions at hand.
This fixes a fatal #GP (non-canonical memory reference) when trying to enter
the crash kernel.
Fixes: 9c20d3c591 ("x86/IDT: Make idt_tables[] be per_cpu(idt)")
Reported-by: Lin Liu <Lin.Liu01@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
With FRED just as with IDT, it's unsafe to run the exception handlers after
switching stack.
To remove this unsafe window, %cr4 needs clearing earlier. In turn, we may
need to switch to PCID 0 earlier too in order to be able to clear CR4.PCIDE.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
After switching stack, it is not safe to run any exception handlers, because
attempts to access the cpu_info block are out-of-bounds and will generate wild
accesses.
Invalidating the IDT in the common path means there's no need to do so again
in the 32bit path, so drop compat_mode_idt entirely.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
When FRED is active, it is not possible to hook NMIs like this.
NMI hooking in the crash path has undergone several revisions since its
introduction. Notably since commit e7f147bf4a ("x86/crash: Drop manual
hooking of exception_table[]") we use the regular nmi_callback()
infrastructure.
Instead of asserting that we don't enter do_nmi_crash() on the crashing CPU,
tolerate it and return early. It's a marginally longer codepath but behaves
the same and is compatible with FRED.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
The configuration uses a mix of dashes and underscores, which is irritating to
develop for. Switch to using the underscore form consistently; it is the more
common form and it has the benefit that it allows splitting on dashes to work
sensibly.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Perform standard syntax cleanup and make it a non-root container. Switch yajl
for json-c given the deprecation of the former. Drop dev86 which is an
x86-only dependency, and QEMU dependencies as we don't build QEMU in this
environment any more.
When updating the job names, also rename some for consistency so the arm64
fragment comes before the compiler.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Perform standard syntax cleanup and make it a non-root container. Switch yajl
for json-c given the deprecation of the former.
Add an x86_64 suffix for naming consistency with everything else.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
This container is tied to gitlab-runner environment in the RPis driving the
test systems, not a specific version of Alpine. Intentionally give it a
generic name so it need not change in the future.
Switch to Alpine 3.24 right away, as it doesn't interact with the 3.18 builds
under test.
The container needs to remain a root container. By no longer using the
arm64v8 build container for dual-purpose, we can finally make the build
containers be non-root.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
GCC 15.2 with Alpine Linux 3.24 fails with -Werror=nonnull, complaining that
we're calling bzero(NULL, 128).
This is a legitimate diagnostic. xenalyze has it's own error() function
shadowing the standard library one, and can in principle return when p is
NULL.
Extend the check in error() with ERR_MAX_TOLERABLE to short circuit the
variable tolerance check.
Suggested-by: Anthony PERARD <anthony.perard@vates.tech>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
In order to avoid serialising the testing on both boards, the runner
configuration is being adjusted.
Have the .xilinx-arm64 and .xilinx-x86_64 templates choose the board directly
using a more specific tag.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Victor Lira <victorm.lira@amd.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
The container is tied to the runner, not a version of Ubuntu. Intentionally
give it a generic name so it need not change in the future.
Apply standard cleanup to the dockerfile, except that it must continue to be a
root container to drive real hardware. Explicitly install ca-certificates to
compensate for --no-install-recommends.
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>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Rename it to have an x86_64 suffix, updating the build job names and scheduled
rebuild task.
Apply standard cleanups. Trim the package list down to what is actually
needed. Archlinux's base-devel contains most libraries, but some of the
listed packages have never been dependenices for Xen, and a lot are QEMU
dependenices which aren't useful owing to the lack of the ninja package.
This shrinks the container from 533MB to 380MB.
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>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
This reverts commit 3c4e804607.
This reverts commit 957f0f0b73.
This reverts commit 2bc9bc1978.
This reverts commit 03ebee6ed8.
This reverts commit a9720605b6.
These were committed without a maintainer ack, and therefore in contravention
of policy.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
The default version of python is 3.13, so drop the 3.11 overrides.
Swap yajl for json-c, given the deprecation of the former. Add lz4.
bin86/dev86 are no longer available.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Swap yajl for json-c, given the deprecation of the former. Add lz4.
Add 26.04 GCC/Clang debug/non-debug jobs. Drop the 24.04 debug jobs to keep
the overall job count down.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Ocaml 4.08 and earlier had the compiler package depend on graphics, and
therefore on X. Ocaml 4.09 and later dropped this dependency.
Debian and Ubuntu versions with Ocaml 4.08 and earlier (which are Debian
Bullseye/11, and Ubuntu Focal/20.04 and earlier) had ocaml-nox packages with
this dependency stripped, which we use to keep the size of the containers
down.
In newer versions of Debian and Ubuntu, ocaml-nox is just a transitional
package referring back to ocaml. Ubuntu Resolute/26.04 has finally removed
this transitional package. For all versions where ocaml-nox is just a
transitional package, swap to the ocaml package.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Right now lz4, unlike every other compression scheme, unconditionally uses
Xen's unsafe decompressor. Make it consistent with all other compression
schemes by using liblz4.
The unsafe decompression is still required for the MiniOS build, so rename
xg_dom_decompress_lz4.c to xg_dom_decompress_unsafe_lz4.c and drop the
non-MiniOS content.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
As with other compression libraries, group liblz4 into ZLIB_{CFLAGS,LIBS}.
Add the packages to the Debian Trixie build containers for coverage.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
It is erroneous to check current like this; most commonly, introspection is
dom0 issuing hypercalls against domU.
The use of vm_event_is_enabled() is only for the IS_ENABLED(CONFIG_VM_EVENT)
short circut, so just use that directly.
Reported-by: Hady Azzam <hadyazzam@protonmail.com>
Fixes: b18e38e42d ("xen/mem_access: wrap memory access when VM_EVENT=n")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Tested-by: Hady Azzam <hadyazzam@protonmail.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Christian has just left Citrix, and Dave almost a decade ago.
Andrii (a XAPI committer) has worked on oxenstored before and has agreed to
step up as a maintainer, and Guillaume wishes to get involved and learn too.
In practice I do a lot of the bindings work, so lets make things official.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Andrii Sultanov <andriy.sultanov@vates.tech>
Acked-by: Guillaume Thouvenin <guillaume.thouvenin@vates.tech>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
FRED is now believed to be complete for AMD systems, and has had its tyres
kicked by both XenServer and AMD. Enable FRED by default on capable AMD
systems (Zen6 and later).
Support on Intel is still not yet complete. Leave it as tech preview and not
security supported.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
This removes qemu-system-aarch64-6.0.0-*-export and fixes the outstanding
TODOs about efi-virtio.rom, now that the QEMU in use has it's /usr/share/
properly packaged.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Exactly as per the Bookworm container, but additionally with the ipxe-qemu and
qemu-system-aarch64 packages. These will be used to remove the export jobs.
Switch qemu-arm{32,64} test jobs to use this container, and add two new build
tests.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
The binaries/ directory is a composition from the artefacts, and also used as
a working directory for most of the tests. If the very first artefact is from
a root container, then the test must also be a root container to use it as a
working directory.
For arm64, the existing linux-arm64 artefact suffices. For arm32, pull in the
microcode-x86 artefact as it's the smallest available. This bodge can be
removed when all build containers have become non-root.
For the qemu-xtf-dom0less-arm64-*-xen-version jobs, use *arm64-test-needs
ahead of alpine-3.18-gcc-* (as it's a root container), and to deduplicate
the *-export dependency.
This will allow us to change containers to being non-root one at a time,
rather than all in one go.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Commit 23e2479ff9 ("xen/domain: introduce DOMID_ANY") broke at least:
* xenstored domains
* The Python bindings
* The Ocaml bindings (i.e XAPI, and therefore XenServer/XCP-ng)
by causing Xen to reject 0 (the wildcard sentinel since the start of Xen) and
DOMID_INVALID (since 4.21) without adjusting the callers of xc_domain_create().
Under other circumstances I'd revert, but xenstored has gained a dependent use
of DOMID_ANY.
Fix up the remaining in-tree callers of xc_domain_create(), which covers
init-xenstore-domain and the python bindings.
The Ocaml C bindings, like libxc itself, expose this ABI detail to their
caller, but in this case the caller is still internal to the bindings as a
whole. Adjust the default value in the internal transformation, adding
domid_any for legibility.
Leave a very clear message in CHANGELOG.md that all external callers need
altering.
Fixes: 23e2479ff9 ("xen/domain: introduce DOMID_ANY")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Pau Ruiz Safont <pau.safont@vates.tech>
In FRED mode, the SYSCALL instruction does not modify %rcx/%r11. All current
software using SYSCALL expects the pre-FRED behaviour and spills %rcx/%r11
around the invocation, which is why FRED not doing this goes largely
unnoticed.
However, consider the following migration scenario:
* VM suspends. Hypercall, so SYSCALL, %rcx/%r11 left unmodified
* VM moves to a non-FRED system
* Xen resumes the VM with a real SYSRET instruction
Instead of resuming at the instruction following the SYSCALL instruction, the
VM is resumed at whatever dead value was in %rcx.
In FRED mode, manually adjust %rcx/%r11 when SYSCALL is used and when SYSRET
would have been used.
Regarding the choice of instructions in eretu_exit_to_guest(), a branch would
be a context dependent 50/50 split (i.e. increased chance of mispredict), and
only saves one instruction. The CMOVs read the same cacheline that ERETU is
about to process, so are as close to free as we can reasonably get.
Fixes: 76193ef47d ("x86/pv: System call handling in FRED mode")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Eclair reports that _MSC_VER is undefined, a MISRA Rule 20.9 violation.
This could be fixed by inserting an "&& defined(_MSC_VER)" clause, but the
whole block is entirely useless (it comments out the include of stdint) and
users pull in xen/types.h transitively.
Delete the whole block, and include xen/types.h to make it standalone.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
libxl_nocpuid.c needs the same treatment as libxl_cpuid.c
Fixes: f6c6f2679d ("libxl: Convert libxl__object_to_json() to json-c")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
XENMAPSPACE_gmfn_range was found to be buggy with overlapping ranges, and the
fix is sufficiently far from clear that yours truly decided a test was in
order.
XENMAPSPACE_gmfn_range is exposed by xendevicemodel_relocate_memory() so take
the opportunity to create a better example for others to copy.
The test works as follows. Populate 2M of RAM in the guest, marking the pages
so they can be identified later. Then construct four relocation test
cases (overlapping vs non-overlapping, forwards vs backwards) using the marks
in the guest pages to confirm that the P2M was adjusted correctly.
The use of XEN_DOMCTL_CDF_iommu forces xenmem_add_to_physmap() to limit the
iteration count to a maximum of 16. Use this to force a continuation and
exercise more of the logic.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
... ahead of extending it with a second test.
Rename run_tests() to test_paging_mempool_size() and have it return int so the
new run_tests() can decide whether to run subsequent tests or not.
Comment the parameters to xc_domain_populate_physmap_exact() to make it
clearer what's going on, and remove the page added to the physmap when the
test is done.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Under SVM, there are two copies of guest CR2. One is v->arch.hvm.guest_cr[2]
and one is in the VMCB.
Xen doesn't intercept CR2 accesses, so this mostly goes unnoticed; hardware
loads and saves the guest CR2 in the VMCB across VMRUN/VMExit.
For HAP guests (where #PF is not intercepted, and therefore we don't typically
inject #PF either), this causes the guest CR2 value to be lost on migrate. As
migration is cooperative and not done from the #PF handler, this also goes
unnoticed by guests.
It also means that an emulated MOV-from-CR2 reads a stale value.
Reported-by: Stefano Stabellini <sstabellini@kernel.org>
Fixes: d1bd157fbc ("Big merge the HVM full-virtualisation abstractions.")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Gitlab CI reported a crash on boot on Alder Lake hardware. The bug is years
old, making it an incredibly rare occurance:
(XEN) *** DOUBLE FAULT ***
(XEN) ----[ Xen-4.22-unstable x86_64 debug=y ubsan=y Not tainted ]----
(XEN) CPU: 0
(XEN) RIP: e008:[<ffff82d04077bbc4>] arch/x86/setup.c#reinit_bsp_stack+0xfa/0x160
(XEN) RFLAGS: 0000000000010202 CONTEXT: hypervisor
(XEN) rax: 0000000000000007 rbx: ffff83049a4b0000 rcx: 00000000000006a2
(XEN) rdx: 0000000000000000 rsi: 0000000000000000 rdi: 0000000000000000
(XEN) rbp: ffff83049a4b7f00 rsp: ffff83049a4b7ef8 r8: ffff830497e47000
(XEN) r9: 00000000ffffffff r10: 00000000900c2121 r11: 000000009a392956
(XEN) r12: ffff830497e47000 r13: ffff830497e49f40 r14: 0000000000000000
(XEN) r15: ffff82d0407dad10 cr0: 0000000080050033 cr4: 0000000000f526e0
(XEN) cr3: 0000000043c16000 cr2: fffffffffffffffc
(XEN) fsb: 0000000000000000 gsb: 0000000000000000 gss: 0000000000000000
(XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: 0000 cs: e008
(XEN) Xen code around <ffff82d04077bbc4> (arch/x86/setup.c#reinit_bsp_stack+0xfa/0x160):
(XEN) 00 b9 a2 06 00 00 0f 30 <80> 3d 71 26 f1 ff 00 74 3e 48 8d 93 f8 5f 00 00
(XEN) Valid stack range: ffff83049a4b6000-ffff83049a4b8000, sp=ffff83049a4b7ef8, tss.rsp0=ffff83049a4b7fb0
(XEN) No stack overflow detected. Skipping stack trace.
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) DOUBLE FAULT -- system shutdown
(XEN) ****************************************
This is on the instruction boundary after enabling CET (writing MSR_S_CET) and
prior to establishing SSP. Despite identifying this as a critical window
where any fault was deadly (the CPU tries to push a shadow stack frame at 0,
hence the CR2 value wrapping around to the top of the address space), I
clearly forgot that this meant interrupts too, which are enabled.
Disable interrupts during the critical period.
Fixes: b60ab42db2 ("x86/shstk: Activate Supervisor Shadow Stacks")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
lr_mask doesn't have bits set beyond the hardware limit, with the upper bits
remaining zero. Therefore, for_each_set_bit() is a better option.
For ARM64, bloat-o-meter reports:
Function old new delta
vgic_sync_from_lrs 208 168 -40
but this doesn't highlight that it also removes a call to find_next_bit() from
each loop iteration.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Acked-by: Julien Grall <julien@xen.org>
Andrew Cooper