| Message ID | 20220812192448.43016-4-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: More clean-ups and improvement | expand |
Hi Julien, On 2022/8/13 3:24, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > There are a few places in the code that need to find the slot at a > given page-table level. > > So create a new macro get_table_slot() for that. This will reduce > the effort to figure out whether the code is doing the right thing. > > The new macro is using 'ubfx' (or 'lsr' for the first level) rather > than the existing sequence (mov_w, lsr, and) because it doesn't require > a scratch register and reduce the number of instructions (4 -> 1). > > Signed-off-by: Julien Grall <jgrall@amazon.com> > --- > xen/arch/arm/arm32/head.S | 56 ++++++++++++++++++++++----------------- > 1 file changed, 32 insertions(+), 24 deletions(-) > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S > index 46d93bebbabe..50f6fa4eb38d 100644 > --- a/xen/arch/arm/arm32/head.S > +++ b/xen/arch/arm/arm32/head.S > @@ -358,13 +358,31 @@ cpu_init_done: > mov pc, r5 /* Return address is in r5 */ > ENDPROC(cpu_init) > > +/* > + * Macro to find the slot number at a given page-table level > + * > + * slot: slot computed > + * virt: virtual address > + * lvl: page-table level > + * > + * Note that ubxf is unpredictable when the end bit is above 32-bit. So we > + * can't use it for first level offset. > + */ > +.macro get_table_slot, slot, virt, lvl > + .if \lvl == 1 > + lsr \slot, \virt, #XEN_PT_LEVEL_SHIFT(\lvl) > + .else > + ubfx \slot, \virt, #XEN_PT_LEVEL_SHIFT(\lvl), #XEN_PT_LPAE_SHIFT > + .endif > +.endm > + > /* > * Macro to create a page table entry in \ptbl to \tbl > * > * ptbl: table symbol where the entry will be created > * tbl: table symbol to point to > * virt: virtual address > - * shift: #imm page table shift > + * lvl: page-table level > * mmu: Is the MMU turned on/off. If not specified it will be off > * > * Preserves \virt > @@ -374,11 +392,9 @@ ENDPROC(cpu_init) > * > * Note that \virt should be in a register other than r1 - r4 > */ > -.macro create_table_entry, ptbl, tbl, virt, shift, mmu=0 > - lsr r1, \virt, #\shift > - mov_w r2, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r2 /* r1 := slot in \tlb */ > - lsl r1, r1, #3 /* r1 := slot offset in \tlb */ > +.macro create_table_entry, ptbl, tbl, virt, lvl, mmu=0 > + get_table_slot r1, \virt, \lvl /* r1 := slot in \tlb */ > + lsl r1, r1, #3 /* r1 := slot offset in \tlb */ > > load_paddr r4, \tbl > > @@ -448,8 +464,8 @@ ENDPROC(cpu_init) > create_page_tables: > /* Prepare the page-tables for mapping Xen */ > ldr r0, =XEN_VIRT_START > - create_table_entry boot_pgtable, boot_second, r0, FIRST_SHIFT > - create_table_entry boot_second, boot_third, r0, SECOND_SHIFT > + create_table_entry boot_pgtable, boot_second, r0, 1 > + create_table_entry boot_second, boot_third, r0, 2 > > /* Setup boot_third: */ > adr_l r4, boot_third, mmu=0 > @@ -486,12 +502,10 @@ create_page_tables: > * then the 1:1 mapping will use its own set of page-tables from > * the second level. > */ > - lsr r1, r9, #FIRST_SHIFT > - mov_w r0, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r0 /* r1 := first slot */ > + get_table_slot r1, r9, 1 /* r1 := first slot */ > cmp r1, #XEN_FIRST_SLOT > beq 1f > - create_table_entry boot_pgtable, boot_second_id, r9, FIRST_SHIFT > + create_table_entry boot_pgtable, boot_second_id, r9, 1 > b link_from_second_id > > 1: > @@ -501,16 +515,14 @@ create_page_tables: > * third level. For slot XEN_SECOND_SLOT, Xen is not yet able to handle > * it. > */ > - lsr r1, r9, #SECOND_SHIFT > - mov_w r0, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r0 /* r1 := second slot */ > + get_table_slot r1, r9, 2 /* r1 := second slot */ > cmp r1, #XEN_SECOND_SLOT > beq virtphys_clash > - create_table_entry boot_second, boot_third_id, r9, SECOND_SHIFT > + create_table_entry boot_second, boot_third_id, r9, 2 > b link_from_third_id > > link_from_second_id: > - create_table_entry boot_second_id, boot_third_id, r9, SECOND_SHIFT > + create_table_entry boot_second_id, boot_third_id, r9, 2 > link_from_third_id: > create_mapping_entry boot_third_id, r9, r9 > mov pc, lr > @@ -571,9 +583,7 @@ remove_identity_mapping: > * Find the first slot used. Remove the entry for the first > * table if the slot is not XEN_FIRST_SLOT. > */ > - lsr r1, r9, #FIRST_SHIFT > - mov_w r0, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r0 /* r1 := first slot */ > + get_table_slot r1, r9, 1 /* r1 := first slot */ > cmp r1, #XEN_FIRST_SLOT > beq 1f > /* It is not in slot 0, remove the entry */ > @@ -587,9 +597,7 @@ remove_identity_mapping: > * Find the second slot used. Remove the entry for the first > * table if the slot is not XEN_SECOND_SLOT. > */ > - lsr r1, r9, #SECOND_SHIFT > - mov_w r0, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r0 /* r1 := second slot */ > + get_table_slot r1, r9, 2 /* r1 := second slot */ > cmp r1, #XEN_SECOND_SLOT > beq identity_mapping_removed > /* It is not in slot 1, remove the entry */ > @@ -628,7 +636,7 @@ setup_fixmap: > #endif > /* Map fixmap into boot_second */ > mov_w r0, FIXMAP_ADDR(0) > - create_table_entry boot_second, xen_fixmap, r0, SECOND_SHIFT, mmu=1 > + create_table_entry boot_second, xen_fixmap, r0, 2, mmu=1 > /* Ensure any page table updates made above have occurred. */ > dsb nshst > Reviewed-by: Wei Chen <Wei.Chen@arm.com>
Hi Julien, > On 12 Aug 2022, at 20:24, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <jgrall@amazon.com> > > There are a few places in the code that need to find the slot at a > given page-table level. > > So create a new macro get_table_slot() for that. This will reduce > the effort to figure out whether the code is doing the right thing. > > The new macro is using 'ubfx' (or 'lsr' for the first level) rather > than the existing sequence (mov_w, lsr, and) because it doesn't require > a scratch register and reduce the number of instructions (4 -> 1). > > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Also I passed our test suite on arm32 qemu so: Tested-by: Bertrand Marquis <bertrand.marquis@arm.com> Cheers Bertrand > --- > xen/arch/arm/arm32/head.S | 56 ++++++++++++++++++++++----------------- > 1 file changed, 32 insertions(+), 24 deletions(-) > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S > index 46d93bebbabe..50f6fa4eb38d 100644 > --- a/xen/arch/arm/arm32/head.S > +++ b/xen/arch/arm/arm32/head.S > @@ -358,13 +358,31 @@ cpu_init_done: > mov pc, r5 /* Return address is in r5 */ > ENDPROC(cpu_init) > > +/* > + * Macro to find the slot number at a given page-table level > + * > + * slot: slot computed > + * virt: virtual address > + * lvl: page-table level > + * > + * Note that ubxf is unpredictable when the end bit is above 32-bit. So we > + * can't use it for first level offset. > + */ > +.macro get_table_slot, slot, virt, lvl > + .if \lvl == 1 > + lsr \slot, \virt, #XEN_PT_LEVEL_SHIFT(\lvl) > + .else > + ubfx \slot, \virt, #XEN_PT_LEVEL_SHIFT(\lvl), #XEN_PT_LPAE_SHIFT > + .endif > +.endm > + > /* > * Macro to create a page table entry in \ptbl to \tbl > * > * ptbl: table symbol where the entry will be created > * tbl: table symbol to point to > * virt: virtual address > - * shift: #imm page table shift > + * lvl: page-table level > * mmu: Is the MMU turned on/off. If not specified it will be off > * > * Preserves \virt > @@ -374,11 +392,9 @@ ENDPROC(cpu_init) > * > * Note that \virt should be in a register other than r1 - r4 > */ > -.macro create_table_entry, ptbl, tbl, virt, shift, mmu=0 > - lsr r1, \virt, #\shift > - mov_w r2, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r2 /* r1 := slot in \tlb */ > - lsl r1, r1, #3 /* r1 := slot offset in \tlb */ > +.macro create_table_entry, ptbl, tbl, virt, lvl, mmu=0 > + get_table_slot r1, \virt, \lvl /* r1 := slot in \tlb */ > + lsl r1, r1, #3 /* r1 := slot offset in \tlb */ > > load_paddr r4, \tbl > > @@ -448,8 +464,8 @@ ENDPROC(cpu_init) > create_page_tables: > /* Prepare the page-tables for mapping Xen */ > ldr r0, =XEN_VIRT_START > - create_table_entry boot_pgtable, boot_second, r0, FIRST_SHIFT > - create_table_entry boot_second, boot_third, r0, SECOND_SHIFT > + create_table_entry boot_pgtable, boot_second, r0, 1 > + create_table_entry boot_second, boot_third, r0, 2 > > /* Setup boot_third: */ > adr_l r4, boot_third, mmu=0 > @@ -486,12 +502,10 @@ create_page_tables: > * then the 1:1 mapping will use its own set of page-tables from > * the second level. > */ > - lsr r1, r9, #FIRST_SHIFT > - mov_w r0, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r0 /* r1 := first slot */ > + get_table_slot r1, r9, 1 /* r1 := first slot */ > cmp r1, #XEN_FIRST_SLOT > beq 1f > - create_table_entry boot_pgtable, boot_second_id, r9, FIRST_SHIFT > + create_table_entry boot_pgtable, boot_second_id, r9, 1 > b link_from_second_id > > 1: > @@ -501,16 +515,14 @@ create_page_tables: > * third level. For slot XEN_SECOND_SLOT, Xen is not yet able to handle > * it. > */ > - lsr r1, r9, #SECOND_SHIFT > - mov_w r0, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r0 /* r1 := second slot */ > + get_table_slot r1, r9, 2 /* r1 := second slot */ > cmp r1, #XEN_SECOND_SLOT > beq virtphys_clash > - create_table_entry boot_second, boot_third_id, r9, SECOND_SHIFT > + create_table_entry boot_second, boot_third_id, r9, 2 > b link_from_third_id > > link_from_second_id: > - create_table_entry boot_second_id, boot_third_id, r9, SECOND_SHIFT > + create_table_entry boot_second_id, boot_third_id, r9, 2 > link_from_third_id: > create_mapping_entry boot_third_id, r9, r9 > mov pc, lr > @@ -571,9 +583,7 @@ remove_identity_mapping: > * Find the first slot used. Remove the entry for the first > * table if the slot is not XEN_FIRST_SLOT. > */ > - lsr r1, r9, #FIRST_SHIFT > - mov_w r0, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r0 /* r1 := first slot */ > + get_table_slot r1, r9, 1 /* r1 := first slot */ > cmp r1, #XEN_FIRST_SLOT > beq 1f > /* It is not in slot 0, remove the entry */ > @@ -587,9 +597,7 @@ remove_identity_mapping: > * Find the second slot used. Remove the entry for the first > * table if the slot is not XEN_SECOND_SLOT. > */ > - lsr r1, r9, #SECOND_SHIFT > - mov_w r0, XEN_PT_LPAE_ENTRY_MASK > - and r1, r1, r0 /* r1 := second slot */ > + get_table_slot r1, r9, 2 /* r1 := second slot */ > cmp r1, #XEN_SECOND_SLOT > beq identity_mapping_removed > /* It is not in slot 1, remove the entry */ > @@ -628,7 +636,7 @@ setup_fixmap: > #endif > /* Map fixmap into boot_second */ > mov_w r0, FIXMAP_ADDR(0) > - create_table_entry boot_second, xen_fixmap, r0, SECOND_SHIFT, mmu=1 > + create_table_entry boot_second, xen_fixmap, r0, 2, mmu=1 > /* Ensure any page table updates made above have occurred. */ > dsb nshst > > -- > 2.37.1 >
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 46d93bebbabe..50f6fa4eb38d 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -358,13 +358,31 @@ cpu_init_done: mov pc, r5 /* Return address is in r5 */ ENDPROC(cpu_init) +/* + * Macro to find the slot number at a given page-table level + * + * slot: slot computed + * virt: virtual address + * lvl: page-table level + * + * Note that ubxf is unpredictable when the end bit is above 32-bit. So we + * can't use it for first level offset. + */ +.macro get_table_slot, slot, virt, lvl + .if \lvl == 1 + lsr \slot, \virt, #XEN_PT_LEVEL_SHIFT(\lvl) + .else + ubfx \slot, \virt, #XEN_PT_LEVEL_SHIFT(\lvl), #XEN_PT_LPAE_SHIFT + .endif +.endm + /* * Macro to create a page table entry in \ptbl to \tbl * * ptbl: table symbol where the entry will be created * tbl: table symbol to point to * virt: virtual address - * shift: #imm page table shift + * lvl: page-table level * mmu: Is the MMU turned on/off. If not specified it will be off * * Preserves \virt @@ -374,11 +392,9 @@ ENDPROC(cpu_init) * * Note that \virt should be in a register other than r1 - r4 */ -.macro create_table_entry, ptbl, tbl, virt, shift, mmu=0 - lsr r1, \virt, #\shift - mov_w r2, XEN_PT_LPAE_ENTRY_MASK - and r1, r1, r2 /* r1 := slot in \tlb */ - lsl r1, r1, #3 /* r1 := slot offset in \tlb */ +.macro create_table_entry, ptbl, tbl, virt, lvl, mmu=0 + get_table_slot r1, \virt, \lvl /* r1 := slot in \tlb */ + lsl r1, r1, #3 /* r1 := slot offset in \tlb */ load_paddr r4, \tbl @@ -448,8 +464,8 @@ ENDPROC(cpu_init) create_page_tables: /* Prepare the page-tables for mapping Xen */ ldr r0, =XEN_VIRT_START - create_table_entry boot_pgtable, boot_second, r0, FIRST_SHIFT - create_table_entry boot_second, boot_third, r0, SECOND_SHIFT + create_table_entry boot_pgtable, boot_second, r0, 1 + create_table_entry boot_second, boot_third, r0, 2 /* Setup boot_third: */ adr_l r4, boot_third, mmu=0 @@ -486,12 +502,10 @@ create_page_tables: * then the 1:1 mapping will use its own set of page-tables from * the second level. */ - lsr r1, r9, #FIRST_SHIFT - mov_w r0, XEN_PT_LPAE_ENTRY_MASK - and r1, r1, r0 /* r1 := first slot */ + get_table_slot r1, r9, 1 /* r1 := first slot */ cmp r1, #XEN_FIRST_SLOT beq 1f - create_table_entry boot_pgtable, boot_second_id, r9, FIRST_SHIFT + create_table_entry boot_pgtable, boot_second_id, r9, 1 b link_from_second_id 1: @@ -501,16 +515,14 @@ create_page_tables: * third level. For slot XEN_SECOND_SLOT, Xen is not yet able to handle * it. */ - lsr r1, r9, #SECOND_SHIFT - mov_w r0, XEN_PT_LPAE_ENTRY_MASK - and r1, r1, r0 /* r1 := second slot */ + get_table_slot r1, r9, 2 /* r1 := second slot */ cmp r1, #XEN_SECOND_SLOT beq virtphys_clash - create_table_entry boot_second, boot_third_id, r9, SECOND_SHIFT + create_table_entry boot_second, boot_third_id, r9, 2 b link_from_third_id link_from_second_id: - create_table_entry boot_second_id, boot_third_id, r9, SECOND_SHIFT + create_table_entry boot_second_id, boot_third_id, r9, 2 link_from_third_id: create_mapping_entry boot_third_id, r9, r9 mov pc, lr @@ -571,9 +583,7 @@ remove_identity_mapping: * Find the first slot used. Remove the entry for the first * table if the slot is not XEN_FIRST_SLOT. */ - lsr r1, r9, #FIRST_SHIFT - mov_w r0, XEN_PT_LPAE_ENTRY_MASK - and r1, r1, r0 /* r1 := first slot */ + get_table_slot r1, r9, 1 /* r1 := first slot */ cmp r1, #XEN_FIRST_SLOT beq 1f /* It is not in slot 0, remove the entry */ @@ -587,9 +597,7 @@ remove_identity_mapping: * Find the second slot used. Remove the entry for the first * table if the slot is not XEN_SECOND_SLOT. */ - lsr r1, r9, #SECOND_SHIFT - mov_w r0, XEN_PT_LPAE_ENTRY_MASK - and r1, r1, r0 /* r1 := second slot */ + get_table_slot r1, r9, 2 /* r1 := second slot */ cmp r1, #XEN_SECOND_SLOT beq identity_mapping_removed /* It is not in slot 1, remove the entry */ @@ -628,7 +636,7 @@ setup_fixmap: #endif /* Map fixmap into boot_second */ mov_w r0, FIXMAP_ADDR(0) - create_table_entry boot_second, xen_fixmap, r0, SECOND_SHIFT, mmu=1 + create_table_entry boot_second, xen_fixmap, r0, 2, mmu=1 /* Ensure any page table updates made above have occurred. */ dsb nshst