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

[Qemu-devel] [PATCH v5 0/6] 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 patch 5; the 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 v2 27/51] 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 (6):
  i386/pc: expose identifying the floppy controller
  i386/acpi: make floppy controller object dynamic
  tests/acpi: update test data
  expose floppy drive geometry and CMOS type
  i386: populate floppy drive information in SSDT
  tests/acpi: update test data

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 v4:
 - re-split out code changes from test data updates

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 v5 1/6] 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 allow to dynamically populate 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
---
no changes since v4

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 v5 2/6] 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 v4:
 - re-split from test data update to facilitate review (bisectability not
   affected)

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 ++-----
 4 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
             }
         }
     }
-- 
2.5.0

[Qemu-devel] [PATCH v5 3/6] tests/acpi: update test data

[Roman Kagan]

to match the preceding commit "i386/acpi: make floppy controller object
dynamic".

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 v4:
 - new, split out from code changes

 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
 4 files changed, 0 insertions(+), 0 deletions(-)

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 v5 4/6] 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
---
no changes since v4

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

Re: [Qemu-devel] [PATCH v5 4/6] expose floppy drive geometry and CMOS type

[John Snow]

On 12/30/2015 03:11 PM, Roman Kagan wrote:
> 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
> ---
> no changes since v4
> 
> 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 */
>  
> 

Patches 1,4:

Reviewed-by: John Snow <jsnow@redhat.com>

Aside: Why did they have you split out the test changes to be separate
from the code? Doesn't that introduce commits where the tests now fail?

--js

Re: [Qemu-devel] [PATCH v5 4/6] expose floppy drive geometry and CMOS type

[Michael S. Tsirkin]

On Mon, Jan 04, 2016 at 03:44:42PM -0500, John Snow wrote:
> 
> 
> On 12/30/2015 03:11 PM, Roman Kagan wrote:
> > 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
> > ---
> > no changes since v4
> > 
> > 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 */
> >  
> > 
> 
> Patches 1,4:
> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> Aside: Why did they have you split out the test changes to be separate
> from the code? Doesn't that introduce commits where the tests now fail?
> 
> --js

It's only a warning not a failure.

Re: [Qemu-devel] [Qemu-block] [PATCH v5 4/6] expose floppy drive geometry and CMOS type

[Denis V. Lunev]

On 01/04/2016 11:44 PM, John Snow wrote:
>
> On 12/30/2015 03:11 PM, Roman Kagan wrote:
>> 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
>> ---
>> no changes since v4
>>
>> 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 */
>>   
>>
> Patches 1,4:
>
> Reviewed-by: John Snow <jsnow@redhat.com>
>
> Aside: Why did they have you split out the test changes to be separate
> from the code? Doesn't that introduce commits where the tests now fail?
>
> --js
>
this would make merge process to downstream simplear.

Den

[Qemu-devel] [PATCH v5 5/6] 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 v4:
 - re-split out code changes from test data updates

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 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file 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);
     }
-- 
2.5.0

[Qemu-devel] [PATCH v5 6/6] tests/acpi: update test data

[Roman Kagan]

to match the preceding commit "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 v4:
 - new, split out from code changes

 tests/acpi-test-data/pc/SSDT        | Bin 2554 -> 2635 bytes
 tests/acpi-test-data/pc/SSDT.bridge | Bin 4413 -> 4494 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)

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 v5 0/6] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

On Wed, 30 Dec 2015 23:11:50 +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 patch 5; the 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 v2 27/51] pc: acpi: move FDC0 device from DSDT
> to SSDT"; I haven't managed to avoid that while trying to meet
> maintainer's comments.

Tested with XPsp3 WS2008R2 WS2012R2, no regressions so far it boots fine and can read floppy.

So for whole series:
Reviewed-by: Igor Mammedov <imammedo@redhat.com>


> Roman Kagan (6):
>   i386/pc: expose identifying the floppy controller
>   i386/acpi: make floppy controller object dynamic
>   tests/acpi: update test data
>   expose floppy drive geometry and CMOS type
>   i386: populate floppy drive information in SSDT
>   tests/acpi: update test data
> 
> 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 v4:
>  - re-split out code changes from test data updates
> 
> 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 v5 0/6] i386: expose floppy-related objects in SSDT

[Michael S. Tsirkin]

On Wed, Jan 06, 2016 at 03:04:40PM +0100, Igor Mammedov wrote:
> On Wed, 30 Dec 2015 23:11:50 +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 patch 5; the 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 v2 27/51] pc: acpi: move FDC0 device from DSDT
> > to SSDT"; I haven't managed to avoid that while trying to meet
> > maintainer's comments.
> 
> Tested with XPsp3 WS2008R2 WS2012R2, no regressions so far it boots fine and can read floppy.
> 
> So for whole series:
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> 

Igor, could you pls rebase this on top of your patches?
I've merged them in my tree.

> > Roman Kagan (6):
> >   i386/pc: expose identifying the floppy controller
> >   i386/acpi: make floppy controller object dynamic
> >   tests/acpi: update test data
> >   expose floppy drive geometry and CMOS type
> >   i386: populate floppy drive information in SSDT
> >   tests/acpi: update test data
> > 
> > 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 v4:
> >  - re-split out code changes from test data updates
> > 
> > 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 v5 0/6] i386: expose floppy-related objects in SSDT

[Michael S. Tsirkin]

On Thu, Jan 07, 2016 at 12:56:09PM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 06, 2016 at 03:04:40PM +0100, Igor Mammedov wrote:
> > On Wed, 30 Dec 2015 23:11:50 +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 patch 5; the 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 v2 27/51] pc: acpi: move FDC0 device from DSDT
> > > to SSDT"; I haven't managed to avoid that while trying to meet
> > > maintainer's comments.
> > 
> > Tested with XPsp3 WS2008R2 WS2012R2, no regressions so far it boots fine and can read floppy.
> > 
> > So for whole series:
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > 
> 
> Igor, could you pls rebase this on top of your patches?
> I've merged them in my tree.

Pls remember to keep the author information intact though.

> > > Roman Kagan (6):
> > >   i386/pc: expose identifying the floppy controller
> > >   i386/acpi: make floppy controller object dynamic
> > >   tests/acpi: update test data
> > >   expose floppy drive geometry and CMOS type
> > >   i386: populate floppy drive information in SSDT
> > >   tests/acpi: update test data
> > > 
> > > 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 v4:
> > >  - re-split out code changes from test data updates
> > > 
> > > 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 v5 0/6] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

On Thu, 7 Jan 2016 12:56:58 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Jan 07, 2016 at 12:56:09PM +0200, Michael S. Tsirkin wrote:
> > On Wed, Jan 06, 2016 at 03:04:40PM +0100, Igor Mammedov wrote:  
> > > On Wed, 30 Dec 2015 23:11:50 +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 patch 5; the 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 v2 27/51] pc: acpi: move FDC0 device from DSDT
> > > > to SSDT"; I haven't managed to avoid that while trying to meet
> > > > maintainer's comments.  
> > > 
> > > Tested with XPsp3 WS2008R2 WS2012R2, no regressions so far it boots fine and can read floppy.
> > > 
> > > So for whole series:
> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > >   
> > 
> > Igor, could you pls rebase this on top of your patches?
> > I've merged them in my tree.  
> 
> Pls remember to keep the author information intact though.
Sure,
I'll usually do it or when in doubt I just ask original
authors before changing it.

> 
> > > > Roman Kagan (6):
> > > >   i386/pc: expose identifying the floppy controller
> > > >   i386/acpi: make floppy controller object dynamic
> > > >   tests/acpi: update test data
> > > >   expose floppy drive geometry and CMOS type
> > > >   i386: populate floppy drive information in SSDT
> > > >   tests/acpi: update test data
> > > > 
> > > > 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 v4:
> > > >  - re-split out code changes from test data updates
> > > > 
> > > > 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 v5 0/6] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

On Thu, 7 Jan 2016 12:56:09 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Jan 06, 2016 at 03:04:40PM +0100, Igor Mammedov wrote:
> > On Wed, 30 Dec 2015 23:11:50 +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 patch 5; the 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 v2 27/51] pc: acpi: move FDC0 device from DSDT
> > > to SSDT"; I haven't managed to avoid that while trying to meet
> > > maintainer's comments.  
> > 
> > Tested with XPsp3 WS2008R2 WS2012R2, no regressions so far it boots fine and can read floppy.
> > 
> > So for whole series:
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> >   
> 
> Igor, could you pls rebase this on top of your patches?
> I've merged them in my tree.
I don't see updates to pci tree,
Have you pushed it from local host?

> 
> > > Roman Kagan (6):
> > >   i386/pc: expose identifying the floppy controller
> > >   i386/acpi: make floppy controller object dynamic
> > >   tests/acpi: update test data
> > >   expose floppy drive geometry and CMOS type
> > >   i386: populate floppy drive information in SSDT
> > >   tests/acpi: update test data
> > > 
> > > 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 v4:
> > >  - re-split out code changes from test data updates
> > > 
> > > 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 v5 0/6] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

On Wed, 30 Dec 2015 23:11:50 +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 patch 5; the 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 v2 27/51] pc: acpi: move FDC0 device from DSDT
> to SSDT"; I haven't managed to avoid that while trying to meet
> maintainer's comments.


Hello Roman,

I've rebased/rewrote this series on top of current PCI tree.
Could you tell me if I should keep your Author/SoB on following
patches or change/drop it and if it's the case please specify what
should be changed:

  i386/acpi: make floppy controller object dynamic
      https://github.com/imammedo/qemu/commit/f0a3a4761f8f9698d0f0117d47e2353505de37bf
  i386: populate floppy drive information in DSDT
      https://github.com/imammedo/qemu/commit/97578d32e0a0b1cea0b6229f5ef51f8e104b7fdb

> 
> Roman Kagan (6):
>   i386/pc: expose identifying the floppy controller
>   i386/acpi: make floppy controller object dynamic
>   tests/acpi: update test data
>   expose floppy drive geometry and CMOS type
>   i386: populate floppy drive information in SSDT
>   tests/acpi: update test data
> 
> 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 v4:
>  - re-split out code changes from test data updates
> 
> 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 v5 0/6] i386: expose floppy-related objects in SSDT

[Roman Kagan]

On Mon, Jan 11, 2016 at 02:51:15PM +0100, Igor Mammedov wrote:
> On Wed, 30 Dec 2015 23:11:50 +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 patch 5; the 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 v2 27/51] pc: acpi: move FDC0 device from DSDT
> > to SSDT"; I haven't managed to avoid that while trying to meet
> > maintainer's comments.
> 
> 
> Hello Roman,
> 
> I've rebased/rewrote this series on top of current PCI tree.
> Could you tell me if I should keep your Author/SoB on following
> patches or change/drop it and if it's the case please specify what
> should be changed:
> 
>   i386/acpi: make floppy controller object dynamic
>       https://github.com/imammedo/qemu/commit/f0a3a4761f8f9698d0f0117d47e2353505de37bf
>   i386: populate floppy drive information in DSDT
>       https://github.com/imammedo/qemu/commit/97578d32e0a0b1cea0b6229f5ef51f8e104b7fdb

Both patches look good to me (I just noticed an excessive "to" in the
log message of the second one, in "QEMU doesn't _to_ provide those
objects in its ACPI tables", you may want to delete it before
propagating the patch upstream).

Now what are the plans re. stable branches?  I think the problem of the
floppy being unavaliable in Windows on UEFI/OVMF justifies porting it
there (we are interested, in particular, in stable-2.3), but I'm now
confused as to what state to use as the base.

(As a matter of fact I'd been hoping that my patches made it in before
your dynamic DSDT rework so the backport would be trivial cherry-pick;
as this is no longer the case I'd appreciate your (or anybody else's)
advice on how to move on with stable.)

Thanks,
Roman.

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

[Igor Mammedov]

On Mon, 11 Jan 2016 17:26:26 +0300
Roman Kagan <rkagan@virtuozzo.com> wrote:

> On Mon, Jan 11, 2016 at 02:51:15PM +0100, Igor Mammedov wrote:
> > On Wed, 30 Dec 2015 23:11:50 +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 patch 5; the 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 v2 27/51] pc: acpi: move FDC0 device from DSDT
> > > to SSDT"; I haven't managed to avoid that while trying to meet
> > > maintainer's comments.  
> > 
> > 
> > Hello Roman,
> > 
> > I've rebased/rewrote this series on top of current PCI tree.
> > Could you tell me if I should keep your Author/SoB on following
> > patches or change/drop it and if it's the case please specify what
> > should be changed:
> > 
> >   i386/acpi: make floppy controller object dynamic
> >       https://github.com/imammedo/qemu/commit/f0a3a4761f8f9698d0f0117d47e2353505de37bf
> >   i386: populate floppy drive information in DSDT
> >       https://github.com/imammedo/qemu/commit/97578d32e0a0b1cea0b6229f5ef51f8e104b7fdb  
> 
> Both patches look good to me (I just noticed an excessive "to" in the
> log message of the second one, in "QEMU doesn't _to_ provide those
> objects in its ACPI tables", you may want to delete it before
> propagating the patch upstream).
Just to confirm, so you are agree with me keeping you as Author on
above patches and your SoB on them as well?
 
> Now what are the plans re. stable branches?  I think the problem of the
> floppy being unavaliable in Windows on UEFI/OVMF justifies porting it
> there (we are interested, in particular, in stable-2.3), but I'm now
> confused as to what state to use as the base.
> 
> (As a matter of fact I'd been hoping that my patches made it in before
> your dynamic DSDT rework so the backport would be trivial cherry-pick;
> as this is no longer the case I'd appreciate your (or anybody else's)
> advice on how to move on with stable.)
Stable could use reviewed v5 if Michael agrees to take fix.

> 
> Thanks,
> Roman.

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

[Roman Kagan]

On Mon, Jan 11, 2016 at 03:47:24PM +0100, Igor Mammedov wrote:
> On Mon, 11 Jan 2016 17:26:26 +0300
> Roman Kagan <rkagan@virtuozzo.com> wrote:
> 
> > On Mon, Jan 11, 2016 at 02:51:15PM +0100, Igor Mammedov wrote:
> > > I've rebased/rewrote this series on top of current PCI tree.
> > > Could you tell me if I should keep your Author/SoB on following
> > > patches or change/drop it and if it's the case please specify what
> > > should be changed:
> > > 
> > >   i386/acpi: make floppy controller object dynamic
> > >       https://github.com/imammedo/qemu/commit/f0a3a4761f8f9698d0f0117d47e2353505de37bf
> > >   i386: populate floppy drive information in DSDT
> > >       https://github.com/imammedo/qemu/commit/97578d32e0a0b1cea0b6229f5ef51f8e104b7fdb  
> > 
> > Both patches look good to me (I just noticed an excessive "to" in the
> > log message of the second one, in "QEMU doesn't _to_ provide those
> > objects in its ACPI tables", you may want to delete it before
> > propagating the patch upstream).
> Just to confirm, so you are agree with me keeping you as Author on
> above patches and your SoB on them as well?

Yes, sure.  Sorry I haven't made it clear.

> > Now what are the plans re. stable branches?  I think the problem of the
> > floppy being unavaliable in Windows on UEFI/OVMF justifies porting it
> > there (we are interested, in particular, in stable-2.3), but I'm now
> > confused as to what state to use as the base.
> > 
> > (As a matter of fact I'd been hoping that my patches made it in before
> > your dynamic DSDT rework so the backport would be trivial cherry-pick;
> > as this is no longer the case I'd appreciate your (or anybody else's)
> > advice on how to move on with stable.)
> Stable could use reviewed v5 if Michael agrees to take fix.

OK thanks!

Roman.

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

[Laszlo Ersek]

Hi Roman,

On 12/30/15 21:11, Roman Kagan wrote:
> Windows on UEFI systems is only capable of detecting the presence and
> the type of floppy drives via corresponding ACPI objects.

I'm late to the party, but please allow me a question:

how did you figure out that UEFI Windows requires this?

In general, what the ACPI specification says is at best a "guideline"
for Windows. So how did you prove this was a requirement for Windows?

Thanks
Laszlo

> 
> Those objects are added in patch 5; the 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 v2 27/51] 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 (6):
>   i386/pc: expose identifying the floppy controller
>   i386/acpi: make floppy controller object dynamic
>   tests/acpi: update test data
>   expose floppy drive geometry and CMOS type
>   i386: populate floppy drive information in SSDT
>   tests/acpi: update test data
> 
> 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 v4:
>  - re-split out code changes from test data updates
> 
> 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 v5 0/6] i386: expose floppy-related objects in SSDT

[Roman Kagan]

On Wed, Jan 13, 2016 at 03:36:18PM +0100, Laszlo Ersek wrote:
> On 12/30/15 21:11, Roman Kagan wrote:
> > Windows on UEFI systems is only capable of detecting the presence and
> > the type of floppy drives via corresponding ACPI objects.
> 
> I'm late to the party, but please allow me a question:
> 
> how did you figure out that UEFI Windows requires this?
> 
> In general, what the ACPI specification says is at best a "guideline"
> for Windows. So how did you prove this was a requirement for Windows?

Well, my statement above that Windows on UEFI can detect floppies *only*
via ACPI is probably a bit stronger than I can actually prove but

- Windows on OVMF didn't see floppies before the patch, while Linux did
  (by querying CMOS)

- a number of sources on the internet hinted that Windows needed ACPI
  assistance for that, e.g.:

  https://www.reactos.org/wiki/UEFI#Floppy
  https://social.technet.microsoft.com/Forums/windows/en-US/f17db175-d146-4518-b2e9-c12a15031222/legacy-floppy-compatibility-with-uefi-boot?forum=w7itprohardware
  https://social.technet.microsoft.com/Forums/windows/en-US/e91ec27b-0c2d-44a3-b949-e77fa810a4c0/windows-7-uefi-fdd-how-to?forum=w7itprohardware

- the links mentioned the need in _FDE object but indicated it only
  allowed for successful enumeration of floppies, not the actual access;
  I proved that experimentally

- the ACPI spec stated that _FDE went in concert with _FDI so I tried it
  and it worked out

Voila.  Besides, I later discovered that a similar research had been
carried out for Parallels proprietary hypervisor, with a similar
outcome.

Roman.

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

[Roman Kagan]

On Wed, Jan 13, 2016 at 06:49:44PM +0300, Roman Kagan wrote:
> On Wed, Jan 13, 2016 at 03:36:18PM +0100, Laszlo Ersek wrote:
> > On 12/30/15 21:11, Roman Kagan wrote:
> > > Windows on UEFI systems is only capable of detecting the presence and
> > > the type of floppy drives via corresponding ACPI objects.
> > 
> > I'm late to the party, but please allow me a question:
> > 
> > how did you figure out that UEFI Windows requires this?
> > 
> > In general, what the ACPI specification says is at best a "guideline"
> > for Windows. So how did you prove this was a requirement for Windows?
> 
> Well, my statement above that Windows on UEFI can detect floppies *only*
> via ACPI is probably a bit stronger than I can actually prove but
> 
> - Windows on OVMF didn't see floppies before the patch, while Linux did
>   (by querying CMOS)
> 
> - a number of sources on the internet hinted that Windows needed ACPI
>   assistance for that, e.g.:
> 
>   https://www.reactos.org/wiki/UEFI#Floppy
>   https://social.technet.microsoft.com/Forums/windows/en-US/f17db175-d146-4518-b2e9-c12a15031222/legacy-floppy-compatibility-with-uefi-boot?forum=w7itprohardware
>   https://social.technet.microsoft.com/Forums/windows/en-US/e91ec27b-0c2d-44a3-b949-e77fa810a4c0/windows-7-uefi-fdd-how-to?forum=w7itprohardware
> 
> - the links mentioned the need in _FDE object but indicated it only
>   allowed for successful enumeration of floppies, not the actual access;
>   I proved that experimentally
> 
> - the ACPI spec stated that _FDE went in concert with _FDI so I tried it
>   and it worked out
> 
> Voila.  Besides, I later discovered that a similar research had been
> carried out for Parallels proprietary hypervisor, with a similar
> outcome.

Ah, I wish I saw your comment
https://bugzilla.redhat.com/show_bug.cgi?id=1212317#c5
I'd probably proceed directly to trying _FDI :)

Roman.

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

[Laszlo Ersek]

On 01/13/16 16:49, Roman Kagan wrote:
> On Wed, Jan 13, 2016 at 03:36:18PM +0100, Laszlo Ersek wrote:
>> On 12/30/15 21:11, Roman Kagan wrote:
>>> Windows on UEFI systems is only capable of detecting the presence and
>>> the type of floppy drives via corresponding ACPI objects.
>>
>> I'm late to the party, but please allow me a question:
>>
>> how did you figure out that UEFI Windows requires this?
>>
>> In general, what the ACPI specification says is at best a "guideline"
>> for Windows. So how did you prove this was a requirement for Windows?
> 
> Well, my statement above that Windows on UEFI can detect floppies *only*
> via ACPI is probably a bit stronger than I can actually prove but
> 
> - Windows on OVMF didn't see floppies before the patch, while Linux did
>   (by querying CMOS)

Indeed.

https://bugzilla.redhat.com/show_bug.cgi?id=1212317

> 
> - a number of sources on the internet hinted that Windows needed ACPI
>   assistance for that, e.g.:
> 
>   https://www.reactos.org/wiki/UEFI#Floppy
>   https://social.technet.microsoft.com/Forums/windows/en-US/f17db175-d146-4518-b2e9-c12a15031222/legacy-floppy-compatibility-with-uefi-boot?forum=w7itprohardware
>   https://social.technet.microsoft.com/Forums/windows/en-US/e91ec27b-0c2d-44a3-b949-e77fa810a4c0/windows-7-uefi-fdd-how-to?forum=w7itprohardware

Right, I found these. (The last two anyway.)

I also found technet comments that plainly stated "it would never work".

(Under your first link, I can read as well: "There have been reports
that Windows does not properly support motherboard floppy controllers
when booting from UEFI. The cause is not definitively known though a
couple of pieces of data have emerged, one pointing to an issue with ACPI".)

> - the links mentioned the need in _FDE object but indicated it only
>   allowed for successful enumeration of floppies, not the actual access;
>   I proved that experimentally
> 
> - the ACPI spec stated that _FDE went in concert with _FDI so I tried it
>   and it worked out

Thank you for confirming.

So, improving Windows compat in QEMU remains trial-and-error-based, and
occasionally reverse-engineering-based. Deplorable.

> Voila.  Besides, I later discovered that a similar research had been
> carried out for Parallels proprietary hypervisor, with a similar
> outcome.

That is, large amounts of work are being duplicated between participants
in this industry segment, because Microsoft doesn't give a flying fsck
about documenting their exact platform requirements. (The fact that _FDE
and _FDI are described in the ACPI spec means exactly zilch, because
Microsoft have ignored e.g. DataTableRegion from the same spec, since
ACPI 2.0 -- the year 2000.) I'm quite sure this obscurity is
intentional, and meant to spread uncertainty and waste competitors'
resources.

Whereas they are having a field day whenever they look at open source
components in a hybrid virt stack.

Nothing to see here, move along.

Thanks again.
Laszlo