From: Stefano Stabellini <stefano.stabellini@amd.com>
has_vpci_bridge is a macro to check if the domain is a domU or is dom0
with vPCI (pci-scan=yes) enabled.
Use the macro in drivers/vpci.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
---
xen/drivers/vpci/header.c | 14 +++++++-------
xen/drivers/vpci/vpci.c | 4 ++--
xen/include/xen/vpci.h | 9 +++++++++
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 469f497744..903168ff96 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -230,7 +230,7 @@ bool vpci_process_pending(struct vcpu *v)
read_unlock(&v->domain->pci_lock);
- if ( !is_hardware_domain(v->domain) )
+ if ( has_vpci_bridge(v->domain) )
domain_crash(v->domain);
return false;
@@ -492,7 +492,7 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool rom_only)
}
}
- if ( !is_hardware_domain(d) )
+ if ( has_vpci_bridge(d) )
break;
d = dom_xen;
@@ -522,7 +522,7 @@ static void cf_check cmd_write(
{
struct vpci_header *header = data;
- if ( !is_hardware_domain(pdev->domain) )
+ if ( has_vpci_bridge(pdev->domain) )
{
const struct vpci *vpci = pdev->vpci;
@@ -564,7 +564,7 @@ static void cf_check bar_write(
struct vpci_bar *bar = data;
bool hi = false;
- ASSERT(is_hardware_domain(pdev->domain));
+ ASSERT(!has_vpci_bridge(pdev->domain));
if ( bar->type == VPCI_BAR_MEM64_HI )
{
@@ -747,7 +747,7 @@ static int vpci_init_capability_list(struct pci_dev *pdev)
{
int rc;
bool mask_cap_list = false;
- bool is_hwdom = is_hardware_domain(pdev->domain);
+ bool is_hwdom = !has_vpci_bridge(pdev->domain);
if ( pci_conf_read16(pdev->sbdf, PCI_STATUS) & PCI_STATUS_CAP_LIST )
{
@@ -829,7 +829,7 @@ static int vpci_init_ext_capability_list(const struct pci_dev *pdev)
{
unsigned int pos = PCI_CFG_SPACE_SIZE;
- if ( !is_hardware_domain(pdev->domain) )
+ if ( has_vpci_bridge(pdev->domain) )
/* Extended capabilities read as zero, write ignore for DomU */
return vpci_add_register(pdev->vpci, vpci_read_val, NULL,
pos, 4, (void *)0);
@@ -866,7 +866,7 @@ int vpci_init_header(struct pci_dev *pdev)
struct vpci_header *header = &pdev->vpci->header;
struct vpci_bar *bars = header->bars;
int rc;
- bool is_hwdom = is_hardware_domain(pdev->domain);
+ bool is_hwdom = !has_vpci_bridge(pdev->domain);
ASSERT(rw_is_write_locked(&pdev->domain->pci_lock));
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 07c7071d0a..8ea89b9805 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -48,7 +48,7 @@ static int assign_virtual_sbdf(struct pci_dev *pdev)
ASSERT(rw_is_write_locked(&pdev->domain->pci_lock));
- if ( is_hardware_domain(d) )
+ if ( !has_vpci_bridge(d) )
return 0;
/*
@@ -429,7 +429,7 @@ static const struct pci_dev *translate_virtual_device(const struct domain *d,
#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
const struct pci_dev *pdev;
- ASSERT(!is_hardware_domain(d));
+ ASSERT(has_vpci_bridge(d));
ASSERT(rw_is_locked(&d->pci_lock));
for_each_pdev ( d, pdev )
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 9ae75d946a..e0aecfac72 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -339,6 +339,15 @@ static inline int __must_check vpci_reset_device(struct pci_dev *pdev)
return vpci_assign_device(pdev);
}
+#ifdef CONFIG_ARM
+#include <asm/pci.h>
+
+#define has_vpci_bridge(d) (!is_hardware_domain(d) || \
+ (is_hardware_domain(d) && pci_scan_enabled))
+#else
+#define has_vpci_bridge(d) (!is_hardware_domain(d))
+#endif
+
#endif
/*
--
2.34.1On 24.09.2025 09:59, Mykyta Poturai wrote:
> From: Stefano Stabellini <stefano.stabellini@amd.com>
>
> has_vpci_bridge is a macro to check if the domain is a domU or is dom0
> with vPCI (pci-scan=yes) enabled.
Hmm. Why would DomU-s, now and forever, not have (virtual) bridges? Wasn't them
gaining (virtual) bridges actually the longer-term plan?
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -230,7 +230,7 @@ bool vpci_process_pending(struct vcpu *v)
>
> read_unlock(&v->domain->pci_lock);
>
> - if ( !is_hardware_domain(v->domain) )
> + if ( has_vpci_bridge(v->domain) )
> domain_crash(v->domain);
>
> return false;
> @@ -492,7 +492,7 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool rom_only)
> }
> }
>
> - if ( !is_hardware_domain(d) )
> + if ( has_vpci_bridge(d) )
> break;
>
> d = dom_xen;
> @@ -522,7 +522,7 @@ static void cf_check cmd_write(
> {
> struct vpci_header *header = data;
>
> - if ( !is_hardware_domain(pdev->domain) )
> + if ( has_vpci_bridge(pdev->domain) )
> {
> const struct vpci *vpci = pdev->vpci;
>
> @@ -564,7 +564,7 @@ static void cf_check bar_write(
> struct vpci_bar *bar = data;
> bool hi = false;
>
> - ASSERT(is_hardware_domain(pdev->domain));
> + ASSERT(!has_vpci_bridge(pdev->domain));
>
> if ( bar->type == VPCI_BAR_MEM64_HI )
> {
> @@ -747,7 +747,7 @@ static int vpci_init_capability_list(struct pci_dev *pdev)
> {
> int rc;
> bool mask_cap_list = false;
> - bool is_hwdom = is_hardware_domain(pdev->domain);
> + bool is_hwdom = !has_vpci_bridge(pdev->domain);
>
> if ( pci_conf_read16(pdev->sbdf, PCI_STATUS) & PCI_STATUS_CAP_LIST )
> {
> @@ -829,7 +829,7 @@ static int vpci_init_ext_capability_list(const struct pci_dev *pdev)
> {
> unsigned int pos = PCI_CFG_SPACE_SIZE;
>
> - if ( !is_hardware_domain(pdev->domain) )
> + if ( has_vpci_bridge(pdev->domain) )
> /* Extended capabilities read as zero, write ignore for DomU */
> return vpci_add_register(pdev->vpci, vpci_read_val, NULL,
> pos, 4, (void *)0);
> @@ -866,7 +866,7 @@ int vpci_init_header(struct pci_dev *pdev)
> struct vpci_header *header = &pdev->vpci->header;
> struct vpci_bar *bars = header->bars;
> int rc;
> - bool is_hwdom = is_hardware_domain(pdev->domain);
> + bool is_hwdom = !has_vpci_bridge(pdev->domain);
>
> ASSERT(rw_is_write_locked(&pdev->domain->pci_lock));
For none of the changes (also further ones) it is clear (to me) why the
substitution is (logically) correct. For this last instance the variable
name also ends up wrong after the replacement.
> --- a/xen/include/xen/vpci.h
> +++ b/xen/include/xen/vpci.h
> @@ -339,6 +339,15 @@ static inline int __must_check vpci_reset_device(struct pci_dev *pdev)
> return vpci_assign_device(pdev);
> }
>
> +#ifdef CONFIG_ARM
> +#include <asm/pci.h>
> +
> +#define has_vpci_bridge(d) (!is_hardware_domain(d) || \
> + (is_hardware_domain(d) && pci_scan_enabled))
Nit: Off-by-1 indentation.
Jan
On 06.11.25 14:15, Jan Beulich wrote:> On 24.09.2025 09:59, Mykyta Poturai wrote: >> From: Stefano Stabellini <stefano.stabellini@amd.com> >> >> has_vpci_bridge is a macro to check if the domain is a domU or is dom0 >> with vPCI (pci-scan=yes) enabled. > > Hmm. Why would DomU-s, now and forever, not have (virtual) bridges? Wasn't them > gaining (virtual) bridges actually the longer-term plan? > Seems like there is a misunderstanding here. Indeed it is the plan for DomUs to have virtual bridges and this check is designed to differentiate two categories of domains. 1. All DomUs + Dom0 with the virtual bridge. 2. Dom0 with HW bridge. I will try to rephrase the commit message to be more clear. >> @@ -866,7 +866,7 @@ int vpci_init_header(struct pci_dev *pdev) >> struct vpci_header *header = &pdev->vpci->header; >> struct vpci_bar *bars = header->bars; >> int rc; >> - bool is_hwdom = is_hardware_domain(pdev->domain); >> + bool is_hwdom = !has_vpci_bridge(pdev->domain); >> >> ASSERT(rw_is_write_locked(&pdev->domain->pci_lock)); > > For none of the changes (also further ones) it is clear (to me) why the > substitution is (logically) correct. For this last instance the variable > name also ends up wrong after the replacement. >> Jan The general logic for this change is that before it we assumed that hwdom always uses HW bridge. Now it is not always true and hwdom can also use a virtual bridge, so it needs to be treated the same way as DomUs are. You also mentioned on a previous series > Here and perhaps everywhere else I wonder: Is this really an appropriately > named predicate for the purpose / context? Maybe you have some ideas of a better name? From what I came up with this seems like the best one. -- Mykyta
On 13.11.2025 10:49, Mykyta Poturai wrote: > On 06.11.25 14:15, Jan Beulich wrote:> On 24.09.2025 09:59, Mykyta > Poturai wrote: > >> From: Stefano Stabellini <stefano.stabellini@amd.com> > >> > >> has_vpci_bridge is a macro to check if the domain is a domU or is dom0 > >> with vPCI (pci-scan=yes) enabled. > > > > Hmm. Why would DomU-s, now and forever, not have (virtual) bridges? > Wasn't them > > gaining (virtual) bridges actually the longer-term plan? > > > > Seems like there is a misunderstanding here. Indeed it is the plan for > DomUs to have virtual bridges and this check is designed to > differentiate two categories of domains. > 1. All DomUs + Dom0 with the virtual bridge. > 2. Dom0 with HW bridge. > > I will try to rephrase the commit message to be more clear. > > >> @@ -866,7 +866,7 @@ int vpci_init_header(struct pci_dev *pdev) > >> struct vpci_header *header = &pdev->vpci->header; > >> struct vpci_bar *bars = header->bars; > >> int rc; > >> - bool is_hwdom = is_hardware_domain(pdev->domain); > >> + bool is_hwdom = !has_vpci_bridge(pdev->domain); > >> > >> ASSERT(rw_is_write_locked(&pdev->domain->pci_lock)); > > > > For none of the changes (also further ones) it is clear (to me) why the > > substitution is (logically) correct. For this last instance the variable > > name also ends up wrong after the replacement. > >> Jan > > The general logic for this change is that before it we assumed that > hwdom always uses HW bridge. Now it is not always true and hwdom can > also use a virtual bridge, so it needs to be treated the same way as > DomUs are. > > You also mentioned on a previous series > > > Here and perhaps everywhere else I wonder: Is this really an > appropriately > > named predicate for the purpose / context? > > Maybe you have some ideas of a better name? From what I came up with > this seems like the best one. How to name it depends on the (ultimate) purpose. If, as you say, it's intended to cover Dom0 and DomU uniformly when they have a virtual bridge, the name might be quite okay. Then, as you said you would do, better wording may be needed, variable names like the one mentioned above may need adjustment, and any code paths suddenly becoming usable for DomU-s need to be audited for this being a safe/secure thing to do (quite a few of the Dom0-only paths have been taking some liberties so far). Jan
© 2016 - 2026 Red Hat, Inc.