[Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[Denis V. Lunev]

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;
+}
+
 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,
-- 
2.1.4

Re: [Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[John Snow]

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>

Re: [Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

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,

Re: [Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[Roman Kagan]

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.

Re: [Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

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.

Re: [Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[Roman Kagan]

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.

Re: [Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

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.

[Qemu-devel] [PATCH v3 0/2] i386: expose floppy-related objects in SSDT

[Roman Kagan]

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(-)

-- 
2.5.0

[Qemu-devel] [PATCH v3 1/2] i386: expose floppy-related objects in SSDT

[Roman Kagan]

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>
--
changes since v2:
 - explicit endianness for buffer data
 - reorder code to reduce conflicts with dynamic DSDT patchset

 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 fa866f6..15e50fb 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) {
+        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,
@@ -1071,6 +1129,26 @@ build_ssdt(GArray *table_data, GArray *linker,
     aml_append(scope, aml_name_decl("_S5", pkg));
     aml_append(ssdt, scope);
 
+    {
+        uint32_t fde_buf[5] = {0, 0, 0, 0, cpu_to_le32(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] = cpu_to_le32(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);
+    }
+
     if (misc->applesmc_io_base) {
         scope = aml_scope("\\_SB.PCI0.ISA");
         dev = aml_device("SMC");
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 426424a..c68e55c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -208,7 +208,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;
 
@@ -369,6 +369,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;
@@ -377,8 +402,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,
@@ -408,22 +431,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 4bf4faf..6948e63 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -224,6 +224,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,
-- 
2.5.0

[Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Roman Kagan]

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)

-- 
2.5.0

Re: [Qemu-devel] [PATCH v3 1/2] i386: expose floppy-related objects in SSDT

[Michael S. Tsirkin]

On Fri, Dec 18, 2015 at 10:32:29PM +0300, Roman Kagan wrote:
> 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>
> --

This must be --- otherwise git am does not strip the diff stat.

> changes since v2:
>  - explicit endianness for buffer data
>  - reorder code to reduce conflicts with dynamic DSDT patchset
> 
>  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 fa866f6..15e50fb 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) {
> +        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,
> @@ -1071,6 +1129,26 @@ build_ssdt(GArray *table_data, GArray *linker,
>      aml_append(scope, aml_name_decl("_S5", pkg));
>      aml_append(ssdt, scope);
>  
> +    {
> +        uint32_t fde_buf[5] = {0, 0, 0, 0, cpu_to_le32(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] = cpu_to_le32(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);
> +    }
> +
>      if (misc->applesmc_io_base) {
>          scope = aml_scope("\\_SB.PCI0.ISA");
>          dev = aml_device("SMC");
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 426424a..c68e55c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -208,7 +208,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;
>  
> @@ -369,6 +369,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;
> @@ -377,8 +402,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,
> @@ -408,22 +431,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 4bf4faf..6948e63 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -224,6 +224,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,
> -- 
> 2.5.0

Re: [Qemu-devel] [PATCH v3 1/2] i386: expose floppy-related objects in SSDT

[Roman Kagan]

On Tue, Dec 22, 2015 at 05:07:16PM +0200, Michael S. Tsirkin wrote:
> On Fri, Dec 18, 2015 at 10:32:29PM +0300, Roman Kagan wrote:
> > 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>
> > --
> 
> This must be --- otherwise git am does not strip the diff stat.

OOPS.  I wonder how that happened...  git format-patch certainly did the
right thing; I must have deleted one dash by accident when editing the
changelog.  Want me to resend?

Roman.

Re: [Qemu-devel] [PATCH v3 1/2] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

On Fri, 18 Dec 2015 22:32:29 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> 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>
> --
beside of Michael's comment
for AML part,
 Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> changes since v2:
>  - explicit endianness for buffer data
>  - reorder code to reduce conflicts with dynamic DSDT patchset
> 
>  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 fa866f6..15e50fb 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) {
> +        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,
> @@ -1071,6 +1129,26 @@ build_ssdt(GArray *table_data, GArray *linker,
>      aml_append(scope, aml_name_decl("_S5", pkg));
>      aml_append(ssdt, scope);
>  
> +    {
> +        uint32_t fde_buf[5] = {0, 0, 0, 0, cpu_to_le32(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] = cpu_to_le32(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);
> +    }
> +
>      if (misc->applesmc_io_base) {
>          scope = aml_scope("\\_SB.PCI0.ISA");
>          dev = aml_device("SMC");
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 426424a..c68e55c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -208,7 +208,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;
>  
> @@ -369,6 +369,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;
> @@ -377,8 +402,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,
> @@ -408,22 +431,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 4bf4faf..6948e63 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -224,6 +224,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,

Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Roman Kagan]

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.

Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Michael S. Tsirkin]

On Wed, Dec 23, 2015 at 04:08:19PM +0300, Roman Kagan wrote:
> 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.

This is the actual vs expected diff with both patches applied.

--- /tmp/asl-MN299X.dsl	2015-12-23 15:41:29.369837861 +0200
+++ /tmp/asl-T0S99X.dsl	2015-12-23 15:41:29.373837813 +0200
@@ -5,20 +5,20 @@
  * 
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of /tmp/aml-VVVAAY, Wed Dec 23 15:41:29 2015
+ * Disassembly of tests/acpi-test-data/pc/SSDT, Wed Dec 23 15:41:29 2015
  *
  * Original Table Header:
  *     Signature        "SSDT"
- *     Length           0x000009B6 (2486)
+ *     Length           0x00000A1C (2588)
  *     Revision         0x01
- *     Checksum         0xD7
+ *     Checksum         0x15
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPCSSDT"
  *     OEM Revision     0x00000001 (1)
  *     Compiler ID      "BXPC"
  *     Compiler Version 0x00000001 (1)
  */
-DefinitionBlock ("/tmp/aml-VVVAAY.aml", "SSDT", 1, "BOCHS ", "BXPCSSDT", 0x00000001)
+DefinitionBlock ("tests/acpi-test-data/pc/SSDT.aml", "SSDT", 1, "BOCHS ", "BXPCSSDT", 0x00000001)
 {
 
     External (_SB_.CPEJ, MethodObj)    // 2 Arguments
@@ -26,6 +26,7 @@ DefinitionBlock ("/tmp/aml-VVVAAY.aml",
     External (_SB_.CPST, MethodObj)    // 1 Arguments
     External (_SB_.PCI0, DeviceObj)
     External (_SB_.PCI0.BNUM, FieldUnitObj)
+    External (_SB_.PCI0.ISA_.FDC0, DeviceObj)
     External (_SB_.PCI0.MHPD, DeviceObj)
     External (_SB_.PCI0.PCEJ, MethodObj)    // 2 Arguments
     External (_SB_.PCI0.PCID, FieldUnitObj)
@@ -135,6 +136,40 @@ DefinitionBlock ("/tmp/aml-VVVAAY.aml",
         })
     }
 
+    Scope (\_SB.PCI0.ISA.FDC0)
+    {
+        Device (FLPA)
+        {
+            Name (_ADR, Zero)  // _ADR: Address
+            Name (_FDI, Package (0x10)  // _FDI: Floppy Drive Information
+            {
+                Zero, 
+                0x04, 
+                0x4F, 
+                0x12, 
+                One, 
+                0xAF, 
+                0x02, 
+                0x25, 
+                0x02, 
+                0x12, 
+                0x1B, 
+                0xFF, 
+                0x6C, 
+                0xF6, 
+                0x0F, 
+                0x08
+            })
+        }
+
+        Name (_FDE, Buffer (0x14)  // _FDE: Floppy Disk Enumerate
+        {
+            /* 0000 */  0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  /* ........ */
+            /* 0008 */  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  /* ........ */
+            /* 0010 */  0x02, 0x00, 0x00, 0x00                           /* .... */
+        })
+    }
+
     Scope (\_SB)
     {
         Device (PCI0.PRES)
--- /tmp/asl-NKR59X.dsl	2015-12-23 15:41:30.429825430 +0200
+++ /tmp/asl-3MT59X.dsl	2015-12-23 15:41:30.432825395 +0200
@@ -5,26 +5,27 @@
  * 
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of /tmp/aml-H6V69X, Wed Dec 23 15:41:30 2015
+ * Disassembly of tests/acpi-test-data/q35/SSDT, Wed Dec 23 15:41:30 2015
  *
  * Original Table Header:
  *     Signature        "SSDT"
- *     Length           0x000002B3 (691)
+ *     Length           0x00000368 (872)
  *     Revision         0x01
- *     Checksum         0x7D
+ *     Checksum         0xA6
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "BXPCSSDT"
  *     OEM Revision     0x00000001 (1)
  *     Compiler ID      "BXPC"
  *     Compiler Version 0x00000001 (1)
  */
-DefinitionBlock ("/tmp/aml-H6V69X.aml", "SSDT", 1, "BOCHS ", "BXPCSSDT", 0x00000001)
+DefinitionBlock ("tests/acpi-test-data/q35/SSDT.aml", "SSDT", 1, "BOCHS ", "BXPCSSDT", 0x00000001)
 {
 
     External (_SB_.CPEJ, MethodObj)    // 2 Arguments
     External (_SB_.CPMA, MethodObj)    // 1 Arguments
     External (_SB_.CPST, MethodObj)    // 1 Arguments
     External (_SB_.PCI0, DeviceObj)
+    External (_SB_.PCI0.ISA_.FDC0, DeviceObj)
     External (_SB_.PCI0.MHPD, DeviceObj)
 
     Scope (\_SB.PCI0)
@@ -115,6 +116,64 @@ DefinitionBlock ("/tmp/aml-H6V69X.aml",
         })
     }
 
+    Scope (\_SB.PCI0.ISA.FDC0)
+    {
+        Device (FLPA)
+        {
+            Name (_ADR, Zero)  // _ADR: Address
+            Name (_FDI, Package (0x10)  // _FDI: Floppy Drive Information
+            {
+                Zero, 
+                0x04, 
+                0xFFFFFFFFFFFFFFFF, 
+                Zero, 
+                0xFFFFFFFFFFFFFFFF, 
+                0xAF, 
+                0x02, 
+                0x25, 
+                0x02, 
+                0x12, 
+                0x1B, 
+                0xFF, 
+                0x6C, 
+                0xF6, 
+                0x0F, 
+                0x08
+            })
+        }
+
+        Device (FLPB)
+        {
+            Name (_ADR, One)  // _ADR: Address
+            Name (_FDI, Package (0x10)  // _FDI: Floppy Drive Information
+            {
+                One, 
+                0x04, 
+                0xFFFFFFFFFFFFFFFF, 
+                Zero, 
+                0xFFFFFFFFFFFFFFFF, 
+                0xAF, 
+                0x02, 
+                0x25, 
+                0x02, 
+                0x12, 
+                0x1B, 
+                0xFF, 
+                0x6C, 
+                0xF6, 
+                0x0F, 
+                0x08
+            })
+        }
+
+        Name (_FDE, Buffer (0x14)  // _FDE: Floppy Disk Enumerate
+        {
+            /* 0000 */  0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,  /* ........ */
+            /* 0008 */  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  /* ........ */
+            /* 0010 */  0x02, 0x00, 0x00, 0x00                           /* .... */
+        })
+    }
+
     Scope (\_SB)
     {
         Device (PCI0.PRES)

Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Roman Kagan]

On Wed, Dec 23, 2015 at 03:45:29PM +0200, Michael S. Tsirkin wrote:
> This is the actual vs expected diff with both patches applied.

Interesting...  The diff suggests that qemu running in your test
environment has no floppy drives, while in mine ...

> +    Scope (\_SB.PCI0.ISA.FDC0)
> +    {
> +        Device (FLPA)
> +        {
> +            Name (_ADR, Zero)  // _ADR: Address
> +            Name (_FDI, Package (0x10)  // _FDI: Floppy Drive Information
> +            {
> +                Zero, 
> +                0x04, 
> +                0x4F, 
> +                0x12, 
> +                One, 
[...]
> +            })
> +        }
> +
> +        Name (_FDE, Buffer (0x14)  // _FDE: Floppy Disk Enumerate
> +        {
> +            /* 0000 */  0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  /* ........ */
> +            /* 0008 */  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  /* ........ */
> +            /* 0010 */  0x02, 0x00, 0x00, 0x00                           /* .... */
> +        })
> +    }

... it has one 1.44M drive with correct geometry for pc machine type,
and ...

> +    Scope (\_SB.PCI0.ISA.FDC0)
> +    {
> +        Device (FLPA)
> +        {
> +            Name (_ADR, Zero)  // _ADR: Address
> +            Name (_FDI, Package (0x10)  // _FDI: Floppy Drive Information
> +            {
> +                Zero, 
> +                0x04, 
> +                0xFFFFFFFFFFFFFFFF, 
> +                Zero, 
> +                0xFFFFFFFFFFFFFFFF, 
[...]
> +            })
> +        }
> +
> +        Device (FLPB)
> +        {
> +            Name (_ADR, One)  // _ADR: Address
> +            Name (_FDI, Package (0x10)  // _FDI: Floppy Drive Information
> +            {
> +                One, 
> +                0x04, 
> +                0xFFFFFFFFFFFFFFFF, 
> +                Zero, 
> +                0xFFFFFFFFFFFFFFFF, 
[...]
> +            })
> +        }
> +
> +        Name (_FDE, Buffer (0x14)  // _FDE: Floppy Disk Enumerate
> +        {
> +            /* 0000 */  0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,  /* ........ */
> +            /* 0008 */  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  /* ........ */
> +            /* 0010 */  0x02, 0x00, 0x00, 0x00                           /* .... */
> +        })
> +    }

... two 1.44M drives with bogus geometry for q35.

Looking into this, thanks for the diff.

Roman.

Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Roman Kagan]

On Wed, Dec 23, 2015 at 06:06:31PM +0300, Roman Kagan wrote:
> On Wed, Dec 23, 2015 at 03:45:29PM +0200, Michael S. Tsirkin wrote:
> > This is the actual vs expected diff with both patches applied.
> 
> Interesting...  The diff suggests that qemu running in your test
> environment has no floppy drives

Actually, no.  My patch adds _FDE unconditionally, so I'm positively
unable to tell how you could have obtained this result other than by
not applying it.

> ... two 1.44M drives with bogus geometry for q35.

This one is a bug in my patch, indeed: I was tricked by FDRIVE_DRV_NONE
being non-zero, and forgot to initialize the respective fields in
acpi_get_misc_info() in case there is no floppy controller at all.

I'll resubmit with this bug fixed, but it has nothing to do with 
the diff you're observing.

Roman.

Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Igor Mammedov]

On Wed, 23 Dec 2015 20:20:54 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> On Wed, Dec 23, 2015 at 06:06:31PM +0300, Roman Kagan wrote:
> > On Wed, Dec 23, 2015 at 03:45:29PM +0200, Michael S. Tsirkin wrote:
> > > This is the actual vs expected diff with both patches applied.
> > 
> > Interesting...  The diff suggests that qemu running in your test
> > environment has no floppy drives
> 
> Actually, no.  My patch adds _FDE unconditionally, so I'm positively
> unable to tell how you could have obtained this result other than by
> not applying it.
> 
> > ... two 1.44M drives with bogus geometry for q35.
> 
> This one is a bug in my patch, indeed: I was tricked by FDRIVE_DRV_NONE
> being non-zero, and forgot to initialize the respective fields in
> acpi_get_misc_info() in case there is no floppy controller at all.
so instead of fake initialization, it's worth to make your patch
conditional on presence of controller after all.
i.e. add AML only if controller was present.

> 
> I'll resubmit with this bug fixed, but it has nothing to do with 
> the diff you're observing.
> 
> Roman.
> 

Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Roman Kagan]

On Wed, Dec 23, 2015 at 06:47:16PM +0100, Igor Mammedov wrote:
> On Wed, 23 Dec 2015 20:20:54 +0300
> Roman Kagan <rkagan@virtuozzo.com> wrote:
> > > ... two 1.44M drives with bogus geometry for q35.
> > 
> > This one is a bug in my patch, indeed: I was tricked by FDRIVE_DRV_NONE
> > being non-zero, and forgot to initialize the respective fields in
> > acpi_get_misc_info() in case there is no floppy controller at all.
> so instead of fake initialization, it's worth to make your patch
> conditional on presence of controller after all.
> i.e. add AML only if controller was present.

Indeed :)

Roman.

Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Michael S. Tsirkin]

On Wed, Dec 23, 2015 at 08:51:45PM +0300, Roman Kagan wrote:
> On Wed, Dec 23, 2015 at 06:47:16PM +0100, Igor Mammedov wrote:
> > On Wed, 23 Dec 2015 20:20:54 +0300
> > Roman Kagan <rkagan@virtuozzo.com> wrote:
> > > > ... two 1.44M drives with bogus geometry for q35.
> > > 
> > > This one is a bug in my patch, indeed: I was tricked by FDRIVE_DRV_NONE
> > > being non-zero, and forgot to initialize the respective fields in
> > > acpi_get_misc_info() in case there is no floppy controller at all.
> > so instead of fake initialization, it's worth to make your patch
> > conditional on presence of controller after all.
> > i.e. add AML only if controller was present.
> 
> Indeed :)
> 
> Roman.

Or rather, start series with a patch making FDC conditional,
then update expected ssdt, then tweak methods within -
should not change ssdt since we don't create a floppy in
the test.

-- 
MST

[Qemu-devel] [PATCH v4 0/4] i386: expose floppy-related objects in SSDT

[Roman Kagan]

Windows on UEFI systems is only capable of detecting the presence and
the type of floppy drives via corresponding ACPI objects.

Those objects are added in the last patch of the series; the three
preceding ones pave the way to it, by making the necessary data
public and by moving the whole floppy drive controller description into
runtime-generated SSDT.

Note that the series conflicts with Igor's patchset for dynamic DSDT, in
particular, with "[PATCH 50/74] pc: acpi: move FDC0 device from DSDT
to SSDT"; I haven't managed to avoid that while trying to meet
maintainer's comments.

Roman Kagan (4):
  i386/pc: expose identifying the floppy controller
  i386/acpi: make floppy controller object dynamic
  expose floppy drive geometry and CMOS type
  i386: populate floppy drive information in SSDT

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: qemu-block@nongnu.org
Cc: qemu-stable@nongnu.org
---
changes since v3:
 - make FDC object fully dynamic in a separate patch
 - split out support patches
 - include test data updates with the respective patches to maintain
   bisectability

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                |  92 ++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-dsdt-isa.dsl           |  18 -------
 hw/i386/acpi-dsdt.dsl               |   1 -
 hw/i386/pc.c                        |  46 ++++++++++--------
 hw/i386/q35-acpi-dsdt.dsl           |   7 +--
 include/hw/block/fdc.h              |   2 +
 include/hw/i386/pc.h                |   3 ++
 tests/acpi-test-data/pc/DSDT        | Bin 3028 -> 2946 bytes
 tests/acpi-test-data/pc/SSDT        | Bin 2486 -> 2635 bytes
 tests/acpi-test-data/pc/SSDT.bridge | Bin 4345 -> 4494 bytes
 tests/acpi-test-data/q35/DSDT       | Bin 7666 -> 7578 bytes
 12 files changed, 137 insertions(+), 43 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH v4 1/4] i386/pc: expose identifying the floppy controller

[Roman Kagan]

Factor out and expose the function to locate the floppy controller in
the system.
It will be useful when dynamically populating the relevant objects in
the ACPI tables.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: qemu-block@nongnu.org
Cc: qemu-stable@nongnu.org
---
changes since v3:
 - split out into a separate patch to faciliate review

 hw/i386/pc.c         | 44 ++++++++++++++++++++++++++------------------
 include/hw/i386/pc.h |  2 ++
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 459260b..c36b8cf 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -360,6 +360,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;
@@ -368,8 +393,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,
@@ -399,22 +422,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/i386/pc.h b/include/hw/i386/pc.h
index b0d6283..8122229 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -267,6 +267,8 @@ 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);
+
 /* acpi_piix.c */
 
 I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
-- 
2.5.0

[Qemu-devel] [PATCH v4 2/4] i386/acpi: make floppy controller object dynamic

[Roman Kagan]

Instead of statically declaring the floppy controller in DSDT, with its
_STA method depending on some obscure bit in the parent ISA bridge, add
the object dynamically to SSDT via AML API only when the controller is
present.

The _STA method is no longer necessary and is therefore dropped.  So are
the declarations of the fields indicating whether the contoller is
enabled.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: qemu-block@nongnu.org
Cc: qemu-stable@nongnu.org
---
changes since v3:
 - new patch (note that it conflicts with "[PATCH 50/74] pc: acpi: move
   FDC0 device from DSDT to SSDT" from Igor's series)
 - include test data updates to maintain bisectability

 hw/i386/acpi-build.c                |  24 ++++++++++++++++++++++++
 hw/i386/acpi-dsdt-isa.dsl           |  18 ------------------
 hw/i386/acpi-dsdt.dsl               |   1 -
 hw/i386/q35-acpi-dsdt.dsl           |   7 ++-----
 tests/acpi-test-data/pc/DSDT        | Bin 3028 -> 2946 bytes
 tests/acpi-test-data/pc/SSDT        | Bin 2486 -> 2554 bytes
 tests/acpi-test-data/pc/SSDT.bridge | Bin 4345 -> 4413 bytes
 tests/acpi-test-data/q35/DSDT       | Bin 7666 -> 7578 bytes
 8 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4cc1440..a01e909 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -113,6 +113,7 @@ typedef struct AcpiMiscInfo {
     unsigned dsdt_size;
     uint16_t pvpanic_port;
     uint16_t applesmc_io_base;
+    bool has_fdc;
 } AcpiMiscInfo;
 
 typedef struct AcpiBuildPciBusHotplugState {
@@ -236,10 +237,15 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
 {
+    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();
+    info->has_fdc = !!fdc;
 }
 
 /*
@@ -1099,6 +1105,24 @@ build_ssdt(GArray *table_data, GArray *linker,
     aml_append(scope, aml_name_decl("_S5", pkg));
     aml_append(ssdt, scope);
 
+    if (misc->has_fdc) {
+        scope = aml_scope("\\_SB.PCI0.ISA");
+        dev = aml_device("FDC0");
+
+        aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));
+
+        crs = aml_resource_template();
+        aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
+        aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
+        aml_append(crs, aml_irq_no_flags(6));
+        aml_append(crs, aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER,
+                                AML_TRANSFER8, 2));
+        aml_append(dev, aml_name_decl("_CRS", crs));
+
+        aml_append(scope, dev);
+        aml_append(ssdt, scope);
+    }
+
     if (misc->applesmc_io_base) {
         scope = aml_scope("\\_SB.PCI0.ISA");
         dev = aml_device("SMC");
diff --git a/hw/i386/acpi-dsdt-isa.dsl b/hw/i386/acpi-dsdt-isa.dsl
index 89caa16..061507d 100644
--- a/hw/i386/acpi-dsdt-isa.dsl
+++ b/hw/i386/acpi-dsdt-isa.dsl
@@ -47,24 +47,6 @@ Scope(\_SB.PCI0.ISA) {
         })
     }
 
-    Device(FDC0) {
-        Name(_HID, EisaId("PNP0700"))
-        Method(_STA, 0, NotSerialized) {
-            Store(FDEN, Local0)
-            If (LEqual(Local0, 0)) {
-                Return (0x00)
-            } Else {
-                Return (0x0F)
-            }
-        }
-        Name(_CRS, ResourceTemplate() {
-            IO(Decode16, 0x03F2, 0x03F2, 0x00, 0x04)
-            IO(Decode16, 0x03F7, 0x03F7, 0x00, 0x01)
-            IRQNoFlags() { 6 }
-            DMA(Compatibility, NotBusMaster, Transfer8) { 2 }
-        })
-    }
-
     Device(LPT) {
         Name(_HID, EisaId("PNP0400"))
         Method(_STA, 0, NotSerialized) {
diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 8dba096..aa50990 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -80,7 +80,6 @@ DefinitionBlock (
                 , 3,
                 CBEN, 1,         // COM2
             }
-            Name(FDEN, 1)
         }
     }
 
diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
index 7be7b37..fcb9915 100644
--- a/hw/i386/q35-acpi-dsdt.dsl
+++ b/hw/i386/q35-acpi-dsdt.dsl
@@ -137,16 +137,13 @@ DefinitionBlock (
                 COMB,   3,
 
                 Offset(0x01),
-                LPTD,   2,
-                    ,   2,
-                FDCD,   2
+                LPTD,   2
             }
             OperationRegion(LPCE, PCI_Config, 0x82, 0x2)
             Field(LPCE, AnyAcc, NoLock, Preserve) {
                 CAEN,   1,
                 CBEN,   1,
-                LPEN,   1,
-                FDEN,   1
+                LPEN,   1
             }
         }
     }
diff --git a/tests/acpi-test-data/pc/DSDT b/tests/acpi-test-data/pc/DSDT
index c658203db94a7e7db7c36fde99a7075a8d75498d..d8ebf12cc0abbbbe9f19d131c7b1d7d1b3e681df 100644
GIT binary patch
delta 51
zcmca2-XzZD66_Mv#Ld9KxML%i4I`fet6qGtQ+$B4r$Ka+^W+dlCuRW$@yYWU7jEuh
H^56sjVdo9@

delta 130
zcmZn?zaq}%66_Lkg`0tav1KEd4I`f$t6qGtQ+$B4r$Ka+=j0GZCr%DG7gs+<0Uznf
zGZ`0pc(J&-I2&-pdw9C=I9_095Rr%v4sm2C04YjXz&1I7VF|-RmL**L9P!RU!Gh9U
h67Gzjm_IQyu(&gRXa3I2z^LTFpvA(l*^J4D69A%9AeI0C

diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT
index 210d6a71e58aa34ce8e94121d25bcf58c3bd503c..ffb3fe43970123d0e198328429ee04ebfd0564c9 100644
GIT binary patch
delta 91
zcmdlc{7aZCIM^lR7bgP)<AjY|(Tq;cEHV1b@xe~<0nVNVp23ds(M<+!F3tuV@gANo
vJdPLG893sdgMtO6xg^{fKQVt|W?*q={LcKHnSoKsfkBIfp>lIFV=xB*dq@{?

delta 24
gcmew*yiJ%ZIM^j*8z%z;<MoYP(Ttl{G6r%00Az3ng8%>k

diff --git a/tests/acpi-test-data/pc/SSDT.bridge b/tests/acpi-test-data/pc/SSDT.bridge
index 6e6660b1fbe0bdb7fe0b257cb53c31fa9bb21a14..f0f11bec8108eceb696fb854b51faeb97471146c 100644
GIT binary patch
delta 91
zcmeyVxL1iQIM^k`R*->#@y153XhtVzmKc5J_+Y2_0B27F&tS*+=q3X<7iR;Gcn?n(
v9>)vp3>@*!LBWF3ToUe#pO`-}GqAWberNv9%)qGRz@Wv#P`NpoaT-4WWGfeO

delta 24
gcmdn1^iz>5IM^lRrvL*3qryh6XvWPe8K>|A0A#NRg8%>k

diff --git a/tests/acpi-test-data/q35/DSDT b/tests/acpi-test-data/q35/DSDT
index 4723e5954dccb00995ccaf521b7daf6bf15cf1d4..5e03e26323d50dea63d57a36bd902e061e0442d8 100644
GIT binary patch
delta 91
zcmexlJ<FQQCD<iomMjAUqvA%cw~Txa?0WIRPVoWGo(9oP&XZZ0)EOlw>oQ5GMmP8b
qIJ+`&HE}UTH;RJT49<?OevHmeK*A>gNC-HHPcCCxxH*}*UkU(Bo)>-q

delta 176
zcmbPb{mGikCD<k8lPm)R<DrdQZyEV~*!ALro#F$WJq@Cp{3o+AsWU1})@70~WMFc0
zadu&fZtw|kc4gvf;$n(!lmf9CoE=^L7@eJfgiipFaB~3?0zT4{vzQioc(DL=8F0jV
zc)IX7USMYsk%$itabzd}DN0ztHaUS|3By8`C0zU*@y<cPg3??P?u?(9KQS|~xHEod
U{?5$6sN}$)#lo<eoq4hp0M<4v7ytkO

-- 
2.5.0

[Qemu-devel] [PATCH v4 3/4] expose floppy drive geometry and CMOS type

[Roman Kagan]

Make it possible to query the geometry and the CMOS type of a floppy
drive outside of the respective source files.

It will be useful, in particular, when dynamically building ACPI tables,
and will allow to properly populate the corresponding ACPI objects and
thus enable BIOS-less systems to access the floppy drives.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: qemu-block@nongnu.org
Cc: qemu-stable@nongnu.org
---
changes since v3:
 - split out into a separate patch to faciliate review

 hw/block/fdc.c         | 11 +++++++++++
 hw/i386/pc.c           |  2 +-
 include/hw/block/fdc.h |  2 ++
 include/hw/i386/pc.h   |  1 +
 4 files changed, 15 insertions(+), 1 deletion(-)

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/pc.c b/hw/i386/pc.c
index c36b8cf..99fab83 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -199,7 +199,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;
 
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 8122229..d044a9a 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -268,6 +268,7 @@ 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 */
 
-- 
2.5.0

[Qemu-devel] [PATCH v4 4/4] i386: populate floppy drive information in SSDT

[Roman Kagan]

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) of the floppy controller object.  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 adds those objects to the floppy controller in SSDT,
populating them with the information from the respective QEMU objects.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: qemu-block@nongnu.org
Cc: qemu-stable@nongnu.org
---
changes since v3:
 - build on top of dynamic FDC0 patch
 - include test data updates to maintain bisectability

changes since v2:
 - explicit endianness for buffer data
 - reorder code to reduce conflicts with dynamic DSDT patchset

 hw/i386/acpi-build.c                |  68 ++++++++++++++++++++++++++++++++++++
 tests/acpi-test-data/pc/SSDT        | Bin 2554 -> 2635 bytes
 tests/acpi-test-data/pc/SSDT.bridge | Bin 4413 -> 4494 bytes
 3 files changed, 68 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a01e909..7b8de59 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 "hw/mem/nvdimm.h"
 #include "sysemu/tpm.h"
@@ -106,6 +107,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;
@@ -114,6 +122,7 @@ typedef struct AcpiMiscInfo {
     uint16_t pvpanic_port;
     uint16_t applesmc_io_base;
     bool has_fdc;
+    AcpiFDInfo fdinfo[2];
 } AcpiMiscInfo;
 
 typedef struct AcpiBuildPciBusHotplugState {
@@ -237,6 +246,7 @@ 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();
@@ -246,6 +256,16 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
 
     fdc = pc_find_fdc0();
     info->has_fdc = !!fdc;
+    if (fdc) {
+        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);
+            }
+        }
+    }
 }
 
 /*
@@ -935,6 +955,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,
@@ -1106,6 +1160,8 @@ build_ssdt(GArray *table_data, GArray *linker,
     aml_append(ssdt, scope);
 
     if (misc->has_fdc) {
+        uint32_t fde_buf[5] = {0, 0, 0, 0, cpu_to_le32(0x2)};
+
         scope = aml_scope("\\_SB.PCI0.ISA");
         dev = aml_device("FDC0");
 
@@ -1119,6 +1175,18 @@ build_ssdt(GArray *table_data, GArray *linker,
                                 AML_TRANSFER8, 2));
         aml_append(dev, aml_name_decl("_CRS", crs));
 
+        for (i = 0; i < ARRAY_SIZE(misc->fdinfo); i++) {
+            AcpiFDInfo *fdinfo = &misc->fdinfo[i];
+            if (fdinfo->type != FDRIVE_DRV_NONE) {
+                fde_buf[i] = cpu_to_le32(0x1);
+                aml_append(dev, build_fdinfo_aml(i, fdinfo));
+            }
+        }
+
+        aml_append(dev,
+            aml_name_decl("_FDE",
+                aml_buffer(sizeof(fde_buf), (uint8_t *) fde_buf)));
+
         aml_append(scope, dev);
         aml_append(ssdt, scope);
     }
diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT
index ffb3fe43970123d0e198328429ee04ebfd0564c9..5e70e0b5989bb8c85a14bd0902f32dd48eb17602 100644
GIT binary patch
delta 130
zcmew*d|HGnIM^k`n~Q;g(QPAFG^3~sXN*2`e6Uk|fU~E8XRu>@bdw{;<Vr?;|L7(|
zH=h7Uj(A6xAO?<jHy2MK1px*w7A}7-Ax5tCTufZ5KrF;1&GnxvhwB>`KNkl`m8+mQ
Smk1*RDqvz@*u0!Em;(Tc3K+8h

delta 49
zcmX>t@=KU2IM^lR7bgP)<AjY|(TpO_EHV1b@xe~<0nVNVp23ds(M<-Es~Gh+|6~l}
F003=;4z&OP

diff --git a/tests/acpi-test-data/pc/SSDT.bridge b/tests/acpi-test-data/pc/SSDT.bridge
index f0f11bec8108eceb696fb854b51faeb97471146c..642d9a51203cc591a61b2169b2feb5731ad67a96 100644
GIT binary patch
delta 130
zcmdn1)Thi99PAR(C&<9S*uRl0no-n+Ge(~|KG-Qfz}eHlGuSacy2+7aawVg_e{_?f
zn@@lvN4%p;5CccNn~SHAf&c>-3zt8a5F^)mE+#HjAQs}1=K9Z-!}X1epNj*e%2iOD
SON5aD6)-U{Y+lYdjUND@BpEaS

delta 49
zcmeBE-mAnF9PAQeE6BjWcw-}1G^2<!ON>5qe6Uk|fU~E8XRu>@bd$m4Dn|XyKN+X;
F0{}`q4mJP)

-- 
2.5.0

Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes

[Roman Kagan]

On Thu, Dec 24, 2015 at 08:17:45AM +0200, Michael S. Tsirkin wrote:
> On Wed, Dec 23, 2015 at 08:51:45PM +0300, Roman Kagan wrote:
> > On Wed, Dec 23, 2015 at 06:47:16PM +0100, Igor Mammedov wrote:
> > > On Wed, 23 Dec 2015 20:20:54 +0300
> > > Roman Kagan <rkagan@virtuozzo.com> wrote:
> > > > > ... two 1.44M drives with bogus geometry for q35.
> > > > 
> > > > This one is a bug in my patch, indeed: I was tricked by FDRIVE_DRV_NONE
> > > > being non-zero, and forgot to initialize the respective fields in
> > > > acpi_get_misc_info() in case there is no floppy controller at all.
> > > so instead of fake initialization, it's worth to make your patch
> > > conditional on presence of controller after all.
> > > i.e. add AML only if controller was present.
> > 
> > Indeed :)
> > 
> > Roman.
> 
> Or rather, start series with a patch making FDC conditional,
> then update expected ssdt, then tweak methods within -
> should not change ssdt since we don't create a floppy in
> the test.

Actually we do.  "pc" machine type has it by default regardless of
whether anything is attached to it.

So I ended up with the patch series v4 I just posted but I'm not sure it
addresses all the concerns people have had about v3.

Thanks,
Roman.

Re: [Qemu-devel] [PATCH v4 0/4] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

On Fri, 25 Dec 2015 18:04:08 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> Windows on UEFI systems is only capable of detecting the presence and
> the type of floppy drives via corresponding ACPI objects.
> 
> Those objects are added in the last patch of the series; the three
> preceding ones pave the way to it, by making the necessary data
> public and by moving the whole floppy drive controller description into
> runtime-generated SSDT.
> 
> Note that the series conflicts with Igor's patchset for dynamic DSDT, in
> particular, with "[PATCH 50/74] pc: acpi: move FDC0 device from DSDT
> to SSDT"; I haven't managed to avoid that while trying to meet
> maintainer's comments.
To remove conflicts and to make it more suitable for stable, I'd drop
 "2/4 i386/acpi: make floppy controller object dynamic"
and split test blob out of
 "4/4 i386: populate floppy drive information in SSDT"
into a separate patch so one can see effects of applying 4/4
and then update blobs if resulting ASL diff is as expected.


> 
> Roman Kagan (4):
>   i386/pc: expose identifying the floppy controller
>   i386/acpi: make floppy controller object dynamic
>   expose floppy drive geometry and CMOS type
>   i386: populate floppy drive information in SSDT
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: John Snow <jsnow@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: qemu-block@nongnu.org
> Cc: qemu-stable@nongnu.org
> ---
> changes since v3:
>  - make FDC object fully dynamic in a separate patch
>  - split out support patches
>  - include test data updates with the respective patches to maintain
>    bisectability
> 
> 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                |  92 ++++++++++++++++++++++++++++++++++++
>  hw/i386/acpi-dsdt-isa.dsl           |  18 -------
>  hw/i386/acpi-dsdt.dsl               |   1 -
>  hw/i386/pc.c                        |  46 ++++++++++--------
>  hw/i386/q35-acpi-dsdt.dsl           |   7 +--
>  include/hw/block/fdc.h              |   2 +
>  include/hw/i386/pc.h                |   3 ++
>  tests/acpi-test-data/pc/DSDT        | Bin 3028 -> 2946 bytes
>  tests/acpi-test-data/pc/SSDT        | Bin 2486 -> 2635 bytes
>  tests/acpi-test-data/pc/SSDT.bridge | Bin 4345 -> 4494 bytes
>  tests/acpi-test-data/q35/DSDT       | Bin 7666 -> 7578 bytes
>  12 files changed, 137 insertions(+), 43 deletions(-)
> 

Re: [Qemu-devel] [PATCH v4 0/4] i386: expose floppy-related objects in SSDT

[Roman Kagan]

On Tue, Dec 29, 2015 at 03:09:36PM +0100, Igor Mammedov wrote:
> On Fri, 25 Dec 2015 18:04:08 +0300
> Roman Kagan <rkagan@virtuozzo.com> wrote:
> 
> > Windows on UEFI systems is only capable of detecting the presence and
> > the type of floppy drives via corresponding ACPI objects.
> > 
> > Those objects are added in the last patch of the series; the three
> > preceding ones pave the way to it, by making the necessary data
> > public and by moving the whole floppy drive controller description into
> > runtime-generated SSDT.
> > 
> > Note that the series conflicts with Igor's patchset for dynamic DSDT, in
> > particular, with "[PATCH 50/74] pc: acpi: move FDC0 device from DSDT
> > to SSDT"; I haven't managed to avoid that while trying to meet
> > maintainer's comments.
> To remove conflicts and to make it more suitable for stable, I'd drop
>  "2/4 i386/acpi: make floppy controller object dynamic"

Erm...  I thought I did what Michael requested:

On Thu, Dec 24, 2015 at 08:17:45AM +0200, Michael S. Tsirkin wrote:
> Or rather, start series with a patch making FDC conditional,

So what do I need to do to get this thing merged?


> and split test blob out of
>  "4/4 i386: populate floppy drive information in SSDT"
> into a separate patch so one can see effects of applying 4/4
> and then update blobs if resulting ASL diff is as expected.

This will break bisectability, won't it?

Roman.

Re: [Qemu-devel] [PATCH v4 0/4] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

On Tue, 29 Dec 2015 19:17:46 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> On Tue, Dec 29, 2015 at 03:09:36PM +0100, Igor Mammedov wrote:
> > On Fri, 25 Dec 2015 18:04:08 +0300
> > Roman Kagan <rkagan@virtuozzo.com> wrote:
> > 
> > > Windows on UEFI systems is only capable of detecting the presence and
> > > the type of floppy drives via corresponding ACPI objects.
> > > 
> > > Those objects are added in the last patch of the series; the three
> > > preceding ones pave the way to it, by making the necessary data
> > > public and by moving the whole floppy drive controller description into
> > > runtime-generated SSDT.
> > > 
> > > Note that the series conflicts with Igor's patchset for dynamic DSDT, in
> > > particular, with "[PATCH 50/74] pc: acpi: move FDC0 device from DSDT
> > > to SSDT"; I haven't managed to avoid that while trying to meet
> > > maintainer's comments.
> > To remove conflicts and to make it more suitable for stable, I'd drop
> >  "2/4 i386/acpi: make floppy controller object dynamic"
> 
> Erm...  I thought I did what Michael requested:
> 
> On Thu, Dec 24, 2015 at 08:17:45AM +0200, Michael S. Tsirkin wrote:
> > Or rather, start series with a patch making FDC conditional,
> 
> So what do I need to do to get this thing merged?
> 
> 
> > and split test blob out of
> >  "4/4 i386: populate floppy drive information in SSDT"
> > into a separate patch so one can see effects of applying 4/4
> > and then update blobs if resulting ASL diff is as expected.
> 
> This will break bisectability, won't it?
not really, it's only a warning when AML differs from the expected one.

> 
> Roman.
> 

Re: [Qemu-devel] [PATCH v4 0/4] i386: expose floppy-related objects in SSDT

[Michael S. Tsirkin]

On Tue, Dec 29, 2015 at 07:17:46PM +0300, Roman Kagan wrote:
> On Tue, Dec 29, 2015 at 03:09:36PM +0100, Igor Mammedov wrote:
> > On Fri, 25 Dec 2015 18:04:08 +0300
> > Roman Kagan <rkagan@virtuozzo.com> wrote:
> > 
> > > Windows on UEFI systems is only capable of detecting the presence and
> > > the type of floppy drives via corresponding ACPI objects.
> > > 
> > > Those objects are added in the last patch of the series; the three
> > > preceding ones pave the way to it, by making the necessary data
> > > public and by moving the whole floppy drive controller description into
> > > runtime-generated SSDT.
> > > 
> > > Note that the series conflicts with Igor's patchset for dynamic DSDT, in
> > > particular, with "[PATCH 50/74] pc: acpi: move FDC0 device from DSDT
> > > to SSDT"; I haven't managed to avoid that while trying to meet
> > > maintainer's comments.
> > To remove conflicts and to make it more suitable for stable, I'd drop
> >  "2/4 i386/acpi: make floppy controller object dynamic"
> 
> Erm...  I thought I did what Michael requested:
> 
> On Thu, Dec 24, 2015 at 08:17:45AM +0200, Michael S. Tsirkin wrote:
> > Or rather, start series with a patch making FDC conditional,
> 
> So what do I need to do to get this thing merged?


Just split the test files away from patch itself.
Thanks!

> 
> > and split test blob out of
> >  "4/4 i386: populate floppy drive information in SSDT"
> > into a separate patch so one can see effects of applying 4/4
> > and then update blobs if resulting ASL diff is as expected.
> 
> This will break bisectability, won't it?
> 
> Roman.

No because diff in this test is just a warning.

-- 
MST