[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 3/4] xen/arm: mpu: Create boot-time MPU protection regions


  • To: Luca Fancellu <Luca.Fancellu@xxxxxxx>
  • From: Ayan Kumar Halder <ayankuma@xxxxxxx>
  • Date: Fri, 6 Sep 2024 13:59:41 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ewwe7E7w2q4tL728ze36DY9Z1TMdfBguie6Bji7Tdy8=; b=xfjAjGsLAcWKY+pgGIHCQpbGKwLX1CeiKpjTkY9ul48fPy9vdb4ycER2YmksHKENGTAi2ccJ8IZZBpr7vurKMR909nm1Fs3ApxYSQo88dhSan7N89Slj28sxNiog07B5f0R59KEV+4l1AgNFSwcvuGrThQZHJvbuXrRvlY4sM20o5Vb8XpMOF4KBrtAiouHtcnyKJ5vUz0rFhfD+qMxxfspatLS90McWXWTz8z3ixjkRgE9c76ZDpPgrMFFY/D7Kt/cfMihtdognDjCLex8sjQ9mTptxLOjDWJQ9arpttWYZCp4e5bKzQMwTTQJh1ap1ssgXQoplsjElnvur1UClZw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=sdO8mmoeAZhzghNMFwE7IFLd7i866I5GWDHbTlSu33l021le9VLIG9BzTb5SXbeTcNFat8b/zMIujKysUBUQ03TF+2DyygND2h7aqoTAQghJ7zIUMcyvFLwJEw+OT4riLcitMuAyze+6PpnvceWSvRGgOeHnvjGGhGZEFjNjFWaRY+aBYGG/+5FvYSZMFKC3bsgBUls7G/maYiR1JRravc4StvF+cJGY/JcNt6XLvU0zEPgVwtTWMS9XjeeXIJvq4b90UFZAvN9FkjZkeVtedaHXiBolrpk/tTJ9/0ZvOLn1ZAMcaZQUPJOK0qvlFgF4p8BCPZs3lNMWnoPPIXPnGg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: Julien Grall <julien@xxxxxxx>, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Fri, 06 Sep 2024 13:00:05 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Julien/Luca/all,

On 04/09/2024 19:38, Ayan Kumar Halder wrote:

On 04/09/2024 19:14, Luca Fancellu wrote:
Hi Ayan,
Hi Luca,

Apologies but I can’t do a full review yet,
No worries. :)


+
+/* MPU normal memory attributes. */
+#define PRBAR_NORMAL_MEM        0x30    /* SH=11 AP=00 XN=00 */
+#define PRLAR_NORMAL_MEM        0x0f    /* NS=0 ATTR=111 EN=1 */
+
+.macro write_pr, sel, prbar, prlar
+    msr   PRSELR_EL2, \sel
+    dsb   sy
I am not sure I understand why this is a dsb rather than isb. Can you clarify?
ISB is not needed here as the memory protection hasn't been activated yet. The above instruction just selects the memory region and the below two instructions sets the base address and limit for that memory region. After the three instructions, we need an ISB so that the memory protection takes into affect for further instruction fetches.

However, a DSB is needed here as the below two instructions depend on this. So, we definitely want this instruction to complete.

Further, refer to the note in ARM DDI 0600A.d ID120821, C1.7.1 "Protection region attributes"

0.

   ```Writes to MPU registers are only guaranteed to be visible
   following a Context synchronization event and DSB operation.```

Thus, I infer that DSB is necessary here.
I think this was a mistake from the author of this patch, in my opinion there should be an ISB after setting PRSELR_ELx, to enforce a synchronisation before writing PR{B,L}AR_ELx which
depends on the value written on PRSELR.

That synchronisation is enforced by DSB.

From https://developer.arm.com/documentation/dui0489/c/arm-and-thumb-instructions/miscellaneous-instructions/dmb--dsb--and-isb ,

```Data Synchronization Barrier acts as a special kind of memory barrier. No instruction in program order after this instruction executes until this instruction completes.

...

Instruction Synchronization Barrier flushes the pipeline in the processor, ```


Why should we flush the instruction pipeline after setting PRSELR_ELx ? It does not have any impact on instruction fetch.

After the discussion on matrix, I agree that we need an 'isb' here to ensure that there is no instruction reordering.

Thanks for the explanation.

- Ayan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.