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.web11.79060.1675758590289188383 for ; Tue, 07 Feb 2023 00:29:50 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=S3C1NpoV; 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 14A61B816C6 for ; Tue, 7 Feb 2023 08:29:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BD7A3C4339C for ; Tue, 7 Feb 2023 08:29:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1675758586; bh=tkMX46nDZP/SaCpjA2eK3Z0ndX4tx0hw11dBohbOJzg=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=S3C1NpoVF88yYsPqW53XLEwiz7ydPhVzNhA+idURGLfKlJycqZTiChgnC4mure9s5 WJ1RCUkpFbplZp/4aC/Bq87nnqPLgoXQ748MComV2Y+A7lYHzduHUKgOLic7M2O9Nf UVGeV2q0UPKYrAfG+vvnIXFAMkEmhLc5dFleeB6hWWUrFDAWtP9aeOcY3iNe82T2AR OPSHNBs0EheYughbM2bkXQeA/F1zlnjxhGXneiNVVggGn2PFIgjlfNNBmHjAzFrZqa 4S8IZtryZzkWRer8T6j7F4FTUVl1daPHLHjV3VhacweXI3UEvySKP9d+MyIHcJ5Ijb vuT/CzO+mAOIg== Received: by mail-lf1-f44.google.com with SMTP id j17so21320687lfr.3 for ; Tue, 07 Feb 2023 00:29:46 -0800 (PST) X-Gm-Message-State: AO0yUKVlf6t6gizvRygqzT8XIBzcDUhKgDlSjRAsAY5IwtsDc9fAeyq2 f2jgw/TRZXbcTNoqI611gbZkjyNLh6v/W7mCUeg= X-Google-Smtp-Source: AK7set8NP24xj+2temAeAPqb3j+Oun71JOrCpwghXsI1uFLAXoM7ft+RlyDHCdXKe0qbfvsXle9wR0drqDUqjits8pE= X-Received: by 2002:ac2:5550:0:b0:4b6:e197:3aeb with SMTP id l16-20020ac25550000000b004b6e1973aebmr352674lfk.233.1675758584771; Tue, 07 Feb 2023 00:29:44 -0800 (PST) MIME-Version: 1.0 References: <17027.1675454285173437530@groups.io> <15a314d3-2549-5ed3-b920-2a71b1ba518a@taylorbeebe.com> In-Reply-To: <15a314d3-2549-5ed3-b920-2a71b1ba518a@taylorbeebe.com> From: "Ard Biesheuvel" Date: Tue, 7 Feb 2023 09:29:33 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH 4/4] ArmPkg/CpuDxe: Implement EFI memory attributes protocol To: devel@edk2.groups.io, t@taylorbeebe.com Cc: =?UTF-8?Q?Marvin_H=C3=A4user?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 7 Feb 2023 at 02:18, Taylor Beebe wrote: > > I can't see the Bugzilla you referenced so I requested security Bugzilla > access. But, yes, that's the bug to which I was referring :) > I cannot see that bugzilla entry either. > Once Ard's change to add Memory Attribute Protocol support to ARM > platforms is in, the change you linked may be palatable for the > upstream. However, ARM platforms could run into the infinite loop I > outlined if that change is pushed upstream because of the lack of > per-allocated page tables and a software semaphore to prevent looping. > I still don't see how this happens. There is an ASSERT in CoreInitializeMemoryProtection() to ensure that EfiConventialMemory and EfiBootServicesData have the same memory type, and I added that (in commit 7eb927db3e25a) for precisely this reason, i.e., to ensure that the plumbing of this feature wouldn't recurse. Could this be related to heap guard, perhaps? I could see how changing the boundaries of allocations might trigger a split even if the old and new type should have the exact same type, and perhaps we should use unguarded pages for page tables. > I implemented pre-allocated pages for ARM a while back in a private repo > but never committed it to Mu. Maybe that would also be worth committing > and pushing upstream. > I'd like to understand better whether or not there is a way to avoid the need for this, but if not, I'd be happy to review your solution. Does the issue only exist on ARM? Does it still happen after I rewrote the MMU library? (in 2020) Thanks, Ard. > On 2/3/2023 11:58 AM, Marvin H=C3=A4user wrote: > > Hi Taylor, > > > > Do you by any chance mean this bug? > > https://github.com/microsoft/mu_basecore/blob/release/202208/MdeModuleP= kg/Core/Dxe/Misc/MemoryProtection.c#L1544 > > > > I reported this a while ago at > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D3316 > > > > > > The Mu fix is by no means a workaround and actually fixes this issue in > > a sane way. It should have been fixed upstream ages ago. > > > > Best regards, > > Marvin > > > > > >=20 > >