Similarly as the device attachment from DT overlay to domain, this
commit implements the device detachment from domain. The DOMCTL
XEN_DOMCTL_dt_overlay op is extended to have the operation
XEN_DOMCTL_DT_OVERLAY_DETACH. The detachment of the device is
implemented by unmapping the IRQ and IOMMU resources. Note that with
these changes, the device de-registration from the IOMMU driver should
only happen at the time when the DT overlay is removed from the Xen
device tree.
Signed-off-by: Henry Wang <xin.wang2@amd.com>
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
v4:
- Split the original patch, only do device detachment from domain.
---
xen/common/dt-overlay.c | 243 ++++++++++++++++++++++++++++--------
xen/include/public/domctl.h | 3 +-
2 files changed, 194 insertions(+), 52 deletions(-)
diff --git a/xen/common/dt-overlay.c b/xen/common/dt-overlay.c
index 1087f9b502..693b6e4777 100644
--- a/xen/common/dt-overlay.c
+++ b/xen/common/dt-overlay.c
@@ -392,24 +392,100 @@ find_track_entry_from_tracker(const void *overlay_fdt,
return entry;
}
+static int remove_irq(unsigned long s, unsigned long e, void *data)
+{
+ struct domain *d = data;
+ int rc = 0;
+
+ /*
+ * IRQ should always have access unless there are duplication of
+ * of irqs in device tree. There are few cases of xen device tree
+ * where there are duplicate interrupts for the same node.
+ */
+ if (!irq_access_permitted(d, s))
+ return 0;
+ /*
+ * 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, s);
+ if ( rc )
+ {
+ printk(XENLOG_ERR "unable to revoke access for irq %ld\n", s);
+ return rc;
+ }
+
+ rc = release_guest_irq(d, s);
+ if ( rc )
+ {
+ printk(XENLOG_ERR "unable to release irq %ld\n", s);
+ return rc;
+ }
+
+ return rc;
+}
+
+static int remove_all_irqs(struct rangeset *irq_ranges, struct domain *d)
+{
+ return rangeset_report_ranges(irq_ranges, 0, ~0UL, remove_irq, d);
+}
+
+static int remove_iomem(unsigned long s, unsigned long e, void *data)
+{
+ struct domain *d = data;
+ int rc = 0;
+ p2m_type_t t;
+ mfn_t mfn;
+
+ mfn = p2m_lookup(d, _gfn(s), &t);
+ if ( mfn_x(mfn) == 0 || mfn_x(mfn) == ~0UL )
+ return -EINVAL;
+
+ rc = iomem_deny_access(d, s, e);
+ if ( rc )
+ {
+ printk(XENLOG_ERR "Unable to remove %pd access to %#lx - %#lx\n",
+ d, s, e);
+ return rc;
+ }
+
+ rc = unmap_mmio_regions(d, _gfn(s), e - s, _mfn(s));
+ if ( rc )
+ return rc;
+
+ return rc;
+}
+
+static int remove_all_iomems(struct rangeset *iomem_ranges, struct domain *d)
+{
+ return rangeset_report_ranges(iomem_ranges, 0, ~0UL, remove_iomem, d);
+}
+
/* Check if node itself can be removed and remove node from IOMMU. */
-static int remove_node_resources(struct dt_device_node *device_node)
+static int remove_node_resources(struct dt_device_node *device_node,
+ struct domain *d)
{
int rc = 0;
unsigned int len;
domid_t domid;
- domid = dt_device_used_by(device_node);
+ if ( !d )
+ {
+ domid = dt_device_used_by(device_node);
- dt_dprintk("Checking if node %s is used by any domain\n",
- device_node->full_name);
+ dt_dprintk("Checking if node %s is used by any domain\n",
+ device_node->full_name);
- /* Remove the node if only it's assigned to hardware domain or domain io. */
- if ( domid != hardware_domain->domain_id && domid != DOMID_IO )
- {
- printk(XENLOG_ERR "Device %s is being used by domain %u. Removing nodes failed\n",
- device_node->full_name, domid);
- return -EINVAL;
+ /*
+ * We also check if device is assigned to DOMID_IO as when a domain
+ * is destroyed device is assigned to DOMID_IO.
+ */
+ if ( domid != DOMID_IO )
+ {
+ printk(XENLOG_ERR "Device %s is being assigned to %u. Device is assigned to %d\n",
+ device_node->full_name, DOMID_IO, domid);
+ return -EINVAL;
+ }
}
/* Check if iommu property exists. */
@@ -417,9 +493,12 @@ static int remove_node_resources(struct dt_device_node *device_node)
{
if ( dt_device_is_protected(device_node) )
{
- rc = iommu_remove_dt_device(device_node);
- if ( rc < 0 )
- return rc;
+ if ( !list_empty(&device_node->domain_list) )
+ {
+ rc = iommu_deassign_dt_device(d, device_node);
+ if ( rc < 0 )
+ return rc;
+ }
}
}
@@ -428,7 +507,8 @@ static int remove_node_resources(struct dt_device_node *device_node)
/* Remove all descendants from IOMMU. */
static int
-remove_descendant_nodes_resources(const struct dt_device_node *device_node)
+remove_descendant_nodes_resources(const struct dt_device_node *device_node,
+ struct domain *d)
{
int rc = 0;
struct dt_device_node *child_node;
@@ -438,12 +518,12 @@ remove_descendant_nodes_resources(const struct dt_device_node *device_node)
{
if ( child_node->child )
{
- rc = remove_descendant_nodes_resources(child_node);
+ rc = remove_descendant_nodes_resources(child_node, d);
if ( rc )
return rc;
}
- rc = remove_node_resources(child_node);
+ rc = remove_node_resources(child_node, d);
if ( rc )
return rc;
}
@@ -456,8 +536,7 @@ static int remove_nodes(const struct overlay_track *tracker)
{
int rc = 0;
struct dt_device_node *overlay_node;
- unsigned int j;
- struct domain *d = hardware_domain;
+ unsigned int j, len;
for ( j = 0; j < tracker->num_nodes; j++ )
{
@@ -467,18 +546,15 @@ static int remove_nodes(const struct overlay_track *tracker)
write_lock(&dt_host_lock);
- rc = remove_descendant_nodes_resources(overlay_node);
- if ( rc )
+ /* Check if iommu property exists. */
+ if ( dt_get_property(overlay_node, "iommus", &len) )
{
- write_unlock(&dt_host_lock);
- return rc;
- }
-
- rc = remove_node_resources(overlay_node);
- if ( rc )
- {
- write_unlock(&dt_host_lock);
- return rc;
+ if ( dt_device_is_protected(overlay_node) )
+ {
+ rc = iommu_remove_dt_device(overlay_node);
+ if ( rc < 0 )
+ return rc;
+ }
}
dt_dprintk("Removing node: %s\n", overlay_node->full_name);
@@ -493,22 +569,6 @@ static int remove_nodes(const struct overlay_track *tracker)
write_unlock(&dt_host_lock);
}
- /* Remove IRQ access. */
- if ( tracker->irq_ranges )
- {
- rc = rangeset_consume_ranges(tracker->irq_ranges, irq_remove_cb, d);
- if ( rc )
- return rc;
- }
-
- /* Remove mmio access. */
- if ( tracker->iomem_ranges )
- {
- rc = rangeset_consume_ranges(tracker->iomem_ranges, iomem_remove_cb, d);
- if ( rc )
- return rc;
- }
-
return rc;
}
@@ -534,7 +594,6 @@ static long handle_remove_overlay_nodes(const void *overlay_fdt,
{
rc = -EINVAL;
goto out;
-
}
rc = remove_nodes(entry);
@@ -552,9 +611,6 @@ static long handle_remove_overlay_nodes(const void *overlay_fdt,
xfree(entry->nodes_address);
- rangeset_destroy(entry->irq_ranges);
- rangeset_destroy(entry->iomem_ranges);
-
xfree(entry);
out:
@@ -840,6 +896,88 @@ static long handle_add_overlay_nodes(void *overlay_fdt,
return rc;
}
+static long handle_detach_overlay_nodes(struct domain *d,
+ const void *overlay_fdt,
+ uint32_t overlay_fdt_size)
+{
+ int rc;
+ unsigned int j;
+ struct overlay_track *entry;
+
+ rc = check_overlay_fdt(overlay_fdt, overlay_fdt_size);
+ if ( rc )
+ return rc;
+
+ spin_lock(&overlay_lock);
+
+ entry = find_track_entry_from_tracker(overlay_fdt, overlay_fdt_size);
+ if ( entry == NULL )
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ for ( j = 0; j < entry->num_nodes; j++ )
+ {
+ struct dt_device_node *overlay_node;
+
+ overlay_node = (struct dt_device_node *)entry->nodes_address[j];
+ if ( overlay_node == NULL )
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ write_lock(&dt_host_lock);
+ rc = remove_descendant_nodes_resources(overlay_node, d);
+ if ( rc )
+ {
+ write_unlock(&dt_host_lock);
+ goto out;
+ }
+
+ rc = remove_node_resources(overlay_node, d);
+ if ( rc )
+ {
+ write_unlock(&dt_host_lock);
+ goto out;
+ }
+ write_unlock(&dt_host_lock);
+
+ rc = remove_all_irqs(entry->irq_ranges, d);
+ if ( rc )
+ goto out;
+
+ rc = remove_all_iomems(entry->iomem_ranges, d);
+ if ( rc )
+ goto out;
+ }
+
+ /* Remove IRQ access. */
+ if ( entry->irq_ranges )
+ {
+ rc = rangeset_consume_ranges(entry->irq_ranges, irq_remove_cb, d);
+ if ( rc )
+ goto out;
+ }
+
+ /* Remove mmio access. */
+ if ( entry->iomem_ranges )
+ {
+ rc = rangeset_consume_ranges(entry->iomem_ranges, iomem_remove_cb, d);
+ if ( rc )
+ goto out;
+ }
+
+ rangeset_destroy(entry->irq_ranges);
+ rangeset_destroy(entry->iomem_ranges);
+
+ out:
+ spin_unlock(&overlay_lock);
+
+ return rc;
+}
+
static long handle_attach_overlay_nodes(struct domain *d,
const void *overlay_fdt,
uint32_t overlay_fdt_size)
@@ -956,7 +1094,8 @@ long dt_overlay_domctl(struct domain *d, struct xen_domctl_dt_overlay *op)
long ret;
void *overlay_fdt;
- if ( op->overlay_op != XEN_DOMCTL_DT_OVERLAY_ATTACH )
+ if ( op->overlay_op != XEN_DOMCTL_DT_OVERLAY_ATTACH &&
+ op->overlay_op != XEN_DOMCTL_DT_OVERLAY_DETACH )
return -EOPNOTSUPP;
if ( op->overlay_fdt_size == 0 || op->overlay_fdt_size > KB(500) )
@@ -979,7 +1118,9 @@ long dt_overlay_domctl(struct domain *d, struct xen_domctl_dt_overlay *op)
return -EFAULT;
}
- if ( op->overlay_op == XEN_DOMCTL_DT_OVERLAY_ATTACH )
+ if ( op->overlay_op == XEN_DOMCTL_DT_OVERLAY_DETACH )
+ ret = handle_detach_overlay_nodes(d, overlay_fdt, op->overlay_fdt_size);
+ else
ret = handle_attach_overlay_nodes(d, overlay_fdt, op->overlay_fdt_size);
xfree(overlay_fdt);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ac3c2a7c4c..e91f743fa4 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1195,7 +1195,8 @@ struct xen_domctl_dt_overlay {
XEN_GUEST_HANDLE_64(const_void) overlay_fdt; /* IN: overlay fdt. */
uint32_t overlay_fdt_size; /* IN: Overlay dtb size. */
#define XEN_DOMCTL_DT_OVERLAY_ATTACH 1
- uint8_t overlay_op; /* IN: Attach. */
+#define XEN_DOMCTL_DT_OVERLAY_DETACH 2
+ uint8_t overlay_op; /* IN: Attach/Detach. */
uint8_t pad[3]; /* IN: Must be zero. */
};
#endif
--
2.34.1Hi Henry,
On 23/05/2024 08:40, Henry Wang wrote:
> Similarly as the device attachment from DT overlay to domain, this
> commit implements the device detachment from domain. The DOMCTL
> XEN_DOMCTL_dt_overlay op is extended to have the operation
> XEN_DOMCTL_DT_OVERLAY_DETACH. The detachment of the device is
> implemented by unmapping the IRQ and IOMMU resources. Note that with
> these changes, the device de-registration from the IOMMU driver should
> only happen at the time when the DT overlay is removed from the Xen
> device tree.
>
> Signed-off-by: Henry Wang <xin.wang2@amd.com>
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> ---
> v4:
> - Split the original patch, only do device detachment from domain.
> ---
> xen/common/dt-overlay.c | 243 ++++++++++++++++++++++++++++--------
> xen/include/public/domctl.h | 3 +-
> 2 files changed, 194 insertions(+), 52 deletions(-)
>
> diff --git a/xen/common/dt-overlay.c b/xen/common/dt-overlay.c
> index 1087f9b502..693b6e4777 100644
> --- a/xen/common/dt-overlay.c
> +++ b/xen/common/dt-overlay.c
> @@ -392,24 +392,100 @@ find_track_entry_from_tracker(const void *overlay_fdt,
> return entry;
> }
>
> +static int remove_irq(unsigned long s, unsigned long e, void *data)
> +{
> + struct domain *d = data;
> + int rc = 0;
> +
> + /*
> + * IRQ should always have access unless there are duplication of
> + * of irqs in device tree. There are few cases of xen device tree
> + * where there are duplicate interrupts for the same node.
> + */
> + if (!irq_access_permitted(d, s))
Because of this check, it means that ...
> + return 0;
> + /*
> + * 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, s);
> + if ( rc )
> + {
> + printk(XENLOG_ERR "unable to revoke access for irq %ld\n", s);
> + return rc;
> + }
> +
> + rc = release_guest_irq(d, s);
... release_guest_irq() fails on the next retry it will pass. I don't
think this is what we want.
Instead, we probably want to re-order the call.
> + if ( rc )
> + {
> + printk(XENLOG_ERR "unable to release irq %ld\n", s);
> + return rc;
> + }
> +
> + return rc;
> +}
> +
> +static int remove_all_irqs(struct rangeset *irq_ranges, struct domain *d)
> +{
> + return rangeset_report_ranges(irq_ranges, 0, ~0UL, remove_irq, d);
> +}
> +
> +static int remove_iomem(unsigned long s, unsigned long e, void *data)
> +{
> + struct domain *d = data;
> + int rc = 0;
> + p2m_type_t t;
> + mfn_t mfn;
> +
> + mfn = p2m_lookup(d, _gfn(s), &t);
What are you trying to addres with this check? For instance, the fact
that the first MFN is mapped, doesn't guarantee the rest is.
> + if ( mfn_x(mfn) == 0 || mfn_x(mfn) == ~0UL )
I don't understand why we are checking for 0 here. In theory, it is
valid MFN. Also, the second part wants to be INVALID_MFN.
> + return -EINVAL;
> +
> + rc = iomem_deny_access(d, s, e);
iomem_deny_access() works on MFN but here you pass an MFN. Are you
assuming the GFN == MFN? How would that work for domains that are not
direct mapped?
> + if ( rc )
> + {
> + printk(XENLOG_ERR "Unable to remove %pd access to %#lx - %#lx\n",
> + d, s, e);
> + return rc;
> + }
> +
> + rc = unmap_mmio_regions(d, _gfn(s), e - s, _mfn(s));
> + if ( rc )
> + return rc;
> +
> + return rc;
> +}
> +
> +static int remove_all_iomems(struct rangeset *iomem_ranges, struct domain *d)
> +{
> + return rangeset_report_ranges(iomem_ranges, 0, ~0UL, remove_iomem, d);
> +}
> +
> /* Check if node itself can be removed and remove node from IOMMU. */
> -static int remove_node_resources(struct dt_device_node *device_node)
> +static int remove_node_resources(struct dt_device_node *device_node,
> + struct domain *d)
> {
> int rc = 0;
> unsigned int len;
> domid_t domid;
>
> - domid = dt_device_used_by(device_node);
> + if ( !d )
I looked at the code, I am a bit unsure how "d" can be NULL. Do you have
any pointer?
> + {
> + domid = dt_device_used_by(device_node);
>
> - dt_dprintk("Checking if node %s is used by any domain\n",
> - device_node->full_name);
> + dt_dprintk("Checking if node %s is used by any domain\n",
> + device_node->full_name);
>
> - /* Remove the node if only it's assigned to hardware domain or domain io. */
> - if ( domid != hardware_domain->domain_id && domid != DOMID_IO )
> - {
> - printk(XENLOG_ERR "Device %s is being used by domain %u. Removing nodes failed\n",
> - device_node->full_name, domid);
> - return -EINVAL;
> + /*
> + * We also check if device is assigned to DOMID_IO as when a domain
> + * is destroyed device is assigned to DOMID_IO.
> + */
> + if ( domid != DOMID_IO )
> + {
> + printk(XENLOG_ERR "Device %s is being assigned to %u. Device is assigned to %d\n",
> + device_node->full_name, DOMID_IO, domid);
> + return -EINVAL;
> + }
> }
>
> /* Check if iommu property exists. */
> @@ -417,9 +493,12 @@ static int remove_node_resources(struct dt_device_node *device_node)
> {
> if ( dt_device_is_protected(device_node) )
> {
> - rc = iommu_remove_dt_device(device_node);
> - if ( rc < 0 )
> - return rc;
> + if ( !list_empty(&device_node->domain_list) )
> + {
> + rc = iommu_deassign_dt_device(d, device_node);
> + if ( rc < 0 )
> + return rc;
> + }
> }
> }
>
> @@ -428,7 +507,8 @@ static int remove_node_resources(struct dt_device_node *device_node)
>
> /* Remove all descendants from IOMMU. */
> static int
> -remove_descendant_nodes_resources(const struct dt_device_node *device_node)
> +remove_descendant_nodes_resources(const struct dt_device_node *device_node,
> + struct domain *d)
> {
> int rc = 0;
> struct dt_device_node *child_node;
> @@ -438,12 +518,12 @@ remove_descendant_nodes_resources(const struct dt_device_node *device_node)
> {
> if ( child_node->child )
> {
> - rc = remove_descendant_nodes_resources(child_node);
> + rc = remove_descendant_nodes_resources(child_node, d);
> if ( rc )
> return rc;
> }
>
> - rc = remove_node_resources(child_node);
> + rc = remove_node_resources(child_node, d);
> if ( rc )
> return rc;
> }
> @@ -456,8 +536,7 @@ static int remove_nodes(const struct overlay_track *tracker)
> {
> int rc = 0;
> struct dt_device_node *overlay_node;
> - unsigned int j;
> - struct domain *d = hardware_domain;
> + unsigned int j, len;
>
> for ( j = 0; j < tracker->num_nodes; j++ )
> {
> @@ -467,18 +546,15 @@ static int remove_nodes(const struct overlay_track *tracker)
>
> write_lock(&dt_host_lock);
>
> - rc = remove_descendant_nodes_resources(overlay_node);
> - if ( rc )
> + /* Check if iommu property exists. */
> + if ( dt_get_property(overlay_node, "iommus", &len) )
Why do we need to check for the property in the "iommus" rather than ...
> {
> - write_unlock(&dt_host_lock);
> - return rc;
> - }
> -
> - rc = remove_node_resources(overlay_node);
> - if ( rc )
> - {
> - write_unlock(&dt_host_lock);
> - return rc;
> + if ( dt_device_is_protected(overlay_node) )
... relying on dt_device_is_protected()?
> + {
> + rc = iommu_remove_dt_device(overlay_node);
> + if ( rc < 0 )
> + return rc;
> + }
> }
>
> dt_dprintk("Removing node: %s\n", overlay_node->full_name);
> @@ -493,22 +569,6 @@ static int remove_nodes(const struct overlay_track *tracker)
> write_unlock(&dt_host_lock);
> }
>
> - /* Remove IRQ access. */
> - if ( tracker->irq_ranges )
> - {
> - rc = rangeset_consume_ranges(tracker->irq_ranges, irq_remove_cb, d);
> - if ( rc )
> - return rc;
> - }
> -
> - /* Remove mmio access. */
> - if ( tracker->iomem_ranges )
> - {
> - rc = rangeset_consume_ranges(tracker->iomem_ranges, iomem_remove_cb, d);
> - if ( rc )
> - return rc;
> - }
> -
> return rc;
> }
>
> @@ -534,7 +594,6 @@ static long handle_remove_overlay_nodes(const void *overlay_fdt,
> {
> rc = -EINVAL;
> goto out;
> -
Valid but unrelated change?
> }
>
> rc = remove_nodes(entry);
> @@ -552,9 +611,6 @@ static long handle_remove_overlay_nodes(const void *overlay_fdt,
>
> xfree(entry->nodes_address);
>
> - rangeset_destroy(entry->irq_ranges);
> - rangeset_destroy(entry->iomem_ranges);
> -
> xfree(entry);
>
> out:
> @@ -840,6 +896,88 @@ static long handle_add_overlay_nodes(void *overlay_fdt,
> return rc;
> }
>
> +static long handle_detach_overlay_nodes(struct domain *d,
> + const void *overlay_fdt,
> + uint32_t overlay_fdt_size)
> +{
> + int rc;
> + unsigned int j;
> + struct overlay_track *entry;
> +
> + rc = check_overlay_fdt(overlay_fdt, overlay_fdt_size);
> + if ( rc )
> + return rc;
> +
> + spin_lock(&overlay_lock);
> +
> + entry = find_track_entry_from_tracker(overlay_fdt, overlay_fdt_size);
> + if ( entry == NULL )
> + {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + for ( j = 0; j < entry->num_nodes; j++ )
> + {
> + struct dt_device_node *overlay_node;
> +
> + overlay_node = (struct dt_device_node *)entry->nodes_address[j];
Can you remind me why nodes_address is storing unsigned long rather
struct dt_device_node?
> + if ( overlay_node == NULL )
> + {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + write_lock(&dt_host_lock);
> + rc = remove_descendant_nodes_resources(overlay_node, d);
> + if ( rc )
> + {
> + write_unlock(&dt_host_lock);
> + goto out;
> + }
> +
> + rc = remove_node_resources(overlay_node, d);
> + if ( rc )
> + {
> + write_unlock(&dt_host_lock);
> + goto out;
> + }
> + write_unlock(&dt_host_lock);
> +
> + rc = remove_all_irqs(entry->irq_ranges, d);
I am really confused. This is called in the for loop but entry doesn't
seem to change. So why do we need to call it for every iteration? Also...
> + if ( rc )
> + goto out;
> +
> + rc = remove_all_iomems(entry->iomem_ranges, d);
> + if ( rc )
> + goto out;
> + }
> +
> + /* Remove IRQ access. */
> + if ( entry->irq_ranges )
> + {
> + rc = rangeset_consume_ranges(entry->irq_ranges, irq_remove_cb, d);
... remove_all_irqs() will revert the permission. So why do we need to
do it again?
> + if ( rc )
> + goto out;
> + }
> +
> + /* Remove mmio access. */
> + if ( entry->iomem_ranges )
> + {
> + rc = rangeset_consume_ranges(entry->iomem_ranges, iomem_remove_cb, d);
Same question here.
> + if ( rc )
> + goto out;
> + }
> +
> + rangeset_destroy(entry->irq_ranges);
> + rangeset_destroy(entry->iomem_ranges);
> +
> + out:
> + spin_unlock(&overlay_lock);
> +
> + return rc;
> +}
> +
> static long handle_attach_overlay_nodes(struct domain *d,
> const void *overlay_fdt,
> uint32_t overlay_fdt_size)
> @@ -956,7 +1094,8 @@ long dt_overlay_domctl(struct domain *d, struct xen_domctl_dt_overlay *op)
> long ret;
> void *overlay_fdt;
>
> - if ( op->overlay_op != XEN_DOMCTL_DT_OVERLAY_ATTACH )
> + if ( op->overlay_op != XEN_DOMCTL_DT_OVERLAY_ATTACH &&
> + op->overlay_op != XEN_DOMCTL_DT_OVERLAY_DETACH )
> return -EOPNOTSUPP;
>
> if ( op->overlay_fdt_size == 0 || op->overlay_fdt_size > KB(500) )
> @@ -979,7 +1118,9 @@ long dt_overlay_domctl(struct domain *d, struct xen_domctl_dt_overlay *op)
> return -EFAULT;
> }
>
> - if ( op->overlay_op == XEN_DOMCTL_DT_OVERLAY_ATTACH )
> + if ( op->overlay_op == XEN_DOMCTL_DT_OVERLAY_DETACH )
> + ret = handle_detach_overlay_nodes(d, overlay_fdt, op->overlay_fdt_size);
> + else
> ret = handle_attach_overlay_nodes(d, overlay_fdt, op->overlay_fdt_size);
>
> xfree(overlay_fdt);
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index ac3c2a7c4c..e91f743fa4 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1195,7 +1195,8 @@ struct xen_domctl_dt_overlay {
> XEN_GUEST_HANDLE_64(const_void) overlay_fdt; /* IN: overlay fdt. */
> uint32_t overlay_fdt_size; /* IN: Overlay dtb size. */
> #define XEN_DOMCTL_DT_OVERLAY_ATTACH 1
> - uint8_t overlay_op; /* IN: Attach. */
> +#define XEN_DOMCTL_DT_OVERLAY_DETACH 2
> + uint8_t overlay_op; /* IN: Attach/Detach. */
Changing the comment for every operation we add is not really scalable
:). If you want an explanation, can it be generic?
> uint8_t pad[3]; /* IN: Must be zero. */
> };
> #endif
Cheers,
--
Julien Grall
© 2016 - 2026 Red Hat, Inc.