Re: [PATCH v10] sgx.7: New page with overview of Software Guard eXtensions (SGX)

On Wed, 2021-12-08 at 14:11 -0800, Reinette Chatre wrote:

Hi Jarkko,

On 11/30/2021 9:50 AM, Jarkko Sakkinen wrote:

quoted

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

...

quoted

+.TH SGX 7 2021\-02\-02 "Linux" "Linux Programmer's Manual"
+.PP
+sgx - overview of Software Guard eXtensions

Is the "overview of" text necessary?

It's more or less a convention:

$ git grep "overview of" man7 | wc -l
29

quoted

+.SH SYNOPSIS
+.EX
+.B #include <asm/sgx.h>
+.PP
+.IB enclave " = open(""/dev/sgx_enclave", " O_RDWR);"

I view the man page output using "man -l man7/sgx.7" and when I do so 
the above line is unbalanced: "enclave" and (unexpectedly) the comma are 
underlined and the line is displayed with a single instance of a double 
quote: enclave = open("/dev/sgx_enclave, O_RDWR);

After some trial and error, and looking at symlink.7, this seems to
fix it:

-.IB enclave " = open(""/dev/sgx_enclave", " O_RDWR);"
+.IB enclave " = open(""/dev/sgx_enclave"", O_RDWR);"

Does this fix for you?

quoted

+.EE
+.SH DESCRIPTION
+Intel Software Guard eXtensions (SGX) allow applications to host
+enclaves,
+protected executable objects in memory.
+.PP
+Enclaves are blobs of executable code,
+running inside a CPU enforced container,
+which is mapped to the process address space.
+They are represented as the instances of
+.I /dev/sgx_enclave.

.IR /dev/sgx_enclave .

quoted

+They have a fixed set of entry points,
+defined when the enclave is built.
+.PP
+SGX can be only available if the kernel is configured and built with the

can be only -> can only be?

Agreed, I'll fix this.

quoted

+.B CONFIG_X86_SGX
+option.
+If CPU, BIOS and kernel have SGX enabled,
+.I sgx
+appears in the
+.I flags
+field of
+.IR /proc/cpuinfo .
+.PP
+If SGX appears not to be available,
+ensure that SGX is enabled in the BIOS.
+If a BIOS presents a choice between
+.I Enabled
+and
+.I Software Enabled
+modes for SGX,
+choose
+.I Enabled.

Earlier there is the underlined "/proc/cpuinfo" text followed by a 
period and here the "Enabled" text is followed by a period. In this 
instance the period is also underlined while previously it is not. 
Looking at some other man pages it seems that the custom is that the 
period should not be underlined and I will continue to point out 
instances I noticed where this is not the case.

This most related to my very weak understanding of troff syntax.

I changed it to:

-.I Enabled.
+.IR Enabled .

This does seem to fix the issue, and aligns with this:

https://www.gnu.org/software/groff/manual/html_node/Man-font-macros.html

quoted

+.PP
+.SS Memory mapping
+The file descriptor for an enclave can be shared among multiple processes.
+An enclave is required by the CPU to be placed to an address,
+which is a multiple of its size.
+An address range containing a reasonable base address can be probed with an anonymous
+.BR mmap(2)
+call:
+.PP
+.EX
+void *area = mmap(NULL, size * 2, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
+                  -1, 0);
+
+void *base = ((uint64_t)area + size - 1) & ~(size - 1);
+.EE
+.PP
+The enclave file descriptor itself can be then mapped with the
+.BR MAP_FIXED
+flag set to the carved out memory.
+.SS Construction
+An enclave instance is created by opening
+.I /dev/sgx_enclave.

Underlined period above.

Thanks. I spotted similar error also early in the text.

quoted

+Its contents are populated with the
+.BR ioctl (2)
+interface in
+.IR <asm/sgx.h>:

Here also, should the above colon perhaps not be underlined?

Yeah, similarly:

.IR <asm/sgx.h>:
+.IR <asm/sgx.h> :

quoted

+.TP
+.IB SGX_IOC_ENCLAVE_CREATE
+Create SGX Enclave Control Structure (SECS) for the enclave.
+SECS is a hardware defined structure,
+which contains the global properties of an enclave.
+.IB SGX_IOC_ENCLAVE_CREATE
+is a one-shot call that fixes enclave's address and

fixes enclave's -> fixes the enclave's ?

Ack.

quoted

+size for the rest of its life-cycle.
+.TP
+.IB SGX_IOC_ENCLAVE_ADD_PAGES
+Fill a range of the enclaves pages with the caller provided data and protection bits.

Should this be "the enclave's pages"?

I think so.

quoted

+Memory mappings of the enclave can only have protection bits set,
+which are defined in this ioctl.

This is a bit hard to understand. How about "Memory mappings of the 
enclave can only set protection bits that are defined in this ioctl."

Changed, thanks.

quoted

+The pages added are either regular memory pages for code and data

regular memory pages -> regular pages?

Changed.

,

quoted

+or thread control structures (TCS).
+The latter define the entry points to the enclave,
+which can be entered after the initialization.
+.TP
+.IB SGX_IOC_ENCLAVE_INIT
+Initialize the enclave for the run-time.
+After a successful initialization,
+no new pages can be added to the enclave.
+.SS Invocation
+Thread control structure (TCS) page are the entry points to the enclave,

page are -> pages are ?

Thanks, good catch.

quoted

+which further define an offset inside the enclave where the execution begins.
+The entry points are invoked with
+.I __vdso_sgx_enter_enclave.

Underlined period above.

.I __vdso_sgx_enter_enclave.
+.IR __vdso_sgx_enter_enclave .

quoted

+The prototype for the vDSO is defined by
+.BR vdso_sgx_enter_enclave_t
+in
+.IR <asm/sgx.h>.

Same above with the underlining of "."

quoted

+.SS Permissions
+.PP
+During the build process each enclave page is assigned protection bits,
+as part of
+.BR ioctl(SGX_IOC_ENCLAVE_ADD_PAGES).
+These protections are also the maximum protections to which the page can be be mapped.

to which -> with which ?

Ack.

quoted

+If
+.BR mmap (2)_

Unexpected "_" above

Thanks.

quoted

+is called with higher protections than those defined during the build,
+it will return
+.B -EACCES.
+If
+.BR ioctl(SGX_IOC_ENCLAVE_ADD_PAGES)
+is called after
+.BR mmap (2)
+with lower protections,
+the caller receives
+.BR SIGBUS,
+once it accesses the page for the first time.
+.SH VERSIONS
+The SGX feature was added in Linux 5.11.

This does not document the SGX_IOC_VEPC_REMOVE ioctl that was added in 
v5.16. How do you envision additions to this page as new features are 
added to the Linux support of SGX?

I started this before any of KVM stuff was in upstream. It'd be better
to get the basic ioctl's done first. I cannot really give estimate for
vepc at this point.

For future features (e.g. SGX2), the expectation is that the feature is
supported by an associated man page update.

quoted

+.SH SEE ALSO
+.BR ioctl (2),
+.BR mmap (2),
+.BR mprotect (2)
\ No newline at end of file

Reinette

Thanks for valuable feedback.

/Jarkko