Fix several MISRA Issues Rule 8.4 ("A compatible declaration shall be
visible when an object or function with external linkage is defined")
found by cppcheck affecting xen/xsm/flask.
- policydb_loaded_version is not declared; removed it because it is
unused
- move ss_initialized declaration to xen/xsm/flask/include/conditional.h
- #include <conditional.h> (which is
xen/xsm/flask/include/conditional.h) in xen/xsm/flask/ss/policydb.c so
that policydb.c also gets the declaration of ss_initialized
- #include <conditional.h> in xen/xsm/flask/ss/services.c to declare
security_*_bools functions that services.c is defining and using
This patch solves all the Rule 8.4 violations found by cppcheck on xsm/
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Michal Orzel [Thu, 1 Dec 2022 14:25:32 +0000 (14:25 +0000)]
automation: Add CI test jobs for Yocto
Populate test jobs for Yocto based tests using the provided containers.
Due to the size restrictions, it is currently not possible to split the
build and run tasks, therefore everything is done in a single step.
Test jobs for the supported Yocto targets are generic to avoid the
necessity to add new ones after each Yocto release. The only thing
required to be changed after updating the containers is the variable
YOCTO_VERSION stored in a .yocto-test template.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Add make clean support to remove the containers from the local docker
registry.
make clean-<image_name> must be called to remove an image:
make clean-yocto/kirkstone-qemuarm: remove yocto kirkstone for qemuarm
image
Add containers suitable to run yocto kirkstone build based on ubuntu
22.04. It contains all packages required by Yocto and a checkout of the
layers required to build Xen with Yocto.
Add a generic docker image template to be used to automatically generate
docker files for different configurations:
- specific yocto version
- different targets (qemu arm, arm64 and x86)
- different host platforms (x86 or arm64)
During a call to 'make all', only the images for the host platform we
run it on in the CI is generated.
If needed, images for an other host platform can be generated manually
by calling the right make target (see make help).
Add a build script to build and run xen on qemu using Yocto.
The script supports arm32, arm64 and x86_64 and checks that dom0 is
properly booting. At this stage this does not run any guest on top of
dom0. The script is to be executed in one of the docker images to build
and run a system using a Xen source tree.
Add automation/build/yocto/*.dockerfile to gitignore as those files are
generated.
Add SPDX license information to all the *.c files under arch/arm.
The SPDX tags added by this patch were chosen based on the existing
copyright headers. When the copyright header was missing we used the
default license which is GPLv2 for Xen (as per the COPYING file).
One exception is domain_page.c which is a split from mm.c so the license
is GPLv2+.
Michal Orzel [Thu, 17 Nov 2022 16:16:42 +0000 (17:16 +0100)]
automation: Install packages required by tests in containers
Installation of additional packages from the test scripts when running
the tests has some drawbacks. It is slower than cloning containers and can
fail due to some network issues (apparently it often happens on x86
rackspace). This patch is adding the packages required by the tests to be
installed when building the containers.
>From qemu-alpine-x86_64.sh into debian:stretch:
- cpio,
- busybox-static.
Andrew Cooper [Mon, 6 Dec 2021 13:54:38 +0000 (13:54 +0000)]
x86/boot: Drop pte_update_limit from physical relocation logic
This check has existed in one form or another since c/s 369bafdb1c1 "xen: Big
changes to x86 start-of-day" in 2007.
c/s 0d31d1680868 "x86/setup: do not relocate Xen over current Xen image
placement" demonstrates clearly that the logic was broken.
Without dissecting the myriad changes over the past 14 years, I'm pretty
certain Xen only booted by accident when l2_xenmap[0] was handled specially
and skipped the pte_update_limit check which would have left it corrupt.
The old logic was simply not safe, even if implemented as intended. TLB
entries can be lost for any reason; architectural (e.g. SMI), or uarch
(e.g. enough OoO execution to thrash the TLB). It is never safe to have
non-pagetable data in your live pagetables, for any period of time.
Either way, since c/s 0d31d1680868 there is not a partial overlap of the Xen
image, so drop the vestigial remnants.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Michal Orzel [Fri, 28 Oct 2022 12:49:37 +0000 (14:49 +0200)]
xen/arm: Do not route NS phys timer IRQ to Xen
At the moment, we route NS phys timer IRQ to Xen even though it does not
make use of this timer. Xen uses hypervisor timer for itself and the
physical timer is fully emulated, hence there is nothing that can trigger
such IRQ. This means that requesting/releasing IRQ ends up as a deadcode
as it has no impact on the functional behavior, whereas the code within
a handler ends up being unreachable. This is a left over from the early
days when the CNTHP IRQ was buggy on the HW model used for testing and we
had to use the CNTP instead.
Remove the calls to {request/release}_irq for this timer as well as the
code within the handler. Since timer_interrupt handler is now only used
by the CNTHP, refactor it as follows:
- rename it to htimer_interrupt to reflect its purpose,
- remove the IRQ affiliation test,
- invert the condition to avoid indented code and use unlikely,
- improve readability by adding new lines \btw code and comments.
Keep the calls to zero the CNTP_CTL_EL0 register for sanity and also
remove the corresponding perf counter definition.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Julien Grall <jgrall@amazon.com>
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.
Andrew Cooper [Tue, 26 Jul 2022 11:53:20 +0000 (12:53 +0100)]
x86/tboot: Drop mfn_in_guarded_stack()
To support CET Shadow Stacks, guard pages changed unilaterally from being
holes to being read-only (i.e. they're never holes any more).
Furthermore, being real shadow stacks in some cases, the frames explicitly
should be included in the integrity check.
Fixes: 60016604739b ("x86/shstk: Rework the stack layout to support shadow stacks") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Michal Orzel [Wed, 7 Dec 2022 11:18:36 +0000 (12:18 +0100)]
x86/platform: protect XENPF_get_dom0_console if CONFIG_VIDEO not set
A build failure [1] is observed if CONFIG_VGA (and thus CONFIG_VIDEO) is
not set. This is because XENPF_get_dom0_console cmd of platform hypercall
makes a call to fill_console_start_info, which is defined in video/vga.c
and built only if CONFIG_VGA is set.
To fix this issue, protect XENPF_get_dom0_console with CONFIG_VIDEO
ifdefery.
[1]:
ld: prelink.o: in function `do_platform_op':
(.text.do_platform_op+0x1a7): undefined reference to `fill_console_start_info'
Fixes: 4dd160583c79 ("x86/platform: introduce hypercall to get initial video console settings") Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Jan Beulich <jbeulich@suse.com>
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>
Michal Orzel [Wed, 7 Dec 2022 11:16:49 +0000 (12:16 +0100)]
xen: remove trigraphs from comments
MISRA C rule 4.2 states that trigraphs (sequences of two question marks
followed by a specified third character [=/'()!<>-]) should not be used.
This applies to both code and comments. Thankfully, we do not use them
in the code, but still there are some comments where they are
accidentally used. Fix it.
With regards to the comments and respective macros in pci_regs.h, these
were inherited from Linux. Let's knowingly accept the divergence.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Jan Beulich [Wed, 7 Dec 2022 11:16:08 +0000 (12:16 +0100)]
x86/HVM: drop stale check from hvm_load_cpu_msrs()
Up until f61685a66903 ("x86: remove defunct init/load/save_msr()
hvm_funcs") the check of the _rsvd field served as an error check for
the earlier hvm_funcs.save_msr() invocation. With that invocation gone
the check makes no sense anymore: It is effectively dead code due to the
checking of the field in the earlier loop.
While dropping the conditional also eliminate the "err" local variable
(using a non-standard name anyway), replaced by suitable new/adjusted
"return" statements.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
libxl: arm: make creation of iommu node independent of disk device
The iommu node will be required for other virtio device types too, not
just disk device.
Move the call to make_xen_iommu_node(), out of the disk device specific
block and rename "iommu_created" variable to "iommu_needed", and set it
to true for virtio disk device.
make_virtio_mmio_node() creates the DT node for simple MMIO devices
currently, i.e. the ones that don't require any additional properties.
In order to allow using it for other complex device types, split the
functionality into two, one where the fdt node isn't closed and the
other one to create a simple DT node.
In order to prepare for adding support for more device types, create a
separate routine to allocate base and irq for a device as the same code
will be required for other device types too.
Also move updates to virtio_irq and virtio_enabled out of the disk
device specific block, as they will depend on other device types too.
Michal Orzel [Fri, 18 Nov 2022 11:46:18 +0000 (12:46 +0100)]
xen/arm: Define WLEN_8 macro and use it in debug-pl011
At the moment, early printk code for pl011 uses a hardcoded value
for 8n1 LCR configuration. Define and use macro WLEN_8 for that purpose
(no parity and 1 stop bit are defined as 0).
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Julien Grall <julien@xen.org>
Michal Orzel [Fri, 18 Nov 2022 11:46:17 +0000 (12:46 +0100)]
xen/arm: debug-pl011.inc: Use macros instead of hardcoded values
Make use of the macros defined in asm/pl011-uart.h instead of hardcoding
the values. Also, take the opportunity to fix the file extension in a
top-level comment.
Michal Orzel [Fri, 18 Nov 2022 11:46:16 +0000 (12:46 +0100)]
xen/arm: Drop early_uart_init macro from debug-meson.inc
early_uart_init macro is only used if CONFIG_EARLY_UART_INIT is set.
This config option depends on EARLY_UART_PL011, so there is no point
in defining it for MESON, especially if it is empty. Remove it and adapt
the existing comment.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Julien Grall <julien@xen.org>
Michal Orzel [Wed, 23 Nov 2022 14:39:13 +0000 (15:39 +0100)]
xen/arm: Fix unreachable panic for vpl011 vIRQ
When creating direct mapped domU, the vIRQ for vpl011 is taken from
the SERHND_DTUART serial port using serial_irq. This function can return
-1 (i.e. no interrupt found) in which case we should call a panic.
However, vpl011_virq is defined as unsigned int which causes the panic
to be unreachable, because less than zero comparison of an unsigned value
can never be true. Fix it by defining vpl011_virq as int.
Fixes: 3580c8b2dfc3 ("xen/arm: if direct-map domain use native UART address and IRQ number for vPL011") Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Refer GIC v3 specification (Arm IHI 0069H ID020922), IGRPMODR is emulated
as RAZ / WI for the guests as "GICD_CTLR.ARE_S==0" is true.
Xen is currently supported to run in non-secure mode, so guests will run in
non-secure mode only.
Also, if Xen was supposed to run in secure mode with guests, the programming
of the interrupts (ie whether it belongs to secure/non secure and group 0/1)
will be done by Xen only. The guests will not be allowed to change this.
xen/arm: Enforce alignment check in debug build for {read, write}_atomic
Xen provides helper to atomically read/write memory (see {read,
write}_atomic()). Those helpers can only work if the address is aligned
to the size of the access (see B2.2.1 ARM DDI 08476I.a).
On Arm32, the alignment is already enforced by the processor because
HSCTLR.A bit is set (it enforce alignment for every access). For Arm64,
this bit is not set because memcpy()/memset() can use unaligned access
for performance reason (the implementation is taken from the Cortex
library).
To avoid any overhead in production build, the alignment will only be
checked using an ASSERT. Note that it might be possible to do it in
production build using the acquire/exclusive version of load/store. But
this is left to a follow-up (if wanted).
Roger Pau Monné [Tue, 6 Dec 2022 12:53:43 +0000 (13:53 +0100)]
x86/platform: introduce hypercall to get initial video console settings
This is required so PVH dom0 can get the initial video console state
as handled by Xen. PV dom0 will get this as part of the start_info,
but it doesn't seem necessary to place such information in the
HVM start info.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Tue, 6 Dec 2022 12:51:49 +0000 (13:51 +0100)]
x86/HVM: don't mark evtchn upcall vector as pending when vLAPIC is disabled
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: 7b5b8ca7dffd ("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>
xen/page_alloc: relax the BUILD_BUG_ON() in xenheap_max_mfn()
In the near future, we are considering to use a common xen/domain heap for
Arm 32-bit V8-R. In this setup, both PADDR_BITS and BITS_PER_LONG will be
32. Therefore, the condition PADDR_BITS >= BITS_PER_LONG will become true.
Looking at the commit that introduce the BUILD_BUG_ON (88e3ed61642b
"x86/NUMA: make init_node_heap() respect Xen heap limit") and the current
use, it seems this check was mainly to ensure that the shift of xenheap_bits
is not going to be undefined (>> N for a N-bit type is undefined).
So far, all the shifts are using "xenheap_bits - PAGE_SHIFT". Therefore, the
existing code should work for 32-bit architecture as long as the result of
the substraction is below 32.
Therefore relax the BUILD_BUG_ON() to check that (PADDR_BITS - PAGE_SHIFT)
is not equal of above BITS_PER_LONG.
Note that we would need further precaution if we ended up to use
'xenheap_bits' alone in shifts.
Jan Beulich [Mon, 5 Dec 2022 12:44:50 +0000 (13:44 +0100)]
gnttab: don't silently truncate GFNs in compat setup-table handling
Returning back truncated frame numbers is unhelpful: Quite likely
they're not owned by the domain (if it's PV), or we may misguide the
guest into writing grant entries into a page that it actually uses for
other purposes.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Mon, 5 Dec 2022 12:44:15 +0000 (13:44 +0100)]
x86/p2m: don't calculate page owner twice in p2m_add_page()
Neither page_get_owner() nor mfn_to_page() are entirely trivial
operations - don't do the same thing twice in close succession. Instead
help CSE (when MEM_SHARING=y) by introducing a local variable holding
the page owner.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Mon, 5 Dec 2022 12:40:43 +0000 (13:40 +0100)]
CODING_STYLE: list further brace placement exceptions
For easy spotting of struct/union/enum definitions we already commonly
place the opening braces on the initial line of such a definition.
We also often don't place the opening brace of an initializer on a
separate line.
And finally for compound literals placing the braces on separate lines
often makes the code more difficult to read, so it should (and in
practice does) typically go on the same line as well. The placement of
the closing brace often depends on how large such a compound literal is.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Jan Beulich [Mon, 5 Dec 2022 12:40:23 +0000 (13:40 +0100)]
CODING_STYLE: explicitly call out label indentation
Since the behavior of "diff -p" to use an unindented label as context
identifier often makes it harder to review patches, make explicit the
requirement for labels to be indented.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Jan Beulich [Mon, 5 Dec 2022 12:37:54 +0000 (13:37 +0100)]
unify update_runstate_area()
x86'es variant is a superset of Arm's, with CONFIG_COMPAT parts already
properly marked. The only other missing piece is
update_guest_memory_policy(): For the time being Arm simply gains an
empty struct and inline function.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Julien Grall <jgrall@amazon.com>
Jan Beulich [Tue, 29 Nov 2022 14:39:57 +0000 (15:39 +0100)]
Arm/P2M: reduce locking in p2m_{alloc,free}_page()
It is generally preferable to not hold locks around allocation
functions. And indeed in the hwdom case there's no point at all to hold
the paging lock. Reduce the locked regions to the non-hwdom case, while
at the same time arranging for p2m_alloc_page() to have just a single
return point.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Julien Grall <jgrall@amazon.com>
Jan Beulich [Tue, 29 Nov 2022 14:39:05 +0000 (15:39 +0100)]
Arm64: make setup_virt_paging()'s pa_range_info[] static
While not as inefficient as it would be on x86 (due to suitable constant
loading and register pair storing instructions being available to fill
some of the fields), having the compiler construct an array of constants
on the stack still looks odd to me.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <jgrall@amazon.com>
Not to be confused which hvm_get_param, which also exists and has a
different, more error-prone interface.
This one always returns a 64-bit value, and that is retained in the
OCaml binding as well, returning 'int64' (and not int, or nativeint
which might have different sizes).
Signed-off-by: Edwin Török <edvin.torok@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
Edwin Török [Fri, 2 Dec 2022 10:55:57 +0000 (10:55 +0000)]
tools/ocaml/xenctrl: Add binding for xc_evtchn_status
There is no API or ioctl to query event channel status, it is only
present in xenctrl.h
The C union is mapped to an OCaml variant exposing just the value from
the correct union tag. This causes the xc bindings to now depend on
evtchn to get a useful API for EVTCHNSTAT_virq.
The information provided here is similar to 'lsevtchn', but rather than
parsing its output it queries the underlying API directly.
Signed-off-by: Edwin Török <edvin.torok@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
Andrew Cooper [Thu, 1 Dec 2022 21:06:25 +0000 (21:06 +0000)]
tools/oxenstored: Render backtraces more nicely in Syslog
fallback_exception_handler feeds a string with embedded newlines directly into
syslog(). While this is an improvement on getting nothing, syslogd escapes
all control characters it gets, and emits one (long) log line.
Fix the problem generally in the syslog stub. As we already have a local copy
of the string, split it in place and emit one syslog() call per line.
Also tweak Logging.msg_of to avoid putting an extra newline on a string which
already ends with one.
Fixes: ee7815f49faf ("tools/oxenstored: Set uncaught exception handler") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
Jan Beulich [Fri, 2 Dec 2022 09:40:11 +0000 (10:40 +0100)]
x86/HVM+shim: fix build when !CONFIG_GRANT_TABLE
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: 8523851dbc49 ("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>
Jan Beulich [Fri, 2 Dec 2022 09:39:22 +0000 (10:39 +0100)]
x86/APIC: make a few interrupt handler functions static
Four of them are used in apic.c only and hence better wouldn't be
exposed to other CUs. To avoid the need for forward declarations, move
apic_intr_init() past the four handlers.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Fri, 2 Dec 2022 09:35:32 +0000 (10:35 +0100)]
x86/Viridian: don't mark IRQ vectors as pending when vLAPIC is disabled
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: fda96b7382ea ("viridian: add implementation of the HvSendSyntheticClusterIpi hypercall") Fixes: 26fba3c85571 ("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>
Jan Beulich [Fri, 2 Dec 2022 09:35:01 +0000 (10:35 +0100)]
x86/HVM: don't mark external IRQs as pending when vLAPIC is disabled
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: 4fe21ad3712e ("This patch add virtual IOAPIC support for VMX guest") Fixes: 85715f4bc7c9 ("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>
Roger Pau Monné [Fri, 2 Dec 2022 09:33:40 +0000 (10:33 +0100)]
x86/pvh: do not forward MADT Local APIC NMI structures to dom0
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>
Roger Pau Monné [Fri, 2 Dec 2022 09:32:21 +0000 (10:32 +0100)]
x86/irq: do not release irq until all cleanup is done
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>
Juergen Gross [Fri, 2 Dec 2022 09:28:33 +0000 (10:28 +0100)]
xen/gnttab: reduce size of struct active_grant_entry
The size of struct active_grant_entry for 64-bit builds is 40 or 48
bytes today (with or without NDEBUG).
It can easily be reduced by 8 bytes by replacing the trans_domain
pointer with the domid of the related domain. trans_domain is only ever
used for transitive grants, which doesn't have any known users.
This reduction will result in less memory usage and (for production
builds) in faster code, as indexing into the active_grant_entry array
will be much easier with an entry having a power-of-2 size.
The performance loss when using transitive grants shouldn't really
matter, given the probability that those aren't in use at all.
Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Juergen Gross [Fri, 2 Dec 2022 09:27:38 +0000 (10:27 +0100)]
xen: add knownalive_domain_from_domid() helper
Add a helper knownalive_domain_from_domid() returning the struct domain
pointer for a domain give by its domid and which is known not being
able to be released (its reference count isn't incremented and no
rcu_lock_domain() is called for it).
In order to simplify coding add an internal helper for doing the lookup
and call that from the new function and similar functions.
Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Juergen Gross [Fri, 2 Dec 2022 09:26:55 +0000 (10:26 +0100)]
xen/locks: add dynamic lock recursion checks
Add checking of lock recursion to the hypervisor. This is done by using
a percpu data array for storing the address of each taken lock. Any
attempt to take a lock twice (with the exception of recursive
spinlocks) will result in a crash. This is especially meant for
detecting attempts to take a rwlock multiple times as a reader, which
will only result in a deadlock in case of another cpu trying to get the
lock as a writer in between.
The additional checks are not performance neutral, so they are enabled
only in debug builds per default, as the checks are active only with
CONFIG_DEBUG_LOCKS enabled. The size of the percpu data array can be
selected via a boot parameter.
Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Fri, 2 Dec 2022 09:25:59 +0000 (10:25 +0100)]
gnttab: bail from GFN-storing loops early in case of error
The contents of the output arrays are undefined anyway when the
operation itself gets marked as failed. There's no value in trying to
continue after a guest memory access failure.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Fri, 2 Dec 2022 09:24:31 +0000 (10:24 +0100)]
gnttab: no need to translate handle for gnttab_get_status_frames()
Unlike for GNTTABOP_setup_table, native and compat frame lists for
GNTTABOP_get_status_frames are arrays of the same type (uint64_t). Hence
there's no need to translate the frame values. This then also renders
unnecessary the limit_max parameter of gnttab_get_status_frames().
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Michal Orzel [Tue, 25 Oct 2022 09:59:52 +0000 (11:59 +0200)]
automation: test.yaml: Introduce templates to reduce the overhead
At the moment, we define lots of test jobs in test.yaml, that make use
of the same configuration sections like variables, tags, artifacts.
Introduce templates (hidden jobs whose names start with a dot) to
reduce the overhead and simplify the file (more than 100 lines saved).
This way, the actual jobs can only specify sections that are unique
to them.
Most of the test jobs specify the same set of prerequisite jobs under
needs property with just one additional being unique to the job itself.
Introduce YAML anchors for that purpose, because when using extends, the
needs property is not being merged (the parent property overwrites the
child one).
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Michal Orzel [Tue, 27 Sep 2022 09:47:27 +0000 (11:47 +0200)]
automation: Rename qemu-smoke-arm32.sh to qemu-smoke-dom0-arm32.sh
After qemu arm64 test scripts had been renamed to reflect their
usage, do the same for the qemu arm32 test script. Currently it only
boots dom0, so we can assume that this script will be used to perform
dom0 based testing. In the future we will be able to create corresponding
script qemu-smoke-dom0less-arm32.sh to perform dom0less based testing.
This is the last step to get rid of ambiguity with regards to naming
of Arm test scripts.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Michal Orzel [Tue, 27 Sep 2022 09:47:26 +0000 (11:47 +0200)]
automation: Rename qemu-alpine-arm64.sh to qemu-smoke-dom0-arm64.sh
qemu-alpine-arm64.sh script is used to perform dom0 based testing.
Rename this script to qemu-smoke-dom0-arm64.sh to reflect its usage.
Also rename the corresponding test jobs.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Michal Orzel [Tue, 27 Sep 2022 09:47:25 +0000 (11:47 +0200)]
automation: Rename qemu-smoke-arm64.sh to qemu-smoke-dom0less-arm64.sh
Testing arm64 is done using the qemu-alpine-arm64.sh and
qemu-smoke-arm64.sh scripts. These scripts are executed with exactly
the same artifacts (container, rootfs, kernel, qemu) and the only
difference is that the former is used to perform dom0 based testing
and the latter - dom0less based testing.
Because the current naming is quite ambiguous, rename qemu-smoke-arm64.sh
script to qemu-smoke-dom0less-arm64.sh to reflect its usage.
qemu-alpine-arm64.sh will be renamed in the follow-up patch.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Michal Orzel [Tue, 27 Sep 2022 09:47:24 +0000 (11:47 +0200)]
automation: qemu-alpine-arm64: Cleanup and fixes
Perform the following cleanup:
- rename the device tree from virt-gicv3 to virt-gicv2 as the GIC version
used in this test is v2,
- use fdtput to perform modifications on the dtb,
- use DEBIAN_FRONTEND=noninteractive to prevent interactive prompt being
stuck waiting for answer other than "yes",
- fix the number of cpus in the device tree because currently we generate
it with a single cpu and try to run QEMU with two,
- fix the memory size we pass when generating QEMU device tree as it does
not match the memory size with what we run QEMU.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Michal Orzel [Tue, 27 Sep 2022 09:47:23 +0000 (11:47 +0200)]
automation: qemu-smoke-arm32.sh: Modify script to use ImageBuilder
Take an example from arm64 qemu test scripts and use ImageBuilder
to generate u-boot script automatically. Calculating the addresses
manually is quite error prone and also we will be able to benefit
from using ImageBuilder when adding domUs to this test in the future.
Install and use u-boot from the debian package.
Modify the script so that binaries are loaded from u-boot via tftp.
Remove dtuart=/pl011@9000000 because stdout-path in QEMU dtb is always
set to pl011 node path.
Michal Orzel [Tue, 27 Sep 2022 09:47:22 +0000 (11:47 +0200)]
automation: Add Arm containers to containerize script
Script automation/scripts/containerize makes it easy to build Xen within
predefined containers from gitlab container registry. This script is
currently missing the helpers to select Arm containers, so populate the
necessary entries.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Michal Orzel [Tue, 27 Sep 2022 09:47:21 +0000 (11:47 +0200)]
automation: Add debug versions of Arm tests
At the moment, all the tests are executed on non-debug Xen builds.
To improve the coverage (e.g. we might catch some asserts), add new
test jobs using debug Xen builds.
In the follow-up patch we will add new jobs using debug Xen builds.
Because the debug builds take more space and we might end up in
a situation when there is not enough free space (especially during
a static memory test that reserves some region in the middle), increase
RAM size for QEMU from 1GB to 2GB.
Michal Orzel [Tue, 27 Sep 2022 09:47:19 +0000 (11:47 +0200)]
automation: Add randconfig build jobs for arm64 alpine container
For arm64 we perform builds using debian and alpine containers.
We are missing the randconfig build jobs for the latter, so add them.
This way for each container we have 4 fundamental build jobs:
- defconfig non-debug/debug
- randconfig non-debug/debug
The binding for xc_interface_close() free the underlying handle while leaving
the Ocaml object still in scope and usable. This would make it easy to suffer
a use-after-free, if it weren't for the fact that 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 xc_interface_close(), if the Ocaml object goes out of scope.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
Edwin Török [Mon, 7 Nov 2022 17:41:36 +0000 (17:41 +0000)]
tools/oxenstored: Set uncaught exception handler
Unhandled exceptions go to stderr by default, but this doesn't typically work
for oxenstored because:
* daemonize reopens stderr as /dev/null
* systemd redirects stderr to /dev/null too
Debugging an unhandled exception requires reproducing the issue locally when
using --no-fork, and is not conducive to figuring out what went wrong on a
remote system.
Install a custom handler which also tries to render the backtrace to the
configured syslog facility, and DAEMON|ERR otherwise.
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>
Edwin Török [Tue, 8 Nov 2022 08:57:47 +0000 (08:57 +0000)]
tools/oxenstored: Log live update issues at warning level
During live update, oxenstored tries a best effort approach to recover as many
domains and information as possible even if it encounters errors restoring
some domains.
However, logging about misunderstood input is more severe than simply info.
Log it at warning instead.
Signed-off-by: Edwin Török <edvin.torok@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
Edwin Török [Thu, 3 Nov 2022 15:31:39 +0000 (15:31 +0000)]
tools/oxenstored: Keep /dev/xen/evtchn open across live update
Closing the evtchn handle will unbind and free all local ports. The new
xenstored would need to rebind all evtchns, which is work that we don't want
or need to be doing during the critical handover period.
However, it turns out that the Windows PV drivers also rebind their local port
too across suspend/resume, leaving (o)xenstored with a stale idea of the
remote port to use. In this case, reusing the established connection is the
only robust option.
Therefore:
* Have oxenstored open /dev/xen/evtchn without CLOEXEC at start of day.
* Extend the handover information with the evtchn fd, domexc virq local port,
and the local port number for each domain connection.
* Have (the new) oxenstored recover the open handle using Xeneventchn.fdopen,
and use the provided local ports rather than trying to rebind them.
When this new information isn't present (i.e. live updating from an oxenstored
prior to this change), the best-effort status quo will have to do.
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>
Andrew Cooper [Wed, 30 Nov 2022 11:59:34 +0000 (11:59 +0000)]
tools/oxenstored: Rework Domain evtchn handling to use port_pair
Inter-domain event channels are always a pair of local and remote ports.
Right now the handling is asymmetric, caused by the fact that the evtchn is
bound after the associated Domain object is constructed.
First, move binding of the event channel into the Domain.make() constructor.
This means the local port no longer needs to be an option. It also removes
the final callers of Domain.bind_interdomain.
Next, introduce a new port_pair type to encapsulate the fact that these two
should be updated together, and replace the previous port and remote_port
fields. This refactoring also changes the Domain.get_port interface (removing
an option) so take the opportunity to name it get_local_port instead.
Also, this fixes a use-after-free risk with Domain.close. Once the evtchn has
been unbound, the same local port number can be reused for a different
purpose, so explicitly invalidate the ports to prevent their accidental misuse
in the future.
This also cleans up some of the debugging, to always print a port pair.
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>
Andrew Cooper [Wed, 30 Nov 2022 11:55:58 +0000 (11:55 +0000)]
tools/oxenstored: Implement Domain.rebind_evtchn
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>
Andrew Cooper [Wed, 30 Nov 2022 03:17:28 +0000 (03:17 +0000)]
tools/oxenstored: Rename some 'port' variables to 'remote_port'
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>
Andrew Cooper [Tue, 29 Nov 2022 21:05:43 +0000 (21:05 +0000)]
tools/oxenstored: Bind the DOM_EXC VIRQ in in Event.init()
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>
Andrew Cooper [Wed, 30 Nov 2022 14:56:43 +0000 (14:56 +0000)]
tools/oxenstored: Style fixes to Domain
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>
Edwin Török [Thu, 3 Nov 2022 14:50:38 +0000 (14:50 +0000)]
tools/ocaml/evtchn: Extend the init() binding with a cloexec flag
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>
Edwin Török [Mon, 14 Nov 2022 13:36:19 +0000 (13:36 +0000)]
tools/ocaml/evtchn: Add binding for xenevtchn_fdopen()
For live update, the new oxenstored needs to reconstruct an evtchn object
around an existing file descriptor.
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>
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>
Andrew Cooper [Fri, 11 Nov 2022 18:50:34 +0000 (18:50 +0000)]
tools/oxenstored: Fix incorrect scope after an if statement
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: 42f0581a91d4 ("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>
Edwin Török [Tue, 1 Nov 2022 17:59:17 +0000 (17:59 +0000)]
tools/ocaml/xenctrl: Use larger chunksize in domain_getinfolist
domain_getinfolist() is quadratic with the number of domains, because of the
behaviour of the underlying hypercall. Nevertheless, getting domain info in
blocks of 1024 is far more efficient than blocks of 2.
In a scalability testing scenario with ~1000 VMs, a combination of this and
the previous change takes xenopsd's wallclock time in domain_getinfolist()
down from 88% to 0.02%
Signed-off-by: Edwin Török <edvin.torok@citrix.com> Tested-by: Pau Ruiz Safont <pau.safont@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
Edwin Török [Tue, 1 Nov 2022 17:59:16 +0000 (17:59 +0000)]
tools/ocaml/xenctrl: Make domain_getinfolist tail recursive
domain_getinfolist() is quadratic with the number of domains, because of the
behaviour of the underlying hypercall. xenopsd was further observed to be
wasting excessive quantites of time manipulating the list of already-obtained
domains.
Implement a tail recursive `rev_concat` equivalent to `concat |> rev`, and use
it instead of calling `@` multiple times.
An incidental benefit is that the list of domains will now be in domid order,
instead of having pairs of 2 domains changing direction every time.
In a scalability testing scenario with ~1000 VMs, a combination of this and
the subsequent change takes xenopsd's wallclock time in domain_getinfolist()
down from 88% to 0.02%
Signed-off-by: Edwin Török <edvin.torok@citrix.com> Tested-by: Pau Ruiz Safont <pau.safont@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
Anthony PERARD [Thu, 1 Dec 2022 15:14:59 +0000 (16:14 +0100)]
tools/include: fix clean and rework COPYING for installed Xen public header
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: 4ea75e9a9058 ("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>
Michal Orzel [Fri, 18 Nov 2022 11:45:54 +0000 (12:45 +0100)]
docs/misc/arm: Update references to Linux kernel docs
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>
Andrew Cooper [Thu, 24 Nov 2022 02:50:40 +0000 (10:50 +0800)]
Changelog: Add __ro_after_init and CET
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Henry Wang <Henry.Wang@arm.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Julien Grall <jgrall@amazon.com>
Andrew Cooper [Mon, 21 Nov 2022 12:46:39 +0000 (12:46 +0000)]
xen/flask: Wire up XEN_DOMCTL_{get,set}_paging_mempool_size
These were overlooked in the original patch, and noticed by OSSTest which does
run some Flask tests.
Fixes: 22b20bd98c02 ("xen: Introduce non-broken hypercalls for the paging mempool size") Suggested-by: Daniel Smith <dpsmith@apertussolutions.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jason Andryuk <jandryuk@gmail.com> Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Andrew Cooper [Fri, 18 Nov 2022 16:53:45 +0000 (16:53 +0000)]
tools/libxl: Fixes to libxl__domain_set_paging_mempool_size()
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: 7c3bbd940dd8 ("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 34990446ca91 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: 34990446ca91 ("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>
Roger Pau Monné [Mon, 21 Nov 2022 11:21:51 +0000 (12:21 +0100)]
efifb: ignore frame buffer with invalid configuration
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.
Andrew Cooper [Thu, 20 Oct 2022 11:14:30 +0000 (12:14 +0100)]
xen/arm: Correct the p2m pool size calculations
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: cbea5a1149ca ("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>
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>
Andrew Cooper [Thu, 20 Oct 2022 11:13:46 +0000 (12:13 +0100)]
tools/tests: Unit test for paging mempool size
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>
Andrew Cooper [Fri, 21 Oct 2022 13:13:00 +0000 (14:13 +0100)]
xen: Introduce non-broken hypercalls for the paging mempool size
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>
Andrew Cooper [Mon, 14 Nov 2022 21:47:59 +0000 (21:47 +0000)]
x86/hvm: Revert per-domain APIC acceleration support
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.
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: 2ce11ce249a3 ("x86/HVM: allow per-domain usage of hardware virtualized APIC") Fixes: 6b2b9b340509 ("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>