[isa-dev] Comments on Scalar Crypto v1.0.0-rc2
55 views
Skip to first unread message
L Peter Deutsch
Sep 4, 2021, 8:28:43 PM9/4/21
to RISC-V ISA Dev
Overall comment after a detailed reading: this is a very well-written
specification. I hope Sections 1.1 and 1.3 in particular will be a
model for other extension proposals.
Chapter 1, Introduction
Last paragraph: "in ernest" should be "in earnest."
1.1 Intended Audience
Many thanks to the authors for including this section!
1st paragraph: I think "specialist subject" should be "specialised subject."
2nd paragraph: "peoples understanding" should be "people's
understanding." The comma before "and pass" is inappropriate and
ambiguous: I suggest "... to them, and which they may (safely!) ignore
or pass to a colleague."
Cryptographers section, 2nd sentence: "which we refer to" should be "to
which we refer."
Digital design engineers section, 2nd/3rd sentence: "issues. E.g.,"
should be "issues, e.g.,".
Verification engineers, 3rd sentence: the second clause ("and knowing")
is grammatically incorrect, and its meaning is unclear. I think what is
meant is "and to know how ...," although this is somewhat at odds with
the expectation of no cryptography background.
1.2 Sail Specifications
In the second paragraph, I found the phrase "code snippets" ambiguous as
to whether the sum of the snippets plus Appendix D is to the complete
Sail-language model for this extension, especially since the words
"formal model" are a link to the sail-riscv Web site on Github. I
suggest that an explicit policy decision is needed as to whether the
full Sail-language model for each extension must, or should, be included
in the specification document. I personally am strongly in favor of
doing this, because in my opinion the specification is not actually a
specification otherwise. However, whichever way this decision goes,
this section should state what it is, by including (approximately) one
of the two following statements:
"The code snippets in the individual instruction descriptions, together
with Appendix D, constitute the complete formal model of the extension
described in this document."
or
"This document contains only a subset of the formal model: refer to the
Web site [ref#] for the complete model."
1.3 Policies
Another excellent discussion, thank you.
Second bullet point: "concurrently with, or after" should delete the comma;
likewise "contest, and the".
Chapter 2, Extensions Overview
Middle paragraph, "(e.g., ..., among others)" is redundant. I suggest
removing the "among others".
There several places in this chapter that refer to "the Zbkb". I think
these should be either simply "Zbkb" or "the Zbkb extension"; I advocate
the former.
2.1 Zbkb
Main paragraph, 3rd sentence: I suggest changing "make the
specification" to "make the present specification".
2.2 Zbkc
Same issue as 2.1.
2.3 Zbkx
Same issue as 2.1.
2.4, 2.5 NIST/AES
I suggest adding a note in both places to indicate that the
aes64ks{1i,2} instructions are present in both Zknd and Zkne, which is
not customary for extensions in general.
Chapter 3, Instructions
I didn't attempt to read any of the Sail code: I trust that at the very
least it has all been passed through the relevant Sail software for
checking. The same goes for Appendix D.
In multiple places, "it's execution latency" should use "its".
3.18 packh
Delete the "And" at the beginning of the Description.
3.27 sha256sig0
Description, 3rd sentence: "is operated on" should be "are operated on".
Same issue in 3.28, 3.29, 3.30.
3.31 sha512sig0h
Description: "is represented" should be "are each represented".
Same issue in 3.32, 3.33, 3.34, 3.35, 3.36.
3.43 sm4ed
Description, 4th sentence: "upto" should be just "to".
Same issue in 3.44.
Appendix C: Supplementary Materials
See the note above about inclusion of the full formal model in the
specification.
----------------------------------------------------------------
L Peter Deutsch <gh...@major2nd.com> :: Aladdin Enterprises :: Healdsburg, CA
Was your vote really counted? http://www.verifiedvoting.org
specification. I hope Sections 1.1 and 1.3 in particular will be a
model for other extension proposals.
Chapter 1, Introduction
Last paragraph: "in ernest" should be "in earnest."
1.1 Intended Audience
Many thanks to the authors for including this section!
1st paragraph: I think "specialist subject" should be "specialised subject."
2nd paragraph: "peoples understanding" should be "people's
understanding." The comma before "and pass" is inappropriate and
ambiguous: I suggest "... to them, and which they may (safely!) ignore
or pass to a colleague."
Cryptographers section, 2nd sentence: "which we refer to" should be "to
which we refer."
Digital design engineers section, 2nd/3rd sentence: "issues. E.g.,"
should be "issues, e.g.,".
Verification engineers, 3rd sentence: the second clause ("and knowing")
is grammatically incorrect, and its meaning is unclear. I think what is
meant is "and to know how ...," although this is somewhat at odds with
the expectation of no cryptography background.
1.2 Sail Specifications
In the second paragraph, I found the phrase "code snippets" ambiguous as
to whether the sum of the snippets plus Appendix D is to the complete
Sail-language model for this extension, especially since the words
"formal model" are a link to the sail-riscv Web site on Github. I
suggest that an explicit policy decision is needed as to whether the
full Sail-language model for each extension must, or should, be included
in the specification document. I personally am strongly in favor of
doing this, because in my opinion the specification is not actually a
specification otherwise. However, whichever way this decision goes,
this section should state what it is, by including (approximately) one
of the two following statements:
"The code snippets in the individual instruction descriptions, together
with Appendix D, constitute the complete formal model of the extension
described in this document."
or
"This document contains only a subset of the formal model: refer to the
Web site [ref#] for the complete model."
1.3 Policies
Another excellent discussion, thank you.
Second bullet point: "concurrently with, or after" should delete the comma;
likewise "contest, and the".
Chapter 2, Extensions Overview
Middle paragraph, "(e.g., ..., among others)" is redundant. I suggest
removing the "among others".
There several places in this chapter that refer to "the Zbkb". I think
these should be either simply "Zbkb" or "the Zbkb extension"; I advocate
the former.
2.1 Zbkb
Main paragraph, 3rd sentence: I suggest changing "make the
specification" to "make the present specification".
2.2 Zbkc
Same issue as 2.1.
2.3 Zbkx
Same issue as 2.1.
2.4, 2.5 NIST/AES
I suggest adding a note in both places to indicate that the
aes64ks{1i,2} instructions are present in both Zknd and Zkne, which is
not customary for extensions in general.
Chapter 3, Instructions
I didn't attempt to read any of the Sail code: I trust that at the very
least it has all been passed through the relevant Sail software for
checking. The same goes for Appendix D.
In multiple places, "it's execution latency" should use "its".
3.18 packh
Delete the "And" at the beginning of the Description.
3.27 sha256sig0
Description, 3rd sentence: "is operated on" should be "are operated on".
Same issue in 3.28, 3.29, 3.30.
3.31 sha512sig0h
Description: "is represented" should be "are each represented".
Same issue in 3.32, 3.33, 3.34, 3.35, 3.36.
3.43 sm4ed
Description, 4th sentence: "upto" should be just "to".
Same issue in 3.44.
Appendix C: Supplementary Materials
See the note above about inclusion of the full formal model in the
specification.
----------------------------------------------------------------
L Peter Deutsch <gh...@major2nd.com> :: Aladdin Enterprises :: Healdsburg, CA
Was your vote really counted? http://www.verifiedvoting.org
Ben Marshall
Sep 8, 2021, 8:38:56 AM9/8/21
to RISC-V ISA Dev, L Peter Deutsch
Hi Peter
Thank you for your very thorough reading of the spec. I'm really glad you are so positive about it!
I'll work on the editorial issues in time (I believe some have already been fixed), but the Sail model comments definitely warrant some discussion.
I'll work on the editorial issues in time (I believe some have already been fixed), but the Sail model comments definitely warrant some discussion.
> I
suggest that an explicit policy decision is needed as to whether the
> full Sail-language model for each extension must, or should, be included
> in the specification document. I personally am strongly in favor of
> doing this, because in my opinion the specification is not actually a
> specification otherwise.
> full Sail-language model for each extension must, or should, be included
> in the specification document. I personally am strongly in favor of
> doing this, because in my opinion the specification is not actually a
> specification otherwise.
I agree a consistent policy will be needed, though in practice we might need to be pragmatic. For extensions which just introduce "simple" instructions, including all of the formal model components in the specification makes sense. For extensions with some "complex" instructions, or instructions which do not lend themselves to concise expression in Sail (some of the instructions in this specification are in this category) then calling out to an appendix is, I think, reasonable.
Some extensions though, which are nebulous and touch all over the ISA (thinking mostly of the hypervisor here) cannot possibly include the formal model directly into the specification document. It would be extremely hard to structure as a specification. Better to write a clear best effort specification as prose, and defer to the formal model if a concrete explanation without code becomes too hard.
There's also the issue of "big" instructions (thinking mostly of forthcoming vector cryptography instructions) that implement entire algorithms (SHA2, AES) in a single instruction, and become unwieldy when described purely in Sail in the specification document.
Some extensions though, which are nebulous and touch all over the ISA (thinking mostly of the hypervisor here) cannot possibly include the formal model directly into the specification document. It would be extremely hard to structure as a specification. Better to write a clear best effort specification as prose, and defer to the formal model if a concrete explanation without code becomes too hard.
There's also the issue of "big" instructions (thinking mostly of forthcoming vector cryptography instructions) that implement entire algorithms (SHA2, AES) in a single instruction, and become unwieldy when described purely in Sail in the specification document.
I think there is room for a pragmatic approach, and that it's best not to subvert a readers expectation that the specification PDF documents are mostly prose, and are typeset to make that as clear as possible. We can then link out to the formal model, which is best presented as code.
Thanks,
Ben
Thanks,
Ben
L Peter Deutsch
Sep 8, 2021, 9:25:27 AM9/8/21
to Ben Marshall, isa...@groups.riscv.org
> I agree a consistent policy will be needed, though in practice we might need
> to be pragmatic. For extensions which just introduce "simple" instructions,
> including all of the formal model components in the specification makes
> sense. For extensions with some "complex" instructions, or instructions which
> do not lend themselves to concise expression in Sail (some of the
> instructions in this specification are in this category) then calling out to
> an appendix is, I think, reasonable.
Sorry, I see my comment wasn't clear. I agree about in-line versus
> to be pragmatic. For extensions which just introduce "simple" instructions,
> including all of the formal model components in the specification makes
> sense. For extensions with some "complex" instructions, or instructions which
> do not lend themselves to concise expression in Sail (some of the
> instructions in this specification are in this category) then calling out to
> an appendix is, I think, reasonable.
separate placement of the Sail code. What I'm advocating is simply that
any out-of-line material be actual appendices within the specification
document, rather than residing somewhere else: I believe a single
document minimizes the risk of the two sets of material getting out
of sync.
> Some extensions though, which are nebulous and touch all over the ISA
> (thinking mostly of the hypervisor here) cannot possibly include the formal
> model directly into the specification document. It would be extremely hard to
> structure as a specification. Better to write a clear best effort
> specification as prose, and defer to the formal model if a concrete
> explanation without code becomes too hard.
> There's also the issue of "big" instructions (thinking mostly of forthcoming
> vector cryptography instructions) that implement entire algorithms (SHA2,
> AES) in a single instruction, and become unwieldy when described purely in
> Sail in the specification document.
opposed to prose descriptions, which are certainly more readable for
larger instructions. I only advocate that all of the Sail code be
included in the specification document. If in-line code would be less
readable, the code should be in an included appendix, not a separate
document. I think this give us the best shot at avoiding DDDD
(Deutsch's Dictum on the Dangers of Duplication, see below).
--
L Peter Deutsch <gh...@major2nd.com> :: Aladdin Enterprises :: Healdsburg, CA
Was your vote really counted? http://www.verifiedvoting.org
having to keep multiple pieces of data consistent with each other
manually. The best approach is to generate all needed forms of the data
from a single source; when this is infeasible, the next best thing is to
be able to check mechanically that the multiple data copies are
consistent with each other, and to have automatic means for ensuring
this gets done when necessary." It's amazing how many situations this
applies to when you think about it.
Philipp Tomsich
Sep 10, 2021, 4:39:15 AM9/10/21
to Ben Marshall, RISC-V ISA Dev, L Peter Deutsch
On Wed, 8 Sept 2021 at 15:38, Ben Marshall <benmarsh...@gmail.com> wrote:
> I suggest that an explicit policy decision is needed as to whether the> full Sail-language model for each extension must, or should, be included
> in the specification document. I personally am strongly in favor of
> doing this, because in my opinion the specification is not actually a
> specification otherwise.I agree a consistent policy will be needed, though in practice we might need to be pragmatic. For extensions which just introduce "simple" instructions, including all of the formal model components in the specification makes sense. For extensions with some "complex" instructions, or instructions which do not lend themselves to concise expression in Sail (some of the instructions in this specification are in this category) then calling out to an appendix is, I think, reasonable.
I agree that we will need exceptions if the policy will require SAIL in the specification (note that I also strongly favor making this mandatory): consider operations, such as cache-maintenance or prefetch (i.e., Zicmoc and Zicmop, respectively), which can not be modelled in SAIL.
Philipp.
Allen Baum
Sep 10, 2021, 5:50:23 PM9/10/21
to Philipp Tomsich, Ben Marshall, RISC-V ISA Dev, L Peter Deutsch
Actually, my idea for modeling these in Sail is to configure SAIL to have either no cache or a known small size fully associative cache. We would run tests twice, once with each configuration and ensure that results match one or the other only.
--
You received this message because you are subscribed to the Google Groups "RISC-V ISA Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isa-dev+unsubscribe@groups.riscv.org.
To view this discussion on the web visit https://groups.google.com/a/groups.riscv.org/d/msgid/isa-dev/CAAeLtUBQOSXJx%3D0NzHMGFsd%3DgdyBVEM%3DdKDrikF-9aK1OYs9ZQ%40mail.gmail.com.
Reply all
Reply to author
Forward
0 new messages