On Thu, 23 Jul 2020, Rahul Singh wrote: > The existing VPCI support available for X86 is adapted for Arm. > When the device is added to XEN via the hyper call > “PHYSDEVOP_pci_device_add”, VPCI handler for the config space > access is added to the PCI device to emulate the PCI devices. > > A MMIO trap handler for the PCI ECAM space is registered in XEN > so that when guest is trying to access the PCI config space,XEN > will trap the access and emulate read/write using the VPCI and > not the real PCI hardware. > > VPCI MSI support is disable for ARM as it is not tested on ARM. > > Change-Id: I5501db2781f8064640403fecce53713091cd9ab4 Same question > Signed-off-by: Rahul Singh > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/domain.c | 4 ++ > xen/arch/arm/vpci.c | 102 ++++++++++++++++++++++++++++++++++ > xen/arch/arm/vpci.h | 37 ++++++++++++ > xen/drivers/passthrough/pci.c | 7 +++ > xen/include/asm-arm/domain.h | 5 ++ > xen/include/public/arch-arm.h | 4 ++ > 7 files changed, 160 insertions(+) > create mode 100644 xen/arch/arm/vpci.c > create mode 100644 xen/arch/arm/vpci.h > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 345cb83eed..5a23ec5cc0 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -7,6 +7,7 @@ obj-y += platforms/ > endif > obj-$(CONFIG_TEE) += tee/ > obj-$(CONFIG_ARM_PCI) += pci/ > +obj-$(CONFIG_HAS_VPCI) += vpci.o > > obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o > obj-y += bootfdt.init.o > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 31169326b2..23098ffd02 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -39,6 +39,7 @@ > #include > > #include "vuart.h" > +#include "vpci.h" > > DEFINE_PER_CPU(struct vcpu *, curr_vcpu); > > @@ -747,6 +748,9 @@ int arch_domain_create(struct domain *d, > if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) ) > goto fail; > > + if ( (rc = domain_vpci_init(d)) != 0 ) > + goto fail; > + > return 0; > > fail: > diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c > new file mode 100644 > index 0000000000..49e473ab0d > --- /dev/null > +++ b/xen/arch/arm/vpci.c > @@ -0,0 +1,102 @@ > +/* > + * xen/arch/arm/vpci.c > + * Copyright (c) 2020 Arm Ltd. > + * > + * Based on arch/x86/hvm/io.c > + * Copyright (c) 2004, Intel Corporation. > + * Copyright (c) 2005, International Business Machines Corporation. > + * Copyright (c) 2008, Citrix Systems, Inc. > + * > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > +#include > +#include > + > +/* Do some sanity checks. */ > +static bool vpci_mmio_access_allowed(unsigned int reg, unsigned int len) > +{ > + /* Check access size. */ > + if ( len != 1 && len != 2 && len != 4 && len != 8 ) > + return false; > + > + /* Check that access is size aligned. */ > + if ( (reg & (len - 1)) ) > + return false; > + > + return true; > +} > + > +static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info, > + register_t *r, void *priv) > +{ > + unsigned int reg; > + pci_sbdf_t sbdf; > + uint32_t data = 0; > + unsigned int size = 1U << info->dabt.size; > + > + sbdf.bdf = (((info->gpa) & 0x0ffff000) >> 12); > + reg = (((info->gpa) & 0x00000ffc) | (info->gpa & 3)); > + > + if ( !vpci_mmio_access_allowed(reg, size) ) > + return 1; > + > + data = vpci_read(sbdf, reg, size); > + > + memcpy(r, &data, size); > + > + return 1; > +} > + > +static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info, > + register_t r, void *priv) > +{ > + unsigned int reg; > + pci_sbdf_t sbdf; > + uint32_t data = r; > + unsigned int size = 1U << info->dabt.size; > + > + sbdf.bdf = (((info->gpa) & 0x0ffff000) >> 12); > + reg = (((info->gpa) & 0x00000ffc) | (info->gpa & 3)); > + > + if ( !vpci_mmio_access_allowed(reg, size) ) > + return 1; > + > + vpci_write(sbdf, reg, size, data); > + > + return 1; > +} I wonder if we could share vpci_mmcfg_read/write. Again, it is OK if we can't, or if it is not worth the effort. just want to make sure we thought about it :-) > +static const struct mmio_handler_ops vpci_mmio_handler = { > + .read = vpci_mmio_read, > + .write = vpci_mmio_write, > +}; > + > +int domain_vpci_init(struct domain *d) > +{ > + if ( !has_vpci(d) || is_hardware_domain(d) ) > + return 0; > + > + register_mmio_handler(d, &vpci_mmio_handler, > + GUEST_VPCI_ECAM_BASE,GUEST_VPCI_ECAM_SIZE,NULL); > + > + return 0; > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > + > diff --git a/xen/arch/arm/vpci.h b/xen/arch/arm/vpci.h > new file mode 100644 > index 0000000000..20dce1f4c4 > --- /dev/null > +++ b/xen/arch/arm/vpci.h > @@ -0,0 +1,37 @@ > +/* > + * xen/arch/arm/vpci.h > + * Copyright (c) 2020 Arm Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#ifndef __ARCH_ARM_VPCI_H__ > +#define __ARCH_ARM_VPCI_H__ > + > +#ifdef CONFIG_HAS_VPCI > +int domain_vpci_init(struct domain *d); > +#else > +static inline int domain_vpci_init(struct domain *d) > +{ > + return 0; > +} > +#endif > + > +#endif /* __ARCH_ARM_VPCI_H__ */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c > index 5846978890..28511eb641 100644 > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -804,6 +804,13 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn, > else > iommu_enable_device(pdev); > > +#ifdef CONFIG_ARM > + ret = vpci_add_handlers(pdev); > + if ( ret ) { > + printk(XENLOG_ERR "setup of vPCI for failed: %d\n",ret); > + goto out; > + } > +#endif > pci_enable_acs(pdev); > > out: > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index 4e2f582006..ad70610226 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -34,6 +34,11 @@ enum domain_type { > /* The hardware domain has always its memory direct mapped. */ > #define is_domain_direct_mapped(d) ((d) == hardware_domain) > > +/* For X86 VPCI is enabled and tested for PVH DOM0 only but > + * for ARM we enable support VPCI for guest domain also. > + */ > +#define has_vpci(d) (true) As mentioned we could make this configurabled based on the presence of pci=[] or something similar in device tree for dom0less guests. > struct vtimer { > struct vcpu *v; > int irq; > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index c365b1b39e..7364a07362 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -422,6 +422,10 @@ typedef uint64_t xen_callback_t; > #define GUEST_PL011_BASE xen_mk_ullong(0x22000000) > #define GUEST_PL011_SIZE xen_mk_ullong(0x00001000) > > +/* VPCI ECAM mappings */ > +#define GUEST_VPCI_ECAM_BASE xen_mk_ullong(0x10000000) > +#define GUEST_VPCI_ECAM_SIZE xen_mk_ullong(0x10000000) Is 256MB in size part of the ECAM standard? > /* > * 16MB == 4096 pages reserved for guest to use as a region to map its > * grant table in. > -- > 2.17.1 >