In order to have a simple and easy to remember means to suppress all the
more or less recent workarounds for hardware vulnerabilities, force
settings not controlled by "spec-ctrl=" also to their original defaults,
unless they've been forced to specific values already by earlier command
line options.
This is part of XSA-273.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This mitigation requires up-to-date microcode, and is enabled by default on
affected hardware if available, and is used for HVM guests
The default for SMT/Hyperthreading is far more complicated to reason about,
not least because we don't know if the user is going to want to run any HVM
guests to begin with. If a explicit default isn't given, nag the user to
perform a risk assessment and choose an explicit default, and leave other
configuration to the toolstack.
This is part of XSA-273 / CVE-2018-3620.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Guests (outside of the nested virt case, which isn't supported yet) don't need
L1D_FLUSH for their L1TF mitigations, but offering/emulating MSR_FLUSH_CMD is
easy and doesn't pose an issue for Xen.
The MSR is offered to HVM guests only. PV guests attempting to use it would
trap for emulation, and the L1D cache would fill long before the return to
guest context. As such, PV guests can't make any use of the L1D_FLUSH
functionality.
This is part of XSA-273 / CVE-2018-3646.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
See the comment in shadow.h for an explanation of L1TF and the safety
consideration of the PTEs.
In the case that CONFIG_SHADOW_PAGING isn't compiled in, crash the domain
instead. This allows well-behaved PV guests to function, while preventing
L1TF from being exploited. (Note: PV guest kernels which haven't been updated
with L1TF mitigations will likely be crashed as soon as they try paging a
piece of userspace out to disk.)
This is part of XSA-273 / CVE-2018-3620.
Signed-off-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan <tim@xen.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Switching to shadow mode is performed in tasklet context. To facilitate this,
we schedule the tasklet, then create a hypercall continuation to allow the
switch to take place.
As a consequence, the x86 mm code needs to cope with an L1e operation being
continuable. do_mmu{,ext}_op() may no longer assert that a continuation
doesn't happen on the final iteration.
To handle the arguments correctly on continuation, compat_update_va_mapping*()
may no longer call into their non-compat counterparts. Move the compat
functions into mm.c rather than exporting __do_update_va_mapping() and
{get,put}_pg_owner(), and fix an unsigned long/int inconsistency with
compat_update_va_mapping_otherdomain().
This is part of XSA-273 / CVE-2018-3620.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
To mitigate L1TF, we cannot alter an architecturally-legitimate PTE a PV guest
chooses to write, but we can force the PV domain into shadow mode so Xen
controls the PTEs which are reachable by the CPU pagewalk.
Introduce new shadow mode, PG_SH_forced, and a tasklet to perform the
transition. Later patches will introduce the logic to enable this mode at the
appropriate time.
To simplify vcpu cleanup, make tasklet_kill() idempotent with respect to
tasklet_init(), which involves adding a helper to check for an uninitialised
list head.
This is part of XSA-273 / CVE-2018-3620.
Signed-off-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan <tim@xen.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Shadowing a PV guest is only available when shadow paging is compiled in.
When shadow paging isn't available, guests can be crashed instead as
mitigation from Xen's point of view.
Ideally, dom0 would also be potentially-shadowed-by-default, but dom0 has
never been shadowed before, and there are some stability issues under
investigation.
This is part of XSA-273 / CVE-2018-3620.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Safe PTE addresses for L1TF mitigations are ones which are within the L1D
address width (may be wider than reported in CPUID), and above the highest
cacheable RAM/NVDIMM/BAR/etc.
All logic here is best-effort heuristics, which should in practice be fine for
most hardware. Future work will see about disentangling the SRAT handling
further, as well as having L0 pass this information down to lower levels when
virtualised.
This is part of XSA-273 / CVE-2018-3620.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
In Store.path_write(), Path.apply_modify() updates the node_created
reference and both the value of apply_modify() and node_created are
returned by path_write().
At least with OCaml 4.06.1 this leads to the value of node_created being
returned *before* it is updated by apply_modify(). This in turn leads
to the quota for a domain not being updated in Store.write(). Hence, a
guest can create an unlimited number of entries in xenstore.
The fix is to make evaluation order explicit.
This is XSA-272.
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Rob Hoes <rob.hoes@citrix.com>
The VPMU_MODE_OFF early-exit in vpmu_do_wrmsr() introduced by c/s
11fe998e56 bypasses all reserved bit checking in the general case. As a
result, a guest can enable BTS when it shouldn't be permitted to, and
lock up the entire host.
With vPMU active (not a security supported configuration, but useful for
debugging), the reserved bit checking in broken, caused by the original
BTS changeset 1a8aa75ed.
From a correctness standpoint, it is not possible to have two different
pieces of code responsible for different parts of value checking, if
there isn't an accumulation of bits which have been checked. A
practical upshot of this is that a guest can set any value it
wishes (usually resulting in a vmentry failure for bad guest state).
Therefore, fix this by implementing all the reserved bit checking in the
main MSR_DEBUGCTL block, and removing all handling of DEBUGCTL from the
vPMU MSR logic.
This is XSA-269.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
It was never expected to work, the implementation is incomplete.
As a side effect, it also prevents guests from triggering a
"BUG_ON(page_get_owner(pg) != d)" in gnttab_unpopulate_status_frames().
This is XSA-268.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
As it currently stands, 'xpti=dom0' is indistinguishable from the default
value, which means it will be overridden by ARCH_CAPABILITIES_RDCL_NO on fixed
hardware.
Switch opt_xpti to use -1 as a default like all our other related options, and
clobber it as soon as we have a string to parse.
In addition, 'xpti' alone should be interpreted in its positive boolean form,
rather than resulting in a parse error.
(XEN) parameter "xpti" has invalid value "", rc=-22!
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
A previous patch added support for priv-mapping guest resources directly
(rather than having to foreign-map, which requires P2M modification for
HVM guests).
This patch makes use of the new API to seed the guest grant table unless
the underlying infrastructure (i.e. privcmd) doesn't support it, in which
case the old scheme is used.
NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params() was
actually unnecessary, as the grant table has already been seeded
by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This patch allows grant table frames to be mapped using the
XENMEM_acquire_resource memory op.
NOTE: This patch expands the on-stack mfn_list array in acquire_resource()
but it is still small enough to remain on-stack.
NOTE: This patch also removes a bogus comment above the
grant_to_status_frames() function.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
[Rebase over "Explicitly default to gnttab v1 during domain creation"]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
For reasons which appear to be exclusively down to poor review of the grant
table v2 code, a grant table's version field was wasn't initialised during
creation.
A number of problems (including XSAs) have occurred in the past trying trying
to use a grant table which hasn't been properly set up, and various areas of
the code cope with v0 by defaulting to v1.
In particular, the toolstack using GNTTABOP_setup_table to be able to fill in
the store/console grants has a side effect of switching to v1.
In hindsight however, this "fixup if we see 0" is a very poor, with a
substantial degree of risk. Explicitly default to grant table v1 during
domain create, and let the rest of the code work safely in the knowledge that
the version is sensibly set.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Firstly, there is no 'offset' boundary check on the non-32-bit write path
before the call to vlapic_read_aligned(), which allows an attacker to read
beyond the end of vlapic->regs->data[], which is only 1024 bytes long.
However, as the backing memory is a domheap page, and misaligned accesses get
chunked down to single bytes across page boundaries, I can't spot any
XSA-worthy problems which occur from the overrun.
On real hardware, bad accesses don't instantly crash the machine. Their
behaviour is undefined, but the domain_crash() prohibits sensible testing.
Behave more like other x86 MMIO and terminate bad accesses with appropriate
defaults.
While making these changes, clean up and simplify the the smaller-access
handling. In particular, avoid pointer based mechansims for 1/2-byte reads so
as to avoid forcing the value to be spilled to the stack.
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-175 (-175)
function old new delta
vlapic_read 211 142 -69
vlapic_write 304 198 -106
Finally, there are a plethora of read/write functions in the vlapic namespace,
so rename these to vlapic_mmio_{read,write}() to make their purpose more
clear.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
It is not specific to HVM. It just so happens that PV doesn't need
special handling.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Both are entirely unused (to be fair, reserve_bootmem() has a use inside
an "#if 0" section in x86's mpparse.c, but if we were to re-enable that
code, it would need doing differently anyway).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
By default the container runs as the root user and since the source tree
is bind mounted into the container, any file is created and owned by the
root user which harms ergonomics when working outside of the container
environment. This maps the root user within the container to the uid of
the user outside of the container so files are not owned by root.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
This was something that existed for some scripting support for a totally
unrelated project and when I copied this script I failed to remove it so
this removes it. Build containers for Xen are best as ephemeral
environments and should just utilizes Docker's default container naming
behavior.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Standardized all the environment variable names that the containerize
script uses to start with CONTAINER_
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Add a Xen build target to count the lines of code of the source files
built. Uses `cloc' to do the job.
With Xen on ARM taking off in embedded, IoT, and automotive, we are
seeing more and more uses of Xen in constrained environments. Users and
system integrators want the smallest Xen and Dom0 configurations. Some
of these deployments require certifications, where you definitely want
the smallest lines of code count. I provided this patch to give us the
lines of code count for that purpose.
Use the .o.d files to account for all the built source files. Generate a
list for the `cloc' utility and invoke `cloc'.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
CC: jbeulich@suse.com
CC: andrew.cooper3@citrix.com
---
Changes in v4:
- use grep regex to get multiple source files from .d files
Changes in v3:
- remove build as dependecy for the cloc target
Changes in v2:
- change implementation to use .o.d to find built source files
Add specific per-platform defaults for NR_CPUS. Note that the order of
the defaults matter: they need to go first, otherwise the generic
defaults will be applied.
This is done so that Xen builds customized for a specific hardware
platform can have the right NR_CPUS number.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien.grall@arm.com>
CC: JBeulich@suse.com
CC: andrew.cooper3@citrix.com
---
Changes in v6:
- remove useless additional default for ALL
Add a "Platform Support" choice with four kconfig options: QEMU, RCAR3,
MPSOC and ALL_PLAT. They enable the required options for their hardware
platform. ALL_PLAT enables all available platforms and it's the default.
It doesn't automatically select any of the related drivers, otherwise
they cannot be disabled. ALL_PLAT is implemented by using hidden options
with default values depending on ALL_PLAT.
In the case of the MPSOC that has a platform file under
arch/arm/platforms/, build the file if MPSOC.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
CC: artem_mygaiev@epam.com
CC: volodymyr_babchuk@epam.com
---
Changes in v8:
- remove QEMU_PLATFORM and RCAR3_PLATFORM that are currently unused
- remove selects from ALL
- rename ALL to ALL_PLAT
- introduce ALL64_PLAT and ALL32_PLAT
Changes in v5:
- turn platform support into a choice
- add ALL
Changes in v4:
- fix GICv3/GICV3
- default y to all options
- build xilinx-zynqmp if MPSOC
Add a tiny kconfig configuration. Enabled only the credit scheduler.
It only carries non-default options (use make menuconfig or make
olddefconfig to produce a complete .config file).
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Changes in v7:
- remove NULL because it is still experimental
Introduce a Kconfig option for the ARM SMMUv1 and SMMUv2 driver.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
CC: jbeulich@suse.com
---
Changes in v3:
- rename SMMUv2 to ARM_SMMU
- improve help message
- use if ARM
Changes in v2:
- rename HAS_SMMUv2 to SMMUv2
- move SMMUv2 to xen/drivers/passthrough/Kconfig
All the UART drivers are silent options. Add one line descriptions so
that can be de/selected via menuconfig.
Add an x86 dependency to HAS_EHCI: EHCI PCI has not been used on ARM. In
fact, it depends on PCI, and moreover we have drivers for several
embedded UARTs for various ARM boards.
NS16550 remains not selectable on x86.
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Changes in v4:
- improve commit message
- remove prompt for HAS_EHCI
Changes in v3:
- NS16550 prompt if ARM
Changes in v2:
- make HAS_EHCI depend on x86
Select MEM_ACCESS_ALWAYS_ON on x86 to mark that MEM_ACCESS is not
configurable on x86. Avoid selecting it on ARM.
Rename HAS_MEM_ACCESS to MEM_ACCESS everywhere. Add a prompt and a
description to MEM_ACCESS in xen/common/Kconfig.
The result is that the user-visible option is MEM_ACCESS, and it is
configurable only on ARM (disabled by default). At the moment the
arch-specific mem_access code remains enabled on ARM, even with
MEM_ACCESS=y.
The purpose is to reduce code size. The option doesn't depend on EXPERT
because it would be nice to ecurity-support configurations without
MEM_ACCESS and a non-expert should be able to disable it.
Suggested-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien.grall@arm.com>
CC: dgdegra@tycho.nsa.gov
CC: andrew.cooper3@citrix.com
CC: George.Dunlap@eu.citrix.com
CC: ian.jackson@eu.citrix.com
CC: jbeulich@suse.com
CC: julien.grall@arm.com
CC: konrad.wilk@oracle.com
CC: sstabellini@kernel.org
CC: tim@xen.org
CC: wei.liu2@citrix.com
---
Changes in v5:
- change MEM_ACCESS_ALWAYS_ON to bool
- change default for MEM_ACCESS, default y if MEM_ACCESS_ALWAYS_ON
Changes in v4:
- remove HAS_MEM_ACCESS
- move MEM_ACCESS_ALWAYS_ON to common
- combile default and bool to def_bool
Changes in v3:
- keep HAS_MEM_ACCESS to mark that an arch can do MEM_ACCESS
- introduce MEM_ACCESS_ALWAYS_ON
- the main MEM_ACCESS option is in xen/common/Kconfig
Changes in v2:
- patch added
HAS_GICV3 has become selectable by the user. To mark the change, rename
the option from HAS_GICV3 to GICV3.
Suggested-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Changes in v3:
- no changes
Changes in v2:
- patch added
vmx_vmexit_handler() assumes that if
SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS is set, that the value in
EPTP_INDEX is valid. Unfortunately, the function which sets this bit
(vmx_vcpu_update_vmfunc_ve) doesn't actually set EPTP_INDEX; it will
only be set the next time vmx_vcpu_update_eptp() is called.
This means that if a vcpu makes a vmexit between these two points, the
EPTP_INDEX it reads will be invalid. The first time this race happens
for a domain, EPTP_INDEX will most likely be zero, which is the index
for the "host" p2m -- and thus is often correct. But the second time
this race happens, the value will typically be INVALID_ALTP2M, which
will hit the following BUG:
BUG_ON(idx >= MAX_ALTP2M);
Worse, if for some reason the current altp2m was *not* `0` during this
window (say, because a toolstack changed the VM to a different view),
then the accounting of active vcpus for an altp2m will be thrown off.
Fix this by always updating EPTP_INDEX to the current altp2m index
when enabling #VE.
Reported-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Tested-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
The address of an array slot can't be NULL. Instead add a bounds check
to make sure the array indexing is valid (the check is against 2 since
slot zero of the array - corresponding to C0 - is of no interest here).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Remove the handling for different page sizes now that ia64 is gone.
No functional change.
Suggested by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
domain_crash_synchronous() is unsafe to use in general as it may leave
spinlocks held, temporary memory allocated, etc.
With domain_crash_synchronous() removed from the ARM code in 4.11, take the
opportunity to remove the infrastructure completely by opencoding the softirq
loop in the remaining callsites, all of which are destined for deletion.
None of these sites are at risk of having a pending ioreq to qemu, which means
that the vcpu_end_shutdown_deferral() isn't necessary.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
There is no need for the syncrhonous varient, as the vmentry failure path can
just return to processing softirqs.
This is in aid of trying to remove domain_crash_syncrhonous() from the
codebase.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
If the EPTP pointer can't be located in the altp2m list, the domain
is (legitimately) crashed.
Under those circumstances, execution will continue and guarentee to hit the
BUG_ON(idx >= MAX_ALTP2M) (unfortunately, just out of context).
Return from vmx_vmexit_handler() after the domain_crash(), which also has the
side effect of reentering the scheduler more promptly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
gdb 8.0 fixed bounds checking for 'g' packet (commit
9dc193c3be85aafa60ceff57d3b0430af607b4ce "Check for truncated
registers in process_g_packet"). This revealed that gdbsx did
not properly formatted 'g' packet - segment registers and eflags are
expected to be 32-bit fields in the response (according to
gdb/features/i386/64bit-core.xml in gdb sources). Specific error is:
Truncated register 26 in remote 'g' packet
instead of silently truncating part of register.
Additionally, it looks like segment registers of 64bit guests were never
reported correctly, because of type mismatch.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Parsing/generating the escape sequences used by xenstore-client is non
trivial. So make scripting (for use in stubdom) easier by adding a raw
option.
[added man page entries, facor out expand_buffer]
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Add --replace-escape option to xenconsoled, which replaces ESC with
'.' in console output written to log file. This makes it slightly safer
to do tail -f on a console output of untrusted guest.
The pty output is unaffected by this option.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
[ wei: move variables into a narrower scope ]
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
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>
Jan Beulich
Andrew Cooper
Juergen Gross
Christian Lindig
Stefano Stabellini
Paul Durrant
Wei Liu
Alexandru Isaila
Doug Goldstein
George Dunlap
Roger Pau Monné
Marek Marczykowski-Górecki
Simon Gaiser