qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Claudio Fontana <cfontana@suse.de>
To: Cleber Rosa Junior <crosa@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	kvm@vger.kernel.org,
	"Siddharth Chandrasekaran" <sidcha@amazon.de>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	qemu-devel <qemu-devel@nongnu.org>,
	"Cameron Esfahani" <dirty@apple.com>,
	"Roman Bolshakov" <r.bolshakov@yadro.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Vitaly Kuznetsov" <vkuznets@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH v2 0/2] Fixes for "Windows fails to boot"
Date: Fri, 4 Jun 2021 08:32:10 +0200	[thread overview]
Message-ID: <2e5edcf2-6958-82db-511c-724165a8ddfb@suse.de> (raw)
In-Reply-To: <CA+bd_6K1BOSeswTszBGJrq4Z9F_KpPsSuOL-cLbYWGAfvjPEMA@mail.gmail.com>

On 6/3/21 5:10 PM, Cleber Rosa Junior wrote:
> On Thu, Jun 3, 2021 at 10:29 AM Claudio Fontana <cfontana@suse.de> wrote:
> 
>> On 6/3/21 2:29 PM, Claudio Fontana wrote:
>>> v1 -> v2:
>>>  * moved hyperv realizefn call before cpu expansion (Vitaly)
>>>  * added more comments (Eduardo)
>>>  * fixed references to commit ids (Eduardo)
>>>
>>> The combination of Commits:
>>> f5cc5a5c ("i386: split cpu accelerators from cpu.c"...)
>>
>>> 30565f10 ("cpu: call AccelCPUClass::cpu_realizefn in"...)
>>>
>>> introduced two bugs that break cpu max and host in the refactoring,
>>> by running initializations in the wrong order.
>>>
>>> This small series of two patches is an attempt to correct the situation.
>>>
>>> Please provide your test results and feedback, thanks!
>>>
>>> Claudio
>>>
>>> Claudio Fontana (2):
>>>   i386: reorder call to cpu_exec_realizefn in x86_cpu_realizefn
>>>   i386: run accel_cpu_instance_init as instance_post_init
>>>
>>>  target/i386/cpu.c         | 89 +++++++++++++++++++++++++--------------
>>>  target/i386/kvm/kvm-cpu.c | 12 +++++-
>>>  2 files changed, 68 insertions(+), 33 deletions(-)
>>>
>>
>> Btw, CI/CD is all green, but as mentioned, it does not seem to catch these
>> kind of issues.
>>
>>
> Hi Claudio,
> 
> Not familiar with the specifics of this bug, but can it be caught by
> attempting to boot an image other than Windows?  If so, we can consider
> adding a test along the lines of tests/acceptance/boot_linux_console.py.
> 
> Thanks,
> - Cleber.

Hello Cleber,

yes, all that seems to be required is the "host" cpu, q35 machine, and the firmware ./OVMF_CODE.secboot.fd and ./OVMF_VARS.secboot.fd :

./build/x86_64-softmmu/qemu-system-x86_64 \
        -cpu host \
        -enable-kvm \
        -m 4G \
        -machine q35,smm=on \
        -drive if=pflash,format=raw,readonly=on,unit=0,file="./OVMF_CODE.secboot.fd" \
        -drive if=pflash,format=raw,unit=1,file="./OVMF_VARS.secboot.fd"

With the bugged code, the firmware does not boot, and the cpu does not get into 64-bit long mode.
Applying the patches the firmware boots normally and we get the TianoCore Logo and text output.

Adding something like -display none -serial stdio would also generate text in the OK case that could be "expected" by a test:

BdsDxe: failed to load Boot0001 "UEFI QEMU DVD-ROM QM00005 " from PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0): Not Found

>>Start PXE over IPv4.

even without using any guest to boot at all, just the firmware.
I used this Fedora package for the test, containing the firmware: edk2-ovmf-20200801stable-1.fc33.noarch.rpm

I looked briefly at tests/acceptance/boot_linux_console.py, but did not see where such a test of firmware could be inserted,
could you advise?

Thanks,

Claudio

> 
> 
>> https://gitlab.com/hw-claudio/qemu/-/pipelines/314286751
>>
>> C.
>>
>>
>>
> 



  reply	other threads:[~2021-06-04  6:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-03 12:29 [PATCH v2 0/2] Fixes for "Windows fails to boot" Claudio Fontana
2021-06-03 12:30 ` [PATCH v2 1/2] i386: reorder call to cpu_exec_realizefn Claudio Fontana
2021-06-03 12:30 ` [PATCH v2 2/2] i386: run accel_cpu_instance_init as post_init Claudio Fontana
2021-06-03 14:24 ` [PATCH v2 0/2] Fixes for "Windows fails to boot" Claudio Fontana
2021-06-03 15:10   ` Cleber Rosa Junior
2021-06-04  6:32     ` Claudio Fontana [this message]
2021-06-04  7:01       ` Claudio Fontana
2021-06-04  9:09         ` Claudio Fontana

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2e5edcf2-6958-82db-511c-724165a8ddfb@suse.de \
    --to=cfontana@suse.de \
    --cc=crosa@redhat.com \
    --cc=dirty@apple.com \
    --cc=ehabkost@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=r.bolshakov@yadro.com \
    --cc=richard.henderson@linaro.org \
    --cc=sidcha@amazon.de \
    --cc=vkuznets@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).