Re: [Qemu-devel] [PATCH] sun4m: Add FCode ROM for TCX framebuffer

[Andreas Färber]

Am 21.08.2013 00:41, schrieb Peter Maydell:
> On 20 August 2013 23:25, Mark Cave-Ayland <address@hidden> wrote:
>> Upstream OpenBIOS now implements SBus probing in order to determine the
>> contents of a physical bus slot, which is required to allow OpenBIOS to
>> identify the framebuffer without help from the fw_cfg interface.
>>
>> SBus probing works by detecting the presence of an FCode program
>> (effectively tokenised Forth) at the base address of each slot, and if
>> present executes it so that it creates its own device node in the
>> OpenBIOS device tree.
>>
>> The FCode ROM is generated as part of the OpenBIOS build and should
>> generally be updated at the same time.
> 
>>  hw/display/tcx.c     |    7 +++++++
>>  hw/sparc/sun4m.c     |   32 +++++++++++++++++++++++++-------
>>  pc-bios/QEMU,tcx.bin |  Bin 0 -> 1242 bytes
> 
> Shouldn't this blob come in the same patch as an update to some
> git module, so that we keep track of the sources used to build
> the blob?

I concur. Independent of how to order the .gitmodules update, this patch
is missing Makefile support to actually copy the new binary from
OpenBIOS build to the location it is being added to as binary here.

>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
> 
>> +    fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, TCX_ROM_FILE);
>> +    if (fcode_filename) {
>> +        ret = load_image_targphys(fcode_filename, addr, FCODE_MAX_ROM_SIZE);
>> +    }
> 
> This looks like the wrong place for this -- surely the tcx device
> should load its own fcode blob, not defer to the board code
> to do it?
> 
> (For that matter, presumably if this is an SBus device then
> the offsets of the ROM, DAC, etc etc are all fixed relative to
> the base address of the SBus slot, and the tcx device itself
> should be creating a container with all its component parts
> at the right offset. But that's not an issue for this patch.)

I vaguely recall Mark telling me that SBus is not really
qdev'ified/QOM'ified, right?

PCI devices have support for ROM files, too, and I think they just set
the file name and generic PCI code takes care of the actual loading.
Maybe we would want to do the same for SBus? We're not in a rush yet so
getting this designed right probably only takes a week or so...

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg