| Message ID | 9f5f44878dc0f60b073201e657d6e4dcc940f68c.1593456926.git.balaton@eik.bme.hu |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On 6/29/20 8:55 PM, BALATON Zoltan wrote: > OpenBIOS gets RAM size via fw_cfg but rhe original board firmware Typo "the". > detects RAM using SPD data so generate and add SDP eeproms to cover as EEPROMs? > much RAM as possible to describe with SPD (this may be less than the > actual ram_size due to SDRAM size constraints). > > This patch is more complex as it should be which I intend to fix once > agreement can be made on how to get back the necessary functionality > removed by previous patches. See in this thread: > https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg08710.html > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/ppc/mac_oldworld.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c > index 6276973c95..6a27287c9f 100644 > --- a/hw/ppc/mac_oldworld.c > +++ b/hw/ppc/mac_oldworld.c > @@ -34,6 +34,7 @@ > #include "hw/input/adb.h" > #include "sysemu/sysemu.h" > #include "net/net.h" > +#include "hw/i2c/smbus_eeprom.h" > #include "hw/isa/isa.h" > #include "hw/pci/pci.h" > #include "hw/pci/pci_host.h" > @@ -104,6 +105,8 @@ static void ppc_heathrow_init(MachineState *machine) > DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; > void *fw_cfg; > uint64_t tbfreq; > + uint8_t *spd_data[3] = {}; > + I2CBus *i2c_bus; > > /* init CPUs */ > for (i = 0; i < smp_cpus; i++) { > @@ -121,8 +124,16 @@ static void ppc_heathrow_init(MachineState *machine) > "maximum 2047 MB", ram_size / MiB); > exit(1); > } > - > memory_region_add_subregion(get_system_memory(), 0, machine->ram); > + for (i = 0; i < 3; i++) { 3 -> ARRAY_SIZE(spd_data) > + int size_left = ram_size - i * 512 * MiB; > + if (size_left > 0) { > + uint32_t s = size_left / MiB; > + s = (s > 512 ? 512 : s); MIN()? > + s = 1U << (31 - clz32(s)); > + spd_data[i] = spd_data_generate(SDR, s * MiB); > + } > + } > > /* allocate and load firmware ROM */ > memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE, > @@ -302,6 +313,12 @@ static void ppc_heathrow_init(MachineState *machine) > macio_ide_init_drives(macio_ide, &hd[MAX_IDE_DEVS]); > > dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda")); > + i2c_bus = I2C_BUS(qdev_get_child_bus(dev, "i2c")); > + for (i = 0; i < 3; i++) { 3 -> ARRAY_SIZE(spd_data) > + if (spd_data[i]) { > + smbus_eeprom_init_one(i2c_bus, 0x50 + i, spd_data[i]); > + } > + } > adb_bus = qdev_get_child_bus(dev, "adb.0"); > dev = qdev_new(TYPE_ADB_KEYBOARD); > qdev_realize_and_unref(dev, adb_bus, &error_fatal); >
On Tue, 13 Oct 2020, Philippe Mathieu-Daudé wrote: > On 6/29/20 8:55 PM, BALATON Zoltan wrote: >> OpenBIOS gets RAM size via fw_cfg but rhe original board firmware > > Typo "the". > >> detects RAM using SPD data so generate and add SDP eeproms to cover as > > EEPROMs? > >> much RAM as possible to describe with SPD (this may be less than the >> actual ram_size due to SDRAM size constraints). >> >> This patch is more complex as it should be which I intend to fix once >> agreement can be made on how to get back the necessary functionality >> removed by previous patches. See in this thread: >> https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg08710.html >> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> hw/ppc/mac_oldworld.c | 19 ++++++++++++++++++- >> 1 file changed, 18 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c >> index 6276973c95..6a27287c9f 100644 >> --- a/hw/ppc/mac_oldworld.c >> +++ b/hw/ppc/mac_oldworld.c >> @@ -34,6 +34,7 @@ >> #include "hw/input/adb.h" >> #include "sysemu/sysemu.h" >> #include "net/net.h" >> +#include "hw/i2c/smbus_eeprom.h" >> #include "hw/isa/isa.h" >> #include "hw/pci/pci.h" >> #include "hw/pci/pci_host.h" >> @@ -104,6 +105,8 @@ static void ppc_heathrow_init(MachineState *machine) >> DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; >> void *fw_cfg; >> uint64_t tbfreq; >> + uint8_t *spd_data[3] = {}; >> + I2CBus *i2c_bus; >> /* init CPUs */ >> for (i = 0; i < smp_cpus; i++) { >> @@ -121,8 +124,16 @@ static void ppc_heathrow_init(MachineState *machine) >> "maximum 2047 MB", ram_size / MiB); >> exit(1); >> } >> - >> memory_region_add_subregion(get_system_memory(), 0, machine->ram); >> + for (i = 0; i < 3; i++) { > > 3 -> ARRAY_SIZE(spd_data) > >> + int size_left = ram_size - i * 512 * MiB; >> + if (size_left > 0) { >> + uint32_t s = size_left / MiB; >> + s = (s > 512 ? 512 : s); > > MIN()? Thanks for the review. (You forgot to cc Mark so maybe he missed the other R-b tags but hopefully patchwork will pick these up.) I plan to rewrite this patch eliminating this part and un-breaking spd_data_generate() instead to allow it to signal an error again so we don't need to duplicate part of it here to avoid aborting. (You may remember the thread where this was discussed in, I don't have a link to it now.) So just disregard this one patch for now, I'll take your other suggestions in account when submitting a rewritten version. This and the WIP CUDA i2c patches are not yet ready to be merged and you had alternative suggestion for [v7,6/7] i2c: Match parameters of i2c_start_transfer and i2c_send_recv, but was that ever merged? Do you plan to resubmit or send a ping for that? It may have been lost somewhere because I have those mails in my mailbox but could not find it in Patchew or Patchwork. Other than these 3 patches in the series others (1-5) should be OK and hope most of those could be taken by Mark now in the upcoming Mac fixes series to reduce the size of outstanding patches then I can fix up or rewrite the remaining ones. I'm still not sure what to do about the sign extension problem in include/hw/elf_ops.h that was holding back patch 1 so if anyone has a suggestion please tell me or feel free to propose any fix for it, I'm not confident enough to break the elf loader. Regards, BALATON Zoltan >> + s = 1U << (31 - clz32(s)); >> + spd_data[i] = spd_data_generate(SDR, s * MiB); >> + } >> + } >> /* allocate and load firmware ROM */ >> memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE, >> @@ -302,6 +313,12 @@ static void ppc_heathrow_init(MachineState *machine) >> macio_ide_init_drives(macio_ide, &hd[MAX_IDE_DEVS]); >> dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda")); >> + i2c_bus = I2C_BUS(qdev_get_child_bus(dev, "i2c")); >> + for (i = 0; i < 3; i++) { > > 3 -> ARRAY_SIZE(spd_data) > >> + if (spd_data[i]) { >> + smbus_eeprom_init_one(i2c_bus, 0x50 + i, spd_data[i]); >> + } >> + } >> adb_bus = qdev_get_child_bus(dev, "adb.0"); >> dev = qdev_new(TYPE_ADB_KEYBOARD); >> qdev_realize_and_unref(dev, adb_bus, &error_fatal); >> > > >
On Wed, 14 Oct 2020, BALATON Zoltan via wrote: > On Tue, 13 Oct 2020, Philippe Mathieu-Daudé wrote: >> On 6/29/20 8:55 PM, BALATON Zoltan wrote: >>> This patch is more complex as it should be which I intend to fix once >>> agreement can be made on how to get back the necessary functionality >>> removed by previous patches. See in this thread: >>> https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg08710.html >>> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> --- > > I plan to rewrite this patch eliminating this part and un-breaking > spd_data_generate() instead to allow it to signal an error again so we don't > need to duplicate part of it here to avoid aborting. (You may remember the > thread where this was discussed in, I don't have a link to it now.) So just Oh, I actually have the link in the commit message... Maybe I should also read what I write sometimes. Regards, BALATON Zoltan
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c index 6276973c95..6a27287c9f 100644 --- a/hw/ppc/mac_oldworld.c +++ b/hw/ppc/mac_oldworld.c @@ -34,6 +34,7 @@ #include "hw/input/adb.h" #include "sysemu/sysemu.h" #include "net/net.h" +#include "hw/i2c/smbus_eeprom.h" #include "hw/isa/isa.h" #include "hw/pci/pci.h" #include "hw/pci/pci_host.h" @@ -104,6 +105,8 @@ static void ppc_heathrow_init(MachineState *machine) DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; void *fw_cfg; uint64_t tbfreq; + uint8_t *spd_data[3] = {}; + I2CBus *i2c_bus; /* init CPUs */ for (i = 0; i < smp_cpus; i++) { @@ -121,8 +124,16 @@ static void ppc_heathrow_init(MachineState *machine) "maximum 2047 MB", ram_size / MiB); exit(1); } - memory_region_add_subregion(get_system_memory(), 0, machine->ram); + for (i = 0; i < 3; i++) { + int size_left = ram_size - i * 512 * MiB; + if (size_left > 0) { + uint32_t s = size_left / MiB; + s = (s > 512 ? 512 : s); + s = 1U << (31 - clz32(s)); + spd_data[i] = spd_data_generate(SDR, s * MiB); + } + } /* allocate and load firmware ROM */ memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE, @@ -302,6 +313,12 @@ static void ppc_heathrow_init(MachineState *machine) macio_ide_init_drives(macio_ide, &hd[MAX_IDE_DEVS]); dev = DEVICE(object_resolve_path_component(OBJECT(macio), "cuda")); + i2c_bus = I2C_BUS(qdev_get_child_bus(dev, "i2c")); + for (i = 0; i < 3; i++) { + if (spd_data[i]) { + smbus_eeprom_init_one(i2c_bus, 0x50 + i, spd_data[i]); + } + } adb_bus = qdev_get_child_bus(dev, "adb.0"); dev = qdev_new(TYPE_ADB_KEYBOARD); qdev_realize_and_unref(dev, adb_bus, &error_fatal);
OpenBIOS gets RAM size via fw_cfg but rhe original board firmware detects RAM using SPD data so generate and add SDP eeproms to cover as much RAM as possible to describe with SPD (this may be less than the actual ram_size due to SDRAM size constraints). This patch is more complex as it should be which I intend to fix once agreement can be made on how to get back the necessary functionality removed by previous patches. See in this thread: https://lists.nongnu.org/archive/html/qemu-devel/2020-06/msg08710.html Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/ppc/mac_oldworld.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)