Re: [Qemu-devel] [PATCH] hw/misc/blob-loader: add a generic blob loader

[Li Guang]

Peter Crosthwaite wrote:
> On Thu, Jan 2, 2014 at 3:35 PM, Li Guang<address@hidden>  wrote:
>    
> > this blob loader will be used to load a specified
> > blob into a specified RAM address.
> >
> >      
> Suggested-by: Peter Crosthwaite<address@hidden>
>
>    
> > Signed-off-by: Li Guang<address@hidden>
> > ---
> > it can be used now for allwinner-a10, like:
> > "-device blob-loader,addr=0x43000000,file=/path/script.bin"
> >
> > reference:
> > http://linux-sunxi.org/Sunxi-tools
> >
> > script file address:
> > http://dl.dbank.com/c00aonvlmw
> >
> > Thanks to Peter Crosthwaite for the idea!
> > ---
> >   default-configs/arm-softmmu.mak |    2 +
> >   hw/misc/Makefile.objs           |    2 +
> >   hw/misc/blob-loader.c           |   75 +++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 79 insertions(+), 0 deletions(-)
> >   create mode 100644 hw/misc/blob-loader.c
> >
> > diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> > index ce1d620..50c71a6 100644
> > --- a/default-configs/arm-softmmu.mak
> > +++ b/default-configs/arm-softmmu.mak
> > @@ -87,3 +87,5 @@ CONFIG_INTEGRATOR_DEBUG=y
> >   CONFIG_ALLWINNER_A10_PIT=y
> >   CONFIG_ALLWINNER_A10_PIC=y
> >   CONFIG_ALLWINNER_A10=y
> > +
> > +CONFIG_BLOB_LOADER=y
> >      
> This shouldn't be arm specific. I would make the argument that the
> blob loader has global validity.
>
>    
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index f674365..df28288 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -42,3 +42,5 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
> >   obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> >
> >   obj-$(CONFIG_PVPANIC) += pvpanic.o
> > +
> > +obj-$(CONFIG_BLOB_LOADER) += blob-loader.o
> > diff --git a/hw/misc/blob-loader.c b/hw/misc/blob-loader.c
> > new file mode 100644
> > index 0000000..d7f1408
> > --- /dev/null
> > +++ b/hw/misc/blob-loader.c
> > @@ -0,0 +1,75 @@
> > +#include "hw/sysbus.h"
> > +#include "hw/devices.h"
> > +#include "hw/loader.h"
> > +#include "qemu/error-report.h"
> > +
> > +typedef struct BlobLoaderState {
> >      
> /*<  private>/*
>
>    
> > +    DeviceState qdev;
> >      
> parent_obj;
>
> /*<  public>/*
>
>    
> > +    uint32_t addr;
> >      
> uint64_t or hwaddr. Blob loading shouldn't be limited to 32 bit.
>
>    
> > +    char *file;
> > +} BlobLoaderState;
> > +
> > +#define TYPE_BLOB_LOADER "blob-loader"
> > +#define BLOB_LOADER(obj) OBJECT_CHECK(BlobLoaderState, (obj), TYPE_BLOB_LOADER)
> > +
> > +static Property blob_loader_props[] = {
> > +    DEFINE_PROP_UINT32("addr", BlobLoaderState, addr, 0),
> > +    DEFINE_PROP_STRING("file", BlobLoaderState, file),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void do_blob_load(BlobLoaderState *s)
> > +{
> > +    char *file_name;
> > +    int file_size;
> > +
> > +    if  (s->file == NULL) {
> > +        error_report("please spicify a file for blob loader\n");
> > +        return;
> >      
> Should you exit(1)? Better yet, return true for error and ..
>
>    
> > +    }
> > +    file_name = qemu_find_file(QEMU_FILE_TYPE_BIOS, s->file);
> > +    if (file_name == NULL) {
> > +        error_report("can't find %s\n", s->file);
> > +        return;
> > +    }
> > +    file_size = get_image_size(file_name);
> > +    if (file_size<  0) {
> > +        error_report("can't get file size of %s\n", file_name);
> > +        return;
> > +    }
> > +    if (load_image_targphys(file_name, s->addr, file_size)<  0) {
> > +        error_report("can't load %s\n", file_name);
> > +        return;
> > +    }
> > +}
> > +
> > +static int blob_loader_init(DeviceState *dev)
> > +{
> > +    BlobLoaderState *s = BLOB_LOADER(dev);
> > +
> > +    do_blob_load(s);
> >      
> exit(1) here.
>
>    
> > +    return 0;
> > +}
> > +
> > +static void blob_loader_class_init(ObjectClass *klass, void *data)
> >      
> s/klass/oc.
>
>    
will fix all above,
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->props = blob_loader_props;
> > +    dc->desc = "blob loader";
> > +    dc->init = blob_loader_init;
> >      
> I'm wondering whether blob loading is actually a reset step not an
> init. Will doing it at init will play foul with VMSD, as your blob
> loader will trample non-reset state on machine restore?
>
>
>    

why will trample non-reset state on machine restore?

Thanks!

> > +}
> > +
> > +static TypeInfo blob_loader_info = {
> > +    .name                  = TYPE_BLOB_LOADER,
> > +    .parent                = TYPE_DEVICE,
> > +    .instance_size         = sizeof(BlobLoaderState),
> > +    .class_init            = blob_loader_class_init,
> > +};
> > +
> > +static void blob_loader_register_type(void)
> > +{
> > +    type_register_static(&blob_loader_info);
> > +}
> > +
> > +type_init(blob_loader_register_type)
> > --
> > 1.7.2.5
> >
> >
> >      
>
>