From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tiwei Bie Subject: Re: [RFC v2] virtio: support packed ring Date: Fri, 13 Apr 2018 15:15:29 +0800 Message-ID: <20180413071529.f4esh654dakodf4f@debian> References: <20180401141216.8969-1-tiwei.bie@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Jason Wang Cc: mst@redhat.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, wexu@redhat.com List-Id: virtualization@lists.linuxfoundation.org T24gRnJpLCBBcHIgMTMsIDIwMTggYXQgMTI6MzA6MjRQTSArMDgwMCwgSmFzb24gV2FuZyB3cm90 ZToKPiBPbiAyMDE45bm0MDTmnIgwMeaXpSAyMjoxMiwgVGl3ZWkgQmllIHdyb3RlOgo+ID4gSGVs bG8gZXZlcnlvbmUsCj4gPiAKPiA+IFRoaXMgUkZDIGltcGxlbWVudHMgcGFja2VkIHJpbmcgc3Vw cG9ydCBmb3IgdmlydGlvIGRyaXZlci4KPiA+IAo+ID4gVGhlIGNvZGUgd2FzIHRlc3RlZCB3aXRo IERQREsgdmhvc3QgKHRlc3RwbWQvdmhvc3QtUE1EKSBpbXBsZW1lbnRlZAo+ID4gYnkgSmVucyBh dCBodHRwOi8vZHBkay5vcmcvbWwvYXJjaGl2ZXMvZGV2LzIwMTgtSmFudWFyeS8wODk0MTcuaHRt bAo+ID4gTWlub3IgY2hhbmdlcyBhcmUgbmVlZGVkIGZvciB0aGUgdmhvc3QgY29kZSwgZS5nLiB0 byBraWNrIHRoZSBndWVzdC4KPiA+IAo+ID4gVE9ETzoKPiA+IC0gUmVmaW5lbWVudHMgYW5kIGJ1 ZyBmaXhlczsKPiA+IC0gU3BsaXQgaW50byBzbWFsbCBwYXRjaGVzOwo+ID4gLSBUZXN0IGluZGly ZWN0IGRlc2NyaXB0b3Igc3VwcG9ydDsKPiA+IC0gVGVzdC9maXggZXZlbnQgc3VwcHJlc3Npb24g c3VwcG9ydDsKPiA+IC0gVGVzdCBkZXZpY2VzIG90aGVyIHRoYW4gbmV0Owo+ID4gCj4gPiBSRkMg djEgLT4gUkZDIHYyOgo+ID4gLSBBZGQgaW5kaXJlY3QgZGVzY3JpcHRvciBzdXBwb3J0IC0gY29t cGlsZSB0ZXN0IG9ubHk7Cj4gPiAtIEFkZCBldmVudCBzdXBwcmVzc2lvbiBzdXBwcnQgLSBjb21w aWxlIHRlc3Qgb25seTsKPiA+IC0gTW92ZSB2cmluZ19wYWNrZWRfaW5pdCgpIG91dCBvZiB1YXBp IChKYXNvbiwgTVNUKTsKPiA+IC0gTWVyZ2UgdHdvIGxvb3BzIGludG8gb25lIGluIHZpcnRxdWV1 ZV9hZGRfcGFja2VkKCkgKEphc29uKTsKPiA+IC0gU3BsaXQgdnJpbmdfdW5tYXBfb25lKCkgZm9y IHBhY2tlZCByaW5nIGFuZCBzcGxpdCByaW5nIChKYXNvbik7Cj4gPiAtIEF2b2lkIHVzaW5nICcl JyBvcGVyYXRvciAoSmFzb24pOwo+ID4gLSBSZW5hbWUgZnJlZV9oZWFkIC0+IG5leHRfYXZhaWxf aWR4IChKYXNvbik7Cj4gPiAtIEFkZCBjb21tZW50cyBmb3IgdmlydGlvX3dtYigpIGluIHZpcnRx dWV1ZV9hZGRfcGFja2VkKCkgKEphc29uKTsKPiA+IC0gU29tZSBvdGhlciByZWZpbmVtZW50cyBh bmQgYnVnIGZpeGVzOwo+ID4gCj4gPiBUaGFua3MhCj4gPiAKPiA+IFNpZ25lZC1vZmYtYnk6IFRp d2VpIEJpZSA8dGl3ZWkuYmllQGludGVsLmNvbT4KPiA+IC0tLQo+ID4gICBkcml2ZXJzL3ZpcnRp by92aXJ0aW9fcmluZy5jICAgICAgIHwgMTA5NCArKysrKysrKysrKysrKysrKysrKysrKysrKysr Ky0tLS0tLS0KPiA+ICAgaW5jbHVkZS9saW51eC92aXJ0aW9fcmluZy5oICAgICAgICB8ICAgIDgg Ky0KPiA+ICAgaW5jbHVkZS91YXBpL2xpbnV4L3ZpcnRpb19jb25maWcuaCB8ICAgMTIgKy0KPiA+ ICAgaW5jbHVkZS91YXBpL2xpbnV4L3ZpcnRpb19yaW5nLmggICB8ICAgNjEgKysKPiA+ICAgNCBm aWxlcyBjaGFuZ2VkLCA5ODAgaW5zZXJ0aW9ucygrKSwgMTk1IGRlbGV0aW9ucygtKQpbLi4uXQo+ ID4gK3N0YXRpYyBzdHJ1Y3QgdnJpbmdfcGFja2VkX2Rlc2MgKmFsbG9jX2luZGlyZWN0X3BhY2tl ZChzdHJ1Y3QgdmlydHF1ZXVlICpfdnEsCj4gPiArCQkJCQkJICAgICAgIHVuc2lnbmVkIGludCB0 b3RhbF9zZywKPiA+ICsJCQkJCQkgICAgICAgZ2ZwX3QgZ2ZwKQo+ID4gK3sKPiA+ICsJc3RydWN0 IHZyaW5nX3BhY2tlZF9kZXNjICpkZXNjOwo+ID4gKwo+ID4gKwkvKgo+ID4gKwkgKiBXZSByZXF1 aXJlIGxvd21lbSBtYXBwaW5ncyBmb3IgdGhlIGRlc2NyaXB0b3JzIGJlY2F1c2UKPiA+ICsJICog b3RoZXJ3aXNlIHZpcnRfdG9fcGh5cyB3aWxsIGdpdmUgdXMgYm9ndXMgYWRkcmVzc2VzIGluIHRo ZQo+ID4gKwkgKiB2aXJ0cXVldWUuCj4gPiArCSAqLwo+ID4gKwlnZnAgJj0gfl9fR0ZQX0hJR0hN RU07Cj4gPiArCj4gPiArCWRlc2MgPSBrbWFsbG9jKHRvdGFsX3NnICogc2l6ZW9mKHN0cnVjdCB2 cmluZ19wYWNrZWRfZGVzYyksIGdmcCk7Cj4gCj4gQ2FuIHdlIHNpbXBseSBjaGVjayB2cS0+cGFj a2VkIGhlcmUgdG8gYXZvaWQgZHVwbGljYXRpbmcgaGVscGVycz8KClRoZW4gaXQgd291bGQgYmUg c29tZXRoaW5nIGxpa2UgdGhpczoKCnN0YXRpYyB2b2lkICphbGxvY19pbmRpcmVjdChzdHJ1Y3Qg dmlydHF1ZXVlICpfdnEsIHVuc2lnbmVkIGludCB0b3RhbF9zZywKCQkJICAgIGdmcF90IGdmcCkK ewoJc3RydWN0IHZyaW5nX3ZpcnRxdWV1ZSAqdnEgPSB0b192dnEoX3ZxKTsKCXZvaWQgKmRhdGE7 CgoJLyoKCSAqIFdlIHJlcXVpcmUgbG93bWVtIG1hcHBpbmdzIGZvciB0aGUgZGVzY3JpcHRvcnMg YmVjYXVzZQoJICogb3RoZXJ3aXNlIHZpcnRfdG9fcGh5cyB3aWxsIGdpdmUgdXMgYm9ndXMgYWRk cmVzc2VzIGluIHRoZQoJICogdmlydHF1ZXVlLgoJICovCglnZnAgJj0gfl9fR0ZQX0hJR0hNRU07 CgoJaWYgKHZxLT5wYWNrZWQpIHsKCQlkYXRhID0ga21hbGxvYyh0b3RhbF9zZyAqIHNpemVvZihz dHJ1Y3QgdnJpbmdfcGFja2VkX2Rlc2MpLAoJCQkJZ2ZwKTsKCQlpZiAoIWRhdGEpCgkJCXJldHVy biBOVUxMOwoJfSBlbHNlIHsKCQlzdHJ1Y3QgdnJpbmdfZGVzYyAqZGVzYzsKCQl1bnNpZ25lZCBp bnQgaTsKCgkJZGVzYyA9IGttYWxsb2ModG90YWxfc2cgKiBzaXplb2Yoc3RydWN0IHZyaW5nX2Rl c2MpLCBnZnApOwoJCWlmICghZGVzYykKCQkJcmV0dXJuIE5VTEw7CgoJCWZvciAoaSA9IDA7IGkg PCB0b3RhbF9zZzsgaSsrKQoJCQlkZXNjW2ldLm5leHQgPSBjcHVfdG9fdmlydGlvMTYoX3ZxLT52 ZGV2LCBpICsgMSk7CgoJCWRhdGEgPSBkZXNjOwoJfQoKCXJldHVybiBkYXRhOwp9CgpJIHdvdWxk IHByZWZlciB0byBoYXZlIHR3byBzaW1wbGVyIGhlbHBlcnMgKGFuZCB0byB0aGUgY2FsbGVycywK aXQncyBhbHJlYWR5IHZlcnkgY2xlYXIgYWJvdXQgd2hpY2ggb25lIHRoZXkgc2hvdWxkIGNhbGwp LCBpLmUuCnRoZSBjdXJyZW50IGltcGxlbWVudGF0aW9uOgoKc3RhdGljIHN0cnVjdCB2cmluZ19w YWNrZWRfZGVzYyAqYWxsb2NfaW5kaXJlY3RfcGFja2VkKHN0cnVjdCB2aXJ0cXVldWUgKl92cSwK CQkJCQkJICAgICAgIHVuc2lnbmVkIGludCB0b3RhbF9zZywKCQkJCQkJICAgICAgIGdmcF90IGdm cCkKewoJc3RydWN0IHZyaW5nX3BhY2tlZF9kZXNjICpkZXNjOwoKCS8qCgkgKiBXZSByZXF1aXJl IGxvd21lbSBtYXBwaW5ncyBmb3IgdGhlIGRlc2NyaXB0b3JzIGJlY2F1c2UKCSAqIG90aGVyd2lz ZSB2aXJ0X3RvX3BoeXMgd2lsbCBnaXZlIHVzIGJvZ3VzIGFkZHJlc3NlcyBpbiB0aGUKCSAqIHZp cnRxdWV1ZS4KCSAqLwoJZ2ZwICY9IH5fX0dGUF9ISUdITUVNOwoKCWRlc2MgPSBrbWFsbG9jKHRv dGFsX3NnICogc2l6ZW9mKHN0cnVjdCB2cmluZ19wYWNrZWRfZGVzYyksIGdmcCk7CgoJcmV0dXJu IGRlc2M7Cn0KCnN0YXRpYyBzdHJ1Y3QgdnJpbmdfZGVzYyAqYWxsb2NfaW5kaXJlY3Rfc3BsaXQo c3RydWN0IHZpcnRxdWV1ZSAqX3ZxLAoJCQkJCSAgICAgICB1bnNpZ25lZCBpbnQgdG90YWxfc2cs CgkJCQkJICAgICAgIGdmcF90IGdmcCkKewoJc3RydWN0IHZyaW5nX2Rlc2MgKmRlc2M7Cgl1bnNp Z25lZCBpbnQgaTsKCgkvKgoJICogV2UgcmVxdWlyZSBsb3dtZW0gbWFwcGluZ3MgZm9yIHRoZSBk ZXNjcmlwdG9ycyBiZWNhdXNlCgkgKiBvdGhlcndpc2UgdmlydF90b19waHlzIHdpbGwgZ2l2ZSB1 cyBib2d1cyBhZGRyZXNzZXMgaW4gdGhlCgkgKiB2aXJ0cXVldWUuCgkgKi8KCWdmcCAmPSB+X19H RlBfSElHSE1FTTsKCglkZXNjID0ga21hbGxvYyh0b3RhbF9zZyAqIHNpemVvZihzdHJ1Y3QgdnJp bmdfZGVzYyksIGdmcCk7CglpZiAoIWRlc2MpCgkJcmV0dXJuIE5VTEw7CgoJZm9yIChpID0gMDsg aSA8IHRvdGFsX3NnOyBpKyspCgkJZGVzY1tpXS5uZXh0ID0gY3B1X3RvX3ZpcnRpbzE2KF92cS0+ dmRldiwgaSArIDEpOwoJcmV0dXJuIGRlc2M7Cn0KCj4gCj4gPiArCj4gPiArCXJldHVybiBkZXNj Owo+ID4gK30KWy4uLl0KPiA+ICtzdGF0aWMgaW5saW5lIGludCB2aXJ0cXVldWVfYWRkX3BhY2tl ZChzdHJ1Y3QgdmlydHF1ZXVlICpfdnEsCj4gPiArCQkJCSAgICAgICBzdHJ1Y3Qgc2NhdHRlcmxp c3QgKnNnc1tdLAo+ID4gKwkJCQkgICAgICAgdW5zaWduZWQgaW50IHRvdGFsX3NnLAo+ID4gKwkJ CQkgICAgICAgdW5zaWduZWQgaW50IG91dF9zZ3MsCj4gPiArCQkJCSAgICAgICB1bnNpZ25lZCBp bnQgaW5fc2dzLAo+ID4gKwkJCQkgICAgICAgdm9pZCAqZGF0YSwKPiA+ICsJCQkJICAgICAgIHZv aWQgKmN0eCwKPiA+ICsJCQkJICAgICAgIGdmcF90IGdmcCkKPiA+ICt7Cj4gPiArCXN0cnVjdCB2 cmluZ192aXJ0cXVldWUgKnZxID0gdG9fdnZxKF92cSk7Cj4gPiArCXN0cnVjdCB2cmluZ19wYWNr ZWRfZGVzYyAqZGVzYzsKPiA+ICsJc3RydWN0IHNjYXR0ZXJsaXN0ICpzZzsKPiA+ICsJdW5zaWdu ZWQgaW50IGksIG4sIGRlc2NzX3VzZWQsIHVuaW5pdGlhbGl6ZWRfdmFyKHByZXYpLCBlcnJfaWR4 Owo+ID4gKwlfX3ZpcnRpbzE2IHVuaW5pdGlhbGl6ZWRfdmFyKGhlYWRfZmxhZ3MpLCBmbGFnczsK PiA+ICsJaW50IGhlYWQsIHdyYXBfY291bnRlcjsKPiA+ICsJYm9vbCBpbmRpcmVjdDsKPiA+ICsK PiA+ICsJU1RBUlRfVVNFKHZxKTsKPiA+ICsKPiA+ICsJQlVHX09OKGRhdGEgPT0gTlVMTCk7Cj4g PiArCUJVR19PTihjdHggJiYgdnEtPmluZGlyZWN0KTsKPiA+ICsKPiA+ICsJaWYgKHVubGlrZWx5 KHZxLT5icm9rZW4pKSB7Cj4gPiArCQlFTkRfVVNFKHZxKTsKPiA+ICsJCXJldHVybiAtRUlPOwo+ ID4gKwl9Cj4gPiArCj4gPiArI2lmZGVmIERFQlVHCj4gPiArCXsKPiA+ICsJCWt0aW1lX3Qgbm93 ID0ga3RpbWVfZ2V0KCk7Cj4gPiArCj4gPiArCQkvKiBObyBraWNrIG9yIGdldCwgd2l0aCAuMSBz ZWNvbmQgYmV0d2Vlbj8gIFdhcm4uICovCj4gPiArCQlpZiAodnEtPmxhc3RfYWRkX3RpbWVfdmFs aWQpCj4gPiArCQkJV0FSTl9PTihrdGltZV90b19tcyhrdGltZV9zdWIobm93LCB2cS0+bGFzdF9h ZGRfdGltZSkpCj4gPiArCQkJCQkgICAgPiAxMDApOwo+ID4gKwkJdnEtPmxhc3RfYWRkX3RpbWUg PSBub3c7Cj4gPiArCQl2cS0+bGFzdF9hZGRfdGltZV92YWxpZCA9IHRydWU7Cj4gPiArCX0KPiA+ ICsjZW5kaWYKPiA+ICsKPiA+ICsJQlVHX09OKHRvdGFsX3NnID09IDApOwo+ID4gKwo+ID4gKwlo ZWFkID0gdnEtPm5leHRfYXZhaWxfaWR4Owo+ID4gKwl3cmFwX2NvdW50ZXIgPSB2cS0+d3JhcF9j b3VudGVyOwo+ID4gKwo+ID4gKwkvKiBJZiB0aGUgaG9zdCBzdXBwb3J0cyBpbmRpcmVjdCBkZXNj cmlwdG9yIHRhYmxlcywgYW5kIHdlIGhhdmUgbXVsdGlwbGUKPiA+ICsJICogYnVmZmVycywgdGhl biBnbyBpbmRpcmVjdC4gRklYTUU6IHR1bmUgdGhpcyB0aHJlc2hvbGQgKi8KPiA+ICsJaWYgKHZx LT5pbmRpcmVjdCAmJiB0b3RhbF9zZyA+IDEgJiYgdnEtPnZxLm51bV9mcmVlKQo+IAo+IExldCdz IGludHJvZHVjZSBhIGhlbHBlciBsaWtlIHZpcnRxdWV1ZV9uZWVkX2luZGlyZWN0KCkgdG8gYXZv aWQgZHVwbGljYXRpbmcKPiBjb2RlcyBhbmQgRklYTUUuCgpPa2F5LgoKPiAKPiA+ICsJCWRlc2Mg PSBhbGxvY19pbmRpcmVjdF9wYWNrZWQoX3ZxLCB0b3RhbF9zZywgZ2ZwKTsKPiA+ICsJZWxzZSB7 Cj4gPiArCQlkZXNjID0gTlVMTDsKPiA+ICsJCVdBUk5fT05fT05DRSh0b3RhbF9zZyA+IHZxLT52 cmluZ19wYWNrZWQubnVtICYmICF2cS0+aW5kaXJlY3QpOwo+ID4gKwl9Cj4gPiArCj4gPiArCWlm IChkZXNjKSB7Cj4gPiArCQkvKiBVc2UgYSBzaW5nbGUgYnVmZmVyIHdoaWNoIGRvZXNuJ3QgY29u dGludWUgKi8KPiA+ICsJCWluZGlyZWN0ID0gdHJ1ZTsKPiA+ICsJCS8qIFNldCB1cCByZXN0IHRv IHVzZSB0aGlzIGluZGlyZWN0IHRhYmxlLiAqLwo+ID4gKwkJaSA9IDA7Cj4gPiArCQlkZXNjc191 c2VkID0gMTsKPiA+ICsJfSBlbHNlIHsKPiA+ICsJCWluZGlyZWN0ID0gZmFsc2U7Cj4gPiArCQlk ZXNjID0gdnEtPnZyaW5nX3BhY2tlZC5kZXNjOwo+ID4gKwkJaSA9IGhlYWQ7Cj4gPiArCQlkZXNj c191c2VkID0gdG90YWxfc2c7Cj4gPiArCX0KPiA+ICsKPiA+ICsJaWYgKHZxLT52cS5udW1fZnJl ZSA8IGRlc2NzX3VzZWQpIHsKPiA+ICsJCXByX2RlYnVnKCJDYW4ndCBhZGQgYnVmIGxlbiAlaSAt IGF2YWlsID0gJWlcbiIsCj4gPiArCQkJIGRlc2NzX3VzZWQsIHZxLT52cS5udW1fZnJlZSk7Cj4g PiArCQkvKiBGSVhNRTogZm9yIGhpc3RvcmljYWwgcmVhc29ucywgd2UgZm9yY2UgYSBub3RpZnkg aGVyZSBpZgo+ID4gKwkJICogdGhlcmUgYXJlIG91dGdvaW5nIHBhcnRzIHRvIHRoZSBidWZmZXIu ICBQcmVzdW1hYmx5IHRoZQo+ID4gKwkJICogaG9zdCBzaG91bGQgc2VydmljZSB0aGUgcmluZyBB U0FQLiAqLwo+ID4gKwkJaWYgKG91dF9zZ3MpCj4gPiArCQkJdnEtPm5vdGlmeSgmdnEtPnZxKTsK PiA+ICsJCWlmIChpbmRpcmVjdCkKPiA+ICsJCQlrZnJlZShkZXNjKTsKPiA+ICsJCUVORF9VU0Uo dnEpOwo+ID4gKwkJcmV0dXJuIC1FTk9TUEM7Cj4gPiArCX0KPiA+ICsKPiA+ICsJZm9yIChuID0g MDsgbiA8IG91dF9zZ3MgKyBpbl9zZ3M7IG4rKykgewo+ID4gKwkJZm9yIChzZyA9IHNnc1tuXTsg c2c7IHNnID0gc2dfbmV4dChzZykpIHsKPiA+ICsJCQlkbWFfYWRkcl90IGFkZHIgPSB2cmluZ19t YXBfb25lX3NnKHZxLCBzZywgbiA8IG91dF9zZ3MgPwo+ID4gKwkJCQkJCURNQV9UT19ERVZJQ0Ug OiBETUFfRlJPTV9ERVZJQ0UpOwo+ID4gKwkJCWlmICh2cmluZ19tYXBwaW5nX2Vycm9yKHZxLCBh ZGRyKSkKPiA+ICsJCQkJZ290byB1bm1hcF9yZWxlYXNlOwo+ID4gKwo+ID4gKwkJCWZsYWdzID0g Y3B1X3RvX3ZpcnRpbzE2KF92cS0+dmRldiwgVlJJTkdfREVTQ19GX05FWFQgfAo+ID4gKwkJCQkJ KG4gPCBvdXRfc2dzID8gMCA6IFZSSU5HX0RFU0NfRl9XUklURSkgfAo+ID4gKwkJCQkJVlJJTkdf REVTQ19GX0FWQUlMKHZxLT53cmFwX2NvdW50ZXIpIHwKPiA+ICsJCQkJCVZSSU5HX0RFU0NfRl9V U0VEKCF2cS0+d3JhcF9jb3VudGVyKSk7Cj4gPiArCQkJaWYgKCFpbmRpcmVjdCAmJiBpID09IGhl YWQpCj4gPiArCQkJCWhlYWRfZmxhZ3MgPSBmbGFnczsKPiA+ICsJCQllbHNlCj4gPiArCQkJCWRl c2NbaV0uZmxhZ3MgPSBmbGFnczsKPiA+ICsKPiA+ICsJCQlkZXNjW2ldLmFkZHIgPSBjcHVfdG9f dmlydGlvNjQoX3ZxLT52ZGV2LCBhZGRyKTsKPiA+ICsJCQlkZXNjW2ldLmxlbiA9IGNwdV90b192 aXJ0aW8zMihfdnEtPnZkZXYsIHNnLT5sZW5ndGgpOwo+ID4gKwkJCWRlc2NbaV0uaWQgPSBjcHVf dG9fdmlydGlvMzIoX3ZxLT52ZGV2LCBoZWFkKTsKPiAKPiBTaW1pbGFyIHRvIFYxLCB3ZSBvbmx5 IG5lZWQgdGhpcyBmb3IgdGhlIGxhc3QgZGVzY3JpcHRvci4KCk9rYXksIHdpbGwganVzdCBzZXQg aXQgZm9yIHRoZSBsYXN0IGRlc2MuCgo+IAo+ID4gKwkJCXByZXYgPSBpOwo+IAo+IEl0IGxvb2tz IHRvIG1lIHRoZXJlJ3Mgbm8gbmVlZCB0byB0cmFjayBwcmV2IGluc2lkZSB0aGUgbG9vcCBoZXJl Lgo+IAo+ID4gKwkJCWkrKzsKPiA+ICsJCQlpZiAoIWluZGlyZWN0ICYmIGkgPj0gdnEtPnZyaW5n X3BhY2tlZC5udW0pIHsKPiA+ICsJCQkJaSA9IDA7Cj4gPiArCQkJCXZxLT53cmFwX2NvdW50ZXIg Xj0gMTsKPiA+ICsJCQl9Cj4gPiArCQl9Cj4gPiArCX0KPiA+ICsJLyogTGFzdCBvbmUgZG9lc24n dCBjb250aW51ZS4gKi8KPiA+ICsJaWYgKHRvdGFsX3NnID09IDEpCj4gPiArCQloZWFkX2ZsYWdz ICY9IGNwdV90b192aXJ0aW8xNihfdnEtPnZkZXYsIH5WUklOR19ERVNDX0ZfTkVYVCk7Cj4gPiAr CWVsc2UKPiA+ICsJCWRlc2NbcHJldl0uZmxhZ3MgJj0gY3B1X3RvX3ZpcnRpbzE2KF92cS0+dmRl diwgflZSSU5HX0RFU0NfRl9ORVhUKTsKPiAKPiBUaGUgb25seSBjYXNlIHdoZW4gcHJldiAhPSBp IC0gMSBpcyBpID09IDAsIHdlIGNhbiBhZGQgYSBpZiBoZXJlLgoKSXQncyBqdXN0IGEgbWlycm9y IG9mIHRoZSBleGlzdGluZyBpbXBsZW1lbnRhdGlvbiBpbiBzcGxpdCByaW5nLgpJdCBzZWVtcyB0 aGF0IHNwbGl0IHJpbmcgaW1wbGVtZW50YXRpb24gbmVlZHMgdGhpcyBqdXN0IGJlY2F1c2UKaXQn cyBtdWNoIGhhcmRlciBmb3IgaXQgdG8gZmluZCB0aGUgcHJldiwgd2hpY2ggaXMgbm90IHRydWUg Zm9yCnBhY2tlZCByaW5nLiBTbyBJJ2xsIHRha2UgeW91ciBzdWdnZXN0aW9uLiBUaGFua3MhCgo+ IApbLi4uXQo+ID4gK3N0YXRpYyBib29sIHZpcnRxdWV1ZV9raWNrX3ByZXBhcmVfcGFja2VkKHN0 cnVjdCB2aXJ0cXVldWUgKl92cSkKPiA+ICt7Cj4gPiArCXN0cnVjdCB2cmluZ192aXJ0cXVldWUg KnZxID0gdG9fdnZxKF92cSk7Cj4gPiArCXUxNiBuZXcsIG9sZCwgb2ZmX3dyYXA7Cj4gPiArCWJv b2wgbmVlZHNfa2ljazsKPiA+ICsKPiA+ICsJU1RBUlRfVVNFKHZxKTsKPiA+ICsJLyogV2UgbmVl ZCB0byBleHBvc2UgdGhlIG5ldyBmbGFncyB2YWx1ZSBiZWZvcmUgY2hlY2tpbmcgbm90aWZpY2F0 aW9uCj4gPiArCSAqIHN1cHByZXNzaW9ucy4gKi8KPiA+ICsJdmlydGlvX21iKHZxLT53ZWFrX2Jh cnJpZXJzKTsKPiA+ICsKPiA+ICsJb2xkID0gdnEtPm5leHRfYXZhaWxfaWR4IC0gdnEtPm51bV9h ZGRlZDsKPiA+ICsJbmV3ID0gdnEtPm5leHRfYXZhaWxfaWR4Owo+ID4gKwl2cS0+bnVtX2FkZGVk ID0gMDsKPiA+ICsKPiA+ICsjaWZkZWYgREVCVUcKPiA+ICsJaWYgKHZxLT5sYXN0X2FkZF90aW1l X3ZhbGlkKSB7Cj4gPiArCQlXQVJOX09OKGt0aW1lX3RvX21zKGt0aW1lX3N1YihrdGltZV9nZXQo KSwKPiA+ICsJCQkJCSAgICAgIHZxLT5sYXN0X2FkZF90aW1lKSkgPiAxMDApOwo+ID4gKwl9Cj4g PiArCXZxLT5sYXN0X2FkZF90aW1lX3ZhbGlkID0gZmFsc2U7Cj4gPiArI2VuZGlmCj4gPiArCj4g PiArCW9mZl93cmFwID0gdmlydGlvMTZfdG9fY3B1KF92cS0+dmRldiwgdnEtPnZyaW5nX3BhY2tl ZC5kZXZpY2UtPm9mZl93cmFwKTsKPiA+ICsKPiA+ICsJaWYgKHZxLT5ldmVudCkgewo+IAo+IEl0 IGxvb2tzIHRvIG1lIHdlIHNob3VsZCBleGFtaW5lIFJJTkdfRVZFTlRfRkxBR1NfREVTQyBpbiBk ZXNjX2V2ZW50X2ZsYWdzCj4gaW5zdGVhZCBvZiB2cS0+ZXZlbnQgaGVyZS4gU3BlYyBkb2VzIG5v dCBmb3JjZXMgdG8gdXNlIGV2ZW5mX29mZiBhbmQKPiBldmVudF93cmFwIGlmIGV2ZW50IGluZGV4 IGlzIGVuYWJsZWQuCj4gCj4gPiArCQkvLyBGSVhNRTogZml4IHRoaXMhCj4gPiArCQluZWVkc19r aWNrID0gKChvZmZfd3JhcCA+PiAxNSkgPT0gdnEtPndyYXBfY291bnRlcikgJiYKPiA+ICsJCQkg ICAgIHZyaW5nX25lZWRfZXZlbnQob2ZmX3dyYXAgJiB+KDE8PDE1KSwgbmV3LCBvbGQpOwo+IAo+ IFdoeSBuZWVkIGEgJiBoZXJlPwoKQmVjYXVzZSB3cmFwX2NvdW50ZXIgKHRoZSBtb3N0IHNpZ25p ZmljYW50IGJpdCBpbiBvZmZfd3JhcCkKaXNuJ3QgcGFydCBvZiB0aGUgaW5kZXguCgo+IAo+ID4g Kwl9IGVsc2Ugewo+IAo+IE5lZWQgYSBzbXBfcm1iKCkgdG8gbWFrZSBzdXJlIGRlc2NfZXZlbnRf ZmxhZ3Mgd2FzIGNoZWNrZWQgYmVmb3JlIGZsYWdzLgoKSSBkb24ndCBnZXQgeW91ciBwb2ludCwg aWYgbXkgdW5kZXJzdGFuZGluZyBpcyBjb3JyZWN0LApkZXNjX2V2ZW50X2ZsYWdzIGlzIHZxLT52 cmluZ19wYWNrZWQuZGV2aWNlLT5mbGFncy4gU28Kd2hhdCdzIHRoZSAiZmxhZ3MiPwoKPiAKPiA+ ICsJCW5lZWRzX2tpY2sgPSAodnEtPnZyaW5nX3BhY2tlZC5kZXZpY2UtPmZsYWdzICE9Cj4gPiAr CQkJICAgICAgY3B1X3RvX3ZpcnRpbzE2KF92cS0+dmRldiwgVlJJTkdfRVZFTlRfRl9ESVNBQkxF KSk7Cj4gPiArCX0KPiA+ICsJRU5EX1VTRSh2cSk7Cj4gPiArCXJldHVybiBuZWVkc19raWNrOwo+ ID4gK30KWy4uLl0KPiA+ICtzdGF0aWMgaW50IGRldGFjaF9idWZfcGFja2VkKHN0cnVjdCB2cmlu Z192aXJ0cXVldWUgKnZxLCB1bnNpZ25lZCBpbnQgaGVhZCwKPiA+ICsJCQkgICAgICB2b2lkICoq Y3R4KQo+ID4gK3sKPiA+ICsJc3RydWN0IHZyaW5nX3BhY2tlZF9kZXNjICpkZXNjOwo+ID4gKwl1 bnNpZ25lZCBpbnQgaSwgajsKPiA+ICsKPiA+ICsJLyogQ2xlYXIgZGF0YSBwdHIuICovCj4gPiAr CXZxLT5kZXNjX3N0YXRlW2hlYWRdLmRhdGEgPSBOVUxMOwo+ID4gKwo+ID4gKwlpID0gaGVhZDsK PiA+ICsKPiA+ICsJZm9yIChqID0gMDsgaiA8IHZxLT5kZXNjX3N0YXRlW2hlYWRdLm51bTsgaisr KSB7Cj4gPiArCQlkZXNjID0gJnZxLT52cmluZ19wYWNrZWQuZGVzY1tpXTsKPiA+ICsJCXZyaW5n X3VubWFwX29uZV9wYWNrZWQodnEsIGRlc2MpOwo+ID4gKwkJZGVzYy0+ZmxhZ3MgPSAweDA7Cj4g Cj4gTG9va3MgbGlrZSB0aGlzIGlzIHVubmVjZXNzYXJ5LgoKSXQncyBzYWZlciB0byB6ZXJvIGl0 LiBJZiB3ZSBkb24ndCB6ZXJvIGl0LCBhZnRlciB3ZQpjYWxsIHZpcnRxdWV1ZV9kZXRhY2hfdW51 c2VkX2J1Zl9wYWNrZWQoKSB3aGljaCBjYWxscwp0aGlzIGZ1bmN0aW9uLCB0aGUgZGVzYyBpcyBz dGlsbCBhdmFpbGFibGUgdG8gdGhlCmRldmljZS4KCj4gCj4gPiArCQlpKys7Cj4gPiArCQlpZiAo aSA+PSB2cS0+dnJpbmdfcGFja2VkLm51bSkKPiA+ICsJCQlpID0gMDsKPiA+ICsJfQpbLi4uXQo+ ID4gK3N0YXRpYyB1bnNpZ25lZCB2aXJ0cXVldWVfZW5hYmxlX2NiX3ByZXBhcmVfcGFja2VkKHN0 cnVjdCB2aXJ0cXVldWUgKl92cSkKPiA+ICt7Cj4gPiArCXN0cnVjdCB2cmluZ192aXJ0cXVldWUg KnZxID0gdG9fdnZxKF92cSk7Cj4gPiArCXUxNiBsYXN0X3VzZWRfaWR4LCB3cmFwX2NvdW50ZXIs IG9mZl93cmFwOwo+ID4gKwo+ID4gKwlTVEFSVF9VU0UodnEpOwo+ID4gKwo+ID4gKwlsYXN0X3Vz ZWRfaWR4ID0gdnEtPmxhc3RfdXNlZF9pZHg7Cj4gPiArCXdyYXBfY291bnRlciA9IHZxLT53cmFw X2NvdW50ZXI7Cj4gPiArCj4gPiArCWlmIChsYXN0X3VzZWRfaWR4ID4gdnEtPm5leHRfYXZhaWxf aWR4KQo+ID4gKwkJd3JhcF9jb3VudGVyIF49IDE7Cj4gPiArCj4gPiArCW9mZl93cmFwID0gbGFz dF91c2VkX2lkeCB8ICh3cmFwX2NvdW50ZXIgPDwgMTUpOwo+ID4gKwo+ID4gKwkvKiBXZSBvcHRp bWlzdGljYWxseSB0dXJuIGJhY2sgb24gaW50ZXJydXB0cywgdGhlbiBjaGVjayBpZiB0aGVyZSB3 YXMKPiA+ICsJICogbW9yZSB0byBkby4gKi8KPiA+ICsJLyogRGVwZW5kaW5nIG9uIHRoZSBWSVJU SU9fUklOR19GX0VWRU5UX0lEWCBmZWF0dXJlLCB3ZSBuZWVkIHRvCj4gPiArCSAqIGVpdGhlciBj bGVhciB0aGUgZmxhZ3MgYml0IG9yIHBvaW50IHRoZSBldmVudCBpbmRleCBhdCB0aGUgbmV4dAo+ ID4gKwkgKiBlbnRyeS4gQWx3YXlzIGRvIGJvdGggdG8ga2VlcCBjb2RlIHNpbXBsZS4gKi8KPiA+ ICsJaWYgKHZxLT5ldmVudF9mbGFnc19zaGFkb3cgPT0gVlJJTkdfRVZFTlRfRl9ESVNBQkxFKSB7 Cj4gPiArCQl2cS0+ZXZlbnRfZmxhZ3Nfc2hhZG93ID0gdnEtPmV2ZW50ID8gVlJJTkdfRVZFTlRf Rl9ERVNDOgo+ID4gKwkJCQkJCSAgICAgVlJJTkdfRVZFTlRfRl9FTkFCTEU7Cj4gPiArCQl2cS0+ dnJpbmdfcGFja2VkLmRyaXZlci0+ZmxhZ3MgPSBjcHVfdG9fdmlydGlvMTYoX3ZxLT52ZGV2LAo+ ID4gKwkJCQkJCQl2cS0+ZXZlbnRfZmxhZ3Nfc2hhZG93KTsKPiA+ICsJfQo+IAo+IEEgc21wX3dt YigpIGlzIG1pc3NlZCBoZXJlPwo+IAo+ID4gKwl2cS0+dnJpbmdfcGFja2VkLmRyaXZlci0+b2Zm X3dyYXAgPSBjcHVfdG9fdmlydGlvMTYoX3ZxLT52ZGV2LCBvZmZfd3JhcCk7Cj4gCj4gQW5kIGFj Y29yZGluZyB0byB0aGUgc3BlYywgaXQgbG9va3MgdG8gbWUgd3JpdGUgYSBWUklOR19FVkVOVF9G X0VOQUJMRSBpcwo+IHN1ZmZpY2llbnQgaGVyZS4KCkkgZGlkbid0IHRoaW5rIG11Y2ggd2hlbiBp bXBsZW1lbnRpbmcgdGhlIGV2ZW50IHN1cHByZXNzaW9uCmZvciBwYWNrZWQgcmluZyBwcmV2aW91 c2x5LiBBZnRlciBJIHNhdyB5b3VyIGNvbW1lbnRzLCBJIGZvdW5kCnNvbWV0aGluZyBuZXcuIElu ZGVlZCwgdW5saWtlIHRoZSBzcGxpdCByaW5nLCBmb3IgdGhlIHBhY2tlZApyaW5nLCBzcGVjIGRv ZXNuJ3Qgc2F5IHdlIG11c3QgdXNlIFZSSU5HX0VWRU5UX0ZfREVTQyB3aGVuCkVWRU5UX0lEWCBp cyBuZWdvdGlhdGVkLiBTbyBkbyB5b3UgdGhpbmsgYmVsb3cgdGhvdWdodCBpcwpyaWdodCBvciBt YWtlcyBzZW5zZT8KCi0gRm9yIHZpcnRxdWV1ZV9lbmFibGVfY2JfcHJlcGFyZSgpLCB3ZSBqdXN0 IG5lZWQgdG8gZW5hYmxlCiAgdGhlIHJpbmcgYnkgc2V0dGluZyBmbGFncyB0byBWUklOR19FVkVO VF9GX0VOQUJMRSBpbiBhbnkKICBjYXNlLgoKLSBXZSB3aWxsIHRyeSB0byB1c2UgVlJJTkdfRVZF TlRfRl9ERVNDIChpZiBFVkVOVF9JRFggaXMKICBuZWdvdGlhdGVkKSBvbmx5IHdoZW4gd2Ugd2Fu dCB0byBkZWxheSB0aGUgaW50ZXJydXB0cwogIHZpcnRxdWV1ZV9lbmFibGVfY2JfZGVsYXllZCgp LgoKPiAKPiA+ICsJRU5EX1VTRSh2cSk7Cj4gPiArCXJldHVybiBsYXN0X3VzZWRfaWR4Owo+ID4g K30KPiA+ICsKWy4uLl0KPiA+IEBAIC0xMTU3LDE0ICsxODUyLDE4IEBAIHZvaWQgdnJpbmdfdHJh bnNwb3J0X2ZlYXR1cmVzKHN0cnVjdCB2aXJ0aW9fZGV2aWNlICp2ZGV2KQo+ID4gICAJZm9yIChp ID0gVklSVElPX1RSQU5TUE9SVF9GX1NUQVJUOyBpIDwgVklSVElPX1RSQU5TUE9SVF9GX0VORDsg aSsrKSB7Cj4gPiAgIAkJc3dpdGNoIChpKSB7Cj4gPiAtCQljYXNlIFZJUlRJT19SSU5HX0ZfSU5E SVJFQ1RfREVTQzoKPiA+ICsjaWYgMAo+ID4gKwkJY2FzZSBWSVJUSU9fUklOR19GX0lORElSRUNU X0RFU0M6IC8vIEZJWE1FIG5vdCB0ZXN0ZWQgeWV0Lgo+ID4gICAJCQlicmVhazsKPiA+IC0JCWNh c2UgVklSVElPX1JJTkdfRl9FVkVOVF9JRFg6Cj4gPiArCQljYXNlIFZJUlRJT19SSU5HX0ZfRVZF TlRfSURYOiAvLyBGSVhNRSBwcm9iYWJseSBub3Qgd29yay4KPiA+ICAgCQkJYnJlYWs7Cj4gPiAr I2VuZGlmCj4gCj4gSXQgd291bGQgYmUgYmV0dGVyIGlmIHlvdSBjYW4gc3BsaXQgRVZFTlRfSURY IGFuZCBJTkRJUkVDVF9ERVNDIGludG8KPiBzZXBhcmF0ZSBwYXRjaGVzIHRvby4KClN1cmUuIFdp bGwgZG8gaXQgaW4gdGhlIG5leHQgdmVyc2lvbi4KClRoYW5rcyBmb3IgdGhlIHJldmlldyEKCj4g Cj4gVGhhbmtzCj4gCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fClZpcnR1YWxpemF0aW9uIG1haWxpbmcgbGlzdApWaXJ0dWFsaXphdGlvbkBsaXN0cy5saW51 eC1mb3VuZGF0aW9uLm9yZwpodHRwczovL2xpc3RzLmxpbnV4Zm91bmRhdGlvbi5vcmcvbWFpbG1h bi9saXN0aW5mby92aXJ0dWFsaXphdGlvbg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753724AbeDMHSB (ORCPT ); Fri, 13 Apr 2018 03:18:01 -0400 Received: from mga11.intel.com ([192.55.52.93]:64877 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751162AbeDMHR7 (ORCPT ); Fri, 13 Apr 2018 03:17:59 -0400 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,444,1517904000"; d="scan'208";a="50456390" Date: Fri, 13 Apr 2018 15:15:29 +0800 From: Tiwei Bie To: Jason Wang Cc: mst@redhat.com, wexu@redhat.com, virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, jfreimann@redhat.com Subject: Re: [RFC v2] virtio: support packed ring Message-ID: <20180413071529.f4esh654dakodf4f@debian> References: <20180401141216.8969-1-tiwei.bie@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 13, 2018 at 12:30:24PM +0800, Jason Wang wrote: > On 2018年04月01日 22:12, Tiwei Bie wrote: > > Hello everyone, > > > > This RFC implements packed ring support for virtio driver. > > > > The code was tested with DPDK vhost (testpmd/vhost-PMD) implemented > > by Jens at http://dpdk.org/ml/archives/dev/2018-January/089417.html > > Minor changes are needed for the vhost code, e.g. to kick the guest. > > > > TODO: > > - Refinements and bug fixes; > > - Split into small patches; > > - Test indirect descriptor support; > > - Test/fix event suppression support; > > - Test devices other than net; > > > > RFC v1 -> RFC v2: > > - Add indirect descriptor support - compile test only; > > - Add event suppression supprt - compile test only; > > - Move vring_packed_init() out of uapi (Jason, MST); > > - Merge two loops into one in virtqueue_add_packed() (Jason); > > - Split vring_unmap_one() for packed ring and split ring (Jason); > > - Avoid using '%' operator (Jason); > > - Rename free_head -> next_avail_idx (Jason); > > - Add comments for virtio_wmb() in virtqueue_add_packed() (Jason); > > - Some other refinements and bug fixes; > > > > Thanks! > > > > Signed-off-by: Tiwei Bie > > --- > > drivers/virtio/virtio_ring.c | 1094 +++++++++++++++++++++++++++++------- > > include/linux/virtio_ring.h | 8 +- > > include/uapi/linux/virtio_config.h | 12 +- > > include/uapi/linux/virtio_ring.h | 61 ++ > > 4 files changed, 980 insertions(+), 195 deletions(-) [...] > > +static struct vring_packed_desc *alloc_indirect_packed(struct virtqueue *_vq, > > + unsigned int total_sg, > > + gfp_t gfp) > > +{ > > + struct vring_packed_desc *desc; > > + > > + /* > > + * We require lowmem mappings for the descriptors because > > + * otherwise virt_to_phys will give us bogus addresses in the > > + * virtqueue. > > + */ > > + gfp &= ~__GFP_HIGHMEM; > > + > > + desc = kmalloc(total_sg * sizeof(struct vring_packed_desc), gfp); > > Can we simply check vq->packed here to avoid duplicating helpers? Then it would be something like this: static void *alloc_indirect(struct virtqueue *_vq, unsigned int total_sg, gfp_t gfp) { struct vring_virtqueue *vq = to_vvq(_vq); void *data; /* * We require lowmem mappings for the descriptors because * otherwise virt_to_phys will give us bogus addresses in the * virtqueue. */ gfp &= ~__GFP_HIGHMEM; if (vq->packed) { data = kmalloc(total_sg * sizeof(struct vring_packed_desc), gfp); if (!data) return NULL; } else { struct vring_desc *desc; unsigned int i; desc = kmalloc(total_sg * sizeof(struct vring_desc), gfp); if (!desc) return NULL; for (i = 0; i < total_sg; i++) desc[i].next = cpu_to_virtio16(_vq->vdev, i + 1); data = desc; } return data; } I would prefer to have two simpler helpers (and to the callers, it's already very clear about which one they should call), i.e. the current implementation: static struct vring_packed_desc *alloc_indirect_packed(struct virtqueue *_vq, unsigned int total_sg, gfp_t gfp) { struct vring_packed_desc *desc; /* * We require lowmem mappings for the descriptors because * otherwise virt_to_phys will give us bogus addresses in the * virtqueue. */ gfp &= ~__GFP_HIGHMEM; desc = kmalloc(total_sg * sizeof(struct vring_packed_desc), gfp); return desc; } static struct vring_desc *alloc_indirect_split(struct virtqueue *_vq, unsigned int total_sg, gfp_t gfp) { struct vring_desc *desc; unsigned int i; /* * We require lowmem mappings for the descriptors because * otherwise virt_to_phys will give us bogus addresses in the * virtqueue. */ gfp &= ~__GFP_HIGHMEM; desc = kmalloc(total_sg * sizeof(struct vring_desc), gfp); if (!desc) return NULL; for (i = 0; i < total_sg; i++) desc[i].next = cpu_to_virtio16(_vq->vdev, i + 1); return desc; } > > > + > > + return desc; > > +} [...] > > +static inline int virtqueue_add_packed(struct virtqueue *_vq, > > + struct scatterlist *sgs[], > > + unsigned int total_sg, > > + unsigned int out_sgs, > > + unsigned int in_sgs, > > + void *data, > > + void *ctx, > > + gfp_t gfp) > > +{ > > + struct vring_virtqueue *vq = to_vvq(_vq); > > + struct vring_packed_desc *desc; > > + struct scatterlist *sg; > > + unsigned int i, n, descs_used, uninitialized_var(prev), err_idx; > > + __virtio16 uninitialized_var(head_flags), flags; > > + int head, wrap_counter; > > + bool indirect; > > + > > + START_USE(vq); > > + > > + BUG_ON(data == NULL); > > + BUG_ON(ctx && vq->indirect); > > + > > + if (unlikely(vq->broken)) { > > + END_USE(vq); > > + return -EIO; > > + } > > + > > +#ifdef DEBUG > > + { > > + ktime_t now = ktime_get(); > > + > > + /* No kick or get, with .1 second between? Warn. */ > > + if (vq->last_add_time_valid) > > + WARN_ON(ktime_to_ms(ktime_sub(now, vq->last_add_time)) > > + > 100); > > + vq->last_add_time = now; > > + vq->last_add_time_valid = true; > > + } > > +#endif > > + > > + BUG_ON(total_sg == 0); > > + > > + head = vq->next_avail_idx; > > + wrap_counter = vq->wrap_counter; > > + > > + /* If the host supports indirect descriptor tables, and we have multiple > > + * buffers, then go indirect. FIXME: tune this threshold */ > > + if (vq->indirect && total_sg > 1 && vq->vq.num_free) > > Let's introduce a helper like virtqueue_need_indirect() to avoid duplicating > codes and FIXME. Okay. > > > + desc = alloc_indirect_packed(_vq, total_sg, gfp); > > + else { > > + desc = NULL; > > + WARN_ON_ONCE(total_sg > vq->vring_packed.num && !vq->indirect); > > + } > > + > > + if (desc) { > > + /* Use a single buffer which doesn't continue */ > > + indirect = true; > > + /* Set up rest to use this indirect table. */ > > + i = 0; > > + descs_used = 1; > > + } else { > > + indirect = false; > > + desc = vq->vring_packed.desc; > > + i = head; > > + descs_used = total_sg; > > + } > > + > > + if (vq->vq.num_free < descs_used) { > > + pr_debug("Can't add buf len %i - avail = %i\n", > > + descs_used, vq->vq.num_free); > > + /* FIXME: for historical reasons, we force a notify here if > > + * there are outgoing parts to the buffer. Presumably the > > + * host should service the ring ASAP. */ > > + if (out_sgs) > > + vq->notify(&vq->vq); > > + if (indirect) > > + kfree(desc); > > + END_USE(vq); > > + return -ENOSPC; > > + } > > + > > + for (n = 0; n < out_sgs + in_sgs; n++) { > > + for (sg = sgs[n]; sg; sg = sg_next(sg)) { > > + dma_addr_t addr = vring_map_one_sg(vq, sg, n < out_sgs ? > > + DMA_TO_DEVICE : DMA_FROM_DEVICE); > > + if (vring_mapping_error(vq, addr)) > > + goto unmap_release; > > + > > + flags = cpu_to_virtio16(_vq->vdev, VRING_DESC_F_NEXT | > > + (n < out_sgs ? 0 : VRING_DESC_F_WRITE) | > > + VRING_DESC_F_AVAIL(vq->wrap_counter) | > > + VRING_DESC_F_USED(!vq->wrap_counter)); > > + if (!indirect && i == head) > > + head_flags = flags; > > + else > > + desc[i].flags = flags; > > + > > + desc[i].addr = cpu_to_virtio64(_vq->vdev, addr); > > + desc[i].len = cpu_to_virtio32(_vq->vdev, sg->length); > > + desc[i].id = cpu_to_virtio32(_vq->vdev, head); > > Similar to V1, we only need this for the last descriptor. Okay, will just set it for the last desc. > > > + prev = i; > > It looks to me there's no need to track prev inside the loop here. > > > + i++; > > + if (!indirect && i >= vq->vring_packed.num) { > > + i = 0; > > + vq->wrap_counter ^= 1; > > + } > > + } > > + } > > + /* Last one doesn't continue. */ > > + if (total_sg == 1) > > + head_flags &= cpu_to_virtio16(_vq->vdev, ~VRING_DESC_F_NEXT); > > + else > > + desc[prev].flags &= cpu_to_virtio16(_vq->vdev, ~VRING_DESC_F_NEXT); > > The only case when prev != i - 1 is i == 0, we can add a if here. It's just a mirror of the existing implementation in split ring. It seems that split ring implementation needs this just because it's much harder for it to find the prev, which is not true for packed ring. So I'll take your suggestion. Thanks! > [...] > > +static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq) > > +{ > > + struct vring_virtqueue *vq = to_vvq(_vq); > > + u16 new, old, off_wrap; > > + bool needs_kick; > > + > > + START_USE(vq); > > + /* We need to expose the new flags value before checking notification > > + * suppressions. */ > > + virtio_mb(vq->weak_barriers); > > + > > + old = vq->next_avail_idx - vq->num_added; > > + new = vq->next_avail_idx; > > + vq->num_added = 0; > > + > > +#ifdef DEBUG > > + if (vq->last_add_time_valid) { > > + WARN_ON(ktime_to_ms(ktime_sub(ktime_get(), > > + vq->last_add_time)) > 100); > > + } > > + vq->last_add_time_valid = false; > > +#endif > > + > > + off_wrap = virtio16_to_cpu(_vq->vdev, vq->vring_packed.device->off_wrap); > > + > > + if (vq->event) { > > It looks to me we should examine RING_EVENT_FLAGS_DESC in desc_event_flags > instead of vq->event here. Spec does not forces to use evenf_off and > event_wrap if event index is enabled. > > > + // FIXME: fix this! > > + needs_kick = ((off_wrap >> 15) == vq->wrap_counter) && > > + vring_need_event(off_wrap & ~(1<<15), new, old); > > Why need a & here? Because wrap_counter (the most significant bit in off_wrap) isn't part of the index. > > > + } else { > > Need a smp_rmb() to make sure desc_event_flags was checked before flags. I don't get your point, if my understanding is correct, desc_event_flags is vq->vring_packed.device->flags. So what's the "flags"? > > > + needs_kick = (vq->vring_packed.device->flags != > > + cpu_to_virtio16(_vq->vdev, VRING_EVENT_F_DISABLE)); > > + } > > + END_USE(vq); > > + return needs_kick; > > +} [...] > > +static int detach_buf_packed(struct vring_virtqueue *vq, unsigned int head, > > + void **ctx) > > +{ > > + struct vring_packed_desc *desc; > > + unsigned int i, j; > > + > > + /* Clear data ptr. */ > > + vq->desc_state[head].data = NULL; > > + > > + i = head; > > + > > + for (j = 0; j < vq->desc_state[head].num; j++) { > > + desc = &vq->vring_packed.desc[i]; > > + vring_unmap_one_packed(vq, desc); > > + desc->flags = 0x0; > > Looks like this is unnecessary. It's safer to zero it. If we don't zero it, after we call virtqueue_detach_unused_buf_packed() which calls this function, the desc is still available to the device. > > > + i++; > > + if (i >= vq->vring_packed.num) > > + i = 0; > > + } [...] > > +static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq) > > +{ > > + struct vring_virtqueue *vq = to_vvq(_vq); > > + u16 last_used_idx, wrap_counter, off_wrap; > > + > > + START_USE(vq); > > + > > + last_used_idx = vq->last_used_idx; > > + wrap_counter = vq->wrap_counter; > > + > > + if (last_used_idx > vq->next_avail_idx) > > + wrap_counter ^= 1; > > + > > + off_wrap = last_used_idx | (wrap_counter << 15); > > + > > + /* We optimistically turn back on interrupts, then check if there was > > + * more to do. */ > > + /* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to > > + * either clear the flags bit or point the event index at the next > > + * entry. Always do both to keep code simple. */ > > + if (vq->event_flags_shadow == VRING_EVENT_F_DISABLE) { > > + vq->event_flags_shadow = vq->event ? VRING_EVENT_F_DESC: > > + VRING_EVENT_F_ENABLE; > > + vq->vring_packed.driver->flags = cpu_to_virtio16(_vq->vdev, > > + vq->event_flags_shadow); > > + } > > A smp_wmb() is missed here? > > > + vq->vring_packed.driver->off_wrap = cpu_to_virtio16(_vq->vdev, off_wrap); > > And according to the spec, it looks to me write a VRING_EVENT_F_ENABLE is > sufficient here. I didn't think much when implementing the event suppression for packed ring previously. After I saw your comments, I found something new. Indeed, unlike the split ring, for the packed ring, spec doesn't say we must use VRING_EVENT_F_DESC when EVENT_IDX is negotiated. So do you think below thought is right or makes sense? - For virtqueue_enable_cb_prepare(), we just need to enable the ring by setting flags to VRING_EVENT_F_ENABLE in any case. - We will try to use VRING_EVENT_F_DESC (if EVENT_IDX is negotiated) only when we want to delay the interrupts virtqueue_enable_cb_delayed(). > > > + END_USE(vq); > > + return last_used_idx; > > +} > > + [...] > > @@ -1157,14 +1852,18 @@ void vring_transport_features(struct virtio_device *vdev) > > for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++) { > > switch (i) { > > - case VIRTIO_RING_F_INDIRECT_DESC: > > +#if 0 > > + case VIRTIO_RING_F_INDIRECT_DESC: // FIXME not tested yet. > > break; > > - case VIRTIO_RING_F_EVENT_IDX: > > + case VIRTIO_RING_F_EVENT_IDX: // FIXME probably not work. > > break; > > +#endif > > It would be better if you can split EVENT_IDX and INDIRECT_DESC into > separate patches too. Sure. Will do it in the next version. Thanks for the review! > > Thanks >