The altp2m functionality was originally envisioned to be used in
several different configurations, one of which was a single in-guest
agent that had full operational control of altp2m. This required the
single hypercall to be an HVMOP rather than a DOMCTL, since HVM guests
are not allowed to make DOMCTLs. Access to this HVMOP is controlled
by a per-domain HVM_PARAM, and defaults to 'off'.
Exposing the altp2m functionality to the guest was controversial at
the time, but was ultimately accepted. The fact that altp2m is an
HVMOP rather than a DOMCTL has caused some problems, however, for
those moving forward trying to extend the interface: Extending the
interface even for the 'external' use case now means extending an
HVMOP, which implicitly extends the surface of attack for the
'internal' use case as well. The result has been that every addition
to this interface has also been controversial.
Settle the controversy once and for all by documenting 1) the purpose
of the altp2m interface, and 2) how to extend it. In particular:
* Specify that the fully in-guest agent is a target use case
* Specify that all extensions to altp2m functionality should be subops
of the HVMOP hypercall
* Specify that new subops should be enabled in ALTP2M_mixed mode by
default, but that this mode has not been evaluated for safety.
Hopefully this will allow the altp2m interface to be developed further
without unnecessary controversy.
Further discussion:
As far as I can tell there are three possible solutions to this
controversy.
A. Remove the 'internal' functionality as a target by converting the
current HVMOP into a DOMCTL.
B. Have two hypercalls -- an HVMOP which contains functionality
expected to be used by the 'internal' agent, and a DOMCTL for
functionality which is expected to be used only be the 'external'
agent.
C. Agree to add all new subops to the current hypercall (HVMOP), even
if we're not sure if they should be exposed to the guest.
I think A is a terrible idea. Having a single in-guest agent is a
reasonable design choice, and apparently it was even implemented at
some point; we should make it straightforward for someone in the
future to pick up the work if they want to.
I think B is also a bad idea. The people extending it at the moment
are primarily concerned with the 'external' use case. There is nobody
around to represent whether new functionality should end up in the
HVMOP or the DOMCTL, which means that by default it will end up in the
DOMCTL. If it is discovered, afterwards, that the new operations
*would* be safe and useful for the 'internal' use case, then we will
either have to duplicate them inside the HVMOP (which would be
terrible) or move the operation from the DOMCTL to the HVMOP (which
would make coding an agent against several versions a mess).
It just makes more sense to have all the altp2m operations in a single
place, and a simple way to control whether they're available to the
'internal' use case or not. As such, I am proposing 'C'.
Even within that, we have several options as far as what to do with
the current interface:
C1: Audit the current subops and make a blacklist of subops not
suitable for exposure to the guest. Future subops should be on the
blacklist unless they have been evaluated as safe for exposure.
C2: Don't blacklist the current subops, but require that all future
subops be blacklisted unless they have been evaluated as safe for
exposure.
C3: Don't blacklist current or future subops for the present; just
document that they need to be evaluated (and some potentially
blacklisted) before being exposed to a guest in a safety-critical
environment.
C1 would be ideal, but there's nobody at present to do the work.
Given that, C3 has been seen as the best solution in discussion.
Reviewed-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
When compiling this file with gcc, the compiler happily accepts the
sequence of a label followed by an attribute. However, this sequence does
not follow the gcc documentation. Hence, other compilers might stumble
upon this statement.
To be able to compile Xen with goto-cc (the compiler of the CPROVER tool
suite), the missing semicolon is added in this commit.
Reported-by: Elizabeth Polgreen <polgreen@amazon.de>
Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
So that an ELF binary with support for EFI services will be built when
the compiler supports the MS ABI, regardless of the linker support for
PE.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Tested-by: Daniel Kiper <daniel.kiper@oracle.com>
So that it can be used by other components apart from the efi specific
code. By moving the detection code creating a dummy efi/disabled file
can be avoided.
This is required so that the conditional used to define the efi symbol
in the linker script can be removed and instead the definition of the
efi symbol can be guarded using the preprocessor.
The motivation behind this change is to be able to build Xen using lld
(the LLVM linker), that at least on version 6.0.0 doesn't work
properly with a DEFINED being used in a conditional expression:
ld -melf_x86_64_fbsd -T xen.lds -N prelink.o --build-id=sha1 \
/root/src/xen/xen/common/symbols-dummy.o -o /root/src/xen/xen/.xen-syms.0
ld: error: xen.lds:233: symbol not found: efi
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Daniel Kiper <daniel.kiper@oracle.com>
This image will always track the latest CentOS 7.x release. Add this
container to containerize for easy access.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
In qmp_next(), the inner loop should only try to parse messages from
QMP, if there is more than one.
The handling of the receive buffer ('incomplete'), should be done at the
same scope level as read(). It doesn't need to be handle more that once
after a read.
Before this patch, when on message what handled, the inner loop would
restart by adding the 'buffer' into 'incomplete' (after reallocation).
Since 'rd' was not reset, the buffer would be strcat a second time.
After that, the stream from the QMP server would have syntax error, and
the parsor would throw errors.
This is unlikely to happen as the receive buffer is very large. And
receiving two messages in a row is unlikely. In the current case, this
could be an event and a response to a command.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Also replace LIBXL__LOG_DEBUG by XTL_DEBUG, because it's shorter and
more often used in libxl.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
This patch fix complilation error with #define DEBUG_RECEIVED of the
macro DEBUG_REPORT_RECEIVED.
error: field precision specifier ‘.*’ expects argument of type ‘int’, but argument 9 has type ‘ssize_t {aka long int}’
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Today when setting up a minimal domain configuration file for dom0 and
eventually xenstore-domain the domain type is harcoded as PV. Change
that by asking the hypervisor for the correct type.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
The regexp used with find in order to list the man pages doesn't work
with FreeBSD find, so use a wildcard instead. No functional change.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
The default value of DEFCONFIG_LIST is wrong: it should be the value of
the configured ARCH_DEFCONFIG item, not the string "$ARCH_DEFCONFIG".
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
New field backend_type is added to vkb device in order to have QEMU and user
space backend simultaneously. Each vkb backend shall read appropriate XS entry
and service only own frontends. Id is a string field which used by the backend
to indentify the frontend.
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Logically it is better to move vkb to separate file as vkb device used not only
by vfb and console.
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
The iommu initialization will also create MMIO mappings in the Dom0
p2m, so the paging memory pool needs to be allocated or else iommu
initialization will fail.
Move the call to init the iommu after the Dom0 p2m has been setup in
order to solve this.
Note that issues caused by this wrong ordering have only been seen
when using shadow paging.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
If panic is called before init_idle_domain on a tboot-launched system,
then Xen recursively faults in write_ptbase as seen below.
(XEN) [<ffff82d080286690>] write_ptbase+0/0x10
(XEN) [<ffff82d0802c4c3b>] tboot_shutdown+0x6b/0x260
(XEN) [<ffff82d08029ddac>] machine_restart+0xac/0x2d0
(XEN) [<ffff82d080286690>] write_ptbase+0/0x10
(XEN) [<ffff82d0802446c1>] panic+0x111/0x120
(XEN) [<ffff82d0802a51c1>] do_general_protection+0x171/0x1f0
(XEN) [<ffff82d080287a82>] mm.c#virt_to_xen_l2e+0x12/0x1c0
(XEN) [<ffff82d080354720>] x86_64/entry.S#handle_exception_saved+0x66/0xa4
(XEN) [<ffff82d080286690>] write_ptbase+0/0x10
(XEN) [<ffff82d0802c4c3b>] tboot_shutdown+0x6b/0x260
(XEN) [<ffff82d08029ddac>] machine_restart+0xac/0x2d0
(XEN) [<ffff82d0802446c1>] panic+0x111/0x120
(XEN) [<ffff82d0803c11a0>] setup.c#bootstrap_map+0/0x11a
(XEN) [<ffff82d0803b82a0>] flask_op.c#parse_flask_param+0/0xb0
(XEN) [<ffff82d0803c11a0>] setup.c#bootstrap_map+0/0x11a
(XEN) [<ffff82d0803b6f6c>] xsm_multiboot_init+0x7c/0xb0
(XEN) [<ffff82d0803c34bb>] __start_xen+0x1d2b/0x2da0
(XEN) [<ffff82d0802000f3>] __high_start+0x53/0x60
idle_vcpu[0] is still poisoned with INVALID_VCPU, so write_ptbase faults
dereferencing the pointer. This fault calls panic and recurses through
the same code path.
If tboot_shutdown is called while idle_vcpu[0] == INVALID_VCPU, then we
are still operating with the initial page tables. Therefore changing
page tables with write_ptbase is unnecessary.
An easy way to reproduce this is to use tboot to launch an XSM-enabled
Xen without an XSM policy.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Level triggered interrupts are not an optional feature of HPET, and
must be implemented in order to comply with the HPET specification.
Implement them by adding a callback to the timer which sets the
interrupt bit in the general interrupt status register. Further
interrupts (in case of periodic mode) will not be injected until the
bit is cleared.
In order to reset the interrupts when the status bit is clear Xen must
also detect accesses to such register.
While there convert tn and i in hpet_write to unsigned.
Reported-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Level trigger interrupts will be asserted regardless of whether the
interrupt is masked, and thus the callback will also be executed.
Add a new 'level' parameter to create_periodic_time in order to create
level triggered timers. None of the current users of vpt are switched
to use level triggered interrupts yet.
Note that periodic level triggered interrupts are not supported. This
is because level triggered interrupts always require a deassert of the
IO-APIC pin, which should be done by the caller of vpt at which point
the caller should also reset the timer if required.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The calls to xpti_init_default() in parse_xpti() are buggy. The CPUID data
hasn't been fetched that early, and boot_cpu_has(X86_FEATURE_ARCH_CAPS) will
always evaluate false.
As a result, the default case won't disable XPTI on Intel hardware which
advertises ARCH_CAPABILITIES_RDCL_NO.
Simplify parse_xpti() to solely the setting of opt_xpti according to the
passed string, and have init_speculation_mitigations() call
xpti_init_default() if appropiate. Drop the force parameter, and pass caps
instead, to avoid redundant re-reading of MSR_ARCH_CAPS.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Because of a bug in 2010, LMSL support isn't available to guests.
c/s f2c608444 noticed but avoided fixing the issue for migration reasons. In
addition to migration problems, changes to the segmentation logic for
emulation would be needed before the feature could be enabled.
This feature is entirely unused by operating systems (probably owing to its
semantics which only cover half the segment registers), and no one has
commented on its absence from Xen. As supporting it would involve a large
amount of effort, it seems better to remove the code entirely.
If someone finds a valid usecase, we can resurrecting the code and
implementing the remaining parts, but I doubt anyone will.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
It turns out that nothing ever prevented HVM guests from trying to set unknown
EFER bits. Generally, this results in a vmentry failure.
For Intel hardware, all implemented bits are covered by the checks.
For AMD hardware, the only EFER bit which isn't covered by the checks is TCE
(which AFAICT is specific to AMD Fam15/16 hardware). We never advertise TCE
in CPUID, but it isn't a security problem to have TCE unexpected enabled in
guest context.
Disallow the setting of bits outside of the EFER_KNOWN_MASK, which prevents
any vmentry failures for guests, yielding #GP instead.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Clang complains:
systemd_stubs.c:51:8: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]
ret = Val_int(-1U);
^~~~~~~~~~~~
Since sd_notify_fd has a signature of unit -> unit, we simply change
the return value to Val_unit.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Currently building gdbsx with clang 3.8 gives the following errors:
xg_main.c:783:17: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes]
iop->uva = (uint64_aligned_t)((unsigned long)tobuf);
^~~~~~~~~~~~~~~~
/builds/liuw/xen/tools/debugger/gdbsx/xg/../../../../tools/include/xen/arch-x86/xen-x86_32.h:105:50: note: expanded from macro 'uint64_aligned_t'
^~~~~~~~~~
xg_main.c:816:17: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes]
iop->uva = (uint64_aligned_t)((unsigned long)frombuf);
^~~~~~~~~~~~~~~~
/builds/liuw/xen/tools/debugger/gdbsx/xg/../../../../tools/include/xen/arch-x86/xen-x86_32.h:105:50: note: expanded from macro 'uint64_aligned_t'
According to https://bugs.llvm.org/show_bug.cgi?id=11071, this issue has
been fixed in clang. But we're not going to get that in 3.8.
Explicitly disable that warning to fix the build.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
For the hostp2m, access_required starts off as 0, then it can be
set with xc_domain_set_access_required(). However, all the altp2ms
set it to 1 on init, and ignore both the hostp2m and the hypercall.
This patch sets access_required to the value from the hostp2m
on altp2m init, and propagates the values received via hypercall
to all the active altp2ms, when applicable.
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
This is to facilitate the values being passed in via domain_create(), at which
point the dom0 construction code needs to know them.
While cleaning up, drop the DEFAULT_* defines, which are only used immediately
adjacent in a context which makes it obvious that they are the defaults, and
drop the (unused) logic to allow a per-arch override of
DEFAULT_MAX_NR_GRANT_FRAMES.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
It is identical on all architecture, and this is a better overall than fixing
it up to have a proper boolean return value.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
This adds a script that can be used to do builds easily within the
defined containers under the automation directory. These containers live
in the public GitLab registry under the xen-project namespace. The
script can be executed a number of ways but the default is to drop you
at a bash shell within a Debian Stretch container at the top level of
the source tree.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Remove local definition of MIN and instead include the kernel.h header
for the hypervisor build. Fixes the following error on the tools build:
In file included from xc_dom_decompress_unsafe_lzma.c:8:0:
../../xen/common/unlzma.c:33:0: error: "MIN" redefined [-Werror]
#define MIN(a, b) (((a) < (b)) ? (a) : (b))
^
In file included from xc_private.h:43:0,
from xg_private.h:29,
from xc_dom_decompress_unsafe_lzma.c:5:
/home/osstest/build.125458.build-amd64/xen/stubdom/libxc-x86_64/../../tools/include/xen-tools/libs.h:21:0: note: this is the location of the previous definition
#define MIN(x, y) ((x) < (y) ? (x) : (y))
^
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
If xcr0_max exceeds xfeature_mask, then something is broken with the CPUID
policy derivation or auditing logic. If hardware rejects new_bv, then
something is broken with Xen's xstate logic.
In both cases, crash the domain with an obvious error message, to help
highlight the issues.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
It turns out that Xen has never enforced that a domain remain within the
xstate features advertised in CPUID.
The check of new_bv against xfeature_mask ensures that a domain stays within
the set of features that Xen has enabled in hardware (and therefore isn't a
security problem), but this does means that attempts to level a guest for
migration safety might not be effective if the guest ignores CPUID.
Check the CPUID policy in validate_xstate() (for incoming migration) and in
handle_xsetbv() (for guest XSETBV instructions). This subsumes the PKRU check
for PV guests in handle_xsetbv() (and also demonstrates that I should have
spotted this problem while reviewing c/s fbf9971241).
For migration, this is correct despite the current (mis)ordering of data
because d->arch.cpuid is the applicable max policy.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
To facilitate the shared Xen and toolstack code in libx86, struct msr_policy
needs to be available in the same way as struct cpuid_policy.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Move x86_cpuid_lookup_deep_deps() into the shared library, removing the
individual copies from the hypervisor and libxc respectively.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Both Xen and the toolstack have need of the same logic when it comes to
manipulation and checking of the CPUID and MSR values offered to guests. To
that end, libx86 is being introduced to allow Xen and the toolstack to share a
single implementation, rather than duplicating the logic.
No functional change.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Wei Liu
George Dunlap
Jan Beulich
Norbert Manthey
Roger Pau Monné
Doug Goldstein
Anthony PERARD
Juergen Gross
Oleksandr Grytsov
Jason Andryuk
Andrew Cooper
Razvan Cojocaru
Zhenzhong Duan