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

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

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.