The pseudo keyword fallthrough shall be used to make explicit the
fallthrough intention at the end of a case statement (doing this
using comments is deprecated).
A definition of such pseudo keyword is already present in the
Xen build. This auxiliary definition makes it available also for
for test and fuzzing harness without iterfearing with the one
that the Xen build has.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
While result values and other status flags are unaffected as long as we
can ignore the case of registers having their upper 32 bits non-zero
outside of 64-bit mode, EFLAGS.SF may obtain a wrong value when we
mistakenly re-execute the original insn with VEX.W set.
Note that guest the memory access, if any, is correctly carried out as
32-bit regardless of VEX.W. The emulator-local memory operand will be
accessed as a 64-bit quantity, but it is pre-initialised to zero so no
internal state can leak.
Fixes: 771daacd19 ("x86emul: support BMI1 insns")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Apparently I blindly copied the constants from the BEXTR case, where SF
indeed wants leaving out. For BLSI, BLSMSK, BLSR, and BZHI SF is
defined, and hence wants checking. This is noticable in particular for
BLSR, where with the input we use SF will be set in the result (and
hence is being switched to be clear on input).
Convert to using named constants we have available, while omitting DF,
TF, as well as the MBZ bits 3 and 5 from the masking values in the
checks of the produced output. For BZHI also set SF on input, expecting
it to transition to clear.
Fixes: 771daacd19 ("x86emul: support BMI1 insns")
Fixes: 8e20924de1 ("x86emul: support BMI2 insns")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The work to address Rule 5.3 introduced violations of Rule 8.3 by failing to
rename the parameters in the declaration.
Fixes: b5fd405aa3 ("x86/e820 address violations of MISRA C:2012 Rule 5.3")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
This resolves 4 Misra violations of Rule 8.4 caused by the function
definitions not being able to see their declarations.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
This function is overloaded, creating complexity; 3 of 4 callers already only
want it for it's "applicable to this CPU or not" answer, and handle revision
calculations separately.
Change it to be microcode_fits_cpu(), returning a simple boolean.
Notably, this removes a path where cpu_request_microcode() inspects
currently-loaded microcode revision, just to discard the answer.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This function is overloaded, creating complexity; 3 of 4 callers already only
want it for it's "applicable to this CPU or not" answer, and handle revision
calculations separately.
Change it to be microcode_fits_cpu(), returning a simple boolean. The
checking of the equiv table can be simplified substantially too; A mapping
will only be inserted if it's correct for the CPU, so any nonzero equiv.sig
suffices to know that equiv.id is correct.
Drop compare_header() too, which is simiarly overloaded, and use
compare_revisions() directly.
Notably, this removes a path where cpu_request_microcode() inspects
currently-loaded microcode revision, just to discard the answer.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Fold microcode_update_cpu() into its single remaining caller. Explain why we
bother grabbing the microcode revision even if we can't load microcode.
This avoids a double collect_cpu_info() call on each AP.
Furthermore, delete the -EIO path.
A hard error updating a single CPU's microcode on AP bringup or S3 resume is
definitely bad (needing special investigation), but freeing the cache is about
the worst possible action we can take in response; it prevents subsequent APs
from taking an update they might have accepted.
While we expect a homogeneous system with respect to microcode applicability,
this doesn't mean that all cores behave identically when given the same blob.
e.g. one failure mode seen in practice is a checksum failure caused by a bad
SRAM cell.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
There are two callers of microcode_update_cpu(), and because one passes NULL
and one doesn't, there are effectively two disjoint pieces of logic wrapped in
a single function.
early_microcode_load()'s use skips all the microcode_cache handling, and is
just a simple patch application.
Use ucode_ops.apply_microcode() directly, and remove the non-cache path from
microcode_update_cpu(). This skips a redundant collect_cpu_info()
call (performed in early_microcode_init(), marginally earlier), and avoids
holding microcode_mutex when we're not interacting with microcode_cache at
all.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Not the whole page, which may contain other registers too. The XHCI
specification describes DbC as designed to be controlled by a different
driver, but does not mandate placing registers on a separate page. In fact
on Tiger Lake and newer (at least), this page do contain other registers
that Linux tries to use. And with share=yes, a domU would use them too.
Without this patch, PV dom0 would fail to initialize the controller,
while HVM would be killed on EPT violation.
With `share=yes`, this patch gives domU more access to the emulator
(although a HVM with any emulated device already has plenty of it). This
configuration is already documented as unsafe with untrusted guests and
not security supported.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In some cases, only few registers on a page needs to be write-protected.
Examples include USB3 console (64 bytes worth of registers) or MSI-X's
PBA table (which doesn't need to span the whole table either), although
in the latter case the spec forbids placing other registers on the same
page. Current API allows only marking whole pages pages read-only,
which sometimes may cover other registers that guest may need to
write into.
Currently, when a guest tries to write to an MMIO page on the
mmio_ro_ranges, it's either immediately crashed on EPT violation - if
that's HVM, or if PV, it gets #PF. In case of Linux PV, if access was
from userspace (like, /dev/mem), it will try to fixup by updating page
tables (that Xen again will force to read-only) and will hit that #PF
again (looping endlessly). Both behaviors are undesirable if guest could
actually be allowed the write.
Introduce an API that allows marking part of a page read-only. Since
sub-page permissions are not a thing in page tables (they are in EPT,
but not granular enough), do this via emulation (or simply page fault
handler for PV) that handles writes that are supposed to be allowed.
The new subpage_mmio_ro_add() takes a start physical address and the
region size in bytes. Both start address and the size need to be 8-byte
aligned, as a practical simplification (allows using smaller bitmask,
and a smaller granularity isn't really necessary right now).
It will internally add relevant pages to mmio_ro_ranges, but if either
start or end address is not page-aligned, it additionally adds that page
to a list for sub-page R/O handling. The list holds a bitmask which
qwords are supposed to be read-only and an address where page is mapped
for write emulation - this mapping is done only on the first access. A
plain list is used instead of more efficient structure, because there
isn't supposed to be many pages needing this precise r/o control.
The mechanism this API is plugged in is slightly different for PV and
HVM. For both paths, it's plugged into mmio_ro_emulated_write(). For PV,
it's already called for #PF on read-only MMIO page. For HVM however, EPT
violation on p2m_mmio_direct page results in a direct domain_crash() for
non hardware domains. To reach mmio_ro_emulated_write(), change how
write violations for p2m_mmio_direct are handled - specifically, check
if they relate to such partially protected page via
subpage_mmio_write_accept() and if so, call hvm_emulate_one_mmio() for
them too. This decodes what guest is trying write and finally calls
mmio_ro_emulated_write(). The EPT write violation is detected as
npfec.write_access and npfec.present both being true (similar to other
places), which may cover some other (future?) cases - if that happens,
emulator might get involved unnecessarily, but since it's limited to
pages marked with subpage_mmio_ro_add() only, the impact is minimal.
Both of those paths need an MFN to which guest tried to write (to check
which part of the page is supposed to be read-only, and where
the page is mapped for writes). This information currently isn't
available directly in mmio_ro_emulated_write(), but in both cases it is
already resolved somewhere higher in the call tree. Pass it down to
mmio_ro_emulated_write() via new mmio_ro_emulate_ctxt.mfn field.
This may give a bit more access to the instruction emulator to HVM
guests (the change in hvm_hap_nested_page_fault()), but only for pages
explicitly marked with subpage_mmio_ro_add() - so, if the guest has a
passed through a device partially used by Xen.
As of the next patch, it applies only configuration explicitly
documented as not security supported.
The subpage_mmio_ro_add() function cannot be called with overlapping
ranges, and on pages already added to mmio_ro_ranges separately.
Successful calls would result in correct handling, but error paths may
result in incorrect state (like pages removed from mmio_ro_ranges too
early). Debug build has asserts for relevant cases.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
... to match its definition parameter-name-wise, to please Misra C:2012
Rule 8.3.
Fixes: 9102fcd957 ("mm: introduce xvmalloc() et al and use for grant table allocations")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
xl/libxl memory is leaking into a PVH guest through uninitialized
portions of the ACPI tables.
Use libxl_zalloc() to obtain zero-ed memory to avoid this issue.
This is XSA-464 / CVE-2024-45819.
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
Fixes: 14c0d328da ("libxl/acpi: Build ACPI tables for HVMlite guests")
Reviewed-by: Jan Beulich <jbeulich@suse.com>
bogus_real_magic code uses fs segment so it should be initialised.
Fixes: d8c8fef090 ("Provide basic Xen PM infrastructure")
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
As now documented, this variable holds a page aligned value less than 1M.
However, head.S fills it using 4-byte stores, and reloc_trampoline() is
compiled for both 32bit and 64bit, where unsigned long is a different size.
This happens to work because of the range of the value, but switch to uint32_t
to make it explicit.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
bogus_saved_magic() is assembled in a .code64 section but invoked in 32bit
mode. Moving it causes a real encoding difference.
Before:
66 c7 04 25 14 80 0b 00 53 0e movw $0xe53,0xb8014(,%eiz,1)
After:
66 c7 05 14 80 0b 00 53 0e movw $0xe53,0xb8014
The difference happens to be benign, but move the logic back into a .code32
for sanity sake. Annotate it with ELF metadata while doing so.
Fixes: d8c8fef090 ("Provide basic Xen PM infrastructure")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This header is included in exactly one location. Fold it into mpspec.h
With this done, mach-default/ is empty, so remove the include path.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
It has a single function, and a single user. This is unlikely to change
moving forwards so fold it into mpparse.c.
Update it to use an explicit uint16_t cast, rather than assuming the width of
short.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
irq_vectors.h is included by with multiple paths. Move it to be a regular
header instead.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
All useful content has been moved elsewhere.
enable_apic_mode() and multi_timer_check() are empty stubs. Remove their sole
callers and drop them.
apicid_to_node() and bios_cpu_apicid[] are entirely unused.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
It's an unnecessary wrapper, and longer than the operation it wraps.
It's also the only reason that mpparse.c includes mach_apic.h, other than for
transitive dependencies.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
It's an unnecessary wrapper.
It's also the only reason that smpboot.c includes mach_apic.h, other than for
transitive dependencies.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
It's an unnecessary wrapper, and is longer than the operation it wraps.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
... as the implementations are in genapic/probe.c
This covers the only functions that both setup.c and boot.c were including
mach_apic.h for, although setup.c was depending on io_apic.h transitively too.
The happens to address two MISRA Rule 8.4 violations, as probe.c couldn't
previously see the declarations.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
A number of files don't need mach_apic.h at all, or only need transitive
dependences. Drop the includes.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Neither iommu.c nor quirks.c use any functionality. iommu.c only uses it to
transitively include apic.h and io_apic.h, while quirks.c is only depending on
the ACLINUX wrapping of strtoul() which we spell simple_strtoul() everywhere
else in Xen.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
GNU assembly that supports such feature will unconditionally add a
.note.gnu.property section to object files. The content of that section can
change depending on the generated instructions. The current logic in
livepatch-build-tools doesn't know how to deal with such section changing
as a result of applying a patch and rebuilding.
Since .note.gnu.property is not consumed by the Xen build, suppress its
addition when livepatch support is enabled.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Work to rebase the Trenchboot patch series has encountered:
In file included from ./arch/x86/include/asm/tpm.h:4,
from arch/x86/boot/../tpm.c:23:
./arch/x86/include/asm/bootinfo.h:88:35: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'next_boot_module_index'
88 | static inline unsigned int __init next_boot_module_index(
|
Fix this by including the necessary header.
Fixes: 74af2d9827 ("x86/boot: eliminate module_map")
Reported-by: Krystian Hebel <krystian.hebel@3mdeb.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
The purpose of struct boot_module is to encapsulate the state of boot module as
it is processed by Xen. Locating boot module state struct boot_module reduces
the number of global variables as well as the number of state variables that
must be passed around. It also lays the groundwork for hyperlaunch mult-domain
construction, where multiple instances of state variables like headroom will be
needed.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Xen tracks the total span of physical memory space of each NUMA node,
but the PFN span includes large MMIO holes in e.g. the first NUMA node.
Thus, the span is not the same as the amount of usable RAM of a node.
Xen does not need it, but NUMA node memory amount can be helpful for
management tools and HW information tools like hwloc/lstopo with its
Xen backend for Dom0: https://github.com/xenserver-next/hwloc/
Introduce node_present_pages to node_data[]:
On boot, set the count of usable PFNs and update it on memory_add().
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
After edb48e76458b("x86/fpu: Combine fpu_ctxt and xsave_area in arch_vcpu"),
v->arch.xsave_area is always present and we can just remove these asserts.
Fixes: edb48e76458b("x86/fpu: Combine fpu_ctxt and xsave_area in arch_vcpu")
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Gating the main part of what do_test() does is wrong: As it stands, the
"memory" forms of BNDC{L,N,U} weren't tested because of this mistake.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Rework the container to be non-root, use heredocs for readability, and
use apt-get --no-install-recommends to keep the size down. Rename the
job to x86_32, to be consistent with XEN_TARGET_ARCH and the
naming scheme of all the other CI jobs:
${VERSION}-${ARCH}-${BUILD_NAME}
Remove build dependencies for building QEMU. The absence of ninja/meson means
that the container hasn't been able to build QEMU in years.
Remove build dependencies for the documentation as we don't have to
build it for every single arch.
This reduces the size of the container from 2.22GB to 1.32Gb.
Signed-off-by: Javi Merino <javi.merino@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Rework the container to use heredocs for readability, and use
apt-get --no-install-recommends to keep the size down.
This reduces the size of the (uncompressed) container from 3.44GB to
1.97GB.
The container is left running the builds and tests as root. A
subsequent patch will make the necessary changes to the test scripts
to allow test execution as a non-root user.
Signed-off-by: Javi Merino <javi.merino@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
apt-get update refreshes the package lists. -y doesn't do anything
here. It is needed for "apt-get install" or "apt-get upgrade" but not
for apt-get update. Drop it.
Signed-off-by: Javi Merino <javi.merino@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The existing startup code employs various ad-hoc state tracking about certain
boot module types by each area of the code. A boot module flags bitfield is
added to enable tracking these different states. The first state to be
transition by this commit is module relocation.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
With all boot modules now labeled by type, it is no longer necessary to
track whether a boot module was identified via the module_map bitmap.
Introduce a set of helpers to search the list of boot modules based on type and
the reference type, pointer or array index, desired. Then drop all uses of
setting a bit in module_map and replace its use for looping with the helpers.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
This commit introduces module types for the types of boot modules that may be
passed to Xen. These include xen, kernel, ramdisk, microcode, and xsm policy.
This reduces the need for hard coded order assumptions and global variables to
be used by consumers of boot modules, such as domain construction.
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The data pointer is known good, so the -ENOMEM path is unnecessary. However,
if we find no patch, something's wrong seeing as early_microcode_init()
indicated it was happy.
We are the entity initialising the cache, so we don't need the complexity of
using microcode_update_cache(). Just assert the cache is empty, and populate
it.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Both are used to pass information from early_microcode_load() to
microcode_init_cache(), and both constitute use-after-free's in certain cases.
Later still in microcode_init() is a failed attempt to "free" this
information, long after the damage has been done.
* ucode_mod is a copy of the module_t identified by `ucode=$n`. It is a copy
of the physical pointer from prior to Xen relocating the modules.
microcode_init_cache() might happen to find the data still intact in it's
old location, but it might be an arbitrary part of some other module.
* ucode_blob is a stashed virtual pointer to a bootstrap_map() which becomes
invalid the moment control returns to __start_xen(), where other logic is
free to to make temporary mappings. This was even noticed and
microcode_init_cache() adjusted to "fix" the stashed pointers.
The information which should be passed between these two functions is which
module to look in. Introduce early_mod_idx for this purpose. opt_scan can be
reused to distinguish between CPIO archives and raw containers.
Notably this means microcode_init_cache() doesn't need to scan all modules any
more; we know exactly which one to look in. However, we do re-parse the CPIO
header for simplicity.
Furthermore, microcode_init_cache(), being a presmp_initcall does not need to
use bootstrap_map() to access modules; it can use mfn_to_virt() directly,
which avoids needing to funnel exit paths through bootstrap_unmap().
Fold microcode_scan_module() into what is now it's single caller. It operates
on the function-wide idx/data/size state which allows for a unified "found"
path irrespective of module selection method.
Delete microcode_init() entirely. It was never legitimate logic.
This resolves all issues to do with holding pointers (physical or virtual)
across returning to __start_xen().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
If bootstrap_map() has provided a mapping, we must free it when done. Failing
to do so may cause a spurious failure for unrelated logic later.
Inserting a bootstrap_unmap() here does not break the use of ucode_{blob,mod}
any more than they already are.
Add a printk noting when we didn't find a microcode patch. It's at debug
level, because this is the expected case on AMD Client systems, and SDPs, but
for people trying to figure out why microcode loading isn't work, it might be
helpful.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Federico Serafini