On Wed, Oct 27, 2021 at 02:06:40PM +0000, Oleksandr Andrushchenko wrote: > > > 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...)
I understand that not everything will be supported, that's perfectly fine, but we should aim to not make supporting those use cases harder in the future. > > 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. I understand that BAR sizing will not be needed if the structure is pre-initialized, but I also cannot see why it would be impossible, at least on x86. > > 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. I'm fine to split it if there's a hard requirement, but I'm afraid so far I'm not convinced it's required. Maybe if you could elaborate on why BAR sizing might not be possible when assigning to domU I could be convinced. Another option might be to just modify init_bars to have slightly different paths for dom0 vs domU. Thanks, Roger.
