On Thu, Jan 12, 2012 at 12:31:09PM +1030, Rusty Russell wrote: > On Wed, 11 Jan 2012 12:21:30 +0200, "Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote: > > On Wed, Jan 11, 2012 at 10:55:52AM +1030, Rusty Russell wrote: > > > On Tue, 10 Jan 2012 19:03:36 +0200, "Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote: > > > > On Wed, Dec 21, 2011 at 11:03:25AM +1030, Rusty Russell wrote: > > > > > Yes. The idea that we can alter fields in the device-specific config > > > > > area is flawed. There may be cases where it doesn't matter, but as an > > > > > idea it was holed to begin with. > > > > > > > > > > We can reduce probability by doing a double read to check, but there are > > > > > still cases where it will fail. > > > > > > > > Okay - want me to propose an interface for that? > > > > > > Had a brief chat with BenH (CC'd). > > > > > > I think we should deprecate writing to the config space. Only balloon > > > does it AFAICT, and I can't quite figure out *why* it has an 'active' > > > field. > > > > Are you sure? I think net writes a mac address. > > True. We'll need to disable that, and come up with another mechanism if > we want it back (a new feature and a VIRTIO_NET_HDR_F_SET_MAC flag in > the virtio_net header? Or would that mess up vhost_net?). vhost net is only datapath, so no problem. copying in host is tricky for vhost_net. > > > This solves half the problem, of sync guest writes. For the > > > other half, I suggest a generation counter; odd means inconsistent. The > > > guest can poll. > > > > So we get the counter until it's even, get the config, if it's changed > > repeat? Yes it works. However, I would like to have a way to detect > > config change just by looking at memory. ATM we need to read ISR to > > know. If we used a VQ, the advantage would be that the device can work > > with a single MSIX vector shared by all VQs. > > If we use a 32-bit counter, we also get this though, right? > > If counter has changed, it's a config interrupt... But we need an exit to read the counter. We can put the counter in memory but this looks suspiciously like a simplified VQ - so why not use a VQ then? > > If we do require config VQ anyway, why not use it to notify > > guest of config changes? Guest could pre-post an in buffer > > and host uses that. > > We could, but it's an additional burden on each device. vqs are cheap, > but not free. And the config area is so damn convenient... Not if you start playing with counters, checking it twice, reinvent all kind of barriers ... > > > BenH also convinced me we should finally make the config space LE if > > > we're going to change things. Since PCI is the most common transport, > > > guest-endian confuses people. And it sucks for really weird machines. > > > > Are we going to keep guest endian for e.g. virtio net header? > > If yes the benefit of switching config space is not that big. > > And changes in devices would affect non-PCI transports. > > Yep. It would only make sense if we do it for everything. And yes, > it'll mess up everyone who is BE, so it needs to be a feature bit for > them. > > > > We should also change the ring (to a single ring, I think). Descriptors > > > to 24 bytes long (8 byte cookie, 8 byte addr, 4 byte len, 4 byte flags). > > > We might be able to squeeze it into 20 bytes but that means packing. We > > > should support inline, chained or indirect. Let the other side ack by > > > setting flag, cookie and len (if written). > > > > Quite possibly all or some of these things help performance > > but do we have to change the spec before we have experimental > > proof? > > We change the spec last, once we know what we're doing, ideally. > > > I did experiment with a single ring using tools/virtio and > > I didn't see a measureable performance gain. > > Interesting. It is simpler and more standard than our current design, > but that's not sufficient unless there are other reasons. Needs further > discussion and testing. > > > Two rings do have the advantage of not requiring host side copy, which > > copy would surely add to cache pressure. > > Well, a simple host could process in-order and leave stuff in the ring I > guess. A smarter host would copy and queue, maybe leave one queue entry > in so it doesn't get flooded? > > > Since > > host doesn't change desriptors, we could also > > preformat some descriptors in the current design. > > > > There is a fragmentation problem in theory but it can be alleviated with > > a smart allocator. > > Yeah, the complexity scares me... > > > About inline - it can only help very small buffers. > > Which workloads do you have in mind exactly? > > It was suggested by others, but I think TCP Acks are the classic one. > 12 + 14 + 20 + 40 = 86 bytes with virtio_net_hdr_mrg_rxbuf at the front. That's only the simplest IPv4, right? Anyway, this spans multiple descriptors so this complicates allocation significantly. > > BTW this seems to be the reverse from what you have in Mar 2001, > > see 87mxkjls61.fsf@xxxxxxxxxxxxxxx :) > > (s/2001/2011). Indeed. Noone shared my optimism that having an open > process for a virtio2 would bring more players on board (my original > motivation). But technical requirements are mounting up, which means > we're going to get there anyway. > > > I am much less concerned with what we do for configuration, > > but I do not believe we have learned all performance lessons > > from virtio ring1. Is there any reason why we shouldn't be > > able to experiment with inline within virtio1 and see > > whether that gets us anything? > > Inline in the used ring is possible, but those descriptors 8 bytes, vs > 24/32. Hmm, 86 > 32 anyway, right? > > If we do a bunch of changes to the ring at once, we can't > > figure out what's right, what's wrong, or back out of > > mistakes later. > > > > Since there are non PCI transports that use the ring, > > we really shouldn't make both the configuration and > > the ring changes depend on the same feature bit. > > Yes, I'm thinking #define VIRTIO_F_VIRTIO2 (-1). For PCI, this gets > mapped into a "are we using the new config layout?". For others, it > gets mapped into a transport-specific feature. > > (I'm sure you get it, but for the others) This is because I want to be > draw a clear line between all the legacy stuff at the same time, not > have to support part of it later because someone might not flip the > feature bit. > > Cheers, > Rusty. So my point is, config stuff and ring are completely separate, they are different layers. -- MST _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
