Dario Faggioli [Wed, 11 Mar 2020 18:45:12 +0000 (19:45 +0100)]
xen: credit2: fix credit reset happening too few times
There is a bug in commit 5e4b4199667b9 ("xen: credit2: only reset
credit on reset condition"). In fact, the aim of that commit was to
make sure that we do not perform too many credit reset operations
(which are not super cheap, and in an hot-path). But the check used
to determine whether a reset is necessary was the wrong one.
In fact, knowing just that some vCPUs have been skipped, while
traversing the runqueue (in runq_candidate()), is not enough. We
need to check explicitly whether the first vCPU in the runqueue
has a negative amount of credit.
Since a trace record is changed, this patch updates xentrace format file
and xenalyze as well
This should be backported.
Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
--- Cc: George Dunlap <george.dunlap@citrix.com> Cc: Juergen Gross <jgross@suse.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Charles Arnold <carnold@suse.com> Cc: Glen <glenbarney@gmail.com> Cc: Tomas Mozes <hydrapolic@gmail.com> Cc: Sarah Newman <srn@prgmr.com>
---
About the Credit2 stall issue reported recently, and mentioned in patch
1 of this series. This second patch, alone, was already mitigating the
issue quite substantially.
Still, the proper fix for the issue itself is patch 1, while this is a
fix for a bug in the code, introduced with a previous change, which
happens to help to cure the sympthoms of the problem at hand.
Dario Faggioli [Wed, 11 Mar 2020 18:13:59 +0000 (19:13 +0100)]
xen: credit2: avoid vCPUs to ever reach lower credits than idle
There have been report of stalls of guest vCPUs, when Credit2 was used.
It seemed like these vCPUs were not getting scheduled for very long
time, even under light load conditions (e.g., during dom0 boot).
Investigations led to the discovery that --although rarely-- it can
happen that a vCPU manages to run for very long timeslices. In Credit2,
this means that, when runtime accounting happens, the vCPU will lose a
large quantity of credits. This in turn may lead to the vCPU having less
credits than the idle vCPUs (-2^30). At this point, the scheduler will
pick the idle vCPU, instead of the ready to run vCPU, for a few
"epochs", which often times is enough for the guest kernel to think the
vCPU is not responding and crashing.
An example of this situation is shown here. In fact, we can see d0v1
sitting in the runqueue while all the CPUs are idle, as it has
-1254238270 credits, which is smaller than -2^30 = −1073741824:
We certainly don't want, under any circumstance, this to happen.
Let's, therefore, define a minimum amount of credits a vCPU can have.
During accounting, we make sure that, for however long the vCPU has
run, it will never get to have less than such minimum amount of
credits. Then, we set the credits of the idle vCPU to an even
smaller value.
NOTE: investigations have been done about _how_ it is possible for a
vCPU to execute for so much time that its credits becomes so low. While
still not completely clear, there are evidence that:
- it only happens very rarely,
- it appears to be both machine and workload specific,
- it does not look to be a Credit2 (e.g., as it happens when
running with Credit1 as well) issue, or a scheduler issue.
This patch makes Credit2 more robust to events like this, whatever
the cause is, and should hence be backported (as far as possible).
Reported-by: Glen <glenbarney@gmail.com> Reported-by: Tomas Mozes <hydrapolic@gmail.com> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
--- Cc: George Dunlap <george.dunlap@citrix.com> Cc: Juergen Gross <jgross@suse.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Charles Arnold <carnold@suse.com> Cc: Sarah Newman <srn@prgmr.com>
---
Changes from v1:
- different approach. Instead than using INT_MIN for idle vCPUs'
credits, limit the minimum number of credits regular vCPUs can have.
---
I will provide the backports myself, at least for 4.13 and 4.12.x (and
feel free to ask for more).
---
For Sarah, looking back at the various threads, I am not quite sure
whether you also experienced the issue and reported it. If yes, I'm
happy to add a "Reported-by:" line about you too (or, if this is fine to
go in, for this to be done while committing, if possible).
Igor Druzhinin [Wed, 18 Mar 2020 11:55:54 +0000 (12:55 +0100)]
x86/shim: fix ballooning up the guest
args.preempted is meaningless here as it doesn't signal whether the
hypercall was preempted before. Use start_extent instead which is
correct (as long as the hypercall was invoked in a "normal" way).
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Tue, 17 Mar 2020 15:20:08 +0000 (16:20 +0100)]
libfdt: fix undefined behaviour in _fdt_splice()
Along the lines of commit d0b3ab0a0f46 ("libfdt: Fix undefined behaviour
in fdt_offset_ptr()"), _fdt_splice() similarly may not use pointer
arithmetic to do overflow checks.
David Gibson [Tue, 17 Mar 2020 15:18:57 +0000 (16:18 +0100)]
libfdt: Fix undefined behaviour in fdt_offset_ptr()
Using pointer arithmetic to generate a pointer outside a known object is,
technically, undefined behaviour in C. Unfortunately, we were using that
in fdt_offset_ptr() to detect overflows.
To fix this we need to do our bounds / overflow checking on the offsets
before constructing pointers from them.
Reported-by: David Binderman <dcb314@hotmail.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[upstream commit d0b3ab0a0f46ac929b4713da46f7fdcd893dd3bd] Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <jgrall@amazon.com>
Jan Beulich [Tue, 17 Mar 2020 15:17:20 +0000 (16:17 +0100)]
x86/HVM: reduce hvm.h include dependencies
Drop #include-s not needed by the header itself, and add smaller scope
ones instead. Put the ones needed into whichever other files actually
need them.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Tue, 17 Mar 2020 15:16:34 +0000 (16:16 +0100)]
x86/HVM: reduce io.h include dependencies
Drop #include-s not needed by the header itself as well as one include
of the header which isn't needed. Put the one needed into the file
actually requiring it.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Tue, 17 Mar 2020 15:14:57 +0000 (16:14 +0100)]
x86/HVM: reduce vioapic.h include dependencies
Drop an #include not needed by the header itself. While verifying the
header (now) builds standalone, I noticed an omission in a public header
which gets taken care of here as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper [Wed, 11 Mar 2020 18:22:37 +0000 (18:22 +0000)]
x86/vvmx: Fix deadlock with MSR bitmap merging
c/s c47984aabead "nvmx: implement support for MSR bitmaps" introduced a use of
map_domain_page() which may get used in the middle of context switch.
This is not safe, and causes Xen to deadlock on the mapcache lock:
(XEN) Xen call trace:
(XEN) [<ffff82d08022d6ae>] R _spin_lock+0x34/0x5e
(XEN) [<ffff82d0803219d7>] F map_domain_page+0x250/0x527
(XEN) [<ffff82d080356332>] F do_page_fault+0x420/0x780
(XEN) [<ffff82d08038da3d>] F x86_64/entry.S#handle_exception_saved+0x68/0x94
(XEN) [<ffff82d08031729f>] F __find_next_zero_bit+0x28/0x69
(XEN) [<ffff82d080321a4d>] F map_domain_page+0x2c6/0x527
(XEN) [<ffff82d08029eeb2>] F nvmx_update_exec_control+0x1d7/0x323
(XEN) [<ffff82d080299f5a>] F vmx_update_cpu_exec_control+0x23/0x40
(XEN) [<ffff82d08029a3f7>] F arch/x86/hvm/vmx/vmx.c#vmx_ctxt_switch_from+0xb7/0x121
(XEN) [<ffff82d08031d796>] F arch/x86/domain.c#__context_switch+0x124/0x4a9
(XEN) [<ffff82d080320925>] F context_switch+0x154/0x62c
(XEN) [<ffff82d080252f3e>] F common/sched/core.c#sched_context_switch+0x16a/0x175
(XEN) [<ffff82d080253877>] F common/sched/core.c#schedule+0x2ad/0x2bc
(XEN) [<ffff82d08022cc97>] F common/softirq.c#__do_softirq+0xb7/0xc8
(XEN) [<ffff82d08022cd38>] F do_softirq+0x18/0x1a
(XEN) [<ffff82d0802a2fbb>] F vmx_asm_do_vmentry+0x2b/0x30
Convert the domheap page into being a xenheap page.
Fixes: c47984aabead - nvmx: implement support for MSR bitmaps Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich [Mon, 16 Mar 2020 16:32:41 +0000 (17:32 +0100)]
x86/APIC: reduce rounding errors in calculations
Dividing by HZ/10 just to subsequently multiply by HZ again in all uses
of the respective variable is pretty pointlessly introducing rounding
(really: truncation) errors. While transforming the respective
expressions it became apparent that "result" would be left unused except
for its use as function return value. As the sole caller of the function
doesn't look at the returned value, simply convert the function to have
"void" return type.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Mon, 16 Mar 2020 16:31:35 +0000 (17:31 +0100)]
x86/time: reduce rounding errors in calculations
Plain (unsigned) integer division simply truncates the results. The
overall errors are smaller though if we use proper rounding. (Extend
this to the purely cosmetic aspect of time.c's freq_string(), which
before this change I've frequently observed to report e.g. NN.999MHz
HPET clock speeds.)
While adding the rounding logic, also switch to using an unsigned
constant for the other, original half of bus_cycle's calculation.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Juergen Gross [Mon, 16 Mar 2020 10:27:29 +0000 (11:27 +0100)]
spinlocks: fix placement of preempt_[dis|en]able()
In case Xen ever gains preemption support the spinlock coding's
placement of preempt_disable() and preempt_enable() should be outside
of the locked section.
Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Mon, 16 Mar 2020 10:26:10 +0000 (11:26 +0100)]
x86/APIC: adjust types and comments in calibrate_APIC_clock()
First and foremost the comment talking about potential underflow being
taken care of by using signed long type variables was true only on
32-bit, which we've not been supporting for quite some time. Drop the
comment and change all involved types to unsigned. Take the opportunity
and also replace bus_cycle's fixed width type.
Additionally there's no point using an "arbitrary (but long enough)
timeout" here. Just use the maximum possible value; Linux does so too,
just as an additional data point.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Mon, 16 Mar 2020 10:25:45 +0000 (11:25 +0100)]
kconfig: expose all{yes,no}config targets
Without having them at least at the xen/Makefile level they're (close
to?) inaccessible. As I'm uncertain about their utility at the top
level, I'm leaving it at that for now.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Wei Liu <wl@xen.org> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Mon, 16 Mar 2020 10:24:29 +0000 (11:24 +0100)]
AMD/IOMMU: fix off-by-one in amd_iommu_get_paging_mode() callers
amd_iommu_get_paging_mode() expects a count, not a "maximum possible"
value. Prior to b4f042236ae0 dropping the reference, the use of our mis-
named "max_page" in amd_iommu_domain_init() may have lead to such a
misunderstanding. In an attempt to avoid such confusion in the future,
rename the function's parameter and - while at it - convert it to an
inline function.
Also replace a literal 4 by an expression tying it to a wider use
constant, just like amd_iommu_quarantine_init() does.
Fixes: ea38867831da ("x86 / iommu: set up a scratch page in the quarantine domain") Fixes: b4f042236ae0 ("AMD/IOMMU: Cease using a dynamic height for the IOMMU pagetables") Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Paweł Marczewski [Fri, 13 Mar 2020 11:25:10 +0000 (11:25 +0000)]
libxl: fix cleanup bug in initiate_domain_create()
In case of errors, we immediately call domcreate_complete()
which cleans up the console_xswait object. Make sure it is initialized
before we start cleanup.
Signed-off-by: Paweł Marczewski <pawel@invisiblethingslab.com> Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Roger Pau Monne [Fri, 13 Mar 2020 08:45:58 +0000 (09:45 +0100)]
libfsimage: fix parentheses in macro parameters
VERIFY_DN_TYPE and VERIFY_OS_TYPE should use parentheses when
accessing the type parameter. Note that none of the current usages
require this, it's just done for correctness.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Wei Liu <wl@xen.org>
Juergen Gross [Wed, 11 Mar 2020 12:17:41 +0000 (13:17 +0100)]
rcu: use rcu softirq for forcing quiescent state
As rcu callbacks are processed in __do_softirq() there is no need to
use the scheduling softirq for forcing quiescent state. Any other
softirq would do the job and the scheduling one is the most expensive.
So use the already existing rcu softirq for that purpose. For telling
apart why the rcu softirq was raised add a flag for the current usage.
Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Tue, 10 Mar 2020 16:06:57 +0000 (17:06 +0100)]
memaccess: reduce include dependencies
The common header doesn't itself need to include public/vm_event.h nor
public/memory.h. Drop their inclusion. This requires using the non-
typedef names in two prototypes and an inline function; by not changing
the callers and function definitions at the same time it'll remain
certain that the build would fail if the typedef itself was changed.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Paul Durrant [Tue, 10 Mar 2020 16:06:09 +0000 (17:06 +0100)]
x86 / p2m: replace page_list check in p2m_alloc_table...
... with a check of domain_tot_pages().
The check of page_list prevents the prior allocation of PGC_extra pages,
whereas what the code is trying to verify is that the toolstack has not
already RAM for the domain.
Signed-off-by: Paul Durrant <paul@xen.org> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Tue, 10 Mar 2020 14:38:25 +0000 (15:38 +0100)]
vmevent: reduce include dependencies
There's no need for virtually everything to include public/vm_event.h.
Move its inclusion out of sched.h. This requires using the non-typedef
name in p2m_mem_paging_resume()'s prototype; by not changing the
function definition at the same time it'll remain certain that the build
would fail if the typedef itself was changed.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Ross Lagerwall <ross.lagerwall@citrix.com> Reviewed-by: Alexandru Isaila <aisaila@bitdefender.com> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
The current implementation of the hypervisor assisted flush for HAP is
extremely inefficient.
First of all there's no need to call paging_update_cr3, as the only
relevant part of that function when doing a flush is the ASID vCPU
flush, so just call that function directly.
Since hvm_asid_flush_vcpu is protected against concurrent callers by
using atomic operations there's no need anymore to pause the affected
vCPUs.
Finally the global TLB flush performed by flush_tlb_mask is also not
necessary, since we only want to flush the guest TLB state it's enough
to trigger a vmexit on the pCPUs currently holding any vCPU state, as
such vmexit will already perform an ASID/VPID update, and thus clear
the guest TLB.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Wei Liu <wl@xen.org> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Roger Pau Monné [Tue, 10 Mar 2020 14:29:24 +0000 (15:29 +0100)]
x86/paging: add TLB flush hook
Add shadow and hap implementation specific helpers to perform guest
TLB flushes. Note that the code for both is exactly the same at the
moment, and is copied from hvm_flush_vcpu_tlb. This will be changed by
further patches that will add implementation specific optimizations to
them.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Wei Liu <wl@xen.org> Acked-by: Tim Deegan <tim@xen.org> Reviewed-by: Paul Durrant <pdurrant@amzn.com> [viridian] Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Tue, 10 Mar 2020 14:25:58 +0000 (15:25 +0100)]
AMD/IOMMU: without XT, x2APIC needs to be forced into physical mode
The wider cluster mode APIC IDs aren't generally representable. Convert
the iommu_intremap variable into a tristate, allowing the AMD IOMMU
driver to signal this special restriction to the apic_x2apic_probe().
(Note: assignments to the variable get adjusted, while existing
consumers - all assuming a boolean property - are left alone.)
While we are not aware of any hardware/firmware with this as a
restriction, it is a situation which could be created on fully x2apic-
capable systems via firmware settings.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
George Dunlap [Thu, 5 Mar 2020 11:34:07 +0000 (11:34 +0000)]
golang/xenlight: Fix handling of marshalling of empty elements for keyed unions
Keyed types in libxl_types.idl can have elements of type 'None'. The
golang type generator (correctly) don't implement any union types for
these empty elements. However, the toC and fromC helper generators
incorrectly treat these elements as invalid.
Consider for example, libxl_channelinfo. The idl contains the
following keyed element:
Jan Beulich [Mon, 9 Mar 2020 09:00:26 +0000 (10:00 +0100)]
VT-d: fix and extend RMRR reservation check
First of all in commit d6573bc6e6b7 ("VT-d: check all of an RMRR for
being E820-reserved") along with changing the function used, the enum-
like value passed should have been changed too (to E820_*). Do so now.
(Luckily the actual values of RAM_TYPE_RESERVED and E820_RESERVED
match, so the breakage introduced was "only" latent.)
Furthermore one of my systems surfaces RMRR in an ACPI NVS E820 range.
The purpose of the check is just to make sure there won't be "ordinary"
mappings of these ranges, and domains (including Dom0) won't want to
use the region to e.g. put PCI device BARs there. The two ACPI related
E820 types are good enough for this purpose, so allow them as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Roger Pau Monné [Fri, 6 Mar 2020 09:18:13 +0000 (10:18 +0100)]
x86/hvm: allow ASID flush when v != current
Current implementation of hvm_asid_flush_vcpu is not safe to use
unless the target vCPU is either paused or the currently running one,
as it modifies the generation without any locking.
Fix this by using atomic operations when accessing the generation
field, both in hvm_asid_flush_vcpu_asid and other ASID functions. This
allows to safely flush the current ASID generation. Note that for the
flush to take effect if the vCPU is currently running a vmexit is
required.
Compilers will normally do such writes and reads as a single
instruction, so the usage of atomic operations is mostly used as a
safety measure.
Note the same could be achieved by introducing an extra field to
hvm_vcpu_asid that signals hvm_asid_handle_vmenter the need to call
hvm_asid_flush_vcpu on the given vCPU before vmentry, this however
seems unnecessary as hvm_asid_flush_vcpu itself only sets two vCPU
fields to 0, so there's no need to delay this to the vmentry ASID
helper.
This is not a bugfix as no callers that would violate the assumptions
listed in the first paragraph have been found, but a preparatory
change in order to allow remote flushing of HVM vCPUs.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Wei Liu <wl@xen.org> Acked-by: Jan Beulich <jbeulich@suse.com>
Anthony PERARD [Fri, 6 Mar 2020 09:15:49 +0000 (10:15 +0100)]
build: run targets csopes,tags,.. without Rules.mk
Those targets make use of $(all_sources) which depends on TARGET_ARCH,
so we just need to set TARGET_ARCH earlier and once.
XEN_TARGET_ARCH isn't expected to change during the build, so
TARGET_SUBARCH and TARGET_ARCH aren't going to change either. Set them
once and for all in the Xen root Makefile. This allows to run more
targets without Rules.mk.
XEN_TARGET_ARCH is actually changed in arch/x86/boot/build32.mk, but
it doesn't use the TARGET_{,SUB}ARCH variables either, and doesn't use
Rules.mk (it replaces it).
TARGET_{,SUB}ARCH are no longer overridden because that would have
no effect on the values that Rules.mk will use.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Anthony PERARD [Fri, 6 Mar 2020 09:14:33 +0000 (10:14 +0100)]
build: extract clean target from Rules.mk
Most of the code executed by Rules.mk isn't necessary for the clean
target, especially not the CFLAGS. This patch makes running make clean
much faster.
The patch extract the clean target into a different Makefile,
Makefile.clean.
Since Makefile.clean, doesn't want to include Config.mk, we need to
define the variables DEPS_INCLUDE and DEPS in a place common to
Rules.mk and Makefile.clean, this is Kbuild.include. DEPS_RM is only
needed in Makefile.clean so can be defined there.
Even so Rules.mk includes Config.mk, it includes Kbuild.include after,
so the effective definition of DEPS_INCLUDE is "xen/" one and the
same one as used by Makefile.clean.
This is inspired by Kbuild, with Makefile.clean partially copied from
Linux v5.4.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Anthony PERARD [Fri, 6 Mar 2020 09:11:23 +0000 (10:11 +0100)]
build: use obj-y += subdir/ instead of subdir-y
This is part of upgrading our build system and import more of Linux's
one.
In Linux, subdir-y in Makefiles is only used to descend into
subdirectory when there are no object to build, Xen doesn't have that
and all subdir have object to be included in the final binary.
To allow the new syntax, the "obj-y" and "subdir-*" calculation in
Rules.mk is changed and partially imported from Linux's Kbuild.
The command used to modify the Makefile was:
sed -i -r 's#^subdir-(.*)#obj-\1/#;' **/Makefile
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <jgrall@amazon.com>
Andrew Cooper [Thu, 5 Mar 2020 17:57:37 +0000 (17:57 +0000)]
x86/dom0: Fix build with clang
find_memory() isn't marked as __init, so if it isn't fully inlined, it ends up
tripping:
Error: size of dom0_build.o:.text is 0x0c1
Fixes: 73b47eea21 "x86/dom0: improve PVH initrd and metadata placement" Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Julien Grall [Tue, 25 Feb 2020 12:32:49 +0000 (12:32 +0000)]
xen/grant-table: Remove outdated warning in gnttab_grow_table()
One of the warning message in gnttab_grow_table() refers to a function
was removed in commit 6425f91c72 "xen/gnttab: Fold grant_table_{create,
set_limits}() into grant_table_init()".
Since the commit, gt->active will be allocated while initializing the
grant table at domain creation. Therefore gt-active will always be
valid.
Rather than replacing the warning by another one, drop the check
completely as we will likely not come back to a semi-initialized world.
Signed-off-by: Julien Grall <jgrall@amazon.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Julien Grall [Mon, 3 Feb 2020 23:57:05 +0000 (23:57 +0000)]
xen/x86: hap: Clean-up and harden hap_enable()
Unlike shadow_enable(), hap_enable() can only be called once during
domain creation and with the mode equal to
PG_external | PG_translate | PG_refcounts.
If it were called twice, then we might have some interesting problems
as the p2m tables would be re-allocated (and therefore all the mappings
would be lost).
Add code to sanity check the mode and that the function is only called
once. Take the opportunity to an if checking that PG_translate is set.
Signed-off-by: Julien Grall <jgrall@amazon.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Roger Pau Monné [Thu, 5 Mar 2020 09:43:46 +0000 (10:43 +0100)]
iommu: fix check for autotranslated hardware domain
The current position of the check_hwdom_reqs is wrong, as there's a
is_iommu_enabled at the top of the function that will prevent getting
to the check on systems without an IOMMU, because the hardware domain
won't have the XEN_DOMCTL_CDF_iommu flag set.
Move the position of the check so it's done before the
is_iommu_enabled one, and thus attempts to create a translated
hardware domain without an IOMMU can be detected.
Fixes: f89f555827a ('remove late (on-demand) construction of IOMMU page tables') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Roger Pau Monné [Thu, 5 Mar 2020 09:43:15 +0000 (10:43 +0100)]
x86/dom0: improve PVH initrd and metadata placement
Don't assume there's going to be enough space at the tail of the
loaded kernel and instead try to find a suitable memory area where the
initrd and metadata can be loaded.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Wei Liu [Thu, 5 Mar 2020 09:42:18 +0000 (10:42 +0100)]
x86/mm: switch to new APIs in arch_init_memory
The function will map and unmap pages on demand.
Since we now map and unmap Xen PTE pages, we would like to track the
lifetime of mappings so that 1) we do not dereference memory through a
variable after it is unmapped, 2) we do not unmap more than once.
Therefore, we introduce the UNMAP_DOMAIN_PAGE macro to nullify the
variable after unmapping, and ignore NULL.
Signed-off-by: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Hongyan Xia <hongyxia@amazon.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
libxl: wait for console path before firing console_available
If the path doesn't become available after LIBXL_INIT_TIMEOUT
seconds, fail the domain creation.
If we skip the bootloader, the TTY path will be set by xenconsoled.
However, there is no guarantee that this will happen by the time we
want to call the console_available callback, so we have to wait.
Signed-off-by: Paweł Marczewski <pawel@invisiblethingslab.com> Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Julien Grall [Mon, 17 Feb 2020 22:20:34 +0000 (22:20 +0000)]
xen/arm: Workaround clang/armclang support for register allocation
Clang 8.0 (see [1]) and by extent some of the version of armclang does
not support register allocation using the syntax rN.
Thankfully, both GCC [2] and clang are able to support the xN syntax for
Arm64. Introduce a new macro ASM_REG() and use in common code for
register allocation.
Jan Beulich [Tue, 3 Mar 2020 15:03:13 +0000 (16:03 +0100)]
MAINTAINERS: Paul to co-maintain vendor-independent IOMMU code
Having just a single maintainer is not helpful anywhere, and can be
avoided here quite easily, seeing that Paul has been doing quite a bit
of IOMMU work lately.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <julien@xen.org> Reviewed-by: Paul Durrant <pdurrant@amazon.com>
Juergen Gross [Tue, 3 Mar 2020 15:02:32 +0000 (16:02 +0100)]
sched: fix error path in cpupool_unassign_cpu_start()
In case moving away all domains from the cpu to be removed is failing
in cpupool_unassign_cpu_start() the error path is missing to release
sched_res_rculock.
The normal exit path is releasing domlist_read_lock instead (this is
currently no problem as the reference to the specific rcu lock is not
used by rcu_read_unlock()).
While at it indent the present error label by one space.
Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com> Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Dario Faggioli <dfaggioli@suse.com>
Jan Beulich [Tue, 3 Mar 2020 15:01:30 +0000 (16:01 +0100)]
credit2: avoid NULL deref in csched2_res_pick() when tracing
The issue here results from one of the downsides of using goto: The
early "goto out" and "goto out_up" in the function very clearly bypass
any possible initialization of min_rqd, yet the tracing code at the end
of the function consumes the value. There's even a comment regarding the
trace record not being accurate in this case.
Juergen Gross [Tue, 11 Feb 2020 09:31:22 +0000 (10:31 +0100)]
xen: do live patching only from main idle loop
One of the main design goals of core scheduling is to avoid actions
which are not directly related to the domain currently running on a
given cpu or core. Live patching is one of those actions which are
allowed taking place on a cpu only when the idle scheduling unit is
active on that cpu.
Unfortunately live patching tries to force the cpus into the idle loop
just by raising the schedule softirq, which will no longer be
guaranteed to work with core scheduling active. Additionally there are
still some places in the hypervisor calling check_for_livepatch_work()
without being in the idle loop.
It is easy to force a cpu into the main idle loop by scheduling a
tasklet on it. So switch live patching to use tasklets for switching to
idle and raising scheduling events. Additionally the calls of
check_for_livepatch_work() outside the main idle loop can be dropped.
As tasklets are only running on idle vcpus and stop_machine_run()
is activating tasklets on all cpus but the one it has been called on
to rendezvous, it is mandatory for stop_machine_run() to be called on
an idle vcpu, too, as otherwise there is no way for scheduling to
activate the idle vcpu for the tasklet on the sibling of the cpu
stop_machine_run() has been called on.
Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Acked-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com> Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tony Luck [Mon, 2 Mar 2020 14:40:50 +0000 (15:40 +0100)]
x86/mce: fix logic and comments around MSR_PPIN_CTL
There are two implemented bits in the PPIN_CTL MSR:
Bit0: LockOut (R/WO)
Set 1 to prevent further writes to MSR_PPIN_CTL.
Bit 1: Enable_PPIN (R/W)
If 1, enables MSR_PPIN to be accessible using RDMSR.
If 0, an attempt to read MSR_PPIN will cause #GP.
So there are four defined values:
0: PPIN is disabled, PPIN_CTL may be updated
1: PPIN is disabled. PPIN_CTL is locked against updates
2: PPIN is enabled. PPIN_CTL may be updated
3: PPIN is enabled. PPIN_CTL is locked against updates
Code would only enable the X86_FEATURE_INTEL_PPIN feature for case "2".
When it should have done so for both case "2" and case "3".
Fix the final test to just check for the enable bit.
Also fix some of the other comments in this function.
Signed-off-by: Tony Luck <tony.luck@intel.com>
[Linux commit ???]
One of the adjusted comments doesn't exist in our code, and I disagree
with the adjustment to the other one and its associate code change: I
don't think there's a point trying to enable PPIN if the locked bit is
set. Hence it's just the main code change that gets pulled in, plus it
gets cloned to the AMD side.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tony Luck [Mon, 2 Mar 2020 14:40:09 +0000 (15:40 +0100)]
x86/mce: add Xeon Icelake to list of CPUs that support PPIN
New CPU model, same MSRs to control and read the inventory number.
Signed-off-by: Tony Luck <tony.luck@intel.com>
[Linux commit dc6b025de95bcd22ff37c4fabb022ec8a027abf1] Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Roger Pau Monné [Mon, 2 Mar 2020 14:37:35 +0000 (15:37 +0100)]
xen/guest: prepare hypervisor ops to use alternative calls
Adapt the hypervisor ops framework so it can be used with the
alternative calls framework. So far no hooks are modified to make use
of the alternatives patching, as they are not in any hot path.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Wei Liu <wl@xen.org> Reviewed-by: Paul Durrant <pdurrant@amazon.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Juergen Gross [Fri, 28 Feb 2020 17:13:48 +0000 (18:13 +0100)]
xen: make sure stop_machine_run() is always called in a tasklet
With core scheduling active it is mandatory for stop_machine_run() to
be called in idle context only (so either during boot or in a tasklet),
as otherwise a scheduling deadlock would occur: stop_machine_run()
does a cpu rendezvous by activating a tasklet on all other cpus. In
case stop_machine_run() was not called in an idle vcpu it would block
scheduling the idle vcpu on its siblings with core scheduling being
active, resulting in a hang.
Put a BUG_ON() into stop_machine_run() to test for being called in an
idle vcpu only and adapt the missing call site (ucode loading) to use a
tasklet for calling stop_machine_run().
Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Mon, 2 Mar 2020 09:49:48 +0000 (10:49 +0100)]
IOMMU/x86: don't bypass softirq processing in arch_iommu_hwdom_init()
Even when a page doesn't need mapping, we should check whether softirq
processing should be invoked. Otherwise with sufficiently much RAM
chances of a to-be-mapped page actually occurring with the loop counter
having the "right" value may become diminishingly small.
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> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Fri, 28 Feb 2020 15:25:43 +0000 (16:25 +0100)]
AMD/IOMMU: correct handling when XT's prereq features are unavailable
We should neither cause IOMMU initialization as a whole to fail in this
case (we should still be able to bring up the system in non-x2APIC or
x2APIC physical mode), nor should the remainder of the function be
skipped (as the main part of it won't get entered a 2nd time) in such an
event. It is merely necessary for the function to indicate to the caller
(iov_supports_xt()) that setup failed as far as x2APIC is concerned.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Roger Pau Monné [Fri, 28 Feb 2020 15:24:26 +0000 (16:24 +0100)]
x86/smp: use a dedicated CPU mask in send_IPI_mask
Some callers of send_IPI_mask pass the scratch cpumask as the mask
parameter of send_IPI_mask, so the scratch cpumask cannot be used by
the function. The following trace has been obtained with a debug patch
and shows one of those callers:
(XEN) scratch CPU mask already in use by arch/x86/mm.c#_get_page_type+0x1f9/0x1abf
(XEN) Xen BUG at smp.c:45
[...]
(XEN) Xen call trace:
(XEN) [<ffff82d0802abb53>] R scratch_cpumask+0xd3/0xf9
(XEN) [<ffff82d0802abc21>] F send_IPI_mask+0x72/0x1ca
(XEN) [<ffff82d0802ac13e>] F flush_area_mask+0x10c/0x16c
(XEN) [<ffff82d080296c56>] F arch/x86/mm.c#_get_page_type+0x3ff/0x1abf
(XEN) [<ffff82d080298324>] F get_page_type+0xe/0x2c
(XEN) [<ffff82d08038624f>] F pv_set_gdt+0xa1/0x2aa
(XEN) [<ffff82d08027dfd6>] F arch_set_info_guest+0x1196/0x16ba
(XEN) [<ffff82d080207a55>] F default_initialise_vcpu+0xc7/0xd4
(XEN) [<ffff82d08027e55b>] F arch_initialise_vcpu+0x61/0xcd
(XEN) [<ffff82d080207e78>] F do_vcpu_op+0x219/0x690
(XEN) [<ffff82d08038be16>] F pv_hypercall+0x2f6/0x593
(XEN) [<ffff82d080396432>] F lstar_enter+0x112/0x120
_get_page_type will use the scratch cpumask to call flush_tlb_mask,
which in turn calls send_IPI_mask.
Fix this by using a dedicated per CPU cpumask in send_IPI_mask.
Fixes: 5500d265a2a8 ('x86/smp: use APIC ALLBUT destination shorthand when possible') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
iommu/arm: Don't allow the same micro-TLB to be shared between domains
For the IPMMU-VMSA we need to prevent the use cases where devices
which use the same micro-TLB are assigned to different Xen domains
(micro-TLB cannot be shared between multiple Xen domains, since it
points to the context bank to use for the page walk).
As each Xen domain uses individual context bank pointed by context_id,
we can potentially recognize that use case by comparing current and new
context_id for the already enabled micro-TLB and prevent different
context bank from being set.
Andrew Cooper [Thu, 2 Jan 2020 19:06:54 +0000 (19:06 +0000)]
tools/libxl: Simplify callback handling in libxl-save-helper
The {save,restore}_callback helpers can have their scope reduced vastly, and
helper_setcallbacks_{save,restore}() doesn't need to use a ternary operator to
write 0 (meaning NULL) into an already zeroed structure.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Andrew Cooper [Fri, 21 Feb 2020 15:23:31 +0000 (15:23 +0000)]
x86/cpuid: Introduce and use default CPUID policies
For now, the default and max policies remain identical, but this will change
in the future.
Introduce calculate_{pv,hvm}_def_policy(). As *_def derives from *_max, quite
a bit of the derivation logic can be avoided the second time around - this
will cope with simple feature differences for now.
Update XEN_SYSCTL_get_cpu_* and init_domain_cpuid_policy() to use the default
policies as appropriate.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Wed, 26 Feb 2020 12:26:14 +0000 (12:26 +0000)]
x86/msr: Compile out unused logic/objects
Arrange to compile out the PV or HVM logic and objects as applicable. This
involves a bit of complexity in init_domain_msr_policy() as is_pv_domain()
can't be evaulated at compile time.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Tue, 25 Feb 2020 12:30:49 +0000 (12:30 +0000)]
x86/gen-cpuid: Create max and default variations of INIT_*_FEATURES
For now, write the same content for both. Update the users of the
initialisers to use the new name, and extend xen-cpuid to dump both default
and max featuresets.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Tue, 25 Feb 2020 12:59:35 +0000 (12:59 +0000)]
x86/gen-cpuid: Rework internal logic to ease future changes
Better split the logic between parse/calculate/write. Collect the feature
comment by their comment character(s), and perform the accumulation operations
in crunch_numbers().
Avoid rendering the featuresets to C uint32_t's in crunch_numbers(), and
instead do this in write_results(). Update format_uint32s() to call
featureset_to_uint32s() internally.
No functional change - the generated cpuid-autogen.h is identical.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Drop XC_FEATUREMASK_DEEP_FEATURES. It isn't used by any callers, and unlike
the other static masks, won't be of interest to anyone without other pieces of
cpuid-autogen.h
In xc_get_static_cpu_featuremask(), use a 2d array instead of individually
named variables, and drop the switch statement completely.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Anthony PERARD [Thu, 27 Feb 2020 14:47:23 +0000 (15:47 +0100)]
build: fix section-renaming of libfdt and libelf
In common/libelf/Makefile, when SECTIONS gets defined
SPECIAL_DATA_SECTIONS doesn't exist, so only "text data" sections are
been renamed. This was different before 48115d14743e ("Move more
kernel decompression bits to .init.* sections"). By introducing the
same renaming mechanism the to libfdt (9ba1f198f61e ["xen/libfdt: Put
all libfdt in init"]), the issue was extended to there as well.
Move SPECIAL_DATA_SECTIONS in Rules.mk before including "Makefile" to
fix this.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Anthony PERARD [Thu, 27 Feb 2020 14:46:14 +0000 (15:46 +0100)]
build: allow to test clang .include without asm symlink
The clang test for "asm()-s support .include." needs to be modified
because the symbolic link asm -> asm-x86 may not exist when the test
is runned. Since it's an x86 test, we don't need the link.
This will be an issue with the following patch "xen/build: have the
root Makefile generates the CFLAGS".
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Thu, 27 Feb 2020 14:44:41 +0000 (15:44 +0100)]
libxl/PCI: make "rdm=" parsing comply with documentation
Documentation says "<RDM_RESERVATION_STRING> is a comma separated list
of <KEY=VALUE> settings, from the following list". There's no mention
of a specific order, yet so far the parsing logic did accept only
strategy, then policy (and neither of the two omitted). Make "state"
move
- back to STATE_TYPE when finding a comma after having parsed the
<VALUE> part of a setting,
- to STATE_TERMINAL otherwise.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Wei Liu <wl@xen.org>
Reserved device memory region processing as well as E820 table creation
happen earlier than the adding of (PCI) devices, yet they consume the
policy (e.g. to decide whether to add entries to the E820 table). But so
far it was only at the stage of PCI device addition that the final
policy was established (i.e. if not explicitly specified by the guest
config file).
Note that I couldn't find the domain ID to be available in
libxl__domain_device_construct_rdm(), but observing that
libxl__device_pci_setdefault() also doesn't use it, for the time being
DOMID_INVALID gets passed. An obvious alternative would be to drop the
unused parameter/argument, yet at that time the question would be
whether to also drop other unused ones.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Wei Liu <wl@xen.org>
Paul Durrant [Wed, 26 Feb 2020 13:12:13 +0000 (13:12 +0000)]
libxl: add initializers for libxl__domid_history
This patch fixes Coverity issue CID 1459006 (Insecure data handling
(INTEGER_OVERFLOW)).
The problem is that the error paths for libxl__mark_domid_recent() and
libxl__is_domid_recent() check the 'f' field in struct libxl__domid_history
when it may not have been initialized.
Signed-off-by: Paul Durrant <pdurrant@amazon.com> Acked-by: Wei Liu <wl@xen.org>
Anthony PERARD [Wed, 26 Feb 2020 16:41:37 +0000 (17:41 +0100)]
build: remove confusing comment on the %.s:%.S rule
That comment was introduce by 3943db776371 ("[XEN] Can be built
-std=gnu99 (except for .S files).") to explain why CFLAGS was removed
from the command line. The comment is already written where the
-std=gnu flags gets remove from AFLAGS, no need to repeat it.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Wei Liu <wl@xen.org>
Anthony PERARD [Wed, 26 Feb 2020 16:41:02 +0000 (17:41 +0100)]
Makefile: fix install-tests
The top-level makefile make uses of internal implementation detail of
the xen build system. Avoid that by creating a new target
"install-tests" in xen/Makefile, and by fixing the top-level Makefile
to not call xen/Rules.mk anymore.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
projects / people / dariof / xen.git / log