| Message ID | 1421159133-31526-16-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 20/02/15 16:56, Ian Campbell wrote: > On Tue, 2015-01-13 at 14:25 +0000, Julien Grall wrote: >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > > Is this function still needed in the new model which doesn't do > automatic mappings etc? It's used is the pass-through DOMCTL code. Regards,
Hi Ian, On 23/02/15 15:30, Ian Campbell wrote: > On Tue, 2015-01-13 at 14:25 +0000, Julien Grall wrote: > >> +/* This limit is used by the hypercalls to restrict the size of the path */ >> +#define DEVICE_TREE_MAX_PATHLEN 1024 > > Is this something you've made up or derived from the DT spec/ePAPR etc? I didn't find a such requirements on the spec. I chose this number based on the linux pathlen because the path is also used in the sysfs (/sys/firmware/devicetree). > Apart from this the patch seems fine, clarifying the genesis of that > number in the commit log would be sufficient for me. I will update the commit message. Regards,
On 23/02/15 16:27, Ian Campbell wrote: > On Mon, 2015-02-23 at 16:01 +0000, Julien Grall wrote: >> Hi Ian, >> >> On 23/02/15 15:30, Ian Campbell wrote: >>> On Tue, 2015-01-13 at 14:25 +0000, Julien Grall wrote: >>> >>>> +/* This limit is used by the hypercalls to restrict the size of the path */ >>>> +#define DEVICE_TREE_MAX_PATHLEN 1024 >>> >>> Is this something you've made up or derived from the DT spec/ePAPR etc? >> >> I didn't find a such requirements on the spec. > > I vaguely recall a limit on the length of a single node name, and a > limit on the depth which they may nest, which can be multiplied. It's > probably an unhelpfully large number though, so... > >> I chose this number based on the linux pathlen because the path is also >> used in the sysfs (/sys/firmware/devicetree). > > ...that's a good as anything I suppose! Hmmm... I'm not so sure about the 1024 limit anymore. Linux is defining PATH_MAX to 4096 but I don't see many usage in the sysfs code. And this value may change from one OS to another. Although, 1024 sounds a very long path to write in the configuration file... Maybe we should support alias too? Regards,
On 11/03/15 12:34, Ian Campbell wrote: > On Tue, 2015-03-10 at 13:58 +0000, Julien Grall wrote: >> On 23/02/15 16:27, Ian Campbell wrote: >>> On Mon, 2015-02-23 at 16:01 +0000, Julien Grall wrote: >>>> Hi Ian, >>>> >>>> On 23/02/15 15:30, Ian Campbell wrote: >>>>> On Tue, 2015-01-13 at 14:25 +0000, Julien Grall wrote: >>>>> >>>>>> +/* This limit is used by the hypercalls to restrict the size of the path */ >>>>>> +#define DEVICE_TREE_MAX_PATHLEN 1024 >>>>> >>>>> Is this something you've made up or derived from the DT spec/ePAPR etc? >>>> >>>> I didn't find a such requirements on the spec. >>> >>> I vaguely recall a limit on the length of a single node name, and a >>> limit on the depth which they may nest, which can be multiplied. It's >>> probably an unhelpfully large number though, so... >>> >>>> I chose this number based on the linux pathlen because the path is also >>>> used in the sysfs (/sys/firmware/devicetree). >>> >>> ...that's a good as anything I suppose! >> >> Hmmm... I'm not so sure about the 1024 limit anymore. Linux is defining >> PATH_MAX to 4096 but I don't see many usage in the sysfs code. >> >> And this value may change from one OS to another. Although, 1024 sounds >> a very long path to write in the configuration file... > > I imagine in practice it will be cut-and-pasted, but yes. > >> Maybe we should >> support alias too? > > Perhaps 2048 + (optionally, perhaps later) add alias support? > > 2048 because I assume 4096 being PAGE_SIZE would have some interesting > corner cases, especially for a target buffer on the stack. If not then > why not go straight for 4096? XSM seems to use 4096 to copy string for an hypercall buffer. I will use the same here. Regards,
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index af73b3b..4c9daea 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -13,6 +13,7 @@ #include <xen/config.h> #include <xen/types.h> #include <xen/init.h> +#include <xen/guest_access.h> #include <xen/device_tree.h> #include <xen/kernel.h> #include <xen/lib.h> @@ -23,6 +24,7 @@ #include <xen/cpumask.h> #include <xen/ctype.h> #include <asm/setup.h> +#include <xen/err.h> const void *device_tree_flattened; dt_irq_xlate_func dt_irq_xlate; @@ -277,6 +279,23 @@ struct dt_device_node *dt_find_node_by_path(const char *path) return np; } +int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen, + struct dt_device_node **node) +{ + char *path; + + path = safe_copy_string_from_guest(u_path, u_plen, + DEVICE_TREE_MAX_PATHLEN); + if ( IS_ERR(path) ) + return PTR_ERR(path); + + *node = dt_find_node_by_path(path); + + xfree(path); + + return (*node == NULL) ? -ESRCH : 0; +} + struct dt_device_node *dt_find_node_by_alias(const char *alias) { const struct dt_alias_prop *app; diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index 19d0e45..9b0ed00 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -22,6 +22,9 @@ #define DEVICE_TREE_MAX_DEPTH 16 +/* This limit is used by the hypercalls to restrict the size of the path */ +#define DEVICE_TREE_MAX_PATHLEN 1024 + /* * Struct used for matching a device */ @@ -456,6 +459,20 @@ struct dt_device_node *dt_find_node_by_alias(const char *alias); */ struct dt_device_node *dt_find_node_by_path(const char *path); + +/** + * dt_find_node_by_gpath - Same as dt_find_node_by_path but retrieve the + * path from the guest + * + * @u_path: Xen Guest handle to the buffer containing the path + * @u_plen: Length of the buffer + * @node: TODO + * + * Return 0 if succeed otherwise -errno + */ +int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen, + struct dt_device_node **node); + /** * dt_get_parent - Get a node's parent if any * @node: Node to get parent
Signed-off-by: Julien Grall <julien.grall@linaro.org> --- Changes in v3: - Use the new prototype of safe_copy_string_from_guest Changes in v2: - guest_copy_string_from_guest has been renamed into safe_copy_string_from_guest --- xen/common/device_tree.c | 19 +++++++++++++++++++ xen/include/xen/device_tree.h | 17 +++++++++++++++++ 2 files changed, 36 insertions(+)