Hi,
On 23/11/2021 16:44, Oleksandr Andrushchenko wrote:
On 23.11.21 18:05, Julien Grall wrote:
Hi Oleksandr,
On 23/11/2021 06:31, Oleksandr Andrushchenko wrote:
On 22.11.21 19:17, Julien Grall wrote:
Hi,
On 22/11/2021 16:23, Oleksandr Andrushchenko wrote:
On 22.11.21 17:29, Julien Grall wrote:
I would prefer to go with my way. This can be refined in the future if we find
Device-Tree that matches what you wrote.
Ok, so just to make it clear:
>a PCI device would always be described as a child of the hostbridges. So I would
rework the 'if' to also check if the parent type is not "pci"
That's correct. Thank you!
Ok, so how about
if ( is_pci_passthrough_enabled() && dt_device_type_is_equal(node,
"pci") )
{
bool skip = false;
/*
* If the parent is also a "pci" device, then "linux,pci-domain"
* should already be there, so nothing to do then.
*/
This comment is a bit confusing.
Do you have something on your mind?
Yes, explain that we only want to cover hostbridge (see my reply on the
previous answer).
I think what matter if the parent is a "pci" device, then the current node must
not be a hostbridge. So we can skip it.
By skipping you only mean we do not need to add/assign "linux,pci-domain",
right?
Yes.
However...
if ( node->parent && dt_device_type_is_equal(node->parent, "pci") )
skip = true;
if ( !skip && !dt_find_property(node, "linux,pci-domain", NULL) )
{
I played with a single if and it looks scary...
... how about introducing an helper that will return true if this node is
likely an hostbridge?
This is yet a single use of such a check: why would we need a helper and what
would that
helper do?
I like splitting code in multiple functions even if they are only called
once because this:
1) keeps the functions line count small
2) easier to understand what is the purpose of the check
In fact, I would actually move the handling for "linux,pci-domain" in a
separate helper. Something like:
/*
* When PCI passthrough is available we want to keep the
* "linux,pci-domain" in sync for every hostbridge.
*
* Xen may not have a driver for all the hostbridges. So we have
* to write an heuristic to detect whether a device node describes
* an hostbridge.
*
* The current heuristic assumes that a device is an hostbridge
* if the type is "pci" and then parent type is not "pci".
*/
static int handle_linux_pci_domain(struct dt_device *node)
{
if ( !is_pci_passthrough_enabled() )
return 0;
if ( !dt_device_type_is_equal(node, "pci") )
return 0;
if ( node->parent && dt_device_type_is_equal(node->parent) )
return 0;
if ( dt_find_property(node, "linux,pci-domain", NULL )
return 0;
/* Allocate and create the linux,pci-domain */
}
Cheers,
--
Julien Grall