... and into their parent, enter_state(). This change addresses the concern
about misleading function semantics, as the scheduler operations are not
directly related to the domain pausing/resuming implied by the freeze/thaw
naming.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
... and use them from their installed location.
The full reclusive copy of tools/tests brings in all build and intermediate
artefacts. e.g. for test-tsx alone:
./tests/tsx
./tests/tsx/.test-tsx.o.d
./tests/tsx/test-tsx.o
./tests/tsx/.gitignore
./tests/tsx/test-tsx
./tests/tsx/Makefile
./tests/tsx/test-tsx.c
duplicating the test binary which is also in ./usr/lib/xen/tests
Rewrite run-tools-tests to run tests from their installed
location (/usr/lib/xen/tests in alpine), which effectively removes the outer
loop over $dir.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Unlike the other tests, this is not standalone. It requires poking at a live
system, making it unweildly to use.
It also hasn't been touched in 7 years, despite changes in libraries and
kernel devices using the deprivilege infrastructure.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
An attempt to read access the GICD_ICPENDR<n> register (where n > 0)
which should be RAZ (as not supported) causes the guest data abort
due to incorrect end offset (GICD_ICPENDR) in the case range.
Fix that by using the proper end offset (GICD_ICPENDRN).
Fixes: a2b83f95bf ("xen/arm: vgic: Properly emulate the full register")
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
SGI and PPI descriptors are banked and stored in the per-CPU local_irq_desc
array, so not all elements of the global irq_desc array are used. This is
already accounted for in the descriptor lookup logic inside __irq_to_desc:
return &irq_desc[irq - NR_LOCAL_IRQS];
Therefore, the size of the irq_desc array can be reduced by NR_LOCAL_IRQS,
saving (NR_LOCAL_IRQS * L1_CACHE_BYTES) bytes of memory.
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Code in domain-build.c and dom0less-build.c was migrated from init-only
files. Thus, they contain only __init functions. Enforce this at build
time.
Fixes: ad03faa942 ("xen/common: dom0less: make some parts of Arm's CONFIG_DOM0LESS common")
Fixes: d07b7369aa ("xen/common: dom0less: introduce common domain-build.c")
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
All functions in dom0less-build.c should be __init.
Fixes: 2705f1adb9 ("xen: introduce Kconfig ARCH_PAGING_MEMPOOL")
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
After some recent change the order of MTRR vs PCI initialization is
inverted. MTRR will get initialization ahead of PCI scanning and sizing of
MMIO regions. As a result when setting up MTRRs the MMIO window below 4GB
will always have the same size, and there will be no window above 4GB.
This results in malformed and incomplete MTRRs being setup.
Fix by making sure PCI is initialized ahead of MTRR, also add a comment to
notice the ordering dependency.
Fixes: 2c3dffbaa3 ('tools/hvmloader: Replace LAPIC_ID() with cpu_to_apicid[]')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthoine Bourgeois <anthoine.bourgeois@gmail.com>
x86_merge_dr6() is not a no-op when 0 is passed in; it will discard the
previously latched breakpoint bits.
The combination of do_debug()'s manual call to x86_merge_dr6() for external
debuggers, and pv_inject_DB() calling pv_inject_event(), results in two
x86_merge_dr6() calls.
Feed the same pending_dbg in the second time. This makes pv_inject_event()'s
update of dr6 effectively a no-op, retaining the correct breakpoint bits.
Fixes: db39fa4b27 ("x86/pv: Fix merging of new status bits into %dr6")
Reported-by: Manuel Bouyer <bouyer@antioche.eu.org>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Manuel Bouyer <bouyer@antioche.eu.org>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Paging Writeable depends on EPT so must be disabled in non-EPT guests like the
other EPT dependent features. Otherwise, VMEntry fails with bad control
state.
Drop a piece of trailing whitepsace in context.
Fixes: ff10aa9d8f ("x86: Add Support for Paging-Write Feature")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
This avoids needing to hold rc in a register across the WRMSR, and in most
cases removes direct testing and branching based on rc, as the fault label can
be rearranged to directly land on the out-of-line block.
No functional change.
Resolves: https://gitlab.com/xen-project/xen/-/work_items/214
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Switch to using a mnemonic, and asm_inline as there's EXTABLE metadata.
Update the types, and adjust parameter names. The name xfeatures is specific
to xcr0, so rename to val to be more generic, even if there isn't another
writeable xcr yet.
No functional change.
Resolves: https://gitlab.com/xen-project/xen/-/work_items/212
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This really is a legacy of not having parameters to start with. Give PV dom0
with a PVH domU a real name.
Reformat the table to fix alignment.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
With the current AP bring up code, Xen can get stuck indefinitely if an AP
freezes during boot after the 'callin' step. Introduce a 5s timeout while
waiting for APs to finish startup.
On failure of an AP to complete startup, send an NMI to trigger the
printing of a stack backtrace on the stuck AP and panic on the BSP.
This patch was done while investigating the issue caused by Intel erratum
ICX143. It wasn't helpful in that case, but it's still and improvement
when debugging AP bring up related issues.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Split the code that triggers remote CPUs to dump stacks into a separate
function. Also introduce a parameter that can be set by the caller of the
newly introduced function to force CPUs to dump the full stack, rather than
just dumping the current function name.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Print the CPU and APIC ID that fails to respond to the init sequence, or
that didn't manage to reach the "callin" state. Expand a bit the printed
error messages. Otherwise the "Not responding." message is not easy to
understand by users.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Very few files need the stubs. Move the infrastructure out of
processor.h and config.h into a new stubs.h, and adjust the includes
accordingly.
Make the per-cpu struct stubs be read mostly; they're unmodified
during the uptime of the CPU, and move them into smpboot.c seeing as
that's where they're allocated and freed.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The IPI dispatch functions already have the required barriers to ensure
correct memory ordering.
Note other callers of send_IPI_mask() don't use any barriers.
No functional change intended.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
rangeset_remove_range() uses inclusive ranges, and hence the end of the
range should be calculated using PFN_DOWN(), not PFN_UP().
Fixes: 4acab25a93 ('x86/vpci: fix handling of BAR overlaps with non-hole regions')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The current underlying implementation of GNTTABOP_cache_flush on x86 won't
work as expected. The provided {clean,invalidate}_dcache_va_range()
helpers only do a local pCPU cache flush, so the cache of previous pCPUs
where the vCPU might have run are not flushed.
However instead of attempting to fix this, make the GNTTABOP_cache_flush
operation only available to ARM. There are no known users on x86, the
implementation is broken, and other architectures don't have grant-table
support yet.
With that operation not implemented on x86, the related
{clean,invalidate}_dcache_va_range() helpers can also be removed.
Fixes: f62dc81c2d ("x86: introduce more cache maintenance operations")
Fixes: 18e8d22fe7 ("introduce GNTTABOP_cache_flush")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-By: Oleksii Kurochko<oleksii.kurochko@gmail.com>
Acked-by: Julien Grall <jgrall@amazon.com>
For once the message printed when a BAR overlaps with a non-hole regions is
not accurate on x86. While the BAR won't be mapped by the vPCI logic, it
is quite likely overlapping with a reserved region in the memory map, and
already mapped as by default all reserved regions are identity mapped in
the p2m. Avoid printing the warning message in modify_bars(), and instead
print a more lax message in the x86 implementation of pci_check_bar() to
note the current BAR position overlaps with non-hole region(s).
Secondly, when an overlap is detected the BAR 'enabled' field is not set,
hence other vPCI code that depends on it like vPCI MSI-X handling won't
function properly, as it sees the BAR as disabled, even when memory
decoding is enabled for the device and the BAR is likely mapped in the
p2m. Change the handling of BARs that overlap non-hole regions to instead
remove any overlapped regions from the rangeset, so the resulting ranges to
map just contain the hole regions. This requires introducing a new
pci_sanitize_bar_memory() that's implemented per-arch and sanitizes the
address range to add to the p2m.
For x86 pci_sanitize_bar_memory() removes any regions present in the host
memory map, for ARM this is currently left as a dummy handler to not change
existing behavior.
Ultimately the above changes should fix the vPCI MSI-X handlers not working
correctly when the BAR that contains the MSI-X table overlaps with a
non-hole region, as then the 'enabled' BAR bit won't be set and the MSI-X
traps won't handle accesses as expected.
Reported-by: Stefano Stabellini <stefano.stabellini@amd.com>
Fixes: 53d9133638 ('pci: do not disable memory decoding for devices')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: Victor M Lira <victorm.lira@amd.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
An attempt to write access the register (i.e. GICR_PROPBASER, GICR_PENDBASER)
which should be ignored (i.e. no virtual ITS present) causes the guest data abort
due to incorrect check at the write_ignore_64 label. The check should be
inverted.
While at it, move goto to the next line.
Fixes: c4d6bbdc12 ("xen/arm: vgic-v3: Support 32-bit access for 64-bit registers")
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Introduce conring_flush() to ensure all messages kept in the internal
console ring are sent to all physical consoles (serial, VGA (x86))
after their initialization is completed.
Rename dump_console_ring_key to conring_dump_keyhandler to match the
notation for conring management symbols.
Resolves: https://gitlab.com/xen-project/xen/-/issues/184
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
guest_console_write() duplicates the code from __putstr(), eliminate code
duplication.
Introduce console_send() for sending a message on console devices.
Also, introduce internal console flags to control which console devices
should be used.
No functional change intended.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Move conring tasklet code close to conring definitions in the console driver
and rename conring tasklet variables by adding conring_ prefix for better
readability.
No functional change.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The current cache flushing done in memory_type_changed() is too wide, and
this doesn't scale on boxes with high number of CPUs. Attempt to limit
cache flushes as a result of p2m type changes, and only do them if:
* The CPU doesn't support (or has broken) self-snoop capability, otherwise
there would be leftover aliases in the cache. For guest initiated
memory changes (like changes to MTRRs) the guest should already do a
cache flush.
* The IOMMU cannot force all DMA accesses to be snooping ones.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Avoid the cache flush if the domain is not yet running. There shouldn't be
any cached data resulting from domain accesses that need flushing, as the
domain hasn't run yet.
There can be data in the caches as a result of Xen and/or toolstack
behavior. Ideally we would do a cache flush strictly before starting the
domain, however doing so only makes sense once we can guarantee there are
no leftover mappings of the affected ranges with cacheable attributes,
otherwise the CPU can speculatively populate the cache with data from those
ranges.
No change in domain observable behavior intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
While unlikely, it's possible for PCI devices to not have any IO resources
assigned, yet in such case the owner domain might still need to issue cache
control operations in case the device performs DMA requests.
Adjust cache_flush_permitted() to account for has_arch_pdevs().
While there also switch l1_disallow_mask() to use cache_flush_permitted().
This should be a non-functional change after the adjustment done to
cache_flush_permitted().
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
To better describe the underlying implementation. Define
cache_flush_permitted() as an alias of has_arch_io_resources(), so that
current users of cache_flush_permitted() are not effectively modified.
With the introduction of the new handler, change some of the call sites of
cache_flush_permitted() to instead use has_arch_io_resources() as such
callers are not after whether cache flush is enabled, but rather whether
the domain has any IO resources assigned.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The current emulation of wb{,no}invd is bogus for PV guests: it will only
flush the current pCPU cache, without taking into account pCPUs where the
vCPU had run previously. Resort to flushing the cache on all host pCPUs to
make it correct.
Fixes: 799fed0a7c ("Priv-op emulation in Xen, for RDMSR/WRMSR/WBINVD")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Rules 5.3, 11.2 and 16.6 are already listed in clean_guidelines_common and
apply to all architectures. There's no need for arm64 to give them a second
time.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
MISRA C Directive 4.10 states that "Precautions shall be taken in order
to prevent the contents of a header file being included more than
once".
Refactor inclusion guards to address a violation of Directive 4.10
and follow CODING_STYLE.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Refactor inclusion guards:
1) use a syntax that is more likely to be recognized by static
analyzers;
2) follow the CODING_STYLE.
No functional change.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
MISRA C Directive 4.10 states that:
"Precautions shall be taken in order to prevent the contents of a
header file being included more than once".
Add inclusion guards where missing to address violations of the
guideline.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
MISRA C Directive 4.10 states that:
"Precautions shall be taken in order to prevent the contents of a
header file being included more than once".
Add inclusion guards where missing to address violations of the
guideline.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
MISRA C Directive 4.10 states that:
"Precautions shall be taken in order to prevent the contents of a
header file being included more than once".
Add inclusion guards where missing to address violations of the
guideline.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Despite the best intentions at the time, the current recommendation
lead to very long identifiers, bordering on the max limit we've chosen
for certification.
One observation is that we do have static analysis which will
highlight if duplicate guards are created accidentally.
Therefore, relax the recommendations and in particular remove the
specific tie to the directory structure. This has the other advantage
of being more similar to other projects.
This will hopefully mean there's less churn getting the tree in shape,
and a random contributor is more likely to pick an appropriate guard
given no specific knowledge of Xen.
As always, it's something reviewers and maintainers should be aware
of, and to advise on.
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
... when there really are only a few instructions in line.
In some cases, reformat to reduce left-hand margine space.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
... when there really are only a few instructions in line.
In some cases, reformat to reduce left-hand margine space.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Compilers estimate the size of an asm() block for inlining purposes.
Constructs with embedded metadata (BUG_FRAME, ALTERNATIVE, EXTABLE, etc)
appear large, depsite often only being a handful of instructions. asm
inline() overrides the estimation to identify the block as being small.
This has a substantial impact on inlining decisions, expected to be for the
better given that the compiler has a more accurate picture to work with.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
For asm() statements, clobbers are disjoint from inputs and outputs.
Therefore, run_in_exception_handler() specifying an input and clobbering
BUG_FN_REG forces the compiler to set up fn in the wrong regsiter, just to
have the code block move it into the correct register.
Instead, use `register asm()` to tell the compiler which register fn really
wants to be in. This results in better code generation.
Bloat-o-meter reports:
ARM64:
Function old new delta
dump_registers 356 348 -8
ARM32:
ns16550_poll 52 48 -4
dump_registers 432 428 -4
The other instruction dropped in ARM64's dump_registers() is an alignment nop.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
This reverts commit 6065a05adf.
The discussed "proper fix" has now been implemented, and the #DF path no
longer writes out-of-bounds. Restore the proper #DF IST pointer.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
_show_registers() prints the data selectors from struct cpu_user_regs, but
these fields are sometimes out-of-bounds. See commit 6065a05adf
("x86/traps: 'Fix' safety of read_registers() in #DF path").
There are 3 callers of _show_registers():
1. vcpu_show_registers(), which always operates on a scheduled-out vCPU,
where v->arch.user_regs (or aux_regs on the stack) is always in-bounds.
2. show_registers() where regs is always an on-stack frame. regs is copied
into a local variable first (which is an OoB read for constructs such as
WARN()), before being modified (so no OoB write).
3. do_double_fault(), where regs is adjacent to the stack guard page, and
written into directly. This is an out of bounds read and write, with a
bodge to avoid the writes hitting the guard page.
Include the data segment selectors in struct extra_state, and use those fields
instead of the fields in regs. This resolves the OoB write on the #DF path.
Resolve the OoB read in show_registers() by doing a partial memcpy() rather
than full structure copy. This is temporary until we've finished untangling
the vm86 fields fully.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
... in preference to the crs[8] array. This avoids abusing crs[5..7] for the
fs/gs bases, giving them proper named fields instead, and avoids storage for
cr1 which is unused in the x86 architecture.
In show_registers(), remove a redundant read_cr2(). read_registers() already
did the same, and it is only the PV path which needs to override with
arch_get_cr2().
In vcpu_show_registers(), express the gsb/gss decision using SWAP(). The
determination is going to get even more complicated under FRED.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Remove an open-coded instance of rangeset_subtract(). No functional change
intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Mykola Kvach