From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web11.1770.1593024253446873695 for ; Wed, 24 Jun 2020 11:44:13 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=F/6/F1oT; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593024250; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6N0A8yfcEfnIB5NIvAlsqaTmQSpVExF7+880w9WVbM0=; b=F/6/F1oTsv+WMQOZoxJpNGYWfpPSWIsA+S4Ow+xPvORfkM/0NrG2bzFXnmtJYfrXfU1p92 Zm/YohElByIDIOvcdoy3n7I2LX3R4tqUWcXDttWzsVn6GfxkjYBdRh8xZ9q5VSaPIVc848 f26PH8WuEfd1voiU/jCIiCd9ah2JTvg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-355-Nn1aANAqMBqVIONlWPmxiw-1; Wed, 24 Jun 2020 14:44:08 -0400 X-MC-Unique: Nn1aANAqMBqVIONlWPmxiw-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 815BF100CCC1; Wed, 24 Jun 2020 18:44:07 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-115.ams2.redhat.com [10.36.112.115]) by smtp.corp.redhat.com (Postfix) with ESMTP id 10EA7100EBA4; Wed, 24 Jun 2020 18:43:58 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery To: Andrew Jones , Ard Biesheuvel Cc: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , devel@edk2.groups.io, Eric Auger References: <20200623175700.1564281-1-ard.biesheuvel@arm.com> <85a63fc4-f3d1-1e17-bf1d-dace59bb02a8@arm.com> <2a43b904-5172-0fb3-6d40-e1fd3b652fe7@redhat.com> <20200624134154.isbcqvscs7eidbd4@kamzik.brq.redhat.com> <20200624143709.odjhl5xc7ehxe2xl@kamzik.brq.redhat.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 24 Jun 2020 20:43:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200624143709.odjhl5xc7ehxe2xl@kamzik.brq.redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit 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".) Thank you very much! Laszlo