[PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[Dmitry Osipenko]

This series adds the most minimal EFI partition support for NVIDIA Tegra
consumer devices, like Android tablets and game consoles, making theirs
eMMC accessible out-of-the-box using downstream bootloader and mainline
Linux kernel.  eMMC now works on Acer A500 tablet and Ouya game console
that are already well supported in mainline and internal storage is the
only biggest thing left to support.

Changelog:

v6: - Added comment for the alternative_gpt_sector() callback, which
      was asked by Christoph Hellwig.

    - Changed alternative_gpt_sector() to take disk for the argument
      instead of blkdev. This was asked by Christoph Hellwig.

    - Dropped mmc_bdops check as it was suggested by Christoph Hellwig.

    - Added missing mmc_blk_put() that was spotted by Christoph Hellwig.

    - Moved GPT calculation into MMC core and added MMC_CAP2_ALT_GPT_SECTOR
      flag, like it was asked by Ulf Hansson. Me and Thierry have concerns
      about whether it's better to have Tegra-specific function in a core
      instead of Tegra driver, but it also works, so I decided to try that
      variant.

v5: - Implemented alternative_gpt_sector() blk/mmc callback that was
      suggested by Christoph Hellwig in a comment to v4.

    - mmc_bdev_to_card() now checks blk fops instead of the major number,
      like it was suggested by Christoph Hellwig in a comment to v4.

    - Emailed Rob Herring, which was asked by Ulf Hansson in a comment
      to v4. Although the of-match change is gone now in v5, the matching
      is transformed into the new SDHCI quirk of the Tegra driver.

v4: - Rebased on top of recent linux-next.

v3: - Removed unnecessary v1 hunk that was left by accident in efi.c of v2.

v2: - This is continuation of [1] where Davidlohr Bueso suggested that it
      should be better to avoid supporting in mainline the custom gpt_sector
      kernel cmdline parameter that downstream Android kernels use.  We can
      do this for the devices that are already mainlined, so I dropped the
      cmdline from the v2 and left only the variant with a fixed GPT address.

[1] https://lore.kernel.org/linux-efi/20210327212100.3834-3-digetx@gmail.com/T/

Dmitry Osipenko (5):
  block: Add alternative_gpt_sector() operation
  partitions/efi: Support non-standard GPT location
  mmc: core: Add raw_boot_mult field to mmc_ext_csd
  mmc: block: Support alternative_gpt_sector() operation
  mmc: sdhci-tegra: Enable MMC_CAP2_ALT_GPT_SECTOR

 block/partitions/efi.c         | 12 ++++++++++++
 drivers/mmc/core/block.c       | 21 ++++++++++++++++++++
 drivers/mmc/core/core.c        | 35 ++++++++++++++++++++++++++++++++++
 drivers/mmc/core/core.h        |  2 ++
 drivers/mmc/core/mmc.c         |  2 ++
 drivers/mmc/host/sdhci-tegra.c |  9 +++++++++
 include/linux/blkdev.h         |  7 +++++++
 include/linux/mmc/card.h       |  1 +
 include/linux/mmc/host.h       |  1 +
 9 files changed, 90 insertions(+)

-- 
2.32.0

[PATCH v6 1/5] block: Add alternative_gpt_sector() operation

[Dmitry Osipenko]

Add alternative_gpt_sector() block device operation which specifies
alternative location of a GPT entry. This allows us to support Android
devices that have GPT entry at a non-standard location and can't be
repartitioned easily.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 include/linux/blkdev.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 111a3911c4d2..d65c8f60ad8e 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1850,6 +1850,13 @@ struct block_device_operations {
 	char *(*devnode)(struct gendisk *disk, umode_t *mode);
 	struct module *owner;
 	const struct pr_ops *pr_ops;
+
+	/*
+	 * Special callback for probing GPT entry at a given sector.
+	 * Needed by Android devices, used by GPT scanner and MMC blk
+	 * driver.
+	 */
+	int (*alternative_gpt_sector)(struct gendisk *disk, sector_t *sector);
 };
 
 #ifdef CONFIG_COMPAT
-- 
2.32.0

[PATCH v6 2/5] partitions/efi: Support non-standard GPT location

[Dmitry Osipenko]

Support looking up GPT at a non-standard location specified by a block
device driver.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 block/partitions/efi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index aaa3dc487cb5..7ca5c4c374d4 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -585,6 +585,8 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
 	gpt_header *pgpt = NULL, *agpt = NULL;
 	gpt_entry *pptes = NULL, *aptes = NULL;
 	legacy_mbr *legacymbr;
+	struct gendisk *disk = state->disk;
+	const struct block_device_operations *fops = disk->fops;
 	sector_t total_sectors = get_capacity(state->disk);
 	u64 lastlba;
 
@@ -619,6 +621,16 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
         if (!good_agpt && force_gpt)
                 good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
 
+	if (!good_agpt && force_gpt && fops->alternative_gpt_sector) {
+		sector_t agpt_sector;
+		int err;
+
+		err = fops->alternative_gpt_sector(disk, &agpt_sector);
+		if (!err)
+			good_agpt = is_gpt_valid(state, agpt_sector,
+						 &agpt, &aptes);
+	}
+
         /* The obviously unsuccessful case */
         if (!good_pgpt && !good_agpt)
                 goto fail;
-- 
2.32.0

[PATCH v6 3/5] mmc: core: Add raw_boot_mult field to mmc_ext_csd

[Dmitry Osipenko]

Bootloader of NVIDIA Tegra devices linearizes the boot0/boot1/main
partitions into a single virtual space, and thus, all partition addresses
are shifted by the size of boot0 + boot1 partitions. The offset needs to
be known in order to find the GPT entry on eMMC storage of Tegra devices.
Add raw_boot_mult field to mmc_ext_csd which allows to get size of the
boot partitions.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/mmc/core/mmc.c   | 2 ++
 include/linux/mmc/card.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 838726b68ff3..29e58ffae379 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -418,6 +418,8 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
 		ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT];
 	card->ext_csd.raw_hc_erase_grp_size =
 		ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+	card->ext_csd.raw_boot_mult =
+		ext_csd[EXT_CSD_BOOT_MULT];
 	if (card->ext_csd.rev >= 3) {
 		u8 sa_shift = ext_csd[EXT_CSD_S_A_TIMEOUT];
 		card->ext_csd.part_config = ext_csd[EXT_CSD_PART_CONFIG];
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 74e6c0624d27..37f975875102 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -109,6 +109,7 @@ struct mmc_ext_csd {
 	u8			raw_hc_erase_gap_size;	/* 221 */
 	u8			raw_erase_timeout_mult;	/* 223 */
 	u8			raw_hc_erase_grp_size;	/* 224 */
+	u8			raw_boot_mult;		/* 226 */
 	u8			raw_sec_trim_mult;	/* 229 */
 	u8			raw_sec_erase_mult;	/* 230 */
 	u8			raw_sec_feature_support;/* 231 */
-- 
2.32.0

[PATCH v6 4/5] mmc: block: Support alternative_gpt_sector() operation

[Dmitry Osipenko]

Support generic alternative_gpt_sector() block device operation which
calculates custom GPT location. Add new MMC_CAP2_ALTERNATIVE_GPT_SECTOR
flag that enables scanning of the alternative sector.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/mmc/core/block.c | 21 +++++++++++++++++++++
 drivers/mmc/core/core.c  | 35 +++++++++++++++++++++++++++++++++++
 drivers/mmc/core/core.h  |  2 ++
 include/linux/mmc/host.h |  1 +
 4 files changed, 59 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 672cc505ce37..edd26164be06 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -801,6 +801,26 @@ static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode,
 }
 #endif
 
+static int mmc_blk_alternative_gpt_sector(struct gendisk *disk,
+					  sector_t *sector)
+{
+	struct mmc_blk_data *md;
+	int ret;
+
+	md = mmc_blk_get(disk);
+	if (!md)
+		return -EINVAL;
+
+	if (md->queue.card)
+		ret = mmc_card_alternative_gpt_sector(md->queue.card, sector);
+	else
+		ret = -ENODEV;
+
+	mmc_blk_put(md);
+
+	return ret;
+}
+
 static const struct block_device_operations mmc_bdops = {
 	.open			= mmc_blk_open,
 	.release		= mmc_blk_release,
@@ -810,6 +830,7 @@ static const struct block_device_operations mmc_bdops = {
 #ifdef CONFIG_COMPAT
 	.compat_ioctl		= mmc_blk_compat_ioctl,
 #endif
+	.alternative_gpt_sector	= mmc_blk_alternative_gpt_sector,
 };
 
 static int mmc_blk_part_switch_pre(struct mmc_card *card,
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6249c83d616f..c5cdd56051cb 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2150,6 +2150,41 @@ int mmc_detect_card_removed(struct mmc_host *host)
 }
 EXPORT_SYMBOL(mmc_detect_card_removed);
 
+int mmc_card_alternative_gpt_sector(struct mmc_card *card, sector_t *gpt_sector)
+{
+	unsigned int boot_sectors_num;
+
+	if ((!(card->host->caps2 & MMC_CAP2_ALT_GPT_SECTOR)))
+		return -EOPNOTSUPP;
+
+	/* filter out unrelated cards */
+	if (card->ext_csd.rev < 3 ||
+	    !mmc_card_mmc(card) ||
+	    !mmc_card_is_blockaddr(card) ||
+	     mmc_card_is_removable(card->host))
+		return -ENOENT;
+
+	/*
+	 * eMMC storage has two special boot partitions in addition to the
+	 * main one.  NVIDIA's bootloader linearizes eMMC boot0->boot1->main
+	 * accesses, this means that the partition table addresses are shifted
+	 * by the size of boot partitions.  In accordance with the eMMC
+	 * specification, the boot partition size is calculated as follows:
+	 *
+	 *	boot partition size = 128K byte x BOOT_SIZE_MULT
+	 *
+	 * Calculate number of sectors occupied by the both boot partitions.
+	 */
+	boot_sectors_num = card->ext_csd.raw_boot_mult * SZ_128K /
+			   SZ_512 * MMC_NUM_BOOT_PARTITION;
+
+	/* Defined by NVIDIA and used by Android devices. */
+	*gpt_sector = card->ext_csd.sectors - boot_sectors_num - 1;
+
+	return 0;
+}
+EXPORT_SYMBOL(mmc_card_alternative_gpt_sector);
+
 void mmc_rescan(struct work_struct *work)
 {
 	struct mmc_host *host =
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 0c4de2030b3f..7931a4f0137d 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -119,6 +119,8 @@ void mmc_release_host(struct mmc_host *host);
 void mmc_get_card(struct mmc_card *card, struct mmc_ctx *ctx);
 void mmc_put_card(struct mmc_card *card, struct mmc_ctx *ctx);
 
+int mmc_card_alternative_gpt_sector(struct mmc_card *card, sector_t *sector);
+
 /**
  *	mmc_claim_host - exclusively claim a host
  *	@host: mmc host to claim
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 0abd47e9ef9b..73a4cc063bb0 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -398,6 +398,7 @@ struct mmc_host {
 #else
 #define MMC_CAP2_CRYPTO		0
 #endif
+#define MMC_CAP2_ALT_GPT_SECTOR	(1 << 28)	/* Host with eMMC card that has GPT entry at a non-standard location */
 
 	int			fixed_drv_type;	/* fixed driver type for non-removable media */
 
-- 
2.32.0

[PATCH v6 5/5] mmc: sdhci-tegra: Enable MMC_CAP2_ALT_GPT_SECTOR

[Dmitry Osipenko]

Tegra20/30/114/124 Android devices place GPT at a non-standard location.
Enable GPT entry scanning at that location.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/mmc/host/sdhci-tegra.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 387ce9cdbd7c..39cfbb28ccc3 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -116,6 +116,8 @@
  */
 #define NVQUIRK_HAS_TMCLK				BIT(10)
 
+#define NVQUIRK_HAS_ANDROID_GPT_SECTOR			BIT(11)
+
 /* SDMMC CQE Base Address for Tegra Host Ver 4.1 and Higher */
 #define SDHCI_TEGRA_CQE_BASE_ADDR			0xF000
 
@@ -1361,6 +1363,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra20 = {
 	.pdata = &sdhci_tegra20_pdata,
 	.dma_mask = DMA_BIT_MASK(32),
 	.nvquirks = NVQUIRK_FORCE_SDHCI_SPEC_200 |
+		    NVQUIRK_HAS_ANDROID_GPT_SECTOR |
 		    NVQUIRK_ENABLE_BLOCK_GAP_DET,
 };
 
@@ -1390,6 +1393,7 @@ static const struct sdhci_tegra_soc_data soc_data_tegra30 = {
 	.nvquirks = NVQUIRK_ENABLE_SDHCI_SPEC_300 |
 		    NVQUIRK_ENABLE_SDR50 |
 		    NVQUIRK_ENABLE_SDR104 |
+		    NVQUIRK_HAS_ANDROID_GPT_SECTOR |
 		    NVQUIRK_HAS_PADCALIB,
 };
 
@@ -1422,6 +1426,7 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
 static const struct sdhci_tegra_soc_data soc_data_tegra114 = {
 	.pdata = &sdhci_tegra114_pdata,
 	.dma_mask = DMA_BIT_MASK(32),
+	.nvquirks = NVQUIRK_HAS_ANDROID_GPT_SECTOR,
 };
 
 static const struct sdhci_pltfm_data sdhci_tegra124_pdata = {
@@ -1438,6 +1443,7 @@ static const struct sdhci_pltfm_data sdhci_tegra124_pdata = {
 static const struct sdhci_tegra_soc_data soc_data_tegra124 = {
 	.pdata = &sdhci_tegra124_pdata,
 	.dma_mask = DMA_BIT_MASK(34),
+	.nvquirks = NVQUIRK_HAS_ANDROID_GPT_SECTOR,
 };
 
 static const struct sdhci_ops tegra210_sdhci_ops = {
@@ -1616,6 +1622,9 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
 	tegra_host->pad_control_available = false;
 	tegra_host->soc_data = soc_data;
 
+	if (soc_data->nvquirks & NVQUIRK_HAS_ANDROID_GPT_SECTOR)
+		host->mmc->caps2 |= MMC_CAP2_ALT_GPT_SECTOR;
+
 	if (soc_data->nvquirks & NVQUIRK_NEEDS_PAD_CONTROL) {
 		rc = tegra_sdhci_init_pinctrl_info(&pdev->dev, tegra_host);
 		if (rc == 0)
-- 
2.32.0

Re: [PATCH v6 1/5] block: Add alternative_gpt_sector() operation

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v6 2/5] partitions/efi: Support non-standard GPT location

[Christoph Hellwig]

On Thu, Aug 19, 2021 at 01:19:17AM +0300, Dmitry Osipenko wrote:
> Support looking up GPT at a non-standard location specified by a block
> device driver.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v6 2/5] partitions/efi: Support non-standard GPT location

[Davidlohr Bueso]

On Thu, 19 Aug 2021, Dmitry Osipenko wrote:

>Support looking up GPT at a non-standard location specified by a block
>device driver.
>
>Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: Davidlohr Bueso <dbueso@suse.de>

Re: [PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[Davidlohr Bueso]

On Thu, 19 Aug 2021, Dmitry Osipenko wrote:

>    - Moved GPT calculation into MMC core and added MMC_CAP2_ALT_GPT_SECTOR
>      flag, like it was asked by Ulf Hansson. Me and Thierry have concerns
>      about whether it's better to have Tegra-specific function in a core
>      instead of Tegra driver, but it also works, so I decided to try that
>      variant.

I think this is better as you had it in v5. This is specific to tegra and
shouldn't be in generic code.

Thanks,
Davidlohr

Re: [PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[Dmitry Osipenko]

19.08.2021 20:18, Davidlohr Bueso пишет:
> On Thu, 19 Aug 2021, Dmitry Osipenko wrote:
> 
>>    - Moved GPT calculation into MMC core and added
>> MMC_CAP2_ALT_GPT_SECTOR
>>      flag, like it was asked by Ulf Hansson. Me and Thierry have concerns
>>      about whether it's better to have Tegra-specific function in a core
>>      instead of Tegra driver, but it also works, so I decided to try that
>>      variant.
> 
> I think this is better as you had it in v5. This is specific to tegra and
> shouldn't be in generic code.

Yeah, but Ulf wants it to be in core. On the other hand, MMC core
already carries all kinds of quirks for hosts and cards, so it's not
something extraordinary for the MMC.

Re: [PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[Michał Mirosław]

On Thu, Aug 19, 2021 at 01:19:15AM +0300, Dmitry Osipenko wrote:
> This series adds the most minimal EFI partition support for NVIDIA Tegra
> consumer devices, like Android tablets and game consoles, making theirs
> eMMC accessible out-of-the-box using downstream bootloader and mainline
> Linux kernel.  eMMC now works on Acer A500 tablet and Ouya game console
> that are already well supported in mainline and internal storage is the
> only biggest thing left to support.
[...]

Could we provide the GPT sector via DT? As I understand this is for
non-removable eMMC storage. It would remove the need for a cap bit and
hardcoded calculations instead just checking if DT node of the controller
contains a magic entry with a number.

Best Regards
Michał Mirosław

Re: [PATCH v6 2/5] partitions/efi: Support non-standard GPT location

[Michał Mirosław]

On Thu, Aug 19, 2021 at 01:19:17AM +0300, Dmitry Osipenko wrote:
> Support looking up GPT at a non-standard location specified by a block
> device driver.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  block/partitions/efi.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
> index aaa3dc487cb5..7ca5c4c374d4 100644
> --- a/block/partitions/efi.c
> +++ b/block/partitions/efi.c
> @@ -585,6 +585,8 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>  	gpt_header *pgpt = NULL, *agpt = NULL;
>  	gpt_entry *pptes = NULL, *aptes = NULL;
>  	legacy_mbr *legacymbr;
> +	struct gendisk *disk = state->disk;
> +	const struct block_device_operations *fops = disk->fops;
>  	sector_t total_sectors = get_capacity(state->disk);
>  	u64 lastlba;
>  
> @@ -619,6 +621,16 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>          if (!good_agpt && force_gpt)
>                  good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>  
> +	if (!good_agpt && force_gpt && fops->alternative_gpt_sector) {
> +		sector_t agpt_sector;
> +		int err;
> +
> +		err = fops->alternative_gpt_sector(disk, &agpt_sector);
> +		if (!err)
> +			good_agpt = is_gpt_valid(state, agpt_sector,
> +						 &agpt, &aptes);
> +	}
> +

When alternative_gpt_sector is provided I would expect it to override
the default, not be a fallback for it. But if someone tries to put
a broken (decoy, garbage) GPT at a standard place, current ordering
will prevent overriding it.

Best Regards
Michał Mirosław

Re: [PATCH v6 2/5] partitions/efi: Support non-standard GPT location

[Dmitry Osipenko]

21.08.2021 01:45, Michał Mirosław пишет:
> On Thu, Aug 19, 2021 at 01:19:17AM +0300, Dmitry Osipenko wrote:
>> Support looking up GPT at a non-standard location specified by a block
>> device driver.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  block/partitions/efi.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
>> index aaa3dc487cb5..7ca5c4c374d4 100644
>> --- a/block/partitions/efi.c
>> +++ b/block/partitions/efi.c
>> @@ -585,6 +585,8 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>>  	gpt_header *pgpt = NULL, *agpt = NULL;
>>  	gpt_entry *pptes = NULL, *aptes = NULL;
>>  	legacy_mbr *legacymbr;
>> +	struct gendisk *disk = state->disk;
>> +	const struct block_device_operations *fops = disk->fops;
>>  	sector_t total_sectors = get_capacity(state->disk);
>>  	u64 lastlba;
>>  
>> @@ -619,6 +621,16 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>>          if (!good_agpt && force_gpt)
>>                  good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>>  
>> +	if (!good_agpt && force_gpt && fops->alternative_gpt_sector) {
>> +		sector_t agpt_sector;
>> +		int err;
>> +
>> +		err = fops->alternative_gpt_sector(disk, &agpt_sector);
>> +		if (!err)
>> +			good_agpt = is_gpt_valid(state, agpt_sector,
>> +						 &agpt, &aptes);
>> +	}
>> +
> 
> When alternative_gpt_sector is provided I would expect it to override
> the default, not be a fallback for it. But if someone tries to put
> a broken (decoy, garbage) GPT at a standard place, current ordering
> will prevent overriding it.

This will break devices that have GPT at standard location. If someone
tries to manipulate with GPT entries, then it's a problem of that someone.

Re: [PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[Dmitry Osipenko]

21.08.2021 01:41, Michał Mirosław пишет:
> On Thu, Aug 19, 2021 at 01:19:15AM +0300, Dmitry Osipenko wrote:
>> This series adds the most minimal EFI partition support for NVIDIA Tegra
>> consumer devices, like Android tablets and game consoles, making theirs
>> eMMC accessible out-of-the-box using downstream bootloader and mainline
>> Linux kernel.  eMMC now works on Acer A500 tablet and Ouya game console
>> that are already well supported in mainline and internal storage is the
>> only biggest thing left to support.
> [...]
> 
> Could we provide the GPT sector via DT? As I understand this is for
> non-removable eMMC storage. It would remove the need for a cap bit and
> hardcoded calculations instead just checking if DT node of the controller
> contains a magic entry with a number.

The same device model usually comes in different flavors that have a
different eMMC unit and size. So no, it can't be hardcoded in DT.

Re: [PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[Michał Mirosław]

On Sat, Aug 21, 2021 at 08:27:15PM +0300, Dmitry Osipenko wrote:
> 21.08.2021 01:41, Michał Mirosław пишет:
> > On Thu, Aug 19, 2021 at 01:19:15AM +0300, Dmitry Osipenko wrote:
> >> This series adds the most minimal EFI partition support for NVIDIA Tegra
> >> consumer devices, like Android tablets and game consoles, making theirs
> >> eMMC accessible out-of-the-box using downstream bootloader and mainline
> >> Linux kernel.  eMMC now works on Acer A500 tablet and Ouya game console
> >> that are already well supported in mainline and internal storage is the
> >> only biggest thing left to support.
> > [...]
> > 
> > Could we provide the GPT sector via DT? As I understand this is for
> > non-removable eMMC storage. It would remove the need for a cap bit and
> > hardcoded calculations instead just checking if DT node of the controller
> > contains a magic entry with a number.
> 
> The same device model usually comes in different flavors that have a
> different eMMC unit and size. So no, it can't be hardcoded in DT.

I see. I was thinking how to avoid of going the whole way and creating
another controller capability (since this is going to be core code) -
could this workaround be enabled just by a boolean DT property at
controller's node instead? Or do we expect non-DT platforms to be
similarly broken?

Best Regards
Michał Mirosław

Re: [PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[Michał Mirosław]

On Tue, Aug 24, 2021 at 01:40:02AM +0200, Michał Mirosław wrote:
> On Sat, Aug 21, 2021 at 08:27:15PM +0300, Dmitry Osipenko wrote:
> > 21.08.2021 01:41, Michał Mirosław пишет:
> > > On Thu, Aug 19, 2021 at 01:19:15AM +0300, Dmitry Osipenko wrote:
> > >> This series adds the most minimal EFI partition support for NVIDIA Tegra
> > >> consumer devices, like Android tablets and game consoles, making theirs
> > >> eMMC accessible out-of-the-box using downstream bootloader and mainline
> > >> Linux kernel.  eMMC now works on Acer A500 tablet and Ouya game console
> > >> that are already well supported in mainline and internal storage is the
> > >> only biggest thing left to support.
> > > [...]
> > > 
> > > Could we provide the GPT sector via DT? As I understand this is for
> > > non-removable eMMC storage. It would remove the need for a cap bit and
> > > hardcoded calculations instead just checking if DT node of the controller
> > > contains a magic entry with a number.
> > 
> > The same device model usually comes in different flavors that have a
> > different eMMC unit and size. So no, it can't be hardcoded in DT.
> 
> I see. I was thinking how to avoid of going the whole way and creating
> another controller capability (since this is going to be core code) -
> could this workaround be enabled just by a boolean DT property at
> controller's node instead? Or do we expect non-DT platforms to be
> similarly broken?

Rewording my concern: I believe that this is platform's and not 
a controller's misfeature, so the controller driver feels like wrong
place fix. That's why I'd prefer that the enable came from the DT
and not from driver's code.

Best Regards
Michał Mirosław

Re: [PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[Dmitry Osipenko]

24.08.2021 13:38, Michał Mirosław пишет:
> On Tue, Aug 24, 2021 at 01:40:02AM +0200, Michał Mirosław wrote:
>> On Sat, Aug 21, 2021 at 08:27:15PM +0300, Dmitry Osipenko wrote:
>>> 21.08.2021 01:41, Michał Mirosław пишет:
>>>> On Thu, Aug 19, 2021 at 01:19:15AM +0300, Dmitry Osipenko wrote:
>>>>> This series adds the most minimal EFI partition support for NVIDIA Tegra
>>>>> consumer devices, like Android tablets and game consoles, making theirs
>>>>> eMMC accessible out-of-the-box using downstream bootloader and mainline
>>>>> Linux kernel.  eMMC now works on Acer A500 tablet and Ouya game console
>>>>> that are already well supported in mainline and internal storage is the
>>>>> only biggest thing left to support.
>>>> [...]
>>>>
>>>> Could we provide the GPT sector via DT? As I understand this is for
>>>> non-removable eMMC storage. It would remove the need for a cap bit and
>>>> hardcoded calculations instead just checking if DT node of the controller
>>>> contains a magic entry with a number.
>>>
>>> The same device model usually comes in different flavors that have a
>>> different eMMC unit and size. So no, it can't be hardcoded in DT.
>>
>> I see. I was thinking how to avoid of going the whole way and creating
>> another controller capability (since this is going to be core code) -
>> could this workaround be enabled just by a boolean DT property at
>> controller's node instead? Or do we expect non-DT platforms to be
>> similarly broken?
> 
> Rewording my concern: I believe that this is platform's and not 
> a controller's misfeature, so the controller driver feels like wrong
> place fix. That's why I'd prefer that the enable came from the DT
> and not from driver's code.

The alternative GPT entry requires user to add 'gpt' argument to
kernel's cmdline. If board already uses proper alternative GPT entry at
the last sector, then nothing changed for that board.

The case where board uses 'gpt' cmdline + it had stale GPT entry at the
special location used by Android devices and chance that now suddenly
that GPT entry will pop up is close to zero.

All old partition table entries should be erased on reparation. If it
wasn't done, then it's not a kernel's problem, it's much more a user's
problem. Even though kernel could help that poor user if will be really
needed.

There is no reason to over-engineer unless somebody will tell that it
broke the very special board. Neither of currently supported boards
should require more quirks. Hence, why bother?

Re: [PATCH v6 0/5] Support EFI partition on NVIDIA Tegra devices

[Michał Mirosław]

On Tue, Aug 24, 2021 at 07:06:18PM +0300, Dmitry Osipenko wrote:
> 24.08.2021 13:38, Michał Mirosław пишет:
> > On Tue, Aug 24, 2021 at 01:40:02AM +0200, Michał Mirosław wrote:
> >> On Sat, Aug 21, 2021 at 08:27:15PM +0300, Dmitry Osipenko wrote:
> >>> 21.08.2021 01:41, Michał Mirosław пишет:
> >>>> On Thu, Aug 19, 2021 at 01:19:15AM +0300, Dmitry Osipenko wrote:
> >>>>> This series adds the most minimal EFI partition support for NVIDIA Tegra
> >>>>> consumer devices, like Android tablets and game consoles, making theirs
> >>>>> eMMC accessible out-of-the-box using downstream bootloader and mainline
> >>>>> Linux kernel.  eMMC now works on Acer A500 tablet and Ouya game console
> >>>>> that are already well supported in mainline and internal storage is the
> >>>>> only biggest thing left to support.
> >>>> [...]
> >>>>
> >>>> Could we provide the GPT sector via DT? As I understand this is for
> >>>> non-removable eMMC storage. It would remove the need for a cap bit and
> >>>> hardcoded calculations instead just checking if DT node of the controller
> >>>> contains a magic entry with a number.
> >>>
> >>> The same device model usually comes in different flavors that have a
> >>> different eMMC unit and size. So no, it can't be hardcoded in DT.
> >>
> >> I see. I was thinking how to avoid of going the whole way and creating
> >> another controller capability (since this is going to be core code) -
> >> could this workaround be enabled just by a boolean DT property at
> >> controller's node instead? Or do we expect non-DT platforms to be
> >> similarly broken?
> > 
> > Rewording my concern: I believe that this is platform's and not 
> > a controller's misfeature, so the controller driver feels like wrong
> > place fix. That's why I'd prefer that the enable came from the DT
> > and not from driver's code.
> 
> The alternative GPT entry requires user to add 'gpt' argument to
> kernel's cmdline. If board already uses proper alternative GPT entry at
> the last sector, then nothing changed for that board.
> 
> The case where board uses 'gpt' cmdline + it had stale GPT entry at the
> special location used by Android devices and chance that now suddenly
> that GPT entry will pop up is close to zero.
> 
> All old partition table entries should be erased on reparation. If it
> wasn't done, then it's not a kernel's problem, it's much more a user's
> problem. Even though kernel could help that poor user if will be really
> needed.
> 
> There is no reason to over-engineer unless somebody will tell that it
> broke the very special board. Neither of currently supported boards
> should require more quirks. Hence, why bother?

You could drop patch 4 from v7 if you checked DT boolean property
instead of adding a capability in patch 3. (Patch 4 would be replaced
by DT changes for relevant boards.)

Best Regards
Michał Mirosław