On Wed, 2021-12-08 at 14:11 -0800, Reinette Chatre wrote:
> Hi Jarkko,
>
> On 11/30/2021 9:50 AM, Jarkko Sakkinen wrote:
> > Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
>
> ...
>
> > +.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
> > +.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?
> > +.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 .
> > +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.
> > +.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
> > +.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.
> > +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> :
> > +.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.
> > +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.
> > +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.
> > +The pages added are either regular memory pages for code and data
>
> regular memory pages -> regular pages?
Changed.
>
> ,
> > +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.
> > +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 .
> > +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 "."
>
> > +.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.
>
> > +If
> > +.BR mmap (2)_
>
> Unexpected "_" above
Thanks.
>
> > +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.
> > +.SH SEE ALSO
> > +.BR ioctl (2),
> > +.BR mmap (2),
> > +.BR mprotect (2)
> > \ No newline at end of file
> >
>
> Reinette
Thanks for valuable feedback.
/Jarkko
