[PULL 20/25] docs/system/generic-loader: move TODO to source code

Maintainers: Alistair Francis <alistair@alistair23.me>, Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Radoslaw Biernacki <rad@semihalf.com>, Leif Lindholm <leif.lindholm@oss.qualcomm.com>, Eric Auger <eric.auger@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>
There is a newer version of this series
[PULL 20/25] docs/system/generic-loader: move TODO to source code
Posted by Peter Maydell 3 weeks, 4 days ago
Currently we have a "Restrictions and ToDos" section at the bottom of
the document which notes that there's no way to specify a CPU to load
a file through that doesn't also set that CPU's PC.  This is written
as a developer-facing note.  Move this to a TODO comment in the
source code, and provide a shorter user-facing statement of the
current restriction under the specific sub-option that it applies to.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 docs/system/generic-loader.rst | 14 +++-----------
 hw/core/generic-loader.c       | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/docs/system/generic-loader.rst b/docs/system/generic-loader.rst
index d5416711e9..0df9b66976 100644
--- a/docs/system/generic-loader.rst
+++ b/docs/system/generic-loader.rst
@@ -99,6 +99,9 @@ shown below:
   If this option is not specified, then the data will be loaded via
   the address space of the first CPU, and no CPU will have its PC set.
 
+  Note that there is currently no way to specify the address space to
+  load the data without also causing that CPU's PC to be set.
+
   Since it sets the starting PC, this option should only be used for the boot
   image.
 
@@ -111,14 +114,3 @@ shown below:
 An example of loading an ELF file which CPU0 will boot is shown below::
 
     -device loader,file=./images/boot.elf,cpu-num=0
-
-Restrictions and ToDos
-^^^^^^^^^^^^^^^^^^^^^^
-
-At the moment it is just assumed that if you specify a cpu-num then
-you want to set the PC as well. This might not always be the case. In
-future the internal state 'set_pc' (which exists in the generic loader
-now) should be exposed to the user so that they can choose if the PC
-is set or not.
-
-
diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
index 24f3908b1c..66a24f7b2a 100644
--- a/hw/core/generic-loader.c
+++ b/hw/core/generic-loader.c
@@ -30,6 +30,24 @@
  * separate backend.
  */
 
+/*
+ * TODO: currently the "load a file" functionality provides no way
+ * for the user to specify which CPU address space to load the data
+ * into without also causing that CPU's PC to be set to the start
+ * address of the file.
+ *
+ * We could fix this by having a new suboption set-pc (default: true)
+ * so the user can say
+ *  -device loader,file=<file>,cpu-num=<cpu-num>
+ * for the current "use this address space and set the PC" behaviour
+ * or
+ *  -device loader,file=<file>,cpu-num=<cpu-num>,set-pc=off
+ * to just pick the address space and not set the PC.
+ *
+ * Using set-pc without file= should be handled as an error; otherwise
+ * it can feed through to what we set s->set_pc to.
+ */
+
 #include "qemu/osdep.h"
 #include "system/dma.h"
 #include "system/reset.h"
-- 
2.47.3