On Tue, Jun 06, 2023 at 06:06:04PM +0800, bibo, mao wrote: > Huacai, > > Although I post this patch, I think this should be arch specified > rather than general problem. X86 has solved this problem, arm64 > with 64K page size is not popular. However LoongArch has this > problem, page size is 16K rather than 4K. It is the problem of > LoongArch system rather than generic issue. I think this is only related to the page size, not to any LoongArch-specific details. If that's the case, I don't think the change should be arch-specific. > There is such discussion before: > https://patchwork.kernel.org/project/linux-pci/patch/22400b8828ad44ddbccb876cc5ca3b11@xxxxxxxxxxxxxxxxxxxxxxx/#19319457 > > UEFI bios sets pci memory space 4K aligned, however Loongarch kernel rescans the pci > bus and reassigns pci memory resource. So it it strange like this, here is pci memory info on > my 7A2000 board. > root@user-pc:~# lspci -vvv | grep Region > Region 5: Memory at e003526e800 (32-bit, non-prefetchable) [size=1K] > Region 0: Memory at e003526ec00 (64-bit, non-prefetchable) [size=1K] I guess these BARs are from different devices, and the problem you're pointing out is that both BARs end up in the same 16K page (actually even the same 4K page): (gdb) p/x 0xe003526e800 >> 12 $1 = 0xe003526e (gdb) p/x 0xe003526ec00 >> 12 $2 = 0xe003526e I agree that's a potential issue because a user mmap of the first device also gets access to the BAR of the second device. But it doesn't seem to be a *new* or LoongArch-specific issue. So I *think* the point of this patch is to ensure that BARs of different devices never share a page. Why is that suddenly an issue for LoongArch? Is it only because you see more sharing with 16K pages than other arches do with 4K pages? > 在 2023/6/6 16:45, Bibo Mao 写道: > > Some PCI devices have only 4K memory space size, it is normal in general > > machines and aligned with page size. However some architectures which > > support different page size, default page size on LoongArch is 16K, and > > ARM64 supports page size varying from 4K to 64K. On machines where larger > > page size is use, memory space region of two different pci devices may be > > in one page. It is not safe with mmu protection, also VFIO pci device > > driver requires base address of pci memory space page aligned, so that it > > can be memory mapped to qemu user space when it is passed-through to vm. The minimum memory BAR per spec is 128 bytes (not 4K). You just demonstrated that even with 4K pages, different devices can share a page. > > It consumes more pci memory resource with page size alignment requirement, > > it should not be a problem on 64 bit system. > > > > Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx> > > --- > > drivers/pci/setup-res.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c > > index 967f9a758923..55440ae0128d 100644 > > --- a/drivers/pci/setup-res.c > > +++ b/drivers/pci/setup-res.c > > @@ -339,6 +339,14 @@ int pci_assign_resource(struct pci_dev *dev, int resno) > > return -EINVAL; > > } > > > > +#ifdef CONFIG_64BIT > > + /* > > + * force minimum page alignment for vfio pci usage > > + * supposing there is enough pci memory resource on 64bit system > > + */ > > + if (res->flags & IORESOURCE_MEM) > > + align = max_t(resource_size_t, PAGE_SIZE, align); > > +#endif Why is this under CONFIG_64BIT? That doesn't have anything to do with the BAR size. The only reason I can see is that with CONFIG_64BIT=y, we *might* have more MMIO space to use for BARs. > > size = resource_size(res); > > ret = _pci_assign_resource(dev, resno, size, align);
