| Message ID | 20230626033443.2943270-25-Penny.Zheng@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Add Armv8-R64 MPU support to Xen - Part#1 | expand |
Hi Penny, On 26/06/2023 04:34, Penny Zheng wrote: > CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email. > > > The start-of-day Xen MPU memory region layout shall be like > as follows: > > xen_mpumap[0] : Xen text > xen_mpumap[1] : Xen read-only data > xen_mpumap[2] : Xen read-only after init data > xen_mpumap[3] : Xen read-write data > xen_mpumap[4] : Xen BSS > xen_mpumap[5] : Xen init text > xen_mpumap[6] : Xen init data > > The layout shall be compliant with what we describe in xen.lds.S, > or the codes need adjustment. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > Signed-off-by: Wei Chen <wei.chen@arm.com> > --- > v3: > - cache maintanence for safety when modifying MPU memory mapping table > - Hardcode index for all data/text sections > - To make sure that alternative instructions are included, use "_einitext" > as the start of the "Init data" section. > --- < snip> > +/* > + * Static start-of-day Xen EL2 MPU memory region layout: > + * > + * xen_mpumap[0] : Xen text > + * xen_mpumap[1] : Xen read-only data > + * xen_mpumap[2] : Xen read-only after init data > + * xen_mpumap[3] : Xen read-write data > + * xen_mpumap[4] : Xen BSS > + * xen_mpumap[5] : Xen init text > + * xen_mpumap[6] : Xen init data > + * > + * Clobbers x0 - x6 > + * > + * It shall be compliant with what describes in xen.lds.S, or the below > + * codes need adjustment. > + */ > +ENTRY(prepare_early_mappings) > + /* x0: region sel */ > + mov x0, xzr > + /* Xen text section. */ > + load_paddr x1, _stext > + load_paddr x2, _etext > + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, attr_prbar=REGION_TEXT_PRBAR > + > + add x0, x0, #1 > + /* Xen read-only data section. */ > + load_paddr x1, _srodata > + load_paddr x2, _erodata > + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, attr_prbar=REGION_RO_PRBAR > + > + add x0, x0, #1 > + /* Xen read-only after init data section. */ > + load_paddr x1, __ro_after_init_start > + load_paddr x2, __ro_after_init_end > + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 > + > + add x0, x0, #1 > + /* Xen read-write data section. */ > + load_paddr x1, __ro_after_init_end > + load_paddr x2, __init_begin > + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 > + > + add x0, x0, #1 > + /* Xen BSS section. */ > + load_paddr x1, __bss_start > + load_paddr x2, __bss_end > + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 > + > + add x0, x0, #1 > + /* Xen init text section. */ > + load_paddr x1, _sinittext > + load_paddr x2, _einittext > + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, attr_prbar=REGION_TEXT_PRBAR > + > + add x0, x0, #1 > + /* Xen init data section. */ > + /* > + * Even though we are not using alternative instructions in MPU yet, > + * we want to use "_einitext" for the start of the "Init data" section > + * to make sure they are included. > + */ > + load_paddr x1, _einittext > + roundup_section x1 > + load_paddr x2, __init_end > + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 > + > + /* Ensure any MPU memory mapping table updates made above have occurred. */ > + dsb nshst > + ret > +ENDPROC(prepare_early_mappings) Any reason why this is in assembly ? We have implemented it in C https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/mm_mpu.c#L941 , so that it can be common between R82 and R52. - Ayan
Hi, On 28/06/2023 11:55, Ayan Kumar Halder wrote: > On 26/06/2023 04:34, Penny Zheng wrote: >> CAUTION: This message has originated from an External Source. Please >> use proper judgment and caution when opening attachments, clicking >> links, or responding to this email. >> >> >> The start-of-day Xen MPU memory region layout shall be like >> as follows: >> >> xen_mpumap[0] : Xen text >> xen_mpumap[1] : Xen read-only data >> xen_mpumap[2] : Xen read-only after init data >> xen_mpumap[3] : Xen read-write data >> xen_mpumap[4] : Xen BSS >> xen_mpumap[5] : Xen init text >> xen_mpumap[6] : Xen init data >> >> The layout shall be compliant with what we describe in xen.lds.S, >> or the codes need adjustment. >> >> Signed-off-by: Penny Zheng <penny.zheng@arm.com> >> Signed-off-by: Wei Chen <wei.chen@arm.com> >> --- >> v3: >> - cache maintanence for safety when modifying MPU memory mapping table >> - Hardcode index for all data/text sections >> - To make sure that alternative instructions are included, use >> "_einitext" >> as the start of the "Init data" section. >> --- > < snip> >> +/* >> + * Static start-of-day Xen EL2 MPU memory region layout: >> + * >> + * xen_mpumap[0] : Xen text >> + * xen_mpumap[1] : Xen read-only data >> + * xen_mpumap[2] : Xen read-only after init data >> + * xen_mpumap[3] : Xen read-write data >> + * xen_mpumap[4] : Xen BSS >> + * xen_mpumap[5] : Xen init text >> + * xen_mpumap[6] : Xen init data >> + * >> + * Clobbers x0 - x6 >> + * >> + * It shall be compliant with what describes in xen.lds.S, or the below >> + * codes need adjustment. >> + */ >> +ENTRY(prepare_early_mappings) >> + /* x0: region sel */ >> + mov x0, xzr >> + /* Xen text section. */ >> + load_paddr x1, _stext >> + load_paddr x2, _etext >> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >> attr_prbar=REGION_TEXT_PRBAR >> + >> + add x0, x0, #1 >> + /* Xen read-only data section. */ >> + load_paddr x1, _srodata >> + load_paddr x2, _erodata >> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >> attr_prbar=REGION_RO_PRBAR >> + >> + add x0, x0, #1 >> + /* Xen read-only after init data section. */ >> + load_paddr x1, __ro_after_init_start >> + load_paddr x2, __ro_after_init_end >> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >> + >> + add x0, x0, #1 >> + /* Xen read-write data section. */ >> + load_paddr x1, __ro_after_init_end >> + load_paddr x2, __init_begin >> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >> + >> + add x0, x0, #1 >> + /* Xen BSS section. */ >> + load_paddr x1, __bss_start >> + load_paddr x2, __bss_end >> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >> + >> + add x0, x0, #1 >> + /* Xen init text section. */ >> + load_paddr x1, _sinittext >> + load_paddr x2, _einittext >> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >> attr_prbar=REGION_TEXT_PRBAR >> + >> + add x0, x0, #1 >> + /* Xen init data section. */ >> + /* >> + * Even though we are not using alternative instructions in MPU yet, >> + * we want to use "_einitext" for the start of the "Init data" >> section >> + * to make sure they are included. >> + */ >> + load_paddr x1, _einittext >> + roundup_section x1 >> + load_paddr x2, __init_end >> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >> + >> + /* Ensure any MPU memory mapping table updates made above have >> occurred. */ >> + dsb nshst >> + ret >> +ENDPROC(prepare_early_mappings) > > Any reason why this is in assembly ? I am not Penny. But from my understanding, in your approach, you will require to disable to switch the disable the MPU for using the new sections. While I agree... > > We have implemented it in C > https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/mm_mpu.c#L941 , so that it can be common between R82 and R52. ... this means you can share the code. It also means: * You can't protect Xen properly from the start * You need to flush the cache (not great for performance) * You need to be more cautious as the MPU will be disabled for a short period of time. In fact looking at your switch code in setup_protection_regions(), I am not convinced you can disable the MPU in C and then call set_boot_mpumap(). I think the enable/disable would need to be moved in the assembly function. There are potentially more issues. So overall, I am not convinced of the C/common approach. Cheers,
On 28/06/2023 12:17, Julien Grall wrote: > Hi, Hi Julien, > > On 28/06/2023 11:55, Ayan Kumar Halder wrote: >> On 26/06/2023 04:34, Penny Zheng wrote: >>> CAUTION: This message has originated from an External Source. Please >>> use proper judgment and caution when opening attachments, clicking >>> links, or responding to this email. >>> >>> >>> The start-of-day Xen MPU memory region layout shall be like >>> as follows: >>> >>> xen_mpumap[0] : Xen text >>> xen_mpumap[1] : Xen read-only data >>> xen_mpumap[2] : Xen read-only after init data >>> xen_mpumap[3] : Xen read-write data >>> xen_mpumap[4] : Xen BSS >>> xen_mpumap[5] : Xen init text >>> xen_mpumap[6] : Xen init data >>> >>> The layout shall be compliant with what we describe in xen.lds.S, >>> or the codes need adjustment. >>> >>> Signed-off-by: Penny Zheng <penny.zheng@arm.com> >>> Signed-off-by: Wei Chen <wei.chen@arm.com> >>> --- >>> v3: >>> - cache maintanence for safety when modifying MPU memory mapping table >>> - Hardcode index for all data/text sections >>> - To make sure that alternative instructions are included, use >>> "_einitext" >>> as the start of the "Init data" section. >>> --- >> < snip> >>> +/* >>> + * Static start-of-day Xen EL2 MPU memory region layout: >>> + * >>> + * xen_mpumap[0] : Xen text >>> + * xen_mpumap[1] : Xen read-only data >>> + * xen_mpumap[2] : Xen read-only after init data >>> + * xen_mpumap[3] : Xen read-write data >>> + * xen_mpumap[4] : Xen BSS >>> + * xen_mpumap[5] : Xen init text >>> + * xen_mpumap[6] : Xen init data >>> + * >>> + * Clobbers x0 - x6 >>> + * >>> + * It shall be compliant with what describes in xen.lds.S, or the >>> below >>> + * codes need adjustment. >>> + */ >>> +ENTRY(prepare_early_mappings) >>> + /* x0: region sel */ >>> + mov x0, xzr >>> + /* Xen text section. */ >>> + load_paddr x1, _stext >>> + load_paddr x2, _etext >>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >>> attr_prbar=REGION_TEXT_PRBAR >>> + >>> + add x0, x0, #1 >>> + /* Xen read-only data section. */ >>> + load_paddr x1, _srodata >>> + load_paddr x2, _erodata >>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >>> attr_prbar=REGION_RO_PRBAR >>> + >>> + add x0, x0, #1 >>> + /* Xen read-only after init data section. */ >>> + load_paddr x1, __ro_after_init_start >>> + load_paddr x2, __ro_after_init_end >>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>> + >>> + add x0, x0, #1 >>> + /* Xen read-write data section. */ >>> + load_paddr x1, __ro_after_init_end >>> + load_paddr x2, __init_begin >>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>> + >>> + add x0, x0, #1 >>> + /* Xen BSS section. */ >>> + load_paddr x1, __bss_start >>> + load_paddr x2, __bss_end >>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>> + >>> + add x0, x0, #1 >>> + /* Xen init text section. */ >>> + load_paddr x1, _sinittext >>> + load_paddr x2, _einittext >>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >>> attr_prbar=REGION_TEXT_PRBAR >>> + >>> + add x0, x0, #1 >>> + /* Xen init data section. */ >>> + /* >>> + * Even though we are not using alternative instructions in MPU >>> yet, >>> + * we want to use "_einitext" for the start of the "Init data" >>> section >>> + * to make sure they are included. >>> + */ >>> + load_paddr x1, _einittext >>> + roundup_section x1 >>> + load_paddr x2, __init_end >>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>> + >>> + /* Ensure any MPU memory mapping table updates made above have >>> occurred. */ >>> + dsb nshst >>> + ret >>> +ENDPROC(prepare_early_mappings) >> >> Any reason why this is in assembly ? > > I am not Penny. But from my understanding, in your approach, you will > require to disable to switch the disable the MPU for using the new > sections. While I agree... > >> >> We have implemented it in C >> https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/mm_mpu.c#L941 >> , so that it can be common between R82 and R52. > > ... this means you can share the code. It also means: > * You can't protect Xen properly from the start Yes, I see what you mean. Refer https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/arm32/mpu_v8r.S#L82C7-L82C15 , I am mapping CONFIG_XEN_START_ADDRESS + 2 MB. But, probably you mean individual sections should be properly mapped from the beginning. > * You need to flush the cache (not great for performance) > * You need to be more cautious as the MPU will be disabled for a > short period of time. Yes, MPU is disabled when set_boot_mpumap() is invoked. But is this really a security issue ? I mean only a single core is running and it is executing Xen. The MPU is turned off for few cycles. > > In fact looking at your switch code in setup_protection_regions(), I > am not convinced you can disable the MPU in C and then call > set_boot_mpumap(). I think the enable/disable would need to be moved > in the assembly function. There are potentially more issues. disable_mpu(), enable_mpu() are written in assembly. See https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/arm32/mpu_v8r.S#L128 - Ayan > > So overall, I am not convinced of the C/common approach. > > Cheers, >
Hi Ayan, On 28/06/2023 14:22, Ayan Kumar Halder wrote: > > On 28/06/2023 12:17, Julien Grall wrote: >> Hi, > Hi Julien, >> >> On 28/06/2023 11:55, Ayan Kumar Halder wrote: >>> On 26/06/2023 04:34, Penny Zheng wrote: >>>> CAUTION: This message has originated from an External Source. Please >>>> use proper judgment and caution when opening attachments, clicking >>>> links, or responding to this email. >>>> >>>> >>>> The start-of-day Xen MPU memory region layout shall be like >>>> as follows: >>>> >>>> xen_mpumap[0] : Xen text >>>> xen_mpumap[1] : Xen read-only data >>>> xen_mpumap[2] : Xen read-only after init data >>>> xen_mpumap[3] : Xen read-write data >>>> xen_mpumap[4] : Xen BSS >>>> xen_mpumap[5] : Xen init text >>>> xen_mpumap[6] : Xen init data >>>> >>>> The layout shall be compliant with what we describe in xen.lds.S, >>>> or the codes need adjustment. >>>> >>>> Signed-off-by: Penny Zheng <penny.zheng@arm.com> >>>> Signed-off-by: Wei Chen <wei.chen@arm.com> >>>> --- >>>> v3: >>>> - cache maintanence for safety when modifying MPU memory mapping table >>>> - Hardcode index for all data/text sections >>>> - To make sure that alternative instructions are included, use >>>> "_einitext" >>>> as the start of the "Init data" section. >>>> --- >>> < snip> >>>> +/* >>>> + * Static start-of-day Xen EL2 MPU memory region layout: >>>> + * >>>> + * xen_mpumap[0] : Xen text >>>> + * xen_mpumap[1] : Xen read-only data >>>> + * xen_mpumap[2] : Xen read-only after init data >>>> + * xen_mpumap[3] : Xen read-write data >>>> + * xen_mpumap[4] : Xen BSS >>>> + * xen_mpumap[5] : Xen init text >>>> + * xen_mpumap[6] : Xen init data >>>> + * >>>> + * Clobbers x0 - x6 >>>> + * >>>> + * It shall be compliant with what describes in xen.lds.S, or the >>>> below >>>> + * codes need adjustment. >>>> + */ >>>> +ENTRY(prepare_early_mappings) >>>> + /* x0: region sel */ >>>> + mov x0, xzr >>>> + /* Xen text section. */ >>>> + load_paddr x1, _stext >>>> + load_paddr x2, _etext >>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >>>> attr_prbar=REGION_TEXT_PRBAR >>>> + >>>> + add x0, x0, #1 >>>> + /* Xen read-only data section. */ >>>> + load_paddr x1, _srodata >>>> + load_paddr x2, _erodata >>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >>>> attr_prbar=REGION_RO_PRBAR >>>> + >>>> + add x0, x0, #1 >>>> + /* Xen read-only after init data section. */ >>>> + load_paddr x1, __ro_after_init_start >>>> + load_paddr x2, __ro_after_init_end >>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>>> + >>>> + add x0, x0, #1 >>>> + /* Xen read-write data section. */ >>>> + load_paddr x1, __ro_after_init_end >>>> + load_paddr x2, __init_begin >>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>>> + >>>> + add x0, x0, #1 >>>> + /* Xen BSS section. */ >>>> + load_paddr x1, __bss_start >>>> + load_paddr x2, __bss_end >>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>>> + >>>> + add x0, x0, #1 >>>> + /* Xen init text section. */ >>>> + load_paddr x1, _sinittext >>>> + load_paddr x2, _einittext >>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >>>> attr_prbar=REGION_TEXT_PRBAR >>>> + >>>> + add x0, x0, #1 >>>> + /* Xen init data section. */ >>>> + /* >>>> + * Even though we are not using alternative instructions in MPU >>>> yet, >>>> + * we want to use "_einitext" for the start of the "Init data" >>>> section >>>> + * to make sure they are included. >>>> + */ >>>> + load_paddr x1, _einittext >>>> + roundup_section x1 >>>> + load_paddr x2, __init_end >>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>>> + >>>> + /* Ensure any MPU memory mapping table updates made above have >>>> occurred. */ >>>> + dsb nshst >>>> + ret >>>> +ENDPROC(prepare_early_mappings) >>> >>> Any reason why this is in assembly ? >> >> I am not Penny. But from my understanding, in your approach, you will >> require to disable to switch the disable the MPU for using the new >> sections. While I agree... >> >>> >>> We have implemented it in C >>> https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/mm_mpu.c#L941 , so that it can be common between R82 and R52. >> >> ... this means you can share the code. It also means: >> * You can't protect Xen properly from the start > > Yes, I see what you mean. Refer > https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/arm32/mpu_v8r.S#L82C7-L82C15 , > > I am mapping CONFIG_XEN_START_ADDRESS + 2 MB. But, probably you mean > individual sections should be properly mapped from the beginning. > >> * You need to flush the cache (not great for performance) >> * You need to be more cautious as the MPU will be disabled for a >> short period of time. > > Yes, MPU is disabled when set_boot_mpumap() is invoked. But is this > really a security issue ? My second point is not about security, it is about been compliant with the Arm Arm. To quote what you wrote: " To avoid unexpected unaligment access fault during MPU disabled, set_boot_mpumap shall be written in assembly code." What's the guarantee that the compiler will not generate any instructions that could generate an alignment fault? Furthermore, from my understanding, at least on Armv8-A, there are caching problem because you will need to save some registers (for the call to set_boot_mpumap()) on the stack with cache disabled. This means the cache will be bypassed. But you may then restore the registers with the cache enabled (the compiler could decide that it is not necessary to read the stack before hand). So you could read the wrong data if there is a stale cacheline. So overall, I think you want to tightly control the section where the MPU is off. This means writing the logic in assembly rather than C. > > I mean only a single core is running and it is executing Xen. The MPU is > turned off for few cycles. > >> >> In fact looking at your switch code in setup_protection_regions(), I >> am not convinced you can disable the MPU in C and then call >> set_boot_mpumap(). I think the enable/disable would need to be moved >> in the assembly function. There are potentially more issues. > > disable_mpu(), enable_mpu() are written in assembly. See > https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/arm32/mpu_v8r.S#L128 Right, but then you return to C world to then call set_boot_mpumap(). So you have still some part in C even if it is small. Cheers,
On 28/06/2023 14:42, Julien Grall wrote: > Hi Ayan, Hi Julien, > > On 28/06/2023 14:22, Ayan Kumar Halder wrote: >> >> On 28/06/2023 12:17, Julien Grall wrote: >>> Hi, >> Hi Julien, >>> >>> On 28/06/2023 11:55, Ayan Kumar Halder wrote: >>>> On 26/06/2023 04:34, Penny Zheng wrote: >>>>> CAUTION: This message has originated from an External Source. >>>>> Please use proper judgment and caution when opening attachments, >>>>> clicking links, or responding to this email. >>>>> >>>>> >>>>> The start-of-day Xen MPU memory region layout shall be like >>>>> as follows: >>>>> >>>>> xen_mpumap[0] : Xen text >>>>> xen_mpumap[1] : Xen read-only data >>>>> xen_mpumap[2] : Xen read-only after init data >>>>> xen_mpumap[3] : Xen read-write data >>>>> xen_mpumap[4] : Xen BSS >>>>> xen_mpumap[5] : Xen init text >>>>> xen_mpumap[6] : Xen init data >>>>> >>>>> The layout shall be compliant with what we describe in xen.lds.S, >>>>> or the codes need adjustment. >>>>> >>>>> Signed-off-by: Penny Zheng <penny.zheng@arm.com> >>>>> Signed-off-by: Wei Chen <wei.chen@arm.com> >>>>> --- >>>>> v3: >>>>> - cache maintanence for safety when modifying MPU memory mapping >>>>> table >>>>> - Hardcode index for all data/text sections >>>>> - To make sure that alternative instructions are included, use >>>>> "_einitext" >>>>> as the start of the "Init data" section. >>>>> --- >>>> < snip> >>>>> +/* >>>>> + * Static start-of-day Xen EL2 MPU memory region layout: >>>>> + * >>>>> + * xen_mpumap[0] : Xen text >>>>> + * xen_mpumap[1] : Xen read-only data >>>>> + * xen_mpumap[2] : Xen read-only after init data >>>>> + * xen_mpumap[3] : Xen read-write data >>>>> + * xen_mpumap[4] : Xen BSS >>>>> + * xen_mpumap[5] : Xen init text >>>>> + * xen_mpumap[6] : Xen init data >>>>> + * >>>>> + * Clobbers x0 - x6 >>>>> + * >>>>> + * It shall be compliant with what describes in xen.lds.S, or the >>>>> below >>>>> + * codes need adjustment. >>>>> + */ >>>>> +ENTRY(prepare_early_mappings) >>>>> + /* x0: region sel */ >>>>> + mov x0, xzr >>>>> + /* Xen text section. */ >>>>> + load_paddr x1, _stext >>>>> + load_paddr x2, _etext >>>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >>>>> attr_prbar=REGION_TEXT_PRBAR >>>>> + >>>>> + add x0, x0, #1 >>>>> + /* Xen read-only data section. */ >>>>> + load_paddr x1, _srodata >>>>> + load_paddr x2, _erodata >>>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >>>>> attr_prbar=REGION_RO_PRBAR >>>>> + >>>>> + add x0, x0, #1 >>>>> + /* Xen read-only after init data section. */ >>>>> + load_paddr x1, __ro_after_init_start >>>>> + load_paddr x2, __ro_after_init_end >>>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>>>> + >>>>> + add x0, x0, #1 >>>>> + /* Xen read-write data section. */ >>>>> + load_paddr x1, __ro_after_init_end >>>>> + load_paddr x2, __init_begin >>>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>>>> + >>>>> + add x0, x0, #1 >>>>> + /* Xen BSS section. */ >>>>> + load_paddr x1, __bss_start >>>>> + load_paddr x2, __bss_end >>>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>>>> + >>>>> + add x0, x0, #1 >>>>> + /* Xen init text section. */ >>>>> + load_paddr x1, _sinittext >>>>> + load_paddr x2, _einittext >>>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, >>>>> attr_prbar=REGION_TEXT_PRBAR >>>>> + >>>>> + add x0, x0, #1 >>>>> + /* Xen init data section. */ >>>>> + /* >>>>> + * Even though we are not using alternative instructions in >>>>> MPU yet, >>>>> + * we want to use "_einitext" for the start of the "Init >>>>> data" section >>>>> + * to make sure they are included. >>>>> + */ >>>>> + load_paddr x1, _einittext >>>>> + roundup_section x1 >>>>> + load_paddr x2, __init_end >>>>> + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 >>>>> + >>>>> + /* Ensure any MPU memory mapping table updates made above >>>>> have occurred. */ >>>>> + dsb nshst >>>>> + ret >>>>> +ENDPROC(prepare_early_mappings) >>>> >>>> Any reason why this is in assembly ? >>> >>> I am not Penny. But from my understanding, in your approach, you >>> will require to disable to switch the disable the MPU for using the >>> new sections. While I agree... >>> >>>> >>>> We have implemented it in C >>>> https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/mm_mpu.c#L941 >>>> , so that it can be common between R82 and R52. >>> >>> ... this means you can share the code. It also means: >>> * You can't protect Xen properly from the start >> >> Yes, I see what you mean. Refer >> https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/arm32/mpu_v8r.S#L82C7-L82C15 >> , >> >> I am mapping CONFIG_XEN_START_ADDRESS + 2 MB. But, probably you mean >> individual sections should be properly mapped from the beginning. >> >>> * You need to flush the cache (not great for performance) >>> * You need to be more cautious as the MPU will be disabled for a >>> short period of time. >> >> Yes, MPU is disabled when set_boot_mpumap() is invoked. But is this >> really a security issue ? > > My second point is not about security, it is about been compliant with > the Arm Arm. To quote what you wrote: > > " To avoid unexpected unaligment access fault during MPU disabled, > set_boot_mpumap shall be written in assembly code." > > What's the guarantee that the compiler will not generate any > instructions that could generate an alignment fault? I thought by writing in assembly, we tell the compiler what instructions to generate. For eg ENTRY(set_boot_mpumap) push {r4} mov r2, #0 /* table index */ 1: ldr r3, [r1], #4 /* r3: prbar */ ldr r4, [r1], #12 /* r4: prlar */ write_pr r2, r3, r4 add r2, r2, #1 /* table index ++ */ cmp r2, r0 blt 1b pop {r4} ret ENDPROC(set_boot_mpumap) I ask the compiler to use ldr (and not ldrb) instructions. May be I am missing something very obvious here. However, I might be misunderstanding the comment here. It was originally written by Penny. @Penny, Can you explain "To avoid unexpected unaligment access fault during MPU disabled, set_boot_mpumap shall be written in assembly code." in a bit more detail, please ? > > Furthermore, from my understanding, at least on Armv8-A, there are > caching problem because you will need to save some registers (for the > call to set_boot_mpumap()) on the stack with cache disabled. This > means the cache will be bypassed. But you may then restore the > registers with the cache enabled (the compiler could decide that it is > not necessary to read the stack before hand). So you could read the > wrong data if there is a stale cacheline. Yes, this makes some sense. So will the following make it correct :- 1. Execute 'dmb' before invoking enable_mpu(). This will ensure that the registers are strictly restored in set_boot_mpumap() before the HSCTLR is read. We do have 'dsb sy' before modifying HSCTLR (ie enabling cache), but may be we want to be stricter. 2. Invalidate the D cache after "mcr CP32(r0, HSCTLR)" and then dsb (to ensure d cache is invalidated), isb (flush the instruction cache as MPU is enabled), ret. - Ayan > > So overall, I think you want to tightly control the section where the > MPU is off. This means writing the logic in assembly rather than C. > >> >> I mean only a single core is running and it is executing Xen. The MPU >> is turned off for few cycles. >> >>> >>> In fact looking at your switch code in setup_protection_regions(), I >>> am not convinced you can disable the MPU in C and then call >>> set_boot_mpumap(). I think the enable/disable would need to be moved >>> in the assembly function. There are potentially more issues. >> >> disable_mpu(), enable_mpu() are written in assembly. See >> https://github.com/Xilinx/xen/blob/d1258f1cefe406a3f91237b8106746c089864651/xen/arch/arm_mpu/arm32/mpu_v8r.S#L128 > > Right, but then you return to C world to then call set_boot_mpumap(). > So you have still some part in C even if it is small. > > Cheers, >
Hi Ayan, On 29/06/2023 12:21, Ayan Kumar Halder wrote: > > On 28/06/2023 14:42, Julien Grall wrote: >> What's the guarantee that the compiler will not generate any >> instructions that could generate an alignment fault? > > I thought by writing in assembly, we tell the compiler what instructions > to generate. For eg > > ENTRY(set_boot_mpumap) > push {r4} > mov r2, #0 /* table index */ > 1: ldr r3, [r1], #4 /* r3: prbar */ > ldr r4, [r1], #12 /* r4: prlar */ > write_pr r2, r3, r4 > add r2, r2, #1 /* table index ++ */ > cmp r2, r0 > blt 1b > pop {r4} > ret > ENDPROC(set_boot_mpumap) > > I ask the compiler to use ldr (and not ldrb) instructions. > > May be I am missing something very obvious here. The problem is not the assembly code. The problem is the C code. You wrote: /* * Since it is the MPU protection region which holds the XEN kernel that * needs updating. * The whole MPU system must be disabled for the update. */ disable_mpu(); /* * Set new MPU memory region configuration. * To avoid the mismatch between nr_xen_mpumap and nr_xen_mpumap * after the relocation of some MPU regions later, here * next_xen_mpumap_index is used. * To avoid unexpected unaligment access fault during MPU disabled, * set_boot_mpumap shall be written in assembly code. */ set_boot_mpumap(next_xen_mpumap_index, (pr_t *)boot_mpumap); enable_mpu(); You can't guarantee what assembly instructions the compiler will use for any of this code. So if you are concerned about unaligned access when the MPU is disabled, then you should never return to C (even temporarily) while the MPU is off. >> >> Furthermore, from my understanding, at least on Armv8-A, there are >> caching problem because you will need to save some registers (for the >> call to set_boot_mpumap()) on the stack with cache disabled. This >> means the cache will be bypassed. But you may then restore the >> registers with the cache enabled (the compiler could decide that it is >> not necessary to read the stack before hand). So you could read the >> wrong data if there is a stale cacheline. > > Yes, this makes some sense. So will the following make it correct :- I am confused. In a previous answer, I voiced my concerned with trying to replace the full MPU table. So it is not clear to me why you are asking me if the following work. Do you still want to do it? If so, why? > > 1. Execute 'dmb' before invoking enable_mpu(). This will ensure that the > registers are strictly restored in set_boot_mpumap() before the HSCTLR > is read. I am afraid I don't know how the DMB will enforce that. Can you clarify? > > We do have 'dsb sy' before modifying HSCTLR (ie enabling cache), but may > be we want to be stricter. > > 2. Invalidate the D cache after "mcr CP32(r0, HSCTLR)" and then dsb > (to ensure d cache is invalidated), isb (flush the instruction cache as > MPU is enabled), ret. I might be missing something here. The ISB instruction will not flush the instruction cache, it will flush the pipeline instead and guarantee that previous instructions will complete before continuing. But overall, the easiest solution is to disable the MPU, update the MPU tables, and then re-enable the MPU all in assembly (i.e. no jump back to C even temporarily). So you control the accesses and can limit (if not remove) any write to the memory whilst the cache is disabled. Cheers,
On 29/06/2023 12:55, Julien Grall wrote: > Hi Ayan, Hi Julien, > > On 29/06/2023 12:21, Ayan Kumar Halder wrote: >> >> On 28/06/2023 14:42, Julien Grall wrote: >>> What's the guarantee that the compiler will not generate any >>> instructions that could generate an alignment fault? >> >> I thought by writing in assembly, we tell the compiler what >> instructions to generate. For eg >> >> ENTRY(set_boot_mpumap) >> push {r4} >> mov r2, #0 /* table index */ >> 1: ldr r3, [r1], #4 /* r3: prbar */ >> ldr r4, [r1], #12 /* r4: prlar */ >> write_pr r2, r3, r4 >> add r2, r2, #1 /* table index ++ */ >> cmp r2, r0 >> blt 1b >> pop {r4} >> ret >> ENDPROC(set_boot_mpumap) >> >> I ask the compiler to use ldr (and not ldrb) instructions. >> >> May be I am missing something very obvious here. > > The problem is not the assembly code. The problem is the C code. You > wrote: > > /* > * Since it is the MPU protection region which holds the XEN > kernel that > * needs updating. > * The whole MPU system must be disabled for the update. > */ > disable_mpu(); > > /* > * Set new MPU memory region configuration. > * To avoid the mismatch between nr_xen_mpumap and nr_xen_mpumap > * after the relocation of some MPU regions later, here > * next_xen_mpumap_index is used. > * To avoid unexpected unaligment access fault during MPU disabled, > * set_boot_mpumap shall be written in assembly code. > */ > set_boot_mpumap(next_xen_mpumap_index, (pr_t *)boot_mpumap); > > enable_mpu(); > > You can't guarantee what assembly instructions the compiler will use > for any of this code. So if you are concerned about unaligned access > when the MPU is disabled, then you should never return to C (even > temporarily) while the MPU is off. yes, agreed. > >>> >>> Furthermore, from my understanding, at least on Armv8-A, there are >>> caching problem because you will need to save some registers (for >>> the call to set_boot_mpumap()) on the stack with cache disabled. >>> This means the cache will be bypassed. But you may then restore the >>> registers with the cache enabled (the compiler could decide that it >>> is not necessary to read the stack before hand). So you could read >>> the wrong data if there is a stale cacheline. >> >> Yes, this makes some sense. So will the following make it correct :- > > I am confused. In a previous answer, I voiced my concerned with trying > to replace the full MPU table. So it is not clear to me why you are > asking me if the following work. Do you still want to do it? If so, why? I completely agree with you to set up the MPU table in assembly with the correct permissions for each section (as done by Penny's patch). That would atleast ensure that we don't need to reset the MPU sections for Xen again. What I was trying to understand deeper was some of the objections you had raised and if any sort of mitigations are possible. Again I am not saying that we need to apply the mitigations (if available) in this particular scenario. > >> >> 1. Execute 'dmb' before invoking enable_mpu(). This will ensure that >> the registers are strictly restored in set_boot_mpumap() before the >> HSCTLR is read. > > I am afraid I don't know how the DMB will enforce that. Can you clarify? pop {r4} /* As part of set_boot_mpumap() */ dmb /* This should ensure that r4 is fully restored from the stack before the next instruction. At this point, the D cache is still disabled. */ > >> >> We do have 'dsb sy' before modifying HSCTLR (ie enabling cache), but >> may be we want to be stricter. >> >> 2. Invalidate the D cache after "mcr CP32(r0, HSCTLR)" and then dsb >> (to ensure d cache is invalidated), isb (flush the instruction cache >> as MPU is enabled), ret. > > I might be missing something here. The ISB instruction will not flush > the instruction cache, it will flush the pipeline instead and > guarantee that previous instructions will complete before continuing. Sorry my bad, I meant flushing the pipeline so that pipeline is refilled with the instructions from the MPU regions with the correct permissions. > > But overall, the easiest solution is to disable the MPU, update the > MPU tables, and then re-enable the MPU all in assembly (i.e. no jump > back to C even temporarily). > > So you control the accesses and can limit (if not remove) any write to > the memory whilst the cache is disabled. Yes, agreed. I will rework my patch so that the MPU regions are set up with the sections of Xen in assembly (similar to what is being done in Penny's patch). - Ayan > > Cheers, >
On 30/06/2023 10:26, Ayan Kumar Halder wrote: > On 29/06/2023 12:55, Julien Grall wrote: >>>> Furthermore, from my understanding, at least on Armv8-A, there are >>>> caching problem because you will need to save some registers (for >>>> the call to set_boot_mpumap()) on the stack with cache disabled. >>>> This means the cache will be bypassed. But you may then restore the >>>> registers with the cache enabled (the compiler could decide that it >>>> is not necessary to read the stack before hand). So you could read >>>> the wrong data if there is a stale cacheline. >>> >>> Yes, this makes some sense. So will the following make it correct :- >> >> I am confused. In a previous answer, I voiced my concerned with trying >> to replace the full MPU table. So it is not clear to me why you are >> asking me if the following work. Do you still want to do it? If so, why? > > I completely agree with you to set up the MPU table in assembly with the > correct permissions for each section (as done by Penny's patch). That > would atleast ensure that we don't need to reset the MPU sections for > Xen again. > > What I was trying to understand deeper was some of the objections you > had raised and if any sort of mitigations are possible. > > Again I am not saying that we need to apply the mitigations (if > available) in this particular scenario. > >> >>> >>> 1. Execute 'dmb' before invoking enable_mpu(). This will ensure that >>> the registers are strictly restored in set_boot_mpumap() before the >>> HSCTLR is read. >> >> I am afraid I don't know how the DMB will enforce that. Can you clarify? > > pop {r4} /* As part of set_boot_mpumap() */ > > dmb /* This should ensure that r4 is fully restored from the stack > before the next instruction. At this point, the D cache is still > disabled. */ I don't really see how this helps because the C instruction: set_boot_mpumap(....) could also require to read/write the stack for saving r0-r3. And AFAIK, you can't control when this would happen. Cheers,
On 30/06/2023 10:43 am, Julien Grall wrote: > On 30/06/2023 10:26, Ayan Kumar Halder wrote: >> On 29/06/2023 12:55, Julien Grall wrote: >>>>> Furthermore, from my understanding, at least on Armv8-A, there are >>>>> caching problem because you will need to save some registers (for >>>>> the call to set_boot_mpumap()) on the stack with cache disabled. >>>>> This means the cache will be bypassed. But you may then restore >>>>> the registers with the cache enabled (the compiler could decide >>>>> that it is not necessary to read the stack before hand). So you >>>>> could read the wrong data if there is a stale cacheline. >>>> >>>> Yes, this makes some sense. So will the following make it correct :- >>> >>> I am confused. In a previous answer, I voiced my concerned with >>> trying to replace the full MPU table. So it is not clear to me why >>> you are asking me if the following work. Do you still want to do it? >>> If so, why? >> >> I completely agree with you to set up the MPU table in assembly with >> the correct permissions for each section (as done by Penny's patch). >> That would atleast ensure that we don't need to reset the MPU >> sections for Xen again. >> >> What I was trying to understand deeper was some of the objections you >> had raised and if any sort of mitigations are possible. >> >> Again I am not saying that we need to apply the mitigations (if >> available) in this particular scenario. >> >>> >>>> >>>> 1. Execute 'dmb' before invoking enable_mpu(). This will ensure >>>> that the registers are strictly restored in set_boot_mpumap() >>>> before the HSCTLR is read. >>> >>> I am afraid I don't know how the DMB will enforce that. Can you >>> clarify? >> >> pop {r4} /* As part of set_boot_mpumap() */ >> >> dmb /* This should ensure that r4 is fully restored from the stack >> before the next instruction. At this point, the D cache is still >> disabled. */ > > I don't really see how this helps because the C instruction: > > set_boot_mpumap(....) > > could also require to read/write the stack for saving r0-r3. And > AFAIK, you can't control when this would happen. The argument is far easier than that. At all point, anywhere in code, the C compiler can emit calls to memcpy/memset behind your back, including (in principle) for code which looks like `int foo;` (yes, because of things like -ftrivial-auto-var-init). With things like UBSAN active, it's a much wider range of functions that can be called. If anything is potentially unsafe to the C operating environment, it *must* be handled in assembly. ~Andrew
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index a83a535cd7..3bd193ee32 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -40,6 +40,8 @@ ifeq ($(CONFIG_HAS_MMU), y) obj-y += mmu/mm.o obj-y += mmu/setup.o obj-y += mmu/p2m.o +else +obj-y += mpu/mm.o endif obj-y += mm.o obj-y += monitor.o diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile index 55895ecb53..2641fb13ba 100644 --- a/xen/arch/arm/arm64/Makefile +++ b/xen/arch/arm/arm64/Makefile @@ -11,6 +11,8 @@ obj-y += head.o ifeq ($(CONFIG_HAS_MMU),y) obj-y += mmu/head.o obj-y += mmu/mm.o +else +obj-y += mpu/head.o endif obj-y += insn.o obj-$(CONFIG_LIVEPATCH) += livepatch.o diff --git a/xen/arch/arm/arm64/mpu/head.S b/xen/arch/arm/arm64/mpu/head.S new file mode 100644 index 0000000000..93a7a75029 --- /dev/null +++ b/xen/arch/arm/arm64/mpu/head.S @@ -0,0 +1,178 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Start-of-day code for an Armv8-R AArch64 MPU system. + * + * Copyright (C) 2023 Arm Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <asm/arm64/mpu.h> +#include <asm/page.h> + +/* + * One entry in Xen MPU memory region mapping table(xen_mpumap) is a structure + * of pr_t, which is 16-bytes size, so the entry offset is the order of 4. + */ +#define MPU_ENTRY_SHIFT 0x4 + +#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_NORMAL_PRLAR 0x0f /* NS=0 ATTR=111 EN=1 */ + +/* + * Macro to round up the section address to be PAGE_SIZE aligned + * Each section(e.g. .text, .data, etc) in xen.lds.S is page-aligned, + * which is usually guarded with ". = ALIGN(PAGE_SIZE)" in the head, + * or in the end + */ +.macro roundup_section, xb + add \xb, \xb, #(PAGE_SIZE-1) + and \xb, \xb, #PAGE_MASK +.endm + +/* + * Macro to prepare and configure a particular EL2 MPU memory region with + * base address as \base and limit address as \limit. + * We will also create an according MPU memory region entry, which + * is a structure of pr_t, in Xen EL2 mpu memory region mapping table + * xen_mpumap. + * + * Inputs: + * base: reg storing base address (should be page-aligned) + * limit: reg storing limit address + * sel: region selector + * prbar: store computed PRBAR_EL2 value + * prlar: store computed PRLAR_EL2 value + * attr_prbar: PRBAR_EL2-related memory attributes. If not specified it will be REGION_DATA_PRBAR + * attr_prlar: PRLAR_EL2-related memory attributes. If not specified it will be REGION_NORMAL_PRLAR + * + * Clobber \tmp1, \tmp2 + * + */ +.macro prepare_xen_region, sel, base, limit, prbar, prlar, tmp1, tmp2, attr_prbar=REGION_DATA_PRBAR, attr_prlar=REGION_NORMAL_PRLAR + /* Prepare value for PRBAR_EL2 reg and preserve it in \prbar.*/ + and \base, \base, #MPU_REGION_MASK + mov \prbar, #\attr_prbar + orr \prbar, \prbar, \base + + /* Prepare value for PRLAR_EL2 reg and preserve it in \prlar.*/ + /* Round up limit address to be PAGE_SIZE aligned */ + roundup_section \limit + /* Limit address should be inclusive */ + sub \limit, \limit, #1 + and \limit, \limit, #MPU_REGION_MASK + mov \prlar, #\attr_prlar + orr \prlar, \prlar, \limit + + /* + * Before accessing EL2 MPU region register PRBAR_EL2/PRLAR_EL2, + * PRSELR_EL2.REGION determines which MPU region is selected. + */ + msr PRSELR_EL2, \sel + isb + msr PRBAR_EL2, \prbar + msr PRLAR_EL2, \prlar + isb + + mov \tmp1, \sel + lsl \tmp1, \tmp1, #MPU_ENTRY_SHIFT + load_paddr \tmp2, xen_mpumap + add \tmp2, \tmp2, \tmp1 + stp \prbar, \prlar, [\tmp2] + /* Invalidate data cache for safety */ + dc ivac, \tmp2 + isb +.endm + +.section .text.idmap, "ax", %progbits + +/* + * Static start-of-day Xen EL2 MPU memory region layout: + * + * xen_mpumap[0] : Xen text + * xen_mpumap[1] : Xen read-only data + * xen_mpumap[2] : Xen read-only after init data + * xen_mpumap[3] : Xen read-write data + * xen_mpumap[4] : Xen BSS + * xen_mpumap[5] : Xen init text + * xen_mpumap[6] : Xen init data + * + * Clobbers x0 - x6 + * + * It shall be compliant with what describes in xen.lds.S, or the below + * codes need adjustment. + */ +ENTRY(prepare_early_mappings) + /* x0: region sel */ + mov x0, xzr + /* Xen text section. */ + load_paddr x1, _stext + load_paddr x2, _etext + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, attr_prbar=REGION_TEXT_PRBAR + + add x0, x0, #1 + /* Xen read-only data section. */ + load_paddr x1, _srodata + load_paddr x2, _erodata + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, attr_prbar=REGION_RO_PRBAR + + add x0, x0, #1 + /* Xen read-only after init data section. */ + load_paddr x1, __ro_after_init_start + load_paddr x2, __ro_after_init_end + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 + + add x0, x0, #1 + /* Xen read-write data section. */ + load_paddr x1, __ro_after_init_end + load_paddr x2, __init_begin + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 + + add x0, x0, #1 + /* Xen BSS section. */ + load_paddr x1, __bss_start + load_paddr x2, __bss_end + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 + + add x0, x0, #1 + /* Xen init text section. */ + load_paddr x1, _sinittext + load_paddr x2, _einittext + prepare_xen_region x0, x1, x2, x3, x4, x5, x6, attr_prbar=REGION_TEXT_PRBAR + + add x0, x0, #1 + /* Xen init data section. */ + /* + * Even though we are not using alternative instructions in MPU yet, + * we want to use "_einitext" for the start of the "Init data" section + * to make sure they are included. + */ + load_paddr x1, _einittext + roundup_section x1 + load_paddr x2, __init_end + prepare_xen_region x0, x1, x2, x3, x4, x5, x6 + + /* Ensure any MPU memory mapping table updates made above have occurred. */ + dsb nshst + ret +ENDPROC(prepare_early_mappings) + +/* + * Local variables: + * mode: ASM + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h new file mode 100644 index 0000000000..0c479086f4 --- /dev/null +++ b/xen/arch/arm/include/asm/arm64/mpu.h @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * mpu.h: Arm Memory Protection Region definitions. + */ + +#ifndef __ARM64_MPU_H__ +#define __ARM64_MPU_H__ + +/* + * MPUIR_EL2.Region identifies the number of regions supported by the EL2 MPU. + * It is a 8-bit field, so 255 MPU memory regions at most. + */ +#define ARM_MAX_MPU_MEMORY_REGIONS 255 + +#ifndef __ASSEMBLY__ + +/* Protection Region Base Address Register */ +typedef union { + struct __packed { + unsigned long xn:2; /* Execute-Never */ + unsigned long ap:2; /* Acess Permission */ + unsigned long sh:2; /* Sharebility */ + unsigned long base:42; /* Base Address */ + unsigned long pad:16; + } reg; + uint64_t bits; +} prbar_t; + +/* Protection Region Limit Address Register */ +typedef union { + struct __packed { + unsigned long en:1; /* Region enable */ + unsigned long ai:3; /* Memory Attribute Index */ + unsigned long ns:1; /* Not-Secure */ + unsigned long res:1; /* Reserved 0 by hardware */ + unsigned long limit:42; /* Limit Address */ + unsigned long pad:16; + } reg; + uint64_t bits; +} prlar_t; + +/* MPU Protection Region */ +typedef struct { + prbar_t prbar; + prlar_t prlar; +} pr_t; + +#endif /* __ASSEMBLY__ */ + +#endif /* __ARM64_MPU_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h index 3fdeb9d8cd..c41d805fde 100644 --- a/xen/arch/arm/include/asm/arm64/sysregs.h +++ b/xen/arch/arm/include/asm/arm64/sysregs.h @@ -462,6 +462,20 @@ #define ZCR_ELx_LEN_SIZE 9 #define ZCR_ELx_LEN_MASK 0x1ff +/* System registers for Armv8-R AArch64 */ +#ifdef CONFIG_HAS_MPU + +/* EL2 MPU Protection Region Base Address Register encode */ +#define PRBAR_EL2 S3_4_C6_C8_0 + +/* EL2 MPU Protection Region Limit Address Register encode */ +#define PRLAR_EL2 S3_4_C6_C8_1 + +/* MPU Protection Region Selection Register encode */ +#define PRSELR_EL2 S3_4_C6_C2_1 + +#endif + /* Access to system registers */ #define WRITE_SYSREG64(v, name) do { \ diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c new file mode 100644 index 0000000000..fb6bb721b1 --- /dev/null +++ b/xen/arch/arm/mpu/mm.c @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * xen/arch/arm/mpu/mm.c + * + * MPU-based memory managment code for Armv8-R AArch64. + * + * Copyright (C) 2023 Arm Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <xen/init.h> +#include <xen/page-size.h> +#include <asm/arm64/mpu.h> + +/* EL2 Xen MPU memory region mapping table. */ +pr_t __aligned(PAGE_SIZE) __section(".data.page_aligned") + xen_mpumap[ARM_MAX_MPU_MEMORY_REGIONS]; + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */