[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 1/1] target/i386: Mask xstate_bv based on the cpu enabled
From: |
Leonardo Brás |
Subject: |
Re: [PATCH v1 1/1] target/i386: Mask xstate_bv based on the cpu enabled features |
Date: |
Tue, 01 Feb 2022 16:09:57 -0300 |
User-agent: |
Evolution 3.42.3 |
Hello David, thanks for this feedback!
On Mon, 2022-01-31 at 12:53 +0000, David Edmondson wrote:
> On Saturday, 2022-01-29 at 06:46:45 -03, Leonardo Bras wrote:
>
> > The following steps describe a migration bug:
> > 1 - Bring up a VM with -cpu EPYC on a host with EPYC-Milan cpu
> > 2 - Migrate to a host with EPYC-Naples cpu
> >
> > The guest kernel crashes shortly after the migration.
> >
> > The crash happens due to a fault caused by XRSTOR:
> > A set bit in XSTATE_BV is not set in XCR0.
> > The faulting bit is FEATURE_PKRU (enabled in Milan, but not in
> > Naples)
>
> I'm trying to understand how this happens.
>
> If we boot on EPYC-Milan with "-cpu EPYC", the PKRU feature should
> not
> be exposed to the VM (it is not available in the EPYC CPU).
>
> Given this, how would bit 0x200 (representing PKRU) end up set in
> xstate_bv?
During my debug, I noticed this bit gets set before the kernel even
starts.
It's possible Seabios and/or IPXE are somehow setting 0x200 using the
xrstor command. I am not sure if qemu is able to stop this in KVM mode.
If you have any info on this, please let me know.
Best regards,
Leo
>
> > To avoid this kind of bug:
> > In kvm_get_xsave, mask-out from xstate_bv any bits that are not set
> > in
> > current vcpu's features.
> >
> > This keeps cpu->env->xstate_bv with feature bits compatible with
> > any
> > host machine capable of running the vcpu model.
> >
> > Signed-off-by: Leonardo Bras <leobras@redhat.com>
> > ---
> > target/i386/xsave_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/i386/xsave_helper.c
> > b/target/i386/xsave_helper.c
> > index ac61a96344..0628226234 100644
> > --- a/target/i386/xsave_helper.c
> > +++ b/target/i386/xsave_helper.c
> > @@ -167,7 +167,7 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu,
> > const void *buf, uint32_t buflen)
> > env->xmm_regs[i].ZMM_Q(1) = ldq_p(xmm + 8);
> > }
> >
> > - env->xstate_bv = header->xstate_bv;
> > + env->xstate_bv = header->xstate_bv & env-
> > >features[FEAT_XSAVE_COMP_LO];
> >
> > e = &x86_ext_save_areas[XSTATE_YMM_BIT];
> > if (e->size && e->offset) {
>
> dme.