On 27.10.21 16:23, Roger Pau Monné wrote: > On Wed, Oct 27, 2021 at 11:59:47AM +0000, Oleksandr Andrushchenko wrote: >> Hi, Roger! >> >> On 27.10.21 13:17, Oleksandr Andrushchenko wrote: >>> Hi, Roger! >>> >>> On 13.10.21 16:51, Roger Pau Monné wrote: >>>> On Thu, Sep 30, 2021 at 10:52:15AM +0300, Oleksandr Andrushchenko wrote: >>>>> From: Oleksandr Andrushchenko <[email protected]> >>>>> >>>>> This is in preparation for dynamic assignment of the vPCI register >>>>> handlers depending on the domain: hwdom or guest. >>>>> The need for this step is that it is easier to have all related >>>>> functionality >>>>> put at one place. When the subsequent patches add decisions on which >>>>> handlers to install, e.g. hwdom or guest handlers, then this is easily >>>>> achievable. >>>> Won't it be possible to select the handlers to install in init_bars >>>> itself? >>> It is possible >>>> Splitting it like that means you need to iterate over the numbers of >>>> BARs twice (one in add_bar_handlers and one in init_bars), which makes >>>> it more likely to introduce errors or divergences. >>>> >>>> Decoupling the filling of vpci_bar data with setting the handlers >>>> seems slightly confusing. >>> Ok, I won't introduce add_bar_handlers, thus rendering this patch useless. >>> I'll drop it and re-work the upcoming patches with this respect >> On the other hand after thinking a bit more. >> What actually init_bars do? >> 1. Runs once per each pdev (__init?) >> 2. Sizes the BARs and detects their type, sets up pdev->vpci->header BAR >> values >> 3. Adds register handlers. >> >> For DomU we only need 3), so we can setup guest handlers. > I think you assume that there will always be a hardware domain with > vPCI enabled that will get the device assigned and thus init_bars will > be executed prior to assigning to a domU. Yes, this is the current assumption... > > But what about dom0less, it was decided to put dom0less out of scope for now > or when using a classic PV dom0? I thought that vPCI is only used for PVH Dom0 and it is enough for now (yes, this is a weak argument, but we do not want PCI passthrough on Arm to become a never ending game... since 2015...) > In that case > the device won't get assigned to a hardware domain with vPCI support, > so the vpci structure won't be allocated or filled, Yes, this is true. But because of the 3 functionflities of the init_bars is doing it might still need some dis-aggregation, e.g. BAR sizing is not needed and might not be possible while assigning to a DomU. So, I think that init_bars will need to be split in any case. > and hence > init_bars would have to be executed when assigning to a domU. Please see above: not sure init_bars can exist in its form to achieve that. One of the steps this patch is doing is we split init_bars into a) register assignment b) all the reset: initial pdev's header initialization, sizing etc.
The same is true for MSI/MSI-X. When we add support for MSI/MSI-X on Arm you will see the same: we need to split [1] (this is WIP). So, I am still convinced that we need add_bar_handlers in some form. > Thanks, Roger. > [1] https://gitlab.com/rahsingh/xen-integration/-/commit/7b898601261fc3ad834ac3d06cc4c784f33c95bb
