On 06/24/20 20:43, Laszlo Ersek wrote: > On 06/24/20 16:37, Andrew Jones wrote: >> On Wed, Jun 24, 2020 at 03:48:46PM +0200, Ard Biesheuvel wrote: >>> On 6/24/20 3:41 PM, Andrew Jones wrote: >>>> On Wed, Jun 24, 2020 at 03:02:49PM +0200, Philippe Mathieu-Daudé wrote: >>>>> On 6/24/20 1:43 PM, Ard Biesheuvel wrote: >>> ... >>>>>> I wasn't aware that we even expose the flash in the DSDT. In any case, >>>>>> no driver exists in Linux today that claims the LNRO0015 _HID, and so I >>>>>> agree we should simply remove it entirely. >>>>>> >>>>>> However, I am no longer able to contribute to QEMU, so I was hoping you >>>>>> or Phil could take the action? >>>>> >>>>> I try to stay as far as possible from ACPI, but here it seems >>>>> fair I assign myself to this (except if Drew/Eric prefer to >>>>> do it, of course!). >>>> >>>> I don't mind doing it. IIUC, all we need to do is remove the flash device >>>> from the DSDT to "hide" it from the guest. Of course we'll need some >>>> compat code too in order to only do this for machine types 5.1 and later, >>>> and that means that running guest kernels which want to bind the flash on >>>> 5.0 and older machine types will still have the problem. >>>> >>> >>> Do you think that is really necessary? LNRO0015 never had a driver in Linux >>> to begin with, and I doubt other ACPI capable arm64 OSes would be any >>> different. >> >> I'd rather not add/remove hardware in older machine types. While it's >> unlikely anybody would notice, we can't be sure that there's nothing >> out there which would break. > > I agree. > >>> >>>> Also, it seems a bit odd to hide hardware from the guest OS. Wouldn't it >>>> be better to somehow flag that the hardare may be in use by firmware, >>>> and therefore is only safe to use if runtime services are not used? I'm >>>> not sure ACPI supports that for table entries like these, but maybe some >>>> _STA value indicates something like it. I'll take a look at the spec. >>>> >>> >>> We could do either, but a _STA indicating that the device is not present or >>> not ready amounts to the same afaik. Ultimately, the OS could still access >>> the physical range if it wanted to (e.g., via /dev/mem), so not exposing it >>> in the first place seems more robust to me. >>> >> >> If there's no _STA state that says the device is here and works, but it's >> not available, then I agree removing it is the same. And, thinking about >> it some more, this flash device is really only for our host-controlled >> firmware. Since we don't give the guest any control over the firmware, >> then the device the firmware lives on should probably not even exist as >> far as the guest is concerned. > > I think it's safest to remove the object from the DSDT; at least x86 > Windows used to be really picky about _STA in Device Manager. Best to > avoid yellow triangles (or whatever) there (assuming Device Manager is a > thing on aarch64 Windows -- I don't know). > > Drew, when you remove the flash addition function call, please replace > it with a comment that's similar to the one we have about the RTC. That > way, we can run "git blame" on the comment. (Pure deletions tend to > impede "git blame", as no code lines remain on which git-blame could > report a "latest commit".)
oops, sorry, dumb request -- this isn't going to be a pure removal; the flash addition will remain, it will only be made conditional on a new machine type property. So there's going to be *something* to run git-blame on. Apologies for my mistake. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61693): https://edk2.groups.io/g/devel/message/61693 Mute This Topic: https://groups.io/mt/75065345/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
