Hi Jason,

On 5/21/2024 3:13 AM, Jason Andryuk wrote:
+
+=item B<nr_spis="NR_SPIS">
+
+A 32-bit optional integer parameter specifying the number of SPIs (Shared

I'd phrase it "An optional 32-but integer"

+Peripheral Interrupts) to allocate for the domain. If the `nr_spis` parameter +is missing, the max number of SPIs calculated by the toolstack based on the
+devices allocated for the domain will be used.

This text says the maximum only applies if xl.cfg nr_spis is not setup.

+
+=back
+
  =head3 x86
    =over 4

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 1cb89fa584..a4029e3ac8 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -181,8 +181,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
        LOG(DEBUG, "Configure the domain");
  -    config->arch.nr_spis = nr_spis;
-    LOG(DEBUG, " - Allocate %u SPIs", nr_spis);
+    config->arch.nr_spis = max(nr_spis, d_config->b_info.arch_arm.nr_spis);
+    LOG(DEBUG, " - Allocate %u SPIs", config->arch.nr_spis);

But this is always taking the max.  Should it instead be:

config->arch.nr_spis = d_config->b_info.arch_arm.nr_spis ?: nr_spis;

However, I don't know if that makes sense for ARM.  Does the hardware nr_spis need to be a minimum for a domain?

Really, we just want the documentation to match the code.

Before you pointed this out, I didn't realize the ambiguity in the doc about the "max". The "max" in the doc have different meanings compared to the "max()" in the code. I will drop the "max" in the doc and reword the doc to "If the `nr_spis` parameter is missing, the number of SPIs calculated by the toolstack based on the  devices allocated for the domain will be used.". Thanks for pointing it out.

Kind regards,
Henry


Thanks,
Jason


Reply via email to