Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <da8692bd-b0a7-4f53-8600-ac3fb277b3a1@redhat.com>
Date:   Mon, 7 Aug 2023 09:00:03 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v5 11/19] KVM:VMX: Emulate read and write to CET MSRs
Content-Language: en-US
To:     "Yang, Weijiang" <weijiang.yang@intel.com>,
        Sean Christopherson <seanjc@google.com>,
        Chao Gao <chao.gao@intel.com>
Cc:     peterz@infradead.org, john.allen@amd.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, rick.p.edgecombe@intel.com,
        binbin.wu@linux.intel.com
References: <20230803042732.88515-1-weijiang.yang@intel.com>
 <20230803042732.88515-12-weijiang.yang@intel.com>
 <ZMyJIq4CgXxudJED@chao-email> <ZM1tNJ9ZdQb+VZVo@google.com>
 <7df23a0d-e2a6-71a7-7641-6363f4905f5c@intel.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <7df23a0d-e2a6-71a7-7641-6363f4905f5c@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 8/6/23 10:44, Yang, Weijiang wrote:
>> Similar to my suggestsion for XSS, I think we drop the waiver for 
>> host_initiated
>> accesses, i.e. require the feature to be enabled and exposed to the 
>> guest, even
>> for the host.
>
> I saw Paolo shares different opinion on this, so would hold on for a 
> while...

It's not *so* different: the host initiated access should be allowed, 
but it should only allow writing zero.  So, something like:

> +static bool kvm_cet_is_msr_accessible(struct kvm_vcpu *vcpu,
> +                      struct msr_data *msr)
> +{

bool host_msr_reset =
	msr->host_initiated && msr->data == 0;

and then below you use host_msr_reset instead of msr->host_initiated.

> +        if (msr->index == MSR_KVM_GUEST_SSP)
> +            return msr->host_initiated;
> +
> +        return msr->host_initiated ||
> +            guest_cpuid_has(vcpu, X86_FEATURE_SHSTK);

This can be unified like this:

return
	(host_msr_reset || guest_cpuid_has(vcpu, X86_FEATURE_SHSTK)) &&
	(msr->index != MSR_KVM_GUEST_SSP || msr->host_initiated);

> +    }
> +
> +    if (!kvm_cpu_cap_has(X86_FEATURE_SHSTK) &&
> +        !kvm_cpu_cap_has(X86_FEATURE_IBT))
> +        return false;
> +
> +    return msr->host_initiated ||
> +        guest_cpuid_has(vcpu, X86_FEATURE_IBT) ||
> +        guest_cpuid_has(vcpu, X86_FEATURE_SHSTK); 

while this can simply use host_msr_reset.

Paolo