These are opencoded linked list and hashtable manipulations. Factor them out
into static inline helpers, and discuss the consequence for the domain.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This addresses violations of MISRA C:2012 Rule 5.3 which states as
following: An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope. Right here the conflict is with
the global named "e820".
No functional change.
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
In case cpu_schedule_up() is failing, it needs to undo all externally
visible changes it has done before.
Reason is that cpu_schedule_callback() won't be called with the
CPU_UP_CANCELED notifier in case cpu_schedule_up() did fail.
Fixes: 207589dbac ("xen/sched: move per cpu scheduler private data into struct sched_resource")
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
As a defensive measure, make sure to signal an error to the caller
if an unreachable program point is reached.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
MISRA C Rule 16.3 states that "An unconditional `break' statement shall
terminate every switch-clause".
Add pseudo keyword fallthrough or missing break statement
to address violations of the rule.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
constant_test_bit() is functionally the same as generic_test_bit(),
so constant_test_bit() can be dropped and replaced with
generic_test_bit().
Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Taken from Linux-6.4.0-rc1
Xen's bitops.h consists of several Linux's headers:
* linux/arch/include/asm/bitops.h:
* The following function were removed as they aren't used in Xen:
* test_and_set_bit_lock
* clear_bit_unlock
* __clear_bit_unlock
* The following functions were renamed in the way how they are
used by common code:
* __test_and_set_bit
* __test_and_clear_bit
* The declaration and implementation of the following functios
were updated to make Xen build happy:
* clear_bit
* set_bit
* __test_and_clear_bit
* __test_and_set_bit
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The following generic functions were introduced:
* test_bit
* generic__test_and_set_bit
* generic__test_and_clear_bit
* generic__test_and_change_bit
These functions and macros can be useful for architectures
that don't have corresponding arch-specific instructions.
Also, the patch introduces the following generics which are
used by the functions mentioned above:
* BITOP_BITS_PER_WORD
* BITOP_MASK
* BITOP_WORD
* BITOP_TYPE
The following approach was chosen for generic*() and arch*() bit
operation functions:
If the bit operation function that is going to be generic starts
with the prefix "__", then the corresponding generic/arch function
will also contain the "__" prefix. For example:
* test_bit() will be defined using arch_test_bit() and
generic_test_bit().
* __test_and_set_bit() will be defined using
arch__test_and_set_bit() and generic__test_and_set_bit().
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
The current code in ALT_CALL_ARG() won't successfully workaround the clang
code-generation issue if the arg parameter has a size that's not a power of 2.
While there are no such sized parameters at the moment, improve the workaround
to also be effective when such sizes are used.
Instead of using a union with a long use an unsigned long that's first
initialized to 0 and afterwards set to the argument value.
Reported-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Suggested-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
One of the error paths in the PV dom0 builder section that runs on the guest
page-tables wasn't restoring the Xen value of %cr3, neither removing the
mapcache override.
Fixes: 079ff2d32c ('libelf-loader: introduce elf_load_image')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Therefore with SHADOW_PAGING=n this is better compiled out, to avoid
leaving around unreachable/dead code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
No other arch-*.h does so, and arch-x86/xen.h really just takes the role
of arch-x86_32.h and arch-x86_64.h (by those two forwarding there). With
xen.h itself including the per-arch headers, doing so is also kind of
backwards anyway, and just calling for problems. There's exactly one
place where arch-x86/xen.h is included when really xen.h is meant (for
wanting XEN_GUEST_HANDLE_64() to be made available, the default
definition of which lives in the common xen.h).
This then addresses a violation of Misra C:2012 Directive 4.10
("Precautions shall be taken in order to prevent the contents of a
header file being included more than once").
Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
There's no use of them anymore except in the definitions of the non-
underscore-prefixed aliases.
On Arm convert the (renamed) inline function to a macro.
On x86 rename the inline functions, adjust the virt_to_maddr() #define,
and purge the maddr_to_virt() one, thus eliminating a bogus cast which
would have allowed the passing of a pointer type variable into
maddr_to_virt() to go silently.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Julien Grall <jgrall@amazon.com>
With arch_domain_create() no longer being called with the idle domain, drop
the last remaining logic.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
With x86 implementing arch_init_idle_domain(), there is no longer any need to
call arch_domain_create() with the idle domain.
Have the idle domain exit early with all other system domains. Move the
static-analysis ASSERT() earlier. Then, remove the !is_idle_domain()
protections around the majority of domain_create() and remove one level of
indentation.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The idle domain needs d->arch.ctxt_switch initialised on x86. Implement the
new arch_init_idle_domain() in order to do this.
Intentionally remove cpu_policy's initialisation to ZERO_BLOCK_PTR. It has
never tripped since it's introduction, and is weird to have in isolation
without a similar approach on other pointers.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The idle domain causes a large amount of complexity in domain_create() because
of x86's need to initialise d->arch.ctxt_switch in arch_domain_create().
In order to address this, introduce an optional hook to perform extra
initialisation of the idle domain.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Add a test script and related job for running x86_64 dom0 tests.
Signed-off-by: Victor Lira <victorm.lira@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
In practice, we are already following R18.6 and we have zero violations
reported by ECLAIR (there are some cautions being reported.)
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Bertrand Marquis <bertrand.marquis@arm.com>
As discussed during the last MISRA C meeting, add Rule 12.2 to the list
of MISRA C rules we accept, together with an explanation that we use gcc
-fsanitize=undefined to check for violations.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
This patch adds a bunch of rules to rules.rst that are uncontroversial
and have zero violations in Xen. As such, they have been approved for
adoption.
All the ones that regard the standard library have the link to the
existing footnote in the notes.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
In the file include/xen/event.h macro set_bit is called with argument
current->pause_flags.
Once expanded this set_bit's argument is used in sizeof operations
and thus 'current', being a macro that expands to a function
call with potential side effects, generates a violation.
To address this violation the value of current is therefore stored in a
variable called 'v' before passing it to macro set_bit.
No functional change.
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
To change the cd-rom medium, libxl will:
- QMP eject the medium from QEMU
- block-detach the old PV disk
- block-attach the new PV disk
- QMP change the medium to the new PV disk by fdset-id
The QMP code is reused, and remove and attach are implemented here.
The stubdom must internally handle adding /dev/xvdc to the appropriate
fdset. libxl in dom0 doesn't see the result of adding to the fdset as
that is internal to the stubdom, but the fdset's opaque fields will be
set to stub-devid:$devid, so libxl can identify it. $devid is common
between the stubdom and libxl, so it can be identified on both side.
The stubdom will name the device xvdY regardless of the guest name hdY,
sdY, or xvdY, but the stubdom will be assigned the same devid
facilitating lookup. Because the stubdom add-fd call is asynchronous,
libxl needs to poll query-fdsets to identify when add-fd has completed.
For cd-eject, we still need to attach the empty vbd. This is necessary
since xenstore is used to determine that hdc exists. Otherwise after
eject, hdc would be gone and the cd-insert would fail to find the drive
to insert new media.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Upgrade Yocto to a newer version. Use ext4 as image format for testing
with QEMU on ARM and ARM64 as the default is WIC and it is not available
for our xen-image-minimal target.
Also update the tar.bz2 filename for the rootfs.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
The XSM checks for XEN_DOMCTL_createdomain are problematic. There's a split
between xsm_domctl() called early, and flask_domain_create() called quite late
during domain construction.
All XSM implementations except Flask have a simple IS_PRIV check in
xsm_domctl(), and operate as expected when an unprivileged domain tries to
make a hypercall.
Flask however foregoes any action in xsm_domctl() and defers everything,
including the simple "is the caller permitted to create a domain" check, to
flask_domain_create().
As a consequence, when XSM Flask is active, and irrespective of the policy
loaded, all domains irrespective of privilege can:
* Mutate the global 'rover' variable, used to track the next free domid.
Therefore, all domains can cause a domid wraparound, and combined with a
voluntary reboot, choose their own domid.
* Cause a reasonable amount of a domain to be constructed before ultimately
failing for permission reasons, including the use of settings outside of
supported limits.
In order to remediate this, pass the ssidref into xsm_domctl() and at least
check that the calling domain privileged enough to create domains.
Take the opportunity to also fix the sign of the cmd parameter to be unsigned.
This issue has not been assigned an XSA, because Flask is experimental and not
security supported.
Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Right now the apic_mem_*() helpers only compile because sizeof(void *) ==
sizeof(long long). Switch to using fix_to_virt() which is a void *type.
Also adjust the two places where the APIC/IO-APIC virtual address is rendered
in a printk().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
xeninfo.pl was introduced in commit 1b0a8bb57e ("Added xeninfo.pl, a script
for collecting statistics from Xen hosts using the Xen-API") and has been
touched exactly twice since to remove hardcoded IP addresses and paths.
The configuration files in vnc/* date from when we had a vendered version of
Qemu living in the tree.
These have never (AFAICT) been wired into the `make install` rule.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
This reduces the repeated accessing of v->domain.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There are no callers outside the translation unit where it's defined, so make
the function static.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There's no l{1,2,3,4}e_read() implementation, so drop the _atomic suffix from
the read helpers. This allows unifying the naming with the write helpers,
which are also atomic but don't have the suffix already: l{1,2,3,4}e_write().
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The l{1,2,3,4}e_write_atomic() and non _atomic suffixed helpers share the same
implementation, so it seems pointless and possibly confusing to have both.
x86 32bit mode used to have a non-atomic PTE write that would split the write
in two halves, but with Xen only supporting x86 64bit that's no longer
present.
Remove the l{1,2,3,4}e_write_atomic() helpers and switch it's user to
l{1,2,3,4}e_write(), as that's also atomic. While there also remove
pte_write{,_atomic}() and just use write_atomic() in the wrappers.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This reverts commit e42d107d0d.
The Gitlab CI {adl,zen3p}-pci-hvm-x86-64-gcc-debug tests says this breaks PCI
Passthrough to HVM guests, with -EOPNOTSUPP.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
The decompression code parses a huffman tree and counts the number of
symbols for a given bit length. In rare cases, there may be >= 256
symbols with a given bit length, causing the unsigned char to overflow.
This causes a decompression failure later when the code tries and fails to
find the bit length for a given symbol.
Since the maximum number of symbols is 258, use unsigned short instead.
Fixes: ab77e81f65 ("x86/dom0: support bzip2 and lzma compressed bzImage payloads")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
If run Xen with PVH dom0 and hvm domU, hvm will map a pirq for
a passthrough device by using gsi, see qemu code
xen_pt_realize->xc_physdev_map_pirq and libxl code
pci_add_dm_done->xc_physdev_map_pirq. Then xc_physdev_map_pirq
will call into Xen, but in hvm_physdev_op, PHYSDEVOP_map_pirq
is not allowed because currd is PVH dom0 and PVH has no
X86_EMU_USE_PIRQ flag, it will fail at has_pirq check.
So, allow PHYSDEVOP_map_pirq when dom0 is PVH and also allow
PHYSDEVOP_unmap_pirq for the removal device path to unmap pirq.
And add a new check to prevent (un)map when the subject domain
doesn't have a notion of PIRQ.
So that the interrupt of a passthrough device can be
successfully mapped to pirq for domU with a notion of PIRQ
when dom0 is PVH
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Add deviation comments to address violations of
MISRA C:2012 Directive 4.10 ("Precautions shall be taken in order
to prevent the contents of a header file being included more than
once").
Inclusion guards must appear at the beginning of the headers
(comments are permitted anywhere).
This patch adds deviation comments using the format specified
in docs/misra/safe.json for headers with just the direct
inclusion guard before the inclusion guard since they are
safe and not supposed to comply with the directive.
Note that with SAF-10-safe in place, failures to have proper guards later
in the header files will not be reported
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
This patch modifies deviations for Directive 4.10:
"Precautions shall be taken in order to prevent the contents of
a header file being included more than once"
This patch avoids the file-based deviation for empty headers, and
replaces it with a comment-based one using the format specified in
docs/misra/safe.json.
Generated headers are not generally safe against multi-inclusions,
whether a header is safe depends on the nature of the generated code
in the header. For that reason, this patch drops the deviation for
generated headers.
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Some headers, under specific circumstances (documented in a comment at
the beginning of the file), explicitly do not have strict inclusion
guards: the caller is responsible for including them correctly.
These files are not supposed to comply with Directive 4.10:
"Precautions shall be taken in order to prevent the contents of a header
file being included more than once"
This patch adds deviation cooments for headers that avoid guards.
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Add a missing break statement to address a violation of
MISRA C Rule 16.3: "An unconditional `break' statement shall terminate
every switch-clause".
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>
Add missing escape for the final dot of the fallthrough comment,
extend the search of a fallthrough comment up to 2 lines after the last
statement and improve the text of the justification.
Fixes: a128d8da91 ("automation/eclair: add deviations for MISRA C:2012 Rule 16.3")
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
When building with gcc with -finstrument-functions, optimization level
-O1, CONFIG_HYPFS=y and # CONFIG_HAS_SCHED_GRANULARITY is not set, the
the following build warning (error) is encountered:
common/sched/cpupool.c: In function ‘cpupool_gran_write’:
common/sched/cpupool.c:1220:26: error: ‘gran’ may be used uninitialized [-Werror=maybe-uninitialized]
1220 | 0 : cpupool_check_granularity(gran);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
common/sched/cpupool.c:1207:21: note: ‘gran’ declared here
1207 | enum sched_gran gran;
| ^~~~
This is a false positive. Silence the warning (error) by initializing
the variable.
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
It's sadically misleading to show an error without letters and expect
the dmesg reader to understand it's in hex. The patch adds a 0x prefix
to all hex numbers that don't already have it.
On the one instance in which a boolean is printed as an integer, print
it as a decimal integer instead so it's 0/1 in the common case and not
misleading if it's ever not just that due to a bug.
While at it, rename VIRIDIAN CRASH to VIRIDIAN GUEST_CRASH. Every member
of a support team that looks at the message systematically believes
"viridian" crashed, which is absolutely not what goes on. It's the guest
asking the hypervisor for a sudden shutdown because it crashed, and
stating why.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Andrew Cooper