On Fri, Jun 3, 2022 at 12:23 AM Heinrich Schuchardt
On 6/3/22 08:56, Atish Kumar Patra wrote:
On Thu, Jun 2, 2022 at 2:13 AM Heinrich SchuchardtI have no security concerns. My thought was that it might be better to
I would like to understand more about the security concerns you raised.
On 6/2/22 10:44, Anup Patel wrote:
On Wed, Jun 1, 2022 at 11:59 PM Heinrich SchuchardtCurrently this function is just a nop(). It is not needed in this
Usually, the serial port related code in M-mode firmware only uses
On 6/1/22 18:17, Anup Patel wrote:
Hi All,Thanks for starting to close this gap.
Below is the draft proposal for SBI Debug Console Extension.
Please review it and provide feedback.
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 legacy console putchar (EID #0x01) extension
and it is better in following ways:
1) It follows the new calling convention defined for SBI v1.0I miss a discussion of the conflicts that can arise if the configuration
2) It is based on a shared memory area between SBI implementation
and supervisor-mode software so multiple characters can be
printed using a single SBI call.
of the serial console is changed by the caller.
Do we need an ecall that closes the SBI console to further access?
status and data registers so for most serial ports support the M-mode
firmware will adapt to serial port configuration changes.
In fact, this is why we never had a special SBI call for serial port
reconfiguration in legacy SBI v0.1 as well.
In case of virtualization, the serial port (or console) is emulated so
the special SBI call is not useful for virtualization.
Actually, it's the S-mode software which is voluntarily giving a portion of
Isn't it M-mode software that has to program the MMU to allow all harts
The supervisor-mode software must set the shared memory area before
printing characters on the debug console. Also, all HARTs share the
same shared memory area so only one HART needs to set it at boot-time.
in M-mode and S-mode access to the memory area? What is the S-mode
software to do about the memory area prior to calling
its memory to be used as shared memory. The proposal only mandates
that whatever memory is selected by S-mode software should be a
regular cacheable memory (not IO memory). Also, if Svpbmt is available
then S-mode software should only use memory type 0 in the PTEs.
Theoretically, we can avoid the extra function to set shared memory area
The console output is needed on the very start of the S-mode software,
Function: Set Console Area (FID #0)
struct sbiret sbi_debug_console_set_area(unsigned long addr_div_by_4,
unsigned long size)
before setting up anything.
Can we avoid this extra function?
Can we simply assume that the caller of sbi_debug_console_puts()
provides a physical address pointer to a memory area that is suitable?
but it will complicate things in future when we have supervisor software
encrypting it's own memory (using special ISA support) because in this
case supervisor software will have to unprotect memory every time the
sbi_debug_console_puts() is called and protect it again after the call.
revision of the extension.
The function might be called repeatedly by different threads with
different values. How do you want to keep track of all of these
Memory shared between different security realms will arise in many
different scenarios. As this is not console specific it should be in a
separate extension. That extension should be defined once we have
clarity about how security realms are managed.
Can you please elaborate on this ?
manage shared memory in a separate extension for all use cases.
Ahh I see. Sorry I misunderstood your comment.
The designated shared memory for debug console is shared across
the platforms while STA or PMU use case will have per cpu shared memory.
I agree that all the concerns related to shared memory can be discussed
together and all the common constraints can be described at one place.
But defining a separate extension for the shared memory may be problematic
as we need to manage the extension dependencies.
As you pointed out, there are other possible use cases(e.g STA, PMU)
for shared memory between
S & M mode or VS & HS mode. It would be good to address these concerns right now
instead of discussing those in each individual extension context.
sbi_debug_console_set_area() might be called with different values by
Okay, I will update.
Set the shared memory area specified by `addr_div_by_2` and `size`
Okay, I will update.
parameters. The `addr_div_by_4` parameter is base address of the%s/is base/is the base/
For RV32 S-mode, the physical address space is 34bits wide but
shared memory area right shifted by 2 whereas `size` parameter isWhy shifting the address? I would prefer to keep it simple for the
the size of shared memory area in bytes.
caller. If the alignment is not suitable, return an error.
But why is an alignment needed here at all? And why 4 aligned?
"unsigned long" is 32bits wide. This is because Sv32 PTEs allow
34bits of PPN. In fact, even instructions such as HFENCE.GVMA
have this "address right shift by 2" requirement based on this rationale.
I would prefer to simply pass a physical address pointer here with no
The shared memory area should be normal cacheable memory for the
supervisor-mode software. Also, the shared memory area is global
across all HARTs so SBI implementation must ensure atomicity in
setting the shared memory area.
SBI_SUCCESS - Shared memory area set successfully.
SBI_ERR_INVALID_ADDRESS - The shared memory area pointed by
`addr_div_by_2` and `size` parameters
is not normal cacheable memory or not
accessible to supervisor-mode software.
Function: Console Puts (FID #1)
struct sbiret sbi_debug_console_puts(unsigned long area_offset,
unsigned long num_chars)
requirements on alignment. And no prior SBI call.
different threads. An offset is ambiguous as it does not define to which
of the different shared areas it relates. Please, use a pointer.
Bare-metal tests (or assembly sources) can print sub-strings from
Do we need num_chars? Are we expecting to provide binary output? Using
0x00 as terminator would be adequate in most cases.
a large per-populated string in shared memory. Assuming that string
is always terminated by 0x00 in sbi_debug_console_puts() will break
The platform specification would be the right place to require 8-bit
We need to clarify this. Suggestions ?
What is the requirement on the console? Does it have to support 8bit
output to allow for UTF-8?
support for the console.
We should add to the platform specification that UTF-8 output is assumed
Same as above, this needs more clarification. Suggestions ?
Do we make any assumptions about encoding?
on the serial console.
We should state if extra bits are ignored or the bytes are not send.
I am of the opinion to keep such encoding related assumptions to be
I would consider this to be part of the clarification we add for encoding.
How would we handle a console set up to 7bit + parity if a character >
0x7f is sent?
The easiest thing is to just ignore the extra bits. So let't state this
Okay, I will update.
Print the string specified by `area_offset` and `num_chars` on
the debug console. The `area_offset` parameter is the start of
string in the shard memory area whereas `num_chars` parameter
Okay, I will add.
is the number of characters (or bytes) in the string.There could be other reasons of failures:
This is a blocking SBI call and will only return after printing
all characters of the string.
SBI_SUCCESS - Characters printed successfully.
SBI_ERR_INVALID_ADDRESS - The start of the string (i.e.
`area_offset`) or end of the string
(i.e. `area_offset + num_chars`) is
outside shared memory area.
* set up of the UART failed in OpenSBI
* no UART defined in the device-tree
So let us add SBI_ERR_FAILED to the list.