Re: [PATCH ARM v4 10/12] mini-os: get RAM base and size from the FDT

[Julien Grall <julien.grall@linaro.org>]

Hi Thomas,

On 06/18/2014 04:08 PM, Thomas Leonard wrote:
> +        const char *device_type = fdt_getprop(device_tree, node, "device_type", NULL);
> +        if (device_type && !strcmp(device_type, "memory"))
> +        {
> +            /* Note: we assume there's only a single region of memory.
> +             * Since Xen is already translating our "physical"
> +             * addresses to the real physical RAM, there's no
> +             * reason for it to give us multiple blocks. */

This comment looks wrong to me. Even tho, Xen is providing a stage-2
translation to show you a virtual layout (your guest physical memory),
the new layout in Xen upstream may contain multiple banks. The first
bank will contain up to 3G of RAM.
> index d2d5264..d31ef97 100644
> --- a/extras/mini-os/mm.c
> +++ b/extras/mini-os/mm.c
> @@ -409,8 +409,8 @@ void init_mm(void)
>       * now we can initialise the page allocator
>       */
>      printk("MM: Initialise page allocator for %lx(%lx)-%lx(%lx)\n",
> -           (u_long)to_virt(PFN_PHYS(start_pfn)), PFN_PHYS(start_pfn), 
> -           (u_long)to_virt(PFN_PHYS(max_pfn)), PFN_PHYS(max_pfn));
> +           (u_long)to_virt(PFN_PHYS(start_pfn)), (u_long)PFN_PHYS(start_pfn), 
> +           (u_long)to_virt(PFN_PHYS(max_pfn)), (u_long)PFN_PHYS(max_pfn));

I don't see any modification of the type of max_pfn, start_pfn,
PFN_PHYS. This change should not be part of this patch.

Regards,
-- 
Julien Grall