On 2014/11/11 23:11, Pawel Moll wrote: > On Tue, 2014-11-04 at 09:35 +0000, Shannon Zhao wrote: >> As the current virtio-mmio only support single irq, >> so some advanced features such as vhost-net with irqfd >> are not supported. And the net performance is not >> the best without vhost-net and irqfd supporting. > > Could you, please, help understanding me where does the main issue is? > Is it about: > > 1. The fact that the existing implementation blindly kicks all queues, > instead only of the updated ones? > > or: > > 2. Literally having a dedicated interrupt line (remember, we're talking > "real" interrupts here, not message signalled ones) per queue, so they > can be handled by different processors at the same time? > The main issue is that current virtio-mmio only support one interrupt which is shared by config and queues. Therefore the virtio-mmio driver should read the "VIRTIO_MMIO_INTERRUPT_STATUS" to get the interrupt reason and check whom this interrupt is to. If we use vhost-net which uses irqfd to inject interrupt, the vhost-net doesn't update "VIRTIO_MMIO_INTERRUPT_STATUS", then the guest driver can't read the interrupt reason and doesn't call a handler to process. So we can assign a dedicated interrupt line per queue for virtio-mmio and it can work with irqfd. > Now, if it's only about 1, the simplest solution would be to extend the > VIRTIO_MMIO_INTERRUPT_STATUS register to signal up to 30 queues > "readiness" in bits 2-31, still keeping bit 0 as a "combined" > VIRTIO_MMIO_INT_VRING. In case when VIRTIO_MMIO_INT_VRING is set and > none of the "individual" bits is (a device which doesn't support this > feature or one that has more than 30 queues and of of those is ready) we > would fall back to the original "kick all queues" approach. This could > be a useful (and pretty simple) extension. In the worst case scenario it > could be a post-1.0 standard addition, as it would provide backward > compatibility. > > However, if it's about 2, we're talking larger changes here. From the > device perspective, we can define it as having per-queue (plus one for > config) interrupt output *and* a "combined" output, being simple logical > "or" of all the others. Then, the Device Tree bindings would be used to > express the implementation choices (I'd keep the kernel parameter > approach supporting the single interrupt case only). This is a very > popular and well understood approach for memory mapped peripherals (for > example, see the . It allows the system integrator to make a decision > when it's coming to latency vs number interrupt lines trade off. The > main issue is that we can't really impose a limit on a number of queues, > therefore on a number of interrupts. This would require adding a new > "interrupt acknowledge" register, which would take a number of the queue > (or a symbolic value for the config one) instead of a bit mask. And I Yes, maybe should add a new "interrupt acknowledge" register for backend and frontend to consult the number of queues. > must say that I'm not enjoying the idea of such substantial change to > the specification that late in the process... (in other words: you'll > have to put extra effort into convincing me :-) > >> This patch support virtio-mmio to request multiple >> irqs like virtio-pci. With this patch and qemu assigning >> multiple irqs for virtio-mmio device, it's ok to use >> vhost-net with irqfd on arm/arm64. > > Could you please tell me how many queues (interrupts) are we talking > about in this case? 5? A dozen? Hundreds? > Theoretically the number of interrupts has no limit, but as the limit of ARM interrupt line, the number should be less than ARM interrupt lines. In the real situation, I think, the number is generally less than 17 (8 pairs of vring interrupts and one config interrupt). > Disclaimer: I have no personal experience with virtio and network (due > to the fact how our Fast Models are implemented, I mostly us block > devices and 9p protocol over virtio and I get enough performance from > them :-). > >> As arm doesn't support msi-x now, > > To be precise: "ARM" does "support" MSI-X :-) (google for GICv2m) Sorry, I mean ARM with GICv2. > > The correct statement would be: "normal memory mapped devices have no > interface for message signalled interrupts (like MSI-X)" > Yes, that's right. >> we use GSI for multiple irq. > > I'm not sure what GSI stands for, but looking at the code I assume it's > just a "normal" peripheral interrupt. > >> In this patch we use "vm_try_to_find_vqs" >> to check whether multiple irqs are supported like >> virtio-pci. > > Yeah, I can see that you have followed virtio-pci quite literally. I'm > particularly not convinced to the one interrupt for config, one for all > queues option. Doesn't make any sense to me here. > About one interrupt for all queues, it's not a typical case. But just offer one more choice for users. Users should configure the number of interrupts according to their situation. >> Is this the right direction? is there other ways to >> make virtio-mmio support multiple irq? Hope for feedback. > > One point I'd like to make is that the device was intentionally designed > with simplicity in mind first, performance later (something about > "embedded" etc" :-). Changing this assumption is of course possible, but Ah, I think ARM is not only about embedded things. Maybe it could has a wider application such as micro server. Just my personal opinion. > - I must say - makes me slightly uncomfortable... The extensions we're > discussing here seem doable, but I've noticed your other patches doing > with a shared memory region and I didn't like them at all, sorry. > The approach with a shared memory region is dropped as you can see from the mailing list. The approach of this patch get a net performance improvement about 30%. This maybe makes sense to the paltform without MSI support(e.g ARM with GICv2). > I see the subject has been already touched in the discussions, but let > me bring PCI to the surface again. We're getting more server-class SOCs > in the market, which obviously bring PCI with them to both arm and arm64 > world, something unheard of in the "mobile past". I believe the PCI > patches for the arm64 have been already merged in the kernel. > > Therefore: I'm not your boss so, obviously, I can't tell you what to do, > but could you consider redirecting your efforts into getting the "ARM > PCI" up and running in qemu so you can simply use the existing > infrastructure? This would save us a lot of work and pain in doing late > functional changes to the standard and will be probably more > future-proof from your perspective (PCI will happen, sooner or later - > you can make it sooner ;-) > > Regards > > Pawel > > > . > -- Shannon _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
