From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 83B928046A for ; Mon, 20 Mar 2017 08:24:25 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E307C8E670; Mon, 20 Mar 2017 15:24:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E307C8E670 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E307C8E670 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-55.phx2.redhat.com [10.3.116.55]) by smtp.corp.redhat.com (Postfix) with ESMTP id E7425B2269; Mon, 20 Mar 2017 15:24:24 +0000 (UTC) To: Ard Biesheuvel References: <72d13754-5c7d-9734-33a8-bd2dab32b2bb@redhat.com> Cc: Michael Zimmermann , edk2-devel-01 From: Laszlo Ersek Message-ID: Date: Mon, 20 Mar 2017 16:24:22 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 20 Mar 2017 15:24:26 +0000 (UTC) Subject: Re: SetMemorySpaceAttributes with EFI_MEMORY_XP X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Mar 2017 15:24:25 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 03/20/17 15:08, Ard Biesheuvel wrote: > On 20 March 2017 at 11:38, Laszlo Ersek wrote: >> On 03/20/17 12:20, Ard Biesheuvel wrote: >>> On 20 March 2017 at 11:16, Michael Zimmermann wrote: >>>> Ard, why is SetMSetMemorySpaceAttributes being called in first place? >>>> (ignoring the recent NX patch) >>>> Looking at the initial GCD, it looks like unused memory usually >>>> doesn't have any attributes set anyway. >>>> >>> >>> Originally, we added the new memory with >>> EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC|EFI_MEMORY_UC capabilities, >>> and selected the EFI_MEMORY_WB attribute with the call to >>> gDS->SetMemorySpaceAttributes. Later, we removed all capabilities >>> expect EFI_MEMORY_WB, since the other ones cannot be supported under >>> virtualization with KVM. >>> >>> Whether that makes the SetMemorySpaceAttributes redundant is not >>> entirely clear to me, but it does appear the adding the memory does >>> the right thing wrt non-exec permissions if the policy is enabled. So >>> perhaps we can simply drop this call? >> >> Won't that turn off caching for the memory just added? >> > > I think it may not map the memory at all in this case, so we need to > do something. It looks like calling mCpu->SetMemoryAttributes() should > be sufficient here, and so I wonder whether we violate anything by > replacing gDS->SetMemorySpaceAttributes with mCpu->SetMemoryAttributes > here. Earlier you mentioned that we cannot get some piece of information from the GCD map, which limits what we can do here. Looking at GetMemorySpaceDescriptor() and GetMemorySpaceMap(), they should return both Capabilities and Attributes. Also, looking at vol2 in PI 1.5, I find: * GetMemorySpaceDescriptor() returns EFI_NOT_AVAILABLE_YET if "The attributes cannot be set because CPU architectural protocol is not available yet." * 9.7.3.2 SetMemorySpaceAttributes() -- When the DXE Foundation is notified that the EFI_CPU_ARCH_PROTOCOL has been installed, then the DXE Service SetMemorySpaceAttributes() can be made available. The DXE Foundation can then use the SetMemoryAttributes() service of the EFI_CPU_ARCH_PROTOCOL to implement the DXE Service SetMemorySpaceAttributes(). * 9.7.3.3 GetMemorySpaceMap() -- When the DXE Foundation is notified that the EFI_CPU_ARCH_PROTOCOL has been installed, then the DXE Service GetMemorySpaceMap() is fully functional. This function is made available when the memory-based services are initialized. However, the Attributes field of the array of EFI_GCD_MEMORY_SPACE_DESCRIPTORs is not valid until the EFI_CPU_ARCH_PROTOCOL is installed. So, assuming that you have tested GetMemorySpaceMap() earlier, and have found Attributes useless for the protection (or other) purposes, may that have happened because the CPU arch protocol was not available yet? ... I guess my email is a bit confusing. My points are, (a) we should likely not mess directly with the CPU arch protocol if we know (and we do know) that the GCD services use them internally, (b) are we absolutely sure that the GCD services can't help us here? If nothing else works, I agree we can mess with the CPU arch protocol directly. Thanks Laszlo