Re: [PATCH RFCv2 3/4] i386/pc: warn if phys-bits is too low

[Joao Martins]

On 2/24/22 09:01, Igor Mammedov wrote:
> On Wed, 23 Feb 2022 17:18:55 +0000
> Joao Martins <joao.m.martins@oracle.com> wrote:
> 
>> On 2/14/22 15:18, Joao Martins wrote:
>>> On 2/14/22 15:03, Igor Mammedov wrote:  
>>>> On Mon,  7 Feb 2022 20:24:21 +0000
>>>> Joao Martins <joao.m.martins@oracle.com> wrote:
>>>>  
>>>>> Default phys-bits on Qemu is TCG_PHYS_BITS (40) which is enough
>>>>> to address 1Tb (0xff ffff ffff). On AMD platforms, if a
>>>>> ram-above-4g relocation happens and the CPU wasn't configured
>>>>> with a big enough phys-bits, warn the user. There isn't a
>>>>> catastrophic failure exactly, the guest will still boot, but
>>>>> most likely won't be able to use more than ~4G of RAM.  
>>>>
>>>> how 'unable to use" would manifest?
>>>> It might be better to prevent QEMU startup with broken setup (CLI)
>>>> rather then letting guest run and trying to figure out what's
>>>> going wrong when users start to complain. 
>>>>  
>>> Sounds better to be conservative here.
>>>
>>> I will change from warn_report() to error_report()
>>> and exit.
>>>   
>>
>> I was running through x86_64 qtests prior to submission
>> and it seems that the inclusion of a pci_hole64_size in
>> the check added by this patch would break tests if we were
>> to error out. So far, I'm keeping it as a warning over
>> compatibility concerns, not limited these 5 test failures
>> below. Let me know otherwise if you disagree, or if you
>> prefer another way.
> 
> can you check what exactly breaks tests?
> 
The test prematuralely fails with the above check that.

And on a second look, the problem is obvious. Essentially, I
am not handling the 32-bit case correctly :(

I will revert back what I submitted in v3 to be an error_report()
and exit() and will restrict this 64-bit only (i.e. for memory above-4g)

qemu-system-x86_64: Address space above 4G at 100000000-100000000 phys-bits too 
low (32)
qemu-system-x86_64: Address space above 4G at 100000000-100000000 phys-bits too 
low (32)
qemu-system-x86_64: Address space above 4G at 100000000-100000000 phys-bits too 
low (32)
# child process (/x86/cpuid/parsing-plus-minus/subprocess [188634]) stderr:
"qemu-system-x86_64: warning: Ambiguous CPU model string. Don't mix both 
\"-mce\" and
\"mce=on\"\nqemu-system-x86_64: warning: Ambiguous CPU model string. Don't mix 
both
\"+cx8\" and \"cx8=off\"\nqemu-system-x86_64: warning: Compatibility of 
ambiguous CPU
model strings won't be kept on future QEMU versions\nqemu-system-x86_64: 
Address space
above 4G at 100000000-180000000 phys-bits too low (32)\nBroken pipe\n"
qemu-system-x86_64: Address space above 4G at 100000000-180000000 phys-bits too 
low (32)

>> Summary of Failures:
>>
>>  1/56 qemu:qtest+qtest-x86_64 / qtest-x86_64/qom-test               ERROR    
>>        0.07s
>>   killed by signal 6 SIGABRT
>>  4/56 qemu:qtest+qtest-x86_64 / qtest-x86_64/test-hmp               ERROR    
>>        0.07s
>>   killed by signal 6 SIGABRT
>>  7/56 qemu:qtest+qtest-x86_64 / qtest-x86_64/boot-serial-test       ERROR    
>>        0.07s
>>   killed by signal 6 SIGABRT
>> 44/56 qemu:qtest+qtest-x86_64 / qtest-x86_64/test-x86-cpuid-compat  ERROR    
>>        0.09s
>>   killed by signal 6 SIGABRT
>> 45/56 qemu:qtest+qtest-x86_64 / qtest-x86_64/numa-test              ERROR    
>>        0.17s
>>   killed by signal 6 SIGABRT
>>
>