While be12fcca8b ("efi: fix alignment of function parameters in compat
mode") intentionally bounced them both ways to avoid any functional
change so close to the release of 4.16, the bouncing-in shouldn't really
be needed. In exchange the local variables need to gain initializers to
avoid copying back prior stack contents.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
When it was introduced, it was imo placed way too high up, making it
necessary to forward-declare way too many static functions. Move it down
together with
- the efi_check_dt_boot() stub, which afaict was deliberately placed
immediately ahead of the #include,
- blexit(), because of its use of the efi_arch_blexit() hook.
Move up get_value() and set_color() to before the inclusion so their
forward declarations can also be zapped.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Just like (in protected mode) reads may not go to exec-only segments and
writes may not go to non-writable ones, insn fetches may not access data
segments.
Fixes: 623e837167 ("hvm: Support hardware task switching")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Replace all comparisons against p2m_populate_on_demand (outside of
switch() statements) with the designated predicate.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
paging_mfn_is_dirty() is moderately expensive, so avoid its use unless
its result might actually change anything. This means moving the
surrounding if() down below all other checks that can result in clearing
_PAGE_RW from sflags, in order to then check whether _PAGE_RW is
actually still set there before calling the function.
While moving the block of code, fold two if()s and make a few style
adjustments.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tim Deegan <tim@xen.org>
The supported parameter was added in 2d9b91f1ae ("VMX/vPMU: fix DebugCtl MSR
handling"). It unfortunately laid the groundwork for XSA-269, and the fix
2a8a8e99fe ("x86/vtx: Fix the checking for unknown/invalid MSR_DEBUGCTL
bits") totally rewrote MSR_DEBUGCTL handling.
Strip out the parameter again.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
dummy_xsm_ops was dropped as part of organising XSM to be altcall compatible,
but the extern was accidentally left around.
A later change reintroduced dummy_ops which is logically the same thing, but
is private to xsm/dummy.c
Fixes: 164a0b9653 ("xsm: refactor xsm_ops handling")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
This should have been done at the point xsm_ops became fully altcall'd. This
puts the xsm_ops structure in .init on architectures where it is no longer
referenced at runtime.
Fixes: d868feb95a ("xen/xsm: Complete altcall conversion of xsm interface")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
There is no reason to use void pointer while passing ECAM ops to the
pci_host_common_probe function as it is anyway casted to struct pci_ecam_ops
inside. For that reason remove the void pointer and pass struct pci_ecam_ops
pointer as is.
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Rahul Singh <rahul.singh@arm.com>
Tested-by: Rahul Singh <rahul.singh@arm.com>
Without holding appropriate locks, attempting to remove a prior mapping
of the underlying page is pointless, as the same (or another) mapping
could be re-established by a parallel request on another vCPU. Move the
code to Arm's gnttab_set_frame_gfn(); it cannot be dropped there since
xenmem_add_to_physmap_one() doesn't call it either (unlike on x86). Of
course this new placement doesn't improve things in any way as far as
the security of grant status frame mappings goes (see XSA-379). Proper
locking would be needed here to allow status frames to be mapped
securely.
In turn this then requires replacing the other use in
gnttab_unpopulate_status_frames(), which yet in turn requires adjusting
x86's gnttab_set_frame_gfn(). Note that with proper locking inside
guest_physmap_remove_page() combined with checking the GFN's mapping
there against the passed in MFN, there then is no issue with the
involved multiple gnttab_set_frame_gfn()-s potentially returning varying
values (due to a racing XENMAPSPACE_grant_table request).
This, as a side effect, does away with gnttab_map_frame() having a local
variable "gfn" which shadows a function parameter of the same name.
Together with XSA-379 this points out that XSA-255's addition to
gnttab_map_frame() was really useless.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
The example in section "UEFI boot and dom0less on ARM" has a wrong
compatible for the DTB passthrough, it is "ramdisk" instead of
"device-tree".
This patch fixes the example.
Fixes: a1743fc3a9 ("arm/efi: Use dom0less configuration when using EFI boot")
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Commit 68dcdf9423 (xen/arm:
s/gic_set_guest_irq/gic_raise_guest_irq) forgot to modify a comment
about lr_pending list, referring to a function that has been renamed.
Fix that.
Fixes: 68dcdf9423 ("xen/arm: s/gic_set_guest_irq/gic_raise_guest_irq")
Signed-off-by: Michal Orzel <michal.orzel@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
hvcall_{flush,ipi}_ex() use more almost identical code than what was
isolated into hv_vpset_to_vpmask(). Move that code there as well, to
have just one instance of it. This way all of HV_GENERIC_SET_SPARSE_4K
processing now happens in a single place.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
I'm puzzled about two inconsistencies with other alternative_vcall<N>()
here: There's a check missing that the supplied function pointer is
actually pointing to a function taking no args. And there's a pointless
pair of parentheses. Correct both.
Fixes: 67d01cdb55 ("x86: infrastructure to allow converting certain indirect calls to direct ones")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Chen <Wei.Chen@arm.com>
While commit 46c4061cd2 ("x86/IOMMU: mark IOMMU / intremap not in use
when ACPI tables are missing") deals with apic_x2apic_probe() as called
from x2apic_bsp_setup(), the check_x2apic_preenabled() path is similarly
affected: The call needs to occur after acpi_iommu_init(), such that
iommu_intremap getting disabled there can be properly taken into account
by apic_x2apic_probe().
Note that, for the time being (further cleanup patches following),
reversing the order of the calls to generic_apic_probe() and
acpi_boot_init() is not an option:
- acpi_process_madt() calls clustered_apic_check() and hence relies on
genapic to have got filled before,
- generic_bigsmp_probe() (called from acpi_process_madt()) needs to
occur after generic_apic_probe(),
- acpi_parse_madt() (called from acpi_process_madt()) calls
acpi_madt_oem_check(), which wants to be after generic_apic_probe().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
This is in preparation of adding another "translation" method. Take the
combination of the extra validation both previously open-coded have been
doing: Bounds check and bitmap check. But don't propagate the previous
pointless check of whether ->domid_map[] was actually allocated, as
failure there would lead to overall failure of IOMMU initialization
anyway.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
- Correct struct field type.
- Use unsigned int when that suffices.
- Eliminate a (badly typed) local variable from
context_set_domain_id().
- Don't use -EFAULT inappropriately.
- Move set_bit() such that it won't be done redundantly.
- Constification.
- Reduce scope of some variables.
- Coding style.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Both vendors' code populates all hooks, so there's no need to have any
NULL checks before invoking the hook functions. With that the only
remaining uses of the object are in alternative_{,v}call(), i.e. none
after alternatives patching.
In vpmu_arch_initialise() the check gets replaced by an opt_vpmu_enabled
one, as I couldn't convince myself that the pre-existing checks would be
sufficient to cover all possible cases.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
I see little point in having an open-coded switch() statement to achieve
the same; like other vendor-specific operations the function can be
supplied in the respective ops structure instances.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
At least some vPMU functions will be invoked (and hence can further be
speculated into) even in the vPMU-disabled case. Convert vpmu_ops to
the standard single-instance model being a prerequisite to engaging the
alternative_call() machinery, and convert all respective calls. Note
that this requires vpmu_init() to become a pre-SMP initcall.
This change then also helps performance.
To replace a few vpmu->arch_vpmu_ops NULL checks, introduce a new
VPMU_INITIALIZED state, such that in the absence of any other suitable
vmpu_is_set() checks this state can be checked for.
While adding the inclusion of xen/err.h, also prune other xen/*.h
inclusions.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This reverts commit 228ab9992f.
Performance analysis has shown that dropping the domctl lock during
domain destruction greatly increases the contention in the heap_lock,
thus making parallel destruction of domains slower.
The following lockperf data shows the difference between the current
code and the reverted one:
lock: 3342357(2.268295505s), block: 3263853(18.556650797s)
lock: 2788704(0.362311723s), block: 222681( 0.091152276s)
Those figures are from Dmitry Isaikin, and are gathered after
destroying 5 2GB HVM guests in parallel:
https://lists.xenproject.org/archives/html/xen-devel/2021-09/msg01515.html
Given the current point in the release, revert the commit and
reinstate holding the domctl lock during domain destruction. Further
work should be done in order to re-add more fine grained locking to
the domain destruction path once a proper solution to avoid the
heap_lock contention is found.
Reported-by: Hongyan Xia <hongyxia@amazon.com>
Reported-by: Dmitry Isaikin <isaikin-dmitry@yandex.ru>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Today there is no hypercall with more than 5 parameters, while the ABI
allows up to 6 parameters. Especially for the X86 32-bit case using
6 parameters would require to run without frame pointer, which isn't
very fortunate. Note that for Arm the limit is 5 parameters already.
So limit the maximum number of parameters to 5 for x86, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
There's not really any register state associated with vCPU-s that
haven't been initialized yet, so avoid spamming the log with largely
useless information while still leaving an indication of the fact.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Wire up show_hvm_stack() also on this path. Move the show_guest_stack()
invocation out of show_stack(), rendering dead the is-HVM check there.
While separating guest and host paths, also move the show_code()
invocation - the function bails immediately when guest_mode() returns
"true".
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Leaving shadow setup just to the L1TF tasklet means running Dom0 on a
minimally acceptable shadow memory pool, rather than what normally
would be used (also, for example, for PVH). Populate the pool before
triggering the tasklet (or in preparation for L1TF checking logic to
trigger it), on a best effort basis (again like done for PVH).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Tim Deegan <tim@xen.org>
For security hardening reasons, it advantageous to make setup-once data
immutable after boot. Borrow __ro_after_init from Linux.
On x86, place .data.ro_after_init at the start of .rodata, excluding it from
the early permission restrictions. Re-apply RO restrictions to the whole of
.rodata in init_done(), attempting to reform the superpage if possible.
For architectures which don't implement __ro_after_init explicitly, variables
merges into .data.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
At the moment, we have two locations selecting restricted permissions, not
very far apart on boot, dependent on opposite answers from using_2M_mapping().
The later location however can shatter superpages if needed, while the former
cannot.
Collect together all the permission adjustments at the slightly later point in
boot, as we likely need to shatter a superpage to support __ro_after_init.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The calculation in __start_xen() for xen_virt_end is an opencoding of
ROUNDUP(_end, 2M). This is __2M_rwdata_end as provided by the linker script.
This corrects the bound calculations in arch_livepatch_init() and
update_xen_mappings() to not enforce 2M alignment when Xen is not compiled
with CONFIG_XEN_ALIGN_2M.
Furthermore, since 52975142d1 ("x86/boot: Create the l2_xenmap[] mappings
dynamically"), there have not been extraneous mappings to delete, meaning that
the call to destroy_xen_mappings() has been a no-op.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
multiboot_ptr should be in __initdata - it is only used on the BSP path.
Furthermore, the .align 8 then .long means that stack_start is misaligned.
Move both into setup.c, which lets the compiler handle the details correctly,
as well as providing proper debug information for them.
Declare stack_start in setup.h and avoid extern-ing it locally in smpboot.c.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The first loop relocates all reachable non-leaf entries in idle_pg_table[],
which includes l2_xenmap[511]'s reference to l1_fixmap_x[].
The second loop relocates only leaf mappings in l2_xenmap[], so update the
skip condition to be opposite to the first loop.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
It has been 4 years since the default load address changed from 1M to 2M, and
_stext ceased residing in l2_xenmap[0]. We should not be inserting an unused
mapping.
To ensure we don't create mappings accidentally, loop from 0 and obey
_PAGE_PRESENT on all entries.
Fixes: 7ed93f3a0d ("x86: change default load address from 1 MiB to 2 MiB")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
GENMASK(30, 21) should be 0x7fe00000. Fixed this in the comment
in bitops.h.
Signed-off-by: Ayan Kumar Halder <ayankuma@xilinx.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
[Tweak text, to put an end to any further bikeshedding]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
I have just forward-ported the CHANGELOG.md updates from the
stable-4.16 branch. But we need a new section for work in this
release cycle.
Signed-off-by: Ian Jackson <iwj@xenproject.org>
Document some of the relevant changes during the 4.16 release cycle.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
(cherry picked from commit e2544a28be)
Currently, the code used to handle and possibly load from the filesystem
modules defined in the DT is allocating and closing the filesystem handle
for each module to be loaded.
To improve the performance, the filesystem handle pointer is passed
through the call stack, requested when it's needed only once and closed
if it was allocated.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Update libfdt to v1.6.1 of libfdt taken from git://github.com/dgibson/dtc.
This update is done to support device tree overlays.
A few minor changes are done to make it compatible with Xen:
fdt_overlay.c: overlay_fixup_phandle()
Replace strtoul() with simple_strtoul() as strtoul() is not available in
Xen lib and included lib.h.
Change char *endptr to const char *endptr. This change is required for
using simple_strtoul().
libfdt_env.h:
Remaining Xen changes to libfdt_env.h carried over from existing
libfdt (v1.4.0)
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
NMI hooking in the crash path has undergone several revisions since its
introduction. What we have now is not sufficiently different from the regular
nmi_callback() mechanism to warrant special casing.
Use set_nmi_callback() directly, and do away with patching a read-only data
structure via a read-write alias. This also means that the
vmx_vmexit_handler() can and should call do_nmi() directly, rather than
indirecting through the exception table to pick up the crash path hook.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The unconditional nmi_callback() call in do_nmi() calls dummy_nmi_callback()
in all cases other than for a few specific and rare tasks (alternative
patching, microcode loading, etc).
Indirect calls are expensive under retpoline, so rearrange the logic to use
NULL as the default, and skip the call entirely in the common case.
While rearranging the code, fold the exit paths.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This causes NMIs, #DB and #MC to be counted, rather than being reported as 0.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This is a simple comma separated list, so use the normal form.
* Don't cease processing subsequent elements on an error
* Do report -EINVAL for things like `dom0_nodes=4foo`
* Don't opencode the cmdline_strcmp() helper
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
TLB flushing is a hotpath, and function pointer calls are
expensive (especially under retpoline) for what amounts to an identity
transform on the data. Just pass the vcpu_bitmap bitmap directly.
As we use NULL for all rather than none, introduce a flush_vcpu() helper to
avoid the risk of logical errors from opencoding the expression. This also
means the viridian callers can avoid writing an all-ones bitmap for the
flushing logic to consume.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Function pointers are expensive, and the raw parameter is a constant at the
root of all call trees, meaning that it predicts very well with local branch
history.
Furthermore, the knock-on effects are quite impressive.
$ ../scripts/bloat-o-meter xen-syms-before xen-syms-after
add/remove: 0/4 grow/shrink: 3/9 up/down: 459/-823 (-364)
Function old new delta
__ioapic_write_entry 73 286 +213
__ioapic_read_entry 75 276 +201
save_IO_APIC_setup 182 227 +45
eoi_IO_APIC_irq 241 229 -12
disable_IO_APIC 296 280 -16
mask_IO_APIC_setup 272 240 -32
__io_apic_write 46 - -46
__io_apic_read 46 - -46
io_apic_set_pci_routing 985 930 -55
__io_apic_eoi.part 223 161 -62
io_apic_write 69 - -69
io_apic_read 69 - -69
restore_IO_APIC_setup 325 253 -72
ioapic_guest_write 1413 1333 -80
clear_IO_APIC_pin 447 343 -104
setup_IO_APIC 5148 4988 -160
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
There is no need for this to be an indirect jump at all. Execution always
returns to a specific location, so use a direct jump instead.
Use a named label for the jump. As both 1 and 2 have disappeared as labels,
rename 3 to skip to better describe its purpose.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
There are several cases where the act of interrupting a remote processor has
the required side effect. Explicitly allow NULL function pointers so the
calling code doesn't have to provide a stub implementation.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
CONFIG_ACPI_INTERPRETER does not get defined anywhere, the enclosed code
wouldn't build, and the default-to-phys logic works differently anyway
(see genapic/bigsmp.c:probe_bigsmp()).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The name hasn't been appropriate for a long time: It covers not only
command line overrides, but also x2APIC pre-enabled state.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich
Andrew Cooper
Oleksandr Andrushchenko
Luca Fancellu
Michal Orzel
Stewart Hildebrand
Roger Pau Monné
Juergen Gross
Ayan Kumar Halder
Ian Jackson
Vikram Garhwal