Generally speaking, the event channel local/remote port is fixed for the
lifetime of the associated domain object. The exception to this is a
secondary XS_INTRODUCE (defined to re-bind to a new event channel) which pokes
around at the domain object's internal state.
We need to refactor the evtchn handling to support live update, so start by
moving the relevant manipulation into Domain.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit aecdc28d95)
This will make the logic clearer when we plumb local_port through these
functions.
While doing this, rearrange the construct in Domains.create0 to separate the
remote port handling from the interface handling. (The interface logic is
dubious in several ways, but not altered by this cleanup.)
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 31fbee749a)
Xenstored always needs to bind the DOM_EXC VIRQ.
Instead of doing it shortly after the call to Event.init(), do it in the
constructor directly. This removes the need for the field to be a mutable
option.
It will also simplify a future change to support live update. Rename the
field from virq_port (which could be any VIRQ) to it's proper name.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 9804a5db43)
This file has some style problems so severe that they interfere with the
readability of the subsequent bugfix patches.
Fix these issues ahead of time, to make the subsequent changes more readable.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit b45bfaf359)
For live update, oxenstored wants to clear CLOEXEC on the evtchn handle, so it
survives the execve() into the new oxenstored.
Have the new interface match how cloexec works in other Ocaml standard
libraries.
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 9bafe4a533)
There is no binding for xenevtchn_close(). In principle, this is a resource
leak, but the typical usage is as a singleton that lives for the lifetime of
the program.
Ocaml 5 no longer permits storing a naked C pointer in an Ocaml value.
Therefore, use a Custom block. This allows us to use the finaliser callback
to call xenevtchn_close(), if the Ocaml object goes out of scope.
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 22d5affdf0)
A debug statement got inserted into a single-expression if statement.
Insert brackets to give the intended meaning, rather than the actual meaning
where the "let con = Connections..." is outside and executed unconditionally.
This results in some unnecessary ring checks for domains which otherwise have
IO credit.
Fixes: 42f0581a91 ("tools/oxenstored: Implement live update for socket connections")
Reported-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit ee36179371)
Make sure tsc_khz is promoted to a 64-bit type before multiplying by
1000 to avoid an 'overflow before widen' bug. Otherwise just above
4.294GHz the value will overflow. Processors with clocks this high are
now in production and require this to work correctly.
Signed-off-by: Neowutran <xen@neowutran.ovh>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: ad15a0a8ca
master date: 2022-12-19 11:34:16 +0100
This was missed in the initial patchset.
Move efi_relocate_esrt() up to avoid adding a forward declaration.
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 8d7acf3f7d
master date: 2022-12-14 12:00:35 +0100
Avoid incorrectly triggering an error when a broadcast buffered ioreq
is not handled by all registered clients, as long as the failure is
strictly because the client doesn't handle buffered ioreqs.
Signed-off-by: Per Bilse <per.bilse@citrix.com>
Reviewed-by: Paul Durrant <paul@xen.org>
master commit: a44734df6c
master date: 2022-12-07 12:17:30 +0100
Linux'es relatively new use of HVMOP_set_evtchn_upcall_vector has
exposed a problem with the marking of the respective vector as
pending: For quite some time Linux has been checking whether any stale
ISR or IRR bits would still be set while preparing the LAPIC for use.
This check is now triggering on the upcall vector, as the registration,
at least for APs, happens before the LAPIC is actually enabled.
In software-disabled state an LAPIC would not accept any interrupt
requests and hence no IRR bit would newly become set while in this
state. As a result it is also wrong for us to mark the upcall vector as
having a pending request when the vLAPIC is in this state.
To compensate for the "enabled" check added to the assertion logic, add
logic to (conditionally) mark the upcall vector as having a request
pending at the time the LAPIC is being software-enabled by the guest.
Note however that, like for the pt_may_unmask_irq() we already have
there, long term we may need to find a different solution. This will be
especially relevant in case yet better LAPIC acceleration would
eliminate notifications of guest writes to this and other registers.
Fixes: 7b5b8ca7df ("x86/upcall: inject a spurious event after setting upcall vector")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
master commit: f5d0279839
master date: 2022-12-06 13:51:49 +0100
In software-disabled state an LAPIC does not accept any interrupt
requests and hence no IRR bit would newly become set while in this
state. As a result it is also wrong for us to mark Viridian IPI or timer
vectors as having a pending request when the vLAPIC is in this state.
Such interrupts are simply lost.
Introduce a local variable in send_ipi() to help readability.
Fixes: fda96b7382 ("viridian: add implementation of the HvSendSyntheticClusterIpi hypercall")
Fixes: 26fba3c855 ("viridian: add implementation of synthetic timers")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Paul Durrant <paul@xen.org>
master commit: 831419f829
master date: 2022-12-02 10:35:32 +0100
In software-disabled state an LAPIC does not accept any interrupt
requests and hence no IRR bit would newly become set while in this
state. As a result it is also wrong for us to mark IO-APIC or MSI
originating vectors as having a pending request when the vLAPIC is in
this state. Such interrupts are simply lost.
Introduce (IO-APIC) or re-use (MSI) a local variable to help
readability.
Fixes: 4fe21ad371 ("This patch add virtual IOAPIC support for VMX guest")
Fixes: 85715f4bc7 ("MSI 5/6: add MSI support to passthrough HVM domain")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: f1d7aac1e3
master date: 2022-12-02 10:35:01 +0100
Currently Xen will passthrough any Local APIC NMI Structure found in
the native ACPI MADT table to a PVH dom0. This is wrong because PVH
doesn't have access to the physical local APIC, and instead gets an
emulated local APIC by Xen, that doesn't have the LINT0 or LINT1
pins wired to anything. Furthermore the ACPI Processor UIDs used in
the APIC NMI Structures are likely to not match the ones generated by
Xen for the Local x2APIC Structures, creating confusion to dom0.
Fix this by removing the logic to passthrough the Local APIC NMI
Structure for PVH dom0.
Fixes: 1d74282c45 ('x86: setup PVHv2 Dom0 ACPI tables')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: b39e638525
master date: 2022-12-02 10:33:40 +0100
Current code in _clear_irq_vector() will mark the irq as unused before
doing the cleanup required when move_in_progress is true.
This can lead to races in create_irq() if the function picks an irq
desc that's been marked as unused but has move_in_progress set, as the
call to assign_irq_vector() in that function can then fail with
-EAGAIN.
Prevent that by only marking irq descs as unused when all the cleanup
has been done. While there also use write_atomic() when setting
IRQ_UNUSED in _clear_irq_vector() and add a barrier in order to
prevent the setting of IRQ_UNUSED getting reordered by the compiler.
The check for move_in_progress cannot be removed from
_assign_irq_vector(), as other users (io_apic_set_pci_routing() and
ioapic_guest_write()) can still pass active irq descs to
assign_irq_vector().
Note the trace point is not moved and is now set before the irq is
marked as unused. This is done so that the CPU mask provided in the
trace point is the one belonging to the current vector, not the old
one.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: e267d11969
master date: 2022-12-02 10:32:21 +0100
A previous patch tried to get Linux to use the ESRT under Xen if it is
in memory of type EfiRuntimeServicesData. However, this turns out to be
a bad idea. Ard Biesheuvel pointed out that EfiRuntimeServices* memory
winds up fragmenting both the EFI page tables and the direct map, and
that EfiACPIReclaimMemory is a much better choice for this purpose.
Link: https://lists.xenproject.org/archives/html/xen-devel/2022-09/msg01365.html
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit d7669c1014)
The HVM flavor of the hypercall handlers exists only when GRANT_TABLE is
enabled, while surrogate shim variants exist only for the purpose of PV.
(Also scratch out the Arm variant in that case; what exactly is used in
that cell of the new table row doesn't really matter.)
Fixes: 8523851dbc ("xen/x86: call hypercall handlers via generated macro")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
master commit: 133f35f0f0
master date: 2022-12-02 10:40:11 +0100
Use actual include directory used to install the public header in
COPYING file.
Also, move the input file out of "tools/include/xen/" because that
directory is removed on `make clean`.
We can't used ./configure because $includedir contain another
variable, so the change is done in Makefile.
Fixes: 4ea75e9a90 ("Rework COPYING installed in /usr/include/xen/, due to several licences")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
master commit: 25b55688e1f20ebb1bb15d0e82079e91cd787e5b
master date: 2022-12-01 16:14:59 +0100
Some time ago, Linux switched the format of docs to ReST and the format
of device-tree bindings to json-schema.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
The error message accidentally printed the bytes value as if it were kB.
Furthermore, both b_info.shadow_memkb and shadow_mem are uint64_t, meaning
there is a risk of overflow if the user specified a stupidly large value in
the vm.cfg file. Check and reject such a condition.
Fixes: 7c3bbd940d ("xen/arm, libxl: Revert XEN_DOMCTL_shadow_op; use p2m mempool hypercalls")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Commit 34990446ca started to overwrite the `rc` value from
libxl__arch_domain_create(), thus error aren't propagated anymore.
Check `rc` value before doing the next thing.
Fixes: 34990446ca ("libxl: don't ignore the return value from xc_cpuid_apply_policy")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
On one of my boxes when the HDMI cable is not plugged in the
FrameBufferBase of the EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE structure is
set to 0 by the firmware (while some of the other fields looking
plausible).
Such (bogus address) ends up mapped in vesa_init(), and since it
overlaps with a RAM region the whole system goes down pretty badly,
see:
(XEN) vesafb: framebuffer at 0x0000000000000000, mapped to 0xffff82c000201000, using 35209k, total 35209k
(XEN) vesafb: mode is 0x37557x32, linelength=960, font 8x16
(XEN) vesafb: Truecolor: size=8:8:8:8, shift=24:0:8:16
(XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) �ERROR: Class:0; Subclass:0; Operation: 0
ERROR: No ConOut
ERROR: No ConIn
Do like Linux and prevent using the EFI Frame Buffer if the base
address is 0. This is inline with the logic in Linuxes
fb_base_is_valid() function at drivers/video/fbdev/efifb.c v6.0.9.
See also Linux commit 133bb070e94ab41d750c6f2160c8843e46f11b78 for
further reference.
Also prevent using Frame Buffers that have a 0 height or width, as
those are also invalid.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Allocating or freeing p2m pages doesn't alter the size of the mempool; only
the split between free and used pages.
Right now, the hypercalls operate on the free subset of the pool, meaning that
XEN_DOMCTL_get_paging_mempool_size varies with time as the guest shuffles its
physmap, and XEN_DOMCTL_set_paging_mempool_size ignores the used subset of the
pool and lets the guest grow unbounded.
This fixes test-pagign-mempool on ARM so that the behaviour matches x86.
This is part of XSA-409 / CVE-2022-33747.
Fixes: cbea5a1149 ("xen/arm: Allocate and free P2M pages from the P2M pool")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
This reverts most of commit cf2a68d2ff, and bits
of cbea5a1149.
First of all, with ARM borrowing x86's implementation, the logic to set the
pool size should have been common, not duplicated. Introduce
libxl__domain_set_paging_mempool_size() as a shared implementation, and use it
from the ARM and x86 paths. It is left as an exercise to the reader to judge
how libxl/xl can reasonably function without the ability to query the pool
size...
Remove ARM's p2m_domctl() infrastructure now the functioanlity has been
replaced with a working and unit tested interface.
This is part of XSA-409 / CVE-2022-33747.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Exercise some basic functionality of the new
xc_{get,set}_paging_mempool_size() hypercalls.
This passes on x86, but fails currently on ARM. ARM will be fixed up in
future patches.
This is part of XSA-409 / CVE-2022-33747.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
The existing XEN_DOMCTL_SHADOW_OP_{GET,SET}_ALLOCATION have problems:
* All set_allocation() flavours have an overflow-before-widen bug when
calculating "sc->mb << (20 - PAGE_SHIFT)".
* All flavours have a granularity of 1M. This was tolerable when the size of
the pool could only be set at the same granularity, but is broken now that
ARM has a 16-page stopgap allocation in use.
* All get_allocation() flavours round up, and in particular turn 0 into 1,
meaning the get op returns junk before a successful set op.
* The x86 flavours reject the hypercalls before the VM has vCPUs allocated,
despite the pool size being a domain property.
* Even the hypercall names are long-obsolete.
Implement a better interface, which can be first used to unit test the
behaviour, and subsequently correct a broken implementation. The old
interface will be retired in due course.
The unit of bytes (as opposed pages) is a deliberate API/ABI improvement to
more easily support multiple page granularities.
This is part of XSA-409 / CVE-2022-33747.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
I was really hoping to avoid this, but its now too late in the 4.17 freeze and
we still don't have working fixes.
The in-Xen calculations for assistance capabilities are buggy. For the
avoidance of doubt, the original intention was to be able to control every
aspect of a APIC acceleration so we could comprehensively test Xen's support,
as it has proved to be buggy time and time again.
Even after a protracted discussion on what the new API ought to mean, attempts
to apply it to the existing logic have been unsuccessful, proving that the
API/ABI is too complicated for most people to reason about.
This reverts most of:
2ce11ce2496b2b9b3405
leaving in place the non-APIC specific changes (minimal as they are).
This takes us back to the behaviour of Xen 4.16 where APIC acceleration is
configured on a per system basis.
This work will be revisted in due course.
Fixes: 2ce11ce249 ("x86/HVM: allow per-domain usage of hardware virtualized APIC")
Fixes: 6b2b9b3405 ("x86: report Interrupt Controller Virtualization capabilities")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
The notice in the COPYING file in "xen/include/public/COPYING" doesn't
really apply to the files that ultimately are been install at
"/usr/include/xen". The issue are headers in the "sys/" subdirectory
that comes from other projects such as Linux or FreeBSD.
The main issue is that there are two headers that have a different
licence than the MIT licence:
- xen-sys/Linux/gntalloc.h (installed as "sys/gntalloc.h") is public
domain.
- xen-sys/FreeBSD/gntdev.h (installed as "sys/gntdev.h") is BSD-2.
To clarify this, we'll install a COPYING file with a different notice.
Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
This header have been created by moving code from other part of the
project and miss a licence header. The original source code was some
version of GPL or LGPL but we intend to have the public header to be
MIT so they can be included easily in other projects.
Part of device_tree_defs.h were moved from libxl_arm.c which is
LGPL-2.1-only. And part were moved from device_tree.h that is
GPL-2.0-only.
Part of the original code were added by Julien Grall @ Linaro in
commits c3ba52a84d and 405c167f0e and 886f34045b. The other part
were added by Ian Campbell @ Citrix, with commit 0c64527e7f.
Resolves: xen-project/xen#35
Fixes: 1c898a9fec ("xen/arm: move a few DT related defines to public/device_tree_defs.h")
Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> [Citrix relicensing]
Acked-by: Grant Likely <grant.likely@linaro.org> [Linaro relicensing]
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
The headers install in "/usr/include/xen/foreign/" are missing a
licence header. This patch adds a SPDX identifier to clarify that
the MIT licence is used.
The script now check that the licence of the input file is also MIT,
by checking for the presence of the SPDX identifier.
Also add information about which files are used to generate the
headers.
Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
The script "tools/include/xen-foreign/mkheader.py" is going to do a
sanity check on the licences of these headers. To ease this, we will
replace the verbatim copy of the MIT licence by its SPDX identifier
equivalent.
The text of the licence has been check to be the same as the one at
https://spdx.org/licenses/MIT.html, except we don't have "(including
the next paragraph)". The text is also the same as the one in
"xen/include/public/COPYING".
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
With the previous bugfix, X86_FEATURE_VIRT_SC_MSR_HVM is no longer
needed and can be replaced with an __initdata variable. This also
leaves asm/cpufeatures.h as it was in 4.16 which will simplify
backports.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Rewrite commit message. Move amd_virt_spec_ctrl into __initdata.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
This fixes an issue with running C code in a GIF=0 region, that's
problematic when using UBSAN or other instrumentation techniques.
The current logic for AMD SSBD context switches it on every
vm{entry,exit} if the Xen and guest selections don't match. This is
expensive when not using SPEC_CTRL, and hence should be avoided as
much as possible.
When SSBD is not being set from SPEC_CTRL on AMD don't context switch
at vm{entry,exit} and instead only context switch SSBD when switching
vCPUs. This has the side effect of running Xen code with the guest
selection of SSBD, the documentation is updated to note this behavior.
Also note that then when `ssbd` is selected on the command line guest
SSBD selection will not have an effect, and the hypervisor will run
with SSBD unconditionally enabled when not using SPEC_CTRL itself.
As a result of no longer running the code to set SSBD in a GIF=0
region the locking of amd_set_legacy_ssbd() can be done using normal
spinlocks, and some more checks can be added to assure it works as
intended.
Finally it's also worth noticing that since the guest SSBD selection
is no longer set on vmentry the VIRT_SPEC_MSR handling needs to
propagate the value to the hardware as part of handling the wrmsr.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Extend the msrs->virt_spec_ctrl context switching comment.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper
Edwin Török
Neowutran
Demi Marie Obenour
Per Bilse
Jan Beulich
Roger Pau Monné
Julien Grall
Henry Wang
Anthony PERARD
Julien Grall
Michal Orzel
Stefano Stabellini