[PATCH v2 15/16] lib: sbi: Display domain details in boot prints
Atish Patra
atishp at atishpatra.org
Mon Oct 19 19:38:22 EDT 2020
On Sun, Oct 18, 2020 at 10:28 PM Anup Patel <Anup.Patel at wdc.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Atish Patra <atishp at atishpatra.org>
> > Sent: 19 October 2020 05:16
> > To: Anup Patel <Anup.Patel at wdc.com>
> > Cc: Atish Patra <Atish.Patra at wdc.com>; Alistair Francis
> > <Alistair.Francis at wdc.com>; Anup Patel <anup at brainfault.org>; OpenSBI
> > <opensbi at lists.infradead.org>
> > Subject: Re: [PATCH v2 15/16] lib: sbi: Display domain details in boot prints
> >
> > On Thu, Oct 15, 2020 at 6:28 AM Anup Patel <anup.patel at wdc.com> wrote:
> > >
> > > We extend boot prints to display details of each domain. In the
> > > process, we remove sbi_hart_pmp_dump() because it shows redundant
> > > information which domain details already show.
> > >
> >
> > The updated boot print doesn't show any more PMP regions which may be
> > confusing.
> > The domain memory regions are based on PMP but that information is not
> > explicit to the users.
> > Should we specify the type of memory protection (PMP by default) in either
> > in this line
> >
> > Domain0 Region00
> >
> > or a separate line where what kind of memory protection schemes are used.
>
> The PMP, ePMP, IOPMP, SiFive shield, etc will be configured based on the
> domain memory regions. This means PMP configuration is be based on the
> domain memory regions (not the reverse). We have ensured that domain
> memory region can be easily translated to PMP region.
>
> Regarding what kind of memory protection schemes are use, we already have
> "Boot HART PMP Count" which if zero means that we don't have any PMP
> regions hence no protection. In future, if platform has IOPMP then we show
> details of all IOPMPs in boot prints.
>
Let's say we have both PMP, SiFive shield in XYZ platform in t time
in future :).
Can we have different protection schemes co-exist in that case? That
means, there may be some
regions are protected by PMP and some are protected by SiFive shield.
I don't have a use case where
you want that. I am just trying to understand the capabilities/possibilities.
Looking at the current boot time print, it doesn't indicate what
protection scheme OpenSBI is using.
PMP/IOPMP count can be greater than 0 but the region might be
protected using SiFive shield or something else.
How does a user/developer know this information ?
Domain0 Name : root
Domain0 Boot HART : 0
Domain0 HARTs : 0*,1*,2*,3*,4*,5*,6*,7*
Domain0 Region00 : 0x0000000080000000-0x000000008003ffff ()
Domain0 Region01 : 0x0000000000000000-0xffffffffffffffff (R,W,X)
Domain0 Next Address: 0x0000000080200000
Domain0 Next Arg1 : 0x0000000082200000
Domain0 Next Mode : S-mode
Domain0 SysReset : yes
Boot HART ID : 0
Boot HART Domain : root
Boot HART ISA : rv64imafdcsu
Boot HART Features : scounteren,mcounteren,time
Boot HART PMP Count : 16
Boot HART MHPM Count: 0
Boot HART MIDELEG : 0x0000000000000222
Boot HART MEDELEG : 0x000000000000b109
> >
> > > Signed-off-by: Anup Patel <anup.patel at wdc.com>
> > > ---
> > > include/sbi/sbi_domain.h | 6 +++
> > > include/sbi/sbi_hart.h | 4 +-
> > > lib/sbi/sbi_domain.c | 94
> > ++++++++++++++++++++++++++++++++++++++++
> > > lib/sbi/sbi_hart.c | 44 +++++--------------
> > > lib/sbi/sbi_init.c | 27 +++++++-----
> > > 5 files changed, 127 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/include/sbi/sbi_domain.h b/include/sbi/sbi_domain.h index
> > > 17fb013..646f51d 100644
> > > --- a/include/sbi/sbi_domain.h
> > > +++ b/include/sbi/sbi_domain.h
> > > @@ -133,6 +133,12 @@ bool sbi_domain_check_addr(const struct
> > sbi_domain *dom,
> > > unsigned long addr, unsigned long mode, bool mmio,
> > > bool read, bool write, bool execute);
> > >
> > > +/** Dump domain details on the console */ void sbi_domain_dump(const
> > > +struct sbi_domain *dom, const char *suffix);
> > > +
> > > +/** Dump all domain details on the console */ void
> > > +sbi_domain_dump_all(const char *suffix);
> > > +
> > > /** Finalize domain tables and startup non-root domains */ int
> > > sbi_domain_finalize(struct sbi_scratch *scratch, u32 cold_hartid);
> > >
> > > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h index
> > > 79d745a..3ac1500 100644
> > > --- a/include/sbi/sbi_hart.h
> > > +++ b/include/sbi/sbi_hart.h
> > > @@ -36,9 +36,9 @@ static inline ulong
> > > sbi_hart_expected_trap_addr(void) }
> > >
> > > unsigned int sbi_hart_mhpm_count(struct sbi_scratch *scratch); -void
> > > sbi_hart_delegation_dump(struct sbi_scratch *scratch);
> > > +void sbi_hart_delegation_dump(struct sbi_scratch *scratch,
> > > + const char *prefix, const char *suffix);
> > > unsigned int sbi_hart_pmp_count(struct sbi_scratch *scratch); -void
> > > sbi_hart_pmp_dump(struct sbi_scratch *scratch); int
> > > sbi_hart_pmp_configure(struct sbi_scratch *scratch); bool
> > > sbi_hart_has_feature(struct sbi_scratch *scratch, unsigned long
> > > feature); void sbi_hart_get_features_str(struct sbi_scratch *scratch,
> > > diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c index
> > > 7659409..b51a79d 100644
> > > --- a/lib/sbi/sbi_domain.c
> > > +++ b/lib/sbi/sbi_domain.c
> > > @@ -8,6 +8,7 @@
> > > */
> > >
> > > #include <sbi/riscv_asm.h>
> > > +#include <sbi/sbi_console.h>
> > > #include <sbi/sbi_domain.h>
> > > #include <sbi/sbi_hartmask.h>
> > > #include <sbi/sbi_hsm.h>
> > > @@ -240,6 +241,99 @@ static int sanitize_domain(const struct sbi_platform
> > *plat,
> > > return 0;
> > > }
> > >
> > > +void sbi_domain_dump(const struct sbi_domain *dom, const char
> > > +*suffix) {
> > > + u32 i, k;
> > > + unsigned long rstart, rend;
> > > + struct sbi_domain_memregion *reg;
> > > +
> > > + sbi_printf("Domain%d Name %s: %s\n",
> > > + dom->index, suffix, dom->name);
> > > +
> > > + sbi_printf("Domain%d Boot HART %s: %d\n",
> > > + dom->index, suffix, dom->boot_hartid);
> > > +
> > > + k = 0;
> > > + sbi_printf("Domain%d HARTs %s: ", dom->index, suffix);
> > > + sbi_hartmask_for_each_hart(i, dom->possible_harts)
> > > + sbi_printf("%s%d%s", (k++) ? "," : "",
> > > + i, sbi_domain_is_assigned_hart(dom, i) ? "*" : "");
> > > + sbi_printf("\n");
> > > +
> > > + i = 0;
> > > + sbi_domain_for_each_memregion(dom, reg) {
> > > + rstart = reg->base;
> > > + rend = (reg->order < __riscv_xlen) ?
> > > + rstart + ((1UL << reg->order) - 1) : -1UL;
> > > +
> > > +#if __riscv_xlen == 32
> > > + sbi_printf("Domain%d Region%02d %s: 0x%08lx-0x%08lx ",
> > > +#else
> > > + sbi_printf("Domain%d Region%02d %s: 0x%016lx-0x%016lx ",
> > > +#endif
> > > + dom->index, i, suffix, rstart, rend);
> > > +
> > > + k = 0;
> > > + if (reg->flags & SBI_DOMAIN_MEMREGION_MMODE)
> > > + sbi_printf("%cM", (k++) ? ',' : '(');
> > > + if (reg->flags & SBI_DOMAIN_MEMREGION_MMIO)
> > > + sbi_printf("%cI", (k++) ? ',' : '(');
> > > + if (reg->flags & SBI_DOMAIN_MEMREGION_READABLE)
> > > + sbi_printf("%cR", (k++) ? ',' : '(');
> > > + if (reg->flags & SBI_DOMAIN_MEMREGION_WRITEABLE)
> > > + sbi_printf("%cW", (k++) ? ',' : '(');
> > > + if (reg->flags & SBI_DOMAIN_MEMREGION_EXECUTABLE)
> > > + sbi_printf("%cX", (k++) ? ',' : '(');
> > > + sbi_printf("%s\n", (k++) ? ")" : "()");
> > > +
> > > + i++;
> > > + }
> > > +
> > > +#if __riscv_xlen == 32
> > > + sbi_printf("Domain%d Next Address%s: 0x%08lx\n", #else
> > > + sbi_printf("Domain%d Next Address%s: 0x%016lx\n", #endif
> > > + dom->index, suffix, dom->next_addr);
> > > +
> > > +#if __riscv_xlen == 32
> > > + sbi_printf("Domain%d Next Arg1 %s: 0x%08lx\n",
> > > +#else
> > > + sbi_printf("Domain%d Next Arg1 %s: 0x%016lx\n",
> > > +#endif
> > > + dom->index, suffix, dom->next_arg1);
> > > +
> > > + sbi_printf("Domain%d Next Mode %s: ", dom->index, suffix);
> > > + switch (dom->next_mode) {
> > > + case PRV_M:
> > > + sbi_printf("M-mode\n");
> > > + break;
> > > + case PRV_S:
> > > + sbi_printf("S-mode\n");
> > > + break;
> > > + case PRV_U:
> > > + sbi_printf("U-mode\n");
> > > + break;
> > > + default:
> > > + sbi_printf("Unknown\n");
> > > + break;
> > > + };
> > > +
> > > + sbi_printf("Domain%d SysReset %s: %s\n",
> > > + dom->index, suffix, (dom->system_reset_allowed) ?
> > > +"yes" : "no"); }
> > > +
> > > +void sbi_domain_dump_all(const char *suffix) {
> > > + u32 i;
> > > + const struct sbi_domain *dom;
> > > +
> > > + sbi_domain_for_each(i, dom) {
> > > + sbi_domain_dump(dom, suffix);
> > > + sbi_printf("\n");
> > > + }
> > > +}
> > > +
> > > int sbi_domain_finalize(struct sbi_scratch *scratch, u32 cold_hartid)
> > > {
> > > int rc;
> > > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c index
> > > 1871a1e..ff18c9d 100644
> > > --- a/lib/sbi/sbi_hart.c
> > > +++ b/lib/sbi/sbi_hart.c
> > > @@ -123,18 +123,23 @@ static int delegate_traps(struct sbi_scratch
> > *scratch)
> > > return 0;
> > > }
> > >
> > > -void sbi_hart_delegation_dump(struct sbi_scratch *scratch)
> > > +void sbi_hart_delegation_dump(struct sbi_scratch *scratch,
> > > + const char *prefix, const char *suffix)
> > > {
> > > if (!misa_extension('S'))
> > > /* No delegation possible as mideleg does not exist*/
> > > return;
> > >
> > > #if __riscv_xlen == 32
> > > - sbi_printf("MIDELEG : 0x%08lx\n", csr_read(CSR_MIDELEG));
> > > - sbi_printf("MEDELEG : 0x%08lx\n", csr_read(CSR_MEDELEG));
> > > + sbi_printf("%sMIDELEG%s: 0x%08lx\n",
> > > + prefix, suffix, csr_read(CSR_MIDELEG));
> > > + sbi_printf("%sMEDELEG%s: 0x%08lx\n",
> > > + prefix, suffix, csr_read(CSR_MEDELEG));
> > > #else
> > > - sbi_printf("MIDELEG : 0x%016lx\n", csr_read(CSR_MIDELEG));
> > > - sbi_printf("MEDELEG : 0x%016lx\n", csr_read(CSR_MEDELEG));
> > > + sbi_printf("%sMIDELEG%s: 0x%016lx\n",
> > > + prefix, suffix, csr_read(CSR_MIDELEG));
> > > + sbi_printf("%sMEDELEG%s: 0x%016lx\n",
> > > + prefix, suffix, csr_read(CSR_MEDELEG));
> > > #endif
> > > }
> > >
> > > @@ -154,35 +159,6 @@ unsigned int sbi_hart_pmp_count(struct
> > sbi_scratch *scratch)
> > > return hfeatures->pmp_count;
> > > }
> > >
> > > -void sbi_hart_pmp_dump(struct sbi_scratch *scratch) -{
> > > - unsigned long prot, addr, size, log2size;
> > > - unsigned int i, pmp_count;
> > > -
> > > - pmp_count = sbi_hart_pmp_count(scratch);
> > > - for (i = 0; i < pmp_count; i++) {
> > > - pmp_get(i, &prot, &addr, &log2size);
> > > - if (!(prot & PMP_A))
> > > - continue;
> > > - size = (log2size < __riscv_xlen) ? 1UL << log2size : 0;
> > > -#if __riscv_xlen == 32
> > > - sbi_printf("PMP%d : 0x%08lx-0x%08lx (A",
> > > -#else
> > > - sbi_printf("PMP%d : 0x%016lx-0x%016lx (A",
> > > -#endif
> > > - i, addr, addr + size - 1);
> > > - if (prot & PMP_L)
> > > - sbi_printf(",L");
> > > - if (prot & PMP_R)
> > > - sbi_printf(",R");
> > > - if (prot & PMP_W)
> > > - sbi_printf(",W");
> > > - if (prot & PMP_X)
> > > - sbi_printf(",X");
> > > - sbi_printf(")\n");
> > > - }
> > > -}
> > > -
> > > int sbi_hart_pmp_configure(struct sbi_scratch *scratch) {
> > > struct sbi_domain_memregion *reg; diff --git
> > > a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c index 406cb3f..5151d36
> > > 100644
> > > --- a/lib/sbi/sbi_init.c
> > > +++ b/lib/sbi/sbi_init.c
> > > @@ -39,6 +39,7 @@ static void sbi_boot_prints(struct sbi_scratch
> > > *scratch, u32 hartid) {
> > > int xlen;
> > > char str[128];
> > > + const struct sbi_domain *dom = sbi_domain_thishart_ptr();
> > > const struct sbi_platform *plat = sbi_platform_ptr(scratch);
> > >
> > > #ifdef OPENSBI_VERSION_GIT
> > > @@ -64,27 +65,29 @@ static void sbi_boot_prints(struct sbi_scratch
> > *scratch, u32 hartid)
> > > sbi_printf("Platform HART Count : %u\n",
> > > sbi_platform_hart_count(plat));
> > >
> > > - /* Boot HART details */
> > > - sbi_printf("Boot HART ID : %u\n", hartid);
> > > - misa_string(xlen, str, sizeof(str));
> > > - sbi_printf("Boot HART ISA : %s\n", str);
> > > - sbi_hart_get_features_str(scratch, str, sizeof(str));
> > > - sbi_printf("BOOT HART Features : %s\n", str);
> > > - sbi_printf("BOOT HART PMP Count : %d\n",
> > sbi_hart_pmp_count(scratch));
> > > - sbi_printf("BOOT HART MHPM Count: %d\n",
> > sbi_hart_mhpm_count(scratch));
> > > -
> > > /* Firmware details */
> > > sbi_printf("Firmware Base : 0x%lx\n", scratch->fw_start);
> > > sbi_printf("Firmware Size : %d KB\n",
> > > (u32)(scratch->fw_size / 1024));
> > >
> > > - /* Generic details */
> > > + /* SBI details */
> > > sbi_printf("Runtime SBI Version : %d.%d\n",
> > > sbi_ecall_version_major(), sbi_ecall_version_minor());
> > > sbi_printf("\n");
> > >
> > > - sbi_hart_delegation_dump(scratch);
> > > - sbi_hart_pmp_dump(scratch);
> > > + /* Domain details */
> > > + sbi_domain_dump_all("");
> > > +
> > > + /* Boot HART details */
> > > + sbi_printf("Boot HART ID : %u\n", hartid);
> > > + sbi_printf("Boot HART Domain : %s\n", dom->name);
> > > + misa_string(xlen, str, sizeof(str));
> > > + sbi_printf("Boot HART ISA : %s\n", str);
> > > + sbi_hart_get_features_str(scratch, str, sizeof(str));
> > > + sbi_printf("Boot HART Features : %s\n", str);
> > > + sbi_printf("Boot HART PMP Count : %d\n",
> > sbi_hart_pmp_count(scratch));
> > > + sbi_printf("Boot HART MHPM Count: %d\n",
> > sbi_hart_mhpm_count(scratch));
> > > + sbi_hart_delegation_dump(scratch, "Boot HART ", " ");
> > > }
> > >
> > > static spinlock_t coldboot_lock = SPIN_LOCK_INITIALIZER;
> > > --
> > > 2.25.1
> > >
> > >
> > > --
> > > opensbi mailing list
> > > opensbi at lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/opensbi
> >
> >
> >
> > --
> > Regards,
> > Atish
>
> Regards,
> Anup
--
Regards,
Atish
More information about the opensbi
mailing list