diff mbox series

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

Message ID 20201026143028.3034018-12-pbonzini@redhat.com (mailing list archive)
State New, archived
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.
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);