[dpdk-dev] [PATCH v2 3/3] virtio: Add a new layer to abstract pci access method
On 2016/02/02 11:45, Yuanhan Liu wrote:
> On Tue, Feb 02, 2016 at 11:19:50AM +0900, Tetsuya Mukawa wrote:
>> On 2016/02/01 22:15, Yuanhan Liu wrote:
>>> On Mon, Feb 01, 2016 at 10:50:00AM +0900, Tetsuya Mukawa wrote:
On 2016/01/29 18:17, Yuanhan Liu wrote:
> On Thu, Jan 28, 2016 at 06:33:32PM +0900, Tetsuya Mukawa wrote:
>> This patch addss function pointers to abstract pci access method.
>> This abstraction layer will be used when virtio-net PMD supports
>> container extension.
>>
>> The below functions abstract how to access to pci configuration space.
>>
>> struct virtio_pci_cfg_ops {
>> int (*map)(...);
>> void (*unmap)(...);
>> void *(*get_mapped_addr)(...);
>> int (*read)(...);
>> };
>>
>> The pci configuration space has information how to access to virtio
>> device registers. Basically, there are 2 ways to acccess to the
>> registers. One is using portio and the other is using mapped memory.
>> The below functions abstract this access method.
> One question: is there a way to map PCI memory with Qtest? I'm thinking
> if we can keep the io_read/write() for Qtest as well, if so, code could
> be simplified, a lot, IMO.
>
Yes, I agree with you.
But AFAIK, we don't have a way to mmap it from DPDK application.
We may be able to map PCI configuration space to a memory address space
that guest CPU can handle.
But even in this case, I guess we cannot access the memory without qtest
messaging.
>>> Acutally, I have a concern about this access abstraction, which makes
>>> those simple funciton not inline. It won't be an issue for most of them,
>>> as most of them are invoked during init stage, where has no impact on
>>> performance.
>>>
>>> notify_queue(), however, is a bit different. I was thinking the "inline
>>> to callback (not inline)" convertion might has some impacts on the
>>> performance. Would you do a test for me?
>> Sure, I will be able to.
> Thanks.
>
>> But if we concern about it, I guess it's also nice to implement the PMD
>> on your vtpci abstraction.
>> (It means we don't use the access abstraction)
>> Probably this lets our merging process faster.
>> What do you think?
> Another standalone PMD driver? (sorry that I didn't follow the
> discussion).
Yes, Jianfeng will submit one more virtual virtio-net PMD.
> If so, won't it introduce too much duplicate code?
Quick look, I guess we won't have not so much duplicated code.
Tetsuya
[dpdk-dev] [PATCH v2 3/3] virtio: Add a new layer to abstract pci access method
On 2016/02/01 22:15, Yuanhan Liu wrote:
> On Mon, Feb 01, 2016 at 10:50:00AM +0900, Tetsuya Mukawa wrote:
>> On 2016/01/29 18:17, Yuanhan Liu wrote:
>>> On Thu, Jan 28, 2016 at 06:33:32PM +0900, Tetsuya Mukawa wrote:
This patch addss function pointers to abstract pci access method.
This abstraction layer will be used when virtio-net PMD supports
container extension.
The below functions abstract how to access to pci configuration space.
struct virtio_pci_cfg_ops {
int (*map)(...);
void (*unmap)(...);
void *(*get_mapped_addr)(...);
int (*read)(...);
};
The pci configuration space has information how to access to virtio
device registers. Basically, there are 2 ways to acccess to the
registers. One is using portio and the other is using mapped memory.
The below functions abstract this access method.
>>> One question: is there a way to map PCI memory with Qtest? I'm thinking
>>> if we can keep the io_read/write() for Qtest as well, if so, code could
>>> be simplified, a lot, IMO.
>>>
>> Yes, I agree with you.
>> But AFAIK, we don't have a way to mmap it from DPDK application.
>>
>> We may be able to map PCI configuration space to a memory address space
>> that guest CPU can handle.
>> But even in this case, I guess we cannot access the memory without qtest
>> messaging.
> Acutally, I have a concern about this access abstraction, which makes
> those simple funciton not inline. It won't be an issue for most of them,
> as most of them are invoked during init stage, where has no impact on
> performance.
>
> notify_queue(), however, is a bit different. I was thinking the "inline
> to callback (not inline)" convertion might has some impacts on the
> performance. Would you do a test for me?
Sure, I will be able to.
But if we concern about it, I guess it's also nice to implement the PMD
on your vtpci abstraction.
(It means we don't use the access abstraction)
Probably this lets our merging process faster.
What do you think?
Also I guess Jianfeng will implement his PMD on your abstraction.
If so, I will also follow him.
>
> Another off-topic remind is that I guess you might need to send a new
> version of your vhost-pmd patchset, the sooner the better. Chinese new
> year is coming; I'm having vacation since the end of this week (And,
> Huawei has been on vacation sine the end of last week). I hope we could
> make it in v2.3.
Thanks for notification. Sure I will submit it soon.
Tetsuya
[dpdk-dev] [PATCH v2 3/3] virtio: Add a new layer to abstract pci access method
On Tue, Feb 02, 2016 at 11:19:50AM +0900, Tetsuya Mukawa wrote:
> On 2016/02/01 22:15, Yuanhan Liu wrote:
> > On Mon, Feb 01, 2016 at 10:50:00AM +0900, Tetsuya Mukawa wrote:
> >> On 2016/01/29 18:17, Yuanhan Liu wrote:
> >>> On Thu, Jan 28, 2016 at 06:33:32PM +0900, Tetsuya Mukawa wrote:
> This patch addss function pointers to abstract pci access method.
> This abstraction layer will be used when virtio-net PMD supports
> container extension.
>
> The below functions abstract how to access to pci configuration space.
>
> struct virtio_pci_cfg_ops {
> int (*map)(...);
> void (*unmap)(...);
> void *(*get_mapped_addr)(...);
> int (*read)(...);
> };
>
> The pci configuration space has information how to access to virtio
> device registers. Basically, there are 2 ways to acccess to the
> registers. One is using portio and the other is using mapped memory.
> The below functions abstract this access method.
> >>> One question: is there a way to map PCI memory with Qtest? I'm thinking
> >>> if we can keep the io_read/write() for Qtest as well, if so, code could
> >>> be simplified, a lot, IMO.
> >>>
> >> Yes, I agree with you.
> >> But AFAIK, we don't have a way to mmap it from DPDK application.
> >>
> >> We may be able to map PCI configuration space to a memory address space
> >> that guest CPU can handle.
> >> But even in this case, I guess we cannot access the memory without qtest
> >> messaging.
> > Acutally, I have a concern about this access abstraction, which makes
> > those simple funciton not inline. It won't be an issue for most of them,
> > as most of them are invoked during init stage, where has no impact on
> > performance.
> >
> > notify_queue(), however, is a bit different. I was thinking the "inline
> > to callback (not inline)" convertion might has some impacts on the
> > performance. Would you do a test for me?
>
> Sure, I will be able to.
Thanks.
> But if we concern about it, I guess it's also nice to implement the PMD
> on your vtpci abstraction.
> (It means we don't use the access abstraction)
> Probably this lets our merging process faster.
> What do you think?
Another standalone PMD driver? (sorry that I didn't follow the
discussion). If so, won't it introduce too much duplicate code?
--yliu
[dpdk-dev] [PATCH v2 3/3] virtio: Add a new layer to abstract pci access method
On Mon, Feb 01, 2016 at 10:50:00AM +0900, Tetsuya Mukawa wrote:
> On 2016/01/29 18:17, Yuanhan Liu wrote:
> > On Thu, Jan 28, 2016 at 06:33:32PM +0900, Tetsuya Mukawa wrote:
> >> This patch addss function pointers to abstract pci access method.
> >> This abstraction layer will be used when virtio-net PMD supports
> >> container extension.
> >>
> >> The below functions abstract how to access to pci configuration space.
> >>
> >> struct virtio_pci_cfg_ops {
> >> int (*map)(...);
> >> void (*unmap)(...);
> >> void *(*get_mapped_addr)(...);
> >> int (*read)(...);
> >> };
> >>
> >> The pci configuration space has information how to access to virtio
> >> device registers. Basically, there are 2 ways to acccess to the
> >> registers. One is using portio and the other is using mapped memory.
> >> The below functions abstract this access method.
> > One question: is there a way to map PCI memory with Qtest? I'm thinking
> > if we can keep the io_read/write() for Qtest as well, if so, code could
> > be simplified, a lot, IMO.
> >
>
> Yes, I agree with you.
> But AFAIK, we don't have a way to mmap it from DPDK application.
>
> We may be able to map PCI configuration space to a memory address space
> that guest CPU can handle.
> But even in this case, I guess we cannot access the memory without qtest
> messaging.
Acutally, I have a concern about this access abstraction, which makes
those simple funciton not inline. It won't be an issue for most of them,
as most of them are invoked during init stage, where has no impact on
performance.
notify_queue(), however, is a bit different. I was thinking the "inline
to callback (not inline)" convertion might has some impacts on the
performance. Would you do a test for me?
Another off-topic remind is that I guess you might need to send a new
version of your vhost-pmd patchset, the sooner the better. Chinese new
year is coming; I'm having vacation since the end of this week (And,
Huawei has been on vacation sine the end of last week). I hope we could
make it in v2.3.
--yliu
[dpdk-dev] [PATCH v2 3/3] virtio: Add a new layer to abstract pci access method
On 2016/01/29 18:17, Yuanhan Liu wrote:
> On Thu, Jan 28, 2016 at 06:33:32PM +0900, Tetsuya Mukawa wrote:
>> This patch addss function pointers to abstract pci access method.
>> This abstraction layer will be used when virtio-net PMD supports
>> container extension.
>>
>> The below functions abstract how to access to pci configuration space.
>>
>> struct virtio_pci_cfg_ops {
>> int (*map)(...);
>> void (*unmap)(...);
>> void *(*get_mapped_addr)(...);
>> int (*read)(...);
>> };
>>
>> The pci configuration space has information how to access to virtio
>> device registers. Basically, there are 2 ways to acccess to the
>> registers. One is using portio and the other is using mapped memory.
>> The below functions abstract this access method.
> One question: is there a way to map PCI memory with Qtest? I'm thinking
> if we can keep the io_read/write() for Qtest as well, if so, code could
> be simplified, a lot, IMO.
>
Yes, I agree with you.
But AFAIK, we don't have a way to mmap it from DPDK application.
We may be able to map PCI configuration space to a memory address space
that guest CPU can handle.
But even in this case, I guess we cannot access the memory without qtest
messaging.
Thanks,
Tetsuya
[dpdk-dev] [PATCH v2 3/3] virtio: Add a new layer to abstract pci access method
On Thu, Jan 28, 2016 at 06:33:32PM +0900, Tetsuya Mukawa wrote:
> This patch addss function pointers to abstract pci access method.
> This abstraction layer will be used when virtio-net PMD supports
> container extension.
>
> The below functions abstract how to access to pci configuration space.
>
> struct virtio_pci_cfg_ops {
> int (*map)(...);
> void (*unmap)(...);
> void *(*get_mapped_addr)(...);
> int (*read)(...);
> };
>
> The pci configuration space has information how to access to virtio
> device registers. Basically, there are 2 ways to acccess to the
> registers. One is using portio and the other is using mapped memory.
> The below functions abstract this access method.
One question: is there a way to map PCI memory with Qtest? I'm thinking
if we can keep the io_read/write() for Qtest as well, if so, code could
be simplified, a lot, IMO.
--yliu
[dpdk-dev] [PATCH v2 3/3] virtio: Add a new layer to abstract pci access method
This patch addss function pointers to abstract pci access method.
This abstraction layer will be used when virtio-net PMD supports
container extension.
The below functions abstract how to access to pci configuration space.
struct virtio_pci_cfg_ops {
int (*map)(...);
void (*unmap)(...);
void *(*get_mapped_addr)(...);
int (*read)(...);
};
The pci configuration space has information how to access to virtio
device registers. Basically, there are 2 ways to acccess to the
registers. One is using portio and the other is using mapped memory.
The below functions abstract this access method.
struct virtio_pci_dev_ops {
uint8_t (*read8)(...);
uint16_t (*read16)(...);
uint32_t (*read32)(...);
void (*write8)(...);
void (*write16)(...);
void (*write32)(...);
};
Signed-off-by: Tetsuya Mukawa
---
drivers/net/virtio/virtio_ethdev.c | 4 +-
drivers/net/virtio/virtio_pci.c| 531 +
drivers/net/virtio/virtio_pci.h| 24 +-
3 files changed, 386 insertions(+), 173 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c
b/drivers/net/virtio/virtio_ethdev.c
index 37833a8..c477b05 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1037,7 +1037,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
- if (vtpci_init(pci_dev, hw) < 0)
+ if (vtpci_init(eth_dev, hw) < 0)
return -1;
/* Reset the device although not necessary at startup */
@@ -1177,7 +1177,7 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
rte_intr_callback_unregister(&pci_dev->intr_handle,
virtio_interrupt_handler,
eth_dev);
- vtpci_uninit(pci_dev, hw);
+ vtpci_uninit(eth_dev, hw);
PMD_INIT_LOG(DEBUG, "dev_uninit completed");
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index 3e6be8c..c6d72f9 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -49,24 +49,198 @@
#define PCI_CAPABILITY_LIST0x34
#define PCI_CAP_ID_VNDR0x09
+static uint8_t
+phys_legacy_read8(struct virtio_hw *hw, uint8_t *addr)
+{
+ return inb((unsigned short)(hw->io_base + (uint64_t)addr));
+}
-#define VIRTIO_PCI_REG_ADDR(hw, reg) \
- (unsigned short)((hw)->io_base + (reg))
+static uint16_t
+phys_legacy_read16(struct virtio_hw *hw, uint16_t *addr)
+{
+ return inw((unsigned short)(hw->io_base + (uint64_t)addr));
+}
-#define VIRTIO_READ_REG_1(hw, reg) \
- inb((VIRTIO_PCI_REG_ADDR((hw), (reg
-#define VIRTIO_WRITE_REG_1(hw, reg, value) \
- outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg
+static uint32_t
+phys_legacy_read32(struct virtio_hw *hw, uint32_t *addr)
+{
+ return inl((unsigned short)(hw->io_base + (uint64_t)addr));
+}
-#define VIRTIO_READ_REG_2(hw, reg) \
- inw((VIRTIO_PCI_REG_ADDR((hw), (reg
-#define VIRTIO_WRITE_REG_2(hw, reg, value) \
- outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg
+static void
+phys_legacy_write8(struct virtio_hw *hw, uint8_t *addr, uint8_t val)
+{
+ return outb_p((unsigned char)val,
+ (unsigned short)(hw->io_base + (uint64_t)addr));
+}
-#define VIRTIO_READ_REG_4(hw, reg) \
- inl((VIRTIO_PCI_REG_ADDR((hw), (reg
-#define VIRTIO_WRITE_REG_4(hw, reg, value) \
- outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg
+static void
+phys_legacy_write16(struct virtio_hw *hw, uint16_t *addr, uint16_t val)
+{
+ return outw_p((unsigned short)val,
+ (unsigned short)(hw->io_base + (uint64_t)addr));
+}
+
+static void
+phys_legacy_write32(struct virtio_hw *hw, uint32_t *addr, uint32_t val)
+{
+ return outl_p((unsigned int)val,
+ (unsigned short)(hw->io_base + (uint64_t)addr));
+}
+
+static const struct virtio_pci_dev_ops phys_legacy_dev_ops = {
+ .read8 = phys_legacy_read8,
+ .read16 = phys_legacy_read16,
+ .read32 = phys_legacy_read32,
+ .write8 = phys_legacy_write8,
+ .write16= phys_legacy_write16,
+ .write32= phys_legacy_write32,
+};
+
+static uint8_t
+phys_modern_read8(struct virtio_hw *hw __rte_unused, uint8_t *addr)
+{
+ return *(volatile uint8_t *)addr;
+}
+
+static uint16_t
+phys_modern_read16(struct virtio_hw *hw __rte_unused, uint16_t *addr)
+{
+ return *(volatile uint16_t *)addr;
+}
+
+static uint32_t
+phys_modern_read32(struct virtio_hw *hw __rte_unused, uint32_t *addr)
+{
+ return *(volatile uint32_t *)addr;
+}
+
+static void
+phys_modern_write8(struct virtio_hw *hw __rte_unused,
+ uint8_t *addr, uint8_t val)
+{
+ *(volatile uint8_t *)addr = val;
+}
+
+static
