| Message ID | 20221022150422.17707-2-julien@xen.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Don't switch TTBR while the MMU is on | expand |
> On 22 Oct 2022, at 16:04, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <jgrall@amazon.com> > > In a follow-up patch, the base address for the common mappings will > vary between arm32 and arm64. To avoid any duplication, define > every mapping in the common region from the previous one. > > Take the opportunity to add missing *_SIZE for FIXMAP_VIRT_* and > XEN_VIRT_*. > > Take the opportunity to add missing *_SIZE for some mappings. > > Signed-off-by: Julien Grall <jgrall@amazon.com> > > ---- > Changes in v2: > - Use _AT(vaddr_t, ...) to build on 32-bit. > - Drop COMMON_VIRT_START > --- Hi Julien, Looks ok for me. Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Hi Julien, On 22/10/2022 17:04, Julien Grall wrote: > > > From: Julien Grall <jgrall@amazon.com> > > In a follow-up patch, the base address for the common mappings will > vary between arm32 and arm64. To avoid any duplication, define > every mapping in the common region from the previous one. Maybe the following title would be better suited if you only want to touch the common mappings? "xen/arm: Clean-up the common memory layout" > > Take the opportunity to add missing *_SIZE for FIXMAP_VIRT_* and > XEN_VIRT_*. > > Take the opportunity to add missing *_SIZE for some mappings. I think this sentence can be removed as the previous one already covers it. Apart from that, wouldn't it be useful to do the cleanup for the arm32 macros as you are here (FRAMETABLE, VMAP, {XEN/DOM}HEAP) ? ~Michal
On 25/10/2022 10:21, Michal Orzel wrote: > Hi Julien, Hi Michal, > On 22/10/2022 17:04, Julien Grall wrote: >> >> >> From: Julien Grall <jgrall@amazon.com> >> >> In a follow-up patch, the base address for the common mappings will >> vary between arm32 and arm64. To avoid any duplication, define >> every mapping in the common region from the previous one. > Maybe the following title would be better suited if you only want to touch the common mappings? > "xen/arm: Clean-up the common memory layout" Ok. > >> >> Take the opportunity to add missing *_SIZE for FIXMAP_VIRT_* and >> XEN_VIRT_*. >> >> Take the opportunity to add missing *_SIZE for some mappings. > I think this sentence can be removed as the previous one already covers it. Yes. I will remove it. > > Apart from that, wouldn't it be useful to do the cleanup for the arm32 macros > as you are here (FRAMETABLE, VMAP, {XEN/DOM}HEAP) ? Can you clarify? I looked at the macros and they look clean to me. Cheers,
Hi, On 25/10/2022 12:31, Julien Grall wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On 25/10/2022 10:21, Michal Orzel wrote: >> Hi Julien, > > Hi Michal, > > >> On 22/10/2022 17:04, Julien Grall wrote: >>> >>> >>> From: Julien Grall <jgrall@amazon.com> >>> >>> In a follow-up patch, the base address for the common mappings will >>> vary between arm32 and arm64. To avoid any duplication, define >>> every mapping in the common region from the previous one. >> Maybe the following title would be better suited if you only want to touch the common mappings? >> "xen/arm: Clean-up the common memory layout" > > Ok. > >> >>> >>> Take the opportunity to add missing *_SIZE for FIXMAP_VIRT_* and >>> XEN_VIRT_*. >>> >>> Take the opportunity to add missing *_SIZE for some mappings. >> I think this sentence can be removed as the previous one already covers it. > > Yes. I will remove it. > >> >> Apart from that, wouldn't it be useful to do the cleanup for the arm32 macros >> as you are here (FRAMETABLE, VMAP, {XEN/DOM}HEAP) ? > > Can you clarify? I looked at the macros and they look clean to me. > As you choose to make use of MB() macro, I think it would be beneficial just from the consistency perspective to modify other places that use directly hex values. We already do that for arm64 specific mappings, you are modifying the common ones to adhere to this way, so as a natural consequence I would see the arm32 ones to be modified as well. > Cheers, > > -- > Julien Grall ~Michal
On 25/10/2022 11:36, Michal Orzel wrote: > Hi, > > On 25/10/2022 12:31, Julien Grall wrote: >> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. >> >> >> On 25/10/2022 10:21, Michal Orzel wrote: >>> Hi Julien, >> >> Hi Michal, >> >> >>> On 22/10/2022 17:04, Julien Grall wrote: >>>> >>>> >>>> From: Julien Grall <jgrall@amazon.com> >>>> >>>> In a follow-up patch, the base address for the common mappings will >>>> vary between arm32 and arm64. To avoid any duplication, define >>>> every mapping in the common region from the previous one. >>> Maybe the following title would be better suited if you only want to touch the common mappings? >>> "xen/arm: Clean-up the common memory layout" >> >> Ok. >> >>> >>>> >>>> Take the opportunity to add missing *_SIZE for FIXMAP_VIRT_* and >>>> XEN_VIRT_*. >>>> >>>> Take the opportunity to add missing *_SIZE for some mappings. >>> I think this sentence can be removed as the previous one already covers it. >> >> Yes. I will remove it. >> >>> >>> Apart from that, wouldn't it be useful to do the cleanup for the arm32 macros >>> as you are here (FRAMETABLE, VMAP, {XEN/DOM}HEAP) ? >> >> Can you clarify? I looked at the macros and they look clean to me. >> > As you choose to make use of MB() macro, I think it would be beneficial > just from the consistency perspective to modify other places that use directly hex values. > We already do that for arm64 specific mappings, you are modifying the common ones > to adhere to this way, so as a natural consequence I would see the arm32 ones to be modified as well. You have a point. I will look to do it in this patch. Cheers,
diff --git a/xen/arch/arm/include/asm/config.h b/xen/arch/arm/include/asm/config.h index 0fefed1b8aa9..80d81f127f7e 100644 --- a/xen/arch/arm/include/asm/config.h +++ b/xen/arch/arm/include/asm/config.h @@ -107,14 +107,19 @@ * Unused */ -#define XEN_VIRT_START _AT(vaddr_t,0x00200000) -#define FIXMAP_ADDR(n) (_AT(vaddr_t,0x00400000) + (n) * PAGE_SIZE) +#define XEN_VIRT_START _AT(vaddr_t, MB(2)) +#define XEN_VIRT_SIZE _AT(vaddr_t, MB(2)) -#define BOOT_FDT_VIRT_START _AT(vaddr_t,0x00600000) -#define BOOT_FDT_VIRT_SIZE _AT(vaddr_t, MB(4)) +#define FIXMAP_VIRT_START (XEN_VIRT_START + XEN_VIRT_SIZE) +#define FIXMAP_VIRT_SIZE _AT(vaddr_t, MB(2)) + +#define FIXMAP_ADDR(n) (FIXMAP_VIRT_START + (n) * PAGE_SIZE) + +#define BOOT_FDT_VIRT_START (FIXMAP_VIRT_START + FIXMAP_VIRT_SIZE) +#define BOOT_FDT_VIRT_SIZE _AT(vaddr_t, MB(4)) #ifdef CONFIG_LIVEPATCH -#define LIVEPATCH_VMAP_START _AT(vaddr_t,0x00a00000) +#define LIVEPATCH_VMAP_START (BOOT_FDT_VIRT_START + BOOT_FDT_VIRT_SIZE) #define LIVEPATCH_VMAP_SIZE _AT(vaddr_t, MB(2)) #endif