On Mon, Jun 27, 2022 at 06:58:15PM +0530, Anup Patel wrote:

The rationale is that for M-mode the memory type is always defined by
PMA(s) so if supervisor-mode software overrides memory type using
Svpbmt then M-mode firmware will not see same contents as the
supervisor-mode software.

I get the intent now. But we may not want to prohibit that.

We may want to document that the SBI will access this memory using
the PMA attribute.

If the supervisor has accessed this same location using different cachability attribute than the 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 Svpbmt specification to prevent the loss of coherence and memory ordering.

This does not place a restriction but warns against the issue and
points to the right sequence if there is a legitimate reason to do it.

This should not be specific to this function but applicable to any
function that the SBI defines with a memory operand and so could be
stated more generally in the SBI specification.

regards
ved

On Mon, Jun 27, 2022 at 6:16 PM Ved Shanbhogue <ved@...> wrote:

On Mon, Jun 27, 2022 at 05:38:43PM +0530, Anup Patel wrote:

Also, a system might be partitioned into domains so a supervisor software
running in a particular domain can only pass addresses accessible in that
domain.

So domain is a new term which is not presently defined in the privileged
specification or in the SBI specification.

The term "domain" is not used in the proposal since it is OpenSBI specific.
My comment was mainly for Heinrich since he is already aware of OpenSBI
domains.

In general, other M-mode firmwares might also implement some kind of
system level partitioning.

I think I get what you may be stating here i.e., M-mode may configure/
reconfigure PMPs to restrict the physical addresses that are accessible to
the supervisor-mode. Perhaps a future Smpu extension may add to this mix.

To avoid inventing a new term, we could reword #1 to state that the physical
address must be accessible, as determined by the PMP and/or PMA rules, to the
supervisor mode software invoking this SBI function.

To further restrict this perhaps say "accessible to read"? As we may not want
execute-only to be allowed by this extension.

I mostly agree with your wording. Considering hypervisors and future memory
protection ISA extensions, let's avoid using terms PMP.

For #1, we can say:
The SBI implementation MUST check that the supervisor-mode software
is allowed to read the specified physical address range on the calling HART.

For OpenSBI, we have domain regions so OpenSBI will check the address
against regions of the domain assigned to calling HART.

Since "domain" is a concept that is not defined by either the privilege
specification or the SBI specification we may want to avoid using that
term (or add a definition).

The term "domain" was only in the context of OpenSBI. We should not use
this term in the SBI specification.

This is more a requirement on the supervisor-mode software because
we might have a system with Svpbmt so on such system supervisor-mode
should not pass an address which is mapped as non-cacheable in the
page table.

This is confusing since the description states that the parameter is a
physical address but the later comment states its a virtual address
and the memory type override using page-based memory type provided by
the virtual memory system is allowed. Could we clarify if its a physical
address - in which case only the coherence and cachability PMA should
provide the memory type - or a virtual address in which case page-based
memory type override is possible.

Further, a brief explanation about why cachability matters for this
extension would be useful. Is this trying to imply some kind of early
boot execution where caches may be placed in a special mode that allows
operation when main memory is not available.

I agree the #2 requirement is not clear enough.

For #2, we can say:
The supervisor-mode software MUST not override the memory type of
specified physical address range using page-based memory types.

The rationale is that for M-mode the memory type is always defined by
PMA(s) so if supervisor-mode software overrides memory type using
Svpbmt then M-mode firmware will not see same contents as the
supervisor-mode software.

Regards,
Anup

On Mon, Jun 27, 2022 at 05:38:43PM +0530, Anup Patel wrote:

Also, a system might be partitioned into domains so a supervisor software
running in a particular domain can only pass addresses accessible in that
domain.

So domain is a new term which is not presently defined in the privileged specification or in the SBI specification.

I think I get what you may be stating here i.e., M-mode may configure/
reconfigure PMPs to restrict the physical addresses that are accessible to the supervisor-mode. Perhaps a future Smpu extension may add to this mix.

To avoid inventing a new term, we could reword #1 to state that the physical address must be accessible, as determined by the PMP and/or PMA rules, to the
supervisor mode software invoking this SBI function.

To further restrict this perhaps say "accessible to read"? As we may not want execute-only to be allowed by this extension.

For OpenSBI, we have domain regions so OpenSBI will check the address
against regions of the domain assigned to calling HART.

Since "domain" is a concept that is not defined by either the privilege
specification or the SBI specification we may want to avoid using that
term (or add a definition).

This is more a requirement on the supervisor-mode software because
we might have a system with Svpbmt so on such system supervisor-mode
should not pass an address which is mapped as non-cacheable in the
page table.

This is confusing since the description states that the parameter is a
physical address but the later comment states its a virtual address
and the memory type override using page-based memory type provided by
the virtual memory system is allowed. Could we clarify if its a physical
address - in which case only the coherence and cachability PMA should provide the memory type - or a virtual address in which case page-based memory type override is possible.

Further, a brief explanation about why cachability matters for this extension would be useful. Is this trying to imply some kind of early
boot execution where caches may be placed in a special mode that allows
operation when main memory is not available.

regards
ved

On 6/27/22 14:08, Anup Patel wrote:

On Mon, Jun 27, 2022 at 4:45 PM Heinrich Schuchardt <xypron.glpk@...> wrote:

On 6/27/22 10:46, 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 short, the SBI debug console extension defines a standard way
for boot-time early prints in supervisor-mode software.

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

Why should only S-mode and not HS-mode be allowed? I think the modes
that are allowed to access this extension should be enumerated more clearly.

The term "supervisor-mode" over here means:
* HS-mode or VS-mode on systems with H-extension
* S-mode on systems without H-extension
(Please see the introduction chapter of SBI specification)
In fact, both HS-mode and VS-mode are actually supervisor-mode with
different capabilities.

How are systems treated that only have M-mode and U-mode?

For systems with only M-mode and U-mode, there is no supervisor-mode
hence there is no SBI on such systems. These are embedded-class
systems which usually run some kind of RTOS.

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.0
(or higher).
2) It allows supervisor software to print multiple characters
in a single SBI call.

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 addr_div_by_4,
unsigned long num_chars)

Print the string specified by the `addr_div_by_4` and `num_chars`
parameters on the debug console. The `addr_div_by_4` parameter is
the physical base address of the string right shifted by 2 whereas
the `num_chars` parameter is the number of characters (or bytes) in
the string.

The memory pointed the `addr_div_by_4` and `num_chars` parameters
must be:
1) Accessible to supervisor-mode
2) Cacheable and writable memory

Accessibility by supervisor-mode is required due to security
considerations: You should not be able to print out M-mode's secrets via
S-mode software. This should be clearly stated.

The point#1 tries to say this.
Also, a system might be partitioned into domains so a supervisor software
running in a particular domain can only pass addresses accessible in that
domain.

MUST an SBI implementation check this feature? Probably yes. How shall
it be checked?

It's up to the SBI implementation how to check this feature.

As this is a security feature we should require:

The SBI MUST check that the full memory range is read-accessible by the caller.

For OpenSBI, we have domain regions so OpenSBI will check the address
against regions of the domain assigned to calling HART.
For KVM, the user-space tool (QEMU/KVMTOO) knows the guest physical
address range of Guest RAM which can be used to validate the address.

The SBI is not meant to change the string and therefore needs no write
access. There is no reason to require that the memory should be
writable. The string could reside in NOR flash.

Caching may speed up the SBI code. But why should we require this
specific memory region being cached?

This is more a requirement on the supervisor-mode software because
we might have a system with Svpbmt so on such system supervisor-mode
should not pass an address which is mapped as non-cacheable in the
page table.
We can certainly re-word this requirement to:
"Cacheable and readable memory"

Here too we should make it clear if the SBI MUST or MAY check the property. I guess MAY is enough.

Best regards

Heinrich

One of the replies to your v1 patch requested to have the same
capabilities as putchar(): just pass a single character in a register.
Should a second FID be added?

It's pretty easy to print a single character using the proposed puts()
call.
For assembly source perspective, this will be just one extra instruction.
Regards,
Anup

Best regards

Heinrich

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

Errors:
SBI_SUCCESS - Characters printed successfully.
SBI_ERR_INVALID_ADDRESS - The string pointed by `addr_div_by_4`
and `num_chars` parameters is either not
accessible to supervisor-mode or does not
point to cacheable and writable memory.
SBI_ERR_FAILED - Failed to print characters due to
I/O errors.

On Mon, Jun 27, 2022 at 4:45 PM Heinrich Schuchardt <xypron.glpk@...> wrote:

On 6/27/22 10:46, 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 short, the SBI debug console extension defines a standard way
for boot-time early prints in supervisor-mode software.

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

Why should only S-mode and not HS-mode be allowed? I think the modes
that are allowed to access this extension should be enumerated more clearly.

The term "supervisor-mode" over here means:
* HS-mode or VS-mode on systems with H-extension
* S-mode on systems without H-extension
(Please see the introduction chapter of SBI specification)

In fact, both HS-mode and VS-mode are actually supervisor-mode with
different capabilities.

How are systems treated that only have M-mode and U-mode?

For systems with only M-mode and U-mode, there is no supervisor-mode
hence there is no SBI on such systems. These are embedded-class
systems which usually run some kind of RTOS.

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.0
(or higher).
2) It allows supervisor software to print multiple characters
in a single SBI call.

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 addr_div_by_4,
unsigned long num_chars)

Print the string specified by the `addr_div_by_4` and `num_chars`
parameters on the debug console. The `addr_div_by_4` parameter is
the physical base address of the string right shifted by 2 whereas
the `num_chars` parameter is the number of characters (or bytes) in
the string.

The memory pointed the `addr_div_by_4` and `num_chars` parameters
must be:
1) Accessible to supervisor-mode
2) Cacheable and writable memory

Accessibility by supervisor-mode is required due to security
considerations: You should not be able to print out M-mode's secrets via
S-mode software. This should be clearly stated.

The point#1 tries to say this.

Also, a system might be partitioned into domains so a supervisor software
running in a particular domain can only pass addresses accessible in that
domain.

MUST an SBI implementation check this feature? Probably yes. How shall
it be checked?

It's up to the SBI implementation how to check this feature.

For OpenSBI, we have domain regions so OpenSBI will check the address
against regions of the domain assigned to calling HART.

For KVM, the user-space tool (QEMU/KVMTOO) knows the guest physical
address range of Guest RAM which can be used to validate the address.

The SBI is not meant to change the string and therefore needs no write
access. There is no reason to require that the memory should be
writable. The string could reside in NOR flash.

Caching may speed up the SBI code. But why should we require this
specific memory region being cached?

This is more a requirement on the supervisor-mode software because
we might have a system with Svpbmt so on such system supervisor-mode
should not pass an address which is mapped as non-cacheable in the
page table.

We can certainly re-word this requirement to:
"Cacheable and readable memory"

One of the replies to your v1 patch requested to have the same
capabilities as putchar(): just pass a single character in a register.
Should a second FID be added?

It's pretty easy to print a single character using the proposed puts()
call.

For assembly source perspective, this will be just one extra instruction.

Regards,
Anup

Best regards

Heinrich

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

Errors:
SBI_SUCCESS - Characters printed successfully.
SBI_ERR_INVALID_ADDRESS - The string pointed by `addr_div_by_4`
and `num_chars` parameters is either not
accessible to supervisor-mode or does not
point to cacheable and writable memory.
SBI_ERR_FAILED - Failed to print characters due to
I/O errors.

On 6/27/22 10:46, 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 short, the SBI debug console extension defines a standard way
for boot-time early prints in supervisor-mode software.

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

Why should only S-mode and not HS-mode be allowed? I think the modes
that are allowed to access this extension should be enumerated more clearly.

How are systems treated that only have M-mode and U-mode?

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.0
(or higher).
2) It allows supervisor software to print multiple characters
in a single SBI call.

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 addr_div_by_4,
unsigned long num_chars)

Print the string specified by the `addr_div_by_4` and `num_chars`
parameters on the debug console. The `addr_div_by_4` parameter is
the physical base address of the string right shifted by 2 whereas
the `num_chars` parameter is the number of characters (or bytes) in
the string.

The memory pointed the `addr_div_by_4` and `num_chars` parameters
must be:
1) Accessible to supervisor-mode
2) Cacheable and writable memory

Accessibility by supervisor-mode is required due to security
considerations: You should not be able to print out M-mode's secrets via
S-mode software. This should be clearly stated.

MUST an SBI implementation check this feature? Probably yes. How shall
it be checked?

The SBI is not meant to change the string and therefore needs no write
access. There is no reason to require that the memory should be
writable. The string could reside in NOR flash.

Caching may speed up the SBI code. But why should we require this
specific memory region being cached?

One of the replies to your v1 patch requested to have the same
capabilities as putchar(): just pass a single character in a register.
Should a second FID be added?

Best regards

Heinrich

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

Errors:
SBI_SUCCESS - Characters printed successfully.
SBI_ERR_INVALID_ADDRESS - The string pointed by `addr_div_by_4`
and `num_chars` parameters is either not
accessible to supervisor-mode or does not
point to cacheable and writable memory.
SBI_ERR_FAILED - Failed to print characters due to
I/O errors.

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 short, the SBI debug console extension defines a standard way
for boot-time early prints in supervisor-mode software.

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 legacy console putchar (EID #0x01) extension
and it is better in following ways:
1) It follows the new calling convention defined for SBI v1.0
(or higher).
2) It allows supervisor software to print multiple characters
in a single SBI call.

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 addr_div_by_4,
unsigned long num_chars)

Print the string specified by the `addr_div_by_4` and `num_chars`
parameters on the debug console. The `addr_div_by_4` parameter is
the physical base address of the string right shifted by 2 whereas
the `num_chars` parameter is the number of characters (or bytes) in
the string.

The memory pointed the `addr_div_by_4` and `num_chars` parameters
must be:
1) Accessible to supervisor-mode
2) Cacheable and writable memory

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

Errors:
SBI_SUCCESS - Characters printed successfully.
SBI_ERR_INVALID_ADDRESS - The string pointed by `addr_div_by_4`
and `num_chars` parameters is either not
accessible to supervisor-mode or does not
point to cacheable and writable memory.
SBI_ERR_FAILED - Failed to print characters due to
I/O errors.