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::241; helo=mail-it0-x241.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x241.google.com (mail-it0-x241.google.com [IPv6:2607:f8b0:4001:c0b::241]) (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 9E506224DD13F for ; Wed, 28 Mar 2018 22:02:52 -0700 (PDT) Received: by mail-it0-x241.google.com with SMTP id 19-v6so6349374itw.3 for ; Wed, 28 Mar 2018 22:09:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:date:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=6KGq1t3C5F6LvjGYPacAdjDOIcE+5Q2A9HqB17zmxwg=; b=gmTLEOz7kxpoOm3M2HwbgEJ+XVn4KAYTCdYfq5sxkeEU7v6wB9zrnb4e1T4ipob8E9 XiR/64fF3qjegyz+hFcBXJAdRti3h+g9OjlJ+ycKRTU8R/A3f15M+2+aeBEvp7xD0Ih4 VLlPIYc89kvbPl8MPC3sqejvMaX1NI1hSC/8s= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=6KGq1t3C5F6LvjGYPacAdjDOIcE+5Q2A9HqB17zmxwg=; b=Y3nIW2EUd7nwgHWDIsgIvIB4oyv5rVLNdgBZ1Gqkd1cWUFwJ+0D9E1pFEZpiWeaW5N nqp+CZFWzdGBp1J+1tM+JHvQ4jsgenMFTlupnSo6sksCJ5yxD9YV2pzkwZl3HRXW0VSB 7qpfAY2ze7SUspTOkexRfCQjVX6Tfnl4TavDruAY3T5sAlbKGbQStb8QVNRxfMkCozDS tTRe1y85Z6bKZV0bztjMKBMhs9BpPkhmOhjNH1eeu6mHxOTWNduKhb80+8CmTIeQxUzp US6uiC7e/9TZJbFKGD3pIxVWUkp0DDO+7uHxEmXvQw6R/rCFzDULh9Jba7O6Htdd1xiF 6HQQ== X-Gm-Message-State: AElRT7G2c7q7Bo1EbfmJANzSHkjAyA/Um/a3cO4UY2PiM7ERVRrCkbKN X+F74NvC3xeVXsfUW4vZvxPdSA== X-Google-Smtp-Source: AIpwx485M1vGumRoRr0se3pS4wSCLh+l8n2LeLqAgtZptGu3omZ70e+WdemW++h+aR1RWv9j9wPWYw== X-Received: by 2002:a24:7cc:: with SMTP id f195-v6mr6740255itf.46.1522300171070; Wed, 28 Mar 2018 22:09:31 -0700 (PDT) Received: from SZX1000114654 ([104.237.91.79]) by smtp.gmail.com with ESMTPSA id x87-v6sm586887ita.35.2018.03.28.22.09.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 28 Mar 2018 22:09:30 -0700 (PDT) From: Guo Heyi X-Google-Original-From: Guo Heyi Date: Thu, 29 Mar 2018 13:09:26 +0800 To: "Wang, Jian J" Cc: "Zeng, Star" , Heyi Guo , "edk2-devel@lists.01.org" , Yi Li , Renhao Liang , "Dong, Eric" , "Kinney, Michael D" , "Gao, Liming" Message-ID: <20180329050926.GC97590@SZX1000114654> References: <1522290692-99585-1-git-send-email-heyi.guo@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103BA744FE@shsmsx102.ccr.corp.intel.com> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [RFC 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Mar 2018 05:02:53 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Jian, I excluded CpuArchAttributes == 0 on purpose, for I don't know the expected behaviour of gCpu->SetMemoryAttributes() if CpuArchAttributes == 0. Does that mean we need to keep the cache attribute and remove memory protection attributes? If so, then it seems we don't need the check at all. Thanks, Heyi On Thu, Mar 29, 2018 at 04:40:33AM +0000, Wang, Jian J wrote: > I agree. The only issue here is that case "Attributes == 0" is also excluded in this patch. > I think 0 should be CPU arch supported attributes. > > Regards, > Jian > > > -----Original Message----- > > From: Zeng, Star > > Sent: Thursday, March 29, 2018 11:20 AM > > To: Heyi Guo ; edk2-devel@lists.01.org > > Cc: Yi Li ; Renhao Liang > > ; Dong, Eric ; Kinney, > > Michael D ; Gao, Liming ; > > Wang, Jian J ; Zeng, Star > > Subject: RE: [RFC 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion > > > > The code before 14dde9e903bb9a719ebb8f3381da72b19509bc36 can allow the > > attribute to be a combination of CPU arch attribute and other attributes, for > > example, UC + RUNTIME. > > But current code could not allow the case, that seems a regression in the code. > > So, I agree the statement. > > > > Jian, will you please provide the special case you are awared? > > > > > > Thanks, > > Star > > -----Original Message----- > > From: Heyi Guo [mailto:heyi.guo@linaro.org] > > Sent: Thursday, March 29, 2018 10:32 AM > > To: edk2-devel@lists.01.org > > Cc: Heyi Guo ; Yi Li ; Renhao > > Liang ; Zeng, Star ; Dong, > > Eric ; Kinney, Michael D ; > > Gao, Liming > > Subject: [RFC 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion > > > > For gDS->SetMemorySpaceAttributes(), when user passes a combined memory > > attribute including CPU arch attribute and other attributes, like > > EFI_MEMORY_RUNTIME, ConverToCpuArchAttributes() will return > > INVALID_CPU_ARCH_ATTRIBUTES and skip setting page/cache attribute for the > > specified memory space. > > > > We don't see any reason to forbid combining CPU arch attributes and non-CPU- > > arch attributes when calling gDS->SetMemorySpaceAttributes(), so we change > > ConverToCpuArchAttributes() to only check if there is valid CPU arch attributes in > > the input "Attribute" parameter and just ignore other attributes. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Heyi Guo > > Signed-off-by: Yi Li > > Signed-off-by: Renhao Liang > > Cc: Star Zeng > > Cc: Eric Dong > > Cc: Michael D Kinney > > Cc: Liming Gao > > --- > > MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index 77f4adb4bc01..2ababdd14cc6 > > 100644 > > --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > @@ -673,8 +673,8 @@ ConverToCpuArchAttributes ( { > > UINT64 CpuArchAttributes; > > > > - if ((Attributes & ~(EXCLUSIVE_MEMORY_ATTRIBUTES | > > - NONEXCLUSIVE_MEMORY_ATTRIBUTES)) != 0) { > > + if ((Attributes & (EXCLUSIVE_MEMORY_ATTRIBUTES | > > + NONEXCLUSIVE_MEMORY_ATTRIBUTES)) == 0) { > > return INVALID_CPU_ARCH_ATTRIBUTES; > > } > > > > -- > > 2.7.4 >