Both Intel and AMD manuals agree that in x2APIC mode, the APIC LDR and ID
registers are derivable from each other through a fixed formula.
Xen uses that formula, but applies it to vCPU IDs (which are sequential)
rather than x2APIC IDs (which are not, at the moment). As I understand it,
this is an attempt to tightly pack vCPUs into clusters so each cluster has
16 vCPUs rather than 8, but this is a spec violation.
This patch fixes the implementation so we follow the x2APIC spec for new
VMs, while preserving the behaviour (buggy or fixed) for migrated-in VMs.
While touching that area, remove the existing printk statement in
vlapic_load_fixup() (as the checks it performed didn't make sense in x2APIC
mode and wouldn't affect the outcome) and put another printk as an else
branch so we get warnings trying to load nonsensical LDR values we don't
know about.
Fixes: f9e0cccf7b ("x86/HVM: fix ID handling of x2APIC emulation")
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
<asm/delay.h> only declares udelay() function so udelay()
declaration was moved to xen/delay.h.
For x86, __udelay() was renamed to udelay() and removed
inclusion of <asm/delay.h> in x86 code.
For ppc, udelay() stub definition was moved to ppc/stubs.c.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
The comment-based justifications for MISRA C:2012 Rule 8.4 are replaced
by the asmlinkage pseudo-attribute, for the sake of uniformity.
asmlinkage may in the future have the effect of altering the calling
convention, therefore three special where the calling convention is
altered by another macro retain the textual deviation.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Static analysis tools may detect a possible null pointer
dereference of 'config'. This ASSERT helps them in detecting
that such a condition is not possible given that only
real domains can enter this branch, which are guaranteeed to have
a non-NULL config at this point, but this information is not
inferred by the tool.
Checking that the condition given in the assertion holds via
testing is the means to protect release builds, where the assertion
expands to effectively nothing.
Suggested-by: Julien Grall <julien@xen.org>
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Introduce a Kconfig for the dom0less feature, enabled by default,
to be able to choose if the feature should be compiled or not.
Provide static inline stubs when the option is disabled for the
functions externally visible.
Use the new Kconfig to remove dom0less DT binding from the efi-boot.h
code when the Kconfig is not enabled, do the same for
allocate_bank_memory inside domain_build.c that currently is used
only by dom0less-build.c module, but it's kept there provisioning
its usage by dom0 code.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Move static memory and static shared memory code in separate modules
so that they are included only when the corresponding feature is
enabled, doing that we modularise the features and we remove some
ifdefs from the code to improve readability.
Move process_shm_node function from bootfdt module and make it
externally visible.
A static inline helper called process_shm_chosen is introduced, it
will call the process_shm function for the '/chosen' node, and will
be used by the function construct_dom0 instead of using directly
process_shm, allowing some #ifdef to be removed.
No functional changes are intended.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Currently the dom0less feature code is mostly inside domain_build.c
and setup.c, it is a feature that may not be useful to everyone so
put the code in a different compilation module in order to make it
easier to disable the feature in the future.
Move gic_interrupt_t in domain_build.h to use it with the function
declaration, move its comment above the declaration.
The following functions are now visible externally from domain_build
because they are used also from the dom0less-build module:
- get_allocation_size
- set_interrupt
- domain_fdt_begin_node
- make_memory_node
- make_resv_memory_node
- make_hypervisor_node
- make_psci_node
- make_cpus_node
- make_timer_node
- handle_device_interrupts
- construct_domain
- process_shm
- allocate_bank_memory
The functions allocate_static_memory and assign_static_memory_11
are now externally visible, so put their declarations into
domain_build.h and move the #else and stub definition in the header
as well.
Move is_dom0less_mode from setup.c to dom0less-build.c and make it
externally visible.
The function allocate_bank_memory is used only by dom0less code
at the moment, but it's been decided to leave it in domain_build.c
in case that in the future the dom0 code can use it.
Where spotted, fix code style issues.
No functional change is intended.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
The 'enum domain_type' is defined by 'asm/domain.h' which is not
included (directly or indirectly) by 'asm/kernel.h'.
This currently doesn't break the compilation because asm/domain.h will
included by the user of 'kernel.h'. But it would be better to avoid
relying on it. So add the include in 'asm/domain.h'.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Introduce Kconfig GICV2 to be able to compile the GICv2 driver only
when needed, the option is active by default.
Introduce Kconfig VGICV2 that compiles the Generic Interrupt
Controller v2 emulation for domains, it is required only when using
GICv2 driver, otherwise using the GICv3 driver it is optional and can
be deselected if the user doesn't want to offer the v2 emulation to
domains or maybe its GICv3 hardware can't offer the GICv2 compatible
mode.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
... to evaluate to false at compile-time when the respective Kconfig
control is off, thus allowing the compiler to eliminate then-dead code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Edit casts that unnecessarily remove const qualifiers
to comply with Rule 11.8.
The type of the provided pointer may be const qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Drop an unnecessary cast discarding a const qualifier, to comply with
Rule 11.8. The type of the formal parameter ivhd_block is const
qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Add const qualifier in cast that unnecessarily removes it
to comply with Rule 11.8.
The variable info is declared with a const qualified type.
No functional change.
Signed-off-by: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Currently the livepatch logic inside of Xen will use fields of struct
livepatch_func in order to cache internal state of patched functions. Note
this is a field that is part of the payload, and is loaded as an ELF section
(.livepatch.funcs), taking into account the SHF_* flags in the section
header.
The flags for the .livepatch.funcs section, as set by livepatch-build-tools,
are SHF_ALLOC, which leads to its contents (the array of livepatch_func
structures) being placed in read-only memory:
Section Headers:
[Nr] Name Type Address Offset
Size EntSize Flags Link Info Align
[...]
[ 4] .livepatch.funcs PROGBITS 0000000000000000 00000080
0000000000000068 0000000000000000 A 0 0 8
This previously went unnoticed, as all writes to the fields of livepatch_func
happen in the critical region that had WP disabled in CR0. After 8676092a0f
however WP is no longer toggled in CR0 for patch application, and only the
hypervisor .text mappings are made write-accessible. That leads to the
following page fault when attempting to apply a livepatch:
----[ Xen-4.19-unstable x86_64 debug=y Tainted: C ]----
CPU: 4
RIP: e008:[<ffff82d040221e81>] common/livepatch.c#apply_payload+0x45/0x1e1
[...]
Xen call trace:
[<ffff82d040221e81>] R common/livepatch.c#apply_payload+0x45/0x1e1
[<ffff82d0402235b2>] F check_for_livepatch_work+0x385/0xaa5
[<ffff82d04032508f>] F arch/x86/domain.c#idle_loop+0x92/0xee
Pagetable walk from ffff82d040625079:
L4[0x105] = 000000008c6c9063 ffffffffffffffff
L3[0x141] = 000000008c6c6063 ffffffffffffffff
L2[0x003] = 000000086a1e7063 ffffffffffffffff
L1[0x025] = 800000086ca5d121 ffffffffffffffff
****************************************
Panic on CPU 4:
FATAL PAGE FAULT
[error_code=0003]
Faulting linear address: ffff82d040625079
****************************************
Fix this by moving the internal Xen function patching state out of
livepatch_func into an area not allocated as part of the ELF payload. While
there also constify the array of livepatch_func structures in order to prevent
further surprises.
Note there's still one field (old_addr) that gets set during livepatch load. I
consider this fine since the field is read-only after load, and at the point
the field gets set the underlying mapping hasn't been made read-only yet.
Fixes: 8676092a0f ('x86/livepatch: Fix livepatch application when CET is active')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Add missing parameter names and uniform the interfaces of
modify_xen_mappings() and modify_xen_mappings_lite().
No functional change.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
The objective is to use parameter name "nf" to denote "new flags"
in all the modify_xen_mappings* functions.
Since modify_xen_mappings_lite() is currently using "nf" as identifier
for a local variable, bad things could happen if new uses of such
variable are committed while a renaming patch is waiting for the
approval.
To avoid such danger, as first thing rename the local variable from
"nf" to "flags".
No functional change.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
In case it's not possible to enable memory sharing (mem_sharing_control
fails) we just return the error code without releasing the domain
acquired some lines above by rcu_lock_live_remote_domain_by_id().
Fixes: 72f8d45d69 ("x86/mem_sharing: enable mem_sharing on first memop")
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
The various definitions of macro CNT (and the related BUILD_BUG_ON)
can be rewritten using ISOLATE_LSB, encapsulating a violation of
MISRA C:2012 Rule 10.1.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
The definition of PDX_GROUP_COUNT causes violations of
MISRA C:2012 Rule 10.1, therefore the problematic part now uses
the ISOLATE_LSB macro, which encapsulates the pattern.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Everywhere else the VL tests are grouped with the basic ones,
distinguished simply by the "form" specifiers.
No change to the generated test blobs, and hence no functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The env variable must be interpreted as an integer. As it is, the override
logic simply causes an exception.
Fixes: e0342ae555 ("tools/pygrub: Deprivilege pygrub")
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This is important in order for every mount done inside a mount namespace to
go away after the namespace itself goes away. The comment referring to
unreliability in Linux 4.19 was just wrong.
This patch sets the story straight and makes the depriv pygrub a bit more
confined should a layer of the onion be vulnerable.
Fixes: e0342ae555 ("tools/pygrub: Deprivilege pygrub")
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Add a local .gitignore file for tools/python.
As at least on some systems (e.g. OpenSUSE Leap 15.5) the build will
produce a tools/python/xen.egg-info directory, add it to the new
.gitignore file, too.
Use "/dir/" as pattern for ignoring a directory and its contents, as
the "/dir/*" pattern won't ignore the directory itself (git just
doesn't complain when seeing a directory without any not ignored file
in it).
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Add a local .gitignore file for tools/pygrub.
As at least on some systems (e.g. OpenSUSE Leap 15.5) the build will
produce a tools/pygrub/pygrub.egg-info directory, add it to the new
.gitignore file, too.
Use "/dir/" as pattern for ignoring a directory and its contents, as
the "/dir/*" pattern won't ignore the directory itself (git just
doesn't complain when seeing a directory without any not ignored file
in it).
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
With all architectures no longer wanting an image name in the command line
handling, drop the parameter and forgo making one up.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>
The differences between inline, __inline and __inline__ keywords (as far as
GCC is concenred at least) are a vestigial remnant of older C standards, and
in Xen we use inline almost exclusively.
Replace __inline and __inline__ with regular inline, and remove their
exceptions from the MISRA configuration.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Simone Ballarin <simone.ballarin@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The codebase is now -Wwrite-strings clean. Activate the option to cause
string literals to have a const type, and prevent any violations of MISRA Rule
7.4 being reintroduced.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
-Wwrite-strings is unhappy with assigning "xen" to a mutable pointer, but this
logic looks incorrect. It was inherited from the x86 side, where the logic
was redundant and has now been removed.
In the ARM case it inserts the image name into "xen,xen-bootargs" and there is
no logic at all to strip this before parsing it as the command line.
The absence of any logic to strip an image name suggests that it shouldn't
exist there, or having a Xen image named e.g. "hmp-unsafe" in the filesystem
is going to lead to some unexpected behaviour on boot.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Julien Grall <jgrall@amazon.com>
-Wwrite-strings is unhappy with assigning "xen" to a mutable pointer, but all
this work is useless; it's just prepending the image name which cmdline_cook()
intentionally strips back out (and allocating memory to do so).
Simply forgo the work and identify EFI_LOADER as one of the loaders which
doesn't prepend the image name.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Rework the logic in __start_xen() to not potentially pass NULL into
cmdline_cook(). This makes the logic easier to follow too, and the rest of
__start_xen() is safe when initialising cmdline to the empty string.
Update cmdline_cook() to take and return const pointers, and write a
description of what it does. It now requires a non-NULL input and guarentees
to return a pointer somewhere in the 'p' string.
Note this only compiles because strstr() launders the const off the pointer
when assigning to the mutable kextra, but that logic only mutates the
mbi->cmdline buffer.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
There's a confusing mix of variables; a static dom0_cmdline[], and a cmdline
pointer which points to image->string before being pointed at the static
buffer in order to be passed into construct_dom0(). cmdline being a mutable
pointer falls over -Wwrite-strings builds.
Delete the cmdline pointer, and rename dom0_cmdline[] to cmdline extending it
to have full function scope.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
GICD_CTL_ENABLE is a GICv2 bit. Remove it. The definitions of
GICD_CTL_ENABLE and GICD_CTLR_ENABLE_G1 are identical, so the value
written is unchanged.
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Alejandro Vallejo