Put jobs that are needed for (any) test earlier, so the tests can start
running in parallel to builds.
This commits splits only x86 build jobs into two sections (one on top
and one on bottom), but keep ARM build jobs in one section, as most of
them have some test connected and the few that do not are not worth
reducing readability of the file.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
It currently relies on xc_domain_getinfo() returning the next available
domain past "first_domid", which is a feature that will disappear in a
future patch.
Furthermore and while at it, make it so the hypercall tries to fetch information
about more than one domain per hypercall so we can (hopefully) get away with a
single hypercall in a typical system.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
It has 2 avoidable occurences
* Check whether a domain is valid, which can be done faster with
xc_domain_getinfo_single().
* Domain discovery, which can be done in a race-free way with the sysctl
interface through xc_domain_getinfolist().
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
It's a stricter version of xc_domain_getinfo() where the returned domid
always matches the requested domid or the error code shows an error instead.
A few patches ahead usages of xc_domain_getinfo() are removed until only
xc_domain_getinfo_single() and xc_domain_getinfolist() remain.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
xc_domain_getinfo() is slow and prone to races because N hypercalls are
needed to find information about N domains. xc_domain_getinfolist() finds
the same information in a single hypercall as long as a big enough buffer
is provided. Plus, xc_domain_getinfo() is disappearing on a future patch
so migrate the callers interested in more than 1 domain to the the *list()
version.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Add simple PCI passthrough test to both PV and HVM domU. It passes
through a network adapter (the only one in the system), gets an IP via
DHCP (first basic test) and then ping the gateway (second basic test).
Finally, if device is supposed to use MSI or MSI-X (as set in the
PCIDEV_INTR test variable), check if it's in use via /proc/interrupts.
On the current runner, the device in question is this:
03:00.0 Ethernet controller [0200]: Intel Corporation Ethernet Controller I225-V [8086:15f3] (rev 03)
Subsystem: Micro-Star International Co., Ltd. [MSI] Device [1462:7d25]
Flags: bus master, fast devsel, latency 0, IRQ 18
Memory at 50400000 (32-bit, non-prefetchable) [size=1M]
Memory at 50500000 (32-bit, non-prefetchable) [size=16K]
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
Capabilities: [70] MSI-X: Enable+ Count=5 Masked-
Capabilities: [a0] Express Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number ...
Capabilities: [1c0] Latency Tolerance Reporting
Capabilities: [1f0] Precision Time Measurement
Capabilities: [1e0] L1 PM Substates
Kernel driver in use: igc
Kernel modules: igc
With the current Xen version, it uses MSI-X under PV and MSI under HVM.
This patch moves domU config to a variable, to make it configurable on
per-test basis. Add also a few comments for visual separation of tests.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The login prompt is printed after all the startup (test) scripts, wait
for that instead of "passed" marker. And only then check if test passed.
Before this patch there was a race: "passed" marker could be already
printed, but the final check would fail because login prompt wasn't
there yet.
Also, modify etc/issue in domU rootfs to avoid confusing the one from
domU with the dom0's one. Use the dom0 one as test end marker.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
As the recent work to get PCI Passthrough testing working shows, putting
`/bin/false` as a ROM into guest context doesn't work so well.
For all ROM paths where we're skipping the build, use a slightly-plausible but
likely non-existent path instead.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Currently the script return code is 0 even if an exception is
found, because the return code is written only if the exception
object has the errorcode member.
Fix the issue returning the errorcode member in case it exists,
otherwise use a generic value different from 0.
Fixes: 02b26c02c7 ("xen/scripts: add cppcheck tool to the xen-analysis.py script")
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
rpm post-build-checks found a few code bugs in newlib, and marks them as
errors. Add another newlib patch and apply it during stubdom build.
[ 227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c: In function 'lookup_word':
[ 227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147:10: warning: implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
[ 227s] if (strcmp(ptr->word, word) == 0) return ptr;
[ 227s] ^
[ 460s] I: Program is using implicit definitions of special functions.
[ 460s] these functions need to use their correct prototypes to allow
[ 460s] the lightweight buffer overflow checking to work.
[ 460s] - Implicit memory/string functions need #include <string.h>.
[ 460s] - Implicit *printf functions need #include <stdio.h>.
[ 460s] - Implicit *printf functions need #include <stdio.h>.
[ 460s] - Implicit *read* functions need #include <unistd.h>.
[ 460s] - Implicit *recv* functions need #include <sys/socket.h>.
[ 460s] E: xen implicit-fortify-decl ../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
rpm post-build-checks found a few code bugs in newlib, and marks them as
errors. Add another newlib patch and apply it during stubdom build.
I: A function uses a 'return;' statement, but has actually a value
to return, like an integer ('return 42;') or similar.
W: xen voidreturn ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:117, 125, 146, 157, 330
I: Program is using implicit definitions of special functions.
these functions need to use their correct prototypes to allow
the lightweight buffer overflow checking to work.
- Implicit memory/string functions need #include <string.h>.
- Implicit *printf functions need #include <stdio.h>.
- Implicit *printf functions need #include <stdio.h>.
- Implicit *read* functions need #include <unistd.h>.
- Implicit *recv* functions need #include <sys/socket.h>.
E: xen implicit-fortify-decl ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:119
I: Program returns random data in a function
E: xen no-return-in-nonvoid-function ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:362
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Printing the integer isn't particularly informative. Switch to a
human-readable string when printing the device_kind in
libxl__get_hotplug_script_info().
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Print the rc when an error is found in device_backend_callback() so the
user can have some idea of why things went wrong.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Use a snapshot which includes commit
b0ded89e917b48b73097d3b8b88dfa3afb264ed0 ("[build] Disable dangling
pointer checking for GCC"), which fixes build with gcc12.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
It seems to build just fine with Alpine 3.12, and SeaBIOS is necessary
for a HVM test (that use the Alpine build).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Avoid memory fragmentation that leads to:
(XEN) common/memory.c:277:d0v10 Could not allocate order=9 extent: id=1 memflags=0xc0 (0 of 4)
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
As a result of 241702e064 the
mfn_valid() check in sh_unshadow_for_p2m_change() was lost. That
allows sh_remove_shadows() to be called with gfns that have no backing
page, causing an ASSERT to trigger in debug builds or dereferencing an
arbitrary pointer partially under guest control in non-debug builds:
RIP: e008:[<ffff82d0402dcf2c>] sh_remove_shadows+0x19f/0x722
RFLAGS: 0000000000010246 CONTEXT: hypervisor (d0v2)
[...]
Xen call trace:
[<ffff82d0402dcf2c>] R sh_remove_shadows+0x19f/0x722
[<ffff82d0402e28f4>] F arch/x86/mm/shadow/hvm.c#sh_unshadow_for_p2m_change+0xab/0x2b7
[<ffff82d040311931>] F arch/x86/mm/p2m-pt.c#write_p2m_entry+0x19b/0x4d3
[<ffff82d0403131b2>] F arch/x86/mm/p2m-pt.c#p2m_pt_set_entry+0x67b/0xa8e
[<ffff82d040302c92>] F p2m_set_entry+0xcc/0x149
[<ffff82d040305a50>] F unmap_mmio_regions+0x17b/0x2c9
[<ffff82d040241e5e>] F do_domctl+0x11f3/0x195e
[<ffff82d0402c7e10>] F hvm_hypercall+0x5b1/0xa2d
[<ffff82d0402adc72>] F vmx_vmexit_handler+0x130f/0x1cd5
[<ffff82d040203602>] F vmx_asm_vmexit_handler+0xf2/0x210
****************************************
Panic on CPU 1:
Assertion 'mfn_valid(gmfn)' failed at arch/x86/mm/shadow/common.c:2203
****************************************
Fix this by restoring the mfn_valid() check in
sh_unshadow_for_p2m_change(), unifying it with the rest of the checks
that are done at the start of the function.
This is XSA-430 / CVE-2022-42335
Fixes: 241702e064 ('x86/shadow: slightly consolidate sh_unshadow_for_p2m_change() (part II)')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.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>
Today it is not possible to analyse crash dumps of a system in
hypervisor mode when it had been booted via EFI, as the crash utility
doesn't understand the file format of xen.efi.
This can easily be solved by creating an ELF file from xen.efi via
objcopy. Using that file as name list for crash enables the user to
analyse the dump in hypervisor mode. Note that crash isn't happy with
a file containing no text and data, so using --only-keep-debug is not
an option.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.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>
Some firmware/devices are found to not reset MSI-X properly, leaving
MASKALL set. Jason reports on his machine MASKALL persists through a
warm reboot, but is cleared on cold boot. Xen relies on initial state
being MASKALL clear. Especially, pci_reset_msix_state() assumes if
MASKALL is set, it was Xen setting it due to msix->host_maskall or
msix->guest_maskall. Clearing just MASKALL is risky if ENABLE is set,
so clear them both.
Reported-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Jason Andryuk <jandryuk@gmail.com>
Switching TTBR while the MMU is on is not safe. Now that the identity
mapping will not clash with the rest of the memory layout, we can avoid
creating temporary page-tables every time a CPU is brought up.
The arm32 code will use a different approach. So this issue is for now
only resolved on arm64.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Tested-by: Henry Wang <Henry.Wang@arm.com>
At the moment, switch_ttbr() is switching the TTBR whilst the MMU is
still on.
Switching TTBR is like replacing existing mappings with new ones. So
we need to follow the break-before-make sequence.
In this case, it means the MMU needs to be switched off while the
TTBR is updated. In order to disable the MMU, we need to first
jump to an identity mapping.
Rename switch_ttbr() to switch_ttbr_id() and create an helper on
top to temporary map the identity mapping and call switch_ttbr()
via the identity address.
switch_ttbr_id() is now reworked to temporarily turn off the MMU
before updating the TTBR.
We also need to make sure the helper switch_ttbr() is part of the
identity mapping. So move _end_boot past it.
The arm32 code will use a different approach. So this issue is for now
only resolved on arm64.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Tested-by: Henry Wang <Henry.Wang@arm.com>
In follow-up patches we will need to have part of Xen identity mapped in
order to safely switch the TTBR.
On some platform, the identity mapping may have to start at 0. If we always
keep the identity region mapped, NULL pointer dereference would lead to
access to valid mapping.
It would be possible to relocate Xen to avoid clashing with address 0.
However the identity mapping is only meant to be used in very limited
places. Therefore it would be better to keep the identity region invalid
for most of the time.
Two new external helpers are introduced:
- arch_setup_page_tables() will setup the page-tables so it is
easy to create the mapping afterwards.
- update_identity_mapping() will create/remove the identity mapping
Signed-off-by: Julien Grall <jgrall@amazon.com>
Tested-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Xen is currently not fully compliant with the Arm Arm because it will
switch the TTBR with the MMU on.
In order to be compliant, we need to disable the MMU before
switching the TTBR. The implication is the page-tables should
contain an identity mapping of the code switching the TTBR.
In most of the case we expect Xen to be loaded in low memory. I am aware
of one platform (i.e AMD Seattle) where the memory start above 512GB.
To give us some slack, consider that Xen may be loaded in the first 2TB
of the physical address space.
The memory layout is reshuffled to keep the first four slots of the zeroeth
level free. All the regions currently in L0 slot 0 will not be part of
slot 4 (2TB). This requires a slight tweak of the boot code because
XEN_VIRT_START (2TB + 2MB) cannot be used as an immediate.
This reshuffle will make trivial to create a 1:1 mapping when Xen is
loaded below 2TB.
Lastly, take the opportunity to check at compile time if any of the
regions may overlap with the reserved area for identity mapping.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Tested-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
At the moment, the temporary mapping is only used when the virtual
runtime region of Xen is clashing with the physical region.
In follow-up patches, we will rework how secondary CPU bring-up works
and it will be convenient to use the fixmap area for accessing
the root page-table (it is per-cpu).
Rework the code to use temporary mapping when the Xen physical address
is not overlapping with the temporary mapping.
This also has the advantage to simplify the logic to identity map
Xen.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Henry Wang <Henry.Wang@arm.com>
Tested-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
On 32 bit systems with 64 bit time_t (hello, Y2038 problem),
the following error occurs otherwise:
| xenstored_control.c: In function 'lu_reject_reason':
| xenstored_control.c:646:70: error: format '%ld' expects argument of type 'long int', but argument 5 has type 'time_t' {aka 'long long int'} [-Werror=format=]
Signed-off-by: Alexander Kanavin <alex@linutronix.de>
Reviewed-by: Juergen Gross <jgross@suse.com>
When building the hypervisor with -Og and GCC 11/12, we encounter the
following error:
arch/arm/domain_build.c: In function ‘make_cpus_node’:
arch/arm/domain_build.c:2040:12: error: ‘clock_valid’ may be used uninitialized [-Werror=maybe-uninitialized]
2040 | if ( clock_valid )
| ^
arch/arm/domain_build.c:1947:10: note: ‘clock_valid’ was declared here
1947 | bool clock_valid;
| ^~~~~~~~~~~
cc1: all warnings being treated as errors
Fix it by initializing the variable.
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
When building the hypervisor with -Og, we run into a __bad_cmpxchg link error:
aarch64-none-linux-gnu-ld: prelink.o: in function `__int_cmpxchg':
.../xen/./arch/arm/include/asm/arm64/cmpxchg.h:117: undefined reference to `__bad_cmpxchg'
aarch64-none-linux-gnu-ld: .../xen/./arch/arm/include/asm/arm64/cmpxchg.h:117: undefined reference to `__bad_cmpxchg'
aarch64-none-linux-gnu-ld: ./.xen-syms.0: hidden symbol `__bad_cmpxchg' isn't defined
aarch64-none-linux-gnu-ld: final link failed: bad value
This is due to the function __guest_cmpxchg not being inlined in the -Og build
with gcc 12. Fix this by marking __guest_cmpxchg always_inline.
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Henry Wang <Henry.Wang@arm.com>
The prior check has && vs || mixups, making it tautologically false and thus
providing no safety at all. There are boundary errors too.
First start with a comment describing how the .altinstructions and
.altinstr_replacement sections interact, and perform suitable cross-checking.
Second, rewrite the alt_instr loop entirely from scratch. Origin sites have
non-zero size, and must be fully contained within the livepatches .text
section(s). Any non-zero sized replacements must be fully contained within
the .altinstr_replacement section.
Fixes: f8a10174e8 ("xsplice: Add support for alternatives")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Alternatives auditing for livepatches is currently broken. To fix it, the
livepatch code needs to inspect more fields of alt_instr.
Rename ARM's fields to match x86's, because:
* ARM already exposes alt_offset under the repl name via ALT_REPL_PTR().
* "alt" is ambiguous in a structure entirely about alternatives already.
* "repl", being the same width as orig leads to slightly neater code.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
It is rude to hide width formatting inside a PRI* macro, doubly so when it's
only in one bitness of the macro.
However its fully buggy when all the users use %#"PRI because then it expands
to the common trap of %#08x which does not do what the author intends.
Switch the 32bit ELF PRI formatters to use plain integer PRI's, just like on
the 64bit side already. No practical change.
Fixes: 7597fabca7 ("livepatch: Include sizes when an mismatch occurs")
Fixes: 380b229634 ("xsplice: Implement payload loading")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Note in the changelog that the purpose of
gnttab_max_{maptrack_,}frames command line options has been changed.
Fixes: b2ea81d2b9 ('xen/grants: repurpose command line max options')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Henry Wang <Henry.Wang@arm.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>
Right now, trying to apply a livepatch on any system with CET shstk (AMD Zen3
or later, Intel Tiger Lake or Sapphire Rapids and later) fails as follows:
(XEN) livepatch: lp: Verifying enabled expectations for all functions
(XEN) common/livepatch.c:1591: livepatch: lp: timeout is 30000000ns
(XEN) common/livepatch.c:1703: livepatch: lp: CPU28 - IPIing the other 127 CPUs
(XEN) livepatch: lp: Applying 1 functions
(XEN) hi_func: Hi! (called 1 times)
(XEN) Hook executing.
(XEN) Assertion 'local_irq_is_enabled() || cpumask_subset(mask, cpumask_of(cpu))' failed at arch/x86/smp.c:265
(XEN) *** DOUBLE FAULT ***
<many double faults>
The assertion failure is from a global (system wide) TLB flush initiated by
modify_xen_mappings(). I'm not entirely sure when this broke, and I'm not
sure exactly what causes the #DF's, but it doesn't really matter either
because they highlight a latent bug that I'd overlooked with the CET-SS vs
patching work the first place.
While we're careful to arrange for the patching CPU to avoid encountering
non-shstk memory with transient shstk perms, other CPUs can pick these
mappings up too if they need to re-walk for uarch reasons.
Another bug is that for livepatching, we only disable CET if shadow stacks are
in use. Running on Intel CET systems when Xen is only using CET-IBT will
crash in arch_livepatch_quiesce() when trying to clear CR0.WP with CR4.CET
still active.
Also, we never went and cleared the dirty bits on .rodata. This would
matter (for the same reason it matters on .text - it becomes a valid target
for WRSS), but we never actually patch .rodata anyway.
Therefore rework how we do patching for both alternatives and livepatches.
Introduce modify_xen_mappings_lite() with a purpose similar to
modify_xen_mappings(), but stripped down to the bare minimum as it's used in
weird contexts. Leave all complexity to the caller to handle.
Instead of patching by clearing CR0.WP (and having to jump through some
fragile hoops to disable CET in order to do this), just transiently relax the
permissions on .text via l2_identmap[].
Note that neither alternatives nor livepatching edit .rodata, so we don't need
to relax those permissions at this juncture.
The perms are relaxed globally, but this is safe enough. Alternatives run
before we boot APs, and Livepatching runs in a quiesced state where the other
CPUs are not doing anything interesting.
This approach is far more robust.
Fixes: 48cdc15a42 ("x86/alternatives: Clear CR4.CET when clearing CR0.WP")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Update the help text of the CONFIG_DEBUG_INFO option to be a little
bit more specific.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
In order to support hypervisor analysis of crash dumps, xen-syms needs
to contain debug_info. It should be allowed to configure the hypervisor
to be built with CONFIG_DEBUG_INFO in non-debug builds without having
to enable EXPERT.
Using a rather oldish gcc (7.5) it was verified that code generation
doesn't really differ between CONFIG_DEBUG_INFO on or off without
CONFIG_DEBUG being set (only observed differences were slightly
different symbol addresses, verified via "objdump -d", resulting from
the different config.gz in the binary). The old gcc version selection
was based on the assumption, that newer gcc won't regress in this
regard.
So move CONFIG_DEBUG_INFO out of the section guarded by EXPERT.
It should be mentioned that there have been reports that the linking
of the xen.efi might take considerably longer with CONFIG_DEBUG_INFO
selected when using newer binutils.
Signed-off-by: Juergen Gross <jgross@suse.com>
Tested-by: Dietmar Hahn <dietmar.hahn@fujitsu.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The GOT sections usage should be avoided in the hypervisor
so to catch such use cases earlier when GOT things are
produced the patch introduces .got and .got.plt sections
and adds asserts that they're empty.
The sections won't be created until they remain
empty otherwise the asserts would cause early failure.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
The patch is needed to keep all address of cpu0_boot_stack
PC-relative.
Pseudoinstruction 'la' can be transformed to 'auipc/addi' or
'auipc/l{w|d}'. It depends on the .option directive: nopic and pic
or compiler flags.
Right now, 'la' transforms to 'auipc/l{w|d}', which in case of
cpu0_boot_stack[] will lead to the usage of _GLOBAL_OFFSET_TABLE_
where all addresses will be without counting that it might happen
that linker address != load address ( so addresses inside got
sections will be relative to linker time ).
It happens becuase the compiler from riscv64 docker compiled with
--enable-default-pie:
[user@49295ae49cbe build]$ riscv64-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=riscv64-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/riscv64-linux-gnu/12.2.0/lto-wrapper
Target: riscv64-linux-gnu
Configured with: /build/riscv64-linux-gnu-gcc/src/gcc-12.2.0/configure
--prefix=/usr --program-prefix=riscv64-linux-gnu- --with-local-
prefix=/usr/riscv64-linux-gnu --with-sysroot=/usr/riscv64-linux-gnu --
with-build-sysroot=/usr/riscv64-linux-gnu --libdir=/usr/lib --
libexecdir=/usr/lib --target=riscv64-linux-gnu --host=x86_64-pc-linux-
gnu --build=x86_64-pc-linux-gnu --with-system-zlib --with-isl --with-
linker-hash-style=gnu --disable-nls --disable-libunwind-exceptions --
disable-libstdcxx-pch --disable-libssp --disable-multilib --disable-
werror --enable-languages=c,c++ --enable-shared --enable-threads=posix
--enable-__cxa_atexit --enable-clocale=gnu --enable-gnu-unique-object -
-enable-linker-build-id --enable-lto --enable-plugin --enable-install-
libiberty --enable-gnu-indirect-function --enable-default-pie --enable-
checking=release
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.0 (GCC)
Looking at gcc spec file for the RISC-V architecture:
[user@49295ae49cbe build]$ riscv64-linux-gnu-gcc -dumpspecs | grep -i
pic
--traditional-format %(subtarget_asm_debugging_spec) %{fno-pie|fno-
PIE|fno-pic|fno-PIC:;:-fpic} %{march=*} %{mabi=*} %{mno-relax} %{mbig-
endian} %{mlittle-endian} %(subtarget_asm_spec)%{misa-spec=*}
which means that -fpic is enabled if none of the following options are
present on the command line:
-fno-pie
-fno-PIE
-fno-pic
-fno-PIC
That's the reasons why 'la' is transformed to 'aupic/l{w|d} GOT' and
not be dependent on the toolchain used.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
>From vpl011_rx_char_xen(), we call vpl011_data_avail() that handles both
RX and TX state. Because we are passing 0 as out_fifo_level and
SBSA_UART_FIFO_SIZE as out_size, we end up calling a function
vpl011_update_tx_fifo_status() which performs TXI bit handling
depending on the FIFO trigger level. This does not make sense when backend
is in Xen, as we maintain a single TX state where data can always be
written and as such there is no TX FIFO handling. Furthermore, this
function assumes that the backend is in domain by making use of struct
xencons_interface unconditionally. Fix it by calling this function only
when backend is in domain. Also add an assert for sanity.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Tested-by: Henry Wang <Henry.Wang@arm.com>
When backend is in Xen, the handling of data written to DR register is a
bit special because we want to tell guest that we are always ready for new
data to be written (i.e. no real FIFO, TXFF/BUSY never set and TXI always
set). This conflicts with the current handling of TXFE bit, which we
always clear and never set on a write path (we happen to set it when we
receive char from serial input due to use of vpl011_data_avail() but this
might never be called). This can lead to issues if a guest driver makes
use of TXFE bit to check for TX transmission completion (such guest could
then wait endlessly). Fix it by keeping TXFE always set to match the
current emulation logic.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Henry Wang <Henry.Wang@arm.com>
Tested-by: Henry Wang <Henry.Wang@arm.com>
In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment
stating that the guest is expected to read the DR register only if the
TXFE bit of FR register is not set. This is obviously logically wrong and
it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read).
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Henry Wang <Henry.Wang@arm.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>
Marek Marczykowski-Górecki