From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Assign SBDF to the PCI devices being passed through with bus 0.
The resulting topology is where PCIe devices reside on the bus 0 of the
root complex itself (embedded endpoints).
This implementation is limited to 32 devices which are allowed on
a single PCI bus.
Please note, that at the moment only function 0 of a multifunction
device can be passed through.
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
Since v4:
- moved and re-worked guest sbdf initializers
- s/set_bit/__set_bit
- s/clear_bit/__clear_bit
- minor comment fix s/Virtual/Guest/
- added VPCI_MAX_VIRT_DEV constant (PCI_SLOT(~0) + 1) which will be used
later for counting the number of MMIO handlers required for a guest
(Julien)
Since v3:
- make use of VPCI_INIT
- moved all new code to vpci.c which belongs to it
- changed open-coded 31 to PCI_SLOT(~0)
- added comments and code to reject multifunction devices with
functions other than 0
- updated comment about vpci_dev_next and made it unsigned int
- implement roll back in case of error while assigning/deassigning devices
- s/dom%pd/%pd
Since v2:
- remove casts that are (a) malformed and (b) unnecessary
- add new line for better readability
- remove CONFIG_HAS_VPCI_GUEST_SUPPORT ifdef's as the relevant vPCI
functions are now completely gated with this config
- gate common code with CONFIG_HAS_VPCI_GUEST_SUPPORT
New in v2
---
xen/drivers/vpci/vpci.c | 51 +++++++++++++++++++++++++++++++++++++++++
xen/include/xen/sched.h | 8 +++++++
xen/include/xen/vpci.h | 11 +++++++++
3 files changed, 70 insertions(+)
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 98b12a61be6f..c2fb4d4db233 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -114,6 +114,9 @@ int vpci_add_handlers(struct pci_dev *pdev)
spin_lock(&pdev->vpci_lock);
pdev->vpci = vpci;
INIT_LIST_HEAD(&pdev->vpci->handlers);
+#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
+ pdev->vpci->guest_sbdf.sbdf = ~0;
+#endif
header = &pdev->vpci->header;
for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
@@ -145,6 +148,53 @@ int vpci_add_handlers(struct pci_dev *pdev)
}
#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
+int vpci_add_virtual_device(struct pci_dev *pdev)
+{
+ struct domain *d = pdev->domain;
+ pci_sbdf_t sbdf = { 0 };
+ unsigned long new_dev_number;
+
+ /*
+ * Each PCI bus supports 32 devices/slots at max or up to 256 when
+ * there are multi-function ones which are not yet supported.
+ */
+ if ( pdev->info.is_extfn )
+ {
+ gdprintk(XENLOG_ERR, "%pp: only function 0 passthrough supported\n",
+ &pdev->sbdf);
+ return -EOPNOTSUPP;
+ }
+
+ new_dev_number = find_first_zero_bit(&d->vpci_dev_assigned_map,
+ VPCI_MAX_VIRT_DEV);
+ if ( new_dev_number >= VPCI_MAX_VIRT_DEV )
+ return -ENOSPC;
+
+ __set_bit(new_dev_number, &d->vpci_dev_assigned_map);
+
+ /*
+ * Both segment and bus number are 0:
+ * - we emulate a single host bridge for the guest, e.g. segment 0
+ * - with bus 0 the virtual devices are seen as embedded
+ * endpoints behind the root complex
+ *
+ * TODO: add support for multi-function devices.
+ */
+ sbdf.devfn = PCI_DEVFN(new_dev_number, 0);
+ pdev->vpci->guest_sbdf = sbdf;
+
+ return 0;
+
+}
+REGISTER_VPCI_INIT(vpci_add_virtual_device, VPCI_PRIORITY_MIDDLE);
+
+static void vpci_remove_virtual_device(struct domain *d,
+ const struct pci_dev *pdev)
+{
+ __clear_bit(pdev->vpci->guest_sbdf.dev, &d->vpci_dev_assigned_map);
+ pdev->vpci->guest_sbdf.sbdf = ~0;
+}
+
/* Notify vPCI that device is assigned to guest. */
int vpci_assign_device(struct domain *d, struct pci_dev *pdev)
{
@@ -171,6 +221,7 @@ int vpci_deassign_device(struct domain *d, struct pci_dev *pdev)
return 0;
spin_lock(&pdev->vpci_lock);
+ vpci_remove_virtual_device(d, pdev);
vpci_remove_device_handlers_locked(pdev);
spin_unlock(&pdev->vpci_lock);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 28146ee404e6..10bff103317c 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -444,6 +444,14 @@ struct domain
#ifdef CONFIG_HAS_PCI
struct list_head pdev_list;
+#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
+ /*
+ * The bitmap which shows which device numbers are already used by the
+ * virtual PCI bus topology and is used to assign a unique SBDF to the
+ * next passed through virtual PCI device.
+ */
+ unsigned long vpci_dev_assigned_map;
+#endif
#endif
#ifdef CONFIG_HAS_PASSTHROUGH
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 18319fc329f9..e5258bd7ce90 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -21,6 +21,13 @@ typedef int vpci_register_init_t(struct pci_dev *dev);
#define VPCI_ECAM_BDF(addr) (((addr) & 0x0ffff000) >> 12)
+/*
+ * Maximum number of devices supported by the virtual bus topology:
+ * each PCI bus supports 32 devices/slots at max or up to 256 when
+ * there are multi-function ones which are not yet supported.
+ */
+#define VPCI_MAX_VIRT_DEV (PCI_SLOT(~0) + 1)
+
#define REGISTER_VPCI_INIT(x, p) \
static vpci_register_init_t *const x##_entry \
__used_section(".data.vpci." p) = x
@@ -143,6 +150,10 @@ struct vpci {
struct vpci_arch_msix_entry arch;
} entries[];
} *msix;
+#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
+ /* Guest SBDF of the device. */
+ pci_sbdf_t guest_sbdf;
+#endif
#endif
};
--
2.25.1
On 25.11.2021 12:02, Oleksandr Andrushchenko wrote:
> @@ -145,6 +148,53 @@ int vpci_add_handlers(struct pci_dev *pdev)
> }
>
> #ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
> +int vpci_add_virtual_device(struct pci_dev *pdev)
> +{
> + struct domain *d = pdev->domain;
> + pci_sbdf_t sbdf = { 0 };
> + unsigned long new_dev_number;
> +
> + /*
> + * Each PCI bus supports 32 devices/slots at max or up to 256 when
> + * there are multi-function ones which are not yet supported.
> + */
> + if ( pdev->info.is_extfn )
> + {
> + gdprintk(XENLOG_ERR, "%pp: only function 0 passthrough supported\n",
> + &pdev->sbdf);
> + return -EOPNOTSUPP;
> + }
> +
> + new_dev_number = find_first_zero_bit(&d->vpci_dev_assigned_map,
> + VPCI_MAX_VIRT_DEV);
> + if ( new_dev_number >= VPCI_MAX_VIRT_DEV )
> + return -ENOSPC;
> +
> + __set_bit(new_dev_number, &d->vpci_dev_assigned_map);
> +
> + /*
> + * Both segment and bus number are 0:
> + * - we emulate a single host bridge for the guest, e.g. segment 0
> + * - with bus 0 the virtual devices are seen as embedded
> + * endpoints behind the root complex
> + *
> + * TODO: add support for multi-function devices.
> + */
> + sbdf.devfn = PCI_DEVFN(new_dev_number, 0);
> + pdev->vpci->guest_sbdf = sbdf;
> +
> + return 0;
> +
> +}
> +REGISTER_VPCI_INIT(vpci_add_virtual_device, VPCI_PRIORITY_MIDDLE);
Is this function guaranteed to always be invoked ahead of ...
> +static void vpci_remove_virtual_device(struct domain *d,
> + const struct pci_dev *pdev)
> +{
> + __clear_bit(pdev->vpci->guest_sbdf.dev, &d->vpci_dev_assigned_map);
> + pdev->vpci->guest_sbdf.sbdf = ~0;
> +}
... this one, even when considering error paths? Otherwise you may
wrongly clear bit 31 here afaict.
Jan
Hi, Jan!
On 12.01.22 17:39, Jan Beulich wrote:
> On 25.11.2021 12:02, Oleksandr Andrushchenko wrote:
>> @@ -145,6 +148,53 @@ int vpci_add_handlers(struct pci_dev *pdev)
>> }
>>
>> #ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
>> +int vpci_add_virtual_device(struct pci_dev *pdev)
>> +{
>> + struct domain *d = pdev->domain;
>> + pci_sbdf_t sbdf = { 0 };
>> + unsigned long new_dev_number;
>> +
>> + /*
>> + * Each PCI bus supports 32 devices/slots at max or up to 256 when
>> + * there are multi-function ones which are not yet supported.
>> + */
>> + if ( pdev->info.is_extfn )
>> + {
>> + gdprintk(XENLOG_ERR, "%pp: only function 0 passthrough supported\n",
>> + &pdev->sbdf);
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + new_dev_number = find_first_zero_bit(&d->vpci_dev_assigned_map,
>> + VPCI_MAX_VIRT_DEV);
>> + if ( new_dev_number >= VPCI_MAX_VIRT_DEV )
>> + return -ENOSPC;
>> +
>> + __set_bit(new_dev_number, &d->vpci_dev_assigned_map);
>> +
>> + /*
>> + * Both segment and bus number are 0:
>> + * - we emulate a single host bridge for the guest, e.g. segment 0
>> + * - with bus 0 the virtual devices are seen as embedded
>> + * endpoints behind the root complex
>> + *
>> + * TODO: add support for multi-function devices.
>> + */
>> + sbdf.devfn = PCI_DEVFN(new_dev_number, 0);
>> + pdev->vpci->guest_sbdf = sbdf;
>> +
>> + return 0;
>> +
>> +}
>> +REGISTER_VPCI_INIT(vpci_add_virtual_device, VPCI_PRIORITY_MIDDLE);
> Is this function guaranteed to always be invoked ahead of ...
>
>> +static void vpci_remove_virtual_device(struct domain *d,
>> + const struct pci_dev *pdev)
>> +{
>> + __clear_bit(pdev->vpci->guest_sbdf.dev, &d->vpci_dev_assigned_map);
>> + pdev->vpci->guest_sbdf.sbdf = ~0;
>> +}
> ... this one, even when considering error paths? Otherwise you may
> wrongly clear bit 31 here afaict.
According to Roger's comment I will not use REGISTER_VPCI_INIT
machinery for this.
>
> Jan
>
Thank you,
Oleksandr
On Thu, Nov 25, 2021 at 01:02:48PM +0200, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
> Assign SBDF to the PCI devices being passed through with bus 0.
> The resulting topology is where PCIe devices reside on the bus 0 of the
> root complex itself (embedded endpoints).
> This implementation is limited to 32 devices which are allowed on
> a single PCI bus.
>
> Please note, that at the moment only function 0 of a multifunction
> device can be passed through.
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
> Since v4:
> - moved and re-worked guest sbdf initializers
> - s/set_bit/__set_bit
> - s/clear_bit/__clear_bit
> - minor comment fix s/Virtual/Guest/
> - added VPCI_MAX_VIRT_DEV constant (PCI_SLOT(~0) + 1) which will be used
> later for counting the number of MMIO handlers required for a guest
> (Julien)
> Since v3:
> - make use of VPCI_INIT
> - moved all new code to vpci.c which belongs to it
> - changed open-coded 31 to PCI_SLOT(~0)
> - added comments and code to reject multifunction devices with
> functions other than 0
> - updated comment about vpci_dev_next and made it unsigned int
> - implement roll back in case of error while assigning/deassigning devices
> - s/dom%pd/%pd
> Since v2:
> - remove casts that are (a) malformed and (b) unnecessary
> - add new line for better readability
> - remove CONFIG_HAS_VPCI_GUEST_SUPPORT ifdef's as the relevant vPCI
> functions are now completely gated with this config
> - gate common code with CONFIG_HAS_VPCI_GUEST_SUPPORT
> New in v2
> ---
> xen/drivers/vpci/vpci.c | 51 +++++++++++++++++++++++++++++++++++++++++
> xen/include/xen/sched.h | 8 +++++++
> xen/include/xen/vpci.h | 11 +++++++++
> 3 files changed, 70 insertions(+)
>
> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> index 98b12a61be6f..c2fb4d4db233 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -114,6 +114,9 @@ int vpci_add_handlers(struct pci_dev *pdev)
> spin_lock(&pdev->vpci_lock);
> pdev->vpci = vpci;
> INIT_LIST_HEAD(&pdev->vpci->handlers);
> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
> + pdev->vpci->guest_sbdf.sbdf = ~0;
> +#endif
>
> header = &pdev->vpci->header;
> for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
> @@ -145,6 +148,53 @@ int vpci_add_handlers(struct pci_dev *pdev)
> }
>
> #ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
> +int vpci_add_virtual_device(struct pci_dev *pdev)
> +{
> + struct domain *d = pdev->domain;
> + pci_sbdf_t sbdf = { 0 };
> + unsigned long new_dev_number;
I think this needs to be limited to non-hardware domains?
Or else you will report failures for the hardware domain even if it's
not using the virtual topology at all.
> + /*
> + * Each PCI bus supports 32 devices/slots at max or up to 256 when
> + * there are multi-function ones which are not yet supported.
> + */
> + if ( pdev->info.is_extfn )
> + {
> + gdprintk(XENLOG_ERR, "%pp: only function 0 passthrough supported\n",
> + &pdev->sbdf);
> + return -EOPNOTSUPP;
> + }
> +
> + new_dev_number = find_first_zero_bit(&d->vpci_dev_assigned_map,
> + VPCI_MAX_VIRT_DEV);
> + if ( new_dev_number >= VPCI_MAX_VIRT_DEV )
> + return -ENOSPC;
> +
> + __set_bit(new_dev_number, &d->vpci_dev_assigned_map);
How is vpci_dev_assigned_map protected from concurrent accesses? Does
it rely on the pcidevs lock being held while accessing it?
If so it needs spelling out (and likely an assert added).
> + /*
> + * Both segment and bus number are 0:
> + * - we emulate a single host bridge for the guest, e.g. segment 0
> + * - with bus 0 the virtual devices are seen as embedded
> + * endpoints behind the root complex
> + *
> + * TODO: add support for multi-function devices.
> + */
> + sbdf.devfn = PCI_DEVFN(new_dev_number, 0);
> + pdev->vpci->guest_sbdf = sbdf;
> +
> + return 0;
> +
> +}
> +REGISTER_VPCI_INIT(vpci_add_virtual_device, VPCI_PRIORITY_MIDDLE);
I'm unsure this is the right place to do virtual SBDF assignment, my
plan was to use REGISTER_VPCI_INIT exclusively with PCI capabilities.
I think it would be better to do the virtual SBDF assignment from
vpci_assign_device.
> +
> +static void vpci_remove_virtual_device(struct domain *d,
> + const struct pci_dev *pdev)
> +{
> + __clear_bit(pdev->vpci->guest_sbdf.dev, &d->vpci_dev_assigned_map);
> + pdev->vpci->guest_sbdf.sbdf = ~0;
> +}
> +
> /* Notify vPCI that device is assigned to guest. */
> int vpci_assign_device(struct domain *d, struct pci_dev *pdev)
> {
> @@ -171,6 +221,7 @@ int vpci_deassign_device(struct domain *d, struct pci_dev *pdev)
> return 0;
>
> spin_lock(&pdev->vpci_lock);
> + vpci_remove_virtual_device(d, pdev);
> vpci_remove_device_handlers_locked(pdev);
> spin_unlock(&pdev->vpci_lock);
>
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 28146ee404e6..10bff103317c 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -444,6 +444,14 @@ struct domain
>
> #ifdef CONFIG_HAS_PCI
> struct list_head pdev_list;
> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
> + /*
> + * The bitmap which shows which device numbers are already used by the
> + * virtual PCI bus topology and is used to assign a unique SBDF to the
> + * next passed through virtual PCI device.
> + */
> + unsigned long vpci_dev_assigned_map;
Please use DECLARE_BITMAP with the maximum number of supported
devices as parameter.
> +#endif
> #endif
>
> #ifdef CONFIG_HAS_PASSTHROUGH
> diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
> index 18319fc329f9..e5258bd7ce90 100644
> --- a/xen/include/xen/vpci.h
> +++ b/xen/include/xen/vpci.h
> @@ -21,6 +21,13 @@ typedef int vpci_register_init_t(struct pci_dev *dev);
>
> #define VPCI_ECAM_BDF(addr) (((addr) & 0x0ffff000) >> 12)
>
> +/*
> + * Maximum number of devices supported by the virtual bus topology:
> + * each PCI bus supports 32 devices/slots at max or up to 256 when
> + * there are multi-function ones which are not yet supported.
> + */
> +#define VPCI_MAX_VIRT_DEV (PCI_SLOT(~0) + 1)
> +
> #define REGISTER_VPCI_INIT(x, p) \
> static vpci_register_init_t *const x##_entry \
> __used_section(".data.vpci." p) = x
> @@ -143,6 +150,10 @@ struct vpci {
> struct vpci_arch_msix_entry arch;
> } entries[];
> } *msix;
> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
> + /* Guest SBDF of the device. */
> + pci_sbdf_t guest_sbdf;
> +#endif
> #endif
> };
>
> --
> 2.25.1
>
Hi, Roger!
On 13.01.22 13:35, Roger Pau Monné wrote:
> On Thu, Nov 25, 2021 at 01:02:48PM +0200, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> Assign SBDF to the PCI devices being passed through with bus 0.
>> The resulting topology is where PCIe devices reside on the bus 0 of the
>> root complex itself (embedded endpoints).
>> This implementation is limited to 32 devices which are allowed on
>> a single PCI bus.
>>
>> Please note, that at the moment only function 0 of a multifunction
>> device can be passed through.
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> ---
>> Since v4:
>> - moved and re-worked guest sbdf initializers
>> - s/set_bit/__set_bit
>> - s/clear_bit/__clear_bit
>> - minor comment fix s/Virtual/Guest/
>> - added VPCI_MAX_VIRT_DEV constant (PCI_SLOT(~0) + 1) which will be used
>> later for counting the number of MMIO handlers required for a guest
>> (Julien)
>> Since v3:
>> - make use of VPCI_INIT
>> - moved all new code to vpci.c which belongs to it
>> - changed open-coded 31 to PCI_SLOT(~0)
>> - added comments and code to reject multifunction devices with
>> functions other than 0
>> - updated comment about vpci_dev_next and made it unsigned int
>> - implement roll back in case of error while assigning/deassigning devices
>> - s/dom%pd/%pd
>> Since v2:
>> - remove casts that are (a) malformed and (b) unnecessary
>> - add new line for better readability
>> - remove CONFIG_HAS_VPCI_GUEST_SUPPORT ifdef's as the relevant vPCI
>> functions are now completely gated with this config
>> - gate common code with CONFIG_HAS_VPCI_GUEST_SUPPORT
>> New in v2
>> ---
>> xen/drivers/vpci/vpci.c | 51 +++++++++++++++++++++++++++++++++++++++++
>> xen/include/xen/sched.h | 8 +++++++
>> xen/include/xen/vpci.h | 11 +++++++++
>> 3 files changed, 70 insertions(+)
>>
>> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
>> index 98b12a61be6f..c2fb4d4db233 100644
>> --- a/xen/drivers/vpci/vpci.c
>> +++ b/xen/drivers/vpci/vpci.c
>> @@ -114,6 +114,9 @@ int vpci_add_handlers(struct pci_dev *pdev)
>> spin_lock(&pdev->vpci_lock);
>> pdev->vpci = vpci;
>> INIT_LIST_HEAD(&pdev->vpci->handlers);
>> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
>> + pdev->vpci->guest_sbdf.sbdf = ~0;
>> +#endif
>>
>> header = &pdev->vpci->header;
>> for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
>> @@ -145,6 +148,53 @@ int vpci_add_handlers(struct pci_dev *pdev)
>> }
>>
>> #ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
>> +int vpci_add_virtual_device(struct pci_dev *pdev)
>> +{
>> + struct domain *d = pdev->domain;
>> + pci_sbdf_t sbdf = { 0 };
>> + unsigned long new_dev_number;
> I think this needs to be limited to non-hardware domains?
>
> Or else you will report failures for the hardware domain even if it's
> not using the virtual topology at all.
Yes, this wants an is_hardware_domain check
>
>> + /*
>> + * Each PCI bus supports 32 devices/slots at max or up to 256 when
>> + * there are multi-function ones which are not yet supported.
>> + */
>> + if ( pdev->info.is_extfn )
>> + {
>> + gdprintk(XENLOG_ERR, "%pp: only function 0 passthrough supported\n",
>> + &pdev->sbdf);
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + new_dev_number = find_first_zero_bit(&d->vpci_dev_assigned_map,
>> + VPCI_MAX_VIRT_DEV);
>> + if ( new_dev_number >= VPCI_MAX_VIRT_DEV )
>> + return -ENOSPC;
>> +
>> + __set_bit(new_dev_number, &d->vpci_dev_assigned_map);
> How is vpci_dev_assigned_map protected from concurrent accesses? Does
> it rely on the pcidevs lock being held while accessing it?
It does rely on pcidevs lock, I'll add an assert here
>
> If so it needs spelling out (and likely an assert added).
>
>> + /*
>> + * Both segment and bus number are 0:
>> + * - we emulate a single host bridge for the guest, e.g. segment 0
>> + * - with bus 0 the virtual devices are seen as embedded
>> + * endpoints behind the root complex
>> + *
>> + * TODO: add support for multi-function devices.
>> + */
>> + sbdf.devfn = PCI_DEVFN(new_dev_number, 0);
>> + pdev->vpci->guest_sbdf = sbdf;
>> +
>> + return 0;
>> +
>> +}
>> +REGISTER_VPCI_INIT(vpci_add_virtual_device, VPCI_PRIORITY_MIDDLE);
> I'm unsure this is the right place to do virtual SBDF assignment, my
> plan was to use REGISTER_VPCI_INIT exclusively with PCI capabilities.
>
> I think it would be better to do the virtual SBDF assignment from
> vpci_assign_device.
Ok, will do
>
>> +
>> +static void vpci_remove_virtual_device(struct domain *d,
>> + const struct pci_dev *pdev)
>> +{
>> + __clear_bit(pdev->vpci->guest_sbdf.dev, &d->vpci_dev_assigned_map);
>> + pdev->vpci->guest_sbdf.sbdf = ~0;
>> +}
>> +
>> /* Notify vPCI that device is assigned to guest. */
>> int vpci_assign_device(struct domain *d, struct pci_dev *pdev)
>> {
>> @@ -171,6 +221,7 @@ int vpci_deassign_device(struct domain *d, struct pci_dev *pdev)
>> return 0;
>>
>> spin_lock(&pdev->vpci_lock);
>> + vpci_remove_virtual_device(d, pdev);
>> vpci_remove_device_handlers_locked(pdev);
>> spin_unlock(&pdev->vpci_lock);
>>
>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>> index 28146ee404e6..10bff103317c 100644
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -444,6 +444,14 @@ struct domain
>>
>> #ifdef CONFIG_HAS_PCI
>> struct list_head pdev_list;
>> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
>> + /*
>> + * The bitmap which shows which device numbers are already used by the
>> + * virtual PCI bus topology and is used to assign a unique SBDF to the
>> + * next passed through virtual PCI device.
>> + */
>> + unsigned long vpci_dev_assigned_map;
> Please use DECLARE_BITMAP with the maximum number of supported
> devices as parameter.
Will use
>
>> +#endif
>> #endif
>>
>> #ifdef CONFIG_HAS_PASSTHROUGH
>> diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
>> index 18319fc329f9..e5258bd7ce90 100644
>> --- a/xen/include/xen/vpci.h
>> +++ b/xen/include/xen/vpci.h
>> @@ -21,6 +21,13 @@ typedef int vpci_register_init_t(struct pci_dev *dev);
>>
>> #define VPCI_ECAM_BDF(addr) (((addr) & 0x0ffff000) >> 12)
>>
>> +/*
>> + * Maximum number of devices supported by the virtual bus topology:
>> + * each PCI bus supports 32 devices/slots at max or up to 256 when
>> + * there are multi-function ones which are not yet supported.
>> + */
>> +#define VPCI_MAX_VIRT_DEV (PCI_SLOT(~0) + 1)
>> +
>> #define REGISTER_VPCI_INIT(x, p) \
>> static vpci_register_init_t *const x##_entry \
>> __used_section(".data.vpci." p) = x
>> @@ -143,6 +150,10 @@ struct vpci {
>> struct vpci_arch_msix_entry arch;
>> } entries[];
>> } *msix;
>> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
>> + /* Guest SBDF of the device. */
>> + pci_sbdf_t guest_sbdf;
>> +#endif
>> #endif
>> };
>>
>> --
>> 2.25.1
>>
Thank you,
Oleksandr
© 2016 - 2026 Red Hat, Inc.