[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH RFC v2] vPCI: account for hidden devices
On 25.05.2023 17:02, Roger Pau Monné wrote: > On Thu, May 25, 2023 at 04:39:51PM +0200, Jan Beulich wrote: >> On 24.05.2023 17:56, Roger Pau Monné wrote: >>> On Wed, May 24, 2023 at 03:45:58PM +0200, Jan Beulich wrote: >>>> --- a/xen/drivers/vpci/header.c >>>> +++ b/xen/drivers/vpci/header.c >>>> @@ -218,6 +218,7 @@ static int modify_bars(const struct pci_ >>>> struct vpci_header *header = &pdev->vpci->header; >>>> struct rangeset *mem = rangeset_new(NULL, NULL, 0); >>>> struct pci_dev *tmp, *dev = NULL; >>>> + const struct domain *d; >>>> const struct vpci_msix *msix = pdev->vpci->msix; >>>> unsigned int i; >>>> int rc; >>>> @@ -285,9 +286,11 @@ static int modify_bars(const struct pci_ >>>> >>>> /* >>>> * Check for overlaps with other BARs. Note that only BARs that are >>>> - * currently mapped (enabled) are checked for overlaps. >>>> + * currently mapped (enabled) are checked for overlaps. Note also that >>>> + * for Dom0 we also need to include hidden, i.e. DomXEN's, devices. >>>> */ >>>> - for_each_pdev ( pdev->domain, tmp ) >>>> +for ( d = pdev->domain; ; d = dom_xen ) {//todo >>> >>> Looking at this again, I think this is slightly more complex, as during >>> runtime dom0 will get here with pdev->domain == hardware_domain OR >>> dom_xen, and hence you also need to account that devices that have >>> pdev->domain == dom_xen need to iterate over devices that belong to >>> the hardware_domain, ie: >>> >>> for ( d = pdev->domain; ; >>> d = (pdev->domain == dom_xen) ? hardware_domain : dom_xen ) >> >> Right, something along these lines. To keep loop continuation expression >> and exit condition simple, I'll probably prefer >> >> for ( d = pdev->domain != dom_xen ? pdev->domain : hardware_domain; >> ; d = dom_xen ) > > LGTM. I would add parentheses around the pdev->domain != dom_xen > condition, but that's just my personal taste. > > We might want to add an > > ASSERT(pdev->domain == hardware_domain || pdev->domain == dom_xen); > > here, just to remind that this chunk must be revisited when adding > domU support (but you can also argue we haven't done this elsewhere), > I just feel here it's not so obvious we don't want do to this for > domUs. I could add such an assertion, if only ... >>> And we likely want to limit this to devices that belong to the >>> hardware_domain or to dom_xen (in preparation for vPCI being used for >>> domUs). >> >> I'm afraid I don't understand this remark, though. > > This was looking forward to domU support, so that you already cater > for pdev->domain not being hardware_domain or dom_xen, but we might > want to leave that for later, when domU support is actually > introduced. ... I understood why this checking doesn't apply to DomU-s as well, in your opinion. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |