diff mbox series

[v7,8/8] mac_oldworld: Add SPD data to cover RAM

Message ID 9f5f44878dc0f60b073201e657d6e4dcc940f68c.1593456926.git.balaton@eik.bme.hu
State Superseded
Headers show
Series None | expand

Commit Message

BALATON Zoltan June 29, 2020, 6:55 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Oct. 13, 2020, 7:50 p.m. UTC | #1
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);
>
Xingtao Yao (Fujitsu)" via Oct. 13, 2020, 10:15 p.m. UTC | #2
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);

>> 

>

>

>
Xingtao Yao (Fujitsu)" via Oct. 13, 2020, 10:28 p.m. UTC | #3
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 mbox series

Patch

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);