[PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Pavel Tatashin]

Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
However, it is still not possible todo on systems with device trees.

Here is EFI fixes from Marc:
https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com

For Device Tree variant: lets allow reserve a memory region in interrupt
controller node, and use this property to allocate interrupt tables.

This way we are safe during kexec, as these page tables are going to stay
the same after kexec.

Pavel Tatashin (6):
  rqchip/gic-v3-its: reset prop table outside of allocation
  rqchip/gic-v3-its: use temporary va / pa variables
  rqchip/gic-v3-its: add reset pending table function
  rqchip/gic-v3-its: move reset pending table outside of allocator
  rqchip/gic-v3-its: move reset pending table outside of allocator
  dt-bindings: interrupt-controller: add optional memory-region

 .../interrupt-controller/arm,gic-v3.yaml      |   7 +
 drivers/irqchip/irq-gic-v3-its.c              | 121 +++++++++++++-----
 2 files changed, 96 insertions(+), 32 deletions(-)

-- 
2.23.0

[PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Pavel Tatashin]

Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
However, it is still not possible todo on systems with device trees.

Here is EFI fixes from Marc:
https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com

For Device Tree variant: lets allow reserve a memory region in interrupt
controller node, and use this property to allocate interrupt tables.

This way we are safe during kexec, as these page tables are going to stay
the same after kexec.

Pavel Tatashin (6):
  rqchip/gic-v3-its: reset prop table outside of allocation
  rqchip/gic-v3-its: use temporary va / pa variables
  rqchip/gic-v3-its: add reset pending table function
  rqchip/gic-v3-its: move reset pending table outside of allocator
  rqchip/gic-v3-its: move reset pending table outside of allocator
  dt-bindings: interrupt-controller: add optional memory-region

 .../interrupt-controller/arm,gic-v3.yaml      |   7 +
 drivers/irqchip/irq-gic-v3-its.c              | 121 +++++++++++++-----
 2 files changed, 96 insertions(+), 32 deletions(-)

-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Pavel Tatashin]

Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
However, it is still not possible todo on systems with device trees.

Here is EFI fixes from Marc:
https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com

For Device Tree variant: lets allow reserve a memory region in interrupt
controller node, and use this property to allocate interrupt tables.

This way we are safe during kexec, as these page tables are going to stay
the same after kexec.

Pavel Tatashin (6):
  rqchip/gic-v3-its: reset prop table outside of allocation
  rqchip/gic-v3-its: use temporary va / pa variables
  rqchip/gic-v3-its: add reset pending table function
  rqchip/gic-v3-its: move reset pending table outside of allocator
  rqchip/gic-v3-its: move reset pending table outside of allocator
  dt-bindings: interrupt-controller: add optional memory-region

 .../interrupt-controller/arm,gic-v3.yaml      |   7 +
 drivers/irqchip/irq-gic-v3-its.c              | 121 +++++++++++++-----
 2 files changed, 96 insertions(+), 32 deletions(-)

-- 
2.23.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v1 1/6] rqchip/gic-v3-its: reset prop table outside of allocation

[Pavel Tatashin]

In preparation of adding another variant of allocation, move
the resetting outside of the current allocator.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1b5c3672aea2..ada18748ed1c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1621,15 +1621,7 @@ static void gic_reset_prop_table(void *va)
 
 static struct page *its_allocate_prop_table(gfp_t gfp_flags)
 {
-	struct page *prop_page;
-
-	prop_page = alloc_pages(gfp_flags, get_order(LPI_PROPBASE_SZ));
-	if (!prop_page)
-		return NULL;
-
-	gic_reset_prop_table(page_address(prop_page));
-
-	return prop_page;
+	return alloc_pages(gfp_flags, get_order(LPI_PROPBASE_SZ));
 }
 
 static void its_free_prop_table(struct page *prop_page)
@@ -1685,7 +1677,6 @@ static int __init its_setup_lpi_prop_table(void)
 		gic_rdists->prop_table_va = memremap(gic_rdists->prop_table_pa,
 						     LPI_PROPBASE_SZ,
 						     MEMREMAP_WB);
-		gic_reset_prop_table(gic_rdists->prop_table_va);
 	} else {
 		struct page *page;
 
@@ -1703,6 +1694,7 @@ static int __init its_setup_lpi_prop_table(void)
 		WARN_ON(gic_reserve_range(gic_rdists->prop_table_pa,
 					  LPI_PROPBASE_SZ));
 	}
+	gic_reset_prop_table(gic_rdists->prop_table_va);
 
 	pr_info("GICv3: using LPI property table @%pa\n",
 		&gic_rdists->prop_table_pa);
@@ -3079,6 +3071,7 @@ static int its_vpe_irq_domain_alloc(struct irq_domain *domain, unsigned int virq
 		its_lpi_free(bitmap, base, nr_ids);
 		return -ENOMEM;
 	}
+	gic_reset_prop_table(page_address(vprop_page));
 
 	vm->db_bitmap = bitmap;
 	vm->db_lpi_base = base;
-- 
2.23.0

[PATCH v1 1/6] rqchip/gic-v3-its: reset prop table outside of allocation

[Pavel Tatashin]

In preparation of adding another variant of allocation, move
the resetting outside of the current allocator.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1b5c3672aea2..ada18748ed1c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1621,15 +1621,7 @@ static void gic_reset_prop_table(void *va)
 
 static struct page *its_allocate_prop_table(gfp_t gfp_flags)
 {
-	struct page *prop_page;
-
-	prop_page = alloc_pages(gfp_flags, get_order(LPI_PROPBASE_SZ));
-	if (!prop_page)
-		return NULL;
-
-	gic_reset_prop_table(page_address(prop_page));
-
-	return prop_page;
+	return alloc_pages(gfp_flags, get_order(LPI_PROPBASE_SZ));
 }
 
 static void its_free_prop_table(struct page *prop_page)
@@ -1685,7 +1677,6 @@ static int __init its_setup_lpi_prop_table(void)
 		gic_rdists->prop_table_va = memremap(gic_rdists->prop_table_pa,
 						     LPI_PROPBASE_SZ,
 						     MEMREMAP_WB);
-		gic_reset_prop_table(gic_rdists->prop_table_va);
 	} else {
 		struct page *page;
 
@@ -1703,6 +1694,7 @@ static int __init its_setup_lpi_prop_table(void)
 		WARN_ON(gic_reserve_range(gic_rdists->prop_table_pa,
 					  LPI_PROPBASE_SZ));
 	}
+	gic_reset_prop_table(gic_rdists->prop_table_va);
 
 	pr_info("GICv3: using LPI property table @%pa\n",
 		&gic_rdists->prop_table_pa);
@@ -3079,6 +3071,7 @@ static int its_vpe_irq_domain_alloc(struct irq_domain *domain, unsigned int virq
 		its_lpi_free(bitmap, base, nr_ids);
 		return -ENOMEM;
 	}
+	gic_reset_prop_table(page_address(vprop_page));
 
 	vm->db_bitmap = bitmap;
 	vm->db_lpi_base = base;
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 1/6] rqchip/gic-v3-its: reset prop table outside of allocation

[Pavel Tatashin]

In preparation of adding another variant of allocation, move
the resetting outside of the current allocator.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1b5c3672aea2..ada18748ed1c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1621,15 +1621,7 @@ static void gic_reset_prop_table(void *va)
 
 static struct page *its_allocate_prop_table(gfp_t gfp_flags)
 {
-	struct page *prop_page;
-
-	prop_page = alloc_pages(gfp_flags, get_order(LPI_PROPBASE_SZ));
-	if (!prop_page)
-		return NULL;
-
-	gic_reset_prop_table(page_address(prop_page));
-
-	return prop_page;
+	return alloc_pages(gfp_flags, get_order(LPI_PROPBASE_SZ));
 }
 
 static void its_free_prop_table(struct page *prop_page)
@@ -1685,7 +1677,6 @@ static int __init its_setup_lpi_prop_table(void)
 		gic_rdists->prop_table_va = memremap(gic_rdists->prop_table_pa,
 						     LPI_PROPBASE_SZ,
 						     MEMREMAP_WB);
-		gic_reset_prop_table(gic_rdists->prop_table_va);
 	} else {
 		struct page *page;
 
@@ -1703,6 +1694,7 @@ static int __init its_setup_lpi_prop_table(void)
 		WARN_ON(gic_reserve_range(gic_rdists->prop_table_pa,
 					  LPI_PROPBASE_SZ));
 	}
+	gic_reset_prop_table(gic_rdists->prop_table_va);
 
 	pr_info("GICv3: using LPI property table @%pa\n",
 		&gic_rdists->prop_table_pa);
@@ -3079,6 +3071,7 @@ static int its_vpe_irq_domain_alloc(struct irq_domain *domain, unsigned int virq
 		its_lpi_free(bitmap, base, nr_ids);
 		return -ENOMEM;
 	}
+	gic_reset_prop_table(page_address(vprop_page));
 
 	vm->db_bitmap = bitmap;
 	vm->db_lpi_base = base;
-- 
2.23.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v1 2/6] rqchip/gic-v3-its: use temporary va / pa variables

[Pavel Tatashin]

This is a cleanup, that will help later when a variant that does not
require memremap is added.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ada18748ed1c..656b6c6e1bf8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1668,15 +1668,17 @@ static int gic_reserve_range(phys_addr_t addr, unsigned long size)
 static int __init its_setup_lpi_prop_table(void)
 {
 	if (gic_rdists->flags & RDIST_FLAGS_RD_TABLES_PREALLOCATED) {
+		unsigned long pa;
 		u64 val;
+		void *va;
 
 		val = gicr_read_propbaser(gic_data_rdist_rd_base() + GICR_PROPBASER);
 		lpi_id_bits = (val & GICR_PROPBASER_IDBITS_MASK) + 1;
 
-		gic_rdists->prop_table_pa = val & GENMASK_ULL(51, 12);
-		gic_rdists->prop_table_va = memremap(gic_rdists->prop_table_pa,
-						     LPI_PROPBASE_SZ,
-						     MEMREMAP_WB);
+		pa = val & GENMASK_ULL(51, 12);
+		va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
+		gic_rdists->prop_table_pa = pa;
+		gic_rdists->prop_table_va = va;
 	} else {
 		struct page *page;
 
-- 
2.23.0

[PATCH v1 2/6] rqchip/gic-v3-its: use temporary va / pa variables

[Pavel Tatashin]

This is a cleanup, that will help later when a variant that does not
require memremap is added.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ada18748ed1c..656b6c6e1bf8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1668,15 +1668,17 @@ static int gic_reserve_range(phys_addr_t addr, unsigned long size)
 static int __init its_setup_lpi_prop_table(void)
 {
 	if (gic_rdists->flags & RDIST_FLAGS_RD_TABLES_PREALLOCATED) {
+		unsigned long pa;
 		u64 val;
+		void *va;
 
 		val = gicr_read_propbaser(gic_data_rdist_rd_base() + GICR_PROPBASER);
 		lpi_id_bits = (val & GICR_PROPBASER_IDBITS_MASK) + 1;
 
-		gic_rdists->prop_table_pa = val & GENMASK_ULL(51, 12);
-		gic_rdists->prop_table_va = memremap(gic_rdists->prop_table_pa,
-						     LPI_PROPBASE_SZ,
-						     MEMREMAP_WB);
+		pa = val & GENMASK_ULL(51, 12);
+		va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
+		gic_rdists->prop_table_pa = pa;
+		gic_rdists->prop_table_va = va;
 	} else {
 		struct page *page;
 
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 2/6] rqchip/gic-v3-its: use temporary va / pa variables

[Pavel Tatashin]

This is a cleanup, that will help later when a variant that does not
require memremap is added.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ada18748ed1c..656b6c6e1bf8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1668,15 +1668,17 @@ static int gic_reserve_range(phys_addr_t addr, unsigned long size)
 static int __init its_setup_lpi_prop_table(void)
 {
 	if (gic_rdists->flags & RDIST_FLAGS_RD_TABLES_PREALLOCATED) {
+		unsigned long pa;
 		u64 val;
+		void *va;
 
 		val = gicr_read_propbaser(gic_data_rdist_rd_base() + GICR_PROPBASER);
 		lpi_id_bits = (val & GICR_PROPBASER_IDBITS_MASK) + 1;
 
-		gic_rdists->prop_table_pa = val & GENMASK_ULL(51, 12);
-		gic_rdists->prop_table_va = memremap(gic_rdists->prop_table_pa,
-						     LPI_PROPBASE_SZ,
-						     MEMREMAP_WB);
+		pa = val & GENMASK_ULL(51, 12);
+		va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
+		gic_rdists->prop_table_pa = pa;
+		gic_rdists->prop_table_va = va;
 	} else {
 		struct page *page;
 
-- 
2.23.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v1 3/6] rqchip/gic-v3-its: add reset pending table function

[Pavel Tatashin]

Add function that is similar to gic_reset_prop_table but for pending
table.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 656b6c6e1bf8..124e2cb890cd 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1989,17 +1989,23 @@ static int its_alloc_collections(struct its_node *its)
 	return 0;
 }
 
+static void gic_reset_pending_table(void *va)
+{
+	memset(va, 0, LPI_PENDBASE_SZ);
+
+	/* Make sure the GIC will observe the zero-ed page */
+	gic_flush_dcache_to_poc(va, LPI_PENDBASE_SZ);
+}
+
 static struct page *its_allocate_pending_table(gfp_t gfp_flags)
 {
 	struct page *pend_page;
 
-	pend_page = alloc_pages(gfp_flags | __GFP_ZERO,
-				get_order(LPI_PENDBASE_SZ));
+	pend_page = alloc_pages(gfp_flags, get_order(LPI_PENDBASE_SZ));
 	if (!pend_page)
 		return NULL;
 
-	/* Make sure the GIC will observe the zero-ed page */
-	gic_flush_dcache_to_poc(page_address(pend_page), LPI_PENDBASE_SZ);
+	gic_reset_pending_table(page_address(pend_page));
 
 	return pend_page;
 }
-- 
2.23.0

[PATCH v1 3/6] rqchip/gic-v3-its: add reset pending table function

[Pavel Tatashin]

Add function that is similar to gic_reset_prop_table but for pending
table.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 656b6c6e1bf8..124e2cb890cd 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1989,17 +1989,23 @@ static int its_alloc_collections(struct its_node *its)
 	return 0;
 }
 
+static void gic_reset_pending_table(void *va)
+{
+	memset(va, 0, LPI_PENDBASE_SZ);
+
+	/* Make sure the GIC will observe the zero-ed page */
+	gic_flush_dcache_to_poc(va, LPI_PENDBASE_SZ);
+}
+
 static struct page *its_allocate_pending_table(gfp_t gfp_flags)
 {
 	struct page *pend_page;
 
-	pend_page = alloc_pages(gfp_flags | __GFP_ZERO,
-				get_order(LPI_PENDBASE_SZ));
+	pend_page = alloc_pages(gfp_flags, get_order(LPI_PENDBASE_SZ));
 	if (!pend_page)
 		return NULL;
 
-	/* Make sure the GIC will observe the zero-ed page */
-	gic_flush_dcache_to_poc(page_address(pend_page), LPI_PENDBASE_SZ);
+	gic_reset_pending_table(page_address(pend_page));
 
 	return pend_page;
 }
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 3/6] rqchip/gic-v3-its: add reset pending table function

[Pavel Tatashin]

Add function that is similar to gic_reset_prop_table but for pending
table.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 656b6c6e1bf8..124e2cb890cd 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1989,17 +1989,23 @@ static int its_alloc_collections(struct its_node *its)
 	return 0;
 }
 
+static void gic_reset_pending_table(void *va)
+{
+	memset(va, 0, LPI_PENDBASE_SZ);
+
+	/* Make sure the GIC will observe the zero-ed page */
+	gic_flush_dcache_to_poc(va, LPI_PENDBASE_SZ);
+}
+
 static struct page *its_allocate_pending_table(gfp_t gfp_flags)
 {
 	struct page *pend_page;
 
-	pend_page = alloc_pages(gfp_flags | __GFP_ZERO,
-				get_order(LPI_PENDBASE_SZ));
+	pend_page = alloc_pages(gfp_flags, get_order(LPI_PENDBASE_SZ));
 	if (!pend_page)
 		return NULL;
 
-	/* Make sure the GIC will observe the zero-ed page */
-	gic_flush_dcache_to_poc(page_address(pend_page), LPI_PENDBASE_SZ);
+	gic_reset_pending_table(page_address(pend_page));
 
 	return pend_page;
 }
-- 
2.23.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v1 4/6] rqchip/gic-v3-its: move reset pending table outside of allocator

[Pavel Tatashin]

Again, in preparation of adding a new allocator, move the reset function
outside of the current allocator.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 124e2cb890cd..d5f3508ca11f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1999,15 +1999,7 @@ static void gic_reset_pending_table(void *va)
 
 static struct page *its_allocate_pending_table(gfp_t gfp_flags)
 {
-	struct page *pend_page;
-
-	pend_page = alloc_pages(gfp_flags, get_order(LPI_PENDBASE_SZ));
-	if (!pend_page)
-		return NULL;
-
-	gic_reset_pending_table(page_address(pend_page));
-
-	return pend_page;
+	return alloc_pages(gfp_flags, get_order(LPI_PENDBASE_SZ));
 }
 
 static void its_free_pending_table(struct page *pt)
@@ -2064,6 +2056,7 @@ static int __init allocate_lpi_tables(void)
 			pr_err("Failed to allocate PENDBASE for CPU%d\n", cpu);
 			return -ENOMEM;
 		}
+		gic_reset_pending_table(page_address(pend_page));
 
 		gic_data_rdist_cpu(cpu)->pend_page = pend_page;
 	}
@@ -3007,6 +3000,7 @@ static int its_vpe_init(struct its_vpe *vpe)
 		its_vpe_id_free(vpe_id);
 		return -ENOMEM;
 	}
+	gic_reset_pending_table(page_address(vpt_page));
 
 	if (!its_alloc_vpe_table(vpe_id)) {
 		its_vpe_id_free(vpe_id);
-- 
2.23.0

[PATCH v1 4/6] rqchip/gic-v3-its: move reset pending table outside of allocator

[Pavel Tatashin]

Again, in preparation of adding a new allocator, move the reset function
outside of the current allocator.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 124e2cb890cd..d5f3508ca11f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1999,15 +1999,7 @@ static void gic_reset_pending_table(void *va)
 
 static struct page *its_allocate_pending_table(gfp_t gfp_flags)
 {
-	struct page *pend_page;
-
-	pend_page = alloc_pages(gfp_flags, get_order(LPI_PENDBASE_SZ));
-	if (!pend_page)
-		return NULL;
-
-	gic_reset_pending_table(page_address(pend_page));
-
-	return pend_page;
+	return alloc_pages(gfp_flags, get_order(LPI_PENDBASE_SZ));
 }
 
 static void its_free_pending_table(struct page *pt)
@@ -2064,6 +2056,7 @@ static int __init allocate_lpi_tables(void)
 			pr_err("Failed to allocate PENDBASE for CPU%d\n", cpu);
 			return -ENOMEM;
 		}
+		gic_reset_pending_table(page_address(pend_page));
 
 		gic_data_rdist_cpu(cpu)->pend_page = pend_page;
 	}
@@ -3007,6 +3000,7 @@ static int its_vpe_init(struct its_vpe *vpe)
 		its_vpe_id_free(vpe_id);
 		return -ENOMEM;
 	}
+	gic_reset_pending_table(page_address(vpt_page));
 
 	if (!its_alloc_vpe_table(vpe_id)) {
 		its_vpe_id_free(vpe_id);
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 4/6] rqchip/gic-v3-its: move reset pending table outside of allocator

[Pavel Tatashin]

Again, in preparation of adding a new allocator, move the reset function
outside of the current allocator.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 124e2cb890cd..d5f3508ca11f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1999,15 +1999,7 @@ static void gic_reset_pending_table(void *va)
 
 static struct page *its_allocate_pending_table(gfp_t gfp_flags)
 {
-	struct page *pend_page;
-
-	pend_page = alloc_pages(gfp_flags, get_order(LPI_PENDBASE_SZ));
-	if (!pend_page)
-		return NULL;
-
-	gic_reset_pending_table(page_address(pend_page));
-
-	return pend_page;
+	return alloc_pages(gfp_flags, get_order(LPI_PENDBASE_SZ));
 }
 
 static void its_free_pending_table(struct page *pt)
@@ -2064,6 +2056,7 @@ static int __init allocate_lpi_tables(void)
 			pr_err("Failed to allocate PENDBASE for CPU%d\n", cpu);
 			return -ENOMEM;
 		}
+		gic_reset_pending_table(page_address(pend_page));
 
 		gic_data_rdist_cpu(cpu)->pend_page = pend_page;
 	}
@@ -3007,6 +3000,7 @@ static int its_vpe_init(struct its_vpe *vpe)
 		its_vpe_id_free(vpe_id);
 		return -ENOMEM;
 	}
+	gic_reset_pending_table(page_address(vpt_page));
 
 	if (!its_alloc_vpe_table(vpe_id)) {
 		its_vpe_id_free(vpe_id);
-- 
2.23.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v1 5/6] rqchip/gic-v3-its: move reset pending table outside of allocator

[Pavel Tatashin]

Allow to use reserved memory for interrupt controller tables.

Currently, it is not possible to do kexec reboots without possible memory
corruption using device tree and GICv3 interrupt controller.

GICv3 can be configured once during boot, and location of tables cannot
be changed thereafter.

The fix is to allow to reserve memory region in interrupt controller device
property, and use it to do allocations.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 82 ++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index d5f3508ca11f..aeda8760cc4e 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -47,6 +47,54 @@
 
 static u32 lpi_id_bits;
 
+/*
+ * Describes reserved memory region in interrupt controller.
+ * The memory reserved: [pa_start, pa_end)
+ */
+struct of_resv {
+	unsigned long pa_start;
+	unsigned long pa_end;
+};
+
+static struct page __init *get_of_page(struct of_resv *resv, unsigned long size)
+{
+	unsigned long pa = ALIGN(resv->pa_start, size);
+	unsigned long pa_next = pa + size;
+
+	/* Check if there is enough memory reserved to do another allocation */
+	if (pa_next > resv->pa_end)
+		return NULL;
+
+	resv->pa_start = pa_next;
+	memset(phys_to_virt(pa), 0, size);
+
+	return phys_to_page(pa);
+}
+
+/*
+ * Memory controller might have a reserved memory region to be used for table
+ * allocations. This is a requirement for kexec reboots.
+ */
+static void __init its_of_mem_region(struct device_node *node,
+				     struct of_resv **resv,
+				     struct of_resv *resv_buf)
+{
+	struct device_node *np = of_parse_phandle(node, "memory-region", 0);
+	struct resource mem_res;
+
+	if (!np)
+		return;
+
+	if (of_address_to_resource(np, 0, &mem_res)) {
+		pr_warn("%pOF: address to resource failed\n", np);
+		return;
+	}
+
+	resv_buf->pa_start = mem_res.start;
+	resv_buf->pa_end = mem_res.start + resource_size(&mem_res);
+	*resv = resv_buf;
+}
+
 /*
  * We allocate memory for PROPBASE to cover 2 ^ lpi_id_bits LPIs to
  * deal with (one configuration byte per interrupt). PENDBASE has to
@@ -1665,7 +1713,7 @@ static int gic_reserve_range(phys_addr_t addr, unsigned long size)
 	return 0;
 }
 
-static int __init its_setup_lpi_prop_table(void)
+static int __init its_setup_lpi_prop_table(struct of_resv *resv)
 {
 	if (gic_rdists->flags & RDIST_FLAGS_RD_TABLES_PREALLOCATED) {
 		unsigned long pa;
@@ -1676,7 +1724,10 @@ static int __init its_setup_lpi_prop_table(void)
 		lpi_id_bits = (val & GICR_PROPBASER_IDBITS_MASK) + 1;
 
 		pa = val & GENMASK_ULL(51, 12);
-		va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
+		if (resv)
+			va = phys_to_virt(pa);
+		else
+			va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
 		gic_rdists->prop_table_pa = pa;
 		gic_rdists->prop_table_va = va;
 	} else {
@@ -1685,7 +1736,10 @@ static int __init its_setup_lpi_prop_table(void)
 		lpi_id_bits = min_t(u32,
 				    GICD_TYPER_ID_BITS(gic_rdists->gicd_typer),
 				    ITS_MAX_LPI_NRBITS);
-		page = its_allocate_prop_table(GFP_NOWAIT);
+		if (resv)
+			page = get_of_page(resv, LPI_PROPBASE_SZ);
+		else
+			page = its_allocate_prop_table(GFP_NOWAIT);
 		if (!page) {
 			pr_err("Failed to allocate PROPBASE\n");
 			return -ENOMEM;
@@ -2009,7 +2063,8 @@ static void its_free_pending_table(struct page *pt)
 
 /*
  * Booting with kdump and LPIs enabled is generally fine. Any other
- * case is wrong in the absence of firmware/EFI support.
+ * case is wrong in the absence of firmware/EFI support or reserve-memory
+ * in device tree for interrupt controller.
  */
 static bool enabled_lpis_allowed(void)
 {
@@ -2023,7 +2078,7 @@ static bool enabled_lpis_allowed(void)
 	return gic_check_reserved_range(addr, LPI_PROPBASE_SZ);
 }
 
-static int __init allocate_lpi_tables(void)
+static int __init allocate_lpi_tables(struct of_resv *resv)
 {
 	u64 val;
 	int err, cpu;
@@ -2039,7 +2094,7 @@ static int __init allocate_lpi_tables(void)
 		pr_info("GICv3: Using preallocated redistributor tables\n");
 	}
 
-	err = its_setup_lpi_prop_table();
+	err = its_setup_lpi_prop_table(resv);
 	if (err)
 		return err;
 
@@ -2051,7 +2106,10 @@ static int __init allocate_lpi_tables(void)
 	for_each_possible_cpu(cpu) {
 		struct page *pend_page;
 
-		pend_page = its_allocate_pending_table(GFP_NOWAIT);
+		if (resv)
+			pend_page = get_of_page(resv, LPI_PENDBASE_SZ);
+		else
+			pend_page = its_allocate_pending_table(GFP_NOWAIT);
 		if (!pend_page) {
 			pr_err("Failed to allocate PENDBASE for CPU%d\n", cpu);
 			return -ENOMEM;
@@ -3957,16 +4015,20 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 		    struct irq_domain *parent_domain)
 {
 	struct device_node *of_node;
+	struct of_resv resv_buf;
+	struct of_resv *resv = NULL;
 	struct its_node *its;
 	bool has_v4 = false;
 	int err;
 
 	its_parent = parent_domain;
 	of_node = to_of_node(handle);
-	if (of_node)
+	if (of_node) {
 		its_of_probe(of_node);
-	else
+		its_of_mem_region(of_node, &resv, &resv_buf);
+	} else {
 		its_acpi_probe();
+	}
 
 	if (list_empty(&its_nodes)) {
 		pr_warn("ITS: No ITS available, not enabling LPIs\n");
@@ -3975,7 +4037,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 
 	gic_rdists = rdists;
 
-	err = allocate_lpi_tables();
+	err = allocate_lpi_tables(resv);
 	if (err)
 		return err;
 
-- 
2.23.0

[PATCH v1 5/6] rqchip/gic-v3-its: move reset pending table outside of allocator

[Pavel Tatashin]

Allow to use reserved memory for interrupt controller tables.

Currently, it is not possible to do kexec reboots without possible memory
corruption using device tree and GICv3 interrupt controller.

GICv3 can be configured once during boot, and location of tables cannot
be changed thereafter.

The fix is to allow to reserve memory region in interrupt controller device
property, and use it to do allocations.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 82 ++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index d5f3508ca11f..aeda8760cc4e 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -47,6 +47,54 @@
 
 static u32 lpi_id_bits;
 
+/*
+ * Describes reserved memory region in interrupt controller.
+ * The memory reserved: [pa_start, pa_end)
+ */
+struct of_resv {
+	unsigned long pa_start;
+	unsigned long pa_end;
+};
+
+static struct page __init *get_of_page(struct of_resv *resv, unsigned long size)
+{
+	unsigned long pa = ALIGN(resv->pa_start, size);
+	unsigned long pa_next = pa + size;
+
+	/* Check if there is enough memory reserved to do another allocation */
+	if (pa_next > resv->pa_end)
+		return NULL;
+
+	resv->pa_start = pa_next;
+	memset(phys_to_virt(pa), 0, size);
+
+	return phys_to_page(pa);
+}
+
+/*
+ * Memory controller might have a reserved memory region to be used for table
+ * allocations. This is a requirement for kexec reboots.
+ */
+static void __init its_of_mem_region(struct device_node *node,
+				     struct of_resv **resv,
+				     struct of_resv *resv_buf)
+{
+	struct device_node *np = of_parse_phandle(node, "memory-region", 0);
+	struct resource mem_res;
+
+	if (!np)
+		return;
+
+	if (of_address_to_resource(np, 0, &mem_res)) {
+		pr_warn("%pOF: address to resource failed\n", np);
+		return;
+	}
+
+	resv_buf->pa_start = mem_res.start;
+	resv_buf->pa_end = mem_res.start + resource_size(&mem_res);
+	*resv = resv_buf;
+}
+
 /*
  * We allocate memory for PROPBASE to cover 2 ^ lpi_id_bits LPIs to
  * deal with (one configuration byte per interrupt). PENDBASE has to
@@ -1665,7 +1713,7 @@ static int gic_reserve_range(phys_addr_t addr, unsigned long size)
 	return 0;
 }
 
-static int __init its_setup_lpi_prop_table(void)
+static int __init its_setup_lpi_prop_table(struct of_resv *resv)
 {
 	if (gic_rdists->flags & RDIST_FLAGS_RD_TABLES_PREALLOCATED) {
 		unsigned long pa;
@@ -1676,7 +1724,10 @@ static int __init its_setup_lpi_prop_table(void)
 		lpi_id_bits = (val & GICR_PROPBASER_IDBITS_MASK) + 1;
 
 		pa = val & GENMASK_ULL(51, 12);
-		va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
+		if (resv)
+			va = phys_to_virt(pa);
+		else
+			va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
 		gic_rdists->prop_table_pa = pa;
 		gic_rdists->prop_table_va = va;
 	} else {
@@ -1685,7 +1736,10 @@ static int __init its_setup_lpi_prop_table(void)
 		lpi_id_bits = min_t(u32,
 				    GICD_TYPER_ID_BITS(gic_rdists->gicd_typer),
 				    ITS_MAX_LPI_NRBITS);
-		page = its_allocate_prop_table(GFP_NOWAIT);
+		if (resv)
+			page = get_of_page(resv, LPI_PROPBASE_SZ);
+		else
+			page = its_allocate_prop_table(GFP_NOWAIT);
 		if (!page) {
 			pr_err("Failed to allocate PROPBASE\n");
 			return -ENOMEM;
@@ -2009,7 +2063,8 @@ static void its_free_pending_table(struct page *pt)
 
 /*
  * Booting with kdump and LPIs enabled is generally fine. Any other
- * case is wrong in the absence of firmware/EFI support.
+ * case is wrong in the absence of firmware/EFI support or reserve-memory
+ * in device tree for interrupt controller.
  */
 static bool enabled_lpis_allowed(void)
 {
@@ -2023,7 +2078,7 @@ static bool enabled_lpis_allowed(void)
 	return gic_check_reserved_range(addr, LPI_PROPBASE_SZ);
 }
 
-static int __init allocate_lpi_tables(void)
+static int __init allocate_lpi_tables(struct of_resv *resv)
 {
 	u64 val;
 	int err, cpu;
@@ -2039,7 +2094,7 @@ static int __init allocate_lpi_tables(void)
 		pr_info("GICv3: Using preallocated redistributor tables\n");
 	}
 
-	err = its_setup_lpi_prop_table();
+	err = its_setup_lpi_prop_table(resv);
 	if (err)
 		return err;
 
@@ -2051,7 +2106,10 @@ static int __init allocate_lpi_tables(void)
 	for_each_possible_cpu(cpu) {
 		struct page *pend_page;
 
-		pend_page = its_allocate_pending_table(GFP_NOWAIT);
+		if (resv)
+			pend_page = get_of_page(resv, LPI_PENDBASE_SZ);
+		else
+			pend_page = its_allocate_pending_table(GFP_NOWAIT);
 		if (!pend_page) {
 			pr_err("Failed to allocate PENDBASE for CPU%d\n", cpu);
 			return -ENOMEM;
@@ -3957,16 +4015,20 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 		    struct irq_domain *parent_domain)
 {
 	struct device_node *of_node;
+	struct of_resv resv_buf;
+	struct of_resv *resv = NULL;
 	struct its_node *its;
 	bool has_v4 = false;
 	int err;
 
 	its_parent = parent_domain;
 	of_node = to_of_node(handle);
-	if (of_node)
+	if (of_node) {
 		its_of_probe(of_node);
-	else
+		its_of_mem_region(of_node, &resv, &resv_buf);
+	} else {
 		its_acpi_probe();
+	}
 
 	if (list_empty(&its_nodes)) {
 		pr_warn("ITS: No ITS available, not enabling LPIs\n");
@@ -3975,7 +4037,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 
 	gic_rdists = rdists;
 
-	err = allocate_lpi_tables();
+	err = allocate_lpi_tables(resv);
 	if (err)
 		return err;
 
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 5/6] rqchip/gic-v3-its: move reset pending table outside of allocator

[Pavel Tatashin]

Allow to use reserved memory for interrupt controller tables.

Currently, it is not possible to do kexec reboots without possible memory
corruption using device tree and GICv3 interrupt controller.

GICv3 can be configured once during boot, and location of tables cannot
be changed thereafter.

The fix is to allow to reserve memory region in interrupt controller device
property, and use it to do allocations.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 82 ++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index d5f3508ca11f..aeda8760cc4e 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -47,6 +47,54 @@
 
 static u32 lpi_id_bits;
 
+/*
+ * Describes reserved memory region in interrupt controller.
+ * The memory reserved: [pa_start, pa_end)
+ */
+struct of_resv {
+	unsigned long pa_start;
+	unsigned long pa_end;
+};
+
+static struct page __init *get_of_page(struct of_resv *resv, unsigned long size)
+{
+	unsigned long pa = ALIGN(resv->pa_start, size);
+	unsigned long pa_next = pa + size;
+
+	/* Check if there is enough memory reserved to do another allocation */
+	if (pa_next > resv->pa_end)
+		return NULL;
+
+	resv->pa_start = pa_next;
+	memset(phys_to_virt(pa), 0, size);
+
+	return phys_to_page(pa);
+}
+
+/*
+ * Memory controller might have a reserved memory region to be used for table
+ * allocations. This is a requirement for kexec reboots.
+ */
+static void __init its_of_mem_region(struct device_node *node,
+				     struct of_resv **resv,
+				     struct of_resv *resv_buf)
+{
+	struct device_node *np = of_parse_phandle(node, "memory-region", 0);
+	struct resource mem_res;
+
+	if (!np)
+		return;
+
+	if (of_address_to_resource(np, 0, &mem_res)) {
+		pr_warn("%pOF: address to resource failed\n", np);
+		return;
+	}
+
+	resv_buf->pa_start = mem_res.start;
+	resv_buf->pa_end = mem_res.start + resource_size(&mem_res);
+	*resv = resv_buf;
+}
+
 /*
  * We allocate memory for PROPBASE to cover 2 ^ lpi_id_bits LPIs to
  * deal with (one configuration byte per interrupt). PENDBASE has to
@@ -1665,7 +1713,7 @@ static int gic_reserve_range(phys_addr_t addr, unsigned long size)
 	return 0;
 }
 
-static int __init its_setup_lpi_prop_table(void)
+static int __init its_setup_lpi_prop_table(struct of_resv *resv)
 {
 	if (gic_rdists->flags & RDIST_FLAGS_RD_TABLES_PREALLOCATED) {
 		unsigned long pa;
@@ -1676,7 +1724,10 @@ static int __init its_setup_lpi_prop_table(void)
 		lpi_id_bits = (val & GICR_PROPBASER_IDBITS_MASK) + 1;
 
 		pa = val & GENMASK_ULL(51, 12);
-		va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
+		if (resv)
+			va = phys_to_virt(pa);
+		else
+			va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
 		gic_rdists->prop_table_pa = pa;
 		gic_rdists->prop_table_va = va;
 	} else {
@@ -1685,7 +1736,10 @@ static int __init its_setup_lpi_prop_table(void)
 		lpi_id_bits = min_t(u32,
 				    GICD_TYPER_ID_BITS(gic_rdists->gicd_typer),
 				    ITS_MAX_LPI_NRBITS);
-		page = its_allocate_prop_table(GFP_NOWAIT);
+		if (resv)
+			page = get_of_page(resv, LPI_PROPBASE_SZ);
+		else
+			page = its_allocate_prop_table(GFP_NOWAIT);
 		if (!page) {
 			pr_err("Failed to allocate PROPBASE\n");
 			return -ENOMEM;
@@ -2009,7 +2063,8 @@ static void its_free_pending_table(struct page *pt)
 
 /*
  * Booting with kdump and LPIs enabled is generally fine. Any other
- * case is wrong in the absence of firmware/EFI support.
+ * case is wrong in the absence of firmware/EFI support or reserve-memory
+ * in device tree for interrupt controller.
  */
 static bool enabled_lpis_allowed(void)
 {
@@ -2023,7 +2078,7 @@ static bool enabled_lpis_allowed(void)
 	return gic_check_reserved_range(addr, LPI_PROPBASE_SZ);
 }
 
-static int __init allocate_lpi_tables(void)
+static int __init allocate_lpi_tables(struct of_resv *resv)
 {
 	u64 val;
 	int err, cpu;
@@ -2039,7 +2094,7 @@ static int __init allocate_lpi_tables(void)
 		pr_info("GICv3: Using preallocated redistributor tables\n");
 	}
 
-	err = its_setup_lpi_prop_table();
+	err = its_setup_lpi_prop_table(resv);
 	if (err)
 		return err;
 
@@ -2051,7 +2106,10 @@ static int __init allocate_lpi_tables(void)
 	for_each_possible_cpu(cpu) {
 		struct page *pend_page;
 
-		pend_page = its_allocate_pending_table(GFP_NOWAIT);
+		if (resv)
+			pend_page = get_of_page(resv, LPI_PENDBASE_SZ);
+		else
+			pend_page = its_allocate_pending_table(GFP_NOWAIT);
 		if (!pend_page) {
 			pr_err("Failed to allocate PENDBASE for CPU%d\n", cpu);
 			return -ENOMEM;
@@ -3957,16 +4015,20 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 		    struct irq_domain *parent_domain)
 {
 	struct device_node *of_node;
+	struct of_resv resv_buf;
+	struct of_resv *resv = NULL;
 	struct its_node *its;
 	bool has_v4 = false;
 	int err;
 
 	its_parent = parent_domain;
 	of_node = to_of_node(handle);
-	if (of_node)
+	if (of_node) {
 		its_of_probe(of_node);
-	else
+		its_of_mem_region(of_node, &resv, &resv_buf);
+	} else {
 		its_acpi_probe();
+	}
 
 	if (list_empty(&its_nodes)) {
 		pr_warn("ITS: No ITS available, not enabling LPIs\n");
@@ -3975,7 +4037,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 
 	gic_rdists = rdists;
 
-	err = allocate_lpi_tables();
+	err = allocate_lpi_tables(resv);
 	if (err)
 		return err;
 
-- 
2.23.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v1 6/6] dt-bindings: interrupt-controller: add optional memory-region

[Pavel Tatashin]

Allow pre-reserve memory in device tree that can be used in interrupt
controller tabes. This memory is required when kexec functionality is needed
with GICv3 controler and device trees.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 .../bindings/interrupt-controller/arm,gic-v3.yaml          | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
index c34df35a25fc..7640aaa97302 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
@@ -102,6 +102,13 @@ properties:
       - $ref: /schemas/types.yaml#/definitions/uint32
       - maximum: 4096   # Should be enough?
 
+  memory-region:
+    description:
+      Memory used to allocate property and pending tables.
+      Required if kexec functionality is needed.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint64
+
   msi-controller:
     description:
       Only present if the Message Based Interrupt functionnality is
-- 
2.23.0

[PATCH v1 6/6] dt-bindings: interrupt-controller: add optional memory-region

[Pavel Tatashin]

Allow pre-reserve memory in device tree that can be used in interrupt
controller tabes. This memory is required when kexec functionality is needed
with GICv3 controler and device trees.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 .../bindings/interrupt-controller/arm,gic-v3.yaml          | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
index c34df35a25fc..7640aaa97302 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
@@ -102,6 +102,13 @@ properties:
       - $ref: /schemas/types.yaml#/definitions/uint32
       - maximum: 4096   # Should be enough?
 
+  memory-region:
+    description:
+      Memory used to allocate property and pending tables.
+      Required if kexec functionality is needed.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint64
+
   msi-controller:
     description:
       Only present if the Message Based Interrupt functionnality is
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 6/6] dt-bindings: interrupt-controller: add optional memory-region

[Pavel Tatashin]

Allow pre-reserve memory in device tree that can be used in interrupt
controller tabes. This memory is required when kexec functionality is needed
with GICv3 controler and device trees.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 .../bindings/interrupt-controller/arm,gic-v3.yaml          | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
index c34df35a25fc..7640aaa97302 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
@@ -102,6 +102,13 @@ properties:
       - $ref: /schemas/types.yaml#/definitions/uint32
       - maximum: 4096   # Should be enough?
 
+  memory-region:
+    description:
+      Memory used to allocate property and pending tables.
+      Required if kexec functionality is needed.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint64
+
   msi-controller:
     description:
       Only present if the Message Based Interrupt functionnality is
-- 
2.23.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Marc Zyngier]

On Mon, 26 Aug 2019 15:00:50 -0400
Pavel Tatashin <pasha.tatashin@soleen.com> wrote:

> Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
> However, it is still not possible todo on systems with device trees.
> 
> Here is EFI fixes from Marc:
> https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com
> 
> For Device Tree variant: lets allow reserve a memory region in interrupt
> controller node, and use this property to allocate interrupt tables.

There is no such thing as a "device tree variant". As long as your
bootloader implements EFI, everything will work correctly, whether
you're using DT, ACPI, or the anything else.

This already works today, without any need to add anything to the
kernel (I have systems using EDK II and u-boot, both implementing EFI,
and I'm able to kexec without any issue). If your bootloader doesn't
support EFI, here's a good opportunity to implement it!

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Marc Zyngier]

On Mon, 26 Aug 2019 15:00:50 -0400
Pavel Tatashin <pasha.tatashin@soleen.com> wrote:

> Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
> However, it is still not possible todo on systems with device trees.
> 
> Here is EFI fixes from Marc:
> https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com
> 
> For Device Tree variant: lets allow reserve a memory region in interrupt
> controller node, and use this property to allocate interrupt tables.

There is no such thing as a "device tree variant". As long as your
bootloader implements EFI, everything will work correctly, whether
you're using DT, ACPI, or the anything else.

This already works today, without any need to add anything to the
kernel (I have systems using EDK II and u-boot, both implementing EFI,
and I'm able to kexec without any issue). If your bootloader doesn't
support EFI, here's a good opportunity to implement it!

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Marc Zyngier]

On Mon, 26 Aug 2019 15:00:50 -0400
Pavel Tatashin <pasha.tatashin@soleen.com> wrote:

> Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
> However, it is still not possible todo on systems with device trees.
> 
> Here is EFI fixes from Marc:
> https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com
> 
> For Device Tree variant: lets allow reserve a memory region in interrupt
> controller node, and use this property to allocate interrupt tables.

There is no such thing as a "device tree variant". As long as your
bootloader implements EFI, everything will work correctly, whether
you're using DT, ACPI, or the anything else.

This already works today, without any need to add anything to the
kernel (I have systems using EDK II and u-boot, both implementing EFI,
and I'm able to kexec without any issue). If your bootloader doesn't
support EFI, here's a good opportunity to implement it!

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Pavel Tatashin]

On Mon, Aug 26, 2019 at 3:13 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 26 Aug 2019 15:00:50 -0400
> Pavel Tatashin <pasha.tatashin@soleen.com> wrote:
>
> > Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
> > However, it is still not possible todo on systems with device trees.
> >
> > Here is EFI fixes from Marc:
> > https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com
> >
> > For Device Tree variant: lets allow reserve a memory region in interrupt
> > controller node, and use this property to allocate interrupt tables.
>
> There is no such thing as a "device tree variant". As long as your
> bootloader implements EFI, everything will work correctly, whether
> you're using DT, ACPI, or the anything else.
>
> This already works today, without any need to add anything to the
> kernel (I have systems using EDK II and u-boot, both implementing EFI,
> and I'm able to kexec without any issue). If your bootloader doesn't
> support EFI, here's a good opportunity to implement it!

Hi Marc,

Thank you very much for looking at this work.

Running Linux without EFI is common, and there are scenarios which
make it appropriate. As I understand most of embedded linux do not
have EFI enabled, and thus I do not see a reason why we would not
support a first class feature of Linux (kexec) on non-EFI bootloaders.

We (Microsoft) have a small highly secure device with a high uptime
requirement. The device also has PCIe and thus GICv3. The update for
this device relies on kexec. For a number of reasons, it was decided
to use U-Boot and Linux without EFI enabled. One of those reasons is
to improve boot performance, enabling EFI in U-Boot alone reduces the
boot performance by half a second. Our total reboot budget is under a
second which makes that half a second unacceptable. Also, adding EFI
support to kernel increases its size and there are security
implications from enabling more code both in U-Boot and Linux.

> --
> Without deviation from the norm, progress is not possible.

Totally agreed.

Thank you,
Pasha

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Pavel Tatashin]

On Mon, Aug 26, 2019 at 3:13 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 26 Aug 2019 15:00:50 -0400
> Pavel Tatashin <pasha.tatashin@soleen.com> wrote:
>
> > Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
> > However, it is still not possible todo on systems with device trees.
> >
> > Here is EFI fixes from Marc:
> > https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com
> >
> > For Device Tree variant: lets allow reserve a memory region in interrupt
> > controller node, and use this property to allocate interrupt tables.
>
> There is no such thing as a "device tree variant". As long as your
> bootloader implements EFI, everything will work correctly, whether
> you're using DT, ACPI, or the anything else.
>
> This already works today, without any need to add anything to the
> kernel (I have systems using EDK II and u-boot, both implementing EFI,
> and I'm able to kexec without any issue). If your bootloader doesn't
> support EFI, here's a good opportunity to implement it!

Hi Marc,

Thank you very much for looking at this work.

Running Linux without EFI is common, and there are scenarios which
make it appropriate. As I understand most of embedded linux do not
have EFI enabled, and thus I do not see a reason why we would not
support a first class feature of Linux (kexec) on non-EFI bootloaders.

We (Microsoft) have a small highly secure device with a high uptime
requirement. The device also has PCIe and thus GICv3. The update for
this device relies on kexec. For a number of reasons, it was decided
to use U-Boot and Linux without EFI enabled. One of those reasons is
to improve boot performance, enabling EFI in U-Boot alone reduces the
boot performance by half a second. Our total reboot budget is under a
second which makes that half a second unacceptable. Also, adding EFI
support to kernel increases its size and there are security
implications from enabling more code both in U-Boot and Linux.

> --
> Without deviation from the norm, progress is not possible.

Totally agreed.

Thank you,
Pasha

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Pavel Tatashin]

On Mon, Aug 26, 2019 at 3:13 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 26 Aug 2019 15:00:50 -0400
> Pavel Tatashin <pasha.tatashin@soleen.com> wrote:
>
> > Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
> > However, it is still not possible todo on systems with device trees.
> >
> > Here is EFI fixes from Marc:
> > https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com
> >
> > For Device Tree variant: lets allow reserve a memory region in interrupt
> > controller node, and use this property to allocate interrupt tables.
>
> There is no such thing as a "device tree variant". As long as your
> bootloader implements EFI, everything will work correctly, whether
> you're using DT, ACPI, or the anything else.
>
> This already works today, without any need to add anything to the
> kernel (I have systems using EDK II and u-boot, both implementing EFI,
> and I'm able to kexec without any issue). If your bootloader doesn't
> support EFI, here's a good opportunity to implement it!

Hi Marc,

Thank you very much for looking at this work.

Running Linux without EFI is common, and there are scenarios which
make it appropriate. As I understand most of embedded linux do not
have EFI enabled, and thus I do not see a reason why we would not
support a first class feature of Linux (kexec) on non-EFI bootloaders.

We (Microsoft) have a small highly secure device with a high uptime
requirement. The device also has PCIe and thus GICv3. The update for
this device relies on kexec. For a number of reasons, it was decided
to use U-Boot and Linux without EFI enabled. One of those reasons is
to improve boot performance, enabling EFI in U-Boot alone reduces the
boot performance by half a second. Our total reboot budget is under a
second which makes that half a second unacceptable. Also, adding EFI
support to kernel increases its size and there are security
implications from enabling more code both in U-Boot and Linux.

> --
> Without deviation from the norm, progress is not possible.

Totally agreed.

Thank you,
Pasha

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 5/6] rqchip/gic-v3-its: move reset pending table outside of allocator

[Pavel Tatashin]

This patch requires a small fix (which I will do in later revisions):

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 2ffdb3927549..c9faeac4b3a8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2182,7 +2182,8 @@ static void its_cpu_init_lpis(void)
                paddr &= GENMASK_ULL(51, 16);

                WARN_ON(!gic_check_reserved_range(paddr, LPI_PENDBASE_SZ));
-               its_free_pending_table(gic_data_rdist()->pend_page);
+               if (efi_enabled(EFI_CONFIG_TABLES))
+                       its_free_pending_table(gic_data_rdist()->pend_page);
                gic_data_rdist()->pend_page = NULL;

                goto out;

reserved-memory does not need to be freed. However, I am confused why
it is needed to be freed in EFI case. Marc, can you please explain
this to me?

Thank you,
Pasha

On Mon, Aug 26, 2019 at 3:01 PM Pavel Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> Allow to use reserved memory for interrupt controller tables.
>
> Currently, it is not possible to do kexec reboots without possible memory
> corruption using device tree and GICv3 interrupt controller.
>
> GICv3 can be configured once during boot, and location of tables cannot
> be changed thereafter.
>
> The fix is to allow to reserve memory region in interrupt controller device
> property, and use it to do allocations.
>
> Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 82 ++++++++++++++++++++++++++++----
>  1 file changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index d5f3508ca11f..aeda8760cc4e 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -47,6 +47,54 @@
>
>  static u32 lpi_id_bits;
>
> +/*
> + * Describes reserved memory region in interrupt controller.
> + * The memory reserved: [pa_start, pa_end)
> + */
> +struct of_resv {
> +       unsigned long pa_start;
> +       unsigned long pa_end;
> +};
> +
> +static struct page __init *get_of_page(struct of_resv *resv, unsigned long size)
> +{
> +       unsigned long pa = ALIGN(resv->pa_start, size);
> +       unsigned long pa_next = pa + size;
> +
> +       /* Check if there is enough memory reserved to do another allocation */
> +       if (pa_next > resv->pa_end)
> +               return NULL;
> +
> +       resv->pa_start = pa_next;
> +       memset(phys_to_virt(pa), 0, size);
> +
> +       return phys_to_page(pa);
> +}
> +
> +/*
> + * Memory controller might have a reserved memory region to be used for table
> + * allocations. This is a requirement for kexec reboots.
> + */
> +static void __init its_of_mem_region(struct device_node *node,
> +                                    struct of_resv **resv,
> +                                    struct of_resv *resv_buf)
> +{
> +       struct device_node *np = of_parse_phandle(node, "memory-region", 0);
> +       struct resource mem_res;
> +
> +       if (!np)
> +               return;
> +
> +       if (of_address_to_resource(np, 0, &mem_res)) {
> +               pr_warn("%pOF: address to resource failed\n", np);
> +               return;
> +       }
> +
> +       resv_buf->pa_start = mem_res.start;
> +       resv_buf->pa_end = mem_res.start + resource_size(&mem_res);
> +       *resv = resv_buf;
> +}
> +
>  /*
>   * We allocate memory for PROPBASE to cover 2 ^ lpi_id_bits LPIs to
>   * deal with (one configuration byte per interrupt). PENDBASE has to
> @@ -1665,7 +1713,7 @@ static int gic_reserve_range(phys_addr_t addr, unsigned long size)
>         return 0;
>  }
>
> -static int __init its_setup_lpi_prop_table(void)
> +static int __init its_setup_lpi_prop_table(struct of_resv *resv)
>  {
>         if (gic_rdists->flags & RDIST_FLAGS_RD_TABLES_PREALLOCATED) {
>                 unsigned long pa;
> @@ -1676,7 +1724,10 @@ static int __init its_setup_lpi_prop_table(void)
>                 lpi_id_bits = (val & GICR_PROPBASER_IDBITS_MASK) + 1;
>
>                 pa = val & GENMASK_ULL(51, 12);
> -               va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
> +               if (resv)
> +                       va = phys_to_virt(pa);
> +               else
> +                       va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
>                 gic_rdists->prop_table_pa = pa;
>                 gic_rdists->prop_table_va = va;
>         } else {
> @@ -1685,7 +1736,10 @@ static int __init its_setup_lpi_prop_table(void)
>                 lpi_id_bits = min_t(u32,
>                                     GICD_TYPER_ID_BITS(gic_rdists->gicd_typer),
>                                     ITS_MAX_LPI_NRBITS);
> -               page = its_allocate_prop_table(GFP_NOWAIT);
> +               if (resv)
> +                       page = get_of_page(resv, LPI_PROPBASE_SZ);
> +               else
> +                       page = its_allocate_prop_table(GFP_NOWAIT);
>                 if (!page) {
>                         pr_err("Failed to allocate PROPBASE\n");
>                         return -ENOMEM;
> @@ -2009,7 +2063,8 @@ static void its_free_pending_table(struct page *pt)
>
>  /*
>   * Booting with kdump and LPIs enabled is generally fine. Any other
> - * case is wrong in the absence of firmware/EFI support.
> + * case is wrong in the absence of firmware/EFI support or reserve-memory
> + * in device tree for interrupt controller.
>   */
>  static bool enabled_lpis_allowed(void)
>  {
> @@ -2023,7 +2078,7 @@ static bool enabled_lpis_allowed(void)
>         return gic_check_reserved_range(addr, LPI_PROPBASE_SZ);
>  }
>
> -static int __init allocate_lpi_tables(void)
> +static int __init allocate_lpi_tables(struct of_resv *resv)
>  {
>         u64 val;
>         int err, cpu;
> @@ -2039,7 +2094,7 @@ static int __init allocate_lpi_tables(void)
>                 pr_info("GICv3: Using preallocated redistributor tables\n");
>         }
>
> -       err = its_setup_lpi_prop_table();
> +       err = its_setup_lpi_prop_table(resv);
>         if (err)
>                 return err;
>
> @@ -2051,7 +2106,10 @@ static int __init allocate_lpi_tables(void)
>         for_each_possible_cpu(cpu) {
>                 struct page *pend_page;
>
> -               pend_page = its_allocate_pending_table(GFP_NOWAIT);
> +               if (resv)
> +                       pend_page = get_of_page(resv, LPI_PENDBASE_SZ);
> +               else
> +                       pend_page = its_allocate_pending_table(GFP_NOWAIT);
>                 if (!pend_page) {
>                         pr_err("Failed to allocate PENDBASE for CPU%d\n", cpu);
>                         return -ENOMEM;
> @@ -3957,16 +4015,20 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
>                     struct irq_domain *parent_domain)
>  {
>         struct device_node *of_node;
> +       struct of_resv resv_buf;
> +       struct of_resv *resv = NULL;
>         struct its_node *its;
>         bool has_v4 = false;
>         int err;
>
>         its_parent = parent_domain;
>         of_node = to_of_node(handle);
> -       if (of_node)
> +       if (of_node) {
>                 its_of_probe(of_node);
> -       else
> +               its_of_mem_region(of_node, &resv, &resv_buf);
> +       } else {
>                 its_acpi_probe();
> +       }
>
>         if (list_empty(&its_nodes)) {
>                 pr_warn("ITS: No ITS available, not enabling LPIs\n");
> @@ -3975,7 +4037,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
>
>         gic_rdists = rdists;
>
> -       err = allocate_lpi_tables();
> +       err = allocate_lpi_tables(resv);
>         if (err)
>                 return err;
>
> --
> 2.23.0
>

Re: [PATCH v1 5/6] rqchip/gic-v3-its: move reset pending table outside of allocator

[Pavel Tatashin]

This patch requires a small fix (which I will do in later revisions):

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 2ffdb3927549..c9faeac4b3a8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2182,7 +2182,8 @@ static void its_cpu_init_lpis(void)
                paddr &= GENMASK_ULL(51, 16);

                WARN_ON(!gic_check_reserved_range(paddr, LPI_PENDBASE_SZ));
-               its_free_pending_table(gic_data_rdist()->pend_page);
+               if (efi_enabled(EFI_CONFIG_TABLES))
+                       its_free_pending_table(gic_data_rdist()->pend_page);
                gic_data_rdist()->pend_page = NULL;

                goto out;

reserved-memory does not need to be freed. However, I am confused why
it is needed to be freed in EFI case. Marc, can you please explain
this to me?

Thank you,
Pasha

On Mon, Aug 26, 2019 at 3:01 PM Pavel Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> Allow to use reserved memory for interrupt controller tables.
>
> Currently, it is not possible to do kexec reboots without possible memory
> corruption using device tree and GICv3 interrupt controller.
>
> GICv3 can be configured once during boot, and location of tables cannot
> be changed thereafter.
>
> The fix is to allow to reserve memory region in interrupt controller device
> property, and use it to do allocations.
>
> Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 82 ++++++++++++++++++++++++++++----
>  1 file changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index d5f3508ca11f..aeda8760cc4e 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -47,6 +47,54 @@
>
>  static u32 lpi_id_bits;
>
> +/*
> + * Describes reserved memory region in interrupt controller.
> + * The memory reserved: [pa_start, pa_end)
> + */
> +struct of_resv {
> +       unsigned long pa_start;
> +       unsigned long pa_end;
> +};
> +
> +static struct page __init *get_of_page(struct of_resv *resv, unsigned long size)
> +{
> +       unsigned long pa = ALIGN(resv->pa_start, size);
> +       unsigned long pa_next = pa + size;
> +
> +       /* Check if there is enough memory reserved to do another allocation */
> +       if (pa_next > resv->pa_end)
> +               return NULL;
> +
> +       resv->pa_start = pa_next;
> +       memset(phys_to_virt(pa), 0, size);
> +
> +       return phys_to_page(pa);
> +}
> +
> +/*
> + * Memory controller might have a reserved memory region to be used for table
> + * allocations. This is a requirement for kexec reboots.
> + */
> +static void __init its_of_mem_region(struct device_node *node,
> +                                    struct of_resv **resv,
> +                                    struct of_resv *resv_buf)
> +{
> +       struct device_node *np = of_parse_phandle(node, "memory-region", 0);
> +       struct resource mem_res;
> +
> +       if (!np)
> +               return;
> +
> +       if (of_address_to_resource(np, 0, &mem_res)) {
> +               pr_warn("%pOF: address to resource failed\n", np);
> +               return;
> +       }
> +
> +       resv_buf->pa_start = mem_res.start;
> +       resv_buf->pa_end = mem_res.start + resource_size(&mem_res);
> +       *resv = resv_buf;
> +}
> +
>  /*
>   * We allocate memory for PROPBASE to cover 2 ^ lpi_id_bits LPIs to
>   * deal with (one configuration byte per interrupt). PENDBASE has to
> @@ -1665,7 +1713,7 @@ static int gic_reserve_range(phys_addr_t addr, unsigned long size)
>         return 0;
>  }
>
> -static int __init its_setup_lpi_prop_table(void)
> +static int __init its_setup_lpi_prop_table(struct of_resv *resv)
>  {
>         if (gic_rdists->flags & RDIST_FLAGS_RD_TABLES_PREALLOCATED) {
>                 unsigned long pa;
> @@ -1676,7 +1724,10 @@ static int __init its_setup_lpi_prop_table(void)
>                 lpi_id_bits = (val & GICR_PROPBASER_IDBITS_MASK) + 1;
>
>                 pa = val & GENMASK_ULL(51, 12);
> -               va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
> +               if (resv)
> +                       va = phys_to_virt(pa);
> +               else
> +                       va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
>                 gic_rdists->prop_table_pa = pa;
>                 gic_rdists->prop_table_va = va;
>         } else {
> @@ -1685,7 +1736,10 @@ static int __init its_setup_lpi_prop_table(void)
>                 lpi_id_bits = min_t(u32,
>                                     GICD_TYPER_ID_BITS(gic_rdists->gicd_typer),
>                                     ITS_MAX_LPI_NRBITS);
> -               page = its_allocate_prop_table(GFP_NOWAIT);
> +               if (resv)
> +                       page = get_of_page(resv, LPI_PROPBASE_SZ);
> +               else
> +                       page = its_allocate_prop_table(GFP_NOWAIT);
>                 if (!page) {
>                         pr_err("Failed to allocate PROPBASE\n");
>                         return -ENOMEM;
> @@ -2009,7 +2063,8 @@ static void its_free_pending_table(struct page *pt)
>
>  /*
>   * Booting with kdump and LPIs enabled is generally fine. Any other
> - * case is wrong in the absence of firmware/EFI support.
> + * case is wrong in the absence of firmware/EFI support or reserve-memory
> + * in device tree for interrupt controller.
>   */
>  static bool enabled_lpis_allowed(void)
>  {
> @@ -2023,7 +2078,7 @@ static bool enabled_lpis_allowed(void)
>         return gic_check_reserved_range(addr, LPI_PROPBASE_SZ);
>  }
>
> -static int __init allocate_lpi_tables(void)
> +static int __init allocate_lpi_tables(struct of_resv *resv)
>  {
>         u64 val;
>         int err, cpu;
> @@ -2039,7 +2094,7 @@ static int __init allocate_lpi_tables(void)
>                 pr_info("GICv3: Using preallocated redistributor tables\n");
>         }
>
> -       err = its_setup_lpi_prop_table();
> +       err = its_setup_lpi_prop_table(resv);
>         if (err)
>                 return err;
>
> @@ -2051,7 +2106,10 @@ static int __init allocate_lpi_tables(void)
>         for_each_possible_cpu(cpu) {
>                 struct page *pend_page;
>
> -               pend_page = its_allocate_pending_table(GFP_NOWAIT);
> +               if (resv)
> +                       pend_page = get_of_page(resv, LPI_PENDBASE_SZ);
> +               else
> +                       pend_page = its_allocate_pending_table(GFP_NOWAIT);
>                 if (!pend_page) {
>                         pr_err("Failed to allocate PENDBASE for CPU%d\n", cpu);
>                         return -ENOMEM;
> @@ -3957,16 +4015,20 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
>                     struct irq_domain *parent_domain)
>  {
>         struct device_node *of_node;
> +       struct of_resv resv_buf;
> +       struct of_resv *resv = NULL;
>         struct its_node *its;
>         bool has_v4 = false;
>         int err;
>
>         its_parent = parent_domain;
>         of_node = to_of_node(handle);
> -       if (of_node)
> +       if (of_node) {
>                 its_of_probe(of_node);
> -       else
> +               its_of_mem_region(of_node, &resv, &resv_buf);
> +       } else {
>                 its_acpi_probe();
> +       }
>
>         if (list_empty(&its_nodes)) {
>                 pr_warn("ITS: No ITS available, not enabling LPIs\n");
> @@ -3975,7 +4037,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
>
>         gic_rdists = rdists;
>
> -       err = allocate_lpi_tables();
> +       err = allocate_lpi_tables(resv);
>         if (err)
>                 return err;
>
> --
> 2.23.0
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 5/6] rqchip/gic-v3-its: move reset pending table outside of allocator

[Pavel Tatashin]

This patch requires a small fix (which I will do in later revisions):

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 2ffdb3927549..c9faeac4b3a8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2182,7 +2182,8 @@ static void its_cpu_init_lpis(void)
                paddr &= GENMASK_ULL(51, 16);

                WARN_ON(!gic_check_reserved_range(paddr, LPI_PENDBASE_SZ));
-               its_free_pending_table(gic_data_rdist()->pend_page);
+               if (efi_enabled(EFI_CONFIG_TABLES))
+                       its_free_pending_table(gic_data_rdist()->pend_page);
                gic_data_rdist()->pend_page = NULL;

                goto out;

reserved-memory does not need to be freed. However, I am confused why
it is needed to be freed in EFI case. Marc, can you please explain
this to me?

Thank you,
Pasha

On Mon, Aug 26, 2019 at 3:01 PM Pavel Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> Allow to use reserved memory for interrupt controller tables.
>
> Currently, it is not possible to do kexec reboots without possible memory
> corruption using device tree and GICv3 interrupt controller.
>
> GICv3 can be configured once during boot, and location of tables cannot
> be changed thereafter.
>
> The fix is to allow to reserve memory region in interrupt controller device
> property, and use it to do allocations.
>
> Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 82 ++++++++++++++++++++++++++++----
>  1 file changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index d5f3508ca11f..aeda8760cc4e 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -47,6 +47,54 @@
>
>  static u32 lpi_id_bits;
>
> +/*
> + * Describes reserved memory region in interrupt controller.
> + * The memory reserved: [pa_start, pa_end)
> + */
> +struct of_resv {
> +       unsigned long pa_start;
> +       unsigned long pa_end;
> +};
> +
> +static struct page __init *get_of_page(struct of_resv *resv, unsigned long size)
> +{
> +       unsigned long pa = ALIGN(resv->pa_start, size);
> +       unsigned long pa_next = pa + size;
> +
> +       /* Check if there is enough memory reserved to do another allocation */
> +       if (pa_next > resv->pa_end)
> +               return NULL;
> +
> +       resv->pa_start = pa_next;
> +       memset(phys_to_virt(pa), 0, size);
> +
> +       return phys_to_page(pa);
> +}
> +
> +/*
> + * Memory controller might have a reserved memory region to be used for table
> + * allocations. This is a requirement for kexec reboots.
> + */
> +static void __init its_of_mem_region(struct device_node *node,
> +                                    struct of_resv **resv,
> +                                    struct of_resv *resv_buf)
> +{
> +       struct device_node *np = of_parse_phandle(node, "memory-region", 0);
> +       struct resource mem_res;
> +
> +       if (!np)
> +               return;
> +
> +       if (of_address_to_resource(np, 0, &mem_res)) {
> +               pr_warn("%pOF: address to resource failed\n", np);
> +               return;
> +       }
> +
> +       resv_buf->pa_start = mem_res.start;
> +       resv_buf->pa_end = mem_res.start + resource_size(&mem_res);
> +       *resv = resv_buf;
> +}
> +
>  /*
>   * We allocate memory for PROPBASE to cover 2 ^ lpi_id_bits LPIs to
>   * deal with (one configuration byte per interrupt). PENDBASE has to
> @@ -1665,7 +1713,7 @@ static int gic_reserve_range(phys_addr_t addr, unsigned long size)
>         return 0;
>  }
>
> -static int __init its_setup_lpi_prop_table(void)
> +static int __init its_setup_lpi_prop_table(struct of_resv *resv)
>  {
>         if (gic_rdists->flags & RDIST_FLAGS_RD_TABLES_PREALLOCATED) {
>                 unsigned long pa;
> @@ -1676,7 +1724,10 @@ static int __init its_setup_lpi_prop_table(void)
>                 lpi_id_bits = (val & GICR_PROPBASER_IDBITS_MASK) + 1;
>
>                 pa = val & GENMASK_ULL(51, 12);
> -               va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
> +               if (resv)
> +                       va = phys_to_virt(pa);
> +               else
> +                       va = memremap(pa, LPI_PROPBASE_SZ, MEMREMAP_WB);
>                 gic_rdists->prop_table_pa = pa;
>                 gic_rdists->prop_table_va = va;
>         } else {
> @@ -1685,7 +1736,10 @@ static int __init its_setup_lpi_prop_table(void)
>                 lpi_id_bits = min_t(u32,
>                                     GICD_TYPER_ID_BITS(gic_rdists->gicd_typer),
>                                     ITS_MAX_LPI_NRBITS);
> -               page = its_allocate_prop_table(GFP_NOWAIT);
> +               if (resv)
> +                       page = get_of_page(resv, LPI_PROPBASE_SZ);
> +               else
> +                       page = its_allocate_prop_table(GFP_NOWAIT);
>                 if (!page) {
>                         pr_err("Failed to allocate PROPBASE\n");
>                         return -ENOMEM;
> @@ -2009,7 +2063,8 @@ static void its_free_pending_table(struct page *pt)
>
>  /*
>   * Booting with kdump and LPIs enabled is generally fine. Any other
> - * case is wrong in the absence of firmware/EFI support.
> + * case is wrong in the absence of firmware/EFI support or reserve-memory
> + * in device tree for interrupt controller.
>   */
>  static bool enabled_lpis_allowed(void)
>  {
> @@ -2023,7 +2078,7 @@ static bool enabled_lpis_allowed(void)
>         return gic_check_reserved_range(addr, LPI_PROPBASE_SZ);
>  }
>
> -static int __init allocate_lpi_tables(void)
> +static int __init allocate_lpi_tables(struct of_resv *resv)
>  {
>         u64 val;
>         int err, cpu;
> @@ -2039,7 +2094,7 @@ static int __init allocate_lpi_tables(void)
>                 pr_info("GICv3: Using preallocated redistributor tables\n");
>         }
>
> -       err = its_setup_lpi_prop_table();
> +       err = its_setup_lpi_prop_table(resv);
>         if (err)
>                 return err;
>
> @@ -2051,7 +2106,10 @@ static int __init allocate_lpi_tables(void)
>         for_each_possible_cpu(cpu) {
>                 struct page *pend_page;
>
> -               pend_page = its_allocate_pending_table(GFP_NOWAIT);
> +               if (resv)
> +                       pend_page = get_of_page(resv, LPI_PENDBASE_SZ);
> +               else
> +                       pend_page = its_allocate_pending_table(GFP_NOWAIT);
>                 if (!pend_page) {
>                         pr_err("Failed to allocate PENDBASE for CPU%d\n", cpu);
>                         return -ENOMEM;
> @@ -3957,16 +4015,20 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
>                     struct irq_domain *parent_domain)
>  {
>         struct device_node *of_node;
> +       struct of_resv resv_buf;
> +       struct of_resv *resv = NULL;
>         struct its_node *its;
>         bool has_v4 = false;
>         int err;
>
>         its_parent = parent_domain;
>         of_node = to_of_node(handle);
> -       if (of_node)
> +       if (of_node) {
>                 its_of_probe(of_node);
> -       else
> +               its_of_mem_region(of_node, &resv, &resv_buf);
> +       } else {
>                 its_acpi_probe();
> +       }
>
>         if (list_empty(&its_nodes)) {
>                 pr_warn("ITS: No ITS available, not enabling LPIs\n");
> @@ -3975,7 +4037,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
>
>         gic_rdists = rdists;
>
> -       err = allocate_lpi_tables();
> +       err = allocate_lpi_tables(resv);
>         if (err)
>                 return err;
>
> --
> 2.23.0
>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Marc Zyngier]

On 26/08/2019 22:25, Pavel Tatashin wrote:
> On Mon, Aug 26, 2019 at 3:13 PM Marc Zyngier <maz@kernel.org> wrote:
>>
>> On Mon, 26 Aug 2019 15:00:50 -0400
>> Pavel Tatashin <pasha.tatashin@soleen.com> wrote:
>>
>>> Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
>>> However, it is still not possible todo on systems with device trees.
>>>
>>> Here is EFI fixes from Marc:
>>> https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com
>>>
>>> For Device Tree variant: lets allow reserve a memory region in interrupt
>>> controller node, and use this property to allocate interrupt tables.
>>
>> There is no such thing as a "device tree variant". As long as your
>> bootloader implements EFI, everything will work correctly, whether
>> you're using DT, ACPI, or the anything else.
>>
>> This already works today, without any need to add anything to the
>> kernel (I have systems using EDK II and u-boot, both implementing EFI,
>> and I'm able to kexec without any issue). If your bootloader doesn't
>> support EFI, here's a good opportunity to implement it!
> 
> Hi Marc,
> 
> Thank you very much for looking at this work.
> 
> Running Linux without EFI is common, and there are scenarios which
> make it appropriate. As I understand most of embedded linux do not
> have EFI enabled, and thus I do not see a reason why we would not
> support a first class feature of Linux (kexec) on non-EFI bootloaders.

Define "most". All the arm64 systems I have around (and trust me, that's
quite a number of them) can either use u-boot, which has more than
enough EFI support to use this functionality, or use EDK-II natively.

> We (Microsoft) have a small highly secure device with a high uptime
> requirement. The device also has PCIe and thus GICv3. The update for

PCIe doesn't imply GICv3 at all.

> this device relies on kexec. For a number of reasons, it was decided
> to use U-Boot and Linux without EFI enabled. One of those reasons is
> to improve boot performance, enabling EFI in U-Boot alone reduces the
> boot performance by half a second. Our total reboot budget is under a
> second which makes that half a second unacceptable. Also, adding EFI
> support to kernel increases its size and there are security
> implications from enabling more code both in U-Boot and Linux.

You're are missing the point. kexec with EFI has 0 overhead (no
non-kernel EFI code gets executed), doesn't impact your time budget, and
only relies on a single in-memory table. This can be pretty trivially
provided by the dumbest EFI shim.

All you are describing above is a set of self imposed limitations in
your bootloader, which you are fully in control of. So instead of
reinventing a square wheel, I suggest you adopt the existing implementation.

Another reason not to do this is interoperability: I want to be able to
kexec whatever Linux kernel I want, without having to cope with all
flavours of the same functionality. Effectively, the EFI table is a
private ABI between two Linux kernels. We're not changing it.

	M.
-- 
Jazz is not dead, it just smells funny...

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Marc Zyngier]

On 26/08/2019 22:25, Pavel Tatashin wrote:
> On Mon, Aug 26, 2019 at 3:13 PM Marc Zyngier <maz@kernel.org> wrote:
>>
>> On Mon, 26 Aug 2019 15:00:50 -0400
>> Pavel Tatashin <pasha.tatashin@soleen.com> wrote:
>>
>>> Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
>>> However, it is still not possible todo on systems with device trees.
>>>
>>> Here is EFI fixes from Marc:
>>> https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com
>>>
>>> For Device Tree variant: lets allow reserve a memory region in interrupt
>>> controller node, and use this property to allocate interrupt tables.
>>
>> There is no such thing as a "device tree variant". As long as your
>> bootloader implements EFI, everything will work correctly, whether
>> you're using DT, ACPI, or the anything else.
>>
>> This already works today, without any need to add anything to the
>> kernel (I have systems using EDK II and u-boot, both implementing EFI,
>> and I'm able to kexec without any issue). If your bootloader doesn't
>> support EFI, here's a good opportunity to implement it!
> 
> Hi Marc,
> 
> Thank you very much for looking at this work.
> 
> Running Linux without EFI is common, and there are scenarios which
> make it appropriate. As I understand most of embedded linux do not
> have EFI enabled, and thus I do not see a reason why we would not
> support a first class feature of Linux (kexec) on non-EFI bootloaders.

Define "most". All the arm64 systems I have around (and trust me, that's
quite a number of them) can either use u-boot, which has more than
enough EFI support to use this functionality, or use EDK-II natively.

> We (Microsoft) have a small highly secure device with a high uptime
> requirement. The device also has PCIe and thus GICv3. The update for

PCIe doesn't imply GICv3 at all.

> this device relies on kexec. For a number of reasons, it was decided
> to use U-Boot and Linux without EFI enabled. One of those reasons is
> to improve boot performance, enabling EFI in U-Boot alone reduces the
> boot performance by half a second. Our total reboot budget is under a
> second which makes that half a second unacceptable. Also, adding EFI
> support to kernel increases its size and there are security
> implications from enabling more code both in U-Boot and Linux.

You're are missing the point. kexec with EFI has 0 overhead (no
non-kernel EFI code gets executed), doesn't impact your time budget, and
only relies on a single in-memory table. This can be pretty trivially
provided by the dumbest EFI shim.

All you are describing above is a set of self imposed limitations in
your bootloader, which you are fully in control of. So instead of
reinventing a square wheel, I suggest you adopt the existing implementation.

Another reason not to do this is interoperability: I want to be able to
kexec whatever Linux kernel I want, without having to cope with all
flavours of the same functionality. Effectively, the EFI table is a
private ABI between two Linux kernels. We're not changing it.

	M.
-- 
Jazz is not dead, it just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Marc Zyngier]

On 26/08/2019 22:25, Pavel Tatashin wrote:
> On Mon, Aug 26, 2019 at 3:13 PM Marc Zyngier <maz@kernel.org> wrote:
>>
>> On Mon, 26 Aug 2019 15:00:50 -0400
>> Pavel Tatashin <pasha.tatashin@soleen.com> wrote:
>>
>>> Marc Zyngier added the support for kexec and GICv3 for EFI based systems.
>>> However, it is still not possible todo on systems with device trees.
>>>
>>> Here is EFI fixes from Marc:
>>> https://lore.kernel.org/lkml/20180921195954.21574-1-marc.zyngier@arm.com
>>>
>>> For Device Tree variant: lets allow reserve a memory region in interrupt
>>> controller node, and use this property to allocate interrupt tables.
>>
>> There is no such thing as a "device tree variant". As long as your
>> bootloader implements EFI, everything will work correctly, whether
>> you're using DT, ACPI, or the anything else.
>>
>> This already works today, without any need to add anything to the
>> kernel (I have systems using EDK II and u-boot, both implementing EFI,
>> and I'm able to kexec without any issue). If your bootloader doesn't
>> support EFI, here's a good opportunity to implement it!
> 
> Hi Marc,
> 
> Thank you very much for looking at this work.
> 
> Running Linux without EFI is common, and there are scenarios which
> make it appropriate. As I understand most of embedded linux do not
> have EFI enabled, and thus I do not see a reason why we would not
> support a first class feature of Linux (kexec) on non-EFI bootloaders.

Define "most". All the arm64 systems I have around (and trust me, that's
quite a number of them) can either use u-boot, which has more than
enough EFI support to use this functionality, or use EDK-II natively.

> We (Microsoft) have a small highly secure device with a high uptime
> requirement. The device also has PCIe and thus GICv3. The update for

PCIe doesn't imply GICv3 at all.

> this device relies on kexec. For a number of reasons, it was decided
> to use U-Boot and Linux without EFI enabled. One of those reasons is
> to improve boot performance, enabling EFI in U-Boot alone reduces the
> boot performance by half a second. Our total reboot budget is under a
> second which makes that half a second unacceptable. Also, adding EFI
> support to kernel increases its size and there are security
> implications from enabling more code both in U-Boot and Linux.

You're are missing the point. kexec with EFI has 0 overhead (no
non-kernel EFI code gets executed), doesn't impact your time budget, and
only relies on a single in-memory table. This can be pretty trivially
provided by the dumbest EFI shim.

All you are describing above is a set of self imposed limitations in
your bootloader, which you are fully in control of. So instead of
reinventing a square wheel, I suggest you adopt the existing implementation.

Another reason not to do this is interoperability: I want to be able to
kexec whatever Linux kernel I want, without having to cope with all
flavours of the same functionality. Effectively, the EFI table is a
private ABI between two Linux kernels. We're not changing it.

	M.
-- 
Jazz is not dead, it just smells funny...

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Pavel Tatashin]

> > Running Linux without EFI is common, and there are scenarios which
> > make it appropriate. As I understand most of embedded linux do not
> > have EFI enabled, and thus I do not see a reason why we would not
> > support a first class feature of Linux (kexec) on non-EFI bootloaders.
>
> Define "most". All the arm64 systems I have around (and trust me, that's
> quite a number of them) can either use u-boot, which has more than
> enough EFI support to use this functionality, or use EDK-II natively.

OK. Is this the most common configuration in the embedded ARM64
devices currently deployed: phones, cameras, consoles, players, etc?

> > We (Microsoft) have a small highly secure device with a high uptime
> > requirement. The device also has PCIe and thus GICv3. The update for
>
> PCIe doesn't imply GICv3 at all.

My impression was that without PCIe GICv3 is rarely used, and this
could be the reason why this problem is not seen outside of larger
machines which normally have EFI enabled.

>
> > this device relies on kexec. For a number of reasons, it was decided
> > to use U-Boot and Linux without EFI enabled. One of those reasons is
> > to improve boot performance, enabling EFI in U-Boot alone reduces the
> > boot performance by half a second. Our total reboot budget is under a
> > second which makes that half a second unacceptable. Also, adding EFI
> > support to kernel increases its size and there are security
> > implications from enabling more code both in U-Boot and Linux.
>
> You're are missing the point. kexec with EFI has 0 overhead (no
> non-kernel EFI code gets executed), doesn't impact your time budget, and
> only relies on a single in-memory table. This can be pretty trivially
> provided by the dumbest EFI shim.

Thanks, this makes sense that the Linux boot time won't be affected. I
have not tested how u-boot was affected, but was told 0.5 second
longer to start.

> All you are describing above is a set of self imposed limitations in
> your bootloader, which you are fully in control of. So instead of
> reinventing a square wheel, I suggest you adopt the existing implementation.

I am not sure this analogy is correct, I do not think that non-EFI
enabled kernels became legacy.

> Another reason not to do this is interoperability: I want to be able to
> kexec whatever Linux kernel I want, without having to cope with all
> flavours of the same functionality. Effectively, the EFI table is a
> private ABI between two Linux kernels. We're not changing it.

This is exactly the problem: by having this region defined in signed
DTB file we reduce the amount of communication between the kernels.
Passing modified EFI Table causes us to pass more information from the
first kernel indefinitely through updates. Thus, increases a chance
for a security compromise. We are not changing EFI ABI between
kernels, it will stay as is. All this code does is enables kernels
that do not have EFI table communication between them a way to do
kexec updates with reduced amount of data exchange.

Thank you,
Pasha

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Pavel Tatashin]

> > Running Linux without EFI is common, and there are scenarios which
> > make it appropriate. As I understand most of embedded linux do not
> > have EFI enabled, and thus I do not see a reason why we would not
> > support a first class feature of Linux (kexec) on non-EFI bootloaders.
>
> Define "most". All the arm64 systems I have around (and trust me, that's
> quite a number of them) can either use u-boot, which has more than
> enough EFI support to use this functionality, or use EDK-II natively.

OK. Is this the most common configuration in the embedded ARM64
devices currently deployed: phones, cameras, consoles, players, etc?

> > We (Microsoft) have a small highly secure device with a high uptime
> > requirement. The device also has PCIe and thus GICv3. The update for
>
> PCIe doesn't imply GICv3 at all.

My impression was that without PCIe GICv3 is rarely used, and this
could be the reason why this problem is not seen outside of larger
machines which normally have EFI enabled.

>
> > this device relies on kexec. For a number of reasons, it was decided
> > to use U-Boot and Linux without EFI enabled. One of those reasons is
> > to improve boot performance, enabling EFI in U-Boot alone reduces the
> > boot performance by half a second. Our total reboot budget is under a
> > second which makes that half a second unacceptable. Also, adding EFI
> > support to kernel increases its size and there are security
> > implications from enabling more code both in U-Boot and Linux.
>
> You're are missing the point. kexec with EFI has 0 overhead (no
> non-kernel EFI code gets executed), doesn't impact your time budget, and
> only relies on a single in-memory table. This can be pretty trivially
> provided by the dumbest EFI shim.

Thanks, this makes sense that the Linux boot time won't be affected. I
have not tested how u-boot was affected, but was told 0.5 second
longer to start.

> All you are describing above is a set of self imposed limitations in
> your bootloader, which you are fully in control of. So instead of
> reinventing a square wheel, I suggest you adopt the existing implementation.

I am not sure this analogy is correct, I do not think that non-EFI
enabled kernels became legacy.

> Another reason not to do this is interoperability: I want to be able to
> kexec whatever Linux kernel I want, without having to cope with all
> flavours of the same functionality. Effectively, the EFI table is a
> private ABI between two Linux kernels. We're not changing it.

This is exactly the problem: by having this region defined in signed
DTB file we reduce the amount of communication between the kernels.
Passing modified EFI Table causes us to pass more information from the
first kernel indefinitely through updates. Thus, increases a chance
for a security compromise. We are not changing EFI ABI between
kernels, it will stay as is. All this code does is enables kernels
that do not have EFI table communication between them a way to do
kexec updates with reduced amount of data exchange.

Thank you,
Pasha

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Pavel Tatashin]

> > Running Linux without EFI is common, and there are scenarios which
> > make it appropriate. As I understand most of embedded linux do not
> > have EFI enabled, and thus I do not see a reason why we would not
> > support a first class feature of Linux (kexec) on non-EFI bootloaders.
>
> Define "most". All the arm64 systems I have around (and trust me, that's
> quite a number of them) can either use u-boot, which has more than
> enough EFI support to use this functionality, or use EDK-II natively.

OK. Is this the most common configuration in the embedded ARM64
devices currently deployed: phones, cameras, consoles, players, etc?

> > We (Microsoft) have a small highly secure device with a high uptime
> > requirement. The device also has PCIe and thus GICv3. The update for
>
> PCIe doesn't imply GICv3 at all.

My impression was that without PCIe GICv3 is rarely used, and this
could be the reason why this problem is not seen outside of larger
machines which normally have EFI enabled.

>
> > this device relies on kexec. For a number of reasons, it was decided
> > to use U-Boot and Linux without EFI enabled. One of those reasons is
> > to improve boot performance, enabling EFI in U-Boot alone reduces the
> > boot performance by half a second. Our total reboot budget is under a
> > second which makes that half a second unacceptable. Also, adding EFI
> > support to kernel increases its size and there are security
> > implications from enabling more code both in U-Boot and Linux.
>
> You're are missing the point. kexec with EFI has 0 overhead (no
> non-kernel EFI code gets executed), doesn't impact your time budget, and
> only relies on a single in-memory table. This can be pretty trivially
> provided by the dumbest EFI shim.

Thanks, this makes sense that the Linux boot time won't be affected. I
have not tested how u-boot was affected, but was told 0.5 second
longer to start.

> All you are describing above is a set of self imposed limitations in
> your bootloader, which you are fully in control of. So instead of
> reinventing a square wheel, I suggest you adopt the existing implementation.

I am not sure this analogy is correct, I do not think that non-EFI
enabled kernels became legacy.

> Another reason not to do this is interoperability: I want to be able to
> kexec whatever Linux kernel I want, without having to cope with all
> flavours of the same functionality. Effectively, the EFI table is a
> private ABI between two Linux kernels. We're not changing it.

This is exactly the problem: by having this region defined in signed
DTB file we reduce the amount of communication between the kernels.
Passing modified EFI Table causes us to pass more information from the
first kernel indefinitely through updates. Thus, increases a chance
for a security compromise. We are not changing EFI ABI between
kernels, it will stay as is. All this code does is enables kernels
that do not have EFI table communication between them a way to do
kexec updates with reduced amount of data exchange.

Thank you,
Pasha

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Marc Zyngier]

On 27/08/2019 09:53, Pavel Tatashin wrote:
>>> Running Linux without EFI is common, and there are scenarios which
>>> make it appropriate. As I understand most of embedded linux do not
>>> have EFI enabled, and thus I do not see a reason why we would not
>>> support a first class feature of Linux (kexec) on non-EFI bootloaders.
>>
>> Define "most". All the arm64 systems I have around (and trust me, that's
>> quite a number of them) can either use u-boot, which has more than
>> enough EFI support to use this functionality, or use EDK-II natively.
> 
> OK. Is this the most common configuration in the embedded ARM64
> devices currently deployed: phones, cameras, consoles, players, etc?

Which one of these has kexec as a requirement?

>>> We (Microsoft) have a small highly secure device with a high uptime
>>> requirement. The device also has PCIe and thus GICv3. The update for
>>
>> PCIe doesn't imply GICv3 at all.
> 
> My impression was that without PCIe GICv3 is rarely used, and this
> could be the reason why this problem is not seen outside of larger
> machines which normally have EFI enabled.

Wong impression. All the combinations exist and are wildly deployed.

>>> this device relies on kexec. For a number of reasons, it was decided
>>> to use U-Boot and Linux without EFI enabled. One of those reasons is
>>> to improve boot performance, enabling EFI in U-Boot alone reduces the
>>> boot performance by half a second. Our total reboot budget is under a
>>> second which makes that half a second unacceptable. Also, adding EFI
>>> support to kernel increases its size and there are security
>>> implications from enabling more code both in U-Boot and Linux.
>>
>> You're are missing the point. kexec with EFI has 0 overhead (no
>> non-kernel EFI code gets executed), doesn't impact your time budget, and
>> only relies on a single in-memory table. This can be pretty trivially
>> provided by the dumbest EFI shim.
> 
> Thanks, this makes sense that the Linux boot time won't be affected. I
> have not tested how u-boot was affected, but was told 0.5 second
> longer to start.

So you haven't even tried? :-(

> 
>> All you are describing above is a set of self imposed limitations in
>> your bootloader, which you are fully in control of. So instead of
>> reinventing a square wheel, I suggest you adopt the existing implementation.
> 
> I am not sure this analogy is correct, I do not think that non-EFI
> enabled kernels became legacy.

non-EFI systems always had reduced functionality, such as not being able
to use runtime services.

> 
>> Another reason not to do this is interoperability: I want to be able to
>> kexec whatever Linux kernel I want, without having to cope with all
>> flavours of the same functionality. Effectively, the EFI table is a
>> private ABI between two Linux kernels. We're not changing it.
> 
> This is exactly the problem: by having this region defined in signed
> DTB file we reduce the amount of communication between the kernels.
> Passing modified EFI Table causes us to pass more information from the
> first kernel indefinitely through updates. Thus, increases a chance
> for a security compromise.

Nothing says that it has to be modified. For what it's worth, you could
perform the allocation in your bootloader once and for all, configure
the GIC redistributors and enable LPIs there, and pass the EFI
reservation to the first kernel. The security argument is a fallacy.

> We are not changing EFI ABI between
> kernels, it will stay as is. All this code does is enables kernels
> that do not have EFI table communication between them a way to do
> kexec updates with reduced amount of data exchange.

And to do that, you're adding yet another ABI we have to support, and
creating havoc in the kexec chain (kernel 1 knows about the DT hack,
kernel 2 doesn't, panic follows). My answer to this is *no*. We already
have a flexible interface that allows you to do what you want, and I'm
not adding another one.

Thanks,

	M.
-- 
Jazz is not dead, it just smells funny...

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Marc Zyngier]

On 27/08/2019 09:53, Pavel Tatashin wrote:
>>> Running Linux without EFI is common, and there are scenarios which
>>> make it appropriate. As I understand most of embedded linux do not
>>> have EFI enabled, and thus I do not see a reason why we would not
>>> support a first class feature of Linux (kexec) on non-EFI bootloaders.
>>
>> Define "most". All the arm64 systems I have around (and trust me, that's
>> quite a number of them) can either use u-boot, which has more than
>> enough EFI support to use this functionality, or use EDK-II natively.
> 
> OK. Is this the most common configuration in the embedded ARM64
> devices currently deployed: phones, cameras, consoles, players, etc?

Which one of these has kexec as a requirement?

>>> We (Microsoft) have a small highly secure device with a high uptime
>>> requirement. The device also has PCIe and thus GICv3. The update for
>>
>> PCIe doesn't imply GICv3 at all.
> 
> My impression was that without PCIe GICv3 is rarely used, and this
> could be the reason why this problem is not seen outside of larger
> machines which normally have EFI enabled.

Wong impression. All the combinations exist and are wildly deployed.

>>> this device relies on kexec. For a number of reasons, it was decided
>>> to use U-Boot and Linux without EFI enabled. One of those reasons is
>>> to improve boot performance, enabling EFI in U-Boot alone reduces the
>>> boot performance by half a second. Our total reboot budget is under a
>>> second which makes that half a second unacceptable. Also, adding EFI
>>> support to kernel increases its size and there are security
>>> implications from enabling more code both in U-Boot and Linux.
>>
>> You're are missing the point. kexec with EFI has 0 overhead (no
>> non-kernel EFI code gets executed), doesn't impact your time budget, and
>> only relies on a single in-memory table. This can be pretty trivially
>> provided by the dumbest EFI shim.
> 
> Thanks, this makes sense that the Linux boot time won't be affected. I
> have not tested how u-boot was affected, but was told 0.5 second
> longer to start.

So you haven't even tried? :-(

> 
>> All you are describing above is a set of self imposed limitations in
>> your bootloader, which you are fully in control of. So instead of
>> reinventing a square wheel, I suggest you adopt the existing implementation.
> 
> I am not sure this analogy is correct, I do not think that non-EFI
> enabled kernels became legacy.

non-EFI systems always had reduced functionality, such as not being able
to use runtime services.

> 
>> Another reason not to do this is interoperability: I want to be able to
>> kexec whatever Linux kernel I want, without having to cope with all
>> flavours of the same functionality. Effectively, the EFI table is a
>> private ABI between two Linux kernels. We're not changing it.
> 
> This is exactly the problem: by having this region defined in signed
> DTB file we reduce the amount of communication between the kernels.
> Passing modified EFI Table causes us to pass more information from the
> first kernel indefinitely through updates. Thus, increases a chance
> for a security compromise.

Nothing says that it has to be modified. For what it's worth, you could
perform the allocation in your bootloader once and for all, configure
the GIC redistributors and enable LPIs there, and pass the EFI
reservation to the first kernel. The security argument is a fallacy.

> We are not changing EFI ABI between
> kernels, it will stay as is. All this code does is enables kernels
> that do not have EFI table communication between them a way to do
> kexec updates with reduced amount of data exchange.

And to do that, you're adding yet another ABI we have to support, and
creating havoc in the kexec chain (kernel 1 knows about the DT hack,
kernel 2 doesn't, panic follows). My answer to this is *no*. We already
have a flexible interface that allows you to do what you want, and I'm
not adding another one.

Thanks,

	M.
-- 
Jazz is not dead, it just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 0/6] Allow kexec reboot for GICv3 and device tree

[Marc Zyngier]

On 27/08/2019 09:53, Pavel Tatashin wrote:
>>> Running Linux without EFI is common, and there are scenarios which
>>> make it appropriate. As I understand most of embedded linux do not
>>> have EFI enabled, and thus I do not see a reason why we would not
>>> support a first class feature of Linux (kexec) on non-EFI bootloaders.
>>
>> Define "most". All the arm64 systems I have around (and trust me, that's
>> quite a number of them) can either use u-boot, which has more than
>> enough EFI support to use this functionality, or use EDK-II natively.
> 
> OK. Is this the most common configuration in the embedded ARM64
> devices currently deployed: phones, cameras, consoles, players, etc?

Which one of these has kexec as a requirement?

>>> We (Microsoft) have a small highly secure device with a high uptime
>>> requirement. The device also has PCIe and thus GICv3. The update for
>>
>> PCIe doesn't imply GICv3 at all.
> 
> My impression was that without PCIe GICv3 is rarely used, and this
> could be the reason why this problem is not seen outside of larger
> machines which normally have EFI enabled.

Wong impression. All the combinations exist and are wildly deployed.

>>> this device relies on kexec. For a number of reasons, it was decided
>>> to use U-Boot and Linux without EFI enabled. One of those reasons is
>>> to improve boot performance, enabling EFI in U-Boot alone reduces the
>>> boot performance by half a second. Our total reboot budget is under a
>>> second which makes that half a second unacceptable. Also, adding EFI
>>> support to kernel increases its size and there are security
>>> implications from enabling more code both in U-Boot and Linux.
>>
>> You're are missing the point. kexec with EFI has 0 overhead (no
>> non-kernel EFI code gets executed), doesn't impact your time budget, and
>> only relies on a single in-memory table. This can be pretty trivially
>> provided by the dumbest EFI shim.
> 
> Thanks, this makes sense that the Linux boot time won't be affected. I
> have not tested how u-boot was affected, but was told 0.5 second
> longer to start.

So you haven't even tried? :-(

> 
>> All you are describing above is a set of self imposed limitations in
>> your bootloader, which you are fully in control of. So instead of
>> reinventing a square wheel, I suggest you adopt the existing implementation.
> 
> I am not sure this analogy is correct, I do not think that non-EFI
> enabled kernels became legacy.

non-EFI systems always had reduced functionality, such as not being able
to use runtime services.

> 
>> Another reason not to do this is interoperability: I want to be able to
>> kexec whatever Linux kernel I want, without having to cope with all
>> flavours of the same functionality. Effectively, the EFI table is a
>> private ABI between two Linux kernels. We're not changing it.
> 
> This is exactly the problem: by having this region defined in signed
> DTB file we reduce the amount of communication between the kernels.
> Passing modified EFI Table causes us to pass more information from the
> first kernel indefinitely through updates. Thus, increases a chance
> for a security compromise.

Nothing says that it has to be modified. For what it's worth, you could
perform the allocation in your bootloader once and for all, configure
the GIC redistributors and enable LPIs there, and pass the EFI
reservation to the first kernel. The security argument is a fallacy.

> We are not changing EFI ABI between
> kernels, it will stay as is. All this code does is enables kernels
> that do not have EFI table communication between them a way to do
> kexec updates with reduced amount of data exchange.

And to do that, you're adding yet another ABI we have to support, and
creating havoc in the kexec chain (kernel 1 knows about the DT hack,
kernel 2 doesn't, panic follows). My answer to this is *no*. We already
have a flexible interface that allows you to do what you want, and I'm
not adding another one.

Thanks,

	M.
-- 
Jazz is not dead, it just smells funny...

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec