The data segment registers are part of the on-stack IRET frame when
interrupting Virtual 8086 mode, but this ceased being relevant for Xen in
commit 5d1181a5ea ("xen: Remove x86_32 build target.") in 2012.
With all other cleanup in place, delete the fields so we can introduce FRED
support which uses this space for different data.
Everywhere which used the es field as an offset in cpu_user_regs needs
adjusting. However, they'll change again for FRED, so no cleanup is performed
at this juncture.
This also undoes the OoB Read workaround in show_registers(), which can now
switch back to being simple structure copy.
No functional change, but a lot of rearranging of stack and struct layout
under the hood.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In order to support FRED, we're going to have to remove the {ds..gs} fields
from struct cpu_user_regs, meaning that it is going to have to become a
different type to the structure embedded in vcpu_guest_context_u.
struct cpu_user_regs is a name used in common Xen code (i.e. needs to stay
using this name), so renaming the public struct to be guest_user_regs in Xen's
view only.
Introduce a brand hew cpu-user-regs.h, currently containing a duplicate
structure.
Notably, this removes the need to include pubic/xen.h in ~every translation
unit in Xen (via current.h), and highlights one case where the emulator was
picking up cpu_user_regs transitively.
Include comments describing how hardware interacts with this structure under
IDT delivery, as it's quite magic to start with. FRED is going make things
more complicated.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
In order to support FRED, we're going to have to remove the {ds..gs} fields
from struct cpu_user_regs. This will impact v->arch.user_regs.
These fields are unused for HVM guests, but for PV hold the selector values
when the vCPU is scheduled out.
Introduce new fields for the selectors in struct pv_vcpu, and update:
* {save,load}_segments(), context switching
* arch_{set,set}_info_guest(), hypercalls
* vcpu_show_registers(), diagnostics
* dom0_construct(), PV dom0
to use the new storage. This removes the final user of read_sregs() so drop
it too.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In order to support FRED, we're going to have to remove the {ds..gs} fields
from struct cpu_user_regs, meaning that it is going to have to become a
different type to the structure embedded in vcpu_guest_context_u.
In both arch_{get,set}_info_guest(), expand the memcpy()/XLAT_cpu_user_regs()
to copy the fields individually. This will allow us to eventually make them
different types.
This does cause some minor changes in behaviour for the hypercalls.
It is specifically not the case that a toolstack could set_info(); get_info();
and get an identical bit pattern back. Amongst other things, the
architectural sticky bits in registers are applied during setting.
Previously, XLAT_cpu_user_regs() omitted the _pad fields in the compat case
whereas the non-compat case included them owing to the single memcpy().
Omit the _pad fields in the non-compat case too; for all but the oldest of
CPUs, the segment selectors are zero-extended by hardware when pushed onto the
stack, so non-zero values here get lost naturally. Furthermore, FRED reuses
the space above cs and ss for extra state, and a PV guest for now at least
must not be able to write the control state.
Omit the error_code and entry_vector fields too. They're already identified
as private fields in the public API, and are stale outside of Xen's
interrupt/exception/syscall handler. They're also a very minor information
leak of which event caused the last deschedule of a vCPU.
Finally, omit saved_upcall_mask. Xen doesn't consume this, and only produces
it in {compat_,}create_bounce_frame(), based on the vcpu_info shared state and
settings about the event being injected. Similar to error_code/entry_vector,
it is stale outside of the guest's event handler.
No change that toolstacks or guests are expected to notice or care about.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The API is unergonomic to use, and leads to poor code generation because of
unnecessary forcing of data to the stack.
Rename build_id_p to xen_build_id, and build_id_len to xen_build_id_len, make
them __ro_after_init, and export the variables directly.
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>
Both callsites call them as a pair, and more generally, anyone wanting the
changeset wants the buildid too.
This involves rearranging console_init_preirq() to ensure xen_build_init() is
ahead of print_version().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Having multiple values wrapped onto as few lines as practical is good for
space efficiency, but causes complex collisions for hypercall backports and
local policy changes. Reformat to use one value per line.
No functional change, only whitespace changes.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Most indentation is with tabs, but a few spaces have slipped in. Switch them
back to tabs.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Hypercalls are not free; cache colouring is an experimental feature, and
ignoring an -EOPNOTSUPP is bad form.
Now that Xen has been fixed to initialise colouring information correctly for
domains, xc_domain_set_llc_colors() only needs calling for domains with
explicit configuration.
Rearrange the logic to avoid the hypercall in the general case, leaving a
comment explaining why it is performed so early.
Fixes: 748bd725fb ("tools: add support for cache coloring configuration")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Right now, domains start with no colouring information and depend on a
XEN_DOMCTL_set_llc_colors(nr=0) hypercall in order to get default colours.
This in turn forces the toolstack to make a hypercall for all VMs even those
without colouring configured, and to ignore errors if Xen doesn't have
colouring active.
Introduce domain_llc_coloring_init(), replacing domain_set_default_colors(),
and call it during the trivial initialisation in domain_create(). Leave two
comments explaining the positioning.
The -EEXISTs condition in domain_set_llc_colors() needs adjusting as a
consequence, but that's easy enough to keep working as before.
Fixes: 6cdea3444e ("xen/arm: add Dom0 cache coloring support")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
domain_destroy() is the wrong position to be freeing colouring information,
which is only used for allocations.
The comment in context identifies how domain_destroy() can be called multiple
times on the same domain, leading to a double free of d->llc_colors as it's
the wrong side of the atomic_cmpxchg() to be made safe.
Furthermore, by freeing d->llc_colors but leaving d->num_llc_colors nonzero,
alloc_color_heap_page() can in principle use after free.
Make domain_llc_coloring_free() idempotent, zeroing d->num_llc_colors and
NULL-ing d->llc_colors after freeing, and call it from domain_teardown().
Fixes: 6985aa5e0c ("xen: extend domctl interface for cache coloring")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Using:
xfree(__va(__pa(d->llc_colors)));
is an extraordinarily expensive way of writing:
xfree((void *)d->llc_colours);
Combined with the comment indicating that this was intention, the patch should
have been rejected outright.
Correct the type of d->llc_colours by removing the bogus const on it, and
remove the cast when freeing it.
No functional change.
Fixes: 6985aa5e0c ("xen: extend domctl interface for cache coloring")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
This snapshot is Linux commit db4001f9cc32 ("x86/cpu/vfm: Delete all
the *_FAM6_ CPU #defines"), now that Xen has switched off the old constant
names.
Leave a comment identifying the exact revision Xen is using.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Switch to the new fields and constants.
In mwait_idle_probe(), exit early for non-Intel CPUs. intel_idle_ids[] is a
large (and ever increasing) table and it's not reasonable to scan it for other
vendors, nor is it ideal to be emitting a somewhat-ambiguous error message.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
With the ability to match on steppings, introduce a new X86_MATCH_VFMS()
helper to match a specific stepping, and use it to rework deadline_match[].
Currently, the table is scanned on all systems, even those without the
TSC_DEADLINE feature. Introduce another early exit for that case, which also
removes the need to encode every row in the table with TSC_DEADLINE.
Notably this removes the overloading of driver_data possibly being a function
pointer, and removes the latent bug where the target functions are missing
ENDBR instructions owing to the lack of the cf_check attribute.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Architecturally, stepping is a 4-bit field, so a uint16_t suffices for a
bitmap of steppings.
In order to keep the size of struct x86_cpu_id the same, shrink the vendor and
family fields; there's no need for them to be uint16_t in Xen, and this
matches the size of the fields in cpuinfo_x86.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Xen's use of struct x86_cpu_id is a bit different to Linux's, so we can
simplify the loop termination condition. Leave a comment explaining Xen's
assumptions.
Switch to Xen style, as we've properly deviated from Linux, and switch to the
new vendor/family/model names.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This replaces raw model numbers (and comments in some cases) with names. For
probe_mwait_errata(), merge the comments with the table to make it easier to
see which erratum is which, and drop a stray "Problem" in LNL030.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
mwait-idle's ICPU() is the most convenient place to get started. Introduce
X86_MATCH_CPU() and X86_MATCH_VFM() following their Linux counterparts.
This involves match-cpu.h including more headers, which in turn allows us to
drop a few.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Only 5 files use struct x86_cpu_id, so it should not be in processor.h. This
is in preparation to extend it with VFM support.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In intel.c, drop asm/mwait.h and asm/uaccess.h, neither of which are used.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
NEHALEM_EX is affected by the erratum too.
Change the comment to be the full erratum text, not an interpretation of it.
Fixes: 95807bcae4 ("C6 state with EOI issue fix for some Intel processors")
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In udelay(), use cpu_relax() directly which, for better or worse, is the
common way to refer to the PAUSE instruction.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This snapshot is prior to Linux commit db4001f9cc32 ("x86/cpu/vfm: Delete all
the *_FAM6_ CPU #defines") at the end of their conversion phase.
In addition to non-FAM6 infixed names, defines are added for the Pentium Pro,
ArrowLake U, and reintroduced the PHI defines which were incorrectly deleted
in the past.
In cpufeature.h, provide VFM_* macros to transform constants to/from the
cpuinfo_x86 representation.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Intel have run out of model space in Family 6 and will start using Family 19
starting with Diamond Rapids. Xen, like Linux, has model checking logic which
will malfunction owing to bad assumptions about the family field.
Reorder the family, vendor and model fields so they can be accessed together
as a single vfm field.
As we're cleaning up the logic, take the opportunity to introduce better
names, dropping the x86 prefix.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
TSA affects AMD Fam19h CPUs (Zen3 and 4 microarchitectures).
Three new CPUID bits have been defined. Two (TSA_SQ_NO and TSA_L1_NO)
indicate that the system is unaffected, and must be synthesised by Xen on
unaffected parts to date.
A third new bit indicates that VERW now has a flushing side effect. Xen
must synthesise this bit on affected systems based on microcode version.
As with other VERW-based flushing features, VERW_CLEAR needs OR-ing across
a resource pool, and guests which have seen it can safely migrate in.
This is part of XSA-471 / CVE-2024-36350 / CVE-2024-36357.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In order to mitigate TSA, Xen will need to issue VERW before going idle.
On AMD CPUs, the VERW scrubbing side effects cancel an active MONITOR, causing
the MWAIT to exit without entering an idle state. Therefore the VERW must be
ahead of MONITOR.
Split spec_ctrl_enter_idle() in two and allow the VERW aspect to be handled
separately. While adjusting, update a stale comment concerning MSBDS; more
issues have been mitigated using VERW since it was written.
By moving VERW earlier, it is ahead of the determination of whether to go
idle. We can't move the check on softirq_pending (for correctness reasons),
but we can duplicate it earlier as a best effort attempt to skip the
speculative overhead.
This is part of XSA-471 / CVE-2024-36350 / CVE-2024-36357.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
AMD are releasing microcode for TSA, so extend the known-provenance list with
their hashes. These were produced before the remediation of the microcode
signature issues (the entrysign vulnerability), so can be OS-loaded on
out-of-date firmware.
Include an off-by-default check for the sorted-ness of patch_digests[]. It's
not worth running generally under SELF_TESTS, but is useful when editing the
digest list.
This is part of XSA-471 / CVE-2024-36350 / CVE-2024-36357.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Turn the if()s into switch()es, as we're going to need AMD sections.
Move the RTM adjustments into the Intel section, where they ought to live.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The check of this_softirq_pending must be performed with irqs disabled, but
this property was broken by an attempt to optimise entry/exit latency.
Commit c227233ad64c in Linux (which we copied into Xen) was fixed up by
edc8fc01f608 in Linux, which we have so far missed.
Going to sleep without waking on interrupts is nonsensical outside of
play_dead(), so overload this to select between two possible MWAITs, the
second using the STI shadow to cover MWAIT for exactly the same reason as we
do in safe_halt().
Fixes: b17e0ec72e ("x86/mwait-idle: enable interrupts before C1 on Xeons")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In order elide IPIs, we must be able to identify whether a target CPU is in
MWAIT at the point it is woken up. i.e. the store to wake it up must also
identify the state.
Create a new in_mwait variable beside __softirq_pending, so we can use a
CMPXCHG to set the softirq while also observing the status safely. Implement
an x86 version of arch_pend_softirq() which does this.
In mwait_idle_with_hints(), advertise in_mwait, with an explanation of
precisely what it means. X86_BUG_MONITOR can be accounted for simply by not
advertising in_mwait.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
x86 is the only architecture wanting an optimisation here, but the
test_and_set_bit() is a store into the monitored line (i.e. will wake up the
target) and, prior to the removal of the broken IPI-elision algorithm, was
racy, causing unnecessary IPIs to be sent.
To do this in a race-free way, the store to the monited line needs to also
sample the status of the target in one atomic action. Implement a new arch
helper with different semantics; to make the softirq pending and decide about
IPIs together. For now, implement the default helper. It will be overridden
by x86 in a subsequent change.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
With the recent simplifications, it becomes obvious that smp_mb() isn't the
right barrier. Strictly speaking, MONITOR is ordered as a load, but smp_rmb()
isn't correct either, as this only pertains to local ordering. All we need is
a compiler barrier().
Merge the barier() into the monitor() itself, along with an explantion.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
cpuidle_wakeup_mwait() is a TOCTOU race. The cpumask_and() sampling
cpuidle_mwait_flags can take a arbitrary period of time, and there's no
guarantee that the target CPUs are still in MWAIT when writing into
mwait_wakeup(cpu).
The consequence of the race is that we'll fail to IPI certain targets. Also,
there's no guarantee that mwait_idle_with_hints() will raise a TIMER_SOFTIRQ
on it's way out.
The fundamental bug is that the "in_mwait" variable needs to be in the
monitored line, and not in a separate cpuidle_mwait_flags variable, in order
to do this in a race-free way.
Arranging to fix this while keeping the old implementation is prohibitive, so
strip the current one out in order to implement the new one cleanly. In the
interim, this causes IPIs to be used unconditionally which is safe albeit
suboptimal.
Fixes: 3d521e933e ("cpuidle: mwait on softirq_pending & remove wakeup ipis")
Fixes: 1adb34ea84 ("CPUIDLE: re-implement mwait wakeup process")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
When support was originally introduced, ebx, ecx and edx were reserved and
should have been zeroed in recalculate_misc() to avoid leaking into guests.
Since then, fields have been added into ebx. Guests can't load microcode, so
shouldn't see ucode_size, and while in principle we do want to support larger
RAP sizes in guests, virtualising this for guests depends on AMD procuding any
official documentation for ERAPS, which is long overdue and with no ETA.
This patch will cause a difference in guests on Zen5 CPUs, but as the main
ERAPS feature is hidden, guests should be ignoring the rap_size field too.
Fixes: e9b4fe2636 ("x86/cpuid: support LFENCE always serialising CPUID bit")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
For exception recovery in the stubs, the registered address for fixup is the
return address of the CALL entering the stub.
In invoke_stub(), the '.Lret%=:' label is the wrong side of the 'post'
parameter. The 'post' parameter is non-empty in cases where the arithmetic
flags of the operation need recovering.
Split the line to separate 'pre' and 'post', making it more obvious that the
return address label was in the wrong position.
However, in the case that an exception did occur, we want to skip 'post' as
it's logically part of the operation which had already failed. Therefore, add
a new skip label and use that for the exception recovery path.
This is XSA-470 / CVE-2025-27465
Fixes: 79903e50db ("x86emul: catch exceptions occurring in stubs")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
For features which are unconditionally set in the max policies, making the
default policy to match the host can be done with a conditional clear.
This is simpler than the unconditional clear, conditional set currently
performed.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
I was presented with this:
(XEN) NX (Execute Disable) protection active
(XEN) d0 has maximum 416 PIRQs
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Error creating d0: -95
(XEN) ****************************************
which is less than helpful. It turns out to be the -EOPNOTSUPP from
shadow_domain_init().
The real bug here is create_dom0() unconditionally assuming the presence of
SHADOW_PAGING. Rework it to panic() rather than choosing a dom0_cfg which is
guaranteed to fail. This results in:
(XEN) NX (Execute Disable) protection active
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Neither HAP nor Shadow available for PVH domain
(XEN) ****************************************
which is rather more helpful.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Most of the patch (handling of CPUIDLE_FLAG_IBRS) is fine, but the
adjustements to mwait_idle() are not; spec_ctrl_enter_idle() does more than
just alter MSR_SPEC_CTRL.IBRS.
The only reason this doesn't need an XSA is because the unconditional
spec_ctrl_{enter,exit}_idle() in mwait_idle_with_hints() were left unaltered,
and thus the MWAIT remained properly protected.
There (would have been) two problems. In the ibrs_disable (== deep C) case:
* On entry, VERW and RSB-stuffing are architecturally skipped.
* On exit, there's a branch crossing the WRMSR which reinstates the
speculative safety for indirect branches.
All this change did was double up the expensive operations in the deep C case,
and fail to optimise the intended case.
I have an idea of how to plumb this more nicely, but it requires larger
changes to legacy IBRS handling to not make spec_ctrl_enter_idle() vulnerable
in other ways. In the short term, simply take out the perf hit.
Fixes: 08acdf9a26 ("x86/mwait-idle: disable IBRS during long idle")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Commit 48d32458bc ("x86, idle: add barriers to CLFLUSH workaround") was
inherited from Linux and added MFENCEs around the AAI65 errata fix.
The SDM now states:
Executions of the CLFLUSH instruction are ordered with respect to each
other and with respect to writes, locked read-modify-write instructions,
and fence instructions[1].
with footnote 1 reading:
Earlier versions of this manual specified that executions of the CLFLUSH
instruction were ordered only by the MFENCE instruction. All processors
implementing the CLFLUSH instruction also order it relative to the other
operations enumerated above.
I.e. the MFENCEs came about because of an incorrect statement in the SDM.
The Spec Update (no longer available on Intel's website) simply says "issue a
CLFLUSH", with no mention of MFENCEs.
As this erratum is specific to Intel, it's fine to remove the the MFENCEs; AMD
CPUs of a similar vintage do sport otherwise-unordered CLFLUSHs.
Move the feature bit into the BUG range (rather than FEATURE), and move the
workaround into monitor() itself.
The erratum check itself must use setup_force_cpu_cap(). It needs activating
if any CPU needs it, not if all of them need it.
Fixes: 48d32458bc ("x86, idle: add barriers to CLFLUSH workaround")
Fixes: 96d1b237ae ("x86/Intel: work around Xeon 7400 series erratum AAI65")
Link: https://web.archive.org/web/20090219054841/http://download.intel.com/design/xeon/specupdt/32033601.pdf
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
They're not used by any other translation unit, so shouldn't live in
asm/processor.h, which is included almost everywhere.
Our new toolchain baseline knows the MONITOR/MWAIT instructions, so use them
directly rather than using raw hex.
Change the hint/extention parameters from long to int. They're specified to
remain 32bit operands even 64-bit mode.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This is literally ASID 1 (of 2^16), not a boolean.
Fixes: 2f09f797ba ("x86/svm: Drop the suffix _guest from vmcb bit")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Read The Docs now requires a configuration file, which is awkward when using
RTD to render proposed changes on the list.
Provide the minimal configuration file possible, sacrificing all
reproducibility in order to hopefully not need to touch it moving forwards.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The early exit from domain_cmdline_size() is buggy. Even if there's no
bootloader cmdline and no kextra, there still might be Xen parameters to
forward, and therefore a nonzero cmdline length.
Explain what the function is doing, and rewrite it to be both more legible and
more extendible.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
It's non-init caller was dropped in Xen 4.14.
No functional change.
Fixes: 02e9a9cf20 ("xen: remove XEN_SYSCTL_set_parameter support")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Package domU for dom0 and insert into the uncompressed part of dom0's rootfs,
rather than recompressing it as part of the overlay.
For Qubes, this avoids putting the domU kernel in dom0's rootfs for tests
which aren't going to boot a guest.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
For Qubes, this requires switching from sh to bash.
This reduces the number of times the target filename needs to be written to 1.
Expand the comment to explain the concatenation constraints.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
It's only hvm.c, irq.c and vpci.c which need this header, and they all get it
via asm/hvm/irq.h.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
... and use them from their installed location.
The full reclusive copy of tools/tests brings in all build and intermediate
artefacts. e.g. for test-tsx alone:
./tests/tsx
./tests/tsx/.test-tsx.o.d
./tests/tsx/test-tsx.o
./tests/tsx/.gitignore
./tests/tsx/test-tsx
./tests/tsx/Makefile
./tests/tsx/test-tsx.c
duplicating the test binary which is also in ./usr/lib/xen/tests
Rewrite run-tools-tests to run tests from their installed
location (/usr/lib/xen/tests in alpine), which effectively removes the outer
loop over $dir.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Unlike the other tests, this is not standalone. It requires poking at a live
system, making it unweildly to use.
It also hasn't been touched in 7 years, despite changes in libraries and
kernel devices using the deprivilege infrastructure.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
x86_merge_dr6() is not a no-op when 0 is passed in; it will discard the
previously latched breakpoint bits.
The combination of do_debug()'s manual call to x86_merge_dr6() for external
debuggers, and pv_inject_DB() calling pv_inject_event(), results in two
x86_merge_dr6() calls.
Feed the same pending_dbg in the second time. This makes pv_inject_event()'s
update of dr6 effectively a no-op, retaining the correct breakpoint bits.
Fixes: db39fa4b27 ("x86/pv: Fix merging of new status bits into %dr6")
Reported-by: Manuel Bouyer <bouyer@antioche.eu.org>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Manuel Bouyer <bouyer@antioche.eu.org>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Paging Writeable depends on EPT so must be disabled in non-EPT guests like the
other EPT dependent features. Otherwise, VMEntry fails with bad control
state.
Drop a piece of trailing whitepsace in context.
Fixes: ff10aa9d8f ("x86: Add Support for Paging-Write Feature")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
This avoids needing to hold rc in a register across the WRMSR, and in most
cases removes direct testing and branching based on rc, as the fault label can
be rearranged to directly land on the out-of-line block.
No functional change.
Resolves: https://gitlab.com/xen-project/xen/-/work_items/214
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Switch to using a mnemonic, and asm_inline as there's EXTABLE metadata.
Update the types, and adjust parameter names. The name xfeatures is specific
to xcr0, so rename to val to be more generic, even if there isn't another
writeable xcr yet.
No functional change.
Resolves: https://gitlab.com/xen-project/xen/-/work_items/212
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This really is a legacy of not having parameters to start with. Give PV dom0
with a PVH domU a real name.
Reformat the table to fix alignment.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Very few files need the stubs. Move the infrastructure out of
processor.h and config.h into a new stubs.h, and adjust the includes
accordingly.
Make the per-cpu struct stubs be read mostly; they're unmodified
during the uptime of the CPU, and move them into smpboot.c seeing as
that's where they're allocated and freed.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Rules 5.3, 11.2 and 16.6 are already listed in clean_guidelines_common and
apply to all architectures. There's no need for arm64 to give them a second
time.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
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>
Andrew Cooper