Update the hv_kvm_unit_test to suitable Win10-32

[peixiu]

Set the workaround of hv_kvm_unit_test about lm=off

Remove the lm=off flag form hv_kvm_unit_test test

ID: 1897

Signed-off-by: Peixiu Hou [email protected]

[peixiu]

(1/1) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.i386.io-github-autotest-qemu.hv_info_check.q35: STARTED
(1/1) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.i386.io-github-autotest-qemu.hv_info_check.q35: PASS (42.55 s)

(1/1) Host_RHEL.m10.u0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.x86_64.io-github-autotest-qemu.hv_tlbflush.q35: STARTED
(1/1) Host_RHEL.m10.u0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.x86_64.io-github-autotest-qemu.hv_tlbflush.q35: PASS

@YongxueHong Could you help to review this patch? thanks!

[peixiu]

Hi @YongxueHong

Add some explanation here, after the patch https://github.com/avocado-framework/avocado-vt/pull/3828/files merged , 32 bits guest will add lm=off, pae=on flags. It doesn't suitable this case, with them, the case will fail, and actually, this case is not related to the windows vm version.

So I set a workaround in this case.

Thanks!
Peixiu

[YongxueHong]

Hi @YongxueHong

Add some explanation here, after the patch https://github.com/avocado-framework/avocado-vt/pull/3828/files merged , 32 bits guest will add lm=off, pae=on flags. It doesn't suitable this case, with them, the case will fail, and actually, this case is not related to the windows vm version.

So I set a workaround in this case.

Thanks! Peixiu

Hi, Peixiu
As my understanding of your code changes, you mean that there are some issues for the windows 10 32 bit guest with the cup flag lm=off, pae=on and it will block the test case.
Could you give more detail about it? is it a product issue? or others?
And why don't we add the limitation to the virttest/shared/cfg/guest-os/Windows.cfg for the win10 32 bit,
for example:

    # lm: Long Mode (x86-64: amd64, also known as Intel 64, i.e. 64-bit capable)
    # The flag tells you that the CPU is 64-bit. it's absences tells you that it's 32-bit. (lm=off)
    i386:
        no Win10
        cpu_model_flags += "lm=off,pae=on"

I'd love to hear your thoughts on this.
Thanks.

[YongxueHong]

Hi @YongxueHong
Add some explanation here, after the patch https://github.com/avocado-framework/avocado-vt/pull/3828/files merged , 32 bits guest will add lm=off, pae=on flags. It doesn't suitable this case, with them, the case will fail, and actually, this case is not related to the windows vm version.
So I set a workaround in this case.
Thanks! Peixiu

Hi, Peixiu As my understanding of your code changes, you mean that there are some issues for the windows 10 32 bit guest with the cup flag lm=off, pae=on and it will block the test case. Could you give more detail about it? is it a product issue? or others? And why don't we add the limitation to the virttest/shared/cfg/guest-os/Windows.cfg for the win10 32 bit, for example:

    # lm: Long Mode (x86-64: amd64, also known as Intel 64, i.e. 64-bit capable)
    # The flag tells you that the CPU is 64-bit. it's absences tells you that it's 32-bit. (lm=off)
    i386:
        no Win10
        cpu_model_flags += "lm=off,pae=on"

I'd love to hear your thoughts on this. Thanks.

Hi @leidwang
According to the RP avocado-framework/avocado-vt#3828 , I think that you want to add the new CPU flags for the 32 bit Windows VM since you use the expression += to do that.
So you do not want to clear the original flag's value, just to append to some flags at the end, right?
if it is, I suggest adding a symbol , at the beginning of the flags, like cpu_model_flags += ", xx=aa,yy=bb".
The framework will parse the value we passed:
https://github.com/avocado-framework/avocado-vt/blob/f0424ee74f6e3043327bd38fa069bac1c1a075f6/virttest/cpu.py#L1451-L1468

Please correct me if I am misunderstanding and I would like to hear your thoughts.
Thanks.

[leidwang]

Hi @YongxueHong
Add some explanation here, after the patch https://github.com/avocado-framework/avocado-vt/pull/3828/files merged , 32 bits guest will add lm=off, pae=on flags. It doesn't suitable this case, with them, the case will fail, and actually, this case is not related to the windows vm version.
So I set a workaround in this case.
Thanks! Peixiu

Hi, Peixiu As my understanding of your code changes, you mean that there are some issues for the windows 10 32 bit guest with the cup flag lm=off, pae=on and it will block the test case. Could you give more detail about it? is it a product issue? or others? And why don't we add the limitation to the virttest/shared/cfg/guest-os/Windows.cfg for the win10 32 bit, for example:

    # lm: Long Mode (x86-64: amd64, also known as Intel 64, i.e. 64-bit capable)
    # The flag tells you that the CPU is 64-bit. it's absences tells you that it's 32-bit. (lm=off)
    i386:
        no Win10
        cpu_model_flags += "lm=off,pae=on"

I'd love to hear your thoughts on this. Thanks.

Hi @leidwang According to the RP avocado-framework/avocado-vt#3828 , I think that you want to add the new CPU flags for the 32 bit Windows VM since you use the expression += to do that. So you do not want to clear the original flag's value, just to append to some flags at the end, right? if it is, I suggest adding a symbol , at the beginning of the flags, like cpu_model_flags += ", xx=aa,yy=bb". The framework will parse the value we passed: https://github.com/avocado-framework/avocado-vt/blob/f0424ee74f6e3043327bd38fa069bac1c1a075f6/virttest/cpu.py#L1451-L1468

Please correct me if I am misunderstanding and I would like to hear your thoughts. Thanks.

Hi @YongxueHong

Yes, I just want add these 2 cpu flags instead of rewrite it.

I got your point, but it also works without ,, do you think we need to modify the code currently?Thanks!
ref:https://github.com/avocado-framework/avocado-vt/blob/f0424ee74f6e3043327bd38fa069bac1c1a075f6/virttest/cpu.py#L1534C5-L1534C29
https://github.com/avocado-framework/avocado-vt/blob/f0424ee74f6e3043327bd38fa069bac1c1a075f6/virttest/env_process.py#L1429

[YongxueHong]

Hi @leidwang
Although it works now, it has potential risks for other cases, like this PR.
@peixiu She hit the issue that the framework is not able to parse/separate the parameter correctly.
The related log:

[stdlog]     -cpu 'Icelake-Server-noTSX',hv_reset,hv_crashlm=off,pae=on,hv_stimer,hv_synic,hv_vpindex,hv_relaxed,hv_spinlocks=0x1fff,hv_vapic,hv_time,hv_frequencies,hv_runtime,hv_tlbflush,hv_reenlightenment,hv_stimer_direct,hv_ipi,hv-xmm-input,hv_tlbflush_ext,+kvm_pv_unhalt \
...
[qemu output] qemu-kvm: can't apply global Icelake-Server-noTSX-x86_64-cpu.hv-crashlm=off: Property 'Icelake-Server-noTSX-x86_64-cpu.hv-crashlm' not found

Obviously, the flag hv-crashlm=off should be hv-crash, lm=off, but both are combined without the symbol , which is unexpected.

[leidwang]

Hi @leidwang Although it works now, it has potential risks for other cases, like this PR. @peixiu She hit the issue that the framework is not able to parse/separate the parameter correctly. The related log:

[stdlog]     -cpu 'Icelake-Server-noTSX',hv_reset,hv_crashlm=off,pae=on,hv_stimer,hv_synic,hv_vpindex,hv_relaxed,hv_spinlocks=0x1fff,hv_vapic,hv_time,hv_frequencies,hv_runtime,hv_tlbflush,hv_reenlightenment,hv_stimer_direct,hv_ipi,hv-xmm-input,hv_tlbflush_ext,+kvm_pv_unhalt \
...
[qemu output] qemu-kvm: can't apply global Icelake-Server-noTSX-x86_64-cpu.hv-crashlm=off: Property 'Icelake-Server-noTSX-x86_64-cpu.hv-crashlm' not found

Obviously, the flag hv-crashlm=off should be hv-crash, lm=off, but both are combined without the symbol , which is unexpected.

Okay, got it, thanks for the detailed explanation. I'll send a PR to avocado-vt to fix it.

[leidwang]

cpu_model_flags

avocado-vt PR: avocado-framework/avocado-vt#3988

[peixiu]

Based on the patch avocado-framework/avocado-vt#3988.
For hv_kvm_unit_test, our test no neeed lm=off, pae=on, so workaround it here.

python3 ConfigTest.py --testcase=hv_kvm_unit_test --guestname=Win10 --platform=i386 --clone=no --machines=q35 --firmware=default_bios --debug

(1/1) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.i386.io-github-autotest-qemu.hv_kvm_unit_test.q35: STARTED
(1/1) Host_RHEL.m10.u0.qcow2.virtio_scsi.up.virtio_net.Guest.Win10.i386.io-github-autotest-qemu.hv_kvm_unit_test.q35: PASS (13.56 s)

@YongxueHong Please help to review it again, thanks!

[leidwang]

Hi @peixiu Maybe we can modify other non-standard cpu_model_flags value(hv related test cases) by adding "," at the beginning of it.To avoid similar error.What do you think? cc @YongxueHong

For example: https://github.com/autotest/tp-qemu/blob/af63fc5f56ddc0aa0ad2d974ab97ad7369374632/qemu/tests/cfg/hv_tlbflush.cfg#L7C5-L7C20

[YongxueHong]

Hi @peixiu Maybe we can modify other non-standard cpu_model_flags value(hv related test cases) by adding "," at the beginning of it.To avoid familiar error.What do you think? cc @YongxueHong

For example: https://github.com/autotest/tp-qemu/blob/af63fc5f56ddc0aa0ad2d974ab97ad7369374632/qemu/tests/cfg/hv_tlbflush.cfg#L7C5-L7C20

Yeah, that's it, thank @leidwang for reminding.
We should add the symbol , if we want to append a CPU flag to the cpu_model_flags by using the expression "+=".

Hi, @peixiu
You could open a new PR to improve the above potential issue. Or you can also fix it along with this PR.
It depends on you.
Thanks.

[YongxueHong]

LGTM.

[peixiu]

Hi @peixiu Maybe we can modify other non-standard cpu_model_flags value(hv related test cases) by adding "," at the beginning of it.To avoid familiar error.What do you think? cc @YongxueHong
For example: https://github.com/autotest/tp-qemu/blob/af63fc5f56ddc0aa0ad2d974ab97ad7369374632/qemu/tests/cfg/hv_tlbflush.cfg#L7C5-L7C20

Yeah, that's it, thank @leidwang for reminding. We should add the symbol , if we want to append a CPU flag to the cpu_model_flags by using the expression "+=".

Hi, @peixiu You could open a new PR to improve the above potential issue. Or you can also fix it along with this PR. It depends on you. Thanks.

Thank You @YongxueHong , I prefer to open a new PR instead.

Thanks!
Peixiu

[YongxueHong]

Hi @peixiu
Looks like this PR's branch is out of data, let's update it to the latest one.
And its commit also needs to be signed.
Thanks.

[peixiu]

Hi @peixiu
Looks like this PR's branch is out of data, let's update it to the latest one.
And its commit also needs to be signed.
Thanks.

@YongxueHong Updated, thank you!