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>
Only do the cache flush when there's a p2m type change to propagate,
otherwise there's no change in the p2m effective caching attributes.
If the p2m memory_type_changed hook is not set p2m_memory_type_changed() is
a no-op, no recalculation of caching attributes is needed, nor flushing of
the previous cache contents.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Aiui in principle this constitutes a Misra C:2012 rule 2.2 violation.
Just that we didn't adopt this rule (yet?).
Fixes: afcb4a06c7 ("x86/thunk: Build Xen with Return Thunks")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Introduce defintions of IRQ_TYPE_* which correspond to the Xen internal
representation of the IRQ types and make them the same asthe existing
device tree defintions for convenience.
These defines are going to be re-used, at least, by RISC-V architecture.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
We don't need to invalidate caches here; all we're after is that earlier
writes have made it to main memory (and aiui even that just in case).
Plus we only need to do this if any of the IOMMUs is non-coherent.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
If init_one_irq_desc() fails, ->arch.creator_domid won't be set to the
expected value, and hence the assertion may trigger. Limit it to just
the success case of that function call.
Fixes: 92d9101eab ("x86: allow stubdom access to irq created for msi")
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Add inclusion of xen/bitops.h to asm/cmpxchg.h to avoid compilation issues
connected to GENMASK() which is used inside asm/cmpxchg.h.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The this_isa bitmap should be explicitly initialized to zero to avoid
false positives when detecting supported ISA extensions. Without proper
zero-initialization, the bitmap may retain non-zero values from
uninitialized memory, causing Xen to incorrectly assume that certain
extensions are supported.
This change ensures reliable detection of ISA capabilities.
Fixes: 0c2f717eae ("xen/riscv: identify specific ISA supported by cpu")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The majority of the present callers really aren't after invalidating
cache contents, but only after writeback. Make this available by simply
extending the FLUSH_CACHE handling accordingly. No feature checks are
required here: cache_writeback() falls back to cache_flush() as
necessary, while WBNOINVD degenerates to WBINVD on older hardware.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The implementation of MMUEXT_FLUSH_CACHE is bogus, as it doesn't account to
flush the cache of any previous pCPU where the current vCPU might have run,
and hence is likely to not work as expected.
Fix this by resorting to use the same logic as MMUEXT_FLUSH_CACHE_GLOBAL,
which will be correct in all cases.
Adjust MMUEXT_FLUSH_CACHE_GLOBAL return code in case cache flush is not
permitted for the domain to use -EACCES instead of -EINVAL, as that's more
accurate and also matches the error code used by MMUEXT_FLUSH_CACHE.
Fixes: 8e90e37e6d ("Fix WBINVD by adding a new hypercall.")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The livepatch list sysctl writes metadata into a buffer provided by the
caller. The caller is expected to allocate an appropriately sized buffer
but this is racy and may result in Xen writing beyond the end of the
buffer should the metadata size change.
The name buffer is expected to be an array of elements with size
XEN_LIVEPATCH_NAME_SIZE to avoid this kind of race but the xen-livepatch
tool allocates only as many bytes as needed, therefore encountering the
same potential race condition.
Fix both these issues by requiring the caller to pass in the size of the
name and metadata buffers and then not writing beyond the allocated
size.
The sysctl interface version is bumped due to the change in semantics of
the fields.
Signed-off-by: Kevin Lampis <kevin.lampis@cloud.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Fixes: b145b4a39c ('livepatch: Handle arbitrary size names with the list operation')
Fixes: 5083e0ff93 ('livepatch: Add metadata runtime retrieval mechanism')
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Since commit 434596bbd4 ("x86/smpboot: Write the top-of-stack block in
cpu_smpboot_alloc()"), smp_processor_id() is unconditionally usable on APs.
Drop the global variable.
Also drop the parameter from start_secondary(). It was introduced as unused
in commit e9ac3bbcca ("Move initial stack-pointer adjustment into assembly
bootstrap code.") in 2005. At the time, the caller was a shared codepath with
__start_xen() with a parameter on the stack, but that never mattered for
start_secondary() which ultimately reset_stack_and_jump()'s out of context.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
It is easier to express feature word 17 in terms of word 16 + [32, 64) as
that's how the layout is given in documentation.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The Indirect Target Selection speculative vulnerability means that indirect
branches (including RETs) are unsafe when in the first half of a cacheline.
In order to mitigate this, build with return thunks and arrange for
__x86_return_thunk to be (mis)aligned in the same manner as
__x86_indirect_thunk_* so the RET instruction is placed in a safe location.
place_ret() needs to conditionally emit JMP __x86_return_thunk instead of RET.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The Indirect Target Selection speculative vulnerability means that indirect
branches (including RETs) are unsafe when in the first half of a cacheline.
This means it's not safe for logic using the stubs to write raw 0xc3's.
Introduce place_ret() which, for now, writes a raw 0xc3 but will contain
additional logic when return thunks are in use.
stub_selftest() doesn't strictly need to be converted as they only run on
boot, but doing so gets us a partial test of place_ret() too.
No functional change.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The Indirect Target Selection speculative vulnerability means that indirect
branches (including RETs) are unsafe when in the first half of a cacheline.
clear_bhb_loops() has a precise layout of branches. The alignment for
performance cause the RETs to always be in an unsafe position, and converting
those to return thunks changes the branching pattern. While such a conversion
is believed to be safe, clear_bhb_loops() is also a performance-relevant
fastpath, so (mis)align the RETs to be in a safe position.
No functional change.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The Indirect Target Selection speculative vulnerability means that indirect
branches (including RETs) are unsafe when in the first half of a cacheline.
Arrange for __x86_indirect_thunk_* to always be in the second half.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In order to protect against ITS, Xen needs to start using return thunks.
Therefore the advice in XSA-466 becomes relevant, and the hypercall_page needs
to be removed.
Implement early_hypercall(), with infrastructure to figure out the correct
instruction on first use. Use ALTERNATIVE()s to result in inline hypercalls,
including the ALT_NOT() form so we only need a single synthetic feature bit.
No overall change.
This is part of XSA-469 / CVE-2024-28956
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Use the top bit of a->cpuid to express inverted polarity. This requires
stripping the top bit back out when performing the sanity checks.
Despite only being used once, create a replace boolean to express the decision
more clearly in _apply_alternatives().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Rule 13.2 states: "The value of an expression and its persistent side effects
shall be the same under all permitted evaluation orders".
Eclair complains about a Rule 13.2 violations because validate_far_branch()
assigns to rc, and the entirety of commit_far_branch() is also assigned to rc.
I'm unsure that the complaint is accurate, but rewriting commit_far_branch()
to use the comma operator seems to make Eclair happy.
Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Rule 19.1 states: "An object shall not be assigned or copied
to an overlapping object". In the function like macro "get_rep_prefix",
one member of a union is assigned the value of another member. Reading from one
member and writing to the other violates the rule, while not causing Undefined
Behavior due to their relative sizes. Instead, use casts combined with exactly
overlapping accesses to address violations.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Victor Lira <victorm.lira@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There are two originators for the PCI configuration space access:
1. The domain that owns physical host bridge: MMIO handlers are
there so we can update vPCI register handlers with the values
written by the hardware domain, e.g. physical view of the registers
vs guest's view on the configuration space.
2. Guest access to the passed through PCI devices: we need to properly
map virtual bus topology to the physical one, e.g. pass the configuration
space access to the corresponding physical devices.
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
On Arm, we expect read handlers to have the bits above the access size
zeroed. vPCI read handlers may return all 1s. Mask off the bits above
the access size.
Fixes: 9a5e22b642 ("xen/arm: check read handler behavior")
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Denis Mukhin