Re: [Qemu-devel] [PULL 48/49] i386: populate floppy drive information in DSDT

[Laszlo Ersek]

On 02/09/16 17:22, John Snow wrote:
> 
> 
> On 02/09/2016 10:52 AM, Roman Kagan wrote:
>> On Mon, Feb 08, 2016 at 03:20:47PM -0500, John Snow wrote:
>>> On 02/08/2016 08:14 AM, Roman Kagan wrote:
>>>> On Fri, Feb 05, 2016 at 07:25:07PM +0100, Igor Mammedov wrote:
>>>>>> +    aml_append(fdi,
>>>>>> +        aml_int(cylinders - 1));  /* Maximum Cylinder Number */
>>>>> this puts uint64_t(-1) in AML i.e. cylinders == 0 and overflow happens 
>>>>> here
>>>>>
>>>>> CCing Jon
>>>>
>>>> I guess this is the effect of John's fdc rework.  I used to think zero
>>>> geometry was impossible at the time this patch was developed.
>>>>
>>>> I wonder if it hasn't been fixed already by
>>>>
>>>>   commit fd9bdbd3459e5b9d51534f0747049bc5b6145e07
>>>>   Author: John Snow <address@hidden>
>>>>   Date:   Wed Feb 3 11:28:55 2016 -0500
>>>>
>>>>       fdc: fix detection under Linux
>>>
>>> Yes, hopefully solved on my end. The geometry values for an empty disk
>>> are not well defined (they certainly don't have any *meaning*) so if you
>>> are populating tables based on an empty drive, I just hope you also have
>>> the mechanisms needed to update said tables when the media changes.
>>
>> I don't.  At the time the patch was developed there basically were no
>> mechanisms to update the geometry at all (and this was what you patchset
>> addressed, in particular, wasn't it?) so I didn't care.
>>
> 
> That's not true.
> 
> You could swap different 1.44MB-class diskettes for other geometries,
> check this out:
> 
> static const FDFormat fd_formats[] = {
>     /* First entry is default format */
>     /* 1.44 MB 3"1/2 floppy disks */
>     { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
>     { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
>     { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
>     { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
>     { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
>     { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
>     { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
>     { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
> ...
> 
> You absolutely could get different sector and track counts before my
> patchset.
> 
> 
>> Now if it actually has to be fully dynamic it's gonna be more
>> involved...
>>
>>> What do the guests use these values for? Are they fixed at boot?
>>
>> Only Windows guests use it so it's hard to tell.  I can only claim that
>> if I stick bogus values into that ACPI object the guest fails to read
>> the floppy.

We discussed this with John a bit on IRC.

In my opinion, the real mess in this case is in the ACPI spec itself. If
you re-read the _FDI control method's description, the Package that it
returns contains *dynamic* geometry data, about the *disk* (not *drive*):

- Maximum Cylinder Number // Integer (WORD)
- Maximum Sector Number   // Integer (WORD)
- Maximum Head Number     // Integer (WORD)

What this seems to require is: the firmware developer should write ACPI
code that
- dynamically accesses the floppy drive / controller (using MMIO or IO
  port registers),
- retrieves the geometry of the disk actually inserted,
- and returns the data nicely packaged.

In effect, an ACPI-level driver for the disk.

Now, John explained to me (and please keep in mind that this is my
personal account of his explanation, not a factual rendering thereof),
that there used to be no *standard* way to interrogate the current
disk's geometry, other than trial and error with seeking.

Apparently in UEFI Windows, Microsoft didn't want to implement this
trial and error seeking, so -- given there was also no portable
*hardware spec* to retrieve the same data, directly from the disk drive
or controller -- they punted the entire question to ACPI. That is, to
the firmware implementor.

This is entirely bogus. For one, it ties the platform firmware (the UEFI
binary in the flash chip on your motherboard) to your specific floppy
drive / controller. Say good-bye to any independently bought & installed
floppy drives.

In theory, a floppy controller that comes with a separate PCI card could
offer an option ROM with a UEFI driver in it, and that UEFI driver could
install a separate SSDT with the hardware-matching _FDI in it. Still an
unreasonable requirement, given that the *only* way Windows can be
installed unattended (with external device drivers) is to provide those
drivers on a floppy. Because, end-to-end,

  do you want unattended UEFI installation with 3rd party drivers?

translates to

  better have a PCI-based floppy controller such that its oprom
  installs an _FDI with dynamic hardware access,
  *or* have your platform firmware match your floppy hardware

Implementing this in QEMU would require:
- inventing virt-only registers for the FDC that provide the current
disk geometry,
- and generating AML code that reads those registers

*or*

- implementing the trial and error seeking in AML

Waste of time, don't do it. Microsoft have never documented their usage
of _FDI. (Their forums are full of confused users whose physical floppy
drives don't work under UEFI Windows!)

I'm quite sure the _FDI addition in the ACPI spec is actually from
Microsoft as well, but of course the *reasoning* / background for _FDI
is also not public. Microsoft seem to push stuff into the ACPI spec that
serves them, while conveniently ignoring non-optional parts of the spec
that they don't feel like supporting (I'm looking at you,
DataTableRegion). And their level of support is not public.

So, the last paragraph of Roman's email
<http://thread.gmane.org/gmane.comp.emulators.qemu.block/7978/focus=8081> 
remains
relevant -- do whatever ugly static hack is necessary in QEMU's AML
generator to restore the one use case to working state that matters:
unattended installation to a virtio disk.

Noone in their right mind uses floppy in the guest interactively, and
even for the unattended installation, floppy is used only because
Windows is too stupid to work off a CD-ROM fully automatically. (Where
everything one needs would be interrogable from on hardware / ATAPI / SCSI.)

IMHO, do the *absolute minimum* to adapt this AML generation patch to
John's FDC rework, and ignore all dynamic aspects (like media change).

Thanks
Laszlo