diff mbox

[v2,1/1] i386: expose floppy-related objects in SSDT

Message ID 1450251909-1599-1-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev Dec. 16, 2015, 7:45 a.m. UTC
From: Roman Kagan <rkagan@virtuozzo.com>

On x86-based systems Linux determines the presence and the type of
floppy drives via a query of a CMOS field.  So does SeaBIOS when
populating the return data for int 0x13 function 0x08.

Windows doesn't; instead, it requests this information from BIOS via int
0x13/0x08 or through ACPI objects _FDE (Floppy Drive Enumerate) and _FDI
(Floppy Drive Information).  On UEFI systems only ACPI-based detection
is supported.

QEMU used not to provide those objects in its ACPI tables; as a result
floppy drives were invisible to Windows on UEFI/OVMF.

This patch implements those objects in SSDT, populating them via AML
API.  For that, a couple of functions are extern-ified to facilitate
populating those objects in acpi-build.c.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Igor Mammedov <imammedo@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
- The patch is made against QEMU upstream

 hw/block/fdc.c         | 11 +++++++
 hw/i386/acpi-build.c   | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/pc.c           | 46 +++++++++++++++++------------
 include/hw/block/fdc.h |  2 ++
 include/hw/i386/pc.h   |  3 ++
 5 files changed, 121 insertions(+), 19 deletions(-)

Comments

John Snow Dec. 16, 2015, 4:46 p.m. UTC | #1
On 12/16/2015 02:45 AM, Denis V. Lunev wrote:
> From: Roman Kagan <rkagan@virtuozzo.com>
> 
> On x86-based systems Linux determines the presence and the type of
> floppy drives via a query of a CMOS field.  So does SeaBIOS when
> populating the return data for int 0x13 function 0x08.
> 
> Windows doesn't; instead, it requests this information from BIOS via int
> 0x13/0x08 or through ACPI objects _FDE (Floppy Drive Enumerate) and _FDI
> (Floppy Drive Information).  On UEFI systems only ACPI-based detection
> is supported.
> 
> QEMU used not to provide those objects in its ACPI tables; as a result
> floppy drives were invisible to Windows on UEFI/OVMF.
> 
> This patch implements those objects in SSDT, populating them via AML
> API.  For that, a couple of functions are extern-ified to facilitate
> populating those objects in acpi-build.c.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Igor Mammedov <imammedo@redhat.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
> - The patch is made against QEMU upstream
> 
>  hw/block/fdc.c         | 11 +++++++
>  hw/i386/acpi-build.c   | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/i386/pc.c           | 46 +++++++++++++++++------------
>  include/hw/block/fdc.h |  2 ++
>  include/hw/i386/pc.h   |  3 ++
>  5 files changed, 121 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 4292ece..c858c5f 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2408,6 +2408,17 @@ FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
>      return isa->state.drives[i].drive;
>  }
>  
> +void isa_fdc_get_drive_geometry(ISADevice *fdc, int i, uint8_t *cylinders,
> +                                uint8_t *heads, uint8_t *sectors)
> +{
> +    FDCtrlISABus *isa = ISA_FDC(fdc);
> +    FDrive *drv = &isa->state.drives[i];
> +
> +    *cylinders = drv->max_track;
> +    *heads = (drv->flags & FDISK_DBL_SIDES) ? 2 : 1;
> +    *sectors = drv->last_sect;
> +}
> +
>  static const VMStateDescription vmstate_isa_fdc ={
>      .name = "fdc",
>      .version_id = 2,
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 95e0c65..7f902b1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -38,6 +38,7 @@
>  #include "hw/acpi/bios-linker-loader.h"
>  #include "hw/loader.h"
>  #include "hw/isa/isa.h"
> +#include "hw/block/fdc.h"
>  #include "hw/acpi/memory_hotplug.h"
>  #include "sysemu/tpm.h"
>  #include "hw/acpi/tpm.h"
> @@ -105,6 +106,13 @@ typedef struct AcpiPmInfo {
>      uint16_t pcihp_io_len;
>  } AcpiPmInfo;
>  
> +typedef struct AcpiFDInfo {
> +    uint8_t type;
> +    uint8_t cylinders;
> +    uint8_t heads;
> +    uint8_t sectors;
> +} AcpiFDInfo;
> +
>  typedef struct AcpiMiscInfo {
>      bool has_hpet;
>      TPMVersion tpm_version;
> @@ -112,6 +120,7 @@ typedef struct AcpiMiscInfo {
>      unsigned dsdt_size;
>      uint16_t pvpanic_port;
>      uint16_t applesmc_io_base;
> +    AcpiFDInfo fdinfo[2];
>  } AcpiMiscInfo;
>  
>  typedef struct AcpiBuildPciBusHotplugState {
> @@ -235,10 +244,25 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
>  
>  static void acpi_get_misc_info(AcpiMiscInfo *info)
>  {
> +    int i;
> +    ISADevice *fdc;
> +
>      info->has_hpet = hpet_find();
>      info->tpm_version = tpm_get_version();
>      info->pvpanic_port = pvpanic_port();
>      info->applesmc_io_base = applesmc_port();
> +
> +    fdc = pc_find_fdc0();
> +    if (fdc != NULL) {
> +        for (i = 0; i < ARRAY_SIZE(info->fdinfo); i++) {
> +            AcpiFDInfo *fdinfo = &info->fdinfo[i];
> +            fdinfo->type = isa_fdc_get_drive_type(fdc, i);
> +            if (fdinfo->type < FDRIVE_DRV_NONE) {
> +                isa_fdc_get_drive_geometry(fdc, i, &fdinfo->cylinders,
> +                                           &fdinfo->heads, &fdinfo->sectors);
> +            }
> +        }
> +    }
>  }
>  
>  /*
> @@ -900,6 +924,40 @@ static Aml *build_crs(PCIHostState *host,
>      return crs;
>  }
>  
> +static Aml *build_fdinfo_aml(int idx, AcpiFDInfo *fdinfo)
> +{
> +    Aml *dev, *fdi;
> +
> +    dev = aml_device("FLP%c", 'A' + idx);
> +
> +    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
> +
> +    fdi = aml_package(0x10);
> +    aml_append(fdi, aml_int(idx));  /* Drive Number */
> +    aml_append(fdi,
> +        aml_int(cmos_get_fd_drive_type(fdinfo->type)));  /* Device Type */
> +    aml_append(fdi,
> +        aml_int(fdinfo->cylinders - 1));  /* Maximum Cylinder Number */
> +    aml_append(fdi, aml_int(fdinfo->sectors));  /* Maximum Sector Number */
> +    aml_append(fdi, aml_int(fdinfo->heads - 1));  /* Maximum Head Number */
> +    /* SeaBIOS returns the below values for int 0x13 func 0x08 regardless of
> +     * the drive type, so shall we */
> +    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
> +    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
> +    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
> +    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
> +    aml_append(fdi, aml_int(0x12));  /* disk_eot */
> +    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
> +    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
> +    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
> +    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
> +    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
> +    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
> +
> +    aml_append(dev, aml_name_decl("_FDI", fdi));
> +    return dev;
> +}
> +
>  static void
>  build_ssdt(GArray *table_data, GArray *linker,
>             AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
> @@ -1125,6 +1183,26 @@ build_ssdt(GArray *table_data, GArray *linker,
>          aml_append(ssdt, scope);
>      }
>  
> +    {
> +        uint32_t fde_buf[5] = {0, 0, 0, 0, 0x2};
> +
> +        scope = aml_scope("\\_SB.PCI0.ISA.FDC0");
> +
> +        for (i = 0; i < ARRAY_SIZE(misc->fdinfo); i++) {
> +            AcpiFDInfo *fdinfo = &misc->fdinfo[i];
> +            if (fdinfo->type != FDRIVE_DRV_NONE) {
> +                fde_buf[i] = 0x1;
> +                aml_append(scope, build_fdinfo_aml(i, fdinfo));
> +            }
> +        }
> +
> +        aml_append(scope,
> +            aml_name_decl("_FDE",
> +                aml_buffer(sizeof(fde_buf), (uint8_t *) fde_buf)));
> +
> +        aml_append(ssdt, scope);
> +    }
> +
>      sb_scope = aml_scope("\\_SB");
>      {
>          /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5e20e07..b7b8774 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -207,7 +207,7 @@ static void pic_irq_request(void *opaque, int irq, int level)
>  
>  #define REG_EQUIPMENT_BYTE          0x14
>  
> -static int cmos_get_fd_drive_type(FDriveType fd0)
> +int cmos_get_fd_drive_type(FDriveType fd0)
>  {
>      int val;
>  
> @@ -368,6 +368,31 @@ static const char * const fdc_container_path[] = {
>      "/unattached", "/peripheral", "/peripheral-anon"
>  };
>  
> +/*
> + * Locate the FDC at IO address 0x3f0, in order to configure the CMOS registers
> + * and ACPI objects.
> + */
> +ISADevice *pc_find_fdc0(void)
> +{
> +    int i;
> +    Object *container;
> +    CheckFdcState state = { 0 };
> +
> +    for (i = 0; i < ARRAY_SIZE(fdc_container_path); i++) {
> +        container = container_get(qdev_get_machine(), fdc_container_path[i]);
> +        object_child_foreach(container, check_fdc, &state);
> +    }
> +
> +    if (state.multiple) {
> +        error_report("warning: multiple floppy disk controllers with "
> +                     "iobase=0x3f0 have been found;\n"
> +                     "the one being picked for CMOS setup might not reflect "
> +                     "your intent");
> +    }
> +
> +    return state.floppy;
> +}
> +

Hmm, Didn't Laszlo Ersek work on a similar problem for 2.4? Oh, yeah,
that's literally his code you've factored out from below. :)

>  static void pc_cmos_init_late(void *opaque)
>  {
>      pc_cmos_init_late_arg *arg = opaque;
> @@ -376,8 +401,6 @@ static void pc_cmos_init_late(void *opaque)
>      int8_t heads, sectors;
>      int val;
>      int i, trans;
> -    Object *container;
> -    CheckFdcState state = { 0 };
>  
>      val = 0;
>      if (ide_get_geometry(arg->idebus[0], 0,
> @@ -407,22 +430,7 @@ static void pc_cmos_init_late(void *opaque)
>      }
>      rtc_set_memory(s, 0x39, val);
>  
> -    /*
> -     * Locate the FDC at IO address 0x3f0, and configure the CMOS registers
> -     * accordingly.
> -     */
> -    for (i = 0; i < ARRAY_SIZE(fdc_container_path); i++) {
> -        container = container_get(qdev_get_machine(), fdc_container_path[i]);
> -        object_child_foreach(container, check_fdc, &state);
> -    }
> -
> -    if (state.multiple) {
> -        error_report("warning: multiple floppy disk controllers with "
> -                     "iobase=0x3f0 have been found;\n"
> -                     "the one being picked for CMOS setup might not reflect "
> -                     "your intent");
> -    }
> -    pc_cmos_init_floppy(s, state.floppy);
> +    pc_cmos_init_floppy(s, pc_find_fdc0());
>  
>      qemu_unregister_reset(pc_cmos_init_late, opaque);
>  }
> diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
> index d48b2f8..adaf3dc 100644
> --- a/include/hw/block/fdc.h
> +++ b/include/hw/block/fdc.h
> @@ -22,5 +22,7 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
>                         DriveInfo **fds, qemu_irq *fdc_tc);
>  
>  FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
> +void isa_fdc_get_drive_geometry(ISADevice *fdc, int i, uint8_t *cylinders,
> +                                uint8_t *heads, uint8_t *sectors);
>  
>  #endif
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 854c330..32ceb2f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -211,6 +211,9 @@ typedef void (*cpu_set_smm_t)(int smm, void *arg);
>  
>  void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
>  
> +ISADevice *pc_find_fdc0(void);
> +int cmos_get_fd_drive_type(FDriveType fd0);
> +
>  /* acpi_piix.c */
>  
>  I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> 

fdc.[ch]:
Acked-by: John Snow <jsnow@redhat.com>
Igor Mammedov Dec. 16, 2015, 4:46 p.m. UTC | #2
On Wed, 16 Dec 2015 10:45:09 +0300
"Denis V. Lunev" <den@openvz.org> wrote:

> From: Roman Kagan <rkagan@virtuozzo.com>
> 
> On x86-based systems Linux determines the presence and the type of
> floppy drives via a query of a CMOS field.  So does SeaBIOS when
> populating the return data for int 0x13 function 0x08.
> 
> Windows doesn't; instead, it requests this information from BIOS via
> int 0x13/0x08 or through ACPI objects _FDE (Floppy Drive Enumerate)
> and _FDI (Floppy Drive Information).  On UEFI systems only ACPI-based
> detection is supported.
> 
> QEMU used not to provide those objects in its ACPI tables; as a result
> floppy drives were invisible to Windows on UEFI/OVMF.
> 
> This patch implements those objects in SSDT, populating them via AML
> API.  For that, a couple of functions are extern-ified to facilitate
> populating those objects in acpi-build.c.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Igor Mammedov <imammedo@redhat.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
> - The patch is made against QEMU upstream
> 
>  hw/block/fdc.c         | 11 +++++++
>  hw/i386/acpi-build.c   | 78
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/i386/pc.c           | 46 +++++++++++++++++------------
> include/hw/block/fdc.h |  2 ++ include/hw/i386/pc.h   |  3 ++
>  5 files changed, 121 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 4292ece..c858c5f 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2408,6 +2408,17 @@ FDriveType isa_fdc_get_drive_type(ISADevice
> *fdc, int i) return isa->state.drives[i].drive;
>  }
>  
> +void isa_fdc_get_drive_geometry(ISADevice *fdc, int i, uint8_t
> *cylinders,
> +                                uint8_t *heads, uint8_t *sectors)
> +{
> +    FDCtrlISABus *isa = ISA_FDC(fdc);
> +    FDrive *drv = &isa->state.drives[i];
> +
> +    *cylinders = drv->max_track;
> +    *heads = (drv->flags & FDISK_DBL_SIDES) ? 2 : 1;
> +    *sectors = drv->last_sect;
> +}
> +
>  static const VMStateDescription vmstate_isa_fdc ={
>      .name = "fdc",
>      .version_id = 2,
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 95e0c65..7f902b1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -38,6 +38,7 @@
>  #include "hw/acpi/bios-linker-loader.h"
>  #include "hw/loader.h"
>  #include "hw/isa/isa.h"
> +#include "hw/block/fdc.h"
>  #include "hw/acpi/memory_hotplug.h"
>  #include "sysemu/tpm.h"
>  #include "hw/acpi/tpm.h"
> @@ -105,6 +106,13 @@ typedef struct AcpiPmInfo {
>      uint16_t pcihp_io_len;
>  } AcpiPmInfo;
>  
> +typedef struct AcpiFDInfo {
> +    uint8_t type;
> +    uint8_t cylinders;
> +    uint8_t heads;
> +    uint8_t sectors;
> +} AcpiFDInfo;
> +
>  typedef struct AcpiMiscInfo {
>      bool has_hpet;
>      TPMVersion tpm_version;
> @@ -112,6 +120,7 @@ typedef struct AcpiMiscInfo {
>      unsigned dsdt_size;
>      uint16_t pvpanic_port;
>      uint16_t applesmc_io_base;
> +    AcpiFDInfo fdinfo[2];
>  } AcpiMiscInfo;
>  
>  typedef struct AcpiBuildPciBusHotplugState {
> @@ -235,10 +244,25 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
>  
>  static void acpi_get_misc_info(AcpiMiscInfo *info)
>  {
> +    int i;
> +    ISADevice *fdc;
> +
>      info->has_hpet = hpet_find();
>      info->tpm_version = tpm_get_version();
>      info->pvpanic_port = pvpanic_port();
>      info->applesmc_io_base = applesmc_port();
> +
> +    fdc = pc_find_fdc0();
> +    if (fdc != NULL) {
> +        for (i = 0; i < ARRAY_SIZE(info->fdinfo); i++) {
> +            AcpiFDInfo *fdinfo = &info->fdinfo[i];
> +            fdinfo->type = isa_fdc_get_drive_type(fdc, i);
> +            if (fdinfo->type < FDRIVE_DRV_NONE) {
> +                isa_fdc_get_drive_geometry(fdc, i,
> &fdinfo->cylinders,
> +                                           &fdinfo->heads,
> &fdinfo->sectors);
> +            }
> +        }
> +    }
>  }
>  
>  /*
> @@ -900,6 +924,40 @@ static Aml *build_crs(PCIHostState *host,
>      return crs;
>  }
>  
> +static Aml *build_fdinfo_aml(int idx, AcpiFDInfo *fdinfo)
> +{
> +    Aml *dev, *fdi;
> +
> +    dev = aml_device("FLP%c", 'A' + idx);
> +
> +    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
> +
> +    fdi = aml_package(0x10);
> +    aml_append(fdi, aml_int(idx));  /* Drive Number */
> +    aml_append(fdi,
> +        aml_int(cmos_get_fd_drive_type(fdinfo->type)));  /* Device
> Type */
> +    aml_append(fdi,
> +        aml_int(fdinfo->cylinders - 1));  /* Maximum Cylinder Number
> */
> +    aml_append(fdi, aml_int(fdinfo->sectors));  /* Maximum Sector
> Number */
> +    aml_append(fdi, aml_int(fdinfo->heads - 1));  /* Maximum Head
> Number */
> +    /* SeaBIOS returns the below values for int 0x13 func 0x08
> regardless of
> +     * the drive type, so shall we */
> +    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
> +    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
> +    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
> +    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
> +    aml_append(fdi, aml_int(0x12));  /* disk_eot */
> +    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
> +    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
> +    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
> +    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
> +    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
> +    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
> +
> +    aml_append(dev, aml_name_decl("_FDI", fdi));
> +    return dev;
> +}
> +
>  static void
>  build_ssdt(GArray *table_data, GArray *linker,
>             AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
> @@ -1125,6 +1183,26 @@ build_ssdt(GArray *table_data, GArray *linker,
>          aml_append(ssdt, scope);
>      }
>  
> +    {
add this block only if floppy controller is present

> +        uint32_t fde_buf[5] = {0, 0, 0, 0, 0x2};
numbers in fde_buf[] must be LE encoded

> +
> +        scope = aml_scope("\\_SB.PCI0.ISA.FDC0");
> +
> +        for (i = 0; i < ARRAY_SIZE(misc->fdinfo); i++) {
> +            AcpiFDInfo *fdinfo = &misc->fdinfo[i];
> +            if (fdinfo->type != FDRIVE_DRV_NONE) {
> +                fde_buf[i] = 0x1;
> +                aml_append(scope, build_fdinfo_aml(i, fdinfo));
> +            }
> +        }
> +
> +        aml_append(scope,
> +            aml_name_decl("_FDE",
> +                aml_buffer(sizeof(fde_buf), (uint8_t *) fde_buf)));
> +
> +        aml_append(ssdt, scope);
> +    }
> +
to minimize conflicts with existing series under review,
I'd suggest to put it before:

  if (misc->applesmc_io_base) {


>      sb_scope = aml_scope("\\_SB");
>      {
>          /* create PCI0.PRES device and its _CRS to reserve CPU
> hotplug MMIO */ diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5e20e07..b7b8774 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -207,7 +207,7 @@ static void pic_irq_request(void *opaque, int
> irq, int level) 
>  #define REG_EQUIPMENT_BYTE          0x14
>  
> -static int cmos_get_fd_drive_type(FDriveType fd0)
> +int cmos_get_fd_drive_type(FDriveType fd0)
>  {
>      int val;
>  
> @@ -368,6 +368,31 @@ static const char * const fdc_container_path[] =
> { "/unattached", "/peripheral", "/peripheral-anon"
>  };
>  
> +/*
> + * Locate the FDC at IO address 0x3f0, in order to configure the
> CMOS registers
> + * and ACPI objects.
> + */
> +ISADevice *pc_find_fdc0(void)
> +{
> +    int i;
> +    Object *container;
> +    CheckFdcState state = { 0 };
> +
> +    for (i = 0; i < ARRAY_SIZE(fdc_container_path); i++) {
> +        container = container_get(qdev_get_machine(),
> fdc_container_path[i]);
> +        object_child_foreach(container, check_fdc, &state);
> +    }
> +
> +    if (state.multiple) {
> +        error_report("warning: multiple floppy disk controllers with
> "
> +                     "iobase=0x3f0 have been found;\n"
> +                     "the one being picked for CMOS setup might not
> reflect "
> +                     "your intent");
> +    }
> +
> +    return state.floppy;
> +}
> +
>  static void pc_cmos_init_late(void *opaque)
>  {
>      pc_cmos_init_late_arg *arg = opaque;
> @@ -376,8 +401,6 @@ static void pc_cmos_init_late(void *opaque)
>      int8_t heads, sectors;
>      int val;
>      int i, trans;
> -    Object *container;
> -    CheckFdcState state = { 0 };
>  
>      val = 0;
>      if (ide_get_geometry(arg->idebus[0], 0,
> @@ -407,22 +430,7 @@ static void pc_cmos_init_late(void *opaque)
>      }
>      rtc_set_memory(s, 0x39, val);
>  
> -    /*
> -     * Locate the FDC at IO address 0x3f0, and configure the CMOS
> registers
> -     * accordingly.
> -     */
> -    for (i = 0; i < ARRAY_SIZE(fdc_container_path); i++) {
> -        container = container_get(qdev_get_machine(),
> fdc_container_path[i]);
> -        object_child_foreach(container, check_fdc, &state);
> -    }
> -
> -    if (state.multiple) {
> -        error_report("warning: multiple floppy disk controllers with
> "
> -                     "iobase=0x3f0 have been found;\n"
> -                     "the one being picked for CMOS setup might not
> reflect "
> -                     "your intent");
> -    }
> -    pc_cmos_init_floppy(s, state.floppy);
> +    pc_cmos_init_floppy(s, pc_find_fdc0());
>  
>      qemu_unregister_reset(pc_cmos_init_late, opaque);
>  }
> diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
> index d48b2f8..adaf3dc 100644
> --- a/include/hw/block/fdc.h
> +++ b/include/hw/block/fdc.h
> @@ -22,5 +22,7 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
>                         DriveInfo **fds, qemu_irq *fdc_tc);
>  
>  FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
> +void isa_fdc_get_drive_geometry(ISADevice *fdc, int i, uint8_t
> *cylinders,
> +                                uint8_t *heads, uint8_t *sectors);
>  
>  #endif
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 854c330..32ceb2f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -211,6 +211,9 @@ typedef void (*cpu_set_smm_t)(int smm, void *arg);
>  
>  void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
>  
> +ISADevice *pc_find_fdc0(void);
> +int cmos_get_fd_drive_type(FDriveType fd0);
> +
>  /* acpi_piix.c */
>  
>  I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
Roman Kagan Dec. 16, 2015, 5:34 p.m. UTC | #3
On Wed, Dec 16, 2015 at 05:46:57PM +0100, Igor Mammedov wrote:
> On Wed, 16 Dec 2015 10:45:09 +0300 "Denis V. Lunev" <den@openvz.org>
> wrote:
> > @@ -1125,6 +1183,26 @@ build_ssdt(GArray *table_data, GArray *linker,
> >          aml_append(ssdt, scope);
> >      }
> >  
> > +    {
> add this block only if floppy controller is present

ATM both qemu master (and stable branches) and your patchset define FDC0
unconditionally, making its _STA depend on FDEN field of the containing
ISA device rather than any sort of qemu internal state.  I think it's
more consistent also to always define its _FDE.  The whole definition of
FDC0 can then be made contidional on the presence of the floppy
controller later on top of your patchset.

As for the subdevices representing floppy drives they already are
defined only if present.

> to minimize conflicts with existing series under review,
> I'd suggest to put it before:
> 
>   if (misc->applesmc_io_base) {

No prob, will do in v2.

Thanks,
Roman.
Igor Mammedov Dec. 16, 2015, 10:15 p.m. UTC | #4
On Wed, 16 Dec 2015 20:34:55 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> On Wed, Dec 16, 2015 at 05:46:57PM +0100, Igor Mammedov wrote:
> > On Wed, 16 Dec 2015 10:45:09 +0300 "Denis V. Lunev" <den@openvz.org>
> > wrote:
> > > @@ -1125,6 +1183,26 @@ build_ssdt(GArray *table_data, GArray
> > > *linker, aml_append(ssdt, scope);
> > >      }
> > >  
> > > +    {
> > add this block only if floppy controller is present
> 
> ATM both qemu master (and stable branches) and your patchset define
> FDC0 unconditionally, making its _STA depend on FDEN field of the
> containing ISA device rather than any sort of qemu internal state.  I
> think it's more consistent also to always define its _FDE.  The whole
> definition of FDC0 can then be made contidional on the presence of
> the floppy controller later on top of your patchset.
Ok, I don't insist on it as I'll have to do anyway for whole FDC0 after
merging SSDT into DSDT.

The reason why I haven't done it in conversion patchset, is to keep
DSDT exactly the same as an original one, so 'make check' could prove
conversion correctness.

> 
> As for the subdevices representing floppy drives they already are
> defined only if present.
> 
> > to minimize conflicts with existing series under review,
> > I'd suggest to put it before:
> > 
> >   if (misc->applesmc_io_base) {
> 
> No prob, will do in v2.
> 
> Thanks,
> Roman.
Roman Kagan Dec. 17, 2015, 1:26 p.m. UTC | #5
On Wed, Dec 16, 2015 at 11:15:55PM +0100, Igor Mammedov wrote:
> On Wed, 16 Dec 2015 20:34:55 +0300
> Roman Kagan <rkagan@virtuozzo.com> wrote:
> 
> > On Wed, Dec 16, 2015 at 05:46:57PM +0100, Igor Mammedov wrote:
> > > On Wed, 16 Dec 2015 10:45:09 +0300 "Denis V. Lunev" <den@openvz.org>
> > > wrote:
> > > > @@ -1125,6 +1183,26 @@ build_ssdt(GArray *table_data, GArray
> > > > *linker, aml_append(ssdt, scope);
> > > >      }
> > > >  
> > > > +    {
> > > add this block only if floppy controller is present
> > 
> > ATM both qemu master (and stable branches) and your patchset define
> > FDC0 unconditionally, making its _STA depend on FDEN field of the
> > containing ISA device rather than any sort of qemu internal state.  I
> > think it's more consistent also to always define its _FDE.  The whole
> > definition of FDC0 can then be made contidional on the presence of
> > the floppy controller later on top of your patchset.
> Ok, I don't insist on it as I'll have to do anyway for whole FDC0 after
> merging SSDT into DSDT.
> 
> The reason why I haven't done it in conversion patchset, is to keep
> DSDT exactly the same as an original one, so 'make check' could prove
> conversion correctness.

I just realized that bios-tables-test used to report differences between
the expected and the actual DSDT and SSDT even before the patch but that
would only cause warnings and not a test failure.  With the patch
applied (which adds yet another difference) nothing changes: it still
reports warnings but passes.

Roman.
Igor Mammedov Dec. 17, 2015, 5:08 p.m. UTC | #6
On Thu, 17 Dec 2015 16:26:51 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> On Wed, Dec 16, 2015 at 11:15:55PM +0100, Igor Mammedov wrote:
> > On Wed, 16 Dec 2015 20:34:55 +0300
> > Roman Kagan <rkagan@virtuozzo.com> wrote:
> > 
> > > On Wed, Dec 16, 2015 at 05:46:57PM +0100, Igor Mammedov wrote:
> > > > On Wed, 16 Dec 2015 10:45:09 +0300 "Denis V. Lunev" <den@openvz.org>
> > > > wrote:
> > > > > @@ -1125,6 +1183,26 @@ build_ssdt(GArray *table_data, GArray
> > > > > *linker, aml_append(ssdt, scope);
> > > > >      }
> > > > >  
> > > > > +    {
> > > > add this block only if floppy controller is present
> > > 
> > > ATM both qemu master (and stable branches) and your patchset define
> > > FDC0 unconditionally, making its _STA depend on FDEN field of the
> > > containing ISA device rather than any sort of qemu internal state.  I
> > > think it's more consistent also to always define its _FDE.  The whole
> > > definition of FDC0 can then be made contidional on the presence of
> > > the floppy controller later on top of your patchset.
> > Ok, I don't insist on it as I'll have to do anyway for whole FDC0 after
> > merging SSDT into DSDT.
> > 
> > The reason why I haven't done it in conversion patchset, is to keep
> > DSDT exactly the same as an original one, so 'make check' could prove
> > conversion correctness.
> 
> I just realized that bios-tables-test used to report differences between
> the expected and the actual DSDT and SSDT even before the patch but that
> would only cause warnings and not a test failure.  With the patch
> applied (which adds yet another difference) nothing changes: it still
> reports warnings but passes.

Michael usually updates expected tables after he is done with merging
patches and before sending pull request.
You can send a patch on top of this that does it in case yours would
be the only patch in the batch so he could save several minutes it takes
to update test.

helper script to do update is at:
tests/acpi-test-data/rebuild-expected-aml.sh

> 
> Roman.
Roman Kagan Dec. 18, 2015, 7:32 p.m. UTC | #7
Roman Kagan (2):
  i386: expose floppy-related objects in SSDT
  tests: update expected SSDT for floppy changes

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Igor Mammedov <imammedo@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
changes since v2:
 - explicit endianness for buffer data
 - reorder code to reduce conflicts with dynamic DSDT patchset
 - update test data


 hw/block/fdc.c                       |  11 +++++
 hw/i386/acpi-build.c                 |  78 +++++++++++++++++++++++++++++++++++
 hw/i386/pc.c                         |  46 ++++++++++++---------
 include/hw/block/fdc.h               |   2 +
 include/hw/i386/pc.h                 |   3 ++
 tests/acpi-test-data/pc/SSDT         | Bin 2486 -> 2588 bytes
 tests/acpi-test-data/pc/SSDT.bridge  | Bin 4345 -> 4447 bytes
 tests/acpi-test-data/q35/SSDT        | Bin 691 -> 872 bytes
 tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 889 bytes
 9 files changed, 121 insertions(+), 19 deletions(-)
Roman Kagan Dec. 18, 2015, 7:32 p.m. UTC | #8
Update the expected SSDTs to reflect the changes introduced in the
previous patch.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Igor Mammedov <imammedo@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
changes since v2:
 - new patch

 tests/acpi-test-data/pc/SSDT         | Bin 2486 -> 2588 bytes
 tests/acpi-test-data/pc/SSDT.bridge  | Bin 4345 -> 4447 bytes
 tests/acpi-test-data/q35/SSDT        | Bin 691 -> 872 bytes
 tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 889 bytes
 4 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT
index 210d6a71e58aa34ce8e94121d25bcf58c3bd503c..ba3fa272b8235bdea28eadb0eaa3751f1ef4f59a 100644
GIT binary patch
delta 123
zcmdlcJV%5pIM^jbhKqrLQFJ3$G-Hx0TZ}$Se6Uk|fU~E8XRu?un~SqSbd#Z*Pk<vw
zyrWAH0|!vZQ%FI8fs2L9pG%05YdseemnskoaY=Li=gQ&w#>LOY0aE2ED9$Cq$bbr%
KHYYNMasU8QB^ERQ

delta 24
gcmbOuvQ3yPIM^j*8z%z;<MoYP(Ttl<GX`=109-!@NB{r;

diff --git a/tests/acpi-test-data/pc/SSDT.bridge b/tests/acpi-test-data/pc/SSDT.bridge
index 6e6660b1fbe0bdb7fe0b257cb53c31fa9bb21a14..7350d36cf5c41c9d298d2dca6a893d579897c2c4 100644
GIT binary patch
delta 123
zcmeyVcwdPtIM^j5UXX!-F>WJQG-Hx0TZ}$Se6Uk|fU~E8XRu?un~SqSbd#Z*Pk<vw
zyrWAH0|!vZQ%FI8fs2L9pG%05YdseemnskoaY=Li=gQ&w#>LOY0aE2ED9$Cq$bbr%
KHYYO9;0FNT%ono&

delta 24
gcmcbw^iz>5IM^lRrvL*3qryh6XvWQ_8K>|A0BC0i#{d8T

diff --git a/tests/acpi-test-data/q35/SSDT b/tests/acpi-test-data/q35/SSDT
index 0970c67ddb1456707c0111f7e5e659ef385af408..7d12c6a562c6a7d14099d4819ab469797548549d 100644
GIT binary patch
delta 203
zcmdnY`htxsIM^j5gPDPWaoI*Le#Uwi?ihWR_+Y2_0B27F&tS)RHy3Av=q7tNp8!XW
zct@8Y1`eQ*r;wfi0~ZV5e<)ypv$)oCF>$E^u@ILu*MF`Yu5VoYTpSPsoWKS!!VF-<
gVt~>A|JY3cX>t`5=MrIL0J;^3VSs6~DC0av01bIR_5c6?

delta 24
gcmaFCwwaYHIM^j*GZO;?W9>#Re#Xt`7-um809z#o3IG5A

diff --git a/tests/acpi-test-data/q35/SSDT.bridge b/tests/acpi-test-data/q35/SSDT.bridge
index a77868861754ce0b3333b2b7bc8091c03a53754d..f5c4b44b5c0f8a049f321c5aadcc825c261ba99f 100644
GIT binary patch
delta 203
zcmX@Y`jd?-IM^kml9_>l(QP9aKV!WMcZ@zue6Uk|fU~E8XRu?un~SqSbd$ZCPk<vw
zyrWAH0|!vZQ%FyMfs2LjKNK*)SzPP6n7CAdScprS>pxcx*EcSHE)IwRPGAEVVFoZ_
gF~I2mf9xiJG`R|jbBQoA0No12Fu=4~lyN;H0PbQx+5i9m

delta 24
gcmey#c7&BHIM^lR2onPXqwGd5e#Xt`7*{g_09%v>?f?J)
Michael S. Tsirkin Dec. 22, 2015, 4:41 p.m. UTC | #9
On Fri, Dec 18, 2015 at 10:32:30PM +0300, Roman Kagan wrote:
> Update the expected SSDTs to reflect the changes introduced in the
> previous patch.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Igor Mammedov <imammedo@redhat.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Richard Henderson <rth@twiddle.net>
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>

Something strange is going on here.
If I apply your patch and this one on top, I get
a diff in SSDT.
Please review it manually and repost.

> ---
> changes since v2:
>  - new patch
> 
>  tests/acpi-test-data/pc/SSDT         | Bin 2486 -> 2588 bytes
>  tests/acpi-test-data/pc/SSDT.bridge  | Bin 4345 -> 4447 bytes
>  tests/acpi-test-data/q35/SSDT        | Bin 691 -> 872 bytes
>  tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 889 bytes
>  4 files changed, 0 insertions(+), 0 deletions(-)
> 
> diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT
> index 210d6a71e58aa34ce8e94121d25bcf58c3bd503c..ba3fa272b8235bdea28eadb0eaa3751f1ef4f59a 100644
> GIT binary patch
> delta 123
> zcmdlcJV%5pIM^jbhKqrLQFJ3$G-Hx0TZ}$Se6Uk|fU~E8XRu?un~SqSbd#Z*Pk<vw
> zyrWAH0|!vZQ%FI8fs2L9pG%05YdseemnskoaY=Li=gQ&w#>LOY0aE2ED9$Cq$bbr%
> KHYYNMasU8QB^ERQ
> 
> delta 24
> gcmbOuvQ3yPIM^j*8z%z;<MoYP(Ttl<GX`=109-!@NB{r;
> 
> diff --git a/tests/acpi-test-data/pc/SSDT.bridge b/tests/acpi-test-data/pc/SSDT.bridge
> index 6e6660b1fbe0bdb7fe0b257cb53c31fa9bb21a14..7350d36cf5c41c9d298d2dca6a893d579897c2c4 100644
> GIT binary patch
> delta 123
> zcmeyVcwdPtIM^j5UXX!-F>WJQG-Hx0TZ}$Se6Uk|fU~E8XRu?un~SqSbd#Z*Pk<vw
> zyrWAH0|!vZQ%FI8fs2L9pG%05YdseemnskoaY=Li=gQ&w#>LOY0aE2ED9$Cq$bbr%
> KHYYO9;0FNT%ono&
> 
> delta 24
> gcmcbw^iz>5IM^lRrvL*3qryh6XvWQ_8K>|A0BC0i#{d8T
> 
> diff --git a/tests/acpi-test-data/q35/SSDT b/tests/acpi-test-data/q35/SSDT
> index 0970c67ddb1456707c0111f7e5e659ef385af408..7d12c6a562c6a7d14099d4819ab469797548549d 100644
> GIT binary patch
> delta 203
> zcmdnY`htxsIM^j5gPDPWaoI*Le#Uwi?ihWR_+Y2_0B27F&tS)RHy3Av=q7tNp8!XW
> zct@8Y1`eQ*r;wfi0~ZV5e<)ypv$)oCF>$E^u@ILu*MF`Yu5VoYTpSPsoWKS!!VF-<
> gVt~>A|JY3cX>t`5=MrIL0J;^3VSs6~DC0av01bIR_5c6?
> 
> delta 24
> gcmaFCwwaYHIM^j*GZO;?W9>#Re#Xt`7-um809z#o3IG5A
> 
> diff --git a/tests/acpi-test-data/q35/SSDT.bridge b/tests/acpi-test-data/q35/SSDT.bridge
> index a77868861754ce0b3333b2b7bc8091c03a53754d..f5c4b44b5c0f8a049f321c5aadcc825c261ba99f 100644
> GIT binary patch
> delta 203
> zcmX@Y`jd?-IM^kml9_>l(QP9aKV!WMcZ@zue6Uk|fU~E8XRu?un~SqSbd$ZCPk<vw
> zyrWAH0|!vZQ%FyMfs2LjKNK*)SzPP6n7CAdScprS>pxcx*EcSHE)IwRPGAEVVFoZ_
> gF~I2mf9xiJG`R|jbBQoA0No12Fu=4~lyN;H0PbQx+5i9m
> 
> delta 24
> gcmey#c7&BHIM^lR2onPXqwGd5e#Xt`7*{g_09%v>?f?J)
> 
> -- 
> 2.5.0
Roman Kagan Dec. 23, 2015, 1:08 p.m. UTC | #10
On Tue, Dec 22, 2015 at 06:41:47PM +0200, Michael S. Tsirkin wrote:
> On Fri, Dec 18, 2015 at 10:32:30PM +0300, Roman Kagan wrote:
> > Update the expected SSDTs to reflect the changes introduced in the
> > previous patch.
> > 
> > Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > CC: Michael S. Tsirkin <mst@redhat.com>
> > CC: Igor Mammedov <imammedo@redhat.com>
> > CC: Paolo Bonzini <pbonzini@redhat.com>
> > CC: Richard Henderson <rth@twiddle.net>
> > CC: Eduardo Habkost <ehabkost@redhat.com>
> > CC: John Snow <jsnow@redhat.com>
> > CC: Kevin Wolf <kwolf@redhat.com>
> 
> Something strange is going on here.
> If I apply your patch and this one on top, I get
> a diff in SSDT.

Aren't you by chance applying it on top of other patches that may affect
SSDT?  I double-checked the series on top of

commit 5dc42c186d63b7b338594fc071cf290805dcc5a5
Merge: c595b21 7236975
Author: Peter Maydell <peter.maydell@linaro.org>
Date:   Tue Dec 22 14:21:42 2015 +0000

    Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging


and it passes OK...

Roman.
diff mbox

Patch

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 4292ece..c858c5f 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2408,6 +2408,17 @@  FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
     return isa->state.drives[i].drive;
 }
 
+void isa_fdc_get_drive_geometry(ISADevice *fdc, int i, uint8_t *cylinders,
+                                uint8_t *heads, uint8_t *sectors)
+{
+    FDCtrlISABus *isa = ISA_FDC(fdc);
+    FDrive *drv = &isa->state.drives[i];
+
+    *cylinders = drv->max_track;
+    *heads = (drv->flags & FDISK_DBL_SIDES) ? 2 : 1;
+    *sectors = drv->last_sect;
+}
+
 static const VMStateDescription vmstate_isa_fdc ={
     .name = "fdc",
     .version_id = 2,
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 95e0c65..7f902b1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -38,6 +38,7 @@ 
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/loader.h"
 #include "hw/isa/isa.h"
+#include "hw/block/fdc.h"
 #include "hw/acpi/memory_hotplug.h"
 #include "sysemu/tpm.h"
 #include "hw/acpi/tpm.h"
@@ -105,6 +106,13 @@  typedef struct AcpiPmInfo {
     uint16_t pcihp_io_len;
 } AcpiPmInfo;
 
+typedef struct AcpiFDInfo {
+    uint8_t type;
+    uint8_t cylinders;
+    uint8_t heads;
+    uint8_t sectors;
+} AcpiFDInfo;
+
 typedef struct AcpiMiscInfo {
     bool has_hpet;
     TPMVersion tpm_version;
@@ -112,6 +120,7 @@  typedef struct AcpiMiscInfo {
     unsigned dsdt_size;
     uint16_t pvpanic_port;
     uint16_t applesmc_io_base;
+    AcpiFDInfo fdinfo[2];
 } AcpiMiscInfo;
 
 typedef struct AcpiBuildPciBusHotplugState {
@@ -235,10 +244,25 @@  static void acpi_get_pm_info(AcpiPmInfo *pm)
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
 {
+    int i;
+    ISADevice *fdc;
+
     info->has_hpet = hpet_find();
     info->tpm_version = tpm_get_version();
     info->pvpanic_port = pvpanic_port();
     info->applesmc_io_base = applesmc_port();
+
+    fdc = pc_find_fdc0();
+    if (fdc != NULL) {
+        for (i = 0; i < ARRAY_SIZE(info->fdinfo); i++) {
+            AcpiFDInfo *fdinfo = &info->fdinfo[i];
+            fdinfo->type = isa_fdc_get_drive_type(fdc, i);
+            if (fdinfo->type < FDRIVE_DRV_NONE) {
+                isa_fdc_get_drive_geometry(fdc, i, &fdinfo->cylinders,
+                                           &fdinfo->heads, &fdinfo->sectors);
+            }
+        }
+    }
 }
 
 /*
@@ -900,6 +924,40 @@  static Aml *build_crs(PCIHostState *host,
     return crs;
 }
 
+static Aml *build_fdinfo_aml(int idx, AcpiFDInfo *fdinfo)
+{
+    Aml *dev, *fdi;
+
+    dev = aml_device("FLP%c", 'A' + idx);
+
+    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
+
+    fdi = aml_package(0x10);
+    aml_append(fdi, aml_int(idx));  /* Drive Number */
+    aml_append(fdi,
+        aml_int(cmos_get_fd_drive_type(fdinfo->type)));  /* Device Type */
+    aml_append(fdi,
+        aml_int(fdinfo->cylinders - 1));  /* Maximum Cylinder Number */
+    aml_append(fdi, aml_int(fdinfo->sectors));  /* Maximum Sector Number */
+    aml_append(fdi, aml_int(fdinfo->heads - 1));  /* Maximum Head Number */
+    /* SeaBIOS returns the below values for int 0x13 func 0x08 regardless of
+     * the drive type, so shall we */
+    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
+    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
+    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
+    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
+    aml_append(fdi, aml_int(0x12));  /* disk_eot */
+    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
+    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
+    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
+    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
+    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
+    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
+
+    aml_append(dev, aml_name_decl("_FDI", fdi));
+    return dev;
+}
+
 static void
 build_ssdt(GArray *table_data, GArray *linker,
            AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
@@ -1125,6 +1183,26 @@  build_ssdt(GArray *table_data, GArray *linker,
         aml_append(ssdt, scope);
     }
 
+    {
+        uint32_t fde_buf[5] = {0, 0, 0, 0, 0x2};
+
+        scope = aml_scope("\\_SB.PCI0.ISA.FDC0");
+
+        for (i = 0; i < ARRAY_SIZE(misc->fdinfo); i++) {
+            AcpiFDInfo *fdinfo = &misc->fdinfo[i];
+            if (fdinfo->type != FDRIVE_DRV_NONE) {
+                fde_buf[i] = 0x1;
+                aml_append(scope, build_fdinfo_aml(i, fdinfo));
+            }
+        }
+
+        aml_append(scope,
+            aml_name_decl("_FDE",
+                aml_buffer(sizeof(fde_buf), (uint8_t *) fde_buf)));
+
+        aml_append(ssdt, scope);
+    }
+
     sb_scope = aml_scope("\\_SB");
     {
         /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5e20e07..b7b8774 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -207,7 +207,7 @@  static void pic_irq_request(void *opaque, int irq, int level)
 
 #define REG_EQUIPMENT_BYTE          0x14
 
-static int cmos_get_fd_drive_type(FDriveType fd0)
+int cmos_get_fd_drive_type(FDriveType fd0)
 {
     int val;
 
@@ -368,6 +368,31 @@  static const char * const fdc_container_path[] = {
     "/unattached", "/peripheral", "/peripheral-anon"
 };
 
+/*
+ * Locate the FDC at IO address 0x3f0, in order to configure the CMOS registers
+ * and ACPI objects.
+ */
+ISADevice *pc_find_fdc0(void)
+{
+    int i;
+    Object *container;
+    CheckFdcState state = { 0 };
+
+    for (i = 0; i < ARRAY_SIZE(fdc_container_path); i++) {
+        container = container_get(qdev_get_machine(), fdc_container_path[i]);
+        object_child_foreach(container, check_fdc, &state);
+    }
+
+    if (state.multiple) {
+        error_report("warning: multiple floppy disk controllers with "
+                     "iobase=0x3f0 have been found;\n"
+                     "the one being picked for CMOS setup might not reflect "
+                     "your intent");
+    }
+
+    return state.floppy;
+}
+
 static void pc_cmos_init_late(void *opaque)
 {
     pc_cmos_init_late_arg *arg = opaque;
@@ -376,8 +401,6 @@  static void pc_cmos_init_late(void *opaque)
     int8_t heads, sectors;
     int val;
     int i, trans;
-    Object *container;
-    CheckFdcState state = { 0 };
 
     val = 0;
     if (ide_get_geometry(arg->idebus[0], 0,
@@ -407,22 +430,7 @@  static void pc_cmos_init_late(void *opaque)
     }
     rtc_set_memory(s, 0x39, val);
 
-    /*
-     * Locate the FDC at IO address 0x3f0, and configure the CMOS registers
-     * accordingly.
-     */
-    for (i = 0; i < ARRAY_SIZE(fdc_container_path); i++) {
-        container = container_get(qdev_get_machine(), fdc_container_path[i]);
-        object_child_foreach(container, check_fdc, &state);
-    }
-
-    if (state.multiple) {
-        error_report("warning: multiple floppy disk controllers with "
-                     "iobase=0x3f0 have been found;\n"
-                     "the one being picked for CMOS setup might not reflect "
-                     "your intent");
-    }
-    pc_cmos_init_floppy(s, state.floppy);
+    pc_cmos_init_floppy(s, pc_find_fdc0());
 
     qemu_unregister_reset(pc_cmos_init_late, opaque);
 }
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index d48b2f8..adaf3dc 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -22,5 +22,7 @@  void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
                        DriveInfo **fds, qemu_irq *fdc_tc);
 
 FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
+void isa_fdc_get_drive_geometry(ISADevice *fdc, int i, uint8_t *cylinders,
+                                uint8_t *heads, uint8_t *sectors);
 
 #endif
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 854c330..32ceb2f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -211,6 +211,9 @@  typedef void (*cpu_set_smm_t)(int smm, void *arg);
 
 void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
 
+ISADevice *pc_find_fdc0(void);
+int cmos_get_fd_drive_type(FDriveType fd0);
+
 /* acpi_piix.c */
 
 I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,