From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.8720.1685519640542157657 for ; Wed, 31 May 2023 00:54:00 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=pbqjZKGV; spf=pass (domain: kernel.org, ip: 139.178.84.217, 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 dfw.source.kernel.org (Postfix) with ESMTPS id 128A663526 for ; Wed, 31 May 2023 07:54:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 77F74C433D2 for ; Wed, 31 May 2023 07:53:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685519639; bh=Ra+pJHzlNy0H9WtLivUpL1z/vr4lvmov92gUnZ1tA0U=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=pbqjZKGVJs+1hsQaNvDOJF+gnj/B+y3yQlkjhZ/M04BoPBlJWwqc402nzs6Yyq1e8 FI6mGC3mmJG7AAAIiIAncfYlWVfUHexkATQUSrIhqwqG+9ceUJonuD5ofE3swJuUPV r395CHaLVW6wwN1XAio+gO/EOy8e8r0ZyIFj5yKD8Tydx4Lh0OcX60EX8FtM/ROi8a tYMsJmTNv0EyPdG7GhhIO2ysBL8ymg7YMQroWmjlYpVYNTHk0T4MjSUfE2D+oiwCsa pNGlzGEKT3gvkzzkmBHB6fHyYYun0BOoZ+m651b0/UB6QD7LrC6X6q3+FYup7/g/14 ai+OXbUKER6tA== Received: by mail-lj1-f180.google.com with SMTP id 38308e7fff4ca-2af2d092d7aso58478071fa.2 for ; Wed, 31 May 2023 00:53:59 -0700 (PDT) X-Gm-Message-State: AC+VfDx7vUql6uyVEkUvNifrn5r+rW5+K1LcSPchMfhRen62Ag8E+7/Y i+F4uuH6rgQ2u2lSiZMpUx0LLrfjIVC5HHvo96A= X-Google-Smtp-Source: ACHHUZ6otPug6JWZTmTICu6nS9JzbUn6KkQ+ZlFyE2uNaQjuLW67vyg1sbwVV87lsFQOuEhLrV6oHF53DogwpoRRwaI= X-Received: by 2002:a2e:b00c:0:b0:2ad:8a4b:6a9e with SMTP id y12-20020a2eb00c000000b002ad8a4b6a9emr2019434ljk.26.1685519637522; Wed, 31 May 2023 00:53:57 -0700 (PDT) MIME-Version: 1.0 References: <20230525143041.1172989-1-ardb@kernel.org> <20230525143041.1172989-6-ardb@kernel.org> In-Reply-To: From: "Ard Biesheuvel" Date: Wed, 31 May 2023 09:53:45 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH 05/10] MdeModulePkg: Define memory attribute PPI To: "Ni, Ray" Cc: "devel@edk2.groups.io" , "Yao, Jiewen" , Gerd Hoffmann , Taylor Beebe , Oliver Smith-Denny , "Bi, Dandan" , "Gao, Liming" , "Kinney, Michael D" , Leif Lindholm , Sunil V L , "Warkentin, Andrei" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 31 May 2023 at 09:34, Ni, Ray wrote: > > > > > -----Original Message----- > > From: Ard Biesheuvel > > Sent: Tuesday, May 30, 2023 3:32 PM > > To: Ni, Ray > > Cc: devel@edk2.groups.io; Yao, Jiewen ; Gerd > > Hoffmann ; Taylor Beebe ; Oliver > > Smith-Denny ; Bi, Dandan ; > > Gao, Liming ; Kinney, Michael D > > ; Leif Lindholm = ; > > Sunil V L ; Warkentin, Andrei > > > > Subject: Re: [RFC PATCH 05/10] MdeModulePkg: Define memory attribute PP= I > > > > On Tue, 30 May 2023 at 09:15, Ni, Ray wrote: > > > > > > 1. > > > The PPI interface supports to set and clear any attributes with singl= e > > invocation. > > > That's much better than the existing UEFI protocol prototype which re= quires > > caller to call the interfaces > > > twice to set and clear some attributes. > > > > > > > Agree, I think that was a mistake to define the UEFI memory attributes > > protocol like that, as it requires two traversals of the page tables > > for the most common case of converting RO -> XP or vice versa. > > > > > So far I see two patterns for attributes setting: > > > *. The patten in this patch: SetMask/ClearMask > > > *. The pattern I used in PageTableLib: Attribute/Mask. > > > > > > I think from caller side, they provide the same capabilities. > > > The difference is SetMask/ClearMask expects callers to not set the sa= me BIT > > in both > > > SetMask/ClearMask. > > > > > > (I thought there would be similar existing interfaces as pattern 2. B= ut I didn't > > find any now.) > > > Do you mind to align to pattern #2? > > > > > > > That is fine - I actually prefer that (and this is what ArmMmuLib > > implements internally) but I did not want to deviate from the UEFI > > protocol too much. > > By adding "ClearMask", you already made something different=F0=9F=98=8A > Good to know you prefer pattern #2. > Yeah :-) > > > > > > > > 2. > > > When a memory region is marked from not-present to present, PageTable= Lib > > expects > > > caller to supply all memory attributes (including RW, NX, etc.) as th= e lib > > implementation doesn't > > > want to carry any default attributes.. > > > Do you think the MemoryAttribute PPI should expect the same to caller= ? > > > > > > > I'm not sure I follow. > > > > The PPI (as well as the UEFI protocol) can only operate on valid > > mapping, and can only be used to manipulate RP/RO/XP. It cannot be > > used to create mappings from scratch. > When a range of memory is marked as "RP", X86 page table clears the > "Present" bit for that range memory. > "Present" bit is a master bit in X86 page table. When that bit is clear, = all > other bits ("Writable", "Non-Execution", etc.) are ignored by CPU. > > So, if caller clears the "RP" bit (setting "Present" bit in page table), = that's an > operation to map back some memory. > X86 CpuPageTableLib requires all attributes be provided for mapping back > some memory. > > > > > Do you think this capability should be added? If so, I think it is > > reasonable to require the caller to provide all attributes, and on ARM > > this would have to include the memory cacheability type as we should > > not provide a default for that either. > > Yes. I think this is required. Having this rule can help caller write rob= ust code > instead of depending on some default attributes in PPI/Protocol implement= ation. > I still don't follow. How does that work in the context of the attribute mask? Can you given some examples? Creating new memory mappings from scratch is a totally different use case, so perhaps this should be a separate PPI method.