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>
The work to add the `ucode=nmi` cmdline option left a subtle corner case.
Both scan and an explicit index could be chosen, and we could really find both
a CPIO archive and a microcode file.
Worse, because the if/else chains for processing ucode_{blob,mod} are opposite
ways around in early_microcode_load() and microcode_init_cache(), we can
genuinely perform early microcode loading from the CPIO archive, then cache
from the explicit file.
Therefore, enforce that only one selection method can be active.
Rename ucode_{scan,mod_idx} to have an opt_ prefix. This is both for
consistency with the rest of Xen, and to guarantee that we've got all
instances of the variables covered in this change. Explain how they're use.
During cmdline/config parsing, always update both variables in pairs.
In early_microcode_load(), ASSERT() the invariant just in case. Use a local
variable for idx rather than operating on the static; we're the only consume
of the value.
Expand the index selection logic with comments and warnings to the user when
something went wrong.
Fixes: 5ed12565aa ("microcode: rendezvous CPUs in NMI handler and load ucode")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Currently, building with LLVM's LLD fails:
ld -melf_i386_fbsd --orphan-handling=error -N -T ...
ld: error: <internal>:(.symtab) is being placed in '.symtab'
ld: error: <internal>:(.shstrtab) is being placed in '.shstrtab'
ld: error: <internal>:(.strtab) is being placed in '.strtab'
gmake[11]: *** [arch/x86/boot/Makefile:69: arch/x86/boot/built-in-32.base.bin] Error 1
This is a consequence of --orphan-handling, and it appears that Binutils
doesn't diagnose some orphaned sections even explicitly asked to do so.
List the sections explicitly.
Fixes: aa9045e771 ('x86/boot: Rework how 32bit C is linked/included for early boot')
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
combine_two_binaries.py only understands GNU LD's format, and does
not work with LLVM's LLD.
Use nm command instead to get list of symbols; specifically
BSD format as it does not truncate symbols names like sysv one.
Fixes: aa9045e771 ('x86/boot: Rework how 32bit C is linked/included for early boot')
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The "base" and "offset" definition were inverted, "base" file should be the
files without offsets applied while "offset" should have the offsets applied.
Also update an old usage of "final" to "apply offset" to make more clear and
consistent (in former commit messages the "final" term was used instead of
"offset").
Fixes: aa9045e771 ('x86/boot: Rework how 32bit C is linked/included for early boot')
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
We've got a perfectly good vendor abstraction already for microcode. No need
for a second ad-hoc one in microcode_scan_module().
This is in preparation to use ucode_ops.cpio_path in multiple places.
These paths are only used during __init, so take the opportunity to move them
into __initconst.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Diff-wise, as early_microcode_update_cpu() is the larger function, this more
closely resembles "merge early_microcode_load() into it's single callee", but
the end result is the same.
At the same time, rename the len variable to size. This is for better
consistency with existing logic, and to reduce churn later.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
microcode_grab_module() and early_microcode_update_cpu() are logically one
task that passes state via static variables.
We intend to delete said static variables; start by breaking these functions
out of early_microcode_init().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
There's no need for microcode_init_cache() to be called exactly where it is in
__start_xen(). All that matters is it must be after xmalloc() is available
and before APs start up.
As a consequence, microcode_init_cache() runs a little later on boot now.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Marek Marczykowski-Górecki