Commit Graph
41707 Commits
Author SHA1 Message Date
Juergen Gross 032a96e5ef maintainers: fix libxl paths
Fix the paths of libxl in the MAINTAINERS file.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-10-23 14:29:53 +01:00
Jan Beulich 6ca70821b5 x86emul: increase FPU save area in test harness/fuzzer
Running them on a system (or emulator) with AMX support requires this
to be quite a bit larger than 8k, to avoid triggering the assert() in
emul_test_init(). Bump all the way up to 16k right away.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-10-23 10:13:53 +02:00
Roger Pau Monné 710f62cc82 pci: cleanup MSI interrupts before removing device from IOMMU
Doing the MSI cleanup after removing the device from the IOMMU leads
to the following panic on AMD hardware:

Assertion 'table.ptr && (index < intremap_table_entries(table.ptr, iommu))' failed at iommu_intr.c:172
----[ Xen-4.13.1-10.0.3-d  x86_64  debug=y   Not tainted ]----
CPU:    3
RIP:    e008:[<ffff82d08026ae3c>] drivers/passthrough/amd/iommu_intr.c#get_intremap_entry+0x52/0x7b
[...]
Xen call trace:
   [<ffff82d08026ae3c>] R drivers/passthrough/amd/iommu_intr.c#get_intremap_entry+0x52/0x7b
   [<ffff82d08026af25>] F drivers/passthrough/amd/iommu_intr.c#update_intremap_entry_from_msi_msg+0xc0/0x342
   [<ffff82d08026ba65>] F amd_iommu_msi_msg_update_ire+0x98/0x129
   [<ffff82d08025dd36>] F iommu_update_ire_from_msi+0x1e/0x21
   [<ffff82d080286862>] F msi_free_irq+0x55/0x1a0
   [<ffff82d080286f25>] F pci_cleanup_msi+0x8c/0xb0
   [<ffff82d08025cf52>] F pci_remove_device+0x1af/0x2da
   [<ffff82d0802a42d1>] F do_physdev_op+0xd18/0x1187
   [<ffff82d080383925>] F pv_hypercall+0x1f5/0x567
   [<ffff82d08038a432>] F lstar_enter+0x112/0x120

That's because the call to iommu_remove_device on AMD hardware will
remove the per-device interrupt remapping table, and hence the call to
pci_cleanup_msi done afterwards will find a null intremap table and
crash.

Reorder the calls so that MSI interrupts are torn down before removing
the device from the IOMMU.

Fixes: d7cfeb7c13 ("AMD/IOMMU: don't blindly allocate interrupt remapping tables")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-10-23 10:13:14 +02:00
Jan Beulich b76c3a1b87 evtchn: let evtchn_set_priority() acquire the per-channel lock
Some lock wants to be held to make sure the port doesn't change state,
but there's no point holding the per-domain event lock here. Switch to
using the finer grained per-channel lock instead (albeit as a downside
for the time being this requires disabling interrupts for a short
period of time).

FAOD this doesn't guarantee anything towards in particular
evtchn_fifo_set_pending(), as for interdomain channels that function
would be called with the remote side's per-channel lock held.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
2020-10-23 10:12:31 +02:00
Jan Beulich 451a909860 evtchn: rename and adjust guest_enabled_event()
The function isn't about an "event" in general, but about a vIRQ. The
function also failed to honor global vIRQ-s, instead assuming the caller
would pass vCPU 0 in such a case.

While at it also adjust the
- types the function uses,
- single user to make use of domain_vcpu().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
2020-10-23 10:11:46 +02:00
Jan Beulich 5bc84281a9 evtchn: replace FIFO-specific header by generic private one
Having a FIFO specific header is not (or at least no longer) warranted
with just three function declarations left there. Introduce a private
header instead, moving there some further items from xen/event.h.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
2020-10-23 10:09:55 +02:00
Jan Beulich dcbd1d8ae5 evtchn: avoid race in get_xen_consumer()
There's no global lock around the updating of this global piece of data.
Make use of cmpxchgptr() to avoid two entities racing with their
updates.

While touching the functionality, mark xen_consumers[] read-mostly (or
else the if() condition could use the result of cmpxchgptr(), writing to
the slot unconditionally).

The use of cmpxchgptr() here points out (by way of clang warning about
it) that its original use of const was slightly wrong. Adjust the
placement, or else undefined behavior of const qualifying a function
type will result.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
2020-10-23 10:07:56 +02:00
Jan Beulich 83432adeb8 IOMMU/EPT: avoid double flushing in shared page table case
While the flush coalescing optimization has been helping the non-shared
case, it has actually lead to double flushes in the shared case (which
ought to be the more common one nowadays at least): Once from
*_set_entry() and a second time up the call tree from wherever the
overriding flag gets played with. In alignment with XSA-346 suppress
flushing in this case.

Similarly avoid excessive setting of IOMMU_FLUSHF_added on the batched
flushes: "idx" hasn't been added a new mapping for.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
2020-10-23 10:06:53 +02:00
Jan Beulich f9c53bd9ee x86/mm: avoid playing with directmap when self-snoop can be relied upon
The set of systems affected by XSA-345 would have been smaller is we had
this in place already: When the processor is capable of dealing with
mismatched cacheability, there's no extra work we need to carry out.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
2020-10-23 10:06:20 +02:00
Jan Beulich ba45ae4d7f x86: XENMAPSPACE_gmfn{,_batch,_range} want to special case idx == gpfn
In this case up to now we've been freeing the page (through
guest_remove_page(), with the actual free typically happening at the
put_page() later in the function), but then failing the call on the
subsequent GFN consistency check. However, in my opinion such a request
should complete as an "expensive" no-op (leaving aside the potential
unsharing of the page).

This points out that f33d653f46 ("x86: replace bad ASSERT() in
xenmem_add_to_physmap_one()" would really have needed an XSA, despite
its description claiming otherwise, as in release builds we then put in
place a P2M entry referencing the about to be freed page.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
2020-10-23 10:05:29 +02:00
Elliott Mitchell 861f0c1109 xen/arm: acpi: Don't fail if SPCR table is absent
Absence of a SPCR table likely means the console is a framebuffer.  In
such case acpi_iomem_deny_access() should NOT fail.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2020-10-22 19:41:51 +01:00
Bertrand Marquis 3b49791e4c xen/arm: Print message if reset did not work
If for some reason the hardware reset is not working, print a message to
the user every 5 seconds to warn him that the system did not reset
properly and Xen is still looping.

The message is printed infinitely so that someone connecting to a serial
console with no history would see the message coming after 5 seconds.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
[stefano: remove second '!']
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
2020-10-20 13:20:31 -07:00
Volodymyr Babchuk aace546699 arm: optee: don't print warning about "wrong" RPC buffer
The OP-TEE mediator tracks the cookie value of the last buffer which was
requested by OP-TEE. This tracked value serves one important purpose: if
OP-TEE wants to request another buffer, we know that it finished
importing the previous one and we can free the page list associated with
it.

Also, we had a false premise that OP_TEE frees requested buffers in
reversed order. So we checked rpc_data_cookie during handling of the
OPTEE_RPC_CMD_SHM_FREE call and printed a warning if the cookie of the
buffer which is requested to be freed differs from the last allocated
one.

During testing of RPMB FS services I discovered that RPMB code frees
request and response buffers in the same order is it allocated them. And
this is perfectly fine, actually.

So, we are removing mentioned warning message in Xen, as it is perfectly
normal to free buffers in arbitrary order.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
[stefano: fix grammar]
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
2020-10-20 13:20:28 -07:00
Jan Beulich 0514a3a25f AMD/IOMMU: ensure suitable ordering of DTE modifications
DMA and interrupt translation should be enabled only after other
applicable DTE fields have been written. Similarly when disabling
translation or when moving a device between domains, translation should
first be disabled, before other entry fields get modified. Note however
that the "moving" aspect doesn't apply to the interrupt remapping side,
as domain specifics are maintained in the IRTEs here, not the DTE. We
also never disable interrupt remapping once it got enabled for a device
(the respective argument passed is always the immutable iommu_intremap).

This is part of XSA-347.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
2020-10-20 14:23:12 +02:00
Jan Beulich 3b055121c5 AMD/IOMMU: update live PTEs atomically
Updating a live PTE bitfield by bitfield risks the compiler re-ordering
the individual updates as well as splitting individual updates into
multiple memory writes. Construct the new entry fully in a local
variable, do the check to determine the flushing needs on the thus
established new entry, and then write the new entry by a single insn.

Similarly using memset() to clear a PTE is unsafe, as the order of
writes the function does is, at least in principle, undefined.

This is part of XSA-347.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
2020-10-20 14:22:52 +02:00
Jan Beulich 73f62c7380 AMD/IOMMU: convert amd_iommu_pte from struct to union
This is to add a "raw" counterpart to the bitfield equivalent. Take the
opportunity and
 - convert fields to bool / unsigned int,
 - drop the naming of the reserved field,
 - shorten the names of the ignored ones.

This is part of XSA-347.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Paul Durrant <paul@xen.org>
2020-10-20 14:22:26 +02:00
Jan Beulich 5777a3742d IOMMU: hold page ref until after deferred TLB flush
When moving around a page via XENMAPSPACE_gmfn_range, deferring the TLB
flush for the "from" GFN range requires that the page remains allocated
to the guest until the TLB flush has actually occurred. Otherwise a
parallel hypercall to remove the page would only flush the TLB for the
GFN it has been moved to, but not the one is was mapped at originally.

This is part of XSA-346.

Fixes: cf95b2a9fd ("iommu: Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary iotlb... ")
Reported-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
2020-10-20 14:21:32 +02:00
Jan Beulich dea460d869 IOMMU: suppress "iommu_dont_flush_iotlb" when about to free a page
Deferring flushes to a single, wide range one - as is done when
handling XENMAPSPACE_gmfn_range - is okay only as long as
pages don't get freed ahead of the eventual flush. While the only
function setting the flag (xenmem_add_to_physmap()) suggests by its name
that it's only mapping new entries, in reality the way
xenmem_add_to_physmap_one() works means an unmap would happen not only
for the page being moved (but not freed) but, if the destination GFN is
populated, also for the page being displaced from that GFN. Collapsing
the two flushes for this GFN into just one (end even more so deferring
it to a batched invocation) is not correct.

This is part of XSA-346.

Fixes: cf95b2a9fd ("iommu: Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary iotlb... ")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Acked-by: Julien Grall <jgrall@amazon.com>
2020-10-20 14:21:09 +02:00
Hongyan Xia 1ce75e99d7 x86/mm: Prevent some races in hypervisor mapping updates
map_pages_to_xen will attempt to coalesce mappings into 2MiB and 1GiB
superpages if possible, to maximize TLB efficiency.  This means both
replacing superpage entries with smaller entries, and replacing
smaller entries with superpages.

Unfortunately, while some potential races are handled correctly,
others are not.  These include:

1. When one processor modifies a sub-superpage mapping while another
processor replaces the entire range with a superpage.

Take the following example:

Suppose L3[N] points to L2.  And suppose we have two processors, A and
B.

* A walks the pagetables, get a pointer to L2.
* B replaces L3[N] with a 1GiB mapping.
* B Frees L2
* A writes L2[M] #

This is race exacerbated by the fact that virt_to_xen_l[21]e doesn't
handle higher-level superpages properly: If you call virt_xen_to_l2e
on a virtual address within an L3 superpage, you'll either hit a BUG()
(most likely), or get a pointer into the middle of a data page; same
with virt_xen_to_l1 on a virtual address within either an L3 or L2
superpage.

So take the following example:

* A reads pl3e and discovers it to point to an L2.
* B replaces L3[N] with a 1GiB mapping
* A calls virt_to_xen_l2e() and hits the BUG_ON() #

2. When two processors simultaneously try to replace a sub-superpage
mapping with a superpage mapping.

Take the following example:

Suppose L3[N] points to L2.  And suppose we have two processors, A and B,
both trying to replace L3[N] with a superpage.

* A walks the pagetables, get a pointer to pl3e, and takes a copy ol3e pointing to L2.
* B walks the pagetables, gets a pointre to pl3e, and takes a copy ol3e pointing to L2.
* A writes the new value into L3[N]
* B writes the new value into L3[N]
* A recursively frees all the L1's under L2, then frees L2
* B recursively double-frees all the L1's under L2, then double-frees L2 #

Fix this by grabbing a lock for the entirety of the mapping update
operation.

Rather than grabbing map_pgdir_lock for the entire operation, however,
repurpose the PGT_locked bit from L3's page->type_info as a lock.
This means that rather than locking the entire address space, we
"only" lock a single 512GiB chunk of hypervisor address space at a
time.

There was a proposal for a lock-and-reverify approach, where we walk
the pagetables to the point where we decide what to do; then grab the
map_pgdir_lock, re-verify the information we collected without the
lock, and finally make the change (starting over again if anything had
changed).  Without being able to guarantee that the L2 table wasn't
freed, however, that means every read would need to be considered
potentially unsafe.  Thinking carefully about that is probably
something that wants to be done on public, not under time pressure.

This is part of XSA-345.

Reported-by: Hongyan Xia <hongyxia@amazon.com>
Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-10-20 14:20:19 +02:00
Wei Liu b733f8a8b8 x86/mm: Refactor modify_xen_mappings to have one exit path
We will soon need to perform clean-ups before returning.

No functional change.

This is part of XSA-345.

Reported-by: Hongyan Xia <hongyxia@amazon.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-10-20 14:19:55 +02:00
Wei Liu 08e6c6f80b x86/mm: Refactor map_pages_to_xen to have only a single exit path
We will soon need to perform clean-ups before returning.

No functional change.

This is part of XSA-345.

Reported-by: Hongyan Xia <hongyxia@amazon.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-10-20 14:19:31 +02:00
Jan Beulich a7f0831e58 SVM: avoid VMSAVE in ctxt-switch-to
Of the state saved by the insn and reloaded by the corresponding VMLOAD
- TR and syscall state are invariant while having Xen's state loaded,
- sysenter is unused altogether by Xen,
- FS, GS, and LDTR are not used by Xen and get suitably set in PV
  context switch code.
Note that state is suitably populated in _svm_cpu_up(); a minimal
respective assertion gets added.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-10-20 08:54:59 +02:00
Igor Druzhinin de6d188a51 hvmloader: flip "ACPI data" to "ACPI NVS" type for ACPI table region
ACPI specification contains statements describing memory marked with regular
"ACPI data" type as reclaimable by the guest. Although the guest shouldn't
really do it if it wants kexec or similar functionality to work, there
could still be ambiguities in treating these regions as potentially regular
RAM.

One such example is SeaBIOS which currently reports "ACPI data" regions as
RAM to the guest in its e801 call. Which it might have the right to do as any
user of this is expected to be ACPI unaware. But a QEMU bootloader later seems
to ignore that fact and is instead using e801 to find a place for initrd which
causes the tables to be erased. While arguably QEMU bootloader or SeaBIOS need
to be fixed / improved here, that is just one example of the potential problems
from using a reclaimable memory type.

Flip the type to "ACPI NVS" which doesn't have this ambiguity in it and is
described by the spec as non-reclaimable (so cannot ever be treated like RAM).

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-10-20 08:54:23 +02:00
Jan Beulich 7b36d16d21 xen-detect: make CPUID fallback CPUID-faulting aware
Relying on presence / absence of hypervisor leaves in raw / escaped
CPUID output cannot be used to tell apart PV and HVM on CPUID faulting
capable hardware. Utilize a PV-only feature flag to avoid false positive
HVM detection.

While at it also short circuit the main detection loop: For PV, only
the base group of leaves can possibly hold hypervisor information.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-20 08:53:53 +02:00
Jan Beulich 25467bb5d1 EFI: free unused boot mem in at least some cases
Address at least the primary reason why 52bba67f8b ("efi/boot: Don't
free ebmalloc area at all") was put in place: Make xen_in_range() aware
of the freed range. This is in particular relevant for EFI-enabled
builds not actually running on EFI, as the entire range will be unused
in this case.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
2020-10-20 08:52:53 +02:00
Bertrand Marquis 0dfddb2116 tools/xenpmd: Fix gcc10 snprintf warning
Add a check for snprintf return code and ignore the entry if we get an
error. This should in fact never happen and is more a trick to make gcc
happy and prevent compilation errors.

This is solving the following gcc warning when compiling for arm32 host
platforms with optimization activated:
xenpmd.c:92:37: error: '%s' directive output may be truncated writing
between 4 and 2147483645 bytes into a region of size 271
[-Werror=format-truncation=]

This is also solving the following Debian bug:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=970802

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-16 13:28:55 +00:00
Elliott Mitchell 17d192e023 tools/python: Pass linker to Python build process
Unexpectedly the environment variable which needs to be passed is
$LDSHARED and not $LD.  Otherwise Python may find the build `ld` instead
of the host `ld`.

Replace $(LDFLAGS) with $(SHLIB_LDFLAGS) as Python needs shared objects
it can load at runtime, not executables.

This uses $(CC) instead of $(LD) since Python distutils appends $CFLAGS
to $LDFLAGS which breaks many linkers.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
2020-10-16 13:25:35 +00:00
Bertrand Marquis 40fe714ca4 tools/libs/stat: use memcpy instead of strncpy in getBridge
Use memcpy in getBridge to prevent gcc warnings about truncated
strings. We know that we might truncate it, so the gcc warning
here is wrong.
Revert previous change changing buffer sizes as bigger buffers
are not needed.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-16 13:23:34 +00:00
Andrew Cooper a7952a320c x86/smpboot: Don't unconditionally call memguard_guard_stack() in cpu_smpboot_alloc()
cpu_smpboot_alloc() is designed to be idempotent with respect to partially
initialised state.  This occurs for S3 and CPU parking, where enough state to
handle NMIs/#MCs needs to remain valid for the entire lifetime of Xen, even
when we otherwise want to offline the CPU.

For simplicity between various configuration, Xen always uses shadow stack
mappings (Read-only + Dirty) for the guard page, irrespective of whether
CET-SS is enabled.

Unfortunately, the CET-SS changes in memguard_guard_stack() broke idempotency
by first writing out the supervisor shadow stack tokens with plain writes,
then changing the mapping to being read-only.

This ordering is strictly necessary to configure the BSP, which cannot have
the supervisor tokens be written with WRSS.

Instead of calling memguard_guard_stack() unconditionally, call it only when
actually allocating a new stack.  Xenheap allocates are guaranteed to be
writeable, and the net result is idempotency WRT configuring stack_base[].

Fixes: 91d26ed304 ("x86/shstk: Create shadow stacks")
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-10-16 11:56:59 +01:00
Andrew Cooper 04182d8b79 x86/ucode/intel: Improve description for gathering the microcode revision
Obtaining the microcode revision on Intel CPUs is complicated for backwards
compatibility reasons.  Update apply_microcode() to use a slightly more
efficient CPUID invocation, now that the documentation has been updated to
confirm that any CPUID instruction is fine, not just CPUID.1

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-10-16 11:55:33 +01:00
Andrew Cooper 6065a05adf x86/traps: 'Fix' safety of read_registers() in #DF path
All interrupts and exceptions pass a struct cpu_user_regs up into C.  This
contains the legacy vm86 fields from 32bit days, which are beyond the
hardware-pushed frame.

Accessing these fields is generally illegal, as they are logically out of
bounds for anything other than an interrupt/exception hitting ring1/3 code.

show_registers() unconditionally reads these fields, but the content is
discarded before use.  This is benign right now, as all parts of the stack are
readable, including the guard pages.

However, read_registers() in the #DF handler writes to these fields as part of
preparing the state dump, and being IST, hits the adjacent stack frame.

This has been broken forever, but c/s 6001660473 "x86/shstk: Rework the stack
layout to support shadow stacks" repositioned the #DF stack to be adjacent to
the guard page, which turns this OoB write into a fatal pagefault:

  (XEN) *** DOUBLE FAULT ***
  (XEN) ----[ Xen-4.15-unstable  x86_64  debug=y   Tainted:  C   ]----
  (XEN) ----[ Xen-4.15-unstable  x86_64  debug=y   Tainted:  C   ]----
  (XEN) CPU:    4
  (XEN) RIP:    e008:[<ffff82d04031fd4f>] traps.c#read_registers+0x29/0xc1
  (XEN) RFLAGS: 0000000000050086   CONTEXT: hypervisor (d1v0)
  ...
  (XEN) Xen call trace:
  (XEN)    [<ffff82d04031fd4f>] R traps.c#read_registers+0x29/0xc1
  (XEN)    [<ffff82d0403207b3>] F do_double_fault+0x3d/0x7e
  (XEN)    [<ffff82d04039acd7>] F double_fault+0x107/0x110
  (XEN)
  (XEN) Pagetable walk from ffff830236f6d008:
  (XEN)  L4[0x106] = 80000000bfa9b063 ffffffffffffffff
  (XEN)  L3[0x008] = 0000000236ffd063 ffffffffffffffff
  (XEN)  L2[0x1b7] = 0000000236ffc063 ffffffffffffffff
  (XEN)  L1[0x16d] = 8000000236f6d161 ffffffffffffffff
  (XEN)
  (XEN) ****************************************
  (XEN) Panic on CPU 4:
  (XEN) FATAL PAGE FAULT
  (XEN) [error_code=0003]
  (XEN) Faulting linear address: ffff830236f6d008
  (XEN) ****************************************
  (XEN)

and rendering the main #DF analysis broken.

The proper fix is to delete cpu_user_regs.es and later, so no
interrupt/exception path can access OoB, but this needs disentangling from the
PV ABI first.

Not-really-fixes: 6001660473 ("x86/shstk: Rework the stack layout to support shadow stacks")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-10-16 11:55:33 +01:00
Jan Beulich 6ee2e66674 tools/gdbsx: drop stray recursion into tools/include/
Doing so isn't appropriate here - this gets done very early in the build
process. If the directory is mean to to be buildable on its own,
different arrangements would be needed.

The issue has become more pronounced by 47654a0d73 ("tools/include:
fix (drop) dependencies of when to populate xen/"), but was there before
afaict.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-10-15 17:18:25 +02:00
Bertrand Marquis 27addcca15 xen/arm: print update firmware only once
Fix enable_smccc_arch_workaround_1 to only print the warning asking to
update the firmware once.

Fixes: 976319fa3d ("xen/arm64: Kill PSCI_GET_VERSION as a variant-2 workaround")
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
2020-10-15 15:33:17 +01:00
Jan Beulich a8a85f03c8 EFI: further "need_to_free" adjustments
When processing "chain" directives, the previously loaded config file
gets freed. This needs to be recorded accordingly such that no error
path would try to free the same block of memory a 2nd time.

Furthermore, neither .addr nor .size being zero has any meaning towards
the need to free an allocated chunk anymore. Drop (from read_file()) and
replace (in Arm's efi_arch_use_config_file(), to sensibly retain the
comment) respective assignments.

Fixes: 04be2c3a06 ("efi/boot.c: add file.need_to_free")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2020-10-15 12:30:01 +02:00
Chen Yu 44ac57af81 x86/mwait-idle: customize IceLake server support
On ICX platform, the C1E auto-promotion is enabled by default.
As a result, the CPU might fall into C1E more offen than previous
platforms. So disable C1E auto-promotion and expose C1E as a separate
idle state.

Beside C1 and C1E, the exit latency of C6 was measured
by a dedicated tool. However the exit latency(41us) exposed
by _CST is much smaller than the one we measured(128us). This
is probably due to the _CST uses the exit latency when woken
up from PC0+C6, rather than PC6+C6 when C6 was measured. Choose
the latter as we need the longest latency in theory.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
[Linux commit a472ad2bcea479ba068880125d7273fc95c14b70]
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-10-15 12:29:11 +02:00
Michal Orzel f776e5fb3e xen/arm: Document the erratum #853709 related to Cortex A72
The Cortex-A72 erratum #853709 is the same as the Cortex-A57
erratum #852523. As the latter is already workaround, we only
need to update the documentation.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
[julieng: Reworded the commit message]
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
2020-10-14 18:02:50 +01:00
Jan Beulich 884ef07f4f EFI/Arm64: don't clobber DTB pointer
read_section() needs to be more careful: efi_arch_use_config_file()
may have found a DTB file (but without modules), and there may be no DTB
specified in the EFI config file. In this case the pointer to the blob
must not be overwritten with NULL when no ".dtb" section is present
either.

Fixes: 8a71d50ed4 ("efi: Enable booting unified hypervisor/kernel/initrd images")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-10-14 14:13:16 +02:00
Jan Beulich e3daad6283 kexec: some #include adjustments
In the context of working on x86's elf_core_save_regs() I noticed there
were far more source files getting rebuilt than I would have expected.
While the main offender looks to have been fixmap.h including kexec.h,
also drop use of elfcore.h from kexec.h.

While adjusting machine_kexec.c also replace use of guest_access.h by
domain_page.h.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-10-14 14:11:49 +02:00
Jan Beulich f14a4226bf drop xen/hash.h
It has no users and hasn't been touched in 10 years.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-10-14 14:10:49 +02:00
Roger Pau Monné 6280558224 x86/msr: handle IA32_THERM_STATUS
Windows 8 will attempt to read MSR_IA32_THERM_STATUS and panic if a
#GP fault is injected as a result:

vmx.c:3035:d8v0 RDMSR 0x0000019c unimplemented
d8v0 VIRIDIAN CRASH: 3b c0000096 fffff8061de31651 fffff4088a613720 0

So handle the MSR and return 0 instead.

Note that this is done on the generic MSR handler, and PV guest will
also get 0 back when trying to read the MSR. There doesn't seem to be
much value in handling the MSR for HVM guests only.

Fixes: 84e848fd7a ('x86/hvm: disallow access to unknown MSRs')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-10-14 14:10:08 +02:00
Jan Beulich 8752485279 x86/vLAPIC: vlapic_init() runs only once for a vCPU
Hence there's no need to guard allocation / mapping by checks whether
the same action has been done before. I assume this was a transient
change which should have been undone before 509529e991 ("x86 hvm: Xen
interface and implementation for virtual S3") got committed.

While touching this code, drop the pretty useless dprintk()-s.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-10-14 14:05:10 +02:00
Jan Beulich 6a34e67c11 x86: fix resource leaks on arch_vcpu_create() error path
{hvm,pv}_vcpu_initialise() have always kind of been meant to be the
final possible source of errors in arch_vcpu_create(), hence not
requiring any unrolling of what they've done on the error path. (Of
course this may change once the various involved paths all have become
idempotent.)

But even beyond this aspect I think it is more logical to do policy
initialization ahead of the calling of these two functions, as they may
in principle want to access it.

Fixes: 4187f79dc7 ("x86/msr: introduce struct msr_vcpu_policy")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-10-14 14:03:38 +02:00
Jan Beulich 01d687c9cd tools/include: drop remaining -f from ln invocations
This is once again to allow noticing name collisions right away.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-14 14:02:40 +02:00
Jan Beulich c02fd5b1a4 tools/include: adjust x86-specific population of xen/
There's no need to use a shell loop construct here - ln's destination
can be specified as just the intended directory, as we don't mean to
change the names of any of the files. Also drop XEN_LIB_X86_INCLUDES for
having a single use only, and don't pass -f to ln, to allow noticing
name collisions right away.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-14 14:02:20 +02:00
Jan Beulich 3d77849197 tools/include: properly install Arm public headers
When smccc.h was added, adjustment of the install rule was missed.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-14 14:02:03 +02:00
Jan Beulich edc8d9136b tools/include: adjust population of public headers into xen/
Use a wildcard also for the subdirectories, drop XEN_PUBLIC_INCLUDES for
having a single use only, and don't pass -f to ln to allow noticing name
collisions right away, and add trailing slashes to ln's destination.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-14 14:01:43 +02:00
Jan Beulich 47654a0d73 tools/include: fix (drop) dependencies of when to populate xen/
Making the population of xen/ depend on the time stamps of a subset of
the headers put there is error prone. The creation of a few dozen
symlinks doesn't take a meaningful amount of time (compared to the
overall building of tools/), and hence - to be on the safe side - should
simply be done always. Convert the goal to a phony one and drop its
dependencies, effectively taking further what 8d8d7d6b3d ("tools: fix
linking hypervisor includes to tools include directory") had already
attempted.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-14 14:01:25 +02:00
Jan Beulich 8ea798eb4b tools/include: adjust population of acpi/
Limit what gets exposed - in particular cpufreq/ and apei.h are
hypervisor private headers which the tool stack building shouldn't see
or use. Also don't pass -f to ln, to allow noticing name collisions
right away.

Additionally acpi/ also has been in need of deleting at the start of
the rule, or alternatively the respective ln would have needed to also
be passed -n.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-14 14:01:00 +02:00
Bertrand Marquis 9e5a9d0e68 build: always use BASEDIR for xen sub-directory
Modify Makefiles using $(XEN_ROOT)/xen to use $(BASEDIR) instead.

This is removing the dependency to xen subdirectory preventing using a
wrong configuration file when xen subdirectory is duplicated for
compilation tests.

BASEDIR is set in xen/lib/x86/Makefile as this Makefile is directly
called from the tools build and install process and BASEDIR is not set
there.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-13 14:49:35 +00:00
Nick Rosbrook a95f31376b golang/xenlight: standardize generated code comment
There is a standard format for generated Go code header comments, as set
by [1]. Modify gengotypes.py to follow this standard, and use the
additional

  // source: <IDL file basename>

convention used by protoc-gen-go.

This change is motivated by the fact that since 41aea82de2, the comment
would include the absolute path to libxl_types.idl, therefore creating
unintended diffs when generating code across different machines. This
approach fixes that problem.

[1] https://github.com/golang/go/issues/13560

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2020-10-13 13:31:06 +00:00