From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZNC4K-0007R5-Kt for mharc-grub-devel@gnu.org; Wed, 05 Aug 2015 23:42:12 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53614) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNC4H-0007Qc-K7 for grub-devel@gnu.org; Wed, 05 Aug 2015 23:42:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZNC4E-0003LI-Cs for grub-devel@gnu.org; Wed, 05 Aug 2015 23:42:09 -0400 Received: from mail-lb0-x22e.google.com ([2a00:1450:4010:c04::22e]:36676) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNC4E-0003Ky-5L for grub-devel@gnu.org; Wed, 05 Aug 2015 23:42:06 -0400 Received: by lbbpu9 with SMTP id pu9so8047489lbb.3 for ; Wed, 05 Aug 2015 20:42:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=tnJdHVSzO/WHqPqPJCV0Abwzin12t+rD69gJ+AmJUMc=; b=i8puCtW/9fWXNqDqyCU6vVMaYs5qF5AnSr9T/BEMhNO1iHV5mmes2hVGSjsiNVOo2q WsWy+3WRiCylQE2PZkhIbLmubgTfFdIRhbBvj2YQtVWMpQ2v8E39ElVaABZKSrAl0w8t x19jirECf+Vt6EMJTGWSnBzTPKbdLz2Oo+Bqlk5EZTjBDrtR9N30rSPMisWJaAw83aYJ yH6vYuUqVpxysapvrdtj+mhucMoQzGHI/DFsojnDVZj9J5Ao7e18jxpgBXOvZ/l36ElW E2O6B1wJDm1Zpa9jaghSM9kxP0KZGVz0J/JaaihVlsLZPwrNKax+p9HMCU88XyeJIlYk kGgw== X-Received: by 10.112.129.131 with SMTP id nw3mr12929045lbb.34.1438832525281; Wed, 05 Aug 2015 20:42:05 -0700 (PDT) Received: from opensuse.site (ppp91-76-6-204.pppoe.mtu-net.ru. [91.76.6.204]) by smtp.gmail.com with ESMTPSA id f1sm1038838laf.15.2015.08.05.20.42.03 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 05 Aug 2015 20:42:04 -0700 (PDT) Date: Thu, 6 Aug 2015 06:42:00 +0300 From: Andrei Borzenkov To: "Vladimir 'phcoder' Serbinenko" Subject: Re: [PATCH 1/3] efinet: handle get_status() properly Message-ID: <20150806064200.49ef8397@opensuse.site> In-Reply-To: References: <1438799799-32097-1-git-send-email-jbacik@fb.com> <20150805230442.1bf65b2d@opensuse.site> <55C27179.6020104@fb.com> X-Mailer: Claws Mail 3.11.0 (GTK+ 2.24.28; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:4010:c04::22e Cc: The development of GRUB 2 , mchang@suse.com X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Aug 2015 03:42:10 -0000 =D0=92 Wed, 5 Aug 2015 22:32:13 +0200 "Vladimir 'phcoder' Serbinenko" =D0=BF=D0=B8=D1=88=D0= =B5=D1=82: > This patch improperly assumes that GRUB is the only thing in EFI that > transmits. Actually since recently we try to ensure that grub *is* the only user of network interface.=20 > Your patch surely fixed your machine but likely breaks some > other machines. Could you instead make an explicit check for (void *)1 and > add a comment on which machine it's necessary? Yes, this patch should add verbose comment to code explaining a) what problem it tries to fix and b) why ignoring EFI specification is justified here. Also mention actual hardware/firmware implementation where this bug happens for future reference. Looking at other implementations gPXE opens SNP non-exclusively and explicitly checks returned address. That corresponds to what grub did in the past. iPXE opens SNP exclusively and assumes anything !=3D NULL means transmit completed (it seems to start off with gPXE model). > Le 5 ao=C3=BBt 2015 10:28 PM, "Josef Bacik" a =C3=A9crit : >=20 > > On 08/05/2015 04:04 PM, Andrei Borzenkov wrote: > > > >> =D0=92 Wed, 5 Aug 2015 14:36:37 -0400 > >> Josef Bacik =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > >> > >> The EFI SNP documentation isn't super clear on the value that is retur= ned > >>> in > >>> txbuf when calling into GetStatus. The documentation says its the > >>> pointer to > >>> the recycle buffer, but the documentation for Transmit() says that it > >>> should be > >>> the pointer to the buffer that we transmitted. > >>> > >> > >> Actually it says "Recycled transmit buffer address" and further > >> "GetStatus() until the transmitted buffer shows up in the recycled > >> transmit buffer" so it looks reasonably clear to me. > >> > >> On the boxes I'm using > >>> it's just > >>> a random pointer (usually 0x1). It is definitely transmitting stuff, > >>> but the > >>> get_status call is not returning the pointer to the txbuf we passed i= n. > >>> > >> > >> Which sounds like firmware bug. To be sure - you observe it also using > >> current GIT master? > >> > >> > > This is on git master as of last week, so I have your latest patch > > > > efinet: enable hardware filters when opening interface > > > > and it was still happening. I know what Transmit() says, but GetStatus= () > > says it'll just be a pointer to the recycled transmit buffer address, w= hich > > to me could mean the pointer to whatever the internal queue was being u= sed > > by UEFI. Anyway that's not important, what is important is that the > > current code doesn't work with hardware that exists in the wild. If it= 's a > > firmware bug then fine, what do users do if they have buggy firmware th= at > > isn't being updated anymore? I think making grub more tolerant to crap= py > > firmware is a good thing. Thanks, > > > > Josef > > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.org > > https://lists.gnu.org/mailman/listinfo/grub-devel > >