Re: [PATCH v6 3/3] arm/libxl: Emulated PCI device tree node in libxl [and 1 more messages]

[Julien Grall <julien@xxxxxxx>]

Hi Michal,

On 15/10/2021 12:53, Michal Orzel wrote:
> On 15.10.2021 13:46, Ian Jackson wrote:
> > Michal Orzel writes ("Re: [PATCH v6 3/3] arm/libxl: Emulated PCI device tree node in 
> > libxl [and 1 more messages]"):
> > > On 15.10.2021 12:02, Ian Jackson wrote:
> > > > Can you leave a comment somewhere (near where c_info.passthrough is
> > > > set) pointing to this use of num_pcidevs ?  That might save someone
> > > > some future confusion.
> > > Here, c_info->passthrough is set to enabled if either d_config->num_pcidevs or
> > > d_config->num_dtdevs is set. Do you think we need to add there additional 
> > > comment?
> > > If so can you please help with what should I write there?
> > >
> > > BTW. None of the patch I'm preparing with regards to this discussion modifies 
> > > libxl_create
> > > where c_info.passthrough i set. Do you still want me to add some comment there?
> > Yes, I think so.  I won't insisting on it if you feel it doesn't make
> > sense.
> >
> > Maybe something like
> >
> >    // NB, on ARM, libxl__arch_blah directly examines num_pcidevs to
> >    // decide whether to enable vpci, rather than using c_info->passthrough.
> >    // This will be insufficient if and when ARM does PCI hotplug.
> Well we are not enabling vpci. We are creating a DT node for it.
> So either I will write:
> /*
>   * Note: libxl_arm directly examines num_pcidevs to decide whether
>   * to create a vPCI DT node, rather than using c_info->passthrough.
>   * This will be insufficient if and when ARM does PCI hotplug.
>   */
>
> or I will not add any comment (it can be add in the future when "enabling" 
> vpci).
> @Julien?
I would prefer if we had a comment. Your proposal makes more sense as we 
only create the DT.
Long term, I would expect a similar check to be necessary to set the 
vCPI flag at the domain creation. So it would be best to introduce an 
internal field 'vpci' to avoid duplicating that check. (Note I am not 
requesting this change for Xen 4.16).
Cheers,

--
Julien Grall