2020-03-19 09:21:21

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 05/70] x86/insn: Make inat-tables.c suitable for pre-decompression code

From: Joerg Roedel <[email protected]>

The inat-tables.c file has some arrays in it that contain pointers to
other arrays. These pointers need to be relocated when the kernel
image is moved to a different location.

The pre-decompression boot-code has no support for applying ELF
relocations, so initialize these arrays at runtime in the
pre-decompression code to make sure all pointers are correctly
initialized.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/tools/gen-insn-attr-x86.awk | 50 +++++++++++++++++++++-
tools/arch/x86/tools/gen-insn-attr-x86.awk | 50 +++++++++++++++++++++-
2 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
index a42015b305f4..af38469afd14 100644
--- a/arch/x86/tools/gen-insn-attr-x86.awk
+++ b/arch/x86/tools/gen-insn-attr-x86.awk
@@ -362,6 +362,9 @@ function convert_operands(count,opnd, i,j,imm,mod)
END {
if (awkchecked != "")
exit 1
+
+ print "#ifndef __BOOT_COMPRESSED\n"
+
# print escape opcode map's array
print "/* Escape opcode map array */"
print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
@@ -388,6 +391,51 @@ END {
for (j = 0; j < max_lprefix; j++)
if (atable[i,j])
print " ["i"]["j"] = "atable[i,j]","
- print "};"
+ print "};\n"
+
+ print "#else /* !__BOOT_COMPRESSED */\n"
+
+ print "/* Escape opcode map array */"
+ print "static const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
+ "[INAT_LSTPFX_MAX + 1];"
+ print ""
+
+ print "/* Group opcode map array */"
+ print "static const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
+ "[INAT_LSTPFX_MAX + 1];"
+ print ""
+
+ print "/* AVX opcode map array */"
+ print "static const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
+ "[INAT_LSTPFX_MAX + 1];"
+ print ""
+
+ print "static void inat_init_tables(void)"
+ print "{"
+
+ # print escape opcode map's array
+ print "\t/* Print Escape opcode map array */"
+ for (i = 0; i < geid; i++)
+ for (j = 0; j < max_lprefix; j++)
+ if (etable[i,j])
+ print "\tinat_escape_tables["i"]["j"] = "etable[i,j]";"
+ print ""
+
+ # print group opcode map's array
+ print "\t/* Print Group opcode map array */"
+ for (i = 0; i < ggid; i++)
+ for (j = 0; j < max_lprefix; j++)
+ if (gtable[i,j])
+ print "\tinat_group_tables["i"]["j"] = "gtable[i,j]";"
+ print ""
+ # print AVX opcode map's array
+ print "\t/* Print AVX opcode map array */"
+ for (i = 0; i < gaid; i++)
+ for (j = 0; j < max_lprefix; j++)
+ if (atable[i,j])
+ print "\tinat_avx_tables["i"]["j"] = "atable[i,j]";"
+
+ print "}"
+ print "#endif"
}

diff --git a/tools/arch/x86/tools/gen-insn-attr-x86.awk b/tools/arch/x86/tools/gen-insn-attr-x86.awk
index a42015b305f4..af38469afd14 100644
--- a/tools/arch/x86/tools/gen-insn-attr-x86.awk
+++ b/tools/arch/x86/tools/gen-insn-attr-x86.awk
@@ -362,6 +362,9 @@ function convert_operands(count,opnd, i,j,imm,mod)
END {
if (awkchecked != "")
exit 1
+
+ print "#ifndef __BOOT_COMPRESSED\n"
+
# print escape opcode map's array
print "/* Escape opcode map array */"
print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
@@ -388,6 +391,51 @@ END {
for (j = 0; j < max_lprefix; j++)
if (atable[i,j])
print " ["i"]["j"] = "atable[i,j]","
- print "};"
+ print "};\n"
+
+ print "#else /* !__BOOT_COMPRESSED */\n"
+
+ print "/* Escape opcode map array */"
+ print "static const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
+ "[INAT_LSTPFX_MAX + 1];"
+ print ""
+
+ print "/* Group opcode map array */"
+ print "static const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
+ "[INAT_LSTPFX_MAX + 1];"
+ print ""
+
+ print "/* AVX opcode map array */"
+ print "static const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
+ "[INAT_LSTPFX_MAX + 1];"
+ print ""
+
+ print "static void inat_init_tables(void)"
+ print "{"
+
+ # print escape opcode map's array
+ print "\t/* Print Escape opcode map array */"
+ for (i = 0; i < geid; i++)
+ for (j = 0; j < max_lprefix; j++)
+ if (etable[i,j])
+ print "\tinat_escape_tables["i"]["j"] = "etable[i,j]";"
+ print ""
+
+ # print group opcode map's array
+ print "\t/* Print Group opcode map array */"
+ for (i = 0; i < ggid; i++)
+ for (j = 0; j < max_lprefix; j++)
+ if (gtable[i,j])
+ print "\tinat_group_tables["i"]["j"] = "gtable[i,j]";"
+ print ""
+ # print AVX opcode map's array
+ print "\t/* Print AVX opcode map array */"
+ for (i = 0; i < gaid; i++)
+ for (j = 0; j < max_lprefix; j++)
+ if (atable[i,j])
+ print "\tinat_avx_tables["i"]["j"] = "atable[i,j]";"
+
+ print "}"
+ print "#endif"
}

--
2.17.1


2020-03-25 15:41:45

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 05/70] x86/insn: Make inat-tables.c suitable for pre-decompression code

+ Masami.

On Thu, Mar 19, 2020 at 10:13:02AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> The inat-tables.c file has some arrays in it that contain pointers to
> other arrays. These pointers need to be relocated when the kernel
> image is moved to a different location.
>
> The pre-decompression boot-code has no support for applying ELF
> relocations, so initialize these arrays at runtime in the
> pre-decompression code to make sure all pointers are correctly
> initialized.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> arch/x86/tools/gen-insn-attr-x86.awk | 50 +++++++++++++++++++++-
> tools/arch/x86/tools/gen-insn-attr-x86.awk | 50 +++++++++++++++++++++-
> 2 files changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
> index a42015b305f4..af38469afd14 100644
> --- a/arch/x86/tools/gen-insn-attr-x86.awk
> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
> @@ -362,6 +362,9 @@ function convert_operands(count,opnd, i,j,imm,mod)
> END {
> if (awkchecked != "")
> exit 1
> +
> + print "#ifndef __BOOT_COMPRESSED\n"
> +
> # print escape opcode map's array
> print "/* Escape opcode map array */"
> print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
> @@ -388,6 +391,51 @@ END {
> for (j = 0; j < max_lprefix; j++)
> if (atable[i,j])
> print " ["i"]["j"] = "atable[i,j]","
> - print "};"
> + print "};\n"
> +
> + print "#else /* !__BOOT_COMPRESSED */\n"
> +
> + print "/* Escape opcode map array */"
> + print "static const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
> + "[INAT_LSTPFX_MAX + 1];"
> + print ""
> +
> + print "/* Group opcode map array */"
> + print "static const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
> + "[INAT_LSTPFX_MAX + 1];"
> + print ""
> +
> + print "/* AVX opcode map array */"
> + print "static const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
> + "[INAT_LSTPFX_MAX + 1];"
> + print ""
> +
> + print "static void inat_init_tables(void)"
> + print "{"
> +
> + # print escape opcode map's array
> + print "\t/* Print Escape opcode map array */"
> + for (i = 0; i < geid; i++)
> + for (j = 0; j < max_lprefix; j++)
> + if (etable[i,j])
> + print "\tinat_escape_tables["i"]["j"] = "etable[i,j]";"
> + print ""
> +
> + # print group opcode map's array
> + print "\t/* Print Group opcode map array */"
> + for (i = 0; i < ggid; i++)
> + for (j = 0; j < max_lprefix; j++)
> + if (gtable[i,j])
> + print "\tinat_group_tables["i"]["j"] = "gtable[i,j]";"
> + print ""
> + # print AVX opcode map's array
> + print "\t/* Print AVX opcode map array */"
> + for (i = 0; i < gaid; i++)
> + for (j = 0; j < max_lprefix; j++)
> + if (atable[i,j])
> + print "\tinat_avx_tables["i"]["j"] = "atable[i,j]";"
> +
> + print "}"
> + print "#endif"
> }
>
> diff --git a/tools/arch/x86/tools/gen-insn-attr-x86.awk b/tools/arch/x86/tools/gen-insn-attr-x86.awk
> index a42015b305f4..af38469afd14 100644
> --- a/tools/arch/x86/tools/gen-insn-attr-x86.awk
> +++ b/tools/arch/x86/tools/gen-insn-attr-x86.awk
> @@ -362,6 +362,9 @@ function convert_operands(count,opnd, i,j,imm,mod)
> END {
> if (awkchecked != "")
> exit 1
> +
> + print "#ifndef __BOOT_COMPRESSED\n"
> +
> # print escape opcode map's array
> print "/* Escape opcode map array */"
> print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
> @@ -388,6 +391,51 @@ END {
> for (j = 0; j < max_lprefix; j++)
> if (atable[i,j])
> print " ["i"]["j"] = "atable[i,j]","
> - print "};"
> + print "};\n"
> +
> + print "#else /* !__BOOT_COMPRESSED */\n"
> +
> + print "/* Escape opcode map array */"
> + print "static const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
> + "[INAT_LSTPFX_MAX + 1];"
> + print ""
> +
> + print "/* Group opcode map array */"
> + print "static const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
> + "[INAT_LSTPFX_MAX + 1];"
> + print ""
> +
> + print "/* AVX opcode map array */"
> + print "static const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
> + "[INAT_LSTPFX_MAX + 1];"
> + print ""
> +
> + print "static void inat_init_tables(void)"
> + print "{"
> +
> + # print escape opcode map's array
> + print "\t/* Print Escape opcode map array */"
> + for (i = 0; i < geid; i++)
> + for (j = 0; j < max_lprefix; j++)
> + if (etable[i,j])
> + print "\tinat_escape_tables["i"]["j"] = "etable[i,j]";"
> + print ""
> +
> + # print group opcode map's array
> + print "\t/* Print Group opcode map array */"
> + for (i = 0; i < ggid; i++)
> + for (j = 0; j < max_lprefix; j++)
> + if (gtable[i,j])
> + print "\tinat_group_tables["i"]["j"] = "gtable[i,j]";"
> + print ""
> + # print AVX opcode map's array
> + print "\t/* Print AVX opcode map array */"
> + for (i = 0; i < gaid; i++)
> + for (j = 0; j < max_lprefix; j++)
> + if (atable[i,j])
> + print "\tinat_avx_tables["i"]["j"] = "atable[i,j]";"
> +
> + print "}"
> + print "#endif"
> }
>
> --
> 2.17.1
>

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2020-03-27 03:03:52

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 05/70] x86/insn: Make inat-tables.c suitable for pre-decompression code

Hi,

On Wed, 25 Mar 2020 16:39:45 +0100
Borislav Petkov <[email protected]> wrote:

> + Masami.
>
> On Thu, Mar 19, 2020 at 10:13:02AM +0100, Joerg Roedel wrote:
> > From: Joerg Roedel <[email protected]>
> >
> > The inat-tables.c file has some arrays in it that contain pointers to
> > other arrays. These pointers need to be relocated when the kernel
> > image is moved to a different location.
> >
> > The pre-decompression boot-code has no support for applying ELF
> > relocations, so initialize these arrays at runtime in the
> > pre-decompression code to make sure all pointers are correctly
> > initialized.

I need to check the whole series, but as far as I can understand from
this patch, this seems not allowing to store the address value in
static pointers. It may break more things, for example _kprobe_blacklist
records the NOKPROBE_SYMBOL() symbol addresses at the build time.

I have some comments here.

> > Signed-off-by: Joerg Roedel <[email protected]>
> > ---
> > arch/x86/tools/gen-insn-attr-x86.awk | 50 +++++++++++++++++++++-
> > tools/arch/x86/tools/gen-insn-attr-x86.awk | 50 +++++++++++++++++++++-
> > 2 files changed, 98 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
> > index a42015b305f4..af38469afd14 100644
> > --- a/arch/x86/tools/gen-insn-attr-x86.awk
> > +++ b/arch/x86/tools/gen-insn-attr-x86.awk
> > @@ -362,6 +362,9 @@ function convert_operands(count,opnd, i,j,imm,mod)
> > END {
> > if (awkchecked != "")
> > exit 1
> > +
> > + print "#ifndef __BOOT_COMPRESSED\n"
> > +
> > # print escape opcode map's array
> > print "/* Escape opcode map array */"
> > print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
> > @@ -388,6 +391,51 @@ END {
> > for (j = 0; j < max_lprefix; j++)
> > if (atable[i,j])
> > print " ["i"]["j"] = "atable[i,j]","
> > - print "};"
> > + print "};\n"
> > +
> > + print "#else /* !__BOOT_COMPRESSED */\n"

I think the definitions of inat_*_tables can be shared in both case.
If __BOOT_COMPRESSED is set, we can define inat_init_tables() as a
initialize function, and if not, it will be just a dummy "do {} while (0)".

BTW, where is the __BOOT_COMPRESSED defined?

> > +
> > + print "/* Escape opcode map array */"
> > + print "static const insn_attr_t *inat_escape_tables[INAT_ESC_MAX + 1]" \
> > + "[INAT_LSTPFX_MAX + 1];"
> > + print ""
> > +
> > + print "/* Group opcode map array */"
> > + print "static const insn_attr_t *inat_group_tables[INAT_GRP_MAX + 1]"\
> > + "[INAT_LSTPFX_MAX + 1];"
> > + print ""
> > +
> > + print "/* AVX opcode map array */"
> > + print "static const insn_attr_t *inat_avx_tables[X86_VEX_M_MAX + 1]"\
> > + "[INAT_LSTPFX_MAX + 1];"
> > + print ""
> > +
> > + print "static void inat_init_tables(void)"

This functions should be "inline".
And I can not see the call-site of inat_init_tables() in this patch.

If possible, please include call-site with definition (especially
new init function) so that I can check the init call timing too.

> > + print "{"
> > +
> > + # print escape opcode map's array
> > + print "\t/* Print Escape opcode map array */"
> > + for (i = 0; i < geid; i++)
> > + for (j = 0; j < max_lprefix; j++)
> > + if (etable[i,j])
> > + print "\tinat_escape_tables["i"]["j"] = "etable[i,j]";"
> > + print ""
> > +
> > + # print group opcode map's array
> > + print "\t/* Print Group opcode map array */"
> > + for (i = 0; i < ggid; i++)
> > + for (j = 0; j < max_lprefix; j++)
> > + if (gtable[i,j])
> > + print "\tinat_group_tables["i"]["j"] = "gtable[i,j]";"
> > + print ""
> > + # print AVX opcode map's array
> > + print "\t/* Print AVX opcode map array */"
> > + for (i = 0; i < gaid; i++)
> > + for (j = 0; j < max_lprefix; j++)
> > + if (atable[i,j])
> > + print "\tinat_avx_tables["i"]["j"] = "atable[i,j]";"
> > +
> > + print "}"
> > + print "#endif"
> > }

The code itself looks good to me.

Thank you,

--
Masami Hiramatsu <[email protected]>

2020-04-16 15:27:04

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 05/70] x86/insn: Make inat-tables.c suitable for pre-decompression code

Hi Masami,

On Fri, Mar 27, 2020 at 12:02:32PM +0900, Masami Hiramatsu wrote:
> On Wed, 25 Mar 2020 16:39:45 +0100
> Borislav Petkov <[email protected]> wrote:
>
> > + Masami.
> >
> > On Thu, Mar 19, 2020 at 10:13:02AM +0100, Joerg Roedel wrote:
> > > From: Joerg Roedel <[email protected]>
> > >
> > > The inat-tables.c file has some arrays in it that contain pointers to
> > > other arrays. These pointers need to be relocated when the kernel
> > > image is moved to a different location.
> > >
> > > The pre-decompression boot-code has no support for applying ELF
> > > relocations, so initialize these arrays at runtime in the
> > > pre-decompression code to make sure all pointers are correctly
> > > initialized.
>
> I need to check the whole series, but as far as I can understand from
> this patch, this seems not allowing to store the address value in
> static pointers. It may break more things, for example _kprobe_blacklist
> records the NOKPROBE_SYMBOL() symbol addresses at the build time.

The runtime-initialization function is only used in the
pre-decompression boot code (arch/x86/boot/compressed/) which is not
part of the running kernel image. At that stage of booting there is no
support for kprobe or tracing or any other neat features that might
break things here.


> > > + print "#ifndef __BOOT_COMPRESSED\n"
> > > +
> > > # print escape opcode map's array
> > > print "/* Escape opcode map array */"
> > > print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
> > > @@ -388,6 +391,51 @@ END {
> > > for (j = 0; j < max_lprefix; j++)
> > > if (atable[i,j])
> > > print " ["i"]["j"] = "atable[i,j]","
> > > - print "};"
> > > + print "};\n"
> > > +
> > > + print "#else /* !__BOOT_COMPRESSED */\n"
>
> I think the definitions of inat_*_tables can be shared in both case.
> If __BOOT_COMPRESSED is set, we can define inat_init_tables() as a
> initialize function, and if not, it will be just a dummy "do {} while (0)".

The inat_*_tables are all declared const, so this way it is not possible
to change them at runtime. For the running kernel image this is fine, as
there are ELF relocations which fix things up, but at the
pre-decompression boot stage there are no ELF relocations which can fix
the tables, so the pointers in there need to be initialized at runtime.

> BTW, where is the __BOOT_COMPRESSED defined?

It is defined in arch/x86/boot/compressed/sev-es.c by patch

x86/boot/compressed/64: Setup GHCB Based VC Exception handler

which also includes parts of the instruction decoder into the
pre-decompression boot code and adds the only call-site for
inat_init_tables().

> > > + print "static void inat_init_tables(void)"
>
> This functions should be "inline".
> And I can not see the call-site of inat_init_tables() in this patch.

The call-site is added with the patch that includes the
instruction decoder into the pre-decompression code. If possible I'd
like to keep those things separate, as both patches are already pretty
big by themselfes (and they do different things, in different parts of
the code).

> If possible, please include call-site with definition (especially
> new init function) so that I can check the init call timing too.

The function is called at the first #VC exception after a GHCB has been
set up. Call-path is: boot_vc_handler -> sev_es_setup_ghcb ->
inat_init_tables.

See

https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/tree/arch/x86/boot/compressed/sev-es.c?h=sev-es-client-v5.6-rc6

for the full code there.

Thanks,

Joerg

2020-04-17 12:51:54

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 05/70] x86/insn: Make inat-tables.c suitable for pre-decompression code

On Thu, 16 Apr 2020 17:24:06 +0200
Joerg Roedel <[email protected]> wrote:

> Hi Masami,
>
> On Fri, Mar 27, 2020 at 12:02:32PM +0900, Masami Hiramatsu wrote:
> > On Wed, 25 Mar 2020 16:39:45 +0100
> > Borislav Petkov <[email protected]> wrote:
> >
> > > + Masami.
> > >
> > > On Thu, Mar 19, 2020 at 10:13:02AM +0100, Joerg Roedel wrote:
> > > > From: Joerg Roedel <[email protected]>
> > > >
> > > > The inat-tables.c file has some arrays in it that contain pointers to
> > > > other arrays. These pointers need to be relocated when the kernel
> > > > image is moved to a different location.
> > > >
> > > > The pre-decompression boot-code has no support for applying ELF
> > > > relocations, so initialize these arrays at runtime in the
> > > > pre-decompression code to make sure all pointers are correctly
> > > > initialized.
> >
> > I need to check the whole series, but as far as I can understand from
> > this patch, this seems not allowing to store the address value in
> > static pointers. It may break more things, for example _kprobe_blacklist
> > records the NOKPROBE_SYMBOL() symbol addresses at the build time.
>
> The runtime-initialization function is only used in the
> pre-decompression boot code (arch/x86/boot/compressed/) which is not
> part of the running kernel image. At that stage of booting there is no
> support for kprobe or tracing or any other neat features that might
> break things here.

Ah, I got it. So you intended to port the instruction decoder to
pre-decompression boot code, correct?

> > > > + print "#ifndef __BOOT_COMPRESSED\n"
> > > > +
> > > > # print escape opcode map's array
> > > > print "/* Escape opcode map array */"
> > > > print "const insn_attr_t * const inat_escape_tables[INAT_ESC_MAX + 1]" \
> > > > @@ -388,6 +391,51 @@ END {
> > > > for (j = 0; j < max_lprefix; j++)
> > > > if (atable[i,j])
> > > > print " ["i"]["j"] = "atable[i,j]","
> > > > - print "};"
> > > > + print "};\n"
> > > > +
> > > > + print "#else /* !__BOOT_COMPRESSED */\n"
> >
> > I think the definitions of inat_*_tables can be shared in both case.
> > If __BOOT_COMPRESSED is set, we can define inat_init_tables() as a
> > initialize function, and if not, it will be just a dummy "do {} while (0)".
>
> The inat_*_tables are all declared const, so this way it is not possible
> to change them at runtime.

Indeed.

> For the running kernel image this is fine, as
> there are ELF relocations which fix things up, but at the
> pre-decompression boot stage there are no ELF relocations which can fix
> the tables, so the pointers in there need to be initialized at runtime.

OK.

>
> > BTW, where is the __BOOT_COMPRESSED defined?
>
> It is defined in arch/x86/boot/compressed/sev-es.c by patch
>
> x86/boot/compressed/64: Setup GHCB Based VC Exception handler
>
> which also includes parts of the instruction decoder into the
> pre-decompression boot code and adds the only call-site for
> inat_init_tables().

Thanks, I understand it.

>
> > > > + print "static void inat_init_tables(void)"
> >
> > This functions should be "inline".
> > And I can not see the call-site of inat_init_tables() in this patch.
>
> The call-site is added with the patch that includes the
> instruction decoder into the pre-decompression code. If possible I'd
> like to keep those things separate, as both patches are already pretty
> big by themselfes (and they do different things, in different parts of
> the code).

OK, if you will send v2, please CC both to me.

>
> > If possible, please include call-site with definition (especially
> > new init function) so that I can check the init call timing too.
>
> The function is called at the first #VC exception after a GHCB has been
> set up. Call-path is: boot_vc_handler -> sev_es_setup_ghcb ->
> inat_init_tables.

sound good to me.

Thank you,

>
> See
>
> https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/tree/arch/x86/boot/compressed/sev-es.c?h=sev-es-client-v5.6-rc6
>
> for the full code there.
>
> Thanks,
>
> Joerg


--
Masami Hiramatsu <[email protected]>

2020-04-17 13:43:55

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 05/70] x86/insn: Make inat-tables.c suitable for pre-decompression code

On Fri, Apr 17, 2020 at 09:50:00PM +0900, Masami Hiramatsu wrote:
> On Thu, 16 Apr 2020 17:24:06 +0200
> Joerg Roedel <[email protected]> wrote:

> Ah, I got it. So you intended to port the instruction decoder to
> pre-decompression boot code, correct?

Right, it is needed there to decode instructions which cause #VC
exceptions when running as an SEV-ES guest.

> > The call-site is added with the patch that includes the
> > instruction decoder into the pre-decompression code. If possible I'd
> > like to keep those things separate, as both patches are already pretty
> > big by themselfes (and they do different things, in different parts of
> > the code).
>
> OK, if you will send v2, please CC both to me.

Will do, I added you to the cc-list for future posts of this series.


Regards,

Joerg