>
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index dc8d3a13f5..2eb5734f8e 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -54,6 +54,7 @@ obj-y += wait.o
> obj-bin-y += warning.init.o
> obj-$(CONFIG_XENOPROF) += xenoprof.o
> obj-y += xmalloc_tlsf.o
> +obj-$(CONFIG_OVERLAY_DTB) += dt_overlay.o
I think the entries are in alphabetical order, this should be added after +=
domain.o
> +/*
> + * overlay_get_nodes_info will get the all node's full name with path. This
> is
> + * useful when checking node for duplication i.e. dtbo tries to add nodes
> which
> + * already exists in device tree.
> + */
> +static int overlay_get_nodes_info(const void *fdto, char ***nodes_full_path,
> + unsigned int num_overlay_nodes)
> +{
> + int fragment;
> + unsigned int node_num = 0;
> +
> + *nodes_full_path = xmalloc_bytes(num_overlay_nodes * sizeof(char *));
Here you can use xzalloc_bytes and...
> +
> + if ( *nodes_full_path == NULL )
> + return -ENOMEM;
> + memset(*nodes_full_path, 0x0, num_overlay_nodes * sizeof(char *));
Get rid of this memset
> +
> +/* Remove nodes from dt_host. */
> +static int remove_nodes(char **full_dt_node_path, int **nodes_irq,
> + int *node_num_irq, unsigned int num_nodes)
> +{
> + struct domain *d = hardware_domain;
> + int rc = 0;
> + struct dt_device_node *overlay_node;
> + unsigned int naddr;
> + unsigned int i, j, nirq;
> + u64 addr, size;
> + domid_t domid = 0;
> +
> + for ( j = 0; j < num_nodes; j++ )
> + {
> + dt_dprintk("Finding node %s in the dt_host\n", full_dt_node_path[j]);
> +
> + overlay_node = dt_find_node_by_path(full_dt_node_path[j]);
> +
> + if ( overlay_node == NULL )
> + {
> + printk(XENLOG_ERR "Device %s is not present in the tree.
> Removing nodes failed\n",
> + full_dt_node_path[j]);
> + return -EINVAL;
> + }
> +
> + domid = dt_device_used_by(overlay_node);
> +
> + dt_dprintk("Checking if node %s is used by any domain\n",
> + full_dt_node_path[j]);
> +
> + /* Remove the node iff it's assigned to domain 0 or domain io. */
> + if ( domid != 0 && domid != DOMID_IO )
> + {
> + printk(XENLOG_ERR "Device %s as it is being used by domain %d.
> Removing nodes failed\n",
> + full_dt_node_path[j], domid);
> + return -EINVAL;
> + }
> +
> + dt_dprintk("Removing node: %s\n", full_dt_node_path[j]);
> +
> + nirq = node_num_irq[j];
> +
> + /* Remove IRQ permission */
> + for ( i = 0; i < nirq; i++ )
> + {
> + rc = nodes_irq[j][i];
> + /*
> + * TODO: We don't handle shared IRQs for now. So, it is assumed
> that
> + * the IRQs was not shared with another domain.
> + */
> + rc = irq_deny_access(d, rc);
> + if ( rc )
> + {
> + printk(XENLOG_ERR "unable to revoke access for irq %u for
> %s\n",
> + i, dt_node_full_name(overlay_node));
> + return rc;
> + }
> + }
> +
> + rc = iommu_remove_dt_device(overlay_node);
> + if ( rc != 0 && rc != -ENXIO )
> + return rc;
> +
> + naddr = dt_number_of_address(overlay_node);
> +
> + /* Remove mmio access. */
> + for ( i = 0; i < naddr; i++ )
> + {
> + rc = dt_device_get_address(overlay_node, i, &addr, &size);
> + if ( rc )
> + {
> + printk(XENLOG_ERR "Unable to retrieve address %u for %s\n",
> + i, dt_node_full_name(overlay_node));
> + return rc;
> + }
> +
> + rc = iomem_deny_access(d, paddr_to_pfn(addr),
> + paddr_to_pfn(PAGE_ALIGN(addr + size -
> 1)));
> + if ( rc )
> + {
> + printk(XENLOG_ERR "Unable to remove dom%d access to"
> + " 0x%"PRIx64" - 0x%"PRIx64"\n",
> + d->domain_id,
> + addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
NIT: here in each line under XENLOG_ERR, there is an extra space, these lines
Could be aligned to XENLOG_ERR, just for code style purpose.
> + return rc;
> + }
> + }
> +
> + rc = dt_overlay_remove_node(overlay_node);
> + if ( rc )
> + return rc;
> + }
> +
> + return rc;
> +}
> +
>
> +long dt_sysctl(struct xen_sysctl *op)
> +{
> + long ret = 0;
> + void *overlay_fdt;
> + char **nodes_full_path = NULL;
> + unsigned int num_overlay_nodes = 0;
> +
> + if ( op->u.dt_overlay.overlay_fdt_size <= 0 )
> + return -EINVAL;
> +
> + overlay_fdt = xmalloc_bytes(op->u.dt_overlay.overlay_fdt_size);
> +
> + if ( overlay_fdt == NULL )
> + return -ENOMEM;
> +
> + ret = copy_from_guest(overlay_fdt, op->u.dt_overlay.overlay_fdt,
> + op->u.dt_overlay.overlay_fdt_size);
> + if ( ret )
> + {
> + gprintk(XENLOG_ERR, "copy from guest failed\n");
> + xfree(overlay_fdt);
> +
> + return -EFAULT;
> + }
> +
> + switch ( op->u.dt_overlay.overlay_op )
> + {
> + case XEN_SYSCTL_DT_OVERLAY_REMOVE:
> + ret = check_overlay_fdt(overlay_fdt,
> + op->u.dt_overlay.overlay_fdt_size);
> + if ( ret )
> + {
> + ret = -EFAULT;
> + break;
> + }
> +
> + num_overlay_nodes = overlay_node_count(overlay_fdt);
> + if ( num_overlay_nodes == 0 )
> + {
> + ret = -ENOMEM;
> + break;
> + }
> +
> + ret = overlay_get_nodes_info(overlay_fdt, &nodes_full_path,
> + num_overlay_nodes);
> + if ( ret )
> + break;
> +
> + ret = handle_remove_overlay_nodes(nodes_full_path,
> + num_overlay_nodes);
> + break;
> +
> + default:
> + break;
> + }
> +
> + if ( nodes_full_path != NULL )
> + {
> + int I;
unsigned int
> + for ( i = 0; i < num_overlay_nodes && nodes_full_path[i] != NULL;
> i++ )
> + {
> + xfree(nodes_full_path[i]);
> + }
> + xfree(nodes_full_path);
> + }
> +
> + return ret;
> +}
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index fc4a0b31d6..d685c07159 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -29,6 +29,10 @@
> #include <xen/livepatch.h>
> #include <xen/coverage.h>
>
> +#ifdef CONFIG_OVERLAY_DTB
> +#include <xen/dt_overlay.h>
> +#endif
Maybe this header can be included anyway, removing ifdefs, and...
> +
> long cf_check do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> {
> long ret = 0;
> @@ -482,6 +486,12 @@ long cf_check
> do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> copyback = 1;
> break;
>
> +#ifdef CONFIG_OVERLAY_DTB
> + case XEN_SYSCTL_overlay:
> + ret = dt_sysctl(op);
> + break;
> +#endif
Also here you can remove ifdefs and use the header to switch between the real
implementation
or a static inline returning error if CONFIG_OVERLAY_DTB is not enabled, take a
look in
livepatch_op(struct xen_sysctl_livepatch_op *op).
dt_sysctl can take struct xen_sysctl_dt_overlay* as input.
> +
> default:
> ret = arch_do_sysctl(op, u_sysctl);
> copyback = 0;
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 55252e97f2..e256aeb7c6 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -1069,6 +1069,22 @@ typedef struct xen_sysctl_cpu_policy
> xen_sysctl_cpu_policy_t;
> DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
> #endif
>
> +#define XEN_SYSCTL_DT_OVERLAY_REMOVE 2
> +
> +/*
> + * XEN_SYSCTL_dt_overlay
> + * Performs addition/removal of device tree nodes under parent node using
> dtbo.
> + * This does in three steps:
> + * - Adds/Removes the nodes from dt_host.
> + * - Adds/Removes IRQ permission for the nodes.
> + * - Adds/Removes MMIO accesses.
> + */
> +struct xen_sysctl_dt_overlay {
> + XEN_GUEST_HANDLE_64(void) overlay_fdt;
> + uint32_t overlay_fdt_size; /* Overlay dtb size. */
> + uint8_t overlay_op; /* Add or remove. */
> +};
> +
> struct xen_sysctl {
> uint32_t cmd;
> #define XEN_SYSCTL_readconsole 1
> @@ -1099,6 +1115,7 @@ struct xen_sysctl {
> #define XEN_SYSCTL_livepatch_op 27
> /* #define XEN_SYSCTL_set_parameter 28 */
> #define XEN_SYSCTL_get_cpu_policy 29
> +#define XEN_SYSCTL_dt_overlay 30
> uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
> union {
> struct xen_sysctl_readconsole readconsole;
> @@ -1129,6 +1146,7 @@ struct xen_sysctl {
> #if defined(__i386__) || defined(__x86_64__)
> struct xen_sysctl_cpu_policy cpu_policy;
> #endif
> + struct xen_sysctl_dt_overlay dt_overlay;
Here I would need an opinion from someone more experienced, but I think when a
change
is made in this structure, XEN_SYSCTL_INTERFACE_VERSION should be bumped?
> uint8_t pad[128];
> } u;
> };
> diff --git a/xen/include/xen/dt_overlay.h b/xen/include/xen/dt_overlay.h
> new file mode 100644
> index 0000000000..525818b77c
> --- /dev/null
> +++ b/xen/include/xen/dt_overlay.h
> @@ -0,0 +1,47 @@
> +/*
> + * xen/common/dt_overlay.c
Typo: dt_overlay.h
> + *
> + * Device tree overlay suppoert in Xen.
Typo: support
> + *
> + * Copyright (c) 2021 Xilinx Inc.
> + * Written by Vikram Garhwal <fnu.vik...@xilinx.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms and conditions of the GNU General Public
> + * License, version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#ifndef __XEN_DT_SYSCTL_H__
> +#define __XEN_DT_SYSCTL_H__
> +
> +#include <xen/list.h>
> +#include <xen/libfdt/libfdt.h>
> +#include <xen/device_tree.h>
> +#include <public/sysctl.h>
In case you decide to pass struct xen_sysctl_dt_overlay to dt_sysctl, you can
remove
#include <public/sysctl.h> and use a forward declaration to struct
xen_sysctl_dt_overlay
instead.
> +
> +/*
> + * overlay_node_track describes information about added nodes through dtbo.
> + * @entry: List pointer.
> + * @dt_host_new: Pointer to the updated dt_host_new unflattened 'updated
> fdt'.
> + * @fdt: Stores the fdt.
> + * @nodes_fullname: Stores the full name of nodes.
> + * @nodes_irq: Stores the IRQ added from overlay dtb.
> + * @node_num_irq: Stores num of IRQ for each node in overlay dtb.
> + * @num_nodes: Stores total number of nodes in overlay dtb.
> + */
> +struct overlay_track {
> + struct list_head entry;
> + struct dt_device_node *dt_host_new;
> + void *fdt;
> + char **nodes_fullname;
> + int **nodes_irq;
> + int *node_num_irq;
> + unsigned int num_nodes;
> +};
> +
> +long dt_sysctl(struct xen_sysctl *op);
> +#endif
>
