diff mbox series

[RFC,1/6] xen/arm: mm: Remove special case for CPU0 in dump_hyp_walk()

Message ID 20201119190751.22345-2-julien@xen.org (mailing list archive)
State New, archived
Headers show
Series xen/arm: mm: Add limited support for superpages | expand

Commit Message

Julien Grall Nov. 19, 2020, 7:07 p.m. UTC
From: Stefano Stabellini <sstabellini@kernel.org>

There is no need to have a special case for CPU0 when converting the
page-table virtual address into a physical address. The helper
virt_to_maddr() is able to translate any address as long as the root
page-tables is mapped in the virtual address. This is the case for all
the CPUs at the moment.

So use the same BUG_ON() regardless the CPU.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
[julien: Rework the commit message]
Signed-off-by: Julien Grall <jgrall@amazon.com>

---

I went back through the conversation in [1] regarding the issue when
loading Xen below 2MB on Arm32. The example provided is wrong because to
find the physical address, we need to add 'phys_offset', not substract.

So I removed the comment regarding the code was buggy.

[1] https://marc.info/?l=xen-devel&m=157081398022401
---
 xen/arch/arm/mm.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Bertrand Marquis Nov. 24, 2020, 5:10 p.m. UTC | #1
Hi Julien,


> On 19 Nov 2020, at 19:07, Julien Grall <julien@xen.org> wrote:
> 
> From: Stefano Stabellini <sstabellini@kernel.org>
> 
> There is no need to have a special case for CPU0 when converting the
> page-table virtual address into a physical address. The helper
> virt_to_maddr() is able to translate any address as long as the root
> page-tables is mapped in the virtual address. This is the case for all
> the CPUs at the moment.
> 
> So use the same BUG_ON() regardless the CPU.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> [julien: Rework the commit message]
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> 
> ---
> 
> I went back through the conversation in [1] regarding the issue when
> loading Xen below 2MB on Arm32. The example provided is wrong because to
> find the physical address, we need to add 'phys_offset', not substract.
> 
> So I removed the comment regarding the code was buggy.
> 
> [1] https://marc.info/?l=xen-devel&m=157081398022401
> ---
> xen/arch/arm/mm.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 9c4b26bf079b..4dd886f7c80d 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -284,10 +284,7 @@ void dump_hyp_walk(vaddr_t addr)
>            "on CPU%d via TTBR 0x%016"PRIx64"\n",
>            addr, smp_processor_id(), ttbr);
> 
> -    if ( smp_processor_id() == 0 )
> -        BUG_ON( (lpae_t *)(unsigned long)(ttbr - phys_offset) != pgtable );
> -    else
> -        BUG_ON( virt_to_maddr(pgtable) != ttbr );
> +    BUG_ON( virt_to_maddr(pgtable) != ttbr );
>     dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
> }
> 
> -- 
> 2.17.1
>
Julien Grall Nov. 28, 2020, 11:14 a.m. UTC | #2
On 19/11/2020 19:07, Julien Grall wrote:
> From: Stefano Stabellini <sstabellini@kernel.org>
> 
> There is no need to have a special case for CPU0 when converting the
> page-table virtual address into a physical address. The helper
> virt_to_maddr() is able to translate any address as long as the root
> page-tables is mapped in the virtual address. This is the case for all
> the CPUs at the moment.
> 
> So use the same BUG_ON() regardless the CPU.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> [julien: Rework the commit message]
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> ---
> 
> I went back through the conversation in [1] regarding the issue when
> loading Xen below 2MB on Arm32. The example provided is wrong because to
> find the physical address, we need to add 'phys_offset', not substract.
> 
> So I removed the comment regarding the code was buggy.
> 
> [1] https://marc.info/?l=xen-devel&m=157081398022401

Stefano, can you confirm that you are happy with the new commit message?

Cheers,
Stefano Stabellini Nov. 30, 2020, 9:58 p.m. UTC | #3
On Sat, 28 Nov 2020, Julien Grall wrote:
> On 19/11/2020 19:07, Julien Grall wrote:
> > From: Stefano Stabellini <sstabellini@kernel.org>
> > 
> > There is no need to have a special case for CPU0 when converting the
> > page-table virtual address into a physical address. The helper
> > virt_to_maddr() is able to translate any address as long as the root
> > page-tables is mapped in the virtual address. This is the case for all
> > the CPUs at the moment.
> > 
> > So use the same BUG_ON() regardless the CPU.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > [julien: Rework the commit message]
> > Signed-off-by: Julien Grall <jgrall@amazon.com>
> > 
> > ---
> > 
> > I went back through the conversation in [1] regarding the issue when
> > loading Xen below 2MB on Arm32. The example provided is wrong because to
> > find the physical address, we need to add 'phys_offset', not substract.
> > 
> > So I removed the comment regarding the code was buggy.
> > 
> > [1] https://marc.info/?l=xen-devel&m=157081398022401
> 
> Stefano, can you confirm that you are happy with the new commit message?

Yes, that's OK
diff mbox series

Patch

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 9c4b26bf079b..4dd886f7c80d 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -284,10 +284,7 @@  void dump_hyp_walk(vaddr_t addr)
            "on CPU%d via TTBR 0x%016"PRIx64"\n",
            addr, smp_processor_id(), ttbr);
 
-    if ( smp_processor_id() == 0 )
-        BUG_ON( (lpae_t *)(unsigned long)(ttbr - phys_offset) != pgtable );
-    else
-        BUG_ON( virt_to_maddr(pgtable) != ttbr );
+    BUG_ON( virt_to_maddr(pgtable) != ttbr );
     dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
 }