Add MISRA C Rules 11.1, 11.2, 11.3, 11.6 as discussed.
Explicitly add in the notes that conversions to integer types are
permitted if the destination type has enough bits to hold the entire
value. GCC gives enough guarantees in terms of preserving the bit
content in such situations.
Also allow for bool conversions (e.g. to check if a function point is
valid).
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
As of d58a509e01 ("build,x86: remove the need for build32.mk") we
don't need to use non-standard #include forms anymore in the sources for
the boot code helper blobs. The only place where this kind of #include
needs to remain (for the time being) is mkelf32.c.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
These macros were hiding that the ops are not zero-initialized by the
toolstack. This is needlessly opaque for something so simple, so this patch
removes them and replaces them with explicit zero-initialising versions.
The patch also removes PHYSDEV_OP from there, as that seems to be an old
dead macro.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Before speculation defences, some paths in Xen could genuinely get away with
being IRQs-on at entry. But XPTI invalidated this property on most paths, and
attempting to maintain it on the remaining paths was a mistake.
Fast forward, and DO_SPEC_CTRL_COND_IBPB (protection for AMD BTC/SRSO) is not
IRQ-safe, running with IRQs enabled in some cases. The other actions taken on
these paths happen to be IRQ-safe.
Make entry_int82() and int80_direct_trap() unconditionally Interrupt Gates
rather than Trap Gates. Remove the conditional re-adjustment of
int80_direct_trap() in smp_prepare_cpus(), and have entry_int82() explicitly
enable interrupts when safe to do so.
In smp_prepare_cpus(), with the conditional re-adjustment removed, the
clearing of pv_cr3 is the only remaining action gated on XPTI, and it is out
of place anyway, repeating work already done by smp_prepare_boot_cpu(). Drop
the entire if() condition to avoid leaving an incorrect vestigial remnant.
Also drop comments which make incorrect statements about when its safe to
enable interrupts.
This is XSA-446 / CVE-2023-46836
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The current setup of the quarantine page tables assumes that the quarantine
domain (dom_io) has been initialized with an address width of
DEFAULT_DOMAIN_ADDRESS_WIDTH (48).
However dom_io being a PV domain gets the AMD-Vi IOMMU page tables levels based
on the maximum (hot pluggable) RAM address, and hence on systems with no RAM
above the 512GB mark only 3 page-table levels are configured in the IOMMU.
On systems without RAM above the 512GB boundary amd_iommu_quarantine_init()
will setup page tables for the scratch page with 4 levels, while the IOMMU will
be configured to use 3 levels only. The page destined to be used as level 1,
and to contain a directory of PTEs ends up being the address in a PTE itself,
and thus level 1 page becomes the leaf page. Without the level mismatch it's
level 0 page that should be the leaf page instead.
The level 1 page won't be used as such, and hence it's not possible to use it
to gain access to other memory on the system. However that page is not cleared
in amd_iommu_quarantine_init() as part of re-initialization of the device
quarantine page tables, and hence data on the level 1 page can be leaked
between device usages.
Fix this by making sure the paging levels setup by amd_iommu_quarantine_init()
match the number configured on the IOMMUs.
Note that IVMD regions are not affected by this issue, as those areas are
mapped taking the configured paging levels into account.
This is XSA-445 / CVE-2023-46835
Fixes: ea38867831 ('x86 / iommu: set up a scratch page in the quarantine domain')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
sphinx-build reports:
docs/designs/launch/hyperlaunch.rst: WARNING: document isn't included in any toctree
docs/designs/launch/hyperlaunch-devicetree.rst: WARNING: document isn't included in any toctree
docs/misc/xen-makefiles/makefiles.rst: WARNING: document isn't included in any toctree
docs/misra/C-language-toolchain.rst: WARNING: document isn't included in any toctree
docs/misra/C-runtime-failures.rst: WARNING: document isn't included in any toctree
docs/misra/documenting-violations.rst: WARNING: document isn't included in any toctree
docs/misra/exclude-list.rst: WARNING: document isn't included in any toctree
docs/misra/xen-static-analysis.rst: WARNING: document isn't included in any toctree
Create an up-to-date index.rst in misra/ create an "unsorted docs" section at
the top level to contain everything else. They can be re-filed at a later
date.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
sphinx-build reports:
docs/misra/exclude-list.rst:47: WARNING: Unexpected indentation.
docs/misra/exclude-list.rst:50: WARNING: Inline emphasis start-string without end-string.
Nested lists require newlines as delimiters, and * either needs escaping, or
put in a literal block.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
sphinx-build reports:
docs/designs/launch/hyperlaunch.rst:111: WARNING: Title underline too short.
docs/designs/launch/hyperlaunch.rst:203: WARNING: Unexpected indentation.
docs/designs/launch/hyperlaunch.rst:216: WARNING: Unexpected indentation.
Nested lists require newlines as delimiters.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
These 3 Kconfig docs were imported from Linux erroneously. They are
GPL-2.0-only in Linux, but have no SPDX tag and were placed in such a way to
be included by the blanket statement saying that all RST files are CC-BY-4.0.
We should not be carrying a shadow copy of these docs. They aren't even wired
into our Sphinx docs, and anyone wanting to refer to Kconfig docs is going to
look at the Linux docs anyway. These, and more docs can be found at:
https://www.kernel.org/doc/html/latest/kbuild/
which also have corrections vs the snapshot we took.
Fixes: f80fe2b34f ("xen: Update Kconfig to Linux v5.4")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Two files are missing SDPX tags, but are covered by the blanketing license
statement in docs/README.sources saying that RST files are CC-BY-4.0
Fixes: 3c911be55f ("docs/misra: document the C dialect and translation toolchain assumptions.")
Fixes: 7211904e09 ("docs/misra: add documentation skeleton for MISRA C:2012 Dir 4.1")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Make function definitions and declarations consistent.
No functional change.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Add missing parameter names and make function declarations and
definitions consistent.
No functional change.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Adjust line to use a hard tab and align using spaces afterwards, instead of
using all spaces.
Fixes: fc3090a47b ('x86/boot: Clear XD_DISABLE from the early boot path')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Platforms which doesn't have HAS_PCI enabled it is needed to
have <asm/pci.h>, which contains only an empty definition of
struct arch_pci_dev (except Arm, it introduces several Arm-specific
functions ).
Also, for architectures (such as PPC or RISC-V) on initial
stages of adding support, it is needed to generate <asm/pci.h>
for only define the mentioned above arch_pci_dev structure.
For the Arm-only stubs (mentioned in <asm/pci.h> for disabled
HAS_PCI and Arm-specific) will be needed
to add <asm/pci.h> directly alongside <xen/pci.h>. Only to
<arm/domain.c> <asm/pci.h> was added.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Much like the other PIC ports, Dom0 has no business touching these. Even
our own uses are somewhat questionable, as the corresponding IO-APIC
code in Linux is enclosed in a CONFIG_EISA conditional; I don't think
there are any x86-64 EISA systems.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
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.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
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().
No functional changes are intended.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Make function definitions and declarations consistent. Use "ptr" as
const void* parameter to match the definitions on x86.
No functional change.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
[stefano: drop blank, improve commit message]
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
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>
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>
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.
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
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;
UBSAN:
pointer operation underflowed ffffffff to 00000000
Fixes: 4e3fb2fb47 ("ubsan: add clang 5.0 support")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
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:
x mixed
+ phys
* cluster
N Min Max Median Avg Stddev
x 25 3.5131328e+08 3.5716441e+08 3.5410987e+08 3.5432659e+08 1566737.4
+ 12 1.231082e+09 1.238824e+09 1.2370528e+09 1.2357981e+09 2853892.9
Difference at 95.0% confidence
8.81472e+08 +/- 1.46849e+06
248.774% +/- 0.96566%
(Student's t, pooled s = 2.05985e+06)
* 11 3.5099276e+08 3.5561459e+08 3.5461234e+08 3.5415668e+08 1415071.9
No difference proven at 95.0% confidence
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>
The new bootloader_restrict and bootloader_user fields in the libxl idl
change the bindings. Update them.
Fixes: 1f762642d2 ("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>
The new FF-A TEE type changed the go bindings. Update them.
Fixes: 8abdd8d528 ("tools: add Arm FF-A mediator")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: George Dunlap <george.dunlap@cloud.com>
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>
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: 73b47eea21 ('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>
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
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>
VERBOSE had ceased to exist already before the introduction of this ELF
parsing code.
Fixes: 62ba982424 ("x86: parse Dom0 kernel for PVHv2")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
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".
Signed-off-by: Juergen Gross <jgross@suse.com>
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.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The aforementioned directive requires the project to supply documentation
on the measures taken towards the minimization of run-time failures.
The actual content of the documentation still needs feedback from the
community.
The 'rules.rst' file is updated accordingly to mention the newly
added documentation.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Stefano Stabellini