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

[John Snow <jsnow@redhat.com>]

On 02/09/2016 01:48 PM, Michael S. Tsirkin wrote:
> On Tue, Feb 09, 2016 at 07:36:12PM +0100, Laszlo Ersek wrote:
>> 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 <jsnow@redhat.com>
>>>>>>   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.
>>

At the very least, the Intel 82078 does not appear to be capable of
replying to any query with a CHS triplet. It *can* report back the total
number of sectors and the size of each sector, but that still requires
geometry guesswork outside of the drive.

>> 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,

We do secretly have these registers, but of course there's no spec to
interpreting what they mean. For instance, what do they read when the
drive is empty? I am not sure that information is codified in the ACPI spec.

Could be wrong, you seemed to indicate that the ACPI spec said that the
info matches what you get from a certain legacy bios routine, but I
don't know the specifics of that routine, either.

>> - 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
> 
> I'm fine with reporting some static values that make windows work fine.
> What I think is wrong is reporting geometry that happens to
> match current windows.
> 

Can you rephrase that last sentence? I don't follow, I'm sorry.


Roman, does the 0xFF "empty disk geometry" hack appear to work for
Windows 10?

Maybe it's fine enough as-is, as per Laszlo's good synopsis here.

--js