And use it to replace CODE_ALIGN in assembly. This allows to generalize the
way the code alignment gets set across all architectures.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
The xen sources contain violations of MISRA C:2012 Rule 11.8 whose
headline states:
"A conversion shall not remove any const, volatile or _Atomic qualification
from the type pointed to by a pointer".
Fix violation by adding missing const qualifier in cast.
Signed-off-by: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
restore_all_guest() does a lot of manipulation of the stack after popping the
GPRs, and uses raw %rsp displacements to do so. Also, almost all entrypaths
use raw %rsp displacements prior to pushing GPRs.
Provide better mnemonics, to aid readability and reduce the chance of errors
when editing.
No functional change. The resulting binary is identical.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
compat_restore_all_guest() already has SPEC_CTRL_EXIT_TO_PV with a documented
requirement for %rsp to be both regs and cpuinfo.
Use the now-normal annotations and simplify the expressions which happen to be
a subtraction of 0.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Some retroactive review, for if I'd got to the patch in time.
* The new ASM-friendly BUILD_BUG_ON() should be in a header file.
* entry_int82() wants the movl->movb treatment too.
Fixes: c144b9e324 ("x86: Reduce assembly code size of entry points")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Changeset ef3e8db806 ("x86/hvm: Corrections and improvements to
unhandled vmexit logging") introduced a printk to the default path of
the switch statement in nestedsvm_check_intercepts(), complaining of
an unknown exit reason.
Unfortunately, the "core" switch statement which is meant to handle
all vmexit reasons is in nsvm_vmcb_guest_intercepts_exitcode(); the
switch statement in nestedsvm_check_intercepts() is only meant to
superimpose on top of that some special-casing for how to interaction
between L1 and L0 vmexits.
Remove the printk, and add a comment to prevent future confusion.
Fixes: ef3e8db806 ("x86/hvm: Corrections and improvements to unhandled vmexit logging")
Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Moving them all together has several advantages:
* Collects them all in one part of the struct
* The `caps` field means that we can drop the "_supported" suffix, as it's
clear what is meant.
While here, replace some 0/1 values with false/true.
Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The "effective type" of the cpu_has_svm_feature macro is effectively
an unsigned log with one bit set (or not); at least one place someone
felt compelled to do a !! to make sure that they got a boolean out of
it.
Ideally the whole of this would be folded into the cpufeature.h
infrastructure. But for now, duplicate the more type-safe static
inlines in that file, and remove the !!.
Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
hvm_function_table is an internal structure; rather than manually
|-ing and &-ing bits, just make it a boolean bitfield and let the
compiler do all the work. This makes everything easier to read, and
presumably allows the compiler more flexibility in producing efficient
code.
No functional change intended.
Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The current logic to handle the BRANCH_HARDEN option will report it as enabled
even when build-time disabled. Fix this by only allowing the option to be set
when support for it is built into Xen.
Fixes: 2d6f36daa0 ('x86/nospec: Introduce CONFIG_SPECULATIVE_HARDEN_BRANCH')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Current issues with this test are:
- when the job timeout, the log file is lost as there is no chance to
run the `mv` command.
- GitLab job log is limited in size, so one usually have to download
the artifacts, which may be missing.
Use $GITLAB_CI to detect when the script is run as part of a GitLab
pipeline. GitLab will add "GITLAB_CI=true" in the environment
variables.
When run as part of $GITLAB_CI, ignore "dirty" worktree to allow to
write "build-each-commit-gcc.log", which can then be grabbed as
artifacts, even when the job timeout. The `git clean` command is
changed to keep those build logs.
When run as part of $GITLAB_CI, we will also store the build output in
a log file instead of writing it to stdout, because GitLab's job log
is limited. But we will write the log to stdout in case of error, so
we can find out more quickly why there's been an error.
This patch also make use of a GitLab feature, "log sections", which we
will collapse by default. One section per commit been built.
There's a bit of colour added to the logs.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
elf_load_binary() isn't the primary source of brokenness being
indicated. Therefore make the respective PVH log message there
conditional (much like PV has it), and add another instance when
elf_xen_parse() failed (again matching behavior in the PV case).
Make the PV side match the (new) use of %pd here.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Avoid logging all-identical messages for every vCPU, but make sure to
log unusual events like the vector differing from vCPU 0's (note that
the respective condition also makes sure vCPU 0 itself will have the
vector setting logged), or it changing after it was once set. (Arguably
a downside is that some vCPU not having its vector set would no longer
be recognizable from the logs. But I think that's tolerable as
sufficiently unlikely outside of people actively fiddling with related
code.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Move the macros mentioned in the commit subject to their appropriate
locations.
Additionally, eliminate the dependency of xen/lib.h from xen/bug.h and
include "xen/bug.h" in files where xen/bug.h macros are utilized.
Most of the changes were made because a file requires macros from xen/bug.h,
except for some files for Arm which require definitions of BUG_OPCODE,
BUG_INSTR, BUG_FN_REG.
xen/lib.h was added to list-sort.c ( otherwise compilation errors related
to {d}printk occur during compilation of list-sort.c. ) as xen/lib.h was
removed from xen/list.h. Since nothing in xen/list.h depends on xen/lib.h
functionality and only xen/bug.h is needed.
cpufeature.h requires the inclusion of <xen/cache.h>;
otherwise, the following error will occur:
ld: common/monitor.o:/build/xen/./arch/x86/include/asm/cpufeature.h:41:
multiple definitions of `__cacheline_aligned';
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
The current code for alternative calls uses the caller parameter types as the
types for the register variables that serve as function parameters:
uint8_t foo;
[...]
alternative_call(myfunc, foo);
Would expand roughly into:
register unint8_t a1_ asm("rdi") = foo;
register unsigned long a2_ asm("rsi");
[...]
asm volatile ("call *%c[addr](%%rip)"...);
However with -O2 clang will generate incorrect code, given the following
example:
unsigned int func(uint8_t t)
{
return t;
}
static void bar(uint8_t b)
{
int ret_;
register uint8_t di asm("rdi") = b;
register unsigned long si asm("rsi");
register unsigned long dx asm("rdx");
register unsigned long cx asm("rcx");
register unsigned long r8 asm("r8");
register unsigned long r9 asm("r9");
register unsigned long r10 asm("r10");
register unsigned long r11 asm("r11");
asm volatile ( "call %c[addr]"
: "+r" (di), "=r" (si), "=r" (dx),
"=r" (cx), "=r" (r8), "=r" (r9),
"=r" (r10), "=r" (r11), "=a" (ret_)
: [addr] "i" (&(func)), "g" (func)
: "memory" );
}
void foo(unsigned int a)
{
bar(a);
}
Clang generates the following assembly code:
func: # @func
movl %edi, %eax
retq
foo: # @foo
callq func
retq
Note the truncation of the unsigned int parameter 'a' of foo() to uint8_t when
passed into bar() is lost. clang doesn't zero extend the parameters in the
callee when required, as the psABI mandates.
The above can be worked around by using a union when defining the register
variables, so that `di` becomes:
register union {
uint8_t e;
unsigned long r;
} di asm("rdi") = { .e = b };
Which results in following code generated for `foo()`:
foo: # @foo
movzbl %dil, %edi
callq func
retq
So the truncation is not longer lost. Apply such workaround only when built
with clang.
Reported-by: Matthew Grooms <mgrooms@shrew.net>
Link: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277200
Link: https://github.com/llvm/llvm-project/issues/12579
Link: https://github.com/llvm/llvm-project/issues/82598
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
'*' sets the width field, which is the minimum number of characters to output,
but what we want in no_config_param() is the precision instead, which is '.*'
as it imposes a maximum limit on the output.
Fixes: 68d757df8d ('x86/pv: Options to disable and/or compile out 32bit PV support')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
These functions never saw a usage of their return value since
they were introduced, so it can be dropped since their usages
violate MISRA C Rule 17.7:
"The value returned by a function having non-void return type shall be used".
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The usage of a cmpxchg loop in hpet_get_channel() is unnecessary, as the same
can be achieved with an atomic increment, which is both simpler to read, and
avoid any need for a loop.
Note there can be a small divergence in the channel returned if next_channel
overflows, but returned channel will always be in the [0, num_hpets_used)
range, and that's fine for the purpose of balancing HPET channels across CPUs.
Signed-of-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The test code has a typo in using LIVEPATCH_APPLY_HOOK() instead of
LIVEPATCH_REVERT_HOOK().
Fixes: 6047104c3c ('livepatch: Add per-function applied/reverted state tracking marker')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
With 9pfs now available in Xenstore-stubdom, there is no reason to
have distinct do_control_memreport() variants for the daemon and the
stubdom implementations.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
With 9pfs being fully available in Xenstore-stubdom now, there is no
reason to not fully support all logging capabilities in stubdom.
Open the logfile on stubdom only after the 9pfs file system has been
mounted.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Add some helpers for handling filenames which might need different
implementations between stubdom and daemon environments:
- expansion of relative filenames (those are not really defined today,
just expand them to be relative to /var/lib/xen/xenstore)
- expansion of xenstore_daemon_rundir() (used e.g. for saving the state
file in case of live update - needs to be unchanged in the daemon
case, but should result in /var/lib/xen/xenstore for stubdom)
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Mount the 9pfs device in stubdom enabling it to use files.
This has to happen in a worker thread in order to allow the main thread
handling the required Xenstore accesses in parallel.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Add a 9pfs device to Xenstore stubdom in order to allow it to do e.g.
logging into a dom0 file.
Use the following parameters for the new device:
- tag = "Xen"
- type = "xen_9pfsd"
- path = "/var/lib/xen/xenstore"
- security-model = "none"
For now don't limit allowed file space or number of files.
Add a new libxl function for adding it similar to the function for
adding the console device.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Extend the config files of the Xenstore stubdoms to include XENBUS
and 9PFRONT items in order to support file based logging.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Add support for the new 9pfs backend "xen_9pfsd". For this backend type
the tag defaults to "Xen" and the host side path to
"/var/log/xen/guests/<dom-name>".
Do most of the default settings in libxl. Unfortunately the default
path can't easily be set in libxl, as the domain name isn't available
in the related 9pfs specific function.
Settings the defaults in libxl requires to move the sanity checking
of 9pfs parameters from xl to libxl, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Make the backend type of 9pfs PV devices configurable. The default is
"qemu" with the related Xenstore backend-side directory being "9pfs".
Add another type "xen_9pfsd" with the related Xenstore backend-side
directory "xen_9pfs".
As additional security features it is possible to specify:
- "max-space" for limiting the maximum space consumed on the filesystem
in MBs
- "max-files" for limiting the maximum number of files in the
filesystem
- "max-open-files" for limiting the maximum number of concurrent open
files
For convenience "auto-delete" is available to let the backend delete the
oldest file of the guest in case otherwise "max-space" or "max-files"
would be violated.
The xen-9pfsd daemon will be started by libxenlight automatically when
the first "xen_9pfs" device is being created.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: George Dunlap <george.dunlap@cloud.com> # Golang bits
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Now that we no longer have a hashtable inside we can make Quota.t pure, and
push the mutable update to its callers. Store.t already had a mutable Quota.t
field.
No functional change.
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
On a stress test running 1000 VMs flamegraphs have shown that
`oxenstored` spends a large amount of time in `Hashtbl.copy` and the GC.
Hashtable complexity:
* read/write: O(1) average
* copy: O(domains) -- copying the entire table
Map complexity:
* read/write: O(log n) worst case
* copy: O(1) -- a word copy
We always perform at least one 'copy' when processing each xenstore
packet (regardless whether it is a readonly operation or inside a
transaction or not), so the actual complexity per packet is:
* Hashtbl: O(domains)
* Map: O(log domains)
Maps are the clear winner, and a better fit for the immutable xenstore
tree.
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
This patch addresses a behavior discrepancy in the handling of altp2m views,
where upon the creation and subsequent EPT violation, the page access
permissions were incorrectly inherited from the hostp2m instead of respecting
the altp2m default_access.
Previously, when a new altp2m view was established with restrictive
default_access permissions and activated via xc_altp2m_switch_to_view(),
it failed to trigger an event on the first access violation. This behavior
diverged from the intended mechanism, where the altp2m's default_access
should dictate the initial permissions, ensuring proper event triggering on
access violations.
The correction involves modifying the handling mechanism to respect the
altp2m view's default_access upon its activation, eliminating the need for
setting memory access permissions for the entire altp2m range (e.g. within
xen-access.c). This change not only aligns the behavior with the expected
access control logic but also results in a significant performance improvement
by reducing the overhead associated with setting memory access permissions
across the altp2m range.
Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
All architectures have copy&pasted bad logic from x86.
OFFSET() having a trailing semi-colon within the macro expansion can be a
problematic pattern. It's benign in this case, but fix it anyway.
Perform style fixes for the other macros, and tame the mess of BLANK()
position to be consistent (one BLANK() after each block) so the intermediate
form is legible too.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Implement the in-code suppression comment at the end of the line.
Now it is possible to add a Xen deviaiton comment with the syntax
described in the docs at the end of the line affected by the
violation, to deviate it.
Eclair natively supports it, so the translation for the tool will
be straighforward, but the other tool needs to translate an
occurrence of the tag into a suppressino comment at the end of the
previous line, this will have a corner case where the line number
1 of the file can't be deviated in this way for such tools.
Updated documentation.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Change the code in tag_database.py to substitute only the SAF-*
comment instead of replacing the line.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Use named group for the regex matching a tag identifier, this
is done to ease retrieving the matching group instead of using
an index.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
The type not being used in do_bug_frame() is suspicious. Apparently
that's solely because the type uses a pointer-to-const parameter,
when so far run_in_exception_handler() wanted functions taking pointer-
to-non-const. Expand use of const, in turn requiring common code's
do_bug_frame() as well as [gs]et_irq_regs() to also gain const. This
then brings the former function also closer to the common one, with
Arm's use of vaddr_t remaining as a difference.
While there also replace the bogus use of hard tabs in [gs]et_irq_regs()
(I clearly didn't mean to put it in like this).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
The only place it was needed is in the spurious handler, and there we
can use get_irq_regs() instead.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
It's not needed anymore. This is in preparation of dropping the register
parameters from IRQ handler functions.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
In preparation for further removal of regs parameters, drop it here. In
the two places where it's actually needed, retrieve IRQ context if
available, or else guest context.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
In preparation of dropping the register parameters from
serial_[rt]x_interrupt() and in turn from IRQ handler functions,
register state needs making available another way for the few key
handlers which need it. Fake IRQ-like state.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
When re-working library call wrapping the sed invocation didn't account
for all sources living in the parent directory when building the 32-bit
harness binary.
Fixes: 6fba45ca3b ("x86emul: rework wrapping of libc functions in test and fuzzing harnesses")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
EVEX.R' is not ignored in 64-bit code when encoding a GPR or mask
register. While for mask registers suitable checks are in place (there
also covering EVEX.R), they were missing for the few cases where in
EVEX-encoded instructions ModR/M.reg encodes a GPR. While for VPEXTRW
the bit is replaced before an emulation stub is invoked, for
VCVT{,T}{S,D,H}2{,U}SI this actually would have led to #UD from inside
an emulation stub, in turn raising #UD to the guest, but accompanied by
log messages indicating something's wrong in Xen nevertheless.
Fixes: 001bd91ad8 ("x86emul: support AVX512{F,BW,DQ} extract insns")
Fixes: baf4a376f5 ("x86emul: support AVX512F legacy-equivalent scalar int/FP conversion insns")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
__get_user_bad() and __put_user_bad() are undefined symbols used
to assert the unreachability of a program point:
a call to one of such functions is optimized away if it is considered
unreachable by the compiler. Otherwise, a linker error is reported.
In accordance with the purpose of such constructs:
1) add the attribute noreturn to __get_user_bad() and __put_user_bad();
2) change return type of __get_user_bad() to void (returning long is a
leftover from the past).
Point (1) meets the requirements to deviate MISRA C:2012 Rule 16.3
("An unconditional break statement shall terminate every switch
clause") since functions with noreturn attribute are considered
as allowed terminals for switch clauses.
Point (2) addresses several violations of MISRA C:2012 Rule 17.7
("The value returned by a function having non-void return type
shall be used").
While there also zap "extern".
No functional change.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Because of using "-include", failure to (re)build auto.conf (with
auto.conf.cmd produced as a secondary target) won't stop make from
continuing the build. Arrange for it being possible to drop the - from
Rules.mk, requiring that the include be skipped for tools-only targets.
Note that relying on the inclusion in those cases wouldn't be correct
anyway, as it might be a stale file (yet to be rebuilt) which would be
included, while during initial build, the file would be absent
altogether.
Fixes: 8d4c17a90b ("xen/build: silence make warnings about missing auto.conf*")
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
In xen-9pfsd fill_data() va_end() needs to be called before returning.
Coverity Id CID 1592145
Fixes: bcec59cf7f ("tools/xen-9pfsd: add 9pfs version request support")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Roger Pau Monné