Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2] kvm/svm: PKU not currently supported
To:     Sean Christopherson <sean.j.christopherson@intel.com>,
        John Allen <john.allen@amd.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        rkrcmar@redhat.com, vkuznets@redhat.com
References: <20191219201759.21860-1-john.allen@amd.com>
 <20191219203214.GC6439@linux.intel.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <8a77e3b9-049e-e622-9332-9bebb829bc3d@redhat.com>
Date:   Fri, 20 Dec 2019 10:25:16 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.1
MIME-Version: 1.0
In-Reply-To: <20191219203214.GC6439@linux.intel.com>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 19/12/19 21:32, Sean Christopherson wrote:
> On Thu, Dec 19, 2019 at 02:17:59PM -0600, John Allen wrote:
>> Current SVM implementation does not have support for handling PKU. Guests
>> running on a host with future AMD cpus that support the feature will read
>> garbage from the PKRU register and will hit segmentation faults on boot as
>> memory is getting marked as protected that should not be. Ensure that cpuid
>> from SVM does not advertise the feature.
>>
>> Signed-off-by: John Allen <john.allen@amd.com>
>> ---
>> v2:
>>   -Introduce kvm_x86_ops->pku_supported()
> 
> I like the v1 approach better, it's less code to unwind when SVM gains
> support for virtualizaing PKU.
> 
> The existing cases of kvm_x86_ops->*_supported() in __do_cpuid_func() are
> necessary to handle cases where it may not be possible to expose a feature
> even though it's supported in hardware, host and KVM, e.g. VMX's separate
> MSR-based features and PT's software control to hide it from guest.  In
> this case, hiding PKU is purely due to lack of support in KVM.  The SVM
> series to enable PKU can then delete a single line of SVM code instead of
> having to go back in and do surgery on x86 and VMX.
> 

I sort of liked the V1 approach better, in that I liked using
set_supported_cpuid but I didn't like *removing* features from it.

I think all *_supported() should be removed, and the code moved from
__do_cpuid_func() to set_supported_cpuid.

For now, however, this one is consistent with other features so I am
applying it.

Paolo