Wei Liu [Fri, 29 Apr 2016 15:11:14 +0000 (16:11 +0100)]
rombios/tcgbios: initialise logdataptr in HashLogEvent32
Gcc complains:
tcgbios.c: In function ‘HashLogEvent32’:
tcgbios.c:1131:10: error: ‘logdataptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
entry = tcpa_extend_acpi_log(logdataptr);
It fails to figure out when logdataptr is used it is always initialised
in a if block a few line above.
Signed-off-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Wei Liu [Fri, 29 Apr 2016 15:11:12 +0000 (16:11 +0100)]
rombios/tcgbios: initialise size in tcpa_extend_acpi_log
Gcc complains:
tcgbios.c:362:3: error: ‘size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
memcpy((char *)lasa_last, (char *)entry_ptr, size);
It fails to figure out if size is used in memcpy it is always initialised.
Signed-off-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Doug Goldstein [Thu, 5 May 2016 20:18:09 +0000 (15:18 -0500)]
init: shebang should be the first line
The shebang was not on the first line in the init script and it should
be.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Doug Goldstein [Thu, 5 May 2016 20:18:08 +0000 (15:18 -0500)]
init: drop GNU-isms for sleep command
Most implementations of the sleep command only take integers. GNU
coreutils has a GNU extension to allow any floating point number to be
passed but we shouldn't depend on that.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Signed-off-by: Doug Goldstein <cardoe@cardoe.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Paul Lai [Wed, 4 May 2016 15:54:07 +0000 (08:54 -0700)]
build: Honor '--enable-githttp' in toplevel Makefile generation
During the make world, git mini-os.git didn't honor the 'configure
--enable-githttp' option. The 'enable-githttp' was only honored in
the tools subdirectory.
Signed-off-by: Paul Lai <paul.c.lai@intel.com>
[ wei: add prefix "build:" to title ] Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Dario Faggioli [Tue, 3 May 2016 21:46:50 +0000 (23:46 +0200)]
xen: credit2: fix 2 (minor) issues in load tracking logic
All calculations that involve load_last_update uses quantities
shifted by LOADAVG_GRANULARITY_SHIFT, so make sure that this
is true even when the field is assigned a value for the first
time, during vcpu allocation.
Also, during migration, while the loads of both the source and
destination runqueues certainly need changing, the vcpu being
moved does not change its running/non-running status, and its
calculated load should hence not be affected.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Dario Faggioli [Tue, 3 May 2016 21:46:42 +0000 (23:46 +0200)]
xen: sched: fix killing an uninitialized timer in free_pdata.
commit 64269d9365 "sched: implement .init_pdata in Credit,
Credit2 and RTDS" helped fixing Credit2 runqueues, and
the races we had in switching scheduler for pCPUs, but
introduced another issue. In fact, if CPU bringup fails
during __cpu_up() (and, more precisely, after CPU_UP_PREPARE,
but before CPU_STARTING) the CPU_UP_CANCELED notifier
would be executed, which calls the free_pdata hook.
Such hook does, right now, two things: (1) undo the
initialization done inside the init_pdata hook and (2)
free the memory allocated by the alloc_pdata hook.
However, in the failure path just described, it is possible
that only alloc_pdata were called, and this is potentially
an issue (depending on how actually free_pdata does).
In fact, for Credit1 (the only scheduler that actually
allocates per-pCPU data), this result in calling kill_timer()
on a timer that had not yet been initialized, which causes
the following:
Solve this by making the scheduler hooks API symmetric again,
i.e., by adding a deinit_pdata hook and making it responsible
of undoing what init_pdata did, rather than asking to free_pdata
to do everything.
This is cleaner and, in the case at hand, makes it possible to
only call free_pdata (which is the right thing to do) as only
allocation and no initialization was performed.
Reported-by: Julien Grall <julien.grall@arm.com> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Anthony PERARD [Tue, 3 May 2016 15:59:49 +0000 (16:59 +0100)]
configure: Fix when no libsystemd compat lib are available
From systemd change log, since version 209, libsystemd.so contain
everything, including libsystemd-daemon.so. Distro may, or may not provide
the compatibility libraries which libsystemd-daemon is part of.
So, if libsystemd-daemon is not available, check for the presence of
a recent enough libsystemd.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
[ wei: run autogen.sh ] Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Roger Pau Monne [Tue, 3 May 2016 10:55:07 +0000 (12:55 +0200)]
tools/xsplice: fix mixing system errno values with Xen ones.
Avoid using system errno values when comparing with Xen errno values.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Roger Pau Monne [Tue, 3 May 2016 10:55:06 +0000 (12:55 +0200)]
tools/xsplice: corrently use errno
Some error paths incorrectly used rc instead of errno.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Roger Pau Monne [Tue, 3 May 2016 10:55:05 +0000 (12:55 +0200)]
libxl: add a define for equivalent ENODATA errno on FreeBSD
Currently FreeBSD lacks the ENODATA errno value, so the privcmd driver
always translates ENODATA to ENOENT, add a define to libxl in order to
correctly match ENODATA with ENOENT on FreeBSD.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
xen/arm64: ensure that the correct SP is used for exceptions
The ARMv8 architecture has a SPSel ("stack pointer selection") machine
register that allows us to determine which exception level's stack
pointer is loaded when an exception occurs. As we don't want to
use the non-privileged SP_EL0 stack pointer -- or even assume that SP_EL0
points to a valid address in the hypervisor context-- we'll need to ensure
that our EL2 code sets the SPSel to SP_ELn mode, so exceptions that trap
to EL2 use the EL2 stack pointer.
This corrects an issue that can manifest as a hang-on-IRQ on some
arm64 cores if the firmware/bootloader has previously initialized SPSel
to 0; in which case Xen's exceptions will incorrectly use an invalid SP_EL0,
and will endlessly spin on the synchronous abort handler.
Roger Pau Monné [Wed, 4 May 2016 07:46:57 +0000 (09:46 +0200)]
xsplice: check against ELFOSABI_NONE instead of ELFOSABI_SYSV
They are equivalent, but using ELFOSABI_NONE is more correct in this
context.
Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Wed, 4 May 2016 07:44:32 +0000 (09:44 +0200)]
IOMMU/x86: per-domain control structure is not HVM-specific
... and hence should not live in the HVM part of the PV/HVM union. In
fact it's not even architecture specific (there already is a per-arch
extension type to it), so it gets moved out right to common struct
domain.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Julien Grall <julien.grall@arm.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Jan Beulich [Wed, 4 May 2016 07:43:37 +0000 (09:43 +0200)]
x86/p2m: also tear down altp2m
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com> Acked-by: George Dunlap <george.dunlap@citrix.com>
Razvan Cojocaru [Wed, 4 May 2016 07:42:06 +0000 (09:42 +0200)]
x86/monitor: disallow setting mem_access_emulate_each_rep when vm_event is NULL
It is meaningless (and potentially dangerous - see hvmemul_virtual_to_linear())
to set mem_access_emulate_each_rep before xc_monitor_enable() (which allocates
vcpu->arch.vm_event) has been called, so return an error from the
XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP hypercall when that is the case.
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citirx.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
David Vrabel [Tue, 3 May 2016 16:15:38 +0000 (17:15 +0100)]
x86: show correct code in CPU state
When showing the CPU state (e.g., after a crash) the dump of code
around RIP is incorrect.
Incorrect:
Xen code around <ffff82d0801113cf> (...):
00 c6 c1 ee 08 48 c1 e0 <04> 03 04 f1 8b ...
^^ Uninitialized ^^ Missing 0x48
Correct:
Xen code around <ffff82d0801113cf> (...):
c6 c1 ee 08 48 c1 e0 04 <48> 03 04 f1 8b ...
When coping the bytes before RIP, the destination was off-by-one.
Signed-off-by: David Vrabel <david.vrabel@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reported-by: Jan Beulich <JBeulich@suse.com> Acked-by: Jan Beulich <JBeulich@suse.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
ocaml/xc_get_cpu_featureset/arm: Return not implemented on ARM
... as it is not implemented on it.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Jan Beulich [Mon, 2 May 2016 07:20:17 +0000 (09:20 +0200)]
x86/shadow: account for ioreq server pages before complaining about not found mapping
prepare_ring_for_helper(), just like share_xen_page_with_guest(),
takes a write reference on the page, and hence should similarly be
accounted for when determining whether to log a complaint.
This requires using recursive locking for the ioreq server lock, as the
offending invocation of sh_remove_all_mappings() is down the call stack
from hvm_set_ioreq_server_state(). (While not strictly needed to be
done in all other instances too, convert all of them for consistency.)
At once improve the usefulness of the shadow error message: Log all
values involved in triggering it as well as the GFN (to aid
understanding which guest page it is that there is a problem with - in
cases like the one here the GFN is invariant across invocations, while
the MFN obviously can [and will] vary).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Tim Deegan <tim@xen.org> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Wei Liu [Fri, 29 Apr 2016 17:25:31 +0000 (18:25 +0100)]
mkelf32: fix compilation on 32 bit build host
When cross-compiling xen on a 32 bit build host:
boot/mkelf32.c: In function 'main':
boot/mkelf32.c:360:21: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'Elf64_Off' [-Werror=format]
cc1: all warnings being treated as errors
Fix that by using PRId64 in format string.
Signed-off-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
George Dunlap [Wed, 27 Apr 2016 16:00:37 +0000 (17:00 +0100)]
MAINTAINERS: Clarify the meaning of nested maintainership
Clarify the meaning of nested maintainership.
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
We had a discussion about the meaning of nested maintainership at the
recent Xen Hackathon. The notes of that meeting can be found on this
list [1]. No decision is official until discussed on this list, so
consider this patch the official proposal for this change, and object
or ask for clarification accordingly.
Compared to v1, there is one change that is worth pointing out: The
claim that THE REST consists of all committers. This is the case at
the moment, but this change would codify that this is an invariant we
intend to keep going forward.
The advantage of this is that the dispute resolution mentioned in this
patch for maintainers who can't agree lines up directly with the
fall-back for broader community issues upon which we can't reach
consensus.
Changes in v2:
- fixed spelling of "maintainer"
- fixed path of multi.c
- clarified that the resolution by REST would be by *majority* vote
- Asserted that The REST consists of all committers
CC: Ian Jackson <ian.jackson@eu.citrix.com> CC: Jan Beulich <jbeulich@suse.com> CC: Keir Fraser <keir@xen.org> CC: Tim Deegan <tim@xen.org> CC: Wei Liu <wei.liu2@citrix.com> CC: Konrad Wilk <konrad.wilk@oracle.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: Lars Kurth <lars.kurth@citrix.com>
Jan Beulich [Fri, 29 Apr 2016 16:28:41 +0000 (18:28 +0200)]
x86/vMSI-X: also snoop REP MOVS
... as at least certain versions of Windows use such to update the
MSI-X table. However, to not overly complicate the logic for now
- only EFLAGS.DF=0 is being handled,
- only updates not crossing MSI-X table entry boundaries are handled.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
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>
MAINTAINERS/xsplice: Add myself and Ross as the maintainers.
If you have a patch for xSplice send it our way!
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Ross Lagerwall [Wed, 6 Apr 2016 19:15:01 +0000 (15:15 -0400)]
xsplice: Prevent duplicate payloads from being loaded.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
[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>
XENVER_build_id/libxc: Provide ld-embedded build-id
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>
xsplice: Print build_id in keyhandler and on bootup.
As it should be an useful debug mechanism.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.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:
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>
Ross Lagerwall [Sat, 9 Apr 2016 14:07:21 +0000 (10:07 -0400)]
xsplice: Add support for alternatives
Add support for applying alternative sections within xsplice payload.
At payload load time, apply an alternative sections that are found.
Also we add an test-case exercising a rather useless alternative
(patching a NOP with a NOP) - but it does exercise the code-path.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Ross Lagerwall [Wed, 20 Apr 2016 20:20:26 +0000 (16:20 -0400)]
xsplice: Add support for exception tables.
Add support for exception tables contained within xSplice payloads. If an
exception occurs search either the main exception table or a particular
active payload's exception table depending on the instruction pointer.
Also we add an test-case to make sure we have an exception that
is handled.
To not grow the code-base if xSplice is not compiled in we add
certain #define to help in determining if code needs to be __init
or not.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Ross Lagerwall [Wed, 27 Apr 2016 15:30:54 +0000 (11:30 -0400)]
xsplice: Add support for bug frames.
Add support for handling bug frames contained with xsplice modules. If a
trap occurs search either the kernel bug table or an applied payload's
bug table depending on the instruction pointer.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Ross Lagerwall [Wed, 27 Apr 2016 15:30:25 +0000 (11:30 -0400)]
x86, xsplice: Print payload's symbol name and payload name in backtraces
Naturally the backtrace is presented when an instruction
hits an bug_frame or %p is used.
The payloads do not support bug_frames yet - however the functions
the payloads call could hit an BUG() or WARN().
The traps.c has logic to scan for it this - and eventually it will
find the correct bug_frame and the walk the stack using %p to print
the backtrace. For %p and symbols to print a string - the
'is_active_kernel_text' is consulted which uses an 'struct virtual_region'.
Therefore we register our start->end addresses so that
'is_active_kernel_text' will include our payload address.
We also register our symbol lookup table function so that it can
scan the list of payloads and retrieve the correct name.
Lastly we change vsprintf to take into account s and namebuf.
For core code they are the same, but for payloads they are different.
This gets us:
Which is great if payloads have similar or same symbol names.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
xsplice, symbols: Implement fast symbol names -> virtual addresses lookup
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):
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).
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>
Ross Lagerwall [Wed, 20 Apr 2016 20:19:27 +0000 (16:19 -0400)]
xsplice,symbols: Implement symbol name resolution on address.
If in the payload we do not have the old_addr we can resolve
the virtual address based on the UNDEFined symbols.
We also use an boolean flag: new_symbol to track symbols. The usual
case this is used is by:
* A payload may introduce a new symbol
* A payload may override an existing symbol (introduced in Xen or another
payload)
* Overriding symbols must exist in the symtab for backtraces.
* A payload must always link against the object which defines the new symbol.
Considering that payloads may be loaded in any order it would be incorrect to
link against a payload which simply overrides a symbol because you could end
up with a chain of jumps which is inefficient and may result in the expected
function not being executed.
Since the payload we get is an relocatable image (partial linked ELF file)
we have to match up the symbols. We follow the ELF visibility rules for that
and for local symbols do what bintutils ld does.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.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>
x86/xen_hello_world.xsplice: Test payload for patching 'xen_extra_version'.
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>
Ross Lagerwall [Wed, 27 Apr 2016 13:07:04 +0000 (09:07 -0400)]
xsplice: Implement support for applying/reverting/replacing patches.
Implement support for the apply, revert and replace actions.
To perform and action on a payload, the hypercall sets up a data
structure to schedule the work. A hook is added in the reset_stack_and_jump
to check for work and execute it if needed (specifically we check an
per-cpu flag to make this as quick as possible).
In this way, patches can be applied with all CPUs idle and without
stacks. The first CPU to run check_for_xsplice_work() becomes the
master and triggers a reschedule softirq to trigger all the other CPUs
to enter check_for_xsplice_work() with no stack. Once all CPUs
have rendezvoused, all CPUs disable their IRQs and NMIs are ignored.
The system is then quiscient and the master performs the action.
After this, all CPUs enable IRQs and NMIs are re-enabled.
Note that it is unsafe to patch do_nmi and the xSplice internal functions.
Patching functions on NMI/MCE path is liable to end in disaster on x86.
This is not addressed in this patch and is mentioned in the
design doc as a further TODO.
The action to perform is one of:
- APPLY: For each function in the module, store the first arch-specific
number bytes of the old function and replace it with a jump to the
new function. (on x86 it is 5 bytes, on ARM it will likey be 4 bytes).
- REVERT: Copy the previously stored bytes into the first arch-specific
number of bytes of the old function (again, 5 bytes on x86).
- REPLACE: Revert each applied module and then apply the new module.
To prevent a deadlock with any other barrier in the system, the master
will wait for up to 30ms before timing out.
Measurements found that the patch application to take about 100 μs on a
72 CPU system, whether idle or fully loaded.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: Julien Grall <julien.grall@arm.com> Acked-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Ross Lagerwall [Wed, 27 Apr 2016 13:01:51 +0000 (09:01 -0400)]
xsplice: Implement payload loading
Add support for loading xsplice payloads. This is somewhat similar to
the Linux kernel module loader, implementing the following steps:
- Verify the elf file.
- Parse the elf file.
- Allocate a region of memory mapped within a free area of
[xen_virt_end, XEN_VIRT_END].
- Copy allocated sections into the new region. Split them in three
regions - .text, .data, and .rodata. MUST have at least .text.
- Resolve section symbols. All other symbols must be absolute addresses.
(Note that patch titled "xsplice,symbols: Implement symbol name resolution
on address" implements that)
- Perform relocations.
- Secure the the regions (.text,.data,.rodata) with proper permissions.
We capitalize on the vmalloc callback API (see patch titled:
"rm/x86/vmap: Add v[z|m]alloc_xen, and vm_init_type") to allocate
a region of memory within the [xen_virt_end, XEN_VIRT_END] for the code.
We also use the "x86/mm: Introduce modify_xen_mappings()"
to change the virtual address page-table permissions.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: Julien Grall <julien.grall@arm.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Ross Lagerwall [Fri, 19 Feb 2016 19:37:17 +0000 (14:37 -0500)]
xsplice: Add helper elf routines
Add Elf routines and data structures in preparation for loading an
xSplice payload.
We make an assumption that the max number of sections an ELF payload
can have is 64. We can in future make this be dependent on the
names of the sections and verifying against a list, but for right now
this suffices.
Also we a whole lot of checks to make sure that the ELF payload
file is not corrupted nor that the offsets point past the file.
For most of the checks we print an message if the hypervisor is built
with debug enabled.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Reviewed-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>
arm/x86: Use struct virtual_region to do bug, symbol, and (x86) exception tables lookup.
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>
xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op
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>
Jan Beulich [Thu, 28 Apr 2016 13:10:45 +0000 (15:10 +0200)]
x86/vMSI-X: also snoop qword writes
... the high half of which may be a write to the Vector Control field.
This gets things in sync again with msixtbl_write().
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Jan Beulich [Thu, 28 Apr 2016 13:10:22 +0000 (15:10 +0200)]
x86/vMSI-X: add further checks to snoop logic
msixtbl_range(), as any other MMIO ->check() handlers, may get called
with other than the base address of an access - avoid the snoop logic
considering those.
Also avoid considering vCPU-s not blocked in the hypervisor in
msixtbl_pt_register(), just to be on the safe side.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Jan Beulich [Thu, 28 Apr 2016 13:09:26 +0000 (15:09 +0200)]
x86/HVM: fix forwarding of internally cached requests (part 2)
Commit 96ae556569 ("x86/HVM: fix forwarding of internally cached
requests") wasn't quite complete: hvmemul_do_io() also needs to
propagate up the clipped count. (I really should have re-tested the
forward port resulting in the earlier change, instead of relying on the
testing done on the older version of Xen which the fix was first needed
for.)
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
public/x86: remove HVMMEM_mmio_write_dm from the public interface
HVMMEM_mmio_write_dm is removed for new xen interface versions, and
is replaced with type HVMMEM_unused. Attempts to set a page to this
type will return -EINVAL in xen after 4.7.0. And there will be no
pages with type p2m_mmio_write_dm, therefore HVMOP_get_mem_type will
never get the old type - HVMMEM_mmio_write_dm.
New approaches to write protect guest ram pages will be provided in
future patches.
Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
The current test performed in order to check if the assembler supports
certain instructions doesn't take into account the value of AFLAGS, which
when using clang contains the option that disables the integrated assembler
due to the lack of features.
As a result of this, the current instruction tests were performed against the
integrated assembler, but then at build time the non-integrated assembler
was used. If both have feature-parity, this is a non-issue, but we cannot
assume this.
Fix this by passing AFLAGS in the instruction test, and including the arch
Rules.mk makefile after AFLAGS is set.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Thu, 28 Apr 2016 13:06:56 +0000 (15:06 +0200)]
x86/time: fix gtime_to_gtsc for vtsc=1 PV guests
For vtsc=1 PV guests, rdtsc is trapped and calculated from get_s_time()
using gtime_to_gtsc. Similarly the tsc_timestamp, part of struct
vcpu_time_info, is calculated from stime_local_stamp using
gtime_to_gtsc.
However gtime_to_gtsc can return 0, if time < vtsc_offset, which can
actually happen when gtime_to_gtsc is called passing stime_local_stamp
(the caller function is __update_vcpu_system_time).
In that case the pvclock protocol doesn't work properly and the guest is
unable to calculate the system time correctly. As a consequence when the
guest tries to set a timer event (for example calling the
VCPUOP_set_singleshot_timer hypercall), the event will be in the past
causing Linux to hang.
The purpose of the pvclock protocol is to allow the guest to calculate
the system_time in nanosec correctly. The guest calculates as follow:
Given that with vtsc=1:
rdtsc = to_vtsc_scale(NOW() - vtsc_offset)
vcpu_time_info.tsc_timestamp = to_vtsc_scale(vcpu_time_info.system_time - vtsc_offset)
The expression evaluates to NOW(), which is what we want. However when
stime_local_stamp < vtsc_offset, vcpu_time_info.tsc_timestamp is
actually 0. As a consequence the calculated overall system_time is not
correct.
This patch fixes the issue by letting gtime_to_gtsc return a negative
integer in the form of a wrapped around unsigned integer, thus when the
guest subtracts vcpu_time_info.tsc_timestamp from rdtsc will calculate
the right value.
Signed-off-by: Jan Beulich <JBeulich@suse.com> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> Release-acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper [Wed, 27 Apr 2016 13:06:04 +0000 (14:06 +0100)]
travis: Enable tools when building with clang
tools now build under clang, so let them be tested.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Doug Goldstein <cardoe@cardoe.com>
Andrew Cooper [Wed, 27 Apr 2016 16:10:57 +0000 (17:10 +0100)]
travis: Remove clang-3.8 build
The package appears to have been renamed in Ubuntu. The only reason this test
is currently passing is because the hypervisor build falls back to clang, at
version 3.5
Add an explicit test in the build script that out desired compiler is
available. Note that travis already performs this step, but in a way which
isn't fatal to the build.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by Doug Goldstein <cardoe@cardoe.com>
Andrew Cooper [Wed, 27 Apr 2016 12:58:27 +0000 (13:58 +0100)]
tools/kdd: Fix uninitialised variable warning
Clang warns:
kdd.c:1031:9: error: variable 'fd' is used uninitialized whenever '||'
condition is true [-Werror,-Wsometimes-uninitialized]
if (argc != 4
^~~~~~~~~
kdd.c:1040:20: note: uninitialized use occurs here
if (select(fd + 1, &fds, NULL, NULL, NULL) > 0)
^~
This situation can't actually happen, as usage() is a terminal path. Annotate
it appropriately.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Andrew Cooper [Wed, 27 Apr 2016 12:31:05 +0000 (13:31 +0100)]
tools/blktap2: Fix use of uninitialised variable in _tap_list_join3()
Clang points out:
tap-ctl-list.c:457:28: error: variable 'entry' is uninitialized when
used here [-Werror,-Wuninitialized]
for (; *_entry != NULL; ++entry) {
^~~~~
The content of that loop clearly was meant to iterate over _entry rather than
entry, so is fixed to do so. This presumably fixes a memory leak when
tapdisks get orphed, as only the first item on the list got freed.
There is no use of entry at all. It is referenced in a
list_for_each_entry(tl, &tap->list, entry) construct, but this is just a
member name, and not a reference to local scope variable of the same name.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Andrew Cooper [Wed, 27 Apr 2016 10:23:04 +0000 (11:23 +0100)]
tools/blktap2: Fix array initialisers for tapdisk_disk_{types,drivers}[]
Clang points out:
tapdisk-disktype.c:117:2: error: initializer overrides prior initialization
of this subobject [-Werror,-Winitializer-overrides]
0,
^
tapdisk-disktype.c:115:23: note: previous initialization is here
[DISK_TYPE_VINDEX] = &vhd_index_disk,
^~~~~~~~~~~~~~~
Mixing different initialiser styles should be avoided; The actual behaviour is
different to the expected behaviour. This specific example has been broken
since its introduction in c/s 7b4dea554 "blktap2: Fix tapdisk disktype issues"
in 2010, and is caused by the '#if 0' block removing &tapdisk_{sync,vmdk}.
First of all, remove what were intended to be trailing NULL entries in
tapdisk_disk_{types,drivers}[], making consistent use of Designated
Initialisers for the initialisation.
This requires changing the loop in tapdisk_disktype_find() to be based on the
number of elements in tapdisk_disk_types[], rather than looking for the first
NULL. This fixes a latent bug, as the use of Designated Initializers causes
to intermediate zero entries if not all indices are explicitly specified.
There is a second latent bug where tapdisk_disktype_find() assumes that
tapdisk_disk_drivers[] has at least as many entries as tapdisk_disk_types[].
This is not the case and tapdisk_disk_drivers[] had one entry fewer than
tapdisk_disk_types[], but the NULL loop bound prevented an out-of-bounds read
of tapdisk_disk_drivers[]. Fix the issue by explicitly declaring
tapdisk_disk_drivers[] to have the same number of entries as
tapdisk_disk_types[].
Finally, this leads to a linker error. It turns out that tapdisk_vhd_index
doesn't exist, and I can't find any evidence in the source history to suggest
that it ever did. I can only presume that it would have been #if 0'd out like
tapdisk_sync and tapdisk_vmdk had it not been for this bug preventing a build
failure. Drop all three.
No functional change, but only because of the specific layout of
tapdisk_disk_types[].
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
src/xenstat.c:325:8: warning: comparison of 0 <= unsigned expression is
always true [-Wtautological-compare]
if (0 <= index && index < node->num_domains)
~ ^ ~~~~~
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
xen/arm: Force broadcast of TLB and instruction cache maintenance instructions
UP guest may use TLB instructions to flush only on the local CPU.
Therefore, TLB flush will not be broadcasted across all the CPUs within
the same innershareable domain.
When the vCPU is migrated between different CPUs, it may be rescheduled
to a previous CPU where the TLB has not been flushed. The TLB may
contain stale entries which will result to translate incorrectly a VA to
IPA or even cause TLB conflicts.
To avoid a such situation, it is possible to set HCR_EL2.FB, which will
force the broadcast of TLB and instruction cache maintenance instructions.
The performance impact of setting HCR_EL2.FB will depend on how often
a guest makes use of local flush instructions.
ARM64 Linux kernel is SMP-aware (no possibility to build only for UP).
Most of the flush instructions are innershareable. The local flushes are
limited to the boot (1 per CPU) and when a task is getting a new ASIC.
Therefore the impact of setting HCR.FB for those guests is very limited.
ARM32 Linux kernel offers the possibility to be built either for SMP or
UP. The number of local flush is very limited in the former kernel
whilst the latter will only issue local flushes. Therefore there will be
an impact to set HCR.FB for guest kernel only built for UP.
Note that the SMP kernel can run in a domain using 1 vCPU and it
will still make use of innershareable flush instruction.
Looking at other OSes, such as FreeBSD, they are very similar to ARM32
Linux kernel (i.e offering two configuration: SMP and UP).
However, nothing prevents an SMP-aware kernel to make more often use of
local flush instrutions.
In the case that HCR_EL2.FB is not set, Xen would need to:
* Flush all the TLBs for the VMID associated to this domain
* Invalidate all the entries from the branch predictor
* Invalidate all the entries from the instruction cache
Those actions would only be needed when the vCPU is migrating between 2
physical CPUs.
Whilst this solution would have a negative performance impact on kernels
which do not heavily use local flush instructions, this may improve
performance for kernels only built for UP system.
For now implement the easiest solution (i.e setting HCR_EL2.FB). We can
revisit it if the performance impact is too high for UP kernel.
xen/arm: traps: Correctly interpret the content of the register HPFAR_EL2
The register HPFAR_EL2 (resp. HPFAR on arm32) contains the bits [47:12]
(resp. [39:12]) of the faulting IPA. Unlike other registers that represent
an address, the upper bits of the IPA are stored in the register bits
[4:39] (resp. [4:21]).
However, Xen assumes that the register contains the faulting IPA correctly
offsetted. This will result to get a wrong IPA when the fault is happening
during a translation table walk. Note this is only affecting memaccess.
Introduce a new helper to get the faulting IPA from HPFAR_EL2 and
replace direct read from the register by the helper.
It is incorrect to add the LDFLAGS to the CFLAGS, and some compilers will
error out if linker flags are passed when creating object files. Fix this by
properly passing CFLAGS and LDFLAGS, instead of putting everything in
CFLAGS.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
oxenstored: fix error when shifting negative value
By explicitly casting it to unsigned.
Reasoning on why this is needed, provided by Andrew Cooper:
"Ocaml stores integers shifted left by one, and with the bottom bit set.
Values with the bottom bit clear are pointers into the GC'd heap. Values
with the bottom bit set are integers, and need to be shifted by 1 bit to
have calculations performed."
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
xen/tools: fix substitution of __align8__ uint64_t inside of headers
The current seedery doesn't work with BSD sed, so remove the try to match
int64_t also (since there's none at the moment). Also, apply the same
treatment to all arch headers, currently this is only done to x86_64
headers.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
tools/headers: prevent adding two __align8__ to uint64_t in ARM headers
Due to the fact that on ARM headers types are substituted to uint64_t and
then uint64_t is also substituted to contain the aligment, this would lead
to some types containing two __align8__ directives. Fix this by first
expanding Xen specific types to uint64_t only, and then replacing all the
uint64_t types to __align8__ uint64_t. This relies on the fact that all
Xen-specific types will have longer names, so they will always be replaced
first.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Doug Goldstein <cardoe@cardoe.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
build: set HOSTCXX based on clang value for Kconfig xconfig target
The xconfig Kconfig target requires a C++ compiler because it uses Qt.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
build: make HOSTCC conditional on the value of clang
Previously HOSTCC was always hardcoded to gcc
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Jan Beulich [Tue, 26 Apr 2016 14:53:36 +0000 (16:53 +0200)]
x86/vMSI-X: write snoops should ignore hvm_mmio_internal() requests
Those aren't actual I/O requests (and hence are of no interest here
anyway). Since they don't get copied into struct vcpu, looking at that
copy reads whatever was left there. Use the state of the request to
determine its validity.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Instead of trying to write a snippet of code that crashes the process
just use abort() directly. This is to fix the build on clang which
detects that the snippet of code will crash and fails to compile. At
the same time removed extraneous whitespace in the macro.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
For native (non-cross compiles) we now only require bcc, ld86, as86 for
building rombios, we can build the toolstack sans rombios and using the
system SeaBIOS due to known build issues. At the same time capture the
output of the configure scripts to help with tracking down future build
issues. This does not enable building of the toolstack with clang for
now due to multiple failures.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Andrew Cooper<andrew.cooper3@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
EFLAGS.DF can be assumed to be usually clear, so unlikely()-annotate
the conditionals accordingly.
Also prefer latching p->size (used twice) into a local variable, at
once making it unnecessary for the reader to be sure expressions get
evaluated left to right (operand promotion would yield a different
result if p->addr + p->size - 1 was evaluted right to left).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Andrew Cooper [Tue, 26 Apr 2016 11:46:26 +0000 (13:46 +0200)]
x86/mm: introduce modify_xen_mappings()
To simply change the permissions on existing Xen mappings. The existing
destroy_xen_mappings() is altered to support changing the PTE permissions.
A new destroy_xen_mappings() is introduced, as the special case of not passing
_PAGE_PRESENT to modify_xen_mappings().
As cleanup (and an ideal functional test), the boot logic which remaps Xen's
code and data with reduced permissions is altered to use
modify_xen_mappings(), rather than map_pages_to_xen() passing the same mfn's
back in.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
projects / xen.git / log