Re: [PATCH v5 01/11] xen/arm: xc_domain_ioport_permission(..) not supported on ARM.

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Oct 11, 2021 at 02:16:19PM +0000, Bertrand Marquis wrote:
> Hi Roger,
> 
> > On 11 Oct 2021, at 14:57, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> > 
> > On Mon, Oct 11, 2021 at 01:40:30PM +0000, Bertrand Marquis wrote:
> >> Hi Roger,
> >> 
> >> + Oleksandr to have a better PCI expert then me.
> >> 
> >>> On 11 Oct 2021, at 14:20, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> >>> 
> >>> On Mon, Oct 11, 2021 at 12:11:04PM +0000, Bertrand Marquis wrote:
> >>>> Hi Roger,
> >>>> 
> >>>>> On 11 Oct 2021, at 12:47, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> >>>>> 
> >>>>> On Wed, Oct 06, 2021 at 06:40:27PM +0100, Rahul Singh wrote:
> >>>>>> ARM architecture does not implement I/O ports. Ignore this call on ARM
> >>>>>> to avoid the overhead of making a hypercall just for Xen to return
> >>>>>> -ENOSYS.
> >>>>> 
> >>>>> What is the cal trace of this function actually on Arm?
> >>>>> 
> >>>>> AFAICT libxl will only call xc_domain_ioport_permission if there are
> >>>>> IO ports explicitly defined in the guest configuration, or if any of
> >>>>> the BARs of the PCI device is in the IO space, which is not possible
> >>>>> on Arm.
> >>>> 
> >>>> PCI devices BARs can be in the IO space as the PCI devices are not
> >>>> Arm specific. There is not ioports on arm so to be used those can be
> >>>> in some cases remapped and accessed as MMIOs or are not possible
> >>>> to use at all.
> >>>> 
> >>>> But the IO space does appear when BARs are listed even on Arm.
> >>> 
> >>> Urg, I wonder whether those devices with IO BARs will work correctly
> >>> under Arm then.
> >>> 
> >>> How do you know whether the BAR has been remapped from IO space into
> >>> MMIO?
> >> 
> >> We cannot, I think the platform will define if this is the case and where.
> >> @oleksandr: I remember that this was discussed during some of our
> >> meetings but I have no idea of the details here, can you help ?
> >> 
> >>> 
> >>> IMO instead of faking a successful return value from
> >>> xc_domain_ioport_permission we should avoid the call completely in the
> >>> first place, specially if we need to instead issue a call to
> >>> xc_domain_iomem_permission.
> >> 
> >> At the end we will never have to issue this because this will never be a 
> >> matter
> >> of “iomem” permission as there would not be any way to cut on something 
> >> under
> >> the page. If this is to be supported one day, it will probably have to be 
> >> fully emulated
> >> to keep the isolation.
> > 
> > So you have a set of memory pages that map accesses from
> > MMIO into IO space but it's not possible to isolate specific IO port
> > regions as they are all contiguous in the same page(s).
> 
> Exact.
> 
> > 
> >> Right now on arm you can just make the more simple assumption that ioports 
> >> are
> >> just not supported.
> > 
> > Would it make sense in the future to provide a memory region to guests
> > in order to use for IO port accesses, and call
> > xc_domain_ioport_permission to set which ports would be allowed?
> 
> Right now we do not plan to support this at all and we will have to
> figure this out if we do this one day.
> 
> > 
> > I think the commit message needs to at least be expanded in order to
> > contain the information provided here. It might also be helpful to
> > figure out whether we would have to handle IO port accesses in the
> > future on Arm, or if it's fine to just ignore them.
> 
> All our investigations and tests have been done without supporting it
> without any issues so this is not a critical feature (most devices can
> be operated without using the I/O ports).

IMO we should let the users know they attempted to use a device with
BARs in the IO space, and that those BARs won't be accessible which
could make the device not function as expected.

Do you think it would be reasonable to attempt the hypercall on Arm
also, and in case of error (on Arm) just print a warning message and
continue operations as normal?

Thanks, Roger.