On 06/26, Dan Williams wrote:
> On Mon, Jun 26, 2017 at 12:31 PM, Vishal Verma <[email protected]> 
> wrote:
> > On 06/25, Dan Williams wrote:
> >> On Fri, Jun 23, 2017 at 7:17 PM, Vishal Verma <[email protected]> 
> >> wrote:
> >> > The UEFI 2.7 specification defines an updated BTT metadata format,
> >> > bumping the revision to 2.0. Add support for the new format, while
> >> > retaining compatibility for the old 1.1 format.
> >> >
> >> > New BTTs will be created using the newest (2.0 as of this writing)
> >> > format.
> >> >
> >> > Cc: Toshi Kani <[email protected]>
> >> > Cc: Linda Knippers <[email protected]>
> >> > Cc: Dan Williams <[email protected]>
> >> > Signed-off-by: Vishal Verma <[email protected]>
> >> > ---
> >> >  drivers/nvdimm/btt.c      | 28 ++++++++++++++++++++--------
> >> >  drivers/nvdimm/btt.h      |  7 +++++++
> >> >  drivers/nvdimm/btt_devs.c | 44 
> >> > +++++++++++++++++++++++++++++++++++++++-----
> >> >  drivers/nvdimm/nd.h       |  1 +
> >> >  4 files changed, 67 insertions(+), 13 deletions(-)
> >> >
> >> > diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> >> > index 983718b..629c376 100644
> >> > --- a/drivers/nvdimm/btt.c
> >> > +++ b/drivers/nvdimm/btt.c
> >> [..]
> >> > @@ -576,8 +576,10 @@ static struct arena_info *alloc_arena(struct btt 
> >> > *btt, size_t size,
> >> >         arena->internal_lbasize = roundup(arena->external_lbasize,
> >> >                                         INT_LBASIZE_ALIGNMENT);
> >> >         arena->nfree = BTT_DEFAULT_NFREE;
> >> > -       arena->version_major = 1;
> >> > -       arena->version_minor = 1;
> >> > +
> >> > +       /* New BTTs will always be v2.0 */
> >> > +       arena->version_major = 2;
> >> > +       arena->version_minor = 0;
> >>
> >> I don't think this is correct. The v2.0 format is generally more
> >> dangerous than v1.1 because its info block can coexist with other
> >> nvdimm info-blocks and fs-super-blocks. So we should always default to
> >> v1.1 unless explicitly forced into v2.0 mode with the claim-class /
> >> address-abstraction-guid set in the v1.2 namespace label. Without that
> >> indicator to tie break ambiguous situations we can't use the v2.0
> >> format which is strictly for inter-OS / UEFI compatibility.
> >
> > Hm, I tend to agree, but in that case, shouldn't the holder class for
> > BTTs automatically default to BTT instead of NONE, i.e. add the address
> > abstraction guid by default, thus creating v2 BTTs. I feel it might come
> > as more of a surprise if we continue to create v1.1 BTTs by default when
> > the spec states 2.0..
> 
> That default is up to userspace. If an environment wants to use BTT v2
> it needs to use updated tooling that sets the holder class. The kernel
> default should remain as 'none' because it can't tell if BTT v2 or v1
> is intended even when using v1.2 namespaces. The spec pretends that we
> haven't been shipping this support in the Linux kernel for years, so I
> think it's more important to be backwards compatible by default.

Sounds good, mechanism, not policy :)
I'll send out a v2 with these updates.
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to