Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Tue, Sep 04, 2018 at 09:04:44AM -0700, Dave Hansen wrote: > On 09/03/2018 06:16 AM, Andy Shevchenko wrote: > >> + EBLOCK = 0x9, > >> + EPA = 0xA, > >> + EWB = 0xB, > >> + ETRACK = 0xC, > >> + EAUG= 0xD, > >> + EMODPR = 0xE, > >> + EMODT = 0xF, > >> +}; > > Hmm... This E prefix confuses me with (system wide) error codes. Has > > it been discussed before? If so, can you point on the conclusion why > > the current format is good? > > Making them SGX_EWHATEVER isn't a horrible idea. OK, going with that. /Jarkko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Tue, Sep 04, 2018 at 09:04:44AM -0700, Dave Hansen wrote: > On 09/03/2018 06:16 AM, Andy Shevchenko wrote: > >> + EBLOCK = 0x9, > >> + EPA = 0xA, > >> + EWB = 0xB, > >> + ETRACK = 0xC, > >> + EAUG= 0xD, > >> + EMODPR = 0xE, > >> + EMODT = 0xF, > >> +}; > > Hmm... This E prefix confuses me with (system wide) error codes. Has > > it been discussed before? If so, can you point on the conclusion why > > the current format is good? > > Making them SGX_EWHATEVER isn't a horrible idea. OK, going with that. /Jarkko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Tue, Sep 4, 2018 at 7:04 PM Dave Hansen wrote: > > On 09/03/2018 06:16 AM, Andy Shevchenko wrote: > >> + EBLOCK = 0x9, > >> + EPA = 0xA, > >> + EWB = 0xB, > >> + ETRACK = 0xC, > >> + EAUG= 0xD, > >> + EMODPR = 0xE, > >> + EMODT = 0xF, > >> +}; > > Hmm... This E prefix confuses me with (system wide) error codes. Has > > it been discussed before? If so, can you point on the conclusion why > > the current format is good? > > Making them SGX_EWHATEVER isn't a horrible idea. +1 here. Namespace will not shadow SDM naming scheme. -- With Best Regards, Andy Shevchenko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Tue, Sep 4, 2018 at 7:04 PM Dave Hansen wrote: > > On 09/03/2018 06:16 AM, Andy Shevchenko wrote: > >> + EBLOCK = 0x9, > >> + EPA = 0xA, > >> + EWB = 0xB, > >> + ETRACK = 0xC, > >> + EAUG= 0xD, > >> + EMODPR = 0xE, > >> + EMODT = 0xF, > >> +}; > > Hmm... This E prefix confuses me with (system wide) error codes. Has > > it been discussed before? If so, can you point on the conclusion why > > the current format is good? > > Making them SGX_EWHATEVER isn't a horrible idea. +1 here. Namespace will not shadow SDM naming scheme. -- With Best Regards, Andy Shevchenko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On 09/03/2018 06:16 AM, Andy Shevchenko wrote: >> + EBLOCK = 0x9, >> + EPA = 0xA, >> + EWB = 0xB, >> + ETRACK = 0xC, >> + EAUG= 0xD, >> + EMODPR = 0xE, >> + EMODT = 0xF, >> +}; > Hmm... This E prefix confuses me with (system wide) error codes. Has > it been discussed before? If so, can you point on the conclusion why > the current format is good? Making them SGX_EWHATEVER isn't a horrible idea.
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On 09/03/2018 06:16 AM, Andy Shevchenko wrote: >> + EBLOCK = 0x9, >> + EPA = 0xA, >> + EWB = 0xB, >> + ETRACK = 0xC, >> + EAUG= 0xD, >> + EMODPR = 0xE, >> + EMODT = 0xF, >> +}; > Hmm... This E prefix confuses me with (system wide) error codes. Has > it been discussed before? If so, can you point on the conclusion why > the current format is good? Making them SGX_EWHATEVER isn't a horrible idea.
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Mon, Sep 03, 2018 at 04:16:42PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 27, 2018 at 9:57 PM Jarkko Sakkinen
> wrote:
> >
> > Add arch/x86/include/asm/sgx_arch.h, which contains definitions for the
> > architectural data structures used by the CPU to implement the SGX.
>
> > +/**
> > + * enum sgx_encls_leaves - ENCLS leaf functions
> > + * %ECREATE: Create an enclave.
> > + * %EADD: Add a page to an enclave.
> > + * %EINIT: Launch an enclave.
> > + * %EREMOVE: Remove a page from an enclave.
> > + * %EDBGRD:Read a word from an enclve (peek).
> > + * %EDBGWR:Write a word to an enclave (poke).
> > + * %EEXTEND: Measure 256 bytes of an added enclave page.
> > + * %ELDB: Load a swapped page in blocked state.
> > + * %ELDU: Load a swapped page in unblocked state.
> > + * %EBLOCK:Change page state to blocked i.e. entering hardware threads
> > + * cannot access it and create new TLB entries.
> > + * %EPA: Create a Version Array (VA) page used to store isvsvn number
> > + * for a swapped EPC page.
> > + * %EWB: Swap an enclave page to the regular memory. Checks that all
> > + * threads have exited that were in the previous shoot-down
> > + * sequence.
> > + * %ETRACK:Start a new shoot down sequence. Used to together with
> > EBLOCK
> > + * to make sure that a page is safe to swap.
> > + */
> > +enum sgx_encls_leaves {
> > + ECREATE = 0x0,
> > + EADD= 0x1,
> > + EINIT = 0x2,
> > + EREMOVE = 0x3,
> > + EDGBRD = 0x4,
> > + EDGBWR = 0x5,
> > + EEXTEND = 0x6,
> > + ELDB= 0x7,
> > + ELDU= 0x8,
> > + EBLOCK = 0x9,
> > + EPA = 0xA,
> > + EWB = 0xB,
> > + ETRACK = 0xC,
> > + EAUG= 0xD,
> > + EMODPR = 0xE,
> > + EMODT = 0xF,
> > +};
>
> Hmm... This E prefix confuses me with (system wide) error codes. Has
> it been discussed before? If so, can you point on the conclusion why
> the current format is good?
That is how they are prefixed in the SDM.
> > +enum sgx_miscselect {
> > + SGX_MISC_EXINFO = 0x01,
> > +};
> > +
> > +#define SGX_MISC_RESERVED_MASK 0xFFFEULL
>
> Any idea why we are not using BIT_ULL() / BIT() and GENMASK_ULL() /
> GENMASK() in the code?
No good reason. I'll change it.
> > +enum sgx_attribute {
> > + SGX_ATTR_DEBUG = 0x02,
> > + SGX_ATTR_MODE64BIT = 0x04,
> > + SGX_ATTR_PROVISIONKEY = 0x10,
> > + SGX_ATTR_EINITTOKENKEY = 0x20,
> > +};
> > +
> > +#define SGX_ATTR_RESERVED_MASK 0xFFC9ULL
>
> Some times listing explicitly not-reserved bits might be better and
> figuring out reserved mask is a simple ~ operation.
Yea, agreed.
> > +enum sgx_tcs_flags {
> > + SGX_TCS_DBGOPTIN= 0x01,
> > +};
> > +
> > +#define SGX_TCS_RESERVED_MASK 0xFFFEULL
>
> > +#define SGX_SECINFO_PERMISSION_MASK0x0007ULL
> > +#define SGX_SECINFO_PAGE_TYPE_MASK 0xFF00ULL
> > +#define SGX_SECINFO_RESERVED_MASK 0x00F8ULL
>
> So, something like
>
> MASK1 GENMASK_ULL
> MASK2 GENMASK_ULL
> MASK3 ~(MASK1 | MASK2)
>
> ?
Definitely. I think it is just a matter of legacy why they are like they
are :-) These constants have gone over long time and they are not
usually places where you have to look for regressions. Thanks for
spotting these.
>
> --
> With Best Regards,
> Andy Shevchenko
/Jarkko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Mon, Sep 03, 2018 at 04:16:42PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 27, 2018 at 9:57 PM Jarkko Sakkinen
> wrote:
> >
> > Add arch/x86/include/asm/sgx_arch.h, which contains definitions for the
> > architectural data structures used by the CPU to implement the SGX.
>
> > +/**
> > + * enum sgx_encls_leaves - ENCLS leaf functions
> > + * %ECREATE: Create an enclave.
> > + * %EADD: Add a page to an enclave.
> > + * %EINIT: Launch an enclave.
> > + * %EREMOVE: Remove a page from an enclave.
> > + * %EDBGRD:Read a word from an enclve (peek).
> > + * %EDBGWR:Write a word to an enclave (poke).
> > + * %EEXTEND: Measure 256 bytes of an added enclave page.
> > + * %ELDB: Load a swapped page in blocked state.
> > + * %ELDU: Load a swapped page in unblocked state.
> > + * %EBLOCK:Change page state to blocked i.e. entering hardware threads
> > + * cannot access it and create new TLB entries.
> > + * %EPA: Create a Version Array (VA) page used to store isvsvn number
> > + * for a swapped EPC page.
> > + * %EWB: Swap an enclave page to the regular memory. Checks that all
> > + * threads have exited that were in the previous shoot-down
> > + * sequence.
> > + * %ETRACK:Start a new shoot down sequence. Used to together with
> > EBLOCK
> > + * to make sure that a page is safe to swap.
> > + */
> > +enum sgx_encls_leaves {
> > + ECREATE = 0x0,
> > + EADD= 0x1,
> > + EINIT = 0x2,
> > + EREMOVE = 0x3,
> > + EDGBRD = 0x4,
> > + EDGBWR = 0x5,
> > + EEXTEND = 0x6,
> > + ELDB= 0x7,
> > + ELDU= 0x8,
> > + EBLOCK = 0x9,
> > + EPA = 0xA,
> > + EWB = 0xB,
> > + ETRACK = 0xC,
> > + EAUG= 0xD,
> > + EMODPR = 0xE,
> > + EMODT = 0xF,
> > +};
>
> Hmm... This E prefix confuses me with (system wide) error codes. Has
> it been discussed before? If so, can you point on the conclusion why
> the current format is good?
That is how they are prefixed in the SDM.
> > +enum sgx_miscselect {
> > + SGX_MISC_EXINFO = 0x01,
> > +};
> > +
> > +#define SGX_MISC_RESERVED_MASK 0xFFFEULL
>
> Any idea why we are not using BIT_ULL() / BIT() and GENMASK_ULL() /
> GENMASK() in the code?
No good reason. I'll change it.
> > +enum sgx_attribute {
> > + SGX_ATTR_DEBUG = 0x02,
> > + SGX_ATTR_MODE64BIT = 0x04,
> > + SGX_ATTR_PROVISIONKEY = 0x10,
> > + SGX_ATTR_EINITTOKENKEY = 0x20,
> > +};
> > +
> > +#define SGX_ATTR_RESERVED_MASK 0xFFC9ULL
>
> Some times listing explicitly not-reserved bits might be better and
> figuring out reserved mask is a simple ~ operation.
Yea, agreed.
> > +enum sgx_tcs_flags {
> > + SGX_TCS_DBGOPTIN= 0x01,
> > +};
> > +
> > +#define SGX_TCS_RESERVED_MASK 0xFFFEULL
>
> > +#define SGX_SECINFO_PERMISSION_MASK0x0007ULL
> > +#define SGX_SECINFO_PAGE_TYPE_MASK 0xFF00ULL
> > +#define SGX_SECINFO_RESERVED_MASK 0x00F8ULL
>
> So, something like
>
> MASK1 GENMASK_ULL
> MASK2 GENMASK_ULL
> MASK3 ~(MASK1 | MASK2)
>
> ?
Definitely. I think it is just a matter of legacy why they are like they
are :-) These constants have gone over long time and they are not
usually places where you have to look for regressions. Thanks for
spotting these.
>
> --
> With Best Regards,
> Andy Shevchenko
/Jarkko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Mon, Aug 27, 2018 at 9:57 PM Jarkko Sakkinen
wrote:
>
> Add arch/x86/include/asm/sgx_arch.h, which contains definitions for the
> architectural data structures used by the CPU to implement the SGX.
> +/**
> + * enum sgx_encls_leaves - ENCLS leaf functions
> + * %ECREATE: Create an enclave.
> + * %EADD: Add a page to an enclave.
> + * %EINIT: Launch an enclave.
> + * %EREMOVE: Remove a page from an enclave.
> + * %EDBGRD:Read a word from an enclve (peek).
> + * %EDBGWR:Write a word to an enclave (poke).
> + * %EEXTEND: Measure 256 bytes of an added enclave page.
> + * %ELDB: Load a swapped page in blocked state.
> + * %ELDU: Load a swapped page in unblocked state.
> + * %EBLOCK:Change page state to blocked i.e. entering hardware threads
> + * cannot access it and create new TLB entries.
> + * %EPA: Create a Version Array (VA) page used to store isvsvn number
> + * for a swapped EPC page.
> + * %EWB: Swap an enclave page to the regular memory. Checks that all
> + * threads have exited that were in the previous shoot-down
> + * sequence.
> + * %ETRACK:Start a new shoot down sequence. Used to together with EBLOCK
> + * to make sure that a page is safe to swap.
> + */
> +enum sgx_encls_leaves {
> + ECREATE = 0x0,
> + EADD= 0x1,
> + EINIT = 0x2,
> + EREMOVE = 0x3,
> + EDGBRD = 0x4,
> + EDGBWR = 0x5,
> + EEXTEND = 0x6,
> + ELDB= 0x7,
> + ELDU= 0x8,
> + EBLOCK = 0x9,
> + EPA = 0xA,
> + EWB = 0xB,
> + ETRACK = 0xC,
> + EAUG= 0xD,
> + EMODPR = 0xE,
> + EMODT = 0xF,
> +};
Hmm... This E prefix confuses me with (system wide) error codes. Has
it been discussed before? If so, can you point on the conclusion why
the current format is good?
> +enum sgx_miscselect {
> + SGX_MISC_EXINFO = 0x01,
> +};
> +
> +#define SGX_MISC_RESERVED_MASK 0xFFFEULL
Any idea why we are not using BIT_ULL() / BIT() and GENMASK_ULL() /
GENMASK() in the code?
> +enum sgx_attribute {
> + SGX_ATTR_DEBUG = 0x02,
> + SGX_ATTR_MODE64BIT = 0x04,
> + SGX_ATTR_PROVISIONKEY = 0x10,
> + SGX_ATTR_EINITTOKENKEY = 0x20,
> +};
> +
> +#define SGX_ATTR_RESERVED_MASK 0xFFC9ULL
Some times listing explicitly not-reserved bits might be better and
figuring out reserved mask is a simple ~ operation.
> +enum sgx_tcs_flags {
> + SGX_TCS_DBGOPTIN= 0x01,
> +};
> +
> +#define SGX_TCS_RESERVED_MASK 0xFFFEULL
> +#define SGX_SECINFO_PERMISSION_MASK0x0007ULL
> +#define SGX_SECINFO_PAGE_TYPE_MASK 0xFF00ULL
> +#define SGX_SECINFO_RESERVED_MASK 0x00F8ULL
So, something like
MASK1 GENMASK_ULL
MASK2 GENMASK_ULL
MASK3 ~(MASK1 | MASK2)
?
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Mon, Aug 27, 2018 at 9:57 PM Jarkko Sakkinen
wrote:
>
> Add arch/x86/include/asm/sgx_arch.h, which contains definitions for the
> architectural data structures used by the CPU to implement the SGX.
> +/**
> + * enum sgx_encls_leaves - ENCLS leaf functions
> + * %ECREATE: Create an enclave.
> + * %EADD: Add a page to an enclave.
> + * %EINIT: Launch an enclave.
> + * %EREMOVE: Remove a page from an enclave.
> + * %EDBGRD:Read a word from an enclve (peek).
> + * %EDBGWR:Write a word to an enclave (poke).
> + * %EEXTEND: Measure 256 bytes of an added enclave page.
> + * %ELDB: Load a swapped page in blocked state.
> + * %ELDU: Load a swapped page in unblocked state.
> + * %EBLOCK:Change page state to blocked i.e. entering hardware threads
> + * cannot access it and create new TLB entries.
> + * %EPA: Create a Version Array (VA) page used to store isvsvn number
> + * for a swapped EPC page.
> + * %EWB: Swap an enclave page to the regular memory. Checks that all
> + * threads have exited that were in the previous shoot-down
> + * sequence.
> + * %ETRACK:Start a new shoot down sequence. Used to together with EBLOCK
> + * to make sure that a page is safe to swap.
> + */
> +enum sgx_encls_leaves {
> + ECREATE = 0x0,
> + EADD= 0x1,
> + EINIT = 0x2,
> + EREMOVE = 0x3,
> + EDGBRD = 0x4,
> + EDGBWR = 0x5,
> + EEXTEND = 0x6,
> + ELDB= 0x7,
> + ELDU= 0x8,
> + EBLOCK = 0x9,
> + EPA = 0xA,
> + EWB = 0xB,
> + ETRACK = 0xC,
> + EAUG= 0xD,
> + EMODPR = 0xE,
> + EMODT = 0xF,
> +};
Hmm... This E prefix confuses me with (system wide) error codes. Has
it been discussed before? If so, can you point on the conclusion why
the current format is good?
> +enum sgx_miscselect {
> + SGX_MISC_EXINFO = 0x01,
> +};
> +
> +#define SGX_MISC_RESERVED_MASK 0xFFFEULL
Any idea why we are not using BIT_ULL() / BIT() and GENMASK_ULL() /
GENMASK() in the code?
> +enum sgx_attribute {
> + SGX_ATTR_DEBUG = 0x02,
> + SGX_ATTR_MODE64BIT = 0x04,
> + SGX_ATTR_PROVISIONKEY = 0x10,
> + SGX_ATTR_EINITTOKENKEY = 0x20,
> +};
> +
> +#define SGX_ATTR_RESERVED_MASK 0xFFC9ULL
Some times listing explicitly not-reserved bits might be better and
figuring out reserved mask is a simple ~ operation.
> +enum sgx_tcs_flags {
> + SGX_TCS_DBGOPTIN= 0x01,
> +};
> +
> +#define SGX_TCS_RESERVED_MASK 0xFFFEULL
> +#define SGX_SECINFO_PERMISSION_MASK0x0007ULL
> +#define SGX_SECINFO_PAGE_TYPE_MASK 0xFF00ULL
> +#define SGX_SECINFO_RESERVED_MASK 0x00F8ULL
So, something like
MASK1 GENMASK_ULL
MASK2 GENMASK_ULL
MASK3 ~(MASK1 | MASK2)
?
--
With Best Regards,
Andy Shevchenko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Mon, Aug 27, 2018 at 12:41:29PM -0700, Dave Hansen wrote:
> > +/**
> > + * enum sgx_encls_leaves - return codes for ENCLS, ENCLU and ENCLV
> > + * %SGX_SUCCESS: No error.
> > + * %SGX_INVALID_SIG_STRUCT:SIGSTRUCT contains an invalid value.
> > + * %SGX_INVALID_ATTRIBUTE: Enclave is not attempting to access a resource
> > + * for which it is not authorized.
> > + * %SGX_BLKSTATE: EPC page is already blocked.
> > + * %SGX_INVALID_MEASUREMENT: SIGSTRUCT or EINITTOKEN contains an
> > incorrect
> > + * measurement.
> ...
> > +enum sgx_return_codes {
> > + SGX_SUCCESS = 0,
> > + SGX_INVALID_SIG_STRUCT = 1,
> > + SGX_INVALID_ATTRIBUTE = 2,
> > + SGX_BLKSTATE= 3,
> > + SGX_INVALID_MEASUREMENT = 4,
> ...
>
> I don't think I've ever seen this particular method of commenting
> before. It's rather verbose and duplicates the names twice, which seems
> a bit silly.
>
> Can you talk a bit about why you chose to do it this way? I'd
> personally much rather see at least some brief comments inline with the
> definitions.
The reason that I chose this was
https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt
It is recommended in the "kernel-doc for structs, unions, enums, and
typedefs" section.
/Jarkko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
On Mon, Aug 27, 2018 at 12:41:29PM -0700, Dave Hansen wrote:
> > +/**
> > + * enum sgx_encls_leaves - return codes for ENCLS, ENCLU and ENCLV
> > + * %SGX_SUCCESS: No error.
> > + * %SGX_INVALID_SIG_STRUCT:SIGSTRUCT contains an invalid value.
> > + * %SGX_INVALID_ATTRIBUTE: Enclave is not attempting to access a resource
> > + * for which it is not authorized.
> > + * %SGX_BLKSTATE: EPC page is already blocked.
> > + * %SGX_INVALID_MEASUREMENT: SIGSTRUCT or EINITTOKEN contains an
> > incorrect
> > + * measurement.
> ...
> > +enum sgx_return_codes {
> > + SGX_SUCCESS = 0,
> > + SGX_INVALID_SIG_STRUCT = 1,
> > + SGX_INVALID_ATTRIBUTE = 2,
> > + SGX_BLKSTATE= 3,
> > + SGX_INVALID_MEASUREMENT = 4,
> ...
>
> I don't think I've ever seen this particular method of commenting
> before. It's rather verbose and duplicates the names twice, which seems
> a bit silly.
>
> Can you talk a bit about why you chose to do it this way? I'd
> personally much rather see at least some brief comments inline with the
> definitions.
The reason that I chose this was
https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt
It is recommended in the "kernel-doc for structs, unions, enums, and
typedefs" section.
/Jarkko
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
> +/**
> + * enum sgx_encls_leaves - return codes for ENCLS, ENCLU and ENCLV
> + * %SGX_SUCCESS: No error.
> + * %SGX_INVALID_SIG_STRUCT: SIGSTRUCT contains an invalid value.
> + * %SGX_INVALID_ATTRIBUTE: Enclave is not attempting to access a resource
> + * for which it is not authorized.
> + * %SGX_BLKSTATE:EPC page is already blocked.
> + * %SGX_INVALID_MEASUREMENT: SIGSTRUCT or EINITTOKEN contains an incorrect
> + * measurement.
...
> +enum sgx_return_codes {
> + SGX_SUCCESS = 0,
> + SGX_INVALID_SIG_STRUCT = 1,
> + SGX_INVALID_ATTRIBUTE = 2,
> + SGX_BLKSTATE= 3,
> + SGX_INVALID_MEASUREMENT = 4,
...
I don't think I've ever seen this particular method of commenting
before. It's rather verbose and duplicates the names twice, which seems
a bit silly.
Can you talk a bit about why you chose to do it this way? I'd
personally much rather see at least some brief comments inline with the
definitions.
Re: [PATCH v13 04/13] x86/sgx: Architectural structures
> +/**
> + * enum sgx_encls_leaves - return codes for ENCLS, ENCLU and ENCLV
> + * %SGX_SUCCESS: No error.
> + * %SGX_INVALID_SIG_STRUCT: SIGSTRUCT contains an invalid value.
> + * %SGX_INVALID_ATTRIBUTE: Enclave is not attempting to access a resource
> + * for which it is not authorized.
> + * %SGX_BLKSTATE:EPC page is already blocked.
> + * %SGX_INVALID_MEASUREMENT: SIGSTRUCT or EINITTOKEN contains an incorrect
> + * measurement.
...
> +enum sgx_return_codes {
> + SGX_SUCCESS = 0,
> + SGX_INVALID_SIG_STRUCT = 1,
> + SGX_INVALID_ATTRIBUTE = 2,
> + SGX_BLKSTATE= 3,
> + SGX_INVALID_MEASUREMENT = 4,
...
I don't think I've ever seen this particular method of commenting
before. It's rather verbose and duplicates the names twice, which seems
a bit silly.
Can you talk a bit about why you chose to do it this way? I'd
personally much rather see at least some brief comments inline with the
definitions.
