If the livepatch has only .rodata sections then it is OK to also
apply/revert/apply the livepatch without having to worry about the
unforseen consequences.
See commit 98b728a7b2
"livepatch: Disallow applying after an revert" for details.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
It makes more sense to put a tool to be used by other subsystems
to be in 'tools/misc' along 'mkrpm','mkdeb', etc.
The patch titled "xen/livepatch/x86/arm32: Force .livepatch.depends
section to be uint32_t aligned" uses mkhex.
Acked-by: Wei Liu <wei.liu2@citrix.com>
Suggested-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
If the .bug.frames.X or .livepatch.funcs sizes are different
than what the hypervisor expects - we fail the payload. To help
in diagnosing this include the expected and the payload
sizes.
Also make it more natural by having "Multiples" in the warning.
Also fix one case where we would fail if the size of the .ex_table
was being zero - but that is OK.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The ELF specification mentions nothing about the sh_size being
modulo the sh_addralign. Only that sh_addr MUST be aligned on
sh_addralign if sh_addralign is not zero or one.
We on loading did not take this in-to account so this patch adds
a check on the ELF file as it is being parsed.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
instead of hardcoding values. We also drop the uint64_t
cast as the macro uses ULL to produce the proper width
types.
Acked-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
If we have a large amount of livepatches and want to print them
on the console using 'xl debug-keys x' we eventually hit
the preemption check:
if ( i && !(i % 64) )
{
spin_unlock(&payload_lock);
process_pending_softirqs();
if ( spin_trylock(&payload_lock) )
return
<facepalm> The effect is that we have just effectively
taken the lock and returned without unlocking!
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-and-tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
* 'staging' of git://xenbits.xen.org/people/konradwilk/xen:
Introduce the pvcalls header
Introduce the Xen 9pfs transport header
xen: introduce a C99 headers check
ring.h: introduce macros to handle monodirectional rings with multiple req sizes
tmem: Parse UUIDs correctly.
tmem: Fix tmem-shared-auth 'auth' values
tmem: By default to join an shared pool it must be authorized.
xen/libxc: Move TMEM_AUTH to XEN_SYSCTL_TMEM_OP_SET_AUTH
xen/libcx/tmem: Replace TMEM_RESTORE_NEW with XEN_SYSCTL_TMEM_OP_SET_POOLS
displif: add ABI for para-virtual display
A simple
xl tmem-shared-auth -u 00000000-0000-000A-0000-000000000001 -A 0 0
resulted in uuid_low = 1 (correct) and uuid_high = 0 (umm?).
The issue was that for hex values above 'A' (or 'a') we forgot
to add 10.
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The hypervisor code (tmemc_shared_pool_auth) since the inception
would consider auth values of:
0 - to disable authentication!
1 - to enable authentication for the given UUID.
The docs have it the other way around, so lets fix it.
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Having an off by default option allowing guests to join
_any_ shared pool is not very secure.
Lets eliminate tmem_shared_auth bootup option (which
was disabled by default) and have the code force this by default.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
which surprisingly (or maybe not) looks like
XEN_SYSCTL_TMEM_OP_SET_POOLS.
This hypercall came about, as explained in docs/misc/tmem-internals.html:
When tmem was first proposed to the linux kernel mailing list
(LKML), there was concern expressed about security of shared ephemeral
pools. The initial tmem implementation only
required a client to provide a 128-bit UUID to identify a shared pool, and the
linux-side tmem implementation obtained this UUID from the superblock of the
shared filesystem (in ocfs2). It was
pointed out on LKML that the UUID was essentially a security key and any
malicious domain that guessed it would have access to any data from the shared
filesystem that found its way into tmem.
..
As a result, a Xen boot option -- tmem_shared_auth; -- was
added. The option defaults to disabled,
but when it is enabled, management tools must explicitly authenticate (or may
explicitly deny) shared pool access to any client.
On Xen, this is done with the xm tmem-shared-auth command.
"
However the implementation has some rather large holes:
a) The hypercall was accessed from any guest.
b) If the ->domain id value is 0xFFFF then one can toggle the
tmem_global.shared_auth knob on/off. That with a)
made it pretty bad.
c) If one toggles the tmem_global.shared_auth off, then the
'tmem_shared_auth=1' bootup parameter is ignored and
one can join any shared pool (if UUID is known)!
d) If the 'tmem_shared_auth=1' and tmem_global.shared_auth is
set to 1, then one can only join an shared pool if the
UUID has been set by 'xl tmem-shared-auth'. Otherwise
the joining of a pool fails and a non-shared pool is
created (without errors to guest). Not exactly sure if
the shared pool creation at that point should error out
or not.
e) If a guest is migrated, the policy values (which UUID
can be shared, whether tmem_global.shared_auth is set, etc)
are completely ignored.
This patch only fixes a) and only allows the hypercall to
be called by the control domain. Subsequent patches will
fix the remaining issues.
We also have to call client_create as the guest at this
point may not have done any tmem hypercalls - and hence
the '->tmem' from 'struct domain' is still NULL. Us calling
client_create fixes this.
Acked-by: Wei Liu <wei.liu2@citrix.com> [libxc changes]
Reviewed-by: Jan Beulich <jbeulich@suse.com> [hypervisor changes]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
This used to be done under TMEM_RESTORE_NEW which was an hypercall
accessible by the guest. However there are couple of reasons
not to do it:
- No checking of domid on TMEM_RESTORE_NEW which meant that
any guest could create TMEM pools for other guests.
- The guest can already create pools using TMEM_NEW_POOL
(which is limited to guest doing the hypercall)
- This functionality is only needed during migration - there
is no need for the guest to have this functionality.
However to move this we also have to allocate the 'struct domain'
->tmem pointer. It is by default set to NULL and would be initialized
via the guest do_tmem() hypercalls. Presumarily that was the
initial reason that TMEM_RESTORE_NEW was in the guest accessible
hypercalls.
Acked-by: Wei Liu <wei.liu2@citrix.com> [libxc change]
Reviewed-by: Jan Beulich <jbeulich@suse.com> [hypervisor changes]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
That way we have one person who can: a) poke other maintainers
or pull them in with new drivers are introduced, b) we have
one maintainer who can shepherd the patches along instead of
depending on the REST maintainers which may be busy with
other responsibilities.
Acked-by: Ian Jackson <ian.jackson@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
This is a follow-up to commit d14fffcc6a
"fix EFI part of "symbols: Generate an xen-sym.map" which fixed most of
the issues.
However we still have an issue - The file being installed (xen.efi.map)
does not exist in an ARM64 build (the xen.efi is linked againts xen).
The fix can be done two ways:
a) See if xen.efi.map exists and then copy it
b) Or link xen.efi.map to xen-syms.map (similar to how xen.efi is linked
against xen).
The patch chooses the former.
Reported-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
in one sub-call: XEN_SYSCTL_TMEM_OP_GET_POOLS.
These operations are used during the save process of migration.
Instead of doing 64 hypercalls lets do just one. We modify
the 'struct xen_tmem_client' structure (used in
XEN_SYSCTL_TMEM_OP_[GET|SET]_CLIENT_INFO) to have an extra field
'nr_pools'. Armed with that the code slurping up pages from the
hypervisor can allocate a big enough structure (struct tmem_pool_info)
to contain all the active pools. And then just iterate over each
one and save it in the stream.
We are also re-using one of the subcommands numbers for this,
as such the XEN_SYSCTL_INTERFACE_VERSION should be incremented
and that was done in the patch titled:
"tmem/libxc: Squash XEN_SYSCTL_TMEM_OP_[SET|SAVE].."
In the xc_tmem_[save|restore] we also added proper memory handling
of the 'buf' and 'pools'. Because of the loops and to make it as
easy as possible to review we add a goto label and for almost
all error conditions jump in it.
The include for inttypes is required for the PRId64 macro to
work (which is needed to compile this code under 32-bit).
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
That is what they are used for. Lets make it more clear.
Of all the various sub-commands, the only one that needed
semantic change is XEN_SYSCTL_TMEM_OP_SAVE_BEGIN. That in the
past used 'arg1', and now we are moving it to use 'arg'.
Since that code is only used during migration which is tied
to the toolstack it is OK to change it.
We should increment the XEN_SYSCTL_INTERFACE_VERSION because
of this, and that was fortunatly done in the patch titled:
"tmem/libxc: Squash XEN_SYSCTL_TMEM_OP_[SET|SAVE].."
While at it, also fix xc_tmem_control_oid to properly handle
the 'buf' and bounce it as appropiate.
Acked-by: Andrew cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
return values. For success they used to be 1 ([SAVE,RESTORE]_BEGIN),
0 if guest did not have any tmem (but only for SAVE_BEGIN), and
-1 for any type of failure.
And SAVE_END (which you would think would mirror SAVE_BEGIN)
had 0 for success and -1 if guest did not any tmem enabled for it.
This is confusing. Now the code will return 0 if the operation was
success. Various XEN_EXX values are returned if tmem is not enabled
or the operation could not performed.
The xc_tmem.c code only needs one place to check - where we use
SAVE_BEGIN. The place where RESTORE_BEGIN is used will have errno
with the proper error value and return will be -1, so will still
fail properly.
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Specifically:
XEN_SYSCTL_TMEM_OP_SET_[WEIGHT,COMPRESS] are now done via:
XEN_SYSCTL_TMEM_SET_CLIENT_INFO
and XEN_SYSCTL_TMEM_OP_SAVE_GET_[VERSION,MAXPOOLS,
CLIENT_WEIGHT, CLIENT_FLAGS] can now be retrieved via:
XEN_SYSCTL_TMEM_GET_CLIENT_INFO
All this information is now in 'struct xen_tmem_client' and
that is what we pass around.
We also rev up the XEN_SYSCTL_INTERFACE_VERSION as we are
re-using the value number of the deleted ones (and henceforth
the information is retrieved differently).
On the toolstack, prior to this patch, the xc_tmem_control
would use the bounce buffer only when arg1 was set and the cmd
was to list. With the 'XEN_SYSCTL_TMEM_OP_SET_[WEIGHT|COMPRESS]'
that made sense as the 'arg1' would have the value. However
for the other ones (say XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_UUID)
the 'arg1' would be the length of the 'buf'. If this
confusing don't despair, patch patch titled:
tmem/xc_tmem_control: Rename 'arg1' to 'len' and 'arg2' to arg.
takes care of that.
The acute reader of the toolstack code will discover that
we only used the bounce buffer for LIST, not for any other
subcommands that used 'buf'!?! Which means that the contents
of 'buf' would never be copied back to the calleer 'buf'!
The author is not sure how this could possibly work, perhaps Xen 4.1
(when this was introduced) was more relaxed about the bounce buffer
being enabled. Anyhow this fixes xc_tmem_control to do it for
any subcommand that has 'arg1'.
Lastly some of the checks in xc_tmem_[restore|save] are removed
as they can't ever be reached (not even sure how they could
have been reached in the original submission). One of them
is the check for the weight against -1 when in fact the
hypervisor would never have provided that value.
Now the checks are simple - as the hypercall always returns
->version and ->maxpools (which is mirroring how it was done
prior to this patch). But if one wants to check the if a guest
has any tmem activity then the patch titled
"tmem: Batch and squash XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_
[FLAGS,NPAGES,UUID] in one sub-call: XEN_SYSCTL_TMEM_OP_GET_POOLS."
adds an ->nr_pools to check for that.
Also we add the check for ->version and ->maxpools and remove
the TODO.
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
No functional change. We do this to prepare for another
entry to be added in the union. See patch titled:
"tmem/libxc: Squash XEN_SYSCTL_TMEM_OP_[SET|SAVE]"
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
in its own structure. This paves the way to make only one hypercall
to retrieve/set this information instead of multiple ones.
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Couple of reasons:
- It can lead to security issues (see row-hammer, KSM and such
attacks).
- Code is quite complex.
- Deduplication is good if the pages themselves are the same
but that is hardly guaranteed.
- We got some gains (if pages are deduped) but at the cost of
making code less maintainable.
- tze depends on deduplication code.
As such, deleting it.
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
It is not used by anything. Its intent was to complement
the 'weight' attribute but there hadn't been any request for this.
If there is a need to resurface it, it can be integrated back
via the XEN_SYSCTL_TMEM_SET_CLIENT_INFO introduced in
"tmem/libxc: Squash XEN_SYSCTL_TMEM_OP_[SET|SAVE].."
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The only thing this hypercall returns is TMEM_SPEC_VERSION.
The comment around is also misleading - this call does not
do any domain operation.
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The test-case is quite simple - we NOP the 'xen_minor_version'.
The amount of NOPs depends on the architecture.
On x86 the function is 11 bytes long:
55 push %rbp <- NOP
48 89 e5 mov %rsp,%rbp <- NOP
b8 04 00 00 00 mov $0x4,%eax <- NOP
5d pop %rbp <- NOP
c3 retq
We can NOP everything but the last instruction (so 10 bytes).
On ARM64 its 8 bytes:
52800100 mov w0, #0x8 <- NOP
d65f03c0 ret
We can NOP the first instruction.
While on ARM32 there are 24 bytes:
e52db004 push {fp} <- NOP
e28db000 add fp, sp, #0 <- NOP
e3a00008 mov r0, #8 <- NOP
e24bd000 sub sp, fp, #0 <- NOP
e49db004 pop {fp} <- NOP
e12fff1e bx lr
And we can NOP instructions 1 through 5.
Granted this code may be different per compiler!
Hence if anybody does run this test-case - they should
verify that the assumptions made here are correct.
Acked-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
It is exactly the same in both platforms.
No functional change.
Acked-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The patch piggybacks on: livepatch: Initial ARM64 support, which
brings up all of the necessary livepatch infrastructure pieces in.
This patch adds three major pieces:
1) ELF relocations. ARM32 uses SHT_REL instead of SHT_RELA which
means the adddendum had to be extracted from within the
instruction. Which required parsing BL/BLX, B/BL<cond>,
MOVT, and MOVW instructions.
The code was written from scratch using the ARM ELF manual
(and the ARM Architecture Reference Manual)
2) Inserting an trampoline. We use the B (branch to address)
which uses an offset that is based on the PC value: PC + imm32.
Because we insert the branch at the start of the old function
we have to account for the instruction already being fetched
and subtract -8 from the delta (new_addr - old_addr). See
ARM DDI 0406C.c, see A2.3 (pg 45) and A8.8.18 pg (pg 334,335)
3) Allows the test-cases to be built under ARM 32.
The "livepatch: tests: Make them compile under ARM64"
put in the right infrastructure for it and we piggyback on it.
Acked-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com> [for non-ARM parts]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
We need to two things:
1) Wrap the platform-specific objcopy parameters in defines
The input and output parameters for $(OBJCOPY) are different
based on the platforms. As such provide them in the
OBJCOPY_MAGIC define and use that.
2) The alternative is a bit different (exists only under ARM64
and x86), while and there are no exceptions under ARM at all.
We use the LIVEPATCH_FEATURE CPU id feature for ARM similar to
how it is done on x86.
We are not yet attempting to build them under ARM32 so
that is still ifdefed out.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To use as a common way of testing alternative patching for
livepatches. Both architectures have this FEATURE and the
test-cases can piggyback on that.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Suggested-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Certain platforms, such as ARM [32|64] add extra mapping symbols
such as $x (for ARM64 instructions), or more interesting to
this patch: $t (for Thumb instructions). These symbols are supposed
to help the final linker to make any adjustments (such as
add an veneer). But more importantly - we do not compile Xen
with any Thumb instructions (which are variable length) - and
if we find these mapping symbols we should disallow such payload.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Those symbols are used to help final linkers to replace insn.
The ARM ELF specification mandates that they are present
to denote the start of certain CPU features. There are two
variants of it - short and long format.
Either way - we can ignore these symbols.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> [x86 bits]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
If the distance is too big we are in trouble - as our relocation
distance can surely be clipped, or still have a valid width - but
cause an overflow of distance.
On various architectures the maximum displacement for a unconditional
branch/jump varies. ARM32 is +/- 32MB, ARM64 is +/- 128MB while x86
for 32-bit relocations is +/- 2G.
Note: On x86 we could use the 64-bit jmpq instruction which
would provide much bigger displacement to do a jump, but we would
still have issues with the new function not being able to reach
any of the old functions (as all the relocations would assume 32-bit
displacement). And "furthermore would require an register or
memory location to load/store the address to." (From Jan).
On ARM the conditional branch supports even a smaller displacement
but fortunately we are not using that.
Reviewed-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
As compared to x86 the va of the hypervisor .text
is locked down - we cannot modify the running pagetables
to have the .ro flag unset. We borrow the same idea that
alternative patching has - which is to vmap the entire
.text region and use the alternative virtual address
for patching.
Since we are doing vmap we may fail, hence the
arch_livepatch_quiesce was changed (see "x86,arm:
Change arch_livepatch_quiesce() declaration") to return
an error value which will be bubbled in payload->rc and
provided to the user (along with messages in the ring buffer).
The livepatch virtual address space (where the new functions
are) needs to be close to the hypervisor virtual address
so that the trampoline can reach it. As such we re-use
the BOOT_RELOC_VIRT_START which is not used after bootup
(alternatively we can also use the space after the _end to
FIXMAP_ADDR(0), but that may be too small).
The ELF relocation engine at the start was coded from
the "ELF for the ARM 64-bit Architecture (AArch64)"
(http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056b/IHI0056B_aaelf64.pdf)
but after a while of trying to write the correct bit shifting
and masking from scratch I ended up borrowing from Linux, the
'reloc_insn_imm' (Linux v4.7 arch/arm64/kernel/module.c function.
See 257cb251925f854da435cbf79b140984413871ac "arm64: Loadable modules")
And while at it - we also utilize code from Linux to construct
the right branch instruction (see "arm64/insn: introduce
aarch64_insn_gen_{nop|branch_imm}() helper functions").
In the livepatch payload loading code we tweak the #ifdef to
only exclude ARM_32. The exceptions are not part of ARM 32/64 hence
they are still behind the #ifdef.
We also expand the MAINTAINERS file to include the arm64 and arm32
platform specific livepatch file.
Acked-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com> [non-arm parts]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
That way we can print out the header if we are sure the
hypervisor has been compiled with Xen Livepatching.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Most of the WARN_ON or BUG_ON sections are properly aligned on
x86. However on ARM and on x86 assembler the macros don't include
any alignment information - hence they end up being the default
byte granularity.
On ARM32 it is paramount that the alignment is word-size (4)
otherwise if one tries to use (uint32_t*) access (such
as livepatch ELF relocations) we get a Data Abort.
Enforcing bug_frames to have the proper alignment across all
architectures and in both C and x86 makes them all the same.
Furthermore on x86 the bloat-o-meter detects that with this
change:
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
function old new delta
On ARM32:
add/remove: 1/0 grow/shrink: 0/1 up/down: 384/-288 (96)
function old new delta
gnttab_unpopulate_status_frames - 384 +384
do_grant_table_op 10808 10520 -288
And ARM64:
add/remove: 1/2 grow/shrink: 0/1 up/down: 4164/-4236 (-72)
function old new delta
gnttab_map_grant_ref - 4164 +4164
do_grant_table_op 9892 9836 -56
grant_map_exists 300 - -300
__gnttab_map_grant_ref 3880 - -3880
Reviewed-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com> [x86 parts]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
This is similar to commit fb9d877a9c
"xen/arm64: Add an helper to invalidate all instruction caches"
except it is on ARM32 side.
When we are flushing the cache we are most likely also want
to flush the branch predictor too. Hence we add this.
And we also need to follow this with dsb()/isb() which are
memory barriers().
Reviewed-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
So they can be shared with ARM64 (but not yet, so they
are only built on x86).
No functional change.
We also need to tweak the MAINTAINERS and .gitignore file.
Also we need to update SUBDIRS to include the new 'test'
directory so 'cscope' can show the example livepatches.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com> [arm change]
Acked-by: Jan Beulich <jbeulich@suse.com> [for directory]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The current byte sequence is '0xcc' which makes sense on x86,
but on ARM it is:
cccccccc stclgt 12, cr12, [ip], {204} ; 0xcc
Picking something more ARM applicable such as:
efefefef svc 0x00efefef
Creates a nice crash if one executes that code:
(XEN) CPU1: Unexpected Trap: Supervisor Call
But unfortunately that may not be a good choice either as in the future
we may want to implement support for it.
Julien suggested that we use a 4-byte insn instruction instead
of trying to work with one byte. To make sure nothing goes bad
we also require that the __init_[begin|end] be aligned properly.
As such on ARM 32 we use the udf instruction (see A8.8.247
in ARM DDI 0406C.c) and on ARM 64 use the AARCH64_BREAK_FAULT
instruction (aka brk instruction).
We don't have to worry about Thumb code so this instruction
is a safe to execute.
Reviewed-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
If the payload had the sections mentioned but the hypervisor
did not support some of them (say on ARM the .ex_table) - instead
of ignoring them - it should forbid loading of such payload.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
x86 implements all of them by default - and we just
add two extra HAS_ variables to be declared in autoconf.h.
ARM 64 only has alternative while ARM 32 has none of them.
And while at it change the livepatch common code that
would benefit from this.
Reviewed-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com> [relevant parts]
Suggested-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
No functional change. We resist the temptation to move
the entries in the Kconfig file to be more in alphabetical
order as the "arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE]"
will move one of the entries to common file.
Reviewed-by: Julien Grall <julien.grall@arm.com>
Suggested-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
With "livepatch: NOP if func->new_addr is zero." that name
makes no more sense as we also NOP now.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The NOP functionality will NOP any of the code at
the 'old_addr' or at 'name' if the 'new_addr' is zero.
The purpose of this is to NOP out calls, such as:
e8 <4-bytes-offset>
(5 byte insn), or on ARM a 4 byte insn for branching.
We need the EIP of where we need to the NOP, and that can
be provided via the `old_addr` or `name`.
If the `old_addr` is provided we will NOP 'new_size'
amount of bytes at that location.
The amount is up to 31 instructions if desired (which is
the size of the opaque member). If there is a need to NOP
more then: a) more 'struct livepatch_func' structures need
to be present, b) we have to implement a variable size
buffer (in the future), or c) first byte an unconditional
branch skipping the to be disabled code (of course provided
there are no branch targets in the middle).
While at it, also unify the code on x86 patching so
it is a bit simpler (instead of two seperate writes
just make it one memcpy).
And introduce a general livepatch_insn_len inline function
that would depend on platform specific instruction size
(for a unconditional branch). As such we also rename the
PATCH_INSN_SIZE to ARCH_PATCH_INSN_SIZE.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The initial patch: 11ff40fa7b
"xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op" caps the
size of the binary at 2MB. We follow that in capping the size
of the .BSSes to be at maximum 2MB.
We also bubble up the payload limit and this one in one #define
called LIVEPATCH_MAX_SIZE to make it easier to find these
arbitrary limits.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
On general this is unhealthy - as the payload's .bss (definitly)
or .data (maybe) will be modified once the payload is running.
Doing an revert and then re-applying the payload with a non-pristine
.bss or .data can lead to unforseen consequences (.bss are assumed
to always contain zero value but now they may have a different value).
There is one exception - if the payload contains only one .data section
- the .livepatch.funcs, then it is OK to re-apply an revert.
We detect this rather simply (if there is one RW section and its name
is .livepatch.funcs) - but the payload may have many other RW sections
that are not used at all (such as .bss or .data sections with zero
length). To not account those we also ignore sections with sh_size
being zero.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Right now the contents of 'name' are all located in
the .data section. We want them in the .rodata section
so change the type to have const on them.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
As currently during the injection of the build-id it ends up
being marked as AW. We want it to be read-only.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Which is only used by Livepatch code. The purpose behind
this call is to modify the page table entries flags.
Specifically the .ro and .nx flags. The current mechanism
puts cache attributes in the flags and the .ro and .nx are
locked down and assumed to be .ro=0, nx=1.
Livepatch needs .nx=0 and also .ro to be set to 1.
We introduce a new 'flags' where various bits determine
whether .ro and .nx bits are set or cleared. We can't use
an enum as the function prototype would diverge from x86.
Reviewed-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
On ARM we need an alternative VA region to poke in the
hypervisor .text data. And since this is setup during runtime
we may fail (it uses vmap so most likely error is ENOMEM).
As such this error needs to be bubbled up and also abort
the livepatching if it occurs.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
This is copied from Linux 4.7, and the initial commit
that put this in is 5c5bf25d4f7a950382f94fc120a5818197b48fe9
"arm64: introduce aarch64_insn_gen_{nop|branch_imm}() helper functions"
This lays the groundwork for Livepatch to generate the
trampoline to jump to the new replacement function.
Also allows us to NOP the callsites.
Acked-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
--
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
RFC: First submission
v1: The full copy of insn_gen_branch instead of just the code to make branch
v2: Added Julien's Ack.
Remove the duplicate paragraph in the commit message.
On x86 we squash 'apply_alternatives' in to
'alternative_instructions' (who was its sole user)
and 'apply_alternatives_nocheck' to 'apply_alternatives'.
On ARM we change the parameters for 'apply_alternatives'
to be of 'const struct alt_instr *' instead of void pointer and
size length.
We also add 'const' and make the arguments be on the
proper offset.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> [x86 bits]
Reviewed-by: Julien Grall <julien.grall@arm.com> [ARM bits]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
That way common code can use the same macro to access
the most common attributes without much #ifdef.
Take advantage of it right away in the livepatch code.
Note: on ARM we use tabs to conform to the style of the file.
Acked-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The checks for SHT_REL[,A] ELF sanity checks does not need to
be in the platform specific file and can be bubbled up
in the platform agnostic file.
This makes the ARM 32/64 implementation easier as the
duplicate checks don't have to be in the platform specific files.
Acked-by: Jan Beulich <jbeulich@suse.com> [x86 part]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
When doing cross-compilation we should use proper $(OBJDUMP).
Otherwise decompiling say ARM 32 code using x86 objdump
won't help much.
Acked-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
You could construct _most_ of the names of the functions
by doing 'nm --defined' but unfortunatly you do not get the
<file> prefix that is added on in Xen . For example:
$ cat xen-syms.symbols |grep do_domain_pause
0xffff82d080104920 t domain.c#do_domain_pause
$ nm --defined xen-syms|grep do_domain_pause
ffff82d080104920 t do_domain_pause
This is normally not an issue, but if one is doing livepatching and
wants during build-time verify that the symbols the livepatch payloads
will patch do correspond to the one the hypervisor has built - this helps a lot.
Note that during runtime one can do:
[root@localhost xen]# cat /proc/xen/xensyms |grep do_domain_pause
ffff82d080104920 t domain.c#do_domain_pause
But one may not want to build and verify a livepatch on the same host.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Specifically the code that is looking up f->old_addr - which
can be in its own routine instead of having it part of prepare_payload.
No functional change.
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Livepatch expected at some point to be able to print the
build-id during bootup, which it did not. The reason is
that xen_build_init and livepatch_init are both __initcall
type routines. This meant that when livepatch_init called
xen_build_id, it would return -ENODATA as build_id_len was
not setup yet (b/c xen_build_init would be called later).
The original patch fixed this by calling xen_build_init in
livepatch_init which allows us to print the build-id of
the hypervisor.
However the x86 maintainers pointed out that build-id
is independent of Livepatch and in fact should print
regardless whether Livepatch is enabled or not.
Therefore this patch moves the logic of printing the build-id
to version.c.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
It makes more sense for it to be there. However that
means the version.h has now a dependency on <xen/elfstructs.h>
as the Elf_Note is a macro.
The elfstructs.h has a dependency on types.h as well so
we need that. We cannot put that #include <xen/types.h>
in elfstructs.h as that file is used by tools and they
do not have such file.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
It is possible. Especially if the only thing they do is
NOP functions - in which case there is only .livepatch.funcs
sections.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
As the xen/COPYING file says:
"A few files are licensed under both GPL and a weaker BSD-style
license. This includes all files within the subdirectory
include/public, as described in include/public/COPYING. All such files
include the non-GPL license text as a source-code comment. Although
the license text refers generically to "the software", the non-GPL
license applies *only* to those source files that explicitly include
the non-GPL license text."
The libxenvchan.h is under xen/include/public/io directory
and the xen/include/public/COPYING says:
"XEN NOTICE
==========
This copyright applies to all files within this subdirectory and its
subdirectories:
include/public/*.h
include/public/hvm/*.h
include/public/io/*.h
The intention is that these files can be freely copied into the source
tree of an operating system when porting that OS to run on Xen. Doing
so does *not* cause the OS to become subject to the terms of the GPL.
All other files in the Xen source distribution are covered by version
2 of the GNU General Public License except where explicitly stated
otherwise within individual source files.
"
Having the libxenvchan.h as Lesser GPL v2.1 where the COPYING file
says otherwise is confusing to say at least.
Upon consulting with the authors of libxenvchan they said:
"FWIW Neither I, nor ITL staff (as author of original libvchan library)
have anything against converting it to the BSD-style licence."
(Marek Marczykowski-Górecki,
http://lists.xen.org/archives/html/xen-devel/2016-06/msg00995.html)
so as such lets change it.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Anil Madhavapeddy <anil@recoil.org>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: George Dunlap <George.Dunlap@eu.citrix.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
Acked-by: Jason Andryuk <andryuk@aero.org>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Matthew Daley <mattjd@gmail.com>
Acked-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Roger Pau Monne <roger.pau@entel.upc.edu>
Acked-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
["I have spoken to my line manager. I can confirm that Citrix is happy
with this proposed change. So:
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
This view from Citrix covers all contributions made to these files in
the course of Citrix's employees' employment, which I think is:
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Cc: Roger Pau Monne <roger.pau@entel.upc.edu>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
..
[in subsequent email]:
Wei points out that this ought also to include Keir Fraser's
contribution, which was (only) in 2012.
" (from Ian's email)
In a subsequent mail, Wei also points out that David Scott's
contribution is covered by Ian's ack.
]
The functionality that is related to migration is left inside
tmem.c. The list of control operations that are in tmem_control
with XEN_SYSCTL_TMEM_OP prefix are:
DESTROY, FLUSH, FREEZE, THAW, LIST, QUERY_FREEABLE_MB
SAVE_GET_CLIENT_CAP, SAVE_GET_CLIENT_FLAGS,
SAVE_GET_CLIENT_WEIGHT, SAVE_GET_MAXPOOLS,
SAVE_GET_POOL_FLAGS, SAVE_GET_POOL_NPAGES
SAVE_GET_POOL_UUID, SAVE_GET_VERSION
SET_CAP, SET_COMPRESS, SET_WEIGHT
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Put them all in one structure to make it easier to
figure out what can be removed. The structure is called
'tmem_global' as it will be eventually non-static.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
The macros: atomic_inc_and_max and atomic_dec_and_assert
use also the 'stats' to access them. Had to open-code
access to pool->pgp_count as it would not work anymore.
No functional change.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
And adjust the macro: atomic_inc_and_max to update the structure.
Sadly one entry: pool->pgp_count cannot use this macro anymore
so unroll the macro for this instance.
No functional change. The name has the 'tmem_stats' as it will
be eventually non-local.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Specifically:
s/\.xsplice/\.livepatch/
s/XSPLICE/LIVEPATCH/
s/xsplice/livepatch/
s/livepatch_patch_func/livepatch_func/
s/xSplice/Xen Live Patch/
s/livepatching/livepatch/
s/arch_livepatch_enter/arch_livepatch_quiesce/
s/arch_livepatch_exit/arch_livepatch_revive/
And then modify some of the function arguments
to have two more characters.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Currently it is possible to:
1) xc_xsplice_apply()
\-> xsplice_action
spin_lock(payload_lock)
\- schedule_work()
spin_unlock(payload_lock);
2) xc_xsplice_unload()
\-> xsplice_action
spin_lock(payload_lock)
free_payload(data);
spin_unlock(payload_lock);
.. all CPUs are quiesced.
3) check_for_xsplice_work()
\-> apply_payload
\-> arch_xsplice_apply_jmp
BOOM
The reason is that state is in 'CHECKED' which changes to 'APPLIED'
once check_for_xsplice_work finishes. So we have a race between 1) -> 3)
where one can manipulate the payload.
To guard against this we add a check in xsplice_action to not allow
any actions if schedule_work has been called for this specific payload.
The function 'is_work_scheduled' checks xsplice_work which is safe as:
- The ->do_work changes to 1 under the payload_lock (which we also hold).
- The ->do_work changes to 0 when all CPUs are quisced and IRQs have
been disabled.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reported-and-Tested-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
With this third payload one can do:
-bash-4.1# xen-xsplice load xen_hello_world.xsplice
Uploading xen_hello_world.xsplice (10148 bytes)
Performing check: completed
Performing apply:. completed
[xen_hello_world depends on hypervisor build-id]
-bash-4.1# xen-xsplice load xen_bye_world.xsplice
Uploading xen_bye_world.xsplice (7076 bytes)
Performing check: completed
Performing apply:. completed
[xen_bye_world depends on xen_hello_world build-id]
-bash-4.1# xen-xsplice upload xen_replace_world xen_replace_world.xsplice
Uploading xen_replace_world.xsplice (7148 bytes)
-bash-4.1# xen-xsplice list
ID | status
----------------------------------------+------------
xen_hello_world | APPLIED
xen_bye_world | APPLIED
xen_replace_world | CHECKED
-bash-4.1# xen-xsplice replace xen_replace_world
Performing replace:. completed
-bash-4.1# xl info | grep extra
xen_extra : Hello Again World!
-bash-4.1# xen-xsplice list
ID | status
----------------------------------------+------------
xen_hello_world | CHECKED
xen_bye_world | CHECKED
xen_replace_world | APPLIED
and revert both of the previous payloads and apply
the xen_replace_world.
All the magic of this is in the Makefile - we extract
the build-id from the hypervisor (xen-syms) and jam it
in the xen_replace_world as .xsplice.depends.
We also make .old_addr be zero, forcing the hypervisor
to lookup the xen_extra_version.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
We now expect that the ELF payloads be built with the
--build-id.
Also the .xsplice.deps section has to have the contents
of the hypervisor (or a preceding payload) build-id.
We already have the code to verify the Elf_Note build-id
so export parts of it.
This dependency means the hypervisor MUST be compiled with
--build-id - so we gate the build of xSplice on the availability
of said functionality.
This does not impact the ordering of how the payloads can
be loaded, but it does enforce an STRICT ordering when the
payloads are applied. Also the REPLACE is special - we need
to check that its dependency against the hypervisor - not
the last applied patch.
To make this easier to test we also add an extra test-case
to be used - which can only be applied on top of the
xen_hello_world payload.
As in, one can apply xen_hello_world and then xen_bye_world
on top of that. Not the other way.
We also print the dependency and payloads build_in the keyhandler.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
If the hypervisor was built with build-ids we can expose the
build-id value to the toolstack (if it is not built with
it will just return -ENODATA). This is a priviligied operation
so only the controlling stack is able to request this.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
This patch enables the Elf to be built with the build-id
and provide in the Xen hypervisor the code to extract it.
The man-page for ld --build-id says it is:
"Request the creation of a ".note.gnu.build-id" ELF note
section or a ".build-id" COFF section. The contents of the
note are unique bits identifying this linked file. style can be
"uuid" to use 128 random bits, "sha1" to use a 160-bit SHA1 hash
on the normative parts of the output contents, ..."
One can also retrieve the value of the build-id by doing
'readelf -n xen-syms'.
For EFI builds we re-use the same build-id that the xen-syms
was built with.
The version of ld that first implemented --build-id is v2.18.
We check for to see if the linker supports the --build-id
parameter and if so use it.
For x86 we have two binaries - the xen-syms and the xen - an
smaller version with lots of sections removed. To make it possible
for readelf -n xen we also modify mkelf32 and xen.lds.S to include
the PT_NOTE ELF section.
The EFI binary is more complicated. We only build one type of
binary and expanding the amount of sections the EFI binary has to
include an .note one is pointless - as there is no concept of
PT_NOTE. The best we can do is move this .note in the .rodata section.
Further development wise should move it to .buildid section
so that DataDirectory debug data nor CodeView can view it.
(The author has no clue what those are).
Note that in earlier patches the linker script had:
__note_gnu_build_id_start = .;
*(.rodata.note.gnu.build-id)
__note_gnu_build_id_end = .;
*(.note)
*(.note.*)
Which meant you could have different ELF notes _outside_ the
__note_gnu_build_id_end. However for EFI builds we take the whole
.note* section and jam it in the EFI to be between
__note_gnu_build_id_start and __note_gnu_build_id_end.
To not make this happend we make on the ELF build the section
be called .note.gnu.build-id (instead of just .note).
If there is a need for a different type of note other folks
can add it as a different section name.
Note that we do call --binary-id=sha1 on all linker invocations.
We have to do to enforce that the symbol offsets don't changes
(the side effect is that we we would have multiple binary ids -
except that the last one is the final one).
Without this working the symbol table embedded in Xen ends
up incorrect - some of the values it contains would be offset by the
size of the included build id.
This obviously causes problems when resolving symbols.
We also define the NT_GNU_BUILD_ID in the elfstructs.h as we
need to use it in various places.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Martin Pohlack <mpohlack@amazon.de>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
The current mechanism is geared towards fast virtual address ->
symbol names lookup. This is fine for the normal use cases
(BUG_ON, WARN_ON, etc), but for xSplice - where we need to find
hypervisor symbols - it is slow.
To understand this patch, a description of the existing
method is explained first. For folks familar go to 'NEW CODE:'.
HOW IT WORKS:
The symbol table lookup mechanism uses a simple encoding mechanism
where it extracts the common ascii characters that the symbol's use.
This saves us space. The lookup mechanism is geared towards looking
up symbols based on address. We have one 0..N (where N is
the number of symbols, so 6849 for example) table:
symbols_addresses[0..N]
And an 1-1 (in a loose fashion) of the symbols (encoded) in a
symbols_names stream of size N.
The N is variable (later on that below)
The symbols_names are sorted based on symbols_addresses, which
means that the decoded entries inside symbols_names are not in
ascending or descending order.
There is also the encoding mechanism - the table of 255 entries
called symbols_token_index[]. And the symbols_token_table which
is an stream of ASCIIZ characters, such as (it really
is not a table as the values are variable):
@0 .asciz "credit"
@6 .asciz "mask"
..
@300 .asciz "S"
And the symbols_token_index:
@0 .short 0
@1 .short 7
@2 .short 12
@4 .short 16
...
@84 .short 300
The relationship between them is that the symbols_token_index
gives us the offset to symbols_token_table.
The symbol_names[] array is a stream of encoded values. Each value
follows the same pattern - <len> followed by <encoding values>.
And the another <len> followed by <encoding values>.
Hence to find the right one you need to read <len>, add <len>
(to skip over), read <len>, add <len>, and so on until one
finds the right tuple offset.
The <encoding values> are the indicies into the symbols_token_index.
Meaning if you have:
0x04, 0x54, 0xda, 0xe2, 0x74
[4, 84, 218, 226, 116 in human numbering]
The 0x04 tells us that the symbol is four bytes past this one (so next
symbol offset starts at 5). If we lookup symbols_token_index[84] we get 300.
symbols_token[300] gets us the "S". And so on, the string eventually
end up being decode to be 'S_stext'. The first character is the type,
then optionally follwed by the filename (and # right after filename)
and then lastly the symbol, such as:
tvpmu_intel.c#core2_vpmu_do_interrupt
Keep in mind that there are two fixed sized tables:
symbols_addresses[0..symbols_num_syms], and
symbols_markers[0..symbols_num_syms/255].
The symbols_markers is used to speed searching for the right address.
It gives us the offsets within symbol_names that start at the <len><encoded value>.
The way to find a symbol based on the address is:
1) Figure out the 'tuple offset' from symbols_address[0..symbols_num_syms].
This table is sorted by virtual addresses so finding the value is simple.
2) Get starting offset of symbol_names by retrieving value of
symbol_markers['tuple offset' / 255].
3). Iterate up to 'tuple_offset & 255' in symbols_markers stream starting
at 'offset'.
4). Decode the <len><encoded value>
This however does not work very well if we want to search the other
way - we have the symbol name and want to find the address.
NEW CODE:
To make that work we add one fixed size table called symbols_sorted_offsets which
has two elements: offset in symbol stream, offset in the symbol-address.
This whole array is sorted on the original symbol name during build-time
(in case of collision we also take into account the type).
The values are for example:
symbols_sorted_offsets:
.long 83363, 6302 # [.bss, len=5]
.long 80459, 6084 # [.data, len=5]
..
[The # added for clarity]
Which makes it incredibly easy to get in the symbols_names and also
symbols_addresses (or symbols_offsets)
Searching for symbols is simplified as we can do a binary search
on symbols_sorted_offsets. Since the symbols are sorted it takes on
average 13 calls to symbols_expand_symbol.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
This change demonstrates how to generate an xSplice ELF payload.
The idea here is that we want to patch in the hypervisor
the 'xen_version_extra' function with an function that will
return 'Hello World'. The 'xl info | grep extraversion'
will reflect the new value after the patching.
To generate this ELF payload file we need:
- C code of the new code (xen_hello_world_func.c).
- C code generating the .xsplice.funcs structure
(xen_hello_world.c)
- The address of the old code (xen_extra_version). We
retrieve it by using 'nm --defined' on xen-syms.
- The size of the new and old code for which we use
nm --defined -S on our code and xen-syms respectively.
There are two C files and one header files generated
during build. One could make this one C file if the
size of the newly patched function size was known in
advance (or an random value was choosen).
There is also a strict order of compiling:
1) xen_hello_world_func.c
2) config.h - extract the size of the new function,
the old function and the old function address.
3) xen_hello_world.c - which contains the .xsplice.funcs
structure.
4) Link the object files in an xen_hello_world.xsplice file.
The use-case is simple:
$xen-xsplice load /usr/lib/debug/xen_hello_world.xsplice
$xen-xsplice list
ID | status
----------------------------------------+------------
xen_hello_world APPLIED
$xl info | grep extra
xen_extra : Hello World
$xen-xsplice revert xen_hello_world
Performing revert: completed
$xen-xsplice unload xen_hello_world
Performing unload: completed
$xl info | grep extra
xen_extra : -unstable
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com> [ARM]
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
For those users who want to use the virtual addresses that
are in the hypervisor's code/data region address space -
these three new functions allow that.
Implementation wise the vmap API keeps track of two virtual
address regions now:
a) VMAP_VIRT_START
b) Any provided virtual address space (need start and end).
The a) one is the default one and the existing behavior
for users of vmalloc, vmap, etc is the same.
If however one wishes to use the b) one only has to use
the vm_init_type to initialize and the vmzalloc_xen to utilize
it (vfree and vunmap are capable of searching both address spaces).
This allows users (such as xSplice) to provide their own
mechanism to change the the page flags, and also use virtual
addresses closer to the hypervisor virtual addresses (at least
on x86) while not having to deal with the allocation of
pages.
For example of users, see patch titled "xsplice: Implement payload
loading", where we parse the payload's ELF relocations - which
is defined to be signed 32-bit (on x86) (max displacement hence
is 2GB virtual space, ARM32 is 128MB). The displacement of the
hypervisor virtual addresses to the vmalloc (on x86)
is more than 32-bits - which means that ELF relocations would
truncate the 34 and 33th bit. Hence this alternate API.
We also add add extra checks in case the b) range has not been
initialized.
Part of this patch also removes 'vm_alloc' and 'vm_free'
decleration as we do not have any users of it.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Suggested-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien.grall@arm.com> [ARM]
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
During execution of the hypervisor we have two regions of
executable code - stext -> _etext, and _sinittext -> _einitext.
The later is not needed after bootup.
We also have various built-in macros and functions to search
in between those two swaths depending on the state of the system.
That is either for bug_frames, exceptions (x86) or symbol
names for the instruction.
With xSplice in the picture - we need a mechanism for new payloads
to searched as well for all of this.
Originally we had extra 'if (xsplice)...' but that gets
a bit tiring and does not hook up nicely.
This 'struct virtual_region' and virtual_region_list provide a
mechanism to search for the bug_frames, exception table,
and symbol names entries without having various calls in
other sub-components in the system.
Code which wishes to participate in bug_frames and exception table
entries search has to only use two public APIs:
- register_virtual_region
- unregister_virtual_region
to let the core code know.
If the ->lookup_symbol is not then the default internal symbol lookup
mechanism is used.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com> [ARM]
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
A simple tool that allows an system admin to perform
basic xsplice operations:
- Upload a xsplice file (with an unique name)
- List all the xsplice payloads loaded.
- Apply, revert, replace, or unload the payload using the
unique name.
- Do all two - upload, and apply the payload in one go (load).
Also will use the name of the file as the <name>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
The underlaying toolstack code to do the basic
operations when using the XEN_XSPLICE_op syscalls:
- upload the payload,
- get status of an payload,
- list all the payloads,
- apply, check, replace, and revert the payload.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
The implementation does not actually do any patching.
It just adds the framework for doing the hypercalls,
keeping track of ELF payloads, and the basic operations:
- query which payloads exist,
- query for specific payloads,
- check*1, apply*1, replace*1, and unload payloads.
*1: Which of course in this patch are nops.
The functionality is disabled on ARM until all arch
components are implemented.
Also by default it is disabled until the implementation
is in place.
We also use recursive spinlocks to so that the find_payload
function does not need to have a 'lock' and 'non-lock' variant.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
A mechanism is required to binarily patch the running hypervisor with new
opcodes that have come about due to primarily security updates.
This document describes the design of the API that would allow us to
upload to the hypervisor binary patches.
This document has been shaped by the input from:
Martin Pohlack <mpohlack@amazon.de>
Jan Beulich <jbeulich@suse.com>
Thank you!
Input-from: Martin Pohlack <mpohlack@amazon.de>
Input-from: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
This reverts commit 2716d87537.
As it was decided that the existing XENVER hypercall - while having
grown organically over the years can still be expanded.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Requested-and-acked-by: Jan Beulich <jbeulich@suse.com>
Drop paranthesis and function pointer on nmi_callback_t typedef.
Make it more inline with how x86 maintainers want function
typedefs to be.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
It takes a bit of thinking to parse the original statement. It looks
like it is missing an ',' - right after the 'braced' to make it
obvious that:
if (X)
return XYZ;
is OK, while multiple conditionals require braces.
Changing the 'apart from' with 'except' makes it more obvious
(I hope).
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
We change the xen_version libxc code to use the new hypercall.
Which of course means every user in the code base has to
be changed over.
It is important to note that the xc_version_op has a different
return semantic than the previous one. It returns negative
values on error (like the old one), but it also returns
an positive value on success (unlike the old one). The positive
value is the number of bytes copied in.
Note that both Ocaml and xenstat use tabs instead of four
spaces so they look quite odd.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com> [for the Ocaml stubs]
Acked-by: George Dunlap <george.dunlap@eu.citrix.com> [xenctx bits]
Acked-by: Wei Liu <wei.liu2@citrix.com>
This hypercall mirrors the XENVER_ in that it has similar functionality.
However it is designed differently:
- No compat layer. The data structures are the same size on 32
as on 64-bit.
- The hypercall accepts three arguments - the command, pointer to
an buffer, and the length of the buffer.
- Each sub-ops can be "probed" for size by returning the size of
buffer that will be needed - if the buffer is NULL.
- Subops can complete even if the buffer is too small - truncated
data will be filled and hypercall will return -ENOBUFS.
- VERSION_commandline, VERSION_changeset are privileged.
- There is no XENVER_compile_info equivalent.
- The hypercall can return -EPERM and toolstack/OSes are expected
to deal with. However there are three subops: XEN_VERSION_version,
XEN_VERSION_platform_parameters and XEN_VERSION_get_features
that will always return an value as guests cannot survive without them.
While we combine some of the common code between XENVER_ and VERSION_
take the liberty of moving pae_extended_cr3 in x86 area.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> [XSM bits]
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
* 'pin' of https://github.com/jgross1/xen:
libxl: add force option for xl vcpu-pin
libxl: print message how to recover from xl cpupool-cpu-remove errors
libxc: do some retries in xc_cpupool_removecpu() for EBUSY case
All patches have Acked-by and Reviewed-by tags.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
So that we have a nice mechansim to figure out the upper
bounds of bug.frames and also catch compiler errors in case
one tries to use a higher frame number.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
v3: First time included.
v4: Add BUG_FRAME check also in the assembler version of the macro.
v5: Add Acks, make BUILD_BUG_ON checks look correct. Position the
BUGFRAME_NR properly. Reposition the BUGFRAME_NR again.
---
Most of XENVER_* have now an XSM check for their sub-ops.
The subop for XENVER_commandline is now a priviliged operation.
To not break guests we still return an string - but it is
just '<denied>\0'.
The XENVER_[version|platform_parameters|get_features] - will
always return an value to the guest.
The rest: XENVER_[extraversion|capabilities|page_size|
guest_handle|changeset| compile_info] behave as before -
allowed by default for all guests if using the XSM default
policy or with the dummy one. And if the system admin
wants to curtail access to some of them - they can do
that now with a non-default XSM policy.
Also we add a local variable block.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
v2: Do XSM check for all the XENVER_ ops.
- Add empty data conditions.
- Return <denied> for priv subops.
- Move extraversion from priv to normal. Drop the XSM check
for the non-priv subops.
v3:
- Add +1 for strlen(xen_deny()) to include NULL. Move changeset,
compile_info to non-priv subops.
- Remove the \0 on xen_deny()
- Add new XSM domain for xenver hypercall. Add all subops to it.
- Remove the extra line, Add Ack from Daniel
v4:
- Rename the XSM from xen_version_op to xsm_xen_version.
Prefix the types with 'xen' to distinguish it from another
hypercall performing similar operation. Removed Ack from Daniel
as it was so large. Add local variable block.
v5:
- Make XENVER_platform_parameters,get_features,version be excluded
from the XSM check per Jans' review. Add BUILD_BUG_CHECK and fix
odd line removals. Remove stray changes and fix spelling.
The commit bcfaea685d
"arm/config: Declare ELFSIZE_64" was not correct.
For 32-bit ARM, ELFCLASS32 (i.e. 32-bit data types) will always
be used so we need to set ELFSIZE to 32.
Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Julien Grall <julien.grall@arm.com>
The vmap_to_page macro (three levels deep!) can come up with
a NULL pointer. Lets add the proper ASSERT to catch this errant
behavior.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
* 'tracing/sched-events-improvements' of git://xenbits.xen.org/people/dariof/xen:
xenalyze: handle Credit2 scheduler events
xenalyze: handle Credit1 scheduler events
xenalyze: handle scheduling events
xentrace: formats: add domain create and destroy events.
xentrace: formats: add events from RTDS scheduler
xentrace: formats: add events from Credit2 scheduler
xentrace: formats: add events from Credit scheduler
xentrace: formats: update format of scheduling events
On the mailing list all patches have:
Acked-by: George Dunlap <george.dunlap@citrix.com>
and have been
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
so in they go.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
