On Mon, Sep 24, 2018 at 6:31 AM, Sean Christopherson <sean.j.christopherson@xxxxxxxxx> wrote: > On Thu, 2018-08-30 at 16:05 -0700, Jim Mattson wrote: >> As specified in Intel's SDM, do not allow the L1 hypervisor to launch >> an L2 guest with the VM-execution controls for "unrestricted guest" or >> "mode-based execute control for EPT" set and the VM-execution control >> for "enable EPT" clear. >> >> Note that the VM-execution control for "mode-based execute control for >> EPT" is not yet virtualized by kvm. > > The EPT violation #VE control also falls into this category. Though that makes sense to me, the "enable EPT" requirement isn't documented. > nested_vmx_check_ept_enable() is a bit weird, it makes it sound like > we're always requiring EPT to be enabled. And checking for EPT being > enabled isn't unique to this function, e.g. EPT is also required for > PML and is checked by nested_vmx_check_pml_controls(). And EPTP switching... I was just trying to implement one bullet point from section 26.2.1.1 of the SDM: o If either the “unrestricted guest” VM-execution control or the “mode-based execute control for EPT” VM-execution control is 1, the “enable EPT” VM-execution control must also be 1. > That being said, I actually like the approach of checking all controls > that depend EPT in a single function, i.e. what about moving the EPT > part of the PML check to this function? I think that mixing the checks for multiple bullet points from the SDM makes it harder to verify that all of the checks have been properly implemented. However, the obvious "nested_vmx_check_unrestricted_guest_or_mode_based_exec_controls" seems a bit long. I'd rather split one bullet point into multiple checks than merge portions of multiple bullet points into a single check. Maybe nested_vmx_check_unrestricted_guest_controls and nested_vmx_check_mode_based_exec_controls? > What about borrowing the style of nested_vmx_check_shadow_vmcs_controls() > and returning 0 immediately if nested_cpu_has_ept() is true?! Sure.
