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 <js...@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
