[v2] xen/arm: Clean-up & fixes in boot/mm code

[Xen-devel] [PATCH MM-PART2 RESEND v2 14/19] xen/arm32: mm: Avoid cleaning the cache for secondary CPUs page-tables

Posted by Julien Grall 1 year, 11 months ago

The page-table walker is configured to use the same shareability and
cacheability as the access performed when updating the page-tables. This
means cleaning the cache for secondary CPUs runtime page-tables is
unnecessary.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index cda2847d00..6db7dda0da 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -769,9 +769,6 @@ int init_secondary_pagetables(int cpu)
         write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], pte);
     }
 
-    clean_dcache_va_range(first, PAGE_SIZE);
-    clean_dcache_va_range(domheap, DOMHEAP_SECOND_PAGES*PAGE_SIZE);
-
     per_cpu(xen_pgtable, cpu) = first;
     per_cpu(xen_dommap, cpu) = domheap;
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART2 RESEND v2 14/19] xen/arm32: mm: Avoid cleaning the cache for secondary CPUs page-tables

Posted by Stefano Stabellini 1 year, 10 months ago

On Tue, 14 May 2019, Julien Grall wrote:
> The page-table walker is configured to use the same shareability and
> cacheability as the access performed when updating the page-tables. This
> means cleaning the cache for secondary CPUs runtime page-tables is
> unnecessary.

All right. Is there an explicit configuration for the shareability and
cacheability used by the page-table walker or is it specified as such in
the Arm Arm? Also, isn't it possible that CPUs on a different cluster
(big.LITTLE) would have issues with this if the cache could be split
between the two clusters?


> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> 
> ---
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index cda2847d00..6db7dda0da 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -769,9 +769,6 @@ int init_secondary_pagetables(int cpu)
>          write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], pte);
>      }
>  
> -    clean_dcache_va_range(first, PAGE_SIZE);
> -    clean_dcache_va_range(domheap, DOMHEAP_SECOND_PAGES*PAGE_SIZE);
> -
>      per_cpu(xen_pgtable, cpu) = first;
>      per_cpu(xen_dommap, cpu) = domheap;
>  
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART2 RESEND v2 14/19] xen/arm32: mm: Avoid cleaning the cache for secondary CPUs page-tables

Posted by Julien Grall 1 year, 10 months ago


On 05/06/2019 00:11, Stefano Stabellini wrote:
> On Tue, 14 May 2019, Julien Grall wrote:
>> The page-table walker is configured to use the same shareability and
>> cacheability as the access performed when updating the page-tables. This
>> means cleaning the cache for secondary CPUs runtime page-tables is
>> unnecessary.
> 
> All right. Is there an explicit configuration for the shareability and
> cacheability used by the page-table walker or is it specified as such in
> the Arm Arm?

See the configuration of TCR_EL2, I can mention it.

> Also, isn't it possible that CPUs on a different cluster
> (big.LITTLE) would have issues with this if the cache could be split
> between the two clusters?

I don't understand this... Cache should be coherent when a CPU leaves EL3.
But we already share some bits of the page tables between the processor (see 
create_xen_page_tables). So I don't see where there is a possible problem here.

Cheers,

> 
> 
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
>>
>> ---
>>      Changes in v2:
>>          - Add Andrii's reviewed-by
>> ---
>>   xen/arch/arm/mm.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index cda2847d00..6db7dda0da 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -769,9 +769,6 @@ int init_secondary_pagetables(int cpu)
>>           write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], pte);
>>       }
>>   
>> -    clean_dcache_va_range(first, PAGE_SIZE);
>> -    clean_dcache_va_range(domheap, DOMHEAP_SECOND_PAGES*PAGE_SIZE);
>> -
>>       per_cpu(xen_pgtable, cpu) = first;
>>       per_cpu(xen_dommap, cpu) = domheap;
>>   
>> -- 
>> 2.11.0
>>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART2 RESEND v2 14/19] xen/arm32: mm: Avoid cleaning the cache for secondary CPUs page-tables

Posted by Stefano Stabellini 1 year, 10 months ago

On Wed, 5 Jun 2019, Julien Grall wrote:
> On 05/06/2019 00:11, Stefano Stabellini wrote:
> > On Tue, 14 May 2019, Julien Grall wrote:
> > > The page-table walker is configured to use the same shareability and
> > > cacheability as the access performed when updating the page-tables. This
> > > means cleaning the cache for secondary CPUs runtime page-tables is
> > > unnecessary.
> > 
> > All right. Is there an explicit configuration for the shareability and
> > cacheability used by the page-table walker or is it specified as such in
> > the Arm Arm?
> 
> See the configuration of TCR_EL2, I can mention it.
 
That would be nice. I double-checked and it is as you wrote.


> > Also, isn't it possible that CPUs on a different cluster
> > (big.LITTLE) would have issues with this if the cache could be split
> > between the two clusters?
> 
> I don't understand this... Cache should be coherent when a CPU leaves EL3.
> But we already share some bits of the page tables between the processor (see
> create_xen_page_tables). So I don't see where there is a possible problem
> here.

If the cache is always coherent across the clusters and the
pagetable-walkers of different clusters, then this is fine.

 
> > > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > > Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> > > 
> > > ---
> > >      Changes in v2:
> > >          - Add Andrii's reviewed-by
> > > ---
> > >   xen/arch/arm/mm.c | 3 ---
> > >   1 file changed, 3 deletions(-)
> > > 
> > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > > index cda2847d00..6db7dda0da 100644
> > > --- a/xen/arch/arm/mm.c
> > > +++ b/xen/arch/arm/mm.c
> > > @@ -769,9 +769,6 @@ int init_secondary_pagetables(int cpu)
> > >           write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)],
> > > pte);
> > >       }
> > >   -    clean_dcache_va_range(first, PAGE_SIZE);
> > > -    clean_dcache_va_range(domheap, DOMHEAP_SECOND_PAGES*PAGE_SIZE);
> > > -
> > >       per_cpu(xen_pgtable, cpu) = first;
> > >       per_cpu(xen_dommap, cpu) = domheap;
> > >   
> > > -- 
> > > 2.11.0
> > > 
> 
> -- 
> Julien Grall
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART2 RESEND v2 14/19] xen/arm32: mm: Avoid cleaning the cache for secondary CPUs page-tables

Posted by Julien Grall 1 year, 10 months ago

Hi Steano,

On 6/10/19 9:28 PM, Stefano Stabellini wrote:
> On Wed, 5 Jun 2019, Julien Grall wrote:
>> On 05/06/2019 00:11, Stefano Stabellini wrote:
>>> On Tue, 14 May 2019, Julien Grall wrote:
>>>> The page-table walker is configured to use the same shareability and
>>>> cacheability as the access performed when updating the page-tables. This
>>>> means cleaning the cache for secondary CPUs runtime page-tables is
>>>> unnecessary.
>>>
>>> All right. Is there an explicit configuration for the shareability and
>>> cacheability used by the page-table walker or is it specified as such in
>>> the Arm Arm?
>>
>> See the configuration of TCR_EL2, I can mention it.
>   
> That would be nice. I double-checked and it is as you wrote.

Sure. How about:

"The page-table walker is configured by TCR_EL2 to use shareability and 
cacheability as the access performed when updating the page-tables. [...]"

> 
> 
>>> Also, isn't it possible that CPUs on a different cluster
>>> (big.LITTLE) would have issues with this if the cache could be split
>>> between the two clusters?
>>
>> I don't understand this... Cache should be coherent when a CPU leaves EL3.
>> But we already share some bits of the page tables between the processor (see
>> create_xen_page_tables). So I don't see where there is a possible problem
>> here.
> 
> If the cache is always coherent across the clusters and the
> pagetable-walkers of different clusters, then this is fine.

Xen (and Linux) built on the assumption that all the CPUs (and 
page-table walker) are in the same shareable domain (i.e 
innershareable). If you have a platform where it is not the case, then 
Xen is going to be badly broken.

This is also inline with the expectatio  from the Arm Arm (B2-123 in DDI 
0487D.a):

"The Inner Shareable domain is expected to be the set of PEs controlled 
by a single hypervisor or operating system."

Do you have a case where Xen needs to run on PEs in different domains?

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART2 RESEND v2 14/19] xen/arm32: mm: Avoid cleaning the cache for secondary CPUs page-tables

Posted by Stefano Stabellini 1 year, 10 months ago

On Mon, 10 Jun 2019, Julien Grall wrote:
> Hi Steano,
> 
> On 6/10/19 9:28 PM, Stefano Stabellini wrote:
> > On Wed, 5 Jun 2019, Julien Grall wrote:
> > > On 05/06/2019 00:11, Stefano Stabellini wrote:
> > > > On Tue, 14 May 2019, Julien Grall wrote:
> > > > > The page-table walker is configured to use the same shareability and
> > > > > cacheability as the access performed when updating the page-tables.
> > > > > This
> > > > > means cleaning the cache for secondary CPUs runtime page-tables is
> > > > > unnecessary.
> > > > 
> > > > All right. Is there an explicit configuration for the shareability and
> > > > cacheability used by the page-table walker or is it specified as such in
> > > > the Arm Arm?
> > > 
> > > See the configuration of TCR_EL2, I can mention it.
> >   That would be nice. I double-checked and it is as you wrote.
> 
> Sure. How about:
> 
> "The page-table walker is configured by TCR_EL2 to use shareability and
> cacheability as the access performed when updating the page-tables. [...]"

That's great thank you. With that, add my reviewed-by.


> > > > Also, isn't it possible that CPUs on a different cluster
> > > > (big.LITTLE) would have issues with this if the cache could be split
> > > > between the two clusters?
> > > 
> > > I don't understand this... Cache should be coherent when a CPU leaves EL3.
> > > But we already share some bits of the page tables between the processor
> > > (see
> > > create_xen_page_tables). So I don't see where there is a possible problem
> > > here.
> > 
> > If the cache is always coherent across the clusters and the
> > pagetable-walkers of different clusters, then this is fine.
> 
> Xen (and Linux) built on the assumption that all the CPUs (and page-table
> walker) are in the same shareable domain (i.e innershareable). If you have a
> platform where it is not the case, then Xen is going to be badly broken.
> 
> This is also inline with the expectatio  from the Arm Arm (B2-123 in DDI
> 0487D.a):
> 
> "The Inner Shareable domain is expected to be the set of PEs controlled by a
> single hypervisor or operating system."
> 
> Do you have a case where Xen needs to run on PEs in different domains?

No, thankfully I don't :-)

I was worried that some big.LITTLE SoCs might be built like that so. (I
don't have any big.LITTLE machines here to confirm/deny.) It is good
that we don't have to worry about it.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH MM-PART2 RESEND v2 14/19] xen/arm32: mm: Avoid cleaning the cache for secondary CPUs page-tables

Posted by Julien Grall 1 year, 10 months ago

Hi,

On 05/06/2019 11:19, Julien Grall wrote:
> On 05/06/2019 00:11, Stefano Stabellini wrote:
>> On Tue, 14 May 2019, Julien Grall wrote:
>>> The page-table walker is configured to use the same shareability and
>>> cacheability as the access performed when updating the page-tables. This
>>> means cleaning the cache for secondary CPUs runtime page-tables is
>>> unnecessary.
>>
>> All right. Is there an explicit configuration for the shareability and
>> cacheability used by the page-table walker or is it specified as such in
>> the Arm Arm?
> 
> See the configuration of TCR_EL2, I can mention it.
> 
>> Also, isn't it possible that CPUs on a different cluster
>> (big.LITTLE) would have issues with this if the cache could be split
>> between the two clusters?

Gentle ping, can you please clarify your question.

> 
> I don't understand this... Cache should be coherent when a CPU leaves EL3.
> But we already share some bits of the page tables between the processor (see 
> create_xen_page_tables). So I don't see where there is a possible problem here.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel