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

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 22.11.21 10:22, Jan Beulich wrote:
> On 19.11.2021 15: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!n37Yig9pAAyho7fB9kC-q0T-gjC_utpzjtQxNv8udMX0dXK54PWcHwBqtmzHXSc5lTkzKu4XfQ$
>>  [lore[.]kernel[.]org]
>>
>> It seems like this is future work, but unless such a model is
>> implemented vPCI cannot be used for guest passthrough.
>>
>> I'm fine with doing it in a separate series, but needs to be kept in
>> mind.
> Not just this - it also needs to be recorded in this cover letter and
> imo also in a comment in the sources somewhere. Or else the question
> will (validly) be raised again and again.
I am fine adding such a comment, but am not sure where to put it.
What would be your best bet if you were to look for this information?
I think we can put that in xen/drivers/vpci/vpci.c at the top, right
after the license in the same comment block.
>
> Jan
>
Thank you,
Oleksandr