On Thu, Mar 23, 2017 at 03:53:42PM -0500, Bjorn Helgaas wrote: > Hi Yongji, > > On Wed, Feb 15, 2017 at 02:45:05PM +0800, Yongji Xie wrote: >> When vfio passthroughs a PCI device of which MMIO BARs are >> smaller than PAGE_SIZE, guest will not handle the mmio >> accesses to the BARs which leads to mmio emulations in host. >> >> This is because vfio will not allow to passthrough one BAR's >> mmio page which may be shared with other BARs. Otherwise, >> there will be a backdoor that guest can use to access BARs >> of other guest. > > Please include a pointer to the VFIO code that enforces this. It's > not obvious to me how it would do this. This doesn't change the > *size* of the resource, only the alignment. So if VFIO sees a BAR > like [mem 0x80000000-0x800000ff], it knows the BAR is aligned enough > that it *could* be the only thing on a page, but I don't know how it > could know that there will never be another BAR at 0x80000100. Even > if there's no other BAR on that page *now*, it would have to know that > no hot-added device will ever be placed on that page. > >> This patch adds a macro to set default alignment for all >> PCI devices. Then we could solve this issue on some platforms >> which would easily hit this issue because of their 64K page >> such as PowerNV platform by defining this macro as PAGE_SIZE. >> >> Signed-off-by: Yongji Xie <xyjxie@xxxxxxxxxxxxxxxxxx> >> --- >> arch/powerpc/include/asm/pci.h | 4 ++++ >> drivers/pci/pci.c | 3 +++ >> 2 files changed, 7 insertions(+), 0 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h >> index e9bd6cf..5e31bc2 100644 >> --- a/arch/powerpc/include/asm/pci.h >> +++ b/arch/powerpc/include/asm/pci.h >> @@ -28,6 +28,10 @@ >> #define PCIBIOS_MIN_IO 0x1000 >> #define PCIBIOS_MIN_MEM 0x10000000 >> >> +#ifdef CONFIG_PPC_POWERNV >> +#define PCIBIOS_DEFAULT_ALIGNMENT PAGE_SIZE >> +#endif >> + >> struct pci_dev; >> >> /* Values for the `which' argument to sys_pciconfig_iobase syscall. */ >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >> index a881c0d..2622e9b 100644 >> --- a/drivers/pci/pci.c >> +++ b/drivers/pci/pci.c >> @@ -4965,6 +4965,9 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev) >> resource_size_t align = 0; >> char *p; >> >> +#ifdef PCIBIOS_DEFAULT_ALIGNMENT >> + align = PCIBIOS_DEFAULT_ALIGNMENT; >> +#endif > > I'd prefer to move this #ifdef out of the code path, as in the patch > below. > > I don't know how powerpc kernels are built: can you build a kernel > that will run on PowerNV as well as on different powerpc systems? If > so, is it acceptable to force that kernel to user 64K alignment even > when it's running on non-PowerNV systems? Or do you need a function > call here instead of a #define? > That's a good point, the macro may be also defined on non-PowerNV systems. Maybe an arch-specific function is more reasonable here. Thanks, Yongji
