Hi,
On 23/11/2021 16:41, Oleksandr Andrushchenko wrote:
On 23.11.21 18:12, Julien Grall wrote:
On 23/11/2021 06:58, Oleksandr Andrushchenko wrote:
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.
While I do agree that MSI part and virtual bus topology are not belonging to
this
patch I can't agree with the rest: we already have MMIO handlers for guest
domains
and we introduce domain_vpci_get_num_mmio_handlers which must also account
on guests and stay consistent.
So, despite the patch has "hardware domain" in its name it doesn't mean we
should
break guests here.
We were already registering the handler for guest domain before your
patch. So this is nothing new.
At the moment, we always allocate an extra 16 slot for IO handlers (see
MAX_IO_HANDLER). So we are not breaking anything. Instead, this is
simply a latent bug.
Thus I do think the above is still correct wrt this patch.
The idea of splitting patch is to separate bug fix from new code. This
helps backporting and review.
In this case, we don't care about backport (PCI passthrough is no
supported) and the fix a simple enough. So I am not going to insist on
splitting to a separate patch.
However, this change *must* be explained in the commit message.
Cheers,
--
Julien Grall