From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::22e; helo=mail-it0-x22e.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x22e.google.com (mail-it0-x22e.google.com [IPv6:2607:f8b0:4001:c0b::22e]) (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 276EB222A3375 for ; Wed, 24 Jan 2018 03:29:31 -0800 (PST) Received: by mail-it0-x22e.google.com with SMTP id c16so4842526itc.5 for ; Wed, 24 Jan 2018 03:35:00 -0800 (PST) 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=nS4sapE9ER+12Xg+PfGy5xKIzDtAswJ0f5dqoqeQLe8=; b=N9aydnDsR6NC3NjfxaOBp3ial3LH9ofIvIPiNSPjCUS+23/NETFgD99e7LNdDRFpk7 eign8bpyiYhsf1V/77KzAu5s6fJwwmNOyzc2WXtByEIkAk6I1bYe4wFpsOZiXgxk/+zQ JETKF5IvSUSNmGKyZJXQJ0Hw3EizFqZMXkh54= 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=nS4sapE9ER+12Xg+PfGy5xKIzDtAswJ0f5dqoqeQLe8=; b=K9Zy5a24+QZpmzklQ+cBlrLxKfDELhHc78wI9fAfn9pyH3LfRbDDZlBM1rMeJa7b39 qjaApxJz60lVzAcwPoVr7LcqoFRpAbtsSxmI+hIeMWYP8t2ogiy7Qr9R34lBDsERy9GX qYRzqxpPkaQoc8nMrNlnXVTA18JS1mRaelUVychQ4mveAJwH1q4ZgE5UidWK8Ih650sd NjMbRjHaAjQ3T5v35oMz27hB57vsti0XW5ucjmp1JwGZu7ve5UKqEC8J8rN+iYMNEYQy h7D03ubfiUjf24xp1KKHdjCOqYb22AjJlb226GiCw8w0Cg8DiCyCgU5nNVMbE4Wbe0hl XiTw== X-Gm-Message-State: AKwxytfJpcgKwOhJw2efcRdCTSdL6nz4M/qZhVYKK/7VfaTHYIVa+JAI bufOEBh8bz9sgmayeraZ3abs3trVh+WNDnxnLd8ruA== X-Google-Smtp-Source: AH8x2251UaP7f1D/5fSdN3KIaFN6o3qv1vNAGGfIiqcfCLH4cO1FA6ndoEqEAisfASnMLSt3QJ9WnFmTHKJ/KQFgrZ8= X-Received: by 10.36.7.9 with SMTP id f9mr8014462itf.143.1516793699382; Wed, 24 Jan 2018 03:34:59 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.112.13 with HTTP; Wed, 24 Jan 2018 03:34:59 -0800 (PST) In-Reply-To: References: <20171222190821.12440-1-evan.lloyd@arm.com> <20171222190821.12440-16-evan.lloyd@arm.com> From: Ard Biesheuvel Date: Wed, 24 Jan 2018 11:34:59 +0000 Message-ID: To: Alexei Fedorov Cc: Evan Lloyd , "Matteo.Carlini@arm.com@arm.com" <"Matteo.Carlini@arm.com"@arm.com>, "leif.lindholm@linaro.org@arm.com" <"leif.lindholm@linaro.org"@arm.com>, "nd@arm.com@arm.com" <"nd@arm.com"@arm.com>, "edk2-devel@lists.01.org" , Arvind Chauhan , "ard.biesheuvel@linaro.org@arm.com" <"ard.biesheuvel@linaro.org"@arm.com>, Thomas Abraham , Sami Mujawar , Mitch Ishihara Subject: Re: [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New DP500/DP550/DP650 platform library. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jan 2018 11:29:32 -0000 Content-Type: text/plain; charset="UTF-8" On 24 January 2018 at 11:27, Alexei Fedorov wrote: > Hi Ard, > > > You wrote: > > >> > + Status = gDS->SetMemorySpaceAttributes ( >> > + *VramBaseAddress, >> > + *VramSize, >> > + EFI_MEMORY_WC >> >> Please add EFI_MEMORY_XP here >> > > > Setting EFI_MEMORY_XP causes assertion because > gDS->SetMemorySpaceAttributes() returns Unsupported in > edk2\MdeModulePkg\Core\Dxe\Gcd\Gcd.c with Status set at line #834: > > > if ((Entry->Capabilities & Attributes) != Attributes) { > > //***AF > DEBUG ((DEBUG_GCD, " @%d EFI_UNSUPPORTED: Capabilitie=0x%lX > Attributes=0x%lX\n", > __LINE__, Entry->Capabilities, Attributes)); > > Status = EFI_UNSUPPORTED; > goto Done; > } > > because Entry->Capabilities don't have EFI_MEMORY_XP set, see debug output > below: > > If gDS->SetMemorySpaceAttributes() call is replaced with > Cpu->SetMemoryAttributes(), attributes are set successfully with eXecute bit > for frame buffer being cleared. > Ugh. The GCD map conflates capabilities of the region (i.e., this region can be configured as read-only by the MMU) with permissions (i.e., the contents of this region should not be modified by the program), which is why nobody bothers to set these attributes for the GCD region. If you do set those bits in the GCD map, all allocations carved out of it will be read-only and non-executable (because, hey, why would you allocate writable or executable memory from a region that can be configured as read-only or non-executable?). In any case, your solution is the correct one: use the arch CPU protocol directly instead (although there is something in the PI spec that bans it). Having an executable frame buffer is a much bigger deal IMHO, especially when all other allocations are being locked down, and so my preference is to switch to the arch CPU protocol here. Thanks, Ard.