[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/4] x86: Introduce MSR_UNHANDLED

  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
  • Date: Mon, 22 Feb 2021 16:19:04 -0500
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=FnSLpPpx6znBKYOeSTHZG3qtMo96LSccbYN8BJYyelU=; b=TkycPZ7uJfGIEZHBR6edQyKDpqQZy3jDcPsQJOogxXFPjQvQeWK1jZ0inuJA9Q1r1g/uxkg49XIBVgDQOlaIcIXw/XA76s/fBj5MqYYsEHgcELN7B3nLNifQshnUDQSLErnsLe9UVzyl3pkC/L3zYf56bf+b9ll6ibuSPbVHWmZ8SgibD219JuEv5guv/+WQHuq1ZQ4uzdtehNxLXEJktt4iL4Yv954QXJixBExAqa5EFhpzgVjcX8YZGnVD9NKQQRoytZorlmA7NnDkWeuxUelCz5JAlYY0YdPxBaMtHiMG3PyhUKeckcS/lKZGvMuSKC6zIbp//0fU2XDKKE38rw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=daxE374AKZ1UOri811Iq/KVgE7kKlLl24HunBtx9pfUrQbqu3Dc/7/2KvDI4n0DzgDaT3q5qnO0UFgHRK1bhWzF0qOGIUy7DeVPRO2VIIRPbjKPSO+G+3KeLONsjVlBjfuQp8/kFs2pbltg4N5fI80x7NQnyPfDHBaTsvfkf50n7PX9iQq2NWqQTxgzXdFwRJz+l0nUgG3e67ppjs3a5dPJSkJ7k1HE+hZvbMBTwkNwUDv/d8cBCg7OXMjgEz2p/HuI6Jvhf7SxINZpH9jRBOExj4EBxjtrcrpj/JSToRVmy0iNnNMFOw0ZgCYhdtgIXy1UHIj8NZJGIO8on149HmQ==
  • Authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=oracle.com;
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, iwj@xxxxxxxxxxxxxx, wl@xxxxxxx, anthony.perard@xxxxxxxxxx, jbeulich@xxxxxxxx, andrew.cooper3@xxxxxxxxxx, jun.nakajima@xxxxxxxxx, kevin.tian@xxxxxxxxx
  • Delivery-date: Mon, 22 Feb 2021 21:19:31 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 2/22/21 6:08 AM, Roger Pau Monné wrote:
> On Fri, Feb 19, 2021 at 09:56:32AM -0500, Boris Ostrovsky wrote:
>> On 2/18/21 5:51 AM, Roger Pau Monné wrote:
>>> On Wed, Jan 20, 2021 at 05:49:10PM -0500, Boris Ostrovsky wrote:
>>>> When toolstack updates MSR policy, this MSR offset (which is the last
>>>> index in the hypervisor MSR range) is used to indicate hypervisor
>>>> behavior when guest accesses an MSR which is not explicitly emulated.
>>> It's kind of weird to use an MSR to store this. I assume this is done
>>> for migration reasons?
>> Not really. It just seemed to me that MSR policy is the logical place to do 
>> that. Because it *is* a policy of how to deal with such accesses, isn't it?
> I agree that using the msr_policy seems like the most suitable place
> to convey this information between the toolstack and Xen. I wonder if
> it would be fine to have fields in msr_policy that don't directly
> translate into an MSR value?

We have xen_msr_entry_t.flags that we can use when passing policy array back 
and forth. Then we can ignore xen_msr_entry_t.idx for that entry (although in 
earlier version of this series Jan preferred to use idx and leave flags alone).

> But having such a list of ignored MSRs in msr_policy makes the whole
> get/set logic a bit weird, as the user would have to provide a buffer
> in order to get the list of ignored MSRs.

If we go with ranges/lists of ignored MSRs then we will need to have 
ignore_msrs as a rangeset in msr_policy, not as (current) uint8. And 
xen_msr_entry_t will need to have a range as opposed to single index. Or maybe 
I don't understand what you are referring to as get/set logic.

But I would like to make sure we really want to support such ranges, I am a 
little concerned about over-engineering this (especially wrt migration, I think 
it will need marshalling/unmarshalling)

>>> Isn't is possible to convey this data in the xl migration stream
>>> instead of having to pack it with MSRs?
>> I haven't looked at this but again --- the feature itself has nothing to do 
>> with migration. The fact that folding it into policy makes migration of this 
>> information "just work" is just a nice side benefit of this implementation.
> IMO it feels slightly weird that we have to use a MSR (MSR_UNHANDLED)
> to store this option, seems like wasting an MSR index when there's
> really no need for it to be stored in an MSR, as we don't expose it to
> guests.
> It would seem more natural for such option to live in arch_domain as a
> rangeset for example.
> Maybe introduce a new DOMCTL to set it?
> #define XEN_DOMCTL_msr_set_ignore ...
> struct xen_domctl_msr_set_ignore {
>     uint32_t index;
>     uint32_t size;
> };

That will work too but this is adding 2 new domctls (I think we will need a 
"get" too) whereas with policy we use existing interface.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.