On Thu, Jun 23, 2022 at 10:16 AM Cédric Le Goater <c...@kaod.org> wrote:
> On 6/23/22 17:28, Patrick Venture wrote: > > > > > > On Wed, Jun 22, 2022 at 10:48 AM Jae Hyun Yoo <quic_jaeh...@quicinc.com > <mailto:quic_jaeh...@quicinc.com>> wrote: > > > > From: Graeme Gregory <quic_ggreg...@quicinc.com <mailto: > quic_ggreg...@quicinc.com>> > > > > The FRU device uses the index 0 device on bus IF_NONE. > > > > -drive file=$FRU,format=raw,if=none > > > > file must match FRU size of 128k > > > > Signed-off-by: Graeme Gregory <quic_ggreg...@quicinc.com <mailto: > quic_ggreg...@quicinc.com>> > > --- > > hw/arm/aspeed.c | 22 +++++++++++++++++----- > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > > index 785cc543d046..36d6b2c33e48 100644 > > --- a/hw/arm/aspeed.c > > +++ b/hw/arm/aspeed.c > > @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState > *bmc) > > */ > > } > > > > +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr, > uint32_t rsize) > > +{ > > + I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr); > > + DeviceState *dev = DEVICE(i2c_dev); > > + /* Use First Index for DC-SCM FRU */ > > + DriveInfo *dinfo = drive_get(IF_NONE, 0, 0); > > + > > + qdev_prop_set_uint32(dev, "rom-size", rsize); > > + > > + if (dinfo) { > > + qdev_prop_set_drive(dev, "drive", > blk_by_legacy_dinfo(dinfo)); > > + } > > + > > + i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort); > > +} > > > > > > We've sent a similar patch up for the at24c but in its own file -- but > looking at this, we could likely expand it to suite our cases as well - is > there a reason it's named qcom_dc_scm_fru_init? Presumably that's to > handle the drive_get parameters. If you make it use `drive_get(IF_NONE, > bus, unit);` You'll be able to associate a drive via parameters like you > aim to. > > > I have seen various attempts to populate the Aspeed eeproms with > data. The simplest is the g220a_bmc_i2c_init() approach. Some are > generating C code from the .bin files and compiling the result in > QEMU. Some were generating elf sections, linking them in QEMU and > passing the pointer as an eeprom buf. > > The drive approach is the best I think, but the device/drive > association depends on the drive order on the command line when > devices are created by the platform. > > We could may be extend the GenericLoaderState for eeproms ? > or introduce a routine which would look for a file under a (pc-bios) > per-machine directory and fill the eeprom buf with its contents ? > > Any idea ? > So we have this in our eeprom_at24c module because we use it with Aspeed and Nuvoton: void at24c_eeprom_init_one(I2CBus *i2c_bus, int bus, uint8_t addr, uint32_t rsize, int unit_number) { I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr); DeviceState *dev = DEVICE(i2c_dev); BlockInterfaceType type = IF_NONE; DriveInfo *dinfo; dinfo = drive_get(type, bus, unit_number); if (dinfo) { qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo)); } qdev_prop_set_uint32(dev, "rom-size", rsize); i2c_slave_realize_and_unref(i2c_dev, i2c_bus, &error_abort); } With this style call in the board: snippet from downstream version of https://github.com/qemu/qemu/blob/master/hw/arm/npcm7xx_boards.c#L305 that we still need to finish upstreaming: <snip> /* mb_fru */ at24c_eeprom_init_one(npcm7xx_i2c_get_bus(soc, 5), 5, 0x50, 8192, 0); <snip> /* fan_fru */ at24c_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 2), 5, 0x51, 8192, 1); /* hsbp_fru */ at24c_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 3), 5, 0x52, 8192, 2); <snip> And then we call it with the bus and unit, the pair of those has to be unique for the drive parameter to work: -drive file=/export/hda3/tmp/mb_fru@50 ,if=none,bus=5,unit=0,snapshot=off,format=raw -drive file=/export/hda3/tmp/fan_fru@51 ,if=none,bus=5,unit=1,snapshot=off,format=raw -drive file=/export/hda3/tmp/hsbp_fru@52 ,if=none,bus=5,unit=2,snapshot=off,format=raw The above code snippet is what, I'm fairly certain we had sent up for review before, and we can send it again if you want. > Thanks, > > C. >