Jan Beulich [Thu, 4 Jul 2019 14:07:01 +0000 (16:07 +0200)]
x86/vPIC: avoid speculative out of bounds accesses
Array indexes used in the I/O port read/write emulation functions are
derived from guest controlled values. Where this is not already done,
restrict their ranges to limit the side effects of speculative execution.
This is part of the speculative hardening effort.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Thu, 4 Jul 2019 14:06:27 +0000 (16:06 +0200)]
x86/vMSI: avoid speculative out of bounds accesses
Array indexes used in the MMIO read/write emulation functions are
derived from guest controlled values. Restrict their ranges to limit the
side effects of speculative execution.
Note that the index into .msi_ad[] may also be speculatively out of
bounds, by exactly one (indexes 0...3 are possible while the array has
just 3 elements). This is not a problem with the current data layout, as
such overrun of the array would either touch the next element of the
parent array or (for the last entry of the parent array) access the
subsequent acc_valid bit array.
This is part of the speculative hardening effort.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Thu, 4 Jul 2019 14:05:18 +0000 (16:05 +0200)]
x86emul: avoid speculative out of bounds accesses
There are a few array accesses here the indexes of which are (at least
indirectly) driven by the guest. Use array_access_nospec() to bound
such accesses. In the {,_}decode_gpr() cases replace existing guarding
constructs.
To deal with an otherwise occurring #include cycle, drop the inclusion
of asm/x86_emulate.h from asm/processor.h. This include had been
introduced for obtaining the struct cpuid_leaf declaration, which has
since moved into the x86 helper library.
This is part of the speculative hardening effort.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Anthony PERARD [Thu, 4 Jul 2019 14:04:33 +0000 (16:04 +0200)]
MAINTAINERS: add Anthony as libxl maintainer
Create a new section with only libxl.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Wei Liu <wl@xen.org>
Paul Durrant [Thu, 4 Jul 2019 14:03:47 +0000 (16:03 +0200)]
xmalloc: stop using a magic '1' in alignment padding
Alignment padding inserts a pseudo block header in front of the allocation,
sets its size field to the pad size and then ORs in 1, which is equivalent
to marking it as a free block, so that xfree() can distinguish it from a
real block header.
This patch simply replaces the magic '1' with the defined 'FREE_BLOCK' to
make it more obvious what's going on. Also, whilst in the neighbourhood,
it removes a stray space after a cast.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
x86: make loading of GDT at context switch more modular
In preparation for core scheduling, carve out the GDT related
functionality (writing GDT related PTEs, loading default of full GDT)
into sub-functions.
Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Tue, 2 Jul 2019 11:27:39 +0000 (13:27 +0200)]
AMD/IOMMU: restrict feature logging
The common case is all IOMMUs having the same features. Log them only
for the first IOMMU, or for any that have a differing feature set.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Brian Woods <brian.woods@amd.com>
Thus the loop termination condition is dereferencing a struct pointer that
is being incremented by the loop.
A block of MSI entries stores the number of vectors in entry[0].msi.nvec,
with all subsequent entries using a value of 0. Therefore, for a block of
two or more MSIs will terminate the loop early, as entry[1].msi.nvec is 0.
However, for a single MSI, ++entry moves the pointer out of bounds, and a
bogus read is used for the termination condition. In the case that the
loop body gets entered, there are subsequent OoB writes which clobber
adjacent memory in the heap.
This patch simply initializes a stack variable to the value of
entry->msi.nvec before starting the loop and then uses that in the
termination condition instead.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Fri, 28 Jun 2019 13:18:21 +0000 (14:18 +0100)]
arm/optee: Fix arm32 build
A Travis randconfig build notices:
optee.c: In function ‘allocate_and_pin_shm_rpc’:
optee.c:383:13: error: format ‘%lx’ expects argument of type
‘long unsigned int’, but argument 5 has type ‘uint64_t’ [-Werror=format=]
gdprintk(XENLOG_WARNING, "Guest tries to use the same RPC SHM cookie %lx\n",
^
Use PRIx64 instead of %lx
Full logs https://travis-ci.org/andyhhp/xen/jobs/551754253
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Andrew Cooper [Thu, 20 Jun 2019 12:04:14 +0000 (13:04 +0100)]
x86/svm: Drop svm_vm{load,save}() helpers
Following on from c/s 7d161f6537 "x86/svm: Fix svm_vmcb_dump() when used in
current context", there is now only a single user of svm_vmsave() remaining in
the tree, with all users moved to svm_vm{load,save}_pa().
nv->nv_n1vmcx has a matching nv->nv_n1vmcx_pa which is always correct, and
avoids a redundant __pa() translation behind the scenes.
With this gone, all VM{LOAD,SAVE} operations are using paddr_t's which is more
efficient, so drop the svm_vm{load,save}() helpers to avoid uses of them
reappearing in the future.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Acked-by: Brian Woods <brian.woods@amd.com>
Igor Druzhinin [Thu, 27 Jun 2019 19:41:54 +0000 (20:41 +0100)]
x86/cpuid: leak OSXSAVE only when XSAVE is not clear in policy
This fixes booting of old non-PV-OPS kernels which historically
looked for OSXSAVE instead of XSAVE bit in CPUID to check whether
XSAVE feature is enabled. If such a guest appears to be started on
an XSAVE enabled CPU and the feature is explicitly cleared in
policy, leaked OSXSAVE bit from Xen will lead to guest crash early in
boot.
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper [Wed, 21 Nov 2018 18:38:41 +0000 (18:38 +0000)]
xen/gnttab: Reduce code volume when using union grant_combo
There is no need for 'struct { ... } shorts' to be named. Convert it to being
an anonymous struct, and rename 'word' to the more common 'raw'.
For _set_status_v1() and gnttab_prepare_for_transfer() which use a bounded
cmpxchg loop, rename {prev,new}_scombo to {prev,new} and reduce their scope to
within the loop.
For _set_status_v2(), the flags and id variables are completely unnecessary.
Drop them.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Wed, 21 Nov 2018 18:38:41 +0000 (18:38 +0000)]
xen/gnttab: Reduce complexity when reading grant_entry_header_t
_set_status_v{1,2}() and gnttab_prepare_for_transfer() read the shared header
by always casting to u32. Despite grant_entry_header_t only having an
alignment of 2, this is actually safe because of the grant table ABI itself.
Switch to using an explicit uint32_t *, which removes all subsequent casting.
Furthermore, switch to using ACCESS_ONCE() for the read. There is nothing in
the _set_status_v1() and gnttab_prepare_for_transfer() which prevents the
compiler from issuing multiple memory reads and creating a TOCTOU race around
the sanity checks, although the worst that can happen is Xen stamping a status
flag over a bad grant entry if the guest is misbehaving.
_set_status_v2() does use barrier() to try avoid multiple reads, but this is
overkill. All that matters is that the shared header gets read in one go, and
this allows the compiler more room to optimise.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Sergey Dyasli [Thu, 29 Mar 2018 15:47:06 +0000 (16:47 +0100)]
x86/vvmx: set CR4 before CR0
Otherwise hvm_set_cr0() will check the wrong CR4 bits (L1 instead of L2
and vice-versa).
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Kevin Tian <kevin.tian@intel.com>
Roger Pau Monne [Thu, 27 Jun 2019 09:33:34 +0000 (11:33 +0200)]
xen/link: handle .init.rodata.cst* sections in the linker script
Note that those sections when not prefixed with .init are already
handled by the more general .rodata.* matching pattern in the .rodata
output section.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
[Make .init.rodata consistent with .rodata] Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Roger Pau Monne [Thu, 27 Jun 2019 09:33:33 +0000 (11:33 +0200)]
x86/linker: add a reloc section to ELF linker script
if the hypervisor has been built with EFI support (ie: multiboot2).
This allows to position the .reloc section correctly in the output
binary.
Note that for the ELF output format the .reloc section is moved before
.bss because the data it contains is read-only, so it belongs with the
other sections containing read-only data.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper [Tue, 25 Jun 2019 13:07:23 +0000 (14:07 +0100)]
page-alloc: Rename the first_node local variable
first_node is the name of a local variable, and part of the nodemask API. The
only reason this compiles is because the nodemask API is implemented as a
macro rather than an inline function.
It is confusing to read, and breaks when the nodemask API is cleaned up.
Rename the local variable to just 'first' which is still clear in context.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Wed, 26 Jun 2019 16:50:06 +0000 (17:50 +0100)]
xen/Kconfig: Fix -Wformat-security when compiling with Clang
Clang observes:
tools/kconfig/conf.c:77:10:
warning: format string is not a string literal (potentially insecure)
[-Wformat-security]
printf(_("aborted!\n\n"));
^~~~~~~~~~~~~~~~~
And it is absolutely correct. gettext() can easily return a string with a %
in.
This could be fixed by switching to using printf("%s", _(...)), or by
switching to puts() (as there is no formatting going on), but the better
option is follow Linux and remove localisation support.
The localization support is broken and appears unused.
There is no google hits on the update-po-config target.
And there is no recent (5 years) activity related to the localization.
So lets just drop this as it is no longer used.
Suggested-by: Ulf Magnusson <ulfalizer@gmail.com> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
[Ported to Xen] Reported-by: Roger Pau Monné <roger.pau@citrix.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Doug Goldstein <cardoe@cardoe.com>
Jan Beulich [Thu, 27 Jun 2019 10:34:24 +0000 (12:34 +0200)]
AMD/IOMMU: don't "add" IOMMUs
For find_iommu_for_device() to consistently (independent of ACPI tables)
return NULL for the PCI devices corresponding to IOMMUs, make sure
IOMMUs don't get mapped to themselves by ivrs_mappings[].
While amd_iommu_add_device() won't be called for IOMMUs from
pci_add_device(), as IOMMUs have got marked r/o,
_setup_hwdom_pci_devices() calls there nevertheless. Avoid issuing the
bogus debugging only "No iommu for ...; cannot be handed to ..." log
message as well as the non-debugging "setup ... for ... failed (-19)"
one.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Brian Woods <brian.woods@amd.com>
Jan Beulich [Tue, 25 Jun 2019 15:34:53 +0000 (17:34 +0200)]
drop __get_cpu_var() and __get_cpu_ptr()
this_cpu{,_ptr}() are shorter, and have previously been marked as
preferred in Xen anyway.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <julien.grall@arm.com> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Tue, 25 Jun 2019 15:32:37 +0000 (17:32 +0200)]
x86/mcheck: allow varying bank counts per CPU
Up to now we've been assuming that all CPUs would have the same number
of reporting banks. However, on upcoming AMD CPUs this isn't the case,
and one can observe
(XEN) mce.c:666: Different bank number on cpu <N>
indicating that Machine Check support would not be enabled on the
affected CPUs. Convert the count variable to a per-CPU one, and adjust
code where needed to cope with the values not being the same. In
particular the mcabanks_alloc() invocations during AP bringup need to
now allocate maximum-size bitmaps, because the truly needed size can't
be known until we actually execute on that CPU, yet mcheck_init() gets
called too early to do any allocations itself.
Take the liberty and also
- make mca_cap_init() static,
- replace several __get_cpu_var() uses when a local variable suitable
for use with per_cpu() appears,
- correct which CPU's cpu_data[] entry x86_mc_msrinject_verify() uses,
- replace a BUG() by panic().
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Roger Pau Monné [Tue, 25 Jun 2019 13:39:44 +0000 (15:39 +0200)]
config: don't hardcode toolchain binaries
Currently the names of the build toolchain binaries are hardcoded in
StdGNU.mk, and the values from the environment are ignored.
Switch StdGNU.mk to use '?=' instead of '=', so that values from the
environment are used if present, else default to the values provided
by the config file.
This change fixes the gitlab CI loop, that was relying on passing
custom values in the environment variables for the compiler and the
linker.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
The mask calculation in pdx_init_mask is wrong when the first bank
starts at address 0x0. The reason is that pdx_init_mask will do '0 - 1'
causing an underflow. As a result, the mask becomes 0xffffffffffffffff
which is the biggest possible mask and ends up causing a significant
memory waste in the frametable size computation.
For instance, on platforms that have a low memory bank starting at 0x0
and a high memory bank, the frametable will end up covering all the
holes in between.
The purpose of the mask is to be passed as a parameter to
pfn_pdx_hole_setup, which based on the mask parameter calculates
pfn_pdx_hole_shift, pfn_pdx_bottom_mask, etc. which are actually the
important masks for frametable initialization later on.
pfn_pdx_hole_setup never compresses addresses below MAX_ORDER bits (1GB
on ARM). Thus, it is safe to initialize mask passing 1ULL << (MAX_ORDER
+ PAGE_SHIFT) as start address to pdx_init_mask.
xen: Replace u64 with uint64_t in pdx_init_mask() and callers
Xen is phasing out the use of u64 in favor of uint64_t. Therefore, the
instance of u64 in the pdx_init_mask() (and the callers) are now
replaced with uint64_t. Take the opportunity to also modify
srat_region_mask as this is used to store the result of pdx_init_mask().
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: julien.grall@arm.com
Roger Pau Monne [Fri, 21 Jun 2019 16:38:00 +0000 (18:38 +0200)]
x86/linker: use DECL_SECTION uniformly
Replace the two open-coded EFI related section declarations with the
usage of DECL_SECTION. This is a preparatory change for also adding a
reloc section to the ELF binary.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Paul Durrant [Fri, 21 Jun 2019 15:57:51 +0000 (17:57 +0200)]
viridian: unify time sources
Currently, the time_ref_count enlightened time source maintains an offset
such that time is frozen when the domain paused, but the reference_tsc
enlightened time source does not. After migrate, the reference_tsc source
may become invalidated (e.g. because of host cpu frequency mismatch) which
will cause Windows to fall back to time_ref_count. Thus, the guest will
observe a jump in time equivalent to the offset.
This patch unifies the two enlightened time sources such that the same
offset applies to both of them. Also, it's not really necessary to have
two different functions to calculating a 10MHz counter value, time_now() and
raw_trc_val(), so this patch removes the latter implementation. The
unification also allows removal of the reference_tsc_valid flag.
Whilst in the area, this patch also takes the opportunity to constify a few
pointers which were missed in earlier patches.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Alexandru Isaila [Fri, 21 Jun 2019 15:21:25 +0000 (17:21 +0200)]
MAINTAINERS: add myself as a designated reviewer to vm_event
Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Jan Beulich [Fri, 21 Jun 2019 15:16:52 +0000 (17:16 +0200)]
AMD/IOMMU: initialize IRQ tasklet only once
Don't do this once per IOMMU, nor after setting up the IOMMU interrupt
(which will want to schedule this tasklet). In fact it can be
initialized at build time.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Brian Woods <brian.woods@amd.com>
GCC 4.8 is unable to see that variables guest_pg, guest_data and
xen_data will be always initialized before access, so we need to
initialize them earlier.
Some logging messages made more sense as argo debug
logs rather than standard Xen logs. Use argo_dprintk
to only print this info if argo DEBUG is enabled.
Signed-off-by: Nicholas Tsirakis <tsirakisn@ainfosec.com> Reviewed-by: Christopher Clark <christopher.w.clark@gmail.com>
Andrew Cooper [Mon, 17 Jun 2019 12:17:57 +0000 (12:17 +0000)]
x86/svm: Fix svm_vmcb_dump() when used in current context
VMExit doesn't switch all state. The FS/GS/TS/LDTR/GSBASE segment
information, and SYSCALL/SYSENTER MSRs may still be cached in hardware, rather
than up-to-date in the VMCB.
Export svm_sync_vmcb() via svmdebug.h so svm_vmcb_dump() can use it, and bring
the VMCB into sync in current context.
As a minor optimisation, switch svm_sync_vmcb() to use svm_vm{load,save}_pa(),
as svm->vmcb_pa is always correct, and this avoids a redundant __pa()
translation behind the scenes.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Acked-by: Brian Woods <brian.woods@amd.com>
OP-TEE can issue multiple RPC requests. We are interested mostly in
request that asks NW to allocate/free shared memory for OP-TEE
needs, because mediator needs to do address translation in the same
way as it was done for shared buffers registered by NW.
OP-TEE can ask NW to allocate multiple buffers during the call. We
know that if OP-TEE asks for another buffer, we can free pglist for
the previous one.
As mediator now accesses shared command buffer, we need to shadow
it in the same way, as we shadow request buffers for STD calls.
Earlier, we just passed address of this buffer to OP-TEE, but
now we need to read and write to it, so it should be shadowed.
xen/arm: optee: add support for arbitrary shared memory
Shared memory is widely used by NW (Normal World) to communicate with
TAs (Trusted Applications) in OP-TEE. NW can share part of own memory
with TA or with OP-TEE core, by registering it in OP-TEE, or by
providing a temporal reference. Anyways, information about such memory
buffers are sent to OP-TEE as a list of pages. This mechanism is
described in optee_msg.h.
Mediator should step in when NW tries to share memory with
OP-TEE for two reasons:
1. Do address translation from IPA to PA.
2. Pin domain pages while they are mapped into OP-TEE or TA
address space, so domain can't transfer this pages to
other domain or balloon out them.
Address translation is done by translate_noncontig(...) function.
It allocates new buffer from domheap and then walks on guest
provided list of pages, translates addresses and stores PAs into
newly allocated buffer. This buffer will be provided to OP-TEE
instead of original buffer from the guest. This buffer will
be freed at the end of standard call.
In the same time this function pins pages and stores them in
struct optee_shm_buf object. This object will live all the time,
when given SHM buffer is known to OP-TEE. It will be freed
after guest unregisters shared buffer. At this time pages
will be unpinned.
Guest can share buffer with OP-TEE for duration for one call,
or permanently, using OPTEE_MSG_CMD_REGISTER_SHM call. We need
to handle both options.
Also we want to limit total size of shared buffers. As it is not
possible to get limit from OP-TEE, we need to choose some arbitrary
value. Currently limit is 16384 of 4K pages.
OP-TEE usually uses the same idea with command buffers (see
previous commit) to issue RPC requests. Problem is that initially
it has no buffer, where it can write request. So the first RPC
request it makes is special: it requests NW to allocate shared
buffer for other RPC requests. Usually this buffer is allocated
only once for every OP-TEE thread and it remains allocated all
the time until guest shuts down. Guest can ask OP-TEE to disable
RPC buffers caching, in this case OP-TEE will ask guest to
allocate/free buffer for the each RPC.
Mediator needs to pin this buffer to make sure that page will be
not free while it is shared with OP-TEE.
Life cycle of this buffer is controlled by OP-TEE. It asks guest to
create buffer and it asks it to free it. So it there is not much sense
to limit number of those buffers, because we already limit the number
of concurrent standard calls and prevention of RPC buffer allocation will
impair OP-TEE functionality.
Those buffers can be freed in two ways: either OP-TEE issues
OPTEE_SMC_RPC_FUNC_FREE RPC request or guest tries to disable
buffer caching by calling OPTEE_SMC_DISABLE_SHM_CACHE function.
In the latter case OP-TEE will return cookie of the SHM buffer it
just freed.
OP-TEE expects that this RPC buffer have size of
OPTEE_MSG_NONCONTIG_PAGE_SIZE, which equals to 4096 and is aligned
with the same size. So, basically it expects one 4k page from the
guest. This is the same as Xen's PAGE_SIZE.
The main way to communicate with OP-TEE is to issue standard SMCCC
call. "Standard" is a SMCCC term and it means that call can be
interrupted and OP-TEE can return control to NW before completing
the call.
In contrast with fast calls, where arguments and return values
are passed in registers, standard calls use shared memory. Register
pair a1,a2 holds 64-bit PA of command buffer, where all arguments
are stored and which is used to return data. OP-TEE internally
copies contents of this buffer into own secure memory before accessing
and validating any data in command buffer. This is done to make sure
that NW will not change contents of the validated parameters.
Mediator needs to do the same for number of reasons:
1. To make sure that guest will not change data after validation.
2. To translate IPAs to PAs in the command buffer (this is not done
in this patch).
3. To hide translated address from guest, so it will not be able
to do IPA->PA translation by misusing mediator.
During standard call OP-TEE can issue multiple "RPC returns", asking
NW to do some work for OP-TEE. NW then issues special call
OPTEE_SMC_CALL_RETURN_FROM_RPC to resume handling of the original call.
Thus, mediator needs to maintain context for original standard call
during multiple SMCCC calls.
Standard call is considered complete, when returned value is
not a RPC request.
This patch adds handling for the fast SMCs. As name suggests, those
calls can't be preempted and are used for auxiliary tasks such as
information retrieval. Most handlers are quite trivial, with exception
for capabilities information.
Capabilities exchange should be filtered out, so only caps
known to mediator are used. Also mediator disables static SHM
memory capability, because it can't share OP-TEE memory with a domain.
Only domain can share memory with OP-TEE, so it ensures that OP-TEE
supports dynamic SHM.
Basically, static SHM is a reserved memory region which is always
mapped into OP-TEE address space. It belongs to OP-TEE. Normally,
NW is allowed to access there, so it can communicate with OP-TEE.
On other hand, dynamic SHM is NW's own memory, which it can share
with OP-TEE. OP-TEE maps this memory dynamically, when it wants to
access it.
Because mediator can't share one static SHM region with all guests, it
just disables it for all of them. It is possible to make exception for
Dom0, but it requires separate handling for buffers allocated from
that region. Thus, it is not implemented yet.
Add very basic OP-TEE mediator. It can probe for OP-TEE presence,
tell it about domain creation/destruction and then return an error
to all calls to the guest.
This code issues two non-preemptible calls to OP-TEE: to create and
to destroy client context. They can't block in OP-TEE, as they are
considered "fast calls" in terms of ARM SMCCC.
This header files describes protocol between OP-TEE OS and OP-TEE
clients, which are running in Normal World. This headers are needed
for upcoming OP-TEE mediator, which is added in the next patch. Reason
to add those headers in separate patch is to ease up review. Those
files were taken from OP-TEE OS 3.5.0 tree and mangled a bit to
compile with XEN.
This patch adds basic framework for TEE mediators. Guests can't talk
to TEE directly, we need some entity that will intercept request
and decide what to do with them. "TEE mediator" is a such entity.
This is how it works: user can build XEN with multiple TEE mediators
(see the next patches, where OP-TEE mediator is introduced).
TEE mediator register self with REGISTER_TEE_MEDIATOR() macro in the
same way, as device drivers use DT_DEVICE_START()/DT_DEVICE_END()
macros.
At run-time, during initialization, framework calls probe() function
for each available mediator driver to find which TEE is installed
on the platform. Then generic vSMC handler will call selected mediator
when it intercept SMC/HVC that belongs to TEE OS or TEE application.
Andrew Cooper [Mon, 17 Jun 2019 18:56:11 +0000 (19:56 +0100)]
x86/clear_page: Update clear_page_sse2() after dropping 32bit Xen
This code was never updated when the 32bit build of Xen was dropped.
* Expand the now-redundant ptr_reg macro.
* The number of iterations in the loop can be halfed by using 64bit writes,
without consuming any extra execution resource in the pipeline. Adjust all
numbers/offsets appropriately.
* Replace dec with sub to avoid a eflags stall, and position it to be
macro-fused with the related jmp.
* With no need to preserve eflags across the body of the loop, replace lea
with add which has 1/3'rd the latency on basically all 64bit hardware.
A quick userspace perf test on my Haswell dev box indicates that the old
version takes ~1385 cycles on average (ignoring outliers), and the new version
takes ~1060 cyles, or about 77% of the time.
Reported-by: Edwin Török <edvin.torok@citrix.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Tue, 18 Jun 2019 14:35:35 +0000 (16:35 +0200)]
x86/SMP: don't try to stop already stopped CPUs
In particular with an enabled IOMMU (but not really limited to this
case), trying to invoke fixup_irqs() after having already done
disable_IO_APIC() -> clear_IO_APIC() is a rather bad idea:
Don't call fixup_irqs() and don't send any IPI if there's only one
online CPU anyway, and don't call __stop_this_cpu() at all when the CPU
we're on was already marked offline (by a prior invocation of
__stop_this_cpu()).
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Extend this to the kexec/crash path as well.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Tue, 18 Jun 2019 14:34:51 +0000 (16:34 +0200)]
x86/AMD: limit C1E disable family range
Just like for other family values of 0x17 (see "x86/AMD: correct certain
Fam17 checks"), commit 3157bb4e13 ("Add MSR support for various feature
AMD processor families") made the original check for Fam11 here include
families all the way up to Fam17. The involved MSR (0xC0010055),
however, is fully reserved starting from Fam16, and the two bits of
interest are reserved for Fam12 and onwards (albeit I admit I wasn't
able to find any Fam13 doc). Restore the upper bound to be Fam11.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Tue, 18 Jun 2019 14:33:53 +0000 (16:33 +0200)]
x86/AMD: correct certain Fam17 checks
Commit 3157bb4e13 ("Add MSR support for various feature AMD processor
families") converted certain checks for Fam11 to include families all
the way up to Fam17. The commit having no description, it is hard to
tell whether this was a mechanical dec->hex conversion mistake, or
indeed intended. In any event the NB_CFG handling needs to be restricted
to Fam16 and below: Fam17 doesn't really have such an MSR anymore. As
per observation it's read-zero / write-discard now, so make PV uniformly
(with the exception of pinned Dom0 vCPU-s) behave so, just like HVM
already does.
Mirror the NB_CFG behavior to MSR_FAM10H_MMIO_CONF_BASE as well, except
that here the vendor/model check is kept in place (for now at least).
A non-MMCFG extended config space access mechanism still appears to
exist, but code to deal with it will need to be written down the road,
when it can actually be tested.
Reported-by: Pu Wen <puwen@hygon.cn> Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Mon, 17 Jun 2019 15:38:35 +0000 (17:38 +0200)]
x86/x2APIC: tighten check in cluster mode IPI sending
It is only of limited use to check the full accumulated 32-bit value,
because the high halves are the cluster ID. What needs to be non-zero is
the bit map at the bottom, or else APIC errors will result.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Mon, 17 Jun 2019 15:35:41 +0000 (17:35 +0200)]
x86/IO-APIC: dump full destination ID in x2APIC mode
In x2APIC mode it is 32 bits wide.
In __print_IO_APIC() drop logging of both physical and logical IDs:
The latter covers a superset of the bits of the former in the RTE, and
we write full 8-bit values anyway even in physical mode for all ordinary
interrupts, regardless of INT_DEST_MODE (see the users of SET_DEST()).
Adjust other column arrangement (and heading) a little as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Julien Grall [Fri, 15 Mar 2019 21:19:43 +0000 (21:19 +0000)]
xen/arm: mm: Remove set_pte_flags_on_range()
set_pte_flags_on_range() is yet another function that will open-code
update to a specific range in the Xen page-tables. It can be completely
dropped by using either modify_xen_mappings() or destroy_xen_mappings().
Note that modify_xen_mappings() will keep the field 'pxn' cleared for
the all the cases. This is because the field is RES0 for the stage-1
hypervisor as only a single VA range is supported (see D5.4.5 in
DDI0487D.b).
Julien Grall [Sun, 2 Dec 2018 18:54:06 +0000 (18:54 +0000)]
xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()
{set, clear}_fixmap() are currently open-coding update to the Xen
page-tables. This can be avoided by using the generic helpers
map_pages_to_xen() and destroy_xen_mappings().
Both function are not meant to fail for fixmap, hence the BUG_ON()
checking the return.
xen/arm: mm: Rework Xen page-tables walk during update
Currently, xen_pt_update_entry() is only able to update the region covered
by xen_second (i.e 0 to 0x7fffffff).
Because of the restriction we end to have multiple functions in mm.c
modifying the page-tables differently.
Furthermore, we never walked the page-tables fully. This means that any
change in the layout may requires major rewrite of the page-tables code.
Lastly, we have been quite lucky that no one ever tried to pass an address
outside this range because it would have blown-up.
xen_pt_update_entry() is reworked to walk over the page-tables every
time. The logic has been borrowed from arch/arm/p2m.c and contain some
limitations for the time being:
- Superpage cannot be shattered
- Only level 3 (i.e 4KB) can be done
Note that the parameter 'addr' has been renamed to 'virt' to make clear
we are dealing with a virtual address.
xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables
Currently, the virtual address of the 3rd level page-tables is obtained
using mfn_to_virt().
On Arm32, mfn_to_virt can only work on xenheap page. While in theory
all the page-tables updated will reside in xenheap, in practice the
page-tables covering Xen memory (e.g xen_mapping) is part of Xen binary.
Furthermore, a follow-up change will update xen_pt_update_entry() to
walk all the levels and therefore be more generic. Some of the
page-tables will also part of Xen memory and therefore will not be
reachable using mfn_to_virt().
The easiest way to reach those pages is to use {, un}map_domain_page().
While on arm32 this means an extra mapping in the normal cases, this is not
very important as xen page-tables are not updated often.
In order to allow future change in the way Xen page-tables are mapped,
two new helpers are introduced to map/unmap the page-tables.
Julien Grall [Sat, 23 Mar 2019 11:44:44 +0000 (11:44 +0000)]
xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation
With the newly introduced flags, it is now possible to know how the page
will be updated through the flags.
All the use of xenmap_operation are now replaced with the flags. At the
same time, validity check are now removed as they are gathered in
xen_pt_check_entry().
Julien Grall [Mon, 18 Mar 2019 18:38:27 +0000 (18:38 +0000)]
xen/arm: mm: Sanity check any update of Xen page tables
The code handling Xen PT update has quite a few restrictions on what it
can do. This is not a bad thing as it keeps the code simple.
There are already a few checks scattered in current page table handling.
However they are not sufficient as they could still allow to
modify/remove entry with contiguous bit set.
The checks are divided in two sets:
- per entry check: They are gathered in a new function that will
check whether an update is valid based on the flags passed and the
current value of an entry.
- global check: They are sanity check on xen_pt_update() parameters.
Additionally to contiguous check, we also now check that the caller is
not trying to modify the memory attributes of an entry.
Lastly, it was probably a bit over the top to forbid removing an
invalid mapping. This could just be ignored. The new behavior will be
helpful in future changes.
Julien Grall [Mon, 18 Mar 2019 16:17:01 +0000 (16:17 +0000)]
xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE
At the moment, the flags are not enough to describe what kind of update
will done on the VA range. They need to be used in conjunction with the
enum xenmap_operation.
It would be more convenient to have all the information for the update
in a single place.
Two new flags are added to remove the relience on xenmap_operation:
- _PAGE_PRESENT: Indicate whether we are adding/removing the mapping
- _PAGE_POPULATE: Indicate whether we only populate page-tables
xen/arm: grant-table: Protect gnttab_clear_flag against guest misbehavior
The function gnttab_clear_flag is used to clear the access flags. On
Arm, it is implemented using a loop and guest_cmpxchg.
It is possible that guest_cmpxchg will always return a different value
than old. This can happen if the guest updated the memory before Xen has
time to do the exchange. Because of that, there are no way for to
promise the loop will end.
It is possible to make the current code safe by re-using the same
principle as applied on the guest atomic helper. However this patch
takes a different approach that should lead to more efficient code in
the default case.
A new helper is introduced to clear a set of bits on a 16-bits word.
This should avoid a an extra loop to check cmpxchg succeeded.
Note that a mask is used instead of a bit, so the helper can be re-used
later on for clearing multiple flags at the same time.
This is part of XSA-295.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Julien Grall <julien.grall@arm.com> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
xen: Use guest atomics helpers when modifying atomically guest memory
On Arm, exclusive load-store atomics should only be used between trusted
thread. As not all the guests are trusted, it may be possible to DoS Xen
when updating shared memory with guest atomically.
This patch replaces all the atomics operations on shared memory with
a guest by the new guest atomics helpers. The x86 code was not audited
to know where guest atomics helpers could be used. I will leave that
to the x86 folks.
Note that some rework was required in order to plumb use the new guest
atomics in event channel and grant-table.
Because guest_test_bit is ignoring the parameter "d" for now, it
means there a lot of places do not need to drop the const. We may want
to revisit this in the future if the parameter "d" becomes necessary.
xen/cmpxchg: Provide helper to safely modify guest memory atomically
On Arm, exclusive load-store atomics should only be used between trusted
thread. As not all the guests are trusted, it may be possible to DoS Xen
when updating shared memory with guest atomically.
This patch adds a new helper that will update the guest memory safely.
For x86, it is already possible to use the current helper safely. So
just wrap it.
For Arm, we will first attempt to update the guest memory with the
loop bounded by a maximum number of iterations. If it fails, we will
pause the domain and try again.
Note that this heuristics assumes that a page can only
be shared between Xen and one domain. Not Xen and multiple domain.
The maximum number of iterations is based on how many times atomic_inc()
can be executed in 1uS. The maximum value is per-CPU to cater big.LITTLE
and calculated when the CPU is booting.
The maximum number of iterations is based on how many times a simple
load-store atomic operation can be executed in 1uS. The maximum
value is per-CPU to cater big.LITTLE and calculated when the CPU is
booting. The heuristic was randomly chosen and can be modified if
impact too much good-behaving guest.
xen/bitops: Provide helpers to safely modify guest memory atomically
On Arm, exclusive load-store atomics should only be used between trusted
thread. As not all the guests are trusted, it may be possible to DoS Xen
when updating shared memory with guest atomically.
This patch adds a new set of helper that will update the guest memory
safely. For x86, it is already possible to use the current helpers
safely. So just wrap them.
For Arm, we will first attempt to update the guest memory with the loop
bounded by a maximum number of iterations. If it fails, we will pause the
domain and try again.
Note that this heuristics assumes that a page can only be shared between
Xen and one domain. Not Xen and multiple domain.
The maximum number of iterations is based on how many times a simple
load-store atomic operation can be executed in 1uS. The maximum value is
per-CPU to cater big.LITTLE and calculated when the CPU is booting. The
heuristic was randomly chosen and can be modified if impact too much
good-behaving guest.
Note, while test_bit does not requires to use atomic operation, a
wrapper for test_bit was added for completeness. In this case, the
domain stays constified to avoid major rework in the caller for the
time-being.
On Arm, exclusive load-store atomics should only be used between trusted
thread. As not all the guests are trusted, it may be possible to DoS Xen
when updating shared memory with guest atomically.
Recent patches introduced new helpers to update shared memory with guest
atomically. Those helpers relies on a memory region to be be shared with
Xen and a single guest.
At the moment, nothing prevent a guest sharing a page with Xen and as
well with another guest (e.g via grant table).
For the scope of the XSA, the quickest way is to deny communications
between unprivileged guest. So this patch is enabling and using SILO
mode by default on Arm.
Users wanted finer graine policy could wrote their own Flask policy.
This is part of XSA-295.
Signed-off-by: Julien Grall <julien.grall@arm.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Julien Grall [Wed, 22 May 2019 20:39:17 +0000 (13:39 -0700)]
xen/arm: cmpxchg: Provide a new helper that can timeout
Exclusive load-store atomics should only be used between trusted
threads. As not all the guests are trusted, it may be possible to DoS
Xen when updating shared memory with guest atomically.
To prevent the infinite loop, we introduce a new helper that can timeout.
The timeout is based on the maximum number of iterations.
It will be used in follow-up patch to make atomic operations on shared
memory safe.
xen/arm: bitops: Implement a new set of helpers that can timeout
Exclusive load-store atomics should only be used between trusted
threads. As not all the guests are trusted, it may be possible to DoS
Xen when updating shared memory with guest atomically.
To prevent the infinite loop, we introduce a new set of helpers that can
timeout. The timeout is based on the maximum number of iterations.
They will be used in follow-up patch to make atomic operations
on shared memory safe.
xen/arm32: cmpxchg: Simplify the cmpxchg implementation
The only difference between each case of the cmpxchg is the size of
used. Rather than duplicating the code, provide a macro to generate each
cases.
This makes the code easier to read and modify.
While doing the rework, the case for 64-bit cmpxchg is removed. This is
unused today (already commented) and it would not be possible to use
it directly.
xen/grant_table: Rework the prototype of _set_status* for lisibility
It is not clear from the parameters name whether domid and gt_version
correspond to the local or remote domain. A follow-up patch will make
them more confusing.
So rename domid (resp. gt_version) to ldomid (resp. rgt_version). At
the same time re-order the parameters to hopefully make it more
readable.
This is part of XSA-295.
Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Julien Grall <julien.grall@arm.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
xen/arm: Add an isb() before reading CNTPCT_EL0 to prevent re-ordering
Per D8.2.1 in ARM DDI 0487C.a, "a read to CNTPCT_EL0 can occur
speculatively and out of order relative to other instructions executed
on the same PE."
Add an instruction barrier to get accurate number of cycles when
requested in get_cycles(). For the other users of CNPCT_EL0, replace by
a call to get_cycles().
Julien Grall [Mon, 18 Mar 2019 18:06:55 +0000 (18:06 +0000)]
xen/arm: mm: Protect Xen page-table update with a spinlock
The function create_xen_entries() may be called concurrently. For
instance, while the vmap allocation is protected by a spinlock, the
mapping is not.
The implementation create_xen_entries() contains quite a few TOCTOU
races such as when allocating the 3rd-level page-tables.
Thankfully, they are pretty hard to reach as page-tables are allocated
once and never released. Yet it is possible, so we need to protect with
a spinlock to avoid corrupting the page-tables.
xen/arm32: mm: Avoid cleaning the cache for secondary CPUs page-tables
The page-table walker is configured by TCR_EL2 to use the same
shareability and cacheability as the access performed when updating the
page-tables. This means cleaning the cache for secondary CPUs runtime
page-tables is unnecessary.
When a message is requeue'd in Xen's internal queue, the queue
entry contains the length of the message so that Xen knows to
send a VIRQ to the respective domain when enough space frees up
in the ring. Due to a small bug, however, Xen doesn't populate
the length of the msg if a given write fails, so this length is
always reported as zero. This causes Xen to spuriously wake up
a domain even when the ring doesn't have enough space.
This patch makes sure that the msg len is properly reported by
populating it in the event of a write failure.
Signed-off-by: Nicholas Tsirakis <tsirakisn@ainfosec.com> Reviewed-by: Christopher Clark <christopher.w.clark@gmail.com>
Julien Grall [Mon, 18 Mar 2019 18:01:31 +0000 (18:01 +0000)]
xen/arm: mm: Flush the TLBs even if a mapping failed in create_xen_entries
At the moment, create_xen_entries will only flush the TLBs if the full
range has successfully been updated. This may lead to leave unwanted
entries in the TLBs if we fail to update some entries.
All the TLBs helpers invalidate all the TLB entries are using the same
pattern:
DSB SY
TLBI ...
DSB SY
ISB
This pattern is following pattern recommended by the Arm Arm to ensure
visibility of updates to translation tables (see K11.5.2 in ARM DDI
0487D.b).
We have been a bit too eager in Xen and use system-wide DSBs when this
can be limited to the inner-shareable domain.
Furthermore, the first DSB can be restrict further to only store in the
inner-shareable domain. This is because the DSB is here to ensure
visibility of the update to translation table walks.
Lastly, there are a lack of documentation in most of the TLBs helper.
Rather than trying to update the helpers one by one, this patch
introduce a per-arch macro to generate the TLB helpers. This will be
easier to update the TLBs helper in the future and the documentation.
Now that we dropped flush_xen_text_tlb_local(), we have only one set of
helpers acting on Xen TLBs. There naming are quite confusing because the
TLB instructions used will act on both Data and Instruction TLBs.
Take the opportunity to rework the documentation which can be confusing
to read as they don't match the implementation. Note the mention about
the instruction cache maintenance has been removed because modifying
mapping does not require instruction cache maintenance.
Lastly, switch from unsigned long to vaddr_t as the function technically
deal with virtual address.
Julien Grall [Mon, 13 May 2019 15:02:18 +0000 (16:02 +0100)]
xen/arm: Don't boot Xen on platform using AIVIVT instruction caches
The AIVIVT is a type of instruction cache available on Armv7. This is
the only cache not implementing the IVIPT extension and therefore
requiring specific care.
To simplify maintenance requirements, Xen will not boot on platform
using AIVIVT cache.
This should not be an issue because Xen Arm32 can only boot on a small
number of processors (see arch/arm/arm32/proc-v7.S). All of them are
not using AIVIVT cache.
Andrew Cooper [Wed, 12 Jun 2019 10:28:05 +0000 (11:28 +0100)]
x86/boot: Drop vestigial support for pre-SIPI APICs
The current code in do_boot_cpu() makes a CMOS write (even in the case of an
FADT reduced hardware configuration) and two writes into the BDA for the
start_eip segment and offset.
BDA 0x67 and 0x69 hail from the days of the DOS and the 286, when IBM put
together the fast way to return from Protected Mode back to Real Mode (via a
deliberate triple fault). This vector, when set, redirects the early boot
logic back into OS control.
It is also used by early MP systems, before the Startup IPI message became
standard, which in practice was before Local APICs became integrated into CPU
cores.
Support for non-integrated APICs was dropped in c/s 7b0007af "xen/x86: Remove
APIC_INTEGRATED() checks" because there are no 64-bit capable systems without
them. Therefore, drop smpboot_{setup,restore}_warm_reset_vector().
Dropping smpboot_setup_warm_reset_vector() also lets us drop
TRAMPOLINE_{HIGH,LOW}, which lets us drop mach_wakecpu.h entirely. The final
function in smpboot_hooks.h is smpboot_setup_io_apic() and has a single
caller, so expand it inline and delete smpboot_hooks.h as well.
This removes all reliance on CMOS and the BDA from the AP boot path, which is
especially of interest on reduced_hardware boots and EFI systems.
This was discovered while investigating Xen's use of the BDA during kexec.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
projects / xen.git / log