[Qemu-devel] fdc: refactor device creation causes guest kernel panic

[Qemu-devel] fdc: refactor device creation causes guest kernel panic

[Stefan Hajnoczi]

The following kernel panic occurs when the RHEL6 installer starts on
qemu.git/master:

BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffffa0062ceb>] floppy_ready+0xfb/0x730 [floppy]

For full details see http://pastebin.com/SYE5A6LA.

git-bisect revealed that the following commit causes this panic:

commit 63ffb564dca94f8bda01ed6d209784104630a4d2
Author: Blue Swirl <blauwirbel@gmail.com>
Date:   Sat Feb 5 16:32:23 2011 +0000

    fdc: refactor device creation

    Turn fdc_init_isa into an inline function.

    Get floppy geometry directly from the drives.

    Don't expose FDCtrl.

    Signed-off-by: Blue Swirl <blauwirbel@gmail.com>

The CMOS value at 0x10 has changed from 0x00 to 0x40 but I have not
located the root cause of the problem.

Blue Swirl: Any thoughts on this bug?

Stefan

[Qemu-devel] Re: fdc: refactor device creation causes guest kernel panic

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 1656 bytes --]

On Wed, Mar 9, 2011 at 4:34 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> The following kernel panic occurs when the RHEL6 installer starts on
> qemu.git/master:
>
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffffa0062ceb>] floppy_ready+0xfb/0x730 [floppy]
>
> For full details see http://pastebin.com/SYE5A6LA.
>
> git-bisect revealed that the following commit causes this panic:
>
> commit 63ffb564dca94f8bda01ed6d209784104630a4d2
> Author: Blue Swirl <blauwirbel@gmail.com>
> Date:   Sat Feb 5 16:32:23 2011 +0000
>
>    fdc: refactor device creation
>
>    Turn fdc_init_isa into an inline function.
>
>    Get floppy geometry directly from the drives.
>
>    Don't expose FDCtrl.
>
>    Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>
> The CMOS value at 0x10 has changed from 0x00 to 0x40 but I have not
> located the root cause of the problem.
>
> Blue Swirl: Any thoughts on this bug?

The logic for calculating the drive state in pc.c does not match fdc.c
logic. Please try this patch.

diff --git a/hw/pc.c b/hw/pc.c
index 5966bf1..4d67d9f 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
above_4g_mem_size,
     /* floppy type */
     for (i = 0; i < 2; i++) {
         fd[i] = drive_get(IF_FLOPPY, 0, i);
-        if (fd[i]) {
+        if (fd[i] && bdrv_is_inserted(fd[i]->bdrv)) {
             bdrv_get_floppy_geometry_hint(fd[i]->bdrv, &nb_heads, &max_track,
                                           &last_sect, FDRIVE_DRV_NONE,
                                           &fd_type[i]);

[-- Attachment #2: 0001-pc-fix-wrong-CMOS-values-for-floppy-drives.patch --]
[-- Type: text/x-diff, Size: 1311 bytes --]

From a008e6fd41a31d437670eaa0ddf12352e8a4a8fb Mon Sep 17 00:00:00 2001
Message-Id: <a008e6fd41a31d437670eaa0ddf12352e8a4a8fb.1299926967.git.blauwirbel@gmail.com>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Sat, 12 Mar 2011 09:52:25 +0000
Subject: [PATCH] pc: fix wrong CMOS values for floppy drives

Before commit 63ffb564dca94f8bda01ed6d209784104630a4d2, states for
floppy drives were calculated in fdc.c:fd_revalidate(). There it is
also considered whether a disk is inserted or not. The commit didn't copy
the logic completely to pc.c, which caused a regression.

Fix by adding the same check also to pc.c.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/pc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 5966bf1..4d67d9f 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
     /* floppy type */
     for (i = 0; i < 2; i++) {
         fd[i] = drive_get(IF_FLOPPY, 0, i);
-        if (fd[i]) {
+        if (fd[i] && bdrv_is_inserted(fd[i]->bdrv)) {
             bdrv_get_floppy_geometry_hint(fd[i]->bdrv, &nb_heads, &max_track,
                                           &last_sect, FDRIVE_DRV_NONE,
                                           &fd_type[i]);
-- 
1.7.2.3

[Qemu-devel] Re: fdc: refactor device creation causes guest kernel panic

[Stefan Hajnoczi]

On Sat, Mar 12, 2011 at 10:53 AM, Blue Swirl <blauwirbel@gmail.com> wrote:
> The logic for calculating the drive state in pc.c does not match fdc.c
> logic. Please try this patch.
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 5966bf1..4d67d9f 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
> above_4g_mem_size,
>     /* floppy type */
>     for (i = 0; i < 2; i++) {
>         fd[i] = drive_get(IF_FLOPPY, 0, i);
> -        if (fd[i]) {
> +        if (fd[i] && bdrv_is_inserted(fd[i]->bdrv)) {
>             bdrv_get_floppy_geometry_hint(fd[i]->bdrv, &nb_heads, &max_track,
>                                           &last_sect, FDRIVE_DRV_NONE,
>                                           &fd_type[i]);
>

This patch fixes the problem, thanks!

Tested-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

[Qemu-devel] Re: fdc: refactor device creation causes guest kernel panic

[Blue Swirl]

On Sat, Mar 12, 2011 at 2:58 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Sat, Mar 12, 2011 at 10:53 AM, Blue Swirl <blauwirbel@gmail.com> wrote:
>> The logic for calculating the drive state in pc.c does not match fdc.c
>> logic. Please try this patch.
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 5966bf1..4d67d9f 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
>> above_4g_mem_size,
>>     /* floppy type */
>>     for (i = 0; i < 2; i++) {
>>         fd[i] = drive_get(IF_FLOPPY, 0, i);
>> -        if (fd[i]) {
>> +        if (fd[i] && bdrv_is_inserted(fd[i]->bdrv)) {
>>             bdrv_get_floppy_geometry_hint(fd[i]->bdrv, &nb_heads, &max_track,
>>                                           &last_sect, FDRIVE_DRV_NONE,
>>                                           &fd_type[i]);
>>
>
> This patch fixes the problem, thanks!
>
> Tested-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Thank you for testing, applied.