On 09/03/15 18:41, j...@joshtriplett.org wrote: > On Thu, Sep 03, 2015 at 05:53:45PM +0200, Laszlo Ersek wrote: >> On 09/03/15 16:50, Josh Triplett wrote:
>>> Do you virtualize those I/O ports by CPU, to make them thread-safe, or >>> does the last address written to 0x510 get saved system-wide, making it >>> unsafe for concurrent access? >> >> I think fw_cfg is not meant to be accessed by several CPUs concurrently. >> The protocol is stateful (selected key, offset within blob associated >> with selected key, etc), and "accessing CPU" is not part of that state. > > Not that hard to fix; just keep all the state in the accessing CPU > rather than the system. Current processors do that for the PCI I/O port > pair, to avoid race conditions. You could easily do that for the fw_cfg > I/O ports. As a bonus, you then wouldn't need to take any kind of lock > around accesses to that state, because the CPU owns that state. > > (That's the easy fix; the harder fix would be creating a new race-free > MMIO protocol and mapping all of the data structures into memory > directly, which would provide a performance benefit as well. I'd love > to see a general version of such a protocol for a more efficient virtio > filesystem, though in the simpler case of fw_cfg you can just map all of > the structures into memory.) Well, first of all it should be justified why concurrent access to fw_cfg would be necessary / useful :) But, I'm not arguing against it per se; until very-very recently, the only client looking at fw_cfg has been the firmware (it's called "firmware config" after all), and it's quite natural to require that part of the firmware to be single-threaded. Now, there's a fully independent discussion / development underway that might want to expose fw_cfg to the kernel too. Let's not get into that here, please talk to Gabriel for references :) (Cc'd). ... If you feel tempted to ask "why are then fw_cfg bindings for ARM documented in the kernel?": because when we were extending fw_cfg from x86 to ARM, advertizing the affected register block in the DT that QEMU generated became a necessity, and Peter Maydell (QEMU maintainer and ARM guru) proposed that I document the fw_cfg-related DT stuff *in the kernel Documentation tree* -- simply because that was the largest, most central registry of such DT bindings. This is my recollection of it anyway. There's *yet another* development underway for speeding up fw_cfg transfers; please talk to Marc Marí (Cc'd) about those :) In any case, if what you need resembles a "general virtio filesystem", then please just use that -- a virtio-block or virtio-scsi disk, with a normal filesystem on it. The protocol is industry standard and the performance of the QEMU (and kernel) implementation is splendid. (For ad-hoc uses, even the "vfat" backend can be used, which practically implements semihosting.) fw_cfg was always meant as a set of small bits for the firmware to consume. The fact that we squeeze kernel and initrd blobs through it is just historical abuse that is now impossible to eradicate (and must be sped up instead :(). Whereas other creative reasons that Gabriel could tell you about why the kernel could be interested in fw_cfg, are just that: creativity. Which might be better served by virtio too (this is the subject of ongoing discussion). >> Another question: when you execute an AML method that does, say, IO port >> access, does the AML interpreter of ACPICA actually *perform* that IO >> port access? Because, the one that is embedded in Linux obviously does, >> and the one that is embedded in the userspace ACPICA command line >> utility "acpiexec" obviously doesn't. > > You need to pass unsafe_io=True to evaluate, in which case it'll do I/O. Fantastic! :) > Otherwise, it'll ignore I/O. (On our TODO list: ignoring but logging > I/O so we can look at the I/O accesses as part of the test.) > > Actually, that reminds me: we should probably fix AcpiOsWriteMemory to > do the same thing. > >> I assume (and very much hope) that the IO port access *is* performed >> from BITS, simply because you developed it for physical machines, and it >> wouldn't make much sense to avoid actual hardware access that was >> implemented by the BIOS vendor for that platform. > > We want to default to not performing those accesses, but we definitely > have the option to do so if you know you *want* to trigger real I/O. Yes, me wants that, badly. :) >> If that is the case, then this tool could become the killer ACPI tester >> for QEMU developers -- the hardware accesses in the AML methods >> generated by QEMU would actually poke QEMU devices! (Unlike the >> userspace "acpiexec" utility.) It would completely detach Linux guest >> driver development from host side / firmware development. \o/ > > That's exactly why we went with a pre-OS environment rather than an OS; > you don't want to undermine the OS, and you don't want your tests > affected by whatever the OS has done. And, since you implemented it as part of GRUB, it is not tied to either OVMF or SeaBIOS (speaking in virt terms for now) -- it's flexible. >> (I must say, I have found the LWN article at just the right time. I >> intend to start implementing a VMGenID device for QEMU, and it's all >> ACPI based. Here's our design for that: >> <http://thread.gmane.org/gmane.comp.emulators.qemu/357940>. I've been >> already dreading the need for a Linux guest driver, in order to >> white-box test the device & the ACPI stuff from the guest side. :)) > > Interesting! Yeah, BITS should make testing that trivial. You can read > out the identifier, snapshot and resume, and read it out again. That's the plan, yes. :) > One request there: please make that device optional in qemu, because > some users of qemu and snapshots specifically *won't* want the OS to > know that anything has happened. Oh, absolutely. There's a more direct reason for that, too: if I mess it up (-> likely) and it slips by reviewers (-> much less likely), it shouldn't be unleashed on the unsuspecting user base... Plus it is guest visible (minimally due to the ACPI objects), which could trigger Device Manager stuff in Windows, etc etc etc. Thank you! Laszlo > - Josh Triplett > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
