Re: [PATCH v2 1/4] x86/compat: Test whether guest has 32b shinfo instead of being a PV 32b domain

["Jan Beulich" <JBeulich@suse.com>]

>>> On 09.07.15 at 18:05, <boris.ostrovsky@oracle.com> wrote:
> On 07/09/2015 10:30 AM, Boris Ostrovsky wrote:
>> On 07/09/2015 10:17 AM, Jan Beulich wrote:
>>>>>> On 09.07.15 at 16:10, <boris.ostrovsky@oracle.com> wrote:
>>>> On 07/09/2015 03:02 AM, Jan Beulich wrote:
>>>>>>>> On 08.07.15 at 22:57, <boris.ostrovsky@oracle.com> wrote:
>>>>>> As I started to update the patches I realized that in some cases
>>>>>> (especially in arch_do_domctl():XEN_DOMCTL_get_address_size) we don't
>>>>>> have VCPU (which is what hvm_guest_x86_mode() wants) but rather 
>>>>>> only the
>>>>>> domain. d->vcpu[0] should work. Otherwise I'll either need a new 
>>>>>> field
>>>>>> in struct domain or wrap has_32bit_shinfo into something 
>>>>>> PVH-specific,
>>>>>> like is_32bit_pvh_vcpu().
>>>>> Shouldn't XEN_DOMCTL_get_address_size be handled HVM-like
>>>>> for PVH, especially if you also intend the tools to use the 64-bit
>>>>> guest context variant even for 32-bit PVH? Once again - are you
>>>>> intending to prohibit 32-bit PVH switching to 64-bit mode (which
>>>>> would seem both wrong and possibly cumbersome to me)?
>>>> With current PVH implementation I don't think we can switch. We are
>>>> starting the guest in very much PV-like fashion. That's why we are
>>>> getting into switch_compat() --- via XEN_DOMCTL_set_address_size.
>>>>
>>>> For XEN_DOMCTL_get_address_size specifically we need to be able to
>>>> figure out the mode *before* the guest is running because we use it to
>>>> set cpuid bits in xc_cpuid_pv_policy(). So just that means we can't
>>>> change the mode.
>>> Okay - but is there code (being put) in place to refuse switch
>>> attempts?
>>
>> No, I should add code to deal with this.
> 
> Forgot to include here --- so what is your preference wrt what I am 
> asking in the first paragraph? d->vcpu[0], new field (or maybe a flag 
> with bits per 32bit-pv and 32bit-pvh), or a PVH-wrapper for 
> has_32bit_shinfo?

To be honest, my preference would be to have none of those, and
have the whole thing more HVM-like from the beginning. Considering
that this isn't realistic, a PVH alias for has_32bit_shinfo() would seem
least ugly to me.

Jan