From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
In order for vPCI to work it needs to maintain guest and hardware
domain's views on the configuration space. For example, BARs and
COMMAND registers require emulation for guests and the guest view
on the registers needs to be in sync with the real contents of the
relevant registers. For that ECAM address space needs to also be
trapped for the hardware domain, so we need to implement PCI host
bridge specific callbacks to properly setup MMIO handlers for those
ranges depending on particular host bridge implementation.
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
Since v1:
- Dynamically calculate the number of MMIO handlers required for vPCI
and update the total number accordingly
- s/clb/cb
- Do not introduce a new callback for MMIO handler setup
---
xen/arch/arm/domain.c | 2 ++
xen/arch/arm/pci/pci-host-common.c | 28 +++++++++++++++++++++++++
xen/arch/arm/vpci.c | 33 ++++++++++++++++++++++++++++++
xen/arch/arm/vpci.h | 6 ++++++
xen/include/asm-arm/pci.h | 7 +++++++
5 files changed, 76 insertions(+)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 854e8fed0393..c7b25bc70439 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -733,6 +733,8 @@ int arch_domain_create(struct domain *d,
if ( (rc = domain_vgic_register(d, &count)) != 0 )
goto fail;
+ count += domain_vpci_get_num_mmio_handlers(d);
+
if ( (rc = domain_io_init(d, count + MAX_IO_HANDLER)) != 0 )
goto fail;
diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c
index 1567b6e2956c..155f2a2743af 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -300,6 +300,34 @@ struct dt_device_node *pci_find_host_bridge_node(struct device *dev)
}
return bridge->dt_node;
}
+
+int pci_host_iterate_bridges(struct domain *d,
+ int (*cb)(struct domain *d,
+ struct pci_host_bridge *bridge))
+{
+ struct pci_host_bridge *bridge;
+ int err;
+
+ list_for_each_entry( bridge, &pci_host_bridges, node )
+ {
+ err = cb(d, bridge);
+ if ( err )
+ return err;
+ }
+ return 0;
+}
+
+int pci_host_get_num_bridges(void)
+{
+ struct pci_host_bridge *bridge;
+ int count = 0;
+
+ list_for_each_entry( bridge, &pci_host_bridges, node )
+ count++;
+
+ return count;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 76c12b92814f..14947e975d69 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -80,17 +80,50 @@ static const struct mmio_handler_ops vpci_mmio_handler = {
.write = vpci_mmio_write,
};
+static int vpci_setup_mmio_handler(struct domain *d,
+ struct pci_host_bridge *bridge)
+{
+ struct pci_config_window *cfg = bridge->cfg;
+
+ register_mmio_handler(d, &vpci_mmio_handler,
+ cfg->phys_addr, cfg->size, NULL);
+ return 0;
+}
+
int domain_vpci_init(struct domain *d)
{
if ( !has_vpci(d) )
return 0;
+ if ( is_hardware_domain(d) )
+ return pci_host_iterate_bridges(d, vpci_setup_mmio_handler);
+
+ /* Guest domains use what is programmed in their device tree. */
register_mmio_handler(d, &vpci_mmio_handler,
GUEST_VPCI_ECAM_BASE, GUEST_VPCI_ECAM_SIZE, NULL);
return 0;
}
+int domain_vpci_get_num_mmio_handlers(struct domain *d)
+{
+ int count = 0;
+
+ if ( is_hardware_domain(d) )
+ /* For each PCI host bridge's configuration space. */
+ count += pci_host_get_num_bridges();
+ else
+ /*
+ * VPCI_MSIX_MEM_NUM handlers for MSI-X tables per each PCI device
+ * being passed through. Maximum number of supported devices
+ * is 32 as virtual bus topology emulates the devices as embedded
+ * endpoints.
+ * +1 for a single emulated host bridge's configuration space. */
+ count = VPCI_MSIX_MEM_NUM * 32 + 1;
+
+ return count;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/vpci.h b/xen/arch/arm/vpci.h
index d8a7b0e3e802..27a2b069abd2 100644
--- a/xen/arch/arm/vpci.h
+++ b/xen/arch/arm/vpci.h
@@ -17,11 +17,17 @@
#ifdef CONFIG_HAS_VPCI
int domain_vpci_init(struct domain *d);
+int domain_vpci_get_num_mmio_handlers(struct domain *d);
#else
static inline int domain_vpci_init(struct domain *d)
{
return 0;
}
+
+static inline int domain_vpci_get_num_mmio_handlers(struct domain *d)
+{
+ return 0;
+}
#endif
#endif /* __ARCH_ARM_VPCI_H__ */
diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
index 5b100556225e..7618f0b6725b 100644
--- a/xen/include/asm-arm/pci.h
+++ b/xen/include/asm-arm/pci.h
@@ -15,6 +15,8 @@
#ifndef __ARM_PCI_H__
#define __ARM_PCI_H__
+#include <asm/mmio.h>
+
#ifdef CONFIG_HAS_PCI
#define pci_to_dev(pcidev) (&(pcidev)->arch.dev)
@@ -109,6 +111,11 @@ static always_inline bool is_pci_passthrough_enabled(void)
{
return !!pci_passthrough_enabled;
}
+
+int pci_host_iterate_bridges(struct domain *d,
+ int (*clb)(struct domain *d,
+ struct pci_host_bridge *bridge));
+int pci_host_get_num_bridges(void);
#else /*!CONFIG_HAS_PCI*/
#define pci_passthrough_enabled (false)
--
2.25.1
On Thu, 23 Sep 2021, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > In order for vPCI to work it needs to maintain guest and hardware > domain's views on the configuration space. For example, BARs and ^ of > COMMAND registers require emulation for guests and the guest view > on the registers needs to be in sync with the real contents of the ^ of > relevant registers. For that ECAM address space needs to also be > trapped for the hardware domain, so we need to implement PCI host > bridge specific callbacks to properly setup MMIO handlers for those > ranges depending on particular host bridge implementation. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> The patch looks pretty good, only a couple of minor comments below > --- > Since v1: > - Dynamically calculate the number of MMIO handlers required for vPCI > and update the total number accordingly > - s/clb/cb > - Do not introduce a new callback for MMIO handler setup > --- > xen/arch/arm/domain.c | 2 ++ > xen/arch/arm/pci/pci-host-common.c | 28 +++++++++++++++++++++++++ > xen/arch/arm/vpci.c | 33 ++++++++++++++++++++++++++++++ > xen/arch/arm/vpci.h | 6 ++++++ > xen/include/asm-arm/pci.h | 7 +++++++ > 5 files changed, 76 insertions(+) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 854e8fed0393..c7b25bc70439 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -733,6 +733,8 @@ int arch_domain_create(struct domain *d, > if ( (rc = domain_vgic_register(d, &count)) != 0 ) > goto fail; > > + count += domain_vpci_get_num_mmio_handlers(d); > + > if ( (rc = domain_io_init(d, count + MAX_IO_HANDLER)) != 0 ) > goto fail; > > diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c > index 1567b6e2956c..155f2a2743af 100644 > --- a/xen/arch/arm/pci/pci-host-common.c > +++ b/xen/arch/arm/pci/pci-host-common.c > @@ -300,6 +300,34 @@ struct dt_device_node *pci_find_host_bridge_node(struct device *dev) > } > return bridge->dt_node; > } > + > +int pci_host_iterate_bridges(struct domain *d, > + int (*cb)(struct domain *d, > + struct pci_host_bridge *bridge)) > +{ > + struct pci_host_bridge *bridge; > + int err; > + > + list_for_each_entry( bridge, &pci_host_bridges, node ) > + { > + err = cb(d, bridge); > + if ( err ) > + return err; > + } > + return 0; > +} > + > +int pci_host_get_num_bridges(void) > +{ > + struct pci_host_bridge *bridge; > + int count = 0; > + > + list_for_each_entry( bridge, &pci_host_bridges, node ) > + count++; > + > + return count; > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c > index 76c12b92814f..14947e975d69 100644 > --- a/xen/arch/arm/vpci.c > +++ b/xen/arch/arm/vpci.c > @@ -80,17 +80,50 @@ static const struct mmio_handler_ops vpci_mmio_handler = { > .write = vpci_mmio_write, > }; > > +static int vpci_setup_mmio_handler(struct domain *d, > + struct pci_host_bridge *bridge) > +{ > + struct pci_config_window *cfg = bridge->cfg; > + > + register_mmio_handler(d, &vpci_mmio_handler, > + cfg->phys_addr, cfg->size, NULL); > + return 0; > +} > + > int domain_vpci_init(struct domain *d) > { > if ( !has_vpci(d) ) > return 0; > > + if ( is_hardware_domain(d) ) > + return pci_host_iterate_bridges(d, vpci_setup_mmio_handler); > + > + /* Guest domains use what is programmed in their device tree. */ > register_mmio_handler(d, &vpci_mmio_handler, > GUEST_VPCI_ECAM_BASE, GUEST_VPCI_ECAM_SIZE, NULL); > > return 0; > } > > +int domain_vpci_get_num_mmio_handlers(struct domain *d) > +{ > + int count = 0; > + > + if ( is_hardware_domain(d) ) > + /* For each PCI host bridge's configuration space. */ > + count += pci_host_get_num_bridges(); NIT: why += instead of = ? > + else > + /* > + * VPCI_MSIX_MEM_NUM handlers for MSI-X tables per each PCI device > + * being passed through. Maximum number of supported devices > + * is 32 as virtual bus topology emulates the devices as embedded > + * endpoints. > + * +1 for a single emulated host bridge's configuration space. */ > + count = VPCI_MSIX_MEM_NUM * 32 + 1; > + > + return count; > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/arm/vpci.h b/xen/arch/arm/vpci.h > index d8a7b0e3e802..27a2b069abd2 100644 > --- a/xen/arch/arm/vpci.h > +++ b/xen/arch/arm/vpci.h > @@ -17,11 +17,17 @@ > > #ifdef CONFIG_HAS_VPCI > int domain_vpci_init(struct domain *d); > +int domain_vpci_get_num_mmio_handlers(struct domain *d); > #else > static inline int domain_vpci_init(struct domain *d) > { > return 0; > } > + > +static inline int domain_vpci_get_num_mmio_handlers(struct domain *d) > +{ > + return 0; > +} > #endif > > #endif /* __ARCH_ARM_VPCI_H__ */ > diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h > index 5b100556225e..7618f0b6725b 100644 > --- a/xen/include/asm-arm/pci.h > +++ b/xen/include/asm-arm/pci.h > @@ -15,6 +15,8 @@ > #ifndef __ARM_PCI_H__ > #define __ARM_PCI_H__ > > +#include <asm/mmio.h> This should be no longer required. > #ifdef CONFIG_HAS_PCI > > #define pci_to_dev(pcidev) (&(pcidev)->arch.dev) > @@ -109,6 +111,11 @@ static always_inline bool is_pci_passthrough_enabled(void) > { > return !!pci_passthrough_enabled; > } > + > +int pci_host_iterate_bridges(struct domain *d, > + int (*clb)(struct domain *d, > + struct pci_host_bridge *bridge)); > +int pci_host_get_num_bridges(void); > #else /*!CONFIG_HAS_PCI*/ > > #define pci_passthrough_enabled (false)
On 25.09.21 03:18, Stefano Stabellini wrote: > On Thu, 23 Sep 2021, Oleksandr Andrushchenko wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> In order for vPCI to work it needs to maintain guest and hardware >> domain's views on the configuration space. For example, BARs and > ^ of > Ack >> COMMAND registers require emulation for guests and the guest view >> on the registers needs to be in sync with the real contents of the > ^ of Ack > >> relevant registers. For that ECAM address space needs to also be >> trapped for the hardware domain, so we need to implement PCI host >> bridge specific callbacks to properly setup MMIO handlers for those >> ranges depending on particular host bridge implementation. >> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > The patch looks pretty good, only a couple of minor comments below > > >> --- >> Since v1: >> - Dynamically calculate the number of MMIO handlers required for vPCI >> and update the total number accordingly >> - s/clb/cb >> - Do not introduce a new callback for MMIO handler setup >> --- >> xen/arch/arm/domain.c | 2 ++ >> xen/arch/arm/pci/pci-host-common.c | 28 +++++++++++++++++++++++++ >> xen/arch/arm/vpci.c | 33 ++++++++++++++++++++++++++++++ >> xen/arch/arm/vpci.h | 6 ++++++ >> xen/include/asm-arm/pci.h | 7 +++++++ >> 5 files changed, 76 insertions(+) >> >> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c >> index 854e8fed0393..c7b25bc70439 100644 >> --- a/xen/arch/arm/domain.c >> +++ b/xen/arch/arm/domain.c >> @@ -733,6 +733,8 @@ int arch_domain_create(struct domain *d, >> if ( (rc = domain_vgic_register(d, &count)) != 0 ) >> goto fail; >> >> + count += domain_vpci_get_num_mmio_handlers(d); >> + >> if ( (rc = domain_io_init(d, count + MAX_IO_HANDLER)) != 0 ) >> goto fail; >> >> diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c >> index 1567b6e2956c..155f2a2743af 100644 >> --- a/xen/arch/arm/pci/pci-host-common.c >> +++ b/xen/arch/arm/pci/pci-host-common.c >> @@ -300,6 +300,34 @@ struct dt_device_node *pci_find_host_bridge_node(struct device *dev) >> } >> return bridge->dt_node; >> } >> + >> +int pci_host_iterate_bridges(struct domain *d, >> + int (*cb)(struct domain *d, >> + struct pci_host_bridge *bridge)) >> +{ >> + struct pci_host_bridge *bridge; >> + int err; >> + >> + list_for_each_entry( bridge, &pci_host_bridges, node ) >> + { >> + err = cb(d, bridge); >> + if ( err ) >> + return err; >> + } >> + return 0; >> +} >> + >> +int pci_host_get_num_bridges(void) >> +{ >> + struct pci_host_bridge *bridge; >> + int count = 0; >> + >> + list_for_each_entry( bridge, &pci_host_bridges, node ) >> + count++; >> + >> + return count; >> +} >> + >> /* >> * Local variables: >> * mode: C >> diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c >> index 76c12b92814f..14947e975d69 100644 >> --- a/xen/arch/arm/vpci.c >> +++ b/xen/arch/arm/vpci.c >> @@ -80,17 +80,50 @@ static const struct mmio_handler_ops vpci_mmio_handler = { >> .write = vpci_mmio_write, >> }; >> >> +static int vpci_setup_mmio_handler(struct domain *d, >> + struct pci_host_bridge *bridge) >> +{ >> + struct pci_config_window *cfg = bridge->cfg; >> + >> + register_mmio_handler(d, &vpci_mmio_handler, >> + cfg->phys_addr, cfg->size, NULL); >> + return 0; >> +} >> + >> int domain_vpci_init(struct domain *d) >> { >> if ( !has_vpci(d) ) >> return 0; >> >> + if ( is_hardware_domain(d) ) >> + return pci_host_iterate_bridges(d, vpci_setup_mmio_handler); >> + >> + /* Guest domains use what is programmed in their device tree. */ >> register_mmio_handler(d, &vpci_mmio_handler, >> GUEST_VPCI_ECAM_BASE, GUEST_VPCI_ECAM_SIZE, NULL); >> >> return 0; >> } >> >> +int domain_vpci_get_num_mmio_handlers(struct domain *d) >> +{ >> + int count = 0; >> + >> + if ( is_hardware_domain(d) ) >> + /* For each PCI host bridge's configuration space. */ >> + count += pci_host_get_num_bridges(); > NIT: why += instead of = ? Will fix > > >> + else >> + /* >> + * VPCI_MSIX_MEM_NUM handlers for MSI-X tables per each PCI device >> + * being passed through. Maximum number of supported devices >> + * is 32 as virtual bus topology emulates the devices as embedded >> + * endpoints. >> + * +1 for a single emulated host bridge's configuration space. */ >> + count = VPCI_MSIX_MEM_NUM * 32 + 1; >> + >> + return count; >> +} >> + >> /* >> * Local variables: >> * mode: C >> diff --git a/xen/arch/arm/vpci.h b/xen/arch/arm/vpci.h >> index d8a7b0e3e802..27a2b069abd2 100644 >> --- a/xen/arch/arm/vpci.h >> +++ b/xen/arch/arm/vpci.h >> @@ -17,11 +17,17 @@ >> >> #ifdef CONFIG_HAS_VPCI >> int domain_vpci_init(struct domain *d); >> +int domain_vpci_get_num_mmio_handlers(struct domain *d); >> #else >> static inline int domain_vpci_init(struct domain *d) >> { >> return 0; >> } >> + >> +static inline int domain_vpci_get_num_mmio_handlers(struct domain *d) >> +{ >> + return 0; >> +} >> #endif >> >> #endif /* __ARCH_ARM_VPCI_H__ */ >> diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h >> index 5b100556225e..7618f0b6725b 100644 >> --- a/xen/include/asm-arm/pci.h >> +++ b/xen/include/asm-arm/pci.h >> @@ -15,6 +15,8 @@ >> #ifndef __ARM_PCI_H__ >> #define __ARM_PCI_H__ >> >> +#include <asm/mmio.h> > This should be no longer required. I'll check and remove > > >> #ifdef CONFIG_HAS_PCI >> >> #define pci_to_dev(pcidev) (&(pcidev)->arch.dev) >> @@ -109,6 +111,11 @@ static always_inline bool is_pci_passthrough_enabled(void) >> { >> return !!pci_passthrough_enabled; >> } >> + >> +int pci_host_iterate_bridges(struct domain *d, >> + int (*clb)(struct domain *d, >> + struct pci_host_bridge *bridge)); >> +int pci_host_get_num_bridges(void); >> #else /*!CONFIG_HAS_PCI*/ >> >> #define pci_passthrough_enabled (false) Thank you, Oleksandr
© 2016 - 2024 Red Hat, Inc.