The use of any kind of pointers in the public interface is wrong,
including dimensioning arrays based on the size of pointers. The least
bad option of addressing the issue looks to be to pin down the number
that the (64-bit) hypervisor has used anyway (even when passing
information to compat but privileged guests). There aren't actual
instantiations of the structure apart from ones allocated dynamically
out of struct mc_info's mi_data[], which is entirely controlled by the
hypervisor.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
While HYPERVISOR_mca is a privileged operation, we still shouldn't leak
stack contents (the tail of every array entry's mc_msrvalues[] of
XEN_MC_physcpuinfo output). Simply use a zeroing allocation here.
Take the occasion and also restrict the involved local variable's scope.
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
cpu_smpboot_free() removes the stubs for the cpu going offline, but it
isn't clearing the related percpu variables. This will result in
crashes when a stub page is released due to all related cpus gone
offline and one of those cpus going online later.
Fix that by clearing stubs.addr and stubs.mfn in order to allocate a
new stub page when needed, irrespective of whether the CPU gets parked
or removed.
Fixes: 2e6c8f182c ("x86: distinguish CPU offlining from CPU removal")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Tao Xu <tao3.xu@intel.com>
There is no need to load a non-flat %es to zero the BSS. Use sym_esi()
instead, which is easier to follow, faster (avoids two segment loads) and
doesn't require use of the stack.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
c/s ec92fcd1d0, which caused the trampoline GDT Access bits to be set,
removed the final writes which occurred between enabling paging and switching
to the high mappings. There don't plausibly need to be any memory writes in
few instructions is takes to perform this transition.
As a consequence, we can remove the RWX mapping of the trampoline. It is RX
via its identity mapping below 1M, and RW via the directmap.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
There is no point performing the masking calculations if we are going to
throw the result away.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
MAINTAINERS entries tagged with "L:" should have a pure mail address
as the second word. Fix a malformed entry. Otherwise add_maintainers.pl
will produce an empty "Cc:" line.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
console_force_unlock() might result in subsequent ASSERT() triggering
when CONFIG_DEBUG_LOCKS was active. Avoid that by calling
spin_debug_disable() in console_force_unlock() and make the spinlock
debug assertions trigger only if spin_debug was active.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Travis reports: https://travis-ci.org/andyhhp/xen/jobs/633751811
mem_sharing.c:361:13: error: 'rmap_has_entries' defined but not used [-Werror=unused-function]
static bool rmap_has_entries(const struct page_info *page)
^
cc1: all warnings being treated as errors
This happens in a release build (disables MEM_SHARING_AUDIT) when
CONFIG_MEM_SHARING is enabled.
Expand both trivial helpers into their single callsite.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tamas K Lengyel <tamas@tklengyel.com>
This also allows to run `make src-tarball` without first having to run
`./configure`.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
All structures are already naturally aligned. Linux added those
attributes out of paranoia.
In Xen we've had instance we had to drop pointless __packed to placate
gcc 9 (see ca9310b24e "x86/IO-APIC: fix build with gcc9"), it is better
to drop those attributes in hyperv-tlfs.h as well.
Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Wei Liu <liuwe@microsoft.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
I'm told that GENMASK_ULL shouldn't be used outside of Arm code in its
current form.
Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Wei Liu <liuwe@microsoft.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Due to the unconditional updating of dom->highmem_end in
libxl__domain_device_construct_rdm() I've observed on a 2Gb HVM guest
with a passed through device (without overly large BARs, and with no RDM
ranges at all)
(d2) RAM in high memory; setting high_mem resource base to 100000000
...
(d2) E820 table:
(d2) [00]: 00000000:00000000 - 00000000:000a0000: RAM
(d2) HOLE: 00000000:000a0000 - 00000000:000d0000
(d2) [01]: 00000000:000d0000 - 00000000:00100000: RESERVED
(d2) [02]: 00000000:00100000 - 00000000:7f800000: RAM
(d2) HOLE: 00000000:7f800000 - 00000000:fc000000
(d2) [03]: 00000000:fc000000 - 00000001:00000000: RESERVED
(d2) [04]: 00000001:00000000 - 00000001:00000000: RAM
both of which aren't really appropriate in this case. Arrange for this
to not happen.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
is_{hvm,pv}_*() can be expensive now, so where possible evaluate cheaper
conditions first.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
After dad74b0f9e ("i386: fix handling of Xen entries in final L2 page
table") and the removal of 32-bit support the function doesn't modify
state anymore, and hence its name has been misleading. Change its name,
constify parameters and a local variable, and make it return bool.
Also drop the call to it from mod_l3_entry(): The function explicitly
disallows 32-bit domains to modify slot 3. This way we also won't
re-check slot 3 when a slot other than slot 3 changes. Doing so has
needlessly disallowed making some L2 table recursively link back to an
L2 used in some L3's 3rd slot, as we check for the type ref count to be
1. (Note that allowing dynamic changes of L3 entries in the way we do is
bogus anyway, as that's not how L3s behave in the native and EPT cases:
They get re-evaluated only upon CR3 reloads. NPT is different in this
regard.)
As a result of this we no longer need to play games to get at the start
of the L3 table.
Additionally move the single remaining call site, allowing to drop one
is_pv_32bit_domain() invocation and a _PAGE_PRESENT check (in the
function itself) as well as to exit the loop early (remaining entries
have all been set to empty just ahead of this loop).
Further move a BUG_ON() such that in the common case its condition
wouldn't need evaluating.
Finally, since we're at it, move init_xen_pae_l2_slots() next to the
renamed function, as they really belong together (in fact
init_xen_pae_l2_slots() was [indirectly] broken out of this function).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
mod_l1_entry()'s need to do so went away with commit 2d0557c5cb ("x86:
Fold page_info lock into type_info"), and the other three never had such
a need, at least going back as far as 3.2.0. Replace the uses by
l<N>e_read_atomic().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
When resuming from S3 and smt=0 or maxcpus= are specified we must not
do anything in cpu_schedule_callback(). This is not true today for
taking down a cpu during resume.
If anything goes wrong during resume all the scheduler related error
handling is in cpupool.c, so we can just bail out early from
cpu_schedule_callback() when suspending or resuming.
This fixes commit 0763cd2687 ("xen/sched: don't disable
scheduler on cpus during suspend").
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
We will need to have a variable named pl*e when we rewrite
virt_to_xen_l*e. Change pl*e to l*t to reflect better its purpose.
This will make reviewing later patch easier.
No functional change.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The pl2e and pl1e variables are heavily (ab)used in that function. It
is fine at the moment because all page tables are always mapped so
there is no need to track the life time of each variable.
We will soon have the requirement to map and unmap page tables. We
need to track the life time of each variable to avoid leakage.
Introduce some l{1,2}t variables with limited scope so that we can
track life time of pointers to xen page tables more easily.
No functional change.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The pl2e and pl1e variables are heavily (ab)used in that function. It
is fine at the moment because all page tables are always mapped so
there is no need to track the life time of each variable.
We will soon have the requirement to map and unmap page tables. We
need to track the life time of each variable to avoid leakage.
Introduce some l{1,2}t variables with limited scope so that we can
track life time of pointers to xen page tables more easily.
No functional change.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
They were put into page.h but mm.h is more appropriate.
The real reason is that I will be adding some new functions which
takes mfn_t. It turns out it is a bit difficult to do in page.h.
No functional change.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
xc_dom_p2m() and dom->p2m_host[] implement a linear transform for translated
domains, but waste a substantial chunk of RAM doing so.
ARM literally never reads dom->p2m_host[] (because of the xc_dom_translated()
short circuit in xc_dom_p2m()). Drop it all.
x86 HVM does use dom->p2m_host[] for xc_domain_populate_physmap_exact() calls
when populating 4k pages. Reuse the same tactic from 2M/1G ranges and use an
on-stack array instead. Drop the memory allocation.
x86 PV guests do use dom->p2m_host[] as a non-identity transform. Rename the
field to pv_p2m to make it clear it is PV-only.
No change in the constructed guests.
Reported-by: Varad Gautam <vrd@amazon.de>
Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
In-guest p2m's are a concept specific to x86 PV guests. alloc_p2m_list() is
the only hook which initialises dom->p2m_guest, making
xc_dom_update_guest_p2m() a nop for non-PV guests.
Move p2m_guest into xc_dom_image_x86 and adjust alloc_p2m_list() to match.
Drop xc_dom_update_guest_p2m() entirely.
One caller, move_l3_below_4G(), only uses it to modify a single entry, so
rewriting the whole guest p2m is wasteful - opencode the single update
instead. The other caller is common code. Instead, move the logic into the
setup_pgtables() hooks, which know their own sizeof_pfn and can do away with
the switch statement.
No change in the constructed guests.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Currently, the setup_pgtable() hook is optional, but alloc_pgtable() hook is
not. Both are specific to x86 PV guests, and stubbed in various ways by the
dombuilders for translated guests (x86 HVM, ARM).
Make alloc_pgtables() optional, and drop all the stubs for translated guest
types.
No change in the constructed guests.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Acked-by: Julien Grall <julien@xen.org>
The two xc_dom_params structures for PV pagetables are never modified and can
live in .rodata. Reduce their scope to the alloc_pgtable_*() functions which
construct xc_dom_image_x86 appropriately.
Rename {alloc,setup}_pgtables() to {alloc,setup}_pgtables_pv() to highlight
that they are PV only, and drop some _x86() suffixes from static helpers.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
The net diffstat is:
add/remove: 0/13 grow/shrink: 25/129 up/down: 6297/-20469 (-14172)
With the following objects/functions removed entirely:
iommu_hwdom_none 1 - -1
hwdom_max_order 4 - -4
extra_hwdom_irqs 4 - -4
ctldom_max_order 4 - -4
acpi_c1e_quirk 43 - -43
hvm_pirq_eoi 62 - -62
max_order 94 - -94
conring_puts 104 - -104
propagate_node 119 - -119
mmio_ro_emulate_ops 224 - -224
mmcfg_intercept_ops 224 - -224
pci_cfg_ok 295 - -295
p2m_lock 546 - -546
And the following reduced to stubs:
arch_iommu_hwdom_init 852 2 -850
p2m_add_foreign 880 16 -864
This patch also has the unintended but useful consequence of stopping
hardware_dom= functionality from being usable (in at least PV_SHIM_EXCLUSIVE
builds).
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>
XCFLAGS_CHECKPOINT_COMPRESS has been unused since c/s b15bc4345 (2015),
XCFLAGS_HVM since c/s 9e8672f1c (2013), and XCFLAGS_STDVGA since c/s
087d43326 (2007). Drop the constants, and code which sets them.
The separate hvm parameter (appeared in c/s d11bec8a1, 2007 and ultimately
redundant with XCFLAGS_HVM), is used for sanity checking and debug printing,
then discarded and replaced with Xen's idea of whether the domain is PV or
HVM.
Rearrange the logic in xc_domain_save() to ask Xen sightly earlier, and use a
consistent idea of 'hvm' throughout. Removing this parameter removes the
final user of libxl's dss->hvm, so drop that field as well.
Update the doxygen comment to be accurate.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <Ian.Jackson@citrix.com>
The compiler can't fold because of the write to *perr in the first hunk.
No functional change, but slightly better compiled code.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Fix c&p bug in the livepatch_do_action() code of
LIVEPATCH_ACTION_REPLACE case.
The correct variable handling return code of revert action is
other->rc in this case.
Coverity-ID: 1457467
Fixes: 6047104c3c ("livepatch: Add per-function applied/reverted state tracking marker")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The first attempt resulted in several "Free of address-of
expression (BAD_FREE)" issues, because of code which relies on the fact that
any pointer in the same page is ok to pass to unmap_domain_page()
Model this property to remove the issues.
Coverity IDs: 1135356 113536{0,1} 1401300 141809{0,1} 1438864
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
There are issues as reported by osstest when Xen is running nested on
itself and the L1 Xen is using x2APIC. While those are being
investigated, disable announcing the x2APIC feature in CPUID when nested
HVM mode is enabled.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Gdbsx support in the hypervisor is rarely used and it is opening a
way for dom0 to modify the running hypervisor by very easy means.
Remove the possibility to read/write hypervisor memory, it was never
used by gdbsx.
Add a Kconfig option to control support of gdbsx. Default is on.
While at it correct a wrong comment in related code and remove dead
code.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
debugger_trap_entry() is not needed without CONFIG_CRASH_DEBUG, so only
include it if CONFIG_CRASH_DEBUG is defined.
While at it remove CONFIG_HAS_GDBSX as it can easily be replaced by
CONFIG_CRASH_DEBUG.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The hvm and pae parameters are a remnant of legacy migration. They have 0
passed in from libxl_stream_read.c's process_record(), and are discarded in
xc_domain_restore().
While dropping these, update the doxygen comment to be accurate, and simplify
the other hvm vs pv handling in xc_domain_restore().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
The jmp after setting %cr0 is redundant with the following ljmp.
The CPUID to protect the jump to higher mappings was inserted due to an
abundance of caution/paranoia before Spectre was public. It doesn't usefully
protect against an attack, which is able to leak memory with one single
instruction's worth of onward speculation.
Only CPU Hotplug (if used at all) will use this path while guests are
executing. An attacker would have to be running and primed on an adjacent
thread while a hotplug event occurred, to gain one single data sample, and
have some other way of inferring that a hotplug event has occurred, which it
won't know directly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
We will provide a header file for Hyper-V hypercalls.
No functional change.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
...if a specified domid is already in use.
XEN_DOMCTL_createdomain allows a domid to be specified by its caller and
will correctly fail if that domid is already in use. However the errno
returned in this case will be EINVAL, making it indistinguishable from
several other failures. Also a value of EINVAL does not seem appropriate
as the specified domid is valid [1] but just not (transiently) available.
[1] any invalid value passed in is ignored and causes Xen to choose an
unused and valid value.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
...for patches not (yet) upstream.
This patch is simply adding a comment to reserve save record number space
to avoid the risk of clashes between existent downstream changes made by
Amazon and future upstream changes which may be incompatible.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Wei Liu <wl@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Using memcpy() may result in multiple individual byte accesses
(dependening how memcpy() is implemented and how the resulting insns,
e.g. REP MOVSB, get carried out in hardware), which isn't what we
want/need for carrying out guest insns as correctly as possible. Fall
back to memcpy() only for accesses not 2, 4, or 8 bytes in size.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
cr-daily-branch ought to be called via cr-for-branches so that we take
the lock. Otherwise strange things can occur if cron runs
cr-daily-branch in the same directory - in particular, it will be
likely to update the osstest revision, breaking everything.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Most users pass a vcpu pointer, and only stopmachine_action() takes an integer
parameter. Switch to using void * to substantially reduce the number of
explicit casts.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
The use of return_reg() assumes ARM's 32bit ABI. Therefore, a failure such as
-EINVAL will appear as a large positive number near 4 billion to a 64bit ARM
guest which happens to use continue_hypercall_on_cpu().
Introduce a new arch_hypercall_tasklet_result() hook which is implemented by
both architectures, and drop the return_reg() macros. This logic will be
extended in a later change to make continuations out of the tasklet work.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <julien@xen.org>
Like %cr2 for pagefaults, %dr6 contains ancillary information for debug
exceptions, and needs similar handling.
For xendevicemodel_inject_event(), no ABI change is needed (although an API
one would be ideal). Switch from 'cr2' to 'extra' in variable names which
don't constitute an API change, and update the documentation to match.
For the monitor interface, vm_event_debug needs extending with a pending_dbg
field. This shall behave like the VT-x PENDING_DBG control. Extend
hvm_monitor_debug() and for now, always pass in 0 - this will be fixed
eventually, when other hypervisor bugfixes are complete.
While modifying hvm_monitor_debug(), take the opportunity to correct trap type
and instruction length from unsigned long to unsigned int, as they are both
tiny values.
Finally, adjust xen-access.c to the new expectations. Introspection tools
intercepting debug exceptions should mirror the new pending_dbg field into
xendevicemodel_inject_event() for %dr6 to be processed correctly for the
guest.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Jan Beulich
Juergen Gross
Andrew Cooper
Anthony PERARD
Wei Liu
Wei Liu
Pawel Wieczorkiewicz
Roger Pau Monne
Paul Durrant
Chad Dougherty
Ian Jackson