On Mon, Jun 19, 2023, Weijiang Yang wrote: > > On 6/17/2023 1:56 AM, Sean Christopherson wrote: > > On Fri, Jun 16, 2023, Weijiang Yang wrote: > > > On 6/16/2023 7:30 AM, Sean Christopherson wrote: > > > > On Thu, May 11, 2023, Yang Weijiang wrote: > > > > > The last patch is introduced to support supervisor SHSTK but the feature is > > > > > not enabled on Intel platform for now, the main purpose of this patch is to > > > > > facilitate AMD folks to enable the feature. > > > > I am beyond confused by the SDM's wording of CET_SSS. > > > > > > > > First, it says that CET_SSS says the CPU isn't buggy (or maybe "less buggy" is > > > > more appropriate phrasing). > > > > > > > > Bit 18: CET_SSS. If 1, indicates that an operating system can enable supervisor > > > > shadow stacks as long as it ensures that certain supervisor shadow-stack pushes > > > > will not cause page faults (see Section 17.2.3 of the Intel® 64 and IA-32 > > > > Architectures Software Developer’s Manual, Volume 1). > > > > > > > > But then it says says VMMs shouldn't set the bit. > > > > > > > > When emulating the CPUID instruction, a virtual-machine monitor should return > > > > this bit as 0 if those pushes can cause VM exits. > > > > > > > > Based on the Xen code (which is sadly a far better source of information than the > > > > SDM), I *think* that what the SDM is trying to say is that VMMs should not set > > > > CET_SS if VM-Exits can occur ***and*** the bit is not set in the host CPU. Because > > > > if the SDM really means "VMMs should never set the bit", then what on earth is the > > > > point of the bit. > > > I need to double check for the vague description. > > > > > > From my understanding, on bare metal side, if the bit is 1, OS can enable > > > SSS if pushes won't cause page fault. But for VM case, it's not recommended > > > (regardless of the bit state) to set the bit as vm-exits caused by guest SSS > > > pushes cannot be fully excluded. > > > > > > In other word, the bit is mainly for bare metal guidance now. > > > > > > > > In summary, this new series enables CET user SHSTK/IBT and kernel IBT, but > > > > > doesn't fully support CET supervisor SHSTK, the enabling work is left for > > > > > the future. > > > > Why? If my interpretation of the SDM is correct, then all the pieces are there. > > ... > > > > > And also based on above SDM description, I don't want to add the support > > > blindly now. > > *sigh* > > > > I got filled in on the details offlist. > > > > 1) In the next version of this series, please rework it to reincorporate Supervisor > > Shadow Stack support into the main series, i.e. pretend Intel's implemenation > > isn't horribly flawed. > > Let me make it clear, you want me to do two things: > > 1)Add Supervisor Shadow Stack state support(i.e., XSS.bit12(CET_S)) into > kernel so that host can support guest Supervisor Shadow Stack MSRs in g/h FPU > context switch. If that's necessary for correct functionality, yes. > 2) Add Supervisor Shadow stack support into KVM part so that guest OS is > able to use SSS with risk. Yes. Architecturally, if KVM advertises X86_FEATURE_SHSTK, then KVM needs to provide both User and Supervisor support. CET_SSS doesn't change the architecture, it's little more than a hint. And even if the guest follows SDM's recommendation to not enable shadow stacks, a clever kernel can still utilize SSS assets, e.g. use the MSRs as scratch registers.
