Add a generic loader
cover Add a generic loader
This work is based on the original work by Li Guang with extra features added by Peter C and myself.
The idea of this loader is to allow the user to load multiple images or values into QEMU at startup.
Memory values can be loaded like this: -device loader,addr=0xfd1a0104,data=0x8000000e,data-len=4
Images can be loaded like this: -device loader,file=./images/u-boot.elf,cpu=0
This can be useful and we use it a lot in Xilinx to load multiple images into a machine at creation (ATF, Kernel and DTB for example).
It can also be used to set registers.
The limiation for arch is based off settting the ELF_ARCH macro.
The reset patch is required otherwise the reset will never be registered and the loader can't change the PC in the case of images.
Changes since RFC:
- Add support for BE
Alistair Francis (2):
qdev-monitor.c: Register reset function if the device has one
generic-loader: Add a generic loader
default-configs/arm-softmmu.mak | 1 + hw/misc/Makefile.objs | 2 + hw/misc/generic-loader.c | 127 +++++++++++++++++++++++++++++++++++++++ include/hw/misc/generic-loader.h | 50 +++++++++++++++ qdev-monitor.c | 2 + 5 files changed, 182 insertions(+) create mode 100644 hw/misc/generic-loader.c create mode 100644 include/hw/misc/generic-loader.h
Signature · 1 line
2.5.0
PATCH 1/2 qdev-monitor.c: Register reset function if the device has one
If the device being added when running qdev_device_add() has a reset function, register it so that it can be called.
---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)
qdev-monitor.c +2 -0
@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); } id =qemu_opts_id (opts);
Signature · 1 line
2.5.0
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
Quoted 14 lines · Alistair Francis
If the device being added when running
qdev_device_add()has a reset function, register it so that it can be called.Signed-off-by Alistair Francis <[email protected]>---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)qdev-monitor.c +0
-0
qdev-monitor.c DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) · line 568
Quoted 8 lines
qdev-monitor.c +2
-0@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); } id =qemu_opts_id (opts);
This looks wrong to me.
You stuff all the device reset methods into the global reset_handlers list, where they get called in some semi-random order. This breaks when there are reset order dependencies between devices, e.g. between a device and the bus it plugs into.
Propagating the reset signal to all the devices is a qdev problem. Copying Andreas for further insight.
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
Quoted 23 lines · Markus Armbruster
Quoted 21 lines · Alistair Francis
If the device being added when running
qdev_device_add()has a reset function, register it so that it can be called.Signed-off-by Alistair Francis <[email protected]>---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)qdev-monitor.c +2
-0@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); }
Quoted 8 lines
qdev-monitor.c DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) · line 568
Quoted 1 line
qdev-monitor.c +0
-0id =qemu_opts_id (opts);This looks wrong to me.
You stuff all the device reset methods into the global
reset_handlerslist, where they get called in some semi-random order. This breaks when there are reset order dependencies between devices, e.g. between a device and the bus it plugs into.
So I'm not a expert on this, but from what I see this will also be added near the end of the list (before the rom_resets though) and doesn't seem to be a problem. Do you have any other ideas how the reset function could be registered?
Thanks,
Alistair
Quoted 4 lines
Propagating the reset signal to all the devices is a qdev problem. Copying Andreas for further insight.
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
In reply to Markus ArmbrusterQuoted 23 lines · Markus Armbruster
Quoted 21 lines · Alistair Francis
If the device being added when running
qdev_device_add()has a reset function, register it so that it can be called.Signed-off-by Alistair Francis <[email protected]>---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)qdev-monitor.c +2
-0@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); }
Quoted 8 lines
qdev-monitor.c DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) · line 568
Quoted 1 line
qdev-monitor.c +0
-0id =qemu_opts_id (opts);This looks wrong to me.
You stuff all the device reset methods into the global
reset_handlerslist, where they get called in some semi-random order. This breaks when there are reset order dependencies between devices, e.g. between a device and the bus it plugs into.
There is no bus here, see the "if" above the one that's being added.
However, what devices have done so far is to register/unregister the reset in the realize/unrealize methods, and I suggest doing the same.
If you really want to add the magic qemu_register_reset, you should at least do one of the following:
* add a matching unregister (no idea where)
* assert that the device is not hot-unpluggable, otherwise hot-unplug would leave a dangling pointer.
Paolo
Quoted 2 lines
Propagating the reset signal to all the devices is a qdev problem. Copying Andreas for further insight.
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
Quoted 35 lines · Paolo Bonzini
Quoted 23 lines · Markus Armbruster
Quoted 21 lines · Alistair Francis
If the device being added when running
qdev_device_add()has a reset function, register it so that it can be called.Signed-off-by Alistair Francis <[email protected]>---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)qdev-monitor.c +2
-0@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); }Quoted 8 lines
qdev-monitor.c DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) · line 568
Quoted 1 line
qdev-monitor.c +0
-0id =qemu_opts_id (opts);This looks wrong to me.
You stuff all the device reset methods into the global
reset_handlerslist, where they get called in some semi-random order. This breaks when there are reset order dependencies between devices, e.g. between a device and the bus it plugs into.
Quoted 5 lines
There is no bus here, see the "if" above the one that's being added.
However, what devices have done so far is to register/unregister the reset in the realize/unrealize methods, and I suggest doing the same.
Does this assume the device itself knows whether it is bus-connected or not? This way has the advantage of catchall-ing devices that have no bus connected and the device may or may not know whether it is bus-connected (nor should it need to know). Probably doesn't have in tree precedent yet, but I thought we wanted to move away from qdev/qbus managing the device-tree. So ideally, the new else should become unconditional long term once we debusify (and properly QOMify) the reset tree (and the if goes away).
Quoted 5 lines
If you really want to add the magic
qemu_register_reset, you should at least do one of the following:* add a matching unregister (no idea where)
You could add a boolean flag to DeviceState that is set by this registration. It can then be checked at unrealize to remove reset handler.
Regards,
Peter
Quoted 7 lines
* assert that the device is not hot-unpluggable, otherwise hot-unplug would leave a dangling pointer.
Paolo
Quoted 2 lines
Propagating the reset signal to all the devices is a qdev problem. Copying Andreas for further insight.
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
Quoted 36 lines · Peter Crosthwaite
Quoted 35 lines · Paolo Bonzini
Quoted 31 lines · Markus Armbruster
Quoted 14 lines · Alistair Francis
If the device being added when running
qdev_device_add()has a reset function, register it so that it can be called.Signed-off-by Alistair Francis <[email protected]>---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)qdev-monitor.c +0
-0qdev-monitor.c DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) · line 568
Quoted 8 lines
qdev-monitor.c +2
-0@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); } id =qemu_opts_id (opts);This looks wrong to me.
You stuff all the device reset methods into the global
reset_handlerslist, where they get called in some semi-random order. This breaks when there are reset order dependencies between devices, e.g. between a device and the bus it plugs into.
Quoted 4 lines
Quoted 4 lines
There is no bus here, see the "if" above the one that's being added.
However, what devices have done so far is to register/unregister the reset in the realize/unrealize methods, and I suggest doing the same.
Ok, I am happy to do it that way. It just seemed dodgy to me registering the reset in the realise.
This also seemed like a feature worth having, as I thought this would come up again in the future.
Quoted 10 lines
Quoted 1 line
Does this assume the device itself knows whether it is bus-connected or not? This way has the advantage of catchall-ing devices that have no bus connected and the device may or may not know whether it is bus-connected (nor should it need to know). Probably doesn't have in tree precedent yet, but I thought we wanted to move away from qdev/qbus managing the device-tree. So ideally, the new else should become unconditional long term once we debusify (and properly QOMify) the reset tree (and the if goes away).
That was my general thinking as well.
Thanks,
Alistair
Quoted 22 lines
Quoted 6 lines
If you really want to add the magic
qemu_register_reset, you should at least do one of the following:* add a matching unregister (no idea where)
You could add a boolean flag to DeviceState that is set by this registration. It can then be checked at unrealize to remove reset handler.
Regards,
PeterQuoted 7 lines
* assert that the device is not hot-unpluggable, otherwise hot-unplug would leave a dangling pointer.
Paolo
Quoted 2 lines
Propagating the reset signal to all the devices is a qdev problem. Copying Andreas for further insight.
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
In reply to Peter CrosthwaiteQuoted 38 lines · Peter Crosthwaite
Quoted 37 lines · Paolo Bonzini
Quoted 23 lines · Markus Armbruster
Quoted 21 lines · Alistair Francis
If the device being added when running
qdev_device_add()has a reset function, register it so that it can be called.Signed-off-by Alistair Francis <[email protected]>---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)qdev-monitor.c +2
-0@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); }Quoted 8 lines
qdev-monitor.c DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) · line 568
Quoted 1 line
qdev-monitor.c +0
-0id =qemu_opts_id (opts);This looks wrong to me.
You stuff all the device reset methods into the global
reset_handlerslist, where they get called in some semi-random order. This breaks when there are reset order dependencies between devices, e.g. between a device and the bus it plugs into.There is no bus here, see the "if" above the one that's being added.
Quoted 7 lines
Quoted 2 lines
However, what devices have done so far is to register/unregister the reset in the realize/unrealize methods, and I suggest doing the same.
Does this assume the device itself knows whether it is bus-connected or not? This way has the advantage of catchall-ing devices that have no bus connected and the device may or may not know whether it is bus-connected (nor should it need to know).
A device almost definitely needs to know if it is bus connected. More likely than not, a busless device inherits from DeviceState while a device with a bus inherits from SCSIDevice, PCIDevice, I2CSlave, etc.
Quoted 5 lines
Probably doesn't have in
tree precedent yet, but I thought we wanted to move away from qdev/qbus managing the device-tree. So ideally, the new else should become unconditional long term once we debusify (and properly QOMify) the reset tree (and the if goes away).
Any abstraction we have in QEMU should have at least a parallel (though it need not be the same) in real hardware. Reset signals _do_ propagate along buses, or at least along some buses, so "debusifying" reset seems like a counterproductive goal to me.
For busless devices, I thought the idea was just to have the QOM parent (container) propagate the realize/reset/unrealize signals in the right order. Unfortunately reality is not quite as simple and indeed here however you have a busless device that:
- doesn't have a container (the container is just /machine/unattached or similar).
- triggers a reset for something else that is not contained in it (the CPU) and even some DMA.
Quoted 8 lines
Quoted 4 lines
If you really want to add the magic
qemu_register_reset, you should at least do one of the following:* add a matching unregister (no idea where)
You could add a boolean flag to DeviceState that is set by this registration. It can then be checked at unrealize to remove reset handler.
Yeah, I guess that would work. A few changes:
- you register the callback unconditionally for all busless devices, using qdev_reset_all_fn instead of directly dc->reset
- you do it after the "realized" property has been set successfully. Otherwise, a failed -device or device_add will also leave a dangling callback.
- add a comment that this is because the callback is registered because this is a busless _and_ container-less device
Paolo
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
Quoted 4 lines · Paolo Bonzini
Any abstraction we have in QEMU should have at least a parallel (though it need not be the same) in real hardware. Reset signals _do_ propagate along buses, or at least along some buses, so "debusifying" reset seems like a counterproductive goal to me.
Reset for some buses propagates along buses, but not in all cases. In any case our current "reset" semantics are "power on reset", not any kind of driven-by-hardware-reset-lines reset.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
Quoted 6 lines · Peter Maydell
Quoted 4 lines
Any abstraction we have in QEMU should have at least a parallel (though
Quoted 3 lines
it need not be the same) in real hardware. Reset signals _do_ propagate along buses, or at least along some buses, so "debusifying" reset seems like a counterproductive goal to me.
Reset for some buses propagates along buses, but not in all cases.
Agreed. But still this doesn't change the fact that "debusifying" reset is a non-goal.
Quoted 2 lines
In any case our current "reset" semantics are "power on reset", not any kind of driven-by-hardware-reset-lines reset.
It depends. There are several cases (PCI, SCSI) where qdev_reset_all is used from within the code in order to provide hardware reset semantics.
Paolo
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
In reply to Paolo BonziniQuoted 33 lines · Paolo Bonzini
Quoted 23 lines · Markus Armbruster
Quoted 21 lines · Alistair Francis
If the device being added when running
qdev_device_add()has a reset function, register it so that it can be called.Signed-off-by Alistair Francis <[email protected]>---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)qdev-monitor.c +2
-0@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); }Quoted 8 lines
qdev-monitor.c DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) · line 568
Quoted 1 line
qdev-monitor.c +0
-0id =qemu_opts_id (opts);This looks wrong to me.
You stuff all the device reset methods into the global
reset_handlerslist, where they get called in some semi-random order. This breaks when there are reset order dependencies between devices, e.g. between a device and the bus it plugs into.
Quoted 4 lines
There is no bus here, see the "if" above the one that's being added.
However, what devices have done so far is to register/unregister the reset in the realize/unrealize methods, and I suggest doing the same.
Shows that our support for bus-less devices is still in its infancy.
For me, a device has a number of external connectors that need to be wired up. A qdev bus is merely a standardized package of such connectors. For historical reasons, buses are all qdev has, with a special sysbus for everything that cannot be shoehorned into a single bus. To work with individual connectors, you have to drop into C (ignoring the weird "platform bus" sysbus thing Alex added). Support for doing that at configuration rather than code level would be nice.
[...]
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
In reply to Markus ArmbrusterAm 18.02.2016 um 10:56 schrieb Markus Armbruster:
Quoted 27 lines
Quoted 14 lines · Alistair Francis
If the device being added when running
qdev_device_add()has a reset function, register it so that it can be called.Signed-off-by Alistair Francis <[email protected]>---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)qdev-monitor.c +0
-0qdev-monitor.c DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) · line 568
Quoted 8 lines
qdev-monitor.c +2
-0@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); } id =qemu_opts_id (opts);This looks wrong to me.
Quoted 7 lines
You stuff all the device reset methods into the global
reset_handlerslist, where they get called in some semi-random order. This breaks when there are reset order dependencies between devices, e.g. between a device and the bus it plugs into.Propagating the reset signal to all the devices is a qdev problem. Copying Andreas for further insight.
We had a similar discussion for s390x, and I had started a big reset refactoring, but we agreed to go for a stop-gap solution for 2.5. The recently posted hw/core/bus.c refactoring originated from that branch.
The overall idea was that for buses reset propagates along buses (so yes, NACK to this patch), but where no bus exists it shall propagate to QOM children, too.
So Alistair, if you have a device that needs a reset while not sitting on a bus, please register the register hook in your device's realize hook for now.
Regards,
Andreas
Signature · 2 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH v1 1/2] qdev-monitor.c: Register reset function if the device has one
Quoted 40 lines · Andreas Färber
Am 18.02.2016 um 10:56 schrieb Markus Armbruster:
Quoted 27 lines
Quoted 14 lines · Alistair Francis
If the device being added when running
qdev_device_add()has a reset function, register it so that it can be called.Signed-off-by Alistair Francis <[email protected]>---
qdev-monitor.c | 2 ++ 1 file changed, 2 insertions(+)qdev-monitor.c +0
-0qdev-monitor.c DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) · line 568
Quoted 8 lines
qdev-monitor.c +2
-0@@ -561,6 +561,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)if (bus) {qdev_set_parent_bus (dev, bus); + }else if (dc->reset) { +qemu_register_reset ((void (*)(void *))dc->reset, dev); } id =qemu_opts_id (opts);This looks wrong to me.
Quoted 7 lines
You stuff all the device reset methods into the global
reset_handlerslist, where they get called in some semi-random order. This breaks when there are reset order dependencies between devices, e.g. between a device and the bus it plugs into.Propagating the reset signal to all the devices is a qdev problem. Copying Andreas for further insight.
We had a similar discussion for s390x, and I had started a big reset refactoring, but we agreed to go for a stop-gap solution for 2.5. The recently posted
hw/core/bus.crefactoring originated from that branch.
Quoted 7 lines
The overall idea was that for buses reset propagates along buses (so yes, NACK to this patch), but where no bus exists it shall propagate to QOM children, too.
So Alistair, if you have a device that needs a reset while not sitting on a bus, please register the register hook in your device's realize hook for now.
Ok, that is fair. I have moved the reset register/unregister to the realise/unrealise functions in V2.
Thanks,
Alistair
Quoted 8 lines
Regards,
Andreas--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)
PATCH 2/2 generic-loader: Add a generic loader
In reply to Alistair FrancisAdd a generic loader to QEMU which can be used to load images or set memory values.
This only supports ARM architectures at the moment.
---
Changes since RFC:
- Add BE support
default-configs/arm-softmmu.mak | 1 + hw/misc/Makefile.objs | 2 + hw/misc/generic-loader.c | 127 +++++++++++++++++++++++++++++++++++++++ include/hw/misc/generic-loader.h | 50 +++++++++++++++ 4 files changed, 180 insertions(+) create mode 100644 hw/misc/generic-loader.c create mode 100644 include/hw/misc/generic-loader.h
default-configs/arm-softmmu.mak +1 -0
@@ -110,3 +110,4 @@ CONFIG_IOH3420=y CONFIG_I82801B11=y CONFIG_ACPI=y CONFIG_SMBIOS=y +CONFIG_LOADER=y
hw/misc/Makefile.objs +2 -0
@@ -46,3 +46,5 @@ obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o obj-$(CONFIG_PVPANIC) += pvpanic.o obj-$(CONFIG_EDU) += edu.o obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o + +obj-$(CONFIG_LOADER) += generic-loader.o
hw/misc/generic-loader.c new file +127 -0
127 lines not on this page · raw message
include/hw/misc/generic-loader.h new file +50 -0
50 lines not on this page · raw message
Signature · 1 line
2.5.0
Re: [Qemu-devel] [PATCH v1 2/2] generic-loader: Add a generic loader
Quoted 11 lines · Alistair Francis
Add a generic loader to QEMU which can be used to load images or set memory values.
This only supports ARM architectures at the moment.
Signed-off-by Alistair Francis <[email protected]>---
Changes since RFC:
- Add BE support
Quoted 5 lines
hw/misc/generic-loader.c | 127 +++++++++++++++++++++++++++++++++++++++ include/hw/misc/generic-loader.h | 50 +++++++++++++++ 4 files changed, 180 insertions(+) create mode 100644 hw/misc/generic-loader.c create mode 100644 include/hw/misc/generic-loader.h
We really ought to improve checkpatch.pl to flag patches that add new files not covered by MAINTAINERS.
hw/misc/generic-loader.c line 5
Quoted 7 lines
+++
b/hw/misc/generic-loader.chw/misc/generic-loader.c new file +5
-0@@ -0,0 +1,127 @@ +/* +* Generic Loader +* +* Copyright (C) 2014 Li Guang +* Written by Li Guang <[email protected]>
Want to claim 2016?
hw/misc/generic-loader.c line 21
Quoted 6 lines
hw/misc/generic-loader.c new file +5
-0+ +#include "hw/sysbus.h" +#include "sysemu/dma.h" +#include "hw/loader.h" +#include "hw/misc/generic-loader.h"
New .c files should include "qemu/osdep.h" first, before anything else.
Quoted 8 lines
hw/misc/generic-loader.c new file +8
-0+static void generic_loader_realize (DeviceState *dev,Error **errp) +{ +GenericLoaderState *s =GENERIC_LOADER (dev); +hwaddr entry; +int big_endian; +int size =0 ; + +if (s->cpu_nr !=CPU_NONE ) {
hw/misc/generic-loader.c line 58
Quoted 10 lines
hw/misc/generic-loader.c new file +10
-0+CPUState *cs = first_cpu; +int cpu_num =0 ; + +CPU_FOREACH (cs) { +if (cpu_num == s->cpu_nr) { + s->cpu = cs; +break ; + }else if (!CPU_NEXT (cs)) { +error_setg (errp,"Specified boot CPU#%d is non existant" , + s->cpu_nr);
s/non existant/nonexistent/
hw/misc/generic-loader.c line 5
Quoted 7 lines
hw/misc/generic-loader.c new file +6
-0+++ b/include/hw/misc/generic-loader.h@@ -0,0 +1,50 @@ +/* +* Generic Loader +* +* Copyright (C) 2014 Li Guang +* Written by Li Guang <[email protected]>
2016
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v1 2/2] generic-loader: Add a generic loader
Quoted 12 lines · Eric Blake
Quoted 10 lines · Alistair Francis
Add a generic loader to QEMU which can be used to load images or set memory values.
This only supports ARM architectures at the moment.
Signed-off-by Alistair Francis <[email protected]>---
Changes since RFC:
- Add BE support
Quoted 8 lines
Quoted 5 lines
hw/misc/generic-loader.c | 127 +++++++++++++++++++++++++++++++++++++++ include/hw/misc/generic-loader.h | 50 +++++++++++++++ 4 files changed, 180 insertions(+) create mode 100644 hw/misc/generic-loader.c create mode 100644 include/hw/misc/generic-loader.hWe really ought to improve checkpatch.pl to flag patches that add new files not covered by
MAINTAINERS.
Adding an entry for this.
Quoted 10 lines
hw/misc/generic-loader.c line 5
Quoted 8 lines
+++
b/hw/misc/generic-loader.chw/misc/generic-loader.c new file +5
-0@@ -0,0 +1,127 @@ +/* +* Generic Loader +* +* Copyright (C) 2014 Li Guang +* Written by Li Guang <[email protected]> Want to claim 2016?
Yep, I can do that. I'm never too sure when this can be changed or not. Should I add a written by as well?
Quoted 9 lines
hw/misc/generic-loader.c line 21
Quoted 7 lines
hw/misc/generic-loader.c new file +5
-0+ +#include "hw/sysbus.h" +#include "sysemu/dma.h" +#include "hw/loader.h" +#include "hw/misc/generic-loader.h" New .c files should include "
qemu/osdep.h" first, before anything else.
Adding it
Quoted 13 lines
Quoted 13 lines
hw/misc/generic-loader.c new file +12
-0+static void generic_loader_realize (DeviceState *dev,Error **errp) +{ +GenericLoaderState *s =GENERIC_LOADER (dev); +hwaddr entry; +int big_endian; +int size =0 ; + +if (s->cpu_nr !=CPU_NONE ) { +CPUState *cs = first_cpu; +int cpu_num =0 ; + +CPU_FOREACH (cs) {
Quoted 8 lines
hw/misc/generic-loader.c line 58
Quoted 6 lines
hw/misc/generic-loader.c new file +6
-0+if (cpu_num == s->cpu_nr) { + s->cpu = cs; +break ; + }else if (!CPU_NEXT (cs)) { +error_setg (errp,"Specified boot CPU#%d is non existant" , + s->cpu_nr);s/non existant/nonexistent/
Thanks, fixed
Thanks,
Alistair
Quoted 16 lines
hw/misc/generic-loader.c line 5
Quoted 9 lines
hw/misc/generic-loader.c new file +6
-0+++ b/include/hw/misc/generic-loader.h@@ -0,0 +1,50 @@ +/* +* Generic Loader +* +* Copyright (C) 2014 Li Guang +* Written by Li Guang <[email protected]> 2016
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v1 2/2] generic-loader: Add a generic loader
Quoted 10 lines · Alistair Francis
Quoted 9 lines
hw/misc/generic-loader.c line 5
Quoted 7 lines
+++
b/hw/misc/generic-loader.chw/misc/generic-loader.c new file +5
-0@@ -0,0 +1,127 @@ +/* +* Generic Loader +* +* Copyright (C) 2014 Li Guang +* Written by Li Guang <[email protected]> Want to claim 2016?
Quoted 2 lines
Yep, I can do that. I'm never too sure when this can be changed or not. Should I add a written by as well?
I'm not a lawyer, so my response may not be authoritative; in particular, your employer may have specific rules that you must follow for any code you submit that was written on your employer's time, and that trumps anything I say here (that is, trust your lawyers more than you trust me).
But in general, I tend to go by the simple rule of listing the first year that any of the code was first developed (if you are copying significant portions from some other file, then use the starting year from that file, even if your file didn't exist back then), through the current year, if my change is non-trivial (more than 10 lines, or altering an interface), while ignoring the issue for trivial things (such as fixing a typo, or doing a bulk search-and-replace across the tree). As for an authorship line, I tend to omit those (they quickly go stale, and git history is sufficient for a much more accurate picture); the copyright line is more important legally than any author line.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v1 2/2] generic-loader: Add a generic loader
Quoted 21 lines · Eric Blake
Quoted 10 lines · Alistair Francis
Quoted 9 lines
hw/misc/generic-loader.c line 5
Quoted 7 lines
+++
b/hw/misc/generic-loader.chw/misc/generic-loader.c new file +5
-0@@ -0,0 +1,127 @@ +/* +* Generic Loader +* +* Copyright (C) 2014 Li Guang +* Written by Li Guang <[email protected]> Want to claim 2016?
Quoted 2 lines
Yep, I can do that. I'm never too sure when this can be changed or not. Should I add a written by as well?
I'm not a lawyer, so my response may not be authoritative; in particular, your employer may have specific rules that you must follow for any code you submit that was written on your employer's time, and that trumps anything I say here (that is, trust your lawyers more than you trust me).
Quoted 10 lines
But in general, I tend to go by the simple rule of listing the first year that any of the code was first developed (if you are copying significant portions from some other file, then use the starting year from that file, even if your file didn't exist back then), through the current year, if my change is non-trivial (more than 10 lines, or altering an interface), while ignoring the issue for trivial things (such as fixing a typo, or doing a bulk search-and-replace across the tree). As for an authorship line, I tend to omit those (they quickly go stale, and git history is sufficient for a much more accurate picture); the copyright line is more important legally than any author line.
Ok, I have added a Xilinx copyright line and not bothered with a written by line.
Thanks for your help.
Thanks,
Alistair
Quoted 5 lines
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v1 2/2] generic-loader: Add a generic loader
In reply to Alistair FrancisQuoted 5 lines · Alistair Francis
hw/misc/generic-loader.c new file +5
-0+static void generic_loader_reset (DeviceState *dev) +{ +GenericLoaderState *s =GENERIC_LOADER (dev); + +if (s->cpu) {
hw/misc/generic-loader.c line 39
Quoted 10 lines
hw/misc/generic-loader.c new file +10
-0+CPUClass *cc =CPU_GET_CLASS (s->cpu); +cpu_reset (s->cpu); + cc->set_pc (s->cpu, s->addr); + } + +if (s->data_len) { +dma_memory_write ((s->cpu ? s->cpu : first_cpu)->as, s->addr, &s->data, + s->data_len); + } +}
What happens if I accidentally make "data-len" bigger than sizeof(s->data)? I think some bounds checking is needed?
Hollis Blanchard
Mentor Graphics Emulation Division
Re: [Qemu-devel] [PATCH v1 2/2] generic-loader: Add a generic loader
Quoted 14 lines · Hollis Blanchard
Quoted 13 lines · Alistair Francis
hw/misc/generic-loader.c new file +12
-0+static void generic_loader_reset (DeviceState *dev) +{ +GenericLoaderState *s =GENERIC_LOADER (dev); + +if (s->cpu) { +CPUClass *cc =CPU_GET_CLASS (s->cpu); +cpu_reset (s->cpu); + cc->set_pc (s->cpu, s->addr); + } + +if (s->data_len) { + dma_memory_write((s->cpu ? s->cpu : first_cpu)->as, s->addr,
Quoted 8 lines
hw/misc/generic-loader.c line 39
Quoted 4 lines
&s->data,hw/misc/generic-loader.c new file +3
-0+ s->data_len); + } +}What happens if I accidentally make "data-len" bigger than
sizeof(s->data)? I think some bounds checking is needed?
Good point! I'll add an assert as it isn't a recoverable error.
Thanks,
Alistair
Quoted 4 lines
Hollis Blanchard
Mentor Graphics Emulation Division
Re: [Qemu-devel] [PATCH v1 2/2] generic-loader: Add a generic loader
Quoted 22 lines · Alistair Francis
Quoted 12 lines · Hollis Blanchard
Quoted 11 lines · Alistair Francis
hw/misc/generic-loader.c new file +11
-0+static void generic_loader_reset (DeviceState *dev) +{ +GenericLoaderState *s =GENERIC_LOADER (dev); + +if (s->cpu) { +CPUClass *cc =CPU_GET_CLASS (s->cpu); +cpu_reset (s->cpu); + cc->set_pc (s->cpu, s->addr); + } + +if (s->data_len) {Quoted 8 lines
hw/misc/generic-loader.c line 39
Quoted 5 lines
+ dma_memory_write((s->cpu ?
s->cpu: first_cpu)->as,s->addr,&s->data,hw/misc/generic-loader.c new file +3
-0+ s->data_len); + } +}What happens if I accidentally make "data-len" bigger than
sizeof(s->data)? I think some bounds checking is needed?
Quoted 1 line
Good point! I'll add an assert as it isn't a recoverable error.
Perhaps a more user-friendly error message would be, well, more user-friendly. :-) That could be done when reading the "data-len" property, in addition to an assert when using s->data_len.
Hollis Blanchard
Mentor Graphics Emulation Division
Re: [Qemu-devel] [PATCH v1 2/2] generic-loader: Add a generic loader
Quoted 29 lines · Hollis Blanchard
Quoted 28 lines · Alistair Francis
Quoted 24 lines · Hollis Blanchard
Quoted 14 lines · Alistair Francis
hw/misc/generic-loader.c new file +12
-0+static void generic_loader_reset (DeviceState *dev) +{ +GenericLoaderState *s =GENERIC_LOADER (dev); + +if (s->cpu) { +CPUClass *cc =CPU_GET_CLASS (s->cpu); +cpu_reset (s->cpu); + cc->set_pc (s->cpu, s->addr); + } + +if (s->data_len) { + dma_memory_write((s->cpu ? s->cpu : first_cpu)->as, s->addr,
&s->data,hw/misc/generic-loader.c line 39
Quoted 3 lines
hw/misc/generic-loader.c new file +3
-0+ s->data_len); + } +}What happens if I accidentally make "data-len" bigger than
sizeof(s->data)?
I think some bounds checking is needed?
Quoted 6 lines
Quoted 1 line
Good point! I'll add an assert as it isn't a recoverable error.
Perhaps a more user-friendly error message would be, well, more user-friendly. :-) That could be done when reading the "data-len" property, in addition to an assert when using
s->data_len.
Fair enough, there is now a more appropriate check in the realise function.
Thanks,
Alistair
Quoted 5 lines
Hollis Blanchard
Mentor Graphics Emulation Division
This cover letter has some useful explanation. Perhaps some of the contents should go into a docs/ file so other people can enjoy it too? :-)
I understand the part about loading multiple files, but why would I want to load a kernel with this loader, instead of the -kernel option?
Hollis Blanchard
Mentor Graphics Emulation Division
Quoted 37 lines · Alistair Francis
This work is based on the original work by Li Guang with extra features added by Peter C and myself.
The idea of this loader is to allow the user to load multiple images or values into QEMU at startup.
Memory values can be loaded like this: -device loader,addr=0xfd1a0104,data=0x8000000e,data-len=4
Images can be loaded like this: -device loader,file=./images/u-boot.elf,cpu=0
This can be useful and we use it a lot in Xilinx to load multiple images into a machine at creation (ATF, Kernel and DTB for example).
It can also be used to set registers.
The limiation for arch is based off settting the
ELF_ARCHmacro.The reset patch is required otherwise the reset will never be registered and the loader can't change the PC in the case of images.
Changes since RFC:
- Add support for BEAlistair Francis (2):
qdev-monitor.c: Register reset function if the device has one
generic-loader: Add a generic loaderdefault-configs/arm-softmmu.mak | 1 + hw/misc/Makefile.objs | 2 + hw/misc/generic-loader.c | 127 +++++++++++++++++++++++++++++++++++++++ include/hw/misc/generic-loader.h | 50 +++++++++++++++ qdev-monitor.c | 2 + 5 files changed, 182 insertions(+)create mode 100644
hw/misc/generic-loader.ccreate mode 100644include/hw/misc/generic-loader.h
Quoted 2 lines · Hollis Blanchard
This cover letter has some useful explanation. Perhaps some of the contents should go into a docs/ file so other people can enjoy it too? :-)
Ok, I have created a docs file based on the cover letter
Quoted 3 lines
I understand the part about loading multiple files, but why would I want to load a kernel with this loader, instead of the -kernel option?
If you just want to load a kernel, you should use the -kernel option. The advantage with this is when you need multiple images. For example:
- Load ATF and let it run
- Hands off to a simple loader that sets some registers and drops EL
- Start Linux
To be able to perform the steps above all three images and the DTB need to already be loaded. This allows us to boot Linux on ATF without u-boot. That is just one example, there are others. Mostly it comes down to using ATF and something.
We also use the register write to avoid having to run first stage boot loaders, as the kernel expects this registers to be set but we don't want to run the whole boot loader.
It also allows images to be loaded later in the boot process from the monitor. We use this for some other testing as well.
Thanks,
Alistair
Quoted 49 lines
Hollis Blanchard
Mentor Graphics Emulation DivisionQuoted 41 lines · Alistair Francis
This work is based on the original work by Li Guang with extra features added by Peter C and myself.
The idea of this loader is to allow the user to load multiple images or values into QEMU at startup.
Memory values can be loaded like this: -device
loader,addr=0xfd1a0104,data=0x8000000e,data-len=4Images can be loaded like this: -device
loader,file=./images/u-boot.elf,cpu=0This can be useful and we use it a lot in Xilinx to load multiple images into a machine at creation (ATF, Kernel and DTB for example).
It can also be used to set registers.
The limiation for arch is based off settting the
ELF_ARCHmacro.The reset patch is required otherwise the reset will never be registered and the loader can't change the PC in the case of images.
Changes since RFC:
- Add support for BEAlistair Francis (2):
qdev-monitor.c: Register reset function if the device has one
generic-loader: Add a generic loaderdefault-configs/arm-softmmu.mak | 1 + hw/misc/Makefile.objs | 2 + hw/misc/generic-loader.c | 127 +++++++++++++++++++++++++++++++++++++++ include/hw/misc/generic-loader.h | 50 +++++++++++++++ qdev-monitor.c | 2 + 5 files changed, 182 insertions(+)create mode 100644
hw/misc/generic-loader.ccreate mode 100644include/hw/misc/generic-loader.h
Quoted 11 lines · Alistair Francis
Quoted 2 lines · Hollis Blanchard
I understand the part about loading multiple files, but why would I want to load a kernel with this loader, instead of the -kernel option?
If you just want to load a kernel, you should use the -kernel option. The advantage with this is when you need multiple images. For example:
- Load ATF and let it run
- Hands off to a simple loader that sets some registers and drops EL
- Start Linux
Quoted 4 lines
To be able to perform the steps above all three images and the DTB need to already be loaded. This allows us to boot Linux on ATF without u-boot. That is just one example, there are others. Mostly it comes down to using ATF and something.
FWIW, the ATF stack we have for the 'virt' board uses the semihosting API to load the later stage blobs. (That's not saying we don't need this series.)
thanks
-- PMM