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

Andreas Färber andreas.faerber at web.de
Thu Dec 30 19:20:08 CET 2010


Mark,

Am 29.12.2010 um 12:07 schrieb Mark Cave-Ayland:

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland at siriusit.co.uk>

This does not really address my comments...

> ---
> openbios-devel/arch/ppc/qemu/ofmem.c |   21 +++++++++++++++++++++
> 1 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/openbios-devel/arch/ppc/qemu/ofmem.c b/openbios-devel/ 
> arch/ppc/qemu/ofmem.c
> index c21a112..44b5ef4 100644
> --- a/openbios-devel/arch/ppc/qemu/ofmem.c
> +++ b/openbios-devel/arch/ppc/qemu/ofmem.c
> @@ -180,6 +180,27 @@ void ofmem_arch_create_translation_entry(ucell  
> *transentry, translation_t *t)
> /*	OF private allocations						*/
> / 
> ************************************************************************/
>
> +/* 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;
> +    }
> +}

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
}

> +
> +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.

BR,
Andreas

> +
> void *
> malloc(int size)
> {
> -- 
> 1.7.2.3
>
>
> -- 
> OpenBIOS                 http://openbios.org/
> Mailinglist:  http://lists.openbios.org/mailman/listinfo
> Free your System - May the Forth be with you




More information about the OpenBIOS mailing list