Luca Fancellu [Thu, 26 Oct 2023 10:32:43 +0000 (11:32 +0100)]
exclude-list: generalise exclude-list
Currently exclude-list.json is used by the xen-analysis tool to
remove from the report (cppcheck for now) violations from the
files listed in it, however that list can be used by different
users that might want to exclude some of the files from their
computation for many reasons.
So add a new field that can be part of each entry to link
the tool supposed to consider that exclusion.
Update exclusion_file_list.py to implement the logic and update
the documentation to reflect this change.
Luca Fancellu [Thu, 26 Oct 2023 10:32:42 +0000 (11:32 +0100)]
cppcheck: rework exclusion_file_list.py code
Rework the exclusion_file_list.py code to have the function
load_exclusion_file_list() detached from the xen-analysis.py tool,
in a way so that other modules can use the function.
The xen-analysis tool and in particular its module cppcheck_analysis.py
will use a new function cppcheck_exclusion_file_list().
Michal Orzel [Wed, 25 Oct 2023 08:28:34 +0000 (10:28 +0200)]
build: Allow setting KBUILD_DEFCONFIG in the environment
At the moment, in order to use a different defconfig target than default,
one needs to specify KBUILD_DEFCONFIG=<target> on the command line.
Switch to weak assignment, so that it can be also obtained from
environment similar to other KCONFIG/KBUILD variables.
This change will activate the use of KBUILD_DEFCONFIG variable in CI
build jobs that so far would have had no effect if they didn't match the
default anyway.
Note, that we will deviate from Linux in this regard.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
[stefano: adjust commit message] Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Julien Grall [Mon, 23 Oct 2023 17:52:20 +0000 (18:52 +0100)]
xen/arm64: domctl: Avoid unreachable code in subarch_do_domctl()
The 'break' the XEN_DOMCTL_set_address_size is unreachable and tools
like Eclair will report as a violation of Misra Rule 2.1.
Furthermore, the nested switch is not very easy to read. So move
out the nested switch in a separate function to improve the
readability and hopefully address the MISRA violation.
Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com> Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Henry Wang <Henry.Wang@arm.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
To avoid potentially dangerous sign conversions in bitmap_switch, all
the callers of the bitmap_switch macro (which are all within bitmap.h)
should pass an unsigned int as first parameter.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> Acked-by: Jan Beulich <jbeulich@suse.com>
automation/eclair: update accepted and clean guidelines
This patch updates accepted and clean guidelines selectors.
Align accepted selector with docs/misra/rules.txt.
Add rule 7.3 in the clean selector for ARM.
Avoid regular expressions in all service selectors: usign them
requires escaping dots, which make selectors hard to read.
Michal Orzel [Tue, 7 Nov 2023 09:14:17 +0000 (10:14 +0100)]
ubsan: Fix pointer overflow error message
In __ubsan_handle_pointer_overflow(), fix the condition for determining
whether a pointer operation overflowed or underflowed. Currently, the
function reports "underflowed" when it should be reporting "overflowed"
and vice versa.
Example of incorrect error reporting:
void *foo = (void *)__UINTPTR_MAX__;
foo += 1;
Fixes: 4e3fb2fb47d6 ("ubsan: add clang 5.0 support") Signed-off-by: Michal Orzel <michal.orzel@amd.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Roger Pau Monne [Mon, 6 Nov 2023 14:27:39 +0000 (15:27 +0100)]
x86/x2apic: introduce a mixed physical/cluster mode
The current implementation of x2APIC requires to either use Cluster Logical or
Physical mode for all interrupts. However the selection of Physical vs Logical
is not done at APIC setup, an APIC can be addressed both in Physical or Logical
destination modes concurrently.
Introduce a new x2APIC mode called Mixed, which uses Logical Cluster mode for
IPIs, and Physical mode for external interrupts, thus attempting to use the
best method for each interrupt type.
Using Physical mode for external interrupts allows more vectors to be used, and
interrupt balancing to be more accurate.
Using Logical Cluster mode for IPIs allows fewer accesses to the ICR register
when sending those, as multiple CPUs can be targeted with a single ICR register
write.
A simple test calling flush_tlb_all() 10000 times on a tight loop on AMD EPYC
9754 with 512 CPUs gives the following figures in nano seconds:
So Mixed has no difference when compared to Cluster mode, and Physical mode is
248% slower when compared to either Mixed or Cluster modes with a 95%
confidence.
Note that Xen uses Cluster mode by default, and hence is already using the
fastest way for IPI delivery at the cost of reducing the amount of vectors
available system-wide.
Make the newly introduced mode the default one.
Note the printing of the APIC addressing mode done in connect_bsp_APIC() has
been removed, as with the newly introduced mixed mode this would require more
fine grained printing, or else would be incorrect. The addressing mode can
already be derived from the APIC driver in use, which is printed by different
helpers.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Acked-by: Henry Wang <Henry.Wang@arm.com>
Jason Andryuk [Fri, 3 Nov 2023 19:45:51 +0000 (15:45 -0400)]
golang: Fix bindings after XSA-443
The new bootloader_restrict and bootloader_user fields in the libxl idl
change the bindings. Update them.
Fixes: 1f762642d2ca ("libxl: add support for running bootloader in restricted mode") Signed-off-by: Jason Andryuk <jandryuk@gmail.com> Acked-by: George Dunlap <george.dunlap@cloud.com>
Jason Andryuk [Fri, 3 Nov 2023 19:45:50 +0000 (15:45 -0400)]
golang: Fixup binding for Arm FF-A
The new FF-A TEE type changed the go bindings. Update them.
Fixes: 8abdd8d52862 ("tools: add Arm FF-A mediator") Signed-off-by: Jason Andryuk <jandryuk@gmail.com> Acked-by: George Dunlap <george.dunlap@cloud.com>
Jan Beulich [Fri, 3 Nov 2023 10:28:09 +0000 (11:28 +0100)]
AMD/IOMMU: drop tasklet handler forward declaration
do_amd_iommu_irq()'s forward declaration uses a parameter name different
from what the definition uses, thus vioating Misra C:2012 rule 8.3. We
can get away without such a forward declaration if instead we forward-
declare amd_iommu_irq_tasklet, putting its initialization past the
handler function's definition.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
x86/hvm/dom0: fix PVH initrd and metadata placement
Zephyr image consists of multiple non-contiguous load segments
that reside in different RAM regions. For instance:
ELF: phdr: paddr=0x1000 memsz=0x8000
ELF: phdr: paddr=0x100000 memsz=0x28a90
ELF: phdr: paddr=0x128aa0 memsz=0x7560
ELF: memory: 0x1000 -> 0x130000
However, the logic that determines the best placement for dom0
initrd and metadata, assumes that the image is fully contained
in a single RAM region, not taking into account the cases where:
(1) start > kernel_start && end > kernel_end
(2) start < kernel_start && end < kernel_end
(3) start > kernel_start && end < kernel_end
In case (1), the evaluation will result in end = kernel_start,
i.e. end < start, and will load initrd in the middle of the kernel.
In case (2), the evaluation will result in start = kernel_end,
i.e. end < start, and will load initrd at kernel_end, that is out
of the memory region under evaluation.
In case (3), the evaluation will result in either end = kernel_start
or start = kernel_end but in both cases will be end < start, and
will either load initrd in the middle of the image, or arbitrarily
at kernel_end.
This patch reorganizes the conditionals to include so far unconsidered
cases as well, uniformly returning the lowest available address.
Fixes: 73b47eea2104 ('x86/dom0: improve PVH initrd and metadata placement') Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Fri, 3 Nov 2023 10:25:03 +0000 (11:25 +0100)]
xenoprof: move asm/ include
There's no need for xen/xenoprof.h to include the arch header when the
respective config option is off. The only compensation needed is for
svm.c to explicitly include the arch header instead of the common one
(matching what vmx.c and traps.c do).
With that Arm's header can be deleted, and neither RISC-V nor PPC will
need to introduce one.
While there also adjust and move the (prior) inclusion of inttypes.h.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> # arm
Jan Beulich [Fri, 3 Nov 2023 10:23:18 +0000 (11:23 +0100)]
console: conditionalize console_input_domain()
The function was introduced for just one piece of code to use; it is
dead code in cases where the respective Kconfig option is disabled, thus
violating Misra C:2012 rule 2.1 there.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Juergen Gross [Fri, 3 Nov 2023 10:21:28 +0000 (11:21 +0100)]
build: simplify clean handling of extras directory
The extras directory is used only as a download target for Mini-OS
sources. Instead of special handling extras/mini-os* in .gitignore and
the clean targets, just use extras for that purpose.
So add "extras" to .gitignore and remove it when doing a
"make distclean".
Nicola Vetrini [Wed, 11 Oct 2023 13:03:49 +0000 (15:03 +0200)]
xen/include: make enum perfcounter anonymous
Using enumerators declared in a named enum, such as the one modified,
as operands to arithmetic operators is not allowed by MISRA C:2012 Rule 10.1.
The enumerators of an anonymous enum can be used instead.
Andrew Cooper [Wed, 1 Nov 2023 20:19:52 +0000 (20:19 +0000)]
x86/time: Fix UBSAN failure in __update_vcpu_system_time()
As reported:
(XEN) ================================================================================
(XEN) UBSAN: Undefined behaviour in arch/x86/time.c:1542:32
(XEN) member access within null pointer of type 'union vcpu_info_t'
(XEN) ----[ Xen-4.19-unstable x86_64 debug=y ubsan=y Not tainted ]----
...
(XEN) Xen call trace:
(XEN) [<ffff82d040345036>] R common/ubsan/ubsan.c#ubsan_epilogue+0xa/0xd2
(XEN) [<ffff82d0403456e8>] F __ubsan_handle_type_mismatch+0x133/0x49b
(XEN) [<ffff82d040345b4a>] F __ubsan_handle_type_mismatch_v1+0xfa/0xfc
(XEN) [<ffff82d040623356>] F arch/x86/time.c#__update_vcpu_system_time+0x212/0x30f
(XEN) [<ffff82d040623461>] F update_vcpu_system_time+0xe/0x10
(XEN) [<ffff82d04062389d>] F arch/x86/time.c#local_time_calibration+0x1f7/0x523
(XEN) [<ffff82d0402a64b5>] F common/softirq.c#__do_softirq+0x1f4/0x31a
(XEN) [<ffff82d0402a67ad>] F do_softirq+0x13/0x15
(XEN) [<ffff82d0405a95dc>] F arch/x86/domain.c#idle_loop+0x2e0/0x367
(XEN)
(XEN) ================================================================================
GCC 10 at least doesn't consider it valid to derive a pointer from vcpu_info()
prior to checking that the underlying map pointer is good.
Reorder actions so the map pointer is checked first.
Fixes: 20279afd7323 ("x86: split populating of struct vcpu_time_info into a separate function") 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>
Shawn Anastasio [Thu, 2 Nov 2023 09:52:31 +0000 (10:52 +0100)]
xen/ppc: Implement a basic exception handler
Implement a basic exception handler that dumps the CPU state to the
console, as well as the code required to set the correct exception
vector table's base address in setup.c.
Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Shawn Anastasio [Thu, 2 Nov 2023 09:51:31 +0000 (10:51 +0100)]
xen/ppc: Add .text.exceptions section for exception vectors
On Power, the exception vectors must lie at a fixed address, depending
on the state of the Alternate Interrupt Location (AIL) field of the
Logical Partition Control Register (LPCR). Create a .text.exceptions
section in the linker script at an address suitable for AIL=3 plus an
accompanying assertion to pave the way for implementing exception
support.
Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Roger Pau Monné [Thu, 2 Nov 2023 09:50:59 +0000 (10:50 +0100)]
x86/i8259: do not assume interrupts always target CPU0
Sporadically we have seen the following during AP bringup on AMD platforms
only:
microcode: CPU59 updated from revision 0x830107a to 0x830107a, date = 2023-05-17
microcode: CPU60 updated from revision 0x830104d to 0x830107a, date = 2023-05-17
CPU60: No irq handler for vector 27 (IRQ -2147483648)
microcode: CPU61 updated from revision 0x830107a to 0x830107a, date = 2023-05-17
This is similar to the issue raised on Linux commit 36e9e1eab777e, where they
observed i8259 (active) vectors getting delivered to CPUs different than 0.
On AMD or Hygon platforms adjust the target CPU mask of i8259 interrupt
descriptors to contain all possible CPUs, so that APs will reserve the vector
at startup if any legacy IRQ is still delivered through the i8259. Note that
if the IO-APIC takes over those interrupt descriptors the CPU mask will be
reset.
Spurious i8259 interrupt vectors however (IRQ7 and IRQ15) can be injected even
when all i8259 pins are masked, and hence would need to be handled on all CPUs.
Continue to reserve PIC vectors on CPU0 only, but do check for such spurious
interrupts on all CPUs if the vendor is AMD or Hygon. Note that once the
vectors get used by devices detecting PIC spurious interrupts will no longer be
possible, however the device driver should be able to cope with spurious
interrupts. Such PIC spurious interrupts occurring when the vector is in use
by a local APIC routed source will lead to an extra EOI, which might
unintentionally clear a different vector from ISR. Note this is already the
current behavior, so assume it's infrequent enough to not cause real issues.
Finally, adjust the printed message to display the CPU where the spurious
interrupt has been received, so it looks like:
microcode: CPU1 updated from revision 0x830107a to 0x830107a, date = 2023-05-17
cpu1: spurious 8259A interrupt: IRQ7
microcode: CPU2 updated from revision 0x830104d to 0x830107a, date = 2023-05-17
Amends: 3fba06ba9f8b ('x86/IRQ: re-use legacy vector ranges on APs') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Roger Pau Monné [Thu, 2 Nov 2023 09:50:26 +0000 (10:50 +0100)]
x86/x2apic: remove usage of ACPI_FADT_APIC_CLUSTER
The ACPI FADT APIC_CLUSTER flag mandates that when the interrupt delivery is
Logical mode APIC must be configured for Cluster destination model. However in
apic_x2apic_probe() such flag is incorrectly used to gate whether Physical mode
can be used.
Since Xen when in x2APIC mode only uses Logical mode together with Cluster
model completely remove checking for ACPI_FADT_APIC_CLUSTER, as Xen always
fulfills the requirement signaled by the flag.
Fixes: eb40ae41b658 ('x86/Kconfig: add option for default x2APIC destination mode') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
The way how switch to virtual address was implemented in the
commit e66003e7be ("xen/riscv: introduce setup_initial_pages")
isn't safe enough as:
* enable_mmu() depends on hooking all exceptions
and pagefault.
* Any exception other than pagefault, or not taking a pagefault
causes it to malfunction, which means you will fail to boot
depending on where Xen was loaded into memory.
Instead of the proposed way of switching to virtual addresses was
decided to use identity mapping for area which constains needed code
to switch from identity mapping and after switching to virtual addresses,
identity mapping is removed from page-tables in the following way:
search for top-most page table entry and remove it.
Fixes: e66003e7be ("xen/riscv: introduce setup_initial_pages") Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Acked-by: Jan Beulich <jbeulich@suse.com>
xen/riscv: introduce function for physical offset calculation
The function was introduced to calculate and save physical
offset before MMU is enabled because access to start() is
PC-relative and in case of linker_addr != load_addr it will
result in incorrect value in phys_offset.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Thu, 2 Nov 2023 09:45:39 +0000 (10:45 +0100)]
x86: don't allow Dom0 (direct) access to port F0
This controls the driving of IGNNE# (if such emulation is enabled in
hardware), and hence would need proper handling in the hypervisor to be
safe to use by Dom0 (and fully emulating for PVH/HVM DomU-s).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Thu, 2 Nov 2023 09:44:46 +0000 (10:44 +0100)]
x86/PVH: deny Dom0 access to the ISA DMA controller
Unlike PV, a PVH Dom0 has no sensible way of driving the address and
page registers correctly, as it would need to translate guest physical
addresses to host ones. Rather than allowing data corruption to occur
from e.g. the use of a legacy floppy drive, disallow access altogether.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Thu, 2 Nov 2023 09:44:11 +0000 (10:44 +0100)]
x86: don't allow Dom0 access to port 92
Somewhat like port CF9 this may have a bit controlling the CPU's INIT#
signal, and it also may have a bit involved in the driving of A20M#.
Neither of these - just like CF9 - we want to allow Dom0 to drive.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich [Thu, 2 Nov 2023 09:42:02 +0000 (10:42 +0100)]
x86/vRTC: minor adjustment to reads from index port
Whether to handle this shouldn't depend on the present value of the
index register. Since the handling is done outside of the lock anyway,
pull it out into the sole caller and drop the no longer needed function
parameter.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
David Woodhouse [Mon, 23 Oct 2023 11:34:24 +0000 (12:34 +0100)]
x86/pv-shim: fix grant table operations for 32-bit guests
When switching to call the shim functions from the normal handlers, the
compat_grant_table_op() function was omitted, leaving it calling the
real grant table operations in !PV_SHIM_EXCLUSIVE builds. This leaves a
32-bit shim guest failing to set up its real grant table with the parent
hypervisor.
Fixes: e7db635f4428 ("x86/pv-shim: Don't modify the hypercall table") Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tamas K Lengyel [Wed, 18 Oct 2023 08:02:42 +0000 (04:02 -0400)]
x86/mem_sharing: add missing m2p entry when mapping shared_info page
When mapping in the shared_info page to a fork the m2p entry wasn't set
resulting in the shared_info being reset even when the fork reset was called
with only reset_state and not reset_memory. This results in an extra
unnecessary TLB flush.
Fixes: 1a0000ac775 ("mem_sharing: map shared_info page to same gfn during fork") Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper [Tue, 10 Oct 2023 09:52:53 +0000 (10:52 +0100)]
x86/cpu-policy: Adjust CPUID_MAX_SERIALISED_LEAVES to placate MISRA
MISRA doesn't like !!CONST being used in place of a 1 (Rule 10.1). Update the
expression to just be a plain 1, which still matches the description.
No functional change.
Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Nicola Vetrini [Fri, 6 Oct 2023 08:26:10 +0000 (10:26 +0200)]
x86/mce: Move MC_NCLASSES into the enum mctelem_class
The definition of MC_NCLASSES contained a violation of MISRA C:2012
Rule 10.1, therefore by moving it as an enumeration constant resolves the
violation and makes it more resilient to possible additions to that enum.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper [Tue, 31 Oct 2023 12:02:15 +0000 (12:02 +0000)]
docs: Fix IOMMU command line docs some more
Make the command line docs match the actual implementation, and state that the
default behaviour is selected at compile time.
Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices optional") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
automation: fix race condition in adl-suspend test
If system suspends too quickly, the message for the test controller to
wake up the system may be not sent to the console before suspending.
This will cause the test to timeout.
Fix this by calling sync on the console and waiting a bit after printing
the message. The test controller then resumes the system 30s after the
message, so as long as the delay + suspending takes less time it is
okay.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Julien Grall [Tue, 24 Oct 2023 10:28:58 +0000 (11:28 +0100)]
docs/arm: Document where Xen should be loaded in memory
In commit 9d267c049d92 ("xen/arm64: Rework the memory layout"),
we decided to require Xen to be loaded below 2 TiB to simplify
the logic to enable the MMU. The limit was decided based on
how known platform boot plus some slack.
We had a recent report that this is not sufficient on the AVA
platform with a old firmware [1]. But the restriction is not
going to change in Xen 4.18. So document the limit clearly
in docs/misc/arm/booting.txt.
Henry Wang [Mon, 23 Oct 2023 09:21:21 +0000 (17:21 +0800)]
CHANGELOG.md: Use "xenbits.xenproject.org" in links
Compared to "xenbits.xen.org", "xenbits.xenproject.org" appeared
later as a name, with the intention of becoming the canonical one.
Therefore, this commit unifies all the links to use "xenproject"
in the links.
Signed-off-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Fri, 20 Oct 2023 13:50:05 +0000 (15:50 +0200)]
x86: support data operand independent timing mode
[1] specifies a long list of instructions which are intended to exhibit
timing behavior independent of the data they operate on. On certain
hardware this independence is optional, controlled by a bit in a new
MSR. Provide a command line option to control the mode Xen and its
guests are to operate in, with a build time control over the default.
Longer term we may want to allow guests to control this.
Since Arm64 supposedly also has such a control, put command line option
and Kconfig control in common files.
Requested-by: Demi Marie Obenour <demi@invisiblethingslab.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Andrew Cooper [Thu, 19 Oct 2023 13:56:26 +0000 (14:56 +0100)]
CI: (More) Always pull base image when building a container
Repeat c/s 26ecc08b98fc ("automation: Always pull base image when building a
container") for the other makefile we've got building containers.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Roger Pau Monne [Wed, 18 Oct 2023 16:07:33 +0000 (18:07 +0200)]
iommu/vt-d: fix SAGAW capability parsing
SAGAW is a bitmap field, with bits 1, 2 and 3 signaling support for 3, 4 and 5
level page tables respectively. According to the Intel VT-d specification, an
IOMMU can report multiple SAGAW bits being set.
Commit 859d11b27912 claims to replace the open-coded find_first_set_bit(), but
it's actually replacing an open coded implementation to find the last set bit.
The change forces the used AGAW to the lowest supported by the IOMMU instead of
the highest one between 1 and 2.
Restore the previous SAGAW parsing by using fls() instead of
find_first_set_bit(), in order to get the highest (supported) AGAW to be used.
However there's a caveat related to the value the AW context entry field must
be set to when using passthrough mode:
"When the Translation-type (TT) field indicates pass-through processing (10b),
this field must be programmed to indicate the largest AGAW value supported by
hardware." [0]
Newer Intel IOMMU implementations support 5 level page tables for the IOMMU,
and signal such support in SAGAW bit 3.
Enabling 5 level paging support (AGAW 3) is too risky at this point in the Xen
4.18 release, so instead put a bodge to unconditionally disable passthough
mode if SAGAW has any bits greater than 2 set. Ignore bit 0; it's reserved in
current specifications, but had a meaning in the past and is unlikely to be
reused in the future.
Note the message about unhandled SAGAW bits being set is printed
unconditionally, regardless of whether passthrough mode is enabled. This is
done in order to easily notice IOMMU implementations with not yet supported
SAGAW values.
[0] Intel VT Directed Spec Rev 4.1
Fixes: 859d11b27912 ('VT-d: prune SAGAW recognition') 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>
Roger Pau Monne [Thu, 19 Oct 2023 10:45:51 +0000 (12:45 +0200)]
iommu: fix quarantine mode command line documentation
With the addition of per-device quarantine page tables the sink page is now
exclusive for each device, and thus writable. Update the documentation to
reflect the current implementation.
Fixes: 14dd241aad8a ('IOMMU/x86: use per-device page tables for quarantining') 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>
automation: extract QEMU log in relevant hardware tests
Let it be printed to the console too. QEMU and Linux messages have
different enough format that it should be possible to distinguish them.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
automation: improve checking for MSI/MSI-X in PCI passthrough tests
Checking /proc/interrupts is unreliable because different drivers set
different names there. Install pciutils and use lspci instead.
In fact, the /proc/interrupts content was confusing enough that
adl-pci-hvm had it wrong (MSI-X is in use there). Fix this too.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Remove parts of initramfs for the test system (domU, and in few tests
dom0 too) that are not not working and are not really needed in this
simple system.
This makes the test log much lighter on misleading error messages.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
grep+sleep message every 1s makes job log unnecessary hard to read.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
automation: include real-time view of the domU console log too
Passthrough domU console log to the serial console in real time, not
only after the test. First of all, this gives domU console also in case
of test failure. But also, allows correlation between domU and dom0 or
Xen messages.
To avoid ambiguity, add log prefix with 'sed'.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Manuel Bouyer [Thu, 19 Oct 2023 07:54:50 +0000 (09:54 +0200)]
console: make input work again for pv-shim
The use of rcu_lock_domain_by_id() right in switch_serial_input() makes
assumptions about domain IDs which don't hold when in shim mode: The
sole (initial) domain there has a non-zero ID. Obtain the real domain ID
in that case (generalized as get_initial_domain_id() returns zero when
not in shim mode).
Note that console_input_domain() isn't altered, for not being used when
in shim mode (or more generally on x86).
Fixes: c2581c58bec9 ("xen/console: skip switching serial input to non existing domains") Signed-off-by: Manuel Bouyer <bouyer@antioche.eu.org> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Julien Grall <jgrall@amazon.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Roger Pau Monné [Thu, 19 Oct 2023 07:52:43 +0000 (09:52 +0200)]
x86/pvh: fix identity mapping of low 1MB
The mapping of memory regions below the 1MB mark was all done by the PVH dom0
builder code, causing the region to be avoided by the arch specific IOMMU
hardware domain initialization code. That lead to the IOMMU being enabled
without reserved regions in the low 1MB identity mapped in the p2m for PVH
hardware domains. Firmware which happens to be missing RMRR/IVMD ranges
describing E820 reserved regions in the low 1MB would transiently trigger IOMMU
faults until the p2m is populated by the PVH dom0 builder:
Those errors have been observed on the osstest pinot{0,1} boxes (AMD Fam15h
Opteron(tm) Processor 3350 HE).
Rely on the IOMMU arch init code to create any identity mappings for reserved
regions in the low 1MB range (like it already does for reserved regions
elsewhere), and leave the mapping of any holes to be performed by the dom0
builder code.
Fixes: 6b4f6a31ace1 ('x86/PVH: de-duplicate mappings for first Mb of Dom0 memory') 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>
x86/microcode: Disable microcode update handler if DIS_MCU_UPDATE is set
If IA32_MSR_MCU_CONTROL exists then it's possible a CPU may be unable to
perform microcode updates. This is controlled through the DIS_MCU_LOAD bit
and is intended for baremetal clouds where the owner may not trust the
tenant to choose the microcode version in use. If we notice that bit being
set then simply disable the "apply_microcode" handler so we can't even try
to perform update (as it's known to be silently dropped).
While at it, remove the Intel family check, as microcode loading is
supported on every Intel64 CPU.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
x86: Read MSR_ARCH_CAPS immediately after early_microcode_init()
Move MSR_ARCH_CAPS read code from tsx_init() to early_cpu_init(). Because
microcode updates might make them that MSR to appear/have different values
we also must reload it after a microcode update in early_microcode_init().
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
x86/microcode: Ignore microcode loading interface for revision = -1
Some hypervisors report ~0 as the microcode revision to mean "don't issue
microcode updates". Ignore the microcode loading interface in that case.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
x86/microcode: WARN->INFO for the "no ucode loading" log message
Currently there's a printk statement triggered when no ucode loading
facilities are discovered. This statement should have severity INFO rather
than WARNING because it's not reporting anything wrong. Warnings ought
to be reserved for recoverable system errors.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> Acked-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
tools/pygrub: Fix pygrub's --entry flag for python3
string.atoi() has been deprecated since Python 2.0, has a big scary warning
in the python2.7 docs and is absent from python3 altogether. int() does the
same thing and is compatible with both.
See https://docs.python.org/2/library/string.html#string.atoi:
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
This erratum has been observed to cause #UD exceptions.
Fix adapted off Linux's mailing list:
https://lore.kernel.org/lkml/D99589F4-BC5D-430B-87B2-72C20370CF57@exactcode.com/T/#u
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
xen/pdx: Make CONFIG_PDX_COMPRESSION a common Kconfig option
Adds a new compile-time flag to allow disabling PDX compression and
compiles out compression-related code/data. It also shorts the pdx<->pfn
conversion macros and creates stubs for masking functions.
While at it, removes the old arch-defined CONFIG_HAS_PDX flag. Despite the
illusion of choice, it was not optional.
There are ARM and PPC platforms with sparse RAM banks - leave compression
active by default there. However, there are no known production x86 systems
with sparse RAM banks, so disable compression. RISC-V platforms are unknown
right now. These decisions can be revisited if our understanding changes.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> Reviewed-by: Julien Grall <jgrall@amazon.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Michal Orzel [Mon, 16 Oct 2023 12:45:59 +0000 (14:45 +0200)]
xen/arm: Check return code from recursive calls to scan_pfdt_node()
At the moment, we do not check a return code from scan_pfdt_node()
called recursively. This means that any issue that may occur while
parsing and copying the passthrough nodes is hidden and Xen continues
to boot a domain despite errors. This may lead to incorrect device tree
generation and various guest issues (e.g. trap on attempt to access MMIO
not mapped in P2M). Fix it.
Fixes: 669ecdf8d6cd ("xen/arm: copy dtb fragment to guest dtb") Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
George Dunlap [Fri, 13 Oct 2023 23:06:24 +0000 (16:06 -0700)]
cxenstored: wait until after reset to notify dom0less domains
Commit fc2b57c9a ("xenstored: send an evtchn notification on
introduce_domain") introduced the sending of an event channel to the
guest when first introduced, so that dom0less domains waiting for the
connection would know that xenstore was ready to use.
Unfortunately, it was introduced in introduce_domain(), which 1) is
called by other functions, where such functionality is unneeded, and
2) after the main XS_INTRODUCE call, calls domain_conn_reset(). This
introduces a race condition, whereby if xenstored is delayed, a domain
can wake up, send messages to the buffer, only to have them deleted by
xenstore before finishing its processing of the XS_INTRODUCE message.
Move the connect-and-notfy call into do_introduce() instead, after the
domain_conn_rest(); predicated on the state being in the
XENSTORE_RECONNECT state.
(We don't need to check for "restoring", since that value is always
passed as "false" from do_domain_introduce()).
Also take the opportunity to add a missing wmb barrier after resetting
the indexes of the ring in domain_conn_reset.
This change will also remove an extra event channel notification for
dom0 (because the notification is now done by do_introduce which is not
called for dom0.) The extra dom0 event channel notification was only
introduced by fc2b57c9a and was never present before. It is not needed
because dom0 is the one to tell xenstored the connection parameters, so
dom0 has to know that the ring page is setup correctly by the time
xenstored starts looking at it. It is dom0 that performs the ring page
init.
Signed-off-by: George Dunlap <george.dunlap@cloud.com> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> Reviewed-by: Juergen Gross <jgross@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> CC: jgross@suse.com CC: julien@xen.org CC: wl@xen.org
Anthony PERARD [Tue, 17 Oct 2023 07:53:34 +0000 (09:53 +0200)]
get_maintainer: Add THE REST for sections with reviewers only
Sometime, a contributer would like to be CCed on part of the changes,
and it could happen that we end-up with a section that doesn't have
any maintainer, but a Ack from a maintainer would still be needed.
Rework get_maintainer so if there's no maintainers beside THE REST, it
doesn't drop THE REST emails.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Julien Grall <jgrall@amazon.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
x86/mem_access: address violations of MISRA C:2012 Rule 8.3
Make function declarations and definitions consistent.
No functional change.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> Acked-by: Tamas K Lengyel <tamas@tklengyel.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
George Dunlap [Mon, 9 Oct 2023 10:19:57 +0000 (11:19 +0100)]
xenalyze: Reduce warnings about leaving a vcpu in INIT
We warn when we see data for a vcpu moving into a non-RUNNING state,
just so that people know why we're ignoring it. On full traces, this
happens only once. However, if the trace was limited to a subset of
pcpus, then this will happen every time the domain in question is
woken on that pcpu.
Add a 'delayed_init' flag to the vcpu struct to indicate when a vcpu
has experienced a delayed init. Print a warning message once when
entering the state, and once when leaving it.
Signed-off-by: George Dunlap <george.dunlap@cloud.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
George Dunlap [Fri, 6 Oct 2023 15:54:10 +0000 (16:54 +0100)]
xenalyze: Fix interrupt EIP reporting
EIP lists are generalized across several use cases. For many of them,
it make sense to have a cycle per sample; but not really for interrupt
EIP lists. For this reason, it normally just passes 0 as for the tsc
value, which will in turn down at the bottom of update_cycles(),
update only the summary.event_count, but nothing else.
The dump_eip() function attempted to handle this by calling the generic
cycle print handler if the summary contained *any* cycles, and by collecting
and printing its own stats, based solely on counts, if not.
Unfortunately, it used the wrong element for this: it collected the
total from samples.count rather samples.event_count; in the case that
there are no cycles, this will always be zero. It then divided by
this zero value. This results in output that looked like this:
It's better than nothing, but a lot less informative than one would
like.
Use event_count rather than count for collecting the total, and the
reporting when there are no cycles in the summary information. This results
in output that looks like this:
Signed-off-by: George Dunlap <george.dunlap@cloud.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
George Dunlap [Fri, 6 Oct 2023 15:22:34 +0000 (16:22 +0100)]
xenalyze: Don't expect an HVM_HANDLER trace for PAUSE vmexits
Neither vmx nor svm trace anything, nor is there anything obvious
worth tracing.
Signed-off-by: George Dunlap <george.dunlap@cloud.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
George Dunlap [Thu, 5 Oct 2023 16:26:38 +0000 (17:26 +0100)]
xenalyze: AMD's VMEXIT_VINTR doesn't need a trace record
Just like Intel's PENDING_VIRT_INTR, AMD's VINTR doesn't need an HVM
trace record. Expect that.
Signed-off-by: George Dunlap <george.dunlap@cloud.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
George Dunlap [Fri, 6 Oct 2023 14:55:02 +0000 (15:55 +0100)]
xenalyze: Only accumulate data from one vmexit without a handler
If a vmentry/exit arc unexpectedly doesn't have a handler, we throw an
error, and then log the information under HVM event 0; thus those
particular traces within the vmexit reason will have stats gathered,
and will show up with "(no handler)". This is useful in the event
that there are unusual paths through the hypervisor which don't have
trace points.
However, if there are more than one of these, then although we detect and warn
that this is happening, we subsequently continue to stuff data about all exits
into that one exit, even though we only show it in one place.
Instead, refator things to only allow a single exit reason to be
accumulated into any given event.
Also put a comment explaining what's going on, and how to fix it.
Signed-off-by: George Dunlap <george.dunlap@cloud.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Julien Grall [Thu, 5 Oct 2023 16:52:41 +0000 (17:52 +0100)]
xen/arm: vtimer: Don't read/use the secure physical timer interrupt for ACPI
Per ACPI 6.5 section 5.2.25 ("Generic Timer Description Table (GTDT)"),
the fields "Secure EL1 Timer GSIV/Flags" are optional and an OS running
in non-secure world is meant to ignore the values.
However, Xen is trying to reserve the value. The ACPI tables for Graviton
2 metal instances will provide the value 0 which is not a correct PPI
(PPIs start at 16) and would have in fact been already reserved by Xen
as this is an SGI. Xen will hit the BUG() and panic().
For the Device-Tree case, I couldn't find a statement suggesting
that the secure physical timer interrupt is ignored. In fact, I have
found some code in Linux using it as a fallback. That said, it should
never be used.
As I am not aware of any issue when booting using Device-Tree, the
physical timer interrupt is only ignored for ACPI.
Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Michal Orzel [Thu, 28 Sep 2023 12:34:35 +0000 (14:34 +0200)]
xen/arm: Validate generic timer frequency
Generic timer dt node property "clock-frequency" (refer Linux dt binding
Documentation/devicetree/bindings/timer/arm,arch_timer.yaml) is used to
override the incorrect value set by firmware in CNTFRQ_EL0. If the value
of this property is 0 (i.e. by mistake), Xen would continue to work and
use the value from the sysreg instead. The logic is thus incorrect and
results in inconsistency when creating timer node for domUs:
- libxl domUs: clock_frequency member of struct xen_arch_domainconfig
is set to 0 and libxl ignores setting the "clock-frequency" property,
- dom0less domUs: "clock-frequency" property is taken from dt_host and
thus set to 0.
Property "clock-frequency" is used to override the value from sysreg,
so if it is also invalid, there is nothing we can do and we shall panic
to let user know about incorrect setting. Going even further, we operate
under assumption that the frequency must be at least 1KHz (i.e. cpu_khz
not 0) in order for Xen to boot. Therefore, introduce a new helper
validate_timer_frequency() to verify this assumption and use it to check
the frequency obtained either from dt property or from sysreg.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Andrew Cooper [Tue, 19 Sep 2023 11:13:50 +0000 (12:13 +0100)]
x86/pv: Correct the auditing of guest breakpoint addresses
The use of access_ok() is buggy, because it permits access to the compat
translation area. 64bit PV guests don't use the XLAT area, but on AMD
hardware, the DBEXT feature allows a breakpoint to match up to a 4G aligned
region, allowing the breakpoint to reach outside of the XLAT area.
Prior to c/s cda16c1bb223 ("x86: mirror compat argument translation area for
32-bit PV"), the live GDT was within 4G of the XLAT area.
All together, this allowed a malicious 64bit PV guest on AMD hardware to place
a breakpoint over the live GDT, and trigger a #DB livelock (CVE-2015-8104).
Introduce breakpoint_addr_ok() and explain why __addr_ok() happens to be an
appropriate check in this case.
For Xen 4.14 and later, this is a latent bug because the XLAT area has moved
to be on its own with nothing interesting adjacent. For Xen 4.13 and older on
AMD hardware, this fixes a PV-trigger-able DoS.
This is part of XSA-444 / CVE-2023-34328.
Fixes: 65e355490817 ("x86/PV: support data breakpoint extension registers") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Thu, 21 Sep 2023 16:26:23 +0000 (17:26 +0100)]
x86/svm: Fix asymmetry with AMD DR MASK context switching
The handling of MSR_DR{0..3}_MASK is asymmetric between PV and HVM guests.
HVM guests context switch in based on the guest view of DBEXT, whereas PV
guest switch in base on the host capability. Both guest types leave the
context dirty for the next vCPU.
This leads to the following issue:
* PV or HVM vCPU has debugging active (%dr7 + mask)
* Switch out deactivates %dr7 but leaves other state stale in hardware
* HVM vCPU with debugging activate but can't see DBEXT is switched in
* Switch in loads %dr7 but leaves the mask MSRs alone
Now, the HVM vCPU is operating in the context of the prior vCPU's mask MSR,
and furthermore in a case where it genuinely expects there to be no masking
MSRs.
As a stopgap, adjust the HVM path to switch in/out the masks based on host
capabilities rather than guest visibility (i.e. like the PV path). Adjustment
of the of the intercepts still needs to be dependent on the guest visibility
of DBEXT.
This is part of XSA-444 / CVE-2023-34327
Fixes: c097f54912d3 ("x86/SVM: support data breakpoint extension registers") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>