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

[Li Guang]

Li Guang wrote:
> Peter Crosthwaite wrote:
> > On Mon, Jan 6, 2014 at 2:22 PM, Li Guang<address@hidden>  
> > wrote:
> > > Peter Crosthwaite wrote:
> > > > On Mon, Jan 6, 2014 at 1:55 PM, Li Guang<address@hidden>   
> > > > wrote:
> > > >
> > > > > Li Guang wrote:
> > > > >
> > > > > > Paolo Bonzini wrote:
> > > > > >
> > > > > > > Il 02/01/2014 11:51, Peter Crosthwaite ha scritto:
> > > > > > >
> > > > > > > > > > No, please use "realize" and avoid init.  This way you can 
> > > > > > > > > > use an
> > > > > > > > > > Error*
> > > > > > > > > > to report the error.
> > > > > > > > > >
> > > > > > > > > > Also, the actual load_image_targphys call probably should be 
> > > > > > > > > > done in
> > > > > > > > > > a
> > > > > > > > > > reset handler, not at realize time.
> > > > > > > > Ok I think that settles it. The actual blobbing needs to happen at
> > > > > > > > reset time. Perhaps the correct approach is to do as much as 
> > > > > > > > possible
> > > > > > > > (file-path / address sanitsation etc) at realize time, then 
> > > > > > > > only the
> > > > > > > > actual blob load happens at reset. Going on what Paolo said, I 
> > > > > > > > think
> > > > > > > > for this device ::init is actually a nop.
> > > > > > > Yeah, also because init is in fact a legacy interface to realize.
> > > > > > >
> > > > > > > Paolo
> > > > > > Ok, thanks!
> > > > > Sorry, seems load blob at reset handler can't do the right job,
> > > > > while the same action can play very well at init or realize.
> > > > What's the exact problem with the reset idea?
> > > code snippet:
> > > static void blob_loader_reset(DeviceState *dev)
> > > {
> > >      BlobLoaderState *s = BLOB_LOADER(dev);
> > >
> > >      if (load_image_targphys(s->file, s->hwaddr, MAX_BLOB_SIZE)<  0) {
> > >          error_report("can't load %s\n", s->file);
> > >          exit(1);
> > >      }
> > > }
> > >
> > > if it is device reset handler, no result,
> > > kernel doesn't find and parse blob,
> > > if it is called at device realize phase,
> > > it works.
> > Need to figure out why I think. There's no fundamental problem here
> > AFAIK. It must be a bug somewhere.
> >
> > > Thanks!
> > >
> > >
> > > > > any suggestion to figure out this problem?
> > Run QEMU in GDB and break on your new reset function to see if it is
> > ever called. If not have a look into QOM/qdev to see how resets work
> > and how that plays with -device args. If yes, have a look into
> > load_image_targphys and see why that's not working.
>
> reset handler definitely be called, and
> load_image_targphys has no problem,
> I guess it may be impacted by other RAM related codes,
> just can't figure out it quickly.

Ok, I found the problem finally,
load_image_targphys depends on rom_reset to take effect,
if it's located at reset handler, rom_reset is called before it,
it surely failed to do the right job.

so, I have to write my own code to load blob into RAM.

Thanks!