Hi, Julien! 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? > > Cheers, > Thank you, Oleksandr
