* [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed
@ 2013-12-18 17:31 Julien Grall
2013-12-18 19:28 ` Stefano Stabellini
2013-12-19 9:38 ` Ian Campbell
0 siblings, 2 replies; 6+ messages in thread
From: Julien Grall @ 2013-12-18 17:31 UTC (permalink / raw)
To: xen-devel
Cc: ian.campbell, patches, Julien Grall, tim, george.dunlap,
stefano.stabellini
When Xen is removing/relinquishing mapping, it will create second/third tables
if they don't exist.
Non-existent table means the address range was never mapped, so Xen can safely
skip them.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
Release: This is an improvement for Xen 4.4. It will save time during
relinquish phase and avoid dummy allocation.
The downside is the patch is modifying p2m loop which is used everywhere.
---
xen/arch/arm/p2m.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d24a6fc..9ef8819 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -246,10 +246,12 @@ static int create_p2m_entries(struct domain *d,
cur_first_offset = ~0,
cur_second_offset = ~0;
unsigned long count = 0;
+ bool_t populate = (op == INSERT || op == ALLOCATE);
spin_lock(&p2m->lock);
- for(addr = start_gpaddr; addr < end_gpaddr; addr += PAGE_SIZE)
+ addr = start_gpaddr;
+ while ( addr < end_gpaddr )
{
if ( cur_first_page != p2m_first_level_index(addr) )
{
@@ -265,8 +267,15 @@ static int create_p2m_entries(struct domain *d,
if ( !first[first_table_offset(addr)].p2m.valid )
{
+ if ( !populate )
+ {
+ addr += FIRST_SIZE;
+ continue;
+ }
+
rc = p2m_create_table(d, &first[first_table_offset(addr)]);
- if ( rc < 0 ) {
+ if ( rc < 0 )
+ {
printk("p2m_populate_ram: L1 failed\n");
goto out;
}
@@ -284,6 +293,12 @@ static int create_p2m_entries(struct domain *d,
if ( !second[second_table_offset(addr)].p2m.valid )
{
+ if ( !populate )
+ {
+ addr += SECOND_SIZE;
+ continue;
+ }
+
rc = p2m_create_table(d, &second[second_table_offset(addr)]);
if ( rc < 0 ) {
printk("p2m_populate_ram: L2 failed\n");
@@ -372,6 +387,9 @@ static int create_p2m_entries(struct domain *d,
}
count = 0;
}
+
+ /* Got the next page */
+ addr += PAGE_SIZE;
}
if ( op == ALLOCATE || op == INSERT )
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed
2013-12-18 17:31 [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed Julien Grall
@ 2013-12-18 19:28 ` Stefano Stabellini
2013-12-18 19:29 ` Julien Grall
2013-12-19 9:38 ` Ian Campbell
1 sibling, 1 reply; 6+ messages in thread
From: Stefano Stabellini @ 2013-12-18 19:28 UTC (permalink / raw)
To: Julien Grall
Cc: ian.campbell, patches, tim, george.dunlap, stefano.stabellini,
xen-devel
On Wed, 18 Dec 2013, Julien Grall wrote:
> When Xen is removing/relinquishing mapping, it will create second/third tables
> if they don't exist.
>
> Non-existent table means the address range was never mapped, so Xen can safely
> skip them.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>
> ---
> Release: This is an improvement for Xen 4.4. It will save time during
> relinquish phase and avoid dummy allocation.
> The downside is the patch is modifying p2m loop which is used everywhere.
> ---
> xen/arch/arm/p2m.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index d24a6fc..9ef8819 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -246,10 +246,12 @@ static int create_p2m_entries(struct domain *d,
> cur_first_offset = ~0,
> cur_second_offset = ~0;
> unsigned long count = 0;
> + bool_t populate = (op == INSERT || op == ALLOCATE);
why don't you just check for op == REMOVE?
> spin_lock(&p2m->lock);
>
> - for(addr = start_gpaddr; addr < end_gpaddr; addr += PAGE_SIZE)
> + addr = start_gpaddr;
> + while ( addr < end_gpaddr )
> {
> if ( cur_first_page != p2m_first_level_index(addr) )
> {
> @@ -265,8 +267,15 @@ static int create_p2m_entries(struct domain *d,
>
> if ( !first[first_table_offset(addr)].p2m.valid )
> {
> + if ( !populate )
> + {
> + addr += FIRST_SIZE;
> + continue;
> + }
> +
> rc = p2m_create_table(d, &first[first_table_offset(addr)]);
> - if ( rc < 0 ) {
> + if ( rc < 0 )
> + {
> printk("p2m_populate_ram: L1 failed\n");
> goto out;
> }
> @@ -284,6 +293,12 @@ static int create_p2m_entries(struct domain *d,
>
> if ( !second[second_table_offset(addr)].p2m.valid )
> {
> + if ( !populate )
> + {
> + addr += SECOND_SIZE;
> + continue;
> + }
> +
> rc = p2m_create_table(d, &second[second_table_offset(addr)]);
> if ( rc < 0 ) {
> printk("p2m_populate_ram: L2 failed\n");
> @@ -372,6 +387,9 @@ static int create_p2m_entries(struct domain *d,
> }
> count = 0;
> }
> +
> + /* Got the next page */
> + addr += PAGE_SIZE;
> }
>
> if ( op == ALLOCATE || op == INSERT )
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed
2013-12-18 19:28 ` Stefano Stabellini
@ 2013-12-18 19:29 ` Julien Grall
2013-12-18 19:31 ` Stefano Stabellini
0 siblings, 1 reply; 6+ messages in thread
From: Julien Grall @ 2013-12-18 19:29 UTC (permalink / raw)
To: Stefano Stabellini
Cc: ian.campbell, patches, tim, george.dunlap, stefano.stabellini,
xen-devel
On 12/18/2013 07:28 PM, Stefano Stabellini wrote:
> On Wed, 18 Dec 2013, Julien Grall wrote:
>> When Xen is removing/relinquishing mapping, it will create second/third tables
>> if they don't exist.
>>
>> Non-existent table means the address range was never mapped, so Xen can safely
>> skip them.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>
>> ---
>> Release: This is an improvement for Xen 4.4. It will save time during
>> relinquish phase and avoid dummy allocation.
>> The downside is the patch is modifying p2m loop which is used everywhere.
>> ---
>> xen/arch/arm/p2m.c | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index d24a6fc..9ef8819 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -246,10 +246,12 @@ static int create_p2m_entries(struct domain *d,
>> cur_first_offset = ~0,
>> cur_second_offset = ~0;
>> unsigned long count = 0;
>> + bool_t populate = (op == INSERT || op == ALLOCATE);
>
> why don't you just check for op == REMOVE?
Because you also need to check op == RELINQUISH (added by my patch
series on foreign mapping).
--
Julien Grall
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed
2013-12-18 19:29 ` Julien Grall
@ 2013-12-18 19:31 ` Stefano Stabellini
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Stabellini @ 2013-12-18 19:31 UTC (permalink / raw)
To: Julien Grall
Cc: ian.campbell, patches, Stefano Stabellini, tim, george.dunlap,
stefano.stabellini, xen-devel
On Wed, 18 Dec 2013, Julien Grall wrote:
> On 12/18/2013 07:28 PM, Stefano Stabellini wrote:
> > On Wed, 18 Dec 2013, Julien Grall wrote:
> > > When Xen is removing/relinquishing mapping, it will create second/third
> > > tables
> > > if they don't exist.
> > >
> > > Non-existent table means the address range was never mapped, so Xen can
> > > safely
> > > skip them.
> > >
> > > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > >
> > > ---
> > > Release: This is an improvement for Xen 4.4. It will save time during
> > > relinquish phase and avoid dummy allocation.
> > > The downside is the patch is modifying p2m loop which is used
> > > everywhere.
> > > ---
> > > xen/arch/arm/p2m.c | 22 ++++++++++++++++++++--
> > > 1 file changed, 20 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > > index d24a6fc..9ef8819 100644
> > > --- a/xen/arch/arm/p2m.c
> > > +++ b/xen/arch/arm/p2m.c
> > > @@ -246,10 +246,12 @@ static int create_p2m_entries(struct domain *d,
> > > cur_first_offset = ~0,
> > > cur_second_offset = ~0;
> > > unsigned long count = 0;
> > > + bool_t populate = (op == INSERT || op == ALLOCATE);
> >
> > why don't you just check for op == REMOVE?
>
> Because you also need to check op == RELINQUISH (added by my patch series on
> foreign mapping).
I see. In that case
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed
2013-12-18 17:31 [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed Julien Grall
2013-12-18 19:28 ` Stefano Stabellini
@ 2013-12-19 9:38 ` Ian Campbell
2013-12-19 15:28 ` Julien Grall
1 sibling, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2013-12-19 9:38 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, tim, george.dunlap, stefano.stabellini, patches
On Wed, 2013-12-18 at 17:31 +0000, Julien Grall wrote:
> When Xen is removing/relinquishing mapping, it will create second/third tables
> if they don't exist.
>
> Non-existent table means the address range was never mapped, so Xen can safely
> skip them.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>
> ---
> Release: This is an improvement for Xen 4.4. It will save time during
> relinquish phase and avoid dummy allocation.
> The downside is the patch is modifying p2m loop which is used everywhere.
> ---
> xen/arch/arm/p2m.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index d24a6fc..9ef8819 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -246,10 +246,12 @@ static int create_p2m_entries(struct domain *d,
> cur_first_offset = ~0,
> cur_second_offset = ~0;
> unsigned long count = 0;
> + bool_t populate = (op == INSERT || op == ALLOCATE);
>
> spin_lock(&p2m->lock);
>
> - for(addr = start_gpaddr; addr < end_gpaddr; addr += PAGE_SIZE)
> + addr = start_gpaddr;
> + while ( addr < end_gpaddr )
> {
> if ( cur_first_page != p2m_first_level_index(addr) )
> {
> @@ -265,8 +267,15 @@ static int create_p2m_entries(struct domain *d,
>
> if ( !first[first_table_offset(addr)].p2m.valid )
> {
> + if ( !populate )
> + {
> + addr += FIRST_SIZE;
I think this subtly does the wrong thing if addr is not FIRST_SIZE
aligned, which it might be on the first iteration. That will skip the
start of the next 1st level block.
I think addr needs to be rounded up to the next first boundary.
> + continue;
> + }
> +
> rc = p2m_create_table(d, &first[first_table_offset(addr)]);
> - if ( rc < 0 ) {
> + if ( rc < 0 )
> + {
> printk("p2m_populate_ram: L1 failed\n");
> goto out;
> }
> @@ -284,6 +293,12 @@ static int create_p2m_entries(struct domain *d,
>
> if ( !second[second_table_offset(addr)].p2m.valid )
> {
> + if ( !populate )
> + {
> + addr += SECOND_SIZE;
Likewise here.
> + continue;
> + }
> +
> rc = p2m_create_table(d, &second[second_table_offset(addr)]);
> if ( rc < 0 ) {
> printk("p2m_populate_ram: L2 failed\n");
> @@ -372,6 +387,9 @@ static int create_p2m_entries(struct domain *d,
> }
> count = 0;
> }
> +
> + /* Got the next page */
> + addr += PAGE_SIZE;
> }
>
> if ( op == ALLOCATE || op == INSERT )
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed
2013-12-19 9:38 ` Ian Campbell
@ 2013-12-19 15:28 ` Julien Grall
0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2013-12-19 15:28 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, tim, george.dunlap, stefano.stabellini, patches
On 12/19/2013 09:38 AM, Ian Campbell wrote:
> On Wed, 2013-12-18 at 17:31 +0000, Julien Grall wrote:
>> When Xen is removing/relinquishing mapping, it will create second/third tables
>> if they don't exist.
>>
>> Non-existent table means the address range was never mapped, so Xen can safely
>> skip them.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>
>> ---
>> Release: This is an improvement for Xen 4.4. It will save time during
>> relinquish phase and avoid dummy allocation.
>> The downside is the patch is modifying p2m loop which is used everywhere.
>> ---
>> xen/arch/arm/p2m.c | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index d24a6fc..9ef8819 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -246,10 +246,12 @@ static int create_p2m_entries(struct domain *d,
>> cur_first_offset = ~0,
>> cur_second_offset = ~0;
>> unsigned long count = 0;
>> + bool_t populate = (op == INSERT || op == ALLOCATE);
>>
>> spin_lock(&p2m->lock);
>>
>> - for(addr = start_gpaddr; addr < end_gpaddr; addr += PAGE_SIZE)
>> + addr = start_gpaddr;
>> + while ( addr < end_gpaddr )
>> {
>> if ( cur_first_page != p2m_first_level_index(addr) )
>> {
>> @@ -265,8 +267,15 @@ static int create_p2m_entries(struct domain *d,
>>
>> if ( !first[first_table_offset(addr)].p2m.valid )
>> {
>> + if ( !populate )
>> + {
>> + addr += FIRST_SIZE;
>
> I think this subtly does the wrong thing if addr is not FIRST_SIZE
> aligned, which it might be on the first iteration. That will skip the
> start of the next 1st level block.
>
> I think addr needs to be rounded up to the next first boundary.
Right, I will send a new version of this patch.
--
Julien Grall
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-19 15:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-18 17:31 [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed Julien Grall
2013-12-18 19:28 ` Stefano Stabellini
2013-12-18 19:29 ` Julien Grall
2013-12-18 19:31 ` Stefano Stabellini
2013-12-19 9:38 ` Ian Campbell
2013-12-19 15:28 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.