In {big,little}_endian.h the changes are entirely mechanical, except for
dealing with casting away of const from pointers-to-const on lines
touched anyway.
In swab.h the casting of constants is done away with as well - I simply
don't see what the respective comment is concerned about in our
environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
architecture, sizeof(long long) >= 8). The comment is certainly relevant
in more general cases. Excess parentheses are dropped as well,
___swab64()'s local variable is renamed, and __arch__swab64()'s is
dropped as being redundant with ___swab64()'s.
Excessive casts compared to ___{,constant_}swab{16,32}() are also
dropped. Much like excessive ones in __fswab64().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Frediano Ziglio <frediano.ziglio@cloud.com>
... and move the type itself to linux-compat.h.
While doing so drop casts (instead of modiyfing them) from x86'es
wrmsrl().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Frediano Ziglio <frediano.ziglio@cloud.com>
For quite a while we didn't need to be concerned of 32-bit code
generation anymore: Simply use the 64-bit form of BSWAP here.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Frediano Ziglio <frediano.ziglio@cloud.com>
In {big,little}_endian.h the changes are entirely mechanical, except for
dealing with casting away of const from pointers-to-const on lines
touched anyway.
In swab.h the casting of constants is done away with as well - I simply
don't see what the respective comment is concerned about in our
environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
architecture, sizeof(long long) >= 8). The comment is certainly relevant
in more general cases. Excess parentheses are dropped as well,
___swab32()'s local variable is renamed, and __arch__swab32()'s is
dropped as being redundant with ___swab32()'s.
The masking operation is also dropped from __fswab64().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Frediano Ziglio <frediano.ziglio@cloud.com>
In {big,little}_endian.h the changes are entirely mechanical, except for
dealing with casting away of const from pointers-to-const on lines
touched anyway.
In swab.h the casting of constants is done away with as well - I simply
don't see what the respective comment is concerned about in our
environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
architecture, sizeof(long long) >= 8). The comment is certainly relevant
in more general cases. Excess parentheses are dropped as well,
___swab16()'s local variable is renamed, and __arch__swab16()'s is
dropped as being redundant with ___swab16()'s.
With that no uses of the type remain, so it moves to linux-compat.h.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Xen uses master for QEMU_UPSTREAM_REVISION, and has done for other trees too
in the path. Apparently we've never specified a different branch, because the
git-clone rune only pulls in the master branch; it does not pull in diverging
branches. Fix this by stating which branch/tag is wanted.
$TAG is really a committish, and git-clone's -b/--branch takes a committish
too.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
We already have one migration case opencoded (feat.max_subleaf). A more
recent discovery is that we advertise x2APIC to guests without ensuring that
we provide max_leaf >= 0xb.
In general, any leaf known to Xen can be safely configured by the toolstack if
it doesn't violate other constraints.
Therefore, introduce guest_common_{max,default}_leaves() to generalise the
special case we currently have for feat.max_subleaf, in preparation to be able
to provide x2APIC topology in leaf 0xb even on older hardware.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Add Rule 11.2 and Rule 18.1 to the monitored set.
Tag Rule 7.3 as clean.
Tag Rule 11.2 and Rule 20.7 as clean only for arm.
Rule 2.2, Rule 9.5 and Directive 4.12 are not accepted: do not enable
them and do not tag them as clean. Same for D4.3.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Add x86_64 hardware test that creates a Xen Argo communication
connection between two PVH domains. In the test, dom0 creates a domU and
listens for messages sent by the domU through Argo.
To accomplish this, build Xen with CONFIG_ARGO=y and create a CI test job.
Update the xilinx x86_64 test script to support the new test, and add
"sync_console" to command line to avoid an issue with console messages
being lost.
Requested-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Victor Lira <victorm.lira@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Move code from efi-boot.h to a separate, new, reloc-trampoline.c file.
Reuse this new code, compiling it for 32bit as well, to replace assembly
code in head.S doing the same thing.
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Right now, the two functions which were really too complicated to write
in asm are compiled as 32bit PIC, linked to a blob and included
directly, using global asm() to arrange for them to have function semantics.
This is limiting and fragile; the use of data relocations will compile
fine but malfunction when used, creating hard-to-debug bugs.
Furthermore, we would like to increase the amount of C, to
deduplicate/unify Xen's boot logic, as well as making it easier to
follow. Therefore, rework how the 32bit objects are included.
Link all 32bit objects together first. This allows for sharing of logic
between translation units. Use differential linking and explicit
imports/exports to confirm that we only have the expected relocations,
and write the object back out as an assembly file so it can be linked
again as if it were 64bit, to integrate with the rest of Xen.
This allows for the use of external references (e.g. access to global
variables) with reasonable assurance of doing so safely.
No functional change.
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
discard_initial_images() only works because init_domheap_pages() with ps==pe
is a no-op.
In dom0_construct(), explaining the significance of setting the initrd length
to 0, and put an explicit check in discard_initial_images().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
We can't drop them fully yet, but we can limit their scope to almost nothing,
which serves the same purpose.
This removes the ability to accidentally reintroduce buggy uses of
__va(mbi->mods_addr).
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
The former is about to disappear.
In some cases, introduce a local struct boot_module pointer. Judgement on
where to do this, and on constness, is based on what creates least churn
overall.
No functional change.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Convert converting the call to bzimage_headroom(), as well as using
bi->mod[0]. It will be used externally by later changes.
No functional change.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
We're about to introduce alternative mapping functions, and passing NULL was
always a slightly weird way to express unmap. Make an explicit unmap
function, to avoid having two different valid ways of unmapping.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
This is imported from Linux, but the parameter being signed is dubious in the
first place and we're not plausibly going to gain a use for the functionality.
Linux has subsequently made it an optional parameter to avoid forcing callers
to pass a stack variable they don't care about using.
In the unlikely case that we gain a usecase, we can reintroduce it, but in the
meantime simplify the single caller.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The use of physical addresses in __start_xen() has proved to be fertile soure
of bugs.
The MB1/2 path stashes the MBI pointer in multiboot_ptr (a setup.c variable
even), then re-loads it immediately before calling __start_xen(). For this,
we can just drop the function parameter and read multiboot_ptr in the one
place where it's used.
The EFI path also passes this parameter into __start_xen(). Have the EFI path
set up multiboot_ptr too, and move the explanation of phyiscal-mode pointers.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
As detailed in commit 0fe607b2a1 ("x86/boot: Fix PVH boot during boot_info
transition period"), the use of __va(mbi->mods_addr) constitutes a
use-after-free on the PVH boot path.
This pattern has been in use since before PVH support was added. This has
most likely gone unnoticed because no-one's tried using a detached Flask
policy in a PVH VM before.
Plumb the boot_info pointer down, replacing module_map and mbi. Importantly,
bi->mods[].mod is a safe way to access the module list during PVH boot.
As this is the final non-bi use of mbi in __start_xen(), make the pointer
unusable once bi has been established, to prevent new uses creeping back in.
This is a stopgap until mbi can be fully removed.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
As detailed in commit 0fe607b2a1 ("x86/boot: Fix PVH boot during boot_info
transition period"), the use of __va(mbi->mods_addr) constitutes a
use-after-free on the PVH boot path.
This pattern has been in use since before PVH support was added. Inside a PVH
VM, it will go unnoticed as long as the microcode container parser doesn't
choke on the random data it finds.
The use within early_microcode_init() happens to be safe because it's prior to
move_xen(). microcode_init_cache() is after move_xen(), and therefore unsafe.
Plumb the boot_info pointer down, replacing module_map and mbi. Importantly,
bi->mods[].mod is a safe way to access the module list during PVH boot.
Note: microcode_scan_module() is still bogusly stashing a bootstrap_map()'d
pointer in ucode_blob.data, which constitutes a different
use-after-free, and only works in general because of a second bug. This
is unrelated to PVH, and needs untangling differently.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
To start transitioning consider_modules() over to struct boot_module, begin
with taking the array of struct boot_modules but use the temporary struct
element mod.
No functional change intended.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
While both src and dst are similar, src is mapped only accounting for src's
size, while dst is mapped based on the minimum of both. This means that in
some cases, an overly large mapping is requested for src.
Rework the sz calcuation to be symmetric, and leave an explanation of how
logic works.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Given that soffs is the offset into the 2M superpage,
start = (src - soffs) >> PAGE_SIFT
is a complicated expression for the frame address of the containing superpage.
Except, start is converted straight back to a byte address to use, so the
shifting is unnecessary too.
The only thing done with the mapped pointer is to have soffs added back on for
the memmove() call. bootstrap_map_addr() passes through the offset, so we can
pass src directly in and simplify the memmove() call too. For the end mapping
address, this simplifies to just src + sz too.
The same reasoning holds for dst and doffs.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
move_memory() is very complicated, and buggy. In order to fix the latter, we
have to address the former.
Given prior cleanup, bootstrap_map() is now implemented in terms of
bootstrap_map_addr(), meaning that it is counterproductive to plumb the
mapping through module_t.
Delete mod, and introduce two same-sized/named fields. At this point in boot,
neither fields have their named purpose, so indicate the purpose in comments.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Add missing break statements to address violations of MISRA C:2012
Rule 16.3 (An unconditional `break' statement shall terminate
every switch-clause).
Make explicit unreachability of a program point with
ASSERT_UNREACHABLE() and add defensive code.
No functional change.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
BUG_ON() itself already contains an unlikely() wrapping the bug condition.
No functional change.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
multiboot_fill_boot_info() taking the physical address of the multiboot_info
structure leads to a subtle use-after-free on the PVH path, with rather less
subtle fallout.
The pointers used by __start_xen(), mbi and mod, are either:
- MB: Directmap pointers into the trampoline, or
- PVH: Xen pointers into .initdata, or
- EFI: Directmap pointers into Xen.
Critically, these either remain valid across move_xen() (MB, PVH), or rely on
move_xen() being inhibited (EFI).
The conversion to multiboot_fill_boot_info(), taking only mbi_p, makes the PVH
path use directmap pointers into Xen, as well as move_xen() which invalidates
said pointers.
Switch multiboot_fill_boot_info() to consume the same virtual addresses that
__start_xen() currently uses. This keeps all the pointers valid for the
duration of __start_xen(), for all boot protocols.
It can be safely untangled once multiboot_fill_boot_info() takes a full copy
the multiboot info data, and __start_xen() has been moved over to using the
new boot_info consistently.
Right now, bi->{loader,cmdline,mods} are problematic. Nothing uses
bi->mods[], and nothing uses bi->cmdline after move_xen().
bi->loader is used after move_xen(), although only for cmdline_cook() of
dom0's cmdline, where it happens to be benign because PVH boot skips the
inspection of the bootloader name.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Right now, various blocks rewrite domU_config= as a whole, even though it is
largely the same.
* dom0pvh-hvm does nothing but change the domain type to hvm
* *-pci sets the domain type, clears vif=[], appends earlyprintk=xen to the
cmdline, and adds some PCI config.
Refactor this to be domU_type (defaults to pvh), domU_vif (defaults to
xenbr0), and domU_extra_config (defaults to empty) and use these variables to
build domU_config= once.
Of note, the default domU_config= now sets cmdline=, and extra= is intended
for inclusion via domU_extra_config as necessary.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
* List all the test_variants and summerise what's going on
* Use case rather than an if/else chain for $test_variant
* Adjust indentation inside the case block
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
It was trying to do too many things at once and there was no clear way of
defining what it was meant to do. This commit splits the function in two.
1. A function to return the FPU to power-on reset values.
2. A x87/SSE state loader (equivalent to the old function when it took
a data pointer).
The old function also had a concept of "default" values that the FPU
would be configured for in some cases but not others. This patch removes
that 3rd vague initial state and replaces it with power-on reset.
While doing this make sure the abridged control tag is consistent with the
manuals and starts as 0xFF
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
fpu_ctxt is either a pointer to the legacy x87/SSE save area (used by FXSAVE) or
a pointer aliased with xsave_area that points to its fpu_sse subfield. Such
subfield is at the base and is identical in size and layout to the legacy
buffer.
This patch merges the 2 pointers in the arch_vcpu into a single XSAVE area. In
the very rare case in which the host doesn't support XSAVE all we're doing is
wasting a tiny amount of memory and trading those for a lot more simplicity in
the code.
While at it, dedup the setup logic in vcpu_init_fpu() and integrate it
into xstate_alloc_save_area().
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This will introduce a new struct boot_module to provide a rich state
representation around modules provided by the boot loader. Support is for 64
boot modules, one held in reserve for Xen, and up to 63 can be provided by the
boot loader. The array of struct boot_modules will be accessible via a
reference held in struct boot_info.
A temporary `mod` parameter is included in struct boot_module to ease the
transition from using Multiboot v1 structures over to struct boot_module. Once
the transition is complete, the parameter will be dropped from the structure.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
pvh_init() sets up the mbi pointer, but leaves mbi_p at 0. This isn't
compatbile with multiboot_fill_boot_info() starting from the physical address,
in order to remove the use of the mbi pointer.
Fixes: 038826b61e ("x86/boot: move x86 boot module counting into a new boot_info struct")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
The logic would be more robust disabling SMAP based on its precense in CR4,
rather than SMAP's accociation with a synthetic feature.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Fedora 29 is long out of date. Move forward 5 years to Fedora 40.
Include all the usual improvements. Rework the container to be non-root, use
heredocs for legibility, and switch to the new naming scheme.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Javi Merino <javi.merino@cloud.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
On Fedora 40, the build fails with:
In file included from domain_getinfo_stubs_v1.c:10:
domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
/usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
275 | struct caml__roots_block *caml__frame = *caml_local_roots_ptr
| ^~~~~~~~~~~
domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
48 | CAMLparam0();
| ^~~~~~~~~~
cc1: all warnings being treated as errors
The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
#define CAMLparam0() \
struct caml__roots_block** caml_local_roots_ptr = \
(DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0, \
&CAML_LOCAL_ROOTS); \
struct caml__roots_block *caml__frame = *caml_local_roots_ptr
We can't modify the macro. But, it turns out there's a CAMLnoreturn macro
with the sole purpose of masking this warning.
Fixes: a6576011a4 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
Signed-off-by: Javi Merino <javi.merino@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Jan Beulich
Frediano Ziglio
Andrew Cooper
Federico Serafini
Victor Lira
Daniel P. Smith
Roger Pau Monne
Alejandro Vallejo
Andrei Cherechesu
Javi Merino