Patchwork [1/4] prep: Remove bogus BIOS size check

login
register
mail settings
Submitter Andreas Färber
Date Dec. 14, 2010, 12:49 a.m.
Message ID <1292287758-8009-2-git-send-email-andreas.faerber@web.de>
Download mbox | patch
Permalink /patch/75453/
State New
Headers show

Comments

Andreas Färber - Dec. 14, 2010, 12:49 a.m.
r3480 added this check to account for the entry vector 0xfff00100 to be
available for CPUs that need it. Today however, the NIP is not yet
initialized at this point (zero), so the check always triggers.

Cc: Hervé Poussineau <hpoussin@reactos.org>
Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/ppc_prep.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)
Alexander Graf - Dec. 19, 2010, 9:52 a.m.
On 14.12.2010, at 01:49, Andreas Färber wrote:

> r3480 added this check to account for the entry vector 0xfff00100 to be
> available for CPUs that need it. Today however, the NIP is not yet
> initialized at this point (zero), so the check always triggers.
> 
> Cc: Hervé Poussineau <hpoussin@reactos.org>
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
> hw/ppc_prep.c |    3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 1492266..6b22122 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -600,9 +600,6 @@ static void ppc_prep_init (ram_addr_t ram_size,
>     if (filename) {
>         qemu_free(filename);
>     }
> -    if (env->nip < 0xFFF80000 && bios_size < 0x00100000) {
> -        hw_error("PowerPC 601 / 620 / 970 need a 1MB BIOS\n");
> -    }
> 

The bios gets mapped to 0xfff00000, so it can at most be 0x100000 bytes long which is 1MB. The bogus part of the check is the first, the nip part.

Alex
Andreas Färber - Dec. 19, 2010, 12:26 p.m.
Am 19.12.2010 um 10:52 schrieb Alexander Graf:

> On 14.12.2010, at 01:49, Andreas Färber wrote:
>
>> r3480 added this check to account for the entry vector 0xfff00100  
>> to be
>> available for CPUs that need it. Today however, the NIP is not yet
>> initialized at this point (zero), so the check always triggers.
>>
>> Cc: Hervé Poussineau <hpoussin@reactos.org>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> ---
>> hw/ppc_prep.c |    3 ---
>> 1 files changed, 0 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
>> index 1492266..6b22122 100644
>> --- a/hw/ppc_prep.c
>> +++ b/hw/ppc_prep.c
>> @@ -600,9 +600,6 @@ static void ppc_prep_init (ram_addr_t ram_size,
>>    if (filename) {
>>        qemu_free(filename);
>>    }
>> -    if (env->nip < 0xFFF80000 && bios_size < 0x00100000) {
>> -        hw_error("PowerPC 601 / 620 / 970 need a 1MB BIOS\n");
>> -    }
>>
>
> The bios gets mapped to 0xfff00000, so it can at most be 0x100000  
> bytes long which is 1MB. The bogus part of the check is the first,  
> the nip part.

If you look at 2/4, it is checked using BIOS_SIZE define (1024 * 1024)  
further down in the more correct place. :)

At this time - 1/4 - the BIOS gets mapped to 0xffffffff minus BIOS  
size. If the entry is at 0xfffffffc, the BIOS is allowed to be smaller  
than 1 MB. So marking BIOS sizes smaller as 1 MB as error would break  
both the OHW ppc_rom.bin and the IBM 40p BIOS. Therefore it must  
simply go away.

Unless you happen to know a better place for this NIP-dependent check,  
that is. I believe the reset handling was changed at some point?

Andreas
Alexander Graf - Dec. 19, 2010, 3:27 p.m.
On 19.12.2010, at 13:26, Andreas Färber wrote:

> Am 19.12.2010 um 10:52 schrieb Alexander Graf:
> 
>> On 14.12.2010, at 01:49, Andreas Färber wrote:
>> 
>>> r3480 added this check to account for the entry vector 0xfff00100 to be
>>> available for CPUs that need it. Today however, the NIP is not yet
>>> initialized at this point (zero), so the check always triggers.
>>> 
>>> Cc: Hervé Poussineau <hpoussin@reactos.org>
>>> Cc: Alexander Graf <agraf@suse.de>
>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>> ---
>>> hw/ppc_prep.c |    3 ---
>>> 1 files changed, 0 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
>>> index 1492266..6b22122 100644
>>> --- a/hw/ppc_prep.c
>>> +++ b/hw/ppc_prep.c
>>> @@ -600,9 +600,6 @@ static void ppc_prep_init (ram_addr_t ram_size,
>>>   if (filename) {
>>>       qemu_free(filename);
>>>   }
>>> -    if (env->nip < 0xFFF80000 && bios_size < 0x00100000) {
>>> -        hw_error("PowerPC 601 / 620 / 970 need a 1MB BIOS\n");
>>> -    }
>>> 
>> 
>> The bios gets mapped to 0xfff00000, so it can at most be 0x100000 bytes long which is 1MB. The bogus part of the check is the first, the nip part.
> 
> If you look at 2/4, it is checked using BIOS_SIZE define (1024 * 1024) further down in the more correct place. :)
> 
> At this time - 1/4 - the BIOS gets mapped to 0xffffffff minus BIOS size. If the entry is at 0xfffffffc, the BIOS is allowed to be smaller than 1 MB. So marking BIOS sizes smaller as 1 MB as error would break both the OHW ppc_rom.bin and the IBM 40p BIOS. Therefore it must simply go away.
> 
> Unless you happen to know a better place for this NIP-dependent check, that is. I believe the reset handling was changed at some point?

Hrm, maybe. The reset vector is board dependent and different CPU typed recommend different default values. So I agree - just get rid of this check.


Alex

Patch

diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 1492266..6b22122 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -600,9 +600,6 @@  static void ppc_prep_init (ram_addr_t ram_size,
     if (filename) {
         qemu_free(filename);
     }
-    if (env->nip < 0xFFF80000 && bios_size < 0x00100000) {
-        hw_error("PowerPC 601 / 620 / 970 need a 1MB BIOS\n");
-    }
 
     if (linux_boot) {
         kernel_base = KERNEL_LOAD_ADDR;