Re: [PATCH v4 00/11] PCI devices passthrough on Arm, part 3

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

On Fri, Nov 19, 2021 at 02:26:21PM +0000, Oleksandr Andrushchenko wrote:
> 
> 
> On 19.11.21 16:23, Roger Pau Monné wrote:
> > On Fri, Nov 19, 2021 at 02:56:12PM +0100, Jan Beulich wrote:
> >> On 05.11.2021 07:56, Oleksandr Andrushchenko wrote:
> >>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> >>>
> >>> Hi, all!
> >>>
> >>> This patch series is focusing on vPCI and adds support for non-identity
> >>> PCI BAR mappings which is required while passing through a PCI device to
> >>> a guest. The highlights are:
> >>>
> >>> - Add relevant vpci register handlers when assigning PCI device to a 
> >>> domain
> >>>    and remove those when de-assigning. This allows having different
> >>>    handlers for different domains, e.g. hwdom and other guests.
> >>>
> >>> - Emulate guest BAR register values based on physical BAR values.
> >>>    This allows creating a guest view of the registers and emulates
> >>>    size and properties probe as it is done during PCI device enumeration 
> >>> by
> >>>    the guest.
> >>>
> >>> - Instead of handling a single range set, that contains all the memory
> >>>    regions of all the BARs and ROM, have them per BAR.
> >>>
> >>> - Take into account guest's BAR view and program its p2m accordingly:
> >>>    gfn is guest's view of the BAR and mfn is the physical BAR value as set
> >>>    up by the host bridge in the hardware domain.
> >>>    This way hardware doamin sees physical BAR values and guest sees
> >>>    emulated ones.
> >>>
> >>> The series also adds support for virtual PCI bus topology for guests:
> >>>   - We emulate a single host bridge for the guest, so segment is always 0.
> >>>   - The implementation is limited to 32 devices which are allowed on
> >>>     a single PCI bus.
> >>>   - The virtual bus number is set to 0, so virtual devices are seen
> >>>     as embedded endpoints behind the root complex.
> >>>
> >>> The series was also tested on:
> >>>   - x86 PVH Dom0 and doesn't break it.
> >>>   - x86 HVM with PCI passthrough to DomU and doesn't break it.
> >>>
> >>> Thank you,
> >>> Oleksandr
> >>>
> >>> Oleksandr Andrushchenko (11):
> >>>    vpci: fix function attributes for vpci_process_pending
> >>>    vpci: cancel pending map/unmap on vpci removal
> >>>    vpci: make vpci registers removal a dedicated function
> >>>    vpci: add hooks for PCI device assign/de-assign
> >>>    vpci/header: implement guest BAR register handlers
> >>>    vpci/header: handle p2m range sets per BAR
> >>>    vpci/header: program p2m with guest BAR view
> >>>    vpci/header: emulate PCI_COMMAND register for guests
> >>>    vpci/header: reset the command register when adding devices
> >>>    vpci: add initial support for virtual PCI bus topology
> >>>    xen/arm: translate virtual PCI bus topology for guests
> >> If I'm not mistaken by the end of this series a guest can access a
> >> device handed to it. I couldn't find anything dealing with the
> >> uses of vpci_{read,write}_hw() and vpci_hw_{read,write}*() to cover
> >> config registers not covered by registered handlers. IMO this should
> >> happen before patch 5: Before any handlers get registered the view a
> >> guest would have would be all ones no matter which register it
> >> accesses. Handler registration would then "punch holes" into this
> >> "curtain", as opposed to Dom0, where handler registration hides
> >> previously visible raw hardware registers.
> > FWIW, I've also raised the same concern in a different thread:
> >
> > https://urldefense.com/v3/__https://lore.kernel.org/xen-devel/YYD7VmDGKJRkid4a@Air-de-Roger/__;!!GF_29dbcQIUBPA!gihX6c2Mg87AKSDMmh1xrRnPjTXZkgR3kqPxg-WPghAdbY59gmJK5Ngkf4OJFK6NU5IwCStYAQ$
> >  [lore[.]kernel[.]org]
> >
> > It seems like this is future work,
> Yes, it takes quite some time to get even what we have now...
> >   but unless such a model is
> > implemented vPCI cannot be used for guest passthrough.
> But it can be a tech-preview

I'm afraid 'Tech Preview' requires the feature to be functionally
complete, which I won't consider the case for vPCI unless the above is
solved. I think we could only label this as 'Experimental' until the
remaining work is done, but the limitations would need to be clearly
noted, as it would be completely insecure.

Thanks, Roger.