Hi Ian,
On 14/10/2021 18:54, Ian Jackson wrote:
(Replying to both the earlier subthread on v5 and to the new v6
patch.)
Bertrand Marquis writes ("Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device tree
node in libxl"):
Now you suggest to add a new field arm_vpci in libxl__domain_create_state.
Hi. I was handwaving, hence "probably" :-). I hadn't actually looked
at the existing code to see precisely how it would fit.
Once we have done that I will need to access this structure to know if I need
to add the DT part and somehow to give it a value depending something which
for now would the number of pcidevs as there will be no user parameter anymore.
Right.
I looked at libxl_arm.c again:
It seems that the main entrypoint to all this is libxl__prepare_dtb,
and it is that function you want to do new stuff. That gets
libxl_domain_build_info (which is specified by the IDL and comes from
outside libxl, subject to libxl's default-filling machinery) and
libxl__domain_build_state (which is the general state struct).
The information you need is is libxl_domain_create_info.
libxl__domain_config_setdefault already arranges to set
c_info->passthrough based on the number of PCI PT devices
(search for `need_pt` in libxl_create.c).
That is, as I understand it on ARM vpci should be enabled if
passthrough is enabled and not otherwise. That is precisely what
the variable c_info->passthrough is.
On Arm, c_info->passthrough is also set when assigning platform devives
(e.g. a non-PCI network card). At least for now, we don't want to create
a node for vCPI in the Device-Tree because we don't enable the
emulation. So...
There is a slight issue because of the distinction between create_info
and build_info and domain_config (and, the corresponding _state)
structs. Currently the DT code ony gets b_info, not the whole
libxl_domain_config. These distinctions largely historical nowadays.
Certainly there is no reason not to pass a pointer to the whole
libxl_domain_config, rather than just libxl_domain_build_info, into
libxl__arch_domain_init_hw_description.
So I think the right approach for this looks something like this:
1. Change libxl__arch_domain_init_hw_description to take
libxl_domain_config* rather than libxl_domain_build_info*.
libxl_domain_config contains libxl_domain_build_info so
this is easy.
If you put in a convenience alias variable for the
libxl_domain_build_info* you can avoid extra typing in the function
body. (If you call the convenience alias `info` you won't need to
change the body at all, but maybe `info` isn't the best name so you
could rename it to `b_info` maybe; up to you.)
2. Make the same change to libxl__prepare_dtb.
3. Now you can use d_config->c_info.passthrough to gate the addition
of the additional stuff to the DT. Ie, that is a respin of this
patch 3/3.
... we will need to check d_config->num_pcidevs for the time being.
Cheers,
--
Julien Grall
