Hi Ayan,

> On 24 Oct 2024, at 09:02, Ayan Kumar Halder <[email protected]> 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 <[email protected]> wrote:
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> On 22/10/2024 10:56, Luca Fancellu wrote:
>>>>>>>>> On 22 Oct 2024, at 10:47, Julien Grall <[email protected]> wrote:
>>>>>>>>> 
>>>>>>>>> Hi Luca,
>>>>>>>>> 
>>>>>>>>> On 22/10/2024 10:41, Luca Fancellu wrote:
>>>>>>>>>>> On 21 Oct 2024, at 17:27, Julien Grall <[email protected]> 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

Reply via email to