With XEN_DOMCTL_CDF_nested_virt now passed properly to domain_create(),
reimplement nestedhvm_enabled() to use the property which is fixed for the
lifetime of the domain.
This makes the call to nestedhvm_vcpu_initialise() from hvm_vcpu_initialise()
no longer dead. It became logically dead with the Xend => XL transition, as
they initialise HVM_PARAM_NESTEDHVM in opposite orders with respect to
XEN_DOMCTL_max_vcpus.
There is one opencoded user of nestedhvm_enabled() in HVM_PARAM_ALTP2M's
safety check.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
Like other major areas of functionality, nested virt (or not) needs to be
known at domain creation time for sensible CPUID handling, and wants to be
known this early for sensible infrastructure handling in Xen.
Introduce XEN_DOMCTL_CDF_nested_virt and modify libxl to set it appropriately
when creating domains. There is no need to adjust the ARM logic to reject the
use of this new flag.
No functional change yet.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Introduce some local variables to make the resulting logic easier to follow.
Join the two IOMMU checks in sanitise_domain_config(). Tweak some of the
terminology for better accuracy.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
The use of the ternary operator serves only to obfuscate the code. Rewrite it
in more simple terms, avoiding the need to conditionally OR zero into the
flags.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
OSSTest reports that libvirt is reliably regressed.
The only possible option is a side effect of using libxl_defbool_val(), which
can only be the assert() within. Unfortunately, libvirt actually crashes in
__vfscanf_internal() while presumably trying to render some form of error.
Open code the check without the assert() to unblock staging, while we
investigate what is going on with libvirt. This will want reverting at some
point in the future.
Not-really-fixes: bfcc97c08c ("tools/cpuid: Plumb nested_virt down into xc_cpuid_apply_policy()" reliably breaks libvirt.)
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
Don't hardcode the lookup start level of the page table walk to 1
and instead match the one used in P2M. This should fix scenarios
involving SMMU where the start level is different than 1.
In order for the SMMU driver to also compile on arm32 move the
P2M_ROOT_LEVEL in the p2m header file (while at it, for
consistency also P2M_ROOT_ORDER) and use the macro in the smmu
driver.
Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Both evtchn->priority and evtchn->notify_vcpu_id could change behind the
back of evtchn_fifo_set_pending(), as for it - in the case of
interdomain channels - only the remote side's per-channel lock is held.
Neither the queue's priority nor the vCPU's vcpu_id fields have similar
properties, so they seem better suited for the purpose. In particular
they reflect the respective evtchn fields' values at the time they were
used to determine queue and vCPU.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
The general expectation is that there are only a few open ports left
when a domain asks its event channel configuration to be reset.
Similarly on average half a bucket worth of event channels can be
expected to be inactive. Try to avoid iterating over all channels, by
utilizing usage data we're maintaining anyway.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Acked-by: Julien Grall <jgrall@amazon.com>
xmalloc() & Co may not be called with IRQs off, or else check_lock()
will have its assertion trigger about locks getting acquired
inconsistently. Re-arranging the locking in evtchn_send() doesn't seem
very reasonable, especially since the per-channel lock was introduced to
avoid acquiring the per-domain event lock on the send paths. Issue a
second call to xsm_evtchn_send() instead, before acquiring the lock, to
give XSM / Flask a chance to pre-allocate whatever it may need.
As these nodes are used merely for caching earlier decisions' results,
allocate just one node in AVC code despite two potentially being needed.
Things will merely be not as performant if a second allocation was
wanted, just like when the pre-allocation fails.
Fixes: c0ddc86348 ("evtchn: convert per-channel lock to be IRQ-safe")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Just like HVM, defaulting SHADOW_PAGING and TBOOT to Yes in shim-
exclusive mode makes no sense, as the respective code is dead there.
Also adjust the shim default config file: It needs to specifiy values
only for settings where a non-default value is wanted.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
While there's little point in enabling both, the combination ought to at
least build correctly. Drop the direct PV_SHIM_EXCLUSIVE conditionals
and instead zap PG_log_dirty to zero under the right conditions, and key
other #ifdef-s off of that.
While there also expand on ded576ce07 ("x86/shadow: dirty VRAM
tracking is needed for HVM only"): There was yet another is_hvm_domain()
missing, and code touching the struct fields needs to be guarded by
suitable #ifdef-s as well. While there also guard shadow-mode-only
fields accordingly.
Fixes: 8b5b49ceb3 ("x86: don't include domctl and alike in shim-exclusive builds")
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>
Don't include struct elf_dom_parms in struct xc_dom_image, but rather
use a pointer to reference it. Together with adding accessor functions
for the externally needed elements this enables to drop including the
Xen private header xen/libelf/libelf.h from xenguest.h.
Fixes: 7e0165c193 ("tools/libxc: untangle libxenctrl from libxenguest")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
The pluggable kernel loader interface is used only internally of
libxenguest, so make it private. This removes a dependency on the Xen
internal header xen/libelf/libelf.h from xenguest.h.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Today xenctrl_dom.h is part of libxenctrl as it is included by
xc_private.c. This seems not to be needed, so merge xenctrl_dom.h into
xenguest.h where its contents really should be.
Replace all #includes of xenctrl_dom.h by xenguest.h ones or drop them
if xenguest.h is already included.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
libxlutil doesn't follow the standard name pattern of all other Xen
libraries, so add another make variable which can be used to allow
other names.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Rename *_libxlutil make variables to *_libxenutil in order to avoid
nasty indirections when moving libxlutil under the tools/libs
infrastructure.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Carve out all libxenlight related sources and move them to
tools/libs/light in order to use the generic library build environment.
The closely related sources for libxl-save-helper and the libxl test
environment are being moved, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
MFENCE is overly heavyweight for SMP semantics on WB memory, because it also
orders weaker cached writes, and flushes the WC buffers.
This technique was used as an optimisation in Java[1], and later adopted by
Linux[2] where it was measured to have a 60% performance improvement in VirtIO
benchmarks.
The stack is used because it is hot in the L1 cache, and a -4 offset is used
to avoid creating a false data dependency on live data.
For 64bit userspace, the Red Zone needs to be considered. Use -32 to allow
for a reasonable quantity of Red Zone data, but still have a 50% chance of
hitting the same cache line as %rsp.
Fix up the 32 bit definitions in HVMLoader and libxc to avoid a false data
dependency.
[1] https://shipilev.net/blog/2014/on-the-fence-with-dependencies/
[2] https://git.kernel.org/torvalds/c/450cbdd0125cfa5d7bbf9e2a6b6961cc48d29730
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
The manpage for 'xl' documents an option to force a block device to be
released even if the domain to which it is attached does not co-operate.
The documentation also states that, if the force flag is not specified, the
block-detach operation should fail.
Currently the force option is not implemented and a non-forced block-detach
will auto-force after a time-out of 10s. This patch implements the force
option and also stops auto-forcing a non-forced block-detach by calling
libxl_device_disk_safe_remove() rather than libxl_device_disk_remove(),
allowing the operation to fail cleanly as per the documented behaviour.
NOTE: The documentation is also adjusted since the normal positioning of
options is before compulsory parameters. It is also noted that use of
the --force option may lead to a guest crash.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
... and use it to define libxl_device_disk_safe_remove().
This patch builds on the existent macro magic by using a new value of the
'force' field in in libxl__ao_device.
It is currently defined as an int but is used in a boolean manner where
1 means the operation is forced and 0 means it is not (but is actually forced
after a 10s time-out). In adding a third value, this patch re-defines 'force'
as a struct type (libxl__force) with a single 'flag' field taking an
enumerated value:
LIBXL__FORCE_AUTO - corresponding to the old 0 value
LIBXL__FORCE_ON - corresponding to the old 1 value
LIBXL__FORCE_OFF - the new value
The LIBXL_DEFINE_DEVICE_REMOVE() macro is then modified to define the
libxl_device_<type>_remove() and libxl_device_<type>_destroy() functions,
setting LIBXL__FORCE_AUTO and LIBXL__FORCE_ON (respectively) in the
libxl__ao_device passed to libxl__initiate_device_generic_remove() and a
new macro, LIBXL_DEFINE_DEVICE_SAFE_REMOVE(), is defined that sets
LIBXL__FORCE_OFF instead. This macro is used to define the new
libxl_device_disk_safe_remove() function.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
When running as a stubdom Xenstore should set the maximum number of
grants needed via a call of xengnttab_set_max_grants(), as otherwise
the number of domains which can be supported will be 128 only (the
default number of grants supported by Mini-OS).
We use one grant per domain so the theoretical maximum number is
DOMID_FIRST_RESERVED.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Nested Virt is the final special case in legacy CPUID handling. Pass the
(poorly named) nested_hvm setting down into xc_cpuid_apply_policy() to break
the semantic dependency on HVM_PARAM_NESTEDHVM.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
If the bitmap is used to represent domU pages, the amount of memory is
limited to 8TB due to the 32bit value. Adjust the code to use 64bit
values as input. All callers already use some form of 64bit as input,
so no further adjustment is required.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
QEMU as recently switch its build system to use meson and the
./configure step with meson is more restrictive that the step used to
be, most installation path wants to be within prefix, otherwise we
have this error message:
ERROR: The value of the 'datadir' option is '/usr/share/qemu-xen' which must be a subdir of the prefix '/usr/lib/xen'.
In order to workaround the limitation, we will set prefix to the same
one as for the rest of Xen installation, and set all the other paths.
For reference, a thread in qemu-devel:
"configure with datadir outside of --prefix fails with meson"
https://lore.kernel.org/qemu-devel/20200918133012.GH2024@perard.uk.xensource.com/t/
And an issue in meson:
"artificial limitation of directories (forced to be in prefix)"
https://github.com/mesonbuild/meson/issues/2561
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Paul Durrant <paul@xen.org>
Acked-by: Wei Liu <wl@xen.org>
The caller of writev_exact should be notified about malloc errors
when dealing with partial writes.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
This patch wraps the EFI OutputString() method so that they can be
called with const arguments. The OutputString method does not modify
its argument, although the prototype is missing const, so it is necssary
to cast away the const when calling it.
Signed-off-by: Trammell Hudson <hudson@trmm.net>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
There's no other path causing a terminal unlink_pirq_port() to be called
(evtchn_bind_vcpu() relinks it right away) and hence _if_ pirq can
indeed be NULL when closing the port, list corruption would occur when
bypassing the unlink (unless the structure never gets linked again). As
we can't come here after evtchn_destroy() anymore, (late) domain
destruction also isn't a reason for a possible exception, and hence the
only alternative looks to be that the check was pointless in the first
place. While I haven't observed the case, from code inspection I'm far
from sure I can exclude this being possible, so it feels more safe to
re-arrange the code instead.
Fixes: c24536b636 ("replace d->nr_pirqs sized arrays with radix tree")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Before and after XSA-342 there has been an asymmetry in how not really
usable ports get treated in do_poll(): Ones beyond a certain boundary
(max_evtchns originally, valid_evtchns subsequently) did get refused
with -EINVAL, while lower ones were accepted despite there potentially
being no way to wake the vCPU again from its polling state. Arrange to
also honor evtchn_usable() output in the decision.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Callers have no business knowing the state of the Xen end of an event
channel.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <Paul@xen.org>
is_pv_32bit_domain() is an expensive predicate, but isn't used for speculative
safety in this case. Swap to checking the Long Mode bit in the CPUID policy,
which is the architecturally correct behaviour.
is_canonical_address() is currently moderately simple, but it will become more
complicated when 5-level support is added. Rearrange write_msr() to collapse
the common checks.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Opencode the fs/gs helpers, as the optimiser is unable to rearrange the logic
down to a single X86_CR4_FSGSBASE check. This removes several jumps and
creates bigger basic blocks.
In load_segments(), optimise GS base handling substantially. The call to
svm_load_segs() already needs gsb/gss the correct way around, so hoist the
logic for the later path to use it as well. Swapping the inputs in GPRs is
far more efficient than using SWAPGS.
Previously, there was optionally one SWAPGS from the user/kernel mode check,
two SWAPGS's in write_gs_shadow() and two WRGSBASE's. Updates to GS (4 or 5
here) in quick succession stall all contemporary pipelines repeatedly. (Intel
Core/Xeon pipelines have segment register renaming[1], so can continue to
speculatively execute with one GS update in flight. Other pipelines cannot
have two updates in flight concurrently, and must stall dispatch of the second
until the first has retired.)
Rewrite the logic to have exactly two WRGSBASEs and one SWAPGS, which removes
two stalles all contemporary processors.
Although modest, the resulting delta is:
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-106 (-106)
Function old new delta
paravirt_ctxt_switch_from 235 198 -37
context_switch 3582 3513 -69
in a common path.
[1] https://software.intel.com/security-software-guidance/insights/deep-dive-intel-analysis-speculative-behavior-swapgs-and-segment-registers
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Split into two functions. Passing a load of zeros in results in unnecessary
caller setup code.
Update the prefetching comment to note that the main point is the TLB fill.
Reorder the writes in svm_load_segs() to access the VMCB fields in ascending
order, which gets better next-line prefetch behaviour out of hardware. Update
the prefetch instruction to match.
The net delta is:
add/remove: 1/0 grow/shrink: 0/2 up/down: 38/-39 (-1)
Function old new delta
svm_load_segs_prefetch - 38 +38
__context_switch 967 951 -16
svm_load_segs 291 268 -23
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
To match the read side which is already split out. A future change will want
to use them.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
They are currently named after the FSGSBASE instructions, but are not thin
wrappers around said instructions, and therefore do not accurately reflect the
logic they perform, especially when it comes to functioning safely in non
FSGSBASE context.
Rename them to {read,write}_{fs,gs}_base() to avoid confusion.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
We can just as well specify the CRn structure fields directly in the
asm()s, just like done for all other ones.
Also drop the unecessary "volatile" from the asm()-s: There are no
relevant hidden side effects.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This keeps at least gcc 10 from generating a separate function instance
in common/kexec.o alongside the inlining of the function in its sole
caller. I also think putting the address of the actual code storing the
registers is a better indication to consumers than that of an otherwise
unreferenced function.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
When storing all (data) segment registers in one go, prefer writing the
selector values directly to memory (as opposed to read_sreg()).
Also move the single register variant into the regs.h, dropping the
unecessary "volatile" from the asm() (there are no hidden side effects).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
SMMUv{1, 2} are both marked as security supported, so we would
technically have to issue an XSA for any IOMMU security bug.
However, at the moment, device passthrough is not security supported
on Arm and there is no plan to change that in the next few months.
Therefore, mark Arm SMMUv{1, 2} as supported but not security supported.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Some callers of vcpu_pause() will expect to access the latest vcpu
context when the function returns (see XENDOMCTL_{set,get}vcpucontext}.
However, the latest vCPU context can only be observed after
v->is_running has been observed to be false.
As there is no memory barrier instruction generated, a processor could
try to speculatively access the vCPU context before it was observed.
To prevent the corruption of the vCPU context, we need to insert a
memory barrier instruction after v->is_running is observed and before
the context is accessed. This barrier is added in sync_vcpu_execstate()
as it seems to be the place where we expect the synchronization to
happen.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Fix the resume path to load the shadow stack pointer from saved_ssp (not
saved_rsp), to match what suspend path does.
Fixes: 633ecc4a7c ("x86/S3: Save and restore Shadow Stack configuration")
Backport: 4.14
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Remove the code to compose a MSI message based on the information from
the MSI registers and the data in the interrupt remapping table.
Since the removal of read_msi_msg and its user there's no longer a
need for such code, as the last written (untranslated) MSI message is
cached internally by Xen.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Later additions look to have been put at the end, with MEM_ACCESS*
somewhere in the middle. Re-sort this part of the file, in the hope that
future additions will be made noticing the intentions here.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Andrew Cooper
Laurentiu Tudor
Roger Pau Monné
Jan Beulich
Juergen Gross
Paul Durrant
Olaf Hering
Anthony PERARD
Trammell Hudson
Julien Grall
Marek Marczykowski-Górecki