From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x230.google.com (mail-it0-x230.google.com [IPv6:2607:f8b0:4001:c0b::230]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 254F58044F for ; Mon, 20 Mar 2017 04:20:09 -0700 (PDT) Received: by mail-it0-x230.google.com with SMTP id g138so90910050itb.0 for ; Mon, 20 Mar 2017 04:20:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=LOmFTXzWf9oumurkuReoZJEosyw2CTH+sNO7OLCldO4=; b=GczsX80t+iq7bsQq0jXMcnoXE7JIAVnaJIyEPjaJGYDWCNG2iFRuQdON35vPHZ2Zrr lf9uZGdwk1oXM1O4CDKQaAZE8Gy+yrtohO+07FzeMWELRj/tgrqxKERcZ3ZA/O4LM8Lb RLF6QrzSg00qMyiq7FtaccIBDdGqhbEMqhhPE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=LOmFTXzWf9oumurkuReoZJEosyw2CTH+sNO7OLCldO4=; b=J5B8KcOfq++MXksRVrUCS+r8pNgTIzILXBJ1JcY/lFt/H0j+haKvJdKJ46O8dWHWqh 3edNcnWg6mEv2x7X5G6MojKzFyiC/HY2yJ+Zd2bYDiWvv+0twK8wO9sRGQrTdW3NV+tP nHLhLWuc1ZkhPzmRaJxWTqXyn3dxoO19erVo+8nU8gVIN6LJQYUMLiANCDggBNd0ykgM 7x/voTifylg8MqbqmuNnlYrcSm71ll65nZuSiyYmdwOWzcrZP0jYoS1+xRiswQFjCWfE xnCleob2zAcaimRaZ8fyt6Ifs1m1LO4b2q1gE6H14QF/DOZGGCPkCSCnID7BeLN1E3S4 Y8NA== X-Gm-Message-State: AFeK/H0U6/HZu4IqgxpY1Hl3Yb3ra0+GHvgxyr9G7yxZcBR1A2bLVVB22zrilV08aLUp2ILWcyL5mOJuCTfH7KzS X-Received: by 10.36.77.10 with SMTP id l10mr8519361itb.59.1490008808505; Mon, 20 Mar 2017 04:20:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.10.27 with HTTP; Mon, 20 Mar 2017 04:20:08 -0700 (PDT) In-Reply-To: References: From: Ard Biesheuvel Date: Mon, 20 Mar 2017 11:20:08 +0000 Message-ID: To: Michael Zimmermann Cc: edk2-devel-01 , Laszlo Ersek 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 11:20:09 -0000 Content-Type: text/plain; charset=UTF-8 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? > On Mon, Mar 20, 2017 at 12:04 PM, Ard Biesheuvel > wrote: >> On 20 March 2017 at 10:32, Michael Zimmermann wrote: >>> Hi, >>> >>> I didn't test ArmVirtQemuKernel but I'm trying to use some of the code >>> for another platform. >>> So does this call ever succeed with PcdDxeNxMemoryProtectionPolicy >>> being enabled? >>> https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L95 >>> Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes); >>> >>> Neither the memory that was added by this Dxe nor the one added >>> automatically by GCD has the EFI_MEMORY_XP capability which causes >>> SetMemorySpaceAttributes to return EFI_UNSUPPORTED. >>> >> >> That is a very good point. I have been caught by this more than once >> already (and I did test this, but not as thoroughly as I should have, >> apparently) >> >> This is caused by the unfortunate situation in EDK2 that GCD >> permission attributes are ambiguous: it does not distinguish between >> 'the memory controller allows this range to be configured as >> non-executable' and 'the nature of the contents of this memory region >> allows it to be mapped without executable attributes', and therefore, >> RO/XP are never used in the GCD memory space map. >> >> The solution is to use the CPU_ARCH_PROTOCOL interface explicitly to >> set the XP attribute on the memory itself (but not on the descriptors >> in the GCD or UEFI memory maps). I will spin a patch to fix this. >> >> Thanks, >> Ard.