Add a generic loader
v10 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).
Tested with the latest Xilinx ZynqMP machine, if I enable EL3 and EL2 I can boot ATF through to u-boot using the loader to load the images.
It can also be used to set registers.
This patch series makes the load_elf() function more generic by not requiring an architecture. It also adds new functions load_elf_as(), load_uimage_as and load_image_targphys_as which allows custom AddressSpaces when loading images.
V9:
- Logic and documentation corrections
- Add address space loading support for uImages and targphys
V8:
- Allow custom AddressSpaces when loading images
- Move ELF architecture handling code
- Rebase
- Corrections to loading code
- Corrections to documentation
V7:
- Fix typo in comment
- Rebase
V6:
- Add error checking
V5:
- Rebase
V4:
- Re-write documentation
- Allow the loader to work with every architecture
- Move the file to hw/core
- Increase the maximum number of CPUs
- Make the CPU operations conditional
- Convert the cpu option to cpu-num
- Require the user to specify endianess
V2:
- Add an entry to the maintainers file
- Add some documentation
- Perform bounds checking on the data_len
- Register and unregister the reset in the realise/unrealise
Changes since RFC:
- Add support for BE
Alistair Francis (8):
loader: Allow ELF loader to auto-detect the ELF arch
loader: Use the specified MemoryRegion
loader: Allow a custom AddressSpace when loading ROMs
loader: Add AddressSpace loading support to ELFs
loader: Add AddressSpace loading support to uImages
loader: Add AddressSpace loading support to targphys
generic-loader: Add a generic loader
docs: Add a generic loader explanation document
MAINTAINERS | 6 ++ docs/generic-loader.txt | 81 ++++++++++++++++ hw/core/Makefile.objs | 2 + hw/core/generic-loader.c | 195 +++++++++++++++++++++++++++++++++++++++ hw/core/loader.c | 98 +++++++++++++++----- include/hw/core/generic-loader.h | 46 +++++++++ include/hw/elf_ops.h | 10 +- include/hw/loader.h | 73 ++++++++++++++- 8 files changed, 483 insertions(+), 28 deletions(-) create mode 100644 docs/generic-loader.txt create mode 100644 hw/core/generic-loader.c create mode 100644 include/hw/core/generic-loader.h
Signature · 1 line
2.7.4
PATCH v10 1/8 loader: Allow ELF loader to auto-detect the ELF arch
If the caller didn't specify an architecture for the ELF machine the load_elf() function will auto detect it based on the ELF file.
---
V10:
- Base checks on the EM_NONE macro
V9:
- Update documentation
V8:
- Move into load_elf64/load_elf32
V7:
- Fix typo
include/hw/elf_ops.h | 5 +++++ include/hw/loader.h | 2 ++ 2 files changed, 7 insertions(+)
include/hw/elf_ops.h +5 -0
@@ -280,6 +280,11 @@ static int glue(load_elf, SZ)(const char *name, int fd,glue (bswap_ehdr,SZ )(&ehdr); } +if (elf_machine <=EM_NONE ) { +/* The caller didn't specify an ARCH, we can figure it out */ + elf_machine = ehdr.e_machine; + } +switch (elf_machine) {case EM_PPC64 :if (ehdr.e_machine !=EM_PPC64 ) {
include/hw/loader.h +2 -0
@@ -68,6 +68,8 @@ const char *load_elf_strerror(int error);* load will fail if the target ELF does not match. Some architectures * have some architecture-specific behaviours that come into effect when * their particular values for @elf_machine are set. +* If @elf_machine is EM_NONE then the machine type will be read from the +* ELF header and no checks will be carried out against the machine type. */ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t),
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v10 1/8] loader: Allow ELF loader to auto-detect the ELF arch
Quoted 4 lines · Alistair Francis
If the caller didn't specify an architecture for the ELF machine the
load_elf()function will auto detect it based on the ELF file.Signed-off-by Alistair Francis <[email protected]>
thanks
-- PMM
PATCH v10 2/8 loader: Use the specified MemoryRegion
In reply to Alistair FrancisPrevously the specified MemoryRegion was ignored during the rom register reset. This patch uses the rom MemoryRegion is avaliable.
---
hw/core/loader.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
hw/core/loader.c +2 -1
@@ -1045,7 +1045,8 @@ int rom_check_and_register_reset(void) } addr = rom->addr; addr += rom->romsize; - section =memory_region_find (get_system_memory (), rom->addr,1 ); + section =memory_region_find (rom->mr ? rom->mr :get_system_memory (), + rom->addr,1 ); rom->isrom =int128_nz (section.size) &&memory_region_is_rom (section.mr);memory_region_unref (section.mr); }
Signature · 1 line
2.7.4
PATCH v10 3/8 loader: Allow a custom AddressSpace when loading ROMs
In reply to Alistair FrancisWhen loading ROMs allow the caller to specify an AddressSpace to use for the load.
---
V10:
- Set the rom address space instead of leaving it NULL
- Cleanup ordering logic
V9:
- Fixup the ROM ordering
- Don't allow address space and memory region to be specified
V8:
- Introduce an RFC version of AddressSpace loading support
hw/core/loader.c | 52 ++++++++++++++++++++++++++++++++++++++++------------ include/hw/elf_ops.h | 2 +- include/hw/loader.h | 10 ++++++---- 3 files changed, 47 insertions(+), 17 deletions(-)
hw/core/loader.c +40 -12
@@ -777,6 +777,7 @@ struct Rom {uint8_t *data;MemoryRegion *mr; +AddressSpace *as;int isrom;char *fw_dir;char *fw_file;
@@ -788,6 +789,11 @@ struct Rom {static FWCfgState *fw_cfg; static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms); +static inline bool rom_order_compare (Rom *rom,Rom *item) +{ +return rom->addr >= item->addr; +} +static void rom_insert (Rom *rom) {Rom *item;
@@ -796,12 +802,22 @@ static void rom_insert(Rom *rom)hw_error ("ROM images must be loaded at startup\n" ); } -/* list is ordered by load address */ +/* The user didn't specify an address space, this is the default */ +if (!rom->as) { + rom->as = &address_space_memory; + } + +/* List is ordered by load address in the same address space */ QTAILQ_FOREACH(item, &roms, next) { -if (rom->addr >= item->addr) -continue ; -QTAILQ_INSERT_BEFORE (item, rom, next); -return ; +if (rom->as == item->as) { +if (rom_order_compare (rom, item)) { +QTAILQ_INSERT_AFTER (&roms, item, rom, next); +return ; + }else { +QTAILQ_INSERT_BEFORE (item, rom, next); +return ; + } + } }QTAILQ_INSERT_TAIL (&roms, rom, next); }
@@ -833,16 +849,25 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char *name)int rom_add_file (const char *file,const char *fw_dir,hwaddr addr,int32_t bootindex, -bool option_rom,MemoryRegion *mr) +bool option_rom,MemoryRegion *mr, +AddressSpace *as) {MachineClass *mc =MACHINE_GET_CLASS (qdev_get_machine ());Rom *rom;int rc, fd =-1 ;char devpath[100 ]; +if (as && mr) { +fprintf (stderr,"Specifying an Address Space and Memory Region is " \ +"not valid when loading a rom\n" ); +/* We haven't allocated anything so we don't need any cleanup */ +return -1 ; + } + rom =g_malloc0 (sizeof (*rom)); rom->name =g_strdup (file); rom->path =qemu_find_file (QEMU_FILE_TYPE_BIOS , rom->name); + rom->as = as;if (rom->path ==NULL ) { rom->path =g_strdup (file); }
@@ -969,7 +994,7 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,* memory ownership of "data", so we don't have to allocate and copy the buffer. */ int rom_add_elf_program (const char *name,void *data,size_t datasize, -size_t romsize,hwaddr addr) +size_t romsize,hwaddr addr,AddressSpace *as) {Rom *rom;
@@ -979,18 +1004,19 @@ int rom_add_elf_program(const char *name, void *data, size_t datasize, rom->datasize = datasize; rom->romsize = romsize; rom->data = data; + rom->as = as;rom_insert (rom);return 0 ; }int rom_add_vga (const char *file) { -return rom_add_file (file,"vgaroms" ,0 ,-1 , true,NULL ); +return rom_add_file (file,"vgaroms" ,0 ,-1 , true,NULL ,NULL ); }int rom_add_option (const char *file,int32_t bootindex) { -return rom_add_file (file,"genroms" ,0 , bootindex, true,NULL ); +return rom_add_file (file,"genroms" ,0 , bootindex, true,NULL ,NULL ); }static void rom_reset (void *unused)
@@ -1008,8 +1034,8 @@ static void rom_reset(void *unused)void *host =memory_region_get_ram_ptr (rom->mr);memcpy (host, rom->data, rom->datasize); }else { -cpu_physical_memory_write_rom (&address_space_memory, - rom->addr, rom->data, rom->datasize); +cpu_physical_memory_write_rom (rom->as, rom->addr, rom->data, + rom->datasize); }if (rom->isrom) {/* rom needs to be written only once */
@@ -1031,12 +1057,13 @@ int rom_check_and_register_reset(void)hwaddr addr =0 ;MemoryRegionSection section;Rom *rom; +AddressSpace *as =NULL ; QTAILQ_FOREACH(rom, &roms, next) {if (rom->fw_file) {continue ; } -if (addr > rom->addr) { +if ((addr > rom->addr) && (as == rom->as)) { fprintf(stderr, "rom: requested regions overlap " "(rom %s. free=0x" TARGET_FMT_plx ", addr=0x" TARGET_FMT_plx ")\n",
@@ -1049,6 +1076,7 @@ int rom_check_and_register_reset(void) rom->addr, 1); rom->isrom =int128_nz (section.size) &&memory_region_is_rom (section.mr);memory_region_unref (section.mr); + as = rom->as; }qemu_register_reset (rom_reset,NULL ); roms_loaded =1 ;
include/hw/elf_ops.h +1 -1
@@ -405,7 +405,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,snprintf (label,sizeof (label),"phdr #%d: %s" , i, name);/* rom_add_elf_program() seize the ownership of 'data' */ -rom_add_elf_program (label, data, file_size, mem_size, addr); +rom_add_elf_program (label, data, file_size, mem_size, addr,NULL ); total_size += mem_size;if (addr < low)
include/hw/loader.h +6 -4
@@ -120,14 +120,14 @@ extern bool rom_file_has_mr;int rom_add_file (const char *file,const char *fw_dir,hwaddr addr,int32_t bootindex, -bool option_rom,MemoryRegion *mr); +bool option_rom,MemoryRegion *mr,AddressSpace *as);MemoryRegion *rom_add_blob (const char *name,const void *blob,size_t len,size_t max_len,hwaddr addr,const char *fw_file_name,FWCfgReadCallback fw_callback,void *callback_opaque);int rom_add_elf_program (const char *name,void *data,size_t datasize, -size_t romsize,hwaddr addr); +size_t romsize,hwaddr addr,AddressSpace *as);int rom_check_and_register_reset (void );void rom_set_fw (FWCfgState *f);void rom_set_order_override (int order);
@@ -137,11 +137,13 @@ void *rom_ptr(hwaddr addr);void hmp_info_roms (Monitor *mon,const QDict *qdict);#define rom_add_file_fixed (_f, _a, _i) \ - rom_add_file(_f, NULL, _a, _i, false, NULL) + rom_add_file(_f, NULL, _a, _i, false, NULL, NULL)#define rom_add_blob_fixed (_f, _b, _l, _a) \ rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL)#define rom_add_file_mr (_f, _mr, _i) \ - rom_add_file(_f, NULL, 0, _i, false, _mr) + rom_add_file(_f, NULL, 0, _i, false, _mr, NULL) +#define rom_add_file_as (_f, _as, _i) \ + rom_add_file(_f, NULL, 0, _i, false, NULL, _as)#define PC_ROM_MIN_VGA 0xc0000#define PC_ROM_MIN_OPTION 0xc8000
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v10 3/8] loader: Allow a custom AddressSpace when loading ROMs
Quoted 58 lines · Alistair Francis
When loading ROMs allow the caller to specify an AddressSpace to use for the load.
Signed-off-by Alistair Francis <[email protected]>---
V10:
- Set the rom address space instead of leaving it NULL
- Cleanup ordering logic
V9:
- Fixup the ROM ordering
- Don't allow address space and memory region to be specified
V8:
- Introduce an RFC version of AddressSpace loading supporthw/core/loader.c | 52 ++++++++++++++++++++++++++++++++++++++++------------ include/hw/elf_ops.h | 2 +- include/hw/loader.h | 10 ++++++---- 3 files changed, 47 insertions(+), 17 deletions(-)hw/core/loader.c +12
-4@@ -777,6 +777,7 @@ struct Rom {uint8_t *data;MemoryRegion *mr; +AddressSpace *as;int isrom;char *fw_dir;char *fw_file;@@ -788,6 +789,11 @@ struct Rom {static FWCfgState *fw_cfg; static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms); +static inline bool rom_order_compare (Rom *rom,Rom *item) +{ +return rom->addr >= item->addr; +} +static void rom_insert (Rom *rom) {Rom *item;@@ -796,12 +802,22 @@ static void rom_insert(Rom *rom)hw_error ("ROM images must be loaded at startup\n" ); } -/* list is ordered by load address */ +/* The user didn't specify an address space, this is the default */ +if (!rom->as) { + rom->as = &address_space_memory; + } + +/* List is ordered by load address in the same address space */ QTAILQ_FOREACH(item, &roms, next) { -if (rom->addr >= item->addr) -continue ; -QTAILQ_INSERT_BEFORE (item, rom, next);
hw/core/loader.c static void rom_insert(Rom *rom) · line 820
Quoted 10 lines
hw/core/loader.c +9
-1-return ; +if (rom->as == item->as) { +if (rom_order_compare (rom, item)) { +QTAILQ_INSERT_AFTER (&roms, item, rom, next); +return ; + }else { +QTAILQ_INSERT_BEFORE (item, rom, next); +return ; + } + }
This isn't quite what I meant, and if we're doing it like this we might as well have "rom->addr >= item->addr" opencoded here.
What I had in mind when I suggested a separate rom_order_compare() was that it should be doing the full ordering operation:
return (rom->as > item->as) ||
(rom->as == item->as && rom->addr >= item->addr);(ie it defines a total order on ROMs which for any two ROMs tells you which way round they go in the list).
and then the change in this function would just be to change the old
if (rom->addr >= item->addr)
to
if (rom_order_compare(rom, item))
This has the advantage that it's really obvious that we're maintaining a sorted array and all we've done is change the function defining our total order on ROMs.
The problem which I've realised in the course of thinking about this this evening is that the compare function I suggest above:
(a) requires a total ordering on pointers, which is not guaranteed by the C standard
(b) even if you cast the rom->as pointers all to uintptr_t (getting you defined behaviour) still gives you an ordering which is variable from run to run of QEMU, which isn't great.
Unfortunately there's no other obvious ordering on AddressSpaces (I guess you could look at their position in the address_spaces list, but that's getting pretty heavyweight.)
That said, the code as you have it in this patch doesn't sort things properly I think. Consider three ROMs with addresses 0, 100, 200, all in the same AS, inserted in that order:
* ROM-0 is added as the first and only item in the list
* ROM-100 is added in the list after ROM-0 (same AS, and rom_order_compare is true, so we do INSERT_AFTER)
* when we insert ROM-200 the first item in the list is ROM-0. This has the same AS, and rom_order_compare(ROM-200, ROM-0) is true, so we do an INSERT_AFTER.
Now the list is (ROM-0, ROM-200, ROM-100), which isn't sorted.
So our options are:
(1) fix up the open-coded code to sort properly
(2) figure out a rom_order_compare() that does give a total ordering on ROMs which isn't variable from run to run of QEMU
(3) decide that it's OK for the ROM list ordering not to be the same from run to run (or on two sides of a migration) so we can use the cast-to-uintptr_t version
I don't currently have an opinion (well, I would like 2 except I can't think of a workable definition for the comparison function). Anybody?
thanks
-- PMM
Re: [Qemu-devel] [PATCH v10 3/8] loader: Allow a custom AddressSpace when loading ROMs
Quoted 78 lines · Peter Maydell
Quoted 58 lines · Alistair Francis
When loading ROMs allow the caller to specify an AddressSpace to use for the load.
Signed-off-by Alistair Francis <[email protected]>---
V10:
- Set the rom address space instead of leaving it NULL
- Cleanup ordering logic
V9:
- Fixup the ROM ordering
- Don't allow address space and memory region to be specified
V8:
- Introduce an RFC version of AddressSpace loading supporthw/core/loader.c | 52 ++++++++++++++++++++++++++++++++++++++++------------ include/hw/elf_ops.h | 2 +- include/hw/loader.h | 10 ++++++---- 3 files changed, 47 insertions(+), 17 deletions(-)hw/core/loader.c +12
-4@@ -777,6 +777,7 @@ struct Rom {uint8_t *data;MemoryRegion *mr; +AddressSpace *as;int isrom;char *fw_dir;char *fw_file;@@ -788,6 +789,11 @@ struct Rom {static FWCfgState *fw_cfg; static QTAILQ_HEAD(, Rom) roms = QTAILQ_HEAD_INITIALIZER(roms); +static inline bool rom_order_compare (Rom *rom,Rom *item) +{ +return rom->addr >= item->addr; +} +static void rom_insert (Rom *rom) {Rom *item;@@ -796,12 +802,22 @@ static void rom_insert(Rom *rom)hw_error ("ROM images must be loaded at startup\n" ); } -/* list is ordered by load address */ +/* The user didn't specify an address space, this is the default */ +if (!rom->as) { + rom->as = &address_space_memory; + } + +/* List is ordered by load address in the same address space */ QTAILQ_FOREACH(item, &roms, next) { -if (rom->addr >= item->addr) -continue ; -QTAILQ_INSERT_BEFORE (item, rom, next);hw/core/loader.c static void rom_insert(Rom *rom) · line 820
Quoted 10 lines
hw/core/loader.c +9
-1-return ; +if (rom->as == item->as) { +if (rom_order_compare (rom, item)) { +QTAILQ_INSERT_AFTER (&roms, item, rom, next); +return ; + }else { +QTAILQ_INSERT_BEFORE (item, rom, next); +return ; + } + }This isn't quite what I meant, and if we're doing it like this we might as well have "
rom->addr>=item->addr" opencoded here.What I had in mind when I suggested a separate
rom_order_compare()was that it should be doing the full ordering operation:return (rom->as > item->as) || (rom->as == item->as && rom->addr >= item->addr);
Quoted 7 lines
(ie it defines a total order on ROMs which for any two ROMs tells you which way round they go in the list).
and then the change in this function would just be to change the old
if (rom->addr>=item->addr)
to
if (rom_order_compare(rom, item))
Aw, now I understand. I was a little confused why you wanted the extra function.
Quoted 16 lines
This has the advantage that it's really obvious that we're maintaining a sorted array and all we've done is change the function defining our total order on ROMs.
The problem which I've realised in the course of thinking about this this evening is that the compare function I suggest above:
(a) requires a total ordering on pointers, which is not guaranteed by the C standard
(b) even if you cast therom->aspointers all touintptr_t(getting you defined behaviour) still gives you an ordering which is variable from run to run of QEMU, which isn't great.
Unfortunately there's no other obvious ordering on AddressSpaces (I guess you could look at their position in theaddress_spaceslist, but that's getting pretty heavyweight.)
Quoted 10 lines
That said, the code as you have it in this patch doesn't sort things properly I think. Consider three ROMs with addresses 0, 100, 200, all in the same AS, inserted in that order:
* ROM-0 is added as the first and only item in the list
* ROM-100 is added in the list after ROM-0 (same AS, androm_order_compareis true, so we doINSERT_AFTER)
* when we insert ROM-200 the first item in the list is ROM-0. This has the same AS, androm_order_compare(ROM-200, ROM-0)is true, so we do anINSERT_AFTER.
Now the list is (ROM-0, ROM-200, ROM-100), which isn't sorted.
I missed that case. Good catch.
Quoted 9 lines
So our options are:
(1) fix up the open-coded code to sort properly
(2) figure out arom_order_compare()that does give a total ordering on ROMs which isn't variable from run to run of QEMU
(3) decide that it's OK for the ROM list ordering not to be the same from run to run (or on two sides of a migration) so we can use the cast-to-uintptr_t version
Remember that the original reason to order these ROMs was to ensure that there were no overlaps. So I don't think that it matters too much if the address space ordering changes.
One solution though to sort them is to sort the address spaces by name. Are they guaranteed to have a unique names?
Thanks,
Alistair
Quoted 8 lines
I don't currently have an opinion (well, I would like 2 except I can't think of a workable definition for the comparison function). Anybody?
thanks
-- PMM
PATCH v10 4/8 loader: Add AddressSpace loading support to ELFs
In reply to Alistair FrancisAdd a new function load_elf_as() that allows the caller to specify an AddressSpace to use when loading the ELF. The original load_elf() function doesn't have any change in functionality.
---
V8:
- Introduce an RFC version of AddressSpace support
hw/core/loader.c | 16 ++++++++++++++-- include/hw/elf_ops.h | 5 +++-- include/hw/loader.h | 13 ++++++++++++- 3 files changed, 29 insertions(+), 5 deletions(-)
hw/core/loader.c +14 -2
@@ -417,6 +417,18 @@ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t), uint64_t *highaddr, int big_endian, int elf_machine, int clear_lsb, int data_swab) { + return load_elf_as(filename, translate_fn, translate_opaque, pentry, +lowaddr ,highaddr ,big_endian ,elf_machine ,clear_lsb , +data_swab ,NULL ); +} + +/* return < 0 if error, otherwise the number of bytes loaded in memory */ +int load_elf_as (const char *filename, +uint64_t (*translate_fn)(void *,uint64_t ), +void *translate_opaque,uint64_t *pentry,uint64_t *lowaddr, +uint64_t *highaddr,int big_endian,int elf_machine, +int clear_lsb,int data_swab,AddressSpace *as) +{int fd, data_order, target_data_order, must_swab, ret =ELF_LOAD_FAILED ;uint8_t e_ident[EI_NIDENT ];
@@ -455,11 +467,11 @@ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t),if (e_ident[EI_CLASS ] ==ELFCLASS64 ) { ret =load_elf64 (filename, fd, translate_fn, translate_opaque, must_swab, pentry, lowaddr, highaddr, elf_machine, clear_lsb, - data_swab); + data_swab, as); }else { ret =load_elf32 (filename, fd, translate_fn, translate_opaque, must_swab, pentry, lowaddr, highaddr, elf_machine, clear_lsb, - data_swab); + data_swab, as); } fail:
include/hw/elf_ops.h +3 -2
@@ -263,7 +263,8 @@ static int glue(load_elf, SZ)(const char *name, int fd,void *translate_opaque, int must_swab, uint64_t *pentry, uint64_t *lowaddr, uint64_t *highaddr, - int elf_machine, int clear_lsb, int data_swab) + int elf_machine, int clear_lsb, int data_swab, + AddressSpace *as) { struct elfhdr ehdr;struct elf_phdr *phdr =NULL , *ph;
@@ -405,7 +406,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,snprintf (label,sizeof (label),"phdr #%d: %s" , i, name);/* rom_add_elf_program() seize the ownership of 'data' */ -rom_add_elf_program (label, data, file_size, mem_size, addr,NULL ); +rom_add_elf_program (label, data, file_size, mem_size, addr, as); total_size += mem_size;if (addr < low)
include/hw/loader.h +12 -1
@@ -45,7 +45,7 @@ int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz);#define ELF_LOAD_WRONG_ENDIAN -4const char *load_elf_strerror (int error); -/** load_elf: +/** load_elf_as: * @filename: Path of ELF file * @translate_fn: optional function to translate load addresses * @translate_opaque: opaque data passed to @translate_fn
@@ -59,6 +59,8 @@ const char *load_elf_strerror(int error);* @data_swab: Set to order of byte swapping for data. 0 for no swap, 1 * for swapping bytes within halfwords, 2 for bytes within * words and 3 for within doublewords. +* @as: The AddressSpace to load the ELF to. The value of address_space_memory +* is used if nothing is supplied here. * * Load an ELF file's contents to the emulated system's address space. * Clients may optionally specify a callback to perform address
@@ -71,7 +73,16 @@ const char *load_elf_strerror(int error);* If @elf_machine is EM_NONE then the machine type will be read from the * ELF header and no checks will be carried out against the machine type. */ +int load_elf_as (const char *filename, +uint64_t (*translate_fn)(void *,uint64_t ), +void *translate_opaque,uint64_t *pentry,uint64_t *lowaddr, +uint64_t *highaddr,int big_endian,int elf_machine, +int clear_lsb,int data_swab,AddressSpace *as); +/** load_elf: +* Same as load_elf_as(), but doesn't allow the caller to specify an +* AddressSpace. +*/ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t), void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr, uint64_t *highaddr, int big_endian, int elf_machine,
Signature · 1 line
2.7.4
PATCH v10 5/8 loader: Add AddressSpace loading support to uImages
In reply to Alistair FrancisAdd a new function load_uimage_as() that allows the caller to specify an AddressSpace to use when loading the uImage. The original load_uimage() function doesn't have any change in functionality.
---
V10:
- Add comment about the function in the header file.
hw/core/loader.c | 17 +++++++++++++---- include/hw/loader.h | 26 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-)
hw/core/loader.c +13 -4
@@ -581,7 +581,7 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,static int load_uboot_image (const char *filename,hwaddr *ep,hwaddr *loadaddr,int *is_linux,uint8_t image_type,uint64_t (*translate_fn)(void *,uint64_t ), -void *translate_opaque) +void *translate_opaque,AddressSpace *as) {int fd;int size;
@@ -682,7 +682,7 @@ static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr, hdr->ih_size = bytes; } -rom_add_blob_fixed (filename, data, hdr->ih_size, address); +rom_add_blob_fixed_as (filename, data, hdr->ih_size, address, as); ret = hdr->ih_size;
@@ -698,14 +698,23 @@ int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr, void *translate_opaque) {return load_uboot_image (filename, ep, loadaddr, is_linux,IH_TYPE_KERNEL , - translate_fn, translate_opaque); + translate_fn, translate_opaque,NULL ); +} + +int load_uimage_as (const char *filename,hwaddr *ep,hwaddr *loadaddr, +int *is_linux, +uint64_t (*translate_fn)(void *,uint64_t ), +void *translate_opaque,AddressSpace *as) +{ +return load_uboot_image (filename, ep, loadaddr, is_linux,IH_TYPE_KERNEL , + translate_fn, translate_opaque, as); }/* Load a ramdisk. */ int load_ramdisk (const char *filename,hwaddr addr,uint64_t max_sz) {return load_uboot_image (filename,NULL , &addr,NULL ,IH_TYPE_RAMDISK , -NULL ,NULL ); +NULL ,NULL ,NULL ); }/* Load a gzip-compressed kernel to a dynamically allocated buffer. */
include/hw/loader.h +26 -0
@@ -102,6 +102,30 @@ void load_elf_hdr(const char *filename, void *hdr, bool *is64, Error **errp);int load_aout (const char *filename,hwaddr addr,int max_sz,int bswap_needed,hwaddr target_page_size); + +/** load_uimage_as: +* @filename: Path of uimage file +* @ep: Populated with program entry point. Ignored if NULL. +* @loadaddr: Populated with the load address. Ignored if NULL. +* @is_linux: Is set to true if the image loaded is Linux. Ignored if NULL. +* @translate_fn: optional function to translate load addresses +* @translate_opaque: opaque data passed to @translate_fn +* @as: The AddressSpace to load the ELF to. The value of address_space_memory +* is used if nothing is supplied here. +* +* Loads a u-boot image into memory. +* +* Returns the size of the loaded image on success, -1 otherwise. +*/ +int load_uimage_as (const char *filename,hwaddr *ep, +hwaddr *loadaddr,int *is_linux, +uint64_t (*translate_fn)(void *,uint64_t ), +void *translate_opaque,AddressSpace *as); + +/** load_uimage: +* Same as load_uimage_as(), but doesn't allow the caller to specify an +* AddressSpace. +*/ int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr, int *is_linux, uint64_t (*translate_fn)(void *, uint64_t),
@@ -155,6 +179,8 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict); rom_add_file(_f, NULL, 0, _i, false, _mr, NULL)#define rom_add_file_as (_f, _as, _i) \ rom_add_file(_f, NULL, 0, _i, false, NULL, _as) +#define rom_add_blob_fixed_as (_f, _b, _l, _a, _as) \ + rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, _as)#define PC_ROM_MIN_VGA 0xc0000#define PC_ROM_MIN_OPTION 0xc8000
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v10 5/8] loader: Add AddressSpace loading support to uImages
Quoted 9 lines · Alistair Francis
Add a new function
load_uimage_as()that allows the caller to specify an AddressSpace to use when loading the uImage. The originalload_uimage()function doesn't have any change in functionality.Signed-off-by Alistair Francis <[email protected]>---
V10:
- Add comment about the function in the header file.
thanks
-- PMM
PATCH v10 6/8 loader: Add AddressSpace loading support to targphys
In reply to Alistair FrancisAdd a new function load_image_targphys_as() that allows the caller to specify an AddressSpace to use when loading a targphys. The original load_image_targphys() function doesn't have any change in functionality.
---
V10:
- Add comment about the function in the header file.
hw/core/loader.c | 10 ++++++++-- include/hw/loader.h | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-)
hw/core/loader.c +8 -2
@@ -133,10 +133,16 @@ ssize_t read_targphys(const char *name,return did; } -/* return the size or -1 if error */ int load_image_targphys (const char *filename,hwaddr addr,uint64_t max_sz) { +return load_image_targphys_as (filename, addr, max_sz,NULL ); +} + +/* return the size or -1 if error */ +int load_image_targphys_as (const char *filename, +hwaddr addr,uint64_t max_sz,AddressSpace *as) +{int size; size =get_image_size (filename);
@@ -144,7 +150,7 @@ int load_image_targphys(const char *filename,return -1 ; }if (size >0 ) { -rom_add_file_fixed (filename, addr,-1 ); +rom_add_file_fixed_as (filename, addr,-1 , as); }return size; }
include/hw/loader.h +22 -0
@@ -14,8 +14,28 @@int get_image_size (const char *filename);int load_image (const char *filename,uint8_t *addr);/* deprecated */ ssize_t load_image_size (const char *filename,void *addr,size_t size); + +/**load_image_targphys_as: +* @filename: Path to the image file +* @addr: Address to load the image to +* @max_sz: The maximum size of the image to load +* @as: The AddressSpace to load the ELF to. The value of address_space_memory +* is used if nothing is supplied here. +* +* Load a fixed image into memory. +* +* Returns the size of the loaded image on success, -1 otherwise. +*/ +int load_image_targphys_as (const char *filename, +hwaddr addr,uint64_t max_sz,AddressSpace *as); + +/** load_image_targphys: +* Same as load_image_targphys_as(), but doesn't allow the caller to specify +* an AddressSpace. +*/ int load_image_targphys (const char *filename,hwaddr ,uint64_t max_sz); +/** * load_image_mr: load an image into a memory region * @filename: Path to the image file
@@ -179,6 +199,8 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict); rom_add_file(_f, NULL, 0, _i, false, _mr, NULL)#define rom_add_file_as (_f, _as, _i) \ rom_add_file(_f, NULL, 0, _i, false, NULL, _as) +#define rom_add_file_fixed_as (_f, _a, _i, _as) \ + rom_add_file(_f, NULL, _a, _i, false, NULL, _as)#define rom_add_blob_fixed_as (_f, _b, _l, _a, _as) \ rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, _as)
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v10 6/8] loader: Add AddressSpace loading support to targphys
Quoted 7 lines · Alistair Francis
Add a new function
load_image_targphys_as()that allows the caller to specify an AddressSpace to use when loading a targphys. The originalload_image_targphys()function doesn't have any change in functionality.Signed-off-by Alistair Francis <[email protected]>---
thanks
-- PMM
PATCH v10 7/8 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.
---
V10:
- Split out the PC setting and data loading
V9:
- Fix error messages
- Updated some incorrect logic
- Add address space loading support for all image types
- Explain why the reset is manually registered
V8:
- Code corrections
- Rebase
V7:
- Rebase
V6:
- Add error checking
V5:
- Rebase
V4:
- Allow the loader to work with every architecture
- Move the file to hw/core
- Increase the maximum number of CPUs
- Make the CPU operations conditional
- Convert the cpu option to cpu-num
- Require the user to specify endianess
V3:
- Pass the ram_size to load_image_targphys()
V2:
- Add maintainers entry
- Perform bounds checking
- Register and unregister the reset in the realise/unrealise
Changes since RFC:
- Add BE support
MAINTAINERS | 6 ++ hw/core/Makefile.objs | 2 + hw/core/generic-loader.c | 195 +++++++++++++++++++++++++++++++++++++++ include/hw/core/generic-loader.h | 46 +++++++++ 4 files changed, 249 insertions(+) create mode 100644 hw/core/generic-loader.c create mode 100644 include/hw/core/generic-loader.h
MAINTAINERS +6 -0
@@ -984,6 +984,12 @@ M: Dmitry Fleytman <[email protected]> S: Maintained F: hw/net/e1000e* +Generic Loader +M: Alistair Francis <[email protected]> +S: Maintained +F: hw/core/generic-loader.c +F: include/hw/core/generic-loader.h + Subsystems ---------- Audio
hw/core/Makefile.objs +2 -0
@@ -17,3 +17,5 @@ common-obj-$(CONFIG_SOFTMMU) += loader.o common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o common-obj-$(CONFIG_SOFTMMU) += register.o common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o + +obj-$(CONFIG_SOFTMMU) += generic-loader.o
hw/core/generic-loader.c new file +195 -0
195 lines not on this page · raw message
include/hw/core/generic-loader.h new file +46 -0
46 lines not on this page · raw message
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v10 7/8] generic-loader: Add a generic loader
Quoted 101 lines · Alistair Francis
Add a generic loader to QEMU which can be used to load images or set memory values.
Signed-off-by Alistair Francis <[email protected]>---
V10:
- Split out the PC setting and data loading
V9:
- Fix error messages
- Updated some incorrect logic
- Add address space loading support for all image types
- Explain why the reset is manually registered
V8:
- Code corrections
- Rebase
V7:
- Rebase
V6:
- Add error checking
V5:
- Rebase
V4:
- Allow the loader to work with every architecture
- Move the file to hw/core
- Increase the maximum number of CPUs
- Make the CPU operations conditional
- Convert the cpu option to cpu-num
- Require the user to specify endianess
V3:
- Pass theram_sizetoload_image_targphys()
V2:
- Add maintainers entry
- Perform bounds checking
- Register and unregister the reset in the realise/unrealise
Changes since RFC:
- Add BE supportMAINTAINERS | 6 ++ hw/core/Makefile.objs | 2 + hw/core/generic-loader.c | 195 +++++++++++++++++++++++++++++++++++++++ include/hw/core/generic-loader.h | 46 +++++++++ 4 files changed, 249 insertions(+) create mode 100644 hw/core/generic-loader.c create mode 100644 include/hw/core/generic-loader.hMAINTAINERS +6
-0@@ -984,6 +984,12 @@ M: Dmitry Fleytman <[email protected]> S: Maintained F: hw/net/e1000e* +Generic Loader +M: Alistair Francis <[email protected]> +S: Maintained +F: hw/core/generic-loader.c +F: include/hw/core/generic-loader.h + Subsystems ---------- Audiohw/core/Makefile.objs +2
-0@@ -17,3 +17,5 @@ common-obj-$(CONFIG_SOFTMMU) += loader.o common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o common-obj-$(CONFIG_SOFTMMU) += register.o common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o + +obj-$(CONFIG_SOFTMMU) += generic-loader.ohw/core/generic-loader.c new file +23
-0@@ -0,0 +1,195 @@ +/* +* Generic Loader +* +* Copyright (C) 2014 Li Guang +* Copyright (C) 2016 Xilinx Inc. +* Written by Li Guang <[email protected]> +* +* This program is free software; you can redistribute it and/or modify it +* under the terms of the GNU General Public License as published by the +* Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, but WITHOUT +* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +* for more details. +*/ + +#include "qemu/osdep.h" +#include "qom/cpu.h" +#include "hw/sysbus.h" +#include "sysemu/dma.h" +#include "hw/loader.h"
hw/core/generic-loader.c line 33
Quoted 10 lines
hw/core/generic-loader.c new file +10
-0+#include "qapi/error.h" +#include "hw/core/generic-loader.h" + +#define CPU_NONE 0xFFFFFFFF + +static void generic_loader_reset (void *opaque) +{ +GenericLoaderState *s =GENERIC_LOADER (opaque); + +if (s->set_pc && s->cpu) {
This check on s->cpu means that -device loader,file=<...> won't set the PC to the entry point unless you specifically pass cpu-num=0. The easiest fix for this is just to remove it (see below).
Quoted 32 lines
hw/core/generic-loader.c new file +32
-0+CPUClass *cc =CPU_GET_CLASS (s->cpu); +cpu_reset (s->cpu); +if (cc) { + cc->set_pc (s->cpu, s->addr); + } + } + +if (s->data_len) { +assert (s->data_len <sizeof (s->data)); +dma_memory_write ((s->cpu ? s->cpu : first_cpu)->as, s->addr, &s->data, + s->data_len); + } +} + +static void generic_loader_realize (DeviceState *dev,Error **errp) +{ +GenericLoaderState *s =GENERIC_LOADER (dev); +hwaddr entry; +int big_endian; +int size =0 ; + + s->set_pc = false; + +/* Perform some error checking on the user's options */ +if (s->data || s->data_len || s->data_be) { +/* User is loading memory values */ +if (s->file) { +error_setg (errp,"Specifying a file is not supported when loading " +"memory values" ); +return ; + }else if (s->force_raw) { + error_setg(errp, "Specifying force-raw is not supported when "
hw/core/generic-loader.c line 75
Quoted 10 lines
hw/core/generic-loader.c new file +10
-0+ "loading memory values"); +return ; + }else if (!s->data || !s->data_len) { +error_setg (errp,"Both data and data-len must be specified" ); +return ; + }else if (s->data_len >8 ) { +error_setg (errp,"data-len cannot be greater then 8 bytes" ); +return ; + }else if (s->data_len >sizeof (s->data)) { +error_setg (errp,"data-len cannot be more then the data size" );
This can't ever happen, because sizeof(s->data) is always 8.
Quoted 7 lines
hw/core/generic-loader.c new file +7
-0+return ; + } + }else if (s->file || s->force_raw) { +/* User is loading an image */ +if (s->data || s->data_len || s->data_be) { +error_setg (errp,"data can not be specified when loading an " +"image" );
hw/core/generic-loader.c line 92
Quoted 10 lines
hw/core/generic-loader.c new file +10
-0+return ; + } + s->set_pc = true; + }else if (s->addr) { +/* User is setting the PC */ +if (s->data || s->data_len || s->data_be) { +error_setg (errp,"data can not be specified when setting a " +"program counter" ); +return ; + }
Failing to specify cpu_num in this case needs to be an error.
Quoted 9 lines
hw/core/generic-loader.c new file +7
-0+ s->set_pc = true; + }else { +/* Did the user specify anything? */ +error_setg (errp,"please include valid arguments" ); +return ; + } +
hw/core/generic-loader.c line 109
Quoted 10 lines
hw/core/generic-loader.c new file +10
-0+qemu_register_reset (generic_loader_reset, dev); + +if (s->cpu_num !=CPU_NONE ) { + s->cpu =qemu_get_cpu (s->cpu_num); +if (!s->cpu) { +error_setg (errp,"Specified boot CPU#%d is nonexistent" , + s->cpu_num); +return ; + } + }
If you always set s->cpu to be first_cpu in the "cpu_num is CPU_NONE" case you can avoid all the ?: everywhere you use it, and you don't need to check for 's->cpu' being non-NULL in the function where you set the PC.
Quoted 22 lines
hw/core/generic-loader.c new file +22
-0+ +#ifdef TARGET_WORDS_BIGENDIAN + big_endian =1 ; +#else + big_endian =0 ; +#endif + +if (s->file) { +if (!s->force_raw) { + size =load_elf_as (s->file,NULL ,NULL , &entry,NULL ,NULL , + big_endian,0 ,0 ,0 , + (s->cpu ? s->cpu : first_cpu)->as); + +if (size <0 ) { + size =load_uimage_as (s->file, &entry,NULL ,NULL ,NULL ,NULL , + (s->cpu ? s->cpu : first_cpu)->as); + } + } + +if (size <0 || s->force_raw) { +/* Default to the maximum size being the machine's ram size */ + size = load_image_targphys_as(s->file, s->addr, ram_size,
hw/core/generic-loader.c line 141
Quoted 10 lines
hw/core/generic-loader.c new file +10
-0+ (s->cpu ? s->cpu : first_cpu)->as); + }else { + s->addr = entry; + } + +if (size <0 ) { +error_setg (errp,"Cannot load specified image %s" , s->file); +return ; + } + }
Otherwise:
thanks
-- PMM
Re: [Qemu-devel] [PATCH v10 7/8] generic-loader: Add a generic loader
Quoted 108 lines · Peter Maydell
Quoted 107 lines · Alistair Francis
Add a generic loader to QEMU which can be used to load images or set memory values.
Signed-off-by Alistair Francis <[email protected]>---
V10:
- Split out the PC setting and data loading
V9:
- Fix error messages
- Updated some incorrect logic
- Add address space loading support for all image types
- Explain why the reset is manually registered
V8:
- Code corrections
- Rebase
V7:
- Rebase
V6:
- Add error checking
V5:
- Rebase
V4:
- Allow the loader to work with every architecture
- Move the file to hw/core
- Increase the maximum number of CPUs
- Make the CPU operations conditional
- Convert the cpu option to cpu-num
- Require the user to specify endianess
V3:
- Pass theram_sizetoload_image_targphys()
V2:
- Add maintainers entry
- Perform bounds checking
- Register and unregister the reset in the realise/unrealise
Changes since RFC:
- Add BE supportMAINTAINERS | 6 ++ hw/core/Makefile.objs | 2 + hw/core/generic-loader.c | 195 +++++++++++++++++++++++++++++++++++++++ include/hw/core/generic-loader.h | 46 +++++++++ 4 files changed, 249 insertions(+) create mode 100644 hw/core/generic-loader.c create mode 100644 include/hw/core/generic-loader.hMAINTAINERS +6
-0@@ -984,6 +984,12 @@ M: Dmitry Fleytman <[email protected]> S: Maintained F: hw/net/e1000e* +Generic Loader +M: Alistair Francis <[email protected]> +S: Maintained +F: hw/core/generic-loader.c +F: include/hw/core/generic-loader.h + Subsystems ---------- Audiohw/core/Makefile.objs +2
-0@@ -17,3 +17,5 @@ common-obj-$(CONFIG_SOFTMMU) += loader.o common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o common-obj-$(CONFIG_SOFTMMU) += register.o common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o + +obj-$(CONFIG_SOFTMMU) += generic-loader.ohw/core/generic-loader.c new file +29
-0@@ -0,0 +1,195 @@ +/* +* Generic Loader +* +* Copyright (C) 2014 Li Guang +* Copyright (C) 2016 Xilinx Inc. +* Written by Li Guang <[email protected]> +* +* This program is free software; you can redistribute it and/or modify it +* under the terms of the GNU General Public License as published by the +* Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, but WITHOUT +* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +* for more details. +*/ + +#include "qemu/osdep.h" +#include "qom/cpu.h" +#include "hw/sysbus.h" +#include "sysemu/dma.h" +#include "hw/loader.h" +#include "qapi/error.h" +#include "hw/core/generic-loader.h" + +#define CPU_NONE 0xFFFFFFFF + +static void generic_loader_reset (void *opaque)
Quoted 8 lines
hw/core/generic-loader.c line 33
Quoted 4 lines
hw/core/generic-loader.c new file +4
-0+{ +GenericLoaderState *s =GENERIC_LOADER (opaque); + +if (s->set_pc && s->cpu) {This check on
s->cpumeans that -device loader,file=<...> won't set the PC to the entry point unless you specifically pass cpu-num=0. The easiest fix for this is just to remove it (see below).
Fixed!
Quoted 37 lines
Quoted 37 lines
hw/core/generic-loader.c new file +36
-0+CPUClass *cc =CPU_GET_CLASS (s->cpu); +cpu_reset (s->cpu); +if (cc) { + cc->set_pc (s->cpu, s->addr); + } + } + +if (s->data_len) { +assert (s->data_len <sizeof (s->data)); +dma_memory_write ((s->cpu ? s->cpu : first_cpu)->as, s->addr, &s->data, + s->data_len); + } +} + +static void generic_loader_realize (DeviceState *dev,Error **errp) +{ +GenericLoaderState *s =GENERIC_LOADER (dev); +hwaddr entry; +int big_endian; +int size =0 ; + + s->set_pc = false; + +/* Perform some error checking on the user's options */ +if (s->data || s->data_len || s->data_be) { +/* User is loading memory values */ +if (s->file) { +error_setg (errp,"Specifying a file is not supported when loading " +"memory values" ); +return ; + }else if (s->force_raw) { +error_setg (errp,"Specifying force-raw is not supported when " +"loading memory values" ); +return ; + }else if (!s->data || !s->data_len) { +error_setg (errp,"Both data and data-len must be specified" );
Quoted 8 lines
hw/core/generic-loader.c line 75
Quoted 6 lines
hw/core/generic-loader.c new file +6
-0+return ; + }else if (s->data_len >8 ) { +error_setg (errp,"data-len cannot be greater then 8 bytes" ); +return ; + }else if (s->data_len >sizeof (s->data)) { +error_setg (errp,"data-len cannot be more then the data size" );This can't ever happen, because
sizeof(s->data)is always 8.
Ok, removed.
Quoted 19 lines
Quoted 8 lines
hw/core/generic-loader.c new file +7
-0+return ; + } + }else if (s->file || s->force_raw) { +/* User is loading an image */ +if (s->data || s->data_len || s->data_be) { +error_setg (errp,"data can not be specified when loading an " +"image" );hw/core/generic-loader.c line 92
Quoted 10 lines
hw/core/generic-loader.c new file +10
-0+return ; + } + s->set_pc = true; + }else if (s->addr) { +/* User is setting the PC */ +if (s->data || s->data_len || s->data_be) { +error_setg (errp,"data can not be specified when setting a " +"program counter" ); +return ; + }
Quoted 1 line
Failing to specify
cpu_numin this case needs to be an error.
Fixed.
Quoted 9 lines
Quoted 7 lines
hw/core/generic-loader.c new file +6
-0+ s->set_pc = true; + }else { +/* Did the user specify anything? */ +error_setg (errp,"please include valid arguments" ); +return ; + }
Quoted 15 lines
hw/core/generic-loader.c line 109
Quoted 11 lines
hw/core/generic-loader.c new file +11
-0+ +qemu_register_reset (generic_loader_reset, dev); + +if (s->cpu_num !=CPU_NONE ) { + s->cpu =qemu_get_cpu (s->cpu_num); +if (!s->cpu) { +error_setg (errp,"Specified boot CPU#%d is nonexistent" , + s->cpu_num); +return ; + } + }If you always set
s->cputo befirst_cpuin the "cpu_numisCPU_NONE" case you can avoid all the ?: everywhere you use it, and you don't need to check for 's->cpu' being non-NULL in the function where you set the PC.
Fixed.
Quoted 36 lines
Quoted 23 lines
hw/core/generic-loader.c new file +22
-0+ +#ifdef TARGET_WORDS_BIGENDIAN + big_endian =1 ; +#else + big_endian =0 ; +#endif + +if (s->file) { +if (!s->force_raw) { + size =load_elf_as (s->file,NULL ,NULL , &entry,NULL ,NULL , + big_endian,0 ,0 ,0 , + (s->cpu ? s->cpu : first_cpu)->as); + +if (size <0 ) { + size =load_uimage_as (s->file, &entry,NULL ,NULL ,NULL ,NULL , + (s->cpu ? s->cpu : first_cpu)->as); + } + } + +if (size <0 || s->force_raw) { +/* Default to the maximum size being the machine's ram size */ + size = load_image_targphys_as(s->file, s->addr, ram_size,hw/core/generic-loader.c line 141
Quoted 10 lines
hw/core/generic-loader.c new file +10
-0+ (s->cpu ? s->cpu : first_cpu)->as); + }else { + s->addr = entry; + } + +if (size <0 ) { +error_setg (errp,"Cannot load specified image %s" , s->file); +return ; + } + }Otherwise:
Quoted 1 line
Reviewed-by Peter Maydell <[email protected]>
Thanks!
Alistair
Quoted 4 lines
thanks
-- PMM
PATCH v10 8/8 docs: Add a generic loader explanation document
In reply to Alistair Francis---
V10:
- Split the data loading and PC setting
V9:
- Clarify the image loading options
V8:
- Improve documentation
V6:
- Fixup documentation
V4:
- Re-write to be more comprehensive
docs/generic-loader.txt | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 docs/generic-loader.txt
docs/generic-loader.txt new file +81 -0
@@ -0,0 +1,81 @@ +Copyright (c) 2016 Xilinx Inc. + +This work is licensed under the terms of the GNU GPL, version 2 or later. See +the COPYING file in the top-level directory. + + +The 'loader' device allows the user to load multiple images or values into +QEMU at startup. + +Loading Data into Memory Values +--------------------- +The loader device allows memory values to be set from the command line. This +can be done by following the syntax below: + + -device loader,addr=<addr>,data=<data>,data-len=<data-len> + [,data-be=<data-be>][,cpu-num=<cpu-num>] + + <addr> - The address to store the data in. + <data> - The value to be written to the address. The maximum size of + the data is 8 bytes. + <data-len> - The length of the data in bytes. This argument must be + included if the data argument is. + <data-be> - Set to true if the data to be stored on the guest should be + written as big endian data. The default is to write little + endian data. + <cpu-num> - The number of the CPU's address space where the data should + be loaded. If not sepcified the default address space is + used. + +For all values both hex and decimal values are allowed. By default the values +will be parsed as decimal. To use hex values the user should prefix the number +with a '0x'. + +An example of loading value 0x8000000e to address 0xfd1a0104 is: + -device loader,addr=0xfd1a0104,data=0x8000000e,data-len=4 + +Setting a CPU's Program Counter +--------------------- +The loader device allows the CPU's PC to be set from the command line. This +can be done by following the syntax below: + + -device loader,addr=<addr>,cpu-num=<cpu-num> + + <addr> - The value to use as the CPU's PC. + <cpu-num> - This will cause the CPU to be reset and the PC to be set to + the value of addr. + +For all values both hex and decimal values are allowed. By default the values +will be parsed as decimal. To use hex values the user should prefix the number +with a '0x'. + +An example of loading value 0x8000000e to address 0xfd1a0104 is: + -device loader,addr=0x8000,cpu-num=0 + +Loading Files +--------------------- +The loader device also allows files to be loaded into memory. This can be done +similarly to setting memory values. The syntax is shown below: + + -device loader,file=<file>[,addr=<addr>][,cpu-num=<cpu-num>][,force-raw=<raw>] + + <file> - A file to be loaded into memory + <addr> - The addr in memory that the file should be loaded. This is + ignored if you are using an ELF (unless force-raw is true). + This is required if you aren't loading an ELF. + <cpu-num> - This specifies the CPU that should be used. This is an + optional argument and will cause the CPU's PC to be set to + where the image is stored or in the case of an ELF file to + the value in the header. This option should only be used + for the boot image. + This will also cause the image to be written to the specified + CPUs address space. + <force-raw> - Forces the file to be treated as a raw image. This can be + used to specify the load address of ELF files. + +For all values both hex and decimal values are allowed. By default the values +will be parsed as decimal. To use hex values the user should prefix the number +with a '0x'. + +An example of loading an ELF file which CPU0 will boot is shown below: + -device loader,file=./images/boot.elf,cpu-num=0
Signature · 1 line
2.7.4
Re: [Qemu-devel] [PATCH v10 8/8] docs: Add a generic loader explanation document
Quoted 41 lines · Alistair Francis
Signed-off-by Alistair Francis <[email protected]>---
V10:
- Split the data loading and PC setting
V9:
- Clarify the image loading options
V8:
- Improve documentation
V6:
- Fixup documentation
V4:
- Re-write to be more comprehensivedocs/generic-loader.txt | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 docs/generic-loader.txtdocs/generic-loader.txt new file +18
-0@@ -0,0 +1,81 @@ +Copyright (c) 2016 Xilinx Inc. + +This work is licensed under the terms of the GNU GPL, version 2 or later. See +the COPYING file in the top-level directory. + + +The 'loader' device allows the user to load multiple images or values into +QEMU at startup. + +Loading Data into Memory Values +--------------------- +The loader device allows memory values to be set from the command line. This +can be done by following the syntax below: + + -device loader,addr=<addr>,data=<data>,data-len=<data-len> + [,data-be=<data-be>][,cpu-num=<cpu-num>] + + <addr> - The address to store the data in.
docs/generic-loader.txt line 28
Quoted 10 lines
docs/generic-loader.txt new file +10
-0+ <data> - The value to be written to the address. The maximum size of + the data is 8 bytes. + <data-len> - The length of the data in bytes. This argument must be + included if the data argument is. + <data-be> - Set to true if the data to be stored on the guest should be + written as big endian data. The default is to write little + endian data. + <cpu-num> - The number of the CPU's address space where the data should + be loaded. If not sepcified the default address space is + used.
"specified"
"the address space of the first CPU is used", or at least that's what the code does.
Quoted 8 lines
docs/generic-loader.txt new file +8
-0+ +For all values both hex and decimal values are allowed. By default the values +will be parsed as decimal. To use hex values the user should prefix the number +with a '0x'. + +An example of loading value 0x8000000e to address 0xfd1a0104 is: + -device loader,addr=0xfd1a0104,data=0x8000000e,data-len=4 +
docs/generic-loader.txt line 46
Quoted 10 lines
docs/generic-loader.txt new file +10
-0+Setting a CPU's Program Counter +--------------------- +The loader device allows the CPU's PC to be set from the command line. This +can be done by following the syntax below: + + -device loader,addr=<addr>,cpu-num=<cpu-num> + + <addr> - The value to use as the CPU's PC. + <cpu-num> - This will cause the CPU to be reset and the PC to be set to + the value of addr.
"The number of the CPU whose PC should be set to the specified value."
docs/generic-loader.txt line 34
Quoted 6 lines
docs/generic-loader.txt new file +6
-0+ +For all values both hex and decimal values are allowed. By default the values +will be parsed as decimal. To use hex values the user should prefix the number +with a '0x'. + +An example of loading value 0x8000000e to address 0xfd1a0104 is:
"An example of setting CPU 0's PC to 0x8000 is:"
Quoted 10 lines
docs/generic-loader.txt new file +10
-0+ -device loader,addr=0x8000,cpu-num=0 + +Loading Files +--------------------- +The loader device also allows files to be loaded into memory. This can be done +similarly to setting memory values. The syntax is shown below: + + -device loader,file=<file>[,addr=<addr>][,cpu-num=<cpu-num>][,force-raw=<raw>] + + <file> - A file to be loaded into memory
docs/generic-loader.txt line 72
Quoted 10 lines
docs/generic-loader.txt new file +10
-0+ <addr> - The addr in memory that the file should be loaded. This is + ignored if you are using an ELF (unless force-raw is true). + This is required if you aren't loading an ELF. + <cpu-num> - This specifies the CPU that should be used. This is an + optional argument and will cause the CPU's PC to be set to + where the image is stored or in the case of an ELF file to + the value in the header. This option should only be used + for the boot image. + This will also cause the image to be written to the specified + CPUs address space.
"CPU's".
"If not specified, the default is CPU 0."
Quoted 11 lines
docs/generic-loader.txt new file +9
-1+ <force-raw> - Forces the file to be treated as a raw image. This can be + used to specify the load address of ELF files. + +For all values both hex and decimal values are allowed. By default the values +will be parsed as decimal. To use hex values the user should prefix the number +with a '0x'. + +An example of loading an ELF file which CPU0 will boot is shown below: + -device loader,file=./images/boot.elf,cpu-num=0 --2.7.4
Otherwise
thanks
-- PMM
Re: [Qemu-devel] [PATCH v10 8/8] docs: Add a generic loader explanation document
Quoted 126 lines · Peter Maydell
Quoted 41 lines · Alistair Francis
Signed-off-by Alistair Francis <[email protected]>---
V10:
- Split the data loading and PC setting
V9:
- Clarify the image loading options
V8:
- Improve documentation
V6:
- Fixup documentation
V4:
- Re-write to be more comprehensivedocs/generic-loader.txt | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 docs/generic-loader.txtdocs/generic-loader.txt new file +18
-0@@ -0,0 +1,81 @@ +Copyright (c) 2016 Xilinx Inc. + +This work is licensed under the terms of the GNU GPL, version 2 or later. See +the COPYING file in the top-level directory. + + +The 'loader' device allows the user to load multiple images or values into +QEMU at startup. + +Loading Data into Memory Values +--------------------- +The loader device allows memory values to be set from the command line. This +can be done by following the syntax below: + + -device loader,addr=<addr>,data=<data>,data-len=<data-len> + [,data-be=<data-be>][,cpu-num=<cpu-num>] + + <addr> - The address to store the data in.docs/generic-loader.txt line 28
Quoted 10 lines
docs/generic-loader.txt new file +10
-0+ <data> - The value to be written to the address. The maximum size of + the data is 8 bytes. + <data-len> - The length of the data in bytes. This argument must be + included if the data argument is. + <data-be> - Set to true if the data to be stored on the guest should be + written as big endian data. The default is to write little + endian data. + <cpu-num> - The number of the CPU's address space where the data should + be loaded. If not sepcified the default address space is + used."specified"
"the address space of the first CPU is used", or at least that's what the code does.
Quoted 8 lines
docs/generic-loader.txt new file +8
-0+ +For all values both hex and decimal values are allowed. By default the values +will be parsed as decimal. To use hex values the user should prefix the number +with a '0x'. + +An example of loading value 0x8000000e to address 0xfd1a0104 is: + -device loader,addr=0xfd1a0104,data=0x8000000e,data-len=4 +docs/generic-loader.txt line 46
Quoted 10 lines
docs/generic-loader.txt new file +10
-0+Setting a CPU's Program Counter +--------------------- +The loader device allows the CPU's PC to be set from the command line. This +can be done by following the syntax below: + + -device loader,addr=<addr>,cpu-num=<cpu-num> + + <addr> - The value to use as the CPU's PC. + <cpu-num> - This will cause the CPU to be reset and the PC to be set to + the value of addr."The number of the CPU whose PC should be set to the specified value."
docs/generic-loader.txt line 34
Quoted 6 lines
docs/generic-loader.txt new file +6
-0+ +For all values both hex and decimal values are allowed. By default the values +will be parsed as decimal. To use hex values the user should prefix the number +with a '0x'. + +An example of loading value 0x8000000e to address 0xfd1a0104 is:"An example of setting CPU 0's PC to
0x8000is:"Quoted 10 lines
docs/generic-loader.txt new file +10
-0+ -device loader,addr=0x8000,cpu-num=0 + +Loading Files +--------------------- +The loader device also allows files to be loaded into memory. This can be done +similarly to setting memory values. The syntax is shown below: + + -device loader,file=<file>[,addr=<addr>][,cpu-num=<cpu-num>][,force-raw=<raw>] + + <file> - A file to be loaded into memorydocs/generic-loader.txt line 72
Quoted 10 lines
docs/generic-loader.txt new file +10
-0+ <addr> - The addr in memory that the file should be loaded. This is + ignored if you are using an ELF (unless force-raw is true). + This is required if you aren't loading an ELF. + <cpu-num> - This specifies the CPU that should be used. This is an + optional argument and will cause the CPU's PC to be set to + where the image is stored or in the case of an ELF file to + the value in the header. This option should only be used + for the boot image. + This will also cause the image to be written to the specified + CPUs address space."CPU's".
"If not specified, the default is CPU 0."
Quoted 11 lines
docs/generic-loader.txt new file +9
-1+ <force-raw> - Forces the file to be treated as a raw image. This can be + used to specify the load address of ELF files. + +For all values both hex and decimal values are allowed. By default the values +will be parsed as decimal. To use hex values the user should prefix the number +with a '0x'. + +An example of loading an ELF file which CPU0 will boot is shown below: + -device loader,file=./images/boot.elf,cpu-num=0 --2.7.4
Otherwise
Quoted 1 line
Reviewed-by Peter Maydell <[email protected]>
Fixed them all, thanks for the review.
Thanks,
Alistair
Quoted 4 lines
thanks
-- PMM