Avoid open-coding macros we have readily available. Being able to use MB()
there might have avoided the flaw corrected by 5416c455f6 ("x86/mkelf32:
Actually pad load segment to 2 MiB boundary").
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Allow them to be used by build tools which don't include config.h. (They
also don't really fit into config.h, as there's nothing "configuration"
related about them.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Leaving aside highly disaggregated environments, the control domain is
what will invoke XEN_SYSCTL_SCHEDOP_putinfo. Its vCPU-s therefore need to
be able to run unconditionally, not those of the domain with ID 0 (which
may not exist at all).
Fixes: 9f0c658bae ("arinc: add cpu-pool support to scheduler")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
The per-channel ENABLE bit is to solely be driven by hpet_enable_channel()
and hpet_msi_{,un}mask(). It doesn't need setting immediately. Except for
the (possible) channel put in legacy mode we don't do so during boot
either.
Instead reset ->arch.cpu_mask, to avoid msi_compose_msg() yielding an
all-zero message (when the passed in CPU mask has no online CPUs). Nothing
would later call msi_compose_msg() / hpet_msi_write(), and hence nothing
would later produce a well-formed message template in
hpet_events[].msi.msg.
Fixes: 15aa6c6748 ("amd iommu: use base platform MSI implementation")
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
With an expiry value on the order of 10 hours, and with a bus scale value
of 256k (as supplied by qemu), the (signed) multiplication will be UB. As
we've checked that the value is positive, we mean unsigned multiplication
anyway. Yet let's play safe against even larger expiry and bus scale
values, leveraging the compiler builtin that there is for this purpose.
While there also drop the stray cast from the actual TMICT write.
Fixes: 9062553a0d ("added time and accurate timer support")
Fixes: b95beb1858 ("x86: Clean up APIC local timer handling")
Reported-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Having this immediately below a PCI-dev vs HPET conditional is (mildly)
confusing. Move that if() into the body of the earlier one.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
With opcode D6h now firmly reserved as another #UD-raising one in 64-bit
mode, use that instead of the two-byte UD2 for bug frame marking.
While there also make a minor adjustment to the emulator.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
EVEX.W meaning is unusual for VBMAC{,X}OR16x16x16, but that needs taking
care of only in the test harness.
Like already proposed in "x86emul: support AVX10.1", use just
vcpu_must_have(), not host_and_vcpu_must_have().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
We should not prematurely increment the pointer, to avoid inadvertently
skipping a NUL terminator.
Fixes: 6d9199bd0f ("x86-64: enable hypervisor output on VESA frame buffer")
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Advancing past a character without knowing whether that's the NUL char is
at risk of reading beyond the buffer.
Fixes: 25de1f6920 ("Allow arch-specific defaults to be specified for ns16550")
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
strsep(), unless passed a 1st argument which itself points at a NULL
pointer, won't return NULL. Instead if the separator(s) looked for isn't
found, that pointed-to field would be updated to hold NULL. Hence the
important check in get_token() is for token to be non-NULL after the call
(which get_token()'s callers rely upon), while the checking of param_name
is left there just in case.
Fixes: 97fd49a7e0 ("ns16550: add support for UART parameters to be specifed with name-value pairs")
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
We can easily avoid the risk of wrapping UINT_MAX <-> 0 by applying a
check against the compile-time-constant maximum number of colors.
Additionally the overflow checks suffered from an off-by-1, as the parsed
ranges are inclusive (e.g. end == start being possible, requiring 1 array
slot, while availability of 0 slots was checked in that case).
Fixes: 6cdea3444e ("xen/arm: add Dom0 cache coloring support")
Reported-by: Mykola Kvach <xakep.amatop@gmail.com>
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
These are siblings, so better would live close together. Sadly in
1f9540cd8e ("EFI: move efi-boot.h inclusion point") I moved get_value()
without also moving pre_parse().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
The message emitted by pre_parse() pretty clearly states the original
intention. Yet what it said wasn't done, and would have been unfriendly to
the user. Hence accesses past the allocated buffer were possible. Insert a
terminating NUL immediately past the data read, to then drop the no longer
applicable message.
NB: The iscntrl() check of just the last byte is more strict than what
pre_parse() would accept without issuing its prior message, yet I'd like
to keep the new logic reasonably simple. Config files shouldn't be huge,
and we shouldn't be _that_ short of memory (or we'd fail elsewhere pretty
soon).
Fixes: bf6501a62e ("x86-64: EFI boot code")
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
This function is unreachable on x86 when XSM_FLASK=n, and it is another
good candidate to put in an archive.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Shifting signed quantities has restrictions. Since the wrapping macro of
read_int() type-casts the result anyway, switch function return type as
well as the local variable to the corresponding unsigned type.
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
When we cap negative values to 0 (see code comment as to why), going
through scale_delta() is pointless - it'll return 0 anyway. Therefore make
the call conditional (and then also the one to scale_reciprocal()), adding
a comment as to why there is this capping.
Modernize types used while there, and switch to usiong initializers for
the local variables.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Incrementing ->sched_index between bounds check and array access may
result in accessing one past the array when that is fully filled
(->num_schedule_entries == ARINC653_MAX_DOMAINS_PER_SCHEDULE).
Fixes: 22787f2e10 ("ARINC 653 scheduler")
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Stewart Hildebrand <stewart@stew.dk>
When that code still lived in a653sched_init(), it was redundant with the
earlier memset() / xzalloc(). Once moved, the full structure field needs
setting, as dom_handle_cmp() uses memcmp() and arinc653_sched_set() may
have clobbered entries. Rather than implying the handle to be all zero,
copy the handle out of the domain structure.
Fixes: 9f0c658bae ("arinc: add cpu-pool support to scheduler")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Stewart Hildebrand <stewart@stew.dk>
MCE init for APs was broken when CPU feature re-checking was added. At the
same time make sure we don't bypass setup_doitm() (and whatever else may
be added to the bottom of identify_cpu()).
Fixes: bb502a8ca5 ("x86: check feature flags after resume")
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
This NR_CPUS-dimensioned array is likely unused on most installations.
Therefore it is especially wasteful for it to consume more space than
really needed. Allocate it dynamically ahead of registering the hooks.
Further the array having all fields set to -1 is actually useless. Nothing
relies on it, and core_parking_remove() doesn't restore the sentinel for
vacated slots. Drop the initializers altogether, rather than replacing
them.
Also take the opportunity and update an adjacent variable's type, where
a fixed-width type was pretty clearly inappropriate to use.
Finally drop the redundant initializer from core_parking_init().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
When Dom0 informs us about MMCFG usability, this may change whether
extended capabilities are available (accessible) for devices. Zap what
might be on record, and re-initialize things.
No synchronization is added for the case where devices may already be in
use. That'll need sorting when (a) DomU support was added and (b) DomU-s
may run already while Dom0 / hwdom still boots (dom0less, Hyperlaunch).
vpci_cleanup_capabilities() also shouldn't have used
pci_find_ext_capability(), as already when the function was introduced
extended config space may not have been (properly) accessible anymore,
no matter whether it was during init. Extended capability cleanup hooks
need to cope with being called when the respective capability doesn't
exist (and hence the corresponding ->init() hook was never called).
Fixes: 70e6dace74 ("vpci: Use cleanup to free capability resource during deassign")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The label used upon the function failing is wrong. Instead of correcting
the label, move the invocation up a little, to also avoid it altogether
for the idle domain (where ->vmtrace_size would be zero, and hence the
function would bail right away anyway).
Fixes: 217dd79ee2 ("xen/domain: Add vmtrace_size domain creation parameter")
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Using plain (signed) int variables as array indexes can be unhelpful on at
least x86, where the compiler may see the need to insert sign-extension
insns (strictly speaking it should be able to avoid that when the loop
continuation condition says >= 0, but that's not generally the case even
with gcc15).
Observed effects with gcc15 (will of course vary with compiler version and
level of optimization):
- on x86, one less preserved register in use, yet due to sub-optimal
choice of register variables still a small code size increase (%r12
isn't a good choice when it's used for base-without-index addressing, as
it requires a SIB byte which other registers wouldn't require),
- on Arm64 code size decreases, albeit that's eaten up by padding which is
being inserted ahead of a few labels,
- on Arm32 code size increases for a reason I didn't fully understand (my
ability to read Arm assembly is still somewhat limited).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
We cannot assume extended config space to (still) be accessible when
cleaning up. Necessary values need caching instead. In fact, as the caller
also cannot look up extended capabilities, the cleanup function needs to
cope with being called when there's no ReBAR capability at all.
As kind of a side effect nbars being 0 (which init_rebar() doesn't
explicitly reject) no longer results in a bogus call to
vpci_remove_registers().
Fixes: ee459aeac0 ("vpci/rebar: Implement cleanup function for Rebar")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
... both for when the functions are invoked and where they live in source.
Don't invoke them directly in vpci_init_header(), but instead first thing
in vpci_init_capabilities().
Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Let's keep capability handling together. Start with moving
vpci_init_capabilities() and its helpers, plus splitting out of its
cleanup counterpart.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Before adding more private stuff to xen/vpci.h, split it up. In order to
be able to include the private header first in a CU, the per-arch struct
decls also need to move (to new asm/vpci.h files).
While adjusting the test harness'es Makefile, also switch the pre-existing
header symlink-ing rule to a pattern one.
Apart from in the test harness code, things only move; no functional
change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
As per the standard this is UB, i.e. we're building on an extension in
the compilers we use (for gcc, see "The C Preprocessor" section
"Directives Within Macro Arguments"). Misra C:2012 rule 20.6 disallows
this altogether, though.
Combine the fix with some tidying: Fold the two level-1 checks, adjust in
particular comment style, and move the unlikely() use to a single inner
part of the conditional.
No functional change intended.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This is to aid with MGMT_HYPERCALL work, leaving the functions potentially
unreferenced (which Misra dislikes). By moving them to separate archive
members, the linker simply will not pick them up when not needed.
As the CPUID and MSR ones are always used together, put the "from" and
"to" variants of each together in one file respectively.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
... to a dedicated subdir of x86's private lib/ sub-tree.
For the CPU policy harnesses and libxenguest use $(lib-y) as set by the
new Makefile.common, whereas for the emulator harnesses stick to building
just cpuid.o for the time being.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
wake_up_one() isn't used at all, so violates Misra rule 2.1 (unreachable
code). wake_up_all() is only used locally, yet rather than making it
static its sole user can invoke wake_up_nr() in the intended way directly.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
While 670d6b908f ('x86 shadow: Move the shadow linear mapping for
n-on-3-on-4 shadows so') bumped the amount by one too little for the
32-on-64 case (which luckily was dead code, and hence a bump wasn't
necessary in the first place), 0b841314da ('x86/shadow:
sh_{make,destroy}_monitor_table() are "even more" HVM-only'), dropping
the dead code, then didn't adjust the amount back. Yet even the original
amount was too high in certain cases. Switch to pre-allocating just as
much as is going to be needed.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
With an initial mode installed by shadow_vcpu_init(), there's no need
for sh_update_paging_modes() to deal with the "mode is still unset"
case. Leave an assertion, though.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
While benign at present, it is still a little fragile to operate on a
wrong "old_mode" value in sh_update_paging_modes(). This can happen when
no monitor table was present initially - we'd create one for the new
mode without updating old_mode. Correct this in two ways, each of which
would be sufficient on its own: Once by adding "else" to the second of
the involved if()s in the function, and then by setting the correct
initial mode for HVM domains in shadow_vcpu_init().
Further use the same predicate (paging_mode_external()) consistently
when dealing with shadow mode init/update/cleanup, rather than a mix of
is_hvm_vcpu() (init), is_hvm_domain() (update), and
paging_mode_external() (cleanup).
Finally drop a redundant is_hvm_domain() from inside the bigger if()
(which is being converted to paging_mode_external()) in
sh_update_paging_modes().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
These emulation related functions are involved in HVM handling only, and
in some cases they even invoke such checks after having already done
things which are valid for HVM domains only. OOS active also implies HVM.
In sh_remove_all_mappings() one of the two checks is redundant with an
earlier paging_mode_external() one (the other, however, needs to stay).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The function is used from (HVM-only) OOS code only - replace the
respective #ifdef inside the function to make this more obvious. (Note
that SHOPT_OUT_OF_SYNC won't be set when !HVM, so the #ifdef surrounding
the function is already sufficient.)
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew cooper <andrew.cooper3@citrix.com>
It's not clear why they were put in place - a__ is of the very type
already.
No change in generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There's no need to subtract "bit" in what is passed to __scanbit(), as the
other bits are zero anyway after the shift (in __find_next_bit()) or can
be made so (in __find_next_zero_bit()) by flipping negation and shift. (We
actually leverage the same facts in find_next{,_zero}_bit() as well.) This
way in __scanbit() the TZCNT alternative can be engaged.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
As per the standard this is UB, i.e. we're building on a defacto extension
in the compilers we use. Misra C:2012 rule 20.6 disallows this altogether,
though. Use helper always-inline functions instead.
In sh_audit_l1_table(), along with reducing the scope of "gfn", which now
isn't used anymore by the if() side of the conditional, also reduce the
scope of two other adjacent variables.
For audit_magic() note that both which parameters are needed and what
their types are is attributed to AUDIT_FAIL() accessing variables which
aren't passed as arguments to it.
No functional change intended. Of course codegen does change with this,
first and foremost in register allocation.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
The records reported used to contain Linux enumerators. We first broke
that connection when purging unused ones, and then again when switching to
the bit mask forms.
Fixes: 4084130511 ("x86/cpu: Drop unused X86_VENDOR_* values")
Fixes: 0cd074144c ("x86/cpu: Renumber X86_VENDOR_* to form a bitmap")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
shadow_mode_...(), with the exception of shadow_mode_enabled(), are
shorthands for shadow_mode_enabled() && paging_mode_...(). While
potentially useful outside of shadow-internal functions, when we already
know that we're dealing with a domain in shadow mode, the "paging"
checks are sufficient and cheaper. While the "shadow" ones commonly
translate to a MOV/AND/CMP/Jcc sequence, the "paging" ones typically
resolve to just TEST+Jcc.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
When in 3-level guest mode we help a guest to stay alive, we also
shouldn't emit a trace entry to the contrary. Move the invocation up
into the respective #ifdef, noting that while this moves it into the
locked region, emitting trace records with the paging lock held is okay
(as done elsewhere as well), just needlessly increasing lock holding
time a little.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There's no need for an indirect call here, as the mode is invariant
throughout the entire paging-locked region. All it takes to avoid it is
to have a forward declaration of sh_update_cr3() in place.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
validate_guest_pt_write(), by calling sh_validate_guest_entry(), already
guarantees the needed update of log-dirty information. Move the
operation into the sole code path needing it (when SHOPT_SKIP_VERIFY is
enabled), making clear that only one such call is needed.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Nothing hypercall-related needs setting up there. Nor do we need to
check whether the idle domain is shutting down - it never will.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Permitting writes when the P2M type says "read-only" can't be correct.
Fixes: 1661158723 ("xen/arm: Extend copy_to_guest to support copying from/to guest physical address")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Arrays of .byte or .short don't really need aligning better than 1 or 2
bytes respectively. Arrays of .long also don't need aligning to 8 bytes
(which would have happened when BITS_PER_LONG is 64 and SYMBOLS_ORIGIN is
not defined).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The barrier in vcpu_context_saved() is specifically about the clearing of
->is_running. It isn't needed when we don't clear the flag.
Furthermore, one side of the barrier being in common code, the other would
better be, too. This way, all architectures are covered (beyond any
specific needs they may have).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Julien Grall <julien@xen.org>
When, during boot, we have already correctly determined availability of
the MMCFG access method for a given bus range, there's then no need to
invoke pci_check_extcfg() again for every of the devices. This in
particular avoids ->ext_cfg to transiently indicate the wrong state.
Switch to using Xen style on lines being touched and immediately adjacent
ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
The counterpart in vcpu_context_saved() is smp_wmb(), and here we don't
really need any more than a read barrier: The concern expressed in the
comment is void, as updates to the context are necessarily observed ahead
of ->is_running going false, by virtue of said barrier in
vcpu_context_saved().
Amends: f679038961 ("xen/arm: sched: Ensure the vCPU context is seen before vcpu_pause() returns")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien@xen.org>
Everywhere else gfn_t are passed into respective GFN locking macros: Do so
here as well.
Amends: 819cdc5a73 ("x86/p2m: re-arrange {,__}put_gfn()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Its inclusion placed where it is, it affects the hypervisor build as well.
The hypervisor build, in its _install rule, uses $(DEBUG_DIR), first in
[ -d "$(D)$(DEBUG_DIR)" ] || $(INSTALL_DIR) $(D)$(DEBUG_DIR)
$(D) is an absolute directory (shorthand for $(DESTDIR)). $(DEBUG_DIR) as
set by Paths.mk is, too. Both point into the build tree. The two simply
shouldn't be glued together.
Note that the earlier
[ -d $(D)$(BOOT_DIR) ] || $(INSTALL_DIR) $(D)$(BOOT_DIR)
continues to be working fine, as BOOT_DIR continues to be controlled by
config/StdGNU.mk. Its DEBUG_DIR isn't taking effect anymore, when for the
hypervisor build it should.
And of course behavior now differs between building xen/ in a tree where
tools/ was built before vs in an otherwise clean tree.
Fixes: 82b9cc04a7 ("build: add make macro for making file from file.in")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
As was recently noticed with PCI_X_STATUS_533MHZ, leaving presently unused
#define-s in non-Misra-compliant shape (7.2 is "A `u' or `U' suffix shall
be applied to all integer constants that are represented in an unsigned
type") is putting new code actually using any of the constants at risk of
then causing Misra scan failures in CI. Adjust those few constants which
strictly require a suffix.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Rather than spelling out their amounts (requiring updates when multiples
of 32 are crossed), introduce a sentinel each and calculate the two
numbers from those.
No difference in generated code, albeit debug info size grows quite a bit.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
cpumask_var_t can resolve to a pointer or to an array. While the pointer
typically is allocated once for a CPU and then only read (i.e. wants to be
marked read-mostly), the same isn't necessarily true for the array case.
There things depend on how the variable is actually used. cpu_core_mask
and cpu_sibling_mask (which all architectures have inherited from x86,
which in turn is possibly wrong) are altered only as CPUs are brought up
or down, so may remain uniformly read-mostly. Other (x86-only) instances
want to change, to avoid disturbing adjacent read-mostly data.
While doing the x86 adjustment, also do one in the opposite direction,
i.e. where there was no read-mostly annotation when it is applicable in
the "pointer" case.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
The types are local to the shadow and HAP subsystems respectively, and
HAP has no need for the shadow-specific fields (i.e. it can get away with
smaller allocations). In struct hvm_domain it therefore suffices to have
a union of two (generally opaque) pointers.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
It's really FIRST_IRQ_VECTOR vector which is meant here, i.e. effectively
there is a form of open-coding in use right now.
No change in practice, due to the present aliasing.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
For DomU-s, whether to emulate accesses to the first 32 bits of extended
config space as read-as-zero or read-as-all-ones depends on whether a
device actually has extended config space. If it doesn't, read-as-zero
isn't correct; not getting this right may confuse functions like Linux
6.19-rc's pci_ext_cfg_is_aliased().
For Dom0 this then simply allows dropping a later conditional.
Fixes: a845b50c12 ("vpci/header: Emulate extended capability list for dom0")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Avoid interpreting as extended capabilities what may be about anything. In
doing so, vPCI then also won't mis-interpret data from beyond base config
space anymore.
Fixes: 3b35911d70 ("Enable pci mmcfg and ATS for x86_64")
Fixes: a845b50c12 ("vpci/header: Emulate extended capability list for dom0")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Legacy PCI devices don't have any extended config space. Reading any part
thereof may return all ones or other arbitrary data, e.g. in some cases
base config space contents repeatedly.
Logic follows Linux 6.19-rc's pci_cfg_space_size(), albeit leveraging our
determination of device type; in particular some comments are taken
verbatim from there. Like with Linux'es CONFIG_PCI_QUIRKS, only the alias
detection logic is covered by the new "pci=no-quirks". The singular access
at PCI_CFG_SPACE_SIZE is left unconditional.
For Misra's sake the PCI_X_STATUS_533HMZ definition also needs amending.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Don't know how I managed to overlook the freeing side when adding the case
to alloc_pdev().
Fixes: cd2b9f0b19 ("PCI: handle PCI->PCIe bridges as well in alloc_pdev()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Only explicit writes are subject to e.g. the checking of the MSR intercept
bitmap, which extends to VM-event's hvm_monitor_msr(). Indicate, by way of
a new boolean parameter, to the hook functions which variant it is.
Fixes: 6eb43fcf8a ("x86emul: support SWAPGS")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
While sizes (and possibly positions) of the symbol table related symbols
(and as a result other ones) are expected to change from linking pass 1
to pass 2, no such change should happen anymore from pass 2 to pass 3, or
else the internally recorded symbol table wouldn't represent the ELF or
PE/COFF ones.
For comparing to be actually useful, i.e. most notably also covering the
last of the arrays emitted, symbol sizes need establishing. Make use of
the xen/linkage.h machinery to achieve that.
Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
See the extensive code comment. This isn't really nice, but unless I'm
overlooking something there doesn't look to be a way to have the linker
strip individual symbols while doing its work.
Fixes: bf6501a62e ("x86-64: EFI boot code")
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
The hook is never invoked in that case, and hence needlessly offers an
extra valid indirect call target. With the hook suppressed, no consumer
of the three local per-CPU variables exists either, so they're
suppressed as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Guests can do wider-than-PTE-size writes on page tables. The tracing
helper variable, however, only offers space for a single PTE (and it is
being switched to the more correct type right here). Therefore bound
incoming write sizes to the amount of space available.
To not leave dead code (which is a Misra concern), drop the now unused
guest_pa_t as well.
Also move and adjust GUEST_PTE_SIZE: Derive it rather than using hard-
coded numbers, and put it in the sole source file where it's actually
needed. This then also addresses a Misra rule 20.9 ("All identifiers
used in the controlling expression of #if or #elif preprocessing
directives shall be #define'd before evaluation") violation:
GUEST_PAGING_LEVELS is #define'd only in multi.c.
This is XSA-477 / CVE-2025-58150.
Fixes: 9a86ac1aa3 ("xentrace 5/7: Additional tracing for the shadow code")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
No reason for it to be a separate per-CPU item; it's connected to a
struct cpufreq_policy instance just like other driver data.
This also eliminates the concern over amd_cppc_cpufreq_cpu_init() being
called for all CPUs, or a CPU going offline that's recorded in policy->cpu
(which would result in accesses of per-CPU data of offline CPUs).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Penny Zheng <penny.zheng@amd.com>
No reason for it to be a separate per-CPU item; it's connected to a
struct cpufreq_policy instance just like other driver data.
This further reduces the concern over amd_cppc_cpufreq_cpu_init() being
called for all CPUs, or a CPU going offline that's recorded in policy->cpu
(which would result in accesses of per-CPU data of offline CPUs).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Penny Zheng <penny.zheng@amd.com>
Share space with the ACPI, powernow, and HWP drivers, avoiding a separate
allocation for each CPU.
This then also reduces the concern over amd_cppc_cpufreq_cpu_init() being
called for all CPUs, or a CPU going offline that's recorded in policy->cpu
(which would result in accesses of per-CPU data of offline CPUs).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Penny Zheng <penny.zheng@amd.com>
Share space with the ACPI and powernow drivers, avoiding a separate
allocation for each CPU. Except for get_hwp_para() all use sites already
have the policy available, and this one function can simply be brought
closer to its sibling set_hwp_para().
This then also eliminates the concern over hwp_cpufreq_cpu_init() being
called for all CPUs, or a CPU going offline that's recorded in
policy->cpu (which would result in accesses of per-CPU data of offline
CPUs).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
It's not clear why only x86 had $(build_id_linker) applied to all three
linking passes. Not doing so will alter symbol offsets between the 2nd
and 3rd passes for, potentially, all of the symbols at higher addresses
(intermediate alignment padding may mask this effect, though, so it will
look as if problems appeared randomly).
Fixes: a353cab905 ("build_id: Provide ld-embedded build-ids")
Reported-by: Mykola Kvach <xakep.amatop@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Mykola Kvach <xakep.amatop@gmail.com> # arm
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> # riscv
Tested-by: Jason Andryuk <jason.andryuk@amd.com> # arm64
Acked-by: Michal Orzel <michal.orzel@amd.com>
Discussion of a RISC-V change revealed that for PPC and RISC-V we don't
really set any default, but rather rely on internals of kconfig picking
the lowest of the permitted values in such a case. Let's make this
explicit, requiring architectures that mean to permit SMP by default to
explicitly record some sensible value here.
Leverage the adjustment to the "1" case to simplify all subsequent ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Possibly many slots of it may be unused (all of them when the HWP or CPPC
drivers are in use), as it's always strictly associated with the CPU
recorded in a policy (irrespective of that CPU intermediately being taken
offline). It is shared by all CPUs sharing a policy. We could therefore
put the respective pointers in struct cpufreq_policy, but even that level
of indirection is pointless. Embed the driver data structure directly in
the policy one.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
At the example of _sinitext - that symbol has four aliases on x86:
__init_begin, __2M_init_start, __2M_rodata_end, and whatever the first
function in .init.text. With the GNU toolchain all of them are marked
'T' or 't'. With Clang/LLVM, however, some are marked 'r'. Since, to
save space, we want fake "end" symbols only for text, right now
want_symbol_end() has a respective check. That is getting in the way,
however, when the final of those symbols is 'r'. Remove the check and
instead zap the size for anything that is non-code.
Fixes: 6eede548df ('symbols: avoid emitting "end" symbols for data items')
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
qsort() implementations have freedom towards actions taken when two items
compare equal. The latest with the introduction of fake "end" symbols,
inconsistent sorting between the 1st and 2nd run can lead to extra "end"
symbols in one of the runs, making the resulting symbol table partly
unusable. (Note in particular that --warn-dup or --error-dup are passed
only on the 2nd run, and only for xen.syms, and that option has the effect
of doing a name sort ahead of doing the address sort. I.e. the inputs to
the 2nd qsort() are pretty different between the 1st and 2nd runs.)
Make the result stable by using original order to break ties.
Fixes: d3b637fba3 ("symbols: arrange to know where functions end")
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Gcc's "threading" of conditionals can lead to undue warnings, as reported
in e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116519 (no matter
that the overall situation is different there). While my gcc15 complains
("buf[2] may be used uninitialized in this function") about only two of
the three instances (not about the one in type_read()), adjust all three
to be on the safe side.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
There's no reason to set an arbitrary upper bound of 10 seconds. We can
simply set the comparator such that it'll take a whole cycle through all
32-bit values until the next interrupt would be raised. (For an extremely
fast-running HPET [400 MHz and up] 10 seconds would also be too long.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
No IRQ balancing is supposed to be happening on the broadcast IRQs. The
only entity responsible for fiddling with the CPU affinities is
set_channel_irq_affinity(). They shouldn't even be fiddled with when
offlining a CPU: A CPU going down can't at the same time be idle. Some
properties (->arch.cpu_mask in particular) may transiently reference an
offline CPU, but that'll be adjusted as soon as a channel goes into active
use again.
Along with adjusting fixup_irqs() (in a more general way, i.e. covering all
vectors which are marked in use globally), also adjust section placement of
used_vectors.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
When this was added, the log message was updated correctly, but the zero
case was needlessly checked separately: hpet_broadcast_enter() had a zero
check added at the same time, while handle_hpet_broadcast() can't possibly
pass 0 here anyway.
Also turn the printk() there into an assertion: NOW() should only ever
return valid (positive) values.
Fixes: 7145897cfb ("cpuidle: Fix for timer_deadline==0 case")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Neither caller passes STIME_MAX, so (bogusly) handling the case isn't
necessary.
"Bogusly" because with 32-bit counters, writing 0 means on average half
the wrapping period until an interrupt would be raised, while of course
in extreme cases an interrupt would be raised almost right away.
Leave an assertion.
Amends: aa42fc0e9c ("cpuidle: remove hpet access in hpet_broadcast_exit")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
I'm surprised gcc doesn't manage to do that: At least in debug builds two
call sites exist, just like source code has it. That's not necessary
though - by using do/while we can reduce this to a single call site. Then
the function will be inlined.
While improving code gen, also switch the function's 2nd parameter to
unsigned.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
If already we play with the IRQ count, we should do so only if we actually
"consume" the interrupt; normal timer IRQs should not have any adjustment
done.
Fixes: 3535332327 ("cpuidle: fix the menu governor to enhance IO performance")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
It's not clear why the enumerator was omitted, as these clearly shouldn't
take the "default" path (issuing a warning). Handle them the same as
legacy and PCIe->PCI bridges.
Fixes: e7e08d86ad ("IOMMU/PCI: consolidate pdev_type() and cache its result for a given device")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Unlike the setting of HPET_EVT_LEGACY in hpet_broadcast_init(), the
setting of HPET_EVT_DISABLE in hpet_disable_legacy_broadcast() isn't init-
only and hence can race other flag manipulation (not all of which occur
while holding the channel's lock). While possibly any such updates would
only ever occur when HPET_EVT_LEGACY isn't set in the first place, this
doesn't look straightforward to prove, so better be on the safe side.
Fixes: d09486dba3 ("cpuidle: Enable hpet broadcast by default")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
It's only ever handle_hpet_broadcast() that's used. While we now don't
enable IRQs right away, still play safe and convert the function pointer
to a boolean, to make sure no calls occur too early.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
"cpuid=no-rdrand" cannot actually be used to suppress the respective
boot warning on certain AMD hardware. That's because cleared_caps[] are
only applied after the ->c_init() hooks ran, i.e. cpu_has() still
returns true in init_amd().
Fixes: 93401e28a8 ("x86: clear RDRAND CPUID bit on AMD family 15h/16h")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Callers may pass in TSC values from before the local TSC stamp was last
updated (this would in particular be the case when the TSC was latched, a
time rendezvous occurs, and the latched value is used only afterwards).
scale_delta(), otoh, deals with unsigned values, and hence would treat
negative incoming deltas as huge positive values, yielding entirely bogus
return values.
Fixes: 88e64cb785 ("x86/HVM: use fixed TSC value when saving or restoring domain")
Reported-by: Антон Марков <akmarkov45@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
There's no need to invoke CPUID yet another time. This way two of the
static booleans can also go away.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
There's no need to invoke CPUID yet another time - we assume symmetry
anyway. With symmetry assumed, logging per-CPU also isn't very useful.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
There's no need to invoke CPUID yet another time. However, as the host CPU
policy is set up only shortly after init_intel() ran on the BSP, defer the
logic to a pre-SMP initcall. This can't be (a new) one in cpu/intel.c
though, as that's linked after acpi/cpu_idle.c (which is where we already
need the feature set). Since opt_arat is local to the cpu/ subtree,
introduce a new Intel-specific helper to hold the code needed.
Further, as we assume symmetry anyway, use setup_force_cpu_cap() and hence
limit the checking to the boot CPU.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Use the respective host CPU policy bit instead. This has the (tolerable,
as we generally assume symmetry anyway) effect of using the BSP's
(unleveled) setting, rather than the result of leveling (as is done by
identify_cpu() on boot_cpu_data, rendering the variable name somewhat
misleading).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Add a XEN infix, to properly distinguish it from the CPUID feature flag
(leaf 6 EAX bit 2).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
The tracking lasted only for about two weeks, but the related comment
parts were never purged.
Amends: 50b74f55e0 ("OOS cleanup: Fixup arrays instead of fixup tables")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
The code has been identified as HVM-only, and its main functions are
pretty well isolated. Move them to their own file. While moving, besides
making two functions non-static, do a few style adjustments, mainly
comment formatting, but leave the code otherwise untouched.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Switch to using IS_ENABLED() in both places, thus in particular making
sure XENPF_get_dom0_console handling doesn't take the "default" path: The
behavior better wouldn't differ between VIDEO=y and there not being VGA vs
VIDEO=n. For this to work, fill_console_start_info() needs to be
unconditionally declared; extend that to vga_console_info as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Omit the function when PV=n, by moving it to the sole file using it, thus
allowing it to become static as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Omit the function when HVM=n. With that the !HVM logic can also go away;
leave an assertion.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich