|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 5/6] xen/arm: mpu: Enable MPU
Hi Ayan, > On 24 Oct 2024, at 09:02, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote: > > Hi Julien, > > On 23/10/2024 17:30, Julien Grall wrote: >> >> >> On 23/10/2024 17:18, Julien Grall wrote: >>> >>> >>> On 23/10/2024 17:13, Julien Grall wrote: >>>> >>>> >>>> On 23/10/2024 17:06, Ayan Kumar Halder wrote: >>>>> Hi Luca/Julien, >>>>> >>>>> On 22/10/2024 17:31, Luca Fancellu wrote: >>>>>> Hi Julien, >>>>>> >>>>>>> On 22 Oct 2024, at 14:13, Julien Grall <julien@xxxxxxx> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 22/10/2024 10:56, Luca Fancellu wrote: >>>>>>>>> On 22 Oct 2024, at 10:47, Julien Grall <julien@xxxxxxx> wrote: >>>>>>>>> >>>>>>>>> Hi Luca, >>>>>>>>> >>>>>>>>> On 22/10/2024 10:41, Luca Fancellu wrote: >>>>>>>>>>> On 21 Oct 2024, at 17:27, Julien Grall <julien@xxxxxxx> wrote: >>>>>>>>>> 2) dsb+isb barrier after enabling the MPU, since we are enabling the >>>>>>>>>> MPU but also because we are disabling the background region >>>>>>>>> TBH, I don't understand this one. Why would disabling the background >>>>>>>>> region requires a dsb + isb? Do you have any pointer in the Armv8-R >>>>>>>>> specification? >>>>>>>> I’m afraid this is only my deduction, Section C1.4 of the Arm® >>>>>>>> Architecture Reference Manual Supplement Armv8, for R-profile AArch64 >>>>>>>> architecture, >>>>>>>> (DDI 0600B.a) explains what is the background region, it says it >>>>>>>> implements physical address range(s), so when we disable it, we would >>>>>>>> like any data >>>>>>>> access to complete before changing this implementation defined range >>>>>>>> with the ranges defined by us tweaking PRBAR/PRLAR, am I right? >>>>>>> My mental model for the ordering is similar to a TLB flush which is: >>>>>>> 1/ dsb nsh >>>>>>> 2/ tlbi >>>>>>> 3/ dsb nsh >>>>>>> 4/ isb >>>>>>> >>>>>>> Enabling the MPU is effectively 2. AFAIK, 1 is only necessary to ensure >>>>>>> the update to the page-tables. But it is not a requirement to ensure >>>>>>> any data access are completed (otherwise, we would have needed a dsb sy >>>>>>> because we don't know how far the access are going...). >>>>>> Uhm… I’m not sure we are on the same page, probably I explained that >>>>>> wrongly, so my point is that: >>>>>> >>>>>> FUNC_LOCAL(enable_mpu) >>>>>> mrs x0, SCTLR_EL2 >>>>>> bic x0, x0, #SCTLR_ELx_BR /* Disable Background region */ >>>>>> orr x0, x0, #SCTLR_Axx_ELx_M /* Enable MPU */ >>>>>> orr x0, x0, #SCTLR_Axx_ELx_C /* Enable D-cache */ >>>>>> orr x0, x0, #SCTLR_Axx_ELx_WXN /* Enable WXN */ >>>>>> dsb sy >>>>>> ^— This data barrier is needed in order to complete any data >>>>>> access, which guarantees that all explicit memory accesses before >>>>>> this instruction complete, so we can safely turn ON the MPU >>>>>> and disable the background region. >>>> >>>> I think >>> >>> Sorry I fat fingered and pressed send too early. I think this is the part I >>> disagree with. All explicit accesses don't need to be complete (in the >>> sense observed by everyone in the system). They only need to have gone >>> through the permissions check. >> >> I think I managed to find again the wording that would justify why a "dsb" >> is not necessary for the permission checks. From ARM DDI 0487K.a D23-7349: >> >> ``` >> Direct writes using the instructions in Table D22-2 require synchronization >> before software can rely on the effects >> of changes to the System registers to affect instructions appearing in >> program order after the direct write to the >> System register. Direct writes to these registers are not allowed to affect >> any instructions appearing in program order >> before the direct write. >> ``` >> >> I understand the paragraph as enabling the MPU via SCTLR_EL2 will not affect >> any instruction before hand. > > Yes, I agree. > > However, as the line states > > "Direct writes using the instructions in Table D22-2 require synchronization > before software can rely on the effects" > > This means synchronization is required after the write to SCTLR_EL2. > > However, this synchronization seems to imply dsb sy + isb. > > FromARM DDI 0487K.a ID032224 B2-274 > > "A DSB instruction ordered after a direct write to a System PMU register does > not complete until all observers observe the direct write" I think this is related only to the System PMU register, not to the registers in table D22-2 (which SCTLR_ELx are part) Anyway we could discuss this in person at the Xen meet-up and write a summary in the thread later. Cheers, Luca
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |