From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.18723.1586968832740117865 for ; Wed, 15 Apr 2020 09:40:33 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=fKBfubbv; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1586968831; 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=0SKJBq3Emkijkzp6ptMUoHMu4DgXdR2/WG1lJG/CAnM=; b=fKBfubbvSOcu+AmG3zpZdyl/FCslwMUUh9YyrIiIHJ/iYmx5+3v7pCpuMz90HmU8bP/Sjd b1kLvElsDT3aeQR/GfPxo4TmB2kXm/v/7SwbPxE3aRSF/6ySxTpRU1A0u1FahX3Qzotz6d fcPLeahlacjfJeSb3TWqkC4alYLB1Lc= 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-348-ryRbOjzIOReq1ymIEUGV6w-1; Wed, 15 Apr 2020 12:40:30 -0400 X-MC-Unique: ryRbOjzIOReq1ymIEUGV6w-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 7411E190B2AA; Wed, 15 Apr 2020 16:40:26 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-155.ams2.redhat.com [10.36.112.155]) by smtp.corp.redhat.com (Postfix) with ESMTP id 082781059112; Wed, 15 Apr 2020 16:40:24 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Register SnpNotifyExitBootServices at TPL_CALLBACK From: "Laszlo Ersek" To: "Rabeda, Maciej" , devel@edk2.groups.io, michael.kubacki@outlook.com Cc: Siyuan Fu , Jiaxin Wu , "Tomas Pilar (tpilar)" References: Message-ID: <44e4dac1-e4b3-e158-e291-3f9776235599@redhat.com> Date: Wed, 15 Apr 2020 18:40:24 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: 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: 7bit On 04/15/20 18:25, Laszlo Ersek wrote: > In other words, I personally believe that bug#1974 should have been > closed as INVALID (without patching the edk2 source). The EBS handler in > SnpDxe is necessary (as long as it does not alter the UEFI memory map > itself), because sending the Shutdown CDB at EBS *is* needed, and the > UNDI-providing agent that processes the Shutdown CDB *cannot* release > UEFI-owned memory. For the patch that's being proposed in this thread: Reviewed-by: Laszlo Ersek However, there *is* a bug in SnpNotifyExitBootServices(). Namely, it calls PxeShutdown(), and PxeShutdown() does two things: (a) it sends the Shutdown CDB, (b) it calls Snp->PciIo->FreeBuffer(). Step (a) is required in the EBS handler context. Step (b) is *forbidden* in the EBS handler context. Therefore, the cut must be made *between* steps (a) and (b). PxeShutdown() is alright to call from SimpleNetworkDriverStop(), but it is *not* fit for an EBS handler. (SnpNotifyExitBootServices() also calls PxeStop(). I'm not sure if that is really necessary, but at least PxeStop() only sends a CDB to UNDI, and manipulates "Snp->Mode.State". Those actions do not interfere with the UEFI memmap, so they are harmless in this aspect.) "in my opinion", as always :) Thanks, Laszlo