tianocore / edk2

EDK II
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II
Other
4.64k stars 2.52k forks source link

IDE Cdrom devices not present when using latest Ovmf #43

Closed mutedbytes closed 8 years ago

mutedbytes commented 8 years ago

In current Ovmf code, when using firmware with -cdrom or -drive options using qemu with KVM to present a disc image with an IDE cdrom device, the device never appears in the firmware. In previous Ovmf builds, this would appear in the firmware list as a DVD/CD device, with available access to guest (especially during installation). This presents a problem, for example, when requiring virtio drivers when installing Windows guest.

alex3kov commented 8 years ago

This seems to be an issue with -machine type=q35 only. For example, the following works (Windows installation program can see HDD and second CD-ROM):

-machine type=pc,accel=kvm \
-drive if=ide,media=cdrom,file=/vms/win7_sp1.iso,format=raw \
-drive if=ide,media=cdrom,file=/vms/virtio-win-0.1.102.iso,format=raw \
-drive if=ide,media=disk,format=qcow2,file=/vms/w7.qcow2

On q35, this works:

-machine type=q35,accel=kvm \
-device virtio-scsi-pci,id=scsi0 \
-device scsi-cd,bus=scsi0.0,drive=cdrom,bootindex=0 \
-drive id=cdrom,if=none,readonly,format=raw,file=/vms/win7_sp1.iso

As a workaround, I had to do this bait-and-switch with Windows 7 guest p2v: run on pc/ide -> connect dummy drive with virtio-scsi-pci -> install virtio drivers for it -> run win7 off q35/scsi-hd.

mutedbytes commented 8 years ago

I seem to experience this regardless of q35 or pc type. Though I am trying to do it along with using virtio-scsi-pci with scsi-block passthrough for hard drives, but not sure if correlated (in your first example ONLY ide devices are being used).

lersek commented 8 years ago

Confirmed, and bisected. The following commit is the culprit:

commit 7cac2401635317360226a235d1f95f8347081cbb
Author: Feng Tian <feng.tian@intel.com>
Date:   Thu Jan 7 07:03:32 2016 +0000

    MdeModulePkg/Ide: return correct status when DRQ is not ready for ATAPI

    When executing ATAPI cmd at IDE mode, EFI_SUCCESS may be returned wrongly
    with old logic but in fact DRQ is not ready and the transaction doesn't
    get executed correctly at this time.

    Contributed-under: TianoCore Contribution Agreement 1.0
    Signed-off-by: Feng Tian <feng.tian@intel.com>
    Reviewed-by: Star Zeng <star.zeng@intel.com>

    git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19611 6f19259b-4bc3-4df7-8a09-765794883524
lersek commented 8 years ago

Reported to the list: http://thread.gmane.org/gmane.comp.bios.edk2.devel/3681/focus=6659

lersek commented 8 years ago

Posted patches: http://thread.gmane.org/gmane.comp.bios.edk2.devel/6683

lersek commented 8 years ago

Patches committed to SVN: r19684 and r19685.

The commits should soon show up in the git mirror as well, after which Gerd's Jenkins instance too will build new RPMs with the fix. @mutedbytes, @alex3kov, please report back with test results once the new RPMs are out. (I can't say what build number or git hash to look for in the RPM name -- just check a new build in a few days please.) Thanks.

lersek commented 8 years ago

Well, at least this kind of integration (automatic pingback) between the issue tracker and the git repo is nice.

alex3kov commented 8 years ago

The commits should soon show up in the git mirror as well, after which Gerd's Jenkins instance too will build new RPMs with the fix. @mutedbytes, @alex3kov, please report back with test results once the new RPMs are out.

Can you share a link where those RPMs are uploaded?
I wanted to test patches myself, but building edk2 is a nightmare on modern linux - just don't have time for it.

lersek commented 8 years ago

The yum repo file can be downloaded from https://www.kraxel.org/repos/ . Instructions are given on that page as well.

The package you want is edk2.git-ovmf-x64. If you don't use an RPM-based distro, you can fetch the most recent RPM directly from https://www.kraxel.org/repos/jenkins/edk2/ . In that case you'll have to extract the files from the RPM manually. Various wikis (like the Arch one) explain how to go from there. My recommendation is to just let libvirt handle the files, once they are installed (and libvirt is configured to find them.)

The Fedora wiki also has some hints.

lersek commented 8 years ago

The build I'm seeing there ATM (20160120.b1432.g8d0776f) should contain the patch I posted (git commit 0368497).

jljusten commented 8 years ago

@lersek: 0368497 fixes this bug?

It looks like we might be able to use the "Fixes:" tag in commit message to auto close issues:

https://help.github.com/articles/closing-issues-via-commit-messages/

So, using this in the commit message:

Fixes: https://github.com/tianocore/edk2/issues/43

Might close it when pushed.

lersek commented 8 years ago

@jljusten, https://github.com/tianocore/edk2/commit/036849752427ff8615cfc492a6daf4c06cb203d6 fixes it indeed.

Paolo posted another patch on top that improves things significantly. Feng just provided feedback on it. But, https://github.com/tianocore/edk2/commit/036849752427ff8615cfc492a6daf4c06cb203d6 should suffice to avert the regression.

Re: auto-closing issues when patches are pushed, I'm not so enthusiastic about that feature. For one, what if there are several patches that constitute a fix together? I think the pingback is a nice touch (from the Reference: tag I added), but I'd rather do the closing myself. (Once the migration to git happens, and I get the github permissions, that is.)

lersek commented 8 years ago

@jljusten: BTW, the OVMF tag is not appropriate for this issue; I suggest to remove it. The regression occurred in MdeModulePkg (see the bisection above). OVMF was just quick to expose it to users.

mutedbytes commented 8 years ago

@lersek Thanks a lot! I will give it a test soon. I am building from git though, and am getting build errors when trying to build RELEASE target of ovmf, while DEBUG builds ok. I will test DEBUG target for now and report back. Not sure if should file new issue for build problem.

mutedbytes commented 8 years ago

Turns out I could only build a 2MB image, default RELEASE 1MB complained about size errors in FVMAIN_COMPACT FV when trying to build. Anyways, booted up guest with latest git source with an ide-cd drive attached with an iso presented, and it appears in guest just fine now. So with that, it seems the issue is fixed :+1:

lersek commented 8 years ago

@mutedbytes thanks for testing.

Re: being unable to build RELEASE, that depends on your build options and compiler, I think. For RELEASE, we choose a 1MB (unified) firmware image size by default. For DEBUG, we choose a 2MB (unified) firmware image size by default. In both cases however, you can manually select the other size, if you wish to. For example, -b RELEASE -D FD_SIZE_2MB. Hope this helps.