On Thu, Nov 24, 2022 at 10:14 PM Mitchell Horne <mhorne@...> wrote:

Hi Anup,

Thank you for this proposal, I am happy to see that the legacy console
extension is getting a replacement, as I've found it tremendously useful
for early debugging in the past.

I have a couple of small questions inline below, but otherwise this
looks easy to work with.

Cheers,
Mitchell

On 8/3/22 13:07, Anup Patel wrote:

Hi All,

Based on feedback, below is the updated draft proposal of the
SBI Debug Console Extension ...

The motivations behind this proposal is as follows:
1) There is no new SBI extension replacing the legacy SBI v0.1
putchar() and getchar() calls. These legacy SBI v0.1 calls are
now disabled by default in Linux and will be eventually deprecated.
We need a replacement at least for the SBI v0.1 putchar() which
is widely used for early prints in various OSes and Bootloaders.
2) The OS-A Platforms (or SEE) specification need to mandate
a particular type of UART (8250, ns16550, or SiFive) as a standard
way for boot-time early prints in supervisor-mode software. Using
SBI debug console extension, the OS-A Platforms (or SEE)
specifications don't need to mandate any type of UART.
3) The legacy SBI v0.1 putchar() only supports printing one
character at a time. For some of the hypervisors (particularly KVM),
SBI based early prints (i.e. earlycon=sbi) is slow because each
SBI v0.1 putchar() trap will be forwarded to the KVM user-space
(KVMTOOL or QEMU).

In other words, the SBI debug console extension defines a standard
way for boot-time early prints in supervisor-mode software.

We have completed the proof-of-concept implementation of this
SBI extension proposal. Refer, "riscv_sbi_dbcn_v1" branch in the
following repos:
https://github.com/avpatel/opensbi.git
https://github.com/avpatel/linux.git
https://github.com/avpatel/kvmtool.git

Please review it and provide your feedback.

Regards,
Anup

SBI Debug Console Extension (EID #0x4442434E "DBCN")
====================================================

The debug console extension defines a generic mechanism for boot
time early prints from supervisor-mode software which allows users
to catch boot-time issues in supervisor-mode software.

This extension replaces the legacy console putchar (EID #0x01)
extension with following improvements:
1) It follows the new calling convention defined by the SBI v1.0
(or higher) specification.
2) It allows supervisor software to print multiple characters
in a single SBI call.

Some of the functions defined by this SBI extension have physical
memory as parameters for input and/or output. This physical memory
parameter MUST satisfy following requirements:
1) The SBI implementation MUST check that the supervisor-mode
software is allowed to read and write the specified physical
memory on the calling HART.
2) The SBI implementation MUST access the specified physical
memory using the PMA attribute.
Note: If the supervisor-mode software access the same physical
memory using memory type different from PMA then a loss of
coherence or unexpected memory ordering may occur and the
invoking software should follow the rules and sequences defined
in the RISC-V Svpbmt specification to prevent the loss of
coherence and memory ordering.

If the underlying physical console has extra bits for error checking
(or correction) then these extra bits should be handled by the SBI
implementation.

Function: Console Puts (FID #0)
-------------------------------

struct sbiret sbi_debug_console_puts(unsigned long num_chars,
uint64_t base_addr)

Is there a reason that base_addr is the second argument and not the
first? To me, it is much more natural to have the string's address be
the first argument, as num_chars only really has meaning in relation to
this string.

The uint64_t needs two registers on RV32 whereas it only needs one
register on RV64. Now if base_addr is the first parameter then for:
1) RV32
a0 = lower 32bits of base address
a1 = upper 32bits of base address
a2 = num characters
2) RV64
a0 = 64bits of base address
a1 = num characters

As we can see, "a1" register has different semantics for RV32 vs RV64
when base_addr is first parameter whereas if base_addr was second
parameter then "a2" register has upper 32bits of base address on RV32
and it is unused on RV64.

Considering, this SBI call is going to be used from assembly sources
as well, it seemed simpler/easy-to-use having "base_addr" as second
parameter.

Also, is there a reason that base_addr is a uint64_t and not an unsigned
long? Other physical addresses accepted by SBI interface are defined to
be unsigned long, e.g. start_addr in sbi_hart_start() or
sbi_remote_hfence_gvma_vmid().

If we use "unsigned long" for physical address then we will have to define
it as "base_addr_div_by_4" because RV32 systems can have 34 bits of
physical address space (as-per Sv32 MMU mode). This also means we
will be enforcing a 4-byte alignment on base_address of string to be
printed whereas strings (char []) are typically byte aligned so callers
might have to use an intermediate aligned buffer in this case which is
inconvenient.

In case of sbi_hart_start(), the "start_addr" is actually a execution address
with MMU off so this will be 32bit on RV32 and 64bit on RV64 hence the
"start_addr" is "unsigned long".

In case of sbi_remote_hfence_gvma_vmid() and sbi_remote_hfence_gvma(),
the "start_addr" parameter should have been "start_addr_div_by_4" to be
suitable for both RV32 and RV64 but by the time we realize this it was too
late. We will need to define sbi_remote_hfence_gvma_vmid2() and
sbi_remote_hfence_gvma2() to make SBI RFENCE extension suitable
for both RV32 and RV64.

Maybe these questions have been considered in previous versions of the
proposal; I have not followed closely. You may disregard them as
appropriate.

Yes, we did receive similar questions in previous versions of the proposal
but I am happy to provide the explanation again.

Print a specified input string on the debug console.

The `num_chars` parameter specifies the number of characters (or
bytes) in the input string whereas `base_addr` parameter specifies
the physical base address of the input string.

This is a blocking SBI call and will only return after printing all
characters of the input string.

Errors:
SBI_SUCCESS - Characters printed successfully.
SBI_ERR_INVALID_ADDRESS - The string pointed by `num_chars` and `base_addr`
parameters is not entirely accessible to
supervisor-mode.
SBI_ERR_FAILED - Failed to print characters due to I/O errors.

Regards,
Anup

Hi Anup,

Thank you for this proposal, I am happy to see that the legacy console extension is getting a replacement, as I've found it tremendously useful for early debugging in the past.

I have a couple of small questions inline below, but otherwise this looks easy to work with.

Cheers,
Mitchell

On 8/3/22 13:07, Anup Patel wrote:

Hi All,
Based on feedback, below is the updated draft proposal of the
SBI Debug Console Extension ...
The motivations behind this proposal is as follows:
1) There is no new SBI extension replacing the legacy SBI v0.1
putchar() and getchar() calls. These legacy SBI v0.1 calls are
now disabled by default in Linux and will be eventually deprecated.
We need a replacement at least for the SBI v0.1 putchar() which
is widely used for early prints in various OSes and Bootloaders.
2) The OS-A Platforms (or SEE) specification need to mandate
a particular type of UART (8250, ns16550, or SiFive) as a standard
way for boot-time early prints in supervisor-mode software. Using
SBI debug console extension, the OS-A Platforms (or SEE)
specifications don't need to mandate any type of UART.
3) The legacy SBI v0.1 putchar() only supports printing one
character at a time. For some of the hypervisors (particularly KVM),
SBI based early prints (i.e. earlycon=sbi) is slow because each
SBI v0.1 putchar() trap will be forwarded to the KVM user-space
(KVMTOOL or QEMU).
In other words, the SBI debug console extension defines a standard
way for boot-time early prints in supervisor-mode software.
We have completed the proof-of-concept implementation of this
SBI extension proposal. Refer, "riscv_sbi_dbcn_v1" branch in the
following repos:
https://github.com/avpatel/opensbi.git
https://github.com/avpatel/linux.git
https://github.com/avpatel/kvmtool.git
Please review it and provide your feedback.
Regards,
Anup
SBI Debug Console Extension (EID #0x4442434E "DBCN")
====================================================
The debug console extension defines a generic mechanism for boot
time early prints from supervisor-mode software which allows users
to catch boot-time issues in supervisor-mode software.
This extension replaces the legacy console putchar (EID #0x01)
extension with following improvements:
1) It follows the new calling convention defined by the SBI v1.0
(or higher) specification.
2) It allows supervisor software to print multiple characters
in a single SBI call.
Some of the functions defined by this SBI extension have physical
memory as parameters for input and/or output. This physical memory
parameter MUST satisfy following requirements:
1) The SBI implementation MUST check that the supervisor-mode
software is allowed to read and write the specified physical
memory on the calling HART.
2) The SBI implementation MUST access the specified physical
memory using the PMA attribute.
Note: If the supervisor-mode software access the same physical
memory using memory type different from PMA then a loss of
coherence or unexpected memory ordering may occur and the
invoking software should follow the rules and sequences defined
in the RISC-V Svpbmt specification to prevent the loss of
coherence and memory ordering.
If the underlying physical console has extra bits for error checking
(or correction) then these extra bits should be handled by the SBI
implementation.
Function: Console Puts (FID #0)
-------------------------------
struct sbiret sbi_debug_console_puts(unsigned long num_chars,
uint64_t base_addr)

Is there a reason that base_addr is the second argument and not the first? To me, it is much more natural to have the string's address be the first argument, as num_chars only really has meaning in relation to this string.

Also, is there a reason that base_addr is a uint64_t and not an unsigned long? Other physical addresses accepted by SBI interface are defined to be unsigned long, e.g. start_addr in sbi_hart_start() or sbi_remote_hfence_gvma_vmid().

Maybe these questions have been considered in previous versions of the proposal; I have not followed closely. You may disregard them as appropriate.

Print a specified input string on the debug console.
The `num_chars` parameter specifies the number of characters (or
bytes) in the input string whereas `base_addr` parameter specifies
the physical base address of the input string.
This is a blocking SBI call and will only return after printing all
characters of the input string.
Errors:
SBI_SUCCESS - Characters printed successfully.
SBI_ERR_INVALID_ADDRESS - The string pointed by `num_chars` and `base_addr`
parameters is not entirely accessible to
supervisor-mode.
SBI_ERR_FAILED - Failed to print characters due to I/O errors.

Thanks Anup - I signed up to that list as well (and thanks for clarifying it's purpose w.r.t. this group).

Hi Andrei,

On Mon, Oct 10, 2022 at 10:42 PM Andrei Warkentin
<andrei.warkentin@...> wrote:

Dear all,



I just wanted to introduce myself and say ‘hi’. I recently joined the Intel SSE group as an engineer focusing on RISC-V standardization (in particular with an interest in collaborating on the OS-A spec, and its touchpoints with SBI, UEFI, ACPI, etc). Some of you may remember me in my past life participating in a similar vein in SystemReady activities. I’m excited to be here and look forward to a fruitful collaboration. I think it’s amazing how much has been accomplished in such a short amount of time. Kudos!

We now have a separate PRS TG (Platform Runtime Services) for yearly
updates to SBI, UEFI, and ACPI specification.

Regards,
Anup

I poked around the mailing list server but wasn’t able to find a clear answer… Does this group have any kind of periodical group sync-ups I should add to my calendar?



A