On 31/10/2019 11:07, Jan Beulich wrote:
> On 31.10.2019 11:56, Sergey Dyasli wrote:
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -274,6 +274,15 @@ static inline u32 phys_pkg_id(u32 cpuid_apic, int
>> index_msb)
>> return _phys_pkg_id(get_apic_id(), index_msb);
>> }
>>
>> +/*
>> + * Sometimes it's too early to use cpu_has_hypervisor which is available
>> only
>> + * after early_cpu_init().
>> + */
>> +bool __init early_cpu_has_hypervisor(void)
>> +{
>> + return cpuid_ecx(1) & cpufeat_mask(X86_FEATURE_HYPERVISOR);
>> +}
Static inline alongside the other cpuid helpers please. This assembles
to smaller than the call required to get to a separate translation unit.
> OOI, did you consider introducing a more general early_cpu_has(),
> with X86_FEATURE_* passed as an argument?
This is interim code. Personally, I'm still go with what I said at the
very beginning, of opencoding it just in the hunk below.
>
>> @@ -318,9 +319,10 @@ static int __init copy_e820_map(struct e820entry *
>> biosmap, unsigned int nr_map)
>>
>> /*
>> * Some BIOSes claim RAM in the 640k - 1M region.
>> - * Not right. Fix it up.
>> + * Not right. Fix it up, but only when running on bare metal.
>> */
>> - if (type == E820_RAM) {
>> + if ( !early_cpu_has_hypervisor() && type == E820_RAM )
>> + {
>> if (start < 0x100000ULL && end > 0xA0000ULL) {
>> if (start < 0xA0000ULL)
>> add_memory_region(start, 0xA0000ULL-start, type);
> Seeing original line and lower context - aren't you corrupting
> previously reasonably consistent (Linux) style here? (This could
> be easily taken care of while committing, but I'd first like the
> point below be clarified.)
>
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -5943,7 +5943,7 @@ const struct platform_bad_page *__init
>> get_platform_badpages(unsigned int *array
>> case 0x000806e0: /* erratum KBL??? */
>> case 0x000906e0: /* errata KBL??? / KBW114 / CFW103 */
>> *array_size = (cpuid_eax(0) >= 7 &&
>> - !(cpuid_ecx(1) &
>> cpufeat_mask(X86_FEATURE_HYPERVISOR)) &&
>> + !early_cpu_has_hypervisor() &&
> Strictly speaking this makes clear that in early_cpu_has_hypervisor()
> you should also check cpuid_eax(0) >= 1. We don't currently seem to
> object to running on a system with only basic leaf 0 available (we
> do object to there not being extended leaf 1). Andrew, do you have
> any clear opinion one way or the other?
By the time we are running in C, we know the CPU actually has long mode
and therefore max_leaf > 4(?) and max_extd_leaf >= 0x80000008
I don't see any value in not relying on this property, and a cost (extra
branches) to not relying on it.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
