On 2020/10/22 1:24, Ferruh Yigit wrote:
On 10/21/2020 1:32 PM, 谢华伟(此时此刻） wrote:
On 2020/10/21 19:49, Ferruh Yigit wrote:
On 10/13/2020 9:41 AM, 谢华伟(此时此刻） wrote:
From: "huawei.xhw" <huawei....@alibaba-inc.com>
Legacy virtio-pci only supports PIO BAR resource. As we need to
create lots of
virtio devices and PIO resource on x86 is very limited, we expose
MMIO BAR.
Kernel supports both PIO and MMIO BAR for legacy virtio-pci
device. We handles
different type of BAR in the similar way.
In previous implementation, with igb_uio we get PIO address from
igb_uio
sysfs entry; with uio_pci_generic, we get PIO address from
/proc/ioports.
For PIO/MMIO RW, there is different path for different drivers and
arch.
For VFIO, PIO/MMIO RW is through syscall, which has big performance
issue.
On X86, it assumes only PIO is supported.
All of the above is too much twisted.
This patch unifies the way to get both PIO and MMIO address for
different driver
and arch, all from standard resource attr under pci sysfs.
As mentined above this patch does multiple things.
The main target is, as far as I understand, you have a legacy virtio
device which supports "memory-mapped I/O" and "port-mapped I/O", but
virtio logic forces legacy devices to use the PIO but you want to be
able to use the MMIO with this device.
yes.
The solution below is adding MMIO support in the PIO funciton, and
distinguish MMIO or PIO based on their address check.
Yes, kernel does this in the similar way.
Instead of this, can't this be resolved in the virtio side, like if
the legacy device supports MMIO (detect this somehow) use the MMIO
istead of hacking PIO mapping to support MMIO?
Get your concern.
1>
If we move, I think we should move all those PCI codes into virtio
side, not just the mmio part.
Without my patch, those PCI codes are virtio-pci device specific, not
generic.
With this patch, those pci ioport map/rw code could also be used for
other devices if they support both PIO and MMIO.
I was not suggesting moving any code into virtio, but within
'vtpci_init()' what happens when "hw->modern = 1;" is set?
And if this is set for your device, will it work without change?
Yes, this will only affect legacy_device, which uses legacy_ops to
access port io.
If is is modern_device, port access will go through modern_ops.
We only change the implementation in legacy_ops.
Every option is ok. Hope i make myself clear.
2> I don't think this is hacking. for rte_pci_ioport_map/read/write,
if ioport could be both PIO and MMIO, then everything is reasonable.
Take how kernel does port map for example:
vp_dev->ioaddr = pci_iomap(pci_dev, 0, 0);
Here io doesn't mean PIO only. It could also be MMIO. Kernel then
uses ioread/write to access PIO/MMIO port.
Actually we are pretty much the same in the interface.
I think this patch extends rather then hacks the ioport interface to
support MMIO.
I have other concerns, specially mergin VFIO mapping too, but lets
clarify above first.
vfio doesn't affect other driver but only virtio.
Why it doesn't affect other drivers, can't there be other driver using
PIO?
Currently only virtio-pci uses PIO, and only virtio PMD uses these port
map/read/write functions.
I don't foresee in future any new device uses PIO.
/huawei
igb_uio, uio_pci_generic and vfio-pci all uses the same way to map/rw
ioport.
For vfio, code changes 'pci_vfio_ioport_read()' to the direct address
read, first I don't know if this is always safe, and my question why
there is a syscall introduced at first place if you can read from
address directly?
Original vfio way works, but we don't need that syscall. Under whatever
driver, we could use the simple way as in this patch.
/huawei
Is your device works as expected when vfio-pci kernel module used?
Since it is not suffering from PIO limitation, right?
Certainly i tested vfio module. Firstly, i didn't intend to fix vfio
performance issue, but i heard that igb_uio will be removed.
/huawei
And I wonder if the patch can be done as three patches to simply it, as:
1) Combine 'RTE_PCI_KDRV_IGB_UIO' & 'RTE_PCI_KDRV_UIO_GENERIC' (remove
pci_ioport_map)
2) Update 'pci_uio_ioport_map()' to add memory map support (and update
read/write functions according)
3) Combine vfio & uio
Got it. It makes sense to split, but i think this patch is already
simple enough.
Let me check.
/huawei
Thanks,
ferruh
We distinguish PIO and MMIO by their address like how kernel does.
It is ugly but works.
Signed-off-by: huawei.xhw <huawei....@alibaba-inc.com>
<...>
