[OpenBIOS] [PATCH 3/6] Introduce va2pa() and pa2va() functions for PPC for subsequent use by OFMEM.

[Mark Cave-Ayland]

On 30/12/10 18:20, Andreas Färber wrote:

Hi Andreas,

> This does not really address my comments...

Ah okay - I was still hoping that no news was good news and I admit I 
wanted something in place to unblock the SPARC32 OFMEM patchset.

>> +/* Private functions for mapping between physical/virtual addresses */
>> +inline phys_addr_t
>> +va2pa(unsigned long va)
>> +{
>> + if (va >= OF_CODE_START && va < OF_CODE_START + OF_CODE_SIZE) {
>
> The latter will overflow on 32-bit ppc, and (va < 0) == 0 so that branch
> would never be taken. Is there no compiler warning?
>
>> + return (phys_addr_t)get_rom_base() - OF_CODE_START + va;
>
> Again, what's the reason for duplicating this calculation?
>
>> + } else {
>> + return (phys_addr_t)va;
>> + }
>> +}

Only because I was in two minds with regards to whether to keep pa2va 
and va2pa as "symmetrical" as possible. I'm happy to go with whatever 
you think is best.

> What about the following trivial implementation instead?
>
> phys_addr_t va2pa(unsigned long va)
> {
> ucell mode;
> return ea_to_phys(va, &mode); // calls ofmem_translate() internally for
> the 'else' case
> }

Yes - that seems to work in my tests here.

>> +
>> +inline unsigned long
>> +pa2va(phys_addr_t pa)
>> +{
>> + if ((pa - get_rom_base() + OF_CODE_START >= OF_CODE_START) &&
>> + (pa - get_rom_base() + OF_CODE_START < OF_CODE_START + OF_CODE_SIZE))
>> + return (unsigned long)pa - get_rom_base() + OF_CODE_START;
>> + else
>> + return (unsigned long)pa;
>> +}
>
> Trailing whitespace?
>
> Missing braces for QEMU style.

Ooops my bad. Does the attached patch look better?


ATB,

Mark.

-- 
Mark Cave-Ayland - Senior Technical Architect
PostgreSQL - PostGIS
Sirius Corporation plc - control through freedom
http://www.siriusit.co.uk
t: +44 870 608 0063

Sirius Labs: http://www.siriusit.co.uk/labs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openbios-ppc-vapa.patch
Type: text/x-diff
Size: 1364 bytes
Desc: not available
URL: <http://lists.openbios.org/pipermail/openbios/attachments/20101230/50ff05c0/attachment-0001.bin>