From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web10.20757.1678717450225812461 for ; Mon, 13 Mar 2023 07:24:10 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=CLOAXN+B; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 3416BB81151 for ; Mon, 13 Mar 2023 14:24:08 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8506EC433A0 for ; Mon, 13 Mar 2023 14:24:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1678717446; bh=c1aQysqTUjxE76RIg7z8OA/UNF0xWsmNrWBfbccM5Is=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=CLOAXN+BcrBi/Y0cv/ckIR9tCXmagVFh/9bwOCsoGvFjDl8/F7KRZ0bEYz1B4Z/Vb +Uwvg6iWTj/hUM00tnxskMC0ximgbQr86XKiWs7XFoHaaOHQbI7Oe45W15SwNfrpaC TirKunSAXvS8AFbk5M9YFTUTwZFiFN+ZXZ1ZUwh4yp18I9Bmxabk8tjKoAlq+sX51n fyERIke43L8n8C75i6ULoXVCtAccM01HkZFTTVsrAum/sqSeaT+6A/wki38d2RjbM1 EcDRIH4twboVMwsbdjg+fvROdjT6DCPwxSIvDS1H+BpmmL7ne4M9FpBb7lu+sIZ4PK zjeCz+AfsDFcw== Received: by mail-lf1-f43.google.com with SMTP id y15so6422144lfa.7 for ; Mon, 13 Mar 2023 07:24:06 -0700 (PDT) X-Gm-Message-State: AO0yUKXmPJZEyzEqz/5TtmLQaGgHuO2ICTCKfDgWPzGUTMdhzkdzYNZL oSYnqte6jCBzksQD3AVFoDqKHGYwl7A/J/Q2VGk= X-Google-Smtp-Source: AK7set9nh77X8R3LywgqGGVcbE8XPbEf72/sChYg+q++UWodU4ZdBr3fimdbldGX36aSYtwVMB/AZlxTKrKQx5GV1d4= X-Received: by 2002:ac2:5312:0:b0:4e1:dbbb:493b with SMTP id c18-20020ac25312000000b004e1dbbb493bmr10945625lfh.4.1678717444316; Mon, 13 Mar 2023 07:24:04 -0700 (PDT) MIME-Version: 1.0 References: <20230209135936.789983-1-ardb@kernel.org> <5af401fe-d5f9-15ef-d021-b998395554e7@taylorbeebe.com> In-Reply-To: From: "Ard Biesheuvel" Date: Mon, 13 Mar 2023 15:23:53 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 00/11] ArmPkg: implement EFI memory attributes protocol To: Taylor Beebe Cc: devel@edk2.groups.io, Michael Kinney , Liming Gao , Jiewen Yao , Michael Kubacki , Sean Brogan , Rebecca Cran , Leif Lindholm , Sami Mujawar Content-Type: text/plain; charset="UTF-8" On Wed, 8 Mar 2023 at 18:24, Taylor Beebe wrote: > > My mistake - the DEBUG_PROPERTY_DEBUG_CLEAR_MEMORY_ENABLED feature is > why FreePagesWithProtectionAttributesTestCase might fail. > > To make the clear memory feature more compatible with the memory > attribute protocol, can you add a check to DebugClearMemoryEnabled() on > free and clear EFI_MEMORY_RP and EFI_MEMORY_RO if they're set? > I think the best strategy here is to simply apply the default permissions first, and only then actually free the pages, clear them etc etc That also fixes the existing issue where we may remap fewer pages than what we actually freed. I'll add a patch to my series for this. > > On 3/1/2023 1:57 PM, Ard Biesheuvel wrote: > > On Wed, 1 Mar 2023 at 21:43, Taylor Beebe wrote: > >> > >> > >> > >> On 2/11/2023 2:05 AM, Ard Biesheuvel wrote: > >>> On Sat, 11 Feb 2023 at 01:56, Taylor Beebe wrote: > >>>> > >>>> Hey Ard, > >>>> > >>>> Once the Memory Attribute Protocol is made available, Windows will have > >>>> some expectations about its functionality. Can you run this test app > >>>> created by me and Jiewen to ensure it meets the Windows requirements? > >>>> Part of the test needed an AARCH64 implementation which I just added - > >>>> let me know if it doesn't work. > >>>> > >>> > >>> Thanks, this is rather helpful. > >>> > >>> There appears to be an issue related to > >>> DEBUG_PROPERTY_DEBUG_CLEAR_MEMORY_ENABLED so I had to disable that to > >>> run these tests, as otherwise, the DXE core tries to clear freed pages > >>> before restoring the memory attributes. > >>> > >>> With that out of the way, the only test that fails is 'New > >>> EfiLoaderCode buffer attributes expected' because this firmware build > >>> maps loader code RWX, as existing boot stages for Linux are relying on > >>> this (including the kernel itself at this point) > >> > >> It makes sense that the NewEfiLoaderCode test fails, but I am surprised > >> the FreePagesWithProtectionAttributesTestCase passes. The test ensures > >> that a page with EFI_MEMORY_RP and/or EFI_MEMORY_RO has those attributes > >> cleared before attempting to free the page within the FreePage routine > >> and is related to the concern Marvin had. > >> > >> Did you make a change to the core or is there an execution path I'm not > >> seeing which allows that test to pass? > > > > No, I didn't make any additional changes to the core afair. >