[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC v2 3/8] xen/arm: Export host device-tree to hypfs
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.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |