diff mbox series

[11/15] rx: move BIOS load from MCU to board

Message ID 20201026143028.3034018-12-pbonzini@redhat.com
State New
Headers show
Series remove bios_name variable | expand

Commit Message

Paolo Bonzini Oct. 26, 2020, 2:30 p.m. UTC
The ROM loader state is global and not part of the MCU, and the
BIOS is in machine->firmware.  So just like the kernel case,
load it in the board.

Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/rx/rx-gdbsim.c | 7 +++++++
 hw/rx/rx62n.c     | 9 ---------
 2 files changed, 7 insertions(+), 9 deletions(-)

Comments

Alex Bennée Oct. 26, 2020, 5:24 p.m. UTC | #1
Paolo Bonzini <pbonzini@redhat.com> writes:

> The ROM loader state is global and not part of the MCU, and the

> BIOS is in machine->firmware.  So just like the kernel case,

> load it in the board.

>

> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

> ---

>  hw/rx/rx-gdbsim.c | 7 +++++++

>  hw/rx/rx62n.c     | 9 ---------

>  2 files changed, 7 insertions(+), 9 deletions(-)

>

> diff --git a/hw/rx/rx-gdbsim.c b/hw/rx/rx-gdbsim.c

> index 417ec0564b..040006c1c5 100644

> --- a/hw/rx/rx-gdbsim.c

> +++ b/hw/rx/rx-gdbsim.c

> @@ -142,6 +142,13 @@ static void rx_gdbsim_init(MachineState *machine)

>              /* Set dtb address to R1 */

>              RX_CPU(first_cpu)->env.regs[1] = SDRAM_BASE + dtb_offset;

>          }

> +    } else {

> +        if (machine->firmware) {

> +            rom_add_file_fixed(machine->firmware, RX62N_CFLASH_BASE, 0);

> +        } else if (!qtest_enabled()) {

> +            error_report("No bios or kernel specified");

> +            exit(1);

> +        }

>      }

>  }

>  

> diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c

> index 6eb4eea700..17ec73fc7b 100644

> --- a/hw/rx/rx62n.c

> +++ b/hw/rx/rx62n.c

> @@ -245,15 +245,6 @@ static void rx62n_realize(DeviceState *dev, Error **errp)

>                             rxc->rom_flash_size, &error_abort);

>      memory_region_add_subregion(s->sysmem, RX62N_CFLASH_BASE, &s->c_flash);

>  

> -    if (!s->kernel) {

> -        if (bios_name) {

> -            rom_add_file_fixed(bios_name, RX62N_CFLASH_BASE, 0);

> -        }  else if (!qtest_enabled()) {

> -            error_report("No bios or kernel specified");

> -            exit(1);

> -        }

> -    }

> -


I'm confused because on the face of it these are two different models.
I'll defer to the domain expert on this one.

-- 
Alex Bennée
Paolo Bonzini Oct. 26, 2020, 5:34 p.m. UTC | #2
On 26/10/20 18:24, Alex Bennée wrote:
> 

> Paolo Bonzini <pbonzini@redhat.com> writes:

> 

>> The ROM loader state is global and not part of the MCU, and the

>> BIOS is in machine->firmware.  So just like the kernel case,

>> load it in the board.

>>

>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>

>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

>> ---

>>  hw/rx/rx-gdbsim.c | 7 +++++++

>>  hw/rx/rx62n.c     | 9 ---------

>>  2 files changed, 7 insertions(+), 9 deletions(-)

>>

>> diff --git a/hw/rx/rx-gdbsim.c b/hw/rx/rx-gdbsim.c

>> index 417ec0564b..040006c1c5 100644

>> --- a/hw/rx/rx-gdbsim.c

>> +++ b/hw/rx/rx-gdbsim.c

>> @@ -142,6 +142,13 @@ static void rx_gdbsim_init(MachineState *machine)

>>              /* Set dtb address to R1 */

>>              RX_CPU(first_cpu)->env.regs[1] = SDRAM_BASE + dtb_offset;

>>          }

>> +    } else {

>> +        if (machine->firmware) {

>> +            rom_add_file_fixed(machine->firmware, RX62N_CFLASH_BASE, 0);

>> +        } else if (!qtest_enabled()) {

>> +            error_report("No bios or kernel specified");

>> +            exit(1);

>> +        }

>>      }

>>  }

>>  

>> diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c

>> index 6eb4eea700..17ec73fc7b 100644

>> --- a/hw/rx/rx62n.c

>> +++ b/hw/rx/rx62n.c

>> @@ -245,15 +245,6 @@ static void rx62n_realize(DeviceState *dev, Error **errp)

>>                             rxc->rom_flash_size, &error_abort);

>>      memory_region_add_subregion(s->sysmem, RX62N_CFLASH_BASE, &s->c_flash);

>>  

>> -    if (!s->kernel) {

>> -        if (bios_name) {

>> -            rom_add_file_fixed(bios_name, RX62N_CFLASH_BASE, 0);

>> -        }  else if (!qtest_enabled()) {

>> -            error_report("No bios or kernel specified");

>> -            exit(1);

>> -        }

>> -    }

>> -

> 

> I'm confused because on the face of it these are two different models.

> I'll defer to the domain expert on this one.


rx62n is the SoC, rx-gdbsim.c instead includes the rx62n7 and rx62n8
machines.

Paolo
Philippe Mathieu-Daudé Oct. 26, 2020, 7 p.m. UTC | #3
On 10/26/20 6:34 PM, Paolo Bonzini wrote:
> On 26/10/20 18:24, Alex Bennée wrote:

>>

>> Paolo Bonzini <pbonzini@redhat.com> writes:

>>

>>> The ROM loader state is global and not part of the MCU, and the

>>> BIOS is in machine->firmware.  So just like the kernel case,

>>> load it in the board.

>>>

>>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>

>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

>>> ---

>>>   hw/rx/rx-gdbsim.c | 7 +++++++

>>>   hw/rx/rx62n.c     | 9 ---------

>>>   2 files changed, 7 insertions(+), 9 deletions(-)

>>>

>>> diff --git a/hw/rx/rx-gdbsim.c b/hw/rx/rx-gdbsim.c

>>> index 417ec0564b..040006c1c5 100644

>>> --- a/hw/rx/rx-gdbsim.c

>>> +++ b/hw/rx/rx-gdbsim.c

>>> @@ -142,6 +142,13 @@ static void rx_gdbsim_init(MachineState *machine)

>>>               /* Set dtb address to R1 */

>>>               RX_CPU(first_cpu)->env.regs[1] = SDRAM_BASE + dtb_offset;

>>>           }

>>> +    } else {

>>> +        if (machine->firmware) {

>>> +            rom_add_file_fixed(machine->firmware, RX62N_CFLASH_BASE, 0);

>>> +        } else if (!qtest_enabled()) {

>>> +            error_report("No bios or kernel specified");

>>> +            exit(1);

>>> +        }

>>>       }

>>>   }

>>>   

>>> diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c

>>> index 6eb4eea700..17ec73fc7b 100644

>>> --- a/hw/rx/rx62n.c

>>> +++ b/hw/rx/rx62n.c

>>> @@ -245,15 +245,6 @@ static void rx62n_realize(DeviceState *dev, Error **errp)

>>>                              rxc->rom_flash_size, &error_abort);

>>>       memory_region_add_subregion(s->sysmem, RX62N_CFLASH_BASE, &s->c_flash);

>>>   

>>> -    if (!s->kernel) {

>>> -        if (bios_name) {

>>> -            rom_add_file_fixed(bios_name, RX62N_CFLASH_BASE, 0);


The acceptance test fails. There is some odd reset order problem,
eventually related to this discussion "CPU reset handler priority":
https://www.mail-archive.com/qemu-devel@nongnu.org/msg686362.html

>>> -        }  else if (!qtest_enabled()) {

>>> -            error_report("No bios or kernel specified");

>>> -            exit(1);

>>> -        }

>>> -    }

>>> -

>>

>> I'm confused because on the face of it these are two different models.

>> I'll defer to the domain expert on this one.


Yoshinori started to clean that here:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg734135.html

> 

> rx62n is the SoC, rx-gdbsim.c instead includes the rx62n7 and rx62n8

> machines.

> 

> Paolo

>
diff mbox series

Patch

diff --git a/hw/rx/rx-gdbsim.c b/hw/rx/rx-gdbsim.c
index 417ec0564b..040006c1c5 100644
--- a/hw/rx/rx-gdbsim.c
+++ b/hw/rx/rx-gdbsim.c
@@ -142,6 +142,13 @@  static void rx_gdbsim_init(MachineState *machine)
             /* Set dtb address to R1 */
             RX_CPU(first_cpu)->env.regs[1] = SDRAM_BASE + dtb_offset;
         }
+    } else {
+        if (machine->firmware) {
+            rom_add_file_fixed(machine->firmware, RX62N_CFLASH_BASE, 0);
+        } else if (!qtest_enabled()) {
+            error_report("No bios or kernel specified");
+            exit(1);
+        }
     }
 }
 
diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c
index 6eb4eea700..17ec73fc7b 100644
--- a/hw/rx/rx62n.c
+++ b/hw/rx/rx62n.c
@@ -245,15 +245,6 @@  static void rx62n_realize(DeviceState *dev, Error **errp)
                            rxc->rom_flash_size, &error_abort);
     memory_region_add_subregion(s->sysmem, RX62N_CFLASH_BASE, &s->c_flash);
 
-    if (!s->kernel) {
-        if (bios_name) {
-            rom_add_file_fixed(bios_name, RX62N_CFLASH_BASE, 0);
-        }  else if (!qtest_enabled()) {
-            error_report("No bios or kernel specified");
-            exit(1);
-        }
-    }
-
     /* Initialize CPU */
     object_initialize_child(OBJECT(s), "cpu", &s->cpu, TYPE_RX62N_CPU);
     qdev_realize(DEVICE(&s->cpu), NULL, &error_abort);