[RFC PATCH] fpga: region: Add support for FPGA region variants

[RFC PATCH] fpga: region: Add support for FPGA region variants

[Marco Pagani]

This RFC proposes a proof-of-concept implementation of FPGA region
variants, a mechanism that introduces a common way to handle
dynamic partial reconfiguration from userspace. The proposed approach
is safe and aligned with the mainline kernel's stance on hardware
management by constraining the hardware to a mutually exclusive set
of configurations (variants) defined upfront. This is a realistic
assumption for FPGAs, as regions are typically statically defined and
synthesized during the system design phase. To keep the architecture
realistic, the following additional constraints are introduced:
(i) variants cannot be nested, and (ii) variants cannot contain FPGA
bridges.

The interface and core logic for the variant mechanism are defined
in the fpga-region and implemented in a backwards-compatible way.
The fpga-region now optionally exports sysfs attributes that allow
the user to reconfigure a region that supports variants by selecting
one variant a list of pre-defined variants. Concrete regions can enable
variant support by implementing the new apply_variant and remove_variant
methods and adding them to fpga_region_info before registration.

As part of this RFC, the of-fpga-region concrete region has been extended
to implement the variant interface. Variants are statically specified in
the device tree using an fpga-variants node. Additionally, it introduces
a firmware-cached property to cache bitstreams in memory, enabling a fast
reconfiguration path for real-time (latency-sensitive) applications.

Below is an example of how variants can be statically defined in the
device tree under this architecture:

fake_mgr: fpga-mgr@0 {
        compatible = "linux,fake-fpga-mgr";
};

fpga_region: fpga-region@0 {
        compatible = "fpga-region";
        #address-cells = <2>;
        #size-cells = <2>;
        ranges;

        /* FPGA region properties */
        fpga-mgr = <&fake_mgr>;
        partial-fpga-config;
        region-unfreeze-timeout-us = <10000>;

        /* Base variant */
        base-variant = "variant-1";

        /* Variants container node */
        fpga-variants {
                #address-cells = <2>;
                #size-cells = <2>;
                ranges;

                variant-1 {
                        firmware-name = "variant1-image.bin";

                        #address-cells = <2>;
                        #size-cells = <2>;
                        ranges;

                        variant_1_ip: ip_1@10000 {
                                compatible = "fake,ip_1";
                                reg = <0x0 0x10000 0x0 0x1000>;
                        };
                };

                variant-2 {
                        firmware-name = "variant2-image.bin";
                        firmware-cached;

                        #address-cells = <2>;
                        #size-cells = <2>;
                        ranges;

                        variant_2_ip: ip_2@10000 {
                                compatible = "fake,ip_2";
                                reg = <0x0 0x20000 0x0 0x1000>;
                        };
                };
        };
};

Marco Pagani (2):
  fpga: region: Add support for FPGA region variants
  fpga: of-fpga-region: Add support for region variants

 drivers/fpga/fpga-region.c       | 171 +++++++++++++++++++
 drivers/fpga/of-fpga-region.c    | 272 ++++++++++++++++++++++++++++++-
 include/linux/fpga/fpga-region.h |  32 ++++
 3 files changed, 473 insertions(+), 2 deletions(-)

-- 
2.54.0

[RFC PATCH fpga/for-next 1/2] fpga: region: Add support for FPGA region variants

[Marco Pagani]

Extend the fpga-region to support FPGA variants. Add the logic and
the interface functions to manage a list of variants, and define the
apply_variant() and remove_variant() methods that concrete regions must
implement to define how variants are populated and depopulated.

Signed-off-by: Marco Pagani <marco.pagani@linux.dev>
---
 drivers/fpga/fpga-region.c       | 171 +++++++++++++++++++++++++++++++
 include/linux/fpga/fpga-region.h |  32 ++++++
 2 files changed, 203 insertions(+)

diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index 662e8e4203ca..60404cce6188 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -80,6 +80,101 @@ static void fpga_region_put(struct fpga_region *region)
 	mutex_unlock(&region->mutex);
 }
 
+/**
+ * fpga_region_add_variant - add a new variant to the FPGA region
+ * @region: FPGA region
+ * @name: string identifier for the variant
+ * @is_enabled: whether this variant is the currently enabled variant
+ * @priv: private data to be associated with the variant
+ *
+ * Allocates a new variant and adds it to the region's variant list.
+ * If @is_enabled is true, this variant is marked as the enabled variant.
+ *
+ * Return: 0 on success, or -ENOMEM on memory allocation failure.
+ */
+int fpga_region_add_variant(struct fpga_region *region, const char *name,
+			    bool is_enabled, void *priv)
+{
+	struct fpga_variant *variant;
+	char *variant_name;
+
+	variant_name = devm_kstrdup(&region->dev, name, GFP_KERNEL);
+	if (!variant_name)
+		return -ENOMEM;
+
+	variant = devm_kzalloc(&region->dev, sizeof(*variant), GFP_KERNEL);
+	if (!variant)
+		return -ENOMEM;
+
+	variant->priv = priv;
+	variant->name = variant_name;
+
+	mutex_lock(&region->variant_mutex);
+	if (is_enabled)
+		region->enabled_variant = variant;
+
+	list_add_tail(&variant->node, &region->variant_list);
+	mutex_unlock(&region->variant_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fpga_region_add_variant);
+
+/**
+ * fpga_region_program_variant - switch the variant of an FPGA region
+ * @region: FPGA region
+ * @name: string identifier of the variant to enable
+ *
+ * Get the requested variant from the region's variant list. If found,
+ * and it is not already active, removes the currently enabled variant via
+ * the remove_variant() callback and applies the requested one  via the
+ * apply_variant() callback.
+ *
+ * Return: 0 on success, -EINVAL if the variant is not found, or a negative
+ * error code passed up from the apply_variant() callback.
+ */
+int fpga_region_program_variant(struct fpga_region *region, const char *name)
+{
+	struct fpga_variant *variant;
+	bool found_variant = false;
+	int ret = -EINVAL;
+
+	mutex_lock(&region->variant_mutex);
+
+	list_for_each_entry(variant, &region->variant_list, node) {
+		if (sysfs_streq(variant->name, name)) {
+			found_variant = true;
+			break;
+		}
+	}
+
+	if (!found_variant)
+		goto out_unlock;
+
+	if (region->enabled_variant == variant) {
+		ret = 0;
+		goto out_unlock;
+	}
+
+	if (region->enabled_variant && region->remove_variant)
+		region->remove_variant(region, region->enabled_variant);
+
+	if (region->apply_variant) {
+		ret = region->apply_variant(region, variant);
+		if (ret) {
+			dev_err(&region->dev, "Variant programming failed!\n");
+			region->enabled_variant = NULL;
+		} else {
+			region->enabled_variant = variant;
+		}
+	}
+
+out_unlock:
+	mutex_unlock(&region->variant_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(fpga_region_program_variant);
+
 /**
  * fpga_region_program_fpga - program FPGA
  *
@@ -174,12 +269,76 @@ static ssize_t compat_id_show(struct device *dev,
 
 static DEVICE_ATTR_RO(compat_id);
 
+static ssize_t variants_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct fpga_region *region = to_fpga_region(dev);
+	struct fpga_variant *variant;
+	int len = 0;
+
+	mutex_lock(&region->variant_mutex);
+	list_for_each_entry(variant, &region->variant_list, node)
+		len += sysfs_emit_at(buf, len, "%s\n", variant->name);
+	mutex_unlock(&region->variant_mutex);
+
+	return len;
+}
+static DEVICE_ATTR_RO(variants);
+
+static ssize_t enabled_variant_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct fpga_region *region = to_fpga_region(dev);
+	ssize_t len = 0;
+
+	mutex_lock(&region->variant_mutex);
+	if (region->enabled_variant)
+		len = sysfs_emit(buf, "%s\n", region->enabled_variant->name);
+	mutex_unlock(&region->variant_mutex);
+
+	return len;
+}
+
+static ssize_t enabled_variant_store(struct device *dev, struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct fpga_region *region = to_fpga_region(dev);
+	int ret;
+
+	ret = fpga_region_program_variant(region, buf);
+
+	return ret ? ret : count;
+}
+static DEVICE_ATTR_RW(enabled_variant);
+
+static struct attribute *fpga_region_variant_attrs[] = {
+	&dev_attr_enabled_variant.attr,
+	&dev_attr_variants.attr,
+	NULL,
+};
+
+static umode_t fpga_region_variant_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct fpga_region *region = to_fpga_region(dev);
+
+	return region->apply_variant ? attr->mode : 0;
+}
+
+static const struct attribute_group fpga_region_variant_group = {
+	.attrs = fpga_region_variant_attrs,
+	.is_visible = fpga_region_variant_is_visible,
+};
+
 static struct attribute *fpga_region_attrs[] = {
 	&dev_attr_compat_id.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(fpga_region);
 
+static const struct attribute_group *fpga_region_variant_groups[] = {
+	&fpga_region_variant_group,
+	NULL,
+};
+
 /**
  * __fpga_region_register_full - create and register an FPGA Region device
  * @parent: device parent
@@ -216,14 +375,19 @@ __fpga_region_register_full(struct device *parent, const struct fpga_region_info
 	region->priv = info->priv;
 	region->get_bridges = info->get_bridges;
 	region->ops_owner = owner;
+	region->apply_variant = info->apply_variant;
+	region->remove_variant = info->remove_variant;
 
 	mutex_init(&region->mutex);
+	mutex_init(&region->variant_mutex);
 	INIT_LIST_HEAD(&region->bridge_list);
+	INIT_LIST_HEAD(&region->variant_list);
 
 	region->dev.class = &fpga_region_class;
 	region->dev.parent = parent;
 	region->dev.of_node = parent->of_node;
 	region->dev.id = id;
+	region->dev.groups = fpga_region_variant_groups;
 
 	ret = dev_set_name(&region->dev, "region%d", id);
 	if (ret)
@@ -280,6 +444,13 @@ EXPORT_SYMBOL_GPL(__fpga_region_register);
  */
 void fpga_region_unregister(struct fpga_region *region)
 {
+	mutex_lock(&region->variant_mutex);
+	if (region->enabled_variant && region->remove_variant) {
+		region->remove_variant(region, region->enabled_variant);
+		region->enabled_variant = NULL;
+	}
+	mutex_unlock(&region->variant_mutex);
+
 	device_unregister(&region->dev);
 }
 EXPORT_SYMBOL_GPL(fpga_region_unregister);
diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
index 5fbc05fe70a6..3b3d1f8a1189 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -9,12 +9,27 @@
 
 struct fpga_region;
 
+/**
+ * struct fpga_variant - a variant  configuration for an FPGA region
+ * @node: list node for the region's variant list
+ * @name: identifier of the variant
+ * @priv: private data for the region concrete implementation
+ */
+struct fpga_variant {
+	struct list_head node;
+	const char *name;
+	void *priv;
+};
+
 /**
  * struct fpga_region_info - collection of parameters an FPGA Region
  * @mgr: fpga region manager
  * @compat_id: FPGA region id for compatibility check.
+ * @variant_list: linked list of registered variants
  * @priv: fpga region private data
  * @get_bridges: optional function to get bridges to a list
+ * @apply_variant: optional function to apply one of the variants
+ * @remove_variant: optional function to remove one of the variants
  *
  * fpga_region_info contains parameters for the register_full function.
  * These are separated into an info structure because they some are optional
@@ -26,6 +41,8 @@ struct fpga_region_info {
 	struct fpga_compat_id *compat_id;
 	void *priv;
 	int (*get_bridges)(struct fpga_region *region);
+	int (*apply_variant)(struct fpga_region *region, struct fpga_variant *variant);
+	void (*remove_variant)(struct fpga_region *region, struct fpga_variant *variant);
 };
 
 /**
@@ -37,8 +54,13 @@ struct fpga_region_info {
  * @info: FPGA image info
  * @compat_id: FPGA region id for compatibility check.
  * @ops_owner: module containing the get_bridges function
+ * @variant_list: linked list of registered variants
+ * @enabled_variant: pointer to the currently enabled variant
+ * @variant_mutex: protects the enabled variant and variant list
  * @priv: private data
  * @get_bridges: optional function to get bridges to a list
+ * @apply_variant: optional function to apply one of the variants
+ * @remove_variant: optional function to remove one of the variants
  */
 struct fpga_region {
 	struct device dev;
@@ -48,8 +70,14 @@ struct fpga_region {
 	struct fpga_image_info *info;
 	struct fpga_compat_id *compat_id;
 	struct module *ops_owner;
+	struct list_head variant_list;
+	struct fpga_variant *enabled_variant;
+	struct mutex variant_mutex; /* protects variant_list and enabled_variant */
 	void *priv;
 	int (*get_bridges)(struct fpga_region *region);
+	int (*apply_variant)(struct fpga_region *region, struct fpga_variant *variant);
+	void (*remove_variant)(struct fpga_region *region, struct fpga_variant *variant);
+
 };
 
 #define to_fpga_region(d) container_of(d, struct fpga_region, dev)
@@ -60,6 +88,10 @@ fpga_region_class_find(struct device *start, const void *data,
 
 int fpga_region_program_fpga(struct fpga_region *region);
 
+int fpga_region_add_variant(struct fpga_region *region, const char *name,
+			    bool is_enabled, void *priv);
+int fpga_region_program_variant(struct fpga_region *region, const char *name);
+
 #define fpga_region_register_full(parent, info) \
 	__fpga_region_register_full(parent, info, THIS_MODULE)
 struct fpga_region *
-- 
2.54.0

[RFC PATCH fpga/for-next 2/2] fpga: of-fpga-region: Add support for region variants

[Marco Pagani]

Extend of-fpga-region with support for FPGA region variants statically
defined in the device tree.

During probing, the code now looks for an fpga-variants container
node. If present, it parses each variant child node and registers it.
The variant that is programmed in the region at boot time is defined
by the base-variant property. Additionally, the firmware-cached
property allows caching bitstreams in memory, enabling a fast
reconfiguration path for real-time (latency-sensitive) applications.

Signed-off-by: Marco Pagani <marco.pagani@linux.dev>
---
 drivers/fpga/of-fpga-region.c | 272 +++++++++++++++++++++++++++++++++-
 1 file changed, 270 insertions(+), 2 deletions(-)

diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 9107a5b461d3..ea32775b55ce 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -8,6 +8,7 @@
 #include <linux/fpga/fpga-bridge.h>
 #include <linux/fpga/fpga-mgr.h>
 #include <linux/fpga/fpga-region.h>
+#include <linux/firmware.h>
 #include <linux/idr.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
@@ -18,6 +19,18 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 
+/**
+ * struct of_fpga_variant_priv - private data for FPGA Region variants
+ * @fw_name: name of the firmware file containing the FPGA image
+ * @fw_buf: buffer containing the cached firmware image
+ * @fw_size: size in bytes of the buffer containing the cached firmware image
+ */
+struct of_fpga_variant_priv {
+	char *fw_name;
+	void *fw_buf;
+	size_t fw_size;
+};
+
 static const struct of_device_id fpga_region_of_match[] = {
 	{ .compatible = "fpga-region", },
 	{},
@@ -140,6 +153,168 @@ static int of_fpga_region_get_bridges(struct fpga_region *region)
 	return 0;
 }
 
+static int of_fpga_remove_variant_child(struct device *dev, void *data)
+{
+	struct device_node *variant_np = data;
+
+	if (dev->of_node && dev->of_node->parent == variant_np) {
+		if (dev_is_platform(dev)) {
+			of_node_clear_flag(dev->of_node, OF_POPULATED);
+			platform_device_unregister(to_platform_device(dev));
+		}
+	}
+	return 0;
+}
+
+static int of_fpga_region_populate_variant(struct device *dev,
+					   struct device_node *variant_np)
+{
+	struct device_node *child_np;
+
+	for_each_available_child_of_node(variant_np, child_np) {
+		if (!of_platform_device_create(child_np, NULL, dev))
+			dev_warn(dev, "failed to create device node.\n");
+	}
+
+	return 0;
+}
+
+/**
+ * of_fpga_region_apply_variant - apply a specific FPGA variant to the region
+ * @region: FPGA region to be programmed
+ * @variant: FPGA variant requested to be applied
+ *
+ * Retrieves the specific variant node from the fpga-variants container,
+ * handles firmware loading (including lazy caching if firmware-cached is
+ * set), programs the FPGA region with the variant, and finally populates
+ * the platform devices for the child IPs.
+ *
+ * Return: 0 on success, or a negative error code on failure.
+ */
+static int of_fpga_region_apply_variant(struct fpga_region *region,
+					struct fpga_variant *variant)
+{
+	struct of_fpga_variant_priv *priv = variant->priv;
+	struct device *dev = &region->dev;
+	struct device_node *variants_np, *variant_np;
+	const struct firmware *fw;
+	struct fpga_image_info *info;
+	int ret;
+
+	variants_np = of_get_child_by_name(dev->of_node, "fpga-variants");
+	if (!variants_np)
+		return -ENODEV;
+
+	variant_np = of_get_child_by_name(variants_np, variant->name);
+	of_node_put(variants_np);
+	if (!variant_np)
+		return -ENODEV;
+
+	info = fpga_image_info_alloc(dev);
+	if (!info) {
+		ret = -ENOMEM;
+		goto err_put_node;
+	}
+
+	if (!of_property_read_bool(variant_np, "firmware-cached")) {
+		info->firmware_name = devm_kstrdup(dev, priv->fw_name, GFP_KERNEL);
+		if (!info->firmware_name) {
+			ret = -ENOMEM;
+			goto err_free_info;
+		}
+	} else if (priv->fw_buf) {
+		info->buf = priv->fw_buf;
+		info->count = priv->fw_size;
+	} else {
+		ret = request_firmware(&fw, priv->fw_name, dev);
+		if (ret) {
+			dev_err(dev, "failed to request firmware '%s'\n", priv->fw_name);
+			goto err_free_info;
+		}
+
+		priv->fw_buf = devm_kmemdup(dev, fw->data, fw->size, GFP_KERNEL);
+		priv->fw_size = fw->size;
+		release_firmware(fw);
+
+		if (!priv->fw_buf) {
+			ret = -ENOMEM;
+			goto err_free_info;
+		}
+
+		info->buf = priv->fw_buf;
+		info->count = priv->fw_size;
+	}
+
+	/* Checked during probing */
+	info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
+
+	of_property_read_u32(dev->of_node, "region-unfreeze-timeout-us",
+			     &info->enable_timeout_us);
+	of_property_read_u32(dev->of_node, "region-freeze-timeout-us",
+			     &info->disable_timeout_us);
+	of_property_read_u32(dev->of_node, "config-complete-timeout-us",
+			     &info->config_complete_timeout_us);
+
+	region->info = info;
+
+	if (info->firmware_name || info->buf) {
+		ret = fpga_region_program_fpga(region);
+		if (ret) {
+			dev_err(dev, "failed to program FPGA\n");
+			goto err_clear_info;
+		}
+	}
+
+	ret = of_fpga_region_populate_variant(dev, variant_np);
+	if (ret) {
+		dev_err(dev, "failed to populate variant IP nodes\n");
+		goto err_clear_info;
+	}
+
+	of_node_put(variant_np);
+	return 0;
+
+err_clear_info:
+	region->info = NULL;
+err_free_info:
+	fpga_image_info_free(info);
+err_put_node:
+	of_node_put(variant_np);
+	return ret;
+}
+
+/**
+ * of_fpga_region_remove_variant - remove the current FPGA variant from the region
+ * @region: FPGA region
+ * @variant: FPGA variant being removed
+ *
+ * Unregisters all devices that were populared for the variant's IPs
+ * during apply_variant() and frees the FPGA image.
+ */
+static void of_fpga_region_remove_variant(struct fpga_region *region,
+					  struct fpga_variant *variant)
+{
+	struct device_node *variants_np, *variant_np;
+	struct device *dev = &region->dev;
+
+	variants_np = of_get_child_by_name(dev->of_node, "fpga-variants");
+	if (variants_np) {
+		variant_np = of_get_child_by_name(variants_np, variant->name);
+		of_node_put(variants_np);
+
+		if (variant_np) {
+			device_for_each_child(dev, variant_np,
+					      of_fpga_remove_variant_child);
+			of_node_put(variant_np);
+		}
+	}
+
+	if (region->info) {
+		fpga_image_info_free(region->info);
+		region->info = NULL;
+	}
+}
+
 /**
  * child_regions_with_firmware - Used to check the child region info.
  * @overlay: device node of the overlay
@@ -397,8 +572,16 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
+	struct device_node *var_np, *base_np = NULL, *variants_np = NULL;
 	struct fpga_region *region;
 	struct fpga_manager *mgr;
+	struct fpga_region_info info = { 0 };
+	struct of_fpga_variant_priv *priv;
+	const char *base_variant_name = NULL;
+	const char *fw_name = NULL;
+	bool has_base_variant = false;
+	bool base_variant_found = false;
+	bool is_base = false;
 	int ret;
 
 	/* Find the FPGA mgr specified by region or parent region. */
@@ -406,20 +589,105 @@ static int of_fpga_region_probe(struct platform_device *pdev)
 	if (IS_ERR(mgr))
 		return -EPROBE_DEFER;
 
-	region = fpga_region_register(dev, mgr, of_fpga_region_get_bridges);
+	/* Register variants support only if the device tree is well-formed */
+	if (!of_property_read_string(np, "base-variant", &base_variant_name))
+		has_base_variant = true;
+
+	if (has_base_variant) {
+		if (!of_property_read_bool(np, "partial-fpga-config")) {
+			dev_err(dev, "Region '%s' is missing 'partial-fpga-config' property.\n",
+				np->name);
+			ret = -EINVAL;
+			goto eprobe_mgr_put;
+		}
+
+		variants_np = of_get_child_by_name(np, "fpga-variants");
+		if (!variants_np) {
+			dev_err(dev, "Missing 'fpga-variants' container node.\n");
+			ret = -EINVAL;
+			goto eprobe_mgr_put;
+		}
+
+		for_each_available_child_of_node(variants_np, var_np) {
+			if (!of_property_present(var_np, "firmware-name")) {
+				dev_err(dev, "Variant '%s' is missing 'firmware-name'.\n",
+					var_np->name);
+				of_node_put(var_np);
+				ret = -EINVAL;
+				goto eprobe_mgr_put;
+			}
+
+			if (!strcmp(var_np->name, base_variant_name))
+				base_variant_found = true;
+		}
+
+		if (!base_variant_found) {
+			dev_err(dev, "base-variant '%s' does not match.\n",
+				base_variant_name);
+			ret = -EINVAL;
+			goto eprobe_mgr_put;
+		}
+
+		info.apply_variant = of_fpga_region_apply_variant;
+		info.remove_variant = of_fpga_region_remove_variant;
+	}
+
+	info.mgr = mgr;
+	info.get_bridges = of_fpga_region_get_bridges;
+
+	region = fpga_region_register_full(dev, &info);
 	if (IS_ERR(region)) {
 		ret = PTR_ERR(region);
 		goto eprobe_mgr_put;
 	}
+	if (has_base_variant && variants_np) {
+		for_each_available_child_of_node(variants_np, var_np) {
+			priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+			if (!priv) {
+				of_node_put(var_np);
+				ret = -ENOMEM;
+				goto eprobe_region_unregister;
+			}
+
+			of_property_read_string(var_np, "firmware-name", &fw_name);
+			priv->fw_name = devm_kstrdup(dev, fw_name, GFP_KERNEL);
+			if (!priv->fw_name) {
+				of_node_put(var_np);
+				ret = -ENOMEM;
+				goto eprobe_region_unregister;
+			}
+
+			is_base = !strcmp(var_np->name, base_variant_name);
+			if (is_base)
+				base_np = of_node_get(var_np);
+
+			ret = fpga_region_add_variant(region, var_np->name, is_base, priv);
+			if (ret) {
+				dev_err(dev, "Cannot add variant '%s'.\n", var_np->name);
+				of_node_put(var_np);
+				goto eprobe_region_unregister;
+			}
+		}
+		of_node_put(variants_np);
+	}
 
 	of_platform_populate(np, fpga_region_of_match, NULL, &region->dev);
-	platform_set_drvdata(pdev, region);
 
+	if (base_np) {
+		of_fpga_region_populate_variant(&region->dev, base_np);
+		of_node_put(base_np);
+	}
+
+	platform_set_drvdata(pdev, region);
 	dev_info(dev, "FPGA Region probed\n");
 
 	return 0;
 
+eprobe_region_unregister:
+	fpga_region_unregister(region);
 eprobe_mgr_put:
+	of_node_put(base_np);
+	of_node_put(variants_np);
 	fpga_mgr_put(mgr);
 	return ret;
 }
-- 
2.54.0

Re: [RFC PATCH] fpga: region: Add support for FPGA region variants

[Xu Yilun]

On Mon, Jun 08, 2026 at 06:42:45PM +0200, Marco Pagani wrote:
> This RFC proposes a proof-of-concept implementation of FPGA region
> variants, a mechanism that introduces a common way to handle
> dynamic partial reconfiguration from userspace. The proposed approach

There are many threads talking about userspace reconfiguration, the
latest one is this:

https://lore.kernel.org/linux-fpga/20250519033950.2669858-1-nava.kishore.manne@amd.com/

Before we dive into detail, could you help tell why the previous one
won't work so we must switch to the new interface.

> is safe and aligned with the mainline kernel's stance on hardware
> management by constraining the hardware to a mutually exclusive set
> of configurations (variants) defined upfront. This is a realistic
> assumption for FPGAs, as regions are typically statically defined and
> synthesized during the system design phase. To keep the architecture
> realistic, the following additional constraints are introduced:
> (i) variants cannot be nested, and (ii) variants cannot contain FPGA
> bridges.
> 
> The interface and core logic for the variant mechanism are defined
> in the fpga-region and implemented in a backwards-compatible way.
> The fpga-region now optionally exports sysfs attributes that allow
> the user to reconfigure a region that supports variants by selecting
> one variant a list of pre-defined variants. Concrete regions can enable
> variant support by implementing the new apply_variant and remove_variant
> methods and adding them to fpga_region_info before registration.
> 
> As part of this RFC, the of-fpga-region concrete region has been extended
> to implement the variant interface. Variants are statically specified in
> the device tree using an fpga-variants node. Additionally, it introduces
> a firmware-cached property to cache bitstreams in memory, enabling a fast
> reconfiguration path for real-time (latency-sensitive) applications.
> 
> Below is an example of how variants can be statically defined in the
> device tree under this architecture:
> 
> fake_mgr: fpga-mgr@0 {
>         compatible = "linux,fake-fpga-mgr";
> };
> 
> fpga_region: fpga-region@0 {
>         compatible = "fpga-region";
>         #address-cells = <2>;
>         #size-cells = <2>;
>         ranges;
> 
>         /* FPGA region properties */
>         fpga-mgr = <&fake_mgr>;
>         partial-fpga-config;
>         region-unfreeze-timeout-us = <10000>;
> 
>         /* Base variant */
>         base-variant = "variant-1";

Previously we use DT-overlay file, here we use a list of variants. I'm
actually not sure why it is safer or better managed. As far as I can
tell, the only difference is that DT-overlay puts the region descriptions
in different files/blobs, while your fpga-variant puts them together in
one file/node, and we switch from choosing a file/blob to choosing a
string, is it?

> 
>         /* Variants container node */
>         fpga-variants {
>                 #address-cells = <2>;
>                 #size-cells = <2>;
>                 ranges;
> 
>                 variant-1 {
>                         firmware-name = "variant1-image.bin";
> 
>                         #address-cells = <2>;
>                         #size-cells = <2>;
>                         ranges;
> 
>                         variant_1_ip: ip_1@10000 {
>                                 compatible = "fake,ip_1";
>                                 reg = <0x0 0x10000 0x0 0x1000>;
>                         };
>                 };
> 
>                 variant-2 {
>                         firmware-name = "variant2-image.bin";
>                         firmware-cached;
> 
>                         #address-cells = <2>;
>                         #size-cells = <2>;
>                         ranges;
> 
>                         variant_2_ip: ip_2@10000 {
>                                 compatible = "fake,ip_2";
>                                 reg = <0x0 0x20000 0x0 0x1000>;
>                         };
>                 };
>         };
> };

Re: [RFC PATCH] fpga: region: Add support for FPGA region variants

[Marco Pagani]

On 26/06/2026 14:58, Xu Yilun wrote:
> On Mon, Jun 08, 2026 at 06:42:45PM +0200, Marco Pagani wrote:
>> This RFC proposes a proof-of-concept implementation of FPGA region
>> variants, a mechanism that introduces a common way to handle
>> dynamic partial reconfiguration from userspace. The proposed approach
> 
> There are many threads talking about userspace reconfiguration, the
> latest one is this:
> 
> https://lore.kernel.org/linux-fpga/20250519033950.2669858-1-nava.kishore.manne@amd.com/
> 
> Before we dive into detail, could you help tell why the previous one
> won't work so we must switch to the new interface.

I think that Nava's latest RFC and older proposals for userspace FPGA
configuration are in effect functionally equivalent to Antoniou's RFC
for runtime DT changes via ConfigFS. In other words, they get around
the limitations of the current mainline kernel by using a different
implementation of the same core idea behind Antoniou's RFC, but do not
address the fundamental concerns that led to it being NACKed.

Concerning Nava's latest RFC, scoping the ConfigFS entry point to
/sys/kernel/config/fpga_region/ does not change the underlying code path
in the kernel and leaves the same attack vector open. Userspace can
still load any arbitrary hardware configuration at runtime and mess up
with registers, clocks, DMA mappings, etc. to attack the kernel from
"below" accessing potentially sensitive memory areas.

Conversely, this RFC addresses these concerns by constraining possible
configurations to a limited and statically-defined set that can be
authenticated as part of the DT at boot time to maintain the chain
of trust. This is in accordance with the kernel's security model
which holds that userspace cannot be trusted to define hardware.


>> is safe and aligned with the mainline kernel's stance on hardware
>> management by constraining the hardware to a mutually exclusive set
>> of configurations (variants) defined upfront. This is a realistic
>> assumption for FPGAs, as regions are typically statically defined and
>> synthesized during the system design phase. To keep the architecture
>> realistic, the following additional constraints are introduced:
>> (i) variants cannot be nested, and (ii) variants cannot contain FPGA
>> bridges.
>>
>> The interface and core logic for the variant mechanism are defined
>> in the fpga-region and implemented in a backwards-compatible way.
>> The fpga-region now optionally exports sysfs attributes that allow
>> the user to reconfigure a region that supports variants by selecting
>> one variant a list of pre-defined variants. Concrete regions can enable
>> variant support by implementing the new apply_variant and remove_variant
>> methods and adding them to fpga_region_info before registration.
>>
>> As part of this RFC, the of-fpga-region concrete region has been extended
>> to implement the variant interface. Variants are statically specified in
>> the device tree using an fpga-variants node. Additionally, it introduces
>> a firmware-cached property to cache bitstreams in memory, enabling a fast
>> reconfiguration path for real-time (latency-sensitive) applications.
>>
>> Below is an example of how variants can be statically defined in the
>> device tree under this architecture:
>>
>> fake_mgr: fpga-mgr@0 {
>>         compatible = "linux,fake-fpga-mgr";
>> };
>>
>> fpga_region: fpga-region@0 {
>>         compatible = "fpga-region";
>>         #address-cells = <2>;
>>         #size-cells = <2>;
>>         ranges;
>>
>>         /* FPGA region properties */
>>         fpga-mgr = <&fake_mgr>;
>>         partial-fpga-config;
>>         region-unfreeze-timeout-us = <10000>;
>>
>>         /* Base variant */
>>         base-variant = "variant-1";
> 
> Previously we use DT-overlay file, here we use a list of variants. I'm
> actually not sure why it is safer or better managed. As far as I can
> tell, the only difference is that DT-overlay puts the region descriptions
> in different files/blobs, while your fpga-variant puts them together in
> one file/node, and we switch from choosing a file/blob to choosing a
> string, is it?

For the reasons described above, I think switching from allowing
userspace to load an arbitrary hardware configuration file/blob to
having userspace merely selecting from a pre-authenticated, mutually
exclusive, set of hardware configurations via a string provides
considerable security advantages.

Regarding using the sysfs interface itself, I don't have a strong
opinion. I used sysfs for this RFC since it is a simple approach that is
similar to the one that has already been validated by other subsystems
such as pinctrl. However, I'm open to other alternatives.

Finally, please consider that FPGA variants are not specific or limited
to OF/DT systems. The core of the variants mechanism is generic and
implemented at the region level to remain backward compatible with
existing concrete regions. As part of this RFC, I provided an
implementation of variants for of-fpga-region because it is a relevant
use case for embedded systems.


>>
>>         /* Variants container node */
>>         fpga-variants {
>>                 #address-cells = <2>;
>>                 #size-cells = <2>;
>>                 ranges;
>>
>>                 variant-1 {
>>                         firmware-name = "variant1-image.bin";
>>
>>                         #address-cells = <2>;
>>                         #size-cells = <2>;
>>                         ranges;
>>
>>                         variant_1_ip: ip_1@10000 {
>>                                 compatible = "fake,ip_1";
>>                                 reg = <0x0 0x10000 0x0 0x1000>;
>>                         };
>>                 };
>>
>>                 variant-2 {
>>                         firmware-name = "variant2-image.bin";
>>                         firmware-cached;
>>
>>                         #address-cells = <2>;
>>                         #size-cells = <2>;
>>                         ranges;
>>
>>                         variant_2_ip: ip_2@10000 {
>>                                 compatible = "fake,ip_2";
>>                                 reg = <0x0 0x20000 0x0 0x1000>;
>>                         };
>>                 };
>>         };
>> };

Thanks,
Marco

Re: [RFC PATCH] fpga: region: Add support for FPGA region variants

[Xu Yilun]

On Tue, Jun 30, 2026 at 11:39:50AM +0200, Marco Pagani wrote:
> 
> 
> On 26/06/2026 14:58, Xu Yilun wrote:
> > On Mon, Jun 08, 2026 at 06:42:45PM +0200, Marco Pagani wrote:
> >> This RFC proposes a proof-of-concept implementation of FPGA region
> >> variants, a mechanism that introduces a common way to handle
> >> dynamic partial reconfiguration from userspace. The proposed approach
> > 
> > There are many threads talking about userspace reconfiguration, the
> > latest one is this:
> > 
> > https://lore.kernel.org/linux-fpga/20250519033950.2669858-1-nava.kishore.manne@amd.com/
> > 
> > Before we dive into detail, could you help tell why the previous one
> > won't work so we must switch to the new interface.
> 
> I think that Nava's latest RFC and older proposals for userspace FPGA
> configuration are in effect functionally equivalent to Antoniou's RFC
> for runtime DT changes via ConfigFS. In other words, they get around

I think it is different. Arbitrarily changing any hardware is
problematic, it doesn't align with how the hardware works. But changing
the hardware inside an FPGA region is exactly the functionality of an
FPGA. As long as we stay within the scope of fpga_region, I don't see
the immediate problem.

> the limitations of the current mainline kernel by using a different
> implementation of the same core idea behind Antoniou's RFC, but do not
> address the fundamental concerns that led to it being NACKed.
> 
> Concerning Nava's latest RFC, scoping the ConfigFS entry point to
> /sys/kernel/config/fpga_region/ does not change the underlying code path
> in the kernel and leaves the same attack vector open. Userspace can
> still load any arbitrary hardware configuration at runtime and mess up
> with registers, clocks, DMA mappings, etc. to attack the kernel from
> "below" accessing potentially sensitive memory areas.

If the implementation does go outside fpga_region, fix it.

> 
> Conversely, this RFC addresses these concerns by constraining possible
> configurations to a limited and statically-defined set that can be
> authenticated as part of the DT at boot time to maintain the chain
> of trust. This is in accordance with the kernel's security model

I'm not sure why a DT at boot time could be verified as trusted while a
runtime overlay can't. Root of trust has anything to do with boot time
or runtime?

Or do you mean a DT boot time authentication could ensure the DT
description exactly match the content of a bin file (variant1-image.bin)
on disk, how?

> which holds that userspace cannot be trusted to define hardware.

I'm not sure, my understanding is FPGA programing is much like firmware
loading, whose correctness/trusty can't be ensured by kernel security
model. Only cryptography can help. Without crypto, kernel still loads
the selected binary, if it corrupts, let it corrupt.

Re: [RFC PATCH] fpga: region: Add support for FPGA region variants

[Marco Pagani]

On 30/06/2026 16:58, Xu Yilun wrote:
> On Tue, Jun 30, 2026 at 11:39:50AM +0200, Marco Pagani wrote:
>>
>>
>> On 26/06/2026 14:58, Xu Yilun wrote:
>>> On Mon, Jun 08, 2026 at 06:42:45PM +0200, Marco Pagani wrote:
>>>> This RFC proposes a proof-of-concept implementation of FPGA region
>>>> variants, a mechanism that introduces a common way to handle
>>>> dynamic partial reconfiguration from userspace. The proposed approach
>>>
>>> There are many threads talking about userspace reconfiguration, the
>>> latest one is this:
>>>
>>> https://lore.kernel.org/linux-fpga/20250519033950.2669858-1-nava.kishore.manne@amd.com/
>>>
>>> Before we dive into detail, could you help tell why the previous one
>>> won't work so we must switch to the new interface.
>>
>> I think that Nava's latest RFC and older proposals for userspace FPGA
>> configuration are in effect functionally equivalent to Antoniou's RFC
>> for runtime DT changes via ConfigFS. In other words, they get around
> 
> I think it is different. Arbitrarily changing any hardware is
> problematic, it doesn't align with how the hardware works. But changing
> the hardware inside an FPGA region is exactly the functionality of an
> FPGA. As long as we stay within the scope of fpga_region, I don't see
> the immediate problem.

I agree that changing the hardware is the normal use case for FPGA.
However, my point is that allowing userspace to arbitrarily change the
hardware configuration/topology of the kernel at runtime by loading a DT
overlay through ConfigFS opens a considerable attack vector, and this is
what led to Antoniou's RFC being rejected in the first place.

>> the limitations of the current mainline kernel by using a different
>> implementation of the same core idea behind Antoniou's RFC, but do not
>> address the fundamental concerns that led to it being NACKed.
>>
>> Concerning Nava's latest RFC, scoping the ConfigFS entry point to
>> /sys/kernel/config/fpga_region/ does not change the underlying code path
>> in the kernel and leaves the same attack vector open. Userspace can
>> still load any arbitrary hardware configuration at runtime and mess up
>> with registers, clocks, DMA mappings, etc. to attack the kernel from
>> "below" accessing potentially sensitive memory areas.
> 
> If the implementation does go outside fpga_region, fix it.

My understanding is that Nava's RFC already scopes the configuration to
/sys/kernel/config/fpga_region/. However, that would still leave the
attack vector open. A malevolent userspace component could still load a
malicious DT overlay with a rogue "ranges" property to access kernel
memory or a rouge "dma-ranges" property to hijack a legitimate IP for
the same purpose, or it can mess with the clock configuration. It also
worth considering that DT changes are applied to the global kernel
hardware configuration and cannot be sandboxed to the specific FPGA
region. The root of the problem is that we cannot check and guarantee
at runtime that an arbitrary DT overlay is sane and affects only its
specific FPGA region.

>>
>> Conversely, this RFC addresses these concerns by constraining possible
>> configurations to a limited and statically-defined set that can be
>> authenticated as part of the DT at boot time to maintain the chain
>> of trust. This is in accordance with the kernel's security model
> 
> I'm not sure why a DT at boot time could be verified as trusted while a
> runtime overlay can't. Root of trust has anything to do with boot time
> or runtime?
> 
> Or do you mean a DT boot time authentication could ensure the DT
> description exactly match the content of a bin file (variant1-image.bin)
> on disk, how?

I mean, the base DT of a system can be authenticated at boot-time
using a signed FIT image. Since FPGA variants are statically defined
within the base DT, their configurations are also authenticated
(together with the kernel) to maintain a chain of trust. However, I
think the main benefit of the variants approach is that it guarantees
better security and practicality by default, even without boot image
authentication, as it leaves no doors open for userspace to modify the
kernel hardware configuration/topology.

On the contrary, while runtime DT overlays could theoretically be
secured, enforcing mandatory signature verification for every DTBO
loaded through ConfigFS at runtime is highly impractical as it
introduces key-management complexities that would make the approach
cumbersome for many use cases. Conversely, leaving the interface as
it is now in Nava's RFC fundamentally trusts userspace to dynamically
change the kernel's hardware configuration/topology, and this again
leaves the attack vector open.

>> which holds that userspace cannot be trusted to define hardware.
> 
> I'm not sure, my understanding is FPGA programing is much like firmware
> loading, whose correctness/trusty can't be ensured by kernel security
> model. Only cryptography can help. Without crypto, kernel still loads
> the selected binary, if it corrupts, let it corrupt.

It is true that the kernel allows to load arbitrary firmware blobs at
runtime. However, those are for configuring/programming only the device
hardware and not for the kernel itself. What is particularly dangerous
with runtime DT overlays is that they can change not only the device's
firmware/image but also the kernel configuration/toplogy itself.
A single malicious DT overlay can compromise the entire kernel and
hence the system, not just the FPGA fabric.


Thanks,
Marco

Re: [RFC PATCH] fpga: region: Add support for FPGA region variants

[Xu Yilun]

> My understanding is that Nava's RFC already scopes the configuration to
> /sys/kernel/config/fpga_region/. However, that would still leave the
> attack vector open. A malevolent userspace component could still load a
> malicious DT overlay with a rogue "ranges" property to access kernel
> memory or a rouge "dma-ranges" property to hijack a legitimate IP for
> the same purpose, or it can mess with the clock configuration. It also
> worth considering that DT changes are applied to the global kernel
> hardware configuration and cannot be sandboxed to the specific FPGA
> region. The root of the problem is that we cannot check and guarantee
> at runtime that an arbitrary DT overlay is sane and affects only its
> specific FPGA region.

I see. So your concern is the DT overlay can impact the outside world by
referencing global nodes. My idea is, if we could statically specify the
fpga-region with these assigned resources, no updating of these static
properties, and don't allow global referencing phandle, we are good?

I mean we don't have to verbose on every combination of hardware that a
user might load. We harden the boundaries of the resources (ranges,
dma-ranges, ...) that are initially designed for the fpga-region in base
DT. To me, this seems to be more aligned to "DT describes the HW" and
how the FPGA works.

How do you think?

Re: [RFC PATCH] fpga: region: Add support for FPGA region variants

[Marco Pagani]

On 07/07/2026 10:02, Xu Yilun wrote:
>> My understanding is that Nava's RFC already scopes the configuration to
>> /sys/kernel/config/fpga_region/. However, that would still leave the
>> attack vector open. A malevolent userspace component could still load a
>> malicious DT overlay with a rogue "ranges" property to access kernel
>> memory or a rouge "dma-ranges" property to hijack a legitimate IP for
>> the same purpose, or it can mess with the clock configuration. It also
>> worth considering that DT changes are applied to the global kernel
>> hardware configuration and cannot be sandboxed to the specific FPGA
>> region. The root of the problem is that we cannot check and guarantee
>> at runtime that an arbitrary DT overlay is sane and affects only its
>> specific FPGA region.
> 
> I see. So your concern is the DT overlay can impact the outside world by
> referencing global nodes. My idea is, if we could statically specify the
> fpga-region with these assigned resources, no updating of these static
> properties, and don't allow global referencing phandle, we are good?

I think I see what you mean: since the hardware interface of the region
is fixed and defined during the design phase, we could enforce the
resource boundaries (e.g., ranges, dma-ranges, interrupts, clocks, etc.)
defined in the base DT. This is certainly true. However, I think the
fundamental problem is that safely enforcing these boundaries in-kernel
at runtime remains very challenging.

As far as I know, there isn't a way to sandbox changes made by
of_overlay_fdt_apply(), as it simply digests what it is fed and applies
it globally against the live tree. Therefore, we would need to add a
runtime DT verifier that parses and validates DTO blobs, before calling
of_overlay_fdt_apply(), to ensure that they don't contain rogue global
phandles, and touch only nodes defined in the region. Adding such a
runtime DT verifier seems to me an over-complicated and inherently
fragile approach. Moreover, it can add a weak spot that attackers might
constantly attempt to exploit. Ultimately, I think that a static
whitelisting approach carries many benefits in security, simplicity, and
maintainability over building and maintaining a runtime verifier directly
exposed to userspace.

> I mean we don't have to verbose on every combination of hardware that a
> user might load. We harden the boundaries of the resources (ranges,
> dma-ranges, ...) that are initially designed for the fpga-region in base
> DT. To me, this seems to be more aligned to "DT describes the HW" and
> how the FPGA works.

If I understood correctly, your main concern is that having to specify
all region variants at boot time is verbose and limiting. I see your
point. However, I still think that a statically-defined whitelist of
variants is the most pragmatic approach, as it guarantees the best
security while matching the actual FPGA design phase, where
reconfigurable regions are floorplanned and synthesized, naturally
resulting in a statically-defined set of partial images/bitstreams with
their corresponding kernel configurations.


Thanks,
Marco