CONFIG_EARLY_UART_SIZE is introduced to let user provide physical size of
early UART. Unlike MMU where we map a page in the virtual address space,
here we need to know the exact physical size to be mapped.
As VA == PA in case of MPU, the memory layout follows exactly the hardware
configuration. As a consequence, we set EARLY_UART_VIRTUAL_ADDRESS as physical
address.
EARLY_UART_BASE_ADDRESS and EARLY_UART_SIZE should be aligned to the minimum
size of MPU region (ie 64 bits) as per the hardware restrictions. Refer ARM
DDI 0600A.d ID120821 A1.3 "A minimum protection region size of 64 bytes.".
UART is mapped as nGnRE region (as specified by ATTR=100 , refer G1.3.13,
MAIR_EL2, "---0100 Device memory nGnRE") and Doc ID - 102670_0101_02_en
Table 4-3, Armv8 architecture memory types (nGnRE - Corresponds to Device in
Armv7 architecture). Also, it is mapped as outer shareable, RW at EL2 only
and execution of instructions from the region is not permitted.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
Changes from :-
v1 - 1. UART base address and size should be aligned to the minimum size of MPU
region (and not PAGE_SIZE).
xen/arch/arm/Kconfig.debug | 7 +++++++
xen/arch/arm/arm64/mpu/head.S | 9 ++++++++
xen/arch/arm/include/asm/early_printk.h | 28 ++++++++++++++++++++++++-
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
index 7660e599c0..84a0616102 100644
--- a/xen/arch/arm/Kconfig.debug
+++ b/xen/arch/arm/Kconfig.debug
@@ -121,6 +121,13 @@ config EARLY_UART_BASE_ADDRESS
hex "Early printk, physical base address of debug UART"
range 0x0 0xffffffff if ARM_32
+config EARLY_UART_SIZE
+ depends on EARLY_PRINTK
+ depends on MPU
+ hex "Early printk, physical size of debug UART"
+ range 0x0 0xffffffff if ARM_32
+ default 0x1000
+
config EARLY_UART_PL011_BAUD_RATE
depends on EARLY_UART_PL011
int "Early printk UART baud rate for pl011"
diff --git a/xen/arch/arm/arm64/mpu/head.S b/xen/arch/arm/arm64/mpu/head.S
index f692fc7443..86e4019a0c 100644
--- a/xen/arch/arm/arm64/mpu/head.S
+++ b/xen/arch/arm/arm64/mpu/head.S
@@ -11,8 +11,10 @@
#define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */
#define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */
#define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */
+#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */
#define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */
+#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */
/*
* Macro to prepare and set a EL2 MPU memory region.
@@ -138,6 +140,13 @@ FUNC(enable_boot_cpu_mm)
ldr x2, =__bss_end
prepare_xen_region x0, x1, x2, x3, x4, x5
+#ifdef CONFIG_EARLY_PRINTK
+ /* Xen early UART section. */
+ ldr x1, =CONFIG_EARLY_UART_BASE_ADDRESS
+ ldr x2, =(CONFIG_EARLY_UART_BASE_ADDRESS + CONFIG_EARLY_UART_SIZE)
+ prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR
+#endif
+
b enable_mpu
ret
END(enable_boot_cpu_mm)
diff --git a/xen/arch/arm/include/asm/early_printk.h b/xen/arch/arm/include/asm/early_printk.h
index 46a5e562dd..606aaedd6e 100644
--- a/xen/arch/arm/include/asm/early_printk.h
+++ b/xen/arch/arm/include/asm/early_printk.h
@@ -15,6 +15,29 @@
#ifdef CONFIG_EARLY_PRINTK
+#if defined(CONFIG_MPU)
+
+/*
+ * For MPU systems, there is no VMSA support in EL2, so we use VA == PA
+ * for EARLY_UART_VIRTUAL_ADDRESS.
+ */
+#define EARLY_UART_VIRTUAL_ADDRESS CONFIG_EARLY_UART_BASE_ADDRESS
+
+/*
+ * User-defined EARLY_UART_BASE_ADDRESS and EARLY_UART_SIZE must be aligned to
+ * minimum size of MPU region.
+ */
+
+#if (EARLY_UART_BASE_ADDRESS % MPU_REGION_ALIGN) != 0
+#error "EARLY_UART_BASE_ADDRESS must be aligned to minimum MPU region size"
+#endif
+
+#if (EARLY_UART_SIZE % MPU_REGION_ALIGN) != 0
+#error "EARLY_UART_SIZE must be aligned to minimum MPU region size"
+#endif
+
+#elif defined(CONFIG_MMU)
+
/* need to add the uart address offset in page to the fixmap address */
#define EARLY_UART_VIRTUAL_ADDRESS \
(FIXMAP_ADDR(FIX_CONSOLE) + (CONFIG_EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
@@ -22,6 +45,9 @@
#define TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS \
(TEMPORARY_FIXMAP_ADDR(FIX_CONSOLE) + (CONFIG_EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
-#endif /* !CONFIG_EARLY_PRINTK */
+#else
+#error "Unknown Memory management system"
+#endif
+#endif /* !CONFIG_EARLY_PRINTK */
#endif
--
2.25.1Hi Ayan, sorry for the separate message, I've spotted another thing where I’m in doubt > On 27 Nov 2024, at 18:39, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote: > > CONFIG_EARLY_UART_SIZE is introduced to let user provide physical size of > early UART. Unlike MMU where we map a page in the virtual address space, > here we need to know the exact physical size to be mapped. > As VA == PA in case of MPU, the memory layout follows exactly the hardware > configuration. As a consequence, we set EARLY_UART_VIRTUAL_ADDRESS as physical > address. > > EARLY_UART_BASE_ADDRESS and EARLY_UART_SIZE should be aligned to the minimum > size of MPU region (ie 64 bits) as per the hardware restrictions. Refer ARM > DDI 0600A.d ID120821 A1.3 "A minimum protection region size of 64 bytes.". > > UART is mapped as nGnRE region (as specified by ATTR=100 , refer G1.3.13, > MAIR_EL2, "---0100 Device memory nGnRE") and Doc ID - 102670_0101_02_en > Table 4-3, Armv8 architecture memory types (nGnRE - Corresponds to Device in > Armv7 architecture). Also, it is mapped as outer shareable, RW at EL2 only > and execution of instructions from the region is not permitted. > [...] > diff --git a/xen/arch/arm/arm64/mpu/head.S b/xen/arch/arm/arm64/mpu/head.S > index f692fc7443..86e4019a0c 100644 > --- a/xen/arch/arm/arm64/mpu/head.S > +++ b/xen/arch/arm/arm64/mpu/head.S > @@ -11,8 +11,10 @@ > #define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */ > #define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */ > #define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */ > +#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */ > > #define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ > +#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ Should this point to ATTR=0 instead? From what I see on Zephyr, the pl011 is mapped with nGnRnE, on R82 this works fine because it will treat all device memory as nGnRnE, but I’m not sure that this will work well on other Armv8-R aarch64 platforms. I was trying to check how Linux maps pl011 but I’m kind of lost, so maybe if anyone has more experience is more than welcome to contribute to the discussion. Cheers, Luca
> On 3 Dec 2024, at 15:54, Luca Fancellu <Luca.Fancellu@arm.com> wrote: > > Hi Ayan, > > sorry for the separate message, I've spotted another thing where I’m in doubt > >> On 27 Nov 2024, at 18:39, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote: >> >> CONFIG_EARLY_UART_SIZE is introduced to let user provide physical size of >> early UART. Unlike MMU where we map a page in the virtual address space, >> here we need to know the exact physical size to be mapped. >> As VA == PA in case of MPU, the memory layout follows exactly the hardware >> configuration. As a consequence, we set EARLY_UART_VIRTUAL_ADDRESS as physical >> address. >> >> EARLY_UART_BASE_ADDRESS and EARLY_UART_SIZE should be aligned to the minimum >> size of MPU region (ie 64 bits) as per the hardware restrictions. Refer ARM >> DDI 0600A.d ID120821 A1.3 "A minimum protection region size of 64 bytes.". >> >> UART is mapped as nGnRE region (as specified by ATTR=100 , refer G1.3.13, >> MAIR_EL2, "---0100 Device memory nGnRE") and Doc ID - 102670_0101_02_en >> Table 4-3, Armv8 architecture memory types (nGnRE - Corresponds to Device in >> Armv7 architecture). Also, it is mapped as outer shareable, RW at EL2 only >> and execution of instructions from the region is not permitted. >> > > [...] > > >> diff --git a/xen/arch/arm/arm64/mpu/head.S b/xen/arch/arm/arm64/mpu/head.S >> index f692fc7443..86e4019a0c 100644 >> --- a/xen/arch/arm/arm64/mpu/head.S >> +++ b/xen/arch/arm/arm64/mpu/head.S >> @@ -11,8 +11,10 @@ >> #define REGION_TEXT_PRBAR 0x38 /* SH=11 AP=10 XN=00 */ >> #define REGION_RO_PRBAR 0x3A /* SH=11 AP=10 XN=10 */ >> #define REGION_DATA_PRBAR 0x32 /* SH=11 AP=00 XN=10 */ >> +#define REGION_DEVICE_PRBAR 0x22 /* SH=10 AP=00 XN=10 */ >> >> #define REGION_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ >> +#define REGION_DEVICE_PRLAR 0x09 /* NS=0 ATTR=100 EN=1 */ > > Should this point to ATTR=0 instead? From what I see on Zephyr, the pl011 is > mapped with nGnRnE, on R82 this works fine because it will treat all device memory as > nGnRnE, but I’m not sure that this will work well on other Armv8-R aarch64 platforms. > > I was trying to check how Linux maps pl011 but I’m kind of lost, so maybe if anyone has > more experience is more than welcome to contribute to the discussion. Anyway, changing that to 0x01 (ATTR=0 EN=1) is giving me a weird issue in my branch: "Error getting IRQ number for this serial port 0” from create_domUs(), so I’m not sure now if the right value was indeed 0x09
Hi Ayan,
sorry I've just spotted some issues while compiling
> diff --git a/xen/arch/arm/include/asm/early_printk.h b/xen/arch/arm/include/asm/early_printk.h
> index 46a5e562dd..606aaedd6e 100644
> --- a/xen/arch/arm/include/asm/early_printk.h
> +++ b/xen/arch/arm/include/asm/early_printk.h
> @@ -15,6 +15,29 @@
you need to include
#include <asm/arm64/mpu.h>
in order to see MPU_REGION_ALIGN which otherwise will be zero for the preprocessor
>
> #ifdef CONFIG_EARLY_PRINTK
>
> +#if defined(CONFIG_MPU)
> +
> +/*
> + * For MPU systems, there is no VMSA support in EL2, so we use VA == PA
> + * for EARLY_UART_VIRTUAL_ADDRESS.
> + */
> +#define EARLY_UART_VIRTUAL_ADDRESS CONFIG_EARLY_UART_BASE_ADDRESS
> +
> +/*
> + * User-defined EARLY_UART_BASE_ADDRESS and EARLY_UART_SIZE must be aligned to
> + * minimum size of MPU region.
> + */
> +
> +#if (EARLY_UART_BASE_ADDRESS % MPU_REGION_ALIGN) != 0
^— This needs to be CONFIG_*
> +#error "EARLY_UART_BASE_ADDRESS must be aligned to minimum MPU region size"
> +#endif
> +
> +#if (EARLY_UART_SIZE % MPU_REGION_ALIGN) != 0
^— This needs to be CONFIG_*
Once these are addressed and taking care also of Julien's comment:
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com <mailto:luca.fancellu@arm.com>>
Cheers,
Luca
Hi Ayan, On 27/11/2024 18:39, Ayan Kumar Halder wrote: > CONFIG_EARLY_UART_SIZE is introduced to let user provide physical size of > early UART. Unlike MMU where we map a page in the virtual address space, > here we need to know the exact physical size to be mapped. > As VA == PA in case of MPU, the memory layout follows exactly the hardware > configuration. As a consequence, we set EARLY_UART_VIRTUAL_ADDRESS as physical > address. > > EARLY_UART_BASE_ADDRESS and EARLY_UART_SIZE should be aligned to the minimum > size of MPU region (ie 64 bits) as per the hardware restrictions. Refer ARM > DDI 0600A.d ID120821 A1.3 "A minimum protection region size of 64 bytes.". > > UART is mapped as nGnRE region (as specified by ATTR=100 , refer G1.3.13, > MAIR_EL2, "---0100 Device memory nGnRE") and Doc ID - 102670_0101_02_en I can't find the Doc you point online. Do you have a link? > Table 4-3, Armv8 architecture memory types (nGnRE - Corresponds to Device in > Armv7 architecture). Also, it is mapped as outer shareable, RW at EL2 only I don't quite understand why you mention Armv7 here. The code you modify below is 64-bit so Armv8 only. The code itself LGTM. Cheers, -- Julien Grall
On 02/12/2024 20:53, Julien Grall wrote: > Hi Ayan, Hi Julien, > > On 27/11/2024 18:39, Ayan Kumar Halder wrote: >> CONFIG_EARLY_UART_SIZE is introduced to let user provide physical >> size of >> early UART. Unlike MMU where we map a page in the virtual address space, >> here we need to know the exact physical size to be mapped. >> As VA == PA in case of MPU, the memory layout follows exactly the >> hardware >> configuration. As a consequence, we set EARLY_UART_VIRTUAL_ADDRESS as >> physical >> address. >> >> EARLY_UART_BASE_ADDRESS and EARLY_UART_SIZE should be aligned to the >> minimum >> size of MPU region (ie 64 bits) as per the hardware restrictions. >> Refer ARM >> DDI 0600A.d ID120821 A1.3 "A minimum protection region size of 64 >> bytes.". >> >> UART is mapped as nGnRE region (as specified by ATTR=100 , refer >> G1.3.13, >> MAIR_EL2, "---0100 Device memory nGnRE") and Doc ID - 102670_0101_02_en > > I can't find the Doc you point online. Do you have a link? https://developer.arm.com/documentation/102670/0101 - Cortex-R82 processor TRM > >> Table 4-3, Armv8 architecture memory types (nGnRE - Corresponds to >> Device in >> Armv7 architecture). Also, it is mapped as outer shareable, RW at EL2 >> only > > I don't quite understand why you mention Armv7 here. Actually I was quoting from Cortex-R82 TRM. > The code you modify below is 64-bit so Armv8 only. > > The code itself LGTM. Ack. - Ayan > > Cheers, >
On 03/12/2024 13:34, Ayan Kumar Halder wrote: > > On 02/12/2024 20:53, Julien Grall wrote: >> Hi Ayan, > Hi Julien, >> >> On 27/11/2024 18:39, Ayan Kumar Halder wrote: >>> CONFIG_EARLY_UART_SIZE is introduced to let user provide physical >>> size of >>> early UART. Unlike MMU where we map a page in the virtual address space, >>> here we need to know the exact physical size to be mapped. >>> As VA == PA in case of MPU, the memory layout follows exactly the >>> hardware >>> configuration. As a consequence, we set EARLY_UART_VIRTUAL_ADDRESS as >>> physical >>> address. >>> >>> EARLY_UART_BASE_ADDRESS and EARLY_UART_SIZE should be aligned to the >>> minimum >>> size of MPU region (ie 64 bits) as per the hardware restrictions. >>> Refer ARM >>> DDI 0600A.d ID120821 A1.3 "A minimum protection region size of 64 >>> bytes.". >>> >>> UART is mapped as nGnRE region (as specified by ATTR=100 , refer >>> G1.3.13, >>> MAIR_EL2, "---0100 Device memory nGnRE") and Doc ID - 102670_0101_02_en >> >> I can't find the Doc you point online. Do you have a link? > https://developer.arm.com/documentation/102670/0101 - Cortex-R82 > processor TRM Thanks. But why are you quoting the Cortex-R82 TRM? This code is meant to work on any Armv8-R processor. Cheers, -- Julien Grall
Hi Julien and Luca, (Luca, I will respond to your other comment here) On 03/12/2024 13:50, Julien Grall wrote: > > > On 03/12/2024 13:34, Ayan Kumar Halder wrote: >> >> On 02/12/2024 20:53, Julien Grall wrote: >>> Hi Ayan, >> Hi Julien, >>> >>> On 27/11/2024 18:39, Ayan Kumar Halder wrote: >>>> CONFIG_EARLY_UART_SIZE is introduced to let user provide physical >>>> size of >>>> early UART. Unlike MMU where we map a page in the virtual address >>>> space, >>>> here we need to know the exact physical size to be mapped. >>>> As VA == PA in case of MPU, the memory layout follows exactly the >>>> hardware >>>> configuration. As a consequence, we set EARLY_UART_VIRTUAL_ADDRESS >>>> as physical >>>> address. >>>> >>>> EARLY_UART_BASE_ADDRESS and EARLY_UART_SIZE should be aligned to >>>> the minimum >>>> size of MPU region (ie 64 bits) as per the hardware restrictions. >>>> Refer ARM >>>> DDI 0600A.d ID120821 A1.3 "A minimum protection region size of 64 >>>> bytes.". >>>> >>>> UART is mapped as nGnRE region (as specified by ATTR=100 , refer >>>> G1.3.13, >>>> MAIR_EL2, "---0100 Device memory nGnRE") and Doc ID - >>>> 102670_0101_02_en >>> >>> I can't find the Doc you point online. Do you have a link? >> https://developer.arm.com/documentation/102670/0101 - Cortex-R82 >> processor TRM > > Thanks. But why are you quoting the Cortex-R82 TRM? > This code is meant to work on any Armv8-R processor. You mean Armv8-R AArch64 processor. Actually, I was looking for a reference to tell me if UART is to be mapped as a nGnRE or nGnRnE. Table 4-3 mentions that nGnRE corresponds to the device memory. So, I used this. I assumed that this behavior will apply to all Armv8-R AArch64 processors as it did not mention anything specific about R82. To address Luca's comment >>"on Zephyr, the pl011 is mapped with nGnRnE". I don't really have a good reason whether we should map it as nGnRnE or nGnRE other than what I have mentioned before (ie nGnRE corresponds to device memory). I am happy to change it to make it consistent with Zephyr and Linux (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/include/asm/mpu.h?h=v6.13-rc1#n67 , this is Armv8-R **AArch32** ). Let me know which one you think is correct. Other comment from Luca >>"Anyway, changing that to 0x01 (ATTR=0 EN=1) is giving me a weird issue in my branch:" This is due to https://gitlab.com/xen-project/people/lucafancellu/xen/-/commit/4ec46883b1dffcbafd86c32732d1267102696d84 , |ioremap_cache() in arm/mpu/mm.c . This can be fixed if we choose nGnRnE.| |- Ayan|
© 2016 - 2024 Red Hat, Inc.