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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 20 Dec 2016, Julien Grall wrote:
> 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?

Sorry I haven't been clear. I meant that if Dom0 asks Xen to map a
region which belongs to an emulated device, of course Xen won't actually
do any mappings, but it is not an error condition. Xen shouldn't return
error for mappings that hasn't performed because the region belongs to
an emulated device.

Of course, Xen should report errors for any genuine error conditions.


> 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.


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