Re: [Xen-devel] [PATCH v2] x86: do not enable global pages when virtualized on AMD hardware

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.12.2019 14:17, Roger Pau Monné  wrote:
> On Wed, Dec 04, 2019 at 02:15:58PM +0100, Jan Beulich wrote:
>> On 04.12.2019 12:52, Roger Pau Monné wrote:
>>> On Wed, Dec 04, 2019 at 12:05:35PM +0100, Jan Beulich wrote:
>>>> On 04.12.2019 11:44, Roger Pau Monne wrote:
>>>>> --- a/xen/arch/x86/pv/domain.c
>>>>> +++ b/xen/arch/x86/pv/domain.c
>>>>> @@ -118,11 +118,18 @@ unsigned long pv_fixup_guest_cr4(const struct vcpu 
>>>>> *v, unsigned long cr4)
>>>>>              (mmu_cr4_features & PV_CR4_GUEST_VISIBLE_MASK));
>>>>>  }
>>>>>  
>>>>> +static int opt_global_pages = -1;
>>>>
>>>> int8_t __read_mostly
>>>>
>>>>> +boolean_runtime_param("global-pages", opt_global_pages);
>>>>> +
>>>>>  unsigned long pv_make_cr4(const struct vcpu *v)
>>>>>  {
>>>>>      const struct domain *d = v->domain;
>>>>>      unsigned long cr4 = mmu_cr4_features &
>>>>>          ~(X86_CR4_PCIDE | X86_CR4_PGE | X86_CR4_TSD);
>>>>> +    bool pge = opt_global_pages == -1 ? (!cpu_has_hypervisor ||
>>>>> +                                         boot_cpu_data.x86_vendor !=
>>>>> +                                         X86_VENDOR_AMD)
>>>>> +                                      : !!opt_global_pages;
>>>>
>>>> Let's avoid re-doing this evaluation each time we come here.
>>>> Post boot the value can only change to 0 or 1. Hence in some
>>>> __init function you can apply the default calculation done
>>>> here.
>>>
>>> I've assumed that boolean_runtime_param can be changed during runtime
>>> by the user, and hence the value calculated at boot time would become
>>> stale if the user changes it after boot, which should be fine for this
>>> option.
>>
>> I'm afraid I can't decide whether you agree or disagree with my
>> comment.
> 
> Oh sorry fro not being clear. I was meant to disagree, calculating a
> value at init time would be wrong, since opt_global_pages can change
> during runtime.

But that's what I've explained in my earlier reply: At run time,
the value can only change to 0 or 1, but not to -1. Hence once a
runtime update occurred, the default calculation won't be used
anymore (as much as it wouldn't be used if there was a respective
command line option).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel