We can't use initializers of unnamed struct/union members just yet.
Fixes: d638fe233c ("libxl: use the cpuid feature names from cpufeatureset.h")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Because of -Wshadow the build otherwise fails with old enough glibc.
While there also obey line length limits for msr_add().
Fixes: 6d21cedbaa ("libxl: add support for parsing MSR features")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Convert style from a Xen/Linux mix to pure Xen while doing the move. No
other changes, despite having been heavily tempted to do some - at the
very least to make simple_strtoul() and simple_strtoull() the same in
how they deal with non-numeric digits.
Requested-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Shawn Anastasio <sanastasio@raptorengineering.com>
The primary goal is to eliminate the Misra-non-compliance of "desc"
shadowing at least the local variable in hvm_load(). Suffix both local
variables with underscores, while also
- dropping leading underscores from parameter names (applying this also
to the two wrapper macros),
- correcting indentation,
- correcting brace placement,
- dropping unnecessary parentheses around parameter uses when those are
passed on as plain arguments.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Not exactly overlapping accesses to objects on the left and right hand
sides of an assignment are generally UB, and hence disallowed by Misra.
While in the specific cases we're talking about here no actual UB can
result as long as the compiler doesn't act actively "maliciously", let's
still switch to using casts combined with exactly overlapping accesses.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This is to avoid the need for forward declarations, which in turn
addresses a violation of MISRA C:2012 Rule 8.3 ("All declarations of an
object or function shall use the same names and type qualifiers").
While doing so,
- drop inline (leaving the decision to the compiler),
- add const,
- add unsigned,
- correct style.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Loading the VGA base address involves sym_esi(), i.e. %esi still needs
to hold the relocation base address. Therefore the address of the
message to output cannot be "passed" in %esi. Put the message offset in
%ecx instead, adding it into %esi _after_ its last use as base address.
Fixes: b28044226e ("x86: make Xen early boot code relocatable")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Introduce xen/macros.h for this purpose. For now xen/lib.h simply
includes xen/macro.h, until consumers can be suitable cleaned up.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Shawn Anastasio <sanastasio@raptorengineering.com>
As supported by the printk() (deliberately made visible in context by
also correcting a mis-indented return statement), "above 4GiB" is meant
here. Avoid comparison with a constant to "escape" Misra rule 7.2
complaints. (Note however that even up-to-date Linux, which is where we
"inherited" this code from, still uses the very same off-by-1 check.)
Fixes: 94ea0622c5 ("x86-64/mmcfg: relax base address restriction")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
While the referenced commit came without any update to the public header
(which doesn't clarify how the upper address bits are used), the
intention looks to have been that bits 12..19 and 40..63 form the pIRQ.
Negative values simply make no sense, and pirq_info() also generally
wants invoking with an unsigned (and not just positive) value.
Since the line was pointed out by Eclair, address Misra rule 7.2 at the
same time, by adding the missing U suffix.
Fixes: 88fccdd11c ("xen: event channel remapping for emulated MSIs")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Some of them aren't liked by Misra rule 7.2; rather than fixing them,
drop the affected ones and a few more that aren't used (anymore). (Note
that e.g. some MSR_K7_* are applicable on K8 and newer as well, so need
retaining.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This being x86-only right now, and its x86 bits being heavily outdated,
let's no longer enable this by default.
While there also adjust the "help" keyword spelling.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
talloc_vasprintf() has a va_list type parameter, so this type needs to
be defined (independent of the particular libc implementation).
Fixes: 63b6419d2a ("tools/xenstore: split out rest of live update control code")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
We still don't use it (in the tool stack), and its values (plural) also
aren't fetched correctly, but it is odd to continue to see the
hypervisor log "ELF: note: unknown (0xd)" when loading a Linux Dom0.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
For an approach like that used in "x86: detect PIT aliasing on ports
other than 0x4[0-3]" [1] to work, channel 2 may not (appear to) continue
counting when "gate" is low. Record the time when "gate" goes low, and
adjust pit_get_{count,out}() accordingly. Additionally for most of the
modes a rising edge of "gate" doesn't mean just "resume counting", but
"initiate counting", i.e. specifically the reloading of the counter with
its init value.
No special handling for state save/load: See the comment near the end of
pit_load().
Along with introducing the get_count() helper to have the calculations
(and the locking check) in a single place, switch pit_get_count()'s d,
counter, and return type to unsigned int.
[1] https://lists.xen.org/archives/html/xen-devel/2023-05/msg00898.html
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
To avoid the need for a forward declaration of pit_load_count() in a
subsequent change, move it earlier in the file (along with its helper
callback).
While moving the code, address a few style issues.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Let's try to avoid giving the impression that 32-bit tool stacks are as
capable as 64-bit ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Having ALIGN() inside a section definition usually makes sense only with
a label definition following (an exception case is a few lines out of
context, where cache line sharing is intended to be avoided).
Constituents of .bss.page_aligned need to specify their own alignment
correctly anyway, or else they're susceptible to link order changing.
This requirement is already met: Arm-specific code has no such object,
while common (EFI) code has another one. That one has suitable alignment
specified.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
To comply with Misra rule 8.10 ("An inline function shall be declared
with the static storage class"), convert what is presently
_spin_lock_cb() to an always-inline (and static) helper, while making
the function itself a thin wrapper, just like _spin_lock() is.
While there drop the unlikely() from the callback check, and correct
indentation in _spin_lock().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
This in particular allows to drop a dead assignment to "ptes" from near
the end of the function.
Coverity ID: 1532314
Fixes: bd7a29c3d0 ("tools/libs/ctrl: fix xc_core_arch_map_p2m() to support linear p2m table")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
"t" is written first thing at the "retry_transaction" label.
Coverity ID: 1532321
Fixes: 1057300109 ("libxl: fix error handling (xenstore transaction leak) in libxl__domain_make")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
"rc" is written immediately below the outer if(). Fold the remaining two
if()s.
Coverity ID: 1532320
Fixes: 685e922d6f ("tools/libxc: Rework xc_cpuid_apply_policy() to use {get,set}_cpu_policy()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
The variable isn't used past the loop, and its value also isn't
meaningful across iterations. Reduce its scope to make this more
obvious.
Coverity ID: 1532310
Fixes: ae763e4224 ("tools/misc: introduce xen-mfndump")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Fields that generic_identify() sets unconditionally don't need pre-
setting. (In fact the compiler removes some of those assignments anyway,
at least in release builds.)
With the setting of ->cpuid_level to -1 gone, also drop the respective
BUG_ON() from default_init().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Naming of some of the builtins isn't fully consistent with that of pre-
existing ones, so there's a need for a new BR2() wrapper macro.
With the tests providing some proof of proper functioning of the
emulator code also enable use of the feature by guests, as there's no
other infrastructure involved in enabling this ISA extension.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Henry Wang <Henry.Wang@arm.com> # CHANGELOG
While slightly longer, I agree with Andrew that using it helps
readability. Where touching them anyway, also wrap some overly long
lines.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
These are easiest in that they have same-size source and destination
vectors, yet they're different from other conversion insns in that they
use opcodes which have different meaning in the 0F encoding space
({,V}H{ADD,SUB}P{S,D}), hence requiring a little bit of overriding.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Aspects to consider are that these have 32-bit element size (pairs of
FP16) and that there are restrictions on the registers valid to use.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
While, as before, this leverages that the Map6 encoding space is a very
sparse clone of the "0f38" one, switch around the simd_size overriding
for opcode 2D. This way fewer separate overrides are needed.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The Map6 encoding space is a very sparse clone of the "0f38" one. Once
again re-use that table, as the entries corresponding to invalid opcodes
in Map6 are simply benign with simd_size forced to other than simd_none
(preventing undue memory reads in SrcMem handling early in
x86_emulate()).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This encoding space is a very sparse clone of the "twobyte" one. Re-use
that table, as the entries corresponding to invalid opcodes in Map5 are
simply benign with simd_size forced to other than simd_none (preventing
undue memory reads in SrcMem handling early in x86_emulate()).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
In order to re-use (also in subsequent patches) existing code and tables
as much as possible, simply introduce a new boolean field in emulator
state indicating whether an insn is one with a half-precision source.
Everything else then follows "naturally".
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Checking for what $(SIMD) contains was initially right, but already the
addition of $(FMA) wasn't. Later categories (correctly) weren't added.
Instead what is of interest is anything the main harness source file
uses outside of suitable #if and without resorting to .byte, as that's
the one file (containing actual tests) which has to succeed in building.
The auxiliary binary blobs we utilize may fail to build; the resulting
empty blobs are recognized and reported as "n/a" when the harness is
run.
Note that strictly speaking we'd need to probe the assembler. We assume
that a compiler knowing of a certain ISA extension is backed by an
equally capable assembler.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Hidden devices (e.g. an add-in PCI serial card used for Xen's serial
console) are associated with DomXEN, not Dom0. This means that while
looking for overlapping BARs such devices cannot be found on Dom0's list
of devices; DomXEN's list also needs to be scanned.
Suppress vPCI init altogether for r/o devices (which constitute a subset
of hidden ones).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
There's no point consuming two port-I/O slots. Even less so considering
that some real hardware permits both ports to be accessed in one go,
emulating of which requires there to be only a single instance.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
When either feature is available in hardware, but disabled for a guest,
the respective insn would better cause #UD if attempted to be used.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Both have separate enable bits, which are optional. While on real
hardware we can perhaps expect these VMX controls to be available if
(and only if) the base CPU feature is available, when running
virtualized ourselves this may not be the case.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
PV domains won't use it, and even HVM ones won't when OOS is turned off
for them. There's therefore no point in putting extra pressure on the
(limited) pool of memory.
While there also zap the sh_type_to_size[] entry when OOS is disabled
altogether.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Presumably by copy-and-paste we've accumulated a number of instances of
$(@D)/$(@F), which really is nothing else than $@. The split form only
needs using when we want to e.g. insert a leading . at the beginning of
the file name portion of the full name.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
There's no real need for the associated probing - we can easily convert
to a uniform value without knowing the specific behavior (note also that
the respective comments weren't fully correct and have gone stale).
No difference in generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
While normal booting is properly supported on both x86 and Arm64, secure
boot reportedly requires quite a bit more work to be actually usable
(and providing the intended guarantees). The mere use of the shim
protocol for verifying the Dom0 kernel image isn't enough.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
The change to XEN_DOMCTL_getdomaininfo was a binary incompatible one,
and the interface version wasn't bumped yet during the 4.18 release
cycle.
Fixes: 31c6554974 ("domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Commit 68823df358 ("acpi: Re-license ACPI builder files from GPLv2 to
LGPLv2.1") added references to a "special exception on linking described
in file LICENSE", without actually adding such a file. Quite likely
COPYING was meant instead, yet then its text matches LICENSES/LGPL-2.1
except for some explanatory text (clarifying the "only" aspect) at the
top (and formatting). Hence replace the text in all the files with SPDX
references to LGPL-2.1.
Note that dsdt_acpi_info.asl had no license text. An SPDX tag is being
added there nevertheless.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This symlink is getting in the way of using e.g. "find" on the xen/
subtree, and it isn't really needed when not building out-of-tree:
The one use that there was can easily be avoided.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
I don't view a variable of this name as suitable for exporting, the more
that it carries entirely redundant information. The reasons for its
introduction in Linux commit 051f278e9d81 ("kbuild: replace
KBUILD_SRCTREE with boolean building_out_of_srctree") also don't apply
to us. Ditch exporting of the variable, replacing uses suitably.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
While I was the one to introduce it, I don't think it is correct: A
bogus continuation call issued by a tool stack domain may find another
continuation in progress. IOW we've been asserting caller controlled
state (which is reachable only via a domctl), and the early (lock-less)
check in paging_domctl() helps in a limited way only.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
As of XSA-397 the only caller passes true for it. Simplify things by
getting rid of the parameter for both the internal paging function and
the involved hook.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
It looks like in the combination of aff8bf94ce ('x86/shadow: only
4-level guest code needs building when !HVM') and 0b841314da
('x86/shadow: sh_{make,destroy}_monitor_table() are "even more" HVM-
only') I didn't go quite far enough: SH_type_monitor_table is also
effectively unused when !HVM.
The assertion early in sh_destroy_shadow() can have the type dropped
altogether: it shouldn't make it here in the first place. Pages of
this type are freed directly from sh_destroy_monitor_table() only.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
... in order to also intercept Dom0 accesses through the alias ports.
Also stop intercepting accesses to the CMOS ports if we won't ourselves
use the CMOS RTC, because of there being none. This doesn't go as far as
covering port 0x70, as that also has the NMI disable bit, which we don't
want to permit Dom0 to set.
Note that rtc_init() deliberately uses 16 as the upper loop bound,
despite probe_cmos_alias() using 8: The higher bound is benign now, but
would save us touching the code (or, worse, missing to touch it) in case
the lower one was doubled.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Old gcc won't cope with initializers involving unnamed struct/union
fields.
Fixes: 441b1b2a50 ("x86/emul: Switch x86_emulate_ctxt to cpu_policy")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The function merely needs moving earlier in the file to avoid the need
for a forward declaration. While moving it, also rename it following the
recent folding of CPUID and MSR policies.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
An earlier change converted TARGET-y to TARGETS, but failed to replace
all references. Convert run's dependency, but use $< in the command to
avoid the leading blank that += inserts.
Fixes: 6a9f547763 ("tests/cpu-policy: Rework Makefile")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Matching what was done earlier, explicit tests are added only for
irregular insn / memory access patterns.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
These are close relatives of the AVX-VNNI ISA extension. Since the insns
here and in particular their memory access patterns follow the usual
scheme (and especially the byte variants of AVX-VNNI), I didn't think it
was necessary to add a contrived test specifically for them.
While making the addition also re-wire AVX-VNNI's handling to
simd_0f_ymm: There's no reason to check the AVX feature alongside the
one actually of interest (there are a few features where two checks are
actually necessary, e.g. GFNI+AVX, but this isn't the case here).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
As in a few cases before (in particular: AVX512_IFMA), since the insns
here and in particular their memory access patterns follow the usual
scheme, I didn't think it was necessary to add a contrived test
specifically for them.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This can be used to reduce the number of parameters x86emul_fpu() needs
to take, and will be re-used subsequently.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
For an unclear reason 0552a8cfda ("x86emul: track only rIP in emulator
state") converted the original struct cpu_user_regs instance to a
pointer, rather than dropping the field altogether: The pointer merely
aliases the one in the context structure.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This insn differs from WRMSR solely in the lack of serialization. Hence
the code used there can simply be used here as well, plus a feature
check of course. As there's no other infrastructure needed beyond
permitting the insn for PV privileged-op emulation (in particular no
separate new VMEXIT) we can expose the insn to guests right away.
Don't expose the feature to PV guests, as the involved #UD is
serializing anyway.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
I should have remembered to adjust the splitting patches accordingly,
but I forgot. While making the adjustment also correct fpu.c's first
comment line.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
A use of $(x86_emulate.h) was introduced (mirroring what the testharness
has) without realizing that no such variable exists here. (Re)name the
variable (to) "private.h", which better expresses what is included which
way.
Note that because of automatic dependencies tracking, unlike in the test
harness no $(x86.h) variable is needed here - we solely need explicit
dependencies for files which need symlinks created.
Fixes: 9ace97ab9b ("x86emul: split off opcode 0f01 handling")
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 reverts commit 24c471b6f4. I
wasn't thinking quite right; the line was added just recently,
when the symlink-ed dir was converted to a real one. Thanks to
Athony Perard for noticing the mistake.
The stride values (based on 32-bit element size) were wrong for these
two test, yielding misleading output (especially when comparing with the
test variants also involving AVX512-VBMI2).
Also insert a missing blank on a nearby, related line.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Besides a printk() the main effect is slight corruption of the start
info magic: While that's meant to be xen-3.0-x86_64, it wrongly ended
up as xen-3.0-x86_64p. (The extended-CR3 VM-assist thus won't be
enabled anymore either, but that's meaningless to 64-bit PV anyway.)
Note that no known users exist that would have developed a dependency on
the bogus magic string. In particular Linux, NetBSD, and mini-os have
been checked.
Fixes: 460060f83d ("libelf: use for x86 dom0 builder")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The latter of the two commits referenced below converted x86_emulate
from a symlinked dir to a real one, holding symlinked files. Yet even
before that the split between distclean and clean was suspicious: A
similar split, removing symlinks only in distclean, doesn't exist
anywhere else in the tree afaics.
Fixes: c808475882 ("tools/fuzz: introduce x86 instruction emulator target")
Fixes: 9ace97ab9b ("x86emul: split off opcode 0f01 handling")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Many are needed by the hypervisor only - have one file for this purpose.
Some are also needed by the harness (but not the fuzzer) - have another
file for these.
Code moved gets slightly adjusted in a few places, e.g. replacing
"state" by "s" (like was done for other that has been split off).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
The function is already non-trivial and is expected to further grow.
Code moved gets slightly adjusted in a few places, e.g. replacing EXC_*
by X86_EXC_* (such that EXC_* don't need to move as well; we want these
to be phased out anyway).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
This is a fair chunk of code and data and can easily live separate from
the main emulation function.
Code moved gets slightly adjusted in a few places, e.g. replacing EXC_*
by X86_EXC_* (such that EXC_* don't need to move as well; we want these
to be phased out anyway).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Some of the helper functions/macros are needed only for this, and the
code is otherwise relatively independent of other parts of the emulator.
Code moved gets slightly adjusted in a few places, e.g. replacing EXC_*
by X86_EXC_* (such that EXC_* don't need to move as well; we want these
to be phased out anyway).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
There's a fair amount of sub-cases (with some yet to be implemented), so
a separate function seems warranted.
Code moved gets slightly adjusted in a few places, e.g. replacing EXC_*
by X86_EXC_* (such that EXC_* don't need to move as well; we want these
to be phased out anyway).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
There's a fair amount of sub-cases (with some yet to be implemented), so
a separate function seems warranted.
Code moved gets slightly adjusted in a few places, e.g. replacing EXC_*
by X86_EXC_* (such that EXC_* don't need to move as well; we want these
to be phased out anyway).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
There's a fair amount of sub-cases (with some yet to be implemented), so
a separate function seems warranted.
Code moved gets slightly adjusted in a few places, e.g. replacing EXC_*
by X86_EXC_* (such that EXC_* don't need to move as well; we want these
to be phased out anyway).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
FOREACH_PRESENT_L<N>E(), as their names (now) say, already invoke the
"body" only when the present bit is set; no need to re-do the check.
While there also
- stop open-coding mfn_to_maddr() in code being touched (re-indented)
anyway,
- stop open-coding mfn_eq() in code being touched or in adjacent code,
- drop local variables when they're no longer used at least twice.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
These being local macros, the SHADOW prefix doesn't gain us much. What
is more important to be aware of at use sites is that the supplied code
is invoked for present entries only.
While making the adjustment also properly use NULL for the 3rd argument
at respective invocation sites.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Based on observations on a fair range of hardware from both primary
vendors even zero-iteration-count instances of these insns perform the
port related permission checking first.
Fixes: fe30060046 ("x86: Fix emulation of REP prefix")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
First of all these were inverted: "bridge=" caused the port coordinates
to be established, while "port=" controlled the bridge coordinates. And
then the error messages being identical also wasn't helpful. While
correcting this also move both case blocks close together.
Fixes: 97fd49a7e0 ("ns16550: add support for UART parameters to be specifed with name-value pairs")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
When !GRANT_TABLE and !PV_SHIM headers-n contains grant_table.h twice,
causing make to complain "target '...' given more than once in the same
rule" for the rule generating the stub headers. We don't need duplicate
entries in headers-n anywhere, so zap them (by using $(sort ...)) right
where the final value of the variable is constructed.
Fixes: 6bec713f87 ("include/compat: produce stubs for headers not otherwise generated")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
XEN_DOMCTL_CDF_oos_off is forced set for PV domains, so the logic can't
ever be engaged for them. Conditionalize respective fields and remove
the respective bit from SHADOW_OPTIMIZATIONS when !HVM. As a result the
SH_type_oos_snapshot constant can disappear altogether in that case, and
a couple of #ifdef-s can also be dropped/combined.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
These aren't mode dependent (see 06f04f54ba ["x86/shadow:
sh_{write,cmpxchg}_guest_entry() are PV-only"], where they were moved
out of multi.c) and hence there's no need to have pointers to the
functions in struct shadow_paging_mode. Due to include dependencies,
however, the "paging" wrappers need to move out of paging.h; they're
needed from PV memory management code only anyway, so by moving them
their exposure is reduced at the same time.
By carefully placing the (moved and renamed) shadow function
declarations, #ifdef can also be dropped from the "paging" wrappers
(paging_mode_shadow() is constant false when !SHADOW_PAGING).
While moving the code, drop the (largely wrong) comment from
paging_write_guest_entry() and reduce that of
paging_cmpxchg_guest_entry().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
sh_update_paging_modes() as its last action already invokes
sh_update_cr3(). Therefore there is no reason to invoke update_cr3()
another time immediately after calling paging_update_paging_modes(),
especially as sh_update_cr3() does not short-circuit the "nothing
changed" case.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Ordinary scalar operations are used in a multitude of other places, so
do so here as well. In fact take the opportunity and drop a local
variable then as well, first and foremost to get rid of a bogus cast.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The "32b" and "pae" functions are identical at the source level (they
differ in what they get compiled to, due to differences in
SHADOW_FOREACH_L2E()), leaving aside a comment the PAE variant has and
the non-PAE one doesn't. Replace these infixes by the more usual l<N>
ones (and then also for the "64b" one for consistency; that'll also
allow for re-use once we support 5-level paging, if need be). The two
different instances are still distinguishable by their "level" suffix.
While fiddling with the names, convert the last parameter to boolean
as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
While no caller currently invokes the function without first making sure
there is at least one shadow [1], we'd better eliminate UB here:
find_first_set_bit() requires input to be non-zero to return a well-
defined result.
Further, using find_first_set_bit() isn't very efficient in the first
place for the intended purpose.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
[1] The function has exactly two uses, and both are from OOS code, which
is HVM-only. For HVM (but not for PV) sh_mfn_is_a_page_table(),
guarding the call to sh_unsync(), guarantees at least one shadow.
Hence even if sh_page_has_multiple_shadows() returned a bogus value
when invoked for a PV domain, the subsequent is_hvm_vcpu() and
oos_active checks (the former being redundant with the latter) will
compensate. (Arguably that oos_active check should come first, for
both clarity and efficiency reasons.)
These are used from common code, so their signatures should be
consistent across architectures. This is achieved / guaranteed easiest
when their declarations are in a common header.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
While the RCU variants of list insertion and removal allow lockless list
traversal (with RCU just read-locked), insertions and removals still
need serializing amongst themselves. To keep things simple, use the
domain lock for this purpose.
This is CVE-2022-42334 / part of XSA-428.
Fixes: 642123c512 ("x86/hvm: provide XEN_DMOP_pin_memory_cacheattr")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
This is exposed via DMOP, i.e. to potentially not fully privileged
device models. With that we may not permit registration of an (almost)
unbounded amount of such regions.
This is CVE-2022-42333 / part of XSA-428.
Fixes: 642123c512 ("x86/hvm: provide XEN_DMOP_pin_memory_cacheattr")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Pre-allocation is intended to ensure that in the course of constructing
or updating shadows there won't be any risk of just made shadows or
shadows being acted upon can disappear under our feet. The amount of
pages pre-allocated then, however, needs to account for all possible
subsequent allocations. While the use in sh_page_fault() accounts for
all shadows which may need making, so far it didn't account for
allocations coming from log-dirty tracking (which piggybacks onto the
P2M allocation functions).
Since shadow_prealloc() takes a count of shadows (or other data
structures) rather than a count of pages, putting the adjustment at the
call site of this function won't work very well: We simply can't express
the correct count that way in all cases. Instead take care of this in
the function itself, by "snooping" for L1 type requests. (While not
applicable right now, future new request sites of L1 tables would then
also be covered right away.)
It is relevant to note here that pre-allocations like the one done from
shadow_alloc_p2m_page() are benign when they fall in the "scope" of an
earlier pre-alloc which already included that count: The inner call will
simply find enough pages available then; it'll bail right away.
This is CVE-2022-42332 / XSA-427.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
An earlier change with the same title (commit 1ba66a870e) altered only
the path where x2apic_phys was already set to false (perhaps from the
command line). The same of course needs applying when the variable
wasn't modified yet from its initial value.
Reported-by: Elliott Mitchell <ehem+xen@m5p.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The hook isn't mode dependent, hence it's misplaced in struct
paging_mode. (Or alternatively I see no reason why the alloc_page() and
free_page() hooks don't also live there.) Move it to struct
paging_domain.
The hook also is used for HVM guests only, so make respective pieces
conditional upon CONFIG_HVM.
While there also add __must_check to the hook declaration, as it's
imperative that callers deal with getting back "false".
While moving the shadow implementation, introduce a "curr" local
variable.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The hook isn't mode dependent, hence it's misplaced in struct
paging_mode. (Or alternatively I see no reason why the alloc_page() and
free_page() hooks don't also live there.) Move it to struct
paging_domain.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The fixes for XSA-410 have arranged for P2M pages being freed by P2M
code to be properly freed directly, rather than being put back on the
paging pool list. Therefore whatever p2m_teardown() may return will no
longer need taking care of here. Drop the code, leaving the assertions
in place and adding "total" back to the PAGING_PRINTK() message.
With merely the (optional) log message and the assertions left, there's
really no point anymore to hold the paging lock there, so drop that too.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich