Re: [PATCH v1 4/6] vpci: add SR-IOV support for PVH Dom0

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 3/4/26 10:16, Roger Pau Monné wrote:
> On Wed, Mar 04, 2026 at 08:43:17AM +0000, Mykyta Poturai wrote:
>> On 7/28/25 14:33, Roger Pau Monné wrote:
>>> On Fri, Jul 25, 2025 at 02:24:33PM +0000, Mykyta Poturai wrote:
>>>> From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>>>>
>>>> This code is expected to only be used by privileged domains,
>>>> unprivileged domains should not get access to the SR-IOV capability.
>>>>
>>>> Implement RW handlers for PCI_SRIOV_CTRL register to dynamically
>>>> map/unmap VF BARS. Recalculate BAR sizes before mapping VFs to account
>>>> for possible changes in the system page size register.
>>>>
>>>> Relies on dom0 to enable SR-IOV and PHYSDEVOP to inform Xen about
>>>> addition/removal of VFs.
>>>
>>> Why I'm not opposed to allowing registration of devices using
>>> PHYSDEVOP, can't Xen detect the addition of the VFs and add them
>>> itself?
>>>
>>> When I worked on this long time ago, the version of the series that I
>>> posted had registration of the VFs done by Xen also:
>>>
>>> https://lore.kernel.org/xen-devel/20180717094830.54806-12-roger.pau@xxxxxxxxxx/
>>>
>>>>
>>>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>>>> Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>
>>>> ---
>>>>   CHANGELOG.md              |   3 +-
>>>>   SUPPORT.md                |   2 -
>>>>   xen/drivers/vpci/Makefile |   2 +-
>>>>   xen/drivers/vpci/header.c |   3 +
>>>>   xen/drivers/vpci/sriov.c  | 235 ++++++++++++++++++++++++++++++++++++++
>>>>   xen/drivers/vpci/vpci.c   |   1 +
>>>>   xen/include/xen/vpci.h    |   7 +-
>>>>   7 files changed, 247 insertions(+), 6 deletions(-)
>>>>   create mode 100644 xen/drivers/vpci/sriov.c
>>>>
>>>> diff --git a/CHANGELOG.md b/CHANGELOG.md
>>>> index 5f31ca08fe..7b0e8beb76 100644
>>>> --- a/CHANGELOG.md
>>>> +++ b/CHANGELOG.md
>>>> @@ -23,8 +23,7 @@ The format is based on [Keep a 
>>>> Changelog](https://keepachangelog.com/en/1.0.0/
>>>>    - On x86:
>>>>      - Option to attempt to fixup p2m page-faults on PVH dom0.
>>>>      - Resizable BARs is supported for PVH dom0.
>>>> -   - Support PCI passthrough for HVM domUs when dom0 is PVH (note SR-IOV
>>>> -     capability usage is not yet supported on PVH dom0).
>>>> +   - Support PCI passthrough for HVM domUs when dom0 is PVH.
>>>
>>> Don't you need to move this out of the x86 specific section?
>>>
>>> According to the cover letter you are testing on an ARM board, so this
>>> probably needs to be put in a non-arch part of CHANGELOG?
>>>
>>>>      - Smoke tests for the FreeBSD Xen builds in Cirrus CI.
>>>>   
>>>>    - On Arm:
>>>> diff --git a/SUPPORT.md b/SUPPORT.md
>>>> index 6a82a92189..830b598714 100644
>>>> --- a/SUPPORT.md
>>>> +++ b/SUPPORT.md
>>>> @@ -170,8 +170,6 @@ unexpected behavior or issues on some hardware.
>>>>   
>>>>   At least the following features are missing on a PVH dom0:
>>>>   
>>>> -  * PCI SR-IOV.
>>>> -
>>>>     * Native NMI forwarding (nmi=dom0 command line option).
>>>>   
>>>>     * MCE handling.
>>>> diff --git a/xen/drivers/vpci/Makefile b/xen/drivers/vpci/Makefile
>>>> index a7c8a30a89..fe1e57b64d 100644
>>>> --- a/xen/drivers/vpci/Makefile
>>>> +++ b/xen/drivers/vpci/Makefile
>>>> @@ -1,2 +1,2 @@
>>>> -obj-y += vpci.o header.o rebar.o
>>>> +obj-y += vpci.o header.o rebar.o sriov.o
>>>>   obj-$(CONFIG_HAS_PCI_MSI) += msi.o msix.o
>>>> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
>>>> index f947f652cd..0a840c6dcc 100644
>>>> --- a/xen/drivers/vpci/header.c
>>>> +++ b/xen/drivers/vpci/header.c
>>>> @@ -839,6 +839,9 @@ static int cf_check init_header(struct pci_dev *pdev)
>>>>   
>>>>       ASSERT(rw_is_write_locked(&pdev->domain->pci_lock));
>>>>   
>>>> +    if ( pdev->info.is_virtfn )
>>>> +        return 0;
>>>> +
>>>>       switch ( pci_conf_read8(pdev->sbdf, PCI_HEADER_TYPE) & 0x7f )
>>>>       {
>>>>       case PCI_HEADER_TYPE_NORMAL:
>>>> diff --git a/xen/drivers/vpci/sriov.c b/xen/drivers/vpci/sriov.c
>>>> new file mode 100644
>>>> index 0000000000..640430e3e9
>>>> --- /dev/null
>>>> +++ b/xen/drivers/vpci/sriov.c
>>>> @@ -0,0 +1,235 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>> +/*
>>>> + * Handlers for accesses to the SR-IOV capability structure.
>>>> + *
>>>> + * Copyright (C) 2018 Citrix Systems R&D
>>>
>>> If there's a Citrix copyright header here, shouldn't there be a
>>> matching Signed-off-by from someone at Citrix (I think that's likely
>>> me)?
>>>
>>> Otherwise if there's no content authored by a Citrix employee the
>>> copyright notice must be removed.  We need to be careful with
>>> copyright and attribution.
>>>
>>> And in any case the date should be updated.
>>>
>>
>> Can I add your SOB or is it better to remove the copyright? Looking at 
>> the patches you provided, I think this ones were definitely based on 
>> them, but there are also a lot of changes since then.
> 
> If it's based on the patches that I sent many years ago (2018),
> then yes, you likely need to add my SoB.  Look like that way from the
> copyright notice.
> 
> Thanks, Roger.

A bit of context here: When I worked on this, I used Roger's sriov.c from [1] as
a starting point. Unfortunately, during my haste of development, I neglected to
preserve authorship/Signed-off-by, and didn't get back to addressing it before I
shared the branch with Mykyta. So yes, at minimum Roger's S-o-b should be
re-added (likely as the first S-o-b), and potentially even make Roger author.

[1] 
https://lore.kernel.org/xen-devel/20180717094830.54806-12-roger.pau@xxxxxxxxxx/