Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
To:     Sean Christopherson <seanjc@google.com>,
        Ashish Kalra <Ashish.Kalra@amd.com>
Cc:     tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
        joro@8bytes.org, bp@suse.de, thomas.lendacky@amd.com,
        x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        srutherford@google.com, venu.busireddy@oracle.com,
        brijesh.singh@amd.com
References: <cover.1618498113.git.ashish.kalra@amd.com>
 <3232806199b2f4b307d28f6fd4f756d487b4e482.1618498113.git.ashish.kalra@amd.com>
 <YH4M5guOafToCWd7@google.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v13 10/12] KVM: x86: Introduce new
 KVM_FEATURE_SEV_LIVE_MIGRATION feature & Custom MSR.
Message-ID: <988e38b4-2f69-ff66-eac9-e1714c049867@redhat.com>
Date:   Tue, 20 Apr 2021 12:49:41 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.0
MIME-Version: 1.0
In-Reply-To: <YH4M5guOafToCWd7@google.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 20/04/21 01:06, Sean Christopherson wrote:
>> diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
>> index 950afebfba88..f6bfa138874f 100644
>> --- a/arch/x86/include/uapi/asm/kvm_para.h
>> +++ b/arch/x86/include/uapi/asm/kvm_para.h
>> @@ -33,6 +33,7 @@
>>   #define KVM_FEATURE_PV_SCHED_YIELD	13
>>   #define KVM_FEATURE_ASYNC_PF_INT	14
>>   #define KVM_FEATURE_MSI_EXT_DEST_ID	15
>> +#define KVM_FEATURE_SEV_LIVE_MIGRATION	16
>>   
>>   #define KVM_HINTS_REALTIME      0
>>   
>> @@ -54,6 +55,7 @@
>>   #define MSR_KVM_POLL_CONTROL	0x4b564d05
>>   #define MSR_KVM_ASYNC_PF_INT	0x4b564d06
>>   #define MSR_KVM_ASYNC_PF_ACK	0x4b564d07
>> +#define MSR_KVM_SEV_LIVE_MIGRATION	0x4b564d08
>>   
>>   struct kvm_steal_time {
>>   	__u64 steal;
>> @@ -136,4 +138,6 @@ struct kvm_vcpu_pv_apf_data {
>>   #define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
>>   #define KVM_PV_EOI_DISABLED 0x0
>>   
>> +#define KVM_SEV_LIVE_MIGRATION_ENABLED BIT_ULL(0)
> 
> Even though the intent is to "force" userspace to intercept the MSR, I think KVM
> should at least emulate the legal bits as a nop.  Deferring completely to
> userspace is rather bizarre as there's not really anything to justify KVM
> getting involved.  It would also force userspace to filter the MSR just to
> support the hypercall.

I think this is the intention, the hypercall by itself cannot do much if
you cannot tell userspace that it's up-to-date.

On the other hand it is kind of wrong that KVM_GET_SUPPORTED_CPUID
returns the feature, but the MSR is not supported.

> Somewhat of a nit, but I think we should do something like s/ENABLED/READY,

Agreed.  I'll send a patch that puts everything together.

Paolo