[PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[Ian Campbell]

They might be in e.g. flash or something but more likely they could
bein a bank of RAM which we aren't handling or in RAM which the
bootloader hasn't told us about for some reason.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Fu Wei <fu.wei@linaro.org>
Cc: Roy Franz <roy.franz@linaro.org>
---
 xen/arch/arm/setup.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index e53e491..446b4dc 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -247,8 +247,13 @@ void __init discard_initial_modules(void)
         paddr_t s = mi->module[i].start;
         paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
 
-        if ( mi->module[i].kind != BOOTMOD_XEN )
-            dt_unreserved_regions(s, e, init_domheap_pages, 0);
+        if ( mi->module[i].kind == BOOTMOD_XEN )
+            continue;
+
+        if ( !mfn_valid(paddr_to_pfn(s)) || !mfn_valid(paddr_to_pfn(e)))
+            continue;
+
+        dt_unreserved_regions(s, e, init_domheap_pages, 0);
     }
 
     mi->nr_mods = 0;
-- 
1.7.10.4

Re: [PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[Roy Franz]

[-- Attachment #1.1: Type: text/plain, Size: 1434 bytes --]

On Wed, Jul 23, 2014 at 9:45 AM, Ian Campbell <ian.campbell@citrix.com>
wrote:

> They might be in e.g. flash or something but more likely they could
> bein a bank of RAM which we aren't handling or in RAM which the
> bootloader hasn't told us about for some reason.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Fu Wei <fu.wei@linaro.org>
> Cc: Roy Franz <roy.franz@linaro.org>
> ---
>  xen/arch/arm/setup.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index e53e491..446b4dc 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -247,8 +247,13 @@ void __init discard_initial_modules(void)
>          paddr_t s = mi->module[i].start;
>          paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
>
> -        if ( mi->module[i].kind != BOOTMOD_XEN )
> -            dt_unreserved_regions(s, e, init_domheap_pages, 0);
> +        if ( mi->module[i].kind == BOOTMOD_XEN )
> +            continue;
> +
> +        if ( !mfn_valid(paddr_to_pfn(s)) || !mfn_valid(paddr_to_pfn(e)))
> +            continue;
> +
> +        dt_unreserved_regions(s, e, init_domheap_pages, 0);
>      }
>
>      mi->nr_mods = 0;


This looks good - I'll give it a spin.  I think this will be sufficient to
allow the stub (or GRUB) to use any available EFI memory for loading modules
without worrying about whether XEN will map it.

Thanks,
Roy

[-- Attachment #1.2: Type: text/html, Size: 2127 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[Julien Grall]

Hi Ian,

On 23/07/14 17:45, Ian Campbell wrote:
> They might be in e.g. flash or something but more likely they could
> bein a bank of RAM which we aren't handling or in RAM which the

being?

> bootloader hasn't told us about for some reason.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Fu Wei <fu.wei@linaro.org>
> Cc: Roy Franz <roy.franz@linaro.org>
> ---
>   xen/arch/arm/setup.c |    9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index e53e491..446b4dc 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -247,8 +247,13 @@ void __init discard_initial_modules(void)
>           paddr_t s = mi->module[i].start;
>           paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
>
> -        if ( mi->module[i].kind != BOOTMOD_XEN )
> -            dt_unreserved_regions(s, e, init_domheap_pages, 0);
> +        if ( mi->module[i].kind == BOOTMOD_XEN )
> +            continue;
> +
> +        if ( !mfn_valid(paddr_to_pfn(s)) || !mfn_valid(paddr_to_pfn(e)))
> +            continue;

What happen if the bootloader decide to put the module between 2 banks 
and having the hole in the middle. Such as:

start of the module

end of bank 0

hole

start of bank 1

end of the module

Regards,

-- 
Julien Grall

Re: [PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[Ian Campbell]

On Thu, 2014-07-24 at 11:40 +0100, Julien Grall wrote:
> > -        if ( mi->module[i].kind != BOOTMOD_XEN )
> > -            dt_unreserved_regions(s, e, init_domheap_pages, 0);
> > +        if ( mi->module[i].kind == BOOTMOD_XEN )
> > +            continue;
> > +
> > +        if ( !mfn_valid(paddr_to_pfn(s)) || !mfn_valid(paddr_to_pfn(e)))
> > +            continue;
> 
> What happen if the bootloader decide to put the module between 2 banks 
> and having the hole in the middle. Such as:
> 
> start of the module
> 
> end of bank 0
> 
> hole
> 
> start of bank 1
> 
> end of the module

Either we will ignore bank 1, in which case these checks will prevent us
adding them to the heap or the frame table will span bank0..1 and
include the hole.

We don't really handle the latter case very well, but the first one is
the one which is actually biting people today.

Ian.

Re: [PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[Julien Grall]

Hi Ian,

On 07/24/2014 11:44 AM, Ian Campbell wrote:
> On Thu, 2014-07-24 at 11:40 +0100, Julien Grall wrote:
>>> -        if ( mi->module[i].kind != BOOTMOD_XEN )
>>> -            dt_unreserved_regions(s, e, init_domheap_pages, 0);
>>> +        if ( mi->module[i].kind == BOOTMOD_XEN )
>>> +            continue;
>>> +
>>> +        if ( !mfn_valid(paddr_to_pfn(s)) || !mfn_valid(paddr_to_pfn(e)))
>>> +            continue;
>>
>> What happen if the bootloader decide to put the module between 2 banks 
>> and having the hole in the middle. Such as:
>>
>> start of the module
>>
>> end of bank 0
>>
>> hole
>>
>> start of bank 1
>>
>> end of the module
> 
> Either we will ignore bank 1, in which case these checks will prevent us
> adding them to the heap or the frame table will span bank0..1 and
> include the hole.
> 
> We don't really handle the latter case very well, but the first one is
> the one which is actually biting people today.

Thanks for the explanation. It might be worse to add a TODO to help the
person who will support sparse frame table.

Anyway:

Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

Re: [PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[Ian Campbell]

On Thu, 2014-07-24 at 11:40 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 23/07/14 17:45, Ian Campbell wrote:
> > They might be in e.g. flash or something but more likely they could
> > bein a bank of RAM which we aren't handling or in RAM which the
> 
> being?

"be in". Oops!

I've made a note for myself about the sparseness thing (since I hope to
work on that soon) and applied with your Ack.

Ian.

Re: [PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[Roy Franz]

[-- Attachment #1.1: Type: text/plain, Size: 680 bytes --]

On Thu, Jul 24, 2014 at 8:54 AM, Ian Campbell <Ian.Campbell@citrix.com>
wrote:

>
> On Thu, 2014-07-24 at 11:40 +0100, Julien Grall wrote:
> > Hi Ian,
> >
> > On 23/07/14 17:45, Ian Campbell wrote:
> > > They might be in e.g. flash or something but more likely they could
> > > bein a bank of RAM which we aren't handling or in RAM which the
> >
> > being?
>
> "be in". Oops!
>
> I've made a note for myself about the sparseness thing (since I hope to
> work on that soon) and applied with your Ack.
>
> Ian.
>
>
>
>
>
Thanks Ian - this should allow me to simplify the allocation of buffers for
the modules.
I just got back from vacation today, so I'll give this a try soon.

Roy

[-- Attachment #1.2: Type: text/html, Size: 1320 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[Roy Franz]

[-- Attachment #1.1: Type: text/plain, Size: 992 bytes --]

On Mon, Aug 4, 2014 at 5:16 PM, Roy Franz <roy.franz@linaro.org> wrote:

> On Thu, Jul 24, 2014 at 8:54 AM, Ian Campbell <Ian.Campbell@citrix.com>
> wrote:
>
>>
>> On Thu, 2014-07-24 at 11:40 +0100, Julien Grall wrote:
>> > Hi Ian,
>> >
>> > On 23/07/14 17:45, Ian Campbell wrote:
>> > > They might be in e.g. flash or something but more likely they could
>> > > bein a bank of RAM which we aren't handling or in RAM which the
>> >
>> > being?
>>
>> "be in". Oops!
>>
>> I've made a note for myself about the sparseness thing (since I hope to
>> work on that soon) and applied with your Ack.
>>
>> Ian.
>>
>>
>>
>>
>>
> Thanks Ian - this should allow me to simplify the allocation of buffers
> for the modules.
> I just got back from vacation today, so I'll give this a try soon.
>
> Roy
>
> This works and resolves the issue of the EFI stub trying to put modules
into RAM that XEN will later
use.  I'll remove the max address stuff I put in to deal with this in my
next series.

Thanks,
Roy

[-- Attachment #1.2: Type: text/html, Size: 2143 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xen: arm: don't release modules which aren't in RAM into the heap

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

Roy,
could you please use plain text emails? HTML emails are not easy to read
with my MUA.

On Tue, 5 Aug 2014, Roy Franz wrote:
> On Mon, Aug 4, 2014 at 5:16 PM, Roy Franz <roy.franz@linaro.org> wrote:
>       On Thu, Jul 24, 2014 at 8:54 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
>             On Thu, 2014-07-24 at 11:40 +0100, Julien Grall wrote:
> > Hi Ian,
> >
> > On 23/07/14 17:45, Ian Campbell wrote:
> > > They might be in e.g. flash or something but more likely they could
> > > bein a bank of RAM which we aren't handling or in RAM which the
> >
> > being?
> 
> "be in". Oops!
> 
> I've made a note for myself about the sparseness thing (since I hope to
> work on that soon) and applied with your Ack.
> 
> Ian.
> 
> 
> 
> 
> 
> Thanks Ian - this should allow me to simplify the allocation of buffers for the modules.
> I just got back from vacation today, so I'll give this a try soon.
> 
> Roy
> 
> This works and resolves the issue of the EFI stub trying to put modules into RAM that XEN will later
> use.  I'll remove the max address stuff I put in to deal with this in my next series.
> 
> Thanks,
> Roy
> 
> 
> 

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel