On 23/11/2021 06:58, Oleksandr Andrushchenko wrote:
Hi, Julien!
Hi Oleksandr,
On 22.11.21 19:36, Julien Grall wrote:
On 18/11/2021 10:46, Oleksandr Andrushchenko wrote:
On 18.11.21 09:27, Oleksandr Andrushchenko wrote:
+ unsigned int count;
+
+ if ( is_hardware_domain(d) )
+ /* For each PCI host bridge's configuration space. */
+ count = pci_host_get_num_bridges();
This first part makes sense to me. But...
+ else
... I don't understand how the else is related to this commit. Can you clarify
it?
+ /*
+ * There's a single MSI-X MMIO handler that deals with both PBA
+ * and MSI-X tables per each PCI device being passed through.
+ * Maximum number of supported devices is 32 as virtual bus
+ * topology emulates the devices as embedded endpoints.
+ * +1 for a single emulated host bridge's configuration space.
+ */
+ count = 1;
+#ifdef CONFIG_HAS_PCI_MSI
+ count += 32;
Surely, this is a decision that is based on other factor in the vPCI code. So
can use a define and avoid hardcoding the number?
Well, in the later series [1] this is defined via PCI_SLOT(~0) + 1 and there is
no dedicated
constant for that. I can use the same here, e.g. s/32/PCI_SLOT(~0) + 1
I would prefer if we introduce a new constant for that. This makes easier to
update the code if we decide to increase the number of virtual devices.
However, I am still not sure how the 'else' part is related to this commit. Can
you please clarify it?
Well, yes, this is too early for this patch to introduce some future knowledge,
so I'll instead have:
unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
{
if ( !has_vpci(d) )
return 0;
if ( is_hardware_domain(d) )
{
int ret = pci_host_iterate_bridges_and_count(d,
vpci_get_num_handlers_cb);
return ret < 0 ? 0 : ret;
}
/*
* This is a guest domain:
*
* 1 for a single emulated host bridge's configuration space.
*/
return 1;
I am afraid that my question stands even with this approach. This patch
is only meant to handle the hardware domain, therefore the change seems
to be out of context.
I would prefer if this change is done separately.
Cheers,
--
Julien Grall
