Commit Graph
40932 Commits
Author SHA1 Message Date
Jeff Kubascik ee97008433 sched/core: fix bug when moving a domain between cpupools
For each UNIT, sched_set_affinity is called before unit->priv is updated
to the new cpupool private UNIT data structure. The issue is
sched_set_affinity will call the adjust_affinity method of the cpupool.
If defined, the new cpupool may use unit->priv (e.g. credit), which at
this point still references the old cpupool private UNIT data structure.

This change fixes the bug by moving the switch of unit->priv earler in
the function.

Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>
2020-04-17 09:27:21 +02:00
Wei Liu a48e1323f9 x86_64/mm: map and unmap page tables in destroy_m2p_mapping
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-16 11:05:58 +02:00
Wei Liu 15b7bec578 x86_64/mm: map and unmap page tables in share_hotadd_m2p_table
Fetch lYe by mapping and unmapping lXe instead of using the direct map,
which is now done via the lYe_from_lXe() helpers.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-16 11:05:28 +02:00
Wei Liu f362f5b5d0 x86_64/mm: map and unmap page tables in m2p_mapped
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-16 11:04:51 +02:00
Wei Liu 6c8afe5aad x86/shim: map and unmap page tables in replace_va_mapping
Also, introduce lYe_from_lXe() macros which do not rely on the direct
map when walking page tables. Unfortunately, they cannot be inline
functions due to the header dependency on domain_page.h, so keep them as
macros just like map_lYt_from_lXe().

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-16 11:01:46 +02:00
Harsha Shamsundara Havanur a8e0c228c7 hvmloader: enable MMIO and I/O decode, after all resource allocation
It was observed that PCI MMIO and/or IO BARs were programmed with
memory and I/O decodes (bits 0 and 1 of PCI COMMAND register) enabled,
during PCI setup phase. This resulted in incorrect memory mapping as
soon as the lower half of the 64 bit bar is programmed.
This displaced any RAM mappings under 4G. After the
upper half is programmed PCI memory mapping is restored to its
intended high mem location, but the RAM displaced is not restored.
The OS then continues to boot and function until it tries to access
the displaced RAM at which point it suffers a page fault and crashes.

This patch address the issue by deferring enablement of memory and
I/O decode in command register until all the resources, like interrupts
I/O and/or MMIO BARs for all the PCI device functions are programmed,
in the descending order of memory requested.

Signed-off-by: Harsha Shamsundara Havanur <havanur@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-16 10:58:46 +02:00
Roger Pau Monné 6b93f3ca85 x86/altp2m: add missing break
Add a missing break in the HVMOP_altp2m_set_visibility case, or else
code flow will continue into the default case and trigger the assert.

Fixes: 3fd3e9303e ('x86/altp2m: hypercall to set altp2m view visibility')
Coverity-ID: 1461759
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
2020-04-16 10:55:42 +02:00
Andrew Cooper 615bfe42c6 x86/boot: Fix early exception handling with CONFIG_PERF_COUNTERS
The PERFC_INCR() macro uses current->processor, but current is not valid
during early boot.  This causes the following crash to occur if
e.g. rdmsr_safe() has to recover from a #GP fault.

  (XEN) Early fatal page fault at e008:ffff82d0803b1a39 (cr2=0000000000000004, ec=0000)
  (XEN) ----[ Xen-4.14-unstable  x86_64  debug=y   Not tainted ]----
  (XEN) CPU:    0
  (XEN) RIP:    e008:[<ffff82d0803b1a39>] x86_64/entry.S#handle_exception_saved+0x64/0xb8
  ...
  (XEN) Xen call trace:
  (XEN)    [<ffff82d0803b1a39>] R x86_64/entry.S#handle_exception_saved+0x64/0xb8
  (XEN)    [<ffff82d0806394fe>] F __start_xen+0x2cd/0x2980
  (XEN)    [<ffff82d0802000ec>] F __high_start+0x4c/0x4e

Furthermore, the PERFC_INCR() macro is wildly inefficient.  There has been a
single caller for many releases now, so inline it and delete the macro
completely.

There is no need to reference current at all.  What is actually needed is the
per_cpu_offset which can be obtained directly from the top-of-stack block.
This simplifies the counter handling to 3 instructions and no spilling to the
stack at all.

The same breakage from above is now handled properly:

  (XEN) traps.c:1591: GPF (0000): ffff82d0806394fe [__start_xen+0x2cd/0x2980] -> ffff82d0803b3bfb

Reported-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Julien Grall <jgrall@amazon.com>
2020-04-16 09:48:38 +01:00
Andrew Cooper fcd06227f8 x86/svm: Don't use vmcb->tlb_control as if it is a boolean
svm_asid_handle_vmrun() treats tlb_control as if it were boolean, but this has
been superseded by new additions to the SVM spec.

Introduce an enum containing all legal values, and update
svm_asid_handle_vmrun() to use appropriate constants.

While adjusting this, take the opportunity to fix up two coding style issues,
and trim the include list.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-14 15:56:52 +01:00
Jan Beulich d6f22d5d9e gnttab: fix GNTTABOP_copy continuation handling
The XSA-226 fix was flawed - the backwards transformation on rc was done
too early, causing a continuation to not get invoked when the need for
preemption was determined at the very first iteration of the request.
This in particular means that all of the status fields of the individual
operations would be left untouched, i.e. set to whatever the caller may
or may not have initialized them to.

This is part of XSA-318.

Reported-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Tested-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
2020-04-14 14:42:32 +02:00
Ross Lagerwall da0c66c8f4 xen/gnttab: Fix error path in map_grant_ref()
Part of XSA-295 (c/s 863e74eb2c) inadvertently re-positioned the brackets,
changing the logic.  If the _set_status() call fails, the grant_map hypercall
would fail with a status of 1 (rc != GNTST_okay) instead of the expected
negative GNTST_* error.

This error path can be taken due to bad guest state, and causes net/blk-back
in Linux to crash.

This is XSA-316.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2020-04-14 14:41:02 +02:00
Julien Grall 6890a04072 xen/rwlock: Add missing memory barrier in the unlock path of rwlock
The rwlock unlock paths are using atomic_sub() to release the lock.
However the implementation of atomic_sub() rightfully doesn't contain a
memory barrier. On Arm, this means a processor is allowed to re-order
the memory access with the preceeding access.

In other words, the unlock may be seen by another processor before all
the memory accesses within the "critical" section.

The rwlock paths already contains barrier indirectly, but they are not
very useful without the counterpart in the unlock paths.

The memory barriers are not necessary on x86 because loads/stores are
not re-ordered with lock instructions.

So add arch_lock_release_barrier() in the unlock paths that will only
add memory barrier on Arm.

Take the opportunity to document each lock paths explaining why a
barrier is not necessary.

This is XSA-314.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-04-14 14:37:11 +02:00
Jan Beulich 50ef9a3cb2 xenoprof: limit consumption of shared buffer data
Since a shared buffer can be written to by the guest, we may only read
the head and tail pointers from there (all other fields should only ever
be written to). Furthermore, for any particular operation the two values
must be read exactly once, with both checks and consumption happening
with the thus read values. (The backtrace related xenoprof_buf_space()
use in xenoprof_log_event() is an exception: The values used there get
re-checked by every subsequent xenoprof_add_sample().)

Since that code needed touching, also fix the double increment of the
lost samples count in case the backtrace related xenoprof_add_sample()
invocation in xenoprof_log_event() fails.

Where code is being touched anyway, add const as appropriate, but take
the opportunity to entirely drop the now unused domain parameter of
xenoprof_buf_space().

This is part of XSA-313.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
2020-04-14 14:33:19 +02:00
Jan Beulich 0763a7ebfc xenoprof: clear buffer intended to be shared with guests
alloc_xenheap_pages() making use of MEMF_no_scrub is fine for Xen
internally used allocations, but buffers allocated to be shared with
(unpriviliged) guests need to be zapped of their prior content.

This is part of XSA-313.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
2020-04-14 14:32:33 +02:00
Jan Beulich 0dbc112e72 x86/EFI: also fill boot_tsc_stamp on the xen.efi boot path
Commit e3a379c35e ("x86/time: always count s_time from Xen boot")
introducing this missed adjusting this path as well.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>
2020-04-14 11:42:11 +02:00
Alexandru Isaila 3fd3e9303e x86/altp2m: hypercall to set altp2m view visibility
At this moment a guest can call vmfunc to change the altp2m view. This
should be limited in order to avoid any unwanted view switch.

The new xc_altp2m_set_visibility() solves this by making views invisible
to vmfunc.
This is done by having a separate arch.altp2m_working_eptp that is
populated and made invalid in the same places as altp2m_eptp. This is
written to EPTP_LIST_ADDR.
The views are made in/visible by marking them with INVALID_MFN or
copying them back from altp2m_eptp.
To have consistency the visibility also applies to
p2m_switch_domain_altp2m_by_id().

The usage of this hypercall is aimed at dom0 having a logic with a number of views
created and at some time there is a need to be sure that only some of the views
can be switched, saving the rest and making them visible when the time
is right.

Note: If altp2m mode is set to mixed the guest is able to change the view
visibility and then call vmfunc.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Acked-by: Wei Liu <wl@xen.org>
2020-04-14 11:38:48 +02:00
Andrew Cooper 7372466b21 x86/mem_sharing: Fix build with !CONFIG_XSM
A build fails with:

  mem_sharing.c: In function ‘copy_special_pages’:
  mem_sharing.c:1649:9: error: ‘HVM_PARAM_STORE_PFN’ undeclared (first use in this function)
           HVM_PARAM_STORE_PFN,
           ^~~~~~~~~~~~~~~~~~~
  ...

This is because xsm/xsm.h includes xsm/dummy.h for the !CONFIG_XSM case, which
brings public/hvm/params.h in.

Fixes: 41548c5472 "mem_sharing: VM forking"
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
2020-04-10 15:20:10 +01:00
Julien Grall 13dcb32b6b xen/x86: ioapic: Simplify ioapic_init()
Since commit 9facd54a45 "x86/ioapic: Add register level checks to detect
bogus io-apic entries", Xen is able to cope with IO APICs not mapped in
the fixmap.

Therefore the whole logic to allocate a fake page for some IO APICs is
unnecessary.

With the logic removed, the code can be simplified a lot as we don't
need to go through all the IO APIC if SMP has not been detected or a
bogus zero IO-APIC address has been detected.

To avoid another level of tabulation, the simplification is now moved in
its own function.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-10 12:23:48 +01:00
Julien Grall 996ff16416 xen/x86: ioapic: Rename init_ioapic_mappings() to ioapic_init()
The function init_ioapic_mappings() is doing more than initialization
mappings. It is also initialization the number of IRQs/GSIs supported.

So rename the function to ioapic_init().

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-04-10 12:23:48 +01:00
Julien Grall 8c4aed6ee1 xen/x86: ioapic: Use true/false in bad_ioapic_register()
bad_ioapic_register() is returning a bool, so we should switch to
true/false.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Wei Liu <wl@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-04-10 12:23:48 +01:00
Dmitry Isaykin 4fb402b486 tools/xl: Remove the filelock when building VM if autoballooning is off
The presence of this filelock does not allow building several VMs at the same
time. This filelock was added to prevent other xl instances from using memory
freed for the currently building VM in autoballoon mode.

Signed-off-by: Dmitry Isaykin <isaikin-dmitry@yandex.ru>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-04-09 15:55:50 +01:00
Andrew Panyakin 69f741347d libxc/migration: Abort migration on precopy policy request
libxc defines XGS_POLICY_ABORT for precopy policy to signal that migration
should be aborted (eg. if the estimated pause time is too huge for the
instance). Default simple precopy policy never returns that, but it could be
overriden with a custom one.

Signed-off-by: Andrew Panyakin <apanyaki@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
[wei: fix coding style issue]
2020-04-09 15:43:18 +01:00
Jan Beulich 9be0b2747b x86/PoD: correct ordering of checks in p2m_pod_zero_check()
Commit 0537d246f8 ("mm: add 'is_special_page' inline function...")
moved the is_special_page() checks first in its respective changes to
PoD code. While this is fine for p2m_pod_zero_check_superpage(), the
validity of the MFN is inferred in both cases from the p2m_is_ram()
check, which therefore also needs to come first in this 2nd instance.

Take the opportunity and address latent UB here as well - transform
the MFN into struct page_info * only after having established that
this is a valid page.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-08 13:12:28 +02:00
Jan Beulich d3ff3e48db x86/HVM: __hvm_copy()'s size parameter is an unsigned quantity
There are no negative sizes. Make the function's parameter as well as
that of its derivates "unsigned int". Similarly make its local "count"
variable "unsigned int", and drop "todo" altogether. Don't use min_t()
anymore to calculate "count". Restrict its scope as well as that of
other local variables of the function.

While at it I've also noticed that {copy_{from,to},clear}_user_hvm()
have been returning "unsigned long" for no apparent reason, as their
respective "size" parameters have already been "unsigned int". Adjust
this as well as a slightly wrong comment there at the same time.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <pdurrant@amzn.com>
2020-04-08 13:11:24 +02:00
Tamas K Lengyel c8dbdb32ce mem_sharing: reset a fork
Implement hypercall that allows a fork to shed all memory that got allocated
for it during its execution and re-load its vCPU context from the parent VM.
This allows the forked VM to reset into the same state the parent VM is in a
faster way then creating a new fork would be. Measurements show about a 2x
speedup during normal fuzzing operations. Performance may vary depending how
much memory got allocated for the forked VM. If it has been completely
deduplicated from the parent VM then creating a new fork would likely be more
performant.

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
2020-04-08 13:03:50 +02:00
Tamas K Lengyel 41548c5472 mem_sharing: VM forking
VM forking is the process of creating a domain with an empty memory space and a
parent domain specified from which to populate the memory when necessary. For
the new domain to be functional the VM state is copied over as part of the fork
operation (HVM params, hap allocation, etc).

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
2020-04-08 12:59:58 +02:00
Juergen Gross e013e85143 config: use mini-os master for unstable
We haven't used mini-os master for about 2 years now due to a stubdom
test failing [1]. Booting a guest with mini-os master used for building
stubdom didn't reveal any problem, so use master for unstable in order
to let OSStest find any problems not showing up in the local test.

[1]: https://lists.xen.org/archives/html/minios-devel/2018-04/msg00015.html

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-04-07 21:05:54 +01:00
Andrew Cooper 990b6e38d9 x86/ucode: Simplify the ops->collect_cpu_info() API
All callers pass &this_cpu(cpu_sig) for the cpu_sig parameter, and all
implementations unconditionally return 0.  Simplify it to be void.

Drop the long-stale comment on the AMD side, whose counterpart in
start_update() used to be "collect_cpu_info() doesn't fail so we're fine".

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-03 20:11:46 +01:00
Andrew Cooper 6b8836aa65 x86/ucode: Drop ops->free_patch()
With the newly cleaned up vendor logic, each struct microcode_patch is a
trivial object in memory with no dependent allocations.

This is unlikely to change moving forwards, and function pointers are
expensive in the days of retpoline.  Move the responsibility to xfree() back
to common code.  If the need does arise in the future, we can consider
reintroducing the hook.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-03 20:11:46 +01:00
Andrew Cooper 4853f03dee x86/ucode: Don't try to cope with NULL pointers in apply_microcode()
No paths to apply_microcode() pass a NULL pointer, and other hooks don't
tolerate one in the first place.  We can expect the core logic not to pass us
junk, so drop the checks.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-03 20:11:46 +01:00
Andrew Cooper 62d7b32df7 x86/ucode: Drop ops->match_cpu()
It turns out there are no callers of the hook().  The only callers are the
local, which can easily be rearranged to use the appropriate internal helper.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-03 20:11:46 +01:00
Andrew Cooper c81fb4e4a3 x86/ucode/intel: Remove one CPUID from collect_cpu_info()
The CPUID instruction is expensive.  No point executing it twice when once
will do fine.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-03 20:11:46 +01:00
Andrew Cooper 37053578e8 docs: Render .md files using pandoc
This fixes the fact that qemu-deprivilege.md, non-cooperative-migration.md and
xenstore-migration.md don't currently get rendered at all, and are therefore
missing from xenbits.xen.org/docs

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Backport: 4.12
2020-04-03 20:11:46 +01:00
Juergen Gross bb2a34fd74 tools/xenstore: fix a use after free problem in xenstored
Commit 562a1c0f7e ("tools/xenstore: dont unlink connection object
twice") introduced a potential use after free problem in
domain_cleanup(): after calling talloc_unlink() for domain->conn
domain->conn is set to NULL. The problem is that domain is registered
as talloc child of domain->conn, so it might be freed by the
talloc_unlink() call.

With Xenstore being single threaded there are normally no concurrent
memory allocations running and freeing a virtual memory area normally
doesn't result in that area no longer being accessible. A problem
could occur only in case either a signal received results in some
memory allocation done in the signal handler (SIGHUP is a primary
candidate leading to reopening the log file), or in case the talloc
framework would do some internal memory allocation during freeing of
the memory (which would lead to clobbering of the freed domain
structure).

Fixes: 562a1c0f7e ("tools/xenstore: dont unlink connection object twice")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2020-04-03 17:57:51 +01:00
Jan Beulich 0f0f4b7b1f x86/p2m: make p2m_remove_page()'s parameters type-safe
Also add a couple of blank lines.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-03 17:19:11 +02:00
Jan Beulich d917077b8d x86/p2m: use available local variable in guest_physmap_add_entry()
The domain is being passed in - no need to obtain it from p2m->domain.
Also drop a pointless cast and simplify expressions while touching this
code anyway.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-03 17:17:29 +02:00
Jan Beulich 9ff09aefc4 x86/cpuidle: correct Cannon Lake residency MSRs
As per SDM rev 071 Cannon Lake has
- no CC3 residency MSR at 3FC,
- a CC1 residency MSR ar 660 (like various Atoms),
- a useless (always zero) CC3 residency MSR at 662.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-03 17:15:58 +02:00
Simran Singhal 009360acc9 x86: use macro DIV_ROUND_UP
Use the DIV_ROUND_UP macro to replace open-coded divisor calculation
(((n) + (d) - 1) / (d)) to improve readability.

Signed-off-by: Simran Singhal <singhalsimran0@gmail.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-03 10:58:18 +02:00
Jan Beulich 41cebdd1a6 x86/p2m: drop pointless nested variable from guest_physmap_add_entry()
There's an outer scope rc already, and its use for the mem-sharing logic
does not conflict with its use elsewhere in the function.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-03 10:57:41 +02:00
Jan Beulich c65ea16dbc x86/p2m: don't assert that the passed in MFN matches for a remove
guest_physmap_remove_page() gets handed an MFN from the outside, yet
takes the necessary lock to prevent further changes to the GFN <-> MFN
mapping itself. While some callers, in particular guest_remove_page()
(by way of having called get_gfn_query()), hold the GFN lock already,
various others (most notably perhaps the 2nd instance in
xenmem_add_to_physmap_one()) don't. While it also is an option to fix
all the callers, deal with the issue in p2m_remove_page() instead:
Replace the ASSERT() by a conditional and split the loop into two, such
that all checking gets done before any modification would occur.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-03 10:56:55 +02:00
Jan Beulich a6b051a87a x86/p2m: don't ignore p2m_remove_page()'s return value
It's not very nice to return from guest_physmap_add_entry() after
perhaps already having made some changes to the P2M, but this is pre-
existing practice in the function, and imo better than ignoring errors.

Take the liberty and replace an mfn_add() instance with a local variable
already holding the result (as proven by the check immediately ahead).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-03 10:56:10 +02:00
Jan Beulich 5b6a3c67cd x86emul: inherit HOSTCC when building 32-bit harness on 64-bit host
We're deliberately bringing XEN_COMPILE_ARCH and XEN_TARGET_ARCH out of
sync in this case, and hence HOSTCC won't get set from CC. Therefore
without this addition HOSTCC would not match a possible make command
line override of CC, but default to "gcc", likely causing the build to
fail for test_x86_emulator.c on systems with too old a gcc.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-03 10:55:12 +02:00
Jan Beulich d5997399b7 x86emul: suppress "not built" warning for test harness'es run targets
The run* targets can be used to test whatever the tool chain is capable
of building, as long as at least the main harness source file builds.
Don't probe the tools chain, in particular to avoid issuing the warning,
in this case. While looking into this I also noticed the wording of the
respective comment isn't quite right, which therefore gets altered at
the same time.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-04-03 10:48:39 +02:00
Dario Faggioli dae7b62e97 credit2: fix credit reset happening too few times
There is a bug in commit 5e4b419966 ("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>
Acked-by: George Dunlap <george.dunlap@citrix.com>
2020-04-03 10:46:53 +02:00
Dario Faggioli 36f3662f27 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:

    (XEN) Runqueue 0:
    (XEN)   ncpus              = 28
    (XEN)   cpus               = 0-27
    (XEN)   max_weight         = 256
    (XEN)   pick_bias          = 22
    (XEN)   instload           = 1
    (XEN)   aveload            = 293391 (~111%)
    (XEN)   idlers: 00,00000000,00000000,00000000,00000000,00000000,0fffffff
    (XEN)   tickled: 00,00000000,00000000,00000000,00000000,00000000,00000000
    (XEN)   fully idle cores: 00,00000000,00000000,00000000,00000000,00000000,0fffffff
    [...]
    (XEN) Runqueue 0:
    (XEN) CPU[00] runq=0, sibling=00,..., core=00,...
    (XEN) CPU[01] runq=0, sibling=00,..., core=00,...
    [...]
    (XEN) CPU[26] runq=0, sibling=00,..., core=00,...
    (XEN) CPU[27] runq=0, sibling=00,..., core=00,...
    (XEN) RUNQ:
    (XEN)     0: [0.1] flags=0 cpu=5 credit=-1254238270 [w=256] load=262144 (~100%)

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>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2020-04-03 10:45:43 +02:00
Andrew Cooper 4de936a38a x86/ucode/amd: Rework parsing logic in cpu_request_microcode()
cpu_request_microcode() is still a confusing mess to follow, with sub
functions responsible for maintaining offset.  Rewrite it so all container
structure handling is in this one function.

Rewrite struct mpbhdr as struct container_equiv_table to aid parsing.  Drop
container_fast_forward() entirely, and shrink scan_equiv_cpu_table() to just
its searching/caching logic.

container_fast_forward() gets logically folded into the microcode blob
scanning loop, except that a skip path is inserted, which is conditional on
whether scan_equiv_cpu_table() thinks there is appropriate microcode to find.

With this change, we now scan to the end of all provided microcode containers,
and no longer give up at the first applicable one.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-01 14:00:12 +01:00
Andrew Cooper 6f56491911 x86/ucode/amd: Fold structures together
With all the necessary cleanup now in place, fold struct microcode_header_amd
into struct microcode_patch and drop the struct microcode_amd temporary
ifdef-ary.

This removes the memory allocation of struct microcode_amd which is a single
pointer to a separately allocated object, and therefore a waste.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-01 14:00:12 +01:00
Andrew Cooper f4c663b88b x86/ucode/amd: Remove gratuitous memory allocations from cpu_request_microcode()
Just as on the Intel side, there is no point having
get_ucode_from_buffer_amd() make $N memory allocations and free $N-1 of them.

Delete get_ucode_from_buffer_amd() and rewrite the loop in
cpu_request_microcode() to have 'saved' point into 'buf' until we finally
decide to duplicate that blob and return it to our caller.

Introduce a new struct container_microcode to simplify interpreting the
container format.  Doubly indent the logic to substantially reduce the churn
in a later change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-04-01 14:00:12 +01:00
Andrew Cooper 38f6f11d10 x86/ucode/amd: Rename bufsize to size in cpu_request_microcode()
To simplify future cleanup, rename this variable.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-04-01 14:00:12 +01:00
Andrew Cooper e6ac4d1b45 x86/ucode/amd: Alter API for microcode_fits()
Although it is logically a step in the wrong direction overall, it simplifies
the rearranging of cpu_request_microcode() substantially for microcode_fits()
to take struct microcode_header_amd directly, and not require an intermediate
struct microcode_amd pointing at it.

Make this change (taking time to rename 'mc_amd' to its eventual 'patch' to
reduce the churn in the series), and a later cleanup will make it uniformly
take a struct microcode_patch.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-04-01 14:00:12 +01:00