Re: [PATCH v10 00/26] Control-flow Enforcement: Shadow Stack

Sean Christopherson
2020-07-24T04:51:02Z

On Thu, Jul 23, 2020 at 08:40:33PM -0700, Yu-cheng Yu wrote:
> On Thu, 2020-07-23 at 11:41 -0700, Dave Hansen wrote:
> > On 7/23/20 9:56 AM, Sean Christopherson wrote:
> > > On Thu, Jul 23, 2020 at 09:41:37AM -0700, Dave Hansen wrote:
> > > > On 7/23/20 9:25 AM, Sean Christopherson wrote:
> > > > > How would people feel about taking the above two patches (02 and 03 in the
> > > > > series) through the KVM tree to enable KVM virtualization of CET before the
> > > > > kernel itself gains CET support? I.e. add the MSR and feature bits, along
> > > > > with the XSAVES context switching. The feature definitons could use "" to
> > > > > suppress displaying them in /proc/cpuinfo to avoid falsely advertising CET
> > > > > to userspace.
> > > > >
> > > > > AIUI, there are ABI issues that need to be sorted out, and that is likely
> > > > > going to drag on for some time.
> > > > >
> > > > > Is this a "hell no" sort of idea, or something that would be feasible if we
> > > > > can show that there are no negative impacts to the kernel?
> > > > Negative impacts like bloating every task->fpu with XSAVE state that
> > > > will never get used? ;)
> > > Gah, should have qualified that with "meaningful or measurable negative
> > > impacts". E.g. the extra 40 bytes for CET XSAVE state seems like it would
> > > be acceptable overhead, but noticeably increasing the latency of XSAVES
> > > and/or XRSTORS would not be acceptable.
> >
> > It's 40 bytes, but it's 40 bytes of just pure, unadulterated waste. It
> > would have no *chance* of being used. It's also quite precisely
> > measurable on a given system:
> >
> > cat /proc/slabinfo | grep task_struct | awk '{print $3 * 40}'
>
> If there is value in getting these two patches merged first, we can move
> XFEATURE_MASK_CET_USER to XFEATURE_MASK_SUPERVISOR_UNSUPPORTED for now, until
> CET is eventually merged. That way, there is no space wasted.

Merging them as disabled wouldn't help, KVM needs the features enabled so
that context switching the guest state works as expected.