| Message ID | 20240130172942.52175-1-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Properly clean update to init_ttbr and smp_up_cpu | expand |
On 30.01.24 19:29, Julien Grall wrote: Hello Julien > From: Julien Grall <jgrall@amazon.com> > > Recent rework to the secondary boot code modified how init_ttbr and > smp_up_cpu are accessed. Rather than directly accessing them, we > are using a pointer to them. > > The helper clean_dcache() is expected to take the variable in parameter > and then clean its content. As we now pass a pointer to the variable, > we will clean the area storing the address rather than the content itself. > > Switch to use clean_dcache_va_range() to avoid casting the pointer. > > Fixes: a5ed59e62c6f ("arm/mmu: Move init_ttbr to a new section .data.idmap") > Fixes: 9a5114074b04 ("arm/smpboot: Move smp_up_cpu to a new section .data.idmap) > > Reported-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > Signed-off-by: Julien Grall <jgrall@amazon.com> [on Renesas R-Car Gen3 SoC with 8 cores (Arm64)] Tested-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > --- > xen/arch/arm/mmu/smpboot.c | 2 +- > xen/arch/arm/smpboot.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/mmu/smpboot.c b/xen/arch/arm/mmu/smpboot.c > index bc91fdfe3331..4ffc8254a44b 100644 > --- a/xen/arch/arm/mmu/smpboot.c > +++ b/xen/arch/arm/mmu/smpboot.c > @@ -88,7 +88,7 @@ static void set_init_ttbr(lpae_t *root) > * init_ttbr will be accessed with the MMU off, so ensure the update > * is visible by cleaning the cache. > */ > - clean_dcache(ptr); > + clean_dcache_va_range(ptr, sizeof(uint64_t)); > > unmap_domain_page(ptr); > } > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index 119bfa3160ad..a84e706d77da 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -449,7 +449,7 @@ static void set_smp_up_cpu(unsigned long mpidr) > * smp_up_cpu will be accessed with the MMU off, so ensure the update > * is visible by cleaning the cache. > */ > - clean_dcache(ptr); > + clean_dcache_va_range(ptr, sizeof(unsigned long)); > > unmap_domain_page(ptr); >
Hi Julien, Nice finding :-) > On 30 Jan 2024, at 18:29, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <jgrall@amazon.com> > > Recent rework to the secondary boot code modified how init_ttbr and > smp_up_cpu are accessed. Rather than directly accessing them, we > are using a pointer to them. > > The helper clean_dcache() is expected to take the variable in parameter > and then clean its content. As we now pass a pointer to the variable, > we will clean the area storing the address rather than the content itself. > > Switch to use clean_dcache_va_range() to avoid casting the pointer. > > Fixes: a5ed59e62c6f ("arm/mmu: Move init_ttbr to a new section .data.idmap") > Fixes: 9a5114074b04 ("arm/smpboot: Move smp_up_cpu to a new section .data.idmap) > > Reported-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Cheers Bertrand > --- > xen/arch/arm/mmu/smpboot.c | 2 +- > xen/arch/arm/smpboot.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/mmu/smpboot.c b/xen/arch/arm/mmu/smpboot.c > index bc91fdfe3331..4ffc8254a44b 100644 > --- a/xen/arch/arm/mmu/smpboot.c > +++ b/xen/arch/arm/mmu/smpboot.c > @@ -88,7 +88,7 @@ static void set_init_ttbr(lpae_t *root) > * init_ttbr will be accessed with the MMU off, so ensure the update > * is visible by cleaning the cache. > */ > - clean_dcache(ptr); > + clean_dcache_va_range(ptr, sizeof(uint64_t)); > > unmap_domain_page(ptr); > } > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index 119bfa3160ad..a84e706d77da 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -449,7 +449,7 @@ static void set_smp_up_cpu(unsigned long mpidr) > * smp_up_cpu will be accessed with the MMU off, so ensure the update > * is visible by cleaning the cache. > */ > - clean_dcache(ptr); > + clean_dcache_va_range(ptr, sizeof(unsigned long)); > > unmap_domain_page(ptr); > > -- > 2.40.1 >
Hi, On 31/01/2024 07:57, Bertrand Marquis wrote: >> On 30 Jan 2024, at 18:29, Julien Grall <julien@xen.org> wrote: >> >> From: Julien Grall <jgrall@amazon.com> >> >> Recent rework to the secondary boot code modified how init_ttbr and >> smp_up_cpu are accessed. Rather than directly accessing them, we >> are using a pointer to them. >> >> The helper clean_dcache() is expected to take the variable in parameter >> and then clean its content. As we now pass a pointer to the variable, >> we will clean the area storing the address rather than the content itself. >> >> Switch to use clean_dcache_va_range() to avoid casting the pointer. >> >> Fixes: a5ed59e62c6f ("arm/mmu: Move init_ttbr to a new section .data.idmap") >> Fixes: 9a5114074b04 ("arm/smpboot: Move smp_up_cpu to a new section .data.idmap) >> >> Reported-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> Signed-off-by: Julien Grall <jgrall@amazon.com> > > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Committed. Thanks. Cheers,
diff --git a/xen/arch/arm/mmu/smpboot.c b/xen/arch/arm/mmu/smpboot.c index bc91fdfe3331..4ffc8254a44b 100644 --- a/xen/arch/arm/mmu/smpboot.c +++ b/xen/arch/arm/mmu/smpboot.c @@ -88,7 +88,7 @@ static void set_init_ttbr(lpae_t *root) * init_ttbr will be accessed with the MMU off, so ensure the update * is visible by cleaning the cache. */ - clean_dcache(ptr); + clean_dcache_va_range(ptr, sizeof(uint64_t)); unmap_domain_page(ptr); } diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index 119bfa3160ad..a84e706d77da 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -449,7 +449,7 @@ static void set_smp_up_cpu(unsigned long mpidr) * smp_up_cpu will be accessed with the MMU off, so ensure the update * is visible by cleaning the cache. */ - clean_dcache(ptr); + clean_dcache_va_range(ptr, sizeof(unsigned long)); unmap_domain_page(ptr);