diff mbox series

[RFC,v2,3/8] xen/arm: Export host device-tree to hypfs

Message ID e440e4f16a506ecc87078635dbb3fda2ebd45346.1644341635.git.oleksii_moisieiev@epam.com (mailing list archive)
State New, archived
Headers show
Series Introduce SCI-mediator feature | expand

Commit Message

Oleksii Moisieiev Feb. 8, 2022, 6 p.m. UTC
If enabled, host device-tree will be exported to hypfs and can be
accessed through /devicetree path.
Exported device-tree has the same format, as the device-tree
exported to the sysfs by the Linux kernel.
This is useful when XEN toolstack needs an access to the host device-tree.

Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---
 xen/arch/arm/Kconfig           |   8 +
 xen/arch/arm/Makefile          |   1 +
 xen/arch/arm/host_dtb_export.c | 307 +++++++++++++++++++++++++++++++++
 3 files changed, 316 insertions(+)
 create mode 100644 xen/arch/arm/host_dtb_export.c

Comments

Julien Grall Feb. 8, 2022, 6:26 p.m. UTC | #1
Hi Oleksii,

On 08/02/2022 18:00, Oleksii Moisieiev wrote:
> If enabled, host device-tree will be exported to hypfs and can be
> accessed through /devicetree path.
> Exported device-tree has the same format, as the device-tree
> exported to the sysfs by the Linux kernel.
> This is useful when XEN toolstack needs an access to the host device-tree.
> 
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> ---
>   xen/arch/arm/Kconfig           |   8 +
>   xen/arch/arm/Makefile          |   1 +
>   xen/arch/arm/host_dtb_export.c | 307 +++++++++++++++++++++++++++++++++

There is nothing specific in this file. So can this be moved in common/?

>   3 files changed, 316 insertions(+)
>   create mode 100644 xen/arch/arm/host_dtb_export.c
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index ecfa6822e4..895016b21e 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -33,6 +33,14 @@ config ACPI
>   	  Advanced Configuration and Power Interface (ACPI) support for Xen is
>   	  an alternative to device tree on ARM64.
>   
> +config HOST_DTB_EXPORT
> +	bool "Export host device tree to hypfs if enabled"
> +	depends on ARM && HYPFS && !ACPI

A Xen built with ACPI enabled will still be able to boot on a host using 
Device-Tree. So I don't think should depend on ACPI.

Also, I think this should depend on HAS_DEVICE_TREE rather than ARM.

> +	---help---
> +
> +	  Export host device-tree to hypfs so toolstack can have an access for the
> +	  host device tree from Dom0. If you unsure say N.
> +
>   config GICV3
>   	bool "GICv3 driver"
>   	depends on ARM_64 && !NEW_VGIC
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 07f634508e..0a41f68f8c 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -8,6 +8,7 @@ obj-y += platforms/
>   endif
>   obj-$(CONFIG_TEE) += tee/
>   obj-$(CONFIG_HAS_VPCI) += vpci.o
> +obj-$(CONFIG_HOST_DTB_EXPORT) += host_dtb_export.o
>   
>   obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
>   obj-y += bootfdt.init.o
> diff --git a/xen/arch/arm/host_dtb_export.c b/xen/arch/arm/host_dtb_export.c
> new file mode 100644
> index 0000000000..794395683c
> --- /dev/null
> +++ b/xen/arch/arm/host_dtb_export.c

This is mostly hypfs related. So CCing Juergen for his input on the code.

> @@ -0,0 +1,307 @@
> +/*
> + * xen/arch/arm/host_dtb_export.c
> + *
> + * Export host device-tree to the hypfs so toolstack can access
> + * host device-tree from Dom0
> + *
> + * Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> + * Copyright (C) 2021, EPAM Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + */
> +
> +#include <xen/device_tree.h>
> +#include <xen/err.h>
> +#include <xen/guest_access.h>
> +#include <xen/hypfs.h>
> +#include <xen/init.h>
> +
> +#define HOST_DT_DIR "devicetree"
> +
> +static int host_dt_dir_read(const struct hypfs_entry *entry,
> +                            XEN_GUEST_HANDLE_PARAM(void) uaddr);
> +static unsigned int host_dt_dir_getsize(const struct hypfs_entry *entry);
> +
> +static const struct hypfs_entry *host_dt_dir_enter(
> +    const struct hypfs_entry *entry);
> +static void host_dt_dir_exit(const struct hypfs_entry *entry);
> +
> +static struct hypfs_entry *host_dt_dir_findentry(
> +    const struct hypfs_entry_dir *dir, const char *name, unsigned int name_len);

This is new code. So can you please make sure to avoid forward 
declaration by re-ordering the code.


[...]

> +static char *get_root_from_path(const char *path, char *name)
> +{
> +    const char *nm = strchr(path, '/');
> +    if ( !nm )
> +        nm = path + strlen(path);
> +    else
> +    {
> +        if ( !*nm )
> +            nm--;
> +    }
> +
> +    return memcpy(name, path, nm - path);

What does guaranteee that name will be big enough for the new value?

> +}
> +
> +static int host_dt_dir_read(const struct hypfs_entry *entry,
> +                            XEN_GUEST_HANDLE_PARAM(void) uaddr)
> +{
> +    int ret = 0;
> +    struct dt_device_node *node;
> +    struct dt_device_node *child;

The hypfs should not modify the device-tree. So can this be const?

> +    const struct dt_property *prop;
> +    struct hypfs_dyndir_id *data;
> +
> +    data = hypfs_get_dyndata();
> +    if ( !data )
> +        return -EINVAL;

[...]

> +static struct hypfs_entry *host_dt_dir_findentry(
> +    const struct hypfs_entry_dir *dir, const char *name, unsigned int name_len)
> +{
> +    struct dt_device_node *node;
> +    char root_name[HYPFS_DYNDIR_ID_NAMELEN];
> +    struct dt_device_node *child;
> +    struct hypfs_dyndir_id *data;
> +    struct dt_property *prop;
> +
> +    data = hypfs_get_dyndata();
> +    if ( !data )
> +        return ERR_PTR(-EINVAL);
> +
> +    node = data->data;
> +    if ( !node )
> +        return ERR_PTR(-EINVAL);
> +
> +    memset(root_name, 0, sizeof(root_name));
> +    get_root_from_path(name, root_name);
> +
> +    for ( child = node->child; child != NULL; child = child->sibling )
> +    {
> +        if ( strcmp(get_name_from_path(child->full_name), root_name) == 0 )
> +            return hypfs_gen_dyndir_entry(&dt_dir.e,
> +                                  get_name_from_path(child->full_name), child);
> +    }
> +
> +    dt_for_each_property_node( node, prop )
> +    {
> +
> +        if ( dt_property_name_is_equal(prop, root_name) )
> +            return hypfs_gen_dyndir_entry(&dt_prop.e, prop->name, prop);
> +    }
> +
> +    return ERR_PTR(-ENOENT);

[...]

> +static HYPFS_DIR_INIT_FUNC(host_dt_dir, HOST_DT_DIR, &host_dt_dir_funcs);
> +
> +static int __init host_dtb_export_init(void)
> +{
> +    ASSERT(dt_host && (dt_host->sibling == NULL));

dt_host can be NULL when booting on ACPI platform. So I think this wants 
to be turned to a normal check and return directly.

Also could you explain why you need to check dt_host->sibling?

Cheers,
Oleksii Moisieiev Feb. 9, 2022, 10:20 a.m. UTC | #2
Hi Julien,

On Tue, Feb 08, 2022 at 06:26:54PM +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 08/02/2022 18:00, Oleksii Moisieiev wrote:
> > If enabled, host device-tree will be exported to hypfs and can be
> > accessed through /devicetree path.
> > Exported device-tree has the same format, as the device-tree
> > exported to the sysfs by the Linux kernel.
> > This is useful when XEN toolstack needs an access to the host device-tree.
> > 
> > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> > ---
> >   xen/arch/arm/Kconfig           |   8 +
> >   xen/arch/arm/Makefile          |   1 +
> >   xen/arch/arm/host_dtb_export.c | 307 +++++++++++++++++++++++++++++++++
> 
> There is nothing specific in this file. So can this be moved in common/?

You're right. I will move it to common.

> 
> >   3 files changed, 316 insertions(+)
> >   create mode 100644 xen/arch/arm/host_dtb_export.c
> > 
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index ecfa6822e4..895016b21e 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -33,6 +33,14 @@ config ACPI
> >   	  Advanced Configuration and Power Interface (ACPI) support for Xen is
> >   	  an alternative to device tree on ARM64.
> > +config HOST_DTB_EXPORT
> > +	bool "Export host device tree to hypfs if enabled"
> > +	depends on ARM && HYPFS && !ACPI
> 
> A Xen built with ACPI enabled will still be able to boot on a host using
> Device-Tree. So I don't think should depend on ACPI.
> 
> Also, I think this should depend on HAS_DEVICE_TREE rather than ARM.

I agree. Thank you.

> 
> > +	---help---
> > +
> > +	  Export host device-tree to hypfs so toolstack can have an access for the
> > +	  host device tree from Dom0. If you unsure say N.
> > +
> >   config GICV3
> >   	bool "GICv3 driver"
> >   	depends on ARM_64 && !NEW_VGIC
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > index 07f634508e..0a41f68f8c 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -8,6 +8,7 @@ obj-y += platforms/
> >   endif
> >   obj-$(CONFIG_TEE) += tee/
> >   obj-$(CONFIG_HAS_VPCI) += vpci.o
> > +obj-$(CONFIG_HOST_DTB_EXPORT) += host_dtb_export.o
> >   obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
> >   obj-y += bootfdt.init.o
> > diff --git a/xen/arch/arm/host_dtb_export.c b/xen/arch/arm/host_dtb_export.c
> > new file mode 100644
> > index 0000000000..794395683c
> > --- /dev/null
> > +++ b/xen/arch/arm/host_dtb_export.c
> 
> This is mostly hypfs related. So CCing Juergen for his input on the code.

Thank you.

> 
> > @@ -0,0 +1,307 @@
> > +/*
> > + * xen/arch/arm/host_dtb_export.c
> > + *
> > + * Export host device-tree to the hypfs so toolstack can access
> > + * host device-tree from Dom0
> > + *
> > + * Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> > + * Copyright (C) 2021, EPAM Systems.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * 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.
> > + */
> > +
> > +#include <xen/device_tree.h>
> > +#include <xen/err.h>
> > +#include <xen/guest_access.h>
> > +#include <xen/hypfs.h>
> > +#include <xen/init.h>
> > +
> > +#define HOST_DT_DIR "devicetree"
> > +
> > +static int host_dt_dir_read(const struct hypfs_entry *entry,
> > +                            XEN_GUEST_HANDLE_PARAM(void) uaddr);
> > +static unsigned int host_dt_dir_getsize(const struct hypfs_entry *entry);
> > +
> > +static const struct hypfs_entry *host_dt_dir_enter(
> > +    const struct hypfs_entry *entry);
> > +static void host_dt_dir_exit(const struct hypfs_entry *entry);
> > +
> > +static struct hypfs_entry *host_dt_dir_findentry(
> > +    const struct hypfs_entry_dir *dir, const char *name, unsigned int name_len);
> 
> This is new code. So can you please make sure to avoid forward declaration
> by re-ordering the code.
> 

I can't avoid forward declaration here because all those functions
should be passed as callbacks for node template dt_dir. And dt_dir is
used in read and findentry functions.

> 
> [...]
> 
> > +static char *get_root_from_path(const char *path, char *name)
> > +{
> > +    const char *nm = strchr(path, '/');
> > +    if ( !nm )
> > +        nm = path + strlen(path);
> > +    else
> > +    {
> > +        if ( !*nm )
> > +            nm--;
> > +    }
> > +
> > +    return memcpy(name, path, nm - path);
> 
> What does guaranteee that name will be big enough for the new value?

I will refactor that.

> 
> > +}
> > +
> > +static int host_dt_dir_read(const struct hypfs_entry *entry,
> > +                            XEN_GUEST_HANDLE_PARAM(void) uaddr)
> > +{
> > +    int ret = 0;
> > +    struct dt_device_node *node;
> > +    struct dt_device_node *child;
> 
> The hypfs should not modify the device-tree. So can this be const?

That's a good point.
Unfortunatelly child can't be const because it is going to be passed to
data->data pointer, but node can be const I think. In any case I will go
through the file and see where const for the device_node can be set.

> 
> > +    const struct dt_property *prop;
> > +    struct hypfs_dyndir_id *data;
> > +
> > +    data = hypfs_get_dyndata();
> > +    if ( !data )
> > +        return -EINVAL;
> 
> [...]
> 
> > +static struct hypfs_entry *host_dt_dir_findentry(
> > +    const struct hypfs_entry_dir *dir, const char *name, unsigned int name_len)
> > +{
> > +    struct dt_device_node *node;
> > +    char root_name[HYPFS_DYNDIR_ID_NAMELEN];
> > +    struct dt_device_node *child;
> > +    struct hypfs_dyndir_id *data;
> > +    struct dt_property *prop;
> > +
> > +    data = hypfs_get_dyndata();
> > +    if ( !data )
> > +        return ERR_PTR(-EINVAL);
> > +
> > +    node = data->data;
> > +    if ( !node )
> > +        return ERR_PTR(-EINVAL);
> > +
> > +    memset(root_name, 0, sizeof(root_name));
> > +    get_root_from_path(name, root_name);
> > +
> > +    for ( child = node->child; child != NULL; child = child->sibling )
> > +    {
> > +        if ( strcmp(get_name_from_path(child->full_name), root_name) == 0 )
> > +            return hypfs_gen_dyndir_entry(&dt_dir.e,
> > +                                  get_name_from_path(child->full_name), child);
> > +    }
> > +
> > +    dt_for_each_property_node( node, prop )
> > +    {
> > +
> > +        if ( dt_property_name_is_equal(prop, root_name) )
> > +            return hypfs_gen_dyndir_entry(&dt_prop.e, prop->name, prop);
> > +    }
> > +
> > +    return ERR_PTR(-ENOENT);
> 
> [...]
> 
> > +static HYPFS_DIR_INIT_FUNC(host_dt_dir, HOST_DT_DIR, &host_dt_dir_funcs);
> > +
> > +static int __init host_dtb_export_init(void)
> > +{
> > +    ASSERT(dt_host && (dt_host->sibling == NULL));
> 
> dt_host can be NULL when booting on ACPI platform. So I think this wants to
> be turned to a normal check and return directly.
> 

I will replace if with
if ( !acpi_disabled )
    return -ENODEV;

> Also could you explain why you need to check dt_host->sibling?
> 

This is my way to check if dt_host points to the top of the device-tree.
In any case I will replace it with !acpi_disabled as I mentioned
earlier.

Best regards,
Oleksii
Julien Grall Feb. 9, 2022, 12:17 p.m. UTC | #3
On 09/02/2022 10:20, Oleksii Moisieiev wrote:
> Hi Julien,

Hi,

> 
> On Tue, Feb 08, 2022 at 06:26:54PM +0000, Julien Grall wrote:
>> Hi Oleksii,
>>
>> On 08/02/2022 18:00, Oleksii Moisieiev wrote:
>>> If enabled, host device-tree will be exported to hypfs and can be
>>> accessed through /devicetree path.
>>> Exported device-tree has the same format, as the device-tree
>>> exported to the sysfs by the Linux kernel.
>>> This is useful when XEN toolstack needs an access to the host device-tree.
>>>
>>> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
>>> ---
>>>    xen/arch/arm/Kconfig           |   8 +
>>>    xen/arch/arm/Makefile          |   1 +
>>>    xen/arch/arm/host_dtb_export.c | 307 +++++++++++++++++++++++++++++++++
>>
>> There is nothing specific in this file. So can this be moved in common/?
> 
> You're right. I will move it to common.
> 
>>
>>>    3 files changed, 316 insertions(+)
>>>    create mode 100644 xen/arch/arm/host_dtb_export.c
>>>
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index ecfa6822e4..895016b21e 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -33,6 +33,14 @@ config ACPI
>>>    	  Advanced Configuration and Power Interface (ACPI) support for Xen is
>>>    	  an alternative to device tree on ARM64.
>>> +config HOST_DTB_EXPORT
>>> +	bool "Export host device tree to hypfs if enabled"
>>> +	depends on ARM && HYPFS && !ACPI
>>
>> A Xen built with ACPI enabled will still be able to boot on a host using
>> Device-Tree. So I don't think should depend on ACPI.
>>
>> Also, I think this should depend on HAS_DEVICE_TREE rather than ARM.
> 
> I agree. Thank you.
> 
>>
>>> +	---help---
>>> +
>>> +	  Export host device-tree to hypfs so toolstack can have an access for the
>>> +	  host device tree from Dom0. If you unsure say N.
>>> +
>>>    config GICV3
>>>    	bool "GICv3 driver"
>>>    	depends on ARM_64 && !NEW_VGIC
>>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>>> index 07f634508e..0a41f68f8c 100644
>>> --- a/xen/arch/arm/Makefile
>>> +++ b/xen/arch/arm/Makefile
>>> @@ -8,6 +8,7 @@ obj-y += platforms/
>>>    endif
>>>    obj-$(CONFIG_TEE) += tee/
>>>    obj-$(CONFIG_HAS_VPCI) += vpci.o
>>> +obj-$(CONFIG_HOST_DTB_EXPORT) += host_dtb_export.o
>>>    obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
>>>    obj-y += bootfdt.init.o
>>> diff --git a/xen/arch/arm/host_dtb_export.c b/xen/arch/arm/host_dtb_export.c
>>> new file mode 100644
>>> index 0000000000..794395683c
>>> --- /dev/null
>>> +++ b/xen/arch/arm/host_dtb_export.c
>>
>> This is mostly hypfs related. So CCing Juergen for his input on the code.
> 
> Thank you.
> 
>>
>>> @@ -0,0 +1,307 @@
>>> +/*
>>> + * xen/arch/arm/host_dtb_export.c
>>> + *
>>> + * Export host device-tree to the hypfs so toolstack can access
>>> + * host device-tree from Dom0
>>> + *
>>> + * Oleksii Moisieiev <oleksii_moisieiev@epam.com>
>>> + * Copyright (C) 2021, EPAM Systems.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#include <xen/device_tree.h>
>>> +#include <xen/err.h>
>>> +#include <xen/guest_access.h>
>>> +#include <xen/hypfs.h>
>>> +#include <xen/init.h>
>>> +
>>> +#define HOST_DT_DIR "devicetree"
>>> +
>>> +static int host_dt_dir_read(const struct hypfs_entry *entry,
>>> +                            XEN_GUEST_HANDLE_PARAM(void) uaddr);
>>> +static unsigned int host_dt_dir_getsize(const struct hypfs_entry *entry);
>>> +
>>> +static const struct hypfs_entry *host_dt_dir_enter(
>>> +    const struct hypfs_entry *entry);
>>> +static void host_dt_dir_exit(const struct hypfs_entry *entry);
>>> +
>>> +static struct hypfs_entry *host_dt_dir_findentry(
>>> +    const struct hypfs_entry_dir *dir, const char *name, unsigned int name_len);
>>
>> This is new code. So can you please make sure to avoid forward declaration
>> by re-ordering the code.
>>
> 
> I can't avoid forward declaration here because all those functions
> should be passed as callbacks for node template dt_dir. And dt_dir is
> used in read and findentry functions.

You can avoid most of those forward declarations if you define the 
static variable now but fill them up after (see [1]). I don't think we 
can avoid the static variable forward declaration without reworking the API.

BTW, I could not fully apply the series on the staging tree:

Applying: xen/hypfs: support fo nested dynamic hypfs nodes
Applying: libs: libxenhypfs - handle blob properties
Applying: xen/arm: Export host device-tree to hypfs
Applying: xen/arm: add generic SCI mediator framework
error: patch failed: MAINTAINERS:512
error: MAINTAINERS: patch does not apply
error: patch failed: xen/arch/arm/domain_build.c:1894
error: xen/arch/arm/domain_build.c: patch does not apply
error: xen/include/asm-arm/domain.h: does not exist in index
Patch failed at 0004 xen/arm: add generic SCI mediator framework
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

 From the errors, it sounds like your baseline is from a couple of 
months ago. Please make sure to send your series based on the latest 
staging (at the time you send it).

>>> +static int host_dt_dir_read(const struct hypfs_entry *entry,
>>> +                            XEN_GUEST_HANDLE_PARAM(void) uaddr)
>>> +{
>>> +    int ret = 0;
>>> +    struct dt_device_node *node;
>>> +    struct dt_device_node *child;
>>
>> The hypfs should not modify the device-tree. So can this be const?
> 
> That's a good point.
> Unfortunatelly child can't be const because it is going to be passed to
> data->data pointer, but node can be const I think. In any case I will go
> through the file and see where const for the device_node can be set.

Can you explain why that data->data is not const?
>>> +static HYPFS_DIR_INIT_FUNC(host_dt_dir, HOST_DT_DIR, &host_dt_dir_funcs);
>>> +
>>> +static int __init host_dtb_export_init(void)
>>> +{
>>> +    ASSERT(dt_host && (dt_host->sibling == NULL));
>>
>> dt_host can be NULL when booting on ACPI platform. So I think this wants to
>> be turned to a normal check and return directly.
>>
> 
> I will replace if with
> if ( !acpi_disabled )
>      return -ENODEV;
> 
>> Also could you explain why you need to check dt_host->sibling?
>>
> 
> This is my way to check if dt_host points to the top of the device-tree.
> In any case I will replace it with !acpi_disabled as I mentioned
> earlier.

dt_host will always points to the root of the host device-tree. I don't 
think it is the job of hypfs to enforce it unless you expect the code to 
be buggy if this happens. But then I would argue the code should be 
hardened.

Cheers,

[1]

diff --git a/xen/arch/arm/host_dtb_export.c b/xen/arch/arm/host_dtb_export.c
index 794395683cd1..5f242b2cb683 100644
--- a/xen/arch/arm/host_dtb_export.c
+++ b/xen/arch/arm/host_dtb_export.c
@@ -26,39 +26,9 @@

  #define HOST_DT_DIR "devicetree"

-static int host_dt_dir_read(const struct hypfs_entry *entry,
-                            XEN_GUEST_HANDLE_PARAM(void) uaddr);
-static unsigned int host_dt_dir_getsize(const struct hypfs_entry *entry);
-
-static const struct hypfs_entry *host_dt_dir_enter(
-    const struct hypfs_entry *entry);
-static void host_dt_dir_exit(const struct hypfs_entry *entry);
-
-static struct hypfs_entry *host_dt_dir_findentry(
-    const struct hypfs_entry_dir *dir, const char *name, unsigned int 
name_len);
-
-static const struct hypfs_funcs host_dt_dir_funcs = {
-    .enter = host_dt_dir_enter,
-    .exit = host_dt_dir_exit,
-    .read = host_dt_dir_read,
-    .write = hypfs_write_deny,
-    .getsize = host_dt_dir_getsize,
-    .findentry = host_dt_dir_findentry,
-};
-
-static int host_dt_prop_read(const struct hypfs_entry *entry,
-                    XEN_GUEST_HANDLE_PARAM(void) uaddr);
-
-static unsigned int host_dt_prop_getsize(const struct hypfs_entry *entry);
-
-const struct hypfs_funcs host_dt_prop_ro_funcs = {
-    .enter = host_dt_dir_enter,
-    .exit = host_dt_dir_exit,
-    .read = host_dt_prop_read,
-    .write = hypfs_write_deny,
-    .getsize = host_dt_prop_getsize,
-    .findentry = hypfs_leaf_findentry,
-};
+/* Forward declare it */
+static const struct hypfs_funcs host_dt_dir_funcs;
+static const struct hypfs_funcs host_dt_prop_ro_funcs;

  static HYPFS_DIR_INIT_FUNC(dt_dir, "node_template", &host_dt_dir_funcs);

@@ -260,6 +230,15 @@ static struct hypfs_entry *host_dt_dir_findentry(
      return ERR_PTR(-ENOENT);
  };

+static const struct hypfs_funcs host_dt_dir_funcs = {
+    .enter = host_dt_dir_enter,
+    .exit = host_dt_dir_exit,
+    .read = host_dt_dir_read,
+    .write = hypfs_write_deny,
+    .getsize = host_dt_dir_getsize,
+    .findentry = host_dt_dir_findentry,
+};
+
  static int host_dt_prop_read(const struct hypfs_entry *entry,
                      XEN_GUEST_HANDLE_PARAM(void) uaddr)
  {
@@ -293,6 +272,15 @@ static unsigned int host_dt_prop_getsize(const 
struct hypfs_entry *entry)
      return prop->length;
  }

+static const struct hypfs_funcs host_dt_prop_ro_funcs = {
+    .enter = host_dt_dir_enter,
+    .exit = host_dt_dir_exit,
+    .read = host_dt_prop_read,
+    .write = hypfs_write_deny,
+    .getsize = host_dt_prop_getsize,
+    .findentry = hypfs_leaf_findentry,
+};
+
  static HYPFS_DIR_INIT_FUNC(host_dt_dir, HOST_DT_DIR, &host_dt_dir_funcs);

  static int __init host_dtb_export_init(void)
Jürgen Groß Feb. 9, 2022, 2:03 p.m. UTC | #4
On 08.02.22 19:26, Julien Grall wrote:
> Hi Oleksii,
> 
> On 08/02/2022 18:00, Oleksii Moisieiev wrote:
>> If enabled, host device-tree will be exported to hypfs and can be
>> accessed through /devicetree path.
>> Exported device-tree has the same format, as the device-tree
>> exported to the sysfs by the Linux kernel.
>> This is useful when XEN toolstack needs an access to the host 
>> device-tree.
>>
>> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
>> ---
>>   xen/arch/arm/Kconfig           |   8 +
>>   xen/arch/arm/Makefile          |   1 +
>>   xen/arch/arm/host_dtb_export.c | 307 +++++++++++++++++++++++++++++++++

The related doc changes in docs/misc/hypfs-paths.pandoc are missing.

Without those its rather hard to verify the code is correct.


Juergen
Oleksii Moisieiev Feb. 9, 2022, 2:17 p.m. UTC | #5
Hi Julien,

On Wed, Feb 09, 2022 at 12:17:17PM +0000, Julien Grall wrote:
> 
> 
> On 09/02/2022 10:20, Oleksii Moisieiev wrote:
> > Hi Julien,
> 
> Hi,
> 
> > 
> > On Tue, Feb 08, 2022 at 06:26:54PM +0000, Julien Grall wrote:
> > > Hi Oleksii,
> > > 
> > > On 08/02/2022 18:00, Oleksii Moisieiev wrote:
> > > > If enabled, host device-tree will be exported to hypfs and can be
> > > > accessed through /devicetree path.
> > > > Exported device-tree has the same format, as the device-tree
> > > > exported to the sysfs by the Linux kernel.
> > > > This is useful when XEN toolstack needs an access to the host device-tree.
> > > > 
> > > > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> > > > ---
> > > >    xen/arch/arm/Kconfig           |   8 +
> > > >    xen/arch/arm/Makefile          |   1 +
> > > >    xen/arch/arm/host_dtb_export.c | 307 +++++++++++++++++++++++++++++++++
> > > 
> > > There is nothing specific in this file. So can this be moved in common/?
> > 
> > You're right. I will move it to common.
> > 
> > > 
> > > >    3 files changed, 316 insertions(+)
> > > >    create mode 100644 xen/arch/arm/host_dtb_export.c
> > > > 
> > > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > > index ecfa6822e4..895016b21e 100644
> > > > --- a/xen/arch/arm/Kconfig
> > > > +++ b/xen/arch/arm/Kconfig
> > > > @@ -33,6 +33,14 @@ config ACPI
> > > >    	  Advanced Configuration and Power Interface (ACPI) support for Xen is
> > > >    	  an alternative to device tree on ARM64.
> > > > +config HOST_DTB_EXPORT
> > > > +	bool "Export host device tree to hypfs if enabled"
> > > > +	depends on ARM && HYPFS && !ACPI
> > > 
> > > A Xen built with ACPI enabled will still be able to boot on a host using
> > > Device-Tree. So I don't think should depend on ACPI.
> > > 
> > > Also, I think this should depend on HAS_DEVICE_TREE rather than ARM.
> > 
> > I agree. Thank you.
> > 
> > > 
> > > > +	---help---
> > > > +
> > > > +	  Export host device-tree to hypfs so toolstack can have an access for the
> > > > +	  host device tree from Dom0. If you unsure say N.
> > > > +
> > > >    config GICV3
> > > >    	bool "GICv3 driver"
> > > >    	depends on ARM_64 && !NEW_VGIC
> > > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > > > index 07f634508e..0a41f68f8c 100644
> > > > --- a/xen/arch/arm/Makefile
> > > > +++ b/xen/arch/arm/Makefile
> > > > @@ -8,6 +8,7 @@ obj-y += platforms/
> > > >    endif
> > > >    obj-$(CONFIG_TEE) += tee/
> > > >    obj-$(CONFIG_HAS_VPCI) += vpci.o
> > > > +obj-$(CONFIG_HOST_DTB_EXPORT) += host_dtb_export.o
> > > >    obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
> > > >    obj-y += bootfdt.init.o
> > > > diff --git a/xen/arch/arm/host_dtb_export.c b/xen/arch/arm/host_dtb_export.c
> > > > new file mode 100644
> > > > index 0000000000..794395683c
> > > > --- /dev/null
> > > > +++ b/xen/arch/arm/host_dtb_export.c
> > > 
> > > This is mostly hypfs related. So CCing Juergen for his input on the code.
> > 
> > Thank you.
> > 
> > > 
> > > > @@ -0,0 +1,307 @@
> > > > +/*
> > > > + * xen/arch/arm/host_dtb_export.c
> > > > + *
> > > > + * Export host device-tree to the hypfs so toolstack can access
> > > > + * host device-tree from Dom0
> > > > + *
> > > > + * Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> > > > + * Copyright (C) 2021, EPAM Systems.
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License as published by
> > > > + * the Free Software Foundation; either version 2 of the License, or
> > > > + * (at your option) any later version.
> > > > + *
> > > > + * 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.
> > > > + */
> > > > +
> > > > +#include <xen/device_tree.h>
> > > > +#include <xen/err.h>
> > > > +#include <xen/guest_access.h>
> > > > +#include <xen/hypfs.h>
> > > > +#include <xen/init.h>
> > > > +
> > > > +#define HOST_DT_DIR "devicetree"
> > > > +
> > > > +static int host_dt_dir_read(const struct hypfs_entry *entry,
> > > > +                            XEN_GUEST_HANDLE_PARAM(void) uaddr);
> > > > +static unsigned int host_dt_dir_getsize(const struct hypfs_entry *entry);
> > > > +
> > > > +static const struct hypfs_entry *host_dt_dir_enter(
> > > > +    const struct hypfs_entry *entry);
> > > > +static void host_dt_dir_exit(const struct hypfs_entry *entry);
> > > > +
> > > > +static struct hypfs_entry *host_dt_dir_findentry(
> > > > +    const struct hypfs_entry_dir *dir, const char *name, unsigned int name_len);
> > > 
> > > This is new code. So can you please make sure to avoid forward declaration
> > > by re-ordering the code.
> > > 
> > 
> > I can't avoid forward declaration here because all those functions
> > should be passed as callbacks for node template dt_dir. And dt_dir is
> > used in read and findentry functions.
> 
> You can avoid most of those forward declarations if you define the static
> variable now but fill them up after (see [1]). I don't think we can avoid
> the static variable forward declaration without reworking the API.
> 
> BTW, I could not fully apply the series on the staging tree:
> 
> Applying: xen/hypfs: support fo nested dynamic hypfs nodes
> Applying: libs: libxenhypfs - handle blob properties
> Applying: xen/arm: Export host device-tree to hypfs
> Applying: xen/arm: add generic SCI mediator framework
> error: patch failed: MAINTAINERS:512
> error: MAINTAINERS: patch does not apply
> error: patch failed: xen/arch/arm/domain_build.c:1894
> error: xen/arch/arm/domain_build.c: patch does not apply
> error: xen/include/asm-arm/domain.h: does not exist in index
> Patch failed at 0004 xen/arm: add generic SCI mediator framework
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> From the errors, it sounds like your baseline is from a couple of months
> ago. Please make sure to send your series based on the latest staging (at
> the time you send it).
> 

I'm sorry for that. I will fix all comments, mentioned above, make a
rebase and post v3 shortly.

> > > > +static int host_dt_dir_read(const struct hypfs_entry *entry,
> > > > +                            XEN_GUEST_HANDLE_PARAM(void) uaddr)
> > > > +{
> > > > +    int ret = 0;
> > > > +    struct dt_device_node *node;
> > > > +    struct dt_device_node *child;
> > > 
> > > The hypfs should not modify the device-tree. So can this be const?
> > 
> > That's a good point.
> > Unfortunatelly child can't be const because it is going to be passed to
> > data->data pointer, but node can be const I think. In any case I will go
> > through the file and see where const for the device_node can be set.
> 
> Can you explain why that data->data is not const?

I reused struct hypfs_dyndir_id, added by @Juergen Gross in
4f1e5ed49c2292d3dd18160b7e728b1aa9453206 hope he will help to answer
this question.

> > > > +static HYPFS_DIR_INIT_FUNC(host_dt_dir, HOST_DT_DIR, &host_dt_dir_funcs);
> > > > +
> > > > +static int __init host_dtb_export_init(void)
> > > > +{
> > > > +    ASSERT(dt_host && (dt_host->sibling == NULL));
> > > 
> > > dt_host can be NULL when booting on ACPI platform. So I think this wants to
> > > be turned to a normal check and return directly.
> > > 
> > 
> > I will replace if with
> > if ( !acpi_disabled )
> >      return -ENODEV;
> > 
> > > Also could you explain why you need to check dt_host->sibling?
> > > 
> > 
> > This is my way to check if dt_host points to the top of the device-tree.
> > In any case I will replace it with !acpi_disabled as I mentioned
> > earlier.
> 
> dt_host will always points to the root of the host device-tree. I don't
> think it is the job of hypfs to enforce it unless you expect the code to be
> buggy if this happens. But then I would argue the code should be hardened.
> 

I will refactor this check.

Best regards,
Oleksii.
Oleksii Moisieiev Feb. 9, 2022, 6:51 p.m. UTC | #6
On Wed, Feb 09, 2022 at 12:17:17PM +0000, Julien Grall wrote:
> > > > +static HYPFS_DIR_INIT_FUNC(host_dt_dir, HOST_DT_DIR, &host_dt_dir_funcs);
> > > > +
> > > > +static int __init host_dtb_export_init(void)
> > > > +{
> > > > +    ASSERT(dt_host && (dt_host->sibling == NULL));
> > > 
> > > dt_host can be NULL when booting on ACPI platform. So I think this wants to
> > > be turned to a normal check and return directly.
> > > 
> > 
> > I will replace if with
> > if ( !acpi_disabled )
> >      return -ENODEV;
> > 
> > > Also could you explain why you need to check dt_host->sibling?
> > > 
> > 
> > This is my way to check if dt_host points to the top of the device-tree.
> > In any case I will replace it with !acpi_disabled as I mentioned
> > earlier.
> 
> dt_host will always points to the root of the host device-tree. I don't
> think it is the job of hypfs to enforce it unless you expect the code to be
> buggy if this happens. But then I would argue the code should be hardened.
> 

Hi Julien,

Unfortunatelly I can't use acpi_disabled in host_dtb_export_init because
I've already moved host_dtb_export.c to the common folder.

As for the host->sibling - I took the whole assert:
ASSERT(dt_host && (dt_host->sibling == NULL));
from the prepare_dtb_hwdom function. And this assertion was added by the
commit b8f1c5e7039efbe1103ed3fe4caedf8c34affe13 authored by you.

What do you think if I omit dt_host->sibling check and make it:

if ( !dt_host )
    return -ENODEV;

Best regards,
Olkesii.
Julien Grall Feb. 9, 2022, 7:34 p.m. UTC | #7
Hi,

On 09/02/2022 18:51, Oleksii Moisieiev wrote:
> On Wed, Feb 09, 2022 at 12:17:17PM +0000, Julien Grall wrote:
>>>>> +static HYPFS_DIR_INIT_FUNC(host_dt_dir, HOST_DT_DIR, &host_dt_dir_funcs);
>>>>> +
>>>>> +static int __init host_dtb_export_init(void)
>>>>> +{
>>>>> +    ASSERT(dt_host && (dt_host->sibling == NULL));
>>>>
>>>> dt_host can be NULL when booting on ACPI platform. So I think this wants to
>>>> be turned to a normal check and return directly.
>>>>
>>>
>>> I will replace if with
>>> if ( !acpi_disabled )
>>>       return -ENODEV;
>>>
>>>> Also could you explain why you need to check dt_host->sibling?
>>>>
>>>
>>> This is my way to check if dt_host points to the top of the device-tree.
>>> In any case I will replace it with !acpi_disabled as I mentioned
>>> earlier.
>>
>> dt_host will always points to the root of the host device-tree. I don't
>> think it is the job of hypfs to enforce it unless you expect the code to be
>> buggy if this happens. But then I would argue the code should be hardened.
>>
> 
> Hi Julien,
> 
> Unfortunatelly I can't use acpi_disabled in host_dtb_export_init because
> I've already moved host_dtb_export.c to the common folder.

I am sorry, but I don't understand why moving the code to common code 
prevents you to use !acpi_disabled. Can you clarify?

> 
> As for the host->sibling - I took the whole assert:
> ASSERT(dt_host && (dt_host->sibling == NULL));
> from the prepare_dtb_hwdom function. And this assertion was added by the
> commit b8f1c5e7039efbe1103ed3fe4caedf8c34affe13 authored by you.

I am not sure what's your point... Yes I wrote the same ASSERT() 9 years 
time. But people view evolves over the time.

There are some code I wished I had written differently (How about you? 
;)). However, I don't have the time to rewrite everything I ever wrote. 
That said, I can at least make sure they are not spread.

> 
> What do you think if I omit dt_host->sibling check and make it:
> 
> if ( !dt_host )
>      return -ENODEV;

We used to set dt_host even when booting with ACPI but that shouldn't be 
the case anymore. So I think this check should be fine.

Cheers,
Oleksii Moisieiev Feb. 10, 2022, 9:38 a.m. UTC | #8
On Wed, Feb 09, 2022 at 07:34:57PM +0000, Julien Grall wrote:
> Hi,
> 
> On 09/02/2022 18:51, Oleksii Moisieiev wrote:
> > On Wed, Feb 09, 2022 at 12:17:17PM +0000, Julien Grall wrote:
> > > > > > +static HYPFS_DIR_INIT_FUNC(host_dt_dir, HOST_DT_DIR, &host_dt_dir_funcs);
> > > > > > +
> > > > > > +static int __init host_dtb_export_init(void)
> > > > > > +{
> > > > > > +    ASSERT(dt_host && (dt_host->sibling == NULL));
> > > > > 
> > > > > dt_host can be NULL when booting on ACPI platform. So I think this wants to
> > > > > be turned to a normal check and return directly.
> > > > > 
> > > > 
> > > > I will replace if with
> > > > if ( !acpi_disabled )
> > > >       return -ENODEV;
> > > > 
> > > > > Also could you explain why you need to check dt_host->sibling?
> > > > > 
> > > > 
> > > > This is my way to check if dt_host points to the top of the device-tree.
> > > > In any case I will replace it with !acpi_disabled as I mentioned
> > > > earlier.
> > > 
> > > dt_host will always points to the root of the host device-tree. I don't
> > > think it is the job of hypfs to enforce it unless you expect the code to be
> > > buggy if this happens. But then I would argue the code should be hardened.
> > > 
> > 
> > Hi Julien,
> > 
> > Unfortunatelly I can't use acpi_disabled in host_dtb_export_init because
> > I've already moved host_dtb_export.c to the common folder.
> 
> I am sorry, but I don't understand why moving the code to common code
> prevents you to use !acpi_disabled. Can you clarify?
> 
Sorry, my bad. I thought that acpi_disabled is defined only for arm. Now
I've rechecked and see I was wrong.

> > 
> > As for the host->sibling - I took the whole assert:
> > ASSERT(dt_host && (dt_host->sibling == NULL));
> > from the prepare_dtb_hwdom function. And this assertion was added by the
> > commit b8f1c5e7039efbe1103ed3fe4caedf8c34affe13 authored by you.
> 
> I am not sure what's your point... Yes I wrote the same ASSERT() 9 years
> time. But people view evolves over the time.
> 
> There are some code I wished I had written differently (How about you? ;)).
> However, I don't have the time to rewrite everything I ever wrote. That
> said, I can at least make sure they are not spread.
> 

I'm sorry, I didn't mean to be rude. I've just tried to tell where I
took this assertion from.

> > 
> > What do you think if I omit dt_host->sibling check and make it:
> > 
> > if ( !dt_host )
> >      return -ENODEV;
> 
> We used to set dt_host even when booting with ACPI but that shouldn't be the
> case anymore. So I think this check should be fine.
> 

Ok, thank you. I'll do the change.

Best regards,
Oleksii.
diff mbox series

Patch

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index ecfa6822e4..895016b21e 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -33,6 +33,14 @@  config ACPI
 	  Advanced Configuration and Power Interface (ACPI) support for Xen is
 	  an alternative to device tree on ARM64.
 
+config HOST_DTB_EXPORT
+	bool "Export host device tree to hypfs if enabled"
+	depends on ARM && HYPFS && !ACPI
+	---help---
+
+	  Export host device-tree to hypfs so toolstack can have an access for the
+	  host device tree from Dom0. If you unsure say N.
+
 config GICV3
 	bool "GICv3 driver"
 	depends on ARM_64 && !NEW_VGIC
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 07f634508e..0a41f68f8c 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -8,6 +8,7 @@  obj-y += platforms/
 endif
 obj-$(CONFIG_TEE) += tee/
 obj-$(CONFIG_HAS_VPCI) += vpci.o
+obj-$(CONFIG_HOST_DTB_EXPORT) += host_dtb_export.o
 
 obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
 obj-y += bootfdt.init.o
diff --git a/xen/arch/arm/host_dtb_export.c b/xen/arch/arm/host_dtb_export.c
new file mode 100644
index 0000000000..794395683c
--- /dev/null
+++ b/xen/arch/arm/host_dtb_export.c
@@ -0,0 +1,307 @@ 
+/*
+ * xen/arch/arm/host_dtb_export.c
+ *
+ * Export host device-tree to the hypfs so toolstack can access
+ * host device-tree from Dom0
+ *
+ * Oleksii Moisieiev <oleksii_moisieiev@epam.com>
+ * Copyright (C) 2021, EPAM Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ */
+
+#include <xen/device_tree.h>
+#include <xen/err.h>
+#include <xen/guest_access.h>
+#include <xen/hypfs.h>
+#include <xen/init.h>
+
+#define HOST_DT_DIR "devicetree"
+
+static int host_dt_dir_read(const struct hypfs_entry *entry,
+                            XEN_GUEST_HANDLE_PARAM(void) uaddr);
+static unsigned int host_dt_dir_getsize(const struct hypfs_entry *entry);
+
+static const struct hypfs_entry *host_dt_dir_enter(
+    const struct hypfs_entry *entry);
+static void host_dt_dir_exit(const struct hypfs_entry *entry);
+
+static struct hypfs_entry *host_dt_dir_findentry(
+    const struct hypfs_entry_dir *dir, const char *name, unsigned int name_len);
+
+static const struct hypfs_funcs host_dt_dir_funcs = {
+    .enter = host_dt_dir_enter,
+    .exit = host_dt_dir_exit,
+    .read = host_dt_dir_read,
+    .write = hypfs_write_deny,
+    .getsize = host_dt_dir_getsize,
+    .findentry = host_dt_dir_findentry,
+};
+
+static int host_dt_prop_read(const struct hypfs_entry *entry,
+                    XEN_GUEST_HANDLE_PARAM(void) uaddr);
+
+static unsigned int host_dt_prop_getsize(const struct hypfs_entry *entry);
+
+const struct hypfs_funcs host_dt_prop_ro_funcs = {
+    .enter = host_dt_dir_enter,
+    .exit = host_dt_dir_exit,
+    .read = host_dt_prop_read,
+    .write = hypfs_write_deny,
+    .getsize = host_dt_prop_getsize,
+    .findentry = hypfs_leaf_findentry,
+};
+
+static HYPFS_DIR_INIT_FUNC(dt_dir, "node_template", &host_dt_dir_funcs);
+
+#define HYPFS_PROPERTY_MAX_SIZE 256
+static HYPFS_VARSIZE_INIT(dt_prop, XEN_HYPFS_TYPE_BLOB, "prop_template",
+                            HYPFS_PROPERTY_MAX_SIZE, &host_dt_prop_ro_funcs);
+
+static const char *get_name_from_path(const char *path)
+{
+    const char *name = strrchr(path, '/');
+    if ( !name )
+        name = path;
+    else
+    {
+        name++;
+        if ( !*name )
+            name--;
+    }
+
+    return name;
+}
+
+static char *get_root_from_path(const char *path, char *name)
+{
+    const char *nm = strchr(path, '/');
+    if ( !nm )
+        nm = path + strlen(path);
+    else
+    {
+        if ( !*nm )
+            nm--;
+    }
+
+    return memcpy(name, path, nm - path);
+}
+
+static int host_dt_dir_read(const struct hypfs_entry *entry,
+                            XEN_GUEST_HANDLE_PARAM(void) uaddr)
+{
+    int ret = 0;
+    struct dt_device_node *node;
+    struct dt_device_node *child;
+    const struct dt_property *prop;
+    struct hypfs_dyndir_id *data;
+
+    data = hypfs_get_dyndata();
+    if ( !data )
+        return -EINVAL;
+
+    node = data->data;
+    if ( !node )
+        return -EINVAL;
+
+    dt_for_each_property_node( node, prop )
+    {
+        ret = hypfs_read_dyndir_entry(&dt_prop.e, prop->name,
+                                      strlen(prop->name),
+                                      !prop->next && !node->child,
+                                      &uaddr);
+
+        if ( ret )
+            break;
+    }
+
+    for ( child = node->child; child != NULL; child = child->sibling )
+    {
+        const char *parsed_name = get_name_from_path(child->full_name);
+        data->data = child;
+
+        ret = hypfs_read_dyndir_entry(&dt_dir.e, parsed_name,
+                                         strlen(parsed_name),
+                                         child->sibling == NULL,
+                                         &uaddr);
+
+        if ( ret )
+            break;
+    }
+
+    return ret;
+}
+
+static unsigned int host_dt_dir_getsize(const struct hypfs_entry *entry)
+{
+    struct dt_device_node *node;
+    struct dt_device_node *child;
+    struct hypfs_dyndir_id *data;
+    const struct dt_property *prop;
+    unsigned int size = 0;
+
+    data = hypfs_get_dyndata();
+    if ( !data )
+        return -EINVAL;
+
+    node = data->data;
+    if ( !node )
+        return -EINVAL;
+
+    dt_for_each_property_node( node, prop )
+    {
+        size += hypfs_dyndir_entry_size(entry, prop->name);
+    }
+
+    for ( child = node->child; child != NULL; child = child->sibling )
+    {
+        const char *parsed_name = get_name_from_path(child->full_name);
+        size += hypfs_dyndir_entry_size(entry, parsed_name);
+    }
+
+    return size;
+}
+
+static DEFINE_PER_CPU(bool, data_alloc);
+
+static inline bool data_is_alloc(void)
+{
+    unsigned int cpu = smp_processor_id();
+    return per_cpu(data_alloc, cpu);
+}
+
+static inline void set_data_alloc(void)
+{
+    unsigned int cpu = smp_processor_id();
+    ASSERT(!per_cpu(data_alloc, cpu));
+
+    this_cpu(data_alloc) = true;
+}
+
+static inline void unset_data_alloc(void)
+{
+    this_cpu(data_alloc) = false;
+}
+
+static const struct hypfs_entry *host_dt_dir_enter(
+    const struct hypfs_entry *entry)
+{
+    struct hypfs_dyndir_id *data;
+
+    if ( !data_is_alloc() )
+    {
+        data = hypfs_alloc_dyndata(struct hypfs_dyndir_id);
+        if ( !data )
+            return ERR_PTR(-ENOMEM);
+
+        set_data_alloc();
+    }
+
+    if ( strcmp(entry->name, HOST_DT_DIR) == 0 )
+    {
+        data = hypfs_get_dyndata();
+        data->data = dt_host;
+    }
+
+    return entry;
+}
+
+static void host_dt_dir_exit(const struct hypfs_entry *entry)
+{
+    if ( !data_is_alloc() )
+        return;
+
+    hypfs_free_dyndata();
+    unset_data_alloc();
+}
+
+static struct hypfs_entry *host_dt_dir_findentry(
+    const struct hypfs_entry_dir *dir, const char *name, unsigned int name_len)
+{
+    struct dt_device_node *node;
+    char root_name[HYPFS_DYNDIR_ID_NAMELEN];
+    struct dt_device_node *child;
+    struct hypfs_dyndir_id *data;
+    struct dt_property *prop;
+
+    data = hypfs_get_dyndata();
+    if ( !data )
+        return ERR_PTR(-EINVAL);
+
+    node = data->data;
+    if ( !node )
+        return ERR_PTR(-EINVAL);
+
+    memset(root_name, 0, sizeof(root_name));
+    get_root_from_path(name, root_name);
+
+    for ( child = node->child; child != NULL; child = child->sibling )
+    {
+        if ( strcmp(get_name_from_path(child->full_name), root_name) == 0 )
+            return hypfs_gen_dyndir_entry(&dt_dir.e,
+                                  get_name_from_path(child->full_name), child);
+    }
+
+    dt_for_each_property_node( node, prop )
+    {
+
+        if ( dt_property_name_is_equal(prop, root_name) )
+            return hypfs_gen_dyndir_entry(&dt_prop.e, prop->name, prop);
+    }
+
+    return ERR_PTR(-ENOENT);
+};
+
+static int host_dt_prop_read(const struct hypfs_entry *entry,
+                    XEN_GUEST_HANDLE_PARAM(void) uaddr)
+{
+    const struct dt_property *prop;
+    struct hypfs_dyndir_id *data;
+
+    data = hypfs_get_dyndata();
+    if ( !data )
+        return -EINVAL;
+
+    prop = data->data;
+    if ( !prop )
+        return -EINVAL;
+
+    return copy_to_guest(uaddr, prop->value, prop->length) ?  -EFAULT : 0;
+}
+
+static unsigned int host_dt_prop_getsize(const struct hypfs_entry *entry)
+{
+    const struct hypfs_dyndir_id *data;
+    const struct dt_property *prop;
+
+    data = hypfs_get_dyndata();
+    if ( !data )
+        return -EINVAL;
+
+    prop = data->data;
+    if ( !prop )
+        return -EINVAL;
+
+    return prop->length;
+}
+
+static HYPFS_DIR_INIT_FUNC(host_dt_dir, HOST_DT_DIR, &host_dt_dir_funcs);
+
+static int __init host_dtb_export_init(void)
+{
+    ASSERT(dt_host && (dt_host->sibling == NULL));
+    unset_data_alloc();
+
+    hypfs_add_dir(&hypfs_root, &host_dt_dir, true);
+    hypfs_add_dyndir(&hypfs_root, &dt_dir);
+    return 0;
+}
+__initcall(host_dtb_export_init);