Re: [Xen-devel] [PATCH 2/5] xen/arm: assign devices to boot domains

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 8 Aug 2019, Julien Grall wrote:
> Hi Stefano,
> 
> On 07/08/2019 23:46, Stefano Stabellini wrote:
> > On Tue, 15 Jan 2019, Julien Grall wrote:
> > > > > > diff --git a/xen/arch/arm/domain_build.c
> > > > > > b/xen/arch/arm/domain_build.c
> > > > > > index cc6b464..d48f77e 100644
> > > > > > --- a/xen/arch/arm/domain_build.c
> > > > > > +++ b/xen/arch/arm/domain_build.c
> > > > > > @@ -2094,6 +2094,88 @@ static int __init construct_domain(struct
> > > > > > domain
> > > > > > *d,
> > > > > > struct kernel_info *kinfo)
> > > > > >         return 0;
> > > > > >     }
> > > > > >     +static int __init scan_pt_node(const void *pfdt,
> > > > > > +                               int nodeoff, const char *name, int
> > > > > > depth,
> > > > > > +                               u32 address_cells, u32 size_cells,
> > > > > > +                               void *data)
> > > > > 
> > > > > Is it really necessary to parse the device-tree twice?
> > > > 
> > > > I don't think I understand this comment. The device tree fragment is not
> > > > scanned twice, just once I think. Am I missing something?
> > > 
> > > The previous patch is going through the DT to copy the properties over.
> > > This
> > > patch introduce a new function to go over the DT to find the Device to
> > > attach.
> > > So you are parsing the DT twice. Is there any reason for that?
> > 
> > I got your question now.  I spent quite some time to merge the two
> > paths, the first one used to copy the fragment, the second one used to
> > parse it, into a single function. It is difficult because the
> > information convenient to use for one case, it is not convenient for the
> > other (specifically figuring out when to call fdt_end_node when called
> > from device_tree_for_each_node.) I managed to do it though, in the next
> > version there will be only one parsing of the fragment.
> 
> I wasn't necessarily asking to merge the two. I wanted to understand whether
> we can avoid two parsing yet keeping the code simple. Sorry if it wasn't clear
> enough.
> 
> [...]
> 
> > > > > Furthermore, this is assuming all the nodes in the fragment will be
> > > > > under the GIC controller.  You may want to passthrough a interrupt
> > > > > controller (i.e GPIO) to the guest and the related device.
> > > > 
> > > > I don't think that the non-GIC scenario is supported today. I don't
> > > > think it works even without dom0less. I wrote more about this as a reply
> > > > to the other email.
> > > I believe this works out-of-box with the guest. If we take the example of
> > > the
> > > GPIO controller, it may be behind the GIC. You only care to route those
> > > interrupts used by GPIO and MMIO associated. The rest can be describe
> > > normally
> > > in the DT.
> > > 
> > > Of course, this means that you need to passthrough all devices using the
> > > GPIO
> > > controller to that guest.
> > > 
> > > So I still think you need to check whether the interrupts belongs the GIC
> > > or
> > > not.
> > 
> > I think I understand what you meant now. I could add a check before
> > routing any interrupts to the guest, to make sure that they are GIC
> > interrupts. However, the way to do that check I believe would be using
> > the "interrupt-parent" property, but we have just discussed about
> > removing it.
> 
> I have suggested a way to keep it and avoid writing down the phandle value.
> 
> > 
> > So, if the user provides a fragment that doesn't have any
> > "interrupt-parent" properties, how can I check that the interrupts are
> > GIC interrupts?
> 
> The problem here is you are re-using the guest "interrupts" property for
> finding out the host interrupts. Firstly, this does not cater the case where
> guest DT is using the property "interrupt-extended".
> 
> It feels to me that we should look at the "interrupts" property from the host
> DT and map them 1:1 (i.e irq == virq). The property in the partial DT would
> just be replicating the values from the host DT.

Yes, that works, good suggestion.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel