On 10.09.21 16:33, Julien Grall wrote: > > > On 10/09/2021 14:27, Oleksandr Andrushchenko wrote: >> Hi, >> >> On 10.09.21 16:20, Julien Grall wrote: >>> >>> >>> On 10/09/2021 14:15, Oleksandr Andrushchenko wrote: >>>> Hi, Julien! >>> >>> Hi, >>> >>>> On 10.09.21 16:04, Julien Grall wrote: >>>>> >>>>> >>>>> On 10/09/2021 12:43, Oleksandr Andrushchenko wrote: >>>>>> Hi, Julien! >>>>> >>>>> Hi Oleksandr, >>>>> >>>>>> On 09.09.21 20:43, Julien Grall wrote: >>>>>>> Hi Oleksandr, >>>>>>> >>>>>>> On 03/09/2021 09:33, Oleksandr Andrushchenko wrote: >>>>>>>> From: Oleksandr Andrushchenko <[email protected]> >>>>>>>> >>>>>>>> In order vPCI to work it needs all access to PCI configuration space >>>>>>>> (ECAM) to be synchronized among all entities, e.g. hardware domain and >>>>>>>> guests. >>>>>>> >>>>>>> I am not entirely sure what you mean by "synchronized" here. Are you >>>>>>> refer to the content of the configuration space? >>>>>> >>>>>> We maintain hwdom's and guest's view of the registers we are interested >>>>>> in >>>>>> >>>>>> So, to have hwdom's view we also need to trap its access to the >>>>>> configuration space. >>>>>> >>>>>> Probably "synchronized" is not the right wording here. >>>>> I would simply say that we want to expose an emulated hostbridge to the >>>>> dom0 so we need to unmap the configuration space. >>>> Sounds good >>>>> >>>>>> >>>>>>> >>>>>>>> For that implement PCI host bridge specific callbacks to >>>>>>>> properly setup those ranges depending on particular host bridge >>>>>>>> implementation. >>>>>>>> >>>>>>>> Signed-off-by: Oleksandr Andrushchenko >>>>>>>> <[email protected]> >>>>>>>> --- >>>>>>>> xen/arch/arm/pci/ecam.c | 11 +++++++++++ >>>>>>>> xen/arch/arm/pci/pci-host-common.c | 16 ++++++++++++++++ >>>>>>>> xen/arch/arm/vpci.c | 13 +++++++++++++ >>>>>>>> xen/include/asm-arm/pci.h | 8 ++++++++ >>>>>>>> 4 files changed, 48 insertions(+) >>>>>>>> >>>>>>>> diff --git a/xen/arch/arm/pci/ecam.c b/xen/arch/arm/pci/ecam.c >>>>>>>> index 91c691b41fdf..92ecb2e0762b 100644 >>>>>>>> --- a/xen/arch/arm/pci/ecam.c >>>>>>>> +++ b/xen/arch/arm/pci/ecam.c >>>>>>>> @@ -42,6 +42,16 @@ void __iomem *pci_ecam_map_bus(struct >>>>>>>> pci_host_bridge *bridge, >>>>>>>> return base + (PCI_DEVFN(sbdf_t.dev, sbdf_t.fn) << >>>>>>>> devfn_shift) + where; >>>>>>>> } >>>>>>>> +static int pci_ecam_register_mmio_handler(struct domain *d, >>>>>>>> + struct pci_host_bridge >>>>>>>> *bridge, >>>>>>>> + const struct >>>>>>>> mmio_handler_ops *ops) >>>>>>>> +{ >>>>>>>> + struct pci_config_window *cfg = bridge->sysdata; >>>>>>>> + >>>>>>>> + register_mmio_handler(d, ops, cfg->phys_addr, cfg->size, NULL); >>>>>>> >>>>>>> We have a fixed array for handling the MMIO handlers. >>>>>> >>>>>> Didn't know that: >>>>>> >>>>>> xen/include/asm-arm/mmio.h:27:#define MAX_IO_HANDLER 16 >>>>>> >>>>>>> So you need to make sure we have enough space in it to store one >>>>>>> handler per handler. >>>>>>> >>>>>>> This is quite similar to the problem we had with the re-distribuor on >>>>>>> GICv3. Have a look there to see how we dealt with it. >>>>>> >>>>>> Could you please point me to that solution? I can only see >>>>>> >>>>>> /* Register mmio handle for the Distributor */ >>>>>> register_mmio_handler(d, &vgic_distr_mmio_handler, >>>>>> d->arch.vgic.dbase, >>>>>> SZ_64K, NULL); >>>>>> >>>>>> /* >>>>>> * Register mmio handler per contiguous region occupied by the >>>>>> * redistributors. The handler will take care to choose which >>>>>> * redistributor is targeted. >>>>>> */ >>>>>> for ( i = 0; i < d->arch.vgic.nr_regions; i++ ) >>>>>> { >>>>>> struct vgic_rdist_region *region = >>>>>> &d->arch.vgic.rdist_regions[i]; >>>>>> >>>>>> register_mmio_handler(d, &vgic_rdistr_mmio_handler, >>>>>> region->base, region->size, region); >>>>>> } >>>>>> which IMO doesn't care about the number of MMIOs we can handle >>>>> >>>>> Please see vgic_v3_init(). We update mmio_count that is then used as a >>>>> the second argument for domain_io_init(). >>>> >>>> Ah, so >>>> >>>> 1) This needs to be done before the array for the handlers is allocated >>>> >>>> 2) How do I know at the time of 1) how many bridges we have? >>> >>> By counting the number of bridge you want to expose to dom0? I am not >>> entirely sure what else you expect me to say. >> >> Ok, so I'll go over the device tree and find out all the bridges, e.g. >> devices with DEVICE_PCI type. >> >> Then I'll also need to exclude those being passed through (xen,passthrough) >> and the rest are the bridges for Domain-0? > > What you want to know if how many times register_mmio_handler() will be > called from domain_vpci_init(). > > You introduced a function pci_host_iterate_bridges() that will walk over the > bridges and then call the callback vpci_setup_mmio_handler(). So you could > introduce a new callback that will return 1 if > bridge->ops->register_mmio_handler is not NULL or 0.
Ok, clear. Something like: if ( (rc = domain_vgic_register(d, &count)) != 0 ) goto fail; *find out how many bridges and update count* if ( (rc = domain_io_init(d, count + MAX_IO_HANDLER)) != 0 ) goto fail; > > Cheers, > Thank you, Oleksandr
