On Fri, Dec 13, 2024 at 3:33 PM Hou Wenlong <houwenlong.hwl@xxxxxxxxxxxx> wrote:
On Fri, Dec 13, 2024 at 02:48:13PM +0800, lijiang wrote:
> On Wed, Dec 11, 2024 at 10:22 AM <devel-request@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> wrote:
>
> > Date: Tue, 10 Dec 2024 20:18:43 +0800
> > From: "Hou Wenlong" <houwenlong.hwl@xxxxxxxxxxxx>
> > Subject: [PATCH] x86_64: Mark #VC stack unavailable
> > when CONFIG_AMD_MEM_ENCRYPT is not set
> > To: devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
> > Cc: Hou Wenlong <houwenlong.hwl@xxxxxxxxxxxx>
> > Message-ID: <f1fca9918841de9a48f428d498f50964f1dab543.1733831073.git.h
> > ouwenlong.hwl@xxxxxxxxxxxx>
> >
> > When 'CONFIG_AMD_MEM_ENCRYPT' is not set, the IDT handler for #VC is not
> > registered, but the address of the #VC IST stack is still set in
> > tss_setup_ist(). Therefore, the name of the associated exception stack
> > is "UNKNOWN" instead of "VC". Although the base of the exception stack
> > is not zero and is available, it is not accessible, which will cause the
> > backtrace to fail when attempting to access the #VC stack. To fix this,
> > remove the name check.
> >
> >
> Thank you for the fix, Hou.
>
>
> > Signe-off-by: Hou Wenlong <houwenlong.hwl@xxxxxxxxxxxx>
> > ---
> > x86_64.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/x86_64.c b/x86_64.c
> > index e7f8fe25b31f..ee12ba095e6e 100644
> > --- a/x86_64.c
> > +++ b/x86_64.c
> > @@ -1540,7 +1540,7 @@ x86_64_ist_init(void)
> >
> > ms->stkinfo.available[c][i] = TRUE;
> > /* VC stack can be unmapped if SEV-ES is
> > disabled or not supported. */
> > - if (STREQ(ms->stkinfo.exception_stacks[i],
> > "VC") &&
> > + if (ms->stkinfo.ebase[c][i] &&
> >
> Can you help double check if the above condition is redundant in your case?
> Seems the check '!accessible(ms->stkinfo.ebase[c][i])' should be enough.
>
Hi Lianbo,
Yes, checking '!accessible(ms->stkinfo.ebase[c][i])' is sufficient. I
added the extra check because I noticed that a value of 0 seems to be
invalid for 'ms->stkinfo.ebase[c][i]', and there is a separate check for
'ms->stkinfo.ebase[c][i] != 0' before it is used in other places. If we
remove the check, it could potentially be rewritten as:
diff --git a/x86_64.c b/x86_64.c
index e7f8fe25b31f..c254c6ec7576 100644
--- a/x86_64.c
+++ b/x86_64.c
@@ -1538,11 +1538,8 @@ x86_64_ist_init(void)
if (ms->stkinfo.ebase[c][i])
ms->stkinfo.ebase[c][i] -=
ms->stkinfo.esize[i];
- ms->stkinfo.available[c][i] = TRUE;
/* VC stack can be unmapped if SEV-ES is
* disabled or not supported. */
- if
(STREQ(ms->stkinfo.exception_stacks[i],
"VC") &&
- !accessible(ms->stkinfo.ebase[c][i]))
- ms->stkinfo.available[c][i] =
FALSE;
+ ms->stkinfo.available[c][i] =
accessible(ms->stkinfo.ebase[c][i]);
}
}
This looks good to me. Thanks, Hou.
Thanks!
> Thanks
> Lianbo
>
> !accessible(ms->stkinfo.ebase[c][i]))
> > ms->stkinfo.available[c][i] =
> > FALSE;
> > }
> >
> > base-commit: f13853cef53f5c5463a51021edbc81977e2b1405
> > --
> > 2.31.1
> >
-- Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
