In order to verify that in particular the index register decoding works
correctly in the S/G emulation paths, add dedicated (64-bit only) cases
disallowing the compiler to use the lower registers. Other than in the
generic SIMD case, where occasional uses of %xmm or %ymm registers in
generated code cause various internal compiler errors when disallowing
use of all of the lower 16 registers (apparently due to insn templates
trying to use AVX2 encodings), doing so here in the AVX512F case looks
to be fine.
While the main goal here is the AVX512F case, add an AVX2 variant as
well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There was an encoding mistake in the EVEX Disp8 test code, which was
benign (due to %rdx getting set to zero) to all non-vSIB tests as it
mistakenly encoded <disp8>(%rdx,%rdx) instead of <disp8>(%rdx,%riz). In
the vSIB case this meant <disp8>(%rdx,%zmm2) instead of the intended
<disp8>(%rdx,%zmm4).
Likewise the access count check wasn't entirely correct for the S/G
case: In the quad-word-index but dword-data case only half the number
of full vector elements get accessed.
As an unrelated change in the main test harness source file distinguish
the "n/a" messages by bitness.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
They require getting modrm_reg and sib_index set correctly in the EVEX
case, to account for the high 16 [XYZ]MM registers when used as
addressing index register. Extend the adjustments to modrm_rm as well,
such that x86_insn_modrm() would correctly report register numbers (this
was a latent issue only as we don't currently have callers of that
function which would care about an EVEX case).
The adjustment in turn requires dropping the assertion from decode_gpr()
as well as re-introducing the explicit masking, as we now need to
actively mask off the high bit when a GPR is meant.
_decode_gpr() invocations also need slight adjustments, when invoked in
generic code ahead of the main switch(). All other uses of modrm_reg and
modrm_rm already get suitably masked where necessary.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The setup for calling trace_var() (which itself checks tb_init_done) is
non-negligible, and hence a separate outer-most check is warranted.
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>
Its only caller already has the IRQ descriptor in its hands, so there's
no need for the function to re-obtain it. As a result the leading p of
its name is no longer appropriate and hence gets dropped.
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>
This completes support of AVX512BW in the insn emulator, and leaves just
the scatter/gather ones open in the AVX512F set.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
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>
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>
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>
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>
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>
The for loop that deals with MSI masking is coded as follows:
for ( pos = 0; pos < entry->msi.nvec; ++pos, ++entry )
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>
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>
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>
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>
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>
_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>
The nested if condition can be joined with the outer one, removing
one indentation level.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
After building the hypervisor binary. Note that the check is performed
by searching for the magic header value at the start of the binary.
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>
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>
This fixes the UBSAN build for GCC 8 and later. The sanitiser checks for
passing 0 to the ctz()/clz() builtins.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
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>
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>
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.
Linux changeset: 694c49a7c01cc87194be40cb26404b58b68c291c
Author: Sam Ravnborg <sam@ravnborg.org>
Date: Tue May 22 20:36:12 2018
kconfig: drop localization 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>
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>
this_cpu() is shorter, and when there are multiple uses in a function
per_cpu() it's also more efficient.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
this_cpu() is shorter, and when there are multiple uses in a function
per_cpu() it's also more efficient.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
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>
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.
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
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
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>
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>
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.
Suggested-by: Stefano Stabellini <stefanos@xilinx.com>
Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Jan Beulich
Anthony PERARD
Paul Durrant
Juergen Gross
Andrew Cooper
Igor Druzhinin
Sergey Dyasli
Roger Pau Monne
Stefano Stabellini
Julien Grall
Alexandru Isaila
Petre Pircalabu
Volodymyr Babchuk