On 10/22/2020 10:15 AM, 谢华伟(此时此刻） wrote:
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.
I am saying something else.
When a device is marked as "hw->modern = 1;", it will use MMIO, right?
If, somehow, your device marked as "hw->modern = 1;", will that path work as
expected for your device?
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.
I see but technically there can be other users.
/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.
If vfio works, you have already a solution, that is good. But I see you are not
happy with its performance.
/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.
Yes, it will be removed in the long run.
/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.
The patch is doing many things in one patch, I think it is better to separate
logically separate issues, although they are simple.
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>
<...>
