For whatever reason, when you compile SeaBIOS in Csm16 mode, and use it under EDK2's OvmfPkg, the ATA_CMD_IDENTIFY_PACKET_DEVICE command doesn't work properly, therefore, SeaBIOS detects the "SATA HARDDISK" as a "SATA CDROM" device, in QEMU. Despite the Tianocore developers seem to have removed support for Csm16 some time ago, if we decide to remove Csm16 mode in SeaBIOS in favor of that, at least we have the last commit of Csm16 working properly and not half-broken. In order to fix this bug, I decided to add another command, that command being ATA_CMD_DEVICE_RESET, right before the ATA_CMD_IDENTIFY_PACKET_DEVICE command is called.
Signed-off-by: Christopher Lentocha christopherericlentocha@gmail.com --- src/hw/ahci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/hw/ahci.c b/src/hw/ahci.c index 4f0f640..e0864fa 100644 --- a/src/hw/ahci.c +++ b/src/hw/ahci.c @@ -484,7 +484,8 @@ static int ahci_port_setup(struct ahci_port_s *port) /* start device */ cmd |= PORT_CMD_START; ahci_port_writel(ctrl, pnr, PORT_CMD, cmd); - + sata_prep_simple(&port->cmd->fis, ATA_CMD_DEVICE_RESET); + ahci_command(port, 0, 0, buffer, sizeof(buffer)); sata_prep_simple(&port->cmd->fis, ATA_CMD_IDENTIFY_PACKET_DEVICE); rc = ahci_command(port, 0, 0, buffer, sizeof(buffer)); if (rc == 0) {
On Tue, Jan 21, 2025 at 11:59:14AM -0500, Christopher Lentocha wrote:
For whatever reason, when you compile SeaBIOS in Csm16 mode, and use it under EDK2's OvmfPkg, the ATA_CMD_IDENTIFY_PACKET_DEVICE command doesn't work properly, therefore, SeaBIOS detects the "SATA HARDDISK" as a "SATA CDROM" device, in QEMU. Despite the Tianocore developers seem to have removed support for Csm16 some time ago, if we decide to remove Csm16 mode in SeaBIOS in favor of that, at least we have the last commit of Csm16 working properly and not half-broken. In order to fix this bug, I decided to add another command, that command being ATA_CMD_DEVICE_RESET, right before the ATA_CMD_IDENTIFY_PACKET_DEVICE command is called.
Signed-off-by: Christopher Lentocha christopherericlentocha@gmail.com
Reviewed-by: Gerd Hoffmann kraxel@redhat.com
On Tue, Jan 28, 2025 at 09:14:43AM +0100, Gerd Hoffmann wrote:
On Tue, Jan 21, 2025 at 11:59:14AM -0500, Christopher Lentocha wrote:
For whatever reason, when you compile SeaBIOS in Csm16 mode, and use it under EDK2's OvmfPkg, the ATA_CMD_IDENTIFY_PACKET_DEVICE command doesn't work properly, therefore, SeaBIOS detects the "SATA HARDDISK" as a "SATA CDROM" device, in QEMU. Despite the Tianocore developers seem to have removed support for Csm16 some time ago, if we decide to remove Csm16 mode in SeaBIOS in favor of that, at least we have the last commit of Csm16 working properly and not half-broken. In order to fix this bug, I decided to add another command, that command being ATA_CMD_DEVICE_RESET, right before the ATA_CMD_IDENTIFY_PACKET_DEVICE command is called.
Signed-off-by: Christopher Lentocha christopherericlentocha@gmail.com
Reviewed-by: Gerd Hoffmann kraxel@redhat.com
Sounds good.
Gerd, since you reviewed and you're more familiar with this code, feel free to also commit.
Cheers, -Kevin
On Wed, Jan 29, 2025 at 01:39:29AM +0000, Kevin O'Connor wrote:
Sounds good.
Gerd, since you reviewed and you're more familiar with this code, feel free to also commit.
Committed and pushed.
take care, Gerd
Is it already pushed? I think I need to add a `if (CONFIG_CSM) {` to it
because I think it wasn't working with the regual bios.bin without CSM
on in QEMU. I don't see anything on coreboot/seabios GitHub (mirror)
saying it is pushed (last push 3 weeks ago). Sorry, I forgot about testing this originally after the 2nd patch. (The `if (CONFIG_CSM) {` isn't required for the original patch that reworked one of the checks)
- Christopher Lentocha christopherericlentocha@gmail.com
On 1/29/25 04:50, Gerd Hoffmann wrote:
On Wed, Jan 29, 2025 at 01:39:29AM +0000, Kevin O'Connor wrote:
Sounds good.
Gerd, since you reviewed and you're more familiar with this code, feel free to also commit.
Committed and pushed.
take care, Gerd
On Wed, Feb 05, 2025 at 10:53:04AM -0500, Christopher Lentocha wrote:
Is it already pushed?
Nope, apparently something went wrong ...
I think I need to add a `if (CONFIG_CSM) {` to it
because I think it wasn't working with the regual bios.bin without CSM
Does a controller reset (see below) work in both cases?
take care, Gerd
commit 79d50d3c156dfd15ed710d0e2313d113251aeb2e Author: Gerd Hoffmann kraxel@redhat.com Date: Thu Feb 6 12:10:21 2025 +0100
ahci: add controller reset
diff --git a/src/hw/ahci.c b/src/hw/ahci.c index 4f0f640a1c64..2285d33d4ae2 100644 --- a/src/hw/ahci.c +++ b/src/hw/ahci.c @@ -637,7 +637,7 @@ static void ahci_controller_setup(struct pci_device *pci) { struct ahci_port_s *port; - u32 val, pnr, max; + u32 pnr, max;
if (create_bounce_buf() < 0) return; @@ -660,8 +660,8 @@ ahci_controller_setup(struct pci_device *pci)
pci_enable_busmaster(pci);
- val = ahci_ctrl_readl(ctrl, HOST_CTL); - ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN); + ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_RESET); + ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_AHCI_EN);
ctrl->caps = ahci_ctrl_readl(ctrl, HOST_CAP); ctrl->ports = ahci_ctrl_readl(ctrl, HOST_PORTS_IMPL);
Indeed, the patch you provided works for both CSM in QEMU mode and QEMU's bios.bin mode.
Testing Results (For this whole time, just for reference):
EDK2 Build Revision: e7d7f02c8e157e936855a091948757f78c7d0298 SeaBIOS Build Revision: df9dd418b3b0e586cb208125094620fc7f90f23d (SeaBIOS with your patch applied from your last message) QEMU Build Revision: 621da7789083b80d6f1ff1c0fb499334007b4f51
QEMU command line I used is too long, but if you want it anyways, please do ask!
In short, I used pc-i440fx-7.1 with a Vista Install CD and Vista installed QCOW2 Image, and switched between the firmware with the -bios command-line option.
- Christopher Lentocha christopherericlentocha@gmail.com
On 2/6/25 6:14 AM, Gerd Hoffmann wrote:
On Wed, Feb 05, 2025 at 10:53:04AM -0500, Christopher Lentocha wrote:
Is it already pushed?
Nope, apparently something went wrong ...
I think I need to add a `if (CONFIG_CSM) {` to it
because I think it wasn't working with the regual bios.bin without CSM
Does a controller reset (see below) work in both cases?
take care, Gerd
commit 79d50d3c156dfd15ed710d0e2313d113251aeb2e Author: Gerd Hoffmann kraxel@redhat.com Date: Thu Feb 6 12:10:21 2025 +0100
ahci: add controller resetdiff --git a/src/hw/ahci.c b/src/hw/ahci.c index 4f0f640a1c64..2285d33d4ae2 100644 --- a/src/hw/ahci.c +++ b/src/hw/ahci.c @@ -637,7 +637,7 @@ static void ahci_controller_setup(struct pci_device *pci) { struct ahci_port_s *port;
- u32 val, pnr, max;
u32 pnr, max;
if (create_bounce_buf() < 0) return;
@@ -660,8 +660,8 @@ ahci_controller_setup(struct pci_device *pci)
pci_enable_busmaster(pci);
- val = ahci_ctrl_readl(ctrl, HOST_CTL);
- ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN);
ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_RESET);
ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_AHCI_EN);
ctrl->caps = ahci_ctrl_readl(ctrl, HOST_CAP); ctrl->ports = ahci_ctrl_readl(ctrl, HOST_PORTS_IMPL);
On Thu, Feb 06, 2025 at 07:23:44AM -0500, Christopher Lentocha wrote:
Indeed, the patch you provided works for both CSM in QEMU mode and QEMU's bios.bin mode.
Testing Results (For this whole time, just for reference):
EDK2 Build Revision: e7d7f02c8e157e936855a091948757f78c7d0298 SeaBIOS Build Revision: df9dd418b3b0e586cb208125094620fc7f90f23d (SeaBIOS with your patch applied from your last message) QEMU Build Revision: 621da7789083b80d6f1ff1c0fb499334007b4f51
QEMU command line I used is too long, but if you want it anyways, please do ask!
In short, I used pc-i440fx-7.1 with a Vista Install CD and Vista installed QCOW2 Image, and switched between the firmware with the -bios command-line option.
Then add a sata controller via -device I assume? Because 'pc-i440fx-7.1' uses ide storage by default.
I've tested (the non-csm case only) using '-M q35' which uses ahci by default. More convenient as you can simply use '-hda' and '-cdrom' switches to add sata disk + cdrom (but could very well be that the vista image doesn't boot then if installed using '-M pc').
take care, Gerd
See below for the command I am now using (I changed it to Q35 since the last message I sent), for EDK2, I used -bios OVMF.fd instead and removed -bios bios.bin (which is SeaBIOS).
- Christopher Lentocha
qemu-system-x86_64 \
-name debug-threads=on \
-machine pc-q35-2.10,usb=off,vmport=off,smm=on,kernel_irqchip=on,dump-guest-core=off \
-accel kvm \
-m 8192 \
-overcommit mem-lock=off \
-smp 4,sockets=1,dies=1,cores=2,threads=2 \
-no-user-config \
-nodefaults \
-chardev socket,id=charmonitor,server=on,wait=off \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=localtime,clock=rt \
-global kvm-pit.lost_tick_policy=discard \
-global ICH9-LPC.disable_s3=0 \
-global ICH9-LPC.disable_s4=0 \
-boot menu=on,strict=on \
-device "{'driver':'ich9-usb-ehci1','id':'usb','bus':'pcie.0','addr':'0x2'}" \
-device "{'driver':'virtio-serial-pci','id':'virtio-serial0','bus':'pcie.0','addr':'0x9'}" \
-blockdev "{'driver':'file','filename':'Vista.qcow2','node-name':'libvirt-1-storage','auto-read-only':true,'discard':'unmap'}" \
-blockdev "{'node-name':'libvirt-1-format','read-only':false,'driver':'qcow2','file':'libvirt-1-storage','backing':null}" \
-device "{'driver':'ide-hd','bus':'ide.0','drive':'libvirt-1-format','id':'sata0-0-0'}" \
-netdev user,id=hostnet0 \
-device "{'driver':'e1000e','netdev':'hostnet0','id':'net0','bus':'pcie.0','addr':'0x8'}" \
-chardev spicevmc,id=charchannel0,name=vdagent \
-device "{'driver':'virtserialport','bus':'virtio-serial0.0','nr':1,'chardev':'charchannel0','id':'channel0','name':'com.redhat.spice.0'}" \
-chardev spiceport,id=charchannel1,name=org.spice-space.webdav.0 \
-device "{'driver':'virtserialport','bus':'virtio-serial0.0','nr':2,'chardev':'charchannel1','id':'channel1','name':'org.spice-space.webdav.0'}" \
-device "{'driver':'usb-tablet','id':'input2','bus':'usb.0','port':'1'}" \
-device "{'driver':'usb-kbd','id':'input3','bus':'usb.0','port':'2'}" \
-audiodev "{'id':'audio1','driver':'spice'}" \
-spice port=0,disable-ticketing=on,seamless-migration=on \
-device "{'driver':'vmware-svga','id':'video0','vgamem_mb':512,'bus':'pcie.0','addr':'0xf'}" \
-device "{'driver':'ich9-intel-hda','id':'sound0','bus':'pcie.0','addr':'0xe'}" \
-device "{'driver':'hda-duplex','id':'sound0-codec0','bus':'sound0.0','cad':0,'audiodev':'audio1'}" \
-machine pcspk-audiodev=audio1 \
-msg timestamp=on \
-display gtk \
-cdrom Vista.iso
-bios bios.bin
On 2/7/25 07:41, Gerd Hoffmann wrote:
On Thu, Feb 06, 2025 at 07:23:44AM -0500, Christopher Lentocha wrote:
Indeed, the patch you provided works for both CSM in QEMU mode and QEMU's bios.bin mode.
Testing Results (For this whole time, just for reference):
EDK2 Build Revision: e7d7f02c8e157e936855a091948757f78c7d0298 SeaBIOS Build Revision: df9dd418b3b0e586cb208125094620fc7f90f23d (SeaBIOS with your patch applied from your last message) QEMU Build Revision: 621da7789083b80d6f1ff1c0fb499334007b4f51
QEMU command line I used is too long, but if you want it anyways, please do ask!
In short, I used pc-i440fx-7.1 with a Vista Install CD and Vista installed QCOW2 Image, and switched between the firmware with the -bios command-line option.
Then add a sata controller via -device I assume? Because 'pc-i440fx-7.1' uses ide storage by default.
I've tested (the non-csm case only) using '-M q35' which uses ahci by default. More convenient as you can simply use '-hda' and '-cdrom' switches to add sata disk + cdrom (but could very well be that the vista image doesn't boot then if installed using '-M pc').
take care, Gerd
On Thu, Feb 06, 2025 at 12:14:31PM +0100, Gerd Hoffmann wrote:
On Wed, Feb 05, 2025 at 10:53:04AM -0500, Christopher Lentocha wrote:
Is it already pushed?
Nope, apparently something went wrong ...
I think I need to add a `if (CONFIG_CSM) {` to it
because I think it wasn't working with the regual bios.bin without CSM
Does a controller reset (see below) work in both cases?
take care, Gerd
commit 79d50d3c156dfd15ed710d0e2313d113251aeb2e Author: Gerd Hoffmann kraxel@redhat.com Date: Thu Feb 6 12:10:21 2025 +0100
ahci: add controller reset
Patch pushed now.
take care, Gerd
-
Christopher Lentocha -
Gerd Hoffmann -
Kevin O'Connor