Re: [Xen-devel] Xen: ARM: Support for mapping ECAM PCIe Config Space Specified In Static ACPI Table

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 20/12/2016 00:54, Stefano Stabellini wrote:
> On Mon, 19 Dec 2016, Julien Grall wrote:
> > On 16/12/2016 15:49, Julien Grall wrote:
> > > On 14/12/16 08:00, Jiandi An wrote:
> > > > Xen currently doesn't map ECAM space specified in static ACPI table.
> > > > Seeking opinion on how this should be handled properly.
> > > > Each root complex ECAM region takes up 64K 4K pages (256MB).
> > > > For some platforms there might be multiple root complexes.
> > > > Is the plan to map all at once?Julien has mentioned support
> > > > for mapping ECAM may come when support for PCI passthrough is
> > > > added, is that right? What mechanism will it be? Will Xen or
> > > > dom0 be the one that parses the staic ACPI tables and map the ECAM space?
> > > For performance reason, each ECAM region would need to be mapped at
> > > once, so the stage-2 page table could take advantage of superpage (it
> > > will mostly be 2MB).
> > >
> > > Now, I don't think Xen should map the ECAM region in stage-2 before
> > > hand. All the regions may not be described in the MCFG and I would like
> > > to see a generic solution.
> > >
> > > Looking at the code (see pci_create_ecam_create in drivers/pci/ecam.c),
> > > ioremap is used. I believe the problem is the same for the 2 other
> > > threads you sent ( [1] and [2]).
> > >
> > > So it might be good to look at hooking up a call to
> > > XENMEM_add_to_physmap_range in ioremap.
> > >
> > > Any opinions?
> > I thought a bit more about it and I realized we need to be cautious on how to
> > proceed here.
> >
> > DOM0 will have a mix of real devices and emulated devices (e.g some part of
> > the GIC). For the emulated devices, DOM0 should not call
> > XENMEM_add_to_physmap_range. However, DOM0 is not aware what is emulated or
> > not, so even the current approach (hooking up in platform device) seems
> > fragile. We rely on Xen to say "this region cannot be mapped".
> You are right that Dom0 doesn't and shouldn't be able to tell the
> difference between emulated and real devices. But I don't think that
> should be a problem because Xen knows exactly if an MMIO region belongs
> to an emulated device thanks to the 1:1 mapping. When
> XENMEM_add_to_physmap_range is called, Xen can check whether the region
> belongs to an emulated device or a real device, mapping memory only if
> it belongs to a real device. No need to report errors: from Dom0 point
> of view the operation succeeded either way.
By no need to report errors, did you mean Xen failing, or DOM0 failing?

I looked at the code (map_dev_mmio_region), we check whether DOM0 is 
allowed to access the iomem. If a part of the region is not accessible, 
it will map nothing and return as it has succeeded.
This behavior is fairly odd because it means that a domain will never 
know whether a region has been mapped correctly. It may fail later on 
because, for instance, the domain was trying to map the device with MFN 
!= GFN. Thankfully we only map one page at the time (see the caller 
xenmem_add_to_physmap_one) but still a domain will expect to know what's 
going on.
So I think we need to associate a specific errno to tell the domain this 
region is not accessible.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel