[PATCH v1 0/3] Introduce NVIDIA Tegra Partition Table

[PATCH v1 0/3] Introduce NVIDIA Tegra Partition Table

[Dmitry Osipenko]

Some NVIDIA Tegra devices have GPT entry at a wrong location and others may
even not have it at all. So either a custom workaround for GPT parsing or
TegraPT support is needed for those devices if we want to support them in
upstream kernel. The former solution was already rejected [1], let's try
the latter.

[1] https://patchwork.ozlabs.org/patch/1240809/

Big thanks to everyone who helped with figuring out the TegraPT format!

Dmitry Osipenko (3):
  mmc: core: Add raw_boot_mult field to mmc_ext_csd
  mmc: block: Add mmc_bdev_to_card() helper
  partitions: Introduce NVIDIA Tegra Partition Table

 arch/arm/mach-tegra/tegra.c         |  35 +++
 block/partitions/Kconfig            |   8 +
 block/partitions/Makefile           |   1 +
 block/partitions/check.c            |   4 +
 block/partitions/tegra.c            | 373 ++++++++++++++++++++++++++++
 block/partitions/tegra.h            |  71 ++++++
 drivers/mmc/core/block.c            |  14 ++
 drivers/mmc/core/mmc.c              |   2 +
 include/linux/mmc/card.h            |   4 +
 include/soc/tegra/bct.h             |  42 ++++
 include/soc/tegra/common.h          |   9 +
 include/soc/tegra/partition_table.h |  18 ++
 12 files changed, 581 insertions(+)
 create mode 100644 block/partitions/tegra.c
 create mode 100644 block/partitions/tegra.h
 create mode 100644 include/soc/tegra/bct.h
 create mode 100644 include/soc/tegra/partition_table.h

-- 
2.24.0

[PATCH v1 0/3] Introduce NVIDIA Tegra Partition Table

[Dmitry Osipenko]

Some NVIDIA Tegra devices have GPT entry at a wrong location and others may
even not have it at all. So either a custom workaround for GPT parsing or
TegraPT support is needed for those devices if we want to support them in
upstream kernel. The former solution was already rejected [1], let's try
the latter.

[1] https://patchwork.ozlabs.org/patch/1240809/

Big thanks to everyone who helped with figuring out the TegraPT format!

Dmitry Osipenko (3):
  mmc: core: Add raw_boot_mult field to mmc_ext_csd
  mmc: block: Add mmc_bdev_to_card() helper
  partitions: Introduce NVIDIA Tegra Partition Table

 arch/arm/mach-tegra/tegra.c         |  35 +++
 block/partitions/Kconfig            |   8 +
 block/partitions/Makefile           |   1 +
 block/partitions/check.c            |   4 +
 block/partitions/tegra.c            | 373 ++++++++++++++++++++++++++++
 block/partitions/tegra.h            |  71 ++++++
 drivers/mmc/core/block.c            |  14 ++
 drivers/mmc/core/mmc.c              |   2 +
 include/linux/mmc/card.h            |   4 +
 include/soc/tegra/bct.h             |  42 ++++
 include/soc/tegra/common.h          |   9 +
 include/soc/tegra/partition_table.h |  18 ++
 12 files changed, 581 insertions(+)
 create mode 100644 block/partitions/tegra.c
 create mode 100644 block/partitions/tegra.h
 create mode 100644 include/soc/tegra/bct.h
 create mode 100644 include/soc/tegra/partition_table.h

-- 
2.24.0

[PATCH v1 1/3] mmc: core: Add raw_boot_mult field to mmc_ext_csd

[Dmitry Osipenko]

In order to support parsing of NVIDIA Tegra Partition Table format, we
need to know the BOOT_SIZE_MULT value of the Extended CSD register because
NVIDIA's bootloader 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.

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 f6912ded652d..88e5b4224d3c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -417,6 +417,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 cf3780a6ccc4..90b1d83ce675 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -108,6 +108,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.24.0

[PATCH v1 2/3] mmc: block: Add mmc_bdev_to_card() helper

[Dmitry Osipenko]

NVIDIA Tegra Partition Table takes into account MMC card's BOOT_SIZE_MULT
parameter, and thus, the partition parser needs to retrieve that EXT_CSD
value from the block device. This patch introduces new helper which takes
block device for the input argument and returns corresponding MMC card.

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

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 663d87924e5e..5d853450c764 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -301,6 +301,20 @@ static ssize_t force_ro_store(struct device *dev, struct device_attribute *attr,
 	return ret;
 }
 
+struct mmc_card *mmc_bdev_to_card(struct block_device *bdev)
+{
+	struct mmc_blk_data *md;
+
+	if (bdev->bd_disk->major != MMC_BLOCK_MAJOR)
+		return NULL;
+
+	md = mmc_blk_get(bdev->bd_disk);
+	if (!md)
+		return NULL;
+
+	return md->queue.card;
+}
+
 static int mmc_blk_open(struct block_device *bdev, fmode_t mode)
 {
 	struct mmc_blk_data *md = mmc_blk_get(bdev->bd_disk);
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 90b1d83ce675..daccb0cc25f8 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -7,6 +7,7 @@
 #ifndef LINUX_MMC_CARD_H
 #define LINUX_MMC_CARD_H
 
+#include <linux/blkdev.h>
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
 
@@ -324,4 +325,6 @@ bool mmc_card_is_blockaddr(struct mmc_card *card);
 #define mmc_card_sd(c)		((c)->type == MMC_TYPE_SD)
 #define mmc_card_sdio(c)	((c)->type == MMC_TYPE_SDIO)
 
+struct mmc_card *mmc_bdev_to_card(struct block_device *bdev);
+
 #endif /* LINUX_MMC_CARD_H */
-- 
2.24.0

[PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Dmitry Osipenko]

All NVIDIA Tegra devices use a special partition table format for the
internal storage partitioning. Most of Tegra devices have GPT partition
in addition to TegraPT, but some older Android consumer-grade devices do
not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
in order to support these devices properly in the upstream kernel. This
patch adds support for NVIDIA Tegra Partition Table format that is used
at least by all NVIDIA Tegra20 and Tegra30 devices.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/tegra.c         |  35 +++
 block/partitions/Kconfig            |   8 +
 block/partitions/Makefile           |   1 +
 block/partitions/check.c            |   4 +
 block/partitions/tegra.c            | 373 ++++++++++++++++++++++++++++
 block/partitions/tegra.h            |  71 ++++++
 include/soc/tegra/bct.h             |  42 ++++
 include/soc/tegra/common.h          |   9 +
 include/soc/tegra/partition_table.h |  18 ++
 9 files changed, 561 insertions(+)
 create mode 100644 block/partitions/tegra.c
 create mode 100644 block/partitions/tegra.h
 create mode 100644 include/soc/tegra/bct.h
 create mode 100644 include/soc/tegra/partition_table.h

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index e512e606eabd..22b584b1ecad 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -28,7 +28,9 @@
 
 #include <linux/firmware/trusted_foundations.h>
 
+#include <soc/tegra/bct.h>
 #include <soc/tegra/fuse.h>
+#include <soc/tegra/partition_table.h>
 #include <soc/tegra/pmc.h>
 
 #include <asm/firmware.h>
@@ -63,9 +65,42 @@ u32 tegra_uart_config[3] = {
 	0,
 };
 
+static void __init tegra_boot_config_table_init(void)
+{
+	void __iomem *bct_base;
+	u16 pt_addr, pt_size;
+
+	bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
+
+	if (of_machine_is_compatible("nvidia,tegra20")) {
+		struct tegra20_boot_config_table __iomem *t20_bct = bct_base;
+
+		if (t20_bct->boot_data_version != TEGRA_BOOTDATA_VERSION_T20)
+			return;
+
+		pt_addr = t20_bct->partition_table_logical_sector_address;
+		pt_size = t20_bct->partition_table_num_logical_sectors;
+	} else if (of_machine_is_compatible("nvidia,tegra30")) {
+		struct tegra30_boot_config_table __iomem *t30_bct = bct_base;
+
+		if (t30_bct->boot_data_version != TEGRA_BOOTDATA_VERSION_T30)
+			return;
+
+		pt_addr = t30_bct->partition_table_logical_sector_address;
+		pt_size = t30_bct->partition_table_num_logical_sectors;
+	} else {
+		return;
+	}
+
+	pr_info("%s: BCT found in IRAM\n", __func__);
+
+	tegra_partition_table_setup(pt_addr, pt_size);
+}
+
 static void __init tegra_init_early(void)
 {
 	of_register_trusted_foundations();
+	tegra_boot_config_table_init();
 	tegra_cpu_reset_handler_init();
 	call_firmware_op(l2x0_init);
 }
diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig
index 702689a628f0..8e3b46ec528b 100644
--- a/block/partitions/Kconfig
+++ b/block/partitions/Kconfig
@@ -268,3 +268,11 @@ config CMDLINE_PARTITION
 	help
 	  Say Y here if you want to read the partition table from bootargs.
 	  The format for the command line is just like mtdparts.
+
+config TEGRA_PARTITION
+	bool "NVIDIA Tegra Partition support" if PARTITION_ADVANCED
+	depends on ARCH_TEGRA || COMPILE_TEST
+	select MMC_BLOCK
+	help
+	  Say Y here if you would like to be able to read the hard disk
+	  partition table format used by NVIDIA Tegra machines.
diff --git a/block/partitions/Makefile b/block/partitions/Makefile
index 2f276b677c81..807319883a18 100644
--- a/block/partitions/Makefile
+++ b/block/partitions/Makefile
@@ -21,3 +21,4 @@ obj-$(CONFIG_IBM_PARTITION) += ibm.o
 obj-$(CONFIG_EFI_PARTITION) += efi.o
 obj-$(CONFIG_KARMA_PARTITION) += karma.o
 obj-$(CONFIG_SYSV68_PARTITION) += sysv68.o
+obj-$(CONFIG_TEGRA_PARTITION) += tegra.o
diff --git a/block/partitions/check.c b/block/partitions/check.c
index ffe408fead0c..91268773b6ce 100644
--- a/block/partitions/check.c
+++ b/block/partitions/check.c
@@ -36,6 +36,7 @@
 #include "karma.h"
 #include "sysv68.h"
 #include "cmdline.h"
+#include "tegra.h"
 
 int warn_no_part = 1; /*This is ugly: should make genhd removable media aware*/
 
@@ -108,6 +109,9 @@ static int (*check_part[])(struct parsed_partitions *) = {
 #endif
 #ifdef CONFIG_SYSV68_PARTITION
 	sysv68_partition,
+#endif
+#ifdef CONFIG_TEGRA_PARTITION
+	tegra_partition,
 #endif
 	NULL
 };
diff --git a/block/partitions/tegra.c b/block/partitions/tegra.c
new file mode 100644
index 000000000000..fa517fa12c32
--- /dev/null
+++ b/block/partitions/tegra.c
@@ -0,0 +1,373 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * NVIDIA Tegra Partition Table
+ *
+ * Copyright (C) 2020 GRATE-DRIVER project
+ * Copyright (C) 2020 Dmitry Osipenko <digetx@gmail.com>
+ *
+ * Credits for the partition table format:
+ *
+ *   Andrey Danin <danindrey@mail.ru>       (Toshiba AC100 TegraPT format)
+ *   Gilles Grandou <gilles@grandou.net>    (Toshiba AC100 TegraPT format)
+ *   Ryan Grachek <ryan@edited.us>          (Google TV "Molly" TegraPT format)
+ *   Stephen Warren <swarren@wwwdotorg.org> (Useful suggestions about EMMC)
+ */
+
+#define pr_fmt(fmt) "tegra-partition: " fmt
+
+#include <linux/blkdev.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
+
+#include <soc/tegra/common.h>
+#include <soc/tegra/partition_table.h>
+
+#include "check.h"
+#include "tegra.h"
+
+#define TEGRA_PT_SECTOR_SZ	(TEGRA_PT_LOGICAL_SECTOR_SIZE / SECTOR_SIZE)
+#define TEGRA_PT_SECTOR(s)	(TEGRA_PT_SECTOR_SZ * (s))
+
+#define TEGRA_PT_ERR(ptp, fmt, ...)					\
+	pr_err("%s: " fmt,						\
+		(ptp)->state->bdev->bd_disk->disk_name, ##__VA_ARGS__)
+
+#define TEGRA_PT_PARSE_ERR(ptp, fmt, ...)				\
+	TEGRA_PT_ERR(ptp, "sector %llu: invalid " fmt,			\
+		     (ptp)->sector, ##__VA_ARGS__)
+
+struct tegra_partition_table_parser {
+	struct tegra_partition_table *pt;
+	struct parsed_partitions *state;
+	sector_t sector;
+	int boot_offset;
+};
+
+union tegra_partition_table_u {
+	struct tegra_partition_table pt;
+	u8 pt_parts[TEGRA_PT_SECTOR_SZ][SECTOR_SIZE];
+};
+
+static sector_t tegra_pt_sector_address;
+static sector_t tegra_pt_sectors_num;
+
+/*
+ * Some partitions are very sensitive, changing data on them may brick device.
+ *
+ * For more details about partitions see:
+ *
+ *  "https://docs.nvidia.com/jetson/l4t/Tegra Linux Driver Package Development Guide/part_config.html"
+ */
+static const char * const partitions_blacklist[] = {
+	"BCT", "EBT", "GP1", "GPT", "MBR", "PT",
+};
+
+static bool tegra_partition_name_match(struct tegra_partition *p,
+				       const char *name)
+{
+	return !strncmp(p->partition_name, name, TEGRA_PT_NAME_SIZE);
+}
+
+static bool tegra_partition_skip(struct tegra_partition *p,
+				 struct tegra_partition_table_parser *ptp,
+				 sector_t sector)
+{
+	unsigned int i;
+
+	/* skip EMMC boot partitions */
+	if (sector < ptp->boot_offset)
+		return true;
+
+	for (i = 0; i < ARRAY_SIZE(partitions_blacklist); i++) {
+		if (tegra_partition_name_match(p, partitions_blacklist[i]))
+			return true;
+	}
+
+	return false;
+}
+
+static bool tegra_partition_valid(struct tegra_partition *p,
+				  struct tegra_partition *prev,
+				  struct tegra_partition_table_parser *ptp,
+				  sector_t sector,
+				  sector_t size)
+{
+	struct tegra_partition_info *prev_pi = &prev->part_info;
+	sector_t sect_end = TEGRA_PT_SECTOR(prev_pi->logical_sector_address +
+					    prev_pi->logical_sectors_num);
+	char *type, name[2][TEGRA_PT_NAME_SIZE + 1];
+
+	snprintf(name[0], TEGRA_PT_NAME_SIZE, "%s", p->partition_name);
+	snprintf(name[1], TEGRA_PT_NAME_SIZE, "%s", prev->partition_name);
+
+	/* validate partition table BCT addresses */
+	if (tegra_partition_name_match(p, "PT") &&
+	    sector != tegra_pt_sector_address &&
+	    size   != tegra_pt_sectors_num) {
+		TEGRA_PT_PARSE_ERR(ptp, "PT location: sector=%llu size=%llu\n",
+				   sector, size);
+		return false;
+	}
+
+	/* validate size overflow */
+	if (sector + size < sector) {
+		TEGRA_PT_PARSE_ERR(ptp, "size: [%s] integer overflow sector=%llu size=%llu\n",
+				   name[0], sector, size);
+		return false;
+	}
+
+	/* validate allocation_policy=sequential */
+	if (p != prev && sect_end > sector) {
+		TEGRA_PT_PARSE_ERR(ptp, "allocation_policy: [%s] end=%llu [%s] sector=%llu size=%llu\n",
+				   name[1], sect_end, name[0], sector, size);
+		return false;
+	}
+
+	sect_end = get_capacity(ptp->state->bdev->bd_disk);
+
+	/* EMMC boot partitions are below ptp->boot_offset */
+	if (sector < ptp->boot_offset) {
+		sect_end += ptp->boot_offset;
+		type = "boot";
+	} else {
+		sector -= ptp->boot_offset;
+		type = "main";
+	}
+
+	/* validate size */
+	if (!size || sector + size > sect_end) {
+		TEGRA_PT_PARSE_ERR(ptp, "size: [%s] %s partition boot_offt=%d end=%llu sector=%llu size=%llu\n",
+				   name[0], type, ptp->boot_offset, sect_end,
+				   sector, size);
+		return false;
+	}
+
+	return true;
+}
+
+static bool tegra_partitions_parse(struct tegra_partition_table_parser *ptp,
+				   bool check_only)
+{
+	struct parsed_partitions *state = ptp->state;
+	struct tegra_partition_table *pt = ptp->pt;
+	sector_t sector, size;
+	int i, slot = 1;
+
+	for (i = 0; i < pt->secure.num_partitions; i++) {
+		struct tegra_partition *p = &pt->partitions[i];
+		struct tegra_partition *prev = &pt->partitions[max(i - 1, 0)];
+		struct tegra_partition_info *pi = &p->part_info;
+
+		if (slot == state->limit && !check_only)
+			break;
+
+		sector = TEGRA_PT_SECTOR(pi->logical_sector_address);
+		size   = TEGRA_PT_SECTOR(pi->logical_sectors_num);
+
+		if (!tegra_partition_valid(p, prev, ptp, sector, size))
+			return false;
+
+		if (check_only ||
+		    tegra_partition_skip(p, ptp, sector))
+			continue;
+
+		put_partition(state, slot++, sector - ptp->boot_offset, size);
+	}
+
+	return true;
+}
+
+static bool
+tegra_partition_table_parsed(struct tegra_partition_table_parser *ptp)
+{
+	if (ptp->pt->secure.num_partitions == 0 ||
+	    ptp->pt->secure.num_partitions > TEGRA_PT_MAX_PARTITIONS) {
+		TEGRA_PT_PARSE_ERR(ptp, "num_partitions=%u\n",
+				   ptp->pt->secure.num_partitions);
+		return false;
+	}
+
+	return tegra_partitions_parse(ptp, true) &&
+	       tegra_partitions_parse(ptp, false);
+}
+
+static int
+tegra_partition_table_insec_hdr_valid(struct tegra_partition_table_parser *ptp)
+{
+	if (ptp->pt->insecure.magic   != TEGRA_PT_MAGIC ||
+	    ptp->pt->insecure.version != TEGRA_PT_VERSION) {
+		TEGRA_PT_PARSE_ERR(ptp, "insecure header: magic=0x%llx ver=0x%x\n",
+				   ptp->pt->insecure.magic,
+				   ptp->pt->insecure.version);
+		return 0;
+	}
+
+	return 1;
+}
+
+static int
+tegra_partition_table_sec_hdr_valid(struct tegra_partition_table_parser *ptp)
+{
+	if (ptp->pt->secure.magic   != TEGRA_PT_MAGIC ||
+	    ptp->pt->secure.version != TEGRA_PT_VERSION) {
+		TEGRA_PT_PARSE_ERR(ptp, "secure header: magic=0x%llx ver=0x%x\n",
+				   ptp->pt->secure.magic,
+				   ptp->pt->secure.version);
+		return 0;
+	}
+
+	return 1;
+}
+
+static int
+tegra_partition_table_unencrypted(struct tegra_partition_table_parser *ptp)
+{
+	/* AES IV, all zeros if unencrypted */
+	if (ptp->pt->secure.random_data[0] || ptp->pt->secure.random_data[1] ||
+	    ptp->pt->secure.random_data[2] || ptp->pt->secure.random_data[3]) {
+		pr_err_once("encrypted partition table unsupported\n");
+		return 0;
+	}
+
+	return 1;
+}
+
+static const struct of_device_id tegra_sdhci_match[] = {
+	{ .compatible = "nvidia,tegra20-sdhci", },
+	{ .compatible = "nvidia,tegra30-sdhci", },
+	{}
+};
+
+static int
+tegra_partition_table_emmc_boot_offset(struct tegra_partition_table_parser *ptp)
+{
+	struct mmc_card *card = mmc_bdev_to_card(ptp->state->bdev);
+
+	/* offset applies to the main built-in EMMC storage only */
+	if (!card || mmc_card_is_removable(card->host) ||
+	    !mmc_card_is_blockaddr(card))
+		return 0;
+
+	/* skip everything unrelated to Tegra's EMMC */
+	if (!of_match_node(tegra_sdhci_match, card->host->parent->of_node))
+		return -1;
+
+	/*
+	 * 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
+	 *
+	 * This function returns number of sectors occupied by the both boot
+	 * partitions.
+	 */
+	return card->ext_csd.raw_boot_mult * SZ_128K /
+	       card->ext_csd.data_sector_size * MMC_NUM_BOOT_PARTITION;
+}
+
+static int tegra_read_partition_table(struct tegra_partition_table_parser *ptp)
+{
+	union tegra_partition_table_u *ptu = (typeof(ptu))ptp->pt;
+	unsigned int i;
+	Sector sect;
+	void *part;
+
+	for (i = 0; i < ARRAY_SIZE(ptu->pt_parts); i++) {
+		/*
+		 * Partition table takes at maximum 4096 bytes, but
+		 * read_part_sector() guarantees only that SECTOR_SIZE will
+		 * be read at minimum.
+		 */
+		part = read_part_sector(ptp->state, ptp->sector + i, &sect);
+		if (!part) {
+			TEGRA_PT_ERR(ptp, "failed to read sector %llu\n",
+				     ptp->sector + i);
+			return 0;
+		}
+
+		memcpy(ptu->pt_parts[i], part, SECTOR_SIZE);
+		put_dev_sector(sect);
+	}
+
+	return 1;
+}
+
+void tegra_partition_table_setup(sector_t logical_sector_address,
+				 sector_t logical_sectors_num)
+{
+	tegra_pt_sector_address = TEGRA_PT_SECTOR(logical_sector_address);
+	tegra_pt_sectors_num    = TEGRA_PT_SECTOR(logical_sectors_num);
+
+	pr_info("initialized to sector = %llu sectors_num = %llu\n",
+		tegra_pt_sector_address, tegra_pt_sectors_num);
+}
+
+int tegra_partition(struct parsed_partitions *state)
+{
+	struct tegra_partition_table_parser ptp = { .state = state };
+	sector_t sector_end;
+	int ret = 0;
+
+	if (!soc_is_tegra())
+		return 0;
+
+	ptp.boot_offset = tegra_partition_table_emmc_boot_offset(&ptp);
+	if (ptp.boot_offset < 0)
+		return 0;
+
+	if (tegra_pt_sector_address < ptp.boot_offset) {
+		pr_info("scanning EMMC boot partitions unimplemented\n");
+		return 0;
+	}
+
+	ptp.pt = kmalloc(sizeof(*ptp.pt), GFP_KERNEL);
+	if (!ptp.pt)
+		return 0;
+
+	ptp.sector = tegra_pt_sector_address - ptp.boot_offset;
+	sector_end = ptp.sector + tegra_pt_sectors_num;
+
+	/*
+	 * Partition table is duplicated till the sector_end.
+	 * If first table is corrupted, we will try next.
+	 */
+	while (ptp.sector < sector_end) {
+		ret = tegra_read_partition_table(&ptp);
+		if (!ret)
+			goto next_sector;
+
+		ret = tegra_partition_table_insec_hdr_valid(&ptp);
+		if (!ret)
+			goto next_sector;
+
+		ret = tegra_partition_table_unencrypted(&ptp);
+		if (!ret)
+			goto next_sector;
+
+		ret = tegra_partition_table_sec_hdr_valid(&ptp);
+		if (!ret)
+			goto next_sector;
+
+		ret = tegra_partition_table_parsed(&ptp);
+		if (ret)
+			break;
+next_sector:
+		ptp.sector += TEGRA_PT_SECTOR_SZ;
+	}
+
+	if (ret == 1)
+		strlcat(state->pp_buf, "\n", PAGE_SIZE);
+
+	kfree(ptp.pt);
+
+	return ret;
+}
diff --git a/block/partitions/tegra.h b/block/partitions/tegra.h
new file mode 100644
index 000000000000..c6173782dc2d
--- /dev/null
+++ b/block/partitions/tegra.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __FS_PART_TEGRA_H__
+#define __FS_PART_TEGRA_H__
+
+#include <linux/compiler.h>
+#include <linux/types.h>
+
+#define TEGRA_PT_MAGIC				0xffffffff8f9e8d8bULL
+#define TEGRA_PT_VERSION			0x100
+#define TEGRA_PT_LOGICAL_SECTOR_SIZE		4096
+#define TEGRA_PT_AES_HASH_SIZE			4
+#define TEGRA_PT_NAME_SIZE			4
+
+#define TEGRA_PT_HEADER_SZ						\
+	(sizeof(struct tegra_partition_header_insecure) +		\
+	 sizeof(struct tegra_partition_header_secure))			\
+
+#define TEGRA_PT_MAX_PARTITIONS						\
+	((TEGRA_PT_LOGICAL_SECTOR_SIZE - TEGRA_PT_HEADER_SZ) /		\
+	 sizeof(struct tegra_partition))
+
+struct tegra_partition_mount_info {
+	u32 device_id;
+	u32 device_instance;
+	u32 device_attr;
+	u8  mount_path[TEGRA_PT_NAME_SIZE];
+	u32 file_system_type;
+	u32 file_system_attr;
+} __packed;
+
+struct tegra_partition_info {
+	u32 partition_attr;
+	u32 __pad1;
+	u64 logical_sector_address;
+	u64 logical_sectors_num;
+	u64 __pad2[3];
+} __packed;
+
+struct tegra_partition {
+	u32 partition_id;
+	u8  partition_name[TEGRA_PT_NAME_SIZE];
+	struct tegra_partition_mount_info mount_info;
+	struct tegra_partition_info part_info;
+} __packed;
+
+struct tegra_partition_header_insecure {
+	u64 magic;
+	u32 version;
+	u32 length;
+	u32 signature[TEGRA_PT_AES_HASH_SIZE];
+} __packed;
+
+struct tegra_partition_header_secure {
+	u32 random_data[TEGRA_PT_AES_HASH_SIZE];
+	u64 magic;
+	u32 version;
+	u32 length;
+	u32 num_partitions;
+	u32 __pad;
+} __packed;
+
+struct tegra_partition_table {
+	struct tegra_partition_header_insecure insecure;
+	struct tegra_partition_header_secure secure;
+	struct tegra_partition partitions[TEGRA_PT_MAX_PARTITIONS];
+} __packed;
+
+int tegra_partition(struct parsed_partitions *state);
+
+#endif /* __FS_PART_TEGRA_H__ */
diff --git a/include/soc/tegra/bct.h b/include/soc/tegra/bct.h
new file mode 100644
index 000000000000..dc8547ab4b00
--- /dev/null
+++ b/include/soc/tegra/bct.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SOC_TEGRA_BCT_H__
+#define __SOC_TEGRA_BCT_H__
+
+#include <linux/compiler.h>
+#include <linux/types.h>
+
+#define TEGRA_IRAM_BCT_OFFSET		0x100
+
+#define TEGRA_BOOTDATA_VERSION_T20	NVBOOT_BOOTDATA_VERSION(0x2, 0x1)
+#define TEGRA_BOOTDATA_VERSION_T30	NVBOOT_BOOTDATA_VERSION(0x3, 0x1)
+
+#define NVBOOT_BOOTDATA_VERSION(a, b)	((((a) & 0xffff) << 16) | \
+					  ((b) & 0xffff))
+#define NVBOOT_CMAC_AES_HASH_LENGTH	4
+
+struct tegra20_boot_config_table {
+	u32 crypto_hash[NVBOOT_CMAC_AES_HASH_LENGTH];
+	u32 random_aes_blk[NVBOOT_CMAC_AES_HASH_LENGTH];
+	u32 boot_data_version;
+	u32 unused_data1[712];
+	u32 unused_consumer_data1;
+	u16 partition_table_logical_sector_address;
+	u16 partition_table_num_logical_sectors;
+	u32 unused_consumer_data[292];
+	u32 unused_data[3];
+} __packed;
+
+struct tegra30_boot_config_table {
+	u32 crypto_hash[NVBOOT_CMAC_AES_HASH_LENGTH];
+	u32 random_aes_blk[NVBOOT_CMAC_AES_HASH_LENGTH];
+	u32 boot_data_version;
+	u32 unused_data1[1016];
+	u32 unused_consumer_data1;
+	u16 partition_table_logical_sector_address;
+	u16 partition_table_num_logical_sectors;
+	u32 unused_consumer_data[500];
+	u32 unused_data[3];
+} __packed;
+
+#endif /* __SOC_TEGRA_BCT_H__ */
diff --git a/include/soc/tegra/common.h b/include/soc/tegra/common.h
index 98027a76ce3d..744280ecab5f 100644
--- a/include/soc/tegra/common.h
+++ b/include/soc/tegra/common.h
@@ -6,6 +6,15 @@
 #ifndef __SOC_TEGRA_COMMON_H__
 #define __SOC_TEGRA_COMMON_H__
 
+#include <linux/types.h>
+
+#ifdef CONFIG_ARCH_TEGRA
 bool soc_is_tegra(void);
+#else
+static inline bool soc_is_tegra(void)
+{
+	return false;
+}
+#endif
 
 #endif /* __SOC_TEGRA_COMMON_H__ */
diff --git a/include/soc/tegra/partition_table.h b/include/soc/tegra/partition_table.h
new file mode 100644
index 000000000000..9a7cbc20dbff
--- /dev/null
+++ b/include/soc/tegra/partition_table.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SOC_TEGRA_PARTITION_H__
+#define __SOC_TEGRA_PARTITION_H__
+
+#include <linux/types.h>
+
+#ifdef CONFIG_TEGRA_PARTITION
+void tegra_partition_table_setup(sector_t logical_sector_address,
+				 sector_t logical_sectors_num);
+#else
+static inline void tegra_partition_table_setup(sector_t logical_sector_address,
+					       sector_t logical_sectors_num)
+{
+}
+#endif
+
+#endif /* __SOC_TEGRA_PARTITION_H__ */
-- 
2.24.0

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Stephen Warren]

On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
> All NVIDIA Tegra devices use a special partition table format for the
> internal storage partitioning. Most of Tegra devices have GPT partition
> in addition to TegraPT, but some older Android consumer-grade devices do
> not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
> in order to support these devices properly in the upstream kernel. This
> patch adds support for NVIDIA Tegra Partition Table format that is used
> at least by all NVIDIA Tegra20 and Tegra30 devices.

> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c

> +static void __init tegra_boot_config_table_init(void)
> +{
> +	void __iomem *bct_base;
> +	u16 pt_addr, pt_size;
> +
> +	bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;

This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot 
Information Table) to a fixed location in IRAM, and there's some value 
in the BIT that points to where the BCT is in IRAM. In practice, it 
might work out that the BCT is always at the same place in IRAM, but 
this certainly isn't guaranteed. I think there's code in U-Boot which 
extracts the BCT location from the BIT? Yes, see 
arch/arm/mach-tegra/ap.c:get_odmdata().

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Stephen Warren]

On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
> All NVIDIA Tegra devices use a special partition table format for the
> internal storage partitioning. Most of Tegra devices have GPT partition
> in addition to TegraPT, but some older Android consumer-grade devices do
> not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
> in order to support these devices properly in the upstream kernel. This
> patch adds support for NVIDIA Tegra Partition Table format that is used
> at least by all NVIDIA Tegra20 and Tegra30 devices.

> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c

> +static void __init tegra_boot_config_table_init(void)
> +{
> +	void __iomem *bct_base;
> +	u16 pt_addr, pt_size;
> +
> +	bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;

This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot 
Information Table) to a fixed location in IRAM, and there's some value 
in the BIT that points to where the BCT is in IRAM. In practice, it 
might work out that the BCT is always at the same place in IRAM, but 
this certainly isn't guaranteed. I think there's code in U-Boot which 
extracts the BCT location from the BIT? Yes, see 
arch/arm/mach-tegra/ap.c:get_odmdata().

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Dmitry Osipenko]

25.02.2020 03:20, Stephen Warren пишет:
> On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
>> All NVIDIA Tegra devices use a special partition table format for the
>> internal storage partitioning. Most of Tegra devices have GPT partition
>> in addition to TegraPT, but some older Android consumer-grade devices do
>> not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
>> in order to support these devices properly in the upstream kernel. This
>> patch adds support for NVIDIA Tegra Partition Table format that is used
>> at least by all NVIDIA Tegra20 and Tegra30 devices.
> 
>> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
> 
>> +static void __init tegra_boot_config_table_init(void)
>> +{
>> +    void __iomem *bct_base;
>> +    u16 pt_addr, pt_size;
>> +
>> +    bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
> 
> This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot
> Information Table) to a fixed location in IRAM, and there's some value
> in the BIT that points to where the BCT is in IRAM. In practice, it
> might work out that the BCT is always at the same place in IRAM, but
> this certainly isn't guaranteed. I think there's code in U-Boot which
> extracts the BCT location from the BIT? Yes, see
> arch/arm/mach-tegra/ap.c:get_odmdata().

Thank you very much, I didn't know about that.

I checked whether Nexus 7 and A500 have a correct pointer in the BIT and
they have it. I'll take it into account in v2, thank you again.

Re: [PATCH v1 2/3] mmc: block: Add mmc_bdev_to_card() helper

[Ulf Hansson]

On Tue, 25 Feb 2020 at 00:22, Dmitry Osipenko <digetx@gmail.com> wrote:
>
> NVIDIA Tegra Partition Table takes into account MMC card's BOOT_SIZE_MULT
> parameter, and thus, the partition parser needs to retrieve that EXT_CSD
> value from the block device. This patch introduces new helper which takes
> block device for the input argument and returns corresponding MMC card.

Rather than returning the card, why not return the value you are
looking for instead? That sound more straightforward, but also allows
mmc core code to stay closer to the mmc core.

Kind regards
Uffe

>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/mmc/core/block.c | 14 ++++++++++++++
>  include/linux/mmc/card.h |  3 +++
>  2 files changed, 17 insertions(+)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 663d87924e5e..5d853450c764 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -301,6 +301,20 @@ static ssize_t force_ro_store(struct device *dev, struct device_attribute *attr,
>         return ret;
>  }
>
> +struct mmc_card *mmc_bdev_to_card(struct block_device *bdev)
> +{
> +       struct mmc_blk_data *md;
> +
> +       if (bdev->bd_disk->major != MMC_BLOCK_MAJOR)
> +               return NULL;
> +
> +       md = mmc_blk_get(bdev->bd_disk);
> +       if (!md)
> +               return NULL;
> +
> +       return md->queue.card;
> +}
> +
>  static int mmc_blk_open(struct block_device *bdev, fmode_t mode)
>  {
>         struct mmc_blk_data *md = mmc_blk_get(bdev->bd_disk);
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 90b1d83ce675..daccb0cc25f8 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -7,6 +7,7 @@
>  #ifndef LINUX_MMC_CARD_H
>  #define LINUX_MMC_CARD_H
>
> +#include <linux/blkdev.h>
>  #include <linux/device.h>
>  #include <linux/mod_devicetable.h>
>
> @@ -324,4 +325,6 @@ bool mmc_card_is_blockaddr(struct mmc_card *card);
>  #define mmc_card_sd(c)         ((c)->type == MMC_TYPE_SD)
>  #define mmc_card_sdio(c)       ((c)->type == MMC_TYPE_SDIO)
>
> +struct mmc_card *mmc_bdev_to_card(struct block_device *bdev);
> +
>  #endif /* LINUX_MMC_CARD_H */
> --
> 2.24.0
>

Re: [PATCH v1 2/3] mmc: block: Add mmc_bdev_to_card() helper

[Dmitry Osipenko]

25.02.2020 17:53, Ulf Hansson пишет:
> On Tue, 25 Feb 2020 at 00:22, Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> NVIDIA Tegra Partition Table takes into account MMC card's BOOT_SIZE_MULT
>> parameter, and thus, the partition parser needs to retrieve that EXT_CSD
>> value from the block device. This patch introduces new helper which takes
>> block device for the input argument and returns corresponding MMC card.
> 
> Rather than returning the card, why not return the value you are
> looking for instead? That sound more straightforward, but also allows
> mmc core code to stay closer to the mmc core.

Please take a look at patch #3, in particular see the
tegra_partition_table_emmc_boot_offset(). We already need more than just
the BOOT_SIZE_MULT from the struct mmc_card, in the the v2 of this
series we will probably need even a bit more.

I'll adjust the commit's message of this patch in v2, saying that more
than BOOT_SIZE_MULT is needed from the struct mmc_card. Are you okay
with this variant?

----

BTW, do you have any idea how partition table scanning of MMC's boot0/1
partitions potentially could be implemented?

It's not uncommon for Tegra devices that partition table could reside in
one of the MMC's boot partitions (Nexus 7 is one example). For now I
don't see how the scanning could be implemented easily because all
boot0/boot1/main partitions are very separated from each other in the
kernel's MMC_BLOCK.

One potential hack that comes into my mind is that the boot0/1
partitions could be always registered before the main MMC partition and
then they always will be scanned first, i.e. before the main partition.
This will allow to read out partition table from the boot partitions and
stash it for the main.

Re: [PATCH v1 2/3] mmc: block: Add mmc_bdev_to_card() helper

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4777 bytes --]

Hi Dmitry,

I love your patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on tegra/for-next linus/master v5.6-rc3 next-20200226]
[cannot apply to ulf.hansson-mmc/next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Osipenko/Introduce-NVIDIA-Tegra-Partition-Table/20200225-072610
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: m68k-randconfig-a001-20200227 (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

All errors (new ones prefixed by >>):

   drivers/mmc/host/wbsd.c: In function 'wbsd_request_end':
   drivers/mmc/host/wbsd.c:210:14: error: implicit declaration of function 'claim_dma_lock'; did you mean 'can_do_mlock'? [-Werror=implicit-function-declaration]
      dmaflags = claim_dma_lock();
                 ^~~~~~~~~~~~~~
                 can_do_mlock
>> drivers/mmc/host/wbsd.c:213:3: error: implicit declaration of function 'release_dma_lock'; did you mean 'release_pages'? [-Werror=implicit-function-declaration]
      release_dma_lock(dmaflags);
      ^~~~~~~~~~~~~~~~
      release_pages
   cc1: some warnings being treated as errors

vim +213 drivers/mmc/host/wbsd.c

^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  201  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  202  static void wbsd_request_end(struct wbsd_host *host, struct mmc_request *mrq)
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  203  {
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  204  	unsigned long dmaflags;
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  205  
cfa7f52164d6cd drivers/mmc/wbsd.c Pierre Ossman  2006-01-08  206  	if (host->dma >= 0) {
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  207  		/*
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  208  		 * Release ISA DMA controller.
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  209  		 */
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16 @210  		dmaflags = claim_dma_lock();
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  211  		disable_dma(host->dma);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  212  		clear_dma_ff(host->dma);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16 @213  		release_dma_lock(dmaflags);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  214  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  215  		/*
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  216  		 * Disable DMA on host.
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  217  		 */
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  218  		wbsd_write_index(host, WBSD_IDX_DMA, 0);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  219  	}
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  220  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  221  	host->mrq = NULL;
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  222  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  223  	/*
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  224  	 * MMC layer might call back into the driver so first unlock.
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  225  	 */
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  226  	spin_unlock(&host->lock);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  227  	mmc_request_done(host->mmc, mrq);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  228  	spin_lock(&host->lock);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  229  }
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  230  

:::::: The code at line 213 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds-gWtpgVMusWVb5UGfqNBoRg@public.gmane.org>
:::::: CC: Linus Torvalds <torvalds-gWtpgVMusWVb5UGfqNBoRg@public.gmane.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28023 bytes --]

Re: [PATCH v1 2/3] mmc: block: Add mmc_bdev_to_card() helper

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4672 bytes --]

Hi Dmitry,

I love your patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on tegra/for-next linus/master v5.6-rc3 next-20200226]
[cannot apply to ulf.hansson-mmc/next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Osipenko/Introduce-NVIDIA-Tegra-Partition-Table/20200225-072610
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: m68k-randconfig-a001-20200227 (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mmc/host/wbsd.c: In function 'wbsd_request_end':
   drivers/mmc/host/wbsd.c:210:14: error: implicit declaration of function 'claim_dma_lock'; did you mean 'can_do_mlock'? [-Werror=implicit-function-declaration]
      dmaflags = claim_dma_lock();
                 ^~~~~~~~~~~~~~
                 can_do_mlock
>> drivers/mmc/host/wbsd.c:213:3: error: implicit declaration of function 'release_dma_lock'; did you mean 'release_pages'? [-Werror=implicit-function-declaration]
      release_dma_lock(dmaflags);
      ^~~~~~~~~~~~~~~~
      release_pages
   cc1: some warnings being treated as errors

vim +213 drivers/mmc/host/wbsd.c

^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  201  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  202  static void wbsd_request_end(struct wbsd_host *host, struct mmc_request *mrq)
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  203  {
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  204  	unsigned long dmaflags;
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  205  
cfa7f52164d6cd drivers/mmc/wbsd.c Pierre Ossman  2006-01-08  206  	if (host->dma >= 0) {
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  207  		/*
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  208  		 * Release ISA DMA controller.
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  209  		 */
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16 @210  		dmaflags = claim_dma_lock();
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  211  		disable_dma(host->dma);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  212  		clear_dma_ff(host->dma);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16 @213  		release_dma_lock(dmaflags);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  214  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  215  		/*
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  216  		 * Disable DMA on host.
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  217  		 */
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  218  		wbsd_write_index(host, WBSD_IDX_DMA, 0);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  219  	}
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  220  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  221  	host->mrq = NULL;
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  222  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  223  	/*
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  224  	 * MMC layer might call back into the driver so first unlock.
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  225  	 */
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  226  	spin_unlock(&host->lock);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  227  	mmc_request_done(host->mmc, mrq);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  228  	spin_lock(&host->lock);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  229  }
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  230  

:::::: The code at line 213 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28023 bytes --]

Re: [PATCH v1 2/3] mmc: block: Add mmc_bdev_to_card() helper

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4753 bytes --]

Hi Dmitry,

I love your patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on tegra/for-next linus/master v5.6-rc3 next-20200226]
[cannot apply to ulf.hansson-mmc/next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Osipenko/Introduce-NVIDIA-Tegra-Partition-Table/20200225-072610
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: m68k-randconfig-a001-20200227 (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mmc/host/wbsd.c: In function 'wbsd_request_end':
   drivers/mmc/host/wbsd.c:210:14: error: implicit declaration of function 'claim_dma_lock'; did you mean 'can_do_mlock'? [-Werror=implicit-function-declaration]
      dmaflags = claim_dma_lock();
                 ^~~~~~~~~~~~~~
                 can_do_mlock
>> drivers/mmc/host/wbsd.c:213:3: error: implicit declaration of function 'release_dma_lock'; did you mean 'release_pages'? [-Werror=implicit-function-declaration]
      release_dma_lock(dmaflags);
      ^~~~~~~~~~~~~~~~
      release_pages
   cc1: some warnings being treated as errors

vim +213 drivers/mmc/host/wbsd.c

^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  201  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  202  static void wbsd_request_end(struct wbsd_host *host, struct mmc_request *mrq)
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  203  {
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  204  	unsigned long dmaflags;
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  205  
cfa7f52164d6cd drivers/mmc/wbsd.c Pierre Ossman  2006-01-08  206  	if (host->dma >= 0) {
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  207  		/*
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  208  		 * Release ISA DMA controller.
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  209  		 */
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16 @210  		dmaflags = claim_dma_lock();
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  211  		disable_dma(host->dma);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  212  		clear_dma_ff(host->dma);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16 @213  		release_dma_lock(dmaflags);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  214  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  215  		/*
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  216  		 * Disable DMA on host.
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  217  		 */
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  218  		wbsd_write_index(host, WBSD_IDX_DMA, 0);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  219  	}
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  220  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  221  	host->mrq = NULL;
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  222  
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  223  	/*
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  224  	 * MMC layer might call back into the driver so first unlock.
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  225  	 */
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  226  	spin_unlock(&host->lock);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  227  	mmc_request_done(host->mmc, mrq);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  228  	spin_lock(&host->lock);
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  229  }
^1da177e4c3f41 drivers/mmc/wbsd.c Linus Torvalds 2005-04-16  230  

:::::: The code at line 213 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28023 bytes --]

RE: [PATCH v1 1/3] mmc: core: Add raw_boot_mult field to mmc_ext_csd

[Avri Altman]

Hi,
> 
> 
> In order to support parsing of NVIDIA Tegra Partition Table format, we
> need to know the BOOT_SIZE_MULT value of the Extended CSD register
> because
> NVIDIA's bootloader 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.
> 
> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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 f6912ded652d..88e5b4224d3c 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -417,6 +417,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];
You might want at this point multiply it by 128K,
And get rid of: part_size = ext_csd[EXT_CSD_BOOT_MULT] << 17;
Below...

Thanks,
Avri

RE: [PATCH v1 1/3] mmc: core: Add raw_boot_mult field to mmc_ext_csd

[Avri Altman]

Hi,
> 
> 
> In order to support parsing of NVIDIA Tegra Partition Table format, we
> need to know the BOOT_SIZE_MULT value of the Extended CSD register
> because
> NVIDIA's bootloader 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.
> 
> 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 f6912ded652d..88e5b4224d3c 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -417,6 +417,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];
You might want at this point multiply it by 128K,
And get rid of: part_size = ext_csd[EXT_CSD_BOOT_MULT] << 17;
Below...

Thanks,
Avri

Re: [PATCH v1 1/3] mmc: core: Add raw_boot_mult field to mmc_ext_csd

[Dmitry Osipenko]

01.03.2020 13:50, Avri Altman пишет:
> Hi,
>>
>>
>> In order to support parsing of NVIDIA Tegra Partition Table format, we
>> need to know the BOOT_SIZE_MULT value of the Extended CSD register
>> because
>> NVIDIA's bootloader 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.
>>
>> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  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 f6912ded652d..88e5b4224d3c 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -417,6 +417,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];
> You might want at this point multiply it by 128K,
> And get rid of: part_size = ext_csd[EXT_CSD_BOOT_MULT] << 17;
> Below...

But it's not a *raw* _boot_mult anymore then. I'm not sure that it will
be a worthwhile change.

Re: [PATCH v1 1/3] mmc: core: Add raw_boot_mult field to mmc_ext_csd

[Dmitry Osipenko]

01.03.2020 13:50, Avri Altman пишет:
> Hi,
>>
>>
>> In order to support parsing of NVIDIA Tegra Partition Table format, we
>> need to know the BOOT_SIZE_MULT value of the Extended CSD register
>> because
>> NVIDIA's bootloader 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.
>>
>> 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 f6912ded652d..88e5b4224d3c 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -417,6 +417,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];
> You might want at this point multiply it by 128K,
> And get rid of: part_size = ext_csd[EXT_CSD_BOOT_MULT] << 17;
> Below...

But it's not a *raw* _boot_mult anymore then. I'm not sure that it will
be a worthwhile change.

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Ulf Hansson]

On Tue, 25 Feb 2020 at 01:20, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>
> On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
> > All NVIDIA Tegra devices use a special partition table format for the
> > internal storage partitioning. Most of Tegra devices have GPT partition
> > in addition to TegraPT, but some older Android consumer-grade devices do
> > not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
> > in order to support these devices properly in the upstream kernel. This
> > patch adds support for NVIDIA Tegra Partition Table format that is used
> > at least by all NVIDIA Tegra20 and Tegra30 devices.
>
> > diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
>
> > +static void __init tegra_boot_config_table_init(void)
> > +{
> > +     void __iomem *bct_base;
> > +     u16 pt_addr, pt_size;
> > +
> > +     bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
>
> This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot
> Information Table) to a fixed location in IRAM, and there's some value
> in the BIT that points to where the BCT is in IRAM. In practice, it
> might work out that the BCT is always at the same place in IRAM, but
> this certainly isn't guaranteed. I think there's code in U-Boot which
> extracts the BCT location from the BIT? Yes, see
> arch/arm/mach-tegra/ap.c:get_odmdata().

So, have you considered using the command line partition option,
rather than adding yet another partition scheme to the kernel?

In principle, you would let the boot loader scan for the partitions,
likely from machine specific code in U-boot. Then you append these to
the kernel command line and let block/partitions/cmdline.c scan for
it.

Kind regards
Uffe

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Ulf Hansson]

On Tue, 25 Feb 2020 at 01:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
> > All NVIDIA Tegra devices use a special partition table format for the
> > internal storage partitioning. Most of Tegra devices have GPT partition
> > in addition to TegraPT, but some older Android consumer-grade devices do
> > not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
> > in order to support these devices properly in the upstream kernel. This
> > patch adds support for NVIDIA Tegra Partition Table format that is used
> > at least by all NVIDIA Tegra20 and Tegra30 devices.
>
> > diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
>
> > +static void __init tegra_boot_config_table_init(void)
> > +{
> > +     void __iomem *bct_base;
> > +     u16 pt_addr, pt_size;
> > +
> > +     bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
>
> This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot
> Information Table) to a fixed location in IRAM, and there's some value
> in the BIT that points to where the BCT is in IRAM. In practice, it
> might work out that the BCT is always at the same place in IRAM, but
> this certainly isn't guaranteed. I think there's code in U-Boot which
> extracts the BCT location from the BIT? Yes, see
> arch/arm/mach-tegra/ap.c:get_odmdata().

So, have you considered using the command line partition option,
rather than adding yet another partition scheme to the kernel?

In principle, you would let the boot loader scan for the partitions,
likely from machine specific code in U-boot. Then you append these to
the kernel command line and let block/partitions/cmdline.c scan for
it.

Kind regards
Uffe

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Dmitry Osipenko]

04.03.2020 19:36, Ulf Hansson пишет:
> On Tue, 25 Feb 2020 at 01:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>> On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
>>> All NVIDIA Tegra devices use a special partition table format for the
>>> internal storage partitioning. Most of Tegra devices have GPT partition
>>> in addition to TegraPT, but some older Android consumer-grade devices do
>>> not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
>>> in order to support these devices properly in the upstream kernel. This
>>> patch adds support for NVIDIA Tegra Partition Table format that is used
>>> at least by all NVIDIA Tegra20 and Tegra30 devices.
>>
>>> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
>>
>>> +static void __init tegra_boot_config_table_init(void)
>>> +{
>>> +     void __iomem *bct_base;
>>> +     u16 pt_addr, pt_size;
>>> +
>>> +     bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
>>
>> This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot
>> Information Table) to a fixed location in IRAM, and there's some value
>> in the BIT that points to where the BCT is in IRAM. In practice, it
>> might work out that the BCT is always at the same place in IRAM, but
>> this certainly isn't guaranteed. I think there's code in U-Boot which
>> extracts the BCT location from the BIT? Yes, see
>> arch/arm/mach-tegra/ap.c:get_odmdata().
> 
> So, have you considered using the command line partition option,
> rather than adding yet another partition scheme to the kernel?
> 
> In principle, you would let the boot loader scan for the partitions,
> likely from machine specific code in U-boot. Then you append these to
> the kernel command line and let block/partitions/cmdline.c scan for
> it.

The bootloader is usually locked-down on a consumer Tegra machines (it's
signed / encrypted).

Technically, it should be possible to chain-load some custom secondary
bootloader instead of a kernel image, but this is not very practical
because now:

1. There is a need to make a custom bootloader and it is quite a lot of
work.

2. You'll have to tell everybody that a custom booloader may need to be
used in order to get a working eMMC.

3. NVIDIA's bootloader already passes a command line parameter to kernel
for locating GPT entry, but this hack is not acceptable for the upstream
kernel.

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Ulf Hansson]

On Wed, 4 Mar 2020 at 18:09, Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> 04.03.2020 19:36, Ulf Hansson пишет:
> > On Tue, 25 Feb 2020 at 01:20, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> >>
> >> On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
> >>> All NVIDIA Tegra devices use a special partition table format for the
> >>> internal storage partitioning. Most of Tegra devices have GPT partition
> >>> in addition to TegraPT, but some older Android consumer-grade devices do
> >>> not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
> >>> in order to support these devices properly in the upstream kernel. This
> >>> patch adds support for NVIDIA Tegra Partition Table format that is used
> >>> at least by all NVIDIA Tegra20 and Tegra30 devices.
> >>
> >>> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
> >>
> >>> +static void __init tegra_boot_config_table_init(void)
> >>> +{
> >>> +     void __iomem *bct_base;
> >>> +     u16 pt_addr, pt_size;
> >>> +
> >>> +     bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
> >>
> >> This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot
> >> Information Table) to a fixed location in IRAM, and there's some value
> >> in the BIT that points to where the BCT is in IRAM. In practice, it
> >> might work out that the BCT is always at the same place in IRAM, but
> >> this certainly isn't guaranteed. I think there's code in U-Boot which
> >> extracts the BCT location from the BIT? Yes, see
> >> arch/arm/mach-tegra/ap.c:get_odmdata().
> >
> > So, have you considered using the command line partition option,
> > rather than adding yet another partition scheme to the kernel?
> >
> > In principle, you would let the boot loader scan for the partitions,
> > likely from machine specific code in U-boot. Then you append these to
> > the kernel command line and let block/partitions/cmdline.c scan for
> > it.
>
> The bootloader is usually locked-down on a consumer Tegra machines (it's
> signed / encrypted).

Right, you are you talking about this from a developer point of view,
not from an end product user?

I mean, for sure you can upgrade the bootloader on Nvidia products? No, really?

>
> Technically, it should be possible to chain-load some custom secondary
> bootloader instead of a kernel image, but this is not very practical
> because now:
>
> 1. There is a need to make a custom bootloader and it is quite a lot of
> work.
>
> 2. You'll have to tell everybody that a custom booloader may need to be
> used in order to get a working eMMC.

Yeah, I get the point. It's not an optimal situation, but I assume
it's about informing developers. They can cope with this, no?

>
> 3. NVIDIA's bootloader already passes a command line parameter to kernel
> for locating GPT entry, but this hack is not acceptable for the upstream
> kernel.

Well, I am just worried that we will end up with one partition format
per vendor/product, that wouldn't scale very well.

In any case, from mmc point of view I am less concerned, we can find a
way to support the needed bits. I just need to review the series more
carefully and provide some comments. :-)

However, before I do that, I would like to hear Jens opinion about
adding a new partition format, so I don't waste my time here.

Kind regards
Uffe

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Ulf Hansson]

On Wed, 4 Mar 2020 at 18:09, Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 04.03.2020 19:36, Ulf Hansson пишет:
> > On Tue, 25 Feb 2020 at 01:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >>
> >> On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
> >>> All NVIDIA Tegra devices use a special partition table format for the
> >>> internal storage partitioning. Most of Tegra devices have GPT partition
> >>> in addition to TegraPT, but some older Android consumer-grade devices do
> >>> not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
> >>> in order to support these devices properly in the upstream kernel. This
> >>> patch adds support for NVIDIA Tegra Partition Table format that is used
> >>> at least by all NVIDIA Tegra20 and Tegra30 devices.
> >>
> >>> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
> >>
> >>> +static void __init tegra_boot_config_table_init(void)
> >>> +{
> >>> +     void __iomem *bct_base;
> >>> +     u16 pt_addr, pt_size;
> >>> +
> >>> +     bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
> >>
> >> This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot
> >> Information Table) to a fixed location in IRAM, and there's some value
> >> in the BIT that points to where the BCT is in IRAM. In practice, it
> >> might work out that the BCT is always at the same place in IRAM, but
> >> this certainly isn't guaranteed. I think there's code in U-Boot which
> >> extracts the BCT location from the BIT? Yes, see
> >> arch/arm/mach-tegra/ap.c:get_odmdata().
> >
> > So, have you considered using the command line partition option,
> > rather than adding yet another partition scheme to the kernel?
> >
> > In principle, you would let the boot loader scan for the partitions,
> > likely from machine specific code in U-boot. Then you append these to
> > the kernel command line and let block/partitions/cmdline.c scan for
> > it.
>
> The bootloader is usually locked-down on a consumer Tegra machines (it's
> signed / encrypted).

Right, you are you talking about this from a developer point of view,
not from an end product user?

I mean, for sure you can upgrade the bootloader on Nvidia products? No, really?

>
> Technically, it should be possible to chain-load some custom secondary
> bootloader instead of a kernel image, but this is not very practical
> because now:
>
> 1. There is a need to make a custom bootloader and it is quite a lot of
> work.
>
> 2. You'll have to tell everybody that a custom booloader may need to be
> used in order to get a working eMMC.

Yeah, I get the point. It's not an optimal situation, but I assume
it's about informing developers. They can cope with this, no?

>
> 3. NVIDIA's bootloader already passes a command line parameter to kernel
> for locating GPT entry, but this hack is not acceptable for the upstream
> kernel.

Well, I am just worried that we will end up with one partition format
per vendor/product, that wouldn't scale very well.

In any case, from mmc point of view I am less concerned, we can find a
way to support the needed bits. I just need to review the series more
carefully and provide some comments. :-)

However, before I do that, I would like to hear Jens opinion about
adding a new partition format, so I don't waste my time here.

Kind regards
Uffe

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Stephen Warren]

On 3/6/20 6:37 AM, Ulf Hansson wrote:
> On Wed, 4 Mar 2020 at 18:09, Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> 04.03.2020 19:36, Ulf Hansson пишет:
>>> On Tue, 25 Feb 2020 at 01:20, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>>>>
>>>> On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
>>>>> All NVIDIA Tegra devices use a special partition table format for the
>>>>> internal storage partitioning. Most of Tegra devices have GPT partition
>>>>> in addition to TegraPT, but some older Android consumer-grade devices do
>>>>> not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
>>>>> in order to support these devices properly in the upstream kernel. This
>>>>> patch adds support for NVIDIA Tegra Partition Table format that is used
>>>>> at least by all NVIDIA Tegra20 and Tegra30 devices.
>>>>
>>>>> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
>>>>
>>>>> +static void __init tegra_boot_config_table_init(void)
>>>>> +{
>>>>> +     void __iomem *bct_base;
>>>>> +     u16 pt_addr, pt_size;
>>>>> +
>>>>> +     bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
>>>>
>>>> This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot
>>>> Information Table) to a fixed location in IRAM, and there's some value
>>>> in the BIT that points to where the BCT is in IRAM. In practice, it
>>>> might work out that the BCT is always at the same place in IRAM, but
>>>> this certainly isn't guaranteed. I think there's code in U-Boot which
>>>> extracts the BCT location from the BIT? Yes, see
>>>> arch/arm/mach-tegra/ap.c:get_odmdata().
>>>
>>> So, have you considered using the command line partition option,
>>> rather than adding yet another partition scheme to the kernel?
>>>
>>> In principle, you would let the boot loader scan for the partitions,
>>> likely from machine specific code in U-boot. Then you append these to
>>> the kernel command line and let block/partitions/cmdline.c scan for
>>> it.
>>
>> The bootloader is usually locked-down on a consumer Tegra machines (it's
>> signed / encrypted).
> 
> Right, you are you talking about this from a developer point of view,
> not from an end product user?
> 
> I mean, for sure you can upgrade the bootloader on Nvidia products? No, really?

For developer-oriented products like Jetson developer kits, you can 
upgrade the bootloader, and luckily they haven't used this partition 
table format for many versions.

However, commercial Android products typically have secure boot enabled, 
so you can't replace the bootloader unless you know the secure boot 
keys, which only the manufacturer knows. Dmitry is working on 
re-purposing such products.

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Stephen Warren]

On 3/6/20 6:37 AM, Ulf Hansson wrote:
> On Wed, 4 Mar 2020 at 18:09, Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> 04.03.2020 19:36, Ulf Hansson пишет:
>>> On Tue, 25 Feb 2020 at 01:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
>>>>> All NVIDIA Tegra devices use a special partition table format for the
>>>>> internal storage partitioning. Most of Tegra devices have GPT partition
>>>>> in addition to TegraPT, but some older Android consumer-grade devices do
>>>>> not or GPT is placed in a wrong sector, and thus, the TegraPT is needed
>>>>> in order to support these devices properly in the upstream kernel. This
>>>>> patch adds support for NVIDIA Tegra Partition Table format that is used
>>>>> at least by all NVIDIA Tegra20 and Tegra30 devices.
>>>>
>>>>> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
>>>>
>>>>> +static void __init tegra_boot_config_table_init(void)
>>>>> +{
>>>>> +     void __iomem *bct_base;
>>>>> +     u16 pt_addr, pt_size;
>>>>> +
>>>>> +     bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
>>>>
>>>> This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot
>>>> Information Table) to a fixed location in IRAM, and there's some value
>>>> in the BIT that points to where the BCT is in IRAM. In practice, it
>>>> might work out that the BCT is always at the same place in IRAM, but
>>>> this certainly isn't guaranteed. I think there's code in U-Boot which
>>>> extracts the BCT location from the BIT? Yes, see
>>>> arch/arm/mach-tegra/ap.c:get_odmdata().
>>>
>>> So, have you considered using the command line partition option,
>>> rather than adding yet another partition scheme to the kernel?
>>>
>>> In principle, you would let the boot loader scan for the partitions,
>>> likely from machine specific code in U-boot. Then you append these to
>>> the kernel command line and let block/partitions/cmdline.c scan for
>>> it.
>>
>> The bootloader is usually locked-down on a consumer Tegra machines (it's
>> signed / encrypted).
> 
> Right, you are you talking about this from a developer point of view,
> not from an end product user?
> 
> I mean, for sure you can upgrade the bootloader on Nvidia products? No, really?

For developer-oriented products like Jetson developer kits, you can 
upgrade the bootloader, and luckily they haven't used this partition 
table format for many versions.

However, commercial Android products typically have secure boot enabled, 
so you can't replace the bootloader unless you know the secure boot 
keys, which only the manufacturer knows. Dmitry is working on 
re-purposing such products.

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Dmitry Osipenko]

06.03.2020 19:52, Stephen Warren пишет:
> On 3/6/20 6:37 AM, Ulf Hansson wrote:
>> On Wed, 4 Mar 2020 at 18:09, Dmitry Osipenko <digetx@gmail.com> wrote:
>>>
>>> 04.03.2020 19:36, Ulf Hansson пишет:
>>>> On Tue, 25 Feb 2020 at 01:20, Stephen Warren <swarren@wwwdotorg.org>
>>>> wrote:
>>>>>
>>>>> On 2/24/20 4:18 PM, Dmitry Osipenko wrote:
>>>>>> All NVIDIA Tegra devices use a special partition table format for the
>>>>>> internal storage partitioning. Most of Tegra devices have GPT
>>>>>> partition
>>>>>> in addition to TegraPT, but some older Android consumer-grade
>>>>>> devices do
>>>>>> not or GPT is placed in a wrong sector, and thus, the TegraPT is
>>>>>> needed
>>>>>> in order to support these devices properly in the upstream kernel.
>>>>>> This
>>>>>> patch adds support for NVIDIA Tegra Partition Table format that is
>>>>>> used
>>>>>> at least by all NVIDIA Tegra20 and Tegra30 devices.
>>>>>
>>>>>> diff --git a/arch/arm/mach-tegra/tegra.c
>>>>>> b/arch/arm/mach-tegra/tegra.c
>>>>>
>>>>>> +static void __init tegra_boot_config_table_init(void)
>>>>>> +{
>>>>>> +     void __iomem *bct_base;
>>>>>> +     u16 pt_addr, pt_size;
>>>>>> +
>>>>>> +     bct_base = IO_ADDRESS(TEGRA_IRAM_BASE) + TEGRA_IRAM_BCT_OFFSET;
>>>>>
>>>>> This shouldn't be hard-coded. IIRC, the boot ROM writes a BIT (Boot
>>>>> Information Table) to a fixed location in IRAM, and there's some value
>>>>> in the BIT that points to where the BCT is in IRAM. In practice, it
>>>>> might work out that the BCT is always at the same place in IRAM, but
>>>>> this certainly isn't guaranteed. I think there's code in U-Boot which
>>>>> extracts the BCT location from the BIT? Yes, see
>>>>> arch/arm/mach-tegra/ap.c:get_odmdata().
>>>>
>>>> So, have you considered using the command line partition option,
>>>> rather than adding yet another partition scheme to the kernel?
>>>>
>>>> In principle, you would let the boot loader scan for the partitions,
>>>> likely from machine specific code in U-boot. Then you append these to
>>>> the kernel command line and let block/partitions/cmdline.c scan for
>>>> it.
>>>
>>> The bootloader is usually locked-down on a consumer Tegra machines (it's
>>> signed / encrypted).
>>
>> Right, you are you talking about this from a developer point of view,
>> not from an end product user?
>>
>> I mean, for sure you can upgrade the bootloader on Nvidia products?
>> No, really?
> 
> For developer-oriented products like Jetson developer kits, you can
> upgrade the bootloader, and luckily they haven't used this partition
> table format for many versions.
> 
> However, commercial Android products typically have secure boot enabled,
> so you can't replace the bootloader unless you know the secure boot
> keys, which only the manufacturer knows. Dmitry is working on
> re-purposing such products.

Thank you very much for the good clarification :)

Re: [PATCH v1 3/3] partitions: Introduce NVIDIA Tegra Partition Table

[Dmitry Osipenko]

06.03.2020 16:37, Ulf Hansson пишет:
...
>>
>> Technically, it should be possible to chain-load some custom secondary
>> bootloader instead of a kernel image, but this is not very practical
>> because now:
>>
>> 1. There is a need to make a custom bootloader and it is quite a lot of
>> work.
>>
>> 2. You'll have to tell everybody that a custom booloader may need to be
>> used in order to get a working eMMC.
> 
> Yeah, I get the point. It's not an optimal situation, but I assume
> it's about informing developers. They can cope with this, no?

Perhaps no, it's not only about the informing. The need for a custom
bootloader creates other inconveniences because:

1. It won't be possible to boot a vanilla upstream kernel using
Android's "fastboot boot ..." without applying extra patches to kernel
for the partition table support. Advanced users usually tend to use
fastboot and it's also very useful for a regular development purposes as
well.

2. Somebody (a developer / advanced user) will have to create a custom
bootloader for each device in the first place. This is not what an
average person will be able to do and there are not that many developers
who would want to dedicate theirs time to this.

3. The entry barrier for upstreaming Android devices support to the
kernel is already quite enormous. Adding extra hurdles isn't a step into
the right direction, IMO.

>> 3. NVIDIA's bootloader already passes a command line parameter to kernel
>> for locating GPT entry, but this hack is not acceptable for the upstream
>> kernel.
> 
> Well, I am just worried that we will end up with one partition format
> per vendor/product, that wouldn't scale very well.
> 
> In any case, from mmc point of view I am less concerned, we can find a
> way to support the needed bits. I just need to review the series more
> carefully and provide some comments. :-)
> 
> However, before I do that, I would like to hear Jens opinion about
> adding a new partition format, so I don't waste my time here.

Sure, no problems :) Let's wait for the comments from Jens.