Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220127175505.851391-1-ira.weiny@intel.com> <20220127175505.851391-37-ira.weiny@intel.com>
 <2193142f0cf3785a4225e0393eace397cbbe86e6.camel@intel.com>
In-Reply-To: <2193142f0cf3785a4225e0393eace397cbbe86e6.camel@intel.com>
From:   Dan Williams <dan.j.williams@intel.com>
Date:   Fri, 4 Feb 2022 09:12:11 -0800
Message-ID: <CAPcyv4i_Jc865zEzNxbQB_XHqCwSS6zm_evquLv2BBu9ipa39Q@mail.gmail.com>
Subject: Re: [PATCH V8 36/44] memremap_pages: Reserve a PKS PKey for eventual
 use by PMEM
To:     "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc:     "hpa@zytor.com" <hpa@zytor.com>,
        "Weiny, Ira" <ira.weiny@intel.com>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "Yu, Fenghua" <fenghua.yu@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Feb 1, 2022 at 10:35 AM Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
>
> On Thu, 2022-01-27 at 09:54 -0800, ira.weiny@intel.com wrote:
> >  enum pks_pkey_consumers {
> > -       PKS_KEY_DEFAULT         = 0, /* Must be 0 for default PTE
> > values */
> > -       PKS_KEY_TEST            = 1,
> > -       PKS_KEY_NR_CONSUMERS    = 2,
> > +       PKS_KEY_DEFAULT                 = 0, /* Must be 0 for default
> > PTE values */
> > +       PKS_KEY_TEST                    = 1,
> > +       PKS_KEY_PGMAP_PROTECTION        = 2,
> > +       PKS_KEY_NR_CONSUMERS            = 3,
> >  };
>
> The c spec says that any enum member that doesn't have an "=" will be
> one more than the previous member. As a consequence you can leave the
> "=" off PKS_KEY_NR_CONSUMERS and it will get auto adjusted when you add
> more like this.
>
> I know we've gone around and around on this, but why also specify the
> value for each key? They should auto increment and the first one is
> guaranteed to be zero.
>
> Otherwise this doesn't use any of the features of "enum", it's just a
> verbose series of const int's.

Going further, this can also build in support for dynamically (at
build time) freeing keys based on config, something like:

enum {
#if IS_ENABLED(CONFIG_PKS_TEST)
PKS_KEY_TEST,
#endif
#if IS_ENABLED(CONFIG_DEVMAP_PROTECTION)
PKS_KEY_PGMAP_PROTECTION,
#endif
PKS_KEY_NR_CONSUMERS,
}