Split region permissions into M-mode and SU-mode
cover Split region permissions into M-mode and SU-mode
This is first series in the series of forthcoming patches that would add support for different access permission for different sections of OpenSBI (RO, RX, RW). Eventually, this would lead to support of smepmp extension.
This patch series splits the current R/W/X permissions to M-mode R/W/X and SU-mode R/W/X. The split is in such a way that current permission semantics are not broken.
Himanshu Chauhan (9):
include: sbi: Fine grain the permissions for M and SU modes
lib: sbi: Use finer permission semantics for address validation
lib: sbi: Add permissions for the firmware start till end
lib: sbi: Use finer permission sematics to decide on PMP bits
lib: sbi: Modify the boot time region flag prints
lib: utils: Use SU-{R/W/X} flags for region permissions during parsing
lib: utils: Disallow non-root domains from adding M-mode regions
lib: utils: Add M-mode {R/W} flags to the MMIO regions
docs: Update domain's region permissions and requirements
docs/domain_support.md | 7 +++-- include/sbi/sbi_domain.h | 45 ++++++++++++++++++++++++++++---- lib/sbi/sbi_domain.c | 46 +++++++++++++++++++++++---------- lib/sbi/sbi_hart.c | 16 ++++++++---- lib/utils/fdt/fdt_domain.c | 20 +++++++++++--- lib/utils/ipi/aclint_mswi.c | 4 ++- lib/utils/irqchip/aplic.c | 4 ++- lib/utils/irqchip/imsic.c | 4 ++- lib/utils/timer/aclint_mtimer.c | 16 +++++++++--- 9 files changed, 127 insertions(+), 35 deletions(-)
Signature · 1 line
2.39.0
PATCH 1/9 include: sbi: Fine grain the permissions for M and SU modes
Split the permissions for M-mode and SU-mode. This would help if different sections of OpenSBI need to be given different permissions and if M-mode has different permisssions than the SU-mode over a region.
--- include/sbi/sbi_domain.h | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)
include/sbi/sbi_domain.h +35 -5
@@ -36,11 +36,41 @@ struct sbi_domain_memregion {*/ unsigned long base;/** Flags representing memory region attributes */ -#define SBI_DOMAIN_MEMREGION_READABLE (1UL << 0) -#define SBI_DOMAIN_MEMREGION_WRITEABLE (1UL << 1) -#define SBI_DOMAIN_MEMREGION_EXECUTABLE (1UL << 2) -#define SBI_DOMAIN_MEMREGION_MMODE (1UL << 3) -#define SBI_DOMAIN_MEMREGION_ACCESS_MASK (0xfUL) +#define SBI_DOMAIN_MEMREGION_M_READABLE (1UL << 0) +#define SBI_DOMAIN_MEMREGION_M_WRITABLE (1UL << 1) +#define SBI_DOMAIN_MEMREGION_M_EXECUTABLE (1UL << 2) +#define SBI_DOMAIN_MEMREGION_SU_READABLE (1UL << 3) +#define SBI_DOMAIN_MEMREGION_SU_WRITABLE (1UL << 4) +#define SBI_DOMAIN_MEMREGION_SU_EXECUTABLE (1UL << 5) + +/** Bit to control if permissions are enforced on all modes */ +#define SBI_DOMAIN_MEMREGION_ENF_PERMISSIONS (1UL << 6) + +#define SBI_DOMAIN_MEMREGION_M_RWX (SBI_DOMAIN_MEMREGION_M_READABLE | \ + SBI_DOMAIN_MEMREGION_M_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_EXECUTABLE) + +/* Unrestricted M-mode accesses but enfoced on SU-mode */ +#define SBI_DOMAIN_MEMREGION_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX) +#define SBI_DOMAIN_MEMREGION_WRITEABLE (SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX) +#define SBI_DOMAIN_MEMREGION_EXECUTABLE (SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX) + +/* Enforced accesses across all modes */ +#define SBI_DOMAIN_MEMREGION_ENF_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_M_READABLE) +#define SBI_DOMAIN_MEMREGION_ENF_WRITABLE (SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_WRITABLE) +#define SBI_DOMAIN_MEMREGION_ENF_EXECUTABLE (SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \ + SBI_DOMAIN_MEMREGION_M_EXECUTABLE) + +#define SBI_DOMAIN_MEMREGION_ACCESS_MASK (0x3fUL) +#define SBI_DOMAIN_MEMREGION_M_ACCESS_MASK (0x7UL) +#define SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK (0x38UL) + +#define SBI_DOMAIN_MEMREGION_SU_ACCESS_SHIFT (3)#define SBI_DOMAIN_MEMREGION_MMIO (1UL << 31)unsigned long flags;
Signature · 1 line
2.39.0
PATCH 1/9 include: sbi: Fine grain the permissions for M and SU modes
Quoted 43 lines · Himanshu Chauhan
Split the permissions for M-mode and SU-mode. This would help if different sections of OpenSBI need to be given different permissions and if M-mode has different permisssions than the SU-mode over a region.
Signed-off-by Himanshu Chauhan <[email protected]>--- include/sbi/sbi_domain.h | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)include/sbi/sbi_domain.h +19
-5@@ -36,11 +36,41 @@ struct sbi_domain_memregion {*/ unsigned long base;/** Flags representing memory region attributes */ -#define SBI_DOMAIN_MEMREGION_READABLE (1UL << 0) -#define SBI_DOMAIN_MEMREGION_WRITEABLE (1UL << 1) -#define SBI_DOMAIN_MEMREGION_EXECUTABLE (1UL << 2) -#define SBI_DOMAIN_MEMREGION_MMODE (1UL << 3) -#define SBI_DOMAIN_MEMREGION_ACCESS_MASK (0xfUL) +#define SBI_DOMAIN_MEMREGION_M_READABLE (1UL << 0) +#define SBI_DOMAIN_MEMREGION_M_WRITABLE (1UL << 1) +#define SBI_DOMAIN_MEMREGION_M_EXECUTABLE (1UL << 2) +#define SBI_DOMAIN_MEMREGION_SU_READABLE (1UL << 3) +#define SBI_DOMAIN_MEMREGION_SU_WRITABLE (1UL << 4) +#define SBI_DOMAIN_MEMREGION_SU_EXECUTABLE (1UL << 5) + +/** Bit to control if permissions are enforced on all modes */ +#define SBI_DOMAIN_MEMREGION_ENF_PERMISSIONS (1UL << 6) + +#define SBI_DOMAIN_MEMREGION_M_RWX (SBI_DOMAIN_MEMREGION_M_READABLE | \ + SBI_DOMAIN_MEMREGION_M_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_EXECUTABLE) + +/* Unrestricted M-mode accesses but enfoced on SU-mode */ +#define SBI_DOMAIN_MEMREGION_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX) +#define SBI_DOMAIN_MEMREGION_WRITEABLE (SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX)
include/sbi/sbi_domain.h struct sbi_domain_memregion { · line 67
Quoted 10 lines
include/sbi/sbi_domain.h +10
-0+#define SBI_DOMAIN_MEMREGION_EXECUTABLE (SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX) + +/* Enforced accesses across all modes */ +#define SBI_DOMAIN_MEMREGION_ENF_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_M_READABLE) +#define SBI_DOMAIN_MEMREGION_ENF_WRITABLE (SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_WRITABLE) +#define SBI_DOMAIN_MEMREGION_ENF_EXECUTABLE (SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \ + SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
Small nit: Please try to keep each line within 80 characters
Quoted 13 lines
include/sbi/sbi_domain.h +6
-1+ +#define SBI_DOMAIN_MEMREGION_ACCESS_MASK (0x3fUL) +#define SBI_DOMAIN_MEMREGION_M_ACCESS_MASK (0x7UL) +#define SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK (0x38UL) + +#define SBI_DOMAIN_MEMREGION_SU_ACCESS_SHIFT (3)#define SBI_DOMAIN_MEMREGION_MMIO (1UL << 31)unsigned long flags; --2.39.0
Quoted 4 lines
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
Otherwise, it looks good to me.
Regards,
Anup
PATCH 1/9 include: sbi: Fine grain the permissions for M and SU modes
-----Original Message-----
From: Anup Patel <[email protected]>
Sent: 06 January 2023 23:06
To: Himanshu Chauhan <[email protected]>
Cc: opensbi at lists.infradead.org
Subject: Re: [PATCH 1/9] include: sbi: Fine grain the permissions for M and SU modes
Quoted 47 lines · Himanshu Chauhan
Split the permissions for M-mode and SU-mode. This would help if different sections of OpenSBI need to be given different permissions and if M-mode has different permisssions than the SU-mode over a region.
Signed-off-by Himanshu Chauhan <[email protected]>--- include/sbi/sbi_domain.h | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)include/sbi/sbi_domain.h index renamed from include/sbi/sbi_domain.h +0
-05553d21..955ffa3 100644
include/sbi/sbi_domain.h +22
-5@@ -36,11 +36,41 @@ struct sbi_domain_memregion {*/ unsigned long base;/** Flags representing memory region attributes */ -#define SBI_DOMAIN_MEMREGION_READABLE (1UL << 0) -#define SBI_DOMAIN_MEMREGION_WRITEABLE (1UL << 1) -#define SBI_DOMAIN_MEMREGION_EXECUTABLE (1UL << 2) -#define SBI_DOMAIN_MEMREGION_MMODE (1UL << 3) -#define SBI_DOMAIN_MEMREGION_ACCESS_MASK (0xfUL) +#define SBI_DOMAIN_MEMREGION_M_READABLE (1UL << 0) +#define SBI_DOMAIN_MEMREGION_M_WRITABLE (1UL << 1) +#define SBI_DOMAIN_MEMREGION_M_EXECUTABLE (1UL << 2) +#define SBI_DOMAIN_MEMREGION_SU_READABLE (1UL << 3) +#define SBI_DOMAIN_MEMREGION_SU_WRITABLE (1UL << 4) +#define SBI_DOMAIN_MEMREGION_SU_EXECUTABLE (1UL << 5) + +/** Bit to control if permissions are enforced on all modes */ +#define SBI_DOMAIN_MEMREGION_ENF_PERMISSIONS (1UL << 6) + +#define SBI_DOMAIN_MEMREGION_M_RWX (SBI_DOMAIN_MEMREGION_M_READABLE | \ + SBI_DOMAIN_MEMREGION_M_WRITABLE | \ + +SBI_DOMAIN_MEMREGION_M_EXECUTABLE) + +/* Unrestricted M-mode accesses but enfoced on SU-mode */ +#define SBI_DOMAIN_MEMREGION_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX) +#define SBI_DOMAIN_MEMREGION_WRITEABLE (SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX) +#define SBI_DOMAIN_MEMREGION_EXECUTABLE (SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \ +
include/sbi/sbi_domain.h struct sbi_domain_memregion { · line 70
Quoted 10 lines
include/sbi/sbi_domain.h +10
-0+SBI_DOMAIN_MEMREGION_M_RWX) + +/* Enforced accesses across all modes */ +#define SBI_DOMAIN_MEMREGION_ENF_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_M_READABLE) +#define SBI_DOMAIN_MEMREGION_ENF_WRITABLE (SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_WRITABLE) +#define SBI_DOMAIN_MEMREGION_ENF_EXECUTABLE (SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \ + +SBI_DOMAIN_MEMREGION_M_EXECUTABLE)
Small nit: Please try to keep each line within 80 characters
It becomes difficult to build upon existing macros having large names and keep the column restrictions. Only way to honour the column restriction is to shorten the new and exiting names. I believe it would be too much change. Moreover, 80 column limit comes from very old terminal types. So it may be relaxed to a more sane value like 100 or so. Just my thoughts.
I can make the changes if you think restricting them to 80 column makes more sense. But that would take change to existing macro names.
Regards
Himanshu
Quoted 13 lines
include/sbi/sbi_domain.h +6
-1+ +#define SBI_DOMAIN_MEMREGION_ACCESS_MASK (0x3fUL) +#define SBI_DOMAIN_MEMREGION_M_ACCESS_MASK (0x7UL) +#define SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK (0x38UL) + +#define SBI_DOMAIN_MEMREGION_SU_ACCESS_SHIFT (3)#define SBI_DOMAIN_MEMREGION_MMIO (1UL << 31)unsigned long flags; --2.39.0
Quoted 4 lines
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
Otherwise, it looks good to me.
Regards,
Anup
PATCH 1/9 include: sbi: Fine grain the permissions for M and SU modes
Quoted 71 lines · On Mon, Jan 9, 2023 at 10:13 AM <[email protected]> wrote:
-----Original Message-----
From: Anup Patel <[email protected]>
Sent: 06 January 2023 23:06
To: Himanshu Chauhan <[email protected]>
Cc: opensbi at lists.infradead.org
Subject: Re: [PATCH 1/9] include: sbi: Fine grain the permissions for M and SU modesQuoted 47 lines · Himanshu Chauhan
Split the permissions for M-mode and SU-mode. This would help if different sections of OpenSBI need to be given different permissions and if M-mode has different permisssions than the SU-mode over a region.
Signed-off-by Himanshu Chauhan <[email protected]>--- include/sbi/sbi_domain.h | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)include/sbi/sbi_domain.h index renamed from include/sbi/sbi_domain.h +0
-05553d21..955ffa3 100644
include/sbi/sbi_domain.h +22
-5@@ -36,11 +36,41 @@ struct sbi_domain_memregion {*/ unsigned long base;/** Flags representing memory region attributes */ -#define SBI_DOMAIN_MEMREGION_READABLE (1UL << 0) -#define SBI_DOMAIN_MEMREGION_WRITEABLE (1UL << 1) -#define SBI_DOMAIN_MEMREGION_EXECUTABLE (1UL << 2) -#define SBI_DOMAIN_MEMREGION_MMODE (1UL << 3) -#define SBI_DOMAIN_MEMREGION_ACCESS_MASK (0xfUL) +#define SBI_DOMAIN_MEMREGION_M_READABLE (1UL << 0) +#define SBI_DOMAIN_MEMREGION_M_WRITABLE (1UL << 1) +#define SBI_DOMAIN_MEMREGION_M_EXECUTABLE (1UL << 2) +#define SBI_DOMAIN_MEMREGION_SU_READABLE (1UL << 3) +#define SBI_DOMAIN_MEMREGION_SU_WRITABLE (1UL << 4) +#define SBI_DOMAIN_MEMREGION_SU_EXECUTABLE (1UL << 5) + +/** Bit to control if permissions are enforced on all modes */ +#define SBI_DOMAIN_MEMREGION_ENF_PERMISSIONS (1UL << 6) + +#define SBI_DOMAIN_MEMREGION_M_RWX (SBI_DOMAIN_MEMREGION_M_READABLE | \ + SBI_DOMAIN_MEMREGION_M_WRITABLE | \ + +SBI_DOMAIN_MEMREGION_M_EXECUTABLE) + +/* Unrestricted M-mode accesses but enfoced on SU-mode */ +#define SBI_DOMAIN_MEMREGION_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX) +#define SBI_DOMAIN_MEMREGION_WRITEABLE (SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_RWX) +#define SBI_DOMAIN_MEMREGION_EXECUTABLE (SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \ +include/sbi/sbi_domain.h struct sbi_domain_memregion { · line 70
Quoted 10 lines
include/sbi/sbi_domain.h +10
-0+SBI_DOMAIN_MEMREGION_M_RWX) + +/* Enforced accesses across all modes */ +#define SBI_DOMAIN_MEMREGION_ENF_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_M_READABLE) +#define SBI_DOMAIN_MEMREGION_ENF_WRITABLE (SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_M_WRITABLE) +#define SBI_DOMAIN_MEMREGION_ENF_EXECUTABLE (SBI_DOMAIN_MEMREGION_SU_EXECUTABLE | \ + +SBI_DOMAIN_MEMREGION_M_EXECUTABLE)Small nit: Please try to keep each line within 80 characters
Quoted 3 lines
It becomes difficult to build upon existing macros having large names and keep the column restrictions. Only way to honour the column restriction is to shorten the new and exiting names. I believe it would be too much change. Moreover, 80 column limit comes from very old terminal types. So it may be relaxed to a more sane value like 100 or so. Just my thoughts.
I can make the changes if you think restricting them to 80 column makes more sense. But that would take change to existing macro names.
I am not strict about the 80 column requirement. I will take care of it at the time of merging this patch.
Regards,
Anup
Quoted 34 lines
Regards
HimanshuQuoted 13 lines
include/sbi/sbi_domain.h +6
-1+ +#define SBI_DOMAIN_MEMREGION_ACCESS_MASK (0x3fUL) +#define SBI_DOMAIN_MEMREGION_M_ACCESS_MASK (0x7UL) +#define SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK (0x38UL) + +#define SBI_DOMAIN_MEMREGION_SU_ACCESS_SHIFT (3)#define SBI_DOMAIN_MEMREGION_MMIO (1UL << 31)unsigned long flags; --2.39.0
Quoted 4 lines
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbiOtherwise, it looks good to me.
Reviewed-by Anup Patel <[email protected]>Regards,
Anup--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
PATCH 2/9 lib: sbi: Use finer permission semantics for address validation
In reply to Himanshu ChauhanUse the fine grained permisssion semantics for address validation of a given region.
--- lib/sbi/sbi_domain.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
lib/sbi/sbi_domain.c +16 -7
@@ -107,24 +107,33 @@ bool sbi_domain_check_addr(const struct sbi_domain *dom, {bool rmmio, mmio = FALSE;struct sbi_domain_memregion *reg; -unsigned long rstart, rend, rflags, rwx =0 ; +unsigned long rstart, rend, rflags, rwx =0 , rrwx =0 ;if (!dom)return FALSE; +/* +* Use M_{R/W/X} bits because the SU-bits are at the +* same relative offsets. If the mode is not M, the SU +* bits will fall at same offsets after the shift. +*/ if (access_flags &SBI_DOMAIN_READ ) - rwx |=SBI_DOMAIN_MEMREGION_READABLE ; + rwx |=SBI_DOMAIN_MEMREGION_M_READABLE ; +if (access_flags &SBI_DOMAIN_WRITE ) - rwx |=SBI_DOMAIN_MEMREGION_WRITEABLE ; + rwx |=SBI_DOMAIN_MEMREGION_M_WRITABLE ; +if (access_flags &SBI_DOMAIN_EXECUTE ) - rwx |=SBI_DOMAIN_MEMREGION_EXECUTABLE ; + rwx |=SBI_DOMAIN_MEMREGION_M_EXECUTABLE ; +if (access_flags &SBI_DOMAIN_MMIO ) mmio = TRUE;sbi_domain_for_each_memregion (dom, reg) { rflags = reg->flags; -if (mode ==PRV_M && !(rflags &SBI_DOMAIN_MEMREGION_MMODE )) -continue ; + rrwx = (mode ==PRV_M ? (rflags &SBI_DOMAIN_MEMREGION_M_ACCESS_MASK ) + : (rflags &SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK ) + >>SBI_DOMAIN_MEMREGION_SU_ACCESS_SHIFT ); rstart = reg->base; rend = (reg->order < __riscv_xlen) ?
@@ -133,7 +142,7 @@ bool sbi_domain_check_addr(const struct sbi_domain *dom, rmmio = (rflags &SBI_DOMAIN_MEMREGION_MMIO ) ? TRUE : FALSE;if (mmio != rmmio)return FALSE; -return ((rflags & rwx) == rwx) ? TRUE : FALSE; +return ((rrwx & rwx) == rwx) ? TRUE : FALSE; } }
Signature · 1 line
2.39.0
PATCH 2/9 lib: sbi: Use finer permission semantics for address validation
Quoted 5 lines · Himanshu Chauhan
Use the fine grained permisssion semantics for address validation of a given region.
Signed-off-by Himanshu Chauhan <[email protected]>
Looks good to me.
Regards,
Anup
Quoted 65 lines
--- lib/sbi/sbi_domain.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)lib/sbi/sbi_domain.c +16
-8@@ -107,24 +107,33 @@ bool sbi_domain_check_addr(const struct sbi_domain *dom, {bool rmmio, mmio = FALSE;struct sbi_domain_memregion *reg; -unsigned long rstart, rend, rflags, rwx =0 ; +unsigned long rstart, rend, rflags, rwx =0 , rrwx =0 ;if (!dom)return FALSE; +/* +* Use M_{R/W/X} bits because the SU-bits are at the +* same relative offsets. If the mode is not M, the SU +* bits will fall at same offsets after the shift. +*/ if (access_flags &SBI_DOMAIN_READ ) - rwx |=SBI_DOMAIN_MEMREGION_READABLE ; + rwx |=SBI_DOMAIN_MEMREGION_M_READABLE ; +if (access_flags &SBI_DOMAIN_WRITE ) - rwx |=SBI_DOMAIN_MEMREGION_WRITEABLE ; + rwx |=SBI_DOMAIN_MEMREGION_M_WRITABLE ; +if (access_flags &SBI_DOMAIN_EXECUTE ) - rwx |=SBI_DOMAIN_MEMREGION_EXECUTABLE ; + rwx |=SBI_DOMAIN_MEMREGION_M_EXECUTABLE ; +if (access_flags &SBI_DOMAIN_MMIO ) mmio = TRUE;sbi_domain_for_each_memregion (dom, reg) { rflags = reg->flags; -if (mode ==PRV_M && !(rflags &SBI_DOMAIN_MEMREGION_MMODE )) -continue ; + rrwx = (mode ==PRV_M ? (rflags &SBI_DOMAIN_MEMREGION_M_ACCESS_MASK ) + : (rflags &SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK ) + >>SBI_DOMAIN_MEMREGION_SU_ACCESS_SHIFT ); rstart = reg->base; rend = (reg->order < __riscv_xlen) ?@@ -133,7 +142,7 @@ bool sbi_domain_check_addr(const struct sbi_domain *dom, rmmio = (rflags &SBI_DOMAIN_MEMREGION_MMIO ) ? TRUE : FALSE;if (mmio != rmmio)return FALSE; -return ((rflags & rwx) == rwx) ? TRUE : FALSE; +return ((rrwx & rwx) == rwx) ? TRUE : FALSE; } } --2.39.0
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
PATCH 3/9 lib: sbi: Add permissions for the firmware start till end
In reply to Himanshu ChauhanChange the zero flag to M-mode R/W/X flag for the firmware region.
--- lib/sbi/sbi_domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
lib/sbi/sbi_domain.c +2 -1
@@ -628,7 +628,8 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)const struct sbi_platform *plat =sbi_platform_ptr (scratch);/* Root domain firmware memory region */ -sbi_domain_memregion_init (scratch->fw_start, scratch->fw_size,0 , +sbi_domain_memregion_init (scratch->fw_start, scratch->fw_size, +SBI_DOMAIN_MEMREGION_M_RWX , &root_fw_region);domain_memregion_initfw (&root_memregs[root_memregs_count++]);
Signature · 1 line
2.39.0
PATCH 3/9 lib: sbi: Add permissions for the firmware start till end
Quoted 5 lines · Himanshu Chauhan
Change the zero flag to M-mode R/W/X flag for the firmware region.
Signed-off-by Himanshu Chauhan <[email protected]>
Looks good to me.
Regards,
Anup
Quoted 26 lines
--- lib/sbi/sbi_domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)lib/sbi/sbi_domain.c +2
-2@@ -628,7 +628,8 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)const struct sbi_platform *plat =sbi_platform_ptr (scratch);/* Root domain firmware memory region */ -sbi_domain_memregion_init (scratch->fw_start, scratch->fw_size,0 , +sbi_domain_memregion_init (scratch->fw_start, scratch->fw_size, +SBI_DOMAIN_MEMREGION_M_RWX , &root_fw_region);domain_memregion_initfw (&root_memregs[root_memregs_count++]); --2.39.0
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
PATCH 4/9 lib: sbi: Use finer permission sematics to decide on PMP bits
In reply to Himanshu ChauhanUse the fine grained permission bits to decide if the region permissions are to enforced on all modes. Also use the new permission bits for deciding on R/W/X bits in pmpcfg register.
--- lib/sbi/sbi_hart.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
lib/sbi/sbi_hart.c +11 -5
@@ -303,14 +303,20 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)break ; pmp_flags =0 ; -if (reg->flags &SBI_DOMAIN_MEMREGION_READABLE ) + +/* +* If permissions are to be enforced for all modes on this region, +* the lock bit should be set. +*/ +if (reg->flags &SBI_DOMAIN_MEMREGION_ENF_PERMISSIONS ) + pmp_flags |=PMP_L ; + +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_READABLE ) pmp_flags |=PMP_R ; -if (reg->flags &SBI_DOMAIN_MEMREGION_WRITEABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_WRITABLE ) pmp_flags |=PMP_W ; -if (reg->flags &SBI_DOMAIN_MEMREGION_EXECUTABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_EXECUTABLE ) pmp_flags |=PMP_X ; -if (reg->flags &SBI_DOMAIN_MEMREGION_MMODE ) - pmp_flags |=PMP_L ; pmp_addr = reg->base >>PMP_SHIFT ;if (pmp_gran_log2 <= reg->order && pmp_addr < pmp_addr_max)
Signature · 1 line
2.39.0
PATCH 4/9 lib: sbi: Use finer permission sematics to decide on PMP bits
Quoted 3 lines · Himanshu Chauhan
Use the fine grained permission bits to decide if the region permissions are to enforced on all modes. Also use the new
s/are to/are to be/
Quoted 3 lines
permission bits for deciding on R/W/X bits in pmpcfg register.
Signed-off-by Himanshu Chauhan <[email protected]>
Otherwise, looks good to me.
Regards,
Anup
Quoted 42 lines
--- lib/sbi/sbi_hart.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)lib/sbi/sbi_hart.c +11
-6@@ -303,14 +303,20 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)break ; pmp_flags =0 ; -if (reg->flags &SBI_DOMAIN_MEMREGION_READABLE ) + +/* +* If permissions are to be enforced for all modes on this region, +* the lock bit should be set. +*/ +if (reg->flags &SBI_DOMAIN_MEMREGION_ENF_PERMISSIONS ) + pmp_flags |=PMP_L ; + +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_READABLE ) pmp_flags |=PMP_R ; -if (reg->flags &SBI_DOMAIN_MEMREGION_WRITEABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_WRITABLE ) pmp_flags |=PMP_W ; -if (reg->flags &SBI_DOMAIN_MEMREGION_EXECUTABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_EXECUTABLE ) pmp_flags |=PMP_X ; -if (reg->flags &SBI_DOMAIN_MEMREGION_MMODE ) - pmp_flags |=PMP_L ; pmp_addr = reg->base >>PMP_SHIFT ;if (pmp_gran_log2 <= reg->order && pmp_addr < pmp_addr_max) --2.39.0
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
PATCH 5/9 lib: sbi: Modify the boot time region flag prints
In reply to Himanshu ChauhanWith the finer permission semantics, the region access permissions must be displayed separately for M and SU mode.
--- lib/sbi/sbi_domain.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)
lib/sbi/sbi_domain.c +15 -5
@@ -344,15 +344,25 @@ void sbi_domain_dump(const struct sbi_domain *dom, const char *suffix) dom->index, i, suffix, rstart, rend); k =0 ; -if (reg->flags &SBI_DOMAIN_MEMREGION_MMODE ) -sbi_printf ("%cM" , (k++) ?',' :'(' ); + +sbi_printf ("M: " );if (reg->flags &SBI_DOMAIN_MEMREGION_MMIO )sbi_printf ("%cI" , (k++) ?',' :'(' ); -if (reg->flags &SBI_DOMAIN_MEMREGION_READABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_M_READABLE ) +sbi_printf ("%cR" , (k++) ?',' :'(' ); +if (reg->flags &SBI_DOMAIN_MEMREGION_M_WRITABLE ) +sbi_printf ("%cW" , (k++) ?',' :'(' ); +if (reg->flags &SBI_DOMAIN_MEMREGION_M_EXECUTABLE ) +sbi_printf ("%cX" , (k++) ?',' :'(' ); +sbi_printf ("%s " , (k++) ?")" :"()" ); + + k =0 ; +sbi_printf ("S/U: " ); +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_READABLE )sbi_printf ("%cR" , (k++) ?',' :'(' ); -if (reg->flags &SBI_DOMAIN_MEMREGION_WRITEABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_WRITABLE )sbi_printf ("%cW" , (k++) ?',' :'(' ); -if (reg->flags &SBI_DOMAIN_MEMREGION_EXECUTABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_EXECUTABLE )sbi_printf ("%cX" , (k++) ?',' :'(' );sbi_printf ("%s\n" , (k++) ?")" :"()" );
Signature · 1 line
2.39.0
PATCH 5/9 lib: sbi: Modify the boot time region flag prints
Quoted 5 lines · Himanshu Chauhan
With the finer permission semantics, the region access permissions must be displayed separately for M and SU mode.
Signed-off-by Himanshu Chauhan <[email protected]>
Looks good to me.
Regards,
Anup
Quoted 47 lines
--- lib/sbi/sbi_domain.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)lib/sbi/sbi_domain.c +15
-6@@ -344,15 +344,25 @@ void sbi_domain_dump(const struct sbi_domain *dom, const char *suffix) dom->index, i, suffix, rstart, rend); k =0 ; -if (reg->flags &SBI_DOMAIN_MEMREGION_MMODE ) -sbi_printf ("%cM" , (k++) ?',' :'(' ); + +sbi_printf ("M: " );if (reg->flags &SBI_DOMAIN_MEMREGION_MMIO )sbi_printf ("%cI" , (k++) ?',' :'(' ); -if (reg->flags &SBI_DOMAIN_MEMREGION_READABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_M_READABLE ) +sbi_printf ("%cR" , (k++) ?',' :'(' ); +if (reg->flags &SBI_DOMAIN_MEMREGION_M_WRITABLE ) +sbi_printf ("%cW" , (k++) ?',' :'(' ); +if (reg->flags &SBI_DOMAIN_MEMREGION_M_EXECUTABLE ) +sbi_printf ("%cX" , (k++) ?',' :'(' ); +sbi_printf ("%s " , (k++) ?")" :"()" ); + + k =0 ; +sbi_printf ("S/U: " ); +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_READABLE )sbi_printf ("%cR" , (k++) ?',' :'(' ); -if (reg->flags &SBI_DOMAIN_MEMREGION_WRITEABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_WRITABLE )sbi_printf ("%cW" , (k++) ?',' :'(' ); -if (reg->flags &SBI_DOMAIN_MEMREGION_EXECUTABLE ) +if (reg->flags &SBI_DOMAIN_MEMREGION_SU_EXECUTABLE )sbi_printf ("%cX" , (k++) ?',' :'(' );sbi_printf ("%s\n" , (k++) ?")" :"()" ); --2.39.0
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
PATCH 6/9 lib: utils: Use SU-{R/W/X} flags for region permissions during parsing
In reply to Himanshu ChauhanUse the newer SU-{R/W/X} flags for checking and assigning region permissions.
--- lib/utils/fdt/fdt_domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
lib/utils/fdt/fdt_domain.c +3 -3
@@ -338,9 +338,9 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)* 2) mmio regions protecting M-mode only mmio devices */ sbi_domain_for_each_memregion (&root, reg) { -if ((reg->flags &SBI_DOMAIN_MEMREGION_READABLE ) || - (reg->flags &SBI_DOMAIN_MEMREGION_WRITEABLE ) || - (reg->flags &SBI_DOMAIN_MEMREGION_EXECUTABLE )) +if ((reg->flags &SBI_DOMAIN_MEMREGION_SU_READABLE ) || + (reg->flags &SBI_DOMAIN_MEMREGION_SU_WRITABLE ) || + (reg->flags &SBI_DOMAIN_MEMREGION_SU_EXECUTABLE ))continue ;if (FDT_DOMAIN_REGION_MAX_COUNT <= val32)return SBI_EINVAL ;
Signature · 1 line
2.39.0
PATCH 6/9 lib: utils: Use SU-{R/W/X} flags for region permissions during parsing
Quoted 5 lines · Himanshu Chauhan
Use the newer SU-{R/W/X} flags for checking and assigning region permissions.
Signed-off-by Himanshu Chauhan <[email protected]>
Looks good to me.
Regards,
Anup
Quoted 29 lines
--- lib/utils/fdt/fdt_domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)lib/utils/fdt/fdt_domain.c +3
-4@@ -338,9 +338,9 @@ static int __fdt_parse_domain(void *fdt, int domain_offset, void *opaque)* 2) mmio regions protecting M-mode only mmio devices */ sbi_domain_for_each_memregion (&root, reg) { -if ((reg->flags &SBI_DOMAIN_MEMREGION_READABLE ) || - (reg->flags &SBI_DOMAIN_MEMREGION_WRITEABLE ) || - (reg->flags &SBI_DOMAIN_MEMREGION_EXECUTABLE )) +if ((reg->flags &SBI_DOMAIN_MEMREGION_SU_READABLE ) || + (reg->flags &SBI_DOMAIN_MEMREGION_SU_WRITABLE ) || + (reg->flags &SBI_DOMAIN_MEMREGION_SU_EXECUTABLE ))continue ;if (FDT_DOMAIN_REGION_MAX_COUNT <= val32)return SBI_EINVAL ; --2.39.0
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
PATCH 7/9 lib: utils: Disallow non-root domains from adding M-mode regions
In reply to Himanshu ChauhanThe M-mode regions can only be added by root domain. The non-root domains shouldn't be able to add them from FDT.
--- include/sbi/sbi_domain.h | 5 +++++ lib/utils/fdt/fdt_domain.c | 14 ++++++++++++++ 2 files changed, 19 insertions(+)
include/sbi/sbi_domain.h +5 -0
@@ -50,6 +50,11 @@ struct sbi_domain_memregion { SBI_DOMAIN_MEMREGION_M_WRITABLE | \ SBI_DOMAIN_MEMREGION_M_EXECUTABLE) +#define SBI_DOMAIN_MEMREGION_SU_RWX (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_SU_EXECUTABLE) + +/* Unrestricted M-mode accesses but enfoced on SU-mode */ #define SBI_DOMAIN_MEMREGION_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ SBI_DOMAIN_MEMREGION_M_RWX)
lib/utils/fdt/fdt_domain.c +14 -0
@@ -239,6 +239,20 @@ static int __fdt_parse_region(void *fdt, int domain_offset,u32 *region_count = opaque;struct sbi_domain_memregion *region; +/* +* Non-root domains cannot add a region with only M-mode +* access permissions. M-mode regions can only be part of +* root domain. +* +* SU permission bits can't be all zeroes and M-mode permission +* bits must be all set. +*/ +if (!((region_access &SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK ) + &SBI_DOMAIN_MEMREGION_SU_RWX ) + && ((region_access &SBI_DOMAIN_MEMREGION_M_ACCESS_MASK ) + &SBI_DOMAIN_MEMREGION_M_RWX )) +return SBI_EINVAL ; +/* Find next region of the domain */ if (FDT_DOMAIN_REGION_MAX_COUNT <= *region_count)return SBI_EINVAL ;
Signature · 1 line
2.39.0
PATCH 7/9 lib: utils: Disallow non-root domains from adding M-mode regions
Quoted 2 lines · Himanshu Chauhan
The M-mode regions can only be added by root domain. The non-root
s/be added by/be added to/
Quoted 13 lines
domains shouldn't be able to add them from FDT.
Signed-off-by Himanshu Chauhan <[email protected]>--- include/sbi/sbi_domain.h | 5 +++++ lib/utils/fdt/fdt_domain.c | 14 ++++++++++++++ 2 files changed, 19 insertions(+)include/sbi/sbi_domain.h +0
-0
include/sbi/sbi_domain.h struct sbi_domain_memregion { · line 55
Quoted 6 lines
include/sbi/sbi_domain.h +3
-0@@ -50,6 +50,11 @@ struct sbi_domain_memregion { SBI_DOMAIN_MEMREGION_M_WRITABLE | \ SBI_DOMAIN_MEMREGION_M_EXECUTABLE) +#define SBI_DOMAIN_MEMREGION_SU_RWX (SBI_DOMAIN_MEMREGION_SU_READABLE | \ + SBI_DOMAIN_MEMREGION_SU_WRITABLE | \ + SBI_DOMAIN_MEMREGION_SU_EXECUTABLE)
Small nit: Please ensure line is within 80 characters
include/sbi/sbi_domain.h struct sbi_domain_memregion { · line 57
Quoted 2 lines
include/sbi/sbi_domain.h +2
-0+ +
Redundant new line.
Quoted 32 lines
include/sbi/sbi_domain.h +0
-0/* Unrestricted M-mode accesses but enfoced on SU-mode */ #define SBI_DOMAIN_MEMREGION_READABLE (SBI_DOMAIN_MEMREGION_SU_READABLE | \ SBI_DOMAIN_MEMREGION_M_RWX)lib/utils/fdt/fdt_domain.c +14
-1@@ -239,6 +239,20 @@ static int __fdt_parse_region(void *fdt, int domain_offset,u32 *region_count = opaque;struct sbi_domain_memregion *region; +/* +* Non-root domains cannot add a region with only M-mode +* access permissions. M-mode regions can only be part of +* root domain. +* +* SU permission bits can't be all zeroes and M-mode permission +* bits must be all set. +*/ +if (!((region_access &SBI_DOMAIN_MEMREGION_SU_ACCESS_MASK ) + &SBI_DOMAIN_MEMREGION_SU_RWX ) + && ((region_access &SBI_DOMAIN_MEMREGION_M_ACCESS_MASK ) + &SBI_DOMAIN_MEMREGION_M_RWX )) +return SBI_EINVAL ; +/* Find next region of the domain */ if (FDT_DOMAIN_REGION_MAX_COUNT <= *region_count)return SBI_EINVAL ; --2.39.0
Quoted 4 lines
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
Otherwise, it looks good to me.
Regards,
Anup
PATCH 8/9 lib: utils: Add M-mode {R/W} flags to the MMIO regions
In reply to Himanshu ChauhanAdd the M-mode readable/writable flags to mmio regions of various drivers.
--- lib/utils/ipi/aclint_mswi.c | 4 +++- lib/utils/irqchip/aplic.c | 4 +++- lib/utils/irqchip/imsic.c | 4 +++- lib/utils/timer/aclint_mtimer.c | 16 ++++++++++++---- 4 files changed, 21 insertions(+), 7 deletions(-)
lib/utils/ipi/aclint_mswi.c +3 -1
@@ -88,7 +88,9 @@ int aclint_mswi_cold_init(struct aclint_mswi_data *mswi) region_size = ((mswi->size - pos) <ACLINT_MSWI_ALIGN ) ? (mswi->size - pos) :ACLINT_MSWI_ALIGN ;sbi_domain_memregion_init (mswi->addr + pos, region_size, -SBI_DOMAIN_MEMREGION_MMIO , ®); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ), ®); rc =sbi_domain_root_add_memregion (®);if (rc)return rc;
lib/utils/irqchip/aplic.c +3 -1
@@ -269,7 +269,9 @@ int aplic_cold_irqchip_init(struct aplic_data *aplic) (last_deleg_irq == aplic->num_source) && (first_deleg_irq == 1))) {sbi_domain_memregion_init (aplic->addr, aplic->size, -SBI_DOMAIN_MEMREGION_MMIO , ®); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ), ®); rc =sbi_domain_root_add_memregion (®);if (rc)return rc;
lib/utils/irqchip/imsic.c +3 -1
@@ -313,7 +313,9 @@ int imsic_cold_irqchip_init(struct imsic_data *imsic)for (i =0 ; i <IMSIC_MAX_REGS && imsic->regs[i].size; i++) {sbi_domain_memregion_init (imsic->regs[i].addr, imsic->regs[i].size, -SBI_DOMAIN_MEMREGION_MMIO , ®); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ), ®); rc =sbi_domain_root_add_memregion (®);if (rc)return rc;
lib/utils/timer/aclint_mtimer.c +12 -4
@@ -188,26 +188,34 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt, rc =sbi_domain_root_add_memrange (mt->mtimecmp_addr, mt->mtime_size + mt->mtimecmp_size,MTIMER_REGION_ALIGN , -SBI_DOMAIN_MEMREGION_MMIO ); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ));if (rc)return rc; }else if (mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) { rc =sbi_domain_root_add_memrange (mt->mtime_addr, mt->mtime_size + mt->mtimecmp_size,MTIMER_REGION_ALIGN , -SBI_DOMAIN_MEMREGION_MMIO ); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ));if (rc)return rc; }else { rc =sbi_domain_root_add_memrange (mt->mtime_addr, mt->mtime_size,MTIMER_REGION_ALIGN , -SBI_DOMAIN_MEMREGION_MMIO ); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ));if (rc)return rc; rc =sbi_domain_root_add_memrange (mt->mtimecmp_addr, mt->mtimecmp_size,MTIMER_REGION_ALIGN , -SBI_DOMAIN_MEMREGION_MMIO ); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ));if (rc)return rc; }
Signature · 1 line
2.39.0
PATCH 8/9 lib: utils: Add M-mode {R/W} flags to the MMIO regions
Quoted 5 lines · Himanshu Chauhan
Add the M-mode readable/writable flags to mmio regions of various drivers.
Signed-off-by Himanshu Chauhan <[email protected]>
Looks good to me.
Regards,
Anup
Quoted 103 lines
--- lib/utils/ipi/aclint_mswi.c | 4 +++- lib/utils/irqchip/aplic.c | 4 +++- lib/utils/irqchip/imsic.c | 4 +++- lib/utils/timer/aclint_mtimer.c | 16 ++++++++++++---- 4 files changed, 21 insertions(+), 7 deletions(-)lib/utils/ipi/aclint_mswi.c +3
-1@@ -88,7 +88,9 @@ int aclint_mswi_cold_init(struct aclint_mswi_data *mswi) region_size = ((mswi->size - pos) <ACLINT_MSWI_ALIGN ) ? (mswi->size - pos) :ACLINT_MSWI_ALIGN ;sbi_domain_memregion_init (mswi->addr + pos, region_size, -SBI_DOMAIN_MEMREGION_MMIO , ®); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ), ®); rc =sbi_domain_root_add_memregion (®);if (rc)return rc;lib/utils/irqchip/aplic.c +3
-1@@ -269,7 +269,9 @@ int aplic_cold_irqchip_init(struct aplic_data *aplic) (last_deleg_irq == aplic->num_source) && (first_deleg_irq == 1))) {sbi_domain_memregion_init (aplic->addr, aplic->size, -SBI_DOMAIN_MEMREGION_MMIO , ®); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ), ®); rc =sbi_domain_root_add_memregion (®);if (rc)return rc;lib/utils/irqchip/imsic.c +3
-1@@ -313,7 +313,9 @@ int imsic_cold_irqchip_init(struct imsic_data *imsic)for (i =0 ; i <IMSIC_MAX_REGS && imsic->regs[i].size; i++) {sbi_domain_memregion_init (imsic->regs[i].addr, imsic->regs[i].size, -SBI_DOMAIN_MEMREGION_MMIO , ®); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ), ®); rc =sbi_domain_root_add_memregion (®);if (rc)return rc;lib/utils/timer/aclint_mtimer.c +12
-5@@ -188,26 +188,34 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt, rc =sbi_domain_root_add_memrange (mt->mtimecmp_addr, mt->mtime_size + mt->mtimecmp_size,MTIMER_REGION_ALIGN , -SBI_DOMAIN_MEMREGION_MMIO ); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ));if (rc)return rc; }else if (mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) { rc =sbi_domain_root_add_memrange (mt->mtime_addr, mt->mtime_size + mt->mtimecmp_size,MTIMER_REGION_ALIGN , -SBI_DOMAIN_MEMREGION_MMIO ); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ));if (rc)return rc; }else { rc =sbi_domain_root_add_memrange (mt->mtime_addr, mt->mtime_size,MTIMER_REGION_ALIGN , -SBI_DOMAIN_MEMREGION_MMIO ); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ));if (rc)return rc; rc =sbi_domain_root_add_memrange (mt->mtimecmp_addr, mt->mtimecmp_size,MTIMER_REGION_ALIGN , -SBI_DOMAIN_MEMREGION_MMIO ); + (SBI_DOMAIN_MEMREGION_MMIO | +SBI_DOMAIN_MEMREGION_M_READABLE | +SBI_DOMAIN_MEMREGION_M_WRITABLE ));if (rc)return rc; } --2.39.0
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
PATCH 9/9 docs: Update domain's region permissions and requirements
In reply to Himanshu ChauhanUpdated the various permissions bits available for domains defined in DT node and restrictions on them.
--- docs/domain_support.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
docs/domain_support.md +5 -2
@@ -160,8 +160,11 @@ The DT properties of a domain instance DT node are as follows: * **regions** (Optional) - The list of domain memory region DT node phandle and access permissions for the domain instance. Each list entry is a pair of DT node phandle and access permissions. The access permissions are - represented as a 32bit bitmask having bits: **readable** (BIT[0]), - **writeable** (BIT[1]), **executable** (BIT[2]), and **m-mode** (BIT[3]). + represented as a 32bit bitmask having bits: **M readable** (BIT[0]), + **M writeable** (BIT[1]), **M executable** (BIT[2]), **SU readable** + (BIT[3]), **SU writable** (BIT[4]), and **SU executable** (BIT[5]). + Any region of a domain defined in DT node cannot have only M-bits set + in access permissions i.e. it cannot be an m-mode only accessible region. * **boot-hart** (Optional) - The DT node phandle of the HART booting the domain instance. If coldboot HART is assigned to the domain instance then this DT property is ignored and the coldboot HART is assumed to be the
Signature · 1 line
2.39.0
PATCH 9/9 docs: Update domain's region permissions and requirements
Quoted 14 lines · Himanshu Chauhan
Updated the various permissions bits available for domains defined in DT node and restrictions on them.
Signed-off-by Himanshu Chauhan <[email protected]>--- docs/domain_support.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)docs/domain_support.md +0
-0
docs/domain_support.md The DT properties of a domain instance DT node are as follows: · line 165
Quoted 8 lines
docs/domain_support.md +3
-2@@ -160,8 +160,11 @@ The DT properties of a domain instance DT node are as follows: * **regions** (Optional) - The list of domain memory region DT node phandle and access permissions for the domain instance. Each list entry is a pair of DT node phandle and access permissions. The access permissions are - represented as a 32bit bitmask having bits: **readable** (BIT[0]), - **writeable** (BIT[1]), **executable** (BIT[2]), and **m-mode** (BIT[3]). + represented as a 32bit bitmask having bits: **M readable** (BIT[0]), + **M writeable** (BIT[1]), **M executable** (BIT[2]), **SU readable** + (BIT[3]), **SU writable** (BIT[4]), and **SU executable** (BIT[5]).
I think you missed documenting BIT[6] which is for enforcing.
Quoted 9 lines
docs/domain_support.md +2
-1+ Any region of a domain defined in DT node cannot have only M-bits set + in access permissions i.e. it cannot be an m-mode only accessible region. * **boot-hart** (Optional) - The DT node phandle of the HART booting the domain instance. If coldboot HART is assigned to the domain instance then this DT property is ignored and the coldboot HART is assumed to be the --2.39.0
Quoted 4 lines
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
Regards,
Anup