... before making a related copy of it.
* Drop __constraints() so the output parameters are visible in the same block
as they're defined. Use PASTE() rather than opencoding it.
* Adjust the indentation of trailing \'s for consistency.
* Drop the newline at the end of the instruction.
* Indent the if condition correctly. ___res is always of type
arm_smccc_res (declared in __declare_arg_0()), so drop the typeof().
* Drop arm_smccc_1_0_smc() as it has no users.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Both {get,set}_user_reg() are out-of-line functions, leading to awful code
generation.
Introduce arm_smccc_guest_smc() to operate directly on guest registers.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Contrary to what was claimed in commit 67bcf5eae7 ("xen/arm: Simplify type
handling for SMCCC declarations"), there is an important reason to retain the
intermediate variable. It is unsafe to have any logic between the assignment
of the register variables and the asm() block they're used in.
This logically reverts commit 67bcf5eae7 ("xen/arm: Simplify type handling
for SMCCC declarations") while retaining the conversions from commit
7f15d5d132 ("xen/treewide: More typeof() -> auto conversions").
Adjust __declare_arg_0() to match. It happens to be safe because it's the
first register expression once all macros are expanded, but it really should
be consistent with the others.
Leave a comment explaining why they must be written like this.
Fixes: 67bcf5eae7 ("xen/arm: Simplify type handling for SMCCC declarations")
Reported-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
imsic_init() indexes msi[] by the Xen CPU id hartid_to_cpuid() returns,
but that array is allocated with one entry per parent IRQ of the IMSIC
node, and the only range check compares the index against
num_possible_cpus(). Neither matches the array, and the check comes
after the first access:
- hartid_to_cpuid() returns NR_CPUS when the hart isn't one Xen brought
up, and msi[NR_CPUS].base_addr is read before that is noticed;
- an IMSIC node listing fewer parents than Xen has CPUs makes every
index past nr_parent_irqs go past the end of the allocation, which the
num_possible_cpus() check lets through.
Size the array by nr_cpu_ids, which is what it is indexed by,
and move the range check ahead of the first msi[] access.
Fixes: c9bd8b322e ("xen/riscv: imsic_init() implementation")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
With the split-off of decoding into a separate file, both files' top
comments should have been adjusted accordingly.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
ALT_CALL_PTR() really means to get rid of const (when necessary, i.e. when
used from apply_alt_calls()): We want to patch the original call site,
after all, and memory-wise that's unrelated to the struct alt_call * that
we start from.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
While the general C library strchr() has to do so, the early boot cmdline
parsing has no such requirement.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
On encountering an unexpected VMEXIT (e.g. because L1 has used an
intercept that L0 Xen doesn't know about), crash the domain rather than
the hypervisor.
Fixes: 9a779e4fc1 ("Implement SVM specific part for Nested Virtualization")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Forever since the introduction of the .priv member it has been wrong to
record the section as r/o.
.alt_call_sites, otoh, is legitimately r/o, hence
__alt_call_sites_{start,end}[] would better reflect that.
Fixes: 4008c71d7a ("x86/alt: Support for automatic padding calculations")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Doing so (at the return statement), besides being a bad idea anyway, is a
violation of Misra rule 11.8.
Make "xcap" have an initializer while adjusting the code anyway.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Doing so violates Misra rule 11.8, while being entirely unnecessary here:
All callers already store the return value in pointer-to-const variables.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
We want to patch the original call site, which memory-wise is unrelated
to the struct alt_instr * that we start from.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
The requested PCI segment needs including in the call to
xsm_pci_config_permission(). Otherwise in a multi-segment system we can check
the perimssions on one device but operate on a different one.
This is only not a vulnerability because pci_conf_write_intercept() is only
reachable by the hardware domain.
Fixes: 300bb048ca ("x86/PCI: make all config space writes subject to XSM checking")
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Doing so, besides being a bad idea in general, violates Misra rule 11.8.
No functional change intended.
Fixes: f1879b2c25 ("xen: introduce generic non-atomic test_*bit()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Waiting loops like the one in flush_command_buffer() will degenerate to
infinite ones when used early enough for NOW() to still return constant
zero. Make sure the returned value at least monotonically increases. When
available, use nominal frequency values as initial approximation.
Do this only in get_s_time(), as producing a sane value in
get_s_time_fixed() for non-zero inputs won't be reasonably possible.
Put an assertion there.
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Some early setup doesn't need re-doing after ucode load. Move the call to
initialize_cpu_data() slightly up and add a conditional return point.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Both ROMBIOS and SeaBIOS (with CONFIG_QEMU=y, as we build it) blindly
assume availability of this field (at its conventional index 0x32); OVMF
at least has code to inspect FADT. Hence we ought to have supported it
virtually forever.
As the index is beyond RTC_CMOS_SIZE, leverage the padding field in
struct hvm_hw_rtc to hold its value. Update the field only when involved
values are valid BCD century specifiers. Otherwise (for VMs migrated in
from an older hypervisor) leave handling to the DM.
This makes the Linux rtc-cmos driver report y3k compatibility.
In the new rtc_check(), besides checking the new fields also check the
pre-existing pad0 field.
While extending xen-hvmctx.c:dump_rtc() also add RTC offset there.
Fixes: 4ca1612143 ("[HVM] Move RTC emulation into the hypervisor")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
For dates very far into the future (the MC146818 RTC's century byte can go
up to the 99th century), the present year-wise loop would become somewhat
inefficient (taking perhaps several thousand iterations). Prefix that loop
with a 400-year granular calculation (somewhat like the earlier loop does
for dates in the past).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Indicating it would always use BCD mode is just wrong (and then the
comment there said the opposite). All halfway recent (and really all 64-
bit capable) systems having a CMOS RTC should properly indicate the mode
in control register B.
Make use of the flag, but provide a fallback mechanism in case people run
into systems not matching the above assumption. Additionally, when binary
mode is indicated and when "cmos-rtc-probe" is in use (but "cmos-rtc-bcd"
isn't), probe whether the clock really runs in binary mode. (This probing,
sadly, can take up to 10 seconds.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
This was copy&paste from x86. It causes the padding between functions to be:
a000026dd64: 90909090 adrp x16, 9ff2147d000 <start-0xded83000>
rather than:
a000026dd64: 00000000 udf #0
No functional change.
Fixes: 5da0d3123c ("arm: entry.S and head.S")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
This has been a plain C read for as far back as I can trace in history.
Research into invented-loads has flagged it as a possible vulnerability.
After analysis, it is believed to be a bug only, not a security vulnerability.
The analysis suggests that we can read one value out of the guest, operate on
another, and that this could be an in-guest privliege escalation. Any entity
in the guest able to modify the pagetables already has full privilege, so
while Xen can potentially malfunction, the effects don't cross a privilege
boundary.
The analysis also suggests that this is worse for shadow guests because we may
put the TOCTOU entry in the shadows, but this is inaccurate. What we put in
the shadows is still translated under the P2M and refers to guest physical
address space.
Either way, the code fits the pattern for invented loads, and it is a risk;
harden the accesses.
Link: https://github.com/xoreaxeaxeax/schrodingers-toctou/blob/main/observer-effect/audits/audit-xen-ptwalk-RELEASE-4.21.1.md#86-per-candidate-finding
Fixes: 49f7c7364e ("Replace shadow pagetable code with shadow2.")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Treat "argo" on the command line as a positive boolean, rather than requiring
the user to pass "argo=1/on/enable/true".
Move both opt_argo* variables into __ro_after_init. They're set during
command line parsing and never modified thereafter.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
sched_init_vcpu() calls init_timer() for a vcpu's periodic_timer,
singleshot_timer and poll_timer before it can fail -- these
become live, linked into their target pCPU's per-cpu timer list
regardless of what happens next. If the sched_alloc_udata() call
further down then fails, the function frees the sched_unit via
sched_free_unit() and returns 1, but never unlinks these three
timers.
The caller, vcpu_create(), makes this worse: on sched_init_vcpu()
returning nonzero it jumps to fail_wq, skipping fail_sched and
thus sched_destroy_vcpu() -- the only function on this path that
calls kill_timer() on them. vcpu_destroy() then frees the vcpu,
and the three timers embedded in it, while they are still linked
into that shared list.
This silently corrupts that list. It only shows up later, when
something else touches a neighboring timer: sched_move_domain()
crashed with "Assertion 'entry->prev->next == entry' failed" on a
completely unrelated, valid vcpu's timer.
Call sched_destroy_vcpu() in sched_init_vcpu()'s own failure
branch instead of sched_free_unit(), so it doesn't depend
on the caller reaching sched_destroy_vcpu() to undo what it set
up itself. sched_destroy_vcpu() assumes unit->priv is set, which
is not the case here, so make it only free the udata and remove
the unit if unit->priv in non-NULL.
Fixes: d884b10778 ("Domain creation/destruction cleanups.")
Signed-off-by: Furkan Caliskan <frn1furkan10@gmail.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Cases 0..3 are handled, and a 2-bit mask is applied to the switch()
expression. Therefore the default: case is reported unreachable by Eclair.
Insert BUILD_ERROR() to annotate this for Eclair.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
BUILD_ERROR() is even stronger a guard than assertions in general, and
ASSERT_UNREACHABLE() (or BUG()) in particular. Deviate it just like those
to allow use for marking unreachable portions of code.
In some cases code being unreachable is dependent upon configuration.
Introduce two variants, as constructs like
if ( IS_ENABLED(CONFIG_...) )
BUILD_ERROR("...");
results in the if() still being reported as unreachable. Sadly these two
new macros introduce a new 20.12 violation each, which hence also needs
deviating.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
LPI related functions having just BUG() in them are disliked by Misra /
Eclair, as long as they don't also have a noreturn attribute.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The stub altp2m_vcpu_idx() is recognized as "noreturn" function lacking
respective annotation (or having a return statement), which hence is deemed
unreachable code by Misra / Eclair. All call sites are guarded by
altp2m_active() checks, hence an inline function isn't needed. A
declaration will suffice, with call sites then getting DCE-d.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
In both functions cases 0..3 are handled, and a 2-bit mask is applied to
the switch() expression. Therefore the default: cases are reported
unreachable by Eclair. Subsume the "case 2" blocks each into the
corresponding default ones.
While there also drop all the pointless figure braces inside the various
case blocks, inserting blank lines instead between them.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
asm/riscv_encoding.h already provides INSN_16BIT_MASK and INSN_LEN(), and
emulate.c uses them, so the tree carried two spellings of the same thing
which could drift apart. COMPRESSED_INSN_MASK never had a user.
No functional change.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
Acked-by: Jan Beulich <jbeulich@suse.com>
As nothing is using pregs in upstream or downstream ports of RISC-V drop
it from struct cpu_user_regs. Once it will really be needed re-introduce
it.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
Acked-by: Jan Beulich <jbeulich@suse.com>
The function does not modify context argument.
Also it gives more flexibility to this API usage, because some context strings
in Xen are also const char*.
While there,
- harmonize last parameter's name,
- u32 -> uint32_t.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Of the short-data sections, only .sbss is presently mentioned in the
linker script. Place them next to, but ahead of their "normal" data
sections.
.riscv.attributes can go towards the tail of the image, next to (ahead of)
debug info.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
For whatever reason they didn't have an xsm_default_t first argument (to
cope with XSM=n mode), making it impossible to (easily) cover them in
xsm/hooks.h.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
These are boolean, so they should always have used bool (originally
bool_t), not uint8_t. Leverage recent changes to arrange for this with
(now) fewer places which need changing (within the XSM machinery itself).
Adjust call sites as well, where the conversion wasn't done so far.
While doing this, rename flask_io{port,mem}_mapping()'s last parameters to
"map".
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
vgic_v{2,3}_setup_hw()'s callers are __init, so they should be too.
vgic_v{2,3}_hw and gic_v2_hw_data are written once during init and
unmodified thereafter, so make them __ro_after_init. Reposition
'bool enabled' in these structures to fit in the tail padding, removing
8 bytes from their size when paddr_t is 8B.
While at it, drop dead vgic_v3_setup_hw() dummy implementation
from vgic/vgic.c. GICV3 depends on !NEW_VGIC.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Julien Grall <julien@xen.org>
panic_PAR() is only called from va_to_par(), which translates using
__va_to_par(), i.e. "at s1e2r" on arm64 and ATS1HR on arm32. Both
perform an EL2 stage 1 only translation, so PAR.S (PAR_STAGE2) and
PAR.PTW (PAR_STAGE21) can never be set.
This has been the case since commit a14447dbcf ("xen: arm: do not
panic when failing to translate a guest address"), which made
gva_to_par() and gva_to_ipa() return -EFAULT instead of calling
panic_PAR() for guest translations.
Print stage 1 unconditionally and keep an ASSERT() to document the
invariant. PAR_STAGE21 has no user left, so drop it. While at it, fix
the spacing in the decode_fsc() call.
No functional change intended.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
decode_fsc() derives the fault level from the low two bits of the FSC, so
level 0 is a valid output: FSC_FLT_TRANS is 0x04, i.e. "translation fault,
level 0 or translation table base register".
This is reachable on arm64 because xen_pgtable is the zeroeth-level root,
but fsc_level_str() has no case for it and prints " (level invalid)"
instead. At the time the function was created Xen used only three levels.
Add the missing case. On arm32 the zeroeth level does not exist but FSC
0 denotes address size fault at translation table base register.
While at it, make decode_fsc() decode also address size faults.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
On ARMv8.4-A and newer platforms, booting Dom0 Linux with ACPI enabled
causes the domain to probe advanced PMU feature based on system ID
register ID_AA64DFR0_EL1. During this probe, Linux accesses PMMIR_EL1,
which causes unhandled register traps and crashes the domain.
To address this issue, implement the following:
- Add emulation for ID_AA64DFR{0,1}_EL1, ID_DFR{0,1}_EL1 and
ID_DFR{0,1} register accesses for guest domains.
- Hide PMU registers from a guest domain when its vPMU feature is
disabled.
- Proactively make SPE, TRBE, BRBE, Trace Extensions, SPMU, ITE and
EBEP inaccessible to arm64 guest domains and hide TraceFilt,
MMapTrc and CopTrc to arm32 guest domains, as they could
potentially cause similar issues.
Fixes: dbb948110a ("xen: Expose the PMU to the guests")
Fixes: 07b9acea11 ("xen/arm: Add handler for ID registers on arm64")
Fixes: 3669a1cb95 ("xen/arm: create a cpuinfo structure for guest")
Signed-off-by: Hirokazu Takahashi <taka@valinux.co.jp>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Just having called the function and stored the result, there's no point in
calling it again right away with the same argument; the stored result can
be used instead. And the stored result also doesn't need overwriting again
with the same value.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
FEP is now the only reason for the #UD handler to run.
While the #UD handler is unreachable when CONFIG_HVM_FEP is disabled, it's
helpful to keep the function around. Reinject #UD in the non-FEP case so
DCE can remove the rest.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Hypervisor leaf 5 is PV-specific, but is gated with !is_hvm_domain(),
which stays a runtime test in a build without PV support. Test for a
PV domain directly so the compiler can discard the leaf when
CONFIG_PV is disabled.
Suggested-by: Roger Pau Monné <roger@xenproject.org>
Signed-off-by: Jiaqing Zhao <Zhao.Jiaqing@amd.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
While the CPU policy is obtained from hardware, the CRn values to start
with are taken from fuzzed input. Since most CR4 bits can only be set when
the respective feature is indicated as available by CPUID, the emulator
often only checks the CR4 bit. Without sanitization, assertions like the
one in emul_test_read_xcr() (checking XSAVE support) could therefore
trigger.
Omit most paging-only bits from sanitization, as the core emulator doesn't
itself walk page tables. LA57 wants checking for the bit being used by
CANONICALIZE_MAYBE().
Reported-by: Andrew Mbugua <andrewprecious388@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
It's possible for domains to generate unaligned PCI config space accesses
when using ECAM, and hence vPCI should support those at least for the
hardware domain. Such unaligned accesses to the PCI config space have been
reported to come from ACPI logic.
Relax the checking in vpci_access_allowed() to allow such accesses for the
hardware domain, and fix the handling in pci_conf_{read,write}{16,32}() to
fulfill them using MMCFG.
MMCFG regions are identity exposed to the hardware domain, and hence such
unaligned accesses can only come as a result of the host having MMCFG in the
first place, as otherwise MMCFG won't be exposed to the hardware domain
either.
Note that vpci_ecam_{read,write}() already refuse accesses that cross a
device boundary unconditionally.
Reported-by: Jason Andryuk <jason.andryuk@amd.com>
Signed-off-by: Roger Pau Monné <roger@xenproject.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Andrew Cooper