From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53756) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTD9C-0001YV-AM for qemu-devel@nongnu.org; Tue, 09 Feb 2016 13:36:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTD97-00086k-NN for qemu-devel@nongnu.org; Tue, 09 Feb 2016 13:36:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36379) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTD97-000849-G4 for qemu-devel@nongnu.org; Tue, 09 Feb 2016 13:36:17 -0500 References: <1454612376-7072-1-git-send-email-mst@redhat.com> <1454612376-7072-49-git-send-email-mst@redhat.com> <20160205192507.41fc6024@nial.brq.redhat.com> <20160208131443.GC6420@rkaganb.sw.ru> <56B8F89F.3090603@redhat.com> <20160209155249.GA13787@rkaganb.sw.ru> <56BA1229.8090809@redhat.com> From: Laszlo Ersek Message-ID: <56BA319C.9010505@redhat.com> Date: Tue, 9 Feb 2016 19:36:12 +0100 MIME-Version: 1.0 In-Reply-To: <56BA1229.8090809@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 48/49] i386: populate floppy drive information in DSDT List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Kagan Cc: Peter Maydell , Eduardo Habkost , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Paolo Bonzini , Igor Mammedov , John Snow , Richard Henderson 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 >>>> 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 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