Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
Message-ID: <0d540a6b-8838-bdf5-ddad-f3b9576ca9f2@redhat.com>
Date:   Thu, 9 Dec 2021 18:52:01 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: [PATCH] KVM: x86: Always set kvm_run->if_flag
Content-Language: en-US
To:     Marc Orr <marcorr@google.com>,
        Tom Lendacky <Thomas.Lendacky@amd.com>
Cc:     seanjc@google.com, vkuznets@redhat.com, wanpengli@tencent.com,
        jmattson@google.com, joro@8bytes.org, tglx@linutronix.de,
        mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
        x86@kernel.org, hpa@zytor.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20211207043100.3357474-1-marcorr@google.com>
 <c8889028-9c4e-cade-31b6-ea92a32e4f66@amd.com>
 <CAA03e5E7-ns7w9B9Tu7pSWzCo0Nh7Ba5jwQXcn_XYPf_reRq9Q@mail.gmail.com>
 <5e69c0ca-389c-3ace-7559-edd901a0ab3c@amd.com>
 <CAA03e5Gf=ZsAKhuLCEtYCCf0UuNXSHRXQHgmjOj3MKtbiSMbqQ@mail.gmail.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <CAA03e5Gf=ZsAKhuLCEtYCCf0UuNXSHRXQHgmjOj3MKtbiSMbqQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/7/21 18:28, Marc Orr wrote:
>>>>> +static bool svm_get_if_flag(struct kvm_vcpu *vcpu)
>>>>> +{
>>>>> +     struct vmcb *vmcb = to_svm(vcpu)->vmcb;
>>>>> +
>>>>> +     return !!(vmcb->control.int_state & SVM_GUEST_INTERRUPT_MASK);
>>>> I'm not sure if this is always valid to use for non SEV-ES guests. Maybe
>>>> the better thing would be:
>>>>
>>>>           return sev_es_guest(vcpu->kvm) ? vmcb->control.int_state & SVM_GUEST_INTERRUPT_MASK
>>>>                                          : kvm_get_rflags(vcpu) & X86_EFLAGS_IF;
>>>>
>>>> (Since this function returns a bool, I don't think you need the !!)
>>>
>>> I had the same reservations when writing the patch. (Why fix what's
>>> not broken.) The reason I wrote the patch this way is based on what I
>>> read in APM vol2: Appendix B Layout of VMCB: "GUEST_INTERRUPT_MASK -
>>> Value of the RFLAGS.IF bit for the guest."
>>
>> I just verified with the hardware team that this flag is indeed only set
>> for a guest with protected state (SEV-ES / SEV-SNP). An update to the APM
>> will be made.
>
> Got it now. Then the change you suggested is a must! Thanks, Tom.

Besides, the bit wouldn't have existed on old (pre-SEV-ES) processors.

Paolo