Restructure the code so that one can use pa_range_info[] table for both
arm32 as well as arm64.
Also, removed the hardcoding for P2M_ROOT_ORDER and P2M_ROOT_LEVEL as
p2m_root_order can be obtained from the pa_range_info[].root_order and
p2m_root_level can be obtained from pa_range_info[].sl0.
Refer ARM DDI 0406C.d ID040418, B3-1345,
"Use of concatenated first-level translation tables
...However, a 40-bit input address range with a translation granularity of 4KB
requires a total of 28 bits of address resolution. Therefore, a stage 2
translation that supports a 40-bit input address range requires two concatenated
first-level translation tables,..."
Thus, root-order is 1 for 40-bit IPA on arm32.
Refer ARM DDI 0406C.d ID040418, B3-1348,
"Determining the required first lookup level for stage 2 translations
For a stage 2 translation, the output address range from the stage 1
translations determines the required input address range for the stage 2
translation. The permitted values of VTCR.SL0 are:
0b00 Stage 2 translation lookup must start at the second level.
0b01 Stage 2 translation lookup must start at the first level.
VTCR.T0SZ must indicate the required input address range. The size of the input
address region is 2^(32-T0SZ) bytes."
Thus VTCR.SL0 = 1 (maximum value) and VTCR.T0SZ = -8 when the size of input
address region is 2^40 bytes.
Thus, pa_range_info[].t0sz = 1 (VTCR.S) | 8 (VTCR.T0SZ) ie 11000b which is 24.
VTCR.T0SZ, is bits [5:0] for arm64.
VTCR.T0SZ is bits [3:0] and S(sign extension), bit[4] for arm32.
For this, we have used struct bitfields to convert pa_range_info[].t0sz to its
arm32 variant.
pa_range_info[] is indexed by ID_AA64MMFR0_EL1.PARange which is present in Arm64
only. This is the reason we do not specify the indices for arm32. Also, we
duplicated the entry "{ 40, 24/*24*/, 1, 1 }" between arm64 and
arm32. This is done to avoid introducing extra #if-defs.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
[julien: Tweak some comments and one check]
Acked-by: Julien Grall <jgrall@amazon.com>
The RSBA bit, "RSB Alternative", means that the RSB may use alternative
predictors when empty. From a practical point of view, this mean "Retpoline
not safe".
Enhanced IBRS (officially IBRS_ALL in Intel's docs, previously IBRS_ATT) is a
statement that IBRS is implemented in hardware (as opposed to the form
retrofitted to existing CPUs in microcode).
The RRSBA bit, "Restricted-RSBA", is a combination of RSBA, and the eIBRS
property that predictions are tagged with the mode in which they were learnt.
Therefore, it means "when eIBRS is active, the RSB may fall back to
alternative predictors but restricted to the current prediction mode". As
such, it's stronger statement than RSBA, but still means "Retpoline not safe".
CPUs are not expected to enumerate both RSBA and RRSBA.
Add feature dependencies for EIBRS and RRSBA. While technically they're not
linked, absolutely nothing good can come of letting the guest see RRSBA
without EIBRS. Nor a guest seeing EIBRS without IBRSB. Furthermore, we use
this dependency to simplify the max derivation logic.
The max policies gets RSBA and RRSBA unconditionally set (with the EIBRS
dependency maybe hiding RRSBA). We can run any VM, even if it has been told
"somewhere you might run, Retpoline isn't safe".
The default policies are more complicated. A guest shouldn't see both bits,
but it needs to see one if the current host suffers from any form of RSBA, and
which bit it needs to see depends on whether eIBRS is visible or not.
Therefore, the calculation must be performed after sanitise_featureset().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
In order to level a VM safely for migration, the toolstack needs to know the
RSBA/RRSBA properties of the CPU, whether or not they happen to be enumerated.
See the code comment for details.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This is prep work, split out to simply the diff on the following change.
* Rename to retpoline_calculations(), and call unconditionally. It is
shortly going to synthesise missing enumerations required for guest safety.
* For the model check switch statement, store the result in a variable and
break rather than returning directly.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reword the comment for 'S' to include an incompatible set of features on the
same core.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
FB_CLEAR is a read-only status bit, not a read-write control. Move it from
"Hardware features" into "Hardware hints".
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
This is inconsistent with the rest of the code where macros are used
to define functions, as it results in an empty declaration (i.e.
semicolon with nothing before it) after function definition. This is also
not allowed by C99.
Take the opportunity to undefine TLB_HELPER* macros after last use.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
To comply with MISRA C:2012 Rule 8.10 ("An inline function shall be
declared with the static storage class"), remove inline function
specifier from handle_ro_raz() since asking the compiler to inline
such function does not seem to add any kind of value.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Acked-by: Julien Grall <jgrall@amazon.com>
The name PIN_FAIL() is poor; it's not used only for pinning failures. More
importantly, it interferes with code legibility by hiding control flow.
Expand and drop it.
* Drop redundant "rc = rc" assignment
* Rework gnttab_copy_buf() to be simpler by dropping the rc variable
As a side effect, this fixes several violations of MISRA rule 2.1 (dead code -
the while() following a goto).
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
'int' bitfields in particular have implementation defined behaviour under gcc
and can change signed-ness with -funsigned-bitfields.
There is no need for low_bit_was_clear to be a bitfield in the first place; it
is only used as a boolean. Doing so even improves the code generation in
sh_emulate_map_dest() to avoid emitting a merge with structure padding.
Spotted by Eclair MISRA scanner.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Having ALIGN() inside a section definition usually makes sense only with
a label definition following (an exception case is a few lines out of
context, where cache line sharing is intended to be avoided).
Constituents of .bss.page_aligned need to specify their own alignment
correctly anyway, or else they're susceptible to link order changing.
This requirement is already met: Arm-specific code has no such object,
while common (EFI) code has another one. That one has suitable alignment
specified.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
To comply with Misra rule 8.10 ("An inline function shall be declared
with the static storage class"), convert what is presently
_spin_lock_cb() to an always-inline (and static) helper, while making
the function itself a thin wrapper, just like _spin_lock() is.
While there drop the unlikely() from the callback check, and correct
indentation in _spin_lock().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
This in particular allows to drop a dead assignment to "ptes" from near
the end of the function.
Coverity ID: 1532314
Fixes: bd7a29c3d0 ("tools/libs/ctrl: fix xc_core_arch_map_p2m() to support linear p2m table")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
"t" is written first thing at the "retry_transaction" label.
Coverity ID: 1532321
Fixes: 1057300109 ("libxl: fix error handling (xenstore transaction leak) in libxl__domain_make")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
"rc" is written immediately below the outer if(). Fold the remaining two
if()s.
Coverity ID: 1532320
Fixes: 685e922d6f ("tools/libxc: Rework xc_cpuid_apply_policy() to use {get,set}_cpu_policy()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
The variable isn't used past the loop, and its value also isn't
meaningful across iterations. Reduce its scope to make this more
obvious.
Coverity ID: 1532310
Fixes: ae763e4224 ("tools/misc: introduce xen-mfndump")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Commit 438c5ffa44 ("rpmball: Adjust to
new rpm, do not require --force") attempted to handle stricter
directory permissions in newer distributions.
This introduced a few issues:
- /boot used to be a constant prior commit
6475d70005 ("use BOOT_DIR as xen.gz
install location"), since this commit the location has to be
referenced via ${BOOT_DIR}
- it assumed the prefix and the various configurable paths match the
glob pattern /*/*/*
Adjust the code to build a filelist on demand and filter directories
from an installed filesystem.rpm. This works on a SUSE system, and
will likely work on a RedHat based system as well.
Take the opportunity to replace the usage of $RPM_BUILD_ROOT with
%buildroot, and use pushd/popd pairs.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
The iommu local variable does not point to to a valid amd_iommu element
after the call to for_each_amd_iommu(). Instead check whether any IOMMU
on the system doesn't support Invalidate All in order to perform the
per-domain and per-device flushes.
Fixes: 9c46139de8 ('amd iommu: Support INVALIDATE_IOMMU_ALL command.')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Fields that generic_identify() sets unconditionally don't need pre-
setting. (In fact the compiler removes some of those assignments anyway,
at least in release builds.)
With the setting of ->cpuid_level to -1 gone, also drop the respective
BUG_ON() from default_init().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Guard it with CONFIG_LIVEPATCH. Note alternatives are applied at boot
using _apply_alternatives().
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The _setdefault() function for virtio devices is getting called after
libxl__prepare_dtb(), which is late as libxl__prepare_dtb() expects the
defaults to be already set by this time.
Call libxl__virtio_devtype.set_default() from
libxl__domain_config_setdefault(), in a similar way as other devices
like disk, etc.
Suggested-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Only the VirtIO backend will watch xenstore to find out when a new
instance needs to be created for a guest, and read the parameters from
there. VirtIO frontend are only virtio, so they will not do anything
with the xenstore nodes. They can be removed.
While at it, also add a comment to the libxl_virtio.c file.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Some version of GCC will complain because the prototype and the
declaration of domain_max_chk() don't match:
xenstored_domain.c:1503:6: error: conflicting types for 'domain_max_chk' due to enum/integer mismatch; have '_Bool(const struct connection *, enum accitem, unsigned int)' [-Werror=enum-int-mismatch]
1503 | bool domain_max_chk(const struct connection *conn, enum accitem what,
| ^~~~~~~~~~~~~~
In file included from xenstored_domain.c:31:
xenstored_domain.h:146:6: note: previous declaration of 'domain_max_chk' with type '_Bool(const struct connection *, unsigned int, unsigned int)'
146 | bool domain_max_chk(const struct connection *conn, unsigned int what,
| ^~~~~~~~~~~~~~
Update the prototype to match the declaration.
This was spotted by Gitlab CI with the job opensuse-tumbleweed-gcc.
Fixes: 685048441e ("tools/xenstore: switch quota management to be table based")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Tested-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
There is no real need for including tools/xenstore/utils.h from
libxenstore, as only streq() and ARRAY_SIZE() are obtained via that
header.
streq() is just !strcmp(), and ARRAY_SIZE() is brought in via
xen-tools/common-macros.h.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Replace the usage of the xenstore private list.h header with the
common xen_list.h one.
Use the XEN_TAILQ type list, as it allows to directly swap the
related macros/functions without having to change the logic.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Rename the guest_cpuinfo structure to domain_cpuinfo as it is not only
used for guests but also for dom0 so domain is a more suitable name.
While there also rename the create_guest_cpuinfo function to
create_domain_cpuinfo to be coherent and fix comments accordingly.
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
The original change doesn't compile on ARM:
xenctrl_stubs.c: In function 'stub_xc_physinfo':
xenctrl_stubs.c:821:16: error: unused variable 'arch_cap_flags_tag' [-Werror=unused-variable]
821 | int r, arch_cap_flags_tag;
| ^~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
but it was buggy too.
First, it tried storing an int in a pointer slot, causing heap corruption.
Next, it is not legitimate to exclude arm32 in the toolstack as it explicitly
can operate an arm64 toolstack and build arm64 domains. That in turn means
that you can't stash a C uint32_t in an OCaml int.
Rewrite the arch_capabilities handling from scratch. Break it out into a
separate function, and make the construction of arch_physinfo_cap_flags common
to prevent other indirection bugs.
Reintroduce arm_physinfo_caps with the fields broken out.
Fixes: 56a7aaa16b ("tools: add physinfo arch_capabilities handling for Arm")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
This gets more coverage of optional parts of the build, and makes it easier to
trial Ocaml related changes in the smoke tests.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
In order to prepare the replacement of TDB with direct accessible nodes
in memory, remove the support for a file backed data base.
This allows to remove xs_tdb_dump, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
The TODO file is not really helpful any longer. It contains only
entries which no longer apply or it is unknown what they are meant
for ("Dynamic/supply nodes", "Remove assumption that rename doesn't
fail").
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
There is no need to keep struct wrl_timestampt, as it serves the same
purpose as the more simple time base provided by get_now().
Move some more stuff from xenstored_domain.h into xenstored_domain.c
as it is being used nowhere else.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Some wrl_*() functions are only used in xenstored_domain.c, so make
them static. In order to avoid the need of forward declarations, move
the whole function block to the start of the file.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Today hashtable_insert() returns 0 in case of an error. Change that to
let it return an errno value in the error case and 0 in case of success.
In order to avoid any missed return value checks or related future
backport errors, rename hashtable_insert() to hashtable_add().
Even if not used today, do the same switch for the return value of
hashtable_expand().
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The minsize parameter of create_hashtable() doesn't have any real use
case for Xenstore, so drop it.
For better talloc_report_full() diagnostic output add a name parameter
to create_hashtable().
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Do the following cleanups:
- hashtable_count() isn't used at all, so remove it
- replace prime_table_length and max_load_factor with macros
- make hash() static
- add a loadlimit() helper function
- remove the /***/ lines between functions
- do some style corrections
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
MASK_EXTR() and MASK_INSR() are a matching pair. Keep them together.
Drop the pair from x86-emulate.h which includes common-macros.h.
Fixes: 56a7aaa16b ("tools: add physinfo arch_capabilities handling for Arm")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Some of the current users of hvm_cr4_guest_valid_bits() don't print
the valid mask in case of error, and thus the resulting error messages
are not as helpful as they could be.
Amend callers to always print the value of hvm_cr4_guest_valid_bits()
together with the rejected bits in the checked value. Also take the
opportunity and adjust all the users to use the same print formatter.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The Install step is failing with:
Package python-dev is not available, but is referred to by another package.
This may mean that the package is missing, has been obsoleted, or
E: Package 'python-dev' has no installation candidate
is only available from another source
However the following packages replace it:
python2-dev python2 python-dev-is-python3
which is a side effect of ubuntu-latest having changed from 20.04 to 22.04.
Drop python-dev as python3-dev is already present, and used a fixed runs-on to
avoid such problems in the future.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
A recent change to the documentation for the xl.cfg introduced a build
error, fix the issue.
Fixes: b46e8207d0 ("xen/tools: add sve parameter in XL configuration")
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Tested-by: Michal Orzel <michal.orzel@amd.com>
Instead of having individual quota variables switch to a table based
approach like the generic accounting. Include all the related data in
the same table and add accessor functions.
This enables to use the command line --quota parameter for setting all
possible quota values, keeping the previous parameters for
compatibility.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Let get_optval_int() return an unsigned value and rename it
accordingly.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
The maxrequests, node size, number of node permissions, and path length
quota are a little bit special, as they are either active in
transactions only (maxrequests), or they are just per item instead of
count values. Nevertheless being able to know the maximum number of
those quota related values per domain would be beneficial, so add them
to the generic accounting.
The per domain value will never show current numbers other than zero,
but the maximum number seen can be gathered the same way as the number
of nodes during a transaction.
To be able to use the const qualifier for a new function switch
domain_is_unprivileged() to take a const pointer, too.
For printing the quota/max values, adapt the print format string to
the longest quota name (now 17 characters long).
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Add saving the maximum values of the different accounting data seen
per domain and (for unprivileged domains) globally, and print those
values via the xenstore-control quota command. Add a sub-command for
resetting the global maximum values seen.
This should help for a decision how to set the related quotas.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Ayan Kumar Halder