Raising the UBI version

Dear MTD folks,

For the emerging MLC NAND support we need to change the UBI on-flash format. Of course existing UBI images will keep working and remain fully supported. Our approach to deal with MLC (and basically TLC) NAND is LEB consolidation. In this operation mode a single PEB can host multiple LEBs. In the MLC case 2, for TLC 3. For more details please refer to my announcement[0]. Hosting multiple LEBs in a single PEB means that beside of a single EC header, a PEB will carry multiple VID headers, one for each LEB it contains. This change needs to be annotated in the EC header.

Both EC and VID headers have a version field. Currently it is set to 1. Our original plan was just raising UBI_VERSION to 2, and, of course, accept version 1 image as well. The first hassle was that UBI_VERSION is exported in /sys/class/ubi/version and libubi refuses to work if the version is not 1. Breaking existing userspace tools is not acceptable, so we need another approach.

LEB consolidation is not really a completely new UBI implementation, it is an addon feature. So we came up with the idea of having feature flags in the EC header. Maybe we need later more flags, who knows?

Boris and I sat down and came up with two possible ways to implement such flags:

i) Rename ->version in EC and VID headers to ->features. ->features will be evaluated at attach time and UBI has to figure whether it supports all request features. The field is one byte long, therefore we can encode 8 features.
As starter two features would be supported:

      UBI_FEAT_BASE	= 1
      UBI_FEAT_CONSO	= 2

That means regular UBI images on SLC would only have set UBI_FEAT_BASE and nothing else. Existing UBI implementations would see ->features with UBI_FEAT_BASE set as ->version = 1, so we're safe. On MLC NAND we'd set UBI_FEAT_BASE and UBI_FEAT_CONSO which would be seen as ->version = 3 and rejected by UBI implementations which do not support LEB consolidation. To not break userspace tools /sys/class/ubi/version would be hardcoded to 1 and the ->features field exported in /sys/class/ubi/features and /sys/class/ubi/ubiX/features_used. The features sysfs file denotes what features this UBI implementation supports and features_used shows what features the attached UBI image requested. If we change the UBI on-flash format in a major way, UBI_FEAT_BASE would not be set.

ii) Keep ->version in EC and VID headers and use padding bytes from both headers
to add a new ->features field. If ->version is 1, ->features will remain 0 and not evaluated. If ->features should be evaluated, ->version will be 2. So, on MLC NAND ->version will be 2 and ->features has UBI_FEAT_CONSO set. This approach is less complicated but we have to claim padding bytes. Of course we also have to hardcode /sys/class/ubi/version to 1 too and having a features file in sysfs.

That said, we'd keep ->version and ->features between EC and VID headers in sync. Supporting different versions of EC and VID headers at the same time would complicate the code a lot.

Another thing to think about is storing the number of LEBs a PEB can contain in the EC header. Usually this would be determined at runtime but maybe it makes sense to store it also on-flash to detect misconfiguration, like we do already for VID offset in the EC header.

What do you think? I'm happy with either i) or ii), the biggest concern we have is that we cut something into stone and break stuff or make trouble unavoidable.

Thanks,
//richard

[0] http://lists.infradead.org/pipermail/linux-mtd/2016-April/067322.html

Quoted 44 lines · On Tue, 21 Jun 2016 21:19:50 +0200 Richard Weinberger <[email protected]> wrote:

Dear MTD folks,

For the emerging MLC NAND support we need to change the UBI on-flash format. Of course existing UBI images will keep working and remain fully supported. Our approach to deal with MLC (and basically TLC) NAND is LEB consolidation. In this operation mode a single PEB can host multiple LEBs. In the MLC case 2, for TLC 3. For more details please refer to my announcement[0]. Hosting multiple LEBs in a single PEB means that beside of a single EC header, a PEB will carry multiple VID headers, one for each LEB it contains. This change needs to be annotated in the EC header.

Both EC and VID headers have a version field. Currently it is set to 1. Our original plan was just raising UBI_VERSION to 2, and, of course, accept version 1 image as well. The first hassle was that UBI_VERSION is exported in /sys/class/ubi/version and libubi refuses to work if the version is not 1. Breaking existing userspace tools is not acceptable, so we need another approach.

LEB consolidation is not really a completely new UBI implementation, it is an addon feature. So we came up with the idea of having feature flags in the EC header. Maybe we need later more flags, who knows?

Boris and I sat down and came up with two possible ways to implement such flags:

i) Rename ->version in EC and VID headers to ->features. ->features will be evaluated at attach time and UBI has to figure whether it supports all request features. The field is one byte long, therefore we can encode 8 features.
As starter two features would be supported:
UBI_FEAT_BASE = 1
UBI_FEAT_CONSO = 2
That means regular UBI images on SLC would only have set UBI_FEAT_BASE and nothing else. Existing UBI implementations would see ->features with UBI_FEAT_BASE set as ->version = 1, so we're safe. On MLC NAND we'd set UBI_FEAT_BASE and UBI_FEAT_CONSO which would be seen as ->version = 3 and rejected by UBI implementations which do not support LEB consolidation. To not break userspace tools /sys/class/ubi/version would be hardcoded to 1 and the ->features field exported in /sys/class/ubi/features and /sys/class/ubi/ubiX/features_used. The features sysfs file denotes what features this UBI implementation supports and features_used shows what features the attached UBI image requested. If we change the UBI on-flash format in a major way, UBI_FEAT_BASE would not be set.

Quoted 7 lines

ii) Keep ->version in EC and VID headers and use padding bytes from both headers
to add a new ->features field. If ->version is 1, ->features will remain 0 and not evaluated. If ->features should be evaluated, ->version will be 2. So, on MLC NAND ->version will be 2 and ->features has UBI_FEAT_CONSO set. This approach is less complicated but we have to claim padding bytes. Of course we also have to hardcode /sys/class/ubi/version to 1 too and having a features file in sysfs.

Why do we need to hardcode /sys/class/ubi/version to 1? We just need to update the mtd-utils to support version 2. Am I missing something?

Quoted 55 lines · Boris Brezillon

Quoted 51 lines · On Tue, 21 Jun 2016 21:19:50 +0200 Richard Weinberger <[email protected]> wrote:

Dear MTD folks,

For the emerging MLC NAND support we need to change the UBI on-flash format. Of course existing UBI images will keep working and remain fully supported. Our approach to deal with MLC (and basically TLC) NAND is LEB consolidation. In this operation mode a single PEB can host multiple LEBs. In the MLC case 2, for TLC 3. For more details please refer to my announcement[0]. Hosting multiple LEBs in a single PEB means that beside of a single EC header, a PEB will carry multiple VID headers, one for each LEB it contains. This change needs to be annotated in the EC header.

Both EC and VID headers have a version field. Currently it is set to 1. Our original plan was just raising UBI_VERSION to 2, and, of course, accept version 1 image as well. The first hassle was that UBI_VERSION is exported in /sys/class/ubi/version and libubi refuses to work if the version is not 1. Breaking existing userspace tools is not acceptable, so we need another approach.

LEB consolidation is not really a completely new UBI implementation, it is an addon feature. So we came up with the idea of having feature flags in the EC header. Maybe we need later more flags, who knows?

Boris and I sat down and came up with two possible ways to implement such flags:

i) Rename ->version in EC and VID headers to ->features. ->features will be evaluated at attach time and UBI has to figure whether it supports all request features. The field is one byte long, therefore we can encode 8 features.
As starter two features would be supported:
UBI_FEAT_BASE = 1
UBI_FEAT_CONSO = 2
That means regular UBI images on SLC would only have set UBI_FEAT_BASE and nothing else. Existing UBI implementations would see ->features with UBI_FEAT_BASE set as ->version = 1, so we're safe. On MLC NAND we'd set UBI_FEAT_BASE and UBI_FEAT_CONSO which would be seen as ->version = 3 and rejected by UBI implementations which do not support LEB consolidation. To not break userspace tools /sys/class/ubi/version would be hardcoded to 1 and the ->features field exported in /sys/class/ubi/features and /sys/class/ubi/ubiX/features_used. The features sysfs file denotes what features this UBI implementation supports and features_used shows what features the attached UBI image requested. If we change the UBI on-flash format in a major way, UBI_FEAT_BASE would not be set.

ii) Keep ->version in EC and VID headers and use padding bytes from both headers
to add a new ->features field. If ->version is 1, ->features will remain 0 and not evaluated. If ->features should be evaluated, ->version will be 2. So, on MLC NAND ->version will be 2 and ->features has UBI_FEAT_CONSO set. This approach is less complicated but we have to claim padding bytes. Of course we also have to hardcode /sys/class/ubi/version to 1 too and having a features file in sysfs.

Quoted 2 lines

Why do we need to hardcode /sys/class/ubi/version to 1? We just need to update the mtd-utils to support version 2. Am I missing something?

Is some code change required in mtd-utils other than the change in version check?

If not why force everyone to patch their mtd-utils to support doing the same thing when the file reads back 2 instead of 1?

Quoted 57 lines · On Wed, 22 Jun 2016 15:09:59 +0200 Michal Suchanek <[email protected]> wrote:

Quoted 55 lines · Boris Brezillon

Quoted 51 lines · On Tue, 21 Jun 2016 21:19:50 +0200 Richard Weinberger <[email protected]> wrote:

Dear MTD folks,

For the emerging MLC NAND support we need to change the UBI on-flash format. Of course existing UBI images will keep working and remain fully supported. Our approach to deal with MLC (and basically TLC) NAND is LEB consolidation. In this operation mode a single PEB can host multiple LEBs. In the MLC case 2, for TLC 3. For more details please refer to my announcement[0]. Hosting multiple LEBs in a single PEB means that beside of a single EC header, a PEB will carry multiple VID headers, one for each LEB it contains. This change needs to be annotated in the EC header.

Both EC and VID headers have a version field. Currently it is set to 1. Our original plan was just raising UBI_VERSION to 2, and, of course, accept version 1 image as well. The first hassle was that UBI_VERSION is exported in /sys/class/ubi/version and libubi refuses to work if the version is not 1. Breaking existing userspace tools is not acceptable, so we need another approach.

LEB consolidation is not really a completely new UBI implementation, it is an addon feature. So we came up with the idea of having feature flags in the EC header. Maybe we need later more flags, who knows?

Boris and I sat down and came up with two possible ways to implement such flags:

i) Rename ->version in EC and VID headers to ->features. ->features will be evaluated at attach time and UBI has to figure whether it supports all request features. The field is one byte long, therefore we can encode 8 features.
As starter two features would be supported:
UBI_FEAT_BASE = 1
UBI_FEAT_CONSO = 2
That means regular UBI images on SLC would only have set UBI_FEAT_BASE and nothing else. Existing UBI implementations would see ->features with UBI_FEAT_BASE set as ->version = 1, so we're safe. On MLC NAND we'd set UBI_FEAT_BASE and UBI_FEAT_CONSO which would be seen as ->version = 3 and rejected by UBI implementations which do not support LEB consolidation. To not break userspace tools /sys/class/ubi/version would be hardcoded to 1 and the ->features field exported in /sys/class/ubi/features and /sys/class/ubi/ubiX/features_used. The features sysfs file denotes what features this UBI implementation supports and features_used shows what features the attached UBI image requested. If we change the UBI on-flash format in a major way, UBI_FEAT_BASE would not be set.

ii) Keep ->version in EC and VID headers and use padding bytes from both headers
to add a new ->features field. If ->version is 1, ->features will remain 0 and not evaluated. If ->features should be evaluated, ->version will be 2. So, on MLC NAND ->version will be 2 and ->features has UBI_FEAT_CONSO set. This approach is less complicated but we have to claim padding bytes. Of course we also have to hardcode /sys/class/ubi/version to 1 too and having a features file in sysfs.

Quoted 5 lines

Quoted 2 lines

Why do we need to hardcode /sys/class/ubi/version to 1? We just need to update the mtd-utils to support version 2. Am I missing something?

Is some code change required in mtd-utils other than the change in version check?

Quoted 3 lines

If not why force everyone to patch their mtd-utils to support doing the same thing when the file reads back 2 instead of 1?

Old UBI users will keep exposing version 1. Version 2 will only be used for those wanting to enable support for UBI consolidation, so old mtd-utils should keep working, unless the user explicitly tried to create a consolidated UBI image, or attached the UBI layer to an empty partition on an MLC NAND after enabling consolidation support. That's rather unlikely, and if it happens it's the end-user fault.

Why do we need to hardcode /sys/class/ubi/version to 1? We just need to update the mtd-utils to support version 2. Am I missing something?

Am 22.06.2016 um 14:43 schrieb Boris Brezillon:

Quoted 2 lines

Why do we need to hardcode /sys/class/ubi/version to 1? We just need to update the mtd-utils to support version 2. Am I missing something?

We don't want to break existing userspace.
Why should ubimkvol or ubiattach fail on a system with SLC NAND and CONFIG_MTD_UBI_CONSOLIDATE=y?

Especially since existing tools *will* work with CONFIG_MTD_UBI_CONSOLIDATE=y. Rasing /sys/class/ubi/version and breaking existing tools is only acceptable when we change all UBI ioctl() and sysfs files in a way such that version 1 userspace cannot work. Which is not the case here.

This is a nice example why version numbers is bad and feature flags should be used. Currently UBI mixes the implementation version and the on-flash version. We're changing only the on-flash version. The user visible ABI stays and will only get extended.

Quoted 4 lines · On Wed, 22 Jun 2016 15:54:34 +0200 Richard Weinberger <[email protected]> wrote:

Am 22.06.2016 um 14:43 schrieb Boris Brezillon:

Quoted 2 lines

Why do we need to hardcode /sys/class/ubi/version to 1? We just need to update the mtd-utils to support version 2. Am I missing something?

Quoted 3 lines

We don't want to break existing userspace.
Why should ubimkvol or ubiattach fail on a system with SLC NAND and CONFIG_MTD_UBI_CONSOLIDATE=y?

But version won't be set to 2 in this case. If it's an SLC NAND we don't need to use UBI version 2, do we.

Am 22.06.2016 um 16:01 schrieb Boris Brezillon:

Quoted 11 lines

Quoted 7 lines · On Wed, 22 Jun 2016 15:54:34 +0200 Richard Weinberger <[email protected]> wrote:

Am 22.06.2016 um 14:43 schrieb Boris Brezillon:

Quoted 2 lines

Why do we need to hardcode /sys/class/ubi/version to 1? We just need to update the mtd-utils to support version 2. Am I missing something?

We don't want to break existing userspace.
Why should ubimkvol or ubiattach fail on a system with SLC NAND and CONFIG_MTD_UBI_CONSOLIDATE=y?

Quoted 2 lines

But version won't be set to 2 in this case. If it's an SLC NAND we don't need to use UBI version 2, do we.

/sys/class/ubi/version is the version of the UBI implementation, not the version of the attached UBI image.
It will the here as soon you load the UBI module.

Quoted 3 lines

/sys/class/ubi/version is the version of the UBI implementation, not the version of the attached UBI image.
It will the here as soon you load the UBI module.

Do we have /sys/class/ubi/ubiX/version for the UBI image version?

This is still unclear to me why we need to version the user-space/kernel-space ABI, since it's supposed to be backward compatible, so adding new features requires adding new ioctls and keeping the old ones in a working state.

What is /sys/class/ubi/version actually encoding? Isn't it encoding the fact that a specific UBI implementation is supporting all UBI on-flash formats up to format version X (that was my understanding)?

Am 22.06.2016 um 16:13 schrieb Boris Brezillon:

Quoted 5 lines

Quoted 3 lines

/sys/class/ubi/version is the version of the UBI implementation, not the version of the attached UBI image.
It will the here as soon you load the UBI module.

Do we have /sys/class/ubi/ubiX/version for the UBI image version?

No. That's why I plan to add /sys/class/ubi/ubiX/features_used to show which features the attached UBI image requested.
And having a /sys/class/ubi/ubi/features which denotes what features the _implementation_ supports.

Quoted 6 lines

This is still unclear to me why we need to version the user-space/kernel-space ABI, since it's supposed to be backward compatible, so adding new features requires adding new ioctls and keeping the old ones in a working state.

Quoted 4 lines

What is /sys/class/ubi/version actually encoding? Isn't it encoding the fact that a specific UBI implementation is supporting all UBI on-flash formats up to format version X (that was my understanding)?

Well, /sys/class/ubi/version exports UBI_VERSION from ubi-media.h. It is (ab)used to encode the ABI version *and* the on-flash version.

The problem is that mtd-utils libubi will refuse to work with /sys/class/ubi/version unequal 1.

Here the gem from libubi:

        if (read_positive_int(lib->ubi_version, &version))
                goto out_error;
        if (version != LIBUBI_UBI_VERSION) {
                errmsg("this library was made for UBI version %d, but UBI "
                       "version %d is detected\n", LIBUBI_UBI_VERSION, version);
                goto out_error;
        }

This is why I want to hardcode it to 1.
Everything else will break existing user space in some way. 10 years ago /sys/class/ubi/version seemed like a good idea but now it hits us hard.

Quoted 11 lines · On Wed, 22 Jun 2016 16:21:36 +0200 Richard Weinberger <[email protected]> wrote:

Am 22.06.2016 um 16:13 schrieb Boris Brezillon:

Quoted 5 lines

Quoted 3 lines

/sys/class/ubi/version is the version of the UBI implementation, not the version of the attached UBI image.
It will the here as soon you load the UBI module.

Do we have /sys/class/ubi/ubiX/version for the UBI image version?

No. That's why I plan to add /sys/class/ubi/ubiX/features_used to show which features the attached UBI image requested.
And having a /sys/class/ubi/ubi/features which denotes what features the _implementation_ supports.

Still the version and features are encoding different things IMO. Incrementing the on-flash version means that the on-flash format has changed in an incompatible way, while features denotes the fact that the existing format has been extended with new features but is backward compatible.

Quoted 6 lines

Quoted 6 lines

This is still unclear to me why we need to version the user-space/kernel-space ABI, since it's supposed to be backward compatible, so adding new features requires adding new ioctls and keeping the old ones in a working state.

Quoted 7 lines

Quoted 4 lines

What is /sys/class/ubi/version actually encoding? Isn't it encoding the fact that a specific UBI implementation is supporting all UBI on-flash formats up to format version X (that was my understanding)?

Well, /sys/class/ubi/version exports UBI_VERSION from ubi-media.h. It is (ab)used to encode the ABI version *and* the on-flash version.

The on-flash versions supported by the implementation is a useful information.

Quoted 11 lines

The problem is that mtd-utils libubi will refuse to work with /sys/class/ubi/version unequal 1.

Here the gem from libubi:

        if (read_positive_int(lib->ubi_version, &version))
                goto out_error;
        if (version != LIBUBI_UBI_VERSION) {
                errmsg("this library was made for UBI version %d, but UBI "
                       "version %d is detected\n", LIBUBI_UBI_VERSION, version);
                goto out_error;
        }

And this is where the problem is: libubi does not make proper use of this information. We should either have

if (version >= LIBUBI_UBI_VERSION)

or, if we decide that version is a bitfield directly encoding which versions are supported by the implementation

#define VERSION_SUPPORTED(version, x) ((version) & BIT((x)-1)))

if (VERSION_SUPPORTED(version, 1))

Quoted 5 lines

This is why I want to hardcode it to 1.
Everything else will break existing user space in some way. 10 years ago /sys/class/ubi/version seemed like a good idea but now it hits us hard.

Yes, I understand that, but this also means /sys/class/ubi/version is just a dummy file which only purpose is to make libubi happy :).

If this is the case, then I think we should have another file encoding the supported on-flash formats...

Am 22.06.2016 um 16:39 schrieb Boris Brezillon:

Quoted 15 lines

Quoted 11 lines · On Wed, 22 Jun 2016 16:21:36 +0200 Richard Weinberger <[email protected]> wrote:

Am 22.06.2016 um 16:13 schrieb Boris Brezillon:

Quoted 5 lines

Quoted 3 lines

/sys/class/ubi/version is the version of the UBI implementation, not the version of the attached UBI image.
It will the here as soon you load the UBI module.

Do we have /sys/class/ubi/ubiX/version for the UBI image version?

No. That's why I plan to add /sys/class/ubi/ubiX/features_used to show which features the attached UBI image requested.
And having a /sys/class/ubi/ubi/features which denotes what features the _implementation_ supports.

Quoted 5 lines

Still the version and features are encoding different things IMO. Incrementing the on-flash version means that the on-flash format has changed in an incompatible way, while features denotes the fact that the existing format has been extended with new features but is backward compatible.

Yes. But now we have a mix of both. ;-\

Quoted 32 lines

Quoted 6 lines

Quoted 6 lines

This is still unclear to me why we need to version the user-space/kernel-space ABI, since it's supposed to be backward compatible, so adding new features requires adding new ioctls and keeping the old ones in a working state.

Quoted 7 lines

Quoted 4 lines

What is /sys/class/ubi/version actually encoding? Isn't it encoding the fact that a specific UBI implementation is supporting all UBI on-flash formats up to format version X (that was my understanding)?

Well, /sys/class/ubi/version exports UBI_VERSION from ubi-media.h. It is (ab)used to encode the ABI version *and* the on-flash version.

The on-flash versions supported by the implementation is a useful information.

Quoted 11 lines

The problem is that mtd-utils libubi will refuse to work with /sys/class/ubi/version unequal 1.

Here the gem from libubi:

        if (read_positive_int(lib->ubi_version, &version))
                goto out_error;
        if (version != LIBUBI_UBI_VERSION) {
                errmsg("this library was made for UBI version %d, but UBI "
                       "version %d is detected\n", LIBUBI_UBI_VERSION, version);
                goto out_error;
        }  

And this is where the problem is: libubi does not make proper use of this information. We should either have

Quoted 8 lines

if (version >= LIBUBI_UBI_VERSION)

or, if we decide that version is a bitfield directly encoding which versions are supported by the implementation

#define VERSION_SUPPORTED(version, x) ((version) & BIT((x)-1)))

if (VERSION_SUPPORTED(version, 1))

Yep. But we cannot change already compiled and shipped code.

Quoted 8 lines

Quoted 5 lines

This is why I want to hardcode it to 1.
Everything else will break existing user space in some way. 10 years ago /sys/class/ubi/version seemed like a good idea but now it hits us hard.

Yes, I understand that, but this also means /sys/class/ubi/version is just a dummy file which only purpose is to make libubi happy :).

That's the plan.

Quoted 2 lines

If this is the case, then I think we should have another file encoding the supported on-flash formats...

This is what /sys/class/ubi/features was supposed to do.

So how about defining the following:
- /sys/class/ubi/version: user-space ABI version (should always be one)

- /sys/class/ubi/supported-on-flash-formats: either a bitfield or an integer representing the higher on-flash format version supported by the implementation (which implies that implementations have to support all on-flash formats up-to supported-on-flash-formats)

- /sys/class/ubi/supported-features: the features supported by the implementation (each bit is a specific feature or a set of features). The features may or may not be version dependent.
- /sys/class/ubi/ubiX/on-flash-format: the on-flash format used on the UBIX device
- /sys/class/ubi/ubiX/features: the features exposed by this UBIX device

Am 22.06.2016 um 16:52 schrieb Boris Brezillon:

Quoted 2 lines

So how about defining the following:
- /sys/class/ubi/version: user-space ABI version (should always be one)

Yes. To not break libubi.

Quoted 4 lines

- /sys/class/ubi/supported-on-flash-formats: either a bitfield or an integer representing the higher on-flash format version supported by the implementation (which implies that implementations have to support all on-flash formats up-to supported-on-flash-formats)

We can do that. But who will evaluate this file?

Quoted 8 lines

- /sys/class/ubi/supported-features: the features supported by the implementation (each bit is a specific feature or a set of features). The features may or may not be version dependent.
- /sys/class/ubi/ubiX/on-flash-format: the on-flash format used on the UBIX device
- /sys/class/ubi/ubiX/features: the features exposed by this UBIX device

I think we should make features depend on version = 2.
IOW when we change the UBI format in a major way version will be 3 and we maybe use something else to expose features.

Quoted 11 lines · On Wed, 22 Jun 2016 16:59:50 +0200 Richard Weinberger <[email protected]> wrote:

Am 22.06.2016 um 16:52 schrieb Boris Brezillon:

Quoted 2 lines

So how about defining the following:
- /sys/class/ubi/version: user-space ABI version (should always be one)

Yes. To not break libubi.

Quoted 4 lines

- /sys/class/ubi/supported-on-flash-formats: either a bitfield or an integer representing the higher on-flash format version supported by the implementation (which implies that implementations have to support all on-flash formats up-to supported-on-flash-formats)

Quoted 1 line

We can do that. But who will evaluate this file?

ubiformat may need that one to check if the provided ubi image is supported by the implementation.

Quoted 9 lines

Quoted 8 lines

- /sys/class/ubi/supported-features: the features supported by the implementation (each bit is a specific feature or a set of features). The features may or may not be version dependent.
- /sys/class/ubi/ubiX/on-flash-format: the on-flash format used on the UBIX device
- /sys/class/ubi/ubiX/features: the features exposed by this UBIX device

Quoted 3 lines

I think we should make features depend on version = 2.
IOW when we change the UBI format in a major way version will be 3 and we maybe use something else to expose features.

IIUC, we move to version 2 now, because we're using one of the padding byte (word?) to expose the feature flags, correct?
It makes sense.

Am 22.06.2016 um 17:06 schrieb Boris Brezillon:

Quoted 14 lines

Quoted 11 lines · On Wed, 22 Jun 2016 16:59:50 +0200 Richard Weinberger <[email protected]> wrote:

Am 22.06.2016 um 16:52 schrieb Boris Brezillon:

Quoted 2 lines

So how about defining the following:
- /sys/class/ubi/version: user-space ABI version (should always be one)

Yes. To not break libubi.

Quoted 4 lines

- /sys/class/ubi/supported-on-flash-formats: either a bitfield or an integer representing the higher on-flash format version supported by the implementation (which implies that implementations have to support all on-flash formats up-to supported-on-flash-formats)

Quoted 4 lines

Quoted 1 line

We can do that. But who will evaluate this file?

ubiformat may need that one to check if the provided ubi image is supported by the implementation.

What if someone wants to ubiformat (with MLC support) his mtd0 on an old kernel and then reboots into the new one with MLC support?
I'm not sure if it is wise do forbid that use case since allowing it does not hurt.

Don't get me wrong, I just want to be sure that all new sysfs files we add have a clear defined use case and won't hurt us later.

Quoted 14 lines

Quoted 13 lines

Quoted 9 lines

- /sys/class/ubi/supported-features: the features supported by the implementation (each bit is a specific feature or a set of features). The features may or may not be version dependent.
- /sys/class/ubi/ubiX/on-flash-format: the on-flash format used on the UBIX device
- /sys/class/ubi/ubiX/features: the features exposed by this UBIX device

I think we should make features depend on version = 2.
IOW when we change the UBI format in a major way version will be 3 and we maybe use something else to expose features.

Quoted 3 lines

IIUC, we move to version 2 now, because we're using one of the padding byte (word?) to expose the feature flags, correct?
It makes sense.

I'm not absolutely sure but keeping ->version and adding ->features seems the most feasible solution so far.

Quoted 2 lines

Don't get me wrong, I just want to be sure that all new sysfs files we add have a clear defined use case and won't hurt us later.

Hm, maybe you're right, exposing this information through sysfs is not a requirement, but I think we need to store this information in the EC and/or VID headers, so that the UBI implementation can recognize the on-flash format.

Quoted 18 lines

Quoted 17 lines

Quoted 10 lines

Quoted 9 lines

- /sys/class/ubi/supported-features: the features supported by the implementation (each bit is a specific feature or a set of features). The features may or may not be version dependent.
- /sys/class/ubi/ubiX/on-flash-format: the on-flash format used on the UBIX device
- /sys/class/ubi/ubiX/features: the features exposed by this UBIX device

Quoted 3 lines

I think we should make features depend on version = 2.
IOW when we change the UBI format in a major way version will be 3 and we maybe use something else to expose features.

IIUC, we move to version 2 now, because we're using one of the padding byte (word?) to expose the feature flags, correct?
It makes sense.

Quoted 2 lines

I'm not absolutely sure but keeping ->version and adding ->features seems the most feasible solution so far.

Okay, let's forget what we expose through sysfs for a moment and focus on the on-flash format. Do we agree that it's better to keep a version field encoding the 'major' version of the on-flash format, and introducing a feature field to allow minor backward-compatible improvements of a given 'major' version?